* Re: [PATCH] netxen: The driver doesn't work on NX_P3_B1 so cause probe to fail.
From: David Miller @ 2010-03-23 3:32 UTC (permalink / raw)
To: amit.salecha; +Cc: ebiederm, netdev, ameen.rahman
In-Reply-To: <99737F4847ED0A48AECC9F4A1974A4B80CFEFED1DB@MNEXMB2.qlogic.org>
From: Amit Salecha <amit.salecha@qlogic.com>
Date: Mon, 22 Mar 2010 01:35:20 -0500
> David,
> After discussing this issues with my team, this is fine with Qlogic to remove support of NX_P3_B1.
>
> You can go ahead with Eric's patch.
>
> Sorry for all these hiccups.
Ok, applied.
^ permalink raw reply
* Re: [RFC Patch 1/3] netpoll: add generic support for bridge and bonding devices
From: David Miller @ 2010-03-23 3:49 UTC (permalink / raw)
To: amwang
Cc: mpm, linux-kernel, netdev, bridge, gospo, nhorman, shemminger,
bonding-devel, fubar, jmoyer
In-Reply-To: <4BA823D7.4010106@redhat.com>
From: Cong Wang <amwang@redhat.com>
Date: Tue, 23 Mar 2010 10:13:43 +0800
> Matt Mackall wrote:
>> Seems like a lot of interface for something to be used by only a
>> couple
>> core drivers. Hopefully Dave has an opinion here.
>>
>
> Yeah, I worry about this too, maybe we can group those methods
> for netpoll together into another struct, and just put a pointer
> here?
This looks like it's tackled at the wrong layer, to be honest.
Teaching all of these layers about eachother's states is
going to end up being a nightmare in the end.
All of this "where is the npinfo" business can be handled
generically in net/core/dev.c I think, with none of these
callbacks.
For example, something like "if dev lacks ->npinfo, check
it's master".
Another thing, I wouldn't iterate over all devices, like I
see in the bonding poll controller method. Just whichever
one supports netpoll you see first, use it and exit
immediately. Don't send it to every single port, I can't
see how that might be desirable or useful.
^ permalink raw reply
* Re: forcedeth: cat /sys/class/net/eth0/carrier = Invalid argument
From: David Miller @ 2010-03-23 3:51 UTC (permalink / raw)
To: justinmattock; +Cc: netdev, linux-kernel
In-Reply-To: <4BA7AA92.5040400@gmail.com>
From: "Justin P. mattock" <justinmattock@gmail.com>
Date: Mon, 22 Mar 2010 10:36:18 -0700
> I've pushed my kernel from the latest HEAD to 2.6.31
> and am still seeing:
> cat /sys/class/net/eth0/carrier
> cat: carrier: Invalid argument
>
> with my other machine using sky2
> the same results is:
> cat /sys/class/net/eth0/carrier
> 0
>
> is there anything on this? before I start
> a bisect.(looking through bugzilla, I couldn't
> see anything related).
A device is not able to indicate carrier accurately when
the device is not currently running.
So you will get an -EINVAL until the device is brought up.
The reason is that most drivers don't even probe the link
or negotiate link speed and flow control until the device
is brought up. Many don't even power up the PHY when the
device is down, in order to save power.
So the behavior you observe is completely expected.
^ permalink raw reply
* Re: [Uclinux-dist-devel] [PATCH] can: bfin_can: switch to common Blackfin can header
From: Mike Frysinger @ 2010-03-23 3:53 UTC (permalink / raw)
To: David Miller
Cc: socketcan-core, netdev, uclinux-dist-devel, oliver.hartkopp,
urs.thuermann
In-Reply-To: <20100322.185736.199039558.davem@davemloft.net>
On Mon, Mar 22, 2010 at 21:57, David Miller wrote:
> From: Mike Frysinger <vapier.adi@gmail.com>
>> On Mon, Mar 22, 2010 at 21:48, David Miller wrote:
>>> From: Mike Frysinger
>>>> how about we skip to the end where we agree i'm wrong, you're right,
>>>> and you just merge the patch that i already resent ...
>>>
>>> Sure, if you wish to make it clear that you have zero
>>> interest in understanding the substance of this issue.
>>
>> whatever makes you happy. you dont appear to have much interest in my
>> explanations.
>
> Your explanation is that "resubmit" doesn't really mean "resubmit"
that is never what i said. read the exact words (or dont, i guess it
doesnt matter) -- i said i _thought_ your e-mail was a _poke me when
it's ready_. once you actually explained you explicitly wanted a
resend, i did so. i additionally asked why because i was curious, not
because i wasnt going to resend or i said you were wrong.
> You do it if you don't believe me. I bet you'll have a different view
> of the world and a different set of expectations after that.
i already field multiple reports every single day as maintainer of
core Blackfin/Gentoo projects among other things.
-mike
^ permalink raw reply
* Re: [Uclinux-dist-devel] [PATCH] can: bfin_can: switch to common Blackfin can header
From: Mike Frysinger @ 2010-03-23 3:53 UTC (permalink / raw)
To: David Miller
Cc: socketcan-core, netdev, uclinux-dist-devel, oliver.hartkopp,
urs.thuermann
In-Reply-To: <20100322.200632.129770126.davem@davemloft.net>
On Mon, Mar 22, 2010 at 23:06, David Miller wrote:
> From: Mike Frysinger <vapier@gentoo.org>
>> The MMR bits are being moved to this header, so include it.
>
> Applied.
thanks
-mike
^ permalink raw reply
* Re: ixgbe: Set IXGBE_RSC_CB(skb)->DMA field to zero after unmapping the address
From: Benjamin Herrenschmidt @ 2010-03-23 3:57 UTC (permalink / raw)
To: Mallikarjuna R Chilakala; +Cc: Linux Kernel Mailing List, David Miller, netdev
In-Reply-To: <201003221759.o2MHxEaj015546@hera.kernel.org>
On Mon, 2010-03-22 at 17:59 +0000, Linux Kernel Mailing List wrote:
> Gitweb: http://git.kernel.org/linus/fd3686a842717b890fbe3024b83a616c54d5dba0
> Commit: fd3686a842717b890fbe3024b83a616c54d5dba0
> Parent: 936332b8e00103fc20eb7e915c9a3bcb2835a11a
> Author: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
> AuthorDate: Fri Mar 19 04:41:33 2010 +0000
> Committer: David S. Miller <davem@davemloft.net>
> CommitDate: Fri Mar 19 21:00:44 2010 -0700
>
> ixgbe: Set IXGBE_RSC_CB(skb)->DMA field to zero after unmapping the address
>
> As per Simon Horman's feedback set IXGBE_RSC_CB(skb)->dma to zero
> after unmapping HWRSC DMA address to avoid double freeing.
>
Note that this whole code is bogus :-) You cannot just assume that 0 is
a invalid DMA address. It is not. In fact, while you can check if a
dma_addr_t is invalid using dma_mapping_error(), the generic APIs
don't provide you with a magic "bad" value you can use for what you are
trying to do.
Granted, I think we should make our iommu code reserve the first page
for the sake of everybody's sanity and to avoid such pitfalls, but
this code is wrong with today iommu implementations.
Cheers,
Ben.
> Signed-off-by: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
> Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
> drivers/net/ixgbe/ixgbe_main.c | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
> index 18b5b21..d75c46f 100644
> --- a/drivers/net/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ixgbe/ixgbe_main.c
> @@ -935,10 +935,12 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
> if (skb->prev)
> skb = ixgbe_transform_rsc_queue(skb, &(rx_ring->rsc_count));
> if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) {
> - if (IXGBE_RSC_CB(skb)->dma)
> + if (IXGBE_RSC_CB(skb)->dma) {
> pci_unmap_single(pdev, IXGBE_RSC_CB(skb)->dma,
> rx_ring->rx_buf_len,
> PCI_DMA_FROMDEVICE);
> + IXGBE_RSC_CB(skb)->dma = 0;
> + }
> if (rx_ring->flags & IXGBE_RING_RX_PS_ENABLED)
> rx_ring->rsc_count += skb_shinfo(skb)->nr_frags;
> else
> @@ -3126,10 +3128,12 @@ static void ixgbe_clean_rx_ring(struct ixgbe_adapter *adapter,
> rx_buffer_info->skb = NULL;
> do {
> struct sk_buff *this = skb;
> - if (IXGBE_RSC_CB(this)->dma)
> + if (IXGBE_RSC_CB(this)->dma) {
> pci_unmap_single(pdev, IXGBE_RSC_CB(this)->dma,
> rx_ring->rx_buf_len,
> PCI_DMA_FROMDEVICE);
> + IXGBE_RSC_CB(this)->dma = 0;
> + }
> skb = skb->prev;
> dev_kfree_skb(this);
> } while (skb);
> --
> To unsubscribe from this list: send the line "unsubscribe git-commits-head" 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: [Bug 15582] New: BUG: unable to handle kernel NULL pointer dereference at 0000000000000028
From: David Miller @ 2010-03-23 4:00 UTC (permalink / raw)
To: akpm
Cc: kaber, jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
alexander.h.duyck, peter.p.waskiewicz.jr, bugzilla-daemon, netdev,
stivi
In-Reply-To: <20100322161416.c4b369f9.akpm@linux-foundation.org>
From: Andrew Morton <akpm@linux-foundation.org>
Date: Mon, 22 Mar 2010 16:14:16 -0700
>
> (switched to email. Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
>
> On Fri, 19 Mar 2010 12:01:10 GMT
> bugzilla-daemon@bugzilla.kernel.org wrote:
>
>> http://bugzilla.kernel.org/show_bug.cgi?id=15582
>>
>> Summary: BUG: unable to handle kernel NULL pointer dereference
>> at 0000000000000028
>
> A bug in igb or the vlan code, I guess.
Hmmm, should have been fixed by:
commit d1c76af9e2434fac3add561e26c61b06503de986
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date: Mon Mar 16 10:50:02 2009 -0700
GRO: Move netpoll checks to correct location
...
Nevermind, the backtrace signature is different for this
one.
^ permalink raw reply
* Re: forcedeth: cat /sys/class/net/eth0/carrier = Invalid argument
From: Justin P. Mattock @ 2010-03-23 4:07 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-kernel
In-Reply-To: <20100322.205144.256859987.davem@davemloft.net>
On 03/22/2010 08:51 PM, David Miller wrote:
> From: "Justin P. mattock"<justinmattock@gmail.com>
> Date: Mon, 22 Mar 2010 10:36:18 -0700
>
>> I've pushed my kernel from the latest HEAD to 2.6.31
>> and am still seeing:
>> cat /sys/class/net/eth0/carrier
>> cat: carrier: Invalid argument
>>
>> with my other machine using sky2
>> the same results is:
>> cat /sys/class/net/eth0/carrier
>> 0
>>
>> is there anything on this? before I start
>> a bisect.(looking through bugzilla, I couldn't
>> see anything related).
>
> A device is not able to indicate carrier accurately when
> the device is not currently running.
>
> So you will get an -EINVAL until the device is brought up.
ah.. the machine was up and running but nothing was running
(no dhclient, streaming music, etc..)
now
just ran the machine with ethernet,(up and running) and carrier reported 1
>
> The reason is that most drivers don't even probe the link
> or negotiate link speed and flow control until the device
> is brought up. Many don't even power up the PHY when the
> device is down, in order to save power.
>
> So the behavior you observe is completely expected.
>
cool, this had me confused.
Thanks for the info on this..
(no bisect for this).
Justin P. Mattock
^ permalink raw reply
* Re: [Patch v2] netpoll: warn when there are spaces in parameters
From: Matt Mackall @ 2010-03-23 4:24 UTC (permalink / raw)
To: Cong Wang; +Cc: linux-kernel, netdev, elendil, David Miller
In-Reply-To: <4BA81D18.40509@redhat.com>
On Tue, 2010-03-23 at 09:44 +0800, Cong Wang wrote:
> Matt Mackall wrote:
> > On Mon, 2010-03-22 at 04:59 -0400, Amerigo Wang wrote:
> >> + printk(KERN_INFO "%s: warning: whitespace"
> >> + "is not allowed\n", np->name);
> >
> > Is it a warning or is it info? If it's a warning, then we probably need
> > to add "netpoll" or whatever to the message so that people who've got a
> > warning-level threshold will know what it's about.
> >
>
> If you mean KERN_INFO, yeah, I want to keep it in the same level
> as other messages around.
I should probably be more direct: I think that's the wrong thing to do.
It IS a warning (it even says so!) telling users that something probably
won't work and why and they might miss it if the severity is INFO and
then come and ask us why things aren't working. So use KERN_WARN,
please.
The other messages are INFO because when things are working, they're not
interesting.
> Also, I already put "np->name" which will be "netconsole" when
> we use netconsole.
Ahh, right, I have a brain cell somewhere that knew that.
--
http://selenic.com : development and support for Mercurial and Linux
^ permalink raw reply
* Re: [RFC Patch 2/3] bridge: make bridge support netpoll
From: Matt Mackall @ 2010-03-23 4:27 UTC (permalink / raw)
To: Cong Wang
Cc: linux-kernel, netdev, bridge, Andy Gospodarek, Neil Horman,
Stephen Hemminger, bonding-devel, Jay Vosburgh, David Miller
In-Reply-To: <4BA82186.3010204@redhat.com>
On Tue, 2010-03-23 at 10:03 +0800, Cong Wang wrote:
> Matt Mackall wrote:
> > On Mon, 2010-03-22 at 04:17 -0400, Amerigo Wang wrote:
> >> Based on the previous patch, make bridge support netpoll by:
> >>
> >> 1) implement the 4 methods to support netpoll for bridge;
> >>
> >> 2) modify netpoll during forwarding packets in bridge;
> >>
> >> 3) disable netpoll support of bridge when a netpoll-unabled device
> >> is added to bridge;
> >
> > Not sure if this is the right thing to do. Shouldn't we simply enable
> > polling on all devices that support it and warn about the others (aka
> > best effort)?
> >
>
> I don't think it's a good idea, because we check if a device
> supports netpoll by checking if it has ndo_poll_controller method.
Uh, what? If we have 5 devices on a bridge and 4 support netpoll, then
shouldn't we just send netconsole messages to those 4 devices? Isn't
this much better than simply refusing to work?
--
http://selenic.com : development and support for Mercurial and Linux
^ permalink raw reply
* Re: [Patch v2] netpoll: warn when there are spaces in parameters
From: Cong Wang @ 2010-03-23 4:34 UTC (permalink / raw)
To: Matt Mackall; +Cc: linux-kernel, netdev, elendil, David Miller
In-Reply-To: <1269318256.3552.49.camel@calx>
Matt Mackall wrote:
> On Tue, 2010-03-23 at 09:44 +0800, Cong Wang wrote:
>> Matt Mackall wrote:
>>> On Mon, 2010-03-22 at 04:59 -0400, Amerigo Wang wrote:
>>>> + printk(KERN_INFO "%s: warning: whitespace"
>>>> + "is not allowed\n", np->name);
>>> Is it a warning or is it info? If it's a warning, then we probably need
>>> to add "netpoll" or whatever to the message so that people who've got a
>>> warning-level threshold will know what it's about.
>>>
>> If you mean KERN_INFO, yeah, I want to keep it in the same level
>> as other messages around.
>
> I should probably be more direct: I think that's the wrong thing to do.
> It IS a warning (it even says so!) telling users that something probably
> won't work and why and they might miss it if the severity is INFO and
> then come and ask us why things aren't working. So use KERN_WARN,
> please.
They _are_ working, 0 will be assigned by default.
>
> The other messages are INFO because when things are working, they're not
> interesting.
>
They are not all working, take this as an example:
printk(KERN_INFO "%s: couldn't parse config at %s!\n",
np->name, cur);
^ permalink raw reply
* Re: [RFC Patch 2/3] bridge: make bridge support netpoll
From: Cong Wang @ 2010-03-23 4:39 UTC (permalink / raw)
To: Matt Mackall
Cc: linux-kernel, netdev, bridge, Andy Gospodarek, Neil Horman,
Stephen Hemminger, bonding-devel, Jay Vosburgh, David Miller
In-Reply-To: <1269318470.3552.54.camel@calx>
Matt Mackall wrote:
> On Tue, 2010-03-23 at 10:03 +0800, Cong Wang wrote:
>> Matt Mackall wrote:
>>> On Mon, 2010-03-22 at 04:17 -0400, Amerigo Wang wrote:
>>>> Based on the previous patch, make bridge support netpoll by:
>>>>
>>>> 1) implement the 4 methods to support netpoll for bridge;
>>>>
>>>> 2) modify netpoll during forwarding packets in bridge;
>>>>
>>>> 3) disable netpoll support of bridge when a netpoll-unabled device
>>>> is added to bridge;
>>> Not sure if this is the right thing to do. Shouldn't we simply enable
>>> polling on all devices that support it and warn about the others (aka
>>> best effort)?
>>>
>> I don't think it's a good idea, because we check if a device
>> supports netpoll by checking if it has ndo_poll_controller method.
>
> Uh, what? If we have 5 devices on a bridge and 4 support netpoll, then
> shouldn't we just send netconsole messages to those 4 devices? Isn't
> this much better than simply refusing to work?
>
How could you let the bridge know netpoll is not sent to
the one that doesn't support netpoll during setup? This will
be complex, I am afraid.
Thanks.
^ permalink raw reply
* Re: [Patch v2] netpoll: warn when there are spaces in parameters
From: Matt Mackall @ 2010-03-23 4:45 UTC (permalink / raw)
To: Cong Wang; +Cc: linux-kernel, netdev, elendil, David Miller
In-Reply-To: <4BA844E6.6060602@redhat.com>
On Tue, 2010-03-23 at 12:34 +0800, Cong Wang wrote:
> Matt Mackall wrote:
> > On Tue, 2010-03-23 at 09:44 +0800, Cong Wang wrote:
> >> Matt Mackall wrote:
> >>> On Mon, 2010-03-22 at 04:59 -0400, Amerigo Wang wrote:
> >>>> + printk(KERN_INFO "%s: warning: whitespace"
> >>>> + "is not allowed\n", np->name);
> >>> Is it a warning or is it info? If it's a warning, then we probably need
> >>> to add "netpoll" or whatever to the message so that people who've got a
> >>> warning-level threshold will know what it's about.
> >>>
> >> If you mean KERN_INFO, yeah, I want to keep it in the same level
> >> as other messages around.
> >
> > I should probably be more direct: I think that's the wrong thing to do.
> > It IS a warning (it even says so!) telling users that something probably
> > won't work and why and they might miss it if the severity is INFO and
> > then come and ask us why things aren't working. So use KERN_WARN,
> > please.
>
>
> They _are_ working, 0 will be assigned by default.
If this patch has any point at all other than filling the logs, it
should be WARN.
> >
> > The other messages are INFO because when things are working, they're not
> > interesting.
> >
>
> They are not all working, take this as an example:
>
> printk(KERN_INFO "%s: couldn't parse config at %s!\n",
> np->name, cur);
Yeah, that's obviously wrong too. Let's not add more wrongness just for
consistency's sake.
--
http://selenic.com : development and support for Mercurial and Linux
^ permalink raw reply
* Re: [RFC Patch 1/3] netpoll: add generic support for bridge and bonding devices
From: Cong Wang @ 2010-03-23 4:47 UTC (permalink / raw)
To: David Miller
Cc: mpm, linux-kernel, netdev, bridge, gospo, nhorman, shemminger,
bonding-devel, fubar, jmoyer
In-Reply-To: <20100322.204939.146100390.davem@davemloft.net>
David Miller wrote:
> From: Cong Wang <amwang@redhat.com>
> Date: Tue, 23 Mar 2010 10:13:43 +0800
>
>> Matt Mackall wrote:
>>> Seems like a lot of interface for something to be used by only a
>>> couple
>>> core drivers. Hopefully Dave has an opinion here.
>>>
>> Yeah, I worry about this too, maybe we can group those methods
>> for netpoll together into another struct, and just put a pointer
>> here?
>
> This looks like it's tackled at the wrong layer, to be honest.
>
> Teaching all of these layers about eachother's states is
> going to end up being a nightmare in the end.
>
> All of this "where is the npinfo" business can be handled
> generically in net/core/dev.c I think, with none of these
> callbacks.
>
> For example, something like "if dev lacks ->npinfo, check
> it's master".
This is a good point! I haven't tried but certainly this is
worthy a try. Ideally those callbacks can be all removed,
but I don't know if this is true practically. ;)
I will try.
>
> Another thing, I wouldn't iterate over all devices, like I
> see in the bonding poll controller method. Just whichever
> one supports netpoll you see first, use it and exit
> immediately. Don't send it to every single port, I can't
> see how that might be desirable or useful.
Yeah, for bonding case, probably. But for bridge case, I think
we still need to check all, right?
Thanks!
^ permalink raw reply
* Re: [RFC Patch 2/3] bridge: make bridge support netpoll
From: Matt Mackall @ 2010-03-23 4:51 UTC (permalink / raw)
To: Cong Wang
Cc: linux-kernel, netdev, bridge, Andy Gospodarek, Neil Horman,
Stephen Hemminger, bonding-devel, Jay Vosburgh, David Miller
In-Reply-To: <4BA84607.7030304@redhat.com>
On Tue, 2010-03-23 at 12:39 +0800, Cong Wang wrote:
> Matt Mackall wrote:
> > On Tue, 2010-03-23 at 10:03 +0800, Cong Wang wrote:
> >> Matt Mackall wrote:
> >>> On Mon, 2010-03-22 at 04:17 -0400, Amerigo Wang wrote:
> >>>> Based on the previous patch, make bridge support netpoll by:
> >>>>
> >>>> 1) implement the 4 methods to support netpoll for bridge;
> >>>>
> >>>> 2) modify netpoll during forwarding packets in bridge;
> >>>>
> >>>> 3) disable netpoll support of bridge when a netpoll-unabled device
> >>>> is added to bridge;
> >>> Not sure if this is the right thing to do. Shouldn't we simply enable
> >>> polling on all devices that support it and warn about the others (aka
> >>> best effort)?
> >>>
> >> I don't think it's a good idea, because we check if a device
> >> supports netpoll by checking if it has ndo_poll_controller method.
> >
> > Uh, what? If we have 5 devices on a bridge and 4 support netpoll, then
> > shouldn't we just send netconsole messages to those 4 devices? Isn't
> > this much better than simply refusing to work?
> >
>
> How could you let the bridge know netpoll is not sent to
> the one that doesn't support netpoll during setup? This will
> be complex, I am afraid.
I thought I saw a simple loop over bridge devices at poll time in your
patch. So it should be a simple matter of skipping unsupported devices
in that loop.
But Dave thinks there a bigger problems here, so I recommend first
figuring out the architecture issues, then we can get back to the policy
issues.
--
http://selenic.com : development and support for Mercurial and Linux
^ permalink raw reply
* Re: [Patch v2] netpoll: warn when there are spaces in parameters
From: Cong Wang @ 2010-03-23 4:56 UTC (permalink / raw)
To: Matt Mackall; +Cc: linux-kernel, netdev, elendil, David Miller
In-Reply-To: <1269319520.3552.76.camel@calx>
Matt Mackall wrote:
> On Tue, 2010-03-23 at 12:34 +0800, Cong Wang wrote:
>> Matt Mackall wrote:
>>> On Tue, 2010-03-23 at 09:44 +0800, Cong Wang wrote:
>>>> Matt Mackall wrote:
>>>>> On Mon, 2010-03-22 at 04:59 -0400, Amerigo Wang wrote:
>>>>>> + printk(KERN_INFO "%s: warning: whitespace"
>>>>>> + "is not allowed\n", np->name);
>>>>> Is it a warning or is it info? If it's a warning, then we probably need
>>>>> to add "netpoll" or whatever to the message so that people who've got a
>>>>> warning-level threshold will know what it's about.
>>>>>
>>>> If you mean KERN_INFO, yeah, I want to keep it in the same level
>>>> as other messages around.
>>> I should probably be more direct: I think that's the wrong thing to do.
>>> It IS a warning (it even says so!) telling users that something probably
>>> won't work and why and they might miss it if the severity is INFO and
>>> then come and ask us why things aren't working. So use KERN_WARN,
>>> please.
>>
>> They _are_ working, 0 will be assigned by default.
>
> If this patch has any point at all other than filling the logs, it
> should be WARN.
Others too.
>
>>> The other messages are INFO because when things are working, they're not
>>> interesting.
>>>
>> They are not all working, take this as an example:
>>
>> printk(KERN_INFO "%s: couldn't parse config at %s!\n",
>> np->name, cur);
>
> Yeah, that's obviously wrong too. Let's not add more wrongness just for
> consistency's sake.
>
Since you insist, please send a patch for all of these messages
in netpoll. This patch, from $subject, is aimed to warn spaces.
Thanks.
^ permalink raw reply
* Re: [RFC Patch 2/3] bridge: make bridge support netpoll
From: David Miller @ 2010-03-23 4:57 UTC (permalink / raw)
To: amwang
Cc: mpm, linux-kernel, netdev, bridge, gospo, nhorman, shemminger,
bonding-devel, fubar
In-Reply-To: <4BA84607.7030304@redhat.com>
From: Cong Wang <amwang@redhat.com>
Date: Tue, 23 Mar 2010 12:39:35 +0800
> How could you let the bridge know netpoll is not sent to
> the one that doesn't support netpoll during setup? This will
> be complex, I am afraid.
Why does this matter at all?
I told you in another mail that we should do away with
these callbacks and all the crazy 'npinfo' assignments
and just do it in the generic code.
^ permalink raw reply
* Re: [RFC Patch 1/3] netpoll: add generic support for bridge and bonding devices
From: David Miller @ 2010-03-23 4:58 UTC (permalink / raw)
To: amwang
Cc: mpm, linux-kernel, netdev, bridge, gospo, nhorman, shemminger,
bonding-devel, fubar, jmoyer
In-Reply-To: <4BA847EB.9040808@redhat.com>
From: Cong Wang <amwang@redhat.com>
Date: Tue, 23 Mar 2010 12:47:39 +0800
> Yeah, for bonding case, probably. But for bridge case, I think
> we still need to check all, right?
Why? Who cares?
If it goes out one port and reaches it's destination
the objective has been achieved.
Sending it out N more times achieves nothing.
^ permalink raw reply
* Re: [RFC Patch 2/3] bridge: make bridge support netpoll
From: David Miller @ 2010-03-23 4:59 UTC (permalink / raw)
To: mpm
Cc: amwang, linux-kernel, netdev, bridge, gospo, nhorman, shemminger,
bonding-devel, fubar
In-Reply-To: <1269319861.3552.87.camel@calx>
From: Matt Mackall <mpm@selenic.com>
Date: Mon, 22 Mar 2010 23:51:01 -0500
> On Tue, 2010-03-23 at 12:39 +0800, Cong Wang wrote:
>> Matt Mackall wrote:
>> How could you let the bridge know netpoll is not sent to
>> the one that doesn't support netpoll during setup? This will
>> be complex, I am afraid.
>
> I thought I saw a simple loop over bridge devices at poll time in your
> patch. So it should be a simple matter of skipping unsupported devices
> in that loop.
It's because of all that "assign ->npinfo to slaves" crap he has to do
the way his patches are currently implemented.
It's basically another sign that the design is wrong.
^ permalink raw reply
* Re: [RFC Patch 2/3] bridge: make bridge support netpoll
From: Cong Wang @ 2010-03-23 5:00 UTC (permalink / raw)
To: Matt Mackall
Cc: linux-kernel, netdev, bridge, Andy Gospodarek, Neil Horman,
Stephen Hemminger, bonding-devel, Jay Vosburgh, David Miller
In-Reply-To: <1269319861.3552.87.camel@calx>
Matt Mackall wrote:
> On Tue, 2010-03-23 at 12:39 +0800, Cong Wang wrote:
>> Matt Mackall wrote:
>>> On Tue, 2010-03-23 at 10:03 +0800, Cong Wang wrote:
>>>> Matt Mackall wrote:
>>>>> On Mon, 2010-03-22 at 04:17 -0400, Amerigo Wang wrote:
>>>>>> Based on the previous patch, make bridge support netpoll by:
>>>>>>
>>>>>> 1) implement the 4 methods to support netpoll for bridge;
>>>>>>
>>>>>> 2) modify netpoll during forwarding packets in bridge;
>>>>>>
>>>>>> 3) disable netpoll support of bridge when a netpoll-unabled device
>>>>>> is added to bridge;
>>>>> Not sure if this is the right thing to do. Shouldn't we simply enable
>>>>> polling on all devices that support it and warn about the others (aka
>>>>> best effort)?
>>>>>
>>>> I don't think it's a good idea, because we check if a device
>>>> supports netpoll by checking if it has ndo_poll_controller method.
>>> Uh, what? If we have 5 devices on a bridge and 4 support netpoll, then
>>> shouldn't we just send netconsole messages to those 4 devices? Isn't
>>> this much better than simply refusing to work?
>>>
>> How could you let the bridge know netpoll is not sent to
>> the one that doesn't support netpoll during setup? This will
>> be complex, I am afraid.
>
> I thought I saw a simple loop over bridge devices at poll time in your
> patch. So it should be a simple matter of skipping unsupported devices
> in that loop.
Nope, we need to check if the target address is owned by
a device that doesn't support netpoll or not, simple skipping
will not work.
>
> But Dave thinks there a bigger problems here, so I recommend first
> figuring out the architecture issues, then we can get back to the policy
> issues.
>
Ok. Thanks!
^ permalink raw reply
* Re: [RFC Patch 2/3] bridge: make bridge support netpoll
From: Cong Wang @ 2010-03-23 5:06 UTC (permalink / raw)
To: David Miller
Cc: mpm, linux-kernel, netdev, bridge, gospo, nhorman, shemminger,
bonding-devel, fubar
In-Reply-To: <20100322.215703.77339158.davem@davemloft.net>
David Miller wrote:
> From: Cong Wang <amwang@redhat.com>
> Date: Tue, 23 Mar 2010 12:39:35 +0800
>
>> How could you let the bridge know netpoll is not sent to
>> the one that doesn't support netpoll during setup? This will
>> be complex, I am afraid.
>
> Why does this matter at all?
Because currently we check netpoll support by ->ndo_poll_controller,
for example, tap driver doesn't have ->ndo_poll_controller now,
if I choose the target "@192.168.0.2/br0" where "192.168.0.2" is owned
by "tap0" which is managed by "br0", netconsole may not work.
>
> I told you in another mail that we should do away with
> these callbacks and all the crazy 'npinfo' assignments
> and just do it in the generic code.
I think ->ndo_poll_controller is not in the case that you talked about.
Thanks.
^ permalink raw reply
* Re: [RFC Patch 1/3] netpoll: add generic support for bridge and bonding devices
From: Cong Wang @ 2010-03-23 5:15 UTC (permalink / raw)
To: David Miller
Cc: mpm, linux-kernel, netdev, bridge, gospo, nhorman, shemminger,
bonding-devel, fubar, jmoyer
In-Reply-To: <20100322.215822.123414773.davem@davemloft.net>
David Miller wrote:
> From: Cong Wang <amwang@redhat.com>
> Date: Tue, 23 Mar 2010 12:47:39 +0800
>
>> Yeah, for bonding case, probably. But for bridge case, I think
>> we still need to check all, right?
>
> Why? Who cares?
>
> If it goes out one port and reaches it's destination
> the objective has been achieved.
>
> Sending it out N more times achieves nothing.
We have to check which port has the right destination.
Ideally we should check the right destination address to
choose the port, but currently we don't have a generic
way to check this, thus I chose to send it to all ports.
You are right, this needs to be improved.
Thanks!
^ permalink raw reply
* [PATCH] Fix locking in flush_backlog
From: Tom Herbert @ 2010-03-23 6:04 UTC (permalink / raw)
To: davem, netdev
Need to take spinlocks when dequeuing from input_pkt_queue in
flush_backlog. Also, with the spinlock the backlog queues can
be flushed directly from netdev_run_todo.
Signed-off-by: Tom Herbert <therbert@google.com>
---
diff --git a/net/core/dev.c b/net/core/dev.c
index a03aab4..e7db656 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2765,20 +2765,6 @@ int netif_receive_skb(struct sk_buff *skb)
}
EXPORT_SYMBOL(netif_receive_skb);
-/* Network device is going away, flush any packets still pending */
-static void flush_backlog(void *arg)
-{
- struct net_device *dev = arg;
- struct softnet_data *queue = &__get_cpu_var(softnet_data);
- struct sk_buff *skb, *tmp;
-
- skb_queue_walk_safe(&queue->input_pkt_queue, skb, tmp)
- if (skb->dev == dev) {
- __skb_unlink(skb, &queue->input_pkt_queue);
- kfree_skb(skb);
- }
-}
-
static int napi_gro_complete(struct sk_buff *skb)
{
struct packet_type *ptype;
@@ -5545,6 +5531,7 @@ void netdev_run_todo(void)
while (!list_empty(&list)) {
struct net_device *dev
= list_first_entry(&list, struct net_device, todo_list);
+ int i;
list_del(&dev->todo_list);
if (unlikely(dev->reg_state != NETREG_UNREGISTERING)) {
@@ -5556,7 +5543,22 @@ void netdev_run_todo(void)
dev->reg_state = NETREG_UNREGISTERED;
- on_each_cpu(flush_backlog, dev, 1);
+ /* Flush backlog queues of any pending packets */
+ for_each_online_cpu(i) {
+ struct softnet_data *queue = &per_cpu(softnet_data, i);
+ struct sk_buff *skb, *tmp;
+ unsigned long flags;
+
+ spin_lock_irqsave(&queue->input_pkt_queue.lock, flags);
+ skb_queue_walk_safe(&queue->input_pkt_queue, skb, tmp)
+ if (skb->dev == dev) {
+ __skb_unlink(skb,
+ &queue->input_pkt_queue);
+ kfree_skb(skb);
+ }
+ spin_unlock_irqrestore(&queue->input_pkt_queue.lock,
+ flags);
+ }
netdev_wait_allrefs(dev);
^ permalink raw reply related
* [PATCH] rps: memory leak due to forget to release rx queues
From: Changli Gao @ 2010-03-23 6:05 UTC (permalink / raw)
To: David S. Miller; +Cc: xiaosuo, Tom Herbert, netdev
memory leak due to forget to release rx queues.
rx queues is allocated in alloc_netdev_mq(), but they aren't released in
free_netdev(), and the field rps_map is also miss.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
net/core/dev.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index c0e2608..51b86e0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5746,6 +5746,14 @@ void free_netdev(struct net_device *dev)
release_net(dev_net(dev));
+ if (dev->num_rx_queues) {
+ int i;
+
+ for (i = 0; i < dev->num_rx_queues; i++)
+ kfree(dev->_rx[i].rps_map);
+ kfree(dev->_rx);
+ }
+
kfree(dev->_tx);
/* Flush device addresses */
^ permalink raw reply related
* [PATCH] rps: make distributing packets fairly among all the online CPUs default
From: Changli Gao @ 2010-03-23 6:24 UTC (permalink / raw)
To: David S. Miller; +Cc: Tom Herbert, xiaosuo, netdev
make distributing packets fairly among all the online CPUs default.
Make distributing packets fairly among all the online CPUs default, then
users don't need any explicit configuration to get the benefit of RPS.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
net/core/dev.c | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index c0e2608..a4246f1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5234,6 +5234,24 @@ void netif_stacked_transfer_operstate(const struct net_device *rootdev,
}
EXPORT_SYMBOL(netif_stacked_transfer_operstate);
+static struct rps_map* alloc_rps_map(void)
+{
+ struct rps_map *map;
+ int i, cpu;
+
+ map = kzalloc(max_t(unsigned,
+ RPS_MAP_SIZE(cpumask_weight(cpu_online_mask)),
+ L1_CACHE_BYTES), GFP_KERNEL);
+ if (map == NULL)
+ return NULL;
+ i = 0;
+ for_each_online_cpu(cpu)
+ map->cpus[i++] = cpu;
+ map->len = i;
+
+ return map;
+}
+
/**
* register_netdevice - register a network device
* @dev: device to register
@@ -5282,7 +5300,13 @@ int register_netdevice(struct net_device *dev)
ret = -ENOMEM;
goto out;
}
-
+ dev->_rx->rps_map = alloc_rps_map();
+ if (dev->_rx->rps_map == NULL) {
+ kfree(dev->_rx);
+ dev->_rx = NULL;
+ ret = -ENOMEM;
+ goto out;
+ }
dev->_rx->first = dev->_rx;
atomic_set(&dev->_rx->count, 1);
dev->num_rx_queues = 1;
@@ -5688,8 +5712,12 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
* Set a pointer to first element in the array which holds the
* reference count.
*/
- for (i = 0; i < queue_count; i++)
+ for (i = 0; i < queue_count; i++) {
rx[i].first = rx;
+ rx[i].rps_map = alloc_rps_map();
+ if (rx[i].rps_map == NULL)
+ goto free_rx;
+ }
dev = PTR_ALIGN(p, NETDEV_ALIGN);
dev->padded = (char *)dev - (char *)p;
@@ -5723,6 +5751,8 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
return dev;
free_rx:
+ for (i = 0; i < queue_count; i++)
+ kfree(rx[i].rps_map);
kfree(rx);
free_tx:
kfree(tx);
^ 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