Re: [PATCH net-next 15/16] gve: DQO: Add TX path

[kernel test robot <lkp@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 16480 bytes --]

Hi Bailey,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on 35713d9b8f090d7a226e4aaeeb742265cde33c82]

url:    https://github.com/0day-ci/linux/commits/Bailey-Forrest/gve-Introduce-DQO-descriptor-format/20210625-021110
base:   35713d9b8f090d7a226e4aaeeb742265cde33c82
config: i386-randconfig-a011-20210622 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/af0833aafca5d9abd931a16ee9e761e85f5ad965
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Bailey-Forrest/gve-Introduce-DQO-descriptor-format/20210625-021110
        git checkout af0833aafca5d9abd931a16ee9e761e85f5ad965
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/net/ethernet/google/gve/gve_tx_dqo.c: In function 'gve_tx_clean_pending_packets':
   drivers/net/ethernet/google/gve/gve_tx_dqo.c:88:27: warning: unused variable 'buf' [-Wunused-variable]
      88 |    struct gve_tx_dma_buf *buf = &cur_state->bufs[j];
         |                           ^~~
   drivers/net/ethernet/google/gve/gve_tx_dqo.c: In function 'gve_tx_add_skb_no_copy_dqo':
   drivers/net/ethernet/google/gve/gve_tx_dqo.c:496:26: warning: unused variable 'buf' [-Wunused-variable]
     496 |   struct gve_tx_dma_buf *buf =
         |                          ^~~
   drivers/net/ethernet/google/gve/gve_tx_dqo.c:515:26: warning: unused variable 'buf' [-Wunused-variable]
     515 |   struct gve_tx_dma_buf *buf =
         |                          ^~~
   drivers/net/ethernet/google/gve/gve_tx_dqo.c:556:26: warning: unused variable 'buf' [-Wunused-variable]
     556 |   struct gve_tx_dma_buf *buf = &pending_packet->bufs[i];
         |                          ^~~
   drivers/net/ethernet/google/gve/gve_tx_dqo.c: In function 'remove_from_list':
>> drivers/net/ethernet/google/gve/gve_tx_dqo.c:730:6: warning: variable 'index' set but not used [-Wunused-but-set-variable]
     730 |  s16 index, prev_index, next_index;
         |      ^~~~~
   drivers/net/ethernet/google/gve/gve_tx_dqo.c: In function 'gve_unmap_packet':
>> drivers/net/ethernet/google/gve/gve_tx_dqo.c:753:25: warning: variable 'buf' set but not used [-Wunused-but-set-variable]
     753 |  struct gve_tx_dma_buf *buf;
         |                         ^~~
   In file included from include/linux/printk.h:7,
                    from include/linux/kernel.h:17,
                    from arch/x86/include/asm/percpu.h:27,
                    from arch/x86/include/asm/current.h:6,
                    from include/linux/sched.h:12,
                    from include/linux/ratelimit.h:6,
                    from include/linux/dev_printk.h:16,
                    from include/linux/device.h:15,
                    from include/linux/dma-mapping.h:7,
                    from drivers/net/ethernet/google/gve/gve.h:10,
                    from drivers/net/ethernet/google/gve/gve_tx_dqo.c:7:
   drivers/net/ethernet/google/gve/gve_tx_dqo.c: In function 'remove_miss_completions':
>> include/linux/kern_levels.h:5:18: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'int' [-Wformat=]
       5 | #define KERN_SOH "\001"  /* ASCII Start Of Header */
         |                  ^~~~~~
   include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
      11 | #define KERN_ERR KERN_SOH "3" /* error conditions */
         |                  ^~~~~~~~
   include/linux/printk.h:343:9: note: in expansion of macro 'KERN_ERR'
     343 |  printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~~~
   include/linux/net.h:247:3: note: in expansion of macro 'pr_err'
     247 |   function(__VA_ARGS__);    \
         |   ^~~~~~~~
   include/linux/net.h:257:2: note: in expansion of macro 'net_ratelimited_function'
     257 |  net_ratelimited_function(pr_err, fmt, ##__VA_ARGS__)
         |  ^~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/google/gve/gve_tx_dqo.c:893:3: note: in expansion of macro 'net_err_ratelimited'
     893 |   net_err_ratelimited("%s: No reinjection completion was received for: %ld.\n",
         |   ^~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/google/gve/gve_tx_dqo.c:893:74: note: format string is defined here
     893 |   net_err_ratelimited("%s: No reinjection completion was received for: %ld.\n",
         |                                                                        ~~^
         |                                                                          |
         |                                                                          long int
         |                                                                        %d


vim +/index +730 drivers/net/ethernet/google/gve/gve_tx_dqo.c

   447	
   448	/* Returns 0 on success, or < 0 on error.
   449	 *
   450	 * Before this function is called, the caller must ensure
   451	 * gve_has_pending_packet(tx) returns true.
   452	 */
   453	static int gve_tx_add_skb_no_copy_dqo(struct gve_tx_ring *tx,
   454					      struct sk_buff *skb)
   455	{
   456		const struct skb_shared_info *shinfo = skb_shinfo(skb);
   457		const bool is_gso = skb_is_gso(skb);
   458		u32 desc_idx = tx->dqo_tx.tail;
   459	
   460		struct gve_tx_pending_packet_dqo *pending_packet;
   461		struct gve_tx_metadata_dqo metadata;
   462		s16 completion_tag;
   463		int i;
   464	
   465		pending_packet = gve_alloc_pending_packet(tx);
   466		pending_packet->skb = skb;
   467		pending_packet->num_bufs = 0;
   468		completion_tag = pending_packet - tx->dqo.pending_packets;
   469	
   470		gve_extract_tx_metadata_dqo(skb, &metadata);
   471		if (is_gso) {
   472			int header_len = gve_prep_tso(skb);
   473	
   474			if (unlikely(header_len < 0))
   475				goto err;
   476	
   477			gve_tx_fill_tso_ctx_desc(&tx->dqo.tx_ring[desc_idx].tso_ctx,
   478						 skb, &metadata, header_len);
   479			desc_idx = (desc_idx + 1) & tx->mask;
   480		}
   481	
   482		gve_tx_fill_general_ctx_desc(&tx->dqo.tx_ring[desc_idx].general_ctx,
   483					     &metadata);
   484		desc_idx = (desc_idx + 1) & tx->mask;
   485	
   486		/* Note: HW requires that the size of a non-TSO packet be within the
   487		 * range of [17, 9728].
   488		 *
   489		 * We don't double check because
   490		 * - We limited `netdev->min_mtu` to ETH_MIN_MTU.
   491		 * - Hypervisor won't allow MTU larger than 9216.
   492		 */
   493	
   494		/* Map the linear portion of skb */
   495		{
   496			struct gve_tx_dma_buf *buf =
   497				&pending_packet->bufs[pending_packet->num_bufs];
   498			u32 len = skb_headlen(skb);
   499			dma_addr_t addr;
   500	
   501			addr = dma_map_single(tx->dev, skb->data, len, DMA_TO_DEVICE);
   502			if (unlikely(dma_mapping_error(tx->dev, addr)))
   503				goto err;
   504	
   505			dma_unmap_len_set(buf, len, len);
   506			dma_unmap_addr_set(buf, dma, addr);
   507			++pending_packet->num_bufs;
   508	
   509			gve_tx_fill_pkt_desc_dqo(tx, &desc_idx, skb, len, addr,
   510						 completion_tag,
   511						 /*eop=*/shinfo->nr_frags == 0, is_gso);
   512		}
   513	
   514		for (i = 0; i < shinfo->nr_frags; i++) {
   515			struct gve_tx_dma_buf *buf =
   516				&pending_packet->bufs[pending_packet->num_bufs];
   517			const skb_frag_t *frag = &shinfo->frags[i];
   518			bool is_eop = i == (shinfo->nr_frags - 1);
   519			u32 len = skb_frag_size(frag);
   520			dma_addr_t addr;
   521	
   522			addr = skb_frag_dma_map(tx->dev, frag, 0, len, DMA_TO_DEVICE);
   523			if (unlikely(dma_mapping_error(tx->dev, addr)))
   524				goto err;
   525	
   526			dma_unmap_len_set(buf, len, len);
   527			dma_unmap_addr_set(buf, dma, addr);
   528			++pending_packet->num_bufs;
   529	
   530			gve_tx_fill_pkt_desc_dqo(tx, &desc_idx, skb, len, addr,
   531						 completion_tag, is_eop, is_gso);
   532		}
   533	
   534		/* Commit the changes to our state */
   535		tx->dqo_tx.tail = desc_idx;
   536	
   537		/* Request a descriptor completion on the last descriptor of the
   538		 * packet if we are allowed to by the HW enforced interval.
   539		 */
   540		{
   541			u32 last_desc_idx = (desc_idx - 1) & tx->mask;
   542			u32 last_report_event_interval =
   543				(last_desc_idx - tx->dqo_tx.last_re_idx) & tx->mask;
   544	
   545			if (unlikely(last_report_event_interval >=
   546				     GVE_TX_MIN_RE_INTERVAL)) {
   547				tx->dqo.tx_ring[last_desc_idx].pkt.report_event = true;
   548				tx->dqo_tx.last_re_idx = last_desc_idx;
   549			}
   550		}
   551	
   552		return 0;
   553	
   554	err:
   555		for (i = 0; i < pending_packet->num_bufs; i++) {
 > 556			struct gve_tx_dma_buf *buf = &pending_packet->bufs[i];
   557	
   558			if (i == 0) {
   559				dma_unmap_single(tx->dev, dma_unmap_addr(buf, dma),
   560						 dma_unmap_len(buf, len),
   561						 DMA_TO_DEVICE);
   562			} else {
   563				dma_unmap_page(tx->dev, dma_unmap_addr(buf, dma),
   564					       dma_unmap_len(buf, len), DMA_TO_DEVICE);
   565			}
   566		}
   567	
   568		pending_packet->skb = NULL;
   569		pending_packet->num_bufs = 0;
   570		gve_free_pending_packet(tx, pending_packet);
   571	
   572		return -1;
   573	}
   574	
   575	static int gve_num_descs_per_buf(size_t size)
   576	{
   577		return DIV_ROUND_UP(size, GVE_TX_MAX_BUF_SIZE_DQO);
   578	}
   579	
   580	static int gve_num_buffer_descs_needed(const struct sk_buff *skb)
   581	{
   582		const struct skb_shared_info *shinfo = skb_shinfo(skb);
   583		int num_descs;
   584		int i;
   585	
   586		num_descs = gve_num_descs_per_buf(skb_headlen(skb));
   587	
   588		for (i = 0; i < shinfo->nr_frags; i++) {
   589			unsigned int frag_size = skb_frag_size(&shinfo->frags[i]);
   590	
   591			num_descs += gve_num_descs_per_buf(frag_size);
   592		}
   593	
   594		return num_descs;
   595	}
   596	
   597	/* Returns true if HW is capable of sending TSO represented by `skb`.
   598	 *
   599	 * Each segment must not span more than GVE_TX_MAX_DATA_DESCS buffers.
   600	 * - The header is counted as one buffer for every single segment.
   601	 * - A buffer which is split between two segments is counted for both.
   602	 * - If a buffer contains both header and payload, it is counted as two buffers.
   603	 */
   604	static bool gve_can_send_tso(const struct sk_buff *skb)
   605	{
   606		const int header_len = skb_checksum_start_offset(skb) + tcp_hdrlen(skb);
   607		const int max_bufs_per_seg = GVE_TX_MAX_DATA_DESCS - 1;
   608		const struct skb_shared_info *shinfo = skb_shinfo(skb);
   609		const int gso_size = shinfo->gso_size;
   610		int cur_seg_num_bufs;
   611		int cur_seg_size;
   612		int i;
   613	
   614		cur_seg_size = skb_headlen(skb) - header_len;
   615		cur_seg_num_bufs = cur_seg_size > 0;
   616	
   617		for (i = 0; i < shinfo->nr_frags; i++) {
   618			if (cur_seg_size >= gso_size) {
   619				cur_seg_size %= gso_size;
   620				cur_seg_num_bufs = cur_seg_size > 0;
   621			}
   622	
   623			if (unlikely(++cur_seg_num_bufs > max_bufs_per_seg))
   624				return false;
   625	
   626			cur_seg_size += skb_frag_size(&shinfo->frags[i]);
   627		}
   628	
   629		return true;
   630	}
   631	
   632	/* Attempt to transmit specified SKB.
   633	 *
   634	 * Returns 0 if the SKB was transmitted or dropped.
   635	 * Returns -1 if there is not currently enough space to transmit the SKB.
   636	 */
   637	static int gve_try_tx_skb(struct gve_priv *priv, struct gve_tx_ring *tx,
   638				  struct sk_buff *skb)
   639	{
   640		int num_buffer_descs;
   641		int total_num_descs;
   642	
   643		if (skb_is_gso(skb)) {
   644			/* If TSO doesn't meet HW requirements, attempt to linearize the
   645			 * packet.
   646			 */
   647			if (unlikely(!gve_can_send_tso(skb) &&
   648				     skb_linearize(skb) < 0)) {
   649				net_err_ratelimited("%s: Failed to transmit TSO packet\n",
   650						    priv->dev->name);
   651				goto drop;
   652			}
   653	
   654			num_buffer_descs = gve_num_buffer_descs_needed(skb);
   655		} else {
   656			num_buffer_descs = gve_num_buffer_descs_needed(skb);
   657	
   658			if (unlikely(num_buffer_descs > GVE_TX_MAX_DATA_DESCS)) {
   659				if (unlikely(skb_linearize(skb) < 0))
   660					goto drop;
   661	
   662				num_buffer_descs = 1;
   663			}
   664		}
   665	
   666		/* Metadata + (optional TSO) + data descriptors. */
   667		total_num_descs = 1 + skb_is_gso(skb) + num_buffer_descs;
   668		if (unlikely(gve_maybe_stop_tx_dqo(tx, total_num_descs +
   669				GVE_TX_MIN_DESC_PREVENT_CACHE_OVERLAP))) {
   670			return -1;
   671		}
   672	
   673		if (unlikely(gve_tx_add_skb_no_copy_dqo(tx, skb) < 0))
   674			goto drop;
   675	
   676		netdev_tx_sent_queue(tx->netdev_txq, skb->len);
   677		skb_tx_timestamp(skb);
   678		return 0;
   679	
   680	drop:
   681		tx->dropped_pkt++;
   682		dev_kfree_skb_any(skb);
   683		return 0;
   684	}
   685	
   686	/* Transmit a given skb and ring the doorbell. */
   687	netdev_tx_t gve_tx_dqo(struct sk_buff *skb, struct net_device *dev)
   688	{
   689		struct gve_priv *priv = netdev_priv(dev);
   690		struct gve_tx_ring *tx;
   691	
   692		tx = &priv->tx[skb_get_queue_mapping(skb)];
   693		if (unlikely(gve_try_tx_skb(priv, tx, skb) < 0)) {
   694			/* We need to ring the txq doorbell -- we have stopped the Tx
   695			 * queue for want of resources, but prior calls to gve_tx()
   696			 * may have added descriptors without ringing the doorbell.
   697			 */
   698			gve_tx_put_doorbell_dqo(priv, tx->q_resources, tx->dqo_tx.tail);
   699			return NETDEV_TX_BUSY;
   700		}
   701	
   702		if (!netif_xmit_stopped(tx->netdev_txq) && netdev_xmit_more())
   703			return NETDEV_TX_OK;
   704	
   705		gve_tx_put_doorbell_dqo(priv, tx->q_resources, tx->dqo_tx.tail);
   706		return NETDEV_TX_OK;
   707	}
   708	
   709	static void add_to_list(struct gve_tx_ring *tx, struct gve_index_list *list,
   710				struct gve_tx_pending_packet_dqo *pending_packet)
   711	{
   712		s16 old_tail, index;
   713	
   714		index = pending_packet - tx->dqo.pending_packets;
   715		old_tail = list->tail;
   716		list->tail = index;
   717		if (old_tail == -1)
   718			list->head = index;
   719		else
   720			tx->dqo.pending_packets[old_tail].next = index;
   721	
   722		pending_packet->next = -1;
   723		pending_packet->prev = old_tail;
   724	}
   725	
   726	static void remove_from_list(struct gve_tx_ring *tx,
   727				     struct gve_index_list *list,
   728				     struct gve_tx_pending_packet_dqo *pending_packet)
   729	{
 > 730		s16 index, prev_index, next_index;
   731	
   732		index = pending_packet - tx->dqo.pending_packets;
   733		prev_index = pending_packet->prev;
   734		next_index = pending_packet->next;
   735	
   736		if (prev_index == -1) {
   737			/* Node is head */
   738			list->head = next_index;
   739		} else {
   740			tx->dqo.pending_packets[prev_index].next = next_index;
   741		}
   742		if (next_index == -1) {
   743			/* Node is tail */
   744			list->tail = prev_index;
   745		} else {
   746			tx->dqo.pending_packets[next_index].prev = prev_index;
   747		}
   748	}
   749	
   750	static void gve_unmap_packet(struct device *dev,
   751				     struct gve_tx_pending_packet_dqo *pending_packet)
   752	{
 > 753		struct gve_tx_dma_buf *buf;
   754		int i;
   755	
   756		/* SKB linear portion is guaranteed to be mapped */
   757		buf = &pending_packet->bufs[0];
   758		dma_unmap_single(dev, dma_unmap_addr(buf, dma),
   759				 dma_unmap_len(buf, len), DMA_TO_DEVICE);
   760		for (i = 1; i < pending_packet->num_bufs; i++) {
   761			buf = &pending_packet->bufs[i];
   762			dma_unmap_page(dev, dma_unmap_addr(buf, dma),
   763				       dma_unmap_len(buf, len), DMA_TO_DEVICE);
   764		}
   765		pending_packet->num_bufs = 0;
   766	}
   767	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 48305 bytes --]