* RE: [RFC V3 PATCH] The message size allocated for rtnl info dumps was limited to a single
From: Rose, Gregory V @ 2011-05-17 17:16 UTC (permalink / raw)
To: Rose, Gregory V, netdev@vger.kernel.org
Cc: bhutchings@solarflare.com, davem@davemloft.net,
eric.dumazet@gmail.com
In-Reply-To: <20110517171412.5481.71745.stgit@gitlad.jf.intel.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Greg Rose
> Sent: Tuesday, May 17, 2011 10:14 AM
> To: netdev@vger.kernel.org
> Cc: bhutchings@solarflare.com; davem@davemloft.net; eric.dumazet@gmail.com
> Subject: [RFC V3 PATCH] The message size allocated for rtnl info dumps was
> limited to a single
>
Guh... screwed up the title...
Oh well, it's an RFC.
;^)
^ permalink raw reply
* Re: [PATCH v3 1/1] igmp: call ip_mc_clear_src() only when we have no users of ip_mc_list
From: David Stevens @ 2011-05-17 17:42 UTC (permalink / raw)
To: Veaceslav Falico
Cc: David Miller, jmorris, kaber, kuznet, linux-kbuild, linux-kernel,
mmarek, netdev, pekkas, yoshfuji
In-Reply-To: <20110517143756.GE30366@darkmag.usersys.redhat.com>
Veaceslav,
It looks to me like this will leak the source filters if we are
called from ip_mc_destroy_dev(),
Even with your previous patch, you're assuming that we don't free the
ip_mc_list and so we have the
same one when we up the device, but if there are no timers running, it
looks like refcnt canl go to 0 and free
it. If we can ever free the ip_mc_list when users != 0 (or going to 0
immediately after the drop), we
have to do the ip_mc_clear_src() or leak the list. I haven't looked at
this code in years, so I'll need
to refresh my memory.
So, I'll look at that a bit more; at a minimum, I think you need
to do the clear_src
also in the destroy case. We could lose the filters and set the exclude
count to users, instead
of 1; but I like the idea of keeping the source filters across a down/up,
if we can be sure there
are no cases where we free the ip_mc_list without first freeing all the
filters.
+-DLS
Veaceslav Falico <vfalico@redhat.com> wrote on 05/17/2011 07:37:56 AM:
> From: Veaceslav Falico <vfalico@redhat.com>
> To: David Stevens/Beaverton/IBM@IBMUS
> Cc: David Miller <davem@davemloft.net>, jmorris@namei.org,
> kaber@trash.net, kuznet@ms2.inr.ac.ru, linux-kbuild@vger.kernel.org,
> linux-kernel@vger.kernel.org, mmarek@suse.cz,
> netdev@vger.kernel.org, pekkas@netcore.fi, yoshfuji@linux-ipv6.org
> Date: 05/17/2011 07:39 AM
> Subject: [PATCH v3 1/1] igmp: call ip_mc_clear_src() only when we
> have no users of ip_mc_list
>
> In igmp_group_dropped() we call ip_mc_clear_src(), which resets the
number
> of source filters per mulitcast. However, igmp_group_dropped() is also
> called on NETDEV_DOWN, NETDEV_PRE_TYPE_CHANGE and NETDEV_UNREGISTER,
which
> means that the group might get added back on NETDEV_UP, NETDEV_REGISTER
and
> NETDEV_POST_TYPE_CHANGE respectively, leaving us with broken source
> filters.
>
> To fix that, we must clear the source filters only when there are no
users
> in the ip_mc_list, i.e. in ip_mc_dec_group().
>
> Correct version of the patch.
>
> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
> ---
> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index 1fd3d9c..142ca0d 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -1169,20 +1169,18 @@ static void igmp_group_dropped(struct ip_mc_list
*im)
>
> if (!in_dev->dead) {
> if (IGMP_V1_SEEN(in_dev))
> - goto done;
> + return;
> if (IGMP_V2_SEEN(in_dev)) {
> if (reporter)
> igmp_send_report(in_dev, im, IGMP_HOST_LEAVE_MESSAGE);
> - goto done;
> + return;
> }
> /* IGMPv3 */
> igmpv3_add_delrec(in_dev, im);
>
> igmp_ifc_event(in_dev);
> }
> -done:
> #endif
> - ip_mc_clear_src(im);
> }
>
> static void igmp_group_added(struct ip_mc_list *im)
> @@ -1319,6 +1317,7 @@ void ip_mc_dec_group(struct in_device *in_dev,
> __be32 addr)
> *ip = i->next_rcu;
> in_dev->mc_count--;
> igmp_group_dropped(i);
> + ip_mc_clear_src(i);
>
> if (!in_dev->dead)
> ip_rt_multicast_event(in_dev);
^ permalink raw reply
* Re: [PATCH net-2.6] net: use hlist_del_rcu() in dev_change_name()
From: David Miller @ 2011-05-17 17:57 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1305649260.2850.106.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 17 May 2011 18:21:00 +0200
> Using plain hlist_del() in dev_change_name() is wrong since a
> concurrent reader can crash trying to dereference LIST_POISON1.
>
> Bug introduced in commit 72c9528bab94 (net: Introduce
> dev_get_by_name_rcu())
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] net: ping: fix build failure
From: David Miller @ 2011-05-17 18:00 UTC (permalink / raw)
To: randy.dunlap; +Cc: segoon, sfr, netdev, linux-next, linux-kernel
In-Reply-To: <20110517084125.77b543c8.randy.dunlap@oracle.com>
From: Randy Dunlap <randy.dunlap@oracle.com>
Date: Tue, 17 May 2011 08:41:25 -0700
> On Tue, 17 May 2011 14:16:56 +0400 Vasiliy Kulikov wrote:
>
>> On Mon, May 16, 2011 at 15:38 -0400, David Miller wrote:
>> > From: Randy Dunlap <randy.dunlap@oracle.com>
>> > Date: Mon, 16 May 2011 12:35:34 -0700
>> >
>> > > On Mon, 16 May 2011 15:10:19 +1000 Stephen Rothwell wrote:
>> > > when CONFIG_PROC_SYSCTL is not enabled:
>> > >
>> > > ping.c:(.text+0x52af3): undefined reference to `inet_get_ping_group_range_net'
>> >
>> > Vasiliy, please fix this.
>>
>> I wonder whether there is any way to test such unusual configurations?
>> Only randconfig or are there any (partly-)automated tools for it?
>>
>>
>> [PATCH] net: ping: fix build failure
>>
>> If CONFIG_PROC_SYSCTL=n the building process fails:
>>
>> ping.c:(.text+0x52af3): undefined reference to `inet_get_ping_group_range_net'
>>
>> Moved inet_get_ping_group_range_net() to ping.c.
>>
>> Reported-by: Randy Dunlap <randy.dunlap@oracle.com>
>> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
>
> Acked-by: Randy Dunlap <randy.dunlap@oracle.com>
Applied, thanks everyone.
^ permalink raw reply
* Re: pull request: sfc-next-2.6 2011-05-17
From: David Miller @ 2011-05-17 18:00 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1305647988.2848.14.camel@bwh-desktop>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 17 May 2011 16:59:48 +0100
> The following changes since commit 7be799a70ba3dd90a59e8d2c72bbe06020005b3f:
>
> ipv4: Remove rt->rt_dst reference from ip_forward_options(). (2011-05-13 17:31:02 -0400)
>
> are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next-2.6.git master
>
> Miscellaneous bug fixes.
Pulled, thanks Ben.
^ permalink raw reply
* 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
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