| Message ID | 20240820215620.1245310-4-anthony.l.nguyen@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d53d4dcce69be5773e2d0878c9899ebfbf58c393 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Intel Wired LAN Driver Updates 2024-08-20 (ice) | expand |
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>
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;