* Re: [PATCH bpf-next 05/13] bpf: get better bpf_prog ksyms based on btf func type_id
From: Martin Lau @ 2018-10-15 23:12 UTC (permalink / raw)
To: Yonghong Song
Cc: Alexei Starovoitov, daniel@iogearbox.net, netdev@vger.kernel.org,
Kernel Team
In-Reply-To: <20181012185446.2379289-1-yhs@fb.com>
On Fri, Oct 12, 2018 at 11:54:42AM -0700, Yonghong Song wrote:
> This patch added interface to load a program with the following
> additional information:
> . prog_btf_fd
> . func_info and func_info_len
> where func_info will provides function range and type_id
> corresponding to each function.
>
> If verifier agrees with function range provided by the user,
> the bpf_prog ksym for each function will use the func name
> provided in the type_id, which is supposed to provide better
> encoding as it is not limited by 16 bytes program name
> limitation and this is better for bpf program which contains
> multiple subprograms.
>
> The bpf_prog_info interface is also extended to
> return btf_id and jited_func_types, so user spaces can
> print out the function prototype for each jited function.
Some nits.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
> include/linux/bpf.h | 2 +
> include/linux/bpf_verifier.h | 1 +
> include/linux/btf.h | 2 +
> include/uapi/linux/bpf.h | 11 +++++
> kernel/bpf/btf.c | 16 +++++++
> kernel/bpf/core.c | 9 ++++
> kernel/bpf/syscall.c | 86 +++++++++++++++++++++++++++++++++++-
> kernel/bpf/verifier.c | 50 +++++++++++++++++++++
> 8 files changed, 176 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 9b558713447f..e9c63ffa01af 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -308,6 +308,8 @@ struct bpf_prog_aux {
> void *security;
> #endif
> struct bpf_prog_offload *offload;
> + struct btf *btf;
> + u32 type_id; /* type id for this prog/func */
> union {
> struct work_struct work;
> struct rcu_head rcu;
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 9e8056ec20fa..e84782ec50ac 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -201,6 +201,7 @@ static inline bool bpf_verifier_log_needed(const struct bpf_verifier_log *log)
> struct bpf_subprog_info {
> u32 start; /* insn idx of function entry point */
> u16 stack_depth; /* max. stack depth used by this function */
> + u32 type_id; /* btf type_id for this subprog */
> };
>
> /* single container for all structs
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index e076c4697049..90e91b52aa90 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -46,5 +46,7 @@ void btf_type_seq_show(const struct btf *btf, u32 type_id, void *obj,
> struct seq_file *m);
> int btf_get_fd_by_id(u32 id);
> u32 btf_id(const struct btf *btf);
> +bool is_btf_func_type(const struct btf *btf, u32 type_id);
> +const char *btf_get_name_by_id(const struct btf *btf, u32 type_id);
>
> #endif
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index f9187b41dff6..7ebbf4f06a65 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -332,6 +332,9 @@ union bpf_attr {
> * (context accesses, allowed helpers, etc).
> */
> __u32 expected_attach_type;
> + __u32 prog_btf_fd; /* fd pointing to BTF type data */
> + __u32 func_info_len; /* func_info length */
> + __aligned_u64 func_info; /* func type info */
> };
>
> struct { /* anonymous struct used by BPF_OBJ_* commands */
> @@ -2585,6 +2588,9 @@ struct bpf_prog_info {
> __u32 nr_jited_func_lens;
> __aligned_u64 jited_ksyms;
> __aligned_u64 jited_func_lens;
> + __u32 btf_id;
> + __u32 nr_jited_func_types;
> + __aligned_u64 jited_func_types;
> } __attribute__((aligned(8)));
>
> struct bpf_map_info {
> @@ -2896,4 +2902,9 @@ struct bpf_flow_keys {
> };
> };
>
> +struct bpf_func_info {
> + __u32 insn_offset;
> + __u32 type_id;
> +};
> +
> #endif /* _UAPI__LINUX_BPF_H__ */
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 794a185f11bf..85b8eeccddbd 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -486,6 +486,15 @@ static const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id)
> return btf->types[type_id];
> }
>
> +bool is_btf_func_type(const struct btf *btf, u32 type_id)
> +{
> + const struct btf_type *type = btf_type_by_id(btf, type_id);
> +
> + if (!type || BTF_INFO_KIND(type->info) != BTF_KIND_FUNC)
> + return false;
> + return true;
> +}
Can btf_type_is_func() (from patch 2) be reused?
The btf_type_by_id() can be done by the caller.
I don't think it worths to add a similar helper
for just one user for now.
The !type check can be added to btf_type_is_func() if
it is needed.
> +
> /*
> * Regular int is not a bit field and it must be either
> * u8/u16/u32/u64.
> @@ -2579,3 +2588,10 @@ u32 btf_id(const struct btf *btf)
> {
> return btf->id;
> }
> +
> +const char *btf_get_name_by_id(const struct btf *btf, u32 type_id)
> +{
> + const struct btf_type *t = btf_type_by_id(btf, type_id);
> +
> + return btf_name_by_offset(btf, t->name_off);
> +}
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 3f5bf1af0826..add3866a120e 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -27,6 +27,7 @@
> #include <linux/random.h>
> #include <linux/moduleloader.h>
> #include <linux/bpf.h>
> +#include <linux/btf.h>
> #include <linux/frame.h>
> #include <linux/rbtree_latch.h>
> #include <linux/kallsyms.h>
> @@ -387,6 +388,7 @@ bpf_get_prog_addr_region(const struct bpf_prog *prog,
> static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
> {
> const char *end = sym + KSYM_NAME_LEN;
> + const char *func_name;
>
> BUILD_BUG_ON(sizeof("bpf_prog_") +
> sizeof(prog->tag) * 2 +
> @@ -401,6 +403,13 @@ static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
>
> sym += snprintf(sym, KSYM_NAME_LEN, "bpf_prog_");
> sym = bin2hex(sym, prog->tag, sizeof(prog->tag));
> +
> + if (prog->aux->btf) {
> + func_name = btf_get_name_by_id(prog->aux->btf, prog->aux->type_id);
> + snprintf(sym, (size_t)(end - sym), "_%s", func_name);
> + return;
> + }
> +
> if (prog->aux->name[0])
> snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name);
> else
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 4f416234251f..aa4688a1a137 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1120,6 +1120,7 @@ static void __bpf_prog_put(struct bpf_prog *prog, bool do_idr_lock)
> /* bpf_prog_free_id() must be called first */
> bpf_prog_free_id(prog, do_idr_lock);
> bpf_prog_kallsyms_del_all(prog);
> + btf_put(prog->aux->btf);
>
> call_rcu(&prog->aux->rcu, __bpf_prog_put_rcu);
> }
> @@ -1343,8 +1344,45 @@ bpf_prog_load_check_attach_type(enum bpf_prog_type prog_type,
> }
> }
>
> +static int prog_check_btf(const struct bpf_prog *prog, const struct btf *btf,
> + union bpf_attr *attr)
> +{
> + struct bpf_func_info __user *uinfo, info;
> + int i, nfuncs, usize;
> + u32 prev_offset;
> +
> + usize = sizeof(struct bpf_func_info);
> + if (attr->func_info_len % usize)
> + return -EINVAL;
> +
> + /* func_info section should have increasing and valid insn_offset
> + * and type should be BTF_KIND_FUNC.
> + */
> + nfuncs = attr->func_info_len / usize;
> + uinfo = u64_to_user_ptr(attr->func_info);
> + for (i = 0; i < nfuncs; i++) {
> + if (copy_from_user(&info, uinfo, usize))
> + return -EFAULT;
> +
> + if (!is_btf_func_type(btf, info.type_id))
> + return -EINVAL;
> +
> + if (i == 0) {
> + if (info.insn_offset)
> + return -EINVAL;
> + prev_offset = 0;
> + } else if (info.insn_offset < prev_offset) {
> + return -EINVAL;
> + }
> +
> + prev_offset = info.insn_offset;
> + }
> +
> + return 0;
> +}
> +
> /* last field in 'union bpf_attr' used by this command */
> -#define BPF_PROG_LOAD_LAST_FIELD expected_attach_type
> +#define BPF_PROG_LOAD_LAST_FIELD func_info
>
> static int bpf_prog_load(union bpf_attr *attr)
> {
> @@ -1431,6 +1469,27 @@ static int bpf_prog_load(union bpf_attr *attr)
> if (err)
> goto free_prog;
>
> + /* copy func_info before verifier which may make
> + * some adjustment.
> + */
Is it a left over comment? I don't see the intention of
copying func_info to avoid verifier modification from below.
I could be missing something.
or should the comments be moved to the new "check_btf_func()" below?
> + if (attr->func_info_len) {
> + struct btf *btf;
> +
> + btf = btf_get_by_fd(attr->prog_btf_fd);
> + if (IS_ERR(btf)) {
> + err = PTR_ERR(btf);
> + goto free_prog;
> + }
> +
> + err = prog_check_btf(prog, btf, attr);
> + if (err) {
> + btf_put(btf);
> + goto free_prog;
> + }
> +
> + prog->aux->btf = btf;
> + }
> +
> /* run eBPF verifier */
> err = bpf_check(&prog, attr);
> if (err < 0)
> @@ -1463,6 +1522,7 @@ static int bpf_prog_load(union bpf_attr *attr)
> bpf_prog_kallsyms_del_subprogs(prog);
> free_used_maps(prog->aux);
> free_prog:
> + btf_put(prog->aux->btf);
> bpf_prog_uncharge_memlock(prog);
> free_prog_sec:
> security_bpf_prog_free(prog->aux);
> @@ -2108,6 +2168,30 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
> }
> }
>
> + if (prog->aux->btf) {
> + info.btf_id = btf_id(prog->aux->btf);
> +
> + ulen = info.nr_jited_func_types;
> + info.nr_jited_func_types = prog->aux->func_cnt;
> + if (info.nr_jited_func_types && ulen) {
> + if (bpf_dump_raw_ok()) {
> + u32 __user *user_types;
> + u32 func_type, i;
> +
> + ulen = min_t(u32, info.nr_jited_func_types,
> + ulen);
> + user_types = u64_to_user_ptr(info.jited_func_types);
> + for (i = 0; i < ulen; i++) {
> + func_type = prog->aux->func[i]->aux->type_id;
> + if (put_user(func_type, &user_types[i]))
> + return -EFAULT;
> + }
> + } else {
> + info.jited_func_types = 0;
> + }
> + }
> + }
> +
> done:
> if (copy_to_user(uinfo, &info, info_len) ||
> put_user(info_len, &uattr->info.info_len))
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 3f93a548a642..97c408e84322 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -4589,6 +4589,50 @@ static int check_cfg(struct bpf_verifier_env *env)
> return ret;
> }
>
> +static int check_btf_func(struct bpf_prog *prog, struct bpf_verifier_env *env,
> + union bpf_attr *attr)
> +{
> + struct bpf_func_info *data;
> + int i, nfuncs, ret = 0;
> +
> + if (!attr->func_info_len)
> + return 0;
> +
> + nfuncs = attr->func_info_len / sizeof(struct bpf_func_info);
> + if (env->subprog_cnt != nfuncs) {
> + verbose(env, "number of funcs in func_info does not match verifier\n");
> + return -EINVAL;
> + }
> +
> + data = kvmalloc(attr->func_info_len, GFP_KERNEL | __GFP_NOWARN);
> + if (!data) {
> + verbose(env, "no memory to allocate attr func_info\n");
> + return -ENOMEM;
> + }
> +
> + if (copy_from_user(data, u64_to_user_ptr(attr->func_info),
> + attr->func_info_len)) {
> + verbose(env, "memory copy error for attr func_info\n");
> + ret = -EFAULT;
> + goto cleanup;
> + }
Extra indentation.
> +
> + for (i = 0; i < nfuncs; i++) {
> + if (env->subprog_info[i].start != data[i].insn_offset) {
> + verbose(env, "func_info subprog start (%d) does not match verifier (%d)\n",
> + env->subprog_info[i].start, data[i].insn_offset);
The printing args are swapped? env->subprog_info[i].start should
go to the "verifier (%d)"?
and s/%d/%u/
> + ret = -EINVAL;
> + goto cleanup;
> + }
> + env->subprog_info[i].type_id = data[i].type_id;
> + }
> +
> + prog->aux->type_id = data[0].type_id;
> +cleanup:
> + kvfree(data);
> + return ret;
> +}
> +
> /* check %cur's range satisfies %old's */
> static bool range_within(struct bpf_reg_state *old,
> struct bpf_reg_state *cur)
> @@ -5873,6 +5917,8 @@ static int jit_subprogs(struct bpf_verifier_env *env)
> func[i]->aux->name[0] = 'F';
> func[i]->aux->stack_depth = env->subprog_info[i].stack_depth;
> func[i]->jit_requested = 1;
> + func[i]->aux->btf = prog->aux->btf;
> + func[i]->aux->type_id = env->subprog_info[i].type_id;
> func[i] = bpf_int_jit_compile(func[i]);
> if (!func[i]->jited) {
> err = -ENOTSUPP;
> @@ -6307,6 +6353,10 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)
> if (ret < 0)
> goto skip_full_check;
>
> + ret = check_btf_func(env->prog, env, attr);
> + if (ret < 0)
> + goto skip_full_check;
> +
> ret = do_check(env);
> if (env->cur_state) {
> free_verifier_state(env->cur_state, true);
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH bpf-next 0/2] IPv6 sk-lookup fixes
From: Alexei Starovoitov @ 2018-10-15 23:10 UTC (permalink / raw)
To: Joe Stringer; +Cc: daniel, ast, netdev
In-Reply-To: <20181015172746.6475-1-joe@wand.net.nz>
On Mon, Oct 15, 2018 at 10:27:44AM -0700, Joe Stringer wrote:
> This series includes a couple of fixups for the IPv6 socket lookup
> helper, to make the API more consistent (always supply all arguments in
> network byte-order) and to allow its use when IPv6 is compiled as a
> module.
Applied, Thanks
^ permalink raw reply
* Re: [PATCH bpf-next] tools: bpftool: add map create command
From: Jakub Kicinski @ 2018-10-15 22:53 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: daniel, netdev, oss-drivers
In-Reply-To: <20181015195805.7xob34egcs3pqvag@ast-mbp.dhcp.thefacebook.com>
On Mon, 15 Oct 2018 12:58:07 -0700, Alexei Starovoitov wrote:
> > > > fprintf(stderr,
> > > > "Usage: %s %s { show | list } [MAP]\n"
> > > > + " %s %s create FILE type TYPE key KEY_SIZE value VALUE_SIZE \\\n"
> > > > + " entries MAX_ENTRIES [name NAME] [flags FLAGS] \\\n"
> > > > + " [dev NAME]\n"
> > >
> > > I suspect as soon as bpftool has an ability to create standalone maps
> > > some folks will start relying on such interface.
> >
> > That'd be cool, do you see any real life use cases where its useful
> > outside of corner case testing?
>
> In our XDP use case we have an odd protocol for different apps to share
> common prog_array that is pinned in bpffs.
> If cmdline creation of it via bpftool was available that would have been
> an option to consider. Not saying that it would have been a better option.
> Just another option.
I see, I didn't think of prog arrays.
> > > Therefore I'd like to request to make 'name' argument to be mandatory.
> >
> > Will do in v2!
>
> thx!
>
> > > I think in the future we will require BTF to be mandatory too.
> > > We need to move towards more transparent and debuggable infra.
> > > Do you think requiring json description of key/value would be managable to implement?
> > > Then bpftool could convert it to BTF and the map full be fully defined.
> > > I certainly understand that bpf prog can disregard the key/value layout today,
> > > but we will make verifier to enforce that in the future too.
> >
> > I was hoping that we can leave BTF support as a future extension, and
> > then once we have the option for the verifier to enforce BTF (a sysctl?)
> > the bpftool map create without a BTF will get rejected as one would
> > expect.
>
> right. something like sysctl in the future.
>
> > IOW it's fine not to make BTF required at bpftool level and
> > leave it to system configuration.
> >
> > I'd love to implement the BTF support right away, but I'm not sure I
> > can afford that right now time-wise. The whole map create command is
> > pretty trivial, but for BTF we don't even have a way of dumping it
> > AFAICT. We can pretty print values, but what is the format in which to
> > express the BTF itself? We could do JSON, do we use an external
> > library? Should we have a separate BTF command for that?
>
> I prefer standard C type description for both input and output :)
> Anyway that wasn't a request for you to do it now. More of the feature
> request for somebody to put on todo list :)
Oh, okay :)
I will wait for John's patches to get merged and post v2, otherwise
we'd conflict on the man page.
^ permalink raw reply
* Re: [PATCH bpf-next 02/13] bpf: btf: Add BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO
From: Daniel Borkmann @ 2018-10-15 22:36 UTC (permalink / raw)
To: Yonghong Song, ast, kafai, netdev; +Cc: kernel-team
In-Reply-To: <20181012185424.2378502-3-yhs@fb.com>
On 10/12/2018 08:54 PM, Yonghong Song wrote:
[...]
> +static bool btf_name_valid_identifier(const struct btf *btf, u32 offset)
> +{
> + /* offset must be valid */
> + const char *src = &btf->strings[offset];
> +
> + if (!isalpha(*src) && *src != '_')
> + return false;
> +
> + src++;
> + while (*src) {
> + if (!isalnum(*src) && *src != '_')
> + return false;
> + src++;
> + }
> +
> + return true;
> +}
Should there be an upper name length limit like KSYM_NAME_LEN? (Is it implied
by the kvmalloc() limit?)
> static const char *btf_name_by_offset(const struct btf *btf, u32 offset)
> {
> if (!offset)
> @@ -747,7 +782,9 @@ static bool env_type_is_resolve_sink(const struct btf_verifier_env *env,
> /* int, enum or void is a sink */
> return !btf_type_needs_resolve(next_type);
> case RESOLVE_PTR:
> - /* int, enum, void, struct or array is a sink for ptr */
> + /* int, enum, void, struct, array or func_ptoto is a sink
> + * for ptr
> + */
> return !btf_type_is_modifier(next_type) &&
> !btf_type_is_ptr(next_type);
> case RESOLVE_STRUCT_OR_ARRAY:
^ permalink raw reply
* Re: Fw: [Bug 201423] New: eth0: hw csum failure
From: Fabio Rossi @ 2018-10-15 22:28 UTC (permalink / raw)
To: Eric Dumazet, Stephen Hemminger; +Cc: netdev
In-Reply-To: <CANn89iLA+rdFNXXdzogLHF1FqYg3CjpwXJbscWTJ8Bk8bN2Scw@mail.gmail.com>
On 15 October 2018 17:41:47 CEST, Eric Dumazet <edumazet@google.com> wrote:
>On Mon, Oct 15, 2018 at 8:15 AM Stephen Hemminger
><stephen@networkplumber.org> wrote:
>>
>>
>>
>> Begin forwarded message:
>>
>> Date: Sun, 14 Oct 2018 10:42:48 +0000
>> From: bugzilla-daemon@bugzilla.kernel.org
>> To: stephen@networkplumber.org
>> Subject: [Bug 201423] New: eth0: hw csum failure
>>
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=201423
>>
>> Bug ID: 201423
>> Summary: eth0: hw csum failure
>> Product: Networking
>> Version: 2.5
>> Kernel Version: 4.19.0-rc7
>> Hardware: Intel
>> OS: Linux
>> Tree: Mainline
>> Status: NEW
>> Severity: normal
>> Priority: P1
>> Component: Other
>> Assignee: stephen@networkplumber.org
>> Reporter: rossi.f@inwind.it
>> Regression: No
>>
>> I have a P6T DELUXE V2 motherboard and using the sky2 driver for the
>ethernet
>> ports. I get the following error message:
>>
>> [ 433.727397] eth0: hw csum failure
>> [ 433.727406] CPU: 4 PID: 0 Comm: swapper/4 Not tainted 4.19.0-rc7
>#19
>> [ 433.727406] Hardware name: System manufacturer System Product
>Name/P6T
>> DELUXE V2, BIOS 1202 12/22/2010
>> [ 433.727407] Call Trace:
>> [ 433.727409] <IRQ>
>> [ 433.727415] dump_stack+0x46/0x5b
>> [ 433.727419] __skb_checksum_complete+0xb0/0xc0
>> [ 433.727423] tcp_v4_rcv+0x528/0xb60
>> [ 433.727426] ? ipt_do_table+0x2d0/0x400
>> [ 433.727429] ip_local_deliver_finish+0x5a/0x110
>> [ 433.727430] ip_local_deliver+0xe1/0xf0
>> [ 433.727431] ? ip_sublist_rcv_finish+0x60/0x60
>> [ 433.727432] ip_rcv+0xca/0xe0
>> [ 433.727434] ? ip_rcv_finish_core.isra.0+0x300/0x300
>> [ 433.727436] __netif_receive_skb_one_core+0x4b/0x70
>> [ 433.727438] netif_receive_skb_internal+0x4e/0x130
>> [ 433.727439] napi_gro_receive+0x6a/0x80
>> [ 433.727442] sky2_poll+0x707/0xd20
>> [ 433.727446] ? rcu_check_callbacks+0x1b4/0x900
>> [ 433.727447] net_rx_action+0x237/0x380
>> [ 433.727449] __do_softirq+0xdc/0x1e0
>> [ 433.727452] irq_exit+0xa9/0xb0
>> [ 433.727453] do_IRQ+0x45/0xc0
>> [ 433.727455] common_interrupt+0xf/0xf
>> [ 433.727456] </IRQ>
>> [ 433.727459] RIP: 0010:cpuidle_enter_state+0x124/0x200
>> [ 433.727461] Code: 53 60 89 c3 e8 dd 90 ad ff 65 8b 3d 96 58 a7 7e
>e8 d1 8f
>> ad ff 31 ff 49 89 c4 e8 27 99 ad ff fb 48 ba cf f7 53 e3 a5 9b c4 20
><4c> 89 e1
>> 4c 29 e9 48 89 c8 48 c1 f9 3f 48 f7 ea b8 ff ff ff 7f 48
>> [ 433.727462] RSP: 0000:ffffc900000a3e98 EFLAGS: 00000282 ORIG_RAX:
>> ffffffffffffffde
>> [ 433.727463] RAX: ffff880237b1f280 RBX: 0000000000000004 RCX:
>> 000000000000001f
>> [ 433.727464] RDX: 20c49ba5e353f7cf RSI: 000000002fe419c1 RDI:
>> 0000000000000000
>> [ 433.727465] RBP: ffff880237b263a0 R08: 0000000000000714 R09:
>> 000000650512105d
>> [ 433.727465] R10: 00000000ffffffff R11: 0000000000000342 R12:
>> 00000064fc2a8b1c
>> [ 433.727466] R13: 00000064fc25b35f R14: 0000000000000004 R15:
>> ffffffff8204af20
>> [ 433.727468] ? cpuidle_enter_state+0x119/0x200
>> [ 433.727471] do_idle+0x1bf/0x200
>> [ 433.727473] cpu_startup_entry+0x6a/0x70
>> [ 433.727475] start_secondary+0x17f/0x1c0
>> [ 433.727476] secondary_startup_64+0xa4/0xb0
>> [ 441.662954] eth0: hw csum failure
>> [ 441.662959] CPU: 4 PID: 4347 Comm: radeon_cs:0 Not tainted
>4.19.0-rc7 #19
>> [ 441.662960] Hardware name: System manufacturer System Product
>Name/P6T
>> DELUXE V2, BIOS 1202 12/22/2010
>> [ 441.662960] Call Trace:
>> [ 441.662963] <IRQ>
>> [ 441.662968] dump_stack+0x46/0x5b
>> [ 441.662972] __skb_checksum_complete+0xb0/0xc0
>> [ 441.662975] tcp_v4_rcv+0x528/0xb60
>> [ 441.662979] ? ipt_do_table+0x2d0/0x400
>> [ 441.662981] ip_local_deliver_finish+0x5a/0x110
>> [ 441.662983] ip_local_deliver+0xe1/0xf0
>> [ 441.662985] ? ip_sublist_rcv_finish+0x60/0x60
>> [ 441.662986] ip_rcv+0xca/0xe0
>> [ 441.662988] ? ip_rcv_finish_core.isra.0+0x300/0x300
>> [ 441.662990] __netif_receive_skb_one_core+0x4b/0x70
>> [ 441.662993] netif_receive_skb_internal+0x4e/0x130
>> [ 441.662994] napi_gro_receive+0x6a/0x80
>> [ 441.662998] sky2_poll+0x707/0xd20
>> [ 441.663000] net_rx_action+0x237/0x380
>> [ 441.663002] __do_softirq+0xdc/0x1e0
>> [ 441.663005] irq_exit+0xa9/0xb0
>> [ 441.663007] do_IRQ+0x45/0xc0
>> [ 441.663009] common_interrupt+0xf/0xf
>> [ 441.663010] </IRQ>
>> [ 441.663012] RIP: 0010:merge+0x22/0xb0
>> [ 441.663014] Code: c3 31 c0 c3 90 90 90 90 41 56 41 55 41 54 55 48
>89 d5 53
>> 48 89 cb 48 83 ec 18 65 48 8b 04 25 28 00 00 00 48 89 44 24 10 31 c0
><48> 85 c9
>> 74 70 48 85 d2 74 6b 49 89 fd 49 89 f6 49 89 e4 eb 14 48
>> [ 441.663015] RSP: 0018:ffffc9000090b988 EFLAGS: 00000246 ORIG_RAX:
>> ffffffffffffffde
>> [ 441.663017] RAX: 0000000000000000 RBX: ffff88021ab2d408 RCX:
>> ffff88021ab2d408
>> [ 441.663018] RDX: ffff88021ab2d388 RSI: ffffffffa021c440 RDI:
>> 0000000000000000
>> [ 441.663019] RBP: ffff88021ab2d388 R08: 0000000000005ecf R09:
>> 0000000000008500
>> [ 441.663020] R10: ffffea000877ec00 R11: ffff880236803500 R12:
>> ffffffffa021c440
>> [ 441.663021] R13: ffff88021ab2d448 R14: 0000000000000004 R15:
>> ffffc9000090b9e0
>> [ 441.663048] ? radeon_irq_kms_set_irq_n_enabled+0x120/0x120
>[radeon]
>> [ 441.663063] ? radeon_irq_kms_set_irq_n_enabled+0x120/0x120
>[radeon]
>> [ 441.663065] ? merge+0x57/0xb0
>> [ 441.663080] ? radeon_irq_kms_set_irq_n_enabled+0x120/0x120
>[radeon]
>> [ 441.663082] list_sort+0x8b/0x230
>> [ 441.663094] radeon_cs_parser_fini+0xdf/0x110 [radeon]
>> [ 441.663110] radeon_cs_ioctl+0x2a4/0x710 [radeon]
>> [ 441.663113] ? __switch_to_asm+0x34/0x70
>> [ 441.663114] ? __switch_to_asm+0x40/0x70
>> [ 441.663130] ? radeon_cs_parser_init+0x20/0x20 [radeon]
>> [ 441.663141] drm_ioctl_kernel+0xa3/0xe0 [drm]
>> [ 441.663149] drm_ioctl+0x2e2/0x380 [drm]
>> [ 441.663164] ? radeon_cs_parser_init+0x20/0x20 [radeon]
>> [ 441.663168] ? page_add_new_anon_rmap+0x42/0x70
>> [ 441.663171] do_vfs_ioctl+0x9a/0x600
>> [ 441.663173] ksys_ioctl+0x35/0x60
>> [ 441.663175] __x64_sys_ioctl+0x11/0x20
>> [ 441.663177] do_syscall_64+0x3d/0xf0
>> [ 441.663179] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> [ 441.663180] RIP: 0033:0x7f9377377f37
>> [ 441.663182] Code: 00 00 00 75 0c 48 c7 c0 ff ff ff ff 48 83 c4 18
>c3 e8 ad
>> db 01 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 b8 10 00 00 00 0f 05
><48> 3d 01
>> f0 ff ff 73 01 c3 48 8b 0d 21 4f 2c 00 f7 d8 64 89 01 48
>> [ 441.663183] RSP: 002b:00007f92c3130d28 EFLAGS: 00000246 ORIG_RAX:
>> 0000000000000010
>> [ 441.663185] RAX: ffffffffffffffda RBX: 0000564498327ec0 RCX:
>> 00007f9377377f37
>> [ 441.663186] RDX: 0000564498337ec8 RSI: 00000000c0206466 RDI:
>> 0000000000000010
>> [ 441.663186] RBP: 0000564498337ec8 R08: 0000000000000000 R09:
>> 0000000000000000
>> [ 441.663187] R10: 0000000000000000 R11: 0000000000000246 R12:
>> 00000000c0206466
>> [ 441.663188] R13: 0000000000000010 R14: 0000000000000000 R15:
>> 0000564497a38120
>> [ 462.833418] eth0: hw csum failure
>> [ 462.833428] CPU: 4 PID: 0 Comm: swapper/4 Not tainted 4.19.0-rc7
>#19
>> [ 462.833429] Hardware name: System manufacturer System Product
>Name/P6T
>> DELUXE V2, BIOS 1202 12/22/2010
>> [ 462.833429] Call Trace:
>> [ 462.833432] <IRQ>
>> [ 462.833438] dump_stack+0x46/0x5b
>> [ 462.833442] __skb_checksum_complete+0xb0/0xc0
>> [ 462.833446] tcp_v4_rcv+0x528/0xb60
>> [ 462.833449] ? ipt_do_table+0x2d0/0x400
>> [ 462.833452] ip_local_deliver_finish+0x5a/0x110
>> [ 462.833454] ip_local_deliver+0xe1/0xf0
>> [ 462.833455] ? ip_sublist_rcv_finish+0x60/0x60
>> [ 462.833457] ip_rcv+0xca/0xe0
>> [ 462.833459] ? ip_rcv_finish_core.isra.0+0x300/0x300
>> [ 462.833461] __netif_receive_skb_one_core+0x4b/0x70
>> [ 462.833464] netif_receive_skb_internal+0x4e/0x130
>> [ 462.833466] napi_gro_receive+0x6a/0x80
>> [ 462.833469] sky2_poll+0x707/0xd20
>> [ 462.833471] net_rx_action+0x237/0x380
>> [ 462.833474] __do_softirq+0xdc/0x1e0
>> [ 462.833477] irq_exit+0xa9/0xb0
>> [ 462.833479] do_IRQ+0x45/0xc0
>> [ 462.833481] common_interrupt+0xf/0xf
>> [ 462.833482] </IRQ>
>> [ 462.833486] RIP: 0010:cpuidle_enter_state+0x124/0x200
>> [ 462.833488] Code: 53 60 89 c3 e8 dd 90 ad ff 65 8b 3d 96 58 a7 7e
>e8 d1 8f
>> ad ff 31 ff 49 89 c4 e8 27 99 ad ff fb 48 ba cf f7 53 e3 a5 9b c4 20
><4c> 89 e1
>> 4c 29 e9 48 89 c8 48 c1 f9 3f 48 f7 ea b8 ff ff ff 7f 48
>> [ 462.833489] RSP: 0018:ffffc900000a3e98 EFLAGS: 00000282 ORIG_RAX:
>> ffffffffffffffde
>> [ 462.833491] RAX: ffff880237b1f280 RBX: 0000000000000004 RCX:
>> 000000000000001f
>> [ 462.833492] RDX: 20c49ba5e353f7cf RSI: 000000002fe419c1 RDI:
>> 0000000000000000
>> [ 462.833493] RBP: ffff880237b263a0 R08: 0000000000000000 R09:
>> 0000000000000000
>> [ 462.833494] R10: 00000000ffffffff R11: 0000000000000273 R12:
>> 0000006bc3052131
>> [ 462.833495] R13: 0000006bc2f99f57 R14: 0000000000000004 R15:
>> ffffffff8204af20
>> [ 462.833498] ? cpuidle_enter_state+0x119/0x200
>> [ 462.833503] do_idle+0x1bf/0x200
>> [ 462.833506] cpu_startup_entry+0x6a/0x70
>> [ 462.833510] start_secondary+0x17f/0x1c0
>> [ 462.833513] secondary_startup_64+0xa4/0xb0
>>
>> Something is changed between 4.17.12 and 4.18, after bisecting the
>problem I
>> got the following first bad commit:
>>
>> commit 88078d98d1bb085d72af8437707279e203524fa5
>> Author: Eric Dumazet <edumazet@google.com>
>> Date: Wed Apr 18 11:43:15 2018 -0700
>>
>> net: pskb_trim_rcsum() and CHECKSUM_COMPLETE are friends
>>
>> After working on IP defragmentation lately, I found that some
>large
>> packets defeat CHECKSUM_COMPLETE optimization because of NIC
>adding
>> zero paddings on the last (small) fragment.
>>
>> While removing the padding with pskb_trim_rcsum(), we set
>skb->ip_summed
>> to CHECKSUM_NONE, forcing a full csum validation, even if all
>prior
>> fragments had CHECKSUM_COMPLETE set.
>>
>> We can instead compute the checksum of the part we are trimming,
>> usually smaller than the part we keep.
>>
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>
>
>Thanks for bisecting !
>
>This commit is known to expose some NIC/driver bugs.
>
>Look at commit 12b03558cef6d655d0d394f5e98a6fd07c1f6c0f
>("net: sungem: fix rx checksum support") for one driver needing a fix.
>
>I assume SKY2_HW_NEW_LE is not set on your NIC ?
here is the chip found on my motherboard so it seems that flag is not set
# dmesg | grep sky2
[ 0.545661] sky2: driver version 1.30 [ 0.545781] sky2 0000:06:00.0: Yukon-2 EC Ultra chip revision 3 [ 0.546067] sky2 0000:06:00.0 eth0: addr 00:24:8c:xx:xx:xx [ 0.546188] sky2 0000:04:00.0: Yukon-2 EC Ultra chip revision 3 [ 0.546484] sky2 0000:04:00.0 eth1: addr 00:24:8c:xx:xx:xx [ 38.043074] sky2 0000:06:00.0 eth0: enabling interface [ 39.842161] sky2 0000:06:00.0 eth0: Link is up at 100 Mbps, full duplex, flow control rx
^ permalink raw reply
* Re: [PATCH bpf-next 02/13] bpf: btf: Add BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO
From: Daniel Borkmann @ 2018-10-15 22:30 UTC (permalink / raw)
To: Yonghong Song, ast, kafai, netdev; +Cc: kernel-team
In-Reply-To: <20181012185424.2378502-3-yhs@fb.com>
On 10/12/2018 08:54 PM, Yonghong Song wrote:
> This patch adds BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO
> support to the type section. BTF_KIND_FUNC_PROTO is used
> to specify the type of a function pointer. With this,
> BTF has a complete set of C types (except float).
>
> BTF_KIND_FUNC is used to specify the signature of a
> defined subprogram. BTF_KIND_FUNC_PROTO can be referenced
> by another type, e.g., a pointer type, and BTF_KIND_FUNC
> type cannot be referenced by another type.
>
> For both BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO types,
> the func return type is in t->type (where t is a
> "struct btf_type" object). The func args are an array of
> u32s immediately following object "t".
>
> As a concrete example, for the C program below,
> $ cat test.c
> int foo(int (*bar)(int)) { return bar(5); }
> with latest llvm trunk built with Debug mode, we have
> $ clang -target bpf -g -O2 -mllvm -debug-only=btf -c test.c
> Type Table:
> [1] FUNC name_off=1 info=0x0c000001 size/type=2
> param_type=3
> [2] INT name_off=11 info=0x01000000 size/type=4
> desc=0x01000020
> [3] PTR name_off=0 info=0x02000000 size/type=4
> [4] FUNC_PROTO name_off=0 info=0x0d000001 size/type=2
> param_type=2
>
> String Table:
> 0 :
> 1 : foo
> 5 : .text
> 11 : int
> 15 : test.c
> 22 : int foo(int (*bar)(int)) { return bar(5); }
>
> FuncInfo Table:
> sec_name_off=5
> insn_offset=<Omitted> type_id=1
>
> ...
>
> (Eventually we shall have bpftool to dump btf information
> like the above.)
>
> Function "foo" has a FUNC type (type_id = 1).
> The parameter of "foo" has type_id 3 which is PTR->FUNC_PROTO,
> where FUNC_PROTO refers to function pointer "bar".
Should also "bar" be part of the string table (at least at some point in future)?
Iow, if verifier hints to an issue in the program when it would for example walk
pointers and rewrite ctx access, then it could dump the var name along with it.
It might be useful as well in combination with 22 from str table, when annotating
the source. We might need support for variadic functions, though. How is LLVM
handling the latter with the recent BTF support?
> In FuncInfo Table, for section .text, the function,
> with to-be-determined offset (marked as <Omitted>),
> has type_id=1 which refers to a FUNC type.
> This way, the function signature is
> available to both kernel and user space.
> Here, the insn offset is not available during the dump time
> as relocation is resolved pretty late in the compilation process.
>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> Signed-off-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
* Re: [PATCH net] rxrpc: Fix a missing rxrpc_put_peer() in the error_report handler
From: David Miller @ 2018-10-16 6:14 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-afs, linux-kernel
In-Reply-To: <153963944115.2618.17691876338723925930.stgit@warthog.procyon.org.uk>
From: David Howells <dhowells@redhat.com>
Date: Mon, 15 Oct 2018 22:37:21 +0100
> Fix a missing call to rxrpc_put_peer() on the main path through the
> rxrpc_error_report() function. This manifests itself as a ref leak
> whenever an ICMP packet or other error comes in.
>
> In commit f334430316e7, the hand-off of the ref to a work item was removed
> and was not replaced with a put.
>
> Fixes: f334430316e7 ("rxrpc: Fix error distribution")
> Signed-off-by: David Howells <dhowells@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] sctp: use the pmtu from the icmp packet to update transport pathmtu
From: Marcelo Ricardo Leitner @ 2018-10-15 22:27 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman
In-Reply-To: <de1ad572beb97767e535b7cf60da7b1de6cbfd4f.1539604709.git.lucien.xin@gmail.com>
On Mon, Oct 15, 2018 at 07:58:29PM +0800, Xin Long wrote:
> Other than asoc pmtu sync from all transports, sctp_assoc_sync_pmtu
> is also processing transport pmtu_pending by icmp packets. But it's
> meaningless to use sctp_dst_mtu(t->dst) as new pmtu for a transport.
>
> The right pmtu value should come from the icmp packet, and it would
> be saved into transport->mtu_info in this patch and used later when
> the pmtu sync happens in sctp_sendmsg_to_asoc or sctp_packet_config.
>
> Besides, without this patch, as pmtu can only be updated correctly
> when receiving a icmp packet and no place is holding sock lock, it
> will take long time if the sock is busy with sending packets.
>
> Note that it doesn't process transport->mtu_info in .release_cb(),
> as there is no enough information for pmtu update, like for which
> asoc or transport. It is not worth traversing all asocs to check
> pmtu_pending. So unlike tcp, sctp does this in tx path, for which
> mtu_info needs to be atomic_t.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> include/net/sctp/structs.h | 2 ++
> net/sctp/associola.c | 3 ++-
> net/sctp/input.c | 1 +
> net/sctp/output.c | 6 ++++++
> 4 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index 28a7c8e..a11f937 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -876,6 +876,8 @@ struct sctp_transport {
> unsigned long sackdelay;
> __u32 sackfreq;
>
> + atomic_t mtu_info;
> +
> /* When was the last time that we heard from this transport? We use
> * this to pick new active and retran paths.
> */
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 297d9cf..a827a1f 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -1450,7 +1450,8 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
> /* Get the lowest pmtu of all the transports. */
> list_for_each_entry(t, &asoc->peer.transport_addr_list, transports) {
> if (t->pmtu_pending && t->dst) {
> - sctp_transport_update_pmtu(t, sctp_dst_mtu(t->dst));
> + sctp_transport_update_pmtu(t,
> + atomic_read(&t->mtu_info));
> t->pmtu_pending = 0;
> }
> if (!pmtu || (t->pathmtu < pmtu))
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index 9bbc5f9..5c36a99 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -395,6 +395,7 @@ void sctp_icmp_frag_needed(struct sock *sk, struct sctp_association *asoc,
> return;
>
> if (sock_owned_by_user(sk)) {
> + atomic_set(&t->mtu_info, pmtu);
> asoc->pmtu_pending = 1;
> t->pmtu_pending = 1;
> return;
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 7f849b0..67939ad 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -120,6 +120,12 @@ void sctp_packet_config(struct sctp_packet *packet, __u32 vtag,
> sctp_assoc_sync_pmtu(asoc);
> }
>
> + if (asoc->pmtu_pending) {
> + if (asoc->param_flags & SPP_PMTUD_ENABLE)
> + sctp_assoc_sync_pmtu(asoc);
> + asoc->pmtu_pending = 0;
> + }
> +
> /* If there a is a prepend chunk stick it on the list before
> * any other chunks get appended.
> */
> --
> 2.1.0
>
^ permalink raw reply
* Re: [PATCH net-next,v3] hv_netvsc: fix vf serial matching with pci slot info
From: David Miller @ 2018-10-16 5:58 UTC (permalink / raw)
To: haiyangz, haiyangz
Cc: netdev, kys, sthemmin, olaf, vkuznets, devel, linux-kernel
In-Reply-To: <20181015190615.30628-1-haiyangz@linuxonhyperv.com>
From: Haiyang Zhang <haiyangz@linuxonhyperv.com>
Date: Mon, 15 Oct 2018 19:06:15 +0000
> From: Haiyang Zhang <haiyangz@microsoft.com>
>
> The VF device's serial number is saved as a string in PCI slot's
> kobj name, not the slot->number. This patch corrects the netvsc
> driver, so the VF device can be successfully paired with synthetic
> NIC.
>
> Fixes: 00d7ddba1143 ("hv_netvsc: pair VF based on serial number")
> Reported-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Applied.
^ permalink raw reply
* [PATCH v2] net: fix warning in af_unix
From: Kyeongdon Kim @ 2018-10-16 5:57 UTC (permalink / raw)
To: davem, ktkhai, viro, garsilva, jbaron, dvlasenk, xiyou.wangcong
Cc: netdev, linux-kernel, kyeongdon.kim
This fixes the "'hash' may be used uninitialized in this function"
net/unix/af_unix.c:1041:20: warning: 'hash' may be used uninitialized in this function [-Wmaybe-uninitialized]
addr->hash = hash ^ sk->sk_type;
Signed-off-by: Kyeongdon Kim <kyeongdon.kim@lge.com>
---
net/unix/af_unix.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index d1edfa3..98d34fb 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -225,6 +225,8 @@ static inline void unix_release_addr(struct unix_address *addr)
static int unix_mkname(struct sockaddr_un *sunaddr, int len, unsigned int *hashp)
{
+ *hashp = 0;
+
if (len <= sizeof(short) || len > sizeof(*sunaddr))
return -EINVAL;
if (!sunaddr || sunaddr->sun_family != AF_UNIX)
--
2.6.2
^ permalink raw reply related
* Re: [PATCH net-next] rxrpc: Add /proc/net/rxrpc/peers to display peer list
From: David Miller @ 2018-10-16 5:53 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-afs, linux-kernel
In-Reply-To: <153959946389.27408.16748794272430961849.stgit@warthog.procyon.org.uk>
From: David Howells <dhowells@redhat.com>
Date: Mon, 15 Oct 2018 11:31:03 +0100
> Add /proc/net/rxrpc/peers to display the list of peers currently active.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH iproute2] macsec: fix off-by-one when parsing attributes
From: Sabrina Dubroca @ 2018-10-15 22:02 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, David Ahern
In-Reply-To: <20181015093658.7c590e29@xeon-e3>
2018-10-15, 09:36:58 -0700, Stephen Hemminger wrote:
> On Fri, 12 Oct 2018 17:34:12 +0200
> Sabrina Dubroca <sd@queasysnail.net> wrote:
>
> > I seem to have had a massive brainfart with uses of
> > parse_rtattr_nested(). The rtattr* array must have MAX+1 elements, and
> > the call to parse_rtattr_nested must have MAX as its bound. Let's fix
> > those.
> >
> > Fixes: b26fc590ce62 ("ip: add MACsec support")
> > Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
>
> Applied,
> How did it ever work??
I'm guessing it wrote over some other stack variables before their
first use. It worked without issue until the JSON patch.
Thanks,
--
Sabrina
^ permalink raw reply
* Re: [Potential Spoof] Re: [PATCH net-next v4] net/ncsi: Add NCSI Broadcom OEM command
From: Samuel Mendoza-Jonas @ 2018-10-16 5:46 UTC (permalink / raw)
To: Vijay Khemka, David S. Miller, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: openbmc@lists.ozlabs.org, linux-aspeed@lists.ozlabs.org
In-Reply-To: <E82A7C2E-6DBF-437D-B571-5376F0122176@fb.com>
On Mon, 2018-10-15 at 17:38 +0000, Vijay Khemka wrote:
>
> On 10/15/18, 10:27 AM, "Linux-aspeed on behalf of Vijay Khemka" <linux-aspeed-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:
>
>
>
> On 10/14/18, 8:51 PM, "Samuel Mendoza-Jonas" <sam@mendozajonas.com> wrote:
>
> On Mon, 2018-10-15 at 13:08 +1100, Samuel Mendoza-Jonas wrote:
> > On Fri, 2018-10-12 at 11:20 -0700, Vijay Khemka wrote:
> > > This patch adds OEM Broadcom commands and response handling. It also
> > > defines OEM Get MAC Address handler to get and configure the device.
> > >
> > > ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
> > > getting mac address.
> > > ncsi_rsp_handler_oem_bcm: This handles response received for all
> > > broadcom OEM commands.
> > > ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
> > > set it to device.
> > >
> > > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> > > ---
> > > v4: updated as per comment from Sam, I was just wondering if I can remove
> > > NCSI_OEM_CMD_GET_MAC config option and let this code be valid always and
> > > it will configure mac address if there is get mac address handler for given
> > > manufacture id.
> >
> > Hi Vijay,
> >
> > We can look at handling this a different way, but I don't think we want
> > to unconditionally set the system's MAC address based on the OEM GMA
> > command. If the user wants to set a custom MAC address, or in the case of
> > OpenBMC for example who have their MAC address saved in flash, this will
> > override that value with whatever the Network Controller has saved. In
> > particular as it is set up it will override any MAC address every time a
> > channel is configured, such as during a failover event.
> >
> > We *could* always send the GMA command if it is available and move the
> > decision whether to use the resulting address or not into the response
> > handler. That would simplify the ncsi_configure_channel() logic a bit.
> > Another idea may be to have a Netlink command to tell NCSI to ignore the
> > GMA result; then we could drop the config option and the system can
> > safely change the address if desired.
> >
> > Any thoughts? I'll also ping some of the OpenBMC people and see what
> > their expectations are.
>
> After a bit of a think and an ask around, to quote a colleague:
> > I think we'd want it handled (overall) like any other net device; the MAC
> > address in the device's ROM provides a default, and is overridden by anything
> > specified by userspace
>
> Which describes what I was thinking pretty well.
> So if we can have it such that the NCSI driver only sets the MAC address
> _once_, and then after then does not update it again, we should be able to call
> the OEM GMA command without hiding it behind a config option. So the first time
> a channel was configured we store and set the MAC address given, but then on
> later configure events we don't continue to update it. What do you think?
>
> Cheers,
> Sam
>
> I agree with you setting it only once. I gave a thought about config option and realize that
> we should allow user to configure it. If user wants to set mac address through device tree
> and not through ROM then we must not override mac set by device tree. So my proposal is
> setting of mac address in response should be hidden under config option. Getting mac address
> can still go without config option. Your thought?
>
> or simply guard following block under config and no other function declaration guard required.
> And set static variable flag in function " ncsi_oem_handler" for calling this only once.
>
> #if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> nca.type = NCSI_PKT_CMD_OEM;
> nca.package = np->id;
> nca.channel = nc->id;
> ndp->pending_req_num = 1;
> ret = ncsi_oem_handler(&nca, nc->version.mf_id);
> #endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
>
Hi Vijay,
Either way is likely fine; although you might need to take care you don't
get an unused function warning depending on how you guard
ncsi_oem_handler(). Also a flag in the ncsi_dev_priv struct could be
easier to keep track of than having a static variable in the handler
function.
Sam
^ permalink raw reply
* Re: [PATCH] qed: fix spelling mistake "Ireelevant" -> "Irrelevant"
From: David Miller @ 2018-10-16 5:40 UTC (permalink / raw)
To: colin.king
Cc: Ariel.Elior, everest-linux-l2, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20181013154825.6285-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Sat, 13 Oct 2018 16:48:25 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistake in DP_INFO message
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [PATCH bpf-next 01/13] bpf: btf: Break up btf_type_is_void()
From: Daniel Borkmann @ 2018-10-15 21:50 UTC (permalink / raw)
To: Yonghong Song, ast, kafai, netdev; +Cc: kernel-team
In-Reply-To: <20181012185424.2378502-2-yhs@fb.com>
On 10/12/2018 08:54 PM, Yonghong Song wrote:
> This patch breaks up btf_type_is_void() into
> btf_type_is_void() and btf_type_is_fwd().
>
> It also adds btf_type_nosize() to better describe it is
> testing a type has nosize info.
>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
Yonghong, your SoB is missing here.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net-next] octeontx2-af: remove unused cgx_fwi_link_change
From: David Miller @ 2018-10-16 5:32 UTC (permalink / raw)
To: arnd
Cc: sgoutham, lcherian, gakula, jerinj, yuehaibing, nmani, netdev,
linux-kernel
In-Reply-To: <20181012195232.4024123-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 12 Oct 2018 21:52:22 +0200
> The newly added driver causes a warning about a function that is
> not used anywhere:
>
> drivers/net/ethernet/marvell/octeontx2/af/cgx.c:320:12: error: 'cgx_fwi_link_change' defined but not used [-Werror=unused-function]
>
> Remove it for now, until a user gets added. If we want to use this
> function from another module, we also need a declaration in a header
> file, which is currently missing, so it would have to change anyway.
>
> Fixes: 1463f382f58d ("octeontx2-af: Add support for CGX link management")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: ethernet: ti: cpsw: use for mcast entries only host port
From: David Miller @ 2018-10-16 5:22 UTC (permalink / raw)
To: ivan.khoronzhuk; +Cc: grygorii.strashko, linux-omap, netdev, linux-kernel
In-Reply-To: <20181012160629.7245-1-ivan.khoronzhuk@linaro.org>
From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Date: Fri, 12 Oct 2018 19:06:29 +0300
> In dual-emac mode the cpsw driver sends directed packets, that means
> that packets go to the directed port, but an ALE lookup is performed
> to determine untagged egress only. It means that on tx side no need
> to add port bit for ALE mcast entry mask, and basically ALE entry
> for port identification is needed only on rx side.
>
> So, add only host port in dual_emac mode as used directed
> transmission, and no need in one more port. For single port boards
> and switch mode all ports used, as usual, so no changes for them.
> Also it simplifies farther changes.
>
> In other words, mcast entries for dual-emac should behave exactly
> like unicast. It also can help avoid leaking packets between ports
> with same vlan on h/w level if ports could became members of same vid.
>
> So now, for instance, if mcast address 33:33:00:00:00:01 is added then
> entries in ALE table:
>
> vid = 1, addr = 33:33:00:00:00:01, port_mask = 0x1
> vid = 2, addr = 33:33:00:00:00:01, port_mask = 0x1
>
> Instead of:
> vid = 1, addr = 33:33:00:00:00:01, port_mask = 0x3
> vid = 2, addr = 33:33:00:00:00:01, port_mask = 0x5
>
> With the same considerations, set only host port for unregistered
> mcast for dual-emac mode in case of IFF_ALLMULTI is set, exactly like
> it's done in cpsw_ale_set_allmulti().
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Applied.
^ permalink raw reply
* Re: [PATCH 0/2] net: ethernet: ti: cpsw fix mcast packet lost
From: David Miller @ 2018-10-16 5:21 UTC (permalink / raw)
To: ivan.khoronzhuk; +Cc: grygorii.strashko, linux-omap, netdev, linux-kernel
In-Reply-To: <20181012152815.31320-1-ivan.khoronzhuk@linaro.org>
From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Date: Fri, 12 Oct 2018 18:28:13 +0300
> The patchset omits redundant refresh of mcast address table and
> prevents mcast packet lost.
>
> Based on net-next/master
> tested on am572x evm
Series applied.
^ permalink raw reply
* Re: [PATCH net] rxrpc: Fix incorrect conditional on IPV6
From: David Miller @ 2018-10-16 5:20 UTC (permalink / raw)
To: dhowells; +Cc: netdev, kbuild-all, arnd, linux-afs, linux-kernel
In-Reply-To: <153935871685.8157.12127765977392104630.stgit@warthog.procyon.org.uk>
From: David Howells <dhowells@redhat.com>
Date: Fri, 12 Oct 2018 16:38:36 +0100
> The udpv6_encap_enable() function is part of the ipv6 code, and if that is
> configured as a loadable module and rxrpc is built in then a build failure
> will occur because the conditional check is wrong:
>
> net/rxrpc/local_object.o: In function `rxrpc_lookup_local':
> local_object.c:(.text+0x2688): undefined reference to `udpv6_encap_enable'
>
> Use the correct config symbol (CONFIG_AF_RXRPC_IPV6) in the conditional
> check rather than CONFIG_IPV6 as that will do the right thing.
>
> Fixes: 5271953cad31 ("rxrpc: Use the UDP encap_rcv hook")
> Reported-by: kbuild-all@01.org
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Applied.
^ permalink raw reply
* Re: [PATCH] net: fix waring in af_unix
From: David Miller @ 2018-10-16 5:16 UTC (permalink / raw)
To: kyeongdon.kim
Cc: ktkhai, viro, garsilva, jbaron, dvlasenk, xiyou.wangcong, netdev,
linux-kernel
In-Reply-To: <1539340454-67970-1-git-send-email-kyeongdon.kim@lge.com>
From: Kyeongdon Kim <kyeongdon.kim@lge.com>
Date: Fri, 12 Oct 2018 19:34:14 +0900
> This fixes the "'hash' may be used uninitialized in this function"
>
> net/unix/af_unix.c:1041:20: warning: 'hash' may be used uninitialized in this function [-Wmaybe-uninitialized]
> addr->hash = hash ^ sk->sk_type;
> ^
>
> Signed-off-by: Kyeongdon Kim <kyeongdon.kim@lge.com>
Please put the initialization at the start of unix_mkname() as a similar
situation exists elsewhere in this file, and the compiler will eventually
warn there too.
^ permalink raw reply
* Re: [PATCH net] net: bcmgenet: Poll internal PHY for GENETv5
From: David Miller @ 2018-10-16 5:11 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, opendmb, linux-kernel
In-Reply-To: <20181011220633.24450-1-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Thu, 11 Oct 2018 15:06:33 -0700
> On GENETv5, there is a hardware issue which prevents the GENET hardware
> from generating a link UP interrupt when the link is operating at
> 10Mbits/sec. Since we do not have any way to configure the link
> detection logic, fallback to polling in that case.
>
> Fixes: 421380856d9c ("net: bcmgenet: add support for the GENETv5 hardware")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net] rxrpc: Fix an uninitialised variable
From: David Miller @ 2018-10-16 5:09 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-afs, linux-kernel
In-Reply-To: <153929355195.15518.16495330917691027211.stgit@warthog.procyon.org.uk>
From: David Howells <dhowells@redhat.com>
Date: Thu, 11 Oct 2018 22:32:31 +0100
> Fix an uninitialised variable introduced by the last patch. This can cause
> a crash when a new call comes in to a local service, such as when an AFS
> fileserver calls back to the local cache manager.
>
> Fixes: c1e15b4944c9 ("rxrpc: Fix the packet reception routine")
> Signed-off-by: David Howells <dhowells@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH] ethtool: fix a missing-check bug
From: David Miller @ 2018-10-16 4:39 UTC (permalink / raw)
To: wang6495
Cc: kjlu, f.fainelli, keescook, ilyal, ecree, ynorov, alan.brady,
eugenia, stephen, netdev, linux-kernel
In-Reply-To: <1539090940-5323-1-git-send-email-wang6495@umn.edu>
From: Wenwen Wang <wang6495@umn.edu>
Date: Tue, 9 Oct 2018 08:15:38 -0500
> In ethtool_get_rxnfc(), the eth command 'cmd' is compared against
> 'ETHTOOL_GRXFH' to see whether it is necessary to adjust the variable
> 'info_size'. Then the whole structure of 'info' is copied from the
> user-space buffer 'useraddr' with 'info_size' bytes. In the following
> execution, 'info' may be copied again from the buffer 'useraddr' depending
> on the 'cmd' and the 'info.flow_type'. However, after these two copies,
> there is no check between 'cmd' and 'info.cmd'. In fact, 'cmd' is also
> copied from the buffer 'useraddr' in dev_ethtool(), which is the caller
> function of ethtool_get_rxnfc(). Given that 'useraddr' is in the user
> space, a malicious user can race to change the eth command in the buffer
> between these copies. By doing so, the attacker can supply inconsistent
> data and cause undefined behavior because in the following execution 'info'
> will be passed to ops->get_rxnfc().
>
> This patch adds a necessary check on 'info.cmd' and 'cmd' to confirm that
> they are still same after the two copies in ethtool_get_rxnfc(). Otherwise,
> an error code EINVAL will be returned.
>
> Signed-off-by: Wenwen Wang <wang6495@umn.edu>
Applied.
^ permalink raw reply
* Re: [PATCH net] r8169: Enable MSI-X on RTL8106e
From: David Miller @ 2018-10-16 4:32 UTC (permalink / raw)
To: jian-hong
Cc: hkallweit1, nic_swsd, netdev, linux-kernel, kai.heng.feng, linux
In-Reply-To: <CAPpJ_ecr-tMNcpe=--MYyK3OWJiYEAoX7AnDBN63XO1OGZ4v_w@mail.gmail.com>
From: Jian-Hong Pan <jian-hong@endlessm.com>
Date: Mon, 15 Oct 2018 16:51:12 +0800
> 2018-10-02 13:57 GMT+08:00 Jian-Hong Pan <jian-hong@endlessm.com>:
>> David Miller <davem@davemloft.net> 於 2018年10月2日 週二 下午1:51寫道：
>>>
>>> From: Jian-Hong Pan <jian-hong@endlessm.com>
>>> Date: Thu, 27 Sep 2018 12:09:48 +0800
>>>
>>> > However, there is a commit which resolves the drivers getting nothing in
>>> > PCI BAR=4 after system resumes. It is 04cb3ae895d7 "PCI: Reprogram
>>> > bridge prefetch registers on resume" by Daniel Drake.
>>>
>>> I don't see this upstream yet.
>>
>> It is in linux-next repository:
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=04cb3ae895d7efdc60f0fe17182b200a3da20f09
>
> The commit is also back ported into Linux kernel 4.19-rc7 now.
> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/?id=083874549fdfefa629dfa752785e20427dde1511
Patch applied, thanks.
^ permalink raw reply
* Re: [PATCH bpf-next 0/2] IPv6 sk-lookup fixes
From: Daniel Borkmann @ 2018-10-15 20:39 UTC (permalink / raw)
To: Joe Stringer, ast; +Cc: netdev
In-Reply-To: <20181015172746.6475-1-joe@wand.net.nz>
On 10/15/2018 07:27 PM, Joe Stringer wrote:
> This series includes a couple of fixups for the IPv6 socket lookup
> helper, to make the API more consistent (always supply all arguments in
> network byte-order) and to allow its use when IPv6 is compiled as a
> module.
>
> Joe Stringer (2):
> bpf: Allow sk_lookup with IPv6 module
> bpf: Fix IPv6 dport byte-order in bpf_sk_lookup
>
> include/net/addrconf.h | 5 +++++
> net/core/filter.c | 15 +++++++++------
> net/ipv6/af_inet6.c | 1 +
> 3 files changed, 15 insertions(+), 6 deletions(-)
>
LGTM, thanks for following up on this. Series:
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ 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