* Re: [patch net-next-2.6] bonding: allow resetting slave failure counters
From: Nicolas de Pesloüan @ 2011-06-01 19:34 UTC (permalink / raw)
To: Flavio Leitner; +Cc: David Miller, fubar, jpirko, netdev, andy
In-Reply-To: <4DE68ECB.70802@redhat.com>
Le 01/06/2011 21:11, Flavio Leitner a écrit :
> On 06/01/2011 04:03 PM, David Miller wrote:
>> From: Jay Vosburgh<fubar@us.ibm.com>
>> Date: Wed, 01 Jun 2011 09:13:39 -0700
>>
>>> The "this dingus was added in version X.Y.Z" is there because
>>> users sometimes read the most recent version of the documentation (that
>>> they get from the internet) and then would become confused when their
>>> older distro driver lacked some option described in the documentation.
>>
>> I disagree with this whole concept, because distros backport features
>> like this into their kernel and therefore the feature is showing up in
>> version X.Y.$(Z-20).
>
> It doesn't matter the version if the user can find the feature, so
> distros backporting features works and that info is not useful at all.
> However, when the user doesn't find the feature and search the internet,
> then that info is helpful.
There are *many* new features that get included into the kernel without documenting the exact first
version that provide them. Why should we need this for bonding? Also, because we lack a table that
gives the kernel version matching a bonding version, the user is not really helped by "you need
version X.Y.Z of bonding to have this feature".
So, I'm not sure it helps...
Nicolas.
^ permalink raw reply
* Re: Skipping past TCP lost packet in userspace
From: juice @ 2011-06-01 19:36 UTC (permalink / raw)
To: Josh Lehan, Yuchung Cheng, Josh Lehan, netdev, jiyengar
In-Reply-To: <4DE5F3E3.2080609@krellan.com>
> Nice, thanks for pointing me to this. I appreciate the helpful answer,
> instead of just saying "use UDP" or "use SCTP". That's not the point.
>
> For better or for worse, TCP is realistically the only viable protocol
> for streaming to the largest possible audience these days, hence my
> question about adding this feature to the Linux TCP implementation.
>
For better or for worse, I think the problem in your proposal is that it
just will not be portable, even if you implemented it on Linux stack.
I am quite sure you will not be able to bend other operating systems to
accept this kind of kludge in the TCP stack so the benefits are minimal.
The correct way to address this problem is to make sure that end-to-end
connectivity of all the needed protocols is maintained.
^ permalink raw reply
* Re: [PATCH 7/10] drivers/net/davinci_emac.c: add missing clk_put
From: Kevin Hilman @ 2011-06-01 19:49 UTC (permalink / raw)
To: Julia Lawall
Cc: David S. Miller, kernel-janitors, Cyril Chemparathy,
Sriramakrishnan A G, Stefan Weil, netdev, linux-kernel
In-Reply-To: <1306948213-20767-7-git-send-email-julia@diku.dk>
On Wed, 2011-06-01 at 19:10 +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
>
> Go to existing error handling code at the end of the function that calls
> clk_put.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @r exists@
> expression e1,e2;
> statement S;
> @@
>
> e1 = clk_get@p1(...);
> ... when != e1 = e2
> when != clk_put(e1)
> when any
> if (...) { ... when != clk_put(e1)
> when != if (...) { ... clk_put(e1) ... }
> * return@p3 ...;
> } else S
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
Acked-by: Kevin Hilman <khilman@ti.com>
^ permalink raw reply
* Re: [RFC] Moving files around in drivers/net
From: Jeff Kirsher @ 2011-06-01 19:53 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, David Miller, Paul Gortmaker, Jan Engelhardt
In-Reply-To: <1306956602.32125.28.camel@Joe-Laptop>
[-- Attachment #1: Type: text/plain, Size: 803 bytes --]
On Wed, 2011-06-01 at 12:30 -0700, Joe Perches wrote:
> Does anyone still think moving files around in drivers/net
> would be sensible and a suitable candidate for inclusion
> in 3.1?
>
> Here's what Jeffrey proposed:
> http://vger.kernel.org/netconf2010_slides/netconf-jtk.pdf
>
> Here's what I proposed before that.
> http://www.spinics.net/lists/netdev/msg149717.html
>
> I agree with Jan that the current 10/100, 1000, 10000 speed
> selections mechanisms are less than ideal.
>
> Perhaps it'd be worthwhile to remove that speed distinction.
>
I am over 90% done with the work and have been trying to finish up the
patches so that I could get them out as an RFC here in the next week.
I did remove the current 10/100, 1000, 10000 speed selections in my
current patchset.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [patch net-next-2.6] bonding: allow resetting slave failure counters
From: Jay Vosburgh @ 2011-06-01 19:53 UTC (permalink / raw)
To: David Miller; +Cc: fbl, jpirko, netdev, andy
In-Reply-To: <20110601.122240.1431839726613846748.davem@davemloft.net>
David Miller <davem@davemloft.net> wrote:
>From: Flavio Leitner <fbl@redhat.com>
>Date: Wed, 01 Jun 2011 16:11:07 -0300
>
>> On 06/01/2011 04:03 PM, David Miller wrote:
>>> From: Jay Vosburgh <fubar@us.ibm.com>
>>> Date: Wed, 01 Jun 2011 09:13:39 -0700
>>>
>>>> The "this dingus was added in version X.Y.Z" is there because
>>>> users sometimes read the most recent version of the documentation (that
>>>> they get from the internet) and then would become confused when their
>>>> older distro driver lacked some option described in the documentation.
>>>
>>> I disagree with this whole concept, because distros backport features
>>> like this into their kernel and therefore the feature is showing up in
>>> version X.Y.$(Z-20).
>>
>> It doesn't matter the version if the user can find the feature, so
>> distros backporting features works and that info is not useful at all.
>> However, when the user doesn't find the feature and search the internet,
>> then that info is helpful.
>
>So how is the user going to find that FC14 has the feature even
>though his FC13 kernel does not?
>
>I'll say it again, this version stuff is completely pointless.
>
>If the user is dabbling with upstream kernels he's a minority,
>and clueful enough to figure out this stuff himself.
The problem was that users would search the internet for bonding
documentation, and get the version out of the current mainline. That
document described options not present in their distro kernel, and I got
questions asking why they couldn't enable some option present in
mainline but not their distro kernel. To try and minimize that
confusion, I started documenting when features were added (by bonding
driver version, not by kernel version).
Maybe this doesn't make as much difference today (for whatever
reason), but it certainly seemed to help back in the day.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH 1/1] Fix 802.3az compatible issue
From: Guo-Fu Tseng @ 2011-06-01 19:50 UTC (permalink / raw)
To: Aries Lee, 'David Rehner'; +Cc: netdev, devinchiu
In-Reply-To: <201106011250.p51Coox8031239@jmr105.jmicron.com>
On Wed, 1 Jun 2011 20:50:50 +0800, Aries Lee wrote
> The older JMicron NIC chip -- Jme251A, can NOT connect with a linking
> partner with "802.3az" feature in 1000M speed.
> More specifically speaking, it's HW design has a problem in
> auto-negotiation if the linking partner is running 802.3az function.
>
> This "Auto speed down" patch is a software workaround to avoid the user
> of older jme chip unable to connect with the linking partner with
> 802.3az function.
> If the NIC is down speed to 100M, everything is fine. But if the NIC is
> running by 1000M speed, this problem makes it unable to link up.
>
> The algorithm of this workaround is as following:
> Setup a timer to polling when the PHY link status is down, If the
> ethernet cable was attached but auto-negotiation has not been
> accomplished for a long time (default 11 second), that mean the linking
> partner maybe running 802.3az function, driver will try to
> connect by 100M speed.
> If the link is up, then stop the timer.
>
> Signed-off-by: arieslee <arieslee@jmicron.com>
I've already discussed about this fix approach with JMicron staff.
Personally I do not like the idea of doing "Auto Speed Down"
in driver. The driver shouldn't implicitly change the AutoNeg
setting by itself. It might confuse the sysadmin a lot. And by
design, it is not reasonable.
Their reason of still wanting to send this patch is about the
end user(Someone not familiar with network) experience. If one
can get the link, one might not care about what the driver did.
Anyway, if Davem or other senior developer is fine with this approach.
I can clean-up this patch, discuss more detail and re-submit again.
--
Guo-Fu Tseng
^ permalink raw reply
* Re: [RFC] Moving files around in drivers/net
From: Joe Perches @ 2011-06-01 20:01 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, David Miller, Paul Gortmaker, Jan Engelhardt
In-Reply-To: <1306957998.3076.18.camel@jtkirshe-MOBL1>
On Wed, 2011-06-01 at 12:53 -0700, Jeff Kirsher wrote:
> On Wed, 2011-06-01 at 12:30 -0700, Joe Perches wrote:
> > Does anyone still think moving files around in drivers/net
> > would be sensible and a suitable candidate for inclusion
> > in 3.1?
> > Here's what Jeffrey proposed:
> > http://vger.kernel.org/netconf2010_slides/netconf-jtk.pdf
> > Here's what I proposed before that.
> > http://www.spinics.net/lists/netdev/msg149717.html
> I am over 90% done with the work and have been trying to finish up the
> patches so that I could get them out as an RFC here in the next week.
Excellent, thanks.
^ permalink raw reply
* Re: [PATCH] ethtool: mask out FLOW_EXT
From: Ben Hutchings @ 2011-06-01 20:02 UTC (permalink / raw)
To: s.poehn; +Cc: netdev, sebastian.poehn
In-Reply-To: <31603.80.254.147.148.1306158835.squirrel@webmail.hs-esslingen.de>
On Mon, 2011-05-23 at 15:53 +0200, Sebastian Pöhn wrote:
> Sorry for the disclaimer the last time!
>
> The FLOW_EXT bit must be masked out. Otherwise if e.g. vlan is set a
> driver receiving the ntuple may not detect the flow_type correctly!
>
> Signed-off-by: Sebastian Poehn <sebastian.poehn@belden.com>
> ---
>
> diff --git a/ethtool.c b/ethtool.c
> index 34fe107..0b7ec05 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -3134,6 +3134,9 @@ static int flow_spec_to_ntuple(struct
> ethtool_rx_flow_spec *fsp,
> (u64)ntohl(~fsp->m_ext.data[1]);
> }
> }
> +
> + /*Mask out the extended bit, because ntuple does not know it!*/
> + ntuple->flow_type &= ~FLOW_EXT;
>
> return 0;
> }
Applied, with fixes to spacing. Thanks.
In future, please ensure that your editor and mailer do *not* convert
tabs to spaces. Also put spaces between the comment delimiters and
text, consistent with the rest of the code.
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: [BUG] net: cpu offline cause napi stall
From: Eric Dumazet @ 2011-06-01 20:03 UTC (permalink / raw)
To: Heiko Carstens; +Cc: Frank Blaschka, davem, netdev, linux-s390
In-Reply-To: <20110601181253.GA2374@osiris.boeblingen.de.ibm.com>
Le mercredi 01 juin 2011 à 20:12 +0200, Heiko Carstens a écrit :
> On Wed, Jun 01, 2011 at 06:55:21PM +0200, Eric Dumazet wrote:
> > > + /* Append NAPI poll list from offline CPU. */
> > > + list_splice_init(&oldsd->poll_list, &sd->poll_list);
> > >
> > > raise_softirq_irqoff(NET_TX_SOFTIRQ);
> > > local_irq_enable();
> >
> > Please make sure we raise NET_RX_SOFTIRQ on new cpu if necessary.
>
> Well, see two lines below the list_splice_init() call ;)
I see nothing... NET_TX_SOFTIRQ and NET_RX_SOFTIRQ are not the same
^ permalink raw reply
* Re: [PATCH 4/10] drivers/net/can/flexcan.c: add missing clk_put
From: Julia Lawall @ 2011-06-01 20:04 UTC (permalink / raw)
To: walter harms
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Wolfgang Grandegger
In-Reply-To: <4DE67CBF.5080006-fPG8STNUNVg@public.gmane.org>
On Wed, 1 Jun 2011, walter harms wrote:
>
>
> Am 01.06.2011 19:10, schrieb Julia Lawall:
> > From: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
> >
> > The failed_get label is used after the call to clk_get has succeeded, so it
> > should be moved up above the call to clk_put.
> >
> > A simplified version of the semantic match that finds this problem is as
> > follows: (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @r exists@
> > expression e1,e2;
> > statement S;
> > @@
> >
> > e1 = clk_get@p1(...);
> > ... when != e1 = e2
> > when != clk_put(e1)
> > when any
> > if (...) { ... when != clk_put(e1)
> > when != if (...) { ... clk_put(e1) ... }
> > * return@p3 ...;
> > } else S
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
> >
> > ---
> > drivers/net/can/flexcan.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index d499056..121739c 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -978,8 +978,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
> > failed_map:
> > release_mem_region(mem->start, mem_size);
> > failed_req:
> > - clk_put(clk);
> > failed_get:
> > + clk_put(clk);
> > failed_clock:
> > return err;
> > }
> >
>
> So failed_req == failed_get, is that intended ?
I have the impression that it is. failed_req comes after successful
calls to platform_get_resource and platform_get_irq, which don't allocate
anything.
julia
^ permalink raw reply
* Re: [patch net-next-2.6] bonding: allow resetting slave failure counters
From: Jiri Pirko @ 2011-06-01 20:07 UTC (permalink / raw)
To: Nicolas de Pesloüan
Cc: Flavio Leitner, David Miller, fubar, netdev, andy
In-Reply-To: <4DE69455.3080203@gmail.com>
Wed, Jun 01, 2011 at 09:34:45PM CEST, nicolas.2p.debian@gmail.com wrote:
>Le 01/06/2011 21:11, Flavio Leitner a écrit :
>>On 06/01/2011 04:03 PM, David Miller wrote:
>>>From: Jay Vosburgh<fubar@us.ibm.com>
>>>Date: Wed, 01 Jun 2011 09:13:39 -0700
>>>
>>>> The "this dingus was added in version X.Y.Z" is there because
>>>>users sometimes read the most recent version of the documentation (that
>>>>they get from the internet) and then would become confused when their
>>>>older distro driver lacked some option described in the documentation.
>>>
>>>I disagree with this whole concept, because distros backport features
>>>like this into their kernel and therefore the feature is showing up in
>>>version X.Y.$(Z-20).
>>
>>It doesn't matter the version if the user can find the feature, so
>>distros backporting features works and that info is not useful at all.
>>However, when the user doesn't find the feature and search the internet,
>>then that info is helpful.
>
>There are *many* new features that get included into the kernel
>without documenting the exact first version that provide them. Why
>should we need this for bonding? Also, because we lack a table that
>gives the kernel version matching a bonding version, the user is not
>really helped by "you need version X.Y.Z of bonding to have this
>feature".
I think that doing versioning on multiple parts of the same code is only
confusing. Kernel version should be only version for whole kernel code.
Changes should be only documented in descriptions of changesets. Any
other way is redundant, might be not accurate, and rather confusing.
Jirka
>
>So, I'm not sure it helps...
>
> Nicolas.
^ permalink raw reply
* Re: [PATCH 4/10] drivers/net/can/flexcan.c: add missing clk_put
From: Julia Lawall @ 2011-06-01 20:08 UTC (permalink / raw)
To: Joe Perches
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Wolfgang Grandegger
In-Reply-To: <1306949175.32125.20.camel@Joe-Laptop>
On Wed, 1 Jun 2011, Joe Perches wrote:
> On Wed, 2011-06-01 at 19:10 +0200, Julia Lawall wrote:
> > The failed_get label is used after the call to clk_get has succeeded, so it
> > should be moved up above the call to clk_put.
> []
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> []
> > @@ -978,8 +978,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
> > failed_map:
> > release_mem_region(mem->start, mem_size);
> > failed_req:
> > - clk_put(clk);
> > failed_get:
> > + clk_put(clk);
> > failed_clock:
> > return err;
>
> If this is correct, it might be better to rename all the
> uses of failed_req to failed_get and delete label failed_req.
As I replied to Walter Harms, I believe it is correct, because I don't see
anything in the called functions that needs to be freed. I can redo the
patch to eliminate the label.
julia
^ permalink raw reply
* Re: [patch net-next-2.6 v2] bonding: allow resetting slave failure counters
From: Jiri Pirko @ 2011-06-01 20:08 UTC (permalink / raw)
To: Nicolas de Pesloüan
Cc: Jay Vosburgh, Flavio Leitner, netdev, davem, andy
In-Reply-To: <4DE6919F.3030304@gmail.com>
Wed, Jun 01, 2011 at 09:23:11PM CEST, nicolas.2p.debian@gmail.com wrote:
>Le 01/06/2011 18:31, Jiri Pirko a écrit :
>>This patch allows to reset failure counters for all enslaved devices.
>
>Hi Jiri,
>
>Why do we need a way to reset those counters? What is the problem
>with having those counters monotonically increase until the system is
>rebooted? Do we have a way to reset other network statistics
>(/sys/class/net/eth0/statistics/* for example)?
Well, it's handy to be able to clear this counters when you resolve
a problem so future issues are crearly seen as non-zero.
>
>Except from this "do we need this feature" question, the code sounds good to me.
>
>Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
>
> Nicolas.
^ permalink raw reply
* Re: [PATCH 4/10] drivers/net/can/flexcan.c: add missing clk_put
From: David Miller @ 2011-06-01 20:11 UTC (permalink / raw)
To: julia; +Cc: joe, wg, kernel-janitors, socketcan-core, netdev, linux-kernel
In-Reply-To: <Pine.LNX.4.64.1106012207260.5871@ask.diku.dk>
From: Julia Lawall <julia@diku.dk>
Date: Wed, 1 Jun 2011 22:08:46 +0200 (CEST)
> On Wed, 1 Jun 2011, Joe Perches wrote:
>
>> On Wed, 2011-06-01 at 19:10 +0200, Julia Lawall wrote:
>> > The failed_get label is used after the call to clk_get has succeeded, so it
>> > should be moved up above the call to clk_put.
>> []
>> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>> []
>> > @@ -978,8 +978,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>> > failed_map:
>> > release_mem_region(mem->start, mem_size);
>> > failed_req:
>> > - clk_put(clk);
>> > failed_get:
>> > + clk_put(clk);
>> > failed_clock:
>> > return err;
>>
>> If this is correct, it might be better to rename all the
>> uses of failed_req to failed_get and delete label failed_req.
>
> As I replied to Walter Harms, I believe it is correct, because I don't see
> anything in the called functions that needs to be freed. I can redo the
> patch to eliminate the label.
Please do.
^ permalink raw reply
* Re: [patch net-next-2.6 v2] bonding: allow resetting slave failure counters
From: David Miller @ 2011-06-01 20:12 UTC (permalink / raw)
To: jpirko; +Cc: nicolas.2p.debian, fubar, fbl, netdev, andy
In-Reply-To: <20110601200854.GD2784@psychotron.redhat.com>
From: Jiri Pirko <jpirko@redhat.com>
Date: Wed, 1 Jun 2011 22:08:55 +0200
> Wed, Jun 01, 2011 at 09:23:11PM CEST, nicolas.2p.debian@gmail.com wrote:
>>Le 01/06/2011 18:31, Jiri Pirko a écrit :
>>>This patch allows to reset failure counters for all enslaved devices.
>>
>>Hi Jiri,
>>
>>Why do we need a way to reset those counters? What is the problem
>>with having those counters monotonically increase until the system is
>>rebooted? Do we have a way to reset other network statistics
>>(/sys/class/net/eth0/statistics/* for example)?
>
> Well, it's handy to be able to clear this counters when you resolve
> a problem so future issues are crearly seen as non-zero.
We don't allow this in any way for netdev counters, for good reasons,
so please don't add things like this.
^ permalink raw reply
* Re: [PATCH 4/10] drivers/net/can/flexcan.c: add missing clk_put
From: Julia Lawall @ 2011-06-01 20:13 UTC (permalink / raw)
To: David Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w, joe-6d6DIl74uiNBDgjK7y7TUQ,
wg-5Yr1BZd7O62+XT7JhA+gdA
In-Reply-To: <20110601.131136.391831410750786951.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Wed, 1 Jun 2011, David Miller wrote:
> From: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
> Date: Wed, 1 Jun 2011 22:08:46 +0200 (CEST)
>
> > On Wed, 1 Jun 2011, Joe Perches wrote:
> >
> >> On Wed, 2011-06-01 at 19:10 +0200, Julia Lawall wrote:
> >> > The failed_get label is used after the call to clk_get has succeeded, so it
> >> > should be moved up above the call to clk_put.
> >> []
> >> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> >> []
> >> > @@ -978,8 +978,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
> >> > failed_map:
> >> > release_mem_region(mem->start, mem_size);
> >> > failed_req:
> >> > - clk_put(clk);
> >> > failed_get:
> >> > + clk_put(clk);
> >> > failed_clock:
> >> > return err;
> >>
> >> If this is correct, it might be better to rename all the
> >> uses of failed_req to failed_get and delete label failed_req.
> >
> > As I replied to Walter Harms, I believe it is correct, because I don't see
> > anything in the called functions that needs to be freed. I can redo the
> > patch to eliminate the label.
>
> Please do.
OK, I will do it tomorrow.
julia
^ permalink raw reply
* Re: [PATCH net-next 0/11] bnx2x: Few link changes
From: David Miller @ 2011-06-01 20:14 UTC (permalink / raw)
To: yanivr; +Cc: netdev, eilong
In-Reply-To: <1306913126.20872.107.camel@lb-tlvb-dmitry>
From: "Yaniv Rosner" <yanivr@broadcom.com>
Date: Wed, 1 Jun 2011 10:25:26 +0300
> I'm resending this patch series describing some link changes.
> Please consider applying it to net-next.
All applied, thank you.
^ permalink raw reply
* Re: [PATCH] ethtool: Pause frame reporting fixes
From: Ben Hutchings @ 2011-06-01 20:17 UTC (permalink / raw)
To: Esa-Pekka Pyökkimies; +Cc: netdev@vger.kernel.org
In-Reply-To: <42186456B4D1324A9EB53C74DBCBB1BB0657DE@HKI-EXC-1.stonesoft.com>
On Tue, 2011-05-24 at 10:58 +0000, Esa-Pekka Pyökkimies wrote:
> Hello. Our software needs to modify pause parameters dynamically.
> For this to work reliably the SUPPORTED_Pause and SUPPORTED_Asym_Pause
> flags need to be set correctly by the driver. As it turns out, some drivers
> leave them at zero even though they implement pause frames.
> This patch adds reporting for these flags to ethtool.
> For some reason the current code reports ADVERTISED_Pause
> but not SUPPORTED_Pause.
Yes, that seems to be a bug.
Note that some drivers do change pause advertising flags in response to
the ETHTOOL_SPAUSEPARAM command (ethtool -A) if the command enables
autoneg.
> Also there was a small bug
> in the reporting for ADVERTISED_Pause.
No, it matches table 28B-2 in IEEE 802.3 (though with different
terminology).
> Esa-Pekka Pyokkimies, Software Specialist, Stonesoft
Any patches for ethtool need to include a Developer's Certificate of
Origin, as for the kernel - see
<http://www.kernel.org/doc/Documentation/SubmittingPatches>.
Please resubmit with the 'supported' flags decoded in conformance with
IEEE 802.3.
Ben.
> ethtool.c | 19 ++++++++++++++++---
> 1 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/ethtool.c b/ethtool.c
> index 34fe107..cf6c4b2 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -1193,6 +1193,19 @@ static void dump_supported(struct ethtool_cmd *ep)
> did1++; fprintf(stdout, "10000baseT/Full ");
> }
> fprintf(stdout, "\n");
> +
> + fprintf(stdout, " Supported pause frame use: ");
> + if (mask & SUPPORTED_Pause) {
> + if (mask & SUPPORTED_Asym_Pause)
> + fprintf(stdout, "Receive-only\n");
> + else
> + fprintf(stdout, "Symmetric\n");
> + } else {
> + if (mask & SUPPORTED_Asym_Pause)
> + fprintf(stdout, "Transmit-only\n");
> + else
> + fprintf(stdout, "No\n");
> + }
>
> fprintf(stdout, " Supports auto-negotiation: ");
> if (mask & SUPPORTED_Autoneg)
> @@ -1255,10 +1268,10 @@ static void dump_advertised(struct ethtool_cmd *ep,
>
> fprintf(stdout, " %s pause frame use: ", prefix);
> if (mask & ADVERTISED_Pause) {
> - fprintf(stdout, "Symmetric");
> if (mask & ADVERTISED_Asym_Pause)
> - fprintf(stdout, " Receive-only");
> - fprintf(stdout, "\n");
> + fprintf(stdout, "Receive-only\n");
> + else
> + fprintf(stdout, "Symmetric\n");
> } else {
> if (mask & ADVERTISED_Asym_Pause)
> fprintf(stdout, "Transmit-only\n");
--
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 for ixgbe-3.3.9 (out-of-tree) driver.
From: Alexander Duyck @ 2011-06-01 20:17 UTC (permalink / raw)
To: Ben Greear; +Cc: netdev
In-Reply-To: <4DE67BE1.3060704@candelatech.com>
On 06/01/2011 10:50 AM, Ben Greear wrote:
> Using 'ethtool -t ethX' with 82598 chipset causes
> kernel splats because spin-lock wasn't initialized properly.
> This can probably cause other problems, but I'm not certain
> of that.
>
> I can't tell that anyone actually watches the sourceforge list,
> so posting it here...
>
> The reason I'm using the out-of-tree driver is that a customer
> reports link bouncing when using the 82598 NIC. They are stuck
> on a .34 kernel at the moment, so maybe it's fixed upstream already.
> At any rate, I can't reproduce the problem locally, but the 3.3.9
> driver appears to work for them.
>
> Thanks,
> Ben
>
Ben,
I will make certain this fix is incorporated into our ixgbe out-of-tree
driver for the next release.
Thanks for the patch,
Alex
^ permalink raw reply
* [PATCH] net: dm9000: Get the chip in a known good state before enabling interrupts
From: Mark Brown @ 2011-06-01 20:18 UTC (permalink / raw)
To: David S. Miller, Ben Dooks; +Cc: netdev, Mark Brown
Currently the DM9000 driver requests the primary interrupt before it
resets the chip and puts it into a known good state. This means that if
the chip is asserting interrupt for some reason we can end up with a
screaming IRQ that the interrupt handler is unable to deal with. Avoid
this by only requesting the interrupt after we've reset the chip so we
know what state it's in.
This started manifesting itself on one of my boards in the past month or
so, I suspect as a result of some core infrastructure changes removing
some form of mitigation against bad behaviour here, even when things boot
it seems that the new code brings the interface up more quickly.
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
drivers/net/dm9000.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dm9000.c b/drivers/net/dm9000.c
index fbaff35..ee597e6 100644
--- a/drivers/net/dm9000.c
+++ b/drivers/net/dm9000.c
@@ -1157,9 +1157,6 @@ dm9000_open(struct net_device *dev)
irqflags |= IRQF_SHARED;
- if (request_irq(dev->irq, dm9000_interrupt, irqflags, dev->name, dev))
- return -EAGAIN;
-
/* GPIO0 on pre-activate PHY, Reg 1F is not set by reset */
iow(db, DM9000_GPR, 0); /* REG_1F bit0 activate phyxcer */
mdelay(1); /* delay needs by DM9000B */
@@ -1168,6 +1165,9 @@ dm9000_open(struct net_device *dev)
dm9000_reset(db);
dm9000_init_dm9000(dev);
+ if (request_irq(dev->irq, dm9000_interrupt, irqflags, dev->name, dev))
+ return -EAGAIN;
+
/* Init driver variable */
db->dbug_cnt = 0;
--
1.7.5.3
^ permalink raw reply related
* Re: [patch net-next-2.6] bonding: allow resetting slave failure counters
From: Flavio Leitner @ 2011-06-01 20:22 UTC (permalink / raw)
To: David Miller; +Cc: fubar, jpirko, netdev, andy
In-Reply-To: <20110601.122240.1431839726613846748.davem@davemloft.net>
On 06/01/2011 04:22 PM, David Miller wrote:
> From: Flavio Leitner <fbl@redhat.com>
> Date: Wed, 01 Jun 2011 16:11:07 -0300
>
>> On 06/01/2011 04:03 PM, David Miller wrote:
>>> From: Jay Vosburgh <fubar@us.ibm.com>
>>> Date: Wed, 01 Jun 2011 09:13:39 -0700
>>>
>>>> The "this dingus was added in version X.Y.Z" is there because
>>>> users sometimes read the most recent version of the documentation (that
>>>> they get from the internet) and then would become confused when their
>>>> older distro driver lacked some option described in the documentation.
>>>
>>> I disagree with this whole concept, because distros backport features
>>> like this into their kernel and therefore the feature is showing up in
>>> version X.Y.$(Z-20).
>>
>> It doesn't matter the version if the user can find the feature, so
>> distros backporting features works and that info is not useful at all.
>> However, when the user doesn't find the feature and search the internet,
>> then that info is helpful.
>
> So how is the user going to find that FC14 has the feature even
> though his FC13 kernel does not?
>
> I'll say it again, this version stuff is completely pointless.
>
> If the user is dabbling with upstream kernels he's a minority,
> and clueful enough to figure out this stuff himself.
If the distro's bonding version is updated accordingly then
you have something to compare. Anyway, I agree that it seems
more confusing than helpful.
fbl
^ permalink raw reply
* Re: [patch net-next-2.6 v2] bonding: allow resetting slave failure counters
From: Jiri Pirko @ 2011-06-01 20:27 UTC (permalink / raw)
To: David Miller; +Cc: nicolas.2p.debian, fubar, fbl, netdev, andy
In-Reply-To: <20110601.131243.2063791013719464193.davem@davemloft.net>
Wed, Jun 01, 2011 at 10:12:43PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jpirko@redhat.com>
>Date: Wed, 1 Jun 2011 22:08:55 +0200
>
>> Wed, Jun 01, 2011 at 09:23:11PM CEST, nicolas.2p.debian@gmail.com wrote:
>>>Le 01/06/2011 18:31, Jiri Pirko a écrit :
>>>>This patch allows to reset failure counters for all enslaved devices.
>>>
>>>Hi Jiri,
>>>
>>>Why do we need a way to reset those counters? What is the problem
>>>with having those counters monotonically increase until the system is
>>>rebooted? Do we have a way to reset other network statistics
>>>(/sys/class/net/eth0/statistics/* for example)?
>>
>> Well, it's handy to be able to clear this counters when you resolve
>> a problem so future issues are crearly seen as non-zero.
>
>We don't allow this in any way for netdev counters, for good reasons,
>so please don't add things like this.
>
Well technically this is not netdev counter directly. But fair enough.
Thanks.
Jirka
^ permalink raw reply
* [patch net-next-2.6] bonding: allow all slave speeds
From: Jiri Pirko @ 2011-06-01 20:36 UTC (permalink / raw)
To: netdev; +Cc: davem, fubar, andy
No need to check for 10, 100, 1000, 10000 explicitly. Just make this
generic and check for invalid values only (similar check is in ethtool
userspace app). This enables correct speed handling for slave devices
with "nonstandard" speeds.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/bonding/bond_main.c | 9 +--------
1 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 17b4dd9..716c852 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -629,15 +629,8 @@ static int bond_update_speed_duplex(struct slave *slave)
return -1;
slave_speed = ethtool_cmd_speed(&etool);
- switch (slave_speed) {
- case SPEED_10:
- case SPEED_100:
- case SPEED_1000:
- case SPEED_10000:
- break;
- default:
+ if (slave_speed == 0 || slave_speed == ((__u32) -1))
return -1;
- }
switch (etool.duplex) {
case DUPLEX_FULL:
--
1.7.4.4
^ permalink raw reply related
* Re: [BUG] net: cpu offline cause napi stall
From: Heiko Carstens @ 2011-06-01 20:42 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Frank Blaschka, davem, netdev, linux-s390
In-Reply-To: <1306958592.3946.0.camel@edumazet-laptop>
On Wed, Jun 01, 2011 at 10:03:12PM +0200, Eric Dumazet wrote:
> Le mercredi 01 juin 2011 à 20:12 +0200, Heiko Carstens a écrit :
> > On Wed, Jun 01, 2011 at 06:55:21PM +0200, Eric Dumazet wrote:
> > > > + /* Append NAPI poll list from offline CPU. */
> > > > + list_splice_init(&oldsd->poll_list, &sd->poll_list);
> > > >
> > > > raise_softirq_irqoff(NET_TX_SOFTIRQ);
> > > > local_irq_enable();
> > >
> > > Please make sure we raise NET_RX_SOFTIRQ on new cpu if necessary.
> >
> > Well, see two lines below the list_splice_init() call ;)
>
> I see nothing... NET_TX_SOFTIRQ and NET_RX_SOFTIRQ are not the same
Indeed. I must be blind.
diff --git a/net/core/dev.c b/net/core/dev.c
index 6561021..6189dac 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5981,6 +5981,11 @@ static int dev_cpu_callback(struct notifier_block *nfb,
oldsd->output_queue = NULL;
oldsd->output_queue_tailp = &oldsd->output_queue;
}
+ /* Append NAPI poll list from offline CPU. */
+ if (!list_empty(&oldsd->poll_list)) {
+ list_splice_init(&oldsd->poll_list, &sd->poll_list);
+ raise_softirq_irqoff(NET_RX_SOFTIRQ);
+ }
raise_softirq_irqoff(NET_TX_SOFTIRQ);
local_irq_enable();
^ permalink raw reply related
* Re: [RFC] Moving files around in drivers/net
From: Ben Hutchings @ 2011-06-01 21:07 UTC (permalink / raw)
To: Joe Perches
Cc: netdev, David Miller, Paul Gortmaker, Jan Engelhardt,
Jeffrey Kirsher
In-Reply-To: <1306956602.32125.28.camel@Joe-Laptop>
On Wed, 2011-06-01 at 12:30 -0700, Joe Perches wrote:
> Does anyone still think moving files around in drivers/net
> would be sensible and a suitable candidate for inclusion
> in 3.1?
+1
--
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
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