Re: [PATCH net-next 1/3] rtnetlink: add ndo_fdb_dump_context
From: Ido Schimmel <hidden>
Date: 2024-12-08 17:57:53
Subsystem:
networking [general], the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
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
quoted hunk ↗ jump to hunk
--- .../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,
[...]
quoted hunk ↗ jump to hunk
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);