* Re: [PATCH bpf-next 4/9] libbpf: add libbpf_swap_print to get previous print func
From: Alexei Starovoitov @ 2019-07-27 0:30 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Stanislav Fomichev, Andrii Nakryiko, bpf, Networking,
Alexei Starovoitov, Daniel Borkmann, Kernel Team
In-Reply-To: <CAEf4BzYoiL7XAXFdLaf5TDDas42u+jUTy2WydgmJT7WiniqOqQ@mail.gmail.com>
On Fri, Jul 26, 2019 at 02:47:28PM -0700, Andrii Nakryiko wrote:
> On Fri, Jul 26, 2019 at 2:28 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > On 07/26, Andrii Nakryiko wrote:
> > > libbpf_swap_print allows to restore previously set print function.
> > > This is useful when running many independent test with one default print
> > > function, but overriding log verbosity for particular subset of tests.
> > Can we change the return type of libbpf_set_print instead and return
> > the old function from it? Will it break ABI?
>
> Yeah, thought about that, but I wasn't sure about ABI breakage. It
> seems like it shouldn't, so I'll just change libbpf_set_print
> signature instead.
I think it's ok to change return value of libbpf_set_print() from void
to useful pointer.
This function is not marked as __attribute__((__warn_unused_result__)),
so there should be no abi issues.
Please double check by compiler perf with different gcc-s as Arnaldo's setup does.
^ permalink raw reply
* Re: [PATCH bpf] libbpf: fix erroneous multi-closing of BTF FD
From: Alexei Starovoitov @ 2019-07-27 0:24 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Network Development, Alexei Starovoitov, Daniel Borkmann,
Andrey Ignatov, Yonghong Song, Andrii Nakryiko, Kernel Team
In-Reply-To: <20190726212438.1269599-1-andriin@fb.com>
On Fri, Jul 26, 2019 at 2:25 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> Libbpf stores associated BTF FD per each instance of bpf_program. When
> program is unloaded, that FD is closed. This is wrong, because leads to
> a race and possibly closing of unrelated files, if application
> simultaneously opens new files while bpf_programs are unloaded.
>
> It's also unnecessary, because struct btf "owns" that FD, and
> btf__free(), called from bpf_object__close() will close it. Thus the fix
> is to never have per-program BTF FD and fetch it from obj->btf, when
> necessary.
>
> Fixes: 2993e0515bb4 ("tools/bpf: add support to read .BTF.ext sections")
> Reported-by: Andrey Ignatov <rdna@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Applied. Thanks
^ permalink raw reply
* Re: [PATCH v2 bpf] libbpf: fix missing __WORDSIZE definition
From: Arnaldo Carvalho de Melo @ 2019-07-27 0:03 UTC (permalink / raw)
To: Andrii Nakryiko, Arnaldo Carvalho de Melo
Cc: Daniel Borkmann, Andrii Nakryiko, bpf, Networking,
Alexei Starovoitov, Arnaldo Carvalho de Melo, Kernel Team
In-Reply-To: <CAEf4BzYZT3fmQUuGp45+Mn6hLLyWnT2NE3PxfpD88sThX8JS_w@mail.gmail.com>
On July 26, 2019 8:01:10 PM GMT-03:00, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>On Fri, Jul 26, 2019 at 1:49 PM Arnaldo Carvalho de Melo
><arnaldo.melo@gmail.com> wrote:
>>
>> Em Thu, Jul 18, 2019 at 10:30:21AM -0700, Andrii Nakryiko escreveu:
>> > hashmap.h depends on __WORDSIZE being defined. It is defined by
>> > glibc/musl in different headers. It's an explicit goal for musl to
>be
>> > "non-detectable" at compilation time, so instead include glibc
>header if
>> > glibc is explicitly detected and fall back to musl header
>otherwise.
>> >
>> > Fixes: e3b924224028 ("libbpf: add resizable non-thread safe
>internal hashmap")
>> > Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>>
>> Couldn't find ths in the bpf tree, please consider applying it:
>>
>> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
>Arnaldo, I somehow got impression that you were going to pull this
>into your perf tree. Can you please confirm that it wasn't pulled into
>your tree, so that Alexei can apply it to bpf tree? Thanks!
>
I can process it, just was unsure about where it should go by,
I'll have it in my next perf/urgent pull req,
Thanks,
- Arnaldo
>
>>
>>
>> - Arnaldo
>>
>> > ---
>> > tools/lib/bpf/hashmap.h | 5 +++++
>> > 1 file changed, 5 insertions(+)
>> >
>> > diff --git a/tools/lib/bpf/hashmap.h b/tools/lib/bpf/hashmap.h
>> > index 03748a742146..bae8879cdf58 100644
>> > --- a/tools/lib/bpf/hashmap.h
>> > +++ b/tools/lib/bpf/hashmap.h
>> > @@ -10,6 +10,11 @@
>> >
>> > #include <stdbool.h>
>> > #include <stddef.h>
>> > +#ifdef __GLIBC__
>> > +#include <bits/wordsize.h>
>> > +#else
>> > +#include <bits/reg.h>
>> > +#endif
>> > #include "libbpf_internal.h"
>> >
>> > static inline size_t hash_bits(size_t h, int bits)
>> > --
>> > 2.17.1
>>
>> --
>>
>> - Arnaldo
^ permalink raw reply
* Re: [PATCH bpf-next 2/6] bpf: add BPF_MAP_DUMP command to dump more than one entry per call
From: Jakub Kicinski @ 2019-07-27 0:02 UTC (permalink / raw)
To: Brian Vazquez
Cc: Yonghong Song, Alexei Starovoitov, Song Liu, Brian Vazquez,
Daniel Borkmann, David S . Miller, Stanislav Fomichev,
Willem de Bruijn, Petar Penkov, Networking, bpf
In-Reply-To: <CABCgpaVB+iDGO132d9CTtC_GYiKJuuL6pe5_Krm3-THgvfMO=A@mail.gmail.com>
On Fri, 26 Jul 2019 16:36:19 -0700, Brian Vazquez wrote:
> > In bcc, we have many instances like this:
> > getting all (key value) pairs, do some analysis and output,
> > delete all keys
> >
> > The implementation typically like
> > /* to get all (key, value) pairs */
> > while(bpf_get_next_key() == 0)
> > bpf_map_lookup()
> > /* do analysis and output */
> > for (all keys)
> > bpf_map_delete()
>
> If you do that in a map that is being modified while you are doing the
> analysis and output, you will lose some new data by deleting the keys,
> right?
>
> > get_next+lookup+delete will be definitely useful.
> > batching will be even better to save the number of syscalls.
> >
> > An alternative is to do batch get_next+lookup and batch delete
> > to achieve similar goal as the above code.
>
> What I mentioned above is what it makes me think that with the
> deletion it'd be better if we perform these 3 operations at once:
> get_next+lookup+delete in a jumbo/atomic command and batch them later?
Hm. The lookup+delete are only "atomic" if we insert an RCU sync in
between right? The elements' lifetime is protected by RCU.
CPU 1 CPU 2
val = lookup(map, key)
val = lookup(map, key)
delete(map, key)
dump(val)
val->counter++
So we'd need to walk the hash table, disconnect all lists from the
buckets and splice them onto one combined list, then synchronize_rcu()
and only after that we can dump the elements and free them.
> > There is a minor difference between this approach
> > and the above get_next+lookup+delete.
> > During scanning the hash map, get_next+lookup may get less number
> > of elements compared to get_next+lookup+delete as the latter
> > may have more later-inserted hash elements after the operation
> > start. But both are inaccurate, so probably the difference
> > is minor.
> > For not changing map, looping of (get_next, lookup) and batch
> > get_next+lookup should have the same results.
>
> This is true for the api I'm presenting the only think that I was
> missing was what to do for changing maps to avoid the weird scenario
> (getting the first key due a concurrent deletion). And, in my opinion
> the way to go should be what also Willem supported: return the err to
> the caller and restart the dumping. I could do this with existing code
> just by detecting that we do provide a prev_key and got the first_key
> instead of the next_key or even implement a new function if you want
> to.
My knee jerk reaction to Willem's proposal was to nit pick that sizing
the dump space to the map's max entries doesn't guarantee all entries
will fit if an entry can be removed and re-added in a different place
while dumper proceeds.
If we keep elements disconnected from the hash array _without_
decrementing element count until synchronize_rcu() completes we have
that guarantee, but OTOH it may not be possible to add any new entries
from the datapath, so that doesn't sound great either :/
^ permalink raw reply
* Re: [PATCH bpf-next 2/6] bpf: add BPF_MAP_DUMP command to dump more than one entry per call
From: Brian Vazquez @ 2019-07-26 23:36 UTC (permalink / raw)
To: Yonghong Song
Cc: Alexei Starovoitov, Song Liu, Brian Vazquez, Daniel Borkmann,
David S . Miller, Stanislav Fomichev, Willem de Bruijn,
Petar Penkov, Networking, bpf
In-Reply-To: <beb513cb-2d76-30d4-6500-2892c6566a7e@fb.com>
On Thu, Jul 25, 2019 at 11:10 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 7/25/19 6:47 PM, Alexei Starovoitov wrote:
> > On Thu, Jul 25, 2019 at 6:24 PM Brian Vazquez <brianvv.kernel@gmail.com> wrote:
> >>
> >> On Thu, Jul 25, 2019 at 4:54 PM Alexei Starovoitov
> >> <alexei.starovoitov@gmail.com> wrote:
> >>>
> >>> On Thu, Jul 25, 2019 at 04:25:53PM -0700, Brian Vazquez wrote:
> >>>>>>> If prev_key is deleted before map_get_next_key(), we get the first key
> >>>>>>> again. This is pretty weird.
> >>>>>>
> >>>>>> Yes, I know. But note that the current scenario happens even for the
> >>>>>> old interface (imagine you are walking a map from userspace and you
> >>>>>> tried get_next_key the prev_key was removed, you will start again from
> >>>>>> the beginning without noticing it).
> >>>>>> I tried to sent a patch in the past but I was missing some context:
> >>>>>> before NULL was used to get the very first_key the interface relied in
> >>>>>> a random (non existent) key to retrieve the first_key in the map, and
> >>>>>> I was told what we still have to support that scenario.
> >>>>>
> >>>>> BPF_MAP_DUMP is slightly different, as you may return the first key
> >>>>> multiple times in the same call. Also, BPF_MAP_DUMP is new, so we
> >>>>> don't have to support legacy scenarios.
> >>>>>
> >>>>> Since BPF_MAP_DUMP keeps a list of elements. It is possible to try
> >>>>> to look up previous keys. Would something down this direction work?
> >>>>
> >>>> I've been thinking about it and I think first we need a way to detect
> >>>> that since key was not present we got the first_key instead:
> >>>>
> >>>> - One solution I had in mind was to explicitly asked for the first key
> >>>> with map_get_next_key(map, NULL, first_key) and while walking the map
> >>>> check that map_get_next_key(map, prev_key, key) doesn't return the
> >>>> same key. This could be done using memcmp.
> >>>> - Discussing with Stan, he mentioned that another option is to support
> >>>> a flag in map_get_next_key to let it know that we want an error
> >>>> instead of the first_key.
> >>>>
> >>>> After detecting the problem we also need to define what we want to do,
> >>>> here some options:
> >>>>
> >>>> a) Return the error to the caller
> >>>> b) Try with previous keys if any (which be limited to the keys that we
> >>>> have traversed so far in this dump call)
> >>>> c) continue with next entries in the map. array is easy just get the
> >>>> next valid key (starting on i+1), but hmap might be difficult since
> >>>> starting on the next bucket could potentially skip some keys that were
> >>>> concurrently added to the same bucket where key used to be, and
> >>>> starting on the same bucket could lead us to return repeated elements.
> >>>>
> >>>> Or maybe we could support those 3 cases via flags and let the caller
> >>>> decide which one to use?
> >>>
> >>> this type of indecision is the reason why I wasn't excited about
> >>> batch dumping in the first place and gave 'soft yes' when Stan
> >>> mentioned it during lsf/mm/bpf uconf.
> >>> We probably shouldn't do it.
> >>> It feels this map_dump makes api more complex and doesn't really
> >>> give much benefit to the user other than large map dump becomes faster.
> >>> I think we gotta solve this problem differently.
> >>
> >> Some users are working around the dumping problems with the existing
> >> api by creating a bpf_map_get_next_key_and_delete userspace function
> >> (see https://urldefense.proofpoint.com/v2/url?u=https-3A__www.bouncybouncy.net_blog_bpf-5Fmap-5Fget-5Fnext-5Fkey-2Dpitfalls_&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=XvNxqsDhRi62gzZ04HbLRTOFJX8X6mTuK7PZGn80akY&s=7q7beZxOJJ3Q0el8L0r-xDctedSpnEejJ6PVX1XYotQ&e= )
> >> which in my opinion is actually a good idea. The only problem with
> >> that is that calling bpf_map_get_next_key(fd, key, next_key) and then
> >> bpf_map_delete_elem(fd, key) from userspace is racing with kernel code
> >> and it might lose some information when deleting.
> >> We could then do map_dump_and_delete using that idea but in the kernel
> >> where we could better handle the racing condition. In that scenario
> >> even if we retrieve the same key it will contain different info ( the
> >> delta between old and new value). Would that work?
> >
> > you mean get_next+lookup+delete at once?
> > Sounds useful.
> > Yonghong has been thinking about batching api as well.
>
> In bcc, we have many instances like this:
> getting all (key value) pairs, do some analysis and output,
> delete all keys
>
> The implementation typically like
> /* to get all (key, value) pairs */
> while(bpf_get_next_key() == 0)
> bpf_map_lookup()
> /* do analysis and output */
> for (all keys)
> bpf_map_delete()
If you do that in a map that is being modified while you are doing the
analysis and output, you will lose some new data by deleting the keys,
right?
> get_next+lookup+delete will be definitely useful.
> batching will be even better to save the number of syscalls.
>
> An alternative is to do batch get_next+lookup and batch delete
> to achieve similar goal as the above code.
What I mentioned above is what it makes me think that with the
deletion it'd be better if we perform these 3 operations at once:
get_next+lookup+delete in a jumbo/atomic command and batch them later?
>
> There is a minor difference between this approach
> and the above get_next+lookup+delete.
> During scanning the hash map, get_next+lookup may get less number
> of elements compared to get_next+lookup+delete as the latter
> may have more later-inserted hash elements after the operation
> start. But both are inaccurate, so probably the difference
> is minor.
>
> >
> > I think if we cannot figure out how to make a batch of two commands
> > get_next + lookup to work correctly then we need to identify/invent one
> > command and make batching more generic.
>
> not 100% sure. It will be hard to define what is "correctly".
I agree, it'll be hard to define what is the right behavior.
> For not changing map, looping of (get_next, lookup) and batch
> get_next+lookup should have the same results.
This is true for the api I'm presenting the only think that I was
missing was what to do for changing maps to avoid the weird scenario
(getting the first key due a concurrent deletion). And, in my opinion
the way to go should be what also Willem supported: return the err to
the caller and restart the dumping. I could do this with existing code
just by detecting that we do provide a prev_key and got the first_key
instead of the next_key or even implement a new function if you want
to.
> For constant changing loops, not sure how to define which one
> is correct. If users have concerns, they may need to just pick one
> which gives them more comfort.
>
> > Like make one jumbo/compound/atomic command to be get_next+lookup+delete.
> > Define the semantics of this single compound command.
> > And then let batching to be a multiplier of such command.
> > In a sense that multiplier 1 or N should be have the same way.
> > No extra flags to alter the batching.
> > The high level description of the batch would be:
> > pls execute get_next,lookup,delete and repeat it N times.
> > or
> > pls execute get_next,lookup and repeat N times.
But any attempt to do get_next+lookup will have same problem with
deletions right?
I don't see how we could do it more consistent than what I'm
proposing. Let's just support one case: report an error if the
prev_key was not found instead of retrieving the first_key. Would that
work?
> > where each command action is defined to be composable.
> >
> > Just a rough idea.
> >
^ permalink raw reply
* Re: [PATCH bpf-next 06/10] selftests/bpf: add CO-RE relocs array tests
From: Andrii Nakryiko @ 2019-07-26 23:29 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Yonghong Song, Kernel Team
In-Reply-To: <20190725232633.zt6fxixq72xqwwmz@ast-mbp>
On Thu, Jul 25, 2019 at 4:26 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Jul 24, 2019 at 12:27:38PM -0700, Andrii Nakryiko wrote:
> > Add tests for various array handling/relocation scenarios.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ...
> > +
> > +#define CORE_READ(dst, src) \
> > + bpf_probe_read(dst, sizeof(*src), __builtin_preserve_access_index(src))
>
> This is the key accessor that all progs will use.
> Please split just this single macro into individual commit and add
> detailed comment about its purpose and
> what __builtin_preserve_access_index() does underneath.
I'm planning to add more powerful and flexible set of macros to
support BCC style a->b->c->d accesses using single macro. Something
like BPF_CORE_READ(&dst, sizeof(dst), a, b, c, d). I want to move
bpf_helpers.h into libbpf itself first, after some clean up. How about
I write all that at that time and for now just add this simpler
CORE_READ into bpf_helpers.h?
Relocations recorded by __builtin_preserve_access_index() are
described pretty well in patch #1, which adds bpf_offset_reloc. I'll
double check if I mention this built-in there, and if not - will add
that.
>
> > +SEC("raw_tracepoint/sys_enter")
> > +int test_core_nesting(void *ctx)
> > +{
> > + struct core_reloc_arrays *in = (void *)&data.in;
> > + struct core_reloc_arrays_output *out = (void *)&data.out;
> > +
> > + /* in->a[2] */
> > + if (CORE_READ(&out->a2, &in->a[2]))
> > + return 1;
> > + /* in->b[1][2][3] */
> > + if (CORE_READ(&out->b123, &in->b[1][2][3]))
> > + return 1;
^ permalink raw reply
* Re: memory leak in kobject_set_name_vargs (2)
From: syzbot @ 2019-07-26 23:26 UTC (permalink / raw)
To: catalin.marinas, davem, dvyukov, herbert, kuznet, kvalo,
linux-kernel, linux-mm, luciano.coelho, netdev, steffen.klassert,
syzkaller-bugs, torvalds, yoshfuji
In-Reply-To: <000000000000edcb3c058e6143d5@google.com>
syzbot has bisected this bug to:
commit 0e034f5c4bc408c943f9c4a06244415d75d7108c
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed May 18 18:51:25 2016 +0000
iwlwifi: fix mis-merge that breaks the driver
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=10f955f0600000
start commit: 3bfe1fc4 Merge tag 'for-5.3/dm-changes-2' of git://git.ker..
git tree: upstream
final crash: https://syzkaller.appspot.com/x/report.txt?x=12f955f0600000
console output: https://syzkaller.appspot.com/x/log.txt?x=14f955f0600000
kernel config: https://syzkaller.appspot.com/x/.config?x=dcfc65ee492509c6
dashboard link: https://syzkaller.appspot.com/bug?extid=ad8ca40ecd77896d51e2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=135cbed0600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14dd4e34600000
Reported-by: syzbot+ad8ca40ecd77896d51e2@syzkaller.appspotmail.com
Fixes: 0e034f5c4bc4 ("iwlwifi: fix mis-merge that breaks the driver")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* Re: [PATCH net-next 08/11] net: hns3: add interrupt affinity support for misc interrupt
From: Saeed Mahameed @ 2019-07-26 23:18 UTC (permalink / raw)
To: linyunsheng@huawei.com, tanhuazhong@huawei.com,
davem@davemloft.net
Cc: lipeng321@huawei.com, yisen.zhuang@huawei.com,
salil.mehta@huawei.com, linuxarm@huawei.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <db2d081f-b892-b141-7fa5-44e66dd97eb9@huawei.com>
On Thu, 2019-07-25 at 10:05 +0800, Yunsheng Lin wrote:
> On 2019/7/25 3:24, Saeed Mahameed wrote:
> > On Wed, 2019-07-24 at 11:18 +0800, Huazhong Tan wrote:
> > > From: Yunsheng Lin <linyunsheng@huawei.com>
> > >
[...]
> > >
> > > +static void hclge_irq_affinity_notify(struct irq_affinity_notify
> > > *notify,
> > > + const cpumask_t *mask)
> > > +{
> > > + struct hclge_dev *hdev = container_of(notify, struct hclge_dev,
> > > + affinity_notify);
> > > +
> > > + cpumask_copy(&hdev->affinity_mask, mask);
> > > + del_timer_sync(&hdev->service_timer);
> > > + hdev->service_timer.expires = jiffies + HZ;
> > > + add_timer_on(&hdev->service_timer, cpumask_first(&hdev-
> > > > affinity_mask));
> > > +}
> >
> > I don't see any relation between your misc irq vector and &hdev-
> > > service_timer, to me this looks like an abuse of the irq affinity
> > > API
> > to allow the user to move the service timer affinity.
>
> Hi, thanks for reviewing.
>
> hdev->service_timer is used to schedule the periodic work
> queue hdev->service_task， we want all the management work
> queue including hdev->service_task to bind to the same cpu
> to improve cache and power efficiency, it is better to move
> service timer affinity according to that.
>
> The hdev->service_task is changed to delay work queue in
> next patch " net: hns3: make hclge_service use delayed workqueue",
> So the affinity in the timer of the delay work queue is automatically
> set to the affinity of the delay work queue, we will move the
> "make hclge_service use delayed workqueue" patch before the
> "add interrupt affinity support for misc interrupt" patch, so
> we do not have to set service timer affinity explicitly.
>
> Also, There is there work queues(mbx_service_task, service_task,
> rst_service_task) in the hns3 driver, we plan to combine them
> to one or two workqueue to improve efficiency and readability.
>
So just to make it clear, you have 3 deferred works, 2 are triggered by
the misc irq and 1 is periodic, you want them all on the same core and
you want to control their affinity via the irq affinity ? for works #1
and #2 you get that for free since the irq is triggering them, but for
work #3 (the periodic one) you need to manually move it when the irq
affinity changes.
I guess i am ok with this, since moving the irq affinity isn't only
required by the periodic work but also for the other works that the irq
is actually triggering (so it is not an actual abuse, sort of .. )
^ permalink raw reply
* Re: [PATCH v2 bpf] libbpf: fix missing __WORDSIZE definition
From: Andrii Nakryiko @ 2019-07-26 23:01 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Daniel Borkmann, Andrii Nakryiko, bpf, Networking,
Alexei Starovoitov, Arnaldo Carvalho de Melo, Kernel Team
In-Reply-To: <20190726204937.GD24867@kernel.org>
On Fri, Jul 26, 2019 at 1:49 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Thu, Jul 18, 2019 at 10:30:21AM -0700, Andrii Nakryiko escreveu:
> > hashmap.h depends on __WORDSIZE being defined. It is defined by
> > glibc/musl in different headers. It's an explicit goal for musl to be
> > "non-detectable" at compilation time, so instead include glibc header if
> > glibc is explicitly detected and fall back to musl header otherwise.
> >
> > Fixes: e3b924224028 ("libbpf: add resizable non-thread safe internal hashmap")
> > Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>
> Couldn't find ths in the bpf tree, please consider applying it:
>
> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Arnaldo, I somehow got impression that you were going to pull this
into your perf tree. Can you please confirm that it wasn't pulled into
your tree, so that Alexei can apply it to bpf tree? Thanks!
>
>
> - Arnaldo
>
> > ---
> > tools/lib/bpf/hashmap.h | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/tools/lib/bpf/hashmap.h b/tools/lib/bpf/hashmap.h
> > index 03748a742146..bae8879cdf58 100644
> > --- a/tools/lib/bpf/hashmap.h
> > +++ b/tools/lib/bpf/hashmap.h
> > @@ -10,6 +10,11 @@
> >
> > #include <stdbool.h>
> > #include <stddef.h>
> > +#ifdef __GLIBC__
> > +#include <bits/wordsize.h>
> > +#else
> > +#include <bits/reg.h>
> > +#endif
> > #include "libbpf_internal.h"
> >
> > static inline size_t hash_bits(size_t h, int bits)
> > --
> > 2.17.1
>
> --
>
> - Arnaldo
^ permalink raw reply
* Re: [PATCH net-next v2] mlx4/en_netdev: allow offloading VXLAN over VLAN
From: Saeed Mahameed @ 2019-07-26 22:47 UTC (permalink / raw)
To: dcaratti@redhat.com, netdev@vger.kernel.org, davem@davemloft.net,
Tariq Toukan, pabeni@redhat.com, marcelo.leitner@gmail.com
In-Reply-To: <2beb05557960e04aa588ecc09e9ee5e5a19fc651.1564164688.git.dcaratti@redhat.com>
On Fri, 2019-07-26 at 20:18 +0200, Davide Caratti wrote:
> ConnectX-3 Pro can offload transmission of VLAN packets with VXLAN
> inside:
> enable tunnel offloads in dev->vlan_features, like it's done with
> other
> NIC drivers (e.g. be2net and ixgbe).
>
> It's no more necessary to change dev->hw_enc_features when VXLAN are
> added
> or removed, since .ndo_features_check() already checks for VXLAN
> packet
> where the UDP destination port matches the configured value. Just set
> dev->hw_enc_features when the NIC is initialized, so that overlying
> VLAN
> can correctly inherit the tunnel offload capabilities.
>
> Changes since v1:
> - avoid flipping hw_enc_features, instead of calling netdev
> notifiers,
> thanks to Saeed Mahameed
> - squash two patches into a single one
>
> CC: Paolo Abeni <pabeni@redhat.com>
> CC: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
^ permalink raw reply
* Re: [PATCH bpf-next 6/9] selftests/bpf: abstract away test log output
From: Stanislav Fomichev @ 2019-07-26 22:26 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
In-Reply-To: <CAEf4BzaVCdHT_U+m7niJLsSmbf+M9DrFjf_PNOmQQZvuHsr9Xg@mail.gmail.com>
On 07/26, Andrii Nakryiko wrote:
> On Fri, Jul 26, 2019 at 2:31 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > On 07/26, Andrii Nakryiko wrote:
> > > This patch changes how test output is printed out. By default, if test
> > > had no errors, the only output will be a single line with test number,
> > > name, and verdict at the end, e.g.:
> > >
> > > #31 xdp:OK
> > >
> > > If test had any errors, all log output captured during test execution
> > > will be output after test completes.
> > >
> > > It's possible to force output of log with `-v` (`--verbose`) option, in
> > > which case output won't be buffered and will be output immediately.
> > >
> > > To support this, individual tests are required to use helper methods for
> > > logging: `test__printf()` and `test__vprintf()`.
> > >
> > > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > > ---
> > > .../selftests/bpf/prog_tests/bpf_obj_id.c | 6 +-
> > > .../bpf/prog_tests/bpf_verif_scale.c | 31 ++--
> > > .../bpf/prog_tests/get_stack_raw_tp.c | 4 +-
> > > .../selftests/bpf/prog_tests/l4lb_all.c | 2 +-
> > > .../selftests/bpf/prog_tests/map_lock.c | 10 +-
> > > .../selftests/bpf/prog_tests/send_signal.c | 8 +-
> > > .../selftests/bpf/prog_tests/spinlock.c | 2 +-
> > > .../bpf/prog_tests/stacktrace_build_id.c | 4 +-
> > > .../bpf/prog_tests/stacktrace_build_id_nmi.c | 4 +-
> > > .../selftests/bpf/prog_tests/xdp_noinline.c | 3 +-
> > > tools/testing/selftests/bpf/test_progs.c | 135 +++++++++++++-----
> > > tools/testing/selftests/bpf/test_progs.h | 37 ++++-
> > > 12 files changed, 173 insertions(+), 73 deletions(-)
> > >
>
> [...]
>
> > > error_cnt++;
> > > - printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
> > > + test__printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
> > #define printf(...) test__printf(...) in tests.h?
> >
> > A bit ugly, but no need to retrain everyone to use new printf wrappers.
>
> I try to reduce amount of magic and surprising things, not add new
> ones :) I also led by example and converted all current instances of
> printf usage to test__printf, so anyone new will just copy/paste good
> example, hopefully. Even if not, this non-buffered output will be
> immediately obvious to anyone who just runs `sudo ./test_progs`.
[..]
> And
> author of new test with this problem should hopefully be the first and
> the only one to catch and fix this.
Yeah, that is my only concern, that regular printfs will eventually
creep in. It's already confusing to go to/from printf/printk.
2c:
I'm coming from a perspective of tools/testing/selftests/kselftest.h
which is supposed to be a generic framework with custom
printf variants (ksft_print_msg), but I still see a bunch of tests
calling printf :-/
grep -ril ksft_exit_fail_msg selftests/ | xargs -n1 grep -w printf
Since we don't expect regular buffered io from the tests anyway
it might be easier just to add a bit of magic and call it a day.
> > > }
> > > out:
> > > bpf_object__close(obj);
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/map_lock.c b/tools/testing/selftests/bpf/prog_tests/map_lock.c
> > > index ee99368c595c..2e78217ed3fd 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/map_lock.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/map_lock.c
> > > @@ -9,12 +9,12 @@ static void *parallel_map_access(void *arg)
>
> [...]
^ permalink raw reply
* Re: [PATCH V2 net-next 07/11] net: hns3: adds debug messages to identify eth down cause
From: Joe Perches @ 2019-07-26 22:18 UTC (permalink / raw)
To: Saeed Mahameed, tanhuazhong@huawei.com, davem@davemloft.net
Cc: lipeng321@huawei.com, yisen.zhuang@huawei.com,
salil.mehta@huawei.com, linuxarm@huawei.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
liuyonglong@huawei.com
In-Reply-To: <a32ca755bfd69046cf89aeacbf67fd16313de768.camel@mellanox.com>
On Fri, 2019-07-26 at 22:00 +0000, Saeed Mahameed wrote:
> On Fri, 2019-07-26 at 11:24 +0800, Huazhong Tan wrote:
> > From: Yonglong Liu <liuyonglong@huawei.com>
> >
> > Some times just see the eth interface have been down/up via
> > dmesg, but can not know why the eth down. So adds some debug
> > messages to identify the cause for this.
[]
> > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> []
> > @@ -459,6 +459,10 @@ static int hns3_nic_net_open(struct net_device
> > *netdev)
> > h->ae_algo->ops->set_timer_task(priv->ae_handle, true);
> >
> > hns3_config_xps(priv);
> > +
> > + if (netif_msg_drv(h))
> > + netdev_info(netdev, "net open\n");
> > +
>
> to make sure this is only intended for debug, and to avoid repetition.
> #define hns3_dbg(__dev, format, args...) \
> ({ \
> if (netif_msg_drv(h)) \
> netdev_info(h->netdev, format, ##args); \
> })
netif_dbg(h, drv, h->netdev, "net open\n")
^ permalink raw reply
* [PATCH] gigaset: stop maintaining seperately
From: Paul Bolle @ 2019-07-26 22:05 UTC (permalink / raw)
To: David Miller
Cc: Tilman Schmidt, Hansjoerg Lipp, Arnd Bergmann, Karsten Keil,
netdev, linux-kernel
The Dutch consumer grade ISDN network will be shut down on September 1,
2019. This means I'll be converted to some sort of VOIP shortly. At that
point it would be unwise to try to maintain the gigaset driver, even for
odd fixes as I do. So I'll stop maintaining it as a seperate driver and
bump support to CAPI in staging. De facto this means the driver will be
unmaintained, since no-one seems to be working on CAPI.
I've lighty tested the hardware specific modules of this driver (bas-gigaset,
ser-gigaset, and usb-gigaset) for v5.3-rc1. The basic functionality appears to
be working. It's unclear whether anyone still cares. I'm aware of only one
person sort of using the driver a few years ago.
Thanks to Karsten Keil for the ISDN subsystems gigaset was using (I4L and
CAPI). And many thanks to Hansjoerg Lipp and Tilman Schmidt for writing and
upstreaming this driver.
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
MAINTAINERS | 7 -------
1 file changed, 7 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 783569e3c4b4..e99afbd13355 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6822,13 +6822,6 @@ F: Documentation/filesystems/gfs2*.txt
F: fs/gfs2/
F: include/uapi/linux/gfs2_ondisk.h
-GIGASET ISDN DRIVERS
-M: Paul Bolle <pebolle@tiscali.nl>
-L: gigaset307x-common@lists.sourceforge.net
-W: http://gigaset307x.sourceforge.net/
-S: Odd Fixes
-F: drivers/staging/isdn/gigaset/
-
GNSS SUBSYSTEM
M: Johan Hovold <johan@kernel.org>
T: git git://git.kernel.org/pub/scm/linux/kernel/git/johan/gnss.git
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] iplink_can: fix format output of clock with flag -details
From: Stephen Hemminger @ 2019-07-26 22:04 UTC (permalink / raw)
To: Antonio Borneo; +Cc: netdev
In-Reply-To: <20190726130609.27704-1-borneo.antonio@gmail.com>
On Fri, 26 Jul 2019 15:06:09 +0200
Antonio Borneo <borneo.antonio@gmail.com> wrote:
> The command
> ip -details link show can0
> prints in the last line the value of the clock frequency attached
> to the name of the following value "numtxqueues", e.g.
> clock 49500000numtxqueues 1 numrxqueues 1 gso_max_size
> 65536 gso_max_segs 65535
>
> Add the missing space after the clock value.
>
> Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Applied, but CAN looks like it doesn't support single line output correctly.
^ permalink raw reply
* Re: [PATCH bpf-next 3/9] selftests/bpf: add test selectors by number and name to test_progs
From: Stanislav Fomichev @ 2019-07-26 22:03 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
In-Reply-To: <CAEf4BzZRhHTo+vUFkmLnjPxTL8oi6Fi0zrhvhA6JbY_afU3_Nw@mail.gmail.com>
On 07/26, Andrii Nakryiko wrote:
> On Fri, Jul 26, 2019 at 2:25 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > On 07/26, Andrii Nakryiko wrote:
> > > Add ability to specify either test number or test name substring to
> > > narrow down a set of test to run.
> > >
> > > Usage:
> > > sudo ./test_progs -n 1
> > > sudo ./test_progs -t attach_probe
> > >
> > > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > > ---
> > > tools/testing/selftests/bpf/test_progs.c | 43 +++++++++++++++++++++---
> > > 1 file changed, 39 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> > > index eea88ba59225..6e04b9f83777 100644
> > > --- a/tools/testing/selftests/bpf/test_progs.c
> > > +++ b/tools/testing/selftests/bpf/test_progs.c
> > > @@ -4,6 +4,7 @@
>
> [...]
>
> > >
> > > static error_t parse_arg(int key, char *arg, struct argp_state *state)
> > > {
> > > struct test_env *env = state->input;
> > >
> > > switch (key) {
> > [..]
> > > + case ARG_TEST_NUM: {
> > > + int test_num;
> > > +
> > > + errno = 0;
> > > + test_num = strtol(arg, NULL, 10);
> > > + if (errno)
> > > + return -errno;
> > > + env->test_num_selector = test_num;
> > > + break;
> > > + }
> > Do you think it's really useful? I agree about running by name (I
>
> Special request from Alexei :) But in one of the follow up patches, I
> extended this to allow to specify arbitrary subset of tests, e.g.:
> 1,2,5-10,7-8. So in that regard, it's more powerful than selecting by
> name and gives you ultimate freedom.
I guess I didn't read the series close enough; that '1,2,3' mode does seem
quite useful indeed!
>
> > usually used grep -v in the Makefile :-), but I'm not sure about running
> > by number.
> >
> > Or is the idea is that you can just copy-paste this number from the
> > test_progs output to rerun the tests? In this case, why not copy-paste
> > the name instead?
>
> Both were simple to support, I didn't want to dictate one right way to
> do this :)
>
> >
> > > + case ARG_TEST_NAME:
> > > + env->test_name_selector = arg;
> > > + break;
> > > case ARG_VERIFIER_STATS:
> > > env->verifier_stats = true;
> > > break;
> > > @@ -223,7 +248,7 @@ int main(int argc, char **argv)
> > > .parser = parse_arg,
> > > .doc = argp_program_doc,
> > > };
> > > - const struct prog_test_def *def;
> > > + struct prog_test_def *test;
> > > int err, i;
> > >
> > > err = argp_parse(&argp, argc, argv, 0, NULL, &env);
> > > @@ -237,8 +262,18 @@ int main(int argc, char **argv)
> > > verifier_stats = env.verifier_stats;
> > >
> > > for (i = 0; i < ARRAY_SIZE(prog_test_defs); i++) {
> > > - def = &prog_test_defs[i];
> > > - def->run_test();
> > > + test = &prog_test_defs[i];
> > > +
> > > + test->test_num = i + 1;
> > > +
> > > + if (env.test_num_selector >= 0 &&
> > > + test->test_num != env.test_num_selector)
> > > + continue;
> > > + if (env.test_name_selector &&
> > > + !strstr(test->test_name, env.test_name_selector))
> > > + continue;
> > > +
> > > + test->run_test();
> > > }
> > >
> > > printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
> > > --
> > > 2.17.1
> > >
^ permalink raw reply
* Re: [PATCH v2] iproute2: devlink: port from sys/queue.h to list.h
From: Stephen Hemminger @ 2019-07-26 22:02 UTC (permalink / raw)
To: Sergei Trofimovich; +Cc: netdev
In-Reply-To: <20190726210105.25458-1-slyfox@gentoo.org>
On Fri, 26 Jul 2019 22:01:05 +0100
Sergei Trofimovich <slyfox@gentoo.org> wrote:
> sys/queue.h does not exist on linux-musl targets and fails build as:
>
> devlink.c:28:10: fatal error: sys/queue.h: No such file or directory
> 28 | #include <sys/queue.h>
> | ^~~~~~~~~~~~~
>
> The change ports to list.h API and drops dependency of 'sys/queue.h'.
> The API maps one-to-one.
>
> Build-tested on linux-musl and linux-glibc.
>
> Bug: https://bugs.gentoo.org/690486
> CC: Stephen Hemminger <stephen@networkplumber.org>
> CC: netdev@vger.kernel.org
> Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
Looks good, thanks for following through.
Applied.
^ permalink raw reply
* Re: [PATCH bpf-next 1/9] selftests/bpf: prevent headers to be compiled as C code
From: Stanislav Fomichev @ 2019-07-26 22:01 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
In-Reply-To: <CAEf4BzYDvZENJqrT0KKpHbfHNCdObB9p4ZcJqQj3+rM_1ESF3g@mail.gmail.com>
On 07/26, Andrii Nakryiko wrote:
> On Fri, Jul 26, 2019 at 2:21 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > On 07/26, Andrii Nakryiko wrote:
> > > Apprently listing header as a normal dependency for a binary output
> > > makes it go through compilation as if it was C code. This currently
> > > works without a problem, but in subsequent commits causes problems for
> > > differently generated test.h for test_progs. Marking those headers as
> > > order-only dependency solves the issue.
> > Are you sure it will not result in a situation where
> > test_progs/test_maps is not regenerated if tests.h is updated.
> >
> > If I read the following doc correctly, order deps make sense for
> > directories only:
> > https://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html
> >
> > Can you maybe double check it with:
> > * make
> > * add new prog_tests/test_something.c
> > * make
> > to see if the binary is regenerated with test_something.c?
>
> Yeah, tested that, it triggers test_progs rebuild.
>
> Ordering is still preserved, because test.h is dependency of
> test_progs.c, which is dependency of test_progs binary, so that's why
> it works.
>
> As to why .h file is compiled as C file, I have no idea and ideally
> that should be fixed somehow.
I guess that's because it's a prerequisite and we have a target that
puts all prerequisites when calling CC:
test_progs: a.c b.c tests.h
gcc a.c b.c tests.h -o test_progs
So gcc compiles each input file.
I'm not actually sure why default dependency system that uses 'gcc -M'
is not working for us (see scripts/Kbuild.include) and we need to manually
add tests.h dependency. But that's outside of the scope..
> I also started with just removing header as dependency completely
> (because it's indirect dependency of test_progs.c), but that broke the
> build logic. Dunno, too much magic... This works, tested many-many
> times, so I was satisfied enough :)
Yeah, that's my only concern, too much magic already and we add
quite a bit more.
> > Maybe fix the problem of header compilation by having '#ifndef
> > DECLARE_TEST #define DECLARE_TEST() #endif' in tests.h instead?
>
> That's ugly, I'd like to avoid doing that.
That's your call, but I'm not sure what's uglier: complicating already
complex make rules or making a header self contained.
> > > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > > ---
> > > tools/testing/selftests/bpf/Makefile | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > > index 11c9c62c3362..bb66cc4a7f34 100644
> > > --- a/tools/testing/selftests/bpf/Makefile
> > > +++ b/tools/testing/selftests/bpf/Makefile
> > > @@ -235,7 +235,7 @@ PROG_TESTS_H := $(PROG_TESTS_DIR)/tests.h
> > > PROG_TESTS_FILES := $(wildcard prog_tests/*.c)
> > > test_progs.c: $(PROG_TESTS_H)
> > > $(OUTPUT)/test_progs: CFLAGS += $(TEST_PROGS_CFLAGS)
> > > -$(OUTPUT)/test_progs: test_progs.c $(PROG_TESTS_H) $(PROG_TESTS_FILES)
> > > +$(OUTPUT)/test_progs: test_progs.c $(PROG_TESTS_FILES) | $(PROG_TESTS_H)
> > > $(PROG_TESTS_H): $(PROG_TESTS_FILES) | $(PROG_TESTS_DIR)
> > > $(shell ( cd prog_tests/; \
> > > echo '/* Generated header, do not edit */'; \
> > > @@ -256,7 +256,7 @@ MAP_TESTS_H := $(MAP_TESTS_DIR)/tests.h
> > > MAP_TESTS_FILES := $(wildcard map_tests/*.c)
> > > test_maps.c: $(MAP_TESTS_H)
> > > $(OUTPUT)/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS)
> > > -$(OUTPUT)/test_maps: test_maps.c $(MAP_TESTS_H) $(MAP_TESTS_FILES)
> > > +$(OUTPUT)/test_maps: test_maps.c $(MAP_TESTS_FILES) | $(MAP_TESTS_H)
> > > $(MAP_TESTS_H): $(MAP_TESTS_FILES) | $(MAP_TESTS_DIR)
> > > $(shell ( cd map_tests/; \
> > > echo '/* Generated header, do not edit */'; \
> > > @@ -277,7 +277,7 @@ VERIFIER_TESTS_H := $(VERIFIER_TESTS_DIR)/tests.h
> > > VERIFIER_TEST_FILES := $(wildcard verifier/*.c)
> > > test_verifier.c: $(VERIFIER_TESTS_H)
> > > $(OUTPUT)/test_verifier: CFLAGS += $(TEST_VERIFIER_CFLAGS)
> > > -$(OUTPUT)/test_verifier: test_verifier.c $(VERIFIER_TESTS_H)
> > > +$(OUTPUT)/test_verifier: test_verifier.c | $(VERIFIER_TEST_FILES) $(VERIFIER_TESTS_H)
> > > $(VERIFIER_TESTS_H): $(VERIFIER_TEST_FILES) | $(VERIFIER_TESTS_DIR)
> > > $(shell ( cd verifier/; \
> > > echo '/* Generated header, do not edit */'; \
> > > --
> > > 2.17.1
> > >
^ permalink raw reply
* Re: [PATCH V2 net-next 07/11] net: hns3: adds debug messages to identify eth down cause
From: Saeed Mahameed @ 2019-07-26 22:00 UTC (permalink / raw)
To: tanhuazhong@huawei.com, davem@davemloft.net
Cc: lipeng321@huawei.com, yisen.zhuang@huawei.com,
salil.mehta@huawei.com, linuxarm@huawei.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
liuyonglong@huawei.com
In-Reply-To: <1564111502-15504-8-git-send-email-tanhuazhong@huawei.com>
On Fri, 2019-07-26 at 11:24 +0800, Huazhong Tan wrote:
> From: Yonglong Liu <liuyonglong@huawei.com>
>
> Some times just see the eth interface have been down/up via
> dmesg, but can not know why the eth down. So adds some debug
> messages to identify the cause for this.
>
> Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
> Signed-off-by: Peng Li <lipeng321@huawei.com>
> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
> ---
> drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 24
> ++++++++++++++++++++
> drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 26
> ++++++++++++++++++++++
> .../net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c | 14 ++++++++++++
> 3 files changed, 64 insertions(+)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> index 4d58c53..2e30cfa 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> @@ -459,6 +459,10 @@ static int hns3_nic_net_open(struct net_device
> *netdev)
> h->ae_algo->ops->set_timer_task(priv->ae_handle, true);
>
> hns3_config_xps(priv);
> +
> + if (netif_msg_drv(h))
> + netdev_info(netdev, "net open\n");
> +
to make sure this is only intended for debug, and to avoid repetition.
#define hns3_dbg(__dev, format, args...) \
({ \
if (netif_msg_drv(h)) \
netdev_info(h->netdev, format, ##args); \
})
#endif
^ permalink raw reply
* [PATCH] iplink: document 'change' option to ip link
From: Stephen Hemminger @ 2019-07-26 21:59 UTC (permalink / raw)
To: mcroce; +Cc: netdev, Stephen Hemminger
In-Reply-To: <20190724191218.11757-1-mcroce@redhat.com>
Add the command alias "change" to man page.
Don't show it on usage, since it is not commonly used.
Reported-off-by: Matteo Croce <mcroce@redhat.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
man/man8/ip-link.8.in | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 883d88077d66..a8ae72d2b097 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -1815,6 +1815,11 @@ can move the system to an unpredictable state. The solution
is to avoid changing several parameters with one
.B ip link set
call.
+The modifier
+.B change
+is equivalent to
+.BR "set" .
+
.TP
.BI dev " DEVICE "
--
2.20.1
^ permalink raw reply related
* Re: [PATCH bpf-next 6/9] selftests/bpf: abstract away test log output
From: Andrii Nakryiko @ 2019-07-26 21:51 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
In-Reply-To: <20190726213104.GD24397@mini-arch>
On Fri, Jul 26, 2019 at 2:31 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 07/26, Andrii Nakryiko wrote:
> > This patch changes how test output is printed out. By default, if test
> > had no errors, the only output will be a single line with test number,
> > name, and verdict at the end, e.g.:
> >
> > #31 xdp:OK
> >
> > If test had any errors, all log output captured during test execution
> > will be output after test completes.
> >
> > It's possible to force output of log with `-v` (`--verbose`) option, in
> > which case output won't be buffered and will be output immediately.
> >
> > To support this, individual tests are required to use helper methods for
> > logging: `test__printf()` and `test__vprintf()`.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > .../selftests/bpf/prog_tests/bpf_obj_id.c | 6 +-
> > .../bpf/prog_tests/bpf_verif_scale.c | 31 ++--
> > .../bpf/prog_tests/get_stack_raw_tp.c | 4 +-
> > .../selftests/bpf/prog_tests/l4lb_all.c | 2 +-
> > .../selftests/bpf/prog_tests/map_lock.c | 10 +-
> > .../selftests/bpf/prog_tests/send_signal.c | 8 +-
> > .../selftests/bpf/prog_tests/spinlock.c | 2 +-
> > .../bpf/prog_tests/stacktrace_build_id.c | 4 +-
> > .../bpf/prog_tests/stacktrace_build_id_nmi.c | 4 +-
> > .../selftests/bpf/prog_tests/xdp_noinline.c | 3 +-
> > tools/testing/selftests/bpf/test_progs.c | 135 +++++++++++++-----
> > tools/testing/selftests/bpf/test_progs.h | 37 ++++-
> > 12 files changed, 173 insertions(+), 73 deletions(-)
> >
[...]
> > error_cnt++;
> > - printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
> > + test__printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
> #define printf(...) test__printf(...) in tests.h?
>
> A bit ugly, but no need to retrain everyone to use new printf wrappers.
I try to reduce amount of magic and surprising things, not add new
ones :) I also led by example and converted all current instances of
printf usage to test__printf, so anyone new will just copy/paste good
example, hopefully. Even if not, this non-buffered output will be
immediately obvious to anyone who just runs `sudo ./test_progs`. And
author of new test with this problem should hopefully be the first and
the only one to catch and fix this.
>
> > }
> > out:
> > bpf_object__close(obj);
> > diff --git a/tools/testing/selftests/bpf/prog_tests/map_lock.c b/tools/testing/selftests/bpf/prog_tests/map_lock.c
> > index ee99368c595c..2e78217ed3fd 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/map_lock.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/map_lock.c
> > @@ -9,12 +9,12 @@ static void *parallel_map_access(void *arg)
[...]
^ permalink raw reply
* Re: [PATCH bpf-next 4/9] libbpf: add libbpf_swap_print to get previous print func
From: Andrii Nakryiko @ 2019-07-26 21:47 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
In-Reply-To: <20190726212818.GC24397@mini-arch>
On Fri, Jul 26, 2019 at 2:28 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 07/26, Andrii Nakryiko wrote:
> > libbpf_swap_print allows to restore previously set print function.
> > This is useful when running many independent test with one default print
> > function, but overriding log verbosity for particular subset of tests.
> Can we change the return type of libbpf_set_print instead and return
> the old function from it? Will it break ABI?
Yeah, thought about that, but I wasn't sure about ABI breakage. It
seems like it shouldn't, so I'll just change libbpf_set_print
signature instead.
>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > tools/lib/bpf/libbpf.c | 8 ++++++++
> > tools/lib/bpf/libbpf.h | 1 +
> > tools/lib/bpf/libbpf.map | 5 +++++
> > 3 files changed, 14 insertions(+)
> >
[...]
^ permalink raw reply
* Re: [PATCH bpf-next 3/9] selftests/bpf: add test selectors by number and name to test_progs
From: Andrii Nakryiko @ 2019-07-26 21:45 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
In-Reply-To: <20190726212547.GB24397@mini-arch>
On Fri, Jul 26, 2019 at 2:25 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 07/26, Andrii Nakryiko wrote:
> > Add ability to specify either test number or test name substring to
> > narrow down a set of test to run.
> >
> > Usage:
> > sudo ./test_progs -n 1
> > sudo ./test_progs -t attach_probe
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > tools/testing/selftests/bpf/test_progs.c | 43 +++++++++++++++++++++---
> > 1 file changed, 39 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> > index eea88ba59225..6e04b9f83777 100644
> > --- a/tools/testing/selftests/bpf/test_progs.c
> > +++ b/tools/testing/selftests/bpf/test_progs.c
> > @@ -4,6 +4,7 @@
[...]
> >
> > static error_t parse_arg(int key, char *arg, struct argp_state *state)
> > {
> > struct test_env *env = state->input;
> >
> > switch (key) {
> [..]
> > + case ARG_TEST_NUM: {
> > + int test_num;
> > +
> > + errno = 0;
> > + test_num = strtol(arg, NULL, 10);
> > + if (errno)
> > + return -errno;
> > + env->test_num_selector = test_num;
> > + break;
> > + }
> Do you think it's really useful? I agree about running by name (I
Special request from Alexei :) But in one of the follow up patches, I
extended this to allow to specify arbitrary subset of tests, e.g.:
1,2,5-10,7-8. So in that regard, it's more powerful than selecting by
name and gives you ultimate freedom.
> usually used grep -v in the Makefile :-), but I'm not sure about running
> by number.
>
> Or is the idea is that you can just copy-paste this number from the
> test_progs output to rerun the tests? In this case, why not copy-paste
> the name instead?
Both were simple to support, I didn't want to dictate one right way to
do this :)
>
> > + case ARG_TEST_NAME:
> > + env->test_name_selector = arg;
> > + break;
> > case ARG_VERIFIER_STATS:
> > env->verifier_stats = true;
> > break;
> > @@ -223,7 +248,7 @@ int main(int argc, char **argv)
> > .parser = parse_arg,
> > .doc = argp_program_doc,
> > };
> > - const struct prog_test_def *def;
> > + struct prog_test_def *test;
> > int err, i;
> >
> > err = argp_parse(&argp, argc, argv, 0, NULL, &env);
> > @@ -237,8 +262,18 @@ int main(int argc, char **argv)
> > verifier_stats = env.verifier_stats;
> >
> > for (i = 0; i < ARRAY_SIZE(prog_test_defs); i++) {
> > - def = &prog_test_defs[i];
> > - def->run_test();
> > + test = &prog_test_defs[i];
> > +
> > + test->test_num = i + 1;
> > +
> > + if (env.test_num_selector >= 0 &&
> > + test->test_num != env.test_num_selector)
> > + continue;
> > + if (env.test_name_selector &&
> > + !strstr(test->test_name, env.test_name_selector))
> > + continue;
> > +
> > + test->run_test();
> > }
> >
> > printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
> > --
> > 2.17.1
> >
^ permalink raw reply
* Re: [PATCH bpf-next 1/9] selftests/bpf: prevent headers to be compiled as C code
From: Andrii Nakryiko @ 2019-07-26 21:42 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
In-Reply-To: <20190726212152.GA24397@mini-arch>
On Fri, Jul 26, 2019 at 2:21 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 07/26, Andrii Nakryiko wrote:
> > Apprently listing header as a normal dependency for a binary output
> > makes it go through compilation as if it was C code. This currently
> > works without a problem, but in subsequent commits causes problems for
> > differently generated test.h for test_progs. Marking those headers as
> > order-only dependency solves the issue.
> Are you sure it will not result in a situation where
> test_progs/test_maps is not regenerated if tests.h is updated.
>
> If I read the following doc correctly, order deps make sense for
> directories only:
> https://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html
>
> Can you maybe double check it with:
> * make
> * add new prog_tests/test_something.c
> * make
> to see if the binary is regenerated with test_something.c?
Yeah, tested that, it triggers test_progs rebuild.
Ordering is still preserved, because test.h is dependency of
test_progs.c, which is dependency of test_progs binary, so that's why
it works.
As to why .h file is compiled as C file, I have no idea and ideally
that should be fixed somehow.
I also started with just removing header as dependency completely
(because it's indirect dependency of test_progs.c), but that broke the
build logic. Dunno, too much magic... This works, tested many-many
times, so I was satisfied enough :)
>
> Maybe fix the problem of header compilation by having '#ifndef
> DECLARE_TEST #define DECLARE_TEST() #endif' in tests.h instead?
That's ugly, I'd like to avoid doing that.
>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > tools/testing/selftests/bpf/Makefile | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 11c9c62c3362..bb66cc4a7f34 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -235,7 +235,7 @@ PROG_TESTS_H := $(PROG_TESTS_DIR)/tests.h
> > PROG_TESTS_FILES := $(wildcard prog_tests/*.c)
> > test_progs.c: $(PROG_TESTS_H)
> > $(OUTPUT)/test_progs: CFLAGS += $(TEST_PROGS_CFLAGS)
> > -$(OUTPUT)/test_progs: test_progs.c $(PROG_TESTS_H) $(PROG_TESTS_FILES)
> > +$(OUTPUT)/test_progs: test_progs.c $(PROG_TESTS_FILES) | $(PROG_TESTS_H)
> > $(PROG_TESTS_H): $(PROG_TESTS_FILES) | $(PROG_TESTS_DIR)
> > $(shell ( cd prog_tests/; \
> > echo '/* Generated header, do not edit */'; \
> > @@ -256,7 +256,7 @@ MAP_TESTS_H := $(MAP_TESTS_DIR)/tests.h
> > MAP_TESTS_FILES := $(wildcard map_tests/*.c)
> > test_maps.c: $(MAP_TESTS_H)
> > $(OUTPUT)/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS)
> > -$(OUTPUT)/test_maps: test_maps.c $(MAP_TESTS_H) $(MAP_TESTS_FILES)
> > +$(OUTPUT)/test_maps: test_maps.c $(MAP_TESTS_FILES) | $(MAP_TESTS_H)
> > $(MAP_TESTS_H): $(MAP_TESTS_FILES) | $(MAP_TESTS_DIR)
> > $(shell ( cd map_tests/; \
> > echo '/* Generated header, do not edit */'; \
> > @@ -277,7 +277,7 @@ VERIFIER_TESTS_H := $(VERIFIER_TESTS_DIR)/tests.h
> > VERIFIER_TEST_FILES := $(wildcard verifier/*.c)
> > test_verifier.c: $(VERIFIER_TESTS_H)
> > $(OUTPUT)/test_verifier: CFLAGS += $(TEST_VERIFIER_CFLAGS)
> > -$(OUTPUT)/test_verifier: test_verifier.c $(VERIFIER_TESTS_H)
> > +$(OUTPUT)/test_verifier: test_verifier.c | $(VERIFIER_TEST_FILES) $(VERIFIER_TESTS_H)
> > $(VERIFIER_TESTS_H): $(VERIFIER_TEST_FILES) | $(VERIFIER_TESTS_DIR)
> > $(shell ( cd verifier/; \
> > echo '/* Generated header, do not edit */'; \
> > --
> > 2.17.1
> >
^ permalink raw reply
* Re: [net 7/9] net/mlx5e: kTLS, Call WARN_ONCE on netdev mismatch
From: Saeed Mahameed @ 2019-07-26 21:41 UTC (permalink / raw)
To: jakub.kicinski@netronome.com
Cc: davem@davemloft.net, netdev@vger.kernel.org, Tariq Toukan
In-Reply-To: <20190725134950.74733e62@cakuba.netronome.com>
On Thu, 2019-07-25 at 13:49 -0700, Jakub Kicinski wrote:
> On Thu, 25 Jul 2019 20:36:48 +0000, Saeed Mahameed wrote:
> > From: Tariq Toukan <tariqt@mellanox.com>
> >
> > A netdev mismatch in the processed TLS SKB should not occur,
> > and indicates a kernel bug.
> > Add WARN_ONCE to spot such cases.
> >
> > Fixes: d2ead1f360e8 ("net/mlx5e: Add kTLS TX HW offload support")
> > Suggested-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > ---
> > drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git
> > a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
> > index ea032f54197e..3766545ce259 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
> > @@ -412,7 +412,7 @@ struct sk_buff *mlx5e_ktls_handle_tx_skb(struct
> > net_device *netdev,
> > goto out;
> >
> > tls_ctx = tls_get_ctx(skb->sk);
> > - if (unlikely(tls_ctx->netdev != netdev))
> > + if (unlikely(WARN_ON_ONCE(tls_ctx->netdev != netdev)))
>
> Ah, nit: the unlikely is probably unnecessary but that's no big deal.
>
> #define WARN_ON_ONCE(condition) ({ \
> static int __warned; \
> int __ret_warn_once = !!(condition); \
> \
> if (unlikely(__ret_warn_once && !__warned)) { \
> __warned = true; \
> WARN_ON(1); \
> } \
> unlikely(__ret_warn_once); \
> })
>
indeed, i see Dave already accepted this, will fix in net-next.
Thanks Jakub !
^ permalink raw reply
* Re: [PATCH] sis900: add support for ethtool's EEPROM dump
From: Saeed Mahameed @ 2019-07-26 21:39 UTC (permalink / raw)
To: venza@brownhat.org, netdev@vger.kernel.org,
sergej.benilov@googlemail.com, andrew@lunn.ch
In-Reply-To: <20190725194806.17964-1-sergej.benilov@googlemail.com>
On Thu, 2019-07-25 at 21:48 +0200, Sergej Benilov wrote:
> Implement ethtool's EEPROM dump command (ethtool -e|--eeprom-dump).
>
> Thx to Andrew Lunn for comments.
>
> Signed-off-by: Sergej Benilov <sergej.benilov@googlemail.com>
> ---
> drivers/net/ethernet/sis/sis900.c | 68
> +++++++++++++++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/drivers/net/ethernet/sis/sis900.c
> b/drivers/net/ethernet/sis/sis900.c
> index 6e07f5ebacfc..85eaccbbbac1 100644
> --- a/drivers/net/ethernet/sis/sis900.c
> +++ b/drivers/net/ethernet/sis/sis900.c
> @@ -191,6 +191,8 @@ struct sis900_private {
> unsigned int tx_full; /* The Tx queue is full. */
> u8 host_bridge_rev;
> u8 chipset_rev;
> + /* EEPROM data */
> + int eeprom_size;
> };
>
> MODULE_AUTHOR("Jim Huang <cmhuang@sis.com.tw>, Ollie Lho <
> ollie@sis.com.tw>");
> @@ -475,6 +477,8 @@ static int sis900_probe(struct pci_dev *pci_dev,
> sis_priv->pci_dev = pci_dev;
> spin_lock_init(&sis_priv->lock);
>
> + sis_priv->eeprom_size = 24;
> +
this value isn't changing across this patch,
why do you need to store a constant value in private data ?
Just make a #define ..
> pci_set_drvdata(pci_dev, net_dev);
>
> ring_space = pci_alloc_consistent(pci_dev, TX_TOTAL_SIZE,
> &ring_dma);
> @@ -2122,6 +2126,68 @@ static void sis900_get_wol(struct net_device
> *net_dev, struct ethtool_wolinfo *w
> wol->supported = (WAKE_PHY | WAKE_MAGIC);
> }
>
> +static int sis900_get_eeprom_len(struct net_device *dev)
> +{
> + struct sis900_private *sis_priv = netdev_priv(dev);
> +
> + return sis_priv->eeprom_size;
> +}
> +
> +static int sis900_read_eeprom(struct net_device *net_dev, u8 *buf)
> +{
> + struct sis900_private *sis_priv = netdev_priv(net_dev);
> + void __iomem *ioaddr = sis_priv->ioaddr;
> + int wait, ret = -EAGAIN;
> + u16 signature;
> + u16 *ebuf = (u16 *)buf;
reverse xmas tree.
> + int i;
> +
> + if (sis_priv->chipset_rev == SIS96x_900_REV) {
> + sw32(mear, EEREQ);
> + for (wait = 0; wait < 2000; wait++) {
> + if (sr32(mear) & EEGNT) {
> + /* read 16 bits, and index by 16 bits
> */
> + for (i = 0; i < sis_priv->eeprom_size /
> 2; i++)
> + ebuf[i] =
> (u16)read_eeprom(ioaddr, i);
> + ret = 0;
> + break;
> + }
> + udelay(1);
> + }
cosmetic comment, too much indentations to my taste,
two ways to avoid this,
1) if !SIS96x_900_REV execute the else statement and early return
2) "do while" to wait for (sr32(mear) & EEGNT) and then execute the for
loop which reads the eeprom outs side the wait loop.
> + sw32(mear, EEDONE);
> + } else {
> + signature = (u16)read_eeprom(ioaddr, EEPROMSignature);
> + if (signature != 0xffff && signature != 0x0000) {
> + /* read 16 bits, and index by 16 bits */
> + for (i = 0; i < sis_priv->eeprom_size / 2; i++)
> + ebuf[i] = (u16)read_eeprom(ioaddr, i);
> + ret = 0;
> + }
> + }
> + return ret;
> +}
> +
> +#define SIS900_EEPROM_MAGIC 0xBABE
> +static int sis900_get_eeprom(struct net_device *dev, struct
> ethtool_eeprom *eeprom, u8 *data)
> +{
> + struct sis900_private *sis_priv = netdev_priv(dev);
> + u8 *eebuf;
> + int res;
> +
> + eebuf = kmalloc(sis_priv->eeprom_size, GFP_KERNEL);
> + if (!eebuf)
> + return -ENOMEM;
> +
> + eeprom->magic = SIS900_EEPROM_MAGIC;
> + spin_lock_irq(&sis_priv->lock);
> + res = sis900_read_eeprom(dev, eebuf);
> + spin_unlock_irq(&sis_priv->lock);
> + if (!res)
> + memcpy(data, eebuf + eeprom->offset, eeprom->len);
> + kfree(eebuf);
> + return res;
> +}
> +
> static const struct ethtool_ops sis900_ethtool_ops = {
> .get_drvinfo = sis900_get_drvinfo,
> .get_msglevel = sis900_get_msglevel,
> @@ -2132,6 +2198,8 @@ static const struct ethtool_ops
> sis900_ethtool_ops = {
> .set_wol = sis900_set_wol,
> .get_link_ksettings = sis900_get_link_ksettings,
> .set_link_ksettings = sis900_set_link_ksettings,
> + .get_eeprom_len = sis900_get_eeprom_len,
> + .get_eeprom = sis900_get_eeprom,
> };
>
> /**
^ permalink raw reply
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