From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 92F0F5A3E for ; Mon, 4 Sep 2017 05:29:17 +0200 (CEST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Sep 2017 20:29:16 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,473,1498546800"; d="scan'208";a="147150932" Received: from dpdk15.sh.intel.com ([10.67.111.77]) by fmsmga005.fm.intel.com with ESMTP; 03 Sep 2017 20:29:15 -0700 Date: Mon, 4 Sep 2017 11:31:50 +0800 From: Jiayu Hu To: "Ananyev, Konstantin" Cc: "dev@dpdk.org" , "Kavanagh, Mark B" , "Tan, Jianfeng" Message-ID: <20170904033150.GA9009@dpdk15.sh.intel.com> References: <1503584144-63181-1-git-send-email-jiayu.hu@intel.com> <1503584144-63181-3-git-send-email-jiayu.hu@intel.com> <2601191342CEEE43887BDE71AB9772584F23E097@IRSMSX103.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2601191342CEEE43887BDE71AB9772584F23E097@IRSMSX103.ger.corp.intel.com> User-Agent: Mutt/1.7.1 (2016-10-04) Subject: Re: [dpdk-dev] [PATCH 2/5] gso/lib: add TCP/IPv4 GSO support 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: Mon, 04 Sep 2017 03:29:18 -0000 Hi Konstantin, About the IP identifier, I check the linux codes and have some feedbacks inline. On Wed, Aug 30, 2017 at 09:38:33AM +0800, Ananyev, Konstantin wrote: > > > > -----Original Message----- > > From: Hu, Jiayu > > Sent: Thursday, August 24, 2017 3:16 PM > > To: dev@dpdk.org > > Cc: Kavanagh, Mark B ; Ananyev, Konstantin ; Tan, Jianfeng > > ; Hu, Jiayu > > Subject: [PATCH 2/5] gso/lib: add TCP/IPv4 GSO support > > > > This patch adds GSO support for TCP/IPv4 packets. Supported packets > > may include a single VLAN tag. TCP/IPv4 GSO assumes that all input > > packets have correct checksums, and doesn't update checksums for output > > packets (the responsibility for this lies with the application). > > Additionally, TCP/IPv4 GSO doesn't process IP fragmented packets. > > > > TCP/IPv4 GSO uses two chained MBUFs, one direct MBUF and one indrect > > MBUF, to organize an output packet. Note that we refer to these two > > chained MBUFs as a two-segment MBUF. The direct MBUF stores the packet > > header, while the indirect mbuf simply points to a location within the > > original packet's payload. Consequently, use of the GSO library requires > > multi-segment MBUF support in the TX functions of the NIC driver. > > > > If a packet is GSOed, TCP/IPv4 GSO reduces its MBUF refcnt by 1. As a > > result, when all of its GSOed segments are freed, the packet is freed > > automatically. > > > > Signed-off-by: Jiayu Hu > > Signed-off-by: Mark Kavanagh > > --- > > +void > > +gso_update_pkt_headers(struct rte_mbuf *pkt, uint16_t nb_segments, > > + struct rte_mbuf **out_segments) > > +{ > > + struct ipv4_hdr *ipv4_hdr; > > + struct tcp_hdr *tcp_hdr; > > + struct rte_mbuf *seg; > > + uint32_t sent_seq; > > + uint16_t offset, i; > > + uint16_t tail_seg_idx = nb_segments - 1, id; > > + > > + switch (pkt->packet_type) { > > + case ETHER_VLAN_IPv4_TCP_PKT: > > + case ETHER_IPv4_TCP_PKT: > > Might be worth to put code below in a separate function: > update_inner_tcp_hdr(..) or so. > Then you can reuse it for tunneled cases too. > > > + ipv4_hdr = (struct ipv4_hdr *)(rte_pktmbuf_mtod(pkt, char *) > > + pkt->l2_len); > > + tcp_hdr = (struct tcp_hdr *)((char *)ipv4_hdr + pkt->l3_len); > > + id = rte_be_to_cpu_16(ipv4_hdr->packet_id); > > + sent_seq = rte_be_to_cpu_32(tcp_hdr->sent_seq); > > + > > + for (i = 0; i < nb_segments; i++) { > > + seg = out_segments[i]; > > + > > + offset = seg->l2_len; > > + update_ipv4_header(rte_pktmbuf_mtod(seg, char *), > > + offset, seg->pkt_len, id); > > + id++; > > Who would be responsible to make sure that we wouldn't have consecutive packets with the IPV4 id? > Would be the upper layer that forms the packet or gso library or ...? Linux supports two kinds of IP identifier: fixed identifier and incremental identifier, and which one to use depends on upper protocol modules. Specifically, if the protocol module wants fixed identifiers, it will set SKB_GSO_TCP_FIXEDID to skb->gso_type, and then inet_gso_segment() will keep identifiers the same. Otherwise, all segments will have incremental identifiers. The reason for this design is that some protocols may choose fixed IP identifiers, like TCP (from RFC791). This design also shows that linux ignores the issue of repeated IP identifiers. >>From the perspective of DPDK, we need to solve two problems. One is if ignore the issue of repeated IP identifiers. The other is if the GSO library provides an interface to upper applications to enable them to choose fixed or incremental identifiers, or simply uses incremental IP identifiers. Do you have any suggestions? Thanks, Jiayu > > > + > > + offset += seg->l3_len; > > + update_tcp_header(rte_pktmbuf_mtod(seg, char *), > > + offset, sent_seq, i < tail_seg_idx); > > + sent_seq += seg->next->data_len; > > + } > > + break; > > + } > > +} > > -- > > 2.7.4