Netdev List
 help / color / mirror / Atom feed
* Re: [RFC v2 1/2] PCI-Express Non-Transparent Bridge Support
From: chetan loke @ 2012-07-31 18:02 UTC (permalink / raw)
  To: Jon Mason; +Cc: Bjorn Helgaas, linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <20120731172709.GB14080@jonmason-lab>

On Tue, Jul 31, 2012 at 1:27 PM, Jon Mason <jon.mason@intel.com> wrote:
>
> I don't see the benefit of having the driver in staging.  Any vendors
> who would notice the ntb driver in staging would be sitting on these
> mailing lists and hopefully have planety of comments on the design.
> Stashing the driver in staging while waiting for these comments (which
> may never come) doesn't seem the best course of action.
>

I thought that since others are talking about it then may be there is
some WIP code for foo-NTB. Seems like that's not the case. So no need
to stage.

Correct me if I'm wrong but wouldn't apps just open a socket and route
data via ntb_vir_eth_dev? So I don't see an ABI breakage issue and
hence nothing would prevent us from changing the kernel parts(for
accommodating some foo-NTB part) in future.

It may not be a bad idea to prefix intel-specific(if any)
ntb_structs/variables/logic with the 'intc'(Intel ticker or pick your
string) keyword.

Chetan Loke

^ permalink raw reply

* Re: [PATCH v2 3/6] mv643xx.c: Add basic device tree support.
From: Arnd Bergmann @ 2012-07-31 18:23 UTC (permalink / raw)
  To: Ian Molton; +Cc: linux-arm-kernel, andrew, thomas.petazzoni, ben.dooks, netdev
In-Reply-To: <1343749529-17571-4-git-send-email-ian.molton@codethink.co.uk>

On Tuesday 31 July 2012, Ian Molton wrote:
> @@ -33,6 +34,10 @@ struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = {
>  	OF_DEV_AUXDATA("marvell,orion-wdt", 0xf1020300, "orion_wdt", NULL),
>  	OF_DEV_AUXDATA("marvell,orion-sata", 0xf1080000, "sata_mv.0", NULL),
>  	OF_DEV_AUXDATA("marvell,orion-nand", 0xf4000000, "orion_nand", NULL),
> +	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1072000, MV643XX_ETH_NAME ".0",
> +			NULL),
> +	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1076000, MV643XX_ETH_NAME ".1",
> +			NULL),
>  	{},
>  };

Please don't do string concatenation like this, it just makes it harder to grep for the
strings.

> @@ -2654,15 +2677,22 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
>  	/*
>  	 * Check whether the error interrupt is hooked up.
>  	 */
> -	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> -	if (res != NULL) {
> +	if (pdev->dev.of_node) {
> +		irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> +	} else {
> +		res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +		if (res)
> +			irq = res->start;
> +	}
> +

Why is this necessary? In theory, the old code should be the same as the new one,
unless something goes wrong in the domain registration.

	Arnd

^ permalink raw reply

* wlcore: might_sleep operation in interrupt handler
From: Denis Yefremov @ 2012-07-31 18:28 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: John W. Linville, Eliad Peller, Arik Nemtsov, Eyal Shapira,
	linux-wireless, netdev, linux-kernel, ldv-project

Interrupt handler wlcore_irq in wlcore/main.c file
invokes mutex_lock that is might sleep operation.

Found by Linux Driver Verification project (linuxtesting.org)
within my participation in the GSoC 2012 program.

drivers/net/wireless/ti/wlcore/main.c

static irqreturn_t wlcore_irq(int irq, void *cookie)
{
        int ret;
        unsigned long flags;
        struct wl1271 *wl = cookie;

        /* TX might be handled here, avoid redundant work */
        set_bit(WL1271_FLAG_TX_PENDING, &wl->flags);
        cancel_work_sync(&wl->tx_work);

        mutex_lock(&wl->mutex);
...
}

int __devinit wlcore_probe(struct wl1271 *wl, struct platform_device *pdev)
{
...
        ret = request_threaded_irq(wl->irq, wl12xx_hardirq, wlcore_irq,
                                   irqflags,
                                   pdev->name, wl);
...
}

^ permalink raw reply

* cpu usage on blocking network call
From: Robert Engels @ 2012-07-31 19:14 UTC (permalink / raw)
  To: netdev

I have a strange problem, and was hoping the developers could provide
some insight.

I have tested with ubuntu 12.04, with the 3.2.0-23 lowlatency kernel.
I also tested with a 3.0.31 vanilla realtime kernel on a different
machine.

I open a blocking socket (and connect to a remote client), and start
two threads, a reader and a writer.

I pump LOTS of data down the writer, but do not receive any data on
the reader (by design).

The reader thread is reporting some CPU usage (not a lot, about 10% of
the writer cpu). I determine this using "top", and other profiling
tools.

I also run strace on the thread, and it shows that it is blocked in
read() call, and that is doesn't return.

Why/How is the reader thread consuming CPU? Poorly written driver?
Kernel bug? Or is this expected behavior or the kernel?

I am more than willing to spend more time tracking it down in the
kernel, but I'd like some direction.

--

Robert Engels
OptionsCity Software Inc.
Chicago, IL

^ permalink raw reply

* Re: [PATCH v2 3/6] mv643xx.c: Add basic device tree support.
From: Andrew Lunn @ 2012-07-31 19:24 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Ian Molton, linux-arm-kernel, andrew, thomas.petazzoni, ben.dooks,
	netdev
In-Reply-To: <201207311823.54857.arnd@arndb.de>

On Tue, Jul 31, 2012 at 06:23:54PM +0000, Arnd Bergmann wrote:
> On Tuesday 31 July 2012, Ian Molton wrote:
> > @@ -33,6 +34,10 @@ struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = {
> >  	OF_DEV_AUXDATA("marvell,orion-wdt", 0xf1020300, "orion_wdt", NULL),
> >  	OF_DEV_AUXDATA("marvell,orion-sata", 0xf1080000, "sata_mv.0", NULL),
> >  	OF_DEV_AUXDATA("marvell,orion-nand", 0xf4000000, "orion_nand", NULL),
> > +	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1072000, MV643XX_ETH_NAME ".0",
> > +			NULL),
> > +	OF_DEV_AUXDATA("marvell,mv643xx", 0xf1076000, MV643XX_ETH_NAME ".1",
> > +			NULL),
> >  	{},
> >  };
> 
> Please don't do string concatenation like this, it just makes it harder to grep for the
> strings.

Hi Arnd

This pattern is used in other places, e.g. when creating the
clocks. The macro MV643XX_ETH_NAME is also used when creating the
platform data, and in the driver itself. 

Are you suggested we replace this with "mv643xx_eth.0"? Or is adding
the macro

#define MV643XX_ETH_NAME0 "mv643xx_eth.0" 

in include/linux/mv643xx_eth.h O.K, since grep will find it, and
induce the grep'er to perform a second grep on the macro?

     Andrew

^ permalink raw reply

* Re: [PATCH v2] ipv4: percpu nh_rth_output cache
From: Alexander Duyck @ 2012-07-31 20:10 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1343749530.21269.336.camel@edumazet-glaptop>

On 07/31/2012 08:45 AM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Input path is mostly run under RCU and doesnt touch dst refcnt
>
> But output path on forwarding or UDP workloads hits
> badly dst refcount, and we have lot of false sharing, for example
> in ipv4_mtu() when reading rt->rt_pmtu
>
> Using a percpu cache for nh_rth_output gives a nice performance
> increase at a small cost.
>
> 24 udpflood test on my 24 cpu machine (dummy0 output device)
> (each process sends 1.000.000 udp frames, 24 processes are started)
>
> before : 5.24 s
> after : 2.06 s
> For reference, time on linux-3.5 : 6.60 s
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> v2: use __this_cpu_ptr() and slighly better annotations to avoid
> ugly casts
>
> On top on previous "ipv4: Restore old dst_free() behavior" patch
>
> We probably can remove all paddings in struct dst_entry
>
I've done some quick testing and it looks like it has little to no
effect on routing performance in my system, but for UDP workloads it is
making a huge difference.  I just ran a simple test with 16 sessions of
netperf all sending UDP small packets.  Without your patch it runs at
just over 2.7Mpps, with your patch it is runs at over 10.5Mpps.

Tested-by: Alexander Duyck <alexander.h.duyck@intel.com>

^ permalink raw reply

* Re: wlcore: might_sleep operation in interrupt handler
From: Johannes Berg @ 2012-07-31 20:14 UTC (permalink / raw)
  To: Denis Yefremov
  Cc: Luciano Coelho, John W. Linville, Eliad Peller, Arik Nemtsov,
	Eyal Shapira, linux-wireless, netdev, linux-kernel, ldv-project
In-Reply-To: <CADKZ3FKDw78ac9JPXJuw0s7Mqg1RrVfonbjXZ6zHmtwF2i5HoQ@mail.gmail.com>

On Tue, 2012-07-31 at 22:28 +0400, Denis Yefremov wrote:
> Interrupt handler wlcore_irq in wlcore/main.c file
> invokes mutex_lock that is might sleep operation.

> int __devinit wlcore_probe(struct wl1271 *wl, struct platform_device *pdev)
> {
> ...
>         ret = request_threaded_irq(wl->irq, wl12xx_hardirq, wlcore_irq,
>                                    irqflags,
>                                    pdev->name, wl);

Yes, umm, why do you think they're using a *threaded* IRQ? :-)

johannes

^ permalink raw reply

* Re: [PATCH v2 3/6] mv643xx.c: Add basic device tree support.
From: Arnd Bergmann @ 2012-07-31 20:18 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Ian Molton, linux-arm-kernel, thomas.petazzoni, ben.dooks, netdev
In-Reply-To: <20120731192458.GE28852@lunn.ch>

On Tuesday 31 July 2012, Andrew Lunn wrote:
> This pattern is used in other places, e.g. when creating the
> clocks. The macro MV643XX_ETH_NAME is also used when creating the
> platform data, and in the driver itself. 
> 
> Are you suggested we replace this with "mv643xx_eth.0"? Or is adding
> the macro
> 
> #define MV643XX_ETH_NAME0 "mv643xx_eth.0" 
> 
> in include/linux/mv643xx_eth.h O.K, since grep will find it, and
> induce the grep'er to perform a second grep on the macro?

I meant the former. Just remove the macro entirely, or at least
don't add new users.

	Arnd

^ permalink raw reply

* Re: [PATCH] igb: correct hardware type (i210/i211) check in igb_loopback_test()
From: Jesper Juhl @ 2012-07-31 20:23 UTC (permalink / raw)
  To: Wyborny, Carolyn, David S. Miller
  Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	Rick Jones, Allan, Bruce W, linux-kernel@vger.kernel.org,
	Brandeburg, Jesse, Ronciak, John
In-Reply-To: <9BBC4E0CF881AA4299206E2E1412B62630D64753@ORSMSX102.amr.corp.intel.com>

On Wed, 25 Jul 2012, Wyborny, Carolyn wrote:

> 
> > -----Original Message-----
> > From: Jesper Juhl [mailto:jj@chaosbits.net] 
> > Sent: Wednesday, July 25, 2012 12:06 PM
> > To: linux-kernel@vger.kernel.org
> > Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net; Wyborny, Carolyn; Pieper, Jeffrey E; Kirsher, Jeffrey T; Rick Jones; Ronciak, John; Brandeburg, Jesse; Allan, Bruce W; Skidmore, Donald C; Rose, Gregory V; Waskiewicz Jr, Peter P; Duyck, Alexander H; David S. Miller
> > Subject: [PATCH] igb: correct hardware type (i210/i211) check in igb_loopback_test()
> > 
> > In the original code
> > ...
> >  	if ((adapter->hw.mac.type == e1000_i210)
> > 		|| (adapter->hw.mac.type == e1000_i210)) { ...
> > the second check of 'adapter->hw.mac.type' is pointless since it tests for the exact same value as the first.
> > 
> > After reading through a few other parts of the driver I believe that the second check was actually intended to check for 'e1000_i211'
> > rather than 'e1000_i210', but I admit that I'm not certain so someone with more knowledge about this driver should ACK the patch before it gets merged.
> > 
> > Unfortunately I have no hardware to actually test this on, so it is compile tested only.
> > 
> > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > ---
> >  drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > index a19c84c..ad489b7 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > @@ -1783,7 +1783,7 @@ static int igb_loopback_test(struct igb_adapter *adapter, u64 *data)
> >  		goto out;
> >  	}
> >  	if ((adapter->hw.mac.type == e1000_i210)
> > -		|| (adapter->hw.mac.type == e1000_i210)) {
> > +		|| (adapter->hw.mac.type == e1000_i211)) {
> >  		dev_err(&adapter->pdev->dev,
> >  			"Loopback test not supported "
> >  			"on this part at this time.\n");
> > --
> > 1.7.11.3
> > 
> 
> ACK.
> 
> Good catch.  Thanks Jesper!
> 
> Carolyn
> 
> Carolyn Wyborny
> Linux Development
> LAN Access Division
> Intel Corporation
> 
Thank you for the ack Carolyn.

David: will you take this in the networking tree?


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* Re: wlcore: might_sleep operation in interrupt handler
From: Denis Yefremov @ 2012-07-31 20:48 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Luciano Coelho, John W. Linville, Eliad Peller, Arik Nemtsov,
	Eyal Shapira, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	ldv-project-ufN2psIa012HXe+LvDLADg
In-Reply-To: <1343765698.4474.10.camel-8upI4CBIZJIJvtFkdXX2HixXY32XiHfO@public.gmane.org>

More precise (IRQF_ONESHOT flag may be used):

int __devinit wlcore_probe(struct wl1271 *wl, struct platform_device *pdev)
{
...
        if (wl->platform_quirks & WL12XX_PLATFORM_QUIRK_EDGE_IRQ)
                irqflags = IRQF_TRIGGER_RISING;
        else
                irqflags = IRQF_TRIGGER_HIGH | IRQF_ONESHOT;

        ret = request_threaded_irq(wl->irq, wl12xx_hardirq, wlcore_irq,
                                   irqflags,
                                   pdev->name, wl);


2012/8/1 Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>:
> On Tue, 2012-07-31 at 22:28 +0400, Denis Yefremov wrote:
>> Interrupt handler wlcore_irq in wlcore/main.c file
>> invokes mutex_lock that is might sleep operation.
>
>> int __devinit wlcore_probe(struct wl1271 *wl, struct platform_device *pdev)
>> {
>> ...
>>         ret = request_threaded_irq(wl->irq, wl12xx_hardirq, wlcore_irq,
>>                                    irqflags,
>>                                    pdev->name, wl);
>
> Yes, umm, why do you think they're using a *threaded* IRQ? :-)
>
> johannes
>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH iproute2] ss: Report MSS from internal TCP information
From: Stephen Hemminger @ 2012-07-31 21:07 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev
In-Reply-To: <1343677867.2667.33.camel@bwh-desktop.uk.solarflarecom.com>

On Mon, 30 Jul 2012 20:51:07 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:

> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
>  misc/ss.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/misc/ss.c b/misc/ss.c
> index cf529ef..8ad830b 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
> @@ -1392,6 +1392,8 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r)
>  			       (double)info->tcpi_rttvar/1000);
>  		if (info->tcpi_ato)
>  			printf(" ato:%g", (double)info->tcpi_ato/1000);
> +		if (info->tcpi_snd_mss)
> +			printf(" mss:%d", info->tcpi_snd_mss);
>  		if (info->tcpi_snd_cwnd != 2)
>  			printf(" cwnd:%d", info->tcpi_snd_cwnd);
>  		if (info->tcpi_snd_ssthresh < 0xFFFF)

Applied

^ permalink raw reply

* Re: [PATCH] igb: correct hardware type (i210/i211) check in igb_loopback_test()
From: Jeff Kirsher @ 2012-07-31 21:12 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	Rick Jones, Allan, Bruce W, linux-kernel@vger.kernel.org,
	Brandeburg, Jesse, Ronciak, John, David S. Miller
In-Reply-To: <alpine.LNX.2.00.1207312219080.7100@swampdragon.chaosbits.net>


[-- Attachment #1.1: Type: text/plain, Size: 2778 bytes --]

On Tue, 2012-07-31 at 22:23 +0200, Jesper Juhl wrote:
> On Wed, 25 Jul 2012, Wyborny, Carolyn wrote:
> 
> > 
> > > -----Original Message-----
> > > From: Jesper Juhl [mailto:jj@chaosbits.net] 
> > > Sent: Wednesday, July 25, 2012 12:06 PM
> > > To: linux-kernel@vger.kernel.org
> > > Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net;
> Wyborny, Carolyn; Pieper, Jeffrey E; Kirsher, Jeffrey T; Rick Jones;
> Ronciak, John; Brandeburg, Jesse; Allan, Bruce W; Skidmore, Donald C;
> Rose, Gregory V; Waskiewicz Jr, Peter P; Duyck, Alexander H; David S.
> Miller
> > > Subject: [PATCH] igb: correct hardware type (i210/i211) check in
> igb_loopback_test()
> > > 
> > > In the original code
> > > ...
> > >     if ((adapter->hw.mac.type == e1000_i210)
> > >             || (adapter->hw.mac.type == e1000_i210)) { ...
> > > the second check of 'adapter->hw.mac.type' is pointless since it
> tests for the exact same value as the first.
> > > 
> > > After reading through a few other parts of the driver I believe
> that the second check was actually intended to check for 'e1000_i211'
> > > rather than 'e1000_i210', but I admit that I'm not certain so
> someone with more knowledge about this driver should ACK the patch
> before it gets merged.
> > > 
> > > Unfortunately I have no hardware to actually test this on, so it
> is compile tested only.
> > > 
> > > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > > ---
> > >  drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > index a19c84c..ad489b7 100644
> > > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > @@ -1783,7 +1783,7 @@ static int igb_loopback_test(struct
> igb_adapter *adapter, u64 *data)
> > >             goto out;
> > >     }
> > >     if ((adapter->hw.mac.type == e1000_i210)
> > > -           || (adapter->hw.mac.type == e1000_i210)) {
> > > +           || (adapter->hw.mac.type == e1000_i211)) {
> > >             dev_err(&adapter->pdev->dev,
> > >                     "Loopback test not supported "
> > >                     "on this part at this time.\n");
> > > --
> > > 1.7.11.3
> > > 
> > 
> > ACK.
> > 
> > Good catch.  Thanks Jesper!
> > 
> > Carolyn
> > 
> > Carolyn Wyborny
> > Linux Development
> > LAN Access Division
> > Intel Corporation
> > 
> Thank you for the ack Carolyn.
> 
> David: will you take this in the networking tree? 

I have a few igb patches already, so I will send a pull request for Dave
with this patch added to the igb patches I already have.  Ok?

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 395 bytes --]

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* Re: [PATCH] igb: correct hardware type (i210/i211) check in igb_loopback_test()
From: Jesper Juhl @ 2012-07-31 21:19 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	Rick Jones, Allan, Bruce W, linux-kernel@vger.kernel.org,
	Brandeburg, Jesse, Ronciak, John, David S. Miller
In-Reply-To: <1343769121.2230.28.camel@jtkirshe-mobl>

On Tue, 31 Jul 2012, Jeff Kirsher wrote:

> On Tue, 2012-07-31 at 22:23 +0200, Jesper Juhl wrote:
> > On Wed, 25 Jul 2012, Wyborny, Carolyn wrote:
> > 
> > > 
> > > > -----Original Message-----
> > > > From: Jesper Juhl [mailto:jj@chaosbits.net] 
> > > > Sent: Wednesday, July 25, 2012 12:06 PM
> > > > To: linux-kernel@vger.kernel.org
> > > > Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net;
> > Wyborny, Carolyn; Pieper, Jeffrey E; Kirsher, Jeffrey T; Rick Jones;
> > Ronciak, John; Brandeburg, Jesse; Allan, Bruce W; Skidmore, Donald C;
> > Rose, Gregory V; Waskiewicz Jr, Peter P; Duyck, Alexander H; David S.
> > Miller
> > > > Subject: [PATCH] igb: correct hardware type (i210/i211) check in
> > igb_loopback_test()
> > > > 
> > > > In the original code
> > > > ...
> > > >     if ((adapter->hw.mac.type == e1000_i210)
> > > >             || (adapter->hw.mac.type == e1000_i210)) { ...
> > > > the second check of 'adapter->hw.mac.type' is pointless since it
> > tests for the exact same value as the first.
> > > > 
> > > > After reading through a few other parts of the driver I believe
> > that the second check was actually intended to check for 'e1000_i211'
> > > > rather than 'e1000_i210', but I admit that I'm not certain so
> > someone with more knowledge about this driver should ACK the patch
> > before it gets merged.
> > > > 
> > > > Unfortunately I have no hardware to actually test this on, so it
> > is compile tested only.
> > > > 
> > > > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > > > ---
> > > >  drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > > index a19c84c..ad489b7 100644
> > > > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > > @@ -1783,7 +1783,7 @@ static int igb_loopback_test(struct
> > igb_adapter *adapter, u64 *data)
> > > >             goto out;
> > > >     }
> > > >     if ((adapter->hw.mac.type == e1000_i210)
> > > > -           || (adapter->hw.mac.type == e1000_i210)) {
> > > > +           || (adapter->hw.mac.type == e1000_i211)) {
> > > >             dev_err(&adapter->pdev->dev,
> > > >                     "Loopback test not supported "
> > > >                     "on this part at this time.\n");
> > > > --
> > > > 1.7.11.3
> > > > 
> > > 
> > > ACK.
> > > 
> > > Good catch.  Thanks Jesper!
> > > 
> > > Carolyn
> > > 
> > > Carolyn Wyborny
> > > Linux Development
> > > LAN Access Division
> > > Intel Corporation
> > > 
> > Thank you for the ack Carolyn.
> > 
> > David: will you take this in the networking tree? 
> 
> I have a few igb patches already, so I will send a pull request for Dave
> with this patch added to the igb patches I already have.  Ok?
> 
That's perfectly fine with me.

Perhaps you could consider also picking up the other one I sent a few 
minutes after this one ?
  Subject: [PATCH] igb: don't break user visible strings over multiple lines in igb_ethtool.c
  Message-ID: <alpine.LNX.2.00.1207252115321.11754@swampdragon.chaosbits.net>


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* Re: [PATCH] igb: don't break user visible strings over multiple lines in igb_ethtool.c
From: Jeff Kirsher @ 2012-07-31 21:30 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: linux-kernel, netdev, e1000-devel, Carolyn Wyborny, Jeff Pieper,
	Rick Jones, Jesse Brandeburg, Bruce Allan, Don Skidmore,
	Greg Rose, Peter P Waskiewicz Jr, Alex Duyck, David S. Miller
In-Reply-To: <alpine.LNX.2.00.1207252115321.11754@swampdragon.chaosbits.net>

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

On Wed, 2012-07-25 at 21:19 +0200, Jesper Juhl wrote:
> Even when they go beyond 80 characters, user visible strings should be
> on one line to make them easy to grep for.
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
>  drivers/net/ethernet/intel/igb/igb_ethtool.c | 23
> +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-) 

Applied to my queue of patches, thanks Jesper.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH] igb: correct hardware type (i210/i211) check in igb_loopback_test()
From: Jeff Kirsher @ 2012-07-31 21:30 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	Rick Jones, Allan, Bruce W, linux-kernel@vger.kernel.org,
	Brandeburg, Jesse, Ronciak, John, David S. Miller
In-Reply-To: <alpine.LNX.2.00.1207312315310.7100@swampdragon.chaosbits.net>


[-- Attachment #1.1: Type: text/plain, Size: 3506 bytes --]

On Tue, 2012-07-31 at 23:19 +0200, Jesper Juhl wrote:
> On Tue, 31 Jul 2012, Jeff Kirsher wrote:
> 
> > On Tue, 2012-07-31 at 22:23 +0200, Jesper Juhl wrote:
> > > On Wed, 25 Jul 2012, Wyborny, Carolyn wrote:
> > > 
> > > > 
> > > > > -----Original Message-----
> > > > > From: Jesper Juhl [mailto:jj@chaosbits.net] 
> > > > > Sent: Wednesday, July 25, 2012 12:06 PM
> > > > > To: linux-kernel@vger.kernel.org
> > > > > Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net;
> > > Wyborny, Carolyn; Pieper, Jeffrey E; Kirsher, Jeffrey T; Rick Jones;
> > > Ronciak, John; Brandeburg, Jesse; Allan, Bruce W; Skidmore, Donald C;
> > > Rose, Gregory V; Waskiewicz Jr, Peter P; Duyck, Alexander H; David S.
> > > Miller
> > > > > Subject: [PATCH] igb: correct hardware type (i210/i211) check in
> > > igb_loopback_test()
> > > > > 
> > > > > In the original code
> > > > > ...
> > > > >     if ((adapter->hw.mac.type == e1000_i210)
> > > > >             || (adapter->hw.mac.type == e1000_i210)) { ...
> > > > > the second check of 'adapter->hw.mac.type' is pointless since it
> > > tests for the exact same value as the first.
> > > > > 
> > > > > After reading through a few other parts of the driver I believe
> > > that the second check was actually intended to check for 'e1000_i211'
> > > > > rather than 'e1000_i210', but I admit that I'm not certain so
> > > someone with more knowledge about this driver should ACK the patch
> > > before it gets merged.
> > > > > 
> > > > > Unfortunately I have no hardware to actually test this on, so it
> > > is compile tested only.
> > > > > 
> > > > > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > > > > ---
> > > > >  drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > > > index a19c84c..ad489b7 100644
> > > > > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > > > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > > > @@ -1783,7 +1783,7 @@ static int igb_loopback_test(struct
> > > igb_adapter *adapter, u64 *data)
> > > > >             goto out;
> > > > >     }
> > > > >     if ((adapter->hw.mac.type == e1000_i210)
> > > > > -           || (adapter->hw.mac.type == e1000_i210)) {
> > > > > +           || (adapter->hw.mac.type == e1000_i211)) {
> > > > >             dev_err(&adapter->pdev->dev,
> > > > >                     "Loopback test not supported "
> > > > >                     "on this part at this time.\n");
> > > > > --
> > > > > 1.7.11.3
> > > > > 
> > > > 
> > > > ACK.
> > > > 
> > > > Good catch.  Thanks Jesper!
> > > > 
> > > > Carolyn
> > > > 
> > > > Carolyn Wyborny
> > > > Linux Development
> > > > LAN Access Division
> > > > Intel Corporation
> > > > 
> > > Thank you for the ack Carolyn.
> > > 
> > > David: will you take this in the networking tree? 
> > 
> > I have a few igb patches already, so I will send a pull request for Dave
> > with this patch added to the igb patches I already have.  Ok?
> > 
> That's perfectly fine with me.
> 
> Perhaps you could consider also picking up the other one I sent a few 
> minutes after this one ?
>   Subject: [PATCH] igb: don't break user visible strings over multiple lines in igb_ethtool.c
>   Message-ID: <alpine.LNX.2.00.1207252115321.11754@swampdragon.chaosbits.net>

Done.

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 395 bytes --]

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* [PATCH] bnx2x: fix mem leak when command is unknown
From: Jesper Juhl @ 2012-07-31 21:39 UTC (permalink / raw)
  To: Eilon Greenstein; +Cc: netdev, linux-kernel

In bnx2x_mcast_enqueue_cmd() we'll leak the memory allocated to
'new_cmd' if we hit the deafault case of the 'switch (cmd)'.
Add a 'kfree(new_cmd)' to that case to avoid the leak.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c | 1 +
 1 file changed, 1 insertion(+)

  note: due to lack of hardware this patch is compile tested only.

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
index 734fd87..62f754b 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
@@ -2485,6 +2485,7 @@ static int bnx2x_mcast_enqueue_cmd(struct bnx2x *bp,
 		break;
 
 	default:
+		kfree(new_cmd);
 		BNX2X_ERR("Unknown command: %d\n", cmd);
 		return -EINVAL;
 	}
-- 
1.7.11.3


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* Re: [PATCH v2] ipv4: Restore old dst_free() behavior.
From: David Miller @ 2012-07-31 21:42 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1343732903.21269.190.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 31 Jul 2012 13:08:23 +0200

> [PATCH v2] ipv4: Restore old dst_free() behavior
> 
> commit 404e0a8b6a55 (net: ipv4: fix RCU races on dst refcounts) tried
> to solve a race but added a problem at device/fib dismantle time :
> 
> We really want to call dst_free() as soon as possible, even if sockets
> still have dst in their cache.
> dst_release() calls in free_fib_info_rcu() are not welcomed.
> 
> Root of the problem was that now we also cache output routes (in
> nh_rth_output), we must use call_rcu() instead of call_rcu_bh() in
> rt_free(), because output route lookups are done in process context.
> 
> Based on feedback and initial patch from David Miller (adding another
> call_rcu_bh() call in fib, but it appears it was not the right fix)
> 
> I left the inet_sk_rx_dst_set() helper and added __rcu attributes
> to nh_rth_output and nh_rth_input to better document what is going on in
> this code.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied, thanks Eric.

^ permalink raw reply

* Re: [PATCH v2] ipv4: percpu nh_rth_output cache
From: David Miller @ 2012-07-31 21:43 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, alexander.h.duyck
In-Reply-To: <1343749530.21269.336.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 31 Jul 2012 17:45:30 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> Input path is mostly run under RCU and doesnt touch dst refcnt
> 
> But output path on forwarding or UDP workloads hits
> badly dst refcount, and we have lot of false sharing, for example
> in ipv4_mtu() when reading rt->rt_pmtu
> 
> Using a percpu cache for nh_rth_output gives a nice performance
> increase at a small cost.
> 
> 24 udpflood test on my 24 cpu machine (dummy0 output device)
> (each process sends 1.000.000 udp frames, 24 processes are started)
> 
> before : 5.24 s
> after : 2.06 s
> For reference, time on linux-3.5 : 6.60 s
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

However I think fib_create_info() should fail if it cannot alloc_percpu()
successfully, instead of having funny logic in rt_cache_route().

^ permalink raw reply

* Re: [PATCH 0/7] Deconstruct struct fib_result
From: David Miller @ 2012-07-31 21:43 UTC (permalink / raw)
  To: alexander.duyck; +Cc: eric.dumazet, netdev
In-Reply-To: <CAKgT0UcOn48gVsC37hiNCXCP=FwqKjZnjkLviCOW8TVWGkGGVA@mail.gmail.com>

From: Alexander Duyck <alexander.duyck@gmail.com>
Date: Tue, 31 Jul 2012 10:27:40 -0700

> On Fri, Jul 27, 2012 at 9:18 PM, David Miller <davem@davemloft.net> wrote:
>>
>> This patch set tries to move towards reducing struct fib_result down
>> to it's absolute minimum.
>>
>> The eventual idea is to make it so that fib_lookup() simply
>> returns a "struct fib_nh *" and pointer encoded errnos, instead
>> of writing into a complicated structure as the return value on
>> the stack as is done now.
>>
>> Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> I tested these patches most of yesterday and I haven't seen any
> issues.  Then again I haven't seen much of an improvement either.
> 
> Tested-by: Alexander Duyck <alexander.h.duyck@intel.com>

Thanks for testing.

I'm putting this patch set to the side until I can find a way
to get past the tclassid barrier and also make it show some
difference performance wise.

^ permalink raw reply

* Re: [PATCH v2] dynamic_debug: Restore dev_dbg functionality, optimize stack
From: Andrew Morton @ 2012-07-31 21:55 UTC (permalink / raw)
  To: Joe Perches
  Cc: Greg Kroah-Hartman, David S. Miller, Jason Baron, Jim Cromie,
	Kay Sievers, linux-kernel, netdev
In-Reply-To: <333324dcdfdc891ba214d2de8f9a3d056dd7aea9.1343461712.git.joe@perches.com>

On Sat, 28 Jul 2012 00:55:07 -0700
Joe Perches <joe@perches.com> wrote:

> commit c4e00daaa9 ("driver-core: extend dev_printk() to pass structured data")
> changed __dev_printk and broke dynamic-debug's ability to control the
> dynamic prefix of dev_dbg(dev,..).
> 
> dynamic_emit_prefix() adds "[tid] module:func:line:" to the output and
> those additions got lost.
> 
> In addition, the current dynamic debug code uses up to 3 recursion
> levels via %pV.  This can consume quite a bit of stack.  Directly
> call printk_emit to reduce the recursion depth.
> 
> These changes include:
> 
> o Remove KERN_DEBUG from dynamic_emit_prefix
> o Create and use function create_syslog_header to format the syslog
>   header for printk_emit uses.
> o Call create_syslog_header and neaten __dev_printk
> o Call create_syslog_header and printk_emit from dynamic_dev_dbg
> o Call create_syslog_header and printk_emit from dynamic_netdev_dbg
> o Make __dev_printk and __netdev_printk static not global
> o Remove include header declarations of __dev_printk and __netdev_printk
> o Remove now unused EXPORT_SYMBOL()s of __dev_printk and __netdev_printk
> o Whitespace neatening
> 
> Changes in v2:
> 
> o Fix dynamic_emit_prefix to always initialize output
> o Call create_syslog_header and emit_printk from__netdev_printk and
>   eliminate call to dev_printk to remove another recursion via %pV

This one causes an oops-on-boot
(http://ozlabs.org/~akpm/stuff/IMG_20120731_144047.jpg).  v1 didn't do
that.

config: http://ozlabs.org/~akpm/stuff/config-akpm2

^ permalink raw reply

* [PATCH 1/2] ipv4: Cache routes in nexthop exception entries.
From: David Miller @ 2012-07-31 22:20 UTC (permalink / raw)
  To: netdev


Signed-off-by: David S. Miller <davem@davemloft.net>
---

This is just a respin of what I posted the other day, against Eric's
fixes of today.  Pushed to 'net'.

 include/net/ip_fib.h     |    1 +
 net/ipv4/fib_semantics.c |   39 ++++++++++--------
 net/ipv4/route.c         |  103 +++++++++++++++++++++++++---------------------
 3 files changed, 79 insertions(+), 64 deletions(-)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index e331746..926142e 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -55,6 +55,7 @@ struct fib_nh_exception {
 	u32				fnhe_pmtu;
 	__be32				fnhe_gw;
 	unsigned long			fnhe_expires;
+	struct rtable __rcu		*fnhe_rth;
 	unsigned long			fnhe_stamp;
 };
 
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index fe2ca02..da80dc1 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -140,6 +140,21 @@ const struct fib_prop fib_props[RTN_MAX + 1] = {
 	},
 };
 
+static void rt_fibinfo_free(struct rtable __rcu **rtp)
+{
+	struct rtable *rt = rcu_dereference_protected(*rtp, 1);
+
+	if (!rt)
+		return;
+
+	/* Not even needed : RCU_INIT_POINTER(*rtp, NULL);
+	 * because we waited an RCU grace period before calling
+	 * free_fib_info_rcu()
+	 */
+
+	dst_free(&rt->dst);
+}
+
 static void free_nh_exceptions(struct fib_nh *nh)
 {
 	struct fnhe_hash_bucket *hash = nh->nh_exceptions;
@@ -153,6 +168,9 @@ static void free_nh_exceptions(struct fib_nh *nh)
 			struct fib_nh_exception *next;
 			
 			next = rcu_dereference_protected(fnhe->fnhe_next, 1);
+
+			rt_fibinfo_free(&fnhe->fnhe_rth);
+
 			kfree(fnhe);
 
 			fnhe = next;
@@ -161,22 +179,7 @@ static void free_nh_exceptions(struct fib_nh *nh)
 	kfree(hash);
 }
 
-static void rt_nexthop_free(struct rtable __rcu **rtp)
-{
-	struct rtable *rt = rcu_dereference_protected(*rtp, 1);
-
-	if (!rt)
-		return;
-
-	/* Not even needed : RCU_INIT_POINTER(*rtp, NULL);
-	 * because we waited an RCU grace period before calling
-	 * free_fib_info_rcu()
-	 */
-
-	dst_free(&rt->dst);
-}
-
-static void rt_nexthop_free_cpus(struct rtable __rcu * __percpu *rtp)
+static void rt_fibinfo_free_cpus(struct rtable __rcu * __percpu *rtp)
 {
 	int cpu;
 
@@ -203,8 +206,8 @@ static void free_fib_info_rcu(struct rcu_head *head)
 			dev_put(nexthop_nh->nh_dev);
 		if (nexthop_nh->nh_exceptions)
 			free_nh_exceptions(nexthop_nh);
-		rt_nexthop_free_cpus(nexthop_nh->nh_pcpu_rth_output);
-		rt_nexthop_free(&nexthop_nh->nh_rth_input);
+		rt_fibinfo_free_cpus(nexthop_nh->nh_pcpu_rth_output);
+		rt_fibinfo_free(&nexthop_nh->nh_rth_input);
 	} endfor_nexthops(fi);
 
 	release_net(fi->fib_net);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 4f6276c..b102eeb 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -587,11 +587,17 @@ static void ip_rt_build_flow_key(struct flowi4 *fl4, const struct sock *sk,
 		build_sk_flow_key(fl4, sk);
 }
 
-static DEFINE_SEQLOCK(fnhe_seqlock);
+static inline void rt_free(struct rtable *rt)
+{
+	call_rcu(&rt->dst.rcu_head, dst_rcu_free);
+}
+
+static DEFINE_SPINLOCK(fnhe_lock);
 
 static struct fib_nh_exception *fnhe_oldest(struct fnhe_hash_bucket *hash)
 {
 	struct fib_nh_exception *fnhe, *oldest;
+	struct rtable *orig;
 
 	oldest = rcu_dereference(hash->chain);
 	for (fnhe = rcu_dereference(oldest->fnhe_next); fnhe;
@@ -599,6 +605,11 @@ static struct fib_nh_exception *fnhe_oldest(struct fnhe_hash_bucket *hash)
 		if (time_before(fnhe->fnhe_stamp, oldest->fnhe_stamp))
 			oldest = fnhe;
 	}
+	orig = rcu_dereference(oldest->fnhe_rth);
+	if (orig) {
+		RCU_INIT_POINTER(oldest->fnhe_rth, NULL);
+		rt_free(orig);
+	}
 	return oldest;
 }
 
@@ -620,7 +631,7 @@ static void update_or_create_fnhe(struct fib_nh *nh, __be32 daddr, __be32 gw,
 	int depth;
 	u32 hval = fnhe_hashfun(daddr);
 
-	write_seqlock_bh(&fnhe_seqlock);
+	spin_lock_bh(&fnhe_lock);
 
 	hash = nh->nh_exceptions;
 	if (!hash) {
@@ -667,7 +678,7 @@ static void update_or_create_fnhe(struct fib_nh *nh, __be32 daddr, __be32 gw,
 	fnhe->fnhe_stamp = jiffies;
 
 out_unlock:
-	write_sequnlock_bh(&fnhe_seqlock);
+	spin_unlock_bh(&fnhe_lock);
 	return;
 }
 
@@ -1167,41 +1178,40 @@ static struct fib_nh_exception *find_exception(struct fib_nh *nh, __be32 daddr)
 static void rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
 			      __be32 daddr)
 {
-	__be32 fnhe_daddr, gw;
-	unsigned long expires;
-	unsigned int seq;
-	u32 pmtu;
-
-restart:
-	seq = read_seqbegin(&fnhe_seqlock);
-	fnhe_daddr = fnhe->fnhe_daddr;
-	gw = fnhe->fnhe_gw;
-	pmtu = fnhe->fnhe_pmtu;
-	expires = fnhe->fnhe_expires;
-	if (read_seqretry(&fnhe_seqlock, seq))
-		goto restart;
-
-	if (daddr != fnhe_daddr)
-		return;
+	spin_lock_bh(&fnhe_lock);
 
-	if (pmtu) {
-		unsigned long diff = expires - jiffies;
+	if (daddr == fnhe->fnhe_daddr) {
+		struct rtable *orig;
 
-		if (time_before(jiffies, expires)) {
-			rt->rt_pmtu = pmtu;
-			dst_set_expires(&rt->dst, diff);
+		if (fnhe->fnhe_pmtu) {
+			unsigned long expires = fnhe->fnhe_expires;
+			unsigned long diff = expires - jiffies;
+
+			if (time_before(jiffies, expires)) {
+				rt->rt_pmtu = fnhe->fnhe_pmtu;
+				dst_set_expires(&rt->dst, diff);
+			}
+		}
+		if (fnhe->fnhe_gw) {
+			rt->rt_flags |= RTCF_REDIRECTED;
+			rt->rt_gateway = fnhe->fnhe_gw;
 		}
-	}
-	if (gw) {
-		rt->rt_flags |= RTCF_REDIRECTED;
-		rt->rt_gateway = gw;
-	}
-	fnhe->fnhe_stamp = jiffies;
-}
 
-static inline void rt_free(struct rtable *rt)
-{
-	call_rcu(&rt->dst.rcu_head, dst_rcu_free);
+		orig = rcu_dereference(fnhe->fnhe_rth);
+		rcu_assign_pointer(fnhe->fnhe_rth, rt);
+		if (orig)
+			rt_free(orig);
+
+		fnhe->fnhe_stamp = jiffies;
+	} else {
+		/* Routes we intend to cache in nexthop exception have
+		 * the DST_NOCACHE bit clear.  However, if we are
+		 * unsuccessful at storing this route into the cache
+		 * we really need to set it.
+		 */
+		rt->dst.flags |= DST_NOCACHE;
+	}
+	spin_unlock_bh(&fnhe_lock);
 }
 
 static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
@@ -1249,13 +1259,13 @@ static void rt_set_nexthop(struct rtable *rt, __be32 daddr,
 
 		if (nh->nh_gw && nh->nh_scope == RT_SCOPE_LINK)
 			rt->rt_gateway = nh->nh_gw;
-		if (unlikely(fnhe))
-			rt_bind_exception(rt, fnhe, daddr);
 		dst_init_metrics(&rt->dst, fi->fib_metrics, true);
 #ifdef CONFIG_IP_ROUTE_CLASSID
 		rt->dst.tclassid = nh->nh_tclassid;
 #endif
-		if (!(rt->dst.flags & DST_NOCACHE))
+		if (unlikely(fnhe))
+			rt_bind_exception(rt, fnhe, daddr);
+		else if (!(rt->dst.flags & DST_NOCACHE))
 			rt_cache_route(nh, rt);
 	}
 
@@ -1753,22 +1763,23 @@ static struct rtable *__mkroute_output(const struct fib_result *res,
 
 	fnhe = NULL;
 	if (fi) {
-		fnhe = find_exception(&FIB_RES_NH(*res), fl4->daddr);
-		if (!fnhe && FIB_RES_NH(*res).nh_pcpu_rth_output) {
-			struct rtable __rcu **prth;
+		struct rtable __rcu **prth;
 
+		fnhe = find_exception(&FIB_RES_NH(*res), fl4->daddr);
+		if (fnhe)
+			prth = &fnhe->fnhe_rth;
+		else
 			prth = __this_cpu_ptr(FIB_RES_NH(*res).nh_pcpu_rth_output);
-			rth = rcu_dereference(*prth);
-			if (rt_cache_valid(rth)) {
-				dst_hold(&rth->dst);
-				return rth;
-			}
+		rth = rcu_dereference(*prth);
+		if (rt_cache_valid(rth)) {
+			dst_hold(&rth->dst);
+			return rth;
 		}
 	}
 	rth = rt_dst_alloc(dev_out,
 			   IN_DEV_CONF_GET(in_dev, NOPOLICY),
 			   IN_DEV_CONF_GET(in_dev, NOXFRM),
-			   fi && !fnhe);
+			   fi);
 	if (!rth)
 		return ERR_PTR(-ENOBUFS);
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 2/2] ipv4: Properly purge netdev references on uncached routes.
From: David Miller @ 2012-07-31 22:20 UTC (permalink / raw)
  To: netdev


When a device is unregistered, we have to purge all of the
references to it that may exist in the entire system.

If a route is uncached, we currently have no way of accomplishing
this.

So create a global list that is scanned when a network device goes
down.  This mirrors the logic in net/core/dst.c's dst_ifdown().

Signed-off-by: David S. Miller <davem@davemloft.net>
---

This is just a respin of what I posted the other day, against Eric's
fixes of today.  Pushed to 'net'.

diff --git a/include/net/route.h b/include/net/route.h
index 8c52bc6..776a27f 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -57,6 +57,8 @@ struct rtable {
 
 	/* Miscellaneous cached information */
 	u32			rt_pmtu;
+
+	struct list_head	rt_uncached;
 };
 
 static inline bool rt_is_input_route(const struct rtable *rt)
@@ -107,6 +109,7 @@ extern struct ip_rt_acct __percpu *ip_rt_acct;
 struct in_device;
 extern int		ip_rt_init(void);
 extern void		rt_cache_flush(struct net *net, int how);
+extern void		rt_flush_dev(struct net_device *dev);
 extern struct rtable *__ip_route_output_key(struct net *, struct flowi4 *flp);
 extern struct rtable *ip_route_output_flow(struct net *, struct flowi4 *flp,
 					   struct sock *sk);
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 8732cc7..c43ae3f 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -1046,6 +1046,7 @@ static int fib_netdev_event(struct notifier_block *this, unsigned long event, vo
 
 	if (event == NETDEV_UNREGISTER) {
 		fib_disable_ip(dev, 2, -1);
+		rt_flush_dev(dev);
 		return NOTIFY_DONE;
 	}
 
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index b102eeb..c035251 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -147,6 +147,7 @@ static void		 ip_rt_update_pmtu(struct dst_entry *dst, struct sock *sk,
 					   struct sk_buff *skb, u32 mtu);
 static void		 ip_do_redirect(struct dst_entry *dst, struct sock *sk,
 					struct sk_buff *skb);
+static void		ipv4_dst_destroy(struct dst_entry *dst);
 
 static void ipv4_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
 			    int how)
@@ -170,6 +171,7 @@ static struct dst_ops ipv4_dst_ops = {
 	.default_advmss =	ipv4_default_advmss,
 	.mtu =			ipv4_mtu,
 	.cow_metrics =		ipv4_cow_metrics,
+	.destroy =		ipv4_dst_destroy,
 	.ifdown =		ipv4_dst_ifdown,
 	.negative_advice =	ipv4_negative_advice,
 	.link_failure =		ipv4_link_failure,
@@ -1175,9 +1177,11 @@ static struct fib_nh_exception *find_exception(struct fib_nh *nh, __be32 daddr)
 	return NULL;
 }
 
-static void rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
+static bool rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
 			      __be32 daddr)
 {
+	bool ret = false;
+
 	spin_lock_bh(&fnhe_lock);
 
 	if (daddr == fnhe->fnhe_daddr) {
@@ -1203,6 +1207,7 @@ static void rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
 			rt_free(orig);
 
 		fnhe->fnhe_stamp = jiffies;
+		ret = true;
 	} else {
 		/* Routes we intend to cache in nexthop exception have
 		 * the DST_NOCACHE bit clear.  However, if we are
@@ -1212,11 +1217,14 @@ static void rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
 		rt->dst.flags |= DST_NOCACHE;
 	}
 	spin_unlock_bh(&fnhe_lock);
+
+	return ret;
 }
 
-static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
+static bool rt_cache_route(struct fib_nh *nh, struct rtable *rt)
 {
 	struct rtable *orig, *prev, **p;
+	bool ret = true;
 
 	if (rt_is_input_route(rt)) {
 		p = (struct rtable **)&nh->nh_rth_input;
@@ -1239,6 +1247,48 @@ static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
 		 */
 nocache:
 		rt->dst.flags |= DST_NOCACHE;
+		ret = false;
+	}
+
+	return ret;
+}
+
+static DEFINE_SPINLOCK(rt_uncached_lock);
+static LIST_HEAD(rt_uncached_list);
+
+static void rt_add_uncached_list(struct rtable *rt)
+{
+	spin_lock_bh(&rt_uncached_lock);
+	list_add_tail(&rt->rt_uncached, &rt_uncached_list);
+	spin_unlock_bh(&rt_uncached_lock);
+}
+
+static void ipv4_dst_destroy(struct dst_entry *dst)
+{
+	struct rtable *rt = (struct rtable *) dst;
+
+	if (dst->flags & DST_NOCACHE) {
+		spin_lock_bh(&rt_uncached_lock);
+		list_del(&rt->rt_uncached);
+		spin_unlock_bh(&rt_uncached_lock);
+	}
+}
+
+void rt_flush_dev(struct net_device *dev)
+{
+	if (!list_empty(&rt_uncached_list)) {
+		struct net *net = dev_net(dev);
+		struct rtable *rt;
+
+		spin_lock_bh(&rt_uncached_lock);
+		list_for_each_entry(rt, &rt_uncached_list, rt_uncached) {
+			if (rt->dst.dev != dev)
+				continue;
+			rt->dst.dev = net->loopback_dev;
+			dev_hold(rt->dst.dev);
+			dev_put(dev);
+		}
+		spin_unlock_bh(&rt_uncached_lock);
 	}
 }
 
@@ -1254,6 +1304,8 @@ static void rt_set_nexthop(struct rtable *rt, __be32 daddr,
 			   struct fib_nh_exception *fnhe,
 			   struct fib_info *fi, u16 type, u32 itag)
 {
+	bool cached = false;
+
 	if (fi) {
 		struct fib_nh *nh = &FIB_RES_NH(*res);
 
@@ -1264,10 +1316,12 @@ static void rt_set_nexthop(struct rtable *rt, __be32 daddr,
 		rt->dst.tclassid = nh->nh_tclassid;
 #endif
 		if (unlikely(fnhe))
-			rt_bind_exception(rt, fnhe, daddr);
+			cached = rt_bind_exception(rt, fnhe, daddr);
 		else if (!(rt->dst.flags & DST_NOCACHE))
-			rt_cache_route(nh, rt);
+			cached = rt_cache_route(nh, rt);
 	}
+	if (unlikely(!cached))
+		rt_add_uncached_list(rt);
 
 #ifdef CONFIG_IP_ROUTE_CLASSID
 #ifdef CONFIG_IP_MULTIPLE_TABLES
@@ -1334,6 +1388,7 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 	rth->rt_iif	= 0;
 	rth->rt_pmtu	= 0;
 	rth->rt_gateway	= 0;
+	INIT_LIST_HEAD(&rth->rt_uncached);
 	if (our) {
 		rth->dst.input= ip_local_deliver;
 		rth->rt_flags |= RTCF_LOCAL;
@@ -1459,6 +1514,7 @@ static int __mkroute_input(struct sk_buff *skb,
 	rth->rt_iif 	= 0;
 	rth->rt_pmtu	= 0;
 	rth->rt_gateway	= 0;
+	INIT_LIST_HEAD(&rth->rt_uncached);
 
 	rth->dst.input = ip_forward;
 	rth->dst.output = ip_output;
@@ -1625,6 +1681,7 @@ local_input:
 	rth->rt_iif	= 0;
 	rth->rt_pmtu	= 0;
 	rth->rt_gateway	= 0;
+	INIT_LIST_HEAD(&rth->rt_uncached);
 	if (res.type == RTN_UNREACHABLE) {
 		rth->dst.input= ip_error;
 		rth->dst.error= -err;
@@ -1792,6 +1849,7 @@ static struct rtable *__mkroute_output(const struct fib_result *res,
 	rth->rt_iif	= orig_oif ? : 0;
 	rth->rt_pmtu	= 0;
 	rth->rt_gateway = 0;
+	INIT_LIST_HEAD(&rth->rt_uncached);
 
 	RT_CACHE_STAT_INC(out_slow_tot);
 
@@ -2071,6 +2129,8 @@ struct dst_entry *ipv4_blackhole_route(struct net *net, struct dst_entry *dst_or
 		rt->rt_type = ort->rt_type;
 		rt->rt_gateway = ort->rt_gateway;
 
+		INIT_LIST_HEAD(&rt->rt_uncached);
+
 		dst_free(new);
 	}
 
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index c628184..681ea2f 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -92,6 +92,7 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
 	xdst->u.rt.rt_type = rt->rt_type;
 	xdst->u.rt.rt_gateway = rt->rt_gateway;
 	xdst->u.rt.rt_pmtu = rt->rt_pmtu;
+	INIT_LIST_HEAD(&xdst->u.rt.rt_uncached);
 
 	return 0;
 }
-- 
1.7.10.4

^ permalink raw reply related

* Re: [RFC v2 1/2] PCI-Express Non-Transparent Bridge Support
From: Greg KH @ 2012-07-31 22:23 UTC (permalink / raw)
  To: Jon Mason; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <1343607994-32415-2-git-send-email-jon.mason@intel.com>

On Sun, Jul 29, 2012 at 05:26:33PM -0700, Jon Mason wrote:
> + *   You should have received a copy of the GNU General Public License
> + *   along with this program; if not, write to the Free Software
> + *   Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.

You really are going to track the FSF's office movements for the next
40+ years?

> + *   The full GNU General Public License is included in this distribution
> + *   in the file called LICENSE.GPL.

No it isn't, this sentance is totally wrong for any in-kernel code,
please remove it.

Also do the same for all the other files you added please.

greg k-h

^ permalink raw reply

* Re: [RFC v2 1/2] PCI-Express Non-Transparent Bridge Support
From: Greg KH @ 2012-07-31 22:25 UTC (permalink / raw)
  To: Jon Mason; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <1343607994-32415-2-git-send-email-jon.mason@intel.com>

On Sun, Jul 29, 2012 at 05:26:33PM -0700, Jon Mason wrote:
> +struct ntb_transport_qp;
> +
> +struct ntb_client {
> +	char *name;
> +	int (*probe) (struct pci_dev *pdev);
> +	void (*remove) (struct pci_dev *pdev);
> +};

Why isn't this tied into the driver model?  That looks like you really
want to use a 'struct device' here, right?

Then you have drivers that bind to them, and your devices show up in
sysfs, which is probably a requirement you have, right?

greg k-h

^ permalink raw reply

* Re: [RFC v2 2/2] net: Add support for NTB virtual ethernet device
From: Greg KH @ 2012-07-31 22:28 UTC (permalink / raw)
  To: Jon Mason; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <1343607994-32415-3-git-send-email-jon.mason@intel.com>

On Sun, Jul 29, 2012 at 05:26:34PM -0700, Jon Mason wrote:
> A virtual ethernet device that uses the NTB transport API to send/receive data.
> 
> Signed-off-by: Jon Mason <jon.mason@intel.com>
> ---
>  drivers/net/Kconfig      |    4 +
>  drivers/net/Makefile     |    1 +
>  drivers/net/ntb_netdev.c |  417 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 422 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/net/ntb_netdev.c
> 
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 0c2bd80..9bf8a71 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -178,6 +178,10 @@ config NETPOLL_TRAP
>  config NET_POLL_CONTROLLER
>  	def_bool NETPOLL
>  
> +config NTB_NETDEV
> +	tristate "Virtual Ethernet over NTB"
> +	depends on NTB

How will this module be auto-loaded by a distro?  You need some way to
detect this device on the "bus" right?  What is that method?  And why
isn't it hooked up to the "traditional" method the kernel uses to
determine this?

Don't expect distros to just always load this module, that's not going
to work, sorry.

greg k-h

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox