Re: [PATCH net-next] net: link_watch: mark bonding link events urgent

[Jiri Pirko <jiri@resnulli.us>]

Mon, Jan 22, 2018 at 04:14:17PM CET, roopa@cumulusnetworks.com wrote:
>On Mon, Jan 22, 2018 at 12:13 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Mon, Jan 22, 2018 at 07:07:53AM CET, roopa@cumulusnetworks.com wrote:
>>>From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>
>>>It takes 1sec for bond link down notification to hit user-space
>>>when all slaves of the bond go down. 1sec is too long for
>>>protocol daemons in user-space relying on bond link notification
>>>to failover/recover (eg: multichassis lag implementations in user-space).
>>>Since the link event code already marks team device port link events
>>> urgent, this patch does the same for bonding link events.
>>>
>>>Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>---
>>> net/core/link_watch.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>>diff --git a/net/core/link_watch.c b/net/core/link_watch.c
>>>index 9828616..63bb2ad 100644
>>>--- a/net/core/link_watch.c
>>>+++ b/net/core/link_watch.c
>>>@@ -92,7 +92,7 @@ static bool linkwatch_urgent_event(struct net_device *dev)
>>>       if (dev->ifindex != dev_get_iflink(dev))
>>>               return true;
>>>
>>>-      if (dev->priv_flags & IFF_TEAM_PORT)
>>>+      if (dev->priv_flags & (IFF_TEAM_PORT | IFF_BONDING))
>>
>> Don't you want to do that for bonding slaves? IFF_BONDING is set for
>> both master and slave. netif_is_bond_slave() helper checks that. In fact
>> netif_is_team_port() should be used here instead of checking IFF_TEAM_PORT
>> directly. And then you can use netif_is_lag_port() to check them both.
>
>
>IFF_BONDING Is set for both slaves and master (and thats what I need).
>In my case I consistently see link_watch throttle bond link down event
>...since it follows immediately after the the bond slave link down
>events
>(any optimizations in the bonding driver for link events are squashed
>at this point by link watch).
>so I do need to cover the bond master case. I think for the use-case I
>am talking about, team master device might also have the same
>problem....if somebody else in
>user-space is relying on the team master link down (though i do
>understand the sequence of events are a bit different there...where
>you set the master link down from user-space).
>
>thanks for the pointers to the helpers..., given above, are you ok if
>I move the check to netif_is_lag_port || netif_is_lag_master ?

Looks fine. Thanks.