> -----Original Message----- > From: Nikolay Aleksandrov [mailto:[email protected]] > Sent: Friday, September 09, 2016 4:39 PM > To: Nikolay Aleksandrov <[email protected]> > Cc: Jiri Pirko <[email protected]>; Linux Kernel Network Developers > <[email protected]>; David S. Miller > <[email protected]>; Nogah Frankel <[email protected]>; Ido Schimmel > <[email protected]>; Elad Raz > <[email protected]>; Yotam Gigi <[email protected]>; Or Gerlitz > <[email protected]>; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; Eran Ben Elisha > <[email protected]>; [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected] > 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 > > <[email protected]> wrote: > > > >> > >> On Sep 8, 2016, at 9:19 AM, Jiri Pirko <[email protected]> wrote: > >> > >> From: Nogah Frankel <[email protected]> > >> > >> 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 <[email protected]> > >> Signed-off-by: Jiri Pirko <[email protected]> > >> --- > >> 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
