netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch net 0/2] mlxsw: Fixes in GRE offloading
@ 2017-10-02 10:14 Jiri Pirko
  2017-10-02 10:14 ` [patch net 1/2] mlxsw: spectrum_router: Move VRF refcounting Jiri Pirko
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jiri Pirko @ 2017-10-02 10:14 UTC (permalink / raw)
  To: netdev; +Cc: davem, petrm, idosch, mlxsw

From: Jiri Pirko <jiri@mellanox.com>

Petr says:

This patchset fixes a couple unrelated problems in offloading IP-in-IP tunnels
in mlxsw driver.

- The first patch fixes a potential reference-counting problem that might lead
  to a kernel crash.

- The second patch associates IPIP next hops with their loopback RIFs. Besides
  being the right thing to do, it also fixes a problem where offloaded IPv6
  routes that forward to IP-in-IP netdevices were not flagged as such.

Petr Machata (2):
  mlxsw: spectrum_router: Move VRF refcounting
  mlxsw: spectrum_router: Track RIF of IPIP next hops

 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

-- 
2.9.5

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [patch net 1/2] mlxsw: spectrum_router: Move VRF refcounting
  2017-10-02 10:14 [patch net 0/2] mlxsw: Fixes in GRE offloading Jiri Pirko
@ 2017-10-02 10:14 ` Jiri Pirko
  2017-10-02 10:14 ` [patch net 2/2] mlxsw: spectrum_router: Track RIF of IPIP next hops Jiri Pirko
  2017-10-02 18:19 ` [patch net 0/2] mlxsw: Fixes in GRE offloading David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Jiri Pirko @ 2017-10-02 10:14 UTC (permalink / raw)
  To: netdev; +Cc: davem, petrm, idosch, mlxsw

From: Petr Machata <petrm@mellanox.com>

When creating a new RIF, bumping RIF count of the containing VR is the
last thing to be done. Symmetrically, when destroying a RIF, RIF count
is first dropped and only then the rest of the cleanup proceeds.

That's a problem for loopback RIFs. Those hold two VR references: one
for overlay and one for underlay. mlxsw_sp_rif_destroy() releases the
overlay one, and the deconfigure() callback the underlay one. But if
both overlay and underlay are the same, and if there are no other
artifacts holding the VR alive, this put actually destroys the VR. Later
on, when mlxsw_sp_rif_destroy() calls mlxsw_sp_vr_put() for the same VR,
the VR will already have been released and the kernel crashes with NULL
pointer dereference.

The underlying problem is that the RIF under destruction ends up
referencing the overlay VR much longer than it claims: all the way until
the call to mlxsw_sp_vr_put(). So line up the reference counting
properly to reflect this. Make corresponding changes in
mlxsw_sp_rif_create() as well for symmetry.

Fixes: 6ddb7426a7d4 ("mlxsw: spectrum_router: Introduce loopback RIFs")
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 2cfb3f5..3917b4d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -5068,6 +5068,7 @@ mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
 	vr = mlxsw_sp_vr_get(mlxsw_sp, tb_id ? : RT_TABLE_MAIN);
 	if (IS_ERR(vr))
 		return ERR_CAST(vr);
+	vr->rif_count++;
 
 	err = mlxsw_sp_rif_index_alloc(mlxsw_sp, &rif_index);
 	if (err)
@@ -5099,7 +5100,6 @@ mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
 
 	mlxsw_sp_rif_counters_alloc(rif);
 	mlxsw_sp->router->rifs[rif_index] = rif;
-	vr->rif_count++;
 
 	return rif;
 
@@ -5110,6 +5110,7 @@ mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
 	kfree(rif);
 err_rif_alloc:
 err_rif_index_alloc:
+	vr->rif_count--;
 	mlxsw_sp_vr_put(vr);
 	return ERR_PTR(err);
 }
@@ -5124,7 +5125,6 @@ void mlxsw_sp_rif_destroy(struct mlxsw_sp_rif *rif)
 	mlxsw_sp_router_rif_gone_sync(mlxsw_sp, rif);
 	vr = &mlxsw_sp->router->vrs[rif->vr_id];
 
-	vr->rif_count--;
 	mlxsw_sp->router->rifs[rif->rif_index] = NULL;
 	mlxsw_sp_rif_counters_free(rif);
 	ops->deconfigure(rif);
@@ -5132,6 +5132,7 @@ void mlxsw_sp_rif_destroy(struct mlxsw_sp_rif *rif)
 		/* Loopback RIFs are not associated with a FID. */
 		mlxsw_sp_fid_put(fid);
 	kfree(rif);
+	vr->rif_count--;
 	mlxsw_sp_vr_put(vr);
 }
 
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [patch net 2/2] mlxsw: spectrum_router: Track RIF of IPIP next hops
  2017-10-02 10:14 [patch net 0/2] mlxsw: Fixes in GRE offloading Jiri Pirko
  2017-10-02 10:14 ` [patch net 1/2] mlxsw: spectrum_router: Move VRF refcounting Jiri Pirko
@ 2017-10-02 10:14 ` Jiri Pirko
  2017-10-02 18:19 ` [patch net 0/2] mlxsw: Fixes in GRE offloading David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Jiri Pirko @ 2017-10-02 10:14 UTC (permalink / raw)
  To: netdev; +Cc: davem, petrm, idosch, mlxsw

From: Petr Machata <petrm@mellanox.com>

When considering whether to set RTNH_F_OFFLOAD flag on an IPv6 route,
mlxsw_sp_fib6_entry_offload_set() looks up the mlxsw_sp_nexthop
corresponding to a given route, and decides based on whether the next
hop's offloaded flag was set. When looking for the matching next hop, it
also takes into account the device of the route, which must match next
hop's RIF.

IPIP next hops however hitherto didn't set the RIF. As a result, IPv6
routes forwarding traffic to IP-in-IP netdevices are never marked as
offloaded, even when they actually are.

Thus track RIF of IPIP next hops the same way as that of ETHERNET next
hops.

Fixes: 8f28a3097645 ("mlxsw: spectrum_router: Support IPv6 overlay encap")
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 3917b4d..032089e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -2723,6 +2723,7 @@ static void mlxsw_sp_nexthop_type_fini(struct mlxsw_sp *mlxsw_sp,
 		mlxsw_sp_nexthop_rif_fini(nh);
 		break;
 	case MLXSW_SP_NEXTHOP_TYPE_IPIP:
+		mlxsw_sp_nexthop_rif_fini(nh);
 		mlxsw_sp_nexthop_ipip_fini(mlxsw_sp, nh);
 		break;
 	}
@@ -2742,7 +2743,11 @@ static int mlxsw_sp_nexthop4_type_init(struct mlxsw_sp *mlxsw_sp,
 	    router->ipip_ops_arr[ipipt]->can_offload(mlxsw_sp, dev,
 						     MLXSW_SP_L3_PROTO_IPV4)) {
 		nh->type = MLXSW_SP_NEXTHOP_TYPE_IPIP;
-		return mlxsw_sp_nexthop_ipip_init(mlxsw_sp, ipipt, nh, dev);
+		err = mlxsw_sp_nexthop_ipip_init(mlxsw_sp, ipipt, nh, dev);
+		if (err)
+			return err;
+		mlxsw_sp_nexthop_rif_init(nh, &nh->ipip_entry->ol_lb->common);
+		return 0;
 	}
 
 	nh->type = MLXSW_SP_NEXTHOP_TYPE_ETH;
@@ -4009,7 +4014,11 @@ static int mlxsw_sp_nexthop6_type_init(struct mlxsw_sp *mlxsw_sp,
 	    router->ipip_ops_arr[ipipt]->can_offload(mlxsw_sp, dev,
 						     MLXSW_SP_L3_PROTO_IPV6)) {
 		nh->type = MLXSW_SP_NEXTHOP_TYPE_IPIP;
-		return mlxsw_sp_nexthop_ipip_init(mlxsw_sp, ipipt, nh, dev);
+		err = mlxsw_sp_nexthop_ipip_init(mlxsw_sp, ipipt, nh, dev);
+		if (err)
+			return err;
+		mlxsw_sp_nexthop_rif_init(nh, &nh->ipip_entry->ol_lb->common);
+		return 0;
 	}
 
 	nh->type = MLXSW_SP_NEXTHOP_TYPE_ETH;
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [patch net 0/2] mlxsw: Fixes in GRE offloading
  2017-10-02 10:14 [patch net 0/2] mlxsw: Fixes in GRE offloading Jiri Pirko
  2017-10-02 10:14 ` [patch net 1/2] mlxsw: spectrum_router: Move VRF refcounting Jiri Pirko
  2017-10-02 10:14 ` [patch net 2/2] mlxsw: spectrum_router: Track RIF of IPIP next hops Jiri Pirko
@ 2017-10-02 18:19 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2017-10-02 18:19 UTC (permalink / raw)
  To: jiri; +Cc: netdev, petrm, idosch, mlxsw

From: Jiri Pirko <jiri@resnulli.us>
Date: Mon,  2 Oct 2017 12:14:55 +0200

> From: Jiri Pirko <jiri@mellanox.com>
> 
> Petr says:
> 
> This patchset fixes a couple unrelated problems in offloading IP-in-IP tunnels
> in mlxsw driver.
> 
> - The first patch fixes a potential reference-counting problem that might lead
>   to a kernel crash.
> 
> - The second patch associates IPIP next hops with their loopback RIFs. Besides
>   being the right thing to do, it also fixes a problem where offloaded IPv6
>   routes that forward to IP-in-IP netdevices were not flagged as such.

Series applied.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-10-02 18:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-02 10:14 [patch net 0/2] mlxsw: Fixes in GRE offloading Jiri Pirko
2017-10-02 10:14 ` [patch net 1/2] mlxsw: spectrum_router: Move VRF refcounting Jiri Pirko
2017-10-02 10:14 ` [patch net 2/2] mlxsw: spectrum_router: Track RIF of IPIP next hops Jiri Pirko
2017-10-02 18:19 ` [patch net 0/2] mlxsw: Fixes in GRE offloading David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).