* Re: KS8695: possible NAPI issue
From: Stephen Hemminger @ 2010-03-05 16:22 UTC (permalink / raw)
To: Yegor Yefremov; +Cc: Figo.zhang, Dick Hollenbeck, netdev, zealcook
In-Reply-To: <f69abfc31003050700y57718a6aje3c1ff41d04748ae@mail.gmail.com>
On Fri, 5 Mar 2010 16:00:11 +0100
Yegor Yefremov <yegorslists@googlemail.com> wrote:
> >> My tests look like following:
> >>
> >> 1. system start
> >> 2. ping one host: O.K.
> >> 3. nc -l -p 5000 > /var/test (about 1Mb): O.K.
> >> 4. ping the same host: failed
> > 1. type "arp" command, see the arp is exit or expire?
>
> debian:~# nc -l -p 5000 > /var/zImage
>
> debian:~# ping -c 1 192.168.1.38
>
> PING 192.168.1.38 (192.168.1.38) 56(84) bytes of data.
>
> From 192.168.1.66 icmp_seq=1 Destination Host Unreachable
>
>
>
> --- 192.168.1.38 ping statistics ---
>
> 1 packets transmitted, 0 received, +1 errors, 100% packet loss, time 0ms
>
>
>
> debian:~# arp
>
> Address HWtype HWaddress Flags Mask Iface
> 192.168.1.38 (incomplete) eth0
>
> 192.168.1.36 ether 00:10:18:39:19:aa C eth0
>
>
> > 2. at this point, see is it still have RX interrpt and receive packet in
> > ks8695_rx()? (in some watchpoint add printk).
>
> I've inserted some printks and I can see that interrupts are coming
> and the received count will be increased. I can also see my system
> sending arp requests. Even this ping request is visible in wireshark
> and its reply, but the ping utility sees nothing of it.
One of the requirements for NAPI to work is that the IRQ is level triggered.
Otherwise there can be a race between packet arrival and the end of
NAPI processing:
static int ks8695_poll(struct napi_struct *napi, int budget)
{
struct ks8695_priv *ksp = container_of(napi, struct ks8695_priv, napi);
unsigned long work_done;
unsigned long isr = readl(KS8695_IRQ_VA + KS8695_INTEN);
unsigned long mask_bit = 1 << ks8695_get_rx_enable_bit(ksp);
work_done = ks8695_rx(ksp, budget);
if (work_done < budget) {
unsigned long flags;
spin_lock_irqsave(&ksp->rx_lock, flags);
>>> packet arrives >>
/*enable rx interrupt*/
writel(isr | mask_bit, KS8695_IRQ_VA + KS8695_INTEN);
>>> or here >>
__napi_complete(napi);
spin_unlock_irqrestore(&ksp->rx_lock, flags);
If the hardware can't be reprogrammed to level mode;
the solution used in some drivers is to poll for lost packet after re-enabling
NAPI.
--
^ permalink raw reply
* Re: [PATCH] Fix netdev_printk null dereference
From: Steve.Glendinning @ 2010-03-05 16:32 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100305.074335.248610315.davem@davemloft.net>
David Miller <davem@davemloft.net> wrote on 05/03/2010 15:43:35:
> It seems to me that really you only need this parent NULL check where
> you notice the USB control operation failed and want to print a
> message about that.
>
> That should cover all the necessary cases shouldn't it?
That's the easily reproducible case - during initialisation there are
a lot of register reads and writes, so it's quite easy to trigger by
manually unplugging ~500ms after connection. In this case usbnet is
still in usbnet_probe.
Theoretically though, this device could be disconnected at any time,
and there are other places in the code where we print messages not
triggered by a usb failure (for example set_multicast). Could it
be possibly unsafe to call netdev_printk here?
> Even more importantly, why does a USB disconnect NULL out the netdev
> parent device pointer? Until you actually release this USB device in
> the driver, the parent pointer should stay there.
Most of the time it's not nulled out, and the code succesfully prints
errors as expected, but maybe 1 time in 20 dev.parent is NULL.
^ permalink raw reply
* Re: [PATCH] Fix netdev_printk null dereference
From: David Miller @ 2010-03-05 16:42 UTC (permalink / raw)
To: Steve.Glendinning; +Cc: netdev
In-Reply-To: <OFD404CD3D.9F15042C-ON802576DD.00595AB8-802576DD.005AD9A8@smsc.com>
From: Steve.Glendinning@smsc.com
Date: Fri, 5 Mar 2010 16:32:03 +0000
> David Miller <davem@davemloft.net> wrote on 05/03/2010 15:43:35:
>
>> Even more importantly, why does a USB disconnect NULL out the netdev
>> parent device pointer? Until you actually release this USB device in
>> the driver, the parent pointer should stay there.
>
> Most of the time it's not nulled out, and the code succesfully prints
> errors as expected, but maybe 1 time in 20 dev.parent is NULL.
I think until the device driver puts it's references and whatnot
of the device it's driving, that parent pointer should be kept
non-NULL.
As long as the netdevice exists and is registered, for example, people
can get at the parent device chain via SYSFS file accesses and
similar.
So it seems to me this is a huge problem waiting to happen anyways and
this netdev_printk() issue is merely making the problem more obvious
:-)
^ permalink raw reply
* bnx2x crash dump on high network load
From: Fischer, Anna @ 2010-03-05 17:07 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: eliezert@broadcom.com, eilong@broadcom.com, Michael Chan
[-- Attachment #1: Type: text/plain, Size: 540 bytes --]
I am seeing driver crashes with a bnx2x version 1.50.16. I have attached two crash dumps that I have captured.
I run Linux with the following kernel:
# uname -a
Linux sup-prj-441106 2.6.16.60-0.54.5-debug #1 SMP Fri Sep 4 01:28:03 UTC 2009 x86_64 x86_64 x86_64 GNU/Linux
I am seeing these driver crashes when I run virtual machines (VMware Server) and those VMs do high bandwidth network I/O, so there are a lot of packets going through the NIC.
Can you give me any hints for what the problem here could be?
Thanks,
Anna
[-- Attachment #2: sup-prj-441104-bnx-crash-dump.log --]
[-- Type: application/octet-stream, Size: 108012 bytes --]
[-- Attachment #3: sup-prj-441104-bnx-crash-dump2.log --]
[-- Type: application/octet-stream, Size: 105323 bytes --]
^ permalink raw reply
* RE: bnx2x crash dump on high network load
From: Eilon Greenstein @ 2010-03-05 17:31 UTC (permalink / raw)
To: Fischer, Anna; +Cc: eliezert, Michael Chan, netdev@vger.kernel.org
In-Reply-To: <0199E0D51A61344794750DC57738F58E6D79C12E3E@GVW1118EXC.americas.hpqcorp.net>
Hi Anna,
On this kernel, enabling bridging (like you do on virtualization) does not turn off LRO automatically. Please try to load the bnx2x with disable_tpa=1 (TPA is the HW based LRO feature).
Regards,
Eilon
-----Original Message-----
From: Fischer, Anna [mailto:anna.fischer@hp.com]
Sent: Friday, March 05, 2010 7:08 PM
To: netdev@vger.kernel.org
Cc: eliezert; Eilon Greenstein; Michael Chan
Subject: bnx2x crash dump on high network load
I am seeing driver crashes with a bnx2x version 1.50.16. I have attached two crash dumps that I have captured.
I run Linux with the following kernel:
# uname -a
Linux sup-prj-441106 2.6.16.60-0.54.5-debug #1 SMP Fri Sep 4 01:28:03 UTC 2009 x86_64 x86_64 x86_64 GNU/Linux
I am seeing these driver crashes when I run virtual machines (VMware Server) and those VMs do high bandwidth network I/O, so there are a lot of packets going through the NIC.
Can you give me any hints for what the problem here could be?
Thanks,
Anna
^ permalink raw reply
* net: smc91x: Support Qualcomm MSM development boards.
From: Daniel Walker @ 2010-03-05 19:12 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: David Brown, netdev
From: David Brown <davidb@quicinc.com>
Signed-off-by: David Brown <davidb@quicinc.com>
Signed-off-by: Daniel Walker <dwalker@codeaurora.org>
---
drivers/net/smc91x.h | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/drivers/net/smc91x.h b/drivers/net/smc91x.h
index 5479954..a6ee883 100644
--- a/drivers/net/smc91x.h
+++ b/drivers/net/smc91x.h
@@ -330,6 +330,20 @@ static inline void LPD7_SMC_outsw (unsigned char* a, int r,
#include <unit/smc91111.h>
+#elif defined(CONFIG_ARCH_MSM)
+
+#define SMC_CAN_USE_8BIT 0
+#define SMC_CAN_USE_16BIT 1
+#define SMC_CAN_USE_32BIT 0
+#define SMC_NOWAIT 1
+
+#define SMC_inw(a, r) readw((a) + (r))
+#define SMC_outw(v, a, r) writew(v, (a) + (r))
+#define SMC_insw(a, r, p, l) readsw((a) + (r), p, l)
+#define SMC_outsw(a, r, p, l) writesw((a) + (r), p, l)
+
+#define SMC_IRQ_FLAGS IRQF_TRIGGER_HIGH
+
#else
/*
--
1.6.3.3
^ permalink raw reply related
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Pavel Emelyanov @ 2010-03-05 19:18 UTC (permalink / raw)
To: Eric W. Biederman, Sukadev Bhattiprolu
Cc: Daniel Lezcano, Serge Hallyn, Linux Netdev List, containers,
Netfilter Development Mailinglist, Ben Greear
In-Reply-To: <m17hptjh3m.fsf@fess.ebiederm.org>
> 2 parallel enters? I meant you have pid 0 in the entered pid namespace.
> You have pid 0 because your pid simply does not map.
Oh, I see.
> There is nothing that makes to parallel enters impossible in that.
> Even today we have one thread per cpu that has task->pid == &init_struct_pid
> which is pid 0.
How about the forked processes then? Who will be their parent?
> For the case of unshare where we are designed to be used with PAM I don't
> think my proposed semantics work. For a join needed an extra fork before
> you are really in the pid namespace should be minor.
Hm... One more proposal - can we adopt the planned new fork_with_pids system
call to fork the process right into a new pid namespace?
> That doesn't handle the case of cached struct pids. A good example is
> waitpid, where it waits for a specific struct pid. Which means that
> allocating a new struct pid and changing task->pid will cause
> waitpid(pid) to wait forever...
OK. Good example. Thanks.
> To change struct pid would require the refcount on struct pid to show
> no references from anywhere except the task_struct.
I think this is OK to return -EBUSY for this. And fix the waitpid
respectively not to block this common case. All the others I think
can be stayed as is.
> At the cost of a little memory we can solve that problem for unshare
> if we have a an extra upid in struct pid, how we verify there is space
> in struct pid I'm not certain.
>
> I do think that at least until someone calls exec the namespace pids are
> reported to the process itself should not change. That is kill and
Wait a second - in that case the wait will be blocked too! No?
> waitpid etc. Which suggests an implementation the opposite of what
> I proposed. With ns_of_pid(task_pid(current)) being used as the
> pid namespace of children, and current->nsproxy->pid_ns not changing
> in the case of unshare.
>
> Shrug.
>
> Or perhaps this is a case where we use we can implement join with
> an extra process but we can't implement unshare, because the effect
> cannot be immediate.
Well, I'm talking only about the join now.
> Eric
>
^ permalink raw reply
* [PATCH] net: Fix race condition on receive path.
From: Daniel Walker @ 2010-03-05 19:34 UTC (permalink / raw)
To: David S. Miller; +Cc: Jim Harford, netdev
From: Jim Harford <c_jharfo@quicinc.com>
Fixes a race condition on the networking receive path that causes all
received packets to be dropped after 15-60 minutes of heavy network usage.
Function process_backlog() empties the receive queue, re-enables
interrupts, then "completes" the softIRQ. This provides a time window for
netif_rx() to execute (in IRQ context) and enqueue a received packet
without re-scheduling the softIRQ. After this, the receive queue is never
processed and the system eventually begins to drop all received packets.
The fix has netif_rx() calling napi_schedule(), whether or not the
receive queue is empty. There are two questions that must be addressed:
1) Is the fix correct? Or, is it safe to always call napi_schedule()?
2) Does the fix result in a performance degradation?
With respect to correctness, the napi softIRQ routine should NOT be
scheduled if it is already running. This is controlled by the bit flag
NAPI_STATE_SCHED in the "state" field of struct napi_struct. When this
flag is set, it means the softIRQ routine is either already pending or
currently running. When the softIRQ is finished, the bit is cleared via
__napi_complete(), which is called by napi_complete(), which is called by
process_backlog() after re-enabling interrupts and just prior to
returning.
When netif_rx() calls napi_schedule(), that routine checks the
NAPI_STATE_SCHED bit flag by calling napi_schedule_prep(), which calls
test_and_set_bit(); if the flag is already set, napi_schedule() returns
without doing anything. Note that test_and_set_bit() implements an
atomic "test and set" operation that is commonly used as the foundation
of mutual exclusion mechanisms.
So it is safe for netif_rx() to call napi_schedule(), whether or not the
receive queue is empty.
With respect to performance, the following behavior was observed using an
ARM CPU and cross-compiling for ARM using gcc. Removing the test of
whether the receive queue is empty saves 2 assembly instructions. The
addition of calling napi_schedule(), when that routine does nothing
(because the NAPI_STATE_SCHED flag is set) adds 11 assembly instructions.
Thus, there is a net addition of 9 assembly instructions. Furthermore,
we observe in our testing that even under heavy network load, the receive
queue is empty on at least 95% of netif_rx() invocations. The 9 assembly
instructions amortized over all netif_rx() invocations yield an average
performance penalty of less than 1 assembly instruction per invocation of
netif_rx(). Finally, the test-and-set operation does not entail testing
common memory shared by multiple CPU cores (which can be costly), because
the data structures in question are specific to a single CPU by design.
So calling napi_schedule() at every invocation of netif_rx() does not
impair performance.
CRs-fixed: 184599
Signed-off-by: Jim Harford <c_jharfo@quicinc.com>
Signed-off-by: Daniel Walker <dwalker@codeaurora.org>
---
net/core/dev.c | 11 +++--------
1 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index be9924f..43161c6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2150,15 +2150,10 @@ int netif_rx(struct sk_buff *skb)
__get_cpu_var(netdev_rx_stat).total++;
if (queue->input_pkt_queue.qlen <= netdev_max_backlog) {
- if (queue->input_pkt_queue.qlen) {
-enqueue:
- __skb_queue_tail(&queue->input_pkt_queue, skb);
- local_irq_restore(flags);
- return NET_RX_SUCCESS;
- }
-
+ __skb_queue_tail(&queue->input_pkt_queue, skb);
napi_schedule(&queue->backlog);
- goto enqueue;
+ local_irq_restore(flags);
+ return NET_RX_SUCCESS;
}
__get_cpu_var(netdev_rx_stat).dropped++;
--
1.6.3.3
^ permalink raw reply related
* Re: net: smc91x: Support Qualcomm MSM development boards.
From: Nicolas Pitre @ 2010-03-05 19:49 UTC (permalink / raw)
To: Daniel Walker; +Cc: David Brown, netdev
In-Reply-To: <1267816354.30393.4.camel@c-dwalke-linux.qualcomm.com>
On Fri, 5 Mar 2010, Daniel Walker wrote:
> From: David Brown <davidb@quicinc.com>
>
> Signed-off-by: David Brown <davidb@quicinc.com>
> Signed-off-by: Daniel Walker <dwalker@codeaurora.org>
Acked-by: Nicolas Pitre <nico@fluxnic.net>
> ---
> drivers/net/smc91x.h | 14 ++++++++++++++
> 1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/smc91x.h b/drivers/net/smc91x.h
> index 5479954..a6ee883 100644
> --- a/drivers/net/smc91x.h
> +++ b/drivers/net/smc91x.h
> @@ -330,6 +330,20 @@ static inline void LPD7_SMC_outsw (unsigned char* a, int r,
>
> #include <unit/smc91111.h>
>
> +#elif defined(CONFIG_ARCH_MSM)
> +
> +#define SMC_CAN_USE_8BIT 0
> +#define SMC_CAN_USE_16BIT 1
> +#define SMC_CAN_USE_32BIT 0
> +#define SMC_NOWAIT 1
> +
> +#define SMC_inw(a, r) readw((a) + (r))
> +#define SMC_outw(v, a, r) writew(v, (a) + (r))
> +#define SMC_insw(a, r, p, l) readsw((a) + (r), p, l)
> +#define SMC_outsw(a, r, p, l) writesw((a) + (r), p, l)
> +
> +#define SMC_IRQ_FLAGS IRQF_TRIGGER_HIGH
> +
> #else
>
> /*
> --
> 1.6.3.3
>
>
>
^ permalink raw reply
* Re: net: smc91x: Support Qualcomm MSM development boards.
From: Daniel Walker @ 2010-03-05 20:05 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: David Brown, netdev
In-Reply-To: <alpine.LFD.2.00.1003051449000.31128@xanadu.home>
On Fri, 2010-03-05 at 14:49 -0500, Nicolas Pitre wrote:
> On Fri, 5 Mar 2010, Daniel Walker wrote:
>
> > From: David Brown <davidb@quicinc.com>
> >
> > Signed-off-by: David Brown <davidb@quicinc.com>
> > Signed-off-by: Daniel Walker <dwalker@codeaurora.org>
>
> Acked-by: Nicolas Pitre <nico@fluxnic.net>
>
>
Are you going to send it up stream, or should I?
Daniel
^ permalink raw reply
* Re: net: smc91x: Support Qualcomm MSM development boards.
From: David Miller @ 2010-03-05 20:09 UTC (permalink / raw)
To: dwalker; +Cc: nico, davidb, netdev
In-Reply-To: <1267819507.30393.9.camel@c-dwalke-linux.qualcomm.com>
From: Daniel Walker <dwalker@codeaurora.org>
Date: Fri, 05 Mar 2010 12:05:07 -0800
> On Fri, 2010-03-05 at 14:49 -0500, Nicolas Pitre wrote:
>> On Fri, 5 Mar 2010, Daniel Walker wrote:
>>
>> > From: David Brown <davidb@quicinc.com>
>> >
>> > Signed-off-by: David Brown <davidb@quicinc.com>
>> > Signed-off-by: Daniel Walker <dwalker@codeaurora.org>
>>
>> Acked-by: Nicolas Pitre <nico@fluxnic.net>
>>
>>
>
> Are you going to send it up stream, or should I?
By virtue of having been properly posted to netdev it's in patchwork
and therefore I'll integrate it.
^ permalink raw reply
* Re: net: smc91x: Support Qualcomm MSM development boards.
From: Daniel Walker @ 2010-03-05 20:11 UTC (permalink / raw)
To: David Miller; +Cc: nico, davidb, netdev
In-Reply-To: <20100305.120945.51475611.davem@davemloft.net>
On Fri, 2010-03-05 at 12:09 -0800, David Miller wrote:
> From: Daniel Walker <dwalker@codeaurora.org>
> Date: Fri, 05 Mar 2010 12:05:07 -0800
>
> > On Fri, 2010-03-05 at 14:49 -0500, Nicolas Pitre wrote:
> >> On Fri, 5 Mar 2010, Daniel Walker wrote:
> >>
> >> > From: David Brown <davidb@quicinc.com>
> >> >
> >> > Signed-off-by: David Brown <davidb@quicinc.com>
> >> > Signed-off-by: Daniel Walker <dwalker@codeaurora.org>
> >>
> >> Acked-by: Nicolas Pitre <nico@fluxnic.net>
> >>
> >>
> >
> > Are you going to send it up stream, or should I?
>
> By virtue of having been properly posted to netdev it's in patchwork
> and therefore I'll integrate it.
Well I didn't add [PATCH] to the subject , but I guess that doesn't
matter..
Ok.. Thanks.
Daniel
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Eric W. Biederman @ 2010-03-05 20:26 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: Sukadev Bhattiprolu, Daniel Lezcano, Serge Hallyn,
Linux Netdev List, containers, Netfilter Development Mailinglist,
Ben Greear
In-Reply-To: <4B9158F5.5040205@parallels.com>
Pavel Emelyanov <xemul@parallels.com> writes:
>> 2 parallel enters? I meant you have pid 0 in the entered pid namespace.
>> You have pid 0 because your pid simply does not map.
>
> Oh, I see.
>
>> There is nothing that makes to parallel enters impossible in that.
>> Even today we have one thread per cpu that has task->pid == &init_struct_pid
>> which is pid 0.
>
> How about the forked processes then? Who will be their parent?
The normal rules of parentage apply. So the child will see simply
see it's parent as ppid == 0. If that child daemonizes it will become
a child of the pid namespaces init.
This is a lot like something that gets started from call_usermodehelper. It's
parent process is not a descendant of init either.
The implementation of the join is to simply change current->nsproxy->pid_ns.
Then to use it you simply fork to get a child in the target pid namespace.
>> For the case of unshare where we are designed to be used with PAM I don't
>> think my proposed semantics work. For a join needed an extra fork before
>> you are really in the pid namespace should be minor.
>
> Hm... One more proposal - can we adopt the planned new fork_with_pids system
> call to fork the process right into a new pid namespace?
In a lot of ways I like this idea of sys_hijack/sys_cloneat, and I
don't think anything I am doing fundamentally undermines it. The use
case of doing things in fork is that there is automatic inheritance of
everything. All of the namespaces and all of the control groups, and
possibly also the parent process. It does have the high cost that the
process we are copying from must be stopped because there are no locks
that let us take everything. I haven't looked at the recent proposals
to see if anyone has solved that problem cleanly.
If we can do a sys_hijack/sys_cloneat style of join, that means we can
afford a fork. At which point the my proposed pid namespace semantics
should be fine.
aka:
setns(NSTYPE_PID);
pid = fork();
if (pid == 0) {
getpid() == 2; /* Or whatever the first free pid is joined pid namespace */
getppid() == 0;
} else {
pid == 6400; /* Or whatever the first free pid is in the original pid namespace */
waitpid(pid);
}
>> That doesn't handle the case of cached struct pids. A good example is
>> waitpid, where it waits for a specific struct pid. Which means that
>> allocating a new struct pid and changing task->pid will cause
>> waitpid(pid) to wait forever...
>
> OK. Good example. Thanks.
>
>> To change struct pid would require the refcount on struct pid to show
>> no references from anywhere except the task_struct.
>
> I think this is OK to return -EBUSY for this. And fix the waitpid
> respectively not to block this common case. All the others I think
> can be stayed as is.
That would probably work. setsid() and setpgrp() have similar sorts
of restrictions. That is both more challenging and more limiting than
the semantics that come out of my unshare(CLONE_NEWPID) patch. So I
would prefer to keep this sort of thing as a last resort.
>> At the cost of a little memory we can solve that problem for unshare
>> if we have a an extra upid in struct pid, how we verify there is space
>> in struct pid I'm not certain.
>>
>> I do think that at least until someone calls exec the namespace pids are
>> reported to the process itself should not change. That is kill and
>
> Wait a second - in that case the wait will be blocked too! No?
If all we do is populate an unused struct upid in struct pid there
isn't a chance of a problem.
>> waitpid etc. Which suggests an implementation the opposite of what
>> I proposed. With ns_of_pid(task_pid(current)) being used as the
>> pid namespace of children, and current->nsproxy->pid_ns not changing
>> in the case of unshare.
>>
>> Shrug.
>>
>> Or perhaps this is a case where we use we can implement join with
>> an extra process but we can't implement unshare, because the effect
>> cannot be immediate.
>
> Well, I'm talking only about the join now.
Overall it sounds like the semantics I have proposed with
unshare(CLONE_NEWPID) are workable, and simple to implement. The
extra fork is a bit surprising but it certainly does not
look like a show stopper for implementing a pid namespace join.
Eric
^ permalink raw reply
* RE: [PATCH V3 6/8] tipc: use limited socket backlog
From: Stephens, Allan @ 2010-03-05 20:48 UTC (permalink / raw)
To: Zhu Yi; +Cc: netdev, Jon Maloy, davem, Eric Dumazet
In-Reply-To: <1267761707-15605-6-git-send-email-yi.zhu@intel.com>
>
> Make tipc adapt to the limited socket backlog change.
>
> Cc: Jon Maloy <jon.maloy@ericsson.com>
> Cc: Allan Stephens <allan.stephens@windriver.com>
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
>From visual inspection and basic sanity testing:
Acked-by: Allan Stephens <allan.stephens@windriver.com>
Nice work!
-- Al
^ permalink raw reply
* Re: net: smc91x: Support Qualcomm MSM development boards.
From: Nicolas Pitre @ 2010-03-05 20:57 UTC (permalink / raw)
To: Daniel Walker; +Cc: David Brown, netdev
In-Reply-To: <1267819507.30393.9.camel@c-dwalke-linux.qualcomm.com>
On Fri, 5 Mar 2010, Daniel Walker wrote:
> On Fri, 2010-03-05 at 14:49 -0500, Nicolas Pitre wrote:
> > On Fri, 5 Mar 2010, Daniel Walker wrote:
> >
> > > From: David Brown <davidb@quicinc.com>
> > >
> > > Signed-off-by: David Brown <davidb@quicinc.com>
> > > Signed-off-by: Daniel Walker <dwalker@codeaurora.org>
> >
> > Acked-by: Nicolas Pitre <nico@fluxnic.net>
> >
> >
>
> Are you going to send it up stream, or should I?
Please do it.
Nicolas
^ permalink raw reply
* Re: net: smc91x: Support Qualcomm MSM development boards.
From: Nicolas Pitre @ 2010-03-05 20:57 UTC (permalink / raw)
To: David Miller; +Cc: dwalker, davidb, netdev
In-Reply-To: <20100305.120945.51475611.davem@davemloft.net>
On Fri, 5 Mar 2010, David Miller wrote:
> From: Daniel Walker <dwalker@codeaurora.org>
> Date: Fri, 05 Mar 2010 12:05:07 -0800
>
> > On Fri, 2010-03-05 at 14:49 -0500, Nicolas Pitre wrote:
> >> On Fri, 5 Mar 2010, Daniel Walker wrote:
> >>
> >> > From: David Brown <davidb@quicinc.com>
> >> >
> >> > Signed-off-by: David Brown <davidb@quicinc.com>
> >> > Signed-off-by: Daniel Walker <dwalker@codeaurora.org>
> >>
> >> Acked-by: Nicolas Pitre <nico@fluxnic.net>
> >>
> >>
> >
> > Are you going to send it up stream, or should I?
>
> By virtue of having been properly posted to netdev it's in patchwork
> and therefore I'll integrate it.
Good.
Nicolas
^ permalink raw reply
* Re: [PATCH] net: Fix race condition on receive path.
From: Stephen Hemminger @ 2010-03-05 21:21 UTC (permalink / raw)
To: Daniel Walker; +Cc: David S. Miller, Jim Harford, netdev
In-Reply-To: <1267817699.30393.8.camel@c-dwalke-linux.qualcomm.com>
On Fri, 05 Mar 2010 11:34:59 -0800
Daniel Walker <dwalker@codeaurora.org> wrote:
> Fixes a race condition on the networking receive path that causes all
> received packets to be dropped after 15-60 minutes of heavy network usage.
> Function process_backlog() empties the receive queue, re-enables
> interrupts, then "completes" the softIRQ. This provides a time window for
> netif_rx() to execute (in IRQ context) and enqueue a received packet
> without re-scheduling the softIRQ. After this, the receive queue is never
> processed and the system eventually begins to drop all received packets.
I wonder why this hasn't shown up before?
Where exactly is the window between empty process_backlog and netif_rx?
Maybe it is ARM specific behavior of softirq?
^ permalink raw reply
* Re: [PATCH V3 8/8] net: backlog functions rename
From: David Miller @ 2010-03-05 21:36 UTC (permalink / raw)
To: eric.dumazet; +Cc: yi.zhu, netdev
In-Reply-To: <1267770735.2867.9.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 05 Mar 2010 07:32:15 +0100
> Le vendredi 05 mars 2010 à 12:01 +0800, Zhu Yi a écrit :
>> sk_add_backlog -> __sk_add_backlog
>> sk_add_backlog_limited -> sk_add_backlog
>>
>> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
>> ---
>> include/net/sock.h | 6 +++---
>> net/core/sock.c | 2 +-
>> net/dccp/minisocks.c | 2 +-
>> net/ipv4/tcp_ipv4.c | 2 +-
>> net/ipv4/tcp_minisocks.c | 2 +-
>> net/ipv4/udp.c | 2 +-
>> net/ipv6/tcp_ipv6.c | 2 +-
>> net/ipv6/udp.c | 4 ++--
>> net/llc/llc_c_ac.c | 2 +-
>> net/llc/llc_conn.c | 2 +-
>> net/sctp/input.c | 4 ++--
>> net/tipc/socket.c | 2 +-
>> net/x25/x25_dev.c | 2 +-
>> 13 files changed, 17 insertions(+), 17 deletions(-)
>>
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
All 8 patches applied to net-2.6, thanks Zhu!
Feel free to send me a patch which adds the "__must_check"
tag to sk_add_backlog() so that any failure to check and
drop packets will be at least warned about.
Thanks!
^ permalink raw reply
* Re: [PATCH v4] ethtool: Add direct access to ops->get_sset_count
From: David Miller @ 2010-03-05 22:00 UTC (permalink / raw)
To: jeff; +Cc: jeffrey.t.kirsher, netdev, gospo
In-Reply-To: <4B8FB439.8050901@garzik.org>
From: Jeff Garzik <jeff@garzik.org>
Date: Thu, 04 Mar 2010 08:23:05 -0500
> On 03/04/2010 03:51 AM, Jeff Kirsher wrote:
>> From: Jeff Garzik<jgarzik@redhat.com>
>>
>> This patch is an alternative approach for accessing string
>> counts, vs. the drvinfo indirect approach. This way the drvinfo
>> space doesn't run out, and we don't break ABI later.
>>
>> Signed-off-by: Jeff Garzik<jgarzik@redhat.com>
>> Signed-off-by: Peter P Waskiewicz Jr<peter.p.waskiewicz.jr@intel.com>
>> Signed-off-by: Jeff Kirsher<jeffrey.t.kirsher@intel.com>
>> ---
>>
>> include/linux/ethtool.h | 17 +++++++++--
>> net/core/ethtool.c | 72
>> +++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 86 insertions(+), 3 deletions(-)
>
> Both patches look good to me. There is a cosmetic issue of needing to
> sync up userspace and kernel ethtool.h WRT whitespace and deleted
> constants, but I can do that after DaveM applies this patch.
>
> Waiting for upstream application, or other objections...
Applied, thanks guys.
^ permalink raw reply
* Re: [PATCH] Re: [PATCH v4] ethtool: Add direct access to ops->get_sset_count
From: David Miller @ 2010-03-05 22:00 UTC (permalink / raw)
To: jeff; +Cc: bhutchings, jeffrey.t.kirsher, netdev, gospo
In-Reply-To: <4B8FFA41.2020700@garzik.org>
From: Jeff Garzik <jeff@garzik.org>
Date: Thu, 04 Mar 2010 13:21:53 -0500
> I've attached a follow-up patch, which should enable my/Jeff's kernel
> patch to be applied, followed by this one.
Applied, thanks Jeff.
^ permalink raw reply
* Re: [PATCH] s2io: Fixing debug message
From: David Miller @ 2010-03-05 22:01 UTC (permalink / raw)
To: leitao; +Cc: sreenivasa.honnur, netdev
In-Reply-To: <1267735244-8367-1-git-send-email-leitao@linux.vnet.ibm.com>
From: leitao@linux.vnet.ibm.com
Date: Thu, 4 Mar 2010 15:40:44 -0500
> Currently s2io is dumping debug messages using the interface name
> before it was allocated, showing a message like the following:
>
> s2io: eth%d: Ring Mem PHY: 0x7ef80000
> s2io: s2io_reset: Resetting XFrame card eth%d
>
> This patch just fixes it, printing the pci bus information for
> the card instead of the interface name.
>
> Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>
Applied, thanks.
The netdev_*() printing macros we now have in linux/netdev.h
allow to handle these cases transparently btw.
^ permalink raw reply
* Re: [PATCH] typhoon: fix incorrect use of smp_wmb()
From: David Miller @ 2010-03-05 22:01 UTC (permalink / raw)
To: dave; +Cc: netdev
In-Reply-To: <1267713436.2328.12.camel@lap75545.ornl.gov>
From: David Dillow <dave@thedillows.org>
Date: Thu, 04 Mar 2010 09:37:16 -0500
> The typhoon driver was incorrectly using smp_wmb() to order memory
> accesses against IO to the NIC in a few instances. Use wmb() instead,
> which is required to actually order between memory types.
>
> Signed-off-by: David Dillow <dave@thedillows.org>
Applied.
^ permalink raw reply
* Re: [PATCH] e1000e: fix packet corruption and tx hang during NFSv2
From: David Miller @ 2010-03-05 22:01 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, nhorman, jesse.brandeburg
In-Reply-To: <20100305122131.8250.21132.stgit@localhost.localdomain>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 05 Mar 2010 04:21:44 -0800
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
>
> when receiving a particular type of NFS v2 UDP traffic, the hardware could
> DMA some bad data and then hang, possibly corrupting memory.
>
> Disable the NFS parsing in this hardware, verified to fix the bug.
>
> Originally reported and reproduced by RedHat's Neil Horman
> CC: nhorman@tuxdriver.com
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied, thanks.
^ permalink raw reply
* RE: [PATCH] net: Fix race condition on receive path.
From: Harford, Jim @ 2010-03-05 23:41 UTC (permalink / raw)
To: Stephen Hemminger, Daniel Walker
Cc: David S. Miller, netdev@vger.kernel.org, Smith, Alan
In-Reply-To: <20100305132118.1855617a@nehalam>
It appears that this patch is no longer necessary. It was made against 2.6.29, but I see that more recent kernel versions don't have the problem code. For a more detailed explanation, see below. All code references are in routine process_backlog(), file net/core/dev.c.
In kernel version 2.6.27.45, __napi_complete() is invoked BEFORE interrupts are re-enabled. Thus, the receive queue status is cleaned up before another interrupt (due to a receive packet) can occur. This is good design.
In kernel version 2.6.29, git commit ID 303c6a025 inverts this ordering. Routine napi_complete() is invoked AFTER interrupts are re-enabled. We observed interrupts taken after interrupts were re-enabled, but before napi_complete cleaned up the receive queue. This would then shut down the processing of subsequent received packets.
In kernel versions 2.6.30.10 and later, the sequence of operations is identical to 2.6.27.45, so there is no problem.
Jim Harford
Qualcomm Innovation Center
-----Original Message-----
From: Stephen Hemminger [mailto:shemminger@vyatta.com]
Sent: Friday, March 05, 2010 4:21 PM
To: Daniel Walker
Cc: David S. Miller; Harford, Jim; netdev@vger.kernel.org
Subject: Re: [PATCH] net: Fix race condition on receive path.
On Fri, 05 Mar 2010 11:34:59 -0800
Daniel Walker <dwalker@codeaurora.org> wrote:
> Fixes a race condition on the networking receive path that causes all
> received packets to be dropped after 15-60 minutes of heavy network usage.
> Function process_backlog() empties the receive queue, re-enables
> interrupts, then "completes" the softIRQ. This provides a time window for
> netif_rx() to execute (in IRQ context) and enqueue a received packet
> without re-scheduling the softIRQ. After this, the receive queue is never
> processed and the system eventually begins to drop all received packets.
I wonder why this hasn't shown up before?
Where exactly is the window between empty process_backlog and netif_rx?
Maybe it is ARM specific behavior of softirq?
^ permalink raw reply
* Re: [PATCH 6/13] bridge: Add core IGMP snooping support
From: Paul E. McKenney @ 2010-03-05 23:43 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, netdev, Stephen Hemminger
In-Reply-To: <E1NlbuT-00021C-0b@gondolin.me.apana.org.au>
On Sun, Feb 28, 2010 at 01:41:45PM +0800, Herbert Xu wrote:
> bridge: Add core IGMP snooping support
>
> This patch adds the core functionality of IGMP snooping support
> without actually hooking it up. So this patch should be a no-op
> as far as the bridge's external behaviour is concerned.
>
> All the new code and data is controlled by the Kconfig option
> BRIDGE_IGMP_SNOOPING. A run-time toggle is also available.
>
> The multicast switching is done using an hash table that is
> lockless on the read-side through RCU. On the write-side the
> new multicast_lock is used for all operations. The hash table
> supports dynamic growth/rehashing.
Cool!!! You use a pair of list_head structures, so that a given
element can be in both the old and the new hash table simultaneously.
Of course, an RCU grace period must elapse between consecutive resizings.
Which appears to be addressed.
The teardown needs an rcu_barrier_bh() rather than the current
synchronize_rcu_bh(), please see below.
Also, I don't see how the teardown code is preventing new readers from
finding the data structures before they are being passed to call_rcu_bh().
You can't safely start the RCU grace period until -after- all new readers
have been excluded. (But I could easily be missing something here.)
The br_multicast_del_pg() looks to need rcu_read_lock_bh() and
rcu_read_unlock_bh() around its loop, if I understand the pointer-walking
scheme correctly.
It looks like all updates are protected by an appropriate lock, although
I don't claim to fully understand how the data structures are linked
together. The key requirement is of course that you normally don't
get to protect insertion and deletion of a given data structure using a
lock located within that same data structure. It looks to me that this
requirement is satisfied -- all the lists that you manipulate seem to
hang off of the net_bridge, which contains the lock protecting all of
those lists.
Hmmm... Where is the read-side code? Wherever it is, it cannot safely
dereference the ->old pointer.
> The hash table will be rehashed if any chain length exceeds a
> preset limit. If rehashing does not reduce the maximum chain
> length then snooping will be disabled.
>
> These features may be added in future (in no particular order):
>
> * IGMPv3 source support
> * Non-querier router detection
> * IPv6
But the bugs all look fixable to me (assuming that they are in fact
bugs). Very cool to see an RCU-protected resizeable hash table!!! ;-)
Thanx, Paul
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> ---
>
> net/bridge/Kconfig | 12
> net/bridge/Makefile | 2
> net/bridge/br_multicast.c | 1135 ++++++++++++++++++++++++++++++++++++++++++++++
> net/bridge/br_private.h | 139 +++++
> 4 files changed, 1288 insertions(+)
>
> diff --git a/net/bridge/Kconfig b/net/bridge/Kconfig
> index e143ca6..78dd549 100644
> --- a/net/bridge/Kconfig
> +++ b/net/bridge/Kconfig
> @@ -31,3 +31,15 @@ config BRIDGE
> will be called bridge.
>
> If unsure, say N.
> +
> +config BRIDGE_IGMP_SNOOPING
> + bool "IGMP snooping"
> + default y
> + ---help---
> + If you say Y here, then the Ethernet bridge will be able selectively
> + forward multicast traffic based on IGMP traffic received from each
> + port.
> +
> + Say N to exclude this support and reduce the binary size.
> +
> + If unsure, say Y.
> diff --git a/net/bridge/Makefile b/net/bridge/Makefile
> index f444c12..d0359ea 100644
> --- a/net/bridge/Makefile
> +++ b/net/bridge/Makefile
> @@ -12,4 +12,6 @@ bridge-$(CONFIG_SYSFS) += br_sysfs_if.o br_sysfs_br.o
>
> bridge-$(CONFIG_BRIDGE_NETFILTER) += br_netfilter.o
>
> +bridge-$(CONFIG_BRIDGE_IGMP_SNOOPING) += br_multicast.o
> +
> obj-$(CONFIG_BRIDGE_NF_EBTABLES) += netfilter/
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> new file mode 100644
> index 0000000..746b5a6
> --- /dev/null
> +++ b/net/bridge/br_multicast.c
> @@ -0,0 +1,1135 @@
> +/*
> + * Bridge multicast support.
> + *
> + * Copyright (c) 2010 Herbert Xu <herbert@gondor.apana.org.au>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + */
> +
> +#include <linux/err.h>
> +#include <linux/if_ether.h>
> +#include <linux/igmp.h>
> +#include <linux/jhash.h>
> +#include <linux/kernel.h>
> +#include <linux/netdevice.h>
> +#include <linux/netfilter_bridge.h>
> +#include <linux/random.h>
> +#include <linux/rculist.h>
> +#include <linux/skbuff.h>
> +#include <linux/slab.h>
> +#include <linux/timer.h>
> +#include <net/ip.h>
> +
> +#include "br_private.h"
> +
> +static inline int br_ip_hash(struct net_bridge_mdb_htable *mdb, __be32 ip)
> +{
> + return jhash_1word(mdb->secret, (u32)ip) & (mdb->max - 1);
> +}
> +
> +static struct net_bridge_mdb_entry *__br_mdb_ip_get(
> + struct net_bridge_mdb_htable *mdb, __be32 dst, int hash)
> +{
> + struct net_bridge_mdb_entry *mp;
> + struct hlist_node *p;
> +
> + hlist_for_each_entry(mp, p, &mdb->mhash[hash], hlist[mdb->ver]) {
> + if (dst == mp->addr)
> + return mp;
> + }
> +
> + return NULL;
> +}
> +
> +static struct net_bridge_mdb_entry *br_mdb_ip_get(
> + struct net_bridge_mdb_htable *mdb, __be32 dst)
> +{
> + return __br_mdb_ip_get(mdb, dst, br_ip_hash(mdb, dst));
> +}
> +
> +struct net_bridge_mdb_entry *br_mdb_get(struct net_bridge *br,
> + struct sk_buff *skb)
> +{
> + struct net_bridge_mdb_htable *mdb = br->mdb;
> +
> + if (!mdb || br->multicast_disabled)
> + return NULL;
> +
> + switch (skb->protocol) {
> + case htons(ETH_P_IP):
> + if (BR_INPUT_SKB_CB(skb)->igmp)
> + break;
> + return br_mdb_ip_get(mdb, ip_hdr(skb)->daddr);
> + }
> +
> + return NULL;
> +}
> +
> +static void br_mdb_free(struct rcu_head *head)
> +{
> + struct net_bridge_mdb_htable *mdb =
> + container_of(head, struct net_bridge_mdb_htable, rcu);
> + struct net_bridge_mdb_htable *old = mdb->old;
> +
> + mdb->old = NULL;
So one way to figure out when it is safe to do another resize is when
the ->old pointer is NULLed out here.
> + kfree(old->mhash);
> + kfree(old);
> +}
> +
> +static int br_mdb_copy(struct net_bridge_mdb_htable *new,
> + struct net_bridge_mdb_htable *old,
> + int elasticity)
> +{
> + struct net_bridge_mdb_entry *mp;
> + struct hlist_node *p;
> + int maxlen;
> + int len;
> + int i;
> +
> + for (i = 0; i < old->max; i++)
> + hlist_for_each_entry(mp, p, &old->mhash[i], hlist[old->ver])
> + hlist_add_head(&mp->hlist[new->ver],
> + &new->mhash[br_ip_hash(new, mp->addr)]);
> +
> + if (!elasticity)
> + return 0;
> +
> + maxlen = 0;
> + for (i = 0; i < new->max; i++) {
> + len = 0;
> + hlist_for_each_entry(mp, p, &new->mhash[i], hlist[new->ver])
> + len++;
> + if (len > maxlen)
> + maxlen = len;
> + }
> +
> + return maxlen > elasticity ? -EINVAL : 0;
> +}
> +
> +static void br_multicast_free_pg(struct rcu_head *head)
> +{
> + struct net_bridge_port_group *p =
> + container_of(head, struct net_bridge_port_group, rcu);
> +
> + kfree(p);
> +}
> +
> +static void br_multicast_free_group(struct rcu_head *head)
> +{
> + struct net_bridge_mdb_entry *mp =
> + container_of(head, struct net_bridge_mdb_entry, rcu);
> +
> + kfree(mp);
> +}
> +
> +static void br_multicast_group_expired(unsigned long data)
> +{
> + struct net_bridge_mdb_entry *mp = (void *)data;
> + struct net_bridge *br = mp->br;
> + struct net_bridge_mdb_htable *mdb;
> +
> + spin_lock(&br->multicast_lock);
> + if (!netif_running(br->dev) || timer_pending(&mp->timer))
> + goto out;
> +
> + if (!hlist_unhashed(&mp->mglist))
> + hlist_del_init(&mp->mglist);
> +
> + if (mp->ports)
> + goto out;
> +
> + mdb = br->mdb;
> + hlist_del_rcu(&mp->hlist[mdb->ver]);
Also protected by br->multicast_lock.
> + mdb->size--;
> +
> + del_timer(&mp->query_timer);
> + call_rcu_bh(&mp->rcu, br_multicast_free_group);
> +
> +out:
> + spin_unlock(&br->multicast_lock);
> +}
> +
> +static void br_multicast_del_pg(struct net_bridge *br,
> + struct net_bridge_port_group *pg)
> +{
> + struct net_bridge_mdb_htable *mdb = br->mdb;
> + struct net_bridge_mdb_entry *mp;
> + struct net_bridge_port_group *p;
> + struct net_bridge_port_group **pp;
> +
> + mp = br_mdb_ip_get(mdb, pg->addr);
> + if (WARN_ON(!mp))
> + return;
> +
> + for (pp = &mp->ports; (p = *pp); pp = &p->next) {
> + if (p != pg)
> + continue;
> +
> + *pp = p->next;
If I understand this code correctly, we are relying on the RCU-bh grace
period not completing until we process the next element. But we don't
appear to be in an RCU-bh read-side critical section.
I believe that you need an rcu_read_lock_bh() and an rcu_read_unlock_bh()
around this loop or in all callers. @@@
> + hlist_del_init(&p->mglist);
> + del_timer(&p->timer);
> + del_timer(&p->query_timer);
> + call_rcu_bh(&p->rcu, br_multicast_free_pg);
> +
> + if (!mp->ports && hlist_unhashed(&mp->mglist) &&
> + netif_running(br->dev))
> + mod_timer(&mp->timer, jiffies);
> +
> + return;
> + }
> +
> + WARN_ON(1);
> +}
> +
> +static void br_multicast_port_group_expired(unsigned long data)
> +{
> + struct net_bridge_port_group *pg = (void *)data;
> + struct net_bridge *br = pg->port->br;
> +
> + spin_lock(&br->multicast_lock);
> + if (!netif_running(br->dev) || timer_pending(&pg->timer) ||
> + hlist_unhashed(&pg->mglist))
> + goto out;
> +
> + br_multicast_del_pg(br, pg);
> +
> +out:
> + spin_unlock(&br->multicast_lock);
> +}
> +
> +static int br_mdb_rehash(struct net_bridge_mdb_htable **mdbp, int max,
> + int elasticity)
> +{
> + struct net_bridge_mdb_htable *old = *mdbp;
> + struct net_bridge_mdb_htable *mdb;
> + int err;
> +
> + mdb = kmalloc(sizeof(*mdb), GFP_ATOMIC);
> + if (!mdb)
> + return -ENOMEM;
> +
> + mdb->max = max;
> + mdb->old = old;
OK, so the current hash table points to the old one.
The way this is set up, it looks to be illegal for RCU readers to
traverse the ->old pointer, since it is NULLed after but one RCU
grace period.
> + mdb->mhash = kzalloc(max * sizeof(*mdb->mhash), GFP_ATOMIC);
> + if (!mdb->mhash) {
> + kfree(mdb);
> + return -ENOMEM;
> + }
> +
> + mdb->size = old ? old->size : 0;
> + mdb->ver = old ? old->ver ^ 1 : 0;
> +
> + if (!old || elasticity)
> + get_random_bytes(&mdb->secret, sizeof(mdb->secret));
> + else
> + mdb->secret = old->secret;
> +
> + if (!old)
> + goto out;
> +
> + err = br_mdb_copy(mdb, old, elasticity);
> + if (err) {
> + kfree(mdb->mhash);
> + kfree(mdb);
> + return err;
> + }
> +
> + call_rcu_bh(&mdb->rcu, br_mdb_free);
And we are using RCU-bh. OK.
> +
> +out:
> + rcu_assign_pointer(*mdbp, mdb);
Also protected by br->multicast_lock.
> +
> + return 0;
> +}
> +
> +static struct sk_buff *br_multicast_alloc_query(struct net_bridge *br,
> + __be32 group)
> +{
> + struct sk_buff *skb;
> + struct igmphdr *ih;
> + struct ethhdr *eth;
> + struct iphdr *iph;
> +
> + skb = netdev_alloc_skb_ip_align(br->dev, sizeof(*eth) + sizeof(*iph) +
> + sizeof(*ih) + 4);
> + if (!skb)
> + goto out;
> +
> + skb->protocol = htons(ETH_P_IP);
> +
> + skb_reset_mac_header(skb);
> + eth = eth_hdr(skb);
> +
> + memcpy(eth->h_source, br->dev->dev_addr, 6);
> + eth->h_dest[0] = 1;
> + eth->h_dest[1] = 0;
> + eth->h_dest[2] = 0x5e;
> + eth->h_dest[3] = 0;
> + eth->h_dest[4] = 0;
> + eth->h_dest[5] = 1;
> + eth->h_proto = htons(ETH_P_IP);
> + skb_put(skb, sizeof(*eth));
> +
> + skb_set_network_header(skb, skb->len);
> + iph = ip_hdr(skb);
> +
> + iph->version = 4;
> + iph->ihl = 6;
> + iph->tos = 0xc0;
> + iph->tot_len = htons(sizeof(*iph) + sizeof(*ih) + 4);
> + iph->id = 0;
> + iph->frag_off = htons(IP_DF);
> + iph->ttl = 1;
> + iph->protocol = IPPROTO_IGMP;
> + iph->saddr = 0;
> + iph->daddr = htonl(INADDR_ALLHOSTS_GROUP);
> + ((u8 *)&iph[1])[0] = IPOPT_RA;
> + ((u8 *)&iph[1])[1] = 4;
> + ((u8 *)&iph[1])[2] = 0;
> + ((u8 *)&iph[1])[3] = 0;
> + ip_send_check(iph);
> + skb_put(skb, 24);
> +
> + skb_set_transport_header(skb, skb->len);
> + ih = igmp_hdr(skb);
> + ih->type = IGMP_HOST_MEMBERSHIP_QUERY;
> + ih->code = (group ? br->multicast_last_member_interval :
> + br->multicast_query_response_interval) /
> + (HZ / IGMP_TIMER_SCALE);
> + ih->group = group;
> + ih->csum = 0;
> + ih->csum = ip_compute_csum((void *)ih, sizeof(struct igmphdr));
> + skb_put(skb, sizeof(*ih));
> +
> + __skb_pull(skb, sizeof(*eth));
> +
> +out:
> + return skb;
> +}
> +
> +static void br_multicast_send_group_query(struct net_bridge_mdb_entry *mp)
> +{
> + struct net_bridge *br = mp->br;
> + struct sk_buff *skb;
> +
> + skb = br_multicast_alloc_query(br, mp->addr);
> + if (!skb)
> + goto timer;
> +
> + netif_rx(skb);
> +
> +timer:
> + if (++mp->queries_sent < br->multicast_last_member_count)
> + mod_timer(&mp->query_timer,
> + jiffies + br->multicast_last_member_interval);
> +}
> +
> +static void br_multicast_group_query_expired(unsigned long data)
> +{
> + struct net_bridge_mdb_entry *mp = (void *)data;
> + struct net_bridge *br = mp->br;
> +
> + spin_lock(&br->multicast_lock);
> + if (!netif_running(br->dev) || hlist_unhashed(&mp->mglist) ||
> + mp->queries_sent >= br->multicast_last_member_count)
> + goto out;
> +
> + br_multicast_send_group_query(mp);
> +
> +out:
> + spin_unlock(&br->multicast_lock);
> +}
> +
> +static void br_multicast_send_port_group_query(struct net_bridge_port_group *pg)
> +{
> + struct net_bridge_port *port = pg->port;
> + struct net_bridge *br = port->br;
> + struct sk_buff *skb;
> +
> + skb = br_multicast_alloc_query(br, pg->addr);
> + if (!skb)
> + goto timer;
> +
> + br_deliver(port, skb);
> +
> +timer:
> + if (++pg->queries_sent < br->multicast_last_member_count)
> + mod_timer(&pg->query_timer,
> + jiffies + br->multicast_last_member_interval);
> +}
> +
> +static void br_multicast_port_group_query_expired(unsigned long data)
> +{
> + struct net_bridge_port_group *pg = (void *)data;
> + struct net_bridge_port *port = pg->port;
> + struct net_bridge *br = port->br;
> +
> + spin_lock(&br->multicast_lock);
> + if (!netif_running(br->dev) || hlist_unhashed(&pg->mglist) ||
> + pg->queries_sent >= br->multicast_last_member_count)
> + goto out;
> +
> + br_multicast_send_port_group_query(pg);
> +
> +out:
> + spin_unlock(&br->multicast_lock);
> +}
> +
> +static struct net_bridge_mdb_entry *br_multicast_get_group(
> + struct net_bridge *br, struct net_bridge_port *port, __be32 group,
> + int hash)
> +{
> + struct net_bridge_mdb_htable *mdb = br->mdb;
> + struct net_bridge_mdb_entry *mp;
> + struct hlist_node *p;
> + unsigned count = 0;
> + unsigned max;
> + int elasticity;
> + int err;
> +
> + hlist_for_each_entry(mp, p, &mdb->mhash[hash], hlist[mdb->ver]) {
> + count++;
> + if (unlikely(group == mp->addr)) {
> + return mp;
> + }
> + }
> +
> + elasticity = 0;
> + max = mdb->max;
> +
> + if (unlikely(count > br->hash_elasticity && count)) {
> + if (net_ratelimit())
> + printk(KERN_INFO "%s: Multicast hash table "
> + "chain limit reached: %s\n",
> + br->dev->name, port ? port->dev->name :
> + br->dev->name);
> +
> + elasticity = br->hash_elasticity;
> + }
> +
> + if (mdb->size >= max) {
> + max *= 2;
> + if (unlikely(max >= br->hash_max)) {
> + printk(KERN_WARNING "%s: Multicast hash table maximum "
> + "reached, disabling snooping: %s, %d\n",
> + br->dev->name, port ? port->dev->name :
> + br->dev->name,
> + max);
> + err = -E2BIG;
> +disable:
> + br->multicast_disabled = 1;
> + goto err;
> + }
> + }
> +
> + if (max > mdb->max || elasticity) {
> + if (mdb->old) {
And here is the ->old check, as required.
> + if (net_ratelimit())
> + printk(KERN_INFO "%s: Multicast hash table "
> + "on fire: %s\n",
> + br->dev->name, port ? port->dev->name :
> + br->dev->name);
> + err = -EEXIST;
> + goto err;
> + }
> +
> + err = br_mdb_rehash(&br->mdb, max, elasticity);
> + if (err) {
> + printk(KERN_WARNING "%s: Cannot rehash multicast "
> + "hash table, disabling snooping: "
> + "%s, %d, %d\n",
> + br->dev->name, port ? port->dev->name :
> + br->dev->name,
> + mdb->size, err);
> + goto disable;
> + }
> +
> + err = -EAGAIN;
> + goto err;
> + }
> +
> + return NULL;
> +
> +err:
> + mp = ERR_PTR(err);
> + return mp;
> +}
> +
> +static struct net_bridge_mdb_entry *br_multicast_new_group(
> + struct net_bridge *br, struct net_bridge_port *port, __be32 group)
> +{
> + struct net_bridge_mdb_htable *mdb = br->mdb;
> + struct net_bridge_mdb_entry *mp;
> + int hash;
> +
> + if (!mdb) {
> + if (br_mdb_rehash(&br->mdb, BR_HASH_SIZE, 0))
> + return NULL;
> + goto rehash;
> + }
> +
> + hash = br_ip_hash(mdb, group);
> + mp = br_multicast_get_group(br, port, group, hash);
> + switch (PTR_ERR(mp)) {
> + case 0:
> + break;
> +
> + case -EAGAIN:
> +rehash:
> + mdb = br->mdb;
> + hash = br_ip_hash(mdb, group);
> + break;
> +
> + default:
> + goto out;
> + }
> +
> + mp = kzalloc(sizeof(*mp), GFP_ATOMIC);
> + if (unlikely(!mp))
> + goto out;
> +
> + mp->br = br;
> + mp->addr = group;
> + setup_timer(&mp->timer, br_multicast_group_expired,
> + (unsigned long)mp);
> + setup_timer(&mp->query_timer, br_multicast_group_query_expired,
> + (unsigned long)mp);
> +
> + hlist_add_head_rcu(&mp->hlist[mdb->ver], &mdb->mhash[hash]);
Protected by br->multicast_lock, so OK.
> + mdb->size++;
> +
> +out:
> + return mp;
> +}
> +
> +static int br_multicast_add_group(struct net_bridge *br,
> + struct net_bridge_port *port, __be32 group)
> +{
> + struct net_bridge_mdb_entry *mp;
> + struct net_bridge_port_group *p;
> + struct net_bridge_port_group **pp;
> + unsigned long now = jiffies;
> + int err;
> +
> + if (ipv4_is_local_multicast(group))
> + return 0;
> +
> + spin_lock(&br->multicast_lock);
> + if (!netif_running(br->dev) ||
> + (port && port->state == BR_STATE_DISABLED))
> + goto out;
> +
> + mp = br_multicast_new_group(br, port, group);
> + err = PTR_ERR(mp);
> + if (unlikely(IS_ERR(mp) || !mp))
> + goto err;
> +
> + if (!port) {
> + hlist_add_head(&mp->mglist, &br->mglist);
> + mod_timer(&mp->timer, now + br->multicast_membership_interval);
> + goto out;
> + }
> +
> + for (pp = &mp->ports; (p = *pp); pp = &p->next) {
> + if (p->port == port)
> + goto found;
> + if ((unsigned long)p->port < (unsigned long)port)
> + break;
> + }
> +
> + p = kzalloc(sizeof(*p), GFP_ATOMIC);
> + err = -ENOMEM;
> + if (unlikely(!p))
> + goto err;
> +
> + p->addr = group;
> + p->port = port;
> + p->next = *pp;
> + hlist_add_head(&p->mglist, &port->mglist);
> + setup_timer(&p->timer, br_multicast_port_group_expired,
> + (unsigned long)p);
> + setup_timer(&p->query_timer, br_multicast_port_group_query_expired,
> + (unsigned long)p);
> +
> + rcu_assign_pointer(*pp, p);
Also protected by br->multicast_lock.
> +
> +found:
> + mod_timer(&p->timer, now + br->multicast_membership_interval);
> +out:
> + err = 0;
> +
> +err:
> + spin_unlock(&br->multicast_lock);
> + return err;
> +}
> +
> +static void br_multicast_router_expired(unsigned long data)
> +{
> + struct net_bridge_port *port = (void *)data;
> + struct net_bridge *br = port->br;
> +
> + spin_lock(&br->multicast_lock);
> + if (port->multicast_router != 1 ||
> + timer_pending(&port->multicast_router_timer) ||
> + hlist_unhashed(&port->rlist))
> + goto out;
> +
> + hlist_del_init_rcu(&port->rlist);
Also protected by br->multicast_lock.
> +out:
> + spin_unlock(&br->multicast_lock);
> +}
> +
> +static void br_multicast_local_router_expired(unsigned long data)
> +{
> +}
> +
> +static void br_multicast_send_query(struct net_bridge *br,
> + struct net_bridge_port *port, u32 sent)
> +{
> + unsigned long time;
> + struct sk_buff *skb;
> +
> + if (!netif_running(br->dev) || br->multicast_disabled ||
> + timer_pending(&br->multicast_querier_timer))
> + return;
> +
> + skb = br_multicast_alloc_query(br, 0);
> + if (!skb)
> + goto timer;
> +
> + if (port) {
> + __skb_push(skb, sizeof(struct ethhdr));
> + skb->dev = port->dev;
> + NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_OUT, skb, NULL, skb->dev,
> + dev_queue_xmit);
> + } else
> + netif_rx(skb);
> +
> +timer:
> + time = jiffies;
> + time += sent < br->multicast_startup_query_count ?
> + br->multicast_startup_query_interval :
> + br->multicast_query_interval;
> + mod_timer(port ? &port->multicast_query_timer :
> + &br->multicast_query_timer, time);
> +}
> +
> +static void br_multicast_port_query_expired(unsigned long data)
> +{
> + struct net_bridge_port *port = (void *)data;
> + struct net_bridge *br = port->br;
> +
> + spin_lock(&br->multicast_lock);
> + if (port && (port->state == BR_STATE_DISABLED ||
> + port->state == BR_STATE_BLOCKING))
> + goto out;
> +
> + if (port->multicast_startup_queries_sent <
> + br->multicast_startup_query_count)
> + port->multicast_startup_queries_sent++;
> +
> + br_multicast_send_query(port->br, port,
> + port->multicast_startup_queries_sent);
> +
> +out:
> + spin_unlock(&br->multicast_lock);
> +}
> +
> +void br_multicast_add_port(struct net_bridge_port *port)
> +{
> + port->multicast_router = 1;
> +
> + setup_timer(&port->multicast_router_timer, br_multicast_router_expired,
> + (unsigned long)port);
> + setup_timer(&port->multicast_query_timer,
> + br_multicast_port_query_expired, (unsigned long)port);
> +}
> +
> +void br_multicast_del_port(struct net_bridge_port *port)
> +{
> + del_timer_sync(&port->multicast_router_timer);
> +}
> +
> +void br_multicast_enable_port(struct net_bridge_port *port)
> +{
> + struct net_bridge *br = port->br;
> +
> + spin_lock(&br->multicast_lock);
> + if (br->multicast_disabled || !netif_running(br->dev))
> + goto out;
> +
> + port->multicast_startup_queries_sent = 0;
> +
> + if (try_to_del_timer_sync(&port->multicast_query_timer) >= 0 ||
> + del_timer(&port->multicast_query_timer))
> + mod_timer(&port->multicast_query_timer, jiffies);
> +
> +out:
> + spin_unlock(&br->multicast_lock);
> +}
> +
> +void br_multicast_disable_port(struct net_bridge_port *port)
> +{
> + struct net_bridge *br = port->br;
> + struct net_bridge_port_group *pg;
> + struct hlist_node *p, *n;
> +
> + spin_lock(&br->multicast_lock);
> + hlist_for_each_entry_safe(pg, p, n, &port->mglist, mglist)
> + br_multicast_del_pg(br, pg);
> +
> + if (!hlist_unhashed(&port->rlist))
> + hlist_del_init_rcu(&port->rlist);
Also protected by br->multicast_lock.
> + del_timer(&port->multicast_router_timer);
> + del_timer(&port->multicast_query_timer);
> + spin_unlock(&br->multicast_lock);
> +}
> +
> +static int br_multicast_igmp3_report(struct net_bridge *br,
> + struct net_bridge_port *port,
> + struct sk_buff *skb)
> +{
> + struct igmpv3_report *ih;
> + struct igmpv3_grec *grec;
> + int i;
> + int len;
> + int num;
> + int type;
> + int err = 0;
> + __be32 group;
> +
> + if (!pskb_may_pull(skb, sizeof(*ih)))
> + return -EINVAL;
> +
> + ih = igmpv3_report_hdr(skb);
> + num = ntohs(ih->ngrec);
> + len = sizeof(*ih);
> +
> + for (i = 0; i < num; i++) {
> + len += sizeof(*grec);
> + if (!pskb_may_pull(skb, len))
> + return -EINVAL;
> +
> + grec = (void *)(skb->data + len);
> + group = grec->grec_mca;
> + type = grec->grec_type;
> +
> + len += grec->grec_nsrcs * 4;
> + if (!pskb_may_pull(skb, len))
> + return -EINVAL;
> +
> + /* We treat this as an IGMPv2 report for now. */
> + switch (type) {
> + case IGMPV3_MODE_IS_INCLUDE:
> + case IGMPV3_MODE_IS_EXCLUDE:
> + case IGMPV3_CHANGE_TO_INCLUDE:
> + case IGMPV3_CHANGE_TO_EXCLUDE:
> + case IGMPV3_ALLOW_NEW_SOURCES:
> + case IGMPV3_BLOCK_OLD_SOURCES:
> + break;
> +
> + default:
> + continue;
> + }
> +
> + err = br_multicast_add_group(br, port, group);
> + if (err)
> + break;
> + }
> +
> + return err;
> +}
> +
> +static void br_multicast_mark_router(struct net_bridge *br,
> + struct net_bridge_port *port)
> +{
> + unsigned long now = jiffies;
> + struct hlist_node *p;
> + struct hlist_node **h;
> +
> + if (!port) {
> + if (br->multicast_router == 1)
> + mod_timer(&br->multicast_router_timer,
> + now + br->multicast_querier_interval);
> + return;
> + }
> +
> + if (port->multicast_router != 1)
> + return;
> +
> + if (!hlist_unhashed(&port->rlist))
> + goto timer;
> +
> + for (h = &br->router_list.first;
> + (p = *h) &&
> + (unsigned long)container_of(p, struct net_bridge_port, rlist) >
> + (unsigned long)port;
> + h = &p->next)
> + ;
> +
> + port->rlist.pprev = h;
> + port->rlist.next = p;
> + rcu_assign_pointer(*h, &port->rlist);
Also protected by br->multicast_lock.
> + if (p)
> + p->pprev = &port->rlist.next;
> +
> +timer:
> + mod_timer(&port->multicast_router_timer,
> + now + br->multicast_querier_interval);
> +}
> +
> +static void br_multicast_query_received(struct net_bridge *br,
> + struct net_bridge_port *port,
> + __be32 saddr)
> +{
> + if (saddr)
> + mod_timer(&br->multicast_querier_timer,
> + jiffies + br->multicast_querier_interval);
> + else if (timer_pending(&br->multicast_querier_timer))
> + return;
> +
> + br_multicast_mark_router(br, port);
> +}
> +
> +static int br_multicast_query(struct net_bridge *br,
> + struct net_bridge_port *port,
> + struct sk_buff *skb)
> +{
> + struct iphdr *iph = ip_hdr(skb);
> + struct igmphdr *ih = igmp_hdr(skb);
> + struct net_bridge_mdb_entry *mp;
> + struct igmpv3_query *ih3;
> + struct net_bridge_port_group *p;
> + struct net_bridge_port_group **pp;
> + unsigned long max_delay;
> + unsigned long now = jiffies;
> + __be32 group;
> +
> + spin_lock(&br->multicast_lock);
> + if (!netif_running(br->dev) ||
> + (port && port->state == BR_STATE_DISABLED))
> + goto out;
> +
> + br_multicast_query_received(br, port, iph->saddr);
> +
> + group = ih->group;
> +
> + if (skb->len == sizeof(*ih)) {
> + max_delay = ih->code * (HZ / IGMP_TIMER_SCALE);
> +
> + if (!max_delay) {
> + max_delay = 10 * HZ;
> + group = 0;
> + }
> + } else {
> + if (!pskb_may_pull(skb, sizeof(struct igmpv3_query)))
> + return -EINVAL;
> +
> + ih3 = igmpv3_query_hdr(skb);
> + if (ih3->nsrcs)
> + return 0;
> +
> + max_delay = ih3->code ? 1 :
> + IGMPV3_MRC(ih3->code) * (HZ / IGMP_TIMER_SCALE);
> + }
> +
> + if (!group)
> + goto out;
> +
> + mp = br_mdb_ip_get(br->mdb, group);
> + if (!mp)
> + goto out;
> +
> + max_delay *= br->multicast_last_member_count;
> +
> + if (!hlist_unhashed(&mp->mglist) &&
> + (timer_pending(&mp->timer) ?
> + time_after(mp->timer.expires, now + max_delay) :
> + try_to_del_timer_sync(&mp->timer) >= 0))
> + mod_timer(&mp->timer, now + max_delay);
> +
> + for (pp = &mp->ports; (p = *pp); pp = &p->next) {
> + if (timer_pending(&p->timer) ?
> + time_after(p->timer.expires, now + max_delay) :
> + try_to_del_timer_sync(&p->timer) >= 0)
> + mod_timer(&mp->timer, now + max_delay);
> + }
> +
> +out:
> + spin_unlock(&br->multicast_lock);
> + return 0;
> +}
> +
> +static void br_multicast_leave_group(struct net_bridge *br,
> + struct net_bridge_port *port,
> + __be32 group)
> +{
> + struct net_bridge_mdb_htable *mdb;
> + struct net_bridge_mdb_entry *mp;
> + struct net_bridge_port_group *p;
> + unsigned long now;
> + unsigned long time;
> +
> + if (ipv4_is_local_multicast(group))
> + return;
> +
> + spin_lock(&br->multicast_lock);
> + if (!netif_running(br->dev) ||
> + (port && port->state == BR_STATE_DISABLED) ||
> + timer_pending(&br->multicast_querier_timer))
> + goto out;
> +
> + mdb = br->mdb;
> + mp = br_mdb_ip_get(mdb, group);
> + if (!mp)
> + goto out;
> +
> + now = jiffies;
> + time = now + br->multicast_last_member_count *
> + br->multicast_last_member_interval;
> +
> + if (!port) {
> + if (!hlist_unhashed(&mp->mglist) &&
> + (timer_pending(&mp->timer) ?
> + time_after(mp->timer.expires, time) :
> + try_to_del_timer_sync(&mp->timer) >= 0)) {
> + mod_timer(&mp->timer, time);
> +
> + mp->queries_sent = 0;
> + mod_timer(&mp->query_timer, now);
> + }
> +
> + goto out;
> + }
> +
> + for (p = mp->ports; p; p = p->next) {
> + if (p->port != port)
> + continue;
> +
> + if (!hlist_unhashed(&p->mglist) &&
> + (timer_pending(&p->timer) ?
> + time_after(p->timer.expires, time) :
> + try_to_del_timer_sync(&p->timer) >= 0)) {
> + mod_timer(&p->timer, time);
> +
> + p->queries_sent = 0;
> + mod_timer(&p->query_timer, now);
> + }
> +
> + break;
> + }
> +
> +out:
> + spin_unlock(&br->multicast_lock);
> +}
> +
> +static int br_multicast_ipv4_rcv(struct net_bridge *br,
> + struct net_bridge_port *port,
> + struct sk_buff *skb)
> +{
> + struct sk_buff *skb2 = skb;
> + struct iphdr *iph;
> + struct igmphdr *ih;
> + unsigned len;
> + unsigned offset;
> + int err;
> +
> + BR_INPUT_SKB_CB(skb)->igmp = 0;
> + BR_INPUT_SKB_CB(skb)->mrouters_only = 0;
> +
> + /* We treat OOM as packet loss for now. */
> + if (!pskb_may_pull(skb, sizeof(*iph)))
> + return -EINVAL;
> +
> + iph = ip_hdr(skb);
> +
> + if (iph->ihl < 5 || iph->version != 4)
> + return -EINVAL;
> +
> + if (!pskb_may_pull(skb, ip_hdrlen(skb)))
> + return -EINVAL;
> +
> + iph = ip_hdr(skb);
> +
> + if (unlikely(ip_fast_csum((u8 *)iph, iph->ihl)))
> + return -EINVAL;
> +
> + if (iph->protocol != IPPROTO_IGMP)
> + return 0;
> +
> + len = ntohs(iph->tot_len);
> + if (skb->len < len || len < ip_hdrlen(skb))
> + return -EINVAL;
> +
> + if (skb->len > len) {
> + skb2 = skb_clone(skb, GFP_ATOMIC);
> + if (!skb2)
> + return -ENOMEM;
> +
> + err = pskb_trim_rcsum(skb2, len);
> + if (err)
> + return err;
> + }
> +
> + len -= ip_hdrlen(skb2);
> + offset = skb_network_offset(skb2) + ip_hdrlen(skb2);
> + __skb_pull(skb2, offset);
> + skb_reset_transport_header(skb2);
> +
> + err = -EINVAL;
> + if (!pskb_may_pull(skb2, sizeof(*ih)))
> + goto out;
> +
> + iph = ip_hdr(skb2);
> +
> + switch (skb2->ip_summed) {
> + case CHECKSUM_COMPLETE:
> + if (!csum_fold(skb2->csum))
> + break;
> + /* fall through */
> + case CHECKSUM_NONE:
> + skb2->csum = 0;
> + if (skb_checksum_complete(skb2))
> + return -EINVAL;
> + }
> +
> + err = 0;
> +
> + BR_INPUT_SKB_CB(skb)->igmp = 1;
> + ih = igmp_hdr(skb2);
> +
> + switch (ih->type) {
> + case IGMP_HOST_MEMBERSHIP_REPORT:
> + case IGMPV2_HOST_MEMBERSHIP_REPORT:
> + BR_INPUT_SKB_CB(skb2)->mrouters_only = 1;
> + err = br_multicast_add_group(br, port, ih->group);
> + break;
> + case IGMPV3_HOST_MEMBERSHIP_REPORT:
> + err = br_multicast_igmp3_report(br, port, skb2);
> + break;
> + case IGMP_HOST_MEMBERSHIP_QUERY:
> + err = br_multicast_query(br, port, skb2);
> + break;
> + case IGMP_HOST_LEAVE_MESSAGE:
> + br_multicast_leave_group(br, port, ih->group);
> + break;
> + }
> +
> +out:
> + __skb_push(skb2, offset);
> + if (skb2 != skb)
> + kfree_skb(skb2);
> + return err;
> +}
> +
> +int br_multicast_rcv(struct net_bridge *br, struct net_bridge_port *port,
> + struct sk_buff *skb)
> +{
> + if (br->multicast_disabled)
> + return 0;
> +
> + switch (skb->protocol) {
> + case htons(ETH_P_IP):
> + return br_multicast_ipv4_rcv(br, port, skb);
> + }
> +
> + return 0;
> +}
> +
> +static void br_multicast_query_expired(unsigned long data)
> +{
> + struct net_bridge *br = (void *)data;
> +
> + spin_lock(&br->multicast_lock);
> + if (br->multicast_startup_queries_sent <
> + br->multicast_startup_query_count)
> + br->multicast_startup_queries_sent++;
> +
> + br_multicast_send_query(br, NULL, br->multicast_startup_queries_sent);
> +
> + spin_unlock(&br->multicast_lock);
> +}
> +
> +void br_multicast_init(struct net_bridge *br)
> +{
> + br->hash_elasticity = 4;
> + br->hash_max = 512;
> +
> + br->multicast_router = 1;
> + br->multicast_last_member_count = 2;
> + br->multicast_startup_query_count = 2;
> +
> + br->multicast_last_member_interval = HZ;
> + br->multicast_query_response_interval = 10 * HZ;
> + br->multicast_startup_query_interval = 125 * HZ / 4;
> + br->multicast_query_interval = 125 * HZ;
> + br->multicast_querier_interval = 255 * HZ;
> + br->multicast_membership_interval = 260 * HZ;
> +
> + spin_lock_init(&br->multicast_lock);
> + setup_timer(&br->multicast_router_timer,
> + br_multicast_local_router_expired, 0);
> + setup_timer(&br->multicast_querier_timer,
> + br_multicast_local_router_expired, 0);
> + setup_timer(&br->multicast_query_timer, br_multicast_query_expired,
> + (unsigned long)br);
> +}
> +
> +void br_multicast_open(struct net_bridge *br)
> +{
> + br->multicast_startup_queries_sent = 0;
> +
> + if (br->multicast_disabled)
> + return;
> +
> + mod_timer(&br->multicast_query_timer, jiffies);
> +}
> +
> +void br_multicast_stop(struct net_bridge *br)
> +{
> + struct net_bridge_mdb_htable *mdb;
> + struct net_bridge_mdb_entry *mp;
> + struct hlist_node *p, *n;
> + u32 ver;
> + int i;
> +
> + del_timer_sync(&br->multicast_router_timer);
> + del_timer_sync(&br->multicast_querier_timer);
> + del_timer_sync(&br->multicast_query_timer);
> +
> + spin_lock_bh(&br->multicast_lock);
> + mdb = br->mdb;
> + if (!mdb)
> + goto out;
> +
> + br->mdb = NULL;
> +
> + ver = mdb->ver;
> + for (i = 0; i < mdb->max; i++) {
> + hlist_for_each_entry_safe(mp, p, n, &mdb->mhash[i],
> + hlist[ver]) {
> + del_timer(&mp->timer);
> + del_timer(&mp->query_timer);
> + call_rcu_bh(&mp->rcu, br_multicast_free_group);
I don't see what prevents new readers from finding the net_bridge_mdb_entry
referenced by mp. Unless you stop new readers, an RCU grace period
cannot help you! @@@
> + }
> + }
> +
> + if (mdb->old) {
And another ->old check for teardown purposes?
> + spin_unlock_bh(&br->multicast_lock);
> + synchronize_rcu_bh();
Ah, but the above only guarantees that the grace period has elapsed,
not that all of the callbacks have been invoked, so...
> + spin_lock_bh(&br->multicast_lock);
> + WARN_ON(mdb->old);
This WARN_ON() can fire!!!
Suggested fix: s/synchronize_rcu_bh()/rcu_barrier_bh()/ @@@
> + }
> +
> + mdb->old = mdb;
I can't say I understand how making mdb->old point back at mdb helps...
Ah, clever -- this arrangement causes the RCU callback to free up the
whole mess. ;-)
> + call_rcu_bh(&mdb->rcu, br_mdb_free);
I don't see prevents new readers from finding the net_bridge_mdb_htable
referenced by mdb. Again, if new readers are permitted to reference
this structure, an RCU grace period won't help you -- you cannot start
the RCU grace period until after all new readers have been excluded.
> +
> +out:
> + spin_unlock_bh(&br->multicast_lock);
> +}
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 7b0aed5..0871775 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -57,6 +57,41 @@ struct net_bridge_fdb_entry
> unsigned char is_static;
> };
>
> +struct net_bridge_port_group {
> + struct net_bridge_port *port;
> + struct net_bridge_port_group *next;
> + struct hlist_node mglist;
> + struct rcu_head rcu;
> + struct timer_list timer;
> + struct timer_list query_timer;
> + __be32 addr;
> + u32 queries_sent;
> +};
> +
> +struct net_bridge_mdb_entry
> +{
> + struct hlist_node hlist[2];
> + struct hlist_node mglist;
> + struct net_bridge *br;
> + struct net_bridge_port_group *ports;
> + struct rcu_head rcu;
> + struct timer_list timer;
> + struct timer_list query_timer;
> + __be32 addr;
> + u32 queries_sent;
> +};
> +
> +struct net_bridge_mdb_htable
> +{
> + struct hlist_head *mhash;
> + struct rcu_head rcu;
> + struct net_bridge_mdb_htable *old;
> + u32 size;
> + u32 max;
> + u32 secret;
> + u32 ver;
> +};
> +
> struct net_bridge_port
> {
> struct net_bridge *br;
> @@ -84,6 +119,15 @@ struct net_bridge_port
>
> unsigned long flags;
> #define BR_HAIRPIN_MODE 0x00000001
> +
> +#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
> + u32 multicast_startup_queries_sent;
> + unsigned char multicast_router;
> + struct timer_list multicast_router_timer;
> + struct timer_list multicast_query_timer;
> + struct hlist_head mglist;
> + struct hlist_node rlist;
> +#endif
> };
>
> struct net_bridge
> @@ -125,6 +169,35 @@ struct net_bridge
> unsigned char topology_change;
> unsigned char topology_change_detected;
>
> +#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
> + unsigned char multicast_router;
> +
> + u8 multicast_disabled:1;
> +
> + u32 hash_elasticity;
> + u32 hash_max;
> +
> + u32 multicast_last_member_count;
> + u32 multicast_startup_queries_sent;
> + u32 multicast_startup_query_count;
> +
> + unsigned long multicast_last_member_interval;
> + unsigned long multicast_membership_interval;
> + unsigned long multicast_querier_interval;
> + unsigned long multicast_query_interval;
> + unsigned long multicast_query_response_interval;
> + unsigned long multicast_startup_query_interval;
> +
> + spinlock_t multicast_lock;
> + struct net_bridge_mdb_htable *mdb;
> + struct hlist_head router_list;
> + struct hlist_head mglist;
> +
> + struct timer_list multicast_router_timer;
> + struct timer_list multicast_querier_timer;
> + struct timer_list multicast_query_timer;
> +#endif
> +
> struct timer_list hello_timer;
> struct timer_list tcn_timer;
> struct timer_list topology_change_timer;
> @@ -134,6 +207,8 @@ struct net_bridge
>
> struct br_input_skb_cb {
> struct net_device *brdev;
> + int igmp;
> + int mrouters_only;
> };
>
> #define BR_INPUT_SKB_CB(__skb) ((struct br_input_skb_cb *)(__skb)->cb)
> @@ -205,6 +280,70 @@ extern struct sk_buff *br_handle_frame(struct net_bridge_port *p,
> extern int br_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
> extern int br_ioctl_deviceless_stub(struct net *net, unsigned int cmd, void __user *arg);
>
> +/* br_multicast.c */
> +#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
> +extern int br_multicast_rcv(struct net_bridge *br,
> + struct net_bridge_port *port,
> + struct sk_buff *skb);
> +extern struct net_bridge_mdb_entry *br_mdb_get(struct net_bridge *br,
> + struct sk_buff *skb);
> +extern void br_multicast_add_port(struct net_bridge_port *port);
> +extern void br_multicast_del_port(struct net_bridge_port *port);
> +extern void br_multicast_enable_port(struct net_bridge_port *port);
> +extern void br_multicast_disable_port(struct net_bridge_port *port);
> +extern void br_multicast_init(struct net_bridge *br);
> +extern void br_multicast_open(struct net_bridge *br);
> +extern void br_multicast_stop(struct net_bridge *br);
> +#else
> +static inline int br_multicast_rcv(struct net_bridge *br,
> + struct net_bridge_port *port,
> + struct sk_buff *skb)
> +{
> + return 0;
> +}
> +
> +static inline struct net_bridge_mdb_entry *br_mdb_get(struct net_bridge *br,
> + struct sk_buff *skb)
> +{
> + return NULL;
> +}
> +
> +static inline void br_multicast_add_port(struct net_bridge_port *port)
> +{
> +}
> +
> +static inline void br_multicast_del_port(struct net_bridge_port *port)
> +{
> +}
> +
> +static inline void br_multicast_enable_port(struct net_bridge_port *port)
> +{
> +}
> +
> +static inline void br_multicast_disable_port(struct net_bridge_port *port)
> +{
> +}
> +
> +static inline void br_multicast_init(struct net_bridge *br)
> +{
> +}
> +
> +static inline void br_multicast_open(struct net_bridge *br)
> +{
> +}
> +
> +static inline void br_multicast_stop(struct net_bridge *br)
> +{
> +}
> +#endif
> +
> +static inline bool br_multicast_is_router(struct net_bridge *br)
> +{
> + return br->multicast_router == 2 ||
> + (br->multicast_router == 1 &&
> + timer_pending(&br->multicast_router_timer));
> +}
> +
> /* br_netfilter.c */
> #ifdef CONFIG_BRIDGE_NETFILTER
> extern int br_netfilter_init(void);
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox