[RFC PATCH bpf-next] bpf: add bpf_get_skb_hash helper function

[RFC PATCH bpf-next] bpf: add bpf_get_skb_hash helper function

[Harshitha Ramamurthy]

This patch adds a helper function called bpf_get_skb_hash to calculate
the skb hash for a packet at the XDP layer. In the helper function,
a local skb is allocated and we populate the fields needed in the skb
before calling skb_get_hash. To avoid memory allocations for each packet,
we allocate an skb per CPU and use the same buffer for subsequent hash
calculations on the same CPU.

Signed-off-by: Harshitha Ramamurthy <harshitha.ramamurthy@intel.com>
---
 include/uapi/linux/bpf.h       |  8 ++++++
 net/core/filter.c              | 50 ++++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h |  8 ++++++
 3 files changed, 66 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index b134e679e9db..25aa850c8a40 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3394,6 +3394,13 @@ union bpf_attr {
  *		A non-negative value equal to or less than *size* on success,
  *		or a negative error in case of failure.
  *
+ * u32 bpf_get_skb_hash(struct xdp_buff *xdp_md)
+ *	Description
+ *		Return the skb hash for the xdp context passed. This function
+ *		allocates a temporary skb and populates the fields needed. It
+ *		then calls skb_get_hash to calculate the skb hash for the packet.
+ *	Return
+ *		The 32-bit hash.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -3538,6 +3545,7 @@ union bpf_attr {
 	FN(skc_to_tcp_request_sock),	\
 	FN(skc_to_udp6_sock),		\
 	FN(get_task_stack),		\
+	FN(get_skb_hash),		\
 	/* */
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
diff --git a/net/core/filter.c b/net/core/filter.c
index 7124f0fe6974..9f6ad7209b44 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3765,6 +3765,54 @@ static const struct bpf_func_proto bpf_xdp_redirect_map_proto = {
 	.arg3_type      = ARG_ANYTHING,
 };
 
+static DEFINE_PER_CPU(struct sk_buff *, hash_skb);
+
+BPF_CALL_1(bpf_get_skb_hash, struct xdp_buff *, xdp)
+{
+	void *data_end = xdp->data_end;
+	struct ethhdr *eth = xdp->data;
+	void *data = xdp->data;
+	unsigned long flags;
+	struct sk_buff *skb;
+	int nh_off, len;
+	u32 ret = 0;
+
+	/* disable interrupts to get the correct skb pointer */
+	local_irq_save(flags);
+
+	len = data_end - data;
+	skb = this_cpu_read(hash_skb);
+	if (!skb) {
+		skb = alloc_skb(len, GFP_ATOMIC);
+		if (!skb)
+			goto out;
+		this_cpu_write(hash_skb, skb);
+	}
+
+	nh_off = sizeof(*eth);
+	if (data + nh_off > data_end)
+		goto out;
+
+	skb->data = data;
+	skb->head = data;
+	skb->network_header = nh_off;
+	skb->protocol = eth->h_proto;
+	skb->len = len;
+	skb->dev = xdp->rxq->dev;
+
+	ret = skb_get_hash(skb);
+out:
+	local_irq_restore(flags);
+	return ret;
+}
+
+const struct bpf_func_proto bpf_get_skb_hash_proto = {
+	.func		= bpf_get_skb_hash,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+};
+
 static unsigned long bpf_skb_copy(void *dst_buff, const void *skb,
 				  unsigned long off, unsigned long len)
 {
@@ -6503,6 +6551,8 @@ xdp_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_xdp_adjust_tail_proto;
 	case BPF_FUNC_fib_lookup:
 		return &bpf_xdp_fib_lookup_proto;
+	case BPF_FUNC_get_skb_hash:
+		return &bpf_get_skb_hash_proto;
 #ifdef CONFIG_INET
 	case BPF_FUNC_sk_lookup_udp:
 		return &bpf_xdp_sk_lookup_udp_proto;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index b134e679e9db..25aa850c8a40 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3394,6 +3394,13 @@ union bpf_attr {
  *		A non-negative value equal to or less than *size* on success,
  *		or a negative error in case of failure.
  *
+ * u32 bpf_get_skb_hash(struct xdp_buff *xdp_md)
+ *	Description
+ *		Return the skb hash for the xdp context passed. This function
+ *		allocates a temporary skb and populates the fields needed. It
+ *		then calls skb_get_hash to calculate the skb hash for the packet.
+ *	Return
+ *		The 32-bit hash.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -3538,6 +3545,7 @@ union bpf_attr {
 	FN(skc_to_tcp_request_sock),	\
 	FN(skc_to_udp6_sock),		\
 	FN(get_task_stack),		\
+	FN(get_skb_hash),		\
 	/* */
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
-- 
2.20.1

Re: [RFC PATCH bpf-next] bpf: add bpf_get_skb_hash helper function

[David Ahern]

On 8/10/20 12:28 PM, Harshitha Ramamurthy wrote:
> This patch adds a helper function called bpf_get_skb_hash to calculate
> the skb hash for a packet at the XDP layer. In the helper function,

Why? i.e., expected use case?

Pulling this from hardware when possible is better. e.g., Saeed's
hardware hints proposal includes it.

> a local skb is allocated and we populate the fields needed in the skb
> before calling skb_get_hash. To avoid memory allocations for each packet,
> we allocate an skb per CPU and use the same buffer for subsequent hash
> calculations on the same CPU.
> 
> Signed-off-by: Harshitha Ramamurthy <harshitha.ramamurthy@intel.com>
> ---
>  include/uapi/linux/bpf.h       |  8 ++++++
>  net/core/filter.c              | 50 ++++++++++++++++++++++++++++++++++
>  tools/include/uapi/linux/bpf.h |  8 ++++++
>  3 files changed, 66 insertions(+)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index b134e679e9db..25aa850c8a40 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -3394,6 +3394,13 @@ union bpf_attr {
>   *		A non-negative value equal to or less than *size* on success,
>   *		or a negative error in case of failure.
>   *
> + * u32 bpf_get_skb_hash(struct xdp_buff *xdp_md)
> + *	Description
> + *		Return the skb hash for the xdp context passed. This function
> + *		allocates a temporary skb and populates the fields needed. It
> + *		then calls skb_get_hash to calculate the skb hash for the packet.
> + *	Return
> + *		The 32-bit hash.
>   */
>  #define __BPF_FUNC_MAPPER(FN)		\
>  	FN(unspec),			\
> @@ -3538,6 +3545,7 @@ union bpf_attr {
>  	FN(skc_to_tcp_request_sock),	\
>  	FN(skc_to_udp6_sock),		\
>  	FN(get_task_stack),		\
> +	FN(get_skb_hash),		\
>  	/* */
>  
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 7124f0fe6974..9f6ad7209b44 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3765,6 +3765,54 @@ static const struct bpf_func_proto bpf_xdp_redirect_map_proto = {
>  	.arg3_type      = ARG_ANYTHING,
>  };
>  
> +static DEFINE_PER_CPU(struct sk_buff *, hash_skb);
> +
> +BPF_CALL_1(bpf_get_skb_hash, struct xdp_buff *, xdp)
> +{
> +	void *data_end = xdp->data_end;
> +	struct ethhdr *eth = xdp->data;
> +	void *data = xdp->data;
> +	unsigned long flags;
> +	struct sk_buff *skb;
> +	int nh_off, len;
> +	u32 ret = 0;
> +
> +	/* disable interrupts to get the correct skb pointer */
> +	local_irq_save(flags);
> +
> +	len = data_end - data;
> +	skb = this_cpu_read(hash_skb);
> +	if (!skb) {
> +		skb = alloc_skb(len, GFP_ATOMIC);
> +		if (!skb)
> +			goto out;
> +		this_cpu_write(hash_skb, skb);
> +	}
> +
> +	nh_off = sizeof(*eth);

vlans?

> +	if (data + nh_off > data_end)
> +		goto out;
> +
> +	skb->data = data;
> +	skb->head = data;
> +	skb->network_header = nh_off;
> +	skb->protocol = eth->h_proto;
> +	skb->len = len;
> +	skb->dev = xdp->rxq->dev;
> +
> +	ret = skb_get_hash(skb);

static inline __u32 skb_get_hash(struct sk_buff *skb)
{
        if (!skb->l4_hash && !skb->sw_hash)
                __skb_get_hash(skb);

        return skb->hash;
}

__skb_get_hash -> __skb_set_sw_hash -> __skb_set_hash which sets
sw_hash as a minimum, so it seems to me you will always be returning the
hash of the first packet since you do not clear relevant fields of the skb.

Re: [RFC PATCH bpf-next] bpf: add bpf_get_skb_hash helper function

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 11457 bytes --]

Hi Harshitha,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Harshitha-Ramamurthy/bpf-add-bpf_get_skb_hash-helper-function/20200811-023515
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: x86_64-randconfig-s022-20200811 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.2-168-g9554805c-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

   net/core/filter.c:426:33: sparse: sparse: subtraction of functions? Share your drugs
   net/core/filter.c:429:33: sparse: sparse: subtraction of functions? Share your drugs
   net/core/filter.c:432:33: sparse: sparse: subtraction of functions? Share your drugs
   net/core/filter.c:435:33: sparse: sparse: subtraction of functions? Share your drugs
   net/core/filter.c:438:33: sparse: sparse: subtraction of functions? Share your drugs
   net/core/filter.c:512:27: sparse: sparse: subtraction of functions? Share your drugs
   net/core/filter.c:515:27: sparse: sparse: subtraction of functions? Share your drugs
   net/core/filter.c:518:27: sparse: sparse: subtraction of functions? Share your drugs
   net/core/filter.c:1406:39: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct sock_filter const *filter @@     got struct sock_filter [noderef] __user *filter @@
   net/core/filter.c:1406:39: sparse:     expected struct sock_filter const *filter
   net/core/filter.c:1406:39: sparse:     got struct sock_filter [noderef] __user *filter
   net/core/filter.c:1484:39: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct sock_filter const *filter @@     got struct sock_filter [noderef] __user *filter @@
   net/core/filter.c:1484:39: sparse:     expected struct sock_filter const *filter
   net/core/filter.c:1484:39: sparse:     got struct sock_filter [noderef] __user *filter
>> net/core/filter.c:3809:29: sparse: sparse: symbol 'bpf_get_skb_hash_proto' was not declared. Should it be static?
   net/core/filter.c:7130:27: sparse: sparse: subtraction of functions? Share your drugs
   net/core/filter.c:7133:27: sparse: sparse: subtraction of functions? Share your drugs
   net/core/filter.c:7136:27: sparse: sparse: subtraction of functions? Share your drugs
   net/core/filter.c:8856:31: sparse: sparse: symbol 'sk_filter_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:8863:27: sparse: sparse: symbol 'sk_filter_prog_ops' was not declared. Should it be static?
   net/core/filter.c:8867:31: sparse: sparse: symbol 'tc_cls_act_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:8875:27: sparse: sparse: symbol 'tc_cls_act_prog_ops' was not declared. Should it be static?
   net/core/filter.c:8879:31: sparse: sparse: symbol 'xdp_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:8890:31: sparse: sparse: symbol 'cg_skb_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:8896:27: sparse: sparse: symbol 'cg_skb_prog_ops' was not declared. Should it be static?
   net/core/filter.c:8900:31: sparse: sparse: symbol 'lwt_in_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:8906:27: sparse: sparse: symbol 'lwt_in_prog_ops' was not declared. Should it be static?
   net/core/filter.c:8910:31: sparse: sparse: symbol 'lwt_out_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:8916:27: sparse: sparse: symbol 'lwt_out_prog_ops' was not declared. Should it be static?
   net/core/filter.c:8920:31: sparse: sparse: symbol 'lwt_xmit_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:8927:27: sparse: sparse: symbol 'lwt_xmit_prog_ops' was not declared. Should it be static?
   net/core/filter.c:8931:31: sparse: sparse: symbol 'lwt_seg6local_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:8937:27: sparse: sparse: symbol 'lwt_seg6local_prog_ops' was not declared. Should it be static?
   net/core/filter.c:8941:31: sparse: sparse: symbol 'cg_sock_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:8947:27: sparse: sparse: symbol 'cg_sock_prog_ops' was not declared. Should it be static?
   net/core/filter.c:8950:31: sparse: sparse: symbol 'cg_sock_addr_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:8956:27: sparse: sparse: symbol 'cg_sock_addr_prog_ops' was not declared. Should it be static?
   net/core/filter.c:8959:31: sparse: sparse: symbol 'sock_ops_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:8965:27: sparse: sparse: symbol 'sock_ops_prog_ops' was not declared. Should it be static?
   net/core/filter.c:8968:31: sparse: sparse: symbol 'sk_skb_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:8975:27: sparse: sparse: symbol 'sk_skb_prog_ops' was not declared. Should it be static?
   net/core/filter.c:8978:31: sparse: sparse: symbol 'sk_msg_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:8985:27: sparse: sparse: symbol 'sk_msg_prog_ops' was not declared. Should it be static?
   net/core/filter.c:8988:31: sparse: sparse: symbol 'flow_dissector_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:8994:27: sparse: sparse: symbol 'flow_dissector_prog_ops' was not declared. Should it be static?
   net/core/filter.c:9300:31: sparse: sparse: symbol 'sk_reuseport_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:9306:27: sparse: sparse: symbol 'sk_reuseport_prog_ops' was not declared. Should it be static?
   net/core/filter.c:9482:27: sparse: sparse: symbol 'sk_lookup_prog_ops' was not declared. Should it be static?
   net/core/filter.c:9485:31: sparse: sparse: symbol 'sk_lookup_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:241:32: sparse: sparse: cast to restricted __be16
   net/core/filter.c:241:32: sparse: sparse: cast to restricted __be16
   net/core/filter.c:241:32: sparse: sparse: cast to restricted __be16
   net/core/filter.c:241:32: sparse: sparse: cast to restricted __be16
   net/core/filter.c:268:32: sparse: sparse: cast to restricted __be32
   net/core/filter.c:268:32: sparse: sparse: cast to restricted __be32
   net/core/filter.c:268:32: sparse: sparse: cast to restricted __be32
   net/core/filter.c:268:32: sparse: sparse: cast to restricted __be32
   net/core/filter.c:268:32: sparse: sparse: cast to restricted __be32
   net/core/filter.c:268:32: sparse: sparse: cast to restricted __be32
   net/core/filter.c:1908:43: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __wsum [usertype] diff @@     got unsigned long long [usertype] to @@
   net/core/filter.c:1908:43: sparse:     expected restricted __wsum [usertype] diff
   net/core/filter.c:1908:43: sparse:     got unsigned long long [usertype] to
   net/core/filter.c:1911:36: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __be16 [usertype] old @@     got unsigned long long [usertype] from @@
   net/core/filter.c:1911:36: sparse:     expected restricted __be16 [usertype] old
   net/core/filter.c:1911:36: sparse:     got unsigned long long [usertype] from
   net/core/filter.c:1911:42: sparse: sparse: incorrect type in argument 3 (different base types) @@     expected restricted __be16 [usertype] new @@     got unsigned long long [usertype] to @@
   net/core/filter.c:1911:42: sparse:     expected restricted __be16 [usertype] new
   net/core/filter.c:1911:42: sparse:     got unsigned long long [usertype] to
   net/core/filter.c:1914:36: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __be32 [usertype] from @@     got unsigned long long [usertype] from @@
   net/core/filter.c:1914:36: sparse:     expected restricted __be32 [usertype] from
   net/core/filter.c:1914:36: sparse:     got unsigned long long [usertype] from
   net/core/filter.c:1914:42: sparse: sparse: incorrect type in argument 3 (different base types) @@     expected restricted __be32 [usertype] to @@     got unsigned long long [usertype] to @@
   net/core/filter.c:1914:42: sparse:     expected restricted __be32 [usertype] to
   net/core/filter.c:1914:42: sparse:     got unsigned long long [usertype] to
   net/core/filter.c:1959:59: sparse: sparse: incorrect type in argument 3 (different base types) @@     expected restricted __wsum [usertype] diff @@     got unsigned long long [usertype] to @@
   net/core/filter.c:1959:59: sparse:     expected restricted __wsum [usertype] diff
   net/core/filter.c:1959:59: sparse:     got unsigned long long [usertype] to
   net/core/filter.c:1962:52: sparse: sparse: incorrect type in argument 3 (different base types) @@     expected restricted __be16 [usertype] from @@     got unsigned long long [usertype] from @@
   net/core/filter.c:1962:52: sparse:     expected restricted __be16 [usertype] from
   net/core/filter.c:1962:52: sparse:     got unsigned long long [usertype] from
   net/core/filter.c:1962:58: sparse: sparse: incorrect type in argument 4 (different base types) @@     expected restricted __be16 [usertype] to @@     got unsigned long long [usertype] to @@
   net/core/filter.c:1962:58: sparse:     expected restricted __be16 [usertype] to
   net/core/filter.c:1962:58: sparse:     got unsigned long long [usertype] to
   net/core/filter.c:1965:52: sparse: sparse: incorrect type in argument 3 (different base types) @@     expected restricted __be32 [usertype] from @@     got unsigned long long [usertype] from @@
   net/core/filter.c:1965:52: sparse:     expected restricted __be32 [usertype] from
   net/core/filter.c:1965:52: sparse:     got unsigned long long [usertype] from
   net/core/filter.c:1965:58: sparse: sparse: incorrect type in argument 4 (different base types) @@     expected restricted __be32 [usertype] to @@     got unsigned long long [usertype] to @@
   net/core/filter.c:1965:58: sparse:     expected restricted __be32 [usertype] to
   net/core/filter.c:1965:58: sparse:     got unsigned long long [usertype] to
   net/core/filter.c:2011:28: sparse: sparse: incorrect type in return expression (different base types) @@     expected unsigned long long @@     got restricted __wsum @@
   net/core/filter.c:2011:28: sparse:     expected unsigned long long
   net/core/filter.c:2011:28: sparse:     got restricted __wsum
   net/core/filter.c:2033:35: sparse: sparse: incorrect type in return expression (different base types) @@     expected unsigned long long @@     got restricted __wsum [usertype] csum @@
   net/core/filter.c:2033:35: sparse:     expected unsigned long long
   net/core/filter.c:2033:35: sparse:     got restricted __wsum [usertype] csum

Please review and possibly fold the followup patch.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33772 bytes --]

[RFC PATCH] bpf: bpf_get_skb_hash_proto can be static

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 570 bytes --]


Signed-off-by: kernel test robot <lkp@intel.com>
---
 filter.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 9f6ad7209b448..85b741884443b 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3806,7 +3806,7 @@ BPF_CALL_1(bpf_get_skb_hash, struct xdp_buff *, xdp)
 	return ret;
 }
 
-const struct bpf_func_proto bpf_get_skb_hash_proto = {
+static const struct bpf_func_proto bpf_get_skb_hash_proto = {
 	.func		= bpf_get_skb_hash,
 	.gpl_only	= false,
 	.ret_type	= RET_INTEGER,

Re: [RFC PATCH bpf-next] bpf: add bpf_get_skb_hash helper function

[Ramamurthy, Harshitha]

On Mon, 2020-08-10 at 13:02 -0600, David Ahern wrote:
> On 8/10/20 12:28 PM, Harshitha Ramamurthy wrote:
> > This patch adds a helper function called bpf_get_skb_hash to
> > calculate
> > the skb hash for a packet at the XDP layer. In the helper function,
> 
> Why? i.e., expected use case?
> 
> Pulling this from hardware when possible is better. e.g., Saeed's
> hardware hints proposal includes it.

Thanks for the review and comments.

So, when possible, it might be better to pull the HW hash but not all
HW provides it so this function would be useful in those cases. Also,
this is a precursor to potentially calling the in-kernel flow dissector
from a helper function.
> 
> > a local skb is allocated and we populate the fields needed in the
> > skb
> > before calling skb_get_hash. To avoid memory allocations for each
> > packet,
> > we allocate an skb per CPU and use the same buffer for subsequent
> > hash
> > calculations on the same CPU.
> > 
> > Signed-off-by: Harshitha Ramamurthy <harshitha.ramamurthy@intel.com
> > >
> > ---
> >  include/uapi/linux/bpf.h       |  8 ++++++
> >  net/core/filter.c              | 50
> > ++++++++++++++++++++++++++++++++++
> >  tools/include/uapi/linux/bpf.h |  8 ++++++
> >  3 files changed, 66 insertions(+)
> > 
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index b134e679e9db..25aa850c8a40 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -3394,6 +3394,13 @@ union bpf_attr {
> >   *		A non-negative value equal to or less than *size* on
> > success,
> >   *		or a negative error in case of failure.
> >   *
> > + * u32 bpf_get_skb_hash(struct xdp_buff *xdp_md)
> > + *	Description
> > + *		Return the skb hash for the xdp context passed. This
> > function
> > + *		allocates a temporary skb and populates the fields
> > needed. It
> > + *		then calls skb_get_hash to calculate the skb hash for
> > the packet.
> > + *	Return
> > + *		The 32-bit hash.
> >   */
> >  #define __BPF_FUNC_MAPPER(FN)		\
> >  	FN(unspec),			\
> > @@ -3538,6 +3545,7 @@ union bpf_attr {
> >  	FN(skc_to_tcp_request_sock),	\
> >  	FN(skc_to_udp6_sock),		\
> >  	FN(get_task_stack),		\
> > +	FN(get_skb_hash),		\
> >  	/* */
> >  
> >  /* integer value in 'imm' field of BPF_CALL instruction selects
> > which helper
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index 7124f0fe6974..9f6ad7209b44 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -3765,6 +3765,54 @@ static const struct bpf_func_proto
> > bpf_xdp_redirect_map_proto = {
> >  	.arg3_type      = ARG_ANYTHING,
> >  };
> >  
> > +static DEFINE_PER_CPU(struct sk_buff *, hash_skb);
> > +
> > +BPF_CALL_1(bpf_get_skb_hash, struct xdp_buff *, xdp)
> > +{
> > +	void *data_end = xdp->data_end;
> > +	struct ethhdr *eth = xdp->data;
> > +	void *data = xdp->data;
> > +	unsigned long flags;
> > +	struct sk_buff *skb;
> > +	int nh_off, len;
> > +	u32 ret = 0;
> > +
> > +	/* disable interrupts to get the correct skb pointer */
> > +	local_irq_save(flags);
> > +
> > +	len = data_end - data;
> > +	skb = this_cpu_read(hash_skb);
> > +	if (!skb) {
> > +		skb = alloc_skb(len, GFP_ATOMIC);
> > +		if (!skb)
> > +			goto out;
> > +		this_cpu_write(hash_skb, skb);
> > +	}
> > +
> > +	nh_off = sizeof(*eth);
> 
> vlans?

Yes, need to factor for vlans. Will fix it.
> 
> > +	if (data + nh_off > data_end)
> > +		goto out;
> > +
> > +	skb->data = data;
> > +	skb->head = data;
> > +	skb->network_header = nh_off;
> > +	skb->protocol = eth->h_proto;
> > +	skb->len = len;
> > +	skb->dev = xdp->rxq->dev;
> > +
> > +	ret = skb_get_hash(skb);
> 
> static inline __u32 skb_get_hash(struct sk_buff *skb)
> {
>         if (!skb->l4_hash && !skb->sw_hash)
>                 __skb_get_hash(skb);
> 
>         return skb->hash;
> }
> 
> __skb_get_hash -> __skb_set_sw_hash -> __skb_set_hash which sets
> sw_hash as a minimum, so it seems to me you will always be returning
> the
> hash of the first packet since you do not clear relevant fields of
> the skb.

yes, that's true. Calling skb_clear_hash first should fix this. I will
try it out.

Thanks,
Harshitha.

Re: [RFC PATCH bpf-next] bpf: add bpf_get_skb_hash helper function

[David Ahern]

On 8/11/20 3:55 PM, Ramamurthy, Harshitha wrote:
> is a pre-cursor to potentially calling the in-kernel flow dissector from
> a helper function.
> 

That is going to be a challenge to do in a way that does not impact the
kernel's ability to change as needed, and I suspect it will be much
slower than doing in ebpf. Modular ebpf code that can be adapted to use
cases is going to be more appropriate for XDP for example.