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

[swss]: Wait for vlan intf to start ndppd #10119

Merged
merged 2 commits into from
Mar 3, 2022

Conversation

theasianpianist
Copy link
Contributor

Second try for PR #10036 (had to be reverted due to a bug)

Why I did it

If the VLAN interface is not up when ndppd starts, it will fail to enable allmulti mode on the interface and be unable to process received NDP packets

The following logs are seen:

/var/log/syslog.33.gz:Feb 18 10:33:12.825406 sonic INFO swss#/supervisord: ndppd (error) Failed to set allmulti: No such device

How I did it

Use the wait_for_link script currently used by radv to delay ndppd startup until the vlan interface is ready

How to verify it

Apply the changes to a device. config reload the device and confirm that the above error logs are not observed when ndppd starts. Run the arp/test_arp_dualtor.py::test_proxy_arp test case and verify it passes.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
- Make wait_for_link.sh executable
- Break ndppd supervisord configs into separate file
    - Allows us to start ndppd only when VLAN table is present

Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
@theasianpianist theasianpianist merged commit 4d2a55d into sonic-net:master Mar 3, 2022
@qiluo-msft
Copy link
Collaborator

This commit could not be cleanly cherry-picked to 202012. Please submit another PR.

theasianpianist added a commit to theasianpianist/sonic-buildimage that referenced this pull request Mar 3, 2022
- Use the `wait_for_link.sh` script to delay ndppd start until after the VLAN interface is ready
- Avoids issue where ndppd tries to change interface attributes before the interface is ready
lguohan pushed a commit that referenced this pull request Mar 5, 2022
202012 version of #10119

Why I did it
If the VLAN interface is not up when ndppd starts, it will fail to enable allmulti mode on the interface and be unable to process received NDP packets

The following logs are seen:

/var/log/syslog.33.gz:Feb 18 10:33:12.825406 sonic INFO swss#/supervisord: ndppd (error) Failed to set allmulti: No such device

How I did it
Use the wait_for_link script currently used by radv to delay ndppd startup until the vlan interface is ready

How to verify it
Apply the changes to a device. config reload the device and confirm that the above error logs are not observed when ndppd starts. Run the arp/test_arp_dualtor.py::test_proxy_arp test case and verify it passes.
zjswhhh pushed a commit that referenced this pull request Mar 7, 2022
202012 version of #10119

Why I did it
If the VLAN interface is not up when ndppd starts, it will fail to enable allmulti mode on the interface and be unable to process received NDP packets

The following logs are seen:

/var/log/syslog.33.gz:Feb 18 10:33:12.825406 sonic INFO swss#/supervisord: ndppd (error) Failed to set allmulti: No such device

How I did it
Use the wait_for_link script currently used by radv to delay ndppd startup until the vlan interface is ready

How to verify it
Apply the changes to a device. config reload the device and confirm that the above error logs are not observed when ndppd starts. Run the arp/test_arp_dualtor.py::test_proxy_arp test case and verify it passes.
@gord1306
Copy link

gord1306 commented Mar 28, 2022

Hi @theasianpianist @prsunny, there was a similar PR 9476 that I created before to check PortInitDone, what I was thinking is whether the XXXsyncd/XXXmgrd should not be started until the PortInitDone being presented?

@theasianpianist
Copy link
Contributor Author

Hi @theasianpianist @prsunny, there was a similar PR 9476 that I created before to check PortInitDone, what I was thinking is whether the XXXsyncd/XXXmgrd should not be started until the PortInitDone being presented?

@prsunny can you comment on this?

@prsunny
Copy link
Contributor

prsunny commented Mar 30, 2022

Hi @theasianpianist @prsunny, there was a similar PR 9476 that I created before to check PortInitDone, what I was thinking is whether the XXXsyncd/XXXmgrd should not be started until the PortInitDone being presented?

@prsunny can you comment on this?

This change seems to be impactful and has to be tested with warmboot scenarios etc before confirming.

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.

4 participants