Re: [PATCH RFC net-next 21/23] net: dsa: add tagging driver for MaxLinear GSW1xx switch family

[Andrew Lunn]

> @@ -27,6 +27,7 @@ obj-$(CONFIG_NET_DSA_TAG_GSWIP) += tag_gswip.o
>  obj-$(CONFIG_NET_DSA_TAG_HELLCREEK) += tag_hellcreek.o
>  obj-$(CONFIG_NET_DSA_TAG_KSZ) += tag_ksz.o
>  obj-$(CONFIG_NET_DSA_TAG_LAN9303) += tag_lan9303.o
> +obj-$(CONFIG_NET_DSA_TAG_MXL_GSW1XX) += tag_mxl-gsw1xx.o
>  obj-$(CONFIG_NET_DSA_TAG_MTK) += tag_mtk.o
>  obj-$(CONFIG_NET_DSA_TAG_NONE) += tag_none.o
>  obj-$(CONFIG_NET_DSA_TAG_OCELOT) += tag_ocelot.o

DSA is not particularly good at sorting, many lists are not.
But this list is sorted, and MX > MT.

> +static struct sk_buff *gsw1xx_tag_xmit(struct sk_buff *skb,
> +				       struct net_device *dev)
> +{
> +	struct dsa_port *dp = dsa_user_to_port(dev);
> +	u8 *gsw1xx_tag;
> +
> +	if (!skb)
> +		return skb;

Can that happen?

> +	/* provide additional space 'GSW1XX_TX_HEADER_LEN' bytes */
> +	skb_push(skb, GSW1XX_TX_HEADER_LEN);
> +
> +	/* add space between MAC address and Ethertype */
> +	memmove(skb->data, skb->data + GSW1XX_TX_HEADER_LEN, 2 * ETH_ALEN);

dsa_alloc_etype_header() seems appropriate here.

> +
> +	/* special tag ingress */
> +	gsw1xx_tag = skb->data + 2 * ETH_ALEN;

and dsa_etype_header_pos_tx().

> +	gsw1xx_tag[0] = 0x88;
> +	gsw1xx_tag[1] = 0xc3;
> +	gsw1xx_tag[2] = GSW1XX_TX_PORT_MAP_EN | GSW1XX_TX_LRN_DIS;
> +	gsw1xx_tag[3] = BIT(dp->index + GSW1XX_TX_PORT_MAP_LOW_SHIFT) & GSW1XX_TX_PORT_MAP_LOW_MASK;
> +	gsw1xx_tag[4] = 0;
> +	gsw1xx_tag[5] = 0;
> +	gsw1xx_tag[6] = 0;
> +	gsw1xx_tag[7] = 0;
> +
> +	return skb;
> +}
> +
> +static struct sk_buff *gsw1xx_tag_rcv(struct sk_buff *skb,
> +				      struct net_device *dev)
> +{
> +	int port;
> +	u8 *gsw1xx_tag;
> +
> +	if (unlikely(!pskb_may_pull(skb, GSW1XX_RX_HEADER_LEN))) {
> +		dev_warn_ratelimited(&dev->dev, "Dropping packet, cannot pull SKB\n");
> +		return NULL;
> +	}
> +
> +	gsw1xx_tag = skb->data - 2;

dsa_etype_header_pos_rx()

> +	if (gsw1xx_tag[0] != 0x88 && gsw1xx_tag[1] != 0xc3) {
> +		dev_warn_ratelimited(&dev->dev, "Dropping packet due to invalid special tag\n");
> +		dev_warn_ratelimited(&dev->dev,
> +				     "Tag: 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x\n",
> +				     gsw1xx_tag[0], gsw1xx_tag[1], gsw1xx_tag[2], gsw1xx_tag[3],
> +				     gsw1xx_tag[4], gsw1xx_tag[5], gsw1xx_tag[6], gsw1xx_tag[7]);
> +		return NULL;
> +	}
> +
> +	/* Get source port information */
> +	port = (gsw1xx_tag[2] & GSW1XX_RX_PORT_MAP_LOW_MASK) >> GSW1XX_RX_PORT_MAP_LOW_SHIFT;
> +	skb->dev = dsa_conduit_find_user(dev, 0, port);
> +	if (!skb->dev) {
> +		dev_warn_ratelimited(&dev->dev, "Dropping packet due to invalid source port\n");
> +		dev_warn_ratelimited(&dev->dev,
> +				     "Tag: 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x\n",
> +				     gsw1xx_tag[0], gsw1xx_tag[1], gsw1xx_tag[2], gsw1xx_tag[3],
> +				     gsw1xx_tag[4], gsw1xx_tag[5], gsw1xx_tag[6], gsw1xx_tag[7]);
> +		return NULL;
> +	}
> +
> +	/* remove the GSW1xx special tag between MAC addresses and the current ethertype field. */
> +	skb_pull_rcsum(skb, GSW1XX_RX_HEADER_LEN);
> +	memmove(skb->data - ETH_HLEN, skb->data - (ETH_HLEN + GSW1XX_RX_HEADER_LEN), 2 * ETH_ALEN);

dsa_strip_etype_header() might also be useful here.

    Andrew

---
pw-bot: cr