All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To: Simon Horman <horms@kernel.org>
Cc: netdev@vger.kernel.org, xudu@redhat.com,
	anthony.l.nguyen@intel.com, przemyslaw.kitszel@intel.com,
	jacob.e.keller@intel.com, intel-wired-lan@lists.osuosl.org,
	jmaxwell@redhat.com, magnus.karlsson@intel.com
Subject: Re: [Intel-wired-lan] [PATCH v4 iwl-net 3/3] ice: stop storing XDP verdict within ice_rx_buf
Date: Thu, 23 Jan 2025 11:51:06 +0100	[thread overview]
Message-ID: <Z5IfGpKdnskgoJJ3@boxer> (raw)
In-Reply-To: <20250123104536.GL395043@kernel.org>

On Thu, Jan 23, 2025 at 10:45:36AM +0000, Simon Horman wrote:
> On Wed, Jan 22, 2025 at 04:10:46PM +0100, Maciej Fijalkowski wrote:
> > Idea behind having ice_rx_buf::act was to simplify and speed up the Rx
> > data path by walking through buffers that were representing cleaned HW
> > Rx descriptors. Since it caused us a major headache recently and we
> > rolled back to old approach that 'puts' Rx buffers right after running
> > XDP prog/creating skb, this is useless now and should be removed.
> > 
> > Get rid of ice_rx_buf::act and related logic. We still need to take care
> > of a corner case where XDP program releases a particular fragment.
> > 
> > Make ice_run_xdp() to return its result and use it within
> > ice_put_rx_mbuf().
> > 
> > Fixes: 2fba7dc5157b ("ice: Add support for XDP multi-buffer on Rx side")
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > ---
> >  drivers/net/ethernet/intel/ice/ice_txrx.c     | 61 +++++++++++--------
> >  drivers/net/ethernet/intel/ice/ice_txrx.h     |  1 -
> >  drivers/net/ethernet/intel/ice/ice_txrx_lib.h | 43 -------------
> >  3 files changed, 35 insertions(+), 70 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
> 
> ...
> 
> > @@ -1139,23 +1136,27 @@ ice_put_rx_buf(struct ice_rx_ring *rx_ring, struct ice_rx_buf *rx_buf)
> >   * returned by XDP program;
> >   */
> >  static void ice_put_rx_mbuf(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp,
> > -			    u32 *xdp_xmit, u32 ntc)
> > +			    u32 *xdp_xmit, u32 ntc, u32 verdict)
> 
> Hi Marciej,
> 
> Sorry, there is one more Kernel doc nit. As reported by the Kernel Test
> Robot, verdict should be added to the Kernel doc for this function.

Yeah that is embarrassing. I have now included

./scripts/kernel-doc -none $FILE

to my pre-upstreaming checks so that it won't be happening again...
(or is there a way to run the kernel-doc against patch itself?)

> 
> With that addressed feel free to add:
> 
> Reviewed-by: Simon Horman <horms@kernel.org>

Thanks! Will include them in v5.

> 
> ...

WARNING: multiple messages have this Message-ID (diff)
From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To: Simon Horman <horms@kernel.org>
Cc: <intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>,
	<anthony.l.nguyen@intel.com>, <magnus.karlsson@intel.com>,
	<jacob.e.keller@intel.com>, <xudu@redhat.com>,
	<mschmidt@redhat.com>, <jmaxwell@redhat.com>, <poros@redhat.com>,
	<przemyslaw.kitszel@intel.com>
Subject: Re: [PATCH v4 iwl-net 3/3] ice: stop storing XDP verdict within ice_rx_buf
Date: Thu, 23 Jan 2025 11:51:06 +0100	[thread overview]
Message-ID: <Z5IfGpKdnskgoJJ3@boxer> (raw)
In-Reply-To: <20250123104536.GL395043@kernel.org>

On Thu, Jan 23, 2025 at 10:45:36AM +0000, Simon Horman wrote:
> On Wed, Jan 22, 2025 at 04:10:46PM +0100, Maciej Fijalkowski wrote:
> > Idea behind having ice_rx_buf::act was to simplify and speed up the Rx
> > data path by walking through buffers that were representing cleaned HW
> > Rx descriptors. Since it caused us a major headache recently and we
> > rolled back to old approach that 'puts' Rx buffers right after running
> > XDP prog/creating skb, this is useless now and should be removed.
> > 
> > Get rid of ice_rx_buf::act and related logic. We still need to take care
> > of a corner case where XDP program releases a particular fragment.
> > 
> > Make ice_run_xdp() to return its result and use it within
> > ice_put_rx_mbuf().
> > 
> > Fixes: 2fba7dc5157b ("ice: Add support for XDP multi-buffer on Rx side")
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > ---
> >  drivers/net/ethernet/intel/ice/ice_txrx.c     | 61 +++++++++++--------
> >  drivers/net/ethernet/intel/ice/ice_txrx.h     |  1 -
> >  drivers/net/ethernet/intel/ice/ice_txrx_lib.h | 43 -------------
> >  3 files changed, 35 insertions(+), 70 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
> 
> ...
> 
> > @@ -1139,23 +1136,27 @@ ice_put_rx_buf(struct ice_rx_ring *rx_ring, struct ice_rx_buf *rx_buf)
> >   * returned by XDP program;
> >   */
> >  static void ice_put_rx_mbuf(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp,
> > -			    u32 *xdp_xmit, u32 ntc)
> > +			    u32 *xdp_xmit, u32 ntc, u32 verdict)
> 
> Hi Marciej,
> 
> Sorry, there is one more Kernel doc nit. As reported by the Kernel Test
> Robot, verdict should be added to the Kernel doc for this function.

Yeah that is embarrassing. I have now included

./scripts/kernel-doc -none $FILE

to my pre-upstreaming checks so that it won't be happening again...
(or is there a way to run the kernel-doc against patch itself?)

> 
> With that addressed feel free to add:
> 
> Reviewed-by: Simon Horman <horms@kernel.org>

Thanks! Will include them in v5.

> 
> ...

  reply	other threads:[~2025-01-23 10:51 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-22 15:10 [Intel-wired-lan] [PATCH v4 iwl-net 0/3] ice: fix Rx data path for heavy 9k MTU traffic Maciej Fijalkowski
2025-01-22 15:10 ` Maciej Fijalkowski
2025-01-22 15:10 ` [Intel-wired-lan] [PATCH v4 iwl-net 1/3] ice: put Rx buffers after being done with current frame Maciej Fijalkowski
2025-01-22 15:10   ` Maciej Fijalkowski
2025-01-23 10:43   ` [Intel-wired-lan] " Simon Horman
2025-01-23 10:43     ` Simon Horman
2025-01-22 15:10 ` [Intel-wired-lan] [PATCH v4 iwl-net 2/3] ice: gather page_count()'s of each frag right before XDP prog call Maciej Fijalkowski
2025-01-22 15:10   ` Maciej Fijalkowski
2025-01-23 10:43   ` [Intel-wired-lan] " Simon Horman
2025-01-23 10:43     ` Simon Horman
2025-01-22 15:10 ` [Intel-wired-lan] [PATCH v4 iwl-net 3/3] ice: stop storing XDP verdict within ice_rx_buf Maciej Fijalkowski
2025-01-22 15:10   ` Maciej Fijalkowski
2025-01-22 19:42   ` [Intel-wired-lan] " kernel test robot
2025-01-23 10:45   ` Simon Horman
2025-01-23 10:45     ` Simon Horman
2025-01-23 10:51     ` Maciej Fijalkowski [this message]
2025-01-23 10:51       ` Maciej Fijalkowski
2025-01-24  7:49       ` [Intel-wired-lan] " Przemek Kitszel
2025-01-24  7:49         ` Przemek Kitszel

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=Z5IfGpKdnskgoJJ3@boxer \
    --to=maciej.fijalkowski@intel.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=horms@kernel.org \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jacob.e.keller@intel.com \
    --cc=jmaxwell@redhat.com \
    --cc=magnus.karlsson@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=xudu@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.