Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net v2] ipv6: fix rtnl locking in setsockopt for anycast and multicast
From: Cong Wang @ 2014-09-05 19:34 UTC (permalink / raw)
  To: David Miller
  Cc: Hannes Frederic Sowa, Sabrina Dubroca, Tommi Rantala,
	Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, netdev, linux-kernel@vger.kernel.org, trinity,
	Dave Jones
In-Reply-To: <20140905.122539.670916286627732128.davem@davemloft.net>

On Fri, Sep 5, 2014 at 12:25 PM, David Miller <davem@davemloft.net> wrote:
>
> I thought the retention of RCU locking was reasonable, and that your
> feedback was something I disagreed with.
>
> This is different from ignoring your feedback.

The point is we don't need to backport the patch that far, as I already
mentioned, up to  commit c15b1ccadb323ea
("ipv6: move DAD and addrconf_verify processing to workqueue"),
simply because no one reports any bug.

So we don't have to adjust the patch just for stable in this case.

^ permalink raw reply

* Re: [PATCH net v2] ipv6: fix rtnl locking in setsockopt for anycast and multicast
From: David Miller @ 2014-09-05 19:25 UTC (permalink / raw)
  To: cwang
  Cc: hannes, sd, tt.rantala, kuznet, jmorris, yoshfuji, kaber, netdev,
	linux-kernel, trinity, davej
In-Reply-To: <CAHA+R7NSu=wjGsmLapjM4Mbk_oeZ07P+yN0b982otvjoW2jodA@mail.gmail.com>

From: Cong Wang <cwang@twopensource.com>
Date: Fri, 5 Sep 2014 12:23:37 -0700

> On Fri, Sep 5, 2014 at 12:12 PM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
>>
>> What games are you playing? You know how patches are processed by David
>> and I even let him the choice by pointing out a problem in your patch so
>> that you could an update and send v2.
> 
> I assume David goes over all the discussion before applying any patch.
> So in the original discussion, obviously I disagree with your point on RCU,
> and sent my own version.

I thought the retention of RCU locking was reasonable, and that your
feedback was something I disagreed with.

This is different from ignoring your feedback.

^ permalink raw reply

* Re: [PATCH net v2] ipv6: fix rtnl locking in setsockopt for anycast and multicast
From: Cong Wang @ 2014-09-05 19:23 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: David Miller, Sabrina Dubroca, Tommi Rantala, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev,
	linux-kernel@vger.kernel.org, trinity, Dave Jones
In-Reply-To: <1409944376.5306.9.camel@localhost>

On Fri, Sep 5, 2014 at 12:12 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
>
> What games are you playing? You know how patches are processed by David
> and I even let him the choice by pointing out a problem in your patch so
> that you could an update and send v2.

I assume David goes over all the discussion before applying any patch.
So in the original discussion, obviously I disagree with your point on RCU,
and sent my own version.

I do understand David may miss something due to massive emails in
this list, but you make no point here nor I understand your broken English
"let him the choice...".

>
> I really feel miffed about your behavior!
>

I SEE. Please do redirect my email to your /dev/null. Thanks a lot!

> Anyway, I saw the hunk adding the return -ENODEV and didn't see any
> problems with it. Sure it might be better if it would gone into a
> separate patch. Can you elaborate what problems you see?
>

Yes, it should be not in this patch at the very very least.

^ permalink raw reply

* Re: [PATCH net v2] ipv6: fix rtnl locking in setsockopt for anycast and multicast
From: David Miller @ 2014-09-05 19:21 UTC (permalink / raw)
  To: cwang
  Cc: sd, hannes, tt.rantala, kuznet, jmorris, yoshfuji, kaber, netdev,
	linux-kernel, trinity, davej
In-Reply-To: <CAHA+R7M1qd5g1ta=YFqefgXDf1EfF2_VsJGUvH6Dd+ttnHc_VQ@mail.gmail.com>

From: Cong Wang <cwang@twopensource.com>
Date: Fri, 5 Sep 2014 11:58:31 -0700

> On Fri, Sep 5, 2014 at 11:53 AM, David Miller <davem@davemloft.net> wrote:
>> From: Sabrina Dubroca <sd@queasysnail.net>
>> Date: Tue, 2 Sep 2014 10:29:29 +0200
>>
>>> Calling setsockopt with IPV6_JOIN_ANYCAST or IPV6_LEAVE_ANYCAST
>>> triggers the assertion in addrconf_join_solict()/addrconf_leave_solict()
>>>
>>> ipv6_sock_ac_join(), ipv6_sock_ac_drop(), ipv6_sock_ac_close() need to
>>> take RTNL before calling ipv6_dev_ac_inc/dec. Same thing with
>>> ipv6_sock_mc_join(), ipv6_sock_mc_drop(), ipv6_sock_mc_close() before
>>> calling ipv6_dev_mc_inc/dec.
>>>
>>> This patch moves ASSERT_RTNL() up a level in the call stack.
>>>
>>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>>> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
>>> Reported-by: Tommi Rantala <tt.rantala@gmail.com>
>>
>> Applied and queued up for -stable, thanks.
> 
> I believe you applied a wrong version, at least the following
> is not correct:
> 
> +       if (!dev)
> +               return -ENODEV;
> 
> Sabrina took that from my draft patch, but they all don't
> realize this is wrong.
> 
> (I did provide a correct version which is just ignored by you.)

Not ignored, but rather it was hard to interpret the situation due to
poor communication in the feedback emails.

The onus is on you guys to communicate things precisely so that I
understand what patch is in what state.

^ permalink raw reply

* Re: [PATCH net-next v3 0/2] r8152: random MAC address
From: David Miller @ 2014-09-05 19:18 UTC (permalink / raw)
  To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-32-Taiwan-albertk@realtek.com>

From: Hayes Wang <hayeswang@realtek.com>
Date: Thu, 4 Sep 2014 16:15:40 +0800

> If the interface has invalid MAC address, it couldn't
> be used. In order to let it work normally, give a
> random one.
> 
> v3:
>   Remove
> 	ether_addr_copy(dev->perm_addr, dev->dev_addr);
> 
> v2:
>   Use "%pM" format specifier for printing a MAC address.

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next v4 0/3] enic: Add support for rx_copybreak
From: David Miller @ 2014-09-05 19:13 UTC (permalink / raw)
  To: _govind; +Cc: netdev, ssujith, ben
In-Reply-To: <1409694441-3843-1-git-send-email-_govind@gmx.com>

From: Govindarajulu Varadarajan <_govind@gmx.com>
Date: Wed,  3 Sep 2014 03:17:18 +0530

> The following series implements rx_copybreak.

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH net v2] ipv6: fix rtnl locking in setsockopt for anycast and multicast
From: Hannes Frederic Sowa @ 2014-09-05 19:12 UTC (permalink / raw)
  To: Cong Wang
  Cc: David Miller, Sabrina Dubroca, Tommi Rantala, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev,
	linux-kernel@vger.kernel.org, trinity, Dave Jones
In-Reply-To: <CAHA+R7M1qd5g1ta=YFqefgXDf1EfF2_VsJGUvH6Dd+ttnHc_VQ@mail.gmail.com>

On Fr, 2014-09-05 at 11:58 -0700, Cong Wang wrote:
> On Fri, Sep 5, 2014 at 11:53 AM, David Miller <davem@davemloft.net> wrote:
> > From: Sabrina Dubroca <sd@queasysnail.net>
> > Date: Tue, 2 Sep 2014 10:29:29 +0200
> >
> >> Calling setsockopt with IPV6_JOIN_ANYCAST or IPV6_LEAVE_ANYCAST
> >> triggers the assertion in addrconf_join_solict()/addrconf_leave_solict()
> >>
> >> ipv6_sock_ac_join(), ipv6_sock_ac_drop(), ipv6_sock_ac_close() need to
> >> take RTNL before calling ipv6_dev_ac_inc/dec. Same thing with
> >> ipv6_sock_mc_join(), ipv6_sock_mc_drop(), ipv6_sock_mc_close() before
> >> calling ipv6_dev_mc_inc/dec.
> >>
> >> This patch moves ASSERT_RTNL() up a level in the call stack.
> >>
> >> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> >> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> >> Reported-by: Tommi Rantala <tt.rantala@gmail.com>
> >
> > Applied and queued up for -stable, thanks.
> 
> I believe you applied a wrong version, at least the following
> is not correct:
> 
> +       if (!dev)
> +               return -ENODEV;
> 
> Sabrina took that from my draft patch, but they all don't
> realize this is wrong.
> 
> (I did provide a correct version which is just ignored by you.)

What games are you playing? You know how patches are processed by David
and I even let him the choice by pointing out a problem in your patch so
that you could an update and send v2.

I really feel miffed about your behavior!

Anyway, I saw the hunk adding the return -ENODEV and didn't see any
problems with it. Sure it might be better if it would gone into a
separate patch. Can you elaborate what problems you see?

Thanks,
Hannes

^ permalink raw reply

* Re: [PATCH net-next] dev_ioctl: remove dev_load() CAP_SYS_MODULE message
From: David Miller @ 2014-09-05 19:05 UTC (permalink / raw)
  To: dborkman; +Cc: marcel, stephen, netdev, segoon
In-Reply-To: <1409693405-24671-1-git-send-email-dborkman@redhat.com>

From: Daniel Borkmann <dborkman@redhat.com>
Date: Tue,  2 Sep 2014 23:30:05 +0200

> Marcel reported to see the following message when autoloading
> is being triggered when adding nlmon device:
> 
>   Loading kernel module for a network device with
>   CAP_SYS_MODULE (deprecated). Use CAP_NET_ADMIN and alias
>   netdev-nlmon instead.
> 
> This false-positive happens despite with having correct
> capabilities set, e.g. through issuing `ip link del dev nlmon`
> more than once on a valid device with name nlmon, but Marcel
> has also seen it on creation time when no nlmon module is
> previously compiled-in or loaded as module and the device
> name equals a link type name (e.g. nlmon, vxlan, team).
> 
> Stephen says:
> 
>   The netdev module alias is a hold over from the past. For
>   normal devices, people used to create a alias eth0 to and
>   point it to the type of network device used, that was back
>   in the bad old ISA days before real discovery.
> 
>   Also, the tunnels create module alias for the control device
>   and ip used to use this to autoload the tunnel device.
> 
>   The message is bogus and should just be removed, I also see
>   it in a couple of other cases where tap devices are renamed
>   for other usese.
> 
> As mentioned in 8909c9ad8ff0 ("net: don't allow CAP_NET_ADMIN
> to load non-netdev kernel modules"), we nevertheless still
> might want to leave the old autoloading behaviour in place
> as it could break old scripts, so for now, lets just remove
> the log message as Stephen suggests.
> 
> Reference: http://thread.gmane.org/gmane.linux.kernel/1105168
> Reported-by: Marcel Holtmann <marcel@holtmann.org>
> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Vasiliy Kulikov <segoon@openwall.com>
> ---
>  (Sending to net-next as I don't think it's very urgent.)

Applied, thanks Daniel.

^ permalink raw reply

* Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
From: David Miller @ 2014-09-05 19:03 UTC (permalink / raw)
  To: hannes; +Cc: netdev, linux-kernel, spender, dborkman, ast, keescook
In-Reply-To: <2bf2e54282097642db88e2b596b06a9ac3742883.1409690849.git.hannes@stressinduktion.org>

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Tue,  2 Sep 2014 22:53:44 +0200

> From: Daniel Borkmann <dborkman@redhat.com>
> 
> With eBPF getting more extended and exposure to user space is on it's way,
> hardening the memory range the interpreter uses to steer its command flow
> seems appropriate.  This patch moves the to be interpreted bytecode to
> read-only pages.
> 
> In case we execute a corrupted BPF interpreter image for some reason e.g.
> caused by an attacker which got past a verifier stage, it would not only
> provide arbitrary read/write memory access but arbitrary function calls
> as well. After setting up the BPF interpreter image, its contents do not
> change until destruction time, thus we can setup the image on immutable
> made pages in order to mitigate modifications to that code. The idea
> is derived from commit 314beb9bcabf ("x86: bpf_jit_comp: secure bpf jit
> against spraying attacks").
> 
> This is possible because bpf_prog is not part of sk_filter anymore.
> After setup bpf_prog cannot be altered during its life-time. This prevents
> any modifications to the entire bpf_prog structure (incl. function/JIT
> image pointer).
> 
> Every eBPF program (including classic BPF that are migrated) have to call
> bpf_prog_select_runtime() to select either interpreter or a JIT image
> as a last setup step, and they all are being freed via bpf_prog_free(),
> including non-JIT. Therefore, we can easily integrate this into the
> eBPF life-time, plus since we directly allocate a bpf_prog, we have no
> performance penalty.
> 
> Tested with seccomp and test_bpf testsuite in JIT/non-JIT mode and manual
> inspection of kernel_page_tables.  Brad Spengler proposed the same idea
> via Twitter during development of this patch.
> 
> Joint work with Hannes Frederic Sowa.
> 
> Suggested-by: Brad Spengler <spender@grsecurity.net>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net v2] ipv6: fix rtnl locking in setsockopt for anycast and multicast
From: Cong Wang @ 2014-09-05 18:58 UTC (permalink / raw)
  To: David Miller
  Cc: Sabrina Dubroca, Hannes Frederic Sowa, Tommi Rantala,
	Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, netdev, linux-kernel@vger.kernel.org, trinity,
	Dave Jones
In-Reply-To: <20140905.115305.809714259412705649.davem@davemloft.net>

On Fri, Sep 5, 2014 at 11:53 AM, David Miller <davem@davemloft.net> wrote:
> From: Sabrina Dubroca <sd@queasysnail.net>
> Date: Tue, 2 Sep 2014 10:29:29 +0200
>
>> Calling setsockopt with IPV6_JOIN_ANYCAST or IPV6_LEAVE_ANYCAST
>> triggers the assertion in addrconf_join_solict()/addrconf_leave_solict()
>>
>> ipv6_sock_ac_join(), ipv6_sock_ac_drop(), ipv6_sock_ac_close() need to
>> take RTNL before calling ipv6_dev_ac_inc/dec. Same thing with
>> ipv6_sock_mc_join(), ipv6_sock_mc_drop(), ipv6_sock_mc_close() before
>> calling ipv6_dev_mc_inc/dec.
>>
>> This patch moves ASSERT_RTNL() up a level in the call stack.
>>
>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
>> Reported-by: Tommi Rantala <tt.rantala@gmail.com>
>
> Applied and queued up for -stable, thanks.

I believe you applied a wrong version, at least the following
is not correct:

+       if (!dev)
+               return -ENODEV;

Sabrina took that from my draft patch, but they all don't
realize this is wrong.

(I did provide a correct version which is just ignored by you.)

^ permalink raw reply

* Re: [PATCH net v2] ipv6: fix rtnl locking in setsockopt for anycast and multicast
From: David Miller @ 2014-09-05 18:53 UTC (permalink / raw)
  To: sd
  Cc: hannes, cwang, tt.rantala, kuznet, jmorris, yoshfuji, kaber,
	netdev, linux-kernel, trinity, davej
In-Reply-To: <20140902082929.GA8483@kria>

From: Sabrina Dubroca <sd@queasysnail.net>
Date: Tue, 2 Sep 2014 10:29:29 +0200

> Calling setsockopt with IPV6_JOIN_ANYCAST or IPV6_LEAVE_ANYCAST
> triggers the assertion in addrconf_join_solict()/addrconf_leave_solict()
> 
> ipv6_sock_ac_join(), ipv6_sock_ac_drop(), ipv6_sock_ac_close() need to
> take RTNL before calling ipv6_dev_ac_inc/dec. Same thing with
> ipv6_sock_mc_join(), ipv6_sock_mc_drop(), ipv6_sock_mc_close() before
> calling ipv6_dev_mc_inc/dec.
> 
> This patch moves ASSERT_RTNL() up a level in the call stack.
> 
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> Reported-by: Tommi Rantala <tt.rantala@gmail.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply

* Re: [PATCH] VMXNET3: Check for map error in vmxnet3_set_mc
From: David Miller @ 2014-09-05 18:50 UTC (permalink / raw)
  To: acking
  Cc: sergei.shtylyov, pv-drivers, netdev, linux-kernel, virtualization,
	penguin-kernel
In-Reply-To: <1409688824-32619-2-git-send-email-acking@vmware.com>

From: Andy King <acking@vmware.com>
Date: Tue,  2 Sep 2014 13:13:44 -0700

> We should check if the map of the table actually succeeds, and also free
> resources accordingly.
> 
> Version bumped to 1.2.1.0
> 
> Acked-by: Shelley Gong <shelleygong@vmware.com>
> Acked-by: Bhavesh Davda <bhavesh@vmware.com>
> Signed-off-by: Andy King <acking@vmware.com>
> Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

Applied, thanks.

^ permalink raw reply

* page allocation failure. order:2, mode:0x204020 while enabling Jumbo frame
From: Jose M Calhariz @ 2014-09-05 18:44 UTC (permalink / raw)
  To: netdev, ns-list

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


Hi

I have a Debian 7.0 server currently running kernel 3.16 from
kernel.org.  This server is a client of nfsv3, iSCSI and use dmcrypt.
When and only when enabling Jumbo Frames I get "page allocation
failure: order:2, mode:0x204020", in the end of the email is a stack
trace from this error.

Sometimes the rate of "page allocation failure" is very big or the
network stack stops, making the machine "network unreachable" to a
nagios server.


Sep  5 10:51:14 XXXXXX kernel: warn_alloc_failed: 5 callbacks suppressed
Sep  5 10:51:14 XXXXXX kernel: kworker/0:1: page allocation failure: order:2, mode:0x204020
Sep  5 10:51:14 XXXXXX kernel: CPU: 0 PID: 6178 Comm: kworker/0:1 Tainted: G          I   3.16.0-dsi #9
Sep  5 10:51:14 XXXXXX kernel: Hardware name: Dell Inc. PowerEdge R410/01V648, BIOS 1.12.0 07/30/2013
Sep  5 10:51:14 XXXXXX kernel: Workqueue: kcryptd kcryptd_crypt [dm_crypt]
Sep  5 10:51:14 XXXXXX kernel: 0000000000000000 0000000000000000 ffffffff814fd161 0000000000204020
Sep  5 10:51:14 XXXXXX kernel: ffffffff811395d2 ffff88032fffac00 ffffffff818b04b0 0000000000000003
Sep  5 10:51:14 XXXXXX kernel: 0000000000000018 000000012fffac18 ffff8801affff0a8 0000000000000096
Sep  5 10:51:14 XXXXXX kernel: Call Trace:
Sep  5 10:51:14 XXXXXX kernel: <IRQ>  [<ffffffff814fd161>] ? dump_stack+0x41/0x51
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff811395d2>] ? warn_alloc_failed+0xf2/0x150
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff8113d930>] ? __alloc_pages_nodemask+0x8e0/0xb40
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff8118355a>] ? kmem_getpages+0x5a/0x110
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff8118522f>] ? fallback_alloc+0x1cf/0x220
Sep  5 10:51:14 XXXXXX kernel: [<ffffffffa019ef1d>] ? bnx2x_alloc_rx_data.isra.75+0x5d/0x1e0 [bnx2x]
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff811854bc>] ? __kmalloc+0x19c/0x1f0
Sep  5 10:51:14 XXXXXX kernel: [<ffffffffa019ef1d>] ? bnx2x_alloc_rx_data.isra.75+0x5d/0x1e0 [bnx2x]
Sep  5 10:51:14 XXXXXX kernel: [<ffffffffa019f894>] ? bnx2x_rx_int+0x7f4/0x1790 [bnx2x]
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff814005e8>] ? skb_release_head_state+0x48/0xf0
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff814006f9>] ? __kfree_skb+0x9/0xa0
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff81461f6d>] ? tcp_schedule_loss_probe+0x12d/0x1a0
Sep  5 10:51:14 XXXXXX kernel: [<ffffffffa01a25a8>] ? bnx2x_poll+0xe8/0x3b0 [bnx2x]
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff8140fc18>] ? net_rx_action+0x138/0x240
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff8106af4f>] ? __do_softirq+0xdf/0x270
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff8106b315>] ? irq_exit+0x95/0xa0
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff81015701>] ? do_IRQ+0x61/0x110
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff81503aed>] ? common_interrupt+0x6d/0x6d
Sep  5 10:51:14 XXXXXX kernel: <EOI>  [<ffffffffa0378c10>] ? _aesni_enc1+0x5c/0xac [aesni_intel]
Sep  5 10:51:14 XXXXXX kernel: [<ffffffffa0379170>] ? aesni_cbc_enc+0x20/0x40 [aesni_intel]
Sep  5 10:51:14 XXXXXX kernel: [<ffffffffa037ab24>] ? cbc_encrypt+0x74/0xd0 [aesni_intel]
Sep  5 10:51:14 XXXXXX kernel: [<ffffffffa02270fe>] ? __ablk_encrypt+0x3e/0x50 [ablk_helper]
Sep  5 10:51:14 XXXXXX kernel: [<ffffffffa069641b>] ? crypt_convert+0x20b/0x460 [dm_crypt]
Sep  5 10:51:14 XXXXXX kernel: [<ffffffffa069682b>] ? kcryptd_crypt+0x1bb/0x400 [dm_crypt]
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff81080adb>] ? process_one_work+0x16b/0x400
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff81081184>] ? worker_thread+0x114/0x510
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff814ff3f8>] ? __schedule+0x2c8/0x760
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff81081070>] ? rescuer_thread+0x2c0/0x2c0
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff810876cc>] ? kthread+0xbc/0xe0
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff81087610>] ? flush_kthread_worker+0x80/0x80
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff81502f3c>] ? ret_from_fork+0x7c/0xb0
Sep  5 10:51:14 XXXXXX kernel: [<ffffffff81087610>] ? flush_kthread_worker+0x80/0x80
Sep  5 10:51:14 XXXXXX kernel: Mem-Info:
Sep  5 10:51:14 XXXXXX kernel: Node 0 Normal per-cpu:
Sep  5 10:51:14 XXXXXX kernel: CPU    0: hi:  186, btch:  31 usd: 141
Sep  5 10:51:14 XXXXXX kernel: CPU    1: hi:  186, btch:  31 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    2: hi:  186, btch:  31 usd: 178
Sep  5 10:51:14 XXXXXX kernel: CPU    3: hi:  186, btch:  31 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    4: hi:  186, btch:  31 usd: 169
Sep  5 10:51:14 XXXXXX kernel: CPU    5: hi:  186, btch:  31 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    6: hi:  186, btch:  31 usd: 181
Sep  5 10:51:14 XXXXXX kernel: CPU    7: hi:  186, btch:  31 usd:   0
Sep  5 10:51:14 XXXXXX kernel: Node 1 DMA per-cpu:
Sep  5 10:51:14 XXXXXX kernel: CPU    0: hi:    0, btch:   1 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    1: hi:    0, btch:   1 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    2: hi:    0, btch:   1 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    3: hi:    0, btch:   1 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    4: hi:    0, btch:   1 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    5: hi:    0, btch:   1 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    6: hi:    0, btch:   1 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    7: hi:    0, btch:   1 usd:   0
Sep  5 10:51:14 XXXXXX kernel: Node 1 DMA32 per-cpu:
Sep  5 10:51:14 XXXXXX kernel: CPU    0: hi:  186, btch:  31 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    1: hi:  186, btch:  31 usd:  51
Sep  5 10:51:14 XXXXXX kernel: CPU    2: hi:  186, btch:  31 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    3: hi:  186, btch:  31 usd:  36
Sep  5 10:51:14 XXXXXX kernel: CPU    4: hi:  186, btch:  31 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    5: hi:  186, btch:  31 usd:  51
Sep  5 10:51:14 XXXXXX kernel: CPU    6: hi:  186, btch:  31 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    7: hi:  186, btch:  31 usd:  59
Sep  5 10:51:14 XXXXXX kernel: Node 1 Normal per-cpu:
Sep  5 10:51:14 XXXXXX kernel: CPU    0: hi:  186, btch:  31 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    1: hi:  186, btch:  31 usd:  95
Sep  5 10:51:14 XXXXXX kernel: CPU    2: hi:  186, btch:  31 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    3: hi:  186, btch:  31 usd: 100
Sep  5 10:51:14 XXXXXX kernel: CPU    4: hi:  186, btch:  31 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    5: hi:  186, btch:  31 usd:  95
Sep  5 10:51:14 XXXXXX kernel: CPU    6: hi:  186, btch:  31 usd:   0
Sep  5 10:51:14 XXXXXX kernel: CPU    7: hi:  186, btch:  31 usd:  79
Sep  5 10:51:14 XXXXXX kernel: active_anon:391978 inactive_anon:217234 isolated_anon:0
Sep  5 10:51:14 XXXXXX kernel: active_file:405690 inactive_file:251497 isolated_file:0
Sep  5 10:51:14 XXXXXX kernel: unevictable:2192 dirty:0 writeback:0 unstable:0
Sep  5 10:51:14 XXXXXX kernel: free:482104 slab_reclaimable:827503 slab_unreclaimable:171240
Sep  5 10:51:14 XXXXXX kernel: mapped:3469 shmem:99 pagetables:2164 bounce:0
Sep  5 10:51:14 XXXXXX kernel: free_cma:0
Sep  5 10:51:14 XXXXXX kernel: Node 0 Normal free:801464kB min:528080kB low:660100kB high:792120kB active_anon:1161624kB inactive_anon:387580kB active_file:534424kB inactive_file:291488kB unevictable:4940kB isolated(anon):0kB isolated(file):0kB present:6291456kB managed:6203092kB mlocked:4944kB dirty:0kB
+writeback:0kB mapped:5116kB shmem:124kB slab_reclaimable:1923736kB slab_unreclaimable:386892kB kernel_stack:4368kB pagetables:5768kB unstable:0kB bounce:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:47 all_unreclaimable? no
Sep  5 10:51:14 XXXXXX kernel: lowmem_reserve[]: 0 0 0 0
Sep  5 10:51:14 XXXXXX kernel: Node 1 DMA free:15896kB min:1352kB low:1688kB high:2028kB active_anon:0kB inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:15980kB managed:15896kB mlocked:0kB dirty:0kB writeback:0kB mapped:0kB shmem:0kB
+slab_reclaimable:0kB slab_unreclaimable:0kB kernel_stack:0kB pagetables:0kB unstable:0kB bounce:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? yes
Sep  5 10:51:14 XXXXXX kernel: lowmem_reserve[]: 0 3224 5952 5952
Sep  5 10:51:14 XXXXXX kernel: Node 1 DMA32 free:804396kB min:281328kB low:351660kB high:421992kB active_anon:240284kB inactive_anon:288392kB active_file:558572kB inactive_file:412684kB unevictable:1972kB isolated(anon):0kB isolated(file):0kB present:3378660kB managed:3304628kB mlocked:1972kB dirty:0kB writeback:0kB
+mapped:4144kB shmem:204kB slab_reclaimable:618324kB slab_unreclaimable:133696kB kernel_stack:1168kB pagetables:1512kB unstable:0kB bounce:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
Sep  5 10:51:14 XXXXXX kernel: lowmem_reserve[]: 0 0 2727 2727
Sep  5 10:51:14 XXXXXX kernel: Node 1 Normal free:306660kB min:237808kB low:297260kB high:356712kB active_anon:166004kB inactive_anon:192964kB active_file:529764kB inactive_file:301816kB unevictable:1856kB isolated(anon):0kB isolated(file):0kB present:2883584kB managed:2793404kB mlocked:1856kB dirty:0kB writeback:0kB
+mapped:4616kB shmem:68kB slab_reclaimable:767952kB slab_unreclaimable:164372kB kernel_stack:1312kB pagetables:1376kB unstable:0kB bounce:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
Sep  5 10:51:14 XXXXXX kernel: lowmem_reserve[]: 0 0 0 0
Sep  5 10:51:14 XXXXXX kernel: Node 0 Normal: 186511*4kB (UEM) 5585*8kB (UEM) 421*16kB (UM) 1*32kB (M) 0*64kB 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 1*4096kB (R) = 801588kB
Sep  5 10:51:14 XXXXXX kernel: Node 1 DMA: 0*4kB 1*8kB (U) 1*16kB (U) 0*32kB 2*64kB (U) 1*128kB (U) 1*256kB (U) 0*512kB 1*1024kB (U) 1*2048kB (R) 3*4096kB (M) = 15896kB
Sep  5 10:51:14 XXXXXX kernel: Node 1 DMA32: 97773*4kB (EM) 49708*8kB (EM) 721*16kB (M) 0*32kB 0*64kB 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 1*4096kB (R) = 804388kB
Sep  5 10:51:14 XXXXXX kernel: Node 1 Normal: 48558*4kB (EM) 13544*8kB (M) 0*16kB 0*32kB 0*64kB 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 1*4096kB (R) = 306680kB
Sep  5 10:51:14 XXXXXX kernel: Node 0 hugepages_total=0 hugepages_free=0 hugepages_surp=0 hugepages_size=2048kB
Sep  5 10:51:14 XXXXXX kernel: Node 1 hugepages_total=0 hugepages_free=0 hugepages_surp=0 hugepages_size=2048kB
Sep  5 10:51:14 XXXXXX kernel: 658273 total pagecache pages
Sep  5 10:51:14 XXXXXX kernel: 37 pages in swap cache
Sep  5 10:51:14 XXXXXX kernel: Swap cache stats: add 499, delete 462, find 7/10
Sep  5 10:51:14 XXXXXX kernel: Free swap  = 37456928kB
Sep  5 10:51:14 XXXXXX kernel: Total swap = 37458864kB
Sep  5 10:51:14 XXXXXX kernel: 3142420 pages RAM
Sep  5 10:51:14 XXXXXX kernel: 0 pages HighMem/MovableOnly
Sep  5 10:51:14 XXXXXX kernel: 22545 pages reserved
Sep  5 10:51:14 XXXXXX kernel: 0 pages hwpoisoned



-- 
--
	Citação: ato de repetir de modo errado as palavras alheias.
		--  Ambrose Bierce; jornalista e cientista americano.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: Does NetworkManager kill IPv6 connections?
From: Dan Williams @ 2014-09-05 18:42 UTC (permalink / raw)
  To: David Laight; +Cc: netdev@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1748838C@AcuExch.aculab.com>

On Fri, 2014-09-05 at 14:18 +0000, David Laight wrote:
> From: Dan Williams 
> > On Thu, 2014-09-04 at 14:47 +0000, David Laight wrote:
> > > I was running a overnight test that failed because an (almost idle) IPv6 connection
> > > unexpectedly disconnected.
> > > 30 seconds before the disconnect syslog contains (retyped):
> > >   NetworkManager[pid]: <info> policy set 'Wired connection 2' (eth0) as default for IPv6 routing and DNS.
> > > The same message appeared exactly 70 minutes earlier - with no ill effect.
> > > A second connection between the same pair if addresses (carrying a lot of traffic)
> > > was unaffected.
> > >
> > > Any idea what/why NetworkManager is doing?
> > > (Yes I know it is userspace, but it seems to be breaking the kernel....)
> > >
> > > The system is an ubuntu 13.04 desktop (3.8.0-19 kernel), that recent, but
> > > newer than the ones a lot of our customers use.
> > 
> > Is the version of the Ubuntu "network-manager" package on your systems
> > 0.9.8.0-0ubuntu6.1?
> 
> It is actually the slightly older 0.9.8.0-0ubuntu6.0
> 
> > If so, I know we've fixed a ton of issues with IPv6 since 0.9.8.0 (which
> > was released about 18 months ago), and the ones that may be affecting
> > you are related to router advertisement timing and expiry.  The latest
> > stable NM version upstream is 0.9.10 and should contain all of those
> > fixes.
> 
> I've flagged it with our management, they'll decide what to say
> about IPv6 support in various Linux distros.

Ok, keep me (and the NM upstream) in the loop depending on what gets
decided.  Like I said, subsequent stable versions of NetworkManager in
recent distros have much improved IPv6 support, and if you have a chance
to try it out and let me know how it goes, that would be awesome.

Dan

> 	David
> 
> > Since I'm pretty sure this isn't a kernel problem, would you mind
> > discussing this over on the NM lists here:
> > 
> > https://mail.gnome.org/mailman/listinfo/networkmanager-list
> > 
> > If you don't want to subscribe that's fine, I'll moderate your message
> > through and we can just use "reply all".
> > 
> > Thanks,
> > Dan
> > 
> > > I can rerun the test, but don't know what to look for.
> > > 'eth0' is the only ethernet interface in the system that is actually
> > > connected to a network.
> > >
> > > Some indications of where to look would be appreciated.
> > >
> > > 	David
> > >
> > >
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 

^ permalink raw reply

* Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM
From: Dmitry Torokhov @ 2014-09-05 18:29 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Tejun Heo, Greg Kroah-Hartman, Wu Zhangjin, Takashi Iwai,
	Arjan van de Ven, linux-kernel@vger.kernel.org, Oleg Nesterov,
	hare, Andrew Morton, Tetsuo Handa, Joseph Salisbury,
	Benjamin Poirier, Santosh Rastapur, Kay Sievers,
	One Thousand Gnomes, Tim Gardner, Pierre Fersing,
	Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
	Abhijit Mahajan <abhijit.mah
In-Reply-To: <CAB=NE6VPTV39pNujOgO422wbUvU44HZ7s=aaxPVraNECqA6e4Q@mail.gmail.com>

On Fri, Sep 05, 2014 at 11:12:17AM -0700, Luis R. Rodriguez wrote:
> On Fri, Sep 5, 2014 at 10:49 AM, Tejun Heo <tj@kernel.org> wrote:
> > Hello,
> >
> > On Fri, Sep 05, 2014 at 09:44:05AM -0700, Dmitry Torokhov wrote:
> >> Which problem are we talking about here though? It does solve the slow device
> >> stalling the rest if the kernel booting (non-module case) for me.
> >
> > The other one.  The one with timeout.  Neither cxgb4 or pata_marvell
> > has slow probing stalling boot problem.
> >
> >> I also reject the notion that anyone should be relying on drivers to be fully
> >> bound on module loading. It is not nineties anymore. We have hot pluggable
> >> buses, deferred probing, and even for not hot-pluggable ones the module
> >> providing the device itself might not be yet loaded. Any scripts that expect to
> >> find device 100% ready after module loading are simply broken.
> >
> > We've been treating loading + probing as a single operation when
> > loading drivers and the assumption has always been that the existing
> > devices at the time of loading finished probing by the time insmod
> > finishes.  We now need to split loading and probing and wait for each
> > of them differently.  The *only* thing we can do is somehow making the
> > issuer specify that it's gonna wait for probing separately.  I'm not
> > sure this can even be up for discussion.  We're talking about a major
> > userland visible behavior change.  We simply can't change it
> > underneath the existing users.
> 
> Meanwhile we are allowing a major design consideration such as a 30
> second timeout for both init + probe all of a sudden become a hard
> requirement for device drivers. I see your point but can't also be
> introducing major design changes willy nilly either. We *need* a
> solution for the affected drivers.
> 
> Also what stops drivers from going ahead and just implementing their
> own async probe? 

They already do and the problem is that they do that poorly. One of the issues
is that the device is considered bound and so may attempt to suspend/resume
them, or unbind them, and the driver is not ready for such operations to take
place.

And even though driver is bound "synchronously" it does not help the user in
the slightest and the object that is the result of driver initialization is
still created asynchronously and is not ready (well, it might if drivers use
async_schedule as we are doing asych_sycnhronize_full() in module load.unload).

Thanks.

-- 
Dmitry

^ permalink raw reply

* [PATCH net-next 3/3] ixgbe: use new eth_get_headlen interface
From: Alexander Duyck @ 2014-09-05 18:11 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet, jeffrey.t.kirsher
In-Reply-To: <20140905180642.730.22292.stgit@ahduyck-bv4.jf.intel.com>

Update ixgbe to drop the ixgbe_get_headlen function in favor of eth_get_headlen.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  116 -------------------------
 1 file changed, 1 insertion(+), 115 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 53fbf06..09c3746 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1521,120 +1521,6 @@ void ixgbe_alloc_rx_buffers(struct ixgbe_ring *rx_ring, u16 cleaned_count)
 		ixgbe_release_rx_desc(rx_ring, i);
 }
 
-/**
- * ixgbe_get_headlen - determine size of header for RSC/LRO/GRO/FCOE
- * @data: pointer to the start of the headers
- * @max_len: total length of section to find headers in
- *
- * This function is meant to determine the length of headers that will
- * be recognized by hardware for LRO, GRO, and RSC offloads.  The main
- * motivation of doing this is to only perform one pull for IPv4 TCP
- * packets so that we can do basic things like calculating the gso_size
- * based on the average data per packet.
- **/
-static unsigned int ixgbe_get_headlen(unsigned char *data,
-				      unsigned int max_len)
-{
-	union {
-		unsigned char *network;
-		/* l2 headers */
-		struct ethhdr *eth;
-		struct vlan_hdr *vlan;
-		/* l3 headers */
-		struct iphdr *ipv4;
-		struct ipv6hdr *ipv6;
-	} hdr;
-	__be16 protocol;
-	u8 nexthdr = 0;	/* default to not TCP */
-	u8 hlen;
-
-	/* this should never happen, but better safe than sorry */
-	if (max_len < ETH_HLEN)
-		return max_len;
-
-	/* initialize network frame pointer */
-	hdr.network = data;
-
-	/* set first protocol and move network header forward */
-	protocol = hdr.eth->h_proto;
-	hdr.network += ETH_HLEN;
-
-	/* handle any vlan tag if present */
-	if (protocol == htons(ETH_P_8021Q)) {
-		if ((hdr.network - data) > (max_len - VLAN_HLEN))
-			return max_len;
-
-		protocol = hdr.vlan->h_vlan_encapsulated_proto;
-		hdr.network += VLAN_HLEN;
-	}
-
-	/* handle L3 protocols */
-	if (protocol == htons(ETH_P_IP)) {
-		if ((hdr.network - data) > (max_len - sizeof(struct iphdr)))
-			return max_len;
-
-		/* access ihl as a u8 to avoid unaligned access on ia64 */
-		hlen = (hdr.network[0] & 0x0F) << 2;
-
-		/* verify hlen meets minimum size requirements */
-		if (hlen < sizeof(struct iphdr))
-			return hdr.network - data;
-
-		/* record next protocol if header is present */
-		if (!(hdr.ipv4->frag_off & htons(IP_OFFSET)))
-			nexthdr = hdr.ipv4->protocol;
-	} else if (protocol == htons(ETH_P_IPV6)) {
-		if ((hdr.network - data) > (max_len - sizeof(struct ipv6hdr)))
-			return max_len;
-
-		/* record next protocol */
-		nexthdr = hdr.ipv6->nexthdr;
-		hlen = sizeof(struct ipv6hdr);
-#ifdef IXGBE_FCOE
-	} else if (protocol == htons(ETH_P_FCOE)) {
-		if ((hdr.network - data) > (max_len - FCOE_HEADER_LEN))
-			return max_len;
-		hlen = FCOE_HEADER_LEN;
-#endif
-	} else {
-		return hdr.network - data;
-	}
-
-	/* relocate pointer to start of L4 header */
-	hdr.network += hlen;
-
-	/* finally sort out TCP/UDP */
-	if (nexthdr == IPPROTO_TCP) {
-		if ((hdr.network - data) > (max_len - sizeof(struct tcphdr)))
-			return max_len;
-
-		/* access doff as a u8 to avoid unaligned access on ia64 */
-		hlen = (hdr.network[12] & 0xF0) >> 2;
-
-		/* verify hlen meets minimum size requirements */
-		if (hlen < sizeof(struct tcphdr))
-			return hdr.network - data;
-
-		hdr.network += hlen;
-	} else if (nexthdr == IPPROTO_UDP) {
-		if ((hdr.network - data) > (max_len - sizeof(struct udphdr)))
-			return max_len;
-
-		hdr.network += sizeof(struct udphdr);
-	}
-
-	/*
-	 * If everything has gone correctly hdr.network should be the
-	 * data section of the packet and will be the end of the header.
-	 * If not then it probably represents the end of the last recognized
-	 * header.
-	 */
-	if ((hdr.network - data) < max_len)
-		return hdr.network - data;
-	else
-		return max_len;
-}
-
 static void ixgbe_set_rsc_gso_size(struct ixgbe_ring *ring,
 				   struct sk_buff *skb)
 {
@@ -1793,7 +1679,7 @@ static void ixgbe_pull_tail(struct ixgbe_ring *rx_ring,
 	 * we need the header to contain the greater of either ETH_HLEN or
 	 * 60 bytes if the skb->len is less than 60 for skb_pad.
 	 */
-	pull_len = ixgbe_get_headlen(va, IXGBE_RX_HDR_SIZE);
+	pull_len = eth_get_headlen(va, IXGBE_RX_HDR_SIZE);
 
 	/* align pull length to size of long to optimize memcpy performance */
 	skb_copy_to_linear_data(skb, va, ALIGN(pull_len, sizeof(long)));

^ permalink raw reply related

* [PATCH net-next 1/3] net: Add function for parsing the header length out of linear ethernet frames
From: Alexander Duyck @ 2014-09-05 18:10 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet, jeffrey.t.kirsher
In-Reply-To: <20140905180642.730.22292.stgit@ahduyck-bv4.jf.intel.com>

This patch updates some of the flow_dissector api so that it can be used to
parse the length of ethernet buffers stored in fragments.  Most of the
changes needed were to __skb_get_poff as it needed to be updated to support
sending a linear buffer instead of a skb.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 include/linux/etherdevice.h |    1 +
 include/linux/skbuff.h      |    2 ++
 include/net/flow_keys.h     |    2 ++
 net/core/flow_dissector.c   |   46 ++++++++++++++++++++++++++++++-------------
 net/ethernet/eth.c          |   27 +++++++++++++++++++++++++
 5 files changed, 64 insertions(+), 14 deletions(-)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 9c5529d..733980f 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -29,6 +29,7 @@
 #include <asm/bitsperlong.h>
 
 #ifdef __KERNEL__
+u32 eth_get_headlen(void *data, unsigned int max_len);
 __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev);
 extern const struct header_ops eth_header_ops;
 
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 02529fc..77d4d6f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3217,6 +3217,8 @@ bool skb_partial_csum_set(struct sk_buff *skb, u16 start, u16 off);
 int skb_checksum_setup(struct sk_buff *skb, bool recalculate);
 
 u32 __skb_get_poff(const struct sk_buff *skb);
+u32 ___skb_get_poff(const struct sk_buff *skb, void *data,
+	 	    const struct flow_keys *keys, int hlen);
 
 /**
  * skb_head_is_locked - Determine if the skb->head is locked down
diff --git a/include/net/flow_keys.h b/include/net/flow_keys.h
index 9a03f73..7ee2df0 100644
--- a/include/net/flow_keys.h
+++ b/include/net/flow_keys.h
@@ -40,4 +40,6 @@ static inline __be32 skb_flow_get_ports(const struct sk_buff *skb, int thoff, u8
 	return __skb_flow_get_ports(skb, thoff, ip_proto, NULL, 0);
 }
 u32 flow_hash_from_keys(struct flow_keys *keys);
+unsigned int flow_get_hlen(const unsigned char *data, unsigned int max_len,
+			   __be16 protocol);
 #endif
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 12f48ca..179b2aa 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -13,6 +13,7 @@
 #include <linux/if_pppox.h>
 #include <linux/ppp_defs.h>
 #include <net/flow_keys.h>
+#include <scsi/fc/fc_fcoe.h>
 
 /* copy saddr & daddr, possibly using 64bit load/store
  * Equivalent to :	flow->src = iph->saddr;
@@ -117,6 +118,13 @@ ipv6:
 		flow->dst = (__force __be32)ipv6_addr_hash(&iph->daddr);
 		nhoff += sizeof(struct ipv6hdr);
 
+		/* skip the flow label processing if skb is NULL.  The
+		 * assumption here is that if there is no skb we are not
+		 * looking for flow info as much as we are length.
+		 */
+		if (!skb)
+			break;
+
 		flow_label = ip6_flowlabel(iph);
 		if (flow_label) {
 			/* Awesome, IPv6 packet has a flow label so we can
@@ -165,6 +173,9 @@ ipv6:
 			return false;
 		}
 	}
+	case htons(ETH_P_FCOE):
+		flow->thoff = (u16)(nhoff + FCOE_HEADER_LEN);
+		/* fall through */
 	default:
 		return false;
 	}
@@ -316,26 +327,18 @@ u16 __skb_tx_hash(const struct net_device *dev, struct sk_buff *skb,
 }
 EXPORT_SYMBOL(__skb_tx_hash);
 
-/* __skb_get_poff() returns the offset to the payload as far as it could
- * be dissected. The main user is currently BPF, so that we can dynamically
- * truncate packets without needing to push actual payload to the user
- * space and can analyze headers only, instead.
- */
-u32 __skb_get_poff(const struct sk_buff *skb)
+u32 ___skb_get_poff(const struct sk_buff *skb, void *data,
+	 	    const struct flow_keys *keys, int hlen)
 {
-	struct flow_keys keys;
-	u32 poff = 0;
-
-	if (!skb_flow_dissect(skb, &keys))
-		return 0;
+	u32 poff = keys->thoff;
 
-	poff += keys.thoff;
-	switch (keys.ip_proto) {
+	switch (keys->ip_proto) {
 	case IPPROTO_TCP: {
 		const struct tcphdr *tcph;
 		struct tcphdr _tcph;
 
-		tcph = skb_header_pointer(skb, poff, sizeof(_tcph), &_tcph);
+		tcph = __skb_header_pointer(skb, poff, sizeof(_tcph),
+					    data, hlen, &_tcph);
 		if (!tcph)
 			return poff;
 
@@ -369,6 +372,21 @@ u32 __skb_get_poff(const struct sk_buff *skb)
 	return poff;
 }
 
+/* __skb_get_poff() returns the offset to the payload as far as it could
+ * be dissected. The main user is currently BPF, so that we can dynamically
+ * truncate packets without needing to push actual payload to the user
+ * space and can analyze headers only, instead.
+ */
+u32 __skb_get_poff(const struct sk_buff *skb)
+{
+	struct flow_keys keys;
+
+	if (!skb_flow_dissect(skb, &keys))
+		return 0;
+
+	return ___skb_get_poff(skb, skb->data, &keys, skb_headlen(skb));
+}
+
 static inline int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
 {
 #ifdef CONFIG_XPS
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 5cebca1..f0299ac 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -146,6 +146,33 @@ int eth_rebuild_header(struct sk_buff *skb)
 EXPORT_SYMBOL(eth_rebuild_header);
 
 /**
+ * eth_get_headlen - determine the the length of header for an ethernet frame
+ * @data: pointer to start of frame
+ * @len: total length of frame
+ *
+ * Make a best effort attempt to pull the length for all of the headers for
+ * a given frame in a linear buffer.
+ */
+u32 eth_get_headlen(void *data, unsigned int len)
+{
+	const struct ethhdr *eth = (const struct ethhdr *)data;
+	struct flow_keys keys;
+
+	/* this should never happen, but better safe than sorry */
+	if (len < sizeof(*eth))
+		return len;
+
+	/* parse any remaining L2/L3 headers, check for L4 */
+	if (!__skb_flow_dissect(NULL, &keys, data,
+				eth->h_proto, sizeof(*eth), len))
+		return max_t(u32, keys.thoff, sizeof(*eth));
+
+	/* parse for any L4 headers */
+	return min_t(u32, ___skb_get_poff(NULL, data, &keys, len), len);
+}
+EXPORT_SYMBOL(eth_get_headlen);
+
+/**
  * eth_type_trans - determine the packet's protocol ID.
  * @skb: received socket data
  * @dev: receiving network device

^ permalink raw reply related

* [PATCH net-next 2/3] igb: use new eth_get_headlen interface
From: Alexander Duyck @ 2014-09-05 18:11 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet, jeffrey.t.kirsher
In-Reply-To: <20140905180642.730.22292.stgit@ahduyck-bv4.jf.intel.com>

Update igb to drop the igb_get_headlen function in favor of eth_get_headlen.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c |  109 -----------------------------
 1 file changed, 1 insertion(+), 108 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 89de7fe..4c023f0 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6769,113 +6769,6 @@ static bool igb_is_non_eop(struct igb_ring *rx_ring,
 }
 
 /**
- *  igb_get_headlen - determine size of header for LRO/GRO
- *  @data: pointer to the start of the headers
- *  @max_len: total length of section to find headers in
- *
- *  This function is meant to determine the length of headers that will
- *  be recognized by hardware for LRO, and GRO offloads.  The main
- *  motivation of doing this is to only perform one pull for IPv4 TCP
- *  packets so that we can do basic things like calculating the gso_size
- *  based on the average data per packet.
- **/
-static unsigned int igb_get_headlen(unsigned char *data,
-				    unsigned int max_len)
-{
-	union {
-		unsigned char *network;
-		/* l2 headers */
-		struct ethhdr *eth;
-		struct vlan_hdr *vlan;
-		/* l3 headers */
-		struct iphdr *ipv4;
-		struct ipv6hdr *ipv6;
-	} hdr;
-	__be16 protocol;
-	u8 nexthdr = 0;	/* default to not TCP */
-	u8 hlen;
-
-	/* this should never happen, but better safe than sorry */
-	if (max_len < ETH_HLEN)
-		return max_len;
-
-	/* initialize network frame pointer */
-	hdr.network = data;
-
-	/* set first protocol and move network header forward */
-	protocol = hdr.eth->h_proto;
-	hdr.network += ETH_HLEN;
-
-	/* handle any vlan tag if present */
-	if (protocol == htons(ETH_P_8021Q)) {
-		if ((hdr.network - data) > (max_len - VLAN_HLEN))
-			return max_len;
-
-		protocol = hdr.vlan->h_vlan_encapsulated_proto;
-		hdr.network += VLAN_HLEN;
-	}
-
-	/* handle L3 protocols */
-	if (protocol == htons(ETH_P_IP)) {
-		if ((hdr.network - data) > (max_len - sizeof(struct iphdr)))
-			return max_len;
-
-		/* access ihl as a u8 to avoid unaligned access on ia64 */
-		hlen = (hdr.network[0] & 0x0F) << 2;
-
-		/* verify hlen meets minimum size requirements */
-		if (hlen < sizeof(struct iphdr))
-			return hdr.network - data;
-
-		/* record next protocol if header is present */
-		if (!(hdr.ipv4->frag_off & htons(IP_OFFSET)))
-			nexthdr = hdr.ipv4->protocol;
-	} else if (protocol == htons(ETH_P_IPV6)) {
-		if ((hdr.network - data) > (max_len - sizeof(struct ipv6hdr)))
-			return max_len;
-
-		/* record next protocol */
-		nexthdr = hdr.ipv6->nexthdr;
-		hlen = sizeof(struct ipv6hdr);
-	} else {
-		return hdr.network - data;
-	}
-
-	/* relocate pointer to start of L4 header */
-	hdr.network += hlen;
-
-	/* finally sort out TCP */
-	if (nexthdr == IPPROTO_TCP) {
-		if ((hdr.network - data) > (max_len - sizeof(struct tcphdr)))
-			return max_len;
-
-		/* access doff as a u8 to avoid unaligned access on ia64 */
-		hlen = (hdr.network[12] & 0xF0) >> 2;
-
-		/* verify hlen meets minimum size requirements */
-		if (hlen < sizeof(struct tcphdr))
-			return hdr.network - data;
-
-		hdr.network += hlen;
-	} else if (nexthdr == IPPROTO_UDP) {
-		if ((hdr.network - data) > (max_len - sizeof(struct udphdr)))
-			return max_len;
-
-		hdr.network += sizeof(struct udphdr);
-	}
-
-	/* If everything has gone correctly hdr.network should be the
-	 * data section of the packet and will be the end of the header.
-	 * If not then it probably represents the end of the last recognized
-	 * header.
-	 */
-	if ((hdr.network - data) < max_len)
-		return hdr.network - data;
-	else
-		return max_len;
-}
-
-/**
  *  igb_pull_tail - igb specific version of skb_pull_tail
  *  @rx_ring: rx descriptor ring packet is being transacted on
  *  @rx_desc: pointer to the EOP Rx descriptor
@@ -6919,7 +6812,7 @@ static void igb_pull_tail(struct igb_ring *rx_ring,
 	/* we need the header to contain the greater of either ETH_HLEN or
 	 * 60 bytes if the skb->len is less than 60 for skb_pad.
 	 */
-	pull_len = igb_get_headlen(va, IGB_RX_HDR_LEN);
+	pull_len = eth_get_headlen(va, IGB_RX_HDR_LEN);
 
 	/* align pull length to size of long to optimize memcpy performance */
 	skb_copy_to_linear_data(skb, va, ALIGN(pull_len, sizeof(long)));

^ permalink raw reply related

* [PATCH net-next 0/3] Drop get_headlen functions in favor of generic funciton
From: Alexander Duyck @ 2014-09-05 18:09 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet, jeffrey.t.kirsher

This series replaces the igb_get_headlen and ixgbe_get_headlen functions
with a generic function named eth_get_headlen.

I have done some performance testing on ixgbe with 258 byte frames since
the calls are only used on frames larger than 256 bytes and have seen no
significant difference in CPU utilization.

---

Alexander Duyck (3):
      net: Add function for parsing the header length out of linear ethernet frames
      igb: use new eth_get_headlen interface
      ixgbe: use new eth_get_headlen interface


 drivers/net/ethernet/intel/igb/igb_main.c     |  109 -----------------------
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  116 -------------------------
 include/linux/etherdevice.h                   |    1 
 include/linux/skbuff.h                        |    2 
 include/net/flow_keys.h                       |    2 
 net/core/flow_dissector.c                     |   46 +++++++---
 net/ethernet/eth.c                            |   27 ++++++
 7 files changed, 66 insertions(+), 237 deletions(-)

-- 

^ permalink raw reply

* [RFC net-next 5/5] bonding: remove last users of bond->lock and bond->lock itself
From: Nikolay Aleksandrov @ 2014-09-05 18:16 UTC (permalink / raw)
  To: netdev; +Cc: vfalico, j.vosburgh, andy, davem, Nikolay Aleksandrov
In-Reply-To: <1409941011-5494-1-git-send-email-nikolay@redhat.com>

The usage of bond->lock in bond_main.c was completely unnecessary as it
didn't help to sync with anything, most of the spots already had RTNL.
Since there're no more users of bond->lock, remove it.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c | 39 ---------------------------------------
 drivers/net/bonding/bonding.h   |  8 ++------
 2 files changed, 2 insertions(+), 45 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index dcd331bd0c17..4301c3d49fce 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3101,7 +3101,6 @@ static int bond_open(struct net_device *bond_dev)
 	struct slave *slave;
 
 	/* reset slave->backup and slave->inactive */
-	read_lock(&bond->lock);
 	if (bond_has_slaves(bond)) {
 		read_lock(&bond->curr_slave_lock);
 		bond_for_each_slave(bond, slave, iter) {
@@ -3116,7 +3115,6 @@ static int bond_open(struct net_device *bond_dev)
 		}
 		read_unlock(&bond->curr_slave_lock);
 	}
-	read_unlock(&bond->lock);
 
 	bond_work_init_all(bond);
 
@@ -3171,7 +3169,6 @@ static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev,
 
 	memset(stats, 0, sizeof(*stats));
 
-	read_lock_bh(&bond->lock);
 	bond_for_each_slave(bond, slave, iter) {
 		const struct rtnl_link_stats64 *sstats =
 			dev_get_stats(slave->dev, &temp);
@@ -3202,7 +3199,6 @@ static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev,
 		stats->tx_heartbeat_errors += sstats->tx_heartbeat_errors;
 		stats->tx_window_errors += sstats->tx_window_errors;
 	}
-	read_unlock_bh(&bond->lock);
 
 	return stats;
 }
@@ -3242,13 +3238,11 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
 
 		if (mii->reg_num == 1) {
 			mii->val_out = 0;
-			read_lock(&bond->lock);
 			read_lock(&bond->curr_slave_lock);
 			if (netif_carrier_ok(bond->dev))
 				mii->val_out = BMSR_LSTATUS;
 
 			read_unlock(&bond->curr_slave_lock);
-			read_unlock(&bond->lock);
 		}
 
 		return 0;
@@ -3424,21 +3418,6 @@ static int bond_change_mtu(struct net_device *bond_dev, int new_mtu)
 
 	netdev_dbg(bond_dev, "bond=%p, new_mtu=%d\n", bond, new_mtu);
 
-	/* Can't hold bond->lock with bh disabled here since
-	 * some base drivers panic. On the other hand we can't
-	 * hold bond->lock without bh disabled because we'll
-	 * deadlock. The only solution is to rely on the fact
-	 * that we're under rtnl_lock here, and the slaves
-	 * list won't change. This doesn't solve the problem
-	 * of setting the slave's MTU while it is
-	 * transmitting, but the assumption is that the base
-	 * driver can handle that.
-	 *
-	 * TODO: figure out a way to safely iterate the slaves
-	 * list, but without holding a lock around the actual
-	 * call to the base driver.
-	 */
-
 	bond_for_each_slave(bond, slave, iter) {
 		netdev_dbg(bond_dev, "s %p c_m %p\n",
 			   slave, slave->dev->netdev_ops->ndo_change_mtu);
@@ -3513,21 +3492,6 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr)
 	if (!is_valid_ether_addr(sa->sa_data))
 		return -EADDRNOTAVAIL;
 
-	/* Can't hold bond->lock with bh disabled here since
-	 * some base drivers panic. On the other hand we can't
-	 * hold bond->lock without bh disabled because we'll
-	 * deadlock. The only solution is to rely on the fact
-	 * that we're under rtnl_lock here, and the slaves
-	 * list won't change. This doesn't solve the problem
-	 * of setting the slave's hw address while it is
-	 * transmitting, but the assumption is that the base
-	 * driver can handle that.
-	 *
-	 * TODO: figure out a way to safely iterate the slaves
-	 * list, but without holding a lock around the actual
-	 * call to the base driver.
-	 */
-
 	bond_for_each_slave(bond, slave, iter) {
 		netdev_dbg(bond_dev, "slave %p %s\n", slave, slave->dev->name);
 		res = dev_set_mac_address(slave->dev, addr);
@@ -3853,7 +3817,6 @@ static int bond_ethtool_get_settings(struct net_device *bond_dev,
 	 * the true receive or transmit bandwidth (not all modes are symmetric)
 	 * this is an accurate maximum.
 	 */
-	read_lock(&bond->lock);
 	bond_for_each_slave(bond, slave, iter) {
 		if (bond_slave_can_tx(slave)) {
 			if (slave->speed != SPEED_UNKNOWN)
@@ -3864,7 +3827,6 @@ static int bond_ethtool_get_settings(struct net_device *bond_dev,
 		}
 	}
 	ethtool_cmd_speed_set(ecmd, speed ? : SPEED_UNKNOWN);
-	read_unlock(&bond->lock);
 
 	return 0;
 }
@@ -3927,7 +3889,6 @@ void bond_setup(struct net_device *bond_dev)
 	struct bonding *bond = netdev_priv(bond_dev);
 
 	/* initialize rwlocks */
-	rwlock_init(&bond->lock);
 	rwlock_init(&bond->curr_slave_lock);
 	bond->params = bonding_defaults;
 
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index aace510d08d1..1f44a78283ad 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -83,7 +83,7 @@
  * @pos:	current slave
  * @iter:	list_head * iterator
  *
- * Caller must hold bond->lock
+ * Caller must hold RTNL
  */
 #define bond_for_each_slave(bond, pos, iter) \
 	netdev_for_each_lower_private((bond)->dev, pos, iter)
@@ -185,11 +185,8 @@ struct slave {
 /*
  * Here are the locking policies for the two bonding locks:
  *
- * 1) Get bond->lock when reading/writing slave list.
+ * 1) Get rcu_read_lock when reading or RTNL when writing slave list.
  * 2) Get bond->curr_slave_lock when reading/writing bond->curr_active_slave.
- *    (It is unnecessary when the write-lock is put with bond->lock.)
- * 3) When we lock with bond->curr_slave_lock, we must lock with bond->lock
- *    beforehand.
  */
 struct bonding {
 	struct   net_device *dev; /* first - useful for panic debug */
@@ -200,7 +197,6 @@ struct bonding {
 	s32      slave_cnt; /* never change this value outside the attach/detach wrappers */
 	int     (*recv_probe)(const struct sk_buff *, struct bonding *,
 			      struct slave *);
-	rwlock_t lock;
 	rwlock_t curr_slave_lock;
 	u8	 send_peer_notif;
 	u8       igmp_retrans;
-- 
1.9.3

^ permalink raw reply related

* [RFC net-next 3/5] bonding: procfs: clean bond->lock usage and use RTNL
From: Nikolay Aleksandrov @ 2014-09-05 18:16 UTC (permalink / raw)
  To: netdev; +Cc: vfalico, j.vosburgh, andy, davem, Nikolay Aleksandrov
In-Reply-To: <1409941011-5494-1-git-send-email-nikolay@redhat.com>

Use RTNL because bond->primary_slave might change otherwise.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_procfs.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index de62c0385dfb..eee937afe94e 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -7,7 +7,6 @@
 
 static void *bond_info_seq_start(struct seq_file *seq, loff_t *pos)
 	__acquires(RCU)
-	__acquires(&bond->lock)
 {
 	struct bonding *bond = seq->private;
 	struct list_head *iter;
@@ -15,8 +14,8 @@ static void *bond_info_seq_start(struct seq_file *seq, loff_t *pos)
 	loff_t off = 0;
 
 	/* make sure the bond won't be taken away */
+	rtnl_lock();
 	rcu_read_lock();
-	read_lock(&bond->lock);
 
 	if (*pos == 0)
 		return SEQ_START_TOKEN;
@@ -53,13 +52,10 @@ static void *bond_info_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 }
 
 static void bond_info_seq_stop(struct seq_file *seq, void *v)
-	__releases(&bond->lock)
 	__releases(RCU)
 {
-	struct bonding *bond = seq->private;
-
-	read_unlock(&bond->lock);
 	rcu_read_unlock();
+	rtnl_unlock();
 }
 
 static void bond_info_show_master(struct seq_file *seq)
-- 
1.9.3

^ permalink raw reply related

* [RFC net-next 4/5] bonding: options: remove bond->lock usage
From: Nikolay Aleksandrov @ 2014-09-05 18:16 UTC (permalink / raw)
  To: netdev; +Cc: vfalico, j.vosburgh, andy, davem, Nikolay Aleksandrov
In-Reply-To: <1409941011-5494-1-git-send-email-nikolay@redhat.com>

We're safe to remove the bond->lock use from the arp targets because
arp_rcv_probe no longer acquires bond->lock, only rcu_read_lock.
Also setting the primary slave is safe because noone uses the bond->lock
as a syncing mechanism for that anymore.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_options.c | 19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index d8dc17faa6b4..67a1c9bcdc43 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -955,14 +955,7 @@ static int _bond_option_arp_ip_target_add(struct bonding *bond, __be32 target)
 
 static int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target)
 {
-	int ret;
-
-	/* not to race with bond_arp_rcv */
-	write_lock_bh(&bond->lock);
-	ret = _bond_option_arp_ip_target_add(bond, target);
-	write_unlock_bh(&bond->lock);
-
-	return ret;
+	return _bond_option_arp_ip_target_add(bond, target);
 }
 
 static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
@@ -991,9 +984,6 @@ static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
 
 	netdev_info(bond->dev, "Removing ARP target %pI4\n", &target);
 
-	/* not to race with bond_arp_rcv */
-	write_lock_bh(&bond->lock);
-
 	bond_for_each_slave(bond, slave, iter) {
 		targets_rx = slave->target_last_arp_rx;
 		for (i = ind; (i < BOND_MAX_ARP_TARGETS-1) && targets[i+1]; i++)
@@ -1004,8 +994,6 @@ static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
 		targets[i] = targets[i+1];
 	targets[i] = 0;
 
-	write_unlock_bh(&bond->lock);
-
 	return 0;
 }
 
@@ -1013,11 +1001,8 @@ void bond_option_arp_ip_targets_clear(struct bonding *bond)
 {
 	int i;
 
-	/* not to race with bond_arp_rcv */
-	write_lock_bh(&bond->lock);
 	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
 		_bond_options_arp_ip_target_set(bond, i, 0, 0);
-	write_unlock_bh(&bond->lock);
 }
 
 static int bond_option_arp_ip_targets_set(struct bonding *bond,
@@ -1081,7 +1066,6 @@ static int bond_option_primary_set(struct bonding *bond,
 	struct slave *slave;
 
 	block_netpoll_tx();
-	read_lock(&bond->lock);
 	write_lock_bh(&bond->curr_slave_lock);
 
 	p = strchr(primary, '\n');
@@ -1120,7 +1104,6 @@ static int bond_option_primary_set(struct bonding *bond,
 
 out:
 	write_unlock_bh(&bond->curr_slave_lock);
-	read_unlock(&bond->lock);
 	unblock_netpoll_tx();
 
 	return 0;
-- 
1.9.3

^ permalink raw reply related

* [RFC net-next 2/5] bonding: alb: clean bond->lock
From: Nikolay Aleksandrov @ 2014-09-05 18:16 UTC (permalink / raw)
  To: netdev; +Cc: vfalico, j.vosburgh, andy, davem, Nikolay Aleksandrov
In-Reply-To: <1409941011-5494-1-git-send-email-nikolay@redhat.com>

We can remove the lock/unlock as it's no longer necessary since
RTNL should be held while calling bond_alb_set_mac_address().

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_alb.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 73c21e233131..028496205f39 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1775,8 +1775,7 @@ void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char
  * Set the bond->curr_active_slave to @new_slave and handle
  * mac address swapping and promiscuity changes as needed.
  *
- * If new_slave is NULL, caller must hold curr_slave_lock or
- * bond->lock for write.
+ * If new_slave is NULL, caller must hold curr_slave_lock for write
  *
  * If new_slave is not NULL, caller must hold RTNL, curr_slave_lock
  * for write.  Processing here may sleep, so no other locks may be held.
@@ -1857,12 +1856,8 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
 	write_lock_bh(&bond->curr_slave_lock);
 }
 
-/*
- * Called with RTNL
- */
+/* Called with RTNL */
 int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr)
-	__acquires(&bond->lock)
-	__releases(&bond->lock)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 	struct sockaddr *sa = addr;
@@ -1895,14 +1890,12 @@ int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr)
 	} else {
 		alb_set_slave_mac_addr(curr_active, bond_dev->dev_addr);
 
-		read_lock(&bond->lock);
 		alb_send_learning_packets(curr_active,
 					  bond_dev->dev_addr, false);
 		if (bond->alb_info.rlb_enabled) {
 			/* inform clients mac address has changed */
 			rlb_req_update_slave_clients(bond, curr_active);
 		}
-		read_unlock(&bond->lock);
 	}
 
 	return 0;
-- 
1.9.3

^ permalink raw reply related

* [RFC net-next 0/5] bonding: get rid of bond->lock
From: Nikolay Aleksandrov @ 2014-09-05 18:16 UTC (permalink / raw)
  To: netdev; +Cc: vfalico, j.vosburgh, andy, davem, Nikolay Aleksandrov

Hi all,
This is only a raw version of the idea, I've only compile-tested this
patch-set but I think it's possible to remove bond->lock completely as
its current users are only few and can be either removed or converted.
I'm sending it early as RFC to see if you have any suggestions, comments
or objections, and I'll do stress-testing and lockdep next week and
resubmit it with any changes/fixes that come up in the process.

If this gets accepted in some form, a re-work of curr_slave_lock will be
following as it can be dropped in a lot of places and only kept to sync the
few users that actually need it (e.g. 3ad wq handler and slave release, alb
wq handler and active change etc).
The goal of these patchsets is to simplify bond locking as it's getting
more convoluted and unnecessarily complex, in most of the cases we should
be able to rely on RTNL or rcu alone, and if the case is more complex we
should explain/document well why we need another lock.

Best regards,
 Nikolay Aleksandrov

Nikolay Aleksandrov (5):
  bonding: 3ad: use curr_slave_lock instead of bond->lock
  bonding: alb: clean bond->lock
  bonding: procfs: clean bond->lock usage and use RTNL
  bonding: options: remove bond->lock usage
  bonding: remove last users of bond->lock and bond->lock itself

 drivers/net/bonding/bond_3ad.c     |  9 +++----
 drivers/net/bonding/bond_alb.c     | 11 ++------
 drivers/net/bonding/bond_main.c    | 51 ++++++--------------------------------
 drivers/net/bonding/bond_options.c | 19 +-------------
 drivers/net/bonding/bond_procfs.c  |  8 ++----
 drivers/net/bonding/bonding.h      |  8 ++----
 6 files changed, 18 insertions(+), 88 deletions(-)

-- 
1.9.3

^ permalink raw reply

* [RFC net-next 1/5] bonding: 3ad: use curr_slave_lock instead of bond->lock
From: Nikolay Aleksandrov @ 2014-09-05 18:16 UTC (permalink / raw)
  To: netdev; +Cc: vfalico, j.vosburgh, andy, davem, Nikolay Aleksandrov
In-Reply-To: <1409941011-5494-1-git-send-email-nikolay@redhat.com>

In 3ad mode the only syncing needed by bond->lock is for the wq
and the recv handler, so change them to use curr_slave_lock.
There're no locking dependencies here as 3ad doesn't use
curr_slave_lock at all.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_3ad.c  |  9 ++++-----
 drivers/net/bonding/bond_main.c | 12 +++++++-----
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index ee2c73a9de39..5d27a6207384 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2057,7 +2057,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 	struct port *port;
 	bool should_notify_rtnl = BOND_SLAVE_NOTIFY_LATER;
 
-	read_lock(&bond->lock);
+	read_lock(&bond->curr_slave_lock);
 	rcu_read_lock();
 
 	/* check if there are any slaves */
@@ -2120,7 +2120,7 @@ re_arm:
 		}
 	}
 	rcu_read_unlock();
-	read_unlock(&bond->lock);
+	read_unlock(&bond->curr_slave_lock);
 
 	if (should_notify_rtnl && rtnl_trylock()) {
 		bond_slave_state_notify(bond);
@@ -2395,7 +2395,6 @@ int __bond_3ad_get_active_agg_info(struct bonding *bond,
 	return 0;
 }
 
-/* Wrapper used to hold bond->lock so no slave manipulation can occur */
 int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
 {
 	int ret;
@@ -2487,9 +2486,9 @@ int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
 	if (!lacpdu)
 		return ret;
 
-	read_lock(&bond->lock);
+	read_lock(&bond->curr_slave_lock);
 	ret = bond_3ad_rx_indication(lacpdu, slave, skb->len);
-	read_unlock(&bond->lock);
+	read_unlock(&bond->curr_slave_lock);
 	return ret;
 }
 
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index f0f5eab0fab1..dcd331bd0c17 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1687,13 +1687,15 @@ static int __bond_release_one(struct net_device *bond_dev,
 	 * for this slave anymore.
 	 */
 	netdev_rx_handler_unregister(slave_dev);
-	write_lock_bh(&bond->lock);
 
-	/* Inform AD package of unbinding of slave. */
-	if (BOND_MODE(bond) == BOND_MODE_8023AD)
+	if (BOND_MODE(bond) == BOND_MODE_8023AD) {
+		/* Sync against bond_3ad_rx_indication and
+		 * bond_3ad_state_machine_handler
+		 */
+		write_lock_bh(&bond->curr_slave_lock);
 		bond_3ad_unbind_slave(slave);
-
-	write_unlock_bh(&bond->lock);
+		write_unlock_bh(&bond->curr_slave_lock);
+	}
 
 	netdev_info(bond_dev, "Releasing %s interface %s\n",
 		    bond_is_active_slave(slave) ? "active" : "backup",
-- 
1.9.3

^ 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