* 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: 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: [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: [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 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: 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: 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: 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: 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: [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: 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: [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: [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: 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: [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: [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: Gianfar: RX Recycle skb->len error
From: David Miller @ 2010-03-23 3:30 UTC (permalink / raw)
To: ben; +Cc: avorontsov, netdev, Sandeep.Kumar
In-Reply-To: <A6A1774AFD79E346AE6D49A33CB294530DC19F18@EX-BE-017-SFO.shared.themessagecenter.com>
From: "Ben Menchaca (ben@bigfootnetworks.com)" <ben@bigfootnetworks.com>
Date: Mon, 22 Mar 2010 14:10:48 -0700
> It's yucky, but skb_reserve(skb, -alignamount) works,
I have no problem with people using that.
> It seems in both these cases for gianfar, the amount of the
> alignment is not immediately available to the code the
> recognizes that the skb_reset_reserved() is required. A bit
> larger rework appears to be needed.
There's no need to make this so complicated. Just remember the
value and then refer to it later, when needed.
struct gianfar_skb_cb {
int alignamount;
};
#define GIANFAR_CB(skb) ((struct gianfar_skb_cb *)((skb)->cb))
...
GIANFAR_CB(skb)->alignamount = alignamount;
...
skb_reserve(skb, -GIANFAR_CB(skb)->alignamount);
^ permalink raw reply
* Re: [PATCH] net_sched: make traffic control network namespace aware
From: David Miller @ 2010-03-23 3:26 UTC (permalink / raw)
To: kaber; +Cc: thomas.goff, netdev
In-Reply-To: <4BA7B13C.7020304@trash.net>
From: Patrick McHardy <kaber@trash.net>
Date: Mon, 22 Mar 2010 19:04:44 +0100
> Patrick McHardy wrote:
>> Tom Goff wrote:
>>> Mostly minor changes to add a net argument to various functions and
>>> remove initial network namespace checks.
>>>
>>> Make /proc/net/psched per network namespace.
>>
>> Looks fine from a qdisc POV. One thing that appears to be missing
>> though is teql master netdev registration in other than the initial
>> namespace.
>
> Actually we could take this opportunity and add rtnl_link support
> for teql device registration. I can look into this in a couple of
> days.
I've applied Tom's patch to net-next-2.6, thanks everyone.
^ permalink raw reply
* Re: [PATCH] netlink: use the appropriate namespace pid
From: David Miller @ 2010-03-23 3:25 UTC (permalink / raw)
To: thomas.goff; +Cc: netdev
In-Reply-To: <20100320013850.GA13239@boeing.com>
From: Tom Goff <thomas.goff@boeing.com>
Date: Fri, 19 Mar 2010 18:38:50 -0700
> This was included in OpenVZ kernels but wasn't integrated upstream.
>>From git://git.openvz.org/pub/linux-2.6.24-openvz:
>
> commit 5c69402f18adf7276352e051ece2cf31feefab02
> Author: Alexey Dobriyan <adobriyan@openvz.org>
> Date: Mon Dec 24 14:37:45 2007 +0300
>
> netlink: fixup ->tgid to work in multiple PID namespaces
>
> Signed-off-by: Tom Goff <thomas.goff@boeing.com>
BTW, we're ok without this.
The 'pid' in this code is just an arbitrary port number. We are
merely using the tgid to seed the port allocator, so it doesn't have
to be the real proper tgid.
I'm therefore dropping this for now. If Alexey can show why we
might need this, we can revisit the patch.
^ permalink raw reply
* Re: [PATCH] netdev: don't always reset iflink when registering
From: David Miller @ 2010-03-23 3:20 UTC (permalink / raw)
To: Thomas.Goff; +Cc: kaber, netdev
In-Reply-To: <6AFD35BEDB60334C9A31FD694E13EF0D22676C2244@XCH-NW-18V.nw.nos.boeing.com>
From: "Goff, Thomas" <Thomas.Goff@boeing.com>
Date: Mon, 22 Mar 2010 12:06:46 -0700
> So maybe it makes more sense to change these if that's more consistent
> with other cases.
Yes, it does. The canonical place to set ->iflink by hand
is the ->ndo_init() method of the device.
^ permalink raw reply
* Re: [PATCH] ixgbevf: Message formatting fixup
From: David Miller @ 2010-03-23 3:08 UTC (permalink / raw)
To: joe; +Cc: gregory.v.rose, netdev, jeffrey.t.kirsher
In-Reply-To: <1269285157.22616.55.camel@Joe-Laptop.home>
From: Joe Perches <joe@perches.com>
Date: Mon, 22 Mar 2010 12:12:37 -0700
> Commit 29b8dd024bd48c3d1d1e5140f5bbb683786f998e
> left a trailing ", " after a message.
> Fix it and make the text used a bit smaller when DEBUG is #defined
>
> Signed-off-by: Joe Perches <joe@perches.com>
Looks good, applied, thanks Joe.
^ permalink raw reply
* Re: [PATCH] can: bfin_can: switch to common Blackfin can header
From: David Miller @ 2010-03-23 3:06 UTC (permalink / raw)
To: vapier-aBrp7R+bbdUdnm+yROfE0A
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA,
uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b,
oliver.hartkopp-l29pVbxQd1IUtdQbppsyvg,
urs.thuermann-l29pVbxQd1IUtdQbppsyvg
In-Reply-To: <1269241561-28956-1-git-send-email-vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
From: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
Date: Mon, 22 Mar 2010 03:06:01 -0400
> The MMR bits are being moved to this header, so include it.
>
> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
> Acked-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
Applied.
^ permalink raw reply
* Re: [Patch v2] netpoll: warn when there are spaces in parameters
From: David Miller @ 2010-03-23 3:06 UTC (permalink / raw)
To: nhorman; +Cc: amwang, linux-kernel, netdev, elendil
In-Reply-To: <20100322123046.GA4288@hmsreliant.think-freely.org>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Mon, 22 Mar 2010 08:30:46 -0400
> On Mon, Mar 22, 2010 at 04:59:58AM -0400, Amerigo Wang wrote:
>> v2: update according to Frans' comments.
>>
>> Currently, if we leave spaces before dst port,
>> netconsole will silently accept it as 0. Warn about this.
>>
>> Also, when spaces appear in other places, make them
>> visible in error messages.
>>
>> Signed-off-by: WANG Cong <amwang@redhat.com>
...
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH] via-velocity: FLOW_CNTL_RX does not disable Asymmetric pause in set_mii_flow_control()
From: David Miller @ 2010-03-23 2:58 UTC (permalink / raw)
To: roel.kluin; +Cc: netdev, akpm, linux-kernel, davej
In-Reply-To: <4BA7D22A.6080300@gmail.com>
From: Roel Kluin <roel.kluin@gmail.com>
Date: Mon, 22 Mar 2010 21:25:14 +0100
> Asymmetric pause was not disabled in the RX case.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> I spotted this because In the FLOW_CNTL_RX and FLOW_CNTL_TX_RX
> cases the same code is executed, is the amendment below required?
The bit settings in the MII_ADVERTISE register should be:
1) RX and TX pause both requested
set only ADVERTISE_PAUSE_CAP
2) Only TX pause requested
set only ADVERTISE_PAUSE_ASYM
3) Only RX pause requested
set both ADVERTISE_PAUSE_CAP and ADVERTISE_PAUSE_ASYM
4) Neither TX nor RX pause requested
set nothing
(drivers/net/tg3.c:tg3_advert_flowctrl_1000t() is a good
reference point)
It would be easier to compare the VIA driver with the rest of
the world if it used the standard mii.h defines instead of
it's own private ones. That would be a good change to make at
some point.
Anyways, the correct fix is therefore something like:
diff --git a/drivers/net/via-velocity.c b/drivers/net/via-velocity.c
index 3a486f3..bc278d4 100644
--- a/drivers/net/via-velocity.c
+++ b/drivers/net/via-velocity.c
@@ -812,7 +812,7 @@ static void set_mii_flow_control(struct velocity_info *vptr)
case FLOW_CNTL_TX_RX:
MII_REG_BITS_ON(ANAR_PAUSE, MII_REG_ANAR, vptr->mac_regs);
- MII_REG_BITS_ON(ANAR_ASMDIR, MII_REG_ANAR, vptr->mac_regs);
+ MII_REG_BITS_OFF(ANAR_ASMDIR, MII_REG_ANAR, vptr->mac_regs);
break;
case FLOW_CNTL_DISABLE:
^ permalink raw reply related
* Re: [RFC Patch 1/3] netpoll: add generic support for bridge and bonding devices
From: Cong Wang @ 2010-03-23 2:13 UTC (permalink / raw)
To: Matt Mackall
Cc: linux-kernel, netdev, bridge, Andy Gospodarek, Neil Horman,
Stephen Hemminger, bonding-devel, Jay Vosburgh, David Miller,
Jeff Moyer
In-Reply-To: <1269297081.3552.19.camel@calx>
Matt Mackall wrote:
> On Mon, 2010-03-22 at 04:17 -0400, Amerigo Wang wrote:
>> This whole patchset is for adding netpoll support to bridge and bonding
>> devices. I already tested it for bridge, bonding, bridge over bonding,
>> and bonding over bridge. It looks fine now.
>
> Ages ago, Jeff Moyer took a run at this, added him to the cc: on the off
> chance he still cares.
>
>> Please comment.
>>
>>
>> To make bridge and bonding support netpoll, we need to adjust
>> some netpoll generic code. This patch does the following things:
>>
>> 1) introduce two new priv_flags for struct net_device:
>> IFF_IN_NETPOLL which identifies we are processing a netpoll;
>> IFF_DISABLE_NETPOLL is used to disable netpoll support for a device
>> at run-time;
>
> This one is a little worrisome. I've tried to keep the netpoll code
> restricted to as tight an area as possible. Adding new flags like these
> that random drivers might try to fiddle with seems like a good way for a
> driver writer to get in trouble. Also flag space is filling up.
Somewhat, but currently I don't have other way to replace this.
Any suggestions?
>
>> 2) introduce three new methods for netdev_ops:
>> ->ndo_netpoll_setup() is used to setup netpoll for a device;
>> ->ndo_netpoll_xmit() is used to transmit netpoll requests;
>> ->ndo_netpoll_cleanup() is used to clean up netpoll when a device is
>> removed.
>
> 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?
Thanks!
^ 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