From: Vladimir Oltean <olteanv@gmail.com>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Andrew Lunn <andrew@lunn.ch>,
Florian Fainelli <f.fainelli@gmail.com>,
Vivien Didelot <vivien.didelot@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
kernel@pengutronix.de, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
Russell King <linux@armlinux.org.uk>
Subject: Re: [PATCH net-next v1] net: dsa: fix bridge support for drivers without port_bridge_flags callback
Date: Wed, 21 Apr 2021 19:40:09 +0300 [thread overview]
Message-ID: <20210421164009.jylcxlab2evi2gcr@skbuf> (raw)
In-Reply-To: <20210421130540.12522-1-o.rempel@pengutronix.de>
On Wed, Apr 21, 2021 at 03:05:40PM +0200, Oleksij Rempel wrote:
> Starting with patch:
> a8b659e7ff75 ("net: dsa: act as passthrough for bridge port flags")
>
> drivers without "port_bridge_flags" callback will fail to join the bridge.
> Looking at the code, -EOPNOTSUPP seems to be the proper return value,
> which makes at least microchip and atheros switches work again.
>
> Fixes: a8b659e7ff75 ("net: dsa: act as passthrough for bridge port flags")
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> net/dsa/port.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index 01e30264b25b..6379d66a6bb3 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -550,7 +550,7 @@ int dsa_port_bridge_flags(const struct dsa_port *dp,
> struct dsa_switch *ds = dp->ds;
>
> if (!ds->ops->port_bridge_flags)
> - return -EINVAL;
> + return -EOPNOTSUPP;
>
> return ds->ops->port_bridge_flags(ds, dp->index, flags, extack);
> }
> --
> 2.29.2
>
The Fixes: tag should be:
Fixes: 5961d6a12c13 ("net: dsa: inherit the actual bridge port flags at join time")
What we return to the bridge is -EINVAL, via dsa_port_pre_bridge_flags()
(it is a two-step calling convention thing). But dsa_port_bridge_flags()
should never return that -EINVAL to the bridge, because the bridge
should just stop if the "pre" call returned an error.
So the -EINVAL return value from dsa_port_bridge_flags() is just for
callers who don't bother to call "pre".
To be honest I don't know why I wrote dsa_port_inherit_brport_flags this
way. It might be better to just do:
-----------------------------[cut here]-----------------------------
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 01e30264b25b..d5e227a77fbc 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -138,8 +138,12 @@ static int dsa_port_inherit_brport_flags(struct dsa_port *dp,
if (br_port_flag_is_set(brport_dev, BIT(flag)))
flags.val = BIT(flag);
+ err = dsa_port_pre_bridge_flags(dp, flags, extack);
+ if (err == -EINVAL)
+ continue;
+
err = dsa_port_bridge_flags(dp, flags, extack);
- if (err && err != -EOPNOTSUPP)
+ if (err)
return err;
}
@@ -159,8 +163,12 @@ static void dsa_port_clear_brport_flags(struct dsa_port *dp)
flags.mask = BIT(flag);
flags.val = val & BIT(flag);
+ err = dsa_port_pre_bridge_flags(dp, flags, NULL);
+ if (err == -EINVAL)
+ continue;
+
err = dsa_port_bridge_flags(dp, flags, NULL);
- if (err && err != -EOPNOTSUPP)
+ if (err)
dev_err(dp->ds->dev,
"failed to clear bridge port flag %lu: %pe\n",
flags.val, ERR_PTR(err));
-----------------------------[cut here]-----------------------------
next prev parent reply other threads:[~2021-04-21 16:41 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-21 13:05 [PATCH net-next v1] net: dsa: fix bridge support for drivers without port_bridge_flags callback Oleksij Rempel
2021-04-21 16:32 ` Florian Fainelli
2021-04-21 16:40 ` Vladimir Oltean [this message]
2021-04-21 17:50 ` patchwork-bot+netdevbpf
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=20210421164009.jylcxlab2evi2gcr@skbuf \
--to=olteanv@gmail.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=f.fainelli@gmail.com \
--cc=kernel@pengutronix.de \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
--cc=o.rempel@pengutronix.de \
--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 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.