DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Hu, Jiayu" <jiayu.hu@intel.com>
To: yang_y_yi <yang_y_yi@163.com>
Cc: "thomas@monjalon.net" <thomas@monjalon.net>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"yangyi01@inspur.com" <yangyi01@inspur.com>
Subject: Re: [dpdk-dev] [PATCH v4 1/2] gro: add UDP GRO support
Date: Thu, 10 Sep 2020 02:47:43 +0000
Message-ID: <9dfa31fd84d847ccbd459e1b54eb32a5@intel.com> (raw)
In-Reply-To: <42e99430.5aa5.17472180a8e.Coremail.yang_y_yi@163.com>

No more comments on this patch.

Thanks,
Jiayu

From: yang_y_yi <yang_y_yi@163.com>
Sent: Wednesday, September 9, 2020 5:00 PM
To: Hu, Jiayu <jiayu.hu@intel.com>
Cc: thomas@monjalon.net; dev@dpdk.org; yangyi01@inspur.com
Subject: Re:Re: [dpdk-dev] [PATCH v4 1/2] gro: add UDP GRO support
Importance: High

This is incremental patch to fix your new comments, I'll send out v5 if no more comments here.

diff --git a/lib/librte_gro/gro_udp4.c b/lib/librte_gro/gro_udp4.c

index 25584fd..061e7b0 100644
--- a/lib/librte_gro/gro_udp4.c
+++ b/lib/librte_gro/gro_udp4.c
@@ -107,7 +107,7 @@
        uint32_t item_idx;

        item_idx = find_an_empty_item(tbl);
-       if (item_idx == INVALID_ARRAY_INDEX)
+       if (unlikely(item_idx == INVALID_ARRAY_INDEX))
                return INVALID_ARRAY_INDEX;

        tbl->items[item_idx].firstseg = pkt;
@@ -163,7 +163,6 @@
        dst->ip_src_addr = src->ip_src_addr;
        dst->ip_dst_addr = src->ip_dst_addr;
        dst->ip_id = src->ip_id;
-       dst->ip_id = src->ip_id;

        tbl->flows[flow_idx].start_index = item_idx;
        tbl->flow_num++;
@@ -226,10 +225,14 @@
         * Don't process the packet whose payload length is less than or
         * equal to 0.
         */
-       if (pkt->pkt_len - hdr_len <= 0)
+       if (pkt->pkt_len <= hdr_len)
+               return -1;
+
+       ip_dl = rte_be_to_cpu_16(ipv4_hdr->total_length);
+       if (ip_dl <= pkt->l3_len)
                return -1;

-       ip_dl = rte_be_to_cpu_16(ipv4_hdr->total_length) - pkt->l3_len;
+       ip_dl -= pkt->l3_len;
        ip_id = rte_be_to_cpu_16(ipv4_hdr->packet_id);
        frag_offset = rte_be_to_cpu_16(ipv4_hdr->fragment_offset);
        is_last_frag = ((frag_offset & RTE_IPV4_HDR_MF_FLAG) == 0) ? 1 : 0;
@@ -263,7 +266,7 @@
                item_idx = insert_new_item(tbl, pkt, start_time,
                                INVALID_ARRAY_INDEX, frag_offset,
                                is_last_frag);
-               if (item_idx == INVALID_ARRAY_INDEX)
+               if (unlikely(item_idx == INVALID_ARRAY_INDEX))
                        return -1;
                if (insert_new_flow(tbl, &key, item_idx) ==
                                INVALID_ARRAY_INDEX) {
@@ -284,7 +287,7 @@
        cur_idx = tbl->flows[i].start_index;
        prev_idx = cur_idx;
        do {
-               cmp = udp_check_neighbor(&(tbl->items[cur_idx]),
+               cmp = udp4_check_neighbor(&(tbl->items[cur_idx]),
                                frag_offset, ip_dl, 0);
                if (cmp) {
                        if (merge_two_udp4_packets(&(tbl->items[cur_idx]),
@@ -319,7 +322,7 @@
                item_idx = insert_new_item(tbl, pkt, start_time,
                                INVALID_ARRAY_INDEX, frag_offset,
                                is_last_frag);
-               if (item_idx == INVALID_ARRAY_INDEX)
+               if (unlikely(item_idx == INVALID_ARRAY_INDEX))
                        return -1;
                tbl->items[item_idx].next_pkt_idx = cur_idx;
                tbl->flows[i].start_index = item_idx;
@@ -348,12 +351,11 @@
        item_idx = tbl->items[start_idx].next_pkt_idx;
        while (item_idx != INVALID_ARRAY_INDEX) {
                pkt = tbl->items[item_idx].firstseg;
-               hdr_len = pkt->outer_l2_len + pkt->outer_l3_len + pkt->l2_len +
-                       pkt->l3_len;
+               hdr_len = pkt->l2_len + pkt->l3_len;
                ip_dl = pkt->pkt_len - hdr_len;
                frag_offset = tbl->items[item_idx].frag_offset;
                is_last_frag = tbl->items[item_idx].is_last_frag;
-               cmp = udp_check_neighbor(&(tbl->items[start_idx]),
+               cmp = udp4_check_neighbor(&(tbl->items[start_idx]),
                                        frag_offset, ip_dl, 0);
                if (cmp) {
                        if (merge_two_udp4_packets(
@@ -364,12 +366,10 @@
                                                        INVALID_ARRAY_INDEX);
                                tbl->items[start_idx].next_pkt_idx
                                        = item_idx;
-                       } else {
+                       } else
                                return 0;
-                       }
-               } else {
+               } else
                        return 0;
-               }
        }

        return 0;
diff --git a/lib/librte_gro/gro_udp4.h b/lib/librte_gro/gro_udp4.h
index bc67eb1..0a078e4 100644
--- a/lib/librte_gro/gro_udp4.h
+++ b/lib/librte_gro/gro_udp4.h
@@ -246,7 +246,7 @@ uint16_t gro_udp4_tbl_timeout_flush(struct gro_udp4_tbl *tbl,
  * Check if two UDP/IPv4 packets are neighbors.
  */
 static inline int
-udp_check_neighbor(struct gro_udp4_item *item,
+udp4_check_neighbor(struct gro_udp4_item *item,
                uint16_t frag_offset,
                uint16_t ip_dl,
                uint16_t l2_offset)











At 2020-09-09 15:25:10, "yang_y_yi" <yang_y_yi@163.com<mailto:yang_y_yi@163.com>> wrote:

>

>

>At 2020-09-08 10:05:16, "Jiayu Hu" <jiayu.hu@intel.com<mailto:jiayu.hu@intel.com>> wrote:

>>Some comments are inline.

>

>Thanks, replies inline.

>

>>

>>Thanks,

>>Jiayu

>>

>>On Fri, Sep 04, 2020 at 04:37:39PM +0800, yang_y_yi@163.com<mailto:yang_y_yi@163.com> wrote:

>>> From: Yi Yang <yangyi01@inspur.com<mailto:yangyi01@inspur.com>>

>>>

>>> UDP GRO can help improve VM-to-VM UDP performance when

>>> VM is enabled UFO or GSO, GRO must be supported if GSO

>>> or UFO is enabled, otherwise, performance gain will be

>>> hurt.

>>>

>>> With this enabled in DPDK, OVS DPDK can leverage it

>>> to improve VM-to-VM UDP performance, this will make

>>> sure IP fragments will be reassembled once it is

>>> received from physical NIC. It is very helpful in OVS

>>> DPDK VLAN TSO case.

>>>

>>> Signed-off-by: Yi Yang <yangyi01@inspur.com<mailto:yangyi01@inspur.com>>

>>> ---

>>>  lib/librte_gro/Makefile    |   1 +

>>>  lib/librte_gro/gro_udp4.c  | 430 +++++++++++++++++++++++++++++++++++++++++++++

>>>  lib/librte_gro/gro_udp4.h  | 281 +++++++++++++++++++++++++++++

>>>  lib/librte_gro/meson.build |   2 +-

>>>  lib/librte_gro/rte_gro.c   |  93 ++++++++--

>>>  lib/librte_gro/rte_gro.h   |   5 +-

>>>  6 files changed, 796 insertions(+), 16 deletions(-)

>>>  create mode 100644 lib/librte_gro/gro_udp4.c

>>>  create mode 100644 lib/librte_gro/gro_udp4.h

>>>

>>> diff --git a/lib/librte_gro/Makefile b/lib/librte_gro/Makefile

>>> index e848687..41ec29e 100644

>>> --- a/lib/librte_gro/Makefile

>>> +++ b/lib/librte_gro/Makefile

>>> @@ -15,6 +15,7 @@ EXPORT_MAP := rte_gro_version.map

>>>  # source files

>>>  SRCS-$(CONFIG_RTE_LIBRTE_GRO) += rte_gro.c

>>>  SRCS-$(CONFIG_RTE_LIBRTE_GRO) += gro_tcp4.c

>>> +SRCS-$(CONFIG_RTE_LIBRTE_GRO) += gro_udp4.c

>>>  SRCS-$(CONFIG_RTE_LIBRTE_GRO) += gro_vxlan_tcp4.c

>>>

>>>  # install this header file

>>> diff --git a/lib/librte_gro/gro_udp4.c b/lib/librte_gro/gro_udp4.c

>>> new file mode 100644

>>> index 0000000..25584fd

>>> --- /dev/null

>>> +++ b/lib/librte_gro/gro_udp4.c

>>> @@ -0,0 +1,430 @@

>>> +/* SPDX-License-Identifier: BSD-3-Clause

>>> + * Copyright(c) 2020 Inspur Corporation

>>> + */

>>> +

>>> +#include <rte_malloc.h>

>>> +#include <rte_mbuf.h>

>>> +#include <rte_cycles.h>

>>> +#include <rte_ethdev.h>

>>> +

>>> +#include "gro_udp4.h"

>>> +

>>> +void *

>>> +gro_udp4_tbl_create(uint16_t socket_id,

>>> +         uint16_t max_flow_num,

>>> +         uint16_t max_item_per_flow)

>>> +{

>>> +  struct gro_udp4_tbl *tbl;

>>> +  size_t size;

>>> +  uint32_t entries_num, i;

>>> +

>>> +  entries_num = max_flow_num * max_item_per_flow;

>>> +  entries_num = RTE_MIN(entries_num, GRO_UDP4_TBL_MAX_ITEM_NUM);

>>> +

>>> +  if (entries_num == 0)

>>> +         return NULL;

>>> +

>>> +  tbl = rte_zmalloc_socket(__func__,

>>> +                 sizeof(struct gro_udp4_tbl),

>>> +                 RTE_CACHE_LINE_SIZE,

>>> +                 socket_id);

>>> +  if (tbl == NULL)

>>> +         return NULL;

>>> +

>>> +  size = sizeof(struct gro_udp4_item) * entries_num;

>>> +  tbl->items = rte_zmalloc_socket(__func__,

>>> +                 size,

>>> +                 RTE_CACHE_LINE_SIZE,

>>> +                 socket_id);

>>> +  if (tbl->items == NULL) {

>>> +         rte_free(tbl);

>>> +         return NULL;

>>> +  }

>>> +  tbl->max_item_num = entries_num;

>>> +

>>> +  size = sizeof(struct gro_udp4_flow) * entries_num;

>>> +  tbl->flows = rte_zmalloc_socket(__func__,

>>> +                 size,

>>> +                 RTE_CACHE_LINE_SIZE,

>>> +                 socket_id);

>>> +  if (tbl->flows == NULL) {

>>> +         rte_free(tbl->items);

>>> +         rte_free(tbl);

>>> +         return NULL;

>>> +  }

>>> +  /* INVALID_ARRAY_INDEX indicates an empty flow */

>>> +  for (i = 0; i < entries_num; i++)

>>> +         tbl->flows[i].start_index = INVALID_ARRAY_INDEX;

>>> +  tbl->max_flow_num = entries_num;

>>> +

>>> +  return tbl;

>>> +}

>>> +

>>> +void

>>> +gro_udp4_tbl_destroy(void *tbl)

>>> +{

>>> +  struct gro_udp4_tbl *udp_tbl = tbl;

>>> +

>>> +  if (udp_tbl) {

>>> +         rte_free(udp_tbl->items);

>>> +         rte_free(udp_tbl->flows);

>>> +  }

>>> +  rte_free(udp_tbl);

>>> +}

>>> +

>>> +static inline uint32_t

>>> +find_an_empty_item(struct gro_udp4_tbl *tbl)

>>> +{

>>> +  uint32_t i;

>>> +  uint32_t max_item_num = tbl->max_item_num;

>>> +

>>> +  for (i = 0; i < max_item_num; i++)

>>> +         if (tbl->items[i].firstseg == NULL)

>>> +                 return i;

>>> +  return INVALID_ARRAY_INDEX;

>>> +}

>>> +

>>> +static inline uint32_t

>>> +find_an_empty_flow(struct gro_udp4_tbl *tbl)

>>> +{

>>> +  uint32_t i;

>>> +  uint32_t max_flow_num = tbl->max_flow_num;

>>> +

>>> +  for (i = 0; i < max_flow_num; i++)

>>> +         if (tbl->flows[i].start_index == INVALID_ARRAY_INDEX)

>>> +                 return i;

>>> +  return INVALID_ARRAY_INDEX;

>>> +}

>>> +

>>> +static inline uint32_t

>>> +insert_new_item(struct gro_udp4_tbl *tbl,

>>> +         struct rte_mbuf *pkt,

>>> +         uint64_t start_time,

>>> +         uint32_t prev_idx,

>>> +         uint16_t frag_offset,

>>> +         uint8_t is_last_frag)

>>> +{

>>> +  uint32_t item_idx;

>>> +

>>> +  item_idx = find_an_empty_item(tbl);

>>> +  if (item_idx == INVALID_ARRAY_INDEX)

>>> +         return INVALID_ARRAY_INDEX;

>>> +

>>> +  tbl->items[item_idx].firstseg = pkt;

>>> +  tbl->items[item_idx].lastseg = rte_pktmbuf_lastseg(pkt);

>>> +  tbl->items[item_idx].start_time = start_time;

>>> +  tbl->items[item_idx].next_pkt_idx = INVALID_ARRAY_INDEX;

>>> +  tbl->items[item_idx].frag_offset = frag_offset;

>>> +  tbl->items[item_idx].is_last_frag = is_last_frag;

>>> +  tbl->items[item_idx].nb_merged = 1;

>>> +  tbl->item_num++;

>>> +

>>> +  /* if the previous packet exists, chain them together. */

>>> +  if (prev_idx != INVALID_ARRAY_INDEX) {

>>> +         tbl->items[item_idx].next_pkt_idx =

>>> +                tbl->items[prev_idx].next_pkt_idx;

>>> +         tbl->items[prev_idx].next_pkt_idx = item_idx;

>>> +  }

>>> +

>>> +  return item_idx;

>>> +}

>>> +

>>> +static inline uint32_t

>>> +delete_item(struct gro_udp4_tbl *tbl, uint32_t item_idx,

>>> +         uint32_t prev_item_idx)

>>> +{

>>> +  uint32_t next_idx = tbl->items[item_idx].next_pkt_idx;

>>> +

>>> +  /* NULL indicates an empty item */

>>> +  tbl->items[item_idx].firstseg = NULL;

>>> +  tbl->item_num--;

>>> +  if (prev_item_idx != INVALID_ARRAY_INDEX)

>>> +         tbl->items[prev_item_idx].next_pkt_idx = next_idx;

>>> +

>>> +  return next_idx;

>>> +}

>>> +

>>> +static inline uint32_t

>>> +insert_new_flow(struct gro_udp4_tbl *tbl,

>>> +         struct udp4_flow_key *src,

>>> +         uint32_t item_idx)

>>> +{

>>> +  struct udp4_flow_key *dst;

>>> +  uint32_t flow_idx;

>>> +

>>> +  flow_idx = find_an_empty_flow(tbl);

>>> +  if (unlikely(flow_idx == INVALID_ARRAY_INDEX))

>>> +         return INVALID_ARRAY_INDEX;

>>> +

>>> +  dst = &(tbl->flows[flow_idx].key);

>>> +

>>> +  rte_ether_addr_copy(&(src->eth_saddr), &(dst->eth_saddr));

>>> +  rte_ether_addr_copy(&(src->eth_daddr), &(dst->eth_daddr));

>>> +  dst->ip_src_addr = src->ip_src_addr;

>>> +  dst->ip_dst_addr = src->ip_dst_addr;

>>> +  dst->ip_id = src->ip_id;

>>> +  dst->ip_id = src->ip_id;

>>

>>Duplicated code.

>

>Thanks, good catch, will remove it.

>

>>

>>> +

>>> +  tbl->flows[flow_idx].start_index = item_idx;

>>> +  tbl->flow_num++;

>>> +

>>> +  return flow_idx;

>>> +}

>>> +

>>> +/*

>>> + * update the packet length for the flushed packet.

>>> + */

>>> +static inline void

>>> +update_header(struct gro_udp4_item *item)

>>> +{

>>> +  struct rte_ipv4_hdr *ipv4_hdr;

>>> +  struct rte_mbuf *pkt = item->firstseg;

>>> +  uint16_t frag_offset;

>>> +

>>> +  ipv4_hdr = (struct rte_ipv4_hdr *)(rte_pktmbuf_mtod(pkt, char *) +

>>> +                 pkt->l2_len);

>>> +  ipv4_hdr->total_length = rte_cpu_to_be_16(pkt->pkt_len -

>>> +                 pkt->l2_len);

>>> +

>>> +  /* Clear MF bit if it is last fragment */

>>> +  if (item->is_last_frag) {

>>> +         frag_offset = rte_be_to_cpu_16(ipv4_hdr->fragment_offset);

>>> +         ipv4_hdr->fragment_offset =

>>> +                 rte_cpu_to_be_16(frag_offset & ~RTE_IPV4_HDR_MF_FLAG);

>>> +  }

>>> +}

>>> +

>>> +int32_t

>>> +gro_udp4_reassemble(struct rte_mbuf *pkt,

>>> +         struct gro_udp4_tbl *tbl,

>>> +         uint64_t start_time)

>>> +{

>>> +  struct rte_ether_hdr *eth_hdr;

>>> +  struct rte_ipv4_hdr *ipv4_hdr;

>>> +  uint16_t ip_dl;

>>> +  uint16_t ip_id, hdr_len;

>>> +  uint16_t frag_offset = 0;

>>> +  uint8_t is_last_frag;

>>> +

>>> +  struct udp4_flow_key key;

>>> +  uint32_t cur_idx, prev_idx, item_idx;

>>> +  uint32_t i, max_flow_num, remaining_flow_num;

>>> +  int cmp;

>>> +  uint8_t find;

>>> +

>>> +  eth_hdr = rte_pktmbuf_mtod(pkt, struct rte_ether_hdr *);

>>> +  ipv4_hdr = (struct rte_ipv4_hdr *)((char *)eth_hdr + pkt->l2_len);

>>> +  hdr_len = pkt->l2_len + pkt->l3_len;

>>> +

>>> +  /*

>>> +   * Don't process non-fragment packet.

>>> +   */

>>> +  if (!is_ipv4_fragment(ipv4_hdr))

>>> +         return -1;

>>> +

>>> +  /*

>>> +   * Don't process the packet whose payload length is less than or

>>> +   * equal to 0.

>>> +   */

>>> +  if (pkt->pkt_len - hdr_len <= 0)

>>> +         return -1;

>>

>>If input packets are malicious, whose mbuf->pkt_len is smaller than

>>(mbuf->l2_len+mbuf->l3_len), the above check wotn't work correctly,

>>as its value is uint16_t, which is always positive.

>

>I tried the below code.

>

>$ cat a.c

>#include

>#include

>

>int main()

>{

>    uint16_t a = 10;

>    uint16_t b = 8;

>

>    if ((b - a) < 0) {

>        printf("%u < %u\n", b , a);

>    }

>}

>$ gcc a.c

>$ ./a.out

>8 < 10

>$

>

>It works, so I think it is ok, nervertheless, I'll change it to the below to avoid your concern.

>

>if (pkt->pkt_len <= hdr_len)

>

>>

>>> +

>>> +  ip_dl = rte_be_to_cpu_16(ipv4_hdr->total_length) - pkt->l3_len;

>>

>>Same as above.

>

>I think "if (pkt->pkt_len <= hdr_len)" can ensure ip_dl will be positive, I can add one more

>condition check here if you want

>

>if (rte_be_to_cpu_16(ipv4_hdr->total_length) <= l3_len)

>        return -1;

>

>But I think it is unnecessary here for a little bit performance as you're caring.

>

>>

>>> +  ip_id = rte_be_to_cpu_16(ipv4_hdr->packet_id);

>>> +  frag_offset = rte_be_to_cpu_16(ipv4_hdr->fragment_offset);

>>> +  is_last_frag = ((frag_offset & RTE_IPV4_HDR_MF_FLAG) == 0) ? 1 : 0;

>>> +  frag_offset = (uint16_t)(frag_offset & RTE_IPV4_HDR_OFFSET_MASK) << 3;

>>> +

>>> +  rte_ether_addr_copy(&(eth_hdr->s_addr), &(key.eth_saddr));

>>> +  rte_ether_addr_copy(&(eth_hdr->d_addr), &(key.eth_daddr));

>>> +  key.ip_src_addr = ipv4_hdr->src_addr;

>>> +  key.ip_dst_addr = ipv4_hdr->dst_addr;

>>> +  key.ip_id = ip_id;

>>> +

>>> +  /* Search for a matched flow. */

>>> +  max_flow_num = tbl->max_flow_num;

>>> +  remaining_flow_num = tbl->flow_num;

>>> +  find = 0;

>>> +  for (i = 0; i < max_flow_num && remaining_flow_num; i++) {

>>> +         if (tbl->flows[i].start_index != INVALID_ARRAY_INDEX) {

>>> +                 if (is_same_udp4_flow(tbl->flows[i].key, key)) {

>>> +                         find = 1;

>>> +                         break;

>>> +                 }

>>> +                 remaining_flow_num--;

>>> +         }

>>> +  }

>>> +

>>> +  /*

>>> +   * Fail to find a matched flow. Insert a new flow and store the

>>> +   * packet into the flow.

>>> +   */

>>> +  if (find == 0) {

>>> +         item_idx = insert_new_item(tbl, pkt, start_time,

>>> +                         INVALID_ARRAY_INDEX, frag_offset,

>>> +                         is_last_frag);

>>> +         if (item_idx == INVALID_ARRAY_INDEX)

>>> +                 return -1;

>>

>>Adding unlikely here could get better performance, IMO.

>

>No problem.

>

>>

>>> +         if (insert_new_flow(tbl, &key, item_idx) ==

>>> +                         INVALID_ARRAY_INDEX) {

>>> +                 /*

>>> +                  * Fail to insert a new flow, so delete the

>>> +                  * stored packet.

>>> +                  */

>>> +                 delete_item(tbl, item_idx, INVALID_ARRAY_INDEX);

>>> +                 return -1;

>>> +         }

>>> +         return 0;

>>> +  }

>>> +

>>> +  /*

>>> +   * Check all packets in the flow and try to find a neighbor for

>>> +   * the input packet.

>>> +   */

>>> +  cur_idx = tbl->flows[i].start_index;

>>> +  prev_idx = cur_idx;

>>> +  do {

>>> +         cmp = udp_check_neighbor(&(tbl->items[cur_idx]),

>>> +                         frag_offset, ip_dl, 0);

>>> +         if (cmp) {

>>> +                 if (merge_two_udp4_packets(&(tbl->items[cur_idx]),

>>> +                                        pkt, cmp, frag_offset,

>>> +                                       is_last_frag, 0))

>>> +                         return 1;

>>> +                 /*

>>> +                  * Fail to merge the two packets, as the packet

>>> +                  * length is greater than the max value. Store

>>> +                  * the packet into the flow.

>>> +                  */

>>> +                 if (insert_new_item(tbl, pkt, start_time, prev_idx,

>>> +                                       frag_offset, is_last_frag) ==

>>> +                                INVALID_ARRAY_INDEX)

>>> +                         return -1;

>>> +                 return 0;

>>> +         }

>>> +

>>> +         /* Ensure inserted items are ordered by frag_offset */

>>> +         if (frag_offset

>>> +                 < tbl->items[cur_idx].frag_offset) {

>>> +                 break;

>>> +         }

>>> +

>>> +         prev_idx = cur_idx;

>>> +         cur_idx = tbl->items[cur_idx].next_pkt_idx;

>>> +  } while (cur_idx != INVALID_ARRAY_INDEX);

>>> +

>>> +  /* Fail to find a neighbor, so store the packet into the flow. */

>>> +  if (cur_idx == tbl->flows[i].start_index) {

>>> +         /* Insert it before the first packet of the flow */

>>> +         item_idx = insert_new_item(tbl, pkt, start_time,

>>> +                         INVALID_ARRAY_INDEX, frag_offset,

>>> +                         is_last_frag);

>>> +         if (item_idx == INVALID_ARRAY_INDEX)

>>> +                 return -1;

>>> +         tbl->items[item_idx].next_pkt_idx = cur_idx;

>>> +         tbl->flows[i].start_index = item_idx;

>>> +  } else {

>>> +         if (insert_new_item(tbl, pkt, start_time, prev_idx,

>>> +                         frag_offset, is_last_frag)

>>> +                 == INVALID_ARRAY_INDEX)

>>> +                 return -1;

>>> +  }

>>> +

>>> +  return 0;

>>> +}

>>> +

>>> +static int

>>> +gro_udp4_merge_items(struct gro_udp4_tbl *tbl,

>>> +                    uint32_t start_idx)

>>> +{

>>> +  uint16_t frag_offset;

>>> +  uint8_t is_last_frag;

>>> +  int16_t ip_dl;

>>> +  struct rte_mbuf *pkt;

>>> +  int cmp;

>>> +  uint32_t item_idx;

>>> +  uint16_t hdr_len;

>>> +

>>> +  item_idx = tbl->items[start_idx].next_pkt_idx;

>>> +  while (item_idx != INVALID_ARRAY_INDEX) {

>>> +         pkt = tbl->items[item_idx].firstseg;

>>> +         hdr_len = pkt->outer_l2_len + pkt->outer_l3_len + pkt->l2_len +

>>> +                 pkt->l3_len;

>>

>>For non-tunnel packets, mbuf->outer_l3/l2_len are not guaranteed 0.

>>I think it's better to pass outer header length as a parameter of

>>gro_udp4_merge_item(), like udp_check_neighbor().

>

>For udp GRO, we can remove outer_l3/l2_len, they are nonsense.

>

>>

>>> +         ip_dl = pkt->pkt_len - hdr_len;

>>> +         frag_offset = tbl->items[item_idx].frag_offset;

>>> +         is_last_frag = tbl->items[item_idx].is_last_frag;

>>> +         cmp = udp_check_neighbor(&(tbl->items[start_idx]),

>>> +                                frag_offset, ip_dl, 0);

>>> +         if (cmp) {

>>> +                 if (merge_two_udp4_packets(

>>> +                                &(tbl->items[start_idx]),

>>> +                                pkt, cmp, frag_offset,

>>> +                                is_last_frag, 0)) {

>>> +                         item_idx = delete_item(tbl, item_idx,

>>> +                                               INVALID_ARRAY_INDEX);

>>> +                        tbl->items[start_idx].next_pkt_idx

>>> +                                = item_idx;

>>> +                 } else {

>>> +                         return 0;

>>> +                 }

>>> +         } else {

>>> +                 return 0;

>>> +         }

>>

>>A single line doesn't need braces.

>

>Got it, will remove { }

>

>>

>>> +  }

>>> +

>>> +  return 0;

>>> +}

>>> +

>>> +uint16_t

>>> +gro_udp4_tbl_timeout_flush(struct gro_udp4_tbl *tbl,

>>> +         uint64_t flush_timestamp,

>>> +         struct rte_mbuf **out,

>>> +         uint16_t nb_out)

>>> +{

>>> +  uint16_t k = 0;

>>> +  uint32_t i, j;

>>> +  uint32_t max_flow_num = tbl->max_flow_num;

>>> +

>>> +  for (i = 0; i < max_flow_num; i++) {

>>> +         if (unlikely(tbl->flow_num == 0))

>>> +                 return k;

>>> +

>>> +         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)

>>> +                                update_header(&(tbl->items[j]));

>>> +                         /*

>>> +                          * Delete the packet and get the next

>>> +                          * packet in the flow.

>>> +                          */

>>> +                         j = delete_item(tbl, j, INVALID_ARRAY_INDEX);

>>> +                         tbl->flows[i].start_index = j;

>>> +                         if (j == INVALID_ARRAY_INDEX)

>>> +                                tbl->flow_num--;

>>> +

>>> +                         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;

>>> +}

>>> +

>>> +uint32_t

>>> +gro_udp4_tbl_pkt_count(void *tbl)

>>> +{

>>> +  struct gro_udp4_tbl *gro_tbl = tbl;

>>> +

>>> +  if (gro_tbl)

>>> +         return gro_tbl->item_num;

>>> +

>>> +  return 0;

>>> +}

>>> diff --git a/lib/librte_gro/gro_udp4.h b/lib/librte_gro/gro_udp4.h

>>> new file mode 100644

>>> index 0000000..bc67eb1

>>> --- /dev/null

>>> +++ b/lib/librte_gro/gro_udp4.h

>>> @@ -0,0 +1,281 @@

>>> +/* SPDX-License-Identifier: BSD-3-Clause

>>> + * Copyright(c) 2020 Inspur Corporation

>>> + */

>>> +

>>> +#ifndef _GRO_UDP4_H_

>>> +#define _GRO_UDP4_H_

>>> +

>>> +#include <rte_ip.h>

>>> +#include <rte_udp.h>

>>> +

>>> +#define INVALID_ARRAY_INDEX 0xffffffffUL

>>> +#define GRO_UDP4_TBL_MAX_ITEM_NUM (1024UL * 1024UL)

>>> +

>>> +/*

>>> + * The max length of a IPv4 packet, which includes the length of the L3

>>> + * header, the L4 header and the data payload.

>>> + */

>>> +#define MAX_IPV4_PKT_LENGTH UINT16_MAX

>>> +

>>> +/* Header fields representing a UDP/IPv4 flow */

>>> +struct udp4_flow_key {

>>> +  struct rte_ether_addr eth_saddr;

>>> +  struct rte_ether_addr eth_daddr;

>>> +  uint32_t ip_src_addr;

>>> +  uint32_t ip_dst_addr;

>>> +

>>> +  /* IP fragment for UDP does not contain UDP header

>>> +   * except the first one. But IP ID must be same.

>>> +   */

>>> +  uint16_t ip_id;

>>> +};

>>> +

>>> +struct gro_udp4_flow {

>>> +  struct udp4_flow_key key;

>>> +  /*

>>> +   * The index of the first packet in the flow.

>>> +   * INVALID_ARRAY_INDEX indicates an empty flow.

>>> +   */

>>> +  uint32_t start_index;

>>> +};

>>> +

>>> +struct gro_udp4_item {

>>> +  /*

>>> +   * The first MBUF segment of the packet. If the value

>>> +   * is NULL, it means the item is empty.

>>> +   */

>>> +  struct rte_mbuf *firstseg;

>>> +  /* The last MBUF segment of the packet */

>>> +  struct rte_mbuf *lastseg;

>>> +  /*

>>> +   * The time when the first packet is inserted into the table.

>>> +   * This value won't be updated, even if the packet is merged

>>> +   * with other packets.

>>> +   */

>>> +  uint64_t start_time;

>>> +  /*

>>> +   * next_pkt_idx is used to chain the packets that

>>> +   * are in the same flow but can't be merged together

>>> +   * (e.g. caused by packet reordering).

>>> +   */

>>> +  uint32_t next_pkt_idx;

>>> +  /* offset of IP fragment packet */

>>> +  uint16_t frag_offset;

>>> +  /* is last IP fragment? */

>>> +  uint8_t is_last_frag;

>>> +  /* the number of merged packets */

>>> +  uint16_t nb_merged;

>>> +};

>>> +

>>> +/*

>>> + * UDP/IPv4 reassembly table structure.

>>> + */

>>> +struct gro_udp4_tbl {

>>> +  /* item array */

>>> +  struct gro_udp4_item *items;

>>> +  /* flow array */

>>> +  struct gro_udp4_flow *flows;

>>> +  /* current item number */

>>> +  uint32_t item_num;

>>> +  /* current flow num */

>>> +  uint32_t flow_num;

>>> +  /* item array size */

>>> +  uint32_t max_item_num;

>>> +  /* flow array size */

>>> +  uint32_t max_flow_num;

>>> +};

>>> +

>>> +/**

>>> + * This function creates a UDP/IPv4 reassembly table.

>>> + *

>>> + * @param socket_id

>>> + *  Socket index for allocating the UDP/IPv4 reassemble table

>>> + * @param max_flow_num

>>> + *  The maximum number of flows in the UDP/IPv4 GRO table

>>> + * @param max_item_per_flow

>>> + *  The maximum number of packets per flow

>>> + *

>>> + * @return

>>> + *  - Return the table pointer on success.

>>> + *  - Return NULL on failure.

>>> + */

>>> +void *gro_udp4_tbl_create(uint16_t socket_id,

>>> +         uint16_t max_flow_num,

>>> +         uint16_t max_item_per_flow);

>>> +

>>> +/**

>>> + * This function destroys a UDP/IPv4 reassembly table.

>>> + *

>>> + * @param tbl

>>> + *  Pointer pointing to the UDP/IPv4 reassembly table.

>>> + */

>>> +void gro_udp4_tbl_destroy(void *tbl);

>>> +

>>> +/**

>>> + * This function merges a UDP/IPv4 packet.

>>> + *

>>> + * This function does not check if the packet has correct checksums and

>>> + * does not re-calculate checksums for the merged packet. It returns the

>>> + * packet if it isn't UDP fragment or there is no available space in

>>> + * the table.

>>> + *

>>> + * @param pkt

>>> + *  Packet to reassemble

>>> + * @param tbl

>>> + *  Pointer pointing to the UDP/IPv4 reassembly table

>>> + * @start_time

>>> + *  The time when the packet is inserted into the table

>>> + *

>>> + * @return

>>> + *  - Return a positive value if the packet is merged.

>>> + *  - Return zero if the packet isn't merged but stored in the table.

>>> + *  - Return a negative value for invalid parameters or no available

>>> + *    space in the table.

>>> + */

>>> +int32_t gro_udp4_reassemble(struct rte_mbuf *pkt,

>>> +         struct gro_udp4_tbl *tbl,

>>> +         uint64_t start_time);

>>> +

>>> +/**

>>> + * This function flushes timeout packets in a UDP/IPv4 reassembly table,

>>> + * and without updating checksums.

>>> + *

>>> + * @param tbl

>>> + *  UDP/IPv4 reassembly table pointer

>>> + * @param flush_timestamp

>>> + *  Flush packets which are inserted into the table before or at the

>>> + *  flush_timestamp.

>>> + * @param out

>>> + *  Pointer array used to keep flushed packets

>>> + * @param nb_out

>>> + *  The element number in 'out'. It also determines the maximum number of

>>> + *  packets that can be flushed finally.

>>> + *

>>> + * @return

>>> + *  The number of flushed packets

>>> + */

>>> +uint16_t gro_udp4_tbl_timeout_flush(struct gro_udp4_tbl *tbl,

>>> +         uint64_t flush_timestamp,

>>> +         struct rte_mbuf **out,

>>> +         uint16_t nb_out);

>>> +

>>> +/**

>>> + * This function returns the number of the packets in a UDP/IPv4

>>> + * reassembly table.

>>> + *

>>> + * @param tbl

>>> + *  UDP/IPv4 reassembly table pointer

>>> + *

>>> + * @return

>>> + *  The number of packets in the table

>>> + */

>>> +uint32_t gro_udp4_tbl_pkt_count(void *tbl);

>>> +

>>> +/*

>>> + * Check if two UDP/IPv4 packets belong to the same flow.

>>> + */

>>> +static inline int

>>> +is_same_udp4_flow(struct udp4_flow_key k1, struct udp4_flow_key k2)

>>> +{

>>> +  return (rte_is_same_ether_addr(&k1.eth_saddr, &k2.eth_saddr) &&

>>> +                rte_is_same_ether_addr(&k1.eth_daddr, &k2.eth_daddr) &&

>>> +                 (k1.ip_src_addr == k2.ip_src_addr) &&

>>> +                 (k1.ip_dst_addr == k2.ip_dst_addr) &&

>>> +                 (k1.ip_id == k2.ip_id));

>>> +}

>>> +

>>> +/*

>>> + * Merge two UDP/IPv4 packets without updating checksums.

>>> + * If cmp is larger than 0, append the new packet to the

>>> + * original packet. Otherwise, pre-pend the new packet to

>>> + * the original packet.

>>> + */

>>> +static inline int

>>> +merge_two_udp4_packets(struct gro_udp4_item *item,

>>> +         struct rte_mbuf *pkt,

>>> +         int cmp,

>>> +         uint16_t frag_offset,

>>> +         uint8_t is_last_frag,

>>> +         uint16_t l2_offset)

>>> +{

>>> +  struct rte_mbuf *pkt_head, *pkt_tail, *lastseg;

>>> +  uint16_t hdr_len, l2_len;

>>> +  uint32_t ip_len;

>>> +

>>> +  if (cmp > 0) {

>>> +         pkt_head = item->firstseg;

>>> +         pkt_tail = pkt;

>>> +  } else {

>>> +         pkt_head = pkt;

>>> +         pkt_tail = item->firstseg;

>>> +  }

>>> +

>>> +  /* check if the IPv4 packet length is greater than the max value */

>>> +  hdr_len = l2_offset + pkt_head->l2_len + pkt_head->l3_len;

>>> +  l2_len = l2_offset > 0 ? pkt_head->outer_l2_len : pkt_head->l2_len;

>>> +  ip_len = pkt_head->pkt_len - l2_len

>>> +          + pkt_tail->pkt_len - hdr_len;

>>> +  if (unlikely(ip_len > MAX_IPV4_PKT_LENGTH))

>>> +         return 0;

>>> +

>>> +  /* remove the packet header for the tail packet */

>>> +  rte_pktmbuf_adj(pkt_tail, hdr_len);

>>> +

>>> +  /* chain two packets together */

>>> +  if (cmp > 0) {

>>> +         item->lastseg->next = pkt;

>>> +         item->lastseg = rte_pktmbuf_lastseg(pkt);

>>> +  } else {

>>> +         lastseg = rte_pktmbuf_lastseg(pkt);

>>> +         lastseg->next = item->firstseg;

>>> +         item->firstseg = pkt;

>>> +         item->frag_offset = frag_offset;

>>> +  }

>>> +  item->nb_merged++;

>>> +  if (is_last_frag)

>>> +         item->is_last_frag = is_last_frag;

>>> +

>>> +  /* update MBUF metadata for the merged packet */

>>> +  pkt_head->nb_segs += pkt_tail->nb_segs;

>>> +  pkt_head->pkt_len += pkt_tail->pkt_len;

>>> +

>>> +  return 1;

>>> +}

>>> +

>>> +/*

>>> + * Check if two UDP/IPv4 packets are neighbors.

>>> + */

>>> +static inline int

>>> +udp_check_neighbor(struct gro_udp4_item *item,

>>> +         uint16_t frag_offset,

>>> +         uint16_t ip_dl,

>>> +         uint16_t l2_offset)

>>

>>It's better to rename the function as udp4_check_neighbor(), IMO.

>

>No problem

>

>>

>>> +{

>>> +  struct rte_mbuf *pkt_orig = item->firstseg;

>>> +  uint16_t len;

>>> +

>>> +  /* check if the two packets are neighbors */

>>> +  len = pkt_orig->pkt_len - l2_offset - pkt_orig->l2_len -

>>> +         pkt_orig->l3_len;

>>> +  if (frag_offset == item->frag_offset + len)

>>> +         /* append the new packet */

>>> +         return 1;

>>> +  else if (frag_offset + ip_dl == item->frag_offset)

>>> +         /* pre-pend the new packet */

>>> +         return -1;

>>> +

>>> +  return 0;

>>> +}

>>> +

>>> +static inline int

>>> +is_ipv4_fragment(const struct rte_ipv4_hdr *hdr)

>>> +{

>>> +  uint16_t flag_offset, ip_flag, ip_ofs;

>>> +

>>> +  flag_offset = rte_be_to_cpu_16(hdr->fragment_offset);

>>> +  ip_ofs = (uint16_t)(flag_offset & RTE_IPV4_HDR_OFFSET_MASK);

>>> +  ip_flag = (uint16_t)(flag_offset & RTE_IPV4_HDR_MF_FLAG);

>>> +

>>> +  return ip_flag != 0 || ip_ofs  != 0;

>>> +}

>>> +#endif

>>> diff --git a/lib/librte_gro/meson.build b/lib/librte_gro/meson.build

>>> index 501668c..0d18dc2 100644

>>> --- a/lib/librte_gro/meson.build

>>> +++ b/lib/librte_gro/meson.build

>>> @@ -1,6 +1,6 @@

>>>  # SPDX-License-Identifier: BSD-3-Clause

>>>  # Copyright(c) 2017 Intel Corporation

>>>

>>> -sources = files('rte_gro.c', 'gro_tcp4.c', 'gro_vxlan_tcp4.c')

>>> +sources = files('rte_gro.c', 'gro_tcp4.c', 'gro_udp4.c', 'gro_vxlan_tcp4.c')

>>>  headers = files('rte_gro.h')

>>>  deps += ['ethdev']

>>> diff --git a/lib/librte_gro/rte_gro.c b/lib/librte_gro/rte_gro.c

>>> index 6618f4d..d094129 100644

>>> --- a/lib/librte_gro/rte_gro.c

>>> +++ b/lib/librte_gro/rte_gro.c

>>> @@ -9,6 +9,7 @@

>>>

>>>  #include "rte_gro.h"

>>>  #include "gro_tcp4.h"

>>> +#include "gro_udp4.h"

>>>  #include "gro_vxlan_tcp4.h"

>>>

>>>  typedef void *(*gro_tbl_create_fn)(uint16_t socket_id,

>>> @@ -18,17 +19,23 @@

>>>  typedef uint32_t (*gro_tbl_pkt_count_fn)(void *tbl);

>>>

>>>  static gro_tbl_create_fn tbl_create_fn[RTE_GRO_TYPE_MAX_NUM] = {

>>> -         gro_tcp4_tbl_create, gro_vxlan_tcp4_tbl_create, NULL};

>>> +         gro_tcp4_tbl_create, gro_vxlan_tcp4_tbl_create,

>>> +         gro_udp4_tbl_create, NULL};

>>>  static gro_tbl_destroy_fn tbl_destroy_fn[RTE_GRO_TYPE_MAX_NUM] = {

>>>                   gro_tcp4_tbl_destroy, gro_vxlan_tcp4_tbl_destroy,

>>> +                 gro_udp4_tbl_destroy,

>>>                   NULL};

>>>  static gro_tbl_pkt_count_fn tbl_pkt_count_fn[RTE_GRO_TYPE_MAX_NUM] = {

>>>                   gro_tcp4_tbl_pkt_count, gro_vxlan_tcp4_tbl_pkt_count,

>>> +                 gro_udp4_tbl_pkt_count,

>>>                   NULL};

>>>

>>>  #define IS_IPV4_TCP_PKT(ptype) (RTE_ETH_IS_IPV4_HDR(ptype) && \

>>>           ((ptype & RTE_PTYPE_L4_TCP) == RTE_PTYPE_L4_TCP))

>>>

>>> +#define IS_IPV4_UDP_PKT(ptype) (RTE_ETH_IS_IPV4_HDR(ptype) && \

>>> +         ((ptype & RTE_PTYPE_L4_UDP) == RTE_PTYPE_L4_UDP))

>>> +

>>>  #define IS_IPV4_VXLAN_TCP4_PKT(ptype) (RTE_ETH_IS_IPV4_HDR(ptype) && \

>>>           ((ptype & RTE_PTYPE_L4_UDP) == RTE_PTYPE_L4_UDP) && \

>>>           ((ptype & RTE_PTYPE_TUNNEL_VXLAN) == \

>>> @@ -40,6 +47,7 @@

>>>                RTE_PTYPE_INNER_L3_IPV4_EXT | \

>>>                RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)) != 0))

>>>

>>> +

>>>  /*

>>>   * GRO context structure. It keeps the table structures, which are

>>>   * used to merge packets, for different GRO types. Before using

>>> @@ -123,20 +131,26 @@ struct gro_ctx {

>>>    struct gro_tcp4_flow tcp_flows[RTE_GRO_MAX_BURST_ITEM_NUM];

>>>    struct gro_tcp4_item tcp_items[RTE_GRO_MAX_BURST_ITEM_NUM] = {{0} };

>>>

>>> -  /* Allocate a reassembly table for VXLAN GRO */

>>> +  /* allocate a reassembly table for UDP/IPv4 GRO */

>>> +  struct gro_udp4_tbl udp_tbl;

>>> +  struct gro_udp4_flow udp_flows[RTE_GRO_MAX_BURST_ITEM_NUM];

>>> +  struct gro_udp4_item udp_items[RTE_GRO_MAX_BURST_ITEM_NUM] = {{0} };

>>> +

>>> +  /* Allocate a reassembly table for VXLAN TCP GRO */

>>>    struct gro_vxlan_tcp4_tbl vxlan_tbl;

>>>    struct gro_vxlan_tcp4_flow vxlan_flows[RTE_GRO_MAX_BURST_ITEM_NUM];

>>> -  struct gro_vxlan_tcp4_item vxlan_items[RTE_GRO_MAX_BURST_ITEM_NUM] = {

>>> -         {{0}, 0, 0} };

>>> +  struct gro_vxlan_tcp4_item vxlan_items[RTE_GRO_MAX_BURST_ITEM_NUM]

>>> +                 = {{{0}, 0, 0} };

>>>

>>>    struct rte_mbuf *unprocess_pkts[nb_pkts];

>>>    uint32_t item_num;

>>>    int32_t ret;

>>>    uint16_t i, unprocess_num = 0, nb_after_gro = nb_pkts;

>>> -  uint8_t do_tcp4_gro = 0, do_vxlan_gro = 0;

>>> +  uint8_t do_tcp4_gro = 0, do_vxlan_gro = 0, do_udp4_gro = 0;

>>>

>>>    if (unlikely((param->gro_types & (RTE_GRO_IPV4_VXLAN_TCP_IPV4 |

>>> -                                RTE_GRO_TCP_IPV4)) == 0))

>>> +                                RTE_GRO_TCP_IPV4 |

>>> +                                RTE_GRO_UDP_IPV4)) == 0))

>>>           return nb_pkts;

>>>

>>>    /* Get the maximum number of packets */

>>> @@ -170,6 +184,20 @@ struct gro_ctx {

>>>           do_tcp4_gro = 1;

>>>    }

>>>

>>> +  if (param->gro_types & RTE_GRO_UDP_IPV4) {

>>> +         for (i = 0; i < item_num; i++)

>>> +                 udp_flows[i].start_index = INVALID_ARRAY_INDEX;

>>> +

>>> +         udp_tbl.flows = udp_flows;

>>> +         udp_tbl.items = udp_items;

>>> +         udp_tbl.flow_num = 0;

>>> +         udp_tbl.item_num = 0;

>>> +         udp_tbl.max_flow_num = item_num;

>>> +         udp_tbl.max_item_num = item_num;

>>> +         do_udp4_gro = 1;

>>> +  }

>>> +

>>> +

>>>    for (i = 0; i < nb_pkts; i++) {

>>>           /*

>>>            * The timestamp is ignored, since all packets

>>> @@ -177,7 +205,8 @@ struct gro_ctx {

>>>            */

>>>           if (IS_IPV4_VXLAN_TCP4_PKT(pkts[i]->packet_type) &&

>>>                           do_vxlan_gro) {

>>> -                 ret = gro_vxlan_tcp4_reassemble(pkts[i], &vxlan_tbl, 0);

>>> +                 ret = gro_vxlan_tcp4_reassemble(pkts[i],

>>> +                                               &vxlan_tbl, 0);

>>>                   if (ret > 0)

>>>                           /* Merge successfully */

>>>                           nb_after_gro--;

>>> @@ -191,27 +220,43 @@ struct gro_ctx {

>>>                           nb_after_gro--;

>>>                   else if (ret < 0)

>>>                          unprocess_pkts[unprocess_num++] = pkts[i];

>>> +         } else if (IS_IPV4_UDP_PKT(pkts[i]->packet_type) &&

>>> +                         do_udp4_gro) {

>>> +                 ret = gro_udp4_reassemble(pkts[i], &udp_tbl, 0);

>>> +                 if (ret > 0)

>>> +                         /* merge successfully */

>>> +                         nb_after_gro--;

>>> +                 else if (ret < 0)

>>> +                        unprocess_pkts[unprocess_num++] = pkts[i];

>>>           } else

>>>                   unprocess_pkts[unprocess_num++] = pkts[i];

>>>    }

>>>

>>> -  if (nb_after_gro < nb_pkts) {

>>> +  if ((nb_after_gro < nb_pkts)

>>> +          || (unprocess_num < nb_pkts)) {

>>

>>Why need to check unprocess_num here?

>

>In the case that packet isn't merged, nb_after_gro won't be changed, we only can use unprocess_num to check, it is possible to reassmble them successfully in flush phase.

>

>>

>>>           i = 0;

>>>           /* Flush all packets from the tables */

>>>           if (do_vxlan_gro) {

>>>                   i = gro_vxlan_tcp4_tbl_timeout_flush(&vxlan_tbl,

>>>                                  0, pkts, nb_pkts);

>>>           }

>>> +

>>>           if (do_tcp4_gro) {

>>>                   i += gro_tcp4_tbl_timeout_flush(&tcp_tbl, 0,

>>>                                  &pkts[i], nb_pkts - i);

>>>           }

>>> +

>>> +         if (do_udp4_gro) {

>>> +                 i += gro_udp4_tbl_timeout_flush(&udp_tbl, 0,

>>> +                                &pkts[i], nb_pkts - i);

>>> +         }

>>>           /* Copy unprocessed packets */

>>>           if (unprocess_num > 0) {

>>>                   memcpy(&pkts[i], unprocess_pkts,

>>>                                  sizeof(struct rte_mbuf *) *

>>>                                  unprocess_num);

>>>           }

>>> +         nb_after_gro = i + unprocess_num;

>>>    }

>>>

>>>    return nb_after_gro;

>>> 1.8.3.1

>

>




  reply	other threads:[~2020-09-10  2:47 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-04  8:37 [dpdk-dev] [PATCH v4 0/2] gro: add UDP GRO and VXLAN " yang_y_yi
2020-09-04  8:37 ` [dpdk-dev] [PATCH v4 1/2] gro: add " yang_y_yi
2020-09-08  2:05   ` Jiayu Hu
2020-09-09  7:25     ` yang_y_yi
2020-09-09  9:00       ` yang_y_yi
2020-09-10  2:47         ` Hu, Jiayu [this message]
2020-09-10  9:31           ` yang_y_yi
2020-09-04  8:37 ` [dpdk-dev] [PATCH v4 2/2] gro: add VXLAN " 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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=9dfa31fd84d847ccbd459e1b54eb32a5@intel.com \
    --to=jiayu.hu@intel.com \
    --cc=dev@dpdk.org \
    --cc=thomas@monjalon.net \
    --cc=yang_y_yi@163.com \
    --cc=yangyi01@inspur.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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 \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


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