From patchwork Wed Oct 5 14:05:04 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: souvikdey33 X-Patchwork-Id: 16389 X-Patchwork-Delegate: yuanhan.liu@linux.intel.com Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id B50C02B96; Wed, 5 Oct 2016 16:05:07 +0200 (CEST) Received: from NAM03-DM3-obe.outbound.protection.outlook.com (mail-dm3nam03on0067.outbound.protection.outlook.com [104.47.41.67]) by dpdk.org (Postfix) with ESMTP id AE8A72B8B for ; Wed, 5 Oct 2016 16:05:06 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=SonusNetworks.onmicrosoft.com; s=selector1-sonusnet-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=YEad9Wap8zHugrDrzg8FfOqVj2Tl6NBPRPFlTvNa+s8=; b=T3eNAMxxcwqRVzM7aI9UVtbSj7pDyk5Zvg2w821oy0vuyIKktNvDf4ZSsLPcCZZnUALnsq2hwlYEIf2xOApDC0P5tr2BZtlAWt77MJPtZcwR011eLLn2YanTSn6QkPKM2tnTKfCIE5+CKMTgXmnJbI77xodNVleV+5EB9csLaLY= Received: from CY1PR03MB1503.namprd03.prod.outlook.com (10.163.17.21) by CY1PR03MB1502.namprd03.prod.outlook.com (10.163.17.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.649.16; Wed, 5 Oct 2016 14:05:05 +0000 Received: from CY1PR03MB1503.namprd03.prod.outlook.com ([10.163.17.21]) by CY1PR03MB1503.namprd03.prod.outlook.com ([10.163.17.21]) with mapi id 15.01.0649.021; Wed, 5 Oct 2016 14:05:05 +0000 From: "Dey, Souvik" To: "Kavanagh, Mark B" , "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: AQHSGpCEO2JH49nnlEKHRbCquIGY6aCTpQrAgAVQiPCAAJaigIAAX2Lg Date: Wed, 5 Oct 2016 14:05:04 +0000 Message-ID: References: <20160922135643.37636-1-sodey@sonusnet.com> <20160929203130.58712-1-sodey@sonusnet.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=sodey@sonusnet.com; x-originating-ip: [208.45.178.4] x-ms-office365-filtering-correlation-id: 40b8be5d-b7a0-4a41-4d30-08d3ed289b79 x-microsoft-exchange-diagnostics: 1; CY1PR03MB1502; 6:qZp0v9T1z2EnWM38PzQRi22waKmmRyPoqUipspaYqUvHqVknqx8q/L3gKr16vRM/n1ktioXBKxsBny9CIVK06F/dRgsA0UOhomjqFoUPl6PJ0wOCAat8b0d+cj4rlPSyF80f9spYF4g/bLPxWey+zDeIUrBLYZtbwJxqKrqXXey7t4jAqGlW44BPRZiXUorrNkj7omI2mI9Fj/lR35gAcjjRUSe/2liQq2gAa19RSr80pE7xkXM+HTqBf4kn3vNvoY+OaAE7hS98DUD/oi84Eo+OI+AA+i9Qq00CBMEN9IAK2J32YNQW1AMFI8qe7v3D; 5:4KtB1p6UKiDRj2JOJVvEi9I7DKpcyl9ocCbHaPIvMHigoJifTXpklDb9DKtIWT83HVj34NWpv4Tvv6eyrx5a6vbtc4WpdXAijmKIlRZFLFwC/D2xGSf6FHx8gA5o7qCTjBGhKVtwQxb9XyMPr0Q1RB+pjKzb/CQtK/2F0FPONlg=; 24:Qymfcc9HZi1h+/Bp31qKJNIuatDbDKD0Kho0eu6uA7CxI+w7rSymCThhZrZJGyyjXiFzkIcO4TuLAj6jFvUsYsQbjjUsKBU0wpYQlPplSHU=; 7:6jDeke5aQmmpdj2f6BWUtorGfyuqbaeyyD0lDCa/eWJEbCECBJc1I0xmvAFCdlvBmPusYEt9VHb08uTgcGv+Dk100/BGDGPbdxaGye4MoKncyvFYQIoGawFLW8825yKfMYW7Vvfi2fpbDEGEQ8oIxof17/Vaqnx2hkETqV0iQXWM49vj9iBAa39zRlS6T3kZC3r2b3/uMb7yDrcU8dY1nlwO6RG+umVRq1tWNvibXtrmRK4EqL2wPWwZsrvlJ0g9S6qLwhoNqwSuWG4CsBVlEezlzDiq/DppH/BRVPZwzSATn85glEiMAC4Cil3lkUP1DUXYwNw4bsv/2uttHfUtyA== x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:CY1PR03MB1502; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(158342451672863)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040176)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046); SRVR:CY1PR03MB1502; BCL:0; PCL:0; RULEID:; SRVR:CY1PR03MB1502; x-forefront-prvs: 008663486A x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(7916002)(377454003)(43544003)(189002)(53754006)(199003)(13464003)(7846002)(7736002)(5002640100001)(2501003)(122556002)(68736007)(2900100001)(19580405001)(9686002)(19580395003)(3846002)(4326007)(11100500001)(77096005)(2950100002)(93886004)(92566002)(87936001)(102836003)(6116002)(8936002)(2201001)(7696004)(3280700002)(81156014)(81166006)(54356999)(50986999)(10400500002)(76176999)(101416001)(76576001)(5001770100001)(3660700001)(5660300001)(74316002)(86362001)(189998001)(586003)(2906002)(99286002)(66066001)(8676002)(33656002)(105586002)(106356001)(305945005)(106116001)(97736004); DIR:OUT; SFP:1101; SCL:1; SRVR:CY1PR03MB1502; H:CY1PR03MB1503.namprd03.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; received-spf: None (protection.outlook.com: sonusnet.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: sonusnet.com X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Oct 2016 14:05:04.8872 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 29a671dc-ed7e-4a54-b1e5-8da1eb495dc3 X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR03MB1502 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Yes Mark, I have modified the patch with the below comments. drivers/net/virtio/virtio_ethdev.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) Sent: Wednesday, October 5, 2016 4:16 AM To: Dey, Souvik ; yuanhan.liu@linux.intel.com; stephen@networkplumber.org Cc: dev@dpdk.org Subject: RE: [PATCH v7] net/virtio: add set_mtu in virtio >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@intel.com; yuanhan.liu@linux.intel.com; >stephen@networkplumber.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 proper. > >-- >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@networkplumber.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/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 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) */ + +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; + 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 - ether_hdr_len)); + return -EINVAL; + } + return 0; +} Let mem know if this looks good or we have few more comments. -- Regards, Souvik -----Original Message----- From: Kavanagh, Mark B [mailto:mark.b.kavanagh@intel.com]