This patch introduces a new API to the offload provider framework that
allows hardware offload implementations to control hash value calculation
for the OVS_ACTION_ATTR_HASH action.
Background and Motivation
=========================
The OVS hash action (OVS_ACTION_ATTR_HASH) is used to compute a hash value
from packet header fields, primarily for load balancing across multiple
paths using the select group action. The hash value is stored in the
packet's metadata and used by subsequent actions to distribute flows
across multiple output ports.
However, hardware offload implementations may require different approaches
to hash calculation:
1. Hardware NICs may use different hash functions or hash inputs than
the software datapath, which can lead to inconsistent load distribution
when mixing hardware and software paths.
2. Some hardware may support enhanced hashing mechanisms (e.g., using
symmetric hashing for bidirectional flows or hardware-specific hash
engines) that provide better load distribution than the default
software implementation.
Design
======
This patch adds a new optional callback to the dpif_offload_class:
bool (*netdev_get_dp_hash)(const struct dpif_offload *,
const struct netdev *ingress_netdev,
struct dp_packet *,
const struct ovs_action_hash *, uint32_t *hash);
To integrate this into the action execution path, a new optional callback
type, odp_hash_cb, is passed to odp_execute_actions() to allow per-packet
hash overrides during OVS_ACTION_ATTR_HASH processing. The dpif-netdev
datapath provides an implementation that calls the offload provider's
netdev_get_dp_hash when hardware offload is enabled and the original
ingress port is known. If the provider returns true, the returned hash
value is used; otherwise, OVS falls back to the standard hash calculation.
Signed-off-by: Eelco Chaudron <[email protected]>
---
lib/dpif-netdev.c | 33 ++++++++++++-
lib/dpif-offload-dummy.c | 98 +++++++++++++++++++++++++++++++++++++
lib/dpif-offload-provider.h | 12 +++++
lib/dpif-offload.c | 19 +++++++
lib/dpif-offload.h | 4 ++
lib/dpif.c | 2 +-
lib/odp-execute.c | 42 +++++++++++-----
lib/odp-execute.h | 13 ++++-
tests/dpif-netdev.at | 47 ++++++++++++++++++
9 files changed, 255 insertions(+), 15 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index a9bd27573..3cb34ba1a 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -7708,6 +7708,8 @@ dp_netdev_recirculate(struct dp_netdev_pmd_thread *pmd,
struct dp_netdev_execute_aux {
struct dp_netdev_pmd_thread *pmd;
const struct flow *flow;
+ struct netdev *cached_in_netdev;
+ odp_port_t cached_in_odpp;
};
static void
@@ -7954,6 +7956,33 @@ dp_execute_lb_output_action(struct dp_netdev_pmd_thread
*pmd,
}
}
+static bool
+dp_hash_override(void *aux_, struct dp_packet *packet,
+ const struct ovs_action_hash *hash_act, uint32_t *hash)
+{
+ struct dp_netdev_execute_aux *aux = aux_;
+ struct dp_netdev_pmd_thread *pmd = aux->pmd;
+
+ if (!dpif_offload_enabled()) {
+ return false;
+ }
+
+ if (aux->cached_in_odpp != packet->md.orig_in_port
+ || !aux->cached_in_netdev) {
+ struct tx_port *in_port = pmd_send_port_cache_lookup(
+ pmd, packet->md.orig_in_port);
+
+ if (!in_port) {
+ return false;
+ }
+ aux->cached_in_odpp = packet->md.orig_in_port;
+ aux->cached_in_netdev = in_port->port->netdev;
+ }
+
+ return dpif_offload_netdev_get_dp_hash(aux->cached_in_netdev, packet,
+ hash_act, hash);
+}
+
static void
dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
const struct nlattr *a, bool should_steal)
@@ -8290,10 +8319,10 @@ dp_netdev_execute_actions(struct dp_netdev_pmd_thread
*pmd,
bool should_steal, const struct flow *flow,
const struct nlattr *actions, size_t actions_len)
{
- struct dp_netdev_execute_aux aux = { pmd, flow };
+ struct dp_netdev_execute_aux aux = { pmd, flow, NULL, ODPP_NONE };
odp_execute_actions(&aux, packets, should_steal, actions,
- actions_len, dp_execute_cb);
+ actions_len, dp_execute_cb, dp_hash_override);
}
struct dp_netdev_ct_dump {
diff --git a/lib/dpif-offload-dummy.c b/lib/dpif-offload-dummy.c
index cc9fc5792..01b20d3b6 100644
--- a/lib/dpif-offload-dummy.c
+++ b/lib/dpif-offload-dummy.c
@@ -640,6 +640,103 @@ dummy_offload_udp_tnl_get_src_port(
return true;
}
+static uint32_t
+fnv1a_add(uint32_t hash, uint32_t word)
+{
+ const uint32_t prime = 16777619U;
+
+ for (size_t i = 0; i < sizeof(uint32_t); i++) {
+ hash ^= (word >> (i * 8)) & 0xff;
+ hash *= prime;
+ }
+ return hash;
+}
+
+static uint32_t
+fnv1a_add64(uint32_t hash, uint64_t word)
+{
+ return fnv1a_add(fnv1a_add(hash, word), word >> 32);
+}
+
+static uint32_t
+fnv1a_init(uint32_t seed)
+{
+ const uint32_t fnv_offset_basis = 2166136261U;
+
+ return seed ? fnv1a_add(fnv_offset_basis, seed) : fnv_offset_basis;
+}
+
+static bool
+get_l4_sym_hash(struct flow *flow, uint32_t seed, uint32_t *hash_)
+{
+ struct ds ds = DS_EMPTY_INITIALIZER;
+ uint32_t hash = fnv1a_init(seed);
+
+ if (flow->dl_type == htons(ETH_TYPE_IP)) {
+ hash = fnv1a_add(hash,
+ (OVS_FORCE uint32_t)(flow->nw_src ^ flow->nw_dst));
+ } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
+ /* IPv6 addresses are 64-bit aligned inside struct flow. */
+ const ovs_be64 *a = ALIGNED_CAST(const ovs_be64 *,
+ flow->ipv6_src.s6_addr);
+ const ovs_be64 *b = ALIGNED_CAST(const ovs_be64 *,
+ flow->ipv6_dst.s6_addr);
+
+ for (int i = 0; i < sizeof flow->ipv6_src / sizeof *a; i++) {
+ hash = fnv1a_add64(hash, ntohll(a[i]) ^ ntohll(b[i]));
+ }
+ } else {
+ return false;
+ }
+
+ hash = fnv1a_add(hash, flow->nw_proto);
+ if (!(flow->nw_frag & FLOW_NW_FRAG_MASK)
+ && (flow->nw_proto == IPPROTO_TCP
+ || flow->nw_proto == IPPROTO_SCTP
+ || flow->nw_proto == IPPROTO_UDP)) {
+ hash = fnv1a_add(hash,
+ (OVS_FORCE uint16_t)(flow->tp_src ^ flow->tp_dst));
+ }
+
+ ds_put_format(&ds, "l4_sym_hash: %8.8x for packet: ", hash);
+ flow_format(&ds, flow, NULL);
+ VLOG_DBG("%s", ds_cstr(&ds));
+ ds_destroy(&ds);
+
+ *hash_ = hash;
+ return true;
+}
+
+static bool
+dummy_offload_get_dp_hash(const struct dpif_offload *offload OVS_UNUSED,
+ const struct netdev *ingress_netdev OVS_UNUSED,
+ struct dp_packet *packet,
+ const struct ovs_action_hash *hash_act,
+ uint32_t *hash)
+{
+ switch ((enum ovs_hash_alg) hash_act->hash_alg) {
+ case OVS_HASH_ALG_L4:
+ case OVS_HASH_ALG_SYM_L4: {
+ /* For our implementation we will just use a symmetric L4 hash.
+ * Note that we will use our own hash that will give consistent results
+ * across all platforms/compilers. */
+ struct flow flow;
+
+ flow_extract(packet, &flow);
+ if (!get_l4_sym_hash(&flow, hash_act->hash_basis, hash)) {
+ return false;
+ }
+ break;
+ }
+
+ case __OVS_HASH_MAX:
+ default:
+ OVS_NOT_REACHED();
+ }
+
+ return true;
+}
+
static bool
dummy_offload_are_all_actions_supported(const struct dpif_offload *offload_,
odp_port_t in_odp,
@@ -1162,6 +1259,7 @@ dummy_netdev_hw_offload_run(struct netdev *netdev)
.get_netdev = dummy_offload_get_netdev, \
.netdev_hw_post_process = dummy_offload_hw_post_process, \
.netdev_udp_tnl_get_src_port = dummy_offload_udp_tnl_get_src_port, \
+ .netdev_get_dp_hash = dummy_offload_get_dp_hash, \
.netdev_flow_put = dummy_flow_put, \
.netdev_flow_del = dummy_flow_del, \
.netdev_flow_stats = dummy_flow_stats, \
diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h
index 444b13138..5dec51173 100644
--- a/lib/dpif-offload-provider.h
+++ b/lib/dpif-offload-provider.h
@@ -288,6 +288,18 @@ struct dpif_offload_class {
struct dp_packet *packet,
ovs_be16 *src_port);
+ /* Allows the offload provider to override the default dp-hash calculation.
+ * Called during packet processing to determine the hash value for datapath
+ * operations (e.g., load balancing, packet distribution).
+ *
+ * If implemented, should return true and set 'hash' to the desired hash
+ * value. If not implemented or if default behavior is desired, should
+ * return false to use the standard hash calculation. */
+ bool (*netdev_get_dp_hash)(const struct dpif_offload *,
+ const struct netdev *ingress_netdev,
+ struct dp_packet *,
+ const struct ovs_action_hash *, uint32_t *hash);
+
/* Add or modify the specified flow directly in the offload datapath.
* The actual implementation may choose to handle the offload
* asynchronously by returning EINPROGRESS and invoking the supplied
diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c
index 587360c51..21215d874 100644
--- a/lib/dpif-offload.c
+++ b/lib/dpif-offload.c
@@ -1482,6 +1482,25 @@ dpif_offload_netdev_udp_tnl_get_src_port(const struct
netdev *ingress_netdev,
packet, src_port);
}
+bool
+dpif_offload_netdev_get_dp_hash(const struct netdev *ingress_netdev,
+ struct dp_packet *packet,
+ const struct ovs_action_hash *hash_action,
+ uint32_t *hash)
+{
+ const struct dpif_offload *offload;
+
+ offload = ovsrcu_get(const struct dpif_offload *,
+ &ingress_netdev->dpif_offload);
+
+ if (OVS_UNLIKELY(!offload) || !offload->class->netdev_get_dp_hash) {
+ return false;
+ }
+
+ return offload->class->netdev_get_dp_hash(offload, ingress_netdev, packet,
+ hash_action, hash);
+}
+
void
dpif_offload_datapath_register_flow_unreference_cb(
struct dpif *dpif, dpif_offload_flow_unreference_cb *cb)
diff --git a/lib/dpif-offload.h b/lib/dpif-offload.h
index bf7643320..5b377de28 100644
--- a/lib/dpif-offload.h
+++ b/lib/dpif-offload.h
@@ -116,6 +116,10 @@ int dpif_offload_netdev_hw_post_process(struct netdev *,
unsigned pmd_id,
bool dpif_offload_netdev_udp_tnl_get_src_port(const struct netdev *,
struct dp_packet *,
ovs_be16 *src_port);
+bool dpif_offload_netdev_get_dp_hash(const struct netdev *,
+ struct dp_packet *,
+ const struct ovs_action_hash *,
+ uint32_t *hash);
/* Callback invoked when a hardware flow offload operation (put/del) completes.
diff --git a/lib/dpif.c b/lib/dpif.c
index 1afc7c662..dac5df7b3 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1308,7 +1308,7 @@ dpif_execute_with_help(struct dpif *dpif, struct
dpif_execute *execute)
ofpbuf_init(&aux.meter_actions, 0);
dp_packet_batch_init_packet(&pb, execute->packet);
odp_execute_actions(&aux, &pb, false, execute->actions,
- execute->actions_len, dpif_execute_helper_cb);
+ execute->actions_len, dpif_execute_helper_cb, NULL);
ofpbuf_uninit(&aux.meter_actions);
return aux.error;
}
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 4642f3375..afde4f8e5 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -706,7 +706,8 @@ odp_execute_masked_set_action(struct dp_packet *packet,
static void
odp_execute_sample(void *dp, struct dp_packet *packet, bool steal,
const struct nlattr *action,
- odp_execute_cb dp_execute_action)
+ odp_execute_cb dp_execute_action,
+ odp_hash_cb hash_override)
{
const struct nlattr *subactions = NULL;
const struct nlattr *a;
@@ -756,13 +757,15 @@ odp_execute_sample(void *dp, struct dp_packet *packet,
bool steal,
}
dp_packet_batch_init_packet(&pb, packet);
odp_execute_actions(dp, &pb, true, nl_attr_get(subactions),
- nl_attr_get_size(subactions), dp_execute_action);
+ nl_attr_get_size(subactions), dp_execute_action,
+ hash_override);
}
static void
odp_execute_clone(void *dp, struct dp_packet_batch *batch, bool steal,
const struct nlattr *actions,
- odp_execute_cb dp_execute_action)
+ odp_execute_cb dp_execute_action,
+ odp_hash_cb hash_override)
{
if (!steal) {
/* The 'actions' may modify the packet, but the modification
@@ -774,18 +777,21 @@ odp_execute_clone(void *dp, struct dp_packet_batch
*batch, bool steal,
dp_packet_batch_clone(&clone_pkt_batch, batch);
dp_packet_batch_reset_cutlen(batch);
odp_execute_actions(dp, &clone_pkt_batch, true, nl_attr_get(actions),
- nl_attr_get_size(actions), dp_execute_action);
+ nl_attr_get_size(actions), dp_execute_action,
+ hash_override);
}
else {
odp_execute_actions(dp, batch, true, nl_attr_get(actions),
- nl_attr_get_size(actions), dp_execute_action);
+ nl_attr_get_size(actions), dp_execute_action,
+ hash_override);
}
}
static void
odp_execute_check_pkt_len(void *dp, struct dp_packet *packet, bool steal,
const struct nlattr *action,
- odp_execute_cb dp_execute_action)
+ odp_execute_cb dp_execute_action,
+ odp_hash_cb hash_override)
{
static const struct nl_policy ovs_cpl_policy[] = {
[OVS_CHECK_PKT_LEN_ATTR_PKT_LEN] = { .type = NL_A_U16 },
@@ -842,7 +848,7 @@ odp_execute_check_pkt_len(void *dp, struct dp_packet
*packet, bool steal,
* odp_execute_actions. */
dp_packet_batch_init_packet(&pb, packet);
odp_execute_actions(dp, &pb, true, nl_attr_get(a), nl_attr_get_size(a),
- dp_execute_action);
+ dp_execute_action, hash_override);
}
static bool
@@ -929,7 +935,8 @@ requires_datapath_assistance(const struct nlattr *a)
void
odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,
const struct nlattr *actions, size_t actions_len,
- odp_execute_cb dp_execute_action)
+ odp_execute_cb dp_execute_action,
+ odp_hash_cb hash_override)
{
struct dp_packet *packet;
const struct nlattr *a;
@@ -989,6 +996,12 @@ odp_execute_actions(void *dp, struct dp_packet_batch
*batch, bool steal,
uint32_t hash;
DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
+ /* Allow the datapath to override the hash. */
+ if (hash_override
+ && hash_override(dp, packet, hash_act, &hash)) {
+ packet->md.dp_hash = hash;
+ continue;
+ }
/* RSS hash can be used here instead of 5tuple for
* performance reasons. */
if (dp_packet_rss_valid(packet)) {
@@ -1007,6 +1020,12 @@ odp_execute_actions(void *dp, struct dp_packet_batch
*batch, bool steal,
uint32_t hash;
DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
+ /* Allow the datapath to override the hash. */
+ if (hash_override
+ && hash_override(dp, packet, hash_act, &hash)) {
+ packet->md.dp_hash = hash;
+ continue;
+ }
flow_extract(packet, &flow);
hash = flow_hash_symmetric_l3l4(&flow,
hash_act->hash_basis,
@@ -1052,7 +1071,7 @@ odp_execute_actions(void *dp, struct dp_packet_batch
*batch, bool steal,
case OVS_ACTION_ATTR_SAMPLE:
DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
odp_execute_sample(dp, packet, steal && last_action, a,
- dp_execute_action);
+ dp_execute_action, hash_override);
}
if (last_action) {
@@ -1075,7 +1094,8 @@ odp_execute_actions(void *dp, struct dp_packet_batch
*batch, bool steal,
case OVS_ACTION_ATTR_CLONE:
odp_execute_clone(dp, batch, steal && last_action, a,
- dp_execute_action);
+ dp_execute_action,
+ hash_override);
if (last_action) {
/* We do not need to free the packets. odp_execute_clone() has
* stolen them. */
@@ -1134,7 +1154,7 @@ odp_execute_actions(void *dp, struct dp_packet_batch
*batch, bool steal,
case OVS_ACTION_ATTR_CHECK_PKT_LEN:
DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
odp_execute_check_pkt_len(dp, packet, steal && last_action, a,
- dp_execute_action);
+ dp_execute_action, hash_override);
}
if (last_action) {
diff --git a/lib/odp-execute.h b/lib/odp-execute.h
index 7a54fa6ec..8e6395190 100644
--- a/lib/odp-execute.h
+++ b/lib/odp-execute.h
@@ -24,11 +24,21 @@
#include "openvswitch/types.h"
struct nlattr;
+struct dp_packet;
struct dp_packet_batch;
+struct ovs_action_hash;
typedef void (*odp_execute_cb)(void *dp, struct dp_packet_batch *batch,
const struct nlattr *action, bool should_steal);
+/* Optional callback that allows a datapath to override the default dp-hash
+ * calculation for OVS_ACTION_ATTR_HASH. Called per-packet during hash action
+ * processing. Should return true and set '*hash' if the override is applied,
+ * or false to fall back to the default software hash. */
+typedef bool (*odp_hash_cb)(void *dp, struct dp_packet *packet,
+ const struct ovs_action_hash *hash_act,
+ uint32_t *hash);
+
/* Actions that need to be executed in the context of a datapath are handed
* to 'dp_execute_action', if non-NULL. Currently this is called only for
* actions OVS_ACTION_ATTR_OUTPUT and OVS_ACTION_ATTR_USERSPACE so
@@ -36,7 +46,8 @@ typedef void (*odp_execute_cb)(void *dp, struct
dp_packet_batch *batch,
void odp_execute_actions(void *dp, struct dp_packet_batch *batch,
bool steal,
const struct nlattr *actions, size_t actions_len,
- odp_execute_cb dp_execute_action);
+ odp_execute_cb dp_execute_action,
+ odp_hash_cb hash_override);
#define odp_get_key_mask(a, type) ((const type *)(const void *)(a + 1) + 1)
diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index 048b826cb..199c85a71 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -662,6 +662,53 @@
arp,in_port=ANY,dl_vlan=11,dl_vlan_pcp=7,vlan_tci1=0x0000,dl_src=00:06:07:08:09:
DPIF_NETDEV_FLOW_HW_OFFLOAD_OFFSETS_VID_ARP([dummy])
DPIF_NETDEV_FLOW_HW_OFFLOAD_OFFSETS_VID_ARP([dummy-pmd])
+AT_SETUP([dpif-netdev - partial hw offload - dp_hash - dummy-pmd])
+OVS_VSWITCHD_START(
+ [add-port br0 p1 -- \
+ add-port br0 p2 -- \
+ set interface p1 type=dummy-pmd ofport_request=1 options:ifindex=1100 -- \
+ set interface p2 type=dummy-pmd ofport_request=2 options:ifindex=1200 \
+ options:tx_pcap=p2.pcap -- \
+ set bridge br0 datapath-type=dummy other-config:datapath-id=1234 \
+ fail-mode=secure], [], [], [--dummy-numa="0"])
+AT_CHECK([ovs-appctl vlog/set dpif_offload_dummy:file:dbg])
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
+OVS_WAIT_UNTIL([grep "Flow HW offload is enabled" ovs-vswitchd.log])
+
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-group br0 \
+ 'group_id=123,type=select,bucket=output:p2,output:p2'])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-flow br0 'ip actions=group:123'])
+
+packet="in_port(1),"\
+"eth(src=50:54:00:00:00:07,dst=50:54:00:00:00:1),eth_type(0x0800),"\
+"ipv4(src=192.168.1.1,dst=192.168.1.100,proto=6,tos=0,ttl=128,frag=no),"\
+"tcp(src=1000,dst=1000)"
+
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 $packet])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 $packet])
+
+AT_CHECK([ovs-appctl dpctl/dump-flows -m | strip_hw_offload], [0], [dnl
+recirc_id(0),in_port(p1),packet_type(ns=0,id=0),eth(),eth_type(0x0800),ipv4(),tcp(src=1000/0,dst=1000/0),
packets:1, bytes:118, used:0.0s, offloaded:partial, dp:ovs,
actions:hash(sym_l4(0)),recirc(0x1)
+recirc_id(0x1),dp_hash(0x34a08190/0xf),in_port(p1),packet_type(ns=0,id=0),eth(),eth_type(0x0800),ipv4(),tcp(src=1000/0,dst=1000/0),
packets:1, bytes:118, used:0.0s, offloaded:yes, dp:dummy, actions:p2
+])
+
+OVS_WAIT_UNTIL([grep "l4_sym_hash: 34a08190 for packet: tcp,in_port=1," \
+ ovs-vswitchd.log])
+
+AT_CHECK(
+ [ovs-appctl --format json dpif/offload/show \
+ | sed 's/.*"p1":{\([[^}]]*\)}.*/\1/; s/,/\n/g; s/"//g' \
+ | sed -n '/^rx_offload_/p' | sort], [0], [dnl
+rx_offload_full:0
+rx_offload_miss:1
+rx_offload_partial:1
+rx_offload_pipe_abort:0
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([dpif-netdev - full hw offload - dummy-pmd])
OVS_VSWITCHD_START(
[add-port br0 p1 -- \
--
2.54.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
