* Re: [PATCH] be2net: fix adapter->big_page_size miscaculation
From: Qian Cai @ 2019-07-18 23:26 UTC (permalink / raw)
To: Bill Wendling
Cc: Nick Desaulniers, 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=3QUvdwJs1wW1w+5Mord-qFLa=_WkjTsiZuwGfcjkoEJGNQ@mail.gmail.com>
> On Jul 18, 2019, at 5:21 PM, Bill Wendling <morbo@google.com> wrote:
>
> [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.
I used clang-7.0.1. So, this is getting worse where both GCC and clang will start to suffer the
same problem.
>
>
> 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] net: ag71xx: fix return value check in ag71xx_probe()
From: David Miller @ 2019-07-18 23:21 UTC (permalink / raw)
To: weiyongjun1; +Cc: jcliburn, chris.snook, o.rempel, netdev, kernel-janitors
In-Reply-To: <20190717115225.23047-1-weiyongjun1@huawei.com>
Please resubmit these two ag71xx patches, you use a different subsystem
prefix in the Subject lines, so you should make them consistent.
Using just "ag71xx: " is perfectly fine.
Thank you.
^ permalink raw reply
* Re: [PATCH iproute2 net-next v5 1/5] etf: Add skip_sock_check
From: David Ahern @ 2019-07-18 22:48 UTC (permalink / raw)
To: Vedang Patel, netdev
Cc: jhs, xiyou.wangcong, jiri, stephen, vinicius.gomes,
leandro.maciel.dorileo, jakub.kicinski, m-karicheri2
In-Reply-To: <1563479743-8371-1-git-send-email-vedang.patel@intel.com>
On 7/18/19 1:55 PM, Vedang Patel wrote:
> 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(-)
>
applied all to iproute2-next. Thanks
^ permalink raw reply
* Re: [PATCH net-next iproute2 v2 0/3] net/sched: Introduce tc connection tracking
From: David Ahern @ 2019-07-18 22:42 UTC (permalink / raw)
To: Paul Blakey, Jiri Pirko, Roi Dayan, Yossi Kuperman, Oz Shlomo,
Marcelo Ricardo Leitner, netdev, David Miller, Aaron Conole,
Zhike Wang
Cc: Rony Efraim, nst-kernel, John Hurley, Simon Horman, Justin Pettit
In-Reply-To: <1562832867-32347-1-git-send-email-paulb@mellanox.com>
On 7/11/19 2:14 AM, Paul Blakey wrote:
> Hi,
>
> This patch series add connection tracking capabilities in tc.
> It does so via a new tc action, called act_ct, and new tc flower classifier matching.
> Act ct and relevant flower matches, are still under review in net-next mailing list.
>
...
>
> Paul Blakey (3):
> tc: add NLA_F_NESTED flag to all actions options nested block
> tc: Introduce tc ct action
> tc: flower: Add matching on conntrack info
>
applied to iproute2-next. Thanks
^ permalink raw reply
* Re: [PATCH] openvswitch: Fix a possible memory leak on dst_cache
From: Gregory Rose @ 2019-07-18 22:12 UTC (permalink / raw)
To: Haishuang Yan, Pravin B Shelar, David S. Miller; +Cc: netdev, linux-kernel
In-Reply-To: <1563466028-2531-1-git-send-email-yanhaishuang@cmss.chinamobile.com>
On 7/18/2019 9:07 AM, Haishuang Yan wrote:
> dst_cache should be destroyed when fail to add flow actions.
>
> Fixes: d71785ffc7e7 ("net: add dst_cache to ovs vxlan lwtunnel")
> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
> ---
> net/openvswitch/flow_netlink.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index d7559c6..1fd1cdd 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -2608,6 +2608,7 @@ static int validate_and_copy_set_tun(const struct nlattr *attr,
> sizeof(*ovs_tun), log);
> if (IS_ERR(a)) {
> dst_release((struct dst_entry *)tun_dst);
> + dst_cache_destroy(&tun_dst->u.tun_info.dst_cache);
> return PTR_ERR(a);
> }
>
Nack.
dst_release will decrement the ref count and will
call_rcu(&dst->rcu_head, dst_destroy_rcu) if the ref count is zero. No
other net drivers call dst_destroy SFAICT.
Haishuang,
are you trying to fix some specific problem here?
Thanks,
- Greg
^ permalink raw reply
* 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox