[dpdk-dev] [PATCH v7] net/virtio: add set_mtu in virtio
Kavanagh, Mark B
mark.b.kavanagh at intel.com
Wed Oct 5 10:15:43 CEST 2016
>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
More information about the dev
mailing list