Skip to content

[swss]: Restore cached FDB and ARP entries after config reload for dual ToR #6912

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

Closed
theasianpianist wants to merge 6 commits into from
Closed

[swss]: Restore cached FDB and ARP entries after config reload for dual ToR #6912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
049b833
[docker_image_ctl]: Copy cached ARP tables to SWSS docker
theasianpianist Feb 25, 2021
b069745
[docker-orchagent]: Create process to restore arp to kernel
theasianpianist Feb 26, 2021
d58bae8
[swss]: Address review comments:
theasianpianist Mar 1, 2021
566d40a
[kernel_arp_restore]: Add newline at EOF
theasianpianist Mar 1, 2021
dcebde3
[swss]: Search for NEIGH_TABLE key before restoring
theasianpianist Mar 2, 2021
afc3ce5
[swssconfig]: Increase code re-use
theasianpianist Mar 9, 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: 1 addition & 1 deletion dockers/docker-orchagent/Dockerfile.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ COPY ["files/arp_update", "/usr/bin"]
COPY ["arp_update.conf", "files/arp_update_vars.j2", "/usr/share/sonic/templates/"]
COPY ["ndppd.conf", "/usr/share/sonic/templates/"]
COPY ["enable_counters.py", "/usr/bin"]
COPY ["docker-init.sh", "orchagent.sh", "swssconfig.sh", "buffermgrd.sh", "/usr/bin/"]
COPY ["docker-init.sh", "orchagent.sh", "swssconfig.sh", "buffermgrd.sh", "kernel_arp_restore.sh", "/usr/bin/"]
COPY ["supervisord.conf", "/etc/supervisor/conf.d/"]
COPY ["files/supervisor-proc-exit-listener", "/usr/bin"]
COPY ["critical_processes", "/etc/supervisor/"]
Expand Down
49 changes: 49 additions & 0 deletions dockers/docker-orchagent/kernel_arp_restore.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#!/usr/bin/env bash

function wait_for_intf_up {
INTF_NAME=$1

until [[ `ip link show $INTF_NAME | grep 'state UP'` ]]; do
sleep 1;
done
}

function restore_arp_to_kernel {
ARP_FILE='/arp.json.1'
NUM_ENTRIES=`jq 'length' $ARP_FILE`

for i in $( seq 0 $(($NUM_ENTRIES - 1)) ); do

# For the ith object, get the first key
# 'jq' sorts the keys by default so this should always be
# the 'NEIGH_TABLE' key, not the 'OP' key
NUM_KEYS=`jq ".[$i] | keys | length" $ARP_FILE`
for j in $( seq 0 $(($NUM_KEYS - 1)) ); do
if [[ `jq ".[$i] | keys[$j] | startswith(\"NEIGH_TABLE\")" $ARP_FILE` == 'true' ]]; then
KEY=`jq ".[$i] | keys[$j]" $ARP_FILE`
break
fi
done
# For all 'jq' commands below, use '-r' for raw output
# to prevent double quoting

# For the object associated with the 'NEIGH_TABLE' key
# store the value of the 'neigh' field (the MAC address)
MAC=`jq -r ".[$i][$KEY][\"neigh\"]" $ARP_FILE`

# Split the 'NEIGH_TABLE' key with delimiter ':' and take the
# second item from the result array which is the device name
DEVICE=`echo $KEY | jq -r ". / \":\" | .[1]"`

# Same as for VLAN, but take the third item which is the IP
IP=`echo $KEY | jq -r ". / \":\" | .[2]"`

wait_for_intf_up $DEVICE
ip neigh replace "$IP" dev "$DEVICE" lladdr "$MAC" nud stale
Copy link
Copy Markdown
Contributor

@prsunny prsunny Mar 1, 2021

Choose a reason for hiding this comment

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

if we are restoring IPv6 entries, this should be "ip -6 neigh ..."

Copy link
Copy Markdown
Contributor Author

@theasianpianist theasianpianist Mar 1, 2021

Choose a reason for hiding this comment

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

I thought ip neigh by default could accept both IPv4 and IPv6 entries?

done
}

if [[ -f /restore-kernel ]]; then
restore_arp_to_kernel
rm -f /restore-kernel
fi
10 changes: 10 additions & 0 deletions dockers/docker-orchagent/supervisord.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,16 @@ stderr_logfile=syslog
dependent_startup=true
dependent_startup_wait_for=orchagent:running

[program:kernel_arp_restore]
Copy link
Copy Markdown
Collaborator

@lguohan lguohan Mar 5, 2021

Choose a reason for hiding this comment

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

what is the different between kernel_arp_restore and restore_neighbors below? it seems the similiar things.

Copy link
Copy Markdown
Contributor

@prsunny prsunny Mar 5, 2021

Choose a reason for hiding this comment

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

restore_neighbors is executed only during warmboot and restoration is by programming and sending arp requests. kernel_arp_restore is only programming to kernel (as dynamic entries) and done for config_reload scenario. Do you prefer we should combine the script?

Copy link
Copy Markdown
Collaborator

@lguohan lguohan Mar 5, 2021

Choose a reason for hiding this comment

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

i recall for fast reboot we also restore the arp entries, which script is that? do we restart kernel arp entries for fast reboot?

Copy link
Copy Markdown
Contributor

@prsunny prsunny Mar 5, 2021

Choose a reason for hiding this comment

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

AFAIK, for fastboot, we only restore to APP_DB so that orchangent programs those ARP entry to ASIC and the arp_update script will send out requests to resolve arp.

Copy link
Copy Markdown
Contributor Author

@theasianpianist theasianpianist Mar 5, 2021

Choose a reason for hiding this comment

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

It looks like restore_neighbors is used primarily during warmboot. It looks like it does accomplish the same thing, but I am hesitant to change anything warmboot related in case it has other side effects.

Copy link
Copy Markdown
Collaborator

@lguohan lguohan Mar 5, 2021

Choose a reason for hiding this comment

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

why fast_reboot cannot use the this kernel_arp_restore script? why only use this script in config reload scenario?

Copy link
Copy Markdown
Collaborator

@lguohan lguohan Mar 5, 2021

Choose a reason for hiding this comment

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

it is fine not to combine with restore_neighbors, but why this arp_restore cannot be combined into swssconfig.sh and also use it for fast reboot?

Copy link
Copy Markdown
Contributor Author

@theasianpianist theasianpianist Mar 5, 2021

Choose a reason for hiding this comment

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

The issue with placing this in swssconfig.sh is that these kernel entries can only be programmed after the VLAN interface comes up, which happens after swssconfig.sh has exited

Copy link
Copy Markdown
Collaborator

@lguohan lguohan Mar 16, 2021

Choose a reason for hiding this comment

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

it is ok not be combine. but can this be used for fast-reboot as well?

command=/usr/bin/timeout 5m /usr/bin/kernel_arp_restore.sh
priority=6
autostart=false
autorestart=false
stdout_logfile=syslog
stderr_logfile=syslog
dependent_startup=true
dependent_startup_wait_for=swssconfig:exited

[program:restore_neighbors]
command=/usr/bin/restore_neighbors.py
priority=6
Expand Down
30 changes: 22 additions & 8 deletions dockers/docker-orchagent/swssconfig.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,19 @@

set -e

function fast_reboot {
case "$(cat /proc/cmdline)" in
*fast-reboot*)
function restore_app_db {

if [[ $(cat /proc/cmdline) == *"fast-reboot"* || -f /config-reload-restore ]];
then
if [[ -f /fdb.json ]];
then
swssconfig /fdb.json
mv -f /fdb.json /fdb.json.1
fi
fi

if [[ $(cat /proc/cmdline) == *"fast-reboot"* ]];
then
if [[ -f /arp.json ]];
then
swssconfig /arp.json
Expand All @@ -22,11 +26,20 @@ function fast_reboot {
swssconfig /default_routes.json
mv -f /default_routes.json /default_routes.json.1
fi
fi
}

;;
*)
;;
esac
function signal_kernel_restore {
Copy link
Copy Markdown
Collaborator

@lguohan lguohan Mar 16, 2021

Choose a reason for hiding this comment

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

why is this only for config reload? can this be used for fast reboot as well?

Copy link
Copy Markdown
Contributor Author

@theasianpianist theasianpianist Mar 16, 2021

Choose a reason for hiding this comment

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

Fast reboot already restores the neighbor table to APP_DB, and to my knowledge does not need it restored to the kernel.

if [[ -f /config-reload-restore ]];
then
if [[ -f /arp.json ]];
then
mv -f /arp.json /arp.json.1
# Tell kernel_arp_restore.sh that it needs to act
touch /restore-kernel
fi
rm -f /config-reload-restore
fi
}

# Wait until swss.sh in the host system create file swss:/ready
Expand All @@ -37,7 +50,8 @@ done
rm -f /ready

# Restore FDB and ARP table ASAP
fast_reboot
restore_app_db
signal_kernel_restore

# read SONiC immutable variables
[ -f /etc/sonic/sonic-environment ] && . /etc/sonic/sonic-environment
Expand Down
7 changes: 7 additions & 0 deletions files/build_templates/docker_image_ctl.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,13 @@ function postStartAction()
test -e /host/fast-reboot/default_routes.json && docker cp /host/fast-reboot/default_routes.json swss$DEV:/
rm -fr /host/fast-reboot
fi

if [[ -d /host/config-reload ]] && [[ -f /host/config-reload/needs-restore ]]; then
test -e /host/config-reload/fdb.json && docker cp /host/config-reload/fdb.json swss$DEV:/
test -e /host/config-reload/arp.json && docker cp /host/config-reload/arp.json swss$DEV:/
docker exec swss$DEV touch /config-reload-restore
rm -fr /host/config-reload
fi
docker exec swss$DEV touch /ready # signal swssconfig.sh to go
{%- elif docker_container_name == "pmon" %}

Expand Down