When IP layer fragments packets, L4 header
is only present in the first packet. The next fragmented
packets donot contain L4 headers. So hash calculation based
on 5tuple should ignore the L4 headers for all fragmented
packets.
If we consider L4 header for any fragments , say first or
subsequent ones, the hash will vary which may result in
selecting different dpdk port for egressing out packet.

Signed-off-by: Somnath Chatterjee <somnath.b.chatter...@ericsson.com>
---
 lib/flow.c             |  41 +++++++----
 tests/automake.mk      |   1 +
 tests/library.at       |   4 +
 tests/test-flow-hash.c | 161 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 193 insertions(+), 14 deletions(-)
 create mode 100644 tests/test-flow-hash.c

diff --git a/lib/flow.c b/lib/flow.c
index c2e1e4ad6..464678012 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1075,10 +1075,12 @@ miniflow_extract(struct dp_packet *packet, struct 
miniflow *dst)
                     miniflow_push_be16(mf, tp_dst, tcp->tcp_dst);
                     miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
                     miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
-                    if (dl_type == htons(ETH_TYPE_IP)) {
-                        dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
-                    } else if (dl_type == htons(ETH_TYPE_IPV6)) {
-                        dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
+                    if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
+                        if (dl_type == htons(ETH_TYPE_IP)) {
+                            dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+                        } else if (dl_type == htons(ETH_TYPE_IPV6)) {
+                            dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
+                        }
                     }
                     dp_packet_ol_l4_csum_check_partial(packet);
                     if (dp_packet_l4_checksum_good(packet)
@@ -1095,10 +1097,12 @@ miniflow_extract(struct dp_packet *packet, struct 
miniflow *dst)
                 miniflow_push_be16(mf, tp_dst, udp->udp_dst);
                 miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
                 miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
-                if (dl_type == htons(ETH_TYPE_IP)) {
-                    dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
-                } else if (dl_type == htons(ETH_TYPE_IPV6)) {
-                    dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
+                if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
+                    if (dl_type == htons(ETH_TYPE_IP)) {
+                        dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+                    } else if (dl_type == htons(ETH_TYPE_IPV6)) {
+                        dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
+                    }
                 }
                 dp_packet_ol_l4_csum_check_partial(packet);
                 if (dp_packet_l4_checksum_good(packet)
@@ -2357,6 +2361,7 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)
     if (flow) {
         ovs_be16 dl_type = MINIFLOW_GET_BE16(flow, dl_type);
         uint8_t nw_proto;
+        uint8_t nw_frag;
 
         if (dl_type == htons(ETH_TYPE_IPV6)) {
             struct flowmap map = FLOWMAP_EMPTY_INITIALIZER;
@@ -2377,10 +2382,12 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)
         }
 
         nw_proto = MINIFLOW_GET_U8(flow, nw_proto);
+        nw_frag = MINIFLOW_GET_U8(flow, nw_frag);
         hash = hash_add(hash, nw_proto);
-        if (nw_proto != IPPROTO_TCP && nw_proto != IPPROTO_UDP
+        if ((nw_frag & FLOW_NW_FRAG_MASK) ||
+            (nw_proto != IPPROTO_TCP && nw_proto != IPPROTO_UDP
             && nw_proto != IPPROTO_SCTP && nw_proto != IPPROTO_ICMP
-            && nw_proto != IPPROTO_ICMPV6) {
+            && nw_proto != IPPROTO_ICMPV6)) {
             goto out;
         }
 
@@ -2420,9 +2427,10 @@ flow_hash_5tuple(const struct flow *flow, uint32_t basis)
         }
 
         hash = hash_add(hash, flow->nw_proto);
-        if (flow->nw_proto != IPPROTO_TCP && flow->nw_proto != IPPROTO_UDP
+        if ((flow->nw_frag & FLOW_NW_FRAG_MASK) ||
+            (flow->nw_proto != IPPROTO_TCP && flow->nw_proto != IPPROTO_UDP
             && flow->nw_proto != IPPROTO_SCTP && flow->nw_proto != IPPROTO_ICMP
-            && flow->nw_proto != IPPROTO_ICMPV6) {
+            && flow->nw_proto != IPPROTO_ICMPV6)) {
             goto out;
         }
 
@@ -2430,6 +2438,7 @@ flow_hash_5tuple(const struct flow *flow, uint32_t basis)
         hash = hash_add(hash,
                         ((const uint32_t *)flow)[offsetof(struct flow, tp_src)
                                                  / sizeof(uint32_t)]);
+
     }
 out:
     return hash_finish(hash, 42); /* Arbitrary number. */
@@ -2467,7 +2476,9 @@ flow_hash_symmetric_l4(const struct flow *flow, uint32_t 
basis)
     if (fields.eth_type == htons(ETH_TYPE_IP)) {
         fields.ipv4_addr = flow->nw_src ^ flow->nw_dst;
         fields.ip_proto = flow->nw_proto;
-        if (fields.ip_proto == IPPROTO_TCP || fields.ip_proto == IPPROTO_SCTP) 
{
+        if (!(flow->nw_frag & FLOW_NW_FRAG_MASK) &&
+            (fields.ip_proto == IPPROTO_TCP ||
+            fields.ip_proto == IPPROTO_SCTP)) {
             fields.tp_port = flow->tp_src ^ flow->tp_dst;
         }
     } else if (fields.eth_type == htons(ETH_TYPE_IPV6)) {
@@ -2479,7 +2490,9 @@ flow_hash_symmetric_l4(const struct flow *flow, uint32_t 
basis)
             ipv6_addr[i] = a[i] ^ b[i];
         }
         fields.ip_proto = flow->nw_proto;
-        if (fields.ip_proto == IPPROTO_TCP || fields.ip_proto == IPPROTO_SCTP) 
{
+        if (!(flow->nw_frag & FLOW_NW_FRAG_MASK) &&
+            (fields.ip_proto == IPPROTO_TCP ||
+            fields.ip_proto == IPPROTO_SCTP)) {
             fields.tp_port = flow->tp_src ^ flow->tp_dst;
         }
     }
diff --git a/tests/automake.mk b/tests/automake.mk
index 59f538761..7586d75f7 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -458,6 +458,7 @@ tests_ovstest_SOURCES = \
        tests/test-cooperative-multitasking.c \
        tests/test-csum.c \
        tests/test-flows.c \
+        tests/test-flow-hash.c \
        tests/test-hash.c \
        tests/test-heap.c \
        tests/test-hindex.c \
diff --git a/tests/library.at b/tests/library.at
index 82ac80a27..a0f5f30fa 100644
--- a/tests/library.at
+++ b/tests/library.at
@@ -308,3 +308,7 @@ AT_CHECK([ovstest 
test-cooperative-multitasking-nested-yield], [0], [], [dnl
 cooperative_multitasking|ERR|Nested yield avoided, this is a bug! Enable debug 
logging for more details.
 ])
 AT_CLEANUP
+
+AT_SETUP([test-flow-hash module])
+AT_CHECK([ovstest test-flow-hash], [0], [], [ignore])
+AT_CLEANUP
diff --git a/tests/test-flow-hash.c b/tests/test-flow-hash.c
new file mode 100644
index 000000000..1d8e92a98
--- /dev/null
+++ b/tests/test-flow-hash.c
@@ -0,0 +1,161 @@
+#include <config.h>
+#include <string.h>
+#include "lib/flow.h"
+#include "lib/dp-packet.h"
+#include "tests/ovstest.h"
+#include "util.h"
+#include "openvswitch/vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(test_flow_hash);
+
+struct packet_hashes {
+    uint32_t flow_hash;
+    uint32_t miniflow_hash;
+};
+
+/* Utility to print hex dump of a packet */
+static void
+log_hex_dump(const char *label, const uint8_t *data, size_t len)
+{
+    char line[128];
+    size_t i;
+
+    VLOG_INFO("%s (len=%lu):", label, (unsigned long) len);
+    for (i = 0; i < len; i += 16) {
+        size_t j, line_len = 0;
+        line_len += snprintf(line + line_len, sizeof(line) - line_len,
+                             "  %04x: ", i);
+        for (j = 0; j < 16 && i + j < len; j++) {
+            line_len += snprintf(line + line_len, sizeof(line) - line_len,
+                                 "%02x ", data[i + j]);
+        }
+        VLOG_INFO("%s", line);
+    }
+}
+
+/* Extracts flow and miniflow from packet and computes their 5-tuple hashes */
+static struct packet_hashes
+get_packet_hashes(const void *data, size_t size,
+                  uint32_t basis,
+                  const char *label)
+{
+    struct packet_hashes hashes;
+    struct dp_packet packet;
+    dp_packet_use_const(&packet, data, size);
+
+    struct flow flow;
+    flow_extract(&packet, &flow);
+    hashes.flow_hash = flow_hash_5tuple(&flow, basis);
+
+    struct {
+        struct miniflow miniflow;
+        uint64_t buffer[FLOW_U64S];
+    } mf_buf;
+    miniflow_extract(&packet, &mf_buf.miniflow);
+    hashes.miniflow_hash = miniflow_hash_5tuple(&mf_buf.miniflow, basis);
+
+    const char *frag_type;
+    uint8_t frag_flags = flow.nw_frag & FLOW_NW_FRAG_MASK;
+
+    if (frag_flags == 0) {
+        frag_type = "not fragmented";
+    } else if (frag_flags & FLOW_NW_FRAG_LATER) {
+        frag_type = "non-first fragment";
+    } else {
+        frag_type = "first fragment or unknown";
+    }
+
+    VLOG_INFO("%s:", label);
+    VLOG_INFO("  Src IP: "IP_FMT", Dst IP: "IP_FMT,
+              IP_ARGS(&flow.nw_src),
+              IP_ARGS(&flow.nw_dst));
+    VLOG_INFO("  Src Port: %u, Dst Port: %u, Proto: %u",
+              ntohs(flow.tp_src),
+              ntohs(flow.tp_dst),
+              flow.nw_proto);
+    VLOG_INFO("  Frag Type: %s", frag_type);
+    VLOG_INFO("  flow_hash:     0x%08x", hashes.flow_hash);
+    VLOG_INFO("  miniflow_hash: 0x%08x", hashes.miniflow_hash);
+
+    log_hex_dump(label, data, size);
+
+    return hashes;
+}
+
+static void
+test_udp_fragment_hash_consistency(int argc OVS_UNUSED,
+                                   char *argv[] OVS_UNUSED)
+{
+    const uint32_t basis = 54321;
+
+    /* Packet 1: Normal, unfragmented UDP packet (DF bit set). */
+    const uint8_t normal_packet[] = {
+        0x00,0x00,0x00,0x00,0x00,0x01,0x00,0x00,
+        0x00,0x00,0x00,0x02,0x08,0x00,0x45,0x00,
+        0x00,0x28,0x12,0x34,0x40,0x00,0x40,0x11,
+        0x7c,0xd1,0xc0,0xa8,0x01,0x01,0xc0,0xa8,
+        0x01,0x02,0xc0,0x01,0xc0,0x02,0x00,0x18,
+        0xab,0xcd,'a','b','c','d','e','f','g',
+        'h','i','j','k','l'
+    };
+
+    /* Packet 2: First fragment (MF bit set, offset 0). */
+    const uint8_t first_frag_packet[] = {
+        0x00,0x00,0x00,0x00,0x00,0x01,0x00,0x00,
+        0x00,0x00,0x00,0x02,0x08,0x00,0x45,0x00,
+        0x00,0x1c,0x12,0x34,0x20,0x00,0x40,0x11,
+        0x7c,0xd7,0xc0,0xa8,0x01,0x01,0xc0,0xa8,
+        0x01,0x02,0xc0,0x01,0xc0,0x02,0x00,0x18,
+        0xab,0xcd
+    };
+
+    /* Packet 3: Second/Middle fragment (MF bit set, non-zero offset). */
+    const uint8_t middle_frag_packet[] = {
+        0x00,0x00,0x00,0x00,0x00,0x01,0x00,0x00,
+        0x00,0x00,0x00,0x02,0x08,0x00,0x45,0x00,
+        0x00,0x1c,0x12,0x34,0x20,0x01,0x40,0x11,
+        0x7c,0xd6,0xc0,0xa8,0x01,0x01,0xc0,0xa8,
+        0x01,0x02,'i','j','k','l','m','n','o','p'
+    };
+
+    /* Packet 4: Last fragment (MF bit clear, non-zero offset). */
+    const uint8_t last_frag_packet[] = {
+        0x00,0x00,0x00,0x00,0x00,0x01,0x00,0x00,
+        0x00,0x00,0x00,0x02,0x08,0x00,0x45,0x00,
+        0x00,0x1a,0x12,0x34,0x00,0x02,0x40,0x11,
+        0x9c,0xd5,0xc0,0xa8,0x01,0x01,0xc0,0xa8,
+        0x01,0x02,'q','r','s','t','u','v'
+    };
+
+    struct packet_hashes
+        normal_h = get_packet_hashes(normal_packet,
+                                     sizeof(normal_packet),
+                                     basis,
+                                     "Normal Packet");
+    struct packet_hashes
+        first_frag_h = get_packet_hashes(first_frag_packet,
+                                         sizeof(first_frag_packet),
+                                         basis,
+                                         "First Fragment");
+    struct packet_hashes
+        middle_frag_h = get_packet_hashes(middle_frag_packet,
+                                          sizeof(middle_frag_packet),
+                                          basis,
+                                          "Middle Fragment");
+    struct packet_hashes
+        last_frag_h = get_packet_hashes(last_frag_packet,
+                                        sizeof(last_frag_packet),
+                                        basis,
+                                        "Last Fragment");
+
+    ovs_assert(normal_h.flow_hash      == normal_h.miniflow_hash);
+    ovs_assert(first_frag_h.flow_hash  == first_frag_h.miniflow_hash);
+    ovs_assert(middle_frag_h.flow_hash == middle_frag_h.miniflow_hash);
+    ovs_assert(last_frag_h.flow_hash   == last_frag_h.miniflow_hash);
+
+    ovs_assert(normal_h.flow_hash != first_frag_h.flow_hash);
+    ovs_assert(middle_frag_h.flow_hash == last_frag_h.flow_hash);
+    ovs_assert(first_frag_h.flow_hash == middle_frag_h.flow_hash);
+}
+
+OVSTEST_REGISTER("test-flow-hash", test_udp_fragment_hash_consistency);
-- 
2.45.1.windows.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to