On 11/12/2019 04:02, Vivien Didelot wrote:
> Hi Nikolay,
>
> On Tue, 10 Dec 2019 23:45:13 +0200, Nikolay Aleksandrov
> <niko...@cumulusnetworks.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
