From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id D072A6CD7 for ; Wed, 5 Oct 2016 10:15:56 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga105.jf.intel.com with ESMTP; 05 Oct 2016 01:15:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,448,1473145200"; d="scan'208";a="176539771" Received: from irsmsx151.ger.corp.intel.com ([163.33.192.59]) by fmsmga004.fm.intel.com with ESMTP; 05 Oct 2016 01:15:45 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.196]) by IRSMSX151.ger.corp.intel.com ([169.254.4.226]) with mapi id 14.03.0248.002; Wed, 5 Oct 2016 09:15:44 +0100 From: "Kavanagh, Mark B" To: "Dey, Souvik" , "yuanhan.liu@linux.intel.com" , "stephen@networkplumber.org" CC: "dev@dpdk.org" Thread-Topic: [PATCH v7] net/virtio: add set_mtu in virtio Thread-Index: AQHSGpCJDQZFRvYfBk2V8UZwgCllbqCTpQrAgAVQiPCAAIwv4A== Date: Wed, 5 Oct 2016 08:15:43 +0000 Message-ID: References: <20160922135643.37636-1-sodey@sonusnet.com> <20160929203130.58712-1-sodey@sonusnet.com> In-Reply-To: Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYjlmNzNiNTktMWM4MC00NmY4LTk5YTMtMDRjMTg3ZmJiNTYyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IkFjVnNXZGk3cDB5SEVoZTV2MW94UE1jK1NOMnBwMWNoS29naWtZVmJzeUk9In0= x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v7] net/virtio: add set_mtu in virtio X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Oct 2016 08:15:57 -0000 >Hi All, > Is there any further comments or modifications required for this patch, o= r 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@intel.com; yuanhan.liu@linux.intel.com; stephen@networ= kplumber.org; >dev@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 pr= oper. > >-- >Regards, >Souvik > >-----Original Message----- >From: Dey, Souvik >Sent: Thursday, September 29, 2016 4:32 PM >To: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com; stephen@networ= kplumber.org; >dev@dpdk.org >Cc: Dey, Souvik >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 > >--- >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/virti= o_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 =3D dev->data->dev_private; >+ uint32_t ether_hdr_len =3D 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 =3D 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 - eth= er_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 =3D= { > .allmulticast_enable =3D virtio_dev_allmulticast_enable, > .allmulticast_disable =3D virtio_dev_allmulticast_disable, >+ .mtu_set =3D virtio_mtu_set, > .dev_infos_get =3D virtio_dev_info_get, > .stats_get =3D virtio_dev_stats_get, > .xstats_get =3D virtio_dev_xstats_get, >-- >2.9.3.windows.1