[PATCH net-next 0/3] net: prepare for removal of net->dev_index_head

[PATCH net-next 0/3] net: prepare for removal of net->dev_index_head

[Eric Dumazet]

This series changes rtnl_fdb_dump, last iterator using net->dev_index_head[]

First patch creates ndo_fdb_dump_context structure, to no longer
assume specific layout for the arguments.

Second patch adopts for_each_netdev_dump() in rtnl_fdb_dump(),
while changing two first fields of ndo_fdb_dump_context.

Third patch removes the padding, thus changing the location
of ctx->fdb_idx now that all users agree on how to retrive it.

After this series, the only users of net->dev_index_head
are __dev_get_by_index() and dev_get_by_index_rcu().

We have to evaluate if switching them to dev_by_index xarray
would be sensible.

Eric Dumazet (3):
  rtnetlink: add ndo_fdb_dump_context
  rtnetlink: switch rtnl_fdb_dump() to for_each_netdev_dump()
  rtnetlink: remove pad field in ndo_fdb_dump_context

 .../ethernet/freescale/dpaa2/dpaa2-switch.c   |   3 +-
 drivers/net/ethernet/mscc/ocelot_net.c        |   3 +-
 drivers/net/vxlan/vxlan_core.c                |   5 +-
 include/linux/rtnetlink.h                     |   6 +
 net/bridge/br_fdb.c                           |   3 +-
 net/core/rtnetlink.c                          | 106 +++++++-----------
 net/dsa/user.c                                |   3 +-
 7 files changed, 59 insertions(+), 70 deletions(-)

-- 
2.47.0.338.g60cca15819-goog

[PATCH net-next 1/3] rtnetlink: add ndo_fdb_dump_context

[Eric Dumazet]

rtnl_fdb_dump() and various ndo_fdb_dump() helpers share
a hidden layout of cb->ctx.

Before switching rtnl_fdb_dump() to for_each_netdev_dump()
in the following patch, make this more explicit.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 .../ethernet/freescale/dpaa2/dpaa2-switch.c   |  3 ++-
 drivers/net/ethernet/mscc/ocelot_net.c        |  3 ++-
 drivers/net/vxlan/vxlan_core.c                |  5 ++--
 include/linux/rtnetlink.h                     |  7 ++++++
 net/bridge/br_fdb.c                           |  3 ++-
 net/core/rtnetlink.c                          | 24 +++++++++----------
 net/dsa/user.c                                |  3 ++-
 7 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index a293b08f36d46dfde7e25412951da78c15e2dfd6..de383e6c6d523f01f02cb3c3977b1c448a3ac9a7 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -781,12 +781,13 @@ static int dpaa2_switch_fdb_dump_nl(struct fdb_dump_entry *entry,
 				    struct ethsw_dump_ctx *dump)
 {
 	int is_dynamic = entry->type & DPSW_FDB_ENTRY_DINAMIC;
+	struct ndo_fdb_dump_context *ctx = (void *)dump->cb->ctx;
 	u32 portid = NETLINK_CB(dump->cb->skb).portid;
 	u32 seq = dump->cb->nlh->nlmsg_seq;
 	struct nlmsghdr *nlh;
 	struct ndmsg *ndm;
 
-	if (dump->idx < dump->cb->args[2])
+	if (dump->idx < ctx->fdb_idx)
 		goto skip;
 
 	nlh = nlmsg_put(dump->skb, portid, seq, RTM_NEWNEIGH,
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index 558e03301aa8ed89e15c5f37d148a287feaf0018..8d48468cddd7cf91fb49ad23a5c57110900160ef 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -758,12 +758,13 @@ static int ocelot_port_fdb_do_dump(const unsigned char *addr, u16 vid,
 				   bool is_static, void *data)
 {
 	struct ocelot_dump_ctx *dump = data;
+	struct ndo_fdb_dump_context *ctx = (void *)dump->cb->ctx;
 	u32 portid = NETLINK_CB(dump->cb->skb).portid;
 	u32 seq = dump->cb->nlh->nlmsg_seq;
 	struct nlmsghdr *nlh;
 	struct ndmsg *ndm;
 
-	if (dump->idx < dump->cb->args[2])
+	if (dump->idx < ctx->fdb_idx)
 		goto skip;
 
 	nlh = nlmsg_put(dump->skb, portid, seq, RTM_NEWNEIGH,
diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index b46a799bd3904c4183775cb2e86172a0b127bb4f..2cb33c2cb836cf38b6e03b8a620594aa616f00fa 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -1352,6 +1352,7 @@ static int vxlan_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,
 			  struct net_device *dev,
 			  struct net_device *filter_dev, int *idx)
 {
+	struct ndo_fdb_dump_context *ctx = (void *)cb->ctx;
 	struct vxlan_dev *vxlan = netdev_priv(dev);
 	unsigned int h;
 	int err = 0;
@@ -1364,7 +1365,7 @@ static int vxlan_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,
 			struct vxlan_rdst *rd;
 
 			if (rcu_access_pointer(f->nh)) {
-				if (*idx < cb->args[2])
+				if (*idx < ctx->fdb_idx)
 					goto skip_nh;
 				err = vxlan_fdb_info(skb, vxlan, f,
 						     NETLINK_CB(cb->skb).portid,
@@ -1381,7 +1382,7 @@ static int vxlan_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,
 			}
 
 			list_for_each_entry_rcu(rd, &f->remotes, list) {
-				if (*idx < cb->args[2])
+				if (*idx < ctx->fdb_idx)
 					goto skip;
 
 				err = vxlan_fdb_info(skb, vxlan, f,
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 14b88f55192085def8f318c7913a76d5447b4975..a91dfea64724615c9db778646e52cb8573f47e06 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -178,6 +178,13 @@ void rtnetlink_init(void);
 void __rtnl_unlock(void);
 void rtnl_kfree_skbs(struct sk_buff *head, struct sk_buff *tail);
 
+/* Shared by rtnl_fdb_dump() and various ndo_fdb_dump() helpers. */
+struct ndo_fdb_dump_context {
+	unsigned long s_h;
+	unsigned long s_idx;
+	unsigned long fdb_idx;
+};
+
 extern int ndo_dflt_fdb_dump(struct sk_buff *skb,
 			     struct netlink_callback *cb,
 			     struct net_device *dev,
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 82bac2426631bcea63ea834e72f074fa2eaf0cee..902694c0ce643ec448978e4c4625692ccb1facd9 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -955,6 +955,7 @@ int br_fdb_dump(struct sk_buff *skb,
 		struct net_device *filter_dev,
 		int *idx)
 {
+	struct ndo_fdb_dump_context *ctx = (void *)cb->ctx;
 	struct net_bridge *br = netdev_priv(dev);
 	struct net_bridge_fdb_entry *f;
 	int err = 0;
@@ -970,7 +971,7 @@ int br_fdb_dump(struct sk_buff *skb,
 
 	rcu_read_lock();
 	hlist_for_each_entry_rcu(f, &br->fdb_list, fdb_node) {
-		if (*idx < cb->args[2])
+		if (*idx < ctx->fdb_idx)
 			goto skip;
 		if (filter_dev && (!f->dst || f->dst->dev != filter_dev)) {
 			if (filter_dev != dev)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index ab5f201bf0ab41b463175f501e8560b4d64d9b0a..02791328102e7590465aab9ab949af093721b256 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -4762,15 +4762,16 @@ static int nlmsg_populate_fdb(struct sk_buff *skb,
 			      int *idx,
 			      struct netdev_hw_addr_list *list)
 {
+	struct ndo_fdb_dump_context *ctx = (void *)cb->ctx;
 	struct netdev_hw_addr *ha;
-	int err;
 	u32 portid, seq;
+	int err;
 
 	portid = NETLINK_CB(cb->skb).portid;
 	seq = cb->nlh->nlmsg_seq;
 
 	list_for_each_entry(ha, &list->list, list) {
-		if (*idx < cb->args[2])
+		if (*idx < ctx->fdb_idx)
 			goto skip;
 
 		err = nlmsg_populate_fdb_fill(skb, dev, ha->addr, 0,
@@ -4909,10 +4910,9 @@ static int valid_fdb_dump_legacy(const struct nlmsghdr *nlh,
 
 static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
 {
-	struct net_device *dev;
-	struct net_device *br_dev = NULL;
-	const struct net_device_ops *ops = NULL;
-	const struct net_device_ops *cops = NULL;
+	const struct net_device_ops *ops = NULL, *cops = NULL;
+	struct ndo_fdb_dump_context *ctx = (void *)cb->ctx;
+	struct net_device *dev, *br_dev = NULL;
 	struct net *net = sock_net(skb->sk);
 	struct hlist_head *head;
 	int brport_idx = 0;
@@ -4939,8 +4939,8 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
 		ops = br_dev->netdev_ops;
 	}
 
-	s_h = cb->args[0];
-	s_idx = cb->args[1];
+	s_h = ctx->s_h;
+	s_idx = ctx->s_idx;
 
 	for (h = s_h; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
 		idx = 0;
@@ -4992,7 +4992,7 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
 			cops = NULL;
 
 			/* reset fdb offset to 0 for rest of the interfaces */
-			cb->args[2] = 0;
+			ctx->fdb_idx = 0;
 			fidx = 0;
 cont:
 			idx++;
@@ -5000,9 +5000,9 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
 	}
 
 out:
-	cb->args[0] = h;
-	cb->args[1] = idx;
-	cb->args[2] = fidx;
+	ctx->s_h = h;
+	ctx->s_idx = idx;
+	ctx->fdb_idx = fidx;
 
 	return skb->len;
 }
diff --git a/net/dsa/user.c b/net/dsa/user.c
index 06c30a9e29ff820d2dd58fb1801d5e76a5928326..c736c019e2af90747738f10b667e6ad936c9eb0b 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -515,12 +515,13 @@ dsa_user_port_fdb_do_dump(const unsigned char *addr, u16 vid,
 			  bool is_static, void *data)
 {
 	struct dsa_user_dump_ctx *dump = data;
+	struct ndo_fdb_dump_context *ctx = (void *)dump->cb->ctx;
 	u32 portid = NETLINK_CB(dump->cb->skb).portid;
 	u32 seq = dump->cb->nlh->nlmsg_seq;
 	struct nlmsghdr *nlh;
 	struct ndmsg *ndm;
 
-	if (dump->idx < dump->cb->args[2])
+	if (dump->idx < ctx->fdb_idx)
 		goto skip;
 
 	nlh = nlmsg_put(dump->skb, portid, seq, RTM_NEWNEIGH,
-- 
2.47.0.338.g60cca15819-goog

[PATCH net-next 2/3] rtnetlink: switch rtnl_fdb_dump() to for_each_netdev_dump()

[Eric Dumazet]

This is the last netdev iterator still using net->dev_index_head[].

Convert to modern for_each_netdev_dump() for better scalability,
and use common patterns in our stack.

Following patch in this series removes the pad field
in struct ndo_fdb_dump_context.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/rtnetlink.h |  4 +-
 net/core/rtnetlink.c      | 92 +++++++++++++++------------------------
 2 files changed, 37 insertions(+), 59 deletions(-)

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index a91dfea64724615c9db778646e52cb8573f47e06..2b17d7eebd92342e472b9eb3b2ade84bc8ae2e94 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -180,8 +180,8 @@ void rtnl_kfree_skbs(struct sk_buff *head, struct sk_buff *tail);
 
 /* Shared by rtnl_fdb_dump() and various ndo_fdb_dump() helpers. */
 struct ndo_fdb_dump_context {
-	unsigned long s_h;
-	unsigned long s_idx;
+	unsigned long ifindex;
+	unsigned long pad;
 	unsigned long fdb_idx;
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 02791328102e7590465aab9ab949af093721b256..cdedb46edc2fd54f6fbd6ce7fb8e9a26486034e7 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -4914,13 +4914,10 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
 	struct ndo_fdb_dump_context *ctx = (void *)cb->ctx;
 	struct net_device *dev, *br_dev = NULL;
 	struct net *net = sock_net(skb->sk);
-	struct hlist_head *head;
 	int brport_idx = 0;
 	int br_idx = 0;
-	int h, s_h;
-	int idx = 0, s_idx;
-	int err = 0;
 	int fidx = 0;
+	int err;
 
 	if (cb->strict_check)
 		err = valid_fdb_dump_strict(cb->nlh, &br_idx, &brport_idx,
@@ -4939,69 +4936,50 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
 		ops = br_dev->netdev_ops;
 	}
 
-	s_h = ctx->s_h;
-	s_idx = ctx->s_idx;
-
-	for (h = s_h; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
-		idx = 0;
-		head = &net->dev_index_head[h];
-		hlist_for_each_entry(dev, head, index_hlist) {
-
-			if (brport_idx && (dev->ifindex != brport_idx))
-				continue;
-
-			if (!br_idx) { /* user did not specify a specific bridge */
-				if (netif_is_bridge_port(dev)) {
-					br_dev = netdev_master_upper_dev_get(dev);
-					cops = br_dev->netdev_ops;
-				}
-			} else {
-				if (dev != br_dev &&
-				    !netif_is_bridge_port(dev))
-					continue;
+	for_each_netdev_dump(net, dev, ctx->ifindex) {
+		if (brport_idx && (dev->ifindex != brport_idx))
+			continue;
 
-				if (br_dev != netdev_master_upper_dev_get(dev) &&
-				    !netif_is_bridge_master(dev))
-					continue;
-				cops = ops;
+		if (!br_idx) { /* user did not specify a specific bridge */
+			if (netif_is_bridge_port(dev)) {
+				br_dev = netdev_master_upper_dev_get(dev);
+				cops = br_dev->netdev_ops;
 			}
+		} else {
+			if (dev != br_dev &&
+			    !netif_is_bridge_port(dev))
+				continue;
 
-			if (idx < s_idx)
-				goto cont;
+			if (br_dev != netdev_master_upper_dev_get(dev) &&
+			    !netif_is_bridge_master(dev))
+				continue;
+			cops = ops;
+		}
 
-			if (netif_is_bridge_port(dev)) {
-				if (cops && cops->ndo_fdb_dump) {
-					err = cops->ndo_fdb_dump(skb, cb,
-								br_dev, dev,
-								&fidx);
-					if (err == -EMSGSIZE)
-						goto out;
-				}
+		if (netif_is_bridge_port(dev)) {
+			if (cops && cops->ndo_fdb_dump) {
+				err = cops->ndo_fdb_dump(skb, cb, br_dev, dev,
+							&fidx);
+				if (err == -EMSGSIZE)
+					break;
 			}
+		}
 
-			if (dev->netdev_ops->ndo_fdb_dump)
-				err = dev->netdev_ops->ndo_fdb_dump(skb, cb,
-								    dev, NULL,
-								    &fidx);
-			else
-				err = ndo_dflt_fdb_dump(skb, cb, dev, NULL,
-							&fidx);
-			if (err == -EMSGSIZE)
-				goto out;
+		if (dev->netdev_ops->ndo_fdb_dump)
+			err = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev, NULL,
+							    &fidx);
+		else
+			err = ndo_dflt_fdb_dump(skb, cb, dev, NULL, &fidx);
+		if (err == -EMSGSIZE)
+			break;
 
-			cops = NULL;
+		cops = NULL;
 
-			/* reset fdb offset to 0 for rest of the interfaces */
-			ctx->fdb_idx = 0;
-			fidx = 0;
-cont:
-			idx++;
-		}
+		/* reset fdb offset to 0 for rest of the interfaces */
+		ctx->fdb_idx = 0;
+		fidx = 0;
 	}
 
-out:
-	ctx->s_h = h;
-	ctx->s_idx = idx;
 	ctx->fdb_idx = fidx;
 
 	return skb->len;
-- 
2.47.0.338.g60cca15819-goog

[PATCH net-next 3/3] rtnetlink: remove pad field in ndo_fdb_dump_context

[Eric Dumazet]

I chose to remove this field in a separate patch to ease
potential bisection, in case one ndo_fdb_dump() is still
using the old way (cb->args[2] instead of ctx->fdb_idx)

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/rtnetlink.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 2b17d7eebd92342e472b9eb3b2ade84bc8ae2e94..af668b79eb757c86970b2455d9d820c902699a13 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -181,7 +181,6 @@ void rtnl_kfree_skbs(struct sk_buff *head, struct sk_buff *tail);
 /* Shared by rtnl_fdb_dump() and various ndo_fdb_dump() helpers. */
 struct ndo_fdb_dump_context {
 	unsigned long ifindex;
-	unsigned long pad;
 	unsigned long fdb_idx;
 };
 
-- 
2.47.0.338.g60cca15819-goog

Re: [PATCH net-next 1/3] rtnetlink: add ndo_fdb_dump_context

[Ido Schimmel]

On Sat, Dec 07, 2024 at 04:22:46PM +0000, Eric Dumazet wrote:
> rtnl_fdb_dump() and various ndo_fdb_dump() helpers share
> a hidden layout of cb->ctx.
> 
> Before switching rtnl_fdb_dump() to for_each_netdev_dump()
> in the following patch, make this more explicit.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

A couple of nits in case you have v2

> ---
>  .../ethernet/freescale/dpaa2/dpaa2-switch.c   |  3 ++-
>  drivers/net/ethernet/mscc/ocelot_net.c        |  3 ++-
>  drivers/net/vxlan/vxlan_core.c                |  5 ++--
>  include/linux/rtnetlink.h                     |  7 ++++++
>  net/bridge/br_fdb.c                           |  3 ++-
>  net/core/rtnetlink.c                          | 24 +++++++++----------
>  net/dsa/user.c                                |  3 ++-
>  7 files changed, 30 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> index a293b08f36d46dfde7e25412951da78c15e2dfd6..de383e6c6d523f01f02cb3c3977b1c448a3ac9a7 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> @@ -781,12 +781,13 @@ static int dpaa2_switch_fdb_dump_nl(struct fdb_dump_entry *entry,
>  				    struct ethsw_dump_ctx *dump)
>  {
>  	int is_dynamic = entry->type & DPSW_FDB_ENTRY_DINAMIC;
> +	struct ndo_fdb_dump_context *ctx = (void *)dump->cb->ctx;

Any reason not to maintain reverse xmas tree here?

>  	u32 portid = NETLINK_CB(dump->cb->skb).portid;
>  	u32 seq = dump->cb->nlh->nlmsg_seq;
>  	struct nlmsghdr *nlh;
>  	struct ndmsg *ndm;
>  
> -	if (dump->idx < dump->cb->args[2])
> +	if (dump->idx < ctx->fdb_idx)
>  		goto skip;
>  
>  	nlh = nlmsg_put(dump->skb, portid, seq, RTM_NEWNEIGH,

[...]

> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> index 14b88f55192085def8f318c7913a76d5447b4975..a91dfea64724615c9db778646e52cb8573f47e06 100644
> --- a/include/linux/rtnetlink.h
> +++ b/include/linux/rtnetlink.h
> @@ -178,6 +178,13 @@ void rtnetlink_init(void);
>  void __rtnl_unlock(void);
>  void rtnl_kfree_skbs(struct sk_buff *head, struct sk_buff *tail);
>  
> +/* Shared by rtnl_fdb_dump() and various ndo_fdb_dump() helpers. */
> +struct ndo_fdb_dump_context {
> +	unsigned long s_h;
> +	unsigned long s_idx;
> +	unsigned long fdb_idx;
> +};
> +
>  extern int ndo_dflt_fdb_dump(struct sk_buff *skb,
>  			     struct netlink_callback *cb,
>  			     struct net_device *dev,
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 82bac2426631bcea63ea834e72f074fa2eaf0cee..902694c0ce643ec448978e4c4625692ccb1facd9 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -955,6 +955,7 @@ int br_fdb_dump(struct sk_buff *skb,
>  		struct net_device *filter_dev,
>  		int *idx)
>  {
> +	struct ndo_fdb_dump_context *ctx = (void *)cb->ctx;
>  	struct net_bridge *br = netdev_priv(dev);
>  	struct net_bridge_fdb_entry *f;
>  	int err = 0;

Unlikely that the context will ever grow past 48 bytes, but might be
worthwhile to add:

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index cdedb46edc2f..8fe252c298a2 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -4919,6 +4919,8 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
 	int fidx = 0;
 	int err;
 
+	NL_ASSERT_CTX_FITS(struct ndo_fdb_dump_context);
+
 	if (cb->strict_check)
 		err = valid_fdb_dump_strict(cb->nlh, &br_idx, &brport_idx,
 					    cb->extack);

Re: [PATCH net-next 2/3] rtnetlink: switch rtnl_fdb_dump() to for_each_netdev_dump()

[Ido Schimmel]

On Sat, Dec 07, 2024 at 04:22:47PM +0000, Eric Dumazet wrote:
> This is the last netdev iterator still using net->dev_index_head[].
> 
> Convert to modern for_each_netdev_dump() for better scalability,
> and use common patterns in our stack.
> 
> Following patch in this series removes the pad field
> in struct ndo_fdb_dump_context.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Conversion looks correct AFAICT:

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

Re: [PATCH net-next 3/3] rtnetlink: remove pad field in ndo_fdb_dump_context

[Ido Schimmel]

On Sat, Dec 07, 2024 at 04:22:48PM +0000, Eric Dumazet wrote:
> I chose to remove this field in a separate patch to ease
> potential bisection, in case one ndo_fdb_dump() is still
> using the old way (cb->args[2] instead of ctx->fdb_idx)
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

Re: [PATCH net-next 1/3] rtnetlink: add ndo_fdb_dump_context

[Kuniyuki Iwashima]

From: Eric Dumazet <edumazet@google.com>
Date: Sat,  7 Dec 2024 16:22:46 +0000
> rtnl_fdb_dump() and various ndo_fdb_dump() helpers share
> a hidden layout of cb->ctx.
> 
> Before switching rtnl_fdb_dump() to for_each_netdev_dump()
> in the following patch, make this more explicit.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>

Re: [PATCH net-next 2/3] rtnetlink: switch rtnl_fdb_dump() to for_each_netdev_dump()

[Kuniyuki Iwashima]

From: Eric Dumazet <edumazet@google.com>
Date: Sat,  7 Dec 2024 16:22:47 +0000
> This is the last netdev iterator still using net->dev_index_head[].
> 
> Convert to modern for_each_netdev_dump() for better scalability,
> and use common patterns in our stack.
> 
> Following patch in this series removes the pad field
> in struct ndo_fdb_dump_context.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>

Re: [PATCH net-next 3/3] rtnetlink: remove pad field in ndo_fdb_dump_context

[Kuniyuki Iwashima]

From: Eric Dumazet <edumazet@google.com>
Date: Sat,  7 Dec 2024 16:22:48 +0000
> I chose to remove this field in a separate patch to ease
> potential bisection, in case one ndo_fdb_dump() is still
> using the old way (cb->args[2] instead of ctx->fdb_idx)
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>

Re: [PATCH net-next 1/3] rtnetlink: add ndo_fdb_dump_context

[Eric Dumazet]

On Sun, Dec 8, 2024 at 6:57 PM Ido Schimmel <idosch@idosch.org> wrote:
>
> On Sat, Dec 07, 2024 at 04:22:46PM +0000, Eric Dumazet wrote:
> > rtnl_fdb_dump() and various ndo_fdb_dump() helpers share
> > a hidden layout of cb->ctx.
> >
> > Before switching rtnl_fdb_dump() to for_each_netdev_dump()
> > in the following patch, make this more explicit.
> >
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
>
> A couple of nits in case you have v2
>
> > ---
> >  .../ethernet/freescale/dpaa2/dpaa2-switch.c   |  3 ++-
> >  drivers/net/ethernet/mscc/ocelot_net.c        |  3 ++-
> >  drivers/net/vxlan/vxlan_core.c                |  5 ++--
> >  include/linux/rtnetlink.h                     |  7 ++++++
> >  net/bridge/br_fdb.c                           |  3 ++-
> >  net/core/rtnetlink.c                          | 24 +++++++++----------
> >  net/dsa/user.c                                |  3 ++-
> >  7 files changed, 30 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> > index a293b08f36d46dfde7e25412951da78c15e2dfd6..de383e6c6d523f01f02cb3c3977b1c448a3ac9a7 100644
> > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> > @@ -781,12 +781,13 @@ static int dpaa2_switch_fdb_dump_nl(struct fdb_dump_entry *entry,
> >                                   struct ethsw_dump_ctx *dump)
> >  {
> >       int is_dynamic = entry->type & DPSW_FDB_ENTRY_DINAMIC;
> > +     struct ndo_fdb_dump_context *ctx = (void *)dump->cb->ctx;
>
> Any reason not to maintain reverse xmas tree here?

None, will fix in v2.

>
> >       u32 portid = NETLINK_CB(dump->cb->skb).portid;
> >       u32 seq = dump->cb->nlh->nlmsg_seq;
> >       struct nlmsghdr *nlh;
> >       struct ndmsg *ndm;
> >
> > -     if (dump->idx < dump->cb->args[2])
> > +     if (dump->idx < ctx->fdb_idx)
> >               goto skip;
> >
> >       nlh = nlmsg_put(dump->skb, portid, seq, RTM_NEWNEIGH,
>
> [...]
>
> > diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> > index 14b88f55192085def8f318c7913a76d5447b4975..a91dfea64724615c9db778646e52cb8573f47e06 100644
> > --- a/include/linux/rtnetlink.h
> > +++ b/include/linux/rtnetlink.h
> > @@ -178,6 +178,13 @@ void rtnetlink_init(void);
> >  void __rtnl_unlock(void);
> >  void rtnl_kfree_skbs(struct sk_buff *head, struct sk_buff *tail);
> >
> > +/* Shared by rtnl_fdb_dump() and various ndo_fdb_dump() helpers. */
> > +struct ndo_fdb_dump_context {
> > +     unsigned long s_h;
> > +     unsigned long s_idx;
> > +     unsigned long fdb_idx;
> > +};
> > +
> >  extern int ndo_dflt_fdb_dump(struct sk_buff *skb,
> >                            struct netlink_callback *cb,
> >                            struct net_device *dev,
> > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> > index 82bac2426631bcea63ea834e72f074fa2eaf0cee..902694c0ce643ec448978e4c4625692ccb1facd9 100644
> > --- a/net/bridge/br_fdb.c
> > +++ b/net/bridge/br_fdb.c
> > @@ -955,6 +955,7 @@ int br_fdb_dump(struct sk_buff *skb,
> >               struct net_device *filter_dev,
> >               int *idx)
> >  {
> > +     struct ndo_fdb_dump_context *ctx = (void *)cb->ctx;
> >       struct net_bridge *br = netdev_priv(dev);
> >       struct net_bridge_fdb_entry *f;
> >       int err = 0;
>
> Unlikely that the context will ever grow past 48 bytes, but might be
> worthwhile to add:
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index cdedb46edc2f..8fe252c298a2 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -4919,6 +4919,8 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
>         int fidx = 0;
>         int err;
>
> +       NL_ASSERT_CTX_FITS(struct ndo_fdb_dump_context);
> +

Good idea, will be included in v2, thanks !