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

[arp] copy arp IO to cpu instead of trap and drop #812

Merged
merged 1 commit into from
May 6, 2019

Conversation

yxieca
Copy link
Contributor

@yxieca yxieca commented Mar 8, 2019

What I did
Switch arp/arp response/neighbor_discovery IO trap action from 'trap' to 'copy'. These IO, instead of trap to CPU and dropped, they will be copy to CPU and forwarded on VLAN.

Why I did it
During warm reboot, we need VLAN take care of ARP traffic by itself.

How I verified it
Tested with matching test: sonic-net/sonic-mgmt#824

During warm reboot, we need VLAN take care of ARP traffic by itself.

Signed-off-by: Ying Xie <[email protected]>
@prsunny
Copy link
Collaborator

prsunny commented Mar 10, 2019

Just to highlight, there will be now two copies of ARP requests forwarded. One flooded by ASIC and other forwarded by kernel. There is no functional impact but a minor behavioral change.

@yxieca
Copy link
Contributor Author

yxieca commented Mar 12, 2019

Thanks for the clarification, Prince!

@@ -22,7 +22,7 @@
{
"COPP_TABLE:trap.group.arp": {
"trap_ids": "arp_req,arp_resp,neigh_discovery",
"trap_action":"trap",
"trap_action":"copy",
Copy link
Contributor

@andriymoroz-mlnx andriymoroz-mlnx Mar 13, 2019

Choose a reason for hiding this comment

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

this crashes on Mellanox switches
please do not merge yet, will clarify with SAI
UPD:
https://github.com/opencomputeproject/SAI/blob/v1.3/inc/saihostif.h#L416
only "trap" and "log" are valid for host interface

Copy link

Choose a reason for hiding this comment

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

I think this change creates a loop for ICMPv6 RS & NS packets in an L2 MC-LAG scenario as i documented here:
sonic-net/SONiC#1253

@lguohan
Copy link
Contributor

lguohan commented Mar 20, 2019

this is used for warm reboot only, so we should only do the change before warm reboot, and change it back after warm reboot.

@yxieca yxieca merged commit 5e4b71d into sonic-net:master May 6, 2019
@yxieca yxieca deleted the arp branch May 6, 2019 17:09
yxieca added a commit that referenced this pull request May 6, 2019
During warm reboot, we need VLAN take care of ARP traffic by itself.

Signed-off-by: Ying Xie <[email protected]>
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
…init_cfg.json implicitly (sonic-net#812)

* [config/main.py] Modify the load() and reload() functions to load config
from config_db.json and init_cfg.json.

Signed-off-by: Yong Zhao <[email protected]>

* [config/main.py] Undo the changes which load the configuration from
init_cfg.json for load() function and define a
constant string for the path of init_cfg.json.

Signed-off-by: Yong Zhao <[email protected]>

* [config/main.py] Correct a typo.

Signed-off-by: Yong Zhao <[email protected]>

* [config/main.py] Added an else statement in line 551 to decide whether
the init_cfg.json exsits or not in reload function.

Signed-off-by: Yong Zhao <[email protected]>

* [config/main.py] Correct a typo error.

Signed-off-by: Yong Zhao <[email protected]>

* [config/main.py] Change the loading order and we should first load
init_cfg.json and then config_db.json.

Signed-off-by: Yong Zhao <[email protected]>

* [config/main.py] Use constant string to represent the path of
init_cfg.json.

Signed-off-by: Yong Zhao <[email protected]>
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
This will remove possible deadlock when notification arrives during
communication channel destructor thread join.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants