Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YANG Model for DHCP DoS Mitigation #18873

Merged
merged 3 commits into from
Oct 17, 2024
Merged

Conversation

ridahanif96
Copy link
Contributor

@ridahanif96 ridahanif96 commented May 5, 2024

Why I did it

Added Support for DHCP rate limit

How I did it

Modifed sonic-port.yang by adding a new leaf for dhcp rate limit

How to verify it

Updated Config DB and YANG model

@ridahanif96 ridahanif96 marked this pull request as ready for review May 6, 2024 07:05
@ridahanif96 ridahanif96 requested a review from qiluo-msft as a code owner May 6, 2024 07:05
@ridahanif96 ridahanif96 changed the title Support for DHCP RATE LIMIT Yang Model YANG Model for DHCP DoS Mitigation May 6, 2024
@lguohan lguohan added the YANG YANG model related changes label May 12, 2024
@ridahanif96
Copy link
Contributor Author

@ganglyu @qiluo-msft Hi, can you please help find reviewer for this PR. Thanks in advance!

@ridahanif96
Copy link
Contributor Author

@ganglyu @qiluo-msft Can you please help review this PR. Thanks in advance.

@asraza07
Copy link

@ganglyu @qiluo-msft kindly help review this PR, thanks in advance!

leaf dhcp_rate_limit {
description "DHCP DOS Mitigation Rate with default value 300";
type uint32 {
range 0..800000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you use 800000 as max value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no specific max standard range for DHCP DOS Range. We added a reasonably large maximum range vale to cater all device rate limit scenarios.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that we don't really need this range.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it can work without range as well. If suggested, we can remove range.

@ganglyu ganglyu requested a review from yaqiangz June 13, 2024 06:59
}
},

"PORT_INVALID_DHCP_RATE_LIMIT": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case will fail

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, Will modify existing testcases

Copy link
Contributor

@ganglyu ganglyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
sonic-net/SONiC#1651
HLD is not approved, please wait for other reviewers

@ridahanif96
Copy link
Contributor Author

Hi @ganglyu can you pls help merge this PR. HLD is already merged

@ganglyu
Copy link
Contributor

ganglyu commented Aug 23, 2024

Hi @ganglyu can you pls help merge this PR. HLD is already merged

I'm not authorized to merge this PR.

@ridahanif96
Copy link
Contributor Author

Hi @qiluo-msft can you pls help review and merge this PR.

@qiluo-msft
Copy link
Collaborator

@yaqiangz Could you help review?

Copy link
Contributor

@yaqiangz yaqiangz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ridahanif96
Copy link
Contributor Author

@qiluo-msft pls help merge this one

@ridahanif96
Copy link
Contributor Author

@qiluo-msft pls help review and merge this PR

@qiluo-msft qiluo-msft merged commit 6d4bdb2 into sonic-net:master Oct 17, 2024
20 checks passed
aidan-gallagher pushed a commit to aidan-gallagher/sonic-buildimage that referenced this pull request Nov 16, 2024
#### Why I did it
Added Support for DHCP rate limit

#### How I did it
Modifed sonic-port.yang by adding a new leaf for dhcp rate limit

#### How to verify it
Updated Config DB and YANG model
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
YANG YANG model related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants