[dpdk-dev] [PATCH v2 0/7] ethdev: add min/max MTU to device info

[dpdk-dev] [PATCH v2 0/7] ethdev: add min/max MTU to device info

[Ian Stokes]

Building upon the discussion around [1], this series introduces MTU min
and MTU max variables. It also provides updates to PMD implementations
for ixgbe, i40e and IGB devices so that these variables are populated
for use when retrieving device info.

This series was tested with OVS DPDK and functions as expected for the
drivers listed below. But a wider selection of PMD drivers would have to
adopt this to ensure jumbo frames functionality remains for drivers not
modified in the series.

There is also ongoing discussion in [2] regarding overhead to be
considered with MTU and how this may change from device to device, this
series uses existing overhead assumptions.

This series was previously posted as an RFC in [3] and a v1 in [4],
this revision implements changes received in feedback on v1.

[1] http://mails.dpdk.org/archives/dev/2018-September/110959.html
[2] http://mails.dpdk.org/archives/dev/2019-February/124457.html
[3] http://mails.dpdk.org/archives/dev/2019-February/124938.html
[4] http://mails.dpdk.org/archives/dev/2019-February/125319.html

Ian Stokes (6):
  net/i40e: set min and max MTU for i40e devices
  net/i40e: set min and max MTU for i40e VF devices
  net/ixgbe: set min and max MTU for ixgbe devices
  net/ixgbe: set min and max MTU for ixgbe VF devices
  net/e1000: set min and max MTU for igb devices
  app/testpmd: verify mtu with rte_eth_dev_info_get()

Stephen Hemminger (1):
  ethdev: add min/max MTU to device info

 app/test-pmd/config.c                  |  4 ++++
 doc/guides/rel_notes/deprecation.rst   | 12 ------------
 doc/guides/rel_notes/release_19_05.rst |  8 +++++++-
 drivers/net/e1000/e1000_ethdev.h       |  6 ++++++
 drivers/net/e1000/igb_ethdev.c         |  7 +++++--
 drivers/net/i40e/i40e_ethdev.c         |  2 ++
 drivers/net/i40e/i40e_ethdev_vf.c      |  2 ++
 drivers/net/ixgbe/ixgbe_ethdev.c       |  7 +++++--
 drivers/net/ixgbe/ixgbe_ethdev.h       |  3 +++
 lib/librte_ethdev/Makefile             |  2 +-
 lib/librte_ethdev/meson.build          |  2 +-
 lib/librte_ethdev/rte_ethdev.c         | 15 +++++++++++++++
 lib/librte_ethdev/rte_ethdev.h         | 33 ++++++++++++++++++++++++++++++++-
 13 files changed, 83 insertions(+), 20 deletions(-)

-- 
2.13.6

[dpdk-dev] [PATCH v2 0/7] ethdev: add min/max MTU to device info

[Ian Stokes]

Building upon the discussion around [1], this series introduces MTU min
and MTU max variables. It also provides updates to PMD implementations
for ixgbe, i40e and IGB devices so that these variables are populated
for use when retrieving device info.

This series was tested with OVS DPDK and functions as expected for the
drivers listed below. But a wider selection of PMD drivers would have to
adopt this to ensure jumbo frames functionality remains for drivers not
modified in the series.

There is also ongoing discussion in [2] regarding overhead to be
considered with MTU and how this may change from device to device, this
series uses existing overhead assumptions.

This series was previously posted as an RFC in [3] and a v1 in [4],
this revision implements changes received in feedback on v1.

[1] http://mails.dpdk.org/archives/dev/2018-September/110959.html
[2] http://mails.dpdk.org/archives/dev/2019-February/124457.html
[3] http://mails.dpdk.org/archives/dev/2019-February/124938.html
[4] http://mails.dpdk.org/archives/dev/2019-February/125319.html

Ian Stokes (6):
  net/i40e: set min and max MTU for i40e devices
  net/i40e: set min and max MTU for i40e VF devices
  net/ixgbe: set min and max MTU for ixgbe devices
  net/ixgbe: set min and max MTU for ixgbe VF devices
  net/e1000: set min and max MTU for igb devices
  app/testpmd: verify mtu with rte_eth_dev_info_get()

Stephen Hemminger (1):
  ethdev: add min/max MTU to device info

 app/test-pmd/config.c                  |  4 ++++
 doc/guides/rel_notes/deprecation.rst   | 12 ------------
 doc/guides/rel_notes/release_19_05.rst |  8 +++++++-
 drivers/net/e1000/e1000_ethdev.h       |  6 ++++++
 drivers/net/e1000/igb_ethdev.c         |  7 +++++--
 drivers/net/i40e/i40e_ethdev.c         |  2 ++
 drivers/net/i40e/i40e_ethdev_vf.c      |  2 ++
 drivers/net/ixgbe/ixgbe_ethdev.c       |  7 +++++--
 drivers/net/ixgbe/ixgbe_ethdev.h       |  3 +++
 lib/librte_ethdev/Makefile             |  2 +-
 lib/librte_ethdev/meson.build          |  2 +-
 lib/librte_ethdev/rte_ethdev.c         | 15 +++++++++++++++
 lib/librte_ethdev/rte_ethdev.h         | 33 ++++++++++++++++++++++++++++++++-
 13 files changed, 83 insertions(+), 20 deletions(-)

-- 
2.13.6

[dpdk-dev] [PATCH v2 1/7] ethdev: add min/max MTU to device info

[Ian Stokes]

From: Stephen Hemminger <stephen@networkplumber.org>

This addresses the usability issue raised by OVS at DPDK Userspace
summit. It adds general min/max mtu into device info. For compatiablity,
and to save space, it fits in a hole in existing structure.

The initial version sets max mtu to normal Ethernet, it is up to
PMD to set larger value if it supports Jumbo frames.

Also remove the deprecation notice introduced in 18.11 regarding this
change and bump ethdev ABI version.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
v1 -> v2
* Document default values set in rte_eth_dev_info_get() in doxygen
  documentation.
* Document causes of EINVAL returns for rte_eth_dev_set_mtu() in doxygen
  documentation.
* Check if dev_infos_get is supported before validating mtu values in
  rte_eth_dev_mtu_set. If unsupported skip validation step.

RFC -> v1
* Removed RFC status.
* dev_info->max_mtu set to UINT16_MAX initially instead of ETHER_MTU to
  avoid breaking jumbo frame support. ETHER_MTU to be re-introduced
  when all PMD drivers modified to support min/max mtu support.
* Bump ethdev ABI version.
* Added ACK from Andrew Rybchenko.
---
 doc/guides/rel_notes/deprecation.rst   | 12 ------------
 doc/guides/rel_notes/release_19_05.rst |  8 +++++++-
 lib/librte_ethdev/Makefile             |  2 +-
 lib/librte_ethdev/meson.build          |  2 +-
 lib/librte_ethdev/rte_ethdev.c         | 15 +++++++++++++++
 lib/librte_ethdev/rte_ethdev.h         | 33 ++++++++++++++++++++++++++++++++-
 6 files changed, 56 insertions(+), 16 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 1b4fcb7e6..0e85c47f3 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -56,18 +56,6 @@ Deprecation Notices
   Target release for removal of the legacy API will be defined once most
   PMDs have switched to rte_flow.
 
-* ethdev: Maximum and minimum MTU values vary between hardware devices. In
-  hardware agnostic DPDK applications access to such information would allow
-  a more accurate way of validating and setting supported MTU values on a per
-  device basis rather than using a defined default for all devices. To
-  resolve this, the following members will be added to ``rte_eth_dev_info``.
-  Note: these can be added to fit a hole in the existing structure for amd64
-  but not for 32-bit, as such ABI change will occur as size of the structure
-  will increase.
-
-  - Member ``uint16_t min_mtu`` the minimum MTU allowed.
-  - Member ``uint16_t max_mtu`` the maximum MTU allowed.
-
 * meter: New ``rte_color`` definition will be added in 19.02 and that will
   replace ``enum rte_meter_color`` in meter library in 19.05. This will help
   to consolidate color definition, which is currently replicated in many places,
diff --git a/doc/guides/rel_notes/release_19_05.rst b/doc/guides/rel_notes/release_19_05.rst
index 61a2c7383..6ba3fab9a 100644
--- a/doc/guides/rel_notes/release_19_05.rst
+++ b/doc/guides/rel_notes/release_19_05.rst
@@ -136,6 +136,12 @@ ABI Changes
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+* ethdev: Additional fields in rte_eth_dev_info.
+
+  The ``rte_eth_dev_info`` structure has had two extra fields
+  added: ``min_mtu`` and ``max_mtu``. Each of these are of type ``uint16_t``.
+  The values of these fields can be set specifically by the PMD drivers as
+  supported values can vary from device to device.
 
 Shared Library Versions
 -----------------------
@@ -171,7 +177,7 @@ The libraries prepended with a plus sign were incremented in this version.
      librte_distributor.so.1
      librte_eal.so.9
      librte_efd.so.1
-     librte_ethdev.so.11
+   + librte_ethdev.so.12
      librte_eventdev.so.6
      librte_flow_classify.so.1
      librte_gro.so.1
diff --git a/lib/librte_ethdev/Makefile b/lib/librte_ethdev/Makefile
index e09c4263a..8d4a02630 100644
--- a/lib/librte_ethdev/Makefile
+++ b/lib/librte_ethdev/Makefile
@@ -16,7 +16,7 @@ LDLIBS += -lrte_mbuf -lrte_kvargs -lrte_cmdline -lrte_meter
 
 EXPORT_MAP := rte_ethdev_version.map
 
-LIBABIVER := 11
+LIBABIVER := 12
 
 SRCS-y += ethdev_private.c
 SRCS-y += rte_ethdev.c
diff --git a/lib/librte_ethdev/meson.build b/lib/librte_ethdev/meson.build
index e98942ff8..8d6165b2a 100644
--- a/lib/librte_ethdev/meson.build
+++ b/lib/librte_ethdev/meson.build
@@ -2,7 +2,7 @@
 # Copyright(c) 2017 Intel Corporation
 
 name = 'ethdev'
-version = 11
+version = 12
 allow_experimental_apis = true
 sources = files('ethdev_private.c',
 	'ethdev_profile.c',
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 85c179496..fe72e88c2 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -2524,6 +2524,8 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	dev_info->rx_desc_lim = lim;
 	dev_info->tx_desc_lim = lim;
 	dev_info->device = dev->device;
+	dev_info->min_mtu = ETHER_MIN_MTU;
+	dev_info->max_mtu = UINT16_MAX;
 
 	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
 	(*dev->dev_ops->dev_infos_get)(dev, dev_info);
@@ -2587,12 +2589,25 @@ int
 rte_eth_dev_set_mtu(uint16_t port_id, uint16_t mtu)
 {
 	int ret;
+	struct rte_eth_dev_info dev_info;
 	struct rte_eth_dev *dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mtu_set, -ENOTSUP);
 
+	/*
+	 * Check if the device supports dev_infos_get, if it does not
+	 * skip min_mtu/max_mtu validation here as this requires values
+	 * that are populated within the call to rte_eth_dev_info_get()
+	 * which relies on dev->dev_ops->dev_infos_get.
+	 */
+	if (*dev->dev_ops->dev_infos_get != NULL) {
+		rte_eth_dev_info_get(port_id, &dev_info);
+		if (mtu < dev_info.min_mtu || mtu > dev_info.max_mtu)
+			return -EINVAL;
+	}
+
 	ret = (*dev->dev_ops->mtu_set)(dev, mtu);
 	if (!ret)
 		dev->data->mtu = mtu;
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index a3c864a13..bea1bd2d4 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1086,6 +1086,8 @@ struct rte_eth_dev_info {
 	const char *driver_name; /**< Device Driver name. */
 	unsigned int if_index; /**< Index to bound host interface, or 0 if none.
 		Use if_indextoname() to translate into an interface name. */
+	uint16_t min_mtu;	/**< Minimum MTU allowed */
+	uint16_t max_mtu;	/**< Maximum MTU allowed */
 	const uint32_t *dev_flags; /**< Device flags */
 	uint32_t min_rx_bufsize; /**< Minimum size of RX buffer. */
 	uint32_t max_rx_pktlen; /**< Maximum configurable length of RX pkt. */
@@ -2202,6 +2204,33 @@ void rte_eth_macaddr_get(uint16_t port_id, struct ether_addr *mac_addr);
 /**
  * Retrieve the contextual information of an Ethernet device.
  *
+ * As part of this function, a number of of fields in dev_info will be
+ * initialized as follows:
+ *
+ * rx_desc_lim = lim
+ * tx_desc_lim = lim
+ *
+ * Where lim is defined within the rte_eth_dev_info_get as
+ *
+ *  const struct rte_eth_desc_lim lim = {
+ *      .nb_max = UINT16_MAX,
+ *      .nb_min = 0,
+ *      .nb_align = 1,
+ *  };
+ *
+ * device = dev->device
+ * min_mtu = ETHER_MIN_MTU
+ * max_mtu = UINT16_MAX
+ *
+ * The following fields will be populated if support for dev_infos_get()
+ * exists for the device and the rte_eth_dev 'dev' has been populated
+ * successfully with a call to it:
+ *
+ * driver_name = dev->device->driver->name
+ * nb_rx_queues = dev->data->nb_rx_queues
+ * nb_tx_queues = dev->data->nb_tx_queues
+ * dev_flags = &dev->data->dev_flags
+ *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param dev_info
@@ -2298,7 +2327,9 @@ int rte_eth_dev_get_mtu(uint16_t port_id, uint16_t *mtu);
  *   - (-ENOTSUP) if operation is not supported.
  *   - (-ENODEV) if *port_id* invalid.
  *   - (-EIO) if device is removed.
- *   - (-EINVAL) if *mtu* invalid.
+ *   - (-EINVAL) if *mtu* invalid, validation of mtu can occur within
+ *     rte_eth_dev_set_mtu if dev_infos_get is supported by the device or
+ *     when the mtu is set using dev->dev_ops->mtu_set.
  *   - (-EBUSY) if operation is not allowed when the port is running
  */
 int rte_eth_dev_set_mtu(uint16_t port_id, uint16_t mtu);
-- 
2.13.6

[dpdk-dev] [PATCH v2 1/7] ethdev: add min/max MTU to device info

[Ian Stokes]

From: Stephen Hemminger <stephen@networkplumber.org>

This addresses the usability issue raised by OVS at DPDK Userspace
summit. It adds general min/max mtu into device info. For compatiablity,
and to save space, it fits in a hole in existing structure.

The initial version sets max mtu to normal Ethernet, it is up to
PMD to set larger value if it supports Jumbo frames.

Also remove the deprecation notice introduced in 18.11 regarding this
change and bump ethdev ABI version.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
v1 -> v2
* Document default values set in rte_eth_dev_info_get() in doxygen
  documentation.
* Document causes of EINVAL returns for rte_eth_dev_set_mtu() in doxygen
  documentation.
* Check if dev_infos_get is supported before validating mtu values in
  rte_eth_dev_mtu_set. If unsupported skip validation step.

RFC -> v1
* Removed RFC status.
* dev_info->max_mtu set to UINT16_MAX initially instead of ETHER_MTU to
  avoid breaking jumbo frame support. ETHER_MTU to be re-introduced
  when all PMD drivers modified to support min/max mtu support.
* Bump ethdev ABI version.
* Added ACK from Andrew Rybchenko.
---
 doc/guides/rel_notes/deprecation.rst   | 12 ------------
 doc/guides/rel_notes/release_19_05.rst |  8 +++++++-
 lib/librte_ethdev/Makefile             |  2 +-
 lib/librte_ethdev/meson.build          |  2 +-
 lib/librte_ethdev/rte_ethdev.c         | 15 +++++++++++++++
 lib/librte_ethdev/rte_ethdev.h         | 33 ++++++++++++++++++++++++++++++++-
 6 files changed, 56 insertions(+), 16 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 1b4fcb7e6..0e85c47f3 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -56,18 +56,6 @@ Deprecation Notices
   Target release for removal of the legacy API will be defined once most
   PMDs have switched to rte_flow.
 
-* ethdev: Maximum and minimum MTU values vary between hardware devices. In
-  hardware agnostic DPDK applications access to such information would allow
-  a more accurate way of validating and setting supported MTU values on a per
-  device basis rather than using a defined default for all devices. To
-  resolve this, the following members will be added to ``rte_eth_dev_info``.
-  Note: these can be added to fit a hole in the existing structure for amd64
-  but not for 32-bit, as such ABI change will occur as size of the structure
-  will increase.
-
-  - Member ``uint16_t min_mtu`` the minimum MTU allowed.
-  - Member ``uint16_t max_mtu`` the maximum MTU allowed.
-
 * meter: New ``rte_color`` definition will be added in 19.02 and that will
   replace ``enum rte_meter_color`` in meter library in 19.05. This will help
   to consolidate color definition, which is currently replicated in many places,
diff --git a/doc/guides/rel_notes/release_19_05.rst b/doc/guides/rel_notes/release_19_05.rst
index 61a2c7383..6ba3fab9a 100644
--- a/doc/guides/rel_notes/release_19_05.rst
+++ b/doc/guides/rel_notes/release_19_05.rst
@@ -136,6 +136,12 @@ ABI Changes
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+* ethdev: Additional fields in rte_eth_dev_info.
+
+  The ``rte_eth_dev_info`` structure has had two extra fields
+  added: ``min_mtu`` and ``max_mtu``. Each of these are of type ``uint16_t``.
+  The values of these fields can be set specifically by the PMD drivers as
+  supported values can vary from device to device.
 
 Shared Library Versions
 -----------------------
@@ -171,7 +177,7 @@ The libraries prepended with a plus sign were incremented in this version.
      librte_distributor.so.1
      librte_eal.so.9
      librte_efd.so.1
-     librte_ethdev.so.11
+   + librte_ethdev.so.12
      librte_eventdev.so.6
      librte_flow_classify.so.1
      librte_gro.so.1
diff --git a/lib/librte_ethdev/Makefile b/lib/librte_ethdev/Makefile
index e09c4263a..8d4a02630 100644
--- a/lib/librte_ethdev/Makefile
+++ b/lib/librte_ethdev/Makefile
@@ -16,7 +16,7 @@ LDLIBS += -lrte_mbuf -lrte_kvargs -lrte_cmdline -lrte_meter
 
 EXPORT_MAP := rte_ethdev_version.map
 
-LIBABIVER := 11
+LIBABIVER := 12
 
 SRCS-y += ethdev_private.c
 SRCS-y += rte_ethdev.c
diff --git a/lib/librte_ethdev/meson.build b/lib/librte_ethdev/meson.build
index e98942ff8..8d6165b2a 100644
--- a/lib/librte_ethdev/meson.build
+++ b/lib/librte_ethdev/meson.build
@@ -2,7 +2,7 @@
 # Copyright(c) 2017 Intel Corporation
 
 name = 'ethdev'
-version = 11
+version = 12
 allow_experimental_apis = true
 sources = files('ethdev_private.c',
 	'ethdev_profile.c',
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 85c179496..fe72e88c2 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -2524,6 +2524,8 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	dev_info->rx_desc_lim = lim;
 	dev_info->tx_desc_lim = lim;
 	dev_info->device = dev->device;
+	dev_info->min_mtu = ETHER_MIN_MTU;
+	dev_info->max_mtu = UINT16_MAX;
 
 	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
 	(*dev->dev_ops->dev_infos_get)(dev, dev_info);
@@ -2587,12 +2589,25 @@ int
 rte_eth_dev_set_mtu(uint16_t port_id, uint16_t mtu)
 {
 	int ret;
+	struct rte_eth_dev_info dev_info;
 	struct rte_eth_dev *dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mtu_set, -ENOTSUP);
 
+	/*
+	 * Check if the device supports dev_infos_get, if it does not
+	 * skip min_mtu/max_mtu validation here as this requires values
+	 * that are populated within the call to rte_eth_dev_info_get()
+	 * which relies on dev->dev_ops->dev_infos_get.
+	 */
+	if (*dev->dev_ops->dev_infos_get != NULL) {
+		rte_eth_dev_info_get(port_id, &dev_info);
+		if (mtu < dev_info.min_mtu || mtu > dev_info.max_mtu)
+			return -EINVAL;
+	}
+
 	ret = (*dev->dev_ops->mtu_set)(dev, mtu);
 	if (!ret)
 		dev->data->mtu = mtu;
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index a3c864a13..bea1bd2d4 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1086,6 +1086,8 @@ struct rte_eth_dev_info {
 	const char *driver_name; /**< Device Driver name. */
 	unsigned int if_index; /**< Index to bound host interface, or 0 if none.
 		Use if_indextoname() to translate into an interface name. */
+	uint16_t min_mtu;	/**< Minimum MTU allowed */
+	uint16_t max_mtu;	/**< Maximum MTU allowed */
 	const uint32_t *dev_flags; /**< Device flags */
 	uint32_t min_rx_bufsize; /**< Minimum size of RX buffer. */
 	uint32_t max_rx_pktlen; /**< Maximum configurable length of RX pkt. */
@@ -2202,6 +2204,33 @@ void rte_eth_macaddr_get(uint16_t port_id, struct ether_addr *mac_addr);
 /**
  * Retrieve the contextual information of an Ethernet device.
  *
+ * As part of this function, a number of of fields in dev_info will be
+ * initialized as follows:
+ *
+ * rx_desc_lim = lim
+ * tx_desc_lim = lim
+ *
+ * Where lim is defined within the rte_eth_dev_info_get as
+ *
+ *  const struct rte_eth_desc_lim lim = {
+ *      .nb_max = UINT16_MAX,
+ *      .nb_min = 0,
+ *      .nb_align = 1,
+ *  };
+ *
+ * device = dev->device
+ * min_mtu = ETHER_MIN_MTU
+ * max_mtu = UINT16_MAX
+ *
+ * The following fields will be populated if support for dev_infos_get()
+ * exists for the device and the rte_eth_dev 'dev' has been populated
+ * successfully with a call to it:
+ *
+ * driver_name = dev->device->driver->name
+ * nb_rx_queues = dev->data->nb_rx_queues
+ * nb_tx_queues = dev->data->nb_tx_queues
+ * dev_flags = &dev->data->dev_flags
+ *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param dev_info
@@ -2298,7 +2327,9 @@ int rte_eth_dev_get_mtu(uint16_t port_id, uint16_t *mtu);
  *   - (-ENOTSUP) if operation is not supported.
  *   - (-ENODEV) if *port_id* invalid.
  *   - (-EIO) if device is removed.
- *   - (-EINVAL) if *mtu* invalid.
+ *   - (-EINVAL) if *mtu* invalid, validation of mtu can occur within
+ *     rte_eth_dev_set_mtu if dev_infos_get is supported by the device or
+ *     when the mtu is set using dev->dev_ops->mtu_set.
  *   - (-EBUSY) if operation is not allowed when the port is running
  */
 int rte_eth_dev_set_mtu(uint16_t port_id, uint16_t mtu);
-- 
2.13.6

Re: [dpdk-dev] [PATCH v2 1/7] ethdev: add min/max MTU to device info

[Ferruh Yigit]

On 3/22/2019 1:01 PM, Ian Stokes wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> 
> This addresses the usability issue raised by OVS at DPDK Userspace
> summit. It adds general min/max mtu into device info. For compatiablity,
> and to save space, it fits in a hole in existing structure.
> 
> The initial version sets max mtu to normal Ethernet, it is up to
> PMD to set larger value if it supports Jumbo frames.
> 
> Also remove the deprecation notice introduced in 18.11 regarding this
> change and bump ethdev ABI version.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH v2 1/7] ethdev: add min/max MTU to device info

[Ferruh Yigit]

On 3/22/2019 1:01 PM, Ian Stokes wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> 
> This addresses the usability issue raised by OVS at DPDK Userspace
> summit. It adds general min/max mtu into device info. For compatiablity,
> and to save space, it fits in a hole in existing structure.
> 
> The initial version sets max mtu to normal Ethernet, it is up to
> PMD to set larger value if it supports Jumbo frames.
> 
> Also remove the deprecation notice introduced in 18.11 regarding this
> change and bump ethdev ABI version.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

[dpdk-dev] [PATCH v2 2/7] net/i40e: set min and max MTU for i40e devices

[Ian Stokes]

This commit sets the min and max supported MTU values for i40e devices
via the i40e_dev_info_get() function. Min MTU supported is set to
ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index dca61f03a..caab1624f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -3499,6 +3499,8 @@ i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_rx_pktlen = I40E_FRAME_SIZE_MAX;
 	dev_info->max_mac_addrs = vsi->max_macaddrs;
 	dev_info->max_vfs = pci_dev->max_vfs;
+	dev_info->max_mtu = dev_info->max_rx_pktlen - I40E_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
 	dev_info->rx_queue_offload_capa = 0;
 	dev_info->rx_offload_capa =
 		DEV_RX_OFFLOAD_VLAN_STRIP |
-- 
2.13.6

[dpdk-dev] [PATCH v2 2/7] net/i40e: set min and max MTU for i40e devices

[Ian Stokes]

This commit sets the min and max supported MTU values for i40e devices
via the i40e_dev_info_get() function. Min MTU supported is set to
ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index dca61f03a..caab1624f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -3499,6 +3499,8 @@ i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_rx_pktlen = I40E_FRAME_SIZE_MAX;
 	dev_info->max_mac_addrs = vsi->max_macaddrs;
 	dev_info->max_vfs = pci_dev->max_vfs;
+	dev_info->max_mtu = dev_info->max_rx_pktlen - I40E_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
 	dev_info->rx_queue_offload_capa = 0;
 	dev_info->rx_offload_capa =
 		DEV_RX_OFFLOAD_VLAN_STRIP |
-- 
2.13.6

[dpdk-dev] [PATCH v2 3/7] net/i40e: set min and max MTU for i40e VF devices

[Ian Stokes]

This commit sets the min and max supported MTU values for i40e VF
devices via the i40evf_dev_info_get() function. Min MTU supported
is set to ETHER_MIN_MTU and max mtu is calculated as the max packet
length supported minus the transport overhead.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/i40e/i40e_ethdev_vf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 7e4bd7314..add7b2223 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -2216,6 +2216,8 @@ i40evf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_tx_queues = I40E_MAX_QP_NUM_PER_VF;
 	dev_info->min_rx_bufsize = I40E_BUF_SIZE_MIN;
 	dev_info->max_rx_pktlen = I40E_FRAME_SIZE_MAX;
+	dev_info->max_mtu = dev_info->max_rx_pktlen - I40E_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
 	dev_info->hash_key_size = (I40E_VFQF_HKEY_MAX_INDEX + 1) * sizeof(uint32_t);
 	dev_info->reta_size = ETH_RSS_RETA_SIZE_64;
 	dev_info->flow_type_rss_offloads = vf->adapter->flow_types_mask;
-- 
2.13.6

[dpdk-dev] [PATCH v2 3/7] net/i40e: set min and max MTU for i40e VF devices

[Ian Stokes]

This commit sets the min and max supported MTU values for i40e VF
devices via the i40evf_dev_info_get() function. Min MTU supported
is set to ETHER_MIN_MTU and max mtu is calculated as the max packet
length supported minus the transport overhead.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/i40e/i40e_ethdev_vf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 7e4bd7314..add7b2223 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -2216,6 +2216,8 @@ i40evf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_tx_queues = I40E_MAX_QP_NUM_PER_VF;
 	dev_info->min_rx_bufsize = I40E_BUF_SIZE_MIN;
 	dev_info->max_rx_pktlen = I40E_FRAME_SIZE_MAX;
+	dev_info->max_mtu = dev_info->max_rx_pktlen - I40E_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
 	dev_info->hash_key_size = (I40E_VFQF_HKEY_MAX_INDEX + 1) * sizeof(uint32_t);
 	dev_info->reta_size = ETH_RSS_RETA_SIZE_64;
 	dev_info->flow_type_rss_offloads = vf->adapter->flow_types_mask;
-- 
2.13.6

[dpdk-dev] [PATCH v2 4/7] net/ixgbe: set min and max MTU for ixgbe devices

[Ian Stokes]

This commit sets the min and max supported MTU values for ixgbe devices
via the ixgbe_dev_info_get() function. Min MTU supported is set to
ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead. To aid in these calculations
a new MACRO 'IXGBE_ETH_OVERHEAD' has been introduced to consolidate
overhead calculation and avoid duplication.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 4 +++-
 drivers/net/ixgbe/ixgbe_ethdev.h | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 97e10217d..60da3508c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3748,6 +3748,8 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		dev_info->max_vmdq_pools = ETH_16_POOLS;
 	else
 		dev_info->max_vmdq_pools = ETH_64_POOLS;
+	dev_info->max_mtu =  dev_info->max_rx_pktlen - IXGBE_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
 	dev_info->vmdq_queue_num = dev_info->max_rx_queues;
 	dev_info->rx_queue_offload_capa = ixgbe_get_rx_queue_offloads(dev);
 	dev_info->rx_offload_capa = (ixgbe_get_rx_port_offloads(dev) |
@@ -4939,7 +4941,7 @@ ixgbe_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	uint32_t maxfrs;
 	struct ixgbe_hw *hw;
 	struct rte_eth_dev_info dev_info;
-	uint32_t frame_size = mtu + ETHER_HDR_LEN + ETHER_CRC_LEN;
+	uint32_t frame_size = mtu + IXGBE_ETH_OVERHEAD;
 	struct rte_eth_dev_data *dev_data = dev->data;
 
 	ixgbe_dev_info_get(dev, &dev_info);
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 3fec61352..d1f61e85e 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -101,6 +101,9 @@
 #define IXGBE_5TUPLE_MAX_PRI            7
 #define IXGBE_5TUPLE_MIN_PRI            1
 
+/* The overhead from MTU to max frame size. */
+#define IXGBE_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN)
+
 /* bit of VXLAN tunnel type | 7 bits of zeros  | 8 bits of zeros*/
 #define IXGBE_FDIR_VXLAN_TUNNEL_TYPE    0x8000
 /* bit of NVGRE tunnel type | 7 bits of zeros  | 8 bits of zeros*/
-- 
2.13.6

[dpdk-dev] [PATCH v2 4/7] net/ixgbe: set min and max MTU for ixgbe devices

[Ian Stokes]

This commit sets the min and max supported MTU values for ixgbe devices
via the ixgbe_dev_info_get() function. Min MTU supported is set to
ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead. To aid in these calculations
a new MACRO 'IXGBE_ETH_OVERHEAD' has been introduced to consolidate
overhead calculation and avoid duplication.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 4 +++-
 drivers/net/ixgbe/ixgbe_ethdev.h | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 97e10217d..60da3508c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3748,6 +3748,8 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		dev_info->max_vmdq_pools = ETH_16_POOLS;
 	else
 		dev_info->max_vmdq_pools = ETH_64_POOLS;
+	dev_info->max_mtu =  dev_info->max_rx_pktlen - IXGBE_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
 	dev_info->vmdq_queue_num = dev_info->max_rx_queues;
 	dev_info->rx_queue_offload_capa = ixgbe_get_rx_queue_offloads(dev);
 	dev_info->rx_offload_capa = (ixgbe_get_rx_port_offloads(dev) |
@@ -4939,7 +4941,7 @@ ixgbe_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	uint32_t maxfrs;
 	struct ixgbe_hw *hw;
 	struct rte_eth_dev_info dev_info;
-	uint32_t frame_size = mtu + ETHER_HDR_LEN + ETHER_CRC_LEN;
+	uint32_t frame_size = mtu + IXGBE_ETH_OVERHEAD;
 	struct rte_eth_dev_data *dev_data = dev->data;
 
 	ixgbe_dev_info_get(dev, &dev_info);
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 3fec61352..d1f61e85e 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -101,6 +101,9 @@
 #define IXGBE_5TUPLE_MAX_PRI            7
 #define IXGBE_5TUPLE_MIN_PRI            1
 
+/* The overhead from MTU to max frame size. */
+#define IXGBE_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN)
+
 /* bit of VXLAN tunnel type | 7 bits of zeros  | 8 bits of zeros*/
 #define IXGBE_FDIR_VXLAN_TUNNEL_TYPE    0x8000
 /* bit of NVGRE tunnel type | 7 bits of zeros  | 8 bits of zeros*/
-- 
2.13.6

[dpdk-dev] [PATCH v2 5/7] net/ixgbe: set min and max MTU for ixgbe VF devices

[Ian Stokes]

This commit sets the min and max supported MTU values for ixgbe VF
devices via the ixgbevf_dev_set_mtu() function. Min MTU supported is
set to ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead. As transport overhead is the
same for VF and PF ixgbe devices, reuse MACRO 'IXGBE_ETH_OVERHEAD' to
avoid duplication.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 60da3508c..feec85634 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3857,6 +3857,7 @@ ixgbevf_dev_info_get(struct rte_eth_dev *dev,
 	dev_info->max_tx_queues = (uint16_t)hw->mac.max_tx_queues;
 	dev_info->min_rx_bufsize = 1024; /* cf BSIZEPACKET in SRRCTL reg */
 	dev_info->max_rx_pktlen = 9728; /* includes CRC, cf MAXFRS reg */
+	dev_info->max_mtu = dev_info->max_rx_pktlen - IXGBE_ETH_OVERHEAD;
 	dev_info->max_mac_addrs = hw->mac.num_rar_entries;
 	dev_info->max_hash_mac_addrs = IXGBE_VMDQ_NUM_UC_MAC;
 	dev_info->max_vfs = pci_dev->max_vfs;
@@ -6344,7 +6345,7 @@ static int
 ixgbevf_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
 {
 	struct ixgbe_hw *hw;
-	uint32_t max_frame = mtu + ETHER_HDR_LEN + ETHER_CRC_LEN;
+	uint32_t max_frame = mtu + IXGBE_ETH_OVERHEAD;
 	struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode;
 
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-- 
2.13.6

[dpdk-dev] [PATCH v2 5/7] net/ixgbe: set min and max MTU for ixgbe VF devices

[Ian Stokes]

This commit sets the min and max supported MTU values for ixgbe VF
devices via the ixgbevf_dev_set_mtu() function. Min MTU supported is
set to ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead. As transport overhead is the
same for VF and PF ixgbe devices, reuse MACRO 'IXGBE_ETH_OVERHEAD' to
avoid duplication.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 60da3508c..feec85634 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3857,6 +3857,7 @@ ixgbevf_dev_info_get(struct rte_eth_dev *dev,
 	dev_info->max_tx_queues = (uint16_t)hw->mac.max_tx_queues;
 	dev_info->min_rx_bufsize = 1024; /* cf BSIZEPACKET in SRRCTL reg */
 	dev_info->max_rx_pktlen = 9728; /* includes CRC, cf MAXFRS reg */
+	dev_info->max_mtu = dev_info->max_rx_pktlen - IXGBE_ETH_OVERHEAD;
 	dev_info->max_mac_addrs = hw->mac.num_rar_entries;
 	dev_info->max_hash_mac_addrs = IXGBE_VMDQ_NUM_UC_MAC;
 	dev_info->max_vfs = pci_dev->max_vfs;
@@ -6344,7 +6345,7 @@ static int
 ixgbevf_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
 {
 	struct ixgbe_hw *hw;
-	uint32_t max_frame = mtu + ETHER_HDR_LEN + ETHER_CRC_LEN;
+	uint32_t max_frame = mtu + IXGBE_ETH_OVERHEAD;
 	struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode;
 
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-- 
2.13.6

[dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Ian Stokes]

This commit sets the min and max supported MTU values for igb devices
via the eth_igb_info_get() function. Min MTU supported is set to
ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead. To aid in these calculations
a new MACRO 'E1000_ETH_OVERHEAD' has been introduced to consolidate
overhead calculation and avoid duplication.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/e1000/e1000_ethdev.h | 6 ++++++
 drivers/net/e1000/igb_ethdev.c   | 7 +++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 94edff08e..3e74cd8fe 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -89,6 +89,12 @@
 	ETH_RSS_IPV6_UDP_EX)
 
 /*
+ * The overhead from MTU to max frame size.
+ * Considering VLAN so a tag needs to be counted.
+ */
+#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE)
+
+/*
  * Maximum number of Ring Descriptors.
  *
  * Since RDLEN/TDLEN should be multiple of 128 bytes, the number of ring
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 87c9aedf2..b897e8ad4 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -2283,6 +2283,10 @@ eth_igb_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->speed_capa = ETH_LINK_SPEED_10M_HD | ETH_LINK_SPEED_10M |
 			ETH_LINK_SPEED_100M_HD | ETH_LINK_SPEED_100M |
 			ETH_LINK_SPEED_1G;
+
+	dev_info->max_mtu = dev_info->max_rx_pktlen - E1000_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
+
 }
 
 static const uint32_t *
@@ -4466,8 +4470,7 @@ eth_igb_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	uint32_t rctl;
 	struct e1000_hw *hw;
 	struct rte_eth_dev_info dev_info;
-	uint32_t frame_size = mtu + (ETHER_HDR_LEN + ETHER_CRC_LEN +
-				     VLAN_TAG_SIZE);
+	uint32_t frame_size = mtu + E1000_ETH_OVERHEAD;
 
 	hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-- 
2.13.6

[dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Ian Stokes]

This commit sets the min and max supported MTU values for igb devices
via the eth_igb_info_get() function. Min MTU supported is set to
ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead. To aid in these calculations
a new MACRO 'E1000_ETH_OVERHEAD' has been introduced to consolidate
overhead calculation and avoid duplication.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/e1000/e1000_ethdev.h | 6 ++++++
 drivers/net/e1000/igb_ethdev.c   | 7 +++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 94edff08e..3e74cd8fe 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -89,6 +89,12 @@
 	ETH_RSS_IPV6_UDP_EX)
 
 /*
+ * The overhead from MTU to max frame size.
+ * Considering VLAN so a tag needs to be counted.
+ */
+#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE)
+
+/*
  * Maximum number of Ring Descriptors.
  *
  * Since RDLEN/TDLEN should be multiple of 128 bytes, the number of ring
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 87c9aedf2..b897e8ad4 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -2283,6 +2283,10 @@ eth_igb_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->speed_capa = ETH_LINK_SPEED_10M_HD | ETH_LINK_SPEED_10M |
 			ETH_LINK_SPEED_100M_HD | ETH_LINK_SPEED_100M |
 			ETH_LINK_SPEED_1G;
+
+	dev_info->max_mtu = dev_info->max_rx_pktlen - E1000_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
+
 }
 
 static const uint32_t *
@@ -4466,8 +4470,7 @@ eth_igb_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	uint32_t rctl;
 	struct e1000_hw *hw;
 	struct rte_eth_dev_info dev_info;
-	uint32_t frame_size = mtu + (ETHER_HDR_LEN + ETHER_CRC_LEN +
-				     VLAN_TAG_SIZE);
+	uint32_t frame_size = mtu + E1000_ETH_OVERHEAD;
 
 	hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-- 
2.13.6

Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Ferruh Yigit]

On 3/22/2019 1:01 PM, Ian Stokes wrote:
> This commit sets the min and max supported MTU values for igb devices
> via the eth_igb_info_get() function. Min MTU supported is set to
> ETHER_MIN_MTU and max mtu is calculated as the max packet length
> supported minus the transport overhead. To aid in these calculations
> a new MACRO 'E1000_ETH_OVERHEAD' has been introduced to consolidate
> overhead calculation and avoid duplication.
> 
> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> ---
>  drivers/net/e1000/e1000_ethdev.h | 6 ++++++
>  drivers/net/e1000/igb_ethdev.c   | 7 +++++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
> index 94edff08e..3e74cd8fe 100644
> --- a/drivers/net/e1000/e1000_ethdev.h
> +++ b/drivers/net/e1000/e1000_ethdev.h
> @@ -89,6 +89,12 @@
>  	ETH_RSS_IPV6_UDP_EX)
>  
>  /*
> + * The overhead from MTU to max frame size.
> + * Considering VLAN so a tag needs to be counted.
> + */
> +#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE)

As an overhead, following drivers set:
i40e: HDR + CRC + 2 * VLAN
ixgbe: HDR + CRC
e1000: HDR + CRC + VLAN

I wonder if this difference is HW limitation, or driver limitation or just
implementation inconsistency.

Better to confirm it that it is not implementation inconsistency.

Wenzhuo, Konstantin, Beilei, Qi,

Can you please comment?

Thanks,
ferruh

Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Ferruh Yigit]

On 3/22/2019 1:01 PM, Ian Stokes wrote:
> This commit sets the min and max supported MTU values for igb devices
> via the eth_igb_info_get() function. Min MTU supported is set to
> ETHER_MIN_MTU and max mtu is calculated as the max packet length
> supported minus the transport overhead. To aid in these calculations
> a new MACRO 'E1000_ETH_OVERHEAD' has been introduced to consolidate
> overhead calculation and avoid duplication.
> 
> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> ---
>  drivers/net/e1000/e1000_ethdev.h | 6 ++++++
>  drivers/net/e1000/igb_ethdev.c   | 7 +++++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
> index 94edff08e..3e74cd8fe 100644
> --- a/drivers/net/e1000/e1000_ethdev.h
> +++ b/drivers/net/e1000/e1000_ethdev.h
> @@ -89,6 +89,12 @@
>  	ETH_RSS_IPV6_UDP_EX)
>  
>  /*
> + * The overhead from MTU to max frame size.
> + * Considering VLAN so a tag needs to be counted.
> + */
> +#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE)

As an overhead, following drivers set:
i40e: HDR + CRC + 2 * VLAN
ixgbe: HDR + CRC
e1000: HDR + CRC + VLAN

I wonder if this difference is HW limitation, or driver limitation or just
implementation inconsistency.

Better to confirm it that it is not implementation inconsistency.

Wenzhuo, Konstantin, Beilei, Qi,

Can you please comment?

Thanks,
ferruh

Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Zhang, Qi Z]

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, March 25, 2019 10:25 PM
> To: Stokes, Ian <ian.stokes@intel.com>; dev@dpdk.org
> Cc: stephen@networkplumber.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb
> devices
> 
> On 3/22/2019 1:01 PM, Ian Stokes wrote:
> > This commit sets the min and max supported MTU values for igb devices
> > via the eth_igb_info_get() function. Min MTU supported is set to
> > ETHER_MIN_MTU and max mtu is calculated as the max packet length
> > supported minus the transport overhead. To aid in these calculations a
> > new MACRO 'E1000_ETH_OVERHEAD' has been introduced to consolidate
> > overhead calculation and avoid duplication.
> >
> > Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> > ---
> >  drivers/net/e1000/e1000_ethdev.h | 6 ++++++
> >  drivers/net/e1000/igb_ethdev.c   | 7 +++++--
> >  2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/e1000/e1000_ethdev.h
> > b/drivers/net/e1000/e1000_ethdev.h
> > index 94edff08e..3e74cd8fe 100644
> > --- a/drivers/net/e1000/e1000_ethdev.h
> > +++ b/drivers/net/e1000/e1000_ethdev.h
> > @@ -89,6 +89,12 @@
> >  	ETH_RSS_IPV6_UDP_EX)
> >
> >  /*
> > + * The overhead from MTU to max frame size.
> > + * Considering VLAN so a tag needs to be counted.
> > + */
> > +#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN +
> > +VLAN_TAG_SIZE)
> 
> As an overhead, following drivers set:
> i40e: HDR + CRC + 2 * VLAN
> ixgbe: HDR + CRC
> e1000: HDR + CRC + VLAN
> 
> I wonder if this difference is HW limitation, or driver limitation or just
> implementation inconsistency.

I think this is implementation inconsistency 

The NIC only accept Max Frame Size.

The problem here is seems all of three setup are not perfect.

HDR + CRC + 2 * VLAN - it may allow non vlan or single vlan packet that exceed mtu. 
HDR + CRC - it may reject vlan or double vlan packet that follow mtu.  
HDR + CRC + VLAN ,  it may reject double vlan packet that follow mtu

I agree it's better to keep consistent on all drivers, but before this, we may need to decide which one we should take :)

Regards
Qi

> 
> Better to confirm it that it is not implementation inconsistency.
> 
> Wenzhuo, Konstantin, Beilei, Qi,
> 
> Can you please comment?
> 
> Thanks,
> ferruh

Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Zhang, Qi Z]

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, March 25, 2019 10:25 PM
> To: Stokes, Ian <ian.stokes@intel.com>; dev@dpdk.org
> Cc: stephen@networkplumber.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb
> devices
> 
> On 3/22/2019 1:01 PM, Ian Stokes wrote:
> > This commit sets the min and max supported MTU values for igb devices
> > via the eth_igb_info_get() function. Min MTU supported is set to
> > ETHER_MIN_MTU and max mtu is calculated as the max packet length
> > supported minus the transport overhead. To aid in these calculations a
> > new MACRO 'E1000_ETH_OVERHEAD' has been introduced to consolidate
> > overhead calculation and avoid duplication.
> >
> > Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> > ---
> >  drivers/net/e1000/e1000_ethdev.h | 6 ++++++
> >  drivers/net/e1000/igb_ethdev.c   | 7 +++++--
> >  2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/e1000/e1000_ethdev.h
> > b/drivers/net/e1000/e1000_ethdev.h
> > index 94edff08e..3e74cd8fe 100644
> > --- a/drivers/net/e1000/e1000_ethdev.h
> > +++ b/drivers/net/e1000/e1000_ethdev.h
> > @@ -89,6 +89,12 @@
> >  	ETH_RSS_IPV6_UDP_EX)
> >
> >  /*
> > + * The overhead from MTU to max frame size.
> > + * Considering VLAN so a tag needs to be counted.
> > + */
> > +#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN +
> > +VLAN_TAG_SIZE)
> 
> As an overhead, following drivers set:
> i40e: HDR + CRC + 2 * VLAN
> ixgbe: HDR + CRC
> e1000: HDR + CRC + VLAN
> 
> I wonder if this difference is HW limitation, or driver limitation or just
> implementation inconsistency.

I think this is implementation inconsistency 

The NIC only accept Max Frame Size.

The problem here is seems all of three setup are not perfect.

HDR + CRC + 2 * VLAN - it may allow non vlan or single vlan packet that exceed mtu. 
HDR + CRC - it may reject vlan or double vlan packet that follow mtu.  
HDR + CRC + VLAN ,  it may reject double vlan packet that follow mtu

I agree it's better to keep consistent on all drivers, but before this, we may need to decide which one we should take :)

Regards
Qi

> 
> Better to confirm it that it is not implementation inconsistency.
> 
> Wenzhuo, Konstantin, Beilei, Qi,
> 
> Can you please comment?
> 
> Thanks,
> ferruh

Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Ananyev, Konstantin]

Hi Qi,

> >
> > On 3/22/2019 1:01 PM, Ian Stokes wrote:
> > > This commit sets the min and max supported MTU values for igb devices
> > > via the eth_igb_info_get() function. Min MTU supported is set to
> > > ETHER_MIN_MTU and max mtu is calculated as the max packet length
> > > supported minus the transport overhead. To aid in these calculations a
> > > new MACRO 'E1000_ETH_OVERHEAD' has been introduced to consolidate
> > > overhead calculation and avoid duplication.
> > >
> > > Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> > > ---
> > >  drivers/net/e1000/e1000_ethdev.h | 6 ++++++
> > >  drivers/net/e1000/igb_ethdev.c   | 7 +++++--
> > >  2 files changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/e1000/e1000_ethdev.h
> > > b/drivers/net/e1000/e1000_ethdev.h
> > > index 94edff08e..3e74cd8fe 100644
> > > --- a/drivers/net/e1000/e1000_ethdev.h
> > > +++ b/drivers/net/e1000/e1000_ethdev.h
> > > @@ -89,6 +89,12 @@
> > >  	ETH_RSS_IPV6_UDP_EX)
> > >
> > >  /*
> > > + * The overhead from MTU to max frame size.
> > > + * Considering VLAN so a tag needs to be counted.
> > > + */
> > > +#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN +
> > > +VLAN_TAG_SIZE)
> >
> > As an overhead, following drivers set:
> > i40e: HDR + CRC + 2 * VLAN
> > ixgbe: HDR + CRC
> > e1000: HDR + CRC + VLAN
> >
> > I wonder if this difference is HW limitation, or driver limitation or just
> > implementation inconsistency.
> 
> I think this is implementation inconsistency
> 
> The NIC only accept Max Frame Size.
> 
> The problem here is seems all of three setup are not perfect.
> 
> HDR + CRC + 2 * VLAN - it may allow non vlan or single vlan packet that exceed mtu.

Hmm, wonder how?

> HDR + CRC - it may reject vlan or double vlan packet that follow mtu.
> HDR + CRC + VLAN ,  it may reject double vlan packet that follow mtu
> 
> I agree it's better to keep consistent on all drivers, but before this, we may need to decide which one we should take :)
> 
> Regards
> Qi
> 
> >
> > Better to confirm it that it is not implementation inconsistency.
> >
> > Wenzhuo, Konstantin, Beilei, Qi,
> >
> > Can you please comment?
> >
> > Thanks,
> > ferruh

Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Ananyev, Konstantin]

Hi Qi,

> >
> > On 3/22/2019 1:01 PM, Ian Stokes wrote:
> > > This commit sets the min and max supported MTU values for igb devices
> > > via the eth_igb_info_get() function. Min MTU supported is set to
> > > ETHER_MIN_MTU and max mtu is calculated as the max packet length
> > > supported minus the transport overhead. To aid in these calculations a
> > > new MACRO 'E1000_ETH_OVERHEAD' has been introduced to consolidate
> > > overhead calculation and avoid duplication.
> > >
> > > Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> > > ---
> > >  drivers/net/e1000/e1000_ethdev.h | 6 ++++++
> > >  drivers/net/e1000/igb_ethdev.c   | 7 +++++--
> > >  2 files changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/e1000/e1000_ethdev.h
> > > b/drivers/net/e1000/e1000_ethdev.h
> > > index 94edff08e..3e74cd8fe 100644
> > > --- a/drivers/net/e1000/e1000_ethdev.h
> > > +++ b/drivers/net/e1000/e1000_ethdev.h
> > > @@ -89,6 +89,12 @@
> > >  	ETH_RSS_IPV6_UDP_EX)
> > >
> > >  /*
> > > + * The overhead from MTU to max frame size.
> > > + * Considering VLAN so a tag needs to be counted.
> > > + */
> > > +#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN +
> > > +VLAN_TAG_SIZE)
> >
> > As an overhead, following drivers set:
> > i40e: HDR + CRC + 2 * VLAN
> > ixgbe: HDR + CRC
> > e1000: HDR + CRC + VLAN
> >
> > I wonder if this difference is HW limitation, or driver limitation or just
> > implementation inconsistency.
> 
> I think this is implementation inconsistency
> 
> The NIC only accept Max Frame Size.
> 
> The problem here is seems all of three setup are not perfect.
> 
> HDR + CRC + 2 * VLAN - it may allow non vlan or single vlan packet that exceed mtu.

Hmm, wonder how?

> HDR + CRC - it may reject vlan or double vlan packet that follow mtu.
> HDR + CRC + VLAN ,  it may reject double vlan packet that follow mtu
> 
> I agree it's better to keep consistent on all drivers, but before this, we may need to decide which one we should take :)
> 
> Regards
> Qi
> 
> >
> > Better to confirm it that it is not implementation inconsistency.
> >
> > Wenzhuo, Konstantin, Beilei, Qi,
> >
> > Can you please comment?
> >
> > Thanks,
> > ferruh

Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Zhang, Qi Z]

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Tuesday, March 26, 2019 10:02 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> Stokes, Ian <ian.stokes@intel.com>; dev@dpdk.org
> Cc: stephen@networkplumber.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing,
> Beilei <beilei.xing@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb
> devices
> 
> 
> Hi Qi,
> 
> > >
> > > On 3/22/2019 1:01 PM, Ian Stokes wrote:
> > > > This commit sets the min and max supported MTU values for igb
> > > > devices via the eth_igb_info_get() function. Min MTU supported is
> > > > set to ETHER_MIN_MTU and max mtu is calculated as the max packet
> > > > length supported minus the transport overhead. To aid in these
> > > > calculations a new MACRO 'E1000_ETH_OVERHEAD' has been introduced
> > > > to consolidate overhead calculation and avoid duplication.
> > > >
> > > > Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> > > > ---
> > > >  drivers/net/e1000/e1000_ethdev.h | 6 ++++++
> > > >  drivers/net/e1000/igb_ethdev.c   | 7 +++++--
> > > >  2 files changed, 11 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/net/e1000/e1000_ethdev.h
> > > > b/drivers/net/e1000/e1000_ethdev.h
> > > > index 94edff08e..3e74cd8fe 100644
> > > > --- a/drivers/net/e1000/e1000_ethdev.h
> > > > +++ b/drivers/net/e1000/e1000_ethdev.h
> > > > @@ -89,6 +89,12 @@
> > > >  	ETH_RSS_IPV6_UDP_EX)
> > > >
> > > >  /*
> > > > + * The overhead from MTU to max frame size.
> > > > + * Considering VLAN so a tag needs to be counted.
> > > > + */
> > > > +#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN +
> > > > +VLAN_TAG_SIZE)
> > >
> > > As an overhead, following drivers set:
> > > i40e: HDR + CRC + 2 * VLAN
> > > ixgbe: HDR + CRC
> > > e1000: HDR + CRC + VLAN
> > >
> > > I wonder if this difference is HW limitation, or driver limitation
> > > or just implementation inconsistency.
> >
> > I think this is implementation inconsistency
> >
> > The NIC only accept Max Frame Size.
> >
> > The problem here is seems all of three setup are not perfect.
> >
> > HDR + CRC + 2 * VLAN - it may allow non vlan or single vlan packet that exceed
> mtu.
> 
> Hmm, wonder how?

I'm talking about the case:

Assume mtu = 1500,  we will set max frame size to 1500 + 14 + 4 + 2*4 = 1526
Let's assume a non vlan packet with 1522 size, so its l2 payload will be 1504 that exceed the mtu,  but it will still be accepted, does it break the configure?

> 
> > HDR + CRC - it may reject vlan or double vlan packet that follow mtu.
> > HDR + CRC + VLAN ,  it may reject double vlan packet that follow mtu
> >
> > I agree it's better to keep consistent on all drivers, but before
> > this, we may need to decide which one we should take :)
> >
> > Regards
> > Qi
> >
> > >
> > > Better to confirm it that it is not implementation inconsistency.
> > >
> > > Wenzhuo, Konstantin, Beilei, Qi,
> > >
> > > Can you please comment?
> > >
> > > Thanks,
> > > ferruh

Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Zhang, Qi Z]

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Tuesday, March 26, 2019 10:02 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> Stokes, Ian <ian.stokes@intel.com>; dev@dpdk.org
> Cc: stephen@networkplumber.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing,
> Beilei <beilei.xing@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb
> devices
> 
> 
> Hi Qi,
> 
> > >
> > > On 3/22/2019 1:01 PM, Ian Stokes wrote:
> > > > This commit sets the min and max supported MTU values for igb
> > > > devices via the eth_igb_info_get() function. Min MTU supported is
> > > > set to ETHER_MIN_MTU and max mtu is calculated as the max packet
> > > > length supported minus the transport overhead. To aid in these
> > > > calculations a new MACRO 'E1000_ETH_OVERHEAD' has been introduced
> > > > to consolidate overhead calculation and avoid duplication.
> > > >
> > > > Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> > > > ---
> > > >  drivers/net/e1000/e1000_ethdev.h | 6 ++++++
> > > >  drivers/net/e1000/igb_ethdev.c   | 7 +++++--
> > > >  2 files changed, 11 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/net/e1000/e1000_ethdev.h
> > > > b/drivers/net/e1000/e1000_ethdev.h
> > > > index 94edff08e..3e74cd8fe 100644
> > > > --- a/drivers/net/e1000/e1000_ethdev.h
> > > > +++ b/drivers/net/e1000/e1000_ethdev.h
> > > > @@ -89,6 +89,12 @@
> > > >  	ETH_RSS_IPV6_UDP_EX)
> > > >
> > > >  /*
> > > > + * The overhead from MTU to max frame size.
> > > > + * Considering VLAN so a tag needs to be counted.
> > > > + */
> > > > +#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN +
> > > > +VLAN_TAG_SIZE)
> > >
> > > As an overhead, following drivers set:
> > > i40e: HDR + CRC + 2 * VLAN
> > > ixgbe: HDR + CRC
> > > e1000: HDR + CRC + VLAN
> > >
> > > I wonder if this difference is HW limitation, or driver limitation
> > > or just implementation inconsistency.
> >
> > I think this is implementation inconsistency
> >
> > The NIC only accept Max Frame Size.
> >
> > The problem here is seems all of three setup are not perfect.
> >
> > HDR + CRC + 2 * VLAN - it may allow non vlan or single vlan packet that exceed
> mtu.
> 
> Hmm, wonder how?

I'm talking about the case:

Assume mtu = 1500,  we will set max frame size to 1500 + 14 + 4 + 2*4 = 1526
Let's assume a non vlan packet with 1522 size, so its l2 payload will be 1504 that exceed the mtu,  but it will still be accepted, does it break the configure?

> 
> > HDR + CRC - it may reject vlan or double vlan packet that follow mtu.
> > HDR + CRC + VLAN ,  it may reject double vlan packet that follow mtu
> >
> > I agree it's better to keep consistent on all drivers, but before
> > this, we may need to decide which one we should take :)
> >
> > Regards
> > Qi
> >
> > >
> > > Better to confirm it that it is not implementation inconsistency.
> > >
> > > Wenzhuo, Konstantin, Beilei, Qi,
> > >
> > > Can you please comment?
> > >
> > > Thanks,
> > > ferruh

Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Ananyev, Konstantin]

> >
> > Hi Qi,
> >
> > > >
> > > > On 3/22/2019 1:01 PM, Ian Stokes wrote:
> > > > > This commit sets the min and max supported MTU values for igb
> > > > > devices via the eth_igb_info_get() function. Min MTU supported is
> > > > > set to ETHER_MIN_MTU and max mtu is calculated as the max packet
> > > > > length supported minus the transport overhead. To aid in these
> > > > > calculations a new MACRO 'E1000_ETH_OVERHEAD' has been introduced
> > > > > to consolidate overhead calculation and avoid duplication.
> > > > >
> > > > > Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> > > > > ---
> > > > >  drivers/net/e1000/e1000_ethdev.h | 6 ++++++
> > > > >  drivers/net/e1000/igb_ethdev.c   | 7 +++++--
> > > > >  2 files changed, 11 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/e1000/e1000_ethdev.h
> > > > > b/drivers/net/e1000/e1000_ethdev.h
> > > > > index 94edff08e..3e74cd8fe 100644
> > > > > --- a/drivers/net/e1000/e1000_ethdev.h
> > > > > +++ b/drivers/net/e1000/e1000_ethdev.h
> > > > > @@ -89,6 +89,12 @@
> > > > >  	ETH_RSS_IPV6_UDP_EX)
> > > > >
> > > > >  /*
> > > > > + * The overhead from MTU to max frame size.
> > > > > + * Considering VLAN so a tag needs to be counted.
> > > > > + */
> > > > > +#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN +
> > > > > +VLAN_TAG_SIZE)
> > > >
> > > > As an overhead, following drivers set:
> > > > i40e: HDR + CRC + 2 * VLAN
> > > > ixgbe: HDR + CRC
> > > > e1000: HDR + CRC + VLAN
> > > >
> > > > I wonder if this difference is HW limitation, or driver limitation
> > > > or just implementation inconsistency.
> > >
> > > I think this is implementation inconsistency
> > >
> > > The NIC only accept Max Frame Size.
> > >
> > > The problem here is seems all of three setup are not perfect.
> > >
> > > HDR + CRC + 2 * VLAN - it may allow non vlan or single vlan packet that exceed
> > mtu.
> >
> > Hmm, wonder how?
> 
> I'm talking about the case:
> 
> Assume mtu = 1500,  we will set max frame size to 1500 + 14 + 4 + 2*4 = 1526
> Let's assume a non vlan packet with 1522 size, so its l2 payload will be 1504 that exceed the mtu,  but it will still be accepted, does it break
> the configure?

Of course it would, but as I can read the mail, we discussing overhead
to subtract from max_rx_pkt_len to report max allowable mtu.
From that perspective bigger overhead is more conservative and makes sure
our tx packet will never be bigger than max_rx_pkt_len.
Konstantin

> 
> >
> > > HDR + CRC - it may reject vlan or double vlan packet that follow mtu.
> > > HDR + CRC + VLAN ,  it may reject double vlan packet that follow mtu
> > >
> > > I agree it's better to keep consistent on all drivers, but before
> > > this, we may need to decide which one we should take :)
> > >
> > > Regards
> > > Qi
> > >
> > > >
> > > > Better to confirm it that it is not implementation inconsistency.
> > > >
> > > > Wenzhuo, Konstantin, Beilei, Qi,
> > > >
> > > > Can you please comment?
> > > >
> > > > Thanks,
> > > > ferruh

Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Ananyev, Konstantin]

> >
> > Hi Qi,
> >
> > > >
> > > > On 3/22/2019 1:01 PM, Ian Stokes wrote:
> > > > > This commit sets the min and max supported MTU values for igb
> > > > > devices via the eth_igb_info_get() function. Min MTU supported is
> > > > > set to ETHER_MIN_MTU and max mtu is calculated as the max packet
> > > > > length supported minus the transport overhead. To aid in these
> > > > > calculations a new MACRO 'E1000_ETH_OVERHEAD' has been introduced
> > > > > to consolidate overhead calculation and avoid duplication.
> > > > >
> > > > > Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> > > > > ---
> > > > >  drivers/net/e1000/e1000_ethdev.h | 6 ++++++
> > > > >  drivers/net/e1000/igb_ethdev.c   | 7 +++++--
> > > > >  2 files changed, 11 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/e1000/e1000_ethdev.h
> > > > > b/drivers/net/e1000/e1000_ethdev.h
> > > > > index 94edff08e..3e74cd8fe 100644
> > > > > --- a/drivers/net/e1000/e1000_ethdev.h
> > > > > +++ b/drivers/net/e1000/e1000_ethdev.h
> > > > > @@ -89,6 +89,12 @@
> > > > >  	ETH_RSS_IPV6_UDP_EX)
> > > > >
> > > > >  /*
> > > > > + * The overhead from MTU to max frame size.
> > > > > + * Considering VLAN so a tag needs to be counted.
> > > > > + */
> > > > > +#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN +
> > > > > +VLAN_TAG_SIZE)
> > > >
> > > > As an overhead, following drivers set:
> > > > i40e: HDR + CRC + 2 * VLAN
> > > > ixgbe: HDR + CRC
> > > > e1000: HDR + CRC + VLAN
> > > >
> > > > I wonder if this difference is HW limitation, or driver limitation
> > > > or just implementation inconsistency.
> > >
> > > I think this is implementation inconsistency
> > >
> > > The NIC only accept Max Frame Size.
> > >
> > > The problem here is seems all of three setup are not perfect.
> > >
> > > HDR + CRC + 2 * VLAN - it may allow non vlan or single vlan packet that exceed
> > mtu.
> >
> > Hmm, wonder how?
> 
> I'm talking about the case:
> 
> Assume mtu = 1500,  we will set max frame size to 1500 + 14 + 4 + 2*4 = 1526
> Let's assume a non vlan packet with 1522 size, so its l2 payload will be 1504 that exceed the mtu,  but it will still be accepted, does it break
> the configure?

Of course it would, but as I can read the mail, we discussing overhead
to subtract from max_rx_pkt_len to report max allowable mtu.
From that perspective bigger overhead is more conservative and makes sure
our tx packet will never be bigger than max_rx_pkt_len.
Konstantin

> 
> >
> > > HDR + CRC - it may reject vlan or double vlan packet that follow mtu.
> > > HDR + CRC + VLAN ,  it may reject double vlan packet that follow mtu
> > >
> > > I agree it's better to keep consistent on all drivers, but before
> > > this, we may need to decide which one we should take :)
> > >
> > > Regards
> > > Qi
> > >
> > > >
> > > > Better to confirm it that it is not implementation inconsistency.
> > > >
> > > > Wenzhuo, Konstantin, Beilei, Qi,
> > > >
> > > > Can you please comment?
> > > >
> > > > Thanks,
> > > > ferruh

Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Zhang, Qi Z]

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Tuesday, March 26, 2019 10:18 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> Stokes, Ian <ian.stokes@intel.com>; dev@dpdk.org
> Cc: stephen@networkplumber.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing,
> Beilei <beilei.xing@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb
> devices
> 
> 
> > >
> > > Hi Qi,
> > >
> > > > >
> > > > > On 3/22/2019 1:01 PM, Ian Stokes wrote:
> > > > > > This commit sets the min and max supported MTU values for igb
> > > > > > devices via the eth_igb_info_get() function. Min MTU supported
> > > > > > is set to ETHER_MIN_MTU and max mtu is calculated as the max
> > > > > > packet length supported minus the transport overhead. To aid
> > > > > > in these calculations a new MACRO 'E1000_ETH_OVERHEAD' has
> > > > > > been introduced to consolidate overhead calculation and avoid
> duplication.
> > > > > >
> > > > > > Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> > > > > > ---
> > > > > >  drivers/net/e1000/e1000_ethdev.h | 6 ++++++
> > > > > >  drivers/net/e1000/igb_ethdev.c   | 7 +++++--
> > > > > >  2 files changed, 11 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/net/e1000/e1000_ethdev.h
> > > > > > b/drivers/net/e1000/e1000_ethdev.h
> > > > > > index 94edff08e..3e74cd8fe 100644
> > > > > > --- a/drivers/net/e1000/e1000_ethdev.h
> > > > > > +++ b/drivers/net/e1000/e1000_ethdev.h
> > > > > > @@ -89,6 +89,12 @@
> > > > > >  	ETH_RSS_IPV6_UDP_EX)
> > > > > >
> > > > > >  /*
> > > > > > + * The overhead from MTU to max frame size.
> > > > > > + * Considering VLAN so a tag needs to be counted.
> > > > > > + */
> > > > > > +#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN +
> > > > > > +VLAN_TAG_SIZE)
> > > > >
> > > > > As an overhead, following drivers set:
> > > > > i40e: HDR + CRC + 2 * VLAN
> > > > > ixgbe: HDR + CRC
> > > > > e1000: HDR + CRC + VLAN
> > > > >
> > > > > I wonder if this difference is HW limitation, or driver
> > > > > limitation or just implementation inconsistency.
> > > >
> > > > I think this is implementation inconsistency
> > > >
> > > > The NIC only accept Max Frame Size.
> > > >
> > > > The problem here is seems all of three setup are not perfect.
> > > >
> > > > HDR + CRC + 2 * VLAN - it may allow non vlan or single vlan packet
> > > > that exceed
> > > mtu.
> > >
> > > Hmm, wonder how?
> >
> > I'm talking about the case:
> >
> > Assume mtu = 1500,  we will set max frame size to 1500 + 14 + 4 + 2*4
> > = 1526 Let's assume a non vlan packet with 1522 size, so its l2
> > payload will be 1504 that exceed the mtu,  but it will still be accepted, does it
> break the configure?
> 
> Of course it would, but as I can read the mail, we discussing overhead to subtract
> from max_rx_pkt_len to report max allowable mtu.
> From that perspective bigger overhead is more conservative and makes sure our
> tx packet will never be bigger than max_rx_pkt_len.
> Konstantin

I'm OK to choose HDR + CRC + 2 * VLAN as MTU overhead to keep all driver consistent.

Qi

> 
> >
> > >
> > > > HDR + CRC - it may reject vlan or double vlan packet that follow mtu.
> > > > HDR + CRC + VLAN ,  it may reject double vlan packet that follow
> > > > mtu
> > > >
> > > > I agree it's better to keep consistent on all drivers, but before
> > > > this, we may need to decide which one we should take :)
> > > >
> > > > Regards
> > > > Qi
> > > >
> > > > >
> > > > > Better to confirm it that it is not implementation inconsistency.
> > > > >
> > > > > Wenzhuo, Konstantin, Beilei, Qi,
> > > > >
> > > > > Can you please comment?
> > > > >
> > > > > Thanks,
> > > > > ferruh

Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Zhang, Qi Z]

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Tuesday, March 26, 2019 10:18 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> Stokes, Ian <ian.stokes@intel.com>; dev@dpdk.org
> Cc: stephen@networkplumber.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing,
> Beilei <beilei.xing@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb
> devices
> 
> 
> > >
> > > Hi Qi,
> > >
> > > > >
> > > > > On 3/22/2019 1:01 PM, Ian Stokes wrote:
> > > > > > This commit sets the min and max supported MTU values for igb
> > > > > > devices via the eth_igb_info_get() function. Min MTU supported
> > > > > > is set to ETHER_MIN_MTU and max mtu is calculated as the max
> > > > > > packet length supported minus the transport overhead. To aid
> > > > > > in these calculations a new MACRO 'E1000_ETH_OVERHEAD' has
> > > > > > been introduced to consolidate overhead calculation and avoid
> duplication.
> > > > > >
> > > > > > Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> > > > > > ---
> > > > > >  drivers/net/e1000/e1000_ethdev.h | 6 ++++++
> > > > > >  drivers/net/e1000/igb_ethdev.c   | 7 +++++--
> > > > > >  2 files changed, 11 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/net/e1000/e1000_ethdev.h
> > > > > > b/drivers/net/e1000/e1000_ethdev.h
> > > > > > index 94edff08e..3e74cd8fe 100644
> > > > > > --- a/drivers/net/e1000/e1000_ethdev.h
> > > > > > +++ b/drivers/net/e1000/e1000_ethdev.h
> > > > > > @@ -89,6 +89,12 @@
> > > > > >  	ETH_RSS_IPV6_UDP_EX)
> > > > > >
> > > > > >  /*
> > > > > > + * The overhead from MTU to max frame size.
> > > > > > + * Considering VLAN so a tag needs to be counted.
> > > > > > + */
> > > > > > +#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN +
> > > > > > +VLAN_TAG_SIZE)
> > > > >
> > > > > As an overhead, following drivers set:
> > > > > i40e: HDR + CRC + 2 * VLAN
> > > > > ixgbe: HDR + CRC
> > > > > e1000: HDR + CRC + VLAN
> > > > >
> > > > > I wonder if this difference is HW limitation, or driver
> > > > > limitation or just implementation inconsistency.
> > > >
> > > > I think this is implementation inconsistency
> > > >
> > > > The NIC only accept Max Frame Size.
> > > >
> > > > The problem here is seems all of three setup are not perfect.
> > > >
> > > > HDR + CRC + 2 * VLAN - it may allow non vlan or single vlan packet
> > > > that exceed
> > > mtu.
> > >
> > > Hmm, wonder how?
> >
> > I'm talking about the case:
> >
> > Assume mtu = 1500,  we will set max frame size to 1500 + 14 + 4 + 2*4
> > = 1526 Let's assume a non vlan packet with 1522 size, so its l2
> > payload will be 1504 that exceed the mtu,  but it will still be accepted, does it
> break the configure?
> 
> Of course it would, but as I can read the mail, we discussing overhead to subtract
> from max_rx_pkt_len to report max allowable mtu.
> From that perspective bigger overhead is more conservative and makes sure our
> tx packet will never be bigger than max_rx_pkt_len.
> Konstantin

I'm OK to choose HDR + CRC + 2 * VLAN as MTU overhead to keep all driver consistent.

Qi

> 
> >
> > >
> > > > HDR + CRC - it may reject vlan or double vlan packet that follow mtu.
> > > > HDR + CRC + VLAN ,  it may reject double vlan packet that follow
> > > > mtu
> > > >
> > > > I agree it's better to keep consistent on all drivers, but before
> > > > this, we may need to decide which one we should take :)
> > > >
> > > > Regards
> > > > Qi
> > > >
> > > > >
> > > > > Better to confirm it that it is not implementation inconsistency.
> > > > >
> > > > > Wenzhuo, Konstantin, Beilei, Qi,
> > > > >
> > > > > Can you please comment?
> > > > >
> > > > > Thanks,
> > > > > ferruh

Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Zhang, Qi Z]

> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Wednesday, March 27, 2019 9:14 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Stokes, Ian <ian.stokes@intel.com>; dev@dpdk.org
> Cc: stephen@networkplumber.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing,
> Beilei <beilei.xing@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb
> devices
> 
> 
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Tuesday, March 26, 2019 10:18 PM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Yigit, Ferruh
> > <ferruh.yigit@intel.com>; Stokes, Ian <ian.stokes@intel.com>;
> > dev@dpdk.org
> > Cc: stephen@networkplumber.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> > Xing, Beilei <beilei.xing@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU
> > for igb devices
> >
> >
> > > >
> > > > Hi Qi,
> > > >
> > > > > >
> > > > > > On 3/22/2019 1:01 PM, Ian Stokes wrote:
> > > > > > > This commit sets the min and max supported MTU values for
> > > > > > > igb devices via the eth_igb_info_get() function. Min MTU
> > > > > > > supported is set to ETHER_MIN_MTU and max mtu is calculated
> > > > > > > as the max packet length supported minus the transport
> > > > > > > overhead. To aid in these calculations a new MACRO
> > > > > > > 'E1000_ETH_OVERHEAD' has been introduced to consolidate
> > > > > > > overhead calculation and avoid
> > duplication.
> > > > > > >
> > > > > > > Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> > > > > > > ---
> > > > > > >  drivers/net/e1000/e1000_ethdev.h | 6 ++++++
> > > > > > >  drivers/net/e1000/igb_ethdev.c   | 7 +++++--
> > > > > > >  2 files changed, 11 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/net/e1000/e1000_ethdev.h
> > > > > > > b/drivers/net/e1000/e1000_ethdev.h
> > > > > > > index 94edff08e..3e74cd8fe 100644
> > > > > > > --- a/drivers/net/e1000/e1000_ethdev.h
> > > > > > > +++ b/drivers/net/e1000/e1000_ethdev.h
> > > > > > > @@ -89,6 +89,12 @@
> > > > > > >  	ETH_RSS_IPV6_UDP_EX)
> > > > > > >
> > > > > > >  /*
> > > > > > > + * The overhead from MTU to max frame size.
> > > > > > > + * Considering VLAN so a tag needs to be counted.
> > > > > > > + */
> > > > > > > +#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN
> +
> > > > > > > +VLAN_TAG_SIZE)
> > > > > >
> > > > > > As an overhead, following drivers set:
> > > > > > i40e: HDR + CRC + 2 * VLAN
> > > > > > ixgbe: HDR + CRC

> > > > > > e1000: HDR + CRC + VLAN
> > > > > >
> > > > > > I wonder if this difference is HW limitation, or driver
> > > > > > limitation or just implementation inconsistency.
> > > > >
> > > > > I think this is implementation inconsistency
> > > > >
> > > > > The NIC only accept Max Frame Size.

Sorry , I need to correct above statement, by checking the ixgbe datasheet, actually max frame size is not include VLAN
Packet with VLAN can be as large as MFS +4 and packet with double vlan can be MFS + 8. It's different with i40e.

So current implementation for mtu overhead subtraction for i40e and ixgbe looks good for me.

> > > > >
> > > > > The problem here is seems all of three setup are not perfect.
> > > > >
> > > > > HDR + CRC + 2 * VLAN - it may allow non vlan or single vlan
> > > > > packet that exceed
> > > > mtu.
> > > >
> > > > Hmm, wonder how?
> > >
> > > I'm talking about the case:
> > >
> > > Assume mtu = 1500,  we will set max frame size to 1500 + 14 + 4 +
> > > 2*4 = 1526 Let's assume a non vlan packet with 1522 size, so its l2
> > > payload will be 1504 that exceed the mtu,  but it will still be
> > > accepted, does it
> > break the configure?
> >
> > Of course it would, but as I can read the mail, we discussing overhead
> > to subtract from max_rx_pkt_len to report max allowable mtu.
> > From that perspective bigger overhead is more conservative and makes
> > sure our tx packet will never be bigger than max_rx_pkt_len.
> > Konstantin
> 
> I'm OK to choose HDR + CRC + 2 * VLAN as MTU overhead to keep all driver
> consistent.

> 
> Qi
> 
> >
> > >
> > > >
> > > > > HDR + CRC - it may reject vlan or double vlan packet that follow mtu.
> > > > > HDR + CRC + VLAN ,  it may reject double vlan packet that follow
> > > > > mtu
> > > > >
> > > > > I agree it's better to keep consistent on all drivers, but
> > > > > before this, we may need to decide which one we should take :)
> > > > >
> > > > > Regards
> > > > > Qi
> > > > >
> > > > > >
> > > > > > Better to confirm it that it is not implementation inconsistency.
> > > > > >
> > > > > > Wenzhuo, Konstantin, Beilei, Qi,
> > > > > >
> > > > > > Can you please comment?
> > > > > >
> > > > > > Thanks,
> > > > > > ferruh

Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

[Zhang, Qi Z]

> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Wednesday, March 27, 2019 9:14 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Stokes, Ian <ian.stokes@intel.com>; dev@dpdk.org
> Cc: stephen@networkplumber.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing,
> Beilei <beilei.xing@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb
> devices
> 
> 
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Tuesday, March 26, 2019 10:18 PM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Yigit, Ferruh
> > <ferruh.yigit@intel.com>; Stokes, Ian <ian.stokes@intel.com>;
> > dev@dpdk.org
> > Cc: stephen@networkplumber.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> > Xing, Beilei <beilei.xing@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU
> > for igb devices
> >
> >
> > > >
> > > > Hi Qi,
> > > >
> > > > > >
> > > > > > On 3/22/2019 1:01 PM, Ian Stokes wrote:
> > > > > > > This commit sets the min and max supported MTU values for
> > > > > > > igb devices via the eth_igb_info_get() function. Min MTU
> > > > > > > supported is set to ETHER_MIN_MTU and max mtu is calculated
> > > > > > > as the max packet length supported minus the transport
> > > > > > > overhead. To aid in these calculations a new MACRO
> > > > > > > 'E1000_ETH_OVERHEAD' has been introduced to consolidate
> > > > > > > overhead calculation and avoid
> > duplication.
> > > > > > >
> > > > > > > Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> > > > > > > ---
> > > > > > >  drivers/net/e1000/e1000_ethdev.h | 6 ++++++
> > > > > > >  drivers/net/e1000/igb_ethdev.c   | 7 +++++--
> > > > > > >  2 files changed, 11 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/net/e1000/e1000_ethdev.h
> > > > > > > b/drivers/net/e1000/e1000_ethdev.h
> > > > > > > index 94edff08e..3e74cd8fe 100644
> > > > > > > --- a/drivers/net/e1000/e1000_ethdev.h
> > > > > > > +++ b/drivers/net/e1000/e1000_ethdev.h
> > > > > > > @@ -89,6 +89,12 @@
> > > > > > >  	ETH_RSS_IPV6_UDP_EX)
> > > > > > >
> > > > > > >  /*
> > > > > > > + * The overhead from MTU to max frame size.
> > > > > > > + * Considering VLAN so a tag needs to be counted.
> > > > > > > + */
> > > > > > > +#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN
> +
> > > > > > > +VLAN_TAG_SIZE)
> > > > > >
> > > > > > As an overhead, following drivers set:
> > > > > > i40e: HDR + CRC + 2 * VLAN
> > > > > > ixgbe: HDR + CRC

> > > > > > e1000: HDR + CRC + VLAN
> > > > > >
> > > > > > I wonder if this difference is HW limitation, or driver
> > > > > > limitation or just implementation inconsistency.
> > > > >
> > > > > I think this is implementation inconsistency
> > > > >
> > > > > The NIC only accept Max Frame Size.

Sorry , I need to correct above statement, by checking the ixgbe datasheet, actually max frame size is not include VLAN
Packet with VLAN can be as large as MFS +4 and packet with double vlan can be MFS + 8. It's different with i40e.

So current implementation for mtu overhead subtraction for i40e and ixgbe looks good for me.

> > > > >
> > > > > The problem here is seems all of three setup are not perfect.
> > > > >
> > > > > HDR + CRC + 2 * VLAN - it may allow non vlan or single vlan
> > > > > packet that exceed
> > > > mtu.
> > > >
> > > > Hmm, wonder how?
> > >
> > > I'm talking about the case:
> > >
> > > Assume mtu = 1500,  we will set max frame size to 1500 + 14 + 4 +
> > > 2*4 = 1526 Let's assume a non vlan packet with 1522 size, so its l2
> > > payload will be 1504 that exceed the mtu,  but it will still be
> > > accepted, does it
> > break the configure?
> >
> > Of course it would, but as I can read the mail, we discussing overhead
> > to subtract from max_rx_pkt_len to report max allowable mtu.
> > From that perspective bigger overhead is more conservative and makes
> > sure our tx packet will never be bigger than max_rx_pkt_len.
> > Konstantin
> 
> I'm OK to choose HDR + CRC + 2 * VLAN as MTU overhead to keep all driver
> consistent.

> 
> Qi
> 
> >
> > >
> > > >
> > > > > HDR + CRC - it may reject vlan or double vlan packet that follow mtu.
> > > > > HDR + CRC + VLAN ,  it may reject double vlan packet that follow
> > > > > mtu
> > > > >
> > > > > I agree it's better to keep consistent on all drivers, but
> > > > > before this, we may need to decide which one we should take :)
> > > > >
> > > > > Regards
> > > > > Qi
> > > > >
> > > > > >
> > > > > > Better to confirm it that it is not implementation inconsistency.
> > > > > >
> > > > > > Wenzhuo, Konstantin, Beilei, Qi,
> > > > > >
> > > > > > Can you please comment?
> > > > > >
> > > > > > Thanks,
> > > > > > ferruh

[dpdk-dev] [PATCH v2 7/7] app/testpmd: verify mtu with rte_eth_dev_info_get()

[Ian Stokes]

This commit uses the mtu fields populated in rte_eth_dev_info_get()
to validate the mtu value being passed in port_mtu_set().

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 app/test-pmd/config.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index b9e5dd923..6ca97d0e7 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1063,9 +1063,13 @@ void
 port_mtu_set(portid_t port_id, uint16_t mtu)
 {
 	int diag;
+	struct rte_eth_dev_info dev_info;
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN))
 		return;
+	rte_eth_dev_info_get(port_id, &dev_info);
+	if (mtu > dev_info.max_mtu || mtu < dev_info.min_mtu)
+		return;
 	diag = rte_eth_dev_set_mtu(port_id, mtu);
 	if (diag == 0)
 		return;
-- 
2.13.6

[dpdk-dev] [PATCH v2 7/7] app/testpmd: verify mtu with rte_eth_dev_info_get()

[Ian Stokes]

This commit uses the mtu fields populated in rte_eth_dev_info_get()
to validate the mtu value being passed in port_mtu_set().

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 app/test-pmd/config.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index b9e5dd923..6ca97d0e7 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1063,9 +1063,13 @@ void
 port_mtu_set(portid_t port_id, uint16_t mtu)
 {
 	int diag;
+	struct rte_eth_dev_info dev_info;
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN))
 		return;
+	rte_eth_dev_info_get(port_id, &dev_info);
+	if (mtu > dev_info.max_mtu || mtu < dev_info.min_mtu)
+		return;
 	diag = rte_eth_dev_set_mtu(port_id, mtu);
 	if (diag == 0)
 		return;
-- 
2.13.6

Re: [dpdk-dev] [PATCH v2 7/7] app/testpmd: verify mtu with rte_eth_dev_info_get()

[Ferruh Yigit]

On 3/22/2019 1:01 PM, Ian Stokes wrote:
> This commit uses the mtu fields populated in rte_eth_dev_info_get()
> to validate the mtu value being passed in port_mtu_set().
> 
> Signed-off-by: Ian Stokes <ian.stokes@intel.com>

For patch title, I think 'rte_eth_dev_info_get()' is implementation detail here,
what we are verifying mtu against min/max MTU information provided by PMDs.
What do you think something like:
app/testpmd: verify MTU with PMD provided limits

> ---
>  app/test-pmd/config.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index b9e5dd923..6ca97d0e7 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1063,9 +1063,13 @@ void
>  port_mtu_set(portid_t port_id, uint16_t mtu)
>  {
>  	int diag;
> +	struct rte_eth_dev_info dev_info;
>  
>  	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> +	rte_eth_dev_info_get(port_id, &dev_info);
> +	if (mtu > dev_info.max_mtu || mtu < dev_info.min_mtu)
> +		return;

This fails silently, I think better to put a log for failure.

>  	diag = rte_eth_dev_set_mtu(port_id, mtu);
>  	if (diag == 0)
>  		return;
> 

Re: [dpdk-dev] [PATCH v2 7/7] app/testpmd: verify mtu with rte_eth_dev_info_get()

[Ferruh Yigit]

On 3/22/2019 1:01 PM, Ian Stokes wrote:
> This commit uses the mtu fields populated in rte_eth_dev_info_get()
> to validate the mtu value being passed in port_mtu_set().
> 
> Signed-off-by: Ian Stokes <ian.stokes@intel.com>

For patch title, I think 'rte_eth_dev_info_get()' is implementation detail here,
what we are verifying mtu against min/max MTU information provided by PMDs.
What do you think something like:
app/testpmd: verify MTU with PMD provided limits

> ---
>  app/test-pmd/config.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index b9e5dd923..6ca97d0e7 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1063,9 +1063,13 @@ void
>  port_mtu_set(portid_t port_id, uint16_t mtu)
>  {
>  	int diag;
> +	struct rte_eth_dev_info dev_info;
>  
>  	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> +	rte_eth_dev_info_get(port_id, &dev_info);
> +	if (mtu > dev_info.max_mtu || mtu < dev_info.min_mtu)
> +		return;

This fails silently, I think better to put a log for failure.

>  	diag = rte_eth_dev_set_mtu(port_id, mtu);
>  	if (diag == 0)
>  		return;
> 

Re: [dpdk-dev] [PATCH v2 7/7] app/testpmd: verify mtu with rte_eth_dev_info_get()

[Ferruh Yigit]

On 3/25/2019 2:20 PM, Ferruh Yigit wrote:
> On 3/22/2019 1:01 PM, Ian Stokes wrote:
>> This commit uses the mtu fields populated in rte_eth_dev_info_get()
>> to validate the mtu value being passed in port_mtu_set().
>>
>> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> 
> For patch title, I think 'rte_eth_dev_info_get()' is implementation detail here,
> what we are verifying mtu against min/max MTU information provided by PMDs.
> What do you think something like:
> app/testpmd: verify MTU with PMD provided limits
> 
>> ---
>>  app/test-pmd/config.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index b9e5dd923..6ca97d0e7 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -1063,9 +1063,13 @@ void
>>  port_mtu_set(portid_t port_id, uint16_t mtu)
>>  {
>>  	int diag;
>> +	struct rte_eth_dev_info dev_info;
>>  
>>  	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>> +	rte_eth_dev_info_get(port_id, &dev_info);
>> +	if (mtu > dev_info.max_mtu || mtu < dev_info.min_mtu)
>> +		return;
> 
> This fails silently, I think better to put a log for failure.

Ian seems out of office, I can update this myself if set of the patchset is good.

Re: [dpdk-dev] [PATCH v2 7/7] app/testpmd: verify mtu with rte_eth_dev_info_get()

[Ferruh Yigit]

On 3/25/2019 2:20 PM, Ferruh Yigit wrote:
> On 3/22/2019 1:01 PM, Ian Stokes wrote:
>> This commit uses the mtu fields populated in rte_eth_dev_info_get()
>> to validate the mtu value being passed in port_mtu_set().
>>
>> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> 
> For patch title, I think 'rte_eth_dev_info_get()' is implementation detail here,
> what we are verifying mtu against min/max MTU information provided by PMDs.
> What do you think something like:
> app/testpmd: verify MTU with PMD provided limits
> 
>> ---
>>  app/test-pmd/config.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index b9e5dd923..6ca97d0e7 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -1063,9 +1063,13 @@ void
>>  port_mtu_set(portid_t port_id, uint16_t mtu)
>>  {
>>  	int diag;
>> +	struct rte_eth_dev_info dev_info;
>>  
>>  	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>> +	rte_eth_dev_info_get(port_id, &dev_info);
>> +	if (mtu > dev_info.max_mtu || mtu < dev_info.min_mtu)
>> +		return;
> 
> This fails silently, I think better to put a log for failure.

Ian seems out of office, I can update this myself if set of the patchset is good.

Re: [dpdk-dev] [PATCH v2 0/7] ethdev: add min/max MTU to device info

[Ferruh Yigit]

On 3/22/2019 1:01 PM, Ian Stokes wrote:
> Building upon the discussion around [1], this series introduces MTU min
> and MTU max variables. It also provides updates to PMD implementations
> for ixgbe, i40e and IGB devices so that these variables are populated
> for use when retrieving device info.
> 
> This series was tested with OVS DPDK and functions as expected for the
> drivers listed below. But a wider selection of PMD drivers would have to
> adopt this to ensure jumbo frames functionality remains for drivers not
> modified in the series.
> 
> There is also ongoing discussion in [2] regarding overhead to be
> considered with MTU and how this may change from device to device, this
> series uses existing overhead assumptions.
> 
> This series was previously posted as an RFC in [3] and a v1 in [4],
> this revision implements changes received in feedback on v1.
> 
> [1] http://mails.dpdk.org/archives/dev/2018-September/110959.html
> [2] http://mails.dpdk.org/archives/dev/2019-February/124457.html
> [3] http://mails.dpdk.org/archives/dev/2019-February/124938.html
> [4] http://mails.dpdk.org/archives/dev/2019-February/125319.html
> 
> Ian Stokes (6):
>   net/i40e: set min and max MTU for i40e devices
>   net/i40e: set min and max MTU for i40e VF devices
>   net/ixgbe: set min and max MTU for ixgbe devices
>   net/ixgbe: set min and max MTU for ixgbe VF devices
>   net/e1000: set min and max MTU for igb devices
>   app/testpmd: verify mtu with rte_eth_dev_info_get()
> 
> Stephen Hemminger (1):
>   ethdev: add min/max MTU to device info

Final notice about the patch, if there is no objection will be merged tomorrow
this for rc1.

Reminder that all net PMD maintainers should update according this new 'min_mtu'
& 'max_mtu' approach.

Re: [dpdk-dev] [PATCH v2 0/7] ethdev: add min/max MTU to device info

[Ferruh Yigit]

On 3/22/2019 1:01 PM, Ian Stokes wrote:
> Building upon the discussion around [1], this series introduces MTU min
> and MTU max variables. It also provides updates to PMD implementations
> for ixgbe, i40e and IGB devices so that these variables are populated
> for use when retrieving device info.
> 
> This series was tested with OVS DPDK and functions as expected for the
> drivers listed below. But a wider selection of PMD drivers would have to
> adopt this to ensure jumbo frames functionality remains for drivers not
> modified in the series.
> 
> There is also ongoing discussion in [2] regarding overhead to be
> considered with MTU and how this may change from device to device, this
> series uses existing overhead assumptions.
> 
> This series was previously posted as an RFC in [3] and a v1 in [4],
> this revision implements changes received in feedback on v1.
> 
> [1] http://mails.dpdk.org/archives/dev/2018-September/110959.html
> [2] http://mails.dpdk.org/archives/dev/2019-February/124457.html
> [3] http://mails.dpdk.org/archives/dev/2019-February/124938.html
> [4] http://mails.dpdk.org/archives/dev/2019-February/125319.html
> 
> Ian Stokes (6):
>   net/i40e: set min and max MTU for i40e devices
>   net/i40e: set min and max MTU for i40e VF devices
>   net/ixgbe: set min and max MTU for ixgbe devices
>   net/ixgbe: set min and max MTU for ixgbe VF devices
>   net/e1000: set min and max MTU for igb devices
>   app/testpmd: verify mtu with rte_eth_dev_info_get()
> 
> Stephen Hemminger (1):
>   ethdev: add min/max MTU to device info

Final notice about the patch, if there is no objection will be merged tomorrow
this for rc1.

Reminder that all net PMD maintainers should update according this new 'min_mtu'
& 'max_mtu' approach.

[dpdk-dev] [PATCH v3 1/7] ethdev: add min/max MTU to device info

[Ferruh Yigit]

From: Stephen Hemminger <stephen@networkplumber.org>

This addresses the usability issue raised by OVS at DPDK Userspace
summit. It adds general min/max mtu into device info. For compatiablity,
and to save space, it fits in a hole in existing structure.

The initial version sets max mtu to normal Ethernet, it is up to
PMD to set larger value if it supports Jumbo frames.

Also remove the deprecation notice introduced in 18.11 regarding this
change and bump ethdev ABI version.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 doc/guides/rel_notes/deprecation.rst   | 12 ----------
 doc/guides/rel_notes/release_19_05.rst |  9 ++++++-
 lib/librte_ethdev/Makefile             |  2 +-
 lib/librte_ethdev/meson.build          |  2 +-
 lib/librte_ethdev/rte_ethdev.c         | 15 ++++++++++++
 lib/librte_ethdev/rte_ethdev.h         | 33 +++++++++++++++++++++++++-
 6 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 73bfa9d4a..6f55c4740 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -52,18 +52,6 @@ Deprecation Notices
   Target release for removal of the legacy API will be defined once most
   PMDs have switched to rte_flow.
 
-* ethdev: Maximum and minimum MTU values vary between hardware devices. In
-  hardware agnostic DPDK applications access to such information would allow
-  a more accurate way of validating and setting supported MTU values on a per
-  device basis rather than using a defined default for all devices. To
-  resolve this, the following members will be added to ``rte_eth_dev_info``.
-  Note: these can be added to fit a hole in the existing structure for amd64
-  but not for 32-bit, as such ABI change will occur as size of the structure
-  will increase.
-
-  - Member ``uint16_t min_mtu`` the minimum MTU allowed.
-  - Member ``uint16_t max_mtu`` the maximum MTU allowed.
-
 * kni: remove KNI ethtool support. To clarify, this is not to remove the KNI,
   but only to remove ethtool support of it that is disabled by default and
   can be enabled via ``CONFIG_RTE_KNI_KMOD_ETHTOOL`` config option.
diff --git a/doc/guides/rel_notes/release_19_05.rst b/doc/guides/rel_notes/release_19_05.rst
index cddd800e4..8d2d8a7e9 100644
--- a/doc/guides/rel_notes/release_19_05.rst
+++ b/doc/guides/rel_notes/release_19_05.rst
@@ -164,6 +164,13 @@ ABI Changes
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+* ethdev: Additional fields in rte_eth_dev_info.
+
+  The ``rte_eth_dev_info`` structure has had two extra fields
+  added: ``min_mtu`` and ``max_mtu``. Each of these are of type ``uint16_t``.
+  The values of these fields can be set specifically by the PMD drivers as
+  supported values can vary from device to device.
+
 * cryptodev: in 18.08 new structure ``rte_crypto_asym_op`` was introduced and
   included into ``rte_crypto_op``. As ``rte_crypto_asym_op`` structure was
   defined as cache-line aligned that caused unintended changes in
@@ -206,7 +213,7 @@ The libraries prepended with a plus sign were incremented in this version.
      librte_distributor.so.1
    + librte_eal.so.10
      librte_efd.so.1
-     librte_ethdev.so.11
+   + librte_ethdev.so.12
      librte_eventdev.so.6
      librte_flow_classify.so.1
      librte_gro.so.1
diff --git a/lib/librte_ethdev/Makefile b/lib/librte_ethdev/Makefile
index e09c4263a..8d4a02630 100644
--- a/lib/librte_ethdev/Makefile
+++ b/lib/librte_ethdev/Makefile
@@ -16,7 +16,7 @@ LDLIBS += -lrte_mbuf -lrte_kvargs -lrte_cmdline -lrte_meter
 
 EXPORT_MAP := rte_ethdev_version.map
 
-LIBABIVER := 11
+LIBABIVER := 12
 
 SRCS-y += ethdev_private.c
 SRCS-y += rte_ethdev.c
diff --git a/lib/librte_ethdev/meson.build b/lib/librte_ethdev/meson.build
index e98942ff8..8d6165b2a 100644
--- a/lib/librte_ethdev/meson.build
+++ b/lib/librte_ethdev/meson.build
@@ -2,7 +2,7 @@
 # Copyright(c) 2017 Intel Corporation
 
 name = 'ethdev'
-version = 11
+version = 12
 allow_experimental_apis = true
 sources = files('ethdev_private.c',
 	'ethdev_profile.c',
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 12b66b68c..10bdfb37e 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -2532,6 +2532,8 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	dev_info->rx_desc_lim = lim;
 	dev_info->tx_desc_lim = lim;
 	dev_info->device = dev->device;
+	dev_info->min_mtu = ETHER_MIN_MTU;
+	dev_info->max_mtu = UINT16_MAX;
 
 	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
 	(*dev->dev_ops->dev_infos_get)(dev, dev_info);
@@ -2595,12 +2597,25 @@ int
 rte_eth_dev_set_mtu(uint16_t port_id, uint16_t mtu)
 {
 	int ret;
+	struct rte_eth_dev_info dev_info;
 	struct rte_eth_dev *dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mtu_set, -ENOTSUP);
 
+	/*
+	 * Check if the device supports dev_infos_get, if it does not
+	 * skip min_mtu/max_mtu validation here as this requires values
+	 * that are populated within the call to rte_eth_dev_info_get()
+	 * which relies on dev->dev_ops->dev_infos_get.
+	 */
+	if (*dev->dev_ops->dev_infos_get != NULL) {
+		rte_eth_dev_info_get(port_id, &dev_info);
+		if (mtu < dev_info.min_mtu || mtu > dev_info.max_mtu)
+			return -EINVAL;
+	}
+
 	ret = (*dev->dev_ops->mtu_set)(dev, mtu);
 	if (!ret)
 		dev->data->mtu = mtu;
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 2be9b4ba1..b6023c050 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1088,6 +1088,8 @@ struct rte_eth_dev_info {
 	const char *driver_name; /**< Device Driver name. */
 	unsigned int if_index; /**< Index to bound host interface, or 0 if none.
 		Use if_indextoname() to translate into an interface name. */
+	uint16_t min_mtu;	/**< Minimum MTU allowed */
+	uint16_t max_mtu;	/**< Maximum MTU allowed */
 	const uint32_t *dev_flags; /**< Device flags */
 	uint32_t min_rx_bufsize; /**< Minimum size of RX buffer. */
 	uint32_t max_rx_pktlen; /**< Maximum configurable length of RX pkt. */
@@ -2204,6 +2206,33 @@ void rte_eth_macaddr_get(uint16_t port_id, struct ether_addr *mac_addr);
 /**
  * Retrieve the contextual information of an Ethernet device.
  *
+ * As part of this function, a number of of fields in dev_info will be
+ * initialized as follows:
+ *
+ * rx_desc_lim = lim
+ * tx_desc_lim = lim
+ *
+ * Where lim is defined within the rte_eth_dev_info_get as
+ *
+ *  const struct rte_eth_desc_lim lim = {
+ *      .nb_max = UINT16_MAX,
+ *      .nb_min = 0,
+ *      .nb_align = 1,
+ *  };
+ *
+ * device = dev->device
+ * min_mtu = ETHER_MIN_MTU
+ * max_mtu = UINT16_MAX
+ *
+ * The following fields will be populated if support for dev_infos_get()
+ * exists for the device and the rte_eth_dev 'dev' has been populated
+ * successfully with a call to it:
+ *
+ * driver_name = dev->device->driver->name
+ * nb_rx_queues = dev->data->nb_rx_queues
+ * nb_tx_queues = dev->data->nb_tx_queues
+ * dev_flags = &dev->data->dev_flags
+ *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param dev_info
@@ -2300,7 +2329,9 @@ int rte_eth_dev_get_mtu(uint16_t port_id, uint16_t *mtu);
  *   - (-ENOTSUP) if operation is not supported.
  *   - (-ENODEV) if *port_id* invalid.
  *   - (-EIO) if device is removed.
- *   - (-EINVAL) if *mtu* invalid.
+ *   - (-EINVAL) if *mtu* invalid, validation of mtu can occur within
+ *     rte_eth_dev_set_mtu if dev_infos_get is supported by the device or
+ *     when the mtu is set using dev->dev_ops->mtu_set.
  *   - (-EBUSY) if operation is not allowed when the port is running
  */
 int rte_eth_dev_set_mtu(uint16_t port_id, uint16_t mtu);
-- 
2.20.1

[dpdk-dev] [PATCH v3 1/7] ethdev: add min/max MTU to device info

[Ferruh Yigit]

From: Stephen Hemminger <stephen@networkplumber.org>

This addresses the usability issue raised by OVS at DPDK Userspace
summit. It adds general min/max mtu into device info. For compatiablity,
and to save space, it fits in a hole in existing structure.

The initial version sets max mtu to normal Ethernet, it is up to
PMD to set larger value if it supports Jumbo frames.

Also remove the deprecation notice introduced in 18.11 regarding this
change and bump ethdev ABI version.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 doc/guides/rel_notes/deprecation.rst   | 12 ----------
 doc/guides/rel_notes/release_19_05.rst |  9 ++++++-
 lib/librte_ethdev/Makefile             |  2 +-
 lib/librte_ethdev/meson.build          |  2 +-
 lib/librte_ethdev/rte_ethdev.c         | 15 ++++++++++++
 lib/librte_ethdev/rte_ethdev.h         | 33 +++++++++++++++++++++++++-
 6 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 73bfa9d4a..6f55c4740 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -52,18 +52,6 @@ Deprecation Notices
   Target release for removal of the legacy API will be defined once most
   PMDs have switched to rte_flow.
 
-* ethdev: Maximum and minimum MTU values vary between hardware devices. In
-  hardware agnostic DPDK applications access to such information would allow
-  a more accurate way of validating and setting supported MTU values on a per
-  device basis rather than using a defined default for all devices. To
-  resolve this, the following members will be added to ``rte_eth_dev_info``.
-  Note: these can be added to fit a hole in the existing structure for amd64
-  but not for 32-bit, as such ABI change will occur as size of the structure
-  will increase.
-
-  - Member ``uint16_t min_mtu`` the minimum MTU allowed.
-  - Member ``uint16_t max_mtu`` the maximum MTU allowed.
-
 * kni: remove KNI ethtool support. To clarify, this is not to remove the KNI,
   but only to remove ethtool support of it that is disabled by default and
   can be enabled via ``CONFIG_RTE_KNI_KMOD_ETHTOOL`` config option.
diff --git a/doc/guides/rel_notes/release_19_05.rst b/doc/guides/rel_notes/release_19_05.rst
index cddd800e4..8d2d8a7e9 100644
--- a/doc/guides/rel_notes/release_19_05.rst
+++ b/doc/guides/rel_notes/release_19_05.rst
@@ -164,6 +164,13 @@ ABI Changes
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+* ethdev: Additional fields in rte_eth_dev_info.
+
+  The ``rte_eth_dev_info`` structure has had two extra fields
+  added: ``min_mtu`` and ``max_mtu``. Each of these are of type ``uint16_t``.
+  The values of these fields can be set specifically by the PMD drivers as
+  supported values can vary from device to device.
+
 * cryptodev: in 18.08 new structure ``rte_crypto_asym_op`` was introduced and
   included into ``rte_crypto_op``. As ``rte_crypto_asym_op`` structure was
   defined as cache-line aligned that caused unintended changes in
@@ -206,7 +213,7 @@ The libraries prepended with a plus sign were incremented in this version.
      librte_distributor.so.1
    + librte_eal.so.10
      librte_efd.so.1
-     librte_ethdev.so.11
+   + librte_ethdev.so.12
      librte_eventdev.so.6
      librte_flow_classify.so.1
      librte_gro.so.1
diff --git a/lib/librte_ethdev/Makefile b/lib/librte_ethdev/Makefile
index e09c4263a..8d4a02630 100644
--- a/lib/librte_ethdev/Makefile
+++ b/lib/librte_ethdev/Makefile
@@ -16,7 +16,7 @@ LDLIBS += -lrte_mbuf -lrte_kvargs -lrte_cmdline -lrte_meter
 
 EXPORT_MAP := rte_ethdev_version.map
 
-LIBABIVER := 11
+LIBABIVER := 12
 
 SRCS-y += ethdev_private.c
 SRCS-y += rte_ethdev.c
diff --git a/lib/librte_ethdev/meson.build b/lib/librte_ethdev/meson.build
index e98942ff8..8d6165b2a 100644
--- a/lib/librte_ethdev/meson.build
+++ b/lib/librte_ethdev/meson.build
@@ -2,7 +2,7 @@
 # Copyright(c) 2017 Intel Corporation
 
 name = 'ethdev'
-version = 11
+version = 12
 allow_experimental_apis = true
 sources = files('ethdev_private.c',
 	'ethdev_profile.c',
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 12b66b68c..10bdfb37e 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -2532,6 +2532,8 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	dev_info->rx_desc_lim = lim;
 	dev_info->tx_desc_lim = lim;
 	dev_info->device = dev->device;
+	dev_info->min_mtu = ETHER_MIN_MTU;
+	dev_info->max_mtu = UINT16_MAX;
 
 	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
 	(*dev->dev_ops->dev_infos_get)(dev, dev_info);
@@ -2595,12 +2597,25 @@ int
 rte_eth_dev_set_mtu(uint16_t port_id, uint16_t mtu)
 {
 	int ret;
+	struct rte_eth_dev_info dev_info;
 	struct rte_eth_dev *dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mtu_set, -ENOTSUP);
 
+	/*
+	 * Check if the device supports dev_infos_get, if it does not
+	 * skip min_mtu/max_mtu validation here as this requires values
+	 * that are populated within the call to rte_eth_dev_info_get()
+	 * which relies on dev->dev_ops->dev_infos_get.
+	 */
+	if (*dev->dev_ops->dev_infos_get != NULL) {
+		rte_eth_dev_info_get(port_id, &dev_info);
+		if (mtu < dev_info.min_mtu || mtu > dev_info.max_mtu)
+			return -EINVAL;
+	}
+
 	ret = (*dev->dev_ops->mtu_set)(dev, mtu);
 	if (!ret)
 		dev->data->mtu = mtu;
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 2be9b4ba1..b6023c050 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1088,6 +1088,8 @@ struct rte_eth_dev_info {
 	const char *driver_name; /**< Device Driver name. */
 	unsigned int if_index; /**< Index to bound host interface, or 0 if none.
 		Use if_indextoname() to translate into an interface name. */
+	uint16_t min_mtu;	/**< Minimum MTU allowed */
+	uint16_t max_mtu;	/**< Maximum MTU allowed */
 	const uint32_t *dev_flags; /**< Device flags */
 	uint32_t min_rx_bufsize; /**< Minimum size of RX buffer. */
 	uint32_t max_rx_pktlen; /**< Maximum configurable length of RX pkt. */
@@ -2204,6 +2206,33 @@ void rte_eth_macaddr_get(uint16_t port_id, struct ether_addr *mac_addr);
 /**
  * Retrieve the contextual information of an Ethernet device.
  *
+ * As part of this function, a number of of fields in dev_info will be
+ * initialized as follows:
+ *
+ * rx_desc_lim = lim
+ * tx_desc_lim = lim
+ *
+ * Where lim is defined within the rte_eth_dev_info_get as
+ *
+ *  const struct rte_eth_desc_lim lim = {
+ *      .nb_max = UINT16_MAX,
+ *      .nb_min = 0,
+ *      .nb_align = 1,
+ *  };
+ *
+ * device = dev->device
+ * min_mtu = ETHER_MIN_MTU
+ * max_mtu = UINT16_MAX
+ *
+ * The following fields will be populated if support for dev_infos_get()
+ * exists for the device and the rte_eth_dev 'dev' has been populated
+ * successfully with a call to it:
+ *
+ * driver_name = dev->device->driver->name
+ * nb_rx_queues = dev->data->nb_rx_queues
+ * nb_tx_queues = dev->data->nb_tx_queues
+ * dev_flags = &dev->data->dev_flags
+ *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param dev_info
@@ -2300,7 +2329,9 @@ int rte_eth_dev_get_mtu(uint16_t port_id, uint16_t *mtu);
  *   - (-ENOTSUP) if operation is not supported.
  *   - (-ENODEV) if *port_id* invalid.
  *   - (-EIO) if device is removed.
- *   - (-EINVAL) if *mtu* invalid.
+ *   - (-EINVAL) if *mtu* invalid, validation of mtu can occur within
+ *     rte_eth_dev_set_mtu if dev_infos_get is supported by the device or
+ *     when the mtu is set using dev->dev_ops->mtu_set.
  *   - (-EBUSY) if operation is not allowed when the port is running
  */
 int rte_eth_dev_set_mtu(uint16_t port_id, uint16_t mtu);
-- 
2.20.1

[dpdk-dev] [PATCH v3 2/7] net/i40e: set min and max MTU for i40e devices

[Ferruh Yigit]

From: Ian Stokes <ian.stokes@intel.com>

This commit sets the min and max supported MTU values for i40e devices
via the i40e_dev_info_get() function. Min MTU supported is set to
ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3815aef6c..891bdc061 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -3499,6 +3499,8 @@ i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_rx_pktlen = I40E_FRAME_SIZE_MAX;
 	dev_info->max_mac_addrs = vsi->max_macaddrs;
 	dev_info->max_vfs = pci_dev->max_vfs;
+	dev_info->max_mtu = dev_info->max_rx_pktlen - I40E_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
 	dev_info->rx_queue_offload_capa = 0;
 	dev_info->rx_offload_capa =
 		DEV_RX_OFFLOAD_VLAN_STRIP |
-- 
2.20.1

[dpdk-dev] [PATCH v3 2/7] net/i40e: set min and max MTU for i40e devices

[Ferruh Yigit]

From: Ian Stokes <ian.stokes@intel.com>

This commit sets the min and max supported MTU values for i40e devices
via the i40e_dev_info_get() function. Min MTU supported is set to
ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3815aef6c..891bdc061 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -3499,6 +3499,8 @@ i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_rx_pktlen = I40E_FRAME_SIZE_MAX;
 	dev_info->max_mac_addrs = vsi->max_macaddrs;
 	dev_info->max_vfs = pci_dev->max_vfs;
+	dev_info->max_mtu = dev_info->max_rx_pktlen - I40E_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
 	dev_info->rx_queue_offload_capa = 0;
 	dev_info->rx_offload_capa =
 		DEV_RX_OFFLOAD_VLAN_STRIP |
-- 
2.20.1

[dpdk-dev] [PATCH v3 3/7] net/i40e: set min and max MTU for i40e VF devices

[Ferruh Yigit]

From: Ian Stokes <ian.stokes@intel.com>

This commit sets the min and max supported MTU values for i40e VF
devices via the i40evf_dev_info_get() function. Min MTU supported
is set to ETHER_MIN_MTU and max mtu is calculated as the max packet
length supported minus the transport overhead.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/i40e/i40e_ethdev_vf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 7e4bd7314..add7b2223 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -2216,6 +2216,8 @@ i40evf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_tx_queues = I40E_MAX_QP_NUM_PER_VF;
 	dev_info->min_rx_bufsize = I40E_BUF_SIZE_MIN;
 	dev_info->max_rx_pktlen = I40E_FRAME_SIZE_MAX;
+	dev_info->max_mtu = dev_info->max_rx_pktlen - I40E_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
 	dev_info->hash_key_size = (I40E_VFQF_HKEY_MAX_INDEX + 1) * sizeof(uint32_t);
 	dev_info->reta_size = ETH_RSS_RETA_SIZE_64;
 	dev_info->flow_type_rss_offloads = vf->adapter->flow_types_mask;
-- 
2.20.1

[dpdk-dev] [PATCH v3 3/7] net/i40e: set min and max MTU for i40e VF devices

[Ferruh Yigit]

From: Ian Stokes <ian.stokes@intel.com>

This commit sets the min and max supported MTU values for i40e VF
devices via the i40evf_dev_info_get() function. Min MTU supported
is set to ETHER_MIN_MTU and max mtu is calculated as the max packet
length supported minus the transport overhead.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/i40e/i40e_ethdev_vf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 7e4bd7314..add7b2223 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -2216,6 +2216,8 @@ i40evf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_tx_queues = I40E_MAX_QP_NUM_PER_VF;
 	dev_info->min_rx_bufsize = I40E_BUF_SIZE_MIN;
 	dev_info->max_rx_pktlen = I40E_FRAME_SIZE_MAX;
+	dev_info->max_mtu = dev_info->max_rx_pktlen - I40E_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
 	dev_info->hash_key_size = (I40E_VFQF_HKEY_MAX_INDEX + 1) * sizeof(uint32_t);
 	dev_info->reta_size = ETH_RSS_RETA_SIZE_64;
 	dev_info->flow_type_rss_offloads = vf->adapter->flow_types_mask;
-- 
2.20.1

[dpdk-dev] [PATCH v3 4/7] net/ixgbe: set min and max MTU for ixgbe devices

[Ferruh Yigit]

From: Ian Stokes <ian.stokes@intel.com>

This commit sets the min and max supported MTU values for ixgbe devices
via the ixgbe_dev_info_get() function. Min MTU supported is set to
ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead. To aid in these calculations
a new MACRO 'IXGBE_ETH_OVERHEAD' has been introduced to consolidate
overhead calculation and avoid duplication.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 4 +++-
 drivers/net/ixgbe/ixgbe_ethdev.h | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 97e10217d..60da3508c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3748,6 +3748,8 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		dev_info->max_vmdq_pools = ETH_16_POOLS;
 	else
 		dev_info->max_vmdq_pools = ETH_64_POOLS;
+	dev_info->max_mtu =  dev_info->max_rx_pktlen - IXGBE_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
 	dev_info->vmdq_queue_num = dev_info->max_rx_queues;
 	dev_info->rx_queue_offload_capa = ixgbe_get_rx_queue_offloads(dev);
 	dev_info->rx_offload_capa = (ixgbe_get_rx_port_offloads(dev) |
@@ -4939,7 +4941,7 @@ ixgbe_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	uint32_t maxfrs;
 	struct ixgbe_hw *hw;
 	struct rte_eth_dev_info dev_info;
-	uint32_t frame_size = mtu + ETHER_HDR_LEN + ETHER_CRC_LEN;
+	uint32_t frame_size = mtu + IXGBE_ETH_OVERHEAD;
 	struct rte_eth_dev_data *dev_data = dev->data;
 
 	ixgbe_dev_info_get(dev, &dev_info);
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 3fec61352..d1f61e85e 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -101,6 +101,9 @@
 #define IXGBE_5TUPLE_MAX_PRI            7
 #define IXGBE_5TUPLE_MIN_PRI            1
 
+/* The overhead from MTU to max frame size. */
+#define IXGBE_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN)
+
 /* bit of VXLAN tunnel type | 7 bits of zeros  | 8 bits of zeros*/
 #define IXGBE_FDIR_VXLAN_TUNNEL_TYPE    0x8000
 /* bit of NVGRE tunnel type | 7 bits of zeros  | 8 bits of zeros*/
-- 
2.20.1

[dpdk-dev] [PATCH v3 4/7] net/ixgbe: set min and max MTU for ixgbe devices

[Ferruh Yigit]

From: Ian Stokes <ian.stokes@intel.com>

This commit sets the min and max supported MTU values for ixgbe devices
via the ixgbe_dev_info_get() function. Min MTU supported is set to
ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead. To aid in these calculations
a new MACRO 'IXGBE_ETH_OVERHEAD' has been introduced to consolidate
overhead calculation and avoid duplication.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 4 +++-
 drivers/net/ixgbe/ixgbe_ethdev.h | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 97e10217d..60da3508c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3748,6 +3748,8 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		dev_info->max_vmdq_pools = ETH_16_POOLS;
 	else
 		dev_info->max_vmdq_pools = ETH_64_POOLS;
+	dev_info->max_mtu =  dev_info->max_rx_pktlen - IXGBE_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
 	dev_info->vmdq_queue_num = dev_info->max_rx_queues;
 	dev_info->rx_queue_offload_capa = ixgbe_get_rx_queue_offloads(dev);
 	dev_info->rx_offload_capa = (ixgbe_get_rx_port_offloads(dev) |
@@ -4939,7 +4941,7 @@ ixgbe_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	uint32_t maxfrs;
 	struct ixgbe_hw *hw;
 	struct rte_eth_dev_info dev_info;
-	uint32_t frame_size = mtu + ETHER_HDR_LEN + ETHER_CRC_LEN;
+	uint32_t frame_size = mtu + IXGBE_ETH_OVERHEAD;
 	struct rte_eth_dev_data *dev_data = dev->data;
 
 	ixgbe_dev_info_get(dev, &dev_info);
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 3fec61352..d1f61e85e 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -101,6 +101,9 @@
 #define IXGBE_5TUPLE_MAX_PRI            7
 #define IXGBE_5TUPLE_MIN_PRI            1
 
+/* The overhead from MTU to max frame size. */
+#define IXGBE_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN)
+
 /* bit of VXLAN tunnel type | 7 bits of zeros  | 8 bits of zeros*/
 #define IXGBE_FDIR_VXLAN_TUNNEL_TYPE    0x8000
 /* bit of NVGRE tunnel type | 7 bits of zeros  | 8 bits of zeros*/
-- 
2.20.1

[dpdk-dev] [PATCH v3 5/7] net/ixgbe: set min and max MTU for ixgbe VF devices

[Ferruh Yigit]

From: Ian Stokes <ian.stokes@intel.com>

This commit sets the min and max supported MTU values for ixgbe VF
devices via the ixgbevf_dev_set_mtu() function. Min MTU supported is
set to ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead. As transport overhead is the
same for VF and PF ixgbe devices, reuse MACRO 'IXGBE_ETH_OVERHEAD' to
avoid duplication.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 60da3508c..feec85634 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3857,6 +3857,7 @@ ixgbevf_dev_info_get(struct rte_eth_dev *dev,
 	dev_info->max_tx_queues = (uint16_t)hw->mac.max_tx_queues;
 	dev_info->min_rx_bufsize = 1024; /* cf BSIZEPACKET in SRRCTL reg */
 	dev_info->max_rx_pktlen = 9728; /* includes CRC, cf MAXFRS reg */
+	dev_info->max_mtu = dev_info->max_rx_pktlen - IXGBE_ETH_OVERHEAD;
 	dev_info->max_mac_addrs = hw->mac.num_rar_entries;
 	dev_info->max_hash_mac_addrs = IXGBE_VMDQ_NUM_UC_MAC;
 	dev_info->max_vfs = pci_dev->max_vfs;
@@ -6344,7 +6345,7 @@ static int
 ixgbevf_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
 {
 	struct ixgbe_hw *hw;
-	uint32_t max_frame = mtu + ETHER_HDR_LEN + ETHER_CRC_LEN;
+	uint32_t max_frame = mtu + IXGBE_ETH_OVERHEAD;
 	struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode;
 
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-- 
2.20.1

[dpdk-dev] [PATCH v3 5/7] net/ixgbe: set min and max MTU for ixgbe VF devices

[Ferruh Yigit]

From: Ian Stokes <ian.stokes@intel.com>

This commit sets the min and max supported MTU values for ixgbe VF
devices via the ixgbevf_dev_set_mtu() function. Min MTU supported is
set to ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead. As transport overhead is the
same for VF and PF ixgbe devices, reuse MACRO 'IXGBE_ETH_OVERHEAD' to
avoid duplication.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 60da3508c..feec85634 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3857,6 +3857,7 @@ ixgbevf_dev_info_get(struct rte_eth_dev *dev,
 	dev_info->max_tx_queues = (uint16_t)hw->mac.max_tx_queues;
 	dev_info->min_rx_bufsize = 1024; /* cf BSIZEPACKET in SRRCTL reg */
 	dev_info->max_rx_pktlen = 9728; /* includes CRC, cf MAXFRS reg */
+	dev_info->max_mtu = dev_info->max_rx_pktlen - IXGBE_ETH_OVERHEAD;
 	dev_info->max_mac_addrs = hw->mac.num_rar_entries;
 	dev_info->max_hash_mac_addrs = IXGBE_VMDQ_NUM_UC_MAC;
 	dev_info->max_vfs = pci_dev->max_vfs;
@@ -6344,7 +6345,7 @@ static int
 ixgbevf_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
 {
 	struct ixgbe_hw *hw;
-	uint32_t max_frame = mtu + ETHER_HDR_LEN + ETHER_CRC_LEN;
+	uint32_t max_frame = mtu + IXGBE_ETH_OVERHEAD;
 	struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode;
 
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-- 
2.20.1

[dpdk-dev] [PATCH v3 6/7] net/e1000: set min and max MTU for igb devices

[Ferruh Yigit]

From: Ian Stokes <ian.stokes@intel.com>

This commit sets the min and max supported MTU values for igb devices
via the eth_igb_info_get() function. Min MTU supported is set to
ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead. To aid in these calculations
a new MACRO 'E1000_ETH_OVERHEAD' has been introduced to consolidate
overhead calculation and avoid duplication.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/e1000/e1000_ethdev.h | 6 ++++++
 drivers/net/e1000/igb_ethdev.c   | 7 +++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 94edff08e..3e74cd8fe 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -88,6 +88,12 @@
 	ETH_RSS_IPV6_TCP_EX | \
 	ETH_RSS_IPV6_UDP_EX)
 
+/*
+ * The overhead from MTU to max frame size.
+ * Considering VLAN so a tag needs to be counted.
+ */
+#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE)
+
 /*
  * Maximum number of Ring Descriptors.
  *
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 87c9aedf2..b897e8ad4 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -2283,6 +2283,10 @@ eth_igb_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->speed_capa = ETH_LINK_SPEED_10M_HD | ETH_LINK_SPEED_10M |
 			ETH_LINK_SPEED_100M_HD | ETH_LINK_SPEED_100M |
 			ETH_LINK_SPEED_1G;
+
+	dev_info->max_mtu = dev_info->max_rx_pktlen - E1000_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
+
 }
 
 static const uint32_t *
@@ -4466,8 +4470,7 @@ eth_igb_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	uint32_t rctl;
 	struct e1000_hw *hw;
 	struct rte_eth_dev_info dev_info;
-	uint32_t frame_size = mtu + (ETHER_HDR_LEN + ETHER_CRC_LEN +
-				     VLAN_TAG_SIZE);
+	uint32_t frame_size = mtu + E1000_ETH_OVERHEAD;
 
 	hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-- 
2.20.1

[dpdk-dev] [PATCH v3 6/7] net/e1000: set min and max MTU for igb devices

[Ferruh Yigit]

From: Ian Stokes <ian.stokes@intel.com>

This commit sets the min and max supported MTU values for igb devices
via the eth_igb_info_get() function. Min MTU supported is set to
ETHER_MIN_MTU and max mtu is calculated as the max packet length
supported minus the transport overhead. To aid in these calculations
a new MACRO 'E1000_ETH_OVERHEAD' has been introduced to consolidate
overhead calculation and avoid duplication.

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/e1000/e1000_ethdev.h | 6 ++++++
 drivers/net/e1000/igb_ethdev.c   | 7 +++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 94edff08e..3e74cd8fe 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -88,6 +88,12 @@
 	ETH_RSS_IPV6_TCP_EX | \
 	ETH_RSS_IPV6_UDP_EX)
 
+/*
+ * The overhead from MTU to max frame size.
+ * Considering VLAN so a tag needs to be counted.
+ */
+#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE)
+
 /*
  * Maximum number of Ring Descriptors.
  *
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 87c9aedf2..b897e8ad4 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -2283,6 +2283,10 @@ eth_igb_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->speed_capa = ETH_LINK_SPEED_10M_HD | ETH_LINK_SPEED_10M |
 			ETH_LINK_SPEED_100M_HD | ETH_LINK_SPEED_100M |
 			ETH_LINK_SPEED_1G;
+
+	dev_info->max_mtu = dev_info->max_rx_pktlen - E1000_ETH_OVERHEAD;
+	dev_info->min_mtu = ETHER_MIN_MTU;
+
 }
 
 static const uint32_t *
@@ -4466,8 +4470,7 @@ eth_igb_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	uint32_t rctl;
 	struct e1000_hw *hw;
 	struct rte_eth_dev_info dev_info;
-	uint32_t frame_size = mtu + (ETHER_HDR_LEN + ETHER_CRC_LEN +
-				     VLAN_TAG_SIZE);
+	uint32_t frame_size = mtu + E1000_ETH_OVERHEAD;
 
 	hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-- 
2.20.1

[dpdk-dev] [PATCH v3 7/7] app/testpmd: verify MTU with device provided limits

[Ferruh Yigit]

From: Ian Stokes <ian.stokes@intel.com>

This commit uses the mtu fields populated in rte_eth_dev_info_get()
to validate the mtu value being passed in port_mtu_set().

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 app/test-pmd/config.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index b9e5dd923..cadcb512f 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1063,9 +1063,16 @@ void
 port_mtu_set(portid_t port_id, uint16_t mtu)
 {
 	int diag;
+	struct rte_eth_dev_info dev_info;
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN))
 		return;
+	rte_eth_dev_info_get(port_id, &dev_info);
+	if (mtu > dev_info.max_mtu || mtu < dev_info.min_mtu) {
+		printf("Set MTU failed. MTU:%u is not in valid range, min:%u - max:%u\n",
+			mtu, dev_info.min_mtu, dev_info.max_mtu);
+		return;
+	}
 	diag = rte_eth_dev_set_mtu(port_id, mtu);
 	if (diag == 0)
 		return;
-- 
2.20.1

[dpdk-dev] [PATCH v3 7/7] app/testpmd: verify MTU with device provided limits

[Ferruh Yigit]

From: Ian Stokes <ian.stokes@intel.com>

This commit uses the mtu fields populated in rte_eth_dev_info_get()
to validate the mtu value being passed in port_mtu_set().

Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 app/test-pmd/config.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index b9e5dd923..cadcb512f 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1063,9 +1063,16 @@ void
 port_mtu_set(portid_t port_id, uint16_t mtu)
 {
 	int diag;
+	struct rte_eth_dev_info dev_info;
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN))
 		return;
+	rte_eth_dev_info_get(port_id, &dev_info);
+	if (mtu > dev_info.max_mtu || mtu < dev_info.min_mtu) {
+		printf("Set MTU failed. MTU:%u is not in valid range, min:%u - max:%u\n",
+			mtu, dev_info.min_mtu, dev_info.max_mtu);
+		return;
+	}
 	diag = rte_eth_dev_set_mtu(port_id, mtu);
 	if (diag == 0)
 		return;
-- 
2.20.1

Re: [dpdk-dev] [PATCH v3 1/7] ethdev: add min/max MTU to device info

[Ferruh Yigit]

On 3/29/2019 5:52 PM, Ferruh Yigit wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> 
> This addresses the usability issue raised by OVS at DPDK Userspace
> summit. It adds general min/max mtu into device info. For compatiablity,
> and to save space, it fits in a hole in existing structure.
> 
> The initial version sets max mtu to normal Ethernet, it is up to
> PMD to set larger value if it supports Jumbo frames.
> 
> Also remove the deprecation notice introduced in 18.11 regarding this
> change and bump ethdev ABI version.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

(series re-sent only for change in 7/7 to add a log for failure, rest is same,
kept all authors/sign-off as it is)

For series,
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH v3 1/7] ethdev: add min/max MTU to device info

[Ferruh Yigit]

On 3/29/2019 5:52 PM, Ferruh Yigit wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> 
> This addresses the usability issue raised by OVS at DPDK Userspace
> summit. It adds general min/max mtu into device info. For compatiablity,
> and to save space, it fits in a hole in existing structure.
> 
> The initial version sets max mtu to normal Ethernet, it is up to
> PMD to set larger value if it supports Jumbo frames.
> 
> Also remove the deprecation notice introduced in 18.11 regarding this
> change and bump ethdev ABI version.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

(series re-sent only for change in 7/7 to add a log for failure, rest is same,
kept all authors/sign-off as it is)

For series,
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH v3 1/7] ethdev: add min/max MTU to device info

[Ferruh Yigit]

On 3/29/2019 5:56 PM, Ferruh Yigit wrote:
> On 3/29/2019 5:52 PM, Ferruh Yigit wrote:
>> From: Stephen Hemminger <stephen@networkplumber.org>
>>
>> This addresses the usability issue raised by OVS at DPDK Userspace
>> summit. It adds general min/max mtu into device info. For compatiablity,
>> and to save space, it fits in a hole in existing structure.
>>
>> The initial version sets max mtu to normal Ethernet, it is up to
>> PMD to set larger value if it supports Jumbo frames.
>>
>> Also remove the deprecation notice introduced in 18.11 regarding this
>> change and bump ethdev ABI version.
>>
>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> (series re-sent only for change in 7/7 to add a log for failure, rest is same,
> kept all authors/sign-off as it is)
> 
> For series,
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
Series applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH v3 1/7] ethdev: add min/max MTU to device info

[Ferruh Yigit]

On 3/29/2019 5:56 PM, Ferruh Yigit wrote:
> On 3/29/2019 5:52 PM, Ferruh Yigit wrote:
>> From: Stephen Hemminger <stephen@networkplumber.org>
>>
>> This addresses the usability issue raised by OVS at DPDK Userspace
>> summit. It adds general min/max mtu into device info. For compatiablity,
>> and to save space, it fits in a hole in existing structure.
>>
>> The initial version sets max mtu to normal Ethernet, it is up to
>> PMD to set larger value if it supports Jumbo frames.
>>
>> Also remove the deprecation notice introduced in 18.11 regarding this
>> change and bump ethdev ABI version.
>>
>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> (series re-sent only for change in 7/7 to add a log for failure, rest is same,
> kept all authors/sign-off as it is)
> 
> For series,
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
Series applied to dpdk-next-net/master, thanks.