Message-ID: <ZCW3gFQStgYJRTBn@boxer>
Date:   Thu, 30 Mar 2023 18:23:28 +0200
From:   Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To:     Pavan Kumar Linga <pavan.kumar.linga@...el.com>
CC:     <intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>,
        <shiraz.saleem@...el.com>, <emil.s.tantilov@...el.com>,
        <willemb@...gle.com>, <decot@...gle.com>, <joshua.a.hay@...el.com>,
        <sridhar.samudrala@...el.com>, Alan Brady <alan.brady@...el.com>,
        Madhu Chittim <madhu.chittim@...el.com>,
        Phani Burra <phani.r.burra@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH net-next 12/15] idpf: add RX splitq
 napi poll support

On Wed, Mar 29, 2023 at 07:04:01AM -0700, Pavan Kumar Linga wrote:
> From: Alan Brady <alan.brady@...el.com>
> 
> Add support to handle interrupts for the RX completion queue and
> RX buffer queue. When the interrupt fires on RX completion queue,
> process the RX descriptors that are received. Allocate and prepare
> the SKB with the RX packet info, for both data and header buffer.
> 
> IDPF uses software maintained refill queues to manage buffers between
> RX queue producer and the buffer queue consumer. They are required in
> order to maintain a lockless buffer management system and are strictly
> software only constructs. Instead of updating the RX buffer queue tail
> with available buffers right after the clean routine, it posts the
> buffer ids to the refill queues, only to post them to the HW later.
> 
> If the generic receive offload (GRO) is enabled in the capabilities
> and turned on by default or via ethtool, then HW performs the
> packet coalescing if certain criteria are met by the incoming
> packets and updates the RX descriptor. Similar to GRO, if generic
> checksum is enabled, HW computes the checksum and updates the
> respective fields in the descriptor. Add support to update the
> SKB fields with the GRO and the generic checksum received.
> 
> Signed-off-by: Alan Brady <alan.brady@...el.com>
> Co-developed-by: Joshua Hay <joshua.a.hay@...el.com>
> Signed-off-by: Joshua Hay <joshua.a.hay@...el.com>
> Co-developed-by: Madhu Chittim <madhu.chittim@...el.com>
> Signed-off-by: Madhu Chittim <madhu.chittim@...el.com>
> Co-developed-by: Phani Burra <phani.r.burra@...el.com>
> Signed-off-by: Phani Burra <phani.r.burra@...el.com>
> Co-developed-by: Pavan Kumar Linga <pavan.kumar.linga@...el.com>
> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@...el.com>
> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@...el.com>
> ---
>  drivers/net/ethernet/intel/idpf/idpf.h        |    2 +
>  drivers/net/ethernet/intel/idpf/idpf_txrx.c   | 1000 ++++++++++++++++-
>  drivers/net/ethernet/intel/idpf/idpf_txrx.h   |   56 +-
>  .../net/ethernet/intel/idpf/idpf_virtchnl.c   |    4 +-
>  4 files changed, 1053 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
> index 9c0404c0d796..5d6a791f10de 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf.h
> @@ -14,6 +14,7 @@ struct idpf_vport_max_q;
>  #include <linux/etherdevice.h>
>  #include <linux/pci.h>
>  #include <linux/bitfield.h>
> +#include <net/gro.h>
>  #include <linux/dim.h>
>  
>  #include "virtchnl2.h"
> @@ -262,6 +263,7 @@ struct idpf_vport {
>  	u8 default_mac_addr[ETH_ALEN];
>  	/* ITR profiles for the DIM algorithm */
>  #define IDPF_DIM_PROFILE_SLOTS  5
> +	u16 rx_itr_profile[IDPF_DIM_PROFILE_SLOTS];
>  	u16 tx_itr_profile[IDPF_DIM_PROFILE_SLOTS];
>  
>  	bool link_up;
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> index 4518ea7b9a31..8a96e5f4ba30 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> @@ -339,6 +339,11 @@ static void idpf_rx_buf_rel(struct idpf_queue *rxq,
>  	idpf_rx_page_rel(rxq, &rx_buf->page_info[0]);
>  	if (PAGE_SIZE < 8192 && rx_buf->buf_size > IDPF_RX_BUF_2048)
>  		idpf_rx_page_rel(rxq, &rx_buf->page_info[1]);
> +
> +	if (rx_buf->skb) {
> +		dev_kfree_skb(rx_buf->skb);
> +		rx_buf->skb = NULL;
> +	}

can you elaborate why you're introducing skb ptr to rx_buf if you have
this ptr already on idpf_queue?

>  }
>  
>  /**
> @@ -641,6 +646,28 @@ static bool idpf_rx_buf_hw_alloc_all(struct idpf_queue *rxbufq, u16 alloc_count)
>  	return !!alloc_count;
>  }
>  
> +/**
> + * idpf_rx_post_buf_refill - Post buffer id to refill queue
> + * @refillq: refill queue to post to
> + * @buf_id: buffer id to post
> + */
> +static void idpf_rx_post_buf_refill(struct idpf_sw_queue *refillq, u16 buf_id)
> +{
> +	u16 nta = refillq->next_to_alloc;
> +
> +	/* store the buffer ID and the SW maintained GEN bit to the refillq */
> +	refillq->ring[nta] =
> +		((buf_id << IDPF_RX_BI_BUFID_S) & IDPF_RX_BI_BUFID_M) |
> +		(!!(test_bit(__IDPF_Q_GEN_CHK, refillq->flags)) <<
> +		 IDPF_RX_BI_GEN_S);

do you explain anywhere in this patchset GEN bit usage?

> +
> +	if (unlikely(++nta == refillq->desc_count)) {
> +		nta = 0;
> +		change_bit(__IDPF_Q_GEN_CHK, refillq->flags);
> +	}
> +	refillq->next_to_alloc = nta;
> +}
> +

[...]

> +/**
> + * idpf_rx_buf_adjust_pg - Prepare rx buffer for reuse
> + * @rx_buf: Rx buffer to adjust
> + * @size: Size of adjustment
> + *
> + * Update the offset within page so that rx buf will be ready to be reused.
> + * For systems with PAGE_SIZE < 8192 this function will flip the page offset
> + * so the second half of page assigned to rx buffer will be used, otherwise
> + * the offset is moved by the @size bytes
> + */
> +static void idpf_rx_buf_adjust_pg(struct idpf_rx_buf *rx_buf, unsigned int size)
> +{
> +	struct idpf_page_info *pinfo;
> +
> +	pinfo = &rx_buf->page_info[rx_buf->page_indx];
> +
> +	if (PAGE_SIZE < 8192)
> +		if (rx_buf->buf_size > IDPF_RX_BUF_2048)

when buf_size can be non-2k?

> +			/* flip to second page */
> +			rx_buf->page_indx = !rx_buf->page_indx;
> +		else
> +			/* flip page offset to other buffer */
> +			pinfo->page_offset ^= size;
> +	else
> +		pinfo->page_offset += size;
> +}
> +
> +/**
> + * idpf_rx_can_reuse_page - Determine if page can be reused for another rx
> + * @rx_buf: buffer containing the page
> + *
> + * If page is reusable, we have a green light for calling idpf_reuse_rx_page,
> + * which will assign the current buffer to the buffer that next_to_alloc is
> + * pointing to; otherwise, the dma mapping needs to be destroyed and
> + * page freed
> + */
> +static bool idpf_rx_can_reuse_page(struct idpf_rx_buf *rx_buf)
> +{
> +	unsigned int last_offset = PAGE_SIZE - rx_buf->buf_size;
> +	struct idpf_page_info *pinfo;
> +	unsigned int pagecnt_bias;
> +	struct page *page;
> +
> +	pinfo = &rx_buf->page_info[rx_buf->page_indx];
> +	pagecnt_bias = pinfo->pagecnt_bias;
> +	page = pinfo->page;
> +
> +	if (unlikely(!dev_page_is_reusable(page)))
> +		return false;
> +
> +	if (PAGE_SIZE < 8192) {
> +		/* For 2K buffers, we can reuse the page if we are the
> +		 * owner. For 4K buffers, we can reuse the page if there are
> +		 * no other others.
> +		 */
> +		int reuse_bias = rx_buf->buf_size > IDPF_RX_BUF_2048 ? 0 : 1;

couldn't this be just:

		bool reuse_bias = !(rx_buf->buf_size > IDPF_RX_BUF_2048);

this is a hot path so avoiding branches is worthy.

> +
> +		if (unlikely((page_count(page) - pagecnt_bias) > reuse_bias))
> +			return false;
> +	} else if (pinfo->page_offset > last_offset) {
> +		return false;
> +	}
> +
> +	/* If we have drained the page fragment pool we need to update
> +	 * the pagecnt_bias and page count so that we fully restock the
> +	 * number of references the driver holds.
> +	 */
> +	if (unlikely(pagecnt_bias == 1)) {
> +		page_ref_add(page, USHRT_MAX - 1);
> +		pinfo->pagecnt_bias = USHRT_MAX;
> +	}
> +
> +	return true;
> +}
> +

[...]

> +/**
> + * idpf_rx_construct_skb - Allocate skb and populate it
> + * @rxq: Rx descriptor queue
> + * @rx_buf: Rx buffer to pull data from
> + * @size: the length of the packet
> + *
> + * This function allocates an skb. It then populates it with the page
> + * data from the current receive descriptor, taking care to set up the
> + * skb correctly.
> + */
> +static struct sk_buff *idpf_rx_construct_skb(struct idpf_queue *rxq,
> +					     struct idpf_rx_buf *rx_buf,
> +					     unsigned int size)
> +{
> +	struct idpf_page_info *pinfo;
> +	unsigned int headlen, truesize;

RCT please

> +	struct sk_buff *skb;
> +	void *va;
> +
> +	pinfo = &rx_buf->page_info[rx_buf->page_indx];
> +	va = page_address(pinfo->page) + pinfo->page_offset;
> +
> +	/* prefetch first cache line of first page */
> +	net_prefetch(va);
> +	/* allocate a skb to store the frags */
> +	skb = __napi_alloc_skb(&rxq->q_vector->napi, IDPF_RX_HDR_SIZE,
> +			       GFP_ATOMIC | __GFP_NOWARN);

any reason why no build_skb() support right from the start?

> +	if (unlikely(!skb))
> +		return NULL;
> +
> +	skb_record_rx_queue(skb, rxq->idx);
> +
> +	/* Determine available headroom for copy */
> +	headlen = size;
> +	if (headlen > IDPF_RX_HDR_SIZE)
> +		headlen = eth_get_headlen(skb->dev, va, IDPF_RX_HDR_SIZE);
> +
> +	/* align pull length to size of long to optimize memcpy performance */
> +	memcpy(__skb_put(skb, headlen), va, ALIGN(headlen, sizeof(long)));
> +
> +	/* if we exhaust the linear part then add what is left as a frag */
> +	size -= headlen;
> +	if (!size) {
> +		/* buffer is unused, reset bias back to rx_buf; data was copied
> +		 * onto skb's linear part so there's no need for adjusting
> +		 * page offset and we can reuse this buffer as-is
> +		 */
> +		pinfo->pagecnt_bias++;
> +
> +		return skb;
> +	}
> +
> +	truesize = idpf_rx_frame_truesize(rx_buf, size);
> +	skb_add_rx_frag(skb, 0, pinfo->page,
> +			pinfo->page_offset + headlen, size,
> +			truesize);
> +	/* buffer is used by skb, update page_offset */
> +	idpf_rx_buf_adjust_pg(rx_buf, truesize);
> +
> +	return skb;
> +}
> +
> +/**
> + * idpf_rx_hdr_construct_skb - Allocate skb and populate it from header buffer
> + * @rxq: Rx descriptor queue
> + * @hdr_buf: Rx buffer to pull data from
> + * @size: the length of the packet
> + *
> + * This function allocates an skb. It then populates it with the page data from
> + * the current receive descriptor, taking care to set up the skb correctly.
> + * This specifcally uses a header buffer to start building the skb.
> + */
> +static struct sk_buff *idpf_rx_hdr_construct_skb(struct idpf_queue *rxq,
> +						 struct idpf_dma_mem *hdr_buf,
> +						 unsigned int size)
> +{
> +	struct sk_buff *skb;
> +
> +	/* allocate a skb to store the frags */
> +	skb = __napi_alloc_skb(&rxq->q_vector->napi, size,
> +			       GFP_ATOMIC | __GFP_NOWARN);

ditto re: build_skb() comment

> +	if (unlikely(!skb))
> +		return NULL;
> +
> +	skb_record_rx_queue(skb, rxq->idx);
> +
> +	memcpy(__skb_put(skb, size), hdr_buf->va, ALIGN(size, sizeof(long)));
> +
> +	return skb;
> +}
> +
> +/**
> + * idpf_rx_splitq_test_staterr - tests bits in Rx descriptor
> + * status and error fields
> + * @stat_err_field: field from descriptor to test bits in
> + * @stat_err_bits: value to mask
> + *
> + */
> +static bool idpf_rx_splitq_test_staterr(const u8 stat_err_field,
> +					const u8 stat_err_bits)
> +{
> +	return !!(stat_err_field & stat_err_bits);
> +}
> +
> +/**
> + * idpf_rx_splitq_is_eop - process handling of EOP buffers
> + * @rx_desc: Rx descriptor for current buffer
> + *
> + * If the buffer is an EOP buffer, this function exits returning true,
> + * otherwise return false indicating that this is in fact a non-EOP buffer.
> + */
> +static bool idpf_rx_splitq_is_eop(struct virtchnl2_rx_flex_desc_adv_nic_3 *rx_desc)
> +{
> +	/* if we are the last buffer then there is nothing else to do */
> +	return likely(idpf_rx_splitq_test_staterr(rx_desc->status_err0_qw1,
> +						  IDPF_RXD_EOF_SPLITQ));
> +}
> +
> +/**
> + * idpf_rx_splitq_recycle_buf - Attempt to recycle or realloc buffer
> + * @rxbufq: receive queue
> + * @rx_buf: Rx buffer to pull data from
> + *
> + * This function will clean up the contents of the rx_buf. It will either
> + * recycle the buffer or unmap it and free the associated resources. The buffer
> + * will then be placed on a refillq where it will later be reclaimed by the
> + * corresponding bufq.
> + *
> + * This works based on page flipping. If we assume e.g., a 4k page, it will be
> + * divided into two 2k buffers. We post the first half to hardware and, after
> + * using it, flip to second half of the page with idpf_adjust_pg_offset and
> + * post that to hardware. The third time through we'll flip back to first half
> + * of page and check if stack is still using it, if not we can reuse the buffer
> + * as is, otherwise we'll drain it and get a new page.
> + */
> +static void idpf_rx_splitq_recycle_buf(struct idpf_queue *rxbufq,
> +				       struct idpf_rx_buf *rx_buf)
> +{
> +	struct idpf_page_info *pinfo = &rx_buf->page_info[rx_buf->page_indx];
> +
> +	if (idpf_rx_can_reuse_page(rx_buf))
> +		return;
> +
> +	/* we are not reusing the buffer so unmap it */
> +	dma_unmap_page_attrs(rxbufq->dev, pinfo->dma, PAGE_SIZE,
> +			     DMA_FROM_DEVICE, IDPF_RX_DMA_ATTR);
> +	__page_frag_cache_drain(pinfo->page, pinfo->pagecnt_bias);
> +
> +	/* clear contents of buffer_info */
> +	pinfo->page = NULL;
> +	rx_buf->skb = NULL;

this skb NULLing is pointless to me. from the callsite of this function
you operate strictly on a skb from idpf_queue.

> +
> +	/* It's possible the alloc can fail here but there's not much
> +	 * we can do, bufq will have to try and realloc to fill the
> +	 * hole.
> +	 */
> +	idpf_alloc_page(rxbufq, pinfo);
> +}
> +
> +/**
> + * idpf_rx_splitq_clean - Clean completed descriptors from Rx queue
> + * @rxq: Rx descriptor queue to retrieve receive buffer queue
> + * @budget: Total limit on number of packets to process
> + *
> + * This function provides a "bounce buffer" approach to Rx interrupt
> + * processing. The advantage to this is that on systems that have
> + * expensive overhead for IOMMU access this provides a means of avoiding
> + * it by maintaining the mapping of the page to the system.
> + *
> + * Returns amount of work completed
> + */
> +static int idpf_rx_splitq_clean(struct idpf_queue *rxq, int budget)
> +{
> +	int total_rx_bytes = 0, total_rx_pkts = 0;
> +	struct idpf_queue *rx_bufq = NULL;
> +	struct sk_buff *skb = rxq->skb;
> +	u16 ntc = rxq->next_to_clean;
> +
> +	/* Process Rx packets bounded by budget */
> +	while (likely(total_rx_pkts < budget)) {
> +		struct virtchnl2_rx_flex_desc_adv_nic_3 *rx_desc;
> +		struct idpf_sw_queue *refillq = NULL;
> +		struct idpf_dma_mem *hdr_buf = NULL;
> +		struct idpf_rxq_set *rxq_set = NULL;
> +		struct idpf_rx_buf *rx_buf = NULL;
> +		union virtchnl2_rx_desc *desc;
> +		unsigned int pkt_len = 0;
> +		unsigned int hdr_len = 0;
> +		u16 gen_id, buf_id = 0;
> +		 /* Header buffer overflow only valid for header split */
> +		bool hbo = false;
> +		int bufq_id;
> +		u8 rxdid;
> +
> +		/* get the Rx desc from Rx queue based on 'next_to_clean' */
> +		desc = IDPF_RX_DESC(rxq, ntc);
> +		rx_desc = (struct virtchnl2_rx_flex_desc_adv_nic_3 *)desc;
> +
> +		/* This memory barrier is needed to keep us from reading
> +		 * any other fields out of the rx_desc
> +		 */
> +		dma_rmb();
> +
> +		/* if the descriptor isn't done, no work yet to do */
> +		gen_id = le16_to_cpu(rx_desc->pktlen_gen_bufq_id);
> +		gen_id = FIELD_GET(VIRTCHNL2_RX_FLEX_DESC_ADV_GEN_M, gen_id);
> +
> +		if (test_bit(__IDPF_Q_GEN_CHK, rxq->flags) != gen_id)
> +			break;
> +
> +		rxdid = FIELD_GET(VIRTCHNL2_RX_FLEX_DESC_ADV_RXDID_M,
> +				  rx_desc->rxdid_ucast);
> +		if (rxdid != VIRTCHNL2_RXDID_2_FLEX_SPLITQ) {
> +			IDPF_RX_BUMP_NTC(rxq, ntc);
> +			u64_stats_update_begin(&rxq->stats_sync);
> +			u64_stats_inc(&rxq->q_stats.rx.bad_descs);
> +			u64_stats_update_end(&rxq->stats_sync);
> +			continue;
> +		}
> +
> +		pkt_len = le16_to_cpu(rx_desc->pktlen_gen_bufq_id);
> +		pkt_len = FIELD_GET(VIRTCHNL2_RX_FLEX_DESC_ADV_LEN_PBUF_M,
> +				    pkt_len);
> +
> +		hbo = FIELD_GET(BIT(VIRTCHNL2_RX_FLEX_DESC_ADV_STATUS0_HBO_S),
> +				rx_desc->status_err0_qw1);
> +
> +		if (unlikely(hbo)) {
> +			/* If a header buffer overflow, occurs, i.e. header is
> +			 * too large to fit in the header split buffer, HW will
> +			 * put the entire packet, including headers, in the
> +			 * data/payload buffer.
> +			 */
> +			u64_stats_update_begin(&rxq->stats_sync);
> +			u64_stats_inc(&rxq->q_stats.rx.hsplit_buf_ovf);
> +			u64_stats_update_end(&rxq->stats_sync);
> +			goto bypass_hsplit;
> +		}
> +
> +		hdr_len = le16_to_cpu(rx_desc->hdrlen_flags);
> +		hdr_len = FIELD_GET(VIRTCHNL2_RX_FLEX_DESC_ADV_LEN_HDR_M,
> +				    hdr_len);
> +
> +bypass_hsplit:
> +		bufq_id = le16_to_cpu(rx_desc->pktlen_gen_bufq_id);
> +		bufq_id = FIELD_GET(VIRTCHNL2_RX_FLEX_DESC_ADV_BUFQ_ID_M,
> +				    bufq_id);
> +
> +		rxq_set = container_of(rxq, struct idpf_rxq_set, rxq);
> +		if (!bufq_id)
> +			refillq = rxq_set->refillq0;
> +		else
> +			refillq = rxq_set->refillq1;
> +
> +		/* retrieve buffer from the rxq */
> +		rx_bufq = &rxq->rxq_grp->splitq.bufq_sets[bufq_id].bufq;
> +
> +		buf_id = le16_to_cpu(rx_desc->buf_id);
> +
> +		if (pkt_len) {
> +			rx_buf = &rx_bufq->rx_buf.buf[buf_id];
> +			idpf_rx_get_buf_page(rx_bufq->dev, rx_buf, pkt_len);
> +		}
> +
> +		if (hdr_len) {
> +			hdr_buf = rx_bufq->rx_buf.hdr_buf[buf_id];
> +
> +			dma_sync_single_for_cpu(rxq->dev, hdr_buf->pa, hdr_buf->size,
> +						DMA_FROM_DEVICE);
> +
> +			skb = idpf_rx_hdr_construct_skb(rxq, hdr_buf, hdr_len);
> +			u64_stats_update_begin(&rxq->stats_sync);
> +			u64_stats_inc(&rxq->q_stats.rx.hsplit_pkts);
> +			u64_stats_update_end(&rxq->stats_sync);
> +		}
> +
> +		if (pkt_len) {
> +			if (skb)
> +				idpf_rx_add_frag(rx_buf, skb, pkt_len);
> +			else
> +				skb = idpf_rx_construct_skb(rxq, rx_buf,
> +							    pkt_len);
> +		}
> +
> +		/* exit if we failed to retrieve a buffer */
> +		if (!skb) {
> +			/* If we fetched a buffer, but didn't use it
> +			 * undo pagecnt_bias decrement
> +			 */
> +			if (rx_buf)
> +				rx_buf->page_info[rx_buf->page_indx].pagecnt_bias++;
> +			break;
> +		}
> +
> +		if (rx_buf)
> +			idpf_rx_splitq_recycle_buf(rx_bufq, rx_buf);
> +		idpf_rx_post_buf_refill(refillq, buf_id);
> +
> +		IDPF_RX_BUMP_NTC(rxq, ntc);
> +		/* skip if it is non EOP desc */
> +		if (!idpf_rx_splitq_is_eop(rx_desc))
> +			continue;
> +
> +		/* pad skb if needed (to make valid ethernet frame) */
> +		if (eth_skb_pad(skb)) {
> +			skb = NULL;
> +			continue;
> +		}
> +
> +		/* probably a little skewed due to removing CRC */
> +		total_rx_bytes += skb->len;
> +
> +		/* protocol */
> +		if (unlikely(idpf_rx_process_skb_fields(rxq, skb, rx_desc))) {
> +			dev_kfree_skb_any(skb);
> +			skb = NULL;
> +			continue;
> +		}
> +
> +		/* send completed skb up the stack */
> +		napi_gro_receive(&rxq->q_vector->napi, skb);
> +		skb = NULL;
> +
> +		/* update budget accounting */
> +		total_rx_pkts++;
> +	}
> +
> +	rxq->next_to_clean = ntc;
> +
> +	rxq->skb = skb;
> +	u64_stats_update_begin(&rxq->stats_sync);
> +	u64_stats_add(&rxq->q_stats.rx.packets, total_rx_pkts);
> +	u64_stats_add(&rxq->q_stats.rx.bytes, total_rx_bytes);
> +	u64_stats_update_end(&rxq->stats_sync);
> +
> +	/* guarantee a trip back through this routine if there was a failure */
> +	return total_rx_pkts;
> +}

keeping above func for a context

[...]

>  /**
>   * idpf_vport_intr_clean_queues - MSIX mode Interrupt Handler
>   * @irq: interrupt number
> @@ -3205,7 +4102,7 @@ static void idpf_net_dim(struct idpf_q_vector *q_vector)
>  	u32 i;
>  
>  	if (!IDPF_ITR_IS_DYNAMIC(q_vector->tx_intr_mode))
> -		return;
> +		goto check_rx_itr;
>  
>  	for (i = 0, packets = 0, bytes = 0; i < q_vector->num_txq; i++) {
>  		struct idpf_queue *txq = q_vector->tx[i];
> @@ -3221,6 +4118,25 @@ static void idpf_net_dim(struct idpf_q_vector *q_vector)
>  	idpf_update_dim_sample(q_vector, &dim_sample, &q_vector->tx_dim,
>  			       packets, bytes);
>  	net_dim(&q_vector->tx_dim, dim_sample);
> +
> +check_rx_itr:
> +	if (!IDPF_ITR_IS_DYNAMIC(q_vector->rx_intr_mode))
> +		return;
> +
> +	for (i = 0, packets = 0, bytes = 0; i < q_vector->num_rxq; i++) {
> +		struct idpf_queue *rxq = q_vector->rx[i];
> +		unsigned int start;
> +
> +		do {
> +			start = u64_stats_fetch_begin(&rxq->stats_sync);
> +			packets += u64_stats_read(&rxq->q_stats.rx.packets);
> +			bytes += u64_stats_read(&rxq->q_stats.rx.bytes);
> +		} while (u64_stats_fetch_retry(&rxq->stats_sync, start));
> +	}
> +
> +	idpf_update_dim_sample(q_vector, &dim_sample, &q_vector->rx_dim,
> +			       packets, bytes);
> +	net_dim(&q_vector->rx_dim, dim_sample);
>  }
>  
>  /**
> @@ -3338,7 +4254,15 @@ static void idpf_vport_intr_ena_irq_all(struct idpf_vport *vport)
>  						  true);
>  		}
>  
> -		if (qv->num_txq)
> +		if (qv->num_rxq) {
> +			dynamic = IDPF_ITR_IS_DYNAMIC(qv->rx_intr_mode);
> +			itr = vport->rx_itr_profile[qv->rx_dim.profile_ix];
> +			idpf_vport_intr_write_itr(qv, dynamic ?
> +						  itr : qv->rx_itr_value,
> +						  false);
> +		}
> +
> +		if (qv->num_txq || qv->num_rxq)
>  			idpf_vport_intr_update_itr_ena_irq(qv);
>  	}
>  }
> @@ -3381,6 +4305,32 @@ static void idpf_tx_dim_work(struct work_struct *work)
>  	dim->state = DIM_START_MEASURE;
>  }
>  
> +/**
> + * idpf_rx_dim_work - Call back from the stack
> + * @work: work queue structure
> + */
> +static void idpf_rx_dim_work(struct work_struct *work)
> +{
> +	struct idpf_q_vector *q_vector;
> +	struct idpf_vport *vport;
> +	struct dim *dim;
> +	u16 itr;
> +
> +	dim = container_of(work, struct dim, work);
> +	q_vector = container_of(dim, struct idpf_q_vector, rx_dim);
> +	vport = q_vector->vport;
> +
> +	if (dim->profile_ix >= ARRAY_SIZE(vport->rx_itr_profile))
> +		dim->profile_ix = ARRAY_SIZE(vport->rx_itr_profile) - 1;
> +
> +	/* look up the values in our local table */
> +	itr = vport->rx_itr_profile[dim->profile_ix];
> +
> +	idpf_vport_intr_write_itr(q_vector, itr, false);
> +
> +	dim->state = DIM_START_MEASURE;
> +}
> +
>  /**
>   * idpf_init_dim - Set up dynamic interrupt moderation
>   * @qv: q_vector structure
> @@ -3390,6 +4340,10 @@ static void idpf_init_dim(struct idpf_q_vector *qv)
>  	INIT_WORK(&qv->tx_dim.work, idpf_tx_dim_work);
>  	qv->tx_dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
>  	qv->tx_dim.profile_ix = IDPF_DIM_DEFAULT_PROFILE_IX;
> +
> +	INIT_WORK(&qv->rx_dim.work, idpf_rx_dim_work);
> +	qv->rx_dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
> +	qv->rx_dim.profile_ix = IDPF_DIM_DEFAULT_PROFILE_IX;
>  }
>  
>  /**
> @@ -3437,6 +4391,44 @@ static bool idpf_tx_splitq_clean_all(struct idpf_q_vector *q_vec,
>  	return clean_complete;
>  }
>  
> +/**
> + * idpf_rx_splitq_clean_all- Clean completetion queues
> + * @q_vec: queue vector
> + * @budget: Used to determine if we are in netpoll
> + * @cleaned: returns number of packets cleaned
> + *
> + * Returns false if clean is not complete else returns true
> + */
> +static bool idpf_rx_splitq_clean_all(struct idpf_q_vector *q_vec, int budget,
> +				     int *cleaned)
> +{
> +	int num_rxq = q_vec->num_rxq;
> +	bool clean_complete = true;
> +	int pkts_cleaned = 0;
> +	int i, budget_per_q;
> +
> +	/* We attempt to distribute budget to each Rx queue fairly, but don't
> +	 * allow the budget to go below 1 because that would exit polling early.
> +	 */
> +	budget_per_q = num_rxq ? max(budget / num_rxq, 1) : 0;
> +	for (i = 0; i < num_rxq; i++) {
> +		struct idpf_queue *rxq = q_vec->rx[i];
> +		int pkts_cleaned_per_q;
> +
> +		pkts_cleaned_per_q = idpf_rx_splitq_clean(rxq, budget_per_q);
> +		/* if we clean as many as budgeted, we must not be done */
> +		if (pkts_cleaned_per_q >= budget_per_q)
> +			clean_complete = false;
> +		pkts_cleaned += pkts_cleaned_per_q;
> +	}
> +	*cleaned = pkts_cleaned;
> +
> +	for (i = 0; i < q_vec->num_bufq; i++)
> +		idpf_rx_clean_refillq_all(q_vec->bufq[i]);
> +
> +	return clean_complete;
> +}
> +
>  /**
>   * idpf_vport_splitq_napi_poll - NAPI handler
>   * @napi: struct from which you get q_vector
> @@ -3456,7 +4448,8 @@ static int idpf_vport_splitq_napi_poll(struct napi_struct *napi, int budget)
>  		return 0;
>  	}
>  
> -	clean_complete = idpf_tx_splitq_clean_all(q_vector, budget, &work_done);
> +	clean_complete = idpf_rx_splitq_clean_all(q_vector, budget, &work_done);
> +	clean_complete &= idpf_tx_splitq_clean_all(q_vector, budget, &work_done);
>  
>  	/* If work not completed, return budget and polling will return */
>  	if (!clean_complete)
> @@ -3810,7 +4803,6 @@ int idpf_init_rss(struct idpf_vport *vport)
>  /**
>   * idpf_deinit_rss - Release RSS resources
>   * @vport: virtual port
> - *
>   */
>  void idpf_deinit_rss(struct idpf_vport *vport)
>  {
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
> index 27bac854e7dc..f89dff970727 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
> @@ -61,10 +61,21 @@
>  
>  #define IDPF_RX_BUFQ_WORKING_SET(rxq)		((rxq)->desc_count - 1)
>  
> +#define IDPF_RX_BUMP_NTC(rxq, ntc)				\
> +do {								\
> +	if (unlikely(++(ntc) == (rxq)->desc_count)) {		\

desc_count won't change within single NAPI instance so i would rather
store this to aux variable on stack and use this in this macro.

> +		ntc = 0;					\
> +		change_bit(__IDPF_Q_GEN_CHK, (rxq)->flags);	\
> +	}							\
> +} while (0)
> +
> +#define IDPF_RX_HDR_SIZE			256
>  #define IDPF_RX_BUF_2048			2048
>  #define IDPF_RX_BUF_4096			4096
>  #define IDPF_RX_BUF_STRIDE			32
> +#define IDPF_RX_BUF_POST_STRIDE			16
>  #define IDPF_LOW_WATERMARK			64
> +/* Size of header buffer specifically for header split */
>  #define IDPF_HDR_BUF_SIZE			256
>  #define IDPF_PACKET_HDR_PAD	\
>  	(ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN * 2)
> @@ -74,10 +85,18 @@
>   */
>  #define IDPF_TX_SPLITQ_RE_MIN_GAP	64
> 

[...]

Powered by blists - more mailing lists