[PATCH net 0/4][pull request] Intel Wired LAN Driver Updates 2024-08-20 (ice)

[PATCH net 0/4][pull request] Intel Wired LAN Driver Updates 2024-08-20 (ice)

[Tony Nguyen]

This series contains updates to ice driver only.

Maciej fixes issues with Rx data path on architectures with
PAGE_SIZE >= 8192; correcting page reuse usage and calculations for
last offset and truesize.

Michal corrects assignment of devlink port number to use PF id.

The following are changes since commit 7565c39da89dc6ac9b1b0733bd70276bc66612b1:
  Merge branch 'bonding-fix-xfrm-offload-bugs'
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue 100GbE

Maciej Fijalkowski (3):
  ice: fix page reuse when PAGE_SIZE is over 8k
  ice: fix ICE_LAST_OFFSET formula
  ice: fix truesize operations for PAGE_SIZE >= 8192

Michal Swiatkowski (1):
  ice: use internal pf id instead of function number

 .../ethernet/intel/ice/devlink/devlink_port.c |  4 +-
 drivers/net/ethernet/intel/ice/ice_base.c     | 21 ++++++++-
 drivers/net/ethernet/intel/ice/ice_txrx.c     | 47 ++-----------------
 3 files changed, 26 insertions(+), 46 deletions(-)

-- 
2.42.0

[PATCH net 1/4] ice: fix page reuse when PAGE_SIZE is over 8k

[Tony Nguyen]

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

Architectures that have PAGE_SIZE >= 8192 such as arm64 should act the
same as x86 currently, meaning reuse of a page should only take place
when no one else is busy with it.

Do two things independently of underlying PAGE_SIZE:
- store the page count under ice_rx_buf::pgcnt
- then act upon its value vs ice_rx_buf::pagecnt_bias when making the
  decision regarding page reuse

Fixes: 2b245cb29421 ("ice: Implement transmit and NAPI support")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Tested-by: Chandan Kumar Rout <chandanx.rout@intel.com> (A Contingent Worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_txrx.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
index 8d25b6981269..50211188c1a7 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -837,16 +837,15 @@ ice_can_reuse_rx_page(struct ice_rx_buf *rx_buf)
 	if (!dev_page_is_reusable(page))
 		return false;
 
-#if (PAGE_SIZE < 8192)
 	/* if we are only owner of page we can reuse it */
 	if (unlikely(rx_buf->pgcnt - pagecnt_bias > 1))
 		return false;
-#else
+#if (PAGE_SIZE >= 8192)
 #define ICE_LAST_OFFSET \
 	(SKB_WITH_OVERHEAD(PAGE_SIZE) - ICE_RXBUF_2048)
 	if (rx_buf->page_offset > ICE_LAST_OFFSET)
 		return false;
-#endif /* PAGE_SIZE < 8192) */
+#endif /* PAGE_SIZE >= 8192) */
 
 	/* If we have drained the page fragment pool we need to update
 	 * the pagecnt_bias and page count so that we fully restock the
@@ -949,12 +948,7 @@ ice_get_rx_buf(struct ice_rx_ring *rx_ring, const unsigned int size,
 	struct ice_rx_buf *rx_buf;
 
 	rx_buf = &rx_ring->rx_buf[ntc];
-	rx_buf->pgcnt =
-#if (PAGE_SIZE < 8192)
-		page_count(rx_buf->page);
-#else
-		0;
-#endif
+	rx_buf->pgcnt = page_count(rx_buf->page);
 	prefetchw(rx_buf->page);
 
 	if (!size)
-- 
2.42.0

[PATCH net 2/4] ice: fix ICE_LAST_OFFSET formula

[Tony Nguyen]

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

For bigger PAGE_SIZE archs, ice driver works on 3k Rx buffers.
Therefore, ICE_LAST_OFFSET should take into account ICE_RXBUF_3072, not
ICE_RXBUF_2048.

Fixes: 7237f5b0dba4 ("ice: introduce legacy Rx flag")
Suggested-by: Luiz Capitulino <luizcap@redhat.com>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Tested-by: Chandan Kumar Rout <chandanx.rout@intel.com> (A Contingent Worker at Intel)
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 50211188c1a7..4b690952bb40 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -842,7 +842,7 @@ ice_can_reuse_rx_page(struct ice_rx_buf *rx_buf)
 		return false;
 #if (PAGE_SIZE >= 8192)
 #define ICE_LAST_OFFSET \
-	(SKB_WITH_OVERHEAD(PAGE_SIZE) - ICE_RXBUF_2048)
+	(SKB_WITH_OVERHEAD(PAGE_SIZE) - ICE_RXBUF_3072)
 	if (rx_buf->page_offset > ICE_LAST_OFFSET)
 		return false;
 #endif /* PAGE_SIZE >= 8192) */
-- 
2.42.0

[PATCH net 3/4] ice: fix truesize operations for PAGE_SIZE >= 8192

[Tony Nguyen]

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

When working on multi-buffer packet on arch that has PAGE_SIZE >= 8192,
truesize is calculated and stored in xdp_buff::frame_sz per each
processed Rx buffer. This means that frame_sz will contain the truesize
based on last received buffer, but commit 1dc1a7e7f410 ("ice:
Centrallize Rx buffer recycling") assumed this value will be constant
for each buffer, which breaks the page recycling scheme and mess up the
way we update the page::page_offset.

To fix this, let us work on constant truesize when PAGE_SIZE >= 8192
instead of basing this on size of a packet read from Rx descriptor. This
way we can simplify the code and avoid calculating truesize per each
received frame and on top of that when using
xdp_update_skb_shared_info(), current formula for truesize update will
be valid.

This means ice_rx_frame_truesize() can be removed altogether.
Furthermore, first call to it within ice_clean_rx_irq() for 4k PAGE_SIZE
was redundant as xdp_buff::frame_sz is initialized via xdp_init_buff()
in ice_vsi_cfg_rxq(). This should have been removed at the point where
xdp_buff struct started to be a member of ice_rx_ring and it was no
longer a stack based variable.

There are two fixes tags as my understanding is that the first one
exposed us to broken truesize and page_offset handling and then second
introduced broken skb_shared_info update in ice_{construct,build}_skb().

Reported-and-tested-by: Luiz Capitulino <luizcap@redhat.com>
Closes: https://lore.kernel.org/netdev/8f9e2a5c-fd30-4206-9311-946a06d031bb@redhat.com/
Fixes: 1dc1a7e7f410 ("ice: Centrallize Rx buffer recycling")
Fixes: 2fba7dc5157b ("ice: Add support for XDP multi-buffer on Rx side")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Tested-by: Chandan Kumar Rout <chandanx.rout@intel.com> (A Contingent Worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_base.c | 21 ++++++++++++++-
 drivers/net/ethernet/intel/ice/ice_txrx.c | 33 -----------------------
 2 files changed, 20 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_base.c b/drivers/net/ethernet/intel/ice/ice_base.c
index 1facf179a96f..f448d3a84564 100644
--- a/drivers/net/ethernet/intel/ice/ice_base.c
+++ b/drivers/net/ethernet/intel/ice/ice_base.c
@@ -512,6 +512,25 @@ static void ice_xsk_pool_fill_cb(struct ice_rx_ring *ring)
 	xsk_pool_fill_cb(ring->xsk_pool, &desc);
 }
 
+/**
+ * ice_get_frame_sz - calculate xdp_buff::frame_sz
+ * @rx_ring: the ring being configured
+ *
+ * Return frame size based on underlying PAGE_SIZE
+ */
+static unsigned int ice_get_frame_sz(struct ice_rx_ring *rx_ring)
+{
+	unsigned int frame_sz;
+
+#if (PAGE_SIZE >= 8192)
+	frame_sz = rx_ring->rx_buf_len;
+#else
+	frame_sz = ice_rx_pg_size(rx_ring) / 2;
+#endif
+
+	return frame_sz;
+}
+
 /**
  * ice_vsi_cfg_rxq - Configure an Rx queue
  * @ring: the ring being configured
@@ -576,7 +595,7 @@ static int ice_vsi_cfg_rxq(struct ice_rx_ring *ring)
 		}
 	}
 
-	xdp_init_buff(&ring->xdp, ice_rx_pg_size(ring) / 2, &ring->xdp_rxq);
+	xdp_init_buff(&ring->xdp, ice_get_frame_sz(ring), &ring->xdp_rxq);
 	ring->xdp.data = NULL;
 	ring->xdp_ext.pkt_ctx = &ring->pkt_ctx;
 	err = ice_setup_rx_ctx(ring);
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
index 4b690952bb40..c9bc3f1add5d 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -521,30 +521,6 @@ int ice_setup_rx_ring(struct ice_rx_ring *rx_ring)
 	return -ENOMEM;
 }
 
-/**
- * ice_rx_frame_truesize
- * @rx_ring: ptr to Rx ring
- * @size: size
- *
- * calculate the truesize with taking into the account PAGE_SIZE of
- * underlying arch
- */
-static unsigned int
-ice_rx_frame_truesize(struct ice_rx_ring *rx_ring, const unsigned int size)
-{
-	unsigned int truesize;
-
-#if (PAGE_SIZE < 8192)
-	truesize = ice_rx_pg_size(rx_ring) / 2; /* Must be power-of-2 */
-#else
-	truesize = rx_ring->rx_offset ?
-		SKB_DATA_ALIGN(rx_ring->rx_offset + size) +
-		SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) :
-		SKB_DATA_ALIGN(size);
-#endif
-	return truesize;
-}
-
 /**
  * ice_run_xdp - Executes an XDP program on initialized xdp_buff
  * @rx_ring: Rx ring
@@ -1154,11 +1130,6 @@ int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int budget)
 	bool failure;
 	u32 first;
 
-	/* Frame size depend on rx_ring setup when PAGE_SIZE=4K */
-#if (PAGE_SIZE < 8192)
-	xdp->frame_sz = ice_rx_frame_truesize(rx_ring, 0);
-#endif
-
 	xdp_prog = READ_ONCE(rx_ring->xdp_prog);
 	if (xdp_prog) {
 		xdp_ring = rx_ring->xdp_ring;
@@ -1217,10 +1188,6 @@ int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int budget)
 			hard_start = page_address(rx_buf->page) + rx_buf->page_offset -
 				     offset;
 			xdp_prepare_buff(xdp, hard_start, offset, size, !!offset);
-#if (PAGE_SIZE > 4096)
-			/* At larger PAGE_SIZE, frame_sz depend on len size */
-			xdp->frame_sz = ice_rx_frame_truesize(rx_ring, size);
-#endif
 			xdp_buff_clear_frags_flag(xdp);
 		} else if (ice_add_xdp_frag(rx_ring, xdp, rx_buf, size)) {
 			break;
-- 
2.42.0

[PATCH net 4/4] ice: use internal pf id instead of function number

[Tony Nguyen]

From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

Use always the same pf id in devlink port number. When doing
pass-through the PF to VM bus info func number can be any value.

Fixes: 2ae0aa4758b0 ("ice: Move devlink port to PF/VF struct")
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Suggested-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ice/devlink/devlink_port.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
index 00fed5a61d62..62ef8e2fb5f1 100644
--- a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
+++ b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
@@ -337,7 +337,7 @@ int ice_devlink_create_pf_port(struct ice_pf *pf)
 		return -EIO;
 
 	attrs.flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
-	attrs.phys.port_number = pf->hw.bus.func;
+	attrs.phys.port_number = pf->hw.pf_id;
 
 	/* As FW supports only port split options for whole device,
 	 * set port split options only for first PF.
@@ -455,7 +455,7 @@ int ice_devlink_create_vf_port(struct ice_vf *vf)
 		return -EINVAL;
 
 	attrs.flavour = DEVLINK_PORT_FLAVOUR_PCI_VF;
-	attrs.pci_vf.pf = pf->hw.bus.func;
+	attrs.pci_vf.pf = pf->hw.pf_id;
 	attrs.pci_vf.vf = vf->vf_id;
 
 	ice_devlink_set_switch_id(pf, &attrs.switch_id);
-- 
2.42.0

Re: [PATCH net 1/4] ice: fix page reuse when PAGE_SIZE is over 8k

[Jacob Keller]

On 8/20/2024 2:56 PM, Tony Nguyen wrote:
> From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> 
> Architectures that have PAGE_SIZE >= 8192 such as arm64 should act the
> same as x86 currently, meaning reuse of a page should only take place
> when no one else is busy with it.
> 
> Do two things independently of underlying PAGE_SIZE:
> - store the page count under ice_rx_buf::pgcnt
> - then act upon its value vs ice_rx_buf::pagecnt_bias when making the
>   decision regarding page reuse
> 
> Fixes: 2b245cb29421 ("ice: Implement transmit and NAPI support")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Tested-by: Chandan Kumar Rout <chandanx.rout@intel.com> (A Contingent Worker at Intel)
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---

Seems reasonable.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  drivers/net/ethernet/intel/ice/ice_txrx.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
> index 8d25b6981269..50211188c1a7 100644
> --- a/drivers/net/ethernet/intel/ice/ice_txrx.c
> +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
> @@ -837,16 +837,15 @@ ice_can_reuse_rx_page(struct ice_rx_buf *rx_buf)
>  	if (!dev_page_is_reusable(page))
>  		return false;
>  
> -#if (PAGE_SIZE < 8192)
>  	/* if we are only owner of page we can reuse it */
>  	if (unlikely(rx_buf->pgcnt - pagecnt_bias > 1))
>  		return false;
> -#else
> +#if (PAGE_SIZE >= 8192)

Whats with adding PAGE_SIZE >= 8192 here? Oh. I see you removed the
previous check, so this code only ever executed if page size was large. Ok.

>  #define ICE_LAST_OFFSET \
>  	(SKB_WITH_OVERHEAD(PAGE_SIZE) - ICE_RXBUF_2048)
>  	if (rx_buf->page_offset > ICE_LAST_OFFSET)
>  		return false;
> -#endif /* PAGE_SIZE < 8192) */
> +#endif /* PAGE_SIZE >= 8192) */
>  
>  	/* If we have drained the page fragment pool we need to update
>  	 * the pagecnt_bias and page count so that we fully restock the
> @@ -949,12 +948,7 @@ ice_get_rx_buf(struct ice_rx_ring *rx_ring, const unsigned int size,
>  	struct ice_rx_buf *rx_buf;
>  
>  	rx_buf = &rx_ring->rx_buf[ntc];
> -	rx_buf->pgcnt =
> -#if (PAGE_SIZE < 8192)
> -		page_count(rx_buf->page);
> -#else
> -		0;
> -#endif
> +	rx_buf->pgcnt = page_count(rx_buf->page);

yea, seems weird that if page size is large we would just not track the
count? Yea this new flow makes more sense to me.

>  	prefetchw(rx_buf->page);
>  
>  	if (!size)

Re: [PATCH net 3/4] ice: fix truesize operations for PAGE_SIZE >= 8192

[Jacob Keller]

On 8/20/2024 2:56 PM, Tony Nguyen wrote:
> From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> 
> When working on multi-buffer packet on arch that has PAGE_SIZE >= 8192,
> truesize is calculated and stored in xdp_buff::frame_sz per each
> processed Rx buffer. This means that frame_sz will contain the truesize
> based on last received buffer, but commit 1dc1a7e7f410 ("ice:
> Centrallize Rx buffer recycling") assumed this value will be constant
> for each buffer, which breaks the page recycling scheme and mess up the
> way we update the page::page_offset.
> 
> To fix this, let us work on constant truesize when PAGE_SIZE >= 8192
> instead of basing this on size of a packet read from Rx descriptor. This
> way we can simplify the code and avoid calculating truesize per each
> received frame and on top of that when using
> xdp_update_skb_shared_info(), current formula for truesize update will
> be valid.
> 
> This means ice_rx_frame_truesize() can be removed altogether.
> Furthermore, first call to it within ice_clean_rx_irq() for 4k PAGE_SIZE
> was redundant as xdp_buff::frame_sz is initialized via xdp_init_buff()
> in ice_vsi_cfg_rxq(). This should have been removed at the point where
> xdp_buff struct started to be a member of ice_rx_ring and it was no
> longer a stack based variable.
> 
> There are two fixes tags as my understanding is that the first one
> exposed us to broken truesize and page_offset handling and then second
> introduced broken skb_shared_info update in ice_{construct,build}_skb().
> 
> Reported-and-tested-by: Luiz Capitulino <luizcap@redhat.com>
> Closes: https://lore.kernel.org/netdev/8f9e2a5c-fd30-4206-9311-946a06d031bb@redhat.com/
> Fixes: 1dc1a7e7f410 ("ice: Centrallize Rx buffer recycling")
> Fixes: 2fba7dc5157b ("ice: Add support for XDP multi-buffer on Rx side")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Tested-by: Chandan Kumar Rout <chandanx.rout@intel.com> (A Contingent Worker at Intel)
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---

Much simpler too!

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Re: [PATCH net 2/4] ice: fix ICE_LAST_OFFSET formula

[Jacob Keller]

On 8/20/2024 2:56 PM, Tony Nguyen wrote:
> From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> 
> For bigger PAGE_SIZE archs, ice driver works on 3k Rx buffers.
> Therefore, ICE_LAST_OFFSET should take into account ICE_RXBUF_3072, not
> ICE_RXBUF_2048.
> 

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Re: [PATCH net 4/4] ice: use internal pf id instead of function number

[Jacob Keller]

On 8/20/2024 2:56 PM, Tony Nguyen wrote:
> From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> 
> Use always the same pf id in devlink port number. When doing
> pass-through the PF to VM bus info func number can be any value.
> 
> Fixes: 2ae0aa4758b0 ("ice: Move devlink port to PF/VF struct")
> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
> Suggested-by: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Re: [PATCH net 0/4][pull request] Intel Wired LAN Driver Updates 2024-08-20 (ice)

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Tony Nguyen <anthony.l.nguyen@intel.com>:

On Tue, 20 Aug 2024 14:56:14 -0700 you wrote:
> This series contains updates to ice driver only.
> 
> Maciej fixes issues with Rx data path on architectures with
> PAGE_SIZE >= 8192; correcting page reuse usage and calculations for
> last offset and truesize.
> 
> Michal corrects assignment of devlink port number to use PF id.
> 
> [...]

Here is the summary with links:
  - [net,1/4] ice: fix page reuse when PAGE_SIZE is over 8k
    https://git.kernel.org/netdev/net/c/50b2143356e8
  - [net,2/4] ice: fix ICE_LAST_OFFSET formula
    https://git.kernel.org/netdev/net/c/b966ad832942
  - [net,3/4] ice: fix truesize operations for PAGE_SIZE >= 8192
    https://git.kernel.org/netdev/net/c/d53d4dcce69b
  - [net,4/4] ice: use internal pf id instead of function number
    https://git.kernel.org/netdev/net/c/503ab6ee40fc

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html