Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH bpf-next v2 2/3] bpf: add BPF_PROG_TEST_RUN support for flow dissector
From: Song Liu @ 2019-01-26  0:31 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Networking, David S . Miller, Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20190124164953.29740-3-sdf@google.com>

On Thu, Jan 24, 2019 at 8:53 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> The input is packet data, the output is struct bpf_flow_key. This should
> make it easy to test flow dissector programs without elaborate
> setup.
>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
>  include/linux/bpf.h |  3 ++
>  net/bpf/test_run.c  | 82 +++++++++++++++++++++++++++++++++++++++++++++
>  net/core/filter.c   |  1 +
>  3 files changed, 86 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index e734f163bd0b..701ef954a258 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -397,6 +397,9 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
>                           union bpf_attr __user *uattr);
>  int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>                           union bpf_attr __user *uattr);
> +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
> +                                    const union bpf_attr *kattr,
> +                                    union bpf_attr __user *uattr);
>
>  /* an array of programs to be executed under rcu_lock.
>   *
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index fa2644d276ef..2c5172b33209 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -240,3 +240,85 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
>         kfree(data);
>         return ret;
>  }
> +
> +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
> +                                    const union bpf_attr *kattr,
> +                                    union bpf_attr __user *uattr)

I think this function duplicates a lot of logic from bpf_prog_test_run_skb().
Can we somehow reuse bpf_prog_test_run_skb()?

Thanks,
Song


> +{
> +       u32 size = kattr->test.data_size_in;
> +       u32 repeat = kattr->test.repeat;
> +       struct bpf_flow_keys flow_keys;
> +       u64 time_start, time_spent = 0;
> +       struct bpf_skb_data_end *cb;
> +       u32 retval, duration;
> +       struct sk_buff *skb;
> +       struct sock *sk;
> +       void *data;
> +       int ret;
> +       u32 i;
> +
> +       if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR)
> +               return -EINVAL;
> +
> +       data = bpf_test_init(kattr, size, NET_SKB_PAD + NET_IP_ALIGN,
> +                            SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
> +       if (IS_ERR(data))
> +               return PTR_ERR(data);
> +
> +       sk = kzalloc(sizeof(*sk), GFP_USER);
> +       if (!sk) {
> +               kfree(data);
> +               return -ENOMEM;
> +       }
> +       sock_net_set(sk, current->nsproxy->net_ns);
> +       sock_init_data(NULL, sk);
> +
> +       skb = build_skb(data, 0);
> +       if (!skb) {
> +               kfree(data);
> +               kfree(sk);
> +               return -ENOMEM;
> +       }
> +       skb->sk = sk;
> +
> +       skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
> +       __skb_put(skb, size);
> +       skb->protocol = eth_type_trans(skb,
> +                                      current->nsproxy->net_ns->loopback_dev);
> +       skb_reset_network_header(skb);
> +
> +       cb = (struct bpf_skb_data_end *)skb->cb;
> +       cb->qdisc_cb.flow_keys = &flow_keys;
> +
> +       if (!repeat)
> +               repeat = 1;
> +
> +       time_start = ktime_get_ns();
> +       for (i = 0; i < repeat; i++) {
> +               preempt_disable();
> +               rcu_read_lock();
> +               retval = __skb_flow_bpf_dissect(prog, skb,
> +                                               &flow_keys_dissector,
> +                                               &flow_keys);
> +               rcu_read_unlock();
> +               preempt_enable();
> +
> +               if (need_resched()) {
> +                       if (signal_pending(current))
> +                               break;
> +                       time_spent += ktime_get_ns() - time_start;
> +                       cond_resched();
> +                       time_start = ktime_get_ns();
> +               }
> +       }
> +       time_spent += ktime_get_ns() - time_start;
> +       do_div(time_spent, repeat);
> +       duration = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
> +
> +       ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys),
> +                             retval, duration);
> +
> +       kfree_skb(skb);
> +       kfree(sk);
> +       return ret;
> +}
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 2b3b436ef545..ff4641dae2be 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -7690,6 +7690,7 @@ const struct bpf_verifier_ops flow_dissector_verifier_ops = {
>  };
>
>  const struct bpf_prog_ops flow_dissector_prog_ops = {
> +       .test_run               = bpf_prog_test_run_flow_dissector,
>  };
>
>  int sk_detach_filter(struct sock *sk)
> --
> 2.20.1.321.g9e740568ce-goog
>

^ permalink raw reply

* Re: [PATCH bpf-next v3 03/16] bpf: verifier support JMP32
From: Daniel Borkmann @ 2019-01-26  0:36 UTC (permalink / raw)
  To: Jiong Wang, ast; +Cc: netdev, oss-drivers
In-Reply-To: <1548375028-8308-4-git-send-email-jiong.wang@netronome.com>

On 01/25/2019 01:10 AM, Jiong Wang wrote:
> This patch teach verifier about the new BPF_JMP32 instruction class.
> Verifier need to treat it similar as the existing BPF_JMP class.
> A BPF_JMP32 insn needs to go through all checks that have been done on
> BPF_JMP.
> 
> Also, verifier is doing runtime optimizations based on the extra info
> conditional jump instruction could offer, especially when the comparison is
> between constant and register that the value range of the register could be
> improved based on the comparison results. These code are updated
> accordingly.
> 
> Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Signed-off-by: Jiong Wang <jiong.wang@netronome.com>

Series looks good to me, but if I spot this correctly one thing that has
not been addressed here is proper rebase on top of Jakub's dead code
removal, e.g. in opt_hard_wire_dead_code_branches() where we check in
insn_is_cond_jump() for jump opcodes it still only tests for BPF_JMP
class whereas BPF_JMP32 handling needs to be taught here as well.

Thanks,
Daniel

^ permalink raw reply

* Re: [PATCH bpf-next v2 3/3] selftests/bpf: add simple BPF_PROG_TEST_RUN examples for flow dissector
From: Song Liu @ 2019-01-26  0:40 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Networking, David S . Miller, Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20190124164953.29740-4-sdf@google.com>

On Thu, Jan 24, 2019 at 8:52 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> Use existing pkt_v4 and pkt_v6 to make sure flow_keys are what we want.
>
> Also, add new bpf_flow_load routine (and flow_dissector_load.h header)
> that loads bpf_flow.o program and does all required setup.
>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
>  tools/testing/selftests/bpf/Makefile          |  3 +
>  .../selftests/bpf/flow_dissector_load.c       | 43 ++--------
>  .../selftests/bpf/flow_dissector_load.h       | 55 +++++++++++++
>  tools/testing/selftests/bpf/test_progs.c      | 78 ++++++++++++++++++-
>  4 files changed, 139 insertions(+), 40 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/flow_dissector_load.h
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 70229de510f5..ade67470a152 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -19,6 +19,9 @@ all: $(TEST_CUSTOM_PROGS)
>  $(TEST_CUSTOM_PROGS): $(OUTPUT)/%: %.c
>         $(CC) -o $(TEST_CUSTOM_PROGS) -static $< -Wl,--build-id
>
> +flow_dissector_load.c: flow_dissector_load.h
> +test_run.c: flow_dissector_load.h
> +
>  # Order correspond to 'make run_tests' order
>  TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
>         test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \
> diff --git a/tools/testing/selftests/bpf/flow_dissector_load.c b/tools/testing/selftests/bpf/flow_dissector_load.c
> index ae8180b11d5f..77cafa66d048 100644
> --- a/tools/testing/selftests/bpf/flow_dissector_load.c
> +++ b/tools/testing/selftests/bpf/flow_dissector_load.c
> @@ -12,6 +12,7 @@
>  #include <bpf/libbpf.h>
>
>  #include "bpf_rlimit.h"
> +#include "flow_dissector_load.h"
>
>  const char *cfg_pin_path = "/sys/fs/bpf/flow_dissector";
>  const char *cfg_map_name = "jmp_table";
> @@ -21,46 +22,13 @@ char *cfg_path_name;
>
>  static void load_and_attach_program(void)
>  {
> -       struct bpf_program *prog, *main_prog;
> -       struct bpf_map *prog_array;
> -       int i, fd, prog_fd, ret;
> +       int prog_fd, ret;
>         struct bpf_object *obj;
> -       int prog_array_fd;
>
> -       ret = bpf_prog_load(cfg_path_name, BPF_PROG_TYPE_FLOW_DISSECTOR, &obj,
> -                           &prog_fd);
> +       ret = bpf_flow_load(&obj, cfg_path_name, cfg_section_name,
> +                           cfg_map_name, &prog_fd);
>         if (ret)
> -               error(1, 0, "bpf_prog_load %s", cfg_path_name);
> -
> -       main_prog = bpf_object__find_program_by_title(obj, cfg_section_name);
> -       if (!main_prog)
> -               error(1, 0, "bpf_object__find_program_by_title %s",
> -                     cfg_section_name);
> -
> -       prog_fd = bpf_program__fd(main_prog);
> -       if (prog_fd < 0)
> -               error(1, 0, "bpf_program__fd");
> -
> -       prog_array = bpf_object__find_map_by_name(obj, cfg_map_name);
> -       if (!prog_array)
> -               error(1, 0, "bpf_object__find_map_by_name %s", cfg_map_name);
> -
> -       prog_array_fd = bpf_map__fd(prog_array);
> -       if (prog_array_fd < 0)
> -               error(1, 0, "bpf_map__fd %s", cfg_map_name);
> -
> -       i = 0;
> -       bpf_object__for_each_program(prog, obj) {
> -               fd = bpf_program__fd(prog);
> -               if (fd < 0)
> -                       error(1, 0, "bpf_program__fd");
> -
> -               if (fd != prog_fd) {
> -                       printf("%d: %s\n", i, bpf_program__title(prog, false));
> -                       bpf_map_update_elem(prog_array_fd, &i, &fd, BPF_ANY);
> -                       ++i;
> -               }
> -       }
> +               error(1, 0, "bpf_flow_load %s", cfg_path_name);
>
>         ret = bpf_prog_attach(prog_fd, 0 /* Ignore */, BPF_FLOW_DISSECTOR, 0);
>         if (ret)
> @@ -69,7 +37,6 @@ static void load_and_attach_program(void)
>         ret = bpf_object__pin(obj, cfg_pin_path);
>         if (ret)
>                 error(1, 0, "bpf_object__pin %s", cfg_pin_path);
> -
>  }
>
>  static void detach_program(void)
> diff --git a/tools/testing/selftests/bpf/flow_dissector_load.h b/tools/testing/selftests/bpf/flow_dissector_load.h
> new file mode 100644
> index 000000000000..41dd6959feb0
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/flow_dissector_load.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +#ifndef FLOW_DISSECTOR_LOAD
> +#define FLOW_DISSECTOR_LOAD
> +
> +#include <bpf/bpf.h>
> +#include <bpf/libbpf.h>
> +
> +static inline int bpf_flow_load(struct bpf_object **obj,
> +                               const char *path,
> +                               const char *section_name,
> +                               const char *map_name,
> +                               int *prog_fd)
> +{
> +       struct bpf_program *prog, *main_prog;
> +       struct bpf_map *prog_array;
> +       int prog_array_fd;
> +       int ret, fd, i;
> +
> +       ret = bpf_prog_load(path, BPF_PROG_TYPE_FLOW_DISSECTOR, obj,
> +                           prog_fd);
> +       if (ret)
> +               return ret;
> +
> +       main_prog = bpf_object__find_program_by_title(*obj, section_name);
> +       if (!main_prog)
> +               return ret;
> +
> +       *prog_fd = bpf_program__fd(main_prog);
> +       if (*prog_fd < 0)
> +               return ret;
> +
> +       prog_array = bpf_object__find_map_by_name(*obj, map_name);
> +       if (!prog_array)
> +               return ret;
> +
> +       prog_array_fd = bpf_map__fd(prog_array);
> +       if (prog_array_fd < 0)
> +               return ret;
> +
> +       i = 0;
> +       bpf_object__for_each_program(prog, *obj) {
> +               fd = bpf_program__fd(prog);
> +               if (fd < 0)
> +                       return fd;
> +
> +               if (fd != *prog_fd) {
> +                       bpf_map_update_elem(prog_array_fd, &i, &fd, BPF_ANY);
> +                       ++i;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +#endif /* FLOW_DISSECTOR_LOAD */
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 126fc624290d..5f46680d4ad4 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -39,6 +39,7 @@ typedef __u16 __sum16;
>  #include "bpf_endian.h"
>  #include "bpf_rlimit.h"
>  #include "trace_helpers.h"
> +#include "flow_dissector_load.h"
>
>  static int error_cnt, pass_cnt;
>  static bool jit_enabled;
> @@ -53,9 +54,10 @@ static struct {
>  } __packed pkt_v4 = {
>         .eth.h_proto = __bpf_constant_htons(ETH_P_IP),
>         .iph.ihl = 5,
> -       .iph.protocol = 6,
> +       .iph.protocol = IPPROTO_TCP,
>         .iph.tot_len = __bpf_constant_htons(MAGIC_BYTES),
>         .tcp.urg_ptr = 123,
> +       .tcp.doff = 5,
>  };
>
>  /* ipv6 test vector */
> @@ -65,9 +67,10 @@ static struct {
>         struct tcphdr tcp;
>  } __packed pkt_v6 = {
>         .eth.h_proto = __bpf_constant_htons(ETH_P_IPV6),
> -       .iph.nexthdr = 6,
> +       .iph.nexthdr = IPPROTO_TCP,
>         .iph.payload_len = __bpf_constant_htons(MAGIC_BYTES),
>         .tcp.urg_ptr = 123,
> +       .tcp.doff = 5,
>  };

Why do we need these changes? They seem unrelated to me.

Thanks,
Song


>
>  #define _CHECK(condition, tag, duration, format...) ({                 \
> @@ -1882,6 +1885,76 @@ static void test_queue_stack_map(int type)
>         bpf_object__close(obj);
>  }
>
> +#define CHECK_FLOW_KEYS(desc, got, expected)                           \
> +       CHECK(memcmp(&got, &expected, sizeof(got)) != 0,                \
> +             desc,                                                     \
> +             "nhoff=%u/%u "                                            \
> +             "thoff=%u/%u "                                            \
> +             "addr_proto=0x%x/0x%x "                                   \
> +             "is_frag=%u/%u "                                          \
> +             "is_first_frag=%u/%u "                                    \
> +             "is_encap=%u/%u "                                         \
> +             "n_proto=0x%x/0x%x "                                      \
> +             "sport=%u/%u "                                            \
> +             "dport=%u/%u\n",                                          \
> +             got.nhoff, expected.nhoff,                                \
> +             got.thoff, expected.thoff,                                \
> +             got.addr_proto, expected.addr_proto,                      \
> +             got.is_frag, expected.is_frag,                            \
> +             got.is_first_frag, expected.is_first_frag,                \
> +             got.is_encap, expected.is_encap,                          \
> +             got.n_proto, expected.n_proto,                            \
> +             got.sport, expected.sport,                                \
> +             got.dport, expected.dport)
> +
> +static struct bpf_flow_keys pkt_v4_flow_keys = {
> +       .nhoff = 0,
> +       .thoff = sizeof(struct iphdr),
> +       .addr_proto = ETH_P_IP,
> +       .ip_proto = IPPROTO_TCP,
> +       .n_proto = bpf_htons(ETH_P_IP),
> +};
> +
> +static struct bpf_flow_keys pkt_v6_flow_keys = {
> +       .nhoff = 0,
> +       .thoff = sizeof(struct ipv6hdr),
> +       .addr_proto = ETH_P_IPV6,
> +       .ip_proto = IPPROTO_TCP,
> +       .n_proto = bpf_htons(ETH_P_IPV6),
> +};
> +
> +static void test_flow_dissector(void)
> +{
> +       struct bpf_flow_keys flow_keys;
> +       struct bpf_object *obj;
> +       __u32 duration, retval;
> +       int err, prog_fd;
> +       __u32 size;
> +
> +       err = bpf_flow_load(&obj, "./bpf_flow.o", "flow_dissector",
> +                           "jmp_table", &prog_fd);
> +       if (err) {
> +               error_cnt++;
> +               return;
> +       }
> +
> +       err = bpf_prog_test_run(prog_fd, 10, &pkt_v4, sizeof(pkt_v4),
> +                               &flow_keys, &size, &retval, &duration);
> +       CHECK(size != sizeof(flow_keys) || err || retval != 1, "ipv4",
> +             "err %d errno %d retval %d duration %d size %u/%lu\n",
> +             err, errno, retval, duration, size, sizeof(flow_keys));
> +       CHECK_FLOW_KEYS("ipv4_flow_keys", flow_keys, pkt_v4_flow_keys);
> +
> +       err = bpf_prog_test_run(prog_fd, 10, &pkt_v6, sizeof(pkt_v6),
> +                               &flow_keys, &size, &retval, &duration);
> +       CHECK(size != sizeof(flow_keys) || err || retval != 1, "ipv6",
> +             "err %d errno %d retval %d duration %d size %u/%lu\n",
> +             err, errno, retval, duration, size, sizeof(flow_keys));
> +       CHECK_FLOW_KEYS("ipv6_flow_keys", flow_keys, pkt_v6_flow_keys);
> +
> +       bpf_object__close(obj);
> +}
> +
>  int main(void)
>  {
>         srand(time(NULL));
> @@ -1909,6 +1982,7 @@ int main(void)
>         test_reference_tracking();
>         test_queue_stack_map(QUEUE);
>         test_queue_stack_map(STACK);
> +       test_flow_dissector();
>
>         printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
>         return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
> --
> 2.20.1.321.g9e740568ce-goog
>

^ permalink raw reply

* Re: [PATCH v4 bpf-next 1/9] bpf: introduce bpf_spin_lock
From: Jann Horn @ 2019-01-26  0:43 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Paul E. McKenney, Peter Zijlstra, Alexei Starovoitov,
	David S. Miller, Daniel Borkmann, jakub.kicinski,
	Network Development, kernel-team, Ingo Molnar, Will Deacon
In-Reply-To: <20190125234403.iisj5woztm4afwgh@ast-mbp.dhcp.thefacebook.com>

On Sat, Jan 26, 2019 at 12:44 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Jan 25, 2019 at 02:51:12PM -0800, Paul E. McKenney wrote:
> > > >
> > > > So no more than (say) 100 milliseconds?
> > >
> > > Depends on RLIMIT_MEMLOCK and on how hard userspace is trying to make
> > > things slow, I guess - if userspace manages to create a hashtable,
> > > with a few dozen megabytes in size, with worst-case assignment of
> > > elements to buckets (everything in a single bucket), every lookup call
> > > on that bucket becomes a linked list traversal through a list that
> > > must be stored in main memory because it's too big for the CPU caches.
> > > I don't know into how much time that translates.
> >
> > So perhaps you have a candidate BPF program for the RCU CPU stall warning
> > challenge, then.  ;-)
>
> I'd like to see one that can defeat jhash + random seed.

Assuming that the map isn't created by root with BPF_F_ZERO_SEED:

The dumb approach would be to put things into the map, try to measure
via timing/sidechannel whether you got collisions, and then keep
trying different keys, and keep them if the timing indicates a
collision. That'd probably be pretty slow and annoying though. Two
years ago, I implemented something similar to leak information about
virtual addresses from Firefox by measuring hash bucket collisions
from JavaScript (but to be fair, it was easier there because you can
resize the hash table):
https://thejh.net/misc/firefox-cve-2016-9904-and-cve-2017-5378-bugreport

But I think there's an easier way, too: The jhash seed is just 32
bits, and AFAICS the BPF API leaks information about that seed through
BPF_MAP_GET_NEXT_KEY: Stuff two random keys into the hash table, run
BPF_MAP_GET_NEXT_KEY with attr->key==NULL, and see which key is
returned. Do that around 32 times, and you should have roughly enough
information to bruteforce the jhash seed? Recovering the seed should
then be relatively quick, 2^32 iterations of a fast hash don't take
terribly long.

That said, I don't think this is interesting enough to spend the time
necessary to implement it. :P

^ permalink raw reply

* Re: [PATCH bpf-next v2 2/3] bpf: add BPF_PROG_TEST_RUN support for flow dissector
From: Stanislav Fomichev @ 2019-01-26  0:56 UTC (permalink / raw)
  To: Song Liu
  Cc: Stanislav Fomichev, Networking, David S . Miller,
	Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <CAPhsuW66rSfVgEs+C3wKMyFVzxx7gNfSnfvHQ-nQ9EX7Agkdpg@mail.gmail.com>

On 01/25, Song Liu wrote:
> On Thu, Jan 24, 2019 at 8:53 AM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > The input is packet data, the output is struct bpf_flow_key. This should
> > make it easy to test flow dissector programs without elaborate
> > setup.
> >
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> >  include/linux/bpf.h |  3 ++
> >  net/bpf/test_run.c  | 82 +++++++++++++++++++++++++++++++++++++++++++++
> >  net/core/filter.c   |  1 +
> >  3 files changed, 86 insertions(+)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index e734f163bd0b..701ef954a258 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -397,6 +397,9 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
> >                           union bpf_attr __user *uattr);
> >  int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> >                           union bpf_attr __user *uattr);
> > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
> > +                                    const union bpf_attr *kattr,
> > +                                    union bpf_attr __user *uattr);
> >
> >  /* an array of programs to be executed under rcu_lock.
> >   *
> > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> > index fa2644d276ef..2c5172b33209 100644
> > --- a/net/bpf/test_run.c
> > +++ b/net/bpf/test_run.c
> > @@ -240,3 +240,85 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
> >         kfree(data);
> >         return ret;
> >  }
> > +
> > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
> > +                                    const union bpf_attr *kattr,
> > +                                    union bpf_attr __user *uattr)
> 
> I think this function duplicates a lot of logic from bpf_prog_test_run_skb().
> Can we somehow reuse bpf_prog_test_run_skb()?
I did that initially
(https://marc.info/?l=linux-netdev&m=154830227529929&w=2), but then
Alexey suggested that there is not much to reuse (plus can hinder the
test performance of the existing types).
> 
> Thanks,
> Song
> 
> 
> > +{
> > +       u32 size = kattr->test.data_size_in;
> > +       u32 repeat = kattr->test.repeat;
> > +       struct bpf_flow_keys flow_keys;
> > +       u64 time_start, time_spent = 0;
> > +       struct bpf_skb_data_end *cb;
> > +       u32 retval, duration;
> > +       struct sk_buff *skb;
> > +       struct sock *sk;
> > +       void *data;
> > +       int ret;
> > +       u32 i;
> > +
> > +       if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR)
> > +               return -EINVAL;
> > +
> > +       data = bpf_test_init(kattr, size, NET_SKB_PAD + NET_IP_ALIGN,
> > +                            SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
> > +       if (IS_ERR(data))
> > +               return PTR_ERR(data);
> > +
> > +       sk = kzalloc(sizeof(*sk), GFP_USER);
> > +       if (!sk) {
> > +               kfree(data);
> > +               return -ENOMEM;
> > +       }
> > +       sock_net_set(sk, current->nsproxy->net_ns);
> > +       sock_init_data(NULL, sk);
> > +
> > +       skb = build_skb(data, 0);
> > +       if (!skb) {
> > +               kfree(data);
> > +               kfree(sk);
> > +               return -ENOMEM;
> > +       }
> > +       skb->sk = sk;
> > +
> > +       skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
> > +       __skb_put(skb, size);
> > +       skb->protocol = eth_type_trans(skb,
> > +                                      current->nsproxy->net_ns->loopback_dev);
> > +       skb_reset_network_header(skb);
> > +
> > +       cb = (struct bpf_skb_data_end *)skb->cb;
> > +       cb->qdisc_cb.flow_keys = &flow_keys;
> > +
> > +       if (!repeat)
> > +               repeat = 1;
> > +
> > +       time_start = ktime_get_ns();
> > +       for (i = 0; i < repeat; i++) {
> > +               preempt_disable();
> > +               rcu_read_lock();
> > +               retval = __skb_flow_bpf_dissect(prog, skb,
> > +                                               &flow_keys_dissector,
> > +                                               &flow_keys);
> > +               rcu_read_unlock();
> > +               preempt_enable();
> > +
> > +               if (need_resched()) {
> > +                       if (signal_pending(current))
> > +                               break;
> > +                       time_spent += ktime_get_ns() - time_start;
> > +                       cond_resched();
> > +                       time_start = ktime_get_ns();
> > +               }
> > +       }
> > +       time_spent += ktime_get_ns() - time_start;
> > +       do_div(time_spent, repeat);
> > +       duration = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
> > +
> > +       ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys),
> > +                             retval, duration);
> > +
> > +       kfree_skb(skb);
> > +       kfree(sk);
> > +       return ret;
> > +}
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index 2b3b436ef545..ff4641dae2be 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -7690,6 +7690,7 @@ const struct bpf_verifier_ops flow_dissector_verifier_ops = {
> >  };
> >
> >  const struct bpf_prog_ops flow_dissector_prog_ops = {
> > +       .test_run               = bpf_prog_test_run_flow_dissector,
> >  };
> >
> >  int sk_detach_filter(struct sock *sk)
> > --
> > 2.20.1.321.g9e740568ce-goog
> >

^ permalink raw reply

* Re: [PATCH bpf-next v2 3/3] selftests/bpf: add simple BPF_PROG_TEST_RUN examples for flow dissector
From: Stanislav Fomichev @ 2019-01-26  0:58 UTC (permalink / raw)
  To: Song Liu
  Cc: Stanislav Fomichev, Networking, David S . Miller,
	Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <CAPhsuW4H0beVbEpqEaSk6dp1T0Lo4+FnfA2t1+k4kOLg+5=r8w@mail.gmail.com>

On 01/25, Song Liu wrote:
> On Thu, Jan 24, 2019 at 8:52 AM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > Use existing pkt_v4 and pkt_v6 to make sure flow_keys are what we want.
> >
> > Also, add new bpf_flow_load routine (and flow_dissector_load.h header)
> > that loads bpf_flow.o program and does all required setup.
> >
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> >  tools/testing/selftests/bpf/Makefile          |  3 +
> >  .../selftests/bpf/flow_dissector_load.c       | 43 ++--------
> >  .../selftests/bpf/flow_dissector_load.h       | 55 +++++++++++++
> >  tools/testing/selftests/bpf/test_progs.c      | 78 ++++++++++++++++++-
> >  4 files changed, 139 insertions(+), 40 deletions(-)
> >  create mode 100644 tools/testing/selftests/bpf/flow_dissector_load.h
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 70229de510f5..ade67470a152 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -19,6 +19,9 @@ all: $(TEST_CUSTOM_PROGS)
> >  $(TEST_CUSTOM_PROGS): $(OUTPUT)/%: %.c
> >         $(CC) -o $(TEST_CUSTOM_PROGS) -static $< -Wl,--build-id
> >
> > +flow_dissector_load.c: flow_dissector_load.h
> > +test_run.c: flow_dissector_load.h
> > +
> >  # Order correspond to 'make run_tests' order
> >  TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
> >         test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \
> > diff --git a/tools/testing/selftests/bpf/flow_dissector_load.c b/tools/testing/selftests/bpf/flow_dissector_load.c
> > index ae8180b11d5f..77cafa66d048 100644
> > --- a/tools/testing/selftests/bpf/flow_dissector_load.c
> > +++ b/tools/testing/selftests/bpf/flow_dissector_load.c
> > @@ -12,6 +12,7 @@
> >  #include <bpf/libbpf.h>
> >
> >  #include "bpf_rlimit.h"
> > +#include "flow_dissector_load.h"
> >
> >  const char *cfg_pin_path = "/sys/fs/bpf/flow_dissector";
> >  const char *cfg_map_name = "jmp_table";
> > @@ -21,46 +22,13 @@ char *cfg_path_name;
> >
> >  static void load_and_attach_program(void)
> >  {
> > -       struct bpf_program *prog, *main_prog;
> > -       struct bpf_map *prog_array;
> > -       int i, fd, prog_fd, ret;
> > +       int prog_fd, ret;
> >         struct bpf_object *obj;
> > -       int prog_array_fd;
> >
> > -       ret = bpf_prog_load(cfg_path_name, BPF_PROG_TYPE_FLOW_DISSECTOR, &obj,
> > -                           &prog_fd);
> > +       ret = bpf_flow_load(&obj, cfg_path_name, cfg_section_name,
> > +                           cfg_map_name, &prog_fd);
> >         if (ret)
> > -               error(1, 0, "bpf_prog_load %s", cfg_path_name);
> > -
> > -       main_prog = bpf_object__find_program_by_title(obj, cfg_section_name);
> > -       if (!main_prog)
> > -               error(1, 0, "bpf_object__find_program_by_title %s",
> > -                     cfg_section_name);
> > -
> > -       prog_fd = bpf_program__fd(main_prog);
> > -       if (prog_fd < 0)
> > -               error(1, 0, "bpf_program__fd");
> > -
> > -       prog_array = bpf_object__find_map_by_name(obj, cfg_map_name);
> > -       if (!prog_array)
> > -               error(1, 0, "bpf_object__find_map_by_name %s", cfg_map_name);
> > -
> > -       prog_array_fd = bpf_map__fd(prog_array);
> > -       if (prog_array_fd < 0)
> > -               error(1, 0, "bpf_map__fd %s", cfg_map_name);
> > -
> > -       i = 0;
> > -       bpf_object__for_each_program(prog, obj) {
> > -               fd = bpf_program__fd(prog);
> > -               if (fd < 0)
> > -                       error(1, 0, "bpf_program__fd");
> > -
> > -               if (fd != prog_fd) {
> > -                       printf("%d: %s\n", i, bpf_program__title(prog, false));
> > -                       bpf_map_update_elem(prog_array_fd, &i, &fd, BPF_ANY);
> > -                       ++i;
> > -               }
> > -       }
> > +               error(1, 0, "bpf_flow_load %s", cfg_path_name);
> >
> >         ret = bpf_prog_attach(prog_fd, 0 /* Ignore */, BPF_FLOW_DISSECTOR, 0);
> >         if (ret)
> > @@ -69,7 +37,6 @@ static void load_and_attach_program(void)
> >         ret = bpf_object__pin(obj, cfg_pin_path);
> >         if (ret)
> >                 error(1, 0, "bpf_object__pin %s", cfg_pin_path);
> > -
> >  }
> >
> >  static void detach_program(void)
> > diff --git a/tools/testing/selftests/bpf/flow_dissector_load.h b/tools/testing/selftests/bpf/flow_dissector_load.h
> > new file mode 100644
> > index 000000000000..41dd6959feb0
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/flow_dissector_load.h
> > @@ -0,0 +1,55 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > +#ifndef FLOW_DISSECTOR_LOAD
> > +#define FLOW_DISSECTOR_LOAD
> > +
> > +#include <bpf/bpf.h>
> > +#include <bpf/libbpf.h>
> > +
> > +static inline int bpf_flow_load(struct bpf_object **obj,
> > +                               const char *path,
> > +                               const char *section_name,
> > +                               const char *map_name,
> > +                               int *prog_fd)
> > +{
> > +       struct bpf_program *prog, *main_prog;
> > +       struct bpf_map *prog_array;
> > +       int prog_array_fd;
> > +       int ret, fd, i;
> > +
> > +       ret = bpf_prog_load(path, BPF_PROG_TYPE_FLOW_DISSECTOR, obj,
> > +                           prog_fd);
> > +       if (ret)
> > +               return ret;
> > +
> > +       main_prog = bpf_object__find_program_by_title(*obj, section_name);
> > +       if (!main_prog)
> > +               return ret;
> > +
> > +       *prog_fd = bpf_program__fd(main_prog);
> > +       if (*prog_fd < 0)
> > +               return ret;
> > +
> > +       prog_array = bpf_object__find_map_by_name(*obj, map_name);
> > +       if (!prog_array)
> > +               return ret;
> > +
> > +       prog_array_fd = bpf_map__fd(prog_array);
> > +       if (prog_array_fd < 0)
> > +               return ret;
> > +
> > +       i = 0;
> > +       bpf_object__for_each_program(prog, *obj) {
> > +               fd = bpf_program__fd(prog);
> > +               if (fd < 0)
> > +                       return fd;
> > +
> > +               if (fd != *prog_fd) {
> > +                       bpf_map_update_elem(prog_array_fd, &i, &fd, BPF_ANY);
> > +                       ++i;
> > +               }
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +#endif /* FLOW_DISSECTOR_LOAD */
> > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> > index 126fc624290d..5f46680d4ad4 100644
> > --- a/tools/testing/selftests/bpf/test_progs.c
> > +++ b/tools/testing/selftests/bpf/test_progs.c
> > @@ -39,6 +39,7 @@ typedef __u16 __sum16;
> >  #include "bpf_endian.h"
> >  #include "bpf_rlimit.h"
> >  #include "trace_helpers.h"
> > +#include "flow_dissector_load.h"
> >
> >  static int error_cnt, pass_cnt;
> >  static bool jit_enabled;
> > @@ -53,9 +54,10 @@ static struct {
> >  } __packed pkt_v4 = {
> >         .eth.h_proto = __bpf_constant_htons(ETH_P_IP),
> >         .iph.ihl = 5,
> > -       .iph.protocol = 6,
> > +       .iph.protocol = IPPROTO_TCP,
> >         .iph.tot_len = __bpf_constant_htons(MAGIC_BYTES),
> >         .tcp.urg_ptr = 123,
> > +       .tcp.doff = 5,
> >  };
> >
> >  /* ipv6 test vector */
> > @@ -65,9 +67,10 @@ static struct {
> >         struct tcphdr tcp;
> >  } __packed pkt_v6 = {
> >         .eth.h_proto = __bpf_constant_htons(ETH_P_IPV6),
> > -       .iph.nexthdr = 6,
> > +       .iph.nexthdr = IPPROTO_TCP,
> >         .iph.payload_len = __bpf_constant_htons(MAGIC_BYTES),
> >         .tcp.urg_ptr = 123,
> > +       .tcp.doff = 5,
> >  };
> 
> Why do we need these changes? They seem unrelated to me.
doff = 5 is required for the current flow dissector program
implementation (bpf_flow.c), without it it doesn't return success:
https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/tools/testing/selftests/bpf/bpf_flow.c#n188

6 to IPPROTO_TCP just to look nice, since I'm already touching it.

> 
> Thanks,
> Song
> 
> 
> >
> >  #define _CHECK(condition, tag, duration, format...) ({                 \
> > @@ -1882,6 +1885,76 @@ static void test_queue_stack_map(int type)
> >         bpf_object__close(obj);
> >  }
> >
> > +#define CHECK_FLOW_KEYS(desc, got, expected)                           \
> > +       CHECK(memcmp(&got, &expected, sizeof(got)) != 0,                \
> > +             desc,                                                     \
> > +             "nhoff=%u/%u "                                            \
> > +             "thoff=%u/%u "                                            \
> > +             "addr_proto=0x%x/0x%x "                                   \
> > +             "is_frag=%u/%u "                                          \
> > +             "is_first_frag=%u/%u "                                    \
> > +             "is_encap=%u/%u "                                         \
> > +             "n_proto=0x%x/0x%x "                                      \
> > +             "sport=%u/%u "                                            \
> > +             "dport=%u/%u\n",                                          \
> > +             got.nhoff, expected.nhoff,                                \
> > +             got.thoff, expected.thoff,                                \
> > +             got.addr_proto, expected.addr_proto,                      \
> > +             got.is_frag, expected.is_frag,                            \
> > +             got.is_first_frag, expected.is_first_frag,                \
> > +             got.is_encap, expected.is_encap,                          \
> > +             got.n_proto, expected.n_proto,                            \
> > +             got.sport, expected.sport,                                \
> > +             got.dport, expected.dport)
> > +
> > +static struct bpf_flow_keys pkt_v4_flow_keys = {
> > +       .nhoff = 0,
> > +       .thoff = sizeof(struct iphdr),
> > +       .addr_proto = ETH_P_IP,
> > +       .ip_proto = IPPROTO_TCP,
> > +       .n_proto = bpf_htons(ETH_P_IP),
> > +};
> > +
> > +static struct bpf_flow_keys pkt_v6_flow_keys = {
> > +       .nhoff = 0,
> > +       .thoff = sizeof(struct ipv6hdr),
> > +       .addr_proto = ETH_P_IPV6,
> > +       .ip_proto = IPPROTO_TCP,
> > +       .n_proto = bpf_htons(ETH_P_IPV6),
> > +};
> > +
> > +static void test_flow_dissector(void)
> > +{
> > +       struct bpf_flow_keys flow_keys;
> > +       struct bpf_object *obj;
> > +       __u32 duration, retval;
> > +       int err, prog_fd;
> > +       __u32 size;
> > +
> > +       err = bpf_flow_load(&obj, "./bpf_flow.o", "flow_dissector",
> > +                           "jmp_table", &prog_fd);
> > +       if (err) {
> > +               error_cnt++;
> > +               return;
> > +       }
> > +
> > +       err = bpf_prog_test_run(prog_fd, 10, &pkt_v4, sizeof(pkt_v4),
> > +                               &flow_keys, &size, &retval, &duration);
> > +       CHECK(size != sizeof(flow_keys) || err || retval != 1, "ipv4",
> > +             "err %d errno %d retval %d duration %d size %u/%lu\n",
> > +             err, errno, retval, duration, size, sizeof(flow_keys));
> > +       CHECK_FLOW_KEYS("ipv4_flow_keys", flow_keys, pkt_v4_flow_keys);
> > +
> > +       err = bpf_prog_test_run(prog_fd, 10, &pkt_v6, sizeof(pkt_v6),
> > +                               &flow_keys, &size, &retval, &duration);
> > +       CHECK(size != sizeof(flow_keys) || err || retval != 1, "ipv6",
> > +             "err %d errno %d retval %d duration %d size %u/%lu\n",
> > +             err, errno, retval, duration, size, sizeof(flow_keys));
> > +       CHECK_FLOW_KEYS("ipv6_flow_keys", flow_keys, pkt_v6_flow_keys);
> > +
> > +       bpf_object__close(obj);
> > +}
> > +
> >  int main(void)
> >  {
> >         srand(time(NULL));
> > @@ -1909,6 +1982,7 @@ int main(void)
> >         test_reference_tracking();
> >         test_queue_stack_map(QUEUE);
> >         test_queue_stack_map(STACK);
> > +       test_flow_dissector();
> >
> >         printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
> >         return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
> > --
> > 2.20.1.321.g9e740568ce-goog
> >

^ permalink raw reply

* Re: [PATCH v4 bpf-next 1/9] bpf: introduce bpf_spin_lock
From: Jann Horn @ 2019-01-26  0:59 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Paul E. McKenney, Peter Zijlstra, Alexei Starovoitov,
	David S. Miller, Daniel Borkmann, jakub.kicinski,
	Network Development, kernel-team, Ingo Molnar, Will Deacon
In-Reply-To: <CAG48ez1dA8_P9iKE4wqeSvQv-uEQ4HDqP_tp-Pa8Ze39qm3=XA@mail.gmail.com>

On Sat, Jan 26, 2019 at 1:43 AM Jann Horn <jannh@google.com> wrote:
> On Sat, Jan 26, 2019 at 12:44 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Fri, Jan 25, 2019 at 02:51:12PM -0800, Paul E. McKenney wrote:
> > > > >
> > > > > So no more than (say) 100 milliseconds?
> > > >
> > > > Depends on RLIMIT_MEMLOCK and on how hard userspace is trying to make
> > > > things slow, I guess - if userspace manages to create a hashtable,
> > > > with a few dozen megabytes in size, with worst-case assignment of
> > > > elements to buckets (everything in a single bucket), every lookup call
> > > > on that bucket becomes a linked list traversal through a list that
> > > > must be stored in main memory because it's too big for the CPU caches.
> > > > I don't know into how much time that translates.
> > >
> > > So perhaps you have a candidate BPF program for the RCU CPU stall warning
> > > challenge, then.  ;-)
> >
> > I'd like to see one that can defeat jhash + random seed.
>
> Assuming that the map isn't created by root with BPF_F_ZERO_SEED:
>
> The dumb approach would be to put things into the map, try to measure
> via timing/sidechannel whether you got collisions, and then keep
> trying different keys, and keep them if the timing indicates a
> collision. That'd probably be pretty slow and annoying though. Two
> years ago, I implemented something similar to leak information about
> virtual addresses from Firefox by measuring hash bucket collisions
> from JavaScript (but to be fair, it was easier there because you can
> resize the hash table):
> https://thejh.net/misc/firefox-cve-2016-9904-and-cve-2017-5378-bugreport
>
> But I think there's an easier way, too: The jhash seed is just 32
> bits, and AFAICS the BPF API leaks information about that seed through
> BPF_MAP_GET_NEXT_KEY: Stuff two random keys into the hash table, run
> BPF_MAP_GET_NEXT_KEY with attr->key==NULL, and see which key is
> returned. Do that around 32 times, and you should have roughly enough
> information to bruteforce the jhash seed? Recovering the seed should
> then be relatively quick, 2^32 iterations of a fast hash don't take
> terribly long.
>
> That said, I don't think this is interesting enough to spend the time
> necessary to implement it. :P

Oh, and actually, you can probably also detect a collision in a simpler way:

 - insert A
 - insert B
 - query BPF_MAP_GET_NEXT_KEY
 - delete A
 - delete B
 - insert B
 - insert A
 - query BPF_MAP_GET_NEXT_KEY
 - delete A
 - delete B

If the two BPF_MAP_GET_NEXT_KEY queries return the same result, A and
B are in different buckets; if they return different results, A and B
are in the same bucket, I think?

^ permalink raw reply

* Re: [RFC PATCH 1/3] riscv: set HAVE_EFFICIENT_UNALIGNED_ACCESS
From: Jim Wilson @ 2019-01-26  1:33 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: bjorn.topel, Christoph Hellwig, linux-riscv, David Lee, daniel,
	netdev
In-Reply-To: <mhng-0770bfe6-73bd-4b8a-9fa7-142ed95a6974@palmer-si-x1c4>

On Fri, Jan 25, 2019 at 12:21 PM Palmer Dabbelt <palmer@sifive.com> wrote:
> Jim, would you be opposed to something like this?

This looks OK to me.

>     +    builtin_define_with_int_value ("__riscv_tune_misaligned_load_cost",
>     +                                   riscv_tune_info->slow_unaligned_access ? 1024 : 1);
>     +    builtin_define_with_int_value ("__riscv_tune_misaligned_store_cost",
>     +                                   riscv_tune_info->slow_unaligned_access ? 1024 : 1);

It would be nice to have a better way to compute these values, maybe
an extra field in the tune structure, but we can always worry about
that later when we need it.

Jim

^ permalink raw reply

* Re: [PATCH] tty: Fix WARNING in tty_set_termios
From: Al Viro @ 2019-01-26  4:14 UTC (permalink / raw)
  To: Shuah Khan
  Cc: marcel, johan.hedberg, w.d.hubbs, chris, kirk, samuel.thibault,
	gregkh, robh, jslaby, sameo, davem, arnd, nishka.dasgupta_ug18,
	m.maya.nakamura, santhameena13, zhongjiang, linux-bluetooth,
	linux-kernel, speakup, devel, linux-serial, linux-wireless,
	netdev
In-Reply-To: <20190125232905.21727-1-shuah@kernel.org>

On Fri, Jan 25, 2019 at 04:29:05PM -0700, Shuah Khan wrote:
> tty_set_termios() has the following WARMN_ON which can be triggered with a
> syscall to invoke TIOCGETD __NR_ioctl.
> 
> WARN_ON(tty->driver->type == TTY_DRIVER_TYPE_PTY &&
>                 tty->driver->subtype == PTY_TYPE_MASTER);
> Reference: https://syzkaller.appspot.com/bug?id=2410d22f1d8e5984217329dd0884b01d99e3e48d
> 
> A simple change would have been to print error message instead of WARN_ON.
> However, the callers assume that tty_set_termios() always returns 0 and
> don't check return value. The complete solution is fixing all the callers
> to check error and bail out to fix the WARN_ON.
> 
> This fix changes tty_set_termios() to return error and all the callers
> to check error and bail out. The reproducer is used to reproduce the
> problem and verify the fix.

> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -321,6 +321,8 @@ void hci_uart_set_flow_control(struct hci_uart *hu, bool enable)
>  		status = tty_set_termios(tty, &ktermios);
>  		BT_DBG("Disabling hardware flow control: %s",
>  		       status ? "failed" : "success");
> +		if (status)
> +			return;

Can that ldisc end up set on pty master?  And does it make any sense there?

> diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> index fa1672993b4c..29b51370deac 100644
> --- a/drivers/tty/serdev/serdev-ttyport.c
> +++ b/drivers/tty/serdev/serdev-ttyport.c
> @@ -136,7 +136,9 @@ static int ttyport_open(struct serdev_controller *ctrl)
>  	ktermios.c_cflag |= CRTSCTS;
>  	/* Hangups are not supported so make sure to ignore carrier detect. */
>  	ktermios.c_cflag |= CLOCAL;
> -	tty_set_termios(tty, &ktermios);
> +	ret = tty_set_termios(tty, &ktermios);

How can _that_ happen to pty master?

> diff --git a/net/nfc/nci/uart.c b/net/nfc/nci/uart.c
> index 78fe622eba65..9978c21ce34d 100644
> --- a/net/nfc/nci/uart.c
> +++ b/net/nfc/nci/uart.c
> @@ -447,6 +447,7 @@ void nci_uart_set_config(struct nci_uart *nu, int baudrate, int flow_ctrl)
>  	else
>  		new_termios.c_cflag &= ~CRTSCTS;
>  
> +	/* FIXME tty_set_termios() could return error */

Ditto for this one.

IOW, I don't believe that this patch makes any sense.  If anything,
we need to prevent unconditional tty_set_termios() on the path
that *does* lead to calling it for pty.

^ permalink raw reply

* Send photos to start
From: Carol @ 2019-01-25  9:04 UTC (permalink / raw)
  To: netdev

Send the photos to start editing.

We can do white background for your photos, can add clipping path if
needed.
We can also sharpen your photos also give retouching.

If you have the photos ready, please send a test photo to start.

Thanks,
Carol


^ permalink raw reply

* Re: [pull request][net-next 0/8] Mellanox, mlx5 misc updates 2019-01-25
From: David Miller @ 2019-01-26  5:22 UTC (permalink / raw)
  To: saeedm; +Cc: netdev
In-Reply-To: <20190125201818.9973-1-saeedm@mellanox.com>

From: Saeed Mahameed <saeedm@mellanox.com>
Date: Fri, 25 Jan 2019 12:18:10 -0800

> This series provides some misc updates to mlx5 driver.
> For more information please see tag log below.
> 
> Please pull and let me know if there is any problem.

Pulled, thanks.

^ permalink raw reply

* Re: [PATCH net-next] Documentation: net: phy: reflect latest changes to phylib API
From: David Miller @ 2019-01-26  5:22 UTC (permalink / raw)
  To: hkallweit1; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <27b5d4a5-4939-54d9-baed-6dd2ae743511@gmail.com>

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Fri, 25 Jan 2019 21:08:24 +0100

> Recent changes to the phylib API
> - removed phy_stop_interrupts
> - replaced phy_start_interrupts with phy_request_interrupt
> - moved some functionality from phy_connect() and phy_disconnect()
>   to phy_start() and phy_stop() respectively.
> Reflect these changes in the documentation.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 0/8] s390/qeth: updates 2019-01-25
From: David Miller @ 2019-01-26  5:24 UTC (permalink / raw)
  To: jwi; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20190125144423.81443-1-jwi@linux.ibm.com>

From: Julian Wiedmann <jwi@linux.ibm.com>
Date: Fri, 25 Jan 2019 15:44:15 +0100

> please apply a first batch of qeth patches for net-next, primarily touching the
> net_device parts of the driver.
> In addition to the usual refactoring & code consolidation, patch 7 makes use of
> netif_device_detach() to let the stack know when our control plane is down. This
> helps quite a bit wrt to overall locking and proper init/shutdown sequencing.

Series applied.

^ permalink raw reply

* Re: [PATCH net-next 0/4] net: IP defrag: use rbtrees in IPv6 defragmentation
From: David Miller @ 2019-01-26  5:37 UTC (permalink / raw)
  To: posk; +Cc: netdev, posk.devel
In-Reply-To: <20190122180253.128336-1-posk@google.com>

From: Peter Oskolkov <posk@google.com>
Date: Tue, 22 Jan 2019 10:02:49 -0800

> Currently, IPv6 defragmentation code drops non-last fragments that
> are smaller than 1280 bytes: see
> commit 0ed4229b08c1 ("ipv6: defrag: drop non-last frags smaller than min mtu")
> 
> This behavior is not specified in IPv6 RFCs and appears to break compatibility
> with some IPv6 implementations, as reported here:
> https://www.spinics.net/lists/netdev/msg543846.html
> 
> This patchset contains four patches:
> - patch 1 moves rbtree-related code from IPv4 to files shared b/w
> IPv4/IPv6
> - patch 2 changes IPv6 defragmenation code to use rbtrees for defrag
> queue
> - patch 3 changes nf_conntrack IPv6 defragmentation code to use rbtrees
> - patch 4 changes ip_defrag selftest to test changes made in the
> previous three patches.
> 
> Along the way, the 1280-byte restrictions are removed.
> 
> I plan to introduce similar changes to 6lowpan defragmentation code
> once I figure out how to test it.

Series applied, thanks.

^ permalink raw reply

* Re: [RFC] Loading BTF and pretty printing maps with bpftool
From: Martin Lau @ 2019-01-26  5:37 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Yonghong Song, Song Liu, Andrii Nakryiko, Alexei Starovoitov,
	Linux Kernel Mailing List,
	Linux Networking Development Mailing List
In-Reply-To: <20190125102057.GB12077@kernel.org>

On Fri, Jan 25, 2019 at 11:20:57AM +0100, Arnaldo Carvalho de Melo wrote:
>   # bpftool version
>   bpftool v5.0.0-rc3
>   #
> 
>   # bpftool prog | tail -6
>   309: tracepoint  name sys_enter  tag 819967866022f1e1  gpl
> 	loaded_at 2019-01-25T11:05:41+0100  uid 0
> 	xlated 528B  jited 381B  memlock 4096B  map_ids 200,199,198
>   310: tracepoint  name sys_exit  tag c1bd85c092d6e4aa  gpl
> 	loaded_at 2019-01-25T11:05:41+0100  uid 0
> 	xlated 256B  jited 191B  memlock 4096B  map_ids 200,199
>   #
> 
> And the maps:
> 
>   # bpftool map | tail -6
>   198: perf_event_array  name __augmented_sys  flags 0x0
> 	key 4B  value 4B  max_entries 8  memlock 4096B
>   199: array  name syscalls  flags 0x0
> 	key 4B  value 1B  max_entries 512  memlock 8192B
>   200: hash  name pids_filtered  flags 0x0
> 	key 4B  value 1B  max_entries 64  memlock 8192B
>   #
> 
> So, dumping the entries for those entries:
> 
> [root@quaco ~]# egrep sleep /tmp/build/perf/arch/x86/include/generated/asm/syscalls_64.c
> 	[35] = "nanosleep",
> 	[230] = "clock_nanosleep",
> [root@quaco ~]#
> 
> Looking at just the open and nanosleep:
> 
>   # bpftool map dump id 199 | grep "value: 01"
>   key: 23 00 00 00  value: 01
>   key: e6 00 00 00  value: 01
>   #
> 
>   # bpftool map lookup id 199 key 35
>   Error: key expected 4 bytes got 1
>   #
> 
>   # bpftool map lookup id 199 key 35 00 00 00
>   key: 23 00 00 00  value: 01
>   # bpftool map lookup id 199 key 230 00 00 00
>   key: e6 00 00 00  value: 01
>   # 
> 
> I thought it was that --pretty option, so I tried:
> 
>   # bpftool map --pretty lookup id 199 key 230 00 00 00
>   {
>     "key": ["0xe6","0x00","0x00","0x00"
>     ],
>     "value": ["0x01"
>     ]
>   }
>   #
libbpf pr_warning on failing to load BTF
or failing to create a MAP after BTF has been loaded and
____btf_map_xxx can be found.

Did you see any of them?  It seems it can load the BTF
from your email.

It may be useful to set the libbpf's __pr_debug which
should be NULL by default iirc.

^ permalink raw reply

* Re: [PATCH bpf-next v2 0/3] selftests: bpf: break up test_verifier
From: Alexei Starovoitov @ 2019-01-26  5:55 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: daniel, oss-drivers, netdev
In-Reply-To: <20190125232444.30172-1-jakub.kicinski@netronome.com>

On Fri, Jan 25, 2019 at 03:24:41PM -0800, Jakub Kicinski wrote:
> Hi!
> 
> The tools/testing/selftests/bpf/test_verifier.c file is
> way too large, and since most people add their at the
> end of the list it's very prone to conflicts.
> 
> Break it up in the simplest possible way - slice the
> array up into smaller C files and include them in the
> right spot.
> 
> Tested:
> $ make -C tools/testing/selftests/bpf/
> $ cd tools/testing/selftests/bpf/ ; make
> 
> v2:
> 
> The indentation is reduced further as discussed and lines folded.
> The conversion was scripted, and double checked by hand.

Looks great to me, but even first patch conflicts too much to apply.
Please respin one more time.
Thanks!


^ permalink raw reply

* Re: [PATCH net-next 0/2] r8169: add EEE support for RTL8168g+
From: David Miller @ 2019-01-26  6:03 UTC (permalink / raw)
  To: hkallweit1; +Cc: nic_swsd, netdev
In-Reply-To: <8cce9784-c943-6d52-1e40-a04fbe9b6db6@gmail.com>

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Fri, 25 Jan 2019 20:37:30 +0100

> This series adds general EEE support to be used with ethtool.
> In addition it implements EEE for chip versions from RTL8168g.
> The first patch leaves the default chip settings and the
> second enables EEE per default. This allows us to revert patch 2
> w/o removing EEE support completely if we should face issues with
> EEE on particular chip versions.
> 
> Unfortunately Realtek decided not to use the standard EEE MMD
> registers but to use proprietary registers. Therefore we can't
> use phylib functions like phy_ethtool_set_eee and have to
> reimplement the functionality.
> 
> Tested on a system with RTL8168g (chip version 40).

Series applied, thanks Heiner.

^ permalink raw reply

* Re: [PATCH -next] ptp: fix debugfs_simple_attr.cocci warnings
From: David Miller @ 2019-01-26  6:05 UTC (permalink / raw)
  To: yuehaibing; +Cc: yangbo.lu, richardcochran, netdev, kernel-janitors
In-Reply-To: <1548383339-159403-1-git-send-email-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Fri, 25 Jan 2019 02:28:59 +0000

> Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
> for debugfs files.
> 
> Semantic patch information:
> Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
> imposes some significant overhead as compared to
> DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
> 
> Generated by: scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH bpf-next v2 0/3] selftests: bpf: break up test_verifier
From: Jakub Kicinski @ 2019-01-26  6:17 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: daniel, oss-drivers, netdev
In-Reply-To: <20190126055528.r3xvfgtazdf44ab6@ast-mbp.dhcp.thefacebook.com>

On Fri, 25 Jan 2019 21:55:30 -0800, Alexei Starovoitov wrote:
> On Fri, Jan 25, 2019 at 03:24:41PM -0800, Jakub Kicinski wrote:
> > Hi!
> > 
> > The tools/testing/selftests/bpf/test_verifier.c file is
> > way too large, and since most people add their at the
> > end of the list it's very prone to conflicts.
> > 
> > Break it up in the simplest possible way - slice the
> > array up into smaller C files and include them in the
> > right spot.
> > 
> > Tested:
> > $ make -C tools/testing/selftests/bpf/
> > $ cd tools/testing/selftests/bpf/ ; make
> > 
> > v2:
> > 
> > The indentation is reduced further as discussed and lines folded.
> > The conversion was scripted, and double checked by hand.  
> 
> Looks great to me, but even first patch conflicts too much to apply.
> Please respin one more time.

I could have sworn when I pulled bpf-next this morning Jiong's patches
were in it ;)  I'll rebase/repost tomorrow morning.

^ permalink raw reply

* Re: [PATCH bpf-next v2 0/3] selftests: bpf: break up test_verifier
From: Alexei Starovoitov @ 2019-01-26  6:21 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: daniel, oss-drivers, netdev
In-Reply-To: <20190125221728.3990b9d1@cakuba.netronome.com>

On Fri, Jan 25, 2019 at 10:17:28PM -0800, Jakub Kicinski wrote:
> On Fri, 25 Jan 2019 21:55:30 -0800, Alexei Starovoitov wrote:
> > On Fri, Jan 25, 2019 at 03:24:41PM -0800, Jakub Kicinski wrote:
> > > Hi!
> > > 
> > > The tools/testing/selftests/bpf/test_verifier.c file is
> > > way too large, and since most people add their at the
> > > end of the list it's very prone to conflicts.
> > > 
> > > Break it up in the simplest possible way - slice the
> > > array up into smaller C files and include them in the
> > > right spot.
> > > 
> > > Tested:
> > > $ make -C tools/testing/selftests/bpf/
> > > $ cd tools/testing/selftests/bpf/ ; make
> > > 
> > > v2:
> > > 
> > > The indentation is reduced further as discussed and lines folded.
> > > The conversion was scripted, and double checked by hand.  
> > 
> > Looks great to me, but even first patch conflicts too much to apply.
> > Please respin one more time.
> 
> I could have sworn when I pulled bpf-next this morning Jiong's patches
> were in it ;)  I'll rebase/repost tomorrow morning.

They were ;) until they were pulled off.
Any order is fine.


^ permalink raw reply

* Re: [PATCH net-next] tcp: allow zerocopy with fastopen
From: David Miller @ 2019-01-26  6:41 UTC (permalink / raw)
  To: willemdebruijn.kernel; +Cc: netdev, ycheng, edumazet, alexey.kodanev, willemb
In-Reply-To: <20190125161723.75429-1-willemdebruijn.kernel@gmail.com>

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Fri, 25 Jan 2019 11:17:23 -0500

> From: Willem de Bruijn <willemb@google.com>
> 
> Accept MSG_ZEROCOPY in all the TCP states that allow sendmsg. Remove
> the explicit check for ESTABLISHED and CLOSE_WAIT states.
> 
> This requires correctly handling zerocopy state (uarg, sk_zckey) in
> all paths reachable from other TCP states. Such as the EPIPE case
> in sk_stream_wait_connect, which a sendmsg() in incorrect state will
> now hit. Most paths are already safe.
> 
> Only extension needed is for TCP Fastopen active open. This can build
> an skb with data in tcp_send_syn_data. Pass the uarg along with other
> fastopen state, so that this skb also generates a zerocopy
> notification on release.
> 
> Tested with active and passive tcp fastopen packetdrill scripts at
> https://github.com/wdebruij/packetdrill/commit/1747eef03d25a2404e8132817d0f1244fd6f129d
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>

Applied, thanks.

^ permalink raw reply

* Re: Clang warnings in net/phonet
From: Nathan Chancellor @ 2019-01-26  8:10 UTC (permalink / raw)
  To: Rémi Denis-Courmont, Remi Denis-Courmont
  Cc: David S. Miller, netdev, linux-kernel, Nick Desaulniers
In-Reply-To: <20190108025420.GA26093@flashbox>

On Mon, Jan 07, 2019 at 07:54:20PM -0700, Nathan Chancellor wrote:
> Hi all,
> 
> When building the kernel with Clang, this warning comes up in net/phonet.
> 
> net/phonet/pep.c:224:16: warning: array index 1 is past the end of the array (which contains 1 element) [-Warray-bounds]
>         ph->data[0] = oph->data[1]; /* CTRL id */
>                       ^         ~
> include/net/phonet/pep.h:66:3: note: array 'data' declared here
>                 u8              data[1];
>                 ^
> net/phonet/pep.c:281:10: warning: array index 1 is past the end of the array (which contains 1 element) [-Warray-bounds]
>         switch (hdr->data[1]) {
>                 ^         ~
> include/net/phonet/pep.h:66:3: note: array 'data' declared here
>                 u8              data[1];
>                 ^
> net/phonet/pep.c:285:12: warning: array index 4 is past the end of the array (which contains 1 element) [-Warray-bounds]
>                         switch (hdr->data[4]) {
>                                 ^         ~
> include/net/phonet/pep.h:66:3: note: array 'data' declared here
>                 u8              data[1];
>                 ^
> net/phonet/pep.c:295:8: warning: array index 4 is past the end of the array (which contains 1 element) [-Warray-bounds]
>                         if (hdr->data[4] == PEP_IND_READY)
>                             ^         ~
> include/net/phonet/pep.h:66:3: note: array 'data' declared here
>                 u8              data[1];
>                 ^
> net/phonet/pep.c:304:21: warning: array index 4 is past the end of the array (which contains 1 element) [-Warray-bounds]
>                 atomic_add(wake = hdr->data[4], &pn->tx_credits);
>                                   ^         ~
> include/net/phonet/pep.h:66:3: note: array 'data' declared here
>                 u8              data[1];
>                 ^
> net/phonet/pep.c:509:9: warning: array index 4 is past the end of the array (which contains 1 element) [-Warray-bounds]
>         n_sb = hdr->data[4];
>                ^         ~
> include/net/phonet/pep.h:66:3: note: array 'data' declared here
>                 u8              data[1];
>                 ^
> net/phonet/pep.c:817:14: warning: array index 1 is past the end of the array (which contains 1 element) [-Warray-bounds]
>         peer_type = hdr->other_pep_type << 8;
>                     ^    ~~~~~~~~~~~~~~
> include/net/phonet/pep.h:66:3: note: array 'data' declared here
>                 u8              data[1];
>                 ^
> net/phonet/pep.c:820:9: warning: array index 4 is past the end of the array (which contains 1 element) [-Warray-bounds]
>         n_sb = hdr->data[4];
>                ^         ~
> include/net/phonet/pep.h:66:3: note: array 'data' declared here
>                 u8              data[1];
>                 ^
> 8 warnings generated.
> 
> I have taken a look at the effected code but I can't really figure out
> the proper fix for this warning (my knowledge of C just isn't there
> yet). Nick had suggested changing 'u8 data[1]' to 'u8 *data' in
> 'struct pnpipehdr', which seems logical but I can't say for sure. Any
> advice would be appreciated :)
> 
> Thanks,
> Nathan

Gentle ping?

^ permalink raw reply

* [PATCH net 0/3] net: hns: code optimizations & bugfixes for HNS driver
From: Peng Li @ 2019-01-26  9:18 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel, linuxarm, yisen.zhuang, salil.mehta,
	lipeng321

This patchset includes bugfixes and code optimizations for the HNS
ethernet controller driver

Yonglong Liu (3):
  net: hns: Fix for missing of_node_put() after of_parse_phandle()
  net: hns: Restart autoneg need return failed when autoneg off
  net: hns: Fix wrong read accesses via Clause 45 MDIO protocol

 drivers/net/ethernet/hisilicon/hns/hns_enet.c    |  5 +++++
 drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 16 +++++++++-------
 drivers/net/ethernet/hisilicon/hns_mdio.c        |  2 +-
 3 files changed, 15 insertions(+), 8 deletions(-)

-- 
1.9.1


^ permalink raw reply

* [PATCH net 1/3] net: hns: Fix for missing of_node_put() after of_parse_phandle()
From: Peng Li @ 2019-01-26  9:18 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel, linuxarm, yisen.zhuang, salil.mehta,
	lipeng321
In-Reply-To: <1548494307-63594-1-git-send-email-lipeng321@huawei.com>

From: Yonglong Liu <liuyonglong@huawei.com>

In hns enet driver, we use of_parse_handle() to get hold of the
device node related to "ae-handle" but we have missed to put
the node reference using of_node_put() after we are done using
the node. This patch fixes it.

Note:
This problem is stated in Link: https://lkml.org/lkml/2018/12/22/217

Fixes: 48189d6aaf1e ("net: hns: enet specifies a reference to dsaf")
Reported-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
Signed-off-by: Peng Li <lipeng321@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns/hns_enet.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index 5b33238..60e7d7a 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -2418,6 +2418,8 @@ static int hns_nic_dev_probe(struct platform_device *pdev)
 out_notify_fail:
 	(void)cancel_work_sync(&priv->service_task);
 out_read_prop_fail:
+	/* safe for ACPI FW */
+	of_node_put(to_of_node(priv->fwnode));
 	free_netdev(ndev);
 	return ret;
 }
@@ -2447,6 +2449,9 @@ static int hns_nic_dev_remove(struct platform_device *pdev)
 	set_bit(NIC_STATE_REMOVING, &priv->state);
 	(void)cancel_work_sync(&priv->service_task);
 
+	/* safe for ACPI FW */
+	of_node_put(to_of_node(priv->fwnode));
+
 	free_netdev(ndev);
 	return 0;
 }
-- 
1.9.1


^ permalink raw reply related

* [PATCH net 2/3] net: hns: Restart autoneg need return failed when autoneg off
From: Peng Li @ 2019-01-26  9:18 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel, linuxarm, yisen.zhuang, salil.mehta,
	lipeng321
In-Reply-To: <1548494307-63594-1-git-send-email-lipeng321@huawei.com>

From: Yonglong Liu <liuyonglong@huawei.com>

The hns driver of earlier devices, when autoneg off, restart autoneg
will return -EINVAL, so make the hns driver for the latest devices
do the same.

Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
Signed-off-by: Peng Li <lipeng321@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
index 8e9b958..ce15d23 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
@@ -1157,16 +1157,18 @@ static int hns_get_regs_len(struct net_device *net_dev)
  */
 static int hns_nic_nway_reset(struct net_device *netdev)
 {
-	int ret = 0;
 	struct phy_device *phy = netdev->phydev;
 
-	if (netif_running(netdev)) {
-		/* if autoneg is disabled, don't restart auto-negotiation */
-		if (phy && phy->autoneg == AUTONEG_ENABLE)
-			ret = genphy_restart_aneg(phy);
-	}
+	if (!netif_running(netdev))
+		return 0;
 
-	return ret;
+	if (!phy)
+		return -EOPNOTSUPP;
+
+	if (phy->autoneg != AUTONEG_ENABLE)
+		return -EINVAL;
+
+	return genphy_restart_aneg(phy);
 }
 
 static u32
-- 
1.9.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