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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAGVrzcYKVm8Pps_DHrXmZ6wd6d6A3k7FjkEn9sVDo1PwA1fZcw@mail.gmail.com>
Date:	Tue, 13 May 2014 17:36:58 -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-13 17:18 GMT-07:00 Florian Fainelli <f.fainelli@...il.com>:
> 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!

In fact, no, despite the Ethernet MAC appending the CRC, the minimum
packet length from the DMA perspective really needs to be 64, as the
hardware inserted CRC is not accounted for to generate the End of
Packet signal. That just translates into ETH_ZLEN + ETH_FCS_LEN
anyway.

>
>> 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



-- 
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