Re: [dpdk-dev] [RFC] ethdev: add min/max MTU to device info

[Andrew Rybchenko <arybchenko@solarflare.com>]

On 09/05/2018 07:41 PM, Stephen Hemminger wrote:
> 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.

It is true for amd64, but it looks like it is false on 32-bit. So, ABI 
breakage.

> The initial version sets max mtu to normal Ethernet, it is up to
> PMD to set larger value if it supports Jumbo frames.
>
> Fixing the drivers to use this is trivial and can be done by 18.11.
> Already have some of the patches done.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>   lib/librte_ethdev/rte_ethdev.c | 7 +++++++
>   lib/librte_ethdev/rte_ethdev.h | 2 ++
>   2 files changed, 9 insertions(+)
>
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 4c320250589a..df0c7536a7c4 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -2408,6 +2408,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 = ETHER_MTU;
>   
>   	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
>   	(*dev->dev_ops->dev_infos_get)(dev, dev_info);
> @@ -2471,12 +2473,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;
> +

The check breaks set MTU to value larger than ETHER_MTU for not
updated drivers. So, IMHO, it should be pushed only with appropriate
updates in all drivers which support bigger MTU.

>   	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 7070e9ab408f..5171a9083288 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1015,6 +1015,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. */