| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20231122132804.3tqmhj2aahmqcf7o@DEN-DL-M31836.microchip.com>
Date: Wed, 22 Nov 2023 14:28:04 +0100
From: Horatiu Vultur <horatiu.vultur@...rochip.com>
To: Vladimir Oltean <olteanv@...il.com>
CC: Rasmus Villemoes <rasmus.villemoes@...vas.dk>, Woojung Huh
<woojung.huh@...rochip.com>, <UNGLinuxDriver@...rochip.com>, Andrew Lunn
<andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, <netdev@...r.kernel.org>,
Per Noergaard Christensen <per.christensen@...vas.dk>
Subject: Re: [PATCH net-next] net: dsa: microchip: add MRP software ring
support
The 11/22/2023 13:35, Vladimir Oltean wrote:
Hi Rasmus,
>
> On Wed, Nov 22, 2023 at 12:20:06PM +0100, Rasmus Villemoes wrote:
> > From: Per Noergaard Christensen <per.christensen@...vas.dk>
> >
> > Add dummy functions that tells the MRP bridge instance to use
> > implemented software routines instead of hardware-offloading.
> >
> > Signed-off-by: Per Noergaard Christensen <per.christensen@...vas.dk>
> > Signed-off-by: Rasmus Villemoes <rasmus.villemoes@...vas.dk>
> > ---
> > drivers/net/dsa/microchip/ksz_common.c | 55 ++++++++++++++++++++++++++
> > drivers/net/dsa/microchip/ksz_common.h | 1 +
> > 2 files changed, 56 insertions(+)
> >
> > diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> > index 3fed406fb46a..b0935997dc05 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.c
> > +++ b/drivers/net/dsa/microchip/ksz_common.c
> > @@ -3566,6 +3566,57 @@ static int ksz_set_wol(struct dsa_switch *ds, int port,
> > return -EOPNOTSUPP;
> > }
> >
> > +static int ksz_port_mrp_add(struct dsa_switch *ds, int port,
> > + const struct switchdev_obj_mrp *mrp)
> > +{
> > + struct dsa_port *dp = dsa_to_port(ds, port);
> > + struct ksz_device *dev = ds->priv;
> > +
> > + /* port different from requested mrp ports */
> > + if (mrp->p_port != dp->user && mrp->s_port != dp->user)
> > + return -EOPNOTSUPP;
> > +
> > + /* save ring id */
> > + dev->ports[port].mrp_ring_id = mrp->ring_id;
> > + return 0;
> > +}
> > +
> > +static int ksz_port_mrp_del(struct dsa_switch *ds, int port,
> > + const struct switchdev_obj_mrp *mrp)
> > +{
> > + struct ksz_device *dev = ds->priv;
> > +
> > + /* check if port not part of ring id */
> > + if (mrp->ring_id != dev->ports[port].mrp_ring_id)
> > + return -EOPNOTSUPP;
> > +
> > + /* clear ring id */
> > + dev->ports[port].mrp_ring_id = 0;
> > + return 0;
> > +}
> > +
> > +static int ksz_port_mrp_add_ring_role(struct dsa_switch *ds, int port,
> > + const struct switchdev_obj_ring_role_mrp *mrp)
> > +{
> > + struct ksz_device *dev = ds->priv;
> > +
> > + if (mrp->sw_backup && dev->ports[port].mrp_ring_id == mrp->ring_id)
> > + return 0;
As Vladimir mentioned, you should add some rules to trap all MRP frames.
Otherwise, if you configure as MRC then you need to foward the TEST
frames and copy to CPU the CONTROL frames. And if you start to have more
than 2 ports under the bridge, then the traffic will be flooded on the
other ports that are not part of the MRP ring.
If you configure as MRM then you will never terminate the frames
otherwise the HW will just forward them. And of course you will have the
same issue if there are more than 2 ports under the bridge.
I hope I didn't forget everything (as I didn't look into this for some
time). But willing to look more into if it is needed.
> > +
> > + return -EOPNOTSUPP;
> > +}
> > +
> > +static int ksz_port_mrp_del_ring_role(struct dsa_switch *ds, int port,
> > + const struct switchdev_obj_ring_role_mrp *mrp)
> > +{
> > + struct ksz_device *dev = ds->priv;
> > +
> > + if (mrp->sw_backup && dev->ports[port].mrp_ring_id == mrp->ring_id)
> > + return 0;
> > +
> > + return -EOPNOTSUPP;
> > +}
> > +
> > static int ksz_port_set_mac_address(struct dsa_switch *ds, int port,
> > const unsigned char *addr)
> > {
> > @@ -3799,6 +3850,10 @@ static const struct dsa_switch_ops ksz_switch_ops = {
> > .port_fdb_del = ksz_port_fdb_del,
> > .port_mdb_add = ksz_port_mdb_add,
> > .port_mdb_del = ksz_port_mdb_del,
> > + .port_mrp_add = ksz_port_mrp_add,
> > + .port_mrp_del = ksz_port_mrp_del,
> > + .port_mrp_add_ring_role = ksz_port_mrp_add_ring_role,
> > + .port_mrp_del_ring_role = ksz_port_mrp_del_ring_role,
> > .port_mirror_add = ksz_port_mirror_add,
> > .port_mirror_del = ksz_port_mirror_del,
> > .get_stats64 = ksz_get_stats64,
> > diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> > index b7e8a403a132..24015f0a9c98 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.h
> > +++ b/drivers/net/dsa/microchip/ksz_common.h
> > @@ -110,6 +110,7 @@ struct ksz_port {
> > bool remove_tag; /* Remove Tag flag set, for ksz8795 only */
> > bool learning;
> > int stp_state;
> > + u32 mrp_ring_id;
> > struct phy_device phydev;
> >
> > u32 fiber:1; /* port is fiber */
> > --
> > 2.40.1.1.g1c60b9335d
> >
>
> Could you please explain a bit the mechanics of this dummy implementation?
> Don't you need to set up any packet traps for MRP PDUs, to avoid
> forwarding them? What ring roles will work with the dummy implementation?
>
> +Horatiu for an expert opinion.
>
--
/Horatiu
Powered by blists - more mailing lists