[net-next,2/6] net: dsa: vsc73xx: add port_stp_state_set function
Commit Message
This isn't fully functional implementation of 802.1D, but
port_stp_state_set is required for future tag8021q operations.
This implementation handle properly all states, but vsc 73xx don't
forward STP packets.
Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
drivers/net/dsa/vitesse-vsc73xx-core.c | 51 +++++++++++++++++++++++---
drivers/net/dsa/vitesse-vsc73xx.h | 1 +
2 files changed, 46 insertions(+), 6 deletions(-)
Comments
> + struct vsc73xx *vsc = ds->priv;
> + /* FIXME: STP frames isn't forwarded at this moment. BPDU frames are
> + * forwarded only from to PI/SI interface. For more info see chapter
> + * 2.7.1 (CPU Forwarding) in datasheet.
Do you mean the CPU never gets to see the BPDU frames?
Does the hardware have any sort of packet matching to trap frames to
the CPU? Can you match on the destination MAC address
01:80:C2:00:00:00 ?
Andrew
śr., 21 cze 2023 o 21:33 Andrew Lunn <andrew@lunn.ch> napisał(a):
>
> > + struct vsc73xx *vsc = ds->priv;
> > + /* FIXME: STP frames isn't forwarded at this moment. BPDU frames are
> > + * forwarded only from to PI/SI interface. For more info see chapter
> > + * 2.7.1 (CPU Forwarding) in datasheet.
>
> Do you mean the CPU never gets to see the BPDU frames?
>
> Does the hardware have any sort of packet matching to trap frames to
> the CPU? Can you match on the destination MAC address
> 01:80:C2:00:00:00 ?
>
Analyzer in VSC73XX switches can send some kind of packages to (and
from) processor via registers available from SPI/Platform BUS (for
some external analysis). In some cases it's possible to configure: if
packet will be copied or forwarded to this special CPU queue. But
BPDU frames could be sent to processor via CPU queue only. So It's
impossible to forward bridge control data via rgmii interface.
--
Pawel Dembicki
On Wed, Jun 21, 2023 at 9:33 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > + struct vsc73xx *vsc = ds->priv;
> > + /* FIXME: STP frames isn't forwarded at this moment. BPDU frames are
> > + * forwarded only from to PI/SI interface. For more info see chapter
> > + * 2.7.1 (CPU Forwarding) in datasheet.
>
> Do you mean the CPU never gets to see the BPDU frames?
>
> Does the hardware have any sort of packet matching to trap frames to
> the CPU? Can you match on the destination MAC address
> 01:80:C2:00:00:00 ?
The hardware contains an embedded Intel 8054 CPU that can
execute programs to do pretty much anything.
The bad news: it requires a custom SDK thingy that we do not
have access to.
So far we used the chips in a bit of vanilla mode, which is all I
have ever seen in the systems we have and it can't do much,
not even add a helpful frame tag, but as can be seen from the
patches it can do VLAN...
Yours,
Linus Walleij
On Wed, Jun 21, 2023 at 9:13 PM Pawel Dembicki <paweldembicki@gmail.com> wrote:
> This isn't fully functional implementation of 802.1D, but
> port_stp_state_set is required for future tag8021q operations.
>
> This implementation handle properly all states, but vsc 73xx don't
> forward STP packets.
>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
I think it is a best effort and should be merged.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
On Wed, Jun 21, 2023 at 10:38:22PM +0200, Paweł Dembicki wrote:
> śr., 21 cze 2023 o 21:33 Andrew Lunn <andrew@lunn.ch> napisał(a):
> >
> > > + struct vsc73xx *vsc = ds->priv;
> > > + /* FIXME: STP frames isn't forwarded at this moment. BPDU frames are
> > > + * forwarded only from to PI/SI interface. For more info see chapter
> > > + * 2.7.1 (CPU Forwarding) in datasheet.
> >
> > Do you mean the CPU never gets to see the BPDU frames?
> >
> > Does the hardware have any sort of packet matching to trap frames to
> > the CPU? Can you match on the destination MAC address
> > 01:80:C2:00:00:00 ?
> >
>
> Analyzer in VSC73XX switches can send some kind of packages to (and
> from) processor via registers available from SPI/Platform BUS (for
> some external analysis). In some cases it's possible to configure: if
> packet will be copied or forwarded to this special CPU queue. But
> BPDU frames could be sent to processor via CPU queue only. So It's
> impossible to forward bridge control data via rgmii interface.
So am i correct in saying, if you actually enable STP, and it decides
to block a port, the BPDUs are also blocked. After a while it will
decide the peer has gone, and unblock the port. A broadcast storm will
then happen for a while, until a BPDU is received, at which point it
will block the port again.
Andrew
On Thu, Jun 22, 2023 at 03:01:02PM +0200, Andrew Lunn wrote:
> On Wed, Jun 21, 2023 at 10:38:22PM +0200, Paweł Dembicki wrote:
> > śr., 21 cze 2023 o 21:33 Andrew Lunn <andrew@lunn.ch> napisał(a):
> > >
> > > > + struct vsc73xx *vsc = ds->priv;
> > > > + /* FIXME: STP frames isn't forwarded at this moment. BPDU frames are
> > > > + * forwarded only from to PI/SI interface. For more info see chapter
> > > > + * 2.7.1 (CPU Forwarding) in datasheet.
> > >
> > > Do you mean the CPU never gets to see the BPDU frames?
> > >
> > > Does the hardware have any sort of packet matching to trap frames to
> > > the CPU? Can you match on the destination MAC address
> > > 01:80:C2:00:00:00 ?
> > >
> >
> > Analyzer in VSC73XX switches can send some kind of packages to (and
> > from) processor via registers available from SPI/Platform BUS (for
> > some external analysis). In some cases it's possible to configure: if
> > packet will be copied or forwarded to this special CPU queue. But
> > BPDU frames could be sent to processor via CPU queue only. So It's
> > impossible to forward bridge control data via rgmii interface.
>
> So am i correct in saying, if you actually enable STP, and it decides
> to block a port, the BPDUs are also blocked. After a while it will
> decide the peer has gone, and unblock the port. A broadcast storm will
> then happen for a while, until a BPDU is received, at which point it
> will block the port again.
>
> Andrew
This is pretty much the expected behavior from a tag_8021q based
implementation with no hardware assist for control packets. tag_8021q
can provide port identification, but it cannot transform a data packet
into a control packet and it cannot force the switch to accept packets
from ports whose data plane is disabled by STP.
I am also going to review this series in the following days, but I don't
have the required amount of time right now. Perhaps during the weekend.
On Wed, 21 Jun 2023 21:12:58 +0200 Pawel Dembicki wrote:
> diff --git a/drivers/net/dsa/vitesse-vsc73xx.h b/drivers/net/dsa/vitesse-vsc73xx.h
> index 30b1f0a36566..1552a9ca06ff 100644
> --- a/drivers/net/dsa/vitesse-vsc73xx.h
> +++ b/drivers/net/dsa/vitesse-vsc73xx.h
> @@ -15,6 +15,7 @@ struct vsc73xx {
> u8 addr[ETH_ALEN];
> const struct vsc73xx_ops *ops;
> void *priv;
> + u8 forward_map[8];
> };
kdoc missing here:
> drivers/net/dsa/vitesse-vsc73xx.h:20: warning: Function parameter or member 'forward_map' not described in 'vsc73xx'
On Wed, Jun 21, 2023 at 11:27:14PM +0200, Linus Walleij wrote:
> On Wed, Jun 21, 2023 at 9:33 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > + struct vsc73xx *vsc = ds->priv;
> > > + /* FIXME: STP frames isn't forwarded at this moment. BPDU frames are
> > > + * forwarded only from to PI/SI interface. For more info see chapter
> > > + * 2.7.1 (CPU Forwarding) in datasheet.
> >
> > Do you mean the CPU never gets to see the BPDU frames?
> >
> > Does the hardware have any sort of packet matching to trap frames to
> > the CPU? Can you match on the destination MAC address
> > 01:80:C2:00:00:00 ?
>
> The hardware contains an embedded Intel 8054 CPU that can
> execute programs to do pretty much anything.
>
> The bad news: it requires a custom SDK thingy that we do not
> have access to.
>
> So far we used the chips in a bit of vanilla mode, which is all I
> have ever seen in the systems we have and it can't do much,
> not even add a helpful frame tag, but as can be seen from the
> patches it can do VLAN...
>
> Yours,
> Linus Walleij
But even without involving the iCPU, it should be possible to inject/extract
control packets over the SI interface, using the CPU_CAPT and CPUTXDAT block
registers, correct?
IIUC, ocelot with tag_8021q does just that for STP and PTP, see
ocelot_port_inject_frame() and ocelot_xtr_poll_frame().
niedz., 25 cze 2023 o 13:21 Vladimir Oltean <olteanv@gmail.com> napisał(a):
>
> On Wed, Jun 21, 2023 at 11:27:14PM +0200, Linus Walleij wrote:
> > On Wed, Jun 21, 2023 at 9:33 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > > + struct vsc73xx *vsc = ds->priv;
> > > > + /* FIXME: STP frames isn't forwarded at this moment. BPDU frames are
> > > > + * forwarded only from to PI/SI interface. For more info see chapter
> > > > + * 2.7.1 (CPU Forwarding) in datasheet.
> > >
> > > Do you mean the CPU never gets to see the BPDU frames?
> > >
> > > Does the hardware have any sort of packet matching to trap frames to
> > > the CPU? Can you match on the destination MAC address
> > > 01:80:C2:00:00:00 ?
> >
> > The hardware contains an embedded Intel 8054 CPU that can
> > execute programs to do pretty much anything.
> >
> > The bad news: it requires a custom SDK thingy that we do not
> > have access to.
> >
> > So far we used the chips in a bit of vanilla mode, which is all I
> > have ever seen in the systems we have and it can't do much,
> > not even add a helpful frame tag, but as can be seen from the
> > patches it can do VLAN...
> >
> > Yours,
> > Linus Walleij
>
> But even without involving the iCPU, it should be possible to inject/extract
> control packets over the SI interface, using the CPU_CAPT and CPUTXDAT block
> registers, correct?
Yes , It should work with CPU_CAPT and CPUTXDAT.
>
> IIUC, ocelot with tag_8021q does just that for STP and PTP, see
> ocelot_port_inject_frame() and ocelot_xtr_poll_frame().
I was planning to do it in the next step after making this driver
however usable.
--
Pawel Dembicki
@@ -165,6 +165,10 @@
#define VSC73XX_AGENCTRL 0xf0
#define VSC73XX_CAPRST 0xff
+#define VSC73XX_SRCMASKS_CPU_COPY BIT(27)
+#define VSC73XX_SRCMASKS_MIRROR BIT(26)
+#define VSC73XX_SRCMASKS_PORTS_MASK GENMASK(7, 0)
+
#define VSC73XX_MACACCESS_CPU_COPY BIT(14)
#define VSC73XX_MACACCESS_FWD_KILL BIT(13)
#define VSC73XX_MACACCESS_IGNORE_VLAN BIT(12)
@@ -621,15 +625,17 @@ static int vsc73xx_setup(struct dsa_switch *ds)
vsc73xx_write(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_GMIIDELAY,
VSC73XX_GMIIDELAY_GMII0_GTXDELAY_2_0_NS |
VSC73XX_GMIIDELAY_GMII0_RXDELAY_2_0_NS);
- /* Enable reception of frames on all ports */
- vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_RECVMASK,
- 0x5f);
/* IP multicast flood mask (table 144) */
vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_IFLODMSK,
0xff);
mdelay(50);
+ /*configure forward map to CPU <-> port only*/
+ for (i = 0; i < vsc->ds->num_ports; i++)
+ vsc->forward_map[i] = VSC73XX_SRCMASKS_PORTS_MASK & BIT(CPU_PORT);
+ vsc->forward_map[CPU_PORT] = VSC73XX_SRCMASKS_PORTS_MASK & ~BIT(CPU_PORT);
+
/* Release reset from the internal PHYs */
vsc73xx_write(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_GLORESET,
VSC73XX_GLORESET_PHY_RESET);
@@ -885,9 +891,6 @@ static void vsc73xx_phylink_mac_link_up(struct dsa_switch *ds, int port,
break;
}
- /* Enable port (forwarding) in the receieve mask */
- vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
- VSC73XX_RECVMASK, BIT(port), BIT(port));
vsc73xx_adjust_enable_port(vsc, port, val);
}
@@ -1054,6 +1057,41 @@ static int vsc73xx_get_max_mtu(struct dsa_switch *ds, int port)
return 9600;
}
+static void vsc73xx_port_stp_state_set(struct dsa_switch *ds, int port,
+ u8 state)
+{
+ struct vsc73xx *vsc = ds->priv;
+ /* FIXME: STP frames isn't forwarded at this moment. BPDU frames are
+ * forwarded only from to PI/SI interface. For more info see chapter
+ * 2.7.1 (CPU Forwarding) in datasheet.
+ * This function is required for tag8021q operations.
+ */
+
+ if (state == BR_STATE_BLOCKING)
+ 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]);
+ 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,
@@ -1070,6 +1108,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)
@@ -15,6 +15,7 @@ struct vsc73xx {
u8 addr[ETH_ALEN];
const struct vsc73xx_ops *ops;
void *priv;
+ u8 forward_map[8];
};
struct vsc73xx_ops {