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 8A743A0588; Tue, 7 Apr 2020 14:52:57 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 79F732B96; Tue, 7 Apr 2020 14:52:56 +0200 (CEST) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) by dpdk.org (Postfix) with ESMTP id 7998B2B86 for ; Tue, 7 Apr 2020 14:52:55 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1586263974; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MBkgALGWLNJ2mLSnjuLDMEd+UTKzNuuljVjsJkorV14=; b=BP+sDM2bGHDSKm7mnV8dMfbT/L0M0iH4Oe+9C4V/KMZLcCr7nXfZKI3mVUTSoaq1JpydqU zgmE0kVJO3699d0mvqLGBxNLPWyffyPN1X8/eblLLZ/H6SQf11svLHZGNtnH7bJKEmihHB sCvjDFM3gCdS6Ao1JTSVaxBlCgJz4nc= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-211-MltYew1-O4qclZJgJv9i-Q-1; Tue, 07 Apr 2020 08:52:51 -0400 X-MC-Unique: MltYew1-O4qclZJgJv9i-Q-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id ED25F800D53; Tue, 7 Apr 2020 12:52:49 +0000 (UTC) Received: from dhcp-25.97.bos.redhat.com (ovpn-116-136.phx2.redhat.com [10.3.116.136]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CA3405C1BB; Tue, 7 Apr 2020 12:52:46 +0000 (UTC) From: Aaron Conole To: "Ananyev\, Konstantin" Cc: "dev\@dpdk.org" , Sunil Kumar Kori , "Burakov\, Anatoly" , Chas Williams , "Richardson\, Bruce" , David Marchand References: <20200401131849.2209336-1-aconole@redhat.com> <20200401183917.3620845-1-aconole@redhat.com> <20200401183917.3620845-2-aconole@redhat.com> Date: Tue, 07 Apr 2020 08:52:45 -0400 In-Reply-To: (Konstantin Ananyev's message of "Tue, 7 Apr 2020 11:10:03 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain Content-Transfer-Encoding: quoted-printable 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" "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). >>=20 >> Signed-off-by: Aaron Conole >> --- >> lib/librte_ip_frag/rte_ipv4_fragmentation.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >>=20 >> 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, >> =09uint16_t fragment_offset, flag_offset, frag_size; >> =09uint16_t frag_bytes_remaining; >>=20 >> +=09/* >> +=09 * Ensure the IP fragmentation size is at least iphdr length + 8 oct= ets >> +=09 */ >> +=09if (unlikely(mtu_size < (sizeof(struct rte_ipv4_hdr) + 8*sizeof(char= )))) >> +=09=09return -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? >> =09/* >> =09 * Ensure the IP payload length of all fragments is aligned to a >> =09 * multiple of 8 bytes as per RFC791 section 2.3. >> -- >> 2.25.1