[PATCH net 0/3] mlxsw: Various fixes

[PATCH net 0/3] mlxsw: Various fixes

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

This patchset contains various fixes for mlxsw.

Patch #1 fixes a race condition in a selftest. The race and fix are
explained in detail in the changelog.

Patch #2 re-adds a link mode that was wrongly removed, resulting in a
regression in some setups.

Patch #3 fixes a race condition in route installation with nexthop
objects.

Please consider patches #2 and #3 for stable.

Danielle Ratson (2):
  selftests: forwarding: Fix race condition in mirror installation
  mlxsw: spectrum_ethtool: Add an external speed to PTYS register

Ido Schimmel (1):
  mlxsw: spectrum_router: Ignore routes using a deleted nexthop object

 drivers/net/ethernet/mellanox/mlxsw/reg.h                | 1 +
 drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c   | 5 +++++
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c    | 7 +++++++
 drivers/net/ethernet/mellanox/mlxsw/switchx2.c           | 3 ++-
 .../net/forwarding/mirror_gre_bridge_1d_vlan.sh          | 9 +++++++++
 5 files changed, 24 insertions(+), 1 deletion(-)

-- 
2.29.2

[PATCH net 1/3] selftests: forwarding: Fix race condition in mirror installation

[Ido Schimmel]

From: Danielle Ratson <danieller@nvidia.com>

When mirroring to a gretap in hardware the device expects to be
programmed with the egress port and all the encapsulating headers. This
requires the driver to resolve the path the packet will take in the
software data path and program the device accordingly.

If the path cannot be resolved (in this case because of an unresolved
neighbor), then mirror installation fails until the path is resolved.
This results in a race that causes the test to sometimes fail.

Fix this by setting the neighbor's state to permanent, so that it is
always valid.

Fixes: b5b029399fa6d ("selftests: forwarding: mirror_gre_bridge_1d_vlan: Add STP test")
Signed-off-by: Danielle Ratson <danieller@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 .../net/forwarding/mirror_gre_bridge_1d_vlan.sh          | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1d_vlan.sh b/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1d_vlan.sh
index 197e769c2ed1..f8cda822c1ce 100755
--- a/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1d_vlan.sh
+++ b/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1d_vlan.sh
@@ -86,11 +86,20 @@ test_ip6gretap()
 
 test_gretap_stp()
 {
+	# Sometimes after mirror installation, the neighbor's state is not valid.
+	# The reason is that there is no SW datapath activity related to the
+	# neighbor for the remote GRE address. Therefore whether the corresponding
+	# neighbor will be valid is a matter of luck, and the test is thus racy.
+	# Set the neighbor's state to permanent, so it would be always valid.
+	ip neigh replace 192.0.2.130 lladdr $(mac_get $h3) \
+		nud permanent dev br2
 	full_test_span_gre_stp gt4 $swp3.555 "mirror to gretap"
 }
 
 test_ip6gretap_stp()
 {
+	ip neigh replace 2001:db8:2::2 lladdr $(mac_get $h3) \
+		nud permanent dev br2
 	full_test_span_gre_stp gt6 $swp3.555 "mirror to ip6gretap"
 }
 
-- 
2.29.2

[PATCH net 2/3] mlxsw: spectrum_ethtool: Add an external speed to PTYS register

[Ido Schimmel]

From: Danielle Ratson <danieller@nvidia.com>

Currently, only external bits are added to the PTYS register, whereas
there is one external bit that is wrongly marked as internal, and so was
recently removed from the register.

Add that bit to the PTYS register again, as this bit is no longer
internal.

Its removal resulted in '100000baseLR4_ER4/Full' link mode no longer
being supported, causing a regression on some setups.

Fixes: 5bf01b571cf4 ("mlxsw: spectrum_ethtool: Remove internal speeds from PTYS register")
Signed-off-by: Danielle Ratson <danieller@nvidia.com>
Reported-by: Eddie Shklaer <eddies@nvidia.com>
Tested-by: Eddie Shklaer <eddies@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/reg.h              | 1 +
 drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c | 5 +++++
 drivers/net/ethernet/mellanox/mlxsw/switchx2.c         | 3 ++-
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 16e2df6ef2f4..c4adc7f740d3 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -4430,6 +4430,7 @@ MLXSW_ITEM32(reg, ptys, ext_eth_proto_cap, 0x08, 0, 32);
 #define MLXSW_REG_PTYS_ETH_SPEED_100GBASE_CR4		BIT(20)
 #define MLXSW_REG_PTYS_ETH_SPEED_100GBASE_SR4		BIT(21)
 #define MLXSW_REG_PTYS_ETH_SPEED_100GBASE_KR4		BIT(22)
+#define MLXSW_REG_PTYS_ETH_SPEED_100GBASE_LR4_ER4	BIT(23)
 #define MLXSW_REG_PTYS_ETH_SPEED_25GBASE_CR		BIT(27)
 #define MLXSW_REG_PTYS_ETH_SPEED_25GBASE_KR		BIT(28)
 #define MLXSW_REG_PTYS_ETH_SPEED_25GBASE_SR		BIT(29)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
index bd7f873f6290..0bd64169bf81 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
@@ -1169,6 +1169,11 @@ static const struct mlxsw_sp1_port_link_mode mlxsw_sp1_port_link_mode[] = {
 		.mask_ethtool	= ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT,
 		.speed		= SPEED_100000,
 	},
+	{
+		.mask		= MLXSW_REG_PTYS_ETH_SPEED_100GBASE_LR4_ER4,
+		.mask_ethtool	= ETHTOOL_LINK_MODE_100000baseLR4_ER4_Full_BIT,
+		.speed		= SPEED_100000,
+	},
 };
 
 #define MLXSW_SP1_PORT_LINK_MODE_LEN ARRAY_SIZE(mlxsw_sp1_port_link_mode)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
index 40e2e79d4517..131b2a53d261 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
@@ -613,7 +613,8 @@ static const struct mlxsw_sx_port_link_mode mlxsw_sx_port_link_mode[] = {
 	{
 		.mask		= MLXSW_REG_PTYS_ETH_SPEED_100GBASE_CR4 |
 				  MLXSW_REG_PTYS_ETH_SPEED_100GBASE_SR4 |
-				  MLXSW_REG_PTYS_ETH_SPEED_100GBASE_KR4,
+				  MLXSW_REG_PTYS_ETH_SPEED_100GBASE_KR4 |
+				  MLXSW_REG_PTYS_ETH_SPEED_100GBASE_LR4_ER4,
 		.speed		= 100000,
 	},
 };
-- 
2.29.2

[PATCH net 3/3] mlxsw: spectrum_router: Ignore routes using a deleted nexthop object

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

Routes are currently processed from a workqueue whereas nexthop objects
are processed in system call context. This can result in the driver not
finding a suitable nexthop group for a route and issuing a warning [1].

Fix this by ignoring such routes earlier in the process. The subsequent
deletion notification will be ignored as well.

[1]
 WARNING: CPU: 2 PID: 7754 at drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c:4853 mlxsw_sp_router_fib_event_work+0x1112/0x1e00 [mlxsw_spectrum]
 [...]
 CPU: 2 PID: 7754 Comm: kworker/u8:0 Not tainted 5.11.0-rc6-cq-20210207-1 #16
 Hardware name: Mellanox Technologies Ltd. MSN2100/SA001390, BIOS 5.6.5 05/24/2018
 Workqueue: mlxsw_core_ordered mlxsw_sp_router_fib_event_work [mlxsw_spectrum]
 RIP: 0010:mlxsw_sp_router_fib_event_work+0x1112/0x1e00 [mlxsw_spectrum]

Fixes: cdd6cfc54c64 ("mlxsw: spectrum_router: Allow programming routes with nexthop objects")
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reported-by: Alex Veber <alexve@nvidia.com>
Tested-by: Alex Veber <alexve@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 9ce90841f92d..eda99d82766a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -5951,6 +5951,10 @@ mlxsw_sp_router_fib4_replace(struct mlxsw_sp *mlxsw_sp,
 	if (mlxsw_sp->router->aborted)
 		return 0;
 
+	if (fen_info->fi->nh &&
+	    !mlxsw_sp_nexthop_obj_group_lookup(mlxsw_sp, fen_info->fi->nh->id))
+		return 0;
+
 	fib_node = mlxsw_sp_fib_node_get(mlxsw_sp, fen_info->tb_id,
 					 &fen_info->dst, sizeof(fen_info->dst),
 					 fen_info->dst_len,
@@ -6601,6 +6605,9 @@ static int mlxsw_sp_router_fib6_replace(struct mlxsw_sp *mlxsw_sp,
 	if (mlxsw_sp_fib6_rt_should_ignore(rt))
 		return 0;
 
+	if (rt->nh && !mlxsw_sp_nexthop_obj_group_lookup(mlxsw_sp, rt->nh->id))
+		return 0;
+
 	fib_node = mlxsw_sp_fib_node_get(mlxsw_sp, rt->fib6_table->tb6_id,
 					 &rt->fib6_dst.addr,
 					 sizeof(rt->fib6_dst.addr),
-- 
2.29.2

Re: [PATCH net 0/3] mlxsw: Various fixes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (refs/heads/master):

On Thu, 25 Feb 2021 18:57:18 +0200 you wrote:
> From: Ido Schimmel <idosch@nvidia.com>
> 
> This patchset contains various fixes for mlxsw.
> 
> Patch #1 fixes a race condition in a selftest. The race and fix are
> explained in detail in the changelog.
> 
> [...]

Here is the summary with links:
  - [net,1/3] selftests: forwarding: Fix race condition in mirror installation
    https://git.kernel.org/netdev/net/c/edcbf5137f09
  - [net,2/3] mlxsw: spectrum_ethtool: Add an external speed to PTYS register
    https://git.kernel.org/netdev/net/c/ae9b24ddb69b
  - [net,3/3] mlxsw: spectrum_router: Ignore routes using a deleted nexthop object
    https://git.kernel.org/netdev/net/c/dc860b88ce0a

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

[PATCH net 0/3] mlxsw: Various fixes

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

This patch set contains various fixes for mlxsw.

Patch #1 ensures that only link modes that are supported by both the
device and the driver are advertised. When a link mode that is not
supported by the driver is negotiated by the device, it will be
presented as an unknown speed by ethtool, causing the bond driver to
wrongly assume that the link is down.

Patch #2 fixes a trivial memory leak upon module removal.

Patch #3 fixes a use-after-free that syzkaller was able to trigger once
on a slow emulator after a few months of fuzzing.

Amit Cohen (2):
  mlxsw: Only advertise link modes supported by both driver and device
  mlxsw: core: Fix use-after-free in mlxsw_emad_trans_finish()

Ido Schimmel (1):
  mlxsw: core: Fix memory leak on module removal

 drivers/net/ethernet/mellanox/mlxsw/core.c    |  5 ++++
 .../net/ethernet/mellanox/mlxsw/spectrum.c    |  9 ++++--
 .../net/ethernet/mellanox/mlxsw/spectrum.h    |  1 +
 .../mellanox/mlxsw/spectrum_ethtool.c         | 30 +++++++++++++++++++
 4 files changed, 43 insertions(+), 2 deletions(-)

-- 
2.26.2

Re: [PATCH net 0/3] mlxsw: Various fixes

[Jakub Kicinski]

On Sat, 24 Oct 2020 16:37:30 +0300 Ido Schimmel wrote:
> From: Ido Schimmel <idosch@nvidia.com>
> 
> This patch set contains various fixes for mlxsw.
> 
> Patch #1 ensures that only link modes that are supported by both the
> device and the driver are advertised. When a link mode that is not
> supported by the driver is negotiated by the device, it will be
> presented as an unknown speed by ethtool, causing the bond driver to
> wrongly assume that the link is down.
> 
> Patch #2 fixes a trivial memory leak upon module removal.
> 
> Patch #3 fixes a use-after-free that syzkaller was able to trigger once
> on a slow emulator after a few months of fuzzing.

Applied, queued #1 and #2, thanks!

[PATCH net 0/3] mlxsw: Various fixes

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

This patchset includes two small fixes for the mlxsw driver and one
patch which clarifies recently introduced devlink-trap documentation.

Patch #1 clears the port's VLAN filters during port initialization. This
ensures that the drop reason reported to the user is consistent. The
problem is explained in detail in the commit message.

Patch #2 clarifies the description of one of the traps exposed via
devlink-trap.

Patch #3 from Danielle forbids the installation of a tc filter with
multiple mirror actions since this is not supported by the device. The
failure is communicated to the user via extack.

Danielle Ratson (1):
  mlxsw: spectrum_flower: Fail in case user specifies multiple mirror
    actions

Ido Schimmel (2):
  mlxsw: spectrum: Clear VLAN filters during port initialization
  Documentation: Clarify trap's description

 Documentation/networking/devlink-trap.rst                | 3 ++-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c           | 9 +++++++++
 drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c    | 6 ++++++
 .../selftests/drivers/net/mlxsw/devlink_trap_l2_drops.sh | 7 -------
 4 files changed, 17 insertions(+), 8 deletions(-)

-- 
2.21.0

Re: [PATCH net 0/3] mlxsw: Various fixes

[David Miller]

From: Ido Schimmel <idosch@idosch.org>
Date: Thu, 26 Sep 2019 14:43:37 +0300

> From: Ido Schimmel <idosch@mellanox.com>
> 
> This patchset includes two small fixes for the mlxsw driver and one
> patch which clarifies recently introduced devlink-trap documentation.
> 
> Patch #1 clears the port's VLAN filters during port initialization. This
> ensures that the drop reason reported to the user is consistent. The
> problem is explained in detail in the commit message.
> 
> Patch #2 clarifies the description of one of the traps exposed via
> devlink-trap.
> 
> Patch #3 from Danielle forbids the installation of a tc filter with
> multiple mirror actions since this is not supported by the device. The
> failure is communicated to the user via extack.

Series applied.