Netdev List
 help / color / mirror / Atom feed
* Re: net/usb/ax88179_178a driver broken in linux-3.12
From: Sarah Sharp @ 2013-11-20 16:54 UTC (permalink / raw)
  To: David Laight
  Cc: David Miller, mlord, eric.dumazet, ming.lei, netdev, linux-usb
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7439@saturn3.aculab.com>

On Wed, Nov 20, 2013 at 09:54:02AM -0000, David Laight wrote:
> > From: Of David Miller
> > > From: "David Laight" <David.Laight@ACULAB.COM>
> > > Date: Tue, 19 Nov 2013 10:04:11 -0000
> > 
> > > There is a patch to xhci-ring.c that should fix the SG problem.
> > > http://www.spinics.net/lists/linux-usb/msg97176.html
> > >
> > > I think it should apply to the 3.12 sources.
> > 
> > David, please get this into Linus's tree and queued up for -stable
> > as soon as possible, thank you.
> 
> What do I have to do to expedite it?
> I thought that Sarah would have to push the patch through.

Yes, I think the patch should come through my tree.

> It might be necessary to limit the number of SG fragments that
> the mass storage (etc) can generate to 63 (from unlimited).
> That should be a 1 line change somewhere.

Greg's USB tree is frozen until after 3.13-rc1.  The patch will have to
wait until then, but it will be marked for stable.

I have no objection to the methodology of the patch, but we do need to
figure out how to limit the number of scatter-gather list entries in the
mass storage driver.  We're still figuring out the exact limitation we
need to use, and then the patch can be queued in my for-usb-linus branch
for 3.13-rc1.

Note that the patch is a bandaid fix, and will only help bulk endpoints
that are submitting scatter-gather transfers.  Interrupt endpoints using
scatter-gather will still run into these issues, but with David's patch,
the URB submission will now fail, rather than allowing the transfer to
be silently corrupted.

The real fix for this will probably be too large for stable, and is
likely to take at least a kernel revision to fix.

Sarah Sharp

^ permalink raw reply

* Re: [PATCH net] virtio-net: fix page refcnt leaking when fail to allocate frag skb
From: Michael S. Tsirkin @ 2013-11-20 17:03 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Michael Dalton, netdev, linux-kernel, virtualization,
	Eric Dumazet
In-Reply-To: <1384964061.8604.151.camel@edumazet-glaptop2.roam.corp.google.com>

On Wed, Nov 20, 2013 at 08:14:21AM -0800, Eric Dumazet wrote:
> On Wed, 2013-11-20 at 18:06 +0200, Michael S. Tsirkin wrote:
> 
> > Hmm some kind of disconnect here.
> > I got you rmanagement about bufferbloat.
> > 
> > What I am saying is that maybe we should drop packets more
> > aggressively: when we drop one packet of a flow, why not
> > drop everything that's queued and is for the same flow?
> 
> I really hope your TCP flows use SACK ;)
> 
> Please read the rfc 2018 introduction for details.
> 
> If a packet is dropped, it doesn't mean following packets _have_ to be
> dropped.


Got it, thanks.

^ permalink raw reply

* RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.
From: David Laight @ 2013-11-20 17:02 UTC (permalink / raw)
  To: Sarah Sharp; +Cc: Alan Stern, Ben Hutchings, netdev, linux-usb
In-Reply-To: <20131120160605.GB9611@xanatos>

> From: Sarah Sharp [mailto:sarah.a.sharp@linux.intel.com]
> On Wed, Nov 20, 2013 at 09:36:11AM -0000, David Laight wrote:
> > > From: Alan Stern [mailto:stern@rowland.harvard.edu]
> > > 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).
> > > >
...
> 
> Ok, so the networking layer should be fine.  However, with the current
> patch, if the mass storage driver sends down a scatter-gather list
> that's bigger than a ring segment, or needs to be split up so it doesn't
> cross 64K boundaries, then the URB submission will fail.  We don't want
> that to happen.
> 
> At the very least, we should limit hcd->self.sg_tablesize in
> drivers/usb/host/xhci.c to 63 (TRBS_PER_SEGMENT - 1).  But we could
> still potentially run into the 64K boundary issue in one or maybe all of
> those entries.  Would it be crazy to limit the number of entries to half
> that (31)?  It may impact performance, but it ensures that SCSI reads
> and writes don't randomly fail.  We can always increase the ring segment
> size in a later patch.

Unless there is a limit on the overall length of the transfer you'll always lose.
An aligned 4MB transfer in a contiguous memory buffer requires 64 TRBs.
So won't fit in a ring segment.

Although I'd guess that such transfers are unusual.
If you bet that crossing a 64k boundaries is unusual then 32 is probably
a sane limit.

Fragments over 16k can easily be split between ring segments - so a more
complex check could allow for that - but it won't be quick to write.

	David

^ permalink raw reply

* RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.
From: David Laight @ 2013-11-20 17:08 UTC (permalink / raw)
  To: Alan Stern
  Cc: Sarah Sharp, Ben Hutchings, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <Pine.LNX.4.44L0.1311201121350.1826-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>

> From: Alan Stern [mailto:stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org]
> On Wed, 20 Nov 2013, David Laight wrote:
> 
> > > From: Sarah Sharp
> > ...
> > > (Also, usb-storage aligns the block sizes to 512K, which explains why
> > > we've never had an issue with TD fragments with that driver.)
> >
> > What is a 'block' in that context?
> 
> I think Sarah means that usb-storage requires the block layer to align
> its data buffers to 512-byte boundaries.  (Note: 512 bytes, not 512K.)

I did think it might be a typo...

> Disk I/O naturally tends to be done in units of the page size, anyway,
> although raw I/O can involve single sectors.
> 
> If a user supplies an unaligned buffer, the block layer will set up a
> bounce buffer.

Ah, ok, some other systems only do that from byte-misaligned buffers
(for general disk access).
The ehci alignment rules force 512 byte alignment.

That does mean that USB2 mass storage devices (attached to xhci)
will never generate incorrectly aligned link TRBs.

The ax88179_178a driver might still do so.

	David



--
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: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Willy Tarreau @ 2013-11-20 17:12 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Thomas Petazzoni, netdev, Arnaud Ebalard, edumazet, Cong Wang,
	linux-arm-kernel
In-Reply-To: <1384710098.8604.58.camel@edumazet-glaptop2.roam.corp.google.com>

Hi guys,

On Sun, Nov 17, 2013 at 09:41:38AM -0800, Eric Dumazet wrote:
> On Sun, 2013-11-17 at 15:19 +0100, Willy Tarreau wrote:
> 
> > 
> > So it is fairly possible that in your case you can't fill the link if you
> > consume too many descriptors. For example, if your server uses TCP_NODELAY
> > and sends incomplete segments (which is quite common), it's very easy to
> > run out of descriptors before the link is full.
> 
> BTW I have a very simple patch for TCP stack that could help this exact
> situation...
> 
> Idea is to use TCP Small Queue so that we dont fill qdisc/TX ring with
> very small frames, and let tcp_sendmsg() have more chance to fill
> complete packets.
> 
> Again, for this to work very well, you need that NIC performs TX
> completion in reasonable amount of time...

Eric, first I would like to confirm that I could reproduce Arnaud's issue
using 3.10.19 (160 kB/s in the worst case).

Second, I confirm that your patch partially fixes it and my performance
can be brought back to what I had with 3.10-rc7, but with a lot of
concurrent streams. In fact, in 3.10-rc7, I managed to constantly saturate
the wire when transfering 7 concurrent streams (118.6 kB/s). With the patch
applied, performance is still only 27 MB/s at 7 concurrent streams, and I
need at least 35 concurrent streams to fill the pipe. Strangely, after
2 GB of cumulated data transferred, the bandwidth divided by 11-fold and
fell to 10 MB/s again.

If I revert both "0ae5f47eff tcp: TSQ can use a dynamic limit" and
your latest patch, the performance is back to original.

Now I understand there's a major issue with the driver. But since the
patch emphasizes the situations where drivers take a lot of time to
wake the queue up, don't you think there could be an issue with low
bandwidth links (eg: PPPoE over xDSL, 10 Mbps ethernet, etc...) ?
I'm a bit worried about what we might discover in this area I must
confess (despite generally being mostly focused on 10+ Gbps).

Best regards,
Willy

^ permalink raw reply

* RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.
From: David Laight @ 2013-11-20 17:16 UTC (permalink / raw)
  To: Sarah Sharp; +Cc: Alan Stern, Ben Hutchings, netdev, linux-usb
In-Reply-To: <20131120160605.GB9611@xanatos>

> Ok, so the networking layer should be fine.  However, with the current
> patch, if the mass storage driver sends down a scatter-gather list
> that's bigger than a ring segment, or needs to be split up so it doesn't
> cross 64K boundaries, then the URB submission will fail.  We don't want
> that to happen.

My suspicion is that long SG lists are unusual - otherwise the
ring expansion code would have been needed much earlier.

Can anyone remember whether that was needed because of long SG lists
or because of large numbers of outstanding requests?

I've seen it for network cards - but only because usbnet sends
down far too many tx buffers.

	David

^ permalink raw reply

* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Eric Dumazet @ 2013-11-20 17:30 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Arnaud Ebalard, Cong Wang, edumazet, linux-arm-kernel, netdev,
	Thomas Petazzoni
In-Reply-To: <20131120171227.GG8581@1wt.eu>

On Wed, 2013-11-20 at 18:12 +0100, Willy Tarreau wrote:
> Hi guys,

> Eric, first I would like to confirm that I could reproduce Arnaud's issue
> using 3.10.19 (160 kB/s in the worst case).
> 
> Second, I confirm that your patch partially fixes it and my performance
> can be brought back to what I had with 3.10-rc7, but with a lot of
> concurrent streams. In fact, in 3.10-rc7, I managed to constantly saturate
> the wire when transfering 7 concurrent streams (118.6 kB/s). With the patch
> applied, performance is still only 27 MB/s at 7 concurrent streams, and I
> need at least 35 concurrent streams to fill the pipe. Strangely, after
> 2 GB of cumulated data transferred, the bandwidth divided by 11-fold and
> fell to 10 MB/s again.
> 
> If I revert both "0ae5f47eff tcp: TSQ can use a dynamic limit" and
> your latest patch, the performance is back to original.
> 
> Now I understand there's a major issue with the driver. But since the
> patch emphasizes the situations where drivers take a lot of time to
> wake the queue up, don't you think there could be an issue with low
> bandwidth links (eg: PPPoE over xDSL, 10 Mbps ethernet, etc...) ?
> I'm a bit worried about what we might discover in this area I must
> confess (despite generally being mostly focused on 10+ Gbps).

Well, all TCP performance results are highly dependent on the workload,
and both receivers and senders behavior.

We made many improvements like TSO auto sizing, DRS (dynamic Right
Sizing), and if the application used some specific settings (like
SO_SNDBUF / SO_RCVBUF or other tweaks), we can not guarantee that same
exact performance is reached from kernel version X to kernel version Y.

We try to make forward progress, there is little gain to revert all
these great works. Linux had this tendency to favor throughput by using
overly large skbs. Its time to do better.

As explained, some drivers are buggy, and need fixes.

If nobody wants to fix them, this really means no one is interested
getting them fixed.

I am willing to help if you provide details, because otherwise I need
a crystal ball ;)

One known problem of TCP is the fact that an incoming ACK making room in
socket write queue immediately wakeup a blocked thread (POLLOUT), even
if only one MSS was ack, and write queue has 2MB of outstanding bytes.

All these scheduling problems should be identified and fixed, and yes,
this will require a dozen more patches.

max (128KB , 1-2 ms) of buffering per flow should be enough to reach
line rate, even for a single flow, but this means the sk_sndbuf value
for the socket must take into account the pipe size _plus_ 1ms of
buffering.

^ permalink raw reply

* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Willy Tarreau @ 2013-11-20 17:34 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Thomas Petazzoni, netdev, Arnaud Ebalard, edumazet, Cong Wang,
	linux-arm-kernel
In-Reply-To: <20131120171227.GG8581@1wt.eu>

On Wed, Nov 20, 2013 at 06:12:27PM +0100, Willy Tarreau wrote:
> Hi guys,
> 
> On Sun, Nov 17, 2013 at 09:41:38AM -0800, Eric Dumazet wrote:
> > On Sun, 2013-11-17 at 15:19 +0100, Willy Tarreau wrote:
> > 
> > > 
> > > So it is fairly possible that in your case you can't fill the link if you
> > > consume too many descriptors. For example, if your server uses TCP_NODELAY
> > > and sends incomplete segments (which is quite common), it's very easy to
> > > run out of descriptors before the link is full.
> > 
> > BTW I have a very simple patch for TCP stack that could help this exact
> > situation...
> > 
> > Idea is to use TCP Small Queue so that we dont fill qdisc/TX ring with
> > very small frames, and let tcp_sendmsg() have more chance to fill
> > complete packets.
> > 
> > Again, for this to work very well, you need that NIC performs TX
> > completion in reasonable amount of time...
> 
> Eric, first I would like to confirm that I could reproduce Arnaud's issue
> using 3.10.19 (160 kB/s in the worst case).
> 
> Second, I confirm that your patch partially fixes it and my performance
> can be brought back to what I had with 3.10-rc7, but with a lot of
> concurrent streams. In fact, in 3.10-rc7, I managed to constantly saturate
> the wire when transfering 7 concurrent streams (118.6 kB/s). With the patch
> applied, performance is still only 27 MB/s at 7 concurrent streams, and I
> need at least 35 concurrent streams to fill the pipe. Strangely, after
> 2 GB of cumulated data transferred, the bandwidth divided by 11-fold and
> fell to 10 MB/s again.
> 
> If I revert both "0ae5f47eff tcp: TSQ can use a dynamic limit" and
> your latest patch, the performance is back to original.
> 
> Now I understand there's a major issue with the driver. But since the
> patch emphasizes the situations where drivers take a lot of time to
> wake the queue up, don't you think there could be an issue with low
> bandwidth links (eg: PPPoE over xDSL, 10 Mbps ethernet, etc...) ?
> I'm a bit worried about what we might discover in this area I must
> confess (despite generally being mostly focused on 10+ Gbps).

One important point, I was looking for the other patch you pointed
in this long thread and finally found it :

> So
> http://git.kernel.org/cgit/linux/kernel/git/davem/net.git/commit/?id=98e09386c0ef4dfd48af7ba60ff908f0d525cdee
> 
> restored this minimal amount of buffering, and let the bigger amount for
> 40Gb NICs ;)

This one definitely restores original performance, so it's a much better
bet in my opinion :-)

Best regards,
Willy

^ permalink raw reply

* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Willy Tarreau @ 2013-11-20 17:38 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Arnaud Ebalard, Cong Wang, edumazet, linux-arm-kernel, netdev,
	Thomas Petazzoni
In-Reply-To: <1384968607.10637.14.camel@edumazet-glaptop2.roam.corp.google.com>

On Wed, Nov 20, 2013 at 09:30:07AM -0800, Eric Dumazet wrote:
> Well, all TCP performance results are highly dependent on the workload,
> and both receivers and senders behavior.
> 
> We made many improvements like TSO auto sizing, DRS (dynamic Right
> Sizing), and if the application used some specific settings (like
> SO_SNDBUF / SO_RCVBUF or other tweaks), we can not guarantee that same
> exact performance is reached from kernel version X to kernel version Y.

Of course, which is why I only care when there's a significant
difference. If I need 6 streams in a version and 8 in another one to
fill the wire, I call them identical. It's only when we dig into the
details that we analyse the differences.

> We try to make forward progress, there is little gain to revert all
> these great works. Linux had this tendency to favor throughput by using
> overly large skbs. Its time to do better.

I agree. Unfortunately our mails have crossed each other, so just to
keep this tread mostly linear, your next patch here :

   http://git.kernel.org/cgit/linux/kernel/git/davem/net.git/commit/?id=98e09386c0ef4dfd48af7ba60ff908f0d525cdee

Fixes that regression and the performance is back to normal which is
good.

> As explained, some drivers are buggy, and need fixes.

Agreed!

> If nobody wants to fix them, this really means no one is interested
> getting them fixed.

I was exactly reading the code when I found a window with your patch
above that I was looking for :-)

> I am willing to help if you provide details, because otherwise I need
> a crystal ball ;)
> 
> One known problem of TCP is the fact that an incoming ACK making room in
> socket write queue immediately wakeup a blocked thread (POLLOUT), even
> if only one MSS was ack, and write queue has 2MB of outstanding bytes.

Indeed.

> All these scheduling problems should be identified and fixed, and yes,
> this will require a dozen more patches.
> 
> max (128KB , 1-2 ms) of buffering per flow should be enough to reach
> line rate, even for a single flow, but this means the sk_sndbuf value
> for the socket must take into account the pipe size _plus_ 1ms of
> buffering.

Which is the purpose of your patch above and I confirm it fixes the
problem.

Now looking at how to workaround this lack of Tx IRQ.

Thanks!
Willy

^ permalink raw reply

* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Eric Dumazet @ 2013-11-20 17:40 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Thomas Petazzoni, netdev, Arnaud Ebalard, edumazet, Cong Wang,
	linux-arm-kernel
In-Reply-To: <20131120173436.GK8581@1wt.eu>

On Wed, 2013-11-20 at 18:34 +0100, Willy Tarreau wrote:

> One important point, I was looking for the other patch you pointed
> in this long thread and finally found it :
> 
> > So
> > http://git.kernel.org/cgit/linux/kernel/git/davem/net.git/commit/?id=98e09386c0ef4dfd48af7ba60ff908f0d525cdee
> > 
> > restored this minimal amount of buffering, and let the bigger amount for
> > 40Gb NICs ;)
> 
> This one definitely restores original performance, so it's a much better
> bet in my opinion :-)

I don't understand. I thought you were using this patch.

I guess we are spending time on a already solved problem.

^ permalink raw reply

* Re: [PATCH 1/1] Workaround for Suspend/Resume issue of AX88772B under ChromeOS
From: Grant Grundler @ 2013-11-20 17:42 UTC (permalink / raw)
  To: David Miller
  Cc: Freddy Xin, netdev, linux-usb, LKML, davemloft,
	ASIX Louis [蘇威陸], Allan Chou, Daniel_ASIX
In-Reply-To: <20131120.004026.2013972041095203460.davem@davemloft.net>

On Tue, Nov 19, 2013 at 9:40 PM, David Miller <davem@davemloft.net> wrote:
> 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.

Dave,
Sorry - I didn't think of that.  I agree the phy settings should be
reprograming in the phy.

The kernel state of the phy is not accurate in any case since the phy
was powered off and left powered off.
While this patch raises a new issue, can you apply this patch anyway
since in most cases (default settings) it improves "the user
experience" for most users?

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

AFAICT, the reset is required.

Since you've seen so many of these patches that do NOT reprogram the
phy on reset, would you consider a patch that added "reprogram phy" to
usbnet_resume() code path?

thanks!
grant

^ permalink raw reply

* [RFC net-next PATCH] macvtap: Add packet capture support
From: Vlad Yasevich @ 2013-11-20 18:04 UTC (permalink / raw)
  To: netdev; +Cc: mst, Vlad Yasevich

Currently it is impossible to capture traffic when using a macvtap
device.  The reason is that all capture handling is done either in
dev_hard_start_xmit() or in __netif_receive_skb_core().  Macvtap
currenlty doesn't use dev_hard_start_xmit(), and at the time the
packet traverses __netif_receive_skb_core, the skb->dev is set to
the lower-level device that doesn't end up matching macvtap.

To solve the issue, use dev_hard_start_xmit() on the output path.
On the input path, it is toughter to solve since macvtap ends up
consuming the skb so there is nothing more left for
__netif_receive_skb_core() to do.  A simple solution is to
pull the code that delivers things to the taps/captures into
its own function and allow macvtap to call it from its receive
routine.

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
This is only an RFC.  I'd like to solicit comments on this simple
approach.  A more complicated solution would have been to give
macvtap its own rx_handler and return RX_HANDLER_ANOTHER during
receive operation to make the packet go through another round of
capturing and hit the macvtap rx_handler.  I thought this would
hurt performance too much for no real gain.

Thanks
-vlad

 drivers/net/macvtap.c     |  4 +++-
 include/linux/netdevice.h |  2 ++
 net/core/dev.c            | 33 ++++++++++++++++++++++++++-------
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index dc76670..0ed8fae 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -334,6 +334,7 @@ drop:
  */
 static int macvtap_receive(struct sk_buff *skb)
 {
+	netif_receive_skb_taps(skb, true);
 	skb_push(skb, ETH_HLEN);
 	return macvtap_forward(skb->dev, skb);
 }
@@ -727,8 +728,9 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
 		skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
 	}
 	if (vlan) {
+		skb->dev = vlan->dev;
 		local_bh_disable();
-		macvlan_start_xmit(skb, vlan->dev);
+		dev_hard_start_xmit(skb, vlan->dev, NULL, NULL);
 		local_bh_enable();
 	} else {
 		kfree_skb(skb);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8b3de7c..84880eb 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2383,6 +2383,8 @@ void dev_kfree_skb_any(struct sk_buff *skb);
 int netif_rx(struct sk_buff *skb);
 int netif_rx_ni(struct sk_buff *skb);
 int netif_receive_skb(struct sk_buff *skb);
+struct packet_type *netif_receive_skb_taps(struct sk_buff *skb,
+					   bool last_deliver);
 gro_result_t napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb);
 void napi_gro_flush(struct napi_struct *napi, bool flush_old);
 struct sk_buff *napi_get_frags(struct napi_struct *napi);
diff --git a/net/core/dev.c b/net/core/dev.c
index da9c5e1..50f0ac4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3484,6 +3484,31 @@ static bool skb_pfmemalloc_protocol(struct sk_buff *skb)
 	}
 }
 
+struct packet_type *netif_receive_skb_taps(struct sk_buff *skb,
+					   bool last_deliver)
+{
+	struct packet_type *ptype, *pt_prev = NULL;
+	struct net_device *orig_dev;
+
+	if (list_empty(&ptype_all))
+		return NULL;
+
+	orig_dev = skb->dev;
+	list_for_each_entry_rcu(ptype, &ptype_all, list) {
+		if (!ptype->dev || ptype->dev == skb->dev) {
+			if (pt_prev)
+				deliver_skb(skb, pt_prev, orig_dev);
+			pt_prev = ptype;
+		}
+	}
+
+	if (last_deliver && pt_prev)
+		deliver_skb(skb, pt_prev, orig_dev);
+
+	return pt_prev;
+}
+EXPORT_SYMBOL_GPL(netif_receive_skb_taps);
+
 static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
 {
 	struct packet_type *ptype, *pt_prev;
@@ -3535,13 +3560,7 @@ another_round:
 	if (pfmemalloc)
 		goto skip_taps;
 
-	list_for_each_entry_rcu(ptype, &ptype_all, list) {
-		if (!ptype->dev || ptype->dev == skb->dev) {
-			if (pt_prev)
-				ret = deliver_skb(skb, pt_prev, orig_dev);
-			pt_prev = ptype;
-		}
-	}
+	pt_prev = netif_receive_skb_taps(skb, false);
 
 skip_taps:
 #ifdef CONFIG_NET_CLS_ACT
-- 
1.8.4.2

^ permalink raw reply related

* RE: [PATCH] rds: Error on offset mismatch if not loopback
From: Venkat Venkatsubra @ 2013-11-20 18:09 UTC (permalink / raw)
  To: Honggang LI, Josh Hunt; +Cc: David Miller, jjolly, LKML, netdev
In-Reply-To: <8744a6a4-d7a8-4e6f-8934-48a4fd4da0ce@default>



-----Original Message-----
From: Venkat Venkatsubra 
Sent: Tuesday, November 19, 2013 5:33 PM
To: Honggang LI; Josh Hunt
Cc: David Miller; jjolly@suse.com; LKML; netdev@vger.kernel.org
Subject: RE: [PATCH] rds: Error on offset mismatch if not loopback

We now have lot more information than we did before.
When sending a "congestion update" in rds_ib_xmit() we are now returning an incorrect number as bytes sent:

        BUG_ON(off % RDS_FRAG_SIZE);
        BUG_ON(hdr_off != 0 && hdr_off != sizeof(struct rds_header));

        /* Do not send cong updates to IB loopback */
        if (conn->c_loopback
            && rm->m_inc.i_hdr.h_flags & RDS_FLAG_CONG_BITMAP) {
                rds_cong_map_updated(conn->c_fcong, ~(u64) 0);
                scat = &rm->data.op_sg[sg];
                ret = sizeof(struct rds_header) + RDS_CONG_MAP_BYTES;
                ret = min_t(int, ret, scat->length - conn->c_xmit_data_off);
                return ret;
        }

It returns min(8240, 4096-0) i.e. 4096 bytes.
The caller rds_send_xmit() is made to think a partial message (4096 out of 8240) was sent.
It calls rds_ib_xmit() again with a data offset "off" of 4096-48 (rds header) (=4048 bytes). And we hit the BUG_ON.

The reason I didn't hit the panic on my test on Oracle UEK2 which is based on 2.6.39 kernel is it had it like this:
        BUG_ON(off % RDS_FRAG_SIZE);
        BUG_ON(hdr_off != 0 && hdr_off != sizeof(struct rds_header));

        /* Do not send cong updates to IB loopback */
        if (conn->c_loopback
            && rm->m_inc.i_hdr.h_flags & RDS_FLAG_CONG_BITMAP) {
                rds_cong_map_updated(conn->c_fcong, ~(u64) 0);
                return sizeof(struct rds_header) + RDS_CONG_MAP_BYTES;
        }
(So it wasn't 100% 2.6.39 ;-). )
It returned 8240 bytes. The caller rds_send_xmit decides the full message was sent (48 byte header + 4096 data + 4096 data).
And it worked.

Then I found this info on the change that was done upstream which now causes the panic:
http://marc.info/?l=linux-netdev&m=129908332903057
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=6094628bfd94323fc1cea05ec2c6affd98c18f7f 

Will investigate more into which problem the above change addressed.

Venkat
--

Looks like the fix pointed to by the above link is for a panic on a PPC system with a PAGE_SIZE of 64Kbytes.
I think the sequence it was going through before that fix was:
/* Do not send cong updates to IB loopback */
        if (conn->c_loopback
            && rm->m_inc.i_hdr.h_flags & RDS_FLAG_CONG_BITMAP) {
                rds_cong_map_updated(conn->c_fcong, ~(u64) 0);
                return sizeof(struct rds_header) + RDS_CONG_MAP_BYTES;
        }
rds_ib_xmit returns 8240
rds_send_xmit : c_xmit_data_off = 0 + 8240 - 48 (rds header the first time) = 8196
                c_xmit_data_off < 65536 (sg->length)
                calls rds_ib_xmit again
rds_ib_xmit returns 8240
rds_send_xmit: c_xmit_data_off = 8192+8240 = 16432 and calls rds_ib_xmit
rds_ib_xmit : returns 8240
rds_send_xmit: c_xmit_data_off 24672 and calls rds_ib_xmit
...
...
and so on till
rds_send_xmit: c_xmit_data_off 57632 and calls rds_ib_xmit
rds_ib_xmit: returns 8240

On the last iteration it hits the below BUG_ON in rds_send_xmit.
while (ret) {
    tmp = min_t(int, ret, sg->length -
                         conn->c_xmit_data_off);
[tmp = 7904]
    conn->c_xmit_data_off += tmp;
[c_xmit_data_off = 65536]
    ret -= tmp;
[ret = 8240-7904 = 336]
    if (conn->c_xmit_data_off == sg->length) {
         conn->c_xmit_data_off = 0;
         sg++;
         conn->c_xmit_sg++;
         BUG_ON(ret != 0 &&
             conn->c_xmit_sg == rm->data.op_nents);
    }
}

Since the congestion update over loopback is not actually transmitted as a message,
the multiple iterations we see in the case of ppc is unnecessary.
All that rds_ib_xmit needs to do is return a number of bytes that will tell the caller that
we are done with this message.
  
This might fix the original problem without introducing the current panic:
/* Do not send cong updates to IB loopback */
        if (conn->c_loopback
            && rm->m_inc.i_hdr.h_flags & RDS_FLAG_CONG_BITMAP) {
                rds_cong_map_updated(conn->c_fcong, ~(u64) 0);
                scat = &rm->data.op_sg[sg];
                ret = max_t(int, RDS_CONG_MAP_BYTES, scat->length);
                return ret + sizeof(struct rds_header);
        }
It will return 8240 when PAGE_SIZE is 4k and 64k+48 in case of ppc when scat->length is 64k and
be done with one iteration of rds_send_xmit/rds_ib_xmit loop.

Venkat

^ permalink raw reply

* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Willy Tarreau @ 2013-11-20 18:15 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Thomas Petazzoni, netdev, Arnaud Ebalard, edumazet, Cong Wang,
	linux-arm-kernel
In-Reply-To: <1384969222.10637.20.camel@edumazet-glaptop2.roam.corp.google.com>

On Wed, Nov 20, 2013 at 09:40:22AM -0800, Eric Dumazet wrote:
> On Wed, 2013-11-20 at 18:34 +0100, Willy Tarreau wrote:
> 
> > One important point, I was looking for the other patch you pointed
> > in this long thread and finally found it :
> > 
> > > So
> > > http://git.kernel.org/cgit/linux/kernel/git/davem/net.git/commit/?id=98e09386c0ef4dfd48af7ba60ff908f0d525cdee
> > > 
> > > restored this minimal amount of buffering, and let the bigger amount for
> > > 40Gb NICs ;)
> > 
> > This one definitely restores original performance, so it's a much better
> > bet in my opinion :-)
> 
> I don't understand. I thought you were using this patch.

No, I was on latest stable (3.10.19) which exhibits the regression but does
not yet have your fix above. Then I tested the patch your proposed in this
thread, then this latest one. Since the patch is not yet even in Linus'
tree, I'm not sure Arnaud has tried it yet.

> I guess we are spending time on a already solved problem.

That's possible indeed. Sorry if I was not clear enough, I tried.

Regards,
Willy

^ permalink raw reply

* Re: [RFC net-next PATCH] macvtap: Add packet capture support
From: Michael S. Tsirkin @ 2013-11-20 18:19 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <1384970649-29839-1-git-send-email-vyasevic@redhat.com>

On Wed, Nov 20, 2013 at 01:04:09PM -0500, Vlad Yasevich wrote:
> Currently it is impossible to capture traffic when using a macvtap
> device.  The reason is that all capture handling is done either in
> dev_hard_start_xmit() or in __netif_receive_skb_core().  Macvtap
> currenlty doesn't use dev_hard_start_xmit(), and at the time the
> packet traverses __netif_receive_skb_core, the skb->dev is set to
> the lower-level device that doesn't end up matching macvtap.
> 
> To solve the issue, use dev_hard_start_xmit() on the output path.
> On the input path, it is toughter to solve since macvtap ends up
> consuming the skb so there is nothing more left for
> __netif_receive_skb_core() to do.  A simple solution is to
> pull the code that delivers things to the taps/captures into
> its own function and allow macvtap to call it from its receive
> routine.
> 
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
> This is only an RFC.  I'd like to solicit comments on this simple
> approach.

I'm kind of worried about this. What worries me is that normally
if you have a packet socket bound to all interfaces, what it shows is
traffic to/from the box.

This might be a bug for someone, but I suspect at this point this
is part of the ABI.

But macvtap bypasses most of the host networking stack,
So I worry that suddenly showing these packets would be confusing.

Assuming we want to show this traffic, I think it's preferable to
-		if (!ptype->dev || ptype->dev == skb->dev) {
+		if (ptype->dev == skb->dev) {
so you have to bind to macvtap explicitly to see the traffic.


Of course when you start binding to specific macvtaps
it doesn't scale well because we'll have a long list
on ptype_all suddenly.
This might mean we need to keep this list per-device ...

> A more complicated solution would have been to give
> macvtap its own rx_handler and return RX_HANDLER_ANOTHER during
> receive operation to make the packet go through another round of
> capturing and hit the macvtap rx_handler.  I thought this would
> hurt performance too much for no real gain.
> 
> Thanks
> -vlad
> 
>  drivers/net/macvtap.c     |  4 +++-
>  include/linux/netdevice.h |  2 ++
>  net/core/dev.c            | 33 ++++++++++++++++++++++++++-------
>  3 files changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index dc76670..0ed8fae 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -334,6 +334,7 @@ drop:
>   */
>  static int macvtap_receive(struct sk_buff *skb)
>  {
> +	netif_receive_skb_taps(skb, true);
>  	skb_push(skb, ETH_HLEN);
>  	return macvtap_forward(skb->dev, skb);
>  }
> @@ -727,8 +728,9 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
>  		skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
>  	}
>  	if (vlan) {
> +		skb->dev = vlan->dev;
>  		local_bh_disable();
> -		macvlan_start_xmit(skb, vlan->dev);
> +		dev_hard_start_xmit(skb, vlan->dev, NULL, NULL);
>  		local_bh_enable();
>  	} else {
>  		kfree_skb(skb);
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 8b3de7c..84880eb 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2383,6 +2383,8 @@ void dev_kfree_skb_any(struct sk_buff *skb);
>  int netif_rx(struct sk_buff *skb);
>  int netif_rx_ni(struct sk_buff *skb);
>  int netif_receive_skb(struct sk_buff *skb);
> +struct packet_type *netif_receive_skb_taps(struct sk_buff *skb,
> +					   bool last_deliver);
>  gro_result_t napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb);
>  void napi_gro_flush(struct napi_struct *napi, bool flush_old);
>  struct sk_buff *napi_get_frags(struct napi_struct *napi);
> diff --git a/net/core/dev.c b/net/core/dev.c
> index da9c5e1..50f0ac4 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3484,6 +3484,31 @@ static bool skb_pfmemalloc_protocol(struct sk_buff *skb)
>  	}
>  }
>  
> +struct packet_type *netif_receive_skb_taps(struct sk_buff *skb,
> +					   bool last_deliver)
> +{
> +	struct packet_type *ptype, *pt_prev = NULL;
> +	struct net_device *orig_dev;
> +
> +	if (list_empty(&ptype_all))
> +		return NULL;
> +
> +	orig_dev = skb->dev;
> +	list_for_each_entry_rcu(ptype, &ptype_all, list) {
> +		if (!ptype->dev || ptype->dev == skb->dev) {
> +			if (pt_prev)
> +				deliver_skb(skb, pt_prev, orig_dev);
> +			pt_prev = ptype;
> +		}
> +	}
> +
> +	if (last_deliver && pt_prev)
> +		deliver_skb(skb, pt_prev, orig_dev);
> +
> +	return pt_prev;
> +}
> +EXPORT_SYMBOL_GPL(netif_receive_skb_taps);
> +
>  static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
>  {
>  	struct packet_type *ptype, *pt_prev;
> @@ -3535,13 +3560,7 @@ another_round:
>  	if (pfmemalloc)
>  		goto skip_taps;
>  
> -	list_for_each_entry_rcu(ptype, &ptype_all, list) {
> -		if (!ptype->dev || ptype->dev == skb->dev) {
> -			if (pt_prev)
> -				ret = deliver_skb(skb, pt_prev, orig_dev);
> -			pt_prev = ptype;
> -		}
> -	}
> +	pt_prev = netif_receive_skb_taps(skb, false);
>  
>  skip_taps:
>  #ifdef CONFIG_NET_CLS_ACT
> -- 
> 1.8.4.2

^ permalink raw reply

* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Eric Dumazet @ 2013-11-20 18:21 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Thomas Petazzoni, netdev, Arnaud Ebalard, edumazet, Cong Wang,
	linux-arm-kernel
In-Reply-To: <20131120181532.GM8581@1wt.eu>

On Wed, 2013-11-20 at 19:15 +0100, Willy Tarreau wrote:

> No, I was on latest stable (3.10.19) which exhibits the regression but does
> not yet have your fix above. Then I tested the patch your proposed in this
> thread, then this latest one. Since the patch is not yet even in Linus'
> tree, I'm not sure Arnaud has tried it yet.

Oh right ;)

BTW Linus tree has the fix, I just checked this now.

(Linus got David changes 19 hours ago)

^ permalink raw reply

* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Willy Tarreau @ 2013-11-20 18:29 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Thomas Petazzoni, netdev, Arnaud Ebalard, edumazet, Cong Wang,
	linux-arm-kernel
In-Reply-To: <1384971684.10637.22.camel@edumazet-glaptop2.roam.corp.google.com>

On Wed, Nov 20, 2013 at 10:21:24AM -0800, Eric Dumazet wrote:
> On Wed, 2013-11-20 at 19:15 +0100, Willy Tarreau wrote:
> 
> > No, I was on latest stable (3.10.19) which exhibits the regression but does
> > not yet have your fix above. Then I tested the patch your proposed in this
> > thread, then this latest one. Since the patch is not yet even in Linus'
> > tree, I'm not sure Arnaud has tried it yet.
> 
> Oh right ;)
> 
> BTW Linus tree has the fix, I just checked this now.
> 
> (Linus got David changes 19 hours ago)

Ah yes I didn't look far enough back.

Thanks,
Willy

^ permalink raw reply

* loop on reset adapter with ixgbe
From: William Dauchy @ 2013-11-20 18:25 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: Jacob Keller, John Fastabend, Emil Tantilov, Leonardo Potenza

[-- Attachment #1: Type: text/plain, Size: 836 bytes --]

Hi,

On some machines with a network card named
"Intel Corporation 82599EB 10-Gigabit SFI/SFP+"

At boot time, I'm getting an infinite loop showing these messages:

ixgbe 0000:03:00.0 eth0: initiating reset to clear Tx work after link loss
ixgbe 0000:03:00.0 eth0: Reset adapter
ixgbe 0000:03:00.0 eth0: detected SFP+: 3

ethtool info:
driver: ixgbe
version: 3.8.21-k
firmware-version: 0x00000000

I'm using the last linux stable branch v3.10.x.

I need to unplug/plug the network/cable (a twinax cable) several times to
stabilize the situation and get some network.
I was wondering if I should consider this as a driver/firmware bug or if I
should look for a physical issue (I changed the physical cable several times).
Or do you need some more info to help understand this issue?

Best regards,
-- 
William

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.
From: Sarah Sharp @ 2013-11-20 18:39 UTC (permalink / raw)
  To: David Laight
  Cc: Alan Stern, Ben Hutchings, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B743D-CgBM+Bx2aUAnGFn1LkZF6NBPR1lH4CV8@public.gmane.org>

On Wed, Nov 20, 2013 at 05:16:12PM -0000, David Laight wrote:
> > Ok, so the networking layer should be fine.  However, with the current
> > patch, if the mass storage driver sends down a scatter-gather list
> > that's bigger than a ring segment, or needs to be split up so it doesn't
> > cross 64K boundaries, then the URB submission will fail.  We don't want
> > that to happen.
> 
> My suspicion is that long SG lists are unusual - otherwise the
> ring expansion code would have been needed much earlier.
> 
> Can anyone remember whether that was needed because of long SG lists
> or because of large numbers of outstanding requests?
>
> I've seen it for network cards - but only because usbnet sends
> down far too many tx buffers.

It was added because USB video and audio drivers queue isochronous URBs
with lots of buffers for each service interval.  So basically, lots of
outstanding requests, not long SG lists was the driving motivation to
add the ring expansion.

Sarah Sharp
--
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: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: David Miller @ 2013-11-20 18:52 UTC (permalink / raw)
  To: eric.dumazet
  Cc: w, arno, xiyou.wangcong, edumazet, linux-arm-kernel, netdev,
	thomas.petazzoni
In-Reply-To: <1384968607.10637.14.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 20 Nov 2013 09:30:07 -0800

> max (128KB , 1-2 ms) of buffering per flow should be enough to reach
> line rate, even for a single flow, but this means the sk_sndbuf value
> for the socket must take into account the pipe size _plus_ 1ms of
> buffering.

And we can implement this using the estimated pacing rate.

^ permalink raw reply

* RE: loop on reset adapter with ixgbe
From: Skidmore, Donald C @ 2013-11-20 18:53 UTC (permalink / raw)
  To: William Dauchy, netdev@vger.kernel.org
  Cc: Keller, Jacob E, Fastabend, John R, Tantilov, Emil S,
	Potenza, Leonardo
In-Reply-To: <20131120182524.GC16953@gandi.net>

Hey William,

Thanks for letting know what you're running into, however I don't remember ever seeing a failure quite like this.  I'm wondering if it is something we might have fixed in later versions of the driver.  By any chance could you see if you get the same failure with the latest source forge driver?  It will make it easier to track down if we are looking for something we already fix or a new problem altogether.

Thanks,
-Don Skidmore <donald.c.skidmore@intel.com>

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of William Dauchy
> Sent: Wednesday, November 20, 2013 10:25 AM
> To: netdev@vger.kernel.org
> Cc: Keller, Jacob E; Fastabend, John R; Tantilov, Emil S; Potenza, Leonardo
> Subject: loop on reset adapter with ixgbe
> 
> Hi,
> 
> On some machines with a network card named "Intel Corporation 82599EB
> 10-Gigabit SFI/SFP+"
> 
> At boot time, I'm getting an infinite loop showing these messages:
> 
> ixgbe 0000:03:00.0 eth0: initiating reset to clear Tx work after link loss ixgbe
> 0000:03:00.0 eth0: Reset adapter ixgbe 0000:03:00.0 eth0: detected SFP+: 3
> 
> ethtool info:
> driver: ixgbe
> version: 3.8.21-k
> firmware-version: 0x00000000
> 
> I'm using the last linux stable branch v3.10.x.
> 
> I need to unplug/plug the network/cable (a twinax cable) several times to
> stabilize the situation and get some network.
> I was wondering if I should consider this as a driver/firmware bug or if I
> should look for a physical issue (I changed the physical cable several times).
> Or do you need some more info to help understand this issue?
> 
> Best regards,
> --
> William

^ permalink raw reply

* Re: [PATCH 1/1] Workaround for Suspend/Resume issue of AX88772B under ChromeOS
From: David Miller @ 2013-11-20 18:54 UTC (permalink / raw)
  To: grundler
  Cc: freddy, netdev, linux-usb, linux-kernel, davemloft, louis, allan,
	daniel
In-Reply-To: <CANEJEGtBmvFNcFzOW9ZnOFd=ZMnz82ajcN52YqW3+n0+=rhxBA@mail.gmail.com>

From: Grant Grundler <grundler@google.com>
Date: Wed, 20 Nov 2013 09:42:42 -0800

> While this patch raises a new issue, can you apply this patch anyway
> since in most cases (default settings) it improves "the user
> experience" for most users?

I'm not applying a patch which knowingly is not implemented correctly.

^ permalink raw reply

* Re: [PATCH] rds: Error on offset mismatch if not loopback
From: David Miller @ 2013-11-20 18:54 UTC (permalink / raw)
  To: venkat.x.venkatsubra; +Cc: honli, joshhunt00, jjolly, linux-kernel, netdev
In-Reply-To: <25bef391-380b-486a-b06f-8be210e8abb7@default>


Why are you posting this message a second time?

^ permalink raw reply

* RE: loop on reset adapter with ixgbe
From: Skidmore, Donald C @ 2013-11-20 18:58 UTC (permalink / raw)
  To: William Dauchy, netdev@vger.kernel.org
  Cc: Keller, Jacob E, Fastabend, John R, Tantilov, Emil S,
	Potenza, Leonardo
In-Reply-To: <20131120182524.GC16953@gandi.net>



> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of William Dauchy
> Sent: Wednesday, November 20, 2013 10:25 AM
> To: netdev@vger.kernel.org
> Cc: Keller, Jacob E; Fastabend, John R; Tantilov, Emil S; Potenza, Leonardo
> Subject: loop on reset adapter with ixgbe
> 
> Hi,
> 
> On some machines with a network card named "Intel Corporation 82599EB
> 10-Gigabit SFI/SFP+"
> 
> At boot time, I'm getting an infinite loop showing these messages:
> 
> ixgbe 0000:03:00.0 eth0: initiating reset to clear Tx work after link loss ixgbe
> 0000:03:00.0 eth0: Reset adapter ixgbe 0000:03:00.0 eth0: detected SFP+: 3
> 
> ethtool info:
> driver: ixgbe
> version: 3.8.21-k
> firmware-version: 0x00000000
> 
> I'm using the last linux stable branch v3.10.x.
> 
> I need to unplug/plug the network/cable (a twinax cable) several times to
> stabilize the situation and get some network.
> I was wondering if I should consider this as a driver/firmware bug or if I
> should look for a physical issue (I changed the physical cable several times).
> Or do you need some more info to help understand this issue?
> 
> Best regards,
> --
> William

I also just noticed your firmware looks in correct.  What is the DevID for your device?  Likewise do you have another NIC and if so could you check it's firmware-version?

Thanks,
-Don Skidmore <donald.c.skidmore@intel.com>

^ permalink raw reply

* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Willy Tarreau @ 2013-11-20 19:11 UTC (permalink / raw)
  To: Arnaud Ebalard
  Cc: Thomas Petazzoni, Florian Fainelli, simon.guinot, Eric Dumazet,
	netdev, edumazet, Cong Wang, linux-arm-kernel
In-Reply-To: <874n780wzc.fsf@natisbad.org>

Hi Arnaud,

first, thanks for all these tests.

On Wed, Nov 20, 2013 at 12:53:43AM +0100, Arnaud Ebalard wrote:
(...)
> In the end, here are the conclusions *I* draw from this test session,
> do not hesitate to correct me:
> 
>  - Eric, it seems something changed in linus tree betwen the beginning
>    of the thread and now, which somehow reduces the effect of the
>    regression we were seen: I never got back the 256KB/s.
>  - You revert patch still improves the perf a lot
>  - It seems reducing MVNETA_TX_DONE_TIMER_PERIOD does not help
>  - w/ your revert patch, I can confirm that mvneta driver is capable of
>    doing line rate w/ proper tweak of TCP send window (256KB instead of
>    4M)
>  - It seems I will I have to spend some time on the SATA issues I
>    previously thought were an artefact of not cleaning my tree during a
>    debug session [1], i.e. there is IMHO an issue.

Could you please try Eric's patch that was just merged into Linus' tree
if it was not yet in the kernel you tried :

  98e09386c0e  tcp: tsq: restore minimal amount of queueing

For me it restored the original performance (I saturate the Gbps with
about 7 concurrent streams).

Further, I wrote the small patch below for mvneta. I'm not sure it's
smp-safe but it's a PoC. In mvneta_poll() which currently is only called
upon Rx interrupt, it tries to flush all possible remaining Tx descriptors
if any. That significantly improved my transfer rate, now I easily achieve
1 Gbps using a single TCP stream on the mirabox. Not tried on the AX3 yet.

It also increased the overall connection rate by 10% on empty HTTP responses
(small packets), very likely by reducing the dead time between some segments!

You'll probably want to give it a try, so here it comes.

Cheers,
Willy

>From d1a00e593841223c7d871007b1e1fc528afe8e4d Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
Date: Wed, 20 Nov 2013 19:47:11 +0100
Subject: EXP: net: mvneta: try to flush Tx descriptor queue upon Rx
 interrupts

Right now the mvneta driver doesn't handle Tx IRQ, and solely relies on a
timer to flush Tx descriptors. This causes jerky output traffic with bursts
and pauses, making it difficult to reach line rate with very few streams.
This patch tries to improve the situation which is complicated by the lack
of public datasheet from Marvell. The workaround consists in trying to flush
pending buffers during the Rx polling. The idea is that for symmetric TCP
traffic, ACKs received in response to the packets sent will trigger the Rx
interrupt and will anticipate the flushing of the descriptors.

The results are quite good, a single TCP stream is now capable of saturating
a gigabit.

This is only a workaround, it doesn't address asymmetric traffic nor datagram
based traffic.

Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 drivers/net/ethernet/marvell/mvneta.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 5aed8ed..59e1c86 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -2013,6 +2013,26 @@ static int mvneta_poll(struct napi_struct *napi, int budget)
 	}
 
 	pp->cause_rx_tx = cause_rx_tx;
+
+	/* Try to flush pending Tx buffers if any */
+	if (test_bit(MVNETA_F_TX_DONE_TIMER_BIT, &pp->flags)) {
+		int tx_todo = 0;
+
+		mvneta_tx_done_gbe(pp,
+	                           (((1 << txq_number) - 1) &
+	                           MVNETA_CAUSE_TXQ_SENT_DESC_ALL_MASK),
+	                           &tx_todo);
+
+		if (tx_todo > 0) {
+			mod_timer(&pp->tx_done_timer,
+			          jiffies + msecs_to_jiffies(MVNETA_TX_DONE_TIMER_PERIOD));
+		}
+		else {
+			clear_bit(MVNETA_F_TX_DONE_TIMER_BIT, &pp->flags);
+			del_timer(&pp->tx_done_timer);
+		}
+	}
+
 	return rx_done;
 }
 
-- 
1.7.12.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox