Re: [dpdk-dev] [PATCH v2 2/4] net/mrvl: add mrvl net pmd driver

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 28 Sep 2017 12:22:36 +0200
Tomasz Duszynski <tdu@semihalf.com> wrote:

> +
> +struct mrvl_rxq;
> +struct mrvl_txq;

These forward decl should not be nececessary
> +static inline int
> +mrvl_get_bpool_size(int	pp2_id, int pool_id)
 No tab here please

Why does this need to be inlined?  Is it in critical path?


> +{
> +	int i;
> +	int size = 0;
> +
> +	for (i = mrvl_lcore_first; i <= mrvl_lcore_last; i++)
> +		size += mrvl_port_bpool_size[pp2_id][pool_id][i];
> +
> +	return size;
> +}
> +

Also, I prefer that the following restrictions from the kernel be
also applied to DPDK code.

### [dpdk-dev] [PATCH v2 2/4] net/mrvl: add mrvl net pmd driver

CHECK:LINE_SPACING: Please don't use multiple blank lines
#452: FILE: drivers/net/mrvl/mrvl_ethdev.c:180:
+
+

WARNING:LINE_SPACING: Missing a blank line after declarations
#457: FILE: drivers/net/mrvl/mrvl_ethdev.c:185:
+	int n = sizeof(*bitmap) * 8 - __builtin_clz(*bitmap);
+	if (n >= max)

CHECK:LINE_SPACING: Please don't use multiple blank lines
#562: FILE: drivers/net/mrvl/mrvl_ethdev.c:290:
+
+

WARNING:MULTILINE_DEREFERENCE: Avoid multiple line dereference - prefer 'priv->ppio_params.inqs_params.tcs_params[i].inqs_params'
#880: FILE: drivers/net/mrvl/mrvl_ethdev.c:608:
+			rte_free(priv->ppio_params.inqs_params.
+				tcs_params[i].inqs_params);

WARNING:MULTILINE_DEREFERENCE: Avoid multiple line dereference - prefer 'priv->ppio_params.inqs_params.tcs_params[i].inqs_params'
#882: FILE: drivers/net/mrvl/mrvl_ethdev.c:610:
+			priv->ppio_params.inqs_params.
+				tcs_params[i].inqs_params = NULL;

WARNING:MULTILINE_DEREFERENCE: Avoid multiple line dereference - prefer 'priv->ppio_params.inqs_params.tcs_params[tc].inqs_params[inq].size'
#1330: FILE: drivers/net/mrvl/mrvl_ethdev.c:1058:
+	qinfo->nb_desc = priv->ppio_params.inqs_params.
+			 tcs_params[tc].inqs_params[inq].size;

WARNING:SPLIT_STRING: quoted string split across lines
#1476: FILE: drivers/net/mrvl/mrvl_ethdev.c:1204:
+		RTE_LOG(ERR, PMD, "Mbuf size must be increased to %u bytes"
+				  " to hold up to %u bytes of data.\n",

WARNING:MULTILINE_DEREFERENCE: Avoid multiple line dereference - prefer 'priv->ppio_params.inqs_params.tcs_params[priv->rxq_map[rxq->queue_id].tc'
#1500: FILE: drivers/net/mrvl/mrvl_ethdev.c:1228:
+	priv->ppio_params.inqs_params.
+		tcs_params[priv->rxq_map[rxq->queue_id].tc].

WARNING:MULTILINE_DEREFERENCE: Avoid multiple line dereference - prefer 'q->priv->ppio_params.inqs_params.tcs_params[q->priv->rxq_map[q->queue_id].tc'
#1532: FILE: drivers/net/mrvl/mrvl_ethdev.c:1260:
+	num = q->priv->ppio_params.inqs_params.
+			tcs_params[q->priv->rxq_map[q->queue_id].tc].

WARNING:SPLIT_STRING: quoted string split across lines
#1902: FILE: drivers/net/mrvl/mrvl_ethdev.c:1630:
+			RTE_LOG(DEBUG, PMD, "\nport-%d:%d: bpool %d oversize -"
+				" remove %d buffers (pool size: %d -> %d)\n",

WARNING:SPLIT_STRING: quoted string split across lines
#2094: FILE: drivers/net/mrvl/mrvl_ethdev.c:1822:
+			"No room in shadow queue for %d packets!!!"
+			"%d packets will be sent.\n",

CHECK:LINE_SPACING: Please don't use multiple blank lines
#2294: FILE: drivers/net/mrvl/mrvl_ethdev.c:2022:
+
+

CHECK:LINE_SPACING: Please don't use multiple blank lines
#2595: FILE: drivers/net/mrvl/mrvl_ethdev.h:40:
+
+

WARNING:LINE_SPACING: Missing a blank line after declarations
#3253: FILE: drivers/net/mrvl/mrvl_qos.c:577:
+			uint8_t idx = port_cfg->tc[tc].inq[i];
+			priv->rxq_map[idx].tc = tc;