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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 23 Sep 2021 11:35:28 +0000 From: Vladimir Oltean <vladimir.oltean@....com> To: Xiaoliang Yang <xiaoliang.yang_1@....com> CC: "davem@...emloft.net" <davem@...emloft.net>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "allan.nielsen@...rochip.com" <allan.nielsen@...rochip.com>, "joergen.andreasen@...rochip.com" <joergen.andreasen@...rochip.com>, "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>, "vinicius.gomes@...el.com" <vinicius.gomes@...el.com>, "michael.chan@...adcom.com" <michael.chan@...adcom.com>, "saeedm@...lanox.com" <saeedm@...lanox.com>, "jiri@...lanox.com" <jiri@...lanox.com>, "idosch@...lanox.com" <idosch@...lanox.com>, "alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>, "kuba@...nel.org" <kuba@...nel.org>, Po Liu <po.liu@....com>, Leo Li <leoyang.li@....com>, "f.fainelli@...il.com" <f.fainelli@...il.com>, "andrew@...n.ch" <andrew@...n.ch>, "vivien.didelot@...il.com" <vivien.didelot@...il.com>, Claudiu Manoil <claudiu.manoil@....com> Subject: Re: [PATCH v4 net-next 5/8] net: dsa: felix: support psfp filter on vsc9959 On Thu, Sep 23, 2021 at 11:23:45AM +0000, Xiaoliang Yang wrote: > Hi Vladimir, > > On Thu, Sep 23, 2021 at 15:45:16 +0000, Vladimir Oltean wrote: > > > Maybe we need to use ocelot_mact_learn() instead of > > > ocelot_mact_write() after setting SFID in StreamData. I think this can > > > avoid writing a wrong entry. > > > > So you're thinking of introducing a new ocelot_mact_learn_with_streamdata(), > > that writes the SFID and SSID of the STREAMDATA too, instead of editing them > > in-place for an existing MAC table entry, and then issuing a LEARN MAC Table > > command which would hopefully transfer the entire data structure to the MAC > > table? > > > > Have you tried that? > > Yes, I have tried. I mean writes SFID of STREAMDATA in > vsc9959_mact_stream_set() first, then calls ocelot_mact_learn() > function to write VID, mac and STREAMDATA in MAC table. We don't need > to introduce a new function. Once we call ocelot_mact_learn() > function, STREAMDATA will be stored in the learned entry. > > > > > In the documentation for the LEARN MAC Table command, I see: > > > > Purpose: Insert/learn new entry in MAC table. Position given by (MAC, VID) > > > > Use: Configure MAC and VID of the new entry in MACHDATA and MACLDATA. > > Configure remaining entry fields in MACACCESS. The location in the MAC > > table is calculated based on (MAC, VID). > > > > I just hope it will transfer the STREAMDATA too, it doesn't explicitly say that it > > will... > > > > And assuming it does, will the LEARN command overwrite an existing static > > FDB entry which has the same MAC DA and VLAN ID, but not SFID? > > I haven't tried that either. > > I tried the case that when MAC table index has changed, STREAMDATA > will keep move with VID and MAC. The entry { STREAMDATA , VID, MAC} > also can overwrite a static exist entry. I think we can do like this. Ok, so maybe we should do that? Even though I must say I don't really like the idea of partially writing MAC table entry data from the vsc9959 driver, and partially from ocelot_mact_learn. I also have this patch pending: https://patchwork.kernel.org/project/netdevbpf/patch/20210824114049.3814660-4-vladimir.oltean@nxp.com/ and concurrency will be an absolute mess. The ocelot->mact_lock will need to be taken _before_ we start writing the STREAMDATA, so this variant of ocelot_mact_learn will still have to stay somewhere in the ocelot library, and be organized something like this: __ocelot_mact_learn() { do what ocelot_mact_learn() currently does } ocelot_mact_learn() { mutex_lock(&ocelot->mact_lock); __ocelot_mact_learn(); mutex_unlock(&ocelot->mact_lock); } ocelot_mact_learn_streamdata() { mutex_lock(&ocelot->mact_lock); write_streamdata(); __ocelot_mact_learn(); mutex_unlock(&ocelot->mact_lock); } otherwise I would need to introduce avoidable refactoring in the driver. In fact, could you please pick up that mact_lock patch? Even if the rtnl_mutex was not dropped yet, the extra lock should not hurt anyone.
Powered by blists - more mailing lists