* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Willy Tarreau @ 2013-11-20 0:52 UTC (permalink / raw)
To: Eric Dumazet
Cc: Thomas Petazzoni, Florian Fainelli, simon.guinot, netdev,
Arnaud Ebalard, edumazet, Cong Wang, linux-arm-kernel
In-Reply-To: <1384908228.8604.136.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Nov 19, 2013 at 04:43:48PM -0800, Eric Dumazet wrote:
> On Wed, 2013-11-20 at 01:35 +0100, Willy Tarreau wrote:
> > On Tue, Nov 19, 2013 at 04:08:49PM -0800, Eric Dumazet wrote:
> > > On Wed, 2013-11-20 at 00:53 +0100, Arnaud Ebalard wrote:
> > >
> > > > Anyway, I think if the thread keeps going on improving mvneta, I'll do
> > > > all additional tests from RAM and will stop polluting netdev w/ possible
> > > > sata/disk/fs issues.
> > >
> > > ;)
> > >
> > > Alternative would be to use netperf or iperf to not use disk at all
> > > and focus on TCP/network issues only.
> >
> > Yes, that's for the same reason that I continue to use inject/httpterm
> > for such purposes :
> > - httpterm uses tee()+splice() to send pre-built pages without copying ;
> > - inject uses recv(MSG_TRUNC) to ack everything without copying.
> >
> > Both of them are really interesting to test the hardware's capabilities
> > and to push components in the middle to their limits without causing too
> > much burden to the end points.
> >
> > I don't know if either netperf or iperf can make use of this now, and
> > I've been used to my tools, but I should take a look again.
>
> netperf -t TCP_SENDFILE does the zero copy at sender.
>
> And more generally -V option does copy avoidance
>
> (Use splice(sockfd -> nullfd) at receiver if I remember well)
OK thanks for the info.
> Anyway, we should do the normal copy, because it might demonstrate
> scheduling problems.
Yes, especially in this case, though I got the issue with GSO only,
so it might vary as well.
> If you want to test raw speed, you could use pktgen ;)
Except I'm mostly focused on HTTP as you know. And for generating higher
packet rates than pktgen, I have an absolutely ugly patch that I'm a
bit ashamed of for mvneta to multiply the number of emitted descriptors
for a given skb by skb->sk->sk_mark (which I set using setsockopt), this
allows me to generate up to 1.488 Mpps on a USB-powered system, not that
bad in my opinion :-)
Willy
^ permalink raw reply
* Re: [PATCH net RESEND] bridge: flush br's address entry in fdb when remove the bridge dev
From: Ding Tianhong @ 2013-11-20 1:03 UTC (permalink / raw)
To: Toshiaki Makita; +Cc: Stephen Hemminger, Netdev, bridge, David S. Miller
In-Reply-To: <1384876004.1759.7.camel@localhost.localdomain>
On 2013/11/19 23:46, Toshiaki Makita wrote:
> On Tue, 2013-11-19 at 10:57 +0800, Ding Tianhong wrote:
>> When the following commands are executed:
>>
>> brctl addbr br0
>> ifconfig br0 hw ether <addr>
>> rmmod bridge
>>
>> The calltrace will occur:
>>
>> [ 563.312114] device eth1 left promiscuous mode
>> [ 563.312188] br0: port 1(eth1) entered disabled state
>> [ 563.468190] kmem_cache_destroy bridge_fdb_cache: Slab cache still has objects
>> [ 563.468197] CPU: 6 PID: 6982 Comm: rmmod Tainted: G O 3.12.0-0.7-default+ #9
>> [ 563.468199] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
>> [ 563.468200] 0000000000000880 ffff88010f111e98 ffffffff814d1c92 ffff88010f111eb8
>> [ 563.468204] ffffffff81148efd ffff88010f111eb8 0000000000000000 ffff88010f111ec8
>> [ 563.468206] ffffffffa062a270 ffff88010f111ed8 ffffffffa063ac76 ffff88010f111f78
>> [ 563.468209] Call Trace:
>> [ 563.468218] [<ffffffff814d1c92>] dump_stack+0x6a/0x78
>> [ 563.468234] [<ffffffff81148efd>] kmem_cache_destroy+0xfd/0x100
>> [ 563.468242] [<ffffffffa062a270>] br_fdb_fini+0x10/0x20 [bridge]
>> [ 563.468247] [<ffffffffa063ac76>] br_deinit+0x4e/0x50 [bridge]
>> [ 563.468254] [<ffffffff810c7dc9>] SyS_delete_module+0x199/0x2b0
>> [ 563.468259] [<ffffffff814e0922>] system_call_fastpath+0x16/0x1b
>> [ 570.377958] Bridge firewalling registered
>>
>> ------------------------------------------------
>>
>> The reason is that when the bridge dev's address is changed, the
>> br_fdb_change_mac_address() will add new address in fdb, but when
>> the bridge was removed, the address entry in the fdb did not free,
>> the bridge_fdb_cache still has objects when destroy the cache, Fix
>> this by flushing the bridge address entry when removing the bridge.
>>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> net/bridge/br_if.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>> index 6e6194f..98084d8 100644
>> --- a/net/bridge/br_if.c
>> +++ b/net/bridge/br_if.c
>> @@ -172,6 +172,10 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
>> del_nbp(p);
>> }
>>
>> + spin_lock_bh(&br->hash_lock);
>> + fdb_delete_by_addr(br, br->dev->dev_addr, 0);
>> + spin_unlock_bh(&br->hash_lock);
>
> You are not deleting entries with vid other than 0.
> If we have added some vid, there might be fdb entries with that vid
> whose dst is NULL, which also should be cleaned up.
>
> How about deleting all fdb entries whose dst is NULL?
> br_fdb_delete_by_port() looks able to be called with p == NULL as well.
>
> Thanks,
> Toshiaki Makita
>
agree, I miss the vid problem, I should deleteing all entries about this br.
Regards
Ding
>> +
>> br_vlan_flush(br);
>> del_timer_sync(&br->gc_timer);
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> .
>
^ permalink raw reply
* Re: [PATCH net RESEND] bridge: flush br's address entry in fdb when remove the bridge dev
From: Ding Tianhong @ 2013-11-20 1:05 UTC (permalink / raw)
To: Vlad Yasevich, Stephen Hemminger, David S. Miller, bridge, Netdev
In-Reply-To: <528B8926.1010201@gmail.com>
On 2013/11/19 23:52, Vlad Yasevich wrote:
> On 11/18/2013 09:57 PM, Ding Tianhong wrote:
>> When the following commands are executed:
>>
>> brctl addbr br0
>> ifconfig br0 hw ether <addr>
>> rmmod bridge
>>
>> The calltrace will occur:
>>
>> [ 563.312114] device eth1 left promiscuous mode
>> [ 563.312188] br0: port 1(eth1) entered disabled state
>> [ 563.468190] kmem_cache_destroy bridge_fdb_cache: Slab cache still has objects
>> [ 563.468197] CPU: 6 PID: 6982 Comm: rmmod Tainted: G O 3.12.0-0.7-default+ #9
>> [ 563.468199] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
>> [ 563.468200] 0000000000000880 ffff88010f111e98 ffffffff814d1c92 ffff88010f111eb8
>> [ 563.468204] ffffffff81148efd ffff88010f111eb8 0000000000000000 ffff88010f111ec8
>> [ 563.468206] ffffffffa062a270 ffff88010f111ed8 ffffffffa063ac76 ffff88010f111f78
>> [ 563.468209] Call Trace:
>> [ 563.468218] [<ffffffff814d1c92>] dump_stack+0x6a/0x78
>> [ 563.468234] [<ffffffff81148efd>] kmem_cache_destroy+0xfd/0x100
>> [ 563.468242] [<ffffffffa062a270>] br_fdb_fini+0x10/0x20 [bridge]
>> [ 563.468247] [<ffffffffa063ac76>] br_deinit+0x4e/0x50 [bridge]
>> [ 563.468254] [<ffffffff810c7dc9>] SyS_delete_module+0x199/0x2b0
>> [ 563.468259] [<ffffffff814e0922>] system_call_fastpath+0x16/0x1b
>> [ 570.377958] Bridge firewalling registered
>>
>> ------------------------------------------------
>>
>> The reason is that when the bridge dev's address is changed, the
>> br_fdb_change_mac_address() will add new address in fdb, but when
>> the bridge was removed, the address entry in the fdb did not free,
>> the bridge_fdb_cache still has objects when destroy the cache, Fix
>> this by flushing the bridge address entry when removing the bridge.
>>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> net/bridge/br_if.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>> index 6e6194f..98084d8 100644
>> --- a/net/bridge/br_if.c
>> +++ b/net/bridge/br_if.c
>> @@ -172,6 +172,10 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
>> del_nbp(p);
>> }
>>
>> + spin_lock_bh(&br->hash_lock);
>> + fdb_delete_by_addr(br, br->dev->dev_addr, 0);
>> + spin_unlock_bh(&br->hash_lock);
>> +
>> br_vlan_flush(br);
>> del_timer_sync(&br->gc_timer);
>>
>>
>
> I think you need to use fdb_delete_by_port(br, NULL, 1); here.
>
> The reason is that if the bridge had vlan filtering configured, when the
> bridge mac address was changed, an fdb would have been created for
> for ever vlan configured on the bridge. You delete only vlan0, so you
> would still have a leak.
>
> -vlad
agree, I miss the vid problem, I will fix it.
Regards
Ding
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> .
>
^ permalink raw reply
* Re: [PATCH net] virtio-net: fix page refcnt leaking when fail to allocate frag skb
From: Michael Dalton @ 2013-11-20 1:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: Michael S. Tsirkin, netdev, linux-kernel, lf-virt, Eric Dumazet
In-Reply-To: <1384898411.8604.127.camel@edumazet-glaptop2.roam.corp.google.com>
Hi,
After further reflection I think we're looking at two related issues:
(a) a memory leak that Jason has identified that occurs when a memory
allocation fails in receive_mergeable. Jasons commit solves this issue.
(b) virtio-net does not dequeue all buffers for a packet in the
case that an error occurs on receive and mergeable receive buffers is
enabled.
For (a), this bug is new and due to changes in 2613af0ed18a, and the
net impact is memory leak on the physical page. However, I believe (b)
has always been possible in some form because if page_to_skb() returns
NULL (e.g., due to SKB allocation failure), receive_mergeable is never
called. AFAICT this is also the behavior prior to 2613af0ed18a.
The net impact of (b) would be that virtio-net would interpret a packet
buffer that is in the middle of a mergeable packet as the start of a
new packet, which is definitely also a bug (and the buffer contents
could contain bytes that resembled a valid virtio-net header).
A solution for (b) will require handling both the page_to_skb memory
allocation failures and the memory allocation failures in
receive_mergeable introduced by 2613af0ed18a.
Best,
Mike
^ permalink raw reply
* Re: [PATCH net] packet: fix use after free race in send path when dev is released
From: David Miller @ 2013-11-20 1:34 UTC (permalink / raw)
To: dborkman; +Cc: netdev, noureddine, greearb
In-Reply-To: <1384902503-1588-1-git-send-email-dborkman@redhat.com>
From: Daniel Borkmann <dborkman@redhat.com>
Date: Wed, 20 Nov 2013 00:08:23 +0100
> To avoid reverting 827d9780 entirely, we could make use of po->running
> member that gets reset when we're calling __unregister_prot_hook() in
> packet_notifier() when we receive NETDEV_DOWN or NETDEV_UNREGISTER
> notification. Plus, we still need to hold ref to the netdev, so
> that we can assure it won't be released while we're in send path.
The avoidance of the atomic ref counting of the network device is the
main performance gain we get from that commit.
Now we'll be doing the refcount _and_ taking a spinlock, it'll be
worse than beforehand.
And this is doubly silly because we already have a reference
when we install the device into po->prot_hook.dev
I bet you can fix this by just deferring the NETDEV_UNREGISTER
AF_PACKET notifier work to RCU.
^ permalink raw reply
* Re: [PATCHv2] net: core: Always propagate flag changes to interfaces
From: Vlad Yasevich @ 2013-11-20 1:41 UTC (permalink / raw)
To: netdev
In-Reply-To: <1384907865-14409-1-git-send-email-vyasevic@redhat.com>
On 11/19/2013 07:37 PM, Vlad Yasevich wrote:
> The following commit:
> b6c40d68ff6498b7f63ddf97cf0aa818d748dee7
> net: only invoke dev->change_rx_flags when device is UP
>
> tried to fix a problem with VLAN devices and promiscuouse flag setting.
> The issue was that VLAN device was setting a flag on an interface that
> was down, thus resulting in bad promiscuity count.
> This commit blocked flag propagation to any device that is currently
> down.
>
> A later commit:
> deede2fabe24e00bd7e246eb81cd5767dc6fcfc7
> vlan: Don't propagate flag changes on down interfaces
>
> fixed VLAN code to only propagate flags when the VLAN interface is up,
> thus fixing the same issue as above, only localized to VLAN.
>
> The problem we have now is that if we have create a complex stack
> involving multiple software devices like bridges, bonds, and vlans,
> then it is possible that the flags would not propagate properly to
> the physical devices. A simple examle of the scenario is the
> following:
>
> eth0----> bond0 ----> bridge0 ---> vlan50
>
> If bond0 or eth0 happen to be down at the time bond0 is added to
> the bridge, then eth0 will never have promisc mode set which is
> currently required for operation as part of the bridge. As a
> result, packets with vlan50 will be dropped by the interface.
>
> The only 2 devices that implement the special flag handling are
> VLAN and DSA and they both have required code to prevent incorrect
> flag propagation. As a result we can remove the generic solution
> introduced in b6c40d68ff6498b7f63ddf97cf0aa818d748dee7 and leave
> it to the individual devices to decide whether they will block
> flag propagation or not.
>
> Reported-by: Stefan Priebe <s.priebe@profihost.ag>
> Suggested-by: Veaceslav Falico <vfalico@redhat.com>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
> v1-v2: rebased on the proper net tree.
>
> net/core/dev.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 974143d..d856e34 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4991,7 +4991,7 @@ static void dev_change_rx_flags(struct net_device *dev, int flags)
> {
> const struct net_device_ops *ops = dev->netdev_ops;
>
> - if ((dev->flags & IFF_UP) && ops->ndo_change_rx_flags)
> + if (ops->ndo_change_rx_flags)
> ops->ndo_change_rx_flags(dev, flags);
> }
>
> @@ -6885,10 +6885,6 @@ static struct pernet_operations __net_initdata default_device_ops = {
> .exit = default_device_exit,
> .exit_batch = default_device_exit_batch,
> };
> -
> -/*
> - * Initialize the DEV module. At boot time this walks the device list and
> - * unhooks any devices that fail to initialise (normally hardware not
> * present) and leaves us with a valid list of present and active devices.
> *
> */
>
Hm.. not sure where this last hunk came from... taking a look. sorry
about this.
-vlad
^ permalink raw reply
* [PATCHv3] net: core: Always propagate flag changes to interfaces
From: Vlad Yasevich @ 2013-11-20 1:47 UTC (permalink / raw)
To: netdev; +Cc: s.priebe, vfalico, Vlad Yasevich
The following commit:
b6c40d68ff6498b7f63ddf97cf0aa818d748dee7
net: only invoke dev->change_rx_flags when device is UP
tried to fix a problem with VLAN devices and promiscuouse flag setting.
The issue was that VLAN device was setting a flag on an interface that
was down, thus resulting in bad promiscuity count.
This commit blocked flag propagation to any device that is currently
down.
A later commit:
deede2fabe24e00bd7e246eb81cd5767dc6fcfc7
vlan: Don't propagate flag changes on down interfaces
fixed VLAN code to only propagate flags when the VLAN interface is up,
thus fixing the same issue as above, only localized to VLAN.
The problem we have now is that if we have create a complex stack
involving multiple software devices like bridges, bonds, and vlans,
then it is possible that the flags would not propagate properly to
the physical devices. A simple examle of the scenario is the
following:
eth0----> bond0 ----> bridge0 ---> vlan50
If bond0 or eth0 happen to be down at the time bond0 is added to
the bridge, then eth0 will never have promisc mode set which is
currently required for operation as part of the bridge. As a
result, packets with vlan50 will be dropped by the interface.
The only 2 devices that implement the special flag handling are
VLAN and DSA and they both have required code to prevent incorrect
flag propagation. As a result we can remove the generic solution
introduced in b6c40d68ff6498b7f63ddf97cf0aa818d748dee7 and leave
it to the individual devices to decide whether they will block
flag propagation or not.
Reported-by: Stefan Priebe <s.priebe@profihost.ag>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
v2->v3: Removed a strange chunk that modified comments. Not sure where it
came from.
net/core/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 974143d..da9c5e1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4991,7 +4991,7 @@ static void dev_change_rx_flags(struct net_device *dev, int flags)
{
const struct net_device_ops *ops = dev->netdev_ops;
- if ((dev->flags & IFF_UP) && ops->ndo_change_rx_flags)
+ if (ops->ndo_change_rx_flags)
ops->ndo_change_rx_flags(dev, flags);
}
--
1.8.4.2
^ permalink raw reply related
* [PATCH net v2] bridge: flush br's address entry in fdb when remove the bridge dev
From: Ding Tianhong @ 2013-11-20 1:58 UTC (permalink / raw)
To: Stephen Hemminger, David S. Miller, bridge, Netdev,
Toshiaki Makita, Vlad Yasevich
>From 241ddeaad73c7ed9e376a4b0926e79bdc3df9f58 Mon Sep 17 00:00:00 2001
From: Ding Tianhong <dingtianhong@huawei.com>
Date: Sat, 7 Dec 2013 22:12:05 +0800
Subject: [PATCH] bridge: flush br's address entry in fdb when remove the
bridge dev
When the following commands are executed:
brctl addbr br0
ifconfig br0 hw ether <addr>
rmmod bridge
The calltrace will occur:
[ 563.312114] device eth1 left promiscuous mode
[ 563.312188] br0: port 1(eth1) entered disabled state
[ 563.468190] kmem_cache_destroy bridge_fdb_cache: Slab cache still has objects
[ 563.468197] CPU: 6 PID: 6982 Comm: rmmod Tainted: G O 3.12.0-0.7-default+ #9
[ 563.468199] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[ 563.468200] 0000000000000880 ffff88010f111e98 ffffffff814d1c92 ffff88010f111eb8
[ 563.468204] ffffffff81148efd ffff88010f111eb8 0000000000000000 ffff88010f111ec8
[ 563.468206] ffffffffa062a270 ffff88010f111ed8 ffffffffa063ac76 ffff88010f111f78
[ 563.468209] Call Trace:
[ 563.468218] [<ffffffff814d1c92>] dump_stack+0x6a/0x78
[ 563.468234] [<ffffffff81148efd>] kmem_cache_destroy+0xfd/0x100
[ 563.468242] [<ffffffffa062a270>] br_fdb_fini+0x10/0x20 [bridge]
[ 563.468247] [<ffffffffa063ac76>] br_deinit+0x4e/0x50 [bridge]
[ 563.468254] [<ffffffff810c7dc9>] SyS_delete_module+0x199/0x2b0
[ 563.468259] [<ffffffff814e0922>] system_call_fastpath+0x16/0x1b
[ 570.377958] Bridge firewalling registered
--------------------------- cut here -------------------------------
The reason is that when the bridge dev's address is changed, the
br_fdb_change_mac_address() will add new address in fdb, but when
the bridge was removed, the address entry in the fdb did not free,
the bridge_fdb_cache still has objects when destroy the cache, Fix
this by flushing the bridge address entry when removing the bridge.
v2: according to the Toshiaki Makita and Vlad's suggestion, I only
delete the vlan0 entry, it still have a leak here if the vlan id
is other number, so I need to call fdb_delete_by_port(br, NULL, 1)
to flush all entries whose dst is NULL for the bridge.
Suggested-by: Toshiaki Makita <toshiaki.makita1@gmail.com>
Suggested-by: Vlad Yasevich <vyasevich@gmail.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
net/bridge/br_if.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 6e6194f..4bf02ad 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -172,6 +172,8 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
del_nbp(p);
}
+ br_fdb_delete_by_port(br, NULL, 1);
+
br_vlan_flush(br);
del_timer_sync(&br->gc_timer);
--
1.8.0
^ permalink raw reply related
* Re: [PATCH net-next 4/4] virtio-net: auto-tune mergeable rx buffer size for improved performance
From: Rusty Russell @ 2013-11-20 2:06 UTC (permalink / raw)
To: Eric Dumazet, Jason Wang
Cc: Michael Dalton, Michael S. Tsirkin, netdev, Daniel Borkmann,
virtualization, Eric Dumazet, David S. Miller
In-Reply-To: <1384328458.28458.96.camel@edumazet-glaptop2.roam.corp.google.com>
Eric Dumazet <eric.dumazet@gmail.com> writes:
> On Wed, 2013-11-13 at 15:10 +0800, Jason Wang wrote:
>
>> There's one concern with EWMA. How well does it handle multiple streams
>> each with different packet size? E.g there may be two flows, one with
>> 256 bytes each packet another is 64K. Looks like it can result we
>> allocate PAGE_SIZE buffer for 256 (which is bad since the
>> payload/truesize is low) bytes or 1500+ for 64K buffer (which is ok
>> since we can do coalescing).
>
> It's hard to predict the future ;)
>
> 256 bytes frames consume 2.5 KB anyway on a traditional NIC.
> If it was a concern, we would have it already.
>
> If you receive a mix of big and small frames, there is no win.
Well, that's not quite true. The device could optimistically look
through the queue a bit for a small buffer; it does not need to consume
in order. We'd probably want a feature bit for this.
I look forward to your thoughts on what mixing algorithm of different
sizes to use, of course.
Meanwhile, I suspect your patch works well because of 4k pages. 80%
non-GSO packets won't drop the average len below 4k. On PPC64 with 64k
pages, that's not true.
I wonder if "last used len" would work about as well; it really might if
we had a smarter device...
Cheers,
Rusty.
^ permalink raw reply
* [PATCH 1/1] Workaround for Suspend/Resume issue of AX88772B under ChromeOS
From: freddy-knRN6Y/kmf1NUHwG+Fw1Kw @ 2013-11-20 2:11 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
davemloft-fT/PcQaiUtIeIZ0/mPfg9Q, louis-knRN6Y/kmf1NUHwG+Fw1Kw,
allan-knRN6Y/kmf1NUHwG+Fw1Kw, grundler-hpIqsD4AKlfQT0dZR+AlfA,
daniel-knRN6Y/kmf1NUHwG+Fw1Kw, Freddy Xin
From: Freddy Xin <freddy-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org>
This patch adds a workaroud to solve Suspend/Resume issue that AX88772B turns
off its Ethernet PHY power in the case that REMOTE_WAKEUP feature doesn't
be set when system suspend. In this case, the PHY power will not be turned
on again when system resume, so the HW reset must be added in the resume function.
Signed-off-by: Freddy Xin <freddy-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org>
Tested-by: Grant Grundler <grundler-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
drivers/net/usb/asix.h | 1 +
drivers/net/usb/asix_devices.c | 16 +++++++++++++++-
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
index bdaa12d..0e975aa 100644
--- a/drivers/net/usb/asix.h
+++ b/drivers/net/usb/asix.h
@@ -48,6 +48,7 @@
#define AX_CMD_SET_SW_MII 0x06
#define AX_CMD_READ_MII_REG 0x07
#define AX_CMD_WRITE_MII_REG 0x08
+#define AX_CMD_STATMNGSTS_REG 0x09
#define AX_CMD_SET_HW_MII 0x0a
#define AX_CMD_READ_EEPROM 0x0b
#define AX_CMD_WRITE_EEPROM 0x0c
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 386a3df..3ac9958 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -845,6 +845,20 @@ static int ax88178_bind(struct usbnet *dev, struct usb_interface *intf)
return 0;
}
+static int asix_resume(struct usb_interface *intf)
+{
+ struct usbnet *dev = usb_get_intfdata(intf);
+ u8 chipcode = 0;
+
+ asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG, 0, 0, 1, &chipcode);
+ chipcode &= 0x70;
+
+ if (dev->driver_info->reset && (chipcode == 0x20))
+ dev->driver_info->reset(dev);
+
+ return usbnet_resume(intf);
+}
+
static const struct driver_info ax8817x_info = {
.description = "ASIX AX8817x USB 2.0 Ethernet",
.bind = ax88172_bind,
@@ -1101,7 +1115,7 @@ static struct usb_driver asix_driver = {
.id_table = products,
.probe = usbnet_probe,
.suspend = usbnet_suspend,
- .resume = usbnet_resume,
+ .resume = asix_resume,
.disconnect = usbnet_disconnect,
.supports_autosuspend = 1,
.disable_hub_initiated_lpm = 1,
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH net v3 1/2] net: sched: tbf: fix calculation of max_size
From: Yang Yingliang @ 2013-11-20 2:14 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: davem, netdev, eric.dumazet, jpirko
In-Reply-To: <20131119103828.5697d5c9@redhat.com>
On 2013/11/19 17:38, Jesper Dangaard Brouer wrote:
> On Tue, 19 Nov 2013 15:25:38 +0800
> Yang Yingliang <yangyingliang@huawei.com> wrote:
>
>> commit b757c9336d63f94c6b57532(tbf: improved accuracy at high rates)
>> introduce a regression.
>>
>> With the follow command:
>> tc qdisc add dev eth1 root handle 1: tbf latency 50ms burst 10KB rate 30gbit mtu 64k
>>
>> Without this patch, the max_size value is 10751(bytes).
>> But, in fact, the real max_size value should be smaller than 7440(bytes).
>> Or a packet whose length is bigger than 7440 will cause network congestion.
>> Because the packet is so big that can't get enough tokens. Even all the tokens
>> in the buffer is given to the packet.
>
> Sorry, but I don't like the commit message. The real problem is the
> value in q->buffer, and that the userspace rate table cannot handle
> these high rates, which you don't mention.
>
> I would write something like:
>
> The kernel no longer uses the userspace provided rate table. Thus, it
> is wrong to calculate max_size based on this rate table. At high rates
> this rate table gets very inaccurate, which can lead wrong calculation
> of max_size.
>
> Consequence of max_size being too large is severe, and cause packets
> being stalled in tbf_dequeue() because it cannot get enough tokens.
> The max_size guards against enqueuing packet sizes above q->buffer
> "time" in tbf_enqueue().
>
> This patch fixes the calculation of max_size. By ... add desc ...
> perhaps also mention how it is connected to p->mtu (with is also a
> "time" value).
>
>
> The rest of the patch looks okay now, one point below though.
OK, change it in v4.
Thanks!
>
>
>> diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
>> index 68f9859..c194129 100644
>> --- a/net/sched/sch_tbf.c
>> +++ b/net/sched/sch_tbf.c
> [...]
>> @@ -339,30 +326,46 @@ static int tbf_change(struct Qdisc *sch, struct nlattr *opt)
> [...]
>> + for (n = 0; n < 65536; n++)
>> + if (psched_l2t_ns(&q->rate, n) > q->buffer)
>> + break;
>> + max_size = min_t(u32, n, (256ULL << qopt->rate.cell_log) - 1);
>
> I'm a little uncertain about, if using the 65536 constant is okay, or
> considered "bad style".
I'll use a MACRO to instead of this constant in v4.
Thanks!
>
> I'm still a little confused/uncertain why we need the "qopt->rate.cell_log".
>
Because we need max_size be smaller than mtu(user input in bytes).
E.g. if user inputs like this "... burst 100KB rate 100mbit mtu 4095",
without this patch, max_size is 4095.
But with this patch, if don't use cell_log, max_size is 102400.
I think it's not correct, so I used cell_log here.
^ permalink raw reply
* [PATCH] ethtool: fix man page for new speeds
From: Jesse Brandeburg @ 2013-11-20 2:25 UTC (permalink / raw)
To: bhutchings; +Cc: netdev
both the 20000 and 40000 speeds either were wrong or just
not documented. Fix both.
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
ethtool.8.in | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/ethtool.8.in b/ethtool.8.in
index f587ec8..f320f7f 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -549,8 +549,12 @@ lB l lB.
0x020 1000 Full
0x8000 2500 Full (not supported by IEEE standards)
0x1000 10000 Full
-0x20000 20000MLD2 Full (not supported by IEEE standards)
-0x40000 20000KR2 Full (not supported by IEEE standards)
+0x200000 20000MLD2 Full (not supported by IEEE standards)
+0x400000 20000KR2 Full (not supported by IEEE standards)
+0x800000 40000KR4 Full
+0x1000000 40000CR4 Full
+0x2000000 40000SR4 Full
+0x4000000 40000LR4 Full
.TE
.TP
.BI phyad \ N
^ permalink raw reply related
* Re: [PATCH net] virtio-net: fix page refcnt leaking when fail to allocate frag skb
From: Jason Wang @ 2013-11-20 3:00 UTC (permalink / raw)
To: Eric Dumazet
Cc: Michael Dalton, mst, netdev, linux-kernel, virtualization,
Eric Dumazet
In-Reply-To: <1384869828.8604.97.camel@edumazet-glaptop2.roam.corp.google.com>
On 11/19/2013 10:03 PM, Eric Dumazet wrote:
> On Tue, 2013-11-19 at 16:05 +0800, Jason Wang wrote:
>> > We need to drop the refcnt of page when we fail to allocate an skb for frag
>> > list, otherwise it will be leaked. The bug was introduced by commit
>> > 2613af0ed18a11d5c566a81f9a6510b73180660a ("virtio_net: migrate mergeable rx
>> > buffers to page frag allocators").
>> >
>> > Cc: Michael Dalton <mwdalton@google.com>
>> > Cc: Eric Dumazet <edumazet@google.com>
>> > Cc: Rusty Russell <rusty@rustcorp.com.au>
>> > Cc: Michael S. Tsirkin <mst@redhat.com>
>> > Signed-off-by: Jason Wang <jasowang@redhat.com>
>> > ---
>> > The patch was needed for 3.12 stable.
> Good catch, but if we return from receive_mergeable() in the 'middle'
> of the frags we would need for the current skb, who will
> call the virtqueue_get_buf() to flush the remaining frags ?
>
> Don't we also need to call virtqueue_get_buf() like
>
> while (--num_buf) {
> buf = virtqueue_get_buf(rq->vq, &len);
> if (!buf)
> break;
> put_page(virt_to_head_page(buf));
> }
>
> ?
Yes we need this, will send V2.
Thanks
^ permalink raw reply
* Re: [PATCH net] virtio-net: fix page refcnt leaking when fail to allocate frag skb
From: Jason Wang @ 2013-11-20 3:05 UTC (permalink / raw)
To: Michael S. Tsirkin, Eric Dumazet
Cc: Michael Dalton, netdev, linux-kernel, virtualization,
Eric Dumazet
In-Reply-To: <20131119204909.GA15004@redhat.com>
On 11/20/2013 04:49 AM, Michael S. Tsirkin wrote:
> On Tue, Nov 19, 2013 at 06:03:48AM -0800, Eric Dumazet wrote:
>> On Tue, 2013-11-19 at 16:05 +0800, Jason Wang wrote:
>>> We need to drop the refcnt of page when we fail to allocate an skb for frag
>>> list, otherwise it will be leaked. The bug was introduced by commit
>>> 2613af0ed18a11d5c566a81f9a6510b73180660a ("virtio_net: migrate mergeable rx
>>> buffers to page frag allocators").
>>>
>>> Cc: Michael Dalton <mwdalton@google.com>
>>> Cc: Eric Dumazet <edumazet@google.com>
>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>> ---
>>> The patch was needed for 3.12 stable.
>> Good catch, but if we return from receive_mergeable() in the 'middle'
>> of the frags we would need for the current skb, who will
>> call the virtqueue_get_buf() to flush the remaining frags ?
>>
>> Don't we also need to call virtqueue_get_buf() like
>>
>> while (--num_buf) {
>> buf = virtqueue_get_buf(rq->vq, &len);
>> if (!buf)
>> break;
>> put_page(virt_to_head_page(buf));
>> }
>>
>> ?
>>
>>
>
> Let me explain what worries me in your suggestion:
>
> struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC);
> if (unlikely(!nskb)) {
> head_skb->dev->stats.rx_dropped++;
> return -ENOMEM;
> }
>
> is this the failure case we are talking about?
>
> I think this is a symprom of a larger problem
> introduced by 2613af0ed18a11d5c566a81f9a6510b73180660a,
> namely that we now need to allocate memory in the
> middle of processing a packet.
>
>
> I think discarding a completely valid and well-formed
> packet from the receive queue because we are unable
> to allocate new memory with GFP_ATOMIC
> for future packets is not a good idea.
>
> It certainly violates the principle of least surprize:
> when one sees host pass packet to guest, one expects
> the packet to get into the networking stack, not get
> dropped by the driver internally.
> Guest stack can do with the packet what it sees fit.
>
> We actually wake up a thread if we can't fill up the queue,
> that will fill it up in GFP_KERNEL context.
>
> So I think we should find a way to pre-allocate if necessary and avoid
> error paths where allocating new memory is a required to avoid drops.
>
The problem happens only on memory pressure, this pre-allocation may add
more stress on this.
^ permalink raw reply
* Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.
From: Alan Stern @ 2013-11-20 3:09 UTC (permalink / raw)
To: Sarah Sharp
Cc: David Laight, Ben Hutchings, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20131119225822.GA9611@xanatos>
On Tue, 19 Nov 2013, Sarah Sharp wrote:
> The xHCI driver can limit the number of sg-list entries through
> hcd->self.sg_tablesize. It's currently set to ~0, which is "however
> many entries you want. You could set that to the number of TRBs in a
> segment (minus one for the link TRB).
>
> The usb-storage and uas drivers currently use sg_tablesize. Could the
> network stack be taught to use sg_tablesize as well?
The sg_tablesize you're talking about is a field in struct usb_bus
(there's a similar field in struct scsi_host_template). It's not
relevant to the network stack, since network interfaces aren't USB host
controllers (or SCSI hosts).
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net] ipv4: fix race in concurrent ip_route_input_slow()
From: Alexei Starovoitov @ 2013-11-20 3:12 UTC (permalink / raw)
To: David S. Miller; +Cc: Eric Dumazet, netdev
CPUs can ask for local route via ip_route_input_noref() concurrently.
if nh_rth_input is not cached yet, CPUs will proceed to allocate
equivalent DSTs on 'lo' and then will try to cache them in nh_rth_input
via rt_cache_route()
Most of the time they succeed, but on occasion the following two lines:
orig = *p;
prev = cmpxchg(p, orig, rt);
in rt_cache_route() do race and one of the cpus fails to complete cmpxchg.
But ip_route_input_slow() doesn't check the return code of rt_cache_route(),
so dst is leaking. dst_destroy() is never called and 'lo' device
refcnt doesn't go to zero, which can be seen in the logs as:
unregister_netdevice: waiting for lo to become free. Usage count = 1
Adding mdelay() between above two lines makes it easily reproducible.
Fix it similar to nh_pcpu_rth_output case.
Fixes: d2d68ba9fe8b ("ipv4: Cache input routes in fib_info nexthops.")
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
David,
looks like caacf05e5ad1 ("ipv4: Properly purge netdev references on uncached routes.")
fixed the race for nexthop/rth_output, but missed it for rth_input.
I'm not sure what was the assumption why it's not needed there.
We're definitely seeing it every 12-24hr during nightly tests.
There are several bugs on ubuntu and debian forums with similar description.
Some were closed, since folks struggled to reproduce it.
It took us more than a month to debug it.
Please queue for stable.
Alternative fix:
rt_free(rth);
goto local_input;
imo is uglier.
Just like rt_free(rth) followed by re-read of nh_rth_input and re-check_valid
net/ipv4/route.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index f428935..f8da282 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1776,8 +1776,12 @@ local_input:
rth->dst.error= -err;
rth->rt_flags &= ~RTCF_LOCAL;
}
- if (do_cache)
- rt_cache_route(&FIB_RES_NH(res), rth);
+ if (do_cache) {
+ if (unlikely(!rt_cache_route(&FIB_RES_NH(res), rth))) {
+ rth->dst.flags |= DST_NOCACHE;
+ rt_add_uncached_list(rth);
+ }
+ }
skb_dst_set(skb, &rth->dst);
err = 0;
goto out;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH net] virtio-net: fix page refcnt leaking when fail to allocate frag skb
From: Jason Wang @ 2013-11-20 3:17 UTC (permalink / raw)
To: Michael Dalton, Eric Dumazet
Cc: Michael S. Tsirkin, netdev, linux-kernel, lf-virt, Eric Dumazet
In-Reply-To: <CANJ5vP+ZxtDf8OzREpyDpgh8JLnWGEjkUcS388hi+AOjdu0fZw@mail.gmail.com>
On 11/20/2013 09:34 AM, Michael Dalton wrote:
> Hi,
>
> After further reflection I think we're looking at two related issues:
> (a) a memory leak that Jason has identified that occurs when a memory
> allocation fails in receive_mergeable. Jasons commit solves this issue.
> (b) virtio-net does not dequeue all buffers for a packet in the
> case that an error occurs on receive and mergeable receive buffers is
> enabled.
>
> For (a), this bug is new and due to changes in 2613af0ed18a, and the
> net impact is memory leak on the physical page. However, I believe (b)
> has always been possible in some form because if page_to_skb() returns
> NULL (e.g., due to SKB allocation failure), receive_mergeable is never
> called. AFAICT this is also the behavior prior to 2613af0ed18a.
>
> The net impact of (b) would be that virtio-net would interpret a packet
> buffer that is in the middle of a mergeable packet as the start of a
> new packet, which is definitely also a bug (and the buffer contents
> could contain bytes that resembled a valid virtio-net header).
>
> A solution for (b) will require handling both the page_to_skb memory
> allocation failures and the memory allocation failures in
> receive_mergeable introduced by 2613af0ed18a.
Ture, so we first need a patch to solve page_to_skb() failure which
could be used for stable tree prior to 2613af0ed18a. Then another patch
to solve the issue introduced by 2613af0ed18a which could be only used
for 3.12 stable. Will draft patches for them.
Thanks
>
> Best,
>
> Mike
^ permalink raw reply
* RE: [PATCH net v4 3/4] r8152: support stopping/waking tx queue
From: hayeswang @ 2013-11-20 3:28 UTC (permalink / raw)
To: 'David Miller'
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, 'nic_swsd',
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20131119.152536.146706009595105149.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
David Miller [mailto:davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org]
[...]
> This is racy.
>
> You have nothing which synchronizes r8152_tx_agg_fill() and
> rtl8152_start_xmit(),
> therefore:
>
> > + if (netif_queue_stopped(tp->netdev))
> > + netif_wake_queue(tp->netdev);
> > +
>
> A netif_stop_queue() can occur right after the
> netif_queue_stopped() check,
> meaning you can end up with the queue being stopped forever
> and the TX queue
> stuck.
If the situation occurs, it means there is no tx buffer at that time. If the
netif_wake_queue() is called, only one more packet would be queued and the tx
queue would be stopped again after calling rtl8152_start_xmit(). That is, it
is not necessary to wake the queue. Besides, after the tx is completed, another
tasklet would be scheduled if there is any packet which is queued in the list.
That is, the r8152_tx_agg_fill() would be called and the netif_queue_stopped()
would be check againg, so the tx queue would not be stopped forever.
Best Regards,
Hayes
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: IPv6: Blackhole route support partial ?
From: Kamala R @ 2013-11-20 4:48 UTC (permalink / raw)
To: Kamala R, David Miller, linux-kernel, netdev
In-Reply-To: <20131115134811.GF26901@order.stressinduktion.org>
Hi,
I have sent across the patch on a separate thread. The subject of the mail is :
[PATCH 1/1] IPv6: Fixed support for blackhole and prohibit routes
Could you review ?
Thanks,
Kamala
On Fri, Nov 15, 2013 at 7:18 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> Hi!
>
> On Fri, Nov 15, 2013 at 03:06:28PM +0530, Kamala R wrote:
>> That's right. We don't show the error for IPv4 routes as it follows a
>> different path in kernel while dumping the information as compared
>> with IPv6. Therefore, "ip route show" does not show an error while "ip
>> -6 route show" does. So it looks to me that this a kernel problem
>> which needs to be fixed for consistent behavior. The simplest way to
>> fix this seems to be to set the error to zero while dumping the
>> information in the v6 path. I have tested this solution and found that
>> it works fine. Do you think this is the way to go ?
>
> If I understand you correctly you propose to drop the output of the error
> attribute for IPv6 routes too? It is not that important that those two
> outputs are identical and if you make a change, please introduce the
> error propagation for IPv4 so one can see the socket errors for those
> routes, too. I wouldn't drop those for IPv6 just for consistency reasons.
>
> Greetings,
>
> Hannes
>
^ permalink raw reply
* Re: [PATCH 1/1] IPv6: Fixed support for blackhole and prohibit routes
From: Hannes Frederic Sowa @ 2013-11-20 4:57 UTC (permalink / raw)
To: Kamala R; +Cc: netdev, David Miller, linux-kernel
In-Reply-To: <1384879773-8800-1-git-send-email-kamala@aristanetwors.com>
On Tue, Nov 19, 2013 at 10:19:33PM +0530, Kamala R wrote:
> From: Kamala R <kamala@aristanetworks.com>
>
> The behaviour of blackhole and prohibit routes has been corrected by setting the input and output
> function pointers of the dst variable appropriately. For blackhole routes, they are set to
> dst_discard and for prohibit routes they are set to ip6_pkt_prohibit and ip6_pkt_prohbit_out
> respectively.
>
> Signed-off-by: Kamala R <kamala@aristanetworks.com>
> ---
> net/ipv6/route.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index f54e3a1..f2289fd 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1564,21 +1564,25 @@ int ip6_route_add(struct fib6_config *cfg)
> goto out;
> }
> }
> - rt->dst.output = ip6_pkt_discard_out;
> - rt->dst.input = ip6_pkt_discard;
> +
> rt->rt6i_flags = RTF_REJECT|RTF_NONEXTHOP;
> + rt->dst.error = (cfg->fc_type == RTN_BLACKHOLE) ? -EINVAL
> + : ((cfg->fc_type == RTN_PROHIBIT ? -EACCES
> + : ((cfg->fc_type == RTN_THROW ? -EAGAIN
> + : -ENETUNREACH))));
Uh, sorry, I was a bit too unclear. This is rather ugly. I meant merging
the RTN_THROW and default case and just use one dst.error assignment
with a ternary operator where it makes sense.
> switch (cfg->fc_type) {
> case RTN_BLACKHOLE:
> - rt->dst.error = -EINVAL;
> + rt->dst.output = dst_discard;
> + rt->dst.input = dst_discard;
> break;
> case RTN_PROHIBIT:
> - rt->dst.error = -EACCES;
> + rt->dst.output = ip6_pkt_prohibit_out;
> + rt->dst.input = ip6_pkt_prohibit;
> break;
> case RTN_THROW:
> - rt->dst.error = -EAGAIN;
> - break;
> default:
> - rt->dst.error = -ENETUNREACH;
> + rt->dst.output = ip6_pkt_discard_out;
> + rt->dst.input = ip6_pkt_discard;
> break;
case RTN_THROW:
default:
rt->dst.error = (cfg->fc_type == ...) ? ... : ...;
rt->dst.output = ....
rt->dst.input = ....
I also don't mind if you are more verbose and specify each case
independent. Just have a look how it looks more pleasing. ;)
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH net v4 3/4] r8152: support stopping/waking tx queue
From: David Miller @ 2013-11-20 5:22 UTC (permalink / raw)
To: hayeswang-Rasf1IRRPZFBDgjK7y7TUQ
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <9C2E97E7BED9426ABE0FF4F63E12E33D-Rasf1IRRPZGoECsaD+WFmw@public.gmane.org>
From: hayeswang <hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
Date: Wed, 20 Nov 2013 11:28:46 +0800
> David Miller [mailto:davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org]
> [...]
>> This is racy.
>>
>> You have nothing which synchronizes r8152_tx_agg_fill() and
>> rtl8152_start_xmit(),
>> therefore:
>>
>> > + if (netif_queue_stopped(tp->netdev))
>> > + netif_wake_queue(tp->netdev);
>> > +
>>
>> A netif_stop_queue() can occur right after the
>> netif_queue_stopped() check,
>> meaning you can end up with the queue being stopped forever
>> and the TX queue
>> stuck.
>
> If the situation occurs, it means there is no tx buffer at that time. If the
> netif_wake_queue() is called, only one more packet would be queued and the tx
> queue would be stopped again after calling rtl8152_start_xmit(). That is, it
> is not necessary to wake the queue. Besides, after the tx is completed, another
> tasklet would be scheduled if there is any packet which is queued in the list.
> That is, the r8152_tx_agg_fill() would be called and the netif_queue_stopped()
> would be check againg, so the tx queue would not be stopped forever.
Then the queue can be woken when in fact r8152_start_xmit() is not able to
actually queue packets. It is just as equally problematic.
You have to synchronize this state, somehow.
tg3 driver does this by taking netif tx queue lock during the wake
test sequence in TX reclaim. This works because ->ndo_start_xmit() is
run with this lock held.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: IPv6: Blackhole route support partial ?
From: David Miller @ 2013-11-20 5:24 UTC (permalink / raw)
To: kamala; +Cc: linux-kernel, netdev
In-Reply-To: <CAP8BHkGb-fsF-SKRZOqKvxAkxStwkuHMzGa1cbdot_pqum1pcA@mail.gmail.com>
From: Kamala R <kamala@aristanetworks.com>
Date: Wed, 20 Nov 2013 10:18:04 +0530
> I have sent across the patch on a separate thread. The subject of the mail is :
>
> [PATCH 1/1] IPv6: Fixed support for blackhole and prohibit routes
>
> Could you review ?
Can you just be patient? You only posted the patch in the last day,
many people review patches in their spare time.
^ permalink raw reply
* Re: [PATCH RFC] net: rework recvmsg handler msg_name and msg_namelen logic
From: Hannes Frederic Sowa @ 2013-11-20 5:37 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20131119.154058.2058377571860676832.davem@davemloft.net>
On Tue, Nov 19, 2013 at 03:40:58PM -0500, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Tue, 19 Nov 2013 06:27:59 +0100
>
> > This patch now always passes msg->msg_namelen as 0. recvmsg handlers must
> > set msg_namelen to the proper size <= sizeof(struct sockaddr_storage)
> > to return msg_name to the user.
> >
> > This prevents numerous uninitialized memory leaks we had in the
> > recvmsg handlers and makes it harder for new code to accidentally leak
> > uninitialized memory.
> >
> > Optimize for the case recvfrom is called with NULL as address. We don't
> > need to copy the address at all, so set it to NULL before invoking the
> > recvmsg handler. We can do so, because all the recvmsg handlers must
> > cope with the case a plain read() is called on them. read() also sets
> > msg_name to NULL.
> >
> > Also document these changes in include/linux/net.h as suggested by David
> > Miller.
> >
> > Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>
> Looks good to me.
>
> The only other invoker of ->recvmsg is vhost.c but it sets msg_name to NULL
> and msg_namelen to 0 so should be fine.
Thanks for the review!
I had no time to look after this change today. I want to make another
round of review tomorrow and will post a final version then.
^ permalink raw reply
* Re: [PATCH 1/1] Workaround for Suspend/Resume issue of AX88772B under ChromeOS
From: David Miller @ 2013-11-20 5:40 UTC (permalink / raw)
To: freddy
Cc: netdev, linux-usb, linux-kernel, davemloft, louis, allan,
grundler, daniel
In-Reply-To: <1384913496-26261-1-git-send-email-freddy@asix.com.tw>
From: freddy@asix.com.tw
Date: Wed, 20 Nov 2013 10:11:36 +0800
> From: Freddy Xin <freddy@asix.com.tw>
>
> This patch adds a workaroud to solve Suspend/Resume issue that AX88772B turns
> off its Ethernet PHY power in the case that REMOTE_WAKEUP feature doesn't
> be set when system suspend. In this case, the PHY power will not be turned
> on again when system resume, so the HW reset must be added in the resume function.
>
> Signed-off-by: Freddy Xin <freddy@asix.com.tw>
> Tested-by: Grant Grundler <grundler@chromium.org>
Like many similar patches I've seen recently, you cannot do this.
Now the the configuration that software things the PHY has is no
longer accurate.
If you absolutely must reset the chip on resume, you have to reprogram
the PHY with whatever settings were existed at the time of the suspend.
^ permalink raw reply
* RE: [PATCH net v4 3/4] r8152: support stopping/waking tx queue
From: hayeswang @ 2013-11-20 6:30 UTC (permalink / raw)
To: 'David Miller'
Cc: netdev, 'nic_swsd', linux-kernel, linux-usb
In-Reply-To: <20131120.002223.1492430030566522391.davem@davemloft.net>
David Miller [mailto:davem@davemloft.net]
[...]
> > If the situation occurs, it means there is no tx buffer at
> that time. If the
> > netif_wake_queue() is called, only one more packet would be
> queued and the tx
> > queue would be stopped again after calling
> rtl8152_start_xmit(). That is, it
> > is not necessary to wake the queue. Besides, after the tx
> is completed, another
> > tasklet would be scheduled if there is any packet which is
> queued in the list.
> > That is, the r8152_tx_agg_fill() would be called and the
> netif_queue_stopped()
> > would be check againg, so the tx queue would not be stopped forever.
>
> Then the queue can be woken when in fact r8152_start_xmit()
> is not able to
> actually queue packets. It is just as equally problematic.
>
> You have to synchronize this state, somehow.
>
> tg3 driver does this by taking netif tx queue lock during the wake
> test sequence in TX reclaim. This works because ->ndo_start_xmit() is
> run with this lock held.
Thanks for your answer. I would modify it.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox