From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f171.google.com (mail-pf0-f171.google.com [209.85.192.171]) by dpdk.org (Postfix) with ESMTP id CC2E429CA for ; Thu, 7 Dec 2017 02:02:07 +0100 (CET) Received: by mail-pf0-f171.google.com with SMTP id j124so3379767pfc.2 for ; Wed, 06 Dec 2017 17:02:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Og4yp9zo4/KhgtroKwjEW9cMLkXqmGEc5EQOOPqf4fc=; b=ESZH7H0FoSv9ci+lVhtPCRflrxbiBcx6NFp8NNi8Mq1MB7JL4kTRk9337HreKXTAG+ st7wxvkJfyjw3d+jGD/ehbUqY4dkXP+Dhf4Ga1SIoxJWzqtzs/w1Kn76pFQcvigAryTG JutsKu4uOhXk9U9hF0FDtTtOxDV3B4Fsv4+NodIYPly8cyToK42Ebv+vIj+KtW6oMzjB kSaD3+eMP4f8kPXrAwqf+m2BNrWquGoMBJldsZvi1XSlPdZc8U6VECbr+9sQRxSBoNjB irOyKeJ6/IJlLq2gc6tIw8NPLYAtIs7kHpEP2h5CDizxdBwMWWGqrj6iTzlm+ew9scoP cq+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Og4yp9zo4/KhgtroKwjEW9cMLkXqmGEc5EQOOPqf4fc=; b=K4UW1k8tRK9/6WQfq7A7KuFezEpCazjL1n2pBa5wQ+sRCN/JjdQXJj71tT0G/OC5v3 L8gb0bxE8msXQjlKV7Tc8wXCUHQ0jGDjdNcSypf3jJ1JVziyhJpCyEkoxZfMdj5X+B5N n1cw3A1J0OYpmmDQwvoesQmtRrZfdAH7Ruuf+i6M7fku/4lemXy7f0R+TWT9KpX6BHB2 MCHN9whEOn4t5fCtZethGqbwOZAYUoNwQq3QGGxZznv4TNsdf+i1oeC0bbAXBgMIuefo SXHoS+uQ3Cx+TRL4a1wSfyC42VHIQEBUfoszJLUrSTZqPih1GXhRWEigfp5HICoYjZHT 29eQ== X-Gm-Message-State: AJaThX4przeHUmddeot1rEnKX6T/2Tc3oj1BTqwtqZzkx4zbOmbCOAML 9dUnn9V5TNUyW4zNv1bnCOBa8g== X-Google-Smtp-Source: AGs4zMZd6FXMQ2j0UsUUaFYKV3YqqisDZw32o28skBrvn3S9ghFVlZ6JvXmaEUd+OdYDaCZ+nkzdkg== X-Received: by 10.99.110.79 with SMTP id j76mr23589007pgc.246.1512608526863; Wed, 06 Dec 2017 17:02:06 -0800 (PST) Received: from xeon-e3 (76-14-207-240.or.wavecable.com. [76.14.207.240]) by smtp.gmail.com with ESMTPSA id b25sm6496211pfd.182.2017.12.06.17.02.05 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 06 Dec 2017 17:02:06 -0800 (PST) Date: Wed, 6 Dec 2017 17:01:57 -0800 From: Stephen Hemminger To: "Ananyev, Konstantin" Cc: Ilya Matveychikov , "dev@dpdk.org" , "Hu, Jiayu" Message-ID: <20171206170157.1d839de0@xeon-e3> In-Reply-To: <2601191342CEEE43887BDE71AB9772585FAC57C1@irsmsx105.ger.corp.intel.com> References: <4F9781B2-338C-4154-BDA1-BC24D1B2B689@gmail.com> <20171206101200.031afa39@shemminger-XPS-13-9360> <2111ED2C-DB90-4AE3-893E-2406EFE129AD@gmail.com> <20171206151532.3abaf2fb@xeon-e3> <2601191342CEEE43887BDE71AB9772585FAC57C1@irsmsx105.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] A question about GRO neighbor packet matching 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: , X-List-Received-Date: Thu, 07 Dec 2017 01:02:08 -0000 On Thu, 7 Dec 2017 00:19:46 +0000 "Ananyev, Konstantin" wrote: > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger > > Sent: Wednesday, December 6, 2017 11:16 PM > > To: Ilya Matveychikov > > Cc: dev@dpdk.org; Hu, Jiayu > > Subject: Re: [dpdk-dev] A question about GRO neighbor packet matching > > > > On Wed, 6 Dec 2017 22:38:12 +0400 > > Ilya Matveychikov wrote: > > > > > > On Dec 6, 2017, at 10:12 PM, Stephen Hemminger wrote: > > > > > > > > On Wed, 6 Dec 2017 18:02:21 +0400 > > > > Ilya Matveychikov wrote: > > > > > > > >> Hello all, > > > >> > > > >> > > > >> My question is about neighbor packet matching algorithm for TCP. Is it > > > >> correct to expect that IP packets should have continuous ID enumeration > > > >> (i.e. iph-next.id = iph-prev.id + 1)? > > > > > > > > > > > > No. > > > > > > > >> ~~~ > > > >> lib/librte_gro/gro_tcp4.c:check_seq_option() > > > >> ... > > > >> /* check if the two packets are neighbors */ > > > >> tcp_dl0 = pkt0->pkt_len - pkt0->l2_len - pkt0->l3_len - tcp_hl0; > > > >> if ((sent_seq == (item->sent_seq + tcp_dl0)) && > > > >> (ip_id == (item->ip_id + 1))) > > > >> /* append the new packet */ > > > >> return 1; > > > >> else if (((sent_seq + tcp_dl) == item->sent_seq) && > > > >> ((ip_id + item->nb_merged) == item->ip_id)) > > > >> /* pre-pend the new packet */ > > > >> return -1; > > > >> else > > > >> return 0; > > > >> ~~~ > > > >> > > > >> As per RFC791: > > > >> > > > >> Identification: 16 bits > > > >> > > > >> An identifying value assigned by the sender to aid in assembling the > > > >> fragments of a datagram. > > > > > > > > The IP header id is meaningless in most TCP sessions. > > > > Good TCP implementations use PMTU discovery which sets the Don't Fragment bit. > > > > With DF, the IP id is unused (since no fragmentation). > > > > Many implementations just send 0 since generating unique IP id requires an > > > > atomic operation which is potential bottleneck. > > > > > > So, is my question correct and the code is wrong? > > > > > > > Yes. This code is wrong on several areas. > > * The ip_id on TCP flows is irrelevant. > > * packet should only be merged if TCP flags are the same. > > > > > > The author should look at Linux net/ipv4/tcp_offload.c > > As I remember, linux GRO implementation *does* require that IP IDs > of the merging packets to be continuous. > > net/ipv4/af_inet.c: > static struct sk_buff **inet_gro_receive(struct sk_buff **head, > struct sk_buff *skb) > { > ... > id = ntohl(*(__be32 *)&iph->id); > flush = (u16)((ntohl(*(__be32 *)iph) ^ skb_gro_len(skb)) | (id & ~IP_DF)); > id >>= 16; > > ... > > NAPI_GRO_CB(p)->flush_id = > ((u16)(ntohs(iph2->id) + NAPI_GRO_CB(p)->count) ^ id); > NAPI_GRO_CB(p)->flush |= flush; > .... > > And then at net/ipv4/tcp_offload.c: > struct sk_buff **tcp_gro_receive(struct sk_buff **head, struct sk_buff *skb) > { > ... > /* Include the IP ID check below from the inner most IP hdr */ > flush = NAPI_GRO_CB(p)->flush | NAPI_GRO_CB(p)->flush_id; > ... > if (flush || skb_gro_receive(head, skb)) { > ... > > The reason why we do need to check that IP ID is continuous - > DPDK GRO library doesn't strip off IPv4 header, instead it has to merge them into one. > If IP ID would be non-contiguous it is unclear which one should be to used. > By same reason packets with different IP/TCP options are not allowed. > So in that case GRO lib makes a decision that it isn't safe to merge these packets. > As I understand linux does pretty much the same. > Konstantin You are right, but still not sure that Linux and DPDK are doing the same thing with reordered packets. Ok, went RFC hunting and the relevant one seems to be RFC 6864. It mandates unique id's for each datagram so TCP does send them.