Hi Konstantin,
On Tue, Sep 12, 2017 at 10:17:27PM +0800, Ananyev, Konstantin wrote:
>
>
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Tuesday, September 12, 2017 12:18 PM
> > To: Hu, Jiayu <jiayu...@intel.com>; dev@dpdk.org
> > Cc: Kavanagh, Mark B <mark.b.kavan...@intel.com>; Tan, Jianfeng
> > <jianfeng....@intel.com>
> > Subject: RE: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support
> >
> > > result, when all of its GSOed segments are freed, the packet is freed
> > > automatically.
> > > diff --git a/lib/librte_gso/rte_gso.c b/lib/librte_gso/rte_gso.c
> > > index dda50ee..95f6ea6 100644
> > > --- a/lib/librte_gso/rte_gso.c
> > > +++ b/lib/librte_gso/rte_gso.c
> > > @@ -33,18 +33,53 @@
> > >
> > > #include <errno.h>
> > >
> > > +#include <rte_log.h>
> > > +
> > > #include "rte_gso.h"
> > > +#include "gso_common.h"
> > > +#include "gso_tcp4.h"
> > >
> > > int
> > > rte_gso_segment(struct rte_mbuf *pkt,
> > > - struct rte_gso_ctx gso_ctx __rte_unused,
> > > + struct rte_gso_ctx gso_ctx,
> > > struct rte_mbuf **pkts_out,
> > > uint16_t nb_pkts_out)
> > > {
> > > + struct rte_mempool *direct_pool, *indirect_pool;
> > > + struct rte_mbuf *pkt_seg;
> > > + uint16_t gso_size;
> > > + uint8_t ipid_delta;
> > > + int ret = 1;
> > > +
> > > if (pkt == NULL || pkts_out == NULL || nb_pkts_out < 1)
> > > return -EINVAL;
> > >
> > > - pkts_out[0] = pkt;
> > > + if (gso_ctx.gso_size >= pkt->pkt_len ||
> > > + (pkt->packet_type & gso_ctx.gso_types) !=
> > > + pkt->packet_type) {
> > > + pkts_out[0] = pkt;
> > > + return ret;
> > > + }
> > > +
> > > + direct_pool = gso_ctx.direct_pool;
> > > + indirect_pool = gso_ctx.indirect_pool;
> > > + gso_size = gso_ctx.gso_size;
> > > + ipid_delta = gso_ctx.ipid_flag == RTE_GSO_IPID_INCREASE;
> > > +
> > > + if (is_ipv4_tcp(pkt->packet_type)) {
> >
> > Probably we need here:
> > If (is_ipv4_tcp(pkt->packet_type) && (gso_ctx->gso_types &
> > DEV_TX_OFFLOAD_TCP_TSO) != 0) {...
>
> Sorry, actually it probably should be:
> If (pkt->ol_flags & (PKT_TX_TCP_SEG | PKT_TX_IPV4) == PKT_TX_IPV4 &&
> (gso_ctx->gso_types & DEV_TX_OFFLOAD_TCP_TSO) != 0) {...
I don't quite understand why the GSO library should be aware if the TSO
flag is set or not. Applications can query device TSO capability before
they call the GSO library. Do I misundertsand anything?
Additionally, we don't need to check if the packet is a TCP/IPv4 packet here?
Thanks,
Jiayu
>
> Konstantin
>
> >
> > > + ret = gso_tcp4_segment(pkt, gso_size, ipid_delta,
> > > + direct_pool, indirect_pool,
> > > + pkts_out, nb_pkts_out);
> > > + } else
> > > + RTE_LOG(WARNING, GSO, "Unsupported packet type\n");
> >
> > Shouldn't we do pkt_out[0] = pkt; here?
> >
> > > +
> > > + if (ret > 1) {
> > > + pkt_seg = pkt;
> > > + while (pkt_seg) {
> > > + rte_mbuf_refcnt_update(pkt_seg, -1);
> > > + pkt_seg = pkt_seg->next;
> > > + }
> > > + }
> > >
> > > - return 1;
> > > + return ret;
> > > }
> > > --
> > > 2.7.4
