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 2C460A0597; Wed, 8 Apr 2020 14:37:33 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A65401BF26; Wed, 8 Apr 2020 14:37:32 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 5799C1C0BF for ; Wed, 8 Apr 2020 14:37:30 +0200 (CEST) IronPort-SDR: ytRXeuLgDd6kqHISrjPtuP8VhYjb1CK+3cY3iSoOcnfV4rcMHnLPYe+kbrs53MM0rihGJMkycb IMKV9fdyByyA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Apr 2020 05:37:29 -0700 IronPort-SDR: fG+PCpn5oj9ZWAlrBtdYrcIxiSUqZQeR+8ytxXHZNT9nMGG3olPnjX6CCZm+DLMdLC9/ebIi5r HcfDRBZWEC2w== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,358,1580803200"; d="scan'208";a="242351888" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga007.fm.intel.com with ESMTP; 08 Apr 2020 05:37:29 -0700 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 8 Apr 2020 05:37:28 -0700 Received: from fmsmsx605.amr.corp.intel.com (10.18.126.85) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Wed, 8 Apr 2020 05:37:28 -0700 Received: from FMSEDG001.ED.cps.intel.com (10.1.192.133) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Wed, 8 Apr 2020 05:37:28 -0700 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.40) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 8 Apr 2020 05:37:26 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=cUUFmNih8XTimg5+lMju6rUoO3eEb6G5CQk8ZTtD5LE8nRP7SdhvwmRgkJGP0Aq4KZOJzA1JBuTIdsLfvp72dC6OnaV3mvZp5XiH7HKkgLGHN7LOEAXyYYWAot++hW3ZqZHOhjyo53/HhhOItMnszLaADs7QGFn6CvY5sBSjpFR5fYpu+VzgVCPiJbmxETpN1Mw7Nks561nhBkZrsEogmiWxqCheDC123JyxLaJKBfl7hB1J465wX8dJl4+eLE1XH5NW3pEY4aHgrnIblZtwn4I/haGwV1pO6WVty2q1rDeFojJKDOexD4T4ETfxgBbQO/YvJ8ZRZcppiYHqBeiKNQ== 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=+n8XLXJyFjvrqoeTS3DPUQJEZ2x6NDDh8wXaaJmGMRY=; b=nV7xfMstZhC+TnpuSvcabXKhk+XUDfPr43wRHQ+O8aD6W6ZDZi4mmmH06qkLDrzop7jTardqf32rLHoO2UjdEFyyQDQQbxLxPJq5X1kLHZZ57ZN00Ri2btKFBCV16z2e6lkKgaTDzlCUtyVop4ikNCwqIzEmg5YIY2uepcc8sHNBIOVBmVZomkeC+su7I+PrCaUHEaGOSyLpiUJpaVwtqeKuq3knj6nfc7eiNGH8h6p07W15s1vR+QiXljROaEQ8KykEAsc89MhTMnHhBoz2BbQWvKfFco9tCuSWCX7OM8HpWUTxoZdIX3i024P8ibEMHoHlPYpQjM4lgeEd4m0pOg== 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=+n8XLXJyFjvrqoeTS3DPUQJEZ2x6NDDh8wXaaJmGMRY=; b=a6A6zZI9SvJJMUx5ZAckOqAw8AfdilKbo7CFvun6zFxNNSZ3sBVLOxv4FkXgcWb5ZV4LlURSXwBJDSclz1L9ZEvmUr3fwWdF3y3baCGDr11c1HzuTlZYzlelFLiFgfSy+c06fmSxaVn3D3e6ApN7OO2ukvWC3bfw5vQVXzvRfbE= Received: from SN6PR11MB2558.namprd11.prod.outlook.com (2603:10b6:805:5d::19) by SN6PR11MB2990.namprd11.prod.outlook.com (2603:10b6:805:cf::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2878.15; Wed, 8 Apr 2020 12:37:24 +0000 Received: from SN6PR11MB2558.namprd11.prod.outlook.com ([fe80::3982:ee47:b95a:4ed4]) by SN6PR11MB2558.namprd11.prod.outlook.com ([fe80::3982:ee47:b95a:4ed4%3]) with mapi id 15.20.2900.015; Wed, 8 Apr 2020 12:37:24 +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: AQHWCFT5t7STmfoiVEqeJTT1QdTvGahtiAqAgAAeepOAAA0foIAAVDMAgAEfokA= Date: Wed, 8 Apr 2020 12:37:24 +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.163] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 1e98985d-65e2-459e-b5f8-08d7dbb996f2 x-ms-traffictypediagnostic: SN6PR11MB2990: 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: 0367A50BB1 x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:SN6PR11MB2558.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(10019020)(396003)(346002)(376002)(39860400002)(136003)(366004)(6506007)(5660300002)(186003)(66476007)(66946007)(86362001)(64756008)(66556008)(66446008)(2906002)(7696005)(26005)(4326008)(33656002)(9686003)(76116006)(8936002)(71200400001)(478600001)(55016002)(54906003)(8676002)(6916009)(81166007)(316002)(81156014)(52536014); DIR:OUT; SFP:1102; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: eZJ2UMWDwOsoaD/ktRquu0jkXxmUGDk+LEw5LPnDPI2kBES8AUlXA45NH/jItj9d7j3lBqGo2qmawfOT69X9lnYucdGAbdVxp/oiTpmflPwTvN5wMXaYAVTqrl15aeOgRnvQZTzZcwMPhm0zpkqb4fJnquQsFXziQbpithJFWpt4gWiZLLHKeBES9gXCPKCbFsrh/0J354hw7fHrgXfA2nDzYysy9jdkf4zwkxCSpGUDDOpbUlWZ1YYqgvjv0OMH+NEAe8ZNtUEsfyFpQQBrG4BpER3ePAmXXRilZudoFyN6S0/pISN8GS0fXrWcSepKQ5EHxcXIOdS3dz4y/nivBGinY9cZiPUdKVL+WvIXPgy5rmAoL8qEto9xKTSX0lRn9j6r/Zxeh2c4XMUYwShtk4uiuNIN4618oyEDbyFqCJSnKOubToF0CSilqi6Cc305 x-ms-exchange-antispam-messagedata: gIWc8h+HE+81A+7/R919JNnE8U6Xi5KNogS7B2Gz8B4/Mpcizgy55HQD1zRaNCArkd1hEgd9icJhupLXO6z8gvgQvqAa4FJPhJI89p7B3R1DPuT8dwn0eM4W1SxZ0a4e2Zocuk8GuEyMW8+bCr/AMA== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 1e98985d-65e2-459e-b5f8-08d7dbb996f2 X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Apr 2020 12:37:24.3156 (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: wbnkxGYfQYBKmsOAlOXKG16IPqhsvttr+WpJAyFO+4w71FNMtnRb05zFtOE5k+SySYwBHU6TmiVnqATtcgroGnfFEeK44p0X6EopCX6jL/I= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR11MB2990 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" > >> >> The IPv4 specification says that each fragment must at least the si= ze of > >> >> an IP header plus 8 octets. When attempting to run ipfrag using a > >> >> smaller size, the fragment library will return successful completio= n, > >> >> 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/libr= te_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 = octets > >> >> + */ > >> >> + if (unlikely(mtu_size < (sizeof(struct rte_ipv4_hdr) + 8*sizeof(c= har)))) > >> >> + return -EINVAL; > >> >> + > >> > > >> > Same comment as for ipv6: ipv4 min MTU is 68B. > >> > >> 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. > >> > >> > Why do we need extra checking here? > >> > >> 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. > >> > >> 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. >=20 > I'll add documentation as another patch. >=20 > > 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 NU= LL || > > pool_indirect =3D=3D NULL || mtu < MIN_MTU) > > return -EINVAL; > > > > wouldn't introduce any real slowdown. >=20 > Agreed. >=20 > > 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 hea= der). > > So, I think we just need to document that it is a user responsibility t= o > > provide not fragmented yet packet, without any pre-fragment headers. >=20 > Makes sense. Then again, the v6 frag code will need to preserve many of > the headers anyway, so since we have to read them, maybe it makes > sense to do the check here anyway. WDYT? If we want to make this function fully compliant to what rfc8200 says, then yes - extra changes is required in current implementation: 1. somehow obtain information about pre-fragment extensions length 2. use info from #1 to put fragment header at proper location. And extra testing of course. Probably safer and easier, for that patch just add formal parameter checkin= g. And if you feel like that - have the hard part as a separate patch. >=20 > > Konstantin > > > >> > >> >> /* > >> >> * 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