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