* Re: [PATCH V5 2/6 net-next] netdevice.h: Add zero-copy flag in netdevice
From: Michael S. Tsirkin @ 2011-05-18 16:36 UTC (permalink / raw)
To: Shirley Ma
Cc: Michał Mirosław, Ben Hutchings, David Miller,
Eric Dumazet, Avi Kivity, Arnd Bergmann, netdev, kvm,
linux-kernel
In-Reply-To: <1305734857.32080.53.camel@localhost.localdomain>
On Wed, May 18, 2011 at 09:07:37AM -0700, Shirley Ma wrote:
> On Wed, 2011-05-18 at 18:47 +0300, Michael S. Tsirkin wrote:
> > On Wed, May 18, 2011 at 07:38:27AM -0700, Shirley Ma wrote:
> > > On Wed, 2011-05-18 at 13:40 +0200, Michał Mirosław wrote:
> > > > >> >> Not more other restrictions, skb clone is OK.
> > pskb_expand_head()
> > > > looks
> > > > >> >> OK to me from code review.
> > > > >> > Hmm. pskb_expand_head calls skb_release_data while keeping
> > > > >> > references to pages. How is that ok? What do I miss?
> > > > >> It's making copy of the skb_shinfo earlier, so the pages
> > refcount
> > > > >> stays the same.
> > > > > Exactly. But the callback is invoked so the guest thinks it's ok
> > to
> > > > > change this memory. If it does a corrupted packet will be sent
> > out.
> > > >
> > > > Hmm. I tool a quick look at skb_clone(), and it looks like this
> > > > sequence will break this scheme:
> > > >
> > > > skb2 = skb_clone(skb...);
> > > > kfree_skb(skb) or pskb_expand_head(skb); /* callback called */
> > > > [use skb2, pages still referenced]
> > > > kfree_skb(skb); /* callback called again */
> > > >
> > > > This sequence is common in bridge, might be in other places.
> > > >
> > > > Maybe this ubuf thing should just track clones? This will make it
> > work
> > > > on all devices then.
> > >
> > > The callback was only invoked when last reference of skb was gone.
> > > skb_clone does increase skb refcnt. I tested tcpdump on lower
> > device, it
> > > worked.
> >
> > Right, it will normally work, but two issues I think you miss:
> > 1. malicious guest can change the memory between when it is sent out
> > by
> > device and consumed by tcpdump, so you will see different things
> > (not sure how important this is).
> > 2. if tcpdump stops consuming stuff from the packet socket (it's
> > userspace, can't be trusted) then we won't get a callback for
> > page potentially forever, guest networking will get blocked etc.
> > > For the sequence of:
> > >
> > > skb_clone -> last refcnt + 1
> > > kfree_skb() or pskb_expand_head -> callback not called
> > > kfree_skb() -> callback called
> > >
> > > I will check page refcount to see whether it's balanced.
> > >
> > > Thanks
> > > shirley
> >
> >
> > pskb_expand_head is a problem anyway I think as it
> > can hang on to pages after it calls release_data.
> > Then guest will modify these pages and you get trash there.
>
> This can be avoid by allowing pskb_expand_head in fastpath only, I
> think. But not sure whether tcpdump can still work with this.
>
> Thanks
> Shirley
Yes, I agree. I think for tcpdump, we really need to copy the data
anyway, to avoid guest changing it in between. So we do that and then
use the copy everywhere, release the old one. Hmm?
--
MST
^ permalink raw reply
* Re: [PATCH V5 2/6 net-next] netdevice.h: Add zero-copy flag in netdevice
From: Shirley Ma @ 2011-05-18 16:45 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Michał Mirosław, Ben Hutchings, David Miller,
Eric Dumazet, Avi Kivity, Arnd Bergmann, netdev, kvm,
linux-kernel
In-Reply-To: <20110518163633.GB22001@redhat.com>
On Wed, 2011-05-18 at 19:36 +0300, Michael S. Tsirkin wrote:
> On Wed, May 18, 2011 at 09:07:37AM -0700, Shirley Ma wrote:
> > On Wed, 2011-05-18 at 18:47 +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 18, 2011 at 07:38:27AM -0700, Shirley Ma wrote:
> > > > On Wed, 2011-05-18 at 13:40 +0200, Michał Mirosław wrote:
> > > > > >> >> Not more other restrictions, skb clone is OK.
> > > pskb_expand_head()
> > > > > looks
> > > > > >> >> OK to me from code review.
> > > > > >> > Hmm. pskb_expand_head calls skb_release_data while
> keeping
> > > > > >> > references to pages. How is that ok? What do I miss?
> > > > > >> It's making copy of the skb_shinfo earlier, so the pages
> > > refcount
> > > > > >> stays the same.
> > > > > > Exactly. But the callback is invoked so the guest thinks
> it's ok
> > > to
> > > > > > change this memory. If it does a corrupted packet will be
> sent
> > > out.
> > > > >
> > > > > Hmm. I tool a quick look at skb_clone(), and it looks like
> this
> > > > > sequence will break this scheme:
> > > > >
> > > > > skb2 = skb_clone(skb...);
> > > > > kfree_skb(skb) or pskb_expand_head(skb); /* callback called
> */
> > > > > [use skb2, pages still referenced]
> > > > > kfree_skb(skb); /* callback called again */
> > > > >
> > > > > This sequence is common in bridge, might be in other places.
> > > > >
> > > > > Maybe this ubuf thing should just track clones? This will make
> it
> > > work
> > > > > on all devices then.
> > > >
> > > > The callback was only invoked when last reference of skb was
> gone.
> > > > skb_clone does increase skb refcnt. I tested tcpdump on lower
> > > device, it
> > > > worked.
> > >
> > > Right, it will normally work, but two issues I think you miss:
> > > 1. malicious guest can change the memory between when it is sent
> out
> > > by
> > > device and consumed by tcpdump, so you will see different
> things
> > > (not sure how important this is).
> > > 2. if tcpdump stops consuming stuff from the packet socket (it's
> > > userspace, can't be trusted) then we won't get a callback for
> > > page potentially forever, guest networking will get blocked
> etc.
> > > > For the sequence of:
> > > >
> > > > skb_clone -> last refcnt + 1
> > > > kfree_skb() or pskb_expand_head -> callback not called
> > > > kfree_skb() -> callback called
> > > >
> > > > I will check page refcount to see whether it's balanced.
> > > >
> > > > Thanks
> > > > shirley
> > >
> > >
> > > pskb_expand_head is a problem anyway I think as it
> > > can hang on to pages after it calls release_data.
> > > Then guest will modify these pages and you get trash there.
> >
> > This can be avoid by allowing pskb_expand_head in fastpath only, I
> > think. But not sure whether tcpdump can still work with this.
> >
> > Thanks
> > Shirley
>
> Yes, I agree. I think for tcpdump, we really need to copy the data
> anyway, to avoid guest changing it in between. So we do that and then
> use the copy everywhere, release the old one. Hmm?
Yes. Old one use zerocopy, new one use copy data.
Thanks
Shirley
^ permalink raw reply
* Re: [PATCH V5 2/6 net-next] netdevice.h: Add zero-copy flag in netdevice
From: Michael S. Tsirkin @ 2011-05-18 16:50 UTC (permalink / raw)
To: Michał Mirosław
Cc: Shirley Ma, Ben Hutchings, David Miller, Eric Dumazet, Avi Kivity,
Arnd Bergmann, netdev, kvm, linux-kernel
In-Reply-To: <BANLkTi=g_PFfS5653K8ZoQ2Jhp8DhCV1hg@mail.gmail.com>
On Wed, May 18, 2011 at 01:40:29PM +0200, Michał Mirosław wrote:
> W dniu 18 maja 2011 13:17 użytkownik Michael S. Tsirkin
> <mst@redhat.com> napisał:
> > On Wed, May 18, 2011 at 01:10:50PM +0200, Michał Mirosław wrote:
> >> 2011/5/18 Michael S. Tsirkin <mst@redhat.com>:
> >> > On Tue, May 17, 2011 at 03:28:38PM -0700, Shirley Ma wrote:
> >> >> On Tue, 2011-05-17 at 23:48 +0200, Michał Mirosław wrote:
> >> >> > 2011/5/17 Shirley Ma <mashirle@us.ibm.com>:
> >> >> > > Hello Michael,
> >> >> > >
> >> >> > > Looks like to use a new flag requires more time/work. I am thinking
> >> >> > > whether we can just use HIGHDMA flag to enable zero-copy in macvtap
> >> >> > to
> >> >> > > avoid the new flag for now since mavctap uses real NICs as lower
> >> >> > device?
> >> >> >
> >> >> > Is there any other restriction besides requiring driver to not recycle
> >> >> > the skb? Are there any drivers that recycle TX skbs?
> >> >
> >> > Not just recycling skbs, keeping reference to any of the pages in the
> >> > skb. Another requirement is to invoke the callback
> >> > in a timely fashion. For example virtio-net doesn't limit the time until
> >> > that happens (skbs are only freed when some other packet is
> >> > transmitted), so we need to avoid zcopy for such (nested-virt)
> >> > scenarious, right?
> >>
> >> Hmm. But every hardware driver supporting SG will keep reference to
> >> the pages until the packet is sent (or DMA'd to the device). This can
> >> take a long time if hardware queue happens to stall for some reason.
> >
> > That's a fundamental property of zero copy transmit.
> > You can't let the application/guest reuse the memory until
> > no one looks at it anymore.
> >
> >> Is it that you mean keeping a reference after all skbs pointing to the
> >> pages are released?
> > No one should reference the pages after the callback is invoked, yes.
>
> >> >> Not more other restrictions, skb clone is OK. pskb_expand_head() looks
> >> >> OK to me from code review.
> >> > Hmm. pskb_expand_head calls skb_release_data while keeping
> >> > references to pages. How is that ok? What do I miss?
> >> It's making copy of the skb_shinfo earlier, so the pages refcount
> >> stays the same.
> > Exactly. But the callback is invoked so the guest thinks it's ok to
> > change this memory. If it does a corrupted packet will be sent out.
>
> Hmm. I tool a quick look at skb_clone(), and it looks like this
> sequence will break this scheme:
>
> skb2 = skb_clone(skb...);
> kfree_skb(skb) or pskb_expand_head(skb); /* callback called */
> [use skb2, pages still referenced]
> kfree_skb(skb); /* callback called again */
> This sequence is common in bridge, might be in other places.
>
> Maybe this ubuf thing should just track clones? This will make it work
> on all devices then.
>
> Best Regards,
> Michał Mirosław
Well bridge has the problem that packet might get anywhere and it's
really hard to track. Same for tun - it can get queued forever.
veth, loopback are all a problem I think.
IOW we really want to limit this to real physical NICs
which mostly all DTRT. Whitelisting them with a new flag
is likely the most concervative approach, no?
--
MST
^ permalink raw reply
* Re: [PATCH V5 2/6 net-next] netdevice.h: Add zero-copy flag in netdevice
From: Michael S. Tsirkin @ 2011-05-18 16:51 UTC (permalink / raw)
To: Shirley Ma
Cc: Michał Mirosław, Ben Hutchings, David Miller,
Eric Dumazet, Avi Kivity, Arnd Bergmann, netdev, kvm,
linux-kernel
In-Reply-To: <1305737140.32080.59.camel@localhost.localdomain>
On Wed, May 18, 2011 at 09:45:40AM -0700, Shirley Ma wrote:
> On Wed, 2011-05-18 at 19:36 +0300, Michael S. Tsirkin wrote:
> > On Wed, May 18, 2011 at 09:07:37AM -0700, Shirley Ma wrote:
> > > On Wed, 2011-05-18 at 18:47 +0300, Michael S. Tsirkin wrote:
> > > > On Wed, May 18, 2011 at 07:38:27AM -0700, Shirley Ma wrote:
> > > > > On Wed, 2011-05-18 at 13:40 +0200, Michał Mirosław wrote:
> > > > > > >> >> Not more other restrictions, skb clone is OK.
> > > > pskb_expand_head()
> > > > > > looks
> > > > > > >> >> OK to me from code review.
> > > > > > >> > Hmm. pskb_expand_head calls skb_release_data while
> > keeping
> > > > > > >> > references to pages. How is that ok? What do I miss?
> > > > > > >> It's making copy of the skb_shinfo earlier, so the pages
> > > > refcount
> > > > > > >> stays the same.
> > > > > > > Exactly. But the callback is invoked so the guest thinks
> > it's ok
> > > > to
> > > > > > > change this memory. If it does a corrupted packet will be
> > sent
> > > > out.
> > > > > >
> > > > > > Hmm. I tool a quick look at skb_clone(), and it looks like
> > this
> > > > > > sequence will break this scheme:
> > > > > >
> > > > > > skb2 = skb_clone(skb...);
> > > > > > kfree_skb(skb) or pskb_expand_head(skb); /* callback called
> > */
> > > > > > [use skb2, pages still referenced]
> > > > > > kfree_skb(skb); /* callback called again */
> > > > > >
> > > > > > This sequence is common in bridge, might be in other places.
> > > > > >
> > > > > > Maybe this ubuf thing should just track clones? This will make
> > it
> > > > work
> > > > > > on all devices then.
> > > > >
> > > > > The callback was only invoked when last reference of skb was
> > gone.
> > > > > skb_clone does increase skb refcnt. I tested tcpdump on lower
> > > > device, it
> > > > > worked.
> > > >
> > > > Right, it will normally work, but two issues I think you miss:
> > > > 1. malicious guest can change the memory between when it is sent
> > out
> > > > by
> > > > device and consumed by tcpdump, so you will see different
> > things
> > > > (not sure how important this is).
> > > > 2. if tcpdump stops consuming stuff from the packet socket (it's
> > > > userspace, can't be trusted) then we won't get a callback for
> > > > page potentially forever, guest networking will get blocked
> > etc.
> > > > > For the sequence of:
> > > > >
> > > > > skb_clone -> last refcnt + 1
> > > > > kfree_skb() or pskb_expand_head -> callback not called
> > > > > kfree_skb() -> callback called
> > > > >
> > > > > I will check page refcount to see whether it's balanced.
> > > > >
> > > > > Thanks
> > > > > shirley
> > > >
> > > >
> > > > pskb_expand_head is a problem anyway I think as it
> > > > can hang on to pages after it calls release_data.
> > > > Then guest will modify these pages and you get trash there.
> > >
> > > This can be avoid by allowing pskb_expand_head in fastpath only, I
> > > think. But not sure whether tcpdump can still work with this.
> > >
> > > Thanks
> > > Shirley
> >
> > Yes, I agree. I think for tcpdump, we really need to copy the data
> > anyway, to avoid guest changing it in between. So we do that and then
> > use the copy everywhere, release the old one. Hmm?
>
> Yes. Old one use zerocopy, new one use copy data.
>
> Thanks
> Shirley
No, that's wrong, as they might become different with a
malicious guest. As long as we copied already, lets realease
the data and have everyone use the copy.
--
MST
^ permalink raw reply
* Re: [PATCH V5 2/6 net-next] netdevice.h: Add zero-copy flag in netdevice
From: Shirley Ma @ 2011-05-18 17:00 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Michał Mirosław, Ben Hutchings, David Miller,
Eric Dumazet, Avi Kivity, Arnd Bergmann, netdev, kvm,
linux-kernel
In-Reply-To: <20110518165138.GD22001@redhat.com>
On Wed, 2011-05-18 at 19:51 +0300, Michael S. Tsirkin wrote:
> > > Yes, I agree. I think for tcpdump, we really need to copy the
> data
> > > anyway, to avoid guest changing it in between. So we do that and
> then
> > > use the copy everywhere, release the old one. Hmm?
> >
> > Yes. Old one use zerocopy, new one use copy data.
> >
> > Thanks
> > Shirley
>
> No, that's wrong, as they might become different with a
> malicious guest. As long as we copied already, lets realease
> the data and have everyone use the copy.
Ok, I will patch pskb_expand_head to test it out.
Shirley
^ permalink raw reply
* RE: [PATCH v2] CDC NCM: release interfaces fix in unbind()
From: Alexey ORISHKO @ 2011-05-18 17:11 UTC (permalink / raw)
To: Alan Stern, gregkh@suse.de, oliver@neukum.org
Cc: netdev@vger.kernel.org, davem@davemloft.net,
linux-usb@vger.kernel.org
In-Reply-To: <Pine.LNX.4.44L0.1105181053110.2131-100000@iolanthe.rowland.org>
> From: Alan Stern [mailto:stern@rowland.harvard.edu]
> Sent: Wednesday, May 18, 2011 4:55 PM
>
> Here and later on, the patch seems to have forgotten about the control
> interface. Is this deliberate or an oversight?
>
> Alan Stern
Kernel docs says, that usb_driver_claim_interface() is used by usb
device drivers that need to claim more than one interface. I assume,
it's needed for second interface only. Am I wrong?
NCM driver was partly based on ECM code. I did check existing drivers
for CDC ACM and CDC ECM, which also uses 2 interfaces: master (control)
and data (bulk), but I was unable to find any code, which claims master
interface.
If we need explicitly claim/release master interface (which is *intf
parameter in bind() and unbind()), then cdc-acm and usb_ether drivers
should be updated as well.
I wonder, if Greg or Oliver could provide any comments why master interface
is not claimed in modem/ether drivers, since they are working with the
code for quite a while.
Regards,
Alexey
^ permalink raw reply
* Re: [PATCH] ethtool: ETHTOOL_SFEATURES: remove NETIF_F_COMPAT return
From: Ben Hutchings @ 2011-05-18 19:02 UTC (permalink / raw)
To: Michał Mirosław; +Cc: netdev, David Miller
In-Reply-To: <1305583775.2885.65.camel@bwh-desktop>
On Mon, 2011-05-16 at 23:09 +0100, Ben Hutchings wrote:
> On Mon, 2011-05-16 at 23:50 +0200, Michał Mirosław wrote:
> > On Mon, May 16, 2011 at 10:08:59PM +0100, Ben Hutchings wrote:
[...]
> > > I've explained before that I do not want to add new options to do
> > > (mostly) the same thing. Users should have not have to use a different
> > > command depending on the kernel version.
> >
> > We can avoid new option by checking feature-strings for unrecognised
> > arguments to -K. This way, we will have the old options which work
> > regardless of kernel version ('tx', 'rx', 'sg', etc.) and new options
> > which need recent kernel anyway (separated 'tx-checksum-*', 'loopback',
> > others coming in for 2.6.40).
>
> This is just too subtle a distinction. It will mostly confuse users.
[...]
Sorry, I think I misunderstood you here. I agree that new feature names
that do not correspond exactly to existing keywords should be supported
as keywords after the -K option. I think those that do (e.g.
"tx-udp-fragmentation" vs "ufo") should not be, as adding a
kernel-version-dependent *alias* would be confusing.
I also want users to benefit from your improvements (as I explained
above) even when they use the old names, if they are using a new kernel
version. That is why I want ethtool to try using ETHTOOL_SFEATURES
first, and why the fallback in the kernel is problematic.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* RE: [PATCH v2] CDC NCM: release interfaces fix in unbind()
From: Alan Stern @ 2011-05-18 19:10 UTC (permalink / raw)
To: Alexey ORISHKO
Cc: gregkh@suse.de, oliver@neukum.org, netdev@vger.kernel.org,
davem@davemloft.net, linux-usb@vger.kernel.org
In-Reply-To: <2AC7D4AD8BA1C640B4C60C61C8E520153E3ACE0774@EXDCVYMBSTM006.EQ1STM.local>
On Wed, 18 May 2011, Alexey ORISHKO wrote:
> > From: Alan Stern [mailto:stern@rowland.harvard.edu]
> > Sent: Wednesday, May 18, 2011 4:55 PM
> >
> > Here and later on, the patch seems to have forgotten about the control
> > interface. Is this deliberate or an oversight?
> >
> > Alan Stern
>
> Kernel docs says, that usb_driver_claim_interface() is used by usb
> device drivers that need to claim more than one interface. I assume,
> it's needed for second interface only. Am I wrong?
Well, if a driver wants to claim three interfaces (which is more than
one), it would call usb_driver_claim_interface() for both the second
and third interfaces, not only the second.
In general, when the driver gets probed for any one of the interfaces,
it should identify all the interfaces it's interested in and claim
them. However, it should skip the interface currently being probed --
that usb_driver_claim_interface() call would fail anyway since an
interface can't be claimed while it is being probed.
Similarly, when any of the interfaces is unbound, the driver should
release them all.
> NCM driver was partly based on ECM code. I did check existing drivers
> for CDC ACM and CDC ECM, which also uses 2 interfaces: master (control)
> and data (bulk), but I was unable to find any code, which claims master
> interface.
>
> If we need explicitly claim/release master interface (which is *intf
> parameter in bind() and unbind()), then cdc-acm and usb_ether drivers
> should be updated as well.
As far as I can tell, those drivers check that they are being probed
for the control interface, so all they need to claim is the data
interface.
You could do something similar -- have the bind routine return
-ENODEV if it's not being called for the control interface. But the
unbind routine would still have to release both interfaces, since it
can't rely on being called for the control interface first.
Alan Stern
^ permalink raw reply
* Re: [PATCH] tcp: Expose the initial RTO via a new sysctl.
From: David Miller @ 2011-05-18 19:26 UTC (permalink / raw)
To: tsunanet; +Cc: kuznet, pekkas, jmorris, yoshfuji, kaber, netdev, linux-kernel
In-Reply-To: <1305715384-81716-1-git-send-email-tsunanet@gmail.com>
From: Benoit Sigoure <tsunanet@gmail.com>
Date: Wed, 18 May 2011 03:43:04 -0700
> Instead of hardcoding the initial RTO to 3s and requiring
> the kernel to be recompiled to change it, expose it as a
> sysctl that can be tuned at runtime. Leave the default
> value unchanged.
>
> Signed-off-by: Benoit Sigoure <tsunanet@gmail.com>
If you read the ietf draft that reduces the initial RTO down to 1
second, it states that if we take a timeout during the initial
connection handshake then we have to revert the RTO back up to 3
seconds.
This fallback logic conflicts with being able to only change the
initial RTO via sysctl, I think. Because there are actually two
values at stake and they depend upon eachother, the initial RTO and
the value we fallback to on initial handshake retransmissions.
So I'd rather get a patch that implements the 1 second initial
RTO with the 3 second fallback on SYN retransmit, than this patch.
We already have too many knobs.
^ permalink raw reply
* Re: Bug, kernel panic, NULL dereference , cleanup_once / icmp_route_lookup.clone.19.clone / nat , 2.6.39-rc7-git11
From: Eric Dumazet @ 2011-05-18 19:29 UTC (permalink / raw)
To: Denys Fedoryshchenko; +Cc: netdev
In-Reply-To: <1305733944.2991.45.camel@edumazet-laptop>
Le mercredi 18 mai 2011 à 17:52 +0200, Eric Dumazet a écrit :
> Hmm, it seems we have some inetpeer refcount leak somewhere.
>
> Maybe one (struct rtable)->peer is not released on dst/rtable removal,
> or we also leak dst/rtable (and their ->peer inetpeer)
>
> Watch :
>
> grep peer /proc/slabinfo
> grep dst /proc/slabinfo
>
FYI, I started a bisection to find the faulty commit.
^ permalink raw reply
* Re: [PATCH] tcp: Expose the initial RTO via a new sysctl.
From: tsuna @ 2011-05-18 19:40 UTC (permalink / raw)
To: David Miller
Cc: kuznet, pekkas, jmorris, yoshfuji, kaber, netdev, linux-kernel
In-Reply-To: <20110518.152653.1486764697527722925.davem@davemloft.net>
On Wed, May 18, 2011 at 12:26 PM, David Miller <davem@davemloft.net> wrote:
> If you read the ietf draft that reduces the initial RTO down to 1
> second, it states that if we take a timeout during the initial
> connection handshake then we have to revert the RTO back up to 3
> seconds.
>
> This fallback logic conflicts with being able to only change the
> initial RTO via sysctl, I think. Because there are actually two
> values at stake and they depend upon eachother, the initial RTO and
> the value we fallback to on initial handshake retransmissions.
>
> So I'd rather get a patch that implements the 1 second initial
> RTO with the 3 second fallback on SYN retransmit, than this patch.
>
> We already have too many knobs.
I was hoping this knob would be accepted because this is such an
important issue that it even warrants an IETF draft to attempt to
change the standard. I'm not sure how long it will take for this
draft to be accepted and then implemented, so I thought adding this
simple knob today would really help in the future.
Plus, should the draft be accepted, this knob will still be just as
useful (e.g. to revert back to today's behavior), and people might
want to consider adding another knob for the fallback initRTO (this is
debatable). I don't believe this knob conflicts with the proposed
change to the standard, it actually goes along with it pretty well and
helps us prepare better for this upcoming change.
I agree that there are too many knobs, and I hate feature creep too,
but I've found many of these knobs to be really useful, and the degree
to which Linux's TCP stack can be tuned is part of what makes it so
versatile.
--
Benoit "tsuna" Sigoure
Software Engineer @ www.StumbleUpon.com
^ permalink raw reply
* net-next-2.6 wireless build failure
From: David Miller @ 2011-05-18 19:55 UTC (permalink / raw)
To: linux-wireless-u79uwXL29TY76Z2rM5mHXA; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: Text/Plain; charset=iso-8859-7, Size: 943 bytes --]
The suspend op member of ieee80211_ops is only available when
CONFIG_PM is enabled.
drivers/net/wireless/wl12xx/main.c:3653:2: error: unknown field ¡suspend¢ specified in initializer
drivers/net/wireless/wl12xx/main.c:3653:2: warning: initialization from incompatible pointer type [enabled by default]
drivers/net/wireless/wl12xx/main.c:3653:2: warning: (near initialization for ¡wl1271_ops.config¢) [enabled by default]
drivers/net/wireless/wl12xx/main.c:3654:2: error: unknown field ¡resume¢ specified in initializer
drivers/net/wireless/wl12xx/main.c:3654:2: warning: initialization from incompatible pointer type [enabled by default]
drivers/net/wireless/wl12xx/main.c:3654:2: warning: (near initialization for ¡wl1271_ops.bss_info_changed¢) [enabled by default]
Someone please fix this.
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±Â*Þ•ë,Š{ayº\x1dʇڙë,j\a¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš
^ permalink raw reply
* Re: [PATCH] tcp: Expose the initial RTO via a new sysctl.
From: David Miller @ 2011-05-18 19:52 UTC (permalink / raw)
To: tsunanet; +Cc: kuznet, pekkas, jmorris, yoshfuji, kaber, netdev, linux-kernel
In-Reply-To: <BANLkTik-6wh8=jb6oMEpJvYC8+KTGGsMsw@mail.gmail.com>
From: tsuna <tsunanet@gmail.com>
Date: Wed, 18 May 2011 12:40:21 -0700
> I was hoping this knob would be accepted because this is such an
> important issue that it even warrants an IETF draft to attempt to
> change the standard. I'm not sure how long it will take for this
> draft to be accepted and then implemented, so I thought adding this
> simple knob today would really help in the future.
I've already changed the initial TCP congestion window in Linux to 10
without some stupid draft being fully accepted.
I'll just as easily accept right now a patch right now which lowers
the initial RTO to 1 second and adds the 3 second RTO fallback.
^ permalink raw reply
* Re: PATCH] SCTP: fix race between sctp_bind_addr_free() and sctp_bind_addr_conflict()
From: David Miller @ 2011-05-18 19:58 UTC (permalink / raw)
To: difrost.kernel; +Cc: netdev, vladislav.yasevich, eric.dumazet
In-Reply-To: <BANLkTimWpmFj1gx=XvvoXFVxL9q927iWfw@mail.gmail.com>
From: Jacek Luczak <difrost.kernel@gmail.com>
Date: Wed, 18 May 2011 16:36:32 +0200
> @@ -140,14 +140,12 @@ void sctp_bind_addr_init(struct sctp_bind_addr
> *bp, __u16 port)
As Eric stated, you need to fix your mailer to not corrupt your patch.
GMAIL users can, and do, submit patches properly, unmangled. Heck,
even Thunderbird users can it.
Please follow the directions in Documentation/email-clients.txt to
get your mailer in a state where it won't mangle your patch postings.
Thanks.
^ permalink raw reply
* ip_vs_ftp causing ip_vs oops on module load.
From: Dave Jones @ 2011-05-18 20:19 UTC (permalink / raw)
To: netdev; +Cc: Wensong Zhang
I get this oops from ip_vs_ftp..
general protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
last sysfs file: /sys/module/nf_nat/refcnt
CPU 3
Modules linked in: ip_vs(+) libcrc32c nf_nat nfsd lockd nfs_acl auth_rpcgss sunrpc cpufreq_ondemand powernow_k8 freq_table mperf ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables snd_hda_codec_realtek ppdev snd_hda_intel snd_hda_codec snd_hwdep snd_seq snd_seq_device snd_pcm microcode edac_core snd_timer k10temp snd pcspkr usb_debug edac_mce_amd soundcore snd_page_alloc sp5100_tco i2c_piix4 parport_pc parport wmi r8169 mii lm63 ipv6 pata_acpi firewire_ohci ata_generic firewire_core crc_itu_t pata_atiixp floppy radeon ttm drm_kms_helper drm i2c_algo_bit i2c_core [last unloaded: nf_nat]
Pid: 1366, comm: modprobe Not tainted 2.6.39-rc7+ #15 Gigabyte Technology Co., Ltd. GA-MA78GM-S2H/GA-MA78GM-S2H
RIP: 0010:[<ffffffff8107bddb>] [<ffffffff8107bddb>] notifier_chain_register+0xb/0x2a
RSP: 0018:ffff880114139e68 EFLAGS: 00010206
RAX: 2f736e74656e2f74 RBX: ffffffffa04265d0 RCX: 0000000000000003
RDX: 00000000656e6567 RSI: ffffffffa04265d0 RDI: ffffffffa04235d8
RBP: ffff880114139e68 R08: ffff880114139df8 R09: 0000000000000001
R10: 0000000000000001 R11: 00000000000001cc R12: ffffffffa0432106
R13: 0000000000000000 R14: 0000000000007f0d R15: 0000000000410e40
FS: 00007f2aaf242720(0000) GS:ffff88012a800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 00007f2aaea0100f CR3: 000000011424f000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process modprobe (pid: 1366, threadinfo ffff880114138000, task ffff8801146cc7a0)
Stack:
ffff880114139e78 ffffffff8107be36 ffff880114139ec8 ffffffff81403058
0000000000000000 0000000000000000 ffff880114139ea8 0000000000000000
ffffffffa0432106 0000000000000000 0000000000007f0d 0000000000410e40
Call Trace:
[<ffffffff8107be36>] raw_notifier_chain_register+0xe/0x10
[<ffffffff81403058>] register_netdevice_notifier+0x2d/0x1b6
[<ffffffffa0432106>] ? ip_vs_conn_init+0x106/0x106 [ip_vs]
[<ffffffffa04322c7>] ip_vs_control_init+0xa5/0xce [ip_vs]
[<ffffffffa0432106>] ? ip_vs_conn_init+0x106/0x106 [ip_vs]
[<ffffffffa0432116>] ip_vs_init+0x10/0x11c [ip_vs]
[<ffffffff81002099>] do_one_initcall+0x7f/0x13a
[<ffffffff81096524>] sys_init_module+0x132/0x281
[<ffffffff814cc702>] system_call_fastpath+0x16/0x1b
Code: 07 ff c8 89 43 48 eb 08 48 89 df e8 dc 95 44 00 4c 89 e6 48 89 df e8 a7 a5 44 00 5b 41 5c 5d c3 55 48 89 e5 66 66 66 66 90 eb 0c <8b> 50 10 39 56 10 7f 0c 48 8d 78 08 48 8b 07 48 85 c0 75 ec 48
RIP [<ffffffff8107bddb>] notifier_chain_register+0xb/0x2a
RSP <ffff880114139e68>
---[ end trace e90d7053ad1a7a5b ]---
This script replicates the bug.
(it usually oopses after just a few loops)
#!/bin/sh
while [ 1 ];
do
modprobe ip_vs_ftp
modprobe -r ip_vs_ftp
done
Looks like something isn't getting cleaned up on module exit
that we fall over when we encounter it next time it gets loaded ?
Dave
^ permalink raw reply
* Re: [PATCH] tcp: Expose the initial RTO via a new sysctl.
From: Hagen Paul Pfeifer @ 2011-05-18 20:20 UTC (permalink / raw)
To: David Miller
Cc: tsunanet, kuznet, pekkas, jmorris, yoshfuji, kaber, netdev,
linux-kernel
In-Reply-To: <20110518.155200.801089483916944725.davem@davemloft.net>
* David Miller | 2011-05-18 15:52:00 [-0400]:
>I've already changed the initial TCP congestion window in Linux to 10
>without some stupid draft being fully accepted.
>
>I'll just as easily accept right now a patch right now which lowers
>the initial RTO to 1 second and adds the 3 second RTO fallback.
I like the idea to make the initial RTO a knob because we in a isolated MANET
environment have a RTT larger then 1 second. Especially the link layer setup
procedure over several hops demand some time-costly setup time. After that the
RTT is <1 second. The current algorithm works great for us. So this RTO change
will be counterproductive: it will always trigger a needless timeout.
The main problem for us is that Google at all pushing their view of Internet
with a lot of pressure. The same is true for the IETF IW adjustments, which is
unsuitable for networks which operates at a bandwidth characteristic some
years ago. The _former_ conservative principle "TCP over everything" is
forgotten.
Hagen
^ permalink raw reply
* Re: [PATCH] tcp: Expose the initial RTO via a new sysctl.
From: David Miller @ 2011-05-18 20:23 UTC (permalink / raw)
To: hagen
Cc: tsunanet, kuznet, pekkas, jmorris, yoshfuji, kaber, netdev,
linux-kernel
In-Reply-To: <20110518202025.GC4175@nuttenaction>
From: Hagen Paul Pfeifer <hagen@jauu.net>
Date: Wed, 18 May 2011 22:20:25 +0200
> I like the idea to make the initial RTO a knob because we in a
> isolated MANET environment have a RTT larger then 1 second.
Then this gets back to the fact that this is a network
attribute and thus more suitable as a route metric not
a global system-wide sysctl.
^ permalink raw reply
* Re: [PATCH] tcp: Expose the initial RTO via a new sysctl.
From: Hagen Paul Pfeifer @ 2011-05-18 20:27 UTC (permalink / raw)
To: David Miller
Cc: tsunanet, kuznet, pekkas, jmorris, yoshfuji, kaber, netdev,
linux-kernel
In-Reply-To: <20110518.162306.441229167098643303.davem@davemloft.net>
* David Miller | 2011-05-18 16:23:06 [-0400]:
>Then this gets back to the fact that this is a network
>attribute and thus more suitable as a route metric not
>a global system-wide sysctl.
Yes, in an Email response to Eric I mentioned this already. The initial RTO is
a perfect candidate for route metric. I waiting for a patch to test it! ;-)
Hagen
^ permalink raw reply
* Re: net-next-2.6 wireless build failure
From: Luciano Coelho @ 2011-05-18 20:38 UTC (permalink / raw)
To: David Miller
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, ido-Ix1uc/W3ht7QT0dZR+AlfA,
eliad-Ix1uc/W3ht7QT0dZR+AlfA
In-Reply-To: <20110518.155501.486816316237445149.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Hi Dave,
On Wed, 2011-05-18 at 15:55 -0400, David Miller wrote:
> The suspend op member of ieee80211_ops is only available when
> CONFIG_PM is enabled.
>
> drivers/net/wireless/wl12xx/main.c:3653:2: error: unknown field ‘suspend’ specified in initializer
> drivers/net/wireless/wl12xx/main.c:3653:2: warning: initialization from incompatible pointer type [enabled by default]
> drivers/net/wireless/wl12xx/main.c:3653:2: warning: (near initialization for ‘wl1271_ops.config’) [enabled by default]
> drivers/net/wireless/wl12xx/main.c:3654:2: error: unknown field ‘resume’ specified in initializer
> drivers/net/wireless/wl12xx/main.c:3654:2: warning: initialization from incompatible pointer type [enabled by default]
> drivers/net/wireless/wl12xx/main.c:3654:2: warning: (near initialization for ‘wl1271_ops.bss_info_changed’) [enabled by default]
>
> Someone please fix this.
I'm really sorry to let this pass through. I'll send a patch to fix it
in a second.
--
Cheers,
Luca.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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] wl12xx: fix compilation error when CONFIG_PM is not set
From: Luciano Coelho @ 2011-05-18 20:45 UTC (permalink / raw)
To: linux-wireless-u79uwXL29TY76Z2rM5mHXA, David Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linville-2XuSBdqkA4R54TAoqtyWWQ,
eliad-Ix1uc/W3ht7QT0dZR+AlfA, ido-Ix1uc/W3ht7QT0dZR+AlfA
In-Reply-To: <1305751102.12586.1546.camel@cumari>
There was a compilation error when PM is not enabled:
CC [M] drivers/net/wireless/wl12xx/main.o
drivers/net/wireless/wl12xx/main.c:3653: error: unknown field 'suspend' specified in initializer
drivers/net/wireless/wl12xx/main.c:3653: warning: initialization from incompatible pointer type
drivers/net/wireless/wl12xx/main.c:3654: error: unknown field 'resume' specified in initializer
drivers/net/wireless/wl12xx/main.c:3654: warning: initialization from incompatible pointer type
Fix this by adding #ifdef's in the appropriate places.
Cc: Eliad Peller <eliad-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Luciano Coelho <coelho-l0cyMroinI0@public.gmane.org>
---
drivers/net/wireless/wl12xx/main.c | 4 ++++
drivers/net/wireless/wl12xx/sdio.c | 4 ++++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 610be03..bc00e52 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -1350,6 +1350,7 @@ static struct notifier_block wl1271_dev_notifier = {
.notifier_call = wl1271_dev_notify,
};
+#ifdef CONFIG_PM
static int wl1271_configure_suspend(struct wl1271 *wl)
{
int ret;
@@ -1493,6 +1494,7 @@ static int wl1271_op_resume(struct ieee80211_hw *hw)
return 0;
}
+#endif
static int wl1271_op_start(struct ieee80211_hw *hw)
{
@@ -3650,8 +3652,10 @@ static const struct ieee80211_ops wl1271_ops = {
.stop = wl1271_op_stop,
.add_interface = wl1271_op_add_interface,
.remove_interface = wl1271_op_remove_interface,
+#ifdef CONFIG_PM
.suspend = wl1271_op_suspend,
.resume = wl1271_op_resume,
+#endif
.config = wl1271_op_config,
.prepare_multicast = wl1271_op_prepare_multicast,
.configure_filter = wl1271_op_configure_filter,
diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c
index 92d29a8..536e506 100644
--- a/drivers/net/wireless/wl12xx/sdio.c
+++ b/drivers/net/wireless/wl12xx/sdio.c
@@ -330,6 +330,7 @@ static void __devexit wl1271_remove(struct sdio_func *func)
wl1271_free_hw(wl);
}
+#ifdef CONFIG_PM
static int wl1271_suspend(struct device *dev)
{
/* Tell MMC/SDIO core it's OK to power down the card
@@ -385,15 +386,18 @@ static const struct dev_pm_ops wl1271_sdio_pm_ops = {
.suspend = wl1271_suspend,
.resume = wl1271_resume,
};
+#endif
static struct sdio_driver wl1271_sdio_driver = {
.name = "wl1271_sdio",
.id_table = wl1271_devices,
.probe = wl1271_probe,
.remove = __devexit_p(wl1271_remove),
+#ifdef CONFIG_PM
.drv = {
.pm = &wl1271_sdio_pm_ops,
},
+#endif
};
static int __init wl1271_init(void)
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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] wl12xx: fix compilation error when CONFIG_PM is not set
From: David Miller @ 2011-05-18 21:00 UTC (permalink / raw)
To: coelho; +Cc: linux-wireless, netdev, linville, eliad, ido
In-Reply-To: <1305751537-27533-1-git-send-email-coelho@ti.com>
From: Luciano Coelho <coelho@ti.com>
Date: Wed, 18 May 2011 23:45:37 +0300
> There was a compilation error when PM is not enabled:
>
> CC [M] drivers/net/wireless/wl12xx/main.o
> drivers/net/wireless/wl12xx/main.c:3653: error: unknown field 'suspend' specified in initializer
> drivers/net/wireless/wl12xx/main.c:3653: warning: initialization from incompatible pointer type
> drivers/net/wireless/wl12xx/main.c:3654: error: unknown field 'resume' specified in initializer
> drivers/net/wireless/wl12xx/main.c:3654: warning: initialization from incompatible pointer type
>
> Fix this by adding #ifdef's in the appropriate places.
>
> Cc: Eliad Peller <eliad@wizery.com>
> Signed-off-by: Luciano Coelho <coelho@ti.com>
Applied, thanks.
^ permalink raw reply
* [PATCH] forcedeth: Improve stats counters
From: David Decotigny @ 2011-05-18 21:09 UTC (permalink / raw)
To: David S. Miller, Joe Perches, Szymon Janc, netdev, linux-kernel
Cc: kernel-net-upstream, Mandeep Baines, David Decotigny
From: Mandeep Baines <msb@google.com>
Rx byte count was off; instead use the hardware's count. Tx packet
count was counting pre-TSO packets; instead count on-the-wire packets.
Report hardware dropped frame count as rx_fifo_errors.
- The count of transmitted packets reported by the forcedeth driver
reports pre-TSO (TCP Segmentation Offload) packet counts and not the
count of the number of packets sent on the wire. This change fixes
the forcedeth driver to report the correct count. Fixed the code by
copying the count stored in the NIC H/W to the value reported by the
driver.
- Count rx_drop_frame errors as rx_fifo_errors:
We see a lot of rx_drop_frame errors if we disable the rx bottom-halves
for too long. Normally, rx_fifo_errors would be counted in this case.
The rx_drop_frame error count is private to forcedeth and is not
reported by ifconfig or sysfs. The rx_fifo_errors count is currently
unused in the forcedeth driver. It is reported by ifconfig as overruns.
This change reports rx_drop_frame errors as rx_fifo_errors.
Signed-off-by: David Decotigny <decot@google.com>
---
drivers/net/forcedeth.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index d09e8b0..895471d 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -1684,6 +1684,7 @@ static void nv_get_hw_stats(struct net_device *dev)
np->estats.tx_pause += readl(base + NvRegTxPause);
np->estats.rx_pause += readl(base + NvRegRxPause);
np->estats.rx_drop_frame += readl(base + NvRegRxDropFrame);
+ np->estats.rx_errors_total += np->estats.rx_drop_frame;
}
if (np->driver_data & DEV_HAS_STATISTICS_V3) {
@@ -1708,11 +1709,14 @@ static struct net_device_stats *nv_get_stats(struct net_device *dev)
nv_get_hw_stats(dev);
/* copy to net_device stats */
+ dev->stats.tx_packets = np->estats.tx_packets;
+ dev->stats.rx_bytes = np->estats.rx_bytes;
dev->stats.tx_bytes = np->estats.tx_bytes;
dev->stats.tx_fifo_errors = np->estats.tx_fifo_errors;
dev->stats.tx_carrier_errors = np->estats.tx_carrier_errors;
dev->stats.rx_crc_errors = np->estats.rx_crc_errors;
dev->stats.rx_over_errors = np->estats.rx_over_errors;
+ dev->stats.rx_fifo_errors = np->estats.rx_drop_frame;
dev->stats.rx_errors = np->estats.rx_errors_total;
dev->stats.tx_errors = np->estats.tx_errors_total;
}
--
1.7.3.1
^ permalink raw reply related
* [PATCH] forcedeth: new ethtool stat "tx_timeout" to account for tx_timeouts
From: David Decotigny @ 2011-05-18 21:09 UTC (permalink / raw)
To: David S. Miller, Joe Perches, Szymon Janc, netdev, linux-kernel
Cc: kernel-net-upstream, Sameer Nanda, David Decotigny
In-Reply-To: <1305752945-14843-1-git-send-email-decot@google.com>
From: Sameer Nanda <snanda@google.com>
This change publishes a new ethtool stats: tx_timeout that counts the
number of times the tx_timeout callback was triggered.
Signed-off-by: David Decotigny <decot@google.com>
---
drivers/net/forcedeth.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index 895471d..112dc0b 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -632,6 +632,7 @@ static const struct nv_ethtool_str nv_estats_str[] = {
{ "rx_packets" },
{ "rx_errors_total" },
{ "tx_errors_total" },
+ { "tx_timeout" },
/* version 2 stats */
{ "tx_deferral" },
@@ -672,6 +673,7 @@ struct nv_ethtool_stats {
u64 rx_packets;
u64 rx_errors_total;
u64 tx_errors_total;
+ u64 tx_timeout;
/* version 2 stats */
u64 tx_deferral;
@@ -2526,6 +2528,8 @@ static void nv_tx_timeout(struct net_device *dev)
spin_lock_irq(&np->lock);
+ np->estats.tx_timeout++;
+
/* 1) stop tx engine */
nv_stop_tx(dev);
--
1.7.3.1
^ permalink raw reply related
* [PATCH 2/2] forcedeth: allow to silence tx_timeout debug messages
From: David Decotigny @ 2011-05-18 21:10 UTC (permalink / raw)
To: David S. Miller, Joe Perches, Szymon Janc, netdev, linux-kernel
Cc: kernel-net-upstream, Sameer Nanda, David Decotigny
In-Reply-To: <1305753000-14933-1-git-send-email-decot@google.com>
From: Sameer Nanda <snanda@google.com>
This change allows to silence most debug messages in case of TX
timeout. These messages don't provide a signare/noise ratio high
enough for production systems and, with ~30kB logged each time, they
tend to add to a cascade effect if the system is already under stress
(memory pressure, disk, etc.).
By default, the debug messages are not displayed but this can be
overriden by setting the debug_tx_timeout module parameter.
Signed-off-by: David Decotigny <decot@google.com>
---
drivers/net/forcedeth.c | 93 ++++++++++++++++++++++++++---------------------
1 files changed, 52 insertions(+), 41 deletions(-)
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index 9566567..2c176ff 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -896,6 +896,11 @@ enum {
static int dma_64bit = NV_DMA_64BIT_ENABLED;
/*
+ * Debug output control for tx_timeout
+ */
+static bool debug_tx_timeout = false;
+
+/*
* Crossover Detection
* Realtek 8201 phy + some OEM boards do not work properly.
*/
@@ -2473,56 +2478,59 @@ static void nv_tx_timeout(struct net_device *dev)
u32 status;
union ring_type put_tx;
int saved_tx_limit;
- int i;
if (np->msi_flags & NV_MSI_X_ENABLED)
status = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQSTAT_MASK;
else
status = readl(base + NvRegIrqStatus) & NVREG_IRQSTAT_MASK;
- netdev_info(dev, "Got tx_timeout. irq: %08x\n", status);
+ if (unlikely(debug_tx_timeout)) {
+ int i;
- netdev_info(dev, "Ring at %lx\n", (unsigned long)np->ring_addr);
- netdev_info(dev, "Dumping tx registers\n");
- for (i = 0; i <= np->register_size; i += 32) {
- netdev_info(dev,
- "%3x: %08x %08x %08x %08x %08x %08x %08x %08x\n",
- i,
- readl(base + i + 0), readl(base + i + 4),
- readl(base + i + 8), readl(base + i + 12),
- readl(base + i + 16), readl(base + i + 20),
- readl(base + i + 24), readl(base + i + 28));
- }
- netdev_info(dev, "Dumping tx ring\n");
- for (i = 0; i < np->tx_ring_size; i += 4) {
- if (!nv_optimized(np)) {
- netdev_info(dev,
- "%03x: %08x %08x // %08x %08x // %08x %08x // %08x %08x\n",
- i,
- le32_to_cpu(np->tx_ring.orig[i].buf),
- le32_to_cpu(np->tx_ring.orig[i].flaglen),
- le32_to_cpu(np->tx_ring.orig[i+1].buf),
- le32_to_cpu(np->tx_ring.orig[i+1].flaglen),
- le32_to_cpu(np->tx_ring.orig[i+2].buf),
- le32_to_cpu(np->tx_ring.orig[i+2].flaglen),
- le32_to_cpu(np->tx_ring.orig[i+3].buf),
- le32_to_cpu(np->tx_ring.orig[i+3].flaglen));
- } else {
+ netdev_warn(dev, "Got tx_timeout. irq: %08x\n", status);
+
+ netdev_info(dev, "Ring at %lx\n", (unsigned long)np->ring_addr);
+ netdev_info(dev, "Dumping tx registers\n");
+ for (i = 0; i <= np->register_size; i += 32) {
netdev_info(dev,
- "%03x: %08x %08x %08x // %08x %08x %08x // %08x %08x %08x // %08x %08x %08x\n",
+ "%3x: %08x %08x %08x %08x %08x %08x %08x %08x\n",
i,
- le32_to_cpu(np->tx_ring.ex[i].bufhigh),
- le32_to_cpu(np->tx_ring.ex[i].buflow),
- le32_to_cpu(np->tx_ring.ex[i].flaglen),
- le32_to_cpu(np->tx_ring.ex[i+1].bufhigh),
- le32_to_cpu(np->tx_ring.ex[i+1].buflow),
- le32_to_cpu(np->tx_ring.ex[i+1].flaglen),
- le32_to_cpu(np->tx_ring.ex[i+2].bufhigh),
- le32_to_cpu(np->tx_ring.ex[i+2].buflow),
- le32_to_cpu(np->tx_ring.ex[i+2].flaglen),
- le32_to_cpu(np->tx_ring.ex[i+3].bufhigh),
- le32_to_cpu(np->tx_ring.ex[i+3].buflow),
- le32_to_cpu(np->tx_ring.ex[i+3].flaglen));
+ readl(base + i + 0), readl(base + i + 4),
+ readl(base + i + 8), readl(base + i + 12),
+ readl(base + i + 16), readl(base + i + 20),
+ readl(base + i + 24), readl(base + i + 28));
+ }
+ netdev_info(dev, "Dumping tx ring\n");
+ for (i = 0; i < np->tx_ring_size; i += 4) {
+ if (!nv_optimized(np)) {
+ netdev_info(dev,
+ "%03x: %08x %08x // %08x %08x // %08x %08x // %08x %08x\n",
+ i,
+ le32_to_cpu(np->tx_ring.orig[i].buf),
+ le32_to_cpu(np->tx_ring.orig[i].flaglen),
+ le32_to_cpu(np->tx_ring.orig[i+1].buf),
+ le32_to_cpu(np->tx_ring.orig[i+1].flaglen),
+ le32_to_cpu(np->tx_ring.orig[i+2].buf),
+ le32_to_cpu(np->tx_ring.orig[i+2].flaglen),
+ le32_to_cpu(np->tx_ring.orig[i+3].buf),
+ le32_to_cpu(np->tx_ring.orig[i+3].flaglen));
+ } else {
+ netdev_info(dev,
+ "%03x: %08x %08x %08x // %08x %08x %08x // %08x %08x %08x // %08x %08x %08x\n",
+ i,
+ le32_to_cpu(np->tx_ring.ex[i].bufhigh),
+ le32_to_cpu(np->tx_ring.ex[i].buflow),
+ le32_to_cpu(np->tx_ring.ex[i].flaglen),
+ le32_to_cpu(np->tx_ring.ex[i+1].bufhigh),
+ le32_to_cpu(np->tx_ring.ex[i+1].buflow),
+ le32_to_cpu(np->tx_ring.ex[i+1].flaglen),
+ le32_to_cpu(np->tx_ring.ex[i+2].bufhigh),
+ le32_to_cpu(np->tx_ring.ex[i+2].buflow),
+ le32_to_cpu(np->tx_ring.ex[i+2].flaglen),
+ le32_to_cpu(np->tx_ring.ex[i+3].bufhigh),
+ le32_to_cpu(np->tx_ring.ex[i+3].buflow),
+ le32_to_cpu(np->tx_ring.ex[i+3].flaglen));
+ }
}
}
@@ -6006,6 +6014,9 @@ module_param(phy_cross, int, S_IRUGO);
MODULE_PARM_DESC(phy_cross, "Phy crossover detection for Realtek 8201 phy is enabled by setting to 1 and disabled by setting to 0.");
module_param(phy_power_down, int, S_IRUGO);
MODULE_PARM_DESC(phy_power_down, "Power down phy and disable link when interface is down (1), or leave phy powered up (0).");
+module_param(debug_tx_timeout, bool, S_IRUGO);
+MODULE_PARM_DESC(debug_tx_timeout,
+ "Dump tx related registers and ring when tx_timeout happens");
MODULE_AUTHOR("Manfred Spraul <manfred@colorfullife.com>");
MODULE_DESCRIPTION("Reverse Engineered nForce ethernet driver");
--
1.7.3.1
^ permalink raw reply related
* [PATCH 1/2] forcedeth: make module parameters readable in /sys/module
From: David Decotigny @ 2011-05-18 21:09 UTC (permalink / raw)
To: David S. Miller, Joe Perches, Szymon Janc, netdev, linux-kernel
Cc: kernel-net-upstream, David Decotigny
This change allows to publish the values of the module parameters in
/sys/module.
Signed-off-by: David Decotigny <decot@google.com>
---
drivers/net/forcedeth.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index 112dc0b..9566567 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -5990,21 +5990,21 @@ static void __exit exit_nic(void)
pci_unregister_driver(&driver);
}
-module_param(max_interrupt_work, int, 0);
+module_param(max_interrupt_work, int, S_IRUGO);
MODULE_PARM_DESC(max_interrupt_work, "forcedeth maximum events handled per interrupt");
-module_param(optimization_mode, int, 0);
+module_param(optimization_mode, int, S_IRUGO);
MODULE_PARM_DESC(optimization_mode, "In throughput mode (0), every tx & rx packet will generate an interrupt. In CPU mode (1), interrupts are controlled by a timer. In dynamic mode (2), the mode toggles between throughput and CPU mode based on network load.");
-module_param(poll_interval, int, 0);
+module_param(poll_interval, int, S_IRUGO);
MODULE_PARM_DESC(poll_interval, "Interval determines how frequent timer interrupt is generated by [(time_in_micro_secs * 100) / (2^10)]. Min is 0 and Max is 65535.");
-module_param(msi, int, 0);
+module_param(msi, int, S_IRUGO);
MODULE_PARM_DESC(msi, "MSI interrupts are enabled by setting to 1 and disabled by setting to 0.");
-module_param(msix, int, 0);
+module_param(msix, int, S_IRUGO);
MODULE_PARM_DESC(msix, "MSIX interrupts are enabled by setting to 1 and disabled by setting to 0.");
-module_param(dma_64bit, int, 0);
+module_param(dma_64bit, int, S_IRUGO);
MODULE_PARM_DESC(dma_64bit, "High DMA is enabled by setting to 1 and disabled by setting to 0.");
-module_param(phy_cross, int, 0);
+module_param(phy_cross, int, S_IRUGO);
MODULE_PARM_DESC(phy_cross, "Phy crossover detection for Realtek 8201 phy is enabled by setting to 1 and disabled by setting to 0.");
-module_param(phy_power_down, int, 0);
+module_param(phy_power_down, int, S_IRUGO);
MODULE_PARM_DESC(phy_power_down, "Power down phy and disable link when interface is down (1), or leave phy powered up (0).");
MODULE_AUTHOR("Manfred Spraul <manfred@colorfullife.com>");
--
1.7.3.1
^ permalink raw reply related
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