03 Jun, 2014
1 commit
-
br_handle_local_finish() is allowing us to insert an FDB entry with
disallowed vlan. For example, when port 1 and 2 are communicating in
vlan 10, and even if vlan 10 is disallowed on port 3, port 3 can
interfere with their communication by spoofed src mac address with
vlan id 10.Note: Even if it is judged that a frame should not be learned, it should
not be dropped because it is destined for not forwarding layer but higher
layer. See IEEE 802.1Q-2011 8.13.10.Signed-off-by: Toshiaki Makita
Acked-by: Vlad Yasevich
Signed-off-by: David S. Miller
30 Mar, 2014
1 commit
-
The gfp parameter was added in:
commit 47be03a28cc6c80e3aa2b3e8ed6d960ff0c5c0af
Author: Amerigo Wang
Date: Fri Aug 10 01:24:37 2012 +0000netpoll: use GFP_ATOMIC in slave_enable_netpoll() and __netpoll_setup()
slave_enable_netpoll() and __netpoll_setup() may be called
with read_lock() held, so should use GFP_ATOMIC to allocate
memory. Eric suggested to pass gfp flags to __netpoll_setup().Cc: Eric Dumazet
Cc: "David S. Miller"
Reported-by: Dan Carpenter
Signed-off-by: Eric Dumazet
Signed-off-by: Cong Wang
Signed-off-by: David S. MillerThe reason for the gfp parameter was removed in:
commit c4cdef9b7183159c23c7302aaf270d64c549f557
Author: dingtianhong
Date: Tue Jul 23 15:25:27 2013 +0800bonding: don't call slave_xxx_netpoll under spinlocks
The slave_xxx_netpoll will call synchronize_rcu_bh(),
so the function may schedule and sleep, it should't be
called under spinlocks.bond_netpoll_setup() and bond_netpoll_cleanup() are always
protected by rtnl lock, it is no need to take the read lock,
as the slave list couldn't be changed outside rtnl lock.Signed-off-by: Ding Tianhong
Cc: Jay Vosburgh
Cc: Andy Gospodarek
Signed-off-by: David S. MillerNothing else that calls __netpoll_setup or ndo_netpoll_setup
requires a gfp paramter, so remove the gfp parameter from both
of these functions making the code clearer.Signed-off-by: "Eric W. Biederman"
Signed-off-by: David S. Miller
25 Feb, 2014
1 commit
-
Convert the more obvious uses of memcpy to ether_addr_copy.
There are still uses of memcpy that could be converted but
these addresses are __aligned(2).Convert a couple uses of 6 in gr_private.h to ETH_ALEN.
Signed-off-by: Joe Perches
Signed-off-by: David S. Miller
11 Feb, 2014
3 commits
-
Vlan codes unconditionally delete local fdb entries.
We should consider the possibility that other ports have the same
address and vlan.Example of problematic case:
ip link set eth0 address 12:34:56:78:90:ab
ip link set eth1 address aa:bb:cc:dd:ee:ff
brctl addif br0 eth0
brctl addif br0 eth1 # br0 will have mac address 12:34:56:78:90:ab
bridge vlan add dev eth0 vid 10
bridge vlan add dev eth1 vid 10
bridge vlan add dev br0 vid 10 self
We will have fdb entry such that f->dst == eth0, f->vlan_id == 10 and
f->addr == 12:34:56:78:90:ab at this time.
Next, delete eth0 vlan 10.
bridge vlan del dev eth0 vid 10
In this case, we still need the entry for br0, but it will be deleted.Note that br0 needs the entry even though its mac address is not set
manually. To delete the entry with proper condition checking,
fdb_delete_local() is suitable to use.Signed-off-by: Toshiaki Makita
Acked-by: Vlad Yasevich
Signed-off-by: David S. Miller -
We should take into account the followings when deleting a local fdb
entry.- nbp_vlan_find() can be used only when vid != 0 to check if an entry is
deletable, because a fdb entry with vid 0 can exist at any time while
nbp_vlan_find() always return false with vid 0.Example of problematic case:
ip link set eth0 address 12:34:56:78:90:ab
ip link set eth1 address 12:34:56:78:90:ab
brctl addif br0 eth0
brctl addif br0 eth1
ip link set eth0 address aa:bb:cc:dd:ee:ff
Then, the fdb entry 12:34:56:78:90:ab will be deleted even though the
bridge port eth1 still has that address.- The port to which the bridge device is attached might needs a local entry
if its mac address is set manually.Example of problematic case:
ip link set eth0 address 12:34:56:78:90:ab
brctl addif br0 eth0
ip link set br0 address 12:34:56:78:90:ab
ip link set eth0 address aa:bb:cc:dd:ee:ff
Then, the fdb still must have the entry 12:34:56:78:90:ab, but it will be
deleted.We can use br->dev->addr_assign_type to check if the address is manually
set or not, but I propose another approach.Since we delete and insert local entries whenever changing mac address
of the bridge device, we can change dst of the entry to NULL regardless of
addr_assign_type when deleting an entry associated with a certain port,
and if it is found to be unnecessary later, then delete it.
That is, if changing mac address of a port, the entry might be changed
to its dst being NULL first, but is eventually deleted when recalculating
and changing bridge id.This approach is especially useful when we want to share the code with
deleting vlan in which the bridge device might want such an entry regardless
of addr_assign_type, and makes things easy because we don't have to consider
if mac address of the bridge device will be changed or not at the time we
delete a local entry of a port, which means fdb code will not be bothered
even if the bridge id calculating logic is changed in the future.Also, this change reduces inconsistent state, where frames whose dst is the
mac address of the bridge, can't reach the bridge because of premature fdb
entry deletion. This change reduces the possibility that the bridge device
replies unreachable mac address to arp requests, which could occur during
the short window between calling del_nbp() and br_stp_recalculate_bridge_id()
in br_del_if(). This will effective after br_fdb_delete_by_port() starts to
use the same code by following patch.Signed-off-by: Toshiaki Makita
Acked-by: Vlad Yasevich
Signed-off-by: David S. Miller -
br_fdb_changeaddr() assumes that there is at most one local entry per port
per vlan. It used to be true, but since commit 36fd2b63e3b4 ("bridge: allow
creating/deleting fdb entries via netlink"), it has not been so.
Therefore, the function might fail to search a correct previous address
to be deleted and delete an arbitrary local entry if user has added local
entries manually.Example of problematic case:
ip link set eth0 address ee:ff:12:34:56:78
brctl addif br0 eth0
bridge fdb add 12:34:56:78:90:ab dev eth0 master
ip link set eth0 address aa:bb:cc:dd:ee:ff
Then, the address 12:34:56:78:90:ab might be deleted instead of
ee:ff:12:34:56:78, the original mac address of eth0.Address this issue by introducing a new flag, added_by_user, to struct
net_bridge_fdb_entry.Note that br_fdb_delete_by_port() has to set added_by_user to 0 in cases
like:
ip link set eth0 address 12:34:56:78:90:ab
ip link set eth1 address aa:bb:cc:dd:ee:ff
brctl addif br0 eth0
bridge fdb add aa:bb:cc:dd:ee:ff dev eth0 master
brctl addif br0 eth1
brctl delif br0 eth0
In this case, kernel should delete the user-added entry aa:bb:cc:dd:ee:ff,
but it also should have been added by "brctl addif br0 eth1" originally,
so we don't delete it and treat it a new kernel-created entry.Signed-off-by: Toshiaki Makita
Signed-off-by: David S. Miller
14 Jan, 2014
1 commit
-
And it can become static.
Cc: Stephen Hemminger
Cc: David S. Miller
Signed-off-by: Cong Wang
Signed-off-by: David S. Miller
05 Jan, 2014
1 commit
-
They are same, so unify them as one, pcpu_sw_netstats.
Define pcpu_sw_netstat in netdevice.h, remove pcpu_tstats
from if_tunnel and remove br_cpu_netstats from br_private.hCc: Cong Wang
Cc: Stephen Hemminger
Signed-off-by: Li RongQing
Signed-off-by: David S. Miller
20 Dec, 2013
1 commit
-
Spaces required before the open parenthesis '(', before the open
brace '{', after that ',' and around that '?/:'.Signed-off-by: Tan Xiaojun
Signed-off-by: David S. Miller
07 Dec, 2013
1 commit
-
br_stp_rcv() is reached by non-rx_handler path. That means there is no
guarantee that dev is bridge port and therefore simple NULL check of
->rx_handler_data is not enough. There is need to check if dev is really
bridge port and since only rcu read lock is held here, do it by checking
->rx_handler pointer.Note that synchronize_net() in netdev_rx_handler_unregister() ensures
this approach as valid.Introduced originally by:
commit f350a0a87374418635689471606454abc7beaa3a
"bridge: use rx_handler_data pointer to store net_bridge_port pointer"Fixed but not in the best way by:
commit b5ed54e94d324f17c97852296d61a143f01b227a
"bridge: fix RCU races with bridge port"Reintroduced by:
commit 716ec052d2280d511e10e90ad54a86f5b5d4dcc2
"bridge: fix NULL pointer deref of br_port_get_rcu"Please apply to stable trees as well. Thanks.
RH bugzilla reference: https://bugzilla.redhat.com/show_bug.cgi?id=1025770
Reported-by: Laine Stump
Debugged-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
Signed-off-by: Jiri Pirko
Acked-by: Michael S. Tsirkin
Acked-by: Eric Dumazet
Signed-off-by: David S. Miller
05 Nov, 2013
1 commit
-
Conflicts:
drivers/net/ethernet/emulex/benet/be.h
drivers/net/netconsole.c
net/bridge/br_private.hThree mostly trivial conflicts.
The net/bridge/br_private.h conflict was a function signature (argument
addition) change overlapping with the extern removals from Joe Perches.In drivers/net/netconsole.c we had one change adjusting a printk message
whilst another changed "printk(KERN_INFO" into "pr_info(".Lastly, the emulex change was a new inline function addition overlapping
with Joe Perches's extern removals.Signed-off-by: David S. Miller
30 Oct, 2013
1 commit
-
Currently multicast code attempts to extrace the vlan id from
the skb even when vlan filtering is disabled. This can lead
to mdb entries being created with the wrong vlan id.
Pass the already extracted vlan id to the multicast
filtering code to make the correct id is used in
creation as well as lookup.Signed-off-by: Vlad Yasevich
Acked-by: Toshiaki Makita
Signed-off-by: David S. Miller
24 Oct, 2013
1 commit
-
Conflicts:
drivers/net/usb/qmi_wwan.c
include/net/dst.hTrivial merge conflicts, both were overlapping changes.
Signed-off-by: David S. Miller
23 Oct, 2013
1 commit
-
While this commit was a good attempt to fix issues occuring when no
multicast querier is present, this commit still has two more issues:1) There are cases where mdb entries do not expire even if there is a
querier present. The bridge will unnecessarily continue flooding
multicast packets on the according ports.2) Never removing an mdb entry could be exploited for a Denial of
Service by an attacker on the local link, slowly, but steadily eating up
all memory.Actually, this commit became obsolete with
"bridge: disable snooping if there is no querier" (b00589af3b)
which included fixes for a few more cases.Therefore reverting the following commits (the commit stated in the
commit message plus three of its follow up fixes):====================
Revert "bridge: update mdb expiration timer upon reports."
This reverts commit f144febd93d5ee534fdf23505ab091b2b9088edc.
Revert "bridge: do not call setup_timer() multiple times"
This reverts commit 1faabf2aab1fdaa1ace4e8c829d1b9cf7bfec2f1.
Revert "bridge: fix some kernel warning in multicast timer"
This reverts commit c7e8e8a8f7a70b343ca1e0f90a31e35ab2d16de1.
Revert "bridge: only expire the mdb entry when query is received"
This reverts commit 9f00b2e7cf241fa389733d41b615efdaa2cb0f5b.
====================CC: Cong Wang
Signed-off-by: Linus Lüssing
Reviewed-by: Vlad Yasevich
Signed-off-by: David S. Miller
20 Oct, 2013
1 commit
-
There are a mix of function prototypes with and without extern
in the kernel sources. Standardize on not using extern for
function prototypes.Function prototypes don't need to be written with extern.
extern is assumed by the compiler. Its use is as unnecessary as
using auto to declare automatic/local variables in a block.Signed-off-by: Joe Perches
Signed-off-by: David S. Miller
19 Oct, 2013
1 commit
-
We are using the VLAN_TAG_PRESENT bit to detect whether the PVID is
set or not at br_get_pvid(), while we don't care about the bit in
adding/deleting the PVID, which makes it impossible to forward any
incomming untagged frame with vlan_filtering enabled.Since vid 0 cannot be used for the PVID, we can use vid 0 to indicate
that the PVID is not set, which is slightly more efficient than using
the VLAN_TAG_PRESENT.Fix the problem by getting rid of using the VLAN_TAG_PRESENT.
Signed-off-by: Toshiaki Makita
Reviewed-by: Vlad Yasevich
Signed-off-by: David S. Miller
16 Sep, 2013
2 commits
-
The NULL deref happens when br_handle_frame is called between these
2 lines of del_nbp:
dev->priv_flags &= ~IFF_BRIDGE_PORT;
/* --> br_handle_frame is called at this time */
netdev_rx_handler_unregister(dev);In br_handle_frame the return of br_port_get_rcu(dev) is dereferenced
without check but br_port_get_rcu(dev) returns NULL if:
!(dev->priv_flags & IFF_BRIDGE_PORT)Eric Dumazet pointed out the testing of IFF_BRIDGE_PORT is not necessary
here since we're in rcu_read_lock and we have synchronize_net() in
netdev_rx_handler_unregister. So remove the testing of IFF_BRIDGE_PORT
and by the previous patch, make sure br_port_get_rcu is called in
bridging code.Signed-off-by: Hong Zhiguo
Acked-by: Eric Dumazet
Signed-off-by: David S. Miller -
current br_port_get_rcu is problematic in bridging path
(NULL deref). Change these calls in netlink path first.Signed-off-by: Hong Zhiguo
Acked-by: Eric Dumazet
Signed-off-by: David S. Miller
13 Sep, 2013
1 commit
-
At some point limits were added to forward_delay. However, the
limits are only enforced when STP is enabled. This created a
scenario where you could have a value outside the allowed range
while STP is disabled, which then stuck around even after STP
is enabled.This patch fixes this by clamping the value when we enable STP.
I had to move the locking around a bit to ensure that there is
no window where someone could insert a value outside the range
while we're in the middle of enabling STP.Signed-off-by: Herbert Xu
Cheers,
Signed-off-by: David S. Miller
06 Sep, 2013
2 commits
-
Conflicts:
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
net/bridge/br_multicast.c
net/ipv6/sit.cThe conflicts were minor:
1) sit.c changes overlap with change to ip_tunnel_xmit() signature.
2) br_multicast.c had an overlap between computing max_delay using
msecs_to_jiffies and turning MLDV2_MRC() into an inline function
with a name using lowercase instead of uppercase letters.3) stmmac had two overlapping changes, one which conditionally allocated
and hooked up a dma_cfg based upon the presence of the pbl OF property,
and another one handling store-and-forward DMA made. The latter of
which should not go into the new of_find_property() basic block.Signed-off-by: David S. Miller
-
The multicast snooping code should have matured enough to be safely
applicable to IPv6 link-local multicast addresses (excluding the
link-local all nodes address, ff02::1), too.Signed-off-by: Linus Lüssing
Signed-off-by: David S. Miller
31 Aug, 2013
1 commit
-
Currently we would still potentially suffer multicast packet loss if there
is just either an IGMP or an MLD querier: For the former case, we would
possibly drop IPv6 multicast packets, for the latter IPv4 ones. This is
because we are currently assuming that if either an IGMP or MLD querier
is present that the other one is present, too.This patch makes the behaviour and fix added in
"bridge: disable snooping if there is no querier" (b00589af3b04)
to also work if there is either just an IGMP or an MLD querier on the
link: It refines the deactivation of the snooping to be protocol
specific by using separate timers for the snooped IGMP and MLD queries
as well as separate timers for our internal IGMP and MLD queriers.Signed-off-by: Linus Lüssing
Signed-off-by: David S. Miller
05 Aug, 2013
1 commit
-
Use of RCU here with out marked pointer and function doesn't match prototype
with sparse.Signed-off-by: Stephen Hemminger
Signed-off-by: David S. Miller
04 Aug, 2013
1 commit
-
Merge net into net-next to setup some infrastructure Eric
Dumazet needs for usbnet changes.Signed-off-by: David S. Miller
01 Aug, 2013
1 commit
-
If there is no querier on a link then we won't get periodic reports and
therefore won't be able to learn about multicast listeners behind ports,
potentially leading to lost multicast packets, especially for multicast
listeners that joined before the creation of the bridge.These lost multicast packets can appear since c5c23260594
("bridge: Add multicast_querier toggle and disable queries by default")
in particular.With this patch we are flooding multicast packets if our querier is
disabled and if we didn't detect any other querier.A grace period of the Maximum Response Delay of the querier is added to
give multicast responses enough time to arrive and to be learned from
before disabling the flooding behaviour again.Signed-off-by: Linus Lüssing
Signed-off-by: David S. Miller
27 Jul, 2013
1 commit
-
This started out with fixing a sparse warning, then I realized that
the wrapper function br_netpoll_info could just be collapsed away
by rolling it into the enable code.Also, eliminate unnecessary goto's
Signed-off-by: Stephen Hemminger
Reviewed-by: Jiri Pirko
Acked-by: Neil Horman
Signed-off-by: David S. Miller
11 Jun, 2013
2 commits
-
Add a flag to control flood of unicast traffic. By default, flood is
on and the bridge will flood unicast traffic if it doesn't know
the destination. When the flag is turned off, unicast traffic
without an FDB will not be forwarded to the specified port.Signed-off-by: Vlad Yasevich
Reviewed-by: Michael S. Tsirkin
Signed-off-by: David S. Miller -
Allow user to control whether mac learning is enabled on the port.
By default, mac learning is enabled. Disabling mac learning will
cause new dynamic FDB entries to not be created for a particular port.Signed-off-by: Vlad Yasevich
Signed-off-by: Stephen Hemminger
Signed-off-by: David S. Miller
23 May, 2013
2 commits
-
Currently we arm the expire timer when the mdb entry is added,
however, this causes problem when there is no querier sent
out after that.So we should only arm the timer when a corresponding query is
received, as suggested by Herbert.And he also mentioned "if there is no querier then group
subscriptions shouldn't expire. There has to be at least one querier
in the network for this thing to work. Otherwise it just degenerates
into a non-snooping switch, which is OK."Cc: Herbert Xu
Cc: Stephen Hemminger
Cc: "David S. Miller"
Cc: Adam Baker
Signed-off-by: Cong Wang
Acked-by: Herbert Xu
Signed-off-by: David S. Miller -
Quote from Adam:
"If it is believed that the use of 0.0.0.0
as the IP address is what is causing strange behaviour on other devices
then is there a good reason that a bridge rather than a router shouldn't
be the active querier? If not then using the bridge IP address and
having the querier enabled by default may be a reasonable solution
(provided that our querier obeys the election rules and shuts up if it
sees a query from a lower IP address that isn't 0.0.0.0). Just because a
device is the elected querier for IGMP doesn't appear to mean it is
required to perform any other routing functions."And introduce a new troggle for it, as suggested by Herbert.
Suggested-by: Adam Baker
Cc: Herbert Xu
Cc: Stephen Hemminger
Cc: "David S. Miller"
Cc: Adam Baker
Signed-off-by: Cong Wang
Acked-by: Herbert Xu
Signed-off-by: David S. Miller
16 Apr, 2013
1 commit
-
Keep a STP port path cost value if it was set by a user.
Don't replace it with the link-speed based path cost
whenever the link goes down and comes back up.Reported-by: Roopa Prabhu
Signed-off-by: Stephen Hemminger
Signed-off-by: David S. Miller
08 Mar, 2013
1 commit
-
Obviously, vid should be considered when searching for multicast
group.Cc: Vlad Yasevich
Cc: Stephen Hemminger
Cc: "David S. Miller"
Signed-off-by: Cong Wang
Acked-by: Vlad Yasevich
Signed-off-by: David S. Miller
14 Feb, 2013
8 commits
-
Add an ability to configure a separate "untagged" egress
policy to the VLAN information of the bridge. This superseeds PVID
policy and makes PVID ingress-only. The policy is configured with a
new flag and is represented as a port bitmap per vlan. Egress frames
with a VLAN id in "untagged" policy bitmap would egress
the port without VLAN header.Signed-off-by: Vlad Yasevich
Signed-off-by: David S. Miller -
When VLAN is added to the port, a local fdb entry for that port
(the entry with the mac address of the port) is added for that
VLAN. This way we can correctly determine if the traffic
is for the bridge itself. If the address of the port changes,
we try to change all the local fdb entries we have for that port.Signed-off-by: Vlad Yasevich
Signed-off-by: David S. Miller -
When a user adds bridge neighbors, allow him to specify VLAN id.
If the VLAN id is not specified, the neighbor will be added
for VLANs currently in the ports filter list. If no VLANs are
configured on the port, we use vlan 0 and only add 1 entry.Signed-off-by: Vlad Yasevich
Acked-by: Jitendra Kalsaria
Signed-off-by: David S. Miller -
Add vlan_id to multicasts groups so that we know which vlan
each group belongs to and can correctly forward to appropriate vlan.Signed-off-by: Vlad Yasevich
Signed-off-by: David S. Miller -
This patch adds vlan to unicast fdb entries that are created for
learned addresses (not the manually configured ones). It adds
vlan id into the hash mix and uses vlan as an addditional parameter
for an entry match.Signed-off-by: Vlad Yasevich
Signed-off-by: David S. Miller -
A user may designate a certain vlan as PVID. This means that
any ingress frame that does not contain a vlan tag is assigned to
this vlan and any forwarding decisions are made with this vlan in mind.Signed-off-by: Vlad Yasevich
Signed-off-by: David S. Miller -
At ingress, any untagged traffic is assigned to the PVID.
Any tagged traffic is filtered according to membership bitmap.At egress, if the vlan matches the PVID, the frame is sent
untagged. Otherwise the frame is sent tagged.Signed-off-by: Vlad Yasevich
Signed-off-by: David S. Miller -
Using the RTM_GETLINK dump the vlan filter list of a given
bridge port. The information depends on setting the filter
flag similar to how nic VF info is dumped.Signed-off-by: Vlad Yasevich
Signed-off-by: David S. Miller