* Re: net: micrel: confusion about phyids used in driver
From: Uwe Kleine-König @ 2019-07-02 20:31 UTC (permalink / raw)
To: Yuiko Oshino
Cc: netdev, Andrew Lunn, Florian Fainelli, kernel, Heiner Kallweit
In-Reply-To: <20190510072243.h6h3bgvr2ovsh5g5@pengutronix.de>
Hello Yuiko Oshino,
On Fri, May 10, 2019 at 09:22:43AM +0200, Uwe Kleine-König wrote:
> On Thu, May 09, 2019 at 11:07:45PM +0200, Andrew Lunn wrote:
> > On Thu, May 09, 2019 at 10:55:29PM +0200, Heiner Kallweit wrote:
> > > On 09.05.2019 22:29, Uwe Kleine-König wrote:
> > > > I have a board here that has a KSZ8051MLL (datasheet:
> > > > http://ww1.microchip.com/downloads/en/DeviceDoc/ksz8051mll.pdf, phyid:
> > > > 0x0022155x) assembled. The actual phyid is 0x00221556.
> > >
> > > I think the datasheets are the source of the confusion. If the
> > > datasheets for different chips list 0x0022155x as PHYID each, and
> > > authors of support for additional chips don't check the existing code,
> > > then happens what happened.
> > >
> > > However it's not a rare exception and not Microchip-specific that
> > > sometimes vendors use the same PHYID for different chips.
>
> From the vendor's POV it is even sensible to reuse the phy IDs iff the
> chips are "compatible".
>
> Assuming that the last nibble of the phy ID actually helps to
> distinguish the different (not completely) compatible chips, we need
> some more detailed information than available in the data sheets I have.
> There is one person in the recipents of this mail with an @microchip.com
> address (hint, hint!).
can you give some input here or forward to a person who can?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH net-next v6 05/15] ethtool: helper functions for netlink interface
From: Jakub Kicinski @ 2019-07-03 1:28 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, Jiri Pirko, David Miller, Andrew Lunn, Florian Fainelli,
John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <20190702163437.GE20101@unicorn.suse.cz>
On Tue, 2 Jul 2019 18:34:37 +0200, Michal Kubecek wrote:
> > >+ ret = nla_parse_nested(tb, ETHTOOL_A_HEADER_MAX, nest,
> > >+ policy ?: dflt_header_policy, extack);
> > >+ if (ret < 0)
> >
> > if (ret)
> >
> > Same remark goes to the rest of the code (also the rest of the patches),
> > in case called function cannot return positive values.
>
> The "if (ret < 0)" idiom for "on error do ..." is so ubiquitous through
> the whole kernel that I don't think it's worth it to carefully check
> which function can return a positive value and which cannot and risk
> that one day I overlook that some function. And yet another question is
> what exactly "cannot return" means: is it whenever the function does not
> return a positive value or only if it's explicitly documented not to?
>
> Looking at existing networking code, e.g. net/netfilter (except ipvs),
> net/sched or net/core/rtnetlink.c are using "if (ret < 0)" rather
> uniformly. And (as you objected to the check of genl_register_family()
> previous patch) even genetlink itself has
>
> err = genl_register_family(&genl_ctrl);
> if (err < 0)
> goto problem;
>
> in genl_init().
I agree with Jiri, if a function only returns "0, or -errno" it's
easier to parse if the error check is not only for negative values.
At least to my eyes.
What I'm not sure about is whether we want to delay the merging of this
interface over this..
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Song Liu @ 2019-07-02 23:48 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Kees Cook, linux-security@vger.kernel.org, Networking, bpf,
Alexei Starovoitov, Daniel Borkmann, Kernel Team, Lorenz Bauer,
Jann Horn, Greg KH, Linux API
In-Reply-To: <CALCETrXTta26CTtEDnzvtd03-WOGdXcnsAogP8JjLkcj4-mHvg@mail.gmail.com>
> On Jul 3, 2019, at 5:32 AM, Andy Lutomirski <luto@kernel.org> wrote:
>
> On Tue, Jul 2, 2019 at 2:04 PM Kees Cook <keescook@chromium.org> wrote:
>>
>> On Mon, Jul 01, 2019 at 06:59:13PM -0700, Andy Lutomirski wrote:
>>> I think I'm understanding your motivation. You're not trying to make
>>> bpf() generically usable without privilege -- you're trying to create
>>> a way to allow certain users to access dangerous bpf functionality
>>> within some limits.
>>>
>>> That's a perfectly fine goal, but I think you're reinventing the
>>> wheel, and the wheel you're reinventing is quite complicated and
>>> already exists. I think you should teach bpftool to be secure when
>>> installed setuid root or with fscaps enabled and put your policy in
>>> bpftool. If you want to harden this a little bit, it would seem
>>> entirely reasonable to add a new CAP_BPF_ADMIN and change some, but
>>> not all, of the capable() checks to check CAP_BPF_ADMIN instead of the
>>> capabilities that they currently check.
>>
>> If finer grained controls are wanted, it does seem like the /dev/bpf
>> path makes the most sense. open, request abilities, use fd. The open can
>> be mediated by DAC and LSM. The request can be mediated by LSM. This
>> provides a way to add policy at the LSM level and at the tool level.
>> (i.e. For tool-level controls: leave LSM wide open, make /dev/bpf owned
>> by "bpfadmin" and bpftool becomes setuid "bpfadmin". For fine-grained
>> controls, leave /dev/bpf wide open and add policy to SELinux, etc.)
>>
>> With only a new CAP, you don't get the fine-grained controls. (The
>> "request abilities" part is the key there.)
>
> Sure you do: the effective set. It has somewhat bizarre defaults, but
> I don't think that's a real problem. Also, this wouldn't be like
> CAP_DAC_READ_SEARCH -- you can't accidentally use your BPF caps.
>
> I think that a /dev capability-like object isn't totally nuts, but I
> think we should do it well, and this patch doesn't really achieve
> that. But I don't think bpf wants fine-grained controls like this at
> all -- as I pointed upthread, a fine-grained solution really wants
> different treatment for the different capable() checks, and a bunch of
> them won't resemble capabilities or /dev/bpf at all.
Thanks everyone again for great inputs. We will discuss this again and
respin the set.
Best,
Song
^ permalink raw reply
* RE: net: micrel: confusion about phyids used in driver
From: Yuiko.Oshino @ 2019-07-02 20:55 UTC (permalink / raw)
To: u.kleine-koenig
Cc: netdev, andrew, f.fainelli, kernel, hkallweit1, Ravi.Hegde,
Tristram.Ha
In-Reply-To: <20190702203152.gviukfldjhdnmu7j@pengutronix.de>
>-----Original Message-----
>From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>Sent: Tuesday, July 2, 2019 4:32 PM
>To: Yuiko Oshino - C18177 <Yuiko.Oshino@microchip.com>
>Cc: netdev@vger.kernel.org; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli
><f.fainelli@gmail.com>; kernel@pengutronix.de; Heiner Kallweit
><hkallweit1@gmail.com>
>Subject: Re: net: micrel: confusion about phyids used in driver
>
>External E-Mail
>
>
>Hello Yuiko Oshino,
>
>On Fri, May 10, 2019 at 09:22:43AM +0200, Uwe Kleine-König wrote:
>> On Thu, May 09, 2019 at 11:07:45PM +0200, Andrew Lunn wrote:
>> > On Thu, May 09, 2019 at 10:55:29PM +0200, Heiner Kallweit wrote:
>> > > On 09.05.2019 22:29, Uwe Kleine-König wrote:
>> > > > I have a board here that has a KSZ8051MLL (datasheet:
>> > > > http://ww1.microchip.com/downloads/en/DeviceDoc/ksz8051mll.pdf,
>phyid:
>> > > > 0x0022155x) assembled. The actual phyid is 0x00221556.
>> > >
>> > > I think the datasheets are the source of the confusion. If the
>> > > datasheets for different chips list 0x0022155x as PHYID each, and
>> > > authors of support for additional chips don't check the existing
>> > > code, then happens what happened.
>> > >
>> > > However it's not a rare exception and not Microchip-specific that
>> > > sometimes vendors use the same PHYID for different chips.
>>
>> From the vendor's POV it is even sensible to reuse the phy IDs iff the
>> chips are "compatible".
>>
>> Assuming that the last nibble of the phy ID actually helps to
>> distinguish the different (not completely) compatible chips, we need
>> some more detailed information than available in the data sheets I have.
>> There is one person in the recipents of this mail with an
>> @microchip.com address (hint, hint!).
>
>can you give some input here or forward to a person who can?
>
>Best regards
>Uwe
>
>--
>Pengutronix e.K. | Uwe Kleine-König |
>Industrial Linux Solutions | http://www.pengutronix.de/ |
Hello Uwe,
I forward this to the team.
Best regards,
Yuiko
^ permalink raw reply
* [PATCH v2] net: don't warn in inet diag when IPV6 is disabled
From: Stephen Hemminger @ 2019-07-02 22:20 UTC (permalink / raw)
To: davem, kuznet, yoshfuji; +Cc: netdev, Stephen Hemminger
If IPV6 was disabled, then ss command would cause a kernel warning
because the command was attempting to dump IPV6 socket information.
The fix is to just remove the warning.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202249
Fixes: 432490f9d455 ("net: ip, diag -- Add diag interface for raw sockets")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
net/ipv4/raw_diag.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/ipv4/raw_diag.c b/net/ipv4/raw_diag.c
index 899e34ceb560..e35736b99300 100644
--- a/net/ipv4/raw_diag.c
+++ b/net/ipv4/raw_diag.c
@@ -24,9 +24,6 @@ raw_get_hashinfo(const struct inet_diag_req_v2 *r)
return &raw_v6_hashinfo;
#endif
} else {
- pr_warn_once("Unexpected inet family %d\n",
- r->sdiag_family);
- WARN_ON_ONCE(1);
return ERR_PTR(-EINVAL);
}
}
--
2.20.1
^ permalink raw reply related
* Re: Validation of forward_delay seems wrong...
From: Nikolay Aleksandrov @ 2019-07-02 21:19 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Roopa Prabhu, bridge, netdev
In-Reply-To: <20190702204705.GC28471@lunn.ch>
On 02/07/2019 23:47, Andrew Lunn wrote:
> Hi Nikolay
>
> The man page says that the bridge forward_delay is in units of
> seconds, and should be between 2 and 30.
>
> I've tested on a couple of different kernel versions, and this appears
> to be not working correctly:
>
> ip link set br0 type bridge forward_delay 2
> RTNETLINK answers: Numerical result out of range
>
> ip link set br0 type bridge forward_delay 199
> RTNETLINK answers: Numerical result out of range
>
> ip link set br0 type bridge forward_delay 200
> #
>
> ip link set br0 type bridge forward_delay 3000
> #
>
> ip link set br0 type bridge forward_delay 3001
> RTNETLINK answers: Numerical result out of range
>
> I've not checked what delay is actually being used here, but clearly
> something is mixed up.
>
> grep HZ .config
> CONFIG_HZ_PERIODIC=y
> # CONFIG_NO_HZ_IDLE is not set
> # CONFIG_NO_HZ_FULL is not set
> # CONFIG_NO_HZ is not set
> CONFIG_HZ_FIXED=0
> CONFIG_HZ_100=y
> # CONFIG_HZ_200 is not set
> # CONFIG_HZ_250 is not set
> # CONFIG_HZ_300 is not set
> # CONFIG_HZ_500 is not set
> # CONFIG_HZ_1000 is not set
> CONFIG_HZ=100
>
> Thanks
> Andrew
>
Hi Andrew,
The man page is wrong, these have been in USER_HZ scaled clock_t format from the beginning.
TBH a lot of the time/delay bridge config options are messed up like that.
We've been discussing adding special _ms versions in iproute2 to make them
more user-friendly and understandable. Will cook a patch for the man page.
Cheers,
Nik
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Andy Lutomirski @ 2019-07-02 21:32 UTC (permalink / raw)
To: Kees Cook
Cc: Andy Lutomirski, Song Liu, linux-security@vger.kernel.org,
Networking, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team,
Lorenz Bauer, Jann Horn, Greg KH, Linux API
In-Reply-To: <201907021115.DCD56BBABB@keescook>
On Tue, Jul 2, 2019 at 2:04 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, Jul 01, 2019 at 06:59:13PM -0700, Andy Lutomirski wrote:
> > I think I'm understanding your motivation. You're not trying to make
> > bpf() generically usable without privilege -- you're trying to create
> > a way to allow certain users to access dangerous bpf functionality
> > within some limits.
> >
> > That's a perfectly fine goal, but I think you're reinventing the
> > wheel, and the wheel you're reinventing is quite complicated and
> > already exists. I think you should teach bpftool to be secure when
> > installed setuid root or with fscaps enabled and put your policy in
> > bpftool. If you want to harden this a little bit, it would seem
> > entirely reasonable to add a new CAP_BPF_ADMIN and change some, but
> > not all, of the capable() checks to check CAP_BPF_ADMIN instead of the
> > capabilities that they currently check.
>
> If finer grained controls are wanted, it does seem like the /dev/bpf
> path makes the most sense. open, request abilities, use fd. The open can
> be mediated by DAC and LSM. The request can be mediated by LSM. This
> provides a way to add policy at the LSM level and at the tool level.
> (i.e. For tool-level controls: leave LSM wide open, make /dev/bpf owned
> by "bpfadmin" and bpftool becomes setuid "bpfadmin". For fine-grained
> controls, leave /dev/bpf wide open and add policy to SELinux, etc.)
>
> With only a new CAP, you don't get the fine-grained controls. (The
> "request abilities" part is the key there.)
Sure you do: the effective set. It has somewhat bizarre defaults, but
I don't think that's a real problem. Also, this wouldn't be like
CAP_DAC_READ_SEARCH -- you can't accidentally use your BPF caps.
I think that a /dev capability-like object isn't totally nuts, but I
think we should do it well, and this patch doesn't really achieve
that. But I don't think bpf wants fine-grained controls like this at
all -- as I pointed upthread, a fine-grained solution really wants
different treatment for the different capable() checks, and a bunch of
them won't resemble capabilities or /dev/bpf at all.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 6/9] libbpf: use negative fd to specify missing BTF
From: Stanislav Fomichev @ 2019-07-02 23:18 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team
In-Reply-To: <20190702225733.GK6757@mini-arch>
On 07/02, Stanislav Fomichev wrote:
> On 05/29, Andrii Nakryiko wrote:
> > 0 is a valid FD, so it's better to initialize it to -1, as is done in
> > other places. Also, technically, BTF type ID 0 is valid (it's a VOID
> > type), so it's more reliable to check btf_fd, instead of
> > btf_key_type_id, to determine if there is any BTF associated with a map.
> >
> > Acked-by: Song Liu <songliubraving@fb.com>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > tools/lib/bpf/libbpf.c | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index c972fa10271f..a27a0351e595 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -1751,7 +1751,7 @@ bpf_object__create_maps(struct bpf_object *obj)
> > create_attr.key_size = def->key_size;
> > create_attr.value_size = def->value_size;
> > create_attr.max_entries = def->max_entries;
> > - create_attr.btf_fd = 0;
> > + create_attr.btf_fd = -1;
> > create_attr.btf_key_type_id = 0;
> > create_attr.btf_value_type_id = 0;
> > if (bpf_map_type__is_map_in_map(def->type) &&
> > @@ -1765,11 +1765,11 @@ bpf_object__create_maps(struct bpf_object *obj)
> > }
> >
> > *pfd = bpf_create_map_xattr(&create_attr);
> > - if (*pfd < 0 && create_attr.btf_key_type_id) {
> > + if (*pfd < 0 && create_attr.btf_fd >= 0) {
> > cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
> > pr_warning("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
> > map->name, cp, errno);
> > - create_attr.btf_fd = 0;
> > + create_attr.btf_fd = -1;
> This breaks libbpf compatibility with the older kernels. If the kernel
> doesn't know about btf_fd and we set it to -1, then CHECK_ATTR
> fails :-(
>
> Any objections to converting BTF retries to bpf_capabilities and then
> knowingly passing bft_fd==0 or proper fd?
Oh, nevermind, it looks like you fixed it already in e55d54f43d3f.
> > create_attr.btf_key_type_id = 0;
> > create_attr.btf_value_type_id = 0;
> > map->btf_key_type_id = 0;
> > @@ -2053,6 +2053,9 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> > char *log_buf;
> > int ret;
> >
> > + if (!insns || !insns_cnt)
> > + return -EINVAL;
> > +
> > memset(&load_attr, 0, sizeof(struct bpf_load_program_attr));
> > load_attr.prog_type = prog->type;
> > load_attr.expected_attach_type = prog->expected_attach_type;
> > @@ -2063,7 +2066,7 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> > load_attr.license = license;
> > load_attr.kern_version = kern_version;
> > load_attr.prog_ifindex = prog->prog_ifindex;
> > - load_attr.prog_btf_fd = prog->btf_fd >= 0 ? prog->btf_fd : 0;
> > + load_attr.prog_btf_fd = prog->btf_fd;
> > load_attr.func_info = prog->func_info;
> > load_attr.func_info_rec_size = prog->func_info_rec_size;
> > load_attr.func_info_cnt = prog->func_info_cnt;
> > @@ -2072,8 +2075,6 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> > load_attr.line_info_cnt = prog->line_info_cnt;
> > load_attr.log_level = prog->log_level;
> > load_attr.prog_flags = prog->prog_flags;
> > - if (!load_attr.insns || !load_attr.insns_cnt)
> > - return -EINVAL;
> >
> > retry_load:
> > log_buf = malloc(log_buf_size);
> > --
> > 2.17.1
> >
^ permalink raw reply
* Re: [PATCH net-next] Revert "r8169: improve handling VLAN tag"
From: David Miller @ 2019-07-02 22:28 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, netdev, viro
In-Reply-To: <df6fcca2-6db6-0d76-deb2-6b2e98e8bf54@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Tue, 2 Jul 2019 07:59:17 +0200
> This reverts commit 759d095741721888b6ee51afa74e0a66ce65e974.
>
> The patch was based on a misunderstanding. As Al Viro pointed out [0]
> it's simply wrong on big endian. So let's revert it.
>
> [0] https://marc.info/?t=156200975600004&r=1&w=2
>
> Reported-by: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] r8169: add random MAC address fallback
From: David Miller @ 2019-07-02 22:29 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, netdev
In-Reply-To: <60c26de4-23bc-a94b-d4a0-1216d8053e1f@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Tue, 2 Jul 2019 18:49:14 +0200
> On 02.07.2019 08:18, Heiner Kallweit wrote:
>>>From 1c8bacf724f1450e5256c68fbff407305faf9cbd Mon Sep 17 00:00:00 2001
>>
>>
>>
>
> Sorry, something went wrong when preparing the commit message. I'll resubmit.
It helps to speak closer to the microphone.
^ permalink raw reply
* Re: [PATCH v2 net-next] r8169: add random MAC address fallback
From: David Miller @ 2019-07-02 22:28 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, netdev
In-Reply-To: <637b6d1f-85b1-dc9a-8e77-68ffcd82eae9@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Tue, 2 Jul 2019 20:46:09 +0200
> It was reported that the GPD MicroPC is broken in a way that no valid
> MAC address can be read from the network chip. The vendor driver deals
> with this by assigning a random MAC address as fallback. So let's do
> the same.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - fix broken commit message
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net: core: page_pool: add user refcnt and reintroduce page_pool_destroy
From: Ilias Apalodimas @ 2019-07-02 21:15 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Ivan Khoronzhuk, netdev, grygorii.strashko, jakub.kicinski,
daniel, john.fastabend, ast, linux-kernel, linux-omap
In-Reply-To: <20190702230241.3be6d787@carbon>
Hi Jesper,
Getting late here, i'll respond in detail tomorrow. One point though
[...]
>
> This special use-case, seems confined to your driver. And Ilias told me
> that XDP is not really a performance benefit for this driver as the HW
> PPS-limit is hit before the XDP and netstack limit. I ask, does it
> make sense to add XDP to this driver, if it complicates the code for
> everybody else?
I think yes. This is a widely used driver on TI embedded devices so having XDP
to play along is a nice feature. It's also the first and only armv7 we have
supporting this. Ivan already found a couple of issues due to the 32-bit
architecture he is trying to fix, i think there's real benefit in having that,
performance aside.
I fully agree we should not impact the performance of the API to support a
special hardware though. I'll have a look on the 2 solutions tomorrow, but the
general approach on this one should be 'the simpler the better'
Cheers
/Ilias
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH 1/2 nf-next v3] netfilter: nft_meta: Add NFT_META_BRI_IIFVPROTO support
From: Pablo Neira Ayuso @ 2019-07-02 23:46 UTC (permalink / raw)
To: wenxu; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <1561682975-21790-1-git-send-email-wenxu@ucloud.cn>
On Fri, Jun 28, 2019 at 08:49:34AM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> This patch provide a meta to get the bridge vlan proto
>
> nft add rule bridge firewall zones counter meta br_vlan_proto 0x8100
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v4 bpf-next] bpf: Add support for fq's EDT to HBM
From: Yonghong Song @ 2019-07-02 23:02 UTC (permalink / raw)
To: Lawrence Brakmo, netdev
Cc: Alexei Starovoitov, Daniel Borkmann, Eric Dumazet, Kernel Team
In-Reply-To: <20190702220952.3929270-1-brakmo@fb.com>
On 7/2/19 3:09 PM, brakmo wrote:
> Adds support for fq's Earliest Departure Time to HBM (Host Bandwidth
> Manager). Includes a new BPF program supporting EDT, and also updates
> corresponding programs.
>
> It will drop packets with an EDT of more than 500us in the future
> unless the packet belongs to a flow with less than 2 packets in flight.
> This is done so each flow has at least 2 packets in flight, so they
> will not starve, and also to help prevent delayed ACK timeouts.
>
> It will also work with ECN enabled traffic, where the packets will be
> CE marked if their EDT is more than 50us in the future.
>
> The table below shows some performance numbers. The flows are back to
> back RPCS. One server sending to another, either 2 or 4 flows.
> One flow is a 10KB RPC, the rest are 1MB RPCs. When there are more
> than one flow of a given RPC size, the numbers represent averages.
>
> The rate limit applies to all flows (they are in the same cgroup).
> Tests ending with "-edt" ran with the new BPF program supporting EDT.
> Tests ending with "-hbt" ran on top HBT qdisc with the specified rate
> (i.e. no HBM). The other tests ran with the HBM BPF program included
> in the HBM patch-set.
>
> EDT has limited value when using DCTCP, but it helps in many cases when
> using Cubic. It usually achieves larger link utilization and lower
> 99% latencies for the 1MB RPCs.
> HBM ends up queueing a lot of packets with its default parameter values,
> reducing the goodput of the 10KB RPCs and increasing their latency. Also,
> the RTTs seen by the flows are quite large.
>
> Aggr 10K 10K 10K 1MB 1MB 1MB
> Limit rate drops RTT rate P90 P99 rate P90 P99
> Test rate Flows Mbps % us Mbps us us Mbps ms ms
> -------- ---- ----- ---- ----- --- ---- ---- ---- ---- ---- ----
> cubic 1G 2 904 0.02 108 257 511 539 647 13.4 24.5
> cubic-edt 1G 2 982 0.01 156 239 656 967 743 14.0 17.2
> dctcp 1G 2 977 0.00 105 324 408 744 653 14.5 15.9
> dctcp-edt 1G 2 981 0.01 142 321 417 811 660 15.7 17.0
> cubic-htb 1G 2 919 0.00 1825 40 2822 4140 879 9.7 9.9
>
> cubic 200M 2 155 0.30 220 81 532 655 74 283 450
> cubic-edt 200M 2 188 0.02 222 87 1035 1095 101 84 85
> dctcp 200M 2 188 0.03 111 77 912 939 111 76 325
> dctcp-edt 200M 2 188 0.03 217 74 1416 1738 114 76 79
> cubic-htb 200M 2 188 0.00 5015 8 14ms 15ms 180 48 50
>
> cubic 1G 4 952 0.03 110 165 516 546 262 38 154
> cubic-edt 1G 4 973 0.01 190 111 1034 1314 287 65 79
> dctcp 1G 4 951 0.00 103 180 617 905 257 37 38
> dctcp-edt 1G 4 967 0.00 163 151 732 1126 272 43 55
> cubic-htb 1G 4 914 0.00 3249 13 7ms 8ms 300 29 34
>
> cubic 5G 4 4236 0.00 134 305 490 624 1310 10 17
> cubic-edt 5G 4 4865 0.00 156 306 425 759 1520 10 16
> dctcp 5G 4 4936 0.00 128 485 221 409 1484 7 9
> dctcp-edt 5G 4 4924 0.00 148 390 392 623 1508 11 26
>
> v1 -> v2: Incorporated Andrii's suggestions
> v2 -> v3: Incorporated Yonghong's suggestions
> v3 -> v4: Removed credit update that is not needed
>
> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
* Re: [PATCH net] net: don't warn in inet diag when IPV6 is disabled
From: David Miller @ 2019-07-02 21:23 UTC (permalink / raw)
To: stephen; +Cc: kuznet, yoshfuji, netdev
In-Reply-To: <20190701152303.4031-1-stephen@networkplumber.org>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 1 Jul 2019 08:23:03 -0700
> @@ -19,9 +19,11 @@ raw_get_hashinfo(const struct inet_diag_req_v2 *r)
> {
> if (r->sdiag_family == AF_INET) {
> return &raw_v4_hashinfo;
> -#if IS_ENABLED(CONFIG_IPV6)
> } else if (r->sdiag_family == AF_INET6) {
> +#if IS_ENABLED(CONFIG_IPV6)
> return &raw_v6_hashinfo;
> +#else
> + return ERR_PTR(-EOPNOTSUPP);
> #endif
> } else {
> pr_warn_once("Unexpected inet family %d\n",
Let's make some consistency in this area please.
The inet_diag code returns -EINVAL, and that's been that way forever.
It also doesn't print a weird warning for unexpected sdiag_family
values outside of AF_INET and AF_INET6.
That's been that way for so long that's probably the behavior to
revolve everything around.
Therefore, please just get rid of the warning message instead of
all of these other changes.
Thank you.
^ permalink raw reply
* Re: [PATCH] net: stmmac: make "snps,reset-delays-us" optional again
From: David Miller @ 2019-07-02 22:23 UTC (permalink / raw)
To: martin.blumenstingl
Cc: netdev, colin.king, peppe.cavallaro, alexandre.torgue, joabreu,
linux-arm-kernel, linux-kernel
In-Reply-To: <20190701224225.19701-1-martin.blumenstingl@googlemail.com>
From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date: Tue, 2 Jul 2019 00:42:25 +0200
> Commit 760f1dc2958022 ("net: stmmac: add sanity check to
> device_property_read_u32_array call") introduced error checking of the
> device_property_read_u32_array() call in stmmac_mdio_reset().
> This results in the following error when the "snps,reset-delays-us"
> property is not defined in devicetree:
> invalid property snps,reset-delays-us
>
> This sanity check made sense until commit 84ce4d0f9f55b4 ("net: stmmac:
> initialize the reset delay array") ensured that there are fallback
> values for the reset delay if the "snps,reset-delays-us" property is
> absent. That was at the cost of making that property mandatory though.
>
> Drop the sanity check for device_property_read_u32_array() and thus make
> the "snps,reset-delays-us" property optional again (avoiding the error
> message while loading the stmmac driver with a .dtb where the property
> is absent).
>
> Fixes: 760f1dc2958022 ("net: stmmac: add sanity check to device_property_read_u32_array call")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> This is a fix for a patch in net-next and should either go into net-next
> or 5.3-rcX.
Ok, applied to net-next.
^ permalink raw reply
* Re: [PATCH V2] net: usb: asix: init MAC address buffers
From: David Miller @ 2019-07-02 22:25 UTC (permalink / raw)
To: tranmanphong
Cc: dcbw, netdev, linux-usb, glider, linux-kernel-mentees,
linux-kernel, lynxis, marcel.ziswiler, skhan,
syzbot+8a3fc6674bbc3978ed4e, syzkaller-bugs, yang.wei9, zhang.run
In-Reply-To: <20190702001008.26048-1-tranmanphong@gmail.com>
From: Phong Tran <tranmanphong@gmail.com>
Date: Tue, 2 Jul 2019 07:10:08 +0700
> This is for fixing bug KMSAN: uninit-value in ax88772_bind
>
> Tested by
> https://groups.google.com/d/msg/syzkaller-bugs/aFQurGotng4/eB_HlNhhCwAJ
>
> Reported-by: syzbot+8a3fc6674bbc3978ed4e@syzkaller.appspotmail.com
>
> syzbot found the following crash on:
>
> HEAD commit: f75e4cfe kmsan: use kmsan_handle_urb() in urb.c
> git tree: kmsan
...
> Signed-off-by: Phong Tran <tranmanphong@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next 0/2] Mellanox, mlx5 devlink versions query
From: Jakub Kicinski @ 2019-07-03 0:51 UTC (permalink / raw)
To: Saeed Mahameed; +Cc: David S. Miller, netdev@vger.kernel.org
In-Reply-To: <20190702235442.1925-1-saeedm@mellanox.com>
On Tue, 2 Jul 2019 23:55:07 +0000, Saeed Mahameed wrote:
> Hi Dave,
>
> This humble 2 patch series from Shay adds the support for devlink fw
> versions query to mlx5 driver.
>
> In the first patch we implement the needed fw commands to support this
> feature.
> In the 2nd patch we implement the devlink callbacks themselves.
>
> I am not sending this as a pull request since i am not sure when my next
> pull request is going to be ready, and these two patches are straight
> forward net-next patches.
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
^ permalink raw reply
* Re: [PATCH net-next] mlxsw: spectrum_ptp: Fix validation in mlxsw_sp1_ptp_packet_finish()
From: David Miller @ 2019-07-02 22:32 UTC (permalink / raw)
To: petrm; +Cc: netdev, colin.king, idosch
In-Reply-To: <3f905fb4d20f266f777ef56648f7615edaaffc9c.1562094119.git.petrm@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
Date: Tue, 2 Jul 2019 19:06:47 +0000
> Before mlxsw_sp1_ptp_packet_finish() sends the packet back, it validates
> whether the corresponding port is still valid. However the condition is
> incorrect: when mlxsw_sp_port == NULL, the code dereferences the port to
> compare it to skb->dev.
>
> The condition needs to check whether the port is present and skb->dev still
> refers to that port (or else is NULL). If that does not hold, bail out.
> Add a pair of parentheses to fix the condition.
>
> Fixes: d92e4e6e33c8 ("mlxsw: spectrum: PTP: Support timestamping on Spectrum-1")
> Reported-by: Colin Ian King <colin.king@canonical.com>
> Signed-off-by: Petr Machata <petrm@mellanox.com>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net 0/2] macsec: fix some bugs in the receive path
From: David Miller @ 2019-07-02 21:13 UTC (permalink / raw)
To: ast; +Cc: netdev, sd
In-Reply-To: <04d1ca9ff24de717746b5e21573656f6cb7069d6.camel@domdv.de>
From: Andreas Steinmetz <ast@domdv.de>
Date: Sun, 30 Jun 2019 22:46:38 +0200
> This series fixes some bugs in the receive path of macsec. The first
> is a use after free when processing macsec frames with a SecTAG that
> has the TCI E bit set but the C bit clear. In the 2nd bug, the driver
> leaves an invalid checksumming state after decrypting the packet.
>
> This is a combined effort of Sabrina Dubroca <sd@queasysnail.net> and me.
Series applied and queued up for -stable, thank you.
^ permalink raw reply
* Re: [PATCH V2] include: linux: Regularise the use of FIELD_SIZEOF macro
From: Kees Cook @ 2019-07-02 16:33 UTC (permalink / raw)
To: Joe Perches
Cc: Alexey Dobriyan, Andreas Dilger, Andrew Morton, Shyam Saini,
kernel-hardening, linux-kernel, linux-arm-kernel, linux-mips,
intel-gvt-dev, intel-gfx, dri-devel, netdev, linux-ext4, devel,
linux-mm, linux-sctp, bpf, kvm, mayhs11saini
In-Reply-To: <c3b83ba7f9b003dd4fb9cad885461ce93165dc04.camel@perches.com>
On Sat, Jun 29, 2019 at 09:45:10AM -0700, Joe Perches wrote:
> On Sat, 2019-06-29 at 17:25 +0300, Alexey Dobriyan wrote:
> > On Tue, Jun 11, 2019 at 03:00:10PM -0600, Andreas Dilger wrote:
> > > On Jun 11, 2019, at 2:48 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> > > > On Wed, 12 Jun 2019 01:08:36 +0530 Shyam Saini <shyam.saini@amarulasolutions.com> wrote:
> > > I did a check, and FIELD_SIZEOF() is used about 350x, while sizeof_field()
> > > is about 30x, and SIZEOF_FIELD() is only about 5x.
> > >
> > > That said, I'm much more in favour of "sizeof_field()" or "sizeof_member()"
> > > than FIELD_SIZEOF(). Not only does that better match "offsetof()", with
> > > which it is closely related, but is also closer to the original "sizeof()".
> > >
> > > Since this is a rather trivial change, it can be split into a number of
> > > patches to get approval/landing via subsystem maintainers, and there is no
> > > huge urgency to remove the original macros until the users are gone. It
> > > would make sense to remove SIZEOF_FIELD() and sizeof_field() quickly so
> > > they don't gain more users, and the remaining FIELD_SIZEOF() users can be
> > > whittled away as the patches come through the maintainer trees.
> >
> > The signature should be
> >
> > sizeof_member(T, m)
> >
> > it is proper English,
> > it is lowercase, so is easier to type,
> > it uses standard term (member, not field),
> > it blends in with standard "sizeof" operator,
>
> yes please.
>
> Also, a simple script conversion applied
> immediately after an rc1 might be easiest
> rather than individual patches.
This seems reasonable to me. I think the patch steps would be:
1) implement sizeof_member(T, m) as a stand-alone macro
2) do a scripted replacement of all identical macros.
3) remove all the identical macros.
Step 2 can be a patch that includes the script used to do the
replacement. That way Linus can choose to just run the script instead of
taking the patch.
--
Kees Cook
^ permalink raw reply
* Re: [PATCH net 0/6] gtp: fix several bugs
From: Harald Welte @ 2019-07-03 1:08 UTC (permalink / raw)
To: Taehee Yoo; +Cc: davem, pablo, Pau Espin, osmocom-net-gprs, netdev
In-Reply-To: <20190702152034.22412-1-ap420073@gmail.com>
Hi Taehee,
On Wed, Jul 03, 2019 at 12:20:34AM +0900, Taehee Yoo wrote:
> This patch series fixes several bugs in the gtp module.
thanks a lot for your patches, they are much appreciated.
They look valid to me after a brief initial review.
However, I'm currently on holidays and don't have the ability to test
any patches until my return on July 17. Maybe Pablo and/or Pau can have
a look meanwhile? Thanks in advance.
Regards,
Harald
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* Re: [PATCH net] net: don't warn in inet diag when IPV6 is disabled
From: Stephen Hemminger @ 2019-07-02 22:14 UTC (permalink / raw)
To: David Miller; +Cc: kuznet, yoshfuji, netdev
In-Reply-To: <20190702.142347.1440800997923616328.davem@davemloft.net>
On Tue, 02 Jul 2019 14:23:47 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Mon, 1 Jul 2019 08:23:03 -0700
>
> > @@ -19,9 +19,11 @@ raw_get_hashinfo(const struct inet_diag_req_v2 *r)
> > {
> > if (r->sdiag_family == AF_INET) {
> > return &raw_v4_hashinfo;
> > -#if IS_ENABLED(CONFIG_IPV6)
> > } else if (r->sdiag_family == AF_INET6) {
> > +#if IS_ENABLED(CONFIG_IPV6)
> > return &raw_v6_hashinfo;
> > +#else
> > + return ERR_PTR(-EOPNOTSUPP);
> > #endif
> > } else {
> > pr_warn_once("Unexpected inet family %d\n",
>
> Let's make some consistency in this area please.
>
> The inet_diag code returns -EINVAL, and that's been that way forever.
> It also doesn't print a weird warning for unexpected sdiag_family
> values outside of AF_INET and AF_INET6.
>
> That's been that way for so long that's probably the behavior to
> revolve everything around.
>
> Therefore, please just get rid of the warning message instead of
> all of these other changes.
>
> Thank you.
Sure, that makes sense.
^ permalink raw reply
* Re: Validation of forward_delay seems wrong...
From: Nikolay Aleksandrov @ 2019-07-02 21:21 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Roopa Prabhu, bridge, netdev
In-Reply-To: <55f24bfb-4239-dda8-24f8-26b6b2fa9f9e@cumulusnetworks.com>
On 03/07/2019 00:19, Nikolay Aleksandrov wrote:
> On 02/07/2019 23:47, Andrew Lunn wrote:
>> Hi Nikolay
>>
>> The man page says that the bridge forward_delay is in units of
>> seconds, and should be between 2 and 30.
>>
>> I've tested on a couple of different kernel versions, and this appears
>> to be not working correctly:
>>
>> ip link set br0 type bridge forward_delay 2
>> RTNETLINK answers: Numerical result out of range
>>
>> ip link set br0 type bridge forward_delay 199
>> RTNETLINK answers: Numerical result out of range
>>
>> ip link set br0 type bridge forward_delay 200
>> #
>>
>> ip link set br0 type bridge forward_delay 3000
>> #
>>
>> ip link set br0 type bridge forward_delay 3001
>> RTNETLINK answers: Numerical result out of range
>>
>> I've not checked what delay is actually being used here, but clearly
>> something is mixed up.
>>
>> grep HZ .config
>> CONFIG_HZ_PERIODIC=y
>> # CONFIG_NO_HZ_IDLE is not set
>> # CONFIG_NO_HZ_FULL is not set
>> # CONFIG_NO_HZ is not set
>> CONFIG_HZ_FIXED=0
>> CONFIG_HZ_100=y
>> # CONFIG_HZ_200 is not set
>> # CONFIG_HZ_250 is not set
>> # CONFIG_HZ_300 is not set
>> # CONFIG_HZ_500 is not set
>> # CONFIG_HZ_1000 is not set
>> CONFIG_HZ=100
>>
>> Thanks
>> Andrew
>>
>
> Hi Andrew,
> The man page is wrong, these have been in USER_HZ scaled clock_t format from the beginning.
> TBH a lot of the time/delay bridge config options are messed up like that.
> We've been discussing adding special _ms versions in iproute2 to make them
> more user-friendly and understandable. Will cook a patch for the man page.
>
> Cheers,
> Nik
>
>
Err, I meant it is seconds just scaled, if it wasn't clear.
^ permalink raw reply
* Re: [PATCH net-next] tipc: remove ub->ubsock checks
From: David Miller @ 2019-07-02 22:13 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, jon.maloy, ying.xue, tipc-discussion
In-Reply-To: <d59889f395b2c224131046c832fe1a8056209107.1562000239.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Tue, 2 Jul 2019 00:57:19 +0800
> Both tipc_udp_enable and tipc_udp_disable are called under rtnl_lock,
> ub->ubsock could never be NULL in tipc_udp_disable and cleanup_bearer,
> so remove the check.
>
> Also remove the one in tipc_udp_enable by adding "free" label.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Looks good, applied.
^ 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