From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To: Larysa Zaremba <larysa.zaremba@intel.com>
Cc: <intel-wired-lan@lists.osuosl.org>,
Tony Nguyen <anthony.l.nguyen@intel.com>,
"David S. Miller" <davem@davemloft.net>,
"Jacob Keller" <jacob.e.keller@intel.com>,
Eric Dumazet <edumazet@google.com>,
"Jakub Kicinski" <kuba@kernel.org>,
Paolo Abeni <pabeni@redhat.com>,
"Alexei Starovoitov" <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
"Jesper Dangaard Brouer" <hawk@kernel.org>,
John Fastabend <john.fastabend@gmail.com>,
<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<bpf@vger.kernel.org>, <magnus.karlsson@intel.com>,
Michal Kubiak <michal.kubiak@intel.com>,
Wojciech Drewek <wojciech.drewek@intel.com>,
Amritha Nambiar <amritha.nambiar@intel.com>
Subject: Re: [PATCH iwl-net v2 3/6] ice: check for XDP rings instead of bpf program when unconfiguring
Date: Mon, 12 Aug 2024 14:58:00 +0200 [thread overview]
Message-ID: <ZroG2LxHn2Rt+Txx@boxer> (raw)
In-Reply-To: <20240724164840.2536605-4-larysa.zaremba@intel.com>
On Wed, Jul 24, 2024 at 06:48:34PM +0200, Larysa Zaremba wrote:
> If VSI rebuild is pending, .ndo_bpf() can attach/detach the XDP program on
> VSI without applying new ring configuration. When unconfiguring the VSI, we
> can encounter the state in which there is an XDP program but no XDP rings
> to destroy or there will be XDP rings that need to be destroyed, but no XDP
> program to indicate their presence.
>
> When unconfiguring, rely on the presence of XDP rings rather then XDP
> program, as they better represent the current state that has to be
> destroyed.
>
No Fixes: tag?
> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_lib.c | 4 ++--
> drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
> drivers/net/ethernet/intel/ice/ice_xsk.c | 6 +++---
> 3 files changed, 7 insertions(+), 7 deletions(-)
[...]
> diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
> index 2c1a843ba200..5dd50a2866cc 100644
> --- a/drivers/net/ethernet/intel/ice/ice_xsk.c
> +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
> @@ -39,7 +39,7 @@ static void ice_qp_reset_stats(struct ice_vsi *vsi, u16 q_idx)
> sizeof(vsi_stat->rx_ring_stats[q_idx]->rx_stats));
> memset(&vsi_stat->tx_ring_stats[q_idx]->stats, 0,
> sizeof(vsi_stat->tx_ring_stats[q_idx]->stats));
> - if (ice_is_xdp_ena_vsi(vsi))
> + if (vsi->xdp_rings)
> memset(&vsi->xdp_rings[q_idx]->ring_stats->stats, 0,
> sizeof(vsi->xdp_rings[q_idx]->ring_stats->stats));
> }
> @@ -52,7 +52,7 @@ static void ice_qp_reset_stats(struct ice_vsi *vsi, u16 q_idx)
> static void ice_qp_clean_rings(struct ice_vsi *vsi, u16 q_idx)
> {
> ice_clean_tx_ring(vsi->tx_rings[q_idx]);
> - if (ice_is_xdp_ena_vsi(vsi)) {
> + if (vsi->xdp_rings) {
> synchronize_rcu();
> ice_clean_tx_ring(vsi->xdp_rings[q_idx]);
> }
> @@ -189,7 +189,7 @@ static int ice_qp_dis(struct ice_vsi *vsi, u16 q_idx)
> err = ice_vsi_stop_tx_ring(vsi, ICE_NO_RESET, 0, tx_ring, &txq_meta);
> if (err)
> return err;
> - if (ice_is_xdp_ena_vsi(vsi)) {
> + if (vsi->xdp_rings) {
From XSK POV these checks are false positive and I will be sending a patch
that gets rid of it (I had this on my tree when working on timeout issues
but I pulled this out as it was not a -net candidate IMHO).
Just a heads up, this can go as-is currently.
> struct ice_tx_ring *xdp_ring = vsi->xdp_rings[q_idx];
>
> memset(&txq_meta, 0, sizeof(txq_meta));
> --
> 2.43.0
>
WARNING: multiple messages have this Message-ID (diff)
From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To: Larysa Zaremba <larysa.zaremba@intel.com>
Cc: Wojciech Drewek <wojciech.drewek@intel.com>,
Jesper Dangaard Brouer <hawk@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
netdev@vger.kernel.org,
Amritha Nambiar <amritha.nambiar@intel.com>,
John Fastabend <john.fastabend@gmail.com>,
Alexei Starovoitov <ast@kernel.org>,
linux-kernel@vger.kernel.org, Eric Dumazet <edumazet@google.com>,
Michal Kubiak <michal.kubiak@intel.com>,
Tony Nguyen <anthony.l.nguyen@intel.com>,
Jakub Kicinski <kuba@kernel.org>,
Jacob Keller <jacob.e.keller@intel.com>,
intel-wired-lan@lists.osuosl.org, bpf@vger.kernel.org,
Paolo Abeni <pabeni@redhat.com>,
"David S. Miller" <davem@davemloft.net>,
magnus.karlsson@intel.com
Subject: Re: [Intel-wired-lan] [PATCH iwl-net v2 3/6] ice: check for XDP rings instead of bpf program when unconfiguring
Date: Mon, 12 Aug 2024 14:58:00 +0200 [thread overview]
Message-ID: <ZroG2LxHn2Rt+Txx@boxer> (raw)
In-Reply-To: <20240724164840.2536605-4-larysa.zaremba@intel.com>
On Wed, Jul 24, 2024 at 06:48:34PM +0200, Larysa Zaremba wrote:
> If VSI rebuild is pending, .ndo_bpf() can attach/detach the XDP program on
> VSI without applying new ring configuration. When unconfiguring the VSI, we
> can encounter the state in which there is an XDP program but no XDP rings
> to destroy or there will be XDP rings that need to be destroyed, but no XDP
> program to indicate their presence.
>
> When unconfiguring, rely on the presence of XDP rings rather then XDP
> program, as they better represent the current state that has to be
> destroyed.
>
No Fixes: tag?
> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_lib.c | 4 ++--
> drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
> drivers/net/ethernet/intel/ice/ice_xsk.c | 6 +++---
> 3 files changed, 7 insertions(+), 7 deletions(-)
[...]
> diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
> index 2c1a843ba200..5dd50a2866cc 100644
> --- a/drivers/net/ethernet/intel/ice/ice_xsk.c
> +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
> @@ -39,7 +39,7 @@ static void ice_qp_reset_stats(struct ice_vsi *vsi, u16 q_idx)
> sizeof(vsi_stat->rx_ring_stats[q_idx]->rx_stats));
> memset(&vsi_stat->tx_ring_stats[q_idx]->stats, 0,
> sizeof(vsi_stat->tx_ring_stats[q_idx]->stats));
> - if (ice_is_xdp_ena_vsi(vsi))
> + if (vsi->xdp_rings)
> memset(&vsi->xdp_rings[q_idx]->ring_stats->stats, 0,
> sizeof(vsi->xdp_rings[q_idx]->ring_stats->stats));
> }
> @@ -52,7 +52,7 @@ static void ice_qp_reset_stats(struct ice_vsi *vsi, u16 q_idx)
> static void ice_qp_clean_rings(struct ice_vsi *vsi, u16 q_idx)
> {
> ice_clean_tx_ring(vsi->tx_rings[q_idx]);
> - if (ice_is_xdp_ena_vsi(vsi)) {
> + if (vsi->xdp_rings) {
> synchronize_rcu();
> ice_clean_tx_ring(vsi->xdp_rings[q_idx]);
> }
> @@ -189,7 +189,7 @@ static int ice_qp_dis(struct ice_vsi *vsi, u16 q_idx)
> err = ice_vsi_stop_tx_ring(vsi, ICE_NO_RESET, 0, tx_ring, &txq_meta);
> if (err)
> return err;
> - if (ice_is_xdp_ena_vsi(vsi)) {
> + if (vsi->xdp_rings) {
From XSK POV these checks are false positive and I will be sending a patch
that gets rid of it (I had this on my tree when working on timeout issues
but I pulled this out as it was not a -net candidate IMHO).
Just a heads up, this can go as-is currently.
> struct ice_tx_ring *xdp_ring = vsi->xdp_rings[q_idx];
>
> memset(&txq_meta, 0, sizeof(txq_meta));
> --
> 2.43.0
>
next prev parent reply other threads:[~2024-08-12 12:58 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-24 16:48 [PATCH iwl-net v2 0/6] ice: fix synchronization between .ndo_bpf() and reset Larysa Zaremba
2024-07-24 16:48 ` [Intel-wired-lan] " Larysa Zaremba
2024-07-24 16:48 ` [PATCH iwl-net v2 1/6] ice: move netif_queue_set_napi to rtnl-protected sections Larysa Zaremba
2024-07-24 16:48 ` [Intel-wired-lan] " Larysa Zaremba
2024-07-24 18:21 ` Jacob Keller
2024-07-24 18:21 ` [Intel-wired-lan] " Jacob Keller
2024-07-24 20:40 ` Nambiar, Amritha
2024-07-24 20:40 ` [Intel-wired-lan] " Nambiar, Amritha
2024-08-08 2:19 ` Rout, ChandanX
2024-08-08 2:19 ` Rout, ChandanX
2024-07-24 16:48 ` [PATCH iwl-net v2 2/6] ice: protect XDP configuration with a mutex Larysa Zaremba
2024-07-24 16:48 ` [Intel-wired-lan] " Larysa Zaremba
2024-07-24 18:24 ` Jacob Keller
2024-07-24 18:24 ` [Intel-wired-lan] " Jacob Keller
2024-08-08 2:16 ` Rout, ChandanX
2024-08-08 2:16 ` Rout, ChandanX
2024-08-13 11:31 ` Maciej Fijalkowski
2024-08-13 11:31 ` [Intel-wired-lan] " Maciej Fijalkowski
2024-08-13 13:36 ` Larysa Zaremba
2024-08-13 13:36 ` [Intel-wired-lan] " Larysa Zaremba
2024-07-24 16:48 ` [PATCH iwl-net v2 3/6] ice: check for XDP rings instead of bpf program when unconfiguring Larysa Zaremba
2024-07-24 16:48 ` [Intel-wired-lan] " Larysa Zaremba
2024-07-24 18:25 ` Jacob Keller
2024-07-24 18:25 ` [Intel-wired-lan] " Jacob Keller
2024-08-08 2:18 ` Rout, ChandanX
2024-08-08 2:18 ` Rout, ChandanX
2024-08-12 12:58 ` Maciej Fijalkowski [this message]
2024-08-12 12:58 ` Maciej Fijalkowski
2024-08-12 15:13 ` Larysa Zaremba
2024-08-12 15:13 ` [Intel-wired-lan] " Larysa Zaremba
2024-07-24 16:48 ` [PATCH iwl-net v2 4/6] ice: check ICE_VSI_DOWN under rtnl_lock when preparing for reset Larysa Zaremba
2024-07-24 16:48 ` [Intel-wired-lan] " Larysa Zaremba
2024-07-24 18:27 ` Jacob Keller
2024-07-24 18:27 ` [Intel-wired-lan] " Jacob Keller
2024-08-08 2:15 ` Rout, ChandanX
2024-08-08 2:15 ` Rout, ChandanX
2024-07-24 16:48 ` [PATCH iwl-net v2 5/6] ice: remove ICE_CFG_BUSY locking from AF_XDP code Larysa Zaremba
2024-07-24 16:48 ` [Intel-wired-lan] " Larysa Zaremba
2024-07-24 18:37 ` Jacob Keller
2024-07-24 18:37 ` [Intel-wired-lan] " Jacob Keller
2024-08-08 2:17 ` Rout, ChandanX
2024-08-08 2:17 ` Rout, ChandanX
2024-08-12 13:03 ` Maciej Fijalkowski
2024-08-12 13:03 ` [Intel-wired-lan] " Maciej Fijalkowski
2024-08-12 15:59 ` Larysa Zaremba
2024-08-12 15:59 ` [Intel-wired-lan] " Larysa Zaremba
2024-08-13 10:28 ` Maciej Fijalkowski
2024-08-13 10:28 ` [Intel-wired-lan] " Maciej Fijalkowski
2024-07-24 16:48 ` [PATCH iwl-net v2 6/6] ice: do not bring the VSI up, if it was down before the XDP setup Larysa Zaremba
2024-07-24 16:48 ` [Intel-wired-lan] " Larysa Zaremba
2024-07-24 18:40 ` Jacob Keller
2024-07-24 18:40 ` [Intel-wired-lan] " Jacob Keller
2024-08-08 2:14 ` Rout, ChandanX
2024-08-08 2:14 ` Rout, ChandanX
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZroG2LxHn2Rt+Txx@boxer \
--to=maciej.fijalkowski@intel.com \
--cc=amritha.nambiar@intel.com \
--cc=anthony.l.nguyen@intel.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hawk@kernel.org \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jacob.e.keller@intel.com \
--cc=john.fastabend@gmail.com \
--cc=kuba@kernel.org \
--cc=larysa.zaremba@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=magnus.karlsson@intel.com \
--cc=michal.kubiak@intel.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=wojciech.drewek@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.