Netdev List
 help / color / mirror / Atom feed
* Re: possible routing table cache bug for mtu/advmss attributes in Linux 3.0.4
From: David Madore @ 2011-09-09 21:28 UTC (permalink / raw)
  To: Yan, Zheng ; +Cc: Linux Netdev Mailing-List
In-Reply-To: <CAAM7YA=t=s5EEVFte84oqWaKfqyoNhX_NsE5xWUKJ53u-+Es=g@mail.gmail.com>

On Thu, Sep 08, 2011 at 10:42:20PM +0800, Yan, Zheng  wrote:
> On Fri, Sep 2, 2011 at 5:46 AM, David Madore <david+ml@madore.org> wrote:
> > In a nutshell, the problem is this: my (manually entered) routing
> > tables specify an explicit MTU/MSS for certain IPv6 routes, and after
> > upgrading from 2.6.38.7 to 3.0.4, I found that this MTU/MSS setting is
> > sometimes ignored.  More precisely, it seems that connecting to a host
> > through the route in question causes a routing cache entry to be
> > created, and it is when this cache entry expires that the route
> > forgets the MTU/MSS setting.  (It may also be relevant that in my case
> > the route belongs to a routing table other than "main".)
> 
> Would you please try patch http://marc.info/?l=linux-netdev&m=131529445904858

Indeed, this appears to solve the problem I encountered.  Thanks!

-- 
     David A. Madore
   ( http://www.madore.org/~david/ )

^ permalink raw reply

* Re: Memory leak in ip_dst_cache
From: Eric Dumazet @ 2011-09-09 21:47 UTC (permalink / raw)
  To: Kumar S; +Cc: netdev@vger.kernel.org
In-Reply-To: <1315601283.91185.YahooMailNeo@web113908.mail.gq1.yahoo.com>

Le vendredi 09 septembre 2011 à 13:48 -0700, Kumar S a écrit :
> Hi Eric,
> I did not flush those routes explicitly. They are flushed out
> automatically by calling "rt_run_flush". It inturn calls rt_free(),
> but __cache_free() never gets called after that. Basically, I have 512
> MBytes of memory on board. I want to restrict the cache to around 64k,
> with max_size. But if I do that due to this leak, TCP/IP communication
> stops working when it reaches 64k.
>  

Your dsts use about 3Mb of memory out of 512Mb. Its certainly not the
reason of the failures you have.

Its not a leak.

All "in use" dst are queued in dst_gc, check net/core/dst.c for details.

^ permalink raw reply

* Re: Backporting the Linux kernel, for good - was: Re: semantic patch inference
From: Julia Lawall @ 2011-09-09 21:53 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Jesper Andersen, Hauke Mehrtens, linux-wireless, linux-kernel,
	netdev
In-Reply-To: <CAB=NE6VVaAgyaEX0iTuuj=r1-Vsi5TgAyCj4+3J=QerrJTWs_Q@mail.gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1838 bytes --]

On Fri, 9 Sep 2011, Luis R. Rodriguez wrote:

> On Fri, Sep 9, 2011 at 1:48 PM, Julia Lawall <julia@diku.dk> wrote:
> > Thanks for your email.  It made me realize that there was one thing that I
> > didn't understand at all.  If the patches are only intended to apply to
> > linux-next, that makes the problem quite a bit simpler.
> 
> Awesome, and yes the patches/ are only targeted at applying onto
> linux-next.git. When Linus decides to merge and out 3.x-rc1 I simply
> then set $GIT_TREE to $HOME/linux-2.6-allstable/ and run the script to
> suck code from there and apply patches from there.Turns out that
> because the effort was done on linux-next and because linux-next will
> look very much like what Linus ends up merging the patches/ will still
> apply. So what I do then is simply create a branch for that target
> stable kernel and keep refreshing the patches for that stable kernel
> on that branch -- while the master branch keeps chugging along with
> linux-next.
> 
> > I guess that the patch that spdiff will receive will already contain the
> > appropriate #ifs, so we don't have to be concerned about them.
> 
> That is correct.
> 
> > We just add them in as is.
> 
> I do not follow, add what?

Sorry.  The + code. The #ifdefs ad the compatibility code.  We don't have 
to interpret it, so we don't care whether it is only related to kernel 
version numbers or something more complex.

julia

> > There was also the question about one or multiple types of changes.  I
> > think this is not a problem, but Jesper should confirm.  If a patch contains
> > two changes and one can be generalized and the other one cannot for some
> > reason, does spdiff give up on the whole thing, or does it do what it can?
> >
> > Overall, the whole thing seems to be doable :)
> 
> Wow. I'm thrilled, so say the least.
> 
>   Luis
> 

^ permalink raw reply

* Re: Memory leak in ip_dst_cache
From: Eric Dumazet @ 2011-09-09 22:08 UTC (permalink / raw)
  To: Kumar S; +Cc: netdev
In-Reply-To: <1315605497.25052.YahooMailNeo@web113916.mail.gq1.yahoo.com>

Le vendredi 09 septembre 2011 à 14:58 -0700, Kumar S a écrit :
> I agree, 3 MB is not a concern. But actually this is exceeding, we are
> seeing more than 1200000 objects in a span of 24 hours, which is close
> to 300 MB.

Hmm, please include netdev in your mails.

Then there is something else.

Maybe a timer bug preventing dst_gc_task() to make its duty.

>  Other tasks require somewhere around 200 MB+. When it reaches this
> number, the board reboots. In lab we could simulate this. That is how
> I'm trying to understand the problem. I'm injecting around 10k routes,
> and waiting for them to ageout.
>  

I dont know, you could try disable IP route cache.

On recent (2.6.29+) kernels, thats very easy

# Disable Ip route cache (negative value in rebuild_count)
echo 3000000000 >/proc/sys/net/ipv4/rt_cache_rebuild_count

On an old kernel, you probably need to patch your kernel, or backport
commit 1080d709fb9d8cd43 (net: implement emergency route cache rebulds
when gc_elasticity is exceeded)

^ permalink raw reply

* Re: Backporting the Linux kernel, for good - was: Re: semantic patch inference
From: Luis R. Rodriguez @ 2011-09-09 22:28 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Jesper Andersen, Hauke Mehrtens, linux-wireless, linux-kernel,
	netdev
In-Reply-To: <Pine.LNX.4.64.1109092352230.13368@ask.diku.dk>

On Fri, Sep 9, 2011 at 2:53 PM, Julia Lawall <julia@diku.dk> wrote:
> On Fri, 9 Sep 2011, Luis R. Rodriguez wrote:
>> On Fri, Sep 9, 2011 at 1:48 PM, Julia Lawall <julia@diku.dk> wrote:
>> > I guess that the patch that spdiff will receive will already contain the
>> > appropriate #ifs, so we don't have to be concerned about them.
>>
>> That is correct.
>>
>> > We just add them in as is.
>>
>> I do not follow, add what?
>
> Sorry.  The + code. The #ifdefs ad the compatibility code.  We don't have
> to interpret it, so we don't care whether it is only related to kernel
> version numbers or something more complex.

Ah yes, indeed!

  Luis

^ permalink raw reply

* Re: Memory leak in ip_dst_cache
From: Kumar S @ 2011-09-09 22:53 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1315606125.2606.19.camel@edumazet-laptop>

Yes Eric, this command doesn't work on 2.6.24. Which timer are you suspecting?


----- Original Message -----
From: Eric Dumazet <eric.dumazet@gmail.com>
To: Kumar S <ps2kumar@yahoo.com>
Cc: netdev <netdev@vger.kernel.org>
Sent: Friday, September 9, 2011 3:08 PM
Subject: Re: Memory leak in ip_dst_cache

Le vendredi 09 septembre 2011 à 14:58 -0700, Kumar S a écrit :
> I agree, 3 MB is not a concern. But actually this is exceeding, we are
> seeing more than 1200000 objects in a span of 24 hours, which is close
> to 300 MB.

Hmm, please include netdev in your mails.

Then there is something else.

Maybe a timer bug preventing dst_gc_task() to make its duty.

>  Other tasks require somewhere around 200 MB+. When it reaches this
> number, the board reboots. In lab we could simulate this. That is how
> I'm trying to understand the problem. I'm injecting around 10k routes,
> and waiting for them to ageout.
>  

I dont know, you could try disable IP route cache.

On recent (2.6.29+) kernels, thats very easy

# Disable Ip route cache (negative value in rebuild_count)
echo 3000000000 >/proc/sys/net/ipv4/rt_cache_rebuild_count

On an old kernel, you probably need to patch your kernel, or backport
commit 1080d709fb9d8cd43 (net: implement emergency route cache rebulds
when gc_elasticity is exceeded)

^ permalink raw reply

* Re: [PATCH 2/3] netdev/of/phy: Add MDIO bus multiplexer support.
From: Andy Fleming @ 2011-09-09 23:22 UTC (permalink / raw)
  To: David Daney
  Cc: linux-mips, ralf, devicetree-discuss, grant.likely, linux-kernel,
	netdev, David S. Miller
In-Reply-To: <1314820906-14004-3-git-send-email-david.daney@cavium.com>

On Wed, Aug 31, 2011 at 3:01 PM, David Daney <david.daney@cavium.com> wrote:
> This patch adds a somewhat generic framework for MDIO bus
> multiplexers.  It is modeled on the I2C multiplexer.
>
> The multiplexer is needed if there are multiple PHYs with the same
> address connected to the same MDIO bus adepter, or if there is
> insufficient electrical drive capability for all the connected PHY
> devices.
>
> Conceptually it could look something like this:
>
>                   ------------------
>                   | Control Signal |
>                   --------+---------
>                           |
>  ---------------   --------+------
>  | MDIO MASTER |---| Multiplexer |
>  ---------------   --+-------+----
>                     |       |
>                     C       C
>                     h       h
>                     i       i
>                     l       l
>                     d       d
>                     |       |
>     ---------       A       B   ---------
>     |       |       |       |   |       |
>     | PHY@1 +-------+       +---+ PHY@1 |
>     |       |       |       |   |       |
>     ---------       |       |   ---------
>     ---------       |       |   ---------
>     |       |       |       |   |       |
>     | PHY@2 +-------+       +---+ PHY@2 |
>     |       |                   |       |
>     ---------                   ---------
>
> This framework configures the bus topology from device tree data.  The
> mechanics of switching the multiplexer is left to device specific
> drivers.
>
> The follow-on patch contains a multiplexer driven by GPIO lines.


It's amazing how various companies' board designers have come up with
the same brain-dead PHY topologies. We (Freescale) have some similar
code in our tree, but it's not this generically applicable.

>
> Signed-off-by: David Daney <david.daney@cavium.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: "David S. Miller" <davem@davemloft.net>

Looks good to me.

Acked-by: Andy Fleming <afleming@freescale.com>

^ permalink raw reply

* Re: [PATCH] per-cgroup tcp buffer limitation
From: Greg Thelen @ 2011-09-09 23:38 UTC (permalink / raw)
  To: Glauber Costa
  Cc: linux-kernel, linux-mm, containers, netdev, xemul,
	David S. Miller, Hiroyouki Kamezawa, Eric W. Biederman,
	Suleiman Souhlal
In-Reply-To: <4E699341.9010606@parallels.com>

On Thu, Sep 8, 2011 at 9:17 PM, Glauber Costa <glommer@parallels.com> wrote:
> On 09/08/2011 06:53 PM, Greg Thelen wrote:
>>
>> If a container must fit within 100M, then a single limit solution
>> would set the limit to 100M and never change it.  In a split limit
>> solution a
>> user daemon (e.g. uswapd) would need to monitor the usage and the amount
>> of
>> active memory vs inactive user memory and unreferenced kernel memory to
>> determine where to apply pressure.
>
> Or it can just define some parameters and let the kernel do the
> rest. Like for instance, a maximum proportion allowed, a maximum
> proportion desired, etc.

Agreed.

>> With some more knobs such a uswapd could
>> attempt to keep ahead of demand.  But eventually direct reclaim would
>> be needed to satisfy rapid growth spikes.  Example: If the 100M container
>> starts with limits of 20M kmem and 80M user memory but later its kernel
>> memory needs grow to 70M.  With separate user and kernel memory
>> limits the kernel memory allocation could fail despite there being
>> reclaimable user pages available.
>
> No no, this is a ratio, not a *limit*. A limit is something you
> should not be allowed to go over. A good limit of kernel memory for
> a 100 Mb container could be something like... 100 mb. But there is
> more to that.
>
> Risking being a bit polemic here, I think that when we do
> containers, we have to view the kernel a little bit like a shared resource
> that is not accounted to anybody. It is easy to account things like tcp
> buffer, but who do you account page tables for shared pages to? Or pinned
> dentries for shared filesystems ?

This is a tough challenge.  If you don't account page tables, then
fork bombs can grab lots of kernel memory.  Containing such pigs is
good.  We are working on some patches that charge page tables to the
memcg they are allocated from.  This charge is checked against the
memcg's limit and thus the page table allocation may fail due if over
limit.

I suggest charging dentries to the cgroup that allocated the dentry.
If containers are used for isolation, then it seems that containers
should typically not share files.  That would break isolation.  Of
course there are shared files (/bin/bash, etc), but at least those are
read-only.

> Being the shared table under everybody, the kernel is more or less
> like buffers inside a physical hdd, or cache lines. You know it is
> there, you know it has a size, but provided you have some sane
> protections, you don't really care - because in most cases you can't
> - who is using it.

I agree that it makes sense to not bother charging fixed sized
resources (struct page, etc.) to containers; they are part of the
platform.  But the set of resources that a process can allocate are
ideally charged to a cgroup to limit container memory usage (i.e.
prevent DoS attacks, isolate performance).

>> The job should have a way to
>> transition to memory limits to 70M+ kernel and 30M- of user.
>
> Yes, and I don't see how what I propose prevents that.

I don't think your current proposal prevents that.  I was thinking
about the future of the proposed kmem cgroup.  I want to make sure the
kernel has a way to apply reclaim pressure bidirectionally between
cgroup kernel memory and cgroup user memory.

>> I suppose a GFP_WAIT slab kernel page allocation could wakeup user space
>> to
>> perform user-assisted direct reclaim.  User space would then lower the
>> user
>> limit thereby causing the kernel to direct reclaim user pages, then
>> the user daemon would raise the kernel limit allowing the slab allocation
>> to
>> succeed.  My hunch is that this would be prone to deadlocks (what prevents
>> uswapd from needing more even more kmem?)  I'll defer to more
>> experienced minds to know if user assisted direct memory reclaim has
>> other pitfalls.  It scares me.
>
> Good that it scares you, it should. OTOH, userspace being
> able to set parameters to it, has nothing scary at all. A daemon in
> userspace can detect that you need more kernel space memory , and
> then - according to a policy you abide to - write to a file allowing
> it more, or maybe not - according to that same policy. It is very
> far away from "userspace driven reclaim".

Agreed.  I have no problem with user space policy being used to
updating kernel control files that alter reclaim.

>> If kmem expands to include reclaimable kernel memory (e.g. dentry) then I
>> presume the kernel would have no way to exchange unused user pages for
>> dentry
>> pages even if the user memory in the container is well below its limit.
>>  This is
>> motivation for the above user assisted direct reclaim.
>
> Dentry is not always reclaimable. If it is pinned, it is non
> reclaimable. Speaking of it, Would you take a look at
> https://lkml.org/lkml/2011/8/14/110 ?
>
> I am targetting dentry as well. But since it is hard to assign a
> dentry to a process all the time, going through a different path. I
> however, haven't entirely given up of doing it cgroups based, so any
> ideas are welcome =)

My hope is that dentry consumption can be effectively limited by
limiting the memory needed to allocate the dentries.  IOW: with a
memcg aware slab allocator which, when possible, charges the calling
process' cgroup.

> Well, I am giving this an extra thought... Having separate knobs
> adds flexibility, but - as usual - also complexity. For the goals I
> have in mind, "kernel memory" would work just as fine.
>
> If you look carefully at the other patches in the series besides
> this one, you'll see that it is just a matter of billing from kernel
> memory instead of tcp-memory, and then all the rest is the same.
>
> Do you think that a single kernel-memory knob would be better for
> your needs? I am willing to give it a try.

Regarding the tcp buffers we're discussing, how does an application
consume lots of buffer memory?  IOW, what would a malicious app do to
cause a grows of tcp buffer usage?  Or is this the kind of resource
that is difficult to directly exploit?

Also, how does the pressure get applied.  I see you have a
per-protocol, per-cgroup pressure pressure setting.  Once set, how
does this pressure cause memory to be freed?  It looks like the
pressure is set when allocating memory and later when packets are
freed the associated memory _might_ be freed if pressure was
previously detected.  Did I get this right?

For me the primary concern is that both user and kernel memory are
eventually charged to the same counter with an associated limit.
Per-container memory pressure is based on this composite limit.  So I
don't have a strong opinion as to how many kernel memory counters
there are so long as they also feed into container memory usage
counter.

>> I agree that kernel memory is somewhat different.  In some (I argue most)
>> situations containers want the ability to exchange job kmem and job umem.
>> Either split or combined accounting protects the system and isolates other
>> containers from kmem allocations of a bad job.  To me it seems natural to
>> indicate that job X gets Y MB of memory.  I have more trouble dividing the
>> Y MB of memory into dedicated slices for different types of memory.
>
> I understand. And I don't think anyone doing containers
> should be mandated to define a division. But a limit...

SGTM, so long as it is an optional kernel memory limit (see below).

>> I think
>> memcg aware slab accounting does a good job of limiting a job's
>> memory allocations.
>> Would such slab accounting meet your needs?
>
> Well, the slab alone, no. There are other objects - like tcp buffers
> - that aren't covered by the slab. Others are usually shared among
> many cgroups, and others don't really belong to anybody in
> particular.igh

Sorry, I am dumb wrt. networking.  I thought that these tcp buffers
were allocated using slab with __alloc_skb().  Are they done in
container process context or arbitrary context?

> How do you think then, about turning this into 2 files inside memcg:
>
>  - kernel_memory_hard_limit.
>  - kernel_memory_soft_limit.
>
> tcp memory would be the one defined in /proc, except if it is
> greater than any of the limits. Instead of testing for memory
> allocation against kmem.tcp_allocated_memory, we'd test it against
> memcg.kmem_pinned_memory.

memcg control files currently include:
  memory.limit_in_bytes
  memory.soft_limit_in_bytes
  memory.usage_in_bytes

If you want a kernel memory limit, then we can introduce two new memcg APIs:
  memory.kernel_limit_in_bytes
  memory.kernel_soft_limit_in_bytes
  memory.kernel_usage_in_bytes

Any memory charged to memory.kernel_limit_in_bytes would also be
charged to memory.limit_in_bytes.  IOW, memory.limit_in_bytes would
include both user pages (as it does today) and some kernel pages.

With these new files there are two cgroup memory limits: total and
kernel.  The total limit is a combination of user and some kinds of
kernel memory.

Kernel page reclaim would occur if memory.kernel_usage_in_bytes
exceeds memory.kernel_limit_in_bytes.  There would be no need to
reclaim user pages in this situation.  If kernel usage exceeds
kernel_soft_limit_in_bytes, then the protocol pressure flags would be
set.

Memcg-wide page reclaim would occur if memory.usage_in_bytes exceeds
memory.limit_in_bytes.  This reclaim would consider either container
kernel memory or user pages associated with the container.

This would be a behavioral memcg change which would need to be
carefully considered.  Today the limit_in_bytes is just a user byte
count that does not include kernel memory.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH -next] drivers/net: Makefile, fix netconsole link order
From: Andrew Morton @ 2011-09-10  0:00 UTC (permalink / raw)
  To: Lin Ming; +Cc: David S. Miller, lkml, netdev, Jeff Kirsher
In-Reply-To: <1315298129.16061.24.camel@snb>

On Tue, 06 Sep 2011 16:35:29 +0800
Lin Ming <ming.m.lin@intel.com> wrote:

> Commit 88491d8(drivers/net: Kconfig & Makefile cleanup) causes a
> regression that netconsole does not work if netconsole and network
> device driver are build into kernel, because netconsole is linked before
> network device driver.
> 
> Fixes it by moving netconsole.o after network device driver.
> 
> Signed-off-by: Lin Ming <ming.m.lin@intel.com>
> ---
>  drivers/net/Makefile |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index fa877cd..ec15311 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -14,7 +14,6 @@ obj-$(CONFIG_MACVTAP) += macvtap.o
>  obj-$(CONFIG_MII) += mii.o
>  obj-$(CONFIG_MDIO) += mdio.o
>  obj-$(CONFIG_NET) += Space.o loopback.o
> -obj-$(CONFIG_NETCONSOLE) += netconsole.o
>  obj-$(CONFIG_PHYLIB) += phy/
>  obj-$(CONFIG_RIONET) += rionet.o
>  obj-$(CONFIG_TUN) += tun.o
> @@ -66,3 +65,9 @@ obj-$(CONFIG_USB_USBNET)        += usb/
>  obj-$(CONFIG_USB_ZD1201)        += usb/
>  obj-$(CONFIG_USB_IPHETH)        += usb/
>  obj-$(CONFIG_USB_CDC_PHONET)   += usb/
> +
> +#
> +# If netconsole and network device driver are build-in,
> +# netconsole must be linked after network device driver
> +#
> +obj-$(CONFIG_NETCONSOLE) += netconsole.o

It would be preferable to fix this with initcall ordering.  Perhaps by
switching init_netconsole() to subsys_initcall.

^ permalink raw reply

* Re: [PATCH -next] drivers/net: Makefile, fix netconsole link order
From: Lin Ming @ 2011-09-10  2:43 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David S. Miller, lkml, netdev@vger.kernel.org, Kirsher, Jeffrey T
In-Reply-To: <20110909170027.c4bf6151.akpm@linux-foundation.org>

On Sat, 2011-09-10 at 08:00 +0800, Andrew Morton wrote:
> On Tue, 06 Sep 2011 16:35:29 +0800
> Lin Ming <ming.m.lin@intel.com> wrote:
> 
> > Commit 88491d8(drivers/net: Kconfig & Makefile cleanup) causes a
> > regression that netconsole does not work if netconsole and network
> > device driver are build into kernel, because netconsole is linked before
> > network device driver.
> > 
> > Fixes it by moving netconsole.o after network device driver.
> > 
> > Signed-off-by: Lin Ming <ming.m.lin@intel.com>
> > ---
> >  drivers/net/Makefile |    7 ++++++-
> >  1 files changed, 6 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> > index fa877cd..ec15311 100644
> > --- a/drivers/net/Makefile
> > +++ b/drivers/net/Makefile
> > @@ -14,7 +14,6 @@ obj-$(CONFIG_MACVTAP) += macvtap.o
> >  obj-$(CONFIG_MII) += mii.o
> >  obj-$(CONFIG_MDIO) += mdio.o
> >  obj-$(CONFIG_NET) += Space.o loopback.o
> > -obj-$(CONFIG_NETCONSOLE) += netconsole.o
> >  obj-$(CONFIG_PHYLIB) += phy/
> >  obj-$(CONFIG_RIONET) += rionet.o
> >  obj-$(CONFIG_TUN) += tun.o
> > @@ -66,3 +65,9 @@ obj-$(CONFIG_USB_USBNET)        += usb/
> >  obj-$(CONFIG_USB_ZD1201)        += usb/
> >  obj-$(CONFIG_USB_IPHETH)        += usb/
> >  obj-$(CONFIG_USB_CDC_PHONET)   += usb/
> > +
> > +#
> > +# If netconsole and network device driver are build-in,
> > +# netconsole must be linked after network device driver
> > +#
> > +obj-$(CONFIG_NETCONSOLE) += netconsole.o
> 
> It would be preferable to fix this with initcall ordering.  Perhaps by
> switching init_netconsole() to subsys_initcall.

subsys_initcall is defined as __define_initcall("4",fn,4).
In !MODULE case, device driver module_int() is defined as
__define_initcall("6",fn,6)

If we use subsys_initcall for init_netconsole, it will still be called
before network device driver is initialized.

How about late_initcall?

(Not tested yet)

>From 11f4f035e96ff430192d0a75552dd715acb5f3b8 Mon Sep 17 00:00:00 2001
From: Lin Ming <ming.m.lin@intel.com>
Date: Sat, 10 Sep 2011 10:36:10 +0800
Subject: [PATCH] drivers/net: Makefile, fix netconsole link order

Commit 88491d8(drivers/net: Kconfig & Makefile cleanup) causes a
regression that netconsole does not work if netconsole and network
device driver are build into kernel, because netconsole is linked before
network device driver.

Fixes it by switching init_netconsole() to late_initcall if it's
build-in.

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
 drivers/net/netconsole.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index dfc8272..914be29 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -799,5 +799,9 @@ static void __exit cleanup_netconsole(void)
 	}
 }
 
+#ifdef MODULE
 module_init(init_netconsole);
 module_exit(cleanup_netconsole);
+#else
+late_initcall(init_netconsole);
+#endif /* !MODULE */
-- 
1.7.2.3

^ permalink raw reply related

* Re: Backporting the Linux kernel, for good - was: Re: semantic patch inference
From: Julia Lawall @ 2011-09-10  3:12 UTC (permalink / raw)
  To: Jesper Andersen
  Cc: Luis R. Rodriguez, Hauke Mehrtens, linux-wireless,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CA+RMdsmEJRv8i81o_GD6o7Bn0MpQJMidghTnt3f-QEeU6V6eNQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Sat, 10 Sep 2011, Jesper Andersen wrote:

> I'm in Singapore now and will head for Denmark tomorrow so I can't promise
> very instant reponses.
> 
> I haven't read the full thread just yet, but I just wanted to clarify one
> thing:
> 
> On Saturday, September 10, 2011, Julia Lawall wrote:
> >
> > There was also the question about one or multiple types of changes.  I
> > think this is not a problem, but Jesper should confirm.  If a patch
> > contains
> > two changes and one can be generalized and the other one cannot for some
> > reason, does spdiff give up on the whole thing, or does it do what it can?
> >
> >
> It's a little difficult to answer this precisely, so let me instead give an
> example and then you can say in what way my example falls short of what you
> thought of.
> 
> Suppose we have two functions that changed:
> 
> f1=
> void foo() {
>   bar(x->dev);
>   foz(117);
> }
> 
> f2=
> void goo() {
>   if(b) bar(y->dev);
> }
> 
> 
> And the new versions are:
> 
> f1'=
> void foo() {
>   bar(x);
>   foz(117, 42);
> }
> 
> f2'=
> void goo() {
>   if(b) bar(y);
> }
> 
> In this example there are basically three changes:
> 
> x->dev becomes x
> y->dev becomes y
> foz(117) becomes foz(117', 42)
> 
> In this case spdiff will find: (I just ran it to be sure)
> @@
> expression X0;
> @@
> - bar(X0->dev);
> + bar(X0);
> 
> So the one ungeneralizable change was simply not included. Does this cover
> your question Julia?

That seems good.  But perhaps the user would like to know about the 
elements that were not generalizable as well.

julia
--
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 -next] drivers/net: Makefile, fix netconsole link order
From: Andrew Morton @ 2011-09-10  4:10 UTC (permalink / raw)
  To: Lin Ming
  Cc: David S. Miller, lkml, netdev@vger.kernel.org, Kirsher, Jeffrey T
In-Reply-To: <1315622612.2425.16.camel@hp6530s>

On Sat, 10 Sep 2011 10:43:32 +0800 Lin Ming <ming.m.lin@intel.com> wrote:

> On Sat, 2011-09-10 at 08:00 +0800, Andrew Morton wrote:
>
> > > +obj-$(CONFIG_NETCONSOLE) += netconsole.o
> > 
> > It would be preferable to fix this with initcall ordering.  Perhaps by
> > switching init_netconsole() to subsys_initcall.
> 
> subsys_initcall is defined as __define_initcall("4",fn,4).
> In !MODULE case, device driver module_int() is defined as
> __define_initcall("6",fn,6)

oop, yes, I am chronologically challenged.

> If we use subsys_initcall for init_netconsole, it will still be called
> before network device driver is initialized.
> 
> How about late_initcall?
> 
> (Not tested yet)
> 
> >From 11f4f035e96ff430192d0a75552dd715acb5f3b8 Mon Sep 17 00:00:00 2001
> From: Lin Ming <ming.m.lin@intel.com>
> Date: Sat, 10 Sep 2011 10:36:10 +0800
> Subject: [PATCH] drivers/net: Makefile, fix netconsole link order
> 
> Commit 88491d8(drivers/net: Kconfig & Makefile cleanup) causes a
> regression that netconsole does not work if netconsole and network
> device driver are build into kernel, because netconsole is linked before
> network device driver.
> 
> Fixes it by switching init_netconsole() to late_initcall if it's
> build-in.
> 
> Signed-off-by: Lin Ming <ming.m.lin@intel.com>
> ---
>  drivers/net/netconsole.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index dfc8272..914be29 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -799,5 +799,9 @@ static void __exit cleanup_netconsole(void)
>  	}
>  }
>  
> +#ifdef MODULE
>  module_init(init_netconsole);
>  module_exit(cleanup_netconsole);
> +#else
> +late_initcall(init_netconsole);
> +#endif /* !MODULE */

That should work, unless any net driver is weirdly also using
late_initcall.  But if there are such drivers, they would have failed
with the old Makefile ordering.

^ permalink raw reply

* Re: [PATCH -next] drivers/net: Makefile, fix netconsole link order
From: Lin Ming @ 2011-09-10  4:26 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David S. Miller, lkml, netdev@vger.kernel.org, Kirsher, Jeffrey T
In-Reply-To: <20110909211029.69dfd68d.akpm@linux-foundation.org>

On Sat, 2011-09-10 at 12:10 +0800, Andrew Morton wrote:
> On Sat, 10 Sep 2011 10:43:32 +0800 Lin Ming <ming.m.lin@intel.com> wrote:
> 
> > On Sat, 2011-09-10 at 08:00 +0800, Andrew Morton wrote:
> >
> > > > +obj-$(CONFIG_NETCONSOLE) += netconsole.o
> > > 
> > > It would be preferable to fix this with initcall ordering.  Perhaps by
> > > switching init_netconsole() to subsys_initcall.
> > 
> > subsys_initcall is defined as __define_initcall("4",fn,4).
> > In !MODULE case, device driver module_int() is defined as
> > __define_initcall("6",fn,6)
> 
> oop, yes, I am chronologically challenged.
> 
> > If we use subsys_initcall for init_netconsole, it will still be called
> > before network device driver is initialized.
> > 
> > How about late_initcall?
> > 
> > (Not tested yet)
> > 
> > >From 11f4f035e96ff430192d0a75552dd715acb5f3b8 Mon Sep 17 00:00:00 2001
> > From: Lin Ming <ming.m.lin@intel.com>
> > Date: Sat, 10 Sep 2011 10:36:10 +0800
> > Subject: [PATCH] drivers/net: Makefile, fix netconsole link order
> > 
> > Commit 88491d8(drivers/net: Kconfig & Makefile cleanup) causes a
> > regression that netconsole does not work if netconsole and network
> > device driver are build into kernel, because netconsole is linked before
> > network device driver.
> > 
> > Fixes it by switching init_netconsole() to late_initcall if it's
> > build-in.
> > 
> > Signed-off-by: Lin Ming <ming.m.lin@intel.com>
> > ---
> >  drivers/net/netconsole.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> > index dfc8272..914be29 100644
> > --- a/drivers/net/netconsole.c
> > +++ b/drivers/net/netconsole.c
> > @@ -799,5 +799,9 @@ static void __exit cleanup_netconsole(void)
> >  	}
> >  }
> >  
> > +#ifdef MODULE
> >  module_init(init_netconsole);
> >  module_exit(cleanup_netconsole);
> > +#else
> > +late_initcall(init_netconsole);
> > +#endif /* !MODULE */
> 
> That should work, unless any net driver is weirdly also using
> late_initcall.  But if there are such drivers, they would have failed
> with the old Makefile ordering.

Yes.

BTW, may I get your ACK after testing this patch?

Thanks.

^ permalink raw reply

* skge: transmit queue 0 timed out
From: Bart Van Assche @ 2011-09-10  9:27 UTC (permalink / raw)
  To: netdev
In-Reply-To: <CAO+b5-p=9NXJrT1h6KQG9BpP=mDp8XjMZsCxsk=WkuBZ_KhtKA@mail.gmail.com>

Hello,

Anyone who can tell me whether I should worry about the kernel warning
shown below ? I've observed that warning for the first time with
3.1-rc5 and hadn't seen it yet with 3.1-rc4 or any older kernel.

------------[ cut here ]------------
WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0x24f/0x260()
Hardware name: P5Q DELUXE
NETDEV WATCHDOG: eth0 (skge): transmit queue 0 timed out
Modules linked in: snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device
af_packet rdma_ucm rdma_cm iw_cm ib_addr ib_ipoib ib_cm ib_sa
ib_uverbs ib_umad mlx4_ib ib_mad ib_core microcode
cpufreq_conservative cpufreq_userspaceq mperf dm_mod
snd_hda_codec_hdmi snd_hda_codec_analog snd_hda_intel snd_hda_codec
snd_hwdep snd_pcm snd_timer snd sr_mod mlx4_core sg cdrom soundcore
i2c_i801 snd_page_alloc i2c_core pcspkr button uhci_hcd ehci_hcd sxt3
mbcache jbd fan ata_generic ata_piix pata_marvell ahci libahci libata
scsi_mod thermal processor thermal_sys hwmon
Pid: 6260, comm: wish Not tainted 3.1.0-rc4-scst-debug+ #2
Call Trace:
 <IRQ>  [<ffffffff8104564f>] warn_slowpath_common+0x7f/0xc0
 [<ffffffff81045746>] warn_slowpath_fmt+0x46/0x50
 [<ffffffff813268df>] dev_watchdog+0x24f/0x260
 [<ffffffff8105602a>] run_timer_softirq+0x19a/0x3f0
 [<ffffffff81055f97>] ? run_timer_softirq+0x107/0x3f0
 [<ffffffff81326690>] ? pfifo_fast_init+0xa0/0xa0
 [<ffffffff8104cecd>] __do_softirq+0xcd/0x250
 [<ffffffff813e5cec>] call_softirq+0x1c/0x30
 [<ffffffff810043bd>] do_softirq+0x8d/0xc0
 [<ffffffff8104d36e>] irq_exit+0xae/0xe0
 [<ffffffff813e637e>] smp_apic_timer_interrupt+0x6e/0x99
 [<ffffffff813e4570>] apic_timer_interrupt+0x70/0x80
 <EOI>  [<ffffffff813dbe0a>] ? retint_swapgs+0xe/0x13
---[ end trace fbbed52d2a66b808 ]---

Bart.

^ permalink raw reply

* Re: Memory leak in ip_dst_cache
From: Neil Horman @ 2011-09-10 13:04 UTC (permalink / raw)
  To: Kumar S; +Cc: Eric Dumazet, netdev
In-Reply-To: <1315608825.98066.YahooMailNeo@web113914.mail.gq1.yahoo.com>

On Fri, Sep 09, 2011 at 03:53:45PM -0700, Kumar S wrote:
> Yes Eric, this command doesn't work on 2.6.24. Which timer are you suspecting?
> 
He means that the timer that starts the the workqueue which calls dst_gc_task is
either delayed indefinately, or otherwise not run, which means that the route
cache will never be scanned for old entries.  The implication being that, while
the dst entries aren't leaked per-se, but never expired, so they just sit
around.  You can check this by instrumenting gc_dst_task with a printk or two
and watch to see if it ever pops up.

If you only have 512Mb of ram and want to really restrict you're cache size, you
should definately consider backporting the route cache rebuild patch to disable
the route cache usage entirely.  It shouldn't be too hard to do.
Neil

> 

^ permalink raw reply

* [PATCH] mac_esp: remove redundant mutual exclusion
From: Finn Thain @ 2011-09-10 15:01 UTC (permalink / raw)
  To: David Miller; +Cc: linux-m68k, netdev

Mutual exclusion is redundant here because all the paths in the call graph 
leading to esp_driver_ops.send_dma_cmd() happen under spin_lock_irqsave/ 
spin_lock_irqrestore. Remove it.

Tested on a Mac Quadra 660av and a Mac LC 630.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---

Part of this patch relates to Geert's m68k generic hard irq conversion 
work. Under genirq, the disable_irq() in mac_esp leads to deadlock. It 
wasn't a problem before because the m68k disable_irq() implementation is 
equivalent to genirq's disable_irq_nosync().

Index: linux-m68k/drivers/scsi/mac_esp.c
===================================================================
--- linux-m68k.orig/drivers/scsi/mac_esp.c	2011-08-28 00:33:13.000000000 +1000
+++ linux-m68k/drivers/scsi/mac_esp.c	2011-08-28 00:35:48.000000000 +1000
@@ -230,9 +230,6 @@ static void mac_esp_send_pdma_cmd(struct
 				  u32 dma_count, int write, u8 cmd)
 {
 	struct mac_esp_priv *mep = MAC_ESP_GET_PRIV(esp);
-	unsigned long flags;
-
-	local_irq_save(flags);
 
 	mep->error = 0;
 
@@ -270,8 +267,6 @@ static void mac_esp_send_pdma_cmd(struct
 			esp_count = n;
 		}
 	} while (esp_count);
-
-	local_irq_restore(flags);
 }
 
 /*
@@ -353,8 +348,6 @@ static void mac_esp_send_pio_cmd(struct
 	struct mac_esp_priv *mep = MAC_ESP_GET_PRIV(esp);
 	u8 *fifo = esp->regs + ESP_FDATA * 16;
 
-	disable_irq(esp->host->irq);
-
 	cmd &= ~ESP_CMD_DMA;
 	mep->error = 0;
 
@@ -431,8 +424,6 @@ static void mac_esp_send_pio_cmd(struct
 			scsi_esp_cmd(esp, ESP_CMD_TI);
 		}
 	}
-
-	enable_irq(esp->host->irq);
 }
 
 static int mac_esp_irq_pending(struct esp *esp)

^ permalink raw reply

* [PATCH] macmace: disable only the dma interrupt
From: Finn Thain @ 2011-09-10 15:02 UTC (permalink / raw)
  To: David Miller; +Cc: linux-m68k, netdev

Don't disable all interrupts, just disable the relevant one.

Also move a couple of printk calls outside of local_irq_save/restore.

Tested on a Quadra 660av.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Index: linux-m68k/drivers/net/macmace.c
===================================================================
--- linux-m68k.orig/drivers/net/macmace.c	2011-08-28 00:33:33.000000000 +1000
+++ linux-m68k/drivers/net/macmace.c	2011-08-28 00:36:10.000000000 +1000
@@ -458,8 +458,9 @@ static int mace_xmit_start(struct sk_buf
 	local_irq_save(flags);
 	netif_stop_queue(dev);
 	if (!mp->tx_count) {
-		printk(KERN_ERR "macmace: tx queue running but no free buffers.\n");
 		local_irq_restore(flags);
+		printk(KERN_ERR
+		       "macmace: tx queue running but no free buffers.\n");
 		return NETDEV_TX_BUSY;
 	}
 	mp->tx_count--;
@@ -563,10 +564,8 @@ static irqreturn_t mace_interrupt(int ir
 	struct mace_data *mp = netdev_priv(dev);
 	volatile struct mace *mb = mp->mace;
 	int intr, fs;
-	unsigned long flags;
 
-	/* don't want the dma interrupt handler to fire */
-	local_irq_save(flags);
+	disable_irq(mp->dma_intr);
 
 	intr = mb->ir; /* read interrupt register */
 	mace_handle_misc_intrs(dev, intr);
@@ -604,7 +603,7 @@ static irqreturn_t mace_interrupt(int ir
 	if (mp->tx_count)
 		netif_wake_queue(dev);
 
-	local_irq_restore(flags);
+	enable_irq(mp->dma_intr);
 
 	return IRQ_HANDLED;
 }
@@ -615,11 +614,12 @@ static void mace_tx_timeout(struct net_d
 	volatile struct mace *mb = mp->mace;
 	unsigned long flags;
 
+	printk(KERN_ERR "macmace: transmit timeout - resetting\n");
+
 	local_irq_save(flags);
 
 	/* turn off both tx and rx and reset the chip */
 	mb->maccc = 0;
-	printk(KERN_ERR "macmace: transmit timeout - resetting\n");
 	mace_txdma_reset(dev);
 	mace_reset(dev);
 

^ permalink raw reply

* [PATCH] fib:fix BUG_ON in fib_nl_newrule when add new fib rule
From: Wanlong Gao @ 2011-09-10 17:19 UTC (permalink / raw)
  To: linux-kernel, netdev; +Cc: davem, eric.dumazet, omarapazanadi, Gao feng

From: Gao feng <gaofeng@cn.fujitsu.com>

add new fib rule can cause BUG_ON
the reproduce shell is

#ip rule add pref 38
#ip rule add pref 38
#ip rule add to 192.168.3.0/24 goto 38
#ip rule del pref 38
#ip rule add to 192.168.3.0/24 goto 38
#ip rule add pref 38

then the BUG_ON will happen
add a var unresolved in struct fib_rule
and use it to identify whether this rule is unresolved

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 include/net/fib_rules.h |    1 +
 net/core/fib_rules.c    |    5 ++++-
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
index 075f1e3..e4bae01 100644
--- a/include/net/fib_rules.h
+++ b/include/net/fib_rules.h
@@ -19,6 +19,7 @@ struct fib_rule {
 	u32			flags;
 	u32			table;
 	u8			action;
+	u8			unresolved;
 	u32			target;
 	struct fib_rule __rcu	*ctarget;
 	char			iifname[IFNAMSIZ];
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index e7ab0c0..aa20560 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -384,9 +384,11 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
 		 */
 		list_for_each_entry(r, &ops->rules_list, list) {
 			if (r->action == FR_ACT_GOTO &&
-			    r->target == rule->pref) {
+			    r->target == rule->pref &&
+			    r->unresolved) {
 				BUG_ON(rtnl_dereference(r->ctarget) != NULL);
 				rcu_assign_pointer(r->ctarget, rule);
+				r->unresolved = 0;
 				if (--ops->unresolved_rules == 0)
 					break;
 			}
@@ -488,6 +490,7 @@ static int fib_nl_delrule(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
 			list_for_each_entry(tmp, &ops->rules_list, list) {
 				if (rtnl_dereference(tmp->ctarget) == rule) {
 					rcu_assign_pointer(tmp->ctarget, NULL);
+					tmp->unresolved = 1;
 					ops->unresolved_rules++;
 				}
 			}
-- 
1.7.6

^ permalink raw reply related

* Re: [PATCH] mac_esp: remove redundant mutual exclusion
From: David Miller @ 2011-09-10 18:28 UTC (permalink / raw)
  To: fthain; +Cc: linux-m68k, netdev
In-Reply-To: <alpine.LNX.2.00.1109110038500.5327@nippy.intranet>

From: Finn Thain <fthain@telegraphics.com.au>
Date: Sun, 11 Sep 2011 01:01:34 +1000 (EST)

> Mutual exclusion is redundant here because all the paths in the call graph 
> leading to esp_driver_ops.send_dma_cmd() happen under spin_lock_irqsave/ 
> spin_lock_irqrestore. Remove it.
> 
> Tested on a Mac Quadra 660av and a Mac LC 630.
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

This patch should be CC:'d to linux-scsi not netdev

I understand how the networking maintainer is very responsive
and the SCSI maintainer generally takes forever, so your
Freudian slip will go unjudged :-)

^ permalink raw reply

* Re: [PATCH] macmace: disable only the dma interrupt
From: David Miller @ 2011-09-10 18:30 UTC (permalink / raw)
  To: fthain; +Cc: linux-m68k, netdev
In-Reply-To: <alpine.LNX.2.00.1109110042210.5327@nippy.intranet>

From: Finn Thain <fthain@telegraphics.com.au>
Date: Sun, 11 Sep 2011 01:02:16 +1000 (EST)

> Don't disable all interrupts, just disable the relevant one.
> 
> Also move a couple of printk calls outside of local_irq_save/restore.
> 
> Tested on a Quadra 660av.
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Using disable_irq() is very expensive, and when done from an interrupt
handler can deadlock especially on SMP (which I understand might not
be relevant here).

I really can't see why you'd do things this way, especially since
interrupt handlers under Linux now unconditionally always run with cpu
interrupts disabled.

^ permalink raw reply

* Re: [PATCH] fib:fix BUG_ON in fib_nl_newrule when add new fib rule
From: Eric Dumazet @ 2011-09-11  7:12 UTC (permalink / raw)
  To: Wanlong Gao; +Cc: linux-kernel, netdev, davem, omarapazanadi, Gao feng
In-Reply-To: <1315675150-3012-1-git-send-email-wanlong.gao@gmail.com>

Le dimanche 11 septembre 2011 à 01:19 +0800, Wanlong Gao a écrit :
> From: Gao feng <gaofeng@cn.fujitsu.com>
> 
> add new fib rule can cause BUG_ON
> the reproduce shell is
> 
> #ip rule add pref 38
> #ip rule add pref 38
> #ip rule add to 192.168.3.0/24 goto 38
> #ip rule del pref 38
> #ip rule add to 192.168.3.0/24 goto 38
> #ip rule add pref 38
> 
> then the BUG_ON will happen
> add a var unresolved in struct fib_rule
> and use it to identify whether this rule is unresolved
> 
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> ---
>  include/net/fib_rules.h |    1 +
>  net/core/fib_rules.c    |    5 ++++-
>  2 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
> index 075f1e3..e4bae01 100644
> --- a/include/net/fib_rules.h
> +++ b/include/net/fib_rules.h
> @@ -19,6 +19,7 @@ struct fib_rule {
>  	u32			flags;
>  	u32			table;
>  	u8			action;
> +	u8			unresolved;
>  	u32			target;
>  	struct fib_rule __rcu	*ctarget;
>  	char			iifname[IFNAMSIZ];
> diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
> index e7ab0c0..aa20560 100644
> --- a/net/core/fib_rules.c
> +++ b/net/core/fib_rules.c
> @@ -384,9 +384,11 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
>  		 */
>  		list_for_each_entry(r, &ops->rules_list, list) {
>  			if (r->action == FR_ACT_GOTO &&
> -			    r->target == rule->pref) {
> +			    r->target == rule->pref &&
> +			    r->unresolved) {
>  				BUG_ON(rtnl_dereference(r->ctarget) != NULL);
>  				rcu_assign_pointer(r->ctarget, rule);
> +				r->unresolved = 0;
>  				if (--ops->unresolved_rules == 0)
>  					break;
>  			}
> @@ -488,6 +490,7 @@ static int fib_nl_delrule(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
>  			list_for_each_entry(tmp, &ops->rules_list, list) {
>  				if (rtnl_dereference(tmp->ctarget) == rule) {
>  					rcu_assign_pointer(tmp->ctarget, NULL);
> +					tmp->unresolved = 1;
>  					ops->unresolved_rules++;
>  				}
>  			}

Hmm, good catch, but you add 'unresolved' field but dont init it
correctly to 1 in all cases.

Following will break :

ip rule add pref 100
ip rule add to 192.168.3.0/24 goto 200
ip rule add pref 200

Normally, we should have 

unresolved = (ctarget == NULL);

What about following patch instead ?

diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index e7ab0c0..3231b46 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -384,8 +384,8 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
 		 */
 		list_for_each_entry(r, &ops->rules_list, list) {
 			if (r->action == FR_ACT_GOTO &&
-			    r->target == rule->pref) {
-				BUG_ON(rtnl_dereference(r->ctarget) != NULL);
+			    r->target == rule->pref &&
+			    rtnl_dereference(r->ctarget) == NULL) {
 				rcu_assign_pointer(r->ctarget, rule);
 				if (--ops->unresolved_rules == 0)
 					break;

^ permalink raw reply related

* kernel.org extended outage - wait or use alternatives?
From: Jan Ceuleers @ 2011-09-11  8:04 UTC (permalink / raw)
  To: netdev

David,

I'm a mere observer of kernel development rather than an active 
participant in it. My observation is that networking development has all 
but come to a standstill while we wait for master.kernel.org to come back.

Linus is now using a github.com account; any reason why the same could 
not apply to net and net-next, so that development can proceed?

Thanks, Jan

^ permalink raw reply

* Re: kernel.org extended outage - wait or use alternatives?
From: Eric Dumazet @ 2011-09-11  8:24 UTC (permalink / raw)
  To: Jan Ceuleers; +Cc: netdev
In-Reply-To: <4E6C6BAA.3020701@computer.org>

Le dimanche 11 septembre 2011 à 10:04 +0200, Jan Ceuleers a écrit :
> David,
> 
> I'm a mere observer of kernel development rather than an active 
> participant in it. My observation is that networking development has all 
> but come to a standstill while we wait for master.kernel.org to come back.
> 
> Linus is now using a github.com account; any reason why the same could 
> not apply to net and net-next, so that development can proceed?
> 

As an active kernel developer, I would vote for being thankful for this
pause. Its always a good thing to relax and enjoy your time ;)

I actually lost my main development trees because of nasty disk
corruptions in 3.1-rc4, and wrong habit of not doing any backups... Oh
well...

More than a repository for net and net-next, we really want a working
netdev mailing list.

Anyway, everything is queued and you dont have to worry.

http://patchwork.ozlabs.org/project/netdev/list/

^ permalink raw reply

* Re: [net-next-2.6 PATCH 0/3 RFC] macvlan: MAC Address filtering support for passthru mode
From: Michael S. Tsirkin @ 2011-09-11  9:38 UTC (permalink / raw)
  To: Roopa Prabhu
  Cc: netdev, dragos.tatulea, arnd, dwang2, benve, kaber, sri, davem,
	eric.dumazet, mchan, kvm
In-Reply-To: <CA8F8DED.33CF5%roprabhu@cisco.com>

On Fri, Sep 09, 2011 at 09:33:33AM -0700, Roopa Prabhu wrote:
> 
> 
> 
> On 9/8/11 10:55 PM, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Thu, Sep 08, 2011 at 07:53:11PM -0700, Roopa Prabhu wrote:
> >>>> Phase 1: Goal: Enable hardware filtering for all macvlan modes
> >>>>     - In macvlan passthru mode the single guest virtio-nic connected will
> >>>>       receive traffic that he requested for
> >>>>     - In macvlan non-passthru mode all guest virtio-nics sharing the
> >>>>       physical nic will see all other guest traffic
> >>>>       but the filtering at guest virtio-nic
> >>> 
> >>> I don't think guests currently filter anything.
> >>> 
> >> I was referring to Qemu-kvm virtio-net in
> >> virtion_net_receive->receive_filter. I think It only passes pkts that the
> >> guest OS is interested. It uses the filter table that I am passing to
> >> macvtap in this patch.
> > 
> > This happens after userspace thread gets woken up and data
> > is copied there. So relying on filtering at that level is
> > going to be very inefficient on a system with
> > multiple active guests. Further, and for that reason, vhost-net
> > doesn't do filtering at all, relying on the backends
> > to pass it correct packets.
> 
> Ok thanks for the info. So in which case, phase 1 is best for PASSTHRU mode
> and for non-PASSTHRU when there is a single guest connected to a VF.
> For non-PASSTHRU multi guest sharing the same VF, Phase 1 is definitely
> better than putting the VF in promiscuous mode.
> But to address the concern you mention above, in phase 2 when we have more
> than one guest sharing the VF,

It's probably more interesting for a card without SRIOV support.

> we will have to add filter lookup in macvlan
> to filter pkts for each guest.

Any chance to enable hardware filters for that?

> This will need some performance tests too.
> 
> Will start investigating the netlink interface comments for phase 1 first.
> 
> Thanks!
> -Roopa

^ permalink raw reply

* Re: [net-next-2.6 PATCH 0/3 RFC] macvlan: MAC Address filtering support for passthru mode
From: Michael S. Tsirkin @ 2011-09-11  9:44 UTC (permalink / raw)
  To: Roopa Prabhu
  Cc: Sridhar Samudrala, netdev, dragos.tatulea, arnd, dwang2, benve,
	kaber, davem, eric.dumazet, mchan, kvm
In-Reply-To: <CA8ECF75.33BFF%roprabhu@cisco.com>

On Thu, Sep 08, 2011 at 08:00:53PM -0700, Roopa Prabhu wrote:
> 
> 
> 
> On 9/8/11 12:33 PM, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Thu, Sep 08, 2011 at 12:23:56PM -0700, Roopa Prabhu wrote:
> >>> 
> >>> I think the main usecase for passthru mode is to assign a SR-IOV VF to
> >>> a single guest.
> >>> 
> >> Yes and for the passthru usecase this patch should be enough to enable
> >> filtering in hw (eventually like I indicated before I need to fix vlan
> >> filtering too).
> > 
> > So with filtering in hw, and in sriov VF case, VFs
> > actually share a filtering table. How will that
> > be partitioned?
> 
> AFAIK, though it might maintain a single filter table space in hw, hw does
> know which filter belongs to which VF. And the OS driver does not need to do
> anything special. The VF driver exposes a VF netdev. And any uc/mc addresses
> registered with a VF netdev are registered with the hw by the driver. And hw
> will filter and send only pkts that the VF has expressed interest in.
> 
> No special filter partitioning in hw is required.
> 
> Thanks,
> Roopa

Yes, but what I mean is, if the size of the single filter table
is limited, we need to decide how many addresses is
each guest allowed. If we let one guest ask for
as many as it wants, it can lock others out.

-- 
MST

^ 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