From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM01-SN1-obe.outbound.protection.outlook.com (mail-sn1nam01on0080.outbound.protection.outlook.com [104.47.32.80]) by dpdk.org (Postfix) with ESMTP id 1DCF15689 for ; Thu, 22 Sep 2016 04:29:21 +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=4cI5nsQg+hRkITcrIQ0V3mr7Xo+kzyWceQFwCMJjOKs=; b=fT1yNHlK2pzq8G22E8vHwm4eF4jKPI7wzmxw/lbHNcn1ZqrPrvlW1BdKhboxYQQV5tMX1+yKS8Iwz8CeI/LlQogRiwZprLvF0TCMhU0VCr2lyr0CjDW2nNr6R+byBM+alteRjlkgTFFq0Cf9xVbOmzYHDiCevWFIa91OPOagCGc= Received: from BN3PR03MB1494.namprd03.prod.outlook.com (10.163.35.145) by BN3PR03MB1495.namprd03.prod.outlook.com (10.163.35.146) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.639.5; Thu, 22 Sep 2016 02:29:19 +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.0629.015; Thu, 22 Sep 2016 02:29:18 +0000 From: "Dey, Souvik" To: Stephen Hemminger CC: "mark.b.kavanagh@intel.com" , "yuanhan.liu@linux.intel.com" , "dev@dpdk.org" Thread-Topic: [PATCH v6] net/virtio: add set_mtu in virtio Thread-Index: AQHSFF2bGYWatJTFW0SUpX36+50+dKCElTyAgAALYiCAAByIgIAACoXQ Date: Thu, 22 Sep 2016 02:29:18 +0000 Message-ID: References: <20160921231147.26820-1-sodey@sonusnet.com> <20160921162213.4b79d1ce@xeon-e3> <20160921184505.584367ef@xeon-e3> In-Reply-To: <20160921184505.584367ef@xeon-e3> 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: 62ebc02d-ea8f-473e-8c10-08d3e2904135 x-microsoft-exchange-diagnostics: 1; BN3PR03MB1495; 6:WVZoGW8YKwXXLMtS3bLZsbr9tY9kWwe9bqTYaoXVnfdHONkv/6u2hNfgv05F2XZ0AdVGhfguhYxxSJyyWb7DbEm0gbLCuBTutvndVUJWJuv5gyT8ScF/asrGqpURdZdD2vyT5M6cK+K6evkofD81GzvVv8tHfp0MYY/pcDAkHYF3B5bzNuwnB6lMkqrU5KZrlQGmXnY6elCSqjR4BdAhLvNgX4jnUQ7Jpt8Xv83zgHiEHaa1Wp3ETlCGAI3o41hE3QUjdO9sYCHj9KXYVKkNNsnId5ljhp1Tw1NLCnkAJUY=; 5:RY7k6yHV9KBg4NzjSV126RCK3ot6kF2ugFh9mRMzLx9olconx23mEPfbyQ2b/EoXcC+7RYyaB7pJ54Gfk2KeNhy3my6/KG96/dOcjKBCMJEbZP0TPqLbt3OTFpwlX+ZaDVu566F/1tdu8BXjtGOw4A==; 24:MUym440uebv0RGtNNgFrFbS9OcU8KzMH1AzQXEGuv7e/IK6f2UD3sEtIpTEwqlIwi9SpTsaRHfMjT695ihbhY1F8QoSS1zT3Xb1xy+vRW+w=; 7:TggDsus53ghWrV4LJsX28mtnpZuIjqPgC/yKQpClYq+DRA3mDsX7ZCiSw6KjmbqvJGKp44i0TjCLVIGEBjEmHBsXlw55gR6TZ03jD5Kqlfib4oxpSK6SCwAb9NR46LqPzju0GZOzBdyDnoU1wCx6bYLGyvYcYRk2G/hSmTVeXlQVdfFUoNPckkEgPh/9BGn9pNXgttnLIfnknQT4cjv7v5K/qn7bSwFtHI8xvux3xDowIihbUEJ5hcRjawM5eQPAw4fBoVWAwUhY301QJkQjjZlrk8tT1zYu/l+T/fsTjSrefqiWnEirgIayou+MGlVb x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BN3PR03MB1495; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040176)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001); SRVR:BN3PR03MB1495; BCL:0; PCL:0; RULEID:; SRVR:BN3PR03MB1495; x-forefront-prvs: 0073BFEF03 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(7916002)(199003)(377454003)(189002)(13464003)(24454002)(3280700002)(8676002)(2906002)(81156014)(4326007)(81166006)(33656002)(3660700001)(101416001)(5660300001)(122556002)(76176999)(2950100001)(2900100001)(54356999)(50986999)(110136003)(7696004)(19580395003)(76576001)(19580405001)(77096005)(66066001)(99286002)(7846002)(7736002)(106116001)(74316002)(305945005)(106356001)(105586002)(8936002)(10400500002)(5002640100001)(92566002)(87936001)(3846002)(86362001)(6116002)(586003)(102836003)(93886004)(68736007)(189998001)(9686002)(97736004); DIR:OUT; SFP:1101; SCL:1; SRVR:BN3PR03MB1495; 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: 22 Sep 2016 02:29:18.2775 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 29a671dc-ed7e-4a54-b1e5-8da1eb495dc3 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN3PR03MB1495 Subject: Re: [dpdk-dev] [PATCH v6] 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: Thu, 22 Sep 2016 02:29:21 -0000 I still fail to understand how the dev_info.max_rx_pktlen is wrong here, an= d the hw->rx_max_pktlen is defined at all ? If you see the other driver cod= e we have used the same way there too. Also for the vlan part isn't it bett= er the take the worst case for the mtu lower boundary check, as that will b= e valid for both vlan offload on and off cases.=20 -----Original Message----- From: Stephen Hemminger [mailto:stephen@networkplumber.org]=20 Sent: Wednesday, September 21, 2016 9:45 PM To: Dey, Souvik Cc: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com; dev@dpdk.org Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio On Thu, 22 Sep 2016 00:08:38 +0000 "Dey, Souvik" wrote: > Answers inline. >=20 > -- > Regards, > Souvik >=20 > -----Original Message----- > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Wednesday, September 21, 2016 7:22 PM > To: Dey, Souvik > Cc: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com;=20 > dev@dpdk.org > Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio >=20 > On Wed, 21 Sep 2016 19:11:47 -0400 > Dey wrote: >=20 > > =20 > > + > > +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */ > > + > > +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) { > > + struct rte_eth_dev_info dev_info; > > + uint32_t ether_hdr_len =3D ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN= _TAG_SIZE; > > + uint32_t frame_size =3D mtu + ether_hdr_len; > > + > > + virtio_dev_info_get(dev, &dev_info); > > + > > + if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen)= { > > + PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n", > > + ETHER_MIN_MTU, > > + (dev_info.max_rx_pktlen - ether_hdr_len= )); > > + return -EINVAL; > > + } > > + return 0; > > +} >=20 > I am fine with the general idea of this patch but: > 1. Calling virtio_dev_info_get is needlessly wasteful when all you want > is to access the max packet length. Since max_rx_pktlen is always > VIRTIO_MAX_RX_PKTLEN, please just use that. > [Dey, Souvik] I am using the virtio_dev_info_get as in future can/may sup= port the max_rx_pktlen as a variable to be set by the application. This wi= ll keep the changes future proof. As we need to support till 65535 instead = of 9728 as the linux does. Fine, then just dereference hw->rx_max_pktlen. Driver code can/should refer= ence its own data directly. >=20 > 2. Defining VLAN_TAG_SIZE is irrelevant if doing vlan offload. > [Dey, Souvik] vlan offload is not mandatory. Se again still have vlan bei= ng sent up to the application. In that case we need to consider the vlan le= ngth in the Ethernet size. The code needs to handle both vlan offload (or not), correctly. You are ass= uming the worst case here. >=20 > 3. Virtio doesn't insert CRC, therefore CRC_LEN is irrelevant [Dey,=20 > Souvik] I am not sure of this. Mark commented earlier to consider this le= ngth too. Mark what do you suggest ? Actually, the thing that matters is the size of the merge rx buf header, no= t the CRC. The patch is still buggy.