Identifying packet types requires masking with the mask, not the value,
before comparing to the value.
The faulty macros were corrected.

Furthermore, GRO for UDP ingests IP fragments, not UDP packets, so
compare to IP fragments instead of UDP packets.
The same applies to GRO for inner UDP in VXLAN packets; compare to
to inner IP fragments instead of inner UDP packets.
This was not obvious looking at the macros, so they were renamed to
reflect what they match, and comments were added to mention what they are
used for.

Note:
This bug has not been caught earlier, probably because the faulty formula
"((ptype & RTE_PTYPE_L4_UDP) == RTE_PTYPE_L4_UDP)" matched IP fragments
by pure luck:
RTE_PTYPE_L4_UDP has the value 0x200, and
RTE_PTYPE_L4_FRAG has the value 0x300, so the faulty formula
evaluates as "((0x300 & 0x200) == 0x200)", which is true.

It also - incorrectly - matched other packet types with the 0x200 bit set,
e.g. RTE_PTYPE_L4_IGMP (value 0x700), which it shouldn't match.

Fixes: 0d2cbe59b719 ("lib/gro: support TCP/IPv4")
Fixes: 9e0b9d2ec0f4 ("gro: support VxLAN GRO")
Fixes: 1ca5e6740852 ("gro: support UDP/IPv4")
Fixes: e2d811063673 ("gro: support VXLAN UDP/IPv4")
Cc: [email protected]

Signed-off-by: Morten Brørup <[email protected]>
---
v2:
* Rebased.
* Updated patch description with many more details.
* UDP offload wants IP fragments, not UDP packets. (Thomas' AI)
* Inner UDP offload of VXLAN wants inner IP fragments,
  not inner UDP packets. (Thomas' AI)
* Renamed the macros regarding GRO for UDP and VXLAN inner UDP,
  and added comments to these.
---
 lib/gro/rte_gro.c | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/lib/gro/rte_gro.c b/lib/gro/rte_gro.c
index 578cc9b801..f9d1c5ad03 100644
--- a/lib/gro/rte_gro.c
+++ b/lib/gro/rte_gro.c
@@ -35,26 +35,24 @@ static gro_tbl_pkt_count_fn 
tbl_pkt_count_fn[RTE_GRO_TYPE_MAX_NUM] = {
                        NULL};
 
 #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) && \
+               ((ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_TCP) && \
                (RTE_ETH_IS_TUNNEL_PKT(ptype) == 0))
 
 /* GRO with extension headers is not supported */
 #define IS_IPV6_TCP_PKT(ptype) (RTE_ETH_IS_IPV6_HDR(ptype) && \
-               ((ptype & RTE_PTYPE_L4_TCP) == RTE_PTYPE_L4_TCP) && \
-               ((ptype & RTE_PTYPE_L4_FRAG) != RTE_PTYPE_L4_FRAG) && \
+               ((ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_TCP) && \
                (RTE_ETH_IS_TUNNEL_PKT(ptype) == 0))
 
-#define IS_IPV4_UDP_PKT(ptype) (RTE_ETH_IS_IPV4_HDR(ptype) && \
-               ((ptype & RTE_PTYPE_L4_UDP) == RTE_PTYPE_L4_UDP) && \
+/* GRO for UDP ingests IP fragments */
+#define IS_IPV4_FRAG_PKT(ptype) (RTE_ETH_IS_IPV4_HDR(ptype) && \
+               ((ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_FRAG) && \
                (RTE_ETH_IS_TUNNEL_PKT(ptype) == 0))
 
 #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_L4_FRAG) != RTE_PTYPE_L4_FRAG) && \
-               ((ptype & RTE_PTYPE_TUNNEL_VXLAN) == \
+               ((ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_UDP) && \
+               ((ptype & RTE_PTYPE_TUNNEL_MASK) == \
                 RTE_PTYPE_TUNNEL_VXLAN) && \
-               ((ptype & RTE_PTYPE_INNER_L4_TCP) == \
+               ((ptype & RTE_PTYPE_INNER_L4_MASK) == \
                 RTE_PTYPE_INNER_L4_TCP) && \
                (((ptype & RTE_PTYPE_INNER_L3_MASK) == \
                  RTE_PTYPE_INNER_L3_IPV4) || \
@@ -63,12 +61,13 @@ static gro_tbl_pkt_count_fn 
tbl_pkt_count_fn[RTE_GRO_TYPE_MAX_NUM] = {
                 ((ptype & RTE_PTYPE_INNER_L3_MASK) == \
                  RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN)))
 
-#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) == \
+/* GRO for inner UDP of VXLAN ingests inner IP fragments */
+#define IS_IPV4_VXLAN_FRAG4_PKT(ptype) (RTE_ETH_IS_IPV4_HDR(ptype) && \
+               ((ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_UDP) && \
+               ((ptype & RTE_PTYPE_TUNNEL_MASK) == \
                 RTE_PTYPE_TUNNEL_VXLAN) && \
-               ((ptype & RTE_PTYPE_INNER_L4_UDP) == \
-                RTE_PTYPE_INNER_L4_UDP) && \
+               ((ptype & RTE_PTYPE_INNER_L4_MASK) == \
+                RTE_PTYPE_INNER_L4_FRAG) && \
                (((ptype & RTE_PTYPE_INNER_L3_MASK) == \
                  RTE_PTYPE_INNER_L3_IPV4) || \
                 ((ptype & RTE_PTYPE_INNER_L3_MASK) == \
@@ -279,7 +278,7 @@ rte_gro_reassemble_burst(struct rte_mbuf **pkts,
                                nb_after_gro--;
                        else if (ret < 0)
                                pkts[unprocess_num++] = pkts[i];
-               } else if (IS_IPV4_VXLAN_UDP4_PKT(pkts[i]->packet_type) &&
+               } else if (IS_IPV4_VXLAN_FRAG4_PKT(pkts[i]->packet_type) &&
                                do_vxlan_udp_gro) {
                        ret = gro_vxlan_udp4_reassemble(pkts[i],
                                                        &vxlan_udp_tbl, 0);
@@ -296,7 +295,7 @@ rte_gro_reassemble_burst(struct rte_mbuf **pkts,
                                nb_after_gro--;
                        else if (ret < 0)
                                pkts[unprocess_num++] = pkts[i];
-               } else if (IS_IPV4_UDP_PKT(pkts[i]->packet_type) &&
+               } else if (IS_IPV4_FRAG_PKT(pkts[i]->packet_type) &&
                                do_udp4_gro) {
                        ret = gro_udp4_reassemble(pkts[i], &udp_tbl, 0);
                        if (ret > 0)
@@ -394,7 +393,7 @@ rte_gro_reassemble(struct rte_mbuf **pkts,
                        if (gro_vxlan_tcp4_reassemble(pkts[i], vxlan_tcp_tbl,
                                                current_time) < 0)
                                pkts[unprocess_num++] = pkts[i];
-               } else if (IS_IPV4_VXLAN_UDP4_PKT(pkts[i]->packet_type) &&
+               } else if (IS_IPV4_VXLAN_FRAG4_PKT(pkts[i]->packet_type) &&
                                do_vxlan_udp_gro) {
                        if (gro_vxlan_udp4_reassemble(pkts[i], vxlan_udp_tbl,
                                                current_time) < 0)
@@ -404,7 +403,7 @@ rte_gro_reassemble(struct rte_mbuf **pkts,
                        if (gro_tcp4_reassemble(pkts[i], tcp_tbl,
                                                current_time) < 0)
                                pkts[unprocess_num++] = pkts[i];
-               } else if (IS_IPV4_UDP_PKT(pkts[i]->packet_type) &&
+               } else if (IS_IPV4_FRAG_PKT(pkts[i]->packet_type) &&
                                do_udp4_gro) {
                        if (gro_udp4_reassemble(pkts[i], udp_tbl,
                                                current_time) < 0)
-- 
2.43.0

Reply via email to