>From: Ananyev, Konstantin
>Sent: Wednesday, October 4, 2017 3:49 PM
>To: Kavanagh, Mark B <mark.b.kavan...@intel.com>; dev@dpdk.org
>Cc: Hu, Jiayu <jiayu...@intel.com>; Tan, Jianfeng <jianfeng....@intel.com>;
>Yigit, Ferruh <ferruh.yi...@intel.com>; tho...@monjalon.net
>Subject: RE: [PATCH v6 2/6] gso: add TCP/IPv4 GSO support
>
>> >> int
>> >> rte_gso_segment(struct rte_mbuf *pkt,
>> >> @@ -41,12 +46,53 @@
>> >> struct rte_mbuf **pkts_out,
>> >> uint16_t nb_pkts_out)
>> >> {
>> >> + struct rte_mempool *direct_pool, *indirect_pool;
>> >> + struct rte_mbuf *pkt_seg;
>> >> + uint64_t ol_flags;
>> >> + uint16_t gso_size;
>> >> + uint8_t ipid_delta;
>> >> + int ret = 1;
>> >> +
>> >> if (pkt == NULL || pkts_out == NULL || gso_ctx == NULL ||
>> >> nb_pkts_out < 1)
>> >> return -EINVAL;
>> >>
>> >> - pkt->ol_flags &= (~PKT_TX_TCP_SEG);
>> >> - pkts_out[0] = pkt;
>> >> + if ((gso_ctx->gso_size >= pkt->pkt_len) || (gso_ctx->gso_types &
>> >> + DEV_TX_OFFLOAD_TCP_TSO) !=
>> >> + gso_ctx->gso_types) {
>> >> + pkt->ol_flags &= (~PKT_TX_TCP_SEG);
>> >> + pkts_out[0] = pkt;
>> >> + return 1;
>> >> + }
>> >> +
>> >> + 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_FIXED);
>> >> + ol_flags = pkt->ol_flags;
>> >> +
>> >> + if (IS_IPV4_TCP(pkt->ol_flags)) {
>> >> + pkt->ol_flags &= (~PKT_TX_TCP_SEG);
>> >> + ret = gso_tcp4_segment(pkt, gso_size, ipid_delta,
>> >> + direct_pool, indirect_pool,
>> >> + pkts_out, nb_pkts_out);
>> >> + } else {
>> >> + pkt->ol_flags &= (~PKT_TX_TCP_SEG);
>> >
>> >Not sure why do you clean this flag if you don't support that packet type
>> >and no action was perfomed?
>> >Suppose you have a mix ipv4 and ipv6 packets - gso lib would do ipv4 and
>> >someone else
>> >(HW?) can do ipv4 segmentation.
>>
>> I can't say for definite, since I didn't implement this change. However, I
>can only presume that the assumption here is that since
>> segmentation is being done in S/W that the underlying H/W does not support
>TSO.
>> Since the underlying HW can't segment the packet in HW, we should clear the
>flag; otherwise, if an mbuf marked for TCP segmentation is
>> passed to the driver of a NIC that does not support/understand that feature,
>the behavior is undefined.
>> Is this a fair assumption in your opinion, or is it the case that the packet
>would simply be transmitted un-segmented in that case, and so we
>> shouldn't clear the flag?
>
>Yes, I think if we shouldn't clear the flag if we didn't do any segmentation
>(we just encounter a packet type that we don't support).
>Konstantin
Okay, thanks for clarifying - I'll update the code accordingly.
-Mark
>
>>
>> Thanks again,
>> Mark
>>
>> >BTW, did you notice that building of shared target fails?
>> >Konstantin
>>
>> I didn't, but I'll take a look right now - thanks for the catch!
>>
>> >
>> >
>> >> + pkts_out[0] = pkt;
>> >> + RTE_LOG(WARNING, GSO, "Unsupported packet type\n");
>> >> + return 1;
>> >> + }
>> >> +
>> >> + if (ret > 1) {
>> >> + pkt_seg = pkt;
>> >> + while (pkt_seg) {
>> >> + rte_mbuf_refcnt_update(pkt_seg, -1);
>> >> + pkt_seg = pkt_seg->next;
>> >> + }
>> >> + } else if (ret < 0) {
>> >> + /* Revert the ol_flags in the event of failure. */
>> >> + pkt->ol_flags = ol_flags;
>> >> + }
>> >>
>> >> - return 1;
>> >> + return ret;
>> >> }
>> >> --
>> >> 1.9.3
