[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <67c19828-6335-3003-b86b-18d72a961e05@gmail.com>
Date: Wed, 28 Oct 2020 19:22:31 -0700
From: Florian Fainelli <f.fainelli@...il.com>
To: Kurt Kanzenbach <kurt@...utronix.de>, Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Vladimir Oltean <olteanv@...il.com>
Cc: "David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Richard Cochran <richardcochran@...il.com>,
Kamil Alkhouri <kamil.alkhouri@...offenburg.de>,
ilias.apalodimas@...aro.org,
Vladimir Oltean <vladimir.oltean@....com>
Subject: Re: [PATCH net-next v7 2/8] net: dsa: Give drivers the chance to veto
certain upper devices
On 10/28/2020 12:42 AM, Kurt Kanzenbach wrote:
> From: Vladimir Oltean <vladimir.oltean@....com>
>
> Some switches rely on unique pvids to ensure port separation in
> standalone mode, because they don't have a port forwarding matrix
> configurable in hardware. So, setups like a group of 2 uppers with the
> same VLAN, swp0.100 and swp1.100, will cause traffic tagged with VLAN
> 100 to be autonomously forwarded between these switch ports, in spite
> of there being no bridge between swp0 and swp1.
>
> These drivers need to prevent this from happening. They need to have
> VLAN filtering enabled in standalone mode (so they'll drop frames tagged
> with unknown VLANs) and they can only accept an 8021q upper on a port as
> long as it isn't installed on any other port too. So give them the
> chance to veto bad user requests.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> Signed-off-by: Kurt Kanzenbach <kurt@...utronix.de>
> ---
> include/net/dsa.h | 6 ++++++
> net/dsa/slave.c | 12 ++++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 04e93bafb7bd..4e60d2610f20 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -536,6 +536,12 @@ struct dsa_switch_ops {
> void (*get_regs)(struct dsa_switch *ds, int port,
> struct ethtool_regs *regs, void *p);
>
> + /*
> + * Upper device tracking.
> + */
> + int (*port_prechangeupper)(struct dsa_switch *ds, int port,
> + struct netdev_notifier_changeupper_info *info);
> +
> /*
> * Bridge integration
> */
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 3bc5ca40c9fb..1919a025c06f 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1987,10 +1987,22 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb,
> switch (event) {
> case NETDEV_PRECHANGEUPPER: {
> struct netdev_notifier_changeupper_info *info = ptr;
> + struct dsa_switch *ds;
> + struct dsa_port *dp;
> + int err;
>
> if (!dsa_slave_dev_check(dev))
> return dsa_prevent_bridging_8021q_upper(dev, ptr);
>
> + dp = dsa_slave_to_port(dev);
> + ds = dp->ds;
> +
> + if (ds->ops->port_prechangeupper) {
> + err = ds->ops->port_prechangeupper(ds, dp->index, ptr);
I would pass 'info' instead of 'ptr' here even if there is no functional
difference, this would be clearer. Not a reason to resubmit if
everything else is fine in this series:
Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
--
Florian
Powered by blists - more mailing lists