Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v2] geneve: Use empty braces for addr6 initializer
From: Stefano Brivio @ 2018-11-13  9:02 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: David S. Miller, Sabrina Dubroca, netdev, linux-kernel,
	Joe Perches
In-Reply-To: <20181113061146.12182-1-natechancellor@gmail.com>

On Mon, 12 Nov 2018 23:11:47 -0700
Nathan Chancellor <natechancellor@gmail.com> wrote:

> Clang warns:
> 
> drivers/net/geneve.c:428:29: error: suggest braces around initialization
> of subobject [-Werror,-Wmissing-braces]
>                 struct in6_addr addr6 = { 0 };
>                                           ^
>                                           {}
> 
> Most initializations of structs in the kernel seem to use this format.
> 
> Fixes: a07966447f39 ("geneve: ICMP error lookup handler")
> Suggested-by: Joe Perches <joe@perches.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>

Also,

Reviewed-by: Stefano Brivio <sbrivio@redhat.com>

-- 
Stefano

^ permalink raw reply

* Hello, this is the second times am sending you this mail and you refused to reply to my email why?
From: KATIE HIGGINS @ 2018-11-12 23:09 UTC (permalink / raw)




^ permalink raw reply

* Re: [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
From: Martin Lau @ 2018-11-12 23:10 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Stanislav Fomichev, netdev@vger.kernel.org,
	linux-kselftest@vger.kernel.org, ast@kernel.org,
	daniel@iogearbox.net, shuah@kernel.org,
	jakub.kicinski@netronome.com, quentin.monnet@netronome.com,
	Roman Gushchin, jiong.wang@netronome.com,
	bhole_prashant_q7@lab.ntt.co.jp, john.fastabend@gmail.com,
	jbenc@redhat.com, treeze.taeung@gmail.com, Yonghong Song,
	Okash Khawaja, "sandipan@linux
In-Reply-To: <20181112221011.qqqkexvr3mw5clnr@mini-arch>

On Mon, Nov 12, 2018 at 02:10:11PM -0800, Stanislav Fomichev wrote:
> On 11/12, Martin Lau wrote:
> > On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> > [ ... ]
> > > @@ -1918,23 +2160,20 @@ void *bpf_object__priv(struct bpf_object *obj)
> > >  }
> > >  
> > >  static struct bpf_program *
> > > -__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> > >  {
> > > -	size_t idx;
> > > +	ssize_t idx;
> > >  
> > >  	if (!obj->programs)
> > >  		return NULL;
> > > -	/* First handler */
> > > -	if (prev == NULL)
> > > -		return &obj->programs[0];
> > >  
> > > -	if (prev->obj != obj) {
> > > +	if (p->obj != obj) {
> > >  		pr_warning("error: program handler doesn't match object\n");
> > >  		return NULL;
> > >  	}
> > >  
> > > -	idx = (prev - obj->programs) + 1;
> > > -	if (idx >= obj->nr_programs)
> > > +	idx = (p - obj->programs) + i;
> > > +	if (idx >= obj->nr_programs || idx < 0)
> > >  		return NULL;
> > >  	return &obj->programs[idx];
> > >  }
> > > @@ -1944,8 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > >  {
> > >  	struct bpf_program *prog = prev;
> > >  
> > > +	if (prev == NULL)
> > > +		return obj->programs;
> > > +
> > This patch breaks the behavior introduced in
> > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > "Make bpf_program__next() skip over '.text' section if object file
> >  has pseudo calls.  The '.text' section is hardly a program in that
> >  case, it's more of a storage for code of functions other than main."
> > 
> > For example, the userspace could have been doing:
> > 	prog = bpf_program__next(NULL, obj);
> > 	bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> > 	bpf_object__load(obj);
> > 
> > For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> > the prog returned by bpf_program__next() could be in ".text" instead of
> > the main bpf program.  The next bpf_program__set_type() has
> > no effect to the main program.  The following bpf_object__load()
> > will catch user in surprise with the main bpf prog in
> > the wrong BPF_PROG_TYPE.
> 
> Will something like the following fix your concern? (plus, assuming the
> same for prev):
> 
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
>  {
>         struct bpf_program *prog = prev;
>  
> -       if (prev == NULL)
> -               return obj->programs;
> +       if (prev == NULL) {
> +               prog = obj->programs;
> +               if (!prog || !bpf_program__is_function_storage(prog, obj))
> +                       return prog;
> +       }
>  
>         do {
>                 prog = __bpf_program__iter(prog, obj, 1);
> 
> Any suggestions for a better way to do it?
I think that would work.  The bpf_program__prev() will need the same
treatment though...

Here is my mostly untested fix to unblock my other dev works.  It moves
the very first NULL check back to __bpf_program__iter():

>From de1c89ae1768e756825a6874268b5b1686695c93 Mon Sep 17 00:00:00 2001
From: Martin KaFai Lau <kafai@fb.com>
Date: Mon, 12 Nov 2018 14:52:39 -0800
Subject: [PATCH] bpf: libbpf: Fix bpf_program__next() API

This patch restores the behavior in
commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
such that bpf_program__next() does not return pseudo programs in ".text".

Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e827542ffa3a..a01eb9584e52 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
 }
 
 static struct bpf_program *
-__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
+__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
 {
+	size_t nr_programs = obj->nr_programs;
 	ssize_t idx;
 
-	if (!obj->programs)
+	if (!nr_programs)
 		return NULL;
 
+	if (!p)
+		/* Iter from the beginning */
+		return forward ? &obj->programs[0] :
+			&obj->programs[nr_programs - 1];
+
 	if (p->obj != obj) {
 		pr_warning("error: program handler doesn't match object\n");
 		return NULL;
 	}
 
-	idx = (p - obj->programs) + i;
+	idx = (p - obj->programs) + (forward ? 1 : -1);
 	if (idx >= obj->nr_programs || idx < 0)
 		return NULL;
 	return &obj->programs[idx];
@@ -2216,11 +2222,8 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
 {
 	struct bpf_program *prog = prev;
 
-	if (prev == NULL)
-		return obj->programs;
-
 	do {
-		prog = __bpf_program__iter(prog, obj, 1);
+		prog = __bpf_program__iter(prog, obj, true);
 	} while (prog && bpf_program__is_function_storage(prog, obj));
 
 	return prog;
@@ -2231,14 +2234,8 @@ bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
 {
 	struct bpf_program *prog = next;
 
-	if (next == NULL) {
-		if (!obj->nr_programs)
-			return NULL;
-		return obj->programs + obj->nr_programs - 1;
-	}
-
 	do {
-		prog = __bpf_program__iter(prog, obj, -1);
+		prog = __bpf_program__iter(prog, obj, false);
 	} while (prog && bpf_program__is_function_storage(prog, obj));
 
 	return prog;
-- 
2.17.1

> 
> > >  	do {
> > > -		prog = __bpf_program__next(prog, obj);
> > > +		prog = __bpf_program__iter(prog, obj, 1);
> > > +	} while (prog && bpf_program__is_function_storage(prog, obj));
> > > +
> > > +	return prog;
> > > +}
> > > +
> > > +struct bpf_program *
> > > +bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
> > > +{
> > > +	struct bpf_program *prog = next;
> > > +
> > > +	if (next == NULL) {
> > > +		if (!obj->nr_programs)
> > > +			return NULL;
> > > +		return obj->programs + obj->nr_programs - 1;
> > > +	}
> > > +
> > > +	do {
> > > +		prog = __bpf_program__iter(prog, obj, -1);
> > >  	} while (prog && bpf_program__is_function_storage(prog, obj));
> > >  
> > >  	return prog;

^ permalink raw reply related

* Re: [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
From: Stanislav Fomichev @ 2018-11-12 23:29 UTC (permalink / raw)
  To: Martin Lau
  Cc: Stanislav Fomichev, netdev@vger.kernel.org,
	linux-kselftest@vger.kernel.org, ast@kernel.org,
	daniel@iogearbox.net, shuah@kernel.org,
	jakub.kicinski@netronome.com, quentin.monnet@netronome.com,
	Roman Gushchin, jiong.wang@netronome.com,
	bhole_prashant_q7@lab.ntt.co.jp, john.fastabend@gmail.com,
	jbenc@redhat.com, treeze.taeung@gmail.com, Yonghong Song,
	Okash Khawaja, "sandipan@linux
In-Reply-To: <20181112231040.l4qdresmbhcyf7vs@kafai-mbp.dhcp.thefacebook.com>

On 11/12, Martin Lau wrote:
> On Mon, Nov 12, 2018 at 02:10:11PM -0800, Stanislav Fomichev wrote:
> > On 11/12, Martin Lau wrote:
> > > On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> > > [ ... ]
> > > > @@ -1918,23 +2160,20 @@ void *bpf_object__priv(struct bpf_object *obj)
> > > >  }
> > > >  
> > > >  static struct bpf_program *
> > > > -__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > > +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> > > >  {
> > > > -	size_t idx;
> > > > +	ssize_t idx;
> > > >  
> > > >  	if (!obj->programs)
> > > >  		return NULL;
> > > > -	/* First handler */
> > > > -	if (prev == NULL)
> > > > -		return &obj->programs[0];
> > > >  
> > > > -	if (prev->obj != obj) {
> > > > +	if (p->obj != obj) {
> > > >  		pr_warning("error: program handler doesn't match object\n");
> > > >  		return NULL;
> > > >  	}
> > > >  
> > > > -	idx = (prev - obj->programs) + 1;
> > > > -	if (idx >= obj->nr_programs)
> > > > +	idx = (p - obj->programs) + i;
> > > > +	if (idx >= obj->nr_programs || idx < 0)
> > > >  		return NULL;
> > > >  	return &obj->programs[idx];
> > > >  }
> > > > @@ -1944,8 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > >  {
> > > >  	struct bpf_program *prog = prev;
> > > >  
> > > > +	if (prev == NULL)
> > > > +		return obj->programs;
> > > > +
> > > This patch breaks the behavior introduced in
> > > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > > "Make bpf_program__next() skip over '.text' section if object file
> > >  has pseudo calls.  The '.text' section is hardly a program in that
> > >  case, it's more of a storage for code of functions other than main."
> > > 
> > > For example, the userspace could have been doing:
> > > 	prog = bpf_program__next(NULL, obj);
> > > 	bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> > > 	bpf_object__load(obj);
> > > 
> > > For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> > > the prog returned by bpf_program__next() could be in ".text" instead of
> > > the main bpf program.  The next bpf_program__set_type() has
> > > no effect to the main program.  The following bpf_object__load()
> > > will catch user in surprise with the main bpf prog in
> > > the wrong BPF_PROG_TYPE.
> > 
> > Will something like the following fix your concern? (plus, assuming the
> > same for prev):
> > 
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> >  {
> >         struct bpf_program *prog = prev;
> >  
> > -       if (prev == NULL)
> > -               return obj->programs;
> > +       if (prev == NULL) {
> > +               prog = obj->programs;
> > +               if (!prog || !bpf_program__is_function_storage(prog, obj))
> > +                       return prog;
> > +       }
> >  
> >         do {
> >                 prog = __bpf_program__iter(prog, obj, 1);
> > 
> > Any suggestions for a better way to do it?
> I think that would work.  The bpf_program__prev() will need the same
> treatment though...
> 
> Here is my mostly untested fix to unblock my other dev works.  It moves
> the very first NULL check back to __bpf_program__iter():
I like your version and it works with my simple flow dissector test :-)
Thanks for spotting and fixing it!


> From de1c89ae1768e756825a6874268b5b1686695c93 Mon Sep 17 00:00:00 2001
> From: Martin KaFai Lau <kafai@fb.com>
> Date: Mon, 12 Nov 2018 14:52:39 -0800
> Subject: [PATCH] bpf: libbpf: Fix bpf_program__next() API
> 
> This patch restores the behavior in
> commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> such that bpf_program__next() does not return pseudo programs in ".text".
> 
> Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
>  tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index e827542ffa3a..a01eb9584e52 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
>  }
>  
>  static struct bpf_program *
> -__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
>  {
> +	size_t nr_programs = obj->nr_programs;
>  	ssize_t idx;
>  
> -	if (!obj->programs)
> +	if (!nr_programs)
>  		return NULL;
>  
> +	if (!p)
> +		/* Iter from the beginning */
> +		return forward ? &obj->programs[0] :
> +			&obj->programs[nr_programs - 1];
> +
>  	if (p->obj != obj) {
>  		pr_warning("error: program handler doesn't match object\n");
>  		return NULL;
>  	}
>  
> -	idx = (p - obj->programs) + i;
> +	idx = (p - obj->programs) + (forward ? 1 : -1);
>  	if (idx >= obj->nr_programs || idx < 0)
>  		return NULL;
>  	return &obj->programs[idx];
> @@ -2216,11 +2222,8 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
>  {
>  	struct bpf_program *prog = prev;
>  
> -	if (prev == NULL)
> -		return obj->programs;
> -
>  	do {
> -		prog = __bpf_program__iter(prog, obj, 1);
> +		prog = __bpf_program__iter(prog, obj, true);
>  	} while (prog && bpf_program__is_function_storage(prog, obj));
>  
>  	return prog;
> @@ -2231,14 +2234,8 @@ bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
>  {
>  	struct bpf_program *prog = next;
>  
> -	if (next == NULL) {
> -		if (!obj->nr_programs)
> -			return NULL;
> -		return obj->programs + obj->nr_programs - 1;
> -	}
> -
>  	do {
> -		prog = __bpf_program__iter(prog, obj, -1);
> +		prog = __bpf_program__iter(prog, obj, false);
>  	} while (prog && bpf_program__is_function_storage(prog, obj));
>  
>  	return prog;
> -- 
> 2.17.1
> 
> > 
> > > >  	do {
> > > > -		prog = __bpf_program__next(prog, obj);
> > > > +		prog = __bpf_program__iter(prog, obj, 1);
> > > > +	} while (prog && bpf_program__is_function_storage(prog, obj));
> > > > +
> > > > +	return prog;
> > > > +}
> > > > +
> > > > +struct bpf_program *
> > > > +bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
> > > > +{
> > > > +	struct bpf_program *prog = next;
> > > > +
> > > > +	if (next == NULL) {
> > > > +		if (!obj->nr_programs)
> > > > +			return NULL;
> > > > +		return obj->programs + obj->nr_programs - 1;
> > > > +	}
> > > > +
> > > > +	do {
> > > > +		prog = __bpf_program__iter(prog, obj, -1);
> > > >  	} while (prog && bpf_program__is_function_storage(prog, obj));
> > > >  
> > > >  	return prog;

^ permalink raw reply

* [PATCH][bpf-next] bpf: fix null pointer dereference on pointer offload
From: Colin King @ 2018-11-13  9:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, netdev; +Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Pointer offload is being null checked however the following statement
dereferences the potentially null pointer offload when assigning
offload->dev_state.  Fix this by only assigning it if offload is not
null.

Detected by CoverityScan, CID#1475437 ("Dereference after null check")

Fixes: 00db12c3d141 ("bpf: call verifier_prep from its callback in struct bpf_offload_dev")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 kernel/bpf/offload.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index 52c5617e3716..54cf2b9c44a4 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -130,9 +130,10 @@ int bpf_prog_offload_verifier_prep(struct bpf_prog *prog)
 
 	down_read(&bpf_devs_lock);
 	offload = prog->aux->offload;
-	if (offload)
+	if (offload) {
 		ret = offload->offdev->ops->prepare(prog);
-	offload->dev_state = !ret;
+		offload->dev_state = !ret;
+	}
 	up_read(&bpf_devs_lock);
 
 	return ret;
-- 
2.19.1

^ permalink raw reply related

* Re: [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
From: Martin Lau @ 2018-11-12 23:41 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Stanislav Fomichev, netdev@vger.kernel.org,
	linux-kselftest@vger.kernel.org, ast@kernel.org,
	daniel@iogearbox.net, shuah@kernel.org,
	jakub.kicinski@netronome.com, quentin.monnet@netronome.com,
	Roman Gushchin, jiong.wang@netronome.com,
	bhole_prashant_q7@lab.ntt.co.jp, john.fastabend@gmail.com,
	jbenc@redhat.com, treeze.taeung@gmail.com, Yonghong Song,
	Okash Khawaja, "sandipan@linux
In-Reply-To: <20181112232925.74cajfncscaf7sc6@mini-arch>

On Mon, Nov 12, 2018 at 03:29:25PM -0800, Stanislav Fomichev wrote:
> On 11/12, Martin Lau wrote:
> > On Mon, Nov 12, 2018 at 02:10:11PM -0800, Stanislav Fomichev wrote:
> > > On 11/12, Martin Lau wrote:
> > > > On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> > > > [ ... ]
> > > > > @@ -1918,23 +2160,20 @@ void *bpf_object__priv(struct bpf_object *obj)
> > > > >  }
> > > > >  
> > > > >  static struct bpf_program *
> > > > > -__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > > > +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> > > > >  {
> > > > > -	size_t idx;
> > > > > +	ssize_t idx;
> > > > >  
> > > > >  	if (!obj->programs)
> > > > >  		return NULL;
> > > > > -	/* First handler */
> > > > > -	if (prev == NULL)
> > > > > -		return &obj->programs[0];
> > > > >  
> > > > > -	if (prev->obj != obj) {
> > > > > +	if (p->obj != obj) {
> > > > >  		pr_warning("error: program handler doesn't match object\n");
> > > > >  		return NULL;
> > > > >  	}
> > > > >  
> > > > > -	idx = (prev - obj->programs) + 1;
> > > > > -	if (idx >= obj->nr_programs)
> > > > > +	idx = (p - obj->programs) + i;
> > > > > +	if (idx >= obj->nr_programs || idx < 0)
> > > > >  		return NULL;
> > > > >  	return &obj->programs[idx];
> > > > >  }
> > > > > @@ -1944,8 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > > >  {
> > > > >  	struct bpf_program *prog = prev;
> > > > >  
> > > > > +	if (prev == NULL)
> > > > > +		return obj->programs;
> > > > > +
> > > > This patch breaks the behavior introduced in
> > > > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > > > "Make bpf_program__next() skip over '.text' section if object file
> > > >  has pseudo calls.  The '.text' section is hardly a program in that
> > > >  case, it's more of a storage for code of functions other than main."
> > > > 
> > > > For example, the userspace could have been doing:
> > > > 	prog = bpf_program__next(NULL, obj);
> > > > 	bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> > > > 	bpf_object__load(obj);
> > > > 
> > > > For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> > > > the prog returned by bpf_program__next() could be in ".text" instead of
> > > > the main bpf program.  The next bpf_program__set_type() has
> > > > no effect to the main program.  The following bpf_object__load()
> > > > will catch user in surprise with the main bpf prog in
> > > > the wrong BPF_PROG_TYPE.
> > > 
> > > Will something like the following fix your concern? (plus, assuming the
> > > same for prev):
> > > 
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > >  {
> > >         struct bpf_program *prog = prev;
> > >  
> > > -       if (prev == NULL)
> > > -               return obj->programs;
> > > +       if (prev == NULL) {
> > > +               prog = obj->programs;
> > > +               if (!prog || !bpf_program__is_function_storage(prog, obj))
> > > +                       return prog;
> > > +       }
> > >  
> > >         do {
> > >                 prog = __bpf_program__iter(prog, obj, 1);
> > > 
> > > Any suggestions for a better way to do it?
> > I think that would work.  The bpf_program__prev() will need the same
> > treatment though...
> > 
> > Here is my mostly untested fix to unblock my other dev works.  It moves
> > the very first NULL check back to __bpf_program__iter():
> I like your version and it works with my simple flow dissector test :-)
Great.  I will send out an offical patch.

> Thanks for spotting and fixing it!
> 
> 
> > From de1c89ae1768e756825a6874268b5b1686695c93 Mon Sep 17 00:00:00 2001
> > From: Martin KaFai Lau <kafai@fb.com>
> > Date: Mon, 12 Nov 2018 14:52:39 -0800
> > Subject: [PATCH] bpf: libbpf: Fix bpf_program__next() API
> > 
> > This patch restores the behavior in
> > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > such that bpf_program__next() does not return pseudo programs in ".text".
> > 
> > Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
> > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> > ---
> >  tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
> >  1 file changed, 11 insertions(+), 14 deletions(-)
> > 
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index e827542ffa3a..a01eb9584e52 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
> >  }
> >  
> >  static struct bpf_program *
> > -__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> > +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
> >  {
> > +	size_t nr_programs = obj->nr_programs;
> >  	ssize_t idx;
> >  
> > -	if (!obj->programs)
> > +	if (!nr_programs)
> >  		return NULL;
> >  
> > +	if (!p)
> > +		/* Iter from the beginning */
> > +		return forward ? &obj->programs[0] :
> > +			&obj->programs[nr_programs - 1];
> > +
> >  	if (p->obj != obj) {
> >  		pr_warning("error: program handler doesn't match object\n");
> >  		return NULL;
> >  	}
> >  
> > -	idx = (p - obj->programs) + i;
> > +	idx = (p - obj->programs) + (forward ? 1 : -1);
> >  	if (idx >= obj->nr_programs || idx < 0)
> >  		return NULL;
> >  	return &obj->programs[idx];
> > @@ -2216,11 +2222,8 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> >  {
> >  	struct bpf_program *prog = prev;
> >  
> > -	if (prev == NULL)
> > -		return obj->programs;
> > -
> >  	do {
> > -		prog = __bpf_program__iter(prog, obj, 1);
> > +		prog = __bpf_program__iter(prog, obj, true);
> >  	} while (prog && bpf_program__is_function_storage(prog, obj));
> >  
> >  	return prog;
> > @@ -2231,14 +2234,8 @@ bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
> >  {
> >  	struct bpf_program *prog = next;
> >  
> > -	if (next == NULL) {
> > -		if (!obj->nr_programs)
> > -			return NULL;
> > -		return obj->programs + obj->nr_programs - 1;
> > -	}
> > -
> >  	do {
> > -		prog = __bpf_program__iter(prog, obj, -1);
> > +		prog = __bpf_program__iter(prog, obj, false);
> >  	} while (prog && bpf_program__is_function_storage(prog, obj));
> >  
> >  	return prog;
> > -- 
> > 2.17.1
> > 
> > > 
> > > > >  	do {
> > > > > -		prog = __bpf_program__next(prog, obj);
> > > > > +		prog = __bpf_program__iter(prog, obj, 1);
> > > > > +	} while (prog && bpf_program__is_function_storage(prog, obj));
> > > > > +
> > > > > +	return prog;
> > > > > +}
> > > > > +
> > > > > +struct bpf_program *
> > > > > +bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
> > > > > +{
> > > > > +	struct bpf_program *prog = next;
> > > > > +
> > > > > +	if (next == NULL) {
> > > > > +		if (!obj->nr_programs)
> > > > > +			return NULL;
> > > > > +		return obj->programs + obj->nr_programs - 1;
> > > > > +	}
> > > > > +
> > > > > +	do {
> > > > > +		prog = __bpf_program__iter(prog, obj, -1);
> > > > >  	} while (prog && bpf_program__is_function_storage(prog, obj));
> > > > >  
> > > > >  	return prog;

^ permalink raw reply

* [PATCH bpf-next] bpf: libbpf: Fix bpf_program__next() API
From: Martin KaFai Lau @ 2018-11-12 23:44 UTC (permalink / raw)
  To: netdev
  Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team,
	Stanislav Fomichev, Stanislav Fomichev

This patch restores the behavior in
commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs")
such that bpf_program__next() does not return pseudo programs in ".text".

Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e827542ffa3a..a01eb9584e52 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
 }
 
 static struct bpf_program *
-__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
+__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
 {
+	size_t nr_programs = obj->nr_programs;
 	ssize_t idx;
 
-	if (!obj->programs)
+	if (!nr_programs)
 		return NULL;
 
+	if (!p)
+		/* Iter from the beginning */
+		return forward ? &obj->programs[0] :
+			&obj->programs[nr_programs - 1];
+
 	if (p->obj != obj) {
 		pr_warning("error: program handler doesn't match object\n");
 		return NULL;
 	}
 
-	idx = (p - obj->programs) + i;
+	idx = (p - obj->programs) + (forward ? 1 : -1);
 	if (idx >= obj->nr_programs || idx < 0)
 		return NULL;
 	return &obj->programs[idx];
@@ -2216,11 +2222,8 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
 {
 	struct bpf_program *prog = prev;
 
-	if (prev == NULL)
-		return obj->programs;
-
 	do {
-		prog = __bpf_program__iter(prog, obj, 1);
+		prog = __bpf_program__iter(prog, obj, true);
 	} while (prog && bpf_program__is_function_storage(prog, obj));
 
 	return prog;
@@ -2231,14 +2234,8 @@ bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
 {
 	struct bpf_program *prog = next;
 
-	if (next == NULL) {
-		if (!obj->nr_programs)
-			return NULL;
-		return obj->programs + obj->nr_programs - 1;
-	}
-
 	do {
-		prog = __bpf_program__iter(prog, obj, -1);
+		prog = __bpf_program__iter(prog, obj, false);
 	} while (prog && bpf_program__is_function_storage(prog, obj));
 
 	return prog;
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH bpf-next] selftests/bpf: Fix uninitialized duration warning
From: Martin Lau @ 2018-11-12 23:51 UTC (permalink / raw)
  To: Joe Stringer; +Cc: daniel@iogearbox.net, netdev@vger.kernel.org
In-Reply-To: <20181109181816.22622-1-joe@wand.net.nz>

On Fri, Nov 09, 2018 at 10:18:16AM -0800, Joe Stringer wrote:
> Daniel Borkmann reports:
> 
> test_progs.c: In function ‘main’:
> test_progs.c:81:3: warning: ‘duration’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>    printf("%s:PASS:%s %d nsec\n", __func__, tag, duration);\
>    ^~~~~~
> test_progs.c:1706:8: note: ‘duration’ was declared here
>   __u32 duration;
>         ^~~~~~~~
> 
> Signed-off-by: Joe Stringer <joe@wand.net.nz>
I can repro with my locally compiled gcc 7.3.  It fixed the warning.

Acked-by: Martin KaFai Lau <kafai@fb.com>

> ---
> 
> I'm actually not able to reproduce this with GCC 7.3 or 8.2, so I'll
> rely on review to establish that this patch works as intended.
> ---
>  tools/testing/selftests/bpf/test_progs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 2d3c04f45530..c1e688f61061 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -1703,7 +1703,7 @@ static void test_reference_tracking()
>  	const char *file = "./test_sk_lookup_kern.o";
>  	struct bpf_object *obj;
>  	struct bpf_program *prog;
> -	__u32 duration;
> +	__u32 duration = 0;
>  	int err = 0;
>  
>  	obj = bpf_object__open(file);
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: [net-next PATCH v3] net: sched: cls_flower: Classify packets using port ranges
From: Nambiar, Amritha @ 2018-11-12 23:54 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, jakub.kicinski, sridhar.samudrala, jhs,
	xiyou.wangcong
In-Reply-To: <20181110071458.GA2019@nanopsycho.orion>

On 11/9/2018 11:14 PM, Jiri Pirko wrote:
> Sat, Nov 10, 2018 at 01:11:10AM CET, amritha.nambiar@intel.com wrote:
> 
> [...]
> 
>> @@ -1026,8 +1122,7 @@ static void fl_init_dissector(struct flow_dissector *dissector,
>> 			     FLOW_DISSECTOR_KEY_IPV4_ADDRS, ipv4);
>> 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
>> 			     FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6);
>> -	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
>> -			     FLOW_DISSECTOR_KEY_PORTS, tp);
>> +	FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_PORTS, tp);
> 
> You still need to set the key under a condition. Something like:
> 	if (FL_KEY_IS_MASKED(mask, tp) ||
> 	    FL_KEY_IS_MASKED(mask, tp_min) ||
> 	    FL_KEY_IS_MASKED(mask, tp_max)
> 		FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_PORTS, tp);
> 

Yes, will do. Thanks!

> 
>> 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
>> 			     FLOW_DISSECTOR_KEY_IP, ip);
>> 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
> 
> [...]
> 

^ permalink raw reply

* [net-next PATCH v4] net: sched: cls_flower: Classify packets using port ranges
From: Amritha Nambiar @ 2018-11-13  0:15 UTC (permalink / raw)
  To: netdev, davem
  Cc: jakub.kicinski, amritha.nambiar, sridhar.samudrala, jhs,
	xiyou.wangcong, jiri

Added support in tc flower for filtering based on port ranges.

Example:
1. Match on a port range:
-------------------------
$ tc filter add dev enp4s0 protocol ip parent ffff:\
  prio 1 flower ip_proto tcp dst_port range 20-30 skip_hw\
  action drop

$ tc -s filter show dev enp4s0 parent ffff:
filter protocol ip pref 1 flower chain 0
filter protocol ip pref 1 flower chain 0 handle 0x1
  eth_type ipv4
  ip_proto tcp
  dst_port range 20-30
  skip_hw
  not_in_hw
        action order 1: gact action drop
         random type none pass val 0
         index 1 ref 1 bind 1 installed 85 sec used 3 sec
        Action statistics:
        Sent 460 bytes 10 pkt (dropped 10, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

2. Match on IP address and port range:
--------------------------------------
$ tc filter add dev enp4s0 protocol ip parent ffff:\
  prio 1 flower dst_ip 192.168.1.1 ip_proto tcp dst_port range 100-200\
  skip_hw action drop

$ tc -s filter show dev enp4s0 parent ffff:
filter protocol ip pref 1 flower chain 0 handle 0x2
  eth_type ipv4
  ip_proto tcp
  dst_ip 192.168.1.1
  dst_port range 100-200
  skip_hw
  not_in_hw
        action order 1: gact action drop
         random type none pass val 0
         index 2 ref 1 bind 1 installed 58 sec used 2 sec
        Action statistics:
        Sent 920 bytes 20 pkt (dropped 20, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

v4:
1. Added condition before setting port key.
2. Organized setting and dumping port range keys into functions
   and added validation of input range.

v3:
1. Moved new fields in UAPI enum to the end of enum.
2. Removed couple of empty lines.

v2:
Addressed Jiri's comments:
1. Added separate functions for dst and src comparisons.
2. Removed endpoint enum.
3. Added new bit TCA_FLOWER_FLAGS_RANGE to decide normal/range
  lookup.
4. Cleaned up fl_lookup function.

Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
---
 include/uapi/linux/pkt_cls.h |    7 ++
 net/sched/cls_flower.c       |  155 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 156 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 401d0c1..95d0db2 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -485,6 +485,11 @@ enum {
 
 	TCA_FLOWER_IN_HW_COUNT,
 
+	TCA_FLOWER_KEY_PORT_SRC_MIN,	/* be16 */
+	TCA_FLOWER_KEY_PORT_SRC_MAX,	/* be16 */
+	TCA_FLOWER_KEY_PORT_DST_MIN,	/* be16 */
+	TCA_FLOWER_KEY_PORT_DST_MAX,	/* be16 */
+
 	__TCA_FLOWER_MAX,
 };
 
@@ -518,6 +523,8 @@ enum {
 	TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
 };
 
+#define TCA_FLOWER_MASK_FLAGS_RANGE	(1 << 0) /* Range-based match */
+
 /* Match-all classifier */
 
 enum {
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index c6c3278..85e9f8e 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -55,6 +55,8 @@ struct fl_flow_key {
 	struct flow_dissector_key_ip ip;
 	struct flow_dissector_key_ip enc_ip;
 	struct flow_dissector_key_enc_opts enc_opts;
+	struct flow_dissector_key_ports tp_min;
+	struct flow_dissector_key_ports tp_max;
 } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
 
 struct fl_flow_mask_range {
@@ -65,6 +67,7 @@ struct fl_flow_mask_range {
 struct fl_flow_mask {
 	struct fl_flow_key key;
 	struct fl_flow_mask_range range;
+	u32 flags;
 	struct rhash_head ht_node;
 	struct rhashtable ht;
 	struct rhashtable_params filter_ht_params;
@@ -179,13 +182,89 @@ static void fl_clear_masked_range(struct fl_flow_key *key,
 	memset(fl_key_get_start(key, mask), 0, fl_mask_range(mask));
 }
 
-static struct cls_fl_filter *fl_lookup(struct fl_flow_mask *mask,
-				       struct fl_flow_key *mkey)
+static bool fl_range_port_dst_cmp(struct cls_fl_filter *filter,
+				  struct fl_flow_key *key,
+				  struct fl_flow_key *mkey)
+{
+	__be16 min_mask, max_mask, min_val, max_val;
+
+	min_mask = htons(filter->mask->key.tp_min.dst);
+	max_mask = htons(filter->mask->key.tp_max.dst);
+	min_val = htons(filter->key.tp_min.dst);
+	max_val = htons(filter->key.tp_max.dst);
+
+	if (min_mask && max_mask) {
+		if (htons(key->tp.dst) < min_val ||
+		    htons(key->tp.dst) > max_val)
+			return false;
+
+		/* skb does not have min and max values */
+		mkey->tp_min.dst = filter->mkey.tp_min.dst;
+		mkey->tp_max.dst = filter->mkey.tp_max.dst;
+	}
+	return true;
+}
+
+static bool fl_range_port_src_cmp(struct cls_fl_filter *filter,
+				  struct fl_flow_key *key,
+				  struct fl_flow_key *mkey)
+{
+	__be16 min_mask, max_mask, min_val, max_val;
+
+	min_mask = htons(filter->mask->key.tp_min.src);
+	max_mask = htons(filter->mask->key.tp_max.src);
+	min_val = htons(filter->key.tp_min.src);
+	max_val = htons(filter->key.tp_max.src);
+
+	if (min_mask && max_mask) {
+		if (htons(key->tp.src) < min_val ||
+		    htons(key->tp.src) > max_val)
+			return false;
+
+		/* skb does not have min and max values */
+		mkey->tp_min.src = filter->mkey.tp_min.src;
+		mkey->tp_max.src = filter->mkey.tp_max.src;
+	}
+	return true;
+}
+
+static struct cls_fl_filter *__fl_lookup(struct fl_flow_mask *mask,
+					 struct fl_flow_key *mkey)
 {
 	return rhashtable_lookup_fast(&mask->ht, fl_key_get_start(mkey, mask),
 				      mask->filter_ht_params);
 }
 
+static struct cls_fl_filter *fl_lookup_range(struct fl_flow_mask *mask,
+					     struct fl_flow_key *mkey,
+					     struct fl_flow_key *key)
+{
+	struct cls_fl_filter *filter, *f;
+
+	list_for_each_entry_rcu(filter, &mask->filters, list) {
+		if (!fl_range_port_dst_cmp(filter, key, mkey))
+			continue;
+
+		if (!fl_range_port_src_cmp(filter, key, mkey))
+			continue;
+
+		f = __fl_lookup(mask, mkey);
+		if (f)
+			return f;
+	}
+	return NULL;
+}
+
+static struct cls_fl_filter *fl_lookup(struct fl_flow_mask *mask,
+				       struct fl_flow_key *mkey,
+				       struct fl_flow_key *key)
+{
+	if ((mask->flags & TCA_FLOWER_MASK_FLAGS_RANGE))
+		return fl_lookup_range(mask, mkey, key);
+
+	return __fl_lookup(mask, mkey);
+}
+
 static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 		       struct tcf_result *res)
 {
@@ -208,7 +287,7 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 
 		fl_set_masked_key(&skb_mkey, &skb_key, mask);
 
-		f = fl_lookup(mask, &skb_mkey);
+		f = fl_lookup(mask, &skb_mkey, &skb_key);
 		if (f && !tc_skip_sw(f->flags)) {
 			*res = f->res;
 			return tcf_exts_exec(skb, &f->exts, res);
@@ -514,6 +593,31 @@ static void fl_set_key_val(struct nlattr **tb,
 		memcpy(mask, nla_data(tb[mask_type]), len);
 }
 
+static int fl_set_key_port_range(struct nlattr **tb, struct fl_flow_key *key,
+				 struct fl_flow_key *mask)
+{
+	fl_set_key_val(tb, &key->tp_min.dst,
+		       TCA_FLOWER_KEY_PORT_DST_MIN, &mask->tp_min.dst,
+		       TCA_FLOWER_UNSPEC, sizeof(key->tp_min.dst));
+	fl_set_key_val(tb, &key->tp_max.dst,
+		       TCA_FLOWER_KEY_PORT_DST_MAX, &mask->tp_max.dst,
+		       TCA_FLOWER_UNSPEC, sizeof(key->tp_max.dst));
+	fl_set_key_val(tb, &key->tp_min.src,
+		       TCA_FLOWER_KEY_PORT_SRC_MIN, &mask->tp_min.src,
+		       TCA_FLOWER_UNSPEC, sizeof(key->tp_min.src));
+	fl_set_key_val(tb, &key->tp_max.src,
+		       TCA_FLOWER_KEY_PORT_SRC_MAX, &mask->tp_max.src,
+		       TCA_FLOWER_UNSPEC, sizeof(key->tp_max.src));
+
+	if ((mask->tp_min.dst && mask->tp_max.dst &&
+	     htons(key->tp_max.dst) <= htons(key->tp_min.dst)) ||
+	     (mask->tp_min.src && mask->tp_max.src &&
+	      htons(key->tp_max.src) <= htons(key->tp_min.src)))
+		return -EINVAL;
+
+	return 0;
+}
+
 static int fl_set_key_mpls(struct nlattr **tb,
 			   struct flow_dissector_key_mpls *key_val,
 			   struct flow_dissector_key_mpls *key_mask)
@@ -921,6 +1025,14 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
 			       sizeof(key->arp.tha));
 	}
 
+	if (key->basic.ip_proto == IPPROTO_TCP ||
+	    key->basic.ip_proto == IPPROTO_UDP ||
+	    key->basic.ip_proto == IPPROTO_SCTP) {
+		ret = fl_set_key_port_range(tb, key, mask);
+		if (ret)
+			return ret;
+	}
+
 	if (tb[TCA_FLOWER_KEY_ENC_IPV4_SRC] ||
 	    tb[TCA_FLOWER_KEY_ENC_IPV4_DST]) {
 		key->enc_control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
@@ -1038,8 +1150,9 @@ static void fl_init_dissector(struct flow_dissector *dissector,
 			     FLOW_DISSECTOR_KEY_IPV4_ADDRS, ipv4);
 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6);
-	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
-			     FLOW_DISSECTOR_KEY_PORTS, tp);
+	if (FL_KEY_IS_MASKED(mask, tp) ||
+	    FL_KEY_IS_MASKED(mask, tp_min) || FL_KEY_IS_MASKED(mask, tp_max))
+		FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_PORTS, tp);
 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_IP, ip);
 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
@@ -1086,6 +1199,10 @@ static struct fl_flow_mask *fl_create_new_mask(struct cls_fl_head *head,
 
 	fl_mask_copy(newmask, mask);
 
+	if ((newmask->key.tp_min.dst && newmask->key.tp_max.dst) ||
+	    (newmask->key.tp_min.src && newmask->key.tp_max.src))
+		newmask->flags |= TCA_FLOWER_MASK_FLAGS_RANGE;
+
 	err = fl_init_mask_hashtable(newmask);
 	if (err)
 		goto errout_free;
@@ -1239,7 +1356,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
 		goto errout_idr;
 
 	if (!tc_skip_sw(fnew->flags)) {
-		if (!fold && fl_lookup(fnew->mask, &fnew->mkey)) {
+		if (!fold && __fl_lookup(fnew->mask, &fnew->mkey)) {
 			err = -EEXIST;
 			goto errout_mask;
 		}
@@ -1476,6 +1593,26 @@ static int fl_dump_key_val(struct sk_buff *skb,
 	return 0;
 }
 
+static int fl_dump_key_port_range(struct sk_buff *skb, struct fl_flow_key *key,
+				  struct fl_flow_key *mask)
+{
+	if (fl_dump_key_val(skb, &key->tp_min.dst, TCA_FLOWER_KEY_PORT_DST_MIN,
+			    &mask->tp_min.dst, TCA_FLOWER_UNSPEC,
+			    sizeof(key->tp_min.dst)) ||
+	    fl_dump_key_val(skb, &key->tp_max.dst, TCA_FLOWER_KEY_PORT_DST_MAX,
+			    &mask->tp_max.dst, TCA_FLOWER_UNSPEC,
+			    sizeof(key->tp_max.dst)) ||
+	    fl_dump_key_val(skb, &key->tp_min.src, TCA_FLOWER_KEY_PORT_SRC_MIN,
+			    &mask->tp_min.src, TCA_FLOWER_UNSPEC,
+			    sizeof(key->tp_min.src)) ||
+	    fl_dump_key_val(skb, &key->tp_max.src, TCA_FLOWER_KEY_PORT_SRC_MAX,
+			    &mask->tp_max.src, TCA_FLOWER_UNSPEC,
+			    sizeof(key->tp_max.src)))
+		return -1;
+
+	return 0;
+}
+
 static int fl_dump_key_mpls(struct sk_buff *skb,
 			    struct flow_dissector_key_mpls *mpls_key,
 			    struct flow_dissector_key_mpls *mpls_mask)
@@ -1812,6 +1949,12 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
 				  sizeof(key->arp.tha))))
 		goto nla_put_failure;
 
+	if ((key->basic.ip_proto == IPPROTO_TCP ||
+	     key->basic.ip_proto == IPPROTO_UDP ||
+	     key->basic.ip_proto == IPPROTO_SCTP) &&
+	     fl_dump_key_port_range(skb, key, mask))
+		goto nla_put_failure;
+
 	if (key->enc_control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS &&
 	    (fl_dump_key_val(skb, &key->enc_ipv4.src,
 			    TCA_FLOWER_KEY_ENC_IPV4_SRC, &mask->enc_ipv4.src,

^ permalink raw reply related

* [Patch net-next] net: get rid of __tcp_checksum_complete()
From: Cong Wang @ 2018-11-13  0:17 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang

__tcp_checksum_complete() is 100% same with __skb_checksum_complete()
and there is no other caller except tcp_checksum_complete().
So, just use __skb_checksum_complete() there.

Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/tcp.h | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 4743836bed2e..b84b694e8b3d 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1315,15 +1315,10 @@ static inline __sum16 tcp_v4_check(int len, __be32 saddr,
 	return csum_tcpudp_magic(saddr,daddr,len,IPPROTO_TCP,base);
 }
 
-static inline __sum16 __tcp_checksum_complete(struct sk_buff *skb)
-{
-	return __skb_checksum_complete(skb);
-}
-
 static inline bool tcp_checksum_complete(struct sk_buff *skb)
 {
 	return !skb_csum_unnecessary(skb) &&
-		__tcp_checksum_complete(skb);
+		__skb_checksum_complete(skb);
 }
 
 bool tcp_add_backlog(struct sock *sk, struct sk_buff *skb);
-- 
2.19.1

^ permalink raw reply related

* [PATCH net] net_sched: sch_fq: ensure maxrate fq parameter applies to EDT flows
From: Eric Dumazet @ 2018-11-13  0:17 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

When EDT conversion happened, fq lost the ability to enfore a maxrate
for all flows. It kept it for non EDT flows.

This commit restores the functionality.

Tested:

tc qd replace dev eth0 root fq maxrate 500Mbit
netperf -P0 -H host -- -O THROUGHPUT
489.75

Fixes: ab408b6dc744 ("tcp: switch tcp and sch_fq to new earliest departure time model")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/sch_fq.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 4b1af706896c07e5a0fe6d542dfcd530acdcf8f5..25a7cf6d380fd1ef5610a43a06dea488121b8206 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -469,22 +469,29 @@ static struct sk_buff *fq_dequeue(struct Qdisc *sch)
 		goto begin;
 	}
 	prefetch(&skb->end);
-	f->credit -= qdisc_pkt_len(skb);
+	plen = qdisc_pkt_len(skb);
+	f->credit -= plen;
 
-	if (ktime_to_ns(skb->tstamp) || !q->rate_enable)
+	if (!q->rate_enable)
 		goto out;
 
 	rate = q->flow_max_rate;
-	if (skb->sk)
-		rate = min(skb->sk->sk_pacing_rate, rate);
 
-	if (rate <= q->low_rate_threshold) {
-		f->credit = 0;
-		plen = qdisc_pkt_len(skb);
-	} else {
-		plen = max(qdisc_pkt_len(skb), q->quantum);
-		if (f->credit > 0)
-			goto out;
+	/* If EDT time was provided for this skb, we need to
+	 * update f->time_next_packet only if this qdisc enforces
+	 * a flow max rate.
+	 */
+	if (!skb->tstamp) {
+		if (skb->sk)
+			rate = min(skb->sk->sk_pacing_rate, rate);
+
+		if (rate <= q->low_rate_threshold) {
+			f->credit = 0;
+		} else {
+			plen = max(plen, q->quantum);
+			if (f->credit > 0)
+				goto out;
+		}
 	}
 	if (rate != ~0UL) {
 		u64 len = (u64)plen * NSEC_PER_SEC;
-- 
2.19.1.930.g4563a0d9d0-goog

^ permalink raw reply related

* [PATCH net-next 0/5] net: hns3: Add support of hardware GRO to HNS3 Driver
From: Salil Mehta @ 2018-11-13 10:13 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
	linux-kernel, linux-rdma, linuxarm

This patch-set adds support of hardware assisted GRO feature to
HNS3 driver on Rev B(=0x21) platform. Current hardware only
supports TCP/IPv{4|6} flows.

Peng Li (5):
  net: hns3: Enable HW GRO for Rev B(=0x21) HNS3 hardware
  net: hns3: Add handling of GRO Pkts not fully RX'ed in NAPI poll
  net: hns3: Add support for ethtool -K to enable/disable HW GRO
  net: hns3: Add skb chain when num of RX buf exceeds MAX_SKB_FRAGS
  net: hns3: Adds GRO params to SKB for the stack

 drivers/net/ethernet/hisilicon/hns3/hnae3.h   |   7 +
 .../net/ethernet/hisilicon/hns3/hns3_enet.c   | 289 ++++++++++++++----
 .../net/ethernet/hisilicon/hns3/hns3_enet.h   |  17 +-
 .../hisilicon/hns3/hns3pf/hclge_cmd.h         |   7 +
 .../hisilicon/hns3/hns3pf/hclge_main.c        |  45 +++
 .../hisilicon/hns3/hns3vf/hclgevf_cmd.h       |   8 +
 .../hisilicon/hns3/hns3vf/hclgevf_main.c      |  45 +++
 7 files changed, 351 insertions(+), 67 deletions(-)

-- 
2.17.1

^ permalink raw reply

* [PATCH net-next 1/5] net: hns3: Enable HW GRO for Rev B(=0x21) HNS3 hardware
From: Salil Mehta @ 2018-11-13 10:13 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
	linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20181113101307.6020-1-salil.mehta@huawei.com>

From: Peng Li <lipeng321@huawei.com>

HNS3 hardware Revision B(=0x21) supports Hardware GRO feature. This
patch enables this feature in the HNS3 PF/VF driver.

Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hnae3.h   |  4 ++
 .../net/ethernet/hisilicon/hns3/hns3_enet.c   |  4 +-
 .../hisilicon/hns3/hns3pf/hclge_cmd.h         |  7 ++++
 .../hisilicon/hns3/hns3pf/hclge_main.c        | 36 ++++++++++++++++++
 .../hisilicon/hns3/hns3vf/hclgevf_cmd.h       |  8 ++++
 .../hisilicon/hns3/hns3vf/hclgevf_main.c      | 37 +++++++++++++++++++
 6 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index f69d39f17bdd..21d934b7a2a3 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -52,6 +52,7 @@
 #define HNAE3_UNIC_CLIENT_INITED_B		0x4
 #define HNAE3_ROCE_CLIENT_INITED_B		0x5
 #define HNAE3_DEV_SUPPORT_FD_B			0x6
+#define HNAE3_DEV_SUPPORT_GRO_B			0x7
 
 #define HNAE3_DEV_SUPPORT_ROCE_DCB_BITS (BIT(HNAE3_DEV_SUPPORT_DCB_B) |\
 		BIT(HNAE3_DEV_SUPPORT_ROCE_B))
@@ -65,6 +66,9 @@
 #define hnae3_dev_fd_supported(hdev) \
 	hnae3_get_bit((hdev)->ae_dev->flag, HNAE3_DEV_SUPPORT_FD_B)
 
+#define hnae3_dev_gro_supported(hdev) \
+	hnae3_get_bit((hdev)->ae_dev->flag, HNAE3_DEV_SUPPORT_GRO_B)
+
 #define ring_ptr_move_fw(ring, p) \
 	((ring)->p = ((ring)->p + 1) % (ring)->desc_num)
 #define ring_ptr_move_bw(ring, p) \
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 8d07ec668d5d..a510ddfd45a5 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -1714,8 +1714,10 @@ static void hns3_disable_sriov(struct pci_dev *pdev)
 static void hns3_get_dev_capability(struct pci_dev *pdev,
 				    struct hnae3_ae_dev *ae_dev)
 {
-	if (pdev->revision >= 0x21)
+	if (pdev->revision >= 0x21) {
 		hnae3_set_bit(ae_dev->flag, HNAE3_DEV_SUPPORT_FD_B, 1);
+		hnae3_set_bit(ae_dev->flag, HNAE3_DEV_SUPPORT_GRO_B, 1);
+	}
 }
 
 /* hns3_probe - Device initialization routine
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
index 872cd4bdd70d..aef044d08b11 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
@@ -152,6 +152,7 @@ enum hclge_opcode_type {
 
 	/* TSO command */
 	HCLGE_OPC_TSO_GENERIC_CONFIG	= 0x0C01,
+	HCLGE_OPC_GRO_GENERIC_CONFIG    = 0x0C10,
 
 	/* RSS commands */
 	HCLGE_OPC_RSS_GENERIC_CONFIG	= 0x0D01,
@@ -758,6 +759,12 @@ struct hclge_cfg_tso_status_cmd {
 	u8 rsv[20];
 };
 
+#define HCLGE_GRO_EN_B		0
+struct hclge_cfg_gro_status_cmd {
+	__le16 gro_en;
+	u8 rsv[22];
+};
+
 #define HCLGE_TSO_MSS_MIN	256
 #define HCLGE_TSO_MSS_MAX	9668
 
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 43bfc730a62d..d02712446d50 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -921,6 +921,28 @@ static int hclge_config_tso(struct hclge_dev *hdev, int tso_mss_min,
 	return hclge_cmd_send(&hdev->hw, &desc, 1);
 }
 
+static int hclge_config_gro(struct hclge_dev *hdev, bool en)
+{
+	struct hclge_cfg_gro_status_cmd *req;
+	struct hclge_desc desc;
+	int ret;
+
+	if (!hnae3_dev_gro_supported(hdev))
+		return 0;
+
+	hclge_cmd_setup_basic_desc(&desc, HCLGE_OPC_GRO_GENERIC_CONFIG, false);
+	req = (struct hclge_cfg_gro_status_cmd *)desc.data;
+
+	req->gro_en = cpu_to_le16(en ? 1 : 0);
+
+	ret = hclge_cmd_send(&hdev->hw, &desc, 1);
+	if (ret)
+		dev_err(&hdev->pdev->dev,
+			"GRO hardware config cmd failed, ret = %d\n", ret);
+
+	return ret;
+}
+
 static int hclge_alloc_tqps(struct hclge_dev *hdev)
 {
 	struct hclge_tqp *tqp;
@@ -7090,6 +7112,13 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
 		goto err_mdiobus_unreg;
 	}
 
+	ret = hclge_config_gro(hdev, true);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Failed to enable GRO in hardware, ret =%d\n", ret);
+		goto err_mdiobus_unreg;
+	}
+
 	ret = hclge_init_vlan_config(hdev);
 	if (ret) {
 		dev_err(&pdev->dev, "VLAN init fail, ret =%d\n", ret);
@@ -7221,6 +7250,13 @@ static int hclge_reset_ae_dev(struct hnae3_ae_dev *ae_dev)
 		return ret;
 	}
 
+	ret = hclge_config_gro(hdev, true);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Failed to enable GRO in hardware, ret =%d\n", ret);
+		return ret;
+	}
+
 	ret = hclge_init_vlan_config(hdev);
 	if (ret) {
 		dev_err(&pdev->dev, "VLAN init fail, ret =%d\n", ret);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.h b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.h
index 090541de0c7d..47030b42341f 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.h
@@ -87,6 +87,8 @@ enum hclgevf_opcode_type {
 	HCLGEVF_OPC_QUERY_TX_STATUS	= 0x0B03,
 	HCLGEVF_OPC_QUERY_RX_STATUS	= 0x0B13,
 	HCLGEVF_OPC_CFG_COM_TQP_QUEUE	= 0x0B20,
+	/* GRO command */
+	HCLGEVF_OPC_GRO_GENERIC_CONFIG  = 0x0C10,
 	/* RSS cmd */
 	HCLGEVF_OPC_RSS_GENERIC_CONFIG	= 0x0D01,
 	HCLGEVF_OPC_RSS_INPUT_TUPLE     = 0x0D02,
@@ -149,6 +151,12 @@ struct hclgevf_query_res_cmd {
 	__le16 rsv[7];
 };
 
+#define HCLGEVF_GRO_EN_B               0
+struct hclgevf_cfg_gro_status_cmd {
+	__le16 gro_en;
+	u8 rsv[22];
+};
+
 #define HCLGEVF_RSS_DEFAULT_OUTPORT_B	4
 #define HCLGEVF_RSS_HASH_KEY_OFFSET_B	4
 #define HCLGEVF_RSS_HASH_KEY_NUM	16
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 6b4d1477055f..3f256414fbe4 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -1655,6 +1655,29 @@ static int hclgevf_init_roce_base_info(struct hclgevf_dev *hdev)
 	return 0;
 }
 
+static int hclgevf_config_gro(struct hclgevf_dev *hdev, bool en)
+{
+	struct hclgevf_cfg_gro_status_cmd *req;
+	struct hclgevf_desc desc;
+	int ret;
+
+	if (!hnae3_dev_gro_supported(hdev))
+		return 0;
+
+	hclgevf_cmd_setup_basic_desc(&desc, HCLGEVF_OPC_GRO_GENERIC_CONFIG,
+				     false);
+	req = (struct hclgevf_cfg_gro_status_cmd *)desc.data;
+
+	req->gro_en = cpu_to_le16(en ? 1 : 0);
+
+	ret = hclgevf_cmd_send(&hdev->hw, &desc, 1);
+	if (ret)
+		dev_err(&hdev->pdev->dev,
+			"VF GRO hardware config cmd failed, ret = %d.\n", ret);
+
+	return ret;
+}
+
 static int hclgevf_rss_init_hw(struct hclgevf_dev *hdev)
 {
 	struct hclgevf_rss_cfg *rss_cfg = &hdev->rss_cfg;
@@ -2122,6 +2145,13 @@ static int hclgevf_reset_hdev(struct hclgevf_dev *hdev)
 		return ret;
 	}
 
+	ret = hclgevf_config_gro(hdev, true);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"failed to enable VF GRO in hardware, ret =%d\n", ret);
+		return ret;
+	}
+
 	ret = hclgevf_init_vlan_config(hdev);
 	if (ret) {
 		dev_err(&hdev->pdev->dev,
@@ -2199,6 +2229,13 @@ static int hclgevf_init_hdev(struct hclgevf_dev *hdev)
 		goto err_config;
 	}
 
+	ret = hclgevf_config_gro(hdev, true);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Failed to enable VF GRO in hardware, ret =%d\n", ret);
+		goto err_config;
+	}
+
 	/* Initialize RSS for this VF */
 	ret = hclgevf_rss_init_hw(hdev);
 	if (ret) {
-- 
2.17.1

^ permalink raw reply related

* [PATCH net-next 2/5] net: hns3: Add handling of GRO Pkts not fully RX'ed in NAPI poll
From: Salil Mehta @ 2018-11-13 10:13 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
	linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20181113101307.6020-1-salil.mehta@huawei.com>

From: Peng Li <lipeng321@huawei.com>

The "FE bit" in the description means the last description for
a packets. When HW GRO enable, HW write data to ring every
packet/buffer, there is greater probability that driver handle
with the describtion but HW still not set the "FE bit".

When driver handle the packet and HW still not set "FE bit",
driver stores skb and bd_num in rx ring, and continue to use the
skb and bd_num in next napi.

Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 .../net/ethernet/hisilicon/hns3/hns3_enet.c   | 196 ++++++++++++------
 .../net/ethernet/hisilicon/hns3/hns3_enet.h   |   6 +
 2 files changed, 140 insertions(+), 62 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index a510ddfd45a5..d8c5e1198670 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -2383,6 +2383,90 @@ static bool hns3_parse_vlan_tag(struct hns3_enet_ring *ring,
 	}
 }
 
+static int hns3_alloc_skb(struct hns3_enet_ring *ring, int length,
+			  unsigned char *va)
+{
+#define HNS3_NEED_ADD_FRAG	1
+	struct hns3_desc_cb *desc_cb = &ring->desc_cb[ring->next_to_clean];
+	struct net_device *netdev = ring->tqp->handle->kinfo.netdev;
+	struct sk_buff *skb;
+
+	ring->skb = napi_alloc_skb(&ring->tqp_vector->napi, HNS3_RX_HEAD_SIZE);
+	skb = ring->skb;
+	if (unlikely(!skb)) {
+		netdev_err(netdev, "alloc rx skb fail\n");
+
+		u64_stats_update_begin(&ring->syncp);
+		ring->stats.sw_err_cnt++;
+		u64_stats_update_end(&ring->syncp);
+
+		return -ENOMEM;
+	}
+
+	prefetchw(skb->data);
+
+	ring->pending_buf = 1;
+	if (length <= HNS3_RX_HEAD_SIZE) {
+		memcpy(__skb_put(skb, length), va, ALIGN(length, sizeof(long)));
+
+		/* We can reuse buffer as-is, just make sure it is local */
+		if (likely(page_to_nid(desc_cb->priv) == numa_node_id()))
+			desc_cb->reuse_flag = 1;
+		else /* This page cannot be reused so discard it */
+			put_page(desc_cb->priv);
+
+		ring_ptr_move_fw(ring, next_to_clean);
+		return 0;
+	}
+	u64_stats_update_begin(&ring->syncp);
+	ring->stats.seg_pkt_cnt++;
+	u64_stats_update_end(&ring->syncp);
+
+	ring->pull_len = eth_get_headlen(va, HNS3_RX_HEAD_SIZE);
+	__skb_put(skb, ring->pull_len);
+	hns3_nic_reuse_page(skb, 0, ring, ring->pull_len,
+			    desc_cb);
+	ring_ptr_move_fw(ring, next_to_clean);
+
+	return HNS3_NEED_ADD_FRAG;
+}
+
+static int hns3_add_frag(struct hns3_enet_ring *ring, struct hns3_desc *desc,
+			 struct sk_buff **out_skb, bool pending)
+{
+	struct sk_buff *skb = *out_skb;
+	struct hns3_desc_cb *desc_cb;
+	struct hns3_desc *pre_desc;
+	u32 bd_base_info;
+	int pre_bd;
+
+	/* if there is pending bd, the SW param next_to_clean has moved
+	 * to next and the next is NULL
+	 */
+	if (pending) {
+		pre_bd = (ring->next_to_clean - 1 + ring->desc_num) %
+			ring->desc_num;
+		pre_desc = &ring->desc[pre_bd];
+		bd_base_info = le32_to_cpu(pre_desc->rx.bd_base_info);
+	} else {
+		bd_base_info = le32_to_cpu(desc->rx.bd_base_info);
+	}
+
+	while (!hnae3_get_bit(bd_base_info, HNS3_RXD_FE_B)) {
+		desc = &ring->desc[ring->next_to_clean];
+		desc_cb = &ring->desc_cb[ring->next_to_clean];
+		bd_base_info = le32_to_cpu(desc->rx.bd_base_info);
+		if (!hnae3_get_bit(bd_base_info, HNS3_RXD_VLD_B))
+			return -ENXIO;
+
+		hns3_nic_reuse_page(skb, ring->pending_buf, ring, 0, desc_cb);
+		ring_ptr_move_fw(ring, next_to_clean);
+		ring->pending_buf++;
+	}
+
+	return 0;
+}
+
 static void hns3_set_rx_skb_rss_type(struct hns3_enet_ring *ring,
 				     struct sk_buff *skb)
 {
@@ -2399,18 +2483,16 @@ static void hns3_set_rx_skb_rss_type(struct hns3_enet_ring *ring,
 }
 
 static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
-			     struct sk_buff **out_skb, int *out_bnum)
+			     struct sk_buff **out_skb)
 {
 	struct net_device *netdev = ring->tqp->handle->kinfo.netdev;
+	struct sk_buff *skb = ring->skb;
 	struct hns3_desc_cb *desc_cb;
 	struct hns3_desc *desc;
-	struct sk_buff *skb;
-	unsigned char *va;
 	u32 bd_base_info;
-	int pull_len;
 	u32 l234info;
 	int length;
-	int bnum;
+	int ret;
 
 	desc = &ring->desc[ring->next_to_clean];
 	desc_cb = &ring->desc_cb[ring->next_to_clean];
@@ -2422,9 +2504,10 @@ static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
 
 	/* Check valid BD */
 	if (unlikely(!hnae3_get_bit(bd_base_info, HNS3_RXD_VLD_B)))
-		return -EFAULT;
+		return -ENXIO;
 
-	va = (unsigned char *)desc_cb->buf + desc_cb->page_offset;
+	if (!skb)
+		ring->va = (unsigned char *)desc_cb->buf + desc_cb->page_offset;
 
 	/* Prefetch first cache line of first page
 	 * Idea is to cache few bytes of the header of the packet. Our L1 Cache
@@ -2433,62 +2516,42 @@ static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
 	 * lines. In such a case, single fetch would suffice to cache in the
 	 * relevant part of the header.
 	 */
-	prefetch(va);
+	prefetch(ring->va);
 #if L1_CACHE_BYTES < 128
-	prefetch(va + L1_CACHE_BYTES);
+	prefetch(ring->va + L1_CACHE_BYTES);
 #endif
 
-	skb = *out_skb = napi_alloc_skb(&ring->tqp_vector->napi,
-					HNS3_RX_HEAD_SIZE);
-	if (unlikely(!skb)) {
-		netdev_err(netdev, "alloc rx skb fail\n");
+	if (!skb) {
+		ret = hns3_alloc_skb(ring, length, ring->va);
+		*out_skb = skb = ring->skb;
 
-		u64_stats_update_begin(&ring->syncp);
-		ring->stats.sw_err_cnt++;
-		u64_stats_update_end(&ring->syncp);
-
-		return -ENOMEM;
-	}
-
-	prefetchw(skb->data);
-
-	bnum = 1;
-	if (length <= HNS3_RX_HEAD_SIZE) {
-		memcpy(__skb_put(skb, length), va, ALIGN(length, sizeof(long)));
-
-		/* We can reuse buffer as-is, just make sure it is local */
-		if (likely(page_to_nid(desc_cb->priv) == numa_node_id()))
-			desc_cb->reuse_flag = 1;
-		else /* This page cannot be reused so discard it */
-			put_page(desc_cb->priv);
+		if (ret < 0) /* alloc buffer fail */
+			return ret;
+		if (ret > 0) { /* need add frag */
+			ret = hns3_add_frag(ring, desc, &skb, false);
+			if (ret)
+				return ret;
 
-		ring_ptr_move_fw(ring, next_to_clean);
+			/* As the head data may be changed when GRO enable, copy
+			 * the head data in after other data rx completed
+			 */
+			memcpy(skb->data, ring->va,
+			       ALIGN(ring->pull_len, sizeof(long)));
+		}
 	} else {
-		u64_stats_update_begin(&ring->syncp);
-		ring->stats.seg_pkt_cnt++;
-		u64_stats_update_end(&ring->syncp);
-
-		pull_len = eth_get_headlen(va, HNS3_RX_HEAD_SIZE);
-
-		memcpy(__skb_put(skb, pull_len), va,
-		       ALIGN(pull_len, sizeof(long)));
-
-		hns3_nic_reuse_page(skb, 0, ring, pull_len, desc_cb);
-		ring_ptr_move_fw(ring, next_to_clean);
+		ret = hns3_add_frag(ring, desc, &skb, true);
+		if (ret)
+			return ret;
 
-		while (!hnae3_get_bit(bd_base_info, HNS3_RXD_FE_B)) {
-			desc = &ring->desc[ring->next_to_clean];
-			desc_cb = &ring->desc_cb[ring->next_to_clean];
-			bd_base_info = le32_to_cpu(desc->rx.bd_base_info);
-			hns3_nic_reuse_page(skb, bnum, ring, 0, desc_cb);
-			ring_ptr_move_fw(ring, next_to_clean);
-			bnum++;
-		}
+		/* As the head data may be changed when GRO enable, copy
+		 * the head data in after other data rx completed
+		 */
+		memcpy(skb->data, ring->va,
+		       ALIGN(ring->pull_len, sizeof(long)));
 	}
 
-	*out_bnum = bnum;
-
 	l234info = le32_to_cpu(desc->rx.l234_info);
+	bd_base_info = le32_to_cpu(desc->rx.bd_base_info);
 
 	/* Based on hw strategy, the tag offloaded will be stored at
 	 * ot_vlan_tag in two layer tag case, and stored at vlan_tag
@@ -2539,6 +2602,7 @@ static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
 	ring->tqp_vector->rx_group.total_bytes += skb->len;
 
 	hns3_rx_checksum(ring, skb, desc);
+	*out_skb = skb;
 	hns3_set_rx_skb_rss_type(ring, skb);
 
 	return 0;
@@ -2551,9 +2615,9 @@ int hns3_clean_rx_ring(
 #define RCB_NOF_ALLOC_RX_BUFF_ONCE 16
 	struct net_device *netdev = ring->tqp->handle->kinfo.netdev;
 	int recv_pkts, recv_bds, clean_count, err;
-	int unused_count = hns3_desc_unused(ring);
-	struct sk_buff *skb = NULL;
-	int num, bnum = 0;
+	int unused_count = hns3_desc_unused(ring) - ring->pending_buf;
+	struct sk_buff *skb = ring->skb;
+	int num;
 
 	num = readl_relaxed(ring->tqp->io_base + HNS3_RING_RX_RING_FBDNUM_REG);
 	rmb(); /* Make sure num taken effect before the other data is touched */
@@ -2567,24 +2631,32 @@ int hns3_clean_rx_ring(
 			hns3_nic_alloc_rx_buffers(ring,
 						  clean_count + unused_count);
 			clean_count = 0;
-			unused_count = hns3_desc_unused(ring);
+			unused_count = hns3_desc_unused(ring) -
+					ring->pending_buf;
 		}
 
 		/* Poll one pkt */
-		err = hns3_handle_rx_bd(ring, &skb, &bnum);
+		err = hns3_handle_rx_bd(ring, &skb);
 		if (unlikely(!skb)) /* This fault cannot be repaired */
 			goto out;
 
-		recv_bds += bnum;
-		clean_count += bnum;
-		if (unlikely(err)) {  /* Do jump the err */
-			recv_pkts++;
+		if (err == -ENXIO) { /* Do not get FE for the packet */
+			goto out;
+		} else if (unlikely(err)) {  /* Do jump the err */
+			recv_bds += ring->pending_buf;
+			clean_count += ring->pending_buf;
+			ring->skb = NULL;
+			ring->pending_buf = 0;
 			continue;
 		}
 
 		/* Do update ip stack process */
 		skb->protocol = eth_type_trans(skb, netdev);
 		rx_fn(ring, skb);
+		recv_bds += ring->pending_buf;
+		clean_count += ring->pending_buf;
+		ring->skb = NULL;
+		ring->pending_buf = 0;
 
 		recv_pkts++;
 	}
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index 10ff18af3cc7..d8c0998127be 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -401,11 +401,17 @@ struct hns3_enet_ring {
 	 */
 	int next_to_clean;
 
+	int pull_len; /* head length for current packet */
+	unsigned char *va; /* first buffer address for current packet */
+
 	u32 flag;          /* ring attribute */
 	int irq_init_flag;
 
 	int numa_node;
 	cpumask_t affinity_mask;
+
+	int pending_buf;
+	struct sk_buff *skb;
 };
 
 struct hns_queue;
-- 
2.17.1

^ permalink raw reply related

* [PATCH net-next 3/5] net: hns3: Add support for ethtool -K to enable/disable HW GRO
From: Salil Mehta @ 2018-11-13 10:13 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
	linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20181113101307.6020-1-salil.mehta@huawei.com>

From: Peng Li <lipeng321@huawei.com>

This patch adds support of ethtool -K to enable/disable
hardware GRO in HNS3 PF/VF driver.

Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hnae3.h         |  3 +++
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.c     | 13 ++++++++++++-
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c |  9 +++++++++
 .../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c   |  8 ++++++++
 4 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index 21d934b7a2a3..1746172ffbf7 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -305,6 +305,8 @@ struct hnae3_ae_dev {
  *   Set vlan filter config of vf
  * enable_hw_strip_rxvtag()
  *   Enable/disable hardware strip vlan tag of packets received
+ * set_gro_en
+ *   Enable/disable HW GRO
  */
 struct hnae3_ae_ops {
 	int (*init_ae_dev)(struct hnae3_ae_dev *ae_dev);
@@ -449,6 +451,7 @@ struct hnae3_ae_ops {
 	bool (*get_hw_reset_stat)(struct hnae3_handle *handle);
 	bool (*ae_dev_resetting)(struct hnae3_handle *handle);
 	unsigned long (*ae_dev_reset_cnt)(struct hnae3_handle *handle);
+	int (*set_gro_en)(struct hnae3_handle *handle, int enable);
 };
 
 struct hnae3_dcb_ops {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index d8c5e1198670..860067898471 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -1345,6 +1345,15 @@ static int hns3_nic_set_features(struct net_device *netdev,
 			priv->ops.maybe_stop_tx = hns3_nic_maybe_stop_tx;
 	}
 
+	if (changed & (NETIF_F_GRO_HW) && h->ae_algo->ops->set_gro_en) {
+		if (features & NETIF_F_GRO_HW)
+			ret = h->ae_algo->ops->set_gro_en(h, true);
+		else
+			ret = h->ae_algo->ops->set_gro_en(h, false);
+		if (ret)
+			return ret;
+	}
+
 	if ((changed & NETIF_F_HW_VLAN_CTAG_FILTER) &&
 	    h->ae_algo->ops->enable_vlan_filter) {
 		if (features & NETIF_F_HW_VLAN_CTAG_FILTER)
@@ -1929,7 +1938,9 @@ static void hns3_set_default_feature(struct net_device *netdev)
 		NETIF_F_GSO_UDP_TUNNEL_CSUM | NETIF_F_SCTP_CRC;
 
 	if (pdev->revision >= 0x21) {
-		netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+		netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER |
+			NETIF_F_GRO_HW;
+		netdev->features |= NETIF_F_GRO_HW;
 
 		if (!(h->flags & HNAE3_SUPPORT_VF)) {
 			netdev->hw_features |= NETIF_F_NTUPLE;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index d02712446d50..ed25d262064a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -7673,6 +7673,14 @@ static void hclge_get_link_mode(struct hnae3_handle *handle,
 	}
 }
 
+static int hclge_gro_en(struct hnae3_handle *handle, int enable)
+{
+	struct hclge_vport *vport = hclge_get_vport(handle);
+	struct hclge_dev *hdev = vport->back;
+
+	return hclge_config_gro(hdev, enable);
+}
+
 static const struct hnae3_ae_ops hclge_ops = {
 	.init_ae_dev = hclge_init_ae_dev,
 	.uninit_ae_dev = hclge_uninit_ae_dev,
@@ -7744,6 +7752,7 @@ static const struct hnae3_ae_ops hclge_ops = {
 	.get_hw_reset_stat = hclge_get_hw_reset_stat,
 	.ae_dev_resetting = hclge_ae_dev_resetting,
 	.ae_dev_reset_cnt = hclge_ae_dev_reset_cnt,
+	.set_gro_en = hclge_gro_en,
 };
 
 static struct hnae3_ae_algo ae_algo = {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 3f256414fbe4..e51f8f7d52f1 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -2374,6 +2374,13 @@ void hclgevf_update_speed_duplex(struct hclgevf_dev *hdev, u32 speed,
 	hdev->hw.mac.duplex = duplex;
 }
 
+static int hclgevf_gro_en(struct hnae3_handle *handle, int enable)
+{
+	struct hclgevf_dev *hdev = hclgevf_ae_get_hdev(handle);
+
+	return hclgevf_config_gro(hdev, enable);
+}
+
 static void hclgevf_get_media_type(struct hnae3_handle *handle,
 				  u8 *media_type)
 {
@@ -2448,6 +2455,7 @@ static const struct hnae3_ae_ops hclgevf_ops = {
 	.get_hw_reset_stat = hclgevf_get_hw_reset_stat,
 	.ae_dev_resetting = hclgevf_ae_dev_resetting,
 	.ae_dev_reset_cnt = hclgevf_ae_dev_reset_cnt,
+	.set_gro_en = hclgevf_gro_en,
 };
 
 static struct hnae3_ae_algo ae_algovf = {
-- 
2.17.1

^ permalink raw reply related

* [PATCH net-next 4/5] net: hns3: Add skb chain when num of RX buf exceeds MAX_SKB_FRAGS
From: Salil Mehta @ 2018-11-13 10:13 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
	linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20181113101307.6020-1-salil.mehta@huawei.com>

From: Peng Li <lipeng321@huawei.com>

MAX_SKB_FRAGS in protocol stack is defined as:

MAX_SKB_FRAGS is 17 when PAGE_SIZE is 4K. If HW enable GRO, it may
merge small packets and the rx buffer may be more than
MAX_SKB_FRAGS. So driver will add skb chain when RX buffer num.
more than MAX_SKB_FRAGS.

Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 .../net/ethernet/hisilicon/hns3/hns3_enet.c   | 37 ++++++++++++++++++-
 .../net/ethernet/hisilicon/hns3/hns3_enet.h   |  2 +
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 860067898471..7776089b6bc2 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -2361,6 +2361,9 @@ static void hns3_rx_checksum(struct hns3_enet_ring *ring, struct sk_buff *skb,
 
 static void hns3_rx_skb(struct hns3_enet_ring *ring, struct sk_buff *skb)
 {
+	if (skb_has_frag_list(skb))
+		napi_gro_flush(&ring->tqp_vector->napi, false);
+
 	napi_gro_receive(&ring->tqp_vector->napi, skb);
 }
 
@@ -2417,6 +2420,8 @@ static int hns3_alloc_skb(struct hns3_enet_ring *ring, int length,
 	prefetchw(skb->data);
 
 	ring->pending_buf = 1;
+	ring->frag_num = 0;
+	ring->tail_skb = NULL;
 	if (length <= HNS3_RX_HEAD_SIZE) {
 		memcpy(__skb_put(skb, length), va, ALIGN(length, sizeof(long)));
 
@@ -2435,7 +2440,7 @@ static int hns3_alloc_skb(struct hns3_enet_ring *ring, int length,
 
 	ring->pull_len = eth_get_headlen(va, HNS3_RX_HEAD_SIZE);
 	__skb_put(skb, ring->pull_len);
-	hns3_nic_reuse_page(skb, 0, ring, ring->pull_len,
+	hns3_nic_reuse_page(skb, ring->frag_num++, ring, ring->pull_len,
 			    desc_cb);
 	ring_ptr_move_fw(ring, next_to_clean);
 
@@ -2446,6 +2451,8 @@ static int hns3_add_frag(struct hns3_enet_ring *ring, struct hns3_desc *desc,
 			 struct sk_buff **out_skb, bool pending)
 {
 	struct sk_buff *skb = *out_skb;
+	struct sk_buff *head_skb = *out_skb;
+	struct sk_buff *new_skb;
 	struct hns3_desc_cb *desc_cb;
 	struct hns3_desc *pre_desc;
 	u32 bd_base_info;
@@ -2470,7 +2477,33 @@ static int hns3_add_frag(struct hns3_enet_ring *ring, struct hns3_desc *desc,
 		if (!hnae3_get_bit(bd_base_info, HNS3_RXD_VLD_B))
 			return -ENXIO;
 
-		hns3_nic_reuse_page(skb, ring->pending_buf, ring, 0, desc_cb);
+		if (unlikely(ring->frag_num >= MAX_SKB_FRAGS)) {
+			new_skb = napi_alloc_skb(&ring->tqp_vector->napi,
+						 HNS3_RX_HEAD_SIZE);
+			if (unlikely(!new_skb)) {
+				netdev_err(ring->tqp->handle->kinfo.netdev,
+					   "alloc rx skb frag fail\n");
+				return -ENXIO;
+			}
+			ring->frag_num = 0;
+
+			if (ring->tail_skb) {
+				ring->tail_skb->next = new_skb;
+				ring->tail_skb = new_skb;
+			} else {
+				skb_shinfo(skb)->frag_list = new_skb;
+				ring->tail_skb = new_skb;
+			}
+		}
+
+		if (ring->tail_skb) {
+			head_skb->truesize += hnae3_buf_size(ring);
+			head_skb->data_len += le16_to_cpu(desc->rx.size);
+			head_skb->len += le16_to_cpu(desc->rx.size);
+			skb = ring->tail_skb;
+		}
+
+		hns3_nic_reuse_page(skb, ring->frag_num++, ring, 0, desc_cb);
 		ring_ptr_move_fw(ring, next_to_clean);
 		ring->pending_buf++;
 	}
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index d8c0998127be..8e56b7e44978 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -402,6 +402,7 @@ struct hns3_enet_ring {
 	int next_to_clean;
 
 	int pull_len; /* head length for current packet */
+	u32 frag_num;
 	unsigned char *va; /* first buffer address for current packet */
 
 	u32 flag;          /* ring attribute */
@@ -412,6 +413,7 @@ struct hns3_enet_ring {
 
 	int pending_buf;
 	struct sk_buff *skb;
+	struct sk_buff *tail_skb;
 };
 
 struct hns_queue;
-- 
2.17.1

^ permalink raw reply related

* [PATCH net-next 5/5] net: hns3: Adds GRO params to SKB for the stack
From: Salil Mehta @ 2018-11-13 10:13 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
	linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20181113101307.6020-1-salil.mehta@huawei.com>

From: Peng Li <lipeng321@huawei.com>

When HW GRO enable, protocol stack will not do GRO again,
driver should add gro param to the skb for the protocol
stack..

Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 .../net/ethernet/hisilicon/hns3/hns3_enet.c   | 43 +++++++++++++++++++
 .../net/ethernet/hisilicon/hns3/hns3_enet.h   |  9 ++--
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 7776089b6bc2..22220af92aa9 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -15,6 +15,7 @@
 #include <linux/vermagic.h>
 #include <net/gre.h>
 #include <net/pkt_cls.h>
+#include <net/tcp.h>
 #include <net/vxlan.h>
 
 #include "hnae3.h"
@@ -2318,6 +2319,12 @@ static void hns3_rx_checksum(struct hns3_enet_ring *ring, struct sk_buff *skb,
 	if (!(netdev->features & NETIF_F_RXCSUM))
 		return;
 
+	/* We MUST enable hardware checksum before enabling hardware GRO */
+	if (skb_shinfo(skb)->gso_size) {
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
+		return;
+	}
+
 	/* check if hardware has done checksum */
 	if (!hnae3_get_bit(bd_base_info, HNS3_RXD_L3L4P_B))
 		return;
@@ -2511,6 +2518,39 @@ static int hns3_add_frag(struct hns3_enet_ring *ring, struct hns3_desc *desc,
 	return 0;
 }
 
+static void hns3_set_gro_param(struct sk_buff *skb, u32 l234info,
+			       u32 bd_base_info)
+{
+	u16 gro_count;
+	u32 l3_type;
+
+	gro_count = hnae3_get_field(l234info, HNS3_RXD_GRO_COUNT_M,
+				    HNS3_RXD_GRO_COUNT_S);
+	/* if there is no HW GRO, do not set gro params */
+	if (!gro_count)
+		return;
+
+	/* tcp_gro_complete() will copy NAPI_GRO_CB(skb)->count
+	 * to skb_shinfo(skb)->gso_segs
+	 */
+	NAPI_GRO_CB(skb)->count = gro_count;
+
+	l3_type = hnae3_get_field(l234info, HNS3_RXD_L3ID_M,
+				  HNS3_RXD_L3ID_S);
+	if (l3_type == HNS3_L3_TYPE_IPV4)
+		skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
+	else if (l3_type == HNS3_L3_TYPE_IPV6)
+		skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+	else
+		return;
+
+	skb_shinfo(skb)->gso_size = hnae3_get_field(bd_base_info,
+						    HNS3_RXD_GRO_SIZE_M,
+						    HNS3_RXD_GRO_SIZE_S);
+	if (skb_shinfo(skb)->gso_size)
+		tcp_gro_complete(skb);
+}
+
 static void hns3_set_rx_skb_rss_type(struct hns3_enet_ring *ring,
 				     struct sk_buff *skb)
 {
@@ -2645,6 +2685,9 @@ static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
 
 	ring->tqp_vector->rx_group.total_bytes += skb->len;
 
+	/* This is needed in order to enable forwarding support */
+	hns3_set_gro_param(skb, l234info, bd_base_info);
+
 	hns3_rx_checksum(ring, skb, desc);
 	*out_skb = skb;
 	hns3_set_rx_skb_rss_type(ring, skb);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index 8e56b7e44978..3365c9596983 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -109,6 +109,10 @@ enum hns3_nic_state {
 #define HNS3_RXD_DOI_B				21
 #define HNS3_RXD_OL3E_B				22
 #define HNS3_RXD_OL4E_B				23
+#define HNS3_RXD_GRO_COUNT_S			24
+#define HNS3_RXD_GRO_COUNT_M			(0x3f << HNS3_RXD_GRO_COUNT_S)
+#define HNS3_RXD_GRO_FIXID_B			30
+#define HNS3_RXD_GRO_ECN_B			31
 
 #define HNS3_RXD_ODMAC_S			0
 #define HNS3_RXD_ODMAC_M			(0x3 << HNS3_RXD_ODMAC_S)
@@ -135,9 +139,8 @@ enum hns3_nic_state {
 #define HNS3_RXD_TSIND_S			12
 #define HNS3_RXD_TSIND_M			(0x7 << HNS3_RXD_TSIND_S)
 #define HNS3_RXD_LKBK_B				15
-#define HNS3_RXD_HDL_S				16
-#define HNS3_RXD_HDL_M				(0x7ff << HNS3_RXD_HDL_S)
-#define HNS3_RXD_HSIND_B			31
+#define HNS3_RXD_GRO_SIZE_S			16
+#define HNS3_RXD_GRO_SIZE_M			(0x3ff << HNS3_RXD_GRO_SIZE_S)
 
 #define HNS3_TXD_L3T_S				0
 #define HNS3_TXD_L3T_M				(0x3 << HNS3_TXD_L3T_S)
-- 
2.17.1

^ permalink raw reply related

* Re: [iproute PATCH] man: ip-route.8: Document nexthop limit
From: David Ahern @ 2018-11-13  0:37 UTC (permalink / raw)
  To: Phil Sutter, Stephen Hemminger; +Cc: netdev
In-Reply-To: <20181112222101.9674-1-phil@nwl.cc>

On 11/12/18 2:21 PM, Phil Sutter wrote:
> diff --git a/man/man8/ip-route.8.in b/man/man8/ip-route.8.in
> index a33ce1f0f4006..383178c11331e 100644
> --- a/man/man8/ip-route.8.in
> +++ b/man/man8/ip-route.8.in
> @@ -589,6 +589,13 @@ argument lists:
>  route reflecting its relative bandwidth or quality.
>  .in -8
>  
> +The internal buffer used in iproute2 limits the maximum number of nexthops to
> +be specified in one go. If only a gateway address is given, the current buffer
> +size allows for 144 IPv6 nexthops and 253 IPv4 ones. If more are required, they
> +may be added to the existing route using
> +.B "ip route append"
> +command.
> +

That is not true for IPv4. 'ip ro append' adds a new route after the
existing route - an entry that can not be hit unless all of the nexthops
in the first route are down. 'ip ro prepend' adds a new entry before the
existing one meaning it takes precedence over the existing entries.

For IPv6, 'append' and 'prepend' both add new nexthops to the existing
entry.

^ permalink raw reply

* [PATCH net] net: bridge: fix per-port vlan stats use-after-free on destruction
From: Nikolay Aleksandrov @ 2018-11-13  1:01 UTC (permalink / raw)
  To: netdev; +Cc: roopa, davem, bridge, syzkaller-bugs, Nikolay Aleksandrov
In-Reply-To: <00000000000047feb5057a714975@google.com>

Syzbot reported a use-after-free of the global vlan context on port vlan
destruction. When I added per-port vlan stats I missed the fact that the
global vlan context can be freed before the per-port vlan rcu callback.
There're a few different ways to deal with this, I've chosen to add a
new private flag that is set only when per-port stats are allocated so
we can directly check it on destruction without dereferencing the global
context at all. The flag is internally controlled by the kernel and
user-space isn't allowed to set it.

Fixes: 9163a0fc1f0c ("net: bridge: add support for per-port vlan stats")
Reported-by: syzbot+04681da557a0e49a52e5@syzkaller.appspotmail.com
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
 include/uapi/linux/if_bridge.h | 3 +++
 net/bridge/br_netlink.c        | 7 ++++++-
 net/bridge/br_vlan.c           | 3 ++-
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index e41eda3c71f1..fa1f72276712 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -130,6 +130,9 @@ enum {
 #define BRIDGE_VLAN_INFO_RANGE_BEGIN	(1<<3) /* VLAN is start of vlan range */
 #define BRIDGE_VLAN_INFO_RANGE_END	(1<<4) /* VLAN is end of vlan range */
 #define BRIDGE_VLAN_INFO_BRENTRY	(1<<5) /* Global bridge VLAN entry */
+#define BRIDGE_VLAN_INFO_PORT_STATS	(1<<6) /* Per-port VLAN stats */
+
+#define BRIDGE_VLAN_INFO_PRIVATE_FLAGS BRIDGE_VLAN_INFO_PORT_STATS
 
 struct bridge_vlan_info {
 	__u16 flags;
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 3345f1984542..c600fedcfb76 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -527,8 +527,12 @@ int br_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 static int br_vlan_info(struct net_bridge *br, struct net_bridge_port *p,
 			int cmd, struct bridge_vlan_info *vinfo, bool *changed)
 {
+	int err = -EINVAL;
 	bool curr_change;
-	int err = 0;
+
+	/* don't allow user-space control over private flags */
+	if (vinfo->flags & BRIDGE_VLAN_INFO_PRIVATE_FLAGS)
+		return err;
 
 	switch (cmd) {
 	case RTM_SETLINK:
@@ -548,6 +552,7 @@ static int br_vlan_info(struct net_bridge *br, struct net_bridge_port *p,
 		break;
 
 	case RTM_DELLINK:
+		err = 0;
 		if (p) {
 			if (!nbp_vlan_delete(p, vinfo->vid))
 				*changed = true;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 8c9297a01947..004e1f8c5040 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -197,7 +197,7 @@ static void nbp_vlan_rcu_free(struct rcu_head *rcu)
 	v = container_of(rcu, struct net_bridge_vlan, rcu);
 	WARN_ON(br_vlan_is_master(v));
 	/* if we had per-port stats configured then free them here */
-	if (v->brvlan->stats != v->stats)
+	if (v->flags & BRIDGE_VLAN_INFO_PORT_STATS)
 		free_percpu(v->stats);
 	v->stats = NULL;
 	kfree(v);
@@ -264,6 +264,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags)
 				err = -ENOMEM;
 				goto out_filt;
 			}
+			v->flags |= BRIDGE_VLAN_INFO_PORT_STATS;
 		} else {
 			v->stats = masterv->stats;
 		}
-- 
2.17.2

^ permalink raw reply related

* [PATCH][net-next][v2] net: remove BUG_ON from __pskb_pull_tail
From: Li RongQing @ 2018-11-13  1:16 UTC (permalink / raw)
  To: netdev

if list is NULL pointer, and the following access of list
will trigger panic, which is same as BUG_ON

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 net/core/skbuff.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 396fcb3baad0..d69503d66021 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1925,8 +1925,6 @@ void *__pskb_pull_tail(struct sk_buff *skb, int delta)
 		struct sk_buff *insp = NULL;
 
 		do {
-			BUG_ON(!list);
-
 			if (list->len <= eat) {
 				/* Eaten as whole. */
 				eat -= list->len;
-- 
2.16.2

^ permalink raw reply related

* Re: [PATCH] brcmfmac: Use standard SKB list accessors in brcmf_sdiod_sglist_rw.
From: Kalle Valo @ 2018-11-13 11:19 UTC (permalink / raw)
  To: David Miller
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20181110.163402.130407398146253939.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> writes:

> [ As I am trying to remove direct SKB list pointer accesses I am
>   committing this to net-next.  If this causes a lot of grief I
>   can and will revert, just let me know. ]
>
> Instead of direct SKB list pointer accesses.
>
> The loops in this function had to be rewritten to accommodate this
> more easily.
>
> The first loop iterates now over the target list in the outer loop,
> and triggers an mmc data operation when the per-operation limits are
> hit.
>
> Then after the loops, if we have any residue, we trigger the last
> and final operation.
>
> For the page aligned workaround, where we have to copy the read data
> back into the original list of SKBs, we use a two-tiered loop.  The
> outer loop stays the same and iterates over pktlist, and then we have
> an inner loop which uses skb_peek_next().  The break logic has been
> simplified because we know that the aggregate length of the SKBs in
> the source and destination lists are the same.
>
> This change also ends up fixing a bug, having to do with the
> maintainance of the seg_sz variable and how it drove the outermost
> loop.  It begins as:
>
> 	seg_sz = target_list->qlen;
>
> ie. the number of packets in the target_list queue.  The loop
> structure was then:
>
> 	while (seq_sz) {
> 		...
> 		while (not at end of target_list) {
> 			...
> 			sg_cnt++
> 			...
> 		}
> 		...
> 		seg_sz -= sg_cnt;
>
> The assumption built into that last statement is that sg_cnt counts
> how many packets from target_list have been fully processed by the
> inner loop.  But this not true.
>
> If we hit one of the limits, such as the max segment size or the max
> request size, we will break and copy a partial packet then contine
> back up to the top of the outermost loop.
>
> With the new loops we don't have this problem as we don't guard the
> loop exit with a packet count, but instead use the progression of the
> pkt_next SKB through the list to the end.  The general structure is:
>
> 	sg_cnt = 0;
> 	skb_queue_walk(target_list, pkt_next) {
> 		pkt_offset = 0;
> 		...
> 		sg_cnt++;
> 		...
> 		while (pkt_offset < pkt_next->len) {
> 			pkt_offset += sg_data_size;
> 			if (queued up max per request)
> 				mmc_submit_one();
> 		}
> 	}
> 	if (sg_cnt)
> 		mmc_submit_one();
>
> The variables that maintain where we are in the MMC command state such
> as req_sz, sg_cnt, and sgl are reset when we emit one of these full
> sized requests.
>
> Signed-off-by: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

Looks good to me, thanks.

Acked-by: Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH net-next 1/5] net: hns3: Enable HW GRO for Rev B(=0x21) HNS3 hardware
From: Leon Romanovsky @ 2018-11-13 11:24 UTC (permalink / raw)
  To: Salil Mehta
  Cc: davem, yisen.zhuang, lipeng321, mehta.salil, netdev, linux-kernel,
	linux-rdma, linuxarm
In-Reply-To: <20181113101307.6020-2-salil.mehta@huawei.com>

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

On Tue, Nov 13, 2018 at 10:13:03AM +0000, Salil Mehta wrote:
> From: Peng Li <lipeng321@huawei.com>
>
> HNS3 hardware Revision B(=0x21) supports Hardware GRO feature. This
> patch enables this feature in the HNS3 PF/VF driver.
>
> Signed-off-by: Peng Li <lipeng321@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>  drivers/net/ethernet/hisilicon/hns3/hnae3.h   |  4 ++
>  .../net/ethernet/hisilicon/hns3/hns3_enet.c   |  4 +-
>  .../hisilicon/hns3/hns3pf/hclge_cmd.h         |  7 ++++
>  .../hisilicon/hns3/hns3pf/hclge_main.c        | 36 ++++++++++++++++++
>  .../hisilicon/hns3/hns3vf/hclgevf_cmd.h       |  8 ++++
>  .../hisilicon/hns3/hns3vf/hclgevf_main.c      | 37 +++++++++++++++++++
>  6 files changed, 95 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
> index f69d39f17bdd..21d934b7a2a3 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
> +++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
> @@ -52,6 +52,7 @@
>  #define HNAE3_UNIC_CLIENT_INITED_B		0x4
>  #define HNAE3_ROCE_CLIENT_INITED_B		0x5
>  #define HNAE3_DEV_SUPPORT_FD_B			0x6
> +#define HNAE3_DEV_SUPPORT_GRO_B			0x7
>
>  #define HNAE3_DEV_SUPPORT_ROCE_DCB_BITS (BIT(HNAE3_DEV_SUPPORT_DCB_B) |\
>  		BIT(HNAE3_DEV_SUPPORT_ROCE_B))
> @@ -65,6 +66,9 @@
>  #define hnae3_dev_fd_supported(hdev) \
>  	hnae3_get_bit((hdev)->ae_dev->flag, HNAE3_DEV_SUPPORT_FD_B)
>
> +#define hnae3_dev_gro_supported(hdev) \
> +	hnae3_get_bit((hdev)->ae_dev->flag, HNAE3_DEV_SUPPORT_GRO_B)
> +
>  #define ring_ptr_move_fw(ring, p) \
>  	((ring)->p = ((ring)->p + 1) % (ring)->desc_num)
>  #define ring_ptr_move_bw(ring, p) \
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> index 8d07ec668d5d..a510ddfd45a5 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> @@ -1714,8 +1714,10 @@ static void hns3_disable_sriov(struct pci_dev *pdev)
>  static void hns3_get_dev_capability(struct pci_dev *pdev,
>  				    struct hnae3_ae_dev *ae_dev)
>  {
> -	if (pdev->revision >= 0x21)
> +	if (pdev->revision >= 0x21) {
>  		hnae3_set_bit(ae_dev->flag, HNAE3_DEV_SUPPORT_FD_B, 1);
> +		hnae3_set_bit(ae_dev->flag, HNAE3_DEV_SUPPORT_GRO_B, 1);
> +	}
>  }
>
>  /* hns3_probe - Device initialization routine
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
> index 872cd4bdd70d..aef044d08b11 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
> @@ -152,6 +152,7 @@ enum hclge_opcode_type {
>
>  	/* TSO command */
>  	HCLGE_OPC_TSO_GENERIC_CONFIG	= 0x0C01,
> +	HCLGE_OPC_GRO_GENERIC_CONFIG    = 0x0C10,
>
>  	/* RSS commands */
>  	HCLGE_OPC_RSS_GENERIC_CONFIG	= 0x0D01,
> @@ -758,6 +759,12 @@ struct hclge_cfg_tso_status_cmd {
>  	u8 rsv[20];
>  };
>
> +#define HCLGE_GRO_EN_B		0
> +struct hclge_cfg_gro_status_cmd {
> +	__le16 gro_en;
> +	u8 rsv[22];
> +};
> +
>  #define HCLGE_TSO_MSS_MIN	256
>  #define HCLGE_TSO_MSS_MAX	9668
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> index 43bfc730a62d..d02712446d50 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> @@ -921,6 +921,28 @@ static int hclge_config_tso(struct hclge_dev *hdev, int tso_mss_min,
>  	return hclge_cmd_send(&hdev->hw, &desc, 1);
>  }
>
> +static int hclge_config_gro(struct hclge_dev *hdev, bool en)
> +{
> +	struct hclge_cfg_gro_status_cmd *req;
> +	struct hclge_desc desc;
> +	int ret;
> +
> +	if (!hnae3_dev_gro_supported(hdev))
> +		return 0;
> +
> +	hclge_cmd_setup_basic_desc(&desc, HCLGE_OPC_GRO_GENERIC_CONFIG, false);
> +	req = (struct hclge_cfg_gro_status_cmd *)desc.data;
> +
> +	req->gro_en = cpu_to_le16(en ? 1 : 0);
> +
> +	ret = hclge_cmd_send(&hdev->hw, &desc, 1);
> +	if (ret)
> +		dev_err(&hdev->pdev->dev,
> +			"GRO hardware config cmd failed, ret = %d\n", ret);
> +
> +	return ret;
> +}
> +
>  static int hclge_alloc_tqps(struct hclge_dev *hdev)
>  {
>  	struct hclge_tqp *tqp;
> @@ -7090,6 +7112,13 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
>  		goto err_mdiobus_unreg;
>  	}
>
> +	ret = hclge_config_gro(hdev, true);
> +	if (ret) {
> +		dev_err(&pdev->dev,
> +			"Failed to enable GRO in hardware, ret =%d\n", ret);

You already printed an error in the hclge_config_gro().

> +		goto err_mdiobus_unreg;
> +	}
> +
>  	ret = hclge_init_vlan_config(hdev);
>  	if (ret) {
>  		dev_err(&pdev->dev, "VLAN init fail, ret =%d\n", ret);
> @@ -7221,6 +7250,13 @@ static int hclge_reset_ae_dev(struct hnae3_ae_dev *ae_dev)
>  		return ret;
>  	}
>
> +	ret = hclge_config_gro(hdev, true);
> +	if (ret) {
> +		dev_err(&pdev->dev,
> +			"Failed to enable GRO in hardware, ret =%d\n", ret);

Ditto

> +		return ret;
> +	}
> +
>  	ret = hclge_init_vlan_config(hdev);
>  	if (ret) {
>  		dev_err(&pdev->dev, "VLAN init fail, ret =%d\n", ret);
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.h b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.h
> index 090541de0c7d..47030b42341f 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.h
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.h
> @@ -87,6 +87,8 @@ enum hclgevf_opcode_type {
>  	HCLGEVF_OPC_QUERY_TX_STATUS	= 0x0B03,
>  	HCLGEVF_OPC_QUERY_RX_STATUS	= 0x0B13,
>  	HCLGEVF_OPC_CFG_COM_TQP_QUEUE	= 0x0B20,
> +	/* GRO command */
> +	HCLGEVF_OPC_GRO_GENERIC_CONFIG  = 0x0C10,
>  	/* RSS cmd */
>  	HCLGEVF_OPC_RSS_GENERIC_CONFIG	= 0x0D01,
>  	HCLGEVF_OPC_RSS_INPUT_TUPLE     = 0x0D02,
> @@ -149,6 +151,12 @@ struct hclgevf_query_res_cmd {
>  	__le16 rsv[7];
>  };
>
> +#define HCLGEVF_GRO_EN_B               0
> +struct hclgevf_cfg_gro_status_cmd {
> +	__le16 gro_en;
> +	u8 rsv[22];
> +};
> +
>  #define HCLGEVF_RSS_DEFAULT_OUTPORT_B	4
>  #define HCLGEVF_RSS_HASH_KEY_OFFSET_B	4
>  #define HCLGEVF_RSS_HASH_KEY_NUM	16
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
> index 6b4d1477055f..3f256414fbe4 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
> @@ -1655,6 +1655,29 @@ static int hclgevf_init_roce_base_info(struct hclgevf_dev *hdev)
>  	return 0;
>  }
>
> +static int hclgevf_config_gro(struct hclgevf_dev *hdev, bool en)
> +{
> +	struct hclgevf_cfg_gro_status_cmd *req;
> +	struct hclgevf_desc desc;
> +	int ret;
> +
> +	if (!hnae3_dev_gro_supported(hdev))
> +		return 0;
> +
> +	hclgevf_cmd_setup_basic_desc(&desc, HCLGEVF_OPC_GRO_GENERIC_CONFIG,
> +				     false);
> +	req = (struct hclgevf_cfg_gro_status_cmd *)desc.data;
> +
> +	req->gro_en = cpu_to_le16(en ? 1 : 0);
> +
> +	ret = hclgevf_cmd_send(&hdev->hw, &desc, 1);
> +	if (ret)
> +		dev_err(&hdev->pdev->dev,
> +			"VF GRO hardware config cmd failed, ret = %d.\n", ret);
> +
> +	return ret;
> +}
> +
>  static int hclgevf_rss_init_hw(struct hclgevf_dev *hdev)
>  {
>  	struct hclgevf_rss_cfg *rss_cfg = &hdev->rss_cfg;
> @@ -2122,6 +2145,13 @@ static int hclgevf_reset_hdev(struct hclgevf_dev *hdev)
>  		return ret;
>  	}
>
> +	ret = hclgevf_config_gro(hdev, true);
> +	if (ret) {
> +		dev_err(&pdev->dev,
> +			"failed to enable VF GRO in hardware, ret =%d\n", ret);
> +		return ret;

Ditto

> +	}
> +
>  	ret = hclgevf_init_vlan_config(hdev);
>  	if (ret) {
>  		dev_err(&hdev->pdev->dev,
> @@ -2199,6 +2229,13 @@ static int hclgevf_init_hdev(struct hclgevf_dev *hdev)
>  		goto err_config;
>  	}
>
> +	ret = hclgevf_config_gro(hdev, true);
> +	if (ret) {
> +		dev_err(&pdev->dev,
> +			"Failed to enable VF GRO in hardware, ret =%d\n", ret);

Ditto

> +		goto err_config;
> +	}
> +
>  	/* Initialize RSS for this VF */
>  	ret = hclgevf_rss_init_hw(hdev);
>  	if (ret) {
> --
> 2.17.1
>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* [PATCH][net-next] net: slightly optimize eth_type_trans
From: Li RongQing @ 2018-11-13  1:34 UTC (permalink / raw)
  To: netdev

netperf udp stream shows that eth_type_trans takes certain cpu,
so adjust the mac address check order, and firstly check if it
is device address, and only check if it is multicast address
only if not the device address.

After this change:
To unicast, and skb dst mac is device mac, this is most of time
reduce a comparision
To unicast, and skb dst mac is not device mac, nothing change
To multicast, increase a comparision

Before:
1.03%  [kernel]          [k] eth_type_trans

After:
0.78%  [kernel]          [k] eth_type_trans

Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 net/ethernet/eth.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index fd8faa0dfa61..1c88f5c5d5b1 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -165,15 +165,17 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev)
 	eth = (struct ethhdr *)skb->data;
 	skb_pull_inline(skb, ETH_HLEN);
 
-	if (unlikely(is_multicast_ether_addr_64bits(eth->h_dest))) {
-		if (ether_addr_equal_64bits(eth->h_dest, dev->broadcast))
-			skb->pkt_type = PACKET_BROADCAST;
-		else
-			skb->pkt_type = PACKET_MULTICAST;
+	if (unlikely(!ether_addr_equal_64bits(eth->h_dest,
+						   dev->dev_addr))) {
+		if (unlikely(is_multicast_ether_addr_64bits(eth->h_dest))) {
+			if (ether_addr_equal_64bits(eth->h_dest, dev->broadcast))
+				skb->pkt_type = PACKET_BROADCAST;
+			else
+				skb->pkt_type = PACKET_MULTICAST;
+		} else {
+			skb->pkt_type = PACKET_OTHERHOST;
+		}
 	}
-	else if (unlikely(!ether_addr_equal_64bits(eth->h_dest,
-						   dev->dev_addr)))
-		skb->pkt_type = PACKET_OTHERHOST;
 
 	/*
 	 * Some variants of DSA tagging don't have an ethertype field
-- 
2.16.2

^ permalink raw reply related

* Re: [PATCH v2] net: Add trace events for all receive exit points
From: Steven Rostedt @ 2018-11-13  1:47 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Geneviève Bastien, David S. Miller, netdev, Ingo Molnar
In-Reply-To: <1354786248.4048.1542055613213.JavaMail.zimbra@efficios.com>

On Mon, 12 Nov 2018 15:46:53 -0500 (EST)
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:

> I also notice that in two cases, a "gro_result_t" is implicitly cast
> to "int". I usually frown upon this kind of stuff, because it's asking
> for trouble if gro_result_t typedef to something else than "int" in the
> future.
> 
> I would recommend going for two templates, one which takes a "int"
> ret parameter, and the other a "gro_result_t" ret parameter.
> 
> Or am I being too cautious ?

That's more of a question for the netdev maintainers. If they think
casting gro_result_t to int is fine, then I'm fine. If it breaks in the
future, they need to deal with it, I don't ;-)

The downside of two templates, is that the templates are the bloated
part of the trace event (DEFINE_EVENT()s are light weight). They can
add a couple of K to the memory foot print.

-- Steve

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox