Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 23 Aug 2022 09:48:17 +0300
From:   Ido Schimmel <idosch@nvidia.com>
To:     netdev@kapio-technology.com
Cc:     Vladimir Oltean <olteanv@gmail.com>, davem@davemloft.net,
        kuba@kernel.org, netdev@vger.kernel.org,
        Andrew Lunn <andrew@lunn.ch>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>, Jiri Pirko <jiri@resnulli.us>,
        Ivan Vecera <ivecera@redhat.com>,
        Roopa Prabhu <roopa@nvidia.com>,
        Nikolay Aleksandrov <razor@blackwall.org>,
        Shuah Khan <shuah@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        linux-kernel@vger.kernel.org, bridge@lists.linux-foundation.org,
        linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v4 net-next 3/6] drivers: net: dsa: add locked fdb entry
 flag to drivers
Message-ID: <YwR4MQ2xOMlvKocw@shredder>
References: <5a4cfc6246f621d006af69d4d1f61ed1@kapio-technology.com>
 <YvkM7UJ0SX+jkts2@shredder>
 <34dd1318a878494e7ab595f8727c7d7d@kapio-technology.com>
 <YwHZ1J9DZW00aJDU@shredder>
 <ce4266571b2b47ae8d56bd1f790cb82a@kapio-technology.com>
 <YwMW4iGccDu6jpaZ@shredder>
 <c2822d6dd66a1239ff8b7bfd06019008@kapio-technology.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <c2822d6dd66a1239ff8b7bfd06019008@kapio-technology.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?AVLSS0nmPVcOuXatzv4in5Kb9yg8X0b0Pi5hKejPnePhKaViH+LeS92+Ay18?=
 =?us-ascii?Q?753PyIok78nK7j+mm/E/s2WG//8MzRbIT14QOyTl0HX/Mhv3ZjP21lXlQh8F?=
 =?us-ascii?Q?8bXK6z+eIPG0yRGjGMul7rQIdFrmUdHfUFDm3/hIfXy6/5aEPYxDo8RJKbz5?=
 =?us-ascii?Q?eyJt8KRci7mpfRJWHcHPpOoZsNp/WopYsjJr3mmXQgnLdRd59n3x4IoxOgqf?=
 =?us-ascii?Q?MfXkhIrQMQ4ORVbO/5pYJxyID5eRv+giq2BYMhsI1I6RnaXm6JMoHV8fdZd/?=
 =?us-ascii?Q?m11zWxWHm3jpYx7r8RnTr34lnRjM7egBvQTw/EMQsG8Br16EYmhiKrJm/cJ3?=
 =?us-ascii?Q?Y4QVUNT3RdPKLPUDcVNjTb7P7PI6l9fr8TtHobfFUoSO91HNZ5y3yJHAY4AJ?=
 =?us-ascii?Q?LP9qyvXsqWmk6zOclP0MVtwfb6PD+aH9SHr2okemtZA99hdQb0u/vYsJwT2U?=
 =?us-ascii?Q?2BP1680bys3BW0mouOpMpMMyJ3ZRNlNa6/hSlTxlGAok/6AgF3Ti8GJRA5d7?=
 =?us-ascii?Q?3yAwuEycrLY0un4SPw/ns5VnwI89Bhd7xCDTT/A/cMHhrc25hEt8nmjZtEPB?=
 =?us-ascii?Q?mh4kZByMURRfuJX5y4jsQOfIGLGH/ZcVwXYa4zkeunRzIWQ1vlRwvmkbKpfq?=
 =?us-ascii?Q?EqH9xYtpqv1uwmF2EOtcbJ5iiAH0319VVgyilRY++CiXjmHU+OBP9MsPhdyl?=
 =?us-ascii?Q?hT0PYINBR/zorUqn9XweePqN2L1u3csdOuWYgcgcsmMepdTrlypbYUReUXCk?=
 =?us-ascii?Q?eQe047QuIF1Gw7lWhcBjDm7mTOR4MxtK7h4u0FuP2dxKAHHtqchEVaxT1jwB?=
 =?us-ascii?Q?FO6vqqgT2DldQvcMUX3k18w5cqt4NgP+z5teWlFKOS0kFNPc8VUwicEqMRo2?=
 =?us-ascii?Q?qvIxqN2UAG9V5yncCFlCslj4IvG5S36WI8POoEVTV9thS2UMninUGQF7gkb5?=
 =?us-ascii?Q?HfFyNvPmdjpGMsnJNE8vCkG1x8Punt7+VcJqsAtkJbG5y1DLOf6xlDxb1/qa?=
 =?us-ascii?Q?UnH5jolfgxKcYkaldZkVH/92M6I5IdNG9NNdYMtTwimjNIlQ8GEBllSitYdN?=
 =?us-ascii?Q?qgueUp/zrD+Trs1/RITHRF/Eg53TS81BaXquJKk6xcOdq/VDupUYmmA4SMh1?=
 =?us-ascii?Q?H3o/Q65G/+nNoQQjkTj2VUu+PtJCIBE9AQqxCj4QqadmhTZLXdlB9C6hvDr0?=
 =?us-ascii?Q?TxLx5a5ONfRJpCl3BRsg24blXX+ijp/YoTj6rzjtGTrEL4uRtZn7W1bSKurx?=
 =?us-ascii?Q?GipEBNNQqXXd9crePrxAMgW4c6cKX2mF5sg0TAUtX4aiRTJEcqwMNdveTIWZ?=
 =?us-ascii?Q?LSDX1NnLXSE8QX+J8YNHlRBKuhJaZinEHpQ9I4QRuN+J9zGRh1m+5QWiMkyp?=
 =?us-ascii?Q?ecfri5C6ea9ShmpjierMgA94b+MIm6zgWncyYe0493jrudWh/uxBtWW+9s8M?=
 =?us-ascii?Q?Kcr7mRp/VtkV9krzHy0RZ8LKnNVwF9j0JJrQfC/+NeKzHtN5K3jIvCINYx89?=
 =?us-ascii?Q?H1xfSpPFt1MSYWnySqQ3IVtzasFD+tNA4uJKjLQJLs6L+qPZ1fJdDKUr9Tov?=
 =?us-ascii?Q?keuVGErh7UfXQqVKe4Ic2XS2Hszb0roeL2MSM+GG?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1990fa58-d901-4ad5-b15e-08da84d3797a
X-MS-Exchange-CrossTenant-AuthSource: IA1PR12MB6163.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Aug 2022 06:48:23.8802
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: iu0t9qxIW9++gLibiyStO5K6Pw0wYuYipnS0KPs79rfjL5GTnBIS0EhWMGnDaqwToG9kg1nWuUFHoVN3pl+r2A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR12MB4609
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Aug 22, 2022 at 09:49:28AM +0200, netdev@kapio-technology.com wrote:
> On 2022-08-22 07:40, Ido Schimmel wrote:
> > On Sun, Aug 21, 2022 at 03:43:04PM +0200, netdev@kapio-technology.com
> > wrote:
> > 
> > I personally think that the mv88e6xxx semantics are very weird (e.g., no
> > roaming, traffic blackhole) and I don't want them to determine how the
> > feature works in the pure software bridge or other hardware
> > implementations. On the other hand, I understand your constraints and I
> > don't want to create a situation where user space is unable to
> > understand how the data path works from the bridge FDB dump with
> > mv88e6xxx.
> > 
> > My suggestion is to have mv88e6xxx report the "locked" entry to the
> > bridge driver with additional flags that describe its behavior in terms
> > of roaming, ageing and forwarding.
> > 
> > In terms of roaming, since in mv88e6xxx the entry can't roam you should
> > report the entry with the "sticky" flag.
> 
> As I am not familiar with roaming in this context, I need to know how the SW
> bridge should behave in this case.

I think I wasn't clear enough. The idea is to make the bridge compatible
with mv88e6xxx in a way that is discoverable by user space by having
mv88e6xxx add the locked entry with flags that describe the hardware
behavior. Therefore, it's not a matter of "how the SW bridge should
behave", but having it behave in a way that matches the offloaded data
path.

From what I was able to understand from you, the "locked" entry cannot
roam at all in mv88e6xxx, which can be described by the "sticky" flag.

> In this I am assuming that roaming is regarding unauthorized entries.

Yes, talking about "locked" entries that are notified by mv88e6xxx to
the bridge.

> In this case, is the roaming only between locked ports or does the
> roaming include that the entry can move to a unlocked port, resulting
> in the locked flag getting removed?

Any two ports. If the "locked" entry in mv88e6xxx cannot move once
installed, then the "sticky" flag accurately describes it.

> 
> > In terms of ageing, since
> > mv88e6xxx is the one doing the ageing and not the bridge driver, report
> > the entry with the "extern_learn" flag.
> 
> Just for the record, I see that entries coming from the driver to the bridge
> will always have the "extern learn" flag set as can be seen from the
> SWITCHDEV_FDB_ADD_TO_BRIDGE events handling in br_switchdev_event() in br.c,
> which I think is the correct behavior.

Yes.

> 
> > In terms of forwarding, in
> > mv88e6xxx the entry discards all matching packets. We can introduce a
> > new FDB flag that instructs the entry to silently discard all matching
> > packets. Like we have with blackhole routes and nexthops.
> 
> Any suggestions to the name of this flag?

I'm not good at naming, but "blackhole" is at least consistent with what
we already have for routes and nexthop objects.

> 
> > 
> > I believe that the above suggestion allows you to fully describe how
> > these entries work in mv88e6xxx while keeping the bridge driver in sync
> > with complete visibility towards user space.
> > 
> > It also frees the pure software implementation from the constraints of
> > mv88e6xxx, allowing "locked" entries to behave like any other
> > dynamically learned entries modulo the fact that they cannot "unlock" a
> > locked port.
> > 
> > Yes, it does mean that user space will get a bit different behavior with
> > mv88e6xxx compared to a pure software solution, but a) It's only the
> > corner cases that act a bit differently. As a whole, the feature works
> > largely the same. b) User space has complete visibility to understand
> > the behavior of the offloaded data path.
> > 
> 
> > > 
> > > I will change it in iproute2 to:
> > > bridge link set dev DEV mab on|off
> > 
> > And s/BR_PORT_MACAUTH/BR_PORT_MAB/ ?
> 
> Sure, I will do that. :-)

Thanks