Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: netdev@vger.kernel.org, davem@davemloft.net
Cc: andrew@lunn.ch, f.fainelli@gmail.com, vivien.didelot@gmail.com,
	idosch@idosch.org, jiri@resnulli.us,
	kurt.kanzenbach@linutronix.de, kuba@kernel.org
Subject: [PATCH v2 net-next 5/9] net: dsa: refuse configuration in prepare phase of dsa_port_vlan_filtering()
Date: Mon, 21 Sep 2020 03:10:27 +0300	[thread overview]
Message-ID: <20200921001031.3650456-6-vladimir.oltean@nxp.com> (raw)
In-Reply-To: <20200921001031.3650456-1-vladimir.oltean@nxp.com>

The current logic beats me a little bit. The comment that "bridge skips
-EOPNOTSUPP, so skip the prepare phase" was introduced in commit
fb2dabad69f0 ("net: dsa: support VLAN filtering switchdev attr").

I'm not sure:
(a) ok, the bridge skips -EOPNOTSUPP, but, so what, where are we
    returning -EOPNOTSUPP?
(b) even if we are, and I'm just not seeing it, what is the causality
    relationship between the bridge skipping -EOPNOTSUPP and DSA
    skipping the prepare phase, and just returning zero?

One thing is certain beyond doubt though, and that is that DSA currently
refuses VLAN filtering from the "commit" phase instead of "prepare", and
that this is not a good thing:

ip link add br0 type bridge
ip link add br1 type bridge vlan_filtering 1
ip link set swp2 master br0
ip link set swp3 master br1
[ 3790.379389] 001: sja1105 spi0.1: VLAN filtering is a global setting
[ 3790.379399] 001: ------------[ cut here ]------------
[ 3790.379403] 001: WARNING: CPU: 1 PID: 515 at net/switchdev/switchdev.c:157 switchdev_port_attr_set_now+0x9c/0xa4
[ 3790.379420] 001: swp3: Commit of attribute (id=6) failed.
[ 3790.379533] 001: [<c11ff588>] (switchdev_port_attr_set_now) from [<c11b62e4>] (nbp_vlan_init+0x84/0x148)
[ 3790.379544] 001: [<c11b62e4>] (nbp_vlan_init) from [<c11a2ff0>] (br_add_if+0x514/0x670)
[ 3790.379554] 001: [<c11a2ff0>] (br_add_if) from [<c1031b5c>] (do_setlink+0x38c/0xab0)
[ 3790.379565] 001: [<c1031b5c>] (do_setlink) from [<c1036fe8>] (__rtnl_newlink+0x44c/0x748)
[ 3790.379573] 001: [<c1036fe8>] (__rtnl_newlink) from [<c1037328>] (rtnl_newlink+0x44/0x60)
[ 3790.379580] 001: [<c1037328>] (rtnl_newlink) from [<c10315fc>] (rtnetlink_rcv_msg+0x124/0x2f8)
[ 3790.379590] 001: [<c10315fc>] (rtnetlink_rcv_msg) from [<c10926b8>] (netlink_rcv_skb+0xb8/0x110)
[ 3790.379806] 001: ---[ end trace 0000000000000002 ]---
[ 3790.379819] 001: sja1105 spi0.1 swp3: failed to initialize vlan filtering on this port

So move the current logic that may fail (except ds->ops->port_vlan_filtering,
that is way harder) into the prepare stage of the switchdev transaction.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
None.

 net/dsa/port.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/dsa/port.c b/net/dsa/port.c
index 46c9bf709683..794a03718838 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -232,15 +232,15 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
 	struct dsa_switch *ds = dp->ds;
 	int err;
 
-	/* bridge skips -EOPNOTSUPP, so skip the prepare phase */
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
+	if (switchdev_trans_ph_prepare(trans)) {
+		if (!ds->ops->port_vlan_filtering)
+			return -EOPNOTSUPP;
 
-	if (!ds->ops->port_vlan_filtering)
-		return 0;
+		if (!dsa_port_can_apply_vlan_filtering(dp, vlan_filtering))
+			return -EINVAL;
 
-	if (!dsa_port_can_apply_vlan_filtering(dp, vlan_filtering))
-		return -EINVAL;
+		return 0;
+	}
 
 	if (dsa_port_is_vlan_filtering(dp) == vlan_filtering)
 		return 0;
-- 
2.25.1


  parent reply	other threads:[~2020-09-21  0:11 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-21  0:10 [PATCH v2 net-next 0/9] DSA with VLAN filtering and offloading masters Vladimir Oltean
2020-09-21  0:10 ` [PATCH v2 net-next 1/9] net: dsa: deny enslaving 802.1Q upper to VLAN-aware bridge from PRECHANGEUPPER Vladimir Oltean
2020-09-21  0:10 ` [PATCH v2 net-next 2/9] net: dsa: rename dsa_slave_upper_vlan_check to something more suggestive Vladimir Oltean
2020-09-21  0:10 ` [PATCH v2 net-next 3/9] net: dsa: convert check for 802.1Q upper when bridged into PRECHANGEUPPER Vladimir Oltean
2020-09-21  0:10 ` [PATCH v2 net-next 4/9] net: dsa: convert denying bridge VLAN with existing 8021q upper to PRECHANGEUPPER Vladimir Oltean
2020-09-21  0:10 ` Vladimir Oltean [this message]
2020-09-21  0:10 ` [PATCH v2 net-next 6/9] net: dsa: allow 8021q uppers while the bridge has vlan_filtering=0 Vladimir Oltean
2020-09-21  0:10 ` [PATCH v2 net-next 7/9] net: dsa: install VLANs into the master's RX filter too Vladimir Oltean
2020-09-21  0:10 ` [PATCH v2 net-next 8/9] net: dsa: tag_8021q: add VLANs to the master interface too Vladimir Oltean
2020-09-21  0:10 ` [PATCH v2 net-next 9/9] net: dsa: tag_sja1105: add compatibility with hwaccel VLAN tags Vladimir Oltean
2020-09-21  2:02 ` [PATCH v2 net-next 0/9] DSA with VLAN filtering and offloading masters 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=20200921001031.3650456-6-vladimir.oltean@nxp.com \
    --to=vladimir.oltean@nxp.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=idosch@idosch.org \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=kurt.kanzenbach@linutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=vivien.didelot@gmail.com \
    --subject='Re: [PATCH v2 net-next 5/9] net: dsa: refuse configuration in prepare phase of dsa_port_vlan_filtering()' \
    /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

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).