netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jarod Wilson <jarod@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: Jarod Wilson <jarod@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jay Vosburgh <j.vosburgh@gmail.com>,
	Veaceslav Falico <vfalico@gmail.com>,
	Andy Gospodarek <gospo@cumulusnetworks.com>,
	Jiri Pirko <jiri@resnulli.us>,
	Nikolay Aleksandrov <razor@blackwall.org>,
	Michal Kubecek <mkubecek@suse.cz>,
	Alexander Duyck <alexander.duyck@gmail.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	netdev@vger.kernel.org
Subject: [PATCH net-next] net/core: fix for_each_netdev_feature
Date: Tue,  3 Nov 2015 10:15:59 -0500	[thread overview]
Message-ID: <1446563759-17713-1-git-send-email-jarod@redhat.com> (raw)
In-Reply-To: <1446519359-21400-1-git-send-email-jarod@redhat.com>

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@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <gospo@cumulusnetworks.com>
CC: Jiri Pirko <jiri@resnulli.us>
CC: Nikolay Aleksandrov <razor@blackwall.org>
CC: Michal Kubecek <mkubecek@suse.cz>
CC: Alexander Duyck <alexander.duyck@gmail.com>
CC: Geert Uytterhoeven <geert@linux-m68k.org>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 include/linux/netdev_features.h | 6 ++----
 net/core/dev.c                  | 8 ++++++--
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index 0f5837a..f0d8734 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -125,10 +125,8 @@ enum {
 #define NETIF_F_HW_L2FW_DOFFLOAD	__NETIF_F(HW_L2FW_DOFFLOAD)
 #define NETIF_F_BUSY_POLL	__NETIF_F(BUSY_POLL)
 
-#define for_each_netdev_feature(mask_addr, feature)				\
-	int bit;								\
-	for_each_set_bit(bit, (unsigned long *)mask_addr, NETDEV_FEATURE_COUNT)	\
-		feature = __NETIF_F_BIT(bit);
+#define for_each_netdev_feature(mask_addr, bit)	\
+	for_each_set_bit(bit, (unsigned long *)mask_addr, NETDEV_FEATURE_COUNT)
 
 /* Features valid for ethtool to change */
 /* = all defined minus driver/device-class-related */
diff --git a/net/core/dev.c b/net/core/dev.c
index c4d2b43..8ce3f74 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6293,8 +6293,10 @@ static netdev_features_t netdev_sync_upper_features(struct net_device *lower,
 {
 	netdev_features_t upper_disables = NETIF_F_UPPER_DISABLES;
 	netdev_features_t feature;
+	int feature_bit;
 
-	for_each_netdev_feature(&upper_disables, feature) {
+	for_each_netdev_feature(&upper_disables, feature_bit) {
+		feature = __NETIF_F_BIT(feature_bit);
 		if (!(upper->wanted_features & feature)
 		    && (features & feature)) {
 			netdev_dbg(lower, "Dropping feature %pNF, upper dev %s has it off.\n",
@@ -6311,8 +6313,10 @@ static void netdev_sync_lower_features(struct net_device *upper,
 {
 	netdev_features_t upper_disables = NETIF_F_UPPER_DISABLES;
 	netdev_features_t feature;
+	int feature_bit;
 
-	for_each_netdev_feature(&upper_disables, feature) {
+	for_each_netdev_feature(&upper_disables, feature_bit) {
+		feature = __NETIF_F_BIT(feature_bit);
 		if (!(features & feature) && (lower->features & feature)) {
 			netdev_dbg(upper, "Disabling feature %pNF on lower dev %s.\n",
 				   &feature, lower->name);
-- 
1.8.3.1

  parent reply	other threads:[~2015-11-03 15:16 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-24  3:40 [RFC PATCH net-next] net/core: initial support for stacked dev feature toggles Jarod Wilson
2015-10-24  4:41 ` Tom Herbert
2015-10-24  5:51 ` Alexander Duyck
2015-10-26  9:42   ` Michal Kubecek
2015-10-30 16:25     ` Jarod Wilson
2015-10-30 20:02       ` Alexander Duyck
2015-11-02 17:37         ` Jarod Wilson
2015-10-30 16:35   ` Jarod Wilson
2015-10-30 20:14     ` Alexander Duyck
2015-11-02 17:53 ` [PATCH net-next] net/core: generic support for disabling netdev features down stack Jarod Wilson
2015-11-02 18:04   ` Alexander Duyck
2015-11-02 21:57     ` Jarod Wilson
2015-11-03  2:55   ` [PATCH v2 " Jarod Wilson
2015-11-03  4:41     ` David Miller
2015-11-03 10:03     ` Nikolay Aleksandrov
2015-11-03 13:52       ` Geert Uytterhoeven
2015-11-03 13:57         ` Jarod Wilson
2015-11-03 14:05           ` Nikolay Aleksandrov
2015-11-03 15:18             ` Jarod Wilson
2015-11-03 15:15     ` Jarod Wilson [this message]
2015-11-03 15:33       ` [PATCH net-next] net/core: fix for_each_netdev_feature Nikolay Aleksandrov
2015-11-03 16:34       ` David Miller
2015-11-03 20:36     ` [PATCH net-next] net/core: ensure features get disabled on new lower devs Jarod Wilson
2015-11-03 21:17       ` Alexander Duyck
2015-11-03 22:11         ` Jarod Wilson
2015-11-03 23:01           ` Alexander Duyck
2015-11-03 21:21       ` Nikolay Aleksandrov
2015-11-03 21:53       ` Michal Kubecek
2015-11-03 21:58         ` Jarod Wilson
2015-11-04  4:09       ` [PATCH v2 " Jarod Wilson
2015-11-05  2:56         ` David Miller
2015-11-13  0:26           ` Florian Fainelli
2015-11-13 10:29             ` Jiri Pirko
2015-11-13 10:51               ` Nikolay Aleksandrov
2015-11-13 13:54                 ` [PATCH net] net: fix feature changes on devices without ndo_set_features Nikolay Aleksandrov
2015-11-13 14:00                   ` Jiri Pirko
2015-11-13 14:06                   ` Andy Gospodarek
2015-11-13 14:34                   ` Jarod Wilson
2015-11-13 18:30                   ` Florian Fainelli
2015-11-15  7:25                   ` [net] " Dave Young
2015-11-16  2:01                     ` Dave Young
2015-11-16 19:56                   ` [PATCH net] " David Miller
2015-11-17 23:03                   ` Sergei Shtylyov
2015-11-17 23:10                     ` Nikolay Aleksandrov
2015-11-18 10:51                       ` Sergei Shtylyov
2015-11-13 22:31                 ` [PATCH v2 net-next] net/core: ensure features get disabled on new lower devs Laura Abbott
2015-11-17  9:02             ` Geert Uytterhoeven
2015-11-17 10:04               ` Geert Uytterhoeven
2016-04-02  2:21     ` [PATCH v2 net-next] net/core: generic support for disabling netdev features down stack Michał Mirosław

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1446563759-17713-1-git-send-email-jarod@redhat.com \
    --to=jarod@redhat.com \
    --cc=alexander.duyck@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=geert@linux-m68k.org \
    --cc=gospo@cumulusnetworks.com \
    --cc=j.vosburgh@gmail.com \
    --cc=jiri@resnulli.us \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mkubecek@suse.cz \
    --cc=netdev@vger.kernel.org \
    --cc=razor@blackwall.org \
    --cc=vfalico@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).