Vladimir Oltean
2022-Jul-17 13:46 UTC
[Bridge] [PATCH net-next v1 1/1] net: bridge: ensure that link-local traffic cannot unlock a locked port
Hi Hans, On Fri, Jul 01, 2022 at 06:07:10PM +0200, Hans S wrote:> 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. > > 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.I think the discussion derailed at this exact point, when you responded that "Mac-Auth requires learning turned on". What precise feature do you describe when you say "Mac-Auth"? Do you mean 802.1X MAC-based authentication in general (where data plane packets on a locked port are dropped unless their MAC SA is in the FDB, and populating the FDB is *entirely* up to user space, there aren't any locked FDB entries on locked ports), or MAC authentication *bypass* (where the kernel auto-adds locked FDB entries on locked ports)? I *think* it's just the bypass that requires learning in mv88e6xxx. But the bypass (the feature where the kernel auto-adds locked FDB entries on locked ports) doesn't exist in net-next. Here, what happens is that a locked port learns the MAC SA from the traffic it didn't drop, i.e. link-local. In other words, the bridge behaves as expected and instructed: +locked +learning will cause just that. It's the administrator's fault for not disabling learning. It's also the mv88e6xxx driver's fault for not validating the "locked" + "learning" brport flag *combination* until it properly supports "+locked +learning" (the feature you are currently working on). I'm still confused why we don't just say that "+locked -learning" means plain 802.1X, "+locked +learning" means MAB where we learn locked FDB entries.
Vladimir Oltean
2022-Jul-17 14:03 UTC
[Bridge] [PATCH net-next v1 1/1] net: bridge: ensure that link-local traffic cannot unlock a locked port
On Sun, Jul 17, 2022 at 04:46:10PM +0300, Vladimir Oltean wrote:> Here, what happens is that a locked port learns the MAC SA from the > traffic it didn't drop, i.e. link-local. In other words, the bridge > behaves as expected and instructed: +locked +learning will cause just > that. It's the administrator's fault for not disabling learning. > It's also the mv88e6xxx driver's fault for not validating the "locked" + > "learning" brport flag *combination* until it properly supports "+locked > +learning" (the feature you are currently working on). > > I'm still confused why we don't just say that "+locked -learning" means > plain 802.1X, "+locked +learning" means MAB where we learn locked FDB entries.Or is it the problem that a "+locked +learning" bridge port will learn MAC SA from link-local traffic, but it will create FDB entries without the locked flag while doing so? The mv88e6xxx driver should react to the 'locked' flag from both directions (ADD_TO_DEVICE too, not just ADD_TO_BRIDGE).