> -----Original Message-----
> From: Fijalkowski, Maciej <maciej.fijalkowski@xxxxxxxxx>
[...]
> > Previously i40e_alloc_rx_buffers() was called for every 32 cleaned
> > buffers. For multi-buffers this may not be optimal as there may be more
> > cleaned buffers in each i40e_clean_rx_irq() call. So this is now called
> > when at least half of the ring size has been cleaned.
>
> Please align this patch with xdp_features update
>
ACK
> >
> > Signed-off-by: Tirthendu Sarkar <tirthendu.sarkar@xxxxxxxxx>
> > ---
> > drivers/net/ethernet/intel/i40e/i40e_main.c | 4 +-
> > drivers/net/ethernet/intel/i40e/i40e_txrx.c | 314 +++++++++++++-------
> > drivers/net/ethernet/intel/i40e/i40e_txrx.h | 8 -
> > 3 files changed, 209 insertions(+), 117 deletions(-)
> >
>
> (...)
>
> > }
> >
> > +/**
> > + * i40e_add_xdp_frag: Add a frag to xdp_buff
> > + * @xdp: xdp_buff pointing to the data
> > + * @nr_frags: return number of buffers for the packet
> > + * @rx_buffer: rx_buffer holding data of the current frag
> > + * @size: size of data of current frag
> > + */
> > +static int i40e_add_xdp_frag(struct xdp_buff *xdp, u32 *nr_frags,
> > + struct i40e_rx_buffer *rx_buffer, u32 size)
> > +{
> > + struct skb_shared_info *sinfo =
> xdp_get_shared_info_from_buff(xdp);
> > +
> > + if (!xdp_buff_has_frags(xdp)) {
> > + sinfo->nr_frags = 0;
> > + sinfo->xdp_frags_size = 0;
> > + xdp_buff_set_frags_flag(xdp);
> > + } else if (unlikely(sinfo->nr_frags >= MAX_SKB_FRAGS)) {
> > + /* Overflowing packet: All frags need to be dropped */
> > + return -ENOMEM;
>
> nit: double space
>
ACK
[...]
> > + xdp_res = i40e_run_xdp(rx_ring, xdp, xdp_prog);
> > +
> > if (xdp_res) {
> > - if (xdp_res & (I40E_XDP_TX | I40E_XDP_REDIR)) {
> > - xdp_xmit |= xdp_res;
> > + xdp_xmit |= xdp_res & (I40E_XDP_TX |
> I40E_XDP_REDIR);
>
> what was wrong with having above included in the
>
> } else if (xdp_res & (I40E_XDP_TX | I40E_XDP_REDIR)) {
>
> branch?
>
For multi-buffer packets, only the first 'if' branch will be executed. We need to set
xdp_xmit for both single and multi-buffer packets.
[...]
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
> b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
> > index e86abc25bb5e..14ad074639ab 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
> > @@ -393,14 +393,6 @@ struct i40e_ring {
> >
> > struct rcu_head rcu; /* to avoid race on free */
> > u16 next_to_alloc;
> > - struct sk_buff *skb; /* When i40e_clean_rx_ring_irq()
> must
> > - * return before it sees the EOP for
> > - * the current packet, we save that
> skb
> > - * here and resume receiving this
> > - * packet the next time
> > - * i40e_clean_rx_ring_irq() is called
> > - * for this ring.
> > - */
>
> this comment was valuable to me back when i was getting started with i40e,
> so maybe we could have something equivalent around xdp_buff now?
>
We have a similar comment for xdp_buff in patch #7 where it was introduced.
> >
> > struct i40e_channel *ch;
> > u16 rx_offset;
> > --
> > 2.34.1
> >
