Skip to content

Commit 87dced9

Browse files
author
Harrison Caldicott
committed
ip: use canonical hashes for L3 flow identity
The fib4/fib6 ECMP lookups in input, ICMP, error, NAT, IP-in-IP and SRv6 paths read m->hash.rss directly. On ports without RSS the field holds stale or zero data, so multipath selection is either unstable or collapses onto a single nexthop, and locally generated packets never had a meaningful hash at all. Use gr_mbuf_flow_hash_get_l3() in these nodes so every lookup shares the packet's cached canonical hash regardless of how it entered the graph. Extend the load balance smoke test to send distinct UDP flows through an ECMP route and check that both group members carry traffic. Signed-off-by: Harrison Caldicott <harrison@itsfubar.com.au>
1 parent f6b19c7 commit 87dced9

12 files changed

Lines changed: 86 additions & 18 deletions

File tree

modules/ip/datapath/icmp_local_send.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Copyright (c) 2024 Christophe Fontaine
33

44
#include "control_input.h"
5+
#include "flow_hash.h"
56
#include "graph.h"
67
#include "iface.h"
78
#include "ip4.h"
@@ -109,10 +110,9 @@ static uint16_t icmp_local_send_process(
109110
icmp->icmp_seq_nb = rte_cpu_to_be_16(msg.seq_num);
110111
icmp->icmp_ident = rte_cpu_to_be_16(msg.ident);
111112

112-
// Fake RSS to spread the traffic
113-
// for ECMP routes or active/active bonds.
114-
mbuf->hash.rss = msg.ident;
115-
mbuf->ol_flags |= RTE_MBUF_F_RX_RSS_HASH;
113+
// Seed canonical flow identity to spread locally generated traffic
114+
// across ECMP routes or active/active bonds.
115+
gr_mbuf_flow_hash_set(mbuf, msg.ident);
116116

117117
data = ip_local_mbuf_data(mbuf);
118118
data->proto = IPPROTO_ICMP;

modules/ip/datapath/icmp_output.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: BSD-3-Clause
22
// Copyright (c) 2024 Robin Jarry
33

4+
#include "flow_hash.h"
45
#include "graph.h"
56
#include "ip4.h"
67
#include "ip4_datapath.h"
@@ -44,7 +45,11 @@ icmp_output_process(struct rte_graph *graph, struct rte_node *node, void **objs,
4445
goto next;
4546
}
4647
ip_set_fields(ip, local_data);
47-
nh = fib4_lookup(local_data->vrf_id, local_data->dst, mbuf->hash.rss);
48+
nh = fib4_lookup(
49+
local_data->vrf_id,
50+
local_data->dst,
51+
gr_mbuf_flow_hash_get_l3(mbuf, RTE_BE16(RTE_ETHER_TYPE_IPV4))
52+
);
4853
if (nh == NULL) {
4954
// Do not let packets go to ip_output from icmp_output
5055
// with no available route to avoid loops of destination

modules/ip/datapath/ip_error.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: BSD-3-Clause
22
// Copyright (c) 2024 Christophe Fontaine
33

4+
#include "flow_hash.h"
45
#include "graph.h"
56
#include "ip4.h"
67
#include "ip4_datapath.h"
@@ -33,13 +34,15 @@ ip_error_process(struct rte_graph *graph, struct rte_node *node, void **objs, ui
3334
struct rte_ipv4_hdr *ip;
3435
struct rte_mbuf *mbuf;
3536
ip4_addr_t src, dst;
37+
uint32_t hash;
3638
rte_edge_t edge;
3739
unsigned len;
3840

3941
for (uint16_t i = 0; i < nb_objs; i++) {
4042
mbuf = objs[i];
4143

4244
ip = rte_pktmbuf_mtod(mbuf, struct rte_ipv4_hdr *);
45+
hash = gr_mbuf_flow_hash_get_l3(mbuf, RTE_BE16(RTE_ETHER_TYPE_IPV4));
4346
src = ip->src_addr;
4447
// RFC792 payload size: ip header + 64 bits of original datagram
4548
len = rte_ipv4_hdr_len(ip) + 8;
@@ -53,8 +56,7 @@ ip_error_process(struct rte_graph *graph, struct rte_node *node, void **objs, ui
5356

5457
// Get the local router IP address from the input iface
5558
iface = l3_mbuf_data(mbuf)->iface;
56-
if (iface == NULL
57-
|| (nh = fib4_lookup(iface->vrf_id, src, mbuf->hash.rss)) == NULL) {
59+
if (iface == NULL || (nh = fib4_lookup(iface->vrf_id, src, hash)) == NULL) {
5860
edge = NO_IP;
5961
goto next;
6062
}

modules/ip/datapath/ip_input.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#include "conntrack.h"
55
#include "eth.h"
6+
#include "flow_hash.h"
67
#include "graph.h"
78
#include "ip4.h"
89
#include "ip4_datapath.h"
@@ -141,7 +142,11 @@ ip_input_process(struct rte_graph *graph, struct rte_node *node, void **objs, ui
141142
goto next;
142143
}
143144

144-
nh = fib4_lookup(iface->vrf_id, ip->dst_addr, mbuf->hash.rss);
145+
nh = fib4_lookup(
146+
iface->vrf_id,
147+
ip->dst_addr,
148+
gr_mbuf_flow_hash_get_l3(mbuf, RTE_BE16(RTE_ETHER_TYPE_IPV4))
149+
);
145150
if (nh == NULL) {
146151
edge = NO_ROUTE;
147152
goto next;
@@ -248,6 +253,7 @@ mock_func(int, drop_format(char *, size_t, const void *, size_t));
248253
mock_func(int, trace_ip_format(char *, size_t, const struct rte_ipv4_hdr *, size_t));
249254
mock_func(void, gr_eth_input_add_type(rte_be16_t, const char *));
250255
mock_func(void, loopback_input_add_type(rte_be16_t, const char *));
256+
mock_func(uint32_t, gr_mbuf_flow_hash_get_l3(struct rte_mbuf *, rte_be16_t));
251257
mock_func(
252258
bool,
253259
gr_conn_parse_key(
@@ -366,6 +372,7 @@ static void ip_input_conntrack_dnat(void **) {
366372
l3->flags = GR_NH_F_LOCAL;
367373
l3->ipv4 = fake_mbuf.ipv4_hdr.dst_addr;
368374
will_return(fib4_lookup, &nh);
375+
will_return(gr_mbuf_flow_hash_get_l3, 0);
369376

370377
iface.flags |= GR_IFACE_F_SNAT_DYNAMIC;
371378
struct conn conn;

modules/ip6/datapath/icmp6_local_send.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Copyright (c) 2025 Olivier Gournet
33

44
#include "control_input.h"
5+
#include "flow_hash.h"
56
#include "graph.h"
67
#include "icmp6.h"
78
#include "iface.h"
@@ -105,10 +106,9 @@ static uint16_t icmp6_local_send_process(
105106
icmp6_echo->ident = rte_cpu_to_be_16(msg.ident);
106107
icmp6_echo->seqnum = rte_cpu_to_be_16(msg.seq_num);
107108

108-
// Fake RSS to spread the traffic
109-
// for ECMP routes or active/active bonds.
110-
mbuf->hash.rss = msg.ident;
111-
mbuf->ol_flags |= RTE_MBUF_F_RX_RSS_HASH;
109+
// Seed canonical flow identity to spread locally generated traffic
110+
// across ECMP routes or active/active bonds.
111+
gr_mbuf_flow_hash_set(mbuf, msg.ident);
112112

113113
payload = PAYLOAD(icmp6_echo);
114114
*payload = gr_clock_ns();

modules/ip6/datapath/icmp6_output.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: BSD-3-Clause
22
// Copyright (c) 2024 Robin Jarry
33

4+
#include "flow_hash.h"
45
#include "graph.h"
56
#include "icmp6.h"
67
#include "ip6.h"
@@ -58,7 +59,12 @@ static uint16_t icmp6_output_process(
5859
if (rte_ipv6_addr_is_mcast(&d->dst))
5960
nh = nh6_lookup(d->iface->vrf_id, d->iface->id, &d->src);
6061
else
61-
nh = fib6_lookup(d->iface->vrf_id, d->iface->id, &d->dst, mbuf->hash.rss);
62+
nh = fib6_lookup(
63+
d->iface->vrf_id,
64+
d->iface->id,
65+
&d->dst,
66+
gr_mbuf_flow_hash_get_l3(mbuf, RTE_BE16(RTE_ETHER_TYPE_IPV6))
67+
);
6268

6369
if (nh == NULL) {
6470
edge = NO_ROUTE;

modules/ip6/datapath/ip6_input.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Copyright (c) 2024 Robin Jarry
33

44
#include "eth.h"
5+
#include "flow_hash.h"
56
#include "graph.h"
67
#include "ip6.h"
78
#include "ip6_datapath.h"
@@ -119,7 +120,12 @@ ip6_input_process(struct rte_graph *graph, struct rte_node *node, void **objs, u
119120
goto next;
120121
}
121122

122-
nh = fib6_lookup(iface->vrf_id, iface->id, &ip->dst_addr, mbuf->hash.rss);
123+
nh = fib6_lookup(
124+
iface->vrf_id,
125+
iface->id,
126+
&ip->dst_addr,
127+
gr_mbuf_flow_hash_get_l3(mbuf, RTE_BE16(RTE_ETHER_TYPE_IPV6))
128+
);
123129
if (nh == NULL) {
124130
edge = DEST_UNREACH;
125131
goto next;
@@ -212,6 +218,7 @@ mock_func(int, drop_format(char *, size_t, const void *, size_t));
212218
mock_func(int, trace_ip6_format(char *, size_t, const struct rte_ipv6_hdr *, size_t));
213219
mock_func(void, gr_eth_input_add_type(rte_be16_t, const char *));
214220
mock_func(void, loopback_input_add_type(rte_be16_t, const char *));
221+
mock_func(uint32_t, gr_mbuf_flow_hash_get_l3(struct rte_mbuf *, rte_be16_t));
215222
mock_func(struct nexthop *, mcast6_get_member(uint16_t, const struct rte_ipv6_addr *));
216223

217224
struct fake_mbuf {

modules/ipip/datapath_out.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: BSD-3-Clause
22
// Copyright (c) 2024 Robin Jarry
33

4+
#include "flow_hash.h"
45
#include "graph.h"
56
#include "ip4.h"
67
#include "ip4_datapath.h"
@@ -32,6 +33,7 @@ ipip_output_process(struct rte_graph *graph, struct rte_node *node, void **objs,
3233
struct rte_ipv4_hdr *outer;
3334
const struct iface *iface;
3435
struct rte_mbuf *mbuf;
36+
uint32_t hash;
3537
rte_edge_t edge;
3638

3739
IFACE_STATS_VARS(tx, self);
@@ -59,6 +61,7 @@ ipip_output_process(struct rte_graph *graph, struct rte_node *node, void **objs,
5961

6062
// Encapsulate with another IPv4 header.
6163
inner = rte_pktmbuf_mtod(mbuf, const struct rte_ipv4_hdr *);
64+
hash = gr_mbuf_flow_hash_get_l3(mbuf, RTE_BE16(RTE_ETHER_TYPE_IPV4));
6265
tunnel.src = ipip->local;
6366
tunnel.dst = ipip->remote;
6467
tunnel.len = rte_be_to_cpu_16(inner->total_length);
@@ -75,7 +78,7 @@ ipip_output_process(struct rte_graph *graph, struct rte_node *node, void **objs,
7578
IFACE_STATS_INC(tx, self, mbuf, iface);
7679

7780
// Resolve nexthop for the encapsulated packet.
78-
ip_data->nh = fib4_lookup(iface->vrf_id, ipip->remote, mbuf->hash.rss);
81+
ip_data->nh = fib4_lookup(iface->vrf_id, ipip->remote, hash);
7982
edge = IP_OUTPUT;
8083

8184
next:

modules/policy/datapath/dnat44_dynamic.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Copyright (c) 2025 Robin Jarry
33

44
#include "conntrack.h"
5+
#include "flow_hash.h"
56
#include "graph.h"
67
#include "ip4.h"
78
#include "l3.h"
@@ -91,7 +92,11 @@ static uint16_t dnat44_dynamic_process(
9192
);
9293

9394
o = l3_mbuf_data(m);
94-
o->nh = fib4_lookup(o->iface->vrf_id, ip->dst_addr, m->hash.rss);
95+
o->nh = fib4_lookup(
96+
o->iface->vrf_id,
97+
ip->dst_addr,
98+
gr_mbuf_flow_hash_get_l3(m, RTE_BE16(RTE_ETHER_TYPE_IPV4))
99+
);
95100

96101
if (o->nh == NULL)
97102
edge = NO_ROUTE;

modules/policy/datapath/dnat44_static.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: BSD-3-Clause
22
// Copyright (c) 2025 Robin Jarry
33

4+
#include "flow_hash.h"
45
#include "graph.h"
56
#include "ip4.h"
67
#include "ip4_datapath.h"
@@ -74,7 +75,11 @@ static uint16_t dnat44_static_process(
7475
// We need the old address value to fixup the checksum properly.
7576
ip->dst_addr = dnat->replace;
7677

77-
d->nh = fib4_lookup(d->iface->vrf_id, ip->dst_addr, mbuf->hash.rss);
78+
d->nh = fib4_lookup(
79+
d->iface->vrf_id,
80+
ip->dst_addr,
81+
gr_mbuf_flow_hash_get_l3(mbuf, RTE_BE16(RTE_ETHER_TYPE_IPV4))
82+
);
7883

7984
if (d->nh == NULL)
8085
edge = NO_ROUTE;

0 commit comments

Comments
 (0)