> -----Original Message----- > From: Nikolay Aleksandrov [mailto:niko...@cumulusnetworks.com] > Sent: Friday, September 09, 2016 4:39 PM > To: Nikolay Aleksandrov <niko...@cumulusnetworks.com> > Cc: Jiri Pirko <j...@resnulli.us>; Linux Kernel Network Developers > <netdev@vger.kernel.org>; David S. Miller > <da...@davemloft.net>; Nogah Frankel <nog...@mellanox.com>; Ido Schimmel > <ido...@mellanox.com>; Elad Raz > <el...@mellanox.com>; Yotam Gigi <yot...@mellanox.com>; Or Gerlitz > <ogerl...@mellanox.com>; ro...@cumulusnetworks.com; > linvi...@tuxdriver.com; tg...@suug.ch; go...@cumulusnetworks.com; > sfel...@gmail.com; s...@queasysnail.net; Eran Ben Elisha > <era...@mellanox.com>; a...@plumgrid.com; eduma...@google.com; > han...@stressinduktion.org; f.faine...@gmail.com; > d...@cumulusnetworks.com > Subject: 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 > > <niko...@cumulusnetworks.com> wrote: > > > >> > >> On Sep 8, 2016, at 9:19 AM, Jiri Pirko <j...@resnulli.us> wrote: > >> > >> From: Nogah Frankel <nog...@mellanox.com> > >> > >> 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 <nog...@mellanox.com> > >> Signed-off-by: Jiri Pirko <j...@mellanox.com> > >> --- > >> include/uapi/linux/if_link.h | 9 ++++ > >> net/core/rtnetlink.c | 97 > >> ++++++++++++++++++++++++++++++++++++++++++-- > >> 2 files changed, 102 insertions(+), 4 deletions(-) > >> > > [snip] > >> @@ -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 > > > >
I'll fix it. Thanks. > >> 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
