Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH bpf-next 02/10] libbpf: implement BPF CO-RE offset relocation algorithm
From: Yonghong Song @ 2019-07-27 21:29 UTC (permalink / raw)
  To: Andrii Nakryiko, Alexei Starovoitov
  Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Networking,
	Daniel Borkmann, Kernel Team
In-Reply-To: <CAEf4Bzbt4+mT8GfQG4xMj4tCnWd2ZqJiY3r8cwOankFFQo8wWA@mail.gmail.com>



On 7/27/19 11:24 AM, Andrii Nakryiko wrote:
> On Sat, Jul 27, 2019 at 10:00 AM Alexei Starovoitov <ast@fb.com> wrote:
>>
>> On 7/26/19 11:25 PM, Andrii Nakryiko wrote:
>>>>> +     } else if (class == BPF_ST && BPF_MODE(insn->code) == BPF_MEM) {
>>>>> +             if (insn->imm != orig_off)
>>>>> +                     return -EINVAL;
>>>>> +             insn->imm = new_off;
>>>>> +             pr_debug("prog '%s': patched insn #%d (ST | MEM) imm %d -> %d\n",
>>>>> +                      bpf_program__title(prog, false),
>>>>> +                      insn_idx, orig_off, new_off);
>>>> I'm pretty sure llvm was not capable of emitting BPF_ST insn.
>>>> When did that change?
>>> I just looked at possible instructions that could have 32-bit
>>> immediate value. This is `*(rX) = offsetof(struct s, field)`, which I
>>> though is conceivable. Do you think I should drop it?
>>
>> Just trying to point out that since it's not emitted by llvm
>> this code is likely untested ?
>> Or you've created a bpf asm test for this?
> 
> 
> Yeah, it's untested right now. Let me try to come up with LLVM
> assembly + relocation (not yet sure how/whether builtin works with
> inline assembly), if that works out, I'll leave this, if not, I'll
> drop BPF_ST|BPF_MEM part.

FYI. The llvm does not have assembly code format for BPF_ST instructions 
as it does not generate code for it. So inline asm through llvm won't 
work. llvm disasseembler won't be able to decode BPF_ST either.

>>
>>

^ permalink raw reply

* Re: [PATCH net] Revert ("r8169: remove 1000/Half from supported modes")
From: David Miller @ 2019-07-27 21:29 UTC (permalink / raw)
  To: hkallweit1; +Cc: nic_swsd, netdev, berny156
In-Reply-To: <56f11453-59fd-3990-7f32-52820fee238e@gmail.com>

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sat, 27 Jul 2019 12:32:28 +0200

> This reverts commit a6851c613fd7fccc5d1f28d5d8a0cbe9b0f4e8cc.
> It was reported that RTL8111b successfully finishes 1000/Full autoneg
> but no data flows. Reverting the original patch fixes the issue.
> It seems to be a HW issue with the integrated RTL8211B PHY. This PHY
> version used also e.g. on RTL8168d, so better revert the original patch.
> 
> Reported-by: Bernhard Held <berny156@gmx.de>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net] r8169: don't use MSI before RTL8168d
From: David Miller @ 2019-07-27 21:31 UTC (permalink / raw)
  To: hkallweit1; +Cc: nic_swsd, netdev, dragic.dusan
In-Reply-To: <c9f89cfc-ec16-62dc-a975-1b614941e723@gmail.com>

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sat, 27 Jul 2019 12:43:31 +0200

> It was reported that after resuming from suspend network fails with
> error "do_IRQ: 3.38 No irq handler for vector", see [0]. Enabling WoL
> can work around the issue, but the only actual fix is to disable MSI.
> So let's mimic the behavior of the vendor driver and disable MSI on
> all chip versions before RTL8168d.
> 
> [0] https://bugzilla.kernel.org/show_bug.cgi?id=204079
> 
> Fixes: 6c6aa15fdea5 ("r8169: improve interrupt handling")
> Reported-by: Dušan Dragić <dragic.dusan@gmail.com>
> Tested-by: Dušan Dragić <dragic.dusan@gmail.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> This version of the fix applies from 5.3 only. I'll submit a separate
> version for previous kernel versions.

Applied and queued up for -stable, thanks for the backport.

^ permalink raw reply

* Re: [PATCH net-next 0/3] mlxsw: spectrum_acl: Forbid unsupported filters
From: David Miller @ 2019-07-27 21:32 UTC (permalink / raw)
  To: idosch; +Cc: netdev, jiri, mlxsw, idosch
In-Reply-To: <20190727173257.6848-1-idosch@idosch.org>

From: Ido Schimmel <idosch@idosch.org>
Date: Sat, 27 Jul 2019 20:32:54 +0300

> From: Ido Schimmel <idosch@mellanox.com>
> 
> Patches #1-#2 make mlxsw reject unsupported egress filters. These
> include filters that match on VLAN and filters associated with a
> redirect action. Patch #1 rejects such filters when they are configured
> on egress and patch #2 rejects such filters when they are configured in
> a shared block that user tries to bind to egress.
> 
> Patch #3 forbids matching on reserved TCP flags as this is not supported
> by the current keys that mlxsw uses.

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH bpf-next 02/10] libbpf: implement BPF CO-RE offset relocation algorithm
From: Andrii Nakryiko @ 2019-07-27 21:36 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Alexei Starovoitov, Alexei Starovoitov, Andrii Nakryiko, bpf,
	Networking, Daniel Borkmann, Kernel Team
In-Reply-To: <363f7363-7031-3160-9f5f-583a1662fe25@fb.com>

On Sat, Jul 27, 2019 at 2:29 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 7/27/19 11:24 AM, Andrii Nakryiko wrote:
> > On Sat, Jul 27, 2019 at 10:00 AM Alexei Starovoitov <ast@fb.com> wrote:
> >>
> >> On 7/26/19 11:25 PM, Andrii Nakryiko wrote:
> >>>>> +     } else if (class == BPF_ST && BPF_MODE(insn->code) == BPF_MEM) {
> >>>>> +             if (insn->imm != orig_off)
> >>>>> +                     return -EINVAL;
> >>>>> +             insn->imm = new_off;
> >>>>> +             pr_debug("prog '%s': patched insn #%d (ST | MEM) imm %d -> %d\n",
> >>>>> +                      bpf_program__title(prog, false),
> >>>>> +                      insn_idx, orig_off, new_off);
> >>>> I'm pretty sure llvm was not capable of emitting BPF_ST insn.
> >>>> When did that change?
> >>> I just looked at possible instructions that could have 32-bit
> >>> immediate value. This is `*(rX) = offsetof(struct s, field)`, which I
> >>> though is conceivable. Do you think I should drop it?
> >>
> >> Just trying to point out that since it's not emitted by llvm
> >> this code is likely untested ?
> >> Or you've created a bpf asm test for this?
> >
> >
> > Yeah, it's untested right now. Let me try to come up with LLVM
> > assembly + relocation (not yet sure how/whether builtin works with
> > inline assembly), if that works out, I'll leave this, if not, I'll
> > drop BPF_ST|BPF_MEM part.
>
> FYI. The llvm does not have assembly code format for BPF_ST instructions
> as it does not generate code for it. So inline asm through llvm won't
> work. llvm disasseembler won't be able to decode BPF_ST either.

Well then, I'll just drop it for now. Thanks!

>
> >>
> >>

^ permalink raw reply

* Re: [PATCH] tcp: add new tcp_mtu_probe_floor sysctl
From: Josh Hunt @ 2019-07-27 23:21 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, David Miller
In-Reply-To: <CANn89iJtw_XrU-F0-frE=P6egH99kF0W0kTzReK701LmigcJ4Q@mail.gmail.com>

On 7/27/19 12:05 AM, Eric Dumazet wrote:
> On Sat, Jul 27, 2019 at 4:23 AM Josh Hunt <johunt@akamai.com> wrote:
>>
>> The current implementation of TCP MTU probing can considerably
>> underestimate the MTU on lossy connections allowing the MSS to get down to
>> 48. We have found that in almost all of these cases on our networks these
>> paths can handle much larger MTUs meaning the connections are being
>> artificially limited. Even though TCP MTU probing can raise the MSS back up
>> we have seen this not to be the case causing connections to be "stuck" with
>> an MSS of 48 when heavy loss is present.
>>
>> Prior to pushing out this change we could not keep TCP MTU probing enabled
>> b/c of the above reasons. Now with a reasonble floor set we've had it
>> enabled for the past 6 months.
> 
> And what reasonable value have you used ???

Reasonable for some may not be reasonable for others hence the new 
sysctl :) We're currently running with a fairly high value based off of 
the v6 min MTU minus headers and options, etc. We went conservative with 
our setting initially as it seemed a reasonable first step when 
re-enabling TCP MTU probing since with no configurable floor we saw a # 
of cases where connections were using severely reduced mss b/c of loss 
and not b/c of actual path restriction. I plan to reevaluate the setting 
at some point, but since the probing method is still the same it means 
the same clients who got stuck with mss of 48 before will land at 
whatever floor we set. Looking forward we are interested in trying to 
improve TCP MTU probing so it does not penalize clients like this.

A suggestion for a more reasonable floor default would be 512, which is 
the same as the min_pmtu. Given both mechanisms are trying to achieve 
the same goal it seems like they should have a similar min/floor.

> 
>>
>> The new sysctl will still default to TCP_MIN_SND_MSS (48), but gives
>> administrators the ability to control the floor of MSS probing.
>>
>> Signed-off-by: Josh Hunt <johunt@akamai.com>
>> ---
>>   Documentation/networking/ip-sysctl.txt | 6 ++++++
>>   include/net/netns/ipv4.h               | 1 +
>>   net/ipv4/sysctl_net_ipv4.c             | 9 +++++++++
>>   net/ipv4/tcp_ipv4.c                    | 1 +
>>   net/ipv4/tcp_timer.c                   | 2 +-
>>   5 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
>> index df33674799b5..49e95f438ed7 100644
>> --- a/Documentation/networking/ip-sysctl.txt
>> +++ b/Documentation/networking/ip-sysctl.txt
>> @@ -256,6 +256,12 @@ tcp_base_mss - INTEGER
>>          Path MTU discovery (MTU probing).  If MTU probing is enabled,
>>          this is the initial MSS used by the connection.
>>
>> +tcp_mtu_probe_floor - INTEGER
>> +       If MTU probing is enabled this caps the minimum MSS used for search_low
>> +       for the connection.
>> +
>> +       Default : 48
>> +
>>   tcp_min_snd_mss - INTEGER
>>          TCP SYN and SYNACK messages usually advertise an ADVMSS option,
>>          as described in RFC 1122 and RFC 6691.
>> diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
>> index bc24a8ec1ce5..c0c0791b1912 100644
>> --- a/include/net/netns/ipv4.h
>> +++ b/include/net/netns/ipv4.h
>> @@ -116,6 +116,7 @@ struct netns_ipv4 {
>>          int sysctl_tcp_l3mdev_accept;
>>   #endif
>>          int sysctl_tcp_mtu_probing;
>> +       int sysctl_tcp_mtu_probe_floor;
>>          int sysctl_tcp_base_mss;
>>          int sysctl_tcp_min_snd_mss;
>>          int sysctl_tcp_probe_threshold;
>> diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
>> index 0b980e841927..59ded25acd04 100644
>> --- a/net/ipv4/sysctl_net_ipv4.c
>> +++ b/net/ipv4/sysctl_net_ipv4.c
>> @@ -820,6 +820,15 @@ static struct ctl_table ipv4_net_table[] = {
>>                  .extra2         = &tcp_min_snd_mss_max,
>>          },
>>          {
>> +               .procname       = "tcp_mtu_probe_floor",
>> +               .data           = &init_net.ipv4.sysctl_tcp_mtu_probe_floor,
>> +               .maxlen         = sizeof(int),
>> +               .mode           = 0644,
>> +               .proc_handler   = proc_dointvec_minmax,
>> +               .extra1         = &tcp_min_snd_mss_min,
>> +               .extra2         = &tcp_min_snd_mss_max,
>> +       },
>> +       {
>>                  .procname       = "tcp_probe_threshold",
>>                  .data           = &init_net.ipv4.sysctl_tcp_probe_threshold,
>>                  .maxlen         = sizeof(int),
>> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
>> index d57641cb3477..e0a372676329 100644
>> --- a/net/ipv4/tcp_ipv4.c
>> +++ b/net/ipv4/tcp_ipv4.c
>> @@ -2637,6 +2637,7 @@ static int __net_init tcp_sk_init(struct net *net)
>>          net->ipv4.sysctl_tcp_min_snd_mss = TCP_MIN_SND_MSS;
>>          net->ipv4.sysctl_tcp_probe_threshold = TCP_PROBE_THRESHOLD;
>>          net->ipv4.sysctl_tcp_probe_interval = TCP_PROBE_INTERVAL;
>> +       net->ipv4.sysctl_tcp_mtu_probe_floor = TCP_MIN_SND_MSS;
>>
>>          net->ipv4.sysctl_tcp_keepalive_time = TCP_KEEPALIVE_TIME;
>>          net->ipv4.sysctl_tcp_keepalive_probes = TCP_KEEPALIVE_PROBES;
>> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
>> index c801cd37cc2a..dbd9d2d0ee63 100644
>> --- a/net/ipv4/tcp_timer.c
>> +++ b/net/ipv4/tcp_timer.c
>> @@ -154,7 +154,7 @@ static void tcp_mtu_probing(struct inet_connection_sock *icsk, struct sock *sk)
>>          } else {
>>                  mss = tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_low) >> 1;
>>                  mss = min(net->ipv4.sysctl_tcp_base_mss, mss);
>> -               mss = max(mss, 68 - tcp_sk(sk)->tcp_header_len);
>> +               mss = max(mss, net->ipv4.sysctl_tcp_mtu_probe_floor);
>>                  mss = max(mss, net->ipv4.sysctl_tcp_min_snd_mss);
>>                  icsk->icsk_mtup.search_low = tcp_mss_to_mtu(sk, mss);
>>          }
> 
> 
> Existing sysctl should be enough ?

I don't think so. Changing tcp_min_snd_mss could impact clients that 
really want/need a small mss. When you added the new sysctl I tried to 
analyze the mss values we're seeing to understand what we could possibly 
raise it to. While not a huge amount, we see more clients than I 
expected announcing mss values in the 180-512 range. Given that I would 
not feel comfortable setting tcp_min_snd_mss to say 512 as I suggested 
above.

> 
> tcp_min_snd_mss  documentation could be slightly updated.
> 
> And maybe its default value could be raised a bit.
> 

Thanks
Josh

^ permalink raw reply

* [PATCH] rocker: fix memory leaks of fib_work on two error return paths
From: Colin King @ 2019-07-27 23:37 UTC (permalink / raw)
  To: David Ahern, Jiri Pirko, David S . Miller, netdev
  Cc: kernel-janitors, linux-kernel

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

Currently there are two error return paths that leak memory allocated
to fib_work. Fix this by kfree'ing fib_work before returning.

Addresses-Coverity: ("Resource leak")
Fixes: 19a9d136f198 ("ipv4: Flag fib_info with a fib_nh using IPv6 gateway")
Fixes: dbcc4fa718ee ("rocker: Fail attempts to use routes with nexthop objects")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/ethernet/rocker/rocker_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index 079f459c73a5..2c5d3f5b84dd 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -2208,10 +2208,12 @@ static int rocker_router_fib_event(struct notifier_block *nb,
 
 			if (fen_info->fi->fib_nh_is_v6) {
 				NL_SET_ERR_MSG_MOD(info->extack, "IPv6 gateway with IPv4 route is not supported");
+				kfree(fib_work);
 				return notifier_from_errno(-EINVAL);
 			}
 			if (fen_info->fi->nh) {
 				NL_SET_ERR_MSG_MOD(info->extack, "IPv4 route with nexthop objects is not supported");
+				kfree(fib_work);
 				return notifier_from_errno(-EINVAL);
 			}
 		}
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH bpf-next 02/10] libbpf: implement BPF CO-RE offset relocation algorithm
From: Song Liu @ 2019-07-28  0:24 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Yonghong Song, Kernel Team
In-Reply-To: <CAEf4BzbwzPsPonkqvGwS-FOWtWYQHQP=PwdVuVEkuEevrUKHWQ@mail.gmail.com>



> On Jul 27, 2019, at 12:09 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> 
> On Sat, Jul 27, 2019 at 11:59 AM Song Liu <songliubraving@fb.com> wrote:
>> 
>> 
>> 
>>> On Jul 26, 2019, at 11:11 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>>> 
>>> On Thu, Jul 25, 2019 at 12:32 PM Song Liu <songliubraving@fb.com> wrote:
>>>> 
>>>> 
>>>> 
>>>>> On Jul 24, 2019, at 12:27 PM, Andrii Nakryiko <andriin@fb.com> wrote:
>>>>> 
>>>>> This patch implements the core logic for BPF CO-RE offsets relocations.
>>>>> All the details are described in code comments.
>>>> 
>>>> Some description in the change log is still useful. Please at least
>>>> copy-paste key comments here.
>>> 
>>> OK, will add some more.
>>> 
>>>> 
>>>> And, this is looooong. I think it is totally possible to split it into
>>>> multiple smaller patches.
>>> 
>>> I don't really know how to split it further without hurting reviewing
>>> by artificially splitting related code into separate patches. Remove
>>> any single function and algorithm will be incomplete.
>>> 
>>> Let me give you some high-level overview of how pieces are put
>>> together. There are 9 non-trivial functions, let's go over their
>>> purpose in the orderd in which they are defined in file:
>>> 
>>> 1. bpf_core_spec_parse()
>>> 
>>> This one take bpf_offset_reloc's type_id and accessor string
>>> ("0:1:2:3") and parses it into more convenient bpf_core_spec
>>> datastructure, which has calculated offset and high-level spec
>>> "steps": either named field or array access.
>>> 
>>> 2. bpf_core_find_cands()
>>> 
>>> Given local type name, finds all possible target BTF types with same
>>> name (modulo "flavor" differences, ___flavor suffix is just ignored).
>>> 
>>> 3. bpf_core_fields_are_compat()
>>> 
>>> Given local and target field match, checks that their types are
>>> compatible (so that we don't accidentally match, e.g., int against
>>> struct).
>>> 
>>> 4. bpf_core_match_member()
>>> 
>>> Given named local field, find corresponding field in target struct. To
>>> understand why it's not trivial, here's an example:
>>> 
>>> Local type:
>>> 
>>> struct s___local {
>>> int a;
>>> };
>>> 
>>> Target type:
>>> 
>>> struct s___target {
>>> struct {
>>>   union {
>>>     int a;
>>>   };
>>> };
>>> };
>>> 
>>> For both cases you can access a as s.a, but in local case, field a is
>>> immediately inside s___local, while for s___target, you have to
>>> traverse two levels deeper into anonymous fields to get to an `a`
>>> inside anonymous union.
>>> 
>>> So this function find that `a` by doing exhaustive search across all
>>> named field and anonymous struct/unions. But otherwise it's pretty
>>> straightforward recursive function.
>>> 
>>> bpf_core_spec_match()
>>> 
>>> Just goes over high-level spec steps in local spec and tries to figure
>>> out both high-level and low-level steps for targe type. Consider the
>>> above example. For both structs accessing s.a is one high-level step,
>>> but for s___local it's single low-level step (just another :0 in spec
>>> string), while for s___target it's three low-level steps: ":0:0:0",
>>> one step for each BTF type we need to traverse.
>>> 
>>> Array access is simpler, it's always one high-level and one low-level step.
>>> 
>>> bpf_core_reloc_insn()
>>> 
>>> Once we match local and target specs and have local and target
>>> offsets, do the relocations - check that instruction has expected
>>> local offset and replace it with target offset.
>>> 
>>> bpf_core_find_kernel_btf()
>>> 
>>> This is the only function that can be moved into separate patch, but
>>> it's also very simple. It just iterates over few known possible
>>> locations for vmlinux image and once found, tries to parse .BTF out of
>>> it, to be used as target BTF.
>>> 
>>> bpf_core_reloc_offset()
>>> 
>>> It combines all the above functions to perform single relocation.
>>> Parse spec, get candidates, for each candidate try to find matching
>>> target spec. All candidates that matched are cached for given local
>>> root type.
>> 
>> Thanks for these explanation. They are really helpful.
>> 
>> I think some example explaining each step of bpf_core_reloc_offset()
>> will be very helpful. Something like:
>> 
>> Example:
>> 
>> struct s {
>>        int a;
>>        struct {
>>                int b;
>>                bool c;
>>        };
>> };
>> 
>> To get offset for c, we do:
>> 
>> bpf_core_reloc_offset() {
>> 
>>        /* input data: xxx */
>> 
>>        /* first step: bpf_core_spec_parse() */
>> 
>>        /* data after first step */
>> 
>>        /* second step: bpf_core_find_cands() */
>> 
>>        /* candidate A and B after second step */
>> 
>>        ...
>> }
>> 
>> Well, it requires quite some work to document this way. Please let me
>> know if you feel this is an overkill.
> 
> Yeah :) And it's not just work, but I think it's bad if comments
> become too specific and document very low-level steps, because code
> might evolve and comments can quickly get out of sync and just add to
> confusion. Which is why I tried to document high-level ideas, leaving
> it up to the source code to be the ultimate reference of minutia
> details.

Fair enough. 

> 
>> 
>>> 
>>> bpf_core_reloc_offsets()
>>> 
>>> High-level coordination. Iterate over all per-program .BTF.ext offset
>>> reloc sections, each relocation within them. Find corresponding
>>> program and try to apply relocations one by one.
>>> 
>>> 
>>> I think the only non-obvious part here is to understand that
>>> relocation records local raw spec with every single anonymous type
>>> traversal, which is not that useful when we try to match it against
>>> target type, which can have very different composition, but still the
>>> same field access pattern, from C language standpoint (which hides all
>>> those anonymous type traversals from programmer).
>>> 
>>> But it should be pretty clear now, plus also check tests, they have
>>> lots of cases showing what's compatible and what's not.
>> 
>> I see. I will review the tests.
>> 
>>>>> 
>>>>> static const struct btf_type *skip_mods_and_typedefs(const struct btf *btf,
>>>>> -                                                  __u32 id)
>>>>> +                                                  __u32 id,
>>>>> +                                                  __u32 *res_id)
>>>>> {
>>>>>     const struct btf_type *t = btf__type_by_id(btf, id);
>>>> 
>>>> Maybe have a local "__u32 rid;"
>>>> 
>>>>> 
>>>>> +     if (res_id)
>>>>> +             *res_id = id;
>>>>> +
>>>> 
>>>> and do "rid = id;" here
>>>> 
>>>>>     while (true) {
>>>>>             switch (BTF_INFO_KIND(t->info)) {
>>>>>             case BTF_KIND_VOLATILE:
>>>>>             case BTF_KIND_CONST:
>>>>>             case BTF_KIND_RESTRICT:
>>>>>             case BTF_KIND_TYPEDEF:
>>>>> +                     if (res_id)
>>>>> +                             *res_id = t->type;
>>>> and here
>>>> 
>>>>>                     t = btf__type_by_id(btf, t->type);
>>>>>                     break;
>>>>>             default:
>>>> and "*res_id = rid;" right before return?
>>> 
>>> Sure, but why?
>> 
>> I think it is cleaner that way. But feel free to ignore if you
>> think otherwise.
>> 
>>> 
>>>> 
>>>>> @@ -1041,7 +1049,7 @@ static const struct btf_type *skip_mods_and_typedefs(const struct btf *btf,
>>>>> static bool get_map_field_int(const char *map_name, const struct btf *btf,
>>>>>                           const struct btf_type *def,
>>>>>                           const struct btf_member *m, __u32 *res) {
>>> 
>>> [...]
>>> 
>>>>> +struct bpf_core_spec {
>>>>> +     const struct btf *btf;
>>>>> +     /* high-level spec: named fields and array indicies only */
>>>> 
>>>> typo: indices
>>> 
>>> thanks!
>>> 
>>>> 
>>>>> +     struct bpf_core_accessor spec[BPF_CORE_SPEC_MAX_LEN];
>>>>> +     /* high-level spec length */
>>>>> +     int len;
>>>>> +     /* raw, low-level spec: 1-to-1 with accessor spec string */
>>>>> +     int raw_spec[BPF_CORE_SPEC_MAX_LEN];
>>>>> +     /* raw spec length */
>>>>> +     int raw_len;
>>>>> +     /* field byte offset represented by spec */
>>>>> +     __u32 offset;
>>>>> +};
>>> 
>>> [...]
>>> 
>>>>> + *
>>>>> + *   int x = &s->a[3]; // access string = '0:1:2:3'
>>>>> + *
>>>>> + * Low-level spec has 1:1 mapping with each element of access string (it's
>>>>> + * just a parsed access string representation): [0, 1, 2, 3].
>>>>> + *
>>>>> + * High-level spec will capture only 3 points:
>>>>> + *   - intial zero-index access by pointer (&s->... is the same as &s[0]...);
>>>>> + *   - field 'a' access (corresponds to '2' in low-level spec);
>>>>> + *   - array element #3 access (corresponds to '3' in low-level spec).
>>>>> + *
>>>>> + */
>>>> 
>>>> IIUC, high-level points are subset of low-level points. How about we introduce
>>>> "anonymous" high-level points, so that high-level points and low-level points
>>>> are 1:1 mapping?
>>> 
>>> No, that will just hurt and complicate things. See above explanation
>>> about why we need high-level points (it's what you as C programmer try
>>> to achieve vs low-level spec is what C-language does in reality, with
>>> all the anonymous struct/union traversal).
>>> 
>>> What's wrong with this separation? Think about it as recording
>>> "intent" (high-level spec) vs "mechanics" (low-level spec, how exactly
>>> to achieve that intent, in excruciating details).
>> 
>> There is nothing wrong with separation. I just personally think it is
>> cleaner the other way. That's why I raised the question.
>> 
>> I will go with your assessment, as you looked into this much more than
>> I did. :-)
> 
> For me it's a machine view of the problem (raw spec) vs human view of
> the problem (high-level spec, which resembles how you think about this
> in C code). I'll keep it separate unless it proves to be problematic
> going forward.

[...]

>>> 
>>> No. spec->offset is carefully maintained across multiple low-level
>>> steps, as we traverse down embedded structs/unions.
>>> 
>>> Think about, e.g.:
>>> 
>>> struct s {
>>>   int a;
>>>   struct {
>>>       int b;
>>>   };
>>> };
>>> 
>>> Imagine you are trying to match s.b access. With what you propose
>>> you'll end up with offset 0, but it should be 4.
>> 
>> Hmm... this is just for i == 0, right? Which line updated spec->offset
>> after "memset(spec, 0, sizeof(*spec));"?
> 
> Ah, I missed that you are referring to the special i == 0 case. I can
> do assignment, yes, you are right. I'll probably also extract it out
> of the loop to make it less confusing.

Yes, please. 

> 
>>>>> +                     continue;
>>>>> +             }
>>>> 
>>>> Maybe pull i == 0 case out of the for loop?

As I mentioned earlier. ;-)

>>>> 
>>>>> +
>>>>> +             if (btf_is_composite(t)) {
>>> 
>>> [...]
>>> 
>>>>> +
>>>>> +     if (spec->len == 0)
>>>>> +             return -EINVAL;
>>>> 
>>>> Can this ever happen?
>>> 
>>> Not really, because I already check raw_len == 0 and exit with error.
>>> I'll remove.
>>> 
>>>> 
>>>>> +
>>>>> +     return 0;
>>>>> +}
>>>>> +
>>> 

[...]


^ permalink raw reply

* Re: [PATCH] rocker: fix memory leaks of fib_work on two error return paths
From: David Ahern @ 2019-07-28  0:32 UTC (permalink / raw)
  To: Colin King, Jiri Pirko, David S . Miller, netdev
  Cc: kernel-janitors, linux-kernel
In-Reply-To: <20190727233726.3121-1-colin.king@canonical.com>

On 7/27/19 5:37 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Currently there are two error return paths that leak memory allocated
> to fib_work. Fix this by kfree'ing fib_work before returning.
> 
> Addresses-Coverity: ("Resource leak")
> Fixes: 19a9d136f198 ("ipv4: Flag fib_info with a fib_nh using IPv6 gateway")
> Fixes: dbcc4fa718ee ("rocker: Fail attempts to use routes with nexthop objects")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/net/ethernet/rocker/rocker_main.c | 2 ++
>  1 file changed, 2 insertions(+)
> 

Thanks for the patch,
Reviewed-by: David Ahern <dsahern@gmail.com>

^ permalink raw reply

* [PATCH net v2] mvpp2: refactor MTU change code
From: Matteo Croce @ 2019-07-28  0:46 UTC (permalink / raw)
  To: netdev
  Cc: Antoine Tenart, Maxime Chevallier, Marcin Wojtas, Stefan Chulski,
	LKML, David Miller

The MTU change code can call napi_disable() with the device already down,
leading to a deadlock. Also, lot of code is duplicated unnecessarily.

Rework mvpp2_change_mtu() to avoid the deadlock and remove duplicated code.

Fixes: 3f518509dedc ("ethernet: Add new driver for Marvell Armada 375 network unit")
Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 41 ++++++-------------
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index b6591ea0c6d6..68fa2d563f0d 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -3700,6 +3700,7 @@ static int mvpp2_set_mac_address(struct net_device *dev, void *p)
 static int mvpp2_change_mtu(struct net_device *dev, int mtu)
 {
 	struct mvpp2_port *port = netdev_priv(dev);
+	bool running = netif_running(dev);
 	int err;
 
 	if (!IS_ALIGNED(MVPP2_RX_PKT_SIZE(mtu), 8)) {
@@ -3708,40 +3709,24 @@ static int mvpp2_change_mtu(struct net_device *dev, int mtu)
 		mtu = ALIGN(MVPP2_RX_PKT_SIZE(mtu), 8);
 	}
 
-	if (!netif_running(dev)) {
-		err = mvpp2_bm_update_mtu(dev, mtu);
-		if (!err) {
-			port->pkt_size =  MVPP2_RX_PKT_SIZE(mtu);
-			return 0;
-		}
-
-		/* Reconfigure BM to the original MTU */
-		err = mvpp2_bm_update_mtu(dev, dev->mtu);
-		if (err)
-			goto log_error;
-	}
-
-	mvpp2_stop_dev(port);
+	if (running)
+		mvpp2_stop_dev(port);
 
 	err = mvpp2_bm_update_mtu(dev, mtu);
-	if (!err) {
+	if (err) {
+		netdev_err(dev, "failed to change MTU\n");
+		/* Reconfigure BM to the original MTU */
+		mvpp2_bm_update_mtu(dev, dev->mtu);
+	} else {
 		port->pkt_size =  MVPP2_RX_PKT_SIZE(mtu);
-		goto out_start;
 	}
 
-	/* Reconfigure BM to the original MTU */
-	err = mvpp2_bm_update_mtu(dev, dev->mtu);
-	if (err)
-		goto log_error;
-
-out_start:
-	mvpp2_start_dev(port);
-	mvpp2_egress_enable(port);
-	mvpp2_ingress_enable(port);
+	if (running) {
+		mvpp2_start_dev(port);
+		mvpp2_egress_enable(port);
+		mvpp2_ingress_enable(port);
+	}
 
-	return 0;
-log_error:
-	netdev_err(dev, "failed to change MTU\n");
 	return err;
 }
 
-- 
2.21.0


^ permalink raw reply related

* [PATCH v3] net: dsa: qca8k: enable port flow control
From: xiaofeis @ 2019-07-28  0:57 UTC (permalink / raw)
  To: davem
  Cc: vkoul, netdev, andrew, linux-arm-msm, bjorn.andersson,
	vivien.didelot, f.fainelli, niklas.cassel, xiazha, xiaofeis

Set phy device advertising to enable MAC flow control.

Signed-off-by: Xiaofei Shen <xiaofeis@codeaurora.org>
---
Changes since V2:
 drivers/net/dsa/qca8k.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 232e8cc..e429e92 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -2,7 +2,7 @@
 /*
  * Copyright (C) 2009 Felix Fietkau <nbd@nbd.name>
  * Copyright (C) 2011-2012 Gabor Juhos <juhosg@openwrt.org>
- * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2015, 2019, The Linux Foundation. All rights reserved.
  * Copyright (c) 2016 John Crispin <john@phrozen.org>
  */
 
@@ -935,6 +935,8 @@
 	qca8k_port_set_status(priv, port, 1);
 	priv->port_sts[port].enabled = 1;
 
+	phy_support_asym_pause(phy);
+
 	return 0;
 }
 
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH v2 bpf-next 0/9] Revamp test_progs as a test running framework
From: Alexei Starovoitov @ 2019-07-28  1:12 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf, Network Development, Alexei Starovoitov, Daniel Borkmann,
	Stanislav Fomichev, Andrii Nakryiko, Kernel Team
In-Reply-To: <20190727190150.649137-1-andriin@fb.com>

On Sat, Jul 27, 2019 at 12:02 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> This patch set makes a number of changes to test_progs selftest, which is
> a collection of many other tests (and sometimes sub-tests as well), to provide
> better testing experience and allow to start convering many individual test
> programs under selftests/bpf into a single and convenient test runner.

I really like the patches, but something isn't right:
#16 raw_tp_writable_reject_nbd_invalid:OK
#17 raw_tp_writable_test_run:OK
#18 reference_tracking:OK
[   87.715996] test_progs[2200]: segfault at 2f ip 00007f56aeea347b sp
00007ffce9720980 error 4 in libc-2.23.so[7f56aee5b000+198000]
[   87.717316] Code: ff ff 44 89 8d 30 fb ff ff e8 01 74 fd ff 44 8b
8d 30 fb ff ff 4c 8b 85 28 fb ff ff e9 fd eb ff ff 31 c0 48 83 c9 ff
4c 89 df <f2> ae c7 85 28 fb ff ff 00 00 00 00 48 89 c8 48 f7 d0 4c 8f
[   87.719493] audit: type=1701 audit(1564276195.971:5): auid=0 uid=0
gid=0 ses=1 subj=kernel pid=2200 comm="test_progs"
exe="/data/users/ast/net-next/tools/testing/selftests/bpf/test_progs"
sig=11 res=1
Segmentation fault (core dumped)

Under gdb fault is different:
#23 stacktrace_build_id:OK
Detaching after fork from child process 2276.
Detaching after fork from child process 2278.
[  149.013116] perf: interrupt took too long (6799 > 6713), lowering
kernel.perf_event_max_sample_rate to 29000
[  149.014634] perf: interrupt took too long (8511 > 8498), lowering
kernel.perf_event_max_sample_rate to 23000
[  149.017038] perf: interrupt took too long (10649 > 10638), lowering
kernel.perf_event_max_sample_rate to 18000
[  149.021901] perf: interrupt took too long (13322 > 13311), lowering
kernel.perf_event_max_sample_rate to 15000
[  149.042946] perf: interrupt took too long (16660 > 16652), lowering
kernel.perf_event_max_sample_rate to 12000
Detaching after fork from child process 2279.
#24 stacktrace_build_id_nmi:OK
#25 stacktrace_map:OK
#26 stacktrace_map_raw_tp:OK

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6
(gdb) bt
#0  0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6
#1  0x00007ffff72655a9 in vsnprintf () from /usr/lib/libc.so.6
#2  0x0000000000403100 in test__vprintf (fmt=0x426754 "%s:PASS:%s %d
nsec\n", args=0x7fffffffe878) at test_progs.c:114
#3  0x000000000040325c in test__printf (fmt=fmt@entry=0x426754
"%s:PASS:%s %d nsec\n") at test_progs.c:147
#4  0x000000000042222d in test_task_fd_query_rawtp () at
prog_tests/task_fd_query_rawtp.c:19
#5  0x0000000000402c76 in main (argc=<optimized out>, argv=<optimized
out>) at test_progs.c:501
(gdb) info threads
  Id   Target Id         Frame
* 1    Thread 0x7ffff7fea700 (LWP 2245) "test_progs"
0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6

^ permalink raw reply

* Re: [PATCH net-next] mvpp2: document HW checksum behaviour
From: Matteo Croce @ 2019-07-28  1:36 UTC (permalink / raw)
  To: Antoine Tenart, Marcin Wojtas, Stefan Chulski, Maxime Chevallier
  Cc: netdev, LKML, David S . Miller
In-Reply-To: <20190726125715.GB5031@kwain>

On Fri, Jul 26, 2019 at 2:57 PM Antoine Tenart
<antoine.tenart@bootlin.com> wrote:
>
> Hi Matteo,
>
> On Fri, Jul 26, 2019 at 01:15:46AM +0200, Matteo Croce wrote:
> > The hardware can only offload checksum calculation on first port due to
> > the Tx FIFO size limitation. Document this in a comment.
> >
> > Fixes: 576193f2d579 ("net: mvpp2: jumbo frames support")
> > Signed-off-by: Matteo Croce <mcroce@redhat.com>
>
> Looks good. Please note there's a similar code path in the probe. You
> could also add a comment there (or move this check/comment in a common
> place).
>
> Thanks!
> Antoine
>

Hi Antoine,

I was making a v2, when I looked at the mvpp2_port_probe() which does:

--------------------------------%<------------------------------
features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_TSO;

if (port->pool_long->id == MVPP2_BM_JUMBO && port->id != 0) {
    dev->features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM);
    dev->hw_features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM);
}

dev->vlan_features |= features;
-------------------------------->%------------------------------

Is it ok to remove NETIF_F_IP*_CSUM from dev->features and
dev->hw_features but keep it in dev->vlan_features?

Regards,
-- 
Matteo Croce
per aspera ad upstream

^ permalink raw reply

* RE: [PATCH net-next v2 1/2] qed: Add API for configuring NVM attributes.
From: Sudarsana Reddy Kalluru @ 2019-07-28  1:39 UTC (permalink / raw)
  To: David Miller; +Cc: netdev@vger.kernel.org, Michal Kalderon, Ariel Elior
In-Reply-To: <20190727.140029.1705855810720310694.davem@davemloft.net>


> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of David Miller
> Sent: Sunday, July 28, 2019 2:30 AM
> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Cc: netdev@vger.kernel.org; Michal Kalderon <mkalderon@marvell.com>;
> Ariel Elior <aelior@marvell.com>
> Subject: Re: [PATCH net-next v2 1/2] qed: Add API for configuring NVM
> attributes.
> 
> From: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Date: Fri, 26 Jul 2019 08:52:14 -0700
> 
> > +int qed_mcp_nvm_set_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt
> *p_ptt,
> > +			u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
> > +			u32 len)
> > +{
> > +	u32 mb_param = 0, resp, param;
> > +	int rc;
>  ...
> > +	rc = qed_mcp_nvm_wr_cmd(p_hwfn, p_ptt,
> > +				DRV_MSG_CODE_SET_NVM_CFG_OPTION,
> > +				mb_param, &resp, &param, len, (u32
> *)p_buf);
> > +
> > +	return rc;
> 
> 'rc' is completely unnecessary, please just return the function result directly.
> 
> Thank you.

Thanks for your comments. Will send the updated patch.

^ permalink raw reply

* [PATCH net-next v3 0/2] qed*: Support for NVM config attributes.
From: Sudarsana Reddy Kalluru @ 2019-07-28  1:55 UTC (permalink / raw)
  To: davem; +Cc: netdev, mkalderon, aelior

The patch series adds support for managing the NVM config attributes.
Patch (1) adds functionality to update config attributes via MFW.
Patch (2) adds driver interface for updating the config attributes.

Changes from previous versions:
-------------------------------
v3: Removed unused variable.
v2: Removed unused API.

Please consider applying this series to "net-next".

Sudarsana Reddy Kalluru (2):
  qed: Add API for configuring NVM attributes.
  qed: Add driver API for flashing the config attributes.

 drivers/net/ethernet/qlogic/qed/qed_hsi.h  | 17 ++++++++
 drivers/net/ethernet/qlogic/qed/qed_main.c | 65 ++++++++++++++++++++++++++++++
 drivers/net/ethernet/qlogic/qed/qed_mcp.c  | 32 +++++++++++++++
 drivers/net/ethernet/qlogic/qed/qed_mcp.h  | 20 +++++++++
 include/linux/qed/qed_if.h                 |  1 +
 5 files changed, 135 insertions(+)

-- 
1.8.3.1


^ permalink raw reply

* [PATCH net-next v3 1/2] qed: Add API for configuring NVM attributes.
From: Sudarsana Reddy Kalluru @ 2019-07-28  1:55 UTC (permalink / raw)
  To: davem; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190728015549.27051-1-skalluru@marvell.com>

The patch adds API for configuring the NVM config attributes using
Management FW (MFW) interfaces.

Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/qlogic/qed/qed_hsi.h | 17 ++++++++++++++++
 drivers/net/ethernet/qlogic/qed/qed_mcp.c | 32 +++++++++++++++++++++++++++++++
 drivers/net/ethernet/qlogic/qed/qed_mcp.h | 20 +++++++++++++++++++
 3 files changed, 69 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_hsi.h b/drivers/net/ethernet/qlogic/qed/qed_hsi.h
index e054f6c..557a12e 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_hsi.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_hsi.h
@@ -12580,6 +12580,8 @@ struct public_drv_mb {
 #define DRV_MSG_CODE_BW_UPDATE_ACK		0x32000000
 #define DRV_MSG_CODE_NIG_DRAIN			0x30000000
 #define DRV_MSG_CODE_S_TAG_UPDATE_ACK		0x3b000000
+#define DRV_MSG_CODE_GET_NVM_CFG_OPTION		0x003e0000
+#define DRV_MSG_CODE_SET_NVM_CFG_OPTION		0x003f0000
 #define DRV_MSG_CODE_INITIATE_PF_FLR            0x02010000
 #define DRV_MSG_CODE_VF_DISABLED_DONE		0xc0000000
 #define DRV_MSG_CODE_CFG_VF_MSIX		0xc0010000
@@ -12748,6 +12750,21 @@ struct public_drv_mb {
 #define DRV_MB_PARAM_FEATURE_SUPPORT_PORT_EEE		0x00000002
 #define DRV_MB_PARAM_FEATURE_SUPPORT_FUNC_VLINK		0x00010000
 
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ID_SHIFT		0
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ID_MASK		0x0000FFFF
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ALL_SHIFT		16
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ALL_MASK		0x00010000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_INIT_SHIFT		17
+#define DRV_MB_PARAM_NVM_CFG_OPTION_INIT_MASK		0x00020000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_COMMIT_SHIFT	18
+#define DRV_MB_PARAM_NVM_CFG_OPTION_COMMIT_MASK		0x00040000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_FREE_SHIFT		19
+#define DRV_MB_PARAM_NVM_CFG_OPTION_FREE_MASK		0x00080000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_SEL_SHIFT	20
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_SEL_MASK	0x00100000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_ID_SHIFT	24
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_ID_MASK	0x0f000000
+
 	u32 fw_mb_header;
 #define FW_MSG_CODE_MASK			0xffff0000
 #define FW_MSG_CODE_UNSUPPORTED                 0x00000000
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
index 758702c..89462c4 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
@@ -3750,3 +3750,35 @@ int qed_mcp_get_ppfid_bitmap(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
 
 	return 0;
 }
+
+int qed_mcp_nvm_set_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+			u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
+			u32 len)
+{
+	u32 mb_param = 0, resp, param;
+
+	QED_MFW_SET_FIELD(mb_param, DRV_MB_PARAM_NVM_CFG_OPTION_ID, option_id);
+	if (flags & QED_NVM_CFG_OPTION_ALL)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_ALL, 1);
+	if (flags & QED_NVM_CFG_OPTION_INIT)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_INIT, 1);
+	if (flags & QED_NVM_CFG_OPTION_COMMIT)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_COMMIT, 1);
+	if (flags & QED_NVM_CFG_OPTION_FREE)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_FREE, 1);
+	if (flags & QED_NVM_CFG_OPTION_ENTITY_SEL) {
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_SEL, 1);
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_ID,
+				  entity_id);
+	}
+
+	return qed_mcp_nvm_wr_cmd(p_hwfn, p_ptt,
+				  DRV_MSG_CODE_SET_NVM_CFG_OPTION,
+				  mb_param, &resp, &param, len, (u32 *)p_buf);
+}
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.h b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
index e4f8fe4..83649a8 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
@@ -251,6 +251,12 @@ struct qed_mfw_tlv_generic {
 	struct qed_mfw_tlv_iscsi iscsi;
 };
 
+#define QED_NVM_CFG_OPTION_ALL		BIT(0)
+#define QED_NVM_CFG_OPTION_INIT		BIT(1)
+#define QED_NVM_CFG_OPTION_COMMIT       BIT(2)
+#define QED_NVM_CFG_OPTION_FREE		BIT(3)
+#define QED_NVM_CFG_OPTION_ENTITY_SEL	BIT(4)
+
 /**
  * @brief - returns the link params of the hw function
  *
@@ -1202,4 +1208,18 @@ void qed_mcp_resc_lock_default_init(struct qed_resc_lock_params *p_lock,
  */
 int qed_mcp_get_ppfid_bitmap(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt);
 
+/**
+ * @brief Set NVM config attribute value.
+ *
+ * @param p_hwfn
+ * @param p_ptt
+ * @param option_id
+ * @param entity_id
+ * @param flags
+ * @param p_buf
+ * @param len
+ */
+int qed_mcp_nvm_set_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+			u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
+			u32 len);
 #endif
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH net-next v3 2/2] qed: Add driver API for flashing the config attributes.
From: Sudarsana Reddy Kalluru @ 2019-07-28  1:55 UTC (permalink / raw)
  To: davem; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190728015549.27051-1-skalluru@marvell.com>

The patch adds driver interface for reading the NVM config request and
update the attributes on nvm config flash partition.
This API can be used by ethtool flash update command (i.e., ethtool -f) to
update config attributes in the NVM flash parition.

Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/qlogic/qed/qed_main.c | 65 ++++++++++++++++++++++++++++++
 include/linux/qed/qed_if.h                 |  1 +
 2 files changed, 66 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 829dd60..54f00d2 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -67,6 +67,8 @@
 #define QED_ROCE_QPS			(8192)
 #define QED_ROCE_DPIS			(8)
 #define QED_RDMA_SRQS                   QED_ROCE_QPS
+#define QED_NVM_CFG_SET_FLAGS		0xE
+#define QED_NVM_CFG_SET_PF_FLAGS	0x1E
 
 static char version[] =
 	"QLogic FastLinQ 4xxxx Core Module qed " DRV_MODULE_VERSION "\n";
@@ -2227,6 +2229,66 @@ static int qed_nvm_flash_image_validate(struct qed_dev *cdev,
 	return 0;
 }
 
+/* Binary file format -
+ *     /----------------------------------------------------------------------\
+ * 0B  |                       0x5 [command index]                            |
+ * 4B  | Entity ID     | Reserved        |  Number of config attributes       |
+ * 8B  | Config ID                       | Length        | Value              |
+ *     |                                                                      |
+ *     \----------------------------------------------------------------------/
+ * There can be several Cfg_id-Length-Value sets as specified by 'Number of...'.
+ * Entity ID - A non zero entity value for which the config need to be updated.
+ */
+static int qed_nvm_flash_cfg_write(struct qed_dev *cdev, const u8 **data)
+{
+	struct qed_hwfn *hwfn = QED_LEADING_HWFN(cdev);
+	u8 entity_id, len, buf[32];
+	struct qed_ptt *ptt;
+	u16 cfg_id, count;
+	int rc = 0, i;
+	u32 flags;
+
+	ptt = qed_ptt_acquire(hwfn);
+	if (!ptt)
+		return -EAGAIN;
+
+	/* NVM CFG ID attribute header */
+	*data += 4;
+	entity_id = **data;
+	*data += 2;
+	count = *((u16 *)*data);
+	*data += 2;
+
+	DP_VERBOSE(cdev, NETIF_MSG_DRV,
+		   "Read config ids: entity id %02x num _attrs = %0d\n",
+		   entity_id, count);
+	/* NVM CFG ID attributes */
+	for (i = 0; i < count; i++) {
+		cfg_id = *((u16 *)*data);
+		*data += 2;
+		len = **data;
+		(*data)++;
+		memcpy(buf, *data, len);
+		*data += len;
+
+		flags = entity_id ? QED_NVM_CFG_SET_PF_FLAGS :
+			QED_NVM_CFG_SET_FLAGS;
+
+		DP_VERBOSE(cdev, NETIF_MSG_DRV,
+			   "cfg_id = %d len = %d\n", cfg_id, len);
+		rc = qed_mcp_nvm_set_cfg(hwfn, ptt, cfg_id, entity_id, flags,
+					 buf, len);
+		if (rc) {
+			DP_ERR(cdev, "Error %d configuring %d\n", rc, cfg_id);
+			break;
+		}
+	}
+
+	qed_ptt_release(hwfn, ptt);
+
+	return rc;
+}
+
 static int qed_nvm_flash(struct qed_dev *cdev, const char *name)
 {
 	const struct firmware *image;
@@ -2268,6 +2330,9 @@ static int qed_nvm_flash(struct qed_dev *cdev, const char *name)
 			rc = qed_nvm_flash_image_access(cdev, &data,
 							&check_resp);
 			break;
+		case QED_NVM_FLASH_CMD_NVM_CFG_ID:
+			rc = qed_nvm_flash_cfg_write(cdev, &data);
+			break;
 		default:
 			DP_ERR(cdev, "Unknown command %08x\n", cmd_type);
 			rc = -EINVAL;
diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
index eef02e6..23805ea 100644
--- a/include/linux/qed/qed_if.h
+++ b/include/linux/qed/qed_if.h
@@ -804,6 +804,7 @@ enum qed_nvm_flash_cmd {
 	QED_NVM_FLASH_CMD_FILE_DATA = 0x2,
 	QED_NVM_FLASH_CMD_FILE_START = 0x3,
 	QED_NVM_FLASH_CMD_NVM_CHANGE = 0x4,
+	QED_NVM_FLASH_CMD_NVM_CFG_ID = 0x5,
 	QED_NVM_FLASH_CMD_NVM_MAX,
 };
 
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH V3 net-next 06/10] net: hns3: add debug messages to identify eth down cause
From: David Miller @ 2019-07-28  2:03 UTC (permalink / raw)
  To: tanhuazhong
  Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm, saeedm,
	liuyonglong, lipeng321
In-Reply-To: <1564206372-42467-7-git-send-email-tanhuazhong@huawei.com>

From: Huazhong Tan <tanhuazhong@huawei.com>
Date: Sat, 27 Jul 2019 13:46:08 +0800

> 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       | 18 ++++++++++++++++++
>  drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c    | 19 +++++++++++++++++++
>  .../net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c    | 11 +++++++++++
>  3 files changed, 48 insertions(+)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> index 4d58c53..973c57b 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> @@ -459,6 +459,9 @@ 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);
> +
> +	netif_info(h, drv, netdev, "net open\n");

These will pollute everyone's kernel logs for normal operations.

This is not reasonable at all, sorry.

Furthermore, even if it was appropriate, "netif_info()" is not "debug".


^ permalink raw reply

* Re: [PATCH net-next v3 0/2] qed*: Support for NVM config attributes.
From: David Miller @ 2019-07-28  2:13 UTC (permalink / raw)
  To: skalluru; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190728015549.27051-1-skalluru@marvell.com>

From: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Date: Sat, 27 Jul 2019 18:55:47 -0700

> The patch series adds support for managing the NVM config attributes.
> Patch (1) adds functionality to update config attributes via MFW.
> Patch (2) adds driver interface for updating the config attributes.
> 
> Changes from previous versions:
> -------------------------------
> v3: Removed unused variable.
> v2: Removed unused API.
> 
> Please consider applying this series to "net-next".

I don't see where an existing ethtool method hooks into and calls this
new NVM code.

^ permalink raw reply

* Re: [PATCH v2 bpf-next 0/9] Revamp test_progs as a test running framework
From: Andrii Nakryiko @ 2019-07-28  2:16 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, bpf, Network Development, Alexei Starovoitov,
	Daniel Borkmann, Stanislav Fomichev, Kernel Team
In-Reply-To: <CAADnVQJoS8Yjt-qFHg4XEkiF_3H8nRx15xZfJDSy8YcRT_UKrg@mail.gmail.com>

On Sat, Jul 27, 2019 at 6:12 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sat, Jul 27, 2019 at 12:02 PM Andrii Nakryiko <andriin@fb.com> wrote:
> >
> > This patch set makes a number of changes to test_progs selftest, which is
> > a collection of many other tests (and sometimes sub-tests as well), to provide
> > better testing experience and allow to start convering many individual test
> > programs under selftests/bpf into a single and convenient test runner.
>
> I really like the patches, but something isn't right:

Argh... Uninitialized `int ret` in test__vprintf(). Should be
initialized to zero, otherwise in some corner cases when log buffer is
completely full and ret's initial value is sufficiently large negative
number, it can underflow env.log_cnt, silently skipping one log
output, and then crashing on next one. You've somehow encountered a
fascinating series of conditions that I've never stumbled upon running
my code dozens of times. Fixing, sorry about that!

> #16 raw_tp_writable_reject_nbd_invalid:OK
> #17 raw_tp_writable_test_run:OK
> #18 reference_tracking:OK
> [   87.715996] test_progs[2200]: segfault at 2f ip 00007f56aeea347b sp
> 00007ffce9720980 error 4 in libc-2.23.so[7f56aee5b000+198000]
> [   87.717316] Code: ff ff 44 89 8d 30 fb ff ff e8 01 74 fd ff 44 8b
> 8d 30 fb ff ff 4c 8b 85 28 fb ff ff e9 fd eb ff ff 31 c0 48 83 c9 ff
> 4c 89 df <f2> ae c7 85 28 fb ff ff 00 00 00 00 48 89 c8 48 f7 d0 4c 8f
> [   87.719493] audit: type=1701 audit(1564276195.971:5): auid=0 uid=0
> gid=0 ses=1 subj=kernel pid=2200 comm="test_progs"
> exe="/data/users/ast/net-next/tools/testing/selftests/bpf/test_progs"
> sig=11 res=1
> Segmentation fault (core dumped)
>
> Under gdb fault is different:
> #23 stacktrace_build_id:OK
> Detaching after fork from child process 2276.
> Detaching after fork from child process 2278.
> [  149.013116] perf: interrupt took too long (6799 > 6713), lowering
> kernel.perf_event_max_sample_rate to 29000
> [  149.014634] perf: interrupt took too long (8511 > 8498), lowering
> kernel.perf_event_max_sample_rate to 23000
> [  149.017038] perf: interrupt took too long (10649 > 10638), lowering
> kernel.perf_event_max_sample_rate to 18000
> [  149.021901] perf: interrupt took too long (13322 > 13311), lowering
> kernel.perf_event_max_sample_rate to 15000
> [  149.042946] perf: interrupt took too long (16660 > 16652), lowering
> kernel.perf_event_max_sample_rate to 12000
> Detaching after fork from child process 2279.
> #24 stacktrace_build_id_nmi:OK
> #25 stacktrace_map:OK
> #26 stacktrace_map_raw_tp:OK
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6
> (gdb) bt
> #0  0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6
> #1  0x00007ffff72655a9 in vsnprintf () from /usr/lib/libc.so.6
> #2  0x0000000000403100 in test__vprintf (fmt=0x426754 "%s:PASS:%s %d
> nsec\n", args=0x7fffffffe878) at test_progs.c:114
> #3  0x000000000040325c in test__printf (fmt=fmt@entry=0x426754
> "%s:PASS:%s %d nsec\n") at test_progs.c:147
> #4  0x000000000042222d in test_task_fd_query_rawtp () at
> prog_tests/task_fd_query_rawtp.c:19
> #5  0x0000000000402c76 in main (argc=<optimized out>, argv=<optimized
> out>) at test_progs.c:501
> (gdb) info threads
>   Id   Target Id         Frame
> * 1    Thread 0x7ffff7fea700 (LWP 2245) "test_progs"
> 0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6

^ permalink raw reply

* Re: [PATCH v2 bpf-next 0/9] Revamp test_progs as a test running framework
From: Andrii Nakryiko @ 2019-07-28  2:57 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, bpf, Network Development, Alexei Starovoitov,
	Daniel Borkmann, Stanislav Fomichev, Kernel Team
In-Reply-To: <CAEf4BzY3snLh5=qhFo6RNL1RQMcmVhkCiB2s4p57jQcovp5TWw@mail.gmail.com>

On Sat, Jul 27, 2019 at 7:16 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Sat, Jul 27, 2019 at 6:12 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Sat, Jul 27, 2019 at 12:02 PM Andrii Nakryiko <andriin@fb.com> wrote:
> > >
> > > This patch set makes a number of changes to test_progs selftest, which is
> > > a collection of many other tests (and sometimes sub-tests as well), to provide
> > > better testing experience and allow to start convering many individual test
> > > programs under selftests/bpf into a single and convenient test runner.
> >
> > I really like the patches, but something isn't right:
>
> Argh... Uninitialized `int ret` in test__vprintf(). Should be
> initialized to zero, otherwise in some corner cases when log buffer is
> completely full and ret's initial value is sufficiently large negative
> number, it can underflow env.log_cnt, silently skipping one log
> output, and then crashing on next one. You've somehow encountered a
> fascinating series of conditions that I've never stumbled upon running
> my code dozens of times. Fixing, sorry about that!

Ok, I doubt it was that specific bug (even though it is a bug). Turns
out that you can't call vprintf with the same va_list twice, as it
consumes it, so I have to do va_copy() if I might call vprintf
multiple times. So fixing that now.

>
> > #16 raw_tp_writable_reject_nbd_invalid:OK
> > #17 raw_tp_writable_test_run:OK
> > #18 reference_tracking:OK
> > [   87.715996] test_progs[2200]: segfault at 2f ip 00007f56aeea347b sp
> > 00007ffce9720980 error 4 in libc-2.23.so[7f56aee5b000+198000]
> > [   87.717316] Code: ff ff 44 89 8d 30 fb ff ff e8 01 74 fd ff 44 8b
> > 8d 30 fb ff ff 4c 8b 85 28 fb ff ff e9 fd eb ff ff 31 c0 48 83 c9 ff
> > 4c 89 df <f2> ae c7 85 28 fb ff ff 00 00 00 00 48 89 c8 48 f7 d0 4c 8f
> > [   87.719493] audit: type=1701 audit(1564276195.971:5): auid=0 uid=0
> > gid=0 ses=1 subj=kernel pid=2200 comm="test_progs"
> > exe="/data/users/ast/net-next/tools/testing/selftests/bpf/test_progs"
> > sig=11 res=1
> > Segmentation fault (core dumped)
> >
> > Under gdb fault is different:
> > #23 stacktrace_build_id:OK
> > Detaching after fork from child process 2276.
> > Detaching after fork from child process 2278.
> > [  149.013116] perf: interrupt took too long (6799 > 6713), lowering
> > kernel.perf_event_max_sample_rate to 29000
> > [  149.014634] perf: interrupt took too long (8511 > 8498), lowering
> > kernel.perf_event_max_sample_rate to 23000
> > [  149.017038] perf: interrupt took too long (10649 > 10638), lowering
> > kernel.perf_event_max_sample_rate to 18000
> > [  149.021901] perf: interrupt took too long (13322 > 13311), lowering
> > kernel.perf_event_max_sample_rate to 15000
> > [  149.042946] perf: interrupt took too long (16660 > 16652), lowering
> > kernel.perf_event_max_sample_rate to 12000
> > Detaching after fork from child process 2279.
> > #24 stacktrace_build_id_nmi:OK
> > #25 stacktrace_map:OK
> > #26 stacktrace_map_raw_tp:OK
> >
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6
> > (gdb) bt
> > #0  0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6
> > #1  0x00007ffff72655a9 in vsnprintf () from /usr/lib/libc.so.6
> > #2  0x0000000000403100 in test__vprintf (fmt=0x426754 "%s:PASS:%s %d
> > nsec\n", args=0x7fffffffe878) at test_progs.c:114
> > #3  0x000000000040325c in test__printf (fmt=fmt@entry=0x426754
> > "%s:PASS:%s %d nsec\n") at test_progs.c:147
> > #4  0x000000000042222d in test_task_fd_query_rawtp () at
> > prog_tests/task_fd_query_rawtp.c:19
> > #5  0x0000000000402c76 in main (argc=<optimized out>, argv=<optimized
> > out>) at test_progs.c:501
> > (gdb) info threads
> >   Id   Target Id         Frame
> > * 1    Thread 0x7ffff7fea700 (LWP 2245) "test_progs"
> > 0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6

^ permalink raw reply

* [PATCH v3 bpf-next 0/9] Revamp test_progs as a test running framework
From: Andrii Nakryiko @ 2019-07-28  3:25 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel, sdf
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

This patch set makes a number of changes to test_progs selftest, which is
a collection of many other tests (and sometimes sub-tests as well), to provide
better testing experience and allow to start convering many individual test
programs under selftests/bpf into a single and convenient test runner.

Patch #1 fixes issue with Makefile, which makes prog_tests/test.h compiled as
a C code. This fix allows to change how test.h is generated, providing ability
to have more control on what and how tests are run.

Patch #2 changes how test.h is auto-generated, which allows to have test
definitions, instead of just running test functions. This gives ability to do
more complicated test run policies.

Patch #3 adds `-t <test-name>` and `-n <test-num>` selectors to run only
subset of tests.

Patch #4 changes libbpf_set_print() to return previously set print callback,
allowing to temporarily replace current print callback and then set it back.
This is necessary for some tests that want more control over libbpf logging.

Patch #5 sets up and takes over libbpf logging from individual tests to
test_prog runner, adding -vv verbosity to capture debug output from libbpf.
This is useful when debugging failing tests.

Patch #6 furthers test output management and buffers it by default, emitting
log output only if test fails. This give succinct and clean default test
output. It's possible to bypass this behavior with -v flag, which will turn
off test output buffering.

Patch #7 adds support for sub-tests. It also enhances -t and -n selectors to
both support ability to specify sub-test selectors, as well as enhancing
number selector to accept sets of test, instead of just individual test
number.

Patch #8 converts bpf_verif_scale.c test to use sub-test APIs.

Patch #9 converts send_signal.c tests to use sub-test APIs.

v2->v3:
  - fix buffered output rare unitialized value bug (Alexei);
  - fix buffered output va_list reuse bug (Alexei);
  - fix buffered output truncation due to interleaving zero terminators;

v1->v2:
  - drop libbpf_swap_print, instead return previous function from
    libbpf_set_print (Stanislav);

Andrii Nakryiko (9):
  selftests/bpf: prevent headers to be compiled as C code
  selftests/bpf: revamp test_progs to allow more control
  selftests/bpf: add test selectors by number and name to test_progs
  libbpf: return previous print callback from libbpf_set_print
  selftest/bpf: centralize libbpf logging management for test_progs
  selftests/bpf: abstract away test log output
  selftests/bpf: add sub-tests support for test_progs
  selftests/bpf: convert bpf_verif_scale.c to sub-tests API
  selftests/bpf: convert send_signal.c to use subtests

 tools/lib/bpf/libbpf.c                        |   5 +-
 tools/lib/bpf/libbpf.h                        |   2 +-
 tools/testing/selftests/bpf/Makefile          |  14 +-
 .../selftests/bpf/prog_tests/bpf_obj_id.c     |   6 +-
 .../bpf/prog_tests/bpf_verif_scale.c          |  90 ++--
 .../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 +-
 .../bpf/prog_tests/reference_tracking.c       |  15 +-
 .../selftests/bpf/prog_tests/send_signal.c    |  17 +-
 .../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      | 383 +++++++++++++++++-
 tools/testing/selftests/bpf/test_progs.h      |  45 +-
 16 files changed, 502 insertions(+), 104 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH v3 bpf-next 1/9] selftests/bpf: prevent headers to be compiled as C code
From: Andrii Nakryiko @ 2019-07-28  3:25 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel, sdf
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190728032531.2358749-1-andriin@fb.com>

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.

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 related

* [PATCH v3 bpf-next 4/9] libbpf: return previous print callback from libbpf_set_print
From: Andrii Nakryiko @ 2019-07-28  3:25 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel, sdf
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190728032531.2358749-1-andriin@fb.com>

By returning previously set print callback from libbpf_set_print, it's
possible to restore it, eventually. This is useful when running many
independent test with one default print function, but overriding log
verbosity for particular subset of tests.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/libbpf.c | 5 ++++-
 tools/lib/bpf/libbpf.h | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 8741c39adb1c..ead915aec349 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -74,9 +74,12 @@ static int __base_pr(enum libbpf_print_level level, const char *format,
 
 static libbpf_print_fn_t __libbpf_pr = __base_pr;
 
-void libbpf_set_print(libbpf_print_fn_t fn)
+libbpf_print_fn_t libbpf_set_print(libbpf_print_fn_t fn)
 {
+	libbpf_print_fn_t old_print_fn = __libbpf_pr;
+
 	__libbpf_pr = fn;
+	return old_print_fn;
 }
 
 __printf(2, 3)
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 5cbf459ece0b..8a9d462a6f6d 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -57,7 +57,7 @@ enum libbpf_print_level {
 typedef int (*libbpf_print_fn_t)(enum libbpf_print_level level,
 				 const char *, va_list ap);
 
-LIBBPF_API void libbpf_set_print(libbpf_print_fn_t fn);
+LIBBPF_API libbpf_print_fn_t libbpf_set_print(libbpf_print_fn_t fn);
 
 /* Hide internal to user */
 struct bpf_object;
-- 
2.17.1


^ permalink raw reply related

* [PATCH v3 bpf-next 2/9] selftests/bpf: revamp test_progs to allow more control
From: Andrii Nakryiko @ 2019-07-28  3:25 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel, sdf
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190728032531.2358749-1-andriin@fb.com>

Refactor test_progs to allow better control on what's being run.
Also use argp to do argument parsing, so that it's easier to keep adding
more options.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/testing/selftests/bpf/Makefile     |  8 +--
 tools/testing/selftests/bpf/test_progs.c | 84 +++++++++++++++++++++---
 2 files changed, 77 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index bb66cc4a7f34..3bd0f4a0336a 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -239,14 +239,8 @@ $(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 */'; \
-		  echo '#ifdef DECLARE'; \
 		  ls *.c 2> /dev/null | \
-			sed -e 's@\([^\.]*\)\.c@extern void test_\1(void);@'; \
-		  echo '#endif'; \
-		  echo '#ifdef CALL'; \
-		  ls *.c 2> /dev/null | \
-			sed -e 's@\([^\.]*\)\.c@test_\1();@'; \
-		  echo '#endif' \
+			sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \
 		 ) > $(PROG_TESTS_H))
 
 MAP_TESTS_DIR = $(OUTPUT)/map_tests
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index dae0819b1141..eea88ba59225 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -3,6 +3,7 @@
  */
 #include "test_progs.h"
 #include "bpf_rlimit.h"
+#include <argp.h>
 
 int error_cnt, pass_cnt;
 bool jit_enabled;
@@ -156,22 +157,89 @@ void *spin_lock_thread(void *arg)
 	pthread_exit(arg);
 }
 
-#define DECLARE
+/* extern declarations for test funcs */
+#define DEFINE_TEST(name) extern void test_##name();
 #include <prog_tests/tests.h>
-#undef DECLARE
+#undef DEFINE_TEST
 
-int main(int ac, char **av)
+struct prog_test_def {
+	const char *test_name;
+	void (*run_test)(void);
+};
+
+static struct prog_test_def prog_test_defs[] = {
+#define DEFINE_TEST(name) {	      \
+	.test_name = #name,	      \
+	.run_test = &test_##name,   \
+},
+#include <prog_tests/tests.h>
+#undef DEFINE_TEST
+};
+
+const char *argp_program_version = "test_progs 0.1";
+const char *argp_program_bug_address = "<bpf@vger.kernel.org>";
+const char argp_program_doc[] = "BPF selftests test runner";
+
+enum ARG_KEYS {
+	ARG_VERIFIER_STATS = 's',
+};
+	
+static const struct argp_option opts[] = {
+	{ "verifier-stats", ARG_VERIFIER_STATS, NULL, 0,
+	  "Output verifier statistics", },
+	{},
+};
+
+struct test_env {
+	bool verifier_stats;
+};
+
+static struct test_env env = {};
+
+static error_t parse_arg(int key, char *arg, struct argp_state *state)
 {
+	struct test_env *env = state->input;
+
+	switch (key) {
+	case ARG_VERIFIER_STATS:
+		env->verifier_stats = true;
+		break;
+	case ARGP_KEY_ARG:
+		argp_usage(state);
+		break;
+	case ARGP_KEY_END:
+		break;
+	default:
+		return ARGP_ERR_UNKNOWN;
+	}
+	return 0;
+}
+
+
+int main(int argc, char **argv)
+{
+	static const struct argp argp = {
+		.options = opts,
+		.parser = parse_arg,
+		.doc = argp_program_doc,
+	};
+	const struct prog_test_def *def;
+	int err, i;
+
+	err = argp_parse(&argp, argc, argv, 0, NULL, &env);
+	if (err)
+		return err;
+
 	srand(time(NULL));
 
 	jit_enabled = is_jit_enabled();
 
-	if (ac == 2 && strcmp(av[1], "-s") == 0)
-		verifier_stats = true;
+	verifier_stats = env.verifier_stats;
 
-#define CALL
-#include <prog_tests/tests.h>
-#undef CALL
+	for (i = 0; i < ARRAY_SIZE(prog_test_defs); i++) {
+		def = &prog_test_defs[i];
+		def->run_test();
+	}
 
 	printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
 	return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
-- 
2.17.1


^ permalink raw reply related


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