* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: David Ahern @ 2018-11-09 16:21 UTC (permalink / raw)
To: Paweł Staszewski, Jesper Dangaard Brouer; +Cc: netdev, Yoel Caspersen
In-Reply-To: <77027868-2b96-8c1d-f485-c7b36c6d9fa9@itcare.pl>
On 11/9/18 3:20 AM, Paweł Staszewski wrote:
>
> I just catch some weird behavior :)
> All was working fine for about 20k packets
>
> Then after xdp start to forward every 10 packets
Interesting. Any counter showing drops?
> ping 172.16.0.2 -i 0.1
> PING 172.16.0.2 (172.16.0.2) 56(84) bytes of data.
> 64 bytes from 172.16.0.2: icmp_seq=1 ttl=64 time=5.12 ms
> 64 bytes from 172.16.0.2: icmp_seq=9 ttl=64 time=5.20 ms
> 64 bytes from 172.16.0.2: icmp_seq=19 ttl=64 time=4.85 ms
> 64 bytes from 172.16.0.2: icmp_seq=29 ttl=64 time=4.91 ms
> 64 bytes from 172.16.0.2: icmp_seq=38 ttl=64 time=4.85 ms
> 64 bytes from 172.16.0.2: icmp_seq=48 ttl=64 time=5.00 ms
> ^C
> --- 172.16.0.2 ping statistics ---
> 55 packets transmitted, 6 received, 89% packet loss, time 5655ms
> rtt min/avg/max/mdev = 4.850/4.992/5.203/0.145 ms
>
>
> And again after some time back to normal
>
> ping 172.16.0.2 -i 0.1
> PING 172.16.0.2 (172.16.0.2) 56(84) bytes of data.
> 64 bytes from 172.16.0.2: icmp_seq=1 ttl=64 time=5.02 ms
> 64 bytes from 172.16.0.2: icmp_seq=2 ttl=64 time=5.06 ms
> 64 bytes from 172.16.0.2: icmp_seq=3 ttl=64 time=5.19 ms
> 64 bytes from 172.16.0.2: icmp_seq=4 ttl=64 time=5.07 ms
> 64 bytes from 172.16.0.2: icmp_seq=5 ttl=64 time=5.08 ms
> 64 bytes from 172.16.0.2: icmp_seq=6 ttl=64 time=5.14 ms
> 64 bytes from 172.16.0.2: icmp_seq=7 ttl=64 time=5.08 ms
> 64 bytes from 172.16.0.2: icmp_seq=8 ttl=64 time=5.17 ms
> 64 bytes from 172.16.0.2: icmp_seq=9 ttl=64 time=5.04 ms
> 64 bytes from 172.16.0.2: icmp_seq=10 ttl=64 time=5.10 ms
> 64 bytes from 172.16.0.2: icmp_seq=11 ttl=64 time=5.11 ms
> 64 bytes from 172.16.0.2: icmp_seq=12 ttl=64 time=5.13 ms
> 64 bytes from 172.16.0.2: icmp_seq=13 ttl=64 time=5.12 ms
> 64 bytes from 172.16.0.2: icmp_seq=14 ttl=64 time=5.15 ms
> 64 bytes from 172.16.0.2: icmp_seq=15 ttl=64 time=5.13 ms
> 64 bytes from 172.16.0.2: icmp_seq=16 ttl=64 time=5.04 ms
> 64 bytes from 172.16.0.2: icmp_seq=17 ttl=64 time=5.12 ms
> 64 bytes from 172.16.0.2: icmp_seq=18 ttl=64 time=5.07 ms
> 64 bytes from 172.16.0.2: icmp_seq=19 ttl=64 time=5.06 ms
> 64 bytes from 172.16.0.2: icmp_seq=20 ttl=64 time=5.12 ms
> 64 bytes from 172.16.0.2: icmp_seq=21 ttl=64 time=5.21 ms
> 64 bytes from 172.16.0.2: icmp_seq=22 ttl=64 time=4.98 ms
> ^C
> --- 172.16.0.2 ping statistics ---
> 22 packets transmitted, 22 received, 0% packet loss, time 2105ms
> rtt min/avg/max/mdev = 4.988/5.104/5.210/0.089 ms
>
>
> I will try to catch this with debug enabled
>
>
>
>
>
> Wondering also - cause xdp will bypass now vlan counters and other stuff
> like tcpdump
yes, xdp is before tcpdump based sockets.
And the counters (vlan just being the current example) is another
problem to be solved. The vlan net_device never sees the packet and you
can not arbitrarily bump the counters just because the device lookups
reference them.
>
> Is there possible to add only counters from xdp for vlans ?
> This will help me in testing.
I will take a look today at adding counters that you can dump using
bpftool. It will be a temporary solution for this xdp program only.
>
>
> And also - for non lab scenario there should be possible to sniff
> sometimes on interface :)
Yes, sampling is another problem.
> Soo wondering if need to attack another xdp program to interface or all
> this can be done by one
>
> I think this is time where i will need to learn more about xdp :)
>
>
^ permalink raw reply
* [PATCH v5 bpf-next 0/7] bpftool: support loading flow dissector
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
To: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
v5 changes:
* FILE -> PATH for load/loadall (can be either file or directory now)
* simpler implementation for __bpf_program__pin_name
* removed p_err for REQ_ARGS checks
* parse_atach_detach_args -> parse_attach_detach_args
* for -> while in bpf_object__pin_{programs,maps} recovery
v4 changes:
* addressed another round of comments/style issues from Jakub Kicinski &
Quentin Monnet (thanks!)
* implemented bpf_object__pin_maps and bpf_object__pin_programs helpers and
used them in bpf_program__pin
* added new pin_name to bpf_program so bpf_program__pin
works with sections that contain '/'
* moved *loadall* command implementation into a separate patch
* added patch that implements *pinmaps* to pin maps when doing
load/loadall
v3 changes:
* (maybe) better cleanup for partial failure in bpf_object__pin
* added special case in bpf_program__pin for programs with single
instances
v2 changes:
* addressed comments/style issues from Jakub Kicinski & Quentin Monnet
* removed logic that populates jump table
* added cleanup for partial failure in bpf_object__pin
This patch series adds support for loading and attaching flow dissector
programs from the bpftool:
* first patch fixes flow dissector section name in the selftests (so
libbpf auto-detection works)
* second patch adds proper cleanup to bpf_object__pin, parts of which are now
being used to attach all flow dissector progs/maps
* third patch adds special case in bpf_program__pin for programs with
single instances (we don't create <prog>/0 pin anymore, just <prog>)
* forth patch adds pin_name to the bpf_program struct
which is now used as a pin name in bpf_program__pin et al
* fifth patch adds *loadall* command that pins all programs, not just
the first one
* sixth patch adds *pinmaps* argument to load/loadall to let users pin
all maps of the obj file
* seventh patch adds actual flow_dissector support to the bpftool and
an example
Stanislav Fomichev (7):
selftests/bpf: rename flow dissector section to flow_dissector
libbpf: cleanup after partial failure in bpf_object__pin
libbpf: bpf_program__pin: add special case for instances.nr == 1
libbpf: add internal pin_name
bpftool: add loadall command
bpftool: add pinmaps argument to the load/loadall
bpftool: support loading flow dissector
.../bpftool/Documentation/bpftool-prog.rst | 42 +-
tools/bpf/bpftool/bash-completion/bpftool | 21 +-
tools/bpf/bpftool/common.c | 31 +-
tools/bpf/bpftool/main.h | 1 +
tools/bpf/bpftool/prog.c | 183 ++++++---
tools/lib/bpf/libbpf.c | 359 ++++++++++++++++--
tools/lib/bpf/libbpf.h | 18 +
tools/testing/selftests/bpf/bpf_flow.c | 2 +-
.../selftests/bpf/test_flow_dissector.sh | 2 +-
9 files changed, 537 insertions(+), 122 deletions(-)
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply
* [PATCH v5 bpf-next 1/7] selftests/bpf: rename flow dissector section to flow_dissector
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
To: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
In-Reply-To: <20181109162146.78019-1-sdf@google.com>
Makes it compatible with the logic that derives program type
from section name in libbpf_prog_type_by_name.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/bpf_flow.c | 2 +-
tools/testing/selftests/bpf/test_flow_dissector.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_flow.c b/tools/testing/selftests/bpf/bpf_flow.c
index 107350a7821d..b9798f558ca7 100644
--- a/tools/testing/selftests/bpf/bpf_flow.c
+++ b/tools/testing/selftests/bpf/bpf_flow.c
@@ -116,7 +116,7 @@ static __always_inline int parse_eth_proto(struct __sk_buff *skb, __be16 proto)
return BPF_DROP;
}
-SEC("dissect")
+SEC("flow_dissector")
int _dissect(struct __sk_buff *skb)
{
if (!skb->vlan_present)
diff --git a/tools/testing/selftests/bpf/test_flow_dissector.sh b/tools/testing/selftests/bpf/test_flow_dissector.sh
index c0fb073b5eab..d23d4da66b83 100755
--- a/tools/testing/selftests/bpf/test_flow_dissector.sh
+++ b/tools/testing/selftests/bpf/test_flow_dissector.sh
@@ -59,7 +59,7 @@ else
fi
# Attach BPF program
-./flow_dissector_load -p bpf_flow.o -s dissect
+./flow_dissector_load -p bpf_flow.o -s flow_dissector
# Setup
tc qdisc add dev lo ingress
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related
* [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
To: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
In-Reply-To: <20181109162146.78019-1-sdf@google.com>
bpftool will use bpf_object__pin in the next commits to pin all programs
and maps from the file; in case of a partial failure, we need to get
back to the clean state (undo previous program/map pins).
As part of a cleanup, I've added and exported separate routines to
pin all maps (bpf_object__pin_maps) and progs (bpf_object__pin_programs)
of an object.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/lib/bpf/libbpf.c | 324 ++++++++++++++++++++++++++++++++++++++---
tools/lib/bpf/libbpf.h | 18 +++
2 files changed, 319 insertions(+), 23 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index d6e62e90e8d4..341008f47c8a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1699,6 +1699,34 @@ int bpf_program__pin_instance(struct bpf_program *prog, const char *path,
return 0;
}
+int bpf_program__unpin_instance(struct bpf_program *prog, const char *path,
+ int instance)
+{
+ int err;
+
+ err = check_path(path);
+ if (err)
+ return err;
+
+ if (prog == NULL) {
+ pr_warning("invalid program pointer\n");
+ return -EINVAL;
+ }
+
+ if (instance < 0 || instance >= prog->instances.nr) {
+ pr_warning("invalid prog instance %d of prog %s (max %d)\n",
+ instance, prog->section_name, prog->instances.nr);
+ return -EINVAL;
+ }
+
+ err = unlink(path);
+ if (err != 0)
+ return -errno;
+ pr_debug("unpinned program '%s'\n", path);
+
+ return 0;
+}
+
static int make_dir(const char *path)
{
char *cp, errmsg[STRERR_BUFSIZE];
@@ -1737,6 +1765,64 @@ int bpf_program__pin(struct bpf_program *prog, const char *path)
if (err)
return err;
+ for (i = 0; i < prog->instances.nr; i++) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%d", path, i);
+ if (len < 0) {
+ err = -EINVAL;
+ goto err_unpin;
+ } else if (len >= PATH_MAX) {
+ err = -ENAMETOOLONG;
+ goto err_unpin;
+ }
+
+ err = bpf_program__pin_instance(prog, buf, i);
+ if (err)
+ goto err_unpin;
+ }
+
+ return 0;
+
+err_unpin:
+ for (i = i - 1; i >= 0; i--) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%d", path, i);
+ if (len < 0)
+ continue;
+ else if (len >= PATH_MAX)
+ continue;
+
+ bpf_program__unpin_instance(prog, buf, i);
+ }
+
+ rmdir(path);
+
+ return err;
+}
+
+int bpf_program__unpin(struct bpf_program *prog, const char *path)
+{
+ int i, err;
+
+ err = check_path(path);
+ if (err)
+ return err;
+
+ if (prog == NULL) {
+ pr_warning("invalid program pointer\n");
+ return -EINVAL;
+ }
+
+ if (prog->instances.nr <= 0) {
+ pr_warning("no instances of prog %s to pin\n",
+ prog->section_name);
+ return -EINVAL;
+ }
+
for (i = 0; i < prog->instances.nr; i++) {
char buf[PATH_MAX];
int len;
@@ -1747,11 +1833,15 @@ int bpf_program__pin(struct bpf_program *prog, const char *path)
else if (len >= PATH_MAX)
return -ENAMETOOLONG;
- err = bpf_program__pin_instance(prog, buf, i);
+ err = bpf_program__unpin_instance(prog, buf, i);
if (err)
return err;
}
+ err = rmdir(path);
+ if (err)
+ return -errno;
+
return 0;
}
@@ -1776,12 +1866,33 @@ int bpf_map__pin(struct bpf_map *map, const char *path)
}
pr_debug("pinned map '%s'\n", path);
+
return 0;
}
-int bpf_object__pin(struct bpf_object *obj, const char *path)
+int bpf_map__unpin(struct bpf_map *map, const char *path)
+{
+ int err;
+
+ err = check_path(path);
+ if (err)
+ return err;
+
+ if (map == NULL) {
+ pr_warning("invalid map pointer\n");
+ return -EINVAL;
+ }
+
+ err = unlink(path);
+ if (err != 0)
+ return -errno;
+ pr_debug("unpinned map '%s'\n", path);
+
+ return 0;
+}
+
+int bpf_object__pin_maps(struct bpf_object *obj, const char *path)
{
- struct bpf_program *prog;
struct bpf_map *map;
int err;
@@ -1797,6 +1908,53 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
if (err)
return err;
+ bpf_map__for_each(map, obj) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ bpf_map__name(map));
+ if (len < 0) {
+ err = -EINVAL;
+ goto err_unpin_maps;
+ } else if (len >= PATH_MAX) {
+ err = -ENAMETOOLONG;
+ goto err_unpin_maps;
+ }
+
+ err = bpf_map__pin(map, buf);
+ if (err)
+ goto err_unpin_maps;
+ }
+
+ return 0;
+
+err_unpin_maps:
+ while ((map = bpf_map__prev(map, obj))) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ bpf_map__name(map));
+ if (len < 0)
+ continue;
+ else if (len >= PATH_MAX)
+ continue;
+
+ bpf_map__unpin(map, buf);
+ }
+
+ return err;
+}
+
+int bpf_object__unpin_maps(struct bpf_object *obj, const char *path)
+{
+ struct bpf_map *map;
+ int err;
+
+ if (!obj)
+ return -ENOENT;
+
bpf_map__for_each(map, obj) {
char buf[PATH_MAX];
int len;
@@ -1808,11 +1966,78 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
else if (len >= PATH_MAX)
return -ENAMETOOLONG;
- err = bpf_map__pin(map, buf);
+ err = bpf_map__unpin(map, buf);
if (err)
return err;
}
+ return 0;
+}
+
+int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
+{
+ struct bpf_program *prog;
+ int err;
+
+ if (!obj)
+ return -ENOENT;
+
+ if (!obj->loaded) {
+ pr_warning("object not yet loaded; load it first\n");
+ return -ENOENT;
+ }
+
+ err = make_dir(path);
+ if (err)
+ return err;
+
+ bpf_object__for_each_program(prog, obj) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ prog->section_name);
+ if (len < 0) {
+ err = -EINVAL;
+ goto err_unpin_programs;
+ } else if (len >= PATH_MAX) {
+ err = -ENAMETOOLONG;
+ goto err_unpin_programs;
+ }
+
+ err = bpf_program__pin(prog, buf);
+ if (err)
+ goto err_unpin_programs;
+ }
+
+ return 0;
+
+err_unpin_programs:
+ while ((prog = bpf_program__prev(prog, obj))) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ prog->section_name);
+ if (len < 0)
+ continue;
+ else if (len >= PATH_MAX)
+ continue;
+
+ bpf_program__unpin(prog, buf);
+ }
+
+ return err;
+}
+
+int bpf_object__unpin_programs(struct bpf_object *obj, const char *path)
+{
+ struct bpf_program *prog;
+ int err;
+
+ if (!obj)
+ return -ENOENT;
+
bpf_object__for_each_program(prog, obj) {
char buf[PATH_MAX];
int len;
@@ -1824,7 +2049,7 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
else if (len >= PATH_MAX)
return -ENAMETOOLONG;
- err = bpf_program__pin(prog, buf);
+ err = bpf_program__unpin(prog, buf);
if (err)
return err;
}
@@ -1832,6 +2057,23 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
return 0;
}
+int bpf_object__pin(struct bpf_object *obj, const char *path)
+{
+ int err;
+
+ err = bpf_object__pin_maps(obj, path);
+ if (err)
+ return err;
+
+ err = bpf_object__pin_programs(obj, path);
+ if (err) {
+ bpf_object__unpin_maps(obj, path);
+ return err;
+ }
+
+ return 0;
+}
+
void bpf_object__close(struct bpf_object *obj)
{
size_t i;
@@ -1918,23 +2160,20 @@ void *bpf_object__priv(struct bpf_object *obj)
}
static struct bpf_program *
-__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
+__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
{
- size_t idx;
+ ssize_t idx;
if (!obj->programs)
return NULL;
- /* First handler */
- if (prev == NULL)
- return &obj->programs[0];
- if (prev->obj != obj) {
+ if (p->obj != obj) {
pr_warning("error: program handler doesn't match object\n");
return NULL;
}
- idx = (prev - obj->programs) + 1;
- if (idx >= obj->nr_programs)
+ idx = (p - obj->programs) + i;
+ if (idx >= obj->nr_programs || idx < 0)
return NULL;
return &obj->programs[idx];
}
@@ -1944,8 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
{
struct bpf_program *prog = prev;
+ if (prev == NULL)
+ return obj->programs;
+
do {
- prog = __bpf_program__next(prog, obj);
+ prog = __bpf_program__iter(prog, obj, 1);
+ } while (prog && bpf_program__is_function_storage(prog, obj));
+
+ return prog;
+}
+
+struct bpf_program *
+bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
+{
+ struct bpf_program *prog = next;
+
+ if (next == NULL) {
+ if (!obj->nr_programs)
+ return NULL;
+ return obj->programs + obj->nr_programs - 1;
+ }
+
+ do {
+ prog = __bpf_program__iter(prog, obj, -1);
} while (prog && bpf_program__is_function_storage(prog, obj));
return prog;
@@ -2272,10 +2532,10 @@ void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex)
map->map_ifindex = ifindex;
}
-struct bpf_map *
-bpf_map__next(struct bpf_map *prev, struct bpf_object *obj)
+static struct bpf_map *
+__bpf_map__iter(struct bpf_map *m, struct bpf_object *obj, int i)
{
- size_t idx;
+ ssize_t idx;
struct bpf_map *s, *e;
if (!obj || !obj->maps)
@@ -2284,21 +2544,39 @@ bpf_map__next(struct bpf_map *prev, struct bpf_object *obj)
s = obj->maps;
e = obj->maps + obj->nr_maps;
- if (prev == NULL)
- return s;
-
- if ((prev < s) || (prev >= e)) {
+ if ((m < s) || (m >= e)) {
pr_warning("error in %s: map handler doesn't belong to object\n",
__func__);
return NULL;
}
- idx = (prev - obj->maps) + 1;
- if (idx >= obj->nr_maps)
+ idx = (m - obj->maps) + i;
+ if (idx >= obj->nr_maps || idx < 0)
return NULL;
return &obj->maps[idx];
}
+struct bpf_map *
+bpf_map__next(struct bpf_map *prev, struct bpf_object *obj)
+{
+ if (prev == NULL)
+ return obj->maps;
+
+ return __bpf_map__iter(prev, obj, 1);
+}
+
+struct bpf_map *
+bpf_map__prev(struct bpf_map *next, struct bpf_object *obj)
+{
+ if (next == NULL) {
+ if (!obj->nr_maps)
+ return NULL;
+ return obj->maps + obj->nr_maps - 1;
+ }
+
+ return __bpf_map__iter(next, obj, -1);
+}
+
struct bpf_map *
bpf_object__find_map_by_name(struct bpf_object *obj, const char *name)
{
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 1f3468dad8b2..b1686a787102 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -71,6 +71,13 @@ struct bpf_object *__bpf_object__open_xattr(struct bpf_object_open_attr *attr,
LIBBPF_API struct bpf_object *bpf_object__open_buffer(void *obj_buf,
size_t obj_buf_sz,
const char *name);
+LIBBPF_API int bpf_object__pin_maps(struct bpf_object *obj, const char *path);
+LIBBPF_API int bpf_object__unpin_maps(struct bpf_object *obj,
+ const char *path);
+LIBBPF_API int bpf_object__pin_programs(struct bpf_object *obj,
+ const char *path);
+LIBBPF_API int bpf_object__unpin_programs(struct bpf_object *obj,
+ const char *path);
LIBBPF_API int bpf_object__pin(struct bpf_object *object, const char *path);
LIBBPF_API void bpf_object__close(struct bpf_object *object);
@@ -112,6 +119,9 @@ LIBBPF_API struct bpf_program *bpf_program__next(struct bpf_program *prog,
(pos) != NULL; \
(pos) = bpf_program__next((pos), (obj)))
+LIBBPF_API struct bpf_program *bpf_program__prev(struct bpf_program *prog,
+ struct bpf_object *obj);
+
typedef void (*bpf_program_clear_priv_t)(struct bpf_program *,
void *);
@@ -131,7 +141,11 @@ LIBBPF_API int bpf_program__fd(struct bpf_program *prog);
LIBBPF_API int bpf_program__pin_instance(struct bpf_program *prog,
const char *path,
int instance);
+LIBBPF_API int bpf_program__unpin_instance(struct bpf_program *prog,
+ const char *path,
+ int instance);
LIBBPF_API int bpf_program__pin(struct bpf_program *prog, const char *path);
+LIBBPF_API int bpf_program__unpin(struct bpf_program *prog, const char *path);
LIBBPF_API void bpf_program__unload(struct bpf_program *prog);
struct bpf_insn;
@@ -260,6 +274,9 @@ bpf_map__next(struct bpf_map *map, struct bpf_object *obj);
(pos) != NULL; \
(pos) = bpf_map__next((pos), (obj)))
+LIBBPF_API struct bpf_map *
+bpf_map__prev(struct bpf_map *map, struct bpf_object *obj);
+
LIBBPF_API int bpf_map__fd(struct bpf_map *map);
LIBBPF_API const struct bpf_map_def *bpf_map__def(struct bpf_map *map);
LIBBPF_API const char *bpf_map__name(struct bpf_map *map);
@@ -274,6 +291,7 @@ LIBBPF_API int bpf_map__reuse_fd(struct bpf_map *map, int fd);
LIBBPF_API bool bpf_map__is_offload_neutral(struct bpf_map *map);
LIBBPF_API void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex);
LIBBPF_API int bpf_map__pin(struct bpf_map *map, const char *path);
+LIBBPF_API int bpf_map__unpin(struct bpf_map *map, const char *path);
LIBBPF_API long libbpf_get_error(const void *ptr);
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related
* [PATCH v5 bpf-next 3/7] libbpf: bpf_program__pin: add special case for instances.nr == 1
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
To: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
In-Reply-To: <20181109162146.78019-1-sdf@google.com>
When bpf_program has only one instance, don't create a subdirectory with
per-instance pin files (<prog>/0). Instead, just create a single pin file
for that single instance. This simplifies object pinning by not creating
unnecessary subdirectories.
This can potentially break existing users that depend on the case
where '/0' is always created. However, I couldn't find any serious
usage of bpf_program__pin inside the kernel tree and I suppose there
should be none outside.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/lib/bpf/libbpf.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 341008f47c8a..97ce9f214002 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1761,6 +1761,11 @@ int bpf_program__pin(struct bpf_program *prog, const char *path)
return -EINVAL;
}
+ if (prog->instances.nr == 1) {
+ /* don't create subdirs when pinning single instance */
+ return bpf_program__pin_instance(prog, path, 0);
+ }
+
err = make_dir(path);
if (err)
return err;
@@ -1823,6 +1828,11 @@ int bpf_program__unpin(struct bpf_program *prog, const char *path)
return -EINVAL;
}
+ if (prog->instances.nr == 1) {
+ /* don't create subdirs when pinning single instance */
+ return bpf_program__unpin_instance(prog, path, 0);
+ }
+
for (i = 0; i < prog->instances.nr; i++) {
char buf[PATH_MAX];
int len;
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related
* [PATCH v5 bpf-next 4/7] libbpf: add internal pin_name
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
To: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
In-Reply-To: <20181109162146.78019-1-sdf@google.com>
pin_name is the same as section_name where '/' is replaced
by '_'. bpf_object__pin_programs is converted to use pin_name
to avoid the situation where section_name would require creating another
subdirectory for a pin (as, for example, when calling bpf_object__pin_programs
for programs in sections like "cgroup/connect6").
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/lib/bpf/libbpf.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 97ce9f214002..e827542ffa3a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -124,6 +124,10 @@ struct bpf_program {
char *name;
int prog_ifindex;
char *section_name;
+ /* section_name with / replaced by _; makes recursive pinning
+ * in bpf_object__pin_programs easier
+ */
+ char *pin_name;
struct bpf_insn *insns;
size_t insns_cnt, main_prog_cnt;
enum bpf_prog_type type;
@@ -253,6 +257,7 @@ static void bpf_program__exit(struct bpf_program *prog)
bpf_program__unload(prog);
zfree(&prog->name);
zfree(&prog->section_name);
+ zfree(&prog->pin_name);
zfree(&prog->insns);
zfree(&prog->reloc_desc);
@@ -261,6 +266,17 @@ static void bpf_program__exit(struct bpf_program *prog)
prog->idx = -1;
}
+static char *__bpf_program__pin_name(struct bpf_program *prog)
+{
+ char *name, *p;
+
+ name = p = strdup(prog->section_name);
+ while ((p = strchr(p, '/')))
+ *p = '_';
+
+ return name;
+}
+
static int
bpf_program__init(void *data, size_t size, char *section_name, int idx,
struct bpf_program *prog)
@@ -279,6 +295,13 @@ bpf_program__init(void *data, size_t size, char *section_name, int idx,
goto errout;
}
+ prog->pin_name = __bpf_program__pin_name(prog);
+ if (!prog->pin_name) {
+ pr_warning("failed to alloc pin name for prog under section(%d) %s\n",
+ idx, section_name);
+ goto errout;
+ }
+
prog->insns = malloc(size);
if (!prog->insns) {
pr_warning("failed to alloc insns for prog under section %s\n",
@@ -2006,7 +2029,7 @@ int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
int len;
len = snprintf(buf, PATH_MAX, "%s/%s", path,
- prog->section_name);
+ prog->pin_name);
if (len < 0) {
err = -EINVAL;
goto err_unpin_programs;
@@ -2028,7 +2051,7 @@ int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
int len;
len = snprintf(buf, PATH_MAX, "%s/%s", path,
- prog->section_name);
+ prog->pin_name);
if (len < 0)
continue;
else if (len >= PATH_MAX)
@@ -2053,7 +2076,7 @@ int bpf_object__unpin_programs(struct bpf_object *obj, const char *path)
int len;
len = snprintf(buf, PATH_MAX, "%s/%s", path,
- prog->section_name);
+ prog->pin_name);
if (len < 0)
return -EINVAL;
else if (len >= PATH_MAX)
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related
* [PATCH v5 bpf-next 5/7] bpftool: add loadall command
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
To: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
In-Reply-To: <20181109162146.78019-1-sdf@google.com>
This patch adds new *loadall* command which slightly differs from the
existing *load*. *load* command loads all programs from the obj file,
but pins only the first programs. *loadall* pins all programs from the
obj file under specified directory.
The intended usecase is flow_dissector, where we want to load a bunch
of progs, pin them all and after that construct a jump table.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 14 ++--
tools/bpf/bpftool/bash-completion/bpftool | 4 +-
tools/bpf/bpftool/common.c | 31 ++++----
tools/bpf/bpftool/main.h | 1 +
tools/bpf/bpftool/prog.c | 74 ++++++++++++++-----
5 files changed, 81 insertions(+), 43 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index ac4e904b10fb..984d125c507a 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -15,7 +15,8 @@ SYNOPSIS
*OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
*COMMANDS* :=
- { **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load** | **help** }
+ { **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load**
+ | **loadall** | **help** }
MAP COMMANDS
=============
@@ -24,7 +25,7 @@ MAP COMMANDS
| **bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes** | **visual**}]
| **bpftool** **prog dump jited** *PROG* [{**file** *FILE* | **opcodes**}]
| **bpftool** **prog pin** *PROG* *FILE*
-| **bpftool** **prog load** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+| **bpftool** **prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
| **bpftool** **prog help**
@@ -79,8 +80,11 @@ DESCRIPTION
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
- **bpftool prog load** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
- Load bpf program from binary *OBJ* and pin as *FILE*.
+ **bpftool prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+ Load bpf program(s) from binary *OBJ* and pin as *PATH*.
+ **bpftool prog load** pins only the first program from the
+ *OBJ* as *PATH*. **bpftool prog loadall** pins all programs
+ from the *OBJ* under *PATH* directory.
**type** is optional, if not specified program type will be
inferred from section names.
By default bpftool will create new maps as declared in the ELF
@@ -93,7 +97,7 @@ DESCRIPTION
If **dev** *NAME* is specified program will be loaded onto
given networking device (offload).
- Note: *FILE* must be located in *bpffs* mount. It must not
+ Note: *PATH* must be located in *bpffs* mount. It must not
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 3f78e6404589..780ebafb756a 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -243,7 +243,7 @@ _bpftool()
# Completion depends on object and command in use
case $object in
prog)
- if [[ $command != "load" ]]; then
+ if [[ $command != "load" && $command != "loadall" ]]; then
case $prev in
id)
_bpftool_get_prog_ids
@@ -309,7 +309,7 @@ _bpftool()
fi
return 0
;;
- load)
+ load|loadall)
local obj
if [[ ${#words[@]} -lt 6 ]]; then
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 25af85304ebe..21ce556c15e1 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -169,34 +169,23 @@ int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type)
return fd;
}
-int do_pin_fd(int fd, const char *name)
+int mount_bpffs_for_pin(const char *name)
{
char err_str[ERR_MAX_LEN];
char *file;
char *dir;
int err = 0;
- err = bpf_obj_pin(fd, name);
- if (!err)
- goto out;
-
file = malloc(strlen(name) + 1);
strcpy(file, name);
dir = dirname(file);
- if (errno != EPERM || is_bpffs(dir)) {
- p_err("can't pin the object (%s): %s", name, strerror(errno));
+ if (is_bpffs(dir))
+ /* nothing to do if already mounted */
goto out_free;
- }
- /* Attempt to mount bpffs, then retry pinning. */
err = mnt_bpffs(dir, err_str, ERR_MAX_LEN);
- if (!err) {
- err = bpf_obj_pin(fd, name);
- if (err)
- p_err("can't pin the object (%s): %s", name,
- strerror(errno));
- } else {
+ if (err) {
err_str[ERR_MAX_LEN - 1] = '\0';
p_err("can't mount BPF file system to pin the object (%s): %s",
name, err_str);
@@ -204,10 +193,20 @@ int do_pin_fd(int fd, const char *name)
out_free:
free(file);
-out:
return err;
}
+int do_pin_fd(int fd, const char *name)
+{
+ int err;
+
+ err = mount_bpffs_for_pin(name);
+ if (err)
+ return err;
+
+ return bpf_obj_pin(fd, name);
+}
+
int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
{
unsigned int id;
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 28322ace2856..1383824c9baf 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -129,6 +129,7 @@ const char *get_fd_type_name(enum bpf_obj_type type);
char *get_fdinfo(int fd, const char *key);
int open_obj_pinned(char *path);
int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type);
+int mount_bpffs_for_pin(const char *name);
int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32));
int do_pin_fd(int fd, const char *name);
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 5302ee282409..3abfee5533e6 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -792,15 +792,16 @@ static int do_detach(int argc, char **argv)
jsonw_null(json_wtr);
return 0;
}
-static int do_load(int argc, char **argv)
+
+static int load_with_options(int argc, char **argv, bool first_prog_only)
{
enum bpf_attach_type expected_attach_type;
struct bpf_object_open_attr attr = {
.prog_type = BPF_PROG_TYPE_UNSPEC,
};
struct map_replace *map_replace = NULL;
+ struct bpf_program *prog = NULL, *pos;
unsigned int old_map_fds = 0;
- struct bpf_program *prog;
struct bpf_object *obj;
struct bpf_map *map;
const char *pinfile;
@@ -918,26 +919,25 @@ static int do_load(int argc, char **argv)
goto err_free_reuse_maps;
}
- prog = bpf_program__next(NULL, obj);
- if (!prog) {
- p_err("object file doesn't contain any bpf program");
- goto err_close_obj;
- }
+ bpf_object__for_each_program(pos, obj) {
+ enum bpf_prog_type prog_type = attr.prog_type;
- bpf_program__set_ifindex(prog, ifindex);
- if (attr.prog_type == BPF_PROG_TYPE_UNSPEC) {
- const char *sec_name = bpf_program__title(prog, false);
+ if (attr.prog_type == BPF_PROG_TYPE_UNSPEC) {
+ const char *sec_name = bpf_program__title(pos, false);
- err = libbpf_prog_type_by_name(sec_name, &attr.prog_type,
- &expected_attach_type);
- if (err < 0) {
- p_err("failed to guess program type based on section name %s\n",
- sec_name);
- goto err_close_obj;
+ err = libbpf_prog_type_by_name(sec_name, &prog_type,
+ &expected_attach_type);
+ if (err < 0) {
+ p_err("failed to guess program type based on section name %s\n",
+ sec_name);
+ goto err_close_obj;
+ }
}
+
+ bpf_program__set_ifindex(pos, ifindex);
+ bpf_program__set_type(pos, prog_type);
+ bpf_program__set_expected_attach_type(pos, expected_attach_type);
}
- bpf_program__set_type(prog, attr.prog_type);
- bpf_program__set_expected_attach_type(prog, expected_attach_type);
qsort(map_replace, old_map_fds, sizeof(*map_replace),
map_replace_compar);
@@ -1001,9 +1001,31 @@ static int do_load(int argc, char **argv)
goto err_close_obj;
}
- if (do_pin_fd(bpf_program__fd(prog), pinfile))
+ err = mount_bpffs_for_pin(pinfile);
+ if (err)
goto err_close_obj;
+ if (first_prog_only) {
+ prog = bpf_program__next(NULL, obj);
+ if (!prog) {
+ p_err("object file doesn't contain any bpf program");
+ goto err_close_obj;
+ }
+
+ err = bpf_obj_pin(bpf_program__fd(prog), pinfile);
+ if (err) {
+ p_err("failed to pin program %s",
+ bpf_program__title(prog, false));
+ goto err_close_obj;
+ }
+ } else {
+ err = bpf_object__pin_programs(obj, pinfile);
+ if (err) {
+ p_err("failed to pin all programs");
+ goto err_close_obj;
+ }
+ }
+
if (json_output)
jsonw_null(json_wtr);
@@ -1023,6 +1045,16 @@ static int do_load(int argc, char **argv)
return -1;
}
+static int do_load(int argc, char **argv)
+{
+ return load_with_options(argc, argv, true);
+}
+
+static int do_loadall(int argc, char **argv)
+{
+ return load_with_options(argc, argv, false);
+}
+
static int do_help(int argc, char **argv)
{
if (json_output) {
@@ -1035,7 +1067,8 @@ static int do_help(int argc, char **argv)
" %s %s dump xlated PROG [{ file FILE | opcodes | visual }]\n"
" %s %s dump jited PROG [{ file FILE | opcodes }]\n"
" %s %s pin PROG FILE\n"
- " %s %s load OBJ FILE [type TYPE] [dev NAME] \\\n"
+ " %s %s { load | loadall } OBJ PATH \\\n"
+ " [type TYPE] [dev NAME] \\\n"
" [map { idx IDX | name NAME } MAP]\n"
" %s %s attach PROG ATTACH_TYPE MAP\n"
" %s %s detach PROG ATTACH_TYPE MAP\n"
@@ -1067,6 +1100,7 @@ static const struct cmd cmds[] = {
{ "dump", do_dump },
{ "pin", do_pin },
{ "load", do_load },
+ { "loadall", do_loadall },
{ "attach", do_attach },
{ "detach", do_detach },
{ 0 }
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related
* [PATCH v5 bpf-next 6/7] bpftool: add pinmaps argument to the load/loadall
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
To: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
In-Reply-To: <20181109162146.78019-1-sdf@google.com>
This new additional argument lets users pin all maps from the object at
specified path.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 4 +++-
tools/bpf/bpftool/bash-completion/bpftool | 3 ++-
tools/bpf/bpftool/prog.c | 24 ++++++++++++++++++-
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 984d125c507a..15e9172f7e55 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -80,7 +80,7 @@ DESCRIPTION
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
- **bpftool prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+ **bpftool prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*] [**pinmaps** *MAP_DIR*]
Load bpf program(s) from binary *OBJ* and pin as *PATH*.
**bpftool prog load** pins only the first program from the
*OBJ* as *PATH*. **bpftool prog loadall** pins all programs
@@ -96,6 +96,8 @@ DESCRIPTION
use, referring to it by **id** or through a **pinned** file.
If **dev** *NAME* is specified program will be loaded onto
given networking device (offload).
+ Optional **pinmaps** argument can be provided to pin all
+ maps under *MAP_DIR* directory.
Note: *PATH* must be located in *bpffs* mount. It must not
contain a dot character ('.'), which is reserved for future
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 780ebafb756a..a05d0071f39f 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -346,7 +346,7 @@ _bpftool()
_bpftool_get_map_ids
return 0
;;
- pinned)
+ pinned|pinmaps)
_filedir
return 0
;;
@@ -358,6 +358,7 @@ _bpftool()
COMPREPLY=( $( compgen -W "map" -- "$cur" ) )
_bpftool_once_attr 'type'
_bpftool_once_attr 'dev'
+ _bpftool_once_attr 'pinmaps'
return 0
;;
esac
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 3abfee5533e6..4bd6ad19fc05 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -802,6 +802,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
struct map_replace *map_replace = NULL;
struct bpf_program *prog = NULL, *pos;
unsigned int old_map_fds = 0;
+ const char *pinmaps = NULL;
struct bpf_object *obj;
struct bpf_map *map;
const char *pinfile;
@@ -906,6 +907,13 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
goto err_free_reuse_maps;
}
NEXT_ARG();
+ } else if (is_prefix(*argv, "pinmaps")) {
+ NEXT_ARG();
+
+ if (!REQ_ARGS(1))
+ goto err_free_reuse_maps;
+
+ pinmaps = GET_ARG();
} else {
p_err("expected no more arguments, 'type', 'map' or 'dev', got: '%s'?",
*argv);
@@ -1026,6 +1034,14 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
}
}
+ if (pinmaps) {
+ err = bpf_object__pin_maps(obj, pinmaps);
+ if (err) {
+ p_err("failed to pin all maps");
+ goto err_unpin;
+ }
+ }
+
if (json_output)
jsonw_null(json_wtr);
@@ -1036,6 +1052,11 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
return 0;
+err_unpin:
+ if (first_prog_only)
+ unlink(pinfile);
+ else
+ bpf_object__unpin_programs(obj, pinfile);
err_close_obj:
bpf_object__close(obj);
err_free_reuse_maps:
@@ -1069,7 +1090,8 @@ static int do_help(int argc, char **argv)
" %s %s pin PROG FILE\n"
" %s %s { load | loadall } OBJ PATH \\\n"
" [type TYPE] [dev NAME] \\\n"
- " [map { idx IDX | name NAME } MAP]\n"
+ " [map { idx IDX | name NAME } MAP]\\\n"
+ " [pinmaps MAP_DIR]\n"
" %s %s attach PROG ATTACH_TYPE MAP\n"
" %s %s detach PROG ATTACH_TYPE MAP\n"
" %s %s help\n"
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related
* [PATCH v5 bpf-next 7/7] bpftool: support loading flow dissector
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
To: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
In-Reply-To: <20181109162146.78019-1-sdf@google.com>
This commit adds support for loading/attaching/detaching flow
dissector program.
When `bpftool loadall` is called with a flow_dissector prog (i.e. when the
'type flow_dissector' argument is passed), we load and pin all programs.
User is responsible to construct the jump table for the tail calls.
The last argument of `bpftool attach` is made optional for this use
case.
Example:
bpftool prog load tools/testing/selftests/bpf/bpf_flow.o \
/sys/fs/bpf/flow type flow_dissector \
pinmaps /sys/fs/bpf/flow
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 0 0 0 0 \
value pinned /sys/fs/bpf/flow/IP
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 1 0 0 0 \
value pinned /sys/fs/bpf/flow/IPV6
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 2 0 0 0 \
value pinned /sys/fs/bpf/flow/IPV6OP
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 3 0 0 0 \
value pinned /sys/fs/bpf/flow/IPV6FR
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 4 0 0 0 \
value pinned /sys/fs/bpf/flow/MPLS
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 5 0 0 0 \
value pinned /sys/fs/bpf/flow/VLAN
bpftool prog attach pinned /sys/fs/bpf/flow/flow_dissector flow_dissector
Tested by using the above lines to load the prog in
the test_flow_dissector.sh selftest.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 26 +++---
tools/bpf/bpftool/bash-completion/bpftool | 14 ++-
tools/bpf/bpftool/prog.c | 85 ++++++++++---------
3 files changed, 74 insertions(+), 51 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 15e9172f7e55..8db78ed82a71 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -26,8 +26,8 @@ MAP COMMANDS
| **bpftool** **prog dump jited** *PROG* [{**file** *FILE* | **opcodes**}]
| **bpftool** **prog pin** *PROG* *FILE*
| **bpftool** **prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
-| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
-| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
+| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
+| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
| **bpftool** **prog help**
|
| *MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
@@ -40,7 +40,9 @@ MAP COMMANDS
| **cgroup/bind4** | **cgroup/bind6** | **cgroup/post_bind4** | **cgroup/post_bind6** |
| **cgroup/connect4** | **cgroup/connect6** | **cgroup/sendmsg4** | **cgroup/sendmsg6**
| }
-| *ATTACH_TYPE* := { **msg_verdict** | **skb_verdict** | **skb_parse** }
+| *ATTACH_TYPE* := {
+| **msg_verdict** | **skb_verdict** | **skb_parse** | **flow_dissector**
+| }
DESCRIPTION
@@ -103,13 +105,17 @@ DESCRIPTION
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
- **bpftool prog attach** *PROG* *ATTACH_TYPE* *MAP*
- Attach bpf program *PROG* (with type specified by *ATTACH_TYPE*)
- to the map *MAP*.
-
- **bpftool prog detach** *PROG* *ATTACH_TYPE* *MAP*
- Detach bpf program *PROG* (with type specified by *ATTACH_TYPE*)
- from the map *MAP*.
+ **bpftool prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
+ Attach bpf program *PROG* (with type specified by
+ *ATTACH_TYPE*). Most *ATTACH_TYPEs* require a *MAP*
+ parameter, with the exception of *flow_dissector* which is
+ attached to current networking name space.
+
+ **bpftool prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
+ Detach bpf program *PROG* (with type specified by
+ *ATTACH_TYPE*). Most *ATTACH_TYPEs* require a *MAP*
+ parameter, with the exception of *flow_dissector* which is
+ detached from the current networking name space.
**bpftool prog help**
Print short help message.
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index a05d0071f39f..45c2db257d2b 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -299,7 +299,8 @@ _bpftool()
fi
if [[ ${#words[@]} == 6 ]]; then
- COMPREPLY=( $( compgen -W "msg_verdict skb_verdict skb_parse" -- "$cur" ) )
+ COMPREPLY=( $( compgen -W "msg_verdict skb_verdict \
+ skb_parse flow_dissector" -- "$cur" ) )
return 0
fi
@@ -338,7 +339,16 @@ _bpftool()
case $prev in
type)
- COMPREPLY=( $( compgen -W "socket kprobe kretprobe classifier action tracepoint raw_tracepoint xdp perf_event cgroup/skb cgroup/sock cgroup/dev lwt_in lwt_out lwt_xmit lwt_seg6local sockops sk_skb sk_msg lirc_mode2 cgroup/bind4 cgroup/bind6 cgroup/connect4 cgroup/connect6 cgroup/sendmsg4 cgroup/sendmsg6 cgroup/post_bind4 cgroup/post_bind6" -- \
+ COMPREPLY=( $( compgen -W "socket kprobe \
+ kretprobe classifier flow_dissector \
+ action tracepoint raw_tracepoint \
+ xdp perf_event cgroup/skb cgroup/sock \
+ cgroup/dev lwt_in lwt_out lwt_xmit \
+ lwt_seg6local sockops sk_skb sk_msg \
+ lirc_mode2 cgroup/bind4 cgroup/bind6 \
+ cgroup/connect4 cgroup/connect6 \
+ cgroup/sendmsg4 cgroup/sendmsg6 \
+ cgroup/post_bind4 cgroup/post_bind6" -- \
"$cur" ) )
return 0
;;
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 4bd6ad19fc05..9adc56e0885c 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -81,6 +81,7 @@ static const char * const attach_type_strings[] = {
[BPF_SK_SKB_STREAM_PARSER] = "stream_parser",
[BPF_SK_SKB_STREAM_VERDICT] = "stream_verdict",
[BPF_SK_MSG_VERDICT] = "msg_verdict",
+ [BPF_FLOW_DISSECTOR] = "flow_dissector",
[__MAX_BPF_ATTACH_TYPE] = NULL,
};
@@ -721,30 +722,49 @@ int map_replace_compar(const void *p1, const void *p2)
return a->idx - b->idx;
}
-static int do_attach(int argc, char **argv)
+static int parse_attach_detach_args(int argc, char **argv, int *progfd,
+ enum bpf_attach_type *attach_type,
+ int *mapfd)
{
- enum bpf_attach_type attach_type;
- int err, mapfd, progfd;
-
- if (!REQ_ARGS(5)) {
- p_err("too few parameters for map attach");
+ if (!REQ_ARGS(3))
return -EINVAL;
- }
- progfd = prog_parse_fd(&argc, &argv);
- if (progfd < 0)
- return progfd;
+ *progfd = prog_parse_fd(&argc, &argv);
+ if (*progfd < 0)
+ return *progfd;
- attach_type = parse_attach_type(*argv);
- if (attach_type == __MAX_BPF_ATTACH_TYPE) {
- p_err("invalid attach type");
+ *attach_type = parse_attach_type(*argv);
+ if (*attach_type == __MAX_BPF_ATTACH_TYPE) {
+ p_err("invalid attach/detach type");
return -EINVAL;
}
+
+ if (*attach_type == BPF_FLOW_DISSECTOR) {
+ *mapfd = -1;
+ return 0;
+ }
+
NEXT_ARG();
+ if (!REQ_ARGS(2))
+ return -EINVAL;
+
+ *mapfd = map_parse_fd(&argc, &argv);
+ if (*mapfd < 0)
+ return *mapfd;
+
+ return 0;
+}
- mapfd = map_parse_fd(&argc, &argv);
- if (mapfd < 0)
- return mapfd;
+static int do_attach(int argc, char **argv)
+{
+ enum bpf_attach_type attach_type;
+ int err, progfd;
+ int mapfd;
+
+ err = parse_attach_detach_args(argc, argv,
+ &progfd, &attach_type, &mapfd);
+ if (err)
+ return err;
err = bpf_prog_attach(progfd, mapfd, attach_type, 0);
if (err) {
@@ -760,27 +780,13 @@ static int do_attach(int argc, char **argv)
static int do_detach(int argc, char **argv)
{
enum bpf_attach_type attach_type;
- int err, mapfd, progfd;
-
- if (!REQ_ARGS(5)) {
- p_err("too few parameters for map detach");
- return -EINVAL;
- }
-
- progfd = prog_parse_fd(&argc, &argv);
- if (progfd < 0)
- return progfd;
+ int err, progfd;
+ int mapfd;
- attach_type = parse_attach_type(*argv);
- if (attach_type == __MAX_BPF_ATTACH_TYPE) {
- p_err("invalid attach type");
- return -EINVAL;
- }
- NEXT_ARG();
-
- mapfd = map_parse_fd(&argc, &argv);
- if (mapfd < 0)
- return mapfd;
+ err = parse_attach_detach_args(argc, argv,
+ &progfd, &attach_type, &mapfd);
+ if (err)
+ return err;
err = bpf_prog_detach2(progfd, mapfd, attach_type);
if (err) {
@@ -1092,8 +1098,8 @@ static int do_help(int argc, char **argv)
" [type TYPE] [dev NAME] \\\n"
" [map { idx IDX | name NAME } MAP]\\\n"
" [pinmaps MAP_DIR]\n"
- " %s %s attach PROG ATTACH_TYPE MAP\n"
- " %s %s detach PROG ATTACH_TYPE MAP\n"
+ " %s %s attach PROG ATTACH_TYPE [MAP]\n"
+ " %s %s detach PROG ATTACH_TYPE [MAP]\n"
" %s %s help\n"
"\n"
" " HELP_SPEC_MAP "\n"
@@ -1105,7 +1111,8 @@ static int do_help(int argc, char **argv)
" cgroup/bind4 | cgroup/bind6 | cgroup/post_bind4 |\n"
" cgroup/post_bind6 | cgroup/connect4 | cgroup/connect6 |\n"
" cgroup/sendmsg4 | cgroup/sendmsg6 }\n"
- " ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse }\n"
+ " ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse |\n"
+ " flow_dissector }\n"
" " HELP_SPEC_OPTIONS "\n"
"",
bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related
* Re: Should the bridge learn from frames with link local destination MAC address?
From: Roopa Prabhu @ 2018-11-09 16:24 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Andrew Lunn, Nikolay Aleksandrov, netdev, Florian Fainelli
In-Reply-To: <20181109080008.022b93cb@xeon-e3>
On Fri, Nov 9, 2018 at 8:00 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Fri, 9 Nov 2018 04:24:43 +0100
> Andrew Lunn <andrew@lunn.ch> wrote:
>
> > Hi Roopa, Nikolay
> >
> > br_handle_frame() looks out for frames with a destination MAC
> > addresses with is Ethernet link local, those which fit
> > 01-80-C2-00-00-XX. It does not normally forward these, but it will
> > deliver them locally.
> >
> > Should the bridge perform learning on such frames?
> >
> > I've got a setup with two bridges connected together with multiple
> > links between them. STP has done its thing, and blocked one of the
> > ports to solve the loop.
> >
> > host0 host1
> > +-----------------+ +-----------------+
> > | lan0 forwarding |-----| lan0 forwarding |
> > | | | |
> > | lan1 forwarding |-----| lan1 blocked |
> > +-----------------+ +-----------------+
> >
> > I have LLDP running on both system, and they are sending out periodic
> > frames on each port.
> >
> > Now, lan0 and lan1 on host1 use the same MAC address. So i see the
> > MAC address bouncing between ports because of the LLDP packets.
> >
> > # bridge monitor
> > 00:26:55:d2:27:a8 dev lan1 master br0
> > 00:26:55:d2:27:a8 dev lan0 master br0
> > 00:26:55:d2:27:a8 dev lan1 master br0
> > 00:26:55:d2:27:a8 dev lan0 master br0
> > 00:26:55:d2:27:a8 dev lan1 master br0
> >
> > This then results in normal traffic from host0 to host1 being sent to
> > the blocked port for some of the time.
> >
> > LLDP is using 01-80-C2-00-00-0E, a link local MAC address. If the
> > bridge did not learn on such frames, i think this setup would
> > work. The bridge would learn from ARP, IP etc, coming from the
> > forwarding port of host1, and the blocked port would be ignored.
> >
> > I've tried a similar setup with a hardware switch, Marvell 6352. It
> > never seems to learn from such frames.
> >
> > Thanks
> > Andrew
>
> I agree with your analysis. A properly operating 802 compliant bridge
> should not learn link local addresses. But changing that in Linux bridge
> would probably break some users. There is already a hack to forward link
> local frames. There are many usages of Linux vswitch where this behavior
> might be a problem:
> 1. a container or VM hub
> 2. bump in the wire filter
> 3. L2 nat etc.
>
> So what ever you decide it has to be optional and unfortunately default
> to off.
>
Andrew, I agree with your analysis also. We have hit this problem too
(and we have an internal bug tracking it).
We have not acted on this so far because of the fear of breaking
existing deployments. I am all for fixing this if there is a
clean way.
^ permalink raw reply
* Re: [PATCH mlx5-next 08/10] IB/mlx5: Call PAGE_FAULT_RESUME command asynchronously
From: Leon Romanovsky @ 2018-11-09 16:26 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Doug Ledford, RDMA mailing list, Artemy Kovalyov, Majd Dibbiny,
Moni Shoua, Saeed Mahameed, linux-netdev
In-Reply-To: <20181108194857.GF5548@mellanox.com>
[-- Attachment #1: Type: text/plain, Size: 3018 bytes --]
On Thu, Nov 08, 2018 at 07:49:03PM +0000, Jason Gunthorpe wrote:
> On Thu, Nov 08, 2018 at 09:10:15PM +0200, Leon Romanovsky wrote:
> > From: Moni Shoua <monis@mellanox.com>
> >
> > Telling the HCA that page fault handling is done and QP can resume
> > its flow is done in the context of the page fault handler. This blocks
> > the handling of the next work in queue without a need.
> > Call the PAGE_FAULT_RESUME command in an asynchronous manner and free
> > the workqueue to pick the next work item for handling. All tasks that
> > were executed after PAGE_FAULT_RESUME need to be done now
> > in the callback of the asynchronous command mechanism.
> >
> > Signed-off-by: Moni Shoua <monis@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > drivers/infiniband/hw/mlx5/odp.c | 110 +++++++++++++++++++++++++------
> > include/linux/mlx5/driver.h | 3 +
> > 2 files changed, 94 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/mlx5/odp.c b/drivers/infiniband/hw/mlx5/odp.c
> > index abce55b8b9ba..0c4f469cdd5b 100644
> > +++ b/drivers/infiniband/hw/mlx5/odp.c
> > @@ -298,20 +298,78 @@ void mlx5_ib_internal_fill_odp_caps(struct mlx5_ib_dev *dev)
> > return;
> > }
> >
> > +struct pfault_resume_cb_ctx {
> > + struct mlx5_ib_dev *dev;
> > + struct mlx5_core_rsc_common *res;
> > + struct mlx5_pagefault *pfault;
> > +};
> > +
> > +static void page_fault_resume_callback(int status, void *context)
> > +{
> > + struct pfault_resume_cb_ctx *ctx = context;
> > + struct mlx5_pagefault *pfault = ctx->pfault;
> > +
> > + if (status)
> > + mlx5_ib_err(ctx->dev, "Resolve the page fault failed with status %d\n",
> > + status);
> > +
> > + if (ctx->res)
> > + mlx5_core_res_put(ctx->res);
> > + kfree(pfault);
> > + kfree(ctx);
> > +}
> > +
> > static void mlx5_ib_page_fault_resume(struct mlx5_ib_dev *dev,
> > + struct mlx5_core_rsc_common *res,
> > struct mlx5_pagefault *pfault,
> > - int error)
> > + int error,
> > + bool async)
> > {
> > + int ret = 0;
> > + u32 *out = pfault->out_pf_resume;
> > + u32 *in = pfault->in_pf_resume;
> > + u32 token = pfault->token;
> > int wq_num = pfault->event_subtype == MLX5_PFAULT_SUBTYPE_WQE ?
> > - pfault->wqe.wq_num : pfault->token;
> > - int ret = mlx5_core_page_fault_resume(dev->mdev,
> > - pfault->token,
> > - wq_num,
> > - pfault->type,
> > - error);
> > - if (ret)
> > - mlx5_ib_err(dev, "Failed to resolve the page fault on WQ 0x%x\n",
> > - wq_num);
> > + pfault->wqe.wq_num : pfault->token;
> > + u8 type = pfault->type;
> > + struct pfault_resume_cb_ctx *ctx = NULL;
> > +
> > + if (async)
> > + ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
>
> Why not allocate this ctx ast part of the mlx5_pagefault and avoid
> this allocation failure strategy?
It is another way to implement it, both of them are correct.
Can I assume that we can progress with patches except patch #2?
Thanks
>
> Jason
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* [PATCH iproute2] testsuite: correctly use CC macros for generate_nlmsg
From: Luca Boccassi @ 2018-11-09 16:31 UTC (permalink / raw)
To: netdev; +Cc: stephen
It's $(QUIET_CC)$(CC) not $(QUIET_CC), copy-paste error. CI does
verbose build so it slipped through.
Fixes: 6e7d347aabbb ("testsuite: build generate_nlmsg with QUIET_CC")
Signed-off-by: Luca Boccassi <bluca@debian.org>
---
testsuite/tools/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/testsuite/tools/Makefile b/testsuite/tools/Makefile
index 85df69ec..344e58b0 100644
--- a/testsuite/tools/Makefile
+++ b/testsuite/tools/Makefile
@@ -3,7 +3,7 @@ CFLAGS=
include ../../config.mk
generate_nlmsg: generate_nlmsg.c ../../lib/libnetlink.c
- $(QUIET_CC) $(CPPFLAGS) $(CFLAGS) $(EXTRA_CFLAGS) -I../../include -include../../include/uapi/linux/netlink.h -o $@ $^ -lmnl
+ $(QUIET_CC)$(CC) $(CPPFLAGS) $(CFLAGS) $(EXTRA_CFLAGS) -I../../include -include../../include/uapi/linux/netlink.h -o $@ $^ -lmnl
clean:
rm -f generate_nlmsg
--
2.19.1
^ permalink raw reply related
* Re: Should the bridge learn from frames with link local destination MAC address?
From: nikolay @ 2018-11-09 16:44 UTC (permalink / raw)
To: Roopa Prabhu, Stephen Hemminger; +Cc: Andrew Lunn, netdev, Florian Fainelli
In-Reply-To: <CAJieiUj5d=uYNRKN_HKF9CaJyWA3ZEHR87MWNLFNtk_1OyUaRA@mail.gmail.com>
On 9 November 2018 18:24:18 EET, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>On Fri, Nov 9, 2018 at 8:00 AM Stephen Hemminger
><stephen@networkplumber.org> wrote:
>>
>> On Fri, 9 Nov 2018 04:24:43 +0100
>> Andrew Lunn <andrew@lunn.ch> wrote:
>>
>> > Hi Roopa, Nikolay
>> >
>> > br_handle_frame() looks out for frames with a destination MAC
>> > addresses with is Ethernet link local, those which fit
>> > 01-80-C2-00-00-XX. It does not normally forward these, but it will
>> > deliver them locally.
>> >
>> > Should the bridge perform learning on such frames?
>> >
>> > I've got a setup with two bridges connected together with multiple
>> > links between them. STP has done its thing, and blocked one of the
>> > ports to solve the loop.
>> >
>> > host0 host1
>> > +-----------------+ +-----------------+
>> > | lan0 forwarding |-----| lan0 forwarding |
>> > | | | |
>> > | lan1 forwarding |-----| lan1 blocked |
>> > +-----------------+ +-----------------+
>> >
>> > I have LLDP running on both system, and they are sending out
>periodic
>> > frames on each port.
>> >
>> > Now, lan0 and lan1 on host1 use the same MAC address. So i see the
>> > MAC address bouncing between ports because of the LLDP packets.
>> >
>> > # bridge monitor
>> > 00:26:55:d2:27:a8 dev lan1 master br0
>> > 00:26:55:d2:27:a8 dev lan0 master br0
>> > 00:26:55:d2:27:a8 dev lan1 master br0
>> > 00:26:55:d2:27:a8 dev lan0 master br0
>> > 00:26:55:d2:27:a8 dev lan1 master br0
>> >
>> > This then results in normal traffic from host0 to host1 being sent
>to
>> > the blocked port for some of the time.
>> >
>> > LLDP is using 01-80-C2-00-00-0E, a link local MAC address. If the
>> > bridge did not learn on such frames, i think this setup would
>> > work. The bridge would learn from ARP, IP etc, coming from the
>> > forwarding port of host1, and the blocked port would be ignored.
>> >
>> > I've tried a similar setup with a hardware switch, Marvell 6352. It
>> > never seems to learn from such frames.
>> >
>> > Thanks
>> > Andrew
>>
>> I agree with your analysis. A properly operating 802 compliant bridge
>> should not learn link local addresses. But changing that in Linux
>bridge
>> would probably break some users. There is already a hack to forward
>link
>> local frames. There are many usages of Linux vswitch where this
>behavior
>> might be a problem:
>> 1. a container or VM hub
>> 2. bump in the wire filter
>> 3. L2 nat etc.
>>
>> So what ever you decide it has to be optional and unfortunately
>default
>> to off.
>>
>
>Andrew, I agree with your analysis also. We have hit this problem too
>(and we have an internal bug tracking it).
>We have not acted on this so far because of the fear of breaking
>existing deployments. I am all for fixing this if there is a
>clean way.
+1 and since this would be a new bridge boolean option I'd like to add one new
64 bit option with mask for new boolean bridge options so we can avoid
increasing the max rtnl attr id for such options. Please let me know
if you plan to work on the new option or I can cook something.
Thanks,
Nik
^ permalink raw reply
* Re: [PATCH iproute2 net-next v2 0/2] Add DF configuration for VXLAN and GENEVE link types
From: David Ahern @ 2018-11-09 16:53 UTC (permalink / raw)
To: Stefano Brivio; +Cc: Stephen Hemminger, netdev
In-Reply-To: <cover.1541641480.git.sbrivio@redhat.com>
On 11/8/18 4:21 AM, Stefano Brivio wrote:
> This series adds configuration of the DF bit in outgoing IPv4 packets for
> VXLAN and GENEVE link types.
>
> Stefano Brivio (2):
> iplink_vxlan: Add DF configuration
> iplink_geneve: Add DF configuration
>
> include/uapi/linux/if_link.h | 18 ++++++++++++++++++
> ip/iplink_geneve.c | 29 +++++++++++++++++++++++++++++
> ip/iplink_vxlan.c | 29 +++++++++++++++++++++++++++++
> man/man8/ip-link.8.in | 28 ++++++++++++++++++++++++++++
> 4 files changed, 104 insertions(+)
>
applied to iproute2-next. Thanks
^ permalink raw reply
* Re: [PATCH mlx5-next 08/10] IB/mlx5: Call PAGE_FAULT_RESUME command asynchronously
From: Jason Gunthorpe @ 2018-11-09 16:59 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, RDMA mailing list, Artemy Kovalyov, Majd Dibbiny,
Moni Shoua, Saeed Mahameed, linux-netdev
In-Reply-To: <20181109162622.GX3695@mtr-leonro.mtl.com>
On Fri, Nov 09, 2018 at 06:26:22PM +0200, Leon Romanovsky wrote:
> On Thu, Nov 08, 2018 at 07:49:03PM +0000, Jason Gunthorpe wrote:
> > On Thu, Nov 08, 2018 at 09:10:15PM +0200, Leon Romanovsky wrote:
> > > From: Moni Shoua <monis@mellanox.com>
> > >
> > > Telling the HCA that page fault handling is done and QP can resume
> > > its flow is done in the context of the page fault handler. This blocks
> > > the handling of the next work in queue without a need.
> > > Call the PAGE_FAULT_RESUME command in an asynchronous manner and free
> > > the workqueue to pick the next work item for handling. All tasks that
> > > were executed after PAGE_FAULT_RESUME need to be done now
> > > in the callback of the asynchronous command mechanism.
> > >
> > > Signed-off-by: Moni Shoua <monis@mellanox.com>
> > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > > drivers/infiniband/hw/mlx5/odp.c | 110 +++++++++++++++++++++++++------
> > > include/linux/mlx5/driver.h | 3 +
> > > 2 files changed, 94 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/hw/mlx5/odp.c b/drivers/infiniband/hw/mlx5/odp.c
> > > index abce55b8b9ba..0c4f469cdd5b 100644
> > > +++ b/drivers/infiniband/hw/mlx5/odp.c
> > > @@ -298,20 +298,78 @@ void mlx5_ib_internal_fill_odp_caps(struct mlx5_ib_dev *dev)
> > > return;
> > > }
> > >
> > > +struct pfault_resume_cb_ctx {
> > > + struct mlx5_ib_dev *dev;
> > > + struct mlx5_core_rsc_common *res;
> > > + struct mlx5_pagefault *pfault;
> > > +};
> > > +
> > > +static void page_fault_resume_callback(int status, void *context)
> > > +{
> > > + struct pfault_resume_cb_ctx *ctx = context;
> > > + struct mlx5_pagefault *pfault = ctx->pfault;
> > > +
> > > + if (status)
> > > + mlx5_ib_err(ctx->dev, "Resolve the page fault failed with status %d\n",
> > > + status);
> > > +
> > > + if (ctx->res)
> > > + mlx5_core_res_put(ctx->res);
> > > + kfree(pfault);
> > > + kfree(ctx);
> > > +}
> > > +
> > > static void mlx5_ib_page_fault_resume(struct mlx5_ib_dev *dev,
> > > + struct mlx5_core_rsc_common *res,
> > > struct mlx5_pagefault *pfault,
> > > - int error)
> > > + int error,
> > > + bool async)
> > > {
> > > + int ret = 0;
> > > + u32 *out = pfault->out_pf_resume;
> > > + u32 *in = pfault->in_pf_resume;
> > > + u32 token = pfault->token;
> > > int wq_num = pfault->event_subtype == MLX5_PFAULT_SUBTYPE_WQE ?
> > > - pfault->wqe.wq_num : pfault->token;
> > > - int ret = mlx5_core_page_fault_resume(dev->mdev,
> > > - pfault->token,
> > > - wq_num,
> > > - pfault->type,
> > > - error);
> > > - if (ret)
> > > - mlx5_ib_err(dev, "Failed to resolve the page fault on WQ 0x%x\n",
> > > - wq_num);
> > > + pfault->wqe.wq_num : pfault->token;
> > > + u8 type = pfault->type;
> > > + struct pfault_resume_cb_ctx *ctx = NULL;
> > > +
> > > + if (async)
> > > + ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
> >
> > Why not allocate this ctx ast part of the mlx5_pagefault and avoid
> > this allocation failure strategy?
>
> It is another way to implement it, both of them are correct.
.. I think it is alot better to move this allocation, it gets rid of
this ugly duplicated code
> Can I assume that we can progress with patches except patch #2?
Lets drop this one too..
Jason
^ permalink raw reply
* Re: [PATCH iproute] ss: Actually print left delimiter for columns
From: Stephen Hemminger @ 2018-11-09 17:05 UTC (permalink / raw)
To: Stefano Brivio; +Cc: Yoann P., netdev
In-Reply-To: <c116373229190175845bc3c45b944a741b09129d.1540850171.git.sbrivio@redhat.com>
On Mon, 29 Oct 2018 23:04:25 +0100
Stefano Brivio <sbrivio@redhat.com> wrote:
> While rendering columns, we use a local variable to keep track of the
> field currently being printed, without touching current_field, which is
> used for buffering.
>
> Use the right pointer to access the left delimiter for the current column,
> instead of always printing the left delimiter for the last buffered field,
> which is usually an empty string.
>
> This fixes an issue especially visible on narrow terminals, where some
> columns might be displayed without separation.
>
> Reported-by: YoyPa <yoann.p.public@gmail.com>
> Fixes: 691bd854bf4a ("ss: Buffer raw fields first, then render them as a table")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> Tested-by: YoyPa <yoann.p.public@gmail.com>
This test broke the testsuite/ss/ssfilter.t test.
Please fix the test to match your new output format, or I will have to revert it.
^ permalink raw reply
* Re: [PATCH 11/20] octeontx2-af: Add support for stripping STAG/CTAG
From: Sunil Kovvuri @ 2018-11-09 17:06 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linux Netdev List, David S. Miller, linux-soc, Tomasz Duszynski,
Sunil Goutham
In-Reply-To: <CAK8P3a0LnNxobmAQ4TPH5xZTKd-wDc4mnzOYoQKNTWRc5r=Jbg@mail.gmail.com>
On Fri, Nov 9, 2018 at 4:42 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Fri, Nov 9, 2018 at 5:29 AM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> > On Fri, Nov 9, 2018 at 2:17 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Thu, Nov 8, 2018 at 7:37 PM <sunil.kovvuri@gmail.com> wrote:
>
> > >
> > > Here is another instance of bitfields in an interface structure. As
> > > before, please try to avoid doing that and use bit shifts and masks
> > > instead.
> > >
> > > Arnd
> >
> > No, this struct is not part of communication interface.
> > This is used to fill up a register in a bit more readable fashion
> > instead of plain bit shifts.
>
> But this is still an interface, isn't it? Writing to the register
> implies that there is some hardware that interprets the
> bits, so they have to be in the right place.
>
> > ===
> > struct nix_rx_vtag_action vtag_action;
> >
> > *(u64 *)&vtag_action = 0;
> > vtag_action.vtag0_valid = 1;
> > /* must match type set in NIX_VTAG_CFG */
> > vtag_action.vtag0_type = 0;
> > vtag_action.vtag0_lid = NPC_LID_LA;
> > vtag_action.vtag0_relptr = 12;
> > entry.vtag_action = *(u64 *)&vtag_action;
> >
> > /* Set TAG 'action' */
> > rvu_write64(rvu, blkaddr, NPC_AF_MCAMEX_BANKX_TAG_ACT(index, actbank),
> > entry->vtag_action);
>
> I assume this rvu_write64() does a cpu_to_le64() swap on big-endian,
> so the contents again are in the wrong place. I don't see any non-reserved
> fields that span an 8-bit boundary, so you can probably rearrange the bits
> to make it work, but generally speaking it's better to not rely on how the
> compiler lays out bit fields.
>
> Arnd
Agreed.
Will fix and submit a new series.
Thanks,
Sunil.
^ permalink raw reply
* Re: [PATCH][net-next] net: tcp: remove BUG_ON from tcp_v4_err
From: Eric Dumazet @ 2018-11-09 17:12 UTC (permalink / raw)
To: Li RongQing, netdev
In-Reply-To: <1541754291-8659-1-git-send-email-lirongqing@baidu.com>
On 11/09/2018 01:04 AM, Li RongQing wrote:
> if skb is NULL pointer, and the following access of skb's
> skb_mstamp_ns will trigger panic, which is same as BUG_ON
>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
> net/ipv4/tcp_ipv4.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index a336787d75e5..5424a4077c27 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -542,7 +542,6 @@ int tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
> icsk->icsk_rto = inet_csk_rto_backoff(icsk, TCP_RTO_MAX);
>
> skb = tcp_rtx_queue_head(sk);
> - BUG_ON(!skb);
>
> tcp_mstamp_refresh(tp);
> delta_us = (u32)(tp->tcp_mstamp - tcp_skb_timestamp_us(skb));
>
SGTM, thanks.
Signed-off-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH 08/20] octeontx2-af: Alloc and config NPC MCAM entry at a time
From: Sunil Kovvuri @ 2018-11-09 17:13 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linux Netdev List, David S. Miller, linux-soc, Sunil Goutham
In-Reply-To: <CAK8P3a27xsV0iWf8oCVxjRmLxhbtrR25R=LjouTdfWqjfduZpw@mail.gmail.com>
On Fri, Nov 9, 2018 at 4:32 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Fri, Nov 9, 2018 at 5:21 AM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> >
> > On Fri, Nov 9, 2018 at 2:13 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > On Thu, Nov 8, 2018 at 7:37 PM <sunil.kovvuri@gmail.com> wrote:
> > > > @@ -666,4 +668,20 @@ struct npc_mcam_unmap_counter_req {
> > > > u8 all; /* Unmap all entries using this counter ? */
> > > > };
> > > >
> > > > +struct npc_mcam_alloc_and_write_entry_req {
> > > > + struct mbox_msghdr hdr;
> > > > + struct mcam_entry entry_data;
> > > > + u16 ref_entry;
> > > > + u8 priority; /* Lower or higher w.r.t ref_entry */
> > > > + u8 intf; /* Rx or Tx interface */
> > > > + u8 enable_entry;/* Enable this MCAM entry ? */
> > > > + u8 alloc_cntr; /* Allocate counter and map ? */
> > > > +};
> > >
> > > I noticed that this structure requires padding at the end because
> > > struct mbox_msghdr has a 32-bit alignment requirement. For
> > > data structures in an interface, I'd recommend avoiding that kind
> > > of padding and adding reserved fields or widening the types
> > > accordingly.
> > >
> >
> > When there are multiple messages in the mailbox, each message starts
> > at a 16byte aligned offset. So struct mbox_msghdr is always aligned.
> > I think adding reserved fields is not needed here.
> >
> > ===
> > struct mbox_msghdr *otx2_mbox_alloc_msg_rsp(struct otx2_mbox *mbox, int devid,
> > int size, int size_rsp)
> > {
> > size = ALIGN(size, MBOX_MSG_ALIGN);
> > ===
> >
> > Is this what you were referring to ?
> >
>
> No, I mean the padding at the end of the structure. An example
> would be a structure like
>
> struct s {
> u16 a;
> u32 b;
> u16 c;
> };
>
> Since b is aligned to four bytes, you get padding between a and b.
> On top of that, you also get padding after c to make the size of
> structure itself be a multiple of its alignment. For interfaces, we
> should avoid both kinds of padding. This can be done by marking
> members as __packed (usually I don't recommend that), by
> changing the size of members, or by adding explicit 'reserved'
> fields in place of the padding.
>
> > > I also noticed a similar problem in struct mbox_msghdr. Maybe
> > > use the 'pahole' tool to check for this kind of padding in the
> > > API structures.
>
> Arnd
Got your point now and agree that padding has to be avoided.
But this is a big change and above pointed structure is not
the only one as this applies to all structures in the file.
Would it be okay if I submit a separate patch after this series
addressing all structures ?
Thanks,
Sunil.
^ permalink raw reply
* Re: [PATCH bpf-next 2/4] bpf: Split bpf_sk_lookup
From: Martin Lau @ 2018-11-09 17:19 UTC (permalink / raw)
To: Andrey Ignatov
Cc: netdev@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
joe@wand.net.nz, Kernel Team
In-Reply-To: <497a6a47655e647ef51d127b4d5ca2cea0d1d268.1541695683.git.rdna@fb.com>
On Thu, Nov 08, 2018 at 08:54:23AM -0800, Andrey Ignatov wrote:
> Split bpf_sk_lookup to separate core functionality, that can be reused
> to make socket lookup available to more program types, from
> functionality specific to program types that have access to skb.
>
> Core functionality is placed to __bpf_sk_lookup. And bpf_sk_lookup only
> gets caller netns and ifindex from skb and passes it to __bpf_sk_lookup.
>
> Program types that don't have access to skb can just pass NULL to
> __bpf_sk_lookup that will be handled correctly by both inet{,6}_sdif and
> lookup functions.
>
> This is refactoring that simply moves blocks around and does NOT change
> existing logic.
>
> Signed-off-by: Andrey Ignatov <rdna@fb.com>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> ---
> net/core/filter.c | 38 +++++++++++++++++++++++---------------
> 1 file changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 9a1327eb25fa..dc0f86a707b7 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4825,14 +4825,10 @@ static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
>
> #ifdef CONFIG_INET
> static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
> - struct sk_buff *skb, u8 family, u8 proto)
> + struct sk_buff *skb, u8 family, u8 proto, int dif)
> {
> bool refcounted = false;
> struct sock *sk = NULL;
> - int dif = 0;
> -
> - if (skb->dev)
> - dif = skb->dev->ifindex;
>
> if (family == AF_INET) {
> __be32 src4 = tuple->ipv4.saddr;
> @@ -4875,16 +4871,16 @@ static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
> return sk;
> }
>
> -/* bpf_sk_lookup performs the core lookup for different types of sockets,
> +/* __bpf_sk_lookup performs the core lookup for different types of sockets,
> * taking a reference on the socket if it doesn't have the flag SOCK_RCU_FREE.
> * Returns the socket as an 'unsigned long' to simplify the casting in the
> * callers to satisfy BPF_CALL declarations.
> */
> static unsigned long
> -bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> - u8 proto, u64 netns_id, u64 flags)
> +__bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> + u8 proto, u64 netns_id, struct net *caller_net, int ifindex,
> + u64 flags)
That looks a bit different from the one landed to bpf-next.
You may need to respin the set.
> {
> - struct net *caller_net;
> struct sock *sk = NULL;
> u8 family = AF_UNSPEC;
> struct net *net;
> @@ -4893,19 +4889,15 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> if (unlikely(family == AF_UNSPEC || netns_id > U32_MAX || flags))
> goto out;
>
> - if (skb->dev)
> - caller_net = dev_net(skb->dev);
> - else
> - caller_net = sock_net(skb->sk);
> if (netns_id) {
> net = get_net_ns_by_id(caller_net, netns_id);
> if (unlikely(!net))
> goto out;
> - sk = sk_lookup(net, tuple, skb, family, proto);
> + sk = sk_lookup(net, tuple, skb, family, proto, ifindex);
> put_net(net);
> } else {
> net = caller_net;
> - sk = sk_lookup(net, tuple, skb, family, proto);
> + sk = sk_lookup(net, tuple, skb, family, proto, ifindex);
> }
>
> if (sk)
> @@ -4914,6 +4906,22 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> return (unsigned long) sk;
> }
>
> +static unsigned long
> +bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> + u8 proto, u64 netns_id, u64 flags)
> +{
> + struct net *caller_net = sock_net(skb->sk);
> + int ifindex = 0;
> +
> + if (skb->dev) {
> + caller_net = dev_net(skb->dev);
> + ifindex = skb->dev->ifindex;
> + }
> +
> + return __bpf_sk_lookup(skb, tuple, len, proto, netns_id, caller_net,
> + ifindex, flags);
> +}
> +
> BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
> struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
> {
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH bpf-next 3/4] bpf: Support socket lookup in CGROUP_SOCK_ADDR progs
From: Martin Lau @ 2018-11-09 17:24 UTC (permalink / raw)
To: Andrey Ignatov
Cc: netdev@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
joe@wand.net.nz, Kernel Team
In-Reply-To: <56e95bcb5c99a4547cb896b1e7e732d7926d23e1.1541695683.git.rdna@fb.com>
On Thu, Nov 08, 2018 at 08:54:24AM -0800, Andrey Ignatov wrote:
> Make bpf_sk_lookup_tcp, bpf_sk_lookup_udp and bpf_sk_release helpers
> available in programs of type BPF_PROG_TYPE_CGROUP_SOCK_ADDR.
>
> Such programs operate on sockets and have access to socket and struct
> sockaddr passed by user to system calls such as sys_bind, sys_connect,
> sys_sendmsg.
>
> It's useful to be able to lookup other sockets from these programs.
> E.g. sys_connect may lookup IP:port endpoint and if there is a server
> socket bound to that endpoint ("server" can be defined by saddr & sport
> being zero), redirect client connection to it by rewriting IP:port in
> sockaddr passed to sys_connect.
>
> Signed-off-by: Andrey Ignatov <rdna@fb.com>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> ---
> net/core/filter.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index dc0f86a707b7..2e8575a34a1e 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4971,6 +4971,51 @@ static const struct bpf_func_proto bpf_sk_release_proto = {
> .ret_type = RET_INTEGER,
> .arg1_type = ARG_PTR_TO_SOCKET,
> };
> +
> +static unsigned long
> +bpf_sock_addr_sk_lookup(struct sock *sk, struct bpf_sock_tuple *tuple, u32 len,
> + u8 proto, u64 netns_id, u64 flags)
Nit. This func looks unnecessary. as good as directly calling __bpf_sk_lookup().
Others LGTM.
> +{
> + return __bpf_sk_lookup(NULL, tuple, len, proto, netns_id, sock_net(sk),
> + 0, flags);
> +}
> +
> +BPF_CALL_5(bpf_sock_addr_sk_lookup_tcp, struct bpf_sock_addr_kern *, ctx,
> + struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
> +{
> + return bpf_sock_addr_sk_lookup(ctx->sk, tuple, len, IPPROTO_TCP,
> + netns_id, flags);
> +}
> +
> +static const struct bpf_func_proto bpf_sock_addr_sk_lookup_tcp_proto = {
> + .func = bpf_sock_addr_sk_lookup_tcp,
> + .gpl_only = false,
> + .ret_type = RET_PTR_TO_SOCKET_OR_NULL,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_PTR_TO_MEM,
> + .arg3_type = ARG_CONST_SIZE,
> + .arg4_type = ARG_ANYTHING,
> + .arg5_type = ARG_ANYTHING,
> +};
> +
> +BPF_CALL_5(bpf_sock_addr_sk_lookup_udp, struct bpf_sock_addr_kern *, ctx,
> + struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
> +{
> + return bpf_sock_addr_sk_lookup(ctx->sk, tuple, len, IPPROTO_UDP,
> + netns_id, flags);
> +}
> +
> +static const struct bpf_func_proto bpf_sock_addr_sk_lookup_udp_proto = {
> + .func = bpf_sock_addr_sk_lookup_udp,
> + .gpl_only = false,
> + .ret_type = RET_PTR_TO_SOCKET_OR_NULL,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_PTR_TO_MEM,
> + .arg3_type = ARG_CONST_SIZE,
> + .arg4_type = ARG_ANYTHING,
> + .arg5_type = ARG_ANYTHING,
> +};
> +
> #endif /* CONFIG_INET */
>
> bool bpf_helper_changes_pkt_data(void *func)
> @@ -5077,6 +5122,14 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> return &bpf_get_socket_cookie_sock_addr_proto;
> case BPF_FUNC_get_local_storage:
> return &bpf_get_local_storage_proto;
> +#ifdef CONFIG_INET
> + case BPF_FUNC_sk_lookup_tcp:
> + return &bpf_sock_addr_sk_lookup_tcp_proto;
> + case BPF_FUNC_sk_lookup_udp:
> + return &bpf_sock_addr_sk_lookup_udp_proto;
> + case BPF_FUNC_sk_release:
> + return &bpf_sk_release_proto;
> +#endif /* CONFIG_INET */
> default:
> return bpf_base_func_proto(func_id);
> }
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH][RFC] udp: cache sock to avoid searching it twice
From: Eric Dumazet @ 2018-11-09 17:25 UTC (permalink / raw)
To: Li RongQing, netdev
In-Reply-To: <1541744479-12810-1-git-send-email-lirongqing@baidu.com>
On 11/08/2018 10:21 PM, Li RongQing wrote:
> GRO for UDP needs to lookup socket twice, first is in gro receive,
> second is gro complete, so if store sock to skb to avoid looking up
> twice, this can give small performance boost
>
> netperf -t UDP_RR -l 10
>
> Before:
> Rate per sec: 28746.01
> After:
> Rate per sec: 29401.67
>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
> net/ipv4/udp_offload.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 0646d61f4fa8..429570112a33 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -408,6 +408,11 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
>
> if (udp_sk(sk)->gro_enabled) {
> pp = call_gro_receive(udp_gro_receive_segment, head, skb);
> +
> + if (!IS_ERR(pp) && NAPI_GRO_CB(pp)->count > 1) {
> + sock_hold(sk);
> + pp->sk = sk;
You also have to set pp->destructor to sock_edemux
flush_gro_hash -> kfree_skb()
If there is no destructor, the reference on pp->sk will never be released.
> + }
> rcu_read_unlock();
> return pp;
> }
> @@ -444,6 +449,10 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
> skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
> pp = call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
>
> + if (!IS_ERR(pp) && NAPI_GRO_CB(pp)->count > 1) {
> + sock_hold(sk);
> + pp->sk = sk;
> + }
> out_unlock:
> rcu_read_unlock();
> skb_gro_flush_final(skb, pp, flush);
> @@ -502,7 +511,9 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff,
> uh->len = newlen;
>
> rcu_read_lock();
> - sk = (*lookup)(skb, uh->source, uh->dest);
> + sk = skb->sk;
> + if (!sk)
> + sk = (*lookup)(skb, uh->source, uh->dest);
> if (sk && udp_sk(sk)->gro_enabled) {
> err = udp_gro_complete_segment(skb);
> } else if (sk && udp_sk(sk)->gro_complete) {
> @@ -516,6 +527,11 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff,
> err = udp_sk(sk)->gro_complete(sk, skb,
> nhoff + sizeof(struct udphdr));
> }
> +
> + if (skb->sk) {
> + sock_put(skb->sk);
> + skb->sk = NULL;
> + }
> rcu_read_unlock();
>
> if (skb->remcsum_offload)
>
^ permalink raw reply
* Re: [PATCH bpf-next 4/4] selftest/bpf: Use bpf_sk_lookup_{tcp,udp} in test_sock_addr
From: Martin Lau @ 2018-11-09 17:25 UTC (permalink / raw)
To: Andrey Ignatov
Cc: netdev@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
joe@wand.net.nz, Kernel Team
In-Reply-To: <8e1a5b824d2b0bee61ef05d8601103b88d91225e.1541695683.git.rdna@fb.com>
On Thu, Nov 08, 2018 at 08:54:25AM -0800, Andrey Ignatov wrote:
> Use bpf_sk_lookup_tcp, bpf_sk_lookup_udp and bpf_sk_release helpers from
> test_sock_addr programs to make sure they're available and can lookup
> and release socket properly for IPv4/IPv4, TCP/UDP.
>
> Reading from a few fields of returned struct bpf_sock is also tested.
>
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply
* Re: bring back IPX and NCPFS, please!
From: Johannes C. Schulz @ 2018-11-09 17:30 UTC (permalink / raw)
Cc: netdev
In-Reply-To: <20181109160903.GA1277@1wt.eu>
Hello Willy, hello Stephen
Thankyou for your reply.
But I'm not able to maintain or code these modules. I'm just a bloody
user/webdev. It would be really nice if these modules will find a good
maintainer!
Best regards
Johannes
Am Fr., 9. Nov. 2018 um 17:09 Uhr schrieb Willy Tarreau <w@1wt.eu>:
>
> On Fri, Nov 09, 2018 at 02:23:27PM +0100, Johannes C. Schulz wrote:
> > Hello all!
> >
> > I like to please you to bring back IPX and NCPFS modules to the kernel.
> > Whyever my admins using Novell-shares on our network which I'm not be
> > able to use anymore - I'm forced to use cifs instead (and the admins
> > will kill the cifs-shares in some time), because my kernel (4.18) does
> > not have support for ncpfs anymore.
> > Maybe we at my work are not enough people that just for us this
> > modules will come back, but maybe out there are other people.
> > Thank you.
>
> Well, like any code, it requires time and skills. If nobody with the
> required skills is available for this anymore, there's no way you'll
> get a feature back. However you could always step up to maintain it
> yourself if you have the time and are willing to develop your own
> skills at it. It's how maintainers change over time for certain parts
> of the system, so you have an opportunity here.
>
> Just my two cents,
> Willy
--
Viele Grüße
Johannes C. Schulz
„Programmer - n. [proh-gram-er] an organism that turns caffeine and
pizza into software“
^ permalink raw reply
* Re: [PATCH bpf-next 2/4] bpf: Split bpf_sk_lookup
From: Andrey Ignatov @ 2018-11-09 17:30 UTC (permalink / raw)
To: Martin Lau
Cc: netdev@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
joe@wand.net.nz, Kernel Team
In-Reply-To: <20181109171928.jb7k2lbe2rdncyet@kafai-mbp.dhcp.thefacebook.com>
Martin Lau <kafai@fb.com> [Fri, 2018-11-09 09:19 -0800]:
> On Thu, Nov 08, 2018 at 08:54:23AM -0800, Andrey Ignatov wrote:
> > Split bpf_sk_lookup to separate core functionality, that can be reused
> > to make socket lookup available to more program types, from
> > functionality specific to program types that have access to skb.
> >
> > Core functionality is placed to __bpf_sk_lookup. And bpf_sk_lookup only
> > gets caller netns and ifindex from skb and passes it to __bpf_sk_lookup.
> >
> > Program types that don't have access to skb can just pass NULL to
> > __bpf_sk_lookup that will be handled correctly by both inet{,6}_sdif and
> > lookup functions.
> >
> > This is refactoring that simply moves blocks around and does NOT change
> > existing logic.
> >
> > Signed-off-by: Andrey Ignatov <rdna@fb.com>
> > Acked-by: Alexei Starovoitov <ast@kernel.org>
> > ---
> > net/core/filter.c | 38 +++++++++++++++++++++++---------------
> > 1 file changed, 23 insertions(+), 15 deletions(-)
> >
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index 9a1327eb25fa..dc0f86a707b7 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -4825,14 +4825,10 @@ static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
> >
> > #ifdef CONFIG_INET
> > static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
> > - struct sk_buff *skb, u8 family, u8 proto)
> > + struct sk_buff *skb, u8 family, u8 proto, int dif)
> > {
> > bool refcounted = false;
> > struct sock *sk = NULL;
> > - int dif = 0;
> > -
> > - if (skb->dev)
> > - dif = skb->dev->ifindex;
> >
> > if (family == AF_INET) {
> > __be32 src4 = tuple->ipv4.saddr;
> > @@ -4875,16 +4871,16 @@ static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
> > return sk;
> > }
> >
> > -/* bpf_sk_lookup performs the core lookup for different types of sockets,
> > +/* __bpf_sk_lookup performs the core lookup for different types of sockets,
> > * taking a reference on the socket if it doesn't have the flag SOCK_RCU_FREE.
> > * Returns the socket as an 'unsigned long' to simplify the casting in the
> > * callers to satisfy BPF_CALL declarations.
> > */
> > static unsigned long
> > -bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> > - u8 proto, u64 netns_id, u64 flags)
> > +__bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> > + u8 proto, u64 netns_id, struct net *caller_net, int ifindex,
> > + u64 flags)
> That looks a bit different from the one landed to bpf-next.
> You may need to respin the set.
Since Nitin's version is landed now, I'll rebase on top of it and this
patch just won't be needed (initially I did it to unblock myself).
I'll also address the nit in patch 3 and send v2 with both changes.
Thanks Martin!
> > {
> > - struct net *caller_net;
> > struct sock *sk = NULL;
> > u8 family = AF_UNSPEC;
> > struct net *net;
> > @@ -4893,19 +4889,15 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> > if (unlikely(family == AF_UNSPEC || netns_id > U32_MAX || flags))
> > goto out;
> >
> > - if (skb->dev)
> > - caller_net = dev_net(skb->dev);
> > - else
> > - caller_net = sock_net(skb->sk);
> > if (netns_id) {
> > net = get_net_ns_by_id(caller_net, netns_id);
> > if (unlikely(!net))
> > goto out;
> > - sk = sk_lookup(net, tuple, skb, family, proto);
> > + sk = sk_lookup(net, tuple, skb, family, proto, ifindex);
> > put_net(net);
> > } else {
> > net = caller_net;
> > - sk = sk_lookup(net, tuple, skb, family, proto);
> > + sk = sk_lookup(net, tuple, skb, family, proto, ifindex);
> > }
> >
> > if (sk)
> > @@ -4914,6 +4906,22 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> > return (unsigned long) sk;
> > }
> >
> > +static unsigned long
> > +bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> > + u8 proto, u64 netns_id, u64 flags)
> > +{
> > + struct net *caller_net = sock_net(skb->sk);
> > + int ifindex = 0;
> > +
> > + if (skb->dev) {
> > + caller_net = dev_net(skb->dev);
> > + ifindex = skb->dev->ifindex;
> > + }
> > +
> > + return __bpf_sk_lookup(skb, tuple, len, proto, netns_id, caller_net,
> > + ifindex, flags);
> > +}
> > +
> > BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
> > struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
> > {
> > --
> > 2.17.1
> >
--
Andrey Ignatov
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox