All of lore.kernel.org
 help / color / mirror / Atom feed
From: kumaraparameshwaran rathinavel <kumaraparamesh92@gmail.com>
To: "Hu, Jiayu" <jiayu.hu@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [PATCH v4] gro: bug fix in identifying fragmented packets
Date: Fri, 17 Jun 2022 17:03:47 +0530	[thread overview]
Message-ID: <CANxNyasQ9S0FJt3vxdhVqtSO+G_wFtDNSTvsCM176kxwJ0=AaQ@mail.gmail.com> (raw)
In-Reply-To: <15ebcc9606c14f75a92a7498a3f78589@intel.com>

[-- Attachment #1: Type: text/plain, Size: 7982 bytes --]

> -----Original Message-----
> From: Kumara Parameshwaran <kumaraparamesh92@gmail.com>
> Sent: Wednesday, June 8, 2022 5:57 PM
> To: Hu, Jiayu <jiayu.hu@intel.com>
> Cc: dev@dpdk.org; Kumara Parameshwaran
> <kumaraparamesh92@gmail.com>; stable@dpdk.org
> Subject: [PATCH v4] gro: bug fix in identifying fragmented packets
>
> From: Kumara Parameshwaran <kumaraparamesh92@gmail.com>
>
> A packet with RTE_PTYPE_L4_FRAG(0x300) contains both RTE_PTYPE_L4_TCP
> (0x100) & RTE_PTYPE_L4_UDP (0x200). A fragmented packet as defined in
> rte_mbuf_ptype.h cannot be recognized as other L4 types and hence the
> GRO layer should not use IS_IPV4_TCP_PKT or IS_IPV4_UDP_PKT for
> RTE_PTYPE_L4_FRAG. Hence, if the packet type is RTE_PTYPE_L4_FRAG the ip

A simpler way is to add a "((ptype & RTE_PTYPE_L4_FRAG) !=
> RTE_PTYPE_L4_FRAG))"
> in IS_IPV4_VXLAN_TCP4_PKT and IS_IPV4_TCP_PKT to avoid processing IP
> fragments
> in TCP based GRO functions. For example:
> #define IS_IPV4_TCP_PKT(ptype) (RTE_ETH_IS_IPV4_HDR(ptype) && \
>                 ((ptype & RTE_PTYPE_L4_TCP) == RTE_PTYPE_L4_TCP) && \
>         ((ptype & RTE_PTYPE_L4_FRAG) != RTE_PTYPE_L4_FRAG) && \
>                 (RTE_ETH_IS_TUNNEL_PKT(ptype) == 0))


This would be handling only the UDP fragmentation case. What if there is
fragmentation of TCP packets?  What happens if the packet is
RTE_PTYPE_L4_FRAG and in the  struct rte_ipv4_hdr the proto type is
IPPROTO_TCP ? What happens to that case?

On Sun, Jun 12, 2022 at 10:50 AM Hu, Jiayu <jiayu.hu@intel.com> wrote:

> Hi Kumara,
>
> > -----Original Message-----
> > From: Kumara Parameshwaran <kumaraparamesh92@gmail.com>
> > Sent: Wednesday, June 8, 2022 5:57 PM
> > To: Hu, Jiayu <jiayu.hu@intel.com>
> > Cc: dev@dpdk.org; Kumara Parameshwaran
> > <kumaraparamesh92@gmail.com>; stable@dpdk.org
> > Subject: [PATCH v4] gro: bug fix in identifying fragmented packets
> >
> > From: Kumara Parameshwaran <kumaraparamesh92@gmail.com>
> >
> > A packet with RTE_PTYPE_L4_FRAG(0x300) contains both RTE_PTYPE_L4_TCP
> > (0x100) & RTE_PTYPE_L4_UDP (0x200). A fragmented packet as defined in
> > rte_mbuf_ptype.h cannot be recognized as other L4 types and hence the
> > GRO layer should not use IS_IPV4_TCP_PKT or IS_IPV4_UDP_PKT for
> > RTE_PTYPE_L4_FRAG. Hence, if the packet type is RTE_PTYPE_L4_FRAG the ip
>
> A simpler way is to add a "((ptype & RTE_PTYPE_L4_FRAG) !=
> RTE_PTYPE_L4_FRAG))"
> in IS_IPV4_VXLAN_TCP4_PKT and IS_IPV4_TCP_PKT to avoid processing IP
> fragments
> in TCP based GRO functions. For example:
> #define IS_IPV4_TCP_PKT(ptype) (RTE_ETH_IS_IPV4_HDR(ptype) && \
>                 ((ptype & RTE_PTYPE_L4_TCP) == RTE_PTYPE_L4_TCP) && \
>         ((ptype & RTE_PTYPE_L4_FRAG) != RTE_PTYPE_L4_FRAG) && \
>                 (RTE_ETH_IS_TUNNEL_PKT(ptype) == 0))
>
> Thanks,
> Jiayu
>
> > header should be parsed to recognize the appropriate IP type and invoke
> the
> > respective gro handler.
> >
> > Fixes: 1ca5e6740852 ("gro: support UDP/IPv4")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Kumara Parameshwaran <kumaraparamesh92@gmail.com>
> > ---
> > v1:
> > * Introduce IS_IPV4_FRAGMENT macro to check if fragmented packet and
> >   if true extract the IP header to identify the protocol type and
> >   invoke the appropriate gro handler. This is done for both
> >   rte_gro_reassemble and rte_gro_reassemble_burst APIs.
> > v2,v3,v4:
> > * Fix extra whitespace and column limit warnings
> >
> >  lib/gro/rte_gro.c | 43 +++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 41 insertions(+), 2 deletions(-)  lib/gro/rte_gro.c | 43
> > +++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 41 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/gro/rte_gro.c b/lib/gro/rte_gro.c index
> > 6f7dd4d709..83d6e21dbb 100644
> > --- a/lib/gro/rte_gro.c
> > +++ b/lib/gro/rte_gro.c
> > @@ -38,6 +38,9 @@ static gro_tbl_pkt_count_fn
> > tbl_pkt_count_fn[RTE_GRO_TYPE_MAX_NUM] = {
> >               ((ptype & RTE_PTYPE_L4_UDP) == RTE_PTYPE_L4_UDP) && \
> >               (RTE_ETH_IS_TUNNEL_PKT(ptype) == 0))
> >
> > +#define IS_IPV4_FRAGMENT(ptype) (RTE_ETH_IS_IPV4_HDR(ptype) && \
> > +             ((ptype & RTE_PTYPE_L4_FRAG) == RTE_PTYPE_L4_FRAG))
> > +
> >  #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) == \ @@ -240,7
> > +243,28 @@ rte_gro_reassemble_burst(struct rte_mbuf **pkts,
> >                * The timestamp is ignored, since all packets
> >                * will be flushed from the tables.
> >                */
> > -             if (IS_IPV4_VXLAN_TCP4_PKT(pkts[i]->packet_type) &&
> > +             if (IS_IPV4_FRAGMENT(pkts[i]->packet_type)) {
> > +                     struct rte_ipv4_hdr ip4h_copy;
> > +                     const struct rte_ipv4_hdr *ip4h =
> > rte_pktmbuf_read(pkts[i], pkts[i]->l2_len,
> > +
> >                                       sizeof(*ip4h), &ip4h_copy);
> > +                     if (ip4h->next_proto_id == IPPROTO_UDP &&
> > do_udp4_gro) {
> > +                             ret = gro_udp4_reassemble(pkts[i],
> > +                                                     &udp_tbl, 0);
> > +                             if (ret > 0)
> > +                                     nb_after_gro--;
> > +                             else if (ret < 0)
> > +                                     unprocess_pkts[unprocess_num++] =
> > pkts[i];
> > +                     } else if (ip4h->next_proto_id == IPPROTO_TCP &&
> > do_tcp4_gro) {
> > +                             ret = gro_tcp4_reassemble(pkts[i],
> > +                                             &tcp_tbl, 0);
> > +                             if (ret > 0)
> > +                                     nb_after_gro--;
> > +                             else if (ret < 0)
> > +                                     unprocess_pkts[unprocess_num++] =
> > pkts[i];
> > +                     } else {
> > +                             unprocess_pkts[unprocess_num++] = pkts[i];
> > +                     }
> > +             } else if (IS_IPV4_VXLAN_TCP4_PKT(pkts[i]->packet_type) &&
> >                               do_vxlan_tcp_gro) {
> >                       ret = gro_vxlan_tcp4_reassemble(pkts[i],
> >                                                       &vxlan_tcp_tbl, 0);
> > @@ -349,7 +373,22 @@ rte_gro_reassemble(struct rte_mbuf **pkts,
> >       current_time = rte_rdtsc();
> >
> >       for (i = 0; i < nb_pkts; i++) {
> > -             if (IS_IPV4_VXLAN_TCP4_PKT(pkts[i]->packet_type) &&
> > +             if (IS_IPV4_FRAGMENT(pkts[i]->packet_type)) {
> > +                     struct rte_ipv4_hdr ip4h_copy;
> > +                     const struct rte_ipv4_hdr *ip4h =
> > rte_pktmbuf_read(pkts[i], pkts[i]->l2_len,
> > +
> >                                       sizeof(*ip4h), &ip4h_copy);
> > +                     if (ip4h->next_proto_id == IPPROTO_UDP &&
> > do_udp4_gro) {
> > +                             if (gro_udp4_reassemble(pkts[i], udp_tbl,
> > +                                             current_time) < 0)
> > +                                     unprocess_pkts[unprocess_num++] =
> > pkts[i];
> > +                     } else if (ip4h->next_proto_id == IPPROTO_TCP &&
> > do_tcp4_gro) {
> > +                             if (gro_tcp4_reassemble(pkts[i], tcp_tbl,
> > +                                             current_time) < 0)
> > +                                     unprocess_pkts[unprocess_num++] =
> > pkts[i];
> > +                     } else {
> > +                             unprocess_pkts[unprocess_num++] = pkts[i];
> > +                     }
> > +             } else if (IS_IPV4_VXLAN_TCP4_PKT(pkts[i]->packet_type) &&
> >                               do_vxlan_tcp_gro) {
> >                       if (gro_vxlan_tcp4_reassemble(pkts[i],
> vxlan_tcp_tbl,
> >                                               current_time) < 0)
> > --
> > 2.25.1
>
>

[-- Attachment #2: Type: text/html, Size: 11432 bytes --]

  reply	other threads:[~2022-06-17 11:34 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-19 11:18 [PATCH v1] gro: fix gro for UDP fragmented packets Kumara Parameshwaran
2022-03-20 10:12 ` [PATCH v2] " Kumara Parameshwaran
2022-06-08  9:46   ` [PATCH v3] gro: bug fix in identifying " Kumara Parameshwaran
2022-06-08  9:57   ` [PATCH v4] " Kumara Parameshwaran
2022-06-09  8:14     ` Thomas Monjalon
2022-06-09 14:19       ` Hu, Jiayu
2022-06-12  5:20     ` Hu, Jiayu
2022-06-17 11:33       ` kumaraparameshwaran rathinavel [this message]
2022-06-20  2:57         ` Hu, Jiayu
2022-06-20  5:23           ` kumaraparameshwaran rathinavel
2022-06-27 10:31   ` [PATCH v5] " Kumara Parameshwaran
2022-06-29  6:57     ` Hu, Jiayu
2022-07-05 16:30       ` Thomas Monjalon

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='CANxNyasQ9S0FJt3vxdhVqtSO+G_wFtDNSTvsCM176kxwJ0=AaQ@mail.gmail.com' \
    --to=kumaraparamesh92@gmail.com \
    --cc=dev@dpdk.org \
    --cc=jiayu.hu@intel.com \
    --cc=stable@dpdk.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.