[sonic-swss] - Netlink messages can be lost inbound from kernel and STATE_DB PORT_TABLE 'admin_status' field is misleading

[snider-nokia]

Description

1.) The STATE_DB PORT_TABLE admin_status attribute/field is being improperly updated from within linksync.cpp (method LinkSync::onMsg) to reflect interface IFF_UP flag status as passed from the kernel via Netlink event(s). Either this admin_status attribute/field should be eliminated or it should be renamed (to properly reflect that it represents the Netdev IFF_UP flag status). Any subscriber to associated (STATE_DB PORT_TABLE) port update messages can currently be victimized when above referenced admin_status is set to ‘down’ (as it is, invariably, when the kernel sees operational state go down for an active Netdev interface). The real admin_status for an interface should (obviously) only reside in the CONFIG_DB.

2.) Netlink messages are sometimes being lost when multiple interfaces are impacted in parallel in the context of operational link status. For example, if several interfaces are administratively disabled at the switch peer then operational status for all associated local links will go down. When the kernel see these links go down it then generates a Netlink message(s) for each impacted Netdev interface. Aside from the fact that these Netlink messages reflect the legacy IFF_UP flag as cleared and linksync.cpp (LinkSync::onMsg) then improperly transmutes said cleared IFF_UP flag into the STATE_DB PORT_TABLE admin_status field/attribute, thus potentially victimizing any STATE_DB PORT_TABLE subscribers via a bogus admin_status = 'down' port status update... when there are too many Netlink messages (being originated by the kernel) to fit in the associated socket buffer, or some other problem manifests, Netlink messages can be lost thus leaving an incoherent STATE_DB PORT_TABLE record for some impacted interface(s).

Steps to reproduce the issue:

1. Configure several local ports as administratively enabled and verify links as operationally up.
2. Simultaneously administratively disable the aforementioned link peer ports at the peer switch.
3. Monitor syslog to ascertain that not all expected STATE_DB PORT_TABLE updates have occurred as expected (Netlink messages have been lost).

Show version:

Master and 202205

Describe the results you received:

Results are simpler to discern with the extant (badly named) admin_status field/attribute still in place at STATE_DB PORT_TABLE.

Netlink messages are indeed lost when the socket buffer overflows and/or when some other kernel related problem occurs whereby linksync.cpp (LinkSync::onMsg) can lose track of the actual state of a given Netdev interface due to this message loss.

Describe the results you expected:

Netlink events are understood to be unreliable, thus it should not be expected that they will always arrive in a coherent manner.
Linksync.cpp (LinkSync::onMsg) logic should be scrutinized heavily in order to ensure that loss of Netlink message(s) will not leave an associated interface with an incoherent STATE_DB PORT_TABLE record.

Germane syslog output is in black and my annotations in red.

The genesis of STATE_DB PORT_TABLE admin_status attribute update is here (linksync.cpp):

Netlink events are not reliable.

The associated Netlink socket buffer can certainly be enlarged from default to attempt to prevent Netlink event/message overflow, but it is still a bad idea to presume that all Netlink messages will arrive coherently. Thus, logic in this path should be scrutinized to ensure that STATE_DB PORT_TABLE coherence will not be lost for interface(s) that suffer from lost Netlink event(s)/message(s)...

https://man7.org/linux/man-pages/man7/netlink.7.html

[prgeor]

@judyjoseph if we need more netlink socket buffer increased

[judyjoseph]

Discussed this with team, @snider-nokia, can we check the behavior after increasing the netlink socket buffer size ( similar to as done earlier in this PR sonic-net/sonic-swss-common#391 and check the behavior ? )

[snider-nokia]

Yes Judy, should be able to get some feedback on this the week after Thanksgiving.

[judyjoseph]

There is a similar effort to fine tune netlink buffer ( increase to like 16MB) here : #12502. To tray with this change @snider-nokia

[judyjoseph]

@snider-nokia Will you be able to check this again, with this increased netlink buffer size?
sonic-net/sonic-swss-common#739
#13965

[snider-nokia]

@snider-nokia Will you be able to check this again, with this increased netlink buffer size? sonic-net/sonic-swss-common#739 #13965

Yes Judy, I have just gotten back to this recently and experienced a couple of problems along the way. I now have a working rebased image (based on 202205) that we will be attempting to run full OC tests on shortly to verify there are no problems.