netdev - Re: [PATCH net-next] net: bridge: add STP xstats

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1a32ac69-a802-e039-215a-feec740e682c@cumulusnetworks.com>
Date:   Tue, 10 Dec 2019 09:54:05 +0200
From:   Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To:     Vivien Didelot <vivien.didelot@...il.com>,
        "David S. Miller" <davem@...emloft.net>
Cc:     Roopa Prabhu <roopa@...ulusnetworks.com>, netdev@...r.kernel.org,
        bridge@...ts.linux-foundation.org,
        Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [PATCH net-next] net: bridge: add STP xstats

On 10/12/2019 09:49, Nikolay Aleksandrov wrote:
> On 10/12/2019 01:05, Vivien Didelot wrote:
>> This adds rx_bpdu, tx_bpdu, rx_tcn, tx_tcn, transition_blk,
>> transition_fwd xstats counters to the bridge ports copied over via
>> netlink, providing useful information for STP.
>>
>> Signed-off-by: Vivien Didelot <vivien.didelot@...il.com>
>> ---
>>  include/uapi/linux/if_bridge.h | 10 ++++++++++
>>  net/bridge/br_if.c             |  8 ++++++++
>>  net/bridge/br_netlink.c        |  9 +++++++++
>>  net/bridge/br_private.h        |  9 +++++++++
>>  net/bridge/br_stp.c            | 25 +++++++++++++++++++++++++
>>  net/bridge/br_stp_bpdu.c       | 12 ++++++++++++
>>  net/bridge/br_stp_if.c         | 27 +++++++++++++++++++++++++++
>>  7 files changed, 100 insertions(+)
>>
[snip]
>>  }
>>  
>> @@ -419,6 +420,12 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
>>  	if (p == NULL)
>>  		return ERR_PTR(-ENOMEM);
>>  
>> +	p->stp_stats = netdev_alloc_pcpu_stats(struct br_stp_stats);
>> +	if (!p->stp_stats) {
>> +		kfree(p);
>> +		return ERR_PTR(-ENOMEM);
>> +	}
>> +
>>  	p->br = br;
>>  	dev_hold(dev);
>>  	p->dev = dev;
>> @@ -432,6 +439,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
>>  	err = br_multicast_add_port(p);
>>  	if (err) {
>>  		dev_put(dev);
>> +		free_percpu(p->stp_stats);
>>  		kfree(p);
>>  		p = ERR_PTR(err);
>>  	}
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index a0a54482aabc..03aced1f862b 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -1597,6 +1597,15 @@ static int br_fill_linkxstats(struct sk_buff *skb,
>>  		}
>>  	}
>>  
>> +	if (p) {
>> +		struct bridge_stp_xstats xstats;
> 
> Please rename the local var here, using just xstats is misleading.
> Maybe stp_xstats ?
> 

Actually if this gets re-written to follow the mcast dump the local var
would disappear altogether.