* [PATCH net] net: usb: lan78xx: fix possible skb leak
@ 2020-01-07 18:57 Eric Dumazet
2020-01-07 22:10 ` David Miller
2020-01-08 19:13 ` RENARD Pierre-Francois
0 siblings, 2 replies; 7+ messages in thread
From: Eric Dumazet @ 2020-01-07 18:57 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, RENARD Pierre-Francois,
Stefan Wahren, Woojung Huh, Microchip Linux Driver Support
If skb_linearize() fails, we need to free the skb.
TSO makes skb bigger, and this bug might be the reason
Raspberry Pi 3B+ users had to disable TSO.
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: RENARD Pierre-Francois <pfrenard@gmail.com>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Woojung Huh <woojung.huh@microchip.com>
Cc: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
---
drivers/net/usb/lan78xx.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index f940dc6485e56a7e8f905082ce920f5dd83232b0..fb4781080d6dec2af22f41c5e064350ea74130b3 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2724,11 +2724,6 @@ static int lan78xx_stop(struct net_device *net)
return 0;
}
-static int lan78xx_linearize(struct sk_buff *skb)
-{
- return skb_linearize(skb);
-}
-
static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net *dev,
struct sk_buff *skb, gfp_t flags)
{
@@ -2740,8 +2735,10 @@ static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net *dev,
return NULL;
}
- if (lan78xx_linearize(skb) < 0)
+ if (skb_linearize(skb)) {
+ dev_kfree_skb_any(skb);
return NULL;
+ }
tx_cmd_a = (u32)(skb->len & TX_CMD_A_LEN_MASK_) | TX_CMD_A_FCS_;
--
2.24.1.735.g03f4e72817-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: usb: lan78xx: fix possible skb leak
2020-01-07 18:57 [PATCH net] net: usb: lan78xx: fix possible skb leak Eric Dumazet
@ 2020-01-07 22:10 ` David Miller
2020-01-08 19:13 ` RENARD Pierre-Francois
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2020-01-07 22:10 UTC (permalink / raw)
To: edumazet
Cc: netdev, eric.dumazet, pfrenard, stefan.wahren, woojung.huh,
UNGLinuxDriver
From: Eric Dumazet <edumazet@google.com>
Date: Tue, 7 Jan 2020 10:57:01 -0800
> If skb_linearize() fails, we need to free the skb.
>
> TSO makes skb bigger, and this bug might be the reason
> Raspberry Pi 3B+ users had to disable TSO.
>
> Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: RENARD Pierre-Francois <pfrenard@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: usb: lan78xx: fix possible skb leak
2020-01-07 18:57 [PATCH net] net: usb: lan78xx: fix possible skb leak Eric Dumazet
2020-01-07 22:10 ` David Miller
@ 2020-01-08 19:13 ` RENARD Pierre-Francois
2020-01-08 20:25 ` Eric Dumazet
1 sibling, 1 reply; 7+ messages in thread
From: RENARD Pierre-Francois @ 2020-01-08 19:13 UTC (permalink / raw)
To: Eric Dumazet, David S . Miller
Cc: netdev, Eric Dumazet, Stefan Wahren, Woojung Huh,
Microchip Linux Driver Support
I tried with last rawhide kernel 5.5.0-0.rc5.git0.1.local.fc32.aarch64
I compiled it this night. (I check it includes the patch for lan78xx.c )
Both tests (scp and nfs ) are failing the same way as before.
Fox
On 1/7/20 7:57 PM, Eric Dumazet wrote:
> If skb_linearize() fails, we need to free the skb.
>
> TSO makes skb bigger, and this bug might be the reason
> Raspberry Pi 3B+ users had to disable TSO.
>
> Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: RENARD Pierre-Francois <pfrenard@gmail.com>
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Cc: Woojung Huh <woojung.huh@microchip.com>
> Cc: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> ---
> drivers/net/usb/lan78xx.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index f940dc6485e56a7e8f905082ce920f5dd83232b0..fb4781080d6dec2af22f41c5e064350ea74130b3 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -2724,11 +2724,6 @@ static int lan78xx_stop(struct net_device *net)
> return 0;
> }
>
> -static int lan78xx_linearize(struct sk_buff *skb)
> -{
> - return skb_linearize(skb);
> -}
> -
> static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net *dev,
> struct sk_buff *skb, gfp_t flags)
> {
> @@ -2740,8 +2735,10 @@ static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net *dev,
> return NULL;
> }
>
> - if (lan78xx_linearize(skb) < 0)
> + if (skb_linearize(skb)) {
> + dev_kfree_skb_any(skb);
> return NULL;
> + }
>
> tx_cmd_a = (u32)(skb->len & TX_CMD_A_LEN_MASK_) | TX_CMD_A_FCS_;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: usb: lan78xx: fix possible skb leak
2020-01-08 19:13 ` RENARD Pierre-Francois
@ 2020-01-08 20:25 ` Eric Dumazet
2020-01-08 23:19 ` RENARD Pierre-Francois
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2020-01-08 20:25 UTC (permalink / raw)
To: RENARD Pierre-Francois
Cc: David S . Miller, netdev, Eric Dumazet, Stefan Wahren,
Woojung Huh, Microchip Linux Driver Support
On Wed, Jan 8, 2020 at 11:13 AM RENARD Pierre-Francois
<pfrenard@gmail.com> wrote:
>
> I tried with last rawhide kernel 5.5.0-0.rc5.git0.1.local.fc32.aarch64
> I compiled it this night. (I check it includes the patch for lan78xx.c )
>
> Both tests (scp and nfs ) are failing the same way as before.
>
> Fox
>
Please report the output of " grep skb /proc/slabinfo"
before and after your test.
The symptoms (of retransmit being not attempted by TCP) match the fact
that skb(s) is(are) not freed by a driver (or some layer)
When TCP detects this (function skb_still_in_host_queue()), one SNMP
counter is incremented
nstat -a | grep TCPSpuriousRtxHostQueues
Thanks.
>
>
> On 1/7/20 7:57 PM, Eric Dumazet wrote:
> > If skb_linearize() fails, we need to free the skb.
> >
> > TSO makes skb bigger, and this bug might be the reason
> > Raspberry Pi 3B+ users had to disable TSO.
> >
> > Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Reported-by: RENARD Pierre-Francois <pfrenard@gmail.com>
> > Cc: Stefan Wahren <stefan.wahren@i2se.com>
> > Cc: Woojung Huh <woojung.huh@microchip.com>
> > Cc: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> > ---
> > drivers/net/usb/lan78xx.c | 9 +++------
> > 1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> > index f940dc6485e56a7e8f905082ce920f5dd83232b0..fb4781080d6dec2af22f41c5e064350ea74130b3 100644
> > --- a/drivers/net/usb/lan78xx.c
> > +++ b/drivers/net/usb/lan78xx.c
> > @@ -2724,11 +2724,6 @@ static int lan78xx_stop(struct net_device *net)
> > return 0;
> > }
> >
> > -static int lan78xx_linearize(struct sk_buff *skb)
> > -{
> > - return skb_linearize(skb);
> > -}
> > -
> > static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net *dev,
> > struct sk_buff *skb, gfp_t flags)
> > {
> > @@ -2740,8 +2735,10 @@ static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net *dev,
> > return NULL;
> > }
> >
> > - if (lan78xx_linearize(skb) < 0)
> > + if (skb_linearize(skb)) {
> > + dev_kfree_skb_any(skb);
> > return NULL;
> > + }
> >
> > tx_cmd_a = (u32)(skb->len & TX_CMD_A_LEN_MASK_) | TX_CMD_A_FCS_;
> >
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: usb: lan78xx: fix possible skb leak
2020-01-08 20:25 ` Eric Dumazet
@ 2020-01-08 23:19 ` RENARD Pierre-Francois
2020-01-10 19:07 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: RENARD Pierre-Francois @ 2020-01-08 23:19 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, Stefan Wahren,
Woojung Huh, Microchip Linux Driver Support
OK
Before scp command ( and after a fresh reboot)
---------------------------------
skbuff_ext_cache 378 378 192 21 1 : tunables 0 0 0
: slabdata 18 18 0
skbuff_fclone_cache 112 112 512 16 2 : tunables 0 0
0 : slabdata 7 7 0
skbuff_head_cache 1936 2160 256 16 1 : tunables 0 0 0
: slabdata 135 135 0
---------------------------------
---------------------------------
After the hang of scp (hanged at 203 MB)
---------------------------------
skbuff_ext_cache 693 693 192 21 1 : tunables 0 0 0
: slabdata 33 33 0
skbuff_fclone_cache 128 128 512 16 2 : tunables 0 0
0 : slabdata 8 8 0
skbuff_head_cache 2032 2176 256 16 1 : tunables 0 0 0
: slabdata 136 136 0
---------------------------------
TcpExtTCPSpuriousRtxHostQueues 120 0.0
---------------------------------
After CTRL-C of scp
---------------------------------
skbuff_ext_cache 693 693 192 21 1 : tunables 0 0 0
: slabdata 33 33 0
skbuff_fclone_cache 128 128 512 16 2 : tunables 0 0
0 : slabdata 8 8 0
skbuff_head_cache 2112 2336 256 16 1 : tunables 0 0 0
: slabdata 146 146 0
---------------------------------
TcpExtTCPSpuriousRtxHostQueues 124 0.0
---------------------------------
After the hang of a second attempt (hanged at 1214 MB)
---------------------------------
skbuff_ext_cache 735 735 192 21 1 : tunables 0 0 0
: slabdata 35 35 0
skbuff_fclone_cache 160 160 512 16 2 : tunables 0 0
0 : slabdata 10 10 0
skbuff_head_cache 2096 2240 256 16 1 : tunables 0 0 0
: slabdata 140 140 0
---------------------------------
TcpExtTCPSpuriousRtxHostQueues 248 0.0
---------------------------------
After a third attempt (hanged at 55 MB)
---------------------------------
skbuff_ext_cache 735 735 192 21 1 : tunables 0 0 0
: slabdata 35 35 0
skbuff_fclone_cache 176 176 512 16 2 : tunables 0 0
0 : slabdata 11 11 0
skbuff_head_cache 2000 2144 256 16 1 : tunables 0 0 0
: slabdata 134 134 0
---------------------------------
TcpExtTCPSpuriousRtxHostQueues 365 0.0
---------------------------------
On 1/8/20 9:25 PM, Eric Dumazet wrote:
> On Wed, Jan 8, 2020 at 11:13 AM RENARD Pierre-Francois
> <pfrenard@gmail.com> wrote:
>> I tried with last rawhide kernel 5.5.0-0.rc5.git0.1.local.fc32.aarch64
>> I compiled it this night. (I check it includes the patch for lan78xx.c )
>>
>> Both tests (scp and nfs ) are failing the same way as before.
>>
>> Fox
>>
> Please report the output of " grep skb /proc/slabinfo"
>
> before and after your test.
>
> The symptoms (of retransmit being not attempted by TCP) match the fact
> that skb(s) is(are) not freed by a driver (or some layer)
>
> When TCP detects this (function skb_still_in_host_queue()), one SNMP
> counter is incremented
>
> nstat -a | grep TCPSpuriousRtxHostQueues
>
> Thanks.
>
>>
>> On 1/7/20 7:57 PM, Eric Dumazet wrote:
>>> If skb_linearize() fails, we need to free the skb.
>>>
>>> TSO makes skb bigger, and this bug might be the reason
>>> Raspberry Pi 3B+ users had to disable TSO.
>>>
>>> Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
>>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>>> Reported-by: RENARD Pierre-Francois <pfrenard@gmail.com>
>>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
>>> Cc: Woojung Huh <woojung.huh@microchip.com>
>>> Cc: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
>>> ---
>>> drivers/net/usb/lan78xx.c | 9 +++------
>>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
>>> index f940dc6485e56a7e8f905082ce920f5dd83232b0..fb4781080d6dec2af22f41c5e064350ea74130b3 100644
>>> --- a/drivers/net/usb/lan78xx.c
>>> +++ b/drivers/net/usb/lan78xx.c
>>> @@ -2724,11 +2724,6 @@ static int lan78xx_stop(struct net_device *net)
>>> return 0;
>>> }
>>>
>>> -static int lan78xx_linearize(struct sk_buff *skb)
>>> -{
>>> - return skb_linearize(skb);
>>> -}
>>> -
>>> static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net *dev,
>>> struct sk_buff *skb, gfp_t flags)
>>> {
>>> @@ -2740,8 +2735,10 @@ static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net *dev,
>>> return NULL;
>>> }
>>>
>>> - if (lan78xx_linearize(skb) < 0)
>>> + if (skb_linearize(skb)) {
>>> + dev_kfree_skb_any(skb);
>>> return NULL;
>>> + }
>>>
>>> tx_cmd_a = (u32)(skb->len & TX_CMD_A_LEN_MASK_) | TX_CMD_A_FCS_;
>>>
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: usb: lan78xx: fix possible skb leak
2020-01-08 23:19 ` RENARD Pierre-Francois
@ 2020-01-10 19:07 ` Eric Dumazet
2020-01-11 8:20 ` RENARD Pierre-Francois
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2020-01-10 19:07 UTC (permalink / raw)
To: RENARD Pierre-Francois, Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, Stefan Wahren,
Woojung Huh, Microchip Linux Driver Support
On 1/8/20 3:19 PM, RENARD Pierre-Francois wrote:
> OK
>
> Before scp command ( and after a fresh reboot)
> ---------------------------------
> skbuff_ext_cache 378 378 192 21 1 : tunables 0 0 0 : slabdata 18 18 0
> skbuff_fclone_cache 112 112 512 16 2 : tunables 0 0 0 : slabdata 7 7 0
> skbuff_head_cache 1936 2160 256 16 1 : tunables 0 0 0 : slabdata 135 135 0
> ---------------------------------
> ---------------------------------
>
> After the hang of scp (hanged at 203 MB)
> ---------------------------------
> skbuff_ext_cache 693 693 192 21 1 : tunables 0 0 0 : slabdata 33 33 0
> skbuff_fclone_cache 128 128 512 16 2 : tunables 0 0 0 : slabdata 8 8 0
> skbuff_head_cache 2032 2176 256 16 1 : tunables 0 0 0 : slabdata 136 136 0
> ---------------------------------
> TcpExtTCPSpuriousRtxHostQueues 120 0.0
> ---------------------------------
>
> After CTRL-C of scp
> ---------------------------------
> skbuff_ext_cache 693 693 192 21 1 : tunables 0 0 0 : slabdata 33 33 0
> skbuff_fclone_cache 128 128 512 16 2 : tunables 0 0 0 : slabdata 8 8 0
> skbuff_head_cache 2112 2336 256 16 1 : tunables 0 0 0 : slabdata 146 146 0
> ---------------------------------
> TcpExtTCPSpuriousRtxHostQueues 124 0.0
> ---------------------------------
>
>
>
> After the hang of a second attempt (hanged at 1214 MB)
> ---------------------------------
> skbuff_ext_cache 735 735 192 21 1 : tunables 0 0 0 : slabdata 35 35 0
> skbuff_fclone_cache 160 160 512 16 2 : tunables 0 0 0 : slabdata 10 10 0
> skbuff_head_cache 2096 2240 256 16 1 : tunables 0 0 0 : slabdata 140 140 0
> ---------------------------------
> TcpExtTCPSpuriousRtxHostQueues 248 0.0
> ---------------------------------
>
>
> After a third attempt (hanged at 55 MB)
> ---------------------------------
> skbuff_ext_cache 735 735 192 21 1 : tunables 0 0 0 : slabdata 35 35 0
> skbuff_fclone_cache 176 176 512 16 2 : tunables 0 0 0 : slabdata 11 11 0
> skbuff_head_cache 2000 2144 256 16 1 : tunables 0 0 0 : slabdata 134 134 0
> ---------------------------------
> TcpExtTCPSpuriousRtxHostQueues 365 0.0
> ---------------------------------
>
>
Thanks for testing.
This seems to suggest there is another bug in the driver, leading to some skb being never freed.
Since the driver seems to limit aggregation to 9000 bytes (MAX_SINGLE_PACKET_SIZE)
I wonder if gso skbs should also be limited to the same value.
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index d3239b49c3bb2803c874a2e8af332bcf03848e18..65dea9a94b90e27889c8f44294560ffeabda2eb9 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -3787,6 +3787,8 @@ static int lan78xx_probe(struct usb_interface *intf,
if (ret < 0)
goto out4;
+ netif_set_gso_max_size(netdev, MAX_SINGLE_PACKET_SIZE - MAX_HEADER);
+
ret = register_netdev(netdev);
if (ret != 0) {
netif_err(dev, probe, netdev, "couldn't register the device\n");
>
>
>
>
>
>
> On 1/8/20 9:25 PM, Eric Dumazet wrote:
>> On Wed, Jan 8, 2020 at 11:13 AM RENARD Pierre-Francois
>> <pfrenard@gmail.com> wrote:
>>> I tried with last rawhide kernel 5.5.0-0.rc5.git0.1.local.fc32.aarch64
>>> I compiled it this night. (I check it includes the patch for lan78xx.c )
>>>
>>> Both tests (scp and nfs ) are failing the same way as before.
>>>
>>> Fox
>>>
>> Please report the output of " grep skb /proc/slabinfo"
>>
>> before and after your test.
>>
>> The symptoms (of retransmit being not attempted by TCP) match the fact
>> that skb(s) is(are) not freed by a driver (or some layer)
>>
>> When TCP detects this (function skb_still_in_host_queue()), one SNMP
>> counter is incremented
>>
>> nstat -a | grep TCPSpuriousRtxHostQueues
>>
>> Thanks.
>>
>>>
>>> On 1/7/20 7:57 PM, Eric Dumazet wrote:
>>>> If skb_linearize() fails, we need to free the skb.
>>>>
>>>> TSO makes skb bigger, and this bug might be the reason
>>>> Raspberry Pi 3B+ users had to disable TSO.
>>>>
>>>> Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
>>>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>>>> Reported-by: RENARD Pierre-Francois <pfrenard@gmail.com>
>>>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
>>>> Cc: Woojung Huh <woojung.huh@microchip.com>
>>>> Cc: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
>>>> ---
>>>> drivers/net/usb/lan78xx.c | 9 +++------
>>>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
>>>> index f940dc6485e56a7e8f905082ce920f5dd83232b0..fb4781080d6dec2af22f41c5e064350ea74130b3 100644
>>>> --- a/drivers/net/usb/lan78xx.c
>>>> +++ b/drivers/net/usb/lan78xx.c
>>>> @@ -2724,11 +2724,6 @@ static int lan78xx_stop(struct net_device *net)
>>>> return 0;
>>>> }
>>>>
>>>> -static int lan78xx_linearize(struct sk_buff *skb)
>>>> -{
>>>> - return skb_linearize(skb);
>>>> -}
>>>> -
>>>> static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net *dev,
>>>> struct sk_buff *skb, gfp_t flags)
>>>> {
>>>> @@ -2740,8 +2735,10 @@ static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net *dev,
>>>> return NULL;
>>>> }
>>>>
>>>> - if (lan78xx_linearize(skb) < 0)
>>>> + if (skb_linearize(skb)) {
>>>> + dev_kfree_skb_any(skb);
>>>> return NULL;
>>>> + }
>>>>
>>>> tx_cmd_a = (u32)(skb->len & TX_CMD_A_LEN_MASK_) | TX_CMD_A_FCS_;
>>>>
>>>
>
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: usb: lan78xx: fix possible skb leak
2020-01-10 19:07 ` Eric Dumazet
@ 2020-01-11 8:20 ` RENARD Pierre-Francois
0 siblings, 0 replies; 7+ messages in thread
From: RENARD Pierre-Francois @ 2020-01-11 8:20 UTC (permalink / raw)
To: Eric Dumazet, Eric Dumazet
Cc: David S . Miller, netdev, Stefan Wahren, Woojung Huh,
Microchip Linux Driver Support
Hello,
I applied patch and I am NOT able to reproduce the issue with both NFS
or SCP !
Congratulations !
Fox
On 1/10/20 8:07 PM, Eric Dumazet wrote:
>
> On 1/8/20 3:19 PM, RENARD Pierre-Francois wrote:
>> OK
>>
>> Before scp command ( and after a fresh reboot)
>> ---------------------------------
>> skbuff_ext_cache 378 378 192 21 1 : tunables 0 0 0 : slabdata 18 18 0
>> skbuff_fclone_cache 112 112 512 16 2 : tunables 0 0 0 : slabdata 7 7 0
>> skbuff_head_cache 1936 2160 256 16 1 : tunables 0 0 0 : slabdata 135 135 0
>> ---------------------------------
>> ---------------------------------
>>
>> After the hang of scp (hanged at 203 MB)
>> ---------------------------------
>> skbuff_ext_cache 693 693 192 21 1 : tunables 0 0 0 : slabdata 33 33 0
>> skbuff_fclone_cache 128 128 512 16 2 : tunables 0 0 0 : slabdata 8 8 0
>> skbuff_head_cache 2032 2176 256 16 1 : tunables 0 0 0 : slabdata 136 136 0
>> ---------------------------------
>> TcpExtTCPSpuriousRtxHostQueues 120 0.0
>> ---------------------------------
>>
>> After CTRL-C of scp
>> ---------------------------------
>> skbuff_ext_cache 693 693 192 21 1 : tunables 0 0 0 : slabdata 33 33 0
>> skbuff_fclone_cache 128 128 512 16 2 : tunables 0 0 0 : slabdata 8 8 0
>> skbuff_head_cache 2112 2336 256 16 1 : tunables 0 0 0 : slabdata 146 146 0
>> ---------------------------------
>> TcpExtTCPSpuriousRtxHostQueues 124 0.0
>> ---------------------------------
>>
>>
>>
>> After the hang of a second attempt (hanged at 1214 MB)
>> ---------------------------------
>> skbuff_ext_cache 735 735 192 21 1 : tunables 0 0 0 : slabdata 35 35 0
>> skbuff_fclone_cache 160 160 512 16 2 : tunables 0 0 0 : slabdata 10 10 0
>> skbuff_head_cache 2096 2240 256 16 1 : tunables 0 0 0 : slabdata 140 140 0
>> ---------------------------------
>> TcpExtTCPSpuriousRtxHostQueues 248 0.0
>> ---------------------------------
>>
>>
>> After a third attempt (hanged at 55 MB)
>> ---------------------------------
>> skbuff_ext_cache 735 735 192 21 1 : tunables 0 0 0 : slabdata 35 35 0
>> skbuff_fclone_cache 176 176 512 16 2 : tunables 0 0 0 : slabdata 11 11 0
>> skbuff_head_cache 2000 2144 256 16 1 : tunables 0 0 0 : slabdata 134 134 0
>> ---------------------------------
>> TcpExtTCPSpuriousRtxHostQueues 365 0.0
>> ---------------------------------
>>
>>
>
> Thanks for testing.
>
> This seems to suggest there is another bug in the driver, leading to some skb being never freed.
>
> Since the driver seems to limit aggregation to 9000 bytes (MAX_SINGLE_PACKET_SIZE)
> I wonder if gso skbs should also be limited to the same value.
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index d3239b49c3bb2803c874a2e8af332bcf03848e18..65dea9a94b90e27889c8f44294560ffeabda2eb9 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -3787,6 +3787,8 @@ static int lan78xx_probe(struct usb_interface *intf,
> if (ret < 0)
> goto out4;
>
> + netif_set_gso_max_size(netdev, MAX_SINGLE_PACKET_SIZE - MAX_HEADER);
> +
> ret = register_netdev(netdev);
> if (ret != 0) {
> netif_err(dev, probe, netdev, "couldn't register the device\n");
>
>
>
>>
>>
>>
>>
>>
>> On 1/8/20 9:25 PM, Eric Dumazet wrote:
>>> On Wed, Jan 8, 2020 at 11:13 AM RENARD Pierre-Francois
>>> <pfrenard@gmail.com> wrote:
>>>> I tried with last rawhide kernel 5.5.0-0.rc5.git0.1.local.fc32.aarch64
>>>> I compiled it this night. (I check it includes the patch for lan78xx.c )
>>>>
>>>> Both tests (scp and nfs ) are failing the same way as before.
>>>>
>>>> Fox
>>>>
>>> Please report the output of " grep skb /proc/slabinfo"
>>>
>>> before and after your test.
>>>
>>> The symptoms (of retransmit being not attempted by TCP) match the fact
>>> that skb(s) is(are) not freed by a driver (or some layer)
>>>
>>> When TCP detects this (function skb_still_in_host_queue()), one SNMP
>>> counter is incremented
>>>
>>> nstat -a | grep TCPSpuriousRtxHostQueues
>>>
>>> Thanks.
>>>
>>>> On 1/7/20 7:57 PM, Eric Dumazet wrote:
>>>>> If skb_linearize() fails, we need to free the skb.
>>>>>
>>>>> TSO makes skb bigger, and this bug might be the reason
>>>>> Raspberry Pi 3B+ users had to disable TSO.
>>>>>
>>>>> Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
>>>>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>>>>> Reported-by: RENARD Pierre-Francois <pfrenard@gmail.com>
>>>>> Cc: Stefan Wahren <stefan.wahren@i2se.com>
>>>>> Cc: Woojung Huh <woojung.huh@microchip.com>
>>>>> Cc: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
>>>>> ---
>>>>> drivers/net/usb/lan78xx.c | 9 +++------
>>>>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
>>>>> index f940dc6485e56a7e8f905082ce920f5dd83232b0..fb4781080d6dec2af22f41c5e064350ea74130b3 100644
>>>>> --- a/drivers/net/usb/lan78xx.c
>>>>> +++ b/drivers/net/usb/lan78xx.c
>>>>> @@ -2724,11 +2724,6 @@ static int lan78xx_stop(struct net_device *net)
>>>>> return 0;
>>>>> }
>>>>>
>>>>> -static int lan78xx_linearize(struct sk_buff *skb)
>>>>> -{
>>>>> - return skb_linearize(skb);
>>>>> -}
>>>>> -
>>>>> static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net *dev,
>>>>> struct sk_buff *skb, gfp_t flags)
>>>>> {
>>>>> @@ -2740,8 +2735,10 @@ static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net *dev,
>>>>> return NULL;
>>>>> }
>>>>>
>>>>> - if (lan78xx_linearize(skb) < 0)
>>>>> + if (skb_linearize(skb)) {
>>>>> + dev_kfree_skb_any(skb);
>>>>> return NULL;
>>>>> + }
>>>>>
>>>>> tx_cmd_a = (u32)(skb->len & TX_CMD_A_LEN_MASK_) | TX_CMD_A_FCS_;
>>>>>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-01-11 8:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-07 18:57 [PATCH net] net: usb: lan78xx: fix possible skb leak Eric Dumazet
2020-01-07 22:10 ` David Miller
2020-01-08 19:13 ` RENARD Pierre-Francois
2020-01-08 20:25 ` Eric Dumazet
2020-01-08 23:19 ` RENARD Pierre-Francois
2020-01-10 19:07 ` Eric Dumazet
2020-01-11 8:20 ` RENARD Pierre-Francois
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.