diff mbox series

[U-Boot,1/2] net: designware: fix tx packet length

Message ID 20181117092442.15638-1-simon.k.r.goldschmidt@gmail.com
State Accepted
Commit ae8ac8d423675904fdbf1510ad71e37d71db0568
Delegated to: Joe Hershberger
Headers show
Series [U-Boot,1/2] net: designware: fix tx packet length | expand

Commit Message

Simon Goldschmidt Nov. 17, 2018, 9:24 a.m. UTC
The designware driver has a bug in setting the tx length into the dma
descriptor: it always or's the length into the descriptor without
zeroing out the length mask before.

This results in occasional packets being transmitted with a length
greater than they should be (trailer). Due to the nature of Ethernet
allowing such a trailer, most packets seem to be parsed fine by remote
hosts, which is probably why this hasn't been noticed.

Fix this by correctly clearing the size mask before setting the new
length.

Tested on socfpga gen5.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

 drivers/net/designware.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Joe Hershberger Nov. 17, 2018, 4:02 p.m. UTC | #1
On Sat, Nov 17, 2018 at 3:25 AM Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> The designware driver has a bug in setting the tx length into the dma
> descriptor: it always or's the length into the descriptor without
> zeroing out the length mask before.
>
> This results in occasional packets being transmitted with a length
> greater than they should be (trailer). Due to the nature of Ethernet
> allowing such a trailer, most packets seem to be parsed fine by remote
> hosts, which is probably why this hasn't been noticed.
>
> Fix this by correctly clearing the size mask before setting the new
> length.
>
> Tested on socfpga gen5.
>
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>
Simon Goldschmidt Dec. 9, 2018, 8:50 p.m. UTC | #2
Am 17.11.2018 um 17:02 schrieb Joe Hershberger:
> On Sat, Nov 17, 2018 at 3:25 AM Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
>>
>> The designware driver has a bug in setting the tx length into the dma
>> descriptor: it always or's the length into the descriptor without
>> zeroing out the length mask before.
>>
>> This results in occasional packets being transmitted with a length
>> greater than they should be (trailer). Due to the nature of Ethernet
>> allowing such a trailer, most packets seem to be parsed fine by remote
>> hosts, which is probably why this hasn't been noticed.
>>
>> Fix this by correctly clearing the size mask before setting the new
>> length.
>>
>> Tested on socfpga gen5.
>>
>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> 
> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
> 

Marek, this and 2/2 is a assigned to you in patchwork, will you push 
them or is it Joe's code as u-boot-net maintainer?

Thanks,
Simon
Philipp Tomsich Dec. 9, 2018, 9:36 p.m. UTC | #3
> On 17.11.2018, at 10:24, Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote:
> 
> The designware driver has a bug in setting the tx length into the dma
> descriptor: it always or's the length into the descriptor without
> zeroing out the length mask before.
> 
> This results in occasional packets being transmitted with a length
> greater than they should be (trailer). Due to the nature of Ethernet
> allowing such a trailer, most packets seem to be parsed fine by remote
> hosts, which is probably why this hasn't been noticed.
> 
> Fix this by correctly clearing the size mask before setting the new
> length.
> 
> Tested on socfpga gen5.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Marek Vasut Dec. 10, 2018, 4:08 a.m. UTC | #4
On 12/09/2018 09:50 PM, Simon Goldschmidt wrote:
> Am 17.11.2018 um 17:02 schrieb Joe Hershberger:
>> On Sat, Nov 17, 2018 at 3:25 AM Simon Goldschmidt
>> <simon.k.r.goldschmidt@gmail.com> wrote:
>>>
>>> The designware driver has a bug in setting the tx length into the dma
>>> descriptor: it always or's the length into the descriptor without
>>> zeroing out the length mask before.
>>>
>>> This results in occasional packets being transmitted with a length
>>> greater than they should be (trailer). Due to the nature of Ethernet
>>> allowing such a trailer, most packets seem to be parsed fine by remote
>>> hosts, which is probably why this hasn't been noticed.
>>>
>>> Fix this by correctly clearing the size mask before setting the new
>>> length.
>>>
>>> Tested on socfpga gen5.
>>>
>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>
>> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
>>
> 
> Marek, this and 2/2 is a assigned to you in patchwork, will you push
> them or is it Joe's code as u-boot-net maintainer?

This is net , so Joe should pick it .
Simon Goldschmidt Jan. 14, 2019, 3:35 p.m. UTC | #5
Joe,

Am 10.12.2018 um 05:08 schrieb Marek Vasut:
> On 12/09/2018 09:50 PM, Simon Goldschmidt wrote:
>> Am 17.11.2018 um 17:02 schrieb Joe Hershberger:
>>> On Sat, Nov 17, 2018 at 3:25 AM Simon Goldschmidt
>>> <simon.k.r.goldschmidt@gmail.com> wrote:
>>>>
>>>> The designware driver has a bug in setting the tx length into the dma
>>>> descriptor: it always or's the length into the descriptor without
>>>> zeroing out the length mask before.
>>>>
>>>> This results in occasional packets being transmitted with a length
>>>> greater than they should be (trailer). Due to the nature of Ethernet
>>>> allowing such a trailer, most packets seem to be parsed fine by remote
>>>> hosts, which is probably why this hasn't been noticed.
>>>>
>>>> Fix this by correctly clearing the size mask before setting the new
>>>> length.
>>>>
>>>> Tested on socfpga gen5.
>>>>
>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>>
>>> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
>>>
>>
>> Marek, this and 2/2 is a assigned to you in patchwork, will you push
>> them or is it Joe's code as u-boot-net maintainer?
> 
> This is net , so Joe should pick it .

I've assigned this one and 2/2 to you in patchwork, is that OK? Could 
you pick these after the release?

Thanks,
Simon
Joe Hershberger Jan. 24, 2019, 5:37 p.m. UTC | #6
Hi Simon,

https://patchwork.ozlabs.org/patch/999267/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe
diff mbox series

Patch

diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index 19db0a8114..688cf9fef2 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -389,15 +389,17 @@  static int _dw_eth_send(struct dw_eth_dev *priv, void *packet, int length)
 
 #if defined(CONFIG_DW_ALTDESCRIPTOR)
 	desc_p->txrx_status |= DESC_TXSTS_TXFIRST | DESC_TXSTS_TXLAST;
-	desc_p->dmamac_cntl |= (length << DESC_TXCTRL_SIZE1SHFT) &
-			       DESC_TXCTRL_SIZE1MASK;
+	desc_p->dmamac_cntl = (desc_p->dmamac_cntl & ~DESC_TXCTRL_SIZE1MASK) |
+			      ((length << DESC_TXCTRL_SIZE1SHFT) &
+			      DESC_TXCTRL_SIZE1MASK);
 
 	desc_p->txrx_status &= ~(DESC_TXSTS_MSK);
 	desc_p->txrx_status |= DESC_TXSTS_OWNBYDMA;
 #else
-	desc_p->dmamac_cntl |= ((length << DESC_TXCTRL_SIZE1SHFT) &
-			       DESC_TXCTRL_SIZE1MASK) | DESC_TXCTRL_TXLAST |
-			       DESC_TXCTRL_TXFIRST;
+	desc_p->dmamac_cntl = (desc_p->dmamac_cntl & ~DESC_TXCTRL_SIZE1MASK) |
+			      ((length << DESC_TXCTRL_SIZE1SHFT) &
+			      DESC_TXCTRL_SIZE1MASK) | DESC_TXCTRL_TXLAST |
+			      DESC_TXCTRL_TXFIRST;
 
 	desc_p->txrx_status = DESC_TXSTS_OWNBYDMA;
 #endif