From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 71FCE6FAF8 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org C28756FAFC 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=FAEjyQCuY5bv3ffrG3YJFzZzlolttdTNtL7f6jneouA=; b=kTmNmQcXRg9Z70rVq6z+siZcJlBPlB6meCadLVdm4IdtNomyMaz+rnMIbqMSdgShXrScXG/wETXejfD9D+v4N4lvENZgOHvttOIfxHQVSSn//0zfus1Brf5dVUyIyCYNVYCbTLGcHO6u27LI9KNzOIe3jrbKmsbmaCWPJacNjp5ob7lcbNiTAyaJS2vpbAGt9u7m9AqtrRmV2lp2S99zoApgHn3IsbRgomkK5qLe1Snh2nbd60kqm7R6nIBQnGgAwIR5ucuVkF9MCOUfLzYnBhPc0KQm2DVGYtLOw66PSW4+en3p78Ey7erTaGCDpwZElLxkDK+XnGp6fIBjfJIggQ== Date: Thu, 20 Oct 2022 15:54:01 +0300 From: Ido Schimmel Message-ID: References: <20221018165619.134535-1-netdev@kapio-technology.com> <20221018165619.134535-2-netdev@kapio-technology.com> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221018165619.134535-2-netdev@kapio-technology.com> MIME-Version: 1.0 Subject: Re: [Bridge] [PATCH v8 net-next 01/12] 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 J. Schultz" Cc: Andrew Lunn , Alexandre Belloni , Nikolay Aleksandrov , Kurt Kanzenbach , Eric Dumazet , linux-kselftest@vger.kernel.org, Joachim Wiberg , Shuah Khan , Ivan Vecera , Florian Fainelli , Daniel Borkmann , Florent Fourcot , bridge@lists.linux-foundation.org, Russell King , linux-arm-kernel@lists.infradead.org, Roopa Prabhu , kuba@kernel.org, Paolo Abeni , Vivien Didelot , Woojung Huh , Landen Chao , Jiri Pirko , Amit Cohen , Christian Marangi , Hauke Mehrtens , Hans Schultz , Sean Wang , DENG Qingfang , Claudiu Manoil , linux-mediatek@lists.infradead.org, Matthias Brugger , Yuwei Wang , Petr Machata , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, UNGLinuxDriver@microchip.com, Vladimir Oltean , davem@davemloft.net On Tue, Oct 18, 2022 at 06:56:08PM +0200, Hans J. 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 The entry itself is not denying the access through the port, but rather the fact that the port is locked and there is no matching FDB entry. > 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. Worth mentioning that the feature is enabled via the 'mab' bridge port option (BR_PORT_MAB). > > 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. > > Locked entries will age out with the set bridge ageing time. > > Signed-off-by: Hans J. Schultz Overall looks OK to me. See one comment below. Reviewed-by: Ido Schimmel [...] > @@ -1178,6 +1192,14 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[], > vg = nbp_vlan_group(p); > } > > + if (tb[NDA_FLAGS_EXT]) > + ext_flags = nla_get_u32(tb[NDA_FLAGS_EXT]); > + > + if (ext_flags & NTF_EXT_LOCKED) { > + pr_info("bridge: RTM_NEWNEIGH has invalid extended flags\n"); I understand this function makes use of pr_info(), but it already gets extack and it's a matter of time until the pr_info() instances will be converted to extack. I would just use extack here like you are doing in the next patch. Also, I find this message more helpful: "Cannot add FDB entry with \"locked\" flag set" > + return -EINVAL; > + } > + > if (tb[NDA_FDB_EXT_ATTRS]) { > attr = tb[NDA_FDB_EXT_ATTRS]; > err = nla_parse_nested(nfea_tb, NFEA_MAX, attr,