Re: [patch net-next v8 2/3] net: core: Add offload stats to if_stats_msg

On Sep 9, 2016, at 4:36 PM, Nikolay Aleksandrov [off-list ref] wrote:

quoted

On Sep 8, 2016, at 9:19 AM, Jiri Pirko [off-list ref] wrote:

From: Nogah Frankel <redacted>

Add a nested attribute of offload stats to if_stats_msg
named IFLA_STATS_LINK_OFFLOAD_XSTATS.
Under it, add SW stats, meaning stats only per packets that went via
slowpath to the cpu, named IFLA_OFFLOAD_XSTATS_CPU_HIT.

Signed-off-by: Nogah Frankel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
include/uapi/linux/if_link.h |  9 ++++
net/core/rtnetlink.c         | 97 ++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 102 insertions(+), 4 deletions(-)

[snip]

quoted

@@ -3655,6 +3725,22 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,

		}
	}

+	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_OFFLOAD_XSTATS,
+			     *idxattr)) {
+		attr = nla_nest_start(skb, IFLA_STATS_LINK_OFFLOAD_XSTATS);
+		if (!attr)
+			goto nla_put_failure;
+
+		err = rtnl_get_offload_stats(skb, dev);
+		if (err == -ENODATA)
+			nla_nest_cancel(skb, attr);
+		else
+			nla_nest_end(skb, attr);
+
+		if ((err) && (err != -ENODATA))
+			goto nla_put_failure;
+	}
+

Hmm, actually on a second read I think there’s a potential problem here. Since you don’t set *idxattr and if the space
isn’t enough the dump will get restarted and this will lead to an infinite loop.

Err, poor choice of words. I meant the loop will be for userspace since the dump will err out at the same spot all the time so it
won’t be able to ever finish. :-)

I.e. if the previous attributes filled the skb and there’s not enough room for this one, it will return an error
but *idxattr will be == 0 from the previous attribute thus the whole dump will be restarted (this is in case someone
requests this attribute with some of the others of course).

Cheers,
Nik

quoted

	nlmsg_end(skb, nlh);

	return 0;

@@ -3712,6 +3798,9 @@ static size_t if_nlmsg_stats_size(const struct net_device *dev,

		}
	}

+	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_OFFLOAD_XSTATS, 0))
+		size += rtnl_get_offload_stats_size(dev);
+
	return size;
}

-- 
2.5.5