Re: [PATCH net-next 0/7] bridge: per-vlan stats

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 27 Apr 2016 18:18:15 +0200
Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:

> Hi,
> This set adds support for bridge per-vlan statistics.
> In order to be able to dump statistics we need a way to continue
> dumping after reaching maximum size, thus patches 01-03 extend the new
> stats API with a per-device extended link stats attribute and callback
> which can save its local state and continue where it left off afterwards.
> I considered using the already existing "fill_xstats" callback but it gets
> confusing since we need to separate the linkinfo dump from the new stats
> api dump and adding a flag/argument to do that just looks messy. I don't
> think the rtnl_link_ops size is an issue, so adding these seemed like the
> cleaner approach.
> 
> Patch 05 converts the pvid to a pointer so we can consolidate the vlan
> stats accounting paths later, also allows to simplify the pvid code.
> Patches 06 and 07 add the stats support and netlink dump support
> respectively.
> I've tested this set with both old and modified iproute2, kmemleak on and
> some traffic stress tests while adding/removing vlans and ports.
> 
> Thank you,
>  Nik
> 
> Note: Jamal I haven't forgotten about the per-port per-vlan stats, I've got
> a follow-up patch that adds it. You can easily see that the infrastructure
> for private port/vlan stats is in place after this set. Though the stats
> api will need some more changes to support that.
> 
> 
> Nikolay Aleksandrov (7):
>   net: rtnetlink: allow rtnl_fill_statsinfo to save private state
>     counter
>   net: rtnetlink: allow only one idx saving stats attribute
>   net: rtnetlink: add linkxstats callbacks and attribute
>   net: constify is_skb_forwardable's arguments
>   bridge: vlan: RCUify pvid
>   bridge: vlan: learn to count
>   bridge: netlink: export per-vlan stats
> 
>  include/linux/netdevice.h      |   3 +-
>  include/net/rtnetlink.h        |  10 +++
>  include/uapi/linux/if_bridge.h |   8 +++
>  include/uapi/linux/if_link.h   |   9 +++
>  net/bridge/br_netlink.c        |  80 ++++++++++++++++++++----
>  net/bridge/br_private.h        |  32 +++++-----
>  net/bridge/br_vlan.c           | 134 +++++++++++++++++++++++++++++------------
>  net/core/dev.c                 |   2 +-
>  net/core/rtnetlink.c           |  64 +++++++++++++++++---
>  9 files changed, 266 insertions(+), 76 deletions(-)
> 

I am concerned this adds unnecessary complexity (more bugs)
and overhead (slower performance). Statistics are not free, and having
them in a convenient place maybe unnecessary duplication.