Feedback-ID: i494840e7:Fastmail
Date:   Tue, 31 Jan 2023 21:25:21 +0200
From:   Ido Schimmel <idosch@idosch.org>
To:     "Hans J. Schultz" <netdev@kapio-technology.com>
Cc:     davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org,
        Florian Fainelli <f.fainelli@gmail.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Vladimir Oltean <olteanv@gmail.com>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Kurt Kanzenbach <kurt@linutronix.de>,
        Hauke Mehrtens <hauke@hauke-m.de>,
        Woojung Huh <woojung.huh@microchip.com>,
        "maintainer:MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER" 
        <UNGLinuxDriver@microchip.com>, Sean Wang <sean.wang@mediatek.com>,
        Landen Chao <Landen.Chao@mediatek.com>,
        DENG Qingfang <dqfext@gmail.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        =?iso-8859-1?Q?Cl=E9ment_L=E9ger?= <clement.leger@bootlin.com>,
        Jiri Pirko <jiri@resnulli.us>,
        Ivan Vecera <ivecera@redhat.com>,
        Roopa Prabhu <roopa@nvidia.com>,
        Nikolay Aleksandrov <razor@blackwall.org>,
        Russell King <linux@armlinux.org.uk>,
        Christian Marangi <ansuelsmth@gmail.com>,
        open list <linux-kernel@vger.kernel.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-arm-kernel@lists.infradead.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@lists.infradead.org>,
        "open list:RENESAS RZ/N1 A5PSW SWITCH DRIVER" 
        <linux-renesas-soc@vger.kernel.org>,
        "moderated list:ETHERNET BRIDGE" <bridge@lists.linux-foundation.org>
Subject: Re: [PATCH net-next 0/5] ATU and FDB synchronization on locked ports
Message-ID: <Y9lrIWMnWLqGreZL@shredder>
References: <20230130173429.3577450-1-netdev@kapio-technology.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230130173429.3577450-1-netdev@kapio-technology.com>
Precedence: bulk

On Mon, Jan 30, 2023 at 06:34:24PM +0100, Hans J. Schultz wrote:
> This patch set makes it possible to have synchronized dynamic ATU and FDB
> entries on locked ports. As locked ports are not able to automatically
> learn, they depend on userspace added entries, where userspace can add
> static or dynamic entries. The lifetime of static entries are completely
> dependent on userspace intervention, and thus not of interest here. We
> are only concerned with dynamic entries, which can be added with a
> command like:
> 
> bridge fdb replace ADDR dev <DEV> master dynamic
> 
> We choose only to support this feature on locked ports, as it involves
> utilizing the CPU to handle ATU related switchcore events (typically
> interrupts) and thus can result in significant performance loss if
> exposed to heavy traffic.

Not sure I understand this reasoning. I was under the impression that
hostapd is installing dynamic entries instead of static ones since the
latter are not flushed when carrier is lost. Therefore, with static
entries it is possible to unplug a host (potentially plugging a
different one) and not lose authentication.

> 
> On locked ports it is important for userspace to know when an authorized
> station has become silent, hence not breaking the communication of a
> station that has been authorized based on the MAC-Authentication Bypass
> (MAB) scheme. Thus if the station keeps being active after authorization,
> it will continue to have an open port as long as it is active. Only after
> a silent period will it have to be reauthorized. As the ageing process in
> the ATU is dependent on incoming traffic to the switchcore port, it is
> necessary for the ATU to signal that an entry has aged out, so that the
> FDB can be updated at the correct time.

Why mention MAB at all? Don't you want user space to always use dynamic
entries to authenticate hosts regardless of 802.1X/MAB?

> 
> This patch set includes a solution for the Marvell mv88e6xxx driver, where
> for this driver we use the Hold-At-One feature so that an age-out
> violation interrupt occurs when a station has been silent for the
> system-set age time. The age out violation interrupt allows the switchcore
> driver to remove both the ATU and the FDB entry at the same time.
> 
> It is up to the maintainers of other switchcore drivers to implement the
> feature for their specific driver.
> 
> Hans J. Schultz (5):
>   net: bridge: add dynamic flag to switchdev notifier
>   net: dsa: propagate flags down towards drivers
>   drivers: net: dsa: add fdb entry flags incoming to switchcore drivers
>   net: bridge: ensure FDB offloaded flag is handled as needed
>   net: dsa: mv88e6xxx: implementation of dynamic ATU entries

Will try to review tomorrow, but it looks like this set is missing
selftests. What about extending bridge_locked_port.sh?