Re: [dpdk-dev] [PATCH v7 2/3] gro: add VXLAN UDP/IPv4 GRO support

[Jiayu Hu <jiayu.hu@intel.com>]

On Sun, Sep 27, 2020 at 11:14:44AM +0800, yang_y_yi wrote:
> Ok, thanks Jiayu, but I don't know before which "}" whitespace should be
> inserted, can you paste your check report here?
> 
> BTW, I used chekpatch.pl in Linux 5.5.9, it doesn't report this,I also tried
> checkpatch.pl in Linus kernel git tree, the same result, so out of curiosity,
> can you send me your checkpatch.pl? I can use it to check my patches later.

My kernel is 4.4.0. But I checked the report of ci/checkpatch in dpdk
website, and no coding style issues are reported. So please ignore
the "}" issue I mentioned in the previous mail.

Thanks,
Jiayu

> 
> 
> At 2020-09-27 09:49:37, "Hu, Jiayu" <jiayu.hu@intel.com> wrote:
> >Hi Yi,
> >
> >Just a small issue, and please see it inline. After change it,
> >you can add my ack in the serial patches.
> >
> >Thanks,
> >Jiayu
> >> -----Original Message-----
> >> From: yang_y_yi@163.com <yang_y_yi@163.com>
> >> Sent: Thursday, September 24, 2020 4:58 PM
> >> To: dev@dpdk.org
> >> Cc: Hu, Jiayu <jiayu.hu@intel.com>; thomas@monjalon.net;
> >> yangyi01@inspur.com; yang_y_yi@163.com
> >> Subject: [PATCH v7 2/3] gro: add VXLAN UDP/IPv4 GRO support
> >>
> >> From: Yi Yang <yangyi01@inspur.com>
> >>
> >> VXLAN UDP/IPv4 GRO can help improve VM-to-VM UDP
> >> performance when UFO or GSO is enabled in VM, GRO
> >> must be supported if UFO or GSO is enabled,
> >> otherwise, performance can't get big improvement
> >> if only GSO is there.
> >>
> >> With this enabled in DPDK, OVS DPDK can leverage it
> >> to improve VM-to-VM UDP performance, it will reassemble
> >> VXLAN UDP/IPv4 fragments immediate after they are
> >> received from a physical NIC. It is very helpful in
> >> OVS DPDK VXLAN use case.
> >>
> >> Signed-off-by: Yi Yang <yangyi01@inspur.com>
> >> diff --git a/lib/librte_gro/rte_gro.c b/lib/librte_gro/rte_gro.c
> >> index ac23df1..e56bd20 100644
> >> --- a/lib/librte_gro/rte_gro.c
> >> +++ b/lib/librte_gro/rte_gro.c
> >> @@ -11,6 +11,7 @@
> >>  #include "gro_tcp4.h"
> >>  #include "gro_udp4.h"
> >>  #include "gro_vxlan_tcp4.h"
> >> +#include "gro_vxlan_udp4.h"
> >>
> >>  typedef void *(*gro_tbl_create_fn)(uint16_t socket_id,
> >>              uint16_t max_flow_num,
> >> @@ -20,14 +21,14 @@
> >>
> >>  static gro_tbl_create_fn tbl_create_fn[RTE_GRO_TYPE_MAX_NUM] = {
> >>              gro_tcp4_tbl_create, gro_vxlan_tcp4_tbl_create,
> >> -            gro_udp4_tbl_create, NULL};
> >> +            gro_udp4_tbl_create, gro_vxlan_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,
> >> +                    gro_udp4_tbl_destroy, gro_vxlan_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,
> >> +                    gro_udp4_tbl_pkt_count,
> >> gro_vxlan_udp4_tbl_pkt_count,
> >>                      NULL};
> >>
> >>  #define IS_IPV4_TCP_PKT(ptype) (RTE_ETH_IS_IPV4_HDR(ptype) && \
> >> @@ -47,6 +48,16 @@
> >>                   RTE_PTYPE_INNER_L3_IPV4_EXT | \
> >>                   RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)) != 0))
> >>
> >> +#define IS_IPV4_VXLAN_UDP4_PKT(ptype) (RTE_ETH_IS_IPV4_HDR(ptype)
> >> && \
> >> +            ((ptype & RTE_PTYPE_L4_UDP) == RTE_PTYPE_L4_UDP) && \
> >> +            ((ptype & RTE_PTYPE_TUNNEL_VXLAN) == \
> >> +             RTE_PTYPE_TUNNEL_VXLAN) && \
> >> +             ((ptype & RTE_PTYPE_INNER_L4_UDP) == \
> >> +              RTE_PTYPE_INNER_L4_UDP) && \
> >> +              (((ptype & RTE_PTYPE_INNER_L3_MASK) & \
> >> +                (RTE_PTYPE_INNER_L3_IPV4 | \
> >> +                 RTE_PTYPE_INNER_L3_IPV4_EXT | \
> >> +                 RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)) != 0))
> >>
> >>  /*
> >>   * GRO context structure. It keeps the table structures, which are
> >> @@ -137,19 +148,27 @@ struct gro_ctx {
> >>      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]
> >> +    struct gro_vxlan_tcp4_tbl vxlan_tcp_tbl;
> >> +    struct gro_vxlan_tcp4_flow
> >> vxlan_tcp_flows[RTE_GRO_MAX_BURST_ITEM_NUM];
> >> +    struct gro_vxlan_tcp4_item
> >> vxlan_tcp_items[RTE_GRO_MAX_BURST_ITEM_NUM]
> >>                      = {{{0}, 0, 0} };
> >>
> >> +    /* Allocate a reassembly table for VXLAN UDP GRO */
> >> +    struct gro_vxlan_udp4_tbl vxlan_udp_tbl;
> >> +    struct gro_vxlan_udp4_flow
> >> vxlan_udp_flows[RTE_GRO_MAX_BURST_ITEM_NUM];
> >> +    struct gro_vxlan_udp4_item
> >> vxlan_udp_items[RTE_GRO_MAX_BURST_ITEM_NUM]
> >> +                    = {{{0}} };
> >
> >It seems you need add a space after the "}", and this is found by checkpatch.
> >
> >> +
> >>      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, do_udp4_gro = 0;
> >> +    uint8_t do_tcp4_gro = 0, do_vxlan_tcp_gro = 0, do_udp4_gro = 0,
> >> +            do_vxlan_udp_gro = 0;
> >>
> >>      if (unlikely((param->gro_types & (RTE_GRO_IPV4_VXLAN_TCP_IPV4 |
> >>                                      RTE_GRO_TCP_IPV4 |
> >> +                                    RTE_GRO_IPV4_VXLAN_UDP_IPV4 |
> >>                                      RTE_GRO_UDP_IPV4)) == 0))
> >>              return nb_pkts;
> >>
> >> @@ -160,15 +179,28 @@ struct gro_ctx {
> >>
> >>      if (param->gro_types & RTE_GRO_IPV4_VXLAN_TCP_IPV4) {
> >>              for (i = 0; i < item_num; i++)
> >> -                    vxlan_flows[i].start_index = INVALID_ARRAY_INDEX;
> >> -
> >> -            vxlan_tbl.flows = vxlan_flows;
> >> -            vxlan_tbl.items = vxlan_items;
> >> -            vxlan_tbl.flow_num = 0;
> >> -            vxlan_tbl.item_num = 0;
> >> -            vxlan_tbl.max_flow_num = item_num;
> >> -            vxlan_tbl.max_item_num = item_num;
> >> -            do_vxlan_gro = 1;
> >> +                    vxlan_tcp_flows[i].start_index =
> >> INVALID_ARRAY_INDEX;
> >> +
> >> +            vxlan_tcp_tbl.flows = vxlan_tcp_flows;
> >> +            vxlan_tcp_tbl.items = vxlan_tcp_items;
> >> +            vxlan_tcp_tbl.flow_num = 0;
> >> +            vxlan_tcp_tbl.item_num = 0;
> >> +            vxlan_tcp_tbl.max_flow_num = item_num;
> >> +            vxlan_tcp_tbl.max_item_num = item_num;
> >> +            do_vxlan_tcp_gro = 1;
> >> +    }
> >> +
> 
> 
> 
>  
>