DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] net/netvsc: allow setting rx and tx copy break
@ 2020-10-22 19:46 Long Li
  2020-10-22 19:46 ` [dpdk-dev] [PATCH 2/2] net/netvsc: introduce driver parameter to control the use of external mbuf on receiving data Long Li
  2020-10-23 16:21 ` [dpdk-dev] [PATCH 1/2] net/netvsc: allow setting rx and tx copy break Ferruh Yigit
  0 siblings, 2 replies; 5+ messages in thread
From: Long Li @ 2020-10-22 19:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Stephen Hemminger, Long Li

From: Stephen Hemminger <stephen@networkplumber.org>

The values for Rx and Tx copy break should be tunable rather
than hard coded constants.

The rx_copybreak sets the threshold where the driver uses an
external mbuf to avoid having to copy data. Setting 0 for copybreak
will cause driver to always create an external mbuf. Setting
a value greater than the MTU would prevent it from ever making
an external mbuf and always copy. The default value is 256 (bytes).

Likewise the tx_copybreak sets the threshold where the driver
aggregates multiple small packets into one request. If tx_copybreak
is 0 then each packet goes as a VMBus request (no copying).
If tx_copybreak is set larger than the MTU, then all packets smaller
than the chunk size of the VMBus send buffer will be copied; larger
packets always have to go as a single direct request. The default
value is 512 (bytes).

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Long Li <longli@microsoft.com>
---
 drivers/net/netvsc/hn_ethdev.c | 63 +++++++++++++++++++---------------
 drivers/net/netvsc/hn_rxtx.c   |  7 ++--
 drivers/net/netvsc/hn_var.h    |  5 +++
 3 files changed, 43 insertions(+), 32 deletions(-)

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index 4a01f1d464..e4f13b962c 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -45,6 +45,10 @@
 			    DEV_RX_OFFLOAD_VLAN_STRIP | \
 			    DEV_RX_OFFLOAD_RSS_HASH)
 
+#define NETVSC_ARG_LATENCY "latency"
+#define NETVSC_ARG_RXBREAK "rx_copybreak"
+#define NETVSC_ARG_TXBREAK "tx_copybreak"
+
 struct hn_xstats_name_off {
 	char name[RTE_ETH_XSTATS_NAME_SIZE];
 	unsigned int offset;
@@ -136,38 +140,20 @@ eth_dev_vmbus_release(struct rte_eth_dev *eth_dev)
 	eth_dev->intr_handle = NULL;
 }
 
-/* handle "latency=X" from devargs */
-static int hn_set_latency(const char *key, const char *value, void *opaque)
-{
-	struct hn_data *hv = opaque;
-	char *endp = NULL;
-	unsigned long lat;
-
-	errno = 0;
-	lat = strtoul(value, &endp, 0);
-
-	if (*value == '\0' || *endp != '\0') {
-		PMD_DRV_LOG(ERR, "invalid parameter %s=%s", key, value);
-		return -EINVAL;
-	}
-
-	PMD_DRV_LOG(DEBUG, "set latency %lu usec", lat);
-
-	hv->latency = lat * 1000;	/* usec to nsec */
-	return 0;
-}
-
 /* Parse device arguments */
 static int hn_parse_args(const struct rte_eth_dev *dev)
 {
 	struct hn_data *hv = dev->data->dev_private;
 	struct rte_devargs *devargs = dev->device->devargs;
 	static const char * const valid_keys[] = {
-		"latency",
+		NETVSC_ARG_LATENCY,
+		NETVSC_ARG_RXBREAK,
+		NETVSC_ARG_TXBREAK,
 		NULL
 	};
+	int latency = -1, rx_break = -1, tx_break = -1;
 	struct rte_kvargs *kvlist;
-	int ret;
+	unsigned int i;
 
 	if (!devargs)
 		return 0;
@@ -181,12 +167,32 @@ static int hn_parse_args(const struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
-	ret = rte_kvargs_process(kvlist, "latency", hn_set_latency, hv);
-	if (ret)
-		PMD_DRV_LOG(ERR, "Unable to process latency arg\n");
+	for (i = 0; i != kvlist->count; ++i) {
+		const struct rte_kvargs_pair *pair = &kvlist->pairs[i];
+
+		if (!strcmp(pair->key, NETVSC_ARG_LATENCY))
+			latency = atoi(pair->value);
+		else if (!strcmp(pair->key, NETVSC_ARG_RXBREAK))
+			rx_break = atoi(pair->value);
+		else if (!strcmp(pair->key, NETVSC_ARG_TXBREAK))
+			tx_break = atoi(pair->value);
+	}
+
+	if (latency >= 0) {
+		PMD_DRV_LOG(DEBUG, "set latency %d usec", latency);
+		hv->latency = latency * 1000;	/* usec to nsec */
+	}
+	if (rx_break >= 0) {
+		PMD_DRV_LOG(DEBUG, "rx copy break set to %d", rx_break);
+		hv->rx_copybreak = rx_break;
+	}
+	if (tx_break >= 0) {
+		PMD_DRV_LOG(DEBUG, "tx copy break set to %d", tx_break);
+		hv->tx_copybreak = tx_break;
+	}
 
 	rte_kvargs_free(kvlist);
-	return ret;
+	return 0;
 }
 
 /* Update link status.
@@ -966,7 +972,10 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
 	hv->chim_res  = &vmbus->resource[HV_SEND_BUF_MAP];
 	hv->port_id = eth_dev->data->port_id;
 	hv->latency = HN_CHAN_LATENCY_NS;
+	hv->rx_copybreak = HN_RXCOPY_THRESHOLD;
+	hv->tx_copybreak = HN_TXCOPY_THRESHOLD;
 	hv->max_queues = 1;
+
 	rte_rwlock_init(&hv->vf_lock);
 	hv->vf_port = HN_INVALID_PORT;
 
diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c
index 5d59db513c..b62c0b0181 100644
--- a/drivers/net/netvsc/hn_rxtx.c
+++ b/drivers/net/netvsc/hn_rxtx.c
@@ -40,9 +40,6 @@
 	(sizeof(struct vmbus_chanpkt_hdr) + sizeof(struct hn_nvs_rndis))
 
 #define HN_TXD_CACHE_SIZE	32 /* per cpu tx_descriptor pool cache */
-#define HN_TXCOPY_THRESHOLD	512
-
-#define HN_RXCOPY_THRESHOLD	256
 #define HN_RXQ_EVENT_DEFAULT	2048
 
 struct hn_rxinfo {
@@ -568,7 +565,7 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb,
 	 * For large packets, avoid copy if possible but need to keep
 	 * some space available in receive area for later packets.
 	 */
-	if (dlen >= HN_RXCOPY_THRESHOLD &&
+	if (dlen > hv->rx_copybreak &&
 	    (uint32_t)rte_atomic32_read(&rxq->rxbuf_outstanding) <
 			hv->rxbuf_section_cnt / 2) {
 		struct rte_mbuf_ext_shared_info *shinfo;
@@ -1516,7 +1513,7 @@ hn_xmit_pkts(void *ptxq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 			break;
 
 		/* For small packets aggregate them in chimney buffer */
-		if (m->pkt_len < HN_TXCOPY_THRESHOLD && pkt_size <= txq->agg_szmax) {
+		if (m->pkt_len <= hv->tx_copybreak && pkt_size <= txq->agg_szmax) {
 			/* If this packet will not fit, then flush  */
 			if (txq->agg_pktleft == 0 ||
 			    RTE_ALIGN(pkt_size, txq->agg_align) > txq->agg_szleft) {
diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h
index 56bfa972d1..c1bdafb413 100644
--- a/drivers/net/netvsc/hn_var.h
+++ b/drivers/net/netvsc/hn_var.h
@@ -23,6 +23,9 @@
 /* Host monitor interval */
 #define HN_CHAN_LATENCY_NS	50000
 
+#define HN_TXCOPY_THRESHOLD	512
+#define HN_RXCOPY_THRESHOLD	256
+
 /* Buffers need to be aligned */
 #ifndef PAGE_SIZE
 #define PAGE_SIZE 4096
@@ -114,6 +117,7 @@ struct hn_data {
 
 	struct rte_mem_resource *rxbuf_res;	/* UIO resource for Rx */
 	uint32_t	rxbuf_section_cnt;	/* # of Rx sections */
+	uint32_t	rx_copybreak;
 	uint16_t	max_queues;		/* Max available queues */
 	uint16_t	num_queues;
 	uint64_t	rss_offloads;
@@ -122,6 +126,7 @@ struct hn_data {
 	struct rte_mem_resource *chim_res;	/* UIO resource for Tx */
 	struct rte_bitmap *chim_bmap;		/* Send buffer map */
 	void		*chim_bmem;
+	uint32_t	tx_copybreak;
 	uint32_t	chim_szmax;		/* Max size per buffer */
 	uint32_t	chim_cnt;		/* Max packets per buffer */
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [dpdk-dev] [PATCH 2/2] net/netvsc: introduce driver parameter to control the use of external mbuf on receiving data
  2020-10-22 19:46 [dpdk-dev] [PATCH 1/2] net/netvsc: allow setting rx and tx copy break Long Li
@ 2020-10-22 19:46 ` Long Li
  2020-10-23 16:23   ` Ferruh Yigit
  2020-10-23 16:21 ` [dpdk-dev] [PATCH 1/2] net/netvsc: allow setting rx and tx copy break Ferruh Yigit
  1 sibling, 1 reply; 5+ messages in thread
From: Long Li @ 2020-10-22 19:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Long Li

From: Long Li <longli@microsoft.com>

When receiving packets, netvsp puts data in a buffer mapped through UIO.
Depending on packet size, netvsc may attach the buffer as an external
mbuf. This is not a problem if this mbuf is consumed in the application,
and the application can correctly read data out of an external mbuf.

However, there are two problems with data in an external mbuf.
1. Due to the limitation of the kernel UIO implementation, physical
address of this external buffer is not exposed to the user-mode. If this
mbuf is passed to another driver, the other driver is unable to map this
buffer to iova.
2. Some DPDK applications are not aware of external mbuf, and may bug when
they receive an mbuf with external buffer attached.

Introduce a driver parameter "rx_extmbuf_enable" to control if netvsc
should use external mbuf for receiving packets. The default value is 0.
(netvsc doesn't use external mbuf, it always allocates mbuf and copy data
to mbuf) A non-zero value tells netvsc to attach external buffers to mbuf
on receiving packets, thus avoid copying memory.

Signed-off-by: Long Li <longli@microsoft.com>
---
 drivers/net/netvsc/hn_ethdev.c | 11 +++++++++++
 drivers/net/netvsc/hn_rxtx.c   |  2 +-
 drivers/net/netvsc/hn_var.h    |  3 +++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index e4f13b962c..735fc6d236 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -48,6 +48,7 @@
 #define NETVSC_ARG_LATENCY "latency"
 #define NETVSC_ARG_RXBREAK "rx_copybreak"
 #define NETVSC_ARG_TXBREAK "tx_copybreak"
+#define NETVSC_ARG_RX_EXTMBUF_ENABLE "rx_extmbuf_enable"
 
 struct hn_xstats_name_off {
 	char name[RTE_ETH_XSTATS_NAME_SIZE];
@@ -149,9 +150,11 @@ static int hn_parse_args(const struct rte_eth_dev *dev)
 		NETVSC_ARG_LATENCY,
 		NETVSC_ARG_RXBREAK,
 		NETVSC_ARG_TXBREAK,
+		NETVSC_ARG_RX_EXTMBUF_ENABLE,
 		NULL
 	};
 	int latency = -1, rx_break = -1, tx_break = -1;
+	int rx_extmbuf_enable = -1;
 	struct rte_kvargs *kvlist;
 	unsigned int i;
 
@@ -176,6 +179,8 @@ static int hn_parse_args(const struct rte_eth_dev *dev)
 			rx_break = atoi(pair->value);
 		else if (!strcmp(pair->key, NETVSC_ARG_TXBREAK))
 			tx_break = atoi(pair->value);
+		else if (!strcmp(pair->key, NETVSC_ARG_RX_EXTMBUF_ENABLE))
+			rx_extmbuf_enable = atoi(pair->value);
 	}
 
 	if (latency >= 0) {
@@ -190,6 +195,11 @@ static int hn_parse_args(const struct rte_eth_dev *dev)
 		PMD_DRV_LOG(DEBUG, "tx copy break set to %d", tx_break);
 		hv->tx_copybreak = tx_break;
 	}
+	if (rx_extmbuf_enable >= 0) {
+		PMD_DRV_LOG(DEBUG, "rx ext mbuf enable set to %d",
+				rx_extmbuf_enable);
+		hv->rx_extmbuf_enable = rx_extmbuf_enable;
+	}
 
 	rte_kvargs_free(kvlist);
 	return 0;
@@ -974,6 +984,7 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
 	hv->latency = HN_CHAN_LATENCY_NS;
 	hv->rx_copybreak = HN_RXCOPY_THRESHOLD;
 	hv->tx_copybreak = HN_TXCOPY_THRESHOLD;
+	hv->rx_extmbuf_enable = HN_RX_EXTMBUF_ENABLE;
 	hv->max_queues = 1;
 
 	rte_rwlock_init(&hv->vf_lock);
diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c
index b62c0b0181..cea3157ca7 100644
--- a/drivers/net/netvsc/hn_rxtx.c
+++ b/drivers/net/netvsc/hn_rxtx.c
@@ -565,7 +565,7 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb,
 	 * For large packets, avoid copy if possible but need to keep
 	 * some space available in receive area for later packets.
 	 */
-	if (dlen > hv->rx_copybreak &&
+	if (hv->rx_extmbuf_enable && dlen > hv->rx_copybreak &&
 	    (uint32_t)rte_atomic32_read(&rxq->rxbuf_outstanding) <
 			hv->rxbuf_section_cnt / 2) {
 		struct rte_mbuf_ext_shared_info *shinfo;
diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h
index c1bdafb413..9dcb669f7a 100644
--- a/drivers/net/netvsc/hn_var.h
+++ b/drivers/net/netvsc/hn_var.h
@@ -26,6 +26,8 @@
 #define HN_TXCOPY_THRESHOLD	512
 #define HN_RXCOPY_THRESHOLD	256
 
+#define HN_RX_EXTMBUF_ENABLE	0
+
 /* Buffers need to be aligned */
 #ifndef PAGE_SIZE
 #define PAGE_SIZE 4096
@@ -118,6 +120,7 @@ struct hn_data {
 	struct rte_mem_resource *rxbuf_res;	/* UIO resource for Rx */
 	uint32_t	rxbuf_section_cnt;	/* # of Rx sections */
 	uint32_t	rx_copybreak;
+	uint32_t	rx_extmbuf_enable;
 	uint16_t	max_queues;		/* Max available queues */
 	uint16_t	num_queues;
 	uint64_t	rss_offloads;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] net/netvsc: allow setting rx and tx copy break
  2020-10-22 19:46 [dpdk-dev] [PATCH 1/2] net/netvsc: allow setting rx and tx copy break Long Li
  2020-10-22 19:46 ` [dpdk-dev] [PATCH 2/2] net/netvsc: introduce driver parameter to control the use of external mbuf on receiving data Long Li
@ 2020-10-23 16:21 ` Ferruh Yigit
  2020-10-23 19:49   ` Long Li
  1 sibling, 1 reply; 5+ messages in thread
From: Ferruh Yigit @ 2020-10-23 16:21 UTC (permalink / raw)
  To: Long Li, Stephen Hemminger; +Cc: dev, Stephen Hemminger, Long Li

On 10/22/2020 8:46 PM, Long Li wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> 
> The values for Rx and Tx copy break should be tunable rather
> than hard coded constants.
> 
> The rx_copybreak sets the threshold where the driver uses an
> external mbuf to avoid having to copy data. Setting 0 for copybreak
> will cause driver to always create an external mbuf. Setting
> a value greater than the MTU would prevent it from ever making
> an external mbuf and always copy. The default value is 256 (bytes).
> 
> Likewise the tx_copybreak sets the threshold where the driver
> aggregates multiple small packets into one request. If tx_copybreak
> is 0 then each packet goes as a VMBus request (no copying).
> If tx_copybreak is set larger than the MTU, then all packets smaller
> than the chunk size of the VMBus send buffer will be copied; larger
> packets always have to go as a single direct request. The default
> value is 512 (bytes).
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Long Li <longli@microsoft.com>

<...>

> @@ -45,6 +45,10 @@
>   			    DEV_RX_OFFLOAD_VLAN_STRIP | \
>   			    DEV_RX_OFFLOAD_RSS_HASH)
>   
> +#define NETVSC_ARG_LATENCY "latency"
> +#define NETVSC_ARG_RXBREAK "rx_copybreak"
> +#define NETVSC_ARG_TXBREAK "tx_copybreak"
> +

Can you please document new devargs in the driver documentation?

<...>

> @@ -181,12 +167,32 @@ static int hn_parse_args(const struct rte_eth_dev *dev)
>   		return -EINVAL;
>   	}
>   
> -	ret = rte_kvargs_process(kvlist, "latency", hn_set_latency, hv);
> -	if (ret)
> -		PMD_DRV_LOG(ERR, "Unable to process latency arg\n");
> +	for (i = 0; i != kvlist->count; ++i) {
> +		const struct rte_kvargs_pair *pair = &kvlist->pairs[i];
> +
> +		if (!strcmp(pair->key, NETVSC_ARG_LATENCY))
> +			latency = atoi(pair->value);
> +		else if (!strcmp(pair->key, NETVSC_ARG_RXBREAK))
> +			rx_break = atoi(pair->value);
> +		else if (!strcmp(pair->key, NETVSC_ARG_TXBREAK))
> +			tx_break = atoi(pair->value);
> +	}
> +

Instead of accessing to the kvlist internals, I think better to use 
'rte_kvargs_process()' as done previously.
If the reason to remove callback is to not create a callback for each argument,
a generic one can be used for all.

> +	if (latency >= 0) {
> +		PMD_DRV_LOG(DEBUG, "set latency %d usec", latency);
> +		hv->latency = latency * 1000;	/* usec to nsec */
> +	}
> +	if (rx_break >= 0) {
> +		PMD_DRV_LOG(DEBUG, "rx copy break set to %d", rx_break);
> +		hv->rx_copybreak = rx_break;
> +	}
> +	if (tx_break >= 0) {
> +		PMD_DRV_LOG(DEBUG, "tx copy break set to %d", tx_break);
> +		hv->tx_copybreak = tx_break;
> +	}
>   

When 'rte_kvargs_process()' used, the valued can be assigned directly to 
'hv->tx_copybreak', if the argument is not available, it won't be updated, so 
above check can be dropped.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] net/netvsc: introduce driver parameter to control the use of external mbuf on receiving data
  2020-10-22 19:46 ` [dpdk-dev] [PATCH 2/2] net/netvsc: introduce driver parameter to control the use of external mbuf on receiving data Long Li
@ 2020-10-23 16:23   ` Ferruh Yigit
  0 siblings, 0 replies; 5+ messages in thread
From: Ferruh Yigit @ 2020-10-23 16:23 UTC (permalink / raw)
  To: Long Li, Stephen Hemminger; +Cc: dev, Long Li

On 10/22/2020 8:46 PM, Long Li wrote:
> From: Long Li <longli@microsoft.com>
> 
> When receiving packets, netvsp puts data in a buffer mapped through UIO.
> Depending on packet size, netvsc may attach the buffer as an external
> mbuf. This is not a problem if this mbuf is consumed in the application,
> and the application can correctly read data out of an external mbuf.
> 
> However, there are two problems with data in an external mbuf.
> 1. Due to the limitation of the kernel UIO implementation, physical
> address of this external buffer is not exposed to the user-mode. If this
> mbuf is passed to another driver, the other driver is unable to map this
> buffer to iova.
> 2. Some DPDK applications are not aware of external mbuf, and may bug when
> they receive an mbuf with external buffer attached.
> 
> Introduce a driver parameter "rx_extmbuf_enable" to control if netvsc
> should use external mbuf for receiving packets. The default value is 0.
> (netvsc doesn't use external mbuf, it always allocates mbuf and copy data
> to mbuf) A non-zero value tells netvsc to attach external buffers to mbuf
> on receiving packets, thus avoid copying memory.
> 
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
>   drivers/net/netvsc/hn_ethdev.c | 11 +++++++++++
>   drivers/net/netvsc/hn_rxtx.c   |  2 +-
>   drivers/net/netvsc/hn_var.h    |  3 +++
>   3 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
> index e4f13b962c..735fc6d236 100644
> --- a/drivers/net/netvsc/hn_ethdev.c
> +++ b/drivers/net/netvsc/hn_ethdev.c
> @@ -48,6 +48,7 @@
>   #define NETVSC_ARG_LATENCY "latency"
>   #define NETVSC_ARG_RXBREAK "rx_copybreak"
>   #define NETVSC_ARG_TXBREAK "tx_copybreak"
> +#define NETVSC_ARG_RX_EXTMBUF_ENABLE "rx_extmbuf_enable"
>   

Same comments for with previous patch, can you please document the new devarg 
and 'rte_kvargs_process()' can be used to parse it.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] net/netvsc: allow setting rx and tx copy break
  2020-10-23 16:21 ` [dpdk-dev] [PATCH 1/2] net/netvsc: allow setting rx and tx copy break Ferruh Yigit
@ 2020-10-23 19:49   ` Long Li
  0 siblings, 0 replies; 5+ messages in thread
From: Long Li @ 2020-10-23 19:49 UTC (permalink / raw)
  To: Ferruh Yigit, Long Li, Stephen Hemminger; +Cc: dev, Stephen Hemminger

> Subject: Re: [dpdk-dev] [PATCH 1/2] net/netvsc: allow setting rx and tx copy
> break
> 
> On 10/22/2020 8:46 PM, Long Li wrote:
> > From: Stephen Hemminger <stephen@networkplumber.org>
> >
> > The values for Rx and Tx copy break should be tunable rather than hard
> > coded constants.
> >
> > The rx_copybreak sets the threshold where the driver uses an external
> > mbuf to avoid having to copy data. Setting 0 for copybreak will cause
> > driver to always create an external mbuf. Setting a value greater than
> > the MTU would prevent it from ever making an external mbuf and always
> > copy. The default value is 256 (bytes).
> >
> > Likewise the tx_copybreak sets the threshold where the driver
> > aggregates multiple small packets into one request. If tx_copybreak is
> > 0 then each packet goes as a VMBus request (no copying).
> > If tx_copybreak is set larger than the MTU, then all packets smaller
> > than the chunk size of the VMBus send buffer will be copied; larger
> > packets always have to go as a single direct request. The default
> > value is 512 (bytes).
> >
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > Signed-off-by: Long Li <longli@microsoft.com>
> 
> <...>
> 
> > @@ -45,6 +45,10 @@
> >   			    DEV_RX_OFFLOAD_VLAN_STRIP | \
> >   			    DEV_RX_OFFLOAD_RSS_HASH)
> >
> > +#define NETVSC_ARG_LATENCY "latency"
> > +#define NETVSC_ARG_RXBREAK "rx_copybreak"
> > +#define NETVSC_ARG_TXBREAK "tx_copybreak"
> > +
> 
> Can you please document new devargs in the driver documentation?
> 
> <...>
> 
> > @@ -181,12 +167,32 @@ static int hn_parse_args(const struct rte_eth_dev
> *dev)
> >   		return -EINVAL;
> >   	}
> >
> > -	ret = rte_kvargs_process(kvlist, "latency", hn_set_latency, hv);
> > -	if (ret)
> > -		PMD_DRV_LOG(ERR, "Unable to process latency arg\n");
> > +	for (i = 0; i != kvlist->count; ++i) {
> > +		const struct rte_kvargs_pair *pair = &kvlist->pairs[i];
> > +
> > +		if (!strcmp(pair->key, NETVSC_ARG_LATENCY))
> > +			latency = atoi(pair->value);
> > +		else if (!strcmp(pair->key, NETVSC_ARG_RXBREAK))
> > +			rx_break = atoi(pair->value);
> > +		else if (!strcmp(pair->key, NETVSC_ARG_TXBREAK))
> > +			tx_break = atoi(pair->value);
> > +	}
> > +
> 
> Instead of accessing to the kvlist internals, I think better to use
> 'rte_kvargs_process()' as done previously.
> If the reason to remove callback is to not create a callback for each argument, a
> generic one can be used for all.
> 
> > +	if (latency >= 0) {
> > +		PMD_DRV_LOG(DEBUG, "set latency %d usec", latency);
> > +		hv->latency = latency * 1000;	/* usec to nsec */
> > +	}
> > +	if (rx_break >= 0) {
> > +		PMD_DRV_LOG(DEBUG, "rx copy break set to %d", rx_break);
> > +		hv->rx_copybreak = rx_break;
> > +	}
> > +	if (tx_break >= 0) {
> > +		PMD_DRV_LOG(DEBUG, "tx copy break set to %d", tx_break);
> > +		hv->tx_copybreak = tx_break;
> > +	}
> >
> 
> When 'rte_kvargs_process()' used, the valued can be assigned directly to 'hv-
> >tx_copybreak', if the argument is not available, it won't be updated, so above
> check can be dropped.

Thanks Ferruh, I will send V2 to address comments.

Long

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-10-23 19:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-22 19:46 [dpdk-dev] [PATCH 1/2] net/netvsc: allow setting rx and tx copy break Long Li
2020-10-22 19:46 ` [dpdk-dev] [PATCH 2/2] net/netvsc: introduce driver parameter to control the use of external mbuf on receiving data Long Li
2020-10-23 16:23   ` Ferruh Yigit
2020-10-23 16:21 ` [dpdk-dev] [PATCH 1/2] net/netvsc: allow setting rx and tx copy break Ferruh Yigit
2020-10-23 19:49   ` Long Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).