* [dpdk-dev] [PATCH v1 1/6] ethdev: add min/max MTU to device info
2019-02-27 21:45 [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info Ian Stokes
@ 2019-02-27 21:45 ` Ian Stokes
2019-03-19 16:15 ` Ferruh Yigit
2019-02-27 21:45 ` [dpdk-dev] [PATCH v1 2/6] net/i40e: set min and max MTU for i40e devices Ian Stokes
` (5 subsequent siblings)
6 siblings, 1 reply; 27+ messages in thread
From: Ian Stokes @ 2019-02-27 21:45 UTC (permalink / raw)
To: dev; +Cc: stephen, 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>
---
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 | 7 +++++++
lib/librte_ethdev/rte_ethdev.h | 2 ++
6 files changed, 18 insertions(+), 15 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 c0390ca16..a5790352c 100644
--- a/doc/guides/rel_notes/release_19_05.rst
+++ b/doc/guides/rel_notes/release_19_05.rst
@@ -116,6 +116,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
-----------------------
@@ -151,7 +157,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..bdb5f2b0d 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,17 @@ 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);
+ 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..9fe51b2bd 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. */
--
2.13.6
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [dpdk-dev] [PATCH v1 1/6] ethdev: add min/max MTU to device info
2019-02-27 21:45 ` [dpdk-dev] [PATCH v1 1/6] " Ian Stokes
@ 2019-03-19 16:15 ` Ferruh Yigit
2019-03-19 16:15 ` Ferruh Yigit
2019-03-21 12:50 ` Ian Stokes
0 siblings, 2 replies; 27+ messages in thread
From: Ferruh Yigit @ 2019-03-19 16:15 UTC (permalink / raw)
To: Ian Stokes, dev; +Cc: stephen
On 2/27/2019 9:45 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>
> @@ -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;
Not only mtu but do you think should we document in 'rte_eth_dev_info_get()'
doxygen documentation, the default values that API sets?
>
> RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
> (*dev->dev_ops->dev_infos_get)(dev, dev_info);
> @@ -2587,12 +2589,17 @@ 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);
>
> + rte_eth_dev_info_get(port_id, &dev_info);
If we rely on 'rte_eth_dev_info_get()', we should add a check if "dev_infos_get"
is supported before calling it, like [1]. Unfortunately 'rte_eth_dev_info_get()'
return type is 'void', so we can't know if the struct has valid values or not
otherwise.
Or perhaps if port doesn't support "dev_infos_get", we can skip the mtu check
instead of returning error.
[1]
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP);
> + if (mtu < dev_info.min_mtu || mtu > dev_info.max_mtu)
> + return -EINVAL;
> +
Should we document this behavior change in the function comment in header file?
Update when -EINVAL returned, etc..
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [dpdk-dev] [PATCH v1 1/6] ethdev: add min/max MTU to device info
2019-03-19 16:15 ` Ferruh Yigit
@ 2019-03-19 16:15 ` Ferruh Yigit
2019-03-21 12:50 ` Ian Stokes
1 sibling, 0 replies; 27+ messages in thread
From: Ferruh Yigit @ 2019-03-19 16:15 UTC (permalink / raw)
To: Ian Stokes, dev; +Cc: stephen
On 2/27/2019 9:45 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>
> @@ -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;
Not only mtu but do you think should we document in 'rte_eth_dev_info_get()'
doxygen documentation, the default values that API sets?
>
> RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
> (*dev->dev_ops->dev_infos_get)(dev, dev_info);
> @@ -2587,12 +2589,17 @@ 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);
>
> + rte_eth_dev_info_get(port_id, &dev_info);
If we rely on 'rte_eth_dev_info_get()', we should add a check if "dev_infos_get"
is supported before calling it, like [1]. Unfortunately 'rte_eth_dev_info_get()'
return type is 'void', so we can't know if the struct has valid values or not
otherwise.
Or perhaps if port doesn't support "dev_infos_get", we can skip the mtu check
instead of returning error.
[1]
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP);
> + if (mtu < dev_info.min_mtu || mtu > dev_info.max_mtu)
> + return -EINVAL;
> +
Should we document this behavior change in the function comment in header file?
Update when -EINVAL returned, etc..
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [dpdk-dev] [PATCH v1 1/6] ethdev: add min/max MTU to device info
2019-03-19 16:15 ` Ferruh Yigit
2019-03-19 16:15 ` Ferruh Yigit
@ 2019-03-21 12:50 ` Ian Stokes
2019-03-21 12:50 ` Ian Stokes
2019-03-21 14:06 ` Ferruh Yigit
1 sibling, 2 replies; 27+ messages in thread
From: Ian Stokes @ 2019-03-21 12:50 UTC (permalink / raw)
To: Ferruh Yigit, dev; +Cc: stephen
On 3/19/2019 4:15 PM, Ferruh Yigit wrote:
> On 2/27/2019 9:45 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>
>
>> @@ -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;
>
> Not only mtu but do you think should we document in 'rte_eth_dev_info_get()'
> doxygen documentation, the default values that API sets?
>
Sure, that would be useful, I can include it in the v2 of this patch.
What would you document the values under? They are not @params and not
@return, is there a particular label/format that should be used for
values set within a function?
>>
>> RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
>> (*dev->dev_ops->dev_infos_get)(dev, dev_info);
>> @@ -2587,12 +2589,17 @@ 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);
>>
>> + rte_eth_dev_info_get(port_id, &dev_info);
>
> If we rely on 'rte_eth_dev_info_get()', we should add a check if "dev_infos_get"
> is supported before calling it, like [1]. Unfortunately 'rte_eth_dev_info_get()'
> return type is 'void', so we can't know if the struct has valid values or not
> otherwise.
> Or perhaps if port doesn't support "dev_infos_get", we can skip the mtu check
> instead of returning error.
>
Hmmm,good point, I assumed rte_eth_dev_info_get() would be implemented
for nearly all devices but again, only assumption, could be wrong.
I'd prefer to err on the side of caution, so for the moment we can skip
the check if dev infos is not available as you suggest. That way it
should still work non supported devices.
> [1]
> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP);
>
>> + if (mtu < dev_info.min_mtu || mtu > dev_info.max_mtu)
>> + return -EINVAL;
>> +
>
> Should we document this behavior change in the function comment in header file?
> Update when -EINVAL returned, etc..
Sure I can take care of that in the v2.
Ian
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [dpdk-dev] [PATCH v1 1/6] ethdev: add min/max MTU to device info
2019-03-21 12:50 ` Ian Stokes
@ 2019-03-21 12:50 ` Ian Stokes
2019-03-21 14:06 ` Ferruh Yigit
1 sibling, 0 replies; 27+ messages in thread
From: Ian Stokes @ 2019-03-21 12:50 UTC (permalink / raw)
To: Ferruh Yigit, dev; +Cc: stephen
On 3/19/2019 4:15 PM, Ferruh Yigit wrote:
> On 2/27/2019 9:45 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>
>
>> @@ -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;
>
> Not only mtu but do you think should we document in 'rte_eth_dev_info_get()'
> doxygen documentation, the default values that API sets?
>
Sure, that would be useful, I can include it in the v2 of this patch.
What would you document the values under? They are not @params and not
@return, is there a particular label/format that should be used for
values set within a function?
>>
>> RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
>> (*dev->dev_ops->dev_infos_get)(dev, dev_info);
>> @@ -2587,12 +2589,17 @@ 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);
>>
>> + rte_eth_dev_info_get(port_id, &dev_info);
>
> If we rely on 'rte_eth_dev_info_get()', we should add a check if "dev_infos_get"
> is supported before calling it, like [1]. Unfortunately 'rte_eth_dev_info_get()'
> return type is 'void', so we can't know if the struct has valid values or not
> otherwise.
> Or perhaps if port doesn't support "dev_infos_get", we can skip the mtu check
> instead of returning error.
>
Hmmm,good point, I assumed rte_eth_dev_info_get() would be implemented
for nearly all devices but again, only assumption, could be wrong.
I'd prefer to err on the side of caution, so for the moment we can skip
the check if dev infos is not available as you suggest. That way it
should still work non supported devices.
> [1]
> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP);
>
>> + if (mtu < dev_info.min_mtu || mtu > dev_info.max_mtu)
>> + return -EINVAL;
>> +
>
> Should we document this behavior change in the function comment in header file?
> Update when -EINVAL returned, etc..
Sure I can take care of that in the v2.
Ian
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [dpdk-dev] [PATCH v1 1/6] ethdev: add min/max MTU to device info
2019-03-21 12:50 ` Ian Stokes
2019-03-21 12:50 ` Ian Stokes
@ 2019-03-21 14:06 ` Ferruh Yigit
2019-03-21 14:06 ` Ferruh Yigit
2019-03-21 14:59 ` Ian Stokes
1 sibling, 2 replies; 27+ messages in thread
From: Ferruh Yigit @ 2019-03-21 14:06 UTC (permalink / raw)
To: Ian Stokes, dev; +Cc: stephen
On 3/21/2019 12:50 PM, Ian Stokes wrote:
> On 3/19/2019 4:15 PM, Ferruh Yigit wrote:
>> On 2/27/2019 9:45 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>
>>
>>> @@ -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;
>>
>> Not only mtu but do you think should we document in 'rte_eth_dev_info_get()'
>> doxygen documentation, the default values that API sets?
>>
>
> Sure, that would be useful, I can include it in the v2 of this patch.
> What would you document the values under? They are not @params and not
> @return, is there a particular label/format that should be used for
> values set within a function?
In the description paragraph of the function perhaps? I am not aware of a label
for this.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [dpdk-dev] [PATCH v1 1/6] ethdev: add min/max MTU to device info
2019-03-21 14:06 ` Ferruh Yigit
@ 2019-03-21 14:06 ` Ferruh Yigit
2019-03-21 14:59 ` Ian Stokes
1 sibling, 0 replies; 27+ messages in thread
From: Ferruh Yigit @ 2019-03-21 14:06 UTC (permalink / raw)
To: Ian Stokes, dev; +Cc: stephen
On 3/21/2019 12:50 PM, Ian Stokes wrote:
> On 3/19/2019 4:15 PM, Ferruh Yigit wrote:
>> On 2/27/2019 9:45 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>
>>
>>> @@ -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;
>>
>> Not only mtu but do you think should we document in 'rte_eth_dev_info_get()'
>> doxygen documentation, the default values that API sets?
>>
>
> Sure, that would be useful, I can include it in the v2 of this patch.
> What would you document the values under? They are not @params and not
> @return, is there a particular label/format that should be used for
> values set within a function?
In the description paragraph of the function perhaps? I am not aware of a label
for this.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [dpdk-dev] [PATCH v1 1/6] ethdev: add min/max MTU to device info
2019-03-21 14:06 ` Ferruh Yigit
2019-03-21 14:06 ` Ferruh Yigit
@ 2019-03-21 14:59 ` Ian Stokes
2019-03-21 14:59 ` Ian Stokes
1 sibling, 1 reply; 27+ messages in thread
From: Ian Stokes @ 2019-03-21 14:59 UTC (permalink / raw)
To: Ferruh Yigit, dev; +Cc: stephen
On 3/21/2019 2:06 PM, Ferruh Yigit wrote:
> On 3/21/2019 12:50 PM, Ian Stokes wrote:
>> On 3/19/2019 4:15 PM, Ferruh Yigit wrote:
>>> On 2/27/2019 9:45 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>
>>>
>>>> @@ -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;
>>>
>>> Not only mtu but do you think should we document in 'rte_eth_dev_info_get()'
>>> doxygen documentation, the default values that API sets?
>>>
>>
>> Sure, that would be useful, I can include it in the v2 of this patch.
>> What would you document the values under? They are not @params and not
>> @return, is there a particular label/format that should be used for
>> values set within a function?
>
> In the description paragraph of the function perhaps? I am not aware of a label
> for this.
>
Perfect, thanks for the clarification.
Ian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [dpdk-dev] [PATCH v1 1/6] ethdev: add min/max MTU to device info
2019-03-21 14:59 ` Ian Stokes
@ 2019-03-21 14:59 ` Ian Stokes
0 siblings, 0 replies; 27+ messages in thread
From: Ian Stokes @ 2019-03-21 14:59 UTC (permalink / raw)
To: Ferruh Yigit, dev; +Cc: stephen
On 3/21/2019 2:06 PM, Ferruh Yigit wrote:
> On 3/21/2019 12:50 PM, Ian Stokes wrote:
>> On 3/19/2019 4:15 PM, Ferruh Yigit wrote:
>>> On 2/27/2019 9:45 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>
>>>
>>>> @@ -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;
>>>
>>> Not only mtu but do you think should we document in 'rte_eth_dev_info_get()'
>>> doxygen documentation, the default values that API sets?
>>>
>>
>> Sure, that would be useful, I can include it in the v2 of this patch.
>> What would you document the values under? They are not @params and not
>> @return, is there a particular label/format that should be used for
>> values set within a function?
>
> In the description paragraph of the function perhaps? I am not aware of a label
> for this.
>
Perfect, thanks for the clarification.
Ian
^ permalink raw reply [flat|nested] 27+ messages in thread
* [dpdk-dev] [PATCH v1 2/6] net/i40e: set min and max MTU for i40e devices
2019-02-27 21:45 [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info Ian Stokes
2019-02-27 21:45 ` [dpdk-dev] [PATCH v1 1/6] " Ian Stokes
@ 2019-02-27 21:45 ` Ian Stokes
2019-03-19 16:18 ` Ferruh Yigit
2019-02-27 21:45 ` [dpdk-dev] [PATCH v1 3/6] net/i40e: set min and max MTU for i40e VF devices Ian Stokes
` (4 subsequent siblings)
6 siblings, 1 reply; 27+ messages in thread
From: Ian Stokes @ 2019-02-27 21:45 UTC (permalink / raw)
To: dev; +Cc: stephen, 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
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [dpdk-dev] [PATCH v1 2/6] net/i40e: set min and max MTU for i40e devices
2019-02-27 21:45 ` [dpdk-dev] [PATCH v1 2/6] net/i40e: set min and max MTU for i40e devices Ian Stokes
@ 2019-03-19 16:18 ` Ferruh Yigit
2019-03-19 16:18 ` Ferruh Yigit
2019-03-21 12:57 ` Ian Stokes
0 siblings, 2 replies; 27+ messages in thread
From: Ferruh Yigit @ 2019-03-19 16:18 UTC (permalink / raw)
To: Ian Stokes, dev; +Cc: stephen, Qi Zhang, Helin Zhang, Beilei Xing
On 2/27/2019 9:45 PM, Ian Stokes wrote:
> 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;
'I40E_ETH_OVERHEAD' [1] is the max overhead, when VLAN and QINQ is not
configured, we are wasting 8 bytes, should we try to be more fine grained when
setting the max_mtu? Does it worth the complexity?
[1]
(ETHER_HDR_LEN + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE * 2)
> + dev_info->min_mtu = ETHER_MIN_MTU;
> dev_info->rx_queue_offload_capa = 0;
> dev_info->rx_offload_capa =
> DEV_RX_OFFLOAD_VLAN_STRIP |
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [dpdk-dev] [PATCH v1 2/6] net/i40e: set min and max MTU for i40e devices
2019-03-19 16:18 ` Ferruh Yigit
@ 2019-03-19 16:18 ` Ferruh Yigit
2019-03-21 12:57 ` Ian Stokes
1 sibling, 0 replies; 27+ messages in thread
From: Ferruh Yigit @ 2019-03-19 16:18 UTC (permalink / raw)
To: Ian Stokes, dev; +Cc: stephen, Qi Zhang, Helin Zhang, Beilei Xing
On 2/27/2019 9:45 PM, Ian Stokes wrote:
> 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;
'I40E_ETH_OVERHEAD' [1] is the max overhead, when VLAN and QINQ is not
configured, we are wasting 8 bytes, should we try to be more fine grained when
setting the max_mtu? Does it worth the complexity?
[1]
(ETHER_HDR_LEN + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE * 2)
> + dev_info->min_mtu = ETHER_MIN_MTU;
> dev_info->rx_queue_offload_capa = 0;
> dev_info->rx_offload_capa =
> DEV_RX_OFFLOAD_VLAN_STRIP |
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [dpdk-dev] [PATCH v1 2/6] net/i40e: set min and max MTU for i40e devices
2019-03-19 16:18 ` Ferruh Yigit
2019-03-19 16:18 ` Ferruh Yigit
@ 2019-03-21 12:57 ` Ian Stokes
2019-03-21 12:57 ` Ian Stokes
1 sibling, 1 reply; 27+ messages in thread
From: Ian Stokes @ 2019-03-21 12:57 UTC (permalink / raw)
To: Ferruh Yigit, dev; +Cc: stephen, Qi Zhang, Helin Zhang, Beilei Xing
On 3/19/2019 4:18 PM, Ferruh Yigit wrote:
> On 2/27/2019 9:45 PM, Ian Stokes wrote:
>> 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;
>
> 'I40E_ETH_OVERHEAD' [1] is the max overhead, when VLAN and QINQ is not
> configured, we are wasting 8 bytes, should we try to be more fine grained when
> setting the max_mtu? Does it worth the complexity?
>
I'm not against this, but for this patchset I was keeping the values to
what have existed already.
There was discussion WRT being more dynamic and whether that was the
responsibility of the application or DPDK.
http://mails.dpdk.org/archives/dev/2019-February/124457.html
I'm open to this changing in the future, but for the moment was happy to
see it stay as it is until a resolution is agreed upon.
Ian
>
> [1]
> (ETHER_HDR_LEN + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE * 2)
>
>> + dev_info->min_mtu = ETHER_MIN_MTU;
>> dev_info->rx_queue_offload_capa = 0;
>> dev_info->rx_offload_capa =
>> DEV_RX_OFFLOAD_VLAN_STRIP |
>>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [dpdk-dev] [PATCH v1 2/6] net/i40e: set min and max MTU for i40e devices
2019-03-21 12:57 ` Ian Stokes
@ 2019-03-21 12:57 ` Ian Stokes
0 siblings, 0 replies; 27+ messages in thread
From: Ian Stokes @ 2019-03-21 12:57 UTC (permalink / raw)
To: Ferruh Yigit, dev; +Cc: stephen, Qi Zhang, Helin Zhang, Beilei Xing
On 3/19/2019 4:18 PM, Ferruh Yigit wrote:
> On 2/27/2019 9:45 PM, Ian Stokes wrote:
>> 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;
>
> 'I40E_ETH_OVERHEAD' [1] is the max overhead, when VLAN and QINQ is not
> configured, we are wasting 8 bytes, should we try to be more fine grained when
> setting the max_mtu? Does it worth the complexity?
>
I'm not against this, but for this patchset I was keeping the values to
what have existed already.
There was discussion WRT being more dynamic and whether that was the
responsibility of the application or DPDK.
http://mails.dpdk.org/archives/dev/2019-February/124457.html
I'm open to this changing in the future, but for the moment was happy to
see it stay as it is until a resolution is agreed upon.
Ian
>
> [1]
> (ETHER_HDR_LEN + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE * 2)
>
>> + dev_info->min_mtu = ETHER_MIN_MTU;
>> dev_info->rx_queue_offload_capa = 0;
>> dev_info->rx_offload_capa =
>> DEV_RX_OFFLOAD_VLAN_STRIP |
>>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [dpdk-dev] [PATCH v1 3/6] net/i40e: set min and max MTU for i40e VF devices
2019-02-27 21:45 [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info Ian Stokes
2019-02-27 21:45 ` [dpdk-dev] [PATCH v1 1/6] " Ian Stokes
2019-02-27 21:45 ` [dpdk-dev] [PATCH v1 2/6] net/i40e: set min and max MTU for i40e devices Ian Stokes
@ 2019-02-27 21:45 ` Ian Stokes
2019-02-27 21:45 ` [dpdk-dev] [PATCH v1 4/6] net/ixgbe: set min and max MTU for ixgbe devices Ian Stokes
` (3 subsequent siblings)
6 siblings, 0 replies; 27+ messages in thread
From: Ian Stokes @ 2019-02-27 21:45 UTC (permalink / raw)
To: dev; +Cc: stephen, 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
^ permalink raw reply [flat|nested] 27+ messages in thread* [dpdk-dev] [PATCH v1 4/6] net/ixgbe: set min and max MTU for ixgbe devices
2019-02-27 21:45 [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info Ian Stokes
` (2 preceding siblings ...)
2019-02-27 21:45 ` [dpdk-dev] [PATCH v1 3/6] net/i40e: set min and max MTU for i40e VF devices Ian Stokes
@ 2019-02-27 21:45 ` Ian Stokes
2019-02-27 21:45 ` [dpdk-dev] [PATCH v1 5/6] net/ixgbe: set min and max MTU for ixgbe VF devices Ian Stokes
` (2 subsequent siblings)
6 siblings, 0 replies; 27+ messages in thread
From: Ian Stokes @ 2019-02-27 21:45 UTC (permalink / raw)
To: dev; +Cc: stephen, 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 749311048..c4f6ff5bd 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3734,6 +3734,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) |
@@ -4925,7 +4927,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 565c69c9e..2217be30c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -100,6 +100,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
^ permalink raw reply [flat|nested] 27+ messages in thread* [dpdk-dev] [PATCH v1 5/6] net/ixgbe: set min and max MTU for ixgbe VF devices
2019-02-27 21:45 [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info Ian Stokes
` (3 preceding siblings ...)
2019-02-27 21:45 ` [dpdk-dev] [PATCH v1 4/6] net/ixgbe: set min and max MTU for ixgbe devices Ian Stokes
@ 2019-02-27 21:45 ` Ian Stokes
2019-02-27 21:45 ` [dpdk-dev] [PATCH v1 6/6] net/e1000: set min and max MTU for igb devices Ian Stokes
2019-03-19 16:30 ` [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info Ferruh Yigit
6 siblings, 0 replies; 27+ messages in thread
From: Ian Stokes @ 2019-02-27 21:45 UTC (permalink / raw)
To: dev; +Cc: stephen, 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 c4f6ff5bd..1a92a71cd 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3843,6 +3843,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;
@@ -6330,7 +6331,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
^ permalink raw reply [flat|nested] 27+ messages in thread* [dpdk-dev] [PATCH v1 6/6] net/e1000: set min and max MTU for igb devices
2019-02-27 21:45 [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info Ian Stokes
` (4 preceding siblings ...)
2019-02-27 21:45 ` [dpdk-dev] [PATCH v1 5/6] net/ixgbe: set min and max MTU for ixgbe VF devices Ian Stokes
@ 2019-02-27 21:45 ` Ian Stokes
2019-03-19 16:30 ` [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info Ferruh Yigit
6 siblings, 0 replies; 27+ messages in thread
From: Ian Stokes @ 2019-02-27 21:45 UTC (permalink / raw)
To: dev; +Cc: stephen, 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
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info
2019-02-27 21:45 [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info Ian Stokes
` (5 preceding siblings ...)
2019-02-27 21:45 ` [dpdk-dev] [PATCH v1 6/6] net/e1000: set min and max MTU for igb devices Ian Stokes
@ 2019-03-19 16:30 ` Ferruh Yigit
2019-03-19 16:30 ` Ferruh Yigit
2019-03-21 13:03 ` Ian Stokes
6 siblings, 2 replies; 27+ messages in thread
From: Ferruh Yigit @ 2019-03-19 16:30 UTC (permalink / raw)
To: Ian Stokes, dev; +Cc: stephen, Thomas Monjalon, Andrew Rybchenko
On 2/27/2019 9:45 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], this revision
> removes RFC status and implements changes received in feedback.
>
> [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
>
> Ian Stokes (5):
> 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
>
> Stephen Hemminger (1):
> ethdev: add min/max MTU to device info
Hi Ian, Stephen,
API and driver updates are included in the patchset, but I believe it would be
good to have some application code that uses it as well, I assume testpmd
already has some code to set MTU, can you please update it too accordingly?
Also, what do you think starting a unit test (which has a long term target to
verify all ethdev APIs) that tests 'rte_eth_dev_set_mtu()' API with various values?
In long term all vendors can run this unit test against their HW and verify
ehtdev API implementation of their...
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info
2019-03-19 16:30 ` [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info Ferruh Yigit
@ 2019-03-19 16:30 ` Ferruh Yigit
2019-03-21 13:03 ` Ian Stokes
1 sibling, 0 replies; 27+ messages in thread
From: Ferruh Yigit @ 2019-03-19 16:30 UTC (permalink / raw)
To: Ian Stokes, dev; +Cc: stephen, Thomas Monjalon, Andrew Rybchenko
On 2/27/2019 9:45 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], this revision
> removes RFC status and implements changes received in feedback.
>
> [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
>
> Ian Stokes (5):
> 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
>
> Stephen Hemminger (1):
> ethdev: add min/max MTU to device info
Hi Ian, Stephen,
API and driver updates are included in the patchset, but I believe it would be
good to have some application code that uses it as well, I assume testpmd
already has some code to set MTU, can you please update it too accordingly?
Also, what do you think starting a unit test (which has a long term target to
verify all ethdev APIs) that tests 'rte_eth_dev_set_mtu()' API with various values?
In long term all vendors can run this unit test against their HW and verify
ehtdev API implementation of their...
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info
2019-03-19 16:30 ` [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info Ferruh Yigit
2019-03-19 16:30 ` Ferruh Yigit
@ 2019-03-21 13:03 ` Ian Stokes
2019-03-21 13:03 ` Ian Stokes
2019-03-22 13:05 ` Ian Stokes
1 sibling, 2 replies; 27+ messages in thread
From: Ian Stokes @ 2019-03-21 13:03 UTC (permalink / raw)
To: Ferruh Yigit, dev; +Cc: stephen, Thomas Monjalon, Andrew Rybchenko
On 3/19/2019 4:30 PM, Ferruh Yigit wrote:
> On 2/27/2019 9:45 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], this revision
>> removes RFC status and implements changes received in feedback.
>>
>> [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
>>
>> Ian Stokes (5):
>> 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
>>
>> Stephen Hemminger (1):
>> ethdev: add min/max MTU to device info
>
> Hi Ian, Stephen,
>
> API and driver updates are included in the patchset, but I believe it would be
> good to have some application code that uses it as well, I assume testpmd
> already has some code to set MTU, can you please update it too accordingly?
>
Thanks Ferruh, sure I had looked at this but held off in the v1 as I
wasn't sure what best practice was, i.e. introduce the change to sample
app now or wait unitl all PMDs were on board. If it's preferred to
introduce usage in a sample app then I can do this in the v2.
> Also, what do you think starting a unit test (which has a long term target to
> verify all ethdev APIs) that tests 'rte_eth_dev_set_mtu()' API with various values?
>
Sounds useful, I can take a look for the v2, first steps might be basic
but can look into it.
Ian
> In long term all vendors can run this unit test against their HW and verify
> ehtdev API implementation of their...
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info
2019-03-21 13:03 ` Ian Stokes
@ 2019-03-21 13:03 ` Ian Stokes
2019-03-22 13:05 ` Ian Stokes
1 sibling, 0 replies; 27+ messages in thread
From: Ian Stokes @ 2019-03-21 13:03 UTC (permalink / raw)
To: Ferruh Yigit, dev; +Cc: stephen, Thomas Monjalon, Andrew Rybchenko
On 3/19/2019 4:30 PM, Ferruh Yigit wrote:
> On 2/27/2019 9:45 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], this revision
>> removes RFC status and implements changes received in feedback.
>>
>> [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
>>
>> Ian Stokes (5):
>> 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
>>
>> Stephen Hemminger (1):
>> ethdev: add min/max MTU to device info
>
> Hi Ian, Stephen,
>
> API and driver updates are included in the patchset, but I believe it would be
> good to have some application code that uses it as well, I assume testpmd
> already has some code to set MTU, can you please update it too accordingly?
>
Thanks Ferruh, sure I had looked at this but held off in the v1 as I
wasn't sure what best practice was, i.e. introduce the change to sample
app now or wait unitl all PMDs were on board. If it's preferred to
introduce usage in a sample app then I can do this in the v2.
> Also, what do you think starting a unit test (which has a long term target to
> verify all ethdev APIs) that tests 'rte_eth_dev_set_mtu()' API with various values?
>
Sounds useful, I can take a look for the v2, first steps might be basic
but can look into it.
Ian
> In long term all vendors can run this unit test against their HW and verify
> ehtdev API implementation of their...
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info
2019-03-21 13:03 ` Ian Stokes
2019-03-21 13:03 ` Ian Stokes
@ 2019-03-22 13:05 ` Ian Stokes
2019-03-22 13:05 ` Ian Stokes
2019-03-22 14:08 ` Ferruh Yigit
1 sibling, 2 replies; 27+ messages in thread
From: Ian Stokes @ 2019-03-22 13:05 UTC (permalink / raw)
To: Ferruh Yigit, dev; +Cc: stephen, Thomas Monjalon, Andrew Rybchenko
On 3/21/2019 1:03 PM, Ian Stokes wrote:
> On 3/19/2019 4:30 PM, Ferruh Yigit wrote:
>> On 2/27/2019 9:45 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], this revision
>>> removes RFC status and implements changes received in feedback.
>>>
>>> [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
>>>
>>> Ian Stokes (5):
>>> 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
>>>
>>> Stephen Hemminger (1):
>>> ethdev: add min/max MTU to device info
>>
>> Hi Ian, Stephen,
>>
>> API and driver updates are included in the patchset, but I believe it
>> would be
>> good to have some application code that uses it as well, I assume testpmd
>> already has some code to set MTU, can you please update it too
>> accordingly?
>>
>
> Thanks Ferruh, sure I had looked at this but held off in the v1 as I
> wasn't sure what best practice was, i.e. introduce the change to sample
> app now or wait unitl all PMDs were on board. If it's preferred to
> introduce usage in a sample app then I can do this in the v2.
>
>> Also, what do you think starting a unit test (which has a long term
>> target to
>> verify all ethdev APIs) that tests 'rte_eth_dev_set_mtu()' API with
>> various values?
>>
>
> Sounds useful, I can take a look for the v2, first steps might be basic
> but can look into it.
>
> Ian
>> In long term all vendors can run this unit test against their HW and
>> verify
>> ehtdev API implementation of their...
>>
>
Hi Ferruh,
I've posted a v2 of the patchset based on the feedback.
http://mails.dpdk.org/archives/dev/2019-March/127344.html
Unfortunately I did not have time to look at implementing the unit test
aspect. I don't think I'll have the bandwidth before the rc1 window next
week to implement this aspect but would be happy to look at it possibly
in the next 19.08 release if this is acceptable, is the unit test a
blocker for the rest of this work?
Thanks
Ian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info
2019-03-22 13:05 ` Ian Stokes
@ 2019-03-22 13:05 ` Ian Stokes
2019-03-22 14:08 ` Ferruh Yigit
1 sibling, 0 replies; 27+ messages in thread
From: Ian Stokes @ 2019-03-22 13:05 UTC (permalink / raw)
To: Ferruh Yigit, dev; +Cc: stephen, Thomas Monjalon, Andrew Rybchenko
On 3/21/2019 1:03 PM, Ian Stokes wrote:
> On 3/19/2019 4:30 PM, Ferruh Yigit wrote:
>> On 2/27/2019 9:45 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], this revision
>>> removes RFC status and implements changes received in feedback.
>>>
>>> [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
>>>
>>> Ian Stokes (5):
>>> 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
>>>
>>> Stephen Hemminger (1):
>>> ethdev: add min/max MTU to device info
>>
>> Hi Ian, Stephen,
>>
>> API and driver updates are included in the patchset, but I believe it
>> would be
>> good to have some application code that uses it as well, I assume testpmd
>> already has some code to set MTU, can you please update it too
>> accordingly?
>>
>
> Thanks Ferruh, sure I had looked at this but held off in the v1 as I
> wasn't sure what best practice was, i.e. introduce the change to sample
> app now or wait unitl all PMDs were on board. If it's preferred to
> introduce usage in a sample app then I can do this in the v2.
>
>> Also, what do you think starting a unit test (which has a long term
>> target to
>> verify all ethdev APIs) that tests 'rte_eth_dev_set_mtu()' API with
>> various values?
>>
>
> Sounds useful, I can take a look for the v2, first steps might be basic
> but can look into it.
>
> Ian
>> In long term all vendors can run this unit test against their HW and
>> verify
>> ehtdev API implementation of their...
>>
>
Hi Ferruh,
I've posted a v2 of the patchset based on the feedback.
http://mails.dpdk.org/archives/dev/2019-March/127344.html
Unfortunately I did not have time to look at implementing the unit test
aspect. I don't think I'll have the bandwidth before the rc1 window next
week to implement this aspect but would be happy to look at it possibly
in the next 19.08 release if this is acceptable, is the unit test a
blocker for the rest of this work?
Thanks
Ian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info
2019-03-22 13:05 ` Ian Stokes
2019-03-22 13:05 ` Ian Stokes
@ 2019-03-22 14:08 ` Ferruh Yigit
2019-03-22 14:08 ` Ferruh Yigit
1 sibling, 1 reply; 27+ messages in thread
From: Ferruh Yigit @ 2019-03-22 14:08 UTC (permalink / raw)
To: Ian Stokes, dev; +Cc: stephen, Thomas Monjalon, Andrew Rybchenko
On 3/22/2019 1:05 PM, Ian Stokes wrote:
> On 3/21/2019 1:03 PM, Ian Stokes wrote:
>> On 3/19/2019 4:30 PM, Ferruh Yigit wrote:
>>> On 2/27/2019 9:45 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], this revision
>>>> removes RFC status and implements changes received in feedback.
>>>>
>>>> [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
>>>>
>>>> Ian Stokes (5):
>>>> 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
>>>>
>>>> Stephen Hemminger (1):
>>>> ethdev: add min/max MTU to device info
>>>
>>> Hi Ian, Stephen,
>>>
>>> API and driver updates are included in the patchset, but I believe it
>>> would be
>>> good to have some application code that uses it as well, I assume testpmd
>>> already has some code to set MTU, can you please update it too
>>> accordingly?
>>>
>>
>> Thanks Ferruh, sure I had looked at this but held off in the v1 as I
>> wasn't sure what best practice was, i.e. introduce the change to sample
>> app now or wait unitl all PMDs were on board. If it's preferred to
>> introduce usage in a sample app then I can do this in the v2.
>>
>>> Also, what do you think starting a unit test (which has a long term
>>> target to
>>> verify all ethdev APIs) that tests 'rte_eth_dev_set_mtu()' API with
>>> various values?
>>>
>>
>> Sounds useful, I can take a look for the v2, first steps might be basic
>> but can look into it.
>>
>> Ian
>>> In long term all vendors can run this unit test against their HW and
>>> verify
>>> ehtdev API implementation of their...
>>>
>>
>
> Hi Ferruh,
>
> I've posted a v2 of the patchset based on the feedback.
>
> http://mails.dpdk.org/archives/dev/2019-March/127344.html
>
> Unfortunately I did not have time to look at implementing the unit test
> aspect. I don't think I'll have the bandwidth before the rc1 window next
> week to implement this aspect but would be happy to look at it possibly
> in the next 19.08 release if this is acceptable, is the unit test a
> blocker for the rest of this work?
Thanks for checking it, I believe it is not a blocker but I thought it may be a
good start for verifying ethdev APIs, we can pursue this goal later.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [dpdk-dev] [PATCH v1 0/6] ethdev: add min/max MTU to device info
2019-03-22 14:08 ` Ferruh Yigit
@ 2019-03-22 14:08 ` Ferruh Yigit
0 siblings, 0 replies; 27+ messages in thread
From: Ferruh Yigit @ 2019-03-22 14:08 UTC (permalink / raw)
To: Ian Stokes, dev; +Cc: stephen, Thomas Monjalon, Andrew Rybchenko
On 3/22/2019 1:05 PM, Ian Stokes wrote:
> On 3/21/2019 1:03 PM, Ian Stokes wrote:
>> On 3/19/2019 4:30 PM, Ferruh Yigit wrote:
>>> On 2/27/2019 9:45 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], this revision
>>>> removes RFC status and implements changes received in feedback.
>>>>
>>>> [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
>>>>
>>>> Ian Stokes (5):
>>>> 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
>>>>
>>>> Stephen Hemminger (1):
>>>> ethdev: add min/max MTU to device info
>>>
>>> Hi Ian, Stephen,
>>>
>>> API and driver updates are included in the patchset, but I believe it
>>> would be
>>> good to have some application code that uses it as well, I assume testpmd
>>> already has some code to set MTU, can you please update it too
>>> accordingly?
>>>
>>
>> Thanks Ferruh, sure I had looked at this but held off in the v1 as I
>> wasn't sure what best practice was, i.e. introduce the change to sample
>> app now or wait unitl all PMDs were on board. If it's preferred to
>> introduce usage in a sample app then I can do this in the v2.
>>
>>> Also, what do you think starting a unit test (which has a long term
>>> target to
>>> verify all ethdev APIs) that tests 'rte_eth_dev_set_mtu()' API with
>>> various values?
>>>
>>
>> Sounds useful, I can take a look for the v2, first steps might be basic
>> but can look into it.
>>
>> Ian
>>> In long term all vendors can run this unit test against their HW and
>>> verify
>>> ehtdev API implementation of their...
>>>
>>
>
> Hi Ferruh,
>
> I've posted a v2 of the patchset based on the feedback.
>
> http://mails.dpdk.org/archives/dev/2019-March/127344.html
>
> Unfortunately I did not have time to look at implementing the unit test
> aspect. I don't think I'll have the bandwidth before the rc1 window next
> week to implement this aspect but would be happy to look at it possibly
> in the next 19.08 release if this is acceptable, is the unit test a
> blocker for the rest of this work?
Thanks for checking it, I believe it is not a blocker but I thought it may be a
good start for verifying ethdev APIs, we can pursue this goal later.
^ permalink raw reply [flat|nested] 27+ messages in thread