On 08/10/2018 04:26 AM, Lad, Prabhakar wrote: > Hi Andrew, > > On Thu, Aug 9, 2018 at 6:23 PM Andrew Lunn <and...@lunn.ch> wrote: >> >>> Its coming from the switch lan4 I have attached the png, where >>> C4:F3:12:08:FE:7F is >>> the mac of lan4, which is broadcast to ff:ff:ff:ff:ff:ff, which is >>> causing rx counter on >>> PC to go up. >> >> So, big packets are making it from the switch to the PC. But the small >> ARP packets are not. >> >> This is what Florian was suggesting. >> >> ARP packets are smaller than 64 bytes, which is the minimum packet >> size for Ethernet. Any packets smaller than 64 bytes are called runt >> packets. They have to be padded upto 64 bytes in order to make them >> valid. Otherwise the destination, or any switch along the path, might >> throw them away. >> >> What could be happening is that the CSPW driver or hardware is padding >> the packet to 64 bytes. But that packet has a DSA header in it. The >> switch removes the header, recalculate the checksum and sends the >> packet. It is now either 4 or 8 bytes smaller, depending on what DSA >> header was used. It then becomes a runt packet. >> > Thank you for the clarification, this really helped me out. > >> Florian had to fix this problem recently. >> >> http://patchwork.ozlabs.org/patch/836534/ >> > But seems like this patch was never accepted, instead > brcm_tag_xmit_ll() does it if I am understanding it correctly. > similarly to this ksz_xmit() is taking care of padding.
net/dsa/tag_brcm.c ended up doing the padding because that was a more generic and central location: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/net/dsa/tag_brcm.c#n73 > >> You probably need something similar for the cpsw. >> > looking at the tag_ksz.c in xmit function this is taken care of I agree, this should be padding packets correctly, can you still instrument cpsw to make sure that what comes to its ndo_start_xmit() is ETH_ZLEN + tag_len or more? > > /* For Ingress (Host -> KSZ), 2 bytes are added before FCS. > * --------------------------------------------------------------------------- > * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes) > * --------------------------------------------------------------------------- > * tag0 : Prioritization (not used now) > * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x10=port5) > * > * For Egress (KSZ -> Host), 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, 0x06=port7) > */ > > #define KSZ_INGRESS_TAG_LEN 2 > #define KSZ_EGRESS_TAG_LEN 1 > > static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev) > { > struct dsa_slave_priv *p = netdev_priv(dev); > struct sk_buff *nskb; > int padlen; > u8 *tag; > > padlen = (skb->len >= ETH_ZLEN) ? 0 : ETH_ZLEN - skb->len; > > if (skb_tailroom(skb) >= padlen + KSZ_INGRESS_TAG_LEN) { > /* Let dsa_slave_xmit() free skb */ > if (__skb_put_padto(skb, skb->len + padlen, false)) > return NULL; > > nskb = skb; > } else { > nskb = alloc_skb(NET_IP_ALIGN + skb->len + > padlen + KSZ_INGRESS_TAG_LEN, GFP_ATOMIC); > if (!nskb) > return NULL; > skb_reserve(nskb, NET_IP_ALIGN); > > skb_reset_mac_header(nskb); > skb_set_network_header(nskb, > skb_network_header(skb) - skb->head); > skb_set_transport_header(nskb, > skb_transport_header(skb) - skb->head); > skb_copy_and_csum_dev(skb, skb_put(nskb, skb->len)); > > /* Let skb_put_padto() free nskb, and let dsa_slave_xmit() free > * skb > */ > if (skb_put_padto(nskb, nskb->len + padlen)) > return NULL; > > consume_skb(skb); > } > > tag = skb_put(nskb, KSZ_INGRESS_TAG_LEN); > tag[0] = 0; > tag[1] = 1 << p->dp->index; /* destination port */ > > return nskb; > } > > Cheers, > --Prabhakar Lad > -- Florian
