Notice targeted 6.3-rc kernel via bpf git tree.
Current API for bpf_xdp_metadata_rx_hash() returns the raw RSS hash value,
but doesn't provide information on the RSS hash type (part of 6.3-rc).
This patchset proposal is to change the function call signature via adding
a pointer value argument for provide the RSS hash type.
Alternatively we disable bpf_xdp_metadata_rx_hash() in 6.3-rc, and have
more time to nitpick the RSS hash-type bits.
---
Jesper Dangaard Brouer (5):
xdp: rss hash types representation
mlx5: bpf_xdp_metadata_rx_hash add xdp rss hash type
veth: bpf_xdp_metadata_rx_hash add xdp rss hash type
mlx4: bpf_xdp_metadata_rx_hash add xdp rss hash type
selftests/bpf: Adjust bpf_xdp_metadata_rx_hash for new arg
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 22 ++++++-
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 3 +-
.../net/ethernet/mellanox/mlx5/core/en/xdp.c | 63 ++++++++++++++++++-
drivers/net/veth.c | 11 +++-
include/linux/mlx5/device.h | 14 ++++-
include/linux/netdevice.h | 3 +-
include/net/xdp.h | 49 +++++++++++++++
net/core/xdp.c | 10 ++-
.../selftests/bpf/prog_tests/xdp_metadata.c | 2 +
.../selftests/bpf/progs/xdp_hw_metadata.c | 14 +++--
.../selftests/bpf/progs/xdp_metadata.c | 6 +-
.../selftests/bpf/progs/xdp_metadata2.c | 7 ++-
tools/testing/selftests/bpf/xdp_hw_metadata.c | 2 +-
tools/testing/selftests/bpf/xdp_metadata.h | 1 +
14 files changed, 183 insertions(+), 24 deletions(-)
--
The RSS hash type specifies what portion of packet data NIC hardware used
when calculating RSS hash value. The RSS types are focused on Internet
traffic protocols at OSI layers L3 and L4. L2 (e.g. ARP) often get hash
value zero and no RSS type. For L3 focused on IPv4 vs. IPv6, and L4
primarily TCP vs UDP, but some hardware supports SCTP.
Hardware RSS types are differently encoded for each hardware NIC. Most
hardware represent RSS hash type as a number. Determining L3 vs L4 often
requires a mapping table as there often isn't a pattern or sorting
according to ISO layer.
The patch introduce a XDP RSS hash type (enum xdp_rss_hash_type) that
contain combinations to be used by drivers, which gets build up with bits
from enum xdp_rss_type_bits. Both enum xdp_rss_type_bits and
xdp_rss_hash_type get exposed to BPF via BTF, and it is up to the
BPF-programmer to match using these defines.
This proposal change the kfunc API bpf_xdp_metadata_rx_hash() adding
a pointer value argument for provide the RSS hash type.
Signed-off-by: Jesper Dangaard Brouer <[email protected]>
---
include/linux/netdevice.h | 3 ++-
include/net/xdp.h | 46 +++++++++++++++++++++++++++++++++++++++++++++
net/core/xdp.c | 10 +++++++++-
3 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 470085b121d3..c35f04f636f1 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1624,7 +1624,8 @@ struct net_device_ops {
struct xdp_metadata_ops {
int (*xmo_rx_timestamp)(const struct xdp_md *ctx, u64 *timestamp);
- int (*xmo_rx_hash)(const struct xdp_md *ctx, u32 *hash);
+ int (*xmo_rx_hash)(const struct xdp_md *ctx, u32 *hash,
+ enum xdp_rss_hash_type *rss_type);
};
/**
diff --git a/include/net/xdp.h b/include/net/xdp.h
index 41c57b8b1671..130091a55a6f 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -8,6 +8,7 @@
#include <linux/skbuff.h> /* skb_shared_info */
#include <uapi/linux/netdev.h>
+#include <linux/bitfield.h>
/**
* DOC: XDP RX-queue information
@@ -425,6 +426,51 @@ XDP_METADATA_KFUNC_xxx
MAX_XDP_METADATA_KFUNC,
};
+enum xdp_rss_type_bits {
+ XDP_RSS_L3_IPV4 = BIT(0),
+ XDP_RSS_L3_IPV6 = BIT(1),
+
+ /* The fixed (L3) IPv4 and IPv6 headers can both be followed by
+ * variable/dynamic headers, IPv4 called Options and IPv6 called
+ * Extension Headers. HW RSS type can contain this info.
+ */
+ XDP_RSS_L3_DYNHDR = BIT(2),
+
+ /* When RSS hash covers L4 then drivers MUST set XDP_RSS_L4 bit in
+ * addition to the protocol specific bit. This ease interaction with
+ * SKBs and avoids reserving a fixed mask for future L4 protocol bits.
+ */
+ XDP_RSS_L4 = BIT(3), /* L4 based hash, proto can be unknown */
+ XDP_RSS_L4_TCP = BIT(4),
+ XDP_RSS_L4_UDP = BIT(5),
+ XDP_RSS_L4_SCTP = BIT(6),
+ XDP_RSS_L4_IPSEC = BIT(7), /* L4 based hash include IPSEC SPI */
+};
+
+/* RSS hash type combinations used for driver HW mapping */
+enum xdp_rss_hash_type {
+ XDP_RSS_TYPE_NONE = 0,
+ XDP_RSS_TYPE_L2 = XDP_RSS_TYPE_NONE,
+
+ XDP_RSS_TYPE_L3_IPV4 = XDP_RSS_L3_IPV4,
+ XDP_RSS_TYPE_L3_IPV6 = XDP_RSS_L3_IPV6,
+ XDP_RSS_TYPE_L3_IPV4_OPT = XDP_RSS_L3_IPV4 | XDP_RSS_L3_DYNHDR,
+ XDP_RSS_TYPE_L3_IPV6_EX = XDP_RSS_L3_IPV6 | XDP_RSS_L3_DYNHDR,
+
+ XDP_RSS_TYPE_L4_ANY = XDP_RSS_L4,
+ XDP_RSS_TYPE_L4_IPV4_TCP = XDP_RSS_L3_IPV4 | XDP_RSS_L4 | XDP_RSS_L4_TCP,
+ XDP_RSS_TYPE_L4_IPV4_UDP = XDP_RSS_L3_IPV4 | XDP_RSS_L4 | XDP_RSS_L4_UDP,
+ XDP_RSS_TYPE_L4_IPV4_SCTP = XDP_RSS_L3_IPV4 | XDP_RSS_L4 | XDP_RSS_L4_SCTP,
+
+ XDP_RSS_TYPE_L4_IPV6_TCP = XDP_RSS_L3_IPV6 | XDP_RSS_L4 | XDP_RSS_L4_TCP,
+ XDP_RSS_TYPE_L4_IPV6_UDP = XDP_RSS_L3_IPV6 | XDP_RSS_L4 | XDP_RSS_L4_UDP,
+ XDP_RSS_TYPE_L4_IPV6_SCTP = XDP_RSS_L3_IPV6 | XDP_RSS_L4 | XDP_RSS_L4_SCTP,
+
+ XDP_RSS_TYPE_L4_IPV6_TCP_EX = XDP_RSS_TYPE_L4_IPV6_TCP |XDP_RSS_L3_DYNHDR,
+ XDP_RSS_TYPE_L4_IPV6_UDP_EX = XDP_RSS_TYPE_L4_IPV6_UDP |XDP_RSS_L3_DYNHDR,
+ XDP_RSS_TYPE_L4_IPV6_SCTP_EX = XDP_RSS_TYPE_L4_IPV6_SCTP|XDP_RSS_L3_DYNHDR,
+};
+
#ifdef CONFIG_NET
u32 bpf_xdp_metadata_kfunc_id(int id);
bool bpf_dev_bound_kfunc_id(u32 btf_id);
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 528d4b37983d..38d2dee16b47 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -734,14 +734,22 @@ __bpf_kfunc int bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx, u64 *tim
* bpf_xdp_metadata_rx_hash - Read XDP frame RX hash.
* @ctx: XDP context pointer.
* @hash: Return value pointer.
+ * @rss_type: Return value pointer for RSS type.
+ *
+ * The RSS hash type (@rss_type) specifies what portion of packet headers NIC
+ * hardware were used when calculating RSS hash value. The type combinations
+ * are defined via &enum xdp_rss_hash_type and individual bits can be decoded
+ * via &enum xdp_rss_type_bits.
*
* Return:
* * Returns 0 on success or ``-errno`` on error.
* * ``-EOPNOTSUPP`` : means device driver doesn't implement kfunc
* * ``-ENODATA`` : means no RX-hash available for this frame
*/
-__bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, u32 *hash)
+__bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, u32 *hash,
+ enum xdp_rss_hash_type *rss_type)
{
+ BTF_TYPE_EMIT(enum xdp_rss_type_bits);
return -EOPNOTSUPP;
}
Update API for bpf_xdp_metadata_rx_hash() with arg for xdp rss hash type
via mapping table.
The mlx5 hardware can also identify and RSS hash IPSEC. This indicate
hash includes SPI (Security Parameters Index) as part of IPSEC hash.
Extend xdp core enum xdp_rss_hash_type with IPSEC hash type.
Signed-off-by: Jesper Dangaard Brouer <[email protected]>
---
drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c | 63 +++++++++++++++++++++-
include/linux/mlx5/device.h | 14 ++++-
include/net/xdp.h | 3 +
3 files changed, 76 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
index c5dae48b7932..0abfed0ff3e5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
@@ -34,6 +34,7 @@
#include <net/xdp_sock_drv.h>
#include "en/xdp.h"
#include "en/params.h"
+#include <linux/bitfield.h>
int mlx5e_xdp_max_mtu(struct mlx5e_params *params, struct mlx5e_xsk_param *xsk)
{
@@ -169,14 +170,72 @@ static int mlx5e_xdp_rx_timestamp(const struct xdp_md *ctx, u64 *timestamp)
return 0;
}
-static int mlx5e_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash)
+/* Mapping HW RSS Type bits CQE_RSS_HTYPE_IP + CQE_RSS_HTYPE_L4 into 4-bits*/
+#define RSS_TYPE_MAX_TABLE 16 /* 4-bits max 16 entries */
+#define RSS_L4 GENMASK(1,0)
+#define RSS_L3 GENMASK(3,2) /* Same as CQE_RSS_HTYPE_IP */
+
+/* Valid combinations of CQE_RSS_HTYPE_IP + CQE_RSS_HTYPE_L4 sorted numerical */
+enum mlx5_rss_hash_type {
+ RSS_TYPE_NO_HASH = (FIELD_PREP_CONST(RSS_L3, CQE_RSS_IP_NONE)| \
+ FIELD_PREP_CONST(RSS_L4, CQE_RSS_L4_NONE)),
+ RSS_TYPE_L3_IPV4 = (FIELD_PREP_CONST(RSS_L3, CQE_RSS_IPV4)| \
+ FIELD_PREP_CONST(RSS_L4, CQE_RSS_L4_NONE)),
+ RSS_TYPE_L4_IPV4_TCP = (FIELD_PREP_CONST(RSS_L3, CQE_RSS_IPV4)| \
+ FIELD_PREP_CONST(RSS_L4, CQE_RSS_L4_TCP)),
+ RSS_TYPE_L4_IPV4_UDP = (FIELD_PREP_CONST(RSS_L3, CQE_RSS_IPV4)| \
+ FIELD_PREP_CONST(RSS_L4, CQE_RSS_L4_UDP)),
+ RSS_TYPE_L4_IPV4_IPSEC = (FIELD_PREP_CONST(RSS_L3, CQE_RSS_IPV4)| \
+ FIELD_PREP_CONST(RSS_L4, CQE_RSS_L4_IPSEC)),
+ RSS_TYPE_L3_IPV6 = (FIELD_PREP_CONST(RSS_L3, CQE_RSS_IPV6)| \
+ FIELD_PREP_CONST(RSS_L4, CQE_RSS_L4_NONE)),
+ RSS_TYPE_L4_IPV6_TCP = (FIELD_PREP_CONST(RSS_L3, CQE_RSS_IPV6)| \
+ FIELD_PREP_CONST(RSS_L4, CQE_RSS_L4_TCP)),
+ RSS_TYPE_L4_IPV6_UDP = (FIELD_PREP_CONST(RSS_L3, CQE_RSS_IPV6)| \
+ FIELD_PREP_CONST(RSS_L4, CQE_RSS_L4_UDP)),
+ RSS_TYPE_L4_IPV6_IPSEC = (FIELD_PREP_CONST(RSS_L3, CQE_RSS_IPV6)| \
+ FIELD_PREP_CONST(RSS_L4, CQE_RSS_L4_IPSEC)),
+} mlx5_rss_hash_type;
+
+/* Invalid combinations will simply return zero, allows no boundry checks */
+static const enum xdp_rss_hash_type mlx5_xdp_rss_type[RSS_TYPE_MAX_TABLE] = {
+ [RSS_TYPE_NO_HASH] = XDP_RSS_TYPE_NONE,
+ [1] = XDP_RSS_TYPE_NONE, /* Implicit zero */
+ [2] = XDP_RSS_TYPE_NONE, /* Implicit zero */
+ [3] = XDP_RSS_TYPE_NONE, /* Implicit zero */
+ [RSS_TYPE_L3_IPV4] = XDP_RSS_TYPE_L3_IPV4,
+ [RSS_TYPE_L4_IPV4_TCP] = XDP_RSS_TYPE_L4_IPV4_TCP,
+ [RSS_TYPE_L4_IPV4_UDP] = XDP_RSS_TYPE_L4_IPV4_UDP,
+ [RSS_TYPE_L4_IPV4_IPSEC]= XDP_RSS_TYPE_L4_IPV4_IPSEC,
+ [RSS_TYPE_L3_IPV6] = XDP_RSS_TYPE_L3_IPV6,
+ [RSS_TYPE_L4_IPV6_TCP] = XDP_RSS_TYPE_L4_IPV6_TCP,
+ [RSS_TYPE_L4_IPV6_UDP] = XDP_RSS_TYPE_L4_IPV6_UDP,
+ [RSS_TYPE_L4_IPV6_IPSEC]= XDP_RSS_TYPE_L4_IPV6_IPSEC,
+ [12] = XDP_RSS_TYPE_NONE, /* Implicit zero */
+ [13] = XDP_RSS_TYPE_NONE, /* Implicit zero */
+ [14] = XDP_RSS_TYPE_NONE, /* Implicit zero */
+ [15] = XDP_RSS_TYPE_NONE, /* Implicit zero */
+};
+
+static int mlx5e_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash,
+ enum xdp_rss_hash_type *rss_type)
{
const struct mlx5e_xdp_buff *_ctx = (void *)ctx;
+ const struct mlx5_cqe64 *cqe = _ctx->cqe;
+ u32 hash_type, l4_type, ip_type, lookup;
if (unlikely(!(_ctx->xdp.rxq->dev->features & NETIF_F_RXHASH)))
return -ENODATA;
- *hash = be32_to_cpu(_ctx->cqe->rss_hash_result);
+ *hash = be32_to_cpu(cqe->rss_hash_result);
+
+ hash_type = cqe->rss_hash_type;
+ BUILD_BUG_ON(CQE_RSS_HTYPE_IP != RSS_L3); /* same mask */
+ ip_type = hash_type & CQE_RSS_HTYPE_IP;
+ l4_type = FIELD_GET(CQE_RSS_HTYPE_L4, hash_type);
+ lookup = ip_type | l4_type;
+ *rss_type = mlx5_xdp_rss_type[lookup];
+
return 0;
}
diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
index 71b06ebad402..27aa9ae10996 100644
--- a/include/linux/mlx5/device.h
+++ b/include/linux/mlx5/device.h
@@ -36,6 +36,7 @@
#include <linux/types.h>
#include <rdma/ib_verbs.h>
#include <linux/mlx5/mlx5_ifc.h>
+#include <linux/bitfield.h>
#if defined(__LITTLE_ENDIAN)
#define MLX5_SET_HOST_ENDIANNESS 0
@@ -980,14 +981,23 @@ enum {
};
enum {
- CQE_RSS_HTYPE_IP = 0x3 << 2,
+ CQE_RSS_HTYPE_IP = GENMASK(3,2),
/* cqe->rss_hash_type[3:2] - IP destination selected for hash
* (00 = none, 01 = IPv4, 10 = IPv6, 11 = Reserved)
*/
- CQE_RSS_HTYPE_L4 = 0x3 << 6,
+ CQE_RSS_IP_NONE = 0x0,
+ CQE_RSS_IPV4 = 0x1,
+ CQE_RSS_IPV6 = 0x2,
+ CQE_RSS_RESERVED = 0x3,
+
+ CQE_RSS_HTYPE_L4 = GENMASK(7,6),
/* cqe->rss_hash_type[7:6] - L4 destination selected for hash
* (00 = none, 01 = TCP. 10 = UDP, 11 = IPSEC.SPI
*/
+ CQE_RSS_L4_NONE = 0x0,
+ CQE_RSS_L4_TCP = 0x1,
+ CQE_RSS_L4_UDP = 0x2,
+ CQE_RSS_L4_IPSEC = 0x3,
};
enum {
diff --git a/include/net/xdp.h b/include/net/xdp.h
index 130091a55a6f..0260e680006a 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -461,14 +461,17 @@ enum xdp_rss_hash_type {
XDP_RSS_TYPE_L4_IPV4_TCP = XDP_RSS_L3_IPV4 | XDP_RSS_L4 | XDP_RSS_L4_TCP,
XDP_RSS_TYPE_L4_IPV4_UDP = XDP_RSS_L3_IPV4 | XDP_RSS_L4 | XDP_RSS_L4_UDP,
XDP_RSS_TYPE_L4_IPV4_SCTP = XDP_RSS_L3_IPV4 | XDP_RSS_L4 | XDP_RSS_L4_SCTP,
+ XDP_RSS_TYPE_L4_IPV4_IPSEC = XDP_RSS_L3_IPV4 | XDP_RSS_L4 | XDP_RSS_L4_IPSEC,
XDP_RSS_TYPE_L4_IPV6_TCP = XDP_RSS_L3_IPV6 | XDP_RSS_L4 | XDP_RSS_L4_TCP,
XDP_RSS_TYPE_L4_IPV6_UDP = XDP_RSS_L3_IPV6 | XDP_RSS_L4 | XDP_RSS_L4_UDP,
XDP_RSS_TYPE_L4_IPV6_SCTP = XDP_RSS_L3_IPV6 | XDP_RSS_L4 | XDP_RSS_L4_SCTP,
+ XDP_RSS_TYPE_L4_IPV6_IPSEC = XDP_RSS_L3_IPV6 | XDP_RSS_L4 | XDP_RSS_L4_IPSEC,
XDP_RSS_TYPE_L4_IPV6_TCP_EX = XDP_RSS_TYPE_L4_IPV6_TCP |XDP_RSS_L3_DYNHDR,
XDP_RSS_TYPE_L4_IPV6_UDP_EX = XDP_RSS_TYPE_L4_IPV6_UDP |XDP_RSS_L3_DYNHDR,
XDP_RSS_TYPE_L4_IPV6_SCTP_EX = XDP_RSS_TYPE_L4_IPV6_SCTP|XDP_RSS_L3_DYNHDR,
+ XDP_RSS_TYPE_L4_IPV6_IPSEC_EX= XDP_RSS_TYPE_L4_IPV6_IPSEC|XDP_RSS_L3_DYNHDR,
};
#ifdef CONFIG_NET
Signed-off-by: Jesper Dangaard Brouer <[email protected]>
---
.../selftests/bpf/prog_tests/xdp_metadata.c | 2 ++
.../testing/selftests/bpf/progs/xdp_hw_metadata.c | 14 +++++++++-----
tools/testing/selftests/bpf/progs/xdp_metadata.c | 6 +++---
tools/testing/selftests/bpf/progs/xdp_metadata2.c | 7 ++++---
tools/testing/selftests/bpf/xdp_hw_metadata.c | 2 +-
tools/testing/selftests/bpf/xdp_metadata.h | 1 +
6 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
index aa4beae99f4f..8c5e98da9ae9 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
@@ -273,6 +273,8 @@ static int verify_xsk_metadata(struct xsk *xsk)
if (!ASSERT_NEQ(meta->rx_hash, 0, "rx_hash"))
return -1;
+ ASSERT_EQ(meta->rx_hash_type, 0, "rx_hash_type");
+
xsk_ring_cons__release(&xsk->rx, 1);
refill_rx(xsk, comp_addr);
diff --git a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c
index 4c55b4d79d3d..7b3fc12e96d6 100644
--- a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c
+++ b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c
@@ -14,8 +14,8 @@ struct {
extern int bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx,
__u64 *timestamp) __ksym;
-extern int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx,
- __u32 *hash) __ksym;
+extern int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, __u32 *hash,
+ enum xdp_rss_hash_type *rss_type) __ksym;
SEC("xdp")
int rx(struct xdp_md *ctx)
@@ -74,10 +74,14 @@ int rx(struct xdp_md *ctx)
else
meta->rx_timestamp = 0; /* Used by AF_XDP as not avail signal */
- if (!bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash))
- bpf_printk("populated rx_hash with %u", meta->rx_hash);
- else
+ if (!bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash, &meta->rx_hash_type)) {
+ bpf_printk("populated rx_hash:0x%X type:0x%X",
+ meta->rx_hash, meta->rx_hash_type);
+ if (!(meta->rx_hash_type & XDP_RSS_L4))
+ bpf_printk("rx_hash low quality L3 hash type");
+ } else {
meta->rx_hash = 0; /* Used by AF_XDP as not avail signal */
+ }
return bpf_redirect_map(&xsk, ctx->rx_queue_index, XDP_PASS);
}
diff --git a/tools/testing/selftests/bpf/progs/xdp_metadata.c b/tools/testing/selftests/bpf/progs/xdp_metadata.c
index 77678b034389..d151d406a123 100644
--- a/tools/testing/selftests/bpf/progs/xdp_metadata.c
+++ b/tools/testing/selftests/bpf/progs/xdp_metadata.c
@@ -21,8 +21,8 @@ struct {
extern int bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx,
__u64 *timestamp) __ksym;
-extern int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx,
- __u32 *hash) __ksym;
+extern int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, __u32 *hash,
+ enum xdp_rss_hash_type *rss_type) __ksym;
SEC("xdp")
int rx(struct xdp_md *ctx)
@@ -56,7 +56,7 @@ int rx(struct xdp_md *ctx)
if (timestamp == 0)
meta->rx_timestamp = 1;
- bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash);
+ bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash, &meta->rx_hash_type);
return bpf_redirect_map(&xsk, ctx->rx_queue_index, XDP_PASS);
}
diff --git a/tools/testing/selftests/bpf/progs/xdp_metadata2.c b/tools/testing/selftests/bpf/progs/xdp_metadata2.c
index cf69d05451c3..85f88d9d7a78 100644
--- a/tools/testing/selftests/bpf/progs/xdp_metadata2.c
+++ b/tools/testing/selftests/bpf/progs/xdp_metadata2.c
@@ -5,17 +5,18 @@
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_endian.h>
-extern int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx,
- __u32 *hash) __ksym;
+extern int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, __u32 *hash,
+ enum xdp_rss_hash_type *rss_type) __ksym;
int called;
SEC("freplace/rx")
int freplace_rx(struct xdp_md *ctx)
{
+ enum xdp_rss_hash_type type = 0;
u32 hash = 0;
/* Call _any_ metadata function to make sure we don't crash. */
- bpf_xdp_metadata_rx_hash(ctx, &hash);
+ bpf_xdp_metadata_rx_hash(ctx, &hash, &type);
called++;
return XDP_PASS;
}
diff --git a/tools/testing/selftests/bpf/xdp_hw_metadata.c b/tools/testing/selftests/bpf/xdp_hw_metadata.c
index 1c8acb68b977..4ca29e0c9646 100644
--- a/tools/testing/selftests/bpf/xdp_hw_metadata.c
+++ b/tools/testing/selftests/bpf/xdp_hw_metadata.c
@@ -141,7 +141,7 @@ static void verify_xdp_metadata(void *data)
meta = data - sizeof(*meta);
printf("rx_timestamp: %llu\n", meta->rx_timestamp);
- printf("rx_hash: %u\n", meta->rx_hash);
+ printf("rx_hash: 0x%X RSS type:0x%X\n", meta->rx_hash, meta->rx_hash_type);
}
static void verify_skb_metadata(int fd)
diff --git a/tools/testing/selftests/bpf/xdp_metadata.h b/tools/testing/selftests/bpf/xdp_metadata.h
index f6780fbb0a21..899da872fee1 100644
--- a/tools/testing/selftests/bpf/xdp_metadata.h
+++ b/tools/testing/selftests/bpf/xdp_metadata.h
@@ -12,4 +12,5 @@
struct xdp_meta {
__u64 rx_timestamp;
__u32 rx_hash;
+ __u32 rx_hash_type;
};
Update API for bpf_xdp_metadata_rx_hash() with arg for xdp rss hash type
via matching indiviual Completion Queue Entry (CQE) status bits.
Signed-off-by: Jesper Dangaard Brouer <[email protected]>
---
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 22 ++++++++++++++++++++--
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 3 ++-
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 4b5e459b6d49..d3f46d8b4160 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -681,14 +681,32 @@ int mlx4_en_xdp_rx_timestamp(const struct xdp_md *ctx, u64 *timestamp)
return 0;
}
-int mlx4_en_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash)
+int mlx4_en_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash,
+ enum xdp_rss_hash_type *rss_type)
{
struct mlx4_en_xdp_buff *_ctx = (void *)ctx;
+ struct mlx4_cqe *cqe = _ctx->cqe;
+ enum xdp_rss_hash_type xht = 0;
+ __be16 status;
if (unlikely(!(_ctx->dev->features & NETIF_F_RXHASH)))
return -ENODATA;
- *hash = be32_to_cpu(_ctx->cqe->immed_rss_invalid);
+ *hash = be32_to_cpu(cqe->immed_rss_invalid);
+ status = cqe->status;
+ if (status & cpu_to_be16(MLX4_CQE_STATUS_TCP))
+ xht = XDP_RSS_L4_TCP;
+ if (status & cpu_to_be16(MLX4_CQE_STATUS_UDP))
+ xht = XDP_RSS_L4_UDP;
+ if (status & cpu_to_be16(MLX4_CQE_STATUS_IPV4|MLX4_CQE_STATUS_IPV4F))
+ xht |= XDP_RSS_L3_IPV4;
+ if (status & cpu_to_be16(MLX4_CQE_STATUS_IPV6)) {
+ xht |= XDP_RSS_L3_IPV6;
+ if (cqe->ipv6_ext_mask)
+ xht |= XDP_RSS_L3_DYNHDR;
+ }
+ *rss_type = xht;
+
return 0;
}
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index 544e09b97483..4ac4d883047b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -798,7 +798,8 @@ int mlx4_en_netdev_event(struct notifier_block *this,
struct xdp_md;
int mlx4_en_xdp_rx_timestamp(const struct xdp_md *ctx, u64 *timestamp);
-int mlx4_en_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash);
+int mlx4_en_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash,
+ enum xdp_rss_hash_type *rss_type);
/*
* Functions for time stamping
Update API for bpf_xdp_metadata_rx_hash() with arg for xdp rss hash type.
The veth driver currently only support XDP-hints based on SKB code path.
The SKB have lost information about the RSS hash type, by compressing
the information down to a single bitfield skb->l4_hash, that only knows
if this was a L4 hash value.
In preparation for veth, the xdp_rss_hash_type have an L4 indication
bit that allow us to return a meaningful L4 indication when working
with SKB based packets.
Signed-off-by: Jesper Dangaard Brouer <[email protected]>
---
drivers/net/veth.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index c1178915496d..f5381e7c9310 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -1648,14 +1648,19 @@ static int veth_xdp_rx_timestamp(const struct xdp_md *ctx, u64 *timestamp)
return 0;
}
-static int veth_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash)
+static int veth_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash,
+ enum xdp_rss_hash_type *rss_type)
+
{
struct veth_xdp_buff *_ctx = (void *)ctx;
+ struct sk_buff *skb = _ctx->skb;
- if (!_ctx->skb)
+ if (!skb)
return -ENODATA;
- *hash = skb_get_hash(_ctx->skb);
+ *hash = skb_get_hash(skb);
+ *rss_type = skb->l4_hash ? XDP_RSS_TYPE_L4_ANY : XDP_RSS_TYPE_NONE;
+
return 0;
}
On 03/30, Jesper Dangaard Brouer wrote:
> The RSS hash type specifies what portion of packet data NIC hardware used
> when calculating RSS hash value. The RSS types are focused on Internet
> traffic protocols at OSI layers L3 and L4. L2 (e.g. ARP) often get hash
> value zero and no RSS type. For L3 focused on IPv4 vs. IPv6, and L4
> primarily TCP vs UDP, but some hardware supports SCTP.
> Hardware RSS types are differently encoded for each hardware NIC. Most
> hardware represent RSS hash type as a number. Determining L3 vs L4 often
> requires a mapping table as there often isn't a pattern or sorting
> according to ISO layer.
> The patch introduce a XDP RSS hash type (enum xdp_rss_hash_type) that
> contain combinations to be used by drivers, which gets build up with bits
> from enum xdp_rss_type_bits. Both enum xdp_rss_type_bits and
> xdp_rss_hash_type get exposed to BPF via BTF, and it is up to the
> BPF-programmer to match using these defines.
> This proposal change the kfunc API bpf_xdp_metadata_rx_hash() adding
> a pointer value argument for provide the RSS hash type.
> Signed-off-by: Jesper Dangaard Brouer <[email protected]>
> ---
> include/linux/netdevice.h | 3 ++-
> include/net/xdp.h | 46
> +++++++++++++++++++++++++++++++++++++++++++++
> net/core/xdp.c | 10 +++++++++-
> 3 files changed, 57 insertions(+), 2 deletions(-)
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 470085b121d3..c35f04f636f1 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1624,7 +1624,8 @@ struct net_device_ops {
> struct xdp_metadata_ops {
> int (*xmo_rx_timestamp)(const struct xdp_md *ctx, u64 *timestamp);
> - int (*xmo_rx_hash)(const struct xdp_md *ctx, u32 *hash);
> + int (*xmo_rx_hash)(const struct xdp_md *ctx, u32 *hash,
> + enum xdp_rss_hash_type *rss_type);
> };
> /**
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index 41c57b8b1671..130091a55a6f 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -8,6 +8,7 @@
> #include <linux/skbuff.h> /* skb_shared_info */
> #include <uapi/linux/netdev.h>
> +#include <linux/bitfield.h>
> /**
> * DOC: XDP RX-queue information
> @@ -425,6 +426,51 @@ XDP_METADATA_KFUNC_xxx
> MAX_XDP_METADATA_KFUNC,
> };
> +enum xdp_rss_type_bits {
> + XDP_RSS_L3_IPV4 = BIT(0),
> + XDP_RSS_L3_IPV6 = BIT(1),
> +
> + /* The fixed (L3) IPv4 and IPv6 headers can both be followed by
> + * variable/dynamic headers, IPv4 called Options and IPv6 called
> + * Extension Headers. HW RSS type can contain this info.
> + */
> + XDP_RSS_L3_DYNHDR = BIT(2),
> +
> + /* When RSS hash covers L4 then drivers MUST set XDP_RSS_L4 bit in
> + * addition to the protocol specific bit. This ease interaction with
> + * SKBs and avoids reserving a fixed mask for future L4 protocol bits.
> + */
> + XDP_RSS_L4 = BIT(3), /* L4 based hash, proto can be unknown */
> + XDP_RSS_L4_TCP = BIT(4),
> + XDP_RSS_L4_UDP = BIT(5),
> + XDP_RSS_L4_SCTP = BIT(6),
> + XDP_RSS_L4_IPSEC = BIT(7), /* L4 based hash include IPSEC SPI */
> +};
> +
> +/* RSS hash type combinations used for driver HW mapping */
> +enum xdp_rss_hash_type {
> + XDP_RSS_TYPE_NONE = 0,
> + XDP_RSS_TYPE_L2 = XDP_RSS_TYPE_NONE,
> +
> + XDP_RSS_TYPE_L3_IPV4 = XDP_RSS_L3_IPV4,
> + XDP_RSS_TYPE_L3_IPV6 = XDP_RSS_L3_IPV6,
> + XDP_RSS_TYPE_L3_IPV4_OPT = XDP_RSS_L3_IPV4 | XDP_RSS_L3_DYNHDR,
> + XDP_RSS_TYPE_L3_IPV6_EX = XDP_RSS_L3_IPV6 | XDP_RSS_L3_DYNHDR,
> +
> + XDP_RSS_TYPE_L4_ANY = XDP_RSS_L4,
> + XDP_RSS_TYPE_L4_IPV4_TCP = XDP_RSS_L3_IPV4 | XDP_RSS_L4 |
> XDP_RSS_L4_TCP,
> + XDP_RSS_TYPE_L4_IPV4_UDP = XDP_RSS_L3_IPV4 | XDP_RSS_L4 |
> XDP_RSS_L4_UDP,
> + XDP_RSS_TYPE_L4_IPV4_SCTP = XDP_RSS_L3_IPV4 | XDP_RSS_L4 |
> XDP_RSS_L4_SCTP,
> +
> + XDP_RSS_TYPE_L4_IPV6_TCP = XDP_RSS_L3_IPV6 | XDP_RSS_L4 |
> XDP_RSS_L4_TCP,
> + XDP_RSS_TYPE_L4_IPV6_UDP = XDP_RSS_L3_IPV6 | XDP_RSS_L4 |
> XDP_RSS_L4_UDP,
> + XDP_RSS_TYPE_L4_IPV6_SCTP = XDP_RSS_L3_IPV6 | XDP_RSS_L4 |
> XDP_RSS_L4_SCTP,
> +
> + XDP_RSS_TYPE_L4_IPV6_TCP_EX = XDP_RSS_TYPE_L4_IPV6_TCP |
> XDP_RSS_L3_DYNHDR,
> + XDP_RSS_TYPE_L4_IPV6_UDP_EX = XDP_RSS_TYPE_L4_IPV6_UDP |
> XDP_RSS_L3_DYNHDR,
> + XDP_RSS_TYPE_L4_IPV6_SCTP_EX = XDP_RSS_TYPE_L4_IPV6_SCTP|
> XDP_RSS_L3_DYNHDR,
> +};
> +
> #ifdef CONFIG_NET
> u32 bpf_xdp_metadata_kfunc_id(int id);
> bool bpf_dev_bound_kfunc_id(u32 btf_id);
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 528d4b37983d..38d2dee16b47 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -734,14 +734,22 @@ __bpf_kfunc int bpf_xdp_metadata_rx_timestamp(const
> struct xdp_md *ctx, u64 *tim
> * bpf_xdp_metadata_rx_hash - Read XDP frame RX hash.
> * @ctx: XDP context pointer.
> * @hash: Return value pointer.
> + * @rss_type: Return value pointer for RSS type.
> + *
> + * The RSS hash type (@rss_type) specifies what portion of packet
> headers NIC
> + * hardware were used when calculating RSS hash value. The type
> combinations
> + * are defined via &enum xdp_rss_hash_type and individual bits can be
> decoded
> + * via &enum xdp_rss_type_bits.
> *
> * Return:
> * * Returns 0 on success or ``-errno`` on error.
> * * ``-EOPNOTSUPP`` : means device driver doesn't implement kfunc
> * * ``-ENODATA`` : means no RX-hash available for this frame
> */
> -__bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, u32
> *hash)
> +__bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, u32
> *hash,
> + enum xdp_rss_hash_type *rss_type)
> {
[..]
> + BTF_TYPE_EMIT(enum xdp_rss_type_bits);
nit: Do we still need this with an extra argument?
> return -EOPNOTSUPP;
> }
On 03/30, Jesper Dangaard Brouer wrote:
> Notice targeted 6.3-rc kernel via bpf git tree.
> Current API for bpf_xdp_metadata_rx_hash() returns the raw RSS hash value,
> but doesn't provide information on the RSS hash type (part of 6.3-rc).
> This patchset proposal is to change the function call signature via adding
> a pointer value argument for provide the RSS hash type.
> Alternatively we disable bpf_xdp_metadata_rx_hash() in 6.3-rc, and have
> more time to nitpick the RSS hash-type bits.
LGTM with one nit about EMIT_BTF.
> ---
> Jesper Dangaard Brouer (5):
> xdp: rss hash types representation
> mlx5: bpf_xdp_metadata_rx_hash add xdp rss hash type
> veth: bpf_xdp_metadata_rx_hash add xdp rss hash type
> mlx4: bpf_xdp_metadata_rx_hash add xdp rss hash type
> selftests/bpf: Adjust bpf_xdp_metadata_rx_hash for new arg
> drivers/net/ethernet/mellanox/mlx4/en_rx.c | 22 ++++++-
> drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 3 +-
> .../net/ethernet/mellanox/mlx5/core/en/xdp.c | 63 ++++++++++++++++++-
> drivers/net/veth.c | 11 +++-
> include/linux/mlx5/device.h | 14 ++++-
> include/linux/netdevice.h | 3 +-
> include/net/xdp.h | 49 +++++++++++++++
> net/core/xdp.c | 10 ++-
> .../selftests/bpf/prog_tests/xdp_metadata.c | 2 +
> .../selftests/bpf/progs/xdp_hw_metadata.c | 14 +++--
> .../selftests/bpf/progs/xdp_metadata.c | 6 +-
> .../selftests/bpf/progs/xdp_metadata2.c | 7 ++-
> tools/testing/selftests/bpf/xdp_hw_metadata.c | 2 +-
> tools/testing/selftests/bpf/xdp_metadata.h | 1 +
> 14 files changed, 183 insertions(+), 24 deletions(-)
> --
On 30/03/2023 20.35, Stanislav Fomichev wrote:
> On 03/30, Jesper Dangaard Brouer wrote:
>> The RSS hash type specifies what portion of packet data NIC hardware used
>> when calculating RSS hash value. The RSS types are focused on Internet
>> traffic protocols at OSI layers L3 and L4. L2 (e.g. ARP) often get hash
>> value zero and no RSS type. For L3 focused on IPv4 vs. IPv6, and L4
>> primarily TCP vs UDP, but some hardware supports SCTP.
>
>> Hardware RSS types are differently encoded for each hardware NIC. Most
>> hardware represent RSS hash type as a number. Determining L3 vs L4 often
>> requires a mapping table as there often isn't a pattern or sorting
>> according to ISO layer.
>
>> The patch introduce a XDP RSS hash type (enum xdp_rss_hash_type) that
>> contain combinations to be used by drivers, which gets build up with bits
>> from enum xdp_rss_type_bits. Both enum xdp_rss_type_bits and
>> xdp_rss_hash_type get exposed to BPF via BTF, and it is up to the
>> BPF-programmer to match using these defines.
>
>> This proposal change the kfunc API bpf_xdp_metadata_rx_hash() adding
>> a pointer value argument for provide the RSS hash type.
>
>> Signed-off-by: Jesper Dangaard Brouer <[email protected]>
>> ---
>> include/linux/netdevice.h | 3 ++-
>> include/net/xdp.h | 46 +++++++++++++++++++++++++++++++++++++++++++++
>> net/core/xdp.c | 10 +++++++++-
>> 3 files changed, 57 insertions(+), 2 deletions(-)
>
[...]
>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>> index 528d4b37983d..38d2dee16b47 100644
>> --- a/net/core/xdp.c
>> +++ b/net/core/xdp.c
>> @@ -734,14 +734,22 @@ __bpf_kfunc int
>> bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx, u64 *tim
>> * bpf_xdp_metadata_rx_hash - Read XDP frame RX hash.
>> * @ctx: XDP context pointer.
>> * @hash: Return value pointer.
>> + * @rss_type: Return value pointer for RSS type.
>> + *
>> + * The RSS hash type (@rss_type) specifies what portion of packet headers NIC
>> + * hardware were used when calculating RSS hash value. The type combinations
>> + * are defined via &enum xdp_rss_hash_type and individual bits can be decoded
>> + * via &enum xdp_rss_type_bits.
>> *
>> * Return:
>> * * Returns 0 on success or ``-errno`` on error.
>> * * ``-EOPNOTSUPP`` : means device driver doesn't implement kfunc
>> * * ``-ENODATA`` : means no RX-hash available for this frame
>> */
>> -__bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx,
>> u32 *hash)
>> +__bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx,
>> u32 *hash,
>> + enum xdp_rss_hash_type *rss_type)
>> {
>
> [..]
>
>> + BTF_TYPE_EMIT(enum xdp_rss_type_bits);
>
> nit: Do we still need this with an extra argument?
>
Yes, unfortunately (compiler optimizes out enum xdp_rss_type_bits).
Do notice the difference xdp_rss_type_bits vs xdp_rss_hash_type.
We don't need it for "xdp_rss_hash_type" but need it for
"xdp_rss_type_bits".
--Jesper
On Thu, Mar 30, 2023 at 11:56 AM Jesper Dangaard Brouer
<[email protected]> wrote:
>
>
> On 30/03/2023 20.35, Stanislav Fomichev wrote:
> > On 03/30, Jesper Dangaard Brouer wrote:
> >> The RSS hash type specifies what portion of packet data NIC hardware used
> >> when calculating RSS hash value. The RSS types are focused on Internet
> >> traffic protocols at OSI layers L3 and L4. L2 (e.g. ARP) often get hash
> >> value zero and no RSS type. For L3 focused on IPv4 vs. IPv6, and L4
> >> primarily TCP vs UDP, but some hardware supports SCTP.
> >
> >> Hardware RSS types are differently encoded for each hardware NIC. Most
> >> hardware represent RSS hash type as a number. Determining L3 vs L4 often
> >> requires a mapping table as there often isn't a pattern or sorting
> >> according to ISO layer.
> >
> >> The patch introduce a XDP RSS hash type (enum xdp_rss_hash_type) that
> >> contain combinations to be used by drivers, which gets build up with bits
> >> from enum xdp_rss_type_bits. Both enum xdp_rss_type_bits and
> >> xdp_rss_hash_type get exposed to BPF via BTF, and it is up to the
> >> BPF-programmer to match using these defines.
> >
> >> This proposal change the kfunc API bpf_xdp_metadata_rx_hash() adding
> >> a pointer value argument for provide the RSS hash type.
> >
> >> Signed-off-by: Jesper Dangaard Brouer <[email protected]>
> >> ---
> >> include/linux/netdevice.h | 3 ++-
> >> include/net/xdp.h | 46 +++++++++++++++++++++++++++++++++++++++++++++
> >> net/core/xdp.c | 10 +++++++++-
> >> 3 files changed, 57 insertions(+), 2 deletions(-)
> >
>
> [...]
> >> diff --git a/net/core/xdp.c b/net/core/xdp.c
> >> index 528d4b37983d..38d2dee16b47 100644
> >> --- a/net/core/xdp.c
> >> +++ b/net/core/xdp.c
> >> @@ -734,14 +734,22 @@ __bpf_kfunc int
> >> bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx, u64 *tim
> >> * bpf_xdp_metadata_rx_hash - Read XDP frame RX hash.
> >> * @ctx: XDP context pointer.
> >> * @hash: Return value pointer.
> >> + * @rss_type: Return value pointer for RSS type.
> >> + *
> >> + * The RSS hash type (@rss_type) specifies what portion of packet headers NIC
> >> + * hardware were used when calculating RSS hash value. The type combinations
> >> + * are defined via &enum xdp_rss_hash_type and individual bits can be decoded
> >> + * via &enum xdp_rss_type_bits.
> >> *
> >> * Return:
> >> * * Returns 0 on success or ``-errno`` on error.
> >> * * ``-EOPNOTSUPP`` : means device driver doesn't implement kfunc
> >> * * ``-ENODATA`` : means no RX-hash available for this frame
> >> */
> >> -__bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx,
> >> u32 *hash)
> >> +__bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx,
> >> u32 *hash,
> >> + enum xdp_rss_hash_type *rss_type)
> >> {
> >
> > [..]
> >
> >> + BTF_TYPE_EMIT(enum xdp_rss_type_bits);
> >
> > nit: Do we still need this with an extra argument?
> >
>
> Yes, unfortunately (compiler optimizes out enum xdp_rss_type_bits).
> Do notice the difference xdp_rss_type_bits vs xdp_rss_hash_type.
> We don't need it for "xdp_rss_hash_type" but need it for
> "xdp_rss_type_bits".
Ah, I missed that. Then why not expose xdp_rss_type_bits?
Keep xdp_rss_hash_type for internal drivers' tables, and export the
enum with the bits?
> --Jesper
>
On 30/03/2023 20.38, Stanislav Fomichev wrote:
> On 03/30, Jesper Dangaard Brouer wrote:
>> Notice targeted 6.3-rc kernel via bpf git tree.
>
>> Current API for bpf_xdp_metadata_rx_hash() returns the raw RSS hash value,
>> but doesn't provide information on the RSS hash type (part of 6.3-rc).
>
>> This patchset proposal is to change the function call signature via adding
>> a pointer value argument for provide the RSS hash type.
>
>> Alternatively we disable bpf_xdp_metadata_rx_hash() in 6.3-rc, and have
>> more time to nitpick the RSS hash-type bits.
>
> LGTM with one nit about EMIT_BTF.
>
Great, others please review, so I can incorporate for tomorrow.
I will send a official patchset V4 tomorrow.
--Jesper
On 30/03/2023 21.02, Stanislav Fomichev wrote:
> On Thu, Mar 30, 2023 at 11:56 AM Jesper Dangaard Brouer
>>
>> On 30/03/2023 20.35, Stanislav Fomichev wrote:
>>> On 03/30, Jesper Dangaard Brouer wrote:
[...]
>> [...]
>>>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>>>> index 528d4b37983d..38d2dee16b47 100644
>>>> --- a/net/core/xdp.c
>>>> +++ b/net/core/xdp.c
>>>> @@ -734,14 +734,22 @@ __bpf_kfunc int
>>>> bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx, u64 *tim
>>>> * bpf_xdp_metadata_rx_hash - Read XDP frame RX hash.
>>>> * @ctx: XDP context pointer.
>>>> * @hash: Return value pointer.
>>>> + * @rss_type: Return value pointer for RSS type.
>>>> + *
>>>> + * The RSS hash type (@rss_type) specifies what portion of packet headers NIC
>>>> + * hardware were used when calculating RSS hash value. The type combinations
>>>> + * are defined via &enum xdp_rss_hash_type and individual bits can be decoded
>>>> + * via &enum xdp_rss_type_bits.
>>>> *
>>>> * Return:
>>>> * * Returns 0 on success or ``-errno`` on error.
>>>> * * ``-EOPNOTSUPP`` : means device driver doesn't implement kfunc
>>>> * * ``-ENODATA`` : means no RX-hash available for this frame
>>>> */
>>>> -__bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx,
>>>> u32 *hash)
>>>> +__bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx,
>>>> u32 *hash,
>>>> + enum xdp_rss_hash_type *rss_type)
>>>> {
>>> [..]
>>>
>>>> + BTF_TYPE_EMIT(enum xdp_rss_type_bits);
>>> nit: Do we still need this with an extra argument?
>>>
>> Yes, unfortunately (compiler optimizes out enum xdp_rss_type_bits).
>> Do notice the difference xdp_rss_type_bits vs xdp_rss_hash_type.
>> We don't need it for "xdp_rss_hash_type" but need it for
>> "xdp_rss_type_bits".
>
> Ah, I missed that. Then why not expose xdp_rss_type_bits?
> Keep xdp_rss_hash_type for internal drivers' tables, and export the
> enum with the bits?
Great suggestion, xdp_rss_hash_type will be internal for drivers.
I will do that in V4.
--Jesper
Jesper Dangaard Brouer <[email protected]> writes:
> Notice targeted 6.3-rc kernel via bpf git tree.
>
> Current API for bpf_xdp_metadata_rx_hash() returns the raw RSS hash value,
> but doesn't provide information on the RSS hash type (part of 6.3-rc).
>
> This patchset proposal is to change the function call signature via adding
> a pointer value argument for provide the RSS hash type.
>
> Alternatively we disable bpf_xdp_metadata_rx_hash() in 6.3-rc, and have
> more time to nitpick the RSS hash-type bits.
LGTM; for the series:
Acked-by: Toke Høiland-Jørgensen <[email protected]>
On 30/03/2023 21.08, Jesper Dangaard Brouer wrote:
>
> On 30/03/2023 21.02, Stanislav Fomichev wrote:
>> On Thu, Mar 30, 2023 at 11:56 AM Jesper Dangaard Brouer
>>>
>>> On 30/03/2023 20.35, Stanislav Fomichev wrote:
>>>> On 03/30, Jesper Dangaard Brouer wrote:
> [...]
>>> [...]
>>>>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>>>>> index 528d4b37983d..38d2dee16b47 100644
>>>>> --- a/net/core/xdp.c
>>>>> +++ b/net/core/xdp.c
>>>>> @@ -734,14 +734,22 @@ __bpf_kfunc int
>>>>> bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx, u64 *tim
>>>>> * bpf_xdp_metadata_rx_hash - Read XDP frame RX hash.
>>>>> * @ctx: XDP context pointer.
>>>>> * @hash: Return value pointer.
>>>>> + * @rss_type: Return value pointer for RSS type.
>>>>> + *
>>>>> + * The RSS hash type (@rss_type) specifies what portion of packet
>>>>> headers NIC
>>>>> + * hardware were used when calculating RSS hash value. The type
>>>>> combinations
>>>>> + * are defined via &enum xdp_rss_hash_type and individual bits can
>>>>> be decoded
>>>>> + * via &enum xdp_rss_type_bits.
>>>>> *
>>>>> * Return:
>>>>> * * Returns 0 on success or ``-errno`` on error.
>>>>> * * ``-EOPNOTSUPP`` : means device driver doesn't implement kfunc
>>>>> * * ``-ENODATA`` : means no RX-hash available for this frame
>>>>> */
>>>>> -__bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx,
>>>>> u32 *hash)
>>>>> +__bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx,
>>>>> u32 *hash,
>>>>> + enum xdp_rss_hash_type *rss_type)
>>>>> {
>>>> [..]
>>>>
>>>>> + BTF_TYPE_EMIT(enum xdp_rss_type_bits);
>>>> nit: Do we still need this with an extra argument?
>>>>
>>> Yes, unfortunately (compiler optimizes out enum xdp_rss_type_bits).
>>> Do notice the difference xdp_rss_type_bits vs xdp_rss_hash_type.
>>> We don't need it for "xdp_rss_hash_type" but need it for
>>> "xdp_rss_type_bits".
> >
>> Ah, I missed that. Then why not expose xdp_rss_type_bits?
>> Keep xdp_rss_hash_type for internal drivers' tables, and export the
>> enum with the bits?
>
> Great suggestion, xdp_rss_hash_type will be internal for drivers.
> I will do that in V4.
I'm running into annoying compiler warnings [-Wenum-conversion]
about enum conversions. I'll try to workaround this...
The easiest solution seem to be to only have a single enum, that both
contains the BIT()s and combinations of bits (for driver usage).
E.g.
warning: implicit conversion from 'enum xdp_rss_type_bits' to 'enum
xdp_rss_hash_type' [-Wenum-conversion]