SwSS Changes for DHCP DoS Mitigation Feature #3130
Conversation
asraza07
changed the title
asraza07
changed the title
|
@Yarden-Z Hi Yarden, Can you please help review this code. Thanks in advance! |
muhammadalihussnain
force-pushed
the
dhcp_dos_mitigation_swss
branch
from
b761b27
to
d22882a
Compare
|
Hi @prsunny, could you please help us by reviewing the code? |
i have to go to previous commit and to do work and then come back new work into my original brnach
orchagent/port/porthlpr.cpp
Outdated
| @@ -667,6 +667,7 @@ bool PortHelper::parsePortLinkTraining(PortConfig &port, const std::string &fiel | |||
| return true; | |||
| } | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Can you please revert the changes that are not part of this PR? like this extra line?
There was a problem hiding this comment.
We removed the extra lines that were added during the coding process.
| << TC_CMD << " filter add dev " << shellquote(alias) << " protocol ip parent ffff: prio 1 u32 match ip protocol 17 0xff match ip dport 67 0xffff police rate " << to_string(byte_rate) << "bps burst " << to_string(byte_rate) << "b conform-exceed drop"; | ||
| cmd_str = cmd.str(); | ||
| ret = swss::exec(cmd_str, res); | ||
| SWSS_LOG_WARN("ret Value in setPortDHCPMitigationRate is ret:%d,", ret); |
There was a problem hiding this comment.
Why is this log warn? Seems it doesn't mean unexpected situation?
There was a problem hiding this comment.
the log SWSS_LOG_WARN("ret Value in setPortDHCPMitigationRate is ret:%d,", ret); was added temporarily for debugging purposes to track failures and will be removed.
orchagent/port/porthlpr.cpp
Outdated
| @@ -1159,13 +1160,8 @@ bool PortHelper::parsePortConfig(PortConfig &port) const | |||
| return false; | |||
| } | |||
| } | |||
| else if (field == PORT_DESCRIPTION) | |||
There was a problem hiding this comment.
this part was removed by mistake
| @@ -138,7 +138,7 @@ def _verify_db_contents(): | |||
| return (True, None) | |||
|
|
|||
| # Verify that ASIC DB has been fully initialized | |||
| init_polling_config = PollingConfig(2, 30, strict=True) | |||
| init_polling_config = PollingConfig(2, 60, strict=True) | |||
There was a problem hiding this comment.
Why we change this value from 30 to 60?
There was a problem hiding this comment.
The timeout duration was increased after analyzing the failures and identifying that the issue might be related to initialization timing. Adding a delay helped resolve the problem, and the build was successful. However, when these delays were removed, the dvs_test failed again, indicating that the delays are necessary for the DVS to be ready.
| @@ -1848,6 +1856,7 @@ def update_dvs(log_path, new_dvs_env=[]): | |||
| dvs.destroy_servers() | |||
| dvs.create_servers() | |||
| dvs.restart() | |||
| time.sleep(60) | |||
There was a problem hiding this comment.
I added a delay here to ensure that once the DVS switch starts, there is a pause before it performs any further actions, allowing sufficient time for the ports to initialize properly.
| @@ -79,7 +79,7 @@ def test_remove_add_remove_port_with_buffer_cfg(self, dvs, testlog): | |||
| 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 = 30.00, strict = True)) | |||
There was a problem hiding this comment.
The timeout duration was increased after analyzing the failures and identifying that the issue might be related to initialization timing. Adding a delay helped resolve the problem, and the build was successful. However, when these delays were removed, the dvs_test failed again, indicating that the delays are necessary for the DVS to be ready.
|
@Blueve, @dgsudharsan, @prabhataravind . could you please help us by reviewing the code? |
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