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> --- drivers/net/vmxnet3/vmxnet3_ethdev.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 57feb37..c32c92d 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -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,24 @@ 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 (mtu < VMXNET3_MIN_MTU || mtu > VMXNET3_MAX_MTU) { + PMD_DRV_LOG(ERR, "MTU should be between %d and %d", + VMXNET3_MIN_MTU, VMXNET3_MAX_MTU); + return -EINVAL; + } + + 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; -- 2.7.4
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> --- SMTP server overwrote the From address display name in previous e-mail. Hopefully this resubmission fixes it. drivers/net/vmxnet3/vmxnet3_ethdev.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 57feb37..c32c92d 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -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,24 @@ 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 (mtu < VMXNET3_MIN_MTU || mtu > VMXNET3_MAX_MTU) { + PMD_DRV_LOG(ERR, "MTU should be between %d and %d", + VMXNET3_MIN_MTU, VMXNET3_MAX_MTU); + return -EINVAL; + } + + 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; -- 2.7.4
On Tue, 20 Aug 2019 04:06:53 +0000
"Myers, Charles" <Charles.Myers@spirent.com> wrote:
> static int
> +vmxnet3_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> +{
> + if (mtu < VMXNET3_MIN_MTU || mtu > VMXNET3_MAX_MTU) {
> + PMD_DRV_LOG(ERR, "MTU should be between %d and %d",
> + VMXNET3_MIN_MTU, VMXNET3_MAX_MTU);
> + return -EINVAL;
> + }
> +
This is not the best way to handle checking validity of MTU value.
The device should report min/max MTU and then rte_eth_dev_set_mtu
will do enforcement there.
This allows application to know what MTU to use, and eliminates
guess/check behavior.
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(+) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 57feb37..c12d70b 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -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; -- 2.7.4
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.
-----Original Message----- From: "Myers, Charles" <Charles.Myers@spirent.com> Date: Monday, August 19, 2019 at 9:07 PM To: Yong Wang <yongwang@vmware.com> Cc: "dev@dpdk.org" <dev@dpdk.org>, "Myers, Charles" <Charles.Myers@spirent.com> Subject: [PATCH v2] net/vmxnet3: Added mtu_set() function to allow setting MTU. 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> --- SMTP server overwrote the From address display name in previous e-mail. Hopefully this resubmission fixes it. drivers/net/vmxnet3/vmxnet3_ethdev.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 57feb37..c32c92d 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -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,24 @@ 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 (mtu < VMXNET3_MIN_MTU || mtu > VMXNET3_MAX_MTU) { + PMD_DRV_LOG(ERR, "MTU should be between %d and %d", + VMXNET3_MIN_MTU, VMXNET3_MAX_MTU); + return -EINVAL; + } + It looks like the above check is redundant as rte_eth_dev_set_mtu() already checks the same based on dev_info->min_mtu and max_mtu. But seems many other drivers are doing the same check so probably a separate patch to clean this up. + if (dev->data->dev_started) { + PMD_DRV_LOG(ERR, "Port %d must be stopped to configure MTU", + dev->data->port_id); + return -EBUSY; + } + Can you add the following for the vmxnet3 backend to see the change: dev->data->mtu = mtu; /* this is needed because vmxnet3_dev_start use this to update the backend but rte_eth_dev_set_mtu() set it after the driver callback */ /* changing mtu for vmxnet3 pmd does not require a restart * We stop and restart the device here * just to pass the mtu info to the backend. */ vmxnet3_dev_stop(dev); vmxnet3_dev_start(dev); + 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; -- 2.7.4
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) > >
(Picked up from @Charles Myers patch https://patchwork.dpdk.org/patch/57771/) 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: Eduard Serra <eserra@vmware.com> --- drivers/net/vmxnet3/vmxnet3_ethdev.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 705e976..40e81a4 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -87,6 +87,7 @@ static int 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); @@ -124,6 +125,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, @@ -1166,6 +1168,8 @@ vmxnet3_dev_info_get(struct rte_eth_dev *dev, 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; @@ -1212,6 +1216,18 @@ vmxnet3_dev_supported_ptypes_get(struct rte_eth_dev *dev) } static int +vmxnet3_dev_mtu_set(struct rte_eth_dev *dev, __rte_unused 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; -- 2.7.4
(Picked up from @Charles Myers patch https://patchwork.dpdk.org/patch/57771/) 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. --- drivers/net/vmxnet3/vmxnet3_ethdev.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 705e976..c6e11ad 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -87,6 +87,7 @@ static int 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); @@ -124,6 +125,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, @@ -1166,6 +1168,8 @@ vmxnet3_dev_info_get(struct rte_eth_dev *dev, 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; @@ -1212,6 +1216,18 @@ vmxnet3_dev_supported_ptypes_get(struct rte_eth_dev *dev) } static int +vmxnet3_dev_mtu_set(struct rte_eth_dev *dev, __rte_unused 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; -- 2.7.4
(Picked up from @Charles Myers patch https://patchwork.dpdk.org/patch/57771/) 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: Eduard Serra <eserra@vmware.com> Acked-by: Yong Wang <yongwang@vmware.com> --- drivers/net/vmxnet3/vmxnet3_ethdev.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 705e976..c6e11ad 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -87,6 +87,7 @@ static int 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); @@ -124,6 +125,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, @@ -1166,6 +1168,8 @@ vmxnet3_dev_info_get(struct rte_eth_dev *dev, 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; @@ -1212,6 +1216,18 @@ vmxnet3_dev_supported_ptypes_get(struct rte_eth_dev *dev) } static int +vmxnet3_dev_mtu_set(struct rte_eth_dev *dev, __rte_unused 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; -- 2.7.4
-----Original Message----- From: Eduard Serra Miralles <eserra@vmware.com> Date: Wednesday, March 25, 2020 at 12:19 PM To: "dev@dpdk.org" <dev@dpdk.org> Cc: Yong Wang <yongwang@vmware.com> Subject: [PATCH v6] net/vmxnet3: Added mtu_set() function to allow setting MTU. (Picked up from @Charles Myers patch https://patchwork.dpdk.org/patch/57771/) 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: Eduard Serra <eserra@vmware.com> Acked-by: Yong Wang <yongwang@vmware.com> --- Acked-by: Yong Wang <yongwang@vmware.com> drivers/net/vmxnet3/vmxnet3_ethdev.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 705e976..c6e11ad 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -87,6 +87,7 @@ static int 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); @@ -124,6 +125,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, @@ -1166,6 +1168,8 @@ vmxnet3_dev_info_get(struct rte_eth_dev *dev, 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; @@ -1212,6 +1216,18 @@ vmxnet3_dev_supported_ptypes_get(struct rte_eth_dev *dev) } static int +vmxnet3_dev_mtu_set(struct rte_eth_dev *dev, __rte_unused 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; -- 2.7.4
On 3/31/2020 6:31 PM, Yong Wang wrote: > > -----Original Message----- > From: Eduard Serra Miralles <eserra@vmware.com> > Date: Wednesday, March 25, 2020 at 12:19 PM > To: "dev@dpdk.org" <dev@dpdk.org> > Cc: Yong Wang <yongwang@vmware.com> > Subject: [PATCH v6] net/vmxnet3: Added mtu_set() function to allow setting MTU. > > (Picked up from @Charles Myers patch > https://patchwork.dpdk.org/patch/57771/) Thanks for resurrecting the patch. > > 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: Eduard Serra <eserra@vmware.com> > Acked-by: Yong Wang <yongwang@vmware.com> > --- > > Acked-by: Yong Wang <yongwang@vmware.com> > Applied to dpdk-next-net/master, thanks.