| 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: <CAJN1Kkyn4V2FNVdZZMWHTSqP+=5bKacKSEpkF5t4sNptc1vtCg@mail.gmail.com>
Date: Tue, 12 Sep 2023 17:27:45 +0200
From: Paweł Dembicki <paweldembicki@...il.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: netdev@...r.kernel.org, Dan Carpenter <dan.carpenter@...aro.org>,
Simon Horman <simon.horman@...igine.com>, Linus Walleij <linus.walleij@...aro.org>,
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>,
Russell King <linux@...linux.org.uk>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v3 4/8] net: dsa: vsc73xx: add port_stp_state_set function
wt., 12 wrz 2023 o 16:48 Vladimir Oltean <olteanv@...il.com> napisał(a):
>
> Hi Pawel,
>
Hi Vladimir,
Thank you for Your time.
> On Tue, Sep 12, 2023 at 02:21:58PM +0200, Pawel Dembicki wrote:
> > This isn't a fully functional implementation of 802.1D, but
> > port_stp_state_set is required for a future tag8021q operations.
> >
> > This implementation handles properly all states, but vsc73xx doesn't
> > forward STP packets.
> >
> > Reviewed-by: Linus Walleij <linus.walleij@...aro.org>
> > Signed-off-by: Pawel Dembicki <paweldembicki@...il.com>
> > ---
> > diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
> > index 8f2285a03e82..541fbc195df1 100644
> > --- a/drivers/net/dsa/vitesse-vsc73xx-core.c
> > +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
> > @@ -1033,9 +1031,59 @@ static int vsc73xx_get_max_mtu(struct dsa_switch *ds, int port)
> > return 9600 - ETH_HLEN - ETH_FCS_LEN;
> > }
> >
> > +static int vsc73xx_port_setup(struct dsa_switch *ds, int port)
> > +{
>
> For bisectability, the series must build patch by patch.
> Here, you are missing:
>
> struct vsc73xx *vsc = ds->priv;
>
> ../drivers/net/dsa/vitesse-vsc73xx-core.c:1038:3: error: use of undeclared identifier 'vsc'
> vsc->forward_map[CPU_PORT] = VSC73XX_SRCMASKS_PORTS_MASK &
> ^
> ../drivers/net/dsa/vitesse-vsc73xx-core.c:1041:3: error: use of undeclared identifier 'vsc'
> vsc->forward_map[port] = VSC73XX_SRCMASKS_PORTS_MASK &
> ^
> 2 errors generated.
>
> > + /* Configure forward map to CPU <-> port only */
> > + if (port == CPU_PORT)
> > + vsc->forward_map[CPU_PORT] = VSC73XX_SRCMASKS_PORTS_MASK &
> > + ~BIT(CPU_PORT);
>
> vsc->forward_map[CPU_PORT] = dsa_user_ports(ds);
>
> > + else
> > + vsc->forward_map[port] = VSC73XX_SRCMASKS_PORTS_MASK &
> > + BIT(CPU_PORT);
>
> vsc->forward_map[port] = BIT(CPU_PORT);
>
> > +
> > + return 0;
> > +}
> > +
> > +/* FIXME: STP frames aren't forwarded at this moment. BPDU frames are
> > + * forwarded only from and to PI/SI interface. For more info see chapter
> > + * 2.7.1 (CPU Forwarding) in datasheet.
> > + * This function is required for tag8021q operations.
> > + */
> > +
> > +static void vsc73xx_port_stp_state_set(struct dsa_switch *ds, int port,
> > + u8 state)
> > +{
> > + struct vsc73xx *vsc = ds->priv;
> > +
> > + if (state == BR_STATE_BLOCKING || state == BR_STATE_DISABLED)
> > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
> > + VSC73XX_RECVMASK, BIT(port), 0);
> > + else
> > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
> > + VSC73XX_RECVMASK, BIT(port), BIT(port));
> > +
> > + if (state == BR_STATE_LEARNING || state == BR_STATE_FORWARDING)
> > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
> > + VSC73XX_LEARNMASK, BIT(port), BIT(port));
> > + else
> > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
> > + VSC73XX_LEARNMASK, BIT(port), 0);
> > +
> > + if (state == BR_STATE_FORWARDING)
> > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
> > + VSC73XX_SRCMASKS + port,
> > + VSC73XX_SRCMASKS_PORTS_MASK,
> > + vsc->forward_map[port]);
>
> To forward a packet between port A and port B, both of them must be in
> BR_STATE_FORWARDING, not just A.
>
In this patch bridges are unimplemented. Please look at 8/8 patch [0].
> > + else
> > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
> > + VSC73XX_SRCMASKS + port,
> > + VSC73XX_SRCMASKS_PORTS_MASK, 0);
> > +}
> > +
> > static const struct dsa_switch_ops vsc73xx_ds_ops = {
> > .get_tag_protocol = vsc73xx_get_tag_protocol,
> > .setup = vsc73xx_setup,
> > + .port_setup = vsc73xx_port_setup,
> > .phy_read = vsc73xx_phy_read,
> > .phy_write = vsc73xx_phy_write,
> > .phylink_get_caps = vsc73xx_phylink_get_caps,
> > @@ -1049,6 +1097,7 @@ static const struct dsa_switch_ops vsc73xx_ds_ops = {
> > .port_disable = vsc73xx_port_disable,
> > .port_change_mtu = vsc73xx_change_mtu,
> > .port_max_mtu = vsc73xx_get_max_mtu,
> > + .port_stp_state_set = vsc73xx_port_stp_state_set,
> > };
> >
> > static int vsc73xx_gpio_get(struct gpio_chip *chip, unsigned int offset)
> > diff --git a/drivers/net/dsa/vitesse-vsc73xx.h b/drivers/net/dsa/vitesse-vsc73xx.h
> > index f79d81ef24fb..224e284a5573 100644
> > --- a/drivers/net/dsa/vitesse-vsc73xx.h
> > +++ b/drivers/net/dsa/vitesse-vsc73xx.h
> > @@ -18,6 +18,7 @@
> >
> > /**
> > * struct vsc73xx - VSC73xx state container
> > + * @forward_map: Forward table cache
>
> If you start describing the member fields, shouldn't all be described?
> I think there will be kdoc warnings otherwise.
>
Jakub in v1 series points kdoc warn in this case. I added a
description to the field added by me. Should I prepare in the v4
series a separate commit for other descriptions in this struct?
> > */
> > struct vsc73xx {
> > struct device *dev;
> > @@ -28,6 +29,7 @@ struct vsc73xx {
> > u8 addr[ETH_ALEN];
> > const struct vsc73xx_ops *ops;
> > void *priv;
> > + u8 forward_map[VSC73XX_MAX_NUM_PORTS];
> > };
> >
> > struct vsc73xx_ops {
> > --
> > 2.34.1
> >
[0] https://lore.kernel.org/netdev/20230912122201.3752918-9-paweldembicki@gmail.com/T/#u
Powered by blists - more mailing lists