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

Utilities Changes for DHCP DoS Mitigation Feature #3301

Open
wants to merge 108 commits into
base: master
Choose a base branch
from

Conversation

asraza07
Copy link

@asraza07 asraza07 commented May 2, 2024

What I did

Added config interface dhcp-mitigation-rate add/del and show interfaces dhcp-mitigation-rate commands for DHCP mitigation rate attribute in config_db and added support for new attribute in DB Migrator

How I did it

Add/del commands add/remove interface DHCP rate limits defined by the user to PORT_TABLE in config_db and show command reads and displays DHCP rate limits on interfaces in the form of a table

@asraza07 asraza07 changed the title Support for DHCP DoS Mitigation CLI Utilities Changes for DHCP DoS Mitigation Feature May 6, 2024
@asraza07 asraza07 marked this pull request as ready for review May 6, 2024 07:00
@ridahanif96
Copy link
Contributor

@yaqiangz Can you help approved this PR. We are also done with Code Coverage. Thanks in advance!

@ridahanif96
Copy link
Contributor

@qiluo-msft Pls help review this PR. HLD Already Approved. Thanks in advance!

@ridahanif96
Copy link
Contributor

Hi @qiluo-msft can you pls help review this PR. HLD is merged

@@ -263,6 +263,13 @@
PortChannel1001 trunk 4000
"""

show_dhcp_rate_limit_in_alias_mode_output = """\
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this var used?

Choose a reason for hiding this comment

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

The var "show_dhcp_rate_limit_in_alias_mode_output" was added mistakenly and isn't used in the current code. I have removed it to keep the code clean

yaqiangz
yaqiangz previously approved these changes Sep 10, 2024
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

@qiluo-msft pls help merge this one

@ridahanif96
Copy link
Contributor

@qiluo-msft please help review and merge this PR

@qiluo-msft
Copy link
Contributor

Please resolve conflict

@ridahanif96
Copy link
Contributor

Please resolve conflict

Resolving! Thanks for help

@ridahanif96
Copy link
Contributor

/azpw run

1 similar comment
@asraza07
Copy link
Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ridahanif96
Copy link
Contributor

@qiluo-msft conflicts has been resolved can you please help review and merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants