diff mbox series

[2/2] net/netvsc: introduce driver parameter to control the use of external mbuf on receiving data

Message ID 1603395970-26797-2-git-send-email-longli@linuxonhyperv.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series [1/2] net/netvsc: allow setting rx and tx copy break |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Long Li Oct. 22, 2020, 7:46 p.m. UTC 
  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(-)

Comments

Ferruh Yigit Oct. 23, 2020, 4:23 p.m. UTC | #1 
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.
diff mbox series

Patch

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;