Linux Ethernet Bridging - Apr 2017 - [Bridge] [PATCH net-next] net: bridge: Fix improper taking over HW learned FDB

Commit 7e26bf45e4cb ("net: bridge: allow SW learn to take over HW fdb
entries") added the ability to "take over an entry which was
previously
learned via HW when it shows up from a SW port".

However, if an entry was learned via HW and then a control packet
(e.g., ARP request) was trapped to the CPU, the bridge driver will
update the entry and remove the externally learned flag, although the
entry is still present in HW. Instead, only clear the externally learned
flag in case of roaming.

Fixes: 7e26bf45e4cb ("net: bridge: allow SW learn to take over HW fdb
entries")
Signed-off-by: Ido Schimmel <idosch at mellanox.com>
Signed-off-by: Arkadi Sharashevsky <arkadis at mellanox.com>
Cc: Nikolay Aleksandrov <nikolay at cumulusnetworks.com>
---
 net/bridge/br_fdb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 5a40a87..ab0c7cc 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -589,14 +589,14 @@ void br_fdb_update(struct net_bridge *br, struct
net_bridge_port *source,
 			if (unlikely(source != fdb->dst)) {
 				fdb->dst = source;
 				fdb_modified = true;
+				/* Take over HW learned entry */
+				if (unlikely(fdb->added_by_external_learn))
+					fdb->added_by_external_learn = 0;
 			}
 			if (now != fdb->updated)
 				fdb->updated = now;
 			if (unlikely(added_by_user))
 				fdb->added_by_user = 1;
-			/* Take over HW learned entry */
-			if (unlikely(fdb->added_by_external_learn))
-				fdb->added_by_external_learn = 0;
 			if (unlikely(fdb_modified))
 				fdb_notify(br, fdb, RTM_NEWNEIGH);
 		}
-- 
2.4.11

On 27/04/17 11:08, Arkadi Sharshevsky wrote:
> Commit 7e26bf45e4cb ("net: bridge: allow SW learn to take over HW fdb
> entries") added the ability to "take over an entry which was
previously
> learned via HW when it shows up from a SW port".
> 
> However, if an entry was learned via HW and then a control packet
> (e.g., ARP request) was trapped to the CPU, the bridge driver will
> update the entry and remove the externally learned flag, although the
> entry is still present in HW. Instead, only clear the externally learned
> flag in case of roaming.
> 
> Fixes: 7e26bf45e4cb ("net: bridge: allow SW learn to take over HW fdb
entries")
> Signed-off-by: Ido Schimmel <idosch at mellanox.com>
> Signed-off-by: Arkadi Sharashevsky <arkadis at mellanox.com>
> Cc: Nikolay Aleksandrov <nikolay at cumulusnetworks.com>
> ---
>  net/bridge/br_fdb.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
Thanks!

Acked-by: Nikolay Aleksandrov <nikolay at cumulusnetworks.com>

On 27/04/17 11:08, Arkadi Sharshevsky wrote:
> Commit 7e26bf45e4cb ("net: bridge: allow SW learn to take over HW fdb
> entries") added the ability to "take over an entry which was
previously
> learned via HW when it shows up from a SW port".
> 
> However, if an entry was learned via HW and then a control packet
> (e.g., ARP request) was trapped to the CPU, the bridge driver will
> update the entry and remove the externally learned flag, although the
> entry is still present in HW. Instead, only clear the externally learned
> flag in case of roaming.
> 
> Fixes: 7e26bf45e4cb ("net: bridge: allow SW learn to take over HW fdb
entries")
> Signed-off-by: Ido Schimmel <idosch at mellanox.com>
> Signed-off-by: Arkadi Sharashevsky <arkadis at mellanox.com>
> Cc: Nikolay Aleksandrov <nikolay at cumulusnetworks.com>
> ---
>  net/bridge/br_fdb.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 5a40a87..ab0c7cc 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -589,14 +589,14 @@ void br_fdb_update(struct net_bridge *br, struct
net_bridge_port *source,
>  			if (unlikely(source != fdb->dst)) {
>  				fdb->dst = source;
>  				fdb_modified = true;
> +				/* Take over HW learned entry */
> +				if (unlikely(fdb->added_by_external_learn))
> +					fdb->added_by_external_learn = 0;
>  			}
>  			if (now != fdb->updated)
>  				fdb->updated = now;
>  			if (unlikely(added_by_user))
>  				fdb->added_by_user = 1;
> -			/* Take over HW learned entry */
> -			if (unlikely(fdb->added_by_external_learn))
> -				fdb->added_by_external_learn = 0;
>  			if (unlikely(fdb_modified))
>  				fdb_notify(br, fdb, RTM_NEWNEIGH);
>  		}
> 
Wait, this won't apply to latest net-next due to my patch that added
fdb_modified when changing the ext learned flag. I think you'll have to
respin on top of latest net-next.