From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM02-CY1-obe.outbound.protection.outlook.com (mail-cys01nam02on0074.outbound.protection.outlook.com [104.47.37.74]) by dpdk.org (Postfix) with ESMTP id 3D5C52BFF for ; Fri, 9 Sep 2016 17:33:34 +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=cyZm4UjQnVSpraYDiXMPIvqyxV5lB/gEwQMSQwPNMRA=; b=FQbUu8kys+3eZMYjUBJbhkRQFhJr2c8yncCXoJXS8mGOZuaAFiFiFevRAl9JEjXe0L7f9w7bNFhqB9fafYDMIagPILiKLVlqK3sLsKIOyEPuhqSWlq606CqgdsRE+BXSbSwzMogV3F00lNaV5VsxnsbLmETmmKX7yGYDr/obhmw= 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_0, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA_P384) id 15.1.609.9; Fri, 9 Sep 2016 15:33:32 +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.0557.032; Fri, 9 Sep 2016 15:33:32 +0000 From: "Dey, Souvik" To: "Kavanagh, Mark B" , Yuanhan Liu CC: "dev@dpdk.org" , "stephen@networkplumber.org" Thread-Topic: [PATCH v4]net/virtio: add mtu set in virtio Thread-Index: AQHSCL7xEgAVZJyiaUu1GlORUNZIaKBtbHzggANRoICAAHiC0IAADvoAgAAGmlA= Date: Fri, 9 Sep 2016 15:33:32 +0000 Message-ID: References: <20160907041832.35384-1-sodey@sonusnet.com> <20160909070009.GS23158@yliu-dev.sh.intel.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: 26de2108-d7eb-453b-7c42-08d3d8c6a835 x-microsoft-exchange-diagnostics: 1; CY1PR03MB1502; 6:42qWFpq0x7vFEFbzReMhWfarpntY26HxGygRFAa2n+7dpVvM1OCaMp1tWpojySeFaVRQP61L6poZzCIjMgztoXRiIIAMfuYDgK3uWK11H0d1FioAbeeqGelQcsyt8fb+9jjJBOxZngs0XG/Ybpj5tpMdGFrJ2uPvSTZoTDUUvg7KsoRxijcKw/UbU1ytNSjhW7iOyBUgdYOXqf0AWeE2TFykv14tdTTZQC2Zl1G3VvVQtwSDz6SuQmcJJc9oa/fUEJcTbrjgQBdx1svlEetqVeJ3P5mzwI9zkJ2iXwCUBVI=; 5:GYuKlAnkO/14HRsTAXiLXyLsP2nwjsYvMZStXzwTBL3R3MNgiP1qTa8iENelACek0uomsxVtitnWy6ZdwwaH2SvVaD/n15xjepOVCeOlW7NqAjQdeFDZV/cPAT3sj9Sl1qo79GdshZE9NdR79UH6tQ==; 24:SkEgX2/IgUV53opD9yAxwtOdGexVFjU0WD+W3/pqtH8fmVRjfFD4PZoZrHIpj33A7EE3ZtlgM7c9fzPBO9nBcWsK84dDxg2TO2U/pBAXTKY=; 7:2/aXxRSQVQQhT+Al4FyzLkRvBL4AYC//ZvyIoGxeSwCPqIVwjf0++yyKCJdRPyRI6qaUV7VIBT5evGyx2liA7W/uJLjmXZe4Tv18e23Scx0VxFY+UNVAyQy6CmEOsXLYXexZBbElvqshdInleOouI3QyjzZCB3ExHdsNfkxeSelGGAVMHkuR+gH9+RvrdXytvSxE6ogkwAvUYRm/Flhj5n7zoanKaClOsRrg/Y0f6zMopDhSURRfSBQwxVBvJFNJ 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)(8121501046)(5005006)(10201501046)(3002001); SRVR:CY1PR03MB1502; BCL:0; PCL:0; RULEID:; SRVR:CY1PR03MB1502; x-forefront-prvs: 00603B7EEF x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(7916002)(54534003)(13464003)(377454003)(189002)(51914003)(24454002)(199003)(81156014)(8676002)(81166006)(10400500002)(9686002)(97736004)(5001770100001)(74316002)(76176999)(8936002)(93886004)(50986999)(54356999)(3900700001)(5002640100001)(66066001)(7736002)(2906002)(3660700001)(7696004)(5660300001)(87936001)(305945005)(7846002)(68736007)(4326007)(189998001)(3280700002)(105586002)(586003)(106356001)(106116001)(76576001)(99286002)(102836003)(11100500001)(3846002)(19580395003)(86362001)(6116002)(101416001)(77096005)(2950100001)(2900100001)(19580405001)(92566002)(33656002)(122556002); 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 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: 09 Sep 2016 15:33:32.3110 (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 v4]net/virtio: add mtu set 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: Fri, 09 Sep 2016 15:33:34 -0000 Hi Mark, Yes I thought I did that change. Sorry once again.. making too many mistake= s. Changed it . Thanks.=20 The MTU here is L3 MTU. Setting this will help in reducing the fragmented/= multi-segmented packets and also in case we want to reduce the MTU below 15= 00, to support VXLAN or GRE tunnel for the packets in openstack and cloud e= nvironments. =20 --- drivers/net/virtio/virtio_ethdev.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio= _ethdev.c index 07d6449..da16ad4 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c static int virtio_dev_queue_stats_mapping_set( __rte_unused struct rte_eth_dev *eth_dev, @@ -652,6 +653,16 @@ virtio_dev_allmulticast_disable(struct rte_eth_dev *de= v) PMD_INIT_LOG(ERR, "Failed to disable allmulticast"); } =20 +static int +virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) +{ + struct virtio_hw *hw =3D dev->data->dev_private; + if (mtu < VIRTIO_MIN_RX_BUFSIZE || mtu > VIRTIO_MAX_RX_PKTLEN) { + PMD_INIT_LOG(ERR, "Mtu should be between VIRTIO_MIN_RX_BUFSIZE and VIRTI= O_MAX_RX_PKTLEN \n"); + return -EINVAL; + } + return 0; +} + /* * dev_ops for virtio, bare necessities for basic operation */ @@ -664,6 +675,7 @@ static const struct eth_dev_ops virtio_eth_dev_ops =3D = { .promiscuous_disable =3D virtio_dev_promiscuous_disable, .allmulticast_enable =3D virtio_dev_allmulticast_enable, .allmulticast_disable =3D virtio_dev_allmulticast_disable, + .mtu_set =3D virtio_mtu_set, =20 .dev_infos_get =3D virtio_dev_info_get, .stats_get =3D virtio_dev_stats_get, -- -- Regards, Souvik -----Original Message----- From: Kavanagh, Mark B [mailto:mark.b.kavanagh@intel.com]=20 Sent: Friday, September 9, 2016 11:05 AM To: Dey, Souvik ; Yuanhan Liu Cc: dev@dpdk.org; stephen@networkplumber.org Subject: RE: [PATCH v4]net/virtio: add mtu set in virtio > >Hi Liu, > >Yes agreed your comment. I will definitely remove the declaration as it=20 >is not really required. > So the latest patch will look like this . Yes I did rush a bit to=20 >submit the patch last will correct my suite. So sending the patch in a=20 >reply if we have more comments we can take a look and they re-submit the f= inal reviewed patch. Thanks for the help though. > >--- > drivers/net/virtio/virtio_ethdev.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > >diff --git a/drivers/net/virtio/virtio_ethdev.c=20 >b/drivers/net/virtio/virtio_ethdev.c >index 07d6449..da16ad4 100644 >--- a/drivers/net/virtio/virtio_ethdev.c >+++ b/drivers/net/virtio/virtio_ethdev.c > >+static int >+virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) { >+ struct virtio_hw *hw =3D dev->data->dev_private; >+ if (mtu < VIRTIO_MIN_RX_BUFSIZE || mtu > VIRTIO_MAX_RX_PKTLEN) { >+ PMD_INIT_LOG(ERR, "Mtu should be between 64 and 9728\n"); Hi Souvik, Why hardcode the values in the PMD_INIT_LOG? Why not do the following: PMD_INIT_LOG(ERR, "MTU should be between %d and = %d",=20 VIRTIO_MIN_RX_BUFSIZE, VIRTIO_MAX_RX_PKTLEN); That way, if the values that the macros evaluate to change, the log will up= date correspondingly. Also, does 'MTU' in this context relate to the L2 or L3 MTU? >+ return -EINVAL; >+ } >+ return 0; >+} >+ > /* > * dev_ops for virtio, bare necessities for basic operation > */ >@@ -664,6 +675,7 @@ static const struct eth_dev_ops virtio_eth_dev_ops =3D= { > .promiscuous_disable =3D virtio_dev_promiscuous_disable, > .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, >-- > >-- >Regards, >Souvik > >-----Original Message----- >From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] >Sent: Friday, September 9, 2016 3:00 AM >To: Dey, Souvik >Cc: dev@dpdk.org; stephen@networkplumber.org >Subject: Re: [PATCH v4]net/virtio: add mtu set in virtio > >On Wed, Sep 07, 2016 at 04:21:30AM +0000, Dey, Souvik wrote: >> Hi Liu, >> Submitted the version 4 of the patch as you suggested , > >Your patch is looking much better. But not really, you ignored few of my c= omments. > >> and have removed the Reviewed by line. >> I have still kept the function definition as to follow the same suit=20 >> as we have done for >other eth_dev_ops. > >That's because most of the method implementions are after the=20 >reference, thus the declaration is needed. > >For your case, I see no good reason to do that. BTW, if you disagree=20 >with my comment, you shoud made a reply, instead of rushing to sending a n= ew version. > >> -- >> Regards, >> Souvik >> >> -----Original Message----- >> From: Dey, Souvik >> Sent: Wednesday, September 7, 2016 12:19 AM >> To: dev@dpdk.org; stephen@networkplumber.org;=20 >> yuanhan.liu@linux.intel.com >> Cc: Dey, Souvik >> Subject: [PATCH v4]net/virtio: add mtu set in virtio >> >> >> Virtio interfaces should also support setting of mtu, as in case of=20 >> cloud it is expected to >have the consistent mtu across the infrastructure that the dhcp server=20 >sends and not hardcoded to 1500(default). >> >> Changes in v4: Incorporated review comments. >> Changes in v3: Corrected few style errors as reported by sys-stv. >> Changes in v2: Incorporated review comments. > >DPDK prefers to put the change log to ... >> >> Signed-off-by: Souvik Dey >> >> --- > >... here. > >So that they will be showed in mailing list (for review), but they will be= gone after apply. >In another word, we don't like to see those change log in git history,=20 >but we'd like to see them while review. > >> drivers/net/virtio/virtio_ethdev.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/drivers/net/virtio/virtio_ethdev.c >> b/drivers/net/virtio/virtio_ethdev.c >> index 07d6449..da16ad4 100644 >> --- a/drivers/net/virtio/virtio_ethdev.c >> +++ b/drivers/net/virtio/virtio_ethdev.c >> @@ -92,6 +92,7 @@ static void virtio_mac_addr_add(struct rte_eth_dev=20 >> *dev, static void >virtio_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index); =20 >static void virtio_mac_addr_set(struct rte_eth_dev *dev, >> struct ether_addr *mac_addr); >> +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu); >> >> static int virtio_dev_queue_stats_mapping_set( >> __rte_unused struct rte_eth_dev *eth_dev, @@ -652,6 +653,16 @@ >virtio_dev_allmulticast_disable(struct rte_eth_dev *dev) >> PMD_INIT_LOG(ERR, "Failed to disable allmulticast"); } >> >> +static int >> +virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) { >> + struct virtio_hw *hw =3D dev->data->dev_private; >> + if (mtu < VIRTIO_MIN_RX_BUFSIZE || mtu > VIRTIO_MAX_RX_PKTLEN) { >> + PMD_INIT_LOG(ERR, "Mtu should be between 64 and 9728\n"); > >I still saw those numbers. > > --yliu