From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f44.google.com (mail-wm0-f44.google.com [74.125.82.44]) by dpdk.org (Postfix) with ESMTP id 9E019530F for ; Thu, 15 Sep 2016 22:09:26 +0200 (CEST) Received: by mail-wm0-f44.google.com with SMTP id k186so3852187wmd.0 for ; Thu, 15 Sep 2016 13:09:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=ktzEKyipzgmSjBHz/eKb+jxS0787bV6JAuQjQ8fhvro=; b=MtZzd1qNiPd9iNZdefHMNjQotWreChfaBncfjTnrvkZDGKbKM/znT5oM1lVrX8sg0r 1yc647xMlgHIMa5py49Rtkpwgss3ek/qk69MWVTOxDf3rjYbmrVCPsWwXtzVdM4mVLDC fK6tzbcuK+mj7IJJca+O/1tpbVSIPjrCkg/fpbnGa8A73CNHHBMo7aSy4wAHiRqbUGL9 VK1RHYKszZSEvkTn7ue9QiuVoD/zzosPbRjE0vj4ki1IUsbC8oiL1UGt/koPFDYUvxMz VNIy8gYmaQgA/wujsH0lF9CBrpbSzd0wSze5Qld3eISBAbNgHFmk75nUbr6ODrHF0OEy hmaA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=ktzEKyipzgmSjBHz/eKb+jxS0787bV6JAuQjQ8fhvro=; b=lZSMLRS/Kid5aSO7vq/bk1f2HXGERUQvuXwa8ajK8xklXsIIlDv8Hke+3Esc9970dF BZ3p4GoeiZ/gI2ZJbpe1UoNhitNFb1oF38tDsZ9Tc6R5WqwIGww6lT2tGJgzGlvuMiH3 GB8U/V4unpSNJ++FrQpvug38o6AGCNBgQsQ/qxgYMRufBvpWAMJXT0VYIVgA6bXhkcBx CIchjlmVxiC1Gwm9eKSpvy9tzPGlZPXqVGC8A00GjC+O39QfJYiCZnsEUvXI19bftEva fDN/ybvr6APwWx6FSAoKoF4BrUZACfv4xbqcmxJX9LFpKO7utaxCVDfPdG95odgT8iad o1Og== X-Gm-Message-State: AE9vXwNvxvaS2EXQlZaEHh5NBEZ7gDND5rq0hAxc2AQVtmI4D3CYFjR5frsdNSnhv0agA4o53gLPV4UU2YxzuA== X-Received: by 10.194.66.41 with SMTP id c9mr10913237wjt.30.1473970166350; Thu, 15 Sep 2016 13:09:26 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.220.134 with HTTP; Thu, 15 Sep 2016 13:09:25 -0700 (PDT) In-Reply-To: <13658731.5PNXAK9RiV@xps13> References: <13658731.5PNXAK9RiV@xps13> From: =?UTF-8?B?0JDQu9C10LrRgdCw0L3QtNGAINCa0LjRgdC10LvQtdCy?= Date: Thu, 15 Sep 2016 23:09:25 +0300 Message-ID: To: Thomas Monjalon Cc: dev@dpdk.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] ipv4 fragmentation bug? 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, 15 Sep 2016 20:09:26 -0000 I am sorry for the late reply. I am not sure anymore about is it a bug I found or the author of rte_ipv4_fragment_packet() realy wanted to constraint the size of mtu writing lines: frag_size =3D (uint16_t)(mtu_size - sizeof(struct ipv4_hdr)); /* Fragment size should be a multiply of 8. */ assert((frag_size & IPV4_HDR_FO_MASK) =3D=3D 0); So, if we assume that any mtu size is valid then it's a bug and the function must be rewriten. Otherwise, since mtu_size is an input parameter of the function, validation should be stronger than just a assertion or it should be noted in the documentation that not all values for the paremater mtu_size are valid. I can write a patch. I just need a confirmation since I am not sure about the networking background regarding MTU. I tried to find anything about MTU in the RFC, but so far nothing. According to RFC all mtu sizes are valid. >2016-08-22 14:31 GMT+03:00 Thomas Monjalon : >Hi, > 2016-08-15 20:30 GMT+03:00 =D0=90=D0=BB=D0=B5=D0=BA=D1=81=D0=B0=D0=BD=D0= =B4=D1=80 =D0=9A=D0=B8=D1=81=D0=B5=D0=BB=D0=B5=D0=B2 : > > While playing with function rte_ipv4_fragment_packet I found that it > > incorrectly fragments packets. > > For example if the function takes 1200 bytes packet and mtu size 1000 i= t > > will produces two fragments. And when those fragments are reassembled b= ack > > the resulting packet will be 4 bytes shorter than it should be. > > > > I played with linux ping program and it reports that a reply is truncat= ed. > > 1204 bytes from 192.168.125.1: icmp_seq=3D1 ttl=3D64 (truncated) > > > > Looking at the source of rte_ipv4_fragment_packet I discovered the caus= e > > of the above behavior. > > > > Function makes the following assumption and the whole calculations are > > bases on that assumption. > > > > /* Fragment size should be a multiply of 8. */ > > IP_FRAG_ASSERT((frag_size & IPV4_HDR_FO_MASK) =3D=3D 0); > > > > The problem is that this assert doesn=E2=80=99t make any sense. It's tr= ue that > > fragment size should be a multiply of 8, but what this line real checks= is > > that > > the size of mtu minus 20 bytes should be multiply of 8. In other words > > it constrains the size of the mtu. So, if I take valid mtu value, say > > 1504, > > it will produce incorrect fragments when asserts are off. >Thanks for reporting. > >2016-08-15 20:48, =D0=90=D0=BB=D0=B5=D0=BA=D1=81=D0=B0=D0=BD=D0=B4=D1=80 = =D0=9A=D0=B8=D1=81=D0=B5=D0=BB=D0=B5=D0=B2: >> I'am sorry. Looks like having an mtu value multiply of 8 is a good pract= ice. >> >> But mtu value 1504 is also widely used in qinq linux interfaces. >Please, would like to write a patch for master branch? >Or do you prefer to delegate it to someone reading this thread? -- Alexander Kiselev