Netdev List
 help / color / mirror / Atom feed
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
To: netdev@vger.kernel.org
Cc: roopa@cumulusnetworks.com, dsa@cumulusnetworks.com,
	mrv@mojatatu.com, stephen@networkplumber.org,
	bridge@lists.linux-foundation.org,
	Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Subject: [PATCH net-next 2/2] bridge: vlan: return EEXIST on add if nothing changed
Date: Thu, 19 Oct 2017 18:54:59 +0300	[thread overview]
Message-ID: <1508428499-32355-3-git-send-email-nikolay@cumulusnetworks.com> (raw)
In-Reply-To: <1508428499-32355-1-git-send-email-nikolay@cumulusnetworks.com>

Before this patch there was no way to tell if the vlan add operation
actually changed anything, thus we would always generate a notification
on adds. Let's make the notifications more precise and generate them
only if anything changed, so use EEXIST error to signal that the vlan
was not updated in any way. We just need to be careful about a few
places that could re-add the same vlan with the same flags not to return
any errors on EEXIST.

Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
 net/bridge/br_netlink.c |  5 +++++
 net/bridge/br_vlan.c    | 38 ++++++++++++++++++++++++++------------
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index e8d74a3f44f7..80d9334a4b46 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -523,6 +523,11 @@ static int br_vlan_info(struct net_bridge *br, struct net_bridge_port *p,
 		}
 		if (!err)
 			*changed = true;
+		else if (err == -EEXIST)
+			/* nothing changed, return 0 for overlapping range add
+			 * and compatibility reasons
+			 */
+			err = 0;
 		break;
 
 	case RTM_DELLINK:
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 233a30040c91..e021a80eb8e9 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -32,27 +32,34 @@ static struct net_bridge_vlan *br_vlan_lookup(struct rhashtable *tbl, u16 vid)
 	return rhashtable_lookup_fast(tbl, &vid, br_vlan_rht_params);
 }
 
-static void __vlan_add_pvid(struct net_bridge_vlan_group *vg, u16 vid)
+static bool __vlan_add_pvid(struct net_bridge_vlan_group *vg, u16 vid)
 {
 	if (vg->pvid == vid)
-		return;
+		return false;
 
 	smp_wmb();
 	vg->pvid = vid;
+
+	return true;
 }
 
-static void __vlan_delete_pvid(struct net_bridge_vlan_group *vg, u16 vid)
+static bool __vlan_delete_pvid(struct net_bridge_vlan_group *vg, u16 vid)
 {
 	if (vg->pvid != vid)
-		return;
+		return false;
 
 	smp_wmb();
 	vg->pvid = 0;
+
+	return true;
 }
 
-static void __vlan_add_flags(struct net_bridge_vlan *v, u16 flags)
+/* return true if anything changed, false otherwise */
+static bool __vlan_add_flags(struct net_bridge_vlan *v, u16 flags)
 {
 	struct net_bridge_vlan_group *vg;
+	u16 old_flags = v->flags;
+	bool ret;
 
 	if (br_vlan_is_master(v))
 		vg = br_vlan_group(v->br);
@@ -60,14 +67,16 @@ static void __vlan_add_flags(struct net_bridge_vlan *v, u16 flags)
 		vg = nbp_vlan_group(v->port);
 
 	if (flags & BRIDGE_VLAN_INFO_PVID)
-		__vlan_add_pvid(vg, v->vid);
+		ret = __vlan_add_pvid(vg, v->vid);
 	else
-		__vlan_delete_pvid(vg, v->vid);
+		ret = __vlan_delete_pvid(vg, v->vid);
 
 	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
 		v->flags |= BRIDGE_VLAN_INFO_UNTAGGED;
 	else
 		v->flags &= ~BRIDGE_VLAN_INFO_UNTAGGED;
+
+	return ret || !!(old_flags ^ v->flags);
 }
 
 static int __vlan_vid_add(struct net_device *dev, struct net_bridge *br,
@@ -234,7 +243,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags)
 		if (flags & BRIDGE_VLAN_INFO_MASTER) {
 			err = br_vlan_add(br, v->vid, flags |
 						      BRIDGE_VLAN_INFO_BRENTRY);
-			if (err)
+			if (err && err != -EEXIST)
 				goto out_filt;
 		}
 
@@ -562,6 +571,7 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags)
 	vg = br_vlan_group(br);
 	vlan = br_vlan_find(vg, vid);
 	if (vlan) {
+		ret = -EEXIST;
 		if (!br_vlan_is_brentry(vlan)) {
 			/* Trying to change flags of non-existent bridge vlan */
 			if (!(flags & BRIDGE_VLAN_INFO_BRENTRY))
@@ -577,8 +587,10 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags)
 			vlan->flags |= BRIDGE_VLAN_INFO_BRENTRY;
 			vg->num_vlans++;
 		}
-		__vlan_add_flags(vlan, flags);
-		return 0;
+		if (__vlan_add_flags(vlan, flags))
+			ret = 0;
+
+		return ret;
 	}
 
 	vlan = kzalloc(sizeof(*vlan), GFP_KERNEL);
@@ -870,7 +882,7 @@ int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid)
 		err = nbp_vlan_add(p, pvid,
 				   BRIDGE_VLAN_INFO_PVID |
 				   BRIDGE_VLAN_INFO_UNTAGGED);
-		if (err)
+		if (err && err != -EEXIST)
 			goto err_port;
 		nbp_vlan_delete(p, old_pvid);
 		set_bit(p->port_no, changed);
@@ -1037,7 +1049,9 @@ int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
 		ret = switchdev_port_obj_add(port->dev, &v.obj);
 		if (ret && ret != -EOPNOTSUPP)
 			return ret;
-		__vlan_add_flags(vlan, flags);
+		if (!__vlan_add_flags(vlan, flags))
+			return -EEXIST;
+
 		return 0;
 	}
 
-- 
2.1.4

  parent reply	other threads:[~2017-10-19 15:55 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-19 15:54 [PATCH net-next 0/2] bridge: make setlink/dellink notifications more accurate Nikolay Aleksandrov
2017-10-19 15:54 ` [PATCH net-next 1/2] bridge: netlink: " Nikolay Aleksandrov
2017-10-19 15:54 ` Nikolay Aleksandrov [this message]
2017-10-19 16:10 ` [PATCH net-next 0/2] bridge: " Nikolay Aleksandrov
2017-10-22  0:45   ` David Miller

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=1508428499-32355-3-git-send-email-nikolay@cumulusnetworks.com \
    --to=nikolay@cumulusnetworks.com \
    --cc=bridge@lists.linux-foundation.org \
    --cc=dsa@cumulusnetworks.com \
    --cc=mrv@mojatatu.com \
    --cc=netdev@vger.kernel.org \
    --cc=roopa@cumulusnetworks.com \
    --cc=stephen@networkplumber.org \
    /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