Subject: Re: [PATCH net-next] net/core: fix for_each_netdev_feature

From: Nikolay Aleksandrov <[email-masked]>
Date: Tue, 3 Nov 2015 16:33:38 +0100

On 11/03/2015 04:15 PM, Jarod Wilson wrote:
> As pointed out by Nikolay and further explained by Geert, the initial
> for_each_netdev_feature macro was broken, as feature would get set outside
> of the block of code it was intended to run in, thus only ever working for
> the first feature bit in the mask. While less pretty this way, this is
> tested and confirmed functional with multiple feature bits set in
> NETIF_F_UPPER_DISABLES.
> 
> [root@dell-per730-01 ~]# ethtool -K bond0 lro off
> ...
> [  242.761394] bond0: Disabling feature 0x0000000000008000 on lower dev p5p2.
> [  243.552178] bnx2x 0000:06:00.1 p5p2: using MSI-X  IRQs: sp 74  fp[0] 76 ... fp[7] 83
> [  244.353978] bond0: Disabling feature 0x0000000000008000 on lower dev p5p1.
> [  245.147420] bnx2x 0000:06:00.0 p5p1: using MSI-X  IRQs: sp 62  fp[0] 64 ... fp[7] 71
> 
> [root@dell-per730-01 ~]# ethtool -K bond0 gro off
> ...
> [  251.925645] bond0: Disabling feature 0x0000000000004000 on lower dev p5p2.
> [  252.713693] bnx2x 0000:06:00.1 p5p2: using MSI-X  IRQs: sp 74  fp[0] 76 ... fp[7] 83
> [  253.499085] bond0: Disabling feature 0x0000000000004000 on lower dev p5p1.
> [  254.290922] bnx2x 0000:06:00.0 p5p1: using MSI-X  IRQs: sp 62  fp[0] 64 ... fp[7] 71
> 
> Fixes: fd867d51f ("net/core: generic support for disabling netdev features down stack")
> CC: "David S. Miller" <[email-masked]>
> CC: Eric Dumazet <[email-masked]>
> CC: Jay Vosburgh <[email-masked]>
> CC: Veaceslav Falico <[email-masked]>
> CC: Andy Gospodarek <[email-masked]>
> CC: Jiri Pirko <[email-masked]>
> CC: Nikolay Aleksandrov <[email-masked]>
> CC: Michal Kubecek <[email-masked]>
> CC: Alexander Duyck <[email-masked]>
> CC: Geert Uytterhoeven <[email-masked]>
> CC: [email-masked]
> Signed-off-by: Jarod Wilson <[email-masked]>
> ---
>  include/linux/netdev_features.h | 6 ++----
>  net/core/dev.c                  | 8 ++++++--
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 

Looks good to me especially without the hidden side-effects,

Acked-by: Nikolay Aleksandrov <[email-masked]>

Thanks,
 Nik