* Re: [PATCH net-next v6 07/23] zinc: ChaCha20 ARM and ARM64 implementations
From: Jason A. Donenfeld @ 2018-09-26 15:41 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Herbert Xu, Thomas Gleixner, LKML, Netdev,
Linux Crypto Mailing List, David Miller, Greg Kroah-Hartman,
Samuel Neves, Andrew Lutomirski, Jean-Philippe Aumasson,
Russell King - ARM Linux, linux-arm-kernel
In-Reply-To: <CAKv+Gu8ih-TsASRGqK+ST_5+EQ0=Zo-zhGCadOdGyPjucMFTCg@mail.gmail.com>
On Wed, Sep 26, 2018 at 4:02 PM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> I don't think it makes sense to keep
> it simple now and add the complexity later (and the same concern
> applies to async support btw).
Ugh, no. I don't want to add needless complexity, period. Zinc is
synchronous, not asynchronous. It provides software implementations.
That's what it does. While many of your reviews have been useful, many
of your comments indicate some desire to change and mold the purpose
and focus of Zinc away from Zinc's intents. Stop that. It's not going
to become a bloated mess of "things Ard wanted and quipped about on
LKML." Things like these only serve to filibuster the patchset
indefinitely. But maybe that's what you'd like all along? Hard to
tell, honestly. So, no, sorry, Zinc isn't gaining an async interface
right now.
^ permalink raw reply
* Re: [PATCH net-next v6 07/23] zinc: ChaCha20 ARM and ARM64 implementations
From: Ard Biesheuvel @ 2018-09-26 15:41 UTC (permalink / raw)
To: Jason A. Donenfeld, Herbert Xu, Thomas Gleixner
Cc: Linux Kernel Mailing List, <netdev@vger.kernel.org>,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, David S. Miller,
Greg Kroah-Hartman, Samuel Neves, Andy Lutomirski,
Jean-Philippe Aumasson, Russell King, linux-arm-kernel
In-Reply-To: <CAKv+Gu8ih-TsASRGqK+ST_5+EQ0=Zo-zhGCadOdGyPjucMFTCg@mail.gmail.com>
On Wed, 26 Sep 2018 at 16:02, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> (+ Herbert, Thomas)
>
> On Wed, 26 Sep 2018 at 15:33, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > Hi Ard,
> >
> > On Wed, Sep 26, 2018 at 10:59 AM Ard Biesheuvel
> > <ard.biesheuvel@linaro.org> wrote:
> > > > +static inline bool chacha20_arch(struct chacha20_ctx *state, u8 *dst,
> > > > + const u8 *src, size_t len,
> > > > + simd_context_t *simd_context)
> > > > +{
> > > > +#if defined(CONFIG_KERNEL_MODE_NEON)
> > > > + if (chacha20_use_neon && len >= CHACHA20_BLOCK_SIZE * 3 &&
> > > > + simd_use(simd_context))
> > > > + chacha20_neon(dst, src, len, state->key, state->counter);
> > > > + else
> > > > +#endif
> > >
> > > Better to use IS_ENABLED() here:
> > >
> > > > + if (IS_ENABLED(CONFIG_KERNEL_MODE_NEON)) &&
> > > > + chacha20_use_neon && len >= CHACHA20_BLOCK_SIZE * 3 &&
> > > > + simd_use(simd_context))
> >
> > Good idea. I'll fix that up.
> >
> > >
> > > Also, this still has unbounded worst case scheduling latency, given
> > > that the outer library function passes its entire input straight into
> > > the NEON routine.
> >
> > The vast majority of crypto routines in arch/*/crypto/ follow this
> > same exact pattern, actually. I realize a few don't -- probably the
> > ones you had a hand in :) -- but I think this is up to the caller to
> > handle.
>
> Anything that uses the scatterwalk API (AEADs and skciphers) will
> handle at most a page at a time. Hashes are different, which is why
> some of them have to handle it explicitly.
>
> > I made a change so that in chacha20poly1305.c, it calls
> > simd_relax after handling each scatter-gather element, so a
> > "construction" will handle this gracefully. But I believe it's up to
> > the caller to decide on what sizes of information it wants to pass to
> > primitives. Put differently, this also hasn't ever been an issue
> > before -- the existing state of the tree indicates this -- and so I
> > don't anticipate this will be a real issue now.
>
> The state of the tree does not capture all relevant context or
> history. The scheduling latency issue was brought up very recently by
> the -rt folks on the mailing lists.
>
> > And if it becomes one,
> > this is something we can address *later*, but certainly there's no use
> > of adding additional complexity to the initial patchset to do this
> > now.
> >
>
> You are introducing a very useful SIMD abstraction, but it lets code
> run with preemption disabled for unbounded amounts of time, and so now
> is the time to ensure we get it right.
>
Actually, looking at the code again, the abstraction does appear to be
fine, it is just the chacha20 code that does not make use of it.
^ permalink raw reply
* Re: [PATCH] mt76: fix building without CONFIG_MT76x0U
From: Arnd Bergmann @ 2018-09-26 15:43 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: Kalle Valo, Felix Fietkau, Lorenzo Bianconi, David Miller,
linux-wireless, Networking, Linux Kernel Mailing List
In-Reply-To: <20180926140627.GA9265@redhat.com>
On Wed, Sep 26, 2018 at 4:06 PM Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>
> On Wed, Sep 26, 2018 at 02:51:59PM +0200, Arnd Bergmann wrote:
> > The recent rework of the mt76 driver caused build failures in
> > configurations that leave the mt76x0u support disabled:
> >
> > ERROR: "mt76u_mcu_deinit" [drivers/net/wireless/mediatek/mt76/mt76x0/mt76x0-common.ko] undefined!
> > ERROR: "mt76x02u_set_txinfo" [drivers/net/wireless/mediatek/mt76/mt76x0/mt76x0-common.ko] undefined!
> > ERROR: "mt76u_queues_deinit" [drivers/net/wireless/mediatek/mt76/mt76x0/mt76x0-common.ko] undefined!
> > ERROR: "mt76u_stop_stat_wk" [drivers/net/wireless/mediatek/mt76/mt76x0/mt76x0-common.ko] undefined!
> >
> > The mt76x0_push_txwi()/mt76x0_tx_prepare_skb()/mt76x0_cleanup() functions
> > that cause some of these are only called from the usb portion, and can
> > be hidden in an #ifdef in this case.
> >
> > mt76u_stop_stat_wk() is called mt76x0_mac_stop(), which is in turn
> > shared between multiple callers. Calling it only when the USB driver
> > is enabled avoids the link error but it is not clear to me whether this
> > can be called from a context where it would not do the right thing.
>
> I think there not yet posted Lorenzo patches that fix this build
> problem.
Ok, good. I assume that is a better fix than mine then. I also noticed
another problem that my patch does not address, but won't send a follow
up patch for that.
Please just ignore this version.
Arnd
^ permalink raw reply
* Re: [PATCH net-next v6 07/23] zinc: ChaCha20 ARM and ARM64 implementations
From: Jason A. Donenfeld @ 2018-09-26 15:45 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Herbert Xu, Thomas Gleixner, LKML, Netdev,
Linux Crypto Mailing List, David Miller, Greg Kroah-Hartman,
Samuel Neves, Andrew Lutomirski, Jean-Philippe Aumasson,
Russell King - ARM Linux, linux-arm-kernel
In-Reply-To: <CAKv+Gu8w74dUSEAGxxh=C6To6F=bSm8DjiExUPmh_LUyhoDLhg@mail.gmail.com>
On Wed, Sep 26, 2018 at 5:42 PM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> On Wed, 26 Sep 2018 at 16:02, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Actually, looking at the code again, the abstraction does appear to be
> fine, it is just the chacha20 code that does not make use of it.
So what you have in mind is something like calling simd_relax() every
4096 bytes or so?
^ permalink raw reply
* RE: [PATCH resend] can: rcar_can: convert to SPDX identifiers
From: Ramesh Shanmugasundaram @ 2018-09-26 9:34 UTC (permalink / raw)
To: Kuninori Morimoto, Wolfgang Grandegger, Marc Kleine-Budde
Cc: Linux-Renesas, Linux-Net, linux-can@vger.kernel.org
In-Reply-To: <874ledyrc6.wl-kuninori.morimoto.gx@renesas.com>
Hi Morimoto-san,
Thank you for the patch.
> Subject: [PATCH resend] can: rcar_can: convert to SPDX identifiers
>
>
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> This patch updates license to use SPDX-License-Identifier
> instead of verbose license text.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Acked-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
Thanks,
Ramesh
> ---
> Wolfgang, Marc
>
> 2 weeks past, resend patch
>
> drivers/net/can/rcar/Kconfig | 1 +
> drivers/net/can/rcar/Makefile | 1 +
> drivers/net/can/rcar/rcar_can.c | 6 +-----
> drivers/net/can/rcar/rcar_canfd.c | 6 +-----
> 4 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/can/rcar/Kconfig b/drivers/net/can/rcar/Kconfig
> index 7b03a3a..bd5a8fc 100644
> --- a/drivers/net/can/rcar/Kconfig
> +++ b/drivers/net/can/rcar/Kconfig
> @@ -1,3 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> config CAN_RCAR
> tristate "Renesas R-Car CAN controller"
> depends on ARCH_RENESAS || ARM
> diff --git a/drivers/net/can/rcar/Makefile b/drivers/net/can/rcar/Makefile
> index 08de36a..c9185b0 100644
> --- a/drivers/net/can/rcar/Makefile
> +++ b/drivers/net/can/rcar/Makefile
> @@ -1,3 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> #
> # Makefile for the Renesas R-Car CAN & CAN FD controller drivers
> #
> diff --git a/drivers/net/can/rcar/rcar_can.c
> b/drivers/net/can/rcar/rcar_can.c
> index 11662f4..06f90a0 100644
> --- a/drivers/net/can/rcar/rcar_can.c
> +++ b/drivers/net/can/rcar/rcar_can.c
> @@ -1,12 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0+
> /* Renesas R-Car CAN device driver
> *
> * Copyright (C) 2013 Cogent Embedded, Inc. <source@cogentembedded.com>
> * Copyright (C) 2013 Renesas Solutions Corp.
> - *
> - * This program is free software; you can redistribute it and/or modify
> it
> - * under the terms of the GNU General Public License as published by
> the
> - * Free Software Foundation; either version 2 of the License, or (at
> your
> - * option) any later version.
> */
>
> #include <linux/module.h>
> diff --git a/drivers/net/can/rcar/rcar_canfd.c
> b/drivers/net/can/rcar/rcar_canfd.c
> index 602c19e..0541000 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -1,11 +1,7 @@
> +// SPDX-License-Identifier: GPL-2.0+
> /* Renesas R-Car CAN FD device driver
> *
> * Copyright (C) 2015 Renesas Electronics Corp.
> - *
> - * This program is free software; you can redistribute it and/or modify
> it
> - * under the terms of the GNU General Public License as published by
> the
> - * Free Software Foundation; either version 2 of the License, or (at
> your
> - * option) any later version.
> */
>
> /* The R-Car CAN FD controller can operate in either one of the below two
> modes
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH] mt76: fix building without CONFIG_MT76x0U
From: Lorenzo Bianconi @ 2018-09-26 15:48 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: Arnd Bergmann, Kalle Valo, Felix Fietkau, David S. Miller,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20180926140627.GA9265-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> On Wed, Sep 26, 2018 at 02:51:59PM +0200, Arnd Bergmann wrote:
> > The recent rework of the mt76 driver caused build failures in
> > configurations that leave the mt76x0u support disabled:
> >
> > ERROR: "mt76u_mcu_deinit" [drivers/net/wireless/mediatek/mt76/mt76x0/mt76x0-common.ko] undefined!
> > ERROR: "mt76x02u_set_txinfo" [drivers/net/wireless/mediatek/mt76/mt76x0/mt76x0-common.ko] undefined!
> > ERROR: "mt76u_queues_deinit" [drivers/net/wireless/mediatek/mt76/mt76x0/mt76x0-common.ko] undefined!
> > ERROR: "mt76u_stop_stat_wk" [drivers/net/wireless/mediatek/mt76/mt76x0/mt76x0-common.ko] undefined!
> >
>
> I think there not yet posted Lorenzo patches that fix this build
> problem.
Hi Arnd,
adding mt76x0e hw initialization I have already fixed
mt76u_mcu_deinit, mt76u_queues_deinit and mt76u_stop_stat_wk
(I have not posted the patchset yet, I will send it later today
or tomorrow). mt76x02u_set_txinfo needs to be fixed since
I have not worked on mt76x0e tx path yet, but it is on my
ToDo list.
Regards,
Lorenzo
>
> Regards
> Stanislaw
>
^ permalink raw reply
* Re: [PATCH net-next v6 07/23] zinc: ChaCha20 ARM and ARM64 implementations
From: Jason A. Donenfeld @ 2018-09-26 15:49 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Herbert Xu, Thomas Gleixner, LKML, Netdev,
Linux Crypto Mailing List, David Miller, Greg Kroah-Hartman,
Samuel Neves, Andrew Lutomirski, Jean-Philippe Aumasson,
Russell King - ARM Linux, linux-arm-kernel
In-Reply-To: <CAHmME9pbRabkGjNh-65S9qBNNS1GLJSwZ_tTM507S_Ezr1=QLg@mail.gmail.com>
On Wed, Sep 26, 2018 at 5:45 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> So what you have in mind is something like calling simd_relax() every
> 4096 bytes or so?
That was actually pretty easy, putting together both of your suggestions:
static inline bool chacha20_arch(struct chacha20_ctx *state, u8 *dst,
u8 *src, size_t len,
simd_context_t *simd_context)
{
while (len > PAGE_SIZE) {
chacha20_arch(state, dst, src, PAGE_SIZE, simd_context);
len -= PAGE_SIZE;
src += PAGE_SIZE;
dst += PAGE_SIZE;
simd_relax(simd_context);
}
if (IS_ENABLED(CONFIG_KERNEL_MODE_NEON) && chacha20_use_neon &&
len >= CHACHA20_BLOCK_SIZE * 3 && simd_use(simd_context))
chacha20_neon(dst, src, len, state->key, state->counter);
else
chacha20_arm(dst, src, len, state->key, state->counter);
state->counter[0] += (len + 63) / 64;
return true;
}
^ permalink raw reply
* Re: [PATCH net-next v6 07/23] zinc: ChaCha20 ARM and ARM64 implementations
From: Jason A. Donenfeld @ 2018-09-26 15:58 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Herbert Xu, Thomas Gleixner, LKML, Netdev,
Linux Crypto Mailing List, David Miller, Greg Kroah-Hartman,
Samuel Neves, Andrew Lutomirski, Jean-Philippe Aumasson,
Russell King - ARM Linux, linux-arm-kernel
In-Reply-To: <CAKv+Gu-WDt8f9qVjLDRPHL7THS0BjtKHThw2RMfHbWBT8Hs8aQ@mail.gmail.com>
On Wed, Sep 26, 2018 at 5:52 PM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> Nice one :-)
>
> This works for me (but perhaps add a comment as well)
Sure. Just a prototype; it'll be clean for v7.
^ permalink raw reply
* Re: netlink: 16 bytes leftover after parsing attributes in process `ip'.
From: Christian Brauner @ 2018-09-26 9:49 UTC (permalink / raw)
To: David Ahern; +Cc: Jiri Benc, netdev@vger.kernel.org, David Miller
In-Reply-To: <02fbfcb8-537c-dc5c-52dc-0f53d7fd0482@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4411 bytes --]
On Tue, Sep 25, 2018 at 09:37:41AM -0600, David Ahern wrote:
> On 9/25/18 8:47 AM, Jiri Benc wrote:
> > On Tue, 25 Sep 2018 11:49:10 +0200, Christian Brauner wrote:
> >> So if people really want to hide this issue as much as we can then we
> >> can play the guessing game. I could send a patch that roughly does the
> >> following:
> >>
> >> if (nlmsg_len(cb->nlh) < sizeof(struct ifinfomsg))
> >> guessed_header_len = sizeof(struct ifaddrmsg);
> >> else
> >> guessed_header_len = sizeof(struct ifinfomsg);
> >>
> >> This will work since sizeof(ifaddrmsg) == 8 and sizeof(ifinfomsg) == 16.
> >> The only valid property for RTM_GETADDR requests is IFA_TARGET_NETNSID.
> >> This propert is a __s32 which should bring the message up to 12 bytes
> >> (not sure about alignment requiremnts and where we might wend up ten)
> >> which is still less than the 16 bytes without that property from
> >> ifinfomsg. That's a hacky hacky hack-hack and will likely work but will
> >> break when ifaddrmsg grows a new member or we introduce another property
> >> that is valid in RTM_GETADDR requests. It also will not work cleanly
> >> when users stuff additional properties in there that are vaif (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,lid for the
> >> address family but are not used int RTM_GETADDR requests.
> >
> > I'd expect that any potential existing code that makes use of other
> > attributes already assumes ifaddrmsg. Hence, if the nlmsg_len is >
> > sizeof(ifinfomsg), you can be sure that there are attributes and thus
> > the struct used was ifaddrmsg.
> >
> > So, in order for RTM_GETADDR to work reliably with attributes, you have
> > to ensure that the length is > sizeof(ifinfomsg).
>
> One of the many on-going efforts I have in progress is kernel side
> filtering of route dumps. It has this same problem. For it I am
> proposing a new flag:
>
> #define RTM_F_PROPER_HEADER 0x4000
>
> ifinfomsg is 16 bytes which is > rtmsg at 12. If the message length is >
> 16, then rtm_flags can be checked to know if the proper header is sent.
>
> For ifaddrmsg things do not line up as well. Worst all of the flag bits
> are used. But, perhaps one can be overloaded with the limit that you can
> never filter on its presence. Since you are adding the first filter to
> address dumps such a limitation should be ok.
>
> For example something like this (whitespace damaged on paste) to remove
> the guess work:
>
> diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
> index dfcf3ce0097f..8e3e9d475db5 100644
> --- a/include/uapi/linux/if_addr.h
> +++ b/include/uapi/linux/if_addr.h
> @@ -41,6 +41,11 @@ enum {
> #define IFA_MAX (__IFA_MAX - 1)
>
> /* ifa_flags */
> +/* IFA_F_PROPER_HEADER is only set in ifa_flags for dumps
> + * to indicate the ancillary header is the expected ifaddrmsg
> + * vs ifinfomsg from legacy userspace
> + */
> +#define IFA_F_PROPER_HEADER 0x01
> #define IFA_F_SECONDARY 0x01
> #define IFA_F_TEMPORARY IFA_F_SECONDARY
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index bfe3ec7ecb14..256b9f88db8f 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -5022,8 +5022,13 @@ static int inet6_dump_addr(struct sk_buff *skb,
> struct netlink_callback *cb,
> s_idx = idx = cb->args[1];
> s_ip_idx = ip_idx = cb->args[2];
>
> - if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
> - ifa_ipv6_policy, NULL) >= 0) {
> + if (nlmsg_len(cb->nlh) >= sizeof(struct ifaddrmsg) &&
> + ((struct ifaddrmsg *) nlmsg_data(cb->nlh))->ifa_flags &
> IFA_F_PROPER_HEADER) {
> +
> + if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb,
> IFA_MAX,
> + ifa_ipv6_policy, NULL) >= 0) {
> + ...
> +
> if (tb[IFA_TARGET_NETNSID]) {
> netnsid = nla_get_s32(tb[IFA_TARGET_NETNSID]);
>
>
> For ifaddrmsg ifa_flags aligns with ifi_type which is set by kernel side
> so this should be ok.
I like this idea way more than forcing userspace to create a nested
attribute. If Jiri's question is answered can we get this patch on the
road soon?
Are you happy to send it on-top of net-next or do you want me to do it?
Christian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] net: nixge: fix warnings on 32-bit platforms
From: Moritz Fischer @ 2018-09-26 16:03 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: David S. Miller, Moritz Fischer, netdev, linux-kernel
In-Reply-To: <20180926124808.1867468-1-arnd@arndb.de>
Hi Arnd,
On Wed, Sep 26, 2018 at 02:47:24PM +0200, Arnd Bergmann wrote:
> Making it work on 64-bit caused 32-bit builds to regress:
>
> drivers/net/ethernet/ni/nixge.c: In function 'nixge_hw_dma_bd_release':
> drivers/net/ethernet/ni/nixge.c:254:9: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
> skb = (struct sk_buff *)
> ^
> In file included from include/linux/skbuff.h:17,
> from include/linux/if_ether.h:23,
> from include/linux/etherdevice.h:25,
> from drivers/net/ethernet/ni/nixge.c:7:
> drivers/net/ethernet/ni/nixge.c: In function 'nixge_hw_dma_bd_init':
> drivers/net/ethernet/ni/nixge.c:130:37: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
> (bd)->field##_lo = lower_32_bits(((u64)addr)); \
> ^
> include/linux/kernel.h:234:33: note: in definition of macro 'lower_32_bits'
> #define lower_32_bits(n) ((u32)(n))
> ^
>
> Casting to a uintptr_t avoids all these warnings.
>
> Fixes: 7e8d5755be0e ("net: nixge: Add support for 64-bit platforms")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/net/ethernet/ni/nixge.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
> index 74cf52e3fb09..2e132f6a7c36 100644
> --- a/drivers/net/ethernet/ni/nixge.c
> +++ b/drivers/net/ethernet/ni/nixge.c
> @@ -127,8 +127,8 @@ struct nixge_hw_dma_bd {
> #ifdef CONFIG_PHYS_ADDR_T_64BIT
> #define nixge_hw_dma_bd_set_addr(bd, field, addr) \
> do { \
> - (bd)->field##_lo = lower_32_bits(((u64)addr)); \
> - (bd)->field##_hi = upper_32_bits(((u64)addr)); \
> + (bd)->field##_lo = lower_32_bits(((uintptr_t)addr)); \
> + (bd)->field##_hi = upper_32_bits(((uintptr_t)addr)); \
> } while (0)
> #else
> #define nixge_hw_dma_bd_set_addr(bd, field, addr) \
> @@ -251,7 +251,7 @@ static void nixge_hw_dma_bd_release(struct net_device *ndev)
> NIXGE_MAX_JUMBO_FRAME_SIZE,
> DMA_FROM_DEVICE);
>
> - skb = (struct sk_buff *)
> + skb = (struct sk_buff *)(uintptr_t)
> nixge_hw_dma_bd_get_addr(&priv->rx_bd_v[i],
> sw_id_offset);
> dev_kfree_skb(skb);
> @@ -601,8 +601,8 @@ static int nixge_recv(struct net_device *ndev, int budget)
> tail_p = priv->rx_bd_p + sizeof(*priv->rx_bd_v) *
> priv->rx_bd_ci;
>
> - skb = (struct sk_buff *)nixge_hw_dma_bd_get_addr(cur_p,
> - sw_id_offset);
> + skb = (struct sk_buff *)(uintptr_t)
> + nixge_hw_dma_bd_get_addr(cur_p, sw_id_offset);
>
> length = cur_p->status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK;
> if (length > NIXGE_MAX_JUMBO_FRAME_SIZE)
> --
> 2.18.0
>
I had submitted a similar patch https://lkml.org/lkml/2018/9/24/1719
here. I should probably add the Fixes tag yours has but other than that
they look largely equivalent. Comments?
Thanks,
Moritz
^ permalink raw reply
* Re: [PATCH net-next v6 23/23] net: WireGuard secure network tunnel
From: Jason A. Donenfeld @ 2018-09-26 16:04 UTC (permalink / raw)
To: labokml, Dave Taht
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman
In-Reply-To: <7830522a-968e-0880-beb7-44904466cf14@labo.rs>
Hi Ivan,
On Wed, Sep 26, 2018 at 6:00 PM Ivan Labáth <labokml@labo.rs> wrote:
>
> On 25.09.2018 16:56, Jason A. Donenfeld wrote:
> > Extensive documentation and description of the protocol and
> > considerations, along with formal proofs of the cryptography, are> available at:
> >
> > * https://www.wireguard.com/
> > * https://www.wireguard.com/papers/wireguard.pdf
> []
> > +enum { HANDSHAKE_DSCP = 0x88 /* AF41, plus 00 ECN */ };
> []
> > + if (skb->protocol == htons(ETH_P_IP)) {
> > + len = ntohs(ip_hdr(skb)->tot_len);
> > + if (unlikely(len < sizeof(struct iphdr)))
> > + goto dishonest_packet_size;
> > + if (INET_ECN_is_ce(PACKET_CB(skb)->ds))
> > + IP_ECN_set_ce(ip_hdr(skb));
> > + } else if (skb->protocol == htons(ETH_P_IPV6)) {
> > + len = ntohs(ipv6_hdr(skb)->payload_len) +
> > + sizeof(struct ipv6hdr);
> > + if (INET_ECN_is_ce(PACKET_CB(skb)->ds))
> > + IP6_ECN_set_ce(skb, ipv6_hdr(skb));
> > + } else
> []
> > + skb_queue_walk (&packets, skb) {
> > + /* 0 for no outer TOS: no leak. TODO: should we use flowi->tos
> > + * as outer? */
> > + PACKET_CB(skb)->ds = ip_tunnel_ecn_encap(0, ip_hdr(skb), skb);
> > + PACKET_CB(skb)->nonce =
> > + atomic64_inc_return(&key->counter.counter) - 1;
> > + if (unlikely(PACKET_CB(skb)->nonce >= REJECT_AFTER_MESSAGES))
> > + goto out_invalid;
> > + }
> Hi,
>
> is there documentation and/or rationale for ecn handling?
> Quick search for ecn and dscp didn't reveal any.
ECN support was developed with Dave Taht so that it does the right
thing with CAKE and such. He's CC'd, so that he can fill in details,
and sure, we can write these up. As well, I can add the rationale for
the handshake-packet-specific DSCP value to the paper in the next few
days; thanks for pointing out these documentation oversights.
Jason
^ permalink raw reply
* Re: [PATCH RFC net-next] openvswitch: Queue upcalls to userspace in per-port round-robin order
From: Stefano Brivio @ 2018-09-26 9:58 UTC (permalink / raw)
To: Pravin Shelar
Cc: Matteo Croce, Justin Pettit, Greg Rose, Ben Pfaff, netdev,
ovs dev, Jiri Benc, Aaron Conole, William Tu
In-Reply-To: <CAOrHB_DaA-+J=jzNOdQiUYrA7RJi30HmRESjsmGs7_z1ffpVOA@mail.gmail.com>
Hi Pravin,
On Wed, 15 Aug 2018 00:19:39 -0700
Pravin Shelar <pshelar@ovn.org> wrote:
> I understand fairness has cost, but we need to find right balance
> between performance and fairness. Current fairness scheme is a
> lockless algorithm without much computational overhead, did you try to
> improve current algorithm so that it uses less number of ports.
In the end, we tried harder as you suggested, and found out a way to
avoid the need for per-thread sets of per-vport sockets: with a few
changes, a process-wide set of per-vport sockets is actually enough to
maintain the current fairness behaviour.
Further, we can get rid of duplicate fd events if the EPOLLEXCLUSIVE
epoll() flag is available, which improves performance noticeably. This
is explained in more detail in the commit message of the ovs-vswitchd
patch Matteo wrote [1], now merged.
This approach solves the biggest issue (i.e. huge amount of netlink
sockets) in ovs-vswitchd itself, without the need for kernel changes.
It doesn't address some proposed improvements though, that is, it does
nothing to improve the current fairness scheme, it won't allow neither
the configurable fairness criteria proposed by Ben, nor usage of BPF
maps for extensibility as suggested by William.
I would however say that those topics bear definitely lower priority,
and perhaps addressing them at all becomes overkill now that we don't
any longer have a serious issue.
[1] https://patchwork.ozlabs.org/patch/974304/
--
Stefano
^ permalink raw reply
* Re: [PATCH v3 bpf-next 03/10] bpf: introduce per-cpu cgroup local storage
From: Song Liu @ 2018-09-26 16:13 UTC (permalink / raw)
To: Roman Gushchin
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Kernel Team,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180926113326.29069-4-guro@fb.com>
> On Sep 26, 2018, at 4:33 AM, Roman Gushchin <guro@fb.com> wrote:
>
> This commit introduced per-cpu cgroup local storage.
>
> Per-cpu cgroup local storage is very similar to simple cgroup storage
> (let's call it shared), except all the data is per-cpu.
>
> The main goal of per-cpu variant is to implement super fast
> counters (e.g. packet counters), which don't require neither
> lookups, neither atomic operations.
>
> From userspace's point of view, accessing a per-cpu cgroup storage
> is similar to other per-cpu map types (e.g. per-cpu hashmaps and
> arrays).
>
> Writing to a per-cpu cgroup storage is not atomic, but is performed
> by copying longs, so some minimal atomicity is here, exactly
> as with other per-cpu maps.
>
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Alexei Starovoitov <ast@kernel.org>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> include/linux/bpf-cgroup.h | 20 ++++-
> include/linux/bpf.h | 1 +
> include/linux/bpf_types.h | 1 +
> include/uapi/linux/bpf.h | 1 +
> kernel/bpf/helpers.c | 8 +-
> kernel/bpf/local_storage.c | 148 ++++++++++++++++++++++++++++++++-----
> kernel/bpf/syscall.c | 11 ++-
> kernel/bpf/verifier.c | 15 +++-
> 8 files changed, 177 insertions(+), 28 deletions(-)
>
> diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
> index 7e0c9a1d48b7..588dd5f0bd85 100644
> --- a/include/linux/bpf-cgroup.h
> +++ b/include/linux/bpf-cgroup.h
> @@ -37,7 +37,10 @@ struct bpf_storage_buffer {
> };
>
> struct bpf_cgroup_storage {
> - struct bpf_storage_buffer *buf;
> + union {
> + struct bpf_storage_buffer *buf;
> + void __percpu *percpu_buf;
> + };
> struct bpf_cgroup_storage_map *map;
> struct bpf_cgroup_storage_key key;
> struct list_head list;
> @@ -109,6 +112,9 @@ int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
> static inline enum bpf_cgroup_storage_type cgroup_storage_type(
> struct bpf_map *map)
> {
> + if (map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
> + return BPF_CGROUP_STORAGE_PERCPU;
> +
> return BPF_CGROUP_STORAGE_SHARED;
> }
>
> @@ -131,6 +137,10 @@ void bpf_cgroup_storage_unlink(struct bpf_cgroup_storage *storage);
> int bpf_cgroup_storage_assign(struct bpf_prog *prog, struct bpf_map *map);
> void bpf_cgroup_storage_release(struct bpf_prog *prog, struct bpf_map *map);
>
> +int bpf_percpu_cgroup_storage_copy(struct bpf_map *map, void *key, void *value);
> +int bpf_percpu_cgroup_storage_update(struct bpf_map *map, void *key,
> + void *value, u64 flags);
> +
> /* Wrappers for __cgroup_bpf_run_filter_skb() guarded by cgroup_bpf_enabled. */
> #define BPF_CGROUP_RUN_PROG_INET_INGRESS(sk, skb) \
> ({ \
> @@ -285,6 +295,14 @@ static inline struct bpf_cgroup_storage *bpf_cgroup_storage_alloc(
> struct bpf_prog *prog, enum bpf_cgroup_storage_type stype) { return 0; }
> static inline void bpf_cgroup_storage_free(
> struct bpf_cgroup_storage *storage) {}
> +static inline int bpf_percpu_cgroup_storage_copy(struct bpf_map *map, void *key,
> + void *value) {
> + return 0;
> +}
> +static inline int bpf_percpu_cgroup_storage_update(struct bpf_map *map,
> + void *key, void *value, u64 flags) {
> + return 0;
> +}
>
> #define cgroup_bpf_enabled (0)
> #define BPF_CGROUP_PRE_CONNECT_ENABLED(sk) (0)
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index b457fbe7b70b..018299a595c8 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -274,6 +274,7 @@ struct bpf_prog_offload {
>
> enum bpf_cgroup_storage_type {
> BPF_CGROUP_STORAGE_SHARED,
> + BPF_CGROUP_STORAGE_PERCPU,
> __BPF_CGROUP_STORAGE_MAX
> };
>
> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> index c9bd6fb765b0..5432f4c9f50e 100644
> --- a/include/linux/bpf_types.h
> +++ b/include/linux/bpf_types.h
> @@ -43,6 +43,7 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_CGROUP_ARRAY, cgroup_array_map_ops)
> #endif
> #ifdef CONFIG_CGROUP_BPF
> BPF_MAP_TYPE(BPF_MAP_TYPE_CGROUP_STORAGE, cgroup_storage_map_ops)
> +BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE, cgroup_storage_map_ops)
> #endif
> BPF_MAP_TYPE(BPF_MAP_TYPE_HASH, htab_map_ops)
> BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_HASH, htab_percpu_map_ops)
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index aa5ccd2385ed..e2070d819e04 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -127,6 +127,7 @@ enum bpf_map_type {
> BPF_MAP_TYPE_SOCKHASH,
> BPF_MAP_TYPE_CGROUP_STORAGE,
> BPF_MAP_TYPE_REUSEPORT_SOCKARRAY,
> + BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
> };
>
> enum bpf_prog_type {
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index e42f8789b7ea..6502115e8f55 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -206,10 +206,16 @@ BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags)
> */
> enum bpf_cgroup_storage_type stype = cgroup_storage_type(map);
> struct bpf_cgroup_storage *storage;
> + void *ptr;
>
> storage = this_cpu_read(bpf_cgroup_storage[stype]);
>
> - return (unsigned long)&READ_ONCE(storage->buf)->data[0];
> + if (stype == BPF_CGROUP_STORAGE_SHARED)
> + ptr = &READ_ONCE(storage->buf)->data[0];
> + else
> + ptr = this_cpu_ptr(storage->percpu_buf);
> +
> + return (unsigned long)ptr;
> }
>
> const struct bpf_func_proto bpf_get_local_storage_proto = {
> diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
> index 6742292fb39e..c739f6dcc3c2 100644
> --- a/kernel/bpf/local_storage.c
> +++ b/kernel/bpf/local_storage.c
> @@ -152,6 +152,71 @@ static int cgroup_storage_update_elem(struct bpf_map *map, void *_key,
> return 0;
> }
>
> +int bpf_percpu_cgroup_storage_copy(struct bpf_map *_map, void *_key,
> + void *value)
> +{
> + struct bpf_cgroup_storage_map *map = map_to_storage(_map);
> + struct bpf_cgroup_storage_key *key = _key;
> + struct bpf_cgroup_storage *storage;
> + int cpu, off = 0;
> + u32 size;
> +
> + rcu_read_lock();
> + storage = cgroup_storage_lookup(map, key, false);
> + if (!storage) {
> + rcu_read_unlock();
> + return -ENOENT;
> + }
> +
> + /* per_cpu areas are zero-filled and bpf programs can only
> + * access 'value_size' of them, so copying rounded areas
> + * will not leak any kernel data
> + */
> + size = round_up(_map->value_size, 8);
> + for_each_possible_cpu(cpu) {
> + bpf_long_memcpy(value + off,
> + per_cpu_ptr(storage->percpu_buf, cpu), size);
> + off += size;
> + }
> + rcu_read_unlock();
> + return 0;
> +}
> +
> +int bpf_percpu_cgroup_storage_update(struct bpf_map *_map, void *_key,
> + void *value, u64 map_flags)
> +{
> + struct bpf_cgroup_storage_map *map = map_to_storage(_map);
> + struct bpf_cgroup_storage_key *key = _key;
> + struct bpf_cgroup_storage *storage;
> + int cpu, off = 0;
> + u32 size;
> +
> + if (unlikely(map_flags & BPF_EXIST))
> + return -EINVAL;
> +
> + rcu_read_lock();
> + storage = cgroup_storage_lookup(map, key, false);
> + if (!storage) {
> + rcu_read_unlock();
> + return -ENOENT;
> + }
> +
> + /* the user space will provide round_up(value_size, 8) bytes that
> + * will be copied into per-cpu area. bpf programs can only access
> + * value_size of it. During lookup the same extra bytes will be
> + * returned or zeros which were zero-filled by percpu_alloc,
> + * so no kernel data leaks possible
> + */
> + size = round_up(_map->value_size, 8);
> + for_each_possible_cpu(cpu) {
> + bpf_long_memcpy(per_cpu_ptr(storage->percpu_buf, cpu),
> + value + off, size);
> + off += size;
> + }
> + rcu_read_unlock();
> + return 0;
> +}
> +
> static int cgroup_storage_get_next_key(struct bpf_map *_map, void *_key,
> void *_next_key)
> {
> @@ -292,55 +357,98 @@ struct bpf_cgroup_storage *bpf_cgroup_storage_alloc(struct bpf_prog *prog,
> {
> struct bpf_cgroup_storage *storage;
> struct bpf_map *map;
> + gfp_t flags;
> + size_t size;
> u32 pages;
>
> map = prog->aux->cgroup_storage[stype];
> if (!map)
> return NULL;
>
> - pages = round_up(sizeof(struct bpf_cgroup_storage) +
> - sizeof(struct bpf_storage_buffer) +
> - map->value_size, PAGE_SIZE) >> PAGE_SHIFT;
> + if (stype == BPF_CGROUP_STORAGE_SHARED) {
> + size = sizeof(struct bpf_storage_buffer) + map->value_size;
> + pages = round_up(sizeof(struct bpf_cgroup_storage) + size,
> + PAGE_SIZE) >> PAGE_SHIFT;
> + } else {
> + size = map->value_size;
> + pages = round_up(round_up(size, 8) * num_possible_cpus(),
> + PAGE_SIZE) >> PAGE_SHIFT;
> + }
> +
> if (bpf_map_charge_memlock(map, pages))
> return ERR_PTR(-EPERM);
>
> storage = kmalloc_node(sizeof(struct bpf_cgroup_storage),
> __GFP_ZERO | GFP_USER, map->numa_node);
> - if (!storage) {
> - bpf_map_uncharge_memlock(map, pages);
> - return ERR_PTR(-ENOMEM);
> - }
> + if (!storage)
> + goto enomem;
>
> - storage->buf = kmalloc_node(sizeof(struct bpf_storage_buffer) +
> - map->value_size, __GFP_ZERO | GFP_USER,
> - map->numa_node);
> - if (!storage->buf) {
> - bpf_map_uncharge_memlock(map, pages);
> - kfree(storage);
> - return ERR_PTR(-ENOMEM);
> + flags = __GFP_ZERO | GFP_USER;
> +
> + if (stype == BPF_CGROUP_STORAGE_SHARED) {
> + storage->buf = kmalloc_node(size, flags, map->numa_node);
> + if (!storage->buf)
> + goto enomem;
> + } else {
> + storage->percpu_buf = __alloc_percpu_gfp(size, 8, flags);
> + if (!storage->percpu_buf)
> + goto enomem;
> }
>
> storage->map = (struct bpf_cgroup_storage_map *)map;
>
> return storage;
> +
> +enomem:
> + bpf_map_uncharge_memlock(map, pages);
> + kfree(storage);
> + return ERR_PTR(-ENOMEM);
> +}
> +
> +static void free_shared_cgroup_storage_rcu(struct rcu_head *rcu)
> +{
> + struct bpf_cgroup_storage *storage =
> + container_of(rcu, struct bpf_cgroup_storage, rcu);
> +
> + kfree(storage->buf);
> + kfree(storage);
> +}
> +
> +static void free_percpu_cgroup_storage_rcu(struct rcu_head *rcu)
> +{
> + struct bpf_cgroup_storage *storage =
> + container_of(rcu, struct bpf_cgroup_storage, rcu);
> +
> + free_percpu(storage->percpu_buf);
> + kfree(storage);
> }
>
> void bpf_cgroup_storage_free(struct bpf_cgroup_storage *storage)
> {
> - u32 pages;
> + enum bpf_cgroup_storage_type stype;
> struct bpf_map *map;
> + u32 pages;
>
> if (!storage)
> return;
>
> map = &storage->map->map;
> - pages = round_up(sizeof(struct bpf_cgroup_storage) +
> - sizeof(struct bpf_storage_buffer) +
> - map->value_size, PAGE_SIZE) >> PAGE_SHIFT;
> + stype = cgroup_storage_type(map);
> + if (stype == BPF_CGROUP_STORAGE_SHARED)
> + pages = round_up(sizeof(struct bpf_cgroup_storage) +
> + sizeof(struct bpf_storage_buffer) +
> + map->value_size, PAGE_SIZE) >> PAGE_SHIFT;
> + else
> + pages = round_up(round_up(map->value_size, 8) *
> + num_possible_cpus(),
> + PAGE_SIZE) >> PAGE_SHIFT;
> +
> bpf_map_uncharge_memlock(map, pages);
>
> - kfree_rcu(storage->buf, rcu);
> - kfree_rcu(storage, rcu);
> + if (stype == BPF_CGROUP_STORAGE_SHARED)
> + call_rcu(&storage->rcu, free_shared_cgroup_storage_rcu);
> + else
> + call_rcu(&storage->rcu, free_percpu_cgroup_storage_rcu);
> }
>
> void bpf_cgroup_storage_link(struct bpf_cgroup_storage *storage,
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 8c91d2b41b1e..5742df21598c 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -686,7 +686,8 @@ static int map_lookup_elem(union bpf_attr *attr)
>
> if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
> map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
> - map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY)
> + map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY ||
> + map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
> value_size = round_up(map->value_size, 8) * num_possible_cpus();
> else if (IS_FD_MAP(map))
> value_size = sizeof(u32);
> @@ -705,6 +706,8 @@ static int map_lookup_elem(union bpf_attr *attr)
> err = bpf_percpu_hash_copy(map, key, value);
> } else if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
> err = bpf_percpu_array_copy(map, key, value);
> + } else if (map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE) {
> + err = bpf_percpu_cgroup_storage_copy(map, key, value);
> } else if (map->map_type == BPF_MAP_TYPE_STACK_TRACE) {
> err = bpf_stackmap_copy(map, key, value);
> } else if (IS_FD_ARRAY(map)) {
> @@ -774,7 +777,8 @@ static int map_update_elem(union bpf_attr *attr)
>
> if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
> map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
> - map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY)
> + map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY ||
> + map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
> value_size = round_up(map->value_size, 8) * num_possible_cpus();
> else
> value_size = map->value_size;
> @@ -809,6 +813,9 @@ static int map_update_elem(union bpf_attr *attr)
> err = bpf_percpu_hash_update(map, key, value, attr->flags);
> } else if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
> err = bpf_percpu_array_update(map, key, value, attr->flags);
> + } else if (map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE) {
> + err = bpf_percpu_cgroup_storage_update(map, key, value,
> + attr->flags);
> } else if (IS_FD_ARRAY(map)) {
> rcu_read_lock();
> err = bpf_fd_array_map_update_elem(map, f.file, key, value,
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e90899df585d..a8cc83a970d1 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2074,6 +2074,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
> goto error;
> break;
> case BPF_MAP_TYPE_CGROUP_STORAGE:
> + case BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE:
> if (func_id != BPF_FUNC_get_local_storage)
> goto error;
> break;
> @@ -2164,7 +2165,8 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
> goto error;
> break;
> case BPF_FUNC_get_local_storage:
> - if (map->map_type != BPF_MAP_TYPE_CGROUP_STORAGE)
> + if (map->map_type != BPF_MAP_TYPE_CGROUP_STORAGE &&
> + map->map_type != BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
> goto error;
> break;
> case BPF_FUNC_sk_select_reuseport:
> @@ -5049,6 +5051,12 @@ static int check_map_prog_compatibility(struct bpf_verifier_env *env,
> return 0;
> }
>
> +static bool bpf_map_is_cgroup_storage(struct bpf_map *map)
> +{
> + return (map->map_type == BPF_MAP_TYPE_CGROUP_STORAGE ||
> + map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE);
> +}
> +
> /* look for pseudo eBPF instructions that access map FDs and
> * replace them with actual map pointers
> */
> @@ -5139,10 +5147,9 @@ static int replace_map_fd_with_map_ptr(struct bpf_verifier_env *env)
> }
> env->used_maps[env->used_map_cnt++] = map;
>
> - if (map->map_type == BPF_MAP_TYPE_CGROUP_STORAGE &&
> + if (bpf_map_is_cgroup_storage(map) &&
> bpf_cgroup_storage_assign(env->prog, map)) {
> - verbose(env,
> - "only one cgroup storage is allowed\n");
> + verbose(env, "only one cgroup storage of each type is allowed\n");
> fdput(f);
> return -EBUSY;
> }
> --
> 2.17.1
>
^ permalink raw reply
* Re: pull-request: bpf-next 2018-09-25
From: Daniel Borkmann @ 2018-09-26 10:03 UTC (permalink / raw)
To: David Miller; +Cc: ast, netdev
In-Reply-To: <20180925.204049.402375716601309433.davem@davemloft.net>
On 09/26/2018 05:40 AM, David Miller wrote:
> From: Daniel Borkmann <daniel@iogearbox.net>
> Date: Tue, 25 Sep 2018 22:43:43 +0200
>
>> The following pull-request contains BPF updates for your *net-next*
>> tree.
>
> Pulled, there was a minor merge conflict. Please double check my work.
The resolution in tools/lib/bpf/Build looks good to me, thanks!
^ permalink raw reply
* KASAN: use-after-free Read in rtnl_fill_ifinfo
From: syzbot @ 2018-09-26 16:16 UTC (permalink / raw)
To: christian, davem, dsahern, fw, jakub.kicinski, jbenc, ktkhai,
linux-kernel, lucien.xin, netdev, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 4b1bd6976945 net: phy: marvell: Fix build.
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=1477df9e400000
kernel config: https://syzkaller.appspot.com/x/.config?x=443816db871edd66
dashboard link: https://syzkaller.appspot.com/bug?extid=1e0c7db78acf0d004daf
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+1e0c7db78acf0d004daf@syzkaller.appspotmail.com
llcp: llcp_sock_recvmsg: Recv datagram failed state 5 -107 0
==================================================================
BUG: KASAN: use-after-free in rtnl_fill_ifinfo+0x420e/0x4670
net/core/rtnetlink.c:1630
Read of size 8 at addr ffff8801c2e7a4d8 by task syz-executor2/9379
CPU: 1 PID: 9379 Comm: syz-executor2 Not tainted 4.19.0-rc5+ #232
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113
print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412
__asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
rtnl_fill_ifinfo+0x420e/0x4670 net/core/rtnetlink.c:1630
rtnl_dump_ifinfo+0x90a/0x1130 net/core/rtnetlink.c:1943
netlink_dump+0x519/0xd50 net/netlink/af_netlink.c:2226
netlink_recvmsg+0xf6f/0x1480 net/netlink/af_netlink.c:1984
sock_recvmsg_nosec+0x8c/0xb0 net/socket.c:794
___sys_recvmsg+0x2b6/0x680 net/socket.c:2278
__sys_recvmmsg+0x303/0xb90 net/socket.c:2390
do_sys_recvmmsg+0xec/0x1a0 net/socket.c:2471
__do_sys_recvmmsg net/socket.c:2484 [inline]
__se_sys_recvmmsg net/socket.c:2480 [inline]
__x64_sys_recvmmsg+0xbe/0x150 net/socket.c:2480
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457579
Code: 1d b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 eb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f3e5c58cc78 EFLAGS: 00000246
ORIG_RAX: 000000000000012b
RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 0000000000457579
RDX: 04000000000000f6 RSI: 00000000200037c0 RDI: 0000000000000003
RBP: 000000000072bfa0 R08: 0000000020003700 R09: 0000000000000000
R10: 0000000000000006 R11: 0000000000000246 R12: 00007f3e5c58d6d4
R13: 00000000004c3258 R14: 00000000004d4de8 R15: 00000000ffffffff
Allocated by task 5629:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553
__do_kmalloc_node mm/slab.c:3682 [inline]
__kmalloc_node+0x47/0x70 mm/slab.c:3689
kmalloc_node include/linux/slab.h:555 [inline]
kzalloc_node include/linux/slab.h:718 [inline]
qdisc_alloc+0x10f/0xb50 net/sched/sch_generic.c:820
qdisc_create_dflt+0x7a/0x1e0 net/sched/sch_generic.c:894
attach_one_default_qdisc net/sched/sch_generic.c:1041 [inline]
netdev_for_each_tx_queue include/linux/netdevice.h:2114 [inline]
attach_default_qdiscs net/sched/sch_generic.c:1060 [inline]
dev_activate+0x82f/0xcb0 net/sched/sch_generic.c:1103
__dev_open+0x2cb/0x410 net/core/dev.c:1400
dev_open+0xf2/0x1b0 net/core/dev.c:1426
bond_enslave+0xfca/0x5f90 drivers/net/bonding/bond_main.c:1531
do_set_master+0x1c9/0x220 net/core/rtnetlink.c:2313
do_setlink+0xbe6/0x3f20 net/core/rtnetlink.c:2447
rtnl_newlink+0x136f/0x1d40 net/core/rtnetlink.c:3054
rtnetlink_rcv_msg+0x46a/0xc20 net/core/rtnetlink.c:4730
netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2447
rtnetlink_rcv+0x1c/0x20 net/core/rtnetlink.c:4748
netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
netlink_unicast+0x5a5/0x760 net/netlink/af_netlink.c:1336
netlink_sendmsg+0xa18/0xfc0 net/netlink/af_netlink.c:1901
sock_sendmsg_nosec net/socket.c:621 [inline]
sock_sendmsg+0xd5/0x120 net/socket.c:631
___sys_sendmsg+0x7fd/0x930 net/socket.c:2116
__sys_sendmsg+0x11d/0x280 net/socket.c:2154
__do_sys_sendmsg net/socket.c:2163 [inline]
__se_sys_sendmsg net/socket.c:2161 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2161
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 5520:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521
kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
__cache_free mm/slab.c:3498 [inline]
kfree+0xcf/0x230 mm/slab.c:3813
qdisc_free+0x89/0x100 net/sched/sch_generic.c:941
qdisc_free_cb+0x19/0x20 net/sched/sch_generic.c:948
__rcu_reclaim kernel/rcu/rcu.h:236 [inline]
rcu_do_batch kernel/rcu/tree.c:2576 [inline]
invoke_rcu_callbacks kernel/rcu/tree.c:2880 [inline]
__rcu_process_callbacks kernel/rcu/tree.c:2847 [inline]
rcu_process_callbacks+0xf23/0x2670 kernel/rcu/tree.c:2864
__do_softirq+0x30b/0xad8 kernel/softirq.c:292
The buggy address belongs to the object at ffff8801c2e7a4c0
which belongs to the cache kmalloc-1024 of size 1024
The buggy address is located 24 bytes inside of
1024-byte region [ffff8801c2e7a4c0, ffff8801c2e7a8c0)
The buggy address belongs to the page:
page:ffffea00070b9e80 count:1 mapcount:0 mapping:ffff8801da800ac0 index:0x0
compound_mapcount: 0
flags: 0x2fffc0000008100(slab|head)
raw: 02fffc0000008100 ffffea0007081708 ffffea00071f2688 ffff8801da800ac0
raw: 0000000000000000 ffff8801c2e7a040 0000000100000007 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8801c2e7a380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801c2e7a400: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
> ffff8801c2e7a480: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
^
ffff8801c2e7a500: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801c2e7a580: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
^ permalink raw reply
* Re: [PATCH bpf-next] bpftool: Fix bpftool net output
From: Daniel Borkmann @ 2018-09-26 10:19 UTC (permalink / raw)
To: Andrey Ignatov, netdev; +Cc: ast, yhs
In-Reply-To: <20180925222037.1858472-1-rdna@fb.com>
On 09/26/2018 12:20 AM, Andrey Ignatov wrote:
> Print `bpftool net` output to stdout instead of stderr. Only errors
> should be printed to stderr. Regular output should go to stdout and this
> is what all other subcommands of bpftool do, including --json and
> --pretty formats of `bpftool net` itself.
>
> Fixes: commit f6f3bac08ff9 ("tools/bpf: bpftool: add net support")
> Signed-off-by: Andrey Ignatov <rdna@fb.com>
> Acked-by: Yonghong Song <yhs@fb.com>
Applied to bpf-next, thanks Andrey!
^ permalink raw reply
* [PATCH] hv_netvsc: Make sure out channel is fully opened on send
From: Mohammed Gamal @ 2018-09-26 16:34 UTC (permalink / raw)
To: sthemmin, netdev
Cc: kys, haiyangz, vkuznets, otubo, cavery, linux-kernel, devel,
Mohammed Gamal
Dring high network traffic changes to network interface parameters
such as number of channels or MTU can cause a kernel panic with a NULL
pointer dereference. This is due to netvsc_device_remove() being
called and deallocating the channel ring buffers, which can then be
accessed by netvsc_send_pkt() before they're allocated on calling
netvsc_device_add()
The patch fixes this problem by checking the channel state and returning
ENODEV if not yet opened. We also move the call to hv_ringbuf_avail_percent()
which may access the uninitialized ring buffer.
Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
---
drivers/net/hyperv/netvsc.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index fe01e14..75f1b31 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -825,7 +825,12 @@ static inline int netvsc_send_pkt(
struct netdev_queue *txq = netdev_get_tx_queue(ndev, packet->q_idx);
u64 req_id;
int ret;
- u32 ring_avail = hv_get_avail_to_write_percent(&out_channel->outbound);
+ u32 ring_avail;
+
+ if (out_channel->state != CHANNEL_OPENED_STATE)
+ return -ENODEV;
+
+ ring_avail = hv_get_avail_to_write_percent(&out_channel->outbound);
nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT;
if (skb)
--
1.8.3.1
^ permalink raw reply related
* Re: kernel 4.18.5 Realtek 8111G network adapter stops responding under high system load
From: David Arendt @ 2018-09-26 16:44 UTC (permalink / raw)
To: Heiner Kallweit, Maciej S. Szmigiero, Gabriel C,
Ortwin Glück
Cc: linux-kernel, nic_swsd, netdev
In-Reply-To: <968f03ee-a271-242b-d90a-5c70ea72ce3b@gmail.com>
Hi,
Thanks.
I have just applied Heiner Kallweit's patch on top of kernel 4.18.10 and
the TxConfig register contains 0x4f000f80.
I will give it 24 hours under high load and report back if the patch
really solves the problem.
Bye,
David Arendt
On 9/25/18 11:03 PM, Heiner Kallweit wrote:
> On 19.09.2018 06:12, David Arendt wrote:
>> Hi,
>>
>> Thanks for the patch.
>>
>> I just applied it and the TxConfig register now contains 0x4f000f80.
>> The next day will show if it really solves the problem.
>>
>> Thanks in advance,
>> David Arendt
>>
>> On 9/19/18 12:30 AM, Maciej S. Szmigiero wrote:
>>> Hi,
>>>
>>> On 18.09.2018 12:23, David Arendt wrote:
>>>> Hi,
>>>>
>>>> Today I had the network adapter problems again.
>>>> So the patch doesn't seem to change anything regarding this problem.
>>>> This week my time is unfortunately very limited, but I will try to
>>>> find some time next weekend to look a bit more into the issue.
>>> If the problem is caused by missing TXCFG_AUTO_FIFO bit in TxConfig,
>>> as the register difference would suggest, then you can try applying
>>> the following patch (hack) on top of 4.18.8 that is already patched
>>> with commit f74dd480cf4e:
>>> --- a/drivers/net/ethernet/realtek/r8169.c
>>> +++ b/drivers/net/ethernet/realtek/r8169.c
>>> @@ -5043,7 +5043,8 @@
>>> {
>>> /* Set DMA burst size and Interframe Gap Time */
>>> RTL_W32(tp, TxConfig, (TX_DMA_BURST << TxDMAShift) |
>>> - (InterFrameGap << TxInterFrameGapShift));
>>> + (InterFrameGap << TxInterFrameGapShift)
>>> + | TXCFG_AUTO_FIFO);
>>> }
>>>
>>> static void rtl_set_rx_max_size(struct rtl8169_private *tp)
>>>
>>> This hack will probably only work properly on RTL_GIGA_MAC_VER_40 or
>>> later NICs.
>>>
>>> Before running any tests please verify with "ethtool -d enp3s0" that
>>> TxConfig register now contains 0x4f000f80, as it did in the old,
>>> working driver version.
>>>
>>> If this does not help then a bisection will most likely be needed.
>>>
>>>> Thanks in advance,
>>>> David Arendt
>>> Maciej
>>
>>
> @Gabriel:
> Thanks for the hint, I wasn't fully aware of this thread.
> @Maciej:
> Thanks for the analysis.
>
> It seems that all chip versions from 34 (= RTL8168E-VL) with the
> exception of version 39 (= RTL8106E, first sub-version) need
> bit TXCFG_AUTO_FIFO.
>
> And indeed, due to reordering of calls this bit is overwritten.
> Following patch moves setting the bit from the chip-specific
> hw_start function to rtl_set_tx_config_registers().
>
> Whoever is hit by the issue and has the option to build a kernel,
> could you please test whether the patch fixes the issue for you?
>
> Thanks, Heiner
>
> ---
> drivers/net/ethernet/realtek/r8169.c | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index f882be49f..ae8abe900 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -4514,9 +4514,14 @@ static void rtl8169_hw_reset(struct rtl8169_private *tp)
>
> static void rtl_set_tx_config_registers(struct rtl8169_private *tp)
> {
> - /* Set DMA burst size and Interframe Gap Time */
> - RTL_W32(tp, TxConfig, (TX_DMA_BURST << TxDMAShift) |
> - (InterFrameGap << TxInterFrameGapShift));
> + u32 val = TX_DMA_BURST << TxDMAShift |
> + InterFrameGap << TxInterFrameGapShift;
> +
> + if (tp->mac_version >= RTL_GIGA_MAC_VER_34 &&
> + tp->mac_version != RTL_GIGA_MAC_VER_39)
> + val |= TXCFG_AUTO_FIFO;
> +
> + RTL_W32(tp, TxConfig, val);
> }
>
> static void rtl_set_rx_max_size(struct rtl8169_private *tp)
> @@ -5011,7 +5016,6 @@ static void rtl_hw_start_8168e_2(struct rtl8169_private *tp)
>
> rtl_disable_clock_request(tp);
>
> - RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
> RTL_W8(tp, MCU, RTL_R8(tp, MCU) & ~NOW_IS_OOB);
>
> /* Adjust EEE LED frequency */
> @@ -5045,7 +5049,6 @@ static void rtl_hw_start_8168f(struct rtl8169_private *tp)
>
> rtl_disable_clock_request(tp);
>
> - RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
> RTL_W8(tp, MCU, RTL_R8(tp, MCU) & ~NOW_IS_OOB);
> RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) | PFM_EN);
> RTL_W32(tp, MISC, RTL_R32(tp, MISC) | PWM_EN);
> @@ -5090,8 +5093,6 @@ static void rtl_hw_start_8411(struct rtl8169_private *tp)
>
> static void rtl_hw_start_8168g(struct rtl8169_private *tp)
> {
> - RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
> -
> rtl_eri_write(tp, 0xc8, ERIAR_MASK_0101, 0x080002, ERIAR_EXGMAC);
> rtl_eri_write(tp, 0xcc, ERIAR_MASK_0001, 0x38, ERIAR_EXGMAC);
> rtl_eri_write(tp, 0xd0, ERIAR_MASK_0001, 0x48, ERIAR_EXGMAC);
> @@ -5189,8 +5190,6 @@ static void rtl_hw_start_8168h_1(struct rtl8169_private *tp)
> rtl_hw_aspm_clkreq_enable(tp, false);
> rtl_ephy_init(tp, e_info_8168h_1, ARRAY_SIZE(e_info_8168h_1));
>
> - RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
> -
> rtl_eri_write(tp, 0xc8, ERIAR_MASK_0101, 0x00080002, ERIAR_EXGMAC);
> rtl_eri_write(tp, 0xcc, ERIAR_MASK_0001, 0x38, ERIAR_EXGMAC);
> rtl_eri_write(tp, 0xd0, ERIAR_MASK_0001, 0x48, ERIAR_EXGMAC);
> @@ -5273,8 +5272,6 @@ static void rtl_hw_start_8168ep(struct rtl8169_private *tp)
> {
> rtl8168ep_stop_cmac(tp);
>
> - RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
> -
> rtl_eri_write(tp, 0xc8, ERIAR_MASK_0101, 0x00080002, ERIAR_EXGMAC);
> rtl_eri_write(tp, 0xcc, ERIAR_MASK_0001, 0x2f, ERIAR_EXGMAC);
> rtl_eri_write(tp, 0xd0, ERIAR_MASK_0001, 0x5f, ERIAR_EXGMAC);
> @@ -5596,7 +5593,6 @@ static void rtl_hw_start_8402(struct rtl8169_private *tp)
> /* Force LAN exit from ASPM if Rx/Tx are not idle */
> RTL_W32(tp, FuncEvent, RTL_R32(tp, FuncEvent) | 0x002800);
>
> - RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
> RTL_W8(tp, MCU, RTL_R8(tp, MCU) & ~NOW_IS_OOB);
>
> rtl_ephy_init(tp, e_info_8402, ARRAY_SIZE(e_info_8402));
^ permalink raw reply
* [PATCH] 9p: potential NULL dereference
From: Dan Carpenter @ 2018-09-26 10:39 UTC (permalink / raw)
To: Eric Van Hensbergen, Matthew Wilcox
Cc: Latchesar Ionkov, Dominique Martinet, David S. Miller,
v9fs-developer, netdev, kernel-janitors
p9_tag_alloc() is supposed to return error pointers, but we accidentally
return a NULL here. It would cause a NULL dereference in the caller.
Fixes: 996d5b4db4b1 ("9p: Use a slab for allocating requests")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/net/9p/client.c b/net/9p/client.c
index 47fa6158a75a..5f23e18eecc0 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -281,7 +281,7 @@ p9_tag_alloc(struct p9_client *c, int8_t type, unsigned int max_size)
int tag;
if (!req)
- return NULL;
+ return ERR_PTR(-ENOMEM);
if (p9_fcall_init(c, &req->tc, alloc_msize))
goto free_req;
^ permalink raw reply related
* Re: netlink: 16 bytes leftover after parsing attributes in process `ip'.
From: Christian Brauner @ 2018-09-26 10:45 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Ahern, netdev@vger.kernel.org, David Miller
In-Reply-To: <20180925141612.13084179@shemminger-XPS-13-9360>
On Tue, Sep 25, 2018 at 02:16:12PM +0100, Stephen Hemminger wrote:
> On Tue, 25 Sep 2018 14:34:08 +0200
> Christian Brauner <christian@brauner.io> wrote:
>
> > On Tue, Sep 25, 2018, 14:07 Stephen Hemminger <stephen@networkplumber.org>
> > wrote:
> >
> > > On Tue, 25 Sep 2018 11:49:10 +0200
> > > Christian Brauner <christian@brauner.io> wrote:
> > >
> > > > On Mon, Sep 24, 2018 at 09:19:06PM -0600, David Ahern wrote:
> > > > > On top of net-next I am see a dmesg error:
> > > > >
> > > > > netlink: 16 bytes leftover after parsing attributes in process `ip'.
> > > > >
> > > > > I traced it to address lists and commit:
> > > > >
> > > > > commit 6ecf4c37eb3e89b0832c9616089a5cdca3747da7
> > > > > Author: Christian Brauner <christian@brauner.io>
> > > > > Date: Tue Sep 4 21:53:50 2018 +0200
> > > > >
> > > > > ipv6: enable IFA_TARGET_NETNSID for RTM_GETADDR
> > > > >
> > > > > Per the commit you are trying to guess whether the ancillary header is
> > > > > an ifinfomsg or a ifaddrmsg. I am guessing you are guessing wrong.
> > > :-)
> > > >
> > > > Well, I currently don't guess at all. :) I'm parsing with struct
> > > > ifaddrmsg as assumed header size but ignore parsing errors when that
> > > > fails. You don't get the niceties of the new property if you don't pack
> > >
> > > There are legacy parts of netlink interface with kernel.
> > > The ABI has evolved over time but some old parts are stuck in the past.
> > >
> > >
> > > > > I don't have time to take this to ground, but address listing is not
> > > the
> > > > > only area subject to iproute2's SNAFU of infomsg everywhere on dumps. I
> > > > > have thought about this for route dumps, but its solution does not work
> > > > > here. You'll need to find something because the current warning on
> > > every
> > > > > address dump is not acceptable.
> > > >
> > > > Two points before I propose a migitation:
> > > >
> > > > 1. The burded of seeing pr_warn_ratelimited() messages in dmesg when
> > > > userspace is doing something wrong is imho justifiable.
> > > > Actually, I would argue that we should not hide the problem from
> > > > userspace at all. The rate-limited (so no logging DOS afaict) warning
> > > > messages are a perfect indicator that a tool is doing something wrong
> > > > *without* introducing any regressions.
> > > > The rtnetlink manpage clearly indicates that ifaddrmsg is supposed to
> > > > be used too. Additionally, userspace stuffs an ifinfomsg in there but
> > > > expects to receive ifaddrmsg. They should be warned loudly. :) So I
> > > > actually like the warning messages.
> > > > 2. Userspace should be fixed. Especially such an important standard tool
> > > > as iproute2 that is maintained on git.kernel.org (glibc is already
> > > > doing the right.).
> > > >
> > > > So if people really want to hide this issue as much as we can then we
> > > > can play the guessing game. I could send a patch that roughly does the
> > > > following:
> > > >
> > > > if (nlmsg_len(cb->nlh) < sizeof(struct ifinfomsg))
> > > > guessed_header_len = sizeof(struct ifaddrmsg);
> > > > else
> > > > guessed_header_len = sizeof(struct ifinfomsg);
> > > >
> > > > This will work since sizeof(ifaddrmsg) == 8 and sizeof(ifinfomsg) == 16.
> > > > The only valid property for RTM_GETADDR requests is IFA_TARGET_NETNSID.
> > > > This propert is a __s32 which should bring the message up to 12 bytes
> > > > (not sure about alignment requiremnts and where we might wend up ten)
> > > > which is still less than the 16 bytes without that property from
> > > > ifinfomsg. That's a hacky hacky hack-hack and will likely work but will
> > > > break when ifaddrmsg grows a new member or we introduce another property
> > > > that is valid in RTM_GETADDR requests. It also will not work cleanly
> > > > when users stuff additional properties in there that are valid for the
> > > > address family but are not used int RTM_GETADDR requests.
> > > >
> > > > I would like to hear what other people and davem think we should do.
> > > > Patch it away or print the warning.
> > > >
> > > > Christian
> > >
> > > You can't break old programs. That is one of the rules of kernel.
> > > Therefore, please either revert the kernel change or put the new attribute
> > > in a place where old versions do not cause problem.
> > >
> >
> > Sorry, there's a misunderstanding here. The code doesn't regress anything.
> > The patch was written in a backward compatible way. The only thing it
> > causes are rate-limited logging messages when the wrong struct is passed.
> > But the request still succeeds. The issue is with the logging afaict.
> >
> > Christian
>
>
> That still means enterprise distributions that use the current kernel will
> get customer complaints. You need to remove the warning.
First, you're the senior developer and I totally accept your decision so
we'll make the warning go away!
Rate-limited messages in dmesg on an edge kernel in response to wrong
userspace behavior *without any functional regressions* is something
that I find justifiable and to some extent necessary.
The promise about not regressing userspace is about identical results
from the kernel on unchanged userspace behavior. The contract doesn't
and shouldn't include "We're also not going to tell you that you're
doing something wrong.".
The log messages are an indicator to userspace that "Hey, you're passing
me something wrong in this request. I still will fulfill your request
just as before but please, think about changing this.". That's basically
the only way we have to get userspace to correct false prior behavior
without introducing regressions. Considering warnings in log messages to
be regressions leaves us with nothing visible to do.
Christian
^ permalink raw reply
* [PATCH net-next] tls: Remove redundant vars from tls record structure
From: Vakul Garg @ 2018-09-26 10:52 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, doronrk, Vakul Garg
Structure 'tls_rec' contains sg_aead_in and sg_aead_out which point
to a aad_space and then chain scatterlists sg_plaintext_data,
sg_encrypted_data respectively. Rather than using chained scatterlists
for plaintext and encrypted data in aead_req, it is efficient to store
aad_space in sg_encrypted_data and sg_plaintext_data itself in the
first index and get rid of sg_aead_in, sg_aead_in and further chaining.
This requires increasing size of sg_encrypted_data & sg_plaintext_data
arrarys by 1 to accommodate entry for aad_space. The code which uses
sg_encrypted_data and sg_plaintext_data has been modified to skip first
index as it points to aad_space.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
include/net/tls.h | 6 ++--
net/tls/tls_sw.c | 92 ++++++++++++++++++++++++++-----------------------------
2 files changed, 45 insertions(+), 53 deletions(-)
diff --git a/include/net/tls.h b/include/net/tls.h
index 1615fb5ea114..262420cdad10 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -101,13 +101,11 @@ struct tls_rec {
struct list_head list;
int tx_ready;
int tx_flags;
- struct scatterlist sg_plaintext_data[MAX_SKB_FRAGS];
- struct scatterlist sg_encrypted_data[MAX_SKB_FRAGS];
/* AAD | sg_plaintext_data | sg_tag */
- struct scatterlist sg_aead_in[2];
+ struct scatterlist sg_plaintext_data[MAX_SKB_FRAGS + 1];
/* AAD | sg_encrypted_data (data contain overhead for hdr&iv&tag) */
- struct scatterlist sg_aead_out[2];
+ struct scatterlist sg_encrypted_data[MAX_SKB_FRAGS + 1];
unsigned int sg_plaintext_size;
unsigned int sg_encrypted_size;
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 4c18b4dba284..8cf7bef7c5a2 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -248,7 +248,7 @@ static void trim_both_sgl(struct sock *sk, int target_size)
struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
struct tls_rec *rec = ctx->open_rec;
- trim_sg(sk, rec->sg_plaintext_data,
+ trim_sg(sk, &rec->sg_plaintext_data[1],
&rec->sg_plaintext_num_elem,
&rec->sg_plaintext_size,
target_size);
@@ -256,7 +256,7 @@ static void trim_both_sgl(struct sock *sk, int target_size)
if (target_size > 0)
target_size += tls_ctx->tx.overhead_size;
- trim_sg(sk, rec->sg_encrypted_data,
+ trim_sg(sk, &rec->sg_encrypted_data[1],
&rec->sg_encrypted_num_elem,
&rec->sg_encrypted_size,
target_size);
@@ -270,12 +270,13 @@ static int alloc_encrypted_sg(struct sock *sk, int len)
int rc = 0;
rc = sk_alloc_sg(sk, len,
- rec->sg_encrypted_data, 0,
+ &rec->sg_encrypted_data[1], 0,
&rec->sg_encrypted_num_elem,
&rec->sg_encrypted_size, 0);
if (rc == -ENOSPC)
- rec->sg_encrypted_num_elem = ARRAY_SIZE(rec->sg_encrypted_data);
+ rec->sg_encrypted_num_elem =
+ ARRAY_SIZE(rec->sg_encrypted_data) - 1;
return rc;
}
@@ -287,12 +288,15 @@ static int alloc_plaintext_sg(struct sock *sk, int len)
struct tls_rec *rec = ctx->open_rec;
int rc = 0;
- rc = sk_alloc_sg(sk, len, rec->sg_plaintext_data, 0,
- &rec->sg_plaintext_num_elem, &rec->sg_plaintext_size,
+ rc = sk_alloc_sg(sk, len,
+ &rec->sg_plaintext_data[1], 0,
+ &rec->sg_plaintext_num_elem,
+ &rec->sg_plaintext_size,
tls_ctx->pending_open_record_frags);
if (rc == -ENOSPC)
- rec->sg_plaintext_num_elem = ARRAY_SIZE(rec->sg_plaintext_data);
+ rec->sg_plaintext_num_elem =
+ ARRAY_SIZE(rec->sg_plaintext_data) - 1;
return rc;
}
@@ -320,11 +324,11 @@ static void tls_free_open_rec(struct sock *sk)
if (!rec)
return;
- free_sg(sk, rec->sg_encrypted_data,
+ free_sg(sk, &rec->sg_encrypted_data[1],
&rec->sg_encrypted_num_elem,
&rec->sg_encrypted_size);
- free_sg(sk, rec->sg_plaintext_data,
+ free_sg(sk, &rec->sg_plaintext_data[1],
&rec->sg_plaintext_num_elem,
&rec->sg_plaintext_size);
@@ -355,7 +359,7 @@ int tls_tx_records(struct sock *sk, int flags)
* Remove the head of tx_list
*/
list_del(&rec->list);
- free_sg(sk, rec->sg_plaintext_data,
+ free_sg(sk, &rec->sg_plaintext_data[1],
&rec->sg_plaintext_num_elem, &rec->sg_plaintext_size);
kfree(rec);
@@ -370,13 +374,13 @@ int tls_tx_records(struct sock *sk, int flags)
tx_flags = flags;
rc = tls_push_sg(sk, tls_ctx,
- &rec->sg_encrypted_data[0],
+ &rec->sg_encrypted_data[1],
0, tx_flags);
if (rc)
goto tx_err;
list_del(&rec->list);
- free_sg(sk, rec->sg_plaintext_data,
+ free_sg(sk, &rec->sg_plaintext_data[1],
&rec->sg_plaintext_num_elem,
&rec->sg_plaintext_size);
@@ -405,16 +409,12 @@ static void tls_encrypt_done(struct crypto_async_request *req, int err)
rec = container_of(aead_req, struct tls_rec, aead_req);
- rec->sg_encrypted_data[0].offset -= tls_ctx->tx.prepend_size;
- rec->sg_encrypted_data[0].length += tls_ctx->tx.prepend_size;
+ rec->sg_encrypted_data[1].offset -= tls_ctx->tx.prepend_size;
+ rec->sg_encrypted_data[1].length += tls_ctx->tx.prepend_size;
- /* Free the record if error is previously set on socket */
+ /* Check if error is previously set on socket */
if (err || sk->sk_err) {
- free_sg(sk, rec->sg_encrypted_data,
- &rec->sg_encrypted_num_elem, &rec->sg_encrypted_size);
-
- kfree(rec);
rec = NULL;
/* If err is already set on socket, return the same code */
@@ -449,7 +449,7 @@ static void tls_encrypt_done(struct crypto_async_request *req, int err)
/* Schedule the transmission */
if (!test_and_set_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask))
- schedule_delayed_work(&ctx->tx_work.work, 1);
+ schedule_delayed_work(&ctx->tx_work.work, 2);
}
static int tls_do_encryption(struct sock *sk,
@@ -461,13 +461,14 @@ static int tls_do_encryption(struct sock *sk,
struct tls_rec *rec = ctx->open_rec;
int rc;
- rec->sg_encrypted_data[0].offset += tls_ctx->tx.prepend_size;
- rec->sg_encrypted_data[0].length -= tls_ctx->tx.prepend_size;
+ /* Skip the first index as it contains AAD data */
+ rec->sg_encrypted_data[1].offset += tls_ctx->tx.prepend_size;
+ rec->sg_encrypted_data[1].length -= tls_ctx->tx.prepend_size;
aead_request_set_tfm(aead_req, ctx->aead_send);
aead_request_set_ad(aead_req, TLS_AAD_SPACE_SIZE);
- aead_request_set_crypt(aead_req, rec->sg_aead_in,
- rec->sg_aead_out,
+ aead_request_set_crypt(aead_req, rec->sg_plaintext_data,
+ rec->sg_encrypted_data,
data_len, tls_ctx->tx.iv);
aead_request_set_callback(aead_req, CRYPTO_TFM_REQ_MAY_BACKLOG,
@@ -480,8 +481,8 @@ static int tls_do_encryption(struct sock *sk,
rc = crypto_aead_encrypt(aead_req);
if (!rc || rc != -EINPROGRESS) {
atomic_dec(&ctx->encrypt_pending);
- rec->sg_encrypted_data[0].offset -= tls_ctx->tx.prepend_size;
- rec->sg_encrypted_data[0].length += tls_ctx->tx.prepend_size;
+ rec->sg_encrypted_data[1].offset -= tls_ctx->tx.prepend_size;
+ rec->sg_encrypted_data[1].length += tls_ctx->tx.prepend_size;
}
if (!rc) {
@@ -512,16 +513,16 @@ static int tls_push_record(struct sock *sk, int flags,
rec->tx_flags = flags;
req = &rec->aead_req;
- sg_mark_end(rec->sg_plaintext_data + rec->sg_plaintext_num_elem - 1);
- sg_mark_end(rec->sg_encrypted_data + rec->sg_encrypted_num_elem - 1);
+ sg_mark_end(rec->sg_plaintext_data + rec->sg_plaintext_num_elem);
+ sg_mark_end(rec->sg_encrypted_data + rec->sg_encrypted_num_elem);
tls_make_aad(rec->aad_space, rec->sg_plaintext_size,
tls_ctx->tx.rec_seq, tls_ctx->tx.rec_seq_size,
record_type);
tls_fill_prepend(tls_ctx,
- page_address(sg_page(&rec->sg_encrypted_data[0])) +
- rec->sg_encrypted_data[0].offset,
+ page_address(sg_page(&rec->sg_encrypted_data[1])) +
+ rec->sg_encrypted_data[1].offset,
rec->sg_plaintext_size, record_type);
tls_ctx->pending_open_record_frags = 0;
@@ -613,7 +614,7 @@ static int memcopy_from_iter(struct sock *sk, struct iov_iter *from,
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
struct tls_rec *rec = ctx->open_rec;
- struct scatterlist *sg = rec->sg_plaintext_data;
+ struct scatterlist *sg = &rec->sg_plaintext_data[1];
int copy, i, rc = 0;
for (i = tls_ctx->pending_open_record_frags;
@@ -659,17 +660,10 @@ struct tls_rec *get_rec(struct sock *sk)
sg_init_table(&rec->sg_encrypted_data[0],
ARRAY_SIZE(rec->sg_encrypted_data));
- sg_init_table(rec->sg_aead_in, 2);
- sg_set_buf(&rec->sg_aead_in[0], rec->aad_space,
+ sg_set_buf(&rec->sg_plaintext_data[0], rec->aad_space,
sizeof(rec->aad_space));
- sg_unmark_end(&rec->sg_aead_in[1]);
- sg_chain(rec->sg_aead_in, 2, rec->sg_plaintext_data);
-
- sg_init_table(rec->sg_aead_out, 2);
- sg_set_buf(&rec->sg_aead_out[0], rec->aad_space,
+ sg_set_buf(&rec->sg_encrypted_data[0], rec->aad_space,
sizeof(rec->aad_space));
- sg_unmark_end(&rec->sg_aead_out[1]);
- sg_chain(rec->sg_aead_out, 2, rec->sg_encrypted_data);
ctx->open_rec = rec;
@@ -763,8 +757,8 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
ret = zerocopy_from_iter(sk, &msg->msg_iter,
try_to_copy, &rec->sg_plaintext_num_elem,
&rec->sg_plaintext_size,
- rec->sg_plaintext_data,
- ARRAY_SIZE(rec->sg_plaintext_data),
+ &rec->sg_plaintext_data[1],
+ ARRAY_SIZE(rec->sg_plaintext_data) - 1,
true);
if (ret)
goto fallback_to_reg_send;
@@ -781,7 +775,7 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
continue;
fallback_to_reg_send:
- trim_sg(sk, rec->sg_plaintext_data,
+ trim_sg(sk, &rec->sg_plaintext_data[1],
&rec->sg_plaintext_num_elem,
&rec->sg_plaintext_size,
orig_size);
@@ -801,7 +795,7 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
try_to_copy -= required_size - rec->sg_plaintext_size;
full_record = true;
- trim_sg(sk, rec->sg_encrypted_data,
+ trim_sg(sk, &rec->sg_encrypted_data[1],
&rec->sg_encrypted_num_elem,
&rec->sg_encrypted_size,
rec->sg_plaintext_size +
@@ -949,7 +943,7 @@ int tls_sw_sendpage(struct sock *sk, struct page *page,
}
get_page(page);
- sg = rec->sg_plaintext_data + rec->sg_plaintext_num_elem;
+ sg = &rec->sg_plaintext_data[1] + rec->sg_plaintext_num_elem;
sg_set_page(sg, page, copy, offset);
sg_unmark_end(sg);
@@ -963,7 +957,7 @@ int tls_sw_sendpage(struct sock *sk, struct page *page,
if (full_record || eor ||
rec->sg_plaintext_num_elem ==
- ARRAY_SIZE(rec->sg_plaintext_data)) {
+ ARRAY_SIZE(rec->sg_plaintext_data) - 1) {
ret = tls_push_record(sk, flags, record_type);
if (ret) {
if (ret == -EINPROGRESS)
@@ -1571,7 +1565,7 @@ void tls_sw_free_resources_tx(struct sock *sk)
rec = list_first_entry(&ctx->tx_list,
struct tls_rec, list);
- free_sg(sk, rec->sg_plaintext_data,
+ free_sg(sk, &rec->sg_plaintext_data[1],
&rec->sg_plaintext_num_elem,
&rec->sg_plaintext_size);
@@ -1580,11 +1574,11 @@ void tls_sw_free_resources_tx(struct sock *sk)
}
list_for_each_entry_safe(rec, tmp, &ctx->tx_list, list) {
- free_sg(sk, rec->sg_encrypted_data,
+ free_sg(sk, &rec->sg_encrypted_data[1],
&rec->sg_encrypted_num_elem,
&rec->sg_encrypted_size);
- free_sg(sk, rec->sg_plaintext_data,
+ free_sg(sk, &rec->sg_plaintext_data[1],
&rec->sg_plaintext_num_elem,
&rec->sg_plaintext_size);
--
2.13.6
^ permalink raw reply related
* Re: [PATCH net-next v6 07/23] zinc: ChaCha20 ARM and ARM64 implementations
From: Jason A. Donenfeld @ 2018-09-26 17:07 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Herbert Xu, Thomas Gleixner, LKML, Netdev,
Linux Crypto Mailing List, David Miller, Greg Kroah-Hartman,
Samuel Neves, Andrew Lutomirski, Jean-Philippe Aumasson,
Russell King - ARM Linux, linux-arm-kernel
In-Reply-To: <CAKv+Gu9hOW0pOjDjrvg1q-v85aiwSZRnjivnvRTCwxMdph4j7g@mail.gmail.com>
On Wed, Sep 26, 2018 at 6:55 PM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> Framing it as /needless/ complexity does not help at all. The changes
> you are proposing are very useful, but nobody wants two crypto
> subsystems with two different maintainers in the kernel, so I would
> like to understand where this is going in the future. I am not saying
> it should block these patches though.
Thanks for clarifying. I understood you to be intending to block the
patches until they were converted to an async interface, which is not
what Zinc's about. Seeing as you're just curious about future
directions, that seems much more tenable.
> Also, I have spent a *lot* of time looking at your code, and trying to
> make it better, especially for use cases that weren't on your radar to
> begin with
I am extremely grateful for a good portion of your reviews indeed. As
I mentioned earlier, much is very useful. But in other places, I fear
you're steering this in a direction I really am hesitant to go.
> (e.g., 'pet projects' [your words]
Taken out of context.
> consideration for other aspects of kernel programming, e.g.,
> preemption under -rt, stack size constraints, coding style, importing
> code from other projects etc.
And indeed all of these concerns I've been pretty amenable to, and
continue to do so. What I'm commenting on are things outside of these.
> - please try to be less dismissive of
> feedback first time around, but try to understand why people are
> raising these issues
Apologies, and duly noted. I'll give you the benefit of the doubt.
Jason
^ permalink raw reply
* Re: [PATCH net-next v6 07/23] zinc: ChaCha20 ARM and ARM64 implementations
From: Ard Biesheuvel @ 2018-09-26 17:08 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Andy Lutomirski, Herbert Xu, Thomas Gleixner,
Linux Kernel Mailing List, <netdev@vger.kernel.org>,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, David S. Miller,
Greg Kroah-Hartman, Samuel Neves, Andy Lutomirski,
Jean-Philippe Aumasson, Russell King, linux-arm-kernel
In-Reply-To: <CAHmME9pZKUMnnSZ0670rUH4H61g0x88c-B-DZYw_X-8+xgH-=g@mail.gmail.com>
On Wed, 26 Sep 2018 at 19:03, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> On Wed, Sep 26, 2018 at 6:21 PM Andy Lutomirski <luto@amacapital.net> wrote:
> > Are, is what you’re saying that the Zinc chacha20 functions should call simd_relax() every n bytes automatically for some reasonable value of n? If so, seems sensible, except that some care might be needed to make sure they interact with preemption correctly.
> >
> > What I mean is: the public Zinc entry points should either be callable in an atomic context or they should not be. I think this should be checked at runtime in an appropriate place with an __might_sleep or similar. Or simd_relax should learn to *not* schedule if the result of preempt_enable() leaves it atomic. (And the latter needs to be done in a way that works even on non-preempt kernels, and I don’t remember whether that’s possible.). And this should happen regardless of how many bytes are processed. IOW, calling into Zinc should be equally not atomic-safe for 100 bytes and for 10 MB.
>
> I'm not sure this is actually a problem. Namely:
>
> preempt_disable();
> kernel_fpu_begin();
> kernel_fpu_end();
> schedule(); <--- bug!
>
> Calling kernel_fpu_end() disables preemption, but AFAIK, preemption
> enabling/disabling is recursive, so kernel_fpu_end's use of
> preempt_disable won't actually do anything until the outer preempt
> enable is called:
>
> preempt_disable();
> kernel_fpu_begin();
> kernel_fpu_end();
> preempt_enable();
> schedule(); <--- works!
>
> Or am I missing some more subtle point?
>
No that seems accurate to me.
^ permalink raw reply
* Re: [PATCH] net: qed: list usage cleanup
From: David Miller @ 2018-09-26 17:14 UTC (permalink / raw)
To: zhongjiang; +Cc: Ariel.Elior, everest-linux-l2, netdev, linux-kernel
In-Reply-To: <1537951981-6464-1-git-send-email-zhongjiang@huawei.com>
From: zhong jiang <zhongjiang@huawei.com>
Date: Wed, 26 Sep 2018 16:53:00 +0800
> Trival cleanup, list_move_tail will implement the same function that
> list_del() + list_add_tail() will do. hence just replace them.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: liquidio: list usage cleanup
From: David Miller @ 2018-09-26 17:14 UTC (permalink / raw)
To: zhongjiang
Cc: derek.chickles, satananda.burla, felix.manlunas, netdev,
linux-kernel
In-Reply-To: <1537952210-6687-1-git-send-email-zhongjiang@huawei.com>
From: zhong jiang <zhongjiang@huawei.com>
Date: Wed, 26 Sep 2018 16:56:50 +0800
> Trival cleanup, list_move_tail will implement the same function that
> list_del() + list_add_tail() will do. hence just replace them.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
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