From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 03AFAA0588; Tue, 7 Apr 2020 16:14:43 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DAB822B96; Tue, 7 Apr 2020 16:14:42 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id BB4FDFFA for ; Tue, 7 Apr 2020 16:14:40 +0200 (CEST) IronPort-SDR: a3Hphl8XzuDM1wJoq4I0NYFtD+EDj67ysrkXfEb8kxZwNlChKflvIFbnRz6q6fGeijVoTA++Hw LPCpAIlHbxtg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Apr 2020 07:14:39 -0700 IronPort-SDR: Zt5coW4UBvTAY2K47WeRHKedbSh6lour44vfhUIEf97Df7O+9HuDkgFI9ZuB06scyslSP8XCPy I9wHoxbEu0Yw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,354,1580803200"; d="scan'208";a="361540318" Received: from orsmsx101.amr.corp.intel.com ([10.22.225.128]) by fmsmga001.fm.intel.com with ESMTP; 07 Apr 2020 07:14:38 -0700 Received: from orsmsx603.amr.corp.intel.com (10.22.229.16) by ORSMSX101.amr.corp.intel.com (10.22.225.128) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 7 Apr 2020 07:14:38 -0700 Received: from orsmsx603.amr.corp.intel.com (10.22.229.16) by ORSMSX603.amr.corp.intel.com (10.22.229.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Tue, 7 Apr 2020 07:14:37 -0700 Received: from ORSEDG002.ED.cps.intel.com (10.7.248.5) by orsmsx603.amr.corp.intel.com (10.22.229.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Tue, 7 Apr 2020 07:14:37 -0700 Received: from NAM02-SN1-obe.outbound.protection.outlook.com (104.47.36.55) by edgegateway.intel.com (134.134.137.101) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 7 Apr 2020 07:14:37 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=aOsT9o0hfmp9UA7u3yC0gE9UxMueDtpiU3CTW5Eyr+e/4PHLQ0bEPhAyFuSjLgrXSdMrIiKMT7hznI1Xq1enjEgvPy07d0tS7Db9MxHt/1fyENUQaRY3wcBh9F6OPJzmPAqs9WsfRWpbh7FcpV8ZHEgjCzbPNXJ/vkh7ct8MyTyNKCux7y7pFujT5T8XJpErhPoygcAFi6dD/ySu6tiBb0qiE2uofVc3edy6IsrvNnr9zJP/MDRfY9PWp8Inrcugrk22BFAVaNn1cr57xVDCgkkw2w0iiksyJriSpfXcXZX4d9Y/gbb+Wk3INeu2jmrpZs43yXnGO9ZZ2oKSEyy+Xw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=DzpK85Nk2JZW5yns9VRNl/5xMw/NtUagvnsFXE76pew=; b=drGBNDEjLYeYlNGq8vtUjpbApQlQzqgdfHLIm2yTDAmTf36UhNFnupvbKWccjkLLfBae+n+LGle6M3clsWCdsJa6Mu6atPScpX6wzX89yo96WcDj6vvMWFqDcgEptjFFGg1KOk68DhKPskfEuL0SovQl/fE8GZ3NhbiPtpj5Sx1hHoqgq/Twknqo+mE4ky84gAACIuz2E/3XbR+pOEXmc5yW9FE6H9o0EHXvo2u6ZpmG54s4cng7B3qendYttj8247VVjyzTNucUJe6dcvyUn1Ndy+8cZd/tBnxK+BT9HGRVFvPZ3QE60Sx2X76P1rmo2sSJBX21YGMHOj1HDjRhJw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=DzpK85Nk2JZW5yns9VRNl/5xMw/NtUagvnsFXE76pew=; b=FDJOzo6tCBLDZWunLyTh2cKWSLrsYIxfsLgql02Sw7Veb/VDfG87vy1eK091ZGffBK99HI2RpL0NderK005RDrObEB/Ovuc+JxJE26Y9SZJ+SEgtC7fvAaetVtkfgBJtzfILVjXbQa6Sjg3AcpUEbAChhmfX0UTj6JIdxUFVSKw= Received: from BYAPR11MB2549.namprd11.prod.outlook.com (2603:10b6:a02:c4::33) by BYAPR11MB3783.namprd11.prod.outlook.com (2603:10b6:a03:f5::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2878.15; Tue, 7 Apr 2020 14:14:35 +0000 Received: from BYAPR11MB2549.namprd11.prod.outlook.com ([fe80::141d:cf28:589e:5f49]) by BYAPR11MB2549.namprd11.prod.outlook.com ([fe80::141d:cf28:589e:5f49%4]) with mapi id 15.20.2878.018; Tue, 7 Apr 2020 14:14:35 +0000 From: "Ananyev, Konstantin" To: Aaron Conole CC: "dev@dpdk.org" , Sunil Kumar Kori , "Burakov, Anatoly" , Chas Williams , "Richardson, Bruce" , "David Marchand" Thread-Topic: [PATCH v3 1/4] ip_frag: ensure minimum v4 fragmentation length Thread-Index: AQHWCFT5t7STmfoiVEqeJTT1QdTvGahtiAqAgAAeepOAAA0foA== Date: Tue, 7 Apr 2020 14:14:34 +0000 Message-ID: References: <20200401131849.2209336-1-aconole@redhat.com> <20200401183917.3620845-1-aconole@redhat.com> <20200401183917.3620845-2-aconole@redhat.com> In-Reply-To: Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.2.0.6 authentication-results: spf=none (sender IP is ) smtp.mailfrom=konstantin.ananyev@intel.com; x-originating-ip: [192.198.151.181] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: e43d967d-ff6b-480d-21cd-08d7dafdffe1 x-ms-traffictypediagnostic: BYAPR11MB3783: x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 036614DD9C x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR11MB2549.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(10019020)(39860400002)(366004)(346002)(396003)(376002)(136003)(4326008)(86362001)(81156014)(8676002)(7696005)(64756008)(54906003)(2906002)(8936002)(81166006)(316002)(55016002)(6916009)(9686003)(76116006)(33656002)(186003)(6506007)(66946007)(66476007)(66446008)(26005)(478600001)(52536014)(66556008)(71200400001)(5660300002); DIR:OUT; SFP:1102; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 6/FvcJGQ0sQxGGjjBDZeDWTWTTa5X4zdRQq/xjo/HiOlpLAZ0C8FpWorLhkKrroDqcXfIwQCZYSTrOwNj5FfKjlo60YCtOdNTEshfMg6DFqqO03AhPB+OjzNUuFQbUmLEkxFTVwneZE945+O+3r1lmVC029j7VGjspEdy6BnJSUxd5d1bD1AA4CaOQQVKzYEAPLdlTb2zBs9VDVy0WiWIeEbnuRU/xWQfhu36RS3BnyP95k5nXaoBDtfi+4e7k8oqLRDsKnPygqtL31+EkrQZSEjF+MVv6cd051LnpPkqqnBxAam43ZkW5ficVxDJIHoDLuW5+n14m8Lt/vXdyxXHkQ56p/6txhaDgFDaL77UFla5ATfvA4JFyKYHAzjawygCkTAB5LbXpS9Jy1JnBBK3OTpELhZ49fH/eHsJ+Xe2D+8UV4x/pmzF1p/u+TNhIX5 x-ms-exchange-antispam-messagedata: IbXFoF2u3H5CEYh3jxMy+sSBJesQ0nF8frfXkMZBppQdsL9QF2ru9zZMkgrqIaPUpJpOMFum3tE6oahXkz3IdGzgPpHLLc2G70dXldMFGpg84whgMDapvxVGgN4B1rJZMR7oY2ai93CnfFguUn8Phg== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: e43d967d-ff6b-480d-21cd-08d7dafdffe1 X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Apr 2020 14:14:34.9640 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: +BpsP0TSDX3usztBI12irCFEGwMUgh7jLpkYyFtp0SlymRPnjQXasYntuLgdmhlIrzAiHEx5UbuspghmDJ+clJ+wyZOrAxMBSDf5Hc8RSio= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB3783 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH v3 1/4] ip_frag: ensure minimum v4 fragmentation length X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" >=20 > "Ananyev, Konstantin" writes: >=20 > >> > >> The IPv4 specification says that each fragment must at least the size = of > >> an IP header plus 8 octets. When attempting to run ipfrag using a > >> smaller size, the fragment library will return successful completion, > >> even though it is a violation of RFC791 (and updates). > >> > >> Signed-off-by: Aaron Conole > >> --- > >> lib/librte_ip_frag/rte_ipv4_fragmentation.c | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >> diff --git a/lib/librte_ip_frag/rte_ipv4_fragmentation.c b/lib/librte_= ip_frag/rte_ipv4_fragmentation.c > >> index 9e9f986cc5..4baaf6355c 100644 > >> --- a/lib/librte_ip_frag/rte_ipv4_fragmentation.c > >> +++ b/lib/librte_ip_frag/rte_ipv4_fragmentation.c > >> @@ -76,6 +76,12 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, > >> uint16_t fragment_offset, flag_offset, frag_size; > >> uint16_t frag_bytes_remaining; > >> > >> + /* > >> + * Ensure the IP fragmentation size is at least iphdr length + 8 oct= ets > >> + */ > >> + if (unlikely(mtu_size < (sizeof(struct rte_ipv4_hdr) + 8*sizeof(char= )))) > >> + return -EINVAL; > >> + > > > > Same comment as for ipv6: ipv4 min MTU is 68B. >=20 > I can change it. I suspected that if I went with 68 here and 1280 in > the v6 code, I would get pushback, but I should have just submitted it > that way to begin. >=20 > > Why do we need extra checking here? >=20 > These are error conditions to submit to fragmentation module. Someone > needs to do the check - either it is done in the application or the > library. If the library doesn't, and the application writer doesn't > know they must write these checks (it isn't documented anywhere), then > we get non compliant behavior. By putting it in the library, we can > clearly signal the application writer such a case has occurred. >=20 > Should we not do error checking? It depends I think... In many data-path functions we skip parameter checking. These fragment() functions are data-path too. Agree, it is not stated clearly in these functions formal comments, as it should be. After another thought - these functions are quite heavy-weighed anyway, so probably formal parameter checking, something like: if (pkt_in =3D=3D NULL || pkts_out =3D=3D NULL || pool_direct =3D=3D NULL |= | pool_indirect =3D=3D NULL || mtu < MIN_MTU) return -EINVAL; wouldn't introduce any real slowdown. About more intense checking - like parsing all extension headers, etc. - I think it would be too much overhead. Again for that there is a special function that user can call directly: rte_ipv6_frag_get_ipv6_fragment_header (though its current implementation also checks only first extension header)= . So, I think we just need to document that it is a user responsibility to provide not fragmented yet packet, without any pre-fragment headers. Konstantin >=20 > >> /* > >> * Ensure the IP payload length of all fragments is aligned to a > >> * multiple of 8 bytes as per RFC791 section 2.3. > >> -- > >> 2.25.1