* Re: [PATCH v6 3/4] Bluetooth: Allow driver specific cmd timeout handling
From: Marcel Holtmann @ 2019-01-25 7:51 UTC (permalink / raw)
To: Rajat Jain
Cc: Johan Hedberg, Greg Kroah-Hartman, David S. Miller,
Dmitry Torokhov, Alex Hung, linux-bluetooth, linux-kernel,
linux-usb, netdev, rajatxjain, dtor, raghuram.hegde,
chethan.tumkur.narayan, sukumar.ghorai
In-Reply-To: <20190124232814.252661-3-rajatja@google.com>
Hi Rajat,
> Add a hook to allow the BT driver to do device or command specific
> handling in case of timeouts. This is to be used by Intel driver to
> reset the device after certain number of timeouts.
>
> Signed-off-by: Rajat Jain <rajatja@google.com>
> ---
> v6: Dropped the "sent command" parameter from cmd_timeout()
> v5: Drop the quirk, and rename the hook function to cmd_timeout()
> v4: same as v1
> v3: same as v1
> v2: same as v1
>
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_core.c | 3 +++
> 2 files changed, 4 insertions(+)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v6 2/4] usb: assign ACPI companions for embedded USB devices
From: Marcel Holtmann @ 2019-01-25 7:51 UTC (permalink / raw)
To: Rajat Jain
Cc: Johan Hedberg, Greg Kroah-Hartman, David S. Miller,
Dmitry Torokhov, Alex Hung, linux-bluetooth, linux-kernel,
linux-usb, netdev, rajatxjain, dtor, raghuram.hegde,
chethan.tumkur.narayan, sukumar.ghorai
In-Reply-To: <20190124232814.252661-2-rajatja@google.com>
Hi Rajat,
> USB devices permanently connected to USB ports may be described in ACPI
> tables and share ACPI devices with ports they are connected to. See [1]
> for details.
>
> This will allow us to describe sideband resources for devices, such as,
> for example, hard reset line for BT USB controllers.
>
> [1] https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/other-acpi-namespace-objects#acpi-namespace-hierarchy-and-adr-for-embedded-usb-devices
>
> Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
> Signed-off-by: Rajat Jain <rajatja@google.com> (changed how we get the usb_port)
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Tested-by: Sukumar Ghorai <sukumar.ghorai@intel.com>
> ---
> v6: same as v4
> v5: same as v4
> v4: Add Acked-by and Tested-by in signatures.
> v3: same as v1
> v2: same as v1
>
> drivers/usb/core/usb-acpi.c | 44 +++++++++++++++++++++++++++++--------
> 1 file changed, 35 insertions(+), 9 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v6 1/4] usb: split code locating ACPI companion into port and device
From: Marcel Holtmann @ 2019-01-25 7:51 UTC (permalink / raw)
To: Rajat Jain
Cc: Johan Hedberg, Greg Kroah-Hartman, David S. Miller,
Dmitry Torokhov, Alex Hung, Bluez mailing list,
Linux Kernel Mailing List, linux-usb, netdev, rajatxjain, dtor,
Raghuram Hegde, Chethan T N, sukumar.ghorai
In-Reply-To: <20190124232814.252661-1-rajatja@google.com>
Hi Rajat,
> In preparation for handling embedded USB devices let's split
> usb_acpi_find_companion() into usb_acpi_find_companion_for_device() and
> usb_acpi_find_companion_for_port().
>
> Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
> Signed-off-by: Rajat Jain <rajatja@google.com>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Tested-by: Sukumar Ghorai <sukumar.ghorai@intel.com>
> ---
> v6: same as v4
> v5: same as v4
> v4: Add Acked-by and Tested-by in signatures.
> v3: same as v1
> v2: same as v1
>
> drivers/usb/core/usb-acpi.c | 133 +++++++++++++++++++-----------------
> 1 file changed, 72 insertions(+), 61 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] Bluetooth: make hw_err static, reduces object code size
From: Marcel Holtmann @ 2019-01-25 7:54 UTC (permalink / raw)
To: Colin King
Cc: Johan Hedberg, David S. Miller, linux-bluetooth, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20190124172254.13186-1-colin.king@canonical.com>
Hi Colin,
> Don't populate the const array hw_err on the stack but instead make
> it static. Makes the object code smaller by 45 bytes:
>
> Before:
> text data bss dec hex filename
> 100880 21090 1088 123058 1e0b2 linux/net/bluetooth/hci_core.o
>
> After:
> text data bss dec hex filename
> 100739 21186 1088 123013 1e085 linux/net/bluetooth/hci_core.o
>
> (gcc version 8.2.0 x86_64)
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> net/bluetooth/hci_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: Annotate implicit fall through in l2cap_config_rsp
From: Marcel Holtmann @ 2019-01-25 7:56 UTC (permalink / raw)
To: Mathieu Malaterre
Cc: Gustavo Padovan, Johan Hedberg, David S. Miller, linux-bluetooth,
netdev, linux-kernel
In-Reply-To: <20190124180724.20910-1-malat@debian.org>
Hi Mathieu,
> There is a plan to build the kernel with -Wimplicit-fallthrough and
> this place in the code produced a warning (W=1).
>
> This commit removes the following warning:
>
> net/bluetooth/l2cap_core.c:4223:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
>
> Cc: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
> net/bluetooth/l2cap_core.c | 1 +
> 1 file changed, 1 insertion(+)
the patches don’t apply cleanly against bluetooth-next tree. Please fix it up and resend.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH RFC 3/3] udp: Support UDP fraglist GRO/GSO.
From: Steffen Klassert @ 2019-01-25 7:58 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev, Willem de Bruijn, Jason A. Donenfeld
In-Reply-To: <7e001c0f99f688da4be762528ddc9287f49797fc.camel@redhat.com>
On Tue, Jan 08, 2019 at 04:00:01PM +0100, Paolo Abeni wrote:
>
> I think we could still avoid the lookup when no vxlan/GRO sockets are
> present moving the lookup into udp{4,6}_gro_receive. Very roughly
> something alike:
>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index f79f1b5b2f9e..b0c0983eac6b 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -420,20 +420,16 @@ static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
> INDIRECT_CALLABLE_DECLARE(struct sock *udp6_lib_lookup_skb(struct sk_buff *skb,
> __be16 sport, __be16 dport));
> struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
> - struct udphdr *uh, udp_lookup_t lookup)
> + struct udphdr *uh, struct sock *sk)
> {
> struct sk_buff *pp = NULL;
> struct sk_buff *p;
> struct udphdr *uh2;
> unsigned int off = skb_gro_offset(skb);
> int flush = 1;
> - struct sock *sk;
>
> - rcu_read_lock();
> - sk = INDIRECT_CALL_INET(lookup, udp6_lib_lookup_skb,
> - udp4_lib_lookup_skb, skb, uh->source, uh->dest);
> - if (!sk) {
> - NAPI_GRO_CB(skb)->is_flist = 1;
> + if (!sk || !udp_sk(sk)->gro_receive) {
> + NAPI_GRO_CB(skb)->is_flist = sk ? !udp_sk(sk)->gro_enabled: 1;
> pp = call_gro_receive(udp_gro_receive_segment, head, skb);
> rcu_read_unlock();
> return pp;
> @@ -506,7 +502,12 @@ struct sk_buff *udp4_gro_receive(struct list_head *head, struct sk_buff *skb)
> inet_gro_compute_pseudo);
> skip:
> NAPI_GRO_CB(skb)->is_ipv6 = 0;
> - return udp_gro_receive(head, skb, uh, udp4_lib_lookup_skb);
> + rcu_read_lock();
> + sk = static_branch_unlikely(&udp_encap_needed_key) ?
> + udp4_lib_lookup_skb(skb, uh->source, uh->dest) : NULL;
> + pp = udp_gro_receive(head, skb, uh, sk);
> + rcu_read_unlock();
> + return pp;
>
> flush:
> NAPI_GRO_CB(skb)->flush = 1;
> ---
>
> Regardless of the above, I think we should drop the later check for
> gro_receive:
>
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -450,8 +450,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
> if (NAPI_GRO_CB(skb)->encap_mark ||
> (skb->ip_summed != CHECKSUM_PARTIAL &&
> NAPI_GRO_CB(skb)->csum_cnt == 0 &&
> - !NAPI_GRO_CB(skb)->csum_valid) ||
> - !udp_sk(sk)->gro_receive)
> + !NAPI_GRO_CB(skb)->csum_valid))
> goto out_unlock;
>
> /* mark that this skb passed once through the tunnel gro layer */
> ---
I've incorporated the above into the next RFC patchset.
> Finally this will cause GRO/GSO for local UDP packets delivery to non
> GSO_SEGMENT sockets. That could be possibly a win or a regression: we
> save on netfilter/IP stack traversal, but we add additional work, some
> performances figures would probably help.
I did some tests for the local receive path with netperf and iperf, but
in this case the sender that generates the packets is the bottleneck.
So the benchmarks are not that meaningful for the receive path.
Do you have some performance tests for UDP GRO receive?
If so, I'd be glad if you could test this, or maybe better
the next patchset I'll send during the next days.
Thanks!
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: Annotate implicit fall through in l2cap_config_rsp
From: Mathieu Malaterre @ 2019-01-25 8:08 UTC (permalink / raw)
To: Marcel Holtmann, Gustavo A. R. Silva
Cc: Gustavo Padovan, Johan Hedberg, David S. Miller, linux-bluetooth,
netdev, LKML
In-Reply-To: <3DC0DB52-E3B0-42F3-BD4D-913FD77EDFBB@holtmann.org>
On Fri, Jan 25, 2019 at 8:56 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Mathieu,
>
> > There is a plan to build the kernel with -Wimplicit-fallthrough and
> > this place in the code produced a warning (W=1).
> >
> > This commit removes the following warning:
> >
> > net/bluetooth/l2cap_core.c:4223:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >
> > Cc: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > Signed-off-by: Mathieu Malaterre <malat@debian.org>
> > ---
> > net/bluetooth/l2cap_core.c | 1 +
> > 1 file changed, 1 insertion(+)
>
> the patches don’t apply cleanly against bluetooth-next tree. Please fix it up and resend.
Never mind, both warnings are already fixed by:
6317950c1b9c Bluetooth: Mark expected switch fall-throughs
Sorry for the noise
> Regards
>
> Marcel
>
^ permalink raw reply
* Re: [PATCH 2/2] Bluetooth: Correctly annotate implicit fall through in __rfcomm_dlc_close
From: Mathieu Malaterre @ 2019-01-25 8:09 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Peter Hurley, Johan Hedberg, David S. Miller, linux-bluetooth,
netdev, LKML
In-Reply-To: <20190124180724.20910-2-malat@debian.org>
On Thu, Jan 24, 2019 at 7:07 PM Mathieu Malaterre <malat@debian.org> wrote:
>
> There is a plan to build the kernel with -Wimplicit-fallthrough and
> this place in the code produced a warning (W=1).
>
> In this particular case put the fall through comment on a separate line so
> as to match the regular expression expected by GCC.
>
> This commit removes the following warning:
>
> net/bluetooth/rfcomm/core.c:479:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
Please discard, already fixed by :
6317950c1b9c Bluetooth: Mark expected switch fall-throughs
> Cc: Peter Hurley <peter@hurleysoftware.com>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
> net/bluetooth/rfcomm/core.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index 1a635df80643..caaae6accafa 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -480,9 +480,8 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
> __rfcomm_dlc_disconn(d);
> break;
> }
> - /* if closing a dlc in a session that hasn't been started,
> - * just close and unlink the dlc
> - */
> + /* fall through - if closing a dlc in a session that hasn't */
> + /* been started, just close and unlink the dlc */
>
> default:
> rfcomm_dlc_clear_timer(d);
> --
> 2.19.2
>
^ permalink raw reply
* Re: [PATCH stable 4.4 05/11] ip: use rb trees for IP frag queue.
From: maowenan @ 2019-01-25 8:12 UTC (permalink / raw)
To: Greg KH; +Cc: netdev, eric.dumazet, davem, stable, edumazet
In-Reply-To: <20190125070754.GA11152@kroah.com>
On 2019/1/25 15:07, Greg KH wrote:
> On Fri, Jan 25, 2019 at 09:50:35AM +0800, maowenan wrote:
>>
>>
>> On 2019/1/25 1:58, Greg KH wrote:
>>> On Wed, Jan 23, 2019 at 10:19:40AM +0800, Mao Wenan wrote:
>>>> From: Peter Oskolkov <posk@google.com>
>>>>
>>>> [ Upstream commit fa0f527358bd900ef92f925878ed6bfbd51305cc ]
>>>
>>> This commit is not in the 4.14.y tree, any specific reason why not?
>>
>> I found the commit 6b921536f1707a240e6f53843f1f26231016fda5 net: sk_buff rbnode reorg in v4.14.y
>> including the fixes.
>
> Yes, that commit is really bffa72cf7f9d ("net: sk_buff rbnode reorg"),
> which is upstream in 4.14.15 and 4.15. But fa0f527358bd ("ip: use rb
> trees for IP frag queue.") is not in 4.14 at all, it showed up in
> 4.9.134 and 4.19. Why did the 4.14 tree not need it and 4.9 and 4.4
> does?
The commit 6b921536f170(net: sk_buff rbnode reorg) in 4.14 combined two commits in
mainline(bffa72cf7f9d net: sk_buff rbnode reorg. and fa0f527358bd ip: use rb trees for IP frag queue.).
The main fix patch for CVE-2018-5391 is fa0f527358bd(ip: use rb trees for IP frag queue), I don't think
it is necessary to backport bffa72cf7f9d to 4.14, but the fa0f527358bd is really needed.
mainline patches
commit bffa72cf7f9df842f0016ba03586039296b4caaf
Author: Eric Dumazet <edumazet@google.com>
Date: Tue Sep 19 05:14:24 2017 -0700
net: sk_buff rbnode reorg
commit fa0f527358bd900ef92f925878ed6bfbd51305cc
Author: Peter Oskolkov <posk@google.com>
Date: Thu Aug 2 23:34:39 2018 +0000
ip: use rb trees for IP frag queue.
linux-4.14.y
commit 6b921536f1707a240e6f53843f1f26231016fda5
Author: Eric Dumazet <edumazet@google.com>
Date: Thu Sep 13 07:58:58 2018 -0700
net: sk_buff rbnode reorg
>
> thanks,
>
> greg k-h
>
>
^ permalink raw reply
* Re: [PATCH RFC 0/3] Support fraglist GRO/GSO
From: Steffen Klassert @ 2019-01-25 8:14 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Network Development, Willem de Bruijn, Paolo Abeni,
Jason A. Donenfeld
In-Reply-To: <CAF=yD-JtW5hEvCe0do3aDNGJ0Ke=HPwrdxPSi-mou6rrh3F3cw@mail.gmail.com>
On Mon, Jan 14, 2019 at 12:09:22PM -0500, Willem de Bruijn wrote:
> On Mon, Jan 14, 2019 at 7:50 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> > On Sun, Dec 23, 2018 at 08:15:40PM -0500, Willem de Bruijn wrote:
> > >
> > > I would prefer to split the patch that adds UDP GRO on the forwarding
> > > path into one that enables it for existing GRO (the hack you refer to
> > > above) and a second one to optionally convert to listified processing.
> >
> > The hack was to skip the socket lookup. While that was ok for a
> > forwarding test, it will affect the local receive path of course.
> >
> > Currently, I check if there is a GRO capable socket. If yes,
> > do standard GRO. If no, do listified GRO regardless if packets
> > are going to be forwarded or locally received. So UDP GRO is
> > always on with this.
>
> I understand. What I suggest is to split into two: enable GRO on the
> forwarding path independently from converting the method to listified
> GRO.
Ok, will do that in the next patchset.
> > We would need to do an early route lookup to check if the packets
> > are going to be forwarded or locally received. The current patchset
> > does not implement this, but could be done. Maybe doing a route
> > lookup based on some static key that will be enabled when forwarding
> > on the receiving device is enabled.
> >
> > But even if the route lookup tell us that the packet should go the
> > forwarding path, netfilter (bpfilter?) could reroute the packet.
> > If we do an early route lookup, it would be good to have some early
> > netfilter (bpfilter?) too, so that we can know which path the packets
> > go. In this case we could do listified GRO even for TCP, if we can
> > know that we have to do software segmentation later on.
> >
> > Btw. do we already have hardware that can do UDP LSO?
>
> Yes, mlx5
>
> I don't think that the route lookup is needed. If listified is cheaper
> for local delivery, too, then we can make that the default unless a
> device is active with h/w offload and ip forwarding is enabled. If it
> isn't, then use it iff ip forwarding is enabled. I think it's fine to
> mispredict between the two in edge cases with netfilter mangling, as
> long as all paths can correctly handle both types of GRO packets.
I'd need at least a route lookup for my usecase, because listified
GRO is always cheaper when a xfrm transformation is needed (even for
TCP). In this case is software GSO needed. So I'd need to either have
an early route lookup or maybe some early ingress hook where a route
lookup could be imlemented in.
> > >
> > > For both existing UDP GRO and listified, we should verify that this is
> > > not a potential DoS vector before enabling by default.
> >
> > Yes, but should'nt this be the same as with TCP GRO?
>
> That is by now well-tested. I don't think we can simply assume
> equivalence for UDP, also because that is easier to spoof.
What would be a good test for this? I played with masscan
and hping3, but did not notice any differences between
net-next and net-next + UDP GRO patches.
^ permalink raw reply
* Re: [PATCH bpf-next 2/6] samples: bpf: Convert XDP samples to libbpf usage
From: Jesper Dangaard Brouer @ 2019-01-25 8:22 UTC (permalink / raw)
To: Maciej Fijalkowski; +Cc: daniel, ast, netdev, jakub.kicinski, brouer
In-Reply-To: <20190117010115.18234-3-maciejromanfijalkowski@gmail.com>
On Thu, 17 Jan 2019 02:01:11 +0100
Maciej Fijalkowski <maciejromanfijalkowski@gmail.com> wrote:
> Some of XDP samples that are attaching the bpf program to the interface
> via libbpf's bpf_set_link_xdp_fd are still using the bpf_load.c for
> loading and manipulating the ebpf program and maps. Convert them to do
> this through libbpf usage and remove bpf_load from the picture.
>
> While at it remove what looks like debug leftover in
> xdp_redirect_map_user.c
>
> xdp_redirect_cpu is omitted because of read_trace_pipe() usage, which
> doesn't seem to be handled in libbpf ATM.
>
> Signed-off-by: Maciej Fijalkowski <maciejromanfijalkowski@gmail.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
> samples/bpf/Makefile | 8 ++--
> samples/bpf/xdp_redirect_map_user.c | 47 ++++++++++++++++-------
> samples/bpf/xdp_redirect_user.c | 44 +++++++++++++++++-----
> samples/bpf/xdp_router_ipv4_user.c | 75 ++++++++++++++++++++++++++-----------
> samples/bpf/xdp_tx_iptunnel_user.c | 37 ++++++++++++------
> 5 files changed, 151 insertions(+), 60 deletions(-)
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Thanks a lot for converting sample/bpf programs to use libbpf. It have
been on my todo list for a very long time.
--
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 bpf-next v3 3/7] samples/bpf: Convert XDP samples to libbpf usage
From: Jesper Dangaard Brouer @ 2019-01-25 8:30 UTC (permalink / raw)
To: Maciej Fijalkowski; +Cc: daniel, ast, netdev, jakub.kicinski, brouer
In-Reply-To: <20190124214747.6986-4-maciejromanfijalkowski@gmail.com>
On Thu, 24 Jan 2019 22:47:43 +0100
Maciej Fijalkowski <maciejromanfijalkowski@gmail.com> wrote:
> From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>
> Some of XDP samples that are attaching the bpf program to the interface
> via libbpf's bpf_set_link_xdp_fd are still using the bpf_load.c for
> loading and manipulating the ebpf program and maps. Convert them to do
> this through libbpf usage and remove bpf_load from the picture.
>
> While at it remove what looks like debug leftover in
> xdp_redirect_map_user.c
>
> In xdp_redirect_cpu, change the way that the program to be loaded onto
> interface is chosen - user now needs to pass the program's section name
> instead of the relative number. In case of typo print out the section
> names to choose from.
>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
> samples/bpf/Makefile | 8 +-
> samples/bpf/xdp_redirect_cpu_user.c | 145 +++++++++++++++++++++++++-----------
> samples/bpf/xdp_redirect_map_user.c | 47 ++++++++----
> samples/bpf/xdp_redirect_user.c | 44 ++++++++---
> samples/bpf/xdp_router_ipv4_user.c | 75 +++++++++++++------
> samples/bpf/xdp_tx_iptunnel_user.c | 37 ++++++---
> 6 files changed, 253 insertions(+), 103 deletions(-)
Thanks a lot for converting sample/bpf programs to use libbpf. It have
been on my todo list for a very long time.
You didn't implement the matching of prog-names in a iproute2 like
fashion, as I described, but the important part was converting this to
libbpf, so I'll Ack this anyway.
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
(I acked V1 just before... meant to ACK this V3)
--
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 bpf-next v3 4/7] samples/bpf: Extend RLIMIT_MEMLOCK for xdp_{sample_pkts, router_ipv4}
From: Jesper Dangaard Brouer @ 2019-01-25 8:32 UTC (permalink / raw)
To: Maciej Fijalkowski; +Cc: daniel, ast, netdev, jakub.kicinski, brouer
In-Reply-To: <20190124214747.6986-5-maciejromanfijalkowski@gmail.com>
On Thu, 24 Jan 2019 22:47:44 +0100
Maciej Fijalkowski <maciejromanfijalkowski@gmail.com> wrote:
> From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>
> There is a common problem with xdp samples that happens when user wants
> to run a particular sample and some bpf program is already loaded. The
> default 64kb RLIMIT_MEMLOCK resource limit will cause a following error
> (assuming that xdp sample that is failing was converted to libbpf
> usage):
>
> libbpf: Error in bpf_object__probe_name():Operation not permitted(1).
> Couldn't load basic 'r0 = 0' BPF program.
> libbpf: failed to load object './xdp_sample_pkts_kern.o'
>
> Fix it in xdp_sample_pkts and xdp_router_ipv4 by setting RLIMIT_MEMLOCK
> to RLIM_INFINITY.
>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
> samples/bpf/xdp_router_ipv4_user.c | 7 +++++++
> samples/bpf/xdp_sample_pkts_user.c | 7 +++++++
> 2 files changed, 14 insertions(+)
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
--
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 net] net: mvpp2: fix condition for setting up link interrupt
From: Antoine Tenart @ 2019-01-25 8:37 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Andrew Lunn, Thomas Bogendoerfer, antoine.tenart,
maxime.chevallier, David S. Miller, netdev, linux-kernel
In-Reply-To: <20190124160741.jady3r2e4dme7c4m@e5254000004ec.dyn.armlinux.org.uk>
Hi,
On Thu, Jan 24, 2019 at 04:07:41PM +0000, Russell King - ARM Linux admin wrote:
> On Thu, Jan 24, 2019 at 04:51:37PM +0100, Andrew Lunn wrote:
> > On Thu, Jan 24, 2019 at 02:18:03PM +0100, Thomas Bogendoerfer wrote:
> > >
> > > Fixes: 4bb043262878 ("net: mvpp2: phylink support")
> > > Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> > > ---
> > > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > index 16066c2d5b3a..0fbfe1945a69 100644
> > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > @@ -3405,7 +3405,7 @@ static int mvpp2_open(struct net_device *dev)
> > > valid = true;
> > > }
> > >
> > > - if (priv->hw_version == MVPP22 && port->link_irq && !port->phylink) {
> > > + if (priv->hw_version == MVPP22 && port->link_irq && port->phylink) {
> > > err = request_irq(port->link_irq, mvpp2_link_status_isr, 0,
> > > dev->name, port);
> > > if (err) {
>
> This still looks fishy to me. mvpp2_link_status_isr() has handling in
> it that is safe to be called for non-phylink cases, so presumably the
> right fix is to drop the "&& !port->phylink" completely?
That's right, mvpp2_link_status_isr() is safe to be called with or
without phylink being used. This IRQ is reporting the link status, and
is used when using in-band status or when phylink isn't used.
We do have a similar fix locally, which looks like:
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 2bcbf9caaf0d..6bab1824a1e4 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -3406,7 +3406,8 @@ static int mvpp2_open(struct net_device *dev)
valid = true;
}
- if (priv->hw_version == MVPP22 && port->link_irq && !port->phylink) {
+ if (priv->hw_version == MVPP22 && port->link_irq &&
+ (!port->phylink || !port->has_phy)) {
err = request_irq(port->link_irq, mvpp2_link_status_isr, 0,
dev->name, port);
if (err) {
We haven't submitted it yet, as we saw several issues when a port has no
PHY and is using the XLG MAC (so, not on the mcbin). We currently are
working on them.
I don't like to ask this, as you submitted the fix first, but I do think
we should hold back a bit while we figure out proper solutions. If you
need in-band status to be working properly, we could share out current
local branch so you can test and validate it do solve your issue.
Thanks!
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply related
* Re: [PATCH bpf-next v3 5/7] samples/bpf: Add a "force" flag to XDP samples
From: Jesper Dangaard Brouer @ 2019-01-25 8:38 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: daniel, ast, netdev, jakub.kicinski, brouer,
Toke Høiland-Jørgensen
In-Reply-To: <20190124214747.6986-6-maciejromanfijalkowski@gmail.com>
On Thu, 24 Jan 2019 22:47:45 +0100
Maciej Fijalkowski <maciejromanfijalkowski@gmail.com> wrote:
> diff --git a/samples/bpf/xdp_sample_pkts_user.c b/samples/bpf/xdp_sample_pkts_user.c
> index 5f5828ee0761..362ad35b524d 100644
> --- a/samples/bpf/xdp_sample_pkts_user.c
> +++ b/samples/bpf/xdp_sample_pkts_user.c
> @@ -13,6 +13,8 @@
> #include <libbpf.h>
> #include <bpf/bpf.h>
> #include <sys/resource.h>
> +#include <libgen.h>
> +#include <linux/if_link.h>
>
> #include "perf-sys.h"
> #include "trace_helpers.h"
> @@ -21,12 +23,13 @@
> static int pmu_fds[MAX_CPUS], if_idx;
> static struct perf_event_mmap_page *headers[MAX_CPUS];
> static char *if_name;
> +static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
>
> static int do_attach(int idx, int fd, const char *name)
> {
> int err;
>
> - err = bpf_set_link_xdp_fd(idx, fd, 0);
> + err = bpf_set_link_xdp_fd(idx, fd, xdp_flags);
> if (err < 0)
> printf("ERROR: failed to attach program to %s\n", name);
>
> @@ -98,21 +101,42 @@ static void sig_handler(int signo)
> exit(0);
> }
>
> +static void usage(const char *prog)
> +{
> + fprintf(stderr,
> + "%s: %s [OPTS] IFINDEX\n\n"
^^^^^^^
The usage help text is wrong, this sample takes the interface name as input.
> + "OPTS:\n"
> + " -F force loading prog\n",
> + __func__, prog);
> +}
> +
> int main(int argc, char **argv)
> {
> struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
> struct bpf_prog_load_attr prog_load_attr = {
> .prog_type = BPF_PROG_TYPE_XDP,
> };
> + const char *optstr = "F";
> + int prog_fd, map_fd, opt;
> struct bpf_object *obj;
> struct bpf_map *map;
> - int prog_fd, map_fd;
> char filename[256];
> int ret, err, i;
> int numcpus;
>
> - if (argc < 2) {
> - printf("Usage: %s <ifname>\n", argv[0]);
^^^^^^^^
Old help text says <ifname>
> + while ((opt = getopt(argc, argv, optstr)) != -1) {
> + switch (opt) {
> + case 'F':
> + xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
> + break;
> + default:
> + usage(basename(argv[0]));
> + return 1;
> + }
> + }
> +
> + if (optind == argc) {
> + usage(basename(argv[0]));
> return 1;
> }
>
> @@ -143,16 +167,16 @@ int main(int argc, char **argv)
> }
> map_fd = bpf_map__fd(map);
>
> - if_idx = if_nametoindex(argv[1]);
> + if_idx = if_nametoindex(argv[optind]);
The we convert the ifname to ifindex.
> if (!if_idx)
> - if_idx = strtoul(argv[1], NULL, 0);
> + if_idx = strtoul(argv[optind], NULL, 0);
>
We do support falling back to using this as an ifindex.
> if (!if_idx) {
> fprintf(stderr, "Invalid ifname\n");
> return 1;
> }
> - if_name = argv[1];
> - err = do_attach(if_idx, prog_fd, argv[1]);
> + if_name = argv[optind];
> + err = do_attach(if_idx, prog_fd, if_name);
> if (err)
> return err;
--
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 v4 bpf-next 1/9] bpf: introduce bpf_spin_lock
From: Peter Zijlstra @ 2019-01-25 8:38 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Eric Dumazet, Alexei Starovoitov, davem, daniel, jakub.kicinski,
netdev, kernel-team, mingo, will.deacon, Paul McKenney, jannh
In-Reply-To: <20190125025659.netyncl6vvtbv6oj@ast-mbp.dhcp.thefacebook.com>
On Thu, Jan 24, 2019 at 06:57:00PM -0800, Alexei Starovoitov wrote:
> On Thu, Jan 24, 2019 at 06:44:20PM -0800, Eric Dumazet wrote:
> > Let see if we understood this well.
> >
> > 1. create perf event PERF_TYPE_HARDWARE:PERF_COUNT_HW_CPU_CYCLES
> > 2. attach bpf probram to this event
> > 3. since that's a hw event, the bpf program is executed in NMI context
> > 4. the bpf program calls bpf_get_stackid to record the trace in a bpf map
> > 5. bpf_get_stackid calls pcpu_freelist_pop and pcpu_freelist_push from NMI
How is this not a straight up bug? NMI code should not ever call code
that uses spinlocks.
> > 6. userspace calls sys_bpf(bpf_map_lookup_elem) which calls bpf_stackmap_copy which can call pcpu_freelist_push
>
> argh. lookup cmd is missing __this_cpu_inc(bpf_prog_active); like update/delete do.
> Will fix.
>
> > It seems pcpu_freelist_pop and pcpu_freelist_push are not NMI safe,
> > so what prevents bad things to happen ?
>
> nmi checks for bpf_prog_active==0. See bpf_overflow_handler.
yuck yuck yuck.. That's horrific :-( That means the whole BPF crud is
unreliable and events can go randomly missing.
^ permalink raw reply
* RE: [PATCH 3/7] net/ethernet: Add parse_protocol header_ops support
From: Maxim Mikityanskiy @ 2019-01-25 8:51 UTC (permalink / raw)
To: Willem de Bruijn
Cc: David S. Miller, Saeed Mahameed, Willem de Bruijn, Jason Wang,
Eric Dumazet, netdev@vger.kernel.org, Eran Ben Elisha,
Tariq Toukan
In-Reply-To: <CAF=yD-+7+CY6Eh0Px2hTPJ5hL+hTpgY29kE7wmk2Fz9gQjJjBQ@mail.gmail.com>
> -----Original Message-----
> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Sent: 24 January, 2019 16:22
> To: Maxim Mikityanskiy <maximmi@mellanox.com>
> Cc: David S. Miller <davem@davemloft.net>; Saeed Mahameed
> <saeedm@mellanox.com>; Willem de Bruijn <willemb@google.com>; Jason Wang
> <jasowang@redhat.com>; Eric Dumazet <edumazet@google.com>;
> netdev@vger.kernel.org; Eran Ben Elisha <eranbe@mellanox.com>; Tariq Toukan
> <tariqt@mellanox.com>
> Subject: Re: [PATCH 3/7] net/ethernet: Add parse_protocol header_ops support
>
> On Thu, Jan 24, 2019 at 4:48 AM Maxim Mikityanskiy <maximmi@mellanox.com>
> wrote:
> >
> > > -----Original Message-----
> > > From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > > Sent: 23 January, 2019 16:15
> > > To: Maxim Mikityanskiy <maximmi@mellanox.com>
> > > Cc: David S. Miller <davem@davemloft.net>; Saeed Mahameed
> > > <saeedm@mellanox.com>; Willem de Bruijn <willemb@google.com>; Jason Wang
> > > <jasowang@redhat.com>; Eric Dumazet <edumazet@google.com>;
> > > netdev@vger.kernel.org; Eran Ben Elisha <eranbe@mellanox.com>; Tariq
> Toukan
> > > <tariqt@mellanox.com>
> > > Subject: Re: [PATCH 3/7] net/ethernet: Add parse_protocol header_ops
> support
> > >
> > > On Mon, Jan 14, 2019 at 8:21 AM Maxim Mikityanskiy
> <maximmi@mellanox.com>
> > > wrote:
> > > >
> > > > The previous commit introduced parse_protocol callback which should
> > > > extract the protocol number from the L2 header. Make all Ethernet
> > > > devices support it.
> > > >
> > > > Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
> > > > ---
> > > > include/linux/etherdevice.h | 1 +
> > > > net/ethernet/eth.c | 13 +++++++++++++
> > > > 2 files changed, 14 insertions(+)
> > > >
> > > > diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> > > > index 2c0af7b00715..e2f3b21cd72a 100644
> > > > --- a/include/linux/etherdevice.h
> > > > +++ b/include/linux/etherdevice.h
> > > > @@ -44,6 +44,7 @@ int eth_header_cache(const struct neighbour *neigh,
> > > struct hh_cache *hh,
> > > > __be16 type);
> > > > void eth_header_cache_update(struct hh_cache *hh, const struct
> net_device
> > > *dev,
> > > > const unsigned char *haddr);
> > > > +__be16 eth_header_parse_protocol(const struct sk_buff *skb);
> > >
> > > Does not need to be exposed in the header file or exported.
> >
> > Are you sure? All the other Ethernet header_ops callbacks are exported
> > and declared in the header. I'm not sure about the reason why it is done
> > in such a way, but my guess is that it will be useful if some driver
> > decides to replace one callback in header_ops but to use the default
> > ones for the rest of callbacks.
>
> I don't exactly follow this. But I think that many are exported
> because Ethernet is so common that of these are also called directly
> instead of through header_ops. Looking at other header_ops
> implementations, or other such callback structs, shows many examples
> where the members are static local functions.
Yes, they are called directly indeed, but not all of them. E.g.,
eth_header_parse is never called directly. On the other hand, look at
drivers/net/macvlan.c:
static const struct header_ops macvlan_hard_header_ops = {
.create = macvlan_hard_header,
.parse = eth_header_parse,
.cache = eth_header_cache,
.cache_update = eth_header_cache_update,
};
This is exactly what I am talking about. In order to support it,
eth_header_parse_protocol needs to be exported. BTW, we should consider
adding it to macvlan_hard_header_ops, ipvlan_header_ops and all other
such structures.
^ permalink raw reply
* [PATCH net-next] netlink: reduce NLA_POLICY_NESTED{,_ARRAY} arguments
From: Johannes Berg @ 2019-01-25 9:08 UTC (permalink / raw)
To: linux-wireless, netdev; +Cc: Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
In typical cases, there's no need to pass both the maxattr
and the policy array pointer, as the maxattr should just be
ARRAY_SIZE(policy) - 1. Therefore, to be less error prone,
just remove the maxattr argument from the default macros
and deduce the size accordingly.
Leave the original macros with a leading underscore to use
here and in case somebody needs to pass a policy pointer
where the policy isn't declared in the same place and thus
ARRAY_SIZE() cannot be used.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
Dave, it looks like we're the only users of this right now,
so let me know if you'd prefer I take it through my tree.
Also, this conflicts with the fix I just made to the place
where I got it wrong, but obviously the resolution is to
just take the version without the maxattr argument.
---
include/net/netlink.h | 8 ++++++--
net/wireless/nl80211.c | 15 +++++----------
2 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 4c1e99303b5a..23f27b0b3cef 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -306,10 +306,14 @@ struct nla_policy {
#define NLA_POLICY_ETH_ADDR NLA_POLICY_EXACT_LEN(ETH_ALEN)
#define NLA_POLICY_ETH_ADDR_COMPAT NLA_POLICY_EXACT_LEN_WARN(ETH_ALEN)
-#define NLA_POLICY_NESTED(maxattr, policy) \
+#define _NLA_POLICY_NESTED(maxattr, policy) \
{ .type = NLA_NESTED, .validation_data = policy, .len = maxattr }
-#define NLA_POLICY_NESTED_ARRAY(maxattr, policy) \
+#define _NLA_POLICY_NESTED_ARRAY(maxattr, policy) \
{ .type = NLA_NESTED_ARRAY, .validation_data = policy, .len = maxattr }
+#define NLA_POLICY_NESTED(policy) \
+ _NLA_POLICY_NESTED(ARRAY_SIZE(policy) - 1, policy)
+#define NLA_POLICY_NESTED_ARRAY(policy) \
+ _NLA_POLICY_NESTED_ARRAY(ARRAY_SIZE(policy) - 1, policy)
#define __NLA_ENSURE(condition) BUILD_BUG_ON_ZERO(!(condition))
#define NLA_ENSURE_INT_TYPE(tp) \
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index e5f9c9ceb6c9..dde03ec279e6 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -259,15 +259,13 @@ nl80211_pmsr_ftm_req_attr_policy[NL80211_PMSR_FTM_REQ_ATTR_MAX + 1] = {
static const struct nla_policy
nl80211_pmsr_req_data_policy[NL80211_PMSR_TYPE_MAX + 1] = {
[NL80211_PMSR_TYPE_FTM] =
- NLA_POLICY_NESTED(NL80211_PMSR_FTM_REQ_ATTR_MAX,
- nl80211_pmsr_ftm_req_attr_policy),
+ NLA_POLICY_NESTED(nl80211_pmsr_ftm_req_attr_policy),
};
static const struct nla_policy
nl80211_pmsr_req_attr_policy[NL80211_PMSR_REQ_ATTR_MAX + 1] = {
[NL80211_PMSR_REQ_ATTR_DATA] =
- NLA_POLICY_NESTED(NL80211_PMSR_TYPE_MAX,
- nl80211_pmsr_req_data_policy),
+ NLA_POLICY_NESTED(nl80211_pmsr_req_data_policy),
[NL80211_PMSR_REQ_ATTR_GET_AP_TSF] = { .type = NLA_FLAG },
};
@@ -280,8 +278,7 @@ nl80211_psmr_peer_attr_policy[NL80211_PMSR_PEER_ATTR_MAX + 1] = {
*/
[NL80211_PMSR_PEER_ATTR_CHAN] = { .type = NLA_NESTED },
[NL80211_PMSR_PEER_ATTR_REQ] =
- NLA_POLICY_NESTED(NL80211_PMSR_REQ_ATTR_MAX,
- nl80211_pmsr_req_attr_policy),
+ NLA_POLICY_NESTED(nl80211_pmsr_req_attr_policy),
[NL80211_PMSR_PEER_ATTR_RESP] = { .type = NLA_REJECT },
};
@@ -292,8 +289,7 @@ nl80211_pmsr_attr_policy[NL80211_PMSR_ATTR_MAX + 1] = {
[NL80211_PMSR_ATTR_RANDOMIZE_MAC_ADDR] = { .type = NLA_REJECT },
[NL80211_PMSR_ATTR_TYPE_CAPA] = { .type = NLA_REJECT },
[NL80211_PMSR_ATTR_PEERS] =
- NLA_POLICY_NESTED_ARRAY(NL80211_PMSR_PEER_ATTR_MAX,
- nl80211_psmr_peer_attr_policy),
+ NLA_POLICY_NESTED_ARRAY(nl80211_psmr_peer_attr_policy),
};
const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
@@ -555,8 +551,7 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
},
[NL80211_ATTR_TIMEOUT] = NLA_POLICY_MIN(NLA_U32, 1),
[NL80211_ATTR_PEER_MEASUREMENTS] =
- NLA_POLICY_NESTED(NL80211_PMSR_FTM_REQ_ATTR_MAX,
- nl80211_pmsr_attr_policy),
+ NLA_POLICY_NESTED(nl80211_pmsr_attr_policy),
[NL80211_ATTR_AIRTIME_WEIGHT] = NLA_POLICY_MIN(NLA_U16, 1),
};
--
2.17.2
^ permalink raw reply related
* Re: [PATCH v4 bpf-next 1/9] bpf: introduce bpf_spin_lock
From: Peter Zijlstra @ 2019-01-25 9:10 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, davem, daniel, jakub.kicinski, netdev,
kernel-team, mingo, will.deacon, Paul McKenney, jannh
In-Reply-To: <20190124235857.xyb5xx2ufr6x5mbt@ast-mbp.dhcp.thefacebook.com>
On Thu, Jan 24, 2019 at 03:58:59PM -0800, Alexei Starovoitov wrote:
> On Thu, Jan 24, 2019 at 07:01:09PM +0100, Peter Zijlstra wrote:
> >
> > Thanks for having kernel/locking people on Cc...
> >
> > On Wed, Jan 23, 2019 at 08:13:55PM -0800, Alexei Starovoitov wrote:
> >
> > > Implementation details:
> > > - on !SMP bpf_spin_lock() becomes nop
> >
> > Because no BPF program is preemptible? I don't see any assertions or
> > even a comment that says this code is non-preemptible.
> >
> > AFAICT some of the BPF_RUN_PROG things are under rcu_read_lock() only,
> > which is not sufficient.
>
> nope. all bpf prog types disable preemption. That is must have for all
> sorts of things to work properly.
> If there is a prog type that doing rcu_read_lock only it's a serious bug.
> About a year or so ago we audited everything specifically to make
> sure everything disables preemption before calling bpf progs.
> I'm pretty sure nothing crept in in the mean time.
Do we want something like (the completely untested) below to avoid
having to manually audit this over and over?
---
include/linux/filter.h | 2 +-
include/linux/kernel.h | 9 +++++++--
kernel/sched/core.c | 28 ++++++++++++++++++++++++++++
3 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index d531d4250bff..4ab51e78da36 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -513,7 +513,7 @@ struct sk_filter {
struct bpf_prog *prog;
};
-#define BPF_PROG_RUN(filter, ctx) (*(filter)->bpf_func)(ctx, (filter)->insnsi)
+#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
#define BPF_SKB_CB_LEN QDISC_CB_PRIV_LEN
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 8f0e68e250a7..f4cea3260a28 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -245,8 +245,10 @@ extern int _cond_resched(void);
#endif
#ifdef CONFIG_DEBUG_ATOMIC_SLEEP
- void ___might_sleep(const char *file, int line, int preempt_offset);
- void __might_sleep(const char *file, int line, int preempt_offset);
+extern void ___might_sleep(const char *file, int line, int preempt_offset);
+extern void __might_sleep(const char *file, int line, int preempt_offset);
+extern void __cant_sleep(const char *file, int line, int preempt_offset);
+
/**
* might_sleep - annotation for functions that can sleep
*
@@ -259,6 +261,8 @@ extern int _cond_resched(void);
*/
# define might_sleep() \
do { __might_sleep(__FILE__, __LINE__, 0); might_resched(); } while (0)
+# define cant_sleep() \
+ do { __cant_sleep(__FILE__, __LINE__, 0); } while (0)
# define sched_annotate_sleep() (current->task_state_change = 0)
#else
static inline void ___might_sleep(const char *file, int line,
@@ -266,6 +270,7 @@ extern int _cond_resched(void);
static inline void __might_sleep(const char *file, int line,
int preempt_offset) { }
# define might_sleep() do { might_resched(); } while (0)
+# define cant_sleep() do { } while (0)
# define sched_annotate_sleep() do { } while (0)
#endif
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index ee7763641348..799c285f4e0f 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6162,6 +6162,34 @@ void ___might_sleep(const char *file, int line, int preempt_offset)
add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
}
EXPORT_SYMBOL(___might_sleep);
+
+void __cant_sleep(const char *file, int line, int preempt_offset)
+{
+ static unsigned long prev_jiffy;
+
+ if (irqs_disabled())
+ return;
+
+ if (!IS_ENABLED(CONFIG_PREEMPT_COUNT))
+ return;
+
+ if (preempt_count() > preempt_offset)
+ return;
+
+ if (time_before(jiffies, prev_jiffy + HZ) && prev_jiffy)
+ return;
+ prev_jiffy = jiffies;
+
+ printk(KERN_ERR "BUG: assuming atomic context at %s:%d\n", file, line);
+ printk(KERN_ERR "in_atomic(): %d, irqs_disabled(): %d, pid: %d, name: %s\n",
+ in_atomic(), irqs_disabled(),
+ current->pid, current->comm);
+
+ debug_show_held_locks(current);
+ dump_stack();
+ add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
+}
+EXPORT_SYMBOL_GPL(__cant_sleep);
#endif
#ifdef CONFIG_MAGIC_SYSRQ
^ permalink raw reply related
* Re: [PATCH net-next 0/7] Devlink health updates
From: Eran Ben Elisha @ 2019-01-25 9:16 UTC (permalink / raw)
To: David Miller, jiri@resnulli.us
Cc: netdev@vger.kernel.org, Jiri Pirko, Saeed Mahameed, Moshe Shemesh
In-Reply-To: <20190124.220846.372838862694196119.davem@davemloft.net>
On 1/25/2019 8:08 AM, David Miller wrote:
> From: Jiri Pirko <jiri@resnulli.us>
> Date: Tue, 22 Jan 2019 17:58:21 +0100
>
>> Tue, Jan 22, 2019 at 04:57:17PM CET, eranbe@mellanox.com wrote:
>>> This patchset fixes some comments that were received for the devlink
>>> health series, mostly around the devlink health buffers API.
>>>
>>> It offers a new devlink<->driver API for passing health dump and diagnose info.
>>> As part of this patchset, the new API is developed and integrated into the
>>> devlink health and mlx5e TX reporter.
>>> Also, added some helpers together with the new API, which reduce the code
>>> required by the driver to fill dump and diagnose significantly.
>>>
>>> Eventually, it also deletes the old API.
>>>
>>> In addition, it includes some small fixes in the devlink and mlx5e TX reporter.
>>
>> Okay, just leaving, going to review this tomorrow. I would much rather
>> review the patchset from the beginning, not this incremental patchset.
>> It changes a lot of things, deprecating api what was just introduced.
>> Review nightmare :/
>>
>> Could we do revert, repost? For my health sakes :)
>
> Eran are you ok with the revert?
Dave, thanks for your consideration.
During the review of this fixes series with Jiri yesterday, we reached
to a conclusion that it would be cleaner to revert and re-post it again.
I thought I shall submit a revert patchset, but if just remove it, it
would be better, I guess.
Jiri,
I will probably be able to provide a new version with fixed comments
from here soon next week.
>
> I'll do it once I have Eran's confirmation.
Note that you will also have to revert ARM compilation fix which was
accepted on top.
https://patchwork.ozlabs.org/patch/1029047/
Thanks.
^ permalink raw reply
* Re: [PATCH 8/8] net: macb: add sam9x60-macb compatibility string
From: Nicolas.Ferre @ 2019-01-25 9:16 UTC (permalink / raw)
To: netdev, davem
Cc: alexandre.belloni, Ludovic.Desroches, linux-kernel,
linux-arm-kernel, sre, linux-pm, linux-usb, stern, gregkh,
robh+dt, devicetree
In-Reply-To: <dfc7db7606e0898fbedc1f45ed9637379595f9cb.1547629763.git.nicolas.ferre@microchip.com>
Hi David,
On 16/01/2019 at 10:57, Nicolas Ferre wrote:
> Add a new compatibility string for this product. It's using
> at91sam9260-macb layout but has a newer hardware revision: it's safer
> to use its own string.
>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
I'm thinking of pushing this patch with the others of the series through
arm-soc tree. Can I have your "Acked-by" tag then?
Best regards,
Nicolas
> ---
> drivers/net/ethernet/cadence/macb_main.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 66cc7927061a..a0889ef107a1 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -3941,6 +3941,7 @@ static const struct of_device_id macb_dt_ids[] = {
> { .compatible = "cdns,np4-macb", .data = &np4_config },
> { .compatible = "cdns,pc302-gem", .data = &pc302gem_config },
> { .compatible = "cdns,gem", .data = &pc302gem_config },
> + { .compatible = "cdns,sam9x60-macb", .data = &at91sam9260_config },
> { .compatible = "atmel,sama5d2-gem", .data = &sama5d2_config },
> { .compatible = "atmel,sama5d3-gem", .data = &sama5d3_config },
> { .compatible = "atmel,sama5d3-macb", .data = &sama5d3macb_config },
>
--
Nicolas Ferre
^ permalink raw reply
* Re: [PATCH net-next V4 5/5] vhost: access vq metadata through kernel virtual address
From: Jason Wang @ 2019-01-25 9:16 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtualization, netdev, linux-kernel, kvm
In-Reply-To: <20190124215912-mutt-send-email-mst@kernel.org>
On 2019/1/25 上午11:00, Michael S. Tsirkin wrote:
> On Thu, Jan 24, 2019 at 12:07:54PM +0800, Jason Wang wrote:
>>> Meanwhile, could you pls post data comparing this last patch with the
>>> below? This removes the speculation barrier replacing it with a
>>> (useless but at least more lightweight) data dependency.
>> SMAP off
>>
>> Your patch: 7.2MPPs
>>
>> vmap: 7.4Mpps
> OK so while we keep looking into vmap, why don't we merge something like
> the below? Seems quite straight forward ...
>
The problem is it gives ~8% regression on PPS when SMAP is on. This is
probably because the latency of lfence is hided by previous stac as
mentioned in the commit b3bbfb3fb5d25776b8e3f361d2eedaabb0b496cd.
Thanks
^ permalink raw reply
* Re: [PATCH net-next V4 5/5] vhost: access vq metadata through kernel virtual address
From: Jason Wang @ 2019-01-25 9:21 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtualization, netdev, linux-kernel, kvm, aarcange
In-Reply-To: <20190124214402-mutt-send-email-mst@kernel.org>
On 2019/1/25 上午11:03, Michael S. Tsirkin wrote:
> On Wed, Jan 23, 2019 at 05:55:57PM +0800, Jason Wang wrote:
>> It was noticed that the copy_user() friends that was used to access
>> virtqueue metdata tends to be very expensive for dataplane
>> implementation like vhost since it involves lots of software checks,
>> speculation barrier, hardware feature toggling (e.g SMAP). The
>> extra cost will be more obvious when transferring small packets since
>> the time spent on metadata accessing become more significant.
>>
>> This patch tries to eliminate those overheads by accessing them
>> through kernel virtual address by vmap(). To make the pages can be
>> migrated, instead of pinning them through GUP, we use MMU notifiers to
>> invalidate vmaps and re-establish vmaps during each round of metadata
>> prefetching if necessary. For devices that doesn't use metadata
>> prefetching, the memory accessors fallback to normal copy_user()
>> implementation gracefully. The invalidation was synchronized with
>> datapath through vq mutex, and in order to avoid hold vq mutex during
>> range checking, MMU notifier was teared down when trying to modify vq
>> metadata.
>>
>> Another thing is kernel lacks efficient solution for tracking dirty
>> pages by vmap(), this will lead issues if vhost is using file backed
>> memory which needs care of writeback. This patch solves this issue by
>> just skipping the vma that is file backed and fallback to normal
>> copy_user() friends. This might introduce some overheads for file
>> backed users but consider this use case is rare we could do
>> optimizations on top.
>>
>> Note that this was only done when device IOTLB is not enabled. We
>> could use similar method to optimize it in the future.
>>
>> Tests shows at most about 22% improvement on TX PPS when using
>> virtio-user + vhost_net + xdp1 + TAP on 2.6GHz Broadwell:
>>
>> SMAP on | SMAP off
>> Before: 5.0Mpps | 6.6Mpps
>> After: 6.1Mpps | 7.4Mpps
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> drivers/vhost/vhost.c | 288 +++++++++++++++++++++++++++++++++++++++++-
>> drivers/vhost/vhost.h | 13 ++
>> mm/shmem.c | 1 +
>> 3 files changed, 300 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>> index 37e2cac8e8b0..096ae3298d62 100644
>> --- a/drivers/vhost/vhost.c
>> +++ b/drivers/vhost/vhost.c
>> @@ -440,6 +440,9 @@ void vhost_dev_init(struct vhost_dev *dev,
>> vq->indirect = NULL;
>> vq->heads = NULL;
>> vq->dev = dev;
>> + memset(&vq->avail_ring, 0, sizeof(vq->avail_ring));
>> + memset(&vq->used_ring, 0, sizeof(vq->used_ring));
>> + memset(&vq->desc_ring, 0, sizeof(vq->desc_ring));
>> mutex_init(&vq->mutex);
>> vhost_vq_reset(dev, vq);
>> if (vq->handle_kick)
>> @@ -510,6 +513,73 @@ static size_t vhost_get_desc_size(struct vhost_virtqueue *vq, int num)
>> return sizeof(*vq->desc) * num;
>> }
>>
>> +static void vhost_uninit_vmap(struct vhost_vmap *map)
>> +{
>> + if (map->addr)
>> + vunmap(map->unmap_addr);
>> +
>> + map->addr = NULL;
>> + map->unmap_addr = NULL;
>> +}
>> +
>> +static int vhost_invalidate_vmap(struct vhost_virtqueue *vq,
>> + struct vhost_vmap *map,
>> + unsigned long ustart,
>> + size_t size,
>> + unsigned long start,
>> + unsigned long end,
>> + bool blockable)
>> +{
>> + if (end < ustart || start > ustart - 1 + size)
>> + return 0;
>> +
>> + if (!blockable)
>> + return -EAGAIN;
>> +
>> + mutex_lock(&vq->mutex);
>> + vhost_uninit_vmap(map);
>> + mutex_unlock(&vq->mutex);
>> +
>> + return 0;
>> +}
>> +
>> +static int vhost_invalidate_range_start(struct mmu_notifier *mn,
>> + const struct mmu_notifier_range *range)
>> +{
>> + struct vhost_dev *dev = container_of(mn, struct vhost_dev,
>> + mmu_notifier);
>> + int i;
>> +
>> + for (i = 0; i < dev->nvqs; i++) {
>> + struct vhost_virtqueue *vq = dev->vqs[i];
>> +
>> + if (vhost_invalidate_vmap(vq, &vq->avail_ring,
>> + (unsigned long)vq->avail,
>> + vhost_get_avail_size(vq, vq->num),
>> + range->start, range->end,
>> + range->blockable))
>> + return -EAGAIN;
>> + if (vhost_invalidate_vmap(vq, &vq->desc_ring,
>> + (unsigned long)vq->desc,
>> + vhost_get_desc_size(vq, vq->num),
>> + range->start, range->end,
>> + range->blockable))
>> + return -EAGAIN;
>> + if (vhost_invalidate_vmap(vq, &vq->used_ring,
>> + (unsigned long)vq->used,
>> + vhost_get_used_size(vq, vq->num),
>> + range->start, range->end,
>> + range->blockable))
>> + return -EAGAIN;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct mmu_notifier_ops vhost_mmu_notifier_ops = {
>> + .invalidate_range_start = vhost_invalidate_range_start,
>> +};
>> +
>> /* Caller should have device mutex */
>> long vhost_dev_set_owner(struct vhost_dev *dev)
>> {
> It seems questionable to merely track .invalidate_range_start.
> Don't we care about keeping pages young/accessed?
My understanding is the young stuffs were only needed for secondary MMU
where the hva is not used. This is not the case of vhost since anyway
guest will access those pages through userspace address.
> MMU will think they aren't and will penalize vhost by pushing
> them out.
>
> I note that MMU documentation says
> * invalidate_range_start() and invalidate_range_end() must be
> * paired
> and it seems questionable that they are not paired here.
I can see some users with the unpaired invalidate_range_start(). Maybe I
miss something but I can not find anything that we need to do after the
page is unmaped.
>
>
> I also wonder about things like write-protecting the pages.
> It does not look like a range is invalidated when page
> is write-protected, even though I might have missed that.
> If not we can be corrupting memory in a variety of ways
> e.g. when using KSM, or with COW.
Yes, we probably need to implement change_pte() method which will do
vunmap().
Thanks
^ permalink raw reply
* Re: [PATCH net-next V4 5/5] vhost: access vq metadata through kernel virtual address
From: Jason Wang @ 2019-01-25 9:24 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtualization, netdev, linux-kernel, kvm, aarcange
In-Reply-To: <20190124214402-mutt-send-email-mst@kernel.org>
On 2019/1/25 上午11:03, Michael S. Tsirkin wrote:
>> +/* Suppress the vma that needs writeback since we can not track dirty
>> + * pages now.
>> + */
>> +static bool vma_can_vmap(struct vm_area_struct *vma)
>> +{
>> + return vma_is_anonymous(vma) || is_vm_hugetlb_page(vma) ||
>> + vma_is_shmem(vma);
>> +}
>> +
> IIUC a second but anonymous memory needs writeback too, just to swap.
> I'm not an MM person so I might be off.
Right, my fault, I mean the vma that needs dirty page tracking.
Thanks
^ permalink raw reply
* Re: [PATCH net-next 0/7] Devlink health updates
From: Jiri Pirko @ 2019-01-25 9:19 UTC (permalink / raw)
To: Eran Ben Elisha
Cc: David Miller, netdev@vger.kernel.org, Jiri Pirko, Saeed Mahameed,
Moshe Shemesh
In-Reply-To: <8ba329b5-f550-2c3e-08d7-1094fbe4e6fb@mellanox.com>
Fri, Jan 25, 2019 at 10:16:16AM CET, eranbe@mellanox.com wrote:
>
>
>On 1/25/2019 8:08 AM, David Miller wrote:
>> From: Jiri Pirko <jiri@resnulli.us>
>> Date: Tue, 22 Jan 2019 17:58:21 +0100
>>
>>> Tue, Jan 22, 2019 at 04:57:17PM CET, eranbe@mellanox.com wrote:
>>>> This patchset fixes some comments that were received for the devlink
>>>> health series, mostly around the devlink health buffers API.
>>>>
>>>> It offers a new devlink<->driver API for passing health dump and diagnose info.
>>>> As part of this patchset, the new API is developed and integrated into the
>>>> devlink health and mlx5e TX reporter.
>>>> Also, added some helpers together with the new API, which reduce the code
>>>> required by the driver to fill dump and diagnose significantly.
>>>>
>>>> Eventually, it also deletes the old API.
>>>>
>>>> In addition, it includes some small fixes in the devlink and mlx5e TX reporter.
>>>
>>> Okay, just leaving, going to review this tomorrow. I would much rather
>>> review the patchset from the beginning, not this incremental patchset.
>>> It changes a lot of things, deprecating api what was just introduced.
>>> Review nightmare :/
>>>
>>> Could we do revert, repost? For my health sakes :)
>>
>> Eran are you ok with the revert?
>
>Dave, thanks for your consideration.
>
>During the review of this fixes series with Jiri yesterday, we reached
>to a conclusion that it would be cleaner to revert and re-post it again.
>I thought I shall submit a revert patchset, but if just remove it, it
>would be better, I guess.
>
>Jiri,
>I will probably be able to provide a new version with fixed comments
>from here soon next week.
Good. Thanks!
>
>>
>> I'll do it once I have Eran's confirmation.
>
>Note that you will also have to revert ARM compilation fix which was
>accepted on top.
>https://patchwork.ozlabs.org/patch/1029047/
>
>Thanks.
^ 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