>Hi All, > Is there any further comments or modifications required for this patch, > or what next >steps do you guys suggest here ?
Hi Souvik, Some minor comments inline. Thanks, Mark > >-- >Regards, >Souvik > >-----Original Message----- >From: Dey, Souvik >Sent: Saturday, October 1, 2016 10:09 AM >To: mark.b.kavanagh at intel.com; yuanhan.liu at linux.intel.com; stephen at >networkplumber.org; >dev at dpdk.org >Subject: RE: [PATCH v7] net/virtio: add set_mtu in virtio > >Hi Liu/Stephen/Mark, > > I have submitted Version 7 of this patch. Do let me know if this looks > proper. > >-- >Regards, >Souvik > >-----Original Message----- >From: Dey, Souvik >Sent: Thursday, September 29, 2016 4:32 PM >To: mark.b.kavanagh at intel.com; yuanhan.liu at linux.intel.com; stephen at >networkplumber.org; >dev at dpdk.org >Cc: Dey, Souvik <sodey at sonusnet.com> >Subject: [PATCH v7] net/virtio: add set_mtu in virtio > > >Virtio interfaces do not currently allow the user to specify a particular >Maximum Transmission Unit (MTU).Consequently, the MTU of Virtio interfaces >is typically set to the Ethernet default value of 1500. >This is problematic in the case of cloud deployments, in which a specific >(and potentially non-standard) MTU needs to be set by a DHCP server, which >needs to be honored by all interfaces across the traffic path.To achieve >this Virtio interfaces should support setting of MTU. >In case when GRE/VXLAN tunneling is used for internal communication, there >will be an overhead added by the infrastructure in the packet over and >above the ETHER MTU of 1518. So to take care of this overhead in these >cases the DHCP server corrects the L3 MTU to 1454. But since virtio >interfaces was not having the MTU set functionality that MTU sent by the >DHCP server was ignored and the instance will still send packets with 1500 >MTU which after encapsulation will become more than 1518 and eventually >gets dropped in the infrastructure. >By adding an additional 'set_mtu' function to the Virtio driver, we can >honor the MTU sent by the DHCP server. The dhcp server/controller can >then leverage this 'set_mtu' functionality to resolve the above >mentioned issue of packets getting dropped due to incorrect size. > > >Signed-off-by: Souvik Dey <sodey at sonusnet.com> > >--- >Changes in v7: >- Replaced the CRC_LEN with the merge rx buf header length. >- Changed the frame_len max validation to VIRTIO_MAX_RX_PKTLEN. >Changes in v6: >- Description of change corrected >- Corrected the identations >- Corrected the subject line too >- The From line was also not correct >- Re-submitting as the below patch was not proper >Changes in v5: >- Fix log message for out-of-bounds MTU parameter in virtio_mtu_set >- Calculate frame size, based on 'mtu' parameter >- Corrected the upper bound and lower bound checks in virtio_mtu_set >Changes in v4: Incorporated review comments. >Changes in v3: Corrected few style errors as reported by sys-stv. >Changes in v2: Incorporated review comments. > > drivers/net/virtio/virtio_ethdev.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > >diff --git a/drivers/net/virtio/virtio_ethdev.c >b/drivers/net/virtio/virtio_ethdev.c >index 423c597..1dbfea6 100644 >--- a/drivers/net/virtio/virtio_ethdev.c >+++ b/drivers/net/virtio/virtio_ethdev.c >@@ -653,12 +653,20 @@ virtio_dev_allmulticast_disable(struct rte_eth_dev *dev) > PMD_INIT_LOG(ERR, "Failed to disable allmulticast"); > } > >+#define VLAN_TAG_LEN 4 /* 802.3ac tag (not DMA'd) */ There should be a blank line between the #define and the function prototype beneath. >+static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) >+{ >+ struct virtio_hw *hw = dev->data->dev_private; >+ uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_LEN + >+ hw->vtnet_hdr_size; I'll rely on Stephen and Yuanhan's judgment for this. >+ uint32_t frame_size = mtu + ether_hdr_len; >+ >+ if (mtu < ETHER_MIN_MTU || frame_size > VIRTIO_MAX_RX_PKTLEN) { >+ PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n", >+ ETHER_MIN_MTU, VIRTIO_MAX_RX_PKTLEN); Shouldn't last format print parameter should be (VIRTIO_MAX_RX_PKTLEN - ether_hdr_len)? i.e PMD_INIT_LOG(ERR, "MTU should........%d\n", ETHER_MIN_MTU, (VIRTIO_MAX_RX_PKTLEN - ether_hdr_len)); >+ return -EINVAL; >+ } >+ return 0; >+} > > /* > * dev_ops for virtio, bare necessities for basic operation > */ >@@ -677,7 +685,6 @@ static const struct eth_dev_ops virtio_eth_dev_ops = { > .allmulticast_enable = virtio_dev_allmulticast_enable, > .allmulticast_disable = virtio_dev_allmulticast_disable, >+ .mtu_set = virtio_mtu_set, > .dev_infos_get = virtio_dev_info_get, > .stats_get = virtio_dev_stats_get, > .xstats_get = virtio_dev_xstats_get, >-- >2.9.3.windows.1