Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net] sctp: not allow to set rto_min with a value below 200 msecs
From: Neal Cardwell @ 2018-05-29 16:03 UTC (permalink / raw)
  To: marcelo.leitner
  Cc: Michael Tuexen, nhorman, Dmitry Vyukov, lucien.xin, Netdev,
	linux-sctp, David Miller, dsa, Eric Dumazet, syzkaller
In-Reply-To: <20180529154514.GC3788@localhost.localdomain>

On Tue, May 29, 2018 at 11:45 AM Marcelo Ricardo Leitner <
marcelo.leitner@gmail.com> wrote:
> - patch2 - fix rtx attack vector
>    - Add the floor value to rto_min to HZ/20 (which fits the values
>      that Michael shared on the other email)

I would encourage allowing minimum RTO values down to 5ms, if the ACK
policy in the receiver makes this feasible. Our experience is that in
datacenter environments it can be advantageous to allow timer-based loss
recoveries using timeout values as low as 5ms, e.g.:


https://www.ietf.org/proceedings/97/slides/slides-97-tcpm-tcp-options-for-low-latency-00.pdf
   https://tools.ietf.org/html/draft-wang-tcpm-low-latency-opt-00

cheers,
neal

^ permalink raw reply

* Re: [PATCH net] net: sched: check netif_xmit_frozen_or_stopped() in sch_direct_xmit()
From: Song Liu @ 2018-05-29 16:51 UTC (permalink / raw)
  To: David Miller; +Cc: Networking, Kernel Team, john.fastabend@gmail.com
In-Reply-To: <20180529.100255.625454563789387164.davem@davemloft.net>



> On May 29, 2018, at 7:02 AM, David Miller <davem@davemloft.net> wrote:
> 
> From: Song Liu <songliubraving@fb.com>
> Date: Fri, 25 May 2018 11:11:44 -0700
> 
>> Summary:
>> 
>> At the end of sch_direct_xmit(), we are in the else path of
>> !dev_xmit_complete(ret), which means ret == NETDEV_TX_OK. The following
>> condition will always fail and netif_xmit_frozen_or_stopped() is not
>> checked at all.
>> 
>>    if (ret && netif_xmit_frozen_or_stopped(txq))
>>         return false;
>> 
>> In this patch, this condition is fixed as:
>> 
>>    if (netif_xmit_frozen_or_stopped(txq))
>>         return false;
>> 
>> and further simplifies the code as:
>> 
>>    return !netif_xmit_frozen_or_stopped(txq);
>> 
>> Fixes: 29b86cdac00a ("net: sched: remove remaining uses for qdisc_qlen in xmit path")
>> Cc: John Fastabend <john.fastabend@gmail.com>
>> Cc: David S. Miller <davem@davemloft.net>
>> Signed-off-by: Song Liu <songliubraving@fb.com>
> 
> I expect a new version of this patch which removes the test entirely.

The new version of it is here: http://patchwork.ozlabs.org/patch/921708/

Thanks,
Song

^ permalink raw reply

* Re: [PATCH net-next] net: remove bypassed check in sch_direct_xmit()
From: Song Liu @ 2018-05-29 17:02 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: netdev@vger.kernel.org, Kernel Team, John Fastabend,
	Alexei Starovoitov, David S . Miller
In-Reply-To: <00399207-8196-1766-330d-624f2ed35c89@cogentembedded.com>



> On May 29, 2018, at 1:58 AM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:
> 
> Hello!
> 
> On 5/29/2018 12:36 AM, Song Liu wrote:
> 
>> Check sch_direct_xmit() at the end of sch_direct_xmit() will be bypassed.
> 
>   "Checking netif_xmit_frozen_or_stopped()", perhaps? Else it doesn't make much sense...

Thanks Sergei!

Sending v2 with fix. 

Song

> 
>> This is because "ret" from sch_direct_xmit() will be either NETDEV_TX_OK
>> or NETDEV_TX_BUSY, and only ret == NETDEV_TX_OK == 0 will reach the
>> condition:
>>     if (ret && netif_xmit_frozen_or_stopped(txq))
>>         return false;
>> This patch cleans up the code by removing  the whole condition.
>> For more discussion about this, please refer to
>>    https://marc.info/?t=152727195700008
>> Signed-off-by: Song Liu <songliubraving@fb.com>
>> Cc: John Fastabend <john.fastabend@gmail.com>
>> Cc: Alexei Starovoitov <ast@kernel.org>
>> Cc: David S. Miller <davem@davemloft.net>
> [...]
> 
> MBR, Sergei

^ permalink raw reply

* [PATCH v2 net-next] net: remove bypassed check in sch_direct_xmit()
From: Song Liu @ 2018-05-29 17:03 UTC (permalink / raw)
  To: netdev
  Cc: sergei.shtylyov, Song Liu, kernel-team, John Fastabend,
	Alexei Starovoitov, David S . Miller

Checking netif_xmit_frozen_or_stopped() at the end of sch_direct_xmit()
is being bypassed. This is because "ret" from sch_direct_xmit() will be
either NETDEV_TX_OK or NETDEV_TX_BUSY, and only ret == NETDEV_TX_OK == 0
will reach the condition:

    if (ret && netif_xmit_frozen_or_stopped(txq))
        return false;

This patch cleans up the code by removing the whole condition.

For more discussion about this, please refer to
   https://marc.info/?t=152727195700008

Signed-off-by: Song Liu <songliubraving@fb.com>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: David S. Miller <davem@davemloft.net>
---
 net/sched/sch_generic.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 760ab1b..69078c8 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -346,9 +346,6 @@ bool sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
 		return false;
 	}
 
-	if (ret && netif_xmit_frozen_or_stopped(txq))
-		return false;
-
 	return true;
 }
 
-- 
2.9.5

^ permalink raw reply related

* Re: [PATCH net] sctp: not allow to set rto_min with a value below 200 msecs
From: Marcelo Ricardo Leitner @ 2018-05-29 17:06 UTC (permalink / raw)
  To: Neal Cardwell
  Cc: Michael Tuexen, nhorman, Dmitry Vyukov, lucien.xin, Netdev,
	linux-sctp, David Miller, dsa, Eric Dumazet, syzkaller
In-Reply-To: <CADVnQymCNEtQFThsbQ0u7jirx24i45f5L7P-2dPf8omkshqwmg@mail.gmail.com>

On Tue, May 29, 2018 at 12:03:46PM -0400, Neal Cardwell wrote:
> On Tue, May 29, 2018 at 11:45 AM Marcelo Ricardo Leitner <
> marcelo.leitner@gmail.com> wrote:
> > - patch2 - fix rtx attack vector
> >    - Add the floor value to rto_min to HZ/20 (which fits the values
> >      that Michael shared on the other email)
> 
> I would encourage allowing minimum RTO values down to 5ms, if the ACK
> policy in the receiver makes this feasible. Our experience is that in
> datacenter environments it can be advantageous to allow timer-based loss
> recoveries using timeout values as low as 5ms, e.g.:

Thanks Neal. On Xin's tests, the hearbeat timer becomes an issue at
~25ms already. Xin, can you share more details on the hw, which CPU
was used?

Anyway, what about we add a floor to rto_max too, so that RTO can
actually grow into something bigger that don't hog the CPU? Like:
rto_min floor = 5ms
rto_max floor = 50ms

  Marcelo

^ permalink raw reply

* Re: [PATCH bpf-next 03/11] bpf: fixup error message from gpl helpers on license mismatch
From: Jesper Dangaard Brouer @ 2018-05-29 17:16 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: brouer, netdev
In-Reply-To: <20180528004344.3606-4-daniel@iogearbox.net>

On Mon, 28 May 2018 02:43:36 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:

> Stating 'proprietary program' in the error is just silly since it
> can also be a different open source license than that which is just
> not compatible.
> 
> Reference: https://twitter.com/majek04/status/998531268039102465
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>

Thank you for cleaning up this confusion :-)

> ---
>  kernel/bpf/verifier.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1fd9667b..4f4786e 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2462,7 +2462,7 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
>  
>  	/* eBPF programs must be GPL compatible to use GPL-ed functions */
>  	if (!env->prog->gpl_compatible && fn->gpl_only) {
> -		verbose(env, "cannot call GPL only function from proprietary program\n");
> +		verbose(env, "cannot call GPL-restricted function from non-GPL compatible program\n");
>  		return -EINVAL;
>  	}
>  



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH bpf 1/2] bpf: fix alignment of netns_dev/netns_ino fields in bpf_{map,prog}_info
From: Song Liu @ 2018-05-29 17:17 UTC (permalink / raw)
  To: Eugene Syromiatnikov
  Cc: netdev, open list, Martin KaFai Lau, Daniel Borkmann,
	Alexei Starovoitov, David S. Miller, Jiri Olsa, Ingo Molnar,
	Lawrence Brakmo, Andrey Ignatov, Jakub Kicinski, John Fastabend,
	Dmitry V. Levin
In-Reply-To: <20180527112842.GA18204@asgard.redhat.com>

On Sun, May 27, 2018 at 4:28 AM, Eugene Syromiatnikov <esyr@redhat.com> wrote:
> Recent introduction of netns_dev/netns_ino to bpf_map_info/bpf_prog info
> has broken compat, as offsets of these fields are different in 32-bit
> and 64-bit ABIs.  One fix (other than implementing compat support in
> syscall in order to handle this discrepancy) is to use __aligned_u64
> instead of __u64 for these fields.
>
> Reported-by: Dmitry V. Levin <ldv@altlinux.org>
> Fixes: 52775b33bb507 ("bpf: offload: report device information about
> offloaded maps")
> Fixes: 675fc275a3a2d ("bpf: offload: report device information for
> offloaded programs")
>
> Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
> ---
>  include/uapi/linux/bpf.h       | 8 ++++----
>  tools/include/uapi/linux/bpf.h | 8 ++++----
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index c5ec897..903010a 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1017,8 +1017,8 @@ struct bpf_prog_info {
>         __aligned_u64 map_ids;
>         char name[BPF_OBJ_NAME_LEN];
>         __u32 ifindex;
> -       __u64 netns_dev;
> -       __u64 netns_ino;
> +       __aligned_u64 netns_dev;
> +       __aligned_u64 netns_ino;
>  } __attribute__((aligned(8)));

Shall we add a __u32 padding variable before netns_dev? We can use it
for in the future.

Thanks,
Song

>
>  struct bpf_map_info {
> @@ -1030,8 +1030,8 @@ struct bpf_map_info {
>         __u32 map_flags;
>         char  name[BPF_OBJ_NAME_LEN];
>         __u32 ifindex;
> -       __u64 netns_dev;
> -       __u64 netns_ino;
> +       __aligned_u64 netns_dev;
> +       __aligned_u64 netns_ino;
>  } __attribute__((aligned(8)));
>
>  /* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index c5ec897..903010a 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -1017,8 +1017,8 @@ struct bpf_prog_info {
>         __aligned_u64 map_ids;
>         char name[BPF_OBJ_NAME_LEN];
>         __u32 ifindex;
> -       __u64 netns_dev;
> -       __u64 netns_ino;
> +       __aligned_u64 netns_dev;
> +       __aligned_u64 netns_ino;
>  } __attribute__((aligned(8)));
>
>  struct bpf_map_info {
> @@ -1030,8 +1030,8 @@ struct bpf_map_info {
>         __u32 map_flags;
>         char  name[BPF_OBJ_NAME_LEN];
>         __u32 ifindex;
> -       __u64 netns_dev;
> -       __u64 netns_ino;
> +       __aligned_u64 netns_dev;
> +       __aligned_u64 netns_ino;
>  } __attribute__((aligned(8)));
>
>  /* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
> --
> 2.1.4
>

^ permalink raw reply

* Re: [PATCH bpf-next 05/11] bpf: avoid retpoline for lookup/update/delete calls on maps
From: Jesper Dangaard Brouer @ 2018-05-29 17:23 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: brouer, netdev
In-Reply-To: <20180528004344.3606-6-daniel@iogearbox.net>

On Mon, 28 May 2018 02:43:38 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:

> While some of the BPF map lookup helpers provide a ->map_gen_lookup()
> callback for inlining the map lookup altogether it is not available
> for every map, so the remaining ones have to call bpf_map_lookup_elem()
> helper which does a dispatch to map->ops->map_lookup_elem(). In
> times of retpolines, this will control and trap speculative execution
> rather than letting it do its work for the indirect call and will
> therefore cause a slowdown. Likewise, bpf_map_update_elem() and
> bpf_map_delete_elem() do not have an inlined version and need to call
> into their map->ops->map_update_elem() resp. map->ops->map_delete_elem()
> handlers.
> 
> Before:
> 
>   # bpftool p d x i 1

I would really appreciate if we can use the long options in these kind
of examples.  It makes the command "self-documenting" and searchable by
google.

Here it would be:

 # bpftool prog dump xlated id 1

>     0: (bf) r2 = r10
>     1: (07) r2 += -8
>     2: (7a) *(u64 *)(r2 +0) = 0
>     3: (18) r1 = map[id:1]
>     5: (85) call __htab_map_lookup_elem#232656
>     6: (15) if r0 == 0x0 goto pc+4
>     7: (71) r1 = *(u8 *)(r0 +35)
>     8: (55) if r1 != 0x0 goto pc+1
>     9: (72) *(u8 *)(r0 +35) = 1
>    10: (07) r0 += 56
>    11: (15) if r0 == 0x0 goto pc+4
>    12: (bf) r2 = r0
>    13: (18) r1 = map[id:1]
>    15: (85) call bpf_map_delete_elem#215008  <-- indirect call via
>    16: (95) exit                                 helper
> 
> After:
> 
>   # bpftool p d x i 1

Same here

>     0: (bf) r2 = r10
>     1: (07) r2 += -8
>     2: (7a) *(u64 *)(r2 +0) = 0
>     3: (18) r1 = map[id:1]
>     5: (85) call __htab_map_lookup_elem#233328
>     6: (15) if r0 == 0x0 goto pc+4
>     7: (71) r1 = *(u8 *)(r0 +35)
>     8: (55) if r1 != 0x0 goto pc+1
>     9: (72) *(u8 *)(r0 +35) = 1
>    10: (07) r0 += 56
>    11: (15) if r0 == 0x0 goto pc+4
>    12: (bf) r2 = r0
>    13: (18) r1 = map[id:1]
>    15: (85) call htab_lru_map_delete_elem#238240  <-- direct call
>    16: (95) exit
> 


-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH bpf-next 04/11] bpf: show prog and map id in fdinfo
From: Jesper Dangaard Brouer @ 2018-05-29 17:27 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: brouer, netdev
In-Reply-To: <20180528004344.3606-5-daniel@iogearbox.net>

On Mon, 28 May 2018 02:43:37 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:

> Its trivial and straight forward to expose it for scripts that can
> then use it along with bpftool in order to inspect an individual
> application's used maps and progs. Right now we dump some basic
> information in the fdinfo file but with the help of the map/prog
> id full introspection becomes possible now.
> 
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> ---

AFAICR iproute uses this proc fdinfo, for pinned maps.  Have you tested
if this change is handled gracefully by tc ?

>  kernel/bpf/syscall.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 388d4fe..79341e8 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -326,13 +326,15 @@ static void bpf_map_show_fdinfo(struct seq_file *m, struct file *filp)
>  		   "value_size:\t%u\n"
>  		   "max_entries:\t%u\n"
>  		   "map_flags:\t%#x\n"
> -		   "memlock:\t%llu\n",
> +		   "memlock:\t%llu\n"
> +		   "map_id:\t%u\n",
>  		   map->map_type,
>  		   map->key_size,
>  		   map->value_size,
>  		   map->max_entries,
>  		   map->map_flags,
> -		   map->pages * 1ULL << PAGE_SHIFT);
> +		   map->pages * 1ULL << PAGE_SHIFT,
> +		   map->id);
>  
>  	if (owner_prog_type) {
>  		seq_printf(m, "owner_prog_type:\t%u\n",
> @@ -1069,11 +1071,13 @@ static void bpf_prog_show_fdinfo(struct seq_file *m, struct file *filp)
>  		   "prog_type:\t%u\n"
>  		   "prog_jited:\t%u\n"
>  		   "prog_tag:\t%s\n"
> -		   "memlock:\t%llu\n",
> +		   "memlock:\t%llu\n"
> +		   "prog_id:\t%u\n",
>  		   prog->type,
>  		   prog->jited,
>  		   prog_tag,
> -		   prog->pages * 1ULL << PAGE_SHIFT);
> +		   prog->pages * 1ULL << PAGE_SHIFT,
> +		   prog->aux->id);
>  }
>  #endif
>  



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH bpf 2/2] bpf: enforce usage of __aligned_u64 in the UAPI header
From: Song Liu @ 2018-05-29 17:35 UTC (permalink / raw)
  To: Eugene Syromiatnikov
  Cc: netdev, open list, Martin KaFai Lau, Daniel Borkmann,
	Alexei Starovoitov, David S. Miller, Jiri Olsa, Ingo Molnar,
	Lawrence Brakmo, Andrey Ignatov, Jakub Kicinski, John Fastabend,
	Dmitry V. Levin
In-Reply-To: <20180527112847.GA18311@asgard.redhat.com>

On Sun, May 27, 2018 at 4:28 AM, Eugene Syromiatnikov <esyr@redhat.com> wrote:
> Use __aligned_u64 instead of __u64 everywhere in the UAPI header,
> similarly to v4.17-rc1~94^2~58^2 "RDMA: Change all uapi headers to use
> __aligned_u64 instead of __u64".
>
> This commit doesn't change structure layouts, but imposes additional
> alignment requirements on struct bpf_stack_build_id, struct
> bpf_sock_ops, struct bpf_perf_event_value, and struct
> bpf_raw_tracepoint_args; of them only struct bpf_sock_ops and struct
> bpf_perf_event_value have 64-bit fields that were present in a released
> kernel without this additional alignment requirement (bytes_received
> and bytes_acked were added to struct bpf_sock_ops in commit
> v4.16-rc1~123^2~33^2~5^2~4, struct bpf_perf_event_value was added
> in commit v4.15-rc1~84^2~532^2~3).
>
> Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
> ---
>  include/uapi/linux/bpf.h       | 22 +++++++++++-----------
>  tools/include/uapi/linux/bpf.h | 22 +++++++++++-----------
>  2 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 903010a..174e99a 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -259,8 +259,8 @@ struct bpf_stack_build_id {
>         __s32           status;
>         unsigned char   build_id[BPF_BUILD_ID_SIZE];
>         union {
> -               __u64   offset;
> -               __u64   ip;
> +               __aligned_u64   offset;
> +               __aligned_u64   ip;
>         };
>  };
>
> @@ -288,7 +288,7 @@ union bpf_attr {
>                         __aligned_u64 value;
>                         __aligned_u64 next_key;
>                 };
> -               __u64           flags;
> +               __aligned_u64   flags;
>         };
>
>         struct { /* anonymous struct used by BPF_PROG_LOAD command */
> @@ -360,7 +360,7 @@ union bpf_attr {
>         } query;
>
>         struct {
> -               __u64 name;
> +               __aligned_u64 name;
>                 __u32 prog_fd;
>         } raw_tracepoint;
>  } __attribute__((aligned(8)));
> @@ -1011,7 +1011,7 @@ struct bpf_prog_info {
>         __u32 xlated_prog_len;
>         __aligned_u64 jited_prog_insns;
>         __aligned_u64 xlated_prog_insns;
> -       __u64 load_time;        /* ns since boottime */
> +       __aligned_u64 load_time;        /* ns since boottime */
>         __u32 created_by_uid;
>         __u32 nr_map_ids;
>         __aligned_u64 map_ids;
> @@ -1101,8 +1101,8 @@ struct bpf_sock_ops {
>         __u32 lost_out;
>         __u32 sacked_out;
>         __u32 sk_txhash;
> -       __u64 bytes_received;
> -       __u64 bytes_acked;
> +       __aligned_u64 bytes_received;
> +       __aligned_u64 bytes_acked;
>  };
>
>  /* Definitions for bpf_sock_ops_cb_flags */
> @@ -1189,9 +1189,9 @@ enum {
>  #define TCP_BPF_SNDCWND_CLAMP  1002    /* Set sndcwnd_clamp */
>
>  struct bpf_perf_event_value {
> -       __u64 counter;
> -       __u64 enabled;
> -       __u64 running;
> +       __aligned_u64 counter;
> +       __aligned_u64 enabled;
> +       __aligned_u64 running;
>  };
>
>  #define BPF_DEVCG_ACC_MKNOD    (1ULL << 0)
> @@ -1209,7 +1209,7 @@ struct bpf_cgroup_dev_ctx {
>  };
>
>  struct bpf_raw_tracepoint_args {
> -       __u64 args[0];
> +       __aligned_u64 args[0];
>  };
>
>  #endif /* _UAPI__LINUX_BPF_H__ */
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 903010a..174e99a 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -259,8 +259,8 @@ struct bpf_stack_build_id {
>         __s32           status;
>         unsigned char   build_id[BPF_BUILD_ID_SIZE];
>         union {
> -               __u64   offset;
> -               __u64   ip;
> +               __aligned_u64   offset;
> +               __aligned_u64   ip;
>         };
>  };
>
> @@ -288,7 +288,7 @@ union bpf_attr {
>                         __aligned_u64 value;
>                         __aligned_u64 next_key;
>                 };
> -               __u64           flags;
> +               __aligned_u64   flags;
>         };
>
>         struct { /* anonymous struct used by BPF_PROG_LOAD command */
> @@ -360,7 +360,7 @@ union bpf_attr {
>         } query;
>
>         struct {
> -               __u64 name;
> +               __aligned_u64 name;
>                 __u32 prog_fd;
>         } raw_tracepoint;
>  } __attribute__((aligned(8)));
> @@ -1011,7 +1011,7 @@ struct bpf_prog_info {
>         __u32 xlated_prog_len;
>         __aligned_u64 jited_prog_insns;
>         __aligned_u64 xlated_prog_insns;
> -       __u64 load_time;        /* ns since boottime */
> +       __aligned_u64 load_time;        /* ns since boottime */
>         __u32 created_by_uid;
>         __u32 nr_map_ids;
>         __aligned_u64 map_ids;
> @@ -1101,8 +1101,8 @@ struct bpf_sock_ops {
>         __u32 lost_out;
>         __u32 sacked_out;
>         __u32 sk_txhash;
> -       __u64 bytes_received;
> -       __u64 bytes_acked;
> +       __aligned_u64 bytes_received;
> +       __aligned_u64 bytes_acked;
>  };
>
>  /* Definitions for bpf_sock_ops_cb_flags */
> @@ -1189,9 +1189,9 @@ enum {
>  #define TCP_BPF_SNDCWND_CLAMP  1002    /* Set sndcwnd_clamp */
>
>  struct bpf_perf_event_value {
> -       __u64 counter;
> -       __u64 enabled;
> -       __u64 running;
> +       __aligned_u64 counter;
> +       __aligned_u64 enabled;
> +       __aligned_u64 running;
>  };
>
>  #define BPF_DEVCG_ACC_MKNOD    (1ULL << 0)
> @@ -1209,7 +1209,7 @@ struct bpf_cgroup_dev_ctx {
>  };
>
>  struct bpf_raw_tracepoint_args {
> -       __u64 args[0];
> +       __aligned_u64 args[0];
>  };
>
>  #endif /* _UAPI__LINUX_BPF_H__ */
> --
> 2.1.4
>

I think these changes are not necessary. Is it a general guidance to
only use 64-bit aligned
variables in UAPI headers?

Thanks,
Song

^ permalink raw reply

* Re: [PATCH net] sctp: not allow to set rto_min with a value below 200 msecs
From: Xin Long @ 2018-05-29 17:45 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Neal Cardwell, Michael Tuexen, Neil Horman, Dmitry Vyukov, Netdev,
	linux-sctp, David Miller, David Ahern, Eric Dumazet, syzkaller
In-Reply-To: <20180529170604.GD3788@localhost.localdomain>

On Wed, May 30, 2018 at 1:06 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Tue, May 29, 2018 at 12:03:46PM -0400, Neal Cardwell wrote:
>> On Tue, May 29, 2018 at 11:45 AM Marcelo Ricardo Leitner <
>> marcelo.leitner@gmail.com> wrote:
>> > - patch2 - fix rtx attack vector
>> >    - Add the floor value to rto_min to HZ/20 (which fits the values
>> >      that Michael shared on the other email)
>>
>> I would encourage allowing minimum RTO values down to 5ms, if the ACK
>> policy in the receiver makes this feasible. Our experience is that in
>> datacenter environments it can be advantageous to allow timer-based loss
>> recoveries using timeout values as low as 5ms, e.g.:
>
> Thanks Neal. On Xin's tests, the hearbeat timer becomes an issue at
> ~25ms already. Xin, can you share more details on the hw, which CPU
> was used?
It was on a KVM guest,  "-smp 2,cores=1,threads=1,sockets=2"
# lscpu
Architecture:          x86_64
CPU op-mode(s):        32-bit, 64-bit
Byte Order:            Little Endian
CPU(s):                2
On-line CPU(s) list:   0,1
Thread(s) per core:    1
Core(s) per socket:    1
Socket(s):             2
NUMA node(s):          1
Vendor ID:             GenuineIntel
CPU family:            6
Model:                 13
Model name:            QEMU Virtual CPU version 1.5.3
Stepping:              3
CPU MHz:               2397.222
BogoMIPS:              4794.44
Hypervisor vendor:     KVM
Virtualization type:   full
L1d cache:             32K
L1i cache:             32K
L2 cache:              4096K
NUMA node0 CPU(s):     0,1
Flags:                 fpu de pse tsc msr pae mce cx8 apic sep mtrr
pge mca cmov pse36 clflush mmx fxsr sse sse2 syscall nx lm rep_good
nopl cpuid pni cx16 hypervisor lahf_lm abm pti

If we're counting on max_t to fix this CPU stuck. It should not that
matter if min rto < the value causing that stuck.

>
> Anyway, what about we add a floor to rto_max too, so that RTO can
> actually grow into something bigger that don't hog the CPU? Like:
> rto_min floor = 5ms
> rto_max floor = 50ms
>
>   Marcelo

^ permalink raw reply

* [PATCH bpf-next] bpf: Drop mpls from bpf_fib_lookup
From: dsahern @ 2018-05-29 17:58 UTC (permalink / raw)
  To: netdev, borkmann, ast; +Cc: David Ahern

From: David Ahern <dsahern@gmail.com>

MPLS support will not be submitted this dev cycle, but in working on it
I do see a few changes are needed to the API. For now, drop mpls from the
API. Since the fields in question are unions, the mpls fields can be added
back later without affecting the uapi.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/uapi/linux/bpf.h | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index cc68787f2d97..2dd440e39802 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1855,10 +1855,10 @@ union bpf_attr {
  *		If lookup is successful and result shows packet is to be
  *		forwarded, the neighbor tables are searched for the nexthop.
  *		If successful (ie., FIB lookup shows forwarding and nexthop
- *		is resolved), the nexthop address is returned in ipv4_dst,
- *		ipv6_dst or mpls_out based on family, smac is set to mac
- *		address of egress device, dmac is set to nexthop mac address,
- *		rt_metric is set to metric from route.
+ *		is resolved), the nexthop address is returned in ipv4_dst
+ *		or ipv6_dst based on family, smac is set to mac address of
+ *		egress device, dmac is set to nexthop mac address, rt_metric
+ *		is set to metric from route (IPv4/IPv6 only).
  *
  *             *plen* argument is the size of the passed in struct.
  *             *flags* argument can be one or more BPF_FIB_LOOKUP_ flags:
@@ -2538,8 +2538,10 @@ struct bpf_raw_tracepoint_args {
 #define BPF_FIB_LOOKUP_OUTPUT  BIT(1)
 
 struct bpf_fib_lookup {
-	/* input */
-	__u8	family;   /* network family, AF_INET, AF_INET6, AF_MPLS */
+	/* input:  network family for lookup (AF_INET, AF_INET6)
+	 * output: network family of egress nexthop
+	 */
+	__u8	family;
 
 	/* set if lookup is to consider L4 data - e.g., FIB rules */
 	__u8	l4_protocol;
@@ -2555,22 +2557,20 @@ struct bpf_fib_lookup {
 		__u8	tos;		/* AF_INET  */
 		__be32	flowlabel;	/* AF_INET6 */
 
-		/* output: metric of fib result */
-		__u32 rt_metric;
+		/* output: metric of fib result (IPv4/IPv6 only) */
+		__u32	rt_metric;
 	};
 
 	union {
-		__be32		mpls_in;
 		__be32		ipv4_src;
 		__u32		ipv6_src[4];  /* in6_addr; network order */
 	};
 
-	/* input to bpf_fib_lookup, *dst is destination address.
-	 * output: bpf_fib_lookup sets to gateway address
+	/* input to bpf_fib_lookup, ipv{4,6}_dst is destination address in
+	 * network header. output: bpf_fib_lookup sets to gateway address
+	 * if FIB lookup returns gateway route
 	 */
 	union {
-		/* return for MPLS lookups */
-		__be32		mpls_out[4];  /* support up to 4 labels */
 		__be32		ipv4_dst;
 		__u32		ipv6_dst[4];  /* in6_addr; network order */
 	};
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH bpf-next 01/11] bpf: test case for map pointer poison with calls/branches
From: Song Liu @ 2018-05-29 18:01 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Alexei Starovoitov, netdev
In-Reply-To: <20180528004344.3606-2-daniel@iogearbox.net>

On Sun, May 27, 2018 at 5:43 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> Add several test cases where the same or different map pointers
> originate from different paths in the program and execute a map
> lookup or tail call at a common location.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>

Acked-by: Song Liu <songliubraving@fb.com>

> ---
>  include/linux/filter.h                      |  10 ++
>  tools/include/linux/filter.h                |  10 ++
>  tools/testing/selftests/bpf/test_verifier.c | 185 ++++++++++++++++++++++++----
>  3 files changed, 178 insertions(+), 27 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index d358d18..b443f70 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -289,6 +289,16 @@ struct xdp_buff;
>                 .off   = OFF,                                   \
>                 .imm   = 0 })
>
> +/* Relative call */
> +
> +#define BPF_CALL_REL(TGT)                                      \
> +       ((struct bpf_insn) {                                    \
> +               .code  = BPF_JMP | BPF_CALL,                    \
> +               .dst_reg = 0,                                   \
> +               .src_reg = BPF_PSEUDO_CALL,                     \
> +               .off   = 0,                                     \
> +               .imm   = TGT })
> +
>  /* Function call */
>
>  #define BPF_EMIT_CALL(FUNC)                                    \
> diff --git a/tools/include/linux/filter.h b/tools/include/linux/filter.h
> index c5e512d..af55acf 100644
> --- a/tools/include/linux/filter.h
> +++ b/tools/include/linux/filter.h
> @@ -263,6 +263,16 @@
>  #define BPF_LD_MAP_FD(DST, MAP_FD)                             \
>         BPF_LD_IMM64_RAW(DST, BPF_PSEUDO_MAP_FD, MAP_FD)
>
> +/* Relative call */
> +
> +#define BPF_CALL_REL(TGT)                                      \
> +       ((struct bpf_insn) {                                    \
> +               .code  = BPF_JMP | BPF_CALL,                    \
> +               .dst_reg = 0,                                   \
> +               .src_reg = BPF_PSEUDO_CALL,                     \
> +               .off   = 0,                                     \
> +               .imm   = TGT })
> +
>  /* Program exit */
>
>  #define BPF_EXIT_INSN()                                                \
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index 4b4f015..7cb1d74 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -50,7 +50,7 @@
>
>  #define MAX_INSNS      BPF_MAXINSNS
>  #define MAX_FIXUPS     8
> -#define MAX_NR_MAPS    4
> +#define MAX_NR_MAPS    7
>  #define POINTER_VALUE  0xcafe4all
>  #define TEST_DATA_LEN  64
>
> @@ -66,7 +66,9 @@ struct bpf_test {
>         int fixup_map1[MAX_FIXUPS];
>         int fixup_map2[MAX_FIXUPS];
>         int fixup_map3[MAX_FIXUPS];
> -       int fixup_prog[MAX_FIXUPS];
> +       int fixup_map4[MAX_FIXUPS];
> +       int fixup_prog1[MAX_FIXUPS];
> +       int fixup_prog2[MAX_FIXUPS];
>         int fixup_map_in_map[MAX_FIXUPS];
>         const char *errstr;
>         const char *errstr_unpriv;
> @@ -2769,7 +2771,7 @@ static struct bpf_test tests[] = {
>                         BPF_MOV64_IMM(BPF_REG_0, 0),
>                         BPF_EXIT_INSN(),
>                 },
> -               .fixup_prog = { 1 },
> +               .fixup_prog1 = { 1 },
>                 .errstr_unpriv = "R3 leaks addr into helper",
>                 .result_unpriv = REJECT,
>                 .result = ACCEPT,
> @@ -2856,7 +2858,7 @@ static struct bpf_test tests[] = {
>                         BPF_MOV64_IMM(BPF_REG_0, 1),
>                         BPF_EXIT_INSN(),
>                 },
> -               .fixup_prog = { 1 },
> +               .fixup_prog1 = { 1 },
>                 .result = ACCEPT,
>                 .retval = 42,
>         },
> @@ -2870,7 +2872,7 @@ static struct bpf_test tests[] = {
>                         BPF_MOV64_IMM(BPF_REG_0, 1),
>                         BPF_EXIT_INSN(),
>                 },
> -               .fixup_prog = { 1 },
> +               .fixup_prog1 = { 1 },
>                 .result = ACCEPT,
>                 .retval = 41,
>         },
> @@ -2884,7 +2886,7 @@ static struct bpf_test tests[] = {
>                         BPF_MOV64_IMM(BPF_REG_0, 1),
>                         BPF_EXIT_INSN(),
>                 },
> -               .fixup_prog = { 1 },
> +               .fixup_prog1 = { 1 },
>                 .result = ACCEPT,
>                 .retval = 1,
>         },
> @@ -2898,7 +2900,7 @@ static struct bpf_test tests[] = {
>                         BPF_MOV64_IMM(BPF_REG_0, 2),
>                         BPF_EXIT_INSN(),
>                 },
> -               .fixup_prog = { 1 },
> +               .fixup_prog1 = { 1 },
>                 .result = ACCEPT,
>                 .retval = 2,
>         },
> @@ -2912,7 +2914,7 @@ static struct bpf_test tests[] = {
>                         BPF_MOV64_IMM(BPF_REG_0, 2),
>                         BPF_EXIT_INSN(),
>                 },
> -               .fixup_prog = { 1 },
> +               .fixup_prog1 = { 1 },
>                 .result = ACCEPT,
>                 .retval = 2,
>         },
> @@ -2926,7 +2928,7 @@ static struct bpf_test tests[] = {
>                         BPF_MOV64_IMM(BPF_REG_0, 2),
>                         BPF_EXIT_INSN(),
>                 },
> -               .fixup_prog = { 2 },
> +               .fixup_prog1 = { 2 },
>                 .result = ACCEPT,
>                 .retval = 42,
>         },
> @@ -11682,6 +11684,112 @@ static struct bpf_test tests[] = {
>                 .prog_type = BPF_PROG_TYPE_XDP,
>         },
>         {
> +               "calls: two calls returning different map pointers for lookup (hash, array)",
> +               .insns = {
> +                       /* main prog */
> +                       BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2),
> +                       BPF_CALL_REL(11),
> +                       BPF_JMP_IMM(BPF_JA, 0, 0, 1),
> +                       BPF_CALL_REL(12),
> +                       BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
> +                       BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
> +                       BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> +                       BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
> +                       BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> +                                    BPF_FUNC_map_lookup_elem),
> +                       BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
> +                       BPF_ST_MEM(BPF_DW, BPF_REG_0, 0,
> +                                  offsetof(struct test_val, foo)),
> +                       BPF_MOV64_IMM(BPF_REG_0, 1),
> +                       BPF_EXIT_INSN(),
> +                       /* subprog 1 */
> +                       BPF_LD_MAP_FD(BPF_REG_0, 0),
> +                       BPF_EXIT_INSN(),
> +                       /* subprog 2 */
> +                       BPF_LD_MAP_FD(BPF_REG_0, 0),
> +                       BPF_EXIT_INSN(),
> +               },
> +               .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> +               .fixup_map2 = { 13 },
> +               .fixup_map4 = { 16 },
> +               .result = ACCEPT,
> +               .retval = 1,
> +       },
> +       {
> +               "calls: two calls returning different map pointers for lookup (hash, map in map)",
> +               .insns = {
> +                       /* main prog */
> +                       BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2),
> +                       BPF_CALL_REL(11),
> +                       BPF_JMP_IMM(BPF_JA, 0, 0, 1),
> +                       BPF_CALL_REL(12),
> +                       BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
> +                       BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
> +                       BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> +                       BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
> +                       BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> +                                    BPF_FUNC_map_lookup_elem),
> +                       BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
> +                       BPF_ST_MEM(BPF_DW, BPF_REG_0, 0,
> +                                  offsetof(struct test_val, foo)),
> +                       BPF_MOV64_IMM(BPF_REG_0, 1),
> +                       BPF_EXIT_INSN(),
> +                       /* subprog 1 */
> +                       BPF_LD_MAP_FD(BPF_REG_0, 0),
> +                       BPF_EXIT_INSN(),
> +                       /* subprog 2 */
> +                       BPF_LD_MAP_FD(BPF_REG_0, 0),
> +                       BPF_EXIT_INSN(),
> +               },
> +               .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> +               .fixup_map_in_map = { 16 },
> +               .fixup_map4 = { 13 },
> +               .result = REJECT,
> +               .errstr = "R0 invalid mem access 'map_ptr'",
> +       },
> +       {
> +               "cond: two branches returning different map pointers for lookup (tail, tail)",
> +               .insns = {
> +                       BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
> +                                   offsetof(struct __sk_buff, mark)),
> +                       BPF_JMP_IMM(BPF_JNE, BPF_REG_6, 0, 3),
> +                       BPF_LD_MAP_FD(BPF_REG_2, 0),
> +                       BPF_JMP_IMM(BPF_JA, 0, 0, 2),
> +                       BPF_LD_MAP_FD(BPF_REG_2, 0),
> +                       BPF_MOV64_IMM(BPF_REG_3, 7),
> +                       BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> +                                    BPF_FUNC_tail_call),
> +                       BPF_MOV64_IMM(BPF_REG_0, 1),
> +                       BPF_EXIT_INSN(),
> +               },
> +               .fixup_prog1 = { 5 },
> +               .fixup_prog2 = { 2 },
> +               .result_unpriv = REJECT,
> +               .errstr_unpriv = "tail_call abusing map_ptr",
> +               .result = ACCEPT,
> +               .retval = 42,
> +       },
> +       {
> +               "cond: two branches returning same map pointers for lookup (tail, tail)",
> +               .insns = {
> +                       BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
> +                                   offsetof(struct __sk_buff, mark)),
> +                       BPF_JMP_IMM(BPF_JEQ, BPF_REG_6, 0, 3),
> +                       BPF_LD_MAP_FD(BPF_REG_2, 0),
> +                       BPF_JMP_IMM(BPF_JA, 0, 0, 2),
> +                       BPF_LD_MAP_FD(BPF_REG_2, 0),
> +                       BPF_MOV64_IMM(BPF_REG_3, 7),
> +                       BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> +                                    BPF_FUNC_tail_call),
> +                       BPF_MOV64_IMM(BPF_REG_0, 1),
> +                       BPF_EXIT_INSN(),
> +               },
> +               .fixup_prog2 = { 2, 5 },
> +               .result_unpriv = ACCEPT,
> +               .result = ACCEPT,
> +               .retval = 42,
> +       },
> +       {
>                 "search pruning: all branches should be verified (nop operation)",
>                 .insns = {
>                         BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> @@ -12162,12 +12270,13 @@ static int probe_filter_length(const struct bpf_insn *fp)
>         return len + 1;
>  }
>
> -static int create_map(uint32_t size_value, uint32_t max_elem)
> +static int create_map(uint32_t type, uint32_t size_key,
> +                     uint32_t size_value, uint32_t max_elem)
>  {
>         int fd;
>
> -       fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(long long),
> -                           size_value, max_elem, BPF_F_NO_PREALLOC);
> +       fd = bpf_create_map(type, size_key, size_value, max_elem,
> +                           type == BPF_MAP_TYPE_HASH ? BPF_F_NO_PREALLOC : 0);
>         if (fd < 0)
>                 printf("Failed to create hash map '%s'!\n", strerror(errno));
>
> @@ -12200,13 +12309,13 @@ static int create_prog_dummy2(int mfd, int idx)
>                                 ARRAY_SIZE(prog), "GPL", 0, NULL, 0);
>  }
>
> -static int create_prog_array(void)
> +static int create_prog_array(uint32_t max_elem, int p1key)
>  {
> -       int p1key = 0, p2key = 1;
> +       int p2key = 1;
>         int mfd, p1fd, p2fd;
>
>         mfd = bpf_create_map(BPF_MAP_TYPE_PROG_ARRAY, sizeof(int),
> -                            sizeof(int), 4, 0);
> +                            sizeof(int), max_elem, 0);
>         if (mfd < 0) {
>                 printf("Failed to create prog array '%s'!\n", strerror(errno));
>                 return -1;
> @@ -12261,7 +12370,9 @@ static void do_test_fixup(struct bpf_test *test, struct bpf_insn *prog,
>         int *fixup_map1 = test->fixup_map1;
>         int *fixup_map2 = test->fixup_map2;
>         int *fixup_map3 = test->fixup_map3;
> -       int *fixup_prog = test->fixup_prog;
> +       int *fixup_map4 = test->fixup_map4;
> +       int *fixup_prog1 = test->fixup_prog1;
> +       int *fixup_prog2 = test->fixup_prog2;
>         int *fixup_map_in_map = test->fixup_map_in_map;
>
>         if (test->fill_helper)
> @@ -12272,7 +12383,8 @@ static void do_test_fixup(struct bpf_test *test, struct bpf_insn *prog,
>          * that really matters is value size in this case.
>          */
>         if (*fixup_map1) {
> -               map_fds[0] = create_map(sizeof(long long), 1);
> +               map_fds[0] = create_map(BPF_MAP_TYPE_HASH, sizeof(long long),
> +                                       sizeof(long long), 1);
>                 do {
>                         prog[*fixup_map1].imm = map_fds[0];
>                         fixup_map1++;
> @@ -12280,7 +12392,8 @@ static void do_test_fixup(struct bpf_test *test, struct bpf_insn *prog,
>         }
>
>         if (*fixup_map2) {
> -               map_fds[1] = create_map(sizeof(struct test_val), 1);
> +               map_fds[1] = create_map(BPF_MAP_TYPE_HASH, sizeof(long long),
> +                                       sizeof(struct test_val), 1);
>                 do {
>                         prog[*fixup_map2].imm = map_fds[1];
>                         fixup_map2++;
> @@ -12288,25 +12401,43 @@ static void do_test_fixup(struct bpf_test *test, struct bpf_insn *prog,
>         }
>
>         if (*fixup_map3) {
> -               map_fds[1] = create_map(sizeof(struct other_val), 1);
> +               map_fds[2] = create_map(BPF_MAP_TYPE_HASH, sizeof(long long),
> +                                       sizeof(struct other_val), 1);
>                 do {
> -                       prog[*fixup_map3].imm = map_fds[1];
> +                       prog[*fixup_map3].imm = map_fds[2];
>                         fixup_map3++;
>                 } while (*fixup_map3);
>         }
>
> -       if (*fixup_prog) {
> -               map_fds[2] = create_prog_array();
> +       if (*fixup_map4) {
> +               map_fds[3] = create_map(BPF_MAP_TYPE_ARRAY, sizeof(int),
> +                                       sizeof(struct test_val), 1);
> +               do {
> +                       prog[*fixup_map4].imm = map_fds[3];
> +                       fixup_map4++;
> +               } while (*fixup_map4);
> +       }
> +
> +       if (*fixup_prog1) {
> +               map_fds[4] = create_prog_array(4, 0);
> +               do {
> +                       prog[*fixup_prog1].imm = map_fds[4];
> +                       fixup_prog1++;
> +               } while (*fixup_prog1);
> +       }
> +
> +       if (*fixup_prog2) {
> +               map_fds[5] = create_prog_array(8, 7);
>                 do {
> -                       prog[*fixup_prog].imm = map_fds[2];
> -                       fixup_prog++;
> -               } while (*fixup_prog);
> +                       prog[*fixup_prog2].imm = map_fds[5];
> +                       fixup_prog2++;
> +               } while (*fixup_prog2);
>         }
>
>         if (*fixup_map_in_map) {
> -               map_fds[3] = create_map_in_map();
> +               map_fds[6] = create_map_in_map();
>                 do {
> -                       prog[*fixup_map_in_map].imm = map_fds[3];
> +                       prog[*fixup_map_in_map].imm = map_fds[6];
>                         fixup_map_in_map++;
>                 } while (*fixup_map_in_map);
>         }
> --
> 2.9.5
>

^ permalink raw reply

* Re: [PATCH net] sctp: not allow to set rto_min with a value below 200 msecs
From: Marcelo Ricardo Leitner @ 2018-05-29 18:02 UTC (permalink / raw)
  To: Xin Long
  Cc: Neal Cardwell, Michael Tuexen, Neil Horman, Dmitry Vyukov, Netdev,
	linux-sctp, David Miller, David Ahern, Eric Dumazet, syzkaller
In-Reply-To: <CADvbK_fbKbH2wm6Xurr+ELVag-LvyQdL+peJd=wp7OL7_zMZTQ@mail.gmail.com>

On Wed, May 30, 2018 at 01:45:08AM +0800, Xin Long wrote:
> If we're counting on max_t to fix this CPU stuck. It should not that
> matter if min rto < the value causing that stuck.

Yes but putting a floor to rto_{min,max} now is to protect the rtx
timer now, not the heartbeat one.

> 
> >
> > Anyway, what about we add a floor to rto_max too, so that RTO can
> > actually grow into something bigger that don't hog the CPU? Like:
> > rto_min floor = 5ms
> > rto_max floor = 50ms
> >
> >   Marcelo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: [PATCH bpf-next 02/11] bpf: add also cbpf long jump test cases with heavy expansion
From: Song Liu @ 2018-05-29 18:09 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Alexei Starovoitov, netdev
In-Reply-To: <20180528004344.3606-3-daniel@iogearbox.net>

On Sun, May 27, 2018 at 5:43 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> We have one triggering on eBPF but lets also add a cBPF example to
> make sure we keep tracking them. Also add anther cBPF test running
> max number of MSH ops.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>

Acked-by: Song Liu <songliubraving@fb.com>


> ---
>  lib/test_bpf.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>
> diff --git a/lib/test_bpf.c b/lib/test_bpf.c
> index 317f231..60aedc8 100644
> --- a/lib/test_bpf.c
> +++ b/lib/test_bpf.c
> @@ -356,6 +356,52 @@ static int bpf_fill_maxinsns11(struct bpf_test *self)
>         return __bpf_fill_ja(self, BPF_MAXINSNS, 68);
>  }
>
> +static int bpf_fill_maxinsns12(struct bpf_test *self)
> +{
> +       unsigned int len = BPF_MAXINSNS;
> +       struct sock_filter *insn;
> +       int i = 0;
> +
> +       insn = kmalloc_array(len, sizeof(*insn), GFP_KERNEL);
> +       if (!insn)
> +               return -ENOMEM;
> +
> +       insn[0] = __BPF_JUMP(BPF_JMP | BPF_JA, len - 2, 0, 0);
> +
> +       for (i = 1; i < len - 1; i++)
> +               insn[i] = __BPF_STMT(BPF_LDX | BPF_B | BPF_MSH, 0);
> +
> +       insn[len - 1] = __BPF_STMT(BPF_RET | BPF_K, 0xabababab);
> +
> +       self->u.ptr.insns = insn;
> +       self->u.ptr.len = len;
> +
> +       return 0;
> +}
> +
> +static int bpf_fill_maxinsns13(struct bpf_test *self)
> +{
> +       unsigned int len = BPF_MAXINSNS;
> +       struct sock_filter *insn;
> +       int i = 0;
> +
> +       insn = kmalloc_array(len, sizeof(*insn), GFP_KERNEL);
> +       if (!insn)
> +               return -ENOMEM;
> +
> +       for (i = 0; i < len - 3; i++)
> +               insn[i] = __BPF_STMT(BPF_LDX | BPF_B | BPF_MSH, 0);
> +
> +       insn[len - 3] = __BPF_STMT(BPF_LD | BPF_IMM, 0xabababab);
> +       insn[len - 2] = __BPF_STMT(BPF_ALU | BPF_XOR | BPF_X, 0);
> +       insn[len - 1] = __BPF_STMT(BPF_RET | BPF_A, 0);
> +
> +       self->u.ptr.insns = insn;
> +       self->u.ptr.len = len;
> +
> +       return 0;
> +}
> +
>  static int bpf_fill_ja(struct bpf_test *self)
>  {
>         /* Hits exactly 11 passes on x86_64 JIT. */
> @@ -5290,6 +5336,23 @@ static struct bpf_test tests[] = {
>                 .expected_errcode = -ENOTSUPP,
>         },
>         {
> +               "BPF_MAXINSNS: jump over MSH",
> +               { },
> +               CLASSIC | FLAG_EXPECTED_FAIL,
> +               { 0xfa, 0xfb, 0xfc, 0xfd, },
> +               { { 4, 0xabababab } },
> +               .fill_helper = bpf_fill_maxinsns12,
> +               .expected_errcode = -EINVAL,
> +       },
> +       {
> +               "BPF_MAXINSNS: exec all MSH",
> +               { },
> +               CLASSIC,
> +               { 0xfa, 0xfb, 0xfc, 0xfd, },
> +               { { 4, 0xababab83 } },
> +               .fill_helper = bpf_fill_maxinsns13,
> +       },
> +       {
>                 "BPF_MAXINSNS: ld_abs+get_processor_id",
>                 { },
>                 CLASSIC,
> --
> 2.9.5
>

^ permalink raw reply

* Re: [PATCH bpf-next 03/11] bpf: fixup error message from gpl helpers on license mismatch
From: Song Liu @ 2018-05-29 18:10 UTC (permalink / raw)
  To: Jesper Dangaard Brouer; +Cc: Daniel Borkmann, netdev
In-Reply-To: <20180529191628.3f6aa7dd@redhat.com>

On Tue, May 29, 2018 at 10:16 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Mon, 28 May 2018 02:43:36 +0200
> Daniel Borkmann <daniel@iogearbox.net> wrote:
>
>> Stating 'proprietary program' in the error is just silly since it
>> can also be a different open source license than that which is just
>> not compatible.
>>
>> Reference: https://twitter.com/majek04/status/998531268039102465
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>> Acked-by: Alexei Starovoitov <ast@kernel.org>
>
> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
> Thank you for cleaning up this confusion :-)
>

Acked-by: Song Liu <songliubraving@fb.com>

>> ---
>>  kernel/bpf/verifier.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 1fd9667b..4f4786e 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -2462,7 +2462,7 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
>>
>>       /* eBPF programs must be GPL compatible to use GPL-ed functions */
>>       if (!env->prog->gpl_compatible && fn->gpl_only) {
>> -             verbose(env, "cannot call GPL only function from proprietary program\n");
>> +             verbose(env, "cannot call GPL-restricted function from non-GPL compatible program\n");
>>               return -EINVAL;
>>       }
>>
>
>
>
> --
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: hide the unused 'off' variable
From: Song Liu @ 2018-05-29 18:18 UTC (permalink / raw)
  To: John Fastabend
  Cc: Arnd Bergmann, YueHaibing, David Miller, Alexei Starovoitov,
	Daniel Borkmann, Networking, Linux Kernel Mailing List
In-Reply-To: <4a060ed5-f147-6778-caa2-7866b1f04479@gmail.com>

On Tue, May 29, 2018 at 8:53 AM, John Fastabend
<john.fastabend@gmail.com> wrote:
> On 05/29/2018 03:35 AM, Arnd Bergmann wrote:
>> On Tue, May 29, 2018 at 4:40 AM, YueHaibing <yuehaibing@huawei.com> wrote:
>>> The local variable is only used while CONFIG_IPV6 enabled
>>>
>>> net/core/filter.c: In function ‘sk_msg_convert_ctx_access’:
>>> net/core/filter.c:6489:6: warning: unused variable ‘off’ [-Wunused-variable]
>>>   int off;
>>>       ^
>>> This puts it into #ifdef.
>>>
>>> Fixes: 303def35f64e ("bpf: allow sk_msg programs to read sock fields")
>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>>
>> I was about to send the same patch and found you had already sent one.
>>
>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>>
>
> Thanks! I'm curious why kbuild bot didn't catch this. Will
> try to dig into that in a bit.
>
> Acked-by: John Fastabend <john.fastabend@gmail.com>

Acked-by: Song Liu <songliubraving@fb.com>

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: clean up eBPF helpers documentation
From: Song Liu @ 2018-05-29 18:27 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Daniel Borkmann, Alexei Starovoitov, Networking, oss-drivers
In-Reply-To: <20180529112744.16406-1-quentin.monnet@netronome.com>

On Tue, May 29, 2018 at 4:27 AM, Quentin Monnet
<quentin.monnet@netronome.com> wrote:
> These are minor edits for the eBPF helpers documentation in
> include/uapi/linux/bpf.h.
>
> The main fix consists in removing "BPF_FIB_LOOKUP_", because it ends
> with a non-escaped underscore that gets interpreted by rst2man and
> produces the following message in the resulting manual page:
>
>     DOCUTILS SYSTEM MESSAGES
>            System Message: ERROR/3 (/tmp/bpf-helpers.rst:, line 1514)
>                   Unknown target name: "bpf_fib_lookup".
>
> Other edits consist in:
>
> - Improving formatting for flag values for "bpf_fib_lookup()" helper.
> - Emphasising a parameter name in description of the return value for
>   "bpf_get_stack()" helper.
> - Removing unnecessary blank lines between "Description" and "Return"
>   sections for the few helpers that would use it, for consistency.
>
> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> ---
>  include/uapi/linux/bpf.h | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index cc68787f2d97..3f556b35ac8d 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1010,7 +1010,6 @@ union bpf_attr {
>   *             ::
>   *
>   *                     # sysctl kernel.perf_event_max_stack=<new value>
> - *
>   *     Return
>   *             The positive or null stack id on success, or a negative error
>   *             in case of failure.
> @@ -1821,10 +1820,9 @@ union bpf_attr {
>   *             ::
>   *
>   *                     # sysctl kernel.perf_event_max_stack=<new value>
> - *
>   *     Return
> - *             a non-negative value equal to or less than size on success, or
> - *             a negative error in case of failure.
> + *             A non-negative value equal to or less than *size* on success,
> + *             or a negative error in case of failure.
>   *
>   * int skb_load_bytes_relative(const struct sk_buff *skb, u32 offset, void *to, u32 len, u32 start_header)
>   *     Description
> @@ -1845,7 +1843,6 @@ union bpf_attr {
>   *             in socket filters where *skb*\ **->data** does not always point
>   *             to the start of the mac header and where "direct packet access"
>   *             is not available.
> - *
>   *     Return
>   *             0 on success, or a negative error in case of failure.
>   *
> @@ -1861,16 +1858,18 @@ union bpf_attr {
>   *             rt_metric is set to metric from route.
>   *
>   *             *plen* argument is the size of the passed in struct.
> - *             *flags* argument can be one or more BPF_FIB_LOOKUP_ flags:
> + *             *flags* argument can be a combination of one or more of the
> + *             following values:
>   *
> - *             **BPF_FIB_LOOKUP_DIRECT** means do a direct table lookup vs
> - *             full lookup using FIB rules
> - *             **BPF_FIB_LOOKUP_OUTPUT** means do lookup from an egress
> - *             perspective (default is ingress)
> + *             **BPF_FIB_LOOKUP_DIRECT**
> + *                     Do a direct table lookup vs full lookup using FIB
> + *                     rules.
> + *             **BPF_FIB_LOOKUP_OUTPUT**
> + *                     Perform lookup from an egress perspective (default is
> + *                     ingress).
>   *
>   *             *ctx* is either **struct xdp_md** for XDP programs or
>   *             **struct sk_buff** tc cls_act programs.
> - *
>   *     Return
>   *             Egress device index on success, 0 if packet needs to continue
>   *             up the stack for further processing or a negative error in case
> --
> 2.14.1
>

Please also apply the same changes to tools/include/uapi/linux/bpf.h.

Other than this, it looks to me.

Acked-by: Song Liu <songliubraving@fb.com>

Thanks,
Song

^ permalink raw reply

* Re: [PATCH 0/4 RFCv2] Realtek SMI RTL836x DSA driver
From: Linus Walleij @ 2018-05-29 18:41 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Vivien Didelot, Florian Fainelli, netdev,
	OpenWrt Development List, LEDE Development List
In-Reply-To: <20180529122424.GC10919@lunn.ch>

On Tue, May 29, 2018 at 2:24 PM, Andrew Lunn <andrew@lunn.ch> wrote:

> Did you look at the switch end? I found a datasheet for the
> 8366/8369. Register at 0x0050, P8GCR. It has two bits for RGMII
> delays.

Unfortunately this datasheet is not applicable to RTL8366RB.

RTL documentation and model numbers are a complete mess
around the time when this chip came out, unfortunately... I even
started to implement using that datasheet and had to toss a bunch
of stuff away.

There might not even be a proper datasheet for RTL8366RB,
I'm afraid. The best we have is different (3 different AFAICT)
vendor code drops. Here is one drop over at DD-WRT:
https://svn.dd-wrt.com//browser/src/linux/universal/linux-3.2/drivers/net/ethernet/raeth/rb

As you can see, the RTL8366RB vendor driver consists of
a hacked version of their RTL8368S driver, so apparently those
two ASICs are similar, they even kept the same filenames.

For example the register defintions:
https://svn.dd-wrt.com/browser/src/linux/universal/linux-3.2/drivers/net/ethernet/raeth/rb/rtl8368s_reg.h

> With RGMII delays, you have 3 'choices'.
>
> 1) The hardware design includes the delay, by zig-zagging the clock
> line to make it longer.
> 2) The 'MAC' side does the delay.
> 3) The 'PHY' side does the delay.
>
> I normally recommend the PHY side doing it, because that's what most
> board do. Gives us some consistency. But it does not really
> matter. Just make sure one side, and only once side is inserting the
> delays.

Makes sense! But I haven't found anything applicable in the
RTL8366RB registers.

There are some jam tables with magic values written all over
the place that have no documentation, I fear this is one of the
settings poked around with there.

However, even if this router did not come with any code for
the RTL8366RB driver, I disassembled the binary to verify
that they use the same magic jam table, so the ASIC is
initialized in the same way.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [pull request][for-next 00/12] Mellanox, mlx5e updates 2018-05-25
From: Saeed Mahameed @ 2018-05-29 18:45 UTC (permalink / raw)
  To: davem@davemloft.net; +Cc: netdev@vger.kernel.org
In-Reply-To: <20180529.094751.1897507632916752927.davem@davemloft.net>

On Tue, 2018-05-29 at 09:47 -0400, David Miller wrote:
> From: Saeed Mahameed <saeedm@mellanox.com>
> Date: Fri, 25 May 2018 17:01:55 -0700
> 
> > This is a mlx5e only pull request, for more information please see
> > tag
> > log below.
> > 
> > Please pull and let me know if there's any problem.
> 
> Pulled, thanks Saeed.
> 
> There was a minor conflict to resolve (simple overlapping changes).

Yes, Sorry i didn't notify on this in advance, the merge commit looks
ok, thanks a lot!.


^ permalink raw reply

* Re: [1/4,RFCv2] net: phy: realtek: Support RTL8366RB variant
From: Heiner Kallweit @ 2018-05-29 18:51 UTC (permalink / raw)
  To: Linus Walleij, Andrew Lunn, Vivien Didelot, Florian Fainelli
  Cc: netdev, openwrt-devel, LEDE Development List,
	Antti Seppälä, Roman Yeryomin, Colin Leitner,
	Gabor Juhos
In-Reply-To: <20180528174752.6806-2-linus.walleij@linaro.org>

Am 28.05.2018 um 19:47 schrieb Linus Walleij:
> The RTL8366RB is an ASIC with five internal PHYs for
> LAN0..LAN3 and WAN. The PHYs are spawn off the main
> device so they can be handled in a distributed manner
> by the Realtek PHY driver. All that is really needed
> is the power save feature enablement and letting the
> PHY driver core pick up the IRQ from the switch chip.
> 
> Cc: Antti Seppälä <a.seppala@gmail.com>
> Cc: Roman Yeryomin <roman@advem.lv>
> Cc: Colin Leitner <colin.leitner@googlemail.com>
> Cc: Gabor Juhos <juhosg@openwrt.org>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog RFCv1->RFCv2:
> - No real changes.
> ---
>  drivers/net/phy/realtek.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> index 9f48ecf9c627..21624d1c9d38 100644
> --- a/drivers/net/phy/realtek.c
> +++ b/drivers/net/phy/realtek.c
> @@ -37,6 +37,9 @@
>  #define RTL8201F_ISR				0x1e
>  #define RTL8201F_IER				0x13
>  
> +#define RTL8366RB_POWER_SAVE	0x21
Typically PHY register addresses are 5 bits wide, is 0x21 correct
and I miss something?

> +#define RTL8366RB_POWER_SAVE_ON 0x1000
Use BIT(12) instead?

> +
>  MODULE_DESCRIPTION("Realtek PHY driver");
>  MODULE_AUTHOR("Johnson Leung");
>  MODULE_LICENSE("GPL");
> @@ -145,6 +148,22 @@ static int rtl8211f_config_init(struct phy_device *phydev)
>  	return phy_modify_paged(phydev, 0xd08, 0x11, RTL8211F_TX_DELAY, val);
>  }
>  
> +static int rtl8366rb_config_init(struct phy_device *phydev)
> +{
> +	int ret;
> +	u16 reg;
> +
> +	ret = genphy_config_init(phydev);
> +	if (ret < 0)
> +		return ret;
> +
> +	reg = phy_read(phydev, RTL8366RB_POWER_SAVE);
> +	reg |= RTL8366RB_POWER_SAVE_ON;
> +	phy_write(phydev, RTL8366RB_POWER_SAVE, reg);
return phy_set_bits(phydev, RTL8366RB_POWER_SAVE, RTL8366RB_POWER_SAVE_ON);
would be easier.

> +
> +	return 0;
> +}
> +
>  static struct phy_driver realtek_drvs[] = {
>  	{
>  		.phy_id         = 0x00008201,
> @@ -207,6 +226,18 @@ static struct phy_driver realtek_drvs[] = {
>  		.resume		= genphy_resume,
>  		.read_page	= rtl821x_read_page,
>  		.write_page	= rtl821x_write_page,
> +	}, {
> +		/* The main part of this DSA is in drivers/net/dsa */
> +		.phy_id		= 0x001cc961,
> +		.name		= "RTL8366RB Gigabit Ethernet",
> +		.phy_id_mask	= 0x001fffff,
> +		.features	= PHY_GBIT_FEATURES,
> +		.flags		= PHY_HAS_INTERRUPT,
> +		.config_aneg	= &genphy_config_aneg,
Can be omitted, genphy_config_aneg is the default since 00fde79532d6
"net: phy: core: use genphy version of callbacks read_status and
config_aneg per default".

> +		.config_init	= &rtl8366rb_config_init,
> +		.read_status	= &genphy_read_status,
Can be omitted, genphy_read_status is the default.

> +		.suspend	= genphy_suspend,
> +		.resume		= genphy_resume,
>  	},
>  };
>  
> @@ -218,6 +249,7 @@ static struct mdio_device_id __maybe_unused realtek_tbl[] = {
>  	{ 0x001cc914, 0x001fffff },
>  	{ 0x001cc915, 0x001fffff },
>  	{ 0x001cc916, 0x001fffff },
> +	{ 0x001cc961, 0x001fffff },
>  	{ }
>  };
>  
> 

^ permalink raw reply

* Re: [OpenWrt-Devel] [PATCH 0/4 RFCv2] Realtek SMI RTL836x DSA driver
From: Kevin Darbyshire-Bryant @ 2018-05-29 18:57 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Andrew Lunn, netdev, Florian Fainelli, LEDE Development List,
	Vivien Didelot
In-Reply-To: <CACRpkdbem_jTe6cQwTeTi9Uhk2B88rGENePZvjbsq38mfvcs0A@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2631 bytes --]



> On 29 May 2018, at 19:41, Linus Walleij <linus.walleij@linaro.org> wrote:
> 
> On Tue, May 29, 2018 at 2:24 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> 
>> Did you look at the switch end? I found a datasheet for the
>> 8366/8369. Register at 0x0050, P8GCR. It has two bits for RGMII
>> delays.
> 
> Unfortunately this datasheet is not applicable to RTL8366RB.
> 
> RTL documentation and model numbers are a complete mess
> around the time when this chip came out, unfortunately... I even
> started to implement using that datasheet and had to toss a bunch
> of stuff away.
> 
> There might not even be a proper datasheet for RTL8366RB,
> I'm afraid. The best we have is different (3 different AFAICT)
> vendor code drops. Here is one drop over at DD-WRT:
> https://svn.dd-wrt.com//browser/src/linux/universal/linux-3.2/drivers/net/ethernet/raeth/rb
> 
> As you can see, the RTL8366RB vendor driver consists of
> a hacked version of their RTL8368S driver, so apparently those
> two ASICs are similar, they even kept the same filenames.
> 
> For example the register defintions:
> https://svn.dd-wrt.com/browser/src/linux/universal/linux-3.2/drivers/net/ethernet/raeth/rb/rtl8368s_reg.h
> 
>> With RGMII delays, you have 3 'choices'.
>> 
>> 1) The hardware design includes the delay, by zig-zagging the clock
>> line to make it longer.
>> 2) The 'MAC' side does the delay.
>> 3) The 'PHY' side does the delay.
>> 
>> I normally recommend the PHY side doing it, because that's what most
>> board do. Gives us some consistency. But it does not really
>> matter. Just make sure one side, and only once side is inserting the
>> delays.
> 
> Makes sense! But I haven't found anything applicable in the
> RTL8366RB registers.
> 
> There are some jam tables with magic values written all over
> the place that have no documentation, I fear this is one of the
> settings poked around with there.
> 
> However, even if this router did not come with any code for
> the RTL8366RB driver, I disassembled the binary to verify
> that they use the same magic jam table, so the ASIC is
> initialized in the same way.
> 
> Yours,
> Linus Walleij
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/listinfo/openwrt-devel

Oh lordy, that horrible device as exhibited in the netgear DGN3500.  Talk about magic values https://git.openwrt.org/?p=openwrt/openwrt.git;a=commit;h=42120bd7f323ff7170b32a5fd4674babd8b184bc

Cheers,

Kevin D-B

012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A


[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH bpf-next] bpf: Verify flags in bpf_fib_lookup
From: dsahern @ 2018-05-29 18:59 UTC (permalink / raw)
  To: netdev, borkmann, ast; +Cc: David Ahern

From: David Ahern <dsahern@gmail.com>

Verify flags argument contains only known flags. Allows programs to probe
for support as more are added.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/filter.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index 24e6ce8be567..4cff6d9cd724 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4270,6 +4270,9 @@ BPF_CALL_4(bpf_xdp_fib_lookup, struct xdp_buff *, ctx,
 	if (plen < sizeof(*params))
 		return -EINVAL;
 
+	if (flags & ~(BPF_FIB_LOOKUP_DIRECT | BPF_FIB_LOOKUP_OUTPUT))
+		return -EINVAL;
+
 	switch (params->family) {
 #if IS_ENABLED(CONFIG_INET)
 	case AF_INET:
@@ -4304,6 +4307,9 @@ BPF_CALL_4(bpf_skb_fib_lookup, struct sk_buff *, skb,
 	if (plen < sizeof(*params))
 		return -EINVAL;
 
+	if (flags & ~(BPF_FIB_LOOKUP_DIRECT | BPF_FIB_LOOKUP_OUTPUT))
+		return -EINVAL;
+
 	switch (params->family) {
 #if IS_ENABLED(CONFIG_INET)
 	case AF_INET:
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH net-next 1/5] net: aquantia: Ethtool based ring size configuration
From: Jakub Kicinski @ 2018-05-29 19:04 UTC (permalink / raw)
  To: Igor Russkikh
  Cc: David S . Miller, netdev, David Arcari, Pavel Belous,
	Anton Mikaev
In-Reply-To: <7b86f8ec337f9c27a362c5a0a5c306928b07b04e.1527596210.git.igor.russkikh@aquantia.com>

On Tue, 29 May 2018 15:56:58 +0300, Igor Russkikh wrote:
> +static int aq_set_ringparam(struct net_device *ndev,
> +			    struct ethtool_ringparam *ring)
> +{
> +	int err = 0;
> +	struct aq_nic_s *aq_nic = netdev_priv(ndev);
> +	struct aq_nic_cfg_s *aq_nic_cfg = aq_nic_get_cfg(aq_nic);
> +	const struct aq_hw_caps_s *hw_caps = aq_nic_cfg->aq_hw_caps;
> +
> +	if (ring->rx_mini_pending || ring->rx_jumbo_pending) {
> +		err = -EOPNOTSUPP;
> +		goto err_exit;
> +	}
> +
> +	spin_lock(&aq_nic->aq_spinlock);
> +
> +	if (netif_running(ndev))
> +		dev_close(ndev);

I don't think you can hold a spinlock around dev_close()/dev_open()
calls.

> +	aq_nic_free_vectors(aq_nic);
> +
> +	aq_nic_cfg->rxds = max(ring->rx_pending, hw_caps->rxds_min);
> +	aq_nic_cfg->rxds = min(aq_nic_cfg->rxds, hw_caps->rxds_max);
> +	aq_nic_cfg->rxds = ALIGN(aq_nic_cfg->rxds, AQ_HW_RXD_MULTIPLE);
> +
> +	aq_nic_cfg->txds = max(ring->tx_pending, hw_caps->txds_min);
> +	aq_nic_cfg->txds = min(aq_nic_cfg->txds, hw_caps->txds_max);
> +	aq_nic_cfg->txds = ALIGN(aq_nic_cfg->txds, AQ_HW_TXD_MULTIPLE);
> +
> +	for (aq_nic->aq_vecs = 0; aq_nic->aq_vecs < aq_nic_cfg->vecs;
> +	     aq_nic->aq_vecs++) {
> +		aq_nic->aq_vec[aq_nic->aq_vecs] =
> +		    aq_vec_alloc(aq_nic, aq_nic->aq_vecs, aq_nic_cfg);
> +		if (unlikely(!aq_nic->aq_vec[aq_nic->aq_vecs])) {
> +			err = -ENOMEM;
> +			goto err_unlock;
> +		}
> +	}
> +	if (!netif_running(ndev))
> +		err = dev_open(ndev);

Will this not open the device regardless if it was open before or not?

> +err_unlock:
> +	spin_unlock(&aq_nic->aq_spinlock);
> +err_exit:
> +	return err;
> +}

^ permalink raw reply

* Re: [PATCH net-next] bpfilter: fix a build err
From: David Miller @ 2018-05-29 19:20 UTC (permalink / raw)
  To: yuehaibing; +Cc: ast, netdev, linux-kernel
In-Reply-To: <20180525101757.13756-1-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Fri, 25 May 2018 18:17:57 +0800

> gcc-7.3.0 report following err:
> 
>   HOSTCC  net/bpfilter/main.o
> In file included from net/bpfilter/main.c:9:0:
> ./include/uapi/linux/bpf.h:12:10: fatal error: linux/bpf_common.h: No such file or directory
>  #include <linux/bpf_common.h>
> 
> remove it by adding a include path.
> Fixes: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module")
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied, thanks.

^ permalink raw reply


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