* RE: [PATCH bpf 1/1] bpf: btf: Fix bitfield extraction for big endian
From: David Laight @ 2018-07-10 16:35 UTC (permalink / raw)
To: 'Martin KaFai Lau', Okash Khawaja
Cc: Daniel Borkmann, Alexei Starovoitov, Yonghong Song,
Jakub Kicinski, David S. Miller, netdev@vger.kernel.org,
kernel-team@fb.com, linux-kernel@vger.kernel.org
In-Reply-To: <20180709183236.r4b7gzmev5h4lcbw@kafai-mbp.dhcp.thefacebook.com>
From: Martin KaFai Lau
> Sent: 09 July 2018 19:33
> On Sun, Jul 08, 2018 at 05:22:03PM -0700, Okash Khawaja wrote:
> > When extracting bitfield from a number, btf_int_bits_seq_show() builds
> > a mask and accesses least significant byte of the number in a way
> > specific to little-endian. This patch fixes that by checking endianness
> > of the machine and then shifting left and right the unneeded bits.
> >
> > Thanks to Martin Lau for the help in navigating potential pitfalls when
> > dealing with endianess and for the final solution.
> >
> > Fixes: b00b8daec828 ("bpf: btf: Add pretty print capability for data with BTF type info")
> > Signed-off-by: Okash Khawaja <osk@fb.com>
> >
> > ---
> > kernel/bpf/btf.c | 32 +++++++++++++++-----------------
> > 1 file changed, 15 insertions(+), 17 deletions(-)
> >
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -162,6 +162,8 @@
> > #define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3)
> > #define BITS_ROUNDUP_BYTES(bits) \
> > (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits))
> > +const int one = 1;
> > +#define is_big_endian() ((*(char *)&one) == 0)
> >
> > #define BTF_INFO_MASK 0x0f00ffff
> > #define BTF_INT_MASK 0x0fffffff
> > @@ -991,16 +993,13 @@ static void btf_int_bits_seq_show(const
> > void *data, u8 bits_offset,
> > struct seq_file *m)
> > {
> > + u8 left_shift_bits, right_shift_bits;
> Nit.
> Although only max 64 bit int is allowed now (ensured by btf_int_check_meta),
> it is better to use u16 such that it will be consistent to BTF_INT_BITS.
Double-nit.
Use 'int' or 'unsigned int'.
Sub-word arithmetic will require extra instructions on almost everything
except x86.
David
^ permalink raw reply
* Re: [PATCH bpf] bpf: fix ldx in ld_abs rewrite for large offsets
From: Alexei Starovoitov @ 2018-07-10 15:19 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: mark.rutland, netdev
In-Reply-To: <1aaf318d0b40fc471a0d1705efd8c69d98fbf866.1531175919.git.daniel@iogearbox.net>
On Tue, Jul 10, 2018 at 12:43:22AM +0200, Daniel Borkmann wrote:
> Mark reported that syzkaller triggered a KASAN detected slab-out-of-bounds
> bug in ___bpf_prog_run() with a BPF_LD | BPF_ABS word load at offset 0x8001.
> After further investigation it became clear that the issue was the
> BPF_LDX_MEM() which takes offset as an argument whereas it cannot encode
> larger than S16_MAX offsets into it. For this synthetical case we need to
> move the full address into tmp register instead and do the LDX without
> immediate value.
>
> Fixes: e0cea7ce988c ("bpf: implement ld_abs/ld_ind in native bpf")
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Reported-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Applied, Thanks
^ permalink raw reply
* Re: [PATCH] ieee802154: add rx LQI from userspace
From: Clément Péron @ 2018-07-10 15:13 UTC (permalink / raw)
To: stefan
Cc: Romuald Cari, linux-wpan, Alexander Aring, Stefan Schmidt,
David S . Miller, netdev, linux-kernel, Clément Peron
In-Reply-To: <ac0ff951-627d-9694-741d-03f6bc4d6ddc@datenfreihafen.org>
Hi Alexander, Stefan,
Thanks for your feedbacks,
On Mon, 9 Jul 2018 at 10:49, Stefan Schmidt <stefan@datenfreihafen.org> wrote:
>
> Hello Clement.
>
> Finally coming to review the patch. Sorry for the delay.
>
> On 07.06.2018 16:08, Clément Péron wrote:
> > From: Romuald CARI <romuald.cari@devialet.com>
> >
> > The Link Quality Indication data exposed by drivers could not be accessed from
> > userspace. Since this data is per-datagram received, it makes sense to make it
> > available to userspace application through the ancillary data mechanism in
> > recvmsg rather than through ioctls. This can be activated using the socket
> > option WPAN_WANTLQI under SOL_IEEE802154 protocol.
>
> I can see that it makes the application life a lot easier to have data
> send out and LQI value synced up by using the socket approach instead of
> dealing with socket and ioctl's. I am good with this patch in general.
>
> So you have some public code that uses this approach? I would be
> interesting in the userspace part of yours. A demo would be fine. If the
> network handling part of your application is public anyway that would be
> even better. :-)
I'm sorry but the userspace code that use this isn't open.
I will check if I can share this part.
Just the idea is to compute an average LQI and when it reach a
threshold we allow the remote to control the device.
>
> As Alex mentiwe oned have some socket examples in the wpa-tools package
> to give people a head start when wanting to use the subsystem. Having
> such a simple example for the LQI feature would really give you bonus
> points. :-)
Indeed, add an example for this feature will be really interesting.
>
> https://github.com/linux-wpan/wpan-tools/tree/master/examples
>
> >
> > This LQI data is available in the ancillary data buffer under the SOL_IEEE802154
> > level as the type WPAN_LQI. The value is an unsigned byte indicating the link
> > quality with values ranging 0-255.
> >
> > Signed-off-by: Romuald Cari <romuald.cari@devialet.com>
> > Signed-off-by: Clément Peron <clement.peron@devialet.com>
> > ---
> > include/net/af_ieee802154.h | 1 +
> > net/ieee802154/socket.c | 17 +++++++++++++++++
> > 2 files changed, 18 insertions(+)
> >
> > diff --git a/include/net/af_ieee802154.h b/include/net/af_ieee802154.h
> > index a5563d27a3eb..8003a9f6eb43 100644
> > --- a/include/net/af_ieee802154.h
> > +++ b/include/net/af_ieee802154.h
> > @@ -56,6 +56,7 @@ struct sockaddr_ieee802154 {
> > #define WPAN_WANTACK 0
> > #define WPAN_SECURITY 1
> > #define WPAN_SECURITY_LEVEL 2
> > +#define WPAN_WANTLQI 3
> >
> > #define WPAN_SECURITY_DEFAULT 0
> > #define WPAN_SECURITY_OFF 1
> > diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
> > index a60658c85a9a..bc6b912603f1 100644
> > --- a/net/ieee802154/socket.c
> > +++ b/net/ieee802154/socket.c
> > @@ -25,6 +25,7 @@
> > #include <linux/termios.h> /* For TIOCOUTQ/INQ */
> > #include <linux/list.h>
> > #include <linux/slab.h>
> > +#include <linux/socket.h>
> > #include <net/datalink.h>
> > #include <net/psnap.h>
> > #include <net/sock.h>
> > @@ -452,6 +453,7 @@ struct dgram_sock {
> > unsigned int bound:1;
> > unsigned int connected:1;
> > unsigned int want_ack:1;
> > + unsigned int want_lqi:1;
> > unsigned int secen:1;
> > unsigned int secen_override:1;
> > unsigned int seclevel:3;
> > @@ -486,6 +488,7 @@ static int dgram_init(struct sock *sk)
> > struct dgram_sock *ro = dgram_sk(sk);
> >
> > ro->want_ack = 1;
> > + ro->want_lqi = 0;
> > return 0;
> > }
> >
> > @@ -713,6 +716,7 @@ static int dgram_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> > size_t copied = 0;
> > int err = -EOPNOTSUPP;
> > struct sk_buff *skb;
> > + struct dgram_sock *ro = dgram_sk(sk);
> > DECLARE_SOCKADDR(struct sockaddr_ieee802154 *, saddr, msg->msg_name);
> >
> > skb = skb_recv_datagram(sk, flags, noblock, &err);
> > @@ -744,6 +748,13 @@ static int dgram_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> > *addr_len = sizeof(*saddr);
> > }
> >
> > + if (ro->want_lqi) {
> > + err = put_cmsg(msg, SOL_IEEE802154, WPAN_WANTLQI,
> > + sizeof(uint8_t), &(mac_cb(skb)->lqi));
> > + if (err)
> > + goto done;
> > + }
> > +
>
> I am wondering a bit about the LQI you get back here. Maybe Alex can
> also shed some lights on it. The LQI value stored here is always from
> the last frame send (to any peer)? Or is it the last frame send to this
> specific peer?
>
> I have not put much thoughts into the LQI thing so far.Just thinking we
> need to be careful what we are providing to not let userspace make bad
> decisions (e.g. wrong routing changes due to link values given for a
> different peer connection).
>
> > if (flags & MSG_TRUNC)
> > copied = skb->len;
> > done:
> > @@ -847,6 +858,9 @@ static int dgram_getsockopt(struct sock *sk, int level, int optname,
> > case WPAN_WANTACK:
> > val = ro->want_ack;
> > break;
> > + case WPAN_WANTLQI:
> > + val = ro->want_lqi;
> > + break;
> > case WPAN_SECURITY:
> > if (!ro->secen_override)
> > val = WPAN_SECURITY_DEFAULT;
> > @@ -892,6 +906,9 @@ static int dgram_setsockopt(struct sock *sk, int level, int optname,
> > case WPAN_WANTACK:
> > ro->want_ack = !!val;
> > break;
> > + case WPAN_WANTLQI:
> > + ro->want_lqi = !!val;
> > + break;
> > case WPAN_SECURITY:
> > if (!ns_capable(net->user_ns, CAP_NET_ADMIN) &&
> > !ns_capable(net->user_ns, CAP_NET_RAW)) {
> >
>
> Review wise I am happy with the patch. I will give it a test.
Thanks, for the review,
Clement
>
> regards
> Stefan Schmidt
^ permalink raw reply
* Re: [PATCH bpf-next v3 2/3] bpf: btf: add btf print functionality
From: Okash Khawaja @ 2018-07-10 15:05 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Daniel Borkmann, Martin KaFai Lau, Alexei Starovoitov,
Yonghong Song, Quentin Monnet, David S. Miller, netdev,
kernel-team, linux-kernel
In-Reply-To: <20180709205612.5041acc9@cakuba.lan>
On Mon, Jul 09, 2018 at 08:56:12PM -0700, Jakub Kicinski wrote:
> On Sun, 8 Jul 2018 13:30:04 -0700, Okash Khawaja wrote:
> > This consumes functionality exported in the previous patch. It does the
> > main job of printing with BTF data. This is used in the following patch
> > to provide a more readable output of a map's dump. It relies on
> > json_writer to do json printing. Below is sample output where map keys
> > are ints and values are of type struct A:
> >
> > typedef int int_type;
> > enum E {
> > E0,
> > E1,
> > };
> >
> > struct B {
> > int x;
> > int y;
> > };
> >
> > struct A {
> > int m;
> > unsigned long long n;
> > char o;
> > int p[8];
> > int q[4][8];
> > enum E r;
> > void *s;
> > struct B t;
> > const int u;
> > int_type v;
> > unsigned int w1: 3;
> > unsigned int w2: 3;
> > };
> >
> > $ sudo bpftool map dump id 14
> > [{
> > "key": 0,
> > "value": {
> > "m": 1,
> > "n": 2,
> > "o": "c",
> > "p": [15,16,17,18,15,16,17,18
> > ],
> > "q": [[25,26,27,28,25,26,27,28
> > ],[35,36,37,38,35,36,37,38
> > ],[45,46,47,48,45,46,47,48
> > ],[55,56,57,58,55,56,57,58
> > ]
> > ],
> > "r": 1,
> > "s": 0x7ffd80531cf8,
> > "t": {
> > "x": 5,
> > "y": 10
> > },
> > "u": 100,
> > "v": 20,
> > "w1": 0x7,
> > "w2": 0x3
> > }
> > }
> > ]
> >
> > This patch uses json's {} and [] to imply struct/union and array. More
> > explicit information can be added later. For example, a command line
> > option can be introduced to print whether a key or value is struct
> > or union, name of a struct etc. This will however come at the expense
> > of duplicating info when, for example, printing an array of structs.
> > enums are printed as ints without their names.
> >
> > Signed-off-by: Okash Khawaja <osk@fb.com>
> > Acked-by: Martin KaFai Lau <kafai@fb.com>
> >
> > ---
> > tools/bpf/bpftool/btf_dumper.c | 253 +++++++++++++++++++++++++++++++++++++++++
> > tools/bpf/bpftool/main.h | 15 ++
> > 2 files changed, 268 insertions(+)
> >
> > --- /dev/null
> > +++ b/tools/bpf/bpftool/btf_dumper.c
> > @@ -0,0 +1,253 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2018 Facebook */
> > +
> > +#include <linux/btf.h>
> > +#include <linux/err.h>
> > +#include <stdio.h> /* for (FILE *) used by json_writer */
> > +#include <linux/bitops.h>
> > +#include <string.h>
> > +#include <ctype.h>
>
> fwiw: the preferred ordering would have been:
>
> #include <ctype.h>
> #include <stdio.h> /* for (FILE *) used by json_writer */
> #include <string.h>
> #include <linux/bitops.h>
> #include <linux/btf.h>
> #include <linux/err.h>
>
> > +#include "btf.h"
> > +#include "json_writer.h"
> > +#include "main.h"
> > +
> > +#define BITS_PER_BYTE_MASK (BITS_PER_BYTE - 1)
> > +#define BITS_PER_BYTE_MASKED(bits) ((bits) & BITS_PER_BYTE_MASK)
> > +#define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3)
> > +#define BITS_ROUNDUP_BYTES(bits) \
> > + (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits))
> > +const int one = 1;
> > +#define is_big_endian() ((*(char *)&one) == 0)
>
> Could we try to do this at compilation time? Without the variable? :(
>
> #include <asm/byteorder.h>
>
> #if defined(__BYTE_ORDER) ? __BYTE_ORDER == __BIG_ENDIAN : defined(__BIG_ENDIAN)
> return true;
> #else
> return false;
> #endif
>
> We could also just include endian.h, but since it's a non-standard
> extension perhaps using kernel header is a safer bet.
>
> > +static int btf_dumper_do_type(const struct btf_dumper *d, __u32 type_id,
> > + __u8 bit_offset, const void *data);
> > +
> > +static void btf_dumper_ptr(const void *data, json_writer_t *jw,
> > + bool is_plain_text)
> > +{
> > + if (is_plain_text)
> > + jsonw_printf(jw, "%p", *((unsigned long *)data));
> > + else
> > + jsonw_printf(jw, "%u", *((unsigned long *)data));
>
> nit: I think you missed these parenthesis
>
> > +}
> > +
>
> > +static void btf_dumper_int_bits(__u32 int_type, __u8 bit_offset,
> > + const void *data, json_writer_t *jw,
> > + bool is_plain_text)
> > +{
> > + int left_shift_bits, right_shift_bits;
> > + int nr_bits = BTF_INT_BITS(int_type);
> > + int total_bits_offset;
> > + int bytes_to_copy;
> > + int bits_to_copy;
> > + __u64 print_num;
> > +
> > + total_bits_offset = bit_offset + BTF_INT_OFFSET(int_type);
> > + data += BITS_ROUNDDOWN_BYTES(total_bits_offset);
> > + bit_offset = BITS_PER_BYTE_MASKED(total_bits_offset);
> > + bits_to_copy = bit_offset + nr_bits;
> > + bytes_to_copy = BITS_ROUNDUP_BYTES(bits_to_copy);
> > +
> > + print_num = 0;
> > + memcpy(&print_num, data, bytes_to_copy);
> > + if (is_big_endian()) {
> > + left_shift_bits = bit_offset;
> > + right_shift_bits = 64 - nr_bits;
> > + } else {
> > + left_shift_bits = 64 - bits_to_copy;
> > + right_shift_bits = 64 - nr_bits;
> > + }
>
> Or you can just put the #if here, since it's the only use.
>
> > + print_num <<= left_shift_bits;
> > + print_num >>= right_shift_bits;
> > + if (is_plain_text)
> > + jsonw_printf(jw, "0x%llx", print_num);
> > + else
> > + jsonw_printf(jw, "%llu", print_num);
> > +}
> > +
> > +static int btf_dumper_int(const struct btf_type *t, __u8 bit_offset,
> > + const void *data, json_writer_t *jw,
> > + bool is_plain_text)
> > +{
> > + __u32 *int_type;
> > + __u32 nr_bits;
> > +
> > + int_type = (__u32 *)(t + 1);
> > + nr_bits = BTF_INT_BITS(*int_type);
> > + /* if this is bit field */
> > + if (bit_offset || BTF_INT_OFFSET(*int_type) ||
> > + BITS_PER_BYTE_MASKED(nr_bits)) {
> > + btf_dumper_int_bits(*int_type, bit_offset, data, jw,
> > + is_plain_text);
> > + return 0;
> > + }
> > +
> > + switch (BTF_INT_ENCODING(*int_type)) {
> > + case 0:
> > + if (BTF_INT_BITS(*int_type) == 64)
> > + jsonw_printf(jw, "%lu", *((__u64 *)data));
>
> nit: more parenthesis here
>
> > + else if (BTF_INT_BITS(*int_type) == 32)
> > + jsonw_printf(jw, "%u", *((__u32 *)data));
> > + else if (BTF_INT_BITS(*int_type) == 16)
> > + jsonw_printf(jw, "%hu", *((__u16 *)data));
> > + else if (BTF_INT_BITS(*int_type) == 8)
> > + jsonw_printf(jw, "%hhu", *((__u8 *)data));
> > + else
> > + btf_dumper_int_bits(*int_type, bit_offset, data, jw,
> > + is_plain_text);
> > + break;
> > + case BTF_INT_SIGNED:
> > + if (BTF_INT_BITS(*int_type) == 64)
> > + jsonw_printf(jw, "%ld", *((long long *)data));
> > + else if (BTF_INT_BITS(*int_type) == 32)
> > + jsonw_printf(jw, "%d", *((int *)data));
> > + else if (BTF_INT_BITS(*int_type) == 16)
> > + jsonw_printf(jw, "%hd", *((short *)data));
> > + else if (BTF_INT_BITS(*int_type) == 8)
> > + jsonw_printf(jw, "%hhd", *((char *)data));
> > + else
> > + btf_dumper_int_bits(*int_type, bit_offset, data, jw,
> > + is_plain_text);
> > + break;
> > + case BTF_INT_CHAR:
> > + if (*((char *)data) == '\0')
>
> nit: here too, etc..
>
> > + jsonw_null(jw);
>
> I don't think the null is good. I thought I mentioned that?
yes! i intended but missed doing that amongst the several other changes :)
> Look for
> example at Python:
>
> >>> import json
> >>> thing = json.loads('{"a": [97, 98, 99, 100]}')
> >>> bytearray(thing["str"]).decode('utf-8')
> 'abcd'
> >>> "".join(map(chr, thing["str"]))
> 'abcd'
> >>> thing = json.loads('{"str": [97, 98, 99, 100, null]}')
> >>> bytearray(thing["str"]).decode('utf-8')
> Traceback (most recent call last):
> File "<stdin>", line 1, in <module>
> TypeError: an integer is required
> >>> "".join(map(chr, thing["str"]))
> Traceback (most recent call last):
> File "<stdin>", line 1, in <module>
> TypeError: an integer is required (got type NoneType)
>
> If you start putting nulls into the array the conversion to a string
> will become more difficult, won't it? Do you have a use case where
> this helps? Maybe my Python-foo is not strong enough?
>
> > + else if (isprint(*((char *)data)))
> > + jsonw_printf(jw, "\"%c\"", *((char *)data));
> > + else
> > + if (is_plain_text)
> > + jsonw_printf(jw, "0x%hhx", *((char *)data));
> > + else
> > + jsonw_printf(jw, "\"\\u00%02hhx\"",
> > + *((char *)data));
> > + break;
> > + case BTF_INT_BOOL:
> > + jsonw_bool(jw, *((int *)data));
> > + break;
> > + default:
> > + /* shouldn't happen */
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int btf_dumper_struct(const struct btf_dumper *d, __u32 type_id,
> > + const void *data)
> > +{
> > + const struct btf_type *t;
> > + struct btf_member *m;
> > + int ret = 0;
> > + int i, vlen;
> > +
> > + t = btf__type_by_id(d->btf, type_id);
> > + if (!t)
> > + return -EINVAL;
> > +
> > + vlen = BTF_INFO_VLEN(t->info);
> > + jsonw_start_object(d->jw);
> > + m = (struct btf_member *)(t + 1);
> > +
> > + for (i = 0; i < vlen; i++) {
> > + const void *data_off = data +
> > + BITS_ROUNDDOWN_BYTES(m[i].offset);
>
> nit: empty line between variable declaration and code, perhaps also
> don't init inline since it doesn't fit that way?
>
> > + jsonw_name(d->jw, btf__name_by_offset(d->btf, m[i].name_off));
> > + ret = btf_dumper_do_type(d, m[i].type,
> > + BITS_PER_BYTE_MASKED(m[i].offset),
> > + data_off);
> > + if (ret)
> > + break;
> > + }
> > +
> > + jsonw_end_object(d->jw);
> > +
> > + return ret;
> > +}
>
> Thanks for all the changes you've made so far!
^ permalink raw reply
* [PATCH bpf-next] bpf: better availability probing for seg6 helpers
From: Mathieu Xhonneux @ 2018-07-10 16:54 UTC (permalink / raw)
To: netdev; +Cc: daniel, alexei.starovoitov
bpf_lwt_seg6_* helpers require CONFIG_IPV6_SEG6_BPF, and currently
return -EOPNOTSUPP to indicate unavailability. This patch forces the
BPF verifier to reject programs using these helpers when
!CONFIG_IPV6_SEG6_BPF, allowing users to more easily probe if they are
available or not.
Signed-off-by: Mathieu Xhonneux <m.xhonneux@gmail.com>
---
net/core/filter.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index b9ec916f4e3a..c910a95e9a7c 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4439,10 +4439,10 @@ static const struct bpf_func_proto bpf_lwt_push_encap_proto = {
.arg4_type = ARG_CONST_SIZE
};
+#if IS_ENABLED(CONFIG_IPV6_SEG6_BPF)
BPF_CALL_4(bpf_lwt_seg6_store_bytes, struct sk_buff *, skb, u32, offset,
const void *, from, u32, len)
{
-#if IS_ENABLED(CONFIG_IPV6_SEG6_BPF)
struct seg6_bpf_srh_state *srh_state =
this_cpu_ptr(&seg6_bpf_srh_states);
void *srh_tlvs, *srh_end, *ptr;
@@ -4468,9 +4468,6 @@ BPF_CALL_4(bpf_lwt_seg6_store_bytes, struct sk_buff *, skb, u32, offset,
memcpy(skb->data + offset, from, len);
return 0;
-#else /* CONFIG_IPV6_SEG6_BPF */
- return -EOPNOTSUPP;
-#endif
}
static const struct bpf_func_proto bpf_lwt_seg6_store_bytes_proto = {
@@ -4486,7 +4483,6 @@ static const struct bpf_func_proto bpf_lwt_seg6_store_bytes_proto = {
BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
u32, action, void *, param, u32, param_len)
{
-#if IS_ENABLED(CONFIG_IPV6_SEG6_BPF)
struct seg6_bpf_srh_state *srh_state =
this_cpu_ptr(&seg6_bpf_srh_states);
struct ipv6_sr_hdr *srh;
@@ -4534,9 +4530,6 @@ BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
default:
return -EINVAL;
}
-#else /* CONFIG_IPV6_SEG6_BPF */
- return -EOPNOTSUPP;
-#endif
}
static const struct bpf_func_proto bpf_lwt_seg6_action_proto = {
@@ -4552,7 +4545,6 @@ static const struct bpf_func_proto bpf_lwt_seg6_action_proto = {
BPF_CALL_3(bpf_lwt_seg6_adjust_srh, struct sk_buff *, skb, u32, offset,
s32, len)
{
-#if IS_ENABLED(CONFIG_IPV6_SEG6_BPF)
struct seg6_bpf_srh_state *srh_state =
this_cpu_ptr(&seg6_bpf_srh_states);
void *srh_end, *srh_tlvs, *ptr;
@@ -4596,9 +4588,6 @@ BPF_CALL_3(bpf_lwt_seg6_adjust_srh, struct sk_buff *, skb, u32, offset,
srh_state->hdrlen += len;
srh_state->valid = 0;
return 0;
-#else /* CONFIG_IPV6_SEG6_BPF */
- return -EOPNOTSUPP;
-#endif
}
static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
@@ -4609,6 +4598,7 @@ static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
.arg2_type = ARG_ANYTHING,
.arg3_type = ARG_ANYTHING,
};
+#endif /* CONFIG_IPV6_SEG6_BPF */
bool bpf_helper_changes_pkt_data(void *func)
{
@@ -4627,10 +4617,12 @@ bool bpf_helper_changes_pkt_data(void *func)
func == bpf_xdp_adjust_meta ||
func == bpf_msg_pull_data ||
func == bpf_xdp_adjust_tail ||
- func == bpf_lwt_push_encap ||
+#if IS_ENABLED(CONFIG_IPV6_SEG6_BPF)
func == bpf_lwt_seg6_store_bytes ||
func == bpf_lwt_seg6_adjust_srh ||
- func == bpf_lwt_seg6_action
+ func == bpf_lwt_seg6_action ||
+#endif
+ func == bpf_lwt_push_encap
)
return true;
@@ -4967,12 +4959,14 @@ static const struct bpf_func_proto *
lwt_seg6local_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
switch (func_id) {
+#if IS_ENABLED(CONFIG_IPV6_SEG6_BPF)
case BPF_FUNC_lwt_seg6_store_bytes:
return &bpf_lwt_seg6_store_bytes_proto;
case BPF_FUNC_lwt_seg6_action:
return &bpf_lwt_seg6_action_proto;
case BPF_FUNC_lwt_seg6_adjust_srh:
return &bpf_lwt_seg6_adjust_srh_proto;
+#endif
default:
return lwt_out_func_proto(func_id, prog);
}
--
2.16.1
^ permalink raw reply related
* [PATCHv2 net 2/2] ipv6/mcast: init as INCLUDE when join SSM INCLUDE group
From: Hangbin Liu @ 2018-07-10 14:41 UTC (permalink / raw)
To: netdev
Cc: David Miller, Stefano Brivio, Paolo Abeni, Daniel Borkmann,
WANG Cong, YOSHIFUJI Hideaki, Flavio Leitner, Hangbin Liu
In-Reply-To: <1531233687-28744-1-git-send-email-liuhangbin@gmail.com>
This an IPv6 version patch of "ipv4/igmp: init group mode as INCLUDE when
join source group". From RFC3810, part 6.1:
If no per-interface state existed for that
multicast address before the change (i.e., the change consisted of
creating a new per-interface record), or if no state exists after the
change (i.e., the change consisted of deleting a per-interface
record), then the "non-existent" state is considered to have an
INCLUDE filter mode and an empty source list.
Which means a new multicast group should start with state IN(). Currently,
for MLDv2 SSM JOIN_SOURCE_GROUP mode, we first call ipv6_sock_mc_join(),
then ip6_mc_source(), which will trigger a TO_IN() message instead of
ALLOW().
The issue was exposed by commit a052517a8ff65 ("net/multicast: should not
send source list records when have filter mode change"). Before this change,
we sent both ALLOW(A) and TO_IN(A). Now, we only send TO_IN(A).
Fix it by adding a new parameter to init group mode. Also add some wrapper
functions to avoid changing too much code.
v1 -> v2:
In the first version I only cleared the group change record. But this is not
enough. Because when a new group join, it will init as EXCLUDE and trigger
a filter mode change in ip/ip6_mc_add_src(), which will clear all source
addresses sf_crcount. This will prevent early joined address sending state
change records if multi source addressed joined at the same time.
In v2 patch, I fixed it by directly initializing the mode to INCLUDE for SSM
JOIN_SOURCE_GROUP. I also split the original patch into two separated patches
for IPv4 and IPv6.
There is also a difference between v4 and v6 version. For IPv6, when the
interface goes down and up, we will send correct state change record with
unspecified IPv6 address (::) with function ipv6_mc_up(). But after DAD is
completed, we resend the change record TO_IN() in mld_send_initial_cr().
Fix it by sending ALLOW() for INCLUDE mode in mld_send_initial_cr().
Fixes: a052517a8ff65 ("net/multicast: should not send source list records when have filter mode change")
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
include/net/ipv6.h | 2 ++
net/ipv6/ipv6_sockglue.c | 5 ++--
net/ipv6/mcast.c | 64 ++++++++++++++++++++++++++++++++++--------------
3 files changed, 50 insertions(+), 21 deletions(-)
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 836f31a..c2283ca 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -1104,6 +1104,8 @@ void ipv6_sysctl_unregister(void);
int ipv6_sock_mc_join(struct sock *sk, int ifindex,
const struct in6_addr *addr);
+int ipv6_sock_mc_join_ssm(struct sock *sk, int ifindex,
+ const struct in6_addr *addr, unsigned int mode);
int ipv6_sock_mc_drop(struct sock *sk, int ifindex,
const struct in6_addr *addr);
#endif /* _NET_IPV6_H */
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 4d780c7..343c561 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -718,8 +718,9 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
struct sockaddr_in6 *psin6;
psin6 = (struct sockaddr_in6 *)&greqs.gsr_group;
- retv = ipv6_sock_mc_join(sk, greqs.gsr_interface,
- &psin6->sin6_addr);
+ retv = ipv6_sock_mc_join_ssm(sk, greqs.gsr_interface,
+ &psin6->sin6_addr,
+ MCAST_INCLUDE);
/* prior join w/ different source is ok */
if (retv && retv != -EADDRINUSE)
break;
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 793159d..cba3519 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -95,6 +95,8 @@ static int ip6_mc_add_src(struct inet6_dev *idev, const struct in6_addr *pmca,
int delta);
static int ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *iml,
struct inet6_dev *idev);
+static int __ipv6_dev_mc_inc(struct net_device *dev,
+ const struct in6_addr *addr, unsigned int mode);
#define MLD_QRV_DEFAULT 2
/* RFC3810, 9.2. Query Interval */
@@ -132,7 +134,8 @@ static int unsolicited_report_interval(struct inet6_dev *idev)
return iv > 0 ? iv : 1;
}
-int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
+static int __ipv6_sock_mc_join(struct sock *sk, int ifindex,
+ const struct in6_addr *addr, unsigned int mode)
{
struct net_device *dev = NULL;
struct ipv6_mc_socklist *mc_lst;
@@ -179,7 +182,7 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
}
mc_lst->ifindex = dev->ifindex;
- mc_lst->sfmode = MCAST_EXCLUDE;
+ mc_lst->sfmode = mode;
rwlock_init(&mc_lst->sflock);
mc_lst->sflist = NULL;
@@ -187,7 +190,7 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
* now add/increase the group membership on the device
*/
- err = ipv6_dev_mc_inc(dev, addr);
+ err = __ipv6_dev_mc_inc(dev, addr, mode);
if (err) {
sock_kfree_s(sk, mc_lst, sizeof(*mc_lst));
@@ -199,8 +202,19 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
return 0;
}
+
+int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
+{
+ return __ipv6_sock_mc_join(sk, ifindex, addr, MCAST_EXCLUDE);
+}
EXPORT_SYMBOL(ipv6_sock_mc_join);
+int ipv6_sock_mc_join_ssm(struct sock *sk, int ifindex,
+ const struct in6_addr *addr, unsigned int mode)
+{
+ return __ipv6_sock_mc_join(sk, ifindex, addr, mode);
+}
+
/*
* socket leave on multicast group
*/
@@ -646,7 +660,7 @@ bool inet6_mc_check(struct sock *sk, const struct in6_addr *mc_addr,
return rv;
}
-static void igmp6_group_added(struct ifmcaddr6 *mc)
+static void igmp6_group_added(struct ifmcaddr6 *mc, unsigned int mode)
{
struct net_device *dev = mc->idev->dev;
char buf[MAX_ADDR_LEN];
@@ -672,7 +686,13 @@ static void igmp6_group_added(struct ifmcaddr6 *mc)
}
/* else v2 */
- mc->mca_crcount = mc->idev->mc_qrv;
+ /* Based on RFC3810 6.1, for newly added INCLUDE SSM, we
+ * should not send filter-mode change record as the mode
+ * should be from IN() to IN(A).
+ */
+ if (mode == MCAST_EXCLUDE)
+ mc->mca_crcount = mc->idev->mc_qrv;
+
mld_ifc_event(mc->idev);
}
@@ -770,13 +790,14 @@ static void mld_del_delrec(struct inet6_dev *idev, struct ifmcaddr6 *im)
spin_lock_bh(&im->mca_lock);
if (pmc) {
im->idev = pmc->idev;
- im->mca_crcount = idev->mc_qrv;
im->mca_sfmode = pmc->mca_sfmode;
if (pmc->mca_sfmode == MCAST_INCLUDE) {
im->mca_tomb = pmc->mca_tomb;
im->mca_sources = pmc->mca_sources;
for (psf = im->mca_sources; psf; psf = psf->sf_next)
- psf->sf_crcount = im->mca_crcount;
+ psf->sf_crcount = idev->mc_qrv;
+ } else {
+ im->mca_crcount = idev->mc_qrv;
}
in6_dev_put(pmc->idev);
kfree(pmc);
@@ -831,7 +852,8 @@ static void ma_put(struct ifmcaddr6 *mc)
}
static struct ifmcaddr6 *mca_alloc(struct inet6_dev *idev,
- const struct in6_addr *addr)
+ const struct in6_addr *addr,
+ unsigned int mode)
{
struct ifmcaddr6 *mc;
@@ -849,9 +871,8 @@ static struct ifmcaddr6 *mca_alloc(struct inet6_dev *idev,
refcount_set(&mc->mca_refcnt, 1);
spin_lock_init(&mc->mca_lock);
- /* initial mode is (EX, empty) */
- mc->mca_sfmode = MCAST_EXCLUDE;
- mc->mca_sfcount[MCAST_EXCLUDE] = 1;
+ mc->mca_sfmode = mode;
+ mc->mca_sfcount[mode] = 1;
if (ipv6_addr_is_ll_all_nodes(&mc->mca_addr) ||
IPV6_ADDR_MC_SCOPE(&mc->mca_addr) < IPV6_ADDR_SCOPE_LINKLOCAL)
@@ -863,7 +884,8 @@ static struct ifmcaddr6 *mca_alloc(struct inet6_dev *idev,
/*
* device multicast group inc (add if not found)
*/
-int ipv6_dev_mc_inc(struct net_device *dev, const struct in6_addr *addr)
+static int __ipv6_dev_mc_inc(struct net_device *dev,
+ const struct in6_addr *addr, unsigned int mode)
{
struct ifmcaddr6 *mc;
struct inet6_dev *idev;
@@ -887,14 +909,13 @@ int ipv6_dev_mc_inc(struct net_device *dev, const struct in6_addr *addr)
if (ipv6_addr_equal(&mc->mca_addr, addr)) {
mc->mca_users++;
write_unlock_bh(&idev->lock);
- ip6_mc_add_src(idev, &mc->mca_addr, MCAST_EXCLUDE, 0,
- NULL, 0);
+ ip6_mc_add_src(idev, &mc->mca_addr, mode, 0, NULL, 0);
in6_dev_put(idev);
return 0;
}
}
- mc = mca_alloc(idev, addr);
+ mc = mca_alloc(idev, addr, mode);
if (!mc) {
write_unlock_bh(&idev->lock);
in6_dev_put(idev);
@@ -911,11 +932,16 @@ int ipv6_dev_mc_inc(struct net_device *dev, const struct in6_addr *addr)
write_unlock_bh(&idev->lock);
mld_del_delrec(idev, mc);
- igmp6_group_added(mc);
+ igmp6_group_added(mc, mode);
ma_put(mc);
return 0;
}
+int ipv6_dev_mc_inc(struct net_device *dev, const struct in6_addr *addr)
+{
+ return __ipv6_dev_mc_inc(dev, addr, MCAST_EXCLUDE);
+}
+
/*
* device multicast group del
*/
@@ -1751,7 +1777,7 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
psf_next = psf->sf_next;
- if (!is_in(pmc, psf, type, gdeleted, sdeleted)) {
+ if (!is_in(pmc, psf, type, gdeleted, sdeleted) && !crsend) {
psf_prev = psf;
continue;
}
@@ -2066,7 +2092,7 @@ static void mld_send_initial_cr(struct inet6_dev *idev)
if (pmc->mca_sfcount[MCAST_EXCLUDE])
type = MLD2_CHANGE_TO_EXCLUDE;
else
- type = MLD2_CHANGE_TO_INCLUDE;
+ type = MLD2_ALLOW_NEW_SOURCES;
skb = add_grec(skb, pmc, type, 0, 0, 1);
spin_unlock_bh(&pmc->mca_lock);
}
@@ -2543,7 +2569,7 @@ void ipv6_mc_up(struct inet6_dev *idev)
ipv6_mc_reset(idev);
for (i = idev->mc_list; i; i = i->next) {
mld_del_delrec(idev, i);
- igmp6_group_added(i);
+ igmp6_group_added(i, i->mca_sfmode);
}
read_unlock_bh(&idev->lock);
}
--
2.5.5
^ permalink raw reply related
* [PATCHv2 net 1/2] ipv4/igmp: init group mode as INCLUDE when join source group
From: Hangbin Liu @ 2018-07-10 14:41 UTC (permalink / raw)
To: netdev
Cc: David Miller, Stefano Brivio, Paolo Abeni, Daniel Borkmann,
WANG Cong, YOSHIFUJI Hideaki, Flavio Leitner, Hangbin Liu
In-Reply-To: <1531233687-28744-1-git-send-email-liuhangbin@gmail.com>
Based on RFC3376 5.1
If no interface
state existed for that multicast address before the change (i.e., the
change consisted of creating a new per-interface record), or if no
state exists after the change (i.e., the change consisted of deleting
a per-interface record), then the "non-existent" state is considered
to have a filter mode of INCLUDE and an empty source list.
Which means a new multicast group should start with state IN().
Function ip_mc_join_group() works correctly for IGMP ASM(Any-Source Multicast)
mode. It adds a group with state EX() and inits crcount to mc_qrv,
so the kernel will send a TO_EX() report message after adding group.
But for IGMPv3 SSM(Source-specific multicast) JOIN_SOURCE_GROUP mode, we
split the group joining into two steps. First we join the group like ASM,
i.e. via ip_mc_join_group(). So the state changes from IN() to EX().
Then we add the source-specific address with INCLUDE mode. So the state
changes from EX() to IN(A).
Before the first step sends a group change record, we finished the second
step. So we will only send the second change record. i.e. TO_IN(A).
Regarding the RFC stands, we should actually send an ALLOW(A) message for
SSM JOIN_SOURCE_GROUP as the state should mimic the 'IN() to IN(A)'
transition.
The issue was exposed by commit a052517a8ff65 ("net/multicast: should not
send source list records when have filter mode change"). Before this change,
we used to send both ALLOW(A) and TO_IN(A). After this change we only send
TO_IN(A).
Fix it by adding a new parameter to init group mode. Also add new wrapper
functions so we don't need to change too much code.
v1 -> v2:
In my first version I only cleared the group change record. But this is not
enough. Because when a new group join, it will init as EXCLUDE and trigger
an filter mode change in ip/ip6_mc_add_src(), which will clear all source
addresses' sf_crcount. This will prevent early joined address sending state
change records if multi source addressed joined at the same time.
In v2 patch, I fixed it by directly initializing the mode to INCLUDE for SSM
JOIN_SOURCE_GROUP. I also split the original patch into two separated patches
for IPv4 and IPv6.
Fixes: a052517a8ff65 ("net/multicast: should not send source list records when have filter mode change")
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
include/linux/igmp.h | 2 ++
net/ipv4/igmp.c | 58 ++++++++++++++++++++++++++++++++++++--------------
net/ipv4/ip_sockglue.c | 4 ++--
3 files changed, 46 insertions(+), 18 deletions(-)
diff --git a/include/linux/igmp.h b/include/linux/igmp.h
index f823185..119f539 100644
--- a/include/linux/igmp.h
+++ b/include/linux/igmp.h
@@ -109,6 +109,8 @@ struct ip_mc_list {
extern int ip_check_mc_rcu(struct in_device *dev, __be32 mc_addr, __be32 src_addr, u8 proto);
extern int igmp_rcv(struct sk_buff *);
extern int ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr);
+extern int ip_mc_join_group_ssm(struct sock *sk, struct ip_mreqn *imr,
+ unsigned int mode);
extern int ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr);
extern void ip_mc_drop_socket(struct sock *sk);
extern int ip_mc_source(int add, int omode, struct sock *sk,
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index b26a81a..ef72499 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -1200,13 +1200,14 @@ static void igmpv3_del_delrec(struct in_device *in_dev, struct ip_mc_list *im)
spin_lock_bh(&im->lock);
if (pmc) {
im->interface = pmc->interface;
- im->crcount = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
im->sfmode = pmc->sfmode;
if (pmc->sfmode == MCAST_INCLUDE) {
im->tomb = pmc->tomb;
im->sources = pmc->sources;
for (psf = im->sources; psf; psf = psf->sf_next)
- psf->sf_crcount = im->crcount;
+ psf->sf_crcount = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
+ } else {
+ im->crcount = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
}
in_dev_put(pmc->interface);
kfree(pmc);
@@ -1288,7 +1289,7 @@ static void igmp_group_dropped(struct ip_mc_list *im)
#endif
}
-static void igmp_group_added(struct ip_mc_list *im)
+static void igmp_group_added(struct ip_mc_list *im, unsigned int mode)
{
struct in_device *in_dev = im->interface;
#ifdef CONFIG_IP_MULTICAST
@@ -1316,7 +1317,13 @@ static void igmp_group_added(struct ip_mc_list *im)
}
/* else, v3 */
- im->crcount = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
+ /* Based on RFC3376 5.1, for newly added INCLUDE SSM, we should
+ * not send filter-mode change record as the mode should be from
+ * IN() to IN(A).
+ */
+ if (mode == MCAST_EXCLUDE)
+ im->crcount = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
+
igmp_ifc_event(in_dev);
#endif
}
@@ -1381,8 +1388,7 @@ static void ip_mc_hash_remove(struct in_device *in_dev,
/*
* A socket has joined a multicast group on device dev.
*/
-
-void ip_mc_inc_group(struct in_device *in_dev, __be32 addr)
+void __ip_mc_inc_group(struct in_device *in_dev, __be32 addr, unsigned int mode)
{
struct ip_mc_list *im;
#ifdef CONFIG_IP_MULTICAST
@@ -1394,7 +1400,7 @@ void ip_mc_inc_group(struct in_device *in_dev, __be32 addr)
for_each_pmc_rtnl(in_dev, im) {
if (im->multiaddr == addr) {
im->users++;
- ip_mc_add_src(in_dev, &addr, MCAST_EXCLUDE, 0, NULL, 0);
+ ip_mc_add_src(in_dev, &addr, mode, 0, NULL, 0);
goto out;
}
}
@@ -1408,8 +1414,8 @@ void ip_mc_inc_group(struct in_device *in_dev, __be32 addr)
in_dev_hold(in_dev);
im->multiaddr = addr;
/* initial mode is (EX, empty) */
- im->sfmode = MCAST_EXCLUDE;
- im->sfcount[MCAST_EXCLUDE] = 1;
+ im->sfmode = mode;
+ im->sfcount[mode] = 1;
refcount_set(&im->refcnt, 1);
spin_lock_init(&im->lock);
#ifdef CONFIG_IP_MULTICAST
@@ -1426,12 +1432,17 @@ void ip_mc_inc_group(struct in_device *in_dev, __be32 addr)
#ifdef CONFIG_IP_MULTICAST
igmpv3_del_delrec(in_dev, im);
#endif
- igmp_group_added(im);
+ igmp_group_added(im, mode);
if (!in_dev->dead)
ip_rt_multicast_event(in_dev);
out:
return;
}
+
+void ip_mc_inc_group(struct in_device *in_dev, __be32 addr)
+{
+ __ip_mc_inc_group(in_dev, addr, MCAST_EXCLUDE);
+}
EXPORT_SYMBOL(ip_mc_inc_group);
static int ip_mc_check_iphdr(struct sk_buff *skb)
@@ -1688,7 +1699,7 @@ void ip_mc_remap(struct in_device *in_dev)
#ifdef CONFIG_IP_MULTICAST
igmpv3_del_delrec(in_dev, pmc);
#endif
- igmp_group_added(pmc);
+ igmp_group_added(pmc, pmc->sfmode);
}
}
@@ -1751,7 +1762,7 @@ void ip_mc_up(struct in_device *in_dev)
#ifdef CONFIG_IP_MULTICAST
igmpv3_del_delrec(in_dev, pmc);
#endif
- igmp_group_added(pmc);
+ igmp_group_added(pmc, pmc->sfmode);
}
}
@@ -2130,8 +2141,8 @@ static void ip_mc_clear_src(struct ip_mc_list *pmc)
/* Join a multicast group
*/
-
-int ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr)
+static int __ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr,
+ unsigned int mode)
{
__be32 addr = imr->imr_multiaddr.s_addr;
struct ip_mc_socklist *iml, *i;
@@ -2172,15 +2183,30 @@ int ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr)
memcpy(&iml->multi, imr, sizeof(*imr));
iml->next_rcu = inet->mc_list;
iml->sflist = NULL;
- iml->sfmode = MCAST_EXCLUDE;
+ iml->sfmode = mode;
rcu_assign_pointer(inet->mc_list, iml);
- ip_mc_inc_group(in_dev, addr);
+ __ip_mc_inc_group(in_dev, addr, mode);
err = 0;
done:
return err;
}
+
+/* Join ASM (Any-Source Multicast) group
+ */
+int ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr)
+{
+ return __ip_mc_join_group(sk, imr, MCAST_EXCLUDE);
+}
EXPORT_SYMBOL(ip_mc_join_group);
+/* Join SSM (Source-Specific Multicast) group
+ */
+int ip_mc_join_group_ssm(struct sock *sk, struct ip_mreqn *imr,
+ unsigned int mode)
+{
+ return __ip_mc_join_group(sk, imr, mode);
+}
+
static int ip_mc_leave_src(struct sock *sk, struct ip_mc_socklist *iml,
struct in_device *in_dev)
{
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 57bbb06..5f28607 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -982,7 +982,7 @@ static int do_ip_setsockopt(struct sock *sk, int level,
mreq.imr_multiaddr.s_addr = mreqs.imr_multiaddr;
mreq.imr_address.s_addr = mreqs.imr_interface;
mreq.imr_ifindex = 0;
- err = ip_mc_join_group(sk, &mreq);
+ err = ip_mc_join_group_ssm(sk, &mreq, MCAST_INCLUDE);
if (err && err != -EADDRINUSE)
break;
omode = MCAST_INCLUDE;
@@ -1059,7 +1059,7 @@ static int do_ip_setsockopt(struct sock *sk, int level,
mreq.imr_multiaddr = psin->sin_addr;
mreq.imr_address.s_addr = 0;
mreq.imr_ifindex = greqs.gsr_interface;
- err = ip_mc_join_group(sk, &mreq);
+ err = ip_mc_join_group_ssm(sk, &mreq, MCAST_INCLUDE);
if (err && err != -EADDRINUSE)
break;
greqs.gsr_interface = mreq.imr_ifindex;
--
2.5.5
^ permalink raw reply related
* [PATCHv2 net 0/2] multicast: init as INCLUDE when join SSM INCLUDE group
From: Hangbin Liu @ 2018-07-10 14:41 UTC (permalink / raw)
To: netdev
Cc: David Miller, Stefano Brivio, Paolo Abeni, Daniel Borkmann,
WANG Cong, YOSHIFUJI Hideaki, Flavio Leitner, Hangbin Liu
In-Reply-To: <1528871551-17879-1-git-send-email-liuhangbin@gmail.com>
Based on RFC3376 5.1 and RFC3810 6.1, we should init as INCLUDE when join SSM
INCLUDE group. In my first version I only clear the group change record. But
this is not enough as when a new group join, it will init as EXCLUDE and
trigger an filter mode change in ip/ip6_mc_add_src(), which will clear all
source addresses' sf_crcount. This will prevent early joined address sending
state change records if multi source addresses joined at the same time.
In this v2 patchset, I fixed it by directly initializing the mode to INCLUDE
for SSM JOIN_SOURCE_GROUP. I also split the original patch into two separated
patches for IPv4 and IPv6.
Test: test by myself and customer.
Hangbin Liu (2):
ipv4/igmp: init group mode as INCLUDE when join source group
ipv6/mcast: init as INCLUDE when join SSM INCLUDE group
include/linux/igmp.h | 2 ++
include/net/ipv6.h | 2 ++
net/ipv4/igmp.c | 58 +++++++++++++++++++++++++++++++------------
net/ipv4/ip_sockglue.c | 4 +--
net/ipv6/ipv6_sockglue.c | 5 ++--
net/ipv6/mcast.c | 64 ++++++++++++++++++++++++++++++++++--------------
6 files changed, 96 insertions(+), 39 deletions(-)
--
2.5.5
^ permalink raw reply
* Re: [PATCH v3 net-next] net/sched: add skbprio scheduler
From: Marcelo Ricardo Leitner @ 2018-07-10 14:33 UTC (permalink / raw)
To: Michel Machado
Cc: Nishanth Devarajan, xiyou.wangcong, jhs, jiri, davem, netdev,
doucette
In-Reply-To: <9322b915-8193-7011-e9ba-e7b22ceb08ba@digirati.com.br>
On Tue, Jul 10, 2018 at 10:03:22AM -0400, Michel Machado wrote:
...
> > You can get 64 different priorities by stacking sch_prio, btw. And if
> > you implement drop_from_tail() as part of Qdisc, you can even get it
> > working for this cascading case too.
>
> A solution would be to add another flag to switch between the current
> prio_classify() and a new one to just use skb->priority as in skbprio. This
Sounds promising.
> way we don't risk breaking applications that rely on tcf_classify() and this
> odd behavior that I found in prio_classify():
>
> band = TC_H_MIN(band) - 1;
> if (band >= q->bands)
> return q->queues[q->prio2band[0]];
> return q->queues[band];
>
> When band is zero, it returns q->queues[q->prio2band[0]] instead of
> q->queues[band] as it would for other bands less than q->bands.
Agreed, this looks odd. It came from 1d8ae3fdeb00 ("pkt_sched: Remove
RR scheduler."):
band = TC_H_MIN(band) - 1;
if (band >= q->bands)
- band = q->prio2band[0];
-out:
- if (q->mq)
- skb_set_queue_mapping(skb, band);
+ return q->queues[q->prio2band[0]];
+
return q->queues[band];
}
I can see how it made sense before the change, but not after.
>
> > > > > 3. The queues of sch_prio.c are struct Qdisc, which don't have a method
> > > > > to drop at its tail.
> > > >
> > > > That can be implemented, most likely as prio_tail_drop() as above.
> > >
> > > struct Qdisc represents *all* qdiscs. My knowledge of the other qdiscs is
> > > limited, but not all qdiscs may have a meaningful method to drop at the
> > > tail. For example: a qdisc that works over flows may not know with flow is
> >
> > True, but it doesn't mean you have to implement it for all available qdiscs.
>
> If it is not implemented for all available qdiscs and the flag to drop at
> the tail is on, sch_prio.c would need to issue a log message whenever a
> packet goes into one of the subqueues that don't drop at the tail and have a
> failsafe behavior.
That's fine. pr_warn_ratelimit() is probably what we need for logging
the error, so it a) doesn't flood kernel log and b) gets activated
even if the sysadmin later try again with another qdisc (as opposed to
pr_warn_once).
For the failsafe behavior, it probably can then just drop the incoming
packet. It is not what you want, yes, but it's an easy way out out of
a non-expected situation and that works well enough.
>
> > > the tail. Not to mention that this would be a widespread patch to only
> > > support this new prio qdisc. It would be prudent to wait for the production
> > > success of the proposed, self-contained qdisc before making this commitment.
> >
> > On the other hand, by adding another qdisc you're adding more work
> > that one needs to do when dealing with qdisc infrastructure, such as
> > updating enqueue() prototype, for example.
> >
> > Once this new qdisc is in, it won't be easy to deprecate it.
>
> We need to choose between (1) having skbprio that has some duplicate code
> with sch_prio.c and (2) adding flags to sch_prio.c and make a major
> refactoring of the schedule subsystem to add drop an the tail to qdiscs.
Yes,
>
> I mean major because we are not just talking about adding the method
> dequeue_tail() to struct Qdisc and adding dequeue_tail() to all qdiscs. One
I think it is. :-)
> will need to come up with definitions of dequeue_tail() for qdiscs that
> don't naturally have it and even rewrite the data structures of qdiscs. To
> substantiate this last point, consider sch_fifo.c, one of the simplest
> qdiscs available. sch_fifo.c keeps its packets in sch->q, which is of type
> struct qdisc_skb_head. struct qdisc_skb_head doesn't set skb->prev, so it
> cannot drop at the tail without walking through its list.
Yes but this would only be needed for the qdiscs that you want to
support with this flag. Nobody said you need to implement it on all
qdiscs that we have...
>
> I do understand the motivation for minimizing duplicate code. But the
> small amount of duplicate code that skbprio adds is cheaper than refactoring
> the scheduler system to only support this new sch_prio.c.
I'm afraid that without the code for option (2) above, this discussion
will become subjective. I'll wait for other opinions here.
Cheers,
Marcelo
^ permalink raw reply
* [PATCH] qed: fix spelling mistake "successffuly" -> "successfully"
From: Ewan D. Milne @ 2018-07-10 14:27 UTC (permalink / raw)
To: David S. Miller, netdev; +Cc: linux-kernel
From: "Ewan D. Milne" <emilne@redhat.com>
Trivial fix to spelling mistake in qed_probe message.
Signed-off-by: Ewan D. Milne <emilne@redhat.com>
---
drivers/net/ethernet/qlogic/qed/qed_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 0cbc74d..758a9a51 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -371,7 +371,7 @@ static struct qed_dev *qed_probe(struct pci_dev *pdev,
goto err2;
}
- DP_INFO(cdev, "qed_probe completed successffuly\n");
+ DP_INFO(cdev, "qed_probe completed successfully\n");
return cdev;
--
2.1.0
^ permalink raw reply related
* [PATCH] net: sched: act_ife: fix memory leak
From: Gustavo A. R. Silva @ 2018-07-10 13:53 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller
Cc: netdev, linux-kernel, Vlad Buslov, Gustavo A. R. Silva
In case memory resources for *p* are allocated, release them
before return.
Addresses-Coverity-ID: 1471714 ("Resource leak")
Fixes: 0190c1d452a9 ("net: sched: atomically check-allocate action")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
net/sched/act_ife.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index a3eef00..3d6e265 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -485,8 +485,10 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
return -ENOMEM;
err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
- if (err < 0)
+ if (err < 0) {
+ kfree(p);
return err;
+ }
exists = err;
if (exists && bind) {
kfree(p);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v3 net-next] net/sched: add skbprio scheduler
From: Michel Machado @ 2018-07-10 14:03 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Nishanth Devarajan, xiyou.wangcong, jhs, jiri, davem, netdev,
doucette
In-Reply-To: <20180709214016.GD10923@localhost.localdomain>
On 07/09/2018 05:40 PM, Marcelo Ricardo Leitner wrote:
> On Mon, Jul 09, 2018 at 05:03:31PM -0400, Michel Machado wrote:
>> On 07/09/2018 03:53 PM, Marcelo Ricardo Leitner wrote:
>>> On Mon, Jul 09, 2018 at 02:18:33PM -0400, Michel Machado wrote:
>>>> On 07/09/2018 11:44 AM, Marcelo Ricardo Leitner wrote:
>>>>> On Sat, Jul 07, 2018 at 03:43:55PM +0530, Nishanth Devarajan wrote:
>>>>>> net/sched: add skbprio scheduer
>>>>>>
>>>>>> Skbprio (SKB Priority Queue) is a queueing discipline that prioritizes packets
>>>>>> according to their skb->priority field. Under congestion, already-enqueued lower
>>>>>> priority packets will be dropped to make space available for higher priority
>>>>>> packets. Skbprio was conceived as a solution for denial-of-service defenses that
>>>>>> need to route packets with different priorities as a means to overcome DoS
>>>>>> attacks.
>>>>>
>>>>> Why can't we implement this as a new flag for sch_prio.c?
>>>>>
>>>>> I don't see why this duplication is needed, especially because it will
>>>>> only be "slower" (as in, it will do more work) when qdisc is already
>>>>> full and dropping packets anyway.
>>>>
>>>> sch_prio.c and skbprio diverge on a number of aspects:
>>>>
>>>> 1. sch_prio.c supports up to 16 priorities whereas skbprio 64. This is
>>>> not just a matter of changing a constant since sch_prio.c doesn't use
>>>> skb->priority.
>>>
>>> Yes it does use skb->priority for classifying into a band:
>>>
>>> prio_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
>>> {
>>> struct prio_sched_data *q = qdisc_priv(sch);
>>> u32 band = skb->priority;
>>> ...
>>
>> Changing TC_PRIO_MAX from 15 to 63 risks breaking backward compatibility
>> with applications.
>
> If done, it needs to be done carefully, indeed. I don't know if it's
> doable, neither I know how hard is your requirement for 64 different
> priorities.
>
> You can get 64 different priorities by stacking sch_prio, btw. And if
> you implement drop_from_tail() as part of Qdisc, you can even get it
> working for this cascading case too.
A solution would be to add another flag to switch between the
current prio_classify() and a new one to just use skb->priority as in
skbprio. This way we don't risk breaking applications that rely on
tcf_classify() and this odd behavior that I found in prio_classify():
band = TC_H_MIN(band) - 1;
if (band >= q->bands)
return q->queues[q->prio2band[0]];
return q->queues[band];
When band is zero, it returns q->queues[q->prio2band[0]] instead of
q->queues[band] as it would for other bands less than q->bands.
>>>> 3. The queues of sch_prio.c are struct Qdisc, which don't have a method
>>>> to drop at its tail.
>>>
>>> That can be implemented, most likely as prio_tail_drop() as above.
>>
>> struct Qdisc represents *all* qdiscs. My knowledge of the other qdiscs is
>> limited, but not all qdiscs may have a meaningful method to drop at the
>> tail. For example: a qdisc that works over flows may not know with flow is
>
> True, but it doesn't mean you have to implement it for all available qdiscs.
If it is not implemented for all available qdiscs and the flag to
drop at the tail is on, sch_prio.c would need to issue a log message
whenever a packet goes into one of the subqueues that don't drop at the
tail and have a failsafe behavior.
>> the tail. Not to mention that this would be a widespread patch to only
>> support this new prio qdisc. It would be prudent to wait for the production
>> success of the proposed, self-contained qdisc before making this commitment.
>
> On the other hand, by adding another qdisc you're adding more work
> that one needs to do when dealing with qdisc infrastructure, such as
> updating enqueue() prototype, for example.
>
> Once this new qdisc is in, it won't be easy to deprecate it.
We need to choose between (1) having skbprio that has some duplicate
code with sch_prio.c and (2) adding flags to sch_prio.c and make a major
refactoring of the schedule subsystem to add drop an the tail to qdiscs.
I mean major because we are not just talking about adding the method
dequeue_tail() to struct Qdisc and adding dequeue_tail() to all qdiscs.
One will need to come up with definitions of dequeue_tail() for qdiscs
that don't naturally have it and even rewrite the data structures of
qdiscs. To substantiate this last point, consider sch_fifo.c, one of the
simplest qdiscs available. sch_fifo.c keeps its packets in sch->q, which
is of type struct qdisc_skb_head. struct qdisc_skb_head doesn't set
skb->prev, so it cannot drop at the tail without walking through its list.
I do understand the motivation for minimizing duplicate code. But
the small amount of duplicate code that skbprio adds is cheaper than
refactoring the scheduler system to only support this new sch_prio.c.
[ ]'s
Michel Machado
^ permalink raw reply
* Re: [PATCH net] ipv4: reset fnhe_mtu_locked after cache route flushed
From: Sabrina Dubroca @ 2018-07-10 14:00 UTC (permalink / raw)
To: David Miller; +Cc: liuhangbin, netdev, sbrivio
In-Reply-To: <20180510.154311.1379589939333253104.davem@davemloft.net>
2018-05-10, 15:43:11 -0400, David Miller wrote:
> From: Hangbin Liu <liuhangbin@gmail.com>
> Date: Wed, 9 May 2018 18:06:44 +0800
>
> > After route cache is flushed via ipv4_sysctl_rtcache_flush(), we forget
> > to reset fnhe_mtu_locked in rt_bind_exception(). When pmtu is updated
> > in __ip_rt_update_pmtu(), it will return directly since the pmtu is
> > still locked. e.g.
> >
> > + ip netns exec client ping 10.10.1.1 -c 1 -s 1400 -M do
> > PING 10.10.1.1 (10.10.1.1) 1400(1428) bytes of data.
> > From 10.10.0.254 icmp_seq=1 Frag needed and DF set (mtu = 0)
> >
> > --- 10.10.1.1 ping statistics ---
> > 1 packets transmitted, 0 received, +1 errors, 100% packet loss, time 0ms
> >
> > + ip netns exec client ip route get 10.10.1.1
> > 10.10.1.1 via 10.10.0.254 dev veth0_c src 10.10.0.1 uid 0
> > cache expires 599sec mtu lock 552
> > + ip netns exec client ip route flush cache
> > + ip netns exec client ip route get 10.10.1.1
> > 10.10.1.1 via 10.10.0.254 dev veth0_c src 10.10.0.1 uid 0
> > cache
> > + ip netns exec client ping 10.10.1.1 -c 1 -s 1400 -M do
> > PING 10.10.1.1 (10.10.1.1) 1400(1428) bytes of data.
> > ping: local error: Message too long, mtu=576
> >
> > --- 10.10.1.1 ping statistics ---
> > 1 packets transmitted, 0 received, +1 errors, 100% packet loss, time 0ms
> >
> > + ip netns exec client ip route get 10.10.1.1
> > 10.10.1.1 via 10.10.0.254 dev veth0_c src 10.10.0.1 uid 0
> > cache
> >
> > Fixes: d52e5a7e7ca49 ("ipv4: lock mtu in fnhe when received PMTU < net.ipv4.route.min_pmtu")
> > Reported-by: Jianlin Shi <jishi@redhat.com>
> > Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>
> Applied.
meh, I know it's way too late, but I just noticed this patch lost most
of its commit message, and all of its tags, when it went into git :(
Patchwork actually thinks the format is ok (it counts one Fixes and
one Reviewed-by), but git cut all that out.
Would there be a way to detect this when you apply patches? "there's
no tag above the first '^---', but there are some tags below it",
something like that?
commit 0e8411e426e277f55bd21e287ec89fab6f8eacae
Author: Hangbin Liu <liuhangbin@gmail.com>
Date: Wed May 9 18:06:44 2018 +0800
ipv4: reset fnhe_mtu_locked after cache route flushed
After route cache is flushed via ipv4_sysctl_rtcache_flush(), we forget
to reset fnhe_mtu_locked in rt_bind_exception(). When pmtu is updated
in __ip_rt_update_pmtu(), it will return directly since the pmtu is
still locked. e.g.
+ ip netns exec client ping 10.10.1.1 -c 1 -s 1400 -M do
PING 10.10.1.1 (10.10.1.1) 1400(1428) bytes of data.
>From 10.10.0.254 icmp_seq=1 Frag needed and DF set (mtu = 0)
Signed-off-by: David S. Miller <davem@davemloft.net>
--
Sabrina
^ permalink raw reply
* Re: [PATCH net-next] net: sched: refactor flower walk to iterate over idr
From: Simon Horman @ 2018-07-10 13:55 UTC (permalink / raw)
To: Vlad Buslov; +Cc: netdev, davem, jhs, xiyou.wangcong, jiri
In-Reply-To: <1531132151-2321-1-git-send-email-vladbu@mellanox.com>
On Mon, Jul 09, 2018 at 01:29:11PM +0300, Vlad Buslov wrote:
> Extend struct tcf_walker with additional 'cookie' field. It is intended to
> be used by classifier walk implementations to continue iteration directly
> from particular filter, instead of iterating 'skip' number of times.
>
> Change flower walk implementation to save filter handle in 'cookie'. Each
> time flower walk is called, it looks up filter with saved handle directly
> with idr, instead of iterating over filter linked list 'skip' number of
> times. This change improves complexity of dumping flower classifier from
> quadratic to linearithmic. (assuming idr lookup has logarithmic complexity)
>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Reported-by: Simon Horman <simon.horman@netronome.com>
Thanks, I'm very pleased to see this change. I would appreciate it if
we could have a little time to test its impact on performance thoroughly.
One question: will this work as expected (i.e. be at least backwards
compatible) with existing user-space code?
^ permalink raw reply
* [PATCH] net: ethernet: ti: cpts: break cycle once late ts is matched
From: Ivan Khoronzhuk @ 2018-07-10 13:04 UTC (permalink / raw)
To: davem, grygorii.strashko; +Cc: netdev, linux-kernel, Ivan Khoronzhuk
The late ts queue can contain a bunch of skbs while hi rate testing,
no need to check all of them if timestamp is already matched.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
Based on net-next/master
drivers/net/ethernet/ti/cpts.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index 6f63c8729afc..b4ea58dc8caf 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -114,7 +114,10 @@ static bool cpts_match_tx_ts(struct cpts *cpts, struct cpts_event *event)
dev_consume_skb_any(skb);
dev_dbg(cpts->dev, "match tx timestamp mtype %u seqid %04x\n",
mtype, seqid);
- } else if (time_after(jiffies, skb_cb->tmo)) {
+ break;
+ }
+
+ if (time_after(jiffies, skb_cb->tmo)) {
/* timeout any expired skbs over 1s */
dev_dbg(cpts->dev,
"expiring tx timestamp mtype %u seqid %04x\n",
--
2.17.1
^ permalink raw reply related
* Re: [net-next,v3] tcp: Improve setsockopt() TCP_USER_TIMEOUT accuracy
From: Eric Dumazet @ 2018-07-10 12:48 UTC (permalink / raw)
To: Jon Maxwell, davem
Cc: edumazet, ncardwell, David.Laight, kuznet, yoshfuji, netdev,
linux-kernel, jmaxwell
In-Reply-To: <3f856638-52bc-6630-a3d2-2b50f1bf7e48@gmail.com>
On 07/10/2018 05:38 AM, Eric Dumazet wrote:
> Note that if we always do jiffies_to_msecs(icsk->icsk_user_timeout) in TCP,
> we also could change the convention and store msecs in this field instead of jiffies.
>
> That would eliminate the msecs_to_jiffies() and jiffies_to_msecs() dance.
>
> (That would be done in a patch of its own, of course)
tcp_keepalive_timer() does use icsk->icsk_user_timeout directly in jiffies unit,
but considering keeapalive timers are rarely used, this point would have to
do the msecs_to_jiffies() conversion.
^ permalink raw reply
* [PATCH net-next] selftests: forwarding: mirror_gre_nh: Unset rp_filter on host VRF
From: Petr Machata @ 2018-07-10 12:44 UTC (permalink / raw)
To: netdev, linux-kselftest; +Cc: davem, shuah, idosch
The mirrored packets arrive at $h3 encapsulated in GRE/IPv4, with IP
address from 192.0.2.128/28 network. However the interface is configured
as a member of 192.0.2.160/28 and there's no route directing traffic
from the former network through that interface. Correspondingly, the RP
filter on the VRF rejects it.
Therefore turn off the VRF's RP filter.
Signed-off-by: Petr Machata <petrm@mellanox.com>
---
tools/testing/selftests/net/forwarding/mirror_gre_nh.sh | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_nh.sh b/tools/testing/selftests/net/forwarding/mirror_gre_nh.sh
index 8fa681eb90e7..6f9ef1820e93 100755
--- a/tools/testing/selftests/net/forwarding/mirror_gre_nh.sh
+++ b/tools/testing/selftests/net/forwarding/mirror_gre_nh.sh
@@ -35,6 +35,8 @@ setup_prepare()
vrf_prepare
mirror_gre_topo_create
+ sysctl_set net.ipv4.conf.v$h3.rp_filter 0
+
ip address add dev $swp3 192.0.2.161/28
ip address add dev $h3 192.0.2.162/28
ip address add dev gt4 192.0.2.129/32
@@ -61,6 +63,8 @@ cleanup()
ip address del dev $h3 192.0.2.162/28
ip address del dev $swp3 192.0.2.161/28
+ sysctl_restore net.ipv4.conf.v$h3.rp_filter 0
+
mirror_gre_topo_destroy
vrf_cleanup
--
2.4.11
^ permalink raw reply related
* BUG: KASAN: use-after-free in ex_handler_refcount
From: Sudip Mukherjee @ 2018-07-10 12:43 UTC (permalink / raw)
To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, David S. Miller
Cc: linux-bluetooth, netdev, linux-kernel
Hi All,
I was running a KASAN enabled kernel and noticed the following:
[ 916.786725] ==================================================================
[ 916.786746] BUG: KASAN: use-after-free in ex_handler_refcount+0x5b/0x127
[ 916.786753] Write of size 4 at addr ffff880105144bc0 by task kworker/u9:0/2298
[ 916.786763] CPU: 1 PID: 2298 Comm: kworker/u9:0 Tainted: G U W O 4.14.47-20180606+ #32
[ 916.786767] Hardware name: xxx yyy/zzz, BIOS 2017.01-00087-g43e04de 08/30/2017
[ 916.786805] Workqueue: hci0 hci_rx_work [bluetooth]
[ 916.786810] Call Trace:
[ 916.786824] dump_stack+0x46/0x59
[ 916.786834] print_address_description+0x6b/0x23b
[ 916.786842] ? ex_handler_refcount+0x5b/0x127
[ 916.786848] kasan_report+0x220/0x246
[ 916.786856] ex_handler_refcount+0x5b/0x127
[ 916.786863] ? ex_handler_clear_fs+0x85/0x85
[ 916.786870] fixup_exception+0x8c/0x96
[ 916.786878] do_trap+0x66/0x2c1
[ 916.786886] do_error_trap+0x152/0x180
[ 916.786893] ? fixup_bug+0x78/0x78
[ 916.786926] ? amp_destroy_logical_link+0xd0/0xf6 [bluetooth]
[ 916.786933] ? __schedule+0x113b/0x1453
[ 916.786939] ? sysctl_net_exit+0xe/0xe
[ 916.786946] ? __wake_up_common+0x343/0x343
[ 916.786952] ? insert_work+0x107/0x163
[ 916.786959] invalid_op+0x1b/0x40
[ 916.786994] RIP: 0010:amp_destroy_logical_link+0xd0/0xf6 [bluetooth]
[ 916.786998] RSP: 0018:ffff88009540f970 EFLAGS: 00010296
[ 916.787004] RAX: 0000000000000000 RBX: ffff880105144b48 RCX: ffff880105144bc0
[ 916.787008] RDX: 000000000000002f RSI: ffff88013b80ed40 RDI: ffffffffa05810c0
[ 916.787012] RBP: ffff8800069c59d8 R08: 000000003fee624d R09: ffffffff81cfcf9b
[ 916.787015] R10: 000000008e0e2c51 R11: 0000000000000001 R12: ffff880042ddc908
[ 916.787019] R13: ffff880105144bc8 R14: 0000000000000068 R15: ffff880093f02168
[ 916.787027] ? __sk_destruct+0x2c6/0x2d4
[ 916.787063] hci_event_packet+0xff5/0x7dd2 [bluetooth]
[ 916.787098] ? hci_le_meta_evt+0x2bab/0x2bab [bluetooth]
[ 916.787117] ? xhci_urb_enqueue+0xbd8/0xcf5 [xhci_hcd]
[ 916.787127] ? __accumulate_pelt_segments+0x24/0x33
[ 916.787133] ? __accumulate_pelt_segments+0x24/0x33
[ 916.787140] ? __update_load_avg_se.isra.2+0x217/0x3a4
[ 916.787146] ? set_next_entity+0x7c3/0x12cd
[ 916.787153] ? pick_next_entity+0x25e/0x26c
[ 916.787159] ? pick_next_task_fair+0x2ca/0xc1a
[ 916.787165] ? __accumulate_pelt_segments+0x24/0x33
[ 916.787172] ? __update_load_avg_cfs_rq.isra.3+0x24b/0x44c
[ 916.787178] ? __switch_to+0x769/0xbc4
[ 916.787185] ? compat_start_thread+0x66/0x66
[ 916.787192] ? finish_task_switch+0x392/0x431
[ 916.787222] ? hci_rx_work+0x154/0x487 [bluetooth]
[ 916.787252] hci_rx_work+0x154/0x487 [bluetooth]
[ 916.787261] process_one_work+0x579/0x9e9
[ 916.787268] worker_thread+0x68f/0x804
[ 916.787277] kthread+0x31c/0x32b
[ 916.787283] ? rescuer_thread+0x70c/0x70c
[ 916.787289] ? kthread_create_on_node+0xa3/0xa3
[ 916.787297] ret_from_fork+0x35/0x40
[ 916.787305] Allocated by task 2298:
[ 916.787315] kasan_kmalloc.part.1+0x51/0xc7
[ 916.787320] __kmalloc+0x17f/0x1b6
[ 916.787326] sk_prot_alloc+0xf2/0x1a3
[ 916.787332] sk_alloc+0x22/0x297
[ 916.787364] sco_sock_alloc.constprop.7+0x23/0x202 [bluetooth]
[ 916.787397] sco_connect_cfm+0x2d0/0x566 [bluetooth]
[ 916.787427] hci_conn_request_evt.isra.53+0x6d3/0x762 [bluetooth]
[ 916.787458] hci_event_packet+0x85e/0x7dd2 [bluetooth]
[ 916.787486] hci_rx_work+0x154/0x487 [bluetooth]
[ 916.787491] process_one_work+0x579/0x9e9
[ 916.787496] worker_thread+0x68f/0x804
[ 916.787502] kthread+0x31c/0x32b
[ 916.787508] ret_from_fork+0x35/0x40
[ 916.787512] Freed by task 2298:
[ 916.787519] kasan_slab_free+0xb3/0x15e
[ 916.787524] kfree+0x103/0x1a9
[ 916.787528] __sk_destruct+0x2c6/0x2d4
[ 916.787560] sco_conn_del.isra.1+0xba/0x10e [bluetooth]
[ 916.787591] hci_event_packet+0xff5/0x7dd2 [bluetooth]
[ 916.787619] hci_rx_work+0x154/0x487 [bluetooth]
[ 916.787624] process_one_work+0x579/0x9e9
[ 916.787629] worker_thread+0x68f/0x804
[ 916.787635] kthread+0x31c/0x32b
[ 916.787641] ret_from_fork+0x35/0x40
[ 916.787647] The buggy address belongs to the object at ffff880105144b48
which belongs to the cache kmalloc-1024 of size 1024
[ 916.787652] The buggy address is located 120 bytes inside of
1024-byte region [ffff880105144b48, ffff880105144f48)
[ 916.787654] The buggy address belongs to the page:
[ 916.787660] page:ffffea0004145000 count:1 mapcount:0 mapping: (null) index:0x0 compound_mapcount: 0
[ 916.798662] flags: 0x8000000000008100(slab|head)
[ 916.803829] raw: 8000000000008100 0000000000000000 0000000000000000 0000000100170017
[ 916.803836] raw: ffffea00001a7220 ffffea0000931420 ffff88013b80ed40 0000000000000000
[ 916.803839] page dumped because: kasan: bad access detected
[ 916.803842] Memory state around the buggy address:
[ 916.803849] ffff880105144a80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 916.803853] ffff880105144b00: fc fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb
[ 916.803858] >ffff880105144b80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 916.803861] ^
[ 916.803865] ffff880105144c00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 916.803870] ffff880105144c80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 916.803872] ==================================================================
Will really appreciate help in finding the issue and fixing it.
It is reproducible on almost all cycles, so I can test any patch if needed.
--
Regards
Sudip
^ permalink raw reply
* Re: [net-next,v3] tcp: Improve setsockopt() TCP_USER_TIMEOUT accuracy
From: Eric Dumazet @ 2018-07-10 12:38 UTC (permalink / raw)
To: Jon Maxwell, davem
Cc: edumazet, eric.dumazet, ncardwell, David.Laight, kuznet, yoshfuji,
netdev, linux-kernel, jmaxwell
In-Reply-To: <20180710065147.27647-1-jmaxwell37@gmail.com>
On 07/09/2018 11:51 PM, Jon Maxwell wrote:
> v3 contains the following suggestions by Neal Cardwell:
>
> 1) Fix up units mismatch regarding msec/jiffies.
> 2) Address possiblility of time_remaining being negative.
> 3) Add a helper routine tcp_clamp_rto_to_user_timeout() to do the rto
> calculation.
> 4) Move start_ts logic into helper routine tcp_retrans_stamp() to
> validate tcp_sk(sk)->retrans_stamp.
> 5) Some u32 declation and return refactoring.
> 6) Return 0 instead of false in tcp_retransmit_stamp(), it's not a bool.
>
> Suggestions by David Laight:
>
> 1) Don't cache rto in tcp_clamp_rto_to_user_timeout().
> 2) Use conditional operator instead of min_t() in
> tcp_clamp_rto_to_user_timeout()
>
> Changes:
>
> 1) Call tcp_clamp_rto_to_user_timeout(sk) as an argument to
> inet_csk_reset_xmit_timer() to save on rto declaration.
>
> Every time the TCP retransmission timer fires. It checks to see if there is a
> timeout before scheduling the next retransmit timer. The retransmit interval
> between each retransmission increases exponentially. The issue is that in order
> for the timeout to occur the retransmit timer needs to fire again. If the user
> timeout check happens after the 9th retransmit for example. It needs to wait for
> the 10th retransmit timer to fire in order to evaluate whether a timeout has
> occurred or not. If the interval is large enough then the timeout will be
> inaccurate.
>
> For example with a TCP_USER_TIMEOUT of 10 seconds without patch:
>
> 1st retransmit:
>
> 22:25:18.973488 IP host1.49310 > host2.search-agent: Flags [.]
>
> Last retransmit:
>
> 22:25:26.205499 IP host1.49310 > host2.search-agent: Flags [.]
>
> Timeout:
>
> send: Connection timed out
> Sun Jul 1 22:25:34 EDT 2018
>
> We can see that last retransmit took ~7 seconds. Which pushed the total
> timeout to ~15 seconds instead of the expected 10 seconds. This gets more
> inaccurate the larger the TCP_USER_TIMEOUT value. As the interval increases.
>
> Add tcp_clamp_rto_to_user_timeout() to determine if the user rto has expired.
> Or whether the rto interval needs to be recalculated. Use the original interval
> if user rto is not set.
>
> Test results with the patch is the expected 10 second timeout:
>
> 1st retransmit:
>
> 01:37:59.022555 IP host1.49310 > host2.search-agent: Flags [.]
>
> Last retransmit:
>
> 01:38:06.486558 IP host1.49310 > host2.search-agent: Flags [.]
>
> Timeout:
>
> send: Connection timed out
> Mon Jul 2 01:38:09 EDT 2018
>
> Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
> ---
> net/ipv4/tcp_timer.c | 49 +++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 39 insertions(+), 10 deletions(-)
>
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index 3b3611729928..93239e58776d 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -22,6 +22,38 @@
> #include <linux/gfp.h>
> #include <net/tcp.h>
>
> +u32 tcp_retransmit_stamp(struct sock *sk)
const struct sock *sk;
(To clearly express the fact this helper does not touch the socket)
> +{
> + u32 start_ts = tcp_sk(sk)->retrans_stamp;
> +
> + if (unlikely(!start_ts)) {
> + struct sk_buff *head = tcp_rtx_queue_head(sk);
> +
> + if (!head)
> + return 0;
> + start_ts = tcp_skb_timestamp(head);
> + }
> + return start_ts;
> +}
> +
> +static __u32 tcp_clamp_rto_to_user_timeout(struct sock *sk)
const struct sock *sk
> +{
> + struct inet_connection_sock *icsk = inet_csk(sk);
> + __u32 elapsed, user_timeout;
> + u32 start_ts;
Mixing __u32 and u32 in new code is confusing.
__u32 are needed in uapi include files, not in the C kernel code.
u32 elapsed, user_timeout, start_ts;
> +
> + start_ts = tcp_retransmit_stamp(sk);
> + if (!icsk->icsk_user_timeout || !start_ts)
> + return icsk->icsk_rto;
> + elapsed = tcp_time_stamp(tcp_sk(sk)) - start_ts;
> + user_timeout = jiffies_to_msecs(icsk->icsk_user_timeout);
Note that if we always do jiffies_to_msecs(icsk->icsk_user_timeout) in TCP,
we also could change the convention and store msecs in this field instead of jiffies.
That would eliminate the msecs_to_jiffies() and jiffies_to_msecs() dance.
(That would be done in a patch of its own, of course)
> + if (elapsed >= user_timeout)
> + return 1; /* user timeout has passed; fire ASAP */
> + else
> + return (icsk->icsk_rto < msecs_to_jiffies(user_timeout - elapsed)) ?
> + icsk->icsk_rto : msecs_to_jiffies(user_timeout - elapsed);
return min_t(i32, icsk->icsk_rto, msecs_to_jiffies(user_timeout - elapsed));
> +}
> +
> /**
> * tcp_write_err() - close socket and save error info
> * @sk: The socket the error has appeared on.
> @@ -161,19 +193,15 @@ static bool retransmits_timed_out(struct sock *sk,
> unsigned int timeout)
> {
> const unsigned int rto_base = TCP_RTO_MIN;
> - unsigned int linear_backoff_thresh, start_ts;
> + unsigned int linear_backoff_thresh;
> + u32 start_ts;
This seems a gratuitous change, making your patch bigger than necessary.
u32 and unsigned int are the same essentially.
>
> if (!inet_csk(sk)->icsk_retransmits)
> return false;
>
> - start_ts = tcp_sk(sk)->retrans_stamp;
> - if (unlikely(!start_ts)) {
> - struct sk_buff *head = tcp_rtx_queue_head(sk);
> -
> - if (!head)
> - return false;
> - start_ts = tcp_skb_timestamp(head);
> - }
> + start_ts = tcp_retransmit_stamp(sk);
> + if (!start_ts)
> + return false;
One thing you could do is to make this refactoring in a single patch,
and provide a patch series, with small units, making the review easier.
>
> if (likely(timeout == 0)) {
> linear_backoff_thresh = ilog2(TCP_RTO_MAX/rto_base);
> @@ -535,7 +563,8 @@ void tcp_retransmit_timer(struct sock *sk)
> /* Use normal (exponential) backoff */
> icsk->icsk_rto = min(icsk->icsk_rto << 1, TCP_RTO_MAX);
> }
> - inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, icsk->icsk_rto, TCP_RTO_MAX);
> + inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
> + tcp_clamp_rto_to_user_timeout(sk), TCP_RTO_MAX);
> if (retransmits_timed_out(sk, net->ipv4.sysctl_tcp_retries1 + 1, 0))
> __sk_dst_reset(sk);
>
>
Thanks !
^ permalink raw reply
* [PATCH net-next 13/13] ARM: net: bpf: use double-word load/stores where available
From: Russell King @ 2018-07-10 12:37 UTC (permalink / raw)
To: netdev, linux-arm-kernel; +Cc: Daniel Borkmann
In-Reply-To: <20180710123340.GK17271@n2100.armlinux.org.uk>
Use double-word load and stores where support for this instruction is
supported by the CPU architecture.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
arch/arm/net/bpf_jit_32.c | 55 ++++++++++++++++++++++++++++++++++++-----------
arch/arm/net/bpf_jit_32.h | 2 ++
2 files changed, 45 insertions(+), 12 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index ca6534cabfa9..7e1d1635c65b 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -22,6 +22,7 @@
#include <asm/cacheflush.h>
#include <asm/hwcap.h>
#include <asm/opcodes.h>
+#include <asm/system_info.h>
#include "bpf_jit_32.h"
@@ -192,6 +193,7 @@ struct jit_ctx {
unsigned int idx;
unsigned int prologue_bytes;
unsigned int epilogue_offset;
+ unsigned int cpu_architecture;
u32 flags;
u32 *offsets;
u32 *target;
@@ -271,10 +273,12 @@ static u32 arm_bpf_ldst_imm8(u32 op, u8 rt, u8 rn, s16 imm8)
#define ARM_LDR_I(rt, rn, off) arm_bpf_ldst_imm12(ARM_INST_LDR_I, rt, rn, off)
#define ARM_LDRB_I(rt, rn, off) arm_bpf_ldst_imm12(ARM_INST_LDRB_I, rt, rn, off)
+#define ARM_LDRD_I(rt, rn, off) arm_bpf_ldst_imm8(ARM_INST_LDRD_I, rt, rn, off)
#define ARM_LDRH_I(rt, rn, off) arm_bpf_ldst_imm8(ARM_INST_LDRH_I, rt, rn, off)
#define ARM_STR_I(rt, rn, off) arm_bpf_ldst_imm12(ARM_INST_STR_I, rt, rn, off)
#define ARM_STRB_I(rt, rn, off) arm_bpf_ldst_imm12(ARM_INST_STRB_I, rt, rn, off)
+#define ARM_STRD_I(rt, rn, off) arm_bpf_ldst_imm8(ARM_INST_STRD_I, rt, rn, off)
#define ARM_STRH_I(rt, rn, off) arm_bpf_ldst_imm8(ARM_INST_STRH_I, rt, rn, off)
/*
@@ -485,10 +489,16 @@ static const s8 *arm_bpf_get_reg64(const s8 *reg, const s8 *tmp,
struct jit_ctx *ctx)
{
if (is_stacked(reg[1])) {
- emit(ARM_LDR_I(tmp[1], ARM_FP, EBPF_SCRATCH_TO_ARM_FP(reg[1])),
- ctx);
- emit(ARM_LDR_I(tmp[0], ARM_FP, EBPF_SCRATCH_TO_ARM_FP(reg[0])),
- ctx);
+ if (__LINUX_ARM_ARCH__ >= 6 ||
+ ctx->cpu_architecture >= CPU_ARCH_ARMv5TE) {
+ emit(ARM_LDRD_I(tmp[1], ARM_FP,
+ EBPF_SCRATCH_TO_ARM_FP(reg[1])), ctx);
+ } else {
+ emit(ARM_LDR_I(tmp[1], ARM_FP,
+ EBPF_SCRATCH_TO_ARM_FP(reg[1])), ctx);
+ emit(ARM_LDR_I(tmp[0], ARM_FP,
+ EBPF_SCRATCH_TO_ARM_FP(reg[0])), ctx);
+ }
reg = tmp;
}
return reg;
@@ -510,10 +520,16 @@ static void arm_bpf_put_reg64(const s8 *reg, const s8 *src,
struct jit_ctx *ctx)
{
if (is_stacked(reg[1])) {
- emit(ARM_STR_I(src[1], ARM_FP, EBPF_SCRATCH_TO_ARM_FP(reg[1])),
- ctx);
- emit(ARM_STR_I(src[0], ARM_FP, EBPF_SCRATCH_TO_ARM_FP(reg[0])),
- ctx);
+ if (__LINUX_ARM_ARCH__ >= 6 ||
+ ctx->cpu_architecture >= CPU_ARCH_ARMv5TE) {
+ emit(ARM_STRD_I(src[1], ARM_FP,
+ EBPF_SCRATCH_TO_ARM_FP(reg[1])), ctx);
+ } else {
+ emit(ARM_STR_I(src[1], ARM_FP,
+ EBPF_SCRATCH_TO_ARM_FP(reg[1])), ctx);
+ emit(ARM_STR_I(src[0], ARM_FP,
+ EBPF_SCRATCH_TO_ARM_FP(reg[0])), ctx);
+ }
} else {
if (reg[1] != src[1])
emit(ARM_MOV_R(reg[1], src[1]), ctx);
@@ -663,13 +679,27 @@ static inline void emit_a32_mov_r(const s8 dst, const s8 src,
static inline void emit_a32_mov_r64(const bool is64, const s8 dst[],
const s8 src[],
struct jit_ctx *ctx) {
- emit_a32_mov_r(dst_lo, src_lo, ctx);
- if (is64) {
+ if (!is64) {
+ emit_a32_mov_r(dst_lo, src_lo, ctx);
+ /* Zero out high 4 bytes */
+ emit_a32_mov_i(dst_hi, 0, ctx);
+ } else if (__LINUX_ARM_ARCH__ < 6 &&
+ ctx->cpu_architecture < CPU_ARCH_ARMv5) {
/* complete 8 byte move */
+ emit_a32_mov_r(dst_lo, src_lo, ctx);
emit_a32_mov_r(dst_hi, src_hi, ctx);
+ } else if (is_stacked(src_lo) && is_stacked(dst_lo)) {
+ const u8 *tmp = bpf2a32[TMP_REG_1];
+
+ emit(ARM_LDRD_I(tmp[1], ARM_FP, EBPF_SCRATCH_TO_ARM_FP(src_lo)), ctx);
+ emit(ARM_STRD_I(tmp[1], ARM_FP, EBPF_SCRATCH_TO_ARM_FP(dst_lo)), ctx);
+ } else if (is_stacked(src_lo)) {
+ emit(ARM_LDRD_I(dst[1], ARM_FP, EBPF_SCRATCH_TO_ARM_FP(src_lo)), ctx);
+ } else if (is_stacked(dst_lo)) {
+ emit(ARM_STRD_I(src[1], ARM_FP, EBPF_SCRATCH_TO_ARM_FP(dst_lo)), ctx);
} else {
- /* Zero out high 4 bytes */
- emit_a32_mov_i(dst_hi, 0, ctx);
+ emit(ARM_MOV_R(dst[0], src[0]), ctx);
+ emit(ARM_MOV_R(dst[1], src[1]), ctx);
}
}
@@ -1729,6 +1759,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
memset(&ctx, 0, sizeof(ctx));
ctx.prog = prog;
+ ctx.cpu_architecture = cpu_architecture();
/* Not able to allocate memory for offsets[] , then
* we must fall back to the interpreter
diff --git a/arch/arm/net/bpf_jit_32.h b/arch/arm/net/bpf_jit_32.h
index 9d605e7a8309..c08eb890f88a 100644
--- a/arch/arm/net/bpf_jit_32.h
+++ b/arch/arm/net/bpf_jit_32.h
@@ -81,6 +81,7 @@
#define ARM_INST_LDST__IMM12 0x00000fff
#define ARM_INST_LDRB_I 0x05500000
#define ARM_INST_LDRB_R 0x07d00000
+#define ARM_INST_LDRD_I 0x014000d0
#define ARM_INST_LDRH_I 0x015000b0
#define ARM_INST_LDRH_R 0x019000b0
#define ARM_INST_LDR_I 0x05100000
@@ -128,6 +129,7 @@
#define ARM_INST_STR_I 0x05000000
#define ARM_INST_STRB_I 0x05400000
+#define ARM_INST_STRD_I 0x014000f0
#define ARM_INST_STRH_I 0x014000b0
#define ARM_INST_TST_R 0x01100000
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 12/13] ARM: net: bpf: always use odd/even register pair
From: Russell King @ 2018-07-10 12:37 UTC (permalink / raw)
To: netdev, linux-arm-kernel; +Cc: Daniel Borkmann
In-Reply-To: <20180710123340.GK17271@n2100.armlinux.org.uk>
Always use an odd/even register pair for our 64-bit registers, so that
we're able to use the double-word load/store instructions in the future.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
arch/arm/net/bpf_jit_32.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index 926e81c15505..ca6534cabfa9 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -47,27 +47,27 @@
* The callee saved registers depends on whether frame pointers are enabled.
* With frame pointers (to be compliant with the ABI):
*
- * high
- * original ARM_SP => +------------------+ \
- * | pc | |
- * current ARM_FP => +------------------+ } callee saved registers
- * |r4-r8,r10,fp,ip,lr| |
- * +------------------+ /
- * low
+ * high
+ * original ARM_SP => +--------------+ \
+ * | pc | |
+ * current ARM_FP => +--------------+ } callee saved registers
+ * |r4-r9,fp,ip,lr| |
+ * +--------------+ /
+ * low
*
* Without frame pointers:
*
- * high
- * original ARM_SP => +------------------+
- * | r4-r8,r10,fp,lr | callee saved registers
- * current ARM_FP => +------------------+
- * low
+ * high
+ * original ARM_SP => +--------------+
+ * | r4-r9,fp,lr | callee saved registers
+ * current ARM_FP => +--------------+
+ * low
*
* When popping registers off the stack at the end of a BPF function, we
* reference them via the current ARM_FP register.
*/
#define CALLEE_MASK (1 << ARM_R4 | 1 << ARM_R5 | 1 << ARM_R6 | \
- 1 << ARM_R7 | 1 << ARM_R8 | 1 << ARM_R10 | \
+ 1 << ARM_R7 | 1 << ARM_R8 | 1 << ARM_R9 | \
1 << ARM_FP)
#define CALLEE_PUSH_MASK (CALLEE_MASK | 1 << ARM_LR)
#define CALLEE_POP_MASK (CALLEE_MASK | 1 << ARM_PC)
@@ -157,7 +157,7 @@ static const s8 bpf2a32[][2] = {
* for constant blindings and others.
*/
[TMP_REG_1] = {ARM_R7, ARM_R6},
- [TMP_REG_2] = {ARM_R10, ARM_R8},
+ [TMP_REG_2] = {ARM_R9, ARM_R8},
/* Tail call count. Stored on stack scratch space. */
[TCALL_CNT] = {STACK_OFFSET(BPF_TC_HI), STACK_OFFSET(BPF_TC_LO)},
/* temporary register for blinding constants.
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 11/13] ARM: net: bpf: avoid reloading 'array'
From: Russell King @ 2018-07-10 12:37 UTC (permalink / raw)
To: netdev, linux-arm-kernel; +Cc: Daniel Borkmann
In-Reply-To: <20180710123340.GK17271@n2100.armlinux.org.uk>
Rearranging the order of the initial tail call code a little allows is
to avoid reloading the 'array' pointer.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
arch/arm/net/bpf_jit_32.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index 8264cac1322c..926e81c15505 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -1015,16 +1015,16 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
BUILD_BUG_ON(offsetof(struct bpf_array, map.max_entries) >
ARM_INST_LDST__IMM12);
off = offsetof(struct bpf_array, map.max_entries);
- /* array->map.max_entries */
- r_array = arm_bpf_get_reg32(r2[1], tmp2[1], ctx);
- emit(ARM_LDR_I(tmp[1], r_array, off), ctx);
+ r_array = arm_bpf_get_reg32(r2[1], tmp2[0], ctx);
/* index is 32-bit for arrays */
r_index = arm_bpf_get_reg32(r3[1], tmp2[1], ctx);
+ /* array->map.max_entries */
+ emit(ARM_LDR_I(tmp[1], r_array, off), ctx);
/* index >= array->map.max_entries */
emit(ARM_CMP_R(r_index, tmp[1]), ctx);
_emit(ARM_COND_CS, ARM_B(jmp_offset), ctx);
- /* tmp2[1] = index */
+ /* tmp2[0] = array, tmp2[1] = index */
/* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
* goto out;
@@ -1046,7 +1046,6 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
*/
BUILD_BUG_ON(offsetof(struct bpf_array, ptrs) > ARM_ALU_IMM);
off = offsetof(struct bpf_array, ptrs);
- r_array = arm_bpf_get_reg32(r2[1], tmp2[0], ctx);
emit(ARM_ADD_I(tmp[1], r_array, off), ctx);
emit(ARM_LDR_R_SI(tmp[1], tmp[1], r_index, SRTYPE_ASL, 2), ctx);
emit(ARM_CMP_I(tmp[1], 0), ctx);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 10/13] ARM: net: bpf: avoid reloading 'index'
From: Russell King @ 2018-07-10 12:36 UTC (permalink / raw)
To: netdev, linux-arm-kernel; +Cc: Daniel Borkmann
In-Reply-To: <20180710123340.GK17271@n2100.armlinux.org.uk>
Avoid reloading 'index' after we have validated it - it remains in
tmp2[1] up to the point that we begin the code to index the pointer
array, so with a little rearrangement of the registers, we can use
the already loaded value.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
arch/arm/net/bpf_jit_32.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index a375eb33ac69..8264cac1322c 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -1024,6 +1024,8 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
emit(ARM_CMP_R(r_index, tmp[1]), ctx);
_emit(ARM_COND_CS, ARM_B(jmp_offset), ctx);
+ /* tmp2[1] = index */
+
/* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
* goto out;
* tail_call_cnt++;
@@ -1044,9 +1046,8 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
*/
BUILD_BUG_ON(offsetof(struct bpf_array, ptrs) > ARM_ALU_IMM);
off = offsetof(struct bpf_array, ptrs);
- r_array = arm_bpf_get_reg32(r2[1], tmp2[1], ctx);
+ r_array = arm_bpf_get_reg32(r2[1], tmp2[0], ctx);
emit(ARM_ADD_I(tmp[1], r_array, off), ctx);
- r_index = arm_bpf_get_reg32(r3[1], tmp2[1], ctx);
emit(ARM_LDR_R_SI(tmp[1], tmp[1], r_index, SRTYPE_ASL, 2), ctx);
emit(ARM_CMP_I(tmp[1], 0), ctx);
_emit(ARM_COND_EQ, ARM_B(jmp_offset), ctx);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 09/13] ARM: net: bpf: use ldr instructions with shifted rm register
From: Russell King @ 2018-07-10 12:36 UTC (permalink / raw)
To: netdev, linux-arm-kernel; +Cc: Daniel Borkmann
In-Reply-To: <20180710123340.GK17271@n2100.armlinux.org.uk>
Rather than pre-shifting the rm register for the ldr in the tail call,
shift it in the load instruction. This eliminates one unnecessary
instruction.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
arch/arm/net/bpf_jit_32.c | 3 +--
arch/arm/net/bpf_jit_32.h | 4 ++++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index c7591877c350..a375eb33ac69 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -1047,8 +1047,7 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
r_array = arm_bpf_get_reg32(r2[1], tmp2[1], ctx);
emit(ARM_ADD_I(tmp[1], r_array, off), ctx);
r_index = arm_bpf_get_reg32(r3[1], tmp2[1], ctx);
- emit(ARM_MOV_SI(tmp[0], r_index, SRTYPE_ASL, 2), ctx);
- emit(ARM_LDR_R(tmp[1], tmp[1], tmp[0]), ctx);
+ emit(ARM_LDR_R_SI(tmp[1], tmp[1], r_index, SRTYPE_ASL, 2), ctx);
emit(ARM_CMP_I(tmp[1], 0), ctx);
_emit(ARM_COND_EQ, ARM_B(jmp_offset), ctx);
diff --git a/arch/arm/net/bpf_jit_32.h b/arch/arm/net/bpf_jit_32.h
index ca9f1f5589f4..9d605e7a8309 100644
--- a/arch/arm/net/bpf_jit_32.h
+++ b/arch/arm/net/bpf_jit_32.h
@@ -190,6 +190,10 @@
#define ARM_LDR_R(rt, rn, rm) (ARM_INST_LDR_R | ARM_INST_LDST__U \
| (rt) << 12 | (rn) << 16 \
| (rm))
+#define ARM_LDR_R_SI(rt, rn, rm, type, imm) \
+ (ARM_INST_LDR_R | ARM_INST_LDST__U \
+ | (rt) << 12 | (rn) << 16 \
+ | (imm) << 7 | (type) << 5 | (rm))
#define ARM_LDRB_R(rt, rn, rm) (ARM_INST_LDRB_R | ARM_INST_LDST__U \
| (rt) << 12 | (rn) << 16 \
| (rm))
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 08/13] ARM: net: bpf: use immediate forms of instructions where possible
From: Russell King @ 2018-07-10 12:36 UTC (permalink / raw)
To: netdev, linux-arm-kernel; +Cc: Daniel Borkmann
In-Reply-To: <20180710123340.GK17271@n2100.armlinux.org.uk>
Rather than moving constants to a register and then using them in a
subsequent instruction, use them directly in the desired instruction
cutting out the "middle" register. This removes two instructions from
the tail call code path.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
arch/arm/net/bpf_jit_32.c | 16 +++++++++-------
arch/arm/net/bpf_jit_32.h | 3 +++
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index 753b5b2b2e3d..c7591877c350 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -256,7 +256,7 @@ static u32 arm_bpf_ldst_imm12(u32 op, u8 rt, u8 rn, s16 imm12)
op |= ARM_INST_LDST__U;
else
imm12 = -imm12;
- return op | (imm12 & 0xfff);
+ return op | (imm12 & ARM_INST_LDST__IMM12);
}
static u32 arm_bpf_ldst_imm8(u32 op, u8 rt, u8 rn, s16 imm8)
@@ -1012,11 +1012,12 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
/* if (index >= array->map.max_entries)
* goto out;
*/
+ BUILD_BUG_ON(offsetof(struct bpf_array, map.max_entries) >
+ ARM_INST_LDST__IMM12);
off = offsetof(struct bpf_array, map.max_entries);
/* array->map.max_entries */
- emit_a32_mov_i(tmp[1], off, ctx);
r_array = arm_bpf_get_reg32(r2[1], tmp2[1], ctx);
- emit(ARM_LDR_R(tmp[1], r_array, tmp[1]), ctx);
+ emit(ARM_LDR_I(tmp[1], r_array, off), ctx);
/* index is 32-bit for arrays */
r_index = arm_bpf_get_reg32(r3[1], tmp2[1], ctx);
/* index >= array->map.max_entries */
@@ -1041,10 +1042,10 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
* if (prog == NULL)
* goto out;
*/
+ BUILD_BUG_ON(offsetof(struct bpf_array, ptrs) > ARM_ALU_IMM);
off = offsetof(struct bpf_array, ptrs);
- emit_a32_mov_i(tmp[1], off, ctx);
r_array = arm_bpf_get_reg32(r2[1], tmp2[1], ctx);
- emit(ARM_ADD_R(tmp[1], r_array, tmp[1]), ctx);
+ emit(ARM_ADD_I(tmp[1], r_array, off), ctx);
r_index = arm_bpf_get_reg32(r3[1], tmp2[1], ctx);
emit(ARM_MOV_SI(tmp[0], r_index, SRTYPE_ASL, 2), ctx);
emit(ARM_LDR_R(tmp[1], tmp[1], tmp[0]), ctx);
@@ -1052,9 +1053,10 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
_emit(ARM_COND_EQ, ARM_B(jmp_offset), ctx);
/* goto *(prog->bpf_func + prologue_size); */
+ BUILD_BUG_ON(offsetof(struct bpf_prog, bpf_func) >
+ ARM_INST_LDST__IMM12);
off = offsetof(struct bpf_prog, bpf_func);
- emit_a32_mov_i(tmp2[1], off, ctx);
- emit(ARM_LDR_R(tmp[1], tmp[1], tmp2[1]), ctx);
+ emit(ARM_LDR_I(tmp[1], tmp[1], off), ctx);
emit(ARM_ADD_I(tmp[1], tmp[1], ctx->prologue_bytes), ctx);
emit_bx_r(tmp[1], ctx);
diff --git a/arch/arm/net/bpf_jit_32.h b/arch/arm/net/bpf_jit_32.h
index c55bc39d3e22..ca9f1f5589f4 100644
--- a/arch/arm/net/bpf_jit_32.h
+++ b/arch/arm/net/bpf_jit_32.h
@@ -78,6 +78,7 @@
#define ARM_INST_EOR_I 0x02200000
#define ARM_INST_LDST__U 0x00800000
+#define ARM_INST_LDST__IMM12 0x00000fff
#define ARM_INST_LDRB_I 0x05500000
#define ARM_INST_LDRB_R 0x07d00000
#define ARM_INST_LDRH_I 0x015000b0
@@ -154,6 +155,8 @@
*/
#define ARM_INST_UDF 0xe7fddef1
+#define ARM_ALU_IMM 0xff
+
/* register */
#define _AL3_R(op, rd, rn, rm) ((op ## _R) | (rd) << 12 | (rn) << 16 | (rm))
/* immediate */
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox