SwSS Changes for DHCP DoS Mitigation Feature #3130

asraza07 · 2024-05-02T10:43:16Z

What I did
Added support for DHCP DoS Mitigation feature via kernel through PortMgrd
Why I did it
DHCP Mitigation Rate attribute added to config_db needed a path to appl_db and kernel configurations. This was done through PortMgrd

ridahanif96 · 2024-05-14T12:24:19Z

@Yarden-Z Hi Yarden, Can you please help review this code. Thanks in advance!

prabhataravind · 2024-12-04T18:44:37Z

tests/conftest.py

@@ -231,7 +231,7 @@ def __init__(self, ctn_name: str, pid: int, i: int):
            # bring up link in the virtual switch
            ensure_system(f"nsenter -t {pid} -n ip link set dev {self.pifname} up")

-            # disable arp, so no neigh on physical interfaces
+            # disable arp, so no neigh on physical interfaces0 / 21 files viewed


The comment is not right. Please fix

prabhataravind · 2024-12-04T18:45:02Z

tests/conftest.py

@@ -1432,7 +1432,7 @@ def get_state_db(self) -> DVSDatabase:
        return self.state_db

    def change_port_breakout_mode(self, intf_name, target_mode, options=""):
-        cmd = f"config interface breakout {intf_name} {target_mode} -y {options}"
+        cmd = f"config interface breakout {intf_name} {target_mode} -y {options}"py


what is the additional "py" at the end?

this py was added mistakenly

prabhataravind · 2024-12-04T18:45:46Z

tests/dvslib/dvs_database.py

@@ -146,7 +146,7 @@ def wait_for_entry(
        """Wait for the entry stored at `key` in the specified table to exist and retrieve it.

        Args:
-            table_name: The name of the table where the entry is stored.
+            table_name: The name of the table where theentry is stored.


Please fix stray characters

prabhataravind · 2024-12-04T18:46:04Z

tests/dvslib/dvs_database.py

@@ -251,7 +248,7 @@ def access_function():

        return result

-    def wait_for_field_negative_match(
+    def wait_for_field_negative_match(


Please fix stray characters

prabhataravind · 2024-12-04T18:46:25Z

tests/test_interface.py

@@ -220,7 +220,7 @@ def test_PortInterfaceAddRemoveIpv6Address(self, dvs, testlog):
            if route["dest"] == "fc00::/126":
                assert False
            if route["dest"] == "fc00::1/128":
-                assert False
+                assert False   


Fix trailing whitespaces

prabhataravind · 2024-12-04T18:48:00Z

cfgmgr/portmgr.cpp

                else if (fvField(i) == "admin_status")
                {
                    admin_status = fvValue(i);
-                }
+                }               


Fix trailing spaces

prabhataravind

There are still many places with incorrect formatting and stray characters. Please review everything and confirm everything looks correct before you post the changes to be reviewed again.

prabhataravind

The latest set of changes lgtm. This branch seems to be out-of-date with master. Could you please fix? Once done, please request @prsunny to help merge if there are no other concerns.

mssonicbld · 2024-12-12T07:15:08Z

/azp run

azure-pipelines · 2024-12-12T07:15:20Z

Azure Pipelines successfully started running 1 pipeline(s).

muhammadalihussnain · 2024-12-12T18:35:37Z

@prsunny , PR has been approved. Please help to merge code PR. Thanks in advance

ridahanif96 · 2024-12-14T20:05:52Z

@prsunny please help merge this PR, pending for long!Thanks!

mssonicbld · 2024-12-14T20:19:51Z

/azp run

azure-pipelines · 2024-12-14T20:20:03Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2024-12-16T05:36:46Z

Commenter does not have sufficient privileges for PR 3130 in repo sonic-net/sonic-swss

azure-pipelines · 2024-12-16T05:37:29Z

Commenter does not have sufficient privileges for PR 3130 in repo sonic-net/sonic-swss

prsunny · 2024-12-19T22:20:53Z

@asraza07 , can you resolve conflicts? Also Please provide the merge sequence. Does this PR has any dependency on sonic-buildimage PR.

prsunny · 2024-12-19T22:25:51Z

cfgmgr/portmgr.cpp

+            }            
+            if (!dhcp_rate_limit.empty())
+            {
+                setPortDHCPMitigationRate(alias, dhcp_rate_limit);


This should be set only if there is explicit configuration of dhcp_rate_limit. It should stay the same as today (no kernel TC) if there is no config.

prsunny · 2024-12-19T22:26:44Z

tests/test_fgnhg.py

@@ -283,6 +284,7 @@ def create_interface_n_fg_ecmp_config(dvs, nh_range_start, nh_range_end, fg_nhg_
        dvs.port_admin_set(if_name_key, "up")
        shutdown_link(dvs, app_db, i)
        startup_link(dvs, app_db, i)
+        time.sleep(5)


Please don't change any tests not part of this feature

prsunny · 2024-12-19T22:27:03Z

tests/test_interface.py

@@ -2293,4 +2299,4 @@ def test_portChannelInterfaceLoopbackActionForward(self, dvs, testlog):
 # Add Dummy always-pass test at end as workaroud
 # for issue when Flaky fail on final test it invokes module tear-down before retrying
 def test_nonflaky_dummy():
-    pass
+    pass


Revert non relevant changes.

prsunny · 2024-12-19T22:27:25Z

tests/test_port_add_remove.py

        # try to remove this port
        config_db.delete_entry('PORT', PORT_A)
        num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT",
                              num_of_ports-1,
-                              polling_config = PollingConfig(polling_interval = 1, timeout = 5.00, strict = True))
+                              polling_config = PollingConfig(polling_interval = 1, timeout = 10.00, strict = True))


What is this change for?

prabhataravind

Please fix Prince's comment about calling setPortDHCPMitigationRate when there is no explicit configuration.

asraza07 added 8 commits May 2, 2024 15:37

Add support for DHCP mitigation rate feature via kernel

5a8b39f

Fix for typo in portmgr.h

0fb822a

Fix for std namespace

dc20bf1

Fix for typo

903a89d

Fix for unused variable

132a6e0

Fix for failing portmgr test cases

312356d

Remove write dhcp rate to appl_db

eb52c01

Fix for failed test case assertion

4d10719

asraza07 changed the title ~~Support for DHCP DoS Mitigation feature~~ SwSS Chnages for DHCP DoS Mitigation Feature May 6, 2024

Merge branch 'master' into dhcp_dos_mitigation_swss

6ec063c

asraza07 marked this pull request as ready for review May 6, 2024 07:01

asraza07 requested a review from prsunny as a code owner May 6, 2024 07:01

muhammadalihussnain mentioned this pull request May 6, 2024

DHCP DOS Mitigation HLD in SONiC sonic-net/SONiC#1651

Merged

asraza07 changed the title ~~SwSS Chnages for DHCP DoS Mitigation Feature~~ SwSS Changes for DHCP DoS Mitigation Feature May 7, 2024

asraza07 and others added 6 commits May 7, 2024 17:38

Dhcp rate changes to Appl Db

703e8a5

changed /sbin/tc to /usr/sbin/tc

79fea07

Write DHCP rate to appldb

5279d61

Portmgr test case fix for appldb addition of DHP rate

c89f3d4

Fix for typo

d167ade

Fix for TC command path

b761b27

Changed set rate and erase delete rate

d22882a

muhammadalihussnain force-pushed the dhcp_dos_mitigation_swss branch from b761b27 to d22882a Compare May 16, 2024 10:17

muhammadalihussnain added 7 commits May 16, 2024 10:44

/usr/sbin/tc to /sbin/tc

893c800

not deleting queue

462c777

erased config->appl DBs

768e1c9

changed to_string(byte_rate)

01331df

Added sudo to it

19cdea5

removed comment from cmd_str=cmd.str()

4c2135a

insert sudo to original command

d5cf62b

prabhataravind reviewed Dec 4, 2024

View reviewed changes

prabhataravind suggested changes Dec 4, 2024

View reviewed changes

muhammadalihussnain added 5 commits December 4, 2024 19:10

resolving formating issues

4aa6953

Merge branch 'sonic-net:master' into dhcp_dos_mitigation_swss

eac36b8

trainling white spaces

d28829a

removed format mistakes

4443dd8

Merge branch 'master' into dhcp_dos_mitigation_swss

76a85d0

prabhataravind approved these changes Dec 11, 2024

View reviewed changes

muhammadalihussnain added 2 commits December 12, 2024 11:45

Merge branch 'sonic-net:master' into dhcp_dos_mitigation_swss

18dec77

updating branch

e11f7da

Merge branch 'master' into dhcp_dos_mitigation_swss

bb8b3de

prsunny reviewed Dec 19, 2024

View reviewed changes

prabhataravind suggested changes Dec 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SwSS Changes for DHCP DoS Mitigation Feature #3130

SwSS Changes for DHCP DoS Mitigation Feature #3130

asraza07 commented May 2, 2024

ridahanif96 commented May 14, 2024

prabhataravind Dec 4, 2024

muhammadalihussnain Dec 4, 2024

muhammadalihussnain Dec 4, 2024

prabhataravind Dec 4, 2024

muhammadalihussnain Dec 4, 2024

prabhataravind Dec 4, 2024

muhammadalihussnain Dec 4, 2024

prabhataravind Dec 4, 2024

muhammadalihussnain Dec 4, 2024

prabhataravind Dec 4, 2024

muhammadalihussnain Dec 4, 2024

prabhataravind Dec 4, 2024

muhammadalihussnain Dec 4, 2024

prabhataravind left a comment

prabhataravind left a comment

mssonicbld commented Dec 12, 2024

azure-pipelines bot commented Dec 12, 2024

muhammadalihussnain commented Dec 12, 2024

ridahanif96 commented Dec 14, 2024

mssonicbld commented Dec 14, 2024

azure-pipelines bot commented Dec 14, 2024

azure-pipelines bot commented Dec 16, 2024

azure-pipelines bot commented Dec 16, 2024

prsunny commented Dec 19, 2024

prsunny Dec 19, 2024

prsunny Dec 19, 2024

prsunny Dec 19, 2024

prsunny Dec 19, 2024

prabhataravind left a comment

SwSS Changes for DHCP DoS Mitigation Feature #3130

Are you sure you want to change the base?

SwSS Changes for DHCP DoS Mitigation Feature #3130

Conversation

asraza07 commented May 2, 2024

ridahanif96 commented May 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prabhataravind left a comment

Choose a reason for hiding this comment

prabhataravind left a comment

Choose a reason for hiding this comment

mssonicbld commented Dec 12, 2024

azure-pipelines bot commented Dec 12, 2024

muhammadalihussnain commented Dec 12, 2024

ridahanif96 commented Dec 14, 2024

mssonicbld commented Dec 14, 2024

azure-pipelines bot commented Dec 14, 2024

azure-pipelines bot commented Dec 16, 2024

azure-pipelines bot commented Dec 16, 2024

prsunny commented Dec 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prabhataravind left a comment

Choose a reason for hiding this comment