Re: [PATCH v3 net-next 04/10] net: dsa: tag_ksz: add tag handling for Microchip LAN937x
From: Vladimir Oltean <olteanv@gmail.com>
Date: 2021-07-23 19:23:58
Also in:
lkml, netdev
On Fri, Jul 23, 2021 at 11:01:02PM +0530, Prasanna Vengateshan wrote:
quoted hunk ↗ jump to hunk
--- a/net/dsa/tag_ksz.c +++ b/net/dsa/tag_ksz.c@@ -187,10 +187,66 @@ static const struct dsa_device_ops ksz9893_netdev_ops = { DSA_TAG_DRIVER(ksz9893_netdev_ops); MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893); +/* For xmit, 2 bytes are added before FCS. + * --------------------------------------------------------------------------- + * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes) + * --------------------------------------------------------------------------- + * tag0 : represents tag override, lookup and valid + * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x80=port8) + * + * For rcv, 1 byte is added before FCS. + * --------------------------------------------------------------------------- + * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes) + * --------------------------------------------------------------------------- + * tag0 : zero-based value represents port + * (eg, 0x00=port1, 0x02=port3, 0x07=port8) + */ +#define LAN937X_EGRESS_TAG_LEN 2 + +#define LAN937X_TAIL_TAG_BLOCKING_OVERRIDE BIT(11) +#define LAN937X_TAIL_TAG_LOOKUP BIT(12) +#define LAN937X_TAIL_TAG_VALID BIT(13) +#define LAN937X_TAIL_TAG_PORT_MASK 7 + +static struct sk_buff *lan937x_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); + __be16 *tag; + u16 val;
There was a recent patch on net.git to fix an issue with DSA master drivers which set NETIF_F_HW_CSUM in dev->vlan_features, which DSA inherits. https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=37120f23ac8998c250573ea3247ff77426551f69 Until we fix the issue at the DSA layer, could you also apply that fix here please?
+ + tag = skb_put(skb, LAN937X_EGRESS_TAG_LEN); + + val = BIT(dp->index); + + if (is_link_local_ether_addr(hdr->h_dest)) + val |= LAN937X_TAIL_TAG_BLOCKING_OVERRIDE; + + /* Tail tag valid bit - This bit should always be set by the CPU*/ + val |= LAN937X_TAIL_TAG_VALID;
Please add an extra space here between "CPU" and the comment ending delimiter.
+ + *tag = cpu_to_be16(val);
This probably works for the arch you are testing on, but it is better to avoid unaligned accesses when the skb->len is odd: https://www.kernel.org/doc/Documentation/unaligned-memory-access.txt You can use put_unaligned_be16().
+ + return skb; +}