[PATCH net-next 00/14] mlxsw: Various trap changes - part 2

[PATCH net-next 00/14] mlxsw: Various trap changes - part 2

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

This patch set contains another set of small changes in mlxsw trap
configuration. It is the last set before exposing control traps (e.g.,
IGMP query, ARP request) via devlink-trap.

Tested with existing devlink-trap selftests. Please see individual
patches for a detailed changelog.

Ido Schimmel (14):
  mlxsw: spectrum: Use dedicated trap group for ACL trap
  mlxsw: spectrum: Use same switch case for identical groups
  mlxsw: spectrum: Rename IPv6 ND trap group
  mlxsw: spectrum: Use same trap group for various IPv6 packets
  mlxsw: spectrum: Use separate trap group for FID miss
  mlxsw: spectrum: Use same trap group for local routes and link-local
    destination
  mlxsw: spectrum: Reduce priority of locally delivered packets
  mlxsw: switchx2: Move SwitchX-2 trap groups out of main enum
  mlxsw: spectrum_trap: Do not hard code "thin" policer identifier
  mlxsw: reg: Move all trap groups under the same enum
  mlxsw: spectrum: Share one group for all locally delivered packets
  mlxsw: spectrum: Treat IPv6 link-local SIP as an exception
  mlxsw: spectrum: Add packet traps for BFD packets
  mlxsw: spectrum_router: Allow programming link-local prefix routes

 drivers/net/ethernet/mellanox/mlxsw/reg.h     | 17 ++++----
 .../net/ethernet/mellanox/mlxsw/spectrum.c    | 40 +++++++++++--------
 .../ethernet/mellanox/mlxsw/spectrum_router.c |  6 ++-
 .../ethernet/mellanox/mlxsw/spectrum_trap.c   | 17 +++++---
 .../ethernet/mellanox/mlxsw/spectrum_trap.h   |  2 +
 .../net/ethernet/mellanox/mlxsw/switchx2.c    |  5 +++
 drivers/net/ethernet/mellanox/mlxsw/trap.h    |  2 +
 .../drivers/net/mlxsw/sharedbuffer.sh         |  2 +-
 8 files changed, 56 insertions(+), 35 deletions(-)

-- 
2.26.2

[PATCH net-next 01/14] mlxsw: spectrum: Use dedicated trap group for ACL trap

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

Packets that are trapped via tc's trap action are currently subject to
the same policer as packets hitting local routes. The latter are
critical to the correct functioning of the control plane, while the
former are mainly used for traffic inspection.

Split the ACL trap to a separate group with its own policer. Use a
higher priority for these traps than for traps using mirror action
(e.g., ARP, IGMP). Otherwise, packets matching both traps will not be
forwarded in hardware (because of trap action) and also not forwarded in
software because they will be marked with 'offload_fwd_mark'.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/reg.h      | 1 +
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 9b27a129b0a6..5b5b96a82a34 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -5548,6 +5548,7 @@ enum mlxsw_reg_htgt_trap_group {
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_PTP1,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_VRRP,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_PKT_SAMPLE,
+	MLXSW_REG_HTGT_TRAP_GROUP_SP_FLOW_LOGGING,
 
 	__MLXSW_REG_HTGT_TRAP_GROUP_MAX,
 	MLXSW_REG_HTGT_TRAP_GROUP_MAX = __MLXSW_REG_HTGT_TRAP_GROUP_MAX - 1
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 943a24975799..e0811a7e13b9 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4105,7 +4105,7 @@ static const struct mlxsw_listener mlxsw_sp_listener[] = {
 	MLXSW_RXL(mlxsw_sp_rx_listener_sample_func, PKT_SAMPLE, MIRROR_TO_CPU,
 		  false, SP_PKT_SAMPLE, DISCARD),
 	/* ACL trap */
-	MLXSW_SP_RXL_NO_MARK(ACL0, TRAP_TO_CPU, IP2ME, false),
+	MLXSW_SP_RXL_NO_MARK(ACL0, TRAP_TO_CPU, FLOW_LOGGING, false),
 	/* Multicast Router Traps */
 	MLXSW_SP_RXL_MARK(IPV4_PIM, TRAP_TO_CPU, PIM, false),
 	MLXSW_SP_RXL_MARK(IPV6_PIM, TRAP_TO_CPU, PIM, false),
@@ -4167,6 +4167,7 @@ static int mlxsw_sp_cpu_policers_set(struct mlxsw_core *mlxsw_core)
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_REMOTE_ROUTE:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_IPV6_ND:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_MULTICAST:
+		case MLXSW_REG_HTGT_TRAP_GROUP_SP_FLOW_LOGGING:
 			rate = 1024;
 			burst_size = 7;
 			break;
@@ -4230,6 +4231,7 @@ static int mlxsw_sp_trap_groups_set(struct mlxsw_core *mlxsw_core)
 			priority = 5;
 			tc = 5;
 			break;
+		case MLXSW_REG_HTGT_TRAP_GROUP_SP_FLOW_LOGGING:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_BGP:
 			priority = 4;
 			tc = 4;
-- 
2.26.2

[PATCH net-next 02/14] mlxsw: spectrum: Use same switch case for identical groups

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

Trap groups that use the same policer settings can share the same switch
case.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index e0811a7e13b9..e8c9fc4cb6fb 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4168,9 +4168,6 @@ static int mlxsw_sp_cpu_policers_set(struct mlxsw_core *mlxsw_core)
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_IPV6_ND:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_MULTICAST:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_FLOW_LOGGING:
-			rate = 1024;
-			burst_size = 7;
-			break;
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_IP2ME:
 			rate = 1024;
 			burst_size = 7;
-- 
2.26.2

[PATCH net-next 03/14] mlxsw: spectrum: Rename IPv6 ND trap group

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

The IPv6 Neighbour Discovery (ND) group will be used for various IPv6
packets, not all of which fall under the definition of ND, so rename it
to "IPV6" which is more appropriate.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/reg.h      |  2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 5b5b96a82a34..6df7cc8a69f1 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -5542,7 +5542,7 @@ enum mlxsw_reg_htgt_trap_group {
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_IP2ME,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_DHCP,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_EVENT,
-	MLXSW_REG_HTGT_TRAP_GROUP_SP_IPV6_ND,
+	MLXSW_REG_HTGT_TRAP_GROUP_SP_IPV6,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_LBERROR,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_PTP0,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_PTP1,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index e8c9fc4cb6fb..b420aa292d7c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4078,15 +4078,15 @@ static const struct mlxsw_listener mlxsw_sp_listener[] = {
 	MLXSW_SP_RXL_MARK(RTR_INGRESS0, TRAP_TO_CPU, REMOTE_ROUTE, false),
 	MLXSW_SP_RXL_MARK(IPV4_BGP, TRAP_TO_CPU, BGP, false),
 	MLXSW_SP_RXL_MARK(IPV6_BGP, TRAP_TO_CPU, BGP, false),
-	MLXSW_SP_RXL_MARK(L3_IPV6_ROUTER_SOLICITATION, TRAP_TO_CPU, IPV6_ND,
+	MLXSW_SP_RXL_MARK(L3_IPV6_ROUTER_SOLICITATION, TRAP_TO_CPU, IPV6,
 			  false),
-	MLXSW_SP_RXL_MARK(L3_IPV6_ROUTER_ADVERTISEMENT, TRAP_TO_CPU, IPV6_ND,
+	MLXSW_SP_RXL_MARK(L3_IPV6_ROUTER_ADVERTISEMENT, TRAP_TO_CPU, IPV6,
 			  false),
 	MLXSW_SP_RXL_MARK(L3_IPV6_NEIGHBOR_SOLICITATION, TRAP_TO_CPU,
 			  NEIGH_DISCOVERY, false),
 	MLXSW_SP_RXL_MARK(L3_IPV6_NEIGHBOR_ADVERTISEMENT, TRAP_TO_CPU,
 			  NEIGH_DISCOVERY, false),
-	MLXSW_SP_RXL_MARK(L3_IPV6_REDIRECTION, TRAP_TO_CPU, IPV6_ND, false),
+	MLXSW_SP_RXL_MARK(L3_IPV6_REDIRECTION, TRAP_TO_CPU, IPV6, false),
 	MLXSW_SP_RXL_MARK(IPV6_MC_LINK_LOCAL_DEST, TRAP_TO_CPU, ROUTER_EXP,
 			  false),
 	MLXSW_SP_RXL_MARK(ROUTER_ALERT_IPV4, TRAP_TO_CPU, ROUTER_EXP, false),
@@ -4165,7 +4165,7 @@ static int mlxsw_sp_cpu_policers_set(struct mlxsw_core *mlxsw_core)
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_NEIGH_DISCOVERY:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_ROUTER_EXP:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_REMOTE_ROUTE:
-		case MLXSW_REG_HTGT_TRAP_GROUP_SP_IPV6_ND:
+		case MLXSW_REG_HTGT_TRAP_GROUP_SP_IPV6:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_MULTICAST:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_FLOW_LOGGING:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_IP2ME:
@@ -4239,7 +4239,7 @@ static int mlxsw_sp_trap_groups_set(struct mlxsw_core *mlxsw_core)
 			tc = 3;
 			break;
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_NEIGH_DISCOVERY:
-		case MLXSW_REG_HTGT_TRAP_GROUP_SP_IPV6_ND:
+		case MLXSW_REG_HTGT_TRAP_GROUP_SP_IPV6:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_PTP1:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_DHCP:
 			priority = 2;
-- 
2.26.2

[PATCH net-next 04/14] mlxsw: spectrum: Use same trap group for various IPv6 packets

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

Group these various IPv6 packets (e.g., router solicitations, router
advertisement) together and subject them to the same policer.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index b420aa292d7c..141a605582c6 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4067,9 +4067,9 @@ static const struct mlxsw_listener mlxsw_sp_listener[] = {
 	MLXSW_SP_RXL_MARK(IPV6_UNSPECIFIED_ADDRESS, TRAP_TO_CPU, ROUTER_EXP,
 			  false),
 	MLXSW_SP_RXL_MARK(IPV6_LINK_LOCAL_DEST, TRAP_TO_CPU, ROUTER_EXP, false),
-	MLXSW_SP_RXL_MARK(IPV6_LINK_LOCAL_SRC, TRAP_TO_CPU, ROUTER_EXP, false),
-	MLXSW_SP_RXL_MARK(IPV6_ALL_NODES_LINK, TRAP_TO_CPU, ROUTER_EXP, false),
-	MLXSW_SP_RXL_MARK(IPV6_ALL_ROUTERS_LINK, TRAP_TO_CPU, ROUTER_EXP,
+	MLXSW_SP_RXL_MARK(IPV6_LINK_LOCAL_SRC, TRAP_TO_CPU, IPV6, false),
+	MLXSW_SP_RXL_MARK(IPV6_ALL_NODES_LINK, TRAP_TO_CPU, IPV6, false),
+	MLXSW_SP_RXL_MARK(IPV6_ALL_ROUTERS_LINK, TRAP_TO_CPU, IPV6,
 			  false),
 	MLXSW_SP_RXL_MARK(IPV4_OSPF, TRAP_TO_CPU, OSPF, false),
 	MLXSW_SP_RXL_MARK(IPV6_OSPF, TRAP_TO_CPU, OSPF, false),
-- 
2.26.2

[PATCH net-next 05/14] mlxsw: spectrum: Use separate trap group for FID miss

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

When a packet enters the device it is classified to a filtering
identifier (FID) based on the ingress port and VLAN. The FID miss trap
is used to trap packets for which a FID could not be found.

In mlxsw this trap should only be triggered when a port is enslaved to
an OVS bridge and a matching ACL rule could not be found, so as to
trigger learning.

These packets are therefore completely unrelated to packets hitting
local routes and should be in a different group. Move them.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/reg.h      | 1 +
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 6df7cc8a69f1..b55a833a5d17 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -5549,6 +5549,7 @@ enum mlxsw_reg_htgt_trap_group {
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_VRRP,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_PKT_SAMPLE,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_FLOW_LOGGING,
+	MLXSW_REG_HTGT_TRAP_GROUP_SP_FID_MISS,
 
 	__MLXSW_REG_HTGT_TRAP_GROUP_MAX,
 	MLXSW_REG_HTGT_TRAP_GROUP_MAX = __MLXSW_REG_HTGT_TRAP_GROUP_MAX - 1
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 141a605582c6..ac71d67457aa 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4052,7 +4052,7 @@ static const struct mlxsw_listener mlxsw_sp_listener[] = {
 	MLXSW_SP_RXL_NO_MARK(IGMP_V3_REPORT, TRAP_TO_CPU, MC_SNOOPING, false),
 	MLXSW_SP_RXL_MARK(ARPBC, MIRROR_TO_CPU, NEIGH_DISCOVERY, false),
 	MLXSW_SP_RXL_MARK(ARPUC, MIRROR_TO_CPU, NEIGH_DISCOVERY, false),
-	MLXSW_SP_RXL_NO_MARK(FID_MISS, TRAP_TO_CPU, IP2ME, false),
+	MLXSW_SP_RXL_NO_MARK(FID_MISS, TRAP_TO_CPU, FID_MISS, false),
 	MLXSW_SP_RXL_MARK(IPV6_MLDV12_LISTENER_QUERY, MIRROR_TO_CPU,
 			  MC_SNOOPING, false),
 	MLXSW_SP_RXL_NO_MARK(IPV6_MLDV1_LISTENER_REPORT, TRAP_TO_CPU,
@@ -4169,6 +4169,7 @@ static int mlxsw_sp_cpu_policers_set(struct mlxsw_core *mlxsw_core)
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_MULTICAST:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_FLOW_LOGGING:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_IP2ME:
+		case MLXSW_REG_HTGT_TRAP_GROUP_SP_FID_MISS:
 			rate = 1024;
 			burst_size = 7;
 			break;
@@ -4248,6 +4249,7 @@ static int mlxsw_sp_trap_groups_set(struct mlxsw_core *mlxsw_core)
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_ROUTER_EXP:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_REMOTE_ROUTE:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_MULTICAST:
+		case MLXSW_REG_HTGT_TRAP_GROUP_SP_FID_MISS:
 			priority = 1;
 			tc = 1;
 			break;
-- 
2.26.2

[PATCH net-next 06/14] mlxsw: spectrum: Use same trap group for local routes and link-local destination

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

Packets with an IPv6 link-local destination (i.e., fe80::/10) should not
be forwarded and are therefore trapped to the CPU for local delivery.
Since these packets are trapped for the same logical reason as packets
hitting local routes, associate both traps with the same group.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index ac71d67457aa..5fe51ee8a206 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4066,7 +4066,7 @@ static const struct mlxsw_listener mlxsw_sp_listener[] = {
 	MLXSW_SP_RXL_MARK(IP2ME, TRAP_TO_CPU, IP2ME, false),
 	MLXSW_SP_RXL_MARK(IPV6_UNSPECIFIED_ADDRESS, TRAP_TO_CPU, ROUTER_EXP,
 			  false),
-	MLXSW_SP_RXL_MARK(IPV6_LINK_LOCAL_DEST, TRAP_TO_CPU, ROUTER_EXP, false),
+	MLXSW_SP_RXL_MARK(IPV6_LINK_LOCAL_DEST, TRAP_TO_CPU, IP2ME, false),
 	MLXSW_SP_RXL_MARK(IPV6_LINK_LOCAL_SRC, TRAP_TO_CPU, IPV6, false),
 	MLXSW_SP_RXL_MARK(IPV6_ALL_NODES_LINK, TRAP_TO_CPU, IPV6, false),
 	MLXSW_SP_RXL_MARK(IPV6_ALL_ROUTERS_LINK, TRAP_TO_CPU, IPV6,
-- 
2.26.2

[PATCH net-next 07/14] mlxsw: spectrum: Reduce priority of locally delivered packets

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

To align with recent recommended values. Will be configurable by future
patches.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c            | 2 +-
 tools/testing/selftests/drivers/net/mlxsw/sharedbuffer.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 5fe51ee8a206..b10e5aeaedef 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4235,11 +4235,11 @@ static int mlxsw_sp_trap_groups_set(struct mlxsw_core *mlxsw_core)
 			tc = 4;
 			break;
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_MC_SNOOPING:
-		case MLXSW_REG_HTGT_TRAP_GROUP_SP_IP2ME:
 			priority = 3;
 			tc = 3;
 			break;
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_NEIGH_DISCOVERY:
+		case MLXSW_REG_HTGT_TRAP_GROUP_SP_IP2ME:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_IPV6:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_PTP1:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_DHCP:
diff --git a/tools/testing/selftests/drivers/net/mlxsw/sharedbuffer.sh b/tools/testing/selftests/drivers/net/mlxsw/sharedbuffer.sh
index 58f3a05f08af..7d9e73a43a49 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/sharedbuffer.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/sharedbuffer.sh
@@ -15,7 +15,7 @@ source mlxsw_lib.sh
 SB_POOL_ING=0
 SB_POOL_EGR_CPU=10
 
-SB_ITC_CPU_IP=3
+SB_ITC_CPU_IP=2
 SB_ITC_CPU_ARP=2
 SB_ITC=0
 
-- 
2.26.2

[PATCH net-next 08/14] mlxsw: switchx2: Move SwitchX-2 trap groups out of main enum

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

The number of Spectrum trap groups is not infinite, but two identifiers
are occupied by SwitchX-2 specific trap groups. Free these identifiers
by moving them out of the main enum.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/reg.h      | 2 --
 drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 5 +++++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index b55a833a5d17..fd5e18b71114 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -5526,8 +5526,6 @@ MLXSW_ITEM32(reg, htgt, type, 0x00, 8, 4);
 
 enum mlxsw_reg_htgt_trap_group {
 	MLXSW_REG_HTGT_TRAP_GROUP_EMAD,
-	MLXSW_REG_HTGT_TRAP_GROUP_SX2_RX,
-	MLXSW_REG_HTGT_TRAP_GROUP_SX2_CTRL,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_STP,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_LACP,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_LLDP,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
index 2503f61db5fb..b438f5576e18 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
@@ -1404,6 +1404,11 @@ static int mlxsw_sx_port_type_set(struct mlxsw_core *mlxsw_core, u8 local_port,
 	return err;
 }
 
+enum {
+	MLXSW_REG_HTGT_TRAP_GROUP_SX2_RX = 1,
+	MLXSW_REG_HTGT_TRAP_GROUP_SX2_CTRL = 2,
+};
+
 #define MLXSW_SX_RXL(_trap_id) \
 	MLXSW_RXL(mlxsw_sx_rx_listener_func, _trap_id, TRAP_TO_CPU,	\
 		  false, SX2_RX, FORWARD)
-- 
2.26.2

[PATCH net-next 09/14] mlxsw: spectrum_trap: Do not hard code "thin" policer identifier

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

As explained in commit e612523041ab ("mlxsw: spectrum_trap: Introduce
dummy group with thin policer"), the purpose of the "thin" policer is to
pass as less packets as possible to the CPU.

The identifier of this policer is currently set according to the maximum
number of used trap groups, but this is fragile: On Spectrum-1 the
maximum number of policers is less than the maximum number of trap
groups, which might result in an invalid policer identifier in case the
number of used trap groups grows beyond the policer limit.

Solve this by dynamically allocating the policer identifier.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_trap.c | 17 +++++++++++------
 .../net/ethernet/mellanox/mlxsw/spectrum_trap.h |  2 ++
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
index 78f983c1a056..f4b812276a5a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
@@ -441,8 +441,6 @@ static const struct mlxsw_sp_trap_item mlxsw_sp_trap_items_arr[] = {
 	},
 };
 
-#define MLXSW_SP_THIN_POLICER_ID	(MLXSW_REG_HTGT_TRAP_GROUP_MAX + 1)
-
 static struct mlxsw_sp_trap_policer_item *
 mlxsw_sp_trap_policer_item_lookup(struct mlxsw_sp *mlxsw_sp, u32 id)
 {
@@ -487,14 +485,21 @@ mlxsw_sp_trap_item_lookup(struct mlxsw_sp *mlxsw_sp, u16 id)
 
 static int mlxsw_sp_trap_cpu_policers_set(struct mlxsw_sp *mlxsw_sp)
 {
+	struct mlxsw_sp_trap *trap = mlxsw_sp->trap;
 	char qpcr_pl[MLXSW_REG_QPCR_LEN];
+	u16 hw_id;
 
 	/* The purpose of "thin" policer is to drop as many packets
 	 * as possible. The dummy group is using it.
 	 */
-	__set_bit(MLXSW_SP_THIN_POLICER_ID, mlxsw_sp->trap->policers_usage);
-	mlxsw_reg_qpcr_pack(qpcr_pl, MLXSW_SP_THIN_POLICER_ID,
-			    MLXSW_REG_QPCR_IR_UNITS_M, false, 1, 4);
+	hw_id = find_first_zero_bit(trap->policers_usage, trap->max_policers);
+	if (WARN_ON(hw_id == trap->max_policers))
+		return -ENOBUFS;
+
+	__set_bit(hw_id, trap->policers_usage);
+	trap->thin_policer_hw_id = hw_id;
+	mlxsw_reg_qpcr_pack(qpcr_pl, hw_id, MLXSW_REG_QPCR_IR_UNITS_M,
+			    false, 1, 4);
 	return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(qpcr), qpcr_pl);
 }
 
@@ -503,7 +508,7 @@ static int mlxsw_sp_trap_dummy_group_init(struct mlxsw_sp *mlxsw_sp)
 	char htgt_pl[MLXSW_REG_HTGT_LEN];
 
 	mlxsw_reg_htgt_pack(htgt_pl, MLXSW_REG_HTGT_TRAP_GROUP_SP_DUMMY,
-			    MLXSW_SP_THIN_POLICER_ID, 0, 1);
+			    mlxsw_sp->trap->thin_policer_hw_id, 0, 1);
 	return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(htgt), htgt_pl);
 }
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h
index 759146897b3a..13ac412f4d53 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h
@@ -17,6 +17,8 @@ struct mlxsw_sp_trap {
 	struct mlxsw_sp_trap_item *trap_items_arr;
 	u64 traps_count; /* Number of registered traps */
 
+	u16 thin_policer_hw_id;
+
 	u64 max_policers;
 	unsigned long policers_usage[]; /* Usage bitmap */
 };
-- 
2.26.2

[PATCH net-next 10/14] mlxsw: reg: Move all trap groups under the same enum

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

After the previous patch the split is no longer necessary and all the
trap groups can be moved under the same enum.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/reg.h | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index fd5e18b71114..586a2f37fd12 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -5548,18 +5548,14 @@ enum mlxsw_reg_htgt_trap_group {
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_PKT_SAMPLE,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_FLOW_LOGGING,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_FID_MISS,
-
-	__MLXSW_REG_HTGT_TRAP_GROUP_MAX,
-	MLXSW_REG_HTGT_TRAP_GROUP_MAX = __MLXSW_REG_HTGT_TRAP_GROUP_MAX - 1
-};
-
-enum mlxsw_reg_htgt_discard_trap_group {
-	MLXSW_REG_HTGT_DISCARD_TRAP_GROUP_BASE = MLXSW_REG_HTGT_TRAP_GROUP_MAX,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_DUMMY,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_L2_DISCARDS,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_L3_DISCARDS,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_TUNNEL_DISCARDS,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_ACL_DISCARDS,
+
+	__MLXSW_REG_HTGT_TRAP_GROUP_MAX,
+	MLXSW_REG_HTGT_TRAP_GROUP_MAX = __MLXSW_REG_HTGT_TRAP_GROUP_MAX - 1
 };
 
 /* reg_htgt_trap_group
-- 
2.26.2

[PATCH net-next 11/14] mlxsw: spectrum: Share one group for all locally delivered packets

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

Routed IP packets with the Router Alert option need to be trapped to
the CPU as they might need to be locally delivered to raw sockets with
the IP_ROUTER_ALERT / IPV6_ROUTER_ALERT socket option.

Move them to the same group with other packets that might need to be
trapped following route lookup.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index b10e5aeaedef..016df2c14f0e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4089,8 +4089,8 @@ static const struct mlxsw_listener mlxsw_sp_listener[] = {
 	MLXSW_SP_RXL_MARK(L3_IPV6_REDIRECTION, TRAP_TO_CPU, IPV6, false),
 	MLXSW_SP_RXL_MARK(IPV6_MC_LINK_LOCAL_DEST, TRAP_TO_CPU, ROUTER_EXP,
 			  false),
-	MLXSW_SP_RXL_MARK(ROUTER_ALERT_IPV4, TRAP_TO_CPU, ROUTER_EXP, false),
-	MLXSW_SP_RXL_MARK(ROUTER_ALERT_IPV6, TRAP_TO_CPU, ROUTER_EXP, false),
+	MLXSW_SP_RXL_MARK(ROUTER_ALERT_IPV4, TRAP_TO_CPU, IP2ME, false),
+	MLXSW_SP_RXL_MARK(ROUTER_ALERT_IPV6, TRAP_TO_CPU, IP2ME, false),
 	MLXSW_SP_RXL_MARK(IPV4_VRRP, TRAP_TO_CPU, VRRP, false),
 	MLXSW_SP_RXL_MARK(IPV6_VRRP, TRAP_TO_CPU, VRRP, false),
 	MLXSW_SP_RXL_NO_MARK(DISCARD_ING_ROUTER_SIP_CLASS_E, FORWARD,
-- 
2.26.2

[PATCH net-next 12/14] mlxsw: spectrum: Treat IPv6 link-local SIP as an exception

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

IPv6 packets that need to be forwarded and have a link-local source IP are
dropped by the kernel and an ICMPv6 "Destination unreachable" is sent to
the sending host.

As such, change the trap group of such packets so that they do not
interfere with IPv6 management packets. In the future this trap will be
exposed as an exception via devlink-trap.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 016df2c14f0e..5cb7fd650156 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4067,7 +4067,7 @@ static const struct mlxsw_listener mlxsw_sp_listener[] = {
 	MLXSW_SP_RXL_MARK(IPV6_UNSPECIFIED_ADDRESS, TRAP_TO_CPU, ROUTER_EXP,
 			  false),
 	MLXSW_SP_RXL_MARK(IPV6_LINK_LOCAL_DEST, TRAP_TO_CPU, IP2ME, false),
-	MLXSW_SP_RXL_MARK(IPV6_LINK_LOCAL_SRC, TRAP_TO_CPU, IPV6, false),
+	MLXSW_SP_RXL_MARK(IPV6_LINK_LOCAL_SRC, TRAP_TO_CPU, ROUTER_EXP, false),
 	MLXSW_SP_RXL_MARK(IPV6_ALL_NODES_LINK, TRAP_TO_CPU, IPV6, false),
 	MLXSW_SP_RXL_MARK(IPV6_ALL_ROUTERS_LINK, TRAP_TO_CPU, IPV6,
 			  false),
-- 
2.26.2

[PATCH net-next 13/14] mlxsw: spectrum: Add packet traps for BFD packets

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

Bidirectional Forwarding Detection (BFD) provides "low-overhead,
short-duration detection of failures in the path between adjacent
forwarding engines" (RFC 5880).

This is accomplished by exchanging BFD packets between the two
forwarding engines. Up until now these packets were trapped via the
general local delivery (i.e., IP2ME) trap which also traps a lot of
other packets that are not as time-sensitive as BFD packets.

Expose dedicated traps for BFD packets so that user space could
configure a dedicated policer for them.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/reg.h      | 1 +
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 7 +++++++
 drivers/net/ethernet/mellanox/mlxsw/trap.h     | 2 ++
 3 files changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 586a2f37fd12..38fa7304af0c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -5548,6 +5548,7 @@ enum mlxsw_reg_htgt_trap_group {
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_PKT_SAMPLE,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_FLOW_LOGGING,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_FID_MISS,
+	MLXSW_REG_HTGT_TRAP_GROUP_SP_BFD,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_DUMMY,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_L2_DISCARDS,
 	MLXSW_REG_HTGT_TRAP_GROUP_SP_L3_DISCARDS,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 5cb7fd650156..c598ae9ed106 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4093,6 +4093,8 @@ static const struct mlxsw_listener mlxsw_sp_listener[] = {
 	MLXSW_SP_RXL_MARK(ROUTER_ALERT_IPV6, TRAP_TO_CPU, IP2ME, false),
 	MLXSW_SP_RXL_MARK(IPV4_VRRP, TRAP_TO_CPU, VRRP, false),
 	MLXSW_SP_RXL_MARK(IPV6_VRRP, TRAP_TO_CPU, VRRP, false),
+	MLXSW_SP_RXL_MARK(IPV4_BFD, TRAP_TO_CPU, BFD, false),
+	MLXSW_SP_RXL_MARK(IPV6_BFD, TRAP_TO_CPU, BFD, false),
 	MLXSW_SP_RXL_NO_MARK(DISCARD_ING_ROUTER_SIP_CLASS_E, FORWARD,
 			     ROUTER_EXP, false),
 	MLXSW_SP_RXL_NO_MARK(DISCARD_ING_ROUTER_MC_DMAC, FORWARD,
@@ -4185,6 +4187,10 @@ static int mlxsw_sp_cpu_policers_set(struct mlxsw_core *mlxsw_core)
 			rate = 360;
 			burst_size = 7;
 			break;
+		case MLXSW_REG_HTGT_TRAP_GROUP_SP_BFD:
+			rate = 20 * 1024;
+			burst_size = 10;
+			break;
 		default:
 			continue;
 		}
@@ -4226,6 +4232,7 @@ static int mlxsw_sp_trap_groups_set(struct mlxsw_core *mlxsw_core)
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_PIM:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_PTP0:
 		case MLXSW_REG_HTGT_TRAP_GROUP_SP_VRRP:
+		case MLXSW_REG_HTGT_TRAP_GROUP_SP_BFD:
 			priority = 5;
 			tc = 5;
 			break;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/trap.h b/drivers/net/ethernet/mellanox/mlxsw/trap.h
index 1b89472a0908..28e60697d14e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/trap.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/trap.h
@@ -66,6 +66,8 @@ enum {
 	MLXSW_TRAP_ID_IPIP_DECAP_ERROR = 0xB1,
 	MLXSW_TRAP_ID_NVE_DECAP_ARP = 0xB8,
 	MLXSW_TRAP_ID_NVE_ENCAP_ARP = 0xBD,
+	MLXSW_TRAP_ID_IPV4_BFD = 0xD0,
+	MLXSW_TRAP_ID_IPV6_BFD = 0xD1,
 	MLXSW_TRAP_ID_ROUTER_ALERT_IPV4 = 0xD6,
 	MLXSW_TRAP_ID_ROUTER_ALERT_IPV6 = 0xD7,
 	MLXSW_TRAP_ID_DISCARD_NON_ROUTABLE = 0x11A,
-- 
2.26.2

[PATCH net-next 14/14] mlxsw: spectrum_router: Allow programming link-local prefix routes

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

The device has a trap for IPv6 packets that need be routed and have a
unicast link-local destination IP (i.e., fe80::/10). This allows mlxsw
to ignore link-local routes, as the packets will be trapped to the CPU
in any case.

However, since link-local routes are not programmed, it is possible for
routed packets to hit the default route which might also be programmed
to trap packets. This means that packets with a link-local destination
IP might be trapped for the wrong reason.

To overcome this, allow programming link-local prefix routes (usually
one fe80::/64 per-table), so that the packets will be forwarded until
reaching the link-local trap.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 71aee4914619..c939b3596566 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -5003,9 +5003,11 @@ static bool mlxsw_sp_fib6_rt_should_ignore(const struct fib6_info *rt)
 {
 	/* Packets with link-local destination IP arriving to the router
 	 * are trapped to the CPU, so no need to program specific routes
-	 * for them.
+	 * for them. Only allow prefix routes (usually one fe80::/64) so
+	 * that packets are trapped for the right reason.
 	 */
-	if (ipv6_addr_type(&rt->fib6_dst.addr) & IPV6_ADDR_LINKLOCAL)
+	if ((ipv6_addr_type(&rt->fib6_dst.addr) & IPV6_ADDR_LINKLOCAL) &&
+	    (rt->fib6_flags & (RTF_LOCAL | RTF_ANYCAST)))
 		return true;
 
 	/* Multicast routes aren't supported, so ignore them. Neighbour
-- 
2.26.2

Re: [PATCH net-next 00/14] mlxsw: Various trap changes - part 2

[Jakub Kicinski]

On Tue, 26 May 2020 02:05:42 +0300 Ido Schimmel wrote:
> From: Ido Schimmel <idosch@mellanox.com>
> 
> This patch set contains another set of small changes in mlxsw trap
> configuration. It is the last set before exposing control traps (e.g.,
> IGMP query, ARP request) via devlink-trap.

When traps were introduced my understanding was that they are for
reporting frames which hit an expectation on the datapath. IOW the
primary use for them was troubleshooting. 

Now, if I'm following things correctly we have explicit DHCP, IGMP,
ARP, ND, BFD etc. traps. Are we still in the troubleshooting realm?

Re: [PATCH net-next 00/14] mlxsw: Various trap changes - part 2

[Ido Schimmel]

On Tue, May 26, 2020 at 03:14:37PM -0700, Jakub Kicinski wrote:
> On Tue, 26 May 2020 02:05:42 +0300 Ido Schimmel wrote:
> > From: Ido Schimmel <idosch@mellanox.com>
> > 
> > This patch set contains another set of small changes in mlxsw trap
> > configuration. It is the last set before exposing control traps (e.g.,
> > IGMP query, ARP request) via devlink-trap.
> 
> When traps were introduced my understanding was that they are for
> reporting frames which hit an expectation on the datapath. IOW the
> primary use for them was troubleshooting.
> 
> Now, if I'm following things correctly we have explicit DHCP, IGMP,
> ARP, ND, BFD etc. traps. Are we still in the troubleshooting realm?

First of all, we always had them. This patch set mainly performs some
cleanups in mlxsw.

Second, I don't understand how you got the impression that the primary
use of devlink-trap is troubleshooting. I was very clear and transparent
about the scope of the work from the very beginning and I don't wish to
be portrayed as if I wasn't.

The first two paragraphs from the kernel documentation [1] explicitly
mention the ability to trap control packets to the CPU:

"Devices capable of offloading the kernel’s datapath and perform
functions such as bridging and routing must also be able to send
specific packets to the kernel (i.e., the CPU) for processing.

For example, a device acting as a multicast-aware bridge must be able to
send IGMP membership reports to the kernel for processing by the bridge
module. Without processing such packets, the bridge module could never
populate its MDB."

In my reply to you from almost a year ago I outlined the entire plan for
devlink-trap [2]:

"Switch ASICs have dedicated traps for specific packets. Usually, these
packets are control packets (e.g., ARP, BGP) which are required for the
correct functioning of the control plane. You can see this in the SAI
interface, which is an abstraction layer over vendors' SDKs:

https://github.com/opencomputeproject/SAI/blob/master/inc/saihostif.h#L157

We need to be able to configure the hardware policers of these traps and
read their statistics to understand how many packets they dropped. We
currently do not have a way to do any of that and we rely on hardcoded
defaults in the driver which do not fit every use case (from
experience):

https://elixir.bootlin.com/linux/v5.2/source/drivers/net/ethernet/mellanox/mlxsw/spectrum.c#L4103

We plan to extend devlink-trap mechanism to cover all these use cases. I
hope you agree that this functionality belongs in devlink given it is a
device-specific configuration and not a netdev-specific one.

That being said, in its current form, this mechanism is focused on traps
that correlate to packets the device decided to drop as this is very
useful for debugging."

In the last cycle, when I added the ability to configure trap policers
[3] I again mentioned under "Future plans" that I plan to "Add more
packet traps.  For example, for control packets (e.g., IGMP)".

If you worry that packets received via control traps will be somehow
tunneled to user space via drop_monitor, then I can assure you this is
not the case. You can refer to this commit [4] from the next submission.

Thanks

[1] https://www.kernel.org/doc/html/latest/networking/devlink/devlink-trap.html
[2] https://lore.kernel.org/netdev/20190709123844.GA27309@splinter/
[3] https://lwn.net/Articles/815948/
[4] https://github.com/jpirko/linux_mlxsw/commit/4a5f0a8034f0f10301680fe68559e3debacf534d 

Re: [PATCH net-next 00/14] mlxsw: Various trap changes - part 2

[Jakub Kicinski]

On Wed, 27 May 2020 02:19:05 +0300 Ido Schimmel wrote:
> On Tue, May 26, 2020 at 03:14:37PM -0700, Jakub Kicinski wrote:
> > On Tue, 26 May 2020 02:05:42 +0300 Ido Schimmel wrote:  
> > > From: Ido Schimmel <idosch@mellanox.com>
> > > 
> > > This patch set contains another set of small changes in mlxsw trap
> > > configuration. It is the last set before exposing control traps (e.g.,
> > > IGMP query, ARP request) via devlink-trap.  
> > 
> > When traps were introduced my understanding was that they are for
> > reporting frames which hit an expectation on the datapath. IOW the
> > primary use for them was troubleshooting.
> > 
> > Now, if I'm following things correctly we have explicit DHCP, IGMP,
> > ARP, ND, BFD etc. traps. Are we still in the troubleshooting realm?  
> 
> First of all, we always had them. This patch set mainly performs some
> cleanups in mlxsw.
> 
> Second, I don't understand how you got the impression that the primary
> use of devlink-trap is troubleshooting. I was very clear and transparent
> about the scope of the work from the very beginning and I don't wish to
> be portrayed as if I wasn't.
> 
> The first two paragraphs from the kernel documentation [1] explicitly
> mention the ability to trap control packets to the CPU:
> 
> "Devices capable of offloading the kernel’s datapath and perform
> functions such as bridging and routing must also be able to send
> specific packets to the kernel (i.e., the CPU) for processing.
> 
> For example, a device acting as a multicast-aware bridge must be able to
> send IGMP membership reports to the kernel for processing by the bridge
> module. Without processing such packets, the bridge module could never
> populate its MDB."
> 
> In my reply to you from almost a year ago I outlined the entire plan for
> devlink-trap [2]:
> 
> "Switch ASICs have dedicated traps for specific packets. Usually, these
> packets are control packets (e.g., ARP, BGP) which are required for the
> correct functioning of the control plane. You can see this in the SAI
> interface, which is an abstraction layer over vendors' SDKs:
> 
> https://github.com/opencomputeproject/SAI/blob/master/inc/saihostif.h#L157
> 
> We need to be able to configure the hardware policers of these traps and
> read their statistics to understand how many packets they dropped. We
> currently do not have a way to do any of that and we rely on hardcoded
> defaults in the driver which do not fit every use case (from
> experience):
> 
> https://elixir.bootlin.com/linux/v5.2/source/drivers/net/ethernet/mellanox/mlxsw/spectrum.c#L4103
> 
> We plan to extend devlink-trap mechanism to cover all these use cases. I
> hope you agree that this functionality belongs in devlink given it is a
> device-specific configuration and not a netdev-specific one.
> 
> That being said, in its current form, this mechanism is focused on traps
> that correlate to packets the device decided to drop as this is very
> useful for debugging."
> 
> In the last cycle, when I added the ability to configure trap policers
> [3] I again mentioned under "Future plans" that I plan to "Add more
> packet traps.  For example, for control packets (e.g., IGMP)".
> 
> If you worry that packets received via control traps will be somehow
> tunneled to user space via drop_monitor, then I can assure you this is
> not the case. You can refer to this commit [4] from the next submission.

Perhaps the troubleshooting is how I justified the necessity of having
devlink traps to myself. It's much harder to get visibility into what
HW is doing and therefore we need this special interface.

But we should be able to configure a DHCP daemon without any special
sauce. What purpose does the DHCP trap serve?

What's the packet flow for BFD? How does the HW case differ from a SW
router?

Re: [PATCH net-next 00/14] mlxsw: Various trap changes - part 2

[David Miller]

From: Ido Schimmel <idosch@idosch.org>
Date: Tue, 26 May 2020 02:05:42 +0300

> From: Ido Schimmel <idosch@mellanox.com>
> 
> This patch set contains another set of small changes in mlxsw trap
> configuration. It is the last set before exposing control traps (e.g.,
> IGMP query, ARP request) via devlink-trap.
> 
> Tested with existing devlink-trap selftests. Please see individual
> patches for a detailed changelog.

Since this is just a cleanup of existing code, series applied since it
is independent of the discussion Ido is having with Jakub.

Re: [PATCH net-next 00/14] mlxsw: Various trap changes - part 2

[Ido Schimmel]

On Tue, May 26, 2020 at 04:43:23PM -0700, Jakub Kicinski wrote:
> On Wed, 27 May 2020 02:19:05 +0300 Ido Schimmel wrote:
> > On Tue, May 26, 2020 at 03:14:37PM -0700, Jakub Kicinski wrote:
> > > On Tue, 26 May 2020 02:05:42 +0300 Ido Schimmel wrote:  
> > > > From: Ido Schimmel <idosch@mellanox.com>
> > > > 
> > > > This patch set contains another set of small changes in mlxsw trap
> > > > configuration. It is the last set before exposing control traps (e.g.,
> > > > IGMP query, ARP request) via devlink-trap.  
> > > 
> > > When traps were introduced my understanding was that they are for
> > > reporting frames which hit an expectation on the datapath. IOW the
> > > primary use for them was troubleshooting.
> > > 
> > > Now, if I'm following things correctly we have explicit DHCP, IGMP,
> > > ARP, ND, BFD etc. traps. Are we still in the troubleshooting realm?  
> > 
> > First of all, we always had them. This patch set mainly performs some
> > cleanups in mlxsw.
> > 
> > Second, I don't understand how you got the impression that the primary
> > use of devlink-trap is troubleshooting. I was very clear and transparent
> > about the scope of the work from the very beginning and I don't wish to
> > be portrayed as if I wasn't.
> > 
> > The first two paragraphs from the kernel documentation [1] explicitly
> > mention the ability to trap control packets to the CPU:
> > 
> > "Devices capable of offloading the kernel’s datapath and perform
> > functions such as bridging and routing must also be able to send
> > specific packets to the kernel (i.e., the CPU) for processing.
> > 
> > For example, a device acting as a multicast-aware bridge must be able to
> > send IGMP membership reports to the kernel for processing by the bridge
> > module. Without processing such packets, the bridge module could never
> > populate its MDB."
> > 
> > In my reply to you from almost a year ago I outlined the entire plan for
> > devlink-trap [2]:
> > 
> > "Switch ASICs have dedicated traps for specific packets. Usually, these
> > packets are control packets (e.g., ARP, BGP) which are required for the
> > correct functioning of the control plane. You can see this in the SAI
> > interface, which is an abstraction layer over vendors' SDKs:
> > 
> > https://github.com/opencomputeproject/SAI/blob/master/inc/saihostif.h#L157
> > 
> > We need to be able to configure the hardware policers of these traps and
> > read their statistics to understand how many packets they dropped. We
> > currently do not have a way to do any of that and we rely on hardcoded
> > defaults in the driver which do not fit every use case (from
> > experience):
> > 
> > https://elixir.bootlin.com/linux/v5.2/source/drivers/net/ethernet/mellanox/mlxsw/spectrum.c#L4103
> > 
> > We plan to extend devlink-trap mechanism to cover all these use cases. I
> > hope you agree that this functionality belongs in devlink given it is a
> > device-specific configuration and not a netdev-specific one.
> > 
> > That being said, in its current form, this mechanism is focused on traps
> > that correlate to packets the device decided to drop as this is very
> > useful for debugging."
> > 
> > In the last cycle, when I added the ability to configure trap policers
> > [3] I again mentioned under "Future plans" that I plan to "Add more
> > packet traps.  For example, for control packets (e.g., IGMP)".
> > 
> > If you worry that packets received via control traps will be somehow
> > tunneled to user space via drop_monitor, then I can assure you this is
> > not the case. You can refer to this commit [4] from the next submission.
> 
> Perhaps the troubleshooting is how I justified the necessity of having
> devlink traps to myself. It's much harder to get visibility into what
> HW is doing and therefore we need this special interface.
> 
> But we should be able to configure a DHCP daemon without any special
> sauce. What purpose does the DHCP trap serve?
> 
> What's the packet flow for BFD? How does the HW case differ from a SW
> router?

There is no special sauce required to get a DHCP daemon working nor BFD.
It is supposed to Just Work. Same for IGMP / MLD snooping, STP etc. This
is enabled by the ASIC trapping the required packets to the CPU.

However, having a 3.2/6.4/12.8 Tbps ASIC (it keeps growing all the time)
send traffic to the CPU can very easily result in denial of service. You
need to have hardware policers and classification to different traffic
classes ensuring the system remains functional regardless of the havoc
happening in the offloaded data path.

This control plane policy has been hard coded in mlxsw for a few years
now (based on sane defaults), but it obviously does not fit everyone's
needs. Different users have different use cases and different CPUs
connected to the ASIC. Some have Celeron / Atom while others have more
high-end Xeon CPUs, which are obviously capable of handling more packets
per second. You also have zero visibility into how many packets were
dropped by these hardware policers.

By exposing these traps we allow users to tune these policers and get
visibility into how many packets they dropped. In the future also
changing their traffic class, so that (for example), packets hitting
local routes are scheduled towards the CPU before packets dropped due to
ingress VLAN filter.

If you don't have any special needs you are probably OK with the
defaults, in which case you don't need to do anything (no special
sauce).

Re: [PATCH net-next 00/14] mlxsw: Various trap changes - part 2

[Jakub Kicinski]

On Wed, 27 May 2020 10:38:57 +0300 Ido Schimmel wrote:
> There is no special sauce required to get a DHCP daemon working nor BFD.
> It is supposed to Just Work. Same for IGMP / MLD snooping, STP etc. This
> is enabled by the ASIC trapping the required packets to the CPU.
> 
> However, having a 3.2/6.4/12.8 Tbps ASIC (it keeps growing all the time)
> send traffic to the CPU can very easily result in denial of service. You
> need to have hardware policers and classification to different traffic
> classes ensuring the system remains functional regardless of the havoc
> happening in the offloaded data path.

I don't see how that's only applicable to a switch ASIC, though.
Ingress classification, and rate limiting applies to any network 
system.

> This control plane policy has been hard coded in mlxsw for a few years
> now (based on sane defaults), but it obviously does not fit everyone's
> needs. Different users have different use cases and different CPUs
> connected to the ASIC. Some have Celeron / Atom while others have more
> high-end Xeon CPUs, which are obviously capable of handling more packets
> per second. You also have zero visibility into how many packets were
> dropped by these hardware policers.

There are embedded Atom systems out there with multi-gig interfaces,
they obviously can't ingest peak traffic, doesn't matter whether they
are connected to a switch ASIC or a NIC.

> By exposing these traps we allow users to tune these policers and get
> visibility into how many packets they dropped. In the future also
> changing their traffic class, so that (for example), packets hitting
> local routes are scheduled towards the CPU before packets dropped due to
> ingress VLAN filter.
> 
> If you don't have any special needs you are probably OK with the
> defaults, in which case you don't need to do anything (no special
> sauce).

As much as traps which forward traffic to the CPU fit the switch
programming model, we'd rather see a solution that offloads constructs
which are also applicable to the software world.

Sniffing dropped frames to troubleshoot is one thing, but IMHO traps
which default to "trap" are a bad smell.

Re: [PATCH net-next 00/14] mlxsw: Various trap changes - part 2

[Ido Schimmel]

On Wed, May 27, 2020 at 12:50:17PM -0700, Jakub Kicinski wrote:
> On Wed, 27 May 2020 10:38:57 +0300 Ido Schimmel wrote:
> > There is no special sauce required to get a DHCP daemon working nor BFD.
> > It is supposed to Just Work. Same for IGMP / MLD snooping, STP etc. This
> > is enabled by the ASIC trapping the required packets to the CPU.
> > 
> > However, having a 3.2/6.4/12.8 Tbps ASIC (it keeps growing all the time)
> > send traffic to the CPU can very easily result in denial of service. You
> > need to have hardware policers and classification to different traffic
> > classes ensuring the system remains functional regardless of the havoc
> > happening in the offloaded data path.
> 
> I don't see how that's only applicable to a switch ASIC, though.
> Ingress classification, and rate limiting applies to any network 
> system.

This is not about ingress classification and rate limiting. The
classification does not happen at ingress. It happens throughout
different points in the pipeline, by hard-coded checks meant to identify
packets of interest. These checks look at both state (e.g., neighbour
miss, route miss) and packet fields (e.g., BGP packet that hit a local
route).

Similarly, the rate limiting does not happen at ingress. It only applies
to packets that your offloaded data path decided should go to the
attached host (the control plane). You cannot perform the rate limiting
at ingress for the simple reason that you still do not know if the
packet should reach the control plane.

> 
> > This control plane policy has been hard coded in mlxsw for a few years
> > now (based on sane defaults), but it obviously does not fit everyone's
> > needs. Different users have different use cases and different CPUs
> > connected to the ASIC. Some have Celeron / Atom while others have more
> > high-end Xeon CPUs, which are obviously capable of handling more packets
> > per second. You also have zero visibility into how many packets were
> > dropped by these hardware policers.
> 
> There are embedded Atom systems out there with multi-gig interfaces,
> they obviously can't ingest peak traffic, doesn't matter whether they
> are connected to a switch ASIC or a NIC.

Not the same thing. Every packet received by such systems should reach
the attached host. The control plane and the data plane are the same.
The whole point of this work is to rate limit packets coming from your
offloaded data plane to the control plane.

> 
> > By exposing these traps we allow users to tune these policers and get
> > visibility into how many packets they dropped. In the future also
> > changing their traffic class, so that (for example), packets hitting
> > local routes are scheduled towards the CPU before packets dropped due to
> > ingress VLAN filter.
> > 
> > If you don't have any special needs you are probably OK with the
> > defaults, in which case you don't need to do anything (no special
> > sauce).
> 
> As much as traps which forward traffic to the CPU fit the switch
> programming model, we'd rather see a solution that offloads constructs
> which are also applicable to the software world.

In the software world the data plane and the control plane are the same.
The CPU sees every packet. IGMP packets trigger MDB modifications,
packets that incurred a neighbour miss trigger an ARP / ND etc. These
are all control plane operations.

Once you separate your control plane from the data plane and offload the
latter to a capable hardware (e.g., switch ASIC), you create a need to
limit the packets coming from your data plane to the control plane. This
is a hardware-specific problem.

> 
> Sniffing dropped frames to troubleshoot is one thing, but IMHO traps
> which default to "trap" are a bad smell.

These traps exist today. They are programmed by mlxsw during
initialization. Without them basic stuff like DHCP/ARP/STP would not
work and you would need the "special sauce" you previously mentioned.

By exposing them via devlink-trap we allow users to configure their rate
from the offloaded data plane towards the control plane running on the
attached host. This is the only set operation you can do. Nothing else.

Anyway, I don't know how to argue with "bad smell". I held off on
sending the next patch set because this discussion was on-going, but at
this point I don't think it's possible for me to explain the problem and
solution in a clearer fashion, so I'll go ahead and send the patches.

Thanks