* Re: [PATCH v2] bpf: Restrict cgroup bpf hooks to the init netns
From: Andy Lutomirski @ 2017-01-24 2:39 UTC (permalink / raw)
To: David Ahern
Cc: Alexei Starovoitov, Andy Lutomirski, Network Development,
David S. Miller, Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <678f4d6f-e2f8-eb37-207a-c77269507f9d@cumulusnetworks.com>
On Mon, Jan 23, 2017 at 6:31 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
> On 1/23/17 7:09 PM, Alexei Starovoitov wrote:
>>> + */
>>> + if (current->nsproxy->net_ns != &init_net)
>>> + return -EINVAL;
>>
>> this restriction I actually don't mind, since it indeed can be
>> relaxed later, but please make it proper with net_eq()
>>
>
> I do mind. Why have different restrictions for the skb and sk filters?
>
> I have already shown that ip can alleviate the management aspects of the implementation -- just like ip netns does.
I'm not sure I followed what you meant. If I understood right (which
is a big if) you're saying that ip vrf when run in a netns works
correctly in that netns. I agree, but I think that it would continue
to work (even more reliably) if the hooks only executed in the netns
in which they were created. I think that would probably be a good
improvement, and it could be done on top of my patch, but I'm not
about to write that code for 4.10.
--Andy
^ permalink raw reply
* Re: [PATCH v2] bpf: Restrict cgroup bpf hooks to the init netns
From: Andy Lutomirski @ 2017-01-24 2:42 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andy Lutomirski, Network Development, David S. Miller,
Daniel Borkmann, Alexei Starovoitov, David Ahern
In-Reply-To: <20170124020950.GA72992@ast-mbp.thefacebook.com>
On Mon, Jan 23, 2017 at 6:09 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Mon, Jan 23, 2017 at 12:36:08PM -0800, Andy Lutomirski wrote:
>> To see how cgroup+bpf interacts with network namespaces, I wrote a
>> little program called show_bind that calls getsockopt(...,
>> SO_BINDTODEVICE, ...) and prints the result. It did this:
>>
>> # ./ip link add dev vrf0 type vrf table 10
>> # ./ip vrf exec vrf0 ./show_bind
>> Default binding is "vrf0"
>> # ./ip vrf exec vrf0 unshare -n ./show_bind
>> show_bind: getsockopt: No such device
>>
>> What's happening here is that "ip vrf" looks up vrf0's ifindex in
>> the init netns and installs a hook that binds sockets to that
>> ifindex. When the hook runs in a different netns, it sets
>> sk_bound_dev_if to an ifindex from the wrong netns, resulting in
>> incorrect behavior. In this particular example, the ifindex was 4
>> and there was no ifindex 4 in the new netns. If there had been,
>> this test would have malfunctioned differently
>>
>> Since it's rather late in the release cycle, let's punt. This patch
>> makes it impossible to install cgroup+bpf hooks outside the init
>> netns and makes them not run on sockets that aren't in the init
>> netns.
>>
>> In a future release, it should be relatively straightforward to make
>> these hooks be local to a netns and, if needed, to add a flag so
>> that hooks can be made global if necessary. Global hooks should
>> presumably be constrained so that they can't write to any ifindex
>> fields.
>>
>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Alexei Starovoitov <ast@kernel.org>
>> Cc: David Ahern <dsa@cumulusnetworks.com>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>>
>> DaveM, this mitigates a bug in a feature that's new in 4.10, and the
>> bug can be hit using current iproute2 -git. please consider this for
>> -net.
>>
>> Changes from v1:
>> - Fix the commit message. 'git commit' was very clever and thought that
>> all the interesting bits of the test case were intended to be comments
>> and stripped them. Whoops!
>>
>> kernel/bpf/cgroup.c | 21 +++++++++++++++++++++
>> kernel/bpf/syscall.c | 11 +++++++++++
>> 2 files changed, 32 insertions(+)
>>
>> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
>> index a515f7b007c6..a824f543de69 100644
>> --- a/kernel/bpf/cgroup.c
>> +++ b/kernel/bpf/cgroup.c
>> @@ -143,6 +143,17 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
>> if (!sk || !sk_fullsock(sk))
>> return 0;
>>
>> + /*
>> + * For now, socket bpf hooks attached to cgroups can only be
>> + * installed in the init netns and only affect the init netns.
>> + * This could be relaxed in the future once some semantic issues
>> + * are resolved. For example, ifindexes belonging to one netns
>> + * should probably not be visible to hooks installed by programs
>> + * running in a different netns.
>> + */
>> + if (sock_net(sk) != &init_net)
>> + return 0;
>
> Nack.
> Such check will create absolutely broken abi that we won't be able to fix later.
Please explain how the change results in a broken ABI and how the
current ABI is better. I gave a fully worked out example of how the
current ABI misbehaves using only standard Linux networking tools.
>
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index e89acea22ecf..c0bbc55e244d 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -902,6 +902,17 @@ static int bpf_prog_attach(const union bpf_attr *attr)
>> struct cgroup *cgrp;
>> enum bpf_prog_type ptype;
>>
>> + /*
>> + * For now, socket bpf hooks attached to cgroups can only be
>> + * installed in the init netns and only affect the init netns.
>> + * This could be relaxed in the future once some semantic issues
>> + * are resolved. For example, ifindexes belonging to one netns
>> + * should probably not be visible to hooks installed by programs
>> + * running in a different netns.
>
> the comment is bogus and shows complete lack of understanding
> how bpf is working and how it's being used.
> See SKF_AD_IFINDEX that was in classic bpf forever.
>
I think I disagree completely.
With classic BPF, a program creates a socket and binds a bpf program
to it. That program can see the ifindex, which is fine because that
ifindex is scoped to the socket's netns, which is (unless the caller
uses setns() or unshare()) the same as the caller's netns, so the
ifindex means exactly what the caller thinks it means.
With cgroup+bpf, the program installing the hook can be completely
unrelated to the program whose sockets are subject to the hook, and,
if they're using different namespaces, it breaks.
--Andy
^ permalink raw reply
* Re: [PULL] vhost: cleanups and fixes
From: Michael S. Tsirkin @ 2017-01-24 2:45 UTC (permalink / raw)
To: Linus Torvalds
Cc: gcampana, KVM list, Network Development, pmorel,
Linux Kernel Mailing List, virtualization, Dan Carpenter,
Colin Ian King, bhumirks, silbe
In-Reply-To: <CA+55aFypi-Vg6CGtDX7tv_DfuN5ztrtSoQxexZiPKxDtmo9ZmQ@mail.gmail.com>
On Mon, Jan 23, 2017 at 01:50:32PM -0800, Linus Torvalds wrote:
> On Mon, Jan 23, 2017 at 7:05 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > virtio, vhost: fixes, cleanups
>
> Was there a reason why you sent this twice?
>
> Or was this *supposed* to be the ARM DMA fix pull request? Because it wasn't.
>
> Linus
Ouch sorry. Was tired and forgot I already sent it. Should have
checked.
--
MST
^ permalink raw reply
* Re: XDP offload to hypervisor
From: Michael S. Tsirkin @ 2017-01-24 2:47 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: John Fastabend, jasowang, john.r.fastabend, netdev, daniel
In-Reply-To: <20170124010201.GB60699@ast-mbp.thefacebook.com>
On Mon, Jan 23, 2017 at 05:02:02PM -0800, Alexei Starovoitov wrote:
> > Another issue is around host/guest ABI. Guest BPF could add new features
> > at any point. What if hypervisor can not support it all? I guess we
> > could try loading program into hypervisor and run it within guest on
> > failure to load, but this ignores question of cross-version
> > compatibility - someone might start guest on a new host
> > then try to move to an old one. So we will need an option
> > "behave like an older host" such that guest can start and then
> > move to an older host later. This will likely mean
> > implementing this validation of programs in qemu userspace unless linux
> > can supply something like this. Is this (disabling some features)
> > something that might be of interest to larger bpf community?
>
> In case of x86->nic offload not all xdp features will be supported
> by the nic and that is expected. The user will request 'offload of xdp prog'
> in some form and if it cannot be done, then xdp programs will run
> on x86 as before. Same thing, I imagine, is applicable to virtio->host
> offload. Therefore I don't see a need for user space visible
> feature negotiation.
Not userspace visible - guest visible. As guests move between hosts,
you need to make sure source host does not commit to a feature that
destination host does not support.
--
MST
^ permalink raw reply
* Re: [PATCH v2] bpf: Restrict cgroup bpf hooks to the init netns
From: Alexei Starovoitov @ 2017-01-24 3:13 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andy Lutomirski, Network Development, David S. Miller,
Daniel Borkmann, Alexei Starovoitov, David Ahern
In-Reply-To: <CALCETrUq_ZSso_252ixzjt+WbFOv_K57zbePUXneLv-Tgi5=jA@mail.gmail.com>
On Mon, Jan 23, 2017 at 06:42:27PM -0800, Andy Lutomirski wrote:
> On Mon, Jan 23, 2017 at 6:09 PM, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> > On Mon, Jan 23, 2017 at 12:36:08PM -0800, Andy Lutomirski wrote:
> >> To see how cgroup+bpf interacts with network namespaces, I wrote a
> >> little program called show_bind that calls getsockopt(...,
> >> SO_BINDTODEVICE, ...) and prints the result. It did this:
> >>
> >> # ./ip link add dev vrf0 type vrf table 10
> >> # ./ip vrf exec vrf0 ./show_bind
> >> Default binding is "vrf0"
> >> # ./ip vrf exec vrf0 unshare -n ./show_bind
> >> show_bind: getsockopt: No such device
> >>
> >> What's happening here is that "ip vrf" looks up vrf0's ifindex in
> >> the init netns and installs a hook that binds sockets to that
> >> ifindex. When the hook runs in a different netns, it sets
> >> sk_bound_dev_if to an ifindex from the wrong netns, resulting in
> >> incorrect behavior. In this particular example, the ifindex was 4
> >> and there was no ifindex 4 in the new netns. If there had been,
> >> this test would have malfunctioned differently
> >>
> >> Since it's rather late in the release cycle, let's punt. This patch
> >> makes it impossible to install cgroup+bpf hooks outside the init
> >> netns and makes them not run on sockets that aren't in the init
> >> netns.
> >>
> >> In a future release, it should be relatively straightforward to make
> >> these hooks be local to a netns and, if needed, to add a flag so
> >> that hooks can be made global if necessary. Global hooks should
> >> presumably be constrained so that they can't write to any ifindex
> >> fields.
> >>
> >> Cc: Daniel Borkmann <daniel@iogearbox.net>
> >> Cc: Alexei Starovoitov <ast@kernel.org>
> >> Cc: David Ahern <dsa@cumulusnetworks.com>
> >> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> >> ---
> >>
> >> DaveM, this mitigates a bug in a feature that's new in 4.10, and the
> >> bug can be hit using current iproute2 -git. please consider this for
> >> -net.
> >>
> >> Changes from v1:
> >> - Fix the commit message. 'git commit' was very clever and thought that
> >> all the interesting bits of the test case were intended to be comments
> >> and stripped them. Whoops!
> >>
> >> kernel/bpf/cgroup.c | 21 +++++++++++++++++++++
> >> kernel/bpf/syscall.c | 11 +++++++++++
> >> 2 files changed, 32 insertions(+)
> >>
> >> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> >> index a515f7b007c6..a824f543de69 100644
> >> --- a/kernel/bpf/cgroup.c
> >> +++ b/kernel/bpf/cgroup.c
> >> @@ -143,6 +143,17 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
> >> if (!sk || !sk_fullsock(sk))
> >> return 0;
> >>
> >> + /*
> >> + * For now, socket bpf hooks attached to cgroups can only be
> >> + * installed in the init netns and only affect the init netns.
> >> + * This could be relaxed in the future once some semantic issues
> >> + * are resolved. For example, ifindexes belonging to one netns
> >> + * should probably not be visible to hooks installed by programs
> >> + * running in a different netns.
> >> + */
> >> + if (sock_net(sk) != &init_net)
> >> + return 0;
> >
> > Nack.
> > Such check will create absolutely broken abi that we won't be able to fix later.
>
> Please explain how the change results in a broken ABI and how the
> current ABI is better. I gave a fully worked out example of how the
> current ABI misbehaves using only standard Linux networking tools.
I gave an example in the other thread that demonstrated
cgroup escape by the appliction when it changes netns.
If application became part of cgroup, it has to stay there,
no matter setns() calls afterwards.
> >
> >> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> >> index e89acea22ecf..c0bbc55e244d 100644
> >> --- a/kernel/bpf/syscall.c
> >> +++ b/kernel/bpf/syscall.c
> >> @@ -902,6 +902,17 @@ static int bpf_prog_attach(const union bpf_attr *attr)
> >> struct cgroup *cgrp;
> >> enum bpf_prog_type ptype;
> >>
> >> + /*
> >> + * For now, socket bpf hooks attached to cgroups can only be
> >> + * installed in the init netns and only affect the init netns.
> >> + * This could be relaxed in the future once some semantic issues
> >> + * are resolved. For example, ifindexes belonging to one netns
> >> + * should probably not be visible to hooks installed by programs
> >> + * running in a different netns.
> >
> > the comment is bogus and shows complete lack of understanding
> > how bpf is working and how it's being used.
> > See SKF_AD_IFINDEX that was in classic bpf forever.
> >
>
> I think I disagree completely.
>
> With classic BPF, a program creates a socket and binds a bpf program
> to it. That program can see the ifindex, which is fine because that
> ifindex is scoped to the socket's netns, which is (unless the caller
> uses setns() or unshare()) the same as the caller's netns, so the
> ifindex means exactly what the caller thinks it means.
>
> With cgroup+bpf, the program installing the hook can be completely
> unrelated to the program whose sockets are subject to the hook, and,
> if they're using different namespaces, it breaks.
Please also see ingress_ifindex, ifindex, bpf_redirect(), bpf_clone_redirect()
that all operate on the ifindex while the program can be detached from
the application. In general bpf program and application that loaded and
attached it are mostly disjoint in case of networking. We have tail_call
mechanism and master bpf prog may call programs installed by other apps
that may have exited already.
cls_bpf is scoped by netdev that belongs to some netns.
If after attaching a program with hardcoded if(ifindex==3) check
to such netdev, this netdev is moved into different netns, this 'if' check
and the program become bogus and won't do what program author
expected. It is expected behavior. The same thing with current 'ip vrf'
implementation that Dave is adjusting. It's not a bug in cgroup+bpf behavior.
^ permalink raw reply
* [PATCH net-next] net: dsa: Drop WARN() in tag_brcm.c
From: Florian Fainelli @ 2017-01-24 3:19 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller,
open list
We may be able to see invalid Broadcom tags when the hardware and drivers are
misconfigured, or just while exercising the error path. Instead of flooding
the console with messages, flat out drop the packet.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/tag_brcm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index af82927674e0..cb5a2b7a0118 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -121,7 +121,8 @@ static int brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev,
/* We should never see a reserved reason code without knowing how to
* handle it
*/
- WARN_ON(brcm_tag[2] & BRCM_EG_RC_RSVD);
+ if (unlikely(brcm_tag[2] & BRCM_EG_RC_RSVD))
+ goto out_drop;
/* Locate which port this is coming from */
source_port = brcm_tag[3] & BRCM_EG_PID_MASK;
--
2.9.3
^ permalink raw reply related
* Re: XDP offload to hypervisor
From: Michael S. Tsirkin @ 2017-01-24 3:33 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: John Fastabend, jasowang, john.r.fastabend, netdev, daniel
In-Reply-To: <20170124010201.GB60699@ast-mbp.thefacebook.com>
On Mon, Jan 23, 2017 at 05:02:02PM -0800, Alexei Starovoitov wrote:
> Frankly I don't understand the whole virtio nit picking that was happening.
> imo virtio+xdp by itself is only useful for debugging, development and testing
> of xdp programs in a VM. The discussion about performance of virtio+xdp
> will only be meaningful when corresponding host part is done.
> Likely in the form of vhost extensions and may be driver changes.
> Trying to optimize virtio+xdp when host is doing traditional skb+vhost
> isn't going to be impactful.
Well if packets can be dropped without a host/guest
transition then yes, that will have an impact even
with traditional skbs.
--
MST
^ permalink raw reply
* Re: [PATCH v2] bpf: Restrict cgroup bpf hooks to the init netns
From: Andy Lutomirski @ 2017-01-24 3:37 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andy Lutomirski, Network Development, David S. Miller,
Daniel Borkmann, Alexei Starovoitov, David Ahern
In-Reply-To: <20170124031259.GA81720@ast-mbp.thefacebook.com>
On Mon, Jan 23, 2017 at 7:13 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Mon, Jan 23, 2017 at 06:42:27PM -0800, Andy Lutomirski wrote:
>> Please explain how the change results in a broken ABI and how the
>> current ABI is better. I gave a fully worked out example of how the
>> current ABI misbehaves using only standard Linux networking tools.
>
> I gave an example in the other thread that demonstrated
> cgroup escape by the appliction when it changes netns.
> If application became part of cgroup, it has to stay there,
> no matter setns() calls afterwards.
The other thread is out of control. Can you restate your example?
Please keep in mind that uprivileged programs can unshare their netns.
>
>> >
>> >> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> >> index e89acea22ecf..c0bbc55e244d 100644
>> >> --- a/kernel/bpf/syscall.c
>> >> +++ b/kernel/bpf/syscall.c
>> >> @@ -902,6 +902,17 @@ static int bpf_prog_attach(const union bpf_attr *attr)
>> >> struct cgroup *cgrp;
>> >> enum bpf_prog_type ptype;
>> >>
>> >> + /*
>> >> + * For now, socket bpf hooks attached to cgroups can only be
>> >> + * installed in the init netns and only affect the init netns.
>> >> + * This could be relaxed in the future once some semantic issues
>> >> + * are resolved. For example, ifindexes belonging to one netns
>> >> + * should probably not be visible to hooks installed by programs
>> >> + * running in a different netns.
>> >
>> > the comment is bogus and shows complete lack of understanding
>> > how bpf is working and how it's being used.
>> > See SKF_AD_IFINDEX that was in classic bpf forever.
>> >
>>
>> I think I disagree completely.
>>
>> With classic BPF, a program creates a socket and binds a bpf program
>> to it. That program can see the ifindex, which is fine because that
>> ifindex is scoped to the socket's netns, which is (unless the caller
>> uses setns() or unshare()) the same as the caller's netns, so the
>> ifindex means exactly what the caller thinks it means.
>>
>> With cgroup+bpf, the program installing the hook can be completely
>> unrelated to the program whose sockets are subject to the hook, and,
>> if they're using different namespaces, it breaks.
>
> Please also see ingress_ifindex, ifindex, bpf_redirect(), bpf_clone_redirect()
> that all operate on the ifindex while the program can be detached from
> the application. In general bpf program and application that loaded and
> attached it are mostly disjoint in case of networking. We have tail_call
> mechanism and master bpf prog may call programs installed by other apps
> that may have exited already.
If program A acquires a BPF object from program B where program B runs
in a different netns from program A, and program B uses or tail-calls
that BPF object, then A is either doing it intentionally and is
netns-aware or it is terminally buggy and deserves what it gets.
> cls_bpf is scoped by netdev that belongs to some netns.
> If after attaching a program with hardcoded if(ifindex==3) check
> to such netdev, this netdev is moved into different netns, this 'if' check
> and the program become bogus and won't do what program author
> expected. It is expected behavior. The same thing with current 'ip vrf'
> implementation that Dave is adjusting. It's not a bug in cgroup+bpf behavior.
>
Yes, it is a bug because cgroup+bpf causes unwitting programs to be
subject to BPF code installed by a different, potentially unrelated
process. That's a new situation. The failure can happen when a
privileged supervisor (whoever runs ip vrf) runs a clueless or
unprivileged program (the thing calling unshare()).
If the way cgroup+bpf worked was that a program did a prctl() or
similar to opt in to being managed by a provided cgroup-aware BPF
program, then I'd agree with everything you're saying. But that's not
at all how this code works.
^ permalink raw reply
* Re: XDP offload to hypervisor
From: Alexei Starovoitov @ 2017-01-24 3:50 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: John Fastabend, jasowang, john.r.fastabend, netdev, daniel
In-Reply-To: <20170124053207-mutt-send-email-mst@kernel.org>
On Tue, Jan 24, 2017 at 05:33:37AM +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 23, 2017 at 05:02:02PM -0800, Alexei Starovoitov wrote:
> > Frankly I don't understand the whole virtio nit picking that was happening.
> > imo virtio+xdp by itself is only useful for debugging, development and testing
> > of xdp programs in a VM. The discussion about performance of virtio+xdp
> > will only be meaningful when corresponding host part is done.
> > Likely in the form of vhost extensions and may be driver changes.
> > Trying to optimize virtio+xdp when host is doing traditional skb+vhost
> > isn't going to be impactful.
>
> Well if packets can be dropped without a host/guest
> transition then yes, that will have an impact even
> with traditional skbs.
I don't think it's worth optimizing for though, since the speed of drop
matters for ddos-like use case and if we let host be flooded with skbs,
we already lost, since the only thing cpu is doing is allocating skbs
and moving them around. Whether drop is happening upon entry into VM
or host does it in some post-vhost layer doesn't change the picture much.
That said, I do like the idea of offloading virto+xdp into host somehow.
^ permalink raw reply
* [PATCH 1/3] Bluetooth: bnep: fix possible might sleep error in bnep_session
From: Jeffy Chen @ 2017-01-24 4:07 UTC (permalink / raw)
To: linux-bluetooth
Cc: Brian Norris, Douglas Anderson, Johan Hedberg, Peter Hurley,
Jeffy Chen, Johan Hedberg, netdev, linux-kernel, David S. Miller,
Marcel Holtmann, Gustavo Padovan
It looks like bnep_session has same pattern as the issue reported in
old rfcomm:
while (1) {
set_current_state(TASK_INTERRUPTIBLE);
if (condition)
break;
// may call might_sleep here
schedule();
}
__set_current_state(TASK_RUNNING);
Which fixed at:
dfb2fae Bluetooth: Fix nested sleeps
So let's fix it at the same way, also follow the suggestion of:
https://lwn.net/Articles/628628/
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---
net/bluetooth/bnep/core.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
index fbf251f..da04d51 100644
--- a/net/bluetooth/bnep/core.c
+++ b/net/bluetooth/bnep/core.c
@@ -484,16 +484,16 @@ static int bnep_session(void *arg)
struct net_device *dev = s->dev;
struct sock *sk = s->sock->sk;
struct sk_buff *skb;
- wait_queue_t wait;
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
BT_DBG("");
set_user_nice(current, -15);
- init_waitqueue_entry(&wait, current);
add_wait_queue(sk_sleep(sk), &wait);
while (1) {
- set_current_state(TASK_INTERRUPTIBLE);
+ /* Ensure session->terminate is updated */
+ smp_mb__before_atomic();
if (atomic_read(&s->terminate))
break;
@@ -515,9 +515,8 @@ static int bnep_session(void *arg)
break;
netif_wake_queue(dev);
- schedule();
+ wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
}
- __set_current_state(TASK_RUNNING);
remove_wait_queue(sk_sleep(sk), &wait);
/* Cleanup session */
@@ -666,7 +665,11 @@ int bnep_del_connection(struct bnep_conndel_req *req)
s = __bnep_get_session(req->dst);
if (s) {
atomic_inc(&s->terminate);
- wake_up_process(s->task);
+
+ /* Ensure session->terminate is updated */
+ smp_mb__after_atomic();
+
+ wake_up_interruptible(sk_sleep(s->sock->sk));
} else
err = -ENOENT;
--
2.1.4
^ permalink raw reply related
* [PATCH 2/3] Bluetooth: cmtp: fix possible might sleep error in cmtp_session
From: Jeffy Chen @ 2017-01-24 4:07 UTC (permalink / raw)
To: linux-bluetooth
Cc: Brian Norris, Douglas Anderson, Johan Hedberg, Peter Hurley,
Jeffy Chen, Johan Hedberg, netdev, linux-kernel, David S. Miller,
Marcel Holtmann, Gustavo Padovan
In-Reply-To: <1485230871-22828-1-git-send-email-jeffy.chen@rock-chips.com>
It looks like cmtp_session has same pattern as the issue reported in
old rfcomm:
while (1) {
set_current_state(TASK_INTERRUPTIBLE);
if (condition)
break;
// may call might_sleep here
schedule();
}
__set_current_state(TASK_RUNNING);
Which fixed at:
dfb2fae Bluetooth: Fix nested sleeps
So let's fix it at the same way, also follow the suggestion of:
https://lwn.net/Articles/628628/
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---
net/bluetooth/cmtp/core.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/net/bluetooth/cmtp/core.c b/net/bluetooth/cmtp/core.c
index 9e59b66..6b03f2b 100644
--- a/net/bluetooth/cmtp/core.c
+++ b/net/bluetooth/cmtp/core.c
@@ -280,16 +280,16 @@ static int cmtp_session(void *arg)
struct cmtp_session *session = arg;
struct sock *sk = session->sock->sk;
struct sk_buff *skb;
- wait_queue_t wait;
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
BT_DBG("session %p", session);
set_user_nice(current, -15);
- init_waitqueue_entry(&wait, current);
add_wait_queue(sk_sleep(sk), &wait);
while (1) {
- set_current_state(TASK_INTERRUPTIBLE);
+ /* Ensure session->terminate is updated */
+ smp_mb__before_atomic();
if (atomic_read(&session->terminate))
break;
@@ -306,9 +306,8 @@ static int cmtp_session(void *arg)
cmtp_process_transmit(session);
- schedule();
+ wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
}
- __set_current_state(TASK_RUNNING);
remove_wait_queue(sk_sleep(sk), &wait);
down_write(&cmtp_session_sem);
@@ -393,7 +392,11 @@ int cmtp_add_connection(struct cmtp_connadd_req *req, struct socket *sock)
err = cmtp_attach_device(session);
if (err < 0) {
atomic_inc(&session->terminate);
- wake_up_process(session->task);
+
+ /* Ensure session->terminate is updated */
+ smp_mb__after_atomic();
+
+ wake_up_interruptible(sk_sleep(session->sock->sk));
up_write(&cmtp_session_sem);
return err;
}
@@ -431,7 +434,11 @@ int cmtp_del_connection(struct cmtp_conndel_req *req)
/* Stop session thread */
atomic_inc(&session->terminate);
- wake_up_process(session->task);
+
+ /* Ensure session->terminate is updated */
+ smp_mb__after_atomic();
+
+ wake_up_interruptible(sk_sleep(session->sock->sk));
} else
err = -ENOENT;
--
2.1.4
^ permalink raw reply related
* [PATCH 3/3] Bluetooth: hidp: fix possible might sleep error in hidp_session_thread
From: Jeffy Chen @ 2017-01-24 4:07 UTC (permalink / raw)
To: linux-bluetooth
Cc: Brian Norris, Douglas Anderson, Johan Hedberg, Peter Hurley,
Jeffy Chen, Johan Hedberg, netdev, linux-kernel, David S. Miller,
Marcel Holtmann, Gustavo Padovan
In-Reply-To: <1485230871-22828-1-git-send-email-jeffy.chen@rock-chips.com>
It looks like hidp_session_thread has same pattern as the issue reported in
old rfcomm:
while (1) {
set_current_state(TASK_INTERRUPTIBLE);
if (condition)
break;
// may call might_sleep here
schedule();
}
__set_current_state(TASK_RUNNING);
Which fixed at:
dfb2fae Bluetooth: Fix nested sleeps
So let's fix it at the same way, also follow the suggestion of:
https://lwn.net/Articles/628628/
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---
net/bluetooth/hidp/core.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 0bec458..43d6e6a 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -36,6 +36,7 @@
#define VERSION "1.2"
static DECLARE_RWSEM(hidp_session_sem);
+static DECLARE_WAIT_QUEUE_HEAD(hidp_session_wq);
static LIST_HEAD(hidp_session_list);
static unsigned char hidp_keycode[256] = {
@@ -1068,12 +1069,15 @@ static int hidp_session_start_sync(struct hidp_session *session)
* Wake up session thread and notify it to stop. This is asynchronous and
* returns immediately. Call this whenever a runtime error occurs and you want
* the session to stop.
- * Note: wake_up_process() performs any necessary memory-barriers for us.
*/
static void hidp_session_terminate(struct hidp_session *session)
{
atomic_inc(&session->terminate);
- wake_up_process(session->task);
+
+ /* Ensure session->terminate is updated */
+ smp_mb__after_atomic();
+
+ wake_up_interruptible(&hidp_session_wq);
}
/*
@@ -1180,7 +1184,9 @@ static void hidp_session_run(struct hidp_session *session)
struct sock *ctrl_sk = session->ctrl_sock->sk;
struct sock *intr_sk = session->intr_sock->sk;
struct sk_buff *skb;
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
+ add_wait_queue(&hidp_session_wq, &wait);
for (;;) {
/*
* This thread can be woken up two ways:
@@ -1188,12 +1194,10 @@ static void hidp_session_run(struct hidp_session *session)
* session->terminate flag and wakes this thread up.
* - Via modifying the socket state of ctrl/intr_sock. This
* thread is woken up by ->sk_state_changed().
- *
- * Note: set_current_state() performs any necessary
- * memory-barriers for us.
*/
- set_current_state(TASK_INTERRUPTIBLE);
+ /* Ensure session->terminate is updated */
+ smp_mb__before_atomic();
if (atomic_read(&session->terminate))
break;
@@ -1227,11 +1231,14 @@ static void hidp_session_run(struct hidp_session *session)
hidp_process_transmit(session, &session->ctrl_transmit,
session->ctrl_sock);
- schedule();
+ wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
}
+ remove_wait_queue(&hidp_session_wq, &wait);
atomic_inc(&session->terminate);
- set_current_state(TASK_RUNNING);
+
+ /* Ensure session->terminate is updated */
+ smp_mb__after_atomic();
}
/*
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v2] bpf: Restrict cgroup bpf hooks to the init netns
From: David Ahern @ 2017-01-24 4:10 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Alexei Starovoitov, Andy Lutomirski, Network Development,
David S. Miller, Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <CALCETrUz1701E5JS5NEqEqeGLExnxpesU44dSi2eYK4iMu=kcQ@mail.gmail.com>
On 1/23/17 7:39 PM, Andy Lutomirski wrote:
> I'm not sure I followed what you meant. If I understood right (which
> is a big if) you're saying that ip vrf when run in a netns works
> correctly in that netns. I agree, but I think that it would continue
> to work (even more reliably) if the hooks only executed in the netns
> in which they were created. I think that would probably be a good
> improvement, and it could be done on top of my patch, but I'm not
> about to write that code for 4.10.
Sounds like an efficiency on the implementation that does not require limiting the code today to just init namespace.
^ permalink raw reply
* Re: [PATCH v2] bpf: Restrict cgroup bpf hooks to the init netns
From: David Ahern @ 2017-01-24 4:05 UTC (permalink / raw)
To: Andy Lutomirski, Alexei Starovoitov
Cc: Andy Lutomirski, Network Development, David S. Miller,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <CALCETrVZqqfMO+=HGgebbAJ0TY6c3XSyY1G-Ve+g3VQiEfGwyA@mail.gmail.com>
On 1/23/17 8:37 PM, Andy Lutomirski wrote:
> Yes, it is a bug because cgroup+bpf causes unwitting programs to be
> subject to BPF code installed by a different, potentially unrelated
> process. That's a new situation. The failure can happen when a
> privileged supervisor (whoever runs ip vrf) runs a clueless or
> unprivileged program (the thing calling unshare()).
There are many, many ways to misconfigure networking and to run programs in a context or with an input argument that causes the program to not work at all, not work as expected or stop working. This situation is no different.
For example, the only aspect of BPF_PROG_TYPE_CGROUP_SOCK filters that are namespace based is the ifindex. You brought up the example of changing namespaces where the ifindex is not defined. Alexei mentioned an example where interfaces can be moved to another namespace breaking any ifindex based programs. Another example is the interface can be deleted. Deleting an interface with sockets bound to it does not impact the program in any way - no notification, no wakeup, nothing. The sockets just don't work.
The point of using a management tool like ip is to handle the details to make things sane -- which is the consistent with the whole point of ip netns vs running unshare -n.
>
> If the way cgroup+bpf worked was that a program did a prctl() or
> similar to opt in to being managed by a provided cgroup-aware BPF
> program, then I'd agree with everything you're saying. But that's not
> at all how this code works.
I don't want an opt-in approach. The program does not need to know or even care that it has some restriction. Today, someone runs 'ip netns exec NAME COMMAND' the command does not need to know or care that the network namespace was changed on it. Same with 'ip vrf exec' -- it is a network policy that is forced on the program by the user.
^ permalink raw reply
* Re: [PATCH v2] bpf: Restrict cgroup bpf hooks to the init netns
From: Andy Lutomirski @ 2017-01-24 4:16 UTC (permalink / raw)
To: David Ahern
Cc: Alexei Starovoitov, Andy Lutomirski, Network Development,
David S. Miller, Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <f494c25b-1ef0-1fe4-9ac5-05eafa903be0@cumulusnetworks.com>
On Mon, Jan 23, 2017 at 8:10 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
> On 1/23/17 7:39 PM, Andy Lutomirski wrote:
>> I'm not sure I followed what you meant. If I understood right (which
>> is a big if) you're saying that ip vrf when run in a netns works
>> correctly in that netns. I agree, but I think that it would continue
>> to work (even more reliably) if the hooks only executed in the netns
>> in which they were created. I think that would probably be a good
>> improvement, and it could be done on top of my patch, but I'm not
>> about to write that code for 4.10.
>
> Sounds like an efficiency on the implementation that does not require limiting the code today to just init namespace.
>
It's an ABI change, not an implementation change. If someone wants to
make the code fully netns-aware in time for 4.10, that sounds great.
I'm not going to do that.
^ permalink raw reply
* Re: [PATCH v2] bpf: Restrict cgroup bpf hooks to the init netns
From: Andy Lutomirski @ 2017-01-24 4:32 UTC (permalink / raw)
To: David Ahern
Cc: Alexei Starovoitov, Andy Lutomirski, Network Development,
David S. Miller, Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <08f3f57e-8c51-c893-01ad-3ce23db8166d@cumulusnetworks.com>
On Mon, Jan 23, 2017 at 8:05 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
> On 1/23/17 8:37 PM, Andy Lutomirski wrote:
>> Yes, it is a bug because cgroup+bpf causes unwitting programs to be
>> subject to BPF code installed by a different, potentially unrelated
>> process. That's a new situation. The failure can happen when a
>> privileged supervisor (whoever runs ip vrf) runs a clueless or
>> unprivileged program (the thing calling unshare()).
>
> There are many, many ways to misconfigure networking and to run programs in a context or with an input argument that causes the program to not work at all, not work as expected or stop working. This situation is no different.
>
> For example, the only aspect of BPF_PROG_TYPE_CGROUP_SOCK filters that are namespace based is the ifindex. You brought up the example of changing namespaces where the ifindex is not defined. Alexei mentioned an example where interfaces can be moved to another namespace breaking any ifindex based programs. Another example is the interface can be deleted. Deleting an interface with sockets bound to it does not impact the program in any way - no notification, no wakeup, nothing. The sockets just don't work.
And if you use 'ip vrf' to bind to a vrf with ifindex 4 and a program
unshares netns and creates an interface with ifindex 4, then that
program will end up with its sockets magically bound to ifindex 4 and
will silently malfunction.
I can think of multiple ways to address this problem. You could scope
the hooks to a netns (which is sort of what my patch does). You could
find a way to force programs in a given cgroup to only execute in a
single netns, although that would probably cause other breakage. You
could improve the BPF hook API to be netns-aware, which could plausbly
address issues related to unshare() but might get very tricky when
setns() is involved.
My point is that, in 4.10-rc, it doesn't work right, and I doubt this
problem is restricted to just 'ip vrf'. Without some kind of change
to the way that netns and cgroup+bpf interact, anything that uses
sk_bound_dev_if or reads the ifindex on an skb will be subject to a
huge footgun that unprivileged programs can trigger and any future
attempt to make the cgroup+bpf work for unprivileged users is going to
be more complicated than it deserves to be.
(Aside: I wonder if a similar goal to 'ip vrf' could be achieved by
moving the vrf to a different network namespace and putting programs
that you want to be subject to the VRF into that namespace.)
>
> The point of using a management tool like ip is to handle the details to make things sane -- which is the consistent with the whole point of ip netns vs running unshare -n.
I still don't understand how you're going to make 'ip netns' make this
problem go away. Keep in mind that there are real programs that call
the unshare() syscall directly.
>
>>
>> If the way cgroup+bpf worked was that a program did a prctl() or
>> similar to opt in to being managed by a provided cgroup-aware BPF
>> program, then I'd agree with everything you're saying. But that's not
>> at all how this code works.
>
> I don't want an opt-in approach. The program does not need to know or even care that it has some restriction. Today, someone runs 'ip netns exec NAME COMMAND' the command does not need to know or care that the network namespace was changed on it. Same with 'ip vrf exec' -- it is a network policy that is forced on the program by the user.
I absolutely agree. My point is that cgroup+bpf is *not* opt-in, so
the bar should be higher. You can't blame the evil program that
called unshare() for breaking things when 'ip vrf' unavoidably sets
things up so that unshare will malfunction. It should avoid
malfunctioning when running Linux programs that are unaware of it.
This should include programs like unshare and 'ip netns'.
^ permalink raw reply
* Re: XDP offload to hypervisor
From: Michael S. Tsirkin @ 2017-01-24 4:35 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: John Fastabend, jasowang, john.r.fastabend, netdev, daniel
In-Reply-To: <20170124035029.GA89215@ast-mbp.thefacebook.com>
On Mon, Jan 23, 2017 at 07:50:31PM -0800, Alexei Starovoitov wrote:
> On Tue, Jan 24, 2017 at 05:33:37AM +0200, Michael S. Tsirkin wrote:
> > On Mon, Jan 23, 2017 at 05:02:02PM -0800, Alexei Starovoitov wrote:
> > > Frankly I don't understand the whole virtio nit picking that was happening.
> > > imo virtio+xdp by itself is only useful for debugging, development and testing
> > > of xdp programs in a VM. The discussion about performance of virtio+xdp
> > > will only be meaningful when corresponding host part is done.
> > > Likely in the form of vhost extensions and may be driver changes.
> > > Trying to optimize virtio+xdp when host is doing traditional skb+vhost
> > > isn't going to be impactful.
> >
> > Well if packets can be dropped without a host/guest
> > transition then yes, that will have an impact even
> > with traditional skbs.
>
> I don't think it's worth optimizing for though, since the speed of drop
> matters for ddos-like use case
It's not just drops. adjust head + xmit can handle bridging
without entering the VM.
> and if we let host be flooded with skbs,
> we already lost, since the only thing cpu is doing is allocating skbs
> and moving them around. Whether drop is happening upon entry into VM
> or host does it in some post-vhost layer doesn't change the picture much.
> That said, I do like the idea of offloading virto+xdp into host somehow.
^ permalink raw reply
* [PATCH net-next v2 0/2] vxlan: misc fdb fixes
From: Roopa Prabhu @ 2017-01-24 4:44 UTC (permalink / raw)
To: davem; +Cc: netdev, ramanb, stephen, jbenc, pshelar
From: Roopa Prabhu <roopa@cumulusnetworks.com>
Balakrishnan Raman (1):
vxlan: do not age static remote mac entries
Roopa Prabhu (1):
vxlan: don't flush static fdb entries on admin down
drivers/net/vxlan.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
--
v1 -> v2: use bool as suggested by DavidM
^ permalink raw reply
* [PATCH net-next v2 1/2] vxlan: don't flush static fdb entries on admin down
From: Roopa Prabhu @ 2017-01-24 4:44 UTC (permalink / raw)
To: davem; +Cc: netdev, ramanb, stephen, jbenc, pshelar
In-Reply-To: <1485233073-11391-1-git-send-email-roopa@cumulusnetworks.com>
From: Roopa Prabhu <roopa@cumulusnetworks.com>
This patch skips flushing static fdb entries in
ndo_stop, but flushes all fdb entries during vxlan
device delete. This is consistent with the bridge
driver fdb
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
drivers/net/vxlan.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 19b1653..b34822f 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2354,7 +2354,7 @@ static int vxlan_open(struct net_device *dev)
}
/* Purge the forwarding table */
-static void vxlan_flush(struct vxlan_dev *vxlan)
+static void vxlan_flush(struct vxlan_dev *vxlan, bool do_all)
{
unsigned int h;
@@ -2364,6 +2364,8 @@ static void vxlan_flush(struct vxlan_dev *vxlan)
hlist_for_each_safe(p, n, &vxlan->fdb_head[h]) {
struct vxlan_fdb *f
= container_of(p, struct vxlan_fdb, hlist);
+ if (!do_all && (f->state & (NUD_PERMANENT | NUD_NOARP)))
+ continue;
/* the all_zeros_mac entry is deleted at vxlan_uninit */
if (!is_zero_ether_addr(f->eth_addr))
vxlan_fdb_destroy(vxlan, f);
@@ -2385,7 +2387,7 @@ static int vxlan_stop(struct net_device *dev)
del_timer_sync(&vxlan->age_timer);
- vxlan_flush(vxlan);
+ vxlan_flush(vxlan, false);
vxlan_sock_release(vxlan);
return ret;
@@ -3058,6 +3060,8 @@ static void vxlan_dellink(struct net_device *dev, struct list_head *head)
struct vxlan_dev *vxlan = netdev_priv(dev);
struct vxlan_net *vn = net_generic(vxlan->net, vxlan_net_id);
+ vxlan_flush(vxlan, true);
+
spin_lock(&vn->sock_lock);
if (!hlist_unhashed(&vxlan->hlist))
hlist_del_rcu(&vxlan->hlist);
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 2/2] vxlan: do not age static remote mac entries
From: Roopa Prabhu @ 2017-01-24 4:44 UTC (permalink / raw)
To: davem; +Cc: netdev, ramanb, stephen, jbenc, pshelar
In-Reply-To: <1485233073-11391-1-git-send-email-roopa@cumulusnetworks.com>
From: Balakrishnan Raman <ramanb@cumulusnetworks.com>
Mac aging is applicable only for dynamically learnt remote mac
entries. Check for user configured static remote mac entries
and skip aging.
Signed-off-by: Balakrishnan Raman <ramanb@cumulusnetworks.com>
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
drivers/net/vxlan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index b34822f..09aa52e 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2268,7 +2268,7 @@ static void vxlan_cleanup(unsigned long arg)
= container_of(p, struct vxlan_fdb, hlist);
unsigned long timeout;
- if (f->state & NUD_PERMANENT)
+ if (f->state & (NUD_PERMANENT | NUD_NOARP))
continue;
timeout = f->used + vxlan->cfg.age_interval * HZ;
--
1.9.1
^ permalink raw reply related
* linux-next: manual merge of the kselftest tree with the net-next tree
From: Stephen Rothwell @ 2017-01-24 4:45 UTC (permalink / raw)
To: Shuah Khan, David Miller, Networking
Cc: linux-next, linux-kernel, David Herrmann, Daniel Mack,
Bamvor Jian Zhang, bamvor.zhangjian@huawei.com
Hi Shuah,
Today's linux-next merge of the kselftest tree got a conflict in:
tools/testing/selftests/bpf/Makefile
between commit:
4d3381f5a322 ("bpf: Add tests for the lpm trie map")
from the net-next tree and commit:
88baa78d1f31 ("selftests: remove duplicated all and clean target")
from the kselftest tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
P.S. Shuah, that kselftest commit has a different email address in its
Author and Signed-off-by.
--
Cheers,
Stephen Rothwell
diff --cc tools/testing/selftests/bpf/Makefile
index 064a3e5f2836,058351b0694f..000000000000
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@@ -1,13 -1,7 +1,7 @@@
CFLAGS += -Wall -O2 -I../../../../usr/include
- test_objs = test_verifier test_maps test_lru_map test_lpm_map
-TEST_GEN_PROGS = test_verifier test_maps test_lru_map
++TEST_GEN_PROGS = test_verifier test_maps test_lru_map test_lpm_map
- TEST_PROGS := test_verifier test_maps test_lru_map test_lpm_map test_kmod.sh
- TEST_FILES := $(test_objs)
-
- all: $(test_objs)
+ TEST_PROGS := test_kmod.sh
include ../lib.mk
-
- clean:
- $(RM) $(test_objs)
^ permalink raw reply
* Re: [PATCH 2/3] ath10k: use dma_zalloc_coherent()
From: Valo, Kalle @ 2017-01-24 5:18 UTC (permalink / raw)
To: Joe Perches
Cc: Srinivas Kandagatla, ath10k@lists.infradead.org,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1485213569.12563.32.camel@perches.com>
Joe Perches <joe@perches.com> writes:
> On Mon, 2017-01-23 at 15:04 +0000, Srinivas Kandagatla wrote:
>> use dma_zalloc_coherent() instead of dma_alloc_coherent and memset().
> []
>> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
> []
>> @@ -896,7 +896,7 @@ static int ath10k_pci_diag_read_mem(struct ath10k *ar, u32 address, void *data,
>> */
>> alloc_nbytes = min_t(unsigned int, nbytes, DIAG_TRANSFER_LIMIT);
>>
>> - data_buf = (unsigned char *)dma_alloc_coherent(ar->dev,
>> + data_buf = (unsigned char *)dma_zalloc_coherent(ar->dev,
>> alloc_nbytes,
>> &ce_data_base,
>> GFP_ATOMIC);
>
> trivia:
>
> Nicer to realign arguments and remove the unnecessary cast.
>
> Perhaps:
>
> data_buf = dma_zalloc_coherent(ar->dev, alloc_nbytes, &ce_data_base,
> GFP_ATOMIC);
Sure, but that should be in a separate patch.
--
Kalle Valo
^ permalink raw reply
* loopback device reference count leakage
From: Kaiwen Xu @ 2017-01-24 5:17 UTC (permalink / raw)
To: netdev@vger.kernel.org
Hi netdev folks,
I am currently experiencing an issue related with the loopback during
network devices shutdown inside a network namespace, which mainfested as
unregister_netdevice: waiting for lo to become free. Usage count = 1
showing up every 10 seconds or so in the kernel log. It eventually
causes a deadlock on new network namespace creation.
When I was trying to debug the issue, I tested from latest kernel 4.10-rc
back to 3.19, which all can be reproduced on. I also tried to disable
IPv6, which doesn't help either.
So I tried to patch the kernel with following change to add addtional
logging message for dev_put() and dev_hold().
https://lkml.org/lkml/2008/7/20/5
Here are all the dev_put/hold() calls for the loopback device inside a
namespace from creation to shutdown, when the issue occurs.
Jan 11 16:17:43 <hostname> kernel: [ 2196.943640] lo: dev_hold 1 rx_queue_add_kobject
Jan 11 16:17:43 <hostname> kernel: [ 2196.943652] lo: dev_hold 2 netdev_queue_add_kobject
Jan 11 16:17:43 <hostname> kernel: [ 2196.943654] lo: dev_hold 3 register_netdevice
Jan 11 16:17:43 <hostname> kernel: [ 2196.943658] lo: dev_hold 4 neigh_parms_alloc
Jan 11 16:17:43 <hostname> kernel: [ 2196.943660] lo: dev_hold 5 inetdev_init
Jan 11 16:17:43 <hostname> kernel: [ 2197.001771] lo: dev_hold 6 qdisc_alloc
Jan 11 16:17:43 <hostname> kernel: [ 2197.001815] lo: dev_hold 7 dev_get_by_index
Jan 11 16:17:43 <hostname> kernel: [ 2197.001824] lo: dev_hold 8 dev_get_by_index
Jan 11 16:17:43 <hostname> kernel: [ 2197.001831] lo: dev_hold 9 fib_check_nh
Jan 11 16:17:43 <hostname> kernel: [ 2197.001836] lo: dev_hold 10 fib_check_nh
Jan 11 16:17:43 <hostname> kernel: [ 2197.001843] lo: dev_hold 11 dev_get_by_index
Jan 11 16:17:43 <hostname> kernel: [ 2197.001849] lo: dev_hold 12 dev_get_by_index
Jan 11 16:17:43 <hostname> kernel: [ 2197.001852] lo: dev_hold 13 fib_check_nh
Jan 11 16:17:43 <hostname> kernel: [ 2197.001856] lo: dev_hold 14 fib_check_nh
Jan 11 16:17:43 <hostname> kernel: [ 2197.025451] lo: dev_put 14 free_fib_info_rcu
Jan 11 16:17:43 <hostname> kernel: [ 2197.025473] lo: dev_put 13 free_fib_info_rcu
Jan 11 16:17:43 <hostname> kernel: [ 2197.025475] lo: dev_put 12 free_fib_info_rcu
Jan 11 16:17:43 <hostname> kernel: [ 2197.025477] lo: dev_put 11 free_fib_info_rcu
Jan 11 16:17:43 <hostname> kernel: [ 2197.025480] lo: dev_put 10 free_fib_info_rcu
Jan 11 16:17:43 <hostname> kernel: [ 2197.025482] lo: dev_put 9 free_fib_info_rcu
Jan 11 16:17:43 <hostname> kernel: [ 2197.414900] lo: dev_hold 9 dst_init
Jan 11 16:17:43 <hostname> kernel: [ 2197.418634] lo: dev_hold 10 dst_init
Jan 11 16:17:43 <hostname> kernel: [ 2197.418638] lo: dev_hold 11 dst_init
Jan 11 16:17:43 <hostname> kernel: [ 2197.445496] lo: dev_put 11 dst_destroy
Jan 11 16:17:44 <hostname> kernel: [ 2197.614240] lo: dev_hold 11 dst_init
Jan 11 16:20:41 <hostname> kernel: [ 2374.898896] lo: dev_hold 12 dst_init
Jan 11 16:20:41 <hostname> kernel: [ 2374.898917] lo: dev_hold 13 __neigh_create
Jan 11 16:23:42 <hostname> kernel: [ 2555.900160] lo: dev_hold 14 dst_init
Jan 11 16:24:10 <hostname> kernel: [ 2583.573193] lo: dev_hold 15 dst_init
Jan 11 16:26:09 <hostname> kernel: [ 2702.992174] lo: dev_hold 16 dst_init
Jan 11 16:27:15 <hostname> kernel: [ 2769.188044] lo: dev_hold 17 dst_init
Jan 11 16:31:57 <hostname> kernel: [ 3050.821593] lo: dev_hold 18 dst_init
Jan 11 16:37:41 <hostname> kernel: [ 3394.962714] lo: dev_hold 19 dst_init
Jan 11 16:46:27 <hostname> kernel: [ 3921.376163] lo: dev_hold 20 dst_init
Jan 11 16:46:30 <hostname> kernel: [ 3923.759813] lo: dev_hold 21 dst_init
Jan 11 16:47:01 <hostname> kernel: [ 3954.802588] lo: dev_hold 22 dst_ifdown
Jan 11 16:47:01 <hostname> kernel: [ 3954.802596] lo: dev_hold 23 dst_ifdown
Jan 11 16:47:01 <hostname> kernel: [ 3954.802599] lo: dev_hold 24 dst_ifdown
Jan 11 16:47:01 <hostname> kernel: [ 3954.802602] lo: dev_hold 25 dst_ifdown
Jan 11 16:47:01 <hostname> kernel: [ 3954.802605] lo: dev_hold 26 dst_ifdown
Jan 11 16:47:01 <hostname> kernel: [ 3954.802609] lo: dev_hold 27 dst_ifdown
Jan 11 16:47:01 <hostname> kernel: [ 3955.482563] lo: dev_put 27 dst_destroy
Jan 11 16:47:01 <hostname> kernel: [ 3955.482566] lo: dev_put 26 dst_destroy
Jan 11 16:47:01 <hostname> kernel: [ 3955.482568] lo: dev_put 25 dst_destroy
Jan 11 16:47:01 <hostname> kernel: [ 3955.482570] lo: dev_put 24 dst_destroy
Jan 11 16:47:01 <hostname> kernel: [ 3955.482572] lo: dev_put 23 dst_destroy
Jan 11 16:47:01 <hostname> kernel: [ 3955.482574] lo: dev_put 22 dst_destroy
Jan 11 16:49:20 <hostname> kernel: [ 4093.651505] lo: dev_put 21 neigh_destroy
Jan 11 16:49:20 <hostname> kernel: [ 4093.653397] lo: dev_put 20 qdisc_destroy
Jan 11 16:49:20 <hostname> kernel: [ 4093.653432] lo: dev_put 19 neigh_parms_release
Jan 11 16:49:20 <hostname> kernel: [ 4093.653463] lo: dev_put 18 rx_queue_release
Jan 11 16:49:20 <hostname> kernel: [ 4093.653474] lo: dev_put 17 netdev_queue_release
Jan 11 16:49:20 <hostname> kernel: [ 4093.653520] lo: dev_put 16 rollback_registered_many
Jan 11 16:49:20 <hostname> kernel: [ 4093.663424] lo: dev_put 15 free_fib_info_rcu
Jan 11 16:49:20 <hostname> kernel: [ 4093.667401] lo: dev_put 14 free_fib_info_rcu
Jan 11 16:49:20 <hostname> kernel: [ 4093.667430] lo: dev_put 13 dst_destroy
Jan 11 16:49:20 <hostname> kernel: [ 4093.667432] lo: dev_put 12 dst_destroy
Jan 11 16:49:20 <hostname> kernel: [ 4093.667434] lo: dev_put 11 dst_destroy
Jan 11 16:49:20 <hostname> kernel: [ 4093.667436] lo: dev_put 10 dst_destroy
Jan 11 16:49:20 <hostname> kernel: [ 4093.667438] lo: dev_put 9 dst_destroy
Jan 11 16:49:20 <hostname> kernel: [ 4093.667441] lo: dev_put 8 dst_destroy
Jan 11 16:49:20 <hostname> kernel: [ 4093.667446] lo: dev_put 7 dst_destroy
Jan 11 16:49:20 <hostname> kernel: [ 4093.667453] lo: dev_put 6 dst_destroy
Jan 11 16:49:20 <hostname> kernel: [ 4093.667459] lo: dev_put 5 dst_destroy
Jan 11 16:49:20 <hostname> kernel: [ 4093.667461] lo: dev_put 4 dst_destroy
Jan 11 16:49:20 <hostname> kernel: [ 4093.667463] lo: dev_put 3 dst_destroy
Jan 11 16:49:20 <hostname> kernel: [ 4093.667478] lo: dev_put 2 in_dev_finish_destroy
Jan 11 16:49:20 <hostname> kernel: [ 4093.667493] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:20 <hostname> kernel: [ 4093.667495] lo: dev_put 2 dst_ifdown
Jan 11 16:49:21 <hostname> kernel: [ 4094.691406] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:21 <hostname> kernel: [ 4094.691409] lo: dev_put 2 dst_ifdown
Jan 11 16:49:22 <hostname> kernel: [ 4095.715386] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:22 <hostname> kernel: [ 4095.715390] lo: dev_put 2 dst_ifdown
Jan 11 16:49:23 <hostname> kernel: [ 4096.739367] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:23 <hostname> kernel: [ 4096.739370] lo: dev_put 2 dst_ifdown
Jan 11 16:49:24 <hostname> kernel: [ 4097.763338] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:24 <hostname> kernel: [ 4097.763342] lo: dev_put 2 dst_ifdown
Jan 11 16:49:25 <hostname> kernel: [ 4098.787312] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:25 <hostname> kernel: [ 4098.787315] lo: dev_put 2 dst_ifdown
Jan 11 16:49:26 <hostname> kernel: [ 4099.811293] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:26 <hostname> kernel: [ 4099.811297] lo: dev_put 2 dst_ifdown
Jan 11 16:49:27 <hostname> kernel: [ 4100.835270] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:27 <hostname> kernel: [ 4100.835273] lo: dev_put 2 dst_ifdown
Jan 11 16:49:28 <hostname> kernel: [ 4101.859249] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:28 <hostname> kernel: [ 4101.859252] lo: dev_put 2 dst_ifdown
Jan 11 16:49:29 <hostname> kernel: [ 4102.883228] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:29 <hostname> kernel: [ 4102.883231] lo: dev_put 2 dst_ifdown
Jan 11 16:49:30 <hostname> kernel: [ 4103.907196] unregister_netdevice: waiting for lo to become free. Usage count = 1
Jan 11 16:49:30 <hostname> kernel: [ 4103.916753] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:30 <hostname> kernel: [ 4103.916755] lo: dev_put 2 dst_ifdown
Jan 11 16:49:31 <hostname> kernel: [ 4104.939171] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:31 <hostname> kernel: [ 4104.939174] lo: dev_put 2 dst_ifdown
Jan 11 16:49:32 <hostname> kernel: [ 4105.963147] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:32 <hostname> kernel: [ 4105.963150] lo: dev_put 2 dst_ifdown
Jan 11 16:49:33 <hostname> kernel: [ 4106.987135] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:33 <hostname> kernel: [ 4106.987138] lo: dev_put 2 dst_ifdown
Jan 11 16:49:34 <hostname> kernel: [ 4108.011109] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:34 <hostname> kernel: [ 4108.011112] lo: dev_put 2 dst_ifdown
Jan 11 16:49:35 <hostname> kernel: [ 4109.035084] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:35 <hostname> kernel: [ 4109.035087] lo: dev_put 2 dst_ifdown
Jan 11 16:49:36 <hostname> kernel: [ 4110.059057] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:36 <hostname> kernel: [ 4110.059060] lo: dev_put 2 dst_ifdown
Jan 11 16:49:37 <hostname> kernel: [ 4111.095053] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:37 <hostname> kernel: [ 4111.095056] lo: dev_put 2 dst_ifdown
Jan 11 16:49:38 <hostname> kernel: [ 4112.119017] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:38 <hostname> kernel: [ 4112.119020] lo: dev_put 2 dst_ifdown
Jan 11 16:49:39 <hostname> kernel: [ 4113.142987] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:39 <hostname> kernel: [ 4113.142990] lo: dev_put 2 dst_ifdown
Jan 11 16:49:40 <hostname> kernel: [ 4114.166957] unregister_netdevice: waiting for lo to become free. Usage count = 1
Jan 11 16:49:40 <hostname> kernel: [ 4114.176525] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:40 <hostname> kernel: [ 4114.176527] lo: dev_put 2 dst_ifdown
Jan 11 16:49:41 <hostname> kernel: [ 4115.198945] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:41 <hostname> kernel: [ 4115.198949] lo: dev_put 2 dst_ifdown
Jan 11 16:49:42 <hostname> kernel: [ 4116.222927] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:42 <hostname> kernel: [ 4116.222931] lo: dev_put 2 dst_ifdown
Jan 11 16:49:43 <hostname> kernel: [ 4117.246894] lo: dev_hold 2 dst_ifdown
Jan 11 16:49:43 <hostname> kernel: [ 4117.246897] lo: dev_put 2 dst_ifdown
As a comparison, here are the calls when the issue didn't occur.
Jan 11 15:59:57 <hostname> kernel: [ 1131.509464] lo: dev_hold 1 rx_queue_add_kobject
Jan 11 15:59:57 <hostname> kernel: [ 1131.509477] lo: dev_hold 2 netdev_queue_add_kobject
Jan 11 15:59:57 <hostname> kernel: [ 1131.509480] lo: dev_hold 3 register_netdevice
Jan 11 15:59:57 <hostname> kernel: [ 1131.509483] lo: dev_hold 4 neigh_parms_alloc
Jan 11 15:59:57 <hostname> kernel: [ 1131.509485] lo: dev_hold 5 inetdev_init
Jan 11 15:59:57 <hostname> kernel: [ 1131.560173] lo: dev_hold 6 qdisc_alloc
Jan 11 15:59:57 <hostname> kernel: [ 1131.560203] lo: dev_hold 7 dev_get_by_index
Jan 11 15:59:57 <hostname> kernel: [ 1131.560210] lo: dev_hold 8 dev_get_by_index
Jan 11 15:59:57 <hostname> kernel: [ 1131.560217] lo: dev_hold 9 fib_check_nh
Jan 11 15:59:57 <hostname> kernel: [ 1131.560221] lo: dev_hold 10 fib_check_nh
Jan 11 15:59:57 <hostname> kernel: [ 1131.560228] lo: dev_hold 11 dev_get_by_index
Jan 11 15:59:57 <hostname> kernel: [ 1131.560234] lo: dev_hold 12 dev_get_by_index
Jan 11 15:59:57 <hostname> kernel: [ 1131.560237] lo: dev_hold 13 fib_check_nh
Jan 11 15:59:57 <hostname> kernel: [ 1131.560240] lo: dev_hold 14 fib_check_nh
Jan 11 15:59:57 <hostname> kernel: [ 1131.580072] lo: dev_put 14 free_fib_info_rcu
Jan 11 15:59:57 <hostname> kernel: [ 1131.580077] lo: dev_put 13 free_fib_info_rcu
Jan 11 15:59:57 <hostname> kernel: [ 1131.580079] lo: dev_put 12 free_fib_info_rcu
Jan 11 15:59:57 <hostname> kernel: [ 1131.580081] lo: dev_put 11 free_fib_info_rcu
Jan 11 15:59:57 <hostname> kernel: [ 1131.580084] lo: dev_put 10 free_fib_info_rcu
Jan 11 15:59:57 <hostname> kernel: [ 1131.580097] lo: dev_put 9 free_fib_info_rcu
Jan 11 16:00:00 <hostname> kernel: [ 1134.594026] lo: dev_hold 9 dst_init
Jan 11 16:00:01 <hostname> kernel: [ 1135.020297] lo: dev_hold 10 dst_init
Jan 11 16:00:01 <hostname> kernel: [ 1135.020304] lo: dev_hold 11 dst_init
Jan 11 16:00:01 <hostname> kernel: [ 1135.039974] lo: dev_put 11 dst_destroy
Jan 11 16:00:02 <hostname> kernel: [ 1136.453739] lo: dev_hold 11 dst_init
Jan 11 16:10:03 <hostname> kernel: [ 1736.603352] lo: dev_hold 12 dst_init
Jan 11 16:10:03 <hostname> kernel: [ 1736.603374] lo: dev_hold 13 __neigh_create
Jan 11 16:11:01 <hostname> kernel: [ 1795.209103] lo: dev_hold 14 dst_init
Jan 11 16:11:26 <hostname> kernel: [ 1820.491721] lo: dev_hold 15 dst_init
Jan 11 16:11:31 <hostname> kernel: [ 1825.519853] lo: dev_hold 16 dst_init
Jan 11 16:12:08 <hostname> kernel: [ 1861.911872] lo: dev_hold 17 dst_init
Jan 11 16:12:10 <hostname> kernel: [ 1864.509095] lo: dev_hold 18 dst_init
Jan 11 16:12:11 <hostname> kernel: [ 1865.094510] lo: dev_hold 19 dst_init
Jan 11 16:12:11 <hostname> kernel: [ 1865.510777] lo: dev_hold 20 dst_init
Jan 11 16:12:12 <hostname> kernel: [ 1865.994909] lo: dev_hold 21 dst_init
Jan 11 16:12:13 <hostname> kernel: [ 1866.861904] lo: dev_hold 22 dst_init
Jan 11 16:12:13 <hostname> kernel: [ 1867.195370] lo: dev_hold 23 dst_init
Jan 11 16:12:14 <hostname> kernel: [ 1868.167117] lo: dev_hold 24 dst_init
Jan 11 16:12:48 <hostname> kernel: [ 1902.280896] lo: dev_hold 25 dst_ifdown
Jan 11 16:12:49 <hostname> kernel: [ 1902.960852] lo: dev_put 25 dst_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.413561] lo: dev_put 24 neigh_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.415441] lo: dev_put 23 qdisc_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.415476] lo: dev_put 22 neigh_parms_release
Jan 11 16:14:59 <hostname> kernel: [ 2033.415508] lo: dev_put 21 rx_queue_release
Jan 11 16:14:59 <hostname> kernel: [ 2033.415518] lo: dev_put 20 netdev_queue_release
Jan 11 16:14:59 <hostname> kernel: [ 2033.415563] lo: dev_put 19 rollback_registered_many
Jan 11 16:14:59 <hostname> kernel: [ 2033.425510] lo: dev_put 18 free_fib_info_rcu
Jan 11 16:14:59 <hostname> kernel: [ 2033.429481] lo: dev_put 17 free_fib_info_rcu
Jan 11 16:14:59 <hostname> kernel: [ 2033.429499] lo: dev_put 16 dst_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.429507] lo: dev_put 15 dst_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.429509] lo: dev_put 14 dst_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.429511] lo: dev_put 13 dst_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.429513] lo: dev_put 12 dst_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.429515] lo: dev_put 11 dst_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.429517] lo: dev_put 10 dst_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.429519] lo: dev_put 9 dst_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.429523] lo: dev_put 8 dst_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.429529] lo: dev_put 7 dst_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.429536] lo: dev_put 6 dst_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.429541] lo: dev_put 5 dst_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.429543] lo: dev_put 4 dst_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.429545] lo: dev_put 3 dst_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.429551] lo: dev_put 2 in_dev_finish_destroy
Jan 11 16:14:59 <hostname> kernel: [ 2033.429563] lo: dev_hold 2 dst_ifdown
Jan 11 16:14:59 <hostname> kernel: [ 2033.429565] lo: dev_put 2 dst_ifdown
Jan 11 16:15:00 <hostname> kernel: [ 2034.453484] lo: dev_hold 2 dst_ifdown
Jan 11 16:15:00 <hostname> kernel: [ 2034.453487] lo: dev_put 2 dst_ifdown
Jan 11 16:15:01 <hostname> kernel: [ 2035.129452] lo: dev_put 1 dst_destroy
Could you please give me some pointers why this issue could occur? I can
also provide addtional information if needed.
Thanks,
Kaiwen
^ permalink raw reply
* Re: [PATCH 2/3] ath10k: use dma_zalloc_coherent()
From: Joe Perches @ 2017-01-24 5:25 UTC (permalink / raw)
To: Valo, Kalle
Cc: Srinivas Kandagatla, ath10k@lists.infradead.org,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <8737g9ox1k.fsf@qca.qualcomm.com>
On Tue, 2017-01-24 at 05:18 +0000, Valo, Kalle wrote:
> Joe Perches <joe@perches.com> writes:
>
> > On Mon, 2017-01-23 at 15:04 +0000, Srinivas Kandagatla wrote:
> > > use dma_zalloc_coherent() instead of dma_alloc_coherent and memset().
> >
> > []
> > > diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
> >
> > []
> > > @@ -896,7 +896,7 @@ static int ath10k_pci_diag_read_mem(struct ath10k *ar, u32 address, void *data,
> > > */
> > > alloc_nbytes = min_t(unsigned int, nbytes, DIAG_TRANSFER_LIMIT);
> > >
> > > - data_buf = (unsigned char *)dma_alloc_coherent(ar->dev,
> > > + data_buf = (unsigned char *)dma_zalloc_coherent(ar->dev,
> > > alloc_nbytes,
> > > &ce_data_base,
> > > GFP_ATOMIC);
> >
> > trivia:
> >
> > Nicer to realign arguments and remove the unnecessary cast.
> >
> > Perhaps:
> >
> > data_buf = dma_zalloc_coherent(ar->dev, alloc_nbytes, &ce_data_base,
> > GFP_ATOMIC);
>
> Sure, but that should be in a separate patch.
I don't think so, trivial patches can be combined.
It's also nicer to realign all modified multiline
arguments when performing these changes.
Coccinelle generally does it automatically.
^ permalink raw reply
* [PATCH net] sctp: sctp_addr_id2transport should verify the addr before looking up assoc
From: Xin Long @ 2017-01-24 6:01 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: davem, Marcelo Ricardo Leitner, Neil Horman
sctp_addr_id2transport is a function for sockopt to look up assoc by
address. As the address is from userspace, it can be a v4-mapped v6
address. But in sctp protocol stack, it always handles a v4-mapped
v6 address as a v4 address. So it's necessary to convert it to a v4
address before looking up assoc by address.
This patch is to fix it by calling sctp_verify_addr in which it can do
this conversion before calling sctp_endpoint_lookup_assoc, just like
what sctp_sendmsg and __sctp_connect do for the address from users.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/socket.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 318c678..37eeab7 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -235,8 +235,12 @@ static struct sctp_transport *sctp_addr_id2transport(struct sock *sk,
sctp_assoc_t id)
{
struct sctp_association *addr_asoc = NULL, *id_asoc = NULL;
- struct sctp_transport *transport;
+ struct sctp_af *af = sctp_get_af_specific(addr->ss_family);
union sctp_addr *laddr = (union sctp_addr *)addr;
+ struct sctp_transport *transport;
+
+ if (sctp_verify_addr(sk, laddr, af->sockaddr_len))
+ return NULL;
addr_asoc = sctp_endpoint_lookup_assoc(sctp_sk(sk)->ep,
laddr,
--
2.1.0
^ permalink raw reply related
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