Ido Schimmel
2022-Jul-01 13:51 UTC
[Bridge] [PATCH net-next v1 1/1] net: bridge: ensure that link-local traffic cannot unlock a locked port
On Fri, Jul 01, 2022 at 09:47:24AM +0200, Hans S wrote:> One question though... wouldn't it be an issue that the mentioned > option setting is bridge wide, while the patch applies a per-port > effect?Why would it be an issue? To me, the bigger issue is changing the semantics of "locked" in 5.20 compared to previous kernels. What is even the use case for enabling learning when the port is locked? In current kernels, only SAs from link local traffic will be learned, but with this patch, nothing will be learned. So why enable learning in the first place? As an administrator, I mark a port as "locked" so that only traffic with SAs that I configured will be allowed. Enabling learning when the port is locked seems to defeat the purpose? It would be helpful to explain why mv88e6xxx needs to have learning enabled in the first place. IIUC, the software bridge can function correctly with learning disabled. It might be better to solve this in mv88e6xxx so that user space would not need to enable learning on the SW bridge and then work around issues caused by it such as learning from link local traffic.
Vladimir Oltean
2022-Jul-01 15:27 UTC
[Bridge] [PATCH net-next v1 1/1] net: bridge: ensure that link-local traffic cannot unlock a locked port
On Fri, Jul 01, 2022 at 04:51:44PM +0300, Ido Schimmel wrote:> On Fri, Jul 01, 2022 at 09:47:24AM +0200, Hans S wrote: > > One question though... wouldn't it be an issue that the mentioned > > option setting is bridge wide, while the patch applies a per-port > > effect? > > Why would it be an issue? To me, the bigger issue is changing the > semantics of "locked" in 5.20 compared to previous kernels. > > What is even the use case for enabling learning when the port is locked? > In current kernels, only SAs from link local traffic will be learned, > but with this patch, nothing will be learned. So why enable learning in > the first place? As an administrator, I mark a port as "locked" so that > only traffic with SAs that I configured will be allowed. Enabling > learning when the port is locked seems to defeat the purpose?I think if learning on a locked port doesn't make sense, the bridge should just reject that configuration.
Hans S
2022-Jul-01 16:07 UTC
[Bridge] [PATCH net-next v1 1/1] net: bridge: ensure that link-local traffic cannot unlock a locked port
There is several issues when learning is turned off with the mv88e6xxx driver: Mac-Auth requires learning turned on, otherwise there will be no miss violation interrupts afair. Refreshing of ATU entries does not work with learning turn off, as the PAV is set to zero when learning is turned off. This then further eliminates the use of the HoldAt1 feature and age-out interrupts. With dynamic ATU entries (an upcoming patch set), an authorized unit gets a dynamic ATU entry, and if it goes quiet for 5 minutes, it's entry will age out and thus get removed. That also solves the port relocation issue as if a device relocates to another port it will be able to get access again after 5 minutes. On Fri, Jul 1, 2022 at 3:51 PM Ido Schimmel <idosch at nvidia.com> wrote:> > On Fri, Jul 01, 2022 at 09:47:24AM +0200, Hans S wrote: > > One question though... wouldn't it be an issue that the mentioned > > option setting is bridge wide, while the patch applies a per-port > > effect? > > Why would it be an issue? To me, the bigger issue is changing the > semantics of "locked" in 5.20 compared to previous kernels. > > What is even the use case for enabling learning when the port is locked? > In current kernels, only SAs from link local traffic will be learned, > but with this patch, nothing will be learned. So why enable learning in > the first place? As an administrator, I mark a port as "locked" so that > only traffic with SAs that I configured will be allowed. Enabling > learning when the port is locked seems to defeat the purpose? > > It would be helpful to explain why mv88e6xxx needs to have learning > enabled in the first place. IIUC, the software bridge can function > correctly with learning disabled. It might be better to solve this in > mv88e6xxx so that user space would not need to enable learning on the SW > bridge and then work around issues caused by it such as learning from > link local traffic.