[PATCH net-next 0/2] mlxsw: Offload blackhole routes

[PATCH net-next 0/2] mlxsw: Offload blackhole routes

[Ido Schimmel]

Blackhole routes are routes that cause matching packets to be silently
dropped. This is in contrast to unreachable routes that generate an ICMP
host unreachable packet in response.

The driver currently programs both route types with a trap action and
lets the kernel drop matching packets. This is sub-optimal as packets
routed using a blackhole route can be directly dropped by the ASIC.

Patch #1 alters mlxsw to program blackhole routes with a discard action.

Patch #2 adds a matching test.

Ido Schimmel (2):
  mlxsw: spectrum_router: Offload blackhole routes
  selftests: mlxsw: Add a test for blackhole routes

 .../ethernet/mellanox/mlxsw/spectrum_router.c |  27 ++-
 .../drivers/net/mlxsw/blackhole_routes.sh     | 200 ++++++++++++++++++
 2 files changed, 225 insertions(+), 2 deletions(-)
 create mode 100755 tools/testing/selftests/drivers/net/mlxsw/blackhole_routes.sh

-- 
2.20.1

[PATCH net-next 1/2] mlxsw: spectrum_router: Offload blackhole routes

[Ido Schimmel]

Create a new FIB entry type for blackhole routes and set it in case the
type of the notified route is 'RTN_BLACKHOLE'.

Program such routes with a discard action and mark them as offloaded
since the device is dropping the packets instead of the kernel.

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

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 230e1f6e192b..6754061d9b72 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -364,6 +364,7 @@ enum mlxsw_sp_fib_entry_type {
 	MLXSW_SP_FIB_ENTRY_TYPE_REMOTE,
 	MLXSW_SP_FIB_ENTRY_TYPE_LOCAL,
 	MLXSW_SP_FIB_ENTRY_TYPE_TRAP,
+	MLXSW_SP_FIB_ENTRY_TYPE_BLACKHOLE,
 
 	/* This is a special case of local delivery, where a packet should be
 	 * decapsulated on reception. Note that there is no corresponding ENCAP,
@@ -3928,6 +3929,7 @@ mlxsw_sp_fib_entry_should_offload(const struct mlxsw_sp_fib_entry *fib_entry)
 		return !!nh_group->adj_index_valid;
 	case MLXSW_SP_FIB_ENTRY_TYPE_LOCAL:
 		return !!nh_group->nh_rif;
+	case MLXSW_SP_FIB_ENTRY_TYPE_BLACKHOLE:
 	case MLXSW_SP_FIB_ENTRY_TYPE_IPIP_DECAP:
 	case MLXSW_SP_FIB_ENTRY_TYPE_NVE_DECAP:
 		return true;
@@ -3963,6 +3965,7 @@ mlxsw_sp_fib4_entry_offload_set(struct mlxsw_sp_fib_entry *fib_entry)
 	int i;
 
 	if (fib_entry->type == MLXSW_SP_FIB_ENTRY_TYPE_LOCAL ||
+	    fib_entry->type == MLXSW_SP_FIB_ENTRY_TYPE_BLACKHOLE ||
 	    fib_entry->type == MLXSW_SP_FIB_ENTRY_TYPE_IPIP_DECAP ||
 	    fib_entry->type == MLXSW_SP_FIB_ENTRY_TYPE_NVE_DECAP) {
 		nh_grp->nexthops->key.fib_nh->nh_flags |= RTNH_F_OFFLOAD;
@@ -4004,7 +4007,8 @@ mlxsw_sp_fib6_entry_offload_set(struct mlxsw_sp_fib_entry *fib_entry)
 	fib6_entry = container_of(fib_entry, struct mlxsw_sp_fib6_entry,
 				  common);
 
-	if (fib_entry->type == MLXSW_SP_FIB_ENTRY_TYPE_LOCAL) {
+	if (fib_entry->type == MLXSW_SP_FIB_ENTRY_TYPE_LOCAL ||
+	    fib_entry->type == MLXSW_SP_FIB_ENTRY_TYPE_BLACKHOLE) {
 		list_first_entry(&fib6_entry->rt6_list, struct mlxsw_sp_rt6,
 				 list)->rt->fib6_nh.nh_flags |= RTNH_F_OFFLOAD;
 		return;
@@ -4172,6 +4176,19 @@ static int mlxsw_sp_fib_entry_op_trap(struct mlxsw_sp *mlxsw_sp,
 	return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(ralue), ralue_pl);
 }
 
+static int mlxsw_sp_fib_entry_op_blackhole(struct mlxsw_sp *mlxsw_sp,
+					   struct mlxsw_sp_fib_entry *fib_entry,
+					   enum mlxsw_reg_ralue_op op)
+{
+	enum mlxsw_reg_ralue_trap_action trap_action;
+	char ralue_pl[MLXSW_REG_RALUE_LEN];
+
+	trap_action = MLXSW_REG_RALUE_TRAP_ACTION_DISCARD_ERROR;
+	mlxsw_sp_fib_entry_ralue_pack(ralue_pl, fib_entry, op);
+	mlxsw_reg_ralue_act_local_pack(ralue_pl, trap_action, 0, 0);
+	return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(ralue), ralue_pl);
+}
+
 static int
 mlxsw_sp_fib_entry_op_ipip_decap(struct mlxsw_sp *mlxsw_sp,
 				 struct mlxsw_sp_fib_entry *fib_entry,
@@ -4211,6 +4228,8 @@ static int __mlxsw_sp_fib_entry_op(struct mlxsw_sp *mlxsw_sp,
 		return mlxsw_sp_fib_entry_op_local(mlxsw_sp, fib_entry, op);
 	case MLXSW_SP_FIB_ENTRY_TYPE_TRAP:
 		return mlxsw_sp_fib_entry_op_trap(mlxsw_sp, fib_entry, op);
+	case MLXSW_SP_FIB_ENTRY_TYPE_BLACKHOLE:
+		return mlxsw_sp_fib_entry_op_blackhole(mlxsw_sp, fib_entry, op);
 	case MLXSW_SP_FIB_ENTRY_TYPE_IPIP_DECAP:
 		return mlxsw_sp_fib_entry_op_ipip_decap(mlxsw_sp,
 							fib_entry, op);
@@ -4279,8 +4298,10 @@ mlxsw_sp_fib4_entry_type_set(struct mlxsw_sp *mlxsw_sp,
 	case RTN_BROADCAST:
 		fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_TRAP;
 		return 0;
+	case RTN_BLACKHOLE:
+		fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_BLACKHOLE;
+		return 0;
 	case RTN_UNREACHABLE: /* fall through */
-	case RTN_BLACKHOLE: /* fall through */
 	case RTN_PROHIBIT:
 		/* Packets hitting these routes need to be trapped, but
 		 * can do so with a lower priority than packets directed
@@ -5229,6 +5250,8 @@ static void mlxsw_sp_fib6_entry_type_set(struct mlxsw_sp *mlxsw_sp,
 	 */
 	if (rt->fib6_flags & (RTF_LOCAL | RTF_ANYCAST))
 		fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_TRAP;
+	else if (rt->fib6_type == RTN_BLACKHOLE)
+		fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_BLACKHOLE;
 	else if (rt->fib6_flags & RTF_REJECT)
 		fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_LOCAL;
 	else if (mlxsw_sp_rt6_is_gateway(mlxsw_sp, rt))
-- 
2.20.1

Re: [PATCH net-next 1/2] mlxsw: spectrum_router: Offload blackhole routes

[David Ahern]

On 2/6/19 11:42 AM, Ido Schimmel wrote:
> Create a new FIB entry type for blackhole routes and set it in case the
> type of the notified route is 'RTN_BLACKHOLE'.
> 
> Program such routes with a discard action and mark them as offloaded
> since the device is dropping the packets instead of the kernel.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  .../ethernet/mellanox/mlxsw/spectrum_router.c | 27 +++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 

One of the feature requests from the FRR team (and a feature I have
implemented) is a blackhole nexthop. The idea is that prefixes are
installed pointing to nexthop id N. That nexthop definition can be
atomically updated to go between a device / gateway and a blackhole.


 [ prefix ] --> [ nhid 1 ] --> [ dev1 / gateway1 ]


 [ prefix ] --> [ nhid 1 ] --> [ blackhole ]


 [ prefix ] --> [ nhid 1 ] --> [ dev2 / gateway2 ]

Do you see this working ok with mlxsw without having to update the
prefix entries (which can be numerous) directly?

Re: [PATCH net-next 1/2] mlxsw: spectrum_router: Offload blackhole routes

[Ido Schimmel]

On Thu, Feb 07, 2019 at 04:40:11PM -0800, David Ahern wrote:
> On 2/6/19 11:42 AM, Ido Schimmel wrote:
> > Create a new FIB entry type for blackhole routes and set it in case the
> > type of the notified route is 'RTN_BLACKHOLE'.
> > 
> > Program such routes with a discard action and mark them as offloaded
> > since the device is dropping the packets instead of the kernel.
> > 
> > Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> > Acked-by: Jiri Pirko <jiri@mellanox.com>
> > ---
> >  .../ethernet/mellanox/mlxsw/spectrum_router.c | 27 +++++++++++++++++--
> >  1 file changed, 25 insertions(+), 2 deletions(-)
> > 
> 
> One of the feature requests from the FRR team (and a feature I have
> implemented) is a blackhole nexthop. The idea is that prefixes are
> installed pointing to nexthop id N. That nexthop definition can be
> atomically updated to go between a device / gateway and a blackhole.
> 
> 
>  [ prefix ] --> [ nhid 1 ] --> [ dev1 / gateway1 ]
> 
> 
>  [ prefix ] --> [ nhid 1 ] --> [ blackhole ]
> 
> 
>  [ prefix ] --> [ nhid 1 ] --> [ dev2 / gateway2 ]
> 
> Do you see this working ok with mlxsw without having to update the
> prefix entries (which can be numerous) directly?

Yes. This patch configures the route itself to drop packets, but we can
instead configure it as a remote route and configure the adjacency entry
to drop packets.

If you later want to change X routes using this blackhole nexthop to a
different one, then create the new one and tell the hardware to do the
switch in a single operation. It will basically grep over all configured
routes and do:

s/blackhole_adjacency_index/new_adjacency_index/
s/black_ecmp_size/new_ecmp_size/

See RALEU in drivers/net/ethernet/mellanox/mlxsw/reg.h

I assume that user can't put blackhole and normal nexthops in the same
group?

Re: [PATCH net-next 1/2] mlxsw: spectrum_router: Offload blackhole routes

[David Ahern]

On 2/7/19 11:34 PM, Ido Schimmel wrote:
> 
> Yes. This patch configures the route itself to drop packets, but we can
> instead configure it as a remote route and configure the adjacency entry
> to drop packets.
> 
> If you later want to change X routes using this blackhole nexthop to a
> different one, then create the new one and tell the hardware to do the
> switch in a single operation. It will basically grep over all configured
> routes and do:
> 
> s/blackhole_adjacency_index/new_adjacency_index/
> s/black_ecmp_size/new_ecmp_size/
> 
> See RALEU in drivers/net/ethernet/mellanox/mlxsw/reg.h

Thanks for the reference.

> 
> I assume that user can't put blackhole and normal nexthops in the same
> group?
> 

I allow a nexthop group to reference a nexthop that is a blackhole, but
the group can only contain the one entry. That allows multipath routes
to toggle between a blackhole and a real spec.

Re: [PATCH net-next 1/2] mlxsw: spectrum_router: Offload blackhole routes

[Ido Schimmel]

On Fri, Feb 08, 2019 at 08:24:49AM -0800, David Ahern wrote:
> On 2/7/19 11:34 PM, Ido Schimmel wrote:
> > I assume that user can't put blackhole and normal nexthops in the same
> > group?
> > 
> 
> I allow a nexthop group to reference a nexthop that is a blackhole, but
> the group can only contain the one entry. That allows multipath routes
> to toggle between a blackhole and a real spec.

Good, thanks. I was afraid users will be able to configure a nexthop
group that will randomly drop flows :)

[PATCH net-next 2/2] selftests: mlxsw: Add a test for blackhole routes

[Ido Schimmel]

Use a simple topology consisting of two hosts directly connected to a
router. Make sure IPv4/IPv6 ping works and then add blackhole routes.
Test that ping fails and that the routes are marked as offloaded. Use a
simple tc filter to test that packets were dropped by the ASIC and not
trapped to the CPU.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../drivers/net/mlxsw/blackhole_routes.sh     | 200 ++++++++++++++++++
 1 file changed, 200 insertions(+)
 create mode 100755 tools/testing/selftests/drivers/net/mlxsw/blackhole_routes.sh

diff --git a/tools/testing/selftests/drivers/net/mlxsw/blackhole_routes.sh b/tools/testing/selftests/drivers/net/mlxsw/blackhole_routes.sh
new file mode 100755
index 000000000000..5ba5bef44d5b
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/mlxsw/blackhole_routes.sh
@@ -0,0 +1,200 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Test that blackhole routes are marked as offloaded and that packets hitting
+# them are dropped by the ASIC and not by the kernel.
+#
+# +---------------------------------+
+# | H1 (vrf)                        |
+# |    + $h1                        |
+# |    | 192.0.2.1/24               |
+# |    | 2001:db8:1::1/64           |
+# |    |                            |
+# |    |  default via 192.0.2.2     |
+# |    |  default via 2001:db8:1::2 |
+# +----|----------------------------+
+#      |
+# +----|----------------------------------------------------------------------+
+# | SW |                                                                      |
+# |    + $rp1                                                                 |
+# |        192.0.2.2/24                                                       |
+# |        2001:db8:1::2/64                                                   |
+# |                                                                           |
+# |        2001:db8:2::2/64                                                   |
+# |        198.51.100.2/24                                                    |
+# |    + $rp2                                                                 |
+# |    |                                                                      |
+# +----|----------------------------------------------------------------------+
+#      |
+# +----|----------------------------+
+# |    |  default via 198.51.100.2  |
+# |    |  default via 2001:db8:2::2 |
+# |    |                            |
+# |    | 2001:db8:2::1/64           |
+# |    | 198.51.100.1/24            |
+# |    + $h2                        |
+# | H2 (vrf)                        |
+# +---------------------------------+
+
+lib_dir=$(dirname $0)/../../../net/forwarding
+
+ALL_TESTS="
+	ping_ipv4
+	ping_ipv6
+	blackhole_ipv4
+	blackhole_ipv6
+"
+NUM_NETIFS=4
+source $lib_dir/tc_common.sh
+source $lib_dir/lib.sh
+
+h1_create()
+{
+	simple_if_init $h1 192.0.2.1/24 2001:db8:1::1/64
+
+	ip -4 route add default vrf v$h1 nexthop via 192.0.2.2
+	ip -6 route add default vrf v$h1 nexthop via 2001:db8:1::2
+}
+
+h1_destroy()
+{
+	ip -6 route del default vrf v$h1 nexthop via 2001:db8:1::2
+	ip -4 route del default vrf v$h1 nexthop via 192.0.2.2
+
+	simple_if_fini $h1 192.0.2.1/24 2001:db8:1::1/64
+}
+
+h2_create()
+{
+	simple_if_init $h2 198.51.100.1/24 2001:db8:2::1/64
+
+	ip -4 route add default vrf v$h2 nexthop via 198.51.100.2
+	ip -6 route add default vrf v$h2 nexthop via 2001:db8:2::2
+}
+
+h2_destroy()
+{
+	ip -6 route del default vrf v$h2 nexthop via 2001:db8:2::2
+	ip -4 route del default vrf v$h2 nexthop via 198.51.100.2
+
+	simple_if_fini $h2 198.51.100.1/24 2001:db8:2::1/64
+}
+
+router_create()
+{
+	ip link set dev $rp1 up
+	ip link set dev $rp2 up
+
+	tc qdisc add dev $rp1 clsact
+
+	__addr_add_del $rp1 add 192.0.2.2/24 2001:db8:1::2/64
+	__addr_add_del $rp2 add 198.51.100.2/24 2001:db8:2::2/64
+}
+
+router_destroy()
+{
+	__addr_add_del $rp2 del 198.51.100.2/24 2001:db8:2::2/64
+	__addr_add_del $rp1 del 192.0.2.2/24 2001:db8:1::2/64
+
+	tc qdisc del dev $rp1 clsact
+
+	ip link set dev $rp2 down
+	ip link set dev $rp1 down
+}
+
+ping_ipv4()
+{
+	ping_test $h1 198.51.100.1 ": h1->h2"
+}
+
+ping_ipv6()
+{
+	ping6_test $h1 2001:db8:2::1 ": h1->h2"
+}
+
+blackhole_ipv4()
+{
+	# Transmit packets from H1 to H2 and make sure they are dropped by the
+	# ASIC and not by the kernel
+	RET=0
+
+	ip -4 route add blackhole 198.51.100.0/30
+	tc filter add dev $rp1 ingress protocol ip pref 1 handle 101 flower \
+		skip_hw dst_ip 198.51.100.1 src_ip 192.0.2.1 ip_proto icmp \
+		action pass
+
+	ip -4 route show 198.51.100.0/30 | grep -q offload
+	check_err $? "route not marked as offloaded when should"
+
+	ping_do $h1 198.51.100.1
+	check_fail $? "ping passed when should not"
+
+	tc_check_packets "dev $rp1 ingress" 101 0
+	check_err $? "packets trapped and not dropped by ASIC"
+
+	log_test "IPv4 blackhole route"
+
+	tc filter del dev $rp1 ingress protocol ip pref 1 handle 101 flower
+	ip -4 route del blackhole 198.51.100.0/30
+}
+
+blackhole_ipv6()
+{
+	RET=0
+
+	ip -6 route add blackhole 2001:db8:2::/120
+	tc filter add dev $rp1 ingress protocol ipv6 pref 1 handle 101 flower \
+		skip_hw dst_ip 2001:db8:2::1 src_ip 2001:db8:1::1 \
+		ip_proto icmpv6 action pass
+
+	ip -6 route show 2001:db8:2::/120 | grep -q offload
+	check_err $? "route not marked as offloaded when should"
+
+	ping6_do $h1 2001:db8:2::1
+	check_fail $? "ping passed when should not"
+
+	tc_check_packets "dev $rp1 ingress" 101 0
+	check_err $? "packets trapped and not dropped by ASIC"
+
+	log_test "IPv6 blackhole route"
+
+	tc filter del dev $rp1 ingress protocol ipv6 pref 1 handle 101 flower
+	ip -6 route del blackhole 2001:db8:2::/120
+}
+
+setup_prepare()
+{
+	h1=${NETIFS[p1]}
+	rp1=${NETIFS[p2]}
+
+	rp2=${NETIFS[p3]}
+	h2=${NETIFS[p4]}
+
+	vrf_prepare
+	forwarding_enable
+
+	h1_create
+	h2_create
+	router_create
+}
+
+cleanup()
+{
+	pre_cleanup
+
+	router_destroy
+	h2_destroy
+	h1_destroy
+
+	forwarding_restore
+	vrf_cleanup
+}
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+exit $EXIT_STATUS
-- 
2.20.1