[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <498b31ea-bafc-1f44-d4bd-354f30a71972@cumulusnetworks.com>
Date: Wed, 11 Dec 2019 11:40:31 +0200
From: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To: Vivien Didelot <vivien.didelot@...il.com>
Cc: "David S. Miller" <davem@...emloft.net>,
Roopa Prabhu <roopa@...ulusnetworks.com>,
netdev@...r.kernel.org, bridge@...ts.linux-foundation.org,
Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [PATCH net-next v2] net: bridge: add STP xstats
On 11/12/2019 04:02, Vivien Didelot wrote:
> Hi Nikolay,
>
> On Tue, 10 Dec 2019 23:45:13 +0200, Nikolay Aleksandrov <nikolay@...ulusnetworks.com> wrote:
>>> + if (p) {
>>> + nla = nla_reserve_64bit(skb, BRIDGE_XSTATS_STP,
>>> + sizeof(p->stp_xstats),
>>> + BRIDGE_XSTATS_PAD);
>>> + if (!nla)
>>> + goto nla_put_failure;
>>> +
>>> + memcpy(nla_data(nla), &p->stp_xstats, sizeof(p->stp_xstats));
>>
>> You need to take the STP lock here to get a proper snapshot of the values.
>
> Good catch! I see a br->multicast_lock but no br->stp_lock. Is this what
> you expect?
>
> spin_lock_bh(&br->lock);
> memcpy(nla_data(nla), &p->stp_xstats, sizeof(p->stp_xstats));
> spin_unlock_bh(&br->lock);
>
Yeah, this is a very old lock (pre-git) which needs some attention. :)
That is the lock and the above looks good to me.
>
> Thanks,
>
> Vivien
>
Cheers,
Nik
Powered by blists - more mailing lists