[v3] net/vmxnet3: Added mtu_set() function to allow setting MTU.
Checks
Commit Message
From: Charles Myers <charles.myers@spirent.com>
When the mtu_set() function is not implemented, rte_eth_dev_set_mtu()
fails with -ENOTSUP and mtu is not stored in the mtu field in the
rte_eth_dev_data. This causes the mtu in Vmxnet3_MiscConf which is
shared with hypervisor to always be set to 1500.
This may cause issues receiving jumbo frames on Enhanced Data Path
N-VDS.
Signed-off-by: Charles Myers <charles.myers@spirent.com>
---
Removed redundant MTU check in vmxnet3_dev_mtu_set() which should already be done
in rte_eth_dev_set_mtu().
drivers/net/vmxnet3/vmxnet3_ethdev.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
Comments
On Wed, 21 Aug 2019 02:16:58 +0000
"Myers, Charles" <Charles.Myers@spirent.com> wrote:
>
> static int
> +vmxnet3_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> +{
> + if (dev->data->dev_started) {
> + PMD_DRV_LOG(ERR, "Port %d must be stopped to configure MTU",
> + dev->data->port_id);
> + return -EBUSY;
> + }
> +
> + return 0;
> +}
> +
Don't you need to reset the rx ring to change mtu on the fly?
At a minimum you need to communicate this value to host through the
shared driver page.
MTU change is currently only allowed when device is stopped.
Current code in master branch already sets MTU in shared data correctly when starting the device:
static int
vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
{
struct rte_eth_conf port_conf = dev->data->dev_conf;
struct vmxnet3_hw *hw = dev->data->dev_private;
uint32_t mtu = dev->data->mtu;
...
devRead->misc.mtu = rte_le_to_cpu_32(mtu);
...
}
However current code in master branch does not implement a mtu_set() function so MTU in the dev->data is never changed from default of 1500:
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;
return eth_err(port_id, ret);
}
The vmxnet3 driver just needs to implement mtu_set() so that rte_eth_dev_set_mtu() allows changing the MTU and puts the value in dev->data->mtu .
-----Original Message-----
From: Stephen Hemminger <stephen@networkplumber.org>
Sent: Tuesday, August 20, 2019 6:43 PM
To: Myers, Charles <Charles.Myers@spirent.com>
Cc: Yong Wang <yongwang@vmware.com>; dev@dpdk.org
Subject: Re: [PATCH v3] net/vmxnet3: Added mtu_set() function to allow setting MTU.
On Wed, 21 Aug 2019 02:16:58 +0000
"Myers, Charles" <Charles.Myers@spirent.com> wrote:
>
> static int
> +vmxnet3_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) {
> + if (dev->data->dev_started) {
> + PMD_DRV_LOG(ERR, "Port %d must be stopped to configure MTU",
> + dev->data->port_id);
> + return -EBUSY;
> + }
> +
> + return 0;
> +}
> +
Don't you need to reset the rx ring to change mtu on the fly?
At a minimum you need to communicate this value to host through the shared driver page.
On 8/21/2019 3:16 AM, Myers, Charles wrote:
> From: Charles Myers <charles.myers@spirent.com>
>
> When the mtu_set() function is not implemented, rte_eth_dev_set_mtu()
> fails with -ENOTSUP and mtu is not stored in the mtu field in the
> rte_eth_dev_data. This causes the mtu in Vmxnet3_MiscConf which is
> shared with hypervisor to always be set to 1500.
>
> This may cause issues receiving jumbo frames on Enhanced Data Path
> N-VDS.
>
> Signed-off-by: Charles Myers <charles.myers@spirent.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Applied to dpdk-next-net/master, thanks.
> @@ -1205,6 +1209,18 @@ vmxnet3_dev_supported_ptypes_get(struct rte_eth_dev *dev)
> }
>
> static int
> +vmxnet3_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> +{
'mtu' is unused and giving build error:
error: unused parameter ‘mtu’ [-Werror=unused-parameter]
Fixed while merging:
+vmxnet3_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu __rte_unused)
On 9/13/2019 7:47 PM, Ferruh Yigit wrote:
> On 8/21/2019 3:16 AM, Myers, Charles wrote:
>> From: Charles Myers <charles.myers@spirent.com>
>>
>> When the mtu_set() function is not implemented, rte_eth_dev_set_mtu()
>> fails with -ENOTSUP and mtu is not stored in the mtu field in the
>> rte_eth_dev_data. This causes the mtu in Vmxnet3_MiscConf which is
>> shared with hypervisor to always be set to 1500.
>>
>> This may cause issues receiving jumbo frames on Enhanced Data Path
>> N-VDS.
>>
>> Signed-off-by: Charles Myers <charles.myers@spirent.com>
>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
>
> Applied to dpdk-next-net/master, thanks.
It seems there is waiting change request from the maintainer that I missed,
dropping the patch from the tree and updating the patchwork state.
Charles, can you please sync with Yong if the change request is not clear?
>
>
>> @@ -1205,6 +1209,18 @@ vmxnet3_dev_supported_ptypes_get(struct rte_eth_dev *dev)
>> }
>>
>> static int
>> +vmxnet3_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>> +{
>
> 'mtu' is unused and giving build error:
> error: unused parameter ‘mtu’ [-Werror=unused-parameter]
>
> Fixed while merging:
> +vmxnet3_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu __rte_unused)
>
>
@@ -88,6 +88,7 @@ static void vmxnet3_dev_info_get(struct rte_eth_dev *dev,
struct rte_eth_dev_info *dev_info);
static const uint32_t *
vmxnet3_dev_supported_ptypes_get(struct rte_eth_dev *dev);
+static int vmxnet3_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev,
uint16_t vid, int on);
static int vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);
@@ -125,6 +126,7 @@ static const struct eth_dev_ops vmxnet3_eth_dev_ops = {
.mac_addr_set = vmxnet3_mac_addr_set,
.dev_infos_get = vmxnet3_dev_info_get,
.dev_supported_ptypes_get = vmxnet3_dev_supported_ptypes_get,
+ .mtu_set = vmxnet3_dev_mtu_set,
.vlan_filter_set = vmxnet3_dev_vlan_filter_set,
.vlan_offload_set = vmxnet3_dev_vlan_offload_set,
.rx_queue_setup = vmxnet3_dev_rx_queue_setup,
@@ -1161,6 +1163,8 @@ vmxnet3_dev_info_get(struct rte_eth_dev *dev __rte_unused,
dev_info->max_tx_queues = VMXNET3_MAX_TX_QUEUES;
dev_info->min_rx_bufsize = 1518 + RTE_PKTMBUF_HEADROOM;
dev_info->max_rx_pktlen = 16384; /* includes CRC, cf MAXFRS register */
+ dev_info->min_mtu = VMXNET3_MIN_MTU;
+ dev_info->max_mtu = VMXNET3_MAX_MTU;
dev_info->speed_capa = ETH_LINK_SPEED_10G;
dev_info->max_mac_addrs = VMXNET3_MAX_MAC_ADDRS;
@@ -1205,6 +1209,18 @@ vmxnet3_dev_supported_ptypes_get(struct rte_eth_dev *dev)
}
static int
+vmxnet3_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+ if (dev->data->dev_started) {
+ PMD_DRV_LOG(ERR, "Port %d must be stopped to configure MTU",
+ dev->data->port_id);
+ return -EBUSY;
+ }
+
+ return 0;
+}
+
+static int
vmxnet3_mac_addr_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr)
{
struct vmxnet3_hw *hw = dev->data->dev_private;