* sfc: an enumeration is not a bitmask
From: David Miller @ 2011-05-17 18:14 UTC (permalink / raw)
To: bhutchings; +Cc: netdev
Ben can you please get rid of "enum efx_fc_type"?
drivers/net/sfc/mcdi_mac.c: In function ‘efx_mcdi_set_mac’:
drivers/net/sfc/mcdi_mac.c:36:2: warning: case value ‘3’ not in enumerated type ‘enum efx_fc_type’
An enumeration is not a bitmask, instead it means one out of the set
of enumerated values will be used. This means that the warning
here about:
switch (efx->wanted_fc) {
case EFX_FC_RX | EFX_FC_TX:
is completely legitimate.
Thanks.
^ permalink raw reply
* Re: sfc: an enumeration is not a bitmask
From: Ben Hutchings @ 2011-05-17 18:18 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110517.141446.140687548350861625.davem@davemloft.net>
On Tue, 2011-05-17 at 14:14 -0400, David Miller wrote:
> Ben can you please get rid of "enum efx_fc_type"?
>
> drivers/net/sfc/mcdi_mac.c: In function ‘efx_mcdi_set_mac’:
> drivers/net/sfc/mcdi_mac.c:36:2: warning: case value ‘3’ not in enumerated type ‘enum efx_fc_type’
>
> An enumeration is not a bitmask, instead it means one out of the set
> of enumerated values will be used. This means that the warning
> here about:
>
> switch (efx->wanted_fc) {
> case EFX_FC_RX | EFX_FC_TX:
>
> is completely legitimate.
I think this is common practice in C. I filed a bug regarding the
warning at <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46457>, which
has not yet been resolved either way.
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 0/2] netfilter: SIP conntrack fixes
From: David Miller @ 2011-05-17 18:18 UTC (permalink / raw)
To: kaber; +Cc: netfilter-devel, netdev
In-Reply-To: <1305610014-3056-1-git-send-email-kaber@trash.net>
From: kaber@trash.net
Date: Tue, 17 May 2011 07:26:52 +0200
> following are two fixes for the SIP connection tracking helper:
>
> - missing validation of the Content-Length field, which is used to calculate
> the end of the SDP body
>
> - incorrect parsing of the SIP message, resulting in a failure to locate
> the SDP body when the Content-Length field is not the last member of the
> SIP message
>
> Please apply or pull from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/kaber/nf-2.6.git master
Pulled, thanks Patrick.
^ permalink raw reply
* Re: sfc: an enumeration is not a bitmask
From: David Miller @ 2011-05-17 18:23 UTC (permalink / raw)
To: bhutchings; +Cc: netdev
In-Reply-To: <1305656319.2848.28.camel@bwh-desktop>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 17 May 2011 19:18:39 +0100
> On Tue, 2011-05-17 at 14:14 -0400, David Miller wrote:
>> Ben can you please get rid of "enum efx_fc_type"?
>>
>> drivers/net/sfc/mcdi_mac.c: In function ‘efx_mcdi_set_mac’:
>> drivers/net/sfc/mcdi_mac.c:36:2: warning: case value ‘3’ not in enumerated type ‘enum efx_fc_type’
>>
>> An enumeration is not a bitmask, instead it means one out of the set
>> of enumerated values will be used. This means that the warning
>> here about:
>>
>> switch (efx->wanted_fc) {
>> case EFX_FC_RX | EFX_FC_TX:
>>
>> is completely legitimate.
>
> I think this is common practice in C. I filed a bug regarding the
> warning at <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46457>, which
> has not yet been resolved either way.
I've been seeing this warning for at least a year, it could take yet
another year before it's "resolved" in a way that people will see the
warning go away on their computers, more likely longer.
I'm not waiting a year, or more, for something as trivial as this
warning to get cured, please fix this the way that I asked.
Thanks.
^ permalink raw reply
* [PATCH] sk-filter: Rate-limit WARNing, print dbg info.
From: greearb @ 2011-05-17 18:30 UTC (permalink / raw)
To: netdev; +Cc: Ben Greear
From: Ben Greear <greearb@candelatech.com>
A mis-configured filter can spam the logs with
lots of stack traces. Rate-limit the warnings
and add printout of the bogus filter information.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 afc5837... 8249745... M net/core/filter.c
net/core/filter.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index afc5837..8249745 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -409,7 +409,13 @@ load_b:
continue;
}
default:
- WARN_ON(1);
+ if (net_ratelimit()) {
+ pr_err("filter: Unknown code: %hu jt: %u tf: %u"
+ " k: %u\n",
+ fentry->code, (unsigned int)(fentry->jt),
+ (unsigned int)(fentry->jf), fentry->k);
+ WARN_ON(1);
+ }
return 0;
}
}
--
1.7.3.4
^ permalink raw reply related
* small RPS cache for fragments?
From: David Miller @ 2011-05-17 18:33 UTC (permalink / raw)
To: netdev
It seems to me that we can solve the UDP fragmentation problem for
flow steering very simply by creating a (saddr/daddr/IPID) entry in a
table that maps to the corresponding RPS flow entry.
When we see the initial frag with the UDP header, we create the
saddr/daddr/IPID mapping, and we tear it down when we hit the
saddr/daddr/IPID mapping and the packet has the IP_MF bit clear.
We only inspect the saddr/daddr/IPID cache when iph->frag_off is
non-zero.
It's best effort and should work quite well.
Even a one-behind cache, per-NAPI instance, would do a lot better than
what happens at the moment. Especially since the IP fragments mostly
arrive as one packet train.
^ permalink raw reply
* [PATCH net-next-2.6] sfc: Suppress warning about combining enum flags by gcc 4.5
From: Ben Hutchings @ 2011-05-17 18:48 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110517.142350.81148594119844192.davem@davemloft.net>
gcc 4.5 warns about switch statements on enumerated types containing
case values that are a bitwise-or of two enumerators for that type.
The use of enumerators as flags is common practice, so I think the
warning is wrong. Keep the compiler quiet by casting the switch
value.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
Will this do?
Ben.
drivers/net/sfc/mcdi_mac.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/sfc/mcdi_mac.c b/drivers/net/sfc/mcdi_mac.c
index 50c2077..316745b 100644
--- a/drivers/net/sfc/mcdi_mac.c
+++ b/drivers/net/sfc/mcdi_mac.c
@@ -32,7 +32,7 @@ static int efx_mcdi_set_mac(struct efx_nic *efx)
(1 << MC_CMD_SET_MAC_IN_REJECT_UNCST_LBN);
MCDI_SET_DWORD(cmdbytes, SET_MAC_IN_REJECT, reject);
- switch (efx->wanted_fc) {
+ switch ((unsigned)efx->wanted_fc) {
case EFX_FC_RX | EFX_FC_TX:
fcntl = MC_CMD_FCNTL_BIDIR;
break;
--
1.7.4
--
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 related
* Re: ath5k regression associating with APs in 2.6.38
From: Seth Forshee @ 2011-05-17 18:50 UTC (permalink / raw)
To: Nick Kossifidis
Cc: John W. Linville, Jiri Slaby, Luis R. Rodriguez, Bob Copeland,
linux-wireless, ath5k-devel, netdev, linux-kernel
In-Reply-To: <BANLkTi=8ZRUVWn3FLAMtPh=4yY1F0k6i9w@mail.gmail.com>
On Tue, May 17, 2011 at 08:14:34PM +0300, Nick Kossifidis wrote:
> 2011/5/17 Seth Forshee <seth.forshee@canonical.com>:
> > On Mon, May 09, 2011 at 09:02:30AM +0200, Seth Forshee wrote:
> >> On Thu, May 05, 2011 at 05:30:42PM +0300, Nick Kossifidis wrote:
> >> > Hmm I don't see any errors from reset/phy code, can you disable
> >> > Network Manager/wpa-supplicant and test connection on an open network
> >> > using iw ? It 'll give us a better picture...
> >> >
> >> > If iw doesn't return any scan results we are probably hitting a PHY/RF
> >> > error specific to your device (not all vendors follow the reference
> >> > design). Maybe we should follow a blacklist/whitelist approach for
> >> > this feature.
> >>
> >> I got the results back from my tester. He was able to get scan results,
> >> but it took multiple tries and the direct probe failures appear in the
> >> log. He didn't enable ATH5K_DEBUG_RESET this time; let me know if you
> >> need that and I'll request he retest with the extra debug logs enabled.
> >
> > I got some more feedback. Most of the time iw does not get scan results,
> > but even when it does connecting to the AP isn't always successful. The
> > tester did note that he doesn't seem to have any trouble if his machine
> > is within a few feet of his AP. Let me know if you'd like something else
> > tested.
> >
> > I noticed that bugzilla #31922 (ath5k: Decreased throughput in IBSS or
> > 802.11n mode) is also fixed by reverting 8aec7af9. It seems like the
> > synth-only channel changes are resulting in poor connection quality.
> > Maybe that patch needs to be reverted?
> >
> > Thanks,
> > Seth
> >
> >
>
> http://www.kernel.org/pub/linux/kernel/people/mickflemm/01-fast-chan-switch-modparm
That looks like it should do the trick. I'll request some testing with
it and let you know how it goes. Thanks!
^ permalink raw reply
* Re: [PATCH] sk-filter: Rate-limit WARNing, print dbg info.
From: Joe Perches @ 2011-05-17 18:52 UTC (permalink / raw)
To: greearb; +Cc: netdev
In-Reply-To: <1305657014-32736-1-git-send-email-greearb@candelatech.com>
On Tue, 2011-05-17 at 11:30 -0700, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> A mis-configured filter can spam the logs with
> lots of stack traces. Rate-limit the warnings
> and add printout of the bogus filter information.
> - WARN_ON(1);
> + if (net_ratelimit()) {
> + pr_err("filter: Unknown code: %hu jt: %u tf: %u"
> + " k: %u\n",
> + fentry->code, (unsigned int)(fentry->jt),
> + (unsigned int)(fentry->jf), fentry->k);
> + WARN_ON(1);
> + }
Maybe just using WARN is better.
I believe the casts aren't necessary.
net/core/filter.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index 0eb8c44..5b967d0 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -350,7 +350,10 @@ load_b:
continue;
}
default:
- WARN_ON(1);
+ if (net_ratelimit())
+ WARN(1, "Unknown code:%u jt:%u tf:%u k:%u\n",
+ fentry->code, fentry->jt,
+ fentry->jf, fentry->k);
return 0;
}
}
^ permalink raw reply related
* Re: sfc: an enumeration is not a bitmask
From: Jeff Garzik @ 2011-05-17 18:57 UTC (permalink / raw)
To: David Miller; +Cc: bhutchings, netdev
In-Reply-To: <20110517.141446.140687548350861625.davem@davemloft.net>
2011/5/17 David Miller <davem@davemloft.net>:
> An enumeration is not a bitmask, instead it means one out of the set
> of enumerated values will be used.
It's a decade-old kernel practice to use 'enum' to define typed
constants, preferred over macros that convey no type information and
disappear after cpp phase.
So your assertion about enumerations is demonstrably not true, as it
is often used in the kernel. Call it enum abuse if you want, but it
is consistent with code all over the kernel.
That said, I agree that warnings should of course be addressed in some manner.
Jeff
^ permalink raw reply
* Re: sfc: an enumeration is not a bitmask
From: Michał Mirosław @ 2011-05-17 19:09 UTC (permalink / raw)
To: Jeff Garzik; +Cc: David Miller, bhutchings, netdev
In-Reply-To: <BANLkTi=n_0JiF0Lm=+1OwRzB4XQFBtg8KA@mail.gmail.com>
2011/5/17 Jeff Garzik <jgarzik@pobox.com>:
> 2011/5/17 David Miller <davem@davemloft.net>:
>> An enumeration is not a bitmask, instead it means one out of the set
>> of enumerated values will be used.
>
> It's a decade-old kernel practice to use 'enum' to define typed
> constants, preferred over macros that convey no type information and
> disappear after cpp phase.
>
> So your assertion about enumerations is demonstrably not true, as it
> is often used in the kernel. Call it enum abuse if you want, but it
> is consistent with code all over the kernel.
>
> That said, I agree that warnings should of course be addressed in some manner.
Old age of the mistake doesn't make it correct.
Disappearance of the #defines can be resolved by using enum of bit
positions (and maybe field lengths) and #define of (1 << bit_position)
if it is useful for something to remain after preprocessing.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: sfc: an enumeration is not a bitmask
From: Ben Hutchings @ 2011-05-17 19:22 UTC (permalink / raw)
To: Michał Mirosław; +Cc: Jeff Garzik, David Miller, netdev
In-Reply-To: <BANLkTinaO0VVGpRRqRauH6FucvwbcXyRUg@mail.gmail.com>
On Tue, 2011-05-17 at 21:09 +0200, Michał Mirosław wrote:
> 2011/5/17 Jeff Garzik <jgarzik@pobox.com>:
> > 2011/5/17 David Miller <davem@davemloft.net>:
> >> An enumeration is not a bitmask, instead it means one out of the set
> >> of enumerated values will be used.
> >
> > It's a decade-old kernel practice to use 'enum' to define typed
> > constants, preferred over macros that convey no type information and
> > disappear after cpp phase.
> >
> > So your assertion about enumerations is demonstrably not true, as it
> > is often used in the kernel. Call it enum abuse if you want, but it
> > is consistent with code all over the kernel.
> >
> > That said, I agree that warnings should of course be addressed in some manner.
>
> Old age of the mistake doesn't make it correct.
>
> Disappearance of the #defines can be resolved by using enum of bit
> positions (and maybe field lengths) and #define of (1 << bit_position)
> if it is useful for something to remain after preprocessing.
The point is that there is no specific type information for macros,
whether they are simple literal numbers or left-shift expressions.
The type of an enumerator in C is actually that of the underlying
integer type, not the enumeration type as in C++. However, a compiler
or static analysis tool (such as sparse) may keep track of both the
language-specified type and some other associated type of an expression
in order to diagnose possible type errors.
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] sk-filter: Rate-limit WARNing, print dbg info.
From: David Miller @ 2011-05-17 19:23 UTC (permalink / raw)
To: joe; +Cc: greearb, netdev
In-Reply-To: <1305658349.1722.30.camel@Joe-Laptop>
From: Joe Perches <joe@perches.com>
Date: Tue, 17 May 2011 11:52:29 -0700
> On Tue, 2011-05-17 at 11:30 -0700, greearb@candelatech.com wrote:
>> From: Ben Greear <greearb@candelatech.com>
>> A mis-configured filter can spam the logs with
>> lots of stack traces. Rate-limit the warnings
>> and add printout of the bogus filter information.
>> - WARN_ON(1);
>> + if (net_ratelimit()) {
>> + pr_err("filter: Unknown code: %hu jt: %u tf: %u"
>> + " k: %u\n",
>> + fentry->code, (unsigned int)(fentry->jt),
>> + (unsigned int)(fentry->jf), fentry->k);
>> + WARN_ON(1);
>> + }
>
> Maybe just using WARN is better.
Yep, it looks better to me.
> I believe the casts aren't necessary.
Also correct.
^ permalink raw reply
* Re: [PATCH net-next-2.6] sfc: Suppress warning about combining enum flags by gcc 4.5
From: David Miller @ 2011-05-17 19:26 UTC (permalink / raw)
To: bhutchings; +Cc: netdev
In-Reply-To: <1305658121.2848.29.camel@bwh-desktop>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 17 May 2011 19:48:41 +0100
> gcc 4.5 warns about switch statements on enumerated types containing
> case values that are a bitwise-or of two enumerators for that type.
> The use of enumerators as flags is common practice, so I think the
> warning is wrong. Keep the compiler quiet by casting the switch
> value.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> Will this do?
What part of "get rid of the enum" is so hard to understand?
I'll say it again: Please get rid of the enum efx_fc_type
You can define bit positions if you like, because those will
be used as a proper enum, as unique bit positions within
a set of bits. Then you can define macros as (1 << XXX)
Thanks.
^ permalink raw reply
* Re: [PATCH] sk-filter: Rate-limit WARNing, print dbg info.
From: Ben Greear @ 2011-05-17 19:34 UTC (permalink / raw)
To: David Miller; +Cc: joe, netdev
In-Reply-To: <20110517.152357.85105240786269960.davem@davemloft.net>
On 05/17/2011 12:23 PM, David Miller wrote:
> From: Joe Perches<joe@perches.com>
> Date: Tue, 17 May 2011 11:52:29 -0700
>
>> On Tue, 2011-05-17 at 11:30 -0700, greearb@candelatech.com wrote:
>>> From: Ben Greear<greearb@candelatech.com>
>>> A mis-configured filter can spam the logs with
>>> lots of stack traces. Rate-limit the warnings
>>> and add printout of the bogus filter information.
>>> - WARN_ON(1);
>>> + if (net_ratelimit()) {
>>> + pr_err("filter: Unknown code: %hu jt: %u tf: %u"
>>> + " k: %u\n",
>>> + fentry->code, (unsigned int)(fentry->jt),
>>> + (unsigned int)(fentry->jf), fentry->k);
>>> + WARN_ON(1);
>>> + }
>>
>> Maybe just using WARN is better.
>
> Yep, it looks better to me.
You want to just take his, or shall I re-spin it?
Either is fine with me.
Thanks,
Ben
>
>> I believe the casts aren't necessary.
>
> Also correct.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH] net: recvmmsg: Strip MSG_WAITFORONE when calling recvmsg
From: David Miller @ 2011-05-17 19:38 UTC (permalink / raw)
To: anton; +Cc: acme, netdev
In-Reply-To: <20110517085924.3a47b8e2@kryten>
From: Anton Blanchard <anton@samba.org>
Date: Tue, 17 May 2011 08:59:24 +1000
>
> recvmmsg fails on a raw socket with EINVAL. The reason for this is
> packet_recvmsg checks the incoming flags:
>
> err = -EINVAL;
> if (flags & ~(MSG_PEEK|MSG_DONTWAIT|MSG_TRUNC|MSG_CMSG_COMPAT|MSG_ERRQUEUE))
> goto out;
>
> This patch strips out MSG_WAITFORONE when calling recvmmsg which
> fixes the issue.
>
> Signed-off-by: Anton Blanchard <anton@samba.org>
> Cc: stable@kernel.org [2.6.34+]
> ---
>
> We could also add MSG_WAITFORONE to the raw socket recvmsg check, or
> just remove the check completely. Thoughts?
I think both should be done.
Tests like the one packet_recvmsg() is doing makes it really hard to
add new MSG_* flags generically (ie. by adding the flag and having
some generic socket operation all recvmsg() implementations call
process that flag bit in some way).
It should just explicitly check for the bits it doesn't support.
And recvmmsg() should clear the bit because it is a completely
internal thing. It's similar to how we hide MSG_CMSG_COMPAT from
userspace.
Anyways I'll apply this patch, and feel free to submit the other
one.
Thanks.
^ permalink raw reply
* Re: [PATCH] sk-filter: Rate-limit WARNing, print dbg info.
From: David Miller @ 2011-05-17 19:39 UTC (permalink / raw)
To: greearb; +Cc: joe, netdev
In-Reply-To: <4DD2CDA9.7000009@candelatech.com>
From: Ben Greear <greearb@candelatech.com>
Date: Tue, 17 May 2011 12:34:01 -0700
> On 05/17/2011 12:23 PM, David Miller wrote:
>> From: Joe Perches<joe@perches.com>
>> Date: Tue, 17 May 2011 11:52:29 -0700
>>
>>> On Tue, 2011-05-17 at 11:30 -0700, greearb@candelatech.com wrote:
>>>> From: Ben Greear<greearb@candelatech.com>
>>>> A mis-configured filter can spam the logs with
>>>> lots of stack traces. Rate-limit the warnings
>>>> and add printout of the bogus filter information.
>>>> - WARN_ON(1);
>>>> + if (net_ratelimit()) {
>>>> + pr_err("filter: Unknown code: %hu jt: %u tf: %u"
>>>> + " k: %u\n",
>>>> + fentry->code, (unsigned int)(fentry->jt),
>>>> + (unsigned int)(fentry->jf), fentry->k);
>>>> + WARN_ON(1);
>>>> + }
>>>
>>> Maybe just using WARN is better.
>>
>> Yep, it looks better to me.
>
> You want to just take his, or shall I re-spin it?
Respin.
^ permalink raw reply
* Re: [PATCH] net: Change netdev_fix_features messages loglevel
From: David Miller @ 2011-05-17 19:45 UTC (permalink / raw)
To: mst; +Cc: mirq-linux, netdev, herbert, bhutchings
In-Reply-To: <20110516203739.GC18148@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Mon, 16 May 2011 23:37:39 +0300
> On Mon, May 16, 2011 at 03:14:34PM -0400, David Miller wrote:
>> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>> Date: Mon, 16 May 2011 15:17:57 +0200 (CEST)
>>
>> > Those reduced to DEBUG can possibly be triggered by unprivileged processes
>> > and are nothing exceptional. Illegal checksum combinations can only be
>> > caused by driver bug, so promote those messages to WARN.
>> >
>> > Since GSO without SG will now only cause DEBUG message from
>> > netdev_fix_features(), remove the workaround from register_netdevice().
>> >
>> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>>
>> Applied, thanks.
>
> Cool, how about we make 'Features changed' debug as well?
> This way userspace can't fill up the log just by tweaking tun features
> with an ioctl.
>
> Untested, but you get the message.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Agreed and applied.
I agree with Herbert's long term assertion that we should handle this
differently, somehow, to avoid these issues.
But as long as userspace can trigger these events, we have to do things
like decrease the logging level of these messages.
^ permalink raw reply
* Re: [PATCH] sk-filter: Rate-limit WARNing, print dbg info.
From: Joe Perches @ 2011-05-17 19:53 UTC (permalink / raw)
To: Ben Greear; +Cc: David Miller, netdev
In-Reply-To: <4DD2CDA9.7000009@candelatech.com>
On Tue, 2011-05-17 at 12:34 -0700, Ben Greear wrote:
> On 05/17/2011 12:23 PM, David Miller wrote:
> > From: Joe Perches<joe@perches.com>
> >> On Tue, 2011-05-17 at 11:30 -0700, greearb@candelatech.com wrote:
> >>> From: Ben Greear<greearb@candelatech.com>
> >>> A mis-configured filter can spam the logs with
> >>> lots of stack traces. Rate-limit the warnings
> >>> and add printout of the bogus filter information.
> >>> - WARN_ON(1);
> >>> + if (net_ratelimit()) {
> >>> + pr_err("filter: Unknown code: %hu jt: %u tf: %u"
> >>> + " k: %u\n",
> >>> + fentry->code, (unsigned int)(fentry->jt),
> >>> + (unsigned int)(fentry->jf), fentry->k);
> >>> + WARN_ON(1);
> >>> + }
> >> Maybe just using WARN is better.
> > Yep, it looks better to me.
> You want to just take his, or shall I re-spin it?
> Either is fine with me.
Another option is to add a WARN_ON_RATELIMIT
(there is already a WARN_RATELIMIT) to avoid
missing other messages.
Something like:
include/asm-generic/bug.h | 16 ++++++++++++++++
net/core/filter.c | 4 +++-
2 files changed, 19 insertions(+), 1 deletions(-)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index e5a3f58..12b250c 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -165,6 +165,22 @@ extern void warn_slowpath_null(const char *file, const int line);
#define WARN_ON_RATELIMIT(condition, state) \
WARN_ON((condition) && __ratelimit(state))
+#define __WARN_RATELIMIT(condition, state, format...) \
+({ \
+ int rtn = 0; \
+ if (unlikely(__ratelimit(state))) \
+ rtn = WARN(condition, format); \
+ rtn; \
+})
+
+#define WARN_RATELIMIT(condition, format...) \
+({ \
+ static DEFINE_RATELIMIT_STATE(_rs, \
+ DEFAULT_RATELIMIT_INTERVAL, \
+ DEFAULT_RATELIMIT_BURST); \
+ __WARN_RATELIMIT(condition, &_rs, format); \
+})
+
/*
* WARN_ON_SMP() is for cases that the warning is either
* meaningless for !SMP or may even cause failures.
diff --git a/net/core/filter.c b/net/core/filter.c
index 0eb8c44..0e3622f 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -350,7 +350,9 @@ load_b:
continue;
}
default:
- WARN_ON(1);
+ WARN_RATELIMIT(1, "Unknown code:%u jt:%u tf:%u k:%u\n",
+ fentry->code, fentry->jt,
+ fentry->jf, fentry->k);
return 0;
}
}
^ permalink raw reply related
* [PATCH v2] CDC NCM: release interfaces fix in unbind()
From: Alexey Orishko @ 2011-05-17 19:59 UTC (permalink / raw)
To: netdev, davem, oliver; +Cc: linux-usb, gregkh, Alexey Orishko
Changes:
- claim slave/data interface during bind() and release in
unbind() unconditionally
- in case of error during bind(), release claimed data
interface in the same function
- remove obsolited "*_claimed" entries from driver context
Signed-off-by: Alexey Orishko <alexey.orishko@stericsson.com>
---
drivers/net/usb/cdc_ncm.c | 66 +++++++++++---------------------------------
1 files changed, 17 insertions(+), 49 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4ab557d..7fa00d7 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -54,7 +54,7 @@
#include <linux/usb/usbnet.h>
#include <linux/usb/cdc.h>
-#define DRIVER_VERSION "06-May-2011"
+#define DRIVER_VERSION "17-May-2011"
/* CDC NCM subclass 3.2.1 */
#define USB_CDC_NCM_NDP16_LENGTH_MIN 0x10
@@ -134,8 +134,6 @@ struct cdc_ncm_ctx {
u16 tx_ndp_modulus;
u16 tx_seq;
u16 connected;
- u8 data_claimed;
- u8 control_claimed;
};
static void cdc_ncm_tx_timeout(unsigned long arg);
@@ -460,17 +458,6 @@ static void cdc_ncm_free(struct cdc_ncm_ctx *ctx)
del_timer_sync(&ctx->tx_timer);
- if (ctx->data_claimed) {
- usb_set_intfdata(ctx->data, NULL);
- usb_driver_release_interface(driver_of(ctx->intf), ctx->data);
- }
-
- if (ctx->control_claimed) {
- usb_set_intfdata(ctx->control, NULL);
- usb_driver_release_interface(driver_of(ctx->intf),
- ctx->control);
- }
-
if (ctx->tx_rem_skb != NULL) {
dev_kfree_skb_any(ctx->tx_rem_skb);
ctx->tx_rem_skb = NULL;
@@ -495,7 +482,7 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
if (ctx == NULL)
- goto error;
+ return -ENODEV;
memset(ctx, 0, sizeof(*ctx));
@@ -572,42 +559,32 @@ advance:
goto error;
/* claim interfaces, if any */
- if (ctx->data != intf) {
- temp = usb_driver_claim_interface(driver, ctx->data, dev);
- if (temp)
- goto error;
- ctx->data_claimed = 1;
- }
-
- if (ctx->control != intf) {
- temp = usb_driver_claim_interface(driver, ctx->control, dev);
- if (temp)
- goto error;
- ctx->control_claimed = 1;
- }
+ temp = usb_driver_claim_interface(driver, ctx->data, dev);
+ if (temp)
+ goto error;
iface_no = ctx->data->cur_altsetting->desc.bInterfaceNumber;
/* reset data interface */
temp = usb_set_interface(dev->udev, iface_no, 0);
if (temp)
- goto error;
+ goto error2;
/* initialize data interface */
if (cdc_ncm_setup(ctx))
- goto error;
+ goto error2;
/* configure data interface */
temp = usb_set_interface(dev->udev, iface_no, 1);
if (temp)
- goto error;
+ goto error2;
cdc_ncm_find_endpoints(ctx, ctx->data);
cdc_ncm_find_endpoints(ctx, ctx->control);
if ((ctx->in_ep == NULL) || (ctx->out_ep == NULL) ||
(ctx->status_ep == NULL))
- goto error;
+ goto error2;
dev->net->ethtool_ops = &cdc_ncm_ethtool_ops;
@@ -617,7 +594,7 @@ advance:
temp = usbnet_get_ethernet_addr(dev, ctx->ether_desc->iMACAddress);
if (temp)
- goto error;
+ goto error2;
dev_info(&dev->udev->dev, "MAC-Address: "
"0x%02x:0x%02x:0x%02x:0x%02x:0x%02x:0x%02x\n",
@@ -642,6 +619,10 @@ advance:
ctx->tx_speed = ctx->rx_speed = 0;
return 0;
+error2:
+ usb_set_intfdata(ctx->control, NULL);
+ usb_set_intfdata(ctx->data, NULL);
+ usb_driver_release_interface(driver, ctx->data);
error:
cdc_ncm_free((struct cdc_ncm_ctx *)dev->data[0]);
dev->data[0] = 0;
@@ -652,27 +633,14 @@ error:
static void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf)
{
struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
- struct usb_driver *driver;
+ struct usb_driver *driver = driver_of(intf);
if (ctx == NULL)
return; /* no setup */
- driver = driver_of(intf);
-
- usb_set_intfdata(ctx->data, NULL);
usb_set_intfdata(ctx->control, NULL);
- usb_set_intfdata(ctx->intf, NULL);
-
- /* release interfaces, if any */
- if (ctx->data_claimed) {
- usb_driver_release_interface(driver, ctx->data);
- ctx->data_claimed = 0;
- }
-
- if (ctx->control_claimed) {
- usb_driver_release_interface(driver, ctx->control);
- ctx->control_claimed = 0;
- }
+ usb_set_intfdata(ctx->data, NULL);
+ usb_driver_release_interface(driver, ctx->data);
cdc_ncm_free(ctx);
}
--
1.7.4.3
^ permalink raw reply related
* Re: small RPS cache for fragments?
From: Tom Herbert @ 2011-05-17 20:02 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110517.143342.1566027350038182221.davem@davemloft.net>
I like it! And this sounds like the sort of algorithm that NICs might
be able to implement to solve the UDP/RSS unpleasantness, so even
better.
Tom
On Tue, May 17, 2011 at 11:33 AM, David Miller <davem@davemloft.net> wrote:
>
> It seems to me that we can solve the UDP fragmentation problem for
> flow steering very simply by creating a (saddr/daddr/IPID) entry in a
> table that maps to the corresponding RPS flow entry.
>
> When we see the initial frag with the UDP header, we create the
> saddr/daddr/IPID mapping, and we tear it down when we hit the
> saddr/daddr/IPID mapping and the packet has the IP_MF bit clear.
>
> We only inspect the saddr/daddr/IPID cache when iph->frag_off is
> non-zero.
>
> It's best effort and should work quite well.
>
> Even a one-behind cache, per-NAPI instance, would do a lot better than
> what happens at the moment. Especially since the IP fragments mostly
> arrive as one packet train.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: Frequent spurious tx_timeouts for libertas
From: David Miller @ 2011-05-17 20:05 UTC (permalink / raw)
To: bhutchings; +Cc: dsd, netdev, libertas-dev, linux-wireless
In-Reply-To: <1304369259.2833.180.camel@localhost>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon, 02 May 2011 21:47:39 +0100
> On Mon, 2011-05-02 at 20:59 +0100, Daniel Drake wrote:
>> On 2 May 2011 03:24, Ben Hutchings <bhutchings@solarflare.com> wrote:
>> >> Also, while looking at this code, I spotted a bug in dev_watchdog():
>> >> /*
>> >> * old device drivers set dev->trans_start
>> >> */
>> >> trans_start = txq->trans_start ? : dev->trans_start;
>> >>
>> >> i.e. it is trying to figure out whether to read trans_start from txq
>> >> or dev. In both cases, trans_start is updated based on the value of
>> >> jiffies, which will occasionally be 0 (as it wraps around). Therefore
>> >> this line of code will occasionally make the wrong decision.
>> >
>> > No, I don't think so.
>> >
>> > If only dev->trans_start is being updated then the watchdog reads that.
>> > If both txq->trans_start and dev->trans_start are being updated then it
>> > doesn't matter much which the watchdog reads.
>> > If only txq->trans_start is being updated then dev->trans_start is
>> > always set to 0, so when txq->trans_start is 0 the watchdog still gets
>> > 0.
>>
>> dev->trans_start is unconditionally initialized by dev_activate() in
>> sch_generic.c:
>>
>> if (need_watchdog) {
>> dev->trans_start = jiffies;
>> dev_watchdog_up(dev);
>> }
>>
>> so it is (usually) not 0.
> [...]
>
> You're right. Seems like we have an incomplete compatibility hack that
> can hurt drivers that are doing the right thing.
>
> For those few single-queue drivers that need to update the transmit
> time, perhaps we could add a dev_trans_update() as a wrapper for
> txq_trans_update(). Then delete net_device::trans_start and change
> dev_trans_start() to avoid using it.
Even though this unconditional assignment exists, it should not cause
problems.
First, in dev_watchdog(), any non-zero txq->trans_start will be preferred
over dev->trans_start.
Second, in dev_trans_start(), netdev->trans_start is used as a baseline,
and any more recent stamp in txq->trans_start will be preferred.
In fact, this makes the assignment of netdev->trans_start to zero in
transition_one_qdisc() look erroneous.
^ permalink raw reply
* Re: small RPS cache for fragments?
From: Eric Dumazet @ 2011-05-17 20:14 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110517.143342.1566027350038182221.davem@davemloft.net>
Le mardi 17 mai 2011 à 14:33 -0400, David Miller a écrit :
> It seems to me that we can solve the UDP fragmentation problem for
> flow steering very simply by creating a (saddr/daddr/IPID) entry in a
> table that maps to the corresponding RPS flow entry.
>
> When we see the initial frag with the UDP header, we create the
> saddr/daddr/IPID mapping, and we tear it down when we hit the
> saddr/daddr/IPID mapping and the packet has the IP_MF bit clear.
>
> We only inspect the saddr/daddr/IPID cache when iph->frag_off is
> non-zero.
>
> It's best effort and should work quite well.
>
> Even a one-behind cache, per-NAPI instance, would do a lot better than
> what happens at the moment. Especially since the IP fragments mostly
> arrive as one packet train.
> --
OK but do we have workloads actually needing this optimization at all ?
(IP defrag hits a read_lock(&ip4_frags.lock)), so maybe steer all frags
on a given cpu ?)
^ permalink raw reply
* Re: small RPS cache for fragments?
From: Rick Jones @ 2011-05-17 20:17 UTC (permalink / raw)
To: Tom Herbert; +Cc: David Miller, netdev
In-Reply-To: <BANLkTikyH=q_6uOvFh3_Z_xwPST3zVijZw@mail.gmail.com>
On Tue, 2011-05-17 at 13:02 -0700, Tom Herbert wrote:
> I like it! And this sounds like the sort of algorithm that NICs might
> be able to implement to solve the UDP/RSS unpleasantness, so even
> better.
Do (m)any devices take "shortcuts" with UDP datagrams these days? By
that I mean that back in the day, the HP-PB and "Slider" FDDI
cards/drivers did checksum offload for fragmented UDP datagrams by
sending the first fragment, the one with the UDP header and thus
checksum, last. It did that to save space on the card and make use of
the checksum accumulator.
rick jones
>
> Tom
>
> On Tue, May 17, 2011 at 11:33 AM, David Miller <davem@davemloft.net> wrote:
> >
> > It seems to me that we can solve the UDP fragmentation problem for
> > flow steering very simply by creating a (saddr/daddr/IPID) entry in a
> > table that maps to the corresponding RPS flow entry.
> >
> > When we see the initial frag with the UDP header, we create the
> > saddr/daddr/IPID mapping, and we tear it down when we hit the
> > saddr/daddr/IPID mapping and the packet has the IP_MF bit clear.
> >
> > We only inspect the saddr/daddr/IPID cache when iph->frag_off is
> > non-zero.
> >
> > It's best effort and should work quite well.
> >
> > Even a one-behind cache, per-NAPI instance, would do a lot better than
> > what happens at the moment. Especially since the IP fragments mostly
> > arrive as one packet train.
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [RFC PATCH ethtool] ethtool: merge ETHTOOL_[GS]FEATURES support to -k/-K modes
From: Michał Mirosław @ 2011-05-17 20:33 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, David Miller
In-Reply-To: <20110517084543.GB18423@rere.qmqm.pl>
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
This depends on the previous patch introducing -w/-W for [GS]FEATURES.
BTW, I noticed an old bug in ethtool (present currently and in debian-lenny's
version 6+20080913-1): "ethtool -k" -- i.e. with no other parameters -- runs
and tries to check device named '-k'.
Example:
icybox:~# ./ethtool -k ge0
Offload parameters for ge0:
rx-checksumming: on
tx-checksumming: off
scatter-gather: off
tcp-segmentation-offload: off
udp-fragmentation-offload: off
generic-segmentation-offload: off
generic-receive-offload: on
large-receive-offload: off
rx-vlan-offload: off
tx-vlan-offload: off
ntuple-filters: off
receive-hashing: off
Full offload state: (feature-name: active,wanted,changable)
tx-scatter-gather no,yes,yes
tx-checksum-ipv4 no, no,yes
tx-checksum-unneeded no,---, no
tx-checksum-ip-generic no,---, no
tx_checksum-ipv6 no, no,yes
highdma no,---, no
tx-scatter-gather-fraglist no,---, no
tx-vlan-hw-insert no,---, no
rx-vlan-hw-parse no,---, no
rx-vlan-filter no,---, no
vlan-challenged no,---,---
tx-generic-segmentation no,yes,yes
tx-lockless no,---,---
netns-local no,---,---
rx-gro yes,yes,yes
rx-lro no,---, no
tx-tcp-segmentation no, no,yes
tx-udp-fragmentation no,---, no
tx-gso-robust no,---, no
tx-tcp-ecn-segmentation no, no,yes
tx-tcp6-segmentation no, no,yes
tx-fcoe-segmentation no,---, no
tx-checksum-fcoe-crc no,---, no
tx-checksum-sctp no,---, no
fcoe-mtu no,---, no
rx-ntuple-filter no,---, no
rx-hashing no,---, no
rx-checksum yes,yes,yes
tx-nocache-copy no, no,yes
loopback no,---, no
icybox:~# ./ethtool -K ge0 tx_checksum-ipv6 on
feature tx-scatter-gather is enabled (expected: disabled, saved: enabled)
feature tx-generic-segmentation is enabled (expected: disabled, saved: enabled)
icybox:~# ./ethtool -K ge0 sg off
[turns off SG and GSO; like before]
icybox:~# ./ethtool -K ge0 tx_checksum-ipv6 off
[no other feature changed state]
icybox:~# ./ethtool -K ge0 sg on
[SG was remembered this time, but is inactive (no checksum offloads)]
icybox:~# ./ethtool -K ge0 tx_checksum-ipv6 on
feature tx-scatter-gather is enabled (expected: disabled, saved: enabled)
feature tx-generic-segmentation is enabled (expected: disabled, saved: enabled)
icybox:~# ./ethtool -K ge0 tx_checksum-ipv6 off
feature tx-scatter-gather is disabled (expected: enabled, saved: enabled)
feature tx-generic-segmentation is disabled (expected: enabled, saved: enabled)
---
ethtool.c | 155 ++++++++++++++++++++++++++++--------------------------------
1 files changed, 72 insertions(+), 83 deletions(-)
diff --git a/ethtool.c b/ethtool.c
index 86a5a8b..a541007 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -123,8 +123,6 @@ static enum {
MODE_SRING,
MODE_GOFFLOAD,
MODE_SOFFLOAD,
- MODE_GFEATURES,
- MODE_SFEATURES,
MODE_GSTATS,
MODE_GNFC,
MODE_SNFC,
@@ -202,9 +200,6 @@ static struct option {
" [ txvlan on|off ]\n"
" [ ntuple on|off ]\n"
" [ rxhash on|off ]\n"
- },
- { "-w", "--show-features", MODE_GFEATURES, "Get offload status" },
- { "-W", "--request-features", MODE_SFEATURES, "Set requested offload",
" [ feature-name on|off [...] ]\n"
" see --show-features output for feature-name strings\n" },
{ "-i", "--driver", MODE_GDRV, "Show driver information" },
@@ -306,7 +301,6 @@ static void show_usage(void)
static char *devname = NULL;
-static int goffload_changed = 0;
static int off_csum_rx_wanted = -1;
static int off_csum_tx_wanted = -1;
static int off_sg_wanted = -1;
@@ -316,6 +310,7 @@ static int off_gso_wanted = -1;
static u32 off_flags_wanted = 0;
static u32 off_flags_mask = 0;
static int off_gro_wanted = -1;
+static struct ethtool_sfeatures *features_req;
static struct ethtool_pauseparam epause;
static int gpause_changed = 0;
@@ -778,8 +773,6 @@ static void parse_cmdline(int argc, char **argp)
(mode == MODE_SRING) ||
(mode == MODE_GOFFLOAD) ||
(mode == MODE_SOFFLOAD) ||
- (mode == MODE_GFEATURES) ||
- (mode == MODE_SFEATURES) ||
(mode == MODE_GSTATS) ||
(mode == MODE_GNFC) ||
(mode == MODE_SNFC) ||
@@ -863,14 +856,6 @@ static void parse_cmdline(int argc, char **argp)
break;
}
if (mode == MODE_SOFFLOAD) {
- parse_generic_cmdline(argc, argp, i,
- &goffload_changed,
- cmdline_offload,
- ARRAY_SIZE(cmdline_offload));
- i = argc;
- break;
- }
- if (mode == MODE_SFEATURES) {
parse_sfeatures_args(argc, argp, i);
i = argc;
break;
@@ -1944,10 +1929,6 @@ static int doit(void)
return do_goffload(fd, &ifr);
} else if (mode == MODE_SOFFLOAD) {
return do_soffload(fd, &ifr);
- } else if (mode == MODE_GFEATURES) {
- return do_gfeatures(fd, &ifr);
- } else if (mode == MODE_SFEATURES) {
- return do_sfeatures(fd, &ifr);
} else if (mode == MODE_GSTATS) {
return do_gstats(fd, &ifr);
} else if (mode == MODE_GNFC) {
@@ -2262,13 +2243,20 @@ static int do_goffload(int fd, struct ifreq *ifr)
allfail = 0;
}
+ if (!allfail)
+ dump_offload(rx, tx, sg, tso, ufo, gso, gro, lro, rxvlan, txvlan,
+ ntuple, rxhash);
+
+ err = do_gfeatures(fd, ifr);
+ if (!err)
+ allfail = 0;
+
if (allfail) {
fprintf(stdout, "no offload info available\n");
return 83;
}
- return dump_offload(rx, tx, sg, tso, ufo, gso, gro, lro, rxvlan, txvlan,
- ntuple, rxhash);
+ return 0;
}
static int do_soffload(int fd, struct ifreq *ifr)
@@ -2277,116 +2265,114 @@ static int do_soffload(int fd, struct ifreq *ifr)
int err, changed = 0;
if (off_csum_rx_wanted >= 0) {
- changed = 1;
eval.cmd = ETHTOOL_SRXCSUM;
eval.data = (off_csum_rx_wanted == 1);
ifr->ifr_data = (caddr_t)&eval;
err = send_ioctl(fd, ifr);
- if (err) {
+ if (err)
perror("Cannot set device rx csum settings");
- return 84;
- }
+ else
+ changed = 1;
}
if (off_csum_tx_wanted >= 0) {
- changed = 1;
eval.cmd = ETHTOOL_STXCSUM;
eval.data = (off_csum_tx_wanted == 1);
ifr->ifr_data = (caddr_t)&eval;
err = send_ioctl(fd, ifr);
- if (err) {
+ if (err)
perror("Cannot set device tx csum settings");
- return 85;
- }
+ else
+ changed = 1;
}
if (off_sg_wanted >= 0) {
- changed = 1;
eval.cmd = ETHTOOL_SSG;
eval.data = (off_sg_wanted == 1);
ifr->ifr_data = (caddr_t)&eval;
err = send_ioctl(fd, ifr);
- if (err) {
+ if (err)
perror("Cannot set device scatter-gather settings");
- return 86;
- }
+ else
+ changed = 1;
}
if (off_tso_wanted >= 0) {
- changed = 1;
eval.cmd = ETHTOOL_STSO;
eval.data = (off_tso_wanted == 1);
ifr->ifr_data = (caddr_t)&eval;
err = send_ioctl(fd, ifr);
- if (err) {
+ if (err)
perror("Cannot set device tcp segmentation offload settings");
- return 88;
- }
+ else
+ changed = 1;
}
if (off_ufo_wanted >= 0) {
- changed = 1;
eval.cmd = ETHTOOL_SUFO;
eval.data = (off_ufo_wanted == 1);
ifr->ifr_data = (caddr_t)&eval;
err = ioctl(fd, SIOCETHTOOL, ifr);
- if (err) {
+ if (err)
perror("Cannot set device udp large send offload settings");
- return 89;
- }
+ else
+ changed = 1;
}
if (off_gso_wanted >= 0) {
- changed = 1;
eval.cmd = ETHTOOL_SGSO;
eval.data = (off_gso_wanted == 1);
ifr->ifr_data = (caddr_t)&eval;
err = ioctl(fd, SIOCETHTOOL, ifr);
- if (err) {
+ if (err)
perror("Cannot set device generic segmentation offload settings");
- return 90;
- }
+ else
+ changed = 1;
}
if (off_flags_mask) {
- changed = 1;
eval.cmd = ETHTOOL_GFLAGS;
eval.data = 0;
ifr->ifr_data = (caddr_t)&eval;
err = ioctl(fd, SIOCETHTOOL, ifr);
if (err) {
perror("Cannot get device flag settings");
- return 91;
- }
+ } else {
+ eval.cmd = ETHTOOL_SFLAGS;
+ eval.data = ((eval.data & ~off_flags_mask) |
+ off_flags_wanted);
- eval.cmd = ETHTOOL_SFLAGS;
- eval.data = ((eval.data & ~off_flags_mask) |
- off_flags_wanted);
-
- err = ioctl(fd, SIOCETHTOOL, ifr);
- if (err) {
- perror("Cannot set device flag settings");
- return 92;
+ err = ioctl(fd, SIOCETHTOOL, ifr);
+ if (err)
+ perror("Cannot set device flag settings");
+ else
+ changed = 1;
}
}
if (off_gro_wanted >= 0) {
- changed = 1;
eval.cmd = ETHTOOL_SGRO;
eval.data = (off_gro_wanted == 1);
ifr->ifr_data = (caddr_t)&eval;
err = ioctl(fd, SIOCETHTOOL, ifr);
- if (err) {
+ if (err)
perror("Cannot set device GRO settings");
- return 93;
- }
+ else
+ changed = 1;
+ }
+
+ if (features_req) {
+ err = do_sfeatures(fd, ifr);
+ if (!err)
+ changed = 1;
}
if (!changed) {
fprintf(stdout, "no offload settings changed\n");
+ return err;
}
return 0;
}
static int get_feature_strings(int fd, struct ifreq *ifr,
- struct ethtool_gstrings **strs)
+ struct ethtool_gstrings **strs, int quiet_nx)
{
struct ethtool_sset_info *sset_info;
struct ethtool_gstrings *strings;
@@ -2398,16 +2384,18 @@ static int get_feature_strings(int fd, struct ifreq *ifr,
ifr->ifr_data = (caddr_t)sset_info;
err = send_ioctl(fd, ifr);
- if ((err < 0) ||
- (!(sset_info->sset_mask & (1ULL << ETH_SS_FEATURES)))) {
- perror("Cannot get driver strings info");
- return -100;
- }
-
n_strings = sset_info->data[0];
free(sset_info);
+
+ if ((err < 0) ||
+ (!(sset_info->sset_mask & (1ULL << ETH_SS_FEATURES))) ||
+ (n_strings == 0)) {
+ if (!quiet_nx)
+ perror("Cannot get driver strings info");
+ return -100;
+ }
+
sz_str = n_strings * ETH_GSTRING_LEN;
-
strings = calloc(1, sz_str + sizeof(struct ethtool_gstrings));
if (!strings) {
fprintf(stderr, "no memory available\n");
@@ -2429,8 +2417,6 @@ static int get_feature_strings(int fd, struct ifreq *ifr,
return n_strings;
}
-struct ethtool_sfeatures *features_req;
-
static void parse_sfeatures_args(int argc, char **argp, int argi)
{
struct cmdline_info *cmdline_desc, *cp;
@@ -2443,13 +2429,21 @@ static void parse_sfeatures_args(int argc, char **argp, int argi)
if (fd < 0)
exit(100);
- n_strings = get_feature_strings(fd, &ifr, &strings);
- if (n_strings < 0)
- exit(-n_strings);
+ n_strings = get_feature_strings(fd, &ifr, &strings, 1);
+ if (n_strings < 0) {
+ /* ETHTOOL_GFEATURES unavailable */
+ parse_generic_cmdline(argc, argp, argi, &changed,
+ cmdline_offload, ARRAY_SIZE(cmdline_offload));
+ return;
+ }
sz_features = sizeof(*features_req->features) * ((n_strings + 31) / 32);
- cp = cmdline_desc = calloc(n_strings, sizeof(*cmdline_desc));
+ cp = cmdline_desc = calloc(n_strings + ARRAY_SIZE(cmdline_offload),
+ sizeof(*cmdline_desc));
+ memcpy(cp, cmdline_offload, sizeof(cmdline_offload));
+ cp += ARRAY_SIZE(cmdline_offload);
+
features_req = calloc(1, sizeof(*features_req) + sz_features);
if (!cmdline_desc || !features_req) {
fprintf(stderr, "no memory available\n");
@@ -2518,7 +2512,7 @@ static int do_gfeatures(int fd, struct ifreq *ifr)
struct ethtool_gfeatures *features;
int n_strings, err, i;
- n_strings = get_feature_strings(fd, ifr, &strings);
+ n_strings = get_feature_strings(fd, ifr, &strings, 1);
if (n_strings < 0)
return -n_strings;
@@ -2528,7 +2522,7 @@ static int do_gfeatures(int fd, struct ifreq *ifr)
return err;
}
- fprintf(stdout, "Offload state: (name: enabled,wanted,changable)\n");
+ fprintf(stdout, "\nFull offload state: (feature-name: active,wanted,changable)\n");
for (i = 0; i < n_strings; i++) {
if (!strings->data[i * ETH_GSTRING_LEN])
continue; /* empty */
@@ -2585,12 +2579,7 @@ static int do_sfeatures(int fd, struct ifreq *ifr)
struct ethtool_gfeatures *features0, *features1;
int n_strings, err, i;
- if (!features_req) {
- fprintf(stderr, "no features changed\n");
- return 97;
- }
-
- n_strings = get_feature_strings(fd, ifr, &strings);
+ n_strings = get_feature_strings(fd, ifr, &strings, 0);
if (n_strings < 0) {
free(features_req);
return -n_strings;
--
1.7.2.5
^ 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