netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vladimir Oltean <olteanv@gmail.com>
To: f.fainelli@gmail.com, vivien.didelot@gmail.com, andrew@lunn.ch,
	idosch@idosch.org, roopa@cumulusnetworks.com,
	nikolay@cumulusnetworks.com, davem@davemloft.net
Cc: netdev@vger.kernel.org, Vladimir Oltean <olteanv@gmail.com>
Subject: [PATCH net-next 2/6] net: bridge: Populate the pvid flag in br_vlan_get_info
Date: Tue, 20 Aug 2019 02:59:58 +0300	[thread overview]
Message-ID: <20190820000002.9776-3-olteanv@gmail.com> (raw)
In-Reply-To: <20190820000002.9776-1-olteanv@gmail.com>

Currently this simplified code snippet fails:

	br_vlan_get_pvid(netdev, &pvid);
	br_vlan_get_info(netdev, pvid, &vinfo);
	ASSERT(!(vinfo.flags & BRIDGE_VLAN_INFO_PVID));

It is intuitive that the pvid of a netdevice should have the
BRIDGE_VLAN_INFO_PVID flag set.

However I can't seem to pinpoint a commit where this behavior was
introduced. It seems like it's been like that since forever.

At a first glance it would make more sense to just handle the
BRIDGE_VLAN_INFO_PVID flag in __vlan_add_flags. However, as Nikolay
explains:

  There are a few reasons why we don't do it, most importantly because
  we need to have only one visible pvid at any single time, even if it's
  stale - it must be just one. Right now that rule will not be violated
  by this change, but people will try using this flag and could see two
  pvids simultaneously. You can see that the pvid code is even using
  memory barriers to propagate the new value faster and everywhere the
  pvid is read only once.  That is the reason the flag is set
  dynamically when dumping entries, too.  A second (weaker) argument
  against would be given the above we don't want another way to do the
  same thing, specifically if it can provide us with two pvids (e.g. if
  walking the vlan list) or if it can provide us with a pvid different
  from the one set in the vg. [Obviously, I'm talking about RCU
  pvid/vlan use cases similar to the dumps.  The locked cases are fine.
  I would like to avoid explaining why this shouldn't be relied upon
  without locking]

So instead of introducing the above change and making sure of the pvid
uniqueness under RCU, simply dynamically populate the pvid flag in
br_vlan_get_info().

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 net/bridge/br_vlan.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index f5b2aeebbfe9..bb98984cd27d 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -1281,6 +1281,8 @@ int br_vlan_get_info(const struct net_device *dev, u16 vid,
 
 	p_vinfo->vid = vid;
 	p_vinfo->flags = v->flags;
+	if (vid == br_get_pvid(vg))
+		p_vinfo->flags |= BRIDGE_VLAN_INFO_PVID;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(br_vlan_get_info);
-- 
2.17.1


  parent reply	other threads:[~2019-08-20  0:00 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-19 23:59 [PATCH net-next 0/6] Dynamic toggling of vlan_filtering for SJA1105 DSA Vladimir Oltean
2019-08-19 23:59 ` [PATCH net-next 1/6] net: dsa: tag_8021q: Future-proof the reserved fields in the custom VID Vladimir Oltean
2019-08-20  3:16   ` Florian Fainelli
2019-08-19 23:59 ` Vladimir Oltean [this message]
2019-08-20  0:12   ` [PATCH net-next 2/6] net: bridge: Populate the pvid flag in br_vlan_get_info Nikolay Aleksandrov
2019-08-19 23:59 ` [PATCH net-next 3/6] net: dsa: Delete the VID from the upstream port as well Vladimir Oltean
2019-08-20  5:51   ` Vivien Didelot
2019-08-20  9:54     ` Vladimir Oltean
2019-08-20 17:52       ` Vivien Didelot
2019-08-20 19:40         ` Florian Fainelli
2019-08-20 20:40           ` Vladimir Oltean
2019-08-20 20:58             ` Vivien Didelot
2019-08-20 21:02               ` Vladimir Oltean
2019-08-20 21:36                 ` Vivien Didelot
2019-08-20 22:09                   ` Vladimir Oltean
2019-08-21  3:30                     ` Vivien Didelot
2019-08-21  9:51                       ` Vladimir Oltean
2019-08-20  0:00 ` [PATCH net-next 4/6] net: dsa: Don't program the VLAN as pvid on the upstream port Vladimir Oltean
2019-08-20  3:15   ` Florian Fainelli
2019-08-20 12:09     ` Vladimir Oltean
2019-08-20 22:43       ` Florian Fainelli
2019-08-20  6:07   ` Vivien Didelot
2019-08-20 10:22     ` Vladimir Oltean
2019-08-20  0:00 ` [PATCH net-next 5/6] net: dsa: Allow proper internal use of VLANs Vladimir Oltean
2019-08-20  3:20   ` Florian Fainelli
2019-08-20  0:00 ` [PATCH net-next 6/6] net: dsa: tag_8021q: Restore bridge pvid when enabling vlan_filtering Vladimir Oltean
2019-08-20  3:32   ` Florian Fainelli
2019-08-20 10:28     ` Vladimir Oltean
2019-08-20 17:36       ` Florian Fainelli
2019-08-20 23:24   ` Florian Fainelli
2019-08-20  8:04 ` [PATCH net-next 0/6] Dynamic toggling of vlan_filtering for SJA1105 DSA Vivien Didelot
2019-08-20 10:18   ` Vladimir Oltean

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=20190820000002.9776-3-olteanv@gmail.com \
    --to=olteanv@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=idosch@idosch.org \
    --cc=netdev@vger.kernel.org \
    --cc=nikolay@cumulusnetworks.com \
    --cc=roopa@cumulusnetworks.com \
    --cc=vivien.didelot@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).