Re: [PATCH net-next 12/13] rtnetlink: make rtnl_fill_link_ifmap() RCU ready

[Jiri Pirko <jiri@resnulli.us>]

Wed, Feb 21, 2024 at 11:59:14AM CET, edumazet@google.com wrote:
>Use READ_ONCE() to read the following device fields:
>
>	dev->mem_start
>	dev->mem_end
>	dev->base_addr
>	dev->irq
>	dev->dma
>	dev->if_port
>
>Provide IFLA_MAP attribute only if at least one of these fields
>is not zero. This saves some space in the output skb for most devices.
>
>Signed-off-by: Eric Dumazet <edumazet@google.com>
>---
> net/core/rtnetlink.c | 26 ++++++++++++++------------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
>diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>index 1b26dfa5668d22fb2e30ceefbf143e98df13ae29..b91ec216c593aaebf97ea69aa0d2d265ab61c098 100644
>--- a/net/core/rtnetlink.c
>+++ b/net/core/rtnetlink.c
>@@ -1455,19 +1455,21 @@ static noinline_for_stack int rtnl_fill_vf(struct sk_buff *skb,
> 	return 0;
> }
> 
>-static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
>+static int rtnl_fill_link_ifmap(struct sk_buff *skb,
>+				const struct net_device *dev)
> {
> 	struct rtnl_link_ifmap map;
> 
> 	memset(&map, 0, sizeof(map));
>-	map.mem_start   = dev->mem_start;
>-	map.mem_end     = dev->mem_end;
>-	map.base_addr   = dev->base_addr;
>-	map.irq         = dev->irq;
>-	map.dma         = dev->dma;
>-	map.port        = dev->if_port;
>-
>-	if (nla_put_64bit(skb, IFLA_MAP, sizeof(map), &map, IFLA_PAD))
>+	map.mem_start = READ_ONCE(dev->mem_start);
>+	map.mem_end   = READ_ONCE(dev->mem_end);
>+	map.base_addr = READ_ONCE(dev->base_addr);
>+	map.irq       = READ_ONCE(dev->irq);
>+	map.dma       = READ_ONCE(dev->dma);
>+	map.port      = READ_ONCE(dev->if_port);
>+	/* Only report non zero information. */
>+	if (memchr_inv(&map, 0, sizeof(map)) &&

This check(optimization) is unrelated to the rest of the patch, correct?
If yes, could it be a separate patch?


>+	    nla_put_64bit(skb, IFLA_MAP, sizeof(map), &map, IFLA_PAD))
> 		return -EMSGSIZE;
> 
> 	return 0;
>@@ -1875,9 +1877,6 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
> 			goto nla_put_failure;
> 	}
> 
>-	if (rtnl_fill_link_ifmap(skb, dev))
>-		goto nla_put_failure;
>-
> 	if (dev->addr_len) {
> 		if (nla_put(skb, IFLA_ADDRESS, dev->addr_len, dev->dev_addr) ||
> 		    nla_put(skb, IFLA_BROADCAST, dev->addr_len, dev->broadcast))
>@@ -1927,6 +1926,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
> 	rcu_read_lock();
> 	if (rtnl_fill_link_af(skb, dev, ext_filter_mask))
> 		goto nla_put_failure_rcu;
>+	if (rtnl_fill_link_ifmap(skb, dev))
>+		goto nla_put_failure_rcu;
>+
> 	rcu_read_unlock();
> 
> 	if (rtnl_fill_prop_list(skb, dev))
>-- 
>2.44.0.rc0.258.g7320e95886-goog
>
>