Re: [Bridge] [PATCH v5 net-next 00/15] RX filtering in DSA
From: Vladimir Oltean Date: Tue, 29 Jun 2021 22:09:23 +0300 > On Tue, Jun 29, 2021 at 09:58:22PM +0300, Vladimir Oltean wrote: >> On Tue, Jun 29, 2021 at 11:52:13AM -0700, David Miller wrote: >> > From: Vladimir Oltean >> > Date: Tue, 29 Jun 2021 17:06:43 +0300 >> > >> > > Changes in v5: >> > > - added READ_ONCE and WRITE_ONCE for fdb->dst >> > > - removed a paranoid WARN_ON in DSA >> > > - added some documentation regarding how 'bridge fdb' is supposed to be >> > > used with DSA >> > >> > Vlad, I applied v4, could you please send me relative fixups to v5? >> > >> > Thank you. >> >> Thanks for applying. I'm going to prepare the delta patches right now. > > Dave, is it possible that you may have applied v5 with the cover letter > from v4? I checked and everything is in its right place: Yes I believe that is what happened. Thanks for checking...
Re: [Bridge] [PATCH v5 net-next 00/15] RX filtering in DSA
From: Vladimir Oltean Date: Tue, 29 Jun 2021 17:06:43 +0300 > Changes in v5: > - added READ_ONCE and WRITE_ONCE for fdb->dst > - removed a paranoid WARN_ON in DSA > - added some documentation regarding how 'bridge fdb' is supposed to be > used with DSA Vlad, I applied v4, could you please send me relative fixups to v5? Thank you.
Re: [Bridge] [PATCH v3 net-next 07/11] net: prep switchdev drivers for concurrent SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS
From: Vladimir Oltean Date: Wed, 10 Feb 2021 11:14:41 +0200 > From: Vladimir Oltean > > Because the bridge will start offloading SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS > while not serialized by any lock such as the br->lock spinlock, existing > drivers that treat that attribute and cache the brport flags might no > longer work correctly. > > The issue is that the brport flags are a single unsigned long bitmask, > and the bridge only guarantees the validity of the changed bits, not the > full state. So when offloading two concurrent switchdev attributes, such > as one for BR_LEARNING and another for BR_FLOOD, it might happen that > the flags having BR_FLOOD are written into the cached value, and this in > turn disables the BR_LEARNING bit which was set previously. > > We can fix this across the board by keeping individual boolean variables > for each brport flag. Note that mlxsw and prestera were setting the > BR_LEARNING_SYNC flag too, but that appears to be just dead code, so I > removed it. > > Signed-off-by: Vladimir Oltean This needs updating because, as discussed, there is no race.
Re: [Bridge] [PATCH net] net: bridge: fdb: don't flush ext_learn entries
From: Nikolay Aleksandrov
Date: Mon, 28 Sep 2020 18:30:02 +0300
> From: Nikolay Aleksandrov
>
> When a user-space software manages fdb entries externally it should
> set the ext_learn flag which marks the fdb entry as externally managed
> and avoids expiring it (they're treated as static fdbs). Unfortunately
> on events where fdb entries are flushed (STP down, netlink fdb flush
> etc) these fdbs are also deleted automatically by the bridge. That in turn
> causes trouble for the managing user-space software (e.g. in MLAG setups
> we lose remote fdb entries on port flaps).
> These entries are completely externally managed so we should avoid
> automatically deleting them, the only exception are offloaded entries
> (i.e. BR_FDB_ADDED_BY_EXT_LEARN + BR_FDB_OFFLOADED). They are flushed as
> before.
>
> Fixes: eb100e0e24a2 ("net: bridge: allow to add externally learned entries
> from user-space")
> Signed-off-by: Nikolay Aleksandrov
Applied and queued up for -stable, thank you.
Re: [Bridge] [PATCH net-next] net: bridge: mcast: remove only S, G port groups from sg_port hash
From: Nikolay Aleksandrov
Date: Fri, 25 Sep 2020 13:25:49 +0300
> From: Nikolay Aleksandrov
>
> We should remove a group from the sg_port hash only if it's an S,G
> entry. This makes it correct and more symmetric with group add. Also
> since *,G groups are not added to that hash we can hide a bug.
>
> Fixes: 085b53c8beab ("net: bridge: mcast: add sg_port rhashtable")
> Signed-off-by: Nikolay Aleksandrov
Applied, thanks.
Re: [Bridge] [PATCH net-next v2 00/16] net: bridge: mcast: IGMPv3/MLDv2 fast-path (part 2)
From: Nikolay Aleksandrov Date: Tue, 22 Sep 2020 10:30:11 +0300 > This is the second part of the IGMPv3/MLDv2 support which adds support > for the fast-path. ... Series applied to net-next and build testing, thanks Nikolay.
Re: [Bridge] [PATCH net] net: bridge: br_vlan_get_pvid_rcu() should dereference the VLAN group under RCU
From: Vladimir Oltean
Date: Tue, 22 Sep 2020 01:07:09 +0300
> When calling the RCU brother of br_vlan_get_pvid(), lockdep warns:
>
> =
> WARNING: suspicious RCU usage
> 5.9.0-rc3-01631-g13c17acb8e38-dirty #814 Not tainted
> -
> net/bridge/br_private.h:1054 suspicious rcu_dereference_protected() usage!
>
> Call trace:
> lockdep_rcu_suspicious+0xd4/0xf8
> __br_vlan_get_pvid+0xc0/0x100
> br_vlan_get_pvid_rcu+0x78/0x108
>
> The warning is because br_vlan_get_pvid_rcu() calls nbp_vlan_group()
> which calls rtnl_dereference() instead of rcu_dereference(). In turn,
> rtnl_dereference() calls rcu_dereference_protected() which assumes
> operation under an RCU write-side critical section, which obviously is
> not the case here. So, when the incorrect primitive is used to access
> the RCU-protected VLAN group pointer, READ_ONCE() is not used, which may
> cause various unexpected problems.
>
> I'm sad to say that br_vlan_get_pvid() and br_vlan_get_pvid_rcu() cannot
> share the same implementation. So fix the bug by splitting the 2
> functions, and making br_vlan_get_pvid_rcu() retrieve the VLAN groups
> under proper locking annotations.
>
> Fixes: 7582f5b70f9a ("bridge: add br_vlan_get_pvid_rcu()")
> Signed-off-by: Vladimir Oltean
Applied and queued up for -stable, thank you.
Re: [Bridge] [PATCH] Revert "net: linkwatch: add check for netdevice being present to linkwatch_do_dev"
From: Nikolay Aleksandrov Date: Fri, 18 Sep 2020 12:35:02 + > Thanks for the analysis, I don't see any issues with checking if the device > isn't present. It will have to go through some testing, but no obvious > objections/issues. Have you tried if it fixes your case? > I have briefly gone over drivers' use of net_device_detach(), mostly it's used > for suspends, but there are a few cases which use it on IO error or when a > device is actually detaching (VF detach). The vlan port event is for vlan > devices on top of the bridge when BROPT_VLAN_BRIDGE_BINDING is enabled and > their > carrier is changed based on vlan participating ports' state. There are two things to resolve: 1) Why does the bridge need to get a change event for devices which have not fully resumed yet? 2) What kind of link state change is happening on devices which are not currently fully resumed yet? Really this whole situation is still quite mysterious to me. If the driver (or the PHY library it is using, etc.) is emitting link state changes before it marks itself as "present", that's the real bug.
Re: [Bridge] [PATCH net-next] net: bridge: mcast: don't ignore return value of __grp_src_toex_excl
From: Nikolay Aleksandrov
Date: Tue, 15 Sep 2020 17:57:24 +0300
> When we're handling TO_EXCLUDE report in EXCLUDE filter mode we should
> not ignore the return value of __grp_src_toex_excl() as we'll miss
> sending notifications about group changes.
>
> Fixes: 5bf1e00b6849 ("net: bridge: mcast: support for IGMPV3/MLDv2
> CHANGE_TO_INCLUDE/EXCLUDE report")
> Signed-off-by: Nikolay Aleksandrov
Applied, thank you.
Re: [Bridge] [PATCH net-next] bridge: mcast: Fix incomplete MDB dump
From: Ido Schimmel
Date: Fri, 11 Sep 2020 16:24:47 +0300
> From: Ido Schimmel
>
> Each MDB entry is encoded in a nested netlink attribute called
> 'MDBA_MDB_ENTRY'. In turn, this attribute contains another nested
> attributed called 'MDBA_MDB_ENTRY_INFO', which encodes a single port
> group entry within the MDB entry.
>
> The cited commit added the ability to restart a dump from a specific
> port group entry. However, on failure to add a port group entry to the
> dump the entire MDB entry (stored in 'nest2') is removed, resulting in
> missing port group entries.
>
> Fix this by finalizing the MDB entry with the partial list of already
> encoded port group entries.
>
> Fixes: 5205e919c9f0 ("net: bridge: mcast: add support for src list and filter
> mode dumping")
> Signed-off-by: Ido Schimmel
> Acked-by: Nikolay Aleksandrov
> Reviewed-by: Jiri Pirko
Applied, thank you.
Re: [Bridge] [PATCH net-next v2 00/15] net: bridge: mcast: initial IGMPv3 support (part 1)
From: Nikolay Aleksandrov Date: Wed, 2 Sep 2020 23:17:46 +0300 > i. e. this doesn't exclude IPv6 or makes it worse for it, on the > contrary the ops needed to > implement MLDv2 state transitions are in this set, they just need to > be extended for v6. > The new br_ip src group field contains also a field for an IPv6 > address. Great, then implement those parts and resubmit and we'll all be happy :-)
Re: [Bridge] [PATCH net-next v2 00/15] net: bridge: mcast: initial IGMPv3 support (part 1)
From: Nikolay Aleksandrov Date: Wed, 2 Sep 2020 23:08:40 +0300 > Once all the infra (with fast-path) for IGMPv3 is in, MLDv2 should > be a much easier change, but I must admit given the amount of work > this required I haven't yet looked into MLDv2 in details. The > majority of the changes would be just switch protocol statements > that take care of the IPv6 part. As difficult as this is for me to do, I really have to be consistent in my policy and push back on this. We are far past the point of accepting new features without ipv6 support as well, sorry.
Re: [Bridge] [PATCH net-next v2 00/15] net: bridge: mcast: initial IGMPv3 support (part 1)
From: Nikolay Aleksandrov Date: Wed, 2 Sep 2020 14:25:14 +0300 > Here're the sets that will come next (in order): > - Fast path patch-set which adds support for (S, G) mdb entries needed >for IGMPv3 forwarding, entry add source (kernel, user-space etc) >needed for IGMPv3 entry management, entry block mode needed for >IGMPv3 exclude mode. This set will also add iproute2 support for >manipulating and showing all the new state. > - Selftests patch-set which will verify all IGMPv3 state transitions >and forwarding > - Explicit host tracking patch-set, needed for proper fast leave and >with it fast leave will be enabled for IGMPv3 > > Not implemented yet: > - Host IGMPv3 support (currently we handle only join/leave as before) > - Proper other querier source timer and value updates > - IGMPv3/v2 compat (I have a few rough patches for this one) What about ipv6 support? The first thing I notice when reading these patches is the source filter code only supports ipv4.
Re: [Bridge] [RFC PATCH net-next] bridge: Implement MLD Querier wake-up calls / Android bug workaround
From: Linus Lüssing Date: Sun, 16 Aug 2020 22:24:24 +0200 > I'm aware that this is quite a hack, so I'm unsure if this is suitable > for upstream. On the other hand, the Android ticket isn't moving > anywhere and even if it were fixed in Android, I'd expect it to take > years until that fix would propagate or unpatched Android devices to > vanish. So I'm wondering if it should be treated like a hardware bug > workaround and by that should be suitable for applying it upstream in > the Linux kernel? Long after those Android devices are deprecated and no longer used, we will still have this ugly hack in the tree. Sorry, we're not doing this.
Re: [Bridge] [RFC PATCH net-next] bridge: Implement MLD Querier wake-up calls / Android bug workaround
From: Stephen Hemminger Date: Sun, 16 Aug 2020 15:08:13 -0700 > Rather than adding yet another feature to the bridge, could this hack be done > by > having a BPF hook? or netfilter module? Stephen please do not quote an entire huge patch just to add a small amount of commentary at the end. Thank you.
Re: [Bridge] [PATCH net v2] net: bridge: clear skb private space on bridge dev xmit
From: Nikolay Aleksandrov
Date: Sun, 2 Aug 2020 15:50:39 +0300
> We need to clear all of the bridge private skb variables as they can be
> stale due to the packet having skb->cb initialized earlier and then
> transmitted through the bridge device. Similar memset is already done on
> bridge's input. We've seen cases where proxyarp_replied was 1 on routed
> multicast packets transmitted through the bridge to ports with neigh
> suppress and were getting dropped. Same thing can in theory happen with the
> port isolation bit as well. We clear only the struct part after the bridge
> pointer (currently 8 bytes) since the pointer is always set later.
> We can now remove the redundant zeroing of frag_max_size.
> Also add a BUILD_BUG_ON to make sure we catch any movement of the bridge
> dev pointer.
>
> Fixes: 821f1b21cabb ("bridge: add new BR_NEIGH_SUPPRESS port flag to suppress
> arp and nd flood")
> Signed-off-by: Nikolay Aleksandrov
Nikolay, I applied v1 already as I'm not at all against the full clear.
Re: [Bridge] [PATCH net] net: bridge: clear bridge's private skb space on xmit
From: Nikolay Aleksandrov
Date: Fri, 31 Jul 2020 19:26:16 +0300
> We need to clear all of the bridge private skb variables as they can be
> stale due to the packet being recirculated through the stack and then
> transmitted through the bridge device. Similar memset is already done on
> bridge's input. We've seen cases where proxyarp_replied was 1 on routed
> multicast packets transmitted through the bridge to ports with neigh
> suppress which were getting dropped. Same thing can in theory happen with
> the port isolation bit as well.
>
> Fixes: 821f1b21cabb ("bridge: add new BR_NEIGH_SUPPRESS port flag to suppress
> arp and nd flood")
> Signed-off-by: Nikolay Aleksandrov
Applied and queued up for -stable, thanks.
Re: [Bridge] get rid of the address_space override in setsockopt v2
From: Christoph Hellwig Date: Sun, 26 Jul 2020 09:03:11 +0200 > On Fri, Jul 24, 2020 at 03:43:42PM -0700, David Miller wrote: >> > Changes since v1: >> > - check that users don't pass in kernel addresses >> > - more bpfilter cleanups >> > - cosmetic mptcp tweak >> >> Series applied to net-next, I'm build testing and will push this out when >> that is done. > > The buildbot found one warning with the isdn debug code after a few > days, here is what I think is the best fix: I already fixed this in net-next.
Re: [Bridge] get rid of the address_space override in setsockopt v2
From: Christoph Hellwig Date: Thu, 23 Jul 2020 08:08:42 +0200 > setsockopt is the last place in architecture-independ code that still > uses set_fs to force the uaccess routines to operate on kernel pointers. > > This series adds a new sockptr_t type that can contained either a kernel > or user pointer, and which has accessors that do the right thing, and > then uses it for setsockopt, starting by refactoring some low-level > helpers and moving them over to it before finally doing the main > setsockopt method. > > Note that apparently the eBPF selftests do not even cover this path, so > the series has been tested with a testing patch that always copies the > data first and passes a kernel pointer. This is something that works for > most common sockopts (and is something that the ePBF support relies on), > but unfortunately in various corner cases we either don't use the passed > in length, or in one case actually copy data back from setsockopt, or in > case of bpfilter straight out do not work with kernel pointers at all. > > Against net-next/master. > > Changes since v1: > - check that users don't pass in kernel addresses > - more bpfilter cleanups > - cosmetic mptcp tweak Series applied to net-next, I'm build testing and will push this out when that is done. Thanks.
Re: [Bridge] [PATCH 24/24] net: pass a sockptr_t into ->setsockopt
From: Stefan Schmidt Date: Mon, 20 Jul 2020 16:19:38 +0200 > For the ieee802154 part: > > Acked-by: Stefan Schmidt Please do not quote an entire patch just to add an ACK, trim it just to the commit message, or even less. Thank you.
Re: [Bridge] sockopt cleanups
From: Christoph Hellwig
Date: Fri, 17 Jul 2020 08:23:09 +0200
> this series cleans up various lose ends in the sockopt code, most
> importantly removing the compat_{get,set}sockopt infrastructure in favor
> of just using in_compat_syscall() in the few places that care.
Series applied to net-next, thanks.
Re: [Bridge] [PATCH net-next v4 00/12] bridge: mrp: Add support for interconnect ring
From: Horatiu Vultur Date: Tue, 14 Jul 2020 09:34:46 +0200 > This patch series extends existing MRP to add support for interconnect ring. ... Series applied, thank you.
Re: [Bridge] [PATCH net-next] net: bridge: notify on vlan tunnel changes done via the old api
From: Nikolay Aleksandrov Date: Sat, 11 Jul 2020 18:05:04 +0300 > If someone uses the old vlan API to configure tunnel mappings we'll only > generate the old-style full port notification. That would be a problem > if we are monitoring the new vlan notifications for changes. The patch > resolves the issue by adding vlan notifications to the old tunnel netlink > code. As usual we try to compress the notifications for as many vlans > in a range as possible, thus a vlan tunnel change is considered able > to enter the "current" vlan notification range if: > 1. vlan exists > 2. it has actually changed (curr_change == true) > 3. it passes all standard vlan notification range checks done by > br_vlan_can_enter_range() such as option equality, id continuity etc > > Note that vlan tunnel changes (add/del) are considered a part of vlan > options so only RTM_NEWVLAN notification is generated with the relevant > information inside. > > Signed-off-by: Nikolay Aleksandrov Applied, thank you.
Re: [Bridge] [PATCH net v2] bridge: mcast: Fix MLD2 Report IPv6 payload length check
From: Linus Lüssing
Date: Sun, 5 Jul 2020 21:10:17 +0200
> Commit e57f61858b7c ("net: bridge: mcast: fix stale nsrcs pointer in
> igmp3/mld2 report handling") introduced a bug in the IPv6 header payload
> length check which would potentially lead to rejecting a valid MLD2 Report:
>
> The check needs to take into account the 2 bytes for the "Number of
> Sources" field in the "Multicast Address Record" before reading it.
> And not the size of a pointer to this field.
>
> Fixes: e57f61858b7c ("net: bridge: mcast: fix stale nsrcs pointer in
> igmp3/mld2 report handling")
> Acked-by: Nikolay Aleksandrov
> Signed-off-by: Linus Lüssing
Applied and queued up for -stable, thank you.
Re: [Bridge] [PATCH net-next 02/12] bridge: uapi: mrp: Extend MRP attributes for MRP interconnect
From: Horatiu Vultur
Date: Mon, 6 Jul 2020 11:18:32 +0200
> +struct br_mrp_in_state {
> + __u16 in_id;
> + __u32 in_state;
> +};
Put the __u32 first then the __u16.
> +struct br_mrp_in_role {
> + __u16 in_id;
> + __u32 ring_id;
> + __u32 in_role;
> + __u32 i_ifindex;
> +};
Likewise.
> +struct br_mrp_start_in_test {
> + __u16 in_id;
> + __u32 interval;
> + __u32 max_miss;
> + __u32 period;
> +};
Likewise.
> +struct br_mrp_in_test_hdr {
> + __be16 id;
> + __u8 sa[ETH_ALEN];
> + __be16 port_role;
> + __be16 state;
> + __be16 transitions;
> + __be32 timestamp;
> +};
Likewise. Put the larger members first. There is lots of unnecessary
padding in this structure.
Re: [Bridge] [PATCH net-next 01/12] switchdev: mrp: Extend switchdev API for MRP Interconnect
From: Horatiu Vultur
Date: Mon, 6 Jul 2020 11:18:31 +0200
> +/* SWITCHDEV_OBJ_ID_IN_TEST_MRP */
> +struct switchdev_obj_in_test_mrp {
> + struct switchdev_obj obj;
> + /* The value is in us and a value of 0 represents to stop */
> + u32 interval;
> + u8 max_miss;
> + u32 in_id;
> + u32 period;
> +};
...
> +#define SWITCHDEV_OBJ_IN_TEST_MRP(OBJ) \
> + container_of((OBJ), struct switchdev_obj_in_test_mrp, obj)
> +
> +/* SWICHDEV_OBJ_ID_IN_ROLE_MRP */
> +struct switchdev_obj_in_role_mrp {
> + struct switchdev_obj obj;
> + u16 in_id;
> + u32 ring_id;
> + u8 in_role;
> + struct net_device *i_port;
> +};
...
> +#define SWITCHDEV_OBJ_IN_ROLE_MRP(OBJ) \
> + container_of((OBJ), struct switchdev_obj_in_role_mrp, obj)
> +
> +struct switchdev_obj_in_state_mrp {
> + struct switchdev_obj obj;
> + u8 in_state;
> + u32 in_id;
> +};
Please arrange these structure members in a more optimal order so that
the resulting object is denser. For example, in switchdev_obj_in_role_mrp
if you order it such that:
> + u32 ring_id;
> + u16 in_id;
> + u8 in_role;
You'll have less wasted space from padding.
Use 'pahole' or similar tools to guide you.
Re: [Bridge] [PATCH net-next v3 0/3] bridge: mrp: Add support for getting the status
From: Horatiu Vultur Date: Thu, 2 Jul 2020 10:13:04 +0200 > This patch series extends the MRP netlink interface to allow the userspace > daemon to get the status of the MRP instances in the kernel. > > v3: > - remove misleading comment > - fix to use correctly the RCU > > v2: > - fix sparse warnings Series applied, thank you.
Re: [Bridge] [PATCH net] bridge: mrp: Fix endian conversion and some other warnings
From: Horatiu Vultur
Date: Sun, 28 Jun 2020 15:45:16 +0200
> The following sparse warnings are fixed:
> net/bridge/br_mrp.c:106:18: warning: incorrect type in assignment (different
> base types)
> net/bridge/br_mrp.c:106:18:expected unsigned short [usertype]
> net/bridge/br_mrp.c:106:18:got restricted __be16 [usertype]
> net/bridge/br_mrp.c:281:23: warning: incorrect type in argument 1 (different
> modifiers)
> net/bridge/br_mrp.c:281:23:expected struct list_head *entry
> net/bridge/br_mrp.c:281:23:got struct list_head [noderef] *
> net/bridge/br_mrp.c:332:28: warning: incorrect type in argument 1 (different
> modifiers)
> net/bridge/br_mrp.c:332:28:expected struct list_head *new
> net/bridge/br_mrp.c:332:28:got struct list_head [noderef] *
> net/bridge/br_mrp.c:332:40: warning: incorrect type in argument 2 (different
> modifiers)
> net/bridge/br_mrp.c:332:40:expected struct list_head *head
> net/bridge/br_mrp.c:332:40:got struct list_head [noderef] *
> net/bridge/br_mrp.c:682:29: warning: incorrect type in argument 1 (different
> modifiers)
> net/bridge/br_mrp.c:682:29:expected struct list_head const *head
> net/bridge/br_mrp.c:682:29:got struct list_head [noderef] *
>
> Reported-by: kernel test robot
> Fixes: 2f1a11ae11d222 ("bridge: mrp: Add MRP interface.")
> Fixes: 4b8d7d4c599182 ("bridge: mrp: Extend bridge interface")
> Fixes: 9a9f26e8f7ea30 ("bridge: mrp: Connect MRP API with the switchdev API")
> Signed-off-by: Horatiu Vultur
Applied, thank you.
Re: [Bridge] [PATCH net-next v3 0/2] bridge: mrp: Extend MRP netlink interface with IFLA_BRIDGE_MRP_CLEAR
From: Horatiu Vultur Date: Fri, 26 Jun 2020 09:33:47 +0200 > This patch series extends MRP netlink interface with IFLA_BRIDGE_MRP_CLEAR. > To allow the userspace to clear all MRP instances when is started. The > second patch in the series fix different sparse warnings. > > v3: > - add the second patch to fix sparse warnings These changes are completely unrelated. The sparse stuff should probably be submitted to 'net'. And I have to ask why you really need a clear operation. Routing daemons come up and see what routes are installed, and update their internal SW tables to match. This not only allows efficient restart after a crash, but it also allows multiple daemons to work cooperatively as an agent for the same forwarding/routing table. Your usage model limits one daemon to manage the table and that limitation is completely unnecessary. Furthermore, even in a one-daemon scenerio, it's wasteful to throw away all the work the previous daemon did to load the MRP entries into the bridge. Thanks.
Re: [Bridge] [PATCH net-next 0/4] net: bridge: fdb activity tracking
From: Nikolay Aleksandrov Date: Tue, 23 Jun 2020 23:47:14 +0300 > This set adds extensions needed for EVPN multi-homing proper and > efficient mac sync. User-space (e.g. FRR) needs to be able to track > non-dynamic entry activity on per-fdb basis depending if a tracked fdb is > currently peer active or locally active and needs to be able to add new > peer active fdb (static + track + inactive) without refreshing it to get > real activity tracking. Patch 02 adds a new NDA attribute - NDA_FDB_EXT_ATTRS > to avoid future pollution of NDA attributes by bridge or vxlan. New > bridge/vxlan specific fdb attributes are embedded in NDA_FDB_EXT_ATTRS, > which is used in patch 03 to pass the new NFEA_ACTIVITY_NOTIFY attribute > which controls if an fdb should be tracked and also reflects its current > state when dumping. It is treated as a bitfield, current valid bits are: > 1 - mark an entry for activity tracking > 2 - mark an entry as inactive to avoid multiple notifications and > reflect state properly > > Patch 04 adds the ability to avoid refreshing an entry when changing it > via the NFEA_DONT_REFRESH flag. That allows user-space to mark a static > entry for tracking and keep its real activity unchanged. > The set has been extensively tested with FRR and those changes will > be upstreamed if/after it gets accepted. Series applied, thanks.
Re: [Bridge] [PATCH net v2 0/2] bridge: mrp: Update MRP_PORT_ROLE
From: Horatiu Vultur Date: Tue, 23 Jun 2020 11:05:39 +0200 > This patch series does the following: > - fixes the enum br_mrp_port_role_type. It removes the port role none(0x2) > because this is in conflict with the standard. The standard defines the > interconnect port role as value 0x2. > - adds checks regarding current defined port roles: primary(0x0) and > secondary(0x1). > > v2: > - add the validation code when setting the port role. Series applied, thank you.
Re: [Bridge] [PATCH net-next v2 0/3] bridge: mrp: Add support for MRA role
From: Horatiu Vultur Date: Sat, 30 May 2020 18:09:45 + > This patch series extends the MRP with the MRA role. > A node that has the MRA role can behave as a MRM or as a MRC. In case there > are > multiple nodes in the topology that has the MRA role then only one node can > behave as MRM and all the others need to be have as MRC. The node that has the > higher priority(lower value) will behave as MRM. > A node that has the MRA role and behaves as MRC, it just needs to forward the > MRP_Test frames between the ring ports but also it needs to detect in case it > stops receiving MRP_Test frames. In that case it would try to behave as MRM. > > v2: > - add new patch that fixes sparse warnings > - fix parsing of prio attribute Series applied, thank you.
Re: [Bridge] [PATCH net 0/2] Fix infinite loop in bridge and vxlan modules
From: Ido Schimmel Date: Mon, 1 Jun 2020 15:58:53 +0300 > From: Ido Schimmel > > When suppressing invalid IPv6 Neighbour Solicitation messages, it is > possible for the bridge and vxlan modules to get stuck in an infinite > loop. See the individual changelogs for detailed explanation of the > problem and solution. > > The bug was originally reported against the bridge module, but after > auditing the code base I found that the buggy code was copied from the > vxlan module. This patch set fixes both modules. Could not find more > instances of the problem. > > Please consider both patches for stable releases. Series applied and queued up for -stable, thank you.
Re: [Bridge] [PATCH] bridge: multicast: work around clang bug
From: Arnd Bergmann Date: Wed, 27 May 2020 15:51:13 +0200 > Clang-10 and clang-11 run into a corner case of the register > allocator on 32-bit ARM, leading to excessive stack usage from > register spilling: > > net/bridge/br_multicast.c:2422:6: error: stack frame size of 1472 bytes in > function 'br_multicast_get_stats' [-Werror,-Wframe-larger-than=] > > Work around this by marking one of the internal functions as > noinline_for_stack. > > Link: https://bugs.llvm.org/show_bug.cgi?id=45802#c9 > Signed-off-by: Arnd Bergmann Applied.
Re: [Bridge] [PATCH net-next v2] bridge: mrp: Rework the MRP netlink interface
From: Horatiu Vultur Date: Wed, 27 May 2020 12:34:30 + > This patch reworks the MRP netlink interface. Before, each attribute > represented a binary structure which made it hard to be extended. > Therefore update the MRP netlink interface such that each existing > attribute to be a nested attribute which contains the fields of the > binary structures. > In this way the MRP netlink interface can be extended without breaking > the backwards compatibility. It is also using strict checking for > attributes under the MRP top attribute. > > Signed-off-by: Horatiu Vultur Applied, thanks.
Re: [Bridge] [PATCH] bridge: mrp: Fix out-of-bounds read in br_mrp_parse
From: Horatiu Vultur
Date: Mon, 25 May 2020 09:55:41 +
> The issue was reported by syzbot. When the function br_mrp_parse was
> called with a valid net_bridge_port, the net_bridge was an invalid
> pointer. Therefore the check br->stp_enabled could pass/fail
> depending where it was pointing in memory.
> The fix consists of setting the net_bridge pointer if the port is a
> valid pointer.
>
> Reported-by: syzbot+9c6f0f1f8e32223df...@syzkaller.appspotmail.com
> Fixes: 6536993371fa ("bridge: mrp: Integrate MRP into the bridge")
> Signed-off-by: Horatiu Vultur
Applied to net-next, thanks.
Re: [Bridge] [PATCH v2 0/3] bridge: mrp: Add br_mrp_unique_ifindex function
From: Horatiu Vultur Date: Thu, 21 May 2020 23:19:04 + > This patch series adds small fixes to MRP implementation. > The following are fixed in this patch series: > - now is not allow to add the same port to multiple MRP rings > - remove unused variable > - restore the port state according to the bridge state when the MRP instance > is deleted > > v2: > - use rtnl_dereference instead of rcu_dereference in the first patch Series applied to net-next, thanks.
Re: [Bridge] [PATCH net-next] net: bridge: return false in br_mrp_enabled()
From: Jason Yan Date: Wed, 6 May 2020 14:16:16 +0800 > Fix the following coccicheck warning: > > net/bridge/br_private.h:1334:8-9: WARNING: return of 0/1 in function > 'br_mrp_enabled' with return type bool > > Signed-off-by: Jason Yan Applied.
Re: [Bridge] [PATCH net] net: bridge: vlan: Add a schedule point during VLAN processing
From: Ido Schimmel
Date: Thu, 30 Apr 2020 22:38:45 +0300
> From: Ido Schimmel
>
> User space can request to delete a range of VLANs from a bridge slave in
> one netlink request. For each deleted VLAN the FDB needs to be traversed
> in order to flush all the affected entries.
>
> If a large range of VLANs is deleted and the number of FDB entries is
> large or the FDB lock is contented, it is possible for the kernel to
> loop through the deleted VLANs for a long time. In case preemption is
> disabled, this can result in a soft lockup.
>
> Fix this by adding a schedule point after each VLAN is deleted to yield
> the CPU, if needed. This is safe because the VLANs are traversed in
> process context.
>
> Fixes: bdced7ef7838 ("bridge: support for multiple vlans and vlan ranges in
> setlink and dellink requests")
> Signed-off-by: Ido Schimmel
> Reported-by: Stefan Priebe - Profihost AG
> Tested-by: Stefan Priebe - Profihost AG
Applied and queued up for -stable.
Re: [Bridge] [PATCH net-next v4 00/11] net: bridge: mrp: Add support for Media Redundancy Protocol(MRP)
From: Horatiu Vultur Date: Sun, 26 Apr 2020 15:21:57 +0200 > Media Redundancy Protocol is a data network protocol standardized by > International Electrotechnical Commission as IEC 62439-2. It allows rings of > Ethernet switches to overcome any single failure with recovery time faster > than > STP. It is primarily used in Industrial Ethernet applications. > > Based on the previous RFC[1][2][3][4][5], and patches[6][7][8], the MRP state > machine and all the timers were moved to userspace, except for the timers used > to generate MRP Test frames. In this way the userspace doesn't know and > should > not know if the HW or the kernel will generate the MRP Test frames. The > following changes were added to the bridge to support the MRP: > - the existing netlink interface was extended with MRP support, > - allow to detect when a MRP frame was received on a MRP ring port > - allow MRP instance to forward/terminate MRP frames > - generate MRP Test frames in case the HW doesn't have support for this > > To be able to offload MRP support to HW, the switchdev API was extend. ... Series applied, thank you.
Re: [Bridge] [PATCH net-next 0/2] net: bridge: vlan options: nest the tunnel options
From: Nikolay Aleksandrov Date: Fri, 20 Mar 2020 13:23:01 +0200 > After a discussion with Roopa about the new tunnel vlan option, she > suggested that we'll be adding more tunnel options and attributes, so > it'd be better to have them all grouped together under one main vlan > entry tunnel attribute instead of making them all main attributes. Since > the tunnel code was added in this net-next cycle and still hasn't been > released we can easily nest the BRIDGE_VLANDB_ENTRY_TUNNEL_ID attribute > in BRIDGE_VLANDB_ENTRY_TUNNEL_INFO and allow for any new tunnel > attributes to be added there. In addition one positive side-effect is > that we can remove the outside vlan info flag which controlled the > operation (setlink/dellink) and move it under a new nested attribute so > user-space can specify it explicitly. > > Thus the vlan tunnel format becomes: > [BRIDGE_VLANDB_ENTRY] > [BRIDGE_VLANDB_ENTRY_TUNNEL_INFO] > [BRIDGE_VLANDB_TINFO_ID] > [BRIDGE_VLANDB_TINFO_CMD] > ... Makes sense, series applied, thanks.
Re: [Bridge] [PATCH net-next v2] net: bridge: vlan: include stats in dumps if requested
From: Nikolay Aleksandrov
Date: Thu, 19 Mar 2020 12:14:14 +0200
> This patch adds support for vlan stats to be included when dumping vlan
> information. We have to dump them only when explicitly requested (thus the
> flag below) because that disables the vlan range compression and will make
> the dump significantly larger. In order to request the stats to be
> included we add a new dump attribute called BRIDGE_VLANDB_DUMP_FLAGS which
> can affect dumps with the following first flag:
> - BRIDGE_VLANDB_DUMPF_STATS
> The stats are intentionally nested and put into separate attributes to make
> it easier for extending later since we plan to add per-vlan mcast stats,
> drop stats and possibly STP stats. This is the last missing piece from the
> new vlan API which makes the dumped vlan information complete.
>
> A dump request which should include stats looks like:
> [BRIDGE_VLANDB_DUMP_FLAGS] |= BRIDGE_VLANDB_DUMPF_STATS
>
> A vlandb entry attribute with stats looks like:
> [BRIDGE_VLANDB_ENTRY] = {
> [BRIDGE_VLANDB_ENTRY_STATS] = {
> [BRIDGE_VLANDB_STATS_RX_BYTES]
> [BRIDGE_VLANDB_STATS_RX_PACKETS]
> ...
> }
> }
>
> Signed-off-by: Nikolay Aleksandrov
> ---
> v2: Use a separate dump attribute for the flags instead of a reserved
> field to avoid uapi breakage as noted by DaveM.
> Rebased and retested on the latest net-next.
Thanks for reworking UAPI.
Applied, thanks.
Re: [Bridge] [PATCH net-next] net: bridge: vlan: include stats in dumps if requested
From: Nikolay Aleksandrov
Date: Wed, 18 Mar 2020 15:03:25 +0200
> @@ -170,11 +170,13 @@ struct bridge_stp_xstats {
> /* Bridge vlan RTM header */
> struct br_vlan_msg {
> __u8 family;
> - __u8 reserved1;
> + __u8 flags;
> __u16 reserved2;
> __u32 ifindex;
> };
I can't allow this for two reasons:
1) Userspace explicitly initializing all members will now get a compile
failure on the reference to ->reserved1
2) Userspace not initiailizing reserved fields, which worked previously,
might send in flags that trigger the new behavior.
Sorry, this is UAPI breakage.
Re: [Bridge] [PATCH net-next 0/4] net: bridge: vlan options: add support for tunnel mapping
From: Nikolay Aleksandrov Date: Tue, 17 Mar 2020 14:08:32 +0200 > In order to bring the new vlan API on par with the old one and be able > to completely migrate to the new one we need to support vlan tunnel mapping > and statistics. This patch-set takes care of the former by making it a > vlan option. There are two notable issues to deal with: > - vlan range to tunnel range mapping >* The tunnel ids are globally unique for the vlan code and a vlan can > be mapped to one tunnel, so the old API took care of ranges by > taking the starting tunnel id value and incrementally mapping > vlan id(i) -> tunnel id(i). This set takes the same approach and > uses one new attribute - BRIDGE_VLANDB_ENTRY_TUNNEL_ID. If used > with a vlan range then it's the starting tunnel id to map. > > - tunnel mapping removal >* Since there are no reserved/special tunnel ids defined, we can't > encode mapping removal within the new attribute, in order to be > able to remove a mapping we add a vlan flag which makes the new > tunnel option remove the mapping > > The rest is pretty straight-forward, in fact we directly re-use the old > code for manipulating tunnels by just mapping the command (set/del). In > order to be able to keep detecting vlan ranges we check that the current > vlan has a tunnel and it's extending the current vlan range end's tunnel > id. Looks good, series applied, thank you.
Re: [Bridge] [PATCH net v2] net: bridge: fix stale eth hdr pointer in br_dev_xmit
From: Nikolay Aleksandrov
Date: Mon, 24 Feb 2020 18:46:22 +0200
> In br_dev_xmit() we perform vlan filtering in br_allowed_ingress() but
> if the packet has the vlan header inside (e.g. bridge with disabled
> tx-vlan-offload) then the vlan filtering code will use skb_vlan_untag()
> to extract the vid before filtering which in turn calls pskb_may_pull()
> and we may end up with a stale eth pointer. Moreover the cached eth header
> pointer will generally be wrong after that operation. Remove the eth header
> caching and just use eth_hdr() directly, the compiler does the right thing
> and calculates it only once so we don't lose anything.
>
> Fixes: 057658cb33fb ("bridge: suppress arp pkts on BR_NEIGH_SUPPRESS ports")
> Signed-off-by: Nikolay Aleksandrov
> ---
> v2: remove syzbot's reported-by tag, this seems to be a different bug
Applied and queued up for -stable, thanks Nikolay.
Re: [Bridge] [PATCH] bridge: br_stp: Use built-in RCU list checking
From: madhuparnabhowmi...@gmail.com Date: Wed, 19 Feb 2020 20:47:46 +0530 > From: Madhuparna Bhowmik > > list_for_each_entry_rcu() has built-in RCU and lock checking. > > Pass cond argument to list_for_each_entry_rcu() to silence > false lockdep warning when CONFIG_PROVE_RCU_LIST is enabled > by default. > > Signed-off-by: Madhuparna Bhowmik Applied, thank you.
Re: [Bridge] [PATCH net-next v2 0/4] net: bridge: add per-vlan state option
From: Nikolay Aleksandrov Date: Fri, 24 Jan 2020 13:40:18 +0200 > This set adds the first per-vlan option - state, which uses the new vlan > infrastructure that was recently added. It gives us forwarding control on > per-vlan basis. The first 3 patches prepare the vlan code to support option > dumping and modification. We still compress vlan ranges which have equal > options, each new option will have to add its own equality check to > br_vlan_opts_eq(). The vlans are created in forwarding state by default to > be backwards compatible and vlan state is considered only when the port > state is forwarding (more info in patch 4). > I'll send the selftest for the vlan state with the iproute2 patch-set. > > v2: patch 3: do full (all-vlan) notification only on vlan > create/delete, otherwise use the per-vlan notifications only, > rework how option change ranges are detected, add more verbose error > messages when setting options and add checks if a vlan should be used. Series applied, thanks.
Re: [Bridge] [PATCH net-next v2 0/8] net: bridge: add vlan notifications and rtm support
From: Nikolay Aleksandrov Date: Tue, 14 Jan 2020 19:56:06 +0200 > This patch-set is a prerequisite for adding per-vlan options support > because we need to be able to send vlan-only notifications and do larger > vlan netlink dumps. Per-vlan options are needed as we move the control > more to vlans and would like to add per-vlan state (needed for per-vlan > STP and EVPN), per-vlan multicast options and control, and I'm sure > there would be many more per-vlan options coming. ... Series applied, thanks Nikolay.
Re: [Bridge] [RFC net-next Patch 0/3] net: bridge: mrp: Add support for Media Redundancy Protocol(MRP)
From: Nikolay Aleksandrov Date: Fri, 10 Jan 2020 16:13:36 +0200 > I agree with Stephen here, IMO you have to take note of how STP has progressed > and that bringing it in the kernel was a mistake, these days mstpd has an > active > community and much better support which is being extended. This looks best > implemented > in user-space in my opinion with minimal kernel changes to support it. You > could simply > open a packet socket with a filter and work through that, you don't need new > netlink > sockets. I'm not familiar with the protocol so can't really be the judge of > that, if > you present a good argument for needing a new netlink socket for these > packets - then > sure, ok. With a userland implementation, what approach do you suggest for DSA/switchdev offload of this stuff?
Re: [Bridge] [PATCH net-next v2] net: bridge: add STP xstats
From: Vivien Didelot Date: Wed, 11 Dec 2019 16:47:54 -0500 > I thought the whole point of using enums was to avoid caring about fixed > numeric values, but well. I don't get where you got that idea from. Each and every netlink attribute value is like IPPROTO_TCP in an ipv4 header, the exact values matter, and therefore you cannot make changes that modify existing values. Therefore, you must add new attributes to the end of the enumberation, right before the __MAX value. > To be more precise, what I don't get is that when > I move the BRIDGE_XSTATS_STP definition *after* BRIDGE_XSTATS_PAD, the STP > xstats don't show up anymore in iproute2. Because you ahve to recompile iproute2 so that it uses the corrected value in the kernel header, did you do that?
Re: [Bridge] [PATCH net-next v2] net: bridge: add STP xstats
From: Vivien Didelot
Date: Wed, 11 Dec 2019 13:41:33 -0500
> Hi David, Nikolay,
>
> On Wed, 11 Dec 2019 17:42:33 +0200, Nikolay Aleksandrov
> wrote:
>> >> /* Bridge multicast database attributes
>> >> * [MDBA_MDB] = {
>> >> * [MDBA_MDB_ENTRY] = {
>> >> @@ -261,6 +270,7 @@ enum {
>> >> BRIDGE_XSTATS_UNSPEC,
>> >> BRIDGE_XSTATS_VLAN,
>> >> BRIDGE_XSTATS_MCAST,
>> >> + BRIDGE_XSTATS_STP,
>> >> BRIDGE_XSTATS_PAD,
>> >> __BRIDGE_XSTATS_MAX
>> >> };
>> >
>> > Shouldn't the new entry be appended to the end - after BRIDGE_XSTATS_PAD
>> >
>>
>> Oh yes, good catch. That has to be fixed, too.
>>
>
> This I don't get. Why new attributes must come between BRIDGE_XSTATS_PAD
> and __BRIDGE_XSTATS_MAX?
Because, just like any other attribute value, BRIDGE_XSTATS_PAD is an
API and fixed in stone. You can't add things before it which change
it's value.
Re: [Bridge] [PATCH net] net: bridge: deny dev_set_mac_address() when unregistering
From: Nikolay Aleksandrov
Date: Tue, 3 Dec 2019 16:48:06 +0200
> We have an interesting memory leak in the bridge when it is being
> unregistered and is a slave to a master device which would change the
> mac of its slaves on unregister (e.g. bond, team). This is a very
> unusual setup but we do end up leaking 1 fdb entry because
> dev_set_mac_address() would cause the bridge to insert the new mac address
> into its table after all fdbs are flushed, i.e. after dellink() on the
> bridge has finished and we call NETDEV_UNREGISTER the bond/team would
> release it and will call dev_set_mac_address() to restore its original
> address and that in turn will add an fdb in the bridge.
> One fix is to check for the bridge dev's reg_state in its
> ndo_set_mac_address callback and return an error if the bridge is not in
> NETREG_REGISTERED.
>
> Easy steps to reproduce:
> 1. add bond in mode != A/B
> 2. add any slave to the bond
> 3. add bridge dev as a slave to the bond
> 4. destroy the bridge device
>
> Trace:
> unreferenced object 0x888035c4d080 (size 128):
...
> Fixes: 43598813386f ("bridge: add local MAC address to forwarding table (v2)")
> Reported-by: syzbot+2add91c08eb181fea...@syzkaller.appspotmail.com
> Signed-off-by: Nikolay Aleksandrov
Looks good, applied and queued up for -stable.
Thanks.
Re: [Bridge] [PATCH net-next] net: bridge: fdb: eliminate extra port state tests from fast-path
From: Nikolay Aleksandrov
Date: Mon, 4 Nov 2019 11:36:51 +0200
> When commit df1c0b8468b3 ("[BRIDGE]: Packets leaking out of
> disabled/blocked ports.") introduced the port state tests in
> br_fdb_update() it was to avoid learning/refreshing from STP BPDUs, it was
> also used to avoid learning/refreshing from user-space with NTF_USE. Those
> two tests are done for every packet entering the bridge if it's learning,
> but for the fast-path we already have them checked in br_handle_frame() and
> is unnecessary to do it again. Thus push the checks to the unlikely cases
> and drop them from br_fdb_update(), the new nbp_state_should_learn() helper
> is used to determine if the port state allows br_fdb_update() to be called.
> The two places which need to do it manually are:
> - user-space add call with NTF_USE set
> - link-local packet learning done in __br_handle_local_finish()
>
> Signed-off-by: Nikolay Aleksandrov
Applied, thank you.
Re: [Bridge] [PATCH net-next 0/7] net: bridge: convert fdbs to use bitops
From: Nikolay Aleksandrov Date: Tue, 29 Oct 2019 13:45:52 +0200 > We'd like to have a well-defined behaviour when changing fdb flags. The > problem is that we've added new fields which are changed from all > contexts without any locking. We are aware of the bit test/change races > and these are fine (we can remove them later), but it is considered > undefined behaviour to change bitfields from multiple threads and also > on some architectures that can result in unexpected results, > specifically when all fields between the changed ones are also > bitfields. The conversion to bitops shows the intent clearly and > makes them use functions with well-defined behaviour in such cases. > There is no overhead for the fast-path, the bit changing functions are > used only in special cases when learning and in the slow path. > In addition this conversion allows us to simplify fdb flag handling and > avoid bugs for future bits (e.g. a forgetting to clear the new bit when > allocating a new fdb). All bridge selftests passed, also tried all of the > converted bits manually in a VM. Series applied, thanks.
Re: [Bridge] [PATCH net v2] bridge/mdb: remove wrong use of NLM_F_MULTI
From: Nicolas Dichtel
Date: Fri, 6 Sep 2019 11:47:02 +0200
> NLM_F_MULTI must be used only when a NLMSG_DONE message is sent at the end.
> In fact, NLMSG_DONE is sent only at the end of a dump.
>
> Libraries like libnl will wait forever for NLMSG_DONE.
>
> Fixes: 949f1e39a617 ("bridge: mdb: notify on router port add and del")
> CC: Nikolay Aleksandrov
> Signed-off-by: Nicolas Dichtel
> Acked-by: Nikolay Aleksandrov
Applied and queued up for -stable.
Re: [Bridge] [PATCH v2 0/3] Add NETIF_F_HW_BR_CAP feature
From: Andrew Lunn Date: Mon, 26 Aug 2019 14:38:11 +0200 > I'm still not convinced this is needed. The model is, the hardware is > there to accelerate what Linux can do in software. Any peculiarities > of the accelerator should be hidden in the driver. If the accelerator > can do its job without needing promisc mode, do that in the driver. I completely agree.
Re: [PATCH 0/3] Add NETIF_F_HW_BRIDGE feature
From: Horatiu Vultur Date: Thu, 22 Aug 2019 21:07:27 +0200 > Current implementation of the SW bridge is setting the interfaces in > promisc mode when they are added to bridge if learning of the frames is > enabled. > In case of Ocelot which has HW capabilities to switch frames, it is not > needed to set the ports in promisc mode because the HW already capable of > doing that. Therefore add NETIF_F_HW_BRIDGE feature to indicate that the > HW has bridge capabilities. Therefore the SW bridge doesn't need to set > the ports in promisc mode to do the switching. > This optimization takes places only if all the interfaces that are part > of the bridge have this flag and have the same network driver. > > If the bridge interfaces is added in promisc mode then also the ports part > of the bridge are set in promisc mode. This doesn't look right at all. The Linux bridge provides a software bridge. By default, all hardware must provide a hardware implementation of that software bridge behavior. Anything that deviates from that behavior has to be explicitly asked for by the user by explicit config commands.
Re: [Bridge] [PATCH net-next v3 0/4] net: bridge: mdb: allow dump/add/del of host-joined entries
From: Nikolay Aleksandrov Date: Sat, 17 Aug 2019 14:22:09 +0300 > This set makes the bridge dump host-joined mdb entries, they should be > treated as normal entries since they take a slot and are aging out. > We already have notifications for them but we couldn't dump them until > now so they remained hidden. We dump them similar to how they're > notified, in order to keep user-space compatibility with the dumped > objects (e.g. iproute2 dumps mdbs in a format which can be fed into > add/del commands) we allow host-joined groups also to be added/deleted via > mdb commands. That can later be used for L2 mcast MAC manipulation as > was recently discussed. Note that iproute2 changes are not necessary, > this set will work with the current user-space mdb code. > > Patch 01 - a trivial comment move > Patch 02 - factors out the mdb filling code so it can be >re-used for the host-joined entries > Patch 03 - dumps host-joined entries > Patch 04 - allows manipulation of host-joined entries via standard mdb >calls > > v3: fix compiler warning in patch 04 (DaveM) > v2: change patch 04 to avoid double notification and improve host group > manual removal if no ports are present in the group Series applied.
Re: [PATCH net-next v2 0/4] net: bridge: mdb: allow dump/add/del of host-joined entries
From: Nikolay Aleksandrov Date: Wed, 14 Aug 2019 20:04:57 +0300 > This set makes the bridge dump host-joined mdb entries, they should be > treated as normal entries since they take a slot and are aging out. ... Please respin with this warning fixed: net/bridge/br_mdb.c: In function ‘br_mdb_add’: net/bridge/br_mdb.c:725:4: warning: ‘p’ may be used uninitialized in this function [-Wmaybe-uninitialized] __br_mdb_notify(dev, p, entry, RTM_NEWMDB); ^~ [davem@localhost net-next]$ gcc --version gcc (GCC) 8.3.1 20190223 (Red Hat 8.3.1-2) Copyright (C) 2018 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Re: [Bridge] [PATCH net v4] net: bridge: move default pvid init/deinit to NETDEV_REGISTER/UNREGISTER
From: Nikolay Aleksandrov
Date: Fri, 2 Aug 2019 13:57:36 +0300
> Most of the bridge device's vlan init bugs come from the fact that its
> default pvid is created at the wrong time, way too early in ndo_init()
> before the device is even assigned an ifindex. It introduces a bug when the
> bridge's dev_addr is added as fdb during the initial default pvid creation
> the notification has ifindex/NDA_MASTER both equal to 0 (see example below)
> which really makes no sense for user-space[0] and is wrong.
> Usually user-space software would ignore such entries, but they are
> actually valid and will eventually have all necessary attributes.
> It makes much more sense to send a notification *after* the device has
> registered and has a proper ifindex allocated rather than before when
> there's a chance that the registration might still fail or to receive
> it with ifindex/NDA_MASTER == 0. Note that we can remove the fdb flush
> from br_vlan_flush() since that case can no longer happen. At
> NETDEV_REGISTER br->default_pvid is always == 1 as it's initialized by
> br_vlan_init() before that and at NETDEV_UNREGISTER it can be anything
> depending why it was called (if called due to NETDEV_REGISTER error
> it'll still be == 1, otherwise it could be any value changed during the
> device life time).
>
> For the demonstration below a small change to iproute2 for printing all fdb
> notifications is added, because it contained a workaround not to show
> entries with ifindex == 0.
> Command executed while monitoring: $ ip l add br0 type bridge
> Before (both ifindex and master == 0):
> $ bridge monitor fdb
> 36:7e:8a:b3:56:ba dev * vlan 1 master * permanent
>
> After (proper br0 ifindex):
> $ bridge monitor fdb
> e6:2a:ae:7a:b7:48 dev br0 vlan 1 master br0 permanent
>
> v4: move only the default pvid init/deinit to NETDEV_REGISTER/UNREGISTER
> v3: send the correct v2 patch with all changes (stub should return 0)
> v2: on error in br_vlan_init set br->vlgrp to NULL and return 0 in
> the br_vlan_bridge_event stub when bridge vlans are disabled
>
> [0] https://bugzilla.kernel.org/show_bug.cgi?id=204389
>
> Reported-by: michael-dev
> Fixes: 5be5a2df40f0 ("bridge: Add filtering support for default_pvid")
> Signed-off-by: Nikolay Aleksandrov
Applied and queued up for -stable, thanks.
Re: [PATCH net-next] net: bridge: mcast: add delete due to fast-leave mdb flag
From: Nikolay Aleksandrov Date: Tue, 30 Jul 2019 15:20:41 +0300 > In user-space there's no way to distinguish why an mdb entry was deleted > and that is a problem for daemons which would like to keep the mdb in > sync with remote ends (e.g. mlag) but would also like to converge faster. > In almost all cases we'd like to age-out the remote entry for performance > and convergence reasons except when fast-leave is enabled. In that case we > want explicit immediate remote delete, thus add mdb flag which is set only > when the entry is being deleted due to fast-leave. > > Signed-off-by: Nikolay Aleksandrov Applied, thanks Nikolay.
Re: [PATCH net] net: bridge: mcast: don't delete permanent entries when fast leave is enabled
From: Nikolay Aleksandrov
Date: Tue, 30 Jul 2019 14:21:00 +0300
> When permanent entries were introduced by the commit below, they were
> exempt from timing out and thus igmp leave wouldn't affect them unless
> fast leave was enabled on the port which was added before permanent
> entries existed. It shouldn't matter if fast leave is enabled or not
> if the user added a permanent entry it shouldn't be deleted on igmp
> leave.
...
> Fixes: ccb1c31a7a87 ("bridge: add flags to distinguish permanent mdb entires")
> Signed-off-by: Nikolay Aleksandrov
Applied and queued up for -stable.
Re: [PATCH net] net: bridge: mcast: don't delete permanent entries when fast leave is enabled
From: Nikolay Aleksandrov Date: Tue, 30 Jul 2019 14:21:00 +0300 > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c > index 3d8deac2353d..f8cac3702712 100644 > --- a/net/bridge/br_multicast.c > +++ b/net/bridge/br_multicast.c > @@ -1388,6 +1388,9 @@ br_multicast_leave_group(struct net_bridge *br, > if (!br_port_group_equal(p, port, src)) > continue; > > + if (p->flags & MDB_PG_FLAGS_PERMANENT) > + break; > + Like David, I also don't understand why this can be a break. Is it because permanent entries are always the last on the list? Why will there be no other entries that might need to be processed on the list?
Re: [PATCH net v2] net: bridge: delete local fdb on device init failure
From: Nikolay Aleksandrov
Date: Mon, 29 Jul 2019 12:28:41 +0300
> On initialization failure we have to delete the local fdb which was
> inserted due to the default pvid creation. This problem has been present
> since the inception of default_pvid. Note that currently there are 2 cases:
> 1) in br_dev_init() when br_multicast_init() fails
> 2) if register_netdevice() fails after calling ndo_init()
>
> This patch takes care of both since br_vlan_flush() is called on both
> occasions. Also the new fdb delete would be a no-op on normal bridge
> device destruction since the local fdb would've been already flushed by
> br_dev_delete(). This is not an issue for ports since nbp_vlan_init() is
> called last when adding a port thus nothing can fail after it.
>
> Reported-by: syzbot+88533dc8b582309bf...@syzkaller.appspotmail.com
> Fixes: 5be5a2df40f0 ("bridge: Add filtering support for default_pvid")
> Signed-off-by: Nikolay Aleksandrov
> ---
> v2: reworded the commit message and comment so they're not plural, we're
> talking about a single bridge local fdb added on the init vlan creation
> of the default pvid
>
> Tested with the provided reproducer and can no longer trigger the leak.
> Also tested the br_multicast_init() failure manually by making it always
> return an error.
Applied and queued up for -stable, thank you.
Re: [PATCH net 0/4] net: bridge: fix possible stale skb pointers
From: Nikolay Aleksandrov Date: Tue, 2 Jul 2019 15:00:17 +0300 > In the bridge driver we have a couple of places which call pskb_may_pull > but we've cached skb pointers before that and use them after which can > lead to out-of-bounds/stale pointer use. I've had these in my "to fix" > list for some time and now we got a report (patch 01) so here they are. > Patches 02-04 are fixes based on code inspection. Also patch 01 was > tested by Martin Weinelt, Martin if you don't mind please add your > tested-by tag to it by replying with Tested-by: name . > I've also briefly tested the set by trying to exercise those code paths. Series applied, thanks.
Re: [Bridge] [PATCH v2] bridge: Fix error path for kobject_init_and_add()
From: "Tobin C. Harding" Date: Fri, 10 May 2019 12:52:12 +1000 > Currently error return from kobject_init_and_add() is not followed by a > call to kobject_put(). This means there is a memory leak. We currently > set p to NULL so that kfree() may be called on it as a noop, the code is > arguably clearer if we move the kfree() up closer to where it is > called (instead of after goto jump). > > Remove a goto label 'err1' and jump to call to kobject_put() in error > return from kobject_init_and_add() fixing the memory leak. Re-name goto > label 'put_back' to 'err1' now that we don't use err1, following current > nomenclature (err1, err2 ...). Move call to kfree out of the error > code at bottom of function up to closer to where memory was allocated. > Add comment to clarify call to kfree(). > > Signed-off-by: Tobin C. Harding > --- > > v1 was a part of a set. I have dropped the other patch until I can work > out a correct solution. Applied and queued up for -stable, thanks.
Re: [Bridge] [PATCH net] net: bridge: fix netlink export of vlan_stats_per_port option
From: Nikolay Aleksandrov
Date: Tue, 16 Apr 2019 16:15:56 +0300
> Since the introduction of the vlan_stats_per_port option the netlink
> export of it has been broken since I made a typo and used the ifla
> attribute instead of the bridge option to retrieve its state.
> Sysfs export is fine, only netlink export has been affected.
>
> Fixes: 9163a0fc1f0c0 ("net: bridge: add support for per-port vlan stats")
> Signed-off-by: Nikolay Aleksandrov
Applied and queued up for -stable.
Re: [Bridge] [PATCH net] net: bridge: fix per-port af_packet sockets
From: Nikolay Aleksandrov
Date: Thu, 11 Apr 2019 13:56:39 +0300
> When the commit below was introduced it changed two visible things:
> - the skb was no longer passed through the protocol handlers with the
>original device
> - the skb was passed up the stack with skb->dev = bridge
>
> The first change broke af_packet sockets on bridge ports. For example we
> use them for hostapd which listens for ETH_P_PAE packets on the ports.
> We discussed two possible fixes:
> - create a clone and pass it through NF_HOOK(), act on the original skb
>based on the result
> - somehow signal to the caller from the okfn() that it was called,
>meaning the skb is ok to be passed, which this patch is trying to
>implement via returning 1 from the bridge link-local okfn()
>
> Note that we rely on the fact that NF_QUEUE/STOLEN would return 0 and
> drop/error would return < 0 thus the okfn() is called only when the
> return was 1, so we signal to the caller that it was called by preserving
> the return value from nf_hook().
>
> Fixes: 8626c56c8279 ("bridge: fix potential use-after-free when hook returns
> QUEUE or STOLEN verdict")
> Signed-off-by: Nikolay Aleksandrov
> ---
> I plan to make a selftest for this one too. Once we agree on the fix.
This seems reasonable, applied and queued up for -stable, thanks Nikolay.
Re: [Bridge] [PATCH net] net: bridge: multicast: use rcu to access port list from br_multicast_start_querier
From: Nikolay Aleksandrov
Date: Thu, 11 Apr 2019 15:08:25 +0300
> br_multicast_start_querier() walks over the port list but it can be
> called from a timer with only multicast_lock held which doesn't protect
> the port list, so use RCU to walk over it.
>
> Fixes: c83b8fab06fc ("bridge: Restart queries when last querier expires")
> Signed-off-by: Nikolay Aleksandrov
Applied and queued up for -stable, thanks.
Re: [Bridge] [PATCH net-next] net: bridge: mcast: remove unused br_ip_equal function
From: Nikolay Aleksandrov Date: Wed, 3 Apr 2019 23:44:18 +0300 > Since the mcast conversion to rhashtable this function has been unused, so > remove it. > > Signed-off-by: Nikolay Aleksandrov Another reason to not use inline functions in foo.c files :-) Applied, thanks Nikolay.
Re: [Bridge] [PATCH net] net: bridge: always clear mcast matching struct on reports and leaves
From: Nikolay Aleksandrov
Date: Wed, 3 Apr 2019 23:27:24 +0300
> We need to be careful and always zero the whole br_ip struct when it is
> used for matching since the rhashtable change. This patch fixes all the
> places which didn't properly clear it which in turn might've caused
> mismatches.
>
> Thanks for the great bug report with reproducing steps and bisection.
...
> Reported-by: liam.mcbir...@boeing.com
> Fixes: 19e3a9c90c53 ("net: bridge: convert multicast to generic rhashtable")
> Signed-off-by: Nikolay Aleksandrov
Applied and queued up for -stable.
> I will cook a selftest for this as well.
Thanks in advance for this, that's great.
Re: [Bridge] [PATCH net-next] net: bridge: optimize backup_port fdb convergence
From: Nikolay Aleksandrov Date: Wed, 3 Apr 2019 13:49:24 +0300 > We can optimize the fdb convergence when a backup_port is present by not > immediately flushing the entries of the stopped port since traffic for > those entries will flow towards the backup_port. > > There are 2 cases specifically that benefit most: > - when the stopped port comes up before the entries expire by themselves > - when there's an external entry refresh and they're kept while the > backup_port is operating (e.g. mlag) > > Signed-off-by: Nikolay Aleksandrov Applied.
Re: [Bridge] [PATCH net-next] net: bridge: update multicast stats from maybe_deliver()
From: Pablo Neira Ayuso
Date: Thu, 4 Apr 2019 13:56:38 +0200
> Simplify this code by updating bridge multicast stats from
> maybe_deliver().
>
> Note that commit 6db6f0eae605 ("bridge: multicast to unicast"), in case
> the port flag BR_MULTICAST_TO_UNICAST is set, never updates the previous
> port pointer, therefore it is always going to be different from the
> existing port in this deduplicated list iteration.
>
> Signed-off-by: Pablo Neira Ayuso
Applied.
Re: [Bridge] [PATCH net-next] net: bridge: use eth_broadcast_addr() to assign broadcast address
From: Mao Wenan Date: Wed, 20 Mar 2019 10:06:57 +0800 > This patch is to use eth_broadcast_addr() to assign broadcast address > insetad of memset(). > > Signed-off-by: Mao Wenan Applied.
Re: [Bridge] [PATCH net-next] switchdev: Remove unused transaction item queue
From: Florian Fainelli
Date: Wed, 27 Feb 2019 16:29:16 -0800
> There are no more in tree users of the
> switchdev_trans_item_{dequeue,enqueue} or switchdev_trans_item structure
> in the kernel since commit 00fc0c51e35b ("rocker: Change world_ops API
> and implementation to be switchdev independant").
>
> Remove this unused code and update the documentation accordingly since.
>
> Signed-off-by: Florian Fainelli
Applied, thanks Florian.
Re: [Bridge] [PATCH net-next v3 0/8] net: Remove switchdev_ops
From: Florian Fainelli Date: Wed, 27 Feb 2019 11:44:24 -0800 > This patch series completes the removal of the switchdev_ops by > converting switchdev_port_attr_set() to use either the blocking > (process) or non-blocking (atomic) notifier since we typically need to > deal with both depending on where in the bridge code we get called from. > > This was tested with the forwarding selftests and DSA hardware. > > Ido, hopefully this captures your comments done on v1, if not, can you > illustrate with some pseudo-code what you had in mind if that's okay? > > Changes in v3: > > - added Reviewed-by tags from Ido where relevant > - added missing notifier_to_errno() in net/bridge/br_switchdev.c when > calling the atomic notifier for PRE_BRIDGE_FLAGS > - kept mlxsw_sp_switchdev_init() in mlxsw/ > > Changes in v2: > > - do not check for SWITCHDEV_F_DEFER when calling the blocking notifier > and instead directly call the atomic notifier from the single location > where this is required Series applied, thanks Florian. I'll push this out after my build tests complete.
Re: [Bridge] [PATCH net] Revert "bridge: do not add port to router list when receives query with source 0.0.0.0"
From: Hangbin Liu
Date: Fri, 22 Feb 2019 21:22:32 +0800
> This reverts commit 5a2de63fd1a5 ("bridge: do not add port to router list
> when receives query with source 0.0.0.0") and commit 0fe5119e267f ("net:
> bridge: remove ipv6 zero address check in mcast queries")
>
> The reason is RFC 4541 is not a standard but suggestive. Currently we
> will elect 0.0.0.0 as Querier if there is no ip address configured on
> bridge. If we do not add the port which recives query with source
> 0.0.0.0 to router list, the IGMP reports will not be about to forward
> to Querier, IGMP data will also not be able to forward to dest.
>
> As Nikolay suggested, revert this change first and add a boolopt api
> to disable none-zero election in future if needed.
>
> Reported-by: Linus Lüssing
> Reported-by: Sebastian Gottschall
> Fixes: 5a2de63fd1a5 ("bridge: do not add port to router list when receives
> query with source 0.0.0.0")
> Fixes: 0fe5119e267f ("net: bridge: remove ipv6 zero address check in mcast
> queries")
> Signed-off-by: Hangbin Liu
Applied and queued up for -stable, thanks.
Re: [Bridge] [PATCH net-next v2 0/9] net: Get rid of switchdev_port_attr_get()
From: Florian Fainelli Date: Fri, 15 Feb 2019 16:37:38 -0800 > David, please ignore this version, I will repost one that actually > builds, need to keep mangling with my kernel configuration and keep > those drivers enabled... Ok.
Re: [Bridge] [PATCH net-next 0/3] Remove getting SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS
From: Florian Fainelli Date: Mon, 11 Feb 2019 13:17:46 -0800 > AFAICT there is no code that attempts to get the value of the attribute > SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS while it is used with > switchdev_port_attr_set(). > > This is effectively no doing anything and it can slow down future work > that tries to make modifications in these areas so remove that. Series applied. > David, there should be no dependency with previous patch series, but > again, feedback from Ido and Jiri would be welcome in case this was > added for a reason. Ok, is there going to be another respin of that switchdev_ops removal series?
Re: [Bridge] [PATCH net-next v4 0/9] net: Remove switchdev_ops
From: Florian Fainelli Date: Mon, 11 Feb 2019 11:09:52 -0800 > David, I would like to get Ido's feedback on this to make sure I did not > miss something, thank you! Ok, Ido please look at this when you can.
Re: [Bridge] [PATCH net-next v3 0/9] net: Remove switchdev_ops
From: Florian Fainelli Date: Mon, 11 Feb 2019 09:31:42 -0800 > David, I will be reposting a v4 with Jiri's Acked-by as well as adding > fallthrough switch/case annotations so we don't regress on that front. > Thank you. Ok, thanks for letting me know.
Re: [Bridge] [PATCH net-next v2 00/16] net: Remove switchdev_ops
From: Florian Fainelli
Date: Sun, 10 Feb 2019 12:44:47 -0800
> I am going to submit a v3 which moves the port_attr_{get,set} to a
> switchdev notifier, but does not yet get rid of
> switchdev_port_attr_get() entirely since there is not a clear path yet
> to split the setting between non-sleeping and sleeping context.
Ok.
Re: [Bridge] [PATCH net-next] bridge: use struct_size() helper
From: "Gustavo A. R. Silva"
Date: Thu, 7 Feb 2019 18:58:56 -0600
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
>
> struct foo {
> int stuff;
> struct boo entry[];
> };
>
> size = sizeof(struct foo) + count * sizeof(struct boo);
> instance = alloc(size, GFP_KERNEL)
>
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
>
> size = struct_size(instance, entry, count);
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva
Applied.
Re: [Bridge] [PATCH net-next v4 00/12] net: Introduce ndo_get_port_parent_id()
From: David Miller Date: Wed, 06 Feb 2019 13:50:50 -0800 (PST) > From: Florian Fainelli > Date: Wed, 6 Feb 2019 09:45:34 -0800 > >> Based on discussion with Ido and feedback from Jakub there are clearly >> two classes of users that implement SWITCHDEV_ATTR_ID_PORT_PARENT_ID: >> >> - PF/VF drivers which typically only implement return the port's parent >> ID, yet have to implement switchdev_port_attr_get() just for that >> >> - Ethernet switch drivers: mlxsw, ocelot, DSA, etc. which implement more >> attributes which we want to be able to eventually veto in the context >> of the caller, thus making them candidates for using a blocking notifier >> chain >> >> Changes in v4: > .. > > Series applied, thanks Florian. > > I'll push this out to net-next after my build tests complete. I had to remove the unused variable 'rocker' to kill a warning introduced by patch #8. Just FYI...
Re: [Bridge] [PATCH net-next v4 00/12] net: Introduce ndo_get_port_parent_id()
From: Florian Fainelli Date: Wed, 6 Feb 2019 09:45:34 -0800 > Based on discussion with Ido and feedback from Jakub there are clearly > two classes of users that implement SWITCHDEV_ATTR_ID_PORT_PARENT_ID: > > - PF/VF drivers which typically only implement return the port's parent > ID, yet have to implement switchdev_port_attr_get() just for that > > - Ethernet switch drivers: mlxsw, ocelot, DSA, etc. which implement more > attributes which we want to be able to eventually veto in the context > of the caller, thus making them candidates for using a blocking notifier > chain > > Changes in v4: .. Series applied, thanks Florian. I'll push this out to net-next after my build tests complete.
Re: [Bridge] [PATCH net-next] net: Fix ip_mc_{dec, inc}_group allocation context
From: Florian Fainelli
Date: Fri, 1 Feb 2019 20:20:52 -0800
> After 4effd28c1245 ("bridge: join all-snoopers multicast address"), I
> started seeing the following sleep in atomic warnings:
...
> while toggling the bridge's multicast_snooping attribute dynamically.
>
> Pass a gfp_t down to igmpv3_add_delrec(), introduce
> __igmp_group_dropped() and introduce __ip_mc_dec_group() to take a gfp_t
> argument.
>
> Similarly introduce ip_mc_inc_group() and __ip_mc_inc_group() to
> allow caller to specify gfp_t.
>
> IPv6 part of the patch appears fine.
>
> Fixes: 4effd28c1245 ("bridge: join all-snoopers multicast address")
> Signed-off-by: Florian Fainelli
Applied.
Maybe you could submit a quick test case since you had a reproducer?
Re: [Bridge] [PATCH net-next] bridge: remove duplicated include from br_multicast.c
From: YueHaibing Date: Fri, 25 Jan 2019 10:59:09 +0800 > Remove duplicated include. > > Signed-off-by: YueHaibing Applied.
Re: [Bridge] [PATCH net v4] net: bridge: Fix ethernet header pointer before check skb forwardable
From: wangyunjian
Date: Thu, 17 Jan 2019 09:46:41 +0800
> From: Yunjian Wang
>
> The skb header should be set to ethernet header before using
> is_skb_forwardable. Because the ethernet header length has been
> considered in is_skb_forwardable(including dev->hard_header_len
> length).
>
> To reproduce the issue:
> 1, add 2 ports on linux bridge br using following commands:
> $ brctl addbr br
> $ brctl addif br eth0
> $ brctl addif br eth1
> 2, the MTU of eth0 and eth1 is 1500
> 3, send a packet(Data 1480, UDP 8, IP 20, Ethernet 14, VLAN 4)
> from eth0 to eth1
>
> So the expect result is packet larger than 1500 cannot pass through
> eth0 and eth1. But currently, the packet passes through success, it
> means eth1's MTU limit doesn't take effect.
>
> Fixes: f6367b4660dd ("bridge: use is_skb_forwardable in forward path")
> Cc: bridge@lists.linux-foundation.org
> Cc: Nkolay Aleksandrov
> Cc: Roopa Prabhu
> Cc: Stephen Hemminger
> Signed-off-by: Yunjian Wang
Applied and queued up for -stable.
Re: [Bridge] [PATCH net] net: rtnetlink: address is mandatory for rtnl_fdb_get
From: Nikolay Aleksandrov
Date: Sun, 30 Dec 2018 14:33:20 +0200
> We must have an address to lookup otherwise we'll derefence a null
> pointer in the ndo_fdb_get callbacks.
>
> CC: Roopa Prabhu
> CC: David Ahern
> Reported-by: syzbot+017b1f61c82a1c3e7...@syzkaller.appspotmail.com
> Fixes: 5b2f94b27622 ("net: rtnetlink: support for fdb get")
> Signed-off-by: Nikolay Aleksandrov
Applied, thanks.
Re: [Bridge] [PATCH net-next] net: bridge: remove unneeded variable 'err'
From: YueHaibing Date: Mon, 17 Dec 2018 17:46:23 +0800 > function br_multicast_toggle now always return 0, > so the variable 'err' is unneeded. > Also cleanup dead branch in br_changelink. > > Signed-off-by: YueHaibing Applied.
Re: [Bridge] [PATCH net-next] Documentation: networking: Clarify switchdev devices behavior
From: Florian Fainelli Date: Wed, 12 Dec 2018 15:09:43 -0800 > This patch provides details on the expected behavior of switchdev > enabled network devices when operating in a "stand alone" mode, as well > as when being bridge members. This clarifies a number of things that > recently came up during a bug fixing session on the b53 DSA switch > driver. > > Signed-off-by: Florian Fainelli > --- > Hi all, > > Please review carefully, and let me know if you think some of the > behaviors described below do not make any sense. Thanks! Looks like Andrew had some feedback, so I'm expecting a new version of this.
Re: [Bridge] [PATCH net-next v2 0/9] Pass extack to SWITCHDEV_PORT_OBJ_ADD
From: Petr Machata Date: Wed, 12 Dec 2018 17:02:46 + > Drivers may need to do validation as a result of port object addition. > An example is mlxsw, which needs to check the configuration of a VXLAN > device attached to an offloaded bridge. Without a mapped VLAN, the > invalidity of the device is not important, but as soon as a pvid, > untagged VLAN is configured for the device, it has to be validated and > offloaded. Should the validation fail, there's currently no way to > communicate details of the failure to the user, beyond an error number. > > Because currently, extack is not available at all in that area of code, > this patch starts down at the RTNL level and progresses up towards the > driver(s). > > In patch #1, ndo_bridge_setlink is updated to include extack, and > callbacks of all clients are updated as well (ignoring the argument). > > In patch #2, the bridge driver is updated to propagate the extack > through to the switchdev border, br_switchdev_port_vlan_add(). > > Patches #3, #4 and #5 then gradually extend switchdev to pass the extack > argument through to the switchdev blocking notifier chain. > > Patches #6 and #7 then update mlxsw to pass the extack argument from > VXLAN events resp. port events on to mlxsw_sp_bridge_8021q_vxlan_join(). > > Finally in patches #8 and #9, the code paths from the previous two > patches are verified to yield an error message. > > v2: > - Patch #1: > - In ndo_bridge_setlink(), keep the whole extack declaration on the > same line. Series applied, thanks Petr.
Re: [Bridge] [PATCH net-next v2 00/12] mlxsw: Un/offload FDB on NVE detach/attach
From: Ido Schimmel Date: Fri, 7 Dec 2018 19:55:01 + > Petr says: > > When a VXLAN device is attached to a bridge of a driver capable of > offloading such, or upped, the FDB entries already present at the device > need to be offloaded. Similarly when an offloaded VXLAN device ceases > being interesting (it is downed, or detached, or a front-panel port > netdevice is detached from the bridge that the VXLAN device is attached > to), any offloaded FDB entries need to be unoffloaded and unmarked. This > attach / detach processing is implemented in this patchset. > > In patch #1, a code pattern is extracted into a named function for > easier reuse. > > In patch #2, vxlan_fdb_replay() is added to send > SWITCHDEV_VXLAN_FDB_ADD_TO_DEVICE for each FDB entry with a given VNI. > The intention is that the offloading driver will interpret these events > like any other and thus offload the FDB entries that existed prior to > VXLAN attach. > > In patches #3 and #4, the functions vxlan_fdb_clear_offload() resp. > br_fdb_clear_offload() are added. These clear the offloaded flag at > matching FDB entries. > > In patches #5-#9, we introduce FID-type-specific and NVE-type-specific > ops necessary to properly abstract invocations of the replay/clear > functions. > > Finally patch #10 implements the FDB management. > > In patch #11, the mlxsw-specific test case is extended to check that the > management of offload marks under the newly-supported situations is > correct. Patch #12, from Ido, exercises the new code paths in actual > functional test. ... Series applied, thanks.
Re: [Bridge] [PATCH net-next 01/12] vxlan: Add a function to init switchdev_notifier_vxlan_fdb_info
From: Ido Schimmel
Date: Wed, 5 Dec 2018 15:50:23 +
> +static struct switchdev_notifier_vxlan_fdb_info
> +vxlan_fdb_switchdev_notifier_info(const struct vxlan_dev *vxlan,
> + const struct vxlan_fdb *fdb,
> + const struct vxlan_rdst *rd)
> +{
> + struct switchdev_notifier_vxlan_fdb_info fdb_info = {
> + .info.dev = vxlan->dev,
> + .remote_ip = rd->remote_ip,
> + .remote_port = rd->remote_port,
> + .remote_vni = rd->remote_vni,
> + .remote_ifindex = rd->remote_ifindex,
> + .vni = fdb->vni,
> + .offloaded = rd->offloaded,
> + .added_by_user = fdb->flags & NTF_VXLAN_ADDED_BY_USER,
> + };
> +
> + memcpy(fdb_info.eth_addr, fdb->eth_addr, ETH_ALEN);
> + return fdb_info;
> +}
Never return a structure by value from a function. Do you have any idea
how the compiler implement this?
In the one call site in vxlan.c after this patch is applied, _two_
switchdev_notifier_cxlan_fdb_info objects are allocated on the stack.
vxlan_fdb_swtich_dev_notifier_info() fills in the first one, and
then the caller copies that into the second one.
A huge waste.
Please just pass "" as an argument.
Re: [Bridge] [PATCH net-next v3 0/4] net: bridge: convert multicast to generic rhashtable
From: Nikolay Aleksandrov Date: Wed, 5 Dec 2018 15:14:23 +0200 > The current bridge multicast code uses a custom rhashtable > implementation which predates the generic rhashtable API. Patch 01 > converts it to use the generic kernel rhashtable which simplifies the > code a lot and removes duplicated functionality. The convert also makes > hash_elasticity obsolete as the generic rhashtable already has such > checks and has a fixed elasticity of RHT_ELASTICITY (16 currently) so we > emit a warning whenever elasticity is set and return RHT_ELASTICITY when > read (patch 03). Patch 02 converts the multicast code to use non-bh RCU > flavor as it was mixing bh and non-bh. Since now we have the generic > rhashtable which autoshrinks we can be more liberal with the default > hash maximum so patch 04 increases it to 4096 and moves it to a define in > br_private.h. > > v3: add non-rcu br_mdb_get variant and use it where we have > multicast_lock, drop special hash_max handling and just set it where > needed and use non-bh RCU consistently (patch 02, new) > v2: send the latest version of the set which handles when IGMP snooping > is not defined, changes are in patch 01 Series applied, thanks Nik.
Re: [Bridge] [PATCH net-next v2 0/3] net: bridge: add an option to disabe linklocal learning
From: Nikolay Aleksandrov Date: Sat, 24 Nov 2018 04:34:19 +0200 > This set adds a new bridge option which can control learning from > link-local packets, by default learning is on to be consistent and avoid > breaking users expectations. If the new no_linklocal_learn option is > enabled then the bridge will stop learning from link-local packets. > > In order to save space for future boolean options, patch 01 adds a new > bool option API that uses a bitmask to control boolean options. The > bridge is by far the largest netlink attr user and we keep adding simple > boolean options which waste nl attr ids and space. We're not directly > mapping these to the in-kernel bridge flags because some might require > more complex configuration changes (e.g. if we were to add the per port > vlan stats now, it'd require multiple checks before changing value). > Any new bool option needs to be handled by both br_boolopt_toggle and get > in order to be able to retrieve its state later. All such options are > automatically exported via netlink. The behaviour of setting such > options is consistent with netlink option handling when a missing > option is being set (silently ignored), e.g. when a newer iproute2 is used > on older kernel. All supported options are exported via bm's optmask > when dumping the new attribute. > > v2: address Andrew Lunn's comments, squash a minor change into patch 01, > export all supported options via optmask when dumping, add patch 03, > pass down extack so options can return meaningful errors, add > WARN_ON on unsupported options (should not happen) Series applied, thanks Nikolay.
Re: [Bridge] [PATCH][V2] net: bridge: remove redundant checks for null p->dev and p->br
From: Colin King
Date: Sun, 25 Nov 2018 16:08:51 +
> From: Colin Ian King
>
> A recent change added a null check on p->dev after p->dev was being
> dereferenced by the ns_capable check on p->dev. It turns out that
> neither the p->dev and p->br null checks are necessary, and can be
> removed, which cleans up a static analyis warning.
>
> As Nikolay Aleksandrov noted, these checks can be removed because:
>
> "My reasoning of why it shouldn't be possible:
> - On port add new_nbp() sets both p->dev and p->br before creating
> kobj/sysfs
>
> - On port del (trickier) del_nbp() calls kobject_del() before call_rcu()
> to destroy the port which in turn calls sysfs_remove_dir() which uses
> kernfs_remove() which deactivates (shouldn't be able to open new
> files) and calls kernfs_drain() to drain current open/mmaped files in
> the respective dir before continuing, thus making it impossible to
> open a bridge port sysfs file with p->dev and p->br equal to NULL.
>
> So I think it's safe to remove those checks altogether. It'd be nice to
> get a second look over my reasoning as I might be missing something in
> sysfs/kernfs call path."
>
> Thanks to Nikolay Aleksandrov's suggestion to remove the check and
> David Miller for sanity checking this.
>
> Detected by CoverityScan, CID#751490 ("Dereference before null check")
>
> Fixes: a5f3ea54f3cc ("net: bridge: add support for raw sysfs port options")
> Signed-off-by: Colin Ian King
Applied.
Re: [Bridge] [PATCH] net: bridge: check for a null p->dev before dereferencing it
From: Nikolay Aleksandrov Date: Sat, 24 Nov 2018 14:21:07 +0200 > I was contacted recently about this privately and this was my reply: > "Checking new_nbp() and del_nbp() it should not be possible to reach that code > with p->dev or p->br as NULL. The cap check code has always been there, I just > shuffled the rest of the function to obtain rtnl lock and kept it as close to > the original as possible, thus the checks remained. > In order to avoid future reports like this I'll send a cleanup once net-next > opens up. > > My reasoning of why it shouldn't be possible: > - On port add new_nbp() sets both p->dev and p->br before creating kobj/sysfs > > - On port del (trickier) del_nbp() calls kobject_del() before call_rcu() to > destroy > the port which in turn calls sysfs_remove_dir() which uses kernfs_remove() > which > deactivates (shouldn't be able to open new files) and calls kernfs_drain() > to drain > current open/mmaped files in the respective dir before continuing, thus > making it > impossible to open a bridge port sysfs file with p->dev and p->br equal to > NULL. > " > > So I think it's safe to remove those checks altogether. It'd be nice to get a > second > look over my reasoning as I might be missing something in sysfs/kernfs call > path. I did a once over your analysis and I agree, the checks should be safe to remove.
Re: [Bridge] [PATCH net-next 00/16] mlxsw: Add VxLAN learning support
From: Ido Schimmel Date: Wed, 21 Nov 2018 08:02:32 + > This patchset adds VxLAN learning support in the mlxsw driver. Looks great, series applied, thanks Ido.
Re: [Bridge] [PATCH net v3] net: bridge: fix vlan stats use-after-free on destruction
From: Nikolay Aleksandrov
Date: Fri, 16 Nov 2018 18:50:01 +0200
> Syzbot reported a use-after-free of the global vlan context on port vlan
> destruction. When I added per-port vlan stats I missed the fact that the
> global vlan context can be freed before the per-port vlan rcu callback.
> There're a few different ways to deal with this, I've chosen to add a
> new private flag that is set only when per-port stats are allocated so
> we can directly check it on destruction without dereferencing the global
> context at all. The new field in net_bridge_vlan uses a hole.
>
> v2: cosmetic change, move the check to br_process_vlan_info where the
> other checks are done
> v3: add change log in the patch, add private (in-kernel only) flags in a
> hole in net_bridge_vlan struct and use that instead of mixing
> user-space flags with private flags
>
> Fixes: 9163a0fc1f0c ("net: bridge: add support for per-port vlan stats")
> Reported-by: syzbot+04681da557a0e49a5...@syzkaller.appspotmail.com
> Signed-off-by: Nikolay Aleksandrov
Applied.
