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

From: Nikolay Aleksandrov
Date: Tue Nov 03 2015 - 10:40:08 EST


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" <davem@xxxxxxxxxxxxx>
> CC: Eric Dumazet <edumazet@xxxxxxxxxx>
> CC: Jay Vosburgh <j.vosburgh@xxxxxxxxx>
> CC: Veaceslav Falico <vfalico@xxxxxxxxx>
> CC: Andy Gospodarek <gospo@xxxxxxxxxxxxxxxxxxx>
> CC: Jiri Pirko <jiri@xxxxxxxxxxx>
> CC: Nikolay Aleksandrov <razor@xxxxxxxxxxxxx>
> CC: Michal Kubecek <mkubecek@xxxxxxx>
> CC: Alexander Duyck <alexander.duyck@xxxxxxxxx>
> CC: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> CC: netdev@xxxxxxxxxxxxxxx
> Signed-off-by: Jarod Wilson <jarod@xxxxxxxxxx>
> ---
> 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 <nikolay@xxxxxxxxxxxxxxxxxxx>

Thanks,
Nik

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/