2023-02-02 12:58:50

by Rakesh Sankaranarayanan

[permalink] [raw]
Subject: [RFC PATCH net-next 01/11] net: dsa: microchip: lan937x: add cascade tailtag

cascade tailtag contains 3 bytes of information, it includes
additional bytes for accomodating port number in second switch.
Destination port bitmap on first switch is at bit position 7:0 and
of second switch is at bit position 15:8, add new tailtag xmit and
rcv functions for cascade with proper formatting. Add new tag protocol
for cascading and link with new xmit and rcv functions.

Signed-off-by: Rakesh Sankaranarayanan <[email protected]>
---
include/net/dsa.h | 2 ++
net/dsa/tag_ksz.c | 80 ++++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 77 insertions(+), 5 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index a15f17a38eca..55651ad29193 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -56,6 +56,7 @@ struct phylink_link_state;
#define DSA_TAG_PROTO_RTL8_4T_VALUE 25
#define DSA_TAG_PROTO_RZN1_A5PSW_VALUE 26
#define DSA_TAG_PROTO_LAN937X_VALUE 27
+#define DSA_TAG_PROTO_LAN937X_CASCADE_VALUE 28

enum dsa_tag_protocol {
DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE,
@@ -86,6 +87,7 @@ enum dsa_tag_protocol {
DSA_TAG_PROTO_RTL8_4T = DSA_TAG_PROTO_RTL8_4T_VALUE,
DSA_TAG_PROTO_RZN1_A5PSW = DSA_TAG_PROTO_RZN1_A5PSW_VALUE,
DSA_TAG_PROTO_LAN937X = DSA_TAG_PROTO_LAN937X_VALUE,
+ DSA_TAG_PROTO_LAN937X_CASCADE = DSA_TAG_PROTO_LAN937X_CASCADE_VALUE,
};

struct dsa_switch;
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index 0eb1c7784c3d..7ab2c7eaa4ca 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -16,6 +16,7 @@
#define KSZ9477_NAME "ksz9477"
#define KSZ9893_NAME "ksz9893"
#define LAN937X_NAME "lan937x"
+#define LAN937X_CASCADE_NAME "lan937x_cascade"

/* Typically only one byte is used for tail tag. */
#define KSZ_PTP_TAG_LEN 4
@@ -24,6 +25,9 @@

#define KSZ_HWTS_EN 0

+#define SWITCH_0 0
+#define SWITCH_1 1
+
struct ksz_tagger_private {
struct ksz_tagger_data data; /* Must be first */
unsigned long state;
@@ -84,10 +88,10 @@ static int ksz_connect(struct dsa_switch *ds)
}

static struct sk_buff *ksz_common_rcv(struct sk_buff *skb,
- struct net_device *dev,
- unsigned int port, unsigned int len)
+ struct net_device *dev, unsigned int port,
+ unsigned int len, u8 device)
{
- skb->dev = dsa_master_find_slave(dev, 0, port);
+ skb->dev = dsa_master_find_slave(dev, device, port);
if (!skb->dev)
return NULL;

@@ -141,7 +145,7 @@ static struct sk_buff *ksz8795_rcv(struct sk_buff *skb, struct net_device *dev)
{
u8 *tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;

- return ksz_common_rcv(skb, dev, tag[0] & 7, KSZ_EGRESS_TAG_LEN);
+ return ksz_common_rcv(skb, dev, tag[0] & 7, KSZ_EGRESS_TAG_LEN, SWITCH_0);
}

static const struct dsa_device_ops ksz8795_netdev_ops = {
@@ -177,6 +181,7 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ8795, KSZ8795_NAME);
#define KSZ9477_INGRESS_TAG_LEN 2
#define KSZ9477_PTP_TAG_LEN 4
#define KSZ9477_PTP_TAG_INDICATION 0x80
+#define LAN937X_CASCADE_CHIP 0x40

#define KSZ9477_TAIL_TAG_PRIO GENMASK(8, 7)
#define KSZ9477_TAIL_TAG_OVERRIDE BIT(9)
@@ -304,6 +309,7 @@ static struct sk_buff *ksz9477_rcv(struct sk_buff *skb, struct net_device *dev)
u8 *tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;
unsigned int port = tag[0] & 7;
unsigned int len = KSZ_EGRESS_TAG_LEN;
+ u8 device = SWITCH_0;

/* Extra 4-bytes PTP timestamp */
if (tag[0] & KSZ9477_PTP_TAG_INDICATION) {
@@ -311,7 +317,10 @@ static struct sk_buff *ksz9477_rcv(struct sk_buff *skb, struct net_device *dev)
len += KSZ_PTP_TAG_LEN;
}

- return ksz_common_rcv(skb, dev, port, len);
+ if (tag[0] & LAN937X_CASCADE_CHIP)
+ device = SWITCH_1;
+
+ return ksz_common_rcv(skb, dev, port, len, device);
}

static const struct dsa_device_ops ksz9477_netdev_ops = {
@@ -390,6 +399,7 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893, KSZ9893_NAME);
* (eg, 0x00=port1, 0x02=port3, 0x07=port8)
*/
#define LAN937X_EGRESS_TAG_LEN 2
+#define LAN937X_CASCADE_TAG_LEN 3

#define LAN937X_TAIL_TAG_BLOCKING_OVERRIDE BIT(11)
#define LAN937X_TAIL_TAG_LOOKUP BIT(12)
@@ -442,11 +452,71 @@ static const struct dsa_device_ops lan937x_netdev_ops = {
DSA_TAG_DRIVER(lan937x_netdev_ops);
MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN937X, LAN937X_NAME);

+/* For xmit, 3/7 bytes are added before FCS.
+ * ---------------------------------------------------------------------------
+ * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|ts(4bytes)|tag0(1byte)|tag1(1byte)|
+ * tag2(1byte)|FCS(4bytes)
+ * ---------------------------------------------------------------------------
+ * ts : time stamp (Present only if PTP is enabled in the Hardware)
+ * tag0 : represents tag override, lookup and valid
+ * tag1 : each bit represents destination port map through switch 2
+ * (eg, 0x01=port1, 0x02=port2, 0x80=port8)
+ * tag2 : each bit represents destination port map through switch 1
+ * (eg, 0x01=port1, 0x02=port2, 0x80=port8)
+ *
+ * For rcv, 1/5 bytes is added before FCS.
+ * ---------------------------------------------------------------------------
+ * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|ts(4bytes)|tag0(1byte)|FCS(4bytes)
+ * ---------------------------------------------------------------------------
+ * ts : time stamp (Present only if bit 7 of tag0 is set)
+ * tag0 : zero-based value represents port
+ * (eg, 0x00=port1, 0x02=port3, 0x07=port8)
+ */
+static struct sk_buff *lan937x_cascade_xmit(struct sk_buff *skb,
+ struct net_device *dev)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+ const struct ethhdr *hdr = eth_hdr(skb);
+ __be32 *tag;
+ u32 val;
+
+ if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
+ return NULL;
+
+ tag = skb_put(skb, LAN937X_CASCADE_TAG_LEN);
+
+ val |= BIT((dp->index + (8 * dp->ds->index)));
+
+ if (is_link_local_ether_addr(hdr->h_dest))
+ val |= (LAN937X_TAIL_TAG_BLOCKING_OVERRIDE << 8);
+
+ val |= (LAN937X_TAIL_TAG_VALID << 8);
+
+ put_unaligned_be24(val, tag);
+
+ return skb;
+}
+
+static const struct dsa_device_ops lan937x_cascade_netdev_ops = {
+ .name = LAN937X_CASCADE_NAME,
+ .proto = DSA_TAG_PROTO_LAN937X_CASCADE,
+ .xmit = lan937x_cascade_xmit,
+ .rcv = ksz9477_rcv,
+ .connect = ksz_connect,
+ .disconnect = ksz_disconnect,
+ .needed_tailroom = LAN937X_CASCADE_TAG_LEN,
+};
+
+DSA_TAG_DRIVER(lan937x_cascade_netdev_ops);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN937x_CASCADE,
+ LAN937X_CASCADE_NAME);
+
static struct dsa_tag_driver *dsa_tag_driver_array[] = {
&DSA_TAG_DRIVER_NAME(ksz8795_netdev_ops),
&DSA_TAG_DRIVER_NAME(ksz9477_netdev_ops),
&DSA_TAG_DRIVER_NAME(ksz9893_netdev_ops),
&DSA_TAG_DRIVER_NAME(lan937x_netdev_ops),
+ &DSA_TAG_DRIVER_NAME(lan937x_cascade_netdev_ops),
};

module_dsa_tag_drivers(dsa_tag_driver_array);
--
2.34.1



2023-02-03 23:10:16

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [RFC PATCH net-next 01/11] net: dsa: microchip: lan937x: add cascade tailtag

On Thu, Feb 02, 2023 at 06:29:20PM +0530, Rakesh Sankaranarayanan wrote:
> cascade tailtag contains 3 bytes of information, it includes
> additional bytes for accomodating port number in second switch.
> Destination port bitmap on first switch is at bit position 7:0 and
> of second switch is at bit position 15:8, add new tailtag xmit and
> rcv functions for cascade with proper formatting. Add new tag protocol
> for cascading and link with new xmit and rcv functions.
>
> Signed-off-by: Rakesh Sankaranarayanan <[email protected]>
> ---
> include/net/dsa.h | 2 ++
> net/dsa/tag_ksz.c | 80 ++++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 77 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index a15f17a38eca..55651ad29193 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -390,6 +399,7 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893, KSZ9893_NAME);
> * (eg, 0x00=port1, 0x02=port3, 0x07=port8)
> */
> #define LAN937X_EGRESS_TAG_LEN 2
> +#define LAN937X_CASCADE_TAG_LEN 3
>
> #define LAN937X_TAIL_TAG_BLOCKING_OVERRIDE BIT(11)
> #define LAN937X_TAIL_TAG_LOOKUP BIT(12)
> @@ -442,11 +452,71 @@ static const struct dsa_device_ops lan937x_netdev_ops = {
> DSA_TAG_DRIVER(lan937x_netdev_ops);
> MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN937X, LAN937X_NAME);
>
> +/* For xmit, 3/7 bytes are added before FCS.
> + * ---------------------------------------------------------------------------
> + * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|ts(4bytes)|tag0(1byte)|tag1(1byte)|
> + * tag2(1byte)|FCS(4bytes)
> + * ---------------------------------------------------------------------------
> + * ts : time stamp (Present only if PTP is enabled in the Hardware)
> + * tag0 : represents tag override, lookup and valid
> + * tag1 : each bit represents destination port map through switch 2
> + * (eg, 0x01=port1, 0x02=port2, 0x80=port8)
> + * tag2 : each bit represents destination port map through switch 1
> + * (eg, 0x01=port1, 0x02=port2, 0x80=port8)
> + *
> + * For rcv, 1/5 bytes is added before FCS.

Plural is only used for more than 5 bytes?
"5 bytes is added" vs "7 bytes are added"

> + * ---------------------------------------------------------------------------
> + * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|ts(4bytes)|tag0(1byte)|FCS(4bytes)
> + * ---------------------------------------------------------------------------
> + * ts : time stamp (Present only if bit 7 of tag0 is set)
> + * tag0 : zero-based value represents port
> + * (eg, 0x00=port1, 0x02=port3, 0x07=port8)
> + */
> +static struct sk_buff *lan937x_cascade_xmit(struct sk_buff *skb,
> + struct net_device *dev)
> +{
> + struct dsa_port *dp = dsa_slave_to_port(dev);
> + const struct ethhdr *hdr = eth_hdr(skb);
> + __be32 *tag;
> + u32 val;
> +
> + if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
> + return NULL;
> +
> + tag = skb_put(skb, LAN937X_CASCADE_TAG_LEN);
> +
> + val |= BIT((dp->index + (8 * dp->ds->index)));

This only works because cascade port is port 0, right?

> +
> + if (is_link_local_ether_addr(hdr->h_dest))
> + val |= (LAN937X_TAIL_TAG_BLOCKING_OVERRIDE << 8);
> +
> + val |= (LAN937X_TAIL_TAG_VALID << 8);
> +
> + put_unaligned_be24(val, tag);
> +
> + return skb;
> +}
> +
> +static const struct dsa_device_ops lan937x_cascade_netdev_ops = {
> + .name = LAN937X_CASCADE_NAME,
> + .proto = DSA_TAG_PROTO_LAN937X_CASCADE,
> + .xmit = lan937x_cascade_xmit,
> + .rcv = ksz9477_rcv,
> + .connect = ksz_connect,
> + .disconnect = ksz_disconnect,
> + .needed_tailroom = LAN937X_CASCADE_TAG_LEN,
> +};

Nope, no new lan937x_cascade tagging protocol.

If you read Documentation/networking/dsa/dsa.rst, it says:

| From the perspective of the network stack, all switches within the same DSA
| switch tree use the same tagging protocol.

Unless you are prepared to remove this assumption from the DSA framework,
you need to fold the cascade tag handling into the regular lan937x
tagging protocol (and declare the larger needed_tailroom to cover the
tail tag case).

You can look at dp->ds->index when figuring out the length of the tail
tag that should be inserted.

There's a lot of consolidation that could (and should) be done first
between lan937x_xmit() and ksz9477_xmit(), prior to adding the support
for cascade tagging.

> +
> +DSA_TAG_DRIVER(lan937x_cascade_netdev_ops);
> +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN937x_CASCADE,
> + LAN937X_CASCADE_NAME);
> +
> static struct dsa_tag_driver *dsa_tag_driver_array[] = {
> &DSA_TAG_DRIVER_NAME(ksz8795_netdev_ops),
> &DSA_TAG_DRIVER_NAME(ksz9477_netdev_ops),
> &DSA_TAG_DRIVER_NAME(ksz9893_netdev_ops),
> &DSA_TAG_DRIVER_NAME(lan937x_netdev_ops),
> + &DSA_TAG_DRIVER_NAME(lan937x_cascade_netdev_ops),
> };
>
> module_dsa_tag_drivers(dsa_tag_driver_array);
> --
> 2.34.1
>