From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0F2E7C433EF for ; Sun, 10 Jul 2022 08:20:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229572AbiGJIUj (ORCPT ); Sun, 10 Jul 2022 04:20:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43738 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229476AbiGJIUg (ORCPT ); Sun, 10 Jul 2022 04:20:36 -0400 Received: from NAM11-CO1-obe.outbound.protection.outlook.com (mail-co1nam11on2066.outbound.protection.outlook.com [40.107.220.66]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 43387BC0E; Sun, 10 Jul 2022 01:20:35 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Dg7V3AP9cETJQSsFjRRw0Ri+5zclojKL8ZjjrXD8JwzovIZLrxCXMkLHFOzJLqoVI87Iwv66TQgT5em085HnQmyZBnrWuaICX36pyJe+RvlXCo5JN2qOdNxmr3p/w/HxidvJ7yv9wByQCaBW8XTE0u1Ta/pavMe8TuHonSVa1DNBBk5uQMf0EnQb6fQzV4TKNB7t2bwuB9WGTJn4fOLUaPBbaM/imQ6KmmJ1aI68NnYfd60Q687zqJtwL74kGlpFWse2WQ4z31AB1IaPooG7iSjrFzb0eCGjTNL6qgM8fjwVn43n2gaqqZ5qS+dquTxDec+UJQfGhU5cDxCM5+IhHg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=6aOSOUd9RBEHqOP1XcSOPcC/VbZMbNhD9MrOCZg4pkA=; b=ZsqW2ANbPVeWiYJLpgF2bgrwZNweXZgyT5OB3gWwBglTCIx5TDq4i3/ITRUUG3lTXxESmznsU/491OAp5Lcu0N36w9nbR25rHKe383bJ5v0jFVQEjsay6pIGdL6FmzblofX1srNVHDijX6ocJyHm3gY1J+lKnwH2dgQf09fhN/VSpi5hftYOE36n+znwp7flhWB9PTPJl8l7ouRA0q9AAT44BNv7fJVCFtB/y7yDJPKe3OZUUgCBzQhVuy2Tq7/7Y2l286n0sCHD74pKQnMB1iUA8OFBSI0wxsM4+P//EqG0vjBROu4XTTlgIjQMhlhQt89GTxEWsCcm0ynMgep9Vw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=6aOSOUd9RBEHqOP1XcSOPcC/VbZMbNhD9MrOCZg4pkA=; b=cYCXhwwFC+6mNEoVFRjlhKGEOUKSpAD6VIWtM1VoiCy0C57G6Uj6RMrW3N9BFCnnZO3F6PxS/TOHVweKZ0pYViulJd6WGMYW36xuD9hva/ykV/qSPCUL0TnsQ72rnaIcgmRvQUZlJcFV6Y+Sg27P5AaxwzjSBf8MByHVjdduJHr7ul71c+DjlYQOxtLPnQh1/Pv30ygmXWE9xqvusUv7bjKaOmbYD+I3KPBugezScCKQ0Jy9/lTS0C6SAPqVo1ZvBNDOWRgcHH0VzE0UORPDPGlQoZwibHFD00bMK/2hYeNPDt2BLZ1hJVu3lI1q/1xMTbAbd9Swc/Mbyz4OqDto1Q== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=nvidia.com; Received: from CY5PR12MB6179.namprd12.prod.outlook.com (2603:10b6:930:24::22) by CH2PR12MB4037.namprd12.prod.outlook.com (2603:10b6:610:7a::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5417.15; Sun, 10 Jul 2022 08:20:33 +0000 Received: from CY5PR12MB6179.namprd12.prod.outlook.com ([fe80::a525:8fcf:95ec:f7ad]) by CY5PR12MB6179.namprd12.prod.outlook.com ([fe80::a525:8fcf:95ec:f7ad%9]) with mapi id 15.20.5417.025; Sun, 10 Jul 2022 08:20:33 +0000 Date: Sun, 10 Jul 2022 11:20:27 +0300 From: Ido Schimmel To: Hans Schultz Cc: davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org, Andrew Lunn , Vivien Didelot , Florian Fainelli , Vladimir Oltean , Eric Dumazet , Paolo Abeni , Jiri Pirko , Ivan Vecera , Roopa Prabhu , Nikolay Aleksandrov , Shuah Khan , Daniel Borkmann , linux-kernel@vger.kernel.org, bridge@lists.linux-foundation.org, linux-kselftest@vger.kernel.org Subject: Re: [PATCH v4 net-next 1/6] net: bridge: add locked entry fdb flag to extend locked port feature Message-ID: References: <20220707152930.1789437-1-netdev@kapio-technology.com> <20220707152930.1789437-2-netdev@kapio-technology.com> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220707152930.1789437-2-netdev@kapio-technology.com> X-ClientProxiedBy: MR1P264CA0153.FRAP264.PROD.OUTLOOK.COM (2603:10a6:501:54::11) To CY5PR12MB6179.namprd12.prod.outlook.com (2603:10b6:930:24::22) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 9b9542cd-be34-46ee-13e6-08da624d0ee9 X-MS-TrafficTypeDiagnostic: CH2PR12MB4037:EE_ X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 2U9f43Eq0iuU+Lk4fZrpjLFahKQymNh20MSjQUn6irw540lUcw8oCrQAHJfVTcGFI2ApcAMnhc40rqafxTXaR1P22FxFJllKcGH1EUUP3It3iSKRdLRSCyKNz+Oiu0ffY8HqA/kwEtj8Ejvx4y5xgPWq8AdJZl6V9uH9lgwlmXblRDWbfNoZnzlvFNeFoXscBGdEIZoW6pxUYhpRjUmZQCJAxfAW7RYsrt4+KdktZqDUODuFMB4W30fIZBNI5EKnS/W14HCMdn5VVZL8lPdCao+6+tYePsrFzWjt03sIaNjiIXMLepcwqA/p6s6RsucxX7sAc1YFyapSBSFG1j6qjjyprsqP3+nO26hA8ZzNR+pAMlPm9J6XNH5wpekqabPcNhU6Q7bgtZ2EjqZgqiHF3aUlSxwjZFCgL45kX/AUp5Hq8Ifp7+o73RcaIm1rM8hlTUb2M1fA7y3NsOkRhPPawWOLUfx9fydiMbtf9Qca1uXBUSqbp5LN7MD7ZVo1tn01jRfevUIjKdey1fz2wR3RN92V1q2SJtnKDhgENxBU99sSYmcrMChJaoH/dgwp+MH/l3a3UQ21FbpufHl+jwiwe5WQhPKKC1o0e9Itzpq2iGc1iRvYNR5Bc0l/8RPAvOUYkjEl9SIS6yC3I8orIMzlkF3eJVp1wlSsIozygHf2KCbgK/rZkn3M5nbPYprbB4xDrgpPmqV4TEJ/Z0r1pAhtthU8/LV6zoPGqB2iCjYO6OKC8khmUIStUhrBho5sXQ6mAlK41naEacb9KDYPed+UVA== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CY5PR12MB6179.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230016)(7916004)(4636009)(136003)(396003)(376002)(366004)(346002)(39860400002)(41300700001)(6512007)(9686003)(6666004)(2906002)(33716001)(86362001)(6486002)(478600001)(26005)(6506007)(316002)(6916009)(54906003)(38100700002)(8936002)(7416002)(8676002)(66476007)(66556008)(66946007)(5660300002)(4326008)(83380400001)(66574015)(186003);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?/K/T0k9eMOhvikHJLFAoLauXFIIB+yEdE2dTPHB6Lid1cFd1LmwN35AcnH0N?= =?us-ascii?Q?UqN1/7kJ7Q3nv59ea5LcIPc2HEPk6xmPNJIWvZ3TN+4podUPBB+1eGv4t7ao?= =?us-ascii?Q?9ExVgwHkWf4RdtHCmnjslTHu3hdqauEuBkm0a0WDKoA3IEJcZSRo75cVap7b?= =?us-ascii?Q?tp7JP3s0hfg2jPuC31tVeZWM7DbG6sflZG5pkI4Pb2p347J7Vs/Xj5K53wkp?= =?us-ascii?Q?DHZt70CExJu2wt7e3aGkYwW39zCMqeNDdCRE16DYFoV/Dt/TcHABa6WOtQaf?= =?us-ascii?Q?3n4DVPtVs2+bEGC8J9FZOxsbR/9SXfcFThA0YlymEpFJ/iSNOjbok4EWm5ch?= =?us-ascii?Q?y3ZKQzc9j0XabWwxIYu9i+viDyN4s7DhQvkEwnflHrMMbmaeAh+r/ACXbywF?= =?us-ascii?Q?ocIX8E4Np5K1BFAt7XPp66BAIZIQl5jOWM8XMw14HfZvUlKHqZazWTQDxCdX?= =?us-ascii?Q?wmeDmom0zX1tn2Vy9gxPa6x2vkeGcgyv2MOO52JIUIYPe9n6PB9KoHW2FFBR?= =?us-ascii?Q?gJv9vYXiBSoDwt+1VxozX0ZV9l4KaT7nvn4LLq/ELF66vJLa4WrGygo2VGO0?= =?us-ascii?Q?30wM9981Yushn+QhUwYQ/542v+qaCVTMJWTlbeywgaEzLcf4sVG6G0n1nPhL?= =?us-ascii?Q?mtNA0C34jPMr6fLou1NOzsNJV6mG7GTb9biR4kmv2db0raXFngo2JRj0UlfG?= =?us-ascii?Q?ICUngwHZP5XZoCqSS7CVP13CrP+LS5jsSUO/eCo6m3rALhI8AhJBHR5gqh4s?= =?us-ascii?Q?GJDajzkxPBNorsq1MGj5n69K7+IzITq3DCNicLkOCF4GtTyigbgACv5raUc6?= =?us-ascii?Q?48Y8cTrQ5d+W3GQ8M43MQ/qb/dIRyOgOOIWD8/4THC9APi8UiGZujTO1AGJ+?= =?us-ascii?Q?T8uHzPJ9Zu19GShAriJjT04j+O2J6S+o/11Wyq5mr7wQSlUQcilO4PVt1PSk?= =?us-ascii?Q?JKC+5WzTvnHseytcYfZNabHf1STWVcaOcryNC6Siq4NIdgyVO8tHfiG5DhII?= =?us-ascii?Q?FnE2PfV6eBOeOIr3KHHQRn1i0jxntC3S+zVX/FrKNe7T1wPnf6aq4yuPpinA?= =?us-ascii?Q?YcTPSEC/CkG5UM745QiwHRyCJoVsEdwqoRevEg97NWKmnfVPGvdvZ/HuQ0zf?= =?us-ascii?Q?7PO/W9hLXRlmOc+VqaTIFMHQ3if7DMXJrv+pMstmQSsYrs2NNxXh8wlJxDRZ?= =?us-ascii?Q?zSlIFiW3oJYpL7TelLPAOwhBG+kVIDe0NgDFRFRa0i23lLNlpPRp3ojmNSX3?= =?us-ascii?Q?9PZIDLBn3lzQwKj03k+6/mBSAzA8ucintpivO5rbKFVKLZu6j5nCwpkqR1f5?= =?us-ascii?Q?saVYob2ymthFro6WhMXqxu3YavzFDLpxoonsK3q1BEyTjEinbjmAS+ygwh1a?= =?us-ascii?Q?bubrWuQzN3QJl5JN2R+8HqoIDm1XrEgGxt5mS69QNGAp+QpPSYrbe8fAsTR5?= =?us-ascii?Q?kcoKbr9ZYmKfl+bkfoFCEJX01kKecxN1kcFmzQwvDTRTOPAqf1+hGV7E1xWZ?= =?us-ascii?Q?+JrHgBh0oGEmoCkk3dPU/9I4d/0u2+B77x1DawtXdIVumOfL/UkvggNspMgD?= =?us-ascii?Q?loQIs9WRdhHtbd3Ruq5K7vPvop+/1nkRi7MhHXVb?= X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-Network-Message-Id: 9b9542cd-be34-46ee-13e6-08da624d0ee9 X-MS-Exchange-CrossTenant-AuthSource: CY5PR12MB6179.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Jul 2022 08:20:32.9755 (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: qMnLUzSs3N6wKpoxApqpO9eavR86ydw+08yYxywhfSpdOXuuWMbC5Pr/WIRF04AwzUap2+FKrrRox4gdJ466IQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR12MB4037 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jul 07, 2022 at 05:29:25PM +0200, Hans Schultz wrote: > Add an intermediate state for clients behind a locked port to allow for > possible opening of the port for said clients. The clients mac address > will be added with the locked flag set, denying access through the port > for the mac address, but also creating a new FDB add event giving > userspace daemons the ability to unlock the mac address. This feature > corresponds to the Mac-Auth and MAC Authentication Bypass (MAB) named > features. The latter defined by Cisco. > > Only the kernel can set this FDB entry flag, while userspace can read > the flag and remove it by replacing or deleting the FDB entry. > > Signed-off-by: Hans Schultz > --- > include/uapi/linux/neighbour.h | 1 + > net/bridge/br_fdb.c | 12 ++++++++++++ > net/bridge/br_input.c | 10 +++++++++- > net/bridge/br_private.h | 3 ++- > 4 files changed, 24 insertions(+), 2 deletions(-) > > diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h > index 39c565e460c7..76d65b481086 100644 > --- a/include/uapi/linux/neighbour.h > +++ b/include/uapi/linux/neighbour.h > @@ -53,6 +53,7 @@ enum { > #define NTF_ROUTER (1 << 7) > /* Extended flags under NDA_FLAGS_EXT: */ > #define NTF_EXT_MANAGED (1 << 0) > +#define NTF_EXT_LOCKED (1 << 1) > > /* > * Neighbor Cache Entry States. > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c > index e7f4fccb6adb..ee9064a536ae 100644 > --- a/net/bridge/br_fdb.c > +++ b/net/bridge/br_fdb.c > @@ -105,6 +105,7 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br, > struct nda_cacheinfo ci; > struct nlmsghdr *nlh; > struct ndmsg *ndm; > + u32 ext_flags = 0; > > nlh = nlmsg_put(skb, portid, seq, type, sizeof(*ndm), flags); > if (nlh == NULL) > @@ -125,11 +126,16 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br, > ndm->ndm_flags |= NTF_EXT_LEARNED; > if (test_bit(BR_FDB_STICKY, &fdb->flags)) > ndm->ndm_flags |= NTF_STICKY; > + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags)) > + ext_flags |= NTF_EXT_LOCKED; > > if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr)) > goto nla_put_failure; > if (nla_put_u32(skb, NDA_MASTER, br->dev->ifindex)) > goto nla_put_failure; > + if (nla_put_u32(skb, NDA_FLAGS_EXT, ext_flags)) > + goto nla_put_failure; > + > ci.ndm_used = jiffies_to_clock_t(now - fdb->used); > ci.ndm_confirmed = 0; > ci.ndm_updated = jiffies_to_clock_t(now - fdb->updated); > @@ -171,6 +177,7 @@ static inline size_t fdb_nlmsg_size(void) > return NLMSG_ALIGN(sizeof(struct ndmsg)) > + nla_total_size(ETH_ALEN) /* NDA_LLADDR */ > + nla_total_size(sizeof(u32)) /* NDA_MASTER */ > + + nla_total_size(sizeof(u32)) /* NDA_FLAGS_EXT */ Need to add validation that 'NTF_EXT_LOCKED' is not set in 'NDA_FLAGS_EXT' in entries installed by user space. > + nla_total_size(sizeof(u16)) /* NDA_VLAN */ > + nla_total_size(sizeof(struct nda_cacheinfo)) > + nla_total_size(0) /* NDA_FDB_EXT_ATTRS */ > @@ -1082,6 +1089,11 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source, > modified = true; > } > > + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags)) { > + clear_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags); > + modified = true; > + } > + > if (fdb_handle_notify(fdb, notify)) > modified = true; > > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > index 68b3e850bcb9..3d15548cfda6 100644 > --- a/net/bridge/br_input.c > +++ b/net/bridge/br_input.c > @@ -110,8 +110,16 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb > br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid); > > if (!fdb_src || READ_ONCE(fdb_src->dst) != p || > - test_bit(BR_FDB_LOCAL, &fdb_src->flags)) > + test_bit(BR_FDB_LOCAL, &fdb_src->flags) || > + test_bit(BR_FDB_ENTRY_LOCKED, &fdb_src->flags)) { > + if (!fdb_src) { > + unsigned long flags = 0; > + > + __set_bit(BR_FDB_ENTRY_LOCKED, &flags); > + br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, flags); > + } We need a better definition of what 'BR_FDB_ENTRY_LOCKED' means. An entry marked with this flag is not good enough to let traffic with this SA ingress from a locked port, but if traffic is received from a different port with this DA, the bridge will forward it as known unicast. If we are going to tell switchdev drivers to ignore locked entries, packets with this DA will be flooded as unknown unicast in hardware. For mv88e6xxx you write "The mac address triggering the ATU miss violation will be added to the ATU with a zero-DPV". What does it mean? Traffic with this DA will be blackholed? It would be good to agree on one consistent behavior for all hardware implementations and the bridge driver. Do you know what happens in other implementations (e.g., Cisco)? FWIW, to me it makes sense to have the bridge ignore locked entries and let traffic be flooded. If user space does not want traffic to egress a locked port, then it can turn off flooding on the port while it is locked. > goto drop; > + } > } > > nbp_switchdev_frame_mark(p, skb); > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index 06e5f6faa431..47a3598d25c8 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -251,7 +251,8 @@ enum { > BR_FDB_ADDED_BY_EXT_LEARN, > BR_FDB_OFFLOADED, > BR_FDB_NOTIFY, > - BR_FDB_NOTIFY_INACTIVE > + BR_FDB_NOTIFY_INACTIVE, > + BR_FDB_ENTRY_LOCKED, > }; > > struct net_bridge_fdb_key { > -- > 2.30.2 > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 1106341619 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 6A65B4161A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=6aOSOUd9RBEHqOP1XcSOPcC/VbZMbNhD9MrOCZg4pkA=; b=cYCXhwwFC+6mNEoVFRjlhKGEOUKSpAD6VIWtM1VoiCy0C57G6Uj6RMrW3N9BFCnnZO3F6PxS/TOHVweKZ0pYViulJd6WGMYW36xuD9hva/ykV/qSPCUL0TnsQ72rnaIcgmRvQUZlJcFV6Y+Sg27P5AaxwzjSBf8MByHVjdduJHr7ul71c+DjlYQOxtLPnQh1/Pv30ygmXWE9xqvusUv7bjKaOmbYD+I3KPBugezScCKQ0Jy9/lTS0C6SAPqVo1ZvBNDOWRgcHH0VzE0UORPDPGlQoZwibHFD00bMK/2hYeNPDt2BLZ1hJVu3lI1q/1xMTbAbd9Swc/Mbyz4OqDto1Q== Date: Sun, 10 Jul 2022 11:20:27 +0300 From: Ido Schimmel Message-ID: References: <20220707152930.1789437-1-netdev@kapio-technology.com> <20220707152930.1789437-2-netdev@kapio-technology.com> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220707152930.1789437-2-netdev@kapio-technology.com> MIME-Version: 1.0 Subject: Re: [Bridge] [PATCH v4 net-next 1/6] net: bridge: add locked entry fdb flag to extend locked port feature List-Id: Linux Ethernet Bridging List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hans Schultz Cc: Ivan Vecera , Andrew Lunn , Florian Fainelli , Jiri Pirko , Daniel Borkmann , netdev@vger.kernel.org, Nikolay Aleksandrov , bridge@lists.linux-foundation.org, linux-kernel@vger.kernel.org, Vivien Didelot , Eric Dumazet , Paolo Abeni , linux-kselftest@vger.kernel.org, Roopa Prabhu , kuba@kernel.org, Vladimir Oltean , Shuah Khan , davem@davemloft.net On Thu, Jul 07, 2022 at 05:29:25PM +0200, Hans Schultz wrote: > Add an intermediate state for clients behind a locked port to allow for > possible opening of the port for said clients. The clients mac address > will be added with the locked flag set, denying access through the port > for the mac address, but also creating a new FDB add event giving > userspace daemons the ability to unlock the mac address. This feature > corresponds to the Mac-Auth and MAC Authentication Bypass (MAB) named > features. The latter defined by Cisco. > > Only the kernel can set this FDB entry flag, while userspace can read > the flag and remove it by replacing or deleting the FDB entry. > > Signed-off-by: Hans Schultz > --- > include/uapi/linux/neighbour.h | 1 + > net/bridge/br_fdb.c | 12 ++++++++++++ > net/bridge/br_input.c | 10 +++++++++- > net/bridge/br_private.h | 3 ++- > 4 files changed, 24 insertions(+), 2 deletions(-) > > diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h > index 39c565e460c7..76d65b481086 100644 > --- a/include/uapi/linux/neighbour.h > +++ b/include/uapi/linux/neighbour.h > @@ -53,6 +53,7 @@ enum { > #define NTF_ROUTER (1 << 7) > /* Extended flags under NDA_FLAGS_EXT: */ > #define NTF_EXT_MANAGED (1 << 0) > +#define NTF_EXT_LOCKED (1 << 1) > > /* > * Neighbor Cache Entry States. > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c > index e7f4fccb6adb..ee9064a536ae 100644 > --- a/net/bridge/br_fdb.c > +++ b/net/bridge/br_fdb.c > @@ -105,6 +105,7 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br, > struct nda_cacheinfo ci; > struct nlmsghdr *nlh; > struct ndmsg *ndm; > + u32 ext_flags = 0; > > nlh = nlmsg_put(skb, portid, seq, type, sizeof(*ndm), flags); > if (nlh == NULL) > @@ -125,11 +126,16 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br, > ndm->ndm_flags |= NTF_EXT_LEARNED; > if (test_bit(BR_FDB_STICKY, &fdb->flags)) > ndm->ndm_flags |= NTF_STICKY; > + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags)) > + ext_flags |= NTF_EXT_LOCKED; > > if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr)) > goto nla_put_failure; > if (nla_put_u32(skb, NDA_MASTER, br->dev->ifindex)) > goto nla_put_failure; > + if (nla_put_u32(skb, NDA_FLAGS_EXT, ext_flags)) > + goto nla_put_failure; > + > ci.ndm_used = jiffies_to_clock_t(now - fdb->used); > ci.ndm_confirmed = 0; > ci.ndm_updated = jiffies_to_clock_t(now - fdb->updated); > @@ -171,6 +177,7 @@ static inline size_t fdb_nlmsg_size(void) > return NLMSG_ALIGN(sizeof(struct ndmsg)) > + nla_total_size(ETH_ALEN) /* NDA_LLADDR */ > + nla_total_size(sizeof(u32)) /* NDA_MASTER */ > + + nla_total_size(sizeof(u32)) /* NDA_FLAGS_EXT */ Need to add validation that 'NTF_EXT_LOCKED' is not set in 'NDA_FLAGS_EXT' in entries installed by user space. > + nla_total_size(sizeof(u16)) /* NDA_VLAN */ > + nla_total_size(sizeof(struct nda_cacheinfo)) > + nla_total_size(0) /* NDA_FDB_EXT_ATTRS */ > @@ -1082,6 +1089,11 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source, > modified = true; > } > > + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags)) { > + clear_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags); > + modified = true; > + } > + > if (fdb_handle_notify(fdb, notify)) > modified = true; > > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > index 68b3e850bcb9..3d15548cfda6 100644 > --- a/net/bridge/br_input.c > +++ b/net/bridge/br_input.c > @@ -110,8 +110,16 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb > br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid); > > if (!fdb_src || READ_ONCE(fdb_src->dst) != p || > - test_bit(BR_FDB_LOCAL, &fdb_src->flags)) > + test_bit(BR_FDB_LOCAL, &fdb_src->flags) || > + test_bit(BR_FDB_ENTRY_LOCKED, &fdb_src->flags)) { > + if (!fdb_src) { > + unsigned long flags = 0; > + > + __set_bit(BR_FDB_ENTRY_LOCKED, &flags); > + br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, flags); > + } We need a better definition of what 'BR_FDB_ENTRY_LOCKED' means. An entry marked with this flag is not good enough to let traffic with this SA ingress from a locked port, but if traffic is received from a different port with this DA, the bridge will forward it as known unicast. If we are going to tell switchdev drivers to ignore locked entries, packets with this DA will be flooded as unknown unicast in hardware. For mv88e6xxx you write "The mac address triggering the ATU miss violation will be added to the ATU with a zero-DPV". What does it mean? Traffic with this DA will be blackholed? It would be good to agree on one consistent behavior for all hardware implementations and the bridge driver. Do you know what happens in other implementations (e.g., Cisco)? FWIW, to me it makes sense to have the bridge ignore locked entries and let traffic be flooded. If user space does not want traffic to egress a locked port, then it can turn off flooding on the port while it is locked. > goto drop; > + } > } > > nbp_switchdev_frame_mark(p, skb); > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index 06e5f6faa431..47a3598d25c8 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -251,7 +251,8 @@ enum { > BR_FDB_ADDED_BY_EXT_LEARN, > BR_FDB_OFFLOADED, > BR_FDB_NOTIFY, > - BR_FDB_NOTIFY_INACTIVE > + BR_FDB_NOTIFY_INACTIVE, > + BR_FDB_ENTRY_LOCKED, > }; > > struct net_bridge_fdb_key { > -- > 2.30.2 >