* Re: [RFC] longer netdev names proposal
From: Stephen Hemminger @ 2019-06-28 15:44 UTC (permalink / raw)
To: Jiri Pirko
Cc: Andrew Lunn, Michal Kubecek, netdev, David Ahern, davem,
jakub.kicinski, mlxsw
In-Reply-To: <20190628135553.GA6640@nanopsycho>
On Fri, 28 Jun 2019 15:55:53 +0200
Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, Jun 28, 2019 at 03:14:01PM CEST, andrew@lunn.ch wrote:
> >On Fri, Jun 28, 2019 at 01:12:16PM +0200, Jiri Pirko wrote:
> >> Thu, Jun 27, 2019 at 09:20:41PM CEST, stephen@networkplumber.org wrote:
> >> >On Thu, 27 Jun 2019 20:39:48 +0200
> >> >Michal Kubecek <mkubecek@suse.cz> wrote:
> >> >
> >> >> >
> >> >> > $ ip li set dev enp3s0 alias "Onboard Ethernet"
> >> >> > # ip link show "Onboard Ethernet"
> >> >> > Device "Onboard Ethernet" does not exist.
> >> >> >
> >> >> > So it does not really appear to be an alias, it is a label. To be
> >> >> > truly useful, it needs to be more than a label, it needs to be a real
> >> >> > alias which you can use.
> >> >>
> >> >> That's exactly what I meant: to be really useful, one should be able to
> >> >> use the alias(es) for setting device options, for adding routes, in
> >> >> netfilter rules etc.
> >> >>
> >> >> Michal
> >> >
> >> >The kernel doesn't enforce uniqueness of alias.
> >> >Also current kernel RTM_GETLINK doesn't do filter by alias (easily fixed).
> >> >
> >> >If it did, then handling it in iproute would be something like:
> >>
> >> I think that it is desired for kernel to work with "real alias" as a
> >> handle. Userspace could either pass ifindex, IFLA_NAME or "real alias".
> >> Userspace mapping like you did here might be perhaps okay for iproute2,
> >> but I think that we need something and easy to use for all.
> >>
> >> Let's call it "altname". Get would return:
> >>
> >> IFLA_NAME eth0
> >> IFLA_ALT_NAME_LIST
> >> IFLA_ALT_NAME eth0
> >> IFLA_ALT_NAME somethingelse
> >> IFLA_ALT_NAME somenamethatisreallylong
> >
> >Hi Jiri
> >
> >What is your user case for having multiple IFLA_ALT_NAME for the same
> >IFLA_NAME?
>
> I don't know about specific usecase for having more. Perhaps Michal
> does.
>
> From the implementation perspective it is handy to have the ifname as
> the first alt name in kernel, so the userspace would just pass
> IFLA_ALT_NAME always. Also for avoiding name collisions etc.
I like the alternate name proposal. The kernel would have to impose uniqueness.
Does alt_name have to be unique across both regular and alt_name?
Having multiple names list seems less interesting but it could be useful.
^ permalink raw reply
* Re: [RFC] longer netdev names proposal
From: Jiri Pirko @ 2019-06-28 15:56 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Andrew Lunn, Michal Kubecek, netdev, David Ahern, davem,
jakub.kicinski, mlxsw
In-Reply-To: <20190628084447.186a0efb@hermes.lan>
Fri, Jun 28, 2019 at 05:44:47PM CEST, stephen@networkplumber.org wrote:
>On Fri, 28 Jun 2019 15:55:53 +0200
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> Fri, Jun 28, 2019 at 03:14:01PM CEST, andrew@lunn.ch wrote:
>> >On Fri, Jun 28, 2019 at 01:12:16PM +0200, Jiri Pirko wrote:
>> >> Thu, Jun 27, 2019 at 09:20:41PM CEST, stephen@networkplumber.org wrote:
>> >> >On Thu, 27 Jun 2019 20:39:48 +0200
>> >> >Michal Kubecek <mkubecek@suse.cz> wrote:
>> >> >
>> >> >> >
>> >> >> > $ ip li set dev enp3s0 alias "Onboard Ethernet"
>> >> >> > # ip link show "Onboard Ethernet"
>> >> >> > Device "Onboard Ethernet" does not exist.
>> >> >> >
>> >> >> > So it does not really appear to be an alias, it is a label. To be
>> >> >> > truly useful, it needs to be more than a label, it needs to be a real
>> >> >> > alias which you can use.
>> >> >>
>> >> >> That's exactly what I meant: to be really useful, one should be able to
>> >> >> use the alias(es) for setting device options, for adding routes, in
>> >> >> netfilter rules etc.
>> >> >>
>> >> >> Michal
>> >> >
>> >> >The kernel doesn't enforce uniqueness of alias.
>> >> >Also current kernel RTM_GETLINK doesn't do filter by alias (easily fixed).
>> >> >
>> >> >If it did, then handling it in iproute would be something like:
>> >>
>> >> I think that it is desired for kernel to work with "real alias" as a
>> >> handle. Userspace could either pass ifindex, IFLA_NAME or "real alias".
>> >> Userspace mapping like you did here might be perhaps okay for iproute2,
>> >> but I think that we need something and easy to use for all.
>> >>
>> >> Let's call it "altname". Get would return:
>> >>
>> >> IFLA_NAME eth0
>> >> IFLA_ALT_NAME_LIST
>> >> IFLA_ALT_NAME eth0
>> >> IFLA_ALT_NAME somethingelse
>> >> IFLA_ALT_NAME somenamethatisreallylong
>> >
>> >Hi Jiri
>> >
>> >What is your user case for having multiple IFLA_ALT_NAME for the same
>> >IFLA_NAME?
>>
>> I don't know about specific usecase for having more. Perhaps Michal
>> does.
>>
>> From the implementation perspective it is handy to have the ifname as
>> the first alt name in kernel, so the userspace would just pass
>> IFLA_ALT_NAME always. Also for avoiding name collisions etc.
>
>I like the alternate name proposal. The kernel would have to impose uniqueness.
>Does alt_name have to be unique across both regular and alt_name?
Yes. That is my idea. To have one big hashtable to contain them all.
>Having multiple names list seems less interesting but it could be useful.
Yeah. Okay, I'm going to jump on this.
^ permalink raw reply
* Re: [PATCH v3 bpf-next 2/9] libbpf: introduce concept of bpf_link
From: Stanislav Fomichev @ 2019-06-28 16:02 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: andrii.nakryiko, ast, daniel, netdev, bpf, kernel-team
In-Reply-To: <20190628055303.1249758-3-andriin@fb.com>
On 06/27, Andrii Nakryiko wrote:
> bpf_link is and abstraction of an association of a BPF program and one
> of many possible BPF attachment points (hooks). This allows to have
> uniform interface for detaching BPF programs regardless of the nature of
> link and how it was created. Details of creation and setting up of
> a specific bpf_link is handled by corresponding attachment methods
> (bpf_program__attach_xxx) added in subsequent commits. Once successfully
> created, bpf_link has to be eventually destroyed with
> bpf_link__destroy(), at which point BPF program is disassociated from
> a hook and all the relevant resources are freed.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 17 +++++++++++++++++
> tools/lib/bpf/libbpf.h | 4 ++++
> tools/lib/bpf/libbpf.map | 3 ++-
> 3 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 6e6ebef11ba3..455795e6f8af 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -3941,6 +3941,23 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
> return 0;
> }
>
> +struct bpf_link {
Maybe call it bpf_attachment? You call the bpf_program__attach_to_blah
and you get an attachment?
> + int (*destroy)(struct bpf_link *link);
> +};
> +
> +int bpf_link__destroy(struct bpf_link *link)
> +{
> + int err;
> +
> + if (!link)
> + return 0;
> +
> + err = link->destroy(link);
> + free(link);
> +
> + return err;
> +}
> +
> enum bpf_perf_event_ret
> bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index d639f47e3110..5082a5ebb0c2 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -165,6 +165,10 @@ 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_link;
> +
> +LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
> +
> struct bpf_insn;
>
> /*
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 2c6d835620d2..3cde850fc8da 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -167,10 +167,11 @@ LIBBPF_0.0.3 {
>
> LIBBPF_0.0.4 {
> global:
> + bpf_link__destroy;
> + bpf_object__load_xattr;
> btf_dump__dump_type;
> btf_dump__free;
> btf_dump__new;
> btf__parse_elf;
> - bpf_object__load_xattr;
> libbpf_num_possible_cpus;
> } LIBBPF_0.0.3;
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH iproute2-next] utils: move parse_percent() to tc_util
From: Andrea Claudi @ 2019-06-28 16:03 UTC (permalink / raw)
To: netdev; +Cc: stephen, dsahern
As parse_percent() is used only in tc.
This reduces ip, bridge and genl binaries size:
$ bloat-o-meter -t bridge/bridge bridge/bridge.new
add/remove: 0/1 grow/shrink: 0/0 up/down: 0/-109 (-109)
Total: Before=50973, After=50864, chg -0.21%
$ bloat-o-meter -t genl/genl genl/genl.new
add/remove: 0/1 grow/shrink: 0/0 up/down: 0/-109 (-109)
Total: Before=30298, After=30189, chg -0.36%
$ bloat-o-meter ip/ip ip/ip.new
add/remove: 0/1 grow/shrink: 0/0 up/down: 0/-109 (-109)
Total: Before=674164, After=674055, chg -0.02%
Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
include/utils.h | 1 -
lib/utils.c | 16 ----------------
tc/tc_util.c | 16 ++++++++++++++++
tc/tc_util.h | 1 +
4 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/include/utils.h b/include/utils.h
index ec0231f9772d0..1d9c11276bbe6 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -146,7 +146,6 @@ int get_addr_rta(inet_prefix *dst, const struct rtattr *rta, int family);
int get_addr_ila(__u64 *val, const char *arg);
int read_prop(const char *dev, char *prop, long *value);
-int parse_percent(double *val, const char *str);
int get_hex(char c);
int get_integer(int *val, const char *arg, int base);
int get_unsigned(unsigned *val, const char *arg, int base);
diff --git a/lib/utils.c b/lib/utils.c
index be0f11b00280d..5da9a47848966 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -101,22 +101,6 @@ out:
return -1;
}
-/* Parse a percent e.g: '30%'
- * return: 0 = ok, -1 = error, 1 = out of range
- */
-int parse_percent(double *val, const char *str)
-{
- char *p;
-
- *val = strtod(str, &p) / 100.;
- if (*val == HUGE_VALF || *val == HUGE_VALL)
- return 1;
- if (*p && strcmp(p, "%"))
- return -1;
-
- return 0;
-}
-
int get_hex(char c)
{
if (c >= 'A' && c <= 'F')
diff --git a/tc/tc_util.c b/tc/tc_util.c
index e5d15281581df..53d15e08e9734 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -190,6 +190,22 @@ static const struct rate_suffix {
{ NULL }
};
+/* Parse a percent e.g: '30%'
+ * return: 0 = ok, -1 = error, 1 = out of range
+ */
+int parse_percent(double *val, const char *str)
+{
+ char *p;
+
+ *val = strtod(str, &p) / 100.;
+ if (*val == HUGE_VALF || *val == HUGE_VALL)
+ return 1;
+ if (*p && strcmp(p, "%"))
+ return -1;
+
+ return 0;
+}
+
static int parse_percent_rate(char *rate, size_t len,
const char *str, const char *dev)
{
diff --git a/tc/tc_util.h b/tc/tc_util.h
index 825fea36a0809..eb4b60db3fdd7 100644
--- a/tc/tc_util.h
+++ b/tc/tc_util.h
@@ -101,6 +101,7 @@ int print_tc_classid(char *buf, int len, __u32 h);
char *sprint_tc_classid(__u32 h, char *buf);
int tc_print_police(FILE *f, struct rtattr *tb);
+int parse_percent(double *val, const char *str);
int parse_police(int *argc_p, char ***argv_p, int tca_id, struct nlmsghdr *n);
int parse_action_control(int *argc_p, char ***argv_p,
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v3 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event
From: Stanislav Fomichev @ 2019-06-28 16:04 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: andrii.nakryiko, ast, daniel, netdev, bpf, kernel-team
In-Reply-To: <20190628055303.1249758-4-andriin@fb.com>
On 06/27, Andrii Nakryiko wrote:
> bpf_program__attach_perf_event allows to attach BPF program to existing
> perf event hook, providing most generic and most low-level way to attach BPF
> programs. It returns struct bpf_link, which should be passed to
> bpf_link__destroy to detach and free resources, associated with a link.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 58 ++++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 3 +++
> tools/lib/bpf/libbpf.map | 1 +
> 3 files changed, 62 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 455795e6f8af..606705f878ba 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -32,6 +32,7 @@
> #include <linux/limits.h>
> #include <linux/perf_event.h>
> #include <linux/ring_buffer.h>
> +#include <sys/ioctl.h>
> #include <sys/stat.h>
> #include <sys/types.h>
> #include <sys/vfs.h>
> @@ -3958,6 +3959,63 @@ int bpf_link__destroy(struct bpf_link *link)
> return err;
> }
>
> +struct bpf_link_fd {
> + struct bpf_link link; /* has to be at the top of struct */
[..]
> + int fd; /* hook FD */
> +};
Any cons to storing everything in bpf_link, instead of creating a
"subclass"? Less things to worry about.
> +static int bpf_link__destroy_perf_event(struct bpf_link *link)
> +{
> + struct bpf_link_fd *l = (void *)link;
> + int err;
> +
> + if (l->fd < 0)
> + return 0;
> +
> + err = ioctl(l->fd, PERF_EVENT_IOC_DISABLE, 0);
> + close(l->fd);
> + return err;
> +}
> +
> +struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> + int pfd)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + struct bpf_link_fd *link;
> + int bpf_fd, err;
> +
> + bpf_fd = bpf_program__fd(prog);
> + if (bpf_fd < 0) {
> + pr_warning("program '%s': can't attach before loaded\n",
> + bpf_program__title(prog, false));
> + return ERR_PTR(-EINVAL);
> + }
> +
> + link = malloc(sizeof(*link));
> + if (!link)
> + return ERR_PTR(-ENOMEM);
> + link->link.destroy = &bpf_link__destroy_perf_event;
> + link->fd = pfd;
> +
> + if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, bpf_fd) < 0) {
> + err = -errno;
> + free(link);
> + pr_warning("program '%s': failed to attach to pfd %d: %s\n",
> + bpf_program__title(prog, false), pfd,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return ERR_PTR(err);
> + }
> + if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
> + err = -errno;
> + free(link);
> + pr_warning("program '%s': failed to enable pfd %d: %s\n",
> + bpf_program__title(prog, false), pfd,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return ERR_PTR(err);
> + }
> + return (struct bpf_link *)link;
> +}
> +
> enum bpf_perf_event_ret
> bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 5082a5ebb0c2..1bf66c4a9330 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -169,6 +169,9 @@ struct bpf_link;
>
> LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
>
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> +
> struct bpf_insn;
>
> /*
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 3cde850fc8da..756f5aa802e9 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -169,6 +169,7 @@ LIBBPF_0.0.4 {
> global:
> bpf_link__destroy;
> bpf_object__load_xattr;
> + bpf_program__attach_perf_event;
> btf_dump__dump_type;
> btf_dump__free;
> btf_dump__new;
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH] net: stmmac: add sanity check to device_property_read_u32_array call
From: Martin Blumenstingl @ 2019-06-28 16:05 UTC (permalink / raw)
To: Colin Ian King
Cc: alexandre.torgue, davem, joabreu, kernel-janitors,
linux-arm-kernel, linux-kernel, linux-stm32, mcoquelin.stm32,
netdev, peppe.cavallaro
In-Reply-To: <26646ff1-059f-fb2d-e05d-43009aeb2150@canonical.com>
Hi Colin,
On Fri, Jun 28, 2019 at 10:32 AM Colin Ian King
<colin.king@canonical.com> wrote:
>
> On 28/06/2019 05:15, Martin Blumenstingl wrote:
> > On Tue, Jun 25, 2019 at 9:58 AM Colin Ian King <colin.king@canonical.com> wrote:
> >>
> >> On 25/06/2019 05:44, Martin Blumenstingl wrote:
> >>> Hi Colin,
> >>>
> >>> On Thu, Jun 20, 2019 at 3:34 AM Martin Blumenstingl
> >>> <martin.blumenstingl@googlemail.com> wrote:
> >>>>
> >>>> Hi Colin,
> >>>>
> >>>> On Wed, Jun 19, 2019 at 8:55 AM Colin Ian King <colin.king@canonical.com> wrote:
> >>>>>
> >>>>> On 19/06/2019 06:13, Martin Blumenstingl wrote:
> >>>>>> Hi Colin,
> >>>>>>
> >>>>>>> Currently the call to device_property_read_u32_array is not error checked
> >>>>>>> leading to potential garbage values in the delays array that are then used
> >>>>>>> in msleep delays. Add a sanity check to the property fetching.
> >>>>>>>
> >>>>>>> Addresses-Coverity: ("Uninitialized scalar variable")
> >>>>>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> >>>>>> I have also sent a patch [0] to fix initialize the array.
> >>>>>> can you please look at my patch so we can work out which one to use?
> >>>>>>
> >>>>>> my concern is that the "snps,reset-delays-us" property is optional,
> >>>>>> the current dt-bindings documentation states that it's a required
> >>>>>> property. in reality it isn't, there are boards (two examples are
> >>>>>> mentioned in my patch: [0]) without it.
> >>>>>>
> >>>>>> so I believe that the resulting behavior has to be:
> >>>>>> 1. don't delay if this property is missing (instead of delaying for
> >>>>>> <garbage value> ms)
> >>>>>> 2. don't error out if this property is missing
> >>>>>>
> >>>>>> your patch covers #1, can you please check whether #2 is also covered?
> >>>>>> I tested case #2 when submitting my patch and it worked fine (even
> >>>>>> though I could not reproduce the garbage values which are being read
> >>>>>> on some boards)
> >>> in the meantime I have tested your patch.
> >>> when I don't set the "snps,reset-delays-us" property then I get the
> >>> following error:
> >>> invalid property snps,reset-delays-us
> >>>
> >>> my patch has landed in the meantime: [0]
> >>> how should we proceed with your patch?
>
> Your fix is good, so I think we should just drop/forget about my fix.
thank you for looking at the situation
as far I understand the -net/-net-next tree all commits are immutable
so if we want to remove your patch we need to send a revert
do you want me to do that (I can do it on Monday) or will you take care of that?
Martin
^ permalink raw reply
* Re: pull-request: wireless-drivers 2019-06-28
From: David Miller @ 2019-06-28 16:15 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87woh6fa93.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Fri, 28 Jun 2019 10:18:48 +0300
> here's a pull request to net tree for 5.2, more info below. Please let
> me know if there are any problems.
Pulled, thanks Kalle.
^ permalink raw reply
* Re: [PATCH 00/11] XDP unaligned chunk placement support
From: Laatz, Kevin @ 2019-06-28 16:19 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Jonathan Lemon, netdev, ast, daniel, bjorn.topel, magnus.karlsson,
bpf, intel-wired-lan, bruce.richardson, ciara.loftus
In-Reply-To: <20190627142534.4f4b8995@cakuba.netronome.com>
On 27/06/2019 22:25, Jakub Kicinski wrote:
> On Thu, 27 Jun 2019 12:14:50 +0100, Laatz, Kevin wrote:
>> On the application side (xdpsock), we don't have to worry about the user
>> defined headroom, since it is 0, so we only need to account for the
>> XDP_PACKET_HEADROOM when computing the original address (in the default
>> scenario).
> That assumes specific layout for the data inside the buffer. Some NICs
> will prepend information like timestamp to the packet, meaning the
> packet would start at offset XDP_PACKET_HEADROOM + metadata len..
Yes, if NICs prepend extra data to the packet that would be a problem for
using this feature in isolation. However, if we also add in support for
in-order
RX and TX rings, that would no longer be an issue. However, even for NICs
which do prepend data, this patchset should not break anything that is
currently
working.
>
> I think that's very limiting. What is the challenge in providing
> aligned addresses, exactly?
The challenges are two-fold:
1) it prevents using arbitrary buffer sizes, which will be an issue
supporting e.g. jumbo frames in future.
2) higher level user-space frameworks which may want to use AF_XDP, such
as DPDK, do not currently support having buffers with 'fixed' alignment.
The reason that DPDK uses arbitrary placement is that:
- it would stop things working on certain NICs which need the
actual writable space specified in units of 1k - therefore we need 2k +
metadata space.
- we place padding between buffers to avoid constantly hitting
the same memory channels when accessing memory.
- it allows the application to choose the actual buffer size it
wants to use.
We make use of the above to allow us to speed up processing
significantly and also reduce the packet buffer memory size.
Not having arbitrary buffer alignment also means an AF_XDP driver
for DPDK cannot be a drop-in replacement for existing drivers in those
frameworks. Even with a new capability to allow an arbitrary buffer
alignment, existing apps will need to be modified to use that new
capability.
^ permalink raw reply
* Re: [PATCH net-next v2] net: stmmac: Fix case when PHY handle is not present
From: David Miller @ 2019-06-28 16:19 UTC (permalink / raw)
To: Jose.Abreu
Cc: linux-kernel, netdev, Joao.Pinto, peppe.cavallaro,
alexandre.torgue, sergei.shtylyov
In-Reply-To: <654cfe790807c6dfcc69c610c9692efb8c9a6179.1561706654.git.joabreu@synopsys.com>
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Fri, 28 Jun 2019 09:25:07 +0200
> Some DT bindings do not have the PHY handle. Let's fallback to manually
> discovery in case phylink_of_phy_connect() fails.
>
> Changes from v1:
> - Fixup comment style (Sergei)
>
> Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
> Reported-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
> Tested-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 00/10] net: stmmac: 10GbE using XGMAC
From: David Miller @ 2019-06-28 16:24 UTC (permalink / raw)
To: Jose.Abreu
Cc: linux-kernel, netdev, Joao.Pinto, peppe.cavallaro,
alexandre.torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Fri, 28 Jun 2019 09:29:11 +0200
> Support for 10Gb Link using XGMAC core plus some performance tweaks.
>
> Tested in a PCI based setup.
>
> iperf3 TCP results:
> TSO ON, MTU=1500, TX Queues = 1, RX Queues = 1, Flow Control ON
> Pinned CPU (-A), Zero-Copy (-Z)
>
> [ ID] Interval Transfer Bitrate Retr
> [ 5] 0.00-600.00 sec 643 GBytes 9.21 Gbits/sec 1 sender
> [ 5] 0.00-600.00 sec 643 GBytes 9.21 Gbits/sec receiver
Series applied, thanks Jose.
About the Kconfig change, maybe it just doesn't make sense to list all
of the various speeds the chip supports... just a thought.
^ permalink raw reply
* [PATCH bpf-next] bpf: fix precision tracking
From: Alexei Starovoitov @ 2019-06-28 16:24 UTC (permalink / raw)
To: davem; +Cc: daniel, netdev, bpf, kernel-team
When equivalent state is found the current state needs to propagate precision marks.
Otherwise the verifier will prune the search incorrectly.
There is a price for correctness:
before before broken fixed
cnst spill precise precise
bpf_lb-DLB_L3.o 1923 8128 1863 1898
bpf_lb-DLB_L4.o 3077 6707 2468 2666
bpf_lb-DUNKNOWN.o 1062 1062 544 544
bpf_lxc-DDROP_ALL.o 166729 380712 22629 36823
bpf_lxc-DUNKNOWN.o 174607 440652 28805 45325
bpf_netdev.o 8407 31904 6801 7002
bpf_overlay.o 5420 23569 4754 4858
bpf_lxc_jit.o 39389 359445 50925 69631
Overall precision tracking is still very effective.
Fixes: b5dc0163d8fd ("bpf: precise scalar_value tracking")
Reported-by: Lawrence Brakmo <brakmo@fb.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
Sending the fix early w/o tests, since I'm traveling.
Will add proper tests when I'm back.
---
kernel/bpf/verifier.c | 121 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 107 insertions(+), 14 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 6b5623d320f9..62afc4058ced 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1659,16 +1659,18 @@ static void mark_all_scalars_precise(struct bpf_verifier_env *env,
}
}
-static int mark_chain_precision(struct bpf_verifier_env *env, int regno)
+static int __mark_chain_precision(struct bpf_verifier_env *env, int regno,
+ int spi)
{
struct bpf_verifier_state *st = env->cur_state;
int first_idx = st->first_insn_idx;
int last_idx = env->insn_idx;
struct bpf_func_state *func;
struct bpf_reg_state *reg;
- u32 reg_mask = 1u << regno;
- u64 stack_mask = 0;
+ u32 reg_mask = regno >= 0 ? 1u << regno : 0;
+ u64 stack_mask = spi >= 0 ? 1ull << spi : 0;
bool skip_first = true;
+ bool new_marks = false;
int i, err;
if (!env->allow_ptr_leaks)
@@ -1676,18 +1678,43 @@ static int mark_chain_precision(struct bpf_verifier_env *env, int regno)
return 0;
func = st->frame[st->curframe];
- reg = &func->regs[regno];
- if (reg->type != SCALAR_VALUE) {
- WARN_ONCE(1, "backtracing misuse");
- return -EFAULT;
+ if (regno >= 0) {
+ reg = &func->regs[regno];
+ if (reg->type != SCALAR_VALUE) {
+ WARN_ONCE(1, "backtracing misuse");
+ return -EFAULT;
+ }
+ if (!reg->precise)
+ new_marks = true;
+ else
+ reg_mask = 0;
+ reg->precise = true;
}
- if (reg->precise)
- return 0;
- func->regs[regno].precise = true;
+ while (spi >= 0) {
+ if (func->stack[spi].slot_type[0] != STACK_SPILL) {
+ stack_mask = 0;
+ break;
+ }
+ reg = &func->stack[spi].spilled_ptr;
+ if (reg->type != SCALAR_VALUE) {
+ stack_mask = 0;
+ break;
+ }
+ if (!reg->precise)
+ new_marks = true;
+ else
+ stack_mask = 0;
+ reg->precise = true;
+ break;
+ }
+
+ if (!new_marks)
+ return 0;
+ if (!reg_mask && !stack_mask)
+ return 0;
for (;;) {
DECLARE_BITMAP(mask, 64);
- bool new_marks = false;
u32 history = st->jmp_history_cnt;
if (env->log.level & BPF_LOG_LEVEL)
@@ -1730,12 +1757,15 @@ static int mark_chain_precision(struct bpf_verifier_env *env, int regno)
if (!st)
break;
+ new_marks = false;
func = st->frame[st->curframe];
bitmap_from_u64(mask, reg_mask);
for_each_set_bit(i, mask, 32) {
reg = &func->regs[i];
- if (reg->type != SCALAR_VALUE)
+ if (reg->type != SCALAR_VALUE) {
+ reg_mask &= ~(1u << i);
continue;
+ }
if (!reg->precise)
new_marks = true;
reg->precise = true;
@@ -1756,11 +1786,15 @@ static int mark_chain_precision(struct bpf_verifier_env *env, int regno)
return -EFAULT;
}
- if (func->stack[i].slot_type[0] != STACK_SPILL)
+ if (func->stack[i].slot_type[0] != STACK_SPILL) {
+ stack_mask &= ~(1ull << i);
continue;
+ }
reg = &func->stack[i].spilled_ptr;
- if (reg->type != SCALAR_VALUE)
+ if (reg->type != SCALAR_VALUE) {
+ stack_mask &= ~(1ull << i);
continue;
+ }
if (!reg->precise)
new_marks = true;
reg->precise = true;
@@ -1772,6 +1806,8 @@ static int mark_chain_precision(struct bpf_verifier_env *env, int regno)
reg_mask, stack_mask);
}
+ if (!reg_mask && !stack_mask)
+ break;
if (!new_marks)
break;
@@ -1781,6 +1817,15 @@ static int mark_chain_precision(struct bpf_verifier_env *env, int regno)
return 0;
}
+static int mark_chain_precision(struct bpf_verifier_env *env, int regno)
+{
+ return __mark_chain_precision(env, regno, -1);
+}
+
+static int mark_chain_precision_stack(struct bpf_verifier_env *env, int spi)
+{
+ return __mark_chain_precision(env, -1, spi);
+}
static bool is_spillable_regtype(enum bpf_reg_type type)
{
@@ -7114,6 +7159,46 @@ static int propagate_liveness(struct bpf_verifier_env *env,
return 0;
}
+/* find precise scalars in the previous equivalent state and
+ * propagate them into the current state
+ */
+static int propagate_precision(struct bpf_verifier_env *env,
+ const struct bpf_verifier_state *old)
+{
+ struct bpf_reg_state *state_reg;
+ struct bpf_func_state *state;
+ int i, err = 0;
+
+ state = old->frame[old->curframe];
+ state_reg = state->regs;
+ for (i = 0; i < BPF_REG_FP; i++, state_reg++) {
+ if (state_reg->type != SCALAR_VALUE ||
+ !state_reg->precise)
+ continue;
+ if (env->log.level & BPF_LOG_LEVEL2)
+ verbose(env, "propagating r%d\n", i);
+ err = mark_chain_precision(env, i);
+ if (err < 0)
+ return err;
+ }
+
+ for (i = 0; i < state->allocated_stack / BPF_REG_SIZE; i++) {
+ if (state->stack[i].slot_type[0] != STACK_SPILL)
+ continue;
+ state_reg = &state->stack[i].spilled_ptr;
+ if (state_reg->type != SCALAR_VALUE ||
+ !state_reg->precise)
+ continue;
+ if (env->log.level & BPF_LOG_LEVEL2)
+ verbose(env, "propagating fp%d\n",
+ (-i - 1) * BPF_REG_SIZE);
+ err = mark_chain_precision_stack(env, i);
+ if (err < 0)
+ return err;
+ }
+ return 0;
+}
+
static bool states_maybe_looping(struct bpf_verifier_state *old,
struct bpf_verifier_state *cur)
{
@@ -7206,6 +7291,14 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
* this state and will pop a new one.
*/
err = propagate_liveness(env, &sl->state, cur);
+
+ /* if previous state reached the exit with precision and
+ * current state is equivalent to it (except precsion marks)
+ * the precision needs to be propagated back in
+ * the current state.
+ */
+ err = err ? : push_jmp_history(env, cur);
+ err = err ? : propagate_precision(env, &sl->state);
if (err)
return err;
return 1;
--
2.20.0
^ permalink raw reply related
* Re: [RFC iproute2 1/1] ip: netns: add mounted state file for each netns
From: David Howells @ 2019-06-28 16:26 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: dhowells, Alexander Aring, netdev, linux-fsdevel, kernel
In-Reply-To: <293c9bd3-f530-d75e-c353-ddeabac27cf6@6wind.com>
Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> David Howells was working on a mount notification mechanism:
> https://lwn.net/Articles/760714/
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=notifications
>
> I don't know what is the status of this series.
It's still alive. I just posted a new version on it. I'm hoping, possibly
futiley, to get it in in this merge window.
David
^ permalink raw reply
* Re: [RFC] longer netdev names proposal
From: Michal Kubecek @ 2019-06-28 16:27 UTC (permalink / raw)
To: netdev
Cc: Jiri Pirko, Andrew Lunn, Stephen Hemminger, David Ahern, davem,
jakub.kicinski, mlxsw
In-Reply-To: <20190628135553.GA6640@nanopsycho>
On Fri, Jun 28, 2019 at 03:55:53PM +0200, Jiri Pirko wrote:
> Fri, Jun 28, 2019 at 03:14:01PM CEST, andrew@lunn.ch wrote:
> >
> >What is your user case for having multiple IFLA_ALT_NAME for the same
> >IFLA_NAME?
>
> I don't know about specific usecase for having more. Perhaps Michal
> does.
One use case that comes to my mind are the "predictable names"
implemented by udev/systemd which can be based on different naming
schemes (bus address, BIOS numbering, MAC address etc.) and it's not
always obvious which scheme is going to be used. I have even seen
multiple times that one schemed was used during system installation and
another in the installed system so that network configuration created by
installer did not work.
For block devices, current practice is not to rename the device and only
create multiple symlinks based on different naming schemes (by id, by
uuid, by label, etc.). With support for multiple altnames, we could also
identify the network device in different ways (all applicable ones).
Michal
^ permalink raw reply
* Re: [PATCH v2 net-next 0/3] Better PHYLINK compliance for SJA1105 DSA
From: David Miller @ 2019-06-28 16:31 UTC (permalink / raw)
To: linux; +Cc: olteanv, f.fainelli, vivien.didelot, andrew, netdev
In-Reply-To: <20190628100542.hmzqnp4bsnkikcvv@shell.armlinux.org.uk>
From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Date: Fri, 28 Jun 2019 11:05:42 +0100
> On Fri, Jun 28, 2019 at 12:46:34AM +0300, Vladimir Oltean wrote:
>> After discussing with Russell King, it appears this driver is making a
>> few confusions and not performing some checks for consistent operation.
>>
>> Changes in v2:
>> - Removed redundant print in the phylink_validate callback (in 2/3).
>>
>> Vladimir Oltean (3):
>> net: dsa: sja1105: Don't check state->link in phylink_mac_config
>> net: dsa: sja1105: Check for PHY mode mismatches with what PHYLINK
>> reports
>> net: dsa: sja1105: Mark in-band AN modes not supported for PHYLINK
>>
>> drivers/net/dsa/sja1105/sja1105_main.c | 56 +++++++++++++++++++++++++-
>> 1 file changed, 54 insertions(+), 2 deletions(-)
>
> Thanks. For the whole series:
>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Series applied, thanks all.
^ permalink raw reply
* Re: [PATCH] hinic: reduce rss_init stack usage
From: David Miller @ 2019-06-28 16:32 UTC (permalink / raw)
To: arnd
Cc: aviad.krawczyk, xuechaojing, jesse.brandeburg, zhaochen6,
edumazet, dann.frazier, netdev, linux-kernel
In-Reply-To: <20190628103158.2446356-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 28 Jun 2019 12:31:44 +0200
> On 32-bit architectures, putting an array of 256 u32 values on the
> stack uses more space than the warning limit:
>
> drivers/net/ethernet/huawei/hinic/hinic_main.c: In function 'hinic_rss_init':
> drivers/net/ethernet/huawei/hinic/hinic_main.c:286:1: error: the frame size of 1068 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>
> I considered changing the code to use u8 values here, since that's
> all the hardware supports, but dynamically allocating the array is
> a more isolated fix here.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied to net-next.
Arnd, please make it clear what tree you are targetting in the
future. Thank you.
^ permalink raw reply
* net: check before dereferencing netdev_ops during busy poll
From: Matteo Croce @ 2019-06-28 16:34 UTC (permalink / raw)
To: stable, Sasha Levin; +Cc: Greg Kroah-Hartman, Josh Elsasser, netdev, LKML
Hi,
Is there any reason for this panic fix not being applied in stable?
https://lore.kernel.org/netdev/20180313053248.13654-1-jelsasser@appneta.com/T/
It seems that linux 4.9.184 has the bug too.
Regards,
--
Matteo Croce
per aspera ad upstream
^ permalink raw reply
* Re: [PATCH 0/5] net: dsa: microchip: Further regmap cleanups
From: Marek Vasut @ 2019-06-28 16:36 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, Andrew Lunn, Florian Fainelli, Tristram Ha, Woojung Huh
In-Reply-To: <20190628113130.GB14087@t480s.localdomain>
On 6/28/19 5:31 PM, Vivien Didelot wrote:
> Hi Marek,
>
> On Thu, 27 Jun 2019 23:55:51 +0200, Marek Vasut <marex@denx.de> wrote:
>> This patchset cleans up KSZ9477 switch driver by replacing various
>> ad-hoc polling implementations and register RMW with regmap functions.
>>
>> Each polling function is replaced separately to make it easier to review
>> and possibly bisect, but maybe the patches can be squashed.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Andrew Lunn <andrew@lunn.ch>
>> Cc: Florian Fainelli <f.fainelli@gmail.com>
>> Cc: Tristram Ha <Tristram.Ha@microchip.com>
>> Cc: Woojung Huh <Woojung.Huh@microchip.com>
>
> Please copy me next time, as per the MAINTAINERS file.
Will do. I plan to submit KSZ8795 support next.
--
Best regards,
Marek Vasut
^ permalink raw reply
* Re: [PATCH net-next 00/12] net: hns3: some code optimizations & cleanups & bugfixes
From: David Miller @ 2019-06-28 16:40 UTC (permalink / raw)
To: tanhuazhong; +Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm
In-Reply-To: <1561722618-12168-1-git-send-email-tanhuazhong@huawei.com>
From: Huazhong Tan <tanhuazhong@huawei.com>
Date: Fri, 28 Jun 2019 19:50:06 +0800
> [patch 01/12] fixes a TX timeout issue.
>
> [patch 02/12 - 04/12] adds some patch related to TM module.
>
> [patch 05/12] fixes a compile warning.
>
> [patch 06/12] adds Asym Pause support for autoneg
>
> [patch 07/12] optimizes the error handler for VF reset.
>
> [patch 08/12] deals with the empty interrupt case.
>
> [patch 09/12 - 12/12] adds some cleanups & optimizations.
Looks good, series applied, thanks.
^ permalink raw reply
* Re: [PATCH v3 bpf-next 2/9] libbpf: introduce concept of bpf_link
From: Andrii Nakryiko @ 2019-06-28 16:42 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Networking,
bpf, Kernel Team
In-Reply-To: <20190628160230.GG4866@mini-arch>
On Fri, Jun 28, 2019 at 9:02 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 06/27, Andrii Nakryiko wrote:
> > bpf_link is and abstraction of an association of a BPF program and one
> > of many possible BPF attachment points (hooks). This allows to have
> > uniform interface for detaching BPF programs regardless of the nature of
> > link and how it was created. Details of creation and setting up of
> > a specific bpf_link is handled by corresponding attachment methods
> > (bpf_program__attach_xxx) added in subsequent commits. Once successfully
> > created, bpf_link has to be eventually destroyed with
> > bpf_link__destroy(), at which point BPF program is disassociated from
> > a hook and all the relevant resources are freed.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > tools/lib/bpf/libbpf.c | 17 +++++++++++++++++
> > tools/lib/bpf/libbpf.h | 4 ++++
> > tools/lib/bpf/libbpf.map | 3 ++-
> > 3 files changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 6e6ebef11ba3..455795e6f8af 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -3941,6 +3941,23 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
> > return 0;
> > }
> >
> > +struct bpf_link {
> Maybe call it bpf_attachment? You call the bpf_program__attach_to_blah
> and you get an attachment?
I wanted to keep it as short as possible, bpf_attachment is way too
long (it's also why as an alternative I've proposed bpf_assoc, not
bpf_association, but bpf_attach isn't great shortening).
>
> > + int (*destroy)(struct bpf_link *link);
> > +};
> > +
> > +int bpf_link__destroy(struct bpf_link *link)
> > +{
> > + int err;
> > +
> > + if (!link)
> > + return 0;
> > +
> > + err = link->destroy(link);
> > + free(link);
> > +
> > + return err;
> > +}
> > +
> > enum bpf_perf_event_ret
> > bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> > void **copy_mem, size_t *copy_size,
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index d639f47e3110..5082a5ebb0c2 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -165,6 +165,10 @@ 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_link;
> > +
> > +LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
> > +
> > struct bpf_insn;
> >
> > /*
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 2c6d835620d2..3cde850fc8da 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -167,10 +167,11 @@ LIBBPF_0.0.3 {
> >
> > LIBBPF_0.0.4 {
> > global:
> > + bpf_link__destroy;
> > + bpf_object__load_xattr;
> > btf_dump__dump_type;
> > btf_dump__free;
> > btf_dump__new;
> > btf__parse_elf;
> > - bpf_object__load_xattr;
> > libbpf_num_possible_cpus;
> > } LIBBPF_0.0.3;
> > --
> > 2.17.1
> >
^ permalink raw reply
* Re: [PATCH 1/3, net-next] net: netsec: Use page_pool API
From: David Miller @ 2019-06-28 16:43 UTC (permalink / raw)
To: brouer
Cc: ilias.apalodimas, netdev, jaswinder.singh, ard.biesheuvel,
bjorn.topel, magnus.karlsson, daniel, ast, makita.toshiaki,
jakub.kicinski, john.fastabend, maciejromanfijalkowski
In-Reply-To: <20190628150434.30da8852@carbon>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Fri, 28 Jun 2019 15:04:34 +0200
> On Fri, 28 Jun 2019 13:39:13 +0300
> Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:
>
>> Use page_pool and it's DMA mapping capabilities for Rx buffers instead
>> of netdev/napi_alloc_frag()
>>
>> Although this will result in a slight performance penalty on small sized
>> packets (~10%) the use of the API will allow to easily add XDP support.
>> The penalty won't be visible in network testing i.e ipef/netperf etc, it
>> only happens during raw packet drops.
>> Furthermore we intend to add recycling capabilities on the API
>> in the future. Once the recycling is added the performance penalty will
>> go away.
>> The only 'real' penalty is the slightly increased memory usage, since we
>> now allocate a page per packet instead of the amount of bytes we need +
>> skb metadata (difference is roughly 2kb per packet).
>> With a minimum of 4BG of RAM on the only SoC that has this NIC the
>> extra memory usage is negligible (a bit more on 64K pages)
>>
>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> ---
>> drivers/net/ethernet/socionext/Kconfig | 1 +
>> drivers/net/ethernet/socionext/netsec.c | 121 +++++++++++++++---------
>> 2 files changed, 75 insertions(+), 47 deletions(-)
>
> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Jesper this is confusing, you just asked if the code needs to be moved
around to be correct and then right now immediately afterwards you ACK
the patch.
^ permalink raw reply
* Re: [PATCH 3/3, net-next] net: netsec: add XDP support
From: David Miller @ 2019-06-28 16:44 UTC (permalink / raw)
To: brouer
Cc: ilias.apalodimas, netdev, jaswinder.singh, ard.biesheuvel,
bjorn.topel, magnus.karlsson, daniel, ast, makita.toshiaki,
jakub.kicinski, john.fastabend, maciejromanfijalkowski
In-Reply-To: <20190628153552.78a8c5ad@carbon>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Fri, 28 Jun 2019 15:35:52 +0200
> On Fri, 28 Jun 2019 13:39:15 +0300
> Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:
>
>> +static int netsec_xdp_setup(struct netsec_priv *priv, struct bpf_prog *prog,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct net_device *dev = priv->ndev;
>> + struct bpf_prog *old_prog;
>> +
>> + /* For now just support only the usual MTU sized frames */
>> + if (prog && dev->mtu > 1500) {
>> + NL_SET_ERR_MSG_MOD(extack, "Jumbo frames not supported on XDP");
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + if (netif_running(dev))
>> + netsec_netdev_stop(dev);
>> +
>> + /* Detach old prog, if any */
>> + old_prog = xchg(&priv->xdp_prog, prog);
>> + if (old_prog)
>> + bpf_prog_put(old_prog);
>> +
>> + if (netif_running(dev))
>> + netsec_netdev_open(dev);
>
> Shouldn't the if-statement be if (!netif_running(dev))
Hmmm, does netsec_netdev_stop() clear the running flag? That just
runs the driver internal routine and doesn't update IFF_UP et
al. which the core networking would do.
^ permalink raw reply
* Re: What to do when a bridge port gets its pvid deleted?
From: Florian Fainelli @ 2019-06-28 16:45 UTC (permalink / raw)
To: Vladimir Oltean, Ido Schimmel
Cc: netdev, Andrew Lunn, Vivien Didelot, stephen
In-Reply-To: <CA+h21hpPD6E_qOS-SxWKeXZKLOnN8og1BN_vSgk1+7XXQVTnBw@mail.gmail.com>
On 6/28/19 5:37 AM, Vladimir Oltean wrote:
> On Fri, 28 Jun 2019 at 15:30, Ido Schimmel <idosch@idosch.org> wrote:
>>
>> On Tue, Jun 25, 2019 at 11:49:29PM +0300, Vladimir Oltean wrote:
>>> A number of DSA drivers (BCM53XX, Microchip KSZ94XX, Mediatek MT7530
>>> at the very least), as well as Mellanox Spectrum (I didn't look at all
>>> the pure switchdev drivers) try to restore the pvid to a default value
>>> on .port_vlan_del.
>>
>> I don't know about DSA drivers, but that's not what mlxsw is doing. If
>> the VLAN that is configured as PVID is deleted from the bridge port, the
>> driver instructs the port to discard untagged and prio-tagged packets.
>> This is consistent with the bridge driver's behavior.
>>
>> We do have a flow the "restores" the PVID, but that's when a port is
>> unlinked from its bridge master. The PVID we set is 4095 which cannot be
>> configured by the 8021q / bridge driver. This is due to the way the
>> underlying hardware works. Even if a port is not bridged and used purely
>> for routing, packets still do L2 lookup first which sends them directly
>> to the router block. If PVID is not configured, untagged packets could
>> not be routed. Obviously, at egress we strip this VLAN.
>>
>>> Sure, the port stops receiving traffic when its pvid is a VLAN ID that
>>> is not installed in its hw filter, but as far as the bridge core is
>>> concerned, this is to be expected:
>>>
>>> # bridge vlan add dev swp2 vid 100 pvid untagged
>>> # bridge vlan
>>> port vlan ids
>>> swp5 1 PVID Egress Untagged
>>>
>>> swp2 1 Egress Untagged
>>> 100 PVID Egress Untagged
>>>
>>> swp3 1 PVID Egress Untagged
>>>
>>> swp4 1 PVID Egress Untagged
>>>
>>> br0 1 PVID Egress Untagged
>>> # ping 10.0.0.1
>>> PING 10.0.0.1 (10.0.0.1) 56(84) bytes of data.
>>> 64 bytes from 10.0.0.1: icmp_seq=1 ttl=64 time=0.682 ms
>>> 64 bytes from 10.0.0.1: icmp_seq=2 ttl=64 time=0.299 ms
>>> 64 bytes from 10.0.0.1: icmp_seq=3 ttl=64 time=0.251 ms
>>> 64 bytes from 10.0.0.1: icmp_seq=4 ttl=64 time=0.324 ms
>>> 64 bytes from 10.0.0.1: icmp_seq=5 ttl=64 time=0.257 ms
>>> ^C
>>> --- 10.0.0.1 ping statistics ---
>>> 5 packets transmitted, 5 received, 0% packet loss, time 4188ms
>>> rtt min/avg/max/mdev = 0.251/0.362/0.682/0.163 ms
>>> # bridge vlan del dev swp2 vid 100
>>> # bridge vlan
>>> port vlan ids
>>> swp5 1 PVID Egress Untagged
>>>
>>> swp2 1 Egress Untagged
>>>
>>> swp3 1 PVID Egress Untagged
>>>
>>> swp4 1 PVID Egress Untagged
>>>
>>> br0 1 PVID Egress Untagged
>>>
>>> # ping 10.0.0.1
>>> PING 10.0.0.1 (10.0.0.1) 56(84) bytes of data.
>>> ^C
>>> --- 10.0.0.1 ping statistics ---
>>> 8 packets transmitted, 0 received, 100% packet loss, time 7267ms
>>>
>>> What is the consensus here? Is there a reason why the bridge driver
>>> doesn't take care of this?
>>
>> Take care of what? :) Always maintaining a PVID on the bridge port? It's
>> completely OK not to have a PVID.
>>
>
> Yes, I didn't think it through during the first email. I came to the
> same conclusion in the second one.
>
>>> Do switchdev drivers have to restore the pvid to always be
>>> operational, even if their state becomes inconsistent with the upper
>>> dev? Is it just 'nice to have'? What if VID 1 isn't in the hw filter
>>> either (perfectly legal)?
>>
>> Are you saying that DSA drivers always maintain a PVID on the bridge
>> port and allow untagged traffic to ingress regardless of the bridge
>> driver's configuration? If so, I think this needs to be fixed.
>
> Well, not at the DSA core level.
> But for Microchip:
> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/dsa/microchip/ksz9477.c#n576
> For Broadcom:
> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/dsa/b53/b53_common.c#n1376
> For Mediatek:
> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/dsa/mt7530.c#n1196
>
> There might be others as well.
That sounds bogus indeed, and I bet that the two other drivers just
followed the b53 driver there. I will have to test this again and come
up with a patch eventually.
When the port leaves the bridge we do bring it back into a default PVID
(which is different than the bridge's default PVID) so that part should
be okay.
--
Florian
^ permalink raw reply
* Re: [PATCH 0/2] pull request for net: batman-adv 2019-06-27
From: David Miller @ 2019-06-28 16:45 UTC (permalink / raw)
To: sw; +Cc: netdev, b.a.t.m.a.n
In-Reply-To: <20190627103119.6969-1-sw@simonwunderlich.de>
From: Simon Wunderlich <sw@simonwunderlich.de>
Date: Thu, 27 Jun 2019 12:31:17 +0200
> here are some bugfixes which we would like to have integrated into net.
>
> Please pull or let me know of any problem!
Pulled, thanks Simon.
^ permalink raw reply
* Re: [PATCH 3/3, net-next] net: netsec: add XDP support
From: Ilias Apalodimas @ 2019-06-28 16:47 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, jaswinder.singh, ard.biesheuvel, bjorn.topel,
magnus.karlsson, daniel, ast, makita.toshiaki, jakub.kicinski,
john.fastabend, davem, maciejromanfijalkowski
In-Reply-To: <20190628153552.78a8c5ad@carbon>
On Fri, Jun 28, 2019 at 03:35:52PM +0200, Jesper Dangaard Brouer wrote:
> On Fri, 28 Jun 2019 13:39:15 +0300
> Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:
>
> > +static int netsec_xdp_setup(struct netsec_priv *priv, struct bpf_prog *prog,
> > + struct netlink_ext_ack *extack)
> > +{
> > + struct net_device *dev = priv->ndev;
> > + struct bpf_prog *old_prog;
> > +
> > + /* For now just support only the usual MTU sized frames */
> > + if (prog && dev->mtu > 1500) {
> > + NL_SET_ERR_MSG_MOD(extack, "Jumbo frames not supported on XDP");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + if (netif_running(dev))
> > + netsec_netdev_stop(dev);
> > +
> > + /* Detach old prog, if any */
> > + old_prog = xchg(&priv->xdp_prog, prog);
> > + if (old_prog)
> > + bpf_prog_put(old_prog);
> > +
> > + if (netif_running(dev))
> > + netsec_netdev_open(dev);
>
> Shouldn't the if-statement be if (!netif_running(dev))
>
> > +
This is there to restart the device if it's up already (to rebuild the rings).
This should be fine as-is
> > + return 0;
> > +}
Thanks
/Ilias
^ permalink raw reply
* Re: [PATCH 00/10] pull request for net-next: batman-adv 2019-06-27 v2
From: David Miller @ 2019-06-28 16:49 UTC (permalink / raw)
To: sw; +Cc: netdev, b.a.t.m.a.n
In-Reply-To: <20190628135604.11581-1-sw@simonwunderlich.de>
From: Simon Wunderlich <sw@simonwunderlich.de>
Date: Fri, 28 Jun 2019 15:55:54 +0200
> here is the updated feature/cleanup pull request of batman-adv for net-next
> from yesterday. Your change suggestions have been integrated into Patch 6
> of the series, everything else is unchanged.
>
> Please pull or let me know of any problem!
Pulled, thanks Simon.
I think that when you have the read_lock held, RCU is not necessary in order
to use __in6_dev_get() but I may be mistaken. Just FYI...
^ 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