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

Linksync - netdev oper status determination using IFF_RUNNING #6295

Closed
vganesan-nokia opened this issue Dec 24, 2020 · 2 comments
Closed

Linksync - netdev oper status determination using IFF_RUNNING #6295

vganesan-nokia opened this issue Dec 24, 2020 · 2 comments
Labels
Chassis 🤖 Modular chassis support

Comments

@vganesan-nokia
Copy link
Contributor

vganesan-nokia commented Dec 24, 2020

Description

This issue is raised as per recommendation from voq inband interface PR code review. This is to request change in netdev event handling in linksync to use IFF_RUNNING to determine the operational status instead of using IFF_LOWER_UP.

Motivation for this request is as follows: There is difference between how SONiC and zebra determine oper status of interfaces. This difference introduces problems on static entries programmed. With SONiC support for voq chassis and with inband interface, we need to add static neighbors and static routes kernel entries for the remote neighbors on Inband interface netdev. These static entries are needed to make BGP/Zebra resolve next hop for routes from different SONiC instances (Ref: https://github.com/Azure/SONiC/blob/master/doc/voq/voq_hld.md, https://github.com/Azure/SONiC/blob/master/doc/voq/architecture.md, sonic-net/SONiC#674). We add these static entries after the netdev of the inband interface is operationally up. Zebra uses IFF_RUNNING to determine operational status of interfaces and to make the neighbors and routes on the interface active. This difference in oper status determination between SONiC and zebra results in zebra not making active of static routes and static neighbors even after the interfaces become oper up in zebra.

Sequence of events that results in problematic situation:

  • Netdev is created for a port
  • Zebra adds interface, SONiC linksync updates STATE db.
  • SONiC (voq chassis neighbor handling in PR Implementation of System ports initialization, Interface & Neighbor Synchronization... sonic-swss#1431) adds static neighbor and static route on netdev as soon as netdev info (an entry for the port) is available in STATE db. In zebra the device is not oper up yet.
  • SONiC linksync gets IFF_LOWER_UP and updates the STATE db. Zebra gets IFF_LOWER_UP but this is not effective.
  • SONiC linksync gets IFF_RUNNING but not used. Zebra gets IFF_RUNNING and makes the interface operationally up. But the static routes and static neighbors added before the interface was oper up are not made active. So zebra's NHT fails to establish reachability of those next hops that use these static neighbor and static routes.

For voq chassis implementation, for inband interface, to postpone static entries programming till netdev is oper up, the oper status of netdev will be tracked and used. So determination of operational state of devices based on IFF_RUNNING (in line with zebra) would help voq chassis implementation to postpone static entries programming till the interfaces are operationally up in zebra and hence make zebra make the static entries and next hops active always.

Steps to reproduce the issue:

Note: The steps to reproduce the issues is based on voq chassis implementation that has inband interface port available as regular port in the PORT table. These steps occur in chassis implementation internally triggered by config loading

  1. An Inband port is configured as voq inband interface
  2. Static neighbors and static routes are added on the inband port. This usually happens quickly for the iBGP neighbors of other SONiC instances of the chassis

Describe the results you received:

  • The static neighbors and static routes added for the iBGP neighbors are available in kernel. But zebra does not bring these active since from zebra's perspective, these static entries were added before the devices are operationally up.
  • Zebra does not make active of all routes with next hop that use these static entries for reachability.
  • Since there is no internal processing to bring back the static entries active when the interfaces become operationally up, this problematic state is not recoverable until the static entries are deleted and added back again manually.

Describe the results you expected:

  • With changing linksync to determine oper status of the netdev-s based on IFF_RUNNING, SONiC's netdev oper status will be in sync with zebra's. So if we postpone static entries addition till the inband interface is operationally up, zebra will have static entries active, the next hops reachable and hence the routes added with these next hops will be active from the time they are programmed.

Additional information you deem important (e.g. issue happens only occasionally):

**Output of `show version`:**

```
(paste your output here)
```

**Attach debug file `sudo generate_dump`:**

```
(paste your output here)
```
@vganesan-nokia
Copy link
Contributor Author

PR sonic-net/sonic-swss#1568 has changes to address this issue/request

@anshuv-mfst anshuv-mfst added the Chassis 🤖 Modular chassis support label Jan 19, 2021
lguohan pushed a commit to sonic-net/sonic-swss that referenced this issue Mar 5, 2021
The objective of this change is to make orchagent/linksync align with FRR/Zebra in handling interface states. Zebra uses IFF_RUNNING flag to determine interfaces' oper state while linksync uses IFF_LOWER_UP. Zebra rightly depends on IFF_RUNNING flag. As a routing daemon, zebra wants to know whether the interface is capable of passing packets or not. The flag IFF_RUNNING indicates exactly that (based on comment in if.h, this flag reflects the interface oper up state as defined in RFC2863). Since orchagent uses IFF_LOWER_UP, which comes earlier than IFF_RUNNING, there is interface state mismatch between zebra and orchagent for a window of time. Since with voq implementation we write static neigh/routes when the netdev (inband port) becomes oper up and bgp depends on these kernel entries, there is a need for this change to have the interface state timing same in both orchagent and zebra.

Refer issue sonic-net/sonic-buildimage#6295 for detailed information.

Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this issue Oct 5, 2021
…net#1568)

The objective of this change is to make orchagent/linksync align with FRR/Zebra in handling interface states. Zebra uses IFF_RUNNING flag to determine interfaces' oper state while linksync uses IFF_LOWER_UP. Zebra rightly depends on IFF_RUNNING flag. As a routing daemon, zebra wants to know whether the interface is capable of passing packets or not. The flag IFF_RUNNING indicates exactly that (based on comment in if.h, this flag reflects the interface oper up state as defined in RFC2863). Since orchagent uses IFF_LOWER_UP, which comes earlier than IFF_RUNNING, there is interface state mismatch between zebra and orchagent for a window of time. Since with voq implementation we write static neigh/routes when the netdev (inband port) becomes oper up and bgp depends on these kernel entries, there is a need for this change to have the interface state timing same in both orchagent and zebra.

Refer issue sonic-net/sonic-buildimage#6295 for detailed information.

Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>
@abdosi
Copy link
Contributor

abdosi commented Aug 17, 2022

Issue is fixed.

@abdosi abdosi closed this as completed Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Chassis 🤖 Modular chassis support
Projects
None yet
Development

No branches or pull requests

3 participants