diff mbox series

[bpf,RFC-V3,1/5] xdp: rss hash types representation

Message ID 168019606574.3557870.15629824904085210321.stgit@firesoul (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series XDP-hints: API change for RX-hash kfunc bpf_xdp_metadata_rx_hash | expand

Checks

Context Check Description
bpf/vmtest-bpf-PR success PR summary
bpf/vmtest-bpf-VM_Test-10 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-VM_Test-36 fail Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-37 fail Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-38 fail Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-39 fail Logs for test_progs_no_alu32 on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-40 fail Logs for test_progs_no_alu32 on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-41 fail Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-VM_Test-42 fail Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-VM_Test-43 fail Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-44 fail Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-45 fail Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-46 fail Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-47 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-48 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-49 success Logs for test_progs_no_alu32_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-50 success Logs for test_progs_no_alu32_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-51 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-52 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-53 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-54 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-55 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-56 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-57 success Logs for test_progs_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-58 success Logs for test_progs_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-59 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-60 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-61 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-62 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-63 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-64 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-65 success Logs for test_verifier on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-66 success Logs for test_verifier on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-67 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-VM_Test-68 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-VM_Test-69 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-70 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-71 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-72 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-VM_Test-2 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-VM_Test-3 success Logs for build for aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-7 success Logs for set-matrix
bpf/vmtest-bpf-VM_Test-4 success Logs for build for s390x with gcc
bpf/vmtest-bpf-VM_Test-5 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-VM_Test-6 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-8 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-9 success Logs for test_maps on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-11 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-12 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-13 fail Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-14 fail Logs for test_progs on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-16 fail Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-17 fail Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-18 fail Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-19 fail Logs for test_progs_no_alu32 on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-21 fail Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-22 fail Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-23 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-24 success Logs for test_progs_no_alu32_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-25 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-26 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-27 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-28 success Logs for test_progs_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-29 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-30 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-31 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-32 success Logs for test_verifier on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-34 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-35 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-20 fail Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-VM_Test-33 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-VM_Test-15 fail Logs for test_progs on s390x with gcc
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf, async
netdev/fixes_present fail Series targets non-next tree, but doesn't contain any Fixes tags
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 4298 this patch: 4172
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang fail Errors and warnings before: 976 this patch: 991
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 4507 this patch: 4385
netdev/checkpatch warning CHECK: spaces preferred around that '|' (ctx:VxV) CHECK: spaces preferred around that '|' (ctx:WxV) WARNING: line length of 83 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns WARNING: line length of 86 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jesper Dangaard Brouer March 30, 2023, 5:07 p.m. UTC 
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 <brouer@redhat.com>
---
 include/linux/netdevice.h |    3 ++-
 include/net/xdp.h         |   46 +++++++++++++++++++++++++++++++++++++++++++++
 net/core/xdp.c            |   10 +++++++++-
 3 files changed, 57 insertions(+), 2 deletions(-)

Comments

Stanislav Fomichev March 30, 2023, 6:35 p.m. UTC | #1 
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 <brouer@redhat.com>
> ---
>   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;
>   }
Jesper Dangaard Brouer March 30, 2023, 6:56 p.m. UTC | #2 
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 <brouer@redhat.com>
>> ---
>>   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
Stanislav Fomichev March 30, 2023, 7:02 p.m. UTC | #3 
On Thu, Mar 30, 2023 at 11:56 AM Jesper Dangaard Brouer
<jbrouer@redhat.com> 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 <brouer@redhat.com>
> >> ---
> >>   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
>
Jesper Dangaard Brouer March 30, 2023, 7:08 p.m. UTC | #4 
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 March 31, 2023, 11:49 a.m. UTC | #5 
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]
diff mbox series

Patch

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;
 }