netdev - Re: [PATCH net-next v4 2/2] net: systemport: pad packets to a minimum of 64 bytes

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:	Tue, 13 May 2014 17:18:51 -0700
From:	Florian Fainelli <f.fainelli@...il.com>
To:	David Laight <David.Laight@...lab.com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"davem@...emloft.net" <davem@...emloft.net>
Subject: Re: [PATCH net-next v4 2/2] net: systemport: pad packets to a minimum
 of 64 bytes

2014-05-12 5:45 GMT-07:00 David Laight <David.Laight@...lab.com>:
> From: Florian Fainelli
>> The switch fabric which is used behind the Broadcom SYSTEMPORT Ethernet
>> controller will discard any incoming packet that is not 64 bytes or
>> more. Since the UniMAC hardware transmits up to the specified size, just
>> make sure that we properly pad the end of the packet with zeroes using
>> skb_padto() and set a minimum packet length of 64 bytes.
>
> Surely the above should just be:
>     "Pad transmit packets to the minimum length (64 bytes)."
> All ethernet packets should be padded to 64 bytes (including the CRC).

Sure, I can definitively simplify the commit message.

>
> ...
>> @@ -848,10 +849,22 @@ static netdev_tx_t bcm_sysport_xmit(struct sk_buff *skb,
>>               }
>>       }
>>
>> -     mapping = dma_map_single(kdev, skb->data, skb->len, DMA_TO_DEVICE);
>> +     /* The Ethernet switch we are interfaced with needs packets to be at
>> +      * least 64 bytes otherwise they will be discarded when they enter
>> +      * the switch port logic. The UniMAC hardware transmits up to the
>> +      * specified length.
>> +      */
>> +     if (skb_padto(skb, 64)) {
>
> Shouldn't that be 60?

It should, absolutely, and that is ETH_ZLEN. Thanks!

> I presume there is an appropriate constant defined for it as well.
>
>> +             ret = NETDEV_TX_OK;
>> +             goto out;
>> +     }
>> +
>> +     skb_len = skb->len < 64 ? 64 : skb->len;
>> +
>> +     mapping = dma_map_single(kdev, skb->data, skb_len, DMA_TO_DEVICE);
>>       if (dma_mapping_error(kdev, mapping)) {
>>               netif_err(priv, tx_err, dev, "DMA map failed at %p (len=%d)\n",
>> -                             skb->data, skb->len);
>> +                             skb->data, skb_len);
>>               ret = NETDEV_TX_OK;
>>               goto out;
>>       }
> ...
>
>         david
>
>



-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html