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

[Jiri Pirko]

Fri, Sep 16, 2016 at 10:00:46AM CEST, niko...@cumulusnetworks.com wrote:
>
>> On Sep 14, 2016, at 12:28 PM, Jiri Pirko  wrote:
>> 
>> From: Nogah Frankel 
>> 
>> 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 
>> Signed-off-by: Jiri Pirko 
>> ---
>> include/uapi/linux/if_link.h |   9 
>> net/core/rtnetlink.c | 111 
>> +--
>> 2 files changed, 116 insertions(+), 4 deletions(-)
>> 
>[snip]
>> @@ -3655,6 +3737,24 @@ 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)) {
>> +*idxattr = IFLA_STATS_LINK_OFFLOAD_XSTATS;
>> +attr = nla_nest_start(skb, IFLA_STATS_LINK_OFFLOAD_XSTATS);
>> +if (!attr)
>> +goto nla_put_failure;
>> +
>> +err = rtnl_get_offload_stats(skb, dev, prividx);
>> +if (err == -ENODATA)
>> +nla_nest_cancel(skb, attr);
>> +else
>> +nla_nest_end(skb, attr);
>> +
>> +if ((err) && (err != -ENODATA))
>      ^^
>The extra braces are still there. [1]

:( Will fix. Thanks.

>The rest looks good to me.
>
>Thanks,
> Nik
>
>[1] http://www.spinics.net/lists/netdev/msg394257.html
>
>> +goto nla_put_failure;
>> +*idxattr = 0;
>> +}
>> +
>>  nlmsg_end(skb, nlh);
>> 
>>  return 0;
>> @@ -3708,6 +3808,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
>> 
>

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

[Nikolay Aleksandrov]

> On Sep 14, 2016, at 12:28 PM, Jiri Pirko  wrote:
> 
> From: Nogah Frankel 
> 
> 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 
> Signed-off-by: Jiri Pirko 
> ---
> include/uapi/linux/if_link.h |   9 
> net/core/rtnetlink.c | 111 +--
> 2 files changed, 116 insertions(+), 4 deletions(-)
> 
[snip]
> @@ -3655,6 +3737,24 @@ 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)) {
> + *idxattr = IFLA_STATS_LINK_OFFLOAD_XSTATS;
> + attr = nla_nest_start(skb, IFLA_STATS_LINK_OFFLOAD_XSTATS);
> + if (!attr)
> + goto nla_put_failure;
> +
> + err = rtnl_get_offload_stats(skb, dev, prividx);
> + if (err == -ENODATA)
> + nla_nest_cancel(skb, attr);
> + else
> + nla_nest_end(skb, attr);
> +
> + if ((err) && (err != -ENODATA))
      ^^
The extra braces are still there. [1]
The rest looks good to me.

Thanks,
 Nik

[1] http://www.spinics.net/lists/netdev/msg394257.html

> + goto nla_put_failure;
> + *idxattr = 0;
> + }
> +
>   nlmsg_end(skb, nlh);
> 
>   return 0;
> @@ -3708,6 +3808,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
>