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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 22 Jan 2018 07:14:17 -0800
From:   Roopa Prabhu <roopa@...ulusnetworks.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        Nikolay Aleksandrov <nikolay@...ulusnetworks.com>,
        David Ahern <dsa@...ulusnetworks.com>
Subject: Re: [PATCH net-next] net: link_watch: mark bonding link events urgent

On Mon, Jan 22, 2018 at 12:13 AM, Jiri Pirko <jiri@...nulli.us> wrote:
> Mon, Jan 22, 2018 at 07:07:53AM CET, roopa@...ulusnetworks.com wrote:
>>From: Roopa Prabhu <roopa@...ulusnetworks.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@...ulusnetworks.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 ?