All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sevinj Aghayeva <sevinj.aghayeva@gmail.com>
To: netdev@vger.kernel.org
Cc: aroulin@nvidia.com, sbrivio@redhat.com, roopa@nvidia.com,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Nikolay Aleksandrov <razor@blackwall.org>,
	linux-kernel@vger.kernel.org, bridge@lists.linux-foundation.org,
	Sevinj Aghayeva <sevinj.aghayeva@gmail.com>
Subject: [PATCH RFC net-next 2/3] net: 8021q: fix bridge binding behavior for vlan interfaces
Date: Tue,  9 Aug 2022 23:11:20 -0400	[thread overview]
Message-ID: <3a01eea27e92133a3130e3d8d5f487d6900298db.1660100506.git.sevinj.aghayeva@gmail.com> (raw)
In-Reply-To: <cover.1660100506.git.sevinj.aghayeva@gmail.com>

Currently, when one creates a vlan interface with the bridge binding flag
disabled (using ip link add... command) and then enables the bridge binding flag
afterwards (using ip link set... command), the second command has no effect. In
other words, the vlan interface does not follow the status of the ports in its
vlan.

The root cause of this problem is as follows. The correct bridge binding
behavior depends on two flags being set: a per vlan interface flag
(VLAN_FLAG_BRIDGE_BINDING) and global per bridge flag
(BROPT_VLAN_BRIDGE_BINDING); the ip link set command calls vlan_dev_change_flags
function, which sets only the per vlan interface flag.

The correct behavior is to set/unset per bridge flag as well, depending on
whether there are other vlan interfaces with bridge binding flags set. The logic
for this behavior is already captured in br_vlan_upper_change function, which is
called whenever NETDEV_CHANGEUPPER event occurs. This patch fixes the bridge
binding behavior by triggering the NETDEV_CHANGEUPPER event from the
vlan_dev_change_flags function whenever the per interface flag is changed.

Signed-off-by: Sevinj Aghayeva <sevinj.aghayeva@gmail.com>
---
 net/8021q/vlan.h     |  2 +-
 net/8021q/vlan_dev.c | 25 ++++++++++++++++++++++---
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index 5eaf38875554..71947cdcfaaa 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -130,7 +130,7 @@ void vlan_dev_set_ingress_priority(const struct net_device *dev,
 int vlan_dev_set_egress_priority(const struct net_device *dev,
 				 u32 skb_prio, u16 vlan_prio);
 void vlan_dev_free_egress_priority(const struct net_device *dev);
-int vlan_dev_change_flags(const struct net_device *dev, u32 flag, u32 mask);
+int vlan_dev_change_flags(struct net_device *dev, u32 flag, u32 mask);
 void vlan_dev_get_realdev_name(const struct net_device *dev, char *result,
 			       size_t size);
 
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 839f2020b015..49cf4cceebef 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -208,12 +208,19 @@ int vlan_dev_set_egress_priority(const struct net_device *dev,
 	return 0;
 }
 
+static inline bool netif_is_bridge(const struct net_device *dev)
+{
+	return dev->rtnl_link_ops &&
+	    !strcmp(dev->rtnl_link_ops->kind, "bridge");
+}
+
 /* Flags are defined in the vlan_flags enum in
  * include/uapi/linux/if_vlan.h file.
  */
-int vlan_dev_change_flags(const struct net_device *dev, u32 flags, u32 mask)
+int vlan_dev_change_flags(struct net_device *dev, u32 flags, u32 mask)
 {
 	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
+	struct netdev_notifier_changeupper_info info;
 	u32 old_flags = vlan->flags;
 
 	if (mask & ~(VLAN_FLAG_REORDER_HDR | VLAN_FLAG_GVRP |
@@ -223,19 +230,31 @@ int vlan_dev_change_flags(const struct net_device *dev, u32 flags, u32 mask)
 
 	vlan->flags = (old_flags & ~mask) | (flags & mask);
 
-	if (netif_running(dev) && (vlan->flags ^ old_flags) & VLAN_FLAG_GVRP) {
+	if (!netif_running(dev))
+		return 0;
+
+	if ((vlan->flags ^ old_flags) & VLAN_FLAG_GVRP) {
 		if (vlan->flags & VLAN_FLAG_GVRP)
 			vlan_gvrp_request_join(dev);
 		else
 			vlan_gvrp_request_leave(dev);
 	}
 
-	if (netif_running(dev) && (vlan->flags ^ old_flags) & VLAN_FLAG_MVRP) {
+	if ((vlan->flags ^ old_flags) & VLAN_FLAG_MVRP) {
 		if (vlan->flags & VLAN_FLAG_MVRP)
 			vlan_mvrp_request_join(dev);
 		else
 			vlan_mvrp_request_leave(dev);
 	}
+
+	if ((vlan->flags ^ old_flags) & VLAN_FLAG_BRIDGE_BINDING &&
+	    netif_is_bridge(vlan->real_dev)) {
+		info.info.dev = vlan->real_dev;
+		info.upper_dev = dev;
+		info.linking = !!(vlan->flags & VLAN_FLAG_BRIDGE_BINDING);
+		call_netdevice_notifiers_info(NETDEV_CHANGEUPPER, &info.info);
+	}
+
 	return 0;
 }
 
-- 
2.25.1


WARNING: multiple messages have this Message-ID (diff)
From: Sevinj Aghayeva <sevinj.aghayeva@gmail.com>
To: netdev@vger.kernel.org
Cc: aroulin@nvidia.com, Nikolay Aleksandrov <razor@blackwall.org>,
	bridge@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
	sbrivio@redhat.com, Eric Dumazet <edumazet@google.com>,
	Sevinj Aghayeva <sevinj.aghayeva@gmail.com>,
	roopa@nvidia.com, Jakub Kicinski <kuba@kernel.org>,
	Paolo Abeni <pabeni@redhat.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [Bridge] [PATCH RFC net-next 2/3] net: 8021q: fix bridge binding behavior for vlan interfaces
Date: Tue,  9 Aug 2022 23:11:20 -0400	[thread overview]
Message-ID: <3a01eea27e92133a3130e3d8d5f487d6900298db.1660100506.git.sevinj.aghayeva@gmail.com> (raw)
In-Reply-To: <cover.1660100506.git.sevinj.aghayeva@gmail.com>

Currently, when one creates a vlan interface with the bridge binding flag
disabled (using ip link add... command) and then enables the bridge binding flag
afterwards (using ip link set... command), the second command has no effect. In
other words, the vlan interface does not follow the status of the ports in its
vlan.

The root cause of this problem is as follows. The correct bridge binding
behavior depends on two flags being set: a per vlan interface flag
(VLAN_FLAG_BRIDGE_BINDING) and global per bridge flag
(BROPT_VLAN_BRIDGE_BINDING); the ip link set command calls vlan_dev_change_flags
function, which sets only the per vlan interface flag.

The correct behavior is to set/unset per bridge flag as well, depending on
whether there are other vlan interfaces with bridge binding flags set. The logic
for this behavior is already captured in br_vlan_upper_change function, which is
called whenever NETDEV_CHANGEUPPER event occurs. This patch fixes the bridge
binding behavior by triggering the NETDEV_CHANGEUPPER event from the
vlan_dev_change_flags function whenever the per interface flag is changed.

Signed-off-by: Sevinj Aghayeva <sevinj.aghayeva@gmail.com>
---
 net/8021q/vlan.h     |  2 +-
 net/8021q/vlan_dev.c | 25 ++++++++++++++++++++++---
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index 5eaf38875554..71947cdcfaaa 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -130,7 +130,7 @@ void vlan_dev_set_ingress_priority(const struct net_device *dev,
 int vlan_dev_set_egress_priority(const struct net_device *dev,
 				 u32 skb_prio, u16 vlan_prio);
 void vlan_dev_free_egress_priority(const struct net_device *dev);
-int vlan_dev_change_flags(const struct net_device *dev, u32 flag, u32 mask);
+int vlan_dev_change_flags(struct net_device *dev, u32 flag, u32 mask);
 void vlan_dev_get_realdev_name(const struct net_device *dev, char *result,
 			       size_t size);
 
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 839f2020b015..49cf4cceebef 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -208,12 +208,19 @@ int vlan_dev_set_egress_priority(const struct net_device *dev,
 	return 0;
 }
 
+static inline bool netif_is_bridge(const struct net_device *dev)
+{
+	return dev->rtnl_link_ops &&
+	    !strcmp(dev->rtnl_link_ops->kind, "bridge");
+}
+
 /* Flags are defined in the vlan_flags enum in
  * include/uapi/linux/if_vlan.h file.
  */
-int vlan_dev_change_flags(const struct net_device *dev, u32 flags, u32 mask)
+int vlan_dev_change_flags(struct net_device *dev, u32 flags, u32 mask)
 {
 	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
+	struct netdev_notifier_changeupper_info info;
 	u32 old_flags = vlan->flags;
 
 	if (mask & ~(VLAN_FLAG_REORDER_HDR | VLAN_FLAG_GVRP |
@@ -223,19 +230,31 @@ int vlan_dev_change_flags(const struct net_device *dev, u32 flags, u32 mask)
 
 	vlan->flags = (old_flags & ~mask) | (flags & mask);
 
-	if (netif_running(dev) && (vlan->flags ^ old_flags) & VLAN_FLAG_GVRP) {
+	if (!netif_running(dev))
+		return 0;
+
+	if ((vlan->flags ^ old_flags) & VLAN_FLAG_GVRP) {
 		if (vlan->flags & VLAN_FLAG_GVRP)
 			vlan_gvrp_request_join(dev);
 		else
 			vlan_gvrp_request_leave(dev);
 	}
 
-	if (netif_running(dev) && (vlan->flags ^ old_flags) & VLAN_FLAG_MVRP) {
+	if ((vlan->flags ^ old_flags) & VLAN_FLAG_MVRP) {
 		if (vlan->flags & VLAN_FLAG_MVRP)
 			vlan_mvrp_request_join(dev);
 		else
 			vlan_mvrp_request_leave(dev);
 	}
+
+	if ((vlan->flags ^ old_flags) & VLAN_FLAG_BRIDGE_BINDING &&
+	    netif_is_bridge(vlan->real_dev)) {
+		info.info.dev = vlan->real_dev;
+		info.upper_dev = dev;
+		info.linking = !!(vlan->flags & VLAN_FLAG_BRIDGE_BINDING);
+		call_netdevice_notifiers_info(NETDEV_CHANGEUPPER, &info.info);
+	}
+
 	return 0;
 }
 
-- 
2.25.1


  parent reply	other threads:[~2022-08-10  3:12 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-10  3:11 [PATCH RFC net-next 0/3] net: vlan: fix bridge binding behavior and add selftests Sevinj Aghayeva
2022-08-10  3:11 ` [Bridge] " Sevinj Aghayeva
2022-08-10  3:11 ` [PATCH RFC net-next 1/3] net: core: export call_netdevice_notifiers_info Sevinj Aghayeva
2022-08-10  3:11   ` [Bridge] " Sevinj Aghayeva
2022-08-10  3:11 ` Sevinj Aghayeva [this message]
2022-08-10  3:11   ` [Bridge] [PATCH RFC net-next 2/3] net: 8021q: fix bridge binding behavior for vlan interfaces Sevinj Aghayeva
2022-08-10  3:11 ` [PATCH RFC net-next 3/3] selftests: net: tests for bridge binding behavior Sevinj Aghayeva
2022-08-10  3:11   ` [Bridge] " Sevinj Aghayeva
2022-08-10  8:54 ` [PATCH RFC net-next 0/3] net: vlan: fix bridge binding behavior and add selftests Nikolay Aleksandrov
2022-08-10  8:54   ` [Bridge] " Nikolay Aleksandrov
2022-08-10 14:42   ` Sevinj Aghayeva
2022-08-10 14:50     ` Nikolay Aleksandrov
2022-08-10 14:50       ` [Bridge] " Nikolay Aleksandrov
2022-08-10 15:00       ` Sevinj Aghayeva
2022-08-10 15:00         ` [Bridge] " Sevinj Aghayeva
2022-08-10 15:10         ` Nikolay Aleksandrov
2022-08-10 15:10           ` [Bridge] " Nikolay Aleksandrov
2022-08-10 15:21           ` Sevinj Aghayeva
2022-08-10 15:21             ` [Bridge] " Sevinj Aghayeva
2022-08-12 15:30   ` Sevinj Aghayeva
2022-08-12 15:30     ` [Bridge] " Sevinj Aghayeva
2022-08-14  7:38     ` Nikolay Aleksandrov
2022-08-14  7:38       ` [Bridge] " Nikolay Aleksandrov
2022-08-18 11:50       ` Sevinj Aghayeva
2022-08-18 11:50         ` [Bridge] " Sevinj Aghayeva
2022-08-18 12:00         ` Nikolay Aleksandrov
2022-08-18 12:00           ` [Bridge] " Nikolay Aleksandrov
2022-08-20 11:33           ` Sevinj Aghayeva
2022-08-20 11:33             ` [Bridge] " Sevinj Aghayeva
2022-08-22  8:01             ` Nikolay Aleksandrov
2022-08-22  8:01               ` [Bridge] " Nikolay Aleksandrov
2022-08-22 23:18               ` Sevinj Aghayeva
2022-08-29 20:22                 ` Sevinj Aghayeva
2022-08-31 10:16                   ` Nikolay Aleksandrov
2022-08-31 10:16                     ` [Bridge] " Nikolay Aleksandrov

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=3a01eea27e92133a3130e3d8d5f487d6900298db.1660100506.git.sevinj.aghayeva@gmail.com \
    --to=sevinj.aghayeva@gmail.com \
    --cc=aroulin@nvidia.com \
    --cc=bridge@lists.linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=razor@blackwall.org \
    --cc=roopa@nvidia.com \
    --cc=sbrivio@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.