Message ID | BN3PR03MB1494CD67AB36557559F2CCCADAC50@BN3PR03MB1494.namprd03.prod.outlook.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Delegated to: | Yuanhan Liu |
Headers |
Return-Path: <dev-bounces@dpdk.org> 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 7C7932BFF; Wed, 5 Oct 2016 01:18:07 +0200 (CEST) Received: from NAM03-BY2-obe.outbound.protection.outlook.com (mail-by2nam03on0086.outbound.protection.outlook.com [104.47.42.86]) by dpdk.org (Postfix) with ESMTP id 305F62BAD for <dev@dpdk.org>; Wed, 5 Oct 2016 01:18: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=m65MA9LfaaSEH/M6F7njAuGcC/fRTv9fLapoOxFAYBw=; b=jgIfZ5h4eFmVcReJH/MUqX496yYSrOLBGuwM0phTEoz2U3evFD+m2K7bAkACACPpgf8MEF0NtnvjZRjMlbwWbQHN+oLiq3T7+juurDFSWxkIHRJKCQA3WKGttmikiKmIKiZXrQ8bFe/xo/HsBmC2rylAJHYafOXHg7xrx3dXOTI= Received: from BN3PR03MB1494.namprd03.prod.outlook.com (10.163.35.145) by BN3PR03MB1494.namprd03.prod.outlook.com (10.163.35.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.649.16; Tue, 4 Oct 2016 23:18:03 +0000 Received: from BN3PR03MB1494.namprd03.prod.outlook.com ([10.163.35.145]) by BN3PR03MB1494.namprd03.prod.outlook.com ([10.163.35.145]) with mapi id 15.01.0649.022; Tue, 4 Oct 2016 23:18:03 +0000 From: "Dey, Souvik" <sodey@sonusnet.com> To: "mark.b.kavanagh@intel.com" <mark.b.kavanagh@intel.com>, "yuanhan.liu@linux.intel.com" <yuanhan.liu@linux.intel.com>, "stephen@networkplumber.org" <stephen@networkplumber.org> CC: "dev@dpdk.org" <dev@dpdk.org> Thread-Topic: [PATCH v7] net/virtio: add set_mtu in virtio Thread-Index: AQHSGpCEO2JH49nnlEKHRbCquIGY6aCTpQrAgAVQiPA= Date: Tue, 4 Oct 2016 23:18:02 +0000 Message-ID: <BN3PR03MB1494CD67AB36557559F2CCCADAC50@BN3PR03MB1494.namprd03.prod.outlook.com> References: <20160922135643.37636-1-sodey@sonusnet.com> <20160929203130.58712-1-sodey@sonusnet.com> 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: [66.30.138.194] x-ms-office365-filtering-correlation-id: 3f40e0f0-e379-4680-3f74-08d3ecacb0b8 x-microsoft-exchange-diagnostics: 1; BN3PR03MB1494; 6:H3y7j0s1gGDOiwkQT76hEvuvPqaoJ+YOvQiH5eFtj+nonpJggtaM5P8EpsOZmTURNZ2qnSLSUTzZJi7cPKhLgTD+i3U5Q0GwMsD1X4RK59DEUAQ4GOjbACdt6J/RUAaKZGJqJFTnXX5Zh4VTd6eRZpkJh+Ew2mYpidGCIZPO2WUSCKrKZUtU5nMvpokOduDDkQ/D614ZdKiw0+UTX0oHMKkjcHiAL94CgmtpvOqHNUECRh44QEEik7TARrea0D+3AWZFv14EJn6jreOWCmQNlzG3ORg53kngW5u8guPkwwHXS/RHUn1byE4b1D7XfT4c; 5:fYfViCokzvUFhI9hV+raudsdpNr9HBlTwm8fQ9//7myR9LcQQz0orI/ETvmZym0npK6IMjUE/2fNbLRS2y7nh80o7Cna49ZGE4QowADwVan8sd3RIja+lOGtbTOkXfrRHIoc3kL5Qe4cLufyRkjXTwtUT3U/KN6W/JfybiHKwDk=; 24:Z3N7X/OS2YJQ2/3nt+rSwdjs1RF/euNMEQotJ6P1j6CQ55a3e/2PpBrZeFeVraAmoKtNX2VAHHA2EWM1g1dMJc2UeJf/In2sBi+tF0YGiGA=; 7:pS86w2Ki15TPh6TF//HN1hJL9IgCZ3rdN1JHctxvQDw2dQ0kSPyqgZpHyI34mIDGkXQrfqATURoOp7CHHBAaHQZ52srq4qhdKePw3mZNW9bgRn7CkcspROkuu+kGP/evd/z2U8gcxy6wGoGtjLTcjzz2T+RZKgvm5icdBWZdh//8D3emQAL8efqtNx7MfUQmQhAQXV5V0ivnsajwaOy2gpAHcunrbTWQNRpAUqnNKUEsfUql6QAVRpYnyzeenJ06iCb8VPBa3+L9m/QnCrk/GcQOoH6n5TJ+FuyMwiLaOFy9tFuQUZ8k3mVmv3awNadOWPv7M0iKPOswpkjKVh9lGw== x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BN3PR03MB1494; x-microsoft-antispam-prvs: <BN3PR03MB1494626351D67B18D3C16964DAC50@BN3PR03MB1494.namprd03.prod.outlook.com> x-exchange-antispam-report-test: UriScan:(158342451672863)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040176)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001); SRVR:BN3PR03MB1494; BCL:0; PCL:0; RULEID:; SRVR:BN3PR03MB1494; x-forefront-prvs: 00851CA28B x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(7916002)(199003)(43544003)(53754006)(13464003)(377454003)(189002)(33656002)(97736004)(68736007)(76576001)(11100500001)(19580405001)(3900700001)(305945005)(101416001)(19580395003)(74316002)(9686002)(189998001)(5002640100001)(5001770100001)(4326007)(92566002)(2900100001)(8936002)(54356999)(76176999)(7696004)(2906002)(50986999)(10400500002)(5660300001)(102836003)(3846002)(2201001)(6116002)(86362001)(586003)(2501003)(3280700002)(7846002)(77096005)(66066001)(87936001)(3660700001)(81156014)(81166006)(106356001)(8676002)(105586002)(106116001)(99286002)(122556002)(7736002); DIR:OUT; SFP:1101; SCL:1; SRVR:BN3PR03MB1494; H:BN3PR03MB1494.namprd03.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; received-spf: None (protection.outlook.com: sonusnet.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: sonusnet.com X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Oct 2016 23:18:03.0114 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 29a671dc-ed7e-4a54-b1e5-8da1eb495dc3 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN3PR03MB1494 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 <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
souvikdey33
Oct. 4, 2016, 11:18 p.m. UTC
Hi All, Is there any further comments or modifications required for this patch, or what next steps do you guys suggest here ? -- 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 <sodey@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 acheive 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@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(+) .xstats_get = virtio_dev_xstats_get,
Comments
>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 <sodey@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@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
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); + 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,