* Re: [Bridge] [PATCH v2] bridge: Fix return values of br_multicast_add_group/br_multicast_new_group
From: Tobias Klauser @ 2010-12-10 22:52 UTC (permalink / raw)
To: David Miller; +Cc: netdev, bridge
In-Reply-To: <20101210.130125.233714141.davem@davemloft.net>
On 2010-12-10 at 22:01:25 +0100, David Miller <davem@davemloft.net> wrote:
> From: Tobias Klauser <tklauser@distanz.ch>
> Date: Fri, 10 Dec 2010 14:18:04 +0100
>
> > If br_multicast_new_group returns NULL, we would return 0 (no error) to
> > the caller of br_multicast_add_group, which is not what we want. Instead
> > br_multicast_new_group should return ERR_PTR(-ENOMEM) in this case.
> > Also propagate the error number returned by br_mdb_rehash properly.
> >
> > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
>
> Looks good, applied to net-next-2.6
>
> Please in the future make is clear, in your subject line, which
> tree this patch is meant for.
>
> I had to figure it out by trial and error, because this patch
> does not apply properly to net-2.6
Sorry about that. I'll do so in the future.
Thanks
^ permalink raw reply
* Re: [Bridge] [PATCH v2] bridge: Fix return values of br_multicast_add_group/br_multicast_new_group
From: David Miller @ 2010-12-10 21:01 UTC (permalink / raw)
To: tklauser; +Cc: netdev, bridge
In-Reply-To: <1291987084-27302-1-git-send-email-tklauser@distanz.ch>
From: Tobias Klauser <tklauser@distanz.ch>
Date: Fri, 10 Dec 2010 14:18:04 +0100
> If br_multicast_new_group returns NULL, we would return 0 (no error) to
> the caller of br_multicast_add_group, which is not what we want. Instead
> br_multicast_new_group should return ERR_PTR(-ENOMEM) in this case.
> Also propagate the error number returned by br_mdb_rehash properly.
>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Looks good, applied to net-next-2.6
Please in the future make is clear, in your subject line, which
tree this patch is meant for.
I had to figure it out by trial and error, because this patch
does not apply properly to net-2.6
^ permalink raw reply
* [Bridge] [PATCH v2] bridge: Fix return values of br_multicast_add_group/br_multicast_new_group
From: Tobias Klauser @ 2010-12-10 13:18 UTC (permalink / raw)
To: Stephen Hemminger, David S. Miller, bridge; +Cc: netdev
In-Reply-To: <20101209082924.6d797871@nehalam>
If br_multicast_new_group returns NULL, we would return 0 (no error) to
the caller of br_multicast_add_group, which is not what we want. Instead
br_multicast_new_group should return ERR_PTR(-ENOMEM) in this case.
Also propagate the error number returned by br_mdb_rehash properly.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
net/bridge/br_multicast.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 326e599..85a0398 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -654,11 +654,13 @@ static struct net_bridge_mdb_entry *br_multicast_new_group(
struct net_bridge_mdb_htable *mdb;
struct net_bridge_mdb_entry *mp;
int hash;
+ int err;
mdb = rcu_dereference_protected(br->mdb, 1);
if (!mdb) {
- if (br_mdb_rehash(&br->mdb, BR_HASH_SIZE, 0))
- return NULL;
+ err = br_mdb_rehash(&br->mdb, BR_HASH_SIZE, 0);
+ if (err)
+ return ERR_PTR(err);
goto rehash;
}
@@ -680,7 +682,7 @@ rehash:
mp = kzalloc(sizeof(*mp), GFP_ATOMIC);
if (unlikely(!mp))
- goto out;
+ return ERR_PTR(-ENOMEM);
mp->br = br;
mp->addr = *group;
@@ -713,7 +715,7 @@ static int br_multicast_add_group(struct net_bridge *br,
mp = br_multicast_new_group(br, port, group);
err = PTR_ERR(mp);
- if (unlikely(IS_ERR(mp) || !mp))
+ if (IS_ERR(mp))
goto err;
if (!port) {
--
1.7.0.4
^ permalink raw reply related
* Re: [Bridge] [PATCH] bridge: Fix return value of br_multicast_add_group()
From: Tobias Klauser @ 2010-12-09 18:53 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, bridge, David S. Miller
In-Reply-To: <20101209082924.6d797871@nehalam>
On 2010-12-09 at 17:29:24 +0100, Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> On Thu, 9 Dec 2010 15:02:36 +0100
> Tobias Klauser <tklauser@distanz.ch> wrote:
>
> > If br_multicast_new_group returns NULL, we would return 0 (no error) to
> > the caller, which is not what we want. Instead we should return -ENOMEM
> > in this case.
> >
> > Also replace IS_ERR(x) || !x by IS_ERR_OR_NULL(x)
> >
> > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > ---
> > net/bridge/br_multicast.c | 5 ++++-
> > 1 files changed, 4 insertions(+), 1 deletions(-)
> >
> > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> > index 326e599..d4e1e81 100644
> > --- a/net/bridge/br_multicast.c
> > +++ b/net/bridge/br_multicast.c
> > @@ -713,8 +713,11 @@ static int br_multicast_add_group(struct net_bridge *br,
> >
> > mp = br_multicast_new_group(br, port, group);
> > err = PTR_ERR(mp);
> > - if (unlikely(IS_ERR(mp) || !mp))
> > + if (IS_ERR_OR_NULL(mp)) {
> > + if (!mp)
> > + err = -ENOMEM;
> > goto err;
> > + }
> >
> > if (!port) {
> > hlist_add_head(&mp->mglist, &br->mglist);
>
> I would rather fix br_multicast_new_group so it never returns
> NULL. Instead return PTR_ERR(-ENOMEM) on out of memory.
Ok, I'll change that and send an updated patch.
Thanks a lot
Tobias
^ permalink raw reply
* Re: [Bridge] ksoftirqd process
From: Stephen Hemminger @ 2010-12-09 16:30 UTC (permalink / raw)
To: Pravin Pardeshi; +Cc: bridge
In-Reply-To: <OFD643E4FF.BA761D6E-ON652577F4.004E3BAB-652577F4.00593751@tcs.com>
On Thu, 9 Dec 2010 21:44:28 +0530
Pravin Pardeshi <pravin.pardeshi@tcs.com> wrote:
> Hi,
> I want to have bridge for two NICs (eth0 and eth1) with Knoppix 6.2 Live CD on virtualbox. I tried the following commands;
>
> # brctl addbr br0
> # brctl sethello 30
> # brctl stp br0 off
>
> # ifconfig eth0 0.0.0.0
> # ifconfig eth1 0.0.0.0
> # ifconfig eth0 down
> # ifconfig eth1 down
>
> # brctl addif br0 eth0
> # brctl addif br0 eth1
>
> After the addition of eth0 and eth1 to the bridge, the ksoftirq gets kicked off and starts consuming CPU. The connection to network is lost. The CPU utilization never stabilizes even after waiting for 10-15 minutes. Knoppix 6.2 has kernel 2.6.32.6 and bridge-utils is the latest version from debian repositories.
>
> Firstly, am I missing something?
> Secondly, is it possible to have a setup I am attempting with the live cd?
>
> Thanks,
> Pravin
You probably created a network loop. And you turned STP off
so it can't be detected.
--
^ permalink raw reply
* Re: [Bridge] [PATCH] bridge: Fix return value of br_multicast_add_group()
From: Stephen Hemminger @ 2010-12-09 16:29 UTC (permalink / raw)
To: Tobias Klauser; +Cc: netdev, bridge, David S. Miller
In-Reply-To: <1291903356-30618-1-git-send-email-tklauser@distanz.ch>
On Thu, 9 Dec 2010 15:02:36 +0100
Tobias Klauser <tklauser@distanz.ch> wrote:
> If br_multicast_new_group returns NULL, we would return 0 (no error) to
> the caller, which is not what we want. Instead we should return -ENOMEM
> in this case.
>
> Also replace IS_ERR(x) || !x by IS_ERR_OR_NULL(x)
>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> ---
> net/bridge/br_multicast.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 326e599..d4e1e81 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -713,8 +713,11 @@ static int br_multicast_add_group(struct net_bridge *br,
>
> mp = br_multicast_new_group(br, port, group);
> err = PTR_ERR(mp);
> - if (unlikely(IS_ERR(mp) || !mp))
> + if (IS_ERR_OR_NULL(mp)) {
> + if (!mp)
> + err = -ENOMEM;
> goto err;
> + }
>
> if (!port) {
> hlist_add_head(&mp->mglist, &br->mglist);
I would rather fix br_multicast_new_group so it never returns
NULL. Instead return PTR_ERR(-ENOMEM) on out of memory.
--
^ permalink raw reply
* [Bridge] ksoftirqd process
From: Pravin Pardeshi @ 2010-12-09 16:14 UTC (permalink / raw)
To: bridge
[-- Attachment #1: Type: text/html, Size: 1785 bytes --]
^ permalink raw reply
* [Bridge] ksoftirqd process
From: Pravin Pardeshi @ 2010-12-09 14:14 UTC (permalink / raw)
To: bridge
[-- Attachment #1: Type: text/html, Size: 1764 bytes --]
^ permalink raw reply
* [Bridge] [PATCH] bridge: Fix return value of br_multicast_add_group()
From: Tobias Klauser @ 2010-12-09 14:02 UTC (permalink / raw)
To: Stephen Hemminger, David S. Miller, bridge; +Cc: netdev
If br_multicast_new_group returns NULL, we would return 0 (no error) to
the caller, which is not what we want. Instead we should return -ENOMEM
in this case.
Also replace IS_ERR(x) || !x by IS_ERR_OR_NULL(x)
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
net/bridge/br_multicast.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 326e599..d4e1e81 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -713,8 +713,11 @@ static int br_multicast_add_group(struct net_bridge *br,
mp = br_multicast_new_group(br, port, group);
err = PTR_ERR(mp);
- if (unlikely(IS_ERR(mp) || !mp))
+ if (IS_ERR_OR_NULL(mp)) {
+ if (!mp)
+ err = -ENOMEM;
goto err;
+ }
if (!port) {
hlist_add_head(&mp->mglist, &br->mglist);
--
1.7.0.4
^ permalink raw reply related
* Re: [Bridge] bridge problem with with ICMP fragment reassembly
From: Nicolas de Pesloüan @ 2010-12-03 9:40 UTC (permalink / raw)
To: Shawn Delaney; +Cc: bridge@lists.linux-foundation.org
In-Reply-To: <DBDEC20E6E29304D8AE5EAA493609B35576467B066@EMBX02-HQ.jnpr.net>
Le 02/12/2010 23:19, Shawn Delaney a écrit :
> Hi,
> I have a bridge (br0) with two enslaved interfaces (int0 and ext0). From
> the ext0 side, with a windows device, I ping with packet size 1932:
> ping –n 1 –l 1932 10.64.160.94
> ping request and reply is bridged.
> Now if I do the same with packet size 1933:
> ping –n 1 –l 1933 10.64.160.94
> the request never seems to make it to the bridge interface
> If I sniff the ping from the windows device on the ext0 side, I see two
> frames sent, request + fragment, as expected.
> In the working case, if I sniff ext0 on the bridge device I see the same
> two frames received.
> In the working case, if I sniff br0 on the bridge I see one frame (I
> assume the bridge reassembles the request + fragment).
> In the non working case, if I sniff ext0 on the bridge device I see the
> same two frames received.
> In the non working case, if I sniff br0 I see nothing.
> It seems there is a bug with bridging and reassembly of packets or
> fragments of a certain size, and am wondering If there is a patch.
Bridges are not expected to change frames in any way. This is not specific to the bridging function
of Linux.
No reassembly occurs at the bridge level. If one host send two frames, the bridge will forward two
frames.
By the way, it means that the MTU for all interfaces of the bridge should be identical.
Nicolas.
> The MTU of all interfaces is 1500.
> I am using connection tracking.
> The problem is seen on kernels 2.6.11 and 2.6.24
> Any help would be much appreciated.
> Regards,
> Shawn
>
>
>
> _______________________________________________
> Bridge mailing list
> Bridge@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/bridge
^ permalink raw reply
* [Bridge] bridge problem with with ICMP fragment reassembly
From: Shawn Delaney @ 2010-12-02 22:19 UTC (permalink / raw)
To: bridge@lists.linux-foundation.org
[-- Attachment #1: Type: text/plain, Size: 1188 bytes --]
Hi,
I have a bridge (br0) with two enslaved interfaces (int0 and ext0). From the ext0 side, with a windows device, I ping with packet size 1932:
ping -n 1 -l 1932 10.64.160.94
ping request and reply is bridged.
Now if I do the same with packet size 1933:
ping -n 1 -l 1933 10.64.160.94
the request never seems to make it to the bridge interface
If I sniff the ping from the windows device on the ext0 side, I see two frames sent, request + fragment, as expected.
In the working case, if I sniff ext0 on the bridge device I see the same two frames received.
In the working case, if I sniff br0 on the bridge I see one frame (I assume the bridge reassembles the request + fragment).
In the non working case, if I sniff ext0 on the bridge device I see the same two frames received.
In the non working case, if I sniff br0 I see nothing.
It seems there is a bug with bridging and reassembly of packets or fragments of a certain size, and am wondering If there is a patch.
The MTU of all interfaces is 1500.
I am using connection tracking.
The problem is seen on kernels 2.6.11 and 2.6.24
Any help would be much appreciated.
Regards,
Shawn
[-- Attachment #2: Type: text/html, Size: 2089 bytes --]
^ permalink raw reply
* [Bridge] 802.1ah (PBB) support in linux
From: Marek Kierdelewicz @ 2010-12-02 11:25 UTC (permalink / raw)
To: bridge, netfilter
Hi,
Did anyone consider implementing IEEE 802.1ah (PBB or MAC-in-MAC)
support in linux? I've googled and found no indication of any work
or even discussion on the subject.
Linux itself, Vyatta, Mikrotik or any linux-based network
appliance would greatly benefit from this new functionality.
Is there anyone who'd like to contribute to the project if it came
to be?
Best regards,
Marek Kierdelewicz
^ permalink raw reply
* [Bridge] Ethernet Bridge with VLAN TAG
From: shailesh panchal @ 2010-12-02 5:29 UTC (permalink / raw)
To: bridge; +Cc: ronak.shah, shailesh.panchal
[-- Attachment #1.1: Type: text/plain, Size: 1559 bytes --]
Requirement: Implement the bridge mode with VLAN TAG in 5VT EVM board
Description:
5VT EVM board has two ethernet port eth0 and eth1. We want to create the
Ethernet bridge using the eth0 and eth1 port.
We find bridge util on the 5VT EVM board name is brctl.
Please see the picture file for more network setup.
In the network setup we have one swich three PC and 5VT EVM board.
Eth0 of 5VT is connected with PC1
Eth1 of 5VT is connected with switch
PC2 and PC3 is connected with switch
VLAN Configuration
Eth0 of 5VT has VLAN 5
Eth1 of 5VT has VLAN 7
PC2 has the VLAN 5
PC3 has the VLAN 7
PC1 has no VLAN
1) Now, PC1 want to ping the PC2.
PC1 generated packet first go to Eth0 of 5VT, Now Eth0 Add the VLAN 5 in
packet and then forward to Eth1 of 5VT because 5VT is working in bridge mode
Now Eth1 of 5VT send the packet to switch and switch forward the packet ot
PC2 and PC3, Now PC2 accept the packet and reply it.
Simlarly when PC2 ping PC1 it get the reply of it.
2) 5VT want to ping PC3
5VT generated packet first go to Eth1, Now Eth1 Add the VLAN 7 in packet
and send the packet to switch and switch forward the packet ot PC2 and
PC3,then PC3 accept the packet and reply it.
Simlarly when PC3 ping 5VT it get the reply of it.
3) PC2 want to ping 5VT
It can not get replay because different VLAG TAG
Simplary PC2 can not ping the PC1
So, please provide the guidence about it how we can achive this
functionality in 5VT EVM board.
Regards,
shailesh
[-- Attachment #1.2: Type: text/html, Size: 1732 bytes --]
[-- Attachment #2: VLAN.jpg --]
[-- Type: image/jpeg, Size: 37597 bytes --]
^ permalink raw reply
* Re: [Bridge] Disable mac address learning
From: matan monitz @ 2010-11-28 13:27 UTC (permalink / raw)
To: Benjamin Vanheuverzwijn; +Cc: bridge
In-Reply-To: <AANLkTikRhBENKqbvmuYv=jVoMaFM-2EH57977Yt_1Y5j@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2091 bytes --]
it works
i use it for transfering a port monitor via bridge
On Sun, Nov 28, 2010 at 12:21 AM, Benjamin Vanheuverzwijn <bvanheu@gmail.com
> wrote:
> Hi,
>
> Yes that's wat i thought but reading
> http://www.linuxfoundation.org/collaborate/workgroups/networking/bridge#Showing_devices_in_a_bridge tells
> me that if i set aging time to 0, it will makes all entry permanent.
>
> The aging time is the number of seconds a MAC address will be
>
> kept in the forwarding database after having received a packet
>
> from this MAC address.
>
> The entries in the forwarding database are periodically timed out
>
> to ensure they won't stay around forever.
>
> Normally there should be no need to modify this parameter,
>
> but it can be changed with (time is in seconds).
>
>
>> # brctl setageing bridgename time
>
>
>> Setting ageing time to zero makes all entries permanent.
>
>
> My bad i did not take time to test it.
>
> Thanks for your answer i will try this!
>
> On Sat, Nov 27, 2010 at 5:16 PM, Stephen Hemminger <
> shemminger@linux-foundation.org> wrote:
>
>> On Sat, 27 Nov 2010 16:46:34 -0500
>> Benjamin Vanheuverzwijn <bvanheu@gmail.com> wrote:
>>
>> > Hi,
>> >
>> > Is there a way to disable the mac address learning in linux using
>> "brctl" or
>> > custom code so any packet sent to the bridge will be flooded in each
>> > interfaces.
>> >
>> > I would like to do something similar as OpenBSD:
>> > $ brconfig <bridge> -learn <iface>
>> > $ man brconfig
>> > ...
>> > -learn interface
>> > Mark interface so that the source address of packets
>> received
>> > from interface are not entered into the address cache.
>> > ...
>> >
>> > I tried to search on google and on the mailing-list without success.
>>
>> Just set ageing time to zero no need for special config option.
>>
>
>
>
> --
> Benjamin Vanheuverzwijn
>
> Google Talk/Jabber - bvanheu@gmail.com
> http://vanheu.ca
>
>
> _______________________________________________
> Bridge mailing list
> Bridge@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/bridge
>
[-- Attachment #2: Type: text/html, Size: 4996 bytes --]
^ permalink raw reply
* Re: [Bridge] Disable mac address learning
From: Benjamin Vanheuverzwijn @ 2010-11-27 22:21 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: bridge
In-Reply-To: <20101127141647.6814f62f@nehalam>
[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]
Hi,
Yes that's wat i thought but reading
http://www.linuxfoundation.org/collaborate/workgroups/networking/bridge#Showing_devices_in_a_bridge
tells
me that if i set aging time to 0, it will makes all entry permanent.
The aging time is the number of seconds a MAC address will be
kept in the forwarding database after having received a packet
from this MAC address.
The entries in the forwarding database are periodically timed out
to ensure they won't stay around forever.
Normally there should be no need to modify this parameter,
but it can be changed with (time is in seconds).
> # brctl setageing bridgename time
> Setting ageing time to zero makes all entries permanent.
My bad i did not take time to test it.
Thanks for your answer i will try this!
On Sat, Nov 27, 2010 at 5:16 PM, Stephen Hemminger <
shemminger@linux-foundation.org> wrote:
> On Sat, 27 Nov 2010 16:46:34 -0500
> Benjamin Vanheuverzwijn <bvanheu@gmail.com> wrote:
>
> > Hi,
> >
> > Is there a way to disable the mac address learning in linux using "brctl"
> or
> > custom code so any packet sent to the bridge will be flooded in each
> > interfaces.
> >
> > I would like to do something similar as OpenBSD:
> > $ brconfig <bridge> -learn <iface>
> > $ man brconfig
> > ...
> > -learn interface
> > Mark interface so that the source address of packets
> received
> > from interface are not entered into the address cache.
> > ...
> >
> > I tried to search on google and on the mailing-list without success.
>
> Just set ageing time to zero no need for special config option.
>
--
Benjamin Vanheuverzwijn
Google Talk/Jabber - bvanheu@gmail.com
http://vanheu.ca
[-- Attachment #2: Type: text/html, Size: 5021 bytes --]
^ permalink raw reply
* Re: [Bridge] Disable mac address learning
From: Stephen Hemminger @ 2010-11-27 22:16 UTC (permalink / raw)
To: Benjamin Vanheuverzwijn; +Cc: bridge
In-Reply-To: <AANLkTikviZkym71gVp6BZUeiqHjbARhjB-J-mnE_5A6J@mail.gmail.com>
On Sat, 27 Nov 2010 16:46:34 -0500
Benjamin Vanheuverzwijn <bvanheu@gmail.com> wrote:
> Hi,
>
> Is there a way to disable the mac address learning in linux using "brctl" or
> custom code so any packet sent to the bridge will be flooded in each
> interfaces.
>
> I would like to do something similar as OpenBSD:
> $ brconfig <bridge> -learn <iface>
> $ man brconfig
> ...
> -learn interface
> Mark interface so that the source address of packets received
> from interface are not entered into the address cache.
> ...
>
> I tried to search on google and on the mailing-list without success.
Just set ageing time to zero no need for special config option.
^ permalink raw reply
* [Bridge] Disable mac address learning
From: Benjamin Vanheuverzwijn @ 2010-11-27 21:46 UTC (permalink / raw)
To: bridge
[-- Attachment #1: Type: text/plain, Size: 629 bytes --]
Hi,
Is there a way to disable the mac address learning in linux using "brctl" or
custom code so any packet sent to the bridge will be flooded in each
interfaces.
I would like to do something similar as OpenBSD:
$ brconfig <bridge> -learn <iface>
$ man brconfig
...
-learn interface
Mark interface so that the source address of packets received
from interface are not entered into the address cache.
...
I tried to search on google and on the mailing-list without success.
Thanks in advance for your replies,
--
Benjamin Vanheuverzwijn
Google Talk/Jabber - bvanheu@gmail.com
http://vanheu.ca
[-- Attachment #2: Type: text/html, Size: 968 bytes --]
^ permalink raw reply
* Re: [Bridge] [PATCH 0/5] bridge: RCU annotation and cleanup
From: Stephen Hemminger @ 2010-11-15 16:19 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: netdev, bridge, davem, eric.dumazet
In-Reply-To: <201011152123.HHB21896.HFOOVSMFOtLJQF@I-love.SAKURA.ne.jp>
On Mon, 15 Nov 2010 21:23:37 +0900
Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
> > + rp = rcu_dereference(hlist_next_rcu(rp->next));
>
> I think this one is hlist_next_rcu(rp).
Yes, you are correct.
--
^ permalink raw reply
* Re: [Bridge] [PATCH 0/5] bridge: RCU annotation and cleanup
From: Eric Dumazet @ 2010-11-15 13:33 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: netdev, shemminger, bridge, davem
In-Reply-To: <201011152123.HHB21896.HFOOVSMFOtLJQF@I-love.SAKURA.ne.jp>
Le lundi 15 novembre 2010 à 21:23 +0900, Tetsuo Handa a écrit :
> Stephen Hemminger wrote:
> > This is a split up of what Eric did with a couple of small changes and additions.
> Something seems to be wrong with this patchset.
>
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> > @@ -173,8 +177,8 @@ forward:
> > switch (p->state) {
> > case BR_STATE_FORWARDING:
> > rhook = rcu_dereference(br_should_route_hook);
> > - if (rhook != NULL) {
> > - if (rhook(skb))
> > + if (rhook) {
> > + if ((*rhook)(skb))
>
> Is *rhook != NULL guaranteed when rhook != NULL?
Its the C standard convention, we call function pointed by rhook, not
*rhook.
$ cat func.c
typedef int (*hook_t)(int a1, int a2);
hook_t *hook;
int foo(int a1, int a2)
{
hook_t *handler = hook;
if (handler)
return handler(a1, a2);
return 0;
}
$ gcc -O2 -c func.c
func.c: In function ‘foo’:
func.c:10:17: error: called object ‘handler’ is not a function
Now, if we use (*handler), it works :
$ cat func.c
typedef int (*hook_t)(int a1, int a2);
hook_t *hook;
int foo(int a1, int a2)
{
hook_t *handler = hook;
if (handler)
return (*handler)(a1, a2);
return 0;
}
$ gcc -O2 -c func.c
$
^ permalink raw reply
* Re: [Bridge] [PATCH 0/5] bridge: RCU annotation and cleanup
From: Tetsuo Handa @ 2010-11-15 12:23 UTC (permalink / raw)
To: shemminger, davem, eric.dumazet; +Cc: netdev, bridge
In-Reply-To: <20101114211201.678755903@vyatta.com>
Stephen Hemminger wrote:
> This is a split up of what Eric did with a couple of small changes and additions.
Something seems to be wrong with this patchset.
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
> @@ -173,8 +177,8 @@ forward:
> switch (p->state) {
> case BR_STATE_FORWARDING:
> rhook = rcu_dereference(br_should_route_hook);
> - if (rhook != NULL) {
> - if (rhook(skb))
> + if (rhook) {
> + if ((*rhook)(skb))
Is *rhook != NULL guaranteed when rhook != NULL?
> return skb;
> dest = eth_hdr(skb)->h_dest;
> }
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
> @@ -242,7 +242,7 @@ static void br_multicast_flood(struct ne
> if ((unsigned long)lport >= (unsigned long)port)
> p = rcu_dereference(p->next);
> if ((unsigned long)rport >= (unsigned long)port)
> - rp = rcu_dereference(rp->next);
> + rp = rcu_dereference(hlist_next_rcu(rp->next));
I think this one is hlist_next_rcu(rp).
> }
>
> if (!prev)
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
> @@ -475,11 +475,8 @@ int br_del_if(struct net_bridge *br, str
> {
> struct net_bridge_port *p;
>
> - if (!br_port_exists(dev))
> - return -EINVAL;
> -
> p = br_port_get(dev);
Don't you need to use br_port_get_rtnl()? (I don't know.)
> - if (p->br != br)
> + if (!p || p->br != br)
> return -EINVAL;
>
> del_nbp(p);
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
> @@ -169,9 +171,9 @@ static int br_rtm_setlink(struct sk_buff
> if (!dev)
> return -ENODEV;
>
> - if (!br_port_exists(dev))
> - return -EINVAL;
> p = br_port_get(dev);
Don't you need to use br_port_get_rtnl()? (I don't know.)
> + if (!p)
> + return -EINVAL;
>
> /* if kernel STP is running, don't allow changes */
> if (p->br->stp_enabled == BR_KERNEL_STP)
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
> @@ -151,11 +151,21 @@ struct net_bridge_port
> #endif
> };
>
> -#define br_port_get_rcu(dev) \
> - ((struct net_bridge_port *) rcu_dereference(dev->rx_handler_data))
> -#define br_port_get(dev) ((struct net_bridge_port *) dev->rx_handler_data)
> #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
>
> +static inline struct net_bridge_port *br_port_get_rcu(const struct net_device *dev)
> +{
> + return br_port_exists(dev) ?
> + rcu_dereference(dev->rx_handler_data) : NULL;
> +}
> +
> +static inline struct net_bridge_port *br_port_get(struct net_device *dev)
> +{
> + return br_port_exists(dev) ? dev->rx_handler_data : NULL;
> +}
> +
> +#define br_port_get(dev) ((struct net_bridge_port *) dev->rx_handler_data)
Why are you defining br_port_get() twice, once as macro and once as inlined
function?
^ permalink raw reply
* [Bridge] [PATCH 5/5] bridge: add RCU annotations to bridge port lookup
From: Stephen Hemminger @ 2010-11-14 21:12 UTC (permalink / raw)
To: David Miller, Eric Dumazet; +Cc: netdev, bridge
In-Reply-To: <20101114211201.678755903@vyatta.com>
[-- Attachment #1: bridgeX.patch --]
[-- Type: text/plain, Size: 2502 bytes --]
From: Eric Dumazet <eric.dumazet@gmail.com>
Add modern __rcu annotations to bridge code, to reduce sparse errors,
and self document code.
(CONFIG_SPARSE_RCU_POINTER=y)
br_port_get() split int br_port_get_rtnl() and br_port_get_rcu()
to make clear RTNL is held.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
net/bridge/br_if.c | 2 +-
net/bridge/br_netlink.c | 4 ++--
net/bridge/br_notify.c | 4 ++--
net/bridge/br_private.h | 9 +++++----
4 files changed, 10 insertions(+), 9 deletions(-)
--- a/net/bridge/br_netlink.c 2010-11-14 12:24:35.000000000 -0800
+++ b/net/bridge/br_netlink.c 2010-11-14 12:26:47.859892139 -0800
@@ -119,7 +119,7 @@ static int br_dump_ifinfo(struct sk_buff
idx = 0;
for_each_netdev(net, dev) {
- struct net_bridge_port *port = br_port_get(dev);
+ struct net_bridge_port *port = br_port_get_rtnl(dev);
/* not a bridge port */
if (!port || idx < cb->args[0])
@@ -171,7 +171,7 @@ static int br_rtm_setlink(struct sk_buff
if (!dev)
return -ENODEV;
- p = br_port_get(dev);
+ p = br_port_get_rtnl(dev);
if (!p)
return -EINVAL;
--- a/net/bridge/br_private.h 2010-11-14 12:25:44.600853008 -0800
+++ b/net/bridge/br_private.h 2010-11-14 12:33:23.880986098 -0800
@@ -159,9 +159,10 @@ static inline struct net_bridge_port *br
rcu_dereference(dev->rx_handler_data) : NULL;
}
-static inline struct net_bridge_port *br_port_get(struct net_device *dev)
+static inline struct net_bridge_port *br_port_get_rtnl(struct net_device *dev)
{
- return br_port_exists(dev) ? dev->rx_handler_data : NULL;
+ return br_port_exists(dev) ?
+ rtnl_dereference(dev->rx_handler_data) : NULL;
}
#define br_port_get(dev) ((struct net_bridge_port *) dev->rx_handler_data)
--- a/net/bridge/br_if.c 2010-11-14 12:23:34.000000000 -0800
+++ b/net/bridge/br_if.c 2010-11-14 12:27:24.267934764 -0800
@@ -475,7 +475,7 @@ int br_del_if(struct net_bridge *br, str
{
struct net_bridge_port *p;
- p = br_port_get(dev);
+ p = br_port_get_rtnl(dev);
if (!p || p->br != br)
return -EINVAL;
--- a/net/bridge/br_notify.c 2010-11-14 12:30:46.250267273 -0800
+++ b/net/bridge/br_notify.c 2010-11-14 12:31:11.749076964 -0800
@@ -37,10 +37,10 @@ static int br_device_event(struct notifi
int err;
/* not a port of a bridge */
- if (!br_port_exists(dev))
+ p = br_port_get_rtnl(dev);
+ if (!p)
return NOTIFY_DONE;
- p = br_port_get(dev);
br = p->br;
switch (event) {
^ permalink raw reply
* [Bridge] [PATCH 4/5] bridge: fix RCU races with bridge port
From: Stephen Hemminger @ 2010-11-14 21:12 UTC (permalink / raw)
To: David Miller, Eric Dumazet; +Cc: netdev, bridge
In-Reply-To: <20101114211201.678755903@vyatta.com>
[-- Attachment #1: br-port-rcu-race.patch --]
[-- Type: text/plain, Size: 6647 bytes --]
The macro br_port_exists() is not enough protection when only
RCU is being used. There is a tiny race where other CPU has cleared port
handler hook, but is bridge port flag might still be set.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
net/bridge/br_fdb.c | 15 +++++++++------
net/bridge/br_if.c | 5 +----
net/bridge/br_netfilter.c | 13 +++++++------
net/bridge/br_netlink.c | 10 ++++++----
net/bridge/br_notify.c | 2 +-
net/bridge/br_private.h | 16 +++++++++++++---
net/bridge/br_stp_bpdu.c | 8 ++++----
net/bridge/netfilter/ebtables.c | 11 +++++------
8 files changed, 46 insertions(+), 34 deletions(-)
--- a/net/bridge/br_netfilter.c 2010-11-14 12:36:22.970441653 -0800
+++ b/net/bridge/br_netfilter.c 2010-11-14 12:44:55.666987357 -0800
@@ -131,17 +131,18 @@ void br_netfilter_rtable_init(struct net
static inline struct rtable *bridge_parent_rtable(const struct net_device *dev)
{
- if (!br_port_exists(dev))
- return NULL;
- return &br_port_get_rcu(dev)->br->fake_rtable;
+ struct net_bridge_port *port;
+
+ port = br_port_get_rcu(dev);
+ return port ? &port->br->fake_rtable : NULL;
}
static inline struct net_device *bridge_parent(const struct net_device *dev)
{
- if (!br_port_exists(dev))
- return NULL;
+ struct net_bridge_port *port;
- return br_port_get_rcu(dev)->br->dev;
+ port = br_port_get_rcu(dev);
+ return port ? port->br->dev : NULL;
}
static inline struct nf_bridge_info *nf_bridge_alloc(struct sk_buff *skb)
--- a/net/bridge/br_stp_bpdu.c 2010-11-14 12:36:22.982443121 -0800
+++ b/net/bridge/br_stp_bpdu.c 2010-11-14 12:44:55.666987357 -0800
@@ -141,10 +141,6 @@ void br_stp_rcv(const struct stp_proto *
struct net_bridge *br;
const unsigned char *buf;
- if (!br_port_exists(dev))
- goto err;
- p = br_port_get_rcu(dev);
-
if (!pskb_may_pull(skb, 4))
goto err;
@@ -153,6 +149,10 @@ void br_stp_rcv(const struct stp_proto *
if (buf[0] != 0 || buf[1] != 0 || buf[2] != 0)
goto err;
+ p = br_port_get_rcu(dev);
+ if (!p)
+ goto err;
+
br = p->br;
spin_lock(&br->lock);
--- a/net/bridge/netfilter/ebtables.c 2010-11-14 12:36:22.998445081 -0800
+++ b/net/bridge/netfilter/ebtables.c 2010-11-14 12:45:49.393153060 -0800
@@ -128,6 +128,7 @@ ebt_basic_match(const struct ebt_entry *
const struct net_device *in, const struct net_device *out)
{
const struct ethhdr *h = eth_hdr(skb);
+ const struct net_bridge_port *p;
__be16 ethproto;
int verdict, i;
@@ -148,13 +149,11 @@ ebt_basic_match(const struct ebt_entry *
if (FWINV2(ebt_dev_check(e->out, out), EBT_IOUT))
return 1;
/* rcu_read_lock()ed by nf_hook_slow */
- if (in && br_port_exists(in) &&
- FWINV2(ebt_dev_check(e->logical_in, br_port_get_rcu(in)->br->dev),
- EBT_ILOGICALIN))
+ if (in && (p = br_port_get_rcu(in)) != NULL &&
+ FWINV2(ebt_dev_check(e->logical_in, p->br->dev), EBT_ILOGICALIN))
return 1;
- if (out && br_port_exists(out) &&
- FWINV2(ebt_dev_check(e->logical_out, br_port_get_rcu(out)->br->dev),
- EBT_ILOGICALOUT))
+ if (out && (p = br_port_get_rcu(out)) != NULL &&
+ FWINV2(ebt_dev_check(e->logical_out, p->br->dev), EBT_ILOGICALOUT))
return 1;
if (e->bitmask & EBT_SOURCEMAC) {
--- a/net/bridge/br_fdb.c 2010-11-14 12:36:23.022448019 -0800
+++ b/net/bridge/br_fdb.c 2010-11-14 12:44:55.670987817 -0800
@@ -238,15 +238,18 @@ struct net_bridge_fdb_entry *__br_fdb_ge
int br_fdb_test_addr(struct net_device *dev, unsigned char *addr)
{
struct net_bridge_fdb_entry *fdb;
+ struct net_bridge_port *port;
int ret;
- if (!br_port_exists(dev))
- return 0;
-
rcu_read_lock();
- fdb = __br_fdb_get(br_port_get_rcu(dev)->br, addr);
- ret = fdb && fdb->dst->dev != dev &&
- fdb->dst->state == BR_STATE_FORWARDING;
+ port = br_port_get_rcu(dev);
+ if (!port)
+ ret = 0;
+ else {
+ fdb = __br_fdb_get(port->br, addr);
+ ret = fdb && fdb->dst->dev != dev &&
+ fdb->dst->state == BR_STATE_FORWARDING;
+ }
rcu_read_unlock();
return ret;
--- a/net/bridge/br_notify.c 2010-11-14 12:36:23.034449489 -0800
+++ b/net/bridge/br_notify.c 2010-11-14 12:44:55.670987817 -0800
@@ -32,7 +32,7 @@ struct notifier_block br_device_notifier
static int br_device_event(struct notifier_block *unused, unsigned long event, void *ptr)
{
struct net_device *dev = ptr;
- struct net_bridge_port *p = br_port_get(dev);
+ struct net_bridge_port *p;
struct net_bridge *br;
int err;
--- a/net/bridge/br_private.h 2010-11-14 12:44:07.257410977 -0800
+++ b/net/bridge/br_private.h 2010-11-14 12:44:55.670987817 -0800
@@ -151,11 +151,21 @@ struct net_bridge_port
#endif
};
-#define br_port_get_rcu(dev) \
- ((struct net_bridge_port *) rcu_dereference(dev->rx_handler_data))
-#define br_port_get(dev) ((struct net_bridge_port *) dev->rx_handler_data)
#define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
+static inline struct net_bridge_port *br_port_get_rcu(const struct net_device *dev)
+{
+ return br_port_exists(dev) ?
+ rcu_dereference(dev->rx_handler_data) : NULL;
+}
+
+static inline struct net_bridge_port *br_port_get(struct net_device *dev)
+{
+ return br_port_exists(dev) ? dev->rx_handler_data : NULL;
+}
+
+#define br_port_get(dev) ((struct net_bridge_port *) dev->rx_handler_data)
+
struct br_cpu_netstats {
u64 rx_packets;
u64 rx_bytes;
--- a/net/bridge/br_if.c 2010-11-14 12:36:23.046450958 -0800
+++ b/net/bridge/br_if.c 2010-11-14 12:44:55.670987817 -0800
@@ -475,11 +475,8 @@ int br_del_if(struct net_bridge *br, str
{
struct net_bridge_port *p;
- if (!br_port_exists(dev))
- return -EINVAL;
-
p = br_port_get(dev);
- if (p->br != br)
+ if (!p || p->br != br)
return -EINVAL;
del_nbp(p);
--- a/net/bridge/br_netlink.c 2010-11-14 12:36:23.062452917 -0800
+++ b/net/bridge/br_netlink.c 2010-11-14 12:44:55.670987817 -0800
@@ -119,11 +119,13 @@ static int br_dump_ifinfo(struct sk_buff
idx = 0;
for_each_netdev(net, dev) {
+ struct net_bridge_port *port = br_port_get(dev);
+
/* not a bridge port */
- if (!br_port_exists(dev) || idx < cb->args[0])
+ if (!port || idx < cb->args[0])
goto skip;
- if (br_fill_ifinfo(skb, br_port_get(dev),
+ if (br_fill_ifinfo(skb, port,
NETLINK_CB(cb->skb).pid,
cb->nlh->nlmsg_seq, RTM_NEWLINK,
NLM_F_MULTI) < 0)
@@ -169,9 +171,9 @@ static int br_rtm_setlink(struct sk_buff
if (!dev)
return -ENODEV;
- if (!br_port_exists(dev))
- return -EINVAL;
p = br_port_get(dev);
+ if (!p)
+ return -EINVAL;
/* if kernel STP is running, don't allow changes */
if (p->br->stp_enabled == BR_KERNEL_STP)
^ permalink raw reply
* [Bridge] [PATCH 3/5] netdev: add rcu annotations to receive handler hook
From: Stephen Hemminger @ 2010-11-14 21:12 UTC (permalink / raw)
To: David Miller, Eric Dumazet; +Cc: netdev, bridge
In-Reply-To: <20101114211201.678755903@vyatta.com>
[-- Attachment #1: rx_handler_rcu.patch --]
[-- Type: text/plain, Size: 595 bytes --]
Suggested by Eric's bridge RCU changes.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
include/linux/netdevice.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/include/linux/netdevice.h 2010-11-14 11:41:53.224298362 -0800
+++ b/include/linux/netdevice.h 2010-11-14 11:42:42.546359900 -0800
@@ -995,8 +995,8 @@ struct net_device {
unsigned int real_num_rx_queues;
#endif
- rx_handler_func_t *rx_handler;
- void *rx_handler_data;
+ rx_handler_func_t __rcu *rx_handler;
+ void __rcu *rx_handler_data;
struct netdev_queue __rcu *ingress_queue;
^ permalink raw reply
* [Bridge] [PATCH 2/5] bridge: add proper RCU annotation to should_route_hook
From: Stephen Hemminger @ 2010-11-14 21:12 UTC (permalink / raw)
To: David Miller, Eric Dumazet; +Cc: netdev, bridge
In-Reply-To: <20101114211201.678755903@vyatta.com>
[-- Attachment #1: bridge-hook-typedef.patch --]
[-- Type: text/plain, Size: 3022 bytes --]
From: Eric Dumazet <eric.dumazet@gmail.com>
Add br_should_route_hook_t typedef, this is the only way we can
get a clean RCU implementation for function pointer.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
include/linux/if_bridge.h | 4 +++-
net/bridge/br.c | 4 ----
net/bridge/br_input.c | 10 +++++++---
net/bridge/netfilter/ebtable_broute.c | 3 ++-
4 files changed, 12 insertions(+), 9 deletions(-)
--- a/net/bridge/br.c 2010-11-14 11:18:54.048692005 -0800
+++ b/net/bridge/br.c 2010-11-14 11:19:47.347027185 -0800
@@ -22,8 +22,6 @@
#include "br_private.h"
-int (*br_should_route_hook)(struct sk_buff *skb);
-
static const struct stp_proto br_stp_proto = {
.rcv = br_stp_rcv,
};
@@ -102,8 +100,6 @@ static void __exit br_deinit(void)
br_fdb_fini();
}
-EXPORT_SYMBOL(br_should_route_hook);
-
module_init(br_init)
module_exit(br_deinit)
MODULE_LICENSE("GPL");
--- a/net/bridge/br_input.c 2010-11-14 11:18:54.048692005 -0800
+++ b/net/bridge/br_input.c 2010-11-14 11:41:40.558700481 -0800
@@ -21,6 +21,10 @@
/* Bridge group multicast address 802.1d (pg 51). */
const u8 br_group_address[ETH_ALEN] = { 0x01, 0x80, 0xc2, 0x00, 0x00, 0x00 };
+/* Hook for brouter */
+br_should_route_hook_t __rcu *br_should_route_hook __read_mostly;
+EXPORT_SYMBOL(br_should_route_hook);
+
static int br_pass_frame_up(struct sk_buff *skb)
{
struct net_device *indev, *brdev = BR_INPUT_SKB_CB(skb)->brdev;
@@ -139,7 +143,7 @@ struct sk_buff *br_handle_frame(struct s
{
struct net_bridge_port *p;
const unsigned char *dest = eth_hdr(skb)->h_dest;
- int (*rhook)(struct sk_buff *skb);
+ br_should_route_hook_t *rhook;
if (unlikely(skb->pkt_type == PACKET_LOOPBACK))
return skb;
@@ -173,8 +177,8 @@ forward:
switch (p->state) {
case BR_STATE_FORWARDING:
rhook = rcu_dereference(br_should_route_hook);
- if (rhook != NULL) {
- if (rhook(skb))
+ if (rhook) {
+ if ((*rhook)(skb))
return skb;
dest = eth_hdr(skb)->h_dest;
}
--- a/include/linux/if_bridge.h 2010-11-14 11:18:54.048692005 -0800
+++ b/include/linux/if_bridge.h 2010-11-14 11:19:47.351028008 -0800
@@ -102,7 +102,9 @@ struct __fdb_entry {
#include <linux/netdevice.h>
extern void brioctl_set(int (*ioctl_hook)(struct net *, unsigned int, void __user *));
-extern int (*br_should_route_hook)(struct sk_buff *skb);
+
+typedef int (*br_should_route_hook_t)(struct sk_buff *skb);
+extern br_should_route_hook_t __rcu *br_should_route_hook;
#endif
--- a/net/bridge/netfilter/ebtable_broute.c 2010-11-14 11:20:39.745149494 -0800
+++ b/net/bridge/netfilter/ebtable_broute.c 2010-11-14 11:21:01.020917528 -0800
@@ -87,7 +87,8 @@ static int __init ebtable_broute_init(vo
if (ret < 0)
return ret;
/* see br_input.c */
- rcu_assign_pointer(br_should_route_hook, ebt_broute);
+ rcu_assign_pointer(br_should_route_hook,
+ (br_should_route_hook_t *)ebt_broute);
return 0;
}
^ permalink raw reply
* [Bridge] [PATCH 1/5] bridge: add RCU annotation to bridge multicast table
From: Stephen Hemminger @ 2010-11-14 21:12 UTC (permalink / raw)
To: David Miller, Eric Dumazet; +Cc: netdev, bridge
In-Reply-To: <20101114211201.678755903@vyatta.com>
[-- Attachment #1: bridge-mlock-rcu.patch --]
[-- Type: text/plain, Size: 10144 bytes --]
From: Eric Dumazet <eric.dumazet@gmail.com>
Add modern __rcu annotatations to bridge multicast table.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
net/bridge/br_forward.c | 4 +-
net/bridge/br_multicast.c | 78 ++++++++++++++++++++++++++++++----------------
net/bridge/br_private.h | 6 +--
3 files changed, 56 insertions(+), 32 deletions(-)
--- a/net/bridge/br_multicast.c 2010-11-14 12:36:30.383348571 -0800
+++ b/net/bridge/br_multicast.c 2010-11-14 12:36:37.084167303 -0800
@@ -33,6 +33,9 @@
#include "br_private.h"
+#define mlock_dereference(X, br) \
+ rcu_dereference_protected(X, lockdep_is_held(&br->multicast_lock))
+
#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
static inline int ipv6_is_local_multicast(const struct in6_addr *addr)
{
@@ -135,7 +138,7 @@ static struct net_bridge_mdb_entry *br_m
struct net_bridge_mdb_entry *br_mdb_get(struct net_bridge *br,
struct sk_buff *skb)
{
- struct net_bridge_mdb_htable *mdb = br->mdb;
+ struct net_bridge_mdb_htable *mdb = rcu_dereference(br->mdb);
struct br_ip ip;
if (br->multicast_disabled)
@@ -235,7 +238,8 @@ static void br_multicast_group_expired(u
if (mp->ports)
goto out;
- mdb = br->mdb;
+ mdb = mlock_dereference(br->mdb, br);
+
hlist_del_rcu(&mp->hlist[mdb->ver]);
mdb->size--;
@@ -249,16 +253,20 @@ out:
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_htable *mdb;
struct net_bridge_mdb_entry *mp;
struct net_bridge_port_group *p;
- struct net_bridge_port_group **pp;
+ struct net_bridge_port_group __rcu **pp;
+
+ mdb = mlock_dereference(br->mdb, br);
mp = br_mdb_ip_get(mdb, &pg->addr);
if (WARN_ON(!mp))
return;
- for (pp = &mp->ports; (p = *pp); pp = &p->next) {
+ for (pp = &mp->ports;
+ (p = mlock_dereference(*pp, br)) != NULL;
+ pp = &p->next) {
if (p != pg)
continue;
@@ -294,10 +302,10 @@ out:
spin_unlock(&br->multicast_lock);
}
-static int br_mdb_rehash(struct net_bridge_mdb_htable **mdbp, int max,
+static int br_mdb_rehash(struct net_bridge_mdb_htable __rcu **mdbp, int max,
int elasticity)
{
- struct net_bridge_mdb_htable *old = *mdbp;
+ struct net_bridge_mdb_htable *old = rcu_dereference_protected(*mdbp, 1);
struct net_bridge_mdb_htable *mdb;
int err;
@@ -569,7 +577,7 @@ static struct net_bridge_mdb_entry *br_m
struct net_bridge *br, struct net_bridge_port *port,
struct br_ip *group, int hash)
{
- struct net_bridge_mdb_htable *mdb = br->mdb;
+ struct net_bridge_mdb_htable *mdb;
struct net_bridge_mdb_entry *mp;
struct hlist_node *p;
unsigned count = 0;
@@ -577,6 +585,7 @@ static struct net_bridge_mdb_entry *br_m
int elasticity;
int err;
+ mdb = rcu_dereference_protected(br->mdb, 1);
hlist_for_each_entry(mp, p, &mdb->mhash[hash], hlist[mdb->ver]) {
count++;
if (unlikely(br_ip_equal(group, &mp->addr)))
@@ -642,10 +651,11 @@ static struct net_bridge_mdb_entry *br_m
struct net_bridge *br, struct net_bridge_port *port,
struct br_ip *group)
{
- struct net_bridge_mdb_htable *mdb = br->mdb;
+ struct net_bridge_mdb_htable *mdb;
struct net_bridge_mdb_entry *mp;
int hash;
+ mdb = rcu_dereference_protected(br->mdb, 1);
if (!mdb) {
if (br_mdb_rehash(&br->mdb, BR_HASH_SIZE, 0))
return NULL;
@@ -660,7 +670,7 @@ static struct net_bridge_mdb_entry *br_m
case -EAGAIN:
rehash:
- mdb = br->mdb;
+ mdb = rcu_dereference_protected(br->mdb, 1);
hash = br_ip_hash(mdb, group);
break;
@@ -692,7 +702,7 @@ static int br_multicast_add_group(struct
{
struct net_bridge_mdb_entry *mp;
struct net_bridge_port_group *p;
- struct net_bridge_port_group **pp;
+ struct net_bridge_port_group __rcu **pp;
unsigned long now = jiffies;
int err;
@@ -712,7 +722,9 @@ static int br_multicast_add_group(struct
goto out;
}
- for (pp = &mp->ports; (p = *pp); pp = &p->next) {
+ for (pp = &mp->ports;
+ (p = mlock_dereference(*pp, br)) != NULL;
+ pp = &p->next) {
if (p->port == port)
goto found;
if ((unsigned long)p->port < (unsigned long)port)
@@ -1106,7 +1118,7 @@ static int br_ip4_multicast_query(struct
struct net_bridge_mdb_entry *mp;
struct igmpv3_query *ih3;
struct net_bridge_port_group *p;
- struct net_bridge_port_group **pp;
+ struct net_bridge_port_group __rcu **pp;
unsigned long max_delay;
unsigned long now = jiffies;
__be32 group;
@@ -1145,7 +1157,7 @@ static int br_ip4_multicast_query(struct
if (!group)
goto out;
- mp = br_mdb_ip4_get(br->mdb, group);
+ mp = br_mdb_ip4_get(mlock_dereference(br->mdb, br), group);
if (!mp)
goto out;
@@ -1157,7 +1169,9 @@ static int br_ip4_multicast_query(struct
try_to_del_timer_sync(&mp->timer) >= 0))
mod_timer(&mp->timer, now + max_delay);
- for (pp = &mp->ports; (p = *pp); pp = &p->next) {
+ for (pp = &mp->ports;
+ (p = mlock_dereference(*pp, br)) != NULL;
+ pp = &p->next) {
if (timer_pending(&p->timer) ?
time_after(p->timer.expires, now + max_delay) :
try_to_del_timer_sync(&p->timer) >= 0)
@@ -1178,7 +1192,8 @@ static int br_ip6_multicast_query(struct
struct mld_msg *mld = (struct mld_msg *) icmp6_hdr(skb);
struct net_bridge_mdb_entry *mp;
struct mld2_query *mld2q;
- struct net_bridge_port_group *p, **pp;
+ struct net_bridge_port_group *p;
+ struct net_bridge_port_group __rcu **pp;
unsigned long max_delay;
unsigned long now = jiffies;
struct in6_addr *group = NULL;
@@ -1214,7 +1229,7 @@ static int br_ip6_multicast_query(struct
if (!group)
goto out;
- mp = br_mdb_ip6_get(br->mdb, group);
+ mp = br_mdb_ip6_get(mlock_dereference(br->mdb, br), group);
if (!mp)
goto out;
@@ -1225,7 +1240,9 @@ static int br_ip6_multicast_query(struct
try_to_del_timer_sync(&mp->timer) >= 0))
mod_timer(&mp->timer, now + max_delay);
- for (pp = &mp->ports; (p = *pp); pp = &p->next) {
+ for (pp = &mp->ports;
+ (p = mlock_dereference(*pp, br)) != NULL;
+ pp = &p->next) {
if (timer_pending(&p->timer) ?
time_after(p->timer.expires, now + max_delay) :
try_to_del_timer_sync(&p->timer) >= 0)
@@ -1254,7 +1271,7 @@ static void br_multicast_leave_group(str
timer_pending(&br->multicast_querier_timer))
goto out;
- mdb = br->mdb;
+ mdb = mlock_dereference(br->mdb, br);
mp = br_mdb_ip_get(mdb, group);
if (!mp)
goto out;
@@ -1277,7 +1294,9 @@ static void br_multicast_leave_group(str
goto out;
}
- for (p = mp->ports; p; p = p->next) {
+ for (p = mlock_dereference(mp->ports, br);
+ p != NULL;
+ p = mlock_dereference(p->next, br)) {
if (p->port != port)
continue;
@@ -1625,7 +1644,7 @@ void br_multicast_stop(struct net_bridge
del_timer_sync(&br->multicast_query_timer);
spin_lock_bh(&br->multicast_lock);
- mdb = br->mdb;
+ mdb = mlock_dereference(br->mdb, br);
if (!mdb)
goto out;
@@ -1729,6 +1748,7 @@ int br_multicast_toggle(struct net_bridg
{
struct net_bridge_port *port;
int err = 0;
+ struct net_bridge_mdb_htable *mdb;
spin_lock(&br->multicast_lock);
if (br->multicast_disabled == !val)
@@ -1741,15 +1761,16 @@ int br_multicast_toggle(struct net_bridg
if (!netif_running(br->dev))
goto unlock;
- if (br->mdb) {
- if (br->mdb->old) {
+ mdb = mlock_dereference(br->mdb, br);
+ if (mdb) {
+ if (mdb->old) {
err = -EEXIST;
rollback:
br->multicast_disabled = !!val;
goto unlock;
}
- err = br_mdb_rehash(&br->mdb, br->mdb->max,
+ err = br_mdb_rehash(&br->mdb, mdb->max,
br->hash_elasticity);
if (err)
goto rollback;
@@ -1774,6 +1795,7 @@ int br_multicast_set_hash_max(struct net
{
int err = -ENOENT;
u32 old;
+ struct net_bridge_mdb_htable *mdb;
spin_lock(&br->multicast_lock);
if (!netif_running(br->dev))
@@ -1782,7 +1804,9 @@ int br_multicast_set_hash_max(struct net
err = -EINVAL;
if (!is_power_of_2(val))
goto unlock;
- if (br->mdb && val < br->mdb->size)
+
+ mdb = mlock_dereference(br->mdb, br);
+ if (mdb && val < mdb->size)
goto unlock;
err = 0;
@@ -1790,8 +1814,8 @@ int br_multicast_set_hash_max(struct net
old = br->hash_max;
br->hash_max = val;
- if (br->mdb) {
- if (br->mdb->old) {
+ if (mdb) {
+ if (mdb->old) {
err = -EEXIST;
rollback:
br->hash_max = old;
--- a/net/bridge/br_private.h 2010-11-14 12:36:30.399350527 -0800
+++ b/net/bridge/br_private.h 2010-11-14 12:44:07.257410977 -0800
@@ -72,7 +72,7 @@ struct net_bridge_fdb_entry
struct net_bridge_port_group {
struct net_bridge_port *port;
- struct net_bridge_port_group *next;
+ struct net_bridge_port_group __rcu *next;
struct hlist_node mglist;
struct rcu_head rcu;
struct timer_list timer;
@@ -86,7 +86,7 @@ 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 net_bridge_port_group __rcu *ports;
struct rcu_head rcu;
struct timer_list timer;
struct timer_list query_timer;
@@ -227,7 +227,7 @@ struct net_bridge
unsigned long multicast_startup_query_interval;
spinlock_t multicast_lock;
- struct net_bridge_mdb_htable *mdb;
+ struct net_bridge_mdb_htable __rcu *mdb;
struct hlist_head router_list;
struct hlist_head mglist;
--- a/net/bridge/br_forward.c 2010-11-14 12:36:47.833478598 -0800
+++ b/net/bridge/br_forward.c 2010-11-14 12:42:22.001208297 -0800
@@ -223,7 +223,7 @@ static void br_multicast_flood(struct ne
struct net_bridge_port_group *p;
struct hlist_node *rp;
- rp = rcu_dereference(br->router_list.first);
+ rp = rcu_dereference(hlist_first_rcu(&br->router_list));
p = mdst ? rcu_dereference(mdst->ports) : NULL;
while (p || rp) {
struct net_bridge_port *port, *lport, *rport;
@@ -242,7 +242,7 @@ static void br_multicast_flood(struct ne
if ((unsigned long)lport >= (unsigned long)port)
p = rcu_dereference(p->next);
if ((unsigned long)rport >= (unsigned long)port)
- rp = rcu_dereference(rp->next);
+ rp = rcu_dereference(hlist_next_rcu(rp->next));
}
if (!prev)
^ 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