* Re: [PATCH 2/2] ipv4: flush ARP entries on device change
From: Timo Teräs @ 2010-03-04 11:15 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev
In-Reply-To: <4B8E75E1.8060909@trash.net>
Patrick McHardy wrote:
> Timo Teräs wrote:
>> Patrick McHardy wrote:
>>> Timo Teräs wrote:
>>>> Patrick McHardy wrote:
>>>>> Timo Teras wrote:
>>>>> It would be nice if we could restrict this to IFF_NOARP changes.
>>>> Yes. But I did not see any easy way to figure out which flags have
>>>> changed.
>>>>
>>>> Should we just keep a copy of the previous IFF_NOARP bit somewhere
>>>> (where?).
>>>> Or did I miss something obvious?
>>> We shouldn't have any arp entries for devices with IFF_NOARP set,
>>> so perhaps we can flush only in that case. The transition IFF_NOARP
>>> -> ~IFF_NOARP shouldn't need flushing.
>> IFF_NOARP devices do have neighbor entries with the nud NOARP.
>> Exactly those entries I want to flush when IFF_NOARP flag is
>> removed.
>>
>> You can see those entries with "ip neigh show nud all". You have
>> them e.g. for loopback stuff and broad-/multicast stuff in general.
>> With IFF_NOARP you get them on all unicast addresses used.
>
> I see. I don't have a better suggestion, except perhaps
> to store the bit in dev->priv_flags.
Ok.
Should I make a patch that uses dev->priv_flags and repost?
Or would make sense to have more generic way, e.g. keep copy
of the previous flags in struct net_device so notifiers can
check which ones changed (or would this have locking issues?).
- Timo
^ permalink raw reply
* Re: [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr
From: David Miller @ 2010-03-04 11:38 UTC (permalink / raw)
To: jpirko; +Cc: netdev, jussi.kivilinna, linux-wireless
In-Reply-To: <20100304092014.GC2721@psychotron.lab.eng.brq.redhat.com>
From: Jiri Pirko <jpirko@redhat.com>
Date: Thu, 4 Mar 2010 10:20:14 +0100
> Slapping myself twice on each cheek.
>
> Subject: [net-2.6 PATCH] rndis_wlan: correct multicast_list handling V3
>
> My previous patch (655ffee284dfcf9a24ac0343f3e5ee6db85b85c5) added locking in
> a bad way. Because rndis_set_oid can sleep, there is need to prepare multicast
> addresses into local buffer under netif_addr_lock first, then call
> rndis_set_oid outside. This caused reorganizing of the whole function.
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> Reported-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
Applied.
^ permalink raw reply
* RE
From: Award Winner @ 2010-03-04 12:50 UTC (permalink / raw)
please contact mr christopher thomas for your lottery endowment
^ permalink raw reply
* Re: [PATCH v4] ethtool: Add direct access to ops->get_sset_count
From: Jeff Garzik @ 2010-03-04 13:23 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, netdev, gospo
In-Reply-To: <20100304085054.4471.44679.stgit@localhost.localdomain>
On 03/04/2010 03:51 AM, Jeff Kirsher wrote:
> From: Jeff Garzik<jgarzik@redhat.com>
>
> This patch is an alternative approach for accessing string
> counts, vs. the drvinfo indirect approach. This way the drvinfo
> space doesn't run out, and we don't break ABI later.
>
> Signed-off-by: Jeff Garzik<jgarzik@redhat.com>
> Signed-off-by: Peter P Waskiewicz Jr<peter.p.waskiewicz.jr@intel.com>
> Signed-off-by: Jeff Kirsher<jeffrey.t.kirsher@intel.com>
> ---
>
> include/linux/ethtool.h | 17 +++++++++--
> net/core/ethtool.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 86 insertions(+), 3 deletions(-)
Both patches look good to me. There is a cosmetic issue of needing to
sync up userspace and kernel ethtool.h WRT whitespace and deleted
constants, but I can do that after DaveM applies this patch.
Waiting for upstream application, or other objections...
^ permalink raw reply
* Re: [PATCH v4] ethtool: Add direct access to ops->get_sset_count
From: Ben Hutchings @ 2010-03-04 14:26 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, jeff, netdev, gospo
In-Reply-To: <20100304085054.4471.44679.stgit@localhost.localdomain>
On Thu, 2010-03-04 at 00:51 -0800, Jeff Kirsher wrote:
> From: Jeff Garzik <jgarzik@redhat.com>
>
> This patch is an alternative approach for accessing string
> counts, vs. the drvinfo indirect approach. This way the drvinfo
> space doesn't run out, and we don't break ABI later.
[...]
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -214,6 +214,10 @@ static noinline int ethtool_get_drvinfo(struct net_device *dev, void __user *use
> info.cmd = ETHTOOL_GDRVINFO;
> ops->get_drvinfo(dev, &info);
>
> + /*
> + * this method of obtaining string set info is deprecated;
> + * consider using ETHTOOL_GSSET_INFO instead
> + */
This comment belongs on the interface (ethtool.h) not the
implementation.
[...]
> +static noinline int ethtool_get_sset_info(struct net_device *dev,
> + void __user *useraddr)
> +{
[...]
> + /* calculate size of return buffer */
> + for (i = 0; i < 64; i++)
> + if (sset_mask & (1ULL << i))
> + n_bits++;
[...]
We have a function for this:
n_bits = hweight64(sset_mask);
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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: Re: [net-next-2.6 PATCH] e1000e: do not modify tx queue len on link speed change
From: Jeff Kirsher @ 2010-03-04 14:41 UTC (permalink / raw)
To: David Miller
Cc: franco, Brandeburg, jesse.brandeburg, netdev, Allan,
bruce.w.allan
In-Reply-To: <20100304.004128.236847399.davem@davemloft.net>
On Thu, Mar 4, 2010 at 00:41, David Miller <davem@davemloft.net> wrote:
> From: franco@lastsummer.de
> Date: Wed, 3 Mar 2010 15:31:37 +0100 (CET)
>
>> e1000e would try to set appropriate values of tx_queue_len for
>> negotiated
>> link speeds overwriting the already set configuration of tx_queue_len in
>> the process. This removes all lines touching tx_queue_len from the
>> driver.
>>
>> Signed-off-by: Franco Fichtner <franco@lastsummer.de>
>
> Intel folks, you got this?
Yes, I have added it to my queue of patches. Sorry for the delayed response.
--
Cheers,
Jeff
^ permalink raw reply
* RE: [PATCH V2 2/7] tcp: use limited socket backlog
From: Eric Dumazet @ 2010-03-04 14:56 UTC (permalink / raw)
To: Zhu, Yi
Cc: netdev@vger.kernel.org, David S. Miller, Alexey Kuznetsov,
Pekka Savola (ipv6), Patrick McHardy
In-Reply-To: <DA586906BA1FFC4384FCFD6429ECE860A45A6D9E@shzsmsx502.ccr.corp.intel.com>
Le jeudi 04 mars 2010 à 19:04 +0800, Zhu, Yi a écrit :
>
> Since neither sk_drops nor the new MIB value are used by TCP currently.
> How about I keep the tcp backlog limit patch as is and you implement
> the above in another patch?
Sure, I will do that
^ permalink raw reply
* [PATCH] typhoon: fix incorrect use of smp_wmb()
From: David Dillow @ 2010-03-04 14:37 UTC (permalink / raw)
To: netdev; +Cc: David Miller
The typhoon driver was incorrectly using smp_wmb() to order memory
accesses against IO to the NIC in a few instances. Use wmb() instead,
which is required to actually order between memory types.
Signed-off-by: David Dillow <dave@thedillows.org>
---
This is against Linus's tree, which doesn't appear to have net-next in
it yet. It should apply with offsets, though. I can redo against
net-next if needed, but likely won't have quick access to that tree
until the weekend.
diff --git a/drivers/net/typhoon.c b/drivers/net/typhoon.c
index 39f1fc6..a192852 100644
--- a/drivers/net/typhoon.c
+++ b/drivers/net/typhoon.c
@@ -487,7 +487,7 @@ typhoon_hello(struct typhoon *tp)
typhoon_inc_cmd_index(&ring->lastWrite, 1);
INIT_COMMAND_NO_RESPONSE(cmd, TYPHOON_CMD_HELLO_RESP);
- smp_wmb();
+ wmb();
iowrite32(ring->lastWrite, tp->ioaddr + TYPHOON_REG_CMD_READY);
spin_unlock(&tp->command_lock);
}
@@ -1321,13 +1321,15 @@ typhoon_init_interface(struct typhoon *tp)
tp->txlo_dma_addr = le32_to_cpu(iface->txLoAddr);
tp->card_state = Sleeping;
- smp_wmb();
tp->offload = TYPHOON_OFFLOAD_IP_CHKSUM | TYPHOON_OFFLOAD_TCP_CHKSUM;
tp->offload |= TYPHOON_OFFLOAD_UDP_CHKSUM | TSO_OFFLOAD_ON;
spin_lock_init(&tp->command_lock);
spin_lock_init(&tp->state_lock);
+
+ /* Force the writes to the shared memory area out before continuing. */
+ wmb();
}
static void
^ permalink raw reply related
* Re: 2.6.33 dies on modprobe
From: Américo Wang @ 2010-03-04 15:30 UTC (permalink / raw)
To: M G Berberich
Cc: Américo Wang, linux-kernel, Linux Kernel Network Developers,
Greg Kroah-Hartman
In-Reply-To: <20100303230438.GA3363@invalid>
On Thu, Mar 04, 2010 at 12:04:38AM +0100, M G Berberich wrote:
>Hello,
>
>Am Mittwoch, den 03. März schrieb Américo Wang:
>> On Wed, Mar 3, 2010 at 5:08 PM, Américo Wang <xiyou.wangcong@gmail.com> wrote:
>> > On Wed, Mar 3, 2010 at 4:58 PM, Américo Wang <xiyou.wangcong@gmail.com> wrote:
>> >> Ok, below is my patch, I am not sure it could fix the BUG for you,
>> >> but it could fix the WARNING. But perhaps they are related.
>> >>
>> >> Please give it a try.
>> >>
>> >
>> > Oops! Ignore the patch, it should not be correct, I will send a
>> > correct version soon.
>> > Sorry.
>> >
>>
>> Here we go:
>
>Seems to fix the warning, but not the modprobe-Oops.
>
Thanks much for your testing!
Could you please try to turn on DEBUG_KOBJECT and boot your kernel
with "initcall_debug"? And then show us your dmesg.
Regards.
^ permalink raw reply
* Re: wanPMC-CxT1E1
From: Greg KH @ 2010-03-04 15:47 UTC (permalink / raw)
To: Bob Beers; +Cc: netdev, Krzysztof Halasa
In-Reply-To: <4f6ba3b1003040540v4619afa8v7f28ef5d75514c47@mail.gmail.com>
On Thu, Mar 04, 2010 at 08:40:46AM -0500, Bob Beers wrote:
> Obviously still needs serious attention, but it compiles.
Great, this looks wonderful, I'll queue it up.
> Original author: Rick Dobbs
Hm, do we have an email address for Rick anywhere?
thanks,
greg k-h
^ permalink raw reply
* Re: wanPMC-CxT1E1
From: Bob Beers @ 2010-03-04 16:06 UTC (permalink / raw)
To: Greg KH; +Cc: netdev, Krzysztof Halasa
In-Reply-To: <20100304154707.GB30191@kroah.com>
On Thu, Mar 4, 2010 at 10:47 AM, Greg KH <greg@kroah.com> wrote:
> On Thu, Mar 04, 2010 at 08:40:46AM -0500, Bob Beers wrote:
>> Obviously still needs serious attention, but it compiles.
>
> Great, this looks wonderful, I'll queue it up.
>
>> Original author: Rick Dobbs
>
> Hm, do we have an email address for Rick anywhere?
I knew you were going to ask that, and I asked at OSS.
They said, "Rick has asked me not to give out his e-mail."
--
thanks,
-Bob Beers
^ permalink raw reply
* Re: Help On function to get IPV6 address of an interface in kernel
From: Brian Haley @ 2010-03-04 16:17 UTC (permalink / raw)
To: Mohan Reddy; +Cc: netdev
In-Reply-To: <20100304094103.GA6599@mohan-laptop>
Mohan Reddy wrote:
> Hi,
>
> Is there a function or a way in kernel to get an interface ipv6 address if interface name or net_device object is known? I searched in the kernel i got a function ipv6_get_ifaddr(). But it is expecting an ipv6 address as a parameter.
What type of IPv6 address? For link-locals you can use ipv6_get_lladdr(), but
for others ipv6_dev_get_saddr() is more comprehensive, but you'll need a
destination address. There's lots of reference code in addrconf.c to do
other things.
-Brian
^ permalink raw reply
* Re: wanPMC-CxT1E1
From: Greg KH @ 2010-03-04 16:18 UTC (permalink / raw)
To: Bob Beers; +Cc: netdev, Krzysztof Halasa
In-Reply-To: <4f6ba3b1003040806m5946c9e3xc3cd7b8adade899c@mail.gmail.com>
On Thu, Mar 04, 2010 at 11:06:59AM -0500, Bob Beers wrote:
> On Thu, Mar 4, 2010 at 10:47 AM, Greg KH <greg@kroah.com> wrote:
> > On Thu, Mar 04, 2010 at 08:40:46AM -0500, Bob Beers wrote:
> >> Obviously still needs serious attention, but it compiles.
> >
> > Great, this looks wonderful, I'll queue it up.
> >
> >> Original author: Rick Dobbs
> >
> > Hm, do we have an email address for Rick anywhere?
>
> I knew you were going to ask that, and I asked at OSS.
> They said, "Rick has asked me not to give out his e-mail."
Heh, ok, I guess "pride of ownership" is not something that he has for
this body of code :)
I'll take it as-is.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] gianfar: Fix TX ring processing on SMP machines
From: Kumar Gala @ 2010-03-04 16:34 UTC (permalink / raw)
To: Anton Vorontsov
Cc: linuxppc-dev list, Netdev, Martyn Welch, Paul Gortmaker,
Sandeep Gopalpet, David Miller
In-Reply-To: <20100304.004157.18721737.davem@davemloft.net>
On Mar 4, 2010, at 2:41 AM, David Miller wrote:
> From: Anton Vorontsov <avorontsov@ru.mvista.com>
> Date: Wed, 3 Mar 2010 21:18:58 +0300
>
>> Starting with commit a3bc1f11e9b867a4f49505 ("gianfar: Revive SKB
>> recycling") gianfar driver sooner or later stops transmitting any
>> packets on SMP machines.
>>
>> start_xmit() prepares new skb for transmitting, generally it does
>> three things:
>>
>> 1. sets up all BDs (marks them ready to send), except the first one.
>> 2. stores skb into tx_queue->tx_skbuff so that clean_tx_ring()
>> would cleanup it later.
>> 3. sets up the first BD, i.e. marks it ready.
>>
>> Here is what clean_tx_ring() does:
>>
>> 1. reads skbs from tx_queue->tx_skbuff
>> 2. checks if the *last* BD is ready. If it's still ready [to send]
>> then it it isn't transmitted, so clean_tx_ring() returns.
>> Otherwise it actually cleanups BDs. All is OK.
>>
>> Now, if there is just one BD, code flow:
>>
>> - start_xmit(): stores skb into tx_skbuff. Note that the first BD
>> (which is also the last one) isn't marked as ready, yet.
>> - clean_tx_ring(): sees that skb is not null, *and* its lstatus
>> says that it is NOT ready (like if BD was sent), so it cleans
>> it up (bad!)
>> - start_xmit(): marks BD as ready [to send], but it's too late.
>>
>> We can fix this simply by reordering lstatus/tx_skbuff writes.
>>
>> Reported-by: Martyn Welch <martyn.welch@ge.com>
>> Bisected-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
>> Tested-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>> Tested-by: Martyn Welch <martyn.welch@ge.com>
>
> Applied.
Anton,
Once this makes it into Linus's tree can you make sure we get it added to -stable.
- k
^ permalink raw reply
* Re: Iproute2 tc pfifo_fast not work
From: Stephen Hemminger @ 2010-03-04 16:40 UTC (permalink / raw)
To: Петухов Дмитрий
Cc: netdev
In-Reply-To: <4B8F7537.7090008@electro-com.ru>
On Thu, 04 Mar 2010 11:54:15 +0300
Петухов Дмитрий <d.petuhov@electro-com.ru> wrote:
> Hi Stephen.
>
> I've found that I can't add pfifo_fast qdisc to my tc class tree. It
> just says:
> -----------
> # /sbin/tc qdisc add dev eth3 parent 2:299B handle 299B: pfifo_fast
> qdisc 'pfifo_fast' does not support option parsing
> -----------
> And nothing else happends.
> strace /sbin/tc qdisc add dev eth3 parent 2:299B handle 299B: pfifo_fast
> &> tc_pfifo_strace.txt
> attached.
>
> Kernel version 2.6.31, .config attached.
> tc version 2.6.31.
>
> I do something wrong or this is a bug?
no
> For now I've switched to PRIO QDISC which seems to work fine, but it
> would be great to use lithweigth pfifo_fast instead of prio...
pfifo_fast is very limited to being just a root qdisc, and with no
children. If you want to do the same thing but in other context
just use pfifo.
^ permalink raw reply
* Re: [PATCH 2/2] iproute2 skbedit: Add support to mark packets
From: Stephen Hemminger @ 2010-03-04 16:48 UTC (permalink / raw)
To: hadi; +Cc: netdev, Alexander Duyck
In-Reply-To: <1267671091.4821.27.camel@bigi>
On Wed, 03 Mar 2010 21:51:31 -0500
jamal <hadi@cyberus.ca> wrote:
> You applied 2/2 but not 1/2.
>
> cheers,
> jamal
> On Wed, 2010-03-03 at 16:36 -0800, Stephen Hemminger wrote:
> > On Thu, 03 Dec 2009 08:32:48 -0500
> > jamal <hadi@cyberus.ca> wrote:
> >
> > > I did this originally because i was getting too many support
> > > questions on ipt with mark. It still makes sense since skbedit
> > > seems to be the mother action of skb metadata (should probably
> > > have been called skbmeta).
> > >
> > > cheers,
> > > jamal
> >
The first was the help message text, and that was already there.
Or was there some other bit.
^ permalink raw reply
* Re: [PATCH 2/2] iproute2 skbedit: Add support to mark packets
From: jamal @ 2010-03-04 16:58 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Alexander Duyck
In-Reply-To: <20100304084829.22a80ace@nehalam>
On Thu, 2010-03-04 at 08:48 -0800, Stephen Hemminger wrote:
> On Wed, 03 Mar 2010 21:51:31 -0500
> jamal <hadi@cyberus.ca> wrote:
>
> > You applied 2/2 but not 1/2.
> > >
>
> The first was the help message text, and that was already there.
> Or was there some other bit.
I can see that now. The git logs dont show how that change
made it in - thats how i concluded it wasnt there. How did that
sneak in there?
cheers,
jamal
^ permalink raw reply
* uninterruptible sleep in unix_dgram_recvmsg
From: Michal Schmidt @ 2010-03-04 17:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Hello David.
When multiple tasks call recv() on a AF_UNIX/SOCK_DGRAM socket where
noone sends anything, only the first one will sleep interruptibly. The
others are in uninterruptible sleep, causing artificial increase of
loadavg. After two minutes, the hung task watchdog triggers and prints
ugly warnings.
The bug is reported here (with a reproducer attached):
https://bugzilla.redhat.com/show_bug.cgi?id=529202
While the first task awaits the arrival of a packet in
skb_recv_datagram(), it holds the u->readlock mutex, on which the
other tasks will be waiting.
My first idea was to simply replace mutex_lock with
mutex_lock_interruptible. This solves the problem, but one
issue still remains - the receiving timeout (SO_RCVTIMEO) would start
ticking only after the process got the mutex and entered into
skb_recv_datagram().
So instead of that I started to think about why u->readlock is held
across skb_recv_datagram() anyway. I found that it was added in 2.6.10
by your patch "[AF_UNIX]: Serialize dgram read using semaphore just
like stream" which apparently fixed an exploitable race condition
(CAN-2004-1068).
I don't know what exactly u->readlock protects here.
IOW, what race would this patch cause?:
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index f255119..01387da 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1660,9 +1660,9 @@ static int unix_dgram_recvmsg(struct kiocb *iocb, struct socket *sock,
msg->msg_namelen = 0;
+ skb = skb_recv_datagram(sk, flags, noblock, &err);
mutex_lock(&u->readlock);
- skb = skb_recv_datagram(sk, flags, noblock, &err);
if (!skb) {
unix_state_lock(sk);
/* Signal EOF on disconnected non-blocking SEQPACKET socket. */
^ permalink raw reply related
* Re: [PATCH 2/2] iproute2 skbedit: Add support to mark packets
From: Stephen Hemminger @ 2010-03-04 17:43 UTC (permalink / raw)
To: hadi; +Cc: netdev, Alexander Duyck
In-Reply-To: <1267721912.4821.37.camel@bigi>
On Thu, 04 Mar 2010 11:58:32 -0500
jamal <hadi@cyberus.ca> wrote:
> On Thu, 2010-03-04 at 08:48 -0800, Stephen Hemminger wrote:
> > On Wed, 03 Mar 2010 21:51:31 -0500
> > jamal <hadi@cyberus.ca> wrote:
> >
> > > You applied 2/2 but not 1/2.
>
> > > >
> >
> > The first was the help message text, and that was already there.
> > Or was there some other bit.
>
> I can see that now. The git logs dont show how that change
> made it in - thats how i concluded it wasnt there. How did that
> sneak in there?
I put the first bit in back in December. It was too early since
kernel wasn't ready yet, but generally harmless
commit e04dd30a38130a3d85065a747cc33274766a9cb6
Author: Jamal Hadi Salim <hadi@cyberus.ca>
Date: Sat Dec 26 11:12:43 2009 -0800
skbedit: Add support to mark packets
This adds support for setting the skb mark.
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
^ permalink raw reply
* [PATCH] Re: [PATCH v4] ethtool: Add direct access to ops->get_sset_count
From: Jeff Garzik @ 2010-03-04 18:21 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Jeff Kirsher, davem, netdev, gospo
In-Reply-To: <1267712797.2819.149.camel@localhost>
[-- Attachment #1: Type: text/plain, Size: 1614 bytes --]
On 03/04/2010 09:26 AM, Ben Hutchings wrote:
> On Thu, 2010-03-04 at 00:51 -0800, Jeff Kirsher wrote:
>> From: Jeff Garzik<jgarzik@redhat.com>
>>
>> This patch is an alternative approach for accessing string
>> counts, vs. the drvinfo indirect approach. This way the drvinfo
>> space doesn't run out, and we don't break ABI later.
> [...]
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
>> @@ -214,6 +214,10 @@ static noinline int ethtool_get_drvinfo(struct net_device *dev, void __user *use
>> info.cmd = ETHTOOL_GDRVINFO;
>> ops->get_drvinfo(dev,&info);
>>
>> + /*
>> + * this method of obtaining string set info is deprecated;
>> + * consider using ETHTOOL_GSSET_INFO instead
>> + */
>
> This comment belongs on the interface (ethtool.h) not the
> implementation.
Debatable -- the current comment is located at the callsite of
ops->get_sset_count(), which is where an implementor might think to add
a new call. Not all the numeric fields in ethtool_drvinfo are obtained
from ->get_sset_count().
Hence the "some" in the attached patch to include/linux/ethtool.h,
addressing your comment.
> [...]
>> +static noinline int ethtool_get_sset_info(struct net_device *dev,
>> + void __user *useraddr)
>> +{
> [...]
>> + /* calculate size of return buffer */
>> + for (i = 0; i< 64; i++)
>> + if (sset_mask& (1ULL<< i))
>> + n_bits++;
> [...]
>
> We have a function for this:
>
> n_bits = hweight64(sset_mask);
Agreed.
I've attached a follow-up patch, which should enable my/Jeff's kernel
patch to be applied, followed by this one.
Jeff
[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1755 bytes --]
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
include/linux/ethtool.h | 7 +++++++
net/core/ethtool.c | 7 +++----
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index f6f961f..b33f316 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -61,6 +61,13 @@ struct ethtool_drvinfo {
/* For PCI devices, use pci_name(pci_dev). */
char reserved1[32];
char reserved2[12];
+ /*
+ * Some struct members below are filled in
+ * using ops->get_sset_count(). Obtaining
+ * this info from ethtool_drvinfo is now
+ * deprecated; Use ETHTOOL_GSSET_INFO
+ * instead.
+ */
__u32 n_priv_flags; /* number of flags valid in ETHTOOL_GPFLAGS */
__u32 n_stats; /* number of u64's from ETHTOOL_GSTATS */
__u32 testinfo_len;
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 70075c4..33d2ded 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -17,6 +17,7 @@
#include <linux/errno.h>
#include <linux/ethtool.h>
#include <linux/netdevice.h>
+#include <linux/bitops.h>
#include <asm/uaccess.h>
/*
@@ -216,7 +217,7 @@ static noinline int ethtool_get_drvinfo(struct net_device *dev, void __user *use
/*
* this method of obtaining string set info is deprecated;
- * consider using ETHTOOL_GSSET_INFO instead
+ * Use ETHTOOL_GSSET_INFO instead.
*/
if (ops->get_sset_count) {
int rc;
@@ -265,9 +266,7 @@ static noinline int ethtool_get_sset_info(struct net_device *dev,
return 0;
/* calculate size of return buffer */
- for (i = 0; i < 64; i++)
- if (sset_mask & (1ULL << i))
- n_bits++;
+ n_bits = hweight64(sset_mask);
memset(&info, 0, sizeof(info));
info.cmd = ETHTOOL_GSSET_INFO;
^ permalink raw reply related
* Re: [PATCH 2/2] iproute2 skbedit: Add support to mark packets
From: jamal @ 2010-03-04 18:30 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Alexander Duyck
In-Reply-To: <20100304094323.05fce73a@nehalam>
On Thu, 2010-03-04 at 09:43 -0800, Stephen Hemminger wrote:
> I put the first bit in back in December. It was too early since
> kernel wasn't ready yet, but generally harmless
Ah, my bad. git whatchanged didnt show that.
cheers,
jamal
^ permalink raw reply
* Re: [net-next PATCH v6 0/3] net: reserve ports for applications using fixed port numbers
From: Eric W. Biederman @ 2010-03-04 19:14 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, opurdila, netdev, linux-kernel, nhorman,
eric.dumazet
In-Reply-To: <4B8F6FF0.5000306@redhat.com>
Cong Wang <amwang@redhat.com> writes:
> David Miller wrote:
>> Eric B., could you look over the first two patches (which touch the
>> sysctl core) and give some review and ACK/NACK?
>>
>
> ping Eric W. Biederman ... :)
I have looked and it is not easy to tell by simple review if
correctness has been maintained in the proc cleanup patch.
Furthermore the code even after the cleanups still feels like code
that is trying too hard to do too much.
I think the example set by bitmap_parse_user in it's user interface is
a good example to follow when constructing bitmap helper functions.
Including having the actual parsing code should live in lib/bitmap.c
The users of bitmap_parse_user do something very nice. They allocate
a temporary bitmap. Parse the userspace value into the temporary
bitmap, and then move that new bitmap into the kernel data structures.
For your large bitmap this seems like the idea way to handle it. That
guarantees no weird intermediate failure states, and really makes the
the bitmap feel like a single value.
I would add the restriction that the values in the list of ranges
always must be increasing, and in general restrict the set of accepted
values as much as possible. If we don't accept it now we don't have
to worry about some userspace application relying on some unitended
side effect a few years into the future.
I think it is a serious bug that you clear the destination bitmap
in the middle of parsing it. That will either open or close all
ports in the middle of parsing, and I can't see how that would
ever be a good thing.
Eric
^ permalink raw reply
* Re: [net-next PATCH v6 0/3] net: reserve ports for applications using fixed port numbers
From: Octavian Purdila @ 2010-03-04 20:11 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Cong Wang, David Miller, netdev, linux-kernel, nhorman,
eric.dumazet
In-Reply-To: <m1vddbdhmo.fsf@fess.ebiederm.org>
On Thursday 04 March 2010 21:14:07 you wrote:
> Cong Wang <amwang@redhat.com> writes:
> > David Miller wrote:
> >> Eric B., could you look over the first two patches (which touch the
> >> sysctl core) and give some review and ACK/NACK?
> >
> > ping Eric W. Biederman ... :)
>
> I have looked and it is not easy to tell by simple review if
> correctness has been maintained in the proc cleanup patch.
> Furthermore the code even after the cleanups still feels like code
> that is trying too hard to do too much.
>
>
> I think the example set by bitmap_parse_user in it's user interface is
> a good example to follow when constructing bitmap helper functions.
> Including having the actual parsing code should live in lib/bitmap.c
>
> The users of bitmap_parse_user do something very nice. They allocate
> a temporary bitmap. Parse the userspace value into the temporary
> bitmap, and then move that new bitmap into the kernel data structures.
> For your large bitmap this seems like the idea way to handle it. That
> guarantees no weird intermediate failure states, and really makes the
> the bitmap feel like a single value.
>
> I would add the restriction that the values in the list of ranges
> always must be increasing, and in general restrict the set of accepted
> values as much as possible. If we don't accept it now we don't have
> to worry about some userspace application relying on some unitended
> side effect a few years into the future.
>
Eric, thanks for taking the time to go over it again. I now do share you
opinion that we need to be more atomic. How about this simple approach:
1. Allocate new kernel buffer for the text and copy the whole userspace buffer
into it.
2. Allocate temporary buffer for bitmap.
3. Parse the kernel text buffer into the temp bitmap.
4. Copy the temp bitmap into the final bitmap.
This is simple and clean but it has the disadvantage of potentially allocating
a large chunk of memory, although even in the case that all ports are going to
be set the temporary buffer will not go over 390K, which is now not an issue
anymore for kmalloc, right?
> I think it is a serious bug that you clear the destination bitmap
> in the middle of parsing it. That will either open or close all
> ports in the middle of parsing, and I can't see how that would
> ever be a good thing.
>
Even when doing the copy from the temp bitmap you still have some inconsistent
bitmap state while copying.
We could solve by replacing the old buffer with the new one + RCU.
^ permalink raw reply
* Re: BNX2: Kernel crashes with 2.6.31 and 2.6.31.9
From: Brian Haley @ 2010-03-04 20:31 UTC (permalink / raw)
To: Michael Chan; +Cc: Bruno Prémont, Benjamin Li, NetDEV, Linux-Kernel
In-Reply-To: <1267567926.19491.175.camel@nseg_linux_HP1.broadcom.com>
Hi Michael,
Michael Chan wrote:
> Do we have timers running in this environment? The timer in the bnx2
> driver, bnx2_timer(), needs to run to provide a heart beat to the
> firmware. In netpoll mode without timer interrupts, if we are regularly
> calling the NAPI poll function, it should also be able to provide the
> heartbeat. Without the heartbeat, the firmware will reset the chip and
> result in the NETDEV WATCHDOG.
We have also been seeing watchdog timeouts with bnx2, below is a
stack trace with Benjamin's debug patch applied. Normally we were
only seeing them under heavy load, but this one was at boot. We haven't
tried the latest firmware/driver from 2.6.33 yet. You can contact me
offline if you need more detailed info.
Thanks,
-Brian
[ 2.428093] bnx2 0000:04:00.0: firmware: requesting bnx2/bnx2-rv2p-06-5.0.0.j3.fw
[ 2.432526] eth0: Broadcom NetXtreme II BCM5708 1000Base-T (B2) PCI-X 64-bit 133MHz found at mem f6000000, IRQ 41, node addr 00:1c:c4:e1:cc:ea
[ 2.439520] bnx2 0000:42:00.0: PCI INT A -> GSI 34 (level, low) -> IRQ 34
<snip>
[ 223.805014] ------------[ cut here ]------------
[ 223.805023] WARNING: at net/sched/sch_generic.c:261 dev_watchdog+0x12d/0x1d5()
[ 223.805026] Hardware name: ProLiant DL385 G2
[ 223.805028] NETDEV WATCHDOG: eth0 (bnx2): transmit queue 0 timed out
[ 223.805031] Modules linked in: itapi iptable_filter ip_tables x_tables mptctl ipmi_devintf deflate zlib_deflate ctr twofish twofish_common camellia serpent blowfish cast5 des_generic cbc cryptd aes_x86_64 aes_generic xcbc rmd160 sha256_generic sha1_generic crypto_null af_key dm_snapshot dm_mirror dm_region_hash dm_log dm_mod sg bonding sctp crc32c libcrc32c loop psmouse serio_raw amd64_edac_mod edac_core k8temp container i2c_piix4 i2c_core ipmi_si ipmi_msghandler shpchp pci_hotplug hpilo processor evdev ext3 jbd mbcache ses enclosure sd_mod crc_t10dif ide_cd_mod cdrom ata_generic libata ide_pci_generic usbhid hid mptsas bnx2 mptscsih mptbase scsi_transport_sas serverworks ehci_hcd scsi_mod ide_core ohci_hcd uhci_hcd button thermal fan thermal_sys edd [last unloaded: scsi_wait_scan]
[ 223.805102] Pid: 0, comm: swapper Not tainted 2.6.32-clim-4-amd64 #1
[ 223.805105] Call Trace:
[ 223.805108] <IRQ> [<ffffffff812697a0>] ? dev_watchdog+0x12d/0x1d5
[ 223.805118] [<ffffffff81049914>] warn_slowpath_common+0x77/0xa4
[ 223.805123] [<ffffffff810499b6>] warn_slowpath_fmt+0x64/0x66
[ 223.805128] [<ffffffff81045df7>] ? default_wake_function+0xd/0xf
[ 223.805133] [<ffffffff81035fa7>] ? __wake_up_common+0x46/0x76
[ 223.805138] [<ffffffff8103b414>] ? __wake_up+0x43/0x50
[ 223.805143] [<ffffffff81253829>] ? netdev_drivername+0x43/0x4b
[ 223.805147] [<ffffffff812697a0>] dev_watchdog+0x12d/0x1d5
[ 223.805152] [<ffffffff8105e84a>] ? delayed_work_timer_fn+0x0/0x3d
[ 223.805156] [<ffffffff8105e7ee>] ? __queue_work+0x35/0x3d
[ 223.805159] [<ffffffff81269673>] ? dev_watchdog+0x0/0x1d5
[ 223.805164] [<ffffffff8105655a>] run_timer_softirq+0x1ff/0x2a1
[ 223.805169] [<ffffffff810205a1>] ? lapic_next_event+0x18/0x1c
[ 223.805174] [<ffffffff8104f9e3>] __do_softirq+0xde/0x19f
[ 223.805179] [<ffffffff8100ccec>] call_softirq+0x1c/0x28
[ 223.805183] [<ffffffff8100e8b1>] do_softirq+0x41/0x81
[ 223.805187] [<ffffffff8104f7bd>] irq_exit+0x36/0x75
[ 223.805191] [<ffffffff81020f33>] smp_apic_timer_interrupt+0x88/0x96
[ 223.805195] [<ffffffff8100c6b3>] apic_timer_interrupt+0x13/0x20
[ 223.805198] <EOI> [<ffffffff81027740>] ? native_safe_halt+0x6/0x8
[ 223.805207] [<ffffffff81012da3>] ? default_idle+0x55/0x74
[ 223.805210] [<ffffffff810131ce>] ? c1e_idle+0xf4/0xfb
[ 223.805215] [<ffffffff81065529>] ? atomic_notifier_call_chain+0x13/0x15
[ 223.805219] [<ffffffff8100aeec>] ? cpu_idle+0x5b/0x93
[ 223.805225] [<ffffffff81304144>] ? start_secondary+0x1a8/0x1ac
[ 223.805228] ---[ end trace b04d103e6c8c23de ]---
[ 223.805231] bnx2: eth0 DEBUG: intr_sem[0]
[ 223.805236] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 223.805242] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 223.805245] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 228.805016] bnx2: eth0 DEBUG: intr_sem[0]
[ 228.805023] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 228.805029] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 228.805033] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 233.805014] bnx2: eth0 DEBUG: intr_sem[0]
[ 233.805019] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 233.805024] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 233.805028] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 238.805013] bnx2: eth0 DEBUG: intr_sem[0]
[ 238.805019] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 238.805025] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 238.805029] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 243.805015] bnx2: eth0 DEBUG: intr_sem[0]
[ 243.805021] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 243.805027] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 243.805031] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 248.805014] bnx2: eth0 DEBUG: intr_sem[0]
[ 248.805019] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 248.805025] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 248.805028] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 253.805015] bnx2: eth0 DEBUG: intr_sem[0]
[ 253.805021] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 253.805027] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 253.805031] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 258.805016] bnx2: eth0 DEBUG: intr_sem[0]
[ 258.805022] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 258.805027] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 258.805031] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 263.805013] bnx2: eth0 DEBUG: intr_sem[0]
[ 263.805018] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 263.805023] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 263.805027] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 268.805014] bnx2: eth0 DEBUG: intr_sem[0]
[ 268.805019] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 268.805025] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 268.805028] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 273.805015] bnx2: eth0 DEBUG: intr_sem[0]
[ 273.805022] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 273.805028] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 273.805032] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 278.805012] bnx2: eth0 DEBUG: intr_sem[0]
[ 278.805017] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 278.805023] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 278.805026] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 283.805012] bnx2: eth0 DEBUG: intr_sem[0]
[ 283.805016] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 283.805022] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 283.805025] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 288.805015] bnx2: eth0 DEBUG: intr_sem[0]
[ 288.805022] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 288.805028] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 288.805031] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 293.805011] bnx2: eth0 DEBUG: intr_sem[0]
[ 293.805016] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 293.805022] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 293.805026] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 298.805015] bnx2: eth0 DEBUG: intr_sem[0]
[ 298.805021] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 298.805027] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 298.805030] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 303.805013] bnx2: eth0 DEBUG: intr_sem[0]
[ 303.805020] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 303.805026] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 303.805030] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 308.805013] bnx2: eth0 DEBUG: intr_sem[0]
[ 308.805018] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 308.805023] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 308.805027] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 313.805011] bnx2: eth0 DEBUG: intr_sem[0]
[ 313.805016] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 313.805022] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 313.805025] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
^ permalink raw reply
* Re: 2.6.33 dies on modprobe
From: M G Berberich @ 2010-03-04 20:38 UTC (permalink / raw)
To: Américo Wang
Cc: linux-kernel, Linux Kernel Network Developers, Greg Kroah-Hartman
In-Reply-To: <20100304153042.GA4614@hack>
Am Donnerstag, den 04. März schrieb Américo Wang:
> On Thu, Mar 04, 2010 at 12:04:38AM +0100, M G Berberich wrote:
> >Am Mittwoch, den 03. März schrieb Américo Wang:
> >> On Wed, Mar 3, 2010 at 5:08 PM, Américo Wang <xiyou.wangcong@gmail.com> wrote:
> >> > On Wed, Mar 3, 2010 at 4:58 PM, Américo Wang <xiyou.wangcong@gmail.com> wrote:
> >> >> Ok, below is my patch, I am not sure it could fix the BUG for you,
> >> >> but it could fix the WARNING. But perhaps they are related.
> >> >>
> >> >> Please give it a try.
> >> >>
> >> >
> >> > Oops! Ignore the patch, it should not be correct, I will send a
> >> > correct version soon.
> >> > Sorry.
> >> >
> >>
> >> Here we go:
> >
> >Seems to fix the warning, but not the modprobe-Oops.
> >
>
> Thanks much for your testing!
>
> Could you please try to turn on DEBUG_KOBJECT and boot your kernel
> with "initcall_debug"? And then show us your dmesg.
If I'm doing this (enabled CONFIG_DEBUG_KERNEL and
CONFIG_DEBUG_KOBJECT), the kernel dies immediately - no output.
MfG
bmg
--
„Des is völlig wurscht, was heut beschlos- | M G Berberich
sen wird: I bin sowieso dagegn!“ | berberic@fmi.uni-passau.de
(SPD-Stadtrat Kurt Schindler; Regensburg) | www.fmi.uni-passau.de/~berberic
^ 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