Netdev List
 help / color / mirror / Atom feed
* Re: selftests/bpf doesn't compile
From: Alexei Starovoitov @ 2017-09-15 16:02 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Daniel Borkmann, Thomas Meyer, linux-kernel, linux-kselftest,
	Shuah Khan, Networking
In-Reply-To: <295553a4-aadc-e5d1-229e-22d1966bc9f5@kernel.org>

On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
> Hi Alexei and Daniel,
> 
> bpf test depends on clang and fails to compile when
> 
> ------------------------------------------------------
> make -C tools/testing/selftests/bpf run_tests
> 
> 
> make: clang: Command not found
> Makefile:39: recipe for target '.linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o' failed
> make: *** [./linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o] Error 127
> make: Leaving directory '.linux-kselftest/tools/testing/selftests/bpf'
> 
> With "make TARGETS=bpf kselftest" it fails earlier:
> 
> 
> make[3]: Entering directory './linux-kselftest/tools/lib/bpf'
> Makefile:40: tools/scripts/Makefile.arch: No such file or directory
> Makefile:84: tools/build/Makefile.feature: No such file or directory
> Makefile:143: tools/build/Makefile.include: No such file or directory
> make[3]: *** No rule to make target 'tools/build/Makefile.include'.  Stop.
> make[3]: Leaving directory '.linux-kselftest/tools/lib/bpf'
> Makefile:34: recipe for target './linux-kselftest/tools/testing/selftests/bpf/libbpf.a' failed
> make[2]: *** [./linux-kselftest/tools/testing/selftests/bpf/libbpf.a] Error 2
> make[2]: Leaving directory './linux-kselftest/tools/testing/selftests/bpf'
> Makefile:69: recipe for target 'all' failed
> make[1]: *** [all] Error 2
> Makefile:1190: recipe for target 'kselftest' failed
> make: *** [kselftest] Error 2
> 
> --------------------------------------------------------------
> 
> Is bpf test intended to be run in kselftest run? The clang dependency might
> not be met on majority of the systems. Is this a hard dependency??

It is a hard dependency and clang should be present on majority of the systems.
More details are in samples/bpf/README.rst
which was written long ago. Nowadays apt-get/yum will install clang
with bpf support builtin.

> Would it make sense to create a special target for bpf test? We do have a few
> tests that do that now. 
> 
> TARGETS_HOTPLUG = cpu-hotplug
> TARGETS_HOTPLUG += memory-hotplug
> 
> I could add a special target for bpf TARGET_BPF perhaps and exclude it from
> the run_tests.

I'm not sure what was the motivation to exclude hotplug from default testing,
since I think it diminishes the value of selftests overall.
Not running all tests all the time risks breaking them.
selftest makefile refactoring broke selftests/bpf in the past,
so I strongly suggest to install clang and make sure the tests are passing
on the test servers otherwise we'd need to move selftests/bpf out of
selftests to avoid further headaches for us when selftests infra keeps
changing.

^ permalink raw reply

* Re: selftests/bpf doesn't compile
From: Edward Cree @ 2017-09-15 16:58 UTC (permalink / raw)
  To: Alexei Starovoitov, Shuah Khan
  Cc: Daniel Borkmann, Thomas Meyer, linux-kernel, linux-kselftest,
	Shuah Khan, Networking
In-Reply-To: <20170915160253.q3x5j7hfkxxh2g6w@ast-mbp>

On 15/09/17 17:02, Alexei Starovoitov wrote:
> On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
>> Is bpf test intended to be run in kselftest run? The clang dependency might
>> not be met on majority of the systems. Is this a hard dependency??
> It is a hard dependency and clang should be present on majority of the systems.
I think this is the wrong approach.  Making kselftest hard-require clang doesn't
 mean that the bpf tests will be run more often, it means that the rest of the
 kselftests will be run less often.  clang is quite big (when I tried to install
 it on one of my test servers, I didn't have enough disk space & had to go on a
 clear-out of unused packages), and most people aren't interested in the bpf
 subsystem specifically; they would rather be able to skip those tests.
I feel that as long as they know they are skipping some tests (so e.g. they
 won't consider it a sufficient test of a kselftest refactor), that's fine.
It's not even as though all of the bpf tests require clang; the (smaller) tests
 written directly in raw eBPF instructions could still be run on such a system.
 So I think we should attempt to run as much as possible but accept that clang
 may not be available and have an option to skip some tests in that case.

-Ed

^ permalink raw reply

* Re: selftests/bpf doesn't compile
From: Shuah Khan @ 2017-09-15 17:00 UTC (permalink / raw)
  To: Alexei Starovoitov, Shuah Khan
  Cc: Daniel Borkmann, Thomas Meyer, linux-kernel, linux-kselftest,
	Networking, Shuah Khan
In-Reply-To: <20170915160253.q3x5j7hfkxxh2g6w@ast-mbp>

On 09/15/2017 10:02 AM, Alexei Starovoitov wrote:
> On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
>> Hi Alexei and Daniel,
>>
>> bpf test depends on clang and fails to compile when
>>
>> ------------------------------------------------------
>> make -C tools/testing/selftests/bpf run_tests
>>
>>
>> make: clang: Command not found
>> Makefile:39: recipe for target '.linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o' failed
>> make: *** [./linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o] Error 127
>> make: Leaving directory '.linux-kselftest/tools/testing/selftests/bpf'
>>
>> With "make TARGETS=bpf kselftest" it fails earlier:
>>
>>
>> make[3]: Entering directory './linux-kselftest/tools/lib/bpf'
>> Makefile:40: tools/scripts/Makefile.arch: No such file or directory
>> Makefile:84: tools/build/Makefile.feature: No such file or directory
>> Makefile:143: tools/build/Makefile.include: No such file or directory
>> make[3]: *** No rule to make target 'tools/build/Makefile.include'.  Stop.
>> make[3]: Leaving directory '.linux-kselftest/tools/lib/bpf'
>> Makefile:34: recipe for target './linux-kselftest/tools/testing/selftests/bpf/libbpf.a' failed
>> make[2]: *** [./linux-kselftest/tools/testing/selftests/bpf/libbpf.a] Error 2
>> make[2]: Leaving directory './linux-kselftest/tools/testing/selftests/bpf'
>> Makefile:69: recipe for target 'all' failed
>> make[1]: *** [all] Error 2
>> Makefile:1190: recipe for target 'kselftest' failed
>> make: *** [kselftest] Error 2
>>
>> --------------------------------------------------------------
>>
>> Is bpf test intended to be run in kselftest run? The clang dependency might
>> not be met on majority of the systems. Is this a hard dependency??
> 
> It is a hard dependency and clang should be present on majority of the systems.
> More details are in samples/bpf/README.rst
> which was written long ago. Nowadays apt-get/yum will install clang
> with bpf support builtin.

Thanks for the clarification.

> 
>> Would it make sense to create a special target for bpf test? We do have a few
>> tests that do that now. 
>>
>> TARGETS_HOTPLUG = cpu-hotplug
>> TARGETS_HOTPLUG += memory-hotplug
>>
>> I could add a special target for bpf TARGET_BPF perhaps and exclude it from
>> the run_test> 
> I'm not sure what was the motivation to exclude hotplug from default testing,

These are considered a bit more disruptive and were excluded a while
back. These take cpus and memory on and off-line. Also require
root access. So even if they are included in the regular run, these
won't run.

> since I think it diminishes the value of selftests overall.

I agree. We do have some timer tests that are destructive/stress that
et run using a special target. It is the idea that if somebody wants
to test all them, there is a way to do that.

In any case, I didn't think bpf falls into this category of tests that
belong in the destructive category. I am looking to understand the failures
and your take on those.

> Not running all tests all the time risks breaking them
It is balance of providing a choice to users if they don't want to
run destructive tests. For example, suspend test which requires root
access. So the idea is for users to run these by choice as opposed
to running them in the normal run and cause disruption.

> selftest makefile refactoring broke selftests/bpf in the past,

Yeah. We have had some fallout from the KBUILD_OUTPUT work that didn't
take all the use-cases into account and tests that require custom
builds such as the bpf tests. Using common build infrastructure doesn't
work for all tests.

Looks like there are other patches that went in later with lcap work.

> so I strongly suggest to install clang and make sure the tests are passing
> on the test servers 

You will have to request kernelci, stable, and It is a choice to be made by
kernelci/zero-day folks.

otherwise we'd need to move selftests/bpf out of
> selftests to avoid further headaches for us when selftests infra keeps
> changing.
> 

Let's not go to extreme options. :) I am merely looking for more information
and trying to understand the dependencies for this test.

Let's look for a constructive option to fix the build failures I am seeing.

The first failure due to clang dependency is not a problem. The second one
in the case of "make kselftest" is the one that requires some work when bpf
make is run from the main Makefile. A lots of users run tests using the
kselftest target from the mail Makefile. hence I would like to get this
working, so it would be easier to run this test on test servers.

thanks,
-- Shuah

^ permalink raw reply

* (unknown), 
From: noreply @ 2017-09-15 17:01 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: EMAIL_75480323541895_netdev.doc --]
[-- Type: application/msword, Size: 76645 bytes --]

^ permalink raw reply

* Re: scheduling while atomic from vmci_transport_recv_stream_cb in 3.16 kernels
From: Ben Hutchings @ 2017-09-15 17:12 UTC (permalink / raw)
  To: Michal Hocko, Jorgen S. Hansen
  Cc: Aditya Sarwade, Thomas Hellstrom, LKML, netdev@vger.kernel.org,
	Masik Petr, Sasha Levin, Stable tree
In-Reply-To: <20170914085959.nkiefbzupwmknncy@dhcp22.suse.cz>

[-- Attachment #1: Type: text/plain, Size: 509 bytes --]

On Thu, 2017-09-14 at 10:59 +0200, Michal Hocko wrote:
> On Wed 13-09-17 18:58:13, Jorgen S. Hansen wrote:
> [...]
> > The patch series look good to me.
> 
> Thanks for double checking. Ben, could you merge this to 3.16 stable
> branch, please?

I have a long list of requests to work through, but I will get to this
eventually.

Ben.

-- 
Ben Hutchings
Kids!  Bringing about Armageddon can be dangerous.  Do not attempt it in
your own home. - Terry Pratchett and Neil Gaiman, `Good Omens'


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] net/packet: fix race condition between fanout_add and __unregister_prot_hook
From: Cong Wang @ 2017-09-15 17:41 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: nixiaoming, David Miller, Eric Dumazet, waltje, gw4pts,
	Andrey Konovalov, Tobias Klauser, Philip Pettersson,
	Alexander Potapenko, Network Development, LKML, dede.wu
In-Reply-To: <CAF=yD-Jt5boxgQne4s1Fzy0y=Lq7RO4fjF6BfOCoh1bgaSECHQ@mail.gmail.com>

On Thu, Sep 14, 2017 at 7:35 AM, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
> On Thu, Sep 14, 2017 at 10:07 AM, nixiaoming <nixiaoming@huawei.com> wrote:
>> From: l00219569 <lisimin@huawei.com>
>>
>> If fanout_add is preempted after running po-> fanout = match
>> and before running __fanout_link,
>> it will cause BUG_ON when __unregister_prot_hook call __fanout_unlink
>>
>> so, we need add mutex_lock(&fanout_mutex) to __unregister_prot_hook
>
> The packet socket code has no shortage of locks, so there are many
> ways to avoid the race condition between fanout_add and packet_set_ring.
>
> Another option would be to lock the socket when calling fanout_add:
>
>     -               return fanout_add(sk, val & 0xffff, val >> 16);
>     +               lock_sock(sk);
>     +               ret = fanout_add(sk, val & 0xffff, val >> 16);
>     +               release_sock(sk);
>     +               return ret;
>

I don't think this is an option, because __unregister_prot_hook()
can be called without lock_sock(), for example in packet_notifier().


> But, for consistency, and to be able to continue to make sense of the
> locking policy, we should use the most appropriate lock. This
> is po->bind_lock, as it ensures atomicity between testing whether
> a protocol hook is active through po->running and the actual existence
> of that hook on the protocol hook list.

Yeah, register_prot_hook() and unregister_prot_hook() already assume
bind_lock.

[...]

>>  out:
>>         mutex_unlock(&fanout_mutex);
>> +       spin_unlock(&po->bind_lock);
>
> This function can call kzalloc with GFP_KERNEL, which may sleep. It is
> not correct to sleep while holding a spinlock. Which is why I take the lock
> later and test po->running again.


Right, no need to mention the mutex_unlock() before the spin_unlock()
is clearly wrong.


>
> I will clean up that patch and send it for review.

How about the following patch?


diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index c26172995511..f5c696a548ed 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1754,10 +1754,14 @@ static int fanout_add(struct sock *sk, u16 id,
u16 type_flags)
            match->prot_hook.dev == po->prot_hook.dev) {
                err = -ENOSPC;
                if (refcount_read(&match->sk_ref) < PACKET_FANOUT_MAX) {
+                       spin_lock(&po->bind_lock);
                        __dev_remove_pack(&po->prot_hook);
-                       po->fanout = match;
-                       refcount_set(&match->sk_ref,
refcount_read(&match->sk_ref) + 1);
-                       __fanout_link(sk, po);
+                       if (po->running) {
+                               refcount_set(&match->sk_ref,
refcount_read(&match->sk_ref) + 1);
+                               po->fanout = match;
+                               __fanout_link(sk, po);
+                       }
+                       spin_unlock(&po->bind_lock);
                        err = 0;
                }
        }

^ permalink raw reply related

* Re: selftests/bpf doesn't compile
From: Shuah Khan @ 2017-09-15 17:44 UTC (permalink / raw)
  To: Alexei Starovoitov, Shuah Khan
  Cc: Daniel Borkmann, Thomas Meyer, linux-kernel, linux-kselftest,
	Networking, Shuah Khan
In-Reply-To: <e07dabee-6c9c-7842-65b0-990a2ed1a2e9@osg.samsung.com>

On 09/15/2017 11:00 AM, Shuah Khan wrote:
> On 09/15/2017 10:02 AM, Alexei Starovoitov wrote:
>> On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
>>> Hi Alexei and Daniel,
>>>
>>> bpf test depends on clang and fails to compile when
>>>
>>> ------------------------------------------------------
>>> make -C tools/testing/selftests/bpf run_tests
>>>
>>>
>>> make: clang: Command not found
>>> Makefile:39: recipe for target '.linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o' failed
>>> make: *** [./linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o] Error 127
>>> make: Leaving directory '.linux-kselftest/tools/testing/selftests/bpf'
>>>
>>> With "make TARGETS=bpf kselftest" it fails earlier:
>>>
>>>
>>> make[3]: Entering directory './linux-kselftest/tools/lib/bpf'
>>> Makefile:40: tools/scripts/Makefile.arch: No such file or directory
>>> Makefile:84: tools/build/Makefile.feature: No such file or directory
>>> Makefile:143: tools/build/Makefile.include: No such file or directory
>>> make[3]: *** No rule to make target 'tools/build/Makefile.include'.  Stop.
>>> make[3]: Leaving directory '.linux-kselftest/tools/lib/bpf'
>>> Makefile:34: recipe for target './linux-kselftest/tools/testing/selftests/bpf/libbpf.a' failed
>>> make[2]: *** [./linux-kselftest/tools/testing/selftests/bpf/libbpf.a] Error 2
>>> make[2]: Leaving directory './linux-kselftest/tools/testing/selftests/bpf'
>>> Makefile:69: recipe for target 'all' failed
>>> make[1]: *** [all] Error 2
>>> Makefile:1190: recipe for target 'kselftest' failed
>>> make: *** [kselftest] Error 2
>>>
>>> --------------------------------------------------------------
>>>
>>> Is bpf test intended to be run in kselftest run? The clang dependency might
>>> not be met on majority of the systems. Is this a hard dependency??
>>
>> It is a hard dependency and clang should be present on majority of the systems.
>> More details are in samples/bpf/README.rst
>> which was written long ago. Nowadays apt-get/yum will install clang
>> with bpf support builtin.
> 
> Thanks for the clarification.
> 
>>
>>> Would it make sense to create a special target for bpf test? We do have a few
>>> tests that do that now. 
>>>
>>> TARGETS_HOTPLUG = cpu-hotplug
>>> TARGETS_HOTPLUG += memory-hotplug
>>>
>>> I could add a special target for bpf TARGET_BPF perhaps and exclude it from
>>> the run_test> 
>> I'm not sure what was the motivation to exclude hotplug from default testing,
> 
> These are considered a bit more disruptive and were excluded a while
> back. These take cpus and memory on and off-line. Also require
> root access. So even if they are included in the regular run, these
> won't run.
> 
>> since I think it diminishes the value of selftests overall.
> 
> I agree. We do have some timer tests that are destructive/stress that
> et run using a special target. It is the idea that if somebody wants
> to test all them, there is a way to do that.
> 
> In any case, I didn't think bpf falls into this category of tests that
> belong in the destructive category. I am looking to understand the failures
> and your take on those.
> 
>> Not running all tests all the time risks breaking them
> It is balance of providing a choice to users if they don't want to
> run destructive tests. For example, suspend test which requires root
> access. So the idea is for users to run these by choice as opposed
> to running them in the normal run and cause disruption.
> 
>> selftest makefile refactoring broke selftests/bpf in the past,
> 
> Yeah. We have had some fallout from the KBUILD_OUTPUT work that didn't
> take all the use-cases into account and tests that require custom
> builds such as the bpf tests. Using common build infrastructure doesn't
> work for all tests.
> 
> Looks like there are other patches that went in later with lcap work.
> 
>> so I strongly suggest to install clang and make sure the tests are passing
>> on the test servers 
> 
> You will have to request kernelci, stable, and It is a choice to be made by
> kernelci/zero-day folks.
> 
> otherwise we'd need to move selftests/bpf out of
>> selftests to avoid further headaches for us when selftests infra keeps
>> changing.
>>
> 
> Let's not go to extreme options. :) I am merely looking for more information
> and trying to understand the dependencies for this test.
> 
> Let's look for a constructive option to fix the build failures I am seeing.
> 
> The first failure due to clang dependency is not a problem.

Let me clarify that. People interested in running bpf test will have to
install clang. In that sense installing clang will solve that issue.

The hard dependency on clang does make it difficult for developers to
ensure they didn't break bpf when they make changes to the kselftest
common infrastructure.

The second one
> in the case of "make kselftest" is the one that requires some work when bpf
> make is run from the main Makefile. A lots of users run tests using the
> kselftest target from the mail Makefile. hence I would like to get this
> working, so it would be easier to run this test on test servers.
> 

Even if this is fixed, unless users choose to install clang, bpf will always
fail run without clang. So clang dependency is an issue for bpf test coverage
in general. But that is your choice as to whether you want to increase the
scope of regression test coverage for bpf or not.

thanks,
-- Shuah

^ permalink raw reply

* Re: [PATCH] net/packet: fix race condition between fanout_add and __unregister_prot_hook
From: Willem de Bruijn @ 2017-09-15 17:46 UTC (permalink / raw)
  To: Cong Wang
  Cc: nixiaoming, David Miller, Eric Dumazet, waltje, gw4pts,
	Andrey Konovalov, Tobias Klauser, Philip Pettersson,
	Alexander Potapenko, Network Development, LKML, dede.wu
In-Reply-To: <CAM_iQpWAeB+8bm9u+eHS3k_h5PSbnztXBvzTC=f0-tohdiSgzQ@mail.gmail.com>

On Fri, Sep 15, 2017 at 1:41 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Thu, Sep 14, 2017 at 7:35 AM, Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
>> On Thu, Sep 14, 2017 at 10:07 AM, nixiaoming <nixiaoming@huawei.com> wrote:
>>> From: l00219569 <lisimin@huawei.com>
>>>
>>> If fanout_add is preempted after running po-> fanout = match
>>> and before running __fanout_link,
>>> it will cause BUG_ON when __unregister_prot_hook call __fanout_unlink
>>>
>>> so, we need add mutex_lock(&fanout_mutex) to __unregister_prot_hook
>>
>> The packet socket code has no shortage of locks, so there are many
>> ways to avoid the race condition between fanout_add and packet_set_ring.
>>
>> Another option would be to lock the socket when calling fanout_add:
>>
>>     -               return fanout_add(sk, val & 0xffff, val >> 16);
>>     +               lock_sock(sk);
>>     +               ret = fanout_add(sk, val & 0xffff, val >> 16);
>>     +               release_sock(sk);
>>     +               return ret;
>>
>
> I don't think this is an option, because __unregister_prot_hook()
> can be called without lock_sock(), for example in packet_notifier().
>
>
>> But, for consistency, and to be able to continue to make sense of the
>> locking policy, we should use the most appropriate lock. This
>> is po->bind_lock, as it ensures atomicity between testing whether
>> a protocol hook is active through po->running and the actual existence
>> of that hook on the protocol hook list.
>
> Yeah, register_prot_hook() and unregister_prot_hook() already assume
> bind_lock.
>
> [...]
>
>>>  out:
>>>         mutex_unlock(&fanout_mutex);
>>> +       spin_unlock(&po->bind_lock);
>>
>> This function can call kzalloc with GFP_KERNEL, which may sleep. It is
>> not correct to sleep while holding a spinlock. Which is why I take the lock
>> later and test po->running again.
>
>
> Right, no need to mention the mutex_unlock() before the spin_unlock()
> is clearly wrong.
>
>
>>
>> I will clean up that patch and send it for review.
>
> How about the following patch?
>
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index c26172995511..f5c696a548ed 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1754,10 +1754,14 @@ static int fanout_add(struct sock *sk, u16 id,
> u16 type_flags)
>             match->prot_hook.dev == po->prot_hook.dev) {
>                 err = -ENOSPC;
>                 if (refcount_read(&match->sk_ref) < PACKET_FANOUT_MAX) {
> +                       spin_lock(&po->bind_lock);
>                         __dev_remove_pack(&po->prot_hook);
> -                       po->fanout = match;
> -                       refcount_set(&match->sk_ref,
> refcount_read(&match->sk_ref) + 1);
> -                       __fanout_link(sk, po);
> +                       if (po->running) {
> +                               refcount_set(&match->sk_ref,
> refcount_read(&match->sk_ref) + 1);
> +                               po->fanout = match;
> +                               __fanout_link(sk, po);
> +                       }
> +                       spin_unlock(&po->bind_lock);
>                         err = 0;
>                 }
>         }

In case of failure we also need to unlink and free match. I
sent the following:

http://patchwork.ozlabs.org/patch/813945/

^ permalink raw reply

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
From: Josef Bacik @ 2017-09-15 17:51 UTC (permalink / raw)
  To: Cole Robinson, Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <813386f2-81ba-db8c-e86d-f36cd1b89537@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 530 bytes --]

Finally got access to a box to run this down myself.  This patch on top of the other patches fixes the problem for me, could you verify it works for you?  Thanks,

Josef

On 9/13/17, 3:49 PM, "Cole Robinson" <crobinso@redhat.com> wrote:

On 09/13/2017 03:44 PM, Josef Bacik wrote:
> Alright thanks, this should fix it.
> 

Still no luck with all three patches applied to fedora 4.12.8-300 RPM. Pretty
sure I didn't mess up the testing but since I rarely do kernel builds it's not
impossible...

Thanks,
Cole




[-- Attachment #2: 0001-net-call-sk_reuseport_match-if-we-are-a-reusesock.patch --]
[-- Type: application/octet-stream, Size: 1223 bytes --]

From b70e7a78af2c4c090ca816d9f127a2f1e5866fb8 Mon Sep 17 00:00:00 2001
From: Josef Bacik <josef@toxicpanda.com>
Date: Fri, 15 Sep 2017 10:48:17 -0700
Subject: [PATCH] net: call sk_reuseport_match if we are a reusesock

When adding the sk_reuseport_match helper I screwed up and made it so we
unconditionally succeeded if our socket was a reuseport socket and the
tb was marked reuseable.  Fix this by actually calling
sk_reuseport_match() to make sure our new socket matches the tb
reuseport settings.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 net/ipv4/inet_connection_sock.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index fe9cf4862de2..0e30e504c7d4 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -316,8 +316,7 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
 		if (sk->sk_reuse == SK_FORCE_REUSE)
 			goto success;
 
-		if ((tb->fastreuse > 0 && reuse) ||
-		    sk_reuseport_match(tb, sk))
+		if (tb->fastreuse > 0 && reuse && sk_reuseport_match(tb, sk))
 			goto success;
 		if (inet_csk_bind_conflict(sk, tb, true, true))
 			goto fail_unlock;
-- 
2.13.5


^ permalink raw reply related

* Re: [PATCH net] bpf/verifier: reject BPF_ALU64|BPF_END
From: Daniel Borkmann @ 2017-09-15 18:01 UTC (permalink / raw)
  To: Edward Cree, David Miller; +Cc: netdev
In-Reply-To: <8c3d5513-6171-3e68-56df-1efd0e87f071@solarflare.com>

On 09/15/2017 03:37 PM, Edward Cree wrote:
> Neither ___bpf_prog_run nor the JITs accept it.
> Also adds a new test case.
>
> Fixes: 17a5267067f3 ("bpf: verifier (add verifier core)")
> Signed-off-by: Edward Cree <ecree@solarflare.com>

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

^ permalink raw reply

* Re: selftests/bpf doesn't compile
From: Alexei Starovoitov @ 2017-09-15 18:07 UTC (permalink / raw)
  To: Edward Cree
  Cc: Shuah Khan, Daniel Borkmann, Thomas Meyer, linux-kernel,
	linux-kselftest, Shuah Khan, Networking
In-Reply-To: <701006c0-f80a-6bd7-fb5e-5bca1dc33a31@solarflare.com>

On Fri, Sep 15, 2017 at 05:58:40PM +0100, Edward Cree wrote:
> On 15/09/17 17:02, Alexei Starovoitov wrote:
> > On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
> >> Is bpf test intended to be run in kselftest run? The clang dependency might
> >> not be met on majority of the systems. Is this a hard dependency??
> > It is a hard dependency and clang should be present on majority of the systems.
> I think this is the wrong approach.  Making kselftest hard-require clang doesn't
>  mean that the bpf tests will be run more often, it means that the rest of the
>  kselftests will be run less often.  clang is quite big (when I tried to install
>  it on one of my test servers, I didn't have enough disk space & had to go on a
>  clear-out of unused packages), and most people aren't interested in the bpf
>  subsystem specifically; they would rather be able to skip those tests.
> I feel that as long as they know they are skipping some tests (so e.g. they
>  won't consider it a sufficient test of a kselftest refactor), that's fine.
> It's not even as though all of the bpf tests require clang; the (smaller) tests
>  written directly in raw eBPF instructions could still be run on such a system.
>  So I think we should attempt to run as much as possible but accept that clang
>  may not be available and have an option to skip some tests in that case.

imo the value of selftests/bpf is twofold:
1. it helps bpf developers avoid regressions
2. as part of continuous integration it helps to catch bpf regressions
that were somehow caused by changes in other parts of the kernel

If a developer didn't bother to satisfy all bpf tests dependencies
(which includes clang) and ran all tests before sending a patch,
I don't want to see such patches. It just wastes maintainers time
to review code and spot bugs that could have been caught by tests.
Collectively we invested years of work into these tests and
developers better take advantage of it by running all.

If a CI server didn't satisfy all bpf test dependencies,
I don't want such CI setup to be running and reporting results,
since it will give false sense of test coverage.
Test failures due to missing dependencies are hard failures.
We cannot skip them.

I'd like generic XDP tests to be added to selftests/bpf which
would mean that the latest iproute2 will become a hard dependency
and bpf developers and CI host owners would need to upgrade
their iproute2.
The tests either pass or fail. Skipping them due to missing
dependencies is the same as fail and in that sense I don't want
to change selftests/bpf/Makefile to make it skip clang.

^ permalink raw reply

* Re: [PATCH] net/packet: fix race condition between fanout_add and __unregister_prot_hook
From: Cong Wang @ 2017-09-15 18:09 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: nixiaoming, David Miller, Eric Dumazet, waltje, gw4pts,
	Andrey Konovalov, Tobias Klauser, Philip Pettersson,
	Alexander Potapenko, Network Development, LKML, dede.wu
In-Reply-To: <CAF=yD-LZMjB5u0JRX36fYgV1HR6WqyJOGKopk5CLR=0rFBEV7g@mail.gmail.com>

On Fri, Sep 15, 2017 at 10:46 AM, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> In case of failure we also need to unlink and free match. I
> sent the following:
>
> http://patchwork.ozlabs.org/patch/813945/

Ah, will take a look.

^ permalink raw reply

* Re: selftests/bpf doesn't compile
From: Alexei Starovoitov @ 2017-09-15 18:14 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Shuah Khan, Daniel Borkmann, Thomas Meyer, linux-kernel,
	linux-kselftest, Networking
In-Reply-To: <e07dabee-6c9c-7842-65b0-990a2ed1a2e9@osg.samsung.com>

On Fri, Sep 15, 2017 at 11:00:31AM -0600, Shuah Khan wrote:
> >> I could add a special target for bpf TARGET_BPF perhaps and exclude it from
> >> the run_test> 
> > I'm not sure what was the motivation to exclude hotplug from default testing,
> 
> These are considered a bit more disruptive and were excluded a while
> back. These take cpus and memory on and off-line. Also require
> root access. So even if they are included in the regular run, these
> won't run.

most of bpf tests require root access as well.

> The first failure due to clang dependency is not a problem. The second one
> in the case of "make kselftest" is the one that requires some work when bpf
> make is run from the main Makefile. A lots of users run tests using the
> kselftest target from the mail Makefile. hence I would like to get this
> working, so it would be easier to run this test on test servers.

'make kselftest' doesn't work for me at all, since I suspect it
assumes in-source build. I always use KBUILD_OUTPUT,
since I build multiple archs with different configs out of the same
source tree, so there is a bigger problem here.

$ make kselftest
make[1]: Entering directory `/data/users/ast/net-next/bld_x64'
make: Entering an unknown directory
make: *** tools/testing/selftests: No such file or directory.  Stop.
make: Leaving an unknown directory
make[1]: *** [kselftest] Error 2
make[1]: Leaving directory `/data/users/ast/net-next/bld_x64'
make: *** [sub-make] Error 2

^ permalink raw reply

* Re: selftests/bpf doesn't compile
From: Daniel Borkmann @ 2017-09-15 18:23 UTC (permalink / raw)
  To: Alexei Starovoitov, Edward Cree
  Cc: Shuah Khan, Thomas Meyer, linux-kernel, linux-kselftest,
	Shuah Khan, Networking
In-Reply-To: <20170915180751.rnmvttf4lnz26qit@ast-mbp>

On 09/15/2017 08:07 PM, Alexei Starovoitov wrote:
> On Fri, Sep 15, 2017 at 05:58:40PM +0100, Edward Cree wrote:
>> On 15/09/17 17:02, Alexei Starovoitov wrote:
>>> On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
>>>> Is bpf test intended to be run in kselftest run? The clang dependency might
>>>> not be met on majority of the systems. Is this a hard dependency??
>>> It is a hard dependency and clang should be present on majority of the systems.
>> I think this is the wrong approach.  Making kselftest hard-require clang doesn't
>>   mean that the bpf tests will be run more often, it means that the rest of the
>>   kselftests will be run less often.  clang is quite big (when I tried to install
>>   it on one of my test servers, I didn't have enough disk space & had to go on a
>>   clear-out of unused packages), and most people aren't interested in the bpf
>>   subsystem specifically; they would rather be able to skip those tests.
>> I feel that as long as they know they are skipping some tests (so e.g. they
>>   won't consider it a sufficient test of a kselftest refactor), that's fine.
>> It's not even as though all of the bpf tests require clang; the (smaller) tests
>>   written directly in raw eBPF instructions could still be run on such a system.
>>   So I think we should attempt to run as much as possible but accept that clang
>>   may not be available and have an option to skip some tests in that case.
>
> imo the value of selftests/bpf is twofold:
> 1. it helps bpf developers avoid regressions
> 2. as part of continuous integration it helps to catch bpf regressions
> that were somehow caused by changes in other parts of the kernel
>
> If a developer didn't bother to satisfy all bpf tests dependencies
> (which includes clang) and ran all tests before sending a patch,
> I don't want to see such patches. It just wastes maintainers time
> to review code and spot bugs that could have been caught by tests.
> Collectively we invested years of work into these tests and
> developers better take advantage of it by running all.

+1

> If a CI server didn't satisfy all bpf test dependencies,
> I don't want such CI setup to be running and reporting results,
> since it will give false sense of test coverage.
> Test failures due to missing dependencies are hard failures.
> We cannot skip them.

+1

> I'd like generic XDP tests to be added to selftests/bpf which
> would mean that the latest iproute2 will become a hard dependency
> and bpf developers and CI host owners would need to upgrade
> their iproute2.
> The tests either pass or fail. Skipping them due to missing
> dependencies is the same as fail and in that sense I don't want
> to change selftests/bpf/Makefile to make it skip clang.

I fully agree that for the BPF selftests it is very desirable
to not only test the verifier with couple of BPF insn snippets,
but to actually load and run programs that more closely resemble
real world programs. For more complex interactions these snippets
are just limited, think of tail calls, testing perf event output
helper, etc, which would all require to write these tests with
restricted C when we add them (unless we want to make writing
these tests a real pain ;) in which case no-one will bother to
write tests at all for them). Mid to long term I would definitely
like to see more programs in BPF selftests (e.g. moved over from
samples/bpf/) to increase the test coverage.

^ permalink raw reply

* Re: [PATCH net] packet: hold bind lock when rebinding to fanout hook
From: Cong Wang @ 2017-09-15 18:33 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Linux Kernel Network Developers, David Miller, nixiaoming
In-Reply-To: <20170914211441.67326-1-willemb@google.com>

On Thu, Sep 14, 2017 at 2:14 PM, Willem de Bruijn <willemb@google.com> wrote:
> Packet socket bind operations must hold the po->bind_lock. This keeps
> po->running consistent with whether the socket is actually on a ptype
> list to receive packets.
>
> fanout_add unbinds a socket and its packet_rcv/tpacket_rcv call, then
> binds the fanout object to receive through packet_rcv_fanout.
>
> Make it hold the po->bind_lock when testing po->running and rebinding.
> Else, it can race with other rebind operations, such as that in
> packet_set_ring from packet_rcv to tpacket_rcv. Concurrent updates
> can result in a socket being added to a fanout group twice, causing
> use-after-free KASAN bug reports, among others.
>
> Reported independently by both trinity and syzkaller.
> Verified that the syzkaller reproducer passes after this patch.
>
> Reported-by: nixioaming <nixiaoming@huawei.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
>  net/packet/af_packet.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index c26172995511..d288f52c53f7 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1684,10 +1684,6 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
>
>         mutex_lock(&fanout_mutex);
>
> -       err = -EINVAL;
> -       if (!po->running)
> -               goto out;
> -
>         err = -EALREADY;
>         if (po->fanout)
>                 goto out;
> @@ -1749,7 +1745,10 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
>                 list_add(&match->list, &fanout_list);
>         }
>         err = -EINVAL;
> -       if (match->type == type &&
> +
> +       spin_lock(&po->bind_lock);
> +       if (po->running &&


As you move the po->running check later after setting po->rollover, I wonder
if po->rollover possibly depends on po>running on other path?


> +           match->type == type &&
>             match->prot_hook.type == po->prot_hook.type &&
>             match->prot_hook.dev == po->prot_hook.dev) {
>                 err = -ENOSPC;
> @@ -1761,6 +1760,13 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
>                         err = 0;
>                 }
>         }
> +       spin_unlock(&po->bind_lock);
> +
> +       if (err && !refcount_read(&match->sk_ref)) {
> +               list_del(&match->list);
> +               kfree(match);
> +       }

This looks correct but still seems odd, it smells you don't use refcnt in an
expected way.

^ permalink raw reply

* Re: selftests/bpf doesn't compile
From: Daniel Borkmann @ 2017-09-15 18:48 UTC (permalink / raw)
  To: Alexei Starovoitov, Edward Cree
  Cc: Shuah Khan, Thomas Meyer, linux-kernel, linux-kselftest,
	Shuah Khan, Networking
In-Reply-To: <59BC1AAC.3040401@iogearbox.net>

On 09/15/2017 08:23 PM, Daniel Borkmann wrote:
> On 09/15/2017 08:07 PM, Alexei Starovoitov wrote:
>> On Fri, Sep 15, 2017 at 05:58:40PM +0100, Edward Cree wrote:
>>> On 15/09/17 17:02, Alexei Starovoitov wrote:
>>>> On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
>>>>> Is bpf test intended to be run in kselftest run? The clang dependency might
>>>>> not be met on majority of the systems. Is this a hard dependency??
>>>> It is a hard dependency and clang should be present on majority of the systems.
>>> I think this is the wrong approach.  Making kselftest hard-require clang doesn't
>>>   mean that the bpf tests will be run more often, it means that the rest of the
>>>   kselftests will be run less often.  clang is quite big (when I tried to install
>>>   it on one of my test servers, I didn't have enough disk space & had to go on a
>>>   clear-out of unused packages), and most people aren't interested in the bpf
>>>   subsystem specifically; they would rather be able to skip those tests.
>>> I feel that as long as they know they are skipping some tests (so e.g. they
>>>   won't consider it a sufficient test of a kselftest refactor), that's fine.
>>> It's not even as though all of the bpf tests require clang; the (smaller) tests
>>>   written directly in raw eBPF instructions could still be run on such a system.
>>>   So I think we should attempt to run as much as possible but accept that clang
>>>   may not be available and have an option to skip some tests in that case.
>>
>> imo the value of selftests/bpf is twofold:
>> 1. it helps bpf developers avoid regressions
>> 2. as part of continuous integration it helps to catch bpf regressions
>> that were somehow caused by changes in other parts of the kernel
>>
>> If a developer didn't bother to satisfy all bpf tests dependencies
>> (which includes clang) and ran all tests before sending a patch,
>> I don't want to see such patches. It just wastes maintainers time
>> to review code and spot bugs that could have been caught by tests.
>> Collectively we invested years of work into these tests and
>> developers better take advantage of it by running all.
>
> +1
>
>> If a CI server didn't satisfy all bpf test dependencies,
>> I don't want such CI setup to be running and reporting results,
>> since it will give false sense of test coverage.
>> Test failures due to missing dependencies are hard failures.
>> We cannot skip them.
>
> +1

Btw, on that note, the folks from zero-day bot run the BPF kselftests
for a while now just fine and they do run them together with clang,
so they have the full, proper coverage how it should be. It's not
how it used to be in the early days, you can just go and install
llvm/clang package on all the major distros today and you get the
bpf target by default enabled already.

>> I'd like generic XDP tests to be added to selftests/bpf which
>> would mean that the latest iproute2 will become a hard dependency
>> and bpf developers and CI host owners would need to upgrade
>> their iproute2.
>> The tests either pass or fail. Skipping them due to missing
>> dependencies is the same as fail and in that sense I don't want
>> to change selftests/bpf/Makefile to make it skip clang.
>
> I fully agree that for the BPF selftests it is very desirable
> to not only test the verifier with couple of BPF insn snippets,
> but to actually load and run programs that more closely resemble
> real world programs. For more complex interactions these snippets
> are just limited, think of tail calls, testing perf event output
> helper, etc, which would all require to write these tests with
> restricted C when we add them (unless we want to make writing
> these tests a real pain ;) in which case no-one will bother to
> write tests at all for them). Mid to long term I would definitely
> like to see more programs in BPF selftests (e.g. moved over from
> samples/bpf/) to increase the test coverage.

^ permalink raw reply

* Re: [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c
From: Oleksandr Natalenko @ 2017-09-15 19:04 UTC (permalink / raw)
  To: Neal Cardwell
  Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Netdev,
	Yuchung Cheng
In-Reply-To: <CADVnQymkqE+mt4OwtkL1ddpRbqJYPH8CJ0JhNn6ML9YPJdCJxA@mail.gmail.com>

Hello.

With net.ipv4.tcp_fack set to 0 the warning still appears:

===
» sysctl net.ipv4.tcp_fack     
net.ipv4.tcp_fack = 0

» LC_TIME=C dmesg -T | grep WARNING
[Fri Sep 15 20:40:30 2017] WARNING: CPU: 1 PID: 711 at net/ipv4/tcp_input.c:
2826 tcp_fastretrans_alert+0x7c8/0x990
[Fri Sep 15 20:40:30 2017] WARNING: CPU: 0 PID: 711 at net/ipv4/tcp_input.c:
2826 tcp_fastretrans_alert+0x7c8/0x990
[Fri Sep 15 20:48:37 2017] WARNING: CPU: 1 PID: 711 at net/ipv4/tcp_input.c:
2826 tcp_fastretrans_alert+0x7c8/0x990
[Fri Sep 15 20:48:55 2017] WARNING: CPU: 0 PID: 711 at net/ipv4/tcp_input.c:
2826 tcp_fastretrans_alert+0x7c8/0x990

» ps -up 711
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root       711  4.3  0.0      0     0 ?        S    18:12   7:23 [irq/123-
enp3s0]
===

Any suggestions?

On pátek 15. září 2017 16:03:00 CEST Neal Cardwell wrote:
> Thanks for testing that. That is a very useful data point.
> 
> I was able to cook up a packetdrill test that could put the connection
> in CA_Disorder with retransmitted packets out, but not in CA_Open. So
> we do not yet have a test case to reproduce this.
> 
> We do not see this warning on our fleet at Google. One significant
> difference I see between our environment and yours is that it seems
> you run with FACK enabled:
> 
>   net.ipv4.tcp_fack = 1
> 
> Note that FACK was disabled by default (since it was replaced by RACK)
> between kernel v4.10 and v4.11. And this is exactly the time when this
> bug started manifesting itself for you and some others, but not our
> fleet. So my new working hypothesis would be that this warning is due
> to a behavior that only shows up in kernels >=4.11 when FACK is
> enabled.
> 
> Would you be able to disable FACK ("sysctl net.ipv4.tcp_fack=0" at
> boot, or net.ipv4.tcp_fack=0 in /etc/sysctl.conf, or equivalent),
> reboot, and test the kernel for a few days to see if the warning still
> pops up?
> 
> thanks,
> neal
> 
> [ps: apologies for the previous, mis-formatted post...]

^ permalink raw reply

* Re: Regression in throughput between kvm guests over virtual bridge
From: Matthew Rosato @ 2017-09-15 19:19 UTC (permalink / raw)
  To: Jason Wang, netdev; +Cc: davem, mst
In-Reply-To: <15abafa1-6d58-cd85-668a-bf361a296f52@redhat.com>

> It looks like vhost is slowed down for some reason which leads to more
> idle time on 4.13+VHOST_RX_BATCH=1. Appreciated if you can collect the
> perf.diff on host, one for rx and one for tx.
> 

perf data below for the associated vhost threads, baseline=4.12,
delta1=4.13, delta2=4.13+VHOST_RX_BATCH=1

Client vhost:

60.12%  -11.11%  -12.34%  [kernel.vmlinux]   [k] raw_copy_from_user
13.76%   -1.28%   -0.74%  [kernel.vmlinux]   [k] get_page_from_freelist
 2.00%   +3.69%   +3.54%  [kernel.vmlinux]   [k] __wake_up_sync_key
 1.19%   +0.60%   +0.66%  [kernel.vmlinux]   [k] __alloc_pages_nodemask
 1.12%   +0.76%   +0.86%  [kernel.vmlinux]   [k] copy_page_from_iter
 1.09%   +0.28%   +0.35%  [vhost]            [k] vhost_get_vq_desc
 1.07%   +0.31%   +0.26%  [kernel.vmlinux]   [k] alloc_skb_with_frags
 0.94%   +0.42%   +0.65%  [kernel.vmlinux]   [k] alloc_pages_current
 0.91%   -0.19%   -0.18%  [kernel.vmlinux]   [k] memcpy
 0.88%   +0.26%   +0.30%  [kernel.vmlinux]   [k] __next_zones_zonelist
 0.85%   +0.05%   +0.12%  [kernel.vmlinux]   [k] iov_iter_advance
 0.79%   +0.09%   +0.19%  [vhost]            [k] __vhost_add_used_n
 0.74%                    [kernel.vmlinux]   [k] get_task_policy.part.7
 0.74%   -0.01%   -0.05%  [kernel.vmlinux]   [k] tun_net_xmit
 0.60%   +0.17%   +0.33%  [kernel.vmlinux]   [k] policy_nodemask
 0.58%   -0.15%   -0.12%  [ebtables]         [k] ebt_do_table
 0.52%   -0.25%   -0.22%  [kernel.vmlinux]   [k] __alloc_skb
   ...
 0.42%   +0.58%   +0.59%  [kernel.vmlinux]   [k] eventfd_signal
   ...
 0.32%   +0.96%   +0.93%  [kernel.vmlinux]   [k] finish_task_switch
   ...
         +1.50%   +1.16%  [kernel.vmlinux]   [k] get_task_policy.part.9
         +0.40%   +0.42%  [kernel.vmlinux]   [k] __skb_get_hash_symmetr
         +0.39%   +0.40%  [kernel.vmlinux]   [k] _copy_from_iter_full
         +0.24%   +0.23%  [vhost_net]        [k] vhost_net_buf_peek

Server vhost:

61.93%  -10.72%  -10.91%  [kernel.vmlinux]   [k] raw_copy_to_user
 9.25%   +0.47%   +0.86%  [kernel.vmlinux]   [k] free_hot_cold_page
 5.16%   +1.41%   +1.57%  [vhost]            [k] vhost_get_vq_desc
 5.12%   -3.81%   -3.78%  [kernel.vmlinux]   [k] skb_release_data
 3.30%   +0.42%   +0.55%  [kernel.vmlinux]   [k] raw_copy_from_user
 1.29%   +2.20%   +2.28%  [kernel.vmlinux]   [k] copy_page_to_iter
 1.24%   +1.65%   +0.45%  [vhost_net]        [k] handle_rx
 1.08%   +3.03%   +2.85%  [kernel.vmlinux]   [k] __wake_up_sync_key
 0.96%   +0.70%   +1.10%  [vhost]            [k] translate_desc
 0.69%   -0.20%   -0.22%  [kernel.vmlinux]   [k] tun_do_read.part.10
 0.69%                    [kernel.vmlinux]   [k] tun_peek_len
 0.67%   +0.75%   +0.78%  [kernel.vmlinux]   [k] eventfd_signal
 0.52%   +0.96%   +0.98%  [kernel.vmlinux]   [k] finish_task_switch
 0.50%   +0.05%   +0.09%  [vhost]            [k] vhost_add_used_n
   ...
         +0.63%   +0.58%  [vhost_net]        [k] vhost_net_buf_peek
         +0.32%   +0.32%  [kernel.vmlinux]   [k] _copy_to_iter
         +0.19%   +0.19%  [kernel.vmlinux]   [k] __skb_get_hash_symmetr
         +0.11%   +0.21%  [vhost]            [k] vhost_umem_interval_tr

^ permalink raw reply

* Re: RTL8192EE PCIe Wireless Network Adapter crashed with linux-4.13
From: Larry Finger @ 2017-09-15 19:21 UTC (permalink / raw)
  To: Zwindl
  Cc: linux-wireless@vger.kernel.org, chaoming_li@realsil.com.cn,
	kvalo@codeaurora.org, pkshih@realtek.com, johannes.berg@intel.com,
	gregkh@linuxfoundation.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <x1duse6v9TTP8Ta7rsTLlZ8DK0Ej7qq9iapef18K0eYSodmCBmfI_mX3c-3ocbPiGzEgV9weOh25dKWSbU5nIetYkfmmUxaxRpUdYFffRZA=@protonmail.com>

On 09/15/2017 12:12 PM, Zwindl wrote:
> Thanks for your patient and advice, I'll keep that in mind.
> I do want help, and I got 1 day to build the system, but I can't recall how to 
> compile it, The last time I compile kernel is 2013, so, maybe I'll ask you so 
> many stupid questions during the build time.
> ZWindL

Building a new kernel is not difficult. In an average week, I make at least 10 
new kernels. Many of them are done on slow machines that take many hours. At 
least, your i5 CPU should do it in less that one hour.

Step 1: Download the kernel sources using

git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

If your system complains that the git command is unknown, then you will need to 
install it with your package manager (pacman?).

Step 2: "cd  linux" and copy the latest /boot/config-..... to the linux source 
directory as ".config". Edit .config, find the line that says
"# CONFIG_LOCALVERSION_AUTO is not set", and change the line to read 
"CONFIG_LOCALVERSION_AUTO=y".

Step 3: Build and install the latest version using

make -j9
sudo make modules_install install

You will need to answer some configuration questions at the start of the first 
make line. Answer with the default value, i.e. just use an ENTER. When the build 
is complete, reboot. Grub should show an entry for something like 
v4.13-12084-ged43e4d190d0. The numbers after the 4.13 will likely be different, 
but the form will match. Check that the new kernel still has the fault. If not, 
it has been fixed and we do not need to find it.

It the problem is still in the latest version of the kernel, then we start the 
bisection with the following:

git bisect start
git bisect bad v4.13
git bisect good v4.12

At this point, git will report the number of revisions to test, the likely 
number of tries, and the SHA hash for the new kernel. Record the first 7 digits 
of the hash, and repeat the make commands above. After the build is complete, 
reboot into the kernel with the hash in the version name and test. Then enter 
the command "git bisect xxx", where xxx is good or bad depending on the test. A 
new trial will be generated by bisecting the appropriate half of the commits. 
Record its hash and redo the build. Repeat until git tells you the bad commit.

This process will generate a number of kernels that will take quite a bit of 
disk space. If you run short, you can delete kernels that have already been 
tested from /boot. You should also delete the corresponding modules from 
/lib/modules.

Good luck,

Larry

^ permalink raw reply

* [PATCH] Staging: irda: Don't use assignment inside if statement
From: Srishti Sharma @ 2017-09-15 19:33 UTC (permalink / raw)
  To: samuel
  Cc: gregkh, netdev, devel, linux-kernel, outreachy-kernel,
	Srishti Sharma

Write assignment statement outside the if statement. Done using
the following semantic patch by coccinelle.

@@
identifier E;
expression F;
statement S;
@@

-if((E = F))
+E = F
+if(E)
  S

Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
---
 drivers/staging/irda/drivers/irda-usb.c | 4 ++--
 drivers/staging/irda/drivers/mcs7780.c  | 9 ++++++---
 drivers/staging/irda/net/irqueue.c      | 3 ++-
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
index 723e49b..82bfc05 100644
--- a/drivers/staging/irda/drivers/irda-usb.c
+++ b/drivers/staging/irda/drivers/irda-usb.c
@@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct irda_usb_cb *self)
 	urb->transfer_flags = 0;
 
 	/* Irq disabled -> GFP_ATOMIC */
-	if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
+	ret = usb_submit_urb(urb, GFP_ATOMIC);
+	if (ret)
 		net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
-	}
 }
 
 /*------------------------------------------------------------------*/
diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
index c3f0b25..2b674d5 100644
--- a/drivers/staging/irda/drivers/mcs7780.c
+++ b/drivers/staging/irda/drivers/mcs7780.c
@@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
 	if (mcs->new_speed <= 115200) {
 		rval &= ~MCS_FIR;
 
-		if ((rst = (mcs->speed > 115200)))
+		rst = (mcs->speed > 115200);
+		if (rst)
 			mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
 
 	} else if (mcs->new_speed <= 1152000) {
 		rval &= ~MCS_FIR;
 
-		if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
+		rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
+		if (rst)
 			mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
 
 	} else {
 		rval |= MCS_FIR;
 
-		if ((rst = (mcs->speed != 4000000)))
+		rst = (mcs->speed != 4000000);
+		if (rst)
 			mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
 
 	}
diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
index 160dc89..5aab072 100644
--- a/drivers/staging/irda/net/irqueue.c
+++ b/drivers/staging/irda/net/irqueue.c
@@ -217,7 +217,8 @@ static __u32 hash( const char* name)
 
 	while(*name) {
 		h = (h<<4) + *name++;
-		if ((g = (h & 0xf0000000)))
+		g = (h & 0xf0000000);
+		if (g)
 			h ^=g>>24;
 		h &=~g;
 	}
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] Staging: irda: Don't use assignment inside if statement
From: Srishti Sharma @ 2017-09-15 19:36 UTC (permalink / raw)
  To: samuel
  Cc: devel, Greg KH, Linux kernel mailing list, outreachy-kernel,
	Srishti Sharma, netdev
In-Reply-To: <1505504036-21807-1-git-send-email-srishtishar@gmail.com>

On Sat, Sep 16, 2017 at 1:03 AM, Srishti Sharma <srishtishar@gmail.com> wrote:
> Write assignment statement outside the if statement. Done using
> the following semantic patch by coccinelle.
>
> @@
> identifier E;
> expression F;
> statement S;
> @@
>
> -if((E = F))
> +E = F
> +if(E)
>   S
>
> Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
> ---
>  drivers/staging/irda/drivers/irda-usb.c | 4 ++--
>  drivers/staging/irda/drivers/mcs7780.c  | 9 ++++++---
>  drivers/staging/irda/net/irqueue.c      | 3 ++-
>  3 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
> index 723e49b..82bfc05 100644
> --- a/drivers/staging/irda/drivers/irda-usb.c
> +++ b/drivers/staging/irda/drivers/irda-usb.c
> @@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct irda_usb_cb *self)
>         urb->transfer_flags = 0;
>
>         /* Irq disabled -> GFP_ATOMIC */
> -       if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
> +       ret = usb_submit_urb(urb, GFP_ATOMIC);
> +       if (ret)
>                 net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
> -       }
>  }
>
>  /*------------------------------------------------------------------*/
> diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
> index c3f0b25..2b674d5 100644
> --- a/drivers/staging/irda/drivers/mcs7780.c
> +++ b/drivers/staging/irda/drivers/mcs7780.c
> @@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
>         if (mcs->new_speed <= 115200) {
>                 rval &= ~MCS_FIR;
>
> -               if ((rst = (mcs->speed > 115200)))
> +               rst = (mcs->speed > 115200);
> +               if (rst)
>                         mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
>
>         } else if (mcs->new_speed <= 1152000) {
>                 rval &= ~MCS_FIR;
>
> -               if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
> +               rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
> +               if (rst)
>                         mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>
>         } else {
>                 rval |= MCS_FIR;
>
> -               if ((rst = (mcs->speed != 4000000)))
> +               rst = (mcs->speed != 4000000);
> +               if (rst)
>                         mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>
>         }
> diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
> index 160dc89..5aab072 100644
> --- a/drivers/staging/irda/net/irqueue.c
> +++ b/drivers/staging/irda/net/irqueue.c
> @@ -217,7 +217,8 @@ static __u32 hash( const char* name)
>
>         while(*name) {
>                 h = (h<<4) + *name++;
> -               if ((g = (h & 0xf0000000)))
> +               g = (h & 0xf0000000);
> +               if (g)
>                         h ^=g>>24;
>                 h &=~g;
>         }
> --
> 2.7.4
>

There is a mistake here please ignore this patch , I'll send a v2 , I
forgot to put a semicolon in the semantic patch .

Regards,
Srishti

^ permalink raw reply

* Re: [Outreachy kernel] [PATCH] Staging: irda: Don't use assignment inside if statement
From: Julia Lawall @ 2017-09-15 19:38 UTC (permalink / raw)
  To: Srishti Sharma
  Cc: samuel, gregkh, netdev, devel, linux-kernel, outreachy-kernel
In-Reply-To: <1505504036-21807-1-git-send-email-srishtishar@gmail.com>



On Sat, 16 Sep 2017, Srishti Sharma wrote:

> Write assignment statement outside the if statement. Done using
> the following semantic patch by coccinelle.
>
> @@
> identifier E;
> expression F;
> statement S;
> @@
>
> -if((E = F))
> +E = F

The line above would need to end in a ;

This ends up with a lot of assignments with () around the right hand side.
Maybe you could make a series removing them afterwards.

julia

> +if(E)
>   S
>
> Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
> ---
>  drivers/staging/irda/drivers/irda-usb.c | 4 ++--
>  drivers/staging/irda/drivers/mcs7780.c  | 9 ++++++---
>  drivers/staging/irda/net/irqueue.c      | 3 ++-
>  3 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
> index 723e49b..82bfc05 100644
> --- a/drivers/staging/irda/drivers/irda-usb.c
> +++ b/drivers/staging/irda/drivers/irda-usb.c
> @@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct irda_usb_cb *self)
>  	urb->transfer_flags = 0;
>
>  	/* Irq disabled -> GFP_ATOMIC */
> -	if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
> +	ret = usb_submit_urb(urb, GFP_ATOMIC);
> +	if (ret)
>  		net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
> -	}
>  }
>
>  /*------------------------------------------------------------------*/
> diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
> index c3f0b25..2b674d5 100644
> --- a/drivers/staging/irda/drivers/mcs7780.c
> +++ b/drivers/staging/irda/drivers/mcs7780.c
> @@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
>  	if (mcs->new_speed <= 115200) {
>  		rval &= ~MCS_FIR;
>
> -		if ((rst = (mcs->speed > 115200)))
> +		rst = (mcs->speed > 115200);
> +		if (rst)
>  			mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
>
>  	} else if (mcs->new_speed <= 1152000) {
>  		rval &= ~MCS_FIR;
>
> -		if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
> +		rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
> +		if (rst)
>  			mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>
>  	} else {
>  		rval |= MCS_FIR;
>
> -		if ((rst = (mcs->speed != 4000000)))
> +		rst = (mcs->speed != 4000000);
> +		if (rst)
>  			mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>
>  	}
> diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
> index 160dc89..5aab072 100644
> --- a/drivers/staging/irda/net/irqueue.c
> +++ b/drivers/staging/irda/net/irqueue.c
> @@ -217,7 +217,8 @@ static __u32 hash( const char* name)
>
>  	while(*name) {
>  		h = (h<<4) + *name++;
> -		if ((g = (h & 0xf0000000)))
> +		g = (h & 0xf0000000);
> +		if (g)
>  			h ^=g>>24;
>  		h &=~g;
>  	}
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1505504036-21807-1-git-send-email-srishtishar%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

^ permalink raw reply

* Re: [Outreachy kernel] [PATCH] Staging: irda: Don't use assignment inside if statement
From: Srishti Sharma @ 2017-09-15 19:42 UTC (permalink / raw)
  To: Julia Lawall
  Cc: devel, samuel, Greg KH, Linux kernel mailing list,
	outreachy-kernel, netdev
In-Reply-To: <alpine.DEB.2.20.1709152136200.2533@hadrien>

On Sat, Sep 16, 2017 at 1:08 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Sat, 16 Sep 2017, Srishti Sharma wrote:
>
>> Write assignment statement outside the if statement. Done using
>> the following semantic patch by coccinelle.
>>
>> @@
>> identifier E;
>> expression F;
>> statement S;
>> @@
>>
>> -if((E = F))
>> +E = F
>
> The line above would need to end in a ;
Yes, I'll send a v2 after correcting it .
>
> This ends up with a lot of assignments with () around the right hand side.
> Maybe you could make a series removing them afterwards.
Yes, I'll do that . Thanks .

Regards,
Srishti
>
> julia
>
>> +if(E)
>>   S
>>
>> Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
>> ---
>>  drivers/staging/irda/drivers/irda-usb.c | 4 ++--
>>  drivers/staging/irda/drivers/mcs7780.c  | 9 ++++++---
>>  drivers/staging/irda/net/irqueue.c      | 3 ++-
>>  3 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
>> index 723e49b..82bfc05 100644
>> --- a/drivers/staging/irda/drivers/irda-usb.c
>> +++ b/drivers/staging/irda/drivers/irda-usb.c
>> @@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct irda_usb_cb *self)
>>       urb->transfer_flags = 0;
>>
>>       /* Irq disabled -> GFP_ATOMIC */
>> -     if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
>> +     ret = usb_submit_urb(urb, GFP_ATOMIC);
>> +     if (ret)
>>               net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
>> -     }
>>  }
>>
>>  /*------------------------------------------------------------------*/
>> diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
>> index c3f0b25..2b674d5 100644
>> --- a/drivers/staging/irda/drivers/mcs7780.c
>> +++ b/drivers/staging/irda/drivers/mcs7780.c
>> @@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
>>       if (mcs->new_speed <= 115200) {
>>               rval &= ~MCS_FIR;
>>
>> -             if ((rst = (mcs->speed > 115200)))
>> +             rst = (mcs->speed > 115200);
>> +             if (rst)
>>                       mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
>>
>>       } else if (mcs->new_speed <= 1152000) {
>>               rval &= ~MCS_FIR;
>>
>> -             if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
>> +             rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
>> +             if (rst)
>>                       mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>>
>>       } else {
>>               rval |= MCS_FIR;
>>
>> -             if ((rst = (mcs->speed != 4000000)))
>> +             rst = (mcs->speed != 4000000);
>> +             if (rst)
>>                       mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>>
>>       }
>> diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
>> index 160dc89..5aab072 100644
>> --- a/drivers/staging/irda/net/irqueue.c
>> +++ b/drivers/staging/irda/net/irqueue.c
>> @@ -217,7 +217,8 @@ static __u32 hash( const char* name)
>>
>>       while(*name) {
>>               h = (h<<4) + *name++;
>> -             if ((g = (h & 0xf0000000)))
>> +             g = (h & 0xf0000000);
>> +             if (g)
>>                       h ^=g>>24;
>>               h &=~g;
>>       }
>> --
>> 2.7.4
>>
>> --
>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1505504036-21807-1-git-send-email-srishtishar%40gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>

^ permalink raw reply

* [PATCH v2] Staging: irda: Don't use assignment inside if statement
From: Srishti Sharma @ 2017-09-15 19:44 UTC (permalink / raw)
  To: samuel
  Cc: gregkh, netdev, devel, linux-kernel, outreachy-kernel,
	Srishti Sharma

Write assignment statement outside the if statement. Done using
the following semantic patch by coccinelle.

@@
identifier E;
expression F;
statement S;
@@

-if((E = F))
+E = F;
+if(E)
  S

Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
---
Changes in v2:
 -Semicolon was missing in one of the statements of the 
  semantic patch
 
 drivers/staging/irda/drivers/irda-usb.c | 4 ++--
 drivers/staging/irda/drivers/mcs7780.c  | 9 ++++++---
 drivers/staging/irda/net/irqueue.c      | 3 ++-
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
index 723e49b..82bfc05 100644
--- a/drivers/staging/irda/drivers/irda-usb.c
+++ b/drivers/staging/irda/drivers/irda-usb.c
@@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct irda_usb_cb *self)
 	urb->transfer_flags = 0;
 
 	/* Irq disabled -> GFP_ATOMIC */
-	if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
+	ret = usb_submit_urb(urb, GFP_ATOMIC);
+	if (ret)
 		net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
-	}
 }
 
 /*------------------------------------------------------------------*/
diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
index c3f0b25..2b674d5 100644
--- a/drivers/staging/irda/drivers/mcs7780.c
+++ b/drivers/staging/irda/drivers/mcs7780.c
@@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
 	if (mcs->new_speed <= 115200) {
 		rval &= ~MCS_FIR;
 
-		if ((rst = (mcs->speed > 115200)))
+		rst = (mcs->speed > 115200);
+		if (rst)
 			mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
 
 	} else if (mcs->new_speed <= 1152000) {
 		rval &= ~MCS_FIR;
 
-		if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
+		rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
+		if (rst)
 			mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
 
 	} else {
 		rval |= MCS_FIR;
 
-		if ((rst = (mcs->speed != 4000000)))
+		rst = (mcs->speed != 4000000);
+		if (rst)
 			mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
 
 	}
diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
index 160dc89..5aab072 100644
--- a/drivers/staging/irda/net/irqueue.c
+++ b/drivers/staging/irda/net/irqueue.c
@@ -217,7 +217,8 @@ static __u32 hash( const char* name)
 
 	while(*name) {
 		h = (h<<4) + *name++;
-		if ((g = (h & 0xf0000000)))
+		g = (h & 0xf0000000);
+		if (g)
 			h ^=g>>24;
 		h &=~g;
 	}
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next v1] bridge: also trigger RTM_NEWLINK when interface is released from bridge
From: Vincent Bernat @ 2017-09-15 19:38 UTC (permalink / raw)
  To: Stephen Hemminger, David S. Miller, bridge, netdev; +Cc: Vincent Bernat

Currently, when an interface is released from a bridge, we get a
RTM_DELLINK event through netlink:

Deleted 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
    link/ether 6e:23:c2:54:3a:b3

Userspace has to interpret that as a removal from the bridge, not as a
complete removal of the interface. When an bridged interface is
completely removed, we get two events:

Deleted 2: dummy0: <BROADCAST,NOARP> mtu 1500 master bridge0 state DOWN
    link/ether 6e:23:c2:54:3a:b3
Deleted 2: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN group default
    link/ether 6e:23:c2:54:3a:b3 brd ff:ff:ff:ff:ff:ff

In constrast, when an interface is released from a bond, we get a
RTM_NEWLINK with only the new characteristics (no master):

3: dummy1: <BROADCAST,NOARP,SLAVE,UP,LOWER_UP> mtu 1500 qdisc noqueue master bond0 state UNKNOWN group default
    link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff
3: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
    link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff
4: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default
    link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff
3: dummy1: <BROADCAST,NOARP> mtu 1500 qdisc noqueue state DOWN group default
    link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff
3: dummy1: <BROADCAST,NOARP> mtu 1500 qdisc noqueue state DOWN group default
    link/ether ca:c8:7b:66:f8:25 brd ff:ff:ff:ff:ff:ff
4: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default
    link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff

Userland may be confused by the fact we say a link is deleted while
its characteristics are only modified. A first solution would have
been to turn the RTM_DELLINK event in del_nbp() into a RTM_NEWLINK
event. However, maybe some piece of userland is relying on this
RTM_DELLINK to detect when a bridged interface is released. Instead,
we also emit a RTM_NEWLINK event once the interface is
released (without master info).

Deleted 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
    link/ether 8a:bb:e7:94:b1:f8
2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
    link/ether 8a:bb:e7:94:b1:f8 brd ff:ff:ff:ff:ff:ff

Signed-off-by: Vincent Bernat <vincent@bernat.im>
---
 net/bridge/br_if.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index f3aef22931ab..636e0a842f8a 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -288,6 +288,8 @@ static void del_nbp(struct net_bridge_port *p)
 
 	dev->priv_flags &= ~IFF_BRIDGE_PORT;
 
+	rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_MASTER, GFP_KERNEL);
+
 	netdev_rx_handler_unregister(dev);
 
 	br_multicast_del_port(p);
-- 
2.14.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox