Linux Ethernet Bridging - Dec 2018 - [Bridge] [PATCH net-next 01/12] vxlan: Add a function to init switchdev_notifier_vxlan_fdb_info

From: Petr Machata <petrm at mellanox.com>

There are currently two places that need to initialize the notifier info
structure, and one more is coming next when vxlan_fdb_replay() is
introduced. These three instances have / will have very similar code
that is easy to abstract away into a named function.

Add such function, vxlan_fdb_switchdev_notifier_info(), and call it from
vxlan_fdb_switchdev_call_notifiers() and vxlan_fdb_find_uc().

Signed-off-by: Petr Machata <petrm at mellanox.com>
Signed-off-by: Ido Schimmel <idosch at mellanox.com>
---
 drivers/net/vxlan.c | 45 ++++++++++++++++++++++-----------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 901eef428280..fe5cd22e4b4d 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -358,6 +358,26 @@ static void __vxlan_fdb_notify(struct vxlan_dev *vxlan,
struct vxlan_fdb *fdb,
 		rtnl_set_sk_err(net, RTNLGRP_NEIGH, err);
 }
 
+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;
+}
+
 static void vxlan_fdb_switchdev_call_notifiers(struct vxlan_dev *vxlan,
 					       struct vxlan_fdb *fdb,
 					       struct vxlan_rdst *rd,
@@ -371,18 +391,7 @@ static void vxlan_fdb_switchdev_call_notifiers(struct
vxlan_dev *vxlan,
 
 	notifier_type = adding ? SWITCHDEV_VXLAN_FDB_ADD_TO_DEVICE
 			       : SWITCHDEV_VXLAN_FDB_DEL_TO_DEVICE;
-
-	info = (struct switchdev_notifier_vxlan_fdb_info){
-		.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(info.eth_addr, fdb->eth_addr, ETH_ALEN);
-
+	info = vxlan_fdb_switchdev_notifier_info(vxlan, fdb, rd);
 	call_switchdev_notifiers(notifier_type, vxlan->dev,
 				 &info.info);
 }
@@ -539,17 +548,7 @@ int vxlan_fdb_find_uc(struct net_device *dev, const u8
*mac, __be32 vni,
 	}
 
 	rdst = first_remote_rcu(f);
-
-	memset(fdb_info, 0, sizeof(*fdb_info));
-	fdb_info->info.dev = dev;
-	fdb_info->remote_ip = rdst->remote_ip;
-	fdb_info->remote_port = rdst->remote_port;
-	fdb_info->remote_vni = rdst->remote_vni;
-	fdb_info->remote_ifindex = rdst->remote_ifindex;
-	fdb_info->vni = vni;
-	fdb_info->offloaded = rdst->offloaded;
-	fdb_info->added_by_user = f->flags & NTF_VXLAN_ADDED_BY_USER;
-	ether_addr_copy(fdb_info->eth_addr, mac);
+	*fdb_info = vxlan_fdb_switchdev_notifier_info(vxlan, f, rdst);
 
 out:
 	rcu_read_unlock();
-- 
2.19.1

From: Ido Schimmel <idosch at mellanox.com>
Date: Wed, 5 Dec 2018 15:50:23 +0000
> +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 "&info" as an argument.