| Message ID | 20221213105023.196409-1-tirthendu.sarkar@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | i40e: support XDP multi-buffer | expand |
On Tue, 2022-12-13 at 16:20 +0530, Tirthendu Sarkar wrote: > This patchset adds multi-buffer support for XDP. The first four patches > are prepatory patches while the fifth one contains actual multi-buffer > changes. > > Tirthendu Sarkar (5): > i40e: add pre-xdp page_count in rx_buffer > i40e: avoid per buffer next_to_clean access from i40e_ring > i40e: introduce next_to_process to i40e_ring > i40e: pull out rx buffer allocation to end of i40e_clean_rx_irq() > i40e: add support for XDP multi-buffer Rx > > drivers/net/ethernet/intel/i40e/i40e_main.c | 18 +- > drivers/net/ethernet/intel/i40e/i40e_txrx.c | 378 ++++++++++++++------ > drivers/net/ethernet/intel/i40e/i40e_txrx.h | 13 +- > 3 files changed, 280 insertions(+), 129 deletions(-) > This approach seems kind of convoluted to me. Basically you are trying to clean the ring without cleaning the ring in the cases where you encounter a non EOP descriptor. Why not just replace the skb pointer with an xdp_buff in the ring? Then you just build an xdp_buff w/ frags and then convert it after after i40e_is_non_eop? You should then still be able to use all the same page counting tricks and the pages would just be dropped into the shared info of an xdp_buff instead of an skb and function the same assuming you have all the logic in place to clean them up correctly.
> From: Alexander H Duyck <alexander.duyck@gmail.com> > Sent: Tuesday, December 13, 2022 9:28 PM > > This approach seems kind of convoluted to me. Basically you are trying > to clean the ring without cleaning the ring in the cases where you > encounter a non EOP descriptor. > > Why not just replace the skb pointer with an xdp_buff in the ring? Then > you just build an xdp_buff w/ frags and then convert it after after > i40e_is_non_eop? You should then still be able to use all the same page > counting tricks and the pages would just be dropped into the shared > info of an xdp_buff instead of an skb and function the same assuming > you have all the logic in place to clean them up correctly. We have another approach similar to what you have suggested which sort of is a bit cleaner but not free of a burden of getting the rx_buffer struct back again for all of the packet frags post i40e_run_xdp() for recycling. We will examine if that turns out to be better.
On Wed, Dec 14, 2022 at 7:56 AM Sarkar, Tirthendu <tirthendu.sarkar@intel.com> wrote: > > > From: Alexander H Duyck <alexander.duyck@gmail.com> > > Sent: Tuesday, December 13, 2022 9:28 PM > > > > This approach seems kind of convoluted to me. Basically you are trying > > to clean the ring without cleaning the ring in the cases where you > > encounter a non EOP descriptor. > > > > Why not just replace the skb pointer with an xdp_buff in the ring? Then > > you just build an xdp_buff w/ frags and then convert it after after > > i40e_is_non_eop? You should then still be able to use all the same page > > counting tricks and the pages would just be dropped into the shared > > info of an xdp_buff instead of an skb and function the same assuming > > you have all the logic in place to clean them up correctly. > > We have another approach similar to what you have suggested which sort > of is a bit cleaner but not free of a burden of getting the rx_buffer struct > back again for all of the packet frags post i40e_run_xdp() for recycling. > We will examine if that turns out to be better. Sounds good. Keep in mind that there are multiple use cases for the NIC so you don't want to optimize for the less likely to be used ones such as XDP_DROP/XDP_ABORT over standard use cases such as simply passing packets up to the network stack.