Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH ghak90 V6 02/10] audit: add container id
From: Paul Moore @ 2019-07-18 21:52 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Serge E. Hallyn, Tycho Andersen, containers, linux-api,
	Linux-Audit Mailing List, linux-fsdevel, LKML, netdev,
	netfilter-devel, sgrubb, omosnace, dhowells, simo, Eric Paris,
	ebiederm, nhorman
In-Reply-To: <20190718005145.eshekqfr3navqqiy@madcap2.tricolour.ca>

On Wed, Jul 17, 2019 at 8:52 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2019-07-16 19:30, Paul Moore wrote:

...

> > We can trust capable(CAP_AUDIT_CONTROL) for enforcing audit container
> > ID policy, we can not trust ns_capable(CAP_AUDIT_CONTROL).
>
> Ok.  So does a process in a non-init user namespace have two (or more)
> sets of capabilities stored in creds, one in the init_user_ns, and one
> in current_user_ns?  Or does it get stripped of all its capabilities in
> init_user_ns once it has its own set in current_user_ns?  If the former,
> then we can use capable().  If the latter, we need another mechanism, as
> you have suggested might be needed.

Unfortunately I think the problem is that ultimately we need to allow
any container orchestrator that has been given privileges to manage
the audit container ID to also grant that privilege to any of the
child process/containers it manages.  I don't believe we can do that
with capabilities based on the code I've looked at, and the
discussions I've had, but if you find a way I would leave to hear it.

> If some random unprivileged user wants to fire up a container
> orchestrator/engine in his own user namespace, then audit needs to be
> namespaced.  Can we safely discard this scenario for now?

I think the only time we want to allow a container orchestrator to
manage the audit container ID is if it has been granted that privilege
by someone who has that privilege already.  In the zero-container, or
single-level of containers, case this is relatively easy, and we can
accomplish it using CAP_AUDIT_CONTROL as the privilege.  If we start
nesting container orchestrators it becomes more complicated as we need
to be able to support granting and inheriting this privilege in a
manner; this is why I suggested a new mechanism *may* be necessary.

--
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: [PATCH] be2net: fix adapter->big_page_size miscaculation
From: Bill Wendling @ 2019-07-18 21:28 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Qian Cai, James Y Knight, David Miller, sathya.perla,
	ajit.khaparde, sriharsha.basavapatna, somnath.kotur,
	Arnd Bergmann, David Howells, H. Peter Anvin, netdev, linux-arch,
	LKML, Nathan Chancellor
In-Reply-To: <CAKwvOd=B=Lj-hTtbe88bo89wLxJrDAsm3fJisSMD=hKkRHf6zw@mail.gmail.com>

Possibly. I'd need to ask him. :-)

On Thu, Jul 18, 2019 at 2:22 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Thu, Jul 18, 2019 at 2:18 PM Bill Wendling <morbo@google.com> wrote:
> >
> > Top-of-tree clang says that it's const:
> >
> > $ gcc a.c -O2 && ./a.out
> > a is a const.
> >
> > $ clang a.c -O2 && ./a.out
> > a is a const.
>
> Right, so I know you (Bill) did a lot of work to refactor
> __builtin_constant_p handling in Clang and LLVM in the
> pre-llvm-9-release timeframe.  I suspect Qian might not be using
> clang-9 built from source (as clang-8 is the current release) and thus
> observing differences.
>
> >
> > On Thu, Jul 18, 2019 at 2:10 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >>
> >> On Thu, Jul 18, 2019 at 2:01 PM Qian Cai <cai@lca.pw> wrote:
> >> >
> >> >
> >> >
> >> > > On Jul 12, 2019, at 8:50 PM, David Miller <davem@davemloft.net> wrote:
> >> > >
> >> > > From: Qian Cai <cai@lca.pw>
> >> > > Date: Fri, 12 Jul 2019 20:27:09 -0400
> >> > >
> >> > >> Actually, GCC would consider it a const with -O2 optimized level because it found that it was never modified and it does not understand it is a module parameter. Considering the following code.
> >> > >>
> >> > >> # cat const.c
> >> > >> #include <stdio.h>
> >> > >>
> >> > >> static int a = 1;
> >> > >>
> >> > >> int main(void)
> >> > >> {
> >> > >>      if (__builtin_constant_p(a))
> >> > >>              printf("a is a const.\n");
> >> > >>
> >> > >>      return 0;
> >> > >> }
> >> > >>
> >> > >> # gcc -O2 const.c -o const
> >> > >
> >> > > That's not a complete test case, and with a proper test case that
> >> > > shows the externalization of the address of &a done by the module
> >> > > parameter macros, gcc should not make this optimization or we should
> >> > > define the module parameter macros in a way that makes this properly
> >> > > clear to the compiler.
> >> > >
> >> > > It makes no sense to hack around this locally in drivers and other
> >> > > modules.
> >> >
> >> > If you see the warning in the original patch,
> >> >
> >> > https://lore.kernel.org/netdev/1562959401-19815-1-git-send-email-cai@lca.pw/
> >> >
> >> > GCC definitely optimize rx_frag_size  to be a constant while I just confirmed clang
> >> > -O2 does not. The problem is that I have no clue about how to let GCC not to
> >> > optimize a module parameter.
> >> >
> >> > Though, I have added a few people who might know more of compilers than myself.
> >>
> >> + Bill and James, who probably knows more than they'd like to about
> >> __builtin_constant_p and more than other LLVM folks at this point.
> >>
> >> --
> >> Thanks,
> >> ~Nick Desaulniers
>
>
>
> --
> Thanks,
> ~Nick Desaulniers

^ permalink raw reply

* Re: [PATCH] be2net: fix adapter->big_page_size miscaculation
From: Nick Desaulniers @ 2019-07-18 21:22 UTC (permalink / raw)
  To: Bill Wendling
  Cc: Qian Cai, James Y Knight, David Miller, sathya.perla,
	ajit.khaparde, sriharsha.basavapatna, somnath.kotur,
	Arnd Bergmann, David Howells, H. Peter Anvin, netdev, linux-arch,
	LKML, Nathan Chancellor
In-Reply-To: <CAGG=3QWkgm+YhC=TWEWwt585Lbm8ZPG-uFre-kBRv+roPzZFbA@mail.gmail.com>

On Thu, Jul 18, 2019 at 2:18 PM Bill Wendling <morbo@google.com> wrote:
>
> Top-of-tree clang says that it's const:
>
> $ gcc a.c -O2 && ./a.out
> a is a const.
>
> $ clang a.c -O2 && ./a.out
> a is a const.

Right, so I know you (Bill) did a lot of work to refactor
__builtin_constant_p handling in Clang and LLVM in the
pre-llvm-9-release timeframe.  I suspect Qian might not be using
clang-9 built from source (as clang-8 is the current release) and thus
observing differences.

>
> On Thu, Jul 18, 2019 at 2:10 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
>>
>> On Thu, Jul 18, 2019 at 2:01 PM Qian Cai <cai@lca.pw> wrote:
>> >
>> >
>> >
>> > > On Jul 12, 2019, at 8:50 PM, David Miller <davem@davemloft.net> wrote:
>> > >
>> > > From: Qian Cai <cai@lca.pw>
>> > > Date: Fri, 12 Jul 2019 20:27:09 -0400
>> > >
>> > >> Actually, GCC would consider it a const with -O2 optimized level because it found that it was never modified and it does not understand it is a module parameter. Considering the following code.
>> > >>
>> > >> # cat const.c
>> > >> #include <stdio.h>
>> > >>
>> > >> static int a = 1;
>> > >>
>> > >> int main(void)
>> > >> {
>> > >>      if (__builtin_constant_p(a))
>> > >>              printf("a is a const.\n");
>> > >>
>> > >>      return 0;
>> > >> }
>> > >>
>> > >> # gcc -O2 const.c -o const
>> > >
>> > > That's not a complete test case, and with a proper test case that
>> > > shows the externalization of the address of &a done by the module
>> > > parameter macros, gcc should not make this optimization or we should
>> > > define the module parameter macros in a way that makes this properly
>> > > clear to the compiler.
>> > >
>> > > It makes no sense to hack around this locally in drivers and other
>> > > modules.
>> >
>> > If you see the warning in the original patch,
>> >
>> > https://lore.kernel.org/netdev/1562959401-19815-1-git-send-email-cai@lca.pw/
>> >
>> > GCC definitely optimize rx_frag_size  to be a constant while I just confirmed clang
>> > -O2 does not. The problem is that I have no clue about how to let GCC not to
>> > optimize a module parameter.
>> >
>> > Though, I have added a few people who might know more of compilers than myself.
>>
>> + Bill and James, who probably knows more than they'd like to about
>> __builtin_constant_p and more than other LLVM folks at this point.
>>
>> --
>> Thanks,
>> ~Nick Desaulniers



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply

* Re: [PATCH] be2net: fix adapter->big_page_size miscaculation
From: Bill Wendling @ 2019-07-18 21:21 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Qian Cai, James Y Knight, David Miller, sathya.perla,
	ajit.khaparde, sriharsha.basavapatna, somnath.kotur,
	Arnd Bergmann, David Howells, H. Peter Anvin, netdev, linux-arch,
	LKML, Nathan Chancellor
In-Reply-To: <CAKwvOdkCfqfpJYYX+iu2nLCUUkeDorDdVP3e7koB9NYsRwgCNw@mail.gmail.com>

[My previous response was marked as spam...]

Top-of-tree clang says that it's const:

$ gcc a.c -O2 && ./a.out
a is a const.

$ clang a.c -O2 && ./a.out
a is a const.


On Thu, Jul 18, 2019 at 2:10 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Thu, Jul 18, 2019 at 2:01 PM Qian Cai <cai@lca.pw> wrote:
> >
> >
> >
> > > On Jul 12, 2019, at 8:50 PM, David Miller <davem@davemloft.net> wrote:
> > >
> > > From: Qian Cai <cai@lca.pw>
> > > Date: Fri, 12 Jul 2019 20:27:09 -0400
> > >
> > >> Actually, GCC would consider it a const with -O2 optimized level because it found that it was never modified and it does not understand it is a module parameter. Considering the following code.
> > >>
> > >> # cat const.c
> > >> #include <stdio.h>
> > >>
> > >> static int a = 1;
> > >>
> > >> int main(void)
> > >> {
> > >>      if (__builtin_constant_p(a))
> > >>              printf("a is a const.\n");
> > >>
> > >>      return 0;
> > >> }
> > >>
> > >> # gcc -O2 const.c -o const
> > >
> > > That's not a complete test case, and with a proper test case that
> > > shows the externalization of the address of &a done by the module
> > > parameter macros, gcc should not make this optimization or we should
> > > define the module parameter macros in a way that makes this properly
> > > clear to the compiler.
> > >
> > > It makes no sense to hack around this locally in drivers and other
> > > modules.
> >
> > If you see the warning in the original patch,
> >
> > https://lore.kernel.org/netdev/1562959401-19815-1-git-send-email-cai@lca.pw/
> >
> > GCC definitely optimize rx_frag_size  to be a constant while I just confirmed clang
> > -O2 does not. The problem is that I have no clue about how to let GCC not to
> > optimize a module parameter.
> >
> > Though, I have added a few people who might know more of compilers than myself.
>
> + Bill and James, who probably knows more than they'd like to about
> __builtin_constant_p and more than other LLVM folks at this point.
>
> --
> Thanks,
> ~Nick Desaulniers

^ permalink raw reply

* Re: [PATCH bpf] libbpf: fix missing __WORDSIZE definition
From: Andrii Nakryiko @ 2019-07-18 21:16 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Andrii Nakryiko, bpf, Networking, Daniel Borkmann,
	Alexei Starovoitov, Kernel Team, Jiri Olsa, Namhyung Kim
In-Reply-To: <20190718191452.GM3624@kernel.org>

On Thu, Jul 18, 2019 at 12:14 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Thu, Jul 18, 2019 at 03:56:19PM -0300, Arnaldo Carvalho de Melo escreveu:
> > I'll stop and replace my patch with yours to see if it survives all the
> > test builds...
>
> So, Alpine:3.4, the first image for this distro I did when I started
> these builds, survives the 6 builds with gcc and clang with your patch:
>
>
> [perfbuilder@quaco linux-perf-tools-build]$ export PERF_TARBALL=http://192.168.124.1/perf/perf-5.2.0.tar.xz
> [perfbuilder@quaco linux-perf-tools-build]$ dm
>    1  alpine:3.4                    : Ok   gcc (Alpine 5.3.0) 5.3.0, clang version 3.8.0 (tags/RELEASE_380/final)
>
>
> [perfbuilder@quaco linux-perf-tools-build]$ grep "+ make" dm.log/alpine\:3.4
> + make ARCH= CROSS_COMPILE= EXTRA_CFLAGS= -C /git/linux/tools/perf O=/tmp/build/perf
> + make ARCH= CROSS_COMPILE= EXTRA_CFLAGS= NO_LIBELF=1 -C /git/linux/tools/perf O=/tmp/build/perf
> + make ARCH= CROSS_COMPILE= EXTRA_CFLAGS= -C /git/linux/tools/perf O=/tmp/build/perf CC=clang
> + make ARCH= CROSS_COMPILE= EXTRA_CFLAGS= NO_LIBELF=1 -C /git/linux/tools/perf O=/tmp/build/perf CC=clang
> + make ARCH= CROSS_COMPILE= EXTRA_CFLAGS= LIBCLANGLLVM=1 -C /git/linux/tools/perf O=/tmp/build/perf CC=clang
> + make ARCH= CROSS_COMPILE= EXTRA_CFLAGS= LIBCLANGLLVM=1 -C /git/linux/tools/perf O=/tmp/build/perf
> [perfbuilder@quaco linux-perf-tools-build]$
>
> Probably all the rest will go well, will let you know.
>
> Daniel, do you mind if I carry this one in my perf/core branch? Its
> small and shouldn't clash with other patches, I think. It should go
> upstream soon:
>
> Andrii, there are these others:

I took a look at them, but I think it would be better, if you could
post them as proper patches to
bpf@vger.kernel.org/netdev@vger.kernel.org, so that others can check
and comment, if necessary.

One nit for all three of them: we typically prefix subject with just
"libbpf: " instead of "tools lib libbpf".

>
> 8dfb6ed300bf tools lib bpf: Avoid designated initializers for unnamed union members

+ attr.sample_period = attr.wakeup_events = 1;

let's instead

+ attr.sample_period = 1;
+ attr.wakeup_events = 1;

I don't like multi-assignments.

Also, if we are doing explicit assignment, let's do it for all the
fields, not split initialization like that.


> 80f7f8f21441 tools lib bpf: Avoid using 'link' as it shadows a global definition in some systems

For this one I'm confused. What compiler/system you are getting it on?

I tried to reproduce it with this example (trying both global
variable, as well as function):

#include <stdio.h>

//int link = 1;
void link() {}

int f(int link) {
        return link;
}
int main() {
        printf("%d\n", f(123));
        return 0;
}

I haven't gotten any errors nor warnings. I'm certainly liking
existing naming better, but my main concern is that we'll probably add
more code like this, and we'll forget about this problem and will
re-introduce.

So I'd rather figure out why it's happening and some rare system and
see if we can mitigate that without all the renames.


> d93fe741e291 tools lib bpf: Fix endianness macro usage for some compilers

This one looks good to me, thanks!

Acked-by: Andrii Nakryiko <andriin@fb.com>


>
> If you could take a look at them at my tmp.perf/core branch at:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=tmp.perf/core
>
> I'm force pushing it now to replace my __WORDSIZE patch with yours, the
> SHAs should be the 3 above and the one below.
>
> And to build cleanly I had to cherry pick this one:
>
> 3091dafc1884 (HEAD -> perf/core) libbpf: fix ptr to u64 conversion warning on 32-bit platforms
>
> Alpine:3.5 just finished:
>
>    2 alpine:3.5                    : Ok   gcc (Alpine 6.2.1) 6.2.1 20160822, clang version 3.8.1 (tags/RELEASE_381/final)
>
> - Arnaldo

^ permalink raw reply

* Re: [PATCH] be2net: fix adapter->big_page_size miscaculation
From: Nick Desaulniers @ 2019-07-18 21:10 UTC (permalink / raw)
  To: Qian Cai, Bill Wendling, James Y Knight
  Cc: David Miller, sathya.perla, ajit.khaparde, sriharsha.basavapatna,
	somnath.kotur, Arnd Bergmann, David Howells, H. Peter Anvin,
	netdev, linux-arch, LKML, Nathan Chancellor
In-Reply-To: <D7E57421-A6F4-4453-878A-8F173A856296@lca.pw>

On Thu, Jul 18, 2019 at 2:01 PM Qian Cai <cai@lca.pw> wrote:
>
>
>
> > On Jul 12, 2019, at 8:50 PM, David Miller <davem@davemloft.net> wrote:
> >
> > From: Qian Cai <cai@lca.pw>
> > Date: Fri, 12 Jul 2019 20:27:09 -0400
> >
> >> Actually, GCC would consider it a const with -O2 optimized level because it found that it was never modified and it does not understand it is a module parameter. Considering the following code.
> >>
> >> # cat const.c
> >> #include <stdio.h>
> >>
> >> static int a = 1;
> >>
> >> int main(void)
> >> {
> >>      if (__builtin_constant_p(a))
> >>              printf("a is a const.\n");
> >>
> >>      return 0;
> >> }
> >>
> >> # gcc -O2 const.c -o const
> >
> > That's not a complete test case, and with a proper test case that
> > shows the externalization of the address of &a done by the module
> > parameter macros, gcc should not make this optimization or we should
> > define the module parameter macros in a way that makes this properly
> > clear to the compiler.
> >
> > It makes no sense to hack around this locally in drivers and other
> > modules.
>
> If you see the warning in the original patch,
>
> https://lore.kernel.org/netdev/1562959401-19815-1-git-send-email-cai@lca.pw/
>
> GCC definitely optimize rx_frag_size  to be a constant while I just confirmed clang
> -O2 does not. The problem is that I have no clue about how to let GCC not to
> optimize a module parameter.
>
> Though, I have added a few people who might know more of compilers than myself.

+ Bill and James, who probably knows more than they'd like to about
__builtin_constant_p and more than other LLVM folks at this point.

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply

* Re: [PATCH V36 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Kees Cook @ 2019-07-18 21:06 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: jmorris, linux-security-module, linux-kernel, linux-api,
	David Howells, Alexei Starovoitov, Matthew Garrett, netdev,
	Chun-Yi Lee, Daniel Borkmann
In-Reply-To: <20190718194415.108476-24-matthewgarrett@google.com>

On Thu, Jul 18, 2019 at 12:44:09PM -0700, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
> 
> bpf_read() and bpf_read_str() could potentially be abused to (eg) allow
> private keys in kernel memory to be leaked. Disable them if the kernel
> has been locked down in confidentiality mode.
> 
> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> cc: netdev@vger.kernel.org
> cc: Chun-Yi Lee <jlee@suse.com>
> cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> ---
>  include/linux/security.h     |  1 +
>  kernel/trace/bpf_trace.c     | 10 ++++++++++
>  security/lockdown/lockdown.c |  1 +
>  3 files changed, 12 insertions(+)
> 
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 987d8427f091..8dd1741a52cd 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -118,6 +118,7 @@ enum lockdown_reason {
>  	LOCKDOWN_INTEGRITY_MAX,
>  	LOCKDOWN_KCORE,
>  	LOCKDOWN_KPROBES,
> +	LOCKDOWN_BPF_READ,
>  	LOCKDOWN_CONFIDENTIALITY_MAX,
>  };
>  
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index ca1255d14576..492a8bfaae98 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -142,8 +142,13 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
>  {
>  	int ret;
>  
> +	ret = security_locked_down(LOCKDOWN_BPF_READ);
> +	if (ret < 0)
> +		goto out;
> +
>  	ret = probe_kernel_read(dst, unsafe_ptr, size);
>  	if (unlikely(ret < 0))
> +out:
>  		memset(dst, 0, size);
>  
>  	return ret;
> @@ -569,6 +574,10 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
>  {
>  	int ret;
>  
> +	ret = security_locked_down(LOCKDOWN_BPF_READ);
> +	if (ret < 0)
> +		goto out;
> +
>  	/*
>  	 * The strncpy_from_unsafe() call will likely not fill the entire
>  	 * buffer, but that's okay in this circumstance as we're probing
> @@ -580,6 +589,7 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
>  	 */
>  	ret = strncpy_from_unsafe(dst, unsafe_ptr, size);
>  	if (unlikely(ret < 0))
> +out:
>  		memset(dst, 0, size);
>  
>  	return ret;
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 6b123cbf3748..1b89d3e8e54d 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -33,6 +33,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
>  	[LOCKDOWN_INTEGRITY_MAX] = "integrity",
>  	[LOCKDOWN_KCORE] = "/proc/kcore access",
>  	[LOCKDOWN_KPROBES] = "use of kprobes",
> +	[LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
>  	[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
>  };
>  
> -- 
> 2.22.0.510.g264f2c817a-goog
> 

-- 
Kees Cook

^ permalink raw reply

* Re: pull-request: bpf 2019-07-18
From: David Miller @ 2019-07-18 21:05 UTC (permalink / raw)
  To: ast; +Cc: daniel, netdev, bpf, kernel-team
In-Reply-To: <20190718210025.21557-1-ast@kernel.org>

From: Alexei Starovoitov <ast@kernel.org>
Date: Thu, 18 Jul 2019 14:00:25 -0700

> The following pull-request contains BPF updates for your *net* tree.
 ...

Pulled, thanks Alexei.

^ permalink raw reply

* Re: [PATCH] be2net: fix adapter->big_page_size miscaculation
From: Qian Cai @ 2019-07-18 21:01 UTC (permalink / raw)
  To: David Miller
  Cc: sathya.perla, ajit.khaparde, sriharsha.basavapatna, somnath.kotur,
	arnd, dhowells, hpa, netdev, linux-arch, LKML, Nick Desaulniers,
	natechancellor
In-Reply-To: <20190712.175038.755685144649934618.davem@davemloft.net>



> On Jul 12, 2019, at 8:50 PM, David Miller <davem@davemloft.net> wrote:
> 
> From: Qian Cai <cai@lca.pw>
> Date: Fri, 12 Jul 2019 20:27:09 -0400
> 
>> Actually, GCC would consider it a const with -O2 optimized level because it found that it was never modified and it does not understand it is a module parameter. Considering the following code.
>> 
>> # cat const.c 
>> #include <stdio.h>
>> 
>> static int a = 1;
>> 
>> int main(void)
>> {
>> 	if (__builtin_constant_p(a))
>> 		printf("a is a const.\n");
>> 
>> 	return 0;
>> }
>> 
>> # gcc -O2 const.c -o const
> 
> That's not a complete test case, and with a proper test case that
> shows the externalization of the address of &a done by the module
> parameter macros, gcc should not make this optimization or we should
> define the module parameter macros in a way that makes this properly
> clear to the compiler.
> 
> It makes no sense to hack around this locally in drivers and other
> modules.

If you see the warning in the original patch,

https://lore.kernel.org/netdev/1562959401-19815-1-git-send-email-cai@lca.pw/

GCC definitely optimize rx_frag_size  to be a constant while I just confirmed clang
-O2 does not. The problem is that I have no clue about how to let GCC not to
optimize a module parameter.

Though, I have added a few people who might know more of compilers than myself.

^ permalink raw reply

* pull-request: bpf 2019-07-18
From: Alexei Starovoitov @ 2019-07-18 21:00 UTC (permalink / raw)
  To: davem; +Cc: daniel, netdev, bpf, kernel-team

Hi David,

The following pull-request contains BPF updates for your *net* tree.

The main changes are:

1) verifier precision propagation fix, from Andrii.

2) BTF size fix for typedefs, from Andrii.

3) a bunch of big endian fixes, from Ilya.

4) wide load from bpf_sock_addr fixes, from Stanislav.

5) a bunch of misc fixes from a number of developers.

Please consider pulling these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git

Thanks a lot!

----------------------------------------------------------------

The following changes since commit 114a5c3240155fdb01bf821c9d326d7bb05bd464:

  Merge tag 'mlx5-fixes-2019-07-11' of git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux (2019-07-11 15:06:37 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git 

for you to fetch changes up to 59fd3486c3dd5678bc2fcac75e14466775465c3e:

  selftests/bpf: fix test_xdp_noinline on s390 (2019-07-18 13:54:54 -0700)

----------------------------------------------------------------
Andrii Nakryiko (9):
      bpf: fix precision bit propagation for BPF_ST instructions
      libbpf: fix ptr to u64 conversion warning on 32-bit platforms
      bpf: fix BTF verifier size resolution logic
      selftests/bpf: add trickier size resolution tests
      selftests/bpf: use typedef'ed arrays as map values
      selftests/bpf: remove logic duplication in test_verifier
      libbpf: fix another GCC8 warning for strncpy
      selftests/bpf: fix test_verifier/test_maps make dependencies
      selftests/bpf: structure test_{progs, maps, verifier} test runners uniformly

Daniel Borkmann (2):
      Merge branch 'bpf-btf-size-verification-fix'
      Merge branch 'bpf-fix-wide-loads-sockaddr'

Daniel T. Lee (1):
      tools: bpftool: add raw_tracepoint_writable prog type to header

Gustavo A. R. Silva (1):
      bpf: verifier: avoid fall-through warnings

Ilya Leoshkevich (15):
      selftests/bpf: fix bpf_target_sparc check
      selftests/bpf: do not ignore clang failures
      selftests/bpf: compile progs with -D__TARGET_ARCH_$(SRCARCH)
      selftests/bpf: fix s930 -> s390 typo
      selftests/bpf: make PT_REGS_* work in userspace
      selftests/bpf: fix compiling loop{1, 2, 3}.c on s390
      selftests/bpf: fix attach_probe on s390
      selftests/bpf: make directory prerequisites order-only
      selftests/bpf: put test_stub.o into $(OUTPUT)
      samples/bpf: build with -D__TARGET_ARCH_$(SRCARCH)
      selftests/bpf: fix "alu with different scalars 1" on s390
      selftests/bpf: skip nmi test when perf hw events are disabled
      selftests/bpf: fix perf_buffer on s390
      selftests/bpf: fix "valid read map access into a read-only array 1" on s390
      selftests/bpf: fix test_xdp_noinline on s390

Ilya Maximets (2):
      xdp: fix possible cq entry leak
      xdp: fix potential deadlock on socket mutex

Stanislav Fomichev (5):
      bpf: rename bpf_ctx_wide_store_ok to bpf_ctx_wide_access_ok
      bpf: allow wide aligned loads for bpf_sock_addr user_ip6 and msg_src_ip6
      selftests/bpf: rename verifier/wide_store.c to verifier/wide_access.c
      selftests/bpf: add selftests for wide loads
      bpf: sync bpf.h to tools/

Vasily Gorbik (1):
      MAINTAINERS: update BPF JIT S390 maintainers

 MAINTAINERS                                        |  2 +-
 include/linux/filter.h                             |  2 +-
 include/uapi/linux/bpf.h                           |  4 +-
 kernel/bpf/btf.c                                   | 19 +++--
 kernel/bpf/verifier.c                              | 13 ++--
 net/core/filter.c                                  | 24 ++++--
 net/xdp/xdp_umem.c                                 | 16 ++--
 net/xdp/xsk.c                                      | 13 ++--
 samples/bpf/Makefile                               |  2 +-
 tools/bpf/bpftool/main.h                           |  1 +
 tools/include/uapi/linux/bpf.h                     |  4 +-
 tools/lib/bpf/libbpf.c                             |  4 +-
 tools/lib/bpf/xsk.c                                |  3 +-
 tools/testing/selftests/bpf/Makefile               | 64 ++++++++--------
 tools/testing/selftests/bpf/bpf_helpers.h          | 89 +++++++++++++++-------
 .../selftests/bpf/prog_tests/attach_probe.c        | 10 +--
 .../testing/selftests/bpf/prog_tests/perf_buffer.c |  8 +-
 .../testing/selftests/bpf/prog_tests/send_signal.c | 33 +++++++-
 tools/testing/selftests/bpf/progs/loop1.c          |  2 +-
 tools/testing/selftests/bpf/progs/loop2.c          |  2 +-
 tools/testing/selftests/bpf/progs/loop3.c          |  2 +-
 .../selftests/bpf/progs/test_get_stack_rawtp.c     |  3 +-
 .../selftests/bpf/progs/test_stacktrace_build_id.c |  3 +-
 .../selftests/bpf/progs/test_stacktrace_map.c      |  2 +-
 .../selftests/bpf/progs/test_xdp_noinline.c        | 17 +++--
 tools/testing/selftests/bpf/test_btf.c             | 88 +++++++++++++++++++++
 tools/testing/selftests/bpf/test_progs.h           |  8 ++
 tools/testing/selftests/bpf/test_verifier.c        | 35 ++++-----
 .../testing/selftests/bpf/verifier/array_access.c  |  2 +-
 .../selftests/bpf/verifier/value_ptr_arith.c       |  2 +-
 tools/testing/selftests/bpf/verifier/wide_access.c | 73 ++++++++++++++++++
 tools/testing/selftests/bpf/verifier/wide_store.c  | 36 ---------
 32 files changed, 391 insertions(+), 195 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/verifier/wide_access.c
 delete mode 100644 tools/testing/selftests/bpf/verifier/wide_store.c

^ permalink raw reply

* Re: [PATCH bpf] selftests/bpf: fix test_xdp_noinline on s390
From: Alexei Starovoitov @ 2019-07-18 20:56 UTC (permalink / raw)
  To: Ilya Leoshkevich; +Cc: bpf, Network Development, gor, Heiko Carstens
In-Reply-To: <20190717122620.58792-1-iii@linux.ibm.com>

On Wed, Jul 17, 2019 at 5:29 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> test_xdp_noinline fails on s390 due to a handful of endianness issues.
> Use ntohs for parsing eth_proto.
> Replace bswaps with ntohs/htons.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> Acked-by: Vasily Gorbik <gor@linux.ibm.com>

Applied. Thanks

^ permalink raw reply

* Re: [PATCH bpf] selftests/bpf: fix "valid read map access into a read-only array 1" on s390
From: Alexei Starovoitov @ 2019-07-18 20:50 UTC (permalink / raw)
  To: Ilya Leoshkevich; +Cc: bpf, Network Development, gor, Heiko Carstens
In-Reply-To: <20190718091335.73695-1-iii@linux.ibm.com>

On Thu, Jul 18, 2019 at 2:14 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> This test looks up a 32-bit map element and then loads it using a 64-bit
> load. This does not work on s390, which is a big-endian machine.
>
> Since the point of this test doesn't seem to be loading a smaller value
> using a larger load, simply use a 32-bit load.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>

Applied. Thanks

^ permalink raw reply

* Re: [PATCH bpf v2] bpf: fix narrower loads on s390
From: Alexei Starovoitov @ 2019-07-18 20:43 UTC (permalink / raw)
  To: Ilya Leoshkevich; +Cc: bpf, netdev, ys114321, gor, heiko.carstens, daniel
In-Reply-To: <20190718150103.84837-1-iii@linux.ibm.com>

On Thu, Jul 18, 2019 at 05:01:03PM +0200, Ilya Leoshkevich wrote:
> The very first check in test_pkt_md_access is failing on s390, which
> happens because loading a part of a struct __sk_buff field produces
> an incorrect result.
> 
> The preprocessed code of the check is:
> 
> {
> 	__u8 tmp = *((volatile __u8 *)&skb->len +
> 		((sizeof(skb->len) - sizeof(__u8)) / sizeof(__u8)));
> 	if (tmp != ((*(volatile __u32 *)&skb->len) & 0xFF)) return 2;
> };
> 
> clang generates the following code for it:
> 
>       0:	71 21 00 03 00 00 00 00	r2 = *(u8 *)(r1 + 3)
>       1:	61 31 00 00 00 00 00 00	r3 = *(u32 *)(r1 + 0)
>       2:	57 30 00 00 00 00 00 ff	r3 &= 255
>       3:	5d 23 00 1d 00 00 00 00	if r2 != r3 goto +29 <LBB0_10>
> 
> Finally, verifier transforms it to:
> 
>   0: (61) r2 = *(u32 *)(r1 +104)
>   1: (bc) w2 = w2
>   2: (74) w2 >>= 24
>   3: (bc) w2 = w2
>   4: (54) w2 &= 255
>   5: (bc) w2 = w2
> 
> The problem is that when verifier emits the code to replace a partial
> load of a struct __sk_buff field (*(u8 *)(r1 + 3)) with a full load of
> struct sk_buff field (*(u32 *)(r1 + 104)), an optional shift and a
> bitwise AND, it assumes that the machine is little endian and
> incorrectly decides to use a shift.
> 
> Adjust shift count calculation to account for endianness.
> 
> Fixes: 31fd85816dbe ("bpf: permits narrower load from bpf program context fields")
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  include/linux/filter.h | 13 +++++++++++++
>  kernel/bpf/verifier.c  |  4 ++--
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index ff65d22cf336..4fe88e43f0fe 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -24,6 +24,8 @@
>  
>  #include <net/sch_generic.h>
>  
> +#include <asm/byteorder.h>
> +

unnecessary empty line

>  #include <uapi/linux/filter.h>
>  #include <uapi/linux/bpf.h>
>  
> @@ -1216,4 +1218,15 @@ struct bpf_sockopt_kern {
>  	s32		retval;
>  };
>  
> +static inline u8 bpf_narrower_load_shift(u32 size_default, u32 size, u32 off)

please place this function right after bpf_ctx_narrow_access_ok()
and order arguments the same way as bpf_ctx_narrow_access_ok does.

> +{
> +	u8 load_off = off & (size_default - 1);
> +
> +#ifdef __LITTLE_ENDIAN
> +	return load_off * 8;
> +#else
> +	return (size_default - (load_off + size)) * 8;
> +#endif
> +}
> +
>  #endif /* __LINUX_FILTER_H__ */
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 5900cbb966b1..48edc9c9a879 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -8616,8 +8616,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
>  		}
>  
>  		if (is_narrower_load && size < target_size) {
> -			u8 shift = (off & (size_default - 1)) * 8;
> -
> +			u8 shift = bpf_narrower_load_shift(size_default, size,
> +							   off);
>  			if (ctx_field_size <= 4) {
>  				if (shift)
>  					insn_buf[cnt++] = BPF_ALU32_IMM(BPF_RSH,
> -- 
> 2.21.0
> 

^ permalink raw reply

* Re: [PATCH net,v4 0/4] flow_offload fixes
From: David Miller @ 2019-07-18 20:36 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev, jiri, jakub.kicinski, pshelar
In-Reply-To: <20190718.123939.886909513952061536.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Thu, 18 Jul 2019 12:39:39 -0700 (PDT)

> Series applied, thank you.

Actually, I had to revert, this breaks the build:

In file included from ./include/net/netfilter/nf_tables_offload.h:4,
                 from <command-line>:
./include/net/flow_offload.h: In function ‘flow_block_cb_add’:
./include/net/flow_offload.h:297:2: error: implicit declaration of function ‘list_add_tail’ [-Werror=implicit-function-declaration]
  list_add_tail(&block_cb->list, &offload->cb_list);
  ^~~~~~~~~~~~~
./include/net/flow_offload.h: In function ‘flow_block_cb_remove’:
./include/net/flow_offload.h:303:2: error: implicit declaration of function ‘list_move’ [-Werror=implicit-function-declaration]
  list_move(&block_cb->list, &offload->cb_list);
  ^~~~~~~~~
./include/net/flow_offload.h: In function ‘flow_block_init’:
./include/net/flow_offload.h:346:2: error: implicit declaration of function ‘INIT_LIST_HEAD’; did you mean ‘XATTR_LIST_MAX’? [-Werror=implicit-function-declaration]
  INIT_LIST_HEAD(&flow_block->cb_list);
  ^~~~~~~~~~~~~~
  XATTR_LIST_MAX
In file included from ./include/net/netfilter/nf_tables.h:5,
                 from ./include/net/netfilter/nf_tables_offload.h:5,
                 from <command-line>:
./include/linux/list.h: At top level:
./include/linux/list.h:26:20: warning: conflicting types for ‘INIT_LIST_HEAD’
 static inline void INIT_LIST_HEAD(struct list_head *list)
                    ^~~~~~~~~~~~~~
./include/linux/list.h:26:20: error: static declaration of ‘INIT_LIST_HEAD’ follows non-static declaration
In file included from ./include/net/netfilter/nf_tables_offload.h:4,
                 from <command-line>:
./include/net/flow_offload.h:346:2: note: previous implicit declaration of ‘INIT_LIST_HEAD’ was here
  INIT_LIST_HEAD(&flow_block->cb_list);
  ^~~~~~~~~~~~~~
In file included from ./include/net/netfilter/nf_tables.h:5,
                 from ./include/net/netfilter/nf_tables_offload.h:5,
                 from <command-line>:
./include/linux/list.h:91:20: warning: conflicting types for ‘list_add_tail’
 static inline void list_add_tail(struct list_head *new, struct list_head *head)
                    ^~~~~~~~~~~~~
./include/linux/list.h:91:20: error: static declaration of ‘list_add_tail’ follows non-static declaration
In file included from ./include/net/netfilter/nf_tables_offload.h:4,
                 from <command-line>:
./include/net/flow_offload.h:297:2: note: previous implicit declaration of ‘list_add_tail’ was here
  list_add_tail(&block_cb->list, &offload->cb_list);
  ^~~~~~~~~~~~~
In file included from ./include/net/netfilter/nf_tables.h:5,
                 from ./include/net/netfilter/nf_tables_offload.h:5,
                 from <command-line>:
./include/linux/list.h:199:20: warning: conflicting types for ‘list_move’
 static inline void list_move(struct list_head *list, struct list_head *head)
                    ^~~~~~~~~
./include/linux/list.h:199:20: error: static declaration of ‘list_move’ follows non-static declaration
In file included from ./include/net/netfilter/nf_tables_offload.h:4,
                 from <command-line>:
./include/net/flow_offload.h:303:2: note: previous implicit declaration of ‘list_move’ was here
  list_move(&block_cb->list, &offload->cb_list);
  ^~~~~~~~~
cc1: some warnings being treated as errors
make[1]: *** [scripts/Makefile.build:304: include/net/netfilter/nf_tables_offload.h.s] Error 1
make: *** [Makefile:1077: include] Error 2
make: *** Waiting for unfinished jobs....

^ permalink raw reply

* [PATCH] ath10k: Use ARRAY_SIZE
From: Vasyl Gomonovych @ 2019-07-18 20:30 UTC (permalink / raw)
  To: kvalo, davem, ath10k, linux-wireless
  Cc: Vasyl Gomonovych, netdev, linux-kernel

fix coccinelle warning, use ARRAY_SIZE

Signed-off-by: Vasyl Gomonovych <gomonovych@gmail.com>
---
 drivers/net/wireless/ath/ath10k/snoc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index b491361e6ed4..49fc04412e9b 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -976,8 +976,7 @@ static int ath10k_snoc_wlan_enable(struct ath10k *ar,
 				  sizeof(struct ath10k_svc_pipe_cfg);
 	cfg.ce_svc_cfg = (struct ath10k_svc_pipe_cfg *)
 		&target_service_to_ce_map_wlan;
-	cfg.num_shadow_reg_cfg = sizeof(target_shadow_reg_cfg_map) /
-					sizeof(struct ath10k_shadow_reg_cfg);
+	cfg.num_shadow_reg_cfg = ARRAY_SIZE(target_shadow_reg_cfg_map);
 	cfg.shadow_reg_cfg = (struct ath10k_shadow_reg_cfg *)
 		&target_shadow_reg_cfg_map;
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v1] dt-bindings: fec: explicitly mark deprecated properties
From: Sven Van Asbroeck @ 2019-07-18 20:14 UTC (permalink / raw)
  To: Fugang Duan, Rob Herring, Mark Rutland
  Cc: David S . Miller, netdev, devicetree, linux-kernel, Andrew Lunn,
	Fabio Estevam, Lucas Stach

fec's gpio phy reset properties have been deprecated.
Update the dt-bindings documentation to explicitly mark
them as such, and provide a short description of the
recommended alternative.

Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
 .../devicetree/bindings/net/fsl-fec.txt       | 30 +++++++++++--------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
index 2d41fb96ce0a..5b88fae0307d 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -7,18 +7,6 @@ Required properties:
 - phy-mode : See ethernet.txt file in the same directory
 
 Optional properties:
-- phy-reset-gpios : Should specify the gpio for phy reset
-- phy-reset-duration : Reset duration in milliseconds.  Should present
-  only if property "phy-reset-gpios" is available.  Missing the property
-  will have the duration be 1 millisecond.  Numbers greater than 1000 are
-  invalid and 1 millisecond will be used instead.
-- phy-reset-active-high : If present then the reset sequence using the GPIO
-  specified in the "phy-reset-gpios" property is reversed (H=reset state,
-  L=operation state).
-- phy-reset-post-delay : Post reset delay in milliseconds. If present then
-  a delay of phy-reset-post-delay milliseconds will be observed after the
-  phy-reset-gpios has been toggled. Can be omitted thus no delay is
-  observed. Delay is in range of 1ms to 1000ms. Other delays are invalid.
 - phy-supply : regulator that powers the Ethernet PHY.
 - phy-handle : phandle to the PHY device connected to this device.
 - fixed-link : Assume a fixed link. See fixed-link.txt in the same directory.
@@ -47,11 +35,27 @@ Optional properties:
   For imx6sx, "int0" handles all 3 queues and ENET_MII. "pps" is for the pulse
   per second interrupt associated with 1588 precision time protocol(PTP).
 
-
 Optional subnodes:
 - mdio : specifies the mdio bus in the FEC, used as a container for phy nodes
   according to phy.txt in the same directory
 
+Deprecated optional properties:
+	To avoid these, create a phy node according to phy.txt in the same
+	directory, and point the fec's "phy-handle" property to it. Then use
+	the phy's reset binding, again described by phy.txt.
+- phy-reset-gpios : Should specify the gpio for phy reset
+- phy-reset-duration : Reset duration in milliseconds.  Should present
+  only if property "phy-reset-gpios" is available.  Missing the property
+  will have the duration be 1 millisecond.  Numbers greater than 1000 are
+  invalid and 1 millisecond will be used instead.
+- phy-reset-active-high : If present then the reset sequence using the GPIO
+  specified in the "phy-reset-gpios" property is reversed (H=reset state,
+  L=operation state).
+- phy-reset-post-delay : Post reset delay in milliseconds. If present then
+  a delay of phy-reset-post-delay milliseconds will be observed after the
+  phy-reset-gpios has been toggled. Can be omitted thus no delay is
+  observed. Delay is in range of 1ms to 1000ms. Other delays are invalid.
+
 Example:
 
 ethernet@83fec000 {
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH] net: fec: generate warning when using deprecated phy reset
From: Andrew Lunn @ 2019-07-18 19:59 UTC (permalink / raw)
  To: Sven Van Asbroeck
  Cc: Fabio Estevam, Lucas Stach, Fugang Duan, David S . Miller, netdev,
	linux-kernel
In-Reply-To: <CAGngYiWESbg6uq4pdtb5--YSzatwAwXiGnRjiAfAQj8nRYPMqw@mail.gmail.com>

> What I keep forgetting in my little arm-imx6 world, is that devicetrees
> aren't in-kernel apis, but that they have out-of-kernel
> dependencies. It makes more sense to to see them as userspace
> apis, albeit directed at firmware/bootloaders, right?

It is an ongoing debate, but generally they should be considered ABI
and follow the ABI rules about not breaking backwards compatibility.

However, there is also an argument that something like a NAS box
running Debian is going to use the DT blob which came with the kernel,
so deprecated DT properties and the code to support them could be
removed after a period of time.

	Andrew

^ permalink raw reply

* [PATCH iproute2 net-next v5 5/5] tc: taprio: Update documentation
From: Vedang Patel @ 2019-07-18 19:55 UTC (permalink / raw)
  To: netdev
  Cc: jhs, xiyou.wangcong, jiri, stephen, vinicius.gomes,
	leandro.maciel.dorileo, jakub.kicinski, m-karicheri2, dsahern,
	Vedang Patel
In-Reply-To: <1563479743-8371-1-git-send-email-vedang.patel@intel.com>

Add documentation for the latest options, flags and txtime-delay, to the
taprio manpage.

This also adds an example to run tc in txtime offload mode.

Signed-off-by: Vedang Patel <vedang.patel@intel.com>
---
 man/man8/tc-taprio.8 | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/man/man8/tc-taprio.8 b/man/man8/tc-taprio.8
index 850be9b03649..e1d19ba19089 100644
--- a/man/man8/tc-taprio.8
+++ b/man/man8/tc-taprio.8
@@ -112,6 +112,26 @@ means that traffic class 0 is "active" for that schedule entry.
 long that state defined by <command> and <gate mask> should be held
 before moving to the next entry.
 
+.TP
+flags
+.br
+Specifies different modes for taprio. Currently, only txtime-assist is
+supported which can be enabled by setting it to 0x1. In this mode, taprio will
+set the transmit timestamp depending on the interval in which the packet needs
+to be transmitted. It will then utililize the
+.BR etf(8)
+qdisc to sort and transmit the packets at the right time. The second example
+can be used as a reference to configure this mode.
+
+.TP
+txtime-delay
+.br
+This parameter is specific to the txtime offload mode. It specifies the maximum
+time a packet might take to reach the network card from the taprio qdisc. The
+value should always be greater than the delta specified in the
+.BR etf(8)
+qdisc.
+
 .SH EXAMPLES
 
 The following example shows how an traffic schedule with three traffic
@@ -137,6 +157,26 @@ reference CLOCK_TAI. The schedule is composed of three entries each of
               clockid CLOCK_TAI
 .EE
 
+Following is an example to enable the txtime offload mode in taprio. See
+.BR etf(8)
+for more information about configuring the ETF qdisc.
+
+.EX
+# tc qdisc replace dev eth0 parent root handle 100 taprio \\
+              num_tc 3 \\
+              map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \\
+              queues 1@0 1@0 1@0 \\
+              base-time 1528743495910289987 \\
+              sched-entry S 01 300000 \\
+              sched-entry S 02 300000 \\
+              sched-entry S 04 400000 \\
+              flags 0x1 \\
+              txtime-delay 200000 \\
+              clockid CLOCK_TAI
+
+# tc qdisc replace dev $IFACE parent 100:1 etf skip_skb_check \\
+              offload delta 200000 clockid CLOCK_TAI
+.EE
 
 .SH AUTHORS
 Vinicius Costa Gomes <vinicius.gomes@intel.com>
-- 
2.7.3


^ permalink raw reply related

* [PATCH iproute2 net-next v5 3/5] taprio: add support for setting txtime_delay.
From: Vedang Patel @ 2019-07-18 19:55 UTC (permalink / raw)
  To: netdev
  Cc: jhs, xiyou.wangcong, jiri, stephen, vinicius.gomes,
	leandro.maciel.dorileo, jakub.kicinski, m-karicheri2, dsahern,
	Vedang Patel
In-Reply-To: <1563479743-8371-1-git-send-email-vedang.patel@intel.com>

This adds support for setting the txtime_delay parameter which is useful
for the txtime offload mode of taprio.

Signed-off-by: Vedang Patel <vedang.patel@intel.com>
---
 tc/q_taprio.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/tc/q_taprio.c b/tc/q_taprio.c
index 1db2aba6efe7..b9954436b0f9 100644
--- a/tc/q_taprio.c
+++ b/tc/q_taprio.c
@@ -52,7 +52,7 @@ static void explain(void)
 		"		[num_tc NUMBER] [map P0 P1 ...] "
 		"		[queues COUNT@OFFSET COUNT@OFFSET COUNT@OFFSET ...] "
 		"		[ [sched-entry index cmd gate-mask interval] ... ] "
-		"		[base-time time] "
+		"		[base-time time] [txtime-delay delay]"
 		"\n"
 		"CLOCKID must be a valid SYS-V id (i.e. CLOCK_TAI)\n");
 }
@@ -160,6 +160,7 @@ static int taprio_parse_opt(struct qdisc_util *qu, int argc,
 	struct list_head sched_entries;
 	struct rtattr *tail, *l;
 	__u32 taprio_flags = 0;
+	__u32 txtime_delay = 0;
 	__s64 cycle_time = 0;
 	__s64 base_time = 0;
 	int err, idx;
@@ -293,6 +294,17 @@ static int taprio_parse_opt(struct qdisc_util *qu, int argc,
 				return -1;
 			}
 
+		} else if (strcmp(*argv, "txtime-delay") == 0) {
+			NEXT_ARG();
+			if (txtime_delay != 0) {
+				fprintf(stderr, "taprio: duplicate \"txtime-delay\" specification\n");
+				return -1;
+			}
+			if (get_u32(&txtime_delay, *argv, 0)) {
+				PREV_ARG();
+				return -1;
+			}
+
 		} else if (strcmp(*argv, "help") == 0) {
 			explain();
 			return -1;
@@ -315,6 +327,9 @@ static int taprio_parse_opt(struct qdisc_util *qu, int argc,
 	if (opt.num_tc > 0)
 		addattr_l(n, 1024, TCA_TAPRIO_ATTR_PRIOMAP, &opt, sizeof(opt));
 
+	if (txtime_delay)
+		addattr_l(n, 1024, TCA_TAPRIO_ATTR_TXTIME_DELAY, &txtime_delay, sizeof(txtime_delay));
+
 	if (base_time)
 		addattr_l(n, 1024, TCA_TAPRIO_ATTR_SCHED_BASE_TIME, &base_time, sizeof(base_time));
 
@@ -464,6 +479,13 @@ static int taprio_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 		print_0xhex(PRINT_ANY, "flags", " flags %#x", flags);
 	}
 
+	if (tb[TCA_TAPRIO_ATTR_TXTIME_DELAY]) {
+		__u32 txtime_delay;
+
+		txtime_delay = rta_getattr_s32(tb[TCA_TAPRIO_ATTR_TXTIME_DELAY]);
+		print_uint(PRINT_ANY, "txtime_delay", " txtime delay %d", txtime_delay);
+	}
+
 	print_schedule(f, tb);
 
 	if (tb[TCA_TAPRIO_ATTR_ADMIN_SCHED]) {
-- 
2.7.3


^ permalink raw reply related

* [PATCH iproute2 net-next v5 4/5] tc: etf: Add documentation for skip_sock_check.
From: Vedang Patel @ 2019-07-18 19:55 UTC (permalink / raw)
  To: netdev
  Cc: jhs, xiyou.wangcong, jiri, stephen, vinicius.gomes,
	leandro.maciel.dorileo, jakub.kicinski, m-karicheri2, dsahern,
	Vedang Patel
In-Reply-To: <1563479743-8371-1-git-send-email-vedang.patel@intel.com>

Document the newly added option (skip_sock_check) on the etf man-page.

Signed-off-by: Vedang Patel <vedang.patel@intel.com>
---
 man/man8/tc-etf.8 | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/man/man8/tc-etf.8 b/man/man8/tc-etf.8
index 30a12de7d2c7..4cb3b9e02d6e 100644
--- a/man/man8/tc-etf.8
+++ b/man/man8/tc-etf.8
@@ -106,6 +106,16 @@ referred to as "Launch Time" or "Time-Based Scheduling" by the
 documentation of network interface controllers.
 The default is for this option to be disabled.
 
+.TP
+skip_sock_check
+.br
+.BR etf(8)
+currently drops any packet which does not have a socket associated with it or
+if the socket does not have SO_TXTIME socket option set. But, this will not
+work if the launchtime is set by another entity inside the kernel (e.g. some
+other Qdisc). Setting the skip_sock_check will skip checking for a socket
+associated with the packet.
+
 .SH EXAMPLES
 
 ETF is used to enforce a Quality of Service. It controls when each
-- 
2.7.3


^ permalink raw reply related

* [PATCH iproute2 net-next v5 2/5] taprio: Add support for setting flags
From: Vedang Patel @ 2019-07-18 19:55 UTC (permalink / raw)
  To: netdev
  Cc: jhs, xiyou.wangcong, jiri, stephen, vinicius.gomes,
	leandro.maciel.dorileo, jakub.kicinski, m-karicheri2, dsahern,
	Vedang Patel
In-Reply-To: <1563479743-8371-1-git-send-email-vedang.patel@intel.com>

From: Vinicius Costa Gomes <vinicius.gomes@intel.com>

This allows a new parameter, flags, to be passed to taprio. Currently, it
only supports enabling the txtime-assist mode. But, we plan to add
different modes for taprio (e.g. hardware offloading) and this parameter
will be useful in enabling those modes.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Vedang Patel <vedang.patel@intel.com>
---
 tc/q_taprio.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tc/q_taprio.c b/tc/q_taprio.c
index 62c8c591da99..1db2aba6efe7 100644
--- a/tc/q_taprio.c
+++ b/tc/q_taprio.c
@@ -159,6 +159,7 @@ static int taprio_parse_opt(struct qdisc_util *qu, int argc,
 	__s64 cycle_time_extension = 0;
 	struct list_head sched_entries;
 	struct rtattr *tail, *l;
+	__u32 taprio_flags = 0;
 	__s64 cycle_time = 0;
 	__s64 base_time = 0;
 	int err, idx;
@@ -281,6 +282,17 @@ static int taprio_parse_opt(struct qdisc_util *qu, int argc,
 				explain_clockid(*argv);
 				return -1;
 			}
+		} else if (strcmp(*argv, "flags") == 0) {
+			NEXT_ARG();
+			if (taprio_flags) {
+				fprintf(stderr, "taprio: duplicate \"flags\" specification\n");
+				return -1;
+			}
+			if (get_u32(&taprio_flags, *argv, 0)) {
+				PREV_ARG();
+				return -1;
+			}
+
 		} else if (strcmp(*argv, "help") == 0) {
 			explain();
 			return -1;
@@ -297,6 +309,9 @@ static int taprio_parse_opt(struct qdisc_util *qu, int argc,
 	if (clockid != CLOCKID_INVALID)
 		addattr_l(n, 1024, TCA_TAPRIO_ATTR_SCHED_CLOCKID, &clockid, sizeof(clockid));
 
+	if (taprio_flags)
+		addattr_l(n, 1024, TCA_TAPRIO_ATTR_FLAGS, &taprio_flags, sizeof(taprio_flags));
+
 	if (opt.num_tc > 0)
 		addattr_l(n, 1024, TCA_TAPRIO_ATTR_PRIOMAP, &opt, sizeof(opt));
 
@@ -442,6 +457,13 @@ static int taprio_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 
 	print_string(PRINT_ANY, "clockid", "clockid %s", get_clock_name(clockid));
 
+	if (tb[TCA_TAPRIO_ATTR_FLAGS]) {
+		__u32 flags;
+
+		flags = rta_getattr_u32(tb[TCA_TAPRIO_ATTR_FLAGS]);
+		print_0xhex(PRINT_ANY, "flags", " flags %#x", flags);
+	}
+
 	print_schedule(f, tb);
 
 	if (tb[TCA_TAPRIO_ATTR_ADMIN_SCHED]) {
-- 
2.7.3


^ permalink raw reply related

* [PATCH iproute2 net-next v5 1/5] etf: Add skip_sock_check
From: Vedang Patel @ 2019-07-18 19:55 UTC (permalink / raw)
  To: netdev
  Cc: jhs, xiyou.wangcong, jiri, stephen, vinicius.gomes,
	leandro.maciel.dorileo, jakub.kicinski, m-karicheri2, dsahern,
	Vedang Patel

ETF Qdisc currently checks for a socket with SO_TXTIME socket option. If
either is not present, the packet is dropped. In the future commits, we
want other Qdiscs to add packet with launchtime to the ETF Qdisc. Also,
there are some packets (e.g. ICMP packets) which may not have a socket
associated with them.  So, add an option to skip this check.

Signed-off-by: Vedang Patel <vedang.patel@intel.com>
---
 tc/q_etf.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tc/q_etf.c b/tc/q_etf.c
index 76aca476c61d..c2090589bc64 100644
--- a/tc/q_etf.c
+++ b/tc/q_etf.c
@@ -130,6 +130,13 @@ static int etf_parse_opt(struct qdisc_util *qu, int argc,
 				explain_clockid(*argv);
 				return -1;
 			}
+		} else if (strcmp(*argv, "skip_sock_check") == 0) {
+			if (opt.flags & TC_ETF_SKIP_SOCK_CHECK) {
+				fprintf(stderr, "etf: duplicate \"skip_sock_check\" specification\n");
+				return -1;
+			}
+
+			opt.flags |= TC_ETF_SKIP_SOCK_CHECK;
 		} else if (strcmp(*argv, "help") == 0) {
 			explain();
 			return -1;
@@ -171,8 +178,10 @@ static int etf_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 	print_uint(PRINT_ANY, "delta", "delta %d ", qopt->delta);
 	print_string(PRINT_ANY, "offload", "offload %s ",
 				(qopt->flags & TC_ETF_OFFLOAD_ON) ? "on" : "off");
-	print_string(PRINT_ANY, "deadline_mode", "deadline_mode %s",
+	print_string(PRINT_ANY, "deadline_mode", "deadline_mode %s ",
 				(qopt->flags & TC_ETF_DEADLINE_MODE_ON) ? "on" : "off");
+	print_string(PRINT_ANY, "skip_sock_check", "skip_sock_check %s",
+				(qopt->flags & TC_ETF_SKIP_SOCK_CHECK) ? "on" : "off");
 
 	return 0;
 }
-- 
2.7.3


^ permalink raw reply related

* Re: [PATCH] net: fec: generate warning when using deprecated phy reset
From: Sven Van Asbroeck @ 2019-07-18 19:52 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Fabio Estevam, Lucas Stach, Fugang Duan, David S . Miller, netdev,
	linux-kernel
In-Reply-To: <20190718194131.GK25635@lunn.ch>

Hi Andrew,

On Thu, Jul 18, 2019 at 3:41 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> Hi Sven
>
> One option would be to submit a patch or a patchset changing all
> existing device tree files to make use of the core method. Anybody
> cut/pasting will then automatically use the correct core way of doing
> it.
>
> There is also a move towards using YAML to verify the correctness of
> DT files. It should be possible to mark the old property as
> deprecated, so there will be a build time warning, not a boot time
> warning.
>

Thanks for the helpful suggestions, that makes sense.

What I keep forgetting in my little arm-imx6 world, is that devicetrees
aren't in-kernel apis, but that they have out-of-kernel
dependencies. It makes more sense to to see them as userspace
apis, albeit directed at firmware/bootloaders, right?

So if bootloaders were as varied/uncontrollable as userspace,
then deprecated properties would have to be supported forever...

^ permalink raw reply

* Re: [RFC PATCH 0/5] PTP: add support for Intel's TGPIO controller
From: Andrew Lunn @ 2019-07-18 19:50 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Richard Cochran, netdev, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H . Peter Anvin, x86, linux-kernel,
	Christopher S . Hall
In-Reply-To: <20190716072038.8408-1-felipe.balbi@linux.intel.com>

On Tue, Jul 16, 2019 at 10:20:33AM +0300, Felipe Balbi wrote:
> TGPIO is a new IP which allows for time synchronization between systems
> without any other means of synchronization such as PTP or NTP. The
> driver is implemented as part of the PTP framework since its features
> covered most of what this controller can do.

Hi Felipe

Given the name TGPIO, can it also be used for plain old boring GPIO?
Does there need to be some sort of mux between GPIO and TGPIO? And an
interface into the generic GPIO core?

Also, is this always embedded into a SoC? Or could it actually be in a
discrete NIC?

Thanks
	Andrew

^ permalink raw reply

* Re: [PATCH iproute2 net-next v4 5/6] tc: etf: Add documentation for skip-skb-check.
From: Patel, Vedang @ 2019-07-18 19:46 UTC (permalink / raw)
  To: David Ahern
  Cc: netdev@vger.kernel.org, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
	stephen@networkplumber.org, Gomes, Vinicius, Dorileo, Leandro,
	jakub.kicinski@netronome.com, m-karicheri2@ti.com
In-Reply-To: <32e4deac-aaab-c437-1b76-529c16731877@gmail.com>



> On Jul 18, 2019, at 10:36 AM, David Ahern <dsahern@gmail.com> wrote:
> 
> On 7/16/19 1:53 PM, Vedang Patel wrote:
>> Document the newly added option (skip-skb-check) on the etf man-page.
>> 
>> Signed-off-by: Vedang Patel <vedang.patel@intel.com>
>> ---
>> man/man8/tc-etf.8 | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>> 
>> diff --git a/man/man8/tc-etf.8 b/man/man8/tc-etf.8
>> index 30a12de7d2c7..2e01a591dbaa 100644
>> --- a/man/man8/tc-etf.8
>> +++ b/man/man8/tc-etf.8
>> @@ -106,6 +106,16 @@ referred to as "Launch Time" or "Time-Based Scheduling" by the
>> documentation of network interface controllers.
>> The default is for this option to be disabled.
>> 
>> +.TP
>> +skip_skb_check
> 
> patch 1 adds skip_sock_check.
> 
Yes. I will fix this typo in the next version.

Thanks,
Vedang
>> +.br
>> +.BR etf(8)
>> +currently drops any packet which does not have a socket associated with it or
>> +if the socket does not have SO_TXTIME socket option set. But, this will not
>> +work if the launchtime is set by another entity inside the kernel (e.g. some
>> +other Qdisc). Setting the skip_skb_check will skip checking for a socket
>> +associated with the packet.
>> +
>> .SH EXAMPLES
>> 
>> ETF is used to enforce a Quality of Service. It controls when each


^ 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