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.

Sign up for GitHub

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

[202012] Fix arp_update script #8412

Merged
merged 2 commits into from
Aug 13, 2021
Merged

[202012] Fix arp_update script #8412

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
897af31
[202012] Fix arp_update script
vmorokhx Aug 10, 2021
5b02057
Add timeout to ping6cmd command
vmorokhx Aug 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions files/scripts/arp_update
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ while /bin/true; do
fi
done

sleep 1
Copy link
Contributor

Choose a reason for hiding this comment

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

From your description, I feel the problem here is the ping6 command could get stuck. Adding a wait might not completely solve the problem since there could still be some cases (e.g., the wait time is not enough, some improper vlan config) where it may get stuck. Maybe we can do something like adding a few seconds timeout to ping6cmd so that the script would not hang with the command? @prsunny what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree with Shi, you can refer line 70 for adding timeout.

Copy link
Contributor Author

@vmorokhx vmorokhx Aug 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Added a one second timeout to ping6 command. If we increase the timeout it can lead to a problem when we have a lot of VLAN's on device. Because each ping to VLAN will be with timeout, now time that we spend on wait is: number of VLAN's * timeout (by now one second).

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is OK since the timeout is not expected to wait every time. For example, if the VLANs come up in one second (as the sleep you added in the previous commit), only the first one or two would have the timeout.


VLAN=$(echo $ARP_UPDATE_VARS | jq -r '.vlan')
for vlan in $VLAN; do
# generate a list of arping commands:
Expand Down