All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nguyen, Anthony L" <anthony.l.nguyen@intel.com>
To: "kuba@kernel.org" <kuba@kernel.org>
Cc: "sassmann@redhat.com" <sassmann@redhat.com>,
	"Kuruvinakunnel, George" <george.kuruvinakunnel@intel.com>,
	"Fijalkowski, Maciej" <maciej.fijalkowski@intel.com>,
	"bjorn@kernel.org" <bjorn@kernel.org>,
	"Karlsson, Magnus" <magnus.karlsson@intel.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"Raczynski, Piotr" <piotr.raczynski@intel.com>
Subject: Re: [PATCH net 4/7] ice: use correct xdp_ring with XDP_TX action
Date: Tue, 26 Jan 2021 21:33:12 +0000	[thread overview]
Message-ID: <b94616ccb26b669154cca29021294dec06eecc9f.camel@intel.com> (raw)
In-Reply-To: <20210123195219.55f6d4e1@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

On Sat, 2021-01-23 at 19:52 -0800, Jakub Kicinski wrote:
> On Fri, 22 Jan 2021 15:57:31 -0800 Tony Nguyen wrote:
> > From: Piotr Raczynski <piotr.raczynski@intel.com>
> > 
> > XDP queue number for XDP_TX action is used inconsistently
> > and may result with no packets transmitted. Fix queue number
> > used by the driver when doing XDP_TX, i.e. use receive queue
> > number as in ice_finalize_xdp_rx.
> > 
> > Also, using smp_processor_id() is wrong here and won't
> > work with less queues.
> > 
> > Fixes: efc2214b6047 ("ice: Add support for XDP")
> > Signed-off-by: Piotr Raczynski <piotr.raczynski@intel.com>
> > Tested-by: George Kuruvinakunnel <george.kuruvinakunnel@intel.com>
> > Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> > ---
> >  drivers/net/ethernet/intel/ice/ice_txrx.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c
> > b/drivers/net/ethernet/intel/ice/ice_txrx.c
> > index b6fa83c619dd..7946a90b2da7 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_txrx.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
> > @@ -546,7 +546,7 @@ ice_run_xdp(struct ice_ring *rx_ring, struct
> > xdp_buff *xdp,
> >  	case XDP_PASS:
> >  		break;
> >  	case XDP_TX:
> > -		xdp_ring = rx_ring->vsi->xdp_rings[smp_processor_id()];
> > +		xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->q_index];
> 
> But then what protects you from one CPU trying to use the tx ring
> from
> XDP_TX and another from ice_xdp_xmit() ?
> 
> Also why does this code not check queue_index < vsi->num_xdp_txq
> like ice_xdp_xmit() does?

Hi Jakub

I'm still waiting for information from the author. I'm going to drop
this patch from the series and resubmit.

> Let me CC your local XDP experts whose tags I'm surprised not to see
> on
> this patch.

I'll add them to the XDP patches in the future.


Thanks,
Tony

  reply	other threads:[~2021-01-27  8:24 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-22 23:57 [PATCH net 0/7][pull request] Intel Wired LAN Driver Updates 2021-01-22 Tony Nguyen
2021-01-22 23:57 ` [PATCH net 1/7] ice: fix FDir IPv6 flexbyte Tony Nguyen
2021-01-22 23:57 ` [PATCH net 2/7] ice: Implement flow for IPv6 next header (extension header) Tony Nguyen
2021-01-22 23:57 ` [PATCH net 3/7] ice: update dev_addr in ice_set_mac_address even if HW filter exists Tony Nguyen
2021-01-22 23:57 ` [PATCH net 4/7] ice: use correct xdp_ring with XDP_TX action Tony Nguyen
2021-01-24  3:52   ` Jakub Kicinski
2021-01-26 21:33     ` Nguyen, Anthony L [this message]
2021-01-22 23:57 ` [PATCH net 5/7] ice: Don't allow more channels than LAN MSI-X available Tony Nguyen
2021-01-22 23:57 ` [PATCH net 6/7] ice: Fix MSI-X vector fallback logic Tony Nguyen
2021-01-22 23:57 ` [PATCH net 7/7] i40e: acquire VSI pointer only after VF is initialized Tony Nguyen

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=b94616ccb26b669154cca29021294dec06eecc9f.camel@intel.com \
    --to=anthony.l.nguyen@intel.com \
    --cc=bjorn@kernel.org \
    --cc=davem@davemloft.net \
    --cc=george.kuruvinakunnel@intel.com \
    --cc=kuba@kernel.org \
    --cc=maciej.fijalkowski@intel.com \
    --cc=magnus.karlsson@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=piotr.raczynski@intel.com \
    --cc=sassmann@redhat.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.