[swss] Chassis db clean up optimization and bug fixes #16454

vganesan-nokia · 2023-09-06T02:32:32Z

Why I did it

The PR is for the following:

For the following chassis DB clean up optimization:
- To avoid delay before deleting system interfaces when no system neighbors were deleted before
- To avoid delay before deleting system lag when no system lag members were deleted before
To avoid stale kernel neighbor after clearing system neighbors when asic comes up.
To avoid accessing CHASSIS_APP_DB in pizzabox devices (PR Config reload results in ERR sonic-db-cli: :- getDbInfo: Failed to find CHASSIS_APP_DB database in namespace in syslog #16451)

Work item tracking

N/A

How I did it

Fix for regression failure due to error in finding CHASSIS_APP_DB in pizzabox (#PR 16451)
- Chassis clean up code is re-arranged to avoid accessing CHASSIS_APP_DB for non voq switches.
After attempting to delete the system neighbor entries from chassis db, before starting clearing the system interface entries,
wait for sometime only if some system neighbors were deleted. If there are no system neighbors entries deleted for the asic coming up, no need to wait.
Similar changes for system lag delete. Before deleting the system lag, wait for some time only if some system lag memebers were deleted. If there are no system lag members deleted no need to wait.
For the above two, the redis eval script to delete entries from chassis app db is made to return the number of entries deleted. Delay is introduced only if the number of entries deleted is greater than 0.
Flush the SYSTEM_NEIGH_TABLE from the local STATE_DB. While asic is coming up, when system neigh entries are deleted from chassis ap db (as part of chassis db clean up), there is no orchs/process running to process the delete messages from chassis redis. Because of this, stale system neigh entries are present in the local STATE_DB. The stale entries result in
creation of orphan (no corresponding data path/asic db entry) kernel neigh entries during STATE_DB:SYSTEM_NEIGH_TABLE entries processing by nbrmgr (after the swss serive came up). This is avoided by flushing the SYSTEM_NEIGH_TABLE from
the local STATE_DB when sevice comes up.

How to verify it

Tested during config reload and chassis reboot.
When asic without system neigbor entries and system lag member entries are restarted it was observed that delay was not introduced. This was indirectly verified by checking the log for the number of entries deleted.
No stale kernel neighbors were found after swss came up.
Tested in pizzabox for config reload. The error "Invalid database name input : 'CHASSIS_APP_DB'" was not seen.

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

Tested branch (Please provide the tested image version)

master

Description for the changelog

Changes done in swss.sh service start script in function clean_up_chassis_db_tables().

Link to config_db schema for YANG module changes

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

This commit includes the following changes: - Fix for regression failure due to error in finding CHASSIS_APP_DB in pizzabox (#PR 16451) - After attempting to delete the system neighbor entries from chassis db, before starting clearing the system interface entries, wait for sometime only if some system neighbors were deleted. If there are no system neighbors entries deleted for the asic coming up, no need to wait. - Similar changes for system lag delete. Before deleting the system lag, wait for some time only if some system lag memebers were deleted. If there are no system lag members deleted no need to wait. - Flush the SYSTEM_NEIGH_TABLE from the local STATE_DB. While asic is coming up, when system neigh entries are deleted from chassis ap db (as part of chassis db clean up), there is no orchs/process running to process the delete messages from chassis redis. Because of this, stale system neigh are entries present in the local STATE_DB. The stale entries result in creation of orphan (no corresponding data path/asic db entry) kernel neigh entries during STATE_DB:SYSTEM_NEIGH_TABLE entries processing by nbrmgr (after the swss serive came up). This is avoided by flushing the SYSTEM_NEIGH_TABLE from the local STATE_DB when sevice comes up. Signed-off-by: vedganes <veda.ganesan@nokia.com>

mlok-nokia · 2023-09-06T02:37:55Z

Change is good.

mlok-nokia · 2023-09-06T02:50:06Z

@arlakshm @judyjoseph @rlhui @gechiang This PR address the Pizzabox try to PING CHASSIS_APP_DB issue. It also contains the optimization (NO delay if there is Neighbor entry or no LAG_MEMBER_GROUP).

arlakshm

as comments

arlakshm · 2023-09-06T03:15:25Z

files/scripts/swss.sh


-    sleep 30
+    if [[ $num_neigh > 0 ]]; then


why do we need this change? is there any issue seen without this change?

No issues seen. This is an optimization change. For scenarios explained by @judyjoseph and @gechiang (for PR #16213) there may be situations when there will not be any entries to be cleaned up (for example when the asic is restarted second time or after). If there are no entries cleaned up the the delay is unnecessary. So we introduce delay conditionally only if there were some entries deleted.

arlakshm · 2023-09-06T03:24:40Z

files/scripts/swss.sh

@@ -275,7 +291,7 @@ start() {
        $SONIC_DB_CLI GB_ASIC_DB FLUSHDB
        $SONIC_DB_CLI GB_COUNTERS_DB FLUSHDB
        $SONIC_DB_CLI RESTAPI_DB FLUSHDB
-        clean_up_tables STATE_DB "'PORT_TABLE*', 'MGMT_PORT_TABLE*', 'VLAN_TABLE*', 'VLAN_MEMBER_TABLE*', 'LAG_TABLE*', 'LAG_MEMBER_TABLE*', 'INTERFACE_TABLE*', 'MIRROR_SESSION*', 'VRF_TABLE*', 'FDB_TABLE*', 'FG_ROUTE_TABLE*', 'BUFFER_POOL*', 'BUFFER_PROFILE*', 'MUX_CABLE_TABLE*', 'ADVERTISE_NETWORK_TABLE*', 'VXLAN_TUNNEL_TABLE*', 'VNET_ROUTE*', 'MACSEC_PORT_TABLE*', 'MACSEC_INGRESS_SA_TABLE*', 'MACSEC_EGRESS_SA_TABLE*', 'MACSEC_INGRESS_SC_TABLE*', 'MACSEC_EGRESS_SC_TABLE*', 'VRF_OBJECT_TABLE*', 'VNET_MONITOR_TABLE*', 'BFD_SESSION_TABLE*'"
+        clean_up_tables STATE_DB "'PORT_TABLE*', 'MGMT_PORT_TABLE*', 'VLAN_TABLE*', 'VLAN_MEMBER_TABLE*', 'LAG_TABLE*', 'LAG_MEMBER_TABLE*', 'INTERFACE_TABLE*', 'MIRROR_SESSION*', 'VRF_TABLE*', 'FDB_TABLE*', 'FG_ROUTE_TABLE*', 'BUFFER_POOL*', 'BUFFER_PROFILE*', 'MUX_CABLE_TABLE*', 'ADVERTISE_NETWORK_TABLE*', 'VXLAN_TUNNEL_TABLE*', 'VNET_ROUTE*', 'MACSEC_PORT_TABLE*', 'MACSEC_INGRESS_SA_TABLE*', 'MACSEC_EGRESS_SA_TABLE*', 'MACSEC_INGRESS_SC_TABLE*', 'MACSEC_EGRESS_SC_TABLE*', 'VRF_OBJECT_TABLE*', 'VNET_MONITOR_TABLE*', 'BFD_SESSION_TABLE*','SYSTEM_NEIGH_TABLE*'"


how will removing from state_db trigger removing of the entry from the kernel?

We are not removing the entries from kernel but avoid creating entries. When there are entries in the SYSTEM_NEIGH_TABLE in the STATE_DB, when nbrmgr comes up, it subscribes to this table. The existing entries in the table are subscribed as SET commands. As part of SET command processing for entries from this table in STATE_DB, we program kernel neighbors. By removing all the stale entries from this table we avoid nbrmgr getting the SET commands and hence the programming of the kernel entries.

Thanks @vganesan-nokia for the explanation. I get this part. This change helps. I was thinking of a scenario where, after swss restart if a neighbor is not learnt anymore on the local linecard, then the kernel entry will not be removed, is that correct? this change may not help in this case?

The STATE_DB SYSTEM_NEIGH_TABLE is only for remote neighbor entries.

arlakshm · 2023-09-06T03:26:40Z

files/scripts/swss.sh

-    return " 0 $lc $asic
+    return nlm" 0 $lc $asic`
+
+    debug "Chassis db clean up for ${SERVICE}$DEV. Number of SYSTEM_LAG_MEMBER_TABLE entries deleted: $num_lag_mem"


add debug statements for the deletion of other entries?

Yes. I'll add debug for other table entries.

Fixed. Debug logs added for deletion of other tables also.

Debug logs added for deletion of other tables (SYSTEM_INTERFACE and SYSTEM_LAG_TABLE) Signed-off-by: vedganes <veda.ganesan@nokia.com>

deepak-singhal0408 · 2023-09-11T17:03:43Z

Microsoft ADO: 25128972
@yxieca @StormLiangMS
Appreciate approval for the requested branches...

Thanks!

mssonicbld · 2023-09-12T05:32:18Z

@vganesan-nokia PR conflicts with 202211 branch

mssonicbld · 2023-09-12T05:32:26Z

@vganesan-nokia PR conflicts with 202205 branch

deepak-singhal0408 · 2023-09-13T04:52:45Z

Hi @vganesan-nokia , could you please create a manual PR for above conflict branches?

vganesan-nokia · 2023-09-13T14:42:32Z

Hi @vganesan-nokia , could you please create a manual PR for above conflict branches?

Yes. I'll create PRs for both branches with conflicts fixed.

* [swss] Chassis db clean up optimization and bug fixes This commit includes the following changes: - Fix for regression failure due to error in finding CHASSIS_APP_DB in pizzabox (#PR 16451) - After attempting to delete the system neighbor entries from chassis db, before starting clearing the system interface entries, wait for sometime only if some system neighbors were deleted. If there are no system neighbors entries deleted for the asic coming up, no need to wait. - Similar changes for system lag delete. Before deleting the system lag, wait for some time only if some system lag memebers were deleted. If there are no system lag members deleted no need to wait. - Flush the SYSTEM_NEIGH_TABLE from the local STATE_DB. While asic is coming up, when system neigh entries are deleted from chassis ap db (as part of chassis db clean up), there is no orchs/process running to process the delete messages from chassis redis. Because of this, stale system neigh are entries present in the local STATE_DB. The stale entries result in creation of orphan (no corresponding data path/asic db entry) kernel neigh entries during STATE_DB:SYSTEM_NEIGH_TABLE entries processing by nbrmgr (after the swss serive came up). This is avoided by flushing the SYSTEM_NEIGH_TABLE from the local STATE_DB when sevice comes up. Signed-off-by: vedganes <veda.ganesan@nokia.com> * [swss] Chassis db clean up bug fixes review comment fix - 1 Debug logs added for deletion of other tables (SYSTEM_INTERFACE and SYSTEM_LAG_TABLE) Signed-off-by: vedganes <veda.ganesan@nokia.com> --------- Signed-off-by: vedganes <veda.ganesan@nokia.com> (cherry picked from commit b13b41f)

vganesan-nokia · 2023-09-13T21:01:13Z

@deepak-singhal0408: PRs have been raised with conflicts fixed for cherry-picking this PR to 202205 and 202211 branches.
The PRs are: For 202211: #16540 and for 202205: #16541

* [swss] Chassis db clean up optimization and bug fixes This commit includes the following changes: - Fix for regression failure due to error in finding CHASSIS_APP_DB in pizzabox (#PR 16451) - After attempting to delete the system neighbor entries from chassis db, before starting clearing the system interface entries, wait for sometime only if some system neighbors were deleted. If there are no system neighbors entries deleted for the asic coming up, no need to wait. - Similar changes for system lag delete. Before deleting the system lag, wait for some time only if some system lag memebers were deleted. If there are no system lag members deleted no need to wait. - Flush the SYSTEM_NEIGH_TABLE from the local STATE_DB. While asic is coming up, when system neigh entries are deleted from chassis ap db (as part of chassis db clean up), there is no orchs/process running to process the delete messages from chassis redis. Because of this, stale system neigh are entries present in the local STATE_DB. The stale entries result in creation of orphan (no corresponding data path/asic db entry) kernel neigh entries during STATE_DB:SYSTEM_NEIGH_TABLE entries processing by nbrmgr (after the swss serive came up). This is avoided by flushing the SYSTEM_NEIGH_TABLE from the local STATE_DB when sevice comes up. Signed-off-by: vedganes <veda.ganesan@nokia.com> * [swss] Chassis db clean up bug fixes review comment fix - 1 Debug logs added for deletion of other tables (SYSTEM_INTERFACE and SYSTEM_LAG_TABLE) Signed-off-by: vedganes <veda.ganesan@nokia.com> --------- Signed-off-by: vedganes <veda.ganesan@nokia.com> (cherry picked from commit b13b41f)

deepak-singhal0408 · 2023-09-19T20:29:51Z

@yxieca @lguohan could we please cherry pick above PR for 202305 branch as well? Thanks

dgsudharsan · 2023-09-19T20:39:31Z

@StormLiangMS Can you please help with cherry-pick request for 202305?

dgsudharsan · 2023-09-20T04:02:28Z

@vganesan-nokia I believe this may have conflicts for 202305. Can you please create a separate PR?

* [swss] Chassis db clean up optimization and bug fixes This commit includes the following changes: - Fix for regression failure due to error in finding CHASSIS_APP_DB in pizzabox (#PR 16451) - After attempting to delete the system neighbor entries from chassis db, before starting clearing the system interface entries, wait for sometime only if some system neighbors were deleted. If there are no system neighbors entries deleted for the asic coming up, no need to wait. - Similar changes for system lag delete. Before deleting the system lag, wait for some time only if some system lag memebers were deleted. If there are no system lag members deleted no need to wait. - Flush the SYSTEM_NEIGH_TABLE from the local STATE_DB. While asic is coming up, when system neigh entries are deleted from chassis ap db (as part of chassis db clean up), there is no orchs/process running to process the delete messages from chassis redis. Because of this, stale system neigh are entries present in the local STATE_DB. The stale entries result in creation of orphan (no corresponding data path/asic db entry) kernel neigh entries during STATE_DB:SYSTEM_NEIGH_TABLE entries processing by nbrmgr (after the swss serive came up). This is avoided by flushing the SYSTEM_NEIGH_TABLE from the local STATE_DB when sevice comes up. Signed-off-by: vedganes <veda.ganesan@nokia.com> * [swss] Chassis db clean up bug fixes review comment fix - 1 Debug logs added for deletion of other tables (SYSTEM_INTERFACE and SYSTEM_LAG_TABLE) Signed-off-by: vedganes <veda.ganesan@nokia.com> --------- Signed-off-by: vedganes <veda.ganesan@nokia.com>

vganesan-nokia · 2023-09-20T16:19:19Z

@vganesan-nokia I believe this may have conflicts for 202305. Can you please create a separate PR?

We need to add PR #16213 in 202305 before we add this PR

mssonicbld · 2023-09-21T05:57:02Z

@vganesan-nokia PR conflicts with 202305 branch

vganesan-nokia · 2023-09-21T17:02:29Z

@vganesan-nokia PR conflicts with 202305 branch

o.k. I'll create a separte PR with conflicts resolved.

* [swss] Chassis db clean up optimization and bug fixes This commit includes the following changes: - Fix for regression failure due to error in finding CHASSIS_APP_DB in pizzabox (#PR 16451) - After attempting to delete the system neighbor entries from chassis db, before starting clearing the system interface entries, wait for sometime only if some system neighbors were deleted. If there are no system neighbors entries deleted for the asic coming up, no need to wait. - Similar changes for system lag delete. Before deleting the system lag, wait for some time only if some system lag memebers were deleted. If there are no system lag members deleted no need to wait. - Flush the SYSTEM_NEIGH_TABLE from the local STATE_DB. While asic is coming up, when system neigh entries are deleted from chassis ap db (as part of chassis db clean up), there is no orchs/process running to process the delete messages from chassis redis. Because of this, stale system neigh are entries present in the local STATE_DB. The stale entries result in creation of orphan (no corresponding data path/asic db entry) kernel neigh entries during STATE_DB:SYSTEM_NEIGH_TABLE entries processing by nbrmgr (after the swss serive came up). This is avoided by flushing the SYSTEM_NEIGH_TABLE from the local STATE_DB when sevice comes up. Signed-off-by: vedganes <veda.ganesan@nokia.com> * [swss] Chassis db clean up bug fixes review comment fix - 1 Debug logs added for deletion of other tables (SYSTEM_INTERFACE and SYSTEM_LAG_TABLE) Signed-off-by: vedganes <veda.ganesan@nokia.com> --------- Signed-off-by: vedganes <veda.ganesan@nokia.com> (cherry picked from commit b13b41f)

vganesan-nokia · 2023-09-21T17:26:03Z

Raised PR #16644 to cherry-pick PR #16454 with conflicts resolved.

* [swss] Chassis db clean up optimization and bug fixes This commit includes the following changes: - Fix for regression failure due to error in finding CHASSIS_APP_DB in pizzabox (#PR 16451) - After attempting to delete the system neighbor entries from chassis db, before starting clearing the system interface entries, wait for sometime only if some system neighbors were deleted. If there are no system neighbors entries deleted for the asic coming up, no need to wait. - Similar changes for system lag delete. Before deleting the system lag, wait for some time only if some system lag memebers were deleted. If there are no system lag members deleted no need to wait. - Flush the SYSTEM_NEIGH_TABLE from the local STATE_DB. While asic is coming up, when system neigh entries are deleted from chassis ap db (as part of chassis db clean up), there is no orchs/process running to process the delete messages from chassis redis. Because of this, stale system neigh are entries present in the local STATE_DB. The stale entries result in creation of orphan (no corresponding data path/asic db entry) kernel neigh entries during STATE_DB:SYSTEM_NEIGH_TABLE entries processing by nbrmgr (after the swss serive came up). This is avoided by flushing the SYSTEM_NEIGH_TABLE from the local STATE_DB when sevice comes up. Signed-off-by: vedganes <veda.ganesan@nokia.com> * [swss] Chassis db clean up bug fixes review comment fix - 1 Debug logs added for deletion of other tables (SYSTEM_INTERFACE and SYSTEM_LAG_TABLE) Signed-off-by: vedganes <veda.ganesan@nokia.com> --------- Signed-off-by: vedganes <veda.ganesan@nokia.com> (cherry picked from commit b13b41f)

vganesan-nokia requested a review from lguohan as a code owner September 6, 2023 02:32

arlakshm reviewed Sep 6, 2023

View reviewed changes

[swss] Chassis db clean up bug fixes review comment fix - 1

b00b70a

Debug logs added for deletion of other tables (SYSTEM_INTERFACE and SYSTEM_LAG_TABLE) Signed-off-by: vedganes <veda.ganesan@nokia.com>

dgsudharsan added Request for 202205 Branch Request for 202305 Branch Request for 202211 Branch labels Sep 7, 2023

arlakshm approved these changes Sep 7, 2023

View reviewed changes

dgsudharsan approved these changes Sep 9, 2023

View reviewed changes

yxieca merged commit b13b41f into sonic-net:master Sep 11, 2023

yxieca added Approved for 202205 Branch Approved for 202211 Branch labels Sep 12, 2023

mssonicbld added the Cherry Pick Conflict_202211 label Sep 12, 2023

mssonicbld added the Cherry Pick Conflict_202205 label Sep 12, 2023

StormLiangMS added the Approved for 202305 Branch label Sep 21, 2023

mssonicbld added the Cherry Pick Conflict_202305 label Sep 21, 2023

yxieca added the Included in 202205 Branch label Sep 25, 2023

judyjoseph mentioned this pull request Sep 28, 2023

[voq][chassis] decap/fib tests fail with error in pipeline run sonic-net/sonic-mgmt#10194

Closed

StormLiangMS added Included in 202305 Branch and removed Cherry Pick Conflict_202305 labels Oct 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[swss] Chassis db clean up optimization and bug fixes #16454

[swss] Chassis db clean up optimization and bug fixes #16454

vganesan-nokia commented Sep 6, 2023 •

edited

Loading

mlok-nokia commented Sep 6, 2023 •

edited

Loading

mlok-nokia commented Sep 6, 2023 •

edited

Loading

arlakshm left a comment

arlakshm Sep 6, 2023

vganesan-nokia Sep 6, 2023

arlakshm Sep 6, 2023

vganesan-nokia Sep 6, 2023

arlakshm Sep 6, 2023 •

edited

Loading

vganesan-nokia Sep 7, 2023

arlakshm Sep 7, 2023

arlakshm Sep 6, 2023

vganesan-nokia Sep 6, 2023

vganesan-nokia Sep 7, 2023

deepak-singhal0408 commented Sep 11, 2023

mssonicbld commented Sep 12, 2023

mssonicbld commented Sep 12, 2023

deepak-singhal0408 commented Sep 13, 2023

vganesan-nokia commented Sep 13, 2023

vganesan-nokia commented Sep 13, 2023

deepak-singhal0408 commented Sep 19, 2023

dgsudharsan commented Sep 19, 2023

dgsudharsan commented Sep 20, 2023

vganesan-nokia commented Sep 20, 2023 •

edited

Loading

mssonicbld commented Sep 21, 2023

vganesan-nokia commented Sep 21, 2023

vganesan-nokia commented Sep 21, 2023 •

edited

Loading

[swss] Chassis db clean up optimization and bug fixes #16454

[swss] Chassis db clean up optimization and bug fixes #16454

Conversation

vganesan-nokia commented Sep 6, 2023 • edited Loading

Why I did it

Work item tracking

How I did it

How to verify it

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

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

mlok-nokia commented Sep 6, 2023 • edited Loading

mlok-nokia commented Sep 6, 2023 • edited Loading

arlakshm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arlakshm Sep 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepak-singhal0408 commented Sep 11, 2023

mssonicbld commented Sep 12, 2023

mssonicbld commented Sep 12, 2023

deepak-singhal0408 commented Sep 13, 2023

vganesan-nokia commented Sep 13, 2023

vganesan-nokia commented Sep 13, 2023

deepak-singhal0408 commented Sep 19, 2023

dgsudharsan commented Sep 19, 2023

dgsudharsan commented Sep 20, 2023

vganesan-nokia commented Sep 20, 2023 • edited Loading

mssonicbld commented Sep 21, 2023

vganesan-nokia commented Sep 21, 2023

vganesan-nokia commented Sep 21, 2023 • edited Loading

vganesan-nokia commented Sep 6, 2023 •

edited

Loading

mlok-nokia commented Sep 6, 2023 •

edited

Loading

mlok-nokia commented Sep 6, 2023 •

edited

Loading

arlakshm Sep 6, 2023 •

edited

Loading

vganesan-nokia commented Sep 20, 2023 •

edited

Loading

vganesan-nokia commented Sep 21, 2023 •

edited

Loading