[linksync] Netdev oper status determination using IFF_RUNNING #1568
Conversation
Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com> Changes in linksync to use IFF_RUNNING to determine link operational status instead of using IFF_LOWER_UP.
|
retest this please |
|
/Azurepipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -174,7 +174,7 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj) | |||
|
|
|||
| unsigned int flags = rtnl_link_get_flags(link); | |||
| bool admin = flags & IFF_UP; | |||
| bool oper = flags & IFF_LOWER_UP; | |||
| bool oper = flags & IFF_RUNNING; | |||
There was a problem hiding this comment.
For e.g, this interface has LOWER_UP but state is UNKNOWN. Will it have IFF_RUNNING set?
58: Loopback0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
link/ether 96:a7:77:d1:73:34 brd ff:ff:ff:ff:ff:ff
There was a problem hiding this comment.
Yes. Linux command "ip link show" does not show it here. The bgp "show interface" will show this flag. Here is an example of interface info got by linux command and bgp command.
admin@Linecard2:~$
admin@Linecard2:~$ ip link show | grep -A1 Loopback0
10: Loopback0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
link/ether f6:55:7a:eb:85:eb brd ff:ff:ff:ff:ff:ff
admin@Linecard2:~$
admin@Linecard2:~$ docker exec bgp vtysh -c "show interface" | grep -A12 Loopback0
Interface Loopback0 is up, line protocol is up
Link ups: 1 last: 2019/03/02 21:32:48.16
Link downs: 0 last: (never)
vrf: default
index 10 metric 0 mtu 65536 speed 0
flags: <UP,BROADCAST,RUNNING,NOARP>
Type: Unknown
HWaddr: f6:55:7a:eb:85:eb
inet 10.1.0.1/32 unnumbered
inet6 fe80::f455:7aff:feeb:85eb/64
inet6 fc00:10::1/128
Interface Type Other
Interface Slave Type None
admin@Linecard2:~$
|
/AzurePipelines run |
|
Commenter does not have sufficient privileges for PR 1568 in repo Azure/sonic-swss |
|
@prsunny, would please re-start the AzurePipelines? It seems they are stuck and not progressing. I tried to start it. It seems I do not have permission to start AzurePipelines. |
|
/Azurepipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@prsunny, thanks for restarting tests. |
|
HERE IS LINUX MANUAL, wonder what the different is between iff_running and if_lower_up? why do we think iff_running is the right thing to do? |
The objective 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. This leads to interface state mismatch between zebra and orchagent for a window of time (IFF_LOWER_UP and IFF_RUNNING events come at different points 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 having the interface state timing same in both orchagent and zebra. I have described this in detail in issue sonic-net/sonic-buildimage#6295. |
|
is iff_running after iff_lower_up? is zebra doing the right thing? |
Yes. IFF_LOWER_UP comes up earlier than IFF_RUNNING. But zebra is not acting on IFF_LOWE_UP.
In our SONiC linux, in the file if.h, the IFF_RUNNING has comment "@IFF_RUNNING: interface RFC2863 OPER_UP. Volatile." As a routing daemon, zebra want to know whether the interface is capable of passing packets or not. So it rightly depends on this IFF_RUNNING flag which indicates whether the interface is able to pass packets or not. So I guess zebra is doing the right thing. |
|
make sense, can you add above discussion in the pr description. we will add it as part of the pr commit message. |
Updated the PR comment as suggested. |
…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>
#### What I did 1. Add CLI support for port auto negotiation feature. 2. Add db_migrator change for auto negotiation feature 2. Add unit test cases for all changes #### How I did it 1. Add new subcommands to "config interface" command group to allow user configuring port auto negotiation 2. Add new subcommands to "show interfaces" command group to allow user show auto negotiation status 3. In db_migrator.py, change auto negotiation related DB field to latest one
…rator test cases as well (sonic-net#1614) - What I did Originally, the method advance_version_for_expected_database was introduced (in sonic-net#1566) to handle the case the latest version in CONFIG_DB is greater than the latest version in mellanox_buffer_migrator. Now there are other database migrators whose test cases can also encounter this situation, like port auto-negotiation (sonic-net#1568) and port-channel for LACP key (sonic-net#1473). So I would like to make the method public, available for all database migrators. Related database migrator test cases have been updated accordingly. - How to verify it Run the unit test. Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: vedganes vedavinayagam.ganesan@nokia.com
Changes in linksync to use IFF_RUNNING to determine netdev operational
status instead of using IFF_LOWER_UP.
What I did
Changed linksync.cpp to use IFF_RUNNING to determine the netdev operational status instead of using IFF_LOWER_UP.
Why I did it
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.
How I verified it
Details if related