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:   Thu, 13 Oct 2022 15:06:07 +0300
From:   Ido Schimmel <idosch@nvidia.com>
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>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        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>,
        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>,
        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>,
        Russell King <linux@armlinux.org.uk>,
        Christian Marangi <ansuelsmth@gmail.com>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Yuwei Wang <wangyuweihx@gmail.com>,
        Petr Machata <petrm@nvidia.com>,
        Florent Fourcot <florent.fourcot@wifirst.fr>,
        Hans Schultz <schultz.hans@gmail.com>,
        Joachim Wiberg <troglobit@gmail.com>,
        Amit Cohen <amcohen@nvidia.com>, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org,
        bridge@lists.linux-foundation.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v7 net-next 9/9] selftests: forwarding: add test of
 MAC-Auth Bypass to locked port tests
Message-ID: <Y0f/L9IT6dbwlZAg@shredder>
References: <20221009174052.1927483-1-netdev@kapio-technology.com>
 <20221009174052.1927483-10-netdev@kapio-technology.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221009174052.1927483-10-netdev@kapio-technology.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?hpyAmbVfoTa2xFauLcPZ2ByehIFSxrsU/HpSzrMzO8p/w0PfvCsVez05rPBK?=
 =?us-ascii?Q?SbhbioIfoGc+RHJvcGttIbAA5SpQ5oL3e1bv4cQvztcbEKypzRgebwaxGSZu?=
 =?us-ascii?Q?f1t7xMtTUcC+0ZVZBXVQ0QMPTV4vdDcMb663sFnlb79wab1lffinKqT05pz0?=
 =?us-ascii?Q?jkMWmEETsn2U/jrJe38aajEK4Xwmij39sjGsDO35HhA9yMDOovNM+qoU6/Ib?=
 =?us-ascii?Q?3gXXIfthrDoS2HZmWQpYEIWoz41MtopJrna578Cf6qX/QgljguJ6VcBbwSpv?=
 =?us-ascii?Q?+NDKg4TWY9F7MPsBNbQZbCpg6aXQV3y/b0lAimjQcCrw3sPwVO9YH7S++fMm?=
 =?us-ascii?Q?2qZIShsPmRd+437p/a5Hf/XsW5sgSFTFSE05IzL/hr4dm0bfHWzm9gsO8zjy?=
 =?us-ascii?Q?RW2LW724cPsFO47HzXKnTnZnX2Ws6WAKabMsMV7bEWCoAHm8IpWcng5igIMm?=
 =?us-ascii?Q?V6tkfe6UPJRiki39kACwXxecsQ/TxzetNI05oi0gIsd2kfGbnzAHDarMP0GH?=
 =?us-ascii?Q?ZUgccm/TTWg9Y/sebZ5XxAJYB5uS1VTIy0lNJqRU1ITCWkCGVjAsUyEBupdw?=
 =?us-ascii?Q?pzmOQY5uJFgIx04EGH7m5OG9V/MJPHtUudXd+5IM0n+Xlx5uzkR9YPLsFJet?=
 =?us-ascii?Q?hgqYrTS1+rO6CUBY2D2nRvZ/hG1ImclyyA+9WY1sheWEkx1B/am4THPSSUL+?=
 =?us-ascii?Q?3RdZsyRngZsrZcTlBfc6FogAr/z/bai7arVCV0r6sc6OWOoiHDaqTwulpdZF?=
 =?us-ascii?Q?DfunxhsEuFQknG4h3TodjnX3gFbfpxgwYRTrCCRpvhE8Fuv/Tdln2feFYdZ+?=
 =?us-ascii?Q?8PelYYKxT/nZgHFhbGe8IOABopkCBvw3YkNRA7Y5AYjbFGl/a5IUqhnAD6H2?=
 =?us-ascii?Q?qaNhHpKYtju6zjssNOWUK57Uv5ujXzq722g8qheXrAe4UvlQnbHxJa0VQIC+?=
 =?us-ascii?Q?QPwS8rCmlj8+JFTBxZ1Rk6INbqVtBzydnkblBvWgGycQFPWJNZO7LM19lyWx?=
 =?us-ascii?Q?6C9mDSHbZiA5Qaw5DpYl7uVwgKGfJTJENa0NBkpdckAdgQF1FEDpk08VRKTT?=
 =?us-ascii?Q?Lc9TqvN8EvSMk+yDFhCErwd3vFm7B69OUEVmquvVEo/xf25UNtrVTQ4fytl4?=
 =?us-ascii?Q?7SIABOxfEInTfWnTachoqa3YTvhgs1t256EEgf0VdbqyDqj0uwE+CgmIY8dD?=
 =?us-ascii?Q?SX/4mCDZoZcXMoON+iopPgekYoUsFd+uqf/Xs5BBtLNW1VxBzPJ4hNY5ObWj?=
 =?us-ascii?Q?R9pAfL8DftuAnNfs4VhJw3IDthBWPZ4mkYOL/0jDXgRPuPE1dZbeXnHd/f4s?=
 =?us-ascii?Q?eVY9tsZQoXkG0bO2VyH5lOLpGV0BBT1aUWB3O6A9PfYs2zuIfbkXFuNd2OXc?=
 =?us-ascii?Q?dMBZ3G/J2arVHb/MSQmXpcP3mUOQAZgHRHxrpcO483+JPNZOpVCYCqj4XBSE?=
 =?us-ascii?Q?BrTCVMhVxWytTzAhRxdx4EeirVTrRTBcJA6fT/VcE32fgm9Hi3NT2noUsDaT?=
 =?us-ascii?Q?QBggeYMySSUAOpza5j1ZIFKCPtudj0G2jqbpEzAvasf/Azm1PFJ8uTZStHsZ?=
 =?us-ascii?Q?au6W7JRCpIH99uF1j1ae+0Xw+0bO78OLL5Qyd24Q?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: cf2d2518-ae5c-448d-5468-08daad135311
X-MS-Exchange-CrossTenant-AuthSource: CY5PR12MB6179.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Oct 2022 12:06:13.9411
 (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: 7izZQMBKP3FaSQShSjJcSAiRWqY0UzlTnoPSF8mm7xLnzp4xmBHbpKlvrbCHKdDXri84xnF01bbX0vdbvrlpIw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO6PR12MB5441
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
        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 Sun, Oct 09, 2022 at 07:40:52PM +0200, Hans J. Schultz wrote:
> +locked_port_mab()
> +{
> +	RET=0
> +	check_port_mab_support || return 0
> +
> +	ping_do $h1 192.0.2.2
> +	check_err $? "MAB: Ping did not work before locking port"
> +
> +	bridge link set dev $swp1 locked on mab on
> +
> +	ping_do $h1 192.0.2.2
> +	check_fail $? "MAB: Ping worked on mab enabled port without FDB entry"
> +
> +	bridge fdb get `mac_get $h1` vlan 1 dev $swp1 | grep "dev $swp1 vlan 1" | grep -q "locked"
> +	check_err $? "MAB: No locked FDB entry after ping on mab enabled port"
> +
> +	bridge fdb replace `mac_get $h1` dev $swp1 master static
> +
> +	ping_do $h1 192.0.2.2
> +	check_err $? "MAB: Ping did not work with FDB entry without locked flag"
> +
> +	bridge fdb del `mac_get $h1` dev $swp1 master
> +	bridge link set dev $swp1 locked off mab off
> +
> +	log_test "Locked port MAB"
> +}
> +
> +# Check that entries cannot roam from an unlocked port to a locked port.
> +locked_port_station_move()
> +{
> +	local mac=a0:b0:c0:c0:b0:a0
> +
> +	RET=0
> +	check_locked_port_support || return 0
> +
> +	bridge link set dev $swp1 locked on learning on
> +
> +	$MZ $h1 -q -c 5 -d 100msec -t udp -a $mac -b rand
> +	bridge fdb show dev $swp1 | grep -q $mac
> +	check_fail $? "Locked port station move: FDB entry on first injection"
> +
> +	$MZ $h2 -q -c 5 -d 100msec -t udp -a $mac -b rand
> +	bridge fdb get $mac vlan 1 dev $swp2 | grep "dev $swp2 vlan 1" | grep -q "master br0"
> +	check_err $? "Locked port station move: Entry not found on unlocked port"
> +
> +	$MZ $h1 -q -c 5 -d 100msec -t udp -a $mac -b rand
> +	bridge fdb get $mac vlan 1 dev $swp1 | grep "dev $swp1 vlan 1" | grep -q "master br0"
> +	check_fail $? "Locked port station move: entry roamed to locked port"
> +
> +	bridge fdb del $mac vlan 1 dev $swp2 master
> +	bridge link set dev $swp1 locked off learning off
> +
> +	log_test "Locked port station move"
> +}
> +
> +# Roaming to and from a MAB enabled port should work if blackhole flag is not set
> +locked_port_mab_station_move()
> +{
> +	local mac=10:20:30:30:20:10
> +
> +	RET=0
> +	check_port_mab_support || return 0
> +
> +	bridge link set dev $swp1 locked on mab on
> +
> +	$MZ $h1 -q -c 5 -d 100 mesc -t udp -a $mac -b rand
> +	if bridge fdb show dev $swp1 | grep "$mac vlan 1" | grep -q "blackhole"; then
> +		echo "SKIP: Roaming not possible with blackhole flag, skipping test..."
> +		bridge link set dev $swp1 locked off mab off
> +		return $ksft_skip
> +	fi
> +
> +	bridge fdb show dev $swp1 | grep "$mac vlan 1" | grep -q "locked"
> +	check_err $? "MAB station move: no locked entry on first injection"
> +
> +	$MZ $h2 -q -c 5 -d 100msec -t udp -a $mac -b rand
> +	bridge fdb get $mac vlan 1 dev $swp1 | grep "dev $swp1 vlan 1" | grep -q "locked"
> +	check_fail $? "MAB station move: locked entry did not move"
> +
> +	bridge fdb get $mac vlan 1 dev $swp2 | grep "dev $swp2 vlan 1" | grep -q "locked"
> +	check_fail $? "MAB station move: roamed entry to unlocked port had locked flag on"
> +
> +	bridge fdb get $mac vlan 1 dev $swp2 | grep "dev $swp2 vlan 1" | grep -q "master br0"
> +	check_err $? "MAB station move: roamed entry not found"
> +
> +	bridge fdb del $mac vlan 1 dev $swp2 master
> +	bridge link set dev $swp1 locked off mab off
> +
> +	log_test "Locked port MAB station move"
> +}

Looks OK to me. I made some change to make sure we are using "bridge fdb
get" in a consistent manner instead of relying on iproute2 dump output
too much. Please consider including them in the next version.

FYI, I ran your version and mine with veth pairs and both are OK.

diff --git a/tools/testing/selftests/net/forwarding/bridge_locked_port.sh b/tools/testing/selftests/net/forwarding/bridge_locked_port.sh
index fbe558f25e44..f0bc0bcbc246 100755
--- a/tools/testing/selftests/net/forwarding/bridge_locked_port.sh
+++ b/tools/testing/selftests/net/forwarding/bridge_locked_port.sh
@@ -187,7 +187,7 @@ locked_port_mab()
 	ping_do $h1 192.0.2.2
 	check_fail $? "MAB: Ping worked on mab enabled port without FDB entry"
 
-	bridge fdb get `mac_get $h1` vlan 1 dev $swp1 | grep "dev $swp1 vlan 1" | grep -q "locked"
+	bridge fdb get `mac_get $h1` br br0 vlan 1 | grep "dev $swp1" | grep -q "locked"
 	check_err $? "MAB: No locked FDB entry after ping on mab enabled port"
 
 	bridge fdb replace `mac_get $h1` dev $swp1 master static
@@ -212,15 +212,15 @@ locked_port_station_move()
 	bridge link set dev $swp1 locked on learning on
 
 	$MZ $h1 -q -c 5 -d 100msec -t udp -a $mac -b rand
-	bridge fdb show dev $swp1 | grep -q $mac
+	bridge fdb get $mac br br0 vlan 1 &> /dev/null
 	check_fail $? "Locked port station move: FDB entry on first injection"
 
 	$MZ $h2 -q -c 5 -d 100msec -t udp -a $mac -b rand
-	bridge fdb get $mac vlan 1 dev $swp2 | grep "dev $swp2 vlan 1" | grep -q "master br0"
+	bridge fdb get $mac br br0 vlan 1 | grep -q "dev $swp2"
 	check_err $? "Locked port station move: Entry not found on unlocked port"
 
 	$MZ $h1 -q -c 5 -d 100msec -t udp -a $mac -b rand
-	bridge fdb get $mac vlan 1 dev $swp1 | grep "dev $swp1 vlan 1" | grep -q "master br0"
+	bridge fdb get $mac br br0 vlan 1 | grep -q "dev $swp1"
 	check_fail $? "Locked port station move: entry roamed to locked port"
 
 	bridge fdb del $mac vlan 1 dev $swp2 master
@@ -229,7 +229,8 @@ locked_port_station_move()
 	log_test "Locked port station move"
 }
 
-# Roaming to and from a MAB enabled port should work if blackhole flag is not set
+# Check that entries can roam from a locked port if blackhole FDB flag is not
+# set.
 locked_port_mab_station_move()
 {
 	local mac=10:20:30:30:20:10
@@ -246,19 +247,16 @@ locked_port_mab_station_move()
 		return $ksft_skip
 	fi
 
-	bridge fdb show dev $swp1 | grep "$mac vlan 1" | grep -q "locked"
+	bridge fdb get $mac br br0 vlan 1 | grep "dev $swp1" | grep -q "locked"
 	check_err $? "MAB station move: no locked entry on first injection"
 
 	$MZ $h2 -q -c 5 -d 100msec -t udp -a $mac -b rand
-	bridge fdb get $mac vlan 1 dev $swp1 | grep "dev $swp1 vlan 1" | grep -q "locked"
-	check_fail $? "MAB station move: locked entry did not move"
+	bridge fdb get $mac br br0 vlan 1 | grep -q "dev $swp2"
+	check_err $? "MAB station move: roamed entry not found"
 
-	bridge fdb get $mac vlan 1 dev $swp2 | grep "dev $swp2 vlan 1" | grep -q "locked"
+	bridge fdb get $mac br br0 vlan 1 | grep -q "locked"
 	check_fail $? "MAB station move: roamed entry to unlocked port had locked flag on"
 
-	bridge fdb get $mac vlan 1 dev $swp2 | grep "dev $swp2 vlan 1" | grep -q "master br0"
-	check_err $? "MAB station move: roamed entry not found"
-
 	bridge fdb del $mac vlan 1 dev $swp2 master
 	bridge link set dev $swp1 locked off mab off