[PATCH net-next 0/2] Fixes for the switchdev FDB fan-out helpers

Hi,

There are already two fixes for the newly added helpers to multiply a
switchdev FDB add/del event times the number of lower interfaces of a
bridge. These are:

(1) the shim definition of switchdev_handle_fdb_del_to_device() is
    broken, as reported by the kernel test robot.
(2) while checking where switchdev_handle_fdb_del_to_device() is called
    from, I realized it is called once from where it shouldn't, aka from
    __switchdev_handle_fdb_del_to_device(). That shouldn't happen,
    instead that function should recurse into itself directly.

Vladimir Oltean (2):
  net: switchdev: remove stray semicolon in
    switchdev_handle_fdb_del_to_device shim
  net: switchdev: recurse into __switchdev_handle_fdb_del_to_device

 include/net/switchdev.h   | 2 +-
 net/switchdev/switchdev.c | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.25.1

The difference between __switchdev_handle_fdb_del_to_device and
switchdev_handle_del_to_device is that the former takes an extra
orig_dev argument, while the latter starts with dev == orig_dev.

We should recurse into the variant that does not lose the orig_dev along
the way. This is relevant when deleting FDB entries pointing towards a
bridge (dev changes to the lower interfaces, but orig_dev shouldn't).

The addition helper already recurses properly, just the deletion one
doesn't.

Fixes: 8ca07176ab00 ("net: switchdev: introduce a fanout helper for SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/switchdev/switchdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 82dd4e4e86f5..42e88d3d66a7 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c

@@ -532,10 +532,10 @@ static int __switchdev_handle_fdb_del_to_device(struct net_device *dev,
 		if (netif_is_bridge_master(lower_dev))
 			continue;
 
-		err = switchdev_handle_fdb_del_to_device(lower_dev, fdb_info,
-							 check_cb,
-							 foreign_dev_check_cb,
-							 del_cb, lag_del_cb);
+		err = __switchdev_handle_fdb_del_to_device(lower_dev, orig_dev,
+							   fdb_info, check_cb,
+							   foreign_dev_check_cb,
+							   del_cb, lag_del_cb);
 		if (err && err != -EOPNOTSUPP)
 			return err;
 	}

-- 
2.25.1

With the semicolon at the end, the compiler sees the shim function as a
declaration and not as a definition, and warns:

'switchdev_handle_fdb_del_to_device' declared 'static' but never defined

Reported-by: kernel test robot <redacted>
Fixes: 8ca07176ab00 ("net: switchdev: introduce a fanout helper for SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 include/net/switchdev.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 6f57eb2e89cc..66468ff8cc0a 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h

@@ -406,7 +406,7 @@ switchdev_handle_fdb_del_to_device(struct net_device *dev,
 			      const struct switchdev_notifier_fdb_info *fdb_info),
 		int (*lag_del_cb)(struct net_device *dev,
 				  const struct net_device *orig_dev, const void *ctx,
-				  const struct switchdev_notifier_fdb_info *fdb_info));
+				  const struct switchdev_notifier_fdb_info *fdb_info))
 {
 	return 0;
 }

-- 
2.25.1

Hi Vladimir,

On 20/07/2021 19:35, Vladimir Oltean wrote:

With the semicolon at the end, the compiler sees the shim function as a
declaration and not as a definition, and warns:

'switchdev_handle_fdb_del_to_device' declared 'static' but never defined

Thank you for the patch!

My CI also reported the same issue and I confirm it removes the warning.

Tested-by: Matthieu Baerts <redacted>

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Tue, 20 Jul 2021 20:35:55 +0300 you wrote:

Hi,

There are already two fixes for the newly added helpers to multiply a
switchdev FDB add/del event times the number of lower interfaces of a
bridge. These are:

(1) the shim definition of switchdev_handle_fdb_del_to_device() is
    broken, as reported by the kernel test robot.
(2) while checking where switchdev_handle_fdb_del_to_device() is called
    from, I realized it is called once from where it shouldn't, aka from
    __switchdev_handle_fdb_del_to_device(). That shouldn't happen,
    instead that function should recurse into itself directly.

[...]

Here is the summary with links:
  - [net-next,1/2] net: switchdev: remove stray semicolon in switchdev_handle_fdb_del_to_device shim
    https://git.kernel.org/netdev/net-next/c/94111dfc18b8
  - [net-next,2/2] net: switchdev: recurse into __switchdev_handle_fdb_del_to_device
    https://git.kernel.org/netdev/net-next/c/71f4f89a0324

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html