netdev - RE: [PATCH net-next v2 1/3] net: bgmac: Pad packets to a minimum size

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 10 Nov 2017 11:10:43 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Florian Fainelli' <f.fainelli@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "vivien.didelot@...oirfairelinux.com" 
        <vivien.didelot@...oirfairelinux.com>
Subject: RE: [PATCH net-next v2 1/3] net: bgmac: Pad packets to a minimum
 size

From: Florian Fainelli
> Sent: 09 November 2017 22:35
> 
> In preparation for enabling Broadcom tags with b53, pad packets to a
> minimum size of 64 bytes (sans FCS) in order for the Broadcom switch to
> accept ingressing frames. Without this, we would typically be able to
> DHCP, but not resolve with ARP because packets are too small and get
> rejected by the switch.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
> ---
>  drivers/net/ethernet/broadcom/bgmac.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> index 48d672b204a4..5130fc96940d 100644
> --- a/drivers/net/ethernet/broadcom/bgmac.c
> +++ b/drivers/net/ethernet/broadcom/bgmac.c
> @@ -127,6 +127,8 @@ bgmac_dma_tx_add_buf(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
>  	dma_desc->ctl1 = cpu_to_le32(ctl1);
>  }
> 
> +#define ENET_BRCM_TAG_LEN	4
> +
>  static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
>  				    struct bgmac_dma_ring *ring,
>  				    struct sk_buff *skb)
> @@ -139,6 +141,16 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
>  	u32 flags;
>  	int i;
> 
> +	/* The Ethernet switch we are interfaced with needs packets to be at
> +	 * least 64 bytes (including FCS) otherwise they will be discarded when
> +	 * they enter the switch port logic. When Broadcom tags are enabled, we
> +	 * need to make sure that packets are at least 68 bytes
> +	 * (including FCS and tag) because the length verification is done after
> +	 * the Broadcom tag is stripped off the ingress packet.
> +	 */

I think that that would be better as:
	/* Ethernet packets are padded to 64 bytes (including FCS).
	 * If 'Broadcom tags' are enabled they must still be 64 bytes
	 * long after the 4 byte tag is removed.
	 * Since the hardware doesn't do it, we must pad them before
	 * transmit.
	 */

Which seems to be to be a bug in the chip.

> +	if (skb_put_padto(skb, ETH_ZLEN + ENET_BRCM_TAG_LEN))
> +		goto err_stats;
> +

But you shouldn't overpad packets that don't have the extra tag.

	David