All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 iwl-next] ice: allow hot-swapping XDP programs
@ 2023-06-15 11:33 ` Maciej Fijalkowski
  0 siblings, 0 replies; 8+ messages in thread
From: Maciej Fijalkowski @ 2023-06-15 11:33 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, anthony.l.nguyen, magnus.karlsson, fred, toke,
	aleksander.lobakin, Maciej Fijalkowski

Currently ice driver's .ndo_bpf callback brings interface down and up
independently of XDP resources' presence. This is only needed when
either these resources have to be configured or removed. It means that
if one is switching XDP programs on-the-fly with running traffic,
packets will be dropped.

To avoid this, compare early on ice_xdp_setup_prog() state of incoming
bpf_prog pointer vs the bpf_prog pointer that is already assigned to
VSI. Do the swap in case VSI has bpf_prog and incoming one are non-NULL.

Lastly, while at it, put old bpf_prog *after* the update of Rx ring's
bpf_prog pointer. In theory previous code could expose us to a state
where Rx ring's bpf_prog would still be referring to old_prog that got
released with earlier bpf_prog_put().

Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---

v2->v3:
- move bpf_prog_put() after ice_rx_ring::xdp_prog update [Toke, Olek]
v1->v2:
- fix missing brace (sigh)

 drivers/net/ethernet/intel/ice/ice_main.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 62e91512aeab..a7c76fded603 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -2633,11 +2633,11 @@ static void ice_vsi_assign_bpf_prog(struct ice_vsi *vsi, struct bpf_prog *prog)
 	int i;
 
 	old_prog = xchg(&vsi->xdp_prog, prog);
-	if (old_prog)
-		bpf_prog_put(old_prog);
-
 	ice_for_each_rxq(vsi, i)
 		WRITE_ONCE(vsi->rx_rings[i]->xdp_prog, vsi->xdp_prog);
+
+	if (old_prog)
+		bpf_prog_put(old_prog);
 }
 
 /**
@@ -2922,6 +2922,12 @@ ice_xdp_setup_prog(struct ice_vsi *vsi, struct bpf_prog *prog,
 		}
 	}
 
+	/* hot swap progs and avoid toggling link */
+	if (ice_is_xdp_ena_vsi(vsi) == !!prog) {
+		ice_vsi_assign_bpf_prog(vsi, prog);
+		return 0;
+	}
+
 	/* need to stop netdev while setting up the program for Rx rings */
 	if (if_running && !test_and_set_bit(ICE_VSI_DOWN, vsi->state)) {
 		ret = ice_down(vsi);
@@ -2954,13 +2960,6 @@ ice_xdp_setup_prog(struct ice_vsi *vsi, struct bpf_prog *prog,
 		xdp_ring_err = ice_realloc_zc_buf(vsi, false);
 		if (xdp_ring_err)
 			NL_SET_ERR_MSG_MOD(extack, "Freeing XDP Rx resources failed");
-	} else {
-		/* safe to call even when prog == vsi->xdp_prog as
-		 * dev_xdp_install in net/core/dev.c incremented prog's
-		 * refcount so corresponding bpf_prog_put won't cause
-		 * underflow
-		 */
-		ice_vsi_assign_bpf_prog(vsi, prog);
 	}
 
 	if (if_running)
-- 
2.34.1


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

* [Intel-wired-lan] [PATCH v3 iwl-next] ice: allow hot-swapping XDP programs
@ 2023-06-15 11:33 ` Maciej Fijalkowski
  0 siblings, 0 replies; 8+ messages in thread
From: Maciej Fijalkowski @ 2023-06-15 11:33 UTC (permalink / raw)
  To: intel-wired-lan; +Cc: toke, netdev, anthony.l.nguyen, magnus.karlsson, fred

Currently ice driver's .ndo_bpf callback brings interface down and up
independently of XDP resources' presence. This is only needed when
either these resources have to be configured or removed. It means that
if one is switching XDP programs on-the-fly with running traffic,
packets will be dropped.

To avoid this, compare early on ice_xdp_setup_prog() state of incoming
bpf_prog pointer vs the bpf_prog pointer that is already assigned to
VSI. Do the swap in case VSI has bpf_prog and incoming one are non-NULL.

Lastly, while at it, put old bpf_prog *after* the update of Rx ring's
bpf_prog pointer. In theory previous code could expose us to a state
where Rx ring's bpf_prog would still be referring to old_prog that got
released with earlier bpf_prog_put().

Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---

v2->v3:
- move bpf_prog_put() after ice_rx_ring::xdp_prog update [Toke, Olek]
v1->v2:
- fix missing brace (sigh)

 drivers/net/ethernet/intel/ice/ice_main.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 62e91512aeab..a7c76fded603 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -2633,11 +2633,11 @@ static void ice_vsi_assign_bpf_prog(struct ice_vsi *vsi, struct bpf_prog *prog)
 	int i;
 
 	old_prog = xchg(&vsi->xdp_prog, prog);
-	if (old_prog)
-		bpf_prog_put(old_prog);
-
 	ice_for_each_rxq(vsi, i)
 		WRITE_ONCE(vsi->rx_rings[i]->xdp_prog, vsi->xdp_prog);
+
+	if (old_prog)
+		bpf_prog_put(old_prog);
 }
 
 /**
@@ -2922,6 +2922,12 @@ ice_xdp_setup_prog(struct ice_vsi *vsi, struct bpf_prog *prog,
 		}
 	}
 
+	/* hot swap progs and avoid toggling link */
+	if (ice_is_xdp_ena_vsi(vsi) == !!prog) {
+		ice_vsi_assign_bpf_prog(vsi, prog);
+		return 0;
+	}
+
 	/* need to stop netdev while setting up the program for Rx rings */
 	if (if_running && !test_and_set_bit(ICE_VSI_DOWN, vsi->state)) {
 		ret = ice_down(vsi);
@@ -2954,13 +2960,6 @@ ice_xdp_setup_prog(struct ice_vsi *vsi, struct bpf_prog *prog,
 		xdp_ring_err = ice_realloc_zc_buf(vsi, false);
 		if (xdp_ring_err)
 			NL_SET_ERR_MSG_MOD(extack, "Freeing XDP Rx resources failed");
-	} else {
-		/* safe to call even when prog == vsi->xdp_prog as
-		 * dev_xdp_install in net/core/dev.c incremented prog's
-		 * refcount so corresponding bpf_prog_put won't cause
-		 * underflow
-		 */
-		ice_vsi_assign_bpf_prog(vsi, prog);
 	}
 
 	if (if_running)
-- 
2.34.1

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

* Re: [PATCH v3 iwl-next] ice: allow hot-swapping XDP programs
  2023-06-15 11:33 ` [Intel-wired-lan] " Maciej Fijalkowski
@ 2023-06-15 12:29   ` Toke Høiland-Jørgensen
  -1 siblings, 0 replies; 8+ messages in thread
From: Toke Høiland-Jørgensen @ 2023-06-15 12:29 UTC (permalink / raw)
  To: Maciej Fijalkowski, intel-wired-lan
  Cc: netdev, anthony.l.nguyen, magnus.karlsson, fred,
	aleksander.lobakin, Maciej Fijalkowski

Maciej Fijalkowski <maciej.fijalkowski@intel.com> writes:

> Currently ice driver's .ndo_bpf callback brings interface down and up
> independently of XDP resources' presence. This is only needed when
> either these resources have to be configured or removed. It means that
> if one is switching XDP programs on-the-fly with running traffic,
> packets will be dropped.
>
> To avoid this, compare early on ice_xdp_setup_prog() state of incoming
> bpf_prog pointer vs the bpf_prog pointer that is already assigned to
> VSI. Do the swap in case VSI has bpf_prog and incoming one are non-NULL.
>
> Lastly, while at it, put old bpf_prog *after* the update of Rx ring's
> bpf_prog pointer. In theory previous code could expose us to a state
> where Rx ring's bpf_prog would still be referring to old_prog that got
> released with earlier bpf_prog_put().
>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>


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

* Re: [Intel-wired-lan] [PATCH v3 iwl-next] ice: allow hot-swapping XDP programs
@ 2023-06-15 12:29   ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 8+ messages in thread
From: Toke Høiland-Jørgensen @ 2023-06-15 12:29 UTC (permalink / raw)
  To: Maciej Fijalkowski, intel-wired-lan
  Cc: netdev, anthony.l.nguyen, magnus.karlsson, fred

Maciej Fijalkowski <maciej.fijalkowski@intel.com> writes:

> Currently ice driver's .ndo_bpf callback brings interface down and up
> independently of XDP resources' presence. This is only needed when
> either these resources have to be configured or removed. It means that
> if one is switching XDP programs on-the-fly with running traffic,
> packets will be dropped.
>
> To avoid this, compare early on ice_xdp_setup_prog() state of incoming
> bpf_prog pointer vs the bpf_prog pointer that is already assigned to
> VSI. Do the swap in case VSI has bpf_prog and incoming one are non-NULL.
>
> Lastly, while at it, put old bpf_prog *after* the update of Rx ring's
> bpf_prog pointer. In theory previous code could expose us to a state
> where Rx ring's bpf_prog would still be referring to old_prog that got
> released with earlier bpf_prog_put().
>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

* Re: [Intel-wired-lan] [PATCH v3 iwl-next] ice: allow hot-swapping XDP programs
  2023-06-15 11:33 ` [Intel-wired-lan] " Maciej Fijalkowski
@ 2023-06-16 14:05   ` Alexander Lobakin
  -1 siblings, 0 replies; 8+ messages in thread
From: Alexander Lobakin @ 2023-06-16 14:05 UTC (permalink / raw)
  To: Maciej Fijalkowski
  Cc: toke, netdev, anthony.l.nguyen, magnus.karlsson, intel-wired-lan, fred

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Date: Thu, 15 Jun 2023 13:33:26 +0200

> Currently ice driver's .ndo_bpf callback brings interface down and up
> independently of XDP resources' presence. This is only needed when
> either these resources have to be configured or removed. It means that
> if one is switching XDP programs on-the-fly with running traffic,
> packets will be dropped.
> 
> To avoid this, compare early on ice_xdp_setup_prog() state of incoming
> bpf_prog pointer vs the bpf_prog pointer that is already assigned to
> VSI. Do the swap in case VSI has bpf_prog and incoming one are non-NULL.
> 
> Lastly, while at it, put old bpf_prog *after* the update of Rx ring's
> bpf_prog pointer. In theory previous code could expose us to a state
> where Rx ring's bpf_prog would still be referring to old_prog that got
> released with earlier bpf_prog_put().
> 
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>

Good stuff, much missing previously.

[...]

Thanks,
Olek
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

* Re: [PATCH v3 iwl-next] ice: allow hot-swapping XDP programs
@ 2023-06-16 14:05   ` Alexander Lobakin
  0 siblings, 0 replies; 8+ messages in thread
From: Alexander Lobakin @ 2023-06-16 14:05 UTC (permalink / raw)
  To: Maciej Fijalkowski
  Cc: intel-wired-lan, netdev, anthony.l.nguyen, magnus.karlsson, fred, toke

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Date: Thu, 15 Jun 2023 13:33:26 +0200

> Currently ice driver's .ndo_bpf callback brings interface down and up
> independently of XDP resources' presence. This is only needed when
> either these resources have to be configured or removed. It means that
> if one is switching XDP programs on-the-fly with running traffic,
> packets will be dropped.
> 
> To avoid this, compare early on ice_xdp_setup_prog() state of incoming
> bpf_prog pointer vs the bpf_prog pointer that is already assigned to
> VSI. Do the swap in case VSI has bpf_prog and incoming one are non-NULL.
> 
> Lastly, while at it, put old bpf_prog *after* the update of Rx ring's
> bpf_prog pointer. In theory previous code could expose us to a state
> where Rx ring's bpf_prog would still be referring to old_prog that got
> released with earlier bpf_prog_put().
> 
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>

Good stuff, much missing previously.

[...]

Thanks,
Olek

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

* RE: [Intel-wired-lan] [PATCH v3 iwl-next] ice: allow hot-swapping XDP programs
  2023-06-15 11:33 ` [Intel-wired-lan] " Maciej Fijalkowski
@ 2023-06-21 12:16   ` Rout, ChandanX
  -1 siblings, 0 replies; 8+ messages in thread
From: Rout, ChandanX @ 2023-06-21 12:16 UTC (permalink / raw)
  To: Fijalkowski, Maciej, intel-wired-lan
  Cc: toke, netdev, Nguyen, Anthony L, Karlsson, Magnus, fred,
	Kuruvinakunnel, George, Nagaraju, Shwetha, Nagraj, Shravan



>-----Original Message-----
>From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
>Fijalkowski, Maciej
>Sent: 15 June 2023 17:03
>To: intel-wired-lan@lists.osuosl.org
>Cc: toke@kernel.org; netdev@vger.kernel.org; Nguyen, Anthony L
><anthony.l.nguyen@intel.com>; Karlsson, Magnus
><magnus.karlsson@intel.com>; fred@cloudflare.com
>Subject: [Intel-wired-lan] [PATCH v3 iwl-next] ice: allow hot-swapping XDP
>programs
>
>Currently ice driver's .ndo_bpf callback brings interface down and up
>independently of XDP resources' presence. This is only needed when either
>these resources have to be configured or removed. It means that if one is
>switching XDP programs on-the-fly with running traffic, packets will be
>dropped.
>
>To avoid this, compare early on ice_xdp_setup_prog() state of incoming
>bpf_prog pointer vs the bpf_prog pointer that is already assigned to VSI. Do
>the swap in case VSI has bpf_prog and incoming one are non-NULL.
>
>Lastly, while at it, put old bpf_prog *after* the update of Rx ring's bpf_prog
>pointer. In theory previous code could expose us to a state where Rx ring's
>bpf_prog would still be referring to old_prog that got released with earlier
>bpf_prog_put().
>
>Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>---
>
>v2->v3:
>- move bpf_prog_put() after ice_rx_ring::xdp_prog update [Toke, Olek]
>v1->v2:
>- fix missing brace (sigh)
>
> drivers/net/ethernet/intel/ice/ice_main.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>

Tested-by: Chandan Kumar Rout <chandanx.rout@intel.com> (A Contingent Worker at Intel)

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

* Re: [Intel-wired-lan] [PATCH v3 iwl-next] ice: allow hot-swapping XDP programs
@ 2023-06-21 12:16   ` Rout, ChandanX
  0 siblings, 0 replies; 8+ messages in thread
From: Rout, ChandanX @ 2023-06-21 12:16 UTC (permalink / raw)
  To: Fijalkowski, Maciej, intel-wired-lan
  Cc: toke, netdev, Nguyen, Anthony L, Karlsson, Magnus, Nagraj, Shravan, fred



>-----Original Message-----
>From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
>Fijalkowski, Maciej
>Sent: 15 June 2023 17:03
>To: intel-wired-lan@lists.osuosl.org
>Cc: toke@kernel.org; netdev@vger.kernel.org; Nguyen, Anthony L
><anthony.l.nguyen@intel.com>; Karlsson, Magnus
><magnus.karlsson@intel.com>; fred@cloudflare.com
>Subject: [Intel-wired-lan] [PATCH v3 iwl-next] ice: allow hot-swapping XDP
>programs
>
>Currently ice driver's .ndo_bpf callback brings interface down and up
>independently of XDP resources' presence. This is only needed when either
>these resources have to be configured or removed. It means that if one is
>switching XDP programs on-the-fly with running traffic, packets will be
>dropped.
>
>To avoid this, compare early on ice_xdp_setup_prog() state of incoming
>bpf_prog pointer vs the bpf_prog pointer that is already assigned to VSI. Do
>the swap in case VSI has bpf_prog and incoming one are non-NULL.
>
>Lastly, while at it, put old bpf_prog *after* the update of Rx ring's bpf_prog
>pointer. In theory previous code could expose us to a state where Rx ring's
>bpf_prog would still be referring to old_prog that got released with earlier
>bpf_prog_put().
>
>Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>---
>
>v2->v3:
>- move bpf_prog_put() after ice_rx_ring::xdp_prog update [Toke, Olek]
>v1->v2:
>- fix missing brace (sigh)
>
> drivers/net/ethernet/intel/ice/ice_main.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>

Tested-by: Chandan Kumar Rout <chandanx.rout@intel.com> (A Contingent Worker at Intel)
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

end of thread, other threads:[~2023-06-21 12:16 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-15 11:33 [PATCH v3 iwl-next] ice: allow hot-swapping XDP programs Maciej Fijalkowski
2023-06-15 11:33 ` [Intel-wired-lan] " Maciej Fijalkowski
2023-06-15 12:29 ` Toke Høiland-Jørgensen
2023-06-15 12:29   ` [Intel-wired-lan] " Toke Høiland-Jørgensen
2023-06-16 14:05 ` Alexander Lobakin
2023-06-16 14:05   ` Alexander Lobakin
2023-06-21 12:16 ` [Intel-wired-lan] " Rout, ChandanX
2023-06-21 12:16   ` Rout, ChandanX

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.