Hi Jiayu,
> -----Original Message-----
> From: Hu, Jiayu
> Sent: Monday, September 4, 2017 4:32 AM
> To: Ananyev, Konstantin <konstantin.anan...@intel.com>
> Cc: dev@dpdk.org; Kavanagh, Mark B <mark.b.kavan...@intel.com>; Tan, Jianfeng
> <jianfeng....@intel.com>
> Subject: Re: [PATCH 2/5] gso/lib: add TCP/IPv4 GSO support
>
> 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 <mark.b.kavan...@intel.com>; Ananyev, Konstantin
> > > <konstantin.anan...@intel.com>; Tan, Jianfeng
> > > <jianfeng....@intel.com>; Hu, Jiayu <jiayu...@intel.com>
> > > 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 <jiayu...@intel.com>
> > > Signed-off-by: Mark Kavanagh <mark.b.kavan...@intel.com>
> > > ---
> > > +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?
Do the same as Linux?
I.E. add some flag RRE_GSO_IPID_FIXED (or so) into gso_ctx?
Konstantin
>
> 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
