DPDK patches and discussions
 help / color / mirror / Atom feed
From: yang_y_yi  <yang_y_yi@163.com>
To: "Hu, Jiayu" <jiayu.hu@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"thomas@monjalon.net" <thomas@monjalon.net>,
	"yangyi01@inspur.com" <yangyi01@inspur.com>
Subject: Re: [dpdk-dev] [PATCH v6 2/3] gro: add VXLAN UDP/IPv4 GRO support
Date: Tue, 22 Sep 2020 14:23:39 +0800 (CST)
Message-ID: <75421977.3967.174b47b6259.Coremail.yang_y_yi@163.com> (raw)
In-Reply-To: <b494d69469d54db38bd63a4e5cb75c89@intel.com>

Not a question, in next flush, they will be flushed, we have to check timestamp in the first time unless we don't strictly follow this time limitation.

At 2020-09-22 14:14:00, "Hu, Jiayu" <jiayu.hu@intel.com> wrote:

Fragments of a flow are sorted by frag_oft, but they may have different

timestamp. For example, there are three fragments, whose frag_oft is:

frag[0].frag_oft=0, frag[1].frag_oft=4, frag[2].frag_oft=6; and they are

fragments of one UDP packet but are not neighbors. In the first RX burst,

host receives frag[1] and calls rte_gro_reassemble(), and we assume the

timestamp of frag[1] is 10; in the second RX burst, host receives frag[0]

and also call rte_gro_reassemble(), and timestamp of frag[0] is 11; the

third time, host receives frag[2] and timestamp of frag[2] is 12. The three

fragments are stored in three items of a UDP GRO table:

items[0]: frag[0], timestamp is 11

items[1]: frag[1], timestamp is 10

items[2]: frag[2], timestamp is 12

Now we want to flush packets whose timestamp is less than or equal to

10. frag[1] should be returned, but in your code, no packets will be flushed.

Because the timestamp of items[0] is greater than 10, the left two fragments

will not be checked. This is what I want to say.


From: yang_y_yi <yang_y_yi@163.com>
Sent: Tuesday, September 22, 2020 9:44 AM
To: Hu, Jiayu <jiayu.hu@intel.com>
Cc: dev@dpdk.org; thomas@monjalon.net; yangyi01@inspur.com
Subject: Re:Re: [dpdk-dev] [PATCH v6 2/3] gro: add VXLAN UDP/IPv4 GRO support
Importance: High


BTW, start_time is checked for the first packet in a flow, gro_udp4_merge_items(tbl, j) will merge all the packets in this flow once if they can be reassembled, gro_udp4_merge_items(tbl, j) doesn't check start_time, so this still can let some new items in this flow have chance to be merged.

At 2020-09-22 09:29:38, "yang_y_yi" <yang_y_yi@163.com> wrote:
>Thanks Jiayu, I have fixed other comments except this one:
>>The items of a flow are ordered by frag_oft, and start_time
>>of these items is not always in ascending order. Therefore,
>>you cannot skip checking the items after the item whose
>>start_time is greater than flush_timestamp. This issue also
>>exists in UDP/IPv4 GRO, and need to correct them both.
>I think the issue here is if we should strictly follow flush_timestamp, it is possible there are new items in items chain. we have chance to merge more packets if we don't follow flush_timestamp. So an ideal change can be this. But is it acceptible if we don't use flush_timestamp? It can flush some packets in advance therefore miss next merge window. Maybe current way is most resonable and a tradeoff between two exterem cases.
>diff --git a/lib/librte_gro/gro_udp4.c b/lib/librte_gro/gro_udp4.c
>index 061e7b0..ffa35a2 100644
>--- a/lib/librte_gro/gro_udp4.c
>+++ b/lib/librte_gro/gro_udp4.c
>@@ -391,7 +391,6 @@
>                j = tbl->flows[i].start_index;
>                while (j != INVALID_ARRAY_INDEX) {
>-                       if (tbl->items[j].start_time <= flush_timestamp) {
>                                gro_udp4_merge_items(tbl, j);
>                                out[k++] = tbl->items[j].firstseg;
>                                if (tbl->items[j].nb_merged > 1)
>@@ -407,12 +406,6 @@
>                                if (unlikely(k == nb_out))
>                                        return k;
>-                       } else
>-                               /*
>-                                * The left packets in this flow won't be
>-                                * timeout. Go to check other flows.
>-                                */
>-                               break;
>                }
>        }
>        return k;

  reply	other threads:[~2020-09-22  6:23 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-17  3:49 [dpdk-dev] [PATCH v6 0/3] gro: add UDP/IPv4 GRO and " yang_y_yi
2020-09-17  3:49 ` [dpdk-dev] [PATCH v6 1/3] gro: add " yang_y_yi
2020-09-21  6:21   ` Hu, Jiayu
2020-09-17  3:49 ` [dpdk-dev] [PATCH v6 2/3] gro: add VXLAN " yang_y_yi
2020-09-21  7:54   ` Hu, Jiayu
2020-09-22  1:29     ` yang_y_yi
2020-09-22  1:44       ` yang_y_yi
2020-09-22  6:14         ` Hu, Jiayu
2020-09-22  6:23           ` yang_y_yi [this message]
2020-09-22  6:55             ` Jiayu Hu
2020-09-22  7:38               ` yang_y_yi
2020-09-23  2:15                 ` Jiayu Hu
2020-09-23  2:28                   ` yang_y_yi
2020-09-23  2:43                     ` Jiayu Hu
2020-09-24  2:41                       ` yang_y_yi
2020-09-22  3:01       ` Jiayu Hu
2020-09-22  3:00         ` yang_y_yi
2020-09-17  3:49 ` [dpdk-dev] [PATCH v6 3/3] doc: update prog_guide and rel_notes for GRO yang_y_yi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=75421977.3967.174b47b6259.Coremail.yang_y_yi@163.com \
    --to=yang_y_yi@163.com \
    --cc=dev@dpdk.org \
    --cc=jiayu.hu@intel.com \
    --cc=thomas@monjalon.net \
    --cc=yangyi01@inspur.com \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git