[dpdk-dev] [PATCH 0/7] virtio/vhost: Add MTU feature support

[dpdk-dev] [PATCH 0/7] virtio/vhost: Add MTU feature support

[Maxime Coquelin]

This series adds support to new Virtio's MTU feature[1]. The MTU
value is set via QEMU parameters.

If the feature is negotiated (i.e supported by both host andcguest,
and valid MTU value is set in QEMU via its host_mtu parameter), QEMU
shares the configured MTU value throught dedicated Vhost protocol
feature.

On vhost side, the value is stored in the virtio_net structure, and
made available to the application thanks to new vhost lib's
rte_vhost_mtu_get() function.

rte_vhost_mtu_set() functions is implemented, but only succeed if the
application sets the same value as the one set in QEMU. Idea is that
it would be used for the application to ensure configured MTU value
is consistent, but maybe the mtu_get() API is enough, and mtu_set()
could just be dropped. Vhost PMD mtu_set callback is also implemented
in the same spirit.

To be able to set eth_dev's MTU value at the right time, i.e. to call
rte_vhost_mtu_get() just after Virtio features have been negotiated
and before the device is really started, a new vhost flag has been
introduced (VIRTIO_DEV_READY), because the VIRTIO_DEV_RUNNING flag is
set too late (after .new_device() ops is called).

Regarding valid MTU values, the maximum MTU value accepted on vhost
side is 65535 bytes, as defined in Virtio Spec and supported in
Virtio-net Kernel driver. But in Virtio PMD, current maximum frame
size is 9728 bytes (~9700 bytes MTU). So maximum MTU size accepted in
Virtio PMD is the minimum between ~9700 bytes and host's MTU.

Initially, we thought about disabling the rx-mergeable feature when
MTU value was low enough to ensure all received packets would fit in
receive buffers (when offloads are disabled). Doing this, we would
save one cache-miss in the receive path. Problem is that we don't
know the buffers size at Virtio feature neogotiation time.
It might be possible for the application to call the configure
callback again once the Rx queue is set up, but it seems a bit hacky.
So I decided to skip this optimization for now, even if feedback and
are of course appreciated.

Finally, this series also adds MTU value printing  in testpmd's
"show port info" command when non-zero.

This series target v17.05 release.

Cheers,
Maxime

Maxime Coquelin (7):
  vhost: Enable VIRTIO_NET_F_MTU feature
  vhost: vhost-user: Add MTU protocol feature support
  vhost: Add new ready status flag
  vhost: Add API to get/set MTU value
  net/vhost: Implement mtu_set callback
  net/virtio: Add MTU feature support
  app/testpmd: print MTU value in show port info

 app/test-pmd/config.c               |  5 +++++
 doc/guides/nics/features/vhost.ini  |  1 +
 doc/guides/nics/features/virtio.ini |  1 +
 drivers/net/vhost/rte_eth_vhost.c   | 18 +++++++++++++++
 drivers/net/virtio/virtio_ethdev.c  | 22 +++++++++++++++++--
 drivers/net/virtio/virtio_ethdev.h  |  3 ++-
 drivers/net/virtio/virtio_pci.h     |  3 +++
 lib/librte_vhost/rte_virtio_net.h   | 31 ++++++++++++++++++++++++++
 lib/librte_vhost/vhost.c            | 42 ++++++++++++++++++++++++++++++++++-
 lib/librte_vhost/vhost.h            |  9 +++++++-
 lib/librte_vhost/vhost_user.c       | 44 +++++++++++++++++++++++++++++++------
 lib/librte_vhost/vhost_user.h       |  5 ++++-
 12 files changed, 171 insertions(+), 13 deletions(-)

-- 
2.9.3

[dpdk-dev] [PATCH 1/7] vhost: Enable VIRTIO_NET_F_MTU feature

[Maxime Coquelin]

This patch enables the new VIRTIO_NET_F_MTU feature,
which makes possible for the host to advise the guest
with its maximum supported MTU.

MTU value is set via QEMU parameters, either via Libvirt XML, or
directly in virtio-net device command line arguments.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost.c | 3 ++-
 lib/librte_vhost/vhost.h | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index e415093..3974087 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -66,7 +66,8 @@
 				(1ULL << VIRTIO_NET_F_GUEST_CSUM) | \
 				(1ULL << VIRTIO_NET_F_GUEST_TSO4) | \
 				(1ULL << VIRTIO_NET_F_GUEST_TSO6) | \
-				(1ULL << VIRTIO_RING_F_INDIRECT_DESC))
+				(1ULL << VIRTIO_RING_F_INDIRECT_DESC) | \
+				(1ULL << VIRTIO_NET_F_MTU))
 
 uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;
 
diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 22564f1..6a57bb3 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -110,11 +110,15 @@ struct vhost_virtqueue {
 	uint16_t                shadow_used_idx;
 } __rte_cache_aligned;
 
-/* Old kernels have no such macro defined */
+/* Old kernels have no such macros defined */
 #ifndef VIRTIO_NET_F_GUEST_ANNOUNCE
  #define VIRTIO_NET_F_GUEST_ANNOUNCE 21
 #endif
 
+#ifndef VIRTIO_NET_F_MTU
+ #define VIRTIO_NET_F_MTU 3
+#endif
+
 
 /*
  * Make an extra wrapper for VIRTIO_NET_F_MQ and
-- 
2.9.3

[dpdk-dev] [PATCH 2/7] vhost: vhost-user: Add MTU protocol feature support

[Maxime Coquelin]

This patch implements the vhost-user MTU protocol feature support.
When VIRTIO_NET_F_MTU is negotiated, QEMU notifies the vhost-user
backend with the configured MTU if dedicated protocol feature is
supported.

The value can be used by the application to ensure consistency with
value set by the user.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost.h      |  1 +
 lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++++++++++
 lib/librte_vhost/vhost_user.h |  5 ++++-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 6a57bb3..549296f 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -170,6 +170,7 @@ struct virtio_net {
 	uint64_t		log_base;
 	uint64_t		log_addr;
 	struct ether_addr	mac;
+	uint16_t		mtu;
 
 	uint32_t		nr_guest_pages;
 	uint32_t		max_guest_pages;
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index cb2156a..69877a4 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -51,6 +51,9 @@
 #include "vhost.h"
 #include "vhost_user.h"
 
+#define VIRTIO_MIN_MTU 68
+#define VIRTIO_MAX_MTU 65535
+
 static const char *vhost_message_str[VHOST_USER_MAX] = {
 	[VHOST_USER_NONE] = "VHOST_USER_NONE",
 	[VHOST_USER_GET_FEATURES] = "VHOST_USER_GET_FEATURES",
@@ -72,6 +75,7 @@ static const char *vhost_message_str[VHOST_USER_MAX] = {
 	[VHOST_USER_GET_QUEUE_NUM]  = "VHOST_USER_GET_QUEUE_NUM",
 	[VHOST_USER_SET_VRING_ENABLE]  = "VHOST_USER_SET_VRING_ENABLE",
 	[VHOST_USER_SEND_RARP]  = "VHOST_USER_SEND_RARP",
+	[VHOST_USER_NET_SET_MTU]  = "VHOST_USER_NET_SET_MTU",
 };
 
 static uint64_t
@@ -865,6 +869,22 @@ vhost_user_send_rarp(struct virtio_net *dev, struct VhostUserMsg *msg)
 	return 0;
 }
 
+static int
+vhost_user_net_set_mtu(struct virtio_net *dev, struct VhostUserMsg *msg)
+{
+	if (msg->payload.u64 < VIRTIO_MIN_MTU ||
+			msg->payload.u64 > VIRTIO_MAX_MTU) {
+		RTE_LOG(ERR, VHOST_CONFIG, "Invalid MTU size (%lu)\n",
+				msg->payload.u64);
+
+		return -1;
+	}
+
+	dev->mtu = (uint16_t)msg->payload.u64;
+
+	return 0;
+}
+
 /* return bytes# of read on success or negative val on failure. */
 static int
 read_vhost_message(int sockfd, struct VhostUserMsg *msg)
@@ -1027,6 +1047,10 @@ vhost_user_msg_handler(int vid, int fd)
 		vhost_user_send_rarp(dev, &msg);
 		break;
 
+	case VHOST_USER_NET_SET_MTU:
+		ret = vhost_user_net_set_mtu(dev, &msg);
+		break;
+
 	default:
 		ret = -1;
 		break;
diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
index 179e441..838dec8 100644
--- a/lib/librte_vhost/vhost_user.h
+++ b/lib/librte_vhost/vhost_user.h
@@ -47,11 +47,13 @@
 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD	1
 #define VHOST_USER_PROTOCOL_F_RARP	2
 #define VHOST_USER_PROTOCOL_F_REPLY_ACK	3
+#define VHOST_USER_PROTOCOL_F_NET_MTU 4
 
 #define VHOST_USER_PROTOCOL_FEATURES	((1ULL << VHOST_USER_PROTOCOL_F_MQ) | \
 					 (1ULL << VHOST_USER_PROTOCOL_F_LOG_SHMFD) |\
 					 (1ULL << VHOST_USER_PROTOCOL_F_RARP) | \
-					 (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
+					 (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK) | \
+					 (1ULL << VHOST_USER_PROTOCOL_F_NET_MTU))
 
 typedef enum VhostUserRequest {
 	VHOST_USER_NONE = 0,
@@ -74,6 +76,7 @@ typedef enum VhostUserRequest {
 	VHOST_USER_GET_QUEUE_NUM = 17,
 	VHOST_USER_SET_VRING_ENABLE = 18,
 	VHOST_USER_SEND_RARP = 19,
+	VHOST_USER_NET_SET_MTU = 20,
 	VHOST_USER_MAX
 } VhostUserRequest;
 
-- 
2.9.3

[dpdk-dev] [PATCH 3/7] vhost: Add new ready status flag

[Maxime Coquelin]

This patch adds a new status flag indicating the Virtio device
is ready to operate.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost.h      |  2 ++
 lib/librte_vhost/vhost_user.c | 20 +++++++++++++-------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 549296f..e8b7e44 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -46,6 +46,8 @@
 
 /* Used to indicate that the device is running on a data core */
 #define VIRTIO_DEV_RUNNING 1
+/* Used to indicate that the device is ready to operate */
+#define VIRTIO_DEV_READY 2
 
 /* Backend value set by guest. */
 #define VIRTIO_DEV_STOPPED -1
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 69877a4..4726aaf 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -691,14 +691,18 @@ vhost_user_set_vring_kick(struct virtio_net *dev, struct VhostUserMsg *pmsg)
 		close(vq->kickfd);
 	vq->kickfd = file.fd;
 
-	if (virtio_is_ready(dev) && !(dev->flags & VIRTIO_DEV_RUNNING)) {
-		if (dev->dequeue_zero_copy) {
-			RTE_LOG(INFO, VHOST_CONFIG,
-				"dequeue zero copy is enabled\n");
-		}
+	if (virtio_is_ready(dev)) {
+		dev->flags |= VIRTIO_DEV_READY;
+
+		if (!(dev->flags & VIRTIO_DEV_RUNNING)) {
+			if (dev->dequeue_zero_copy) {
+				RTE_LOG(INFO, VHOST_CONFIG,
+						"dequeue zero copy is enabled\n");
+			}
 
-		if (notify_ops->new_device(dev->vid) == 0)
-			dev->flags |= VIRTIO_DEV_RUNNING;
+			if (notify_ops->new_device(dev->vid) == 0)
+				dev->flags |= VIRTIO_DEV_RUNNING;
+		}
 	}
 }
 
@@ -733,6 +737,8 @@ vhost_user_get_vring_base(struct virtio_net *dev,
 		notify_ops->destroy_device(dev->vid);
 	}
 
+	dev->flags &= ~VIRTIO_DEV_READY;
+
 	/* Here we are safe to get the last used index */
 	state->num = vq->last_used_idx;
 
-- 
2.9.3

[dpdk-dev] [PATCH 4/7] vhost: Add API to get/set MTU value

[Maxime Coquelin]

This patch implements two functions for the application to
get/set the MTU value.

rte_vhost_mtu_get() set the mtu parameter with the MTU value
set in QEMU if VIRTIO_NET_F_MTU has been negotiated and returns 0,
-ENOTSUP otherwise.

rte_vhost_mtu_set() doesn't actually permit to change the MTU,
which can only be set once before feature negotiation, but the
function returns 0 if the MTU value the application tries to set
is the same that have been set in QEMU, -EINVAL if different and
-ENOTSUP if VIRTIO_NET_F_MTU hasn't been negotiated.

The two functions returns -EAGAIN if Virtio feature negotiation
didn't happened yet.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/rte_virtio_net.h | 31 +++++++++++++++++++++++++++++++
 lib/librte_vhost/vhost.c          | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
index 926039c..8d84dbf 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h
@@ -100,6 +100,37 @@ int rte_vhost_driver_callback_register(struct virtio_net_device_ops const * cons
 int rte_vhost_driver_session_start(void);
 
 /**
+ * Get the MTU value of the device if set in QEMU.
+ *
+ * @param vid
+ *  virtio-net device ID
+ * @param mtu
+ *  The variable to store the MTU value
+ *
+ * @return
+ *  0: success
+ *  -EAGAIN: device not yet started
+ *  -ENOTSUP: device does not support MTU feature
+ */
+int rte_vhost_mtu_get(int vid, uint16_t *mtu);
+
+/**
+ * Set the MTU value of the device.
+ *
+ * @param vid
+ *  virtio-net device ID
+ * @param mtu
+ *  The MTU value
+ *
+ * @return
+ *  0: success
+ *  -EAGAIN: device not yet started
+ *  -ENOTSUP: device does not support MTU feature
+ *  -EINVAL: MTU value different from the one set in QEMU
+ */
+int rte_vhost_mtu_set(int vid, uint16_t mtu);
+
+/**
  * Get the numa node from which the virtio net device's memory
  * is allocated.
  *
diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index 3974087..20d0061 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -313,6 +313,45 @@ vhost_enable_dequeue_zero_copy(int vid)
 }
 
 int
+rte_vhost_mtu_get(int vid, uint16_t *mtu)
+{
+	struct virtio_net *dev = get_device(vid);
+
+	if (!dev)
+		return -ENODEV;
+
+	if (!(dev->flags & VIRTIO_DEV_READY))
+		return -EAGAIN;
+
+	if (!(dev->features & VIRTIO_NET_F_MTU))
+		return -ENOTSUP;
+
+	*mtu = dev->mtu;
+
+	return 0;
+}
+
+int
+rte_vhost_mtu_set(int vid, uint16_t mtu)
+{
+	struct virtio_net *dev = get_device(vid);
+
+	if (!dev)
+		return -ENODEV;
+
+	if (!(dev->flags & VIRTIO_DEV_READY))
+		return -EAGAIN;
+
+	if (!(dev->features & VIRTIO_NET_F_MTU))
+		return -ENOTSUP;
+
+	if (dev->mtu != mtu)
+		return -EINVAL;
+
+	return 0;
+}
+
+int
 rte_vhost_get_numa_node(int vid)
 {
 #ifdef RTE_LIBRTE_VHOST_NUMA
-- 
2.9.3

[dpdk-dev] [PATCH 5/7] net/vhost: Implement mtu_set callback

[Maxime Coquelin]

This patch implements the eth_dev's mtu_set callback.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 doc/guides/nics/features/vhost.ini |  1 +
 drivers/net/vhost/rte_eth_vhost.c  | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
index 23166fb..590dadb 100644
--- a/doc/guides/nics/features/vhost.ini
+++ b/doc/guides/nics/features/vhost.ini
@@ -11,3 +11,4 @@ Basic stats          = Y
 Extended stats       = Y
 x86-32               = Y
 x86-64               = Y
+MTU update           = Y
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index e98cffd..75feef1 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -575,6 +575,8 @@ new_device(int vid)
 	for (i = 0; i < rte_vhost_get_queue_num(vid) * VIRTIO_QNUM; i++)
 		rte_vhost_enable_guest_notification(vid, i, 0);
 
+	rte_vhost_mtu_get(vid, &eth_dev->data->mtu);
+
 	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
 
 	rte_atomic32_set(&internal->dev_attached, 1);
@@ -966,6 +968,21 @@ eth_link_update(struct rte_eth_dev *dev __rte_unused,
 	return 0;
 }
 
+static int
+eth_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+	if (dev->data->dev_link.link_status != ETH_LINK_UP)
+		return -EAGAIN;
+
+	if (!dev->data->mtu)
+		return -ENOTSUP;
+
+	if (dev->data->mtu != mtu)
+		return -EINVAL;
+
+	return 0;
+}
+
 /**
  * Disable features in feature_mask. Returns 0 on success.
  */
@@ -1002,6 +1019,7 @@ static const struct eth_dev_ops ops = {
 	.rx_queue_release = eth_queue_release,
 	.tx_queue_release = eth_queue_release,
 	.link_update = eth_link_update,
+	.mtu_set = eth_mtu_set,
 	.stats_get = eth_stats_get,
 	.stats_reset = eth_stats_reset,
 	.xstats_reset = vhost_dev_xstats_reset,
-- 
2.9.3

[dpdk-dev] [PATCH 6/7] net/virtio: Add MTU feature support

[Maxime Coquelin]

This patch implements support for the Virtio MTU feature.
When negotiated, the host shares its maximum supported MTU,
which is used as initial MTU and as maximum MTU the application
can set.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 doc/guides/nics/features/virtio.ini |  1 +
 drivers/net/virtio/virtio_ethdev.c  | 22 ++++++++++++++++++++--
 drivers/net/virtio/virtio_ethdev.h  |  3 ++-
 drivers/net/virtio/virtio_pci.h     |  3 +++
 4 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/doc/guides/nics/features/virtio.ini b/doc/guides/nics/features/virtio.ini
index 5164937..7bea075 100644
--- a/doc/guides/nics/features/virtio.ini
+++ b/doc/guides/nics/features/virtio.ini
@@ -24,3 +24,4 @@ ARMv8                = Y
 x86-32               = Y
 x86-64               = Y
 Usage doc            = Y
+MTU update           = Y
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d1ff234..ad3e6e1 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -721,10 +721,13 @@ virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_LEN +
 				 hw->vtnet_hdr_size;
 	uint32_t frame_size = mtu + ether_hdr_len;
+	uint32_t max_frame_size = hw->max_mtu + ether_hdr_len;
 
-	if (mtu < ETHER_MIN_MTU || frame_size > VIRTIO_MAX_RX_PKTLEN) {
+	max_frame_size = RTE_MIN(max_frame_size, VIRTIO_MAX_RX_PKTLEN);
+
+	if (mtu < ETHER_MIN_MTU || frame_size > max_frame_size) {
 		PMD_INIT_LOG(ERR, "MTU should be between %d and %d",
-			ETHER_MIN_MTU, VIRTIO_MAX_RX_PKTLEN - ether_hdr_len);
+			ETHER_MIN_MTU, max_frame_size - ether_hdr_len);
 		return -EINVAL;
 	}
 	return 0;
@@ -1392,6 +1395,21 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
 
 		hw->max_queue_pairs = config->max_virtqueue_pairs;
 
+		if (vtpci_with_feature(hw, VIRTIO_NET_F_MTU)) {
+			vtpci_read_dev_config(hw,
+				offsetof(struct virtio_net_config, mtu),
+				&config->mtu,
+				sizeof(config->mtu));
+
+			hw->max_mtu = config->mtu;
+			/* Set initial MTU to maximum one supported by vhost */
+			eth_dev->data->mtu = config->mtu;
+
+		} else {
+			hw->max_mtu = VIRTIO_MAX_RX_PKTLEN - ETHER_HDR_LEN -
+				VLAN_TAG_LEN - hw->vtnet_hdr_size;
+		}
+
 		PMD_INIT_LOG(DEBUG, "config->max_virtqueue_pairs=%d",
 				config->max_virtqueue_pairs);
 		PMD_INIT_LOG(DEBUG, "config->status=%d", config->status);
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index 777a14b..aa78adc 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -51,7 +51,7 @@
 #define VIRTIO_MAX_TX_QUEUES 128U
 #define VIRTIO_MAX_MAC_ADDRS 64
 #define VIRTIO_MIN_RX_BUFSIZE 64
-#define VIRTIO_MAX_RX_PKTLEN  9728
+#define VIRTIO_MAX_RX_PKTLEN  9728U
 
 /* Features desired/implemented by this driver. */
 #define VIRTIO_PMD_DEFAULT_GUEST_FEATURES	\
@@ -66,6 +66,7 @@
 	 1u << VIRTIO_NET_F_HOST_TSO4	  |	\
 	 1u << VIRTIO_NET_F_HOST_TSO6	  |	\
 	 1u << VIRTIO_NET_F_MRG_RXBUF	  |	\
+	 1u << VIRTIO_NET_F_MTU	| \
 	 1u << VIRTIO_RING_F_INDIRECT_DESC |    \
 	 1ULL << VIRTIO_F_VERSION_1       |	\
 	 1ULL << VIRTIO_F_IOMMU_PLATFORM)
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 59e45c4..771f5b1 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -106,6 +106,7 @@ struct virtnet_ctl;
 /* The feature bitmap for virtio net */
 #define VIRTIO_NET_F_CSUM	0	/* Host handles pkts w/ partial csum */
 #define VIRTIO_NET_F_GUEST_CSUM	1	/* Guest handles pkts w/ partial csum */
+#define VIRTIO_NET_F_MTU	3	/* Initial MTU advice. */
 #define VIRTIO_NET_F_MAC	5	/* Host has given MAC address. */
 #define VIRTIO_NET_F_GUEST_TSO4	7	/* Guest can handle TSOv4 in. */
 #define VIRTIO_NET_F_GUEST_TSO6	8	/* Guest can handle TSOv6 in. */
@@ -251,6 +252,7 @@ struct virtio_hw {
 	uint64_t    req_guest_features;
 	uint64_t    guest_features;
 	uint32_t    max_queue_pairs;
+	uint16_t	max_mtu;
 	uint16_t    vtnet_hdr_size;
 	uint8_t	    vlan_strip;
 	uint8_t	    use_msix;
@@ -296,6 +298,7 @@ struct virtio_net_config {
 	/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
 	uint16_t   status;
 	uint16_t   max_virtqueue_pairs;
+	uint16_t   mtu;
 } __attribute__((packed));
 
 /*
-- 
2.9.3

Re: [dpdk-dev] [PATCH 6/7] net/virtio: Add MTU feature support

[Aaron Conole]

Maxime Coquelin <maxime.coquelin@redhat.com> writes:

> This patch implements support for the Virtio MTU feature.
> When negotiated, the host shares its maximum supported MTU,
> which is used as initial MTU and as maximum MTU the application
> can set.
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  doc/guides/nics/features/virtio.ini |  1 +
>  drivers/net/virtio/virtio_ethdev.c  | 22 ++++++++++++++++++++--
>  drivers/net/virtio/virtio_ethdev.h  |  3 ++-
>  drivers/net/virtio/virtio_pci.h     |  3 +++
>  4 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/doc/guides/nics/features/virtio.ini b/doc/guides/nics/features/virtio.ini
> index 5164937..7bea075 100644
> --- a/doc/guides/nics/features/virtio.ini
> +++ b/doc/guides/nics/features/virtio.ini
> @@ -24,3 +24,4 @@ ARMv8                = Y
>  x86-32               = Y
>  x86-64               = Y
>  Usage doc            = Y
> +MTU update           = Y
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index d1ff234..ad3e6e1 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -721,10 +721,13 @@ virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>  	uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_LEN +
>  				 hw->vtnet_hdr_size;
>  	uint32_t frame_size = mtu + ether_hdr_len;
> +	uint32_t max_frame_size = hw->max_mtu + ether_hdr_len;
>  
> -	if (mtu < ETHER_MIN_MTU || frame_size > VIRTIO_MAX_RX_PKTLEN) {
> +	max_frame_size = RTE_MIN(max_frame_size, VIRTIO_MAX_RX_PKTLEN);
> +
> +	if (mtu < ETHER_MIN_MTU || frame_size > max_frame_size) {
>  		PMD_INIT_LOG(ERR, "MTU should be between %d and %d",
> -			ETHER_MIN_MTU, VIRTIO_MAX_RX_PKTLEN - ether_hdr_len);
> +			ETHER_MIN_MTU, max_frame_size - ether_hdr_len);
>  		return -EINVAL;
>  	}
>  	return 0;
> @@ -1392,6 +1395,21 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
>  
>  		hw->max_queue_pairs = config->max_virtqueue_pairs;
>  
> +		if (vtpci_with_feature(hw, VIRTIO_NET_F_MTU)) {
> +			vtpci_read_dev_config(hw,
> +				offsetof(struct virtio_net_config, mtu),
> +				&config->mtu,
> +				sizeof(config->mtu));

I think we need to check the value here against the min mtu, right?

> +			hw->max_mtu = config->mtu;
> +			/* Set initial MTU to maximum one supported by vhost */
> +			eth_dev->data->mtu = config->mtu;
> +
> +		} else {
> +			hw->max_mtu = VIRTIO_MAX_RX_PKTLEN - ETHER_HDR_LEN -
> +				VLAN_TAG_LEN - hw->vtnet_hdr_size;
> +		}
> +
>  		PMD_INIT_LOG(DEBUG, "config->max_virtqueue_pairs=%d",
>  				config->max_virtqueue_pairs);
>  		PMD_INIT_LOG(DEBUG, "config->status=%d", config->status);
> diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
> index 777a14b..aa78adc 100644
> --- a/drivers/net/virtio/virtio_ethdev.h
> +++ b/drivers/net/virtio/virtio_ethdev.h
> @@ -51,7 +51,7 @@
>  #define VIRTIO_MAX_TX_QUEUES 128U
>  #define VIRTIO_MAX_MAC_ADDRS 64
>  #define VIRTIO_MIN_RX_BUFSIZE 64
> -#define VIRTIO_MAX_RX_PKTLEN  9728
> +#define VIRTIO_MAX_RX_PKTLEN  9728U
>  
>  /* Features desired/implemented by this driver. */
>  #define VIRTIO_PMD_DEFAULT_GUEST_FEATURES	\
> @@ -66,6 +66,7 @@
>  	 1u << VIRTIO_NET_F_HOST_TSO4	  |	\
>  	 1u << VIRTIO_NET_F_HOST_TSO6	  |	\
>  	 1u << VIRTIO_NET_F_MRG_RXBUF	  |	\
> +	 1u << VIRTIO_NET_F_MTU	| \
>  	 1u << VIRTIO_RING_F_INDIRECT_DESC |    \
>  	 1ULL << VIRTIO_F_VERSION_1       |	\
>  	 1ULL << VIRTIO_F_IOMMU_PLATFORM)
> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index 59e45c4..771f5b1 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -106,6 +106,7 @@ struct virtnet_ctl;
>  /* The feature bitmap for virtio net */
>  #define VIRTIO_NET_F_CSUM	0	/* Host handles pkts w/ partial csum */
>  #define VIRTIO_NET_F_GUEST_CSUM	1	/* Guest handles pkts w/ partial csum */
> +#define VIRTIO_NET_F_MTU	3	/* Initial MTU advice. */
>  #define VIRTIO_NET_F_MAC	5	/* Host has given MAC address. */
>  #define VIRTIO_NET_F_GUEST_TSO4	7	/* Guest can handle TSOv4 in. */
>  #define VIRTIO_NET_F_GUEST_TSO6	8	/* Guest can handle TSOv6 in. */
> @@ -251,6 +252,7 @@ struct virtio_hw {
>  	uint64_t    req_guest_features;
>  	uint64_t    guest_features;
>  	uint32_t    max_queue_pairs;
> +	uint16_t	max_mtu;
>  	uint16_t    vtnet_hdr_size;
>  	uint8_t	    vlan_strip;
>  	uint8_t	    use_msix;
> @@ -296,6 +298,7 @@ struct virtio_net_config {
>  	/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
>  	uint16_t   status;
>  	uint16_t   max_virtqueue_pairs;
> +	uint16_t   mtu;
>  } __attribute__((packed));
>  
>  /*

Re: [dpdk-dev] [PATCH 6/7] net/virtio: Add MTU feature support

[Maxime Coquelin]

Hi Aaron,

On 02/16/2017 08:31 PM, Aaron Conole wrote:
> Maxime Coquelin <maxime.coquelin@redhat.com> writes:
>
>> This patch implements support for the Virtio MTU feature.
>> When negotiated, the host shares its maximum supported MTU,
>> which is used as initial MTU and as maximum MTU the application
>> can set.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>  doc/guides/nics/features/virtio.ini |  1 +
>>  drivers/net/virtio/virtio_ethdev.c  | 22 ++++++++++++++++++++--
>>  drivers/net/virtio/virtio_ethdev.h  |  3 ++-
>>  drivers/net/virtio/virtio_pci.h     |  3 +++
>>  4 files changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/doc/guides/nics/features/virtio.ini b/doc/guides/nics/features/virtio.ini
>> index 5164937..7bea075 100644
>> --- a/doc/guides/nics/features/virtio.ini
>> +++ b/doc/guides/nics/features/virtio.ini
>> @@ -24,3 +24,4 @@ ARMv8                = Y
>>  x86-32               = Y
>>  x86-64               = Y
>>  Usage doc            = Y
>> +MTU update           = Y
>> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>> index d1ff234..ad3e6e1 100644
>> --- a/drivers/net/virtio/virtio_ethdev.c
>> +++ b/drivers/net/virtio/virtio_ethdev.c
>> @@ -721,10 +721,13 @@ virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>>  	uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_LEN +
>>  				 hw->vtnet_hdr_size;
>>  	uint32_t frame_size = mtu + ether_hdr_len;
>> +	uint32_t max_frame_size = hw->max_mtu + ether_hdr_len;
>>
>> -	if (mtu < ETHER_MIN_MTU || frame_size > VIRTIO_MAX_RX_PKTLEN) {
>> +	max_frame_size = RTE_MIN(max_frame_size, VIRTIO_MAX_RX_PKTLEN);
>> +
>> +	if (mtu < ETHER_MIN_MTU || frame_size > max_frame_size) {
>>  		PMD_INIT_LOG(ERR, "MTU should be between %d and %d",
>> -			ETHER_MIN_MTU, VIRTIO_MAX_RX_PKTLEN - ether_hdr_len);
>> +			ETHER_MIN_MTU, max_frame_size - ether_hdr_len);
>>  		return -EINVAL;
>>  	}
>>  	return 0;
>> @@ -1392,6 +1395,21 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
>>
>>  		hw->max_queue_pairs = config->max_virtqueue_pairs;
>>
>> +		if (vtpci_with_feature(hw, VIRTIO_NET_F_MTU)) {
>> +			vtpci_read_dev_config(hw,
>> +				offsetof(struct virtio_net_config, mtu),
>> +				&config->mtu,
>> +				sizeof(config->mtu));
>
> I think we need to check the value here against the min mtu, right?
Right. Moreover, we don't check it in Qemu.
I need to add the check both DPDK and Qemu.

Thanks,
Maxime

[dpdk-dev] [PATCH 7/7] app/testpmd: print MTU value in show port info

[Maxime Coquelin]

This patch adds MTU display to "show port info" command.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 app/test-pmd/config.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 80491fc..73d9603 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -449,6 +449,7 @@ port_infos_display(portid_t port_id)
 	struct rte_mempool * mp;
 	static const char *info_border = "*********************";
 	portid_t pid;
+	uint16_t mtu;
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
 		printf("Valid port range is [0");
@@ -480,6 +481,10 @@ port_infos_display(portid_t port_id)
 	printf("Link speed: %u Mbps\n", (unsigned) link.link_speed);
 	printf("Link duplex: %s\n", (link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
 	       ("full-duplex") : ("half-duplex"));
+
+	if (!rte_eth_dev_get_mtu(port_id, &mtu))
+		printf("MTU: %u\n", mtu);
+
 	printf("Promiscuous mode: %s\n",
 	       rte_eth_promiscuous_get(port_id) ? "enabled" : "disabled");
 	printf("Allmulticast mode: %s\n",
-- 
2.9.3

Re: [dpdk-dev] [PATCH 0/7] virtio/vhost: Add MTU feature support

[Yuanhan Liu]

On Mon, Feb 13, 2017 at 03:28:13PM +0100, Maxime Coquelin wrote:
> This series adds support to new Virtio's MTU feature[1].

Seems you missed a link here?

> The MTU
> value is set via QEMU parameters.
> 
> If the feature is negotiated (i.e supported by both host andcguest,
> and valid MTU value is set in QEMU via its host_mtu parameter), QEMU
> shares the configured MTU value throught dedicated Vhost protocol
> feature.
> 
> On vhost side, the value is stored in the virtio_net structure, and
> made available to the application thanks to new vhost lib's
> rte_vhost_mtu_get() function.
> 
> rte_vhost_mtu_set() functions is implemented, but only succeed if the
> application sets the same value as the one set in QEMU. Idea is that
> it would be used for the application to ensure configured MTU value
> is consistent, but maybe the mtu_get() API is enough, and mtu_set()
> could just be dropped.

If the vhost MTU is designed to be read-only, then we may should drop
the set_mtu function.

> Vhost PMD mtu_set callback is also implemented
> in the same spirit.
> 
> To be able to set eth_dev's MTU value at the right time, i.e. to call
> rte_vhost_mtu_get() just after Virtio features have been negotiated
> and before the device is really started, a new vhost flag has been
> introduced (VIRTIO_DEV_READY), because the VIRTIO_DEV_RUNNING flag is
> set too late (after .new_device() ops is called).

Okay, and I think this kind of info should be in corresponding commit log.

> Regarding valid MTU values, the maximum MTU value accepted on vhost
> side is 65535 bytes, as defined in Virtio Spec and supported in
> Virtio-net Kernel driver. But in Virtio PMD, current maximum frame
> size is 9728 bytes (~9700 bytes MTU). So maximum MTU size accepted in
> Virtio PMD is the minimum between ~9700 bytes and host's MTU.
> 
> Initially, we thought about disabling the rx-mergeable feature when
> MTU value was low enough to ensure all received packets would fit in
> receive buffers (when offloads are disabled). Doing this, we would
> save one cache-miss in the receive path. Problem is that we don't
> know the buffers size at Virtio feature neogotiation time.
> It might be possible for the application to call the configure
> callback again once the Rx queue is set up, but it seems a bit hacky.

Worse, if multiple queue is involved, one queue could have it's own
mempool, meaning the buffer size could be different, whereas the
MTU feature is global.

	--yliu

> So I decided to skip this optimization for now, even if feedback and
> are of course appreciated.
> 
> Finally, this series also adds MTU value printing  in testpmd's
> "show port info" command when non-zero.
> 
> This series target v17.05 release.
> 
> Cheers,
> Maxime
> 
> Maxime Coquelin (7):
>   vhost: Enable VIRTIO_NET_F_MTU feature
>   vhost: vhost-user: Add MTU protocol feature support
>   vhost: Add new ready status flag
>   vhost: Add API to get/set MTU value
>   net/vhost: Implement mtu_set callback
>   net/virtio: Add MTU feature support
>   app/testpmd: print MTU value in show port info
> 
>  app/test-pmd/config.c               |  5 +++++
>  doc/guides/nics/features/vhost.ini  |  1 +
>  doc/guides/nics/features/virtio.ini |  1 +
>  drivers/net/vhost/rte_eth_vhost.c   | 18 +++++++++++++++
>  drivers/net/virtio/virtio_ethdev.c  | 22 +++++++++++++++++--
>  drivers/net/virtio/virtio_ethdev.h  |  3 ++-
>  drivers/net/virtio/virtio_pci.h     |  3 +++
>  lib/librte_vhost/rte_virtio_net.h   | 31 ++++++++++++++++++++++++++
>  lib/librte_vhost/vhost.c            | 42 ++++++++++++++++++++++++++++++++++-
>  lib/librte_vhost/vhost.h            |  9 +++++++-
>  lib/librte_vhost/vhost_user.c       | 44 +++++++++++++++++++++++++++++++------
>  lib/librte_vhost/vhost_user.h       |  5 ++++-
>  12 files changed, 171 insertions(+), 13 deletions(-)
> 
> -- 
> 2.9.3

Re: [dpdk-dev] [PATCH 0/7] virtio/vhost: Add MTU feature support

[Maxime Coquelin]

On 02/23/2017 08:10 AM, Yuanhan Liu wrote:
> On Mon, Feb 13, 2017 at 03:28:13PM +0100, Maxime Coquelin wrote:
>> This series adds support to new Virtio's MTU feature[1].
>
> Seems you missed a link here?
Oops, I meant this link:
https://www.spinics.net/lists/linux-virtualization/msg28908.html

Will fix in v2.

>
>> The MTU
>> value is set via QEMU parameters.
>>
>> If the feature is negotiated (i.e supported by both host andcguest,
>> and valid MTU value is set in QEMU via its host_mtu parameter), QEMU
>> shares the configured MTU value throught dedicated Vhost protocol
>> feature.
>>
>> On vhost side, the value is stored in the virtio_net structure, and
>> made available to the application thanks to new vhost lib's
>> rte_vhost_mtu_get() function.
>>
>> rte_vhost_mtu_set() functions is implemented, but only succeed if the
>> application sets the same value as the one set in QEMU. Idea is that
>> it would be used for the application to ensure configured MTU value
>> is consistent, but maybe the mtu_get() API is enough, and mtu_set()
>> could just be dropped.
>
> If the vhost MTU is designed to be read-only, then we may should drop
> the set_mtu function.

Ok, I will remove it in next revision.

>> Vhost PMD mtu_set callback is also implemented
>> in the same spirit.
>>
>> To be able to set eth_dev's MTU value at the right time, i.e. to call
>> rte_vhost_mtu_get() just after Virtio features have been negotiated
>> and before the device is really started, a new vhost flag has been
>> introduced (VIRTIO_DEV_READY), because the VIRTIO_DEV_RUNNING flag is
>> set too late (after .new_device() ops is called).
>
> Okay, and I think this kind of info should be in corresponding commit log.
Sure.
>
>> Regarding valid MTU values, the maximum MTU value accepted on vhost
>> side is 65535 bytes, as defined in Virtio Spec and supported in
>> Virtio-net Kernel driver. But in Virtio PMD, current maximum frame
>> size is 9728 bytes (~9700 bytes MTU). So maximum MTU size accepted in
>> Virtio PMD is the minimum between ~9700 bytes and host's MTU.
>>
>> Initially, we thought about disabling the rx-mergeable feature when
>> MTU value was low enough to ensure all received packets would fit in
>> receive buffers (when offloads are disabled). Doing this, we would
>> save one cache-miss in the receive path. Problem is that we don't
>> know the buffers size at Virtio feature neogotiation time.
>> It might be possible for the application to call the configure
>> callback again once the Rx queue is set up, but it seems a bit hacky.
>
> Worse, if multiple queue is involved, one queue could have it's own
> mempool, meaning the buffer size could be different, whereas the
> MTU feature is global.
Indeed, we should ensure that for every queues, the corresponding buf
size can handle a full packet to be able to disable the mergeable
feature.

Let's skip this for now and focus on improving the rx-mergeable path.

Thanks,
Maxime
>
> 	--yliu
>
>> So I decided to skip this optimization for now, even if feedback and
>> are of course appreciated.
>>
>> Finally, this series also adds MTU value printing  in testpmd's
>> "show port info" command when non-zero.
>>
>> This series target v17.05 release.
>>
>> Cheers,
>> Maxime
>>
>> Maxime Coquelin (7):
>>   vhost: Enable VIRTIO_NET_F_MTU feature
>>   vhost: vhost-user: Add MTU protocol feature support
>>   vhost: Add new ready status flag
>>   vhost: Add API to get/set MTU value
>>   net/vhost: Implement mtu_set callback
>>   net/virtio: Add MTU feature support
>>   app/testpmd: print MTU value in show port info
>>
>>  app/test-pmd/config.c               |  5 +++++
>>  doc/guides/nics/features/vhost.ini  |  1 +
>>  doc/guides/nics/features/virtio.ini |  1 +
>>  drivers/net/vhost/rte_eth_vhost.c   | 18 +++++++++++++++
>>  drivers/net/virtio/virtio_ethdev.c  | 22 +++++++++++++++++--
>>  drivers/net/virtio/virtio_ethdev.h  |  3 ++-
>>  drivers/net/virtio/virtio_pci.h     |  3 +++
>>  lib/librte_vhost/rte_virtio_net.h   | 31 ++++++++++++++++++++++++++
>>  lib/librte_vhost/vhost.c            | 42 ++++++++++++++++++++++++++++++++++-
>>  lib/librte_vhost/vhost.h            |  9 +++++++-
>>  lib/librte_vhost/vhost_user.c       | 44 +++++++++++++++++++++++++++++++------
>>  lib/librte_vhost/vhost_user.h       |  5 ++++-
>>  12 files changed, 171 insertions(+), 13 deletions(-)
>>
>> --
>> 2.9.3

[dpdk-dev] [PATCH v2 0/7] virtio/vhost: Add MTU feature support

[Maxime Coquelin]

This series adds support to new Virtio's MTU feature[1]. The MTU
value is set via QEMU parameters.

If the feature is negotiated (i.e supported by both host andcguest,
and valid MTU value is set in QEMU via its host_mtu parameter), QEMU
shares the configured MTU value throught dedicated Vhost protocol
feature.

On vhost side, the value is stored in the virtio_net structure, and
made available to the application thanks to new vhost lib's
rte_vhost_mtu_get() function.

To be able to set eth_dev's MTU value at the right time, i.e. to call
rte_vhost_mtu_get() just after Virtio features have been negotiated
and before the device is really started, a new vhost flag has been
introduced (VIRTIO_DEV_READY), because the VIRTIO_DEV_RUNNING flag is
set too late (after .new_device() ops is called).

Regarding valid MTU values, the maximum MTU value accepted on vhost
side is 65535 bytes, as defined in Virtio Spec and supported in
Virtio-net Kernel driver. But in Virtio PMD, current maximum frame
size is 9728 bytes (~9700 bytes MTU). So maximum MTU size accepted in
Virtio PMD is the minimum between ~9700 bytes and host's MTU.

Finally, this series also adds MTU value printing  in testpmd's
"show port info" command when non-zero.

This series target v17.05 release.

Cheers,
Maxime

[1]: https://lists.oasis-open.org/archives/virtio-dev/201609/msg00128.html

Changes since v1:
-----------------
* Rebased on top of v17.02
* Virtio PMD: ensure MTU value is valid before ack'ing the feature (Aaron)
* Vhost lib/PMD: Remove MTU setting API/op (Yuanhan)

Maxime Coquelin (7):
  vhost: Enable VIRTIO_NET_F_MTU feature
  vhost: vhost-user: Add MTU protocol feature support
  vhost: Add new ready status flag
  vhost: Add API to get MTU value
  net/vhost: Fill rte_eth_dev's MTU property
  net/virtio: Add MTU feature support
  app/testpmd: print MTU value in show port info

 app/test-pmd/config.c               |  5 +++++
 doc/guides/nics/features/virtio.ini |  1 +
 drivers/net/vhost/rte_eth_vhost.c   |  2 ++
 drivers/net/virtio/virtio_ethdev.c  | 45 +++++++++++++++++++++++++++++++++++--
 drivers/net/virtio/virtio_ethdev.h  |  3 ++-
 drivers/net/virtio/virtio_pci.h     |  3 +++
 lib/librte_vhost/rte_virtio_net.h   | 15 +++++++++++++
 lib/librte_vhost/vhost.c            | 22 +++++++++++++++++-
 lib/librte_vhost/vhost.h            |  9 +++++++-
 lib/librte_vhost/vhost_user.c       | 44 ++++++++++++++++++++++++++++++------
 lib/librte_vhost/vhost_user.h       |  5 ++++-
 11 files changed, 141 insertions(+), 13 deletions(-)

-- 
2.9.3

[dpdk-dev] [PATCH v2 1/7] vhost: Enable VIRTIO_NET_F_MTU feature

[Maxime Coquelin]

This patch enables the new VIRTIO_NET_F_MTU feature,
which makes possible for the host to advise the guest
with its maximum supported MTU.

MTU value is set via QEMU parameters, either via Libvirt XML, or
directly in virtio-net device command line arguments.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost.c | 3 ++-
 lib/librte_vhost/vhost.h | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index e415093..3974087 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -66,7 +66,8 @@
 				(1ULL << VIRTIO_NET_F_GUEST_CSUM) | \
 				(1ULL << VIRTIO_NET_F_GUEST_TSO4) | \
 				(1ULL << VIRTIO_NET_F_GUEST_TSO6) | \
-				(1ULL << VIRTIO_RING_F_INDIRECT_DESC))
+				(1ULL << VIRTIO_RING_F_INDIRECT_DESC) | \
+				(1ULL << VIRTIO_NET_F_MTU))
 
 uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;
 
diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 22564f1..6a57bb3 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -110,11 +110,15 @@ struct vhost_virtqueue {
 	uint16_t                shadow_used_idx;
 } __rte_cache_aligned;
 
-/* Old kernels have no such macro defined */
+/* Old kernels have no such macros defined */
 #ifndef VIRTIO_NET_F_GUEST_ANNOUNCE
  #define VIRTIO_NET_F_GUEST_ANNOUNCE 21
 #endif
 
+#ifndef VIRTIO_NET_F_MTU
+ #define VIRTIO_NET_F_MTU 3
+#endif
+
 
 /*
  * Make an extra wrapper for VIRTIO_NET_F_MQ and
-- 
2.9.3

[dpdk-dev] [PATCH v2 2/7] vhost: vhost-user: Add MTU protocol feature support

[Maxime Coquelin]

This patch implements the vhost-user MTU protocol feature support.
When VIRTIO_NET_F_MTU is negotiated, QEMU notifies the vhost-user
backend with the configured MTU if dedicated protocol feature is
supported.

The value can be used by the application to ensure consistency with
value set by the user.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost.h      |  1 +
 lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++++++++++
 lib/librte_vhost/vhost_user.h |  5 ++++-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 6a57bb3..549296f 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -170,6 +170,7 @@ struct virtio_net {
 	uint64_t		log_base;
 	uint64_t		log_addr;
 	struct ether_addr	mac;
+	uint16_t		mtu;
 
 	uint32_t		nr_guest_pages;
 	uint32_t		max_guest_pages;
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index cb2156a..69877a4 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -51,6 +51,9 @@
 #include "vhost.h"
 #include "vhost_user.h"
 
+#define VIRTIO_MIN_MTU 68
+#define VIRTIO_MAX_MTU 65535
+
 static const char *vhost_message_str[VHOST_USER_MAX] = {
 	[VHOST_USER_NONE] = "VHOST_USER_NONE",
 	[VHOST_USER_GET_FEATURES] = "VHOST_USER_GET_FEATURES",
@@ -72,6 +75,7 @@ static const char *vhost_message_str[VHOST_USER_MAX] = {
 	[VHOST_USER_GET_QUEUE_NUM]  = "VHOST_USER_GET_QUEUE_NUM",
 	[VHOST_USER_SET_VRING_ENABLE]  = "VHOST_USER_SET_VRING_ENABLE",
 	[VHOST_USER_SEND_RARP]  = "VHOST_USER_SEND_RARP",
+	[VHOST_USER_NET_SET_MTU]  = "VHOST_USER_NET_SET_MTU",
 };
 
 static uint64_t
@@ -865,6 +869,22 @@ vhost_user_send_rarp(struct virtio_net *dev, struct VhostUserMsg *msg)
 	return 0;
 }
 
+static int
+vhost_user_net_set_mtu(struct virtio_net *dev, struct VhostUserMsg *msg)
+{
+	if (msg->payload.u64 < VIRTIO_MIN_MTU ||
+			msg->payload.u64 > VIRTIO_MAX_MTU) {
+		RTE_LOG(ERR, VHOST_CONFIG, "Invalid MTU size (%lu)\n",
+				msg->payload.u64);
+
+		return -1;
+	}
+
+	dev->mtu = (uint16_t)msg->payload.u64;
+
+	return 0;
+}
+
 /* return bytes# of read on success or negative val on failure. */
 static int
 read_vhost_message(int sockfd, struct VhostUserMsg *msg)
@@ -1027,6 +1047,10 @@ vhost_user_msg_handler(int vid, int fd)
 		vhost_user_send_rarp(dev, &msg);
 		break;
 
+	case VHOST_USER_NET_SET_MTU:
+		ret = vhost_user_net_set_mtu(dev, &msg);
+		break;
+
 	default:
 		ret = -1;
 		break;
diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
index 179e441..838dec8 100644
--- a/lib/librte_vhost/vhost_user.h
+++ b/lib/librte_vhost/vhost_user.h
@@ -47,11 +47,13 @@
 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD	1
 #define VHOST_USER_PROTOCOL_F_RARP	2
 #define VHOST_USER_PROTOCOL_F_REPLY_ACK	3
+#define VHOST_USER_PROTOCOL_F_NET_MTU 4
 
 #define VHOST_USER_PROTOCOL_FEATURES	((1ULL << VHOST_USER_PROTOCOL_F_MQ) | \
 					 (1ULL << VHOST_USER_PROTOCOL_F_LOG_SHMFD) |\
 					 (1ULL << VHOST_USER_PROTOCOL_F_RARP) | \
-					 (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
+					 (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK) | \
+					 (1ULL << VHOST_USER_PROTOCOL_F_NET_MTU))
 
 typedef enum VhostUserRequest {
 	VHOST_USER_NONE = 0,
@@ -74,6 +76,7 @@ typedef enum VhostUserRequest {
 	VHOST_USER_GET_QUEUE_NUM = 17,
 	VHOST_USER_SET_VRING_ENABLE = 18,
 	VHOST_USER_SEND_RARP = 19,
+	VHOST_USER_NET_SET_MTU = 20,
 	VHOST_USER_MAX
 } VhostUserRequest;
 
-- 
2.9.3

Re: [dpdk-dev] [PATCH v2 2/7] vhost: vhost-user: Add MTU protocol feature support

[Yuanhan Liu]

On Mon, Mar 06, 2017 at 09:27:35AM +0100, Maxime Coquelin wrote:
> +static int
> +vhost_user_net_set_mtu(struct virtio_net *dev, struct VhostUserMsg *msg)
> +{
> +	if (msg->payload.u64 < VIRTIO_MIN_MTU ||
> +			msg->payload.u64 > VIRTIO_MAX_MTU) {
> +		RTE_LOG(ERR, VHOST_CONFIG, "Invalid MTU size (%lu)\n",
> +				msg->payload.u64);

This (%lu) would break the 32-bit OS build.

> +
> +		return -1;
> +	}
> +
> +	dev->mtu = (uint16_t)msg->payload.u64;

Besides, the cast seems unnecessary.

	--yliu

Re: [dpdk-dev] [PATCH v2 2/7] vhost: vhost-user: Add MTU protocol feature support

[Maxime Coquelin]

On 03/08/2017 03:31 AM, Yuanhan Liu wrote:
> On Mon, Mar 06, 2017 at 09:27:35AM +0100, Maxime Coquelin wrote:
>> +static int
>> +vhost_user_net_set_mtu(struct virtio_net *dev, struct VhostUserMsg *msg)
>> +{
>> +	if (msg->payload.u64 < VIRTIO_MIN_MTU ||
>> +			msg->payload.u64 > VIRTIO_MAX_MTU) {
>> +		RTE_LOG(ERR, VHOST_CONFIG, "Invalid MTU size (%lu)\n",
>> +				msg->payload.u64);
>
> This (%lu) would break the 32-bit OS build.

Right. Changed to %"PRIu64".
>
>> +
>> +		return -1;
>> +	}
>> +
>> +	dev->mtu = (uint16_t)msg->payload.u64;
>
> Besides, the cast seems unnecessary.
Indeed.

Thanks;
Maxime

[dpdk-dev] [PATCH v2 3/7] vhost: Add new ready status flag

[Maxime Coquelin]

This patch adds a new status flag indicating the Virtio device
is ready to operate.

This is required to be able to call rte_vhost_mtu_get() in the
.new_device() callback, as rte_vhost_mtu_get needs that the
negotiation is done, but it is too early to rely on running status
flag, which is set just after .new_device() returns.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost.h      |  2 ++
 lib/librte_vhost/vhost_user.c | 20 +++++++++++++-------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 549296f..e8b7e44 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -46,6 +46,8 @@
 
 /* Used to indicate that the device is running on a data core */
 #define VIRTIO_DEV_RUNNING 1
+/* Used to indicate that the device is ready to operate */
+#define VIRTIO_DEV_READY 2
 
 /* Backend value set by guest. */
 #define VIRTIO_DEV_STOPPED -1
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 69877a4..4726aaf 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -691,14 +691,18 @@ vhost_user_set_vring_kick(struct virtio_net *dev, struct VhostUserMsg *pmsg)
 		close(vq->kickfd);
 	vq->kickfd = file.fd;
 
-	if (virtio_is_ready(dev) && !(dev->flags & VIRTIO_DEV_RUNNING)) {
-		if (dev->dequeue_zero_copy) {
-			RTE_LOG(INFO, VHOST_CONFIG,
-				"dequeue zero copy is enabled\n");
-		}
+	if (virtio_is_ready(dev)) {
+		dev->flags |= VIRTIO_DEV_READY;
+
+		if (!(dev->flags & VIRTIO_DEV_RUNNING)) {
+			if (dev->dequeue_zero_copy) {
+				RTE_LOG(INFO, VHOST_CONFIG,
+						"dequeue zero copy is enabled\n");
+			}
 
-		if (notify_ops->new_device(dev->vid) == 0)
-			dev->flags |= VIRTIO_DEV_RUNNING;
+			if (notify_ops->new_device(dev->vid) == 0)
+				dev->flags |= VIRTIO_DEV_RUNNING;
+		}
 	}
 }
 
@@ -733,6 +737,8 @@ vhost_user_get_vring_base(struct virtio_net *dev,
 		notify_ops->destroy_device(dev->vid);
 	}
 
+	dev->flags &= ~VIRTIO_DEV_READY;
+
 	/* Here we are safe to get the last used index */
 	state->num = vq->last_used_idx;
 
-- 
2.9.3

[dpdk-dev] [PATCH v2 4/7] vhost: Add API to get MTU value

[Maxime Coquelin]

This patch implements the function for the application to
get the MTU value.

rte_vhost_mtu_get() fills the mtu parameter with the MTU value
set in QEMU if VIRTIO_NET_F_MTU has been negotiated and returns 0,
-ENOTSUP otherwise.

The function returns -EAGAIN if Virtio feature negotiation
didn't happened yet.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/rte_virtio_net.h | 15 +++++++++++++++
 lib/librte_vhost/vhost.c          | 19 +++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
index 926039c..ff02e9b 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h
@@ -100,6 +100,21 @@ int rte_vhost_driver_callback_register(struct virtio_net_device_ops const * cons
 int rte_vhost_driver_session_start(void);
 
 /**
+ * Get the MTU value of the device if set in QEMU.
+ *
+ * @param vid
+ *  virtio-net device ID
+ * @param mtu
+ *  The variable to store the MTU value
+ *
+ * @return
+ *  0: success
+ *  -EAGAIN: device not yet started
+ *  -ENOTSUP: device does not support MTU feature
+ */
+int rte_vhost_mtu_get(int vid, uint16_t *mtu);
+
+/**
  * Get the numa node from which the virtio net device's memory
  * is allocated.
  *
diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index 3974087..bbf7f7e 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -313,6 +313,25 @@ vhost_enable_dequeue_zero_copy(int vid)
 }
 
 int
+rte_vhost_mtu_get(int vid, uint16_t *mtu)
+{
+	struct virtio_net *dev = get_device(vid);
+
+	if (!dev)
+		return -ENODEV;
+
+	if (!(dev->flags & VIRTIO_DEV_READY))
+		return -EAGAIN;
+
+	if (!(dev->features & VIRTIO_NET_F_MTU))
+		return -ENOTSUP;
+
+	*mtu = dev->mtu;
+
+	return 0;
+}
+
+int
 rte_vhost_get_numa_node(int vid)
 {
 #ifdef RTE_LIBRTE_VHOST_NUMA
-- 
2.9.3

Re: [dpdk-dev] [PATCH v2 4/7] vhost: Add API to get MTU value

[Yuanhan Liu]

On Mon, Mar 06, 2017 at 09:27:37AM +0100, Maxime Coquelin wrote:
> This patch implements the function for the application to
> get the MTU value.
> 
> rte_vhost_mtu_get() fills the mtu parameter with the MTU value
> set in QEMU if VIRTIO_NET_F_MTU has been negotiated and returns 0,
> -ENOTSUP otherwise.
> 
> The function returns -EAGAIN if Virtio feature negotiation
> didn't happened yet.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/librte_vhost/rte_virtio_net.h | 15 +++++++++++++++
>  lib/librte_vhost/vhost.c          | 19 +++++++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
> index 926039c..ff02e9b 100644
> --- a/lib/librte_vhost/rte_virtio_net.h
> +++ b/lib/librte_vhost/rte_virtio_net.h
> @@ -100,6 +100,21 @@ int rte_vhost_driver_callback_register(struct virtio_net_device_ops const * cons
>  int rte_vhost_driver_session_start(void);
>  
>  /**
> + * Get the MTU value of the device if set in QEMU.
> + *
> + * @param vid
> + *  virtio-net device ID
> + * @param mtu
> + *  The variable to store the MTU value
> + *
> + * @return
> + *  0: success
> + *  -EAGAIN: device not yet started
> + *  -ENOTSUP: device does not support MTU feature
> + */
> +int rte_vhost_mtu_get(int vid, uint16_t *mtu);

I'd suggest to name it "_get_mtu", to align with the current naming style:
    _get_ifname
    _get_numa_node
    ...

Besides that, you should add an entry to the rte_vhost_version.map.

	--yliu

Re: [dpdk-dev] [PATCH v2 4/7] vhost: Add API to get MTU value

[Maxime Coquelin]

On 03/08/2017 03:45 AM, Yuanhan Liu wrote:
> On Mon, Mar 06, 2017 at 09:27:37AM +0100, Maxime Coquelin wrote:
>> This patch implements the function for the application to
>> get the MTU value.
>>
>> rte_vhost_mtu_get() fills the mtu parameter with the MTU value
>> set in QEMU if VIRTIO_NET_F_MTU has been negotiated and returns 0,
>> -ENOTSUP otherwise.
>>
>> The function returns -EAGAIN if Virtio feature negotiation
>> didn't happened yet.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>  lib/librte_vhost/rte_virtio_net.h | 15 +++++++++++++++
>>  lib/librte_vhost/vhost.c          | 19 +++++++++++++++++++
>>  2 files changed, 34 insertions(+)
>>
>> diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
>> index 926039c..ff02e9b 100644
>> --- a/lib/librte_vhost/rte_virtio_net.h
>> +++ b/lib/librte_vhost/rte_virtio_net.h
>> @@ -100,6 +100,21 @@ int rte_vhost_driver_callback_register(struct virtio_net_device_ops const * cons
>>  int rte_vhost_driver_session_start(void);
>>
>>  /**
>> + * Get the MTU value of the device if set in QEMU.
>> + *
>> + * @param vid
>> + *  virtio-net device ID
>> + * @param mtu
>> + *  The variable to store the MTU value
>> + *
>> + * @return
>> + *  0: success
>> + *  -EAGAIN: device not yet started
>> + *  -ENOTSUP: device does not support MTU feature
>> + */
>> +int rte_vhost_mtu_get(int vid, uint16_t *mtu);
>
> I'd suggest to name it "_get_mtu", to align with the current naming style:
>     _get_ifname
>     _get_numa_node
>     ...

Good point, I named it against the eth_dev's callback name, but it does 
make more sense to be consistent with vhost API style.

>
> Besides that, you should add an entry to the rte_vhost_version.map.
Right.

Thanks,
Maxime

[dpdk-dev] [PATCH v2 5/7] net/vhost: Fill rte_eth_dev's MTU property

[Maxime Coquelin]

This patch adds a call to rte_vhost_mtu_get() at device creation
time to fill device's MTU property when available.

This makes the MTU value defined in QEMU cmdline accessible to the
application by calling rte_eth_dev_get_mtu().

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/vhost/rte_eth_vhost.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index e98cffd..344c328 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -575,6 +575,8 @@ new_device(int vid)
 	for (i = 0; i < rte_vhost_get_queue_num(vid) * VIRTIO_QNUM; i++)
 		rte_vhost_enable_guest_notification(vid, i, 0);
 
+	rte_vhost_mtu_get(vid, &eth_dev->data->mtu);
+
 	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
 
 	rte_atomic32_set(&internal->dev_attached, 1);
-- 
2.9.3

[dpdk-dev] [PATCH v2 6/7] net/virtio: Add MTU feature support

[Maxime Coquelin]

This patch implements support for the Virtio MTU feature.
When negotiated, the host shares its maximum supported MTU,
which is used as initial MTU and as maximum MTU the application
can set.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 doc/guides/nics/features/virtio.ini |  1 +
 drivers/net/virtio/virtio_ethdev.c  | 45 +++++++++++++++++++++++++++++++++++--
 drivers/net/virtio/virtio_ethdev.h  |  3 ++-
 drivers/net/virtio/virtio_pci.h     |  3 +++
 4 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/doc/guides/nics/features/virtio.ini b/doc/guides/nics/features/virtio.ini
index 84d2012..8e3aca1 100644
--- a/doc/guides/nics/features/virtio.ini
+++ b/doc/guides/nics/features/virtio.ini
@@ -25,3 +25,4 @@ ARMv8                = Y
 x86-32               = Y
 x86-64               = Y
 Usage doc            = Y
+MTU update           = Y
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 4dc03b9..fd1213c 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -721,10 +721,13 @@ virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_LEN +
 				 hw->vtnet_hdr_size;
 	uint32_t frame_size = mtu + ether_hdr_len;
+	uint32_t max_frame_size = hw->max_mtu + ether_hdr_len;
 
-	if (mtu < ETHER_MIN_MTU || frame_size > VIRTIO_MAX_RX_PKTLEN) {
+	max_frame_size = RTE_MIN(max_frame_size, VIRTIO_MAX_RX_PKTLEN);
+
+	if (mtu < ETHER_MIN_MTU || frame_size > max_frame_size) {
 		PMD_INIT_LOG(ERR, "MTU should be between %d and %d",
-			ETHER_MIN_MTU, VIRTIO_MAX_RX_PKTLEN - ether_hdr_len);
+			ETHER_MIN_MTU, max_frame_size - ether_hdr_len);
 		return -EINVAL;
 	}
 	return 0;
@@ -1158,6 +1161,18 @@ virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features)
 	PMD_INIT_LOG(DEBUG, "host_features before negotiate = %" PRIx64,
 		host_features);
 
+	/* If supported, ensure MTU value is valid before acknowledging it. */
+	if (host_features & req_features & (1ULL << VIRTIO_NET_F_MTU)) {
+		struct virtio_net_config config;
+
+		vtpci_read_dev_config(hw,
+			offsetof(struct virtio_net_config, mtu),
+			&config.mtu, sizeof(config.mtu));
+
+		if (config.mtu < ETHER_MIN_MTU)
+			req_features &= ~(1ULL << VIRTIO_NET_F_MTU);
+	}
+
 	/*
 	 * Negotiate features: Subset of device feature bits are written back
 	 * guest feature bits.
@@ -1392,6 +1407,32 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
 
 		hw->max_queue_pairs = config->max_virtqueue_pairs;
 
+		if (vtpci_with_feature(hw, VIRTIO_NET_F_MTU)) {
+			vtpci_read_dev_config(hw,
+				offsetof(struct virtio_net_config, mtu),
+				&config->mtu,
+				sizeof(config->mtu));
+
+			/*
+			 * MTU value has already been checked at negotiation
+			 * time, but check again in case it has changed since
+			 * then, which should not happen.
+			 */
+			if (config->mtu < ETHER_MIN_MTU) {
+				PMD_INIT_LOG(ERR, "invalid max MTU value (%u)",
+						config->mtu);
+				return -1;
+			}
+
+			hw->max_mtu = config->mtu;
+			/* Set initial MTU to maximum one supported by vhost */
+			eth_dev->data->mtu = config->mtu;
+
+		} else {
+			hw->max_mtu = VIRTIO_MAX_RX_PKTLEN - ETHER_HDR_LEN -
+				VLAN_TAG_LEN - hw->vtnet_hdr_size;
+		}
+
 		PMD_INIT_LOG(DEBUG, "config->max_virtqueue_pairs=%d",
 				config->max_virtqueue_pairs);
 		PMD_INIT_LOG(DEBUG, "config->status=%d", config->status);
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index 777a14b..aa78adc 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -51,7 +51,7 @@
 #define VIRTIO_MAX_TX_QUEUES 128U
 #define VIRTIO_MAX_MAC_ADDRS 64
 #define VIRTIO_MIN_RX_BUFSIZE 64
-#define VIRTIO_MAX_RX_PKTLEN  9728
+#define VIRTIO_MAX_RX_PKTLEN  9728U
 
 /* Features desired/implemented by this driver. */
 #define VIRTIO_PMD_DEFAULT_GUEST_FEATURES	\
@@ -66,6 +66,7 @@
 	 1u << VIRTIO_NET_F_HOST_TSO4	  |	\
 	 1u << VIRTIO_NET_F_HOST_TSO6	  |	\
 	 1u << VIRTIO_NET_F_MRG_RXBUF	  |	\
+	 1u << VIRTIO_NET_F_MTU	| \
 	 1u << VIRTIO_RING_F_INDIRECT_DESC |    \
 	 1ULL << VIRTIO_F_VERSION_1       |	\
 	 1ULL << VIRTIO_F_IOMMU_PLATFORM)
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 59e45c4..771f5b1 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -106,6 +106,7 @@ struct virtnet_ctl;
 /* The feature bitmap for virtio net */
 #define VIRTIO_NET_F_CSUM	0	/* Host handles pkts w/ partial csum */
 #define VIRTIO_NET_F_GUEST_CSUM	1	/* Guest handles pkts w/ partial csum */
+#define VIRTIO_NET_F_MTU	3	/* Initial MTU advice. */
 #define VIRTIO_NET_F_MAC	5	/* Host has given MAC address. */
 #define VIRTIO_NET_F_GUEST_TSO4	7	/* Guest can handle TSOv4 in. */
 #define VIRTIO_NET_F_GUEST_TSO6	8	/* Guest can handle TSOv6 in. */
@@ -251,6 +252,7 @@ struct virtio_hw {
 	uint64_t    req_guest_features;
 	uint64_t    guest_features;
 	uint32_t    max_queue_pairs;
+	uint16_t	max_mtu;
 	uint16_t    vtnet_hdr_size;
 	uint8_t	    vlan_strip;
 	uint8_t	    use_msix;
@@ -296,6 +298,7 @@ struct virtio_net_config {
 	/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
 	uint16_t   status;
 	uint16_t   max_virtqueue_pairs;
+	uint16_t   mtu;
 } __attribute__((packed));
 
 /*
-- 
2.9.3

[dpdk-dev] [PATCH v2 7/7] app/testpmd: print MTU value in show port info

[Maxime Coquelin]

This patch adds MTU display to "show port info" command.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 app/test-pmd/config.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 80491fc..73d9603 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -449,6 +449,7 @@ port_infos_display(portid_t port_id)
 	struct rte_mempool * mp;
 	static const char *info_border = "*********************";
 	portid_t pid;
+	uint16_t mtu;
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
 		printf("Valid port range is [0");
@@ -480,6 +481,10 @@ port_infos_display(portid_t port_id)
 	printf("Link speed: %u Mbps\n", (unsigned) link.link_speed);
 	printf("Link duplex: %s\n", (link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
 	       ("full-duplex") : ("half-duplex"));
+
+	if (!rte_eth_dev_get_mtu(port_id, &mtu))
+		printf("MTU: %u\n", mtu);
+
 	printf("Promiscuous mode: %s\n",
 	       rte_eth_promiscuous_get(port_id) ? "enabled" : "disabled");
 	printf("Allmulticast mode: %s\n",
-- 
2.9.3

Re: [dpdk-dev] [PATCH v2 0/7] virtio/vhost: Add MTU feature support

[Thomas Monjalon]

2017-03-06 09:27, Maxime Coquelin:
> This series target v17.05 release.

I think you should add an entry in the release notes.

Re: [dpdk-dev] [PATCH v2 0/7] virtio/vhost: Add MTU feature support

[Maxime Coquelin]

On 03/07/2017 10:57 PM, Thomas Monjalon wrote:
> 2017-03-06 09:27, Maxime Coquelin:
>> This series target v17.05 release.
>
> I think you should add an entry in the release notes.

Sure. Will be in upcoming revision.

Thanks,
Maxime

[dpdk-dev] [PATCH v3 0/9] virtio/vhost: Add MTU feature support

[Maxime Coquelin]

This series adds support to new Virtio's MTU feature[1]. The MTU
value is set via QEMU parameters.

If the feature is negotiated (i.e supported by both host and guest,
and valid MTU value is set in QEMU via its host_mtu parameter), QEMU
shares the configured MTU value throught dedicated Vhost protocol
feature.

On vhost side, the value is stored in the virtio_net structure, and
made available to the application thanks to new vhost lib's
rte_vhost_get_mtu() function.

To be able to set eth_dev's MTU value at the right time, i.e. to call
rte_vhost_get_mtu() just after Virtio features have been negotiated
and before the device is really started, a new vhost flag has been
introduced (VIRTIO_DEV_READY), because the VIRTIO_DEV_RUNNING flag is
set too late (after .new_device() ops is called).

Regarding valid MTU values, the maximum MTU value accepted on vhost
side is 65535 bytes, as defined in Virtio Spec and supported in
Virtio-net Kernel driver. But in Virtio PMD, current maximum frame
size is 9728 bytes (~9700 bytes MTU). So maximum MTU size accepted in
Virtio PMD is the minimum between ~9700 bytes and host's MTU.

Finally, this series also adds MTU value printing  in testpmd's
"show port info" command when non-zero.

This series target v17.05 release.

Cheers,
Maxime

[1]: https://lists.oasis-open.org/archives/virtio-dev/201609/msg00128.html

Changes since v1:
-----------------
* Rebased on top of v17.02
* Virtio PMD: ensure MTU value is valid before ack'ing the feature (Aaron)
* Vhost lib/PMD: Remove MTU setting API/op (Yuanhan)

Changes since v2:
-----------------
* Update release notes (Thomas)
* s/rte_vhost_mtu_get/rte_vhost_get_mtu/ (Yuanhan)
* Use %"PRIu64" instead of %lu (Yuanhan)
* Add rte_vhost_get_mtu in rte_vhost_version.map

Maxime Coquelin (9):
  vhost: Enable VIRTIO_NET_F_MTU feature
  vhost: vhost-user: Add MTU protocol feature support
  vhost: Add new ready status flag
  vhost: Add API to get MTU value
  vhost: export MTU value
  net/vhost: Fill rte_eth_dev's MTU property
  net/virtio: Add MTU feature support
  doc: announce Virtio and Vhost MTU support
  app/testpmd: print MTU value in show port info

 app/test-pmd/config.c                  |  5 ++++
 doc/guides/nics/features/virtio.ini    |  1 +
 doc/guides/rel_notes/release_17_05.rst |  8 ++++++
 drivers/net/vhost/rte_eth_vhost.c      |  2 ++
 drivers/net/virtio/virtio_ethdev.c     | 45 ++++++++++++++++++++++++++++++++--
 drivers/net/virtio/virtio_ethdev.h     |  3 ++-
 drivers/net/virtio/virtio_pci.h        |  3 +++
 lib/librte_vhost/rte_vhost_version.map |  7 ++++++
 lib/librte_vhost/rte_virtio_net.h      | 15 ++++++++++++
 lib/librte_vhost/vhost.c               | 22 ++++++++++++++++-
 lib/librte_vhost/vhost.h               |  9 ++++++-
 lib/librte_vhost/vhost_user.c          | 44 +++++++++++++++++++++++++++------
 lib/librte_vhost/vhost_user.h          |  5 +++-
 13 files changed, 156 insertions(+), 13 deletions(-)

-- 
2.9.3

[dpdk-dev] [PATCH v3 1/9] vhost: Enable VIRTIO_NET_F_MTU feature

[Maxime Coquelin]

This patch enables the new VIRTIO_NET_F_MTU feature,
which makes possible for the host to advise the guest
with its maximum supported MTU.

MTU value is set via QEMU parameters, either via Libvirt XML, or
directly in virtio-net device command line arguments.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost.c | 3 ++-
 lib/librte_vhost/vhost.h | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index e415093..3974087 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -66,7 +66,8 @@
 				(1ULL << VIRTIO_NET_F_GUEST_CSUM) | \
 				(1ULL << VIRTIO_NET_F_GUEST_TSO4) | \
 				(1ULL << VIRTIO_NET_F_GUEST_TSO6) | \
-				(1ULL << VIRTIO_RING_F_INDIRECT_DESC))
+				(1ULL << VIRTIO_RING_F_INDIRECT_DESC) | \
+				(1ULL << VIRTIO_NET_F_MTU))
 
 uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;
 
diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 22564f1..6a57bb3 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -110,11 +110,15 @@ struct vhost_virtqueue {
 	uint16_t                shadow_used_idx;
 } __rte_cache_aligned;
 
-/* Old kernels have no such macro defined */
+/* Old kernels have no such macros defined */
 #ifndef VIRTIO_NET_F_GUEST_ANNOUNCE
  #define VIRTIO_NET_F_GUEST_ANNOUNCE 21
 #endif
 
+#ifndef VIRTIO_NET_F_MTU
+ #define VIRTIO_NET_F_MTU 3
+#endif
+
 
 /*
  * Make an extra wrapper for VIRTIO_NET_F_MQ and
-- 
2.9.3

[dpdk-dev] [PATCH v3 2/9] vhost: vhost-user: Add MTU protocol feature support

[Maxime Coquelin]

This patch implements the vhost-user MTU protocol feature support.
When VIRTIO_NET_F_MTU is negotiated, QEMU notifies the vhost-user
backend with the configured MTU if dedicated protocol feature is
supported.

The value can be used by the application to ensure consistency with
value set by the user.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost.h      |  1 +
 lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++++++++++
 lib/librte_vhost/vhost_user.h |  5 ++++-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 6a57bb3..549296f 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -170,6 +170,7 @@ struct virtio_net {
 	uint64_t		log_base;
 	uint64_t		log_addr;
 	struct ether_addr	mac;
+	uint16_t		mtu;
 
 	uint32_t		nr_guest_pages;
 	uint32_t		max_guest_pages;
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index cb2156a..f53fc03 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -51,6 +51,9 @@
 #include "vhost.h"
 #include "vhost_user.h"
 
+#define VIRTIO_MIN_MTU 68
+#define VIRTIO_MAX_MTU 65535
+
 static const char *vhost_message_str[VHOST_USER_MAX] = {
 	[VHOST_USER_NONE] = "VHOST_USER_NONE",
 	[VHOST_USER_GET_FEATURES] = "VHOST_USER_GET_FEATURES",
@@ -72,6 +75,7 @@ static const char *vhost_message_str[VHOST_USER_MAX] = {
 	[VHOST_USER_GET_QUEUE_NUM]  = "VHOST_USER_GET_QUEUE_NUM",
 	[VHOST_USER_SET_VRING_ENABLE]  = "VHOST_USER_SET_VRING_ENABLE",
 	[VHOST_USER_SEND_RARP]  = "VHOST_USER_SEND_RARP",
+	[VHOST_USER_NET_SET_MTU]  = "VHOST_USER_NET_SET_MTU",
 };
 
 static uint64_t
@@ -865,6 +869,22 @@ vhost_user_send_rarp(struct virtio_net *dev, struct VhostUserMsg *msg)
 	return 0;
 }
 
+static int
+vhost_user_net_set_mtu(struct virtio_net *dev, struct VhostUserMsg *msg)
+{
+	if (msg->payload.u64 < VIRTIO_MIN_MTU ||
+			msg->payload.u64 > VIRTIO_MAX_MTU) {
+		RTE_LOG(ERR, VHOST_CONFIG, "Invalid MTU size (%"PRIu64")\n",
+				msg->payload.u64);
+
+		return -1;
+	}
+
+	dev->mtu = msg->payload.u64;
+
+	return 0;
+}
+
 /* return bytes# of read on success or negative val on failure. */
 static int
 read_vhost_message(int sockfd, struct VhostUserMsg *msg)
@@ -1027,6 +1047,10 @@ vhost_user_msg_handler(int vid, int fd)
 		vhost_user_send_rarp(dev, &msg);
 		break;
 
+	case VHOST_USER_NET_SET_MTU:
+		ret = vhost_user_net_set_mtu(dev, &msg);
+		break;
+
 	default:
 		ret = -1;
 		break;
diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
index 179e441..838dec8 100644
--- a/lib/librte_vhost/vhost_user.h
+++ b/lib/librte_vhost/vhost_user.h
@@ -47,11 +47,13 @@
 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD	1
 #define VHOST_USER_PROTOCOL_F_RARP	2
 #define VHOST_USER_PROTOCOL_F_REPLY_ACK	3
+#define VHOST_USER_PROTOCOL_F_NET_MTU 4
 
 #define VHOST_USER_PROTOCOL_FEATURES	((1ULL << VHOST_USER_PROTOCOL_F_MQ) | \
 					 (1ULL << VHOST_USER_PROTOCOL_F_LOG_SHMFD) |\
 					 (1ULL << VHOST_USER_PROTOCOL_F_RARP) | \
-					 (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
+					 (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK) | \
+					 (1ULL << VHOST_USER_PROTOCOL_F_NET_MTU))
 
 typedef enum VhostUserRequest {
 	VHOST_USER_NONE = 0,
@@ -74,6 +76,7 @@ typedef enum VhostUserRequest {
 	VHOST_USER_GET_QUEUE_NUM = 17,
 	VHOST_USER_SET_VRING_ENABLE = 18,
 	VHOST_USER_SEND_RARP = 19,
+	VHOST_USER_NET_SET_MTU = 20,
 	VHOST_USER_MAX
 } VhostUserRequest;
 
-- 
2.9.3

[dpdk-dev] [PATCH v3 3/9] vhost: Add new ready status flag

[Maxime Coquelin]

This patch adds a new status flag indicating the Virtio device
is ready to operate.

This is required to be able to call rte_vhost_mtu_get() in the
.new_device() callback, as rte_vhost_mtu_get needs that the
negotiation is done, but it is too early to rely on running status
flag, which is set just after .new_device() returns.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost.h      |  2 ++
 lib/librte_vhost/vhost_user.c | 20 +++++++++++++-------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 549296f..e8b7e44 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -46,6 +46,8 @@
 
 /* Used to indicate that the device is running on a data core */
 #define VIRTIO_DEV_RUNNING 1
+/* Used to indicate that the device is ready to operate */
+#define VIRTIO_DEV_READY 2
 
 /* Backend value set by guest. */
 #define VIRTIO_DEV_STOPPED -1
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index f53fc03..72361df 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -691,14 +691,18 @@ vhost_user_set_vring_kick(struct virtio_net *dev, struct VhostUserMsg *pmsg)
 		close(vq->kickfd);
 	vq->kickfd = file.fd;
 
-	if (virtio_is_ready(dev) && !(dev->flags & VIRTIO_DEV_RUNNING)) {
-		if (dev->dequeue_zero_copy) {
-			RTE_LOG(INFO, VHOST_CONFIG,
-				"dequeue zero copy is enabled\n");
-		}
+	if (virtio_is_ready(dev)) {
+		dev->flags |= VIRTIO_DEV_READY;
+
+		if (!(dev->flags & VIRTIO_DEV_RUNNING)) {
+			if (dev->dequeue_zero_copy) {
+				RTE_LOG(INFO, VHOST_CONFIG,
+						"dequeue zero copy is enabled\n");
+			}
 
-		if (notify_ops->new_device(dev->vid) == 0)
-			dev->flags |= VIRTIO_DEV_RUNNING;
+			if (notify_ops->new_device(dev->vid) == 0)
+				dev->flags |= VIRTIO_DEV_RUNNING;
+		}
 	}
 }
 
@@ -733,6 +737,8 @@ vhost_user_get_vring_base(struct virtio_net *dev,
 		notify_ops->destroy_device(dev->vid);
 	}
 
+	dev->flags &= ~VIRTIO_DEV_READY;
+
 	/* Here we are safe to get the last used index */
 	state->num = vq->last_used_idx;
 
-- 
2.9.3

[dpdk-dev] [PATCH v3 4/9] vhost: Add API to get MTU value

[Maxime Coquelin]

This patch implements the function for the application to
get the MTU value.

rte_vhost_get_mtu() fills the mtu parameter with the MTU value
set in QEMU if VIRTIO_NET_F_MTU has been negotiated and returns 0,
-ENOTSUP otherwise.

The function returns -EAGAIN if Virtio feature negotiation
didn't happened yet.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/rte_virtio_net.h | 15 +++++++++++++++
 lib/librte_vhost/vhost.c          | 19 +++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
index 926039c..56829aa 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h
@@ -100,6 +100,21 @@ int rte_vhost_driver_callback_register(struct virtio_net_device_ops const * cons
 int rte_vhost_driver_session_start(void);
 
 /**
+ * Get the MTU value of the device if set in QEMU.
+ *
+ * @param vid
+ *  virtio-net device ID
+ * @param mtu
+ *  The variable to store the MTU value
+ *
+ * @return
+ *  0: success
+ *  -EAGAIN: device not yet started
+ *  -ENOTSUP: device does not support MTU feature
+ */
+int rte_vhost_get_mtu(int vid, uint16_t *mtu);
+
+/**
  * Get the numa node from which the virtio net device's memory
  * is allocated.
  *
diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index 3974087..80a75e0 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -313,6 +313,25 @@ vhost_enable_dequeue_zero_copy(int vid)
 }
 
 int
+rte_vhost_get_mtu(int vid, uint16_t *mtu)
+{
+	struct virtio_net *dev = get_device(vid);
+
+	if (!dev)
+		return -ENODEV;
+
+	if (!(dev->flags & VIRTIO_DEV_READY))
+		return -EAGAIN;
+
+	if (!(dev->features & VIRTIO_NET_F_MTU))
+		return -ENOTSUP;
+
+	*mtu = dev->mtu;
+
+	return 0;
+}
+
+int
 rte_vhost_get_numa_node(int vid)
 {
 #ifdef RTE_LIBRTE_VHOST_NUMA
-- 
2.9.3

Re: [dpdk-dev] [PATCH v3 4/9] vhost: Add API to get MTU value

[Yuanhan Liu]

On Sun, Mar 12, 2017 at 05:34:01PM +0100, Maxime Coquelin wrote:
> This patch implements the function for the application to
> get the MTU value.

I'm thinking the other way. As we are making vhost being generic, it
doesn't make too much sense now to store a net specific value at vhost
layer. I'm thinking we may could introduce a vhost-user request handler,
something like:

	rte_vhost_driver_register_msg_handler(socket, handler);

All vhost-user message then will goto the driver's sepcific handler.
if it's handlered, do nothing in vhost lib. Otherwise, we handle it
in vhost lib.

In this way, you could handle the mtu message inside vhost-pmd; thus,
there is no need to introduce one more (net specific) API.

Thoughts?

	--yliu

Re: [dpdk-dev] [PATCH v3 4/9] vhost: Add API to get MTU value

[Maxime Coquelin]

On 03/16/2017 09:00 AM, Yuanhan Liu wrote:
> On Sun, Mar 12, 2017 at 05:34:01PM +0100, Maxime Coquelin wrote:
>> This patch implements the function for the application to
>> get the MTU value.
>
> I'm thinking the other way. As we are making vhost being generic, it
> doesn't make too much sense now to store a net specific value at vhost
> layer. I'm thinking we may could introduce a vhost-user request handler,
> something like:
>
> 	rte_vhost_driver_register_msg_handler(socket, handler);

That's a good point.

> All vhost-user message then will goto the driver's sepcific handler.
> if it's handlered, do nothing in vhost lib. Otherwise, we handle it
> in vhost lib.
>
> In this way, you could handle the mtu message inside vhost-pmd; thus,
> there is no need to introduce one more (net specific) API.
>
> Thoughts?

I need to think more about it, but advantage of having a dedicated API
is that in case the MTU value is not available, you can know from
return code whether it is not yet available (-EAGAIN), or not supported
(-ENOTSUP).

That could be managed with the callback tough, by calling the callback
with a 0 mtu value if not supported, so that the application can be
sure that if the callback hasn't been called, then it is just that it
is not ready yet.

What do you think?

Maxime

Re: [dpdk-dev] [PATCH v3 4/9] vhost: Add API to get MTU value

[Yuanhan Liu]

On Thu, Mar 16, 2017 at 12:37:23PM +0100, Maxime Coquelin wrote:
> 
> 
> On 03/16/2017 09:00 AM, Yuanhan Liu wrote:
> >On Sun, Mar 12, 2017 at 05:34:01PM +0100, Maxime Coquelin wrote:
> >>This patch implements the function for the application to
> >>get the MTU value.
> >
> >I'm thinking the other way. As we are making vhost being generic, it
> >doesn't make too much sense now to store a net specific value at vhost
> >layer. I'm thinking we may could introduce a vhost-user request handler,
> >something like:
> >
> >	rte_vhost_driver_register_msg_handler(socket, handler);
> 
> That's a good point.
> 
> >All vhost-user message then will goto the driver's sepcific handler.
> >if it's handlered, do nothing in vhost lib. Otherwise, we handle it
> >in vhost lib.
> >
> >In this way, you could handle the mtu message inside vhost-pmd; thus,
> >there is no need to introduce one more (net specific) API.
> >
> >Thoughts?
> 
> I need to think more about it, but advantage of having a dedicated API
> is that in case the MTU value is not available, you can know from
> return code whether it is not yet available (-EAGAIN), or not supported
> (-ENOTSUP).
> 
> That could be managed with the callback tough, by calling the callback
> with a 0 mtu value if not supported, so that the application can be
> sure that if the callback hasn't been called, then it is just that it
> is not ready yet.
> 
> What do you think?

I don't think the application should even be aware of the callback.
Application should get the mtu from the ethdev layer, by the API
rte_eth_dev_get_mtu(). And such MTU request should be only handled
in vhost-pmd, to serve the rte_eth_dev_get_mtu() API.

	--yliu

Re: [dpdk-dev] [PATCH v3 4/9] vhost: Add API to get MTU value

[Maxime Coquelin]

On 03/17/2017 06:32 AM, Yuanhan Liu wrote:
> On Thu, Mar 16, 2017 at 12:37:23PM +0100, Maxime Coquelin wrote:
>>
>>
>> On 03/16/2017 09:00 AM, Yuanhan Liu wrote:
>>> On Sun, Mar 12, 2017 at 05:34:01PM +0100, Maxime Coquelin wrote:
>>>> This patch implements the function for the application to
>>>> get the MTU value.
>>>
>>> I'm thinking the other way. As we are making vhost being generic, it
>>> doesn't make too much sense now to store a net specific value at vhost
>>> layer. I'm thinking we may could introduce a vhost-user request handler,
>>> something like:
>>>
>>> 	rte_vhost_driver_register_msg_handler(socket, handler);
>>
>> That's a good point.
>>
>>> All vhost-user message then will goto the driver's sepcific handler.
>>> if it's handlered, do nothing in vhost lib. Otherwise, we handle it
>>> in vhost lib.
>>>
>>> In this way, you could handle the mtu message inside vhost-pmd; thus,
>>> there is no need to introduce one more (net specific) API.
>>>
>>> Thoughts?
>>
>> I need to think more about it, but advantage of having a dedicated API
>> is that in case the MTU value is not available, you can know from
>> return code whether it is not yet available (-EAGAIN), or not supported
>> (-ENOTSUP).
>>
>> That could be managed with the callback tough, by calling the callback
>> with a 0 mtu value if not supported, so that the application can be
>> sure that if the callback hasn't been called, then it is just that it
>> is not ready yet.
>>
>> What do you think?
>
> I don't think the application should even be aware of the callback.
> Application should get the mtu from the ethdev layer, by the API
> rte_eth_dev_get_mtu(). And such MTU request should be only handled
> in vhost-pmd, to serve the rte_eth_dev_get_mtu() API.

I thought about OVS, which doesn't rely on Vhost PMD (at least didn't
last time I checked).

Maxime

Re: [dpdk-dev] [PATCH v3 4/9] vhost: Add API to get MTU value

[Yuanhan Liu]

On Fri, Mar 17, 2017 at 10:59:12AM +0100, Maxime Coquelin wrote:
> 
> 
> On 03/17/2017 06:32 AM, Yuanhan Liu wrote:
> >On Thu, Mar 16, 2017 at 12:37:23PM +0100, Maxime Coquelin wrote:
> >>
> >>
> >>On 03/16/2017 09:00 AM, Yuanhan Liu wrote:
> >>>On Sun, Mar 12, 2017 at 05:34:01PM +0100, Maxime Coquelin wrote:
> >>>>This patch implements the function for the application to
> >>>>get the MTU value.
> >>>
> >>>I'm thinking the other way. As we are making vhost being generic, it
> >>>doesn't make too much sense now to store a net specific value at vhost
> >>>layer. I'm thinking we may could introduce a vhost-user request handler,
> >>>something like:
> >>>
> >>>	rte_vhost_driver_register_msg_handler(socket, handler);
> >>
> >>That's a good point.
> >>
> >>>All vhost-user message then will goto the driver's sepcific handler.
> >>>if it's handlered, do nothing in vhost lib. Otherwise, we handle it
> >>>in vhost lib.
> >>>
> >>>In this way, you could handle the mtu message inside vhost-pmd; thus,
> >>>there is no need to introduce one more (net specific) API.
> >>>
> >>>Thoughts?
> >>
> >>I need to think more about it, but advantage of having a dedicated API
> >>is that in case the MTU value is not available, you can know from
> >>return code whether it is not yet available (-EAGAIN), or not supported
> >>(-ENOTSUP).
> >>
> >>That could be managed with the callback tough, by calling the callback
> >>with a 0 mtu value if not supported, so that the application can be
> >>sure that if the callback hasn't been called, then it is just that it
> >>is not ready yet.
> >>
> >>What do you think?
> >
> >I don't think the application should even be aware of the callback.
> >Application should get the mtu from the ethdev layer, by the API
> >rte_eth_dev_get_mtu(). And such MTU request should be only handled
> >in vhost-pmd, to serve the rte_eth_dev_get_mtu() API.
> 
> I thought about OVS, which doesn't rely on Vhost PMD (at least didn't
> last time I checked).

Oh, right. Let's keep this API then.

	--yliu

[dpdk-dev] [PATCH v3 5/9] vhost: export MTU value

[Maxime Coquelin]

Introduce a new API rte_vhost_get_mtu() to export the MTU value
of the interface to the application.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/rte_vhost_version.map | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
index 5ceaa8a..30b4671 100644
--- a/lib/librte_vhost/rte_vhost_version.map
+++ b/lib/librte_vhost/rte_vhost_version.map
@@ -30,3 +30,10 @@ DPDK_16.07 {
 	rte_vhost_get_queue_num;
 
 } DPDK_2.1;
+
+DPDK_17.05 {
+	global:
+
+	rte_vhost_get_mtu;
+
+} DPDK_16.07;
-- 
2.9.3

[dpdk-dev] [PATCH v3 6/9] net/vhost: Fill rte_eth_dev's MTU property

[Maxime Coquelin]

This patch adds a call to rte_vhost_mtu_get() at device creation
time to fill device's MTU property when available.

This makes the MTU value defined in QEMU cmdline accessible to the
application by calling rte_eth_dev_get_mtu().

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/vhost/rte_eth_vhost.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index e98cffd..36814cb 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -575,6 +575,8 @@ new_device(int vid)
 	for (i = 0; i < rte_vhost_get_queue_num(vid) * VIRTIO_QNUM; i++)
 		rte_vhost_enable_guest_notification(vid, i, 0);
 
+	rte_vhost_get_mtu(vid, &eth_dev->data->mtu);
+
 	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
 
 	rte_atomic32_set(&internal->dev_attached, 1);
-- 
2.9.3

[dpdk-dev] [PATCH v3 7/9] net/virtio: Add MTU feature support

[Maxime Coquelin]

This patch implements support for the Virtio MTU feature.
When negotiated, the host shares its maximum supported MTU,
which is used as initial MTU and as maximum MTU the application
can set.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 doc/guides/nics/features/virtio.ini |  1 +
 drivers/net/virtio/virtio_ethdev.c  | 45 +++++++++++++++++++++++++++++++++++--
 drivers/net/virtio/virtio_ethdev.h  |  3 ++-
 drivers/net/virtio/virtio_pci.h     |  3 +++
 4 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/doc/guides/nics/features/virtio.ini b/doc/guides/nics/features/virtio.ini
index 84d2012..8e3aca1 100644
--- a/doc/guides/nics/features/virtio.ini
+++ b/doc/guides/nics/features/virtio.ini
@@ -25,3 +25,4 @@ ARMv8                = Y
 x86-32               = Y
 x86-64               = Y
 Usage doc            = Y
+MTU update           = Y
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 4dc03b9..fd1213c 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -721,10 +721,13 @@ virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_LEN +
 				 hw->vtnet_hdr_size;
 	uint32_t frame_size = mtu + ether_hdr_len;
+	uint32_t max_frame_size = hw->max_mtu + ether_hdr_len;
 
-	if (mtu < ETHER_MIN_MTU || frame_size > VIRTIO_MAX_RX_PKTLEN) {
+	max_frame_size = RTE_MIN(max_frame_size, VIRTIO_MAX_RX_PKTLEN);
+
+	if (mtu < ETHER_MIN_MTU || frame_size > max_frame_size) {
 		PMD_INIT_LOG(ERR, "MTU should be between %d and %d",
-			ETHER_MIN_MTU, VIRTIO_MAX_RX_PKTLEN - ether_hdr_len);
+			ETHER_MIN_MTU, max_frame_size - ether_hdr_len);
 		return -EINVAL;
 	}
 	return 0;
@@ -1158,6 +1161,18 @@ virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features)
 	PMD_INIT_LOG(DEBUG, "host_features before negotiate = %" PRIx64,
 		host_features);
 
+	/* If supported, ensure MTU value is valid before acknowledging it. */
+	if (host_features & req_features & (1ULL << VIRTIO_NET_F_MTU)) {
+		struct virtio_net_config config;
+
+		vtpci_read_dev_config(hw,
+			offsetof(struct virtio_net_config, mtu),
+			&config.mtu, sizeof(config.mtu));
+
+		if (config.mtu < ETHER_MIN_MTU)
+			req_features &= ~(1ULL << VIRTIO_NET_F_MTU);
+	}
+
 	/*
 	 * Negotiate features: Subset of device feature bits are written back
 	 * guest feature bits.
@@ -1392,6 +1407,32 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
 
 		hw->max_queue_pairs = config->max_virtqueue_pairs;
 
+		if (vtpci_with_feature(hw, VIRTIO_NET_F_MTU)) {
+			vtpci_read_dev_config(hw,
+				offsetof(struct virtio_net_config, mtu),
+				&config->mtu,
+				sizeof(config->mtu));
+
+			/*
+			 * MTU value has already been checked at negotiation
+			 * time, but check again in case it has changed since
+			 * then, which should not happen.
+			 */
+			if (config->mtu < ETHER_MIN_MTU) {
+				PMD_INIT_LOG(ERR, "invalid max MTU value (%u)",
+						config->mtu);
+				return -1;
+			}
+
+			hw->max_mtu = config->mtu;
+			/* Set initial MTU to maximum one supported by vhost */
+			eth_dev->data->mtu = config->mtu;
+
+		} else {
+			hw->max_mtu = VIRTIO_MAX_RX_PKTLEN - ETHER_HDR_LEN -
+				VLAN_TAG_LEN - hw->vtnet_hdr_size;
+		}
+
 		PMD_INIT_LOG(DEBUG, "config->max_virtqueue_pairs=%d",
 				config->max_virtqueue_pairs);
 		PMD_INIT_LOG(DEBUG, "config->status=%d", config->status);
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index 777a14b..aa78adc 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -51,7 +51,7 @@
 #define VIRTIO_MAX_TX_QUEUES 128U
 #define VIRTIO_MAX_MAC_ADDRS 64
 #define VIRTIO_MIN_RX_BUFSIZE 64
-#define VIRTIO_MAX_RX_PKTLEN  9728
+#define VIRTIO_MAX_RX_PKTLEN  9728U
 
 /* Features desired/implemented by this driver. */
 #define VIRTIO_PMD_DEFAULT_GUEST_FEATURES	\
@@ -66,6 +66,7 @@
 	 1u << VIRTIO_NET_F_HOST_TSO4	  |	\
 	 1u << VIRTIO_NET_F_HOST_TSO6	  |	\
 	 1u << VIRTIO_NET_F_MRG_RXBUF	  |	\
+	 1u << VIRTIO_NET_F_MTU	| \
 	 1u << VIRTIO_RING_F_INDIRECT_DESC |    \
 	 1ULL << VIRTIO_F_VERSION_1       |	\
 	 1ULL << VIRTIO_F_IOMMU_PLATFORM)
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 59e45c4..771f5b1 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -106,6 +106,7 @@ struct virtnet_ctl;
 /* The feature bitmap for virtio net */
 #define VIRTIO_NET_F_CSUM	0	/* Host handles pkts w/ partial csum */
 #define VIRTIO_NET_F_GUEST_CSUM	1	/* Guest handles pkts w/ partial csum */
+#define VIRTIO_NET_F_MTU	3	/* Initial MTU advice. */
 #define VIRTIO_NET_F_MAC	5	/* Host has given MAC address. */
 #define VIRTIO_NET_F_GUEST_TSO4	7	/* Guest can handle TSOv4 in. */
 #define VIRTIO_NET_F_GUEST_TSO6	8	/* Guest can handle TSOv6 in. */
@@ -251,6 +252,7 @@ struct virtio_hw {
 	uint64_t    req_guest_features;
 	uint64_t    guest_features;
 	uint32_t    max_queue_pairs;
+	uint16_t	max_mtu;
 	uint16_t    vtnet_hdr_size;
 	uint8_t	    vlan_strip;
 	uint8_t	    use_msix;
@@ -296,6 +298,7 @@ struct virtio_net_config {
 	/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
 	uint16_t   status;
 	uint16_t   max_virtqueue_pairs;
+	uint16_t   mtu;
 } __attribute__((packed));
 
 /*
-- 
2.9.3

Re: [dpdk-dev] [PATCH v3 7/9] net/virtio: Add MTU feature support

[Tan, Jianfeng]

Hi Maxime,

Have some confusion about this feature. Please help confirm.

(1) With this feature, we only support to advertise MTU value which is 
defined by QEMU to frontend and backend driver separately. (2) But it 
does not allow frontend driver to set a different MTU to QEMU and then 
to vhost backend.

Correct?

If it's correct, why not MTU works like (2)?

Thanks,
Jianfeng

Re: [dpdk-dev] [PATCH v3 7/9] net/virtio: Add MTU feature support

[Maxime Coquelin]

Hi Jianfeng,

On 04/05/2017 06:52 AM, Tan, Jianfeng wrote:
> Hi Maxime,
>
> Have some confusion about this feature. Please help confirm.
>
> (1) With this feature, we only support to advertise MTU value which is
> defined by QEMU to frontend and backend driver separately. (2) But it
> does not allow frontend driver to set a different MTU to QEMU and then
> to vhost backend.
>
> Correct?
> If it's correct, why not MTU works like (2)?

Because idea is that the hosts advertises the maximum MTU value it
supports. The frontend driver is free to use a smaller value. The goal
of this change is to make possible to set a uniform MTU value across
the infrastructure, the management tools giving a hint to the guests on
the MTU value it should use.

Having the frontend setting the backend MTU dynamically would require 
some negotiation at runtime, and is out of the scope of this change.
Do you have some use cases for this?

Regards,
Maxime
> Thanks,
> Jianfeng

Re: [dpdk-dev] [PATCH v3 7/9] net/virtio: Add MTU feature support

[Tan, Jianfeng]

Hi Maxime,

Thank you for replying.

On 4/5/2017 3:11 PM, Maxime Coquelin wrote:
> Hi Jianfeng,
>
> On 04/05/2017 06:52 AM, Tan, Jianfeng wrote:
>> Hi Maxime,
>>
>> Have some confusion about this feature. Please help confirm.
>>
>> (1) With this feature, we only support to advertise MTU value which is
>> defined by QEMU to frontend and backend driver separately. (2) But it
>> does not allow frontend driver to set a different MTU to QEMU and then
>> to vhost backend.
>>
>> Correct?
>> If it's correct, why not MTU works like (2)?
>
> Because idea is that the hosts advertises the maximum MTU value it
> supports. The frontend driver is free to use a smaller value. The goal
> of this change is to make possible to set a uniform MTU value across
> the infrastructure, the management tools giving a hint to the guests on
> the MTU value it should use.

Based on that MTU is the maximum packet size that can be sent instead of 
that can be received:
(1) Why vhost (as a device) does not drop any packets which are longer 
than MTU when dequeue()?
(2) See some NICs also use MTU to calculate maximum packet size that can 
be received, like ixgbe, i40e, shall we also do that?

>
> Having the frontend setting the backend MTU dynamically would require 
> some negotiation at runtime, and is out of the scope of this change.
> Do you have some use cases for this?

No, just to understand more about this feature. Thank you!

Thanks,
Jianfeng

>
> Regards,
> Maxime
>> Thanks,
>> Jianfeng

Re: [dpdk-dev] [PATCH v3 7/9] net/virtio: Add MTU feature support

[Maxime Coquelin]

On 04/05/2017 11:42 AM, Tan, Jianfeng wrote:
> Hi Maxime,
>
> Thank you for replying.
>
> On 4/5/2017 3:11 PM, Maxime Coquelin wrote:
>> Hi Jianfeng,
>>
>> On 04/05/2017 06:52 AM, Tan, Jianfeng wrote:
>>> Hi Maxime,
>>>
>>> Have some confusion about this feature. Please help confirm.
>>>
>>> (1) With this feature, we only support to advertise MTU value which is
>>> defined by QEMU to frontend and backend driver separately. (2) But it
>>> does not allow frontend driver to set a different MTU to QEMU and then
>>> to vhost backend.
>>>
>>> Correct?
>>> If it's correct, why not MTU works like (2)?
>>
>> Because idea is that the hosts advertises the maximum MTU value it
>> supports. The frontend driver is free to use a smaller value. The goal
>> of this change is to make possible to set a uniform MTU value across
>> the infrastructure, the management tools giving a hint to the guests on
>> the MTU value it should use.
>
> Based on that MTU is the maximum packet size that can be sent instead of
> that can be received:
> (1) Why vhost (as a device) does not drop any packets which are longer
> than MTU when dequeue()?
That's a good point.
As when MTU value is negotiated, the guest agrees not to send larger
packets. But we cannot trust the guest, so vhost needs to check the
packet length.

> (2) See some NICs also use MTU to calculate maximum packet size that can
> be received, like ixgbe, i40e, shall we also do that?
Can you give me some pointers to the code?

Thanks,
Maxime

Re: [dpdk-dev] [PATCH v3 7/9] net/virtio: Add MTU feature support

[Tan, Jianfeng]

On 4/5/2017 9:54 PM, Maxime Coquelin wrote:
>
>
> On 04/05/2017 11:42 AM, Tan, Jianfeng wrote:
>> Hi Maxime,
>>
>> Thank you for replying.
>>
>> On 4/5/2017 3:11 PM, Maxime Coquelin wrote:
>>> Hi Jianfeng,
>>>
>>> On 04/05/2017 06:52 AM, Tan, Jianfeng wrote:
>>>> Hi Maxime,
>>>>
>>>> Have some confusion about this feature. Please help confirm.
>>>>
>>>> (1) With this feature, we only support to advertise MTU value which is
>>>> defined by QEMU to frontend and backend driver separately. (2) But it
>>>> does not allow frontend driver to set a different MTU to QEMU and then
>>>> to vhost backend.
>>>>
>>>> Correct?
>>>> If it's correct, why not MTU works like (2)?
>>>
>>> Because idea is that the hosts advertises the maximum MTU value it
>>> supports. The frontend driver is free to use a smaller value. The goal
>>> of this change is to make possible to set a uniform MTU value across
>>> the infrastructure, the management tools giving a hint to the guests on
>>> the MTU value it should use.
>>
>> Based on that MTU is the maximum packet size that can be sent instead of
>> that can be received:
>> (1) Why vhost (as a device) does not drop any packets which are longer
>> than MTU when dequeue()?
> That's a good point.
> As when MTU value is negotiated, the guest agrees not to send larger
> packets. But we cannot trust the guest, so vhost needs to check the
> packet length.
>
>> (2) See some NICs also use MTU to calculate maximum packet size that can
>> be received, like ixgbe, i40e, shall we also do that?
> Can you give me some pointers to the code?

Please refer ixgbe_dev_mtu_set(), and i40e_dev_mtu_set().

Thanks,
Jianfeng

Re: [dpdk-dev] [PATCH v3 7/9] net/virtio: Add MTU feature support

[Maxime Coquelin]

Hi Jianfeng,

On 04/05/2017 04:50 PM, Tan, Jianfeng wrote:
>
>
> On 4/5/2017 9:54 PM, Maxime Coquelin wrote:
>>
>>
>> On 04/05/2017 11:42 AM, Tan, Jianfeng wrote:
>>> Hi Maxime,
>>>
>>> Thank you for replying.
>>>
>>> On 4/5/2017 3:11 PM, Maxime Coquelin wrote:
>>>> Hi Jianfeng,
>>>>
>>>> On 04/05/2017 06:52 AM, Tan, Jianfeng wrote:
>>>>> Hi Maxime,
>>>>>
>>>>> Have some confusion about this feature. Please help confirm.
>>>>>
>>>>> (1) With this feature, we only support to advertise MTU value which is
>>>>> defined by QEMU to frontend and backend driver separately. (2) But it
>>>>> does not allow frontend driver to set a different MTU to QEMU and then
>>>>> to vhost backend.
>>>>>
>>>>> Correct?
>>>>> If it's correct, why not MTU works like (2)?
>>>>
>>>> Because idea is that the hosts advertises the maximum MTU value it
>>>> supports. The frontend driver is free to use a smaller value. The goal
>>>> of this change is to make possible to set a uniform MTU value across
>>>> the infrastructure, the management tools giving a hint to the guests on
>>>> the MTU value it should use.
>>>
>>> Based on that MTU is the maximum packet size that can be sent instead of
>>> that can be received:
>>> (1) Why vhost (as a device) does not drop any packets which are longer
>>> than MTU when dequeue()?
>> That's a good point.
>> As when MTU value is negotiated, the guest agrees not to send larger
>> packets. But we cannot trust the guest, so vhost needs to check the
>> packet length.
>>
>>> (2) See some NICs also use MTU to calculate maximum packet size that can
>>> be received, like ixgbe, i40e, shall we also do that?
>> Can you give me some pointers to the code?
>
> Please refer ixgbe_dev_mtu_set(), and i40e_dev_mtu_set().

Thanks. Yes, we could also do that.

I can send a patch for this and another one to check the size of the
packet respects negotiated MTU value. Or maybe you want to do this?

Regards,
Maxime

Re: [dpdk-dev] [PATCH v3 7/9] net/virtio: Add MTU feature support

[Tan, Jianfeng]

> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Saturday, April 8, 2017 12:41 AM
> To: Tan, Jianfeng; aconole@redhat.com; sodey@sonusnet.com;
> yuanhan.liu@linux.intel.com; thomas.monjalon@6wind.com; dev@dpdk.org
> Subject: Re: [PATCH v3 7/9] net/virtio: Add MTU feature support
> 
> Hi Jianfeng,
> 
> On 04/05/2017 04:50 PM, Tan, Jianfeng wrote:
> >
> >
> > On 4/5/2017 9:54 PM, Maxime Coquelin wrote:
> >>
> >>
> >> On 04/05/2017 11:42 AM, Tan, Jianfeng wrote:
> >>> Hi Maxime,
> >>>
> >>> Thank you for replying.
> >>>
> >>> On 4/5/2017 3:11 PM, Maxime Coquelin wrote:
> >>>> Hi Jianfeng,
> >>>>
> >>>> On 04/05/2017 06:52 AM, Tan, Jianfeng wrote:
> >>>>> Hi Maxime,
> >>>>>
> >>>>> Have some confusion about this feature. Please help confirm.
> >>>>>
> >>>>> (1) With this feature, we only support to advertise MTU value which is
> >>>>> defined by QEMU to frontend and backend driver separately. (2) But
> it
> >>>>> does not allow frontend driver to set a different MTU to QEMU and
> then
> >>>>> to vhost backend.
> >>>>>
> >>>>> Correct?
> >>>>> If it's correct, why not MTU works like (2)?
> >>>>
> >>>> Because idea is that the hosts advertises the maximum MTU value it
> >>>> supports. The frontend driver is free to use a smaller value. The goal
> >>>> of this change is to make possible to set a uniform MTU value across
> >>>> the infrastructure, the management tools giving a hint to the guests on
> >>>> the MTU value it should use.
> >>>
> >>> Based on that MTU is the maximum packet size that can be sent instead
> of
> >>> that can be received:
> >>> (1) Why vhost (as a device) does not drop any packets which are longer
> >>> than MTU when dequeue()?
> >> That's a good point.
> >> As when MTU value is negotiated, the guest agrees not to send larger
> >> packets. But we cannot trust the guest, so vhost needs to check the
> >> packet length.
> >>
> >>> (2) See some NICs also use MTU to calculate maximum packet size that
> can
> >>> be received, like ixgbe, i40e, shall we also do that?
> >> Can you give me some pointers to the code?
> >
> > Please refer ixgbe_dev_mtu_set(), and i40e_dev_mtu_set().
> 
> Thanks. Yes, we could also do that.
> 
> I can send a patch for this and another one to check the size of the
> packet respects negotiated MTU value. Or maybe you want to do this?

I'll help review then.

Thanks,
Jianfeng

[dpdk-dev] [PATCH v3 8/9] doc: announce Virtio and Vhost MTU support

[Maxime Coquelin]

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 doc/guides/rel_notes/release_17_05.rst | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/doc/guides/rel_notes/release_17_05.rst b/doc/guides/rel_notes/release_17_05.rst
index 4b90036..86a3363 100644
--- a/doc/guides/rel_notes/release_17_05.rst
+++ b/doc/guides/rel_notes/release_17_05.rst
@@ -46,6 +46,14 @@ New Features
 
   sPAPR IOMMU based pci probing enabled for vfio-pci devices.
 
+* **Added MTU feature support to Virtio and Vhost.**
+
+  Implemented new Virtio MTU feature into Vhost and Virtio:
+
+  * Add ``rte_vhost_mtu_get()`` API to Vhost library.
+  * Enable Vhost PMD's MTU get feature.
+  * Get max MTU value from host in Virtio PMD
+
 Resolved Issues
 ---------------
 
-- 
2.9.3

[dpdk-dev] [PATCH v3 9/9] app/testpmd: print MTU value in show port info

[Maxime Coquelin]

This patch adds MTU display to "show port info" command.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 app/test-pmd/config.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 80491fc..73d9603 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -449,6 +449,7 @@ port_infos_display(portid_t port_id)
 	struct rte_mempool * mp;
 	static const char *info_border = "*********************";
 	portid_t pid;
+	uint16_t mtu;
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
 		printf("Valid port range is [0");
@@ -480,6 +481,10 @@ port_infos_display(portid_t port_id)
 	printf("Link speed: %u Mbps\n", (unsigned) link.link_speed);
 	printf("Link duplex: %s\n", (link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
 	       ("full-duplex") : ("half-duplex"));
+
+	if (!rte_eth_dev_get_mtu(port_id, &mtu))
+		printf("MTU: %u\n", mtu);
+
 	printf("Promiscuous mode: %s\n",
 	       rte_eth_promiscuous_get(port_id) ? "enabled" : "disabled");
 	printf("Allmulticast mode: %s\n",
-- 
2.9.3

Re: [dpdk-dev] [PATCH v3 0/9] virtio/vhost: Add MTU feature support

[Yuanhan Liu]

On Sun, Mar 12, 2017 at 05:33:57PM +0100, Maxime Coquelin wrote:
> This series adds support to new Virtio's MTU feature[1]. The MTU
> value is set via QEMU parameters.

Series applied to dpdk-next-virtio, with following minor changes:

- don't use upper case for subject
- squashed patch 5 to 4
- squashed patch 8 to 7

Thanks.

	--yliu

Re: [dpdk-dev] [PATCH v3 0/9] virtio/vhost: Add MTU feature support

[Maxime Coquelin]

On 03/22/2017 09:58 AM, Yuanhan Liu wrote:
> On Sun, Mar 12, 2017 at 05:33:57PM +0100, Maxime Coquelin wrote:
>> This series adds support to new Virtio's MTU feature[1]. The MTU
>> value is set via QEMU parameters.
>
> Series applied to dpdk-next-virtio, with following minor changes:
>
> - don't use upper case for subject
> - squashed patch 5 to 4
> - squashed patch 8 to 7

Thanks for having handled the changes.

Maxime
>
> Thanks.
>
> 	--yliu
>

Re: [dpdk-dev] [PATCH v3 0/9] virtio/vhost: Add MTU feature support

[Yao, Lei A]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Maxime Coquelin
> Sent: Monday, March 13, 2017 12:34 AM
> To: aconole@redhat.com; sodey@sonusnet.com;
> yuanhan.liu@linux.intel.com; Tan, Jianfeng <jianfeng.tan@intel.com>;
> thomas.monjalon@6wind.com; dev@dpdk.org
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [dpdk-dev] [PATCH v3 0/9] virtio/vhost: Add MTU feature support
> 
> This series adds support to new Virtio's MTU feature[1]. The MTU
> value is set via QEMU parameters.
> 
> If the feature is negotiated (i.e supported by both host and guest,
> and valid MTU value is set in QEMU via its host_mtu parameter), QEMU
> shares the configured MTU value throught dedicated Vhost protocol
> feature.
> 
> On vhost side, the value is stored in the virtio_net structure, and
> made available to the application thanks to new vhost lib's
> rte_vhost_get_mtu() function.
> 
> To be able to set eth_dev's MTU value at the right time, i.e. to call
> rte_vhost_get_mtu() just after Virtio features have been negotiated
> and before the device is really started, a new vhost flag has been
> introduced (VIRTIO_DEV_READY), because the VIRTIO_DEV_RUNNING flag is
> set too late (after .new_device() ops is called).
> 
> Regarding valid MTU values, the maximum MTU value accepted on vhost
> side is 65535 bytes, as defined in Virtio Spec and supported in
> Virtio-net Kernel driver. But in Virtio PMD, current maximum frame
> size is 9728 bytes (~9700 bytes MTU). So maximum MTU size accepted in
> Virtio PMD is the minimum between ~9700 bytes and host's MTU.
> 
> Finally, this series also adds MTU value printing  in testpmd's
> "show port info" command when non-zero.
> 
> This series target v17.05 release.
> 
> Cheers,
> Maxime
> 
> [1]: https://lists.oasis-open.org/archives/virtio-dev/201609/msg00128.html
> 
> Changes since v1:
> -----------------
> * Rebased on top of v17.02
> * Virtio PMD: ensure MTU value is valid before ack'ing the feature (Aaron)
> * Vhost lib/PMD: Remove MTU setting API/op (Yuanhan)
> 
> Changes since v2:
> -----------------
> * Update release notes (Thomas)
> * s/rte_vhost_mtu_get/rte_vhost_get_mtu/ (Yuanhan)
> * Use %"PRIu64" instead of %lu (Yuanhan)
> * Add rte_vhost_get_mtu in rte_vhost_version.map
> 
> Maxime Coquelin (9):
>   vhost: Enable VIRTIO_NET_F_MTU feature
>   vhost: vhost-user: Add MTU protocol feature support
>   vhost: Add new ready status flag
>   vhost: Add API to get MTU value
>   vhost: export MTU value
>   net/vhost: Fill rte_eth_dev's MTU property
>   net/virtio: Add MTU feature support
>   doc: announce Virtio and Vhost MTU support
>   app/testpmd: print MTU value in show port info
> 
>  app/test-pmd/config.c                  |  5 ++++
>  doc/guides/nics/features/virtio.ini    |  1 +
>  doc/guides/rel_notes/release_17_05.rst |  8 ++++++
>  drivers/net/vhost/rte_eth_vhost.c      |  2 ++
>  drivers/net/virtio/virtio_ethdev.c     | 45
> ++++++++++++++++++++++++++++++++--
>  drivers/net/virtio/virtio_ethdev.h     |  3 ++-
>  drivers/net/virtio/virtio_pci.h        |  3 +++
>  lib/librte_vhost/rte_vhost_version.map |  7 ++++++
>  lib/librte_vhost/rte_virtio_net.h      | 15 ++++++++++++
>  lib/librte_vhost/vhost.c               | 22 ++++++++++++++++-
>  lib/librte_vhost/vhost.h               |  9 ++++++-
>  lib/librte_vhost/vhost_user.c          | 44 +++++++++++++++++++++++++++---
> ---
>  lib/librte_vhost/vhost_user.h          |  5 +++-
>  13 files changed, 156 insertions(+), 13 deletions(-)
> 
> --
> 2.9.3
Hi, Maxime

If I want have a try for this MTU function, is there any specific requirement for the settings? 
Such as the qemu version, kernel version or any others? Looks like this feature are very new 
in Qemu and linux side. 
Thanks a lot!

BRs
Lei

Re: [dpdk-dev] [PATCH v3 0/9] virtio/vhost: Add MTU feature support

[Maxime Coquelin]

Hi Lei,

On 03/28/2017 07:39 AM, Yao, Lei A wrote:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Maxime Coquelin
>> Sent: Monday, March 13, 2017 12:34 AM
>> To: aconole@redhat.com; sodey@sonusnet.com;
>> yuanhan.liu@linux.intel.com; Tan, Jianfeng <jianfeng.tan@intel.com>;
>> thomas.monjalon@6wind.com; dev@dpdk.org
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [dpdk-dev] [PATCH v3 0/9] virtio/vhost: Add MTU feature support
>>
>> This series adds support to new Virtio's MTU feature[1]. The MTU
>> value is set via QEMU parameters.
>>
>> If the feature is negotiated (i.e supported by both host and guest,
>> and valid MTU value is set in QEMU via its host_mtu parameter), QEMU
>> shares the configured MTU value throught dedicated Vhost protocol
>> feature.
>>
>> On vhost side, the value is stored in the virtio_net structure, and
>> made available to the application thanks to new vhost lib's
>> rte_vhost_get_mtu() function.
>>
>> To be able to set eth_dev's MTU value at the right time, i.e. to call
>> rte_vhost_get_mtu() just after Virtio features have been negotiated
>> and before the device is really started, a new vhost flag has been
>> introduced (VIRTIO_DEV_READY), because the VIRTIO_DEV_RUNNING flag is
>> set too late (after .new_device() ops is called).
>>
>> Regarding valid MTU values, the maximum MTU value accepted on vhost
>> side is 65535 bytes, as defined in Virtio Spec and supported in
>> Virtio-net Kernel driver. But in Virtio PMD, current maximum frame
>> size is 9728 bytes (~9700 bytes MTU). So maximum MTU size accepted in
>> Virtio PMD is the minimum between ~9700 bytes and host's MTU.
>>
>> Finally, this series also adds MTU value printing  in testpmd's
>> "show port info" command when non-zero.
>>
>> This series target v17.05 release.
>>
>> Cheers,
>> Maxime
>>
>> [1]: https://lists.oasis-open.org/archives/virtio-dev/201609/msg00128.html
>>
>> Changes since v1:
>> -----------------
>> * Rebased on top of v17.02
>> * Virtio PMD: ensure MTU value is valid before ack'ing the feature (Aaron)
>> * Vhost lib/PMD: Remove MTU setting API/op (Yuanhan)
>>
>> Changes since v2:
>> -----------------
>> * Update release notes (Thomas)
>> * s/rte_vhost_mtu_get/rte_vhost_get_mtu/ (Yuanhan)
>> * Use %"PRIu64" instead of %lu (Yuanhan)
>> * Add rte_vhost_get_mtu in rte_vhost_version.map
>>
>> Maxime Coquelin (9):
>>   vhost: Enable VIRTIO_NET_F_MTU feature
>>   vhost: vhost-user: Add MTU protocol feature support
>>   vhost: Add new ready status flag
>>   vhost: Add API to get MTU value
>>   vhost: export MTU value
>>   net/vhost: Fill rte_eth_dev's MTU property
>>   net/virtio: Add MTU feature support
>>   doc: announce Virtio and Vhost MTU support
>>   app/testpmd: print MTU value in show port info
>>
>>  app/test-pmd/config.c                  |  5 ++++
>>  doc/guides/nics/features/virtio.ini    |  1 +
>>  doc/guides/rel_notes/release_17_05.rst |  8 ++++++
>>  drivers/net/vhost/rte_eth_vhost.c      |  2 ++
>>  drivers/net/virtio/virtio_ethdev.c     | 45
>> ++++++++++++++++++++++++++++++++--
>>  drivers/net/virtio/virtio_ethdev.h     |  3 ++-
>>  drivers/net/virtio/virtio_pci.h        |  3 +++
>>  lib/librte_vhost/rte_vhost_version.map |  7 ++++++
>>  lib/librte_vhost/rte_virtio_net.h      | 15 ++++++++++++
>>  lib/librte_vhost/vhost.c               | 22 ++++++++++++++++-
>>  lib/librte_vhost/vhost.h               |  9 ++++++-
>>  lib/librte_vhost/vhost_user.c          | 44 +++++++++++++++++++++++++++---
>> ---
>>  lib/librte_vhost/vhost_user.h          |  5 +++-
>>  13 files changed, 156 insertions(+), 13 deletions(-)
>>
>> --
>> 2.9.3
> Hi, Maxime
>
> If I want have a try for this MTU function, is there any specific requirement for the settings?
> Such as the qemu version, kernel version or any others? Looks like this feature are very new
> in Qemu and linux side.
> Thanks a lot!

Sorry, for the delay.

You need QEMU v2.9.0-rc at least, as this is not in v2.8?
Regarding the Kernel, there are no version dependency for the host.
For the guest, this is only if you use the Kernel virtio-net driver.
In this case, your guest Kernel should be at least v4.10.

Regards,
Maxime