* Re: [PATCH] drivers: net: phy: mdio-mux: Add support for Generic Mux controls
From: Andrew Lunn @ 2019-02-21 15:17 UTC (permalink / raw)
To: Pankaj Bansal
Cc: Leo Li, Peter Rosin, Florian Fainelli, Heiner Kallweit,
netdev@vger.kernel.org
In-Reply-To: <VI1PR0401MB2496D9119F5639278CB14965F17E0@VI1PR0401MB2496.eurprd04.prod.outlook.com>
> > > config MDIO_BUS_MUX
> > > tristate
> > > depends on OF_MDIO
> > > + select MULTIPLEXER
> > > help
> > > This module provides a driver framework for MDIO bus
> > > multiplexers which connect one of several child MDIO busses
> >
> > Hi Pankaj
> >
> > Please add a MDIO_BUS_MUX_MULTIPLEXER and put all the code into mdio-
> > mux-multiplexer.c
>
> Isn't MUX short for MULTIPLEXER ? wouldn't this be more confusing ?
Look at the pattern:
config MDIO_BUS_MUX
config MDIO_BUS_MUX_BCM_IPROC
config MDIO_BUS_MUX_GPIO
config MDIO_BUS_MUX_MMIOREG
You are adding another sort of MUX, A Mux that uses a kernel
Multiplexer. Hence the name should be MDIO_BUS_MUX_MULTIPLEXER.
You can try to avoid confusion by using good help text:
help This module provides a driver for MDIO bus multiplexer
that is controlled via the kernel multiplexer subsystem. The
bus multiplexer connects one of several child MDIO busses to
a parent bus. Child bus selection is under the control of
the kernel multiplexer subsystem.
This test basically follows the other MDIO multiplexers.
Andrew
^ permalink raw reply
* Re: [PATCH] tcp: Reset tcp connections in SYN-SENT state
From: Eric Dumazet @ 2019-02-21 15:17 UTC (permalink / raw)
To: Devi Sandeep Endluri V V; +Cc: netdev, subashab, sharathv, ssaha, stranche
In-Reply-To: <20190221110118.GA24747@dendluri-linux.qualcomm.com>
On Thu, Feb 21, 2019 at 3:01 AM Devi Sandeep Endluri V V
<dendluri@codeaurora.org> wrote:
>
> On Wed, Feb 20, 2019 at 07:47:59AM -0800, Eric Dumazet wrote:
> > On Wed, Feb 20, 2019 at 6:28 AM Devi Sandeep Endluri V V
> > <dendluri@codeaurora.org> wrote:
> > >
> > > Userspace sends tcp connection (sock) destroy on network permission
> > > change. Kernel though doesn't send reset for the connections in
> > > SYN-SENT state and these connections continue to remain. Even as
> > > per RFC 793, there is no hard rule to not send RST on ABORT in
> > > this state. Change to make sure RST are send for connections in
> > > syn-sent state to avoid lingering connections on network switch.
> > >
> > > References from RFC 793
> > >
> > > ABORT Call
> > >
> > > SYN-SENT STATE
> > >
> > > All queued SENDs and RECEIVEs should be given "connection reset"
> > > notification, delete the TCB, enter CLOSED state, and return.
> > >
> > > SEGMENT ARRIVES
> > >
> > > If the state is SYN-SENT then
> > > If the RST bit is set
> > >
> > > If the ACK was acceptable then signal the user "error:
> > > connection reset", drop the segment, enter CLOSED state,
> > > delete TCB, and return. Otherwise (no ACK) drop the segment
> > > and return.
> > >
> > > Signed-off-by: Devi Sandeep Endluri V V <dendluri@codeaurora.org>
> > > ---
> > > net/ipv4/tcp.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git net/ipv4/tcp.c net/ipv4/tcp.c
> > > index cf3c509..8569dc5e 100644
> > > --- net/ipv4/tcp.c
> > > +++ net/ipv4/tcp.c
> > > @@ -2495,7 +2495,7 @@ static inline bool tcp_need_reset(int state)
> > > {
> > > return (1 << state) &
> > > (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT | TCPF_FIN_WAIT1 |
> > > - TCPF_FIN_WAIT2 | TCPF_SYN_RECV);
> > > + TCPF_FIN_WAIT2 | TCPF_SYN_RECV | TCPF_SYN_SENT);
> > > }
> > >
> > > static void tcp_rtx_queue_purge(struct sock *sk)
> >
> > Hi
> >
> > 1) I do not believe this patch is complete :
> > You have not changed tcp_disconnect() which seems to have dead code
> > if we apply your patch.
> >
> > 2) I am not sure we want to send yet another packet if the prior SYN
> > packets elect no answer.
> > (It is not clear if your patch applies to this case)
> >
> > 3) If we do not RESET, the other side has not seen the 3rd packet and
> > should eventually exit from SYN_RECV state and die.
> >
> > 4) A RESET can be lost on the network, and nothing will retransmit it.
> >
> > Can you describe the use case more precisely, because it seems in
> > contradiction of a feature that we plan to upstream.
> > (TCP_SILENT_CLOSE : do not send flood of RST/FIN when a gigantic
> > server process prematurely dies)
> >
> > Thanks.
>
> The algorithm for our network switch needs to have all TCP sessions
> on the previous network cleaned up. So, we are trying to reset the
> connections in SYN-SENT state as well. Is there any other way to
> forcefully reset these connections immediately rather than waiting
> for the other side to eventually exit and die?
There is no way we can make sure all TCP sessions are cleaned up.
For example, we can not make sure no machine eventually crashes or is
unexpectedly powered down.
What is the "algorithm for your network switch" exactly.
Sorry, I am not convinced we want to add more RST packets on the network,
most SRE closely look at RST packets as some proxy of 'badness'.
^ permalink raw reply
* Re: [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers
From: Luis Chamberlain @ 2019-02-21 15:18 UTC (permalink / raw)
To: Eric Sandeen, Andrew Morton
Cc: Eric Sandeen, Linux Kernel Mailing List, fsdevel, netdev,
Kees Cook
In-Reply-To: <76eca432-8dfc-18f3-4977-567ac1707ff5@sandeen.net>
On Wed, Feb 20, 2019 at 05:35:04PM -0600, Eric Sandeen wrote:
> Here's a pretty hacky test script to test this code via
> ip_local_reserved_ports
Thanks Eric!
So /proc/sys/net/ipv4/ip_local_reserved_ports is a production knob, and
if we wanted to stress test it with a selftest it could break other self
tests or change the system behaviour. Because of this we have now have
lib/test_sysctl.c, and we test this with the script:
tools/testing/selftests/sysctl/sysctl.sh
Any chance you can extend lib/test_sysctl.c with a new respective bitmap
knob, and add a respective test? This will ensure we don't regress
later. 0-day runs sysctl.sh so it should catch any regressions in the
future.
If you can think of other ways to test the knob that would be great too.
Luis
^ permalink raw reply
* Re: [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT to a device
From: Jesper Dangaard Brouer @ 2019-02-21 15:19 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: David Miller, netdev, Daniel Borkmann, Alexei Starovoitov, brouer
In-Reply-To: <155075021399.13610.12521373406832889226.stgit@alrua-x1>
You forgot at cover letter describing why we are doing this...
even-though is should be obvious from the performance results ;-)
On Thu, 21 Feb 2019 12:56:54 +0100 Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> Before patch:
> xdp_redirect: 5426035 pkt/s
> xdp_redirect_map: 8412754 pkt/s
>
> After patch:
> xdp_redirect: 8314702 pkt/s
> xdp_redirect_map: 8411854 pkt/s
>
> This corresponds to a 53% increase in xdp_redirect performance, or a
> reduction in per-packet processing time by 64 nanoseconds.
(1/5426035-1/8314702)*10^9 = 64.0277 almost exactly 64 nanosec
(1/8412754-1/8411854)*10^9 = -0.012 => no regression for xdp_redirect_map
--
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 RFC 3/5] sched/cpufreq: Fix incorrect RCU API usage
From: Joel Fernandes @ 2019-02-21 15:21 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ido Schimmel,
Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, Paul E. McKenney,
rcu, Song Liu, Steven Rostedt, xdp-newbies, Yonghong Song
In-Reply-To: <20190221091805.GX32477@hirez.programming.kicks-ass.net>
On Thu, Feb 21, 2019 at 10:18:05AM +0100, Peter Zijlstra wrote:
> On Thu, Feb 21, 2019 at 12:49:40AM -0500, Joel Fernandes (Google) wrote:
> > @@ -34,8 +34,12 @@ void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
> > if (WARN_ON(!data || !func))
> > return;
> >
> > - if (WARN_ON(per_cpu(cpufreq_update_util_data, cpu)))
> > + rcu_read_lock();
> > + if (WARN_ON(rcu_dereference(per_cpu(cpufreq_update_util_data, cpu)))) {
> > + rcu_read_unlock();
> > return;
> > + }
> > + rcu_read_unlock();
> >
> > data->func = func;
> > rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
>
> This doesn't make any kind of sense to me.
>
As per the rcu_assign_pointer() line, I inferred that
cpufreq_update_util_data is expected to be RCU protected. Reading the pointer
value of RCU pointers generally needs to be done from RCU read section, and
using rcu_dereference() (or using rcu_access()).
In this patch, I changed cpufreq_update_util_data to be __rcu annotated to
avoid the sparse error thrown by rcu_assign_pointer().
Instead of doing that, If your intention here is RELEASE barrier, should I
just replace in this function:
rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
with:
smp_store_release(per_cpu(cpufreq_update_util_data, cpu), data))
?
It would be nice IMO to be explicit about the intention of release/publish
semantics by using smp_store_release().
thanks,
- Joel
^ permalink raw reply
* Re: [PATCH iproute2-next v1 02/19] rdma: Remove duplicated print code
From: David Ahern @ 2019-02-21 15:23 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Leon Romanovsky, netdev, RDMA mailing list, Stephen Hemminger
In-Reply-To: <20190220072136.3840-3-leon@kernel.org>
On 2/20/19 2:21 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> There is no need to keep same print functions for
> uint32_t and uint64_t, unify them into one function.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> rdma/res.c | 22 +++++++---------------
> 1 file changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/rdma/res.c b/rdma/res.c
> index 6b0f5fe3..87865ec8 100644
> --- a/rdma/res.c
> +++ b/rdma/res.c
> @@ -808,28 +808,20 @@ static int res_cq_parse_cb(const struct nlmsghdr *nlh, void *data)
> return MNL_CB_OK;
> }
>
> -static void print_key(struct rd *rd, const char *name, uint32_t val)
> +static void print_key(struct rd *rd, const char *name, uint64_t val)
> {
> if (rd->json_output)
> jsonw_xint_field(rd->jw, name, val);
> else
> - pr_out("%s 0x%x ", name, val);
> + pr_out("%s 0x%" PRIx64 " ", name, val);
> }
what's the plan to move rdma tool to iproute2's json functions?
I realize rdma tool inherited this from devlink, but this command does
not need to continue managing this.
^ permalink raw reply
* Re: [PATCH net-next 2/2] xdp: Add devmap_idx map type for looking up devices by ifindex
From: Jesper Dangaard Brouer @ 2019-02-21 15:23 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: David Miller, netdev, Daniel Borkmann, Alexei Starovoitov, brouer
In-Reply-To: <155075021407.13610.6656977312753058829.stgit@alrua-x1>
On Thu, 21 Feb 2019 12:56:54 +0100
Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> The default maps used by xdp_redirect() are changed to use the new map
> type, which means that xdp_redirect() is no longer limited to ifindex < 64,
> but instead to 64 total simultaneous interfaces per network namespace. This
> also provides an easy way to compare the performance of devmap and
> devmap_idx:
>
> xdp_redirect_map (devmap): 8394560 pkt/s
> xdp_redirect (devmap_idx): 8179480 pkt/s
>
> Difference: 215080 pkt/s or 3.1 nanoseconds per packet.
(1/8394560-1/8179480)*10^9 = -3.13239 ns
But was the xdp_redirect_map code-path affected from patch 1/1?
(1/8412754-1/8394560)*10^9 = -0.2576 ns
It doesn't look like any performance regression to xdp_redirect_map
from these code changes :-)
--
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 iproute2-next v1 00/19] Export object IDs to users
From: David Ahern @ 2019-02-21 15:26 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Leon Romanovsky, netdev, RDMA mailing list, Stephen Hemminger
In-Reply-To: <20190220072136.3840-1-leon@kernel.org>
On 2/20/19 2:21 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Changelog:
> v0->v1:
> * Rebased to latest iproute2-next
> * Added latest rdma_netlink.h and updated commit message to point
> to kernel SHA commit.
>
> This series adds ability to present and query all known to rdmatool
> object by their respective, unique IDs (e.g. pdn. mrn, cqn e.t.c).
> All objects which have "parent" object has this information too.
>
> Thanks
>
I would prefer someone from the rdma world review this as well. Thanks
^ permalink raw reply
* Re: [PATCH v2 1/2] Provide in-kernel headers for making it easy to extend the kernel
From: Joel Fernandes @ 2019-02-21 15:29 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Alexei Starovoitov, Networking, Linux Kernel Mailing List,
Andrew Morton, Alexei Starovoitov, atish patra, Daniel Colascione,
Dan Williams, Greg Kroah-Hartman, Jonathan Corbet, Karim Yaghmour,
Kees Cook, kernel-team, open list:DOCUMENTATION,
open list:KERNEL SELFTEST FRAMEWORK, Manoj Rao, Paul McKenney,
Peter Zijlstra (Intel), Randy Dunlap, Steven Rostedt, Shuah Khan,
Thomas Gleixner, Yonghong Song
In-Reply-To: <CAK7LNAQ7GqCNc008ZZ4K0_BdTVMdwwVUgnfqBO-kDBmw4ikr3w@mail.gmail.com>
On Thu, Feb 21, 2019 at 11:34:41PM +0900, Masahiro Yamada wrote:
> On Wed, Feb 20, 2019 at 12:17 AM Joel Fernandes <joel@joelfernandes.org> wrote:
>
> >
> > Firstly, I want to apologize for not testing this and other corner cases you
> > brought up. I should have known better. Since my build was working, I assumed
> > that the feature is working. For that, I am very sorry.
>
>
> You do not need to apologize. 0day bot usually catches build errors.
> I guess 0day bot performs compile-tests only incrementally
> and that is why we did not get any report.
Oh ok :) thanks.
> > Secondly, it turns out Module.symvers circularly dependency problem also
> > exists with another use case.
> > If one does 'make modules_prepare' in a base kernel tree and then tries to
> > build modules with that tree, a warning like this is printed but the module
> > still gets built:
> >
> > WARNING: Symbol version dump ./Module.symvers
> > is missing; modules will have no dependencies and modversions.
> >
> > CC [M] /tmp/testmod/test.o
> > Building modules, stage 2.
> > MODPOST 1 modules
> > CC /tmp/testmod/test.mod.o
> > LD [M] /tmp/testmod/test.ko
> >
> > So, I am thinking that at least for first pass I will just drop the inclusion
> > of Module.symvers in the archive and allow any modules built using
> > /proc/kheaders.tar.xz to not use it.
> >
> > Kbuild will print a warning anyway when anyone tries to build using
> > /proc/kheaders.tar.xz, so if the user really wants module symbol versioning
> > then they should probably use a full kernel source tree with Module.symvers
> > available. For our usecase, kernel symbol versioning is a bit useless when
> > using /proc/kheaders.tar.gz because the proc file is generated with the same
> > kernel that the module is being built against, and subsequently loaded into
> > the kernel. So it is not likely that the CRC of a kernel symbol will be
> > different from what the module expects.
>
>
> Without Module.symver, modpost cannot check whether references are
> resolvable or not.
>
> You will see "WARNING ... undefined" for every symbol referenced from
> the module.
>
>
> I am not an Android developer.
> So, I will leave this judge to other people.
IMO I don't see a way around this limiation but it would be nice if there was
a way to make it work. Since the kernel modules being built by this mechanism
are for tracing/debugging purposes, it is not a major concern for us.
> One more request if you have a chance to submit the next version.
> Please do not hide error messages.
Actually it was intended to suppress noise, not hide errors as such. I have
fixed all the errors in the next version and will be submitting it soon.
Thanks a lot for the review!
- Joel
> I wondered why you redirected stdout/stderr from the script.
>
> I applied the following patch, and I tested. Then I see why.
>
> Please fix your code instead of hiding underlying problems.
>
>
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 1d13a7a..a76ccbd 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -148,7 +148,7 @@ $(obj)/kheaders.o: $(obj)/kheaders_data.h
> targets += kheaders_data.txz
>
> quiet_cmd_genikh = GEN $(obj)/kheaders_data.txz
> -cmd_genikh = $(srctree)/scripts/gen_ikh_data.sh $@ $^ >/dev/null 2>&1
> +cmd_genikh = $(srctree)/scripts/gen_ikh_data.sh $@ $^
> $(obj)/kheaders_data.txz: $(ikh_file_list) FORCE
> $(call cmd,genikh)
>
>
>
>
>
>
> masahiro@grover:~/workspace/linux-yamada$ make
> CALL scripts/checksyscalls.sh
> DESCEND objtool
> CHK include/generated/compile.h
> GEN kernel/kheaders_data.txz
> find: ‘FORCE’: No such file or directory
> 70106 blocks
> Can't do inplace edit: kernel/kheaders_data.txz.tmp is not a regular file.
> Can't do inplace edit: kernel/kheaders_data.txz.tmp/arch is not a regular file.
> Can't do inplace edit: kernel/kheaders_data.txz.tmp/arch/x86 is not a
> regular file.
> Can't do inplace edit: kernel/kheaders_data.txz.tmp/arch/x86/include
> is not a regular file.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/arch/x86/include/uapi is not a regular
> file.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/arch/x86/include/uapi/asm is not a
> regular file.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/arch/x86/include/generated is not a
> regular file.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/arch/x86/include/generated/uapi is not a
> regular file.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/arch/x86/include/generated/uapi/asm is
> not a regular file.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/arch/x86/include/generated/asm is not a
> regular file.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/arch/x86/include/asm is not a regular
> file.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/arch/x86/include/asm/xen is not a regular
> file.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/arch/x86/include/asm/uv is not a regular
> file.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/arch/x86/include/asm/numachip is not a
> regular file.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/arch/x86/include/asm/e820 is not a
> regular file.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/arch/x86/include/asm/fpu is not a regular
> file.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/arch/x86/include/asm/crypto is not a
> regular file.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/arch/x86/include/asm/trace is not a
> regular file.
> Can't do inplace edit: kernel/kheaders_data.txz.tmp/scripts is not a
> regular file.
> Can't do inplace edit: kernel/kheaders_data.txz.tmp/scripts/genksyms
> is not a regular file.
> Can't do inplace edit: kernel/kheaders_data.txz.tmp/scripts/ksymoops
> is not a regular file.
> Can't do inplace edit: kernel/kheaders_data.txz.tmp/scripts/gdb is not
> a regular file.
> Can't do inplace edit: kernel/kheaders_data.txz.tmp/scripts/gdb/linux
> is not a regular file.
> Can't do inplace edit: kernel/kheaders_data.txz.tmp/scripts/basic is
> not a regular file.
> Can't do inplace edit: kernel/kheaders_data.txz.tmp/scripts/dtc is not
> a regular file.
> Can't do inplace edit: kernel/kheaders_data.txz.tmp/scripts/dtc/libfdt
> is not a regular file.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/scripts/dtc/include-prefixes is not a
> regular file.
> Can't open kernel/kheaders_data.txz.tmp/scripts/dtc/include-prefixes/arm64:
> No such file or directory.
> Can't open kernel/kheaders_data.txz.tmp/scripts/dtc/include-prefixes/xtensa:
> No such file or directory.
> Can't open kernel/kheaders_data.txz.tmp/scripts/dtc/include-prefixes/openrisc:
> No such file or directory.
> Can't open kernel/kheaders_data.txz.tmp/scripts/dtc/include-prefixes/nios2:
> No such file or directory.
> Can't open kernel/kheaders_data.txz.tmp/scripts/dtc/include-prefixes/mips:
> No such file or directory.
> Can't open kernel/kheaders_data.txz.tmp/scripts/dtc/include-prefixes/arm:
> No such file or directory.
> Can't open kernel/kheaders_data.txz.tmp/scripts/dtc/include-prefixes/microblaze:
> No such file or directory.
> Can't open kernel/kheaders_data.txz.tmp/scripts/dtc/include-prefixes/arc:
> No such file or directory.
> Can't open kernel/kheaders_data.txz.tmp/scripts/dtc/include-prefixes/sh:
> No such file or directory.
> Can't open kernel/kheaders_data.txz.tmp/scripts/dtc/include-prefixes/powerpc:
> No such file or directory.
> Can't do inplace edit:
> kernel/kheaders_data.txz.tmp/scripts/dtc/include-prefixes/dt-bindings
> is not a regular file.
>
> [ massive amount of error messages continues ]
>
>
>
>
>
>
> > I can't think any other ways at the moment to break the circular dependency
> > so I'm thinking this is good enough for now especially since Kbuild will
> > print a proper warning. Let me know what you think?
> >
> > thanks,
> >
> > - Joel
> >
> --
> Best Regards
> Masahiro Yamada
^ permalink raw reply
* [PATCH] tcp: detect use sendpage for slab-based objects
From: Vasily Averin @ 2019-02-21 15:30 UTC (permalink / raw)
To: Eric Dumazet, netdev
There was few incidents when XFS over network block device generates
IO requests with slab-based metadata. If these requests are processed
via sendpage path tcp_sendpage() calls skb_can_coalesce() and merges
neighbour slab objects into one skb fragment.
If receiving side is located on the same host tcp_recvmsg() can trigger
following BUG_ON
usercopy: kernel memory exposure attempt detected
from XXXXXX (kmalloc-512) (1024 bytes)
This patch helps to detect the reason of similar incidents on sending side.
Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
net/ipv4/tcp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 2079145a3b7c..cf9572f4fc0f 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -996,6 +996,7 @@ ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
goto wait_for_memory;
if (can_coalesce) {
+ WARN_ON_ONCE(PageSlab(page));
skb_frag_size_add(&skb_shinfo(skb)->frags[i - 1], copy);
} else {
get_page(page);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH RFC 4/5] sched/topology: Annonate RCU pointers properly
From: Peter Zijlstra @ 2019-02-21 15:29 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ido Schimmel,
Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, Paul E. McKenney,
rcu, Song Liu, Steven Rostedt, xdp-newbies, Yonghong Song
In-Reply-To: <20190221151057.GA19213@google.com>
On Thu, Feb 21, 2019 at 10:10:57AM -0500, Joel Fernandes wrote:
> Hi Peter,
>
> Thanks for taking a look.
>
> On Thu, Feb 21, 2019 at 10:19:44AM +0100, Peter Zijlstra wrote:
> > On Thu, Feb 21, 2019 at 12:49:41AM -0500, Joel Fernandes (Google) wrote:
> >
> > > Also replace rcu_assign_pointer call on rq->sd with WRITE_ONCE. This
> > > should be sufficient for the rq->sd initialization.
> >
> > > @@ -668,7 +668,7 @@ cpu_attach_domain(struct sched_domain *sd, struct root_domain *rd, int cpu)
> > >
> > > rq_attach_root(rq, rd);
> > > tmp = rq->sd;
> > > - rcu_assign_pointer(rq->sd, sd);
> > > + WRITE_ONCE(rq->sd, sd);
> > > dirty_sched_domain_sysctl(cpu);
> > > destroy_sched_domains(tmp);
> >
> > Where did the RELEASE barrier go?
> >
> > That was a publish operation, now it is not.
>
> Funny thing is, initially I had written this patch with smp_store_release()
> instead of WRITE_ONCE, but checkpatch complaints with that since it needs a
> comment on top of it, and I wasn't sure if RELEASE barrier was the intent of
> using rcu_assign_pointer (all the more reason to replace it with something
> more explicit).
>
> I will replace it with the following and resubmit it then:
>
> /* Release barrier */
> smp_store_release(&rq->sd, sd);
>
> Or do we want to just drop the "Release barrier" comment and live with the
> checkpatch warning?
How about we keep using rcu_assign_pointer(), the whole sched domain
tree is under rcu; peruse that destroy_sched_domains() function for
instance.
Also check how for_each_domain() uses rcu_dereference().
^ permalink raw reply
* Re: [PATCH RFC 3/5] sched/cpufreq: Fix incorrect RCU API usage
From: Peter Zijlstra @ 2019-02-21 15:31 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ido Schimmel,
Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, Paul E. McKenney,
rcu, Song Liu, Steven Rostedt, xdp-newbies, Yonghong Song
In-Reply-To: <20190221152139.GB19213@google.com>
On Thu, Feb 21, 2019 at 10:21:39AM -0500, Joel Fernandes wrote:
> On Thu, Feb 21, 2019 at 10:18:05AM +0100, Peter Zijlstra wrote:
> > On Thu, Feb 21, 2019 at 12:49:40AM -0500, Joel Fernandes (Google) wrote:
> > > @@ -34,8 +34,12 @@ void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
> > > if (WARN_ON(!data || !func))
> > > return;
> > >
> > > - if (WARN_ON(per_cpu(cpufreq_update_util_data, cpu)))
> > > + rcu_read_lock();
> > > + if (WARN_ON(rcu_dereference(per_cpu(cpufreq_update_util_data, cpu)))) {
> > > + rcu_read_unlock();
> > > return;
> > > + }
> > > + rcu_read_unlock();
> > >
> > > data->func = func;
> > > rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
> >
> > This doesn't make any kind of sense to me.
> >
>
> As per the rcu_assign_pointer() line, I inferred that
> cpufreq_update_util_data is expected to be RCU protected. Reading the pointer
> value of RCU pointers generally needs to be done from RCU read section, and
> using rcu_dereference() (or using rcu_access()).
>
> In this patch, I changed cpufreq_update_util_data to be __rcu annotated to
> avoid the sparse error thrown by rcu_assign_pointer().
>
> Instead of doing that, If your intention here is RELEASE barrier, should I
> just replace in this function:
> rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
> with:
> smp_store_release(per_cpu(cpufreq_update_util_data, cpu), data))
> ?
>
> It would be nice IMO to be explicit about the intention of release/publish
> semantics by using smp_store_release().
No, it is RCU managed, it should be RCU. The problem is that the hunk
above is utter crap.
All that does is read the pointer, it never actually dereferences it.
^ permalink raw reply
* Re: [PATCH bpf] Revert "xsk: simplify AF_XDP socket teardown"
From: Daniel Borkmann @ 2019-02-21 15:36 UTC (permalink / raw)
To: Björn Töpel, ast, netdev
Cc: Björn Töpel, magnus.karlsson, magnus.karlsson
In-Reply-To: <20190221120738.24608-1-bjorn.topel@gmail.com>
On 02/21/2019 01:07 PM, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
>
> This reverts commit e2ce3674883ecba2605370404208c9d4a07ae1c3.
>
> It turns out that the sock destructor xsk_destruct was needed after
> all. The cleanup simplification broke the skb transmit cleanup path,
> due to that the umem was prematurely destroyed.
>
> The umem cannot be destroyed until all outstanding skbs are freed,
> which means that we cannot remove the umem until the sk_destruct has
> been called.
>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH] bpf: test_bpf: turn of preemption in function __run_once
From: Daniel Borkmann @ 2019-02-21 15:37 UTC (permalink / raw)
To: Anders Roxell, ast; +Cc: netdev, linux-kernel
In-Reply-To: <20190221084425.9574-1-anders.roxell@linaro.org>
On 02/21/2019 09:44 AM, Anders Roxell wrote:
> When running test seccomp_bpf the following splat occurs:
>
> [ RUN ] global.secseccomp_bpf.c:2136:global.detect_seccomp_filter_flags:Expected 22 (22) == (*__errno_location ()) (14)
> seccomp_bpf.c:2138:global.detect_seccomp_filter_flags:Failed to detect that an unknown
> filter flag (0x8) is unsupported! Does a new flag need to be added to this test?
> [ 2155.677841] BUG: assuming atomic context at kernel/seccomp.c:271
> [ 2155.689351] in_atomic(): 0, irqs_disabled(): 0, pid: 28540, name: seccomp_bpf
> [ 2155.696597] INFO: lockdep is turned off.
> [ 2155.700605] CPU: 5 PID: 28540 Comm: seccomp_bpf Tainted: G W 5.0.0-rc7-next-20190220 #1
> [ 2155.709972] Hardware name: HiKey Development Board (DT)
> [ 2155.715232] Call trace:
> [ 2155.717710] dump_backtrace+0x0/0x160
> [ 2155.721399] show_stack+0x24/0x30
> [ 2155.724742] dump_stack+0xc8/0x114
> [ 2155.728172] __cant_sleep+0xf0/0x108
> [ 2155.731777] __seccomp_filter+0x8c/0x5c8
> [ 2155.735727] __secure_computing+0x4c/0xe8
> [ 2155.739767] syscall_trace_enter+0xf8/0x2b8
> [ 2155.743982] el0_svc_common+0xf0/0x130
> [ 2155.747758] el0_svc_handler+0x38/0x78
> [ 2155.751534] el0_svc+0x8/0xc
>
> Rework so that preemption is disabled when we loop over function
> 'BPF_PROG_RUN(...)'.
> Commit 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
> highlighted the issue.
>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
Hmm, wrong commit description? Below code is not related to seccomp
but rather BPF test suite. Could you fix it up and resubmit? Rest
looks okay to me.
> ---
> lib/test_bpf.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/lib/test_bpf.c b/lib/test_bpf.c
> index f3e570722a7e..0845f635f404 100644
> --- a/lib/test_bpf.c
> +++ b/lib/test_bpf.c
> @@ -6668,12 +6668,14 @@ static int __run_one(const struct bpf_prog *fp, const void *data,
> u64 start, finish;
> int ret = 0, i;
>
> + preempt_disable();
> start = ktime_get_ns();
>
> for (i = 0; i < runs; i++)
> ret = BPF_PROG_RUN(fp, data);
>
> finish = ktime_get_ns();
> + preempt_enable();
>
> *duration = finish - start;
> do_div(*duration, runs);
>
^ permalink raw reply
* Re: [PATCH iproute2-next v1 02/19] rdma: Remove duplicated print code
From: Leon Romanovsky @ 2019-02-21 15:39 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, RDMA mailing list, Stephen Hemminger
In-Reply-To: <c6f27ca0-55d6-4d3b-e498-ed700eb00844@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1294 bytes --]
On Thu, Feb 21, 2019 at 10:23:01AM -0500, David Ahern wrote:
> On 2/20/19 2:21 AM, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > There is no need to keep same print functions for
> > uint32_t and uint64_t, unify them into one function.
> >
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---
> > rdma/res.c | 22 +++++++---------------
> > 1 file changed, 7 insertions(+), 15 deletions(-)
> >
> > diff --git a/rdma/res.c b/rdma/res.c
> > index 6b0f5fe3..87865ec8 100644
> > --- a/rdma/res.c
> > +++ b/rdma/res.c
> > @@ -808,28 +808,20 @@ static int res_cq_parse_cb(const struct nlmsghdr *nlh, void *data)
> > return MNL_CB_OK;
> > }
> >
> > -static void print_key(struct rd *rd, const char *name, uint32_t val)
> > +static void print_key(struct rd *rd, const char *name, uint64_t val)
> > {
> > if (rd->json_output)
> > jsonw_xint_field(rd->jw, name, val);
> > else
> > - pr_out("%s 0x%x ", name, val);
> > + pr_out("%s 0x%" PRIx64 " ", name, val);
> > }
>
> what's the plan to move rdma tool to iproute2's json functions?
>
> I realize rdma tool inherited this from devlink, but this command does
> not need to continue managing this.
I can work on this next cycle, after I'll finish to code some obligations
in rdma-core.
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* [PATCH net-next] ip_tunnel: Add dst_cache management lwtunnel_state of ip tunnel
From: wenxu @ 2019-02-21 15:41 UTC (permalink / raw)
To: netdev, davem
From: wenxu <wenxu@ucloud.cn>
The lwtunnel_state is not init the dst_cache Which make the
ip_md_tunnel_xmit can't use the dst_cache. It will lookup
route table every packets.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
net/ipv4/ip_tunnel_core.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index 9a0e67b..6168b90 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -252,6 +252,12 @@ static int ip_tun_build_state(struct nlattr *attr,
tun_info = lwt_tun_info(new_state);
+ err = dst_cache_init(&tun_info->dst_cache, GFP_ATOMIC);
+ if (err) {
+ lwtstate_free(new_state);
+ return err;
+ }
+
if (tb[LWTUNNEL_IP_ID])
tun_info->key.tun_id = nla_get_be64(tb[LWTUNNEL_IP_ID]);
@@ -278,6 +284,13 @@ static int ip_tun_build_state(struct nlattr *attr,
return 0;
}
+static int ip_tun_destroy_state(struct lwtunnel_state *lwtstate)
+{
+ struct ip_tunnel_info *tun_info = lwt_tun_info(lwtstate);
+
+ dst_cache_destroy(&tun_info->dst_cache);
+}
+
static int ip_tun_fill_encap_info(struct sk_buff *skb,
struct lwtunnel_state *lwtstate)
{
@@ -313,6 +326,7 @@ static int ip_tun_cmp_encap(struct lwtunnel_state *a, struct lwtunnel_state *b)
static const struct lwtunnel_encap_ops ip_tun_lwt_ops = {
.build_state = ip_tun_build_state,
+ .destroy_state = ip_tun_destroy_state,
.fill_encap = ip_tun_fill_encap_info,
.get_encap_size = ip_tun_encap_nlsize,
.cmp_encap = ip_tun_cmp_encap,
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next 2/2] xdp: Add devmap_idx map type for looking up devices by ifindex
From: Toke Høiland-Jørgensen @ 2019-02-21 15:50 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: David Miller, netdev, Daniel Borkmann, Alexei Starovoitov, brouer
In-Reply-To: <20190221162348.51d739a0@carbon>
Jesper Dangaard Brouer <brouer@redhat.com> writes:
> On Thu, 21 Feb 2019 12:56:54 +0100
> Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
>> The default maps used by xdp_redirect() are changed to use the new map
>> type, which means that xdp_redirect() is no longer limited to ifindex < 64,
>> but instead to 64 total simultaneous interfaces per network namespace. This
>> also provides an easy way to compare the performance of devmap and
>> devmap_idx:
>>
>> xdp_redirect_map (devmap): 8394560 pkt/s
>> xdp_redirect (devmap_idx): 8179480 pkt/s
>>
>> Difference: 215080 pkt/s or 3.1 nanoseconds per packet.
>
> (1/8394560-1/8179480)*10^9 = -3.13239 ns
>
> But was the xdp_redirect_map code-path affected from patch 1/1?
> (1/8412754-1/8394560)*10^9 = -0.2576 ns
>
> It doesn't look like any performance regression to xdp_redirect_map
> from these code changes :-)
Nope, the difference between the two patches is just random noise; the
numbers vary more between runs (or even between samples in the same
run), which is why I didn't mention that difference.
-Toke
^ permalink raw reply
* Re: [PATCH net-next v4 07/17] net: sched: protect filter_chain list with filter_chain_lock mutex
From: Vlad Buslov @ 2019-02-21 15:49 UTC (permalink / raw)
To: Cong Wang
Cc: Ido Schimmel, netdev@vger.kernel.org, jhs@mojatatu.com,
jiri@resnulli.us, davem@davemloft.net, ast@kernel.org,
daniel@iogearbox.net
In-Reply-To: <CAM_iQpUDPd8b=K9+0WgBi1QihVJu8i+gUAfRdrdRej-6dUziCQ@mail.gmail.com>
On Wed 20 Feb 2019 at 22:43, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, Feb 19, 2019 at 4:31 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>>
>>
>> On Tue 19 Feb 2019 at 05:26, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> > On Fri, Feb 15, 2019 at 7:35 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>> >>
>> >> Another problem that I found in cls_fw and cls_route is that they set
>> >> arg->stop when empty. Both of them have code unchanged since it was
>> >> committed initially in 2005 so I assume this convention is no longer
>> >> relevant because all other classifiers don't do that (they only set
>> >> arg->stop when arg->fn returns negative value).
>> >>
>> >
>> > The question is why do you want to use arg->stop==0 as
>> > an indication for emptiness? Isn't what arg->count==0
>> > supposed to be?
>>
>> Good question! I initially wanted to implement it like that, but
>> reconsidered because iterating through all filters on classifier to
>> count them is O(N), and terminating on first filter and relying on
>> arg->stop==1 is constant time. Making function that is called
>> "tcf_proto_is_empty" linear on number of filters seemed sloppy to me...
>
> Good point, however arg->stop _was_ supposed to set only when
> error happens. Probably you want a new arg here to stop on the first
> entry.
Got it. I'll prepare a patch for that.
^ permalink raw reply
* Re: [PATCH RFC 3/5] sched/cpufreq: Fix incorrect RCU API usage
From: Paul E. McKenney @ 2019-02-21 15:52 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Joel Fernandes, linux-kernel, Alexei Starovoitov,
Christian Brauner, Daniel Borkmann, David Ahern, David S. Miller,
Ido Schimmel, Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, rcu, Song Liu,
Steven Rostedt, xdp-newbies, Yonghong Song
In-Reply-To: <20190221153117.GT32494@hirez.programming.kicks-ass.net>
On Thu, Feb 21, 2019 at 04:31:17PM +0100, Peter Zijlstra wrote:
> On Thu, Feb 21, 2019 at 10:21:39AM -0500, Joel Fernandes wrote:
> > On Thu, Feb 21, 2019 at 10:18:05AM +0100, Peter Zijlstra wrote:
> > > On Thu, Feb 21, 2019 at 12:49:40AM -0500, Joel Fernandes (Google) wrote:
> > > > @@ -34,8 +34,12 @@ void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
> > > > if (WARN_ON(!data || !func))
> > > > return;
> > > >
> > > > - if (WARN_ON(per_cpu(cpufreq_update_util_data, cpu)))
> > > > + rcu_read_lock();
> > > > + if (WARN_ON(rcu_dereference(per_cpu(cpufreq_update_util_data, cpu)))) {
> > > > + rcu_read_unlock();
> > > > return;
> > > > + }
> > > > + rcu_read_unlock();
> > > >
> > > > data->func = func;
> > > > rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
> > >
> > > This doesn't make any kind of sense to me.
> > >
> >
> > As per the rcu_assign_pointer() line, I inferred that
> > cpufreq_update_util_data is expected to be RCU protected. Reading the pointer
> > value of RCU pointers generally needs to be done from RCU read section, and
> > using rcu_dereference() (or using rcu_access()).
> >
> > In this patch, I changed cpufreq_update_util_data to be __rcu annotated to
> > avoid the sparse error thrown by rcu_assign_pointer().
> >
> > Instead of doing that, If your intention here is RELEASE barrier, should I
> > just replace in this function:
> > rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
> > with:
> > smp_store_release(per_cpu(cpufreq_update_util_data, cpu), data))
> > ?
> >
> > It would be nice IMO to be explicit about the intention of release/publish
> > semantics by using smp_store_release().
>
> No, it is RCU managed, it should be RCU. The problem is that the hunk
> above is utter crap.
>
> All that does is read the pointer, it never actually dereferences it.
For whatever it is worth, in that case it could use rcu_access_pointer().
And this primitive does not do the lockdep check for being within an RCU
read-side critical section. As Peter says, if there is no dereferencing,
there can be no use-after-free bug, so the RCU read-side critical is
not needed.
Good eyes, Peter! ;-)
Thanx, Paul
^ permalink raw reply
* Re: [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT to a device
From: Toke Høiland-Jørgensen @ 2019-02-21 15:52 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: David Miller, netdev, Daniel Borkmann, Alexei Starovoitov, brouer
In-Reply-To: <20190221161950.1938cc8c@carbon>
Jesper Dangaard Brouer <brouer@redhat.com> writes:
> You forgot at cover letter describing why we are doing this...
> even-though is should be obvious from the performance results ;-)
Well, I tried to put the motivation into the first paragraph of each
patch description instead of as a separate cover letter. I guess I could
have put it in a separate cover letter as well, but that was actually a
deliberate omission in this case ;)
-Toke
^ permalink raw reply
* Re: [PATCH] bpfilter: remove extra header search paths for bpfilter_umh
From: Masahiro Yamada @ 2019-02-21 15:54 UTC (permalink / raw)
To: Guenter Roeck
Cc: David S. Miller, Alexei Starovoitov, Daniel Borkmann, Networking,
Linux Kernel Mailing List
In-Reply-To: <20190221144257.GA12892@roeck-us.net>
On Thu, Feb 21, 2019 at 11:46 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Thu, Jan 31, 2019 at 12:15:35PM +0900, Masahiro Yamada wrote:
> > Currently, the header search paths -Itools/include and
> > -Itools/include/uapi are not used. Let's drop the unused code.
> >
> > We can remove -I. too by fixing up one C file.
> >
>
> This patch reintroduces the problem last fixed with commit ae40832e53c3
> ("bpfilter: fix a build err"). Seen (at least) with gcc 7.4.0, 8.2.0.
> binutils version is 2.31.1. Reverting this patch fixes the problem.
Hmm. I cannot reproduce the build error with my gcc,
but you are right.
I'd like to get back only
'KBUILD_HOSTCFLAGS += -Itools/include/ -Itools/include/uapi'
instead of the full revert.
If David is fine with it, I can send a patch with filling commit log.
Thanks.
> Guenter
>
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > ---
> > net/bpfilter/Makefile | 1 -
> > net/bpfilter/main.c | 2 +-
> > 2 files changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/net/bpfilter/Makefile b/net/bpfilter/Makefile
> > index 0947ee7f70d5..5d6c7760142d 100644
> > --- a/net/bpfilter/Makefile
> > +++ b/net/bpfilter/Makefile
> > @@ -5,7 +5,6 @@
> >
> > hostprogs-y := bpfilter_umh
> > bpfilter_umh-objs := main.o
> > -KBUILD_HOSTCFLAGS += -I. -Itools/include/ -Itools/include/uapi
> > HOSTCC := $(CC)
> >
> > ifeq ($(CONFIG_BPFILTER_UMH), y)
> > diff --git a/net/bpfilter/main.c b/net/bpfilter/main.c
> > index 1317f108df8a..61ce8454a88e 100644
> > --- a/net/bpfilter/main.c
> > +++ b/net/bpfilter/main.c
> > @@ -6,7 +6,7 @@
> > #include <sys/socket.h>
> > #include <fcntl.h>
> > #include <unistd.h>
> > -#include "include/uapi/linux/bpf.h"
> > +#include "../../include/uapi/linux/bpf.h"
> > #include <asm/unistd.h>
> > #include "msgfmt.h"
> >
> > --
> > 2.7.4
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH] tcp: detect use sendpage for slab-based objects
From: Eric Dumazet @ 2019-02-21 16:00 UTC (permalink / raw)
To: Vasily Averin; +Cc: netdev
In-Reply-To: <a8655149-80b9-c75d-6528-0b851ea85de8@virtuozzo.com>
On Thu, Feb 21, 2019 at 7:30 AM Vasily Averin <vvs@virtuozzo.com> wrote:
>
> There was few incidents when XFS over network block device generates
> IO requests with slab-based metadata. If these requests are processed
> via sendpage path tcp_sendpage() calls skb_can_coalesce() and merges
> neighbour slab objects into one skb fragment.
>
> If receiving side is located on the same host tcp_recvmsg() can trigger
> following BUG_ON
> usercopy: kernel memory exposure attempt detected
> from XXXXXX (kmalloc-512) (1024 bytes)
>
> This patch helps to detect the reason of similar incidents on sending side.
>
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
> ---
> net/ipv4/tcp.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 2079145a3b7c..cf9572f4fc0f 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -996,6 +996,7 @@ ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
> goto wait_for_memory;
>
> if (can_coalesce) {
> + WARN_ON_ONCE(PageSlab(page));
Please use VM_WARN_ON_ONCE() to make this a nop for CONFIG_VM_DEBUG=n
Also the whole tcp_sendpage() should be protected, not only the coalescing part.
(The get_page() done few lines later should not be attempted either)
> skb_frag_size_add(&skb_shinfo(skb)->frags[i - 1], copy);
> } else {
> get_page(page);
> --
> 2.17.1
>
It seems the bug has nothing to do with TCP, and belongs to the caller.
Otherwise you need to add the check to all existing .sendpage() /
.sendpage_locked() handler out there.
^ permalink raw reply
* [PATCH] samples/bpf: Fix dummy program unloading for xdp_redirect samples
From: Toke Høiland-Jørgensen @ 2019-02-21 16:05 UTC (permalink / raw)
To: netdev; +Cc: Toke Høiland-Jørgensen
The xdp_redirect and xdp_redirect_map sample programs both load a dummy
program onto the egress interfaces. However, the unload code checks these
programs against the wrong fd number, and thus refuses to unload them. Fix
the comparison to avoid this.
Fixes: 3b7a8ec2dec3 ("samples/bpf: Check the prog id before exiting")
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
samples/bpf/xdp_redirect_map_user.c | 2 +-
samples/bpf/xdp_redirect_user.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/samples/bpf/xdp_redirect_map_user.c b/samples/bpf/xdp_redirect_map_user.c
index 327226be5a06..1dbe7fd3a1a8 100644
--- a/samples/bpf/xdp_redirect_map_user.c
+++ b/samples/bpf/xdp_redirect_map_user.c
@@ -57,7 +57,7 @@ static void int_exit(int sig)
printf("bpf_get_link_xdp_id failed\n");
exit(1);
}
- if (prog_id == curr_prog_id)
+ if (dummy_prog_id == curr_prog_id)
bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
else if (!curr_prog_id)
printf("couldn't find a prog id on iface OUT\n");
diff --git a/samples/bpf/xdp_redirect_user.c b/samples/bpf/xdp_redirect_user.c
index a5d8ad3129ed..e9054c0269ff 100644
--- a/samples/bpf/xdp_redirect_user.c
+++ b/samples/bpf/xdp_redirect_user.c
@@ -57,7 +57,7 @@ static void int_exit(int sig)
printf("bpf_get_link_xdp_id failed\n");
exit(1);
}
- if (prog_id == curr_prog_id)
+ if (dummy_prog_id == curr_prog_id)
bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
else if (!curr_prog_id)
printf("couldn't find a prog id on iface OUT\n");
--
2.20.1
^ permalink raw reply related
* Re: [PATCH RFC 3/5] sched/cpufreq: Fix incorrect RCU API usage
From: Peter Zijlstra @ 2019-02-21 16:11 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Joel Fernandes, linux-kernel, Alexei Starovoitov,
Christian Brauner, Daniel Borkmann, David Ahern, David S. Miller,
Ido Schimmel, Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, rcu, Song Liu,
Steven Rostedt, xdp-newbies, Yonghong Song
In-Reply-To: <20190221155218.GZ11787@linux.ibm.com>
On Thu, Feb 21, 2019 at 07:52:18AM -0800, Paul E. McKenney wrote:
> On Thu, Feb 21, 2019 at 04:31:17PM +0100, Peter Zijlstra wrote:
> > On Thu, Feb 21, 2019 at 10:21:39AM -0500, Joel Fernandes wrote:
> > > On Thu, Feb 21, 2019 at 10:18:05AM +0100, Peter Zijlstra wrote:
> > > > On Thu, Feb 21, 2019 at 12:49:40AM -0500, Joel Fernandes (Google) wrote:
> > > > > @@ -34,8 +34,12 @@ void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
> > > > > if (WARN_ON(!data || !func))
> > > > > return;
> > > > >
> > > > > - if (WARN_ON(per_cpu(cpufreq_update_util_data, cpu)))
> > > > > + rcu_read_lock();
> > > > > + if (WARN_ON(rcu_dereference(per_cpu(cpufreq_update_util_data, cpu)))) {
> > > > > + rcu_read_unlock();
> > > > > return;
> > > > > + }
> > > > > + rcu_read_unlock();
> > > > >
> > > > > data->func = func;
> > > > > rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
> For whatever it is worth, in that case it could use rcu_access_pointer().
> And this primitive does not do the lockdep check for being within an RCU
> read-side critical section. As Peter says, if there is no dereferencing,
> there can be no use-after-free bug, so the RCU read-side critical is
> not needed.
On top of that, I suspect this is under the write-side lock (we're doing
assignment after all).
^ permalink raw reply
* Re: [PATCH RFC 3/5] sched/cpufreq: Fix incorrect RCU API usage
From: Steven Rostedt @ 2019-02-21 16:17 UTC (permalink / raw)
To: Joel Fernandes (Google)
Cc: linux-kernel, Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ido Schimmel,
Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, Paul E. McKenney,
Peter Zijlstra, rcu, Song Liu, xdp-newbies, Yonghong Song,
Rafael J. Wysocki
In-Reply-To: <20190221054942.132388-4-joel@joelfernandes.org>
On Thu, 21 Feb 2019 00:49:40 -0500
"Joel Fernandes (Google)" <joel@joelfernandes.org> wrote:
> Recently I added an RCU annotation check to rcu_assign_pointer(). All
> pointers assigned to RCU protected data are to be annotated with __rcu
> inorder to be able to use rcu_assign_pointer() similar to checks in
> other RCU APIs.
>
> This resulted in a sparse error: kernel//sched/cpufreq.c:41:9: sparse:
> error: incompatible types in comparison expression (different address
> spaces)
>
> Fix this by using the correct APIs for RCU accesses. This will
> potentially avoid any future bugs in the code. If it is felt that RCU
> protection is not needed here, then the rcu_assign_pointer call can be
> dropped and replaced with, say, WRITE_ONCE or smp_store_release. Or, may
> be we add a new API to do it. But calls rcu_assign_pointer seems an
> abuse of the RCU API unless RCU is being used.
This all looks broken, and this patch is papering over the issue, or
worse, hiding it.
>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
> kernel/sched/cpufreq.c | 8 ++++++--
> kernel/sched/sched.h | 2 +-
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/sched/cpufreq.c b/kernel/sched/cpufreq.c
> index 22bd8980f32f..c9aeb3bf5dc2 100644
> --- a/kernel/sched/cpufreq.c
> +++ b/kernel/sched/cpufreq.c
> @@ -7,7 +7,7 @@
> */
> #include "sched.h"
>
> -DEFINE_PER_CPU(struct update_util_data *, cpufreq_update_util_data);
> +DEFINE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data);
>
> /**
> * cpufreq_add_update_util_hook - Populate the CPU's update_util_data pointer.
> @@ -34,8 +34,12 @@ void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
> if (WARN_ON(!data || !func))
> return;
>
> - if (WARN_ON(per_cpu(cpufreq_update_util_data, cpu)))
> + rcu_read_lock();
> + if (WARN_ON(rcu_dereference(per_cpu(cpufreq_update_util_data, cpu)))) {
> + rcu_read_unlock();
> return;
> + }
> + rcu_read_unlock();
>
> data->func = func;
> rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
An rcu_assign_pointer() is to update something that is going to be read
under rcu_read_lock() elsewhere. But updates to an rcu variable are not
protected by rcu_read_lock() (hence the "read" in the name). Adding
rcu_read_lock() above does nothing, but perhaps hides an issue.
Writes usually have something else that protects against races. Thus,
the above shouldn't be switched to using a rcu_dereference(), but
perhaps a rcu_dereference_protected(), with whatever is protecting
updates?
Which doing a bit of investigating, looks to be the rwsem
"policy->rwsem", where policy comes from:
policy = cpufreq_cpu_get(cpu);
I would say the code as is, is not broken. But this patch isn't helping
anything.
-- Steve
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index d04530bf251f..2ab545d40381 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -2166,7 +2166,7 @@ static inline u64 irq_time_read(int cpu)
> #endif /* CONFIG_IRQ_TIME_ACCOUNTING */
>
> #ifdef CONFIG_CPU_FREQ
> -DECLARE_PER_CPU(struct update_util_data *, cpufreq_update_util_data);
> +DECLARE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data);
>
> /**
> * cpufreq_update_util - Take a note about CPU utilization changes.
^ 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