* Re: [PATCH v3 4/4] posix-timers: make it configurable
From: Nicolas Pitre @ 2016-11-08 18:19 UTC (permalink / raw)
To: John Stultz
Cc: Michal Marek, Richard Cochran, Paul Bolle, Thomas Gleixner,
Josh Triplett, Edward Cree, netdev, linux-kbuild, lkml
In-Reply-To: <CALAqxLVrGRW3ODzZk=4-8FSLe5wBwHJSOEAW=+kMFjTRoe1HzQ@mail.gmail.com>
On Tue, 8 Nov 2016, John Stultz wrote:
> One spot of concern is that the
> tools/testing/selftests/timers/posix_timers.c test hangs testing
> virtual itimers. Looking through the code I'm not seeing where an
> error case is missed.
>
> The strace looks like:
> ...
> write(1, "Testing posix timers. False nega"..., 66Testing posix
> timers. False negative may happen on CPU execution
> ) = 66
> write(1, "based timers if other threads ru"..., 48based timers if
> other threads run on the CPU...
> ) = 48
> write(1, "Check itimer virtual... ", 24Check itimer virtual... ) = 24
> rt_sigaction(SIGVTALRM, {0x400a80, [VTALRM], SA_RESTORER|SA_RESTART,
> 0x7fb73306ccb0}, {SIG_DFL, [], 0}, 8) = 0
> gettimeofday({1478710402, 937476}, NULL) = 0
> setitimer(ITIMER_VIRTUAL, {it_interval={0, 0}, it_value={2, 0}}, NULL) = 0
> <Hang>
>
>
> Where as with posix timers enabled:
> ...
> write(1, "Testing posix timers. False nega"..., 138Testing posix
> timers. False negative may happen on CPU execution
> based timers if other threads run on the CPU...
> Check itimer virtual... ) = 138
> rt_sigaction(SIGVTALRM, {0x400a80, [VTALRM], SA_RESTORER|SA_RESTART,
> 0x7f231ba8ccb0}, {SIG_DFL, [], 0}, 8) = 0
> gettimeofday({1478626751, 904856}, NULL) = 0
> setitimer(ITIMER_VIRTUAL, {it_interval={0, 0}, it_value={2, 0}}, NULL) = 0
> --- SIGVTALRM {si_signo=SIGVTALRM, si_code=SI_KERNEL} ---
> rt_sigreturn() = 0
I'll have a look.
> So I suspect you were a little too aggressive with the #ifdefs around
> the itimers/signal code, or we need to make sure we return an error on
> the setitimer ITIMER_VIRTUAL case as well.
Well, it seemed to me that with POSIX_TIMERS=n, all the code that would
set up that signal is gone, so there was no point keeping the code to
deliver it.
Now... would it make more sense to remove itimer support as well when
POSIX_TIMERS=n? The same reasoning would apply.
Nicolas
^ permalink raw reply
* RE: Is there a maximum bytes in flight limitation in the tcp stack? -->limit in scp
From: De Schepper, Koen (Nokia - BE) @ 2016-11-08 18:07 UTC (permalink / raw)
To: Yuchung Cheng; +Cc: netdev@vger.kernel.org
Seems to be a limitation in the application. We used scp, and it (still) seems to limit the bytes in flight. Using our own application, we didn't see a limit indeed. Thanks for your response, and sorry for the noise...
Koen.
> -----Original Message-----
> From: Yuchung Cheng [mailto:ycheng@google.com]
> Sent: dinsdag 8 november 2016 5:51
> To: De Schepper, Koen (Nokia - BE) <koen.de_schepper@nokia-bell-
> labs.com>
> Cc: netdev@vger.kernel.org
> Subject: Re: Is there a maximum bytes in flight limitation in the tcp stack?
>
> On Thu, Nov 3, 2016 at 9:37 AM, De Schepper, Koen (Nokia - BE)
> <koen.de_schepper@nokia-bell-labs.com> wrote:
> >
> > Hi,
> >
> > We experience some limit on the maximum packets in flight which seem
> not to be related with the receive or write buffers. Does somebody know if
> there is an issue with a maximum of around 1MByte (or sometimes 2Mbyte)
> of data in flight per TCP flow?
>
> does not ring a bell. I've definitely see cubic reaching >2MB cwnd (inflight)
> some packet trace will help.
>
> btw, tcp_rmem is the maximum receive buffer including all header and
> control overhead. the receive window announced is (very roughly) half
> of your rcvbuf.
>
> >
> > It seems to be a strict and stable limit independent from the CC (tested
> with Cubic, Reno and DCTCP). On a link of 200Mbps and 200ms RTT our link is
> only 20% (sometimes 40%, see conditions below) utilized for a single TCP
> flow with no drop experienced at all (no bottleneck in the AQM or RTT
> emulation, as it supports more throughput if multiple flows are active).
> >
> > Some configuration changes we already tried on both client and server
> (kernel 3.18.9):
> >
> > net.ipv4.tcp_no_metrics_save = 1
> > net.ipv4.tcp_rmem = 4096 87380 6291456
> > net.ipv4.tcp_wmem = 4096 16384 4194304
> >
> > SERVER# ss -i
> > tcp ESTAB 0 1049728 10.187.255.211:46642 10.187.16.194:ssh
> > dctcp wscale:7,7 rto:408 rtt:204.333/0.741 ato:40 mss:1448 cwnd:1466
> send 83.1Mbps unacked:728 rcv_rtt:212 rcv_space:29200
> > CLIENT# ss -i
> > tcp ESTAB 0 288 10.187.16.194:ssh 10.187.255.211:46642
> > dctcp wscale:7,7 rto:404 rtt:203.389/0.213 ato:40 mss:1448 cwnd:78
> send 4.4Mbps unacked:8 rcv_rtt:204 rcv_space:1074844
> >
> > When increasing the write and receive mem further (they were already
> way above 1 or 2 MB) it steps to double (40%; 2Mbytes in flight):
> > net.ipv4.tcp_no_metrics_save = 1
> > net.ipv4.tcp_rmem = 4096 8000000 16291456
> > net.ipv4.tcp_wmem = 4096 8000000 16291456
> >
> > SERVER # ss -i
> > tcp ESTAB 0 2068976 10.187.255.212:54637 10.187.16.112:ssh
> > cubic wscale:8,8 rto:404 rtt:202.622/0.061 ato:40 mss:1448 cwnd:1849
> ssthresh:1140 send 105.7Mbps unacked:1457 rcv_rtt:217.5 rcv_space:29200
> > CLIENT# ss -i
> > tcp ESTAB 0 648 10.187.16.112:ssh 10.187.255.212:54637
> > cubic wscale:8,8 rto:404 rtt:201.956/0.038 ato:40 mss:1448 cwnd:132
> send 7.6Mbps unacked:18 rcv_rtt:204 rcv_space:2093044
> >
> > Further increasing (x10) does not help anymore...
> > net.ipv4.tcp_no_metrics_save = 1
> > net.ipv4.tcp_rmem = 4096 80000000 162914560
> > net.ipv4.tcp_wmem = 4096 80000000 162914560
> >
> > As all these parameters autotune, it is hard to find out which one is
> limiting... In the examples, above unacked does not want to go higher, while
> congestion window in the server is big enough... rcv_space could be limiting,
> but it tunes up if I change the server with the higher buffers (switching to
> 2MByte in flight).
> >
> > We also tried tcp_limit_output_bytes, setting it bigger (x10) and
> smaller(/10), without effect. We've put it in /etc/sysctl.conf and rebooted, to
> make sure that it is effective.
> >
> > Some more detailed tests that had an effect on the 1 or 2MByte:
> > - It seems that with TSO off, if we configure a bigger wmem buffer, an
> ongoing flow suddenly is able to immediately double its bytes in flight limit.
> We configured further up to more than 10x the buffer, but no further
> increase helps, and the limits we saw are only 1MByte and 2Mbyte (no
> intermediate values depending on any parameter). When setting tcp_wmem
> smaller again, the 2MByte limit stays on the ongoing flow. We have to restart
> the flow to make the buffer reduction to 1MByte effective.
> > - With TSO on, only the 2MByte limit is effective, independent from the
> wmem buffer. We have to restart the flow to make a tso change effective.
> >
> > Koen.
> >
^ permalink raw reply
* Re: net/sunrpc/clnt.c:2773 suspicious rcu_dereference_check() usage!
From: Anna Schumaker @ 2016-11-08 17:56 UTC (permalink / raw)
To: Ross Zwisler
Cc: Jeff Layton, Trond Myklebust, J. Bruce Fields, David S. Miller,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andy Adamson
In-Reply-To: <20161108174346.GA17545-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
On 11/08/2016 12:43 PM, Ross Zwisler wrote:
> On Tue, Nov 08, 2016 at 07:42:59AM -0500, Anna Schumaker wrote:
>> On 11/08/2016 07:09 AM, Jeff Layton wrote:
>>> On Tue, 2016-11-08 at 06:53 -0500, Jeff Layton wrote:
>>>> On Mon, 2016-11-07 at 22:42 -0700, Ross Zwisler wrote:
>>>>>
>>>>> I've got a virtual machine that has some NFS mounts, and with a newly compiled
>>>>> kernel based on v4.9-rc3 I see the following warning/info message:
>>>>>
>>>>> [ 42.750181] ===============================
>>>>> [ 42.750192] [ INFO: suspicious RCU usage. ]
>>>>> [ 42.750203] 4.9.0-rc3-00002-g7b6e7de #3 Not tainted
>>>>> [ 42.750213] -------------------------------
>>>>> [ 42.750225] net/sunrpc/clnt.c:2773 suspicious rcu_dereference_check() usage!
>>>>> [ 42.750235]
>>>>> [ 42.750235] other info that might help us debug this:
>>>>> [ 42.750235]
>>>>> [ 42.750246]
>>>>> [ 42.750246] rcu_scheduler_active = 1, debug_locks = 0
>>>>> [ 42.750257] 1 lock held by mount.nfs4/6440:
>>>>> [ 42.750278] #0:
>>>>> [ 42.750299] (
>>>>> [ 42.750319] &(&nn->nfs_client_lock)->rlock
>>>>> [ 42.750340] ){+.+...}
>>>>> [ 42.750362] , at:
>>>>> [ 42.750372] [<ffffffff813012b5>] nfs_get_client+0x105/0x5e0
>>>>> [ 42.750383]
>>>>> [ 42.750383] stack backtrace:
>>>>> [ 42.750394] CPU: 0 PID: 6440 Comm: mount.nfs4 Not tainted 4.9.0-rc3-00002-g7b6e7de #3
>>>>> [ 42.750406] Hardware name: Intel Corporation PURLEY/PURLEY, BIOS PLYDCRB1.MBH.0096.D23.1608240105 08/24/2016
>>>>> [ 42.750429] ffffc9000092fa68 ffffffff8150730f ffff88014ec8da40 0000000000000001
>>>>> [ 42.750452] ffffc9000092fa98 ffffffff810bc3f7 ffff880150b0b228 ffff88015068dbb0
>>>>> [ 42.750475] ffffc9000092fb38 ffff88014fc99180 ffffc9000092fac0 ffffffff81b243e5
>>>>> [ 42.750486] Call Trace:
>>>>> [ 42.750498] [<ffffffff8150730f>] dump_stack+0x67/0x98
>>>>> [ 42.750511] [<ffffffff810bc3f7>] lockdep_rcu_suspicious+0xe7/0x120
>>>>> [ 42.750524] [<ffffffff81b243e5>] rpc_clnt_xprt_switch_has_addr+0x115/0x150
>>>>> [ 42.750536] [<ffffffff813013f4>] nfs_get_client+0x244/0x5e0
>>>>> [ 42.750549] [<ffffffff813012ac>] ? nfs_get_client+0xfc/0x5e0
>>>>> [ 42.750561] [<ffffffff813568f8>] nfs4_set_client+0x98/0x130
>>>>> [ 42.750574] [<ffffffff8135872e>] nfs4_create_server+0x13e/0x390
>>>>> [ 42.750588] [<ffffffff8134cd0e>] nfs4_remote_mount+0x2e/0x60
>>>>> [ 42.750600] [<ffffffff811f3a29>] mount_fs+0x39/0x170
>>>>> [ 42.750614] [<ffffffff81214a0b>] vfs_kern_mount+0x6b/0x150
>>>>> [ 42.750626] [<ffffffff8134cbec>] ? nfs_do_root_mount+0x3c/0xc0
>>>>> [ 42.750639] [<ffffffff8134cc36>] nfs_do_root_mount+0x86/0xc0
>>>>> [ 42.750652] [<ffffffff8134d014>] nfs4_try_mount+0x44/0xc0
>>>>> [ 42.750664] [<ffffffff81302097>] ? get_nfs_version+0x27/0x90
>>>>> [ 42.750677] [<ffffffff81310f8c>] nfs_fs_mount+0x4ac/0xd80
>>>>> [ 42.750689] [<ffffffff810bb938>] ? lockdep_init_map+0x88/0x1f0
>>>>> [ 42.750701] [<ffffffff81311ac0>] ? nfs_clone_super+0x130/0x130
>>>>> [ 42.750713] [<ffffffff8130f300>] ? param_set_portnr+0x70/0x70
>>>>> [ 42.750726] [<ffffffff811f3a29>] mount_fs+0x39/0x170
>>>>> [ 42.750740] [<ffffffff81214a0b>] vfs_kern_mount+0x6b/0x150
>>>>> [ 42.750752] [<ffffffff812176f1>] do_mount+0x1f1/0xd10
>>>>> [ 42.750765] [<ffffffff81217441>] ? copy_mount_options+0xa1/0x140
>>>>> [ 42.750777] [<ffffffff81218543>] SyS_mount+0x83/0xd0
>>>>> [ 42.750790] [<ffffffff81002abc>] do_syscall_64+0x5c/0x130
>>>>> [ 42.750802] [<ffffffff81c479a4>] entry_SYSCALL64_slow_path+0x25/0x25
>>>>>
>>>>> This rcu_dereference_check() was introduced by the following commit:
>>>>>
>>>>> commit 39e5d2df959dd4aea81fa33d765d2a5cc67a0512
>>>>> Author: Andy Adamson <andros-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
>>>>> Date: Fri Sep 9 09:22:25 2016 -0400
>>>>>
>>>>> SUNRPC search xprt switch for sockaddr
>>>>>
>>>>> Signed-off-by: Andy Adamson <andros-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
>>>>> Signed-off-by: Anna Schumaker <Anna.Schumaker-ZwjVKphTwtPQT0dZR+AlfA@public.gmane.org>
>>>>>
>>>>> Thanks,
>>>>> - Ross
>>>>
>>>> Thanks Ross,
>>
>> Hi Ross,
>>
>> Can you try this patch and let me know if it helps:
>>
>> http://git.linux-nfs.org/?p=anna/linux-nfs.git;a=commitdiff;h=bb29dd84333a96f309c6d0f88b285b5b78927058
>>
>> I'm planning on sending it to Linus soon, so it should be in rc5.
>
> Hi Anna,
>
> Yep, this patch makes the warning go away in my setup.
Great! Thanks for testing!
Anna
>
> Thanks,
> - Ross
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: net/sunrpc/clnt.c:2773 suspicious rcu_dereference_check() usage!
From: Ross Zwisler @ 2016-11-08 17:43 UTC (permalink / raw)
To: Anna Schumaker
Cc: Jeff Layton, Ross Zwisler, Trond Myklebust, J. Bruce Fields,
David S. Miller, linux-nfs, netdev, linux-kernel, Andy Adamson
In-Reply-To: <60a0f29b-7a0a-c5e2-0e98-fa9a923dd339@Netapp.com>
On Tue, Nov 08, 2016 at 07:42:59AM -0500, Anna Schumaker wrote:
> On 11/08/2016 07:09 AM, Jeff Layton wrote:
> > On Tue, 2016-11-08 at 06:53 -0500, Jeff Layton wrote:
> >> On Mon, 2016-11-07 at 22:42 -0700, Ross Zwisler wrote:
> >>>
> >>> I've got a virtual machine that has some NFS mounts, and with a newly compiled
> >>> kernel based on v4.9-rc3 I see the following warning/info message:
> >>>
> >>> [ 42.750181] ===============================
> >>> [ 42.750192] [ INFO: suspicious RCU usage. ]
> >>> [ 42.750203] 4.9.0-rc3-00002-g7b6e7de #3 Not tainted
> >>> [ 42.750213] -------------------------------
> >>> [ 42.750225] net/sunrpc/clnt.c:2773 suspicious rcu_dereference_check() usage!
> >>> [ 42.750235]
> >>> [ 42.750235] other info that might help us debug this:
> >>> [ 42.750235]
> >>> [ 42.750246]
> >>> [ 42.750246] rcu_scheduler_active = 1, debug_locks = 0
> >>> [ 42.750257] 1 lock held by mount.nfs4/6440:
> >>> [ 42.750278] #0:
> >>> [ 42.750299] (
> >>> [ 42.750319] &(&nn->nfs_client_lock)->rlock
> >>> [ 42.750340] ){+.+...}
> >>> [ 42.750362] , at:
> >>> [ 42.750372] [<ffffffff813012b5>] nfs_get_client+0x105/0x5e0
> >>> [ 42.750383]
> >>> [ 42.750383] stack backtrace:
> >>> [ 42.750394] CPU: 0 PID: 6440 Comm: mount.nfs4 Not tainted 4.9.0-rc3-00002-g7b6e7de #3
> >>> [ 42.750406] Hardware name: Intel Corporation PURLEY/PURLEY, BIOS PLYDCRB1.MBH.0096.D23.1608240105 08/24/2016
> >>> [ 42.750429] ffffc9000092fa68 ffffffff8150730f ffff88014ec8da40 0000000000000001
> >>> [ 42.750452] ffffc9000092fa98 ffffffff810bc3f7 ffff880150b0b228 ffff88015068dbb0
> >>> [ 42.750475] ffffc9000092fb38 ffff88014fc99180 ffffc9000092fac0 ffffffff81b243e5
> >>> [ 42.750486] Call Trace:
> >>> [ 42.750498] [<ffffffff8150730f>] dump_stack+0x67/0x98
> >>> [ 42.750511] [<ffffffff810bc3f7>] lockdep_rcu_suspicious+0xe7/0x120
> >>> [ 42.750524] [<ffffffff81b243e5>] rpc_clnt_xprt_switch_has_addr+0x115/0x150
> >>> [ 42.750536] [<ffffffff813013f4>] nfs_get_client+0x244/0x5e0
> >>> [ 42.750549] [<ffffffff813012ac>] ? nfs_get_client+0xfc/0x5e0
> >>> [ 42.750561] [<ffffffff813568f8>] nfs4_set_client+0x98/0x130
> >>> [ 42.750574] [<ffffffff8135872e>] nfs4_create_server+0x13e/0x390
> >>> [ 42.750588] [<ffffffff8134cd0e>] nfs4_remote_mount+0x2e/0x60
> >>> [ 42.750600] [<ffffffff811f3a29>] mount_fs+0x39/0x170
> >>> [ 42.750614] [<ffffffff81214a0b>] vfs_kern_mount+0x6b/0x150
> >>> [ 42.750626] [<ffffffff8134cbec>] ? nfs_do_root_mount+0x3c/0xc0
> >>> [ 42.750639] [<ffffffff8134cc36>] nfs_do_root_mount+0x86/0xc0
> >>> [ 42.750652] [<ffffffff8134d014>] nfs4_try_mount+0x44/0xc0
> >>> [ 42.750664] [<ffffffff81302097>] ? get_nfs_version+0x27/0x90
> >>> [ 42.750677] [<ffffffff81310f8c>] nfs_fs_mount+0x4ac/0xd80
> >>> [ 42.750689] [<ffffffff810bb938>] ? lockdep_init_map+0x88/0x1f0
> >>> [ 42.750701] [<ffffffff81311ac0>] ? nfs_clone_super+0x130/0x130
> >>> [ 42.750713] [<ffffffff8130f300>] ? param_set_portnr+0x70/0x70
> >>> [ 42.750726] [<ffffffff811f3a29>] mount_fs+0x39/0x170
> >>> [ 42.750740] [<ffffffff81214a0b>] vfs_kern_mount+0x6b/0x150
> >>> [ 42.750752] [<ffffffff812176f1>] do_mount+0x1f1/0xd10
> >>> [ 42.750765] [<ffffffff81217441>] ? copy_mount_options+0xa1/0x140
> >>> [ 42.750777] [<ffffffff81218543>] SyS_mount+0x83/0xd0
> >>> [ 42.750790] [<ffffffff81002abc>] do_syscall_64+0x5c/0x130
> >>> [ 42.750802] [<ffffffff81c479a4>] entry_SYSCALL64_slow_path+0x25/0x25
> >>>
> >>> This rcu_dereference_check() was introduced by the following commit:
> >>>
> >>> commit 39e5d2df959dd4aea81fa33d765d2a5cc67a0512
> >>> Author: Andy Adamson <andros@netapp.com>
> >>> Date: Fri Sep 9 09:22:25 2016 -0400
> >>>
> >>> SUNRPC search xprt switch for sockaddr
> >>>
> >>> Signed-off-by: Andy Adamson <andros@netapp.com>
> >>> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> >>>
> >>> Thanks,
> >>> - Ross
> >>
> >> Thanks Ross,
>
> Hi Ross,
>
> Can you try this patch and let me know if it helps:
>
> http://git.linux-nfs.org/?p=anna/linux-nfs.git;a=commitdiff;h=bb29dd84333a96f309c6d0f88b285b5b78927058
>
> I'm planning on sending it to Linus soon, so it should be in rc5.
Hi Anna,
Yep, this patch makes the warning go away in my setup.
Thanks,
- Ross
^ permalink raw reply
* Re: [PATCH 13/17] batman-adv: Consume skb in receive handlers
From: Eric Dumazet @ 2016-11-08 17:43 UTC (permalink / raw)
To: Sven Eckelmann
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <2044854.g075mifYzn@bentobox>
On Tue, 2016-11-08 at 18:28 +0100, Sven Eckelmann wrote:
> On Dienstag, 8. November 2016 08:59:49 CET Eric Dumazet wrote:
> [...]
> > > +free_skb:
> > > consume_skb(skb);
> > > - return NET_RX_SUCCESS;
> > > +
> > > + return ret;
> > > }
> >
> >
> > Okay, but we do have kfree_skb() and consume_skb() and they should be
> > used appropriately.
>
> Yes, this patch is one part of reaching this goal. Some other parts are also
> in this patchset. But other changes like the one you've mention here (change
> some consume_skb partially back to kfree_skb) have still to be done. But
> first we have to clean up the main portion of the mess :)
Sure, but your patch 13/17 should address this right away.
You must not call consume_skb() if you are dropping a packet.
Prior to this patch, kfree_skb() was properly called, and after this
patch, consume_skb() is called instead.
- ret = (*batadv_rx_handler[idx])(skb, hard_iface);
-
- if (ret == NET_RX_DROP)
- kfree_skb(skb);
+ (*batadv_rx_handler[idx])(skb, hard_iface);
You can not claim working on these issues and at the same time add them
back.
^ permalink raw reply
* Re: [PATCH v3 4/4] posix-timers: make it configurable
From: John Stultz @ 2016-11-08 17:42 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Michal Marek, Richard Cochran, Paul Bolle, Thomas Gleixner,
Josh Triplett, Edward Cree, netdev, linux-kbuild, lkml
In-Reply-To: <1478556899-2951-5-git-send-email-nicolas.pitre@linaro.org>
On Mon, Nov 7, 2016 at 2:14 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> Some embedded systems have no use for them. This removes about
> 22KB from the kernel binary size when configured out.
>
> Corresponding syscalls are routed to a stub logging the attempt to
> use those syscalls which should be enough of a clue if they were
> disabled without proper consideration. They are: timer_create,
> timer_gettime: timer_getoverrun, timer_settime, timer_delete,
> clock_adjtime.
>
> The clock_settime, clock_gettime, clock_getres and clock_nanosleep
> syscalls are replaced by simple wrappers compatible with CLOCK_REALTIME,
> CLOCK_MONOTONIC and CLOCK_BOOTTIME only which should cover the vast
> majority of use cases with very little code.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> Acked-by: Richard Cochran <richardcochran@gmail.com>
So I have no design objections to the patch overall.
I ran this through my timekeeping tests last night and it passed a
fair number of the tests, considering.
I of course see a lot of failures around timer_creates failing
(set-timer-lat), and cases where clockids aren't supported.
So I'll need to see about updating the tests to fail more gracefully
with this change.
One spot of concern is that the
tools/testing/selftests/timers/posix_timers.c test hangs testing
virtual itimers. Looking through the code I'm not seeing where an
error case is missed.
The strace looks like:
...
write(1, "Testing posix timers. False nega"..., 66Testing posix
timers. False negative may happen on CPU execution
) = 66
write(1, "based timers if other threads ru"..., 48based timers if
other threads run on the CPU...
) = 48
write(1, "Check itimer virtual... ", 24Check itimer virtual... ) = 24
rt_sigaction(SIGVTALRM, {0x400a80, [VTALRM], SA_RESTORER|SA_RESTART,
0x7fb73306ccb0}, {SIG_DFL, [], 0}, 8) = 0
gettimeofday({1478710402, 937476}, NULL) = 0
setitimer(ITIMER_VIRTUAL, {it_interval={0, 0}, it_value={2, 0}}, NULL) = 0
<Hang>
Where as with posix timers enabled:
...
write(1, "Testing posix timers. False nega"..., 138Testing posix
timers. False negative may happen on CPU execution
based timers if other threads run on the CPU...
Check itimer virtual... ) = 138
rt_sigaction(SIGVTALRM, {0x400a80, [VTALRM], SA_RESTORER|SA_RESTART,
0x7f231ba8ccb0}, {SIG_DFL, [], 0}, 8) = 0
gettimeofday({1478626751, 904856}, NULL) = 0
setitimer(ITIMER_VIRTUAL, {it_interval={0, 0}, it_value={2, 0}}, NULL) = 0
--- SIGVTALRM {si_signo=SIGVTALRM, si_code=SI_KERNEL} ---
rt_sigreturn() = 0
gettimeofday({1478626753, 906137}, NULL) = 0
write(1, "[OK]\nCheck itimer prof... ", 26[OK]
...
So I suspect you were a little too aggressive with the #ifdefs around
the itimers/signal code, or we need to make sure we return an error on
the setitimer ITIMER_VIRTUAL case as well.
thanks
-john
^ permalink raw reply
* Re: [PATCH 13/17] batman-adv: Consume skb in receive handlers
From: Sven Eckelmann @ 2016-11-08 17:28 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Simon Wunderlich, davem, netdev, b.a.t.m.a.n
In-Reply-To: <1478624389.17367.12.camel@edumazet-glaptop3.roam.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 574 bytes --]
On Dienstag, 8. November 2016 08:59:49 CET Eric Dumazet wrote:
[...]
> > +free_skb:
> > consume_skb(skb);
> > - return NET_RX_SUCCESS;
> > +
> > + return ret;
> > }
>
>
> Okay, but we do have kfree_skb() and consume_skb() and they should be
> used appropriately.
Yes, this patch is one part of reaching this goal. Some other parts are also
in this patchset. But other changes like the one you've mention here (change
some consume_skb partially back to kfree_skb) have still to be done. But
first we have to clean up the main portion of the mess :)
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH] igb: use igb_adapter->io_addr instead of e1000_hw->hw_addr
From: Hisashi T Fujinaka @ 2016-11-08 17:16 UTC (permalink / raw)
To: Corinna Vinschen
Cc: Cao jin, netdev, intel-wired-lan, linux-kernel, izumi.taku
In-Reply-To: <20161108164214.GF31855@calimero.vinschen.de>
On Tue, 8 Nov 2016, Corinna Vinschen wrote:
> On Nov 8 15:06, Cao jin wrote:
>> When running as guest, under certain condition, it will oops as following.
>> writel() in igb_configure_tx_ring() results in oops, because hw->hw_addr
>> is NULL. While other register access won't oops kernel because they use
>> wr32/rd32 which have a defense against NULL pointer.
>>
>> [ 141.225449] pcieport 0000:00:1c.0: AER: Multiple Uncorrected (Fatal)
>> error received: id=0101
>> [ 141.225523] igb 0000:01:00.1: PCIe Bus Error:
>> severity=Uncorrected (Fatal), type=Unaccessible,
>> id=0101(Unregistered Agent ID)
>> [ 141.299442] igb 0000:01:00.1: broadcast error_detected message
>> [ 141.300539] igb 0000:01:00.0 enp1s0f0: PCIe link lost, device now
>> detached
>> [ 141.351019] igb 0000:01:00.1 enp1s0f1: PCIe link lost, device now
>> detached
>> [ 143.465904] pcieport 0000:00:1c.0: Root Port link has been reset
>> [ 143.465994] igb 0000:01:00.1: broadcast slot_reset message
>> [ 143.466039] igb 0000:01:00.0: enabling device (0000 -> 0002)
>> [ 144.389078] igb 0000:01:00.1: enabling device (0000 -> 0002)
>> [ 145.312078] igb 0000:01:00.1: broadcast resume message
>> [ 145.322211] BUG: unable to handle kernel paging request at
>> 0000000000003818
>> [ 145.361275] IP: [<ffffffffa02fd38d>]
>> igb_configure_tx_ring+0x14d/0x280 [igb]
>> [ 145.400048] PGD 0
>> [ 145.438007] Oops: 0002 [#1] SMP
>>
>> A similiar issue & solution could be found at:
>> http://patchwork.ozlabs.org/patch/689592/
>>
>> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
>> ---
>> drivers/net/ethernet/intel/igb/igb_main.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
>> index edc9a6a..3f240ac 100644
>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>> @@ -3390,7 +3390,7 @@ void igb_configure_tx_ring(struct igb_adapter *adapter,
>> tdba & 0x00000000ffffffffULL);
>> wr32(E1000_TDBAH(reg_idx), tdba >> 32);
>>
>> - ring->tail = hw->hw_addr + E1000_TDT(reg_idx);
>> + ring->tail = adapter->io_addr + E1000_TDT(reg_idx);
>> wr32(E1000_TDH(reg_idx), 0);
>> writel(0, ring->tail);
>>
>> @@ -3729,7 +3729,7 @@ void igb_configure_rx_ring(struct igb_adapter *adapter,
>> ring->count * sizeof(union e1000_adv_rx_desc));
>>
>> /* initialize head and tail */
>> - ring->tail = hw->hw_addr + E1000_RDT(reg_idx);
>> + ring->tail = adapter->io_addr + E1000_RDT(reg_idx);
>> wr32(E1000_RDH(reg_idx), 0);
>> writel(0, ring->tail);
>>
>> --
>> 2.1.0
>
> Incidentally we're just looking for a solution to that problem too.
> Do three patches to fix the same problem at rougly the same time already
> qualify as freak accident?
>
> FTR, I attached my current patch, which I was planning to submit after
> some external testing.
>
> However, all three patches have one thing in common: They workaround
> a somewhat dubious resetting of the hardware address to NULL in case
> reading from a register failed.
>
> That makes me wonder if setting the hardware address to NULL in
> rd32/igb_rd32 is really such a good idea. It's performed in a function
> which return value is *never* tested for validity in the calling
> functions and leads to subsequent crashes since no tests for hw_addr ==
> NULL are performed.
>
> Maybe commit 22a8b2915 should be reconsidered? Isn't there some more
> graceful way to handle the "surprise removal"?
Answering this from my home account because, well, work is Outlook.
"Reconsidering" would be great. In fact, revert if if you'd like. I'm
uncertain that the surprise removal code actually works the way I
thought previously and I think I took a lot of it out of my local code.
Unfortuantely I don't have any equipment that I can use to reproduce
surprise removal any longer so that means I wouldn't be able to test
anything. I have to defer to you or Cao Jin.
--
Hisashi T Fujinaka - htodd@twofifty.com (todd.fujinaka@intel.com)
^ permalink raw reply
* Re: Why are IPv6 host and anycast routes referencing lo device?
From: Hannes Frederic Sowa @ 2016-11-08 17:08 UTC (permalink / raw)
To: David Ahern, YOSHIFUJI Hideaki, netdev@vger.kernel.org
In-Reply-To: <db922fb9-2553-7801-d597-0f40999be944@cumulusnetworks.com>
On 08.11.2016 02:08, David Ahern wrote:
>
> Can anyone explain why host routes and anycast routes for IPv6 are added with the device set to loopback versus the device with the address:
>
> local ::1 dev lo proto none metric 0 pref medium
> local 2000:1:: dev lo proto none metric 0 pref medium
> local 2000:1::3 dev lo proto none metric 0 pref medium
> local 2100:2:: dev lo proto none metric 0 pref medium
> local 2100:2::3 dev lo proto none metric 0 pref medium
Does it really matter? For global valid unicast addresses we still
implement the weak model. Thus the interface does not matter at all.
> This behavior differs from IPv4 where host routes use the device with the address:
>
> broadcast 10.1.1.0 dev eth0 proto kernel scope link src 10.1.1.3
> local 10.1.1.3 dev eth0 proto kernel scope host src 10.1.1.3
> broadcast 10.1.1.255 dev eth0 proto kernel scope link src 10.1.1.3
> broadcast 10.100.2.0 dev eth2 proto kernel scope link src 10.100.2.3
> local 10.100.2.3 dev eth2 proto kernel scope host src 10.100.2.3
> broadcast 10.100.2.255 dev eth2 proto kernel scope link src 10.100.2.3
>
> The use of loopback pre-dates the git history, so wondering if someone recalls the reason why. We would like to change that to make it consistent with IPv4 - with a sysctl to maintain backwards compatibility.
A sysctl for that sounds like a really bad idea.
Internally the sysctl will change the reference counting of interfaces
and routes towards each other, have small but difficult to find
semantically changes inside the kernel, just for switchting the
interface in iproute/netlink dumps?
If there a good reasons (which can very well be) to switch to have the
interface with the address in the routes, we should switch without
providing the backwards compatibility sysctl.
Bye,
Hannes
^ permalink raw reply
* Re: [PATCH 13/17] batman-adv: Consume skb in receive handlers
From: Eric Dumazet @ 2016-11-08 16:59 UTC (permalink / raw)
To: Simon Wunderlich
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20161108164526.7518-14-sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX@public.gmane.org>
On Tue, 2016-11-08 at 17:45 +0100, Simon Wunderlich wrote:
> From: Sven Eckelmann <sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org>
>
> Receiving functions in Linux consume the supplied skbuff. Doing the same in
> the batadv_rx_handler functions makes the behavior more similar to the rest
> of the Linux network code.
>
> Signed-off-by: Sven Eckelmann <sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org>
> Signed-off-by: Simon Wunderlich <sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX@public.gmane.org>
> ---
> net/batman-adv/bat_iv_ogm.c | 17 +++--
> net/batman-adv/bat_v_elp.c | 25 ++++---
> net/batman-adv/bat_v_ogm.c | 10 +--
> net/batman-adv/main.c | 11 +--
> net/batman-adv/network-coding.c | 11 +--
> net/batman-adv/routing.c | 149 +++++++++++++++++++++++++++-------------
> 6 files changed, 141 insertions(+), 82 deletions(-)
>
> diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
> index 310f391..b9941bf 100644
> --- a/net/batman-adv/bat_iv_ogm.c
> +++ b/net/batman-adv/bat_iv_ogm.c
> @@ -1823,17 +1823,18 @@ static int batadv_iv_ogm_receive(struct sk_buff *skb,
> struct batadv_ogm_packet *ogm_packet;
> u8 *packet_pos;
> int ogm_offset;
> - bool ret;
> + bool res;
> + int ret = NET_RX_DROP;
>
> - ret = batadv_check_management_packet(skb, if_incoming, BATADV_OGM_HLEN);
> - if (!ret)
> - return NET_RX_DROP;
> + res = batadv_check_management_packet(skb, if_incoming, BATADV_OGM_HLEN);
> + if (!res)
> + goto free_skb;
>
> /* did we receive a B.A.T.M.A.N. IV OGM packet on an interface
> * that does not have B.A.T.M.A.N. IV enabled ?
> */
> if (bat_priv->algo_ops->iface.enable != batadv_iv_ogm_iface_enable)
> - return NET_RX_DROP;
> + goto free_skb;
>
> batadv_inc_counter(bat_priv, BATADV_CNT_MGMT_RX);
> batadv_add_counter(bat_priv, BATADV_CNT_MGMT_RX_BYTES,
> @@ -1854,8 +1855,12 @@ static int batadv_iv_ogm_receive(struct sk_buff *skb,
> ogm_packet = (struct batadv_ogm_packet *)packet_pos;
> }
>
> + ret = NET_RX_SUCCESS;
> +
> +free_skb:
> consume_skb(skb);
> - return NET_RX_SUCCESS;
> +
> + return ret;
> }
Okay, but we do have kfree_skb() and consume_skb() and they should be
used appropriately.
^ permalink raw reply
* Re: [PATCH v3 0/4] make POSIX timers optional with some Kconfig help
From: Thomas Gleixner @ 2016-11-08 16:47 UTC (permalink / raw)
To: Nicolas Pitre
Cc: John Stultz, Michal Marek, Richard Cochran, Paul Bolle,
Josh Triplett, Edward Cree, netdev, linux-kbuild, linux-kernel
In-Reply-To: <1478556899-2951-1-git-send-email-nicolas.pitre@linaro.org>
On Mon, 7 Nov 2016, Nicolas Pitre wrote:
> Many embedded systems don't need the full POSIX timer support.
> Configuring them out provides a nice kernel image size reduction.
>
> When POSIX timers are configured out, the PTP clock subsystem should be
> left out as well. However a bunch of ethernet drivers currently *select*
> the later in their Kconfig entries. Therefore some more work was needed
> to break that hard dependency from those drivers without preventing their
> usage altogether.
>
> Therefore this series also includes kconfig changes to implement a new
> keyword to express some reverse dependencies like "select" does, named
> "imply", and still allowing for the target config symbol to be disabled
> if the user or a direct dependency says so.
>
> At this point I'd like to gather ACKs especially from people in the "To"
> field. Ideally this would need to go upstream as a single series to avoid
> cross subsystem dependency issues. So far it was suggested that this should go
> via the kbuild tree.
For the whole series:
Acked-by: Thomas Gleixner <tglx@linutronix.de>
^ permalink raw reply
* Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function
From: Bjørn Mork @ 2016-11-08 16:49 UTC (permalink / raw)
To: Alan Stern; +Cc: Kai-Heng Feng, Oliver Neukum, linux-kernel, linux-usb, netdev
In-Reply-To: <Pine.LNX.4.44L0.1611081020550.1499-100000@iolanthe.rowland.org>
Alan Stern <stern@rowland.harvard.edu> writes:
> On Tue, 8 Nov 2016, Kai-Heng Feng wrote:
>
>> Hi,
>>
>> On Mon, Nov 7, 2016 at 7:02 PM, Oliver Neukum <oneukum@suse.com> wrote:
>> > On Fri, 2016-11-04 at 17:57 +0800, Kai-Heng Feng wrote:
>> >> Sometimes cdc_mbim failed to probe if runtime pm is enabled:
>> >> [ 9.305626] cdc_mbim: probe of 2-2:1.12 failed with error -22
>> >>
>> >> This can be solved by increase its pm usage counter.
>> >>
>> >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> >
>> > For the record:
>> >
>> > NAK. This fixes a symptom. If this patch helps something is broken in
>> > device core. We need to find that.
>> >
>>
>> Please check attached dmesg with usbcore.dyndbg="+p".
>
> The log shows that the device went into suspend _before_ the cdc_mbim
> driver was probed, not during the probe. Then just before the probe
> was started, the USB core tried to resume the device and the resume
> failed.
>
> The log shows a bunch of other problems with this device:
>
> [ 3.862253] usb 2-2: config 1 has an invalid interface number: 12 but max is 1
> [ 3.862254] usb 2-2: config 1 has an invalid interface number: 13 but max is 1
> [ 3.862254] usb 2-2: config 1 has an invalid interface number: 13 but max is 1
> [ 3.862255] usb 2-2: config 1 has no interface number 0
> [ 3.862256] usb 2-2: config 1 has no interface number 1
These messages are completely harmless and normal for Sierra Wireless
devices. They use the interface number to identify the type of
function, causing this mismatch between the number of interfaces and the
inteface numbers. Boy, that looks weird in writing :)
Ref this discussion we had a few years ago:
http://www.spinics.net/lists/linux-usb/msg77499.html
No, I didn't expect you to remember that :)
> [ 8.295180] usb 2-2: Disable of device-initiated U1 failed.
> [ 8.295322] usb 2-2: Disable of device-initiated U2 failed.
>
> I get the impression that the device won't work properly with runtime
> PM at all.
I suspect the device is an EM7455? If so, then it does work fine with
runtime PM, as long as we're talking USB2. Not sure about USB3 runtime
PM though. Cannot test it. The Lenovo laptop I got with one of these
modems has disabled the USB3 link on the m.2 modem slot for some reason.
Bjørn
^ permalink raw reply
* [PATCH 16/17] batman-adv: Disallow zero and mcast src address for mgmt frames
From: Simon Wunderlich @ 2016-11-08 16:45 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Simon Wunderlich
In-Reply-To: <20161108164526.7518-1-sw@simonwunderlich.de>
From: Sven Eckelmann <sven@narfation.org>
The routing check for management frames is validating the source mac
address in the outer ethernet header. It rejects every source mac address
which is a broadcast address. But it also has to reject the zero-mac
address and multicast mac addresses.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/routing.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 9646623..381f040 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -196,8 +196,8 @@ bool batadv_check_management_packet(struct sk_buff *skb,
if (!is_broadcast_ether_addr(ethhdr->h_dest))
return false;
- /* packet with broadcast sender address */
- if (is_broadcast_ether_addr(ethhdr->h_source))
+ /* packet with invalid sender address */
+ if (!is_valid_ether_addr(ethhdr->h_source))
return false;
/* create a copy of the skb, if needed, to modify it. */
--
2.10.1
^ permalink raw reply related
* [PATCH 11/17] batman-adv: Consume skb in batadv_frag_send_packet
From: Simon Wunderlich @ 2016-11-08 16:45 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Simon Wunderlich
In-Reply-To: <20161108164526.7518-1-sw@simonwunderlich.de>
From: Sven Eckelmann <sven@narfation.org>
Sending functions in Linux consume the supplied skbuff. Doing the same in
batadv_frag_send_packet avoids the hack of returning -1 (-EPERM) to signal
the caller that he is responsible for cleaning up the skb.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/fragmentation.c | 50 ++++++++++++++++++++++++------------------
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c
index a2e28a1..9c561e6 100644
--- a/net/batman-adv/fragmentation.c
+++ b/net/batman-adv/fragmentation.c
@@ -20,6 +20,7 @@
#include <linux/atomic.h>
#include <linux/byteorder/generic.h>
+#include <linux/errno.h>
#include <linux/etherdevice.h>
#include <linux/fs.h>
#include <linux/if_ether.h>
@@ -441,8 +442,7 @@ static struct sk_buff *batadv_frag_create(struct sk_buff *skb,
* @orig_node: final destination of the created fragments
* @neigh_node: next-hop of the created fragments
*
- * Return: the netdev tx status or -1 in case of error.
- * When -1 is returned the skb is not consumed.
+ * Return: the netdev tx status or a negative errno code on a failure
*/
int batadv_frag_send_packet(struct sk_buff *skb,
struct batadv_orig_node *orig_node,
@@ -455,7 +455,7 @@ int batadv_frag_send_packet(struct sk_buff *skb,
unsigned int mtu = neigh_node->if_incoming->net_dev->mtu;
unsigned int header_size = sizeof(frag_header);
unsigned int max_fragment_size, max_packet_size;
- int ret = -1;
+ int ret;
/* To avoid merge and refragmentation at next-hops we never send
* fragments larger than BATADV_FRAG_MAX_FRAG_SIZE
@@ -465,13 +465,17 @@ int batadv_frag_send_packet(struct sk_buff *skb,
max_packet_size = max_fragment_size * BATADV_FRAG_MAX_FRAGMENTS;
/* Don't even try to fragment, if we need more than 16 fragments */
- if (skb->len > max_packet_size)
- goto out;
+ if (skb->len > max_packet_size) {
+ ret = -EAGAIN;
+ goto free_skb;
+ }
bat_priv = orig_node->bat_priv;
primary_if = batadv_primary_if_get_selected(bat_priv);
- if (!primary_if)
- goto out;
+ if (!primary_if) {
+ ret = -EINVAL;
+ goto put_primary_if;
+ }
/* Create one header to be copied to all fragments */
frag_header.packet_type = BATADV_UNICAST_FRAG;
@@ -496,34 +500,35 @@ int batadv_frag_send_packet(struct sk_buff *skb,
/* Eat and send fragments from the tail of skb */
while (skb->len > max_fragment_size) {
skb_fragment = batadv_frag_create(skb, &frag_header, mtu);
- if (!skb_fragment)
- goto out;
+ if (!skb_fragment) {
+ ret = -ENOMEM;
+ goto free_skb;
+ }
batadv_inc_counter(bat_priv, BATADV_CNT_FRAG_TX);
batadv_add_counter(bat_priv, BATADV_CNT_FRAG_TX_BYTES,
skb_fragment->len + ETH_HLEN);
ret = batadv_send_unicast_skb(skb_fragment, neigh_node);
if (ret != NET_XMIT_SUCCESS) {
- /* return -1 so that the caller can free the original
- * skb
- */
- ret = -1;
- goto out;
+ ret = NET_XMIT_DROP;
+ goto free_skb;
}
frag_header.no++;
/* The initial check in this function should cover this case */
if (frag_header.no == BATADV_FRAG_MAX_FRAGMENTS - 1) {
- ret = -1;
- goto out;
+ ret = -EINVAL;
+ goto free_skb;
}
}
/* Make room for the fragment header. */
if (batadv_skb_head_push(skb, header_size) < 0 ||
- pskb_expand_head(skb, header_size + ETH_HLEN, 0, GFP_ATOMIC) < 0)
- goto out;
+ pskb_expand_head(skb, header_size + ETH_HLEN, 0, GFP_ATOMIC) < 0) {
+ ret = -ENOMEM;
+ goto free_skb;
+ }
memcpy(skb->data, &frag_header, header_size);
@@ -532,10 +537,13 @@ int batadv_frag_send_packet(struct sk_buff *skb,
batadv_add_counter(bat_priv, BATADV_CNT_FRAG_TX_BYTES,
skb->len + ETH_HLEN);
ret = batadv_send_unicast_skb(skb, neigh_node);
+ /* skb was consumed */
+ skb = NULL;
-out:
- if (primary_if)
- batadv_hardif_put(primary_if);
+put_primary_if:
+ batadv_hardif_put(primary_if);
+free_skb:
+ kfree_skb(skb);
return ret;
}
--
2.10.1
^ permalink raw reply related
* [PATCH 17/17] batman-adv: Reject unicast packet with zero/mcast dst address
From: Simon Wunderlich @ 2016-11-08 16:45 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Simon Wunderlich
In-Reply-To: <20161108164526.7518-1-sw@simonwunderlich.de>
From: Sven Eckelmann <sven@narfation.org>
An unicast batman-adv packet cannot be transmitted to a multicast or zero
mac address. So reject incoming packets which still have these classes of
addresses as destination mac address in the outer ethernet header.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/routing.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 381f040..7d9ae4b 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -364,8 +364,8 @@ int batadv_recv_icmp_packet(struct sk_buff *skb,
ethhdr = eth_hdr(skb);
- /* packet with unicast indication but broadcast recipient */
- if (is_broadcast_ether_addr(ethhdr->h_dest))
+ /* packet with unicast indication but non-unicast recipient */
+ if (!is_valid_ether_addr(ethhdr->h_dest))
goto free_skb;
/* packet with broadcast/multicast sender address */
@@ -462,8 +462,8 @@ static int batadv_check_unicast_packet(struct batadv_priv *bat_priv,
ethhdr = eth_hdr(skb);
- /* packet with unicast indication but broadcast recipient */
- if (is_broadcast_ether_addr(ethhdr->h_dest))
+ /* packet with unicast indication but non-unicast recipient */
+ if (!is_valid_ether_addr(ethhdr->h_dest))
return -EBADR;
/* packet with broadcast/multicast sender address */
--
2.10.1
^ permalink raw reply related
* [PATCH 15/17] batman-adv: Disallow mcast src address for data frames
From: Simon Wunderlich @ 2016-11-08 16:45 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Simon Wunderlich
In-Reply-To: <20161108164526.7518-1-sw@simonwunderlich.de>
From: Sven Eckelmann <sven@narfation.org>
The routing checks are validating the source mac address of the outer
ethernet header. They reject every source mac address which is a broadcast
address. But they also have to reject any multicast mac addresses.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
[sw@simonwunderlich.de: fix commit message typo]
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/routing.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index caf1866..9646623 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -368,8 +368,8 @@ int batadv_recv_icmp_packet(struct sk_buff *skb,
if (is_broadcast_ether_addr(ethhdr->h_dest))
goto free_skb;
- /* packet with broadcast sender address */
- if (is_broadcast_ether_addr(ethhdr->h_source))
+ /* packet with broadcast/multicast sender address */
+ if (is_multicast_ether_addr(ethhdr->h_source))
goto free_skb;
/* not for me */
@@ -466,8 +466,8 @@ static int batadv_check_unicast_packet(struct batadv_priv *bat_priv,
if (is_broadcast_ether_addr(ethhdr->h_dest))
return -EBADR;
- /* packet with broadcast sender address */
- if (is_broadcast_ether_addr(ethhdr->h_source))
+ /* packet with broadcast/multicast sender address */
+ if (is_multicast_ether_addr(ethhdr->h_source))
return -EBADR;
/* not for me */
@@ -1159,8 +1159,8 @@ int batadv_recv_bcast_packet(struct sk_buff *skb,
if (!is_broadcast_ether_addr(ethhdr->h_dest))
goto free_skb;
- /* packet with broadcast sender address */
- if (is_broadcast_ether_addr(ethhdr->h_source))
+ /* packet with broadcast/multicast sender address */
+ if (is_multicast_ether_addr(ethhdr->h_source))
goto free_skb;
/* ignore broadcasts sent by myself */
--
2.10.1
^ permalink raw reply related
* [PATCH 14/17] batman-adv: Remove dev_queue_xmit return code exception
From: Simon Wunderlich @ 2016-11-08 16:45 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Simon Wunderlich
In-Reply-To: <20161108164526.7518-1-sw@simonwunderlich.de>
From: Sven Eckelmann <sven@narfation.org>
No caller of batadv_send_skb_to_orig is expecting the results to be -1
(-EPERM) anymore when the skbuff was not consumed. They will instead expect
that the skbuff is always consumed. Having such return code filter is
therefore not needed anymore.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/send.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c
index b00aac7..9ea272e 100644
--- a/net/batman-adv/send.c
+++ b/net/batman-adv/send.c
@@ -64,8 +64,11 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work);
* If neigh_node is NULL, then the packet is broadcasted using hard_iface,
* otherwise it is sent as unicast to the given neighbor.
*
- * Return: NET_TX_DROP in case of error or the result of dev_queue_xmit(skb)
- * otherwise
+ * Regardless of the return value, the skb is consumed.
+ *
+ * Return: A negative errno code is returned on a failure. A success does not
+ * guarantee the frame will be transmitted as it may be dropped due
+ * to congestion or traffic shaping.
*/
int batadv_send_skb_packet(struct sk_buff *skb,
struct batadv_hard_iface *hard_iface,
@@ -73,7 +76,6 @@ int batadv_send_skb_packet(struct sk_buff *skb,
{
struct batadv_priv *bat_priv;
struct ethhdr *ethhdr;
- int ret;
bat_priv = netdev_priv(hard_iface->soft_iface);
@@ -111,15 +113,8 @@ int batadv_send_skb_packet(struct sk_buff *skb,
/* dev_queue_xmit() returns a negative result on error. However on
* congestion and traffic shaping, it drops and returns NET_XMIT_DROP
* (which is > 0). This will not be treated as an error.
- *
- * a negative value cannot be returned because it could be interepreted
- * as not consumed skb by callers of batadv_send_skb_to_orig.
*/
- ret = dev_queue_xmit(skb);
- if (ret < 0)
- ret = NET_XMIT_DROP;
-
- return ret;
+ return dev_queue_xmit(skb);
send_skb_err:
kfree_skb(skb);
return NET_XMIT_DROP;
--
2.10.1
^ permalink raw reply related
* [PATCH 13/17] batman-adv: Consume skb in receive handlers
From: Simon Wunderlich @ 2016-11-08 16:45 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Simon Wunderlich
In-Reply-To: <20161108164526.7518-1-sw@simonwunderlich.de>
From: Sven Eckelmann <sven@narfation.org>
Receiving functions in Linux consume the supplied skbuff. Doing the same in
the batadv_rx_handler functions makes the behavior more similar to the rest
of the Linux network code.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/bat_iv_ogm.c | 17 +++--
net/batman-adv/bat_v_elp.c | 25 ++++---
net/batman-adv/bat_v_ogm.c | 10 +--
net/batman-adv/main.c | 11 +--
net/batman-adv/network-coding.c | 11 +--
net/batman-adv/routing.c | 149 +++++++++++++++++++++++++++-------------
6 files changed, 141 insertions(+), 82 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 310f391..b9941bf 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -1823,17 +1823,18 @@ static int batadv_iv_ogm_receive(struct sk_buff *skb,
struct batadv_ogm_packet *ogm_packet;
u8 *packet_pos;
int ogm_offset;
- bool ret;
+ bool res;
+ int ret = NET_RX_DROP;
- ret = batadv_check_management_packet(skb, if_incoming, BATADV_OGM_HLEN);
- if (!ret)
- return NET_RX_DROP;
+ res = batadv_check_management_packet(skb, if_incoming, BATADV_OGM_HLEN);
+ if (!res)
+ goto free_skb;
/* did we receive a B.A.T.M.A.N. IV OGM packet on an interface
* that does not have B.A.T.M.A.N. IV enabled ?
*/
if (bat_priv->algo_ops->iface.enable != batadv_iv_ogm_iface_enable)
- return NET_RX_DROP;
+ goto free_skb;
batadv_inc_counter(bat_priv, BATADV_CNT_MGMT_RX);
batadv_add_counter(bat_priv, BATADV_CNT_MGMT_RX_BYTES,
@@ -1854,8 +1855,12 @@ static int batadv_iv_ogm_receive(struct sk_buff *skb,
ogm_packet = (struct batadv_ogm_packet *)packet_pos;
}
+ ret = NET_RX_SUCCESS;
+
+free_skb:
consume_skb(skb);
- return NET_RX_SUCCESS;
+
+ return ret;
}
#ifdef CONFIG_BATMAN_ADV_DEBUGFS
diff --git a/net/batman-adv/bat_v_elp.c b/net/batman-adv/bat_v_elp.c
index ee08540..81a0501 100644
--- a/net/batman-adv/bat_v_elp.c
+++ b/net/batman-adv/bat_v_elp.c
@@ -492,20 +492,21 @@ int batadv_v_elp_packet_recv(struct sk_buff *skb,
struct batadv_elp_packet *elp_packet;
struct batadv_hard_iface *primary_if;
struct ethhdr *ethhdr = (struct ethhdr *)skb_mac_header(skb);
- bool ret;
+ bool res;
+ int ret;
- ret = batadv_check_management_packet(skb, if_incoming, BATADV_ELP_HLEN);
- if (!ret)
- return NET_RX_DROP;
+ res = batadv_check_management_packet(skb, if_incoming, BATADV_ELP_HLEN);
+ if (!res)
+ goto free_skb;
if (batadv_is_my_mac(bat_priv, ethhdr->h_source))
- return NET_RX_DROP;
+ goto free_skb;
/* did we receive a B.A.T.M.A.N. V ELP packet on an interface
* that does not have B.A.T.M.A.N. V ELP enabled ?
*/
if (strcmp(bat_priv->algo_ops->name, "BATMAN_V") != 0)
- return NET_RX_DROP;
+ goto free_skb;
elp_packet = (struct batadv_elp_packet *)skb->data;
@@ -516,14 +517,16 @@ int batadv_v_elp_packet_recv(struct sk_buff *skb,
primary_if = batadv_primary_if_get_selected(bat_priv);
if (!primary_if)
- goto out;
+ goto free_skb;
batadv_v_elp_neigh_update(bat_priv, ethhdr->h_source, if_incoming,
elp_packet);
-out:
- if (primary_if)
- batadv_hardif_put(primary_if);
+ ret = NET_RX_SUCCESS;
+ batadv_hardif_put(primary_if);
+
+free_skb:
consume_skb(skb);
- return NET_RX_SUCCESS;
+
+ return ret;
}
diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c
index 9922ccd..ef3a88d 100644
--- a/net/batman-adv/bat_v_ogm.c
+++ b/net/batman-adv/bat_v_ogm.c
@@ -810,18 +810,18 @@ int batadv_v_ogm_packet_recv(struct sk_buff *skb,
* B.A.T.M.A.N. V enabled ?
*/
if (strcmp(bat_priv->algo_ops->name, "BATMAN_V") != 0)
- return NET_RX_DROP;
+ goto free_skb;
if (!batadv_check_management_packet(skb, if_incoming, BATADV_OGM2_HLEN))
- return NET_RX_DROP;
+ goto free_skb;
if (batadv_is_my_mac(bat_priv, ethhdr->h_source))
- return NET_RX_DROP;
+ goto free_skb;
ogm_packet = (struct batadv_ogm2_packet *)skb->data;
if (batadv_is_my_mac(bat_priv, ogm_packet->orig))
- return NET_RX_DROP;
+ goto free_skb;
batadv_inc_counter(bat_priv, BATADV_CNT_MGMT_RX);
batadv_add_counter(bat_priv, BATADV_CNT_MGMT_RX_BYTES,
@@ -842,6 +842,8 @@ int batadv_v_ogm_packet_recv(struct sk_buff *skb,
}
ret = NET_RX_SUCCESS;
+
+free_skb:
consume_skb(skb);
return ret;
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index 5e4e818..6b5dae6 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -402,6 +402,8 @@ void batadv_skb_set_priority(struct sk_buff *skb, int offset)
static int batadv_recv_unhandled_packet(struct sk_buff *skb,
struct batadv_hard_iface *recv_if)
{
+ kfree_skb(skb);
+
return NET_RX_DROP;
}
@@ -416,7 +418,6 @@ int batadv_batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
struct batadv_ogm_packet *batadv_ogm_packet;
struct batadv_hard_iface *hard_iface;
u8 idx;
- int ret;
hard_iface = container_of(ptype, struct batadv_hard_iface,
batman_adv_ptype);
@@ -466,14 +467,8 @@ int batadv_batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
/* reset control block to avoid left overs from previous users */
memset(skb->cb, 0, sizeof(struct batadv_skb_cb));
- /* all receive handlers return whether they received or reused
- * the supplied skb. if not, we have to free the skb.
- */
idx = batadv_ogm_packet->packet_type;
- ret = (*batadv_rx_handler[idx])(skb, hard_iface);
-
- if (ret == NET_RX_DROP)
- kfree_skb(skb);
+ (*batadv_rx_handler[idx])(skb, hard_iface);
batadv_hardif_put(hard_iface);
diff --git a/net/batman-adv/network-coding.c b/net/batman-adv/network-coding.c
index 3af66d3..ab5a3bf 100644
--- a/net/batman-adv/network-coding.c
+++ b/net/batman-adv/network-coding.c
@@ -1819,11 +1819,11 @@ static int batadv_nc_recv_coded_packet(struct sk_buff *skb,
/* Check if network coding is enabled */
if (!atomic_read(&bat_priv->network_coding))
- return NET_RX_DROP;
+ goto free_skb;
/* Make sure we can access (and remove) header */
if (unlikely(!pskb_may_pull(skb, hdr_size)))
- return NET_RX_DROP;
+ goto free_skb;
coded_packet = (struct batadv_coded_packet *)skb->data;
ethhdr = eth_hdr(skb);
@@ -1831,7 +1831,7 @@ static int batadv_nc_recv_coded_packet(struct sk_buff *skb,
/* Verify frame is destined for us */
if (!batadv_is_my_mac(bat_priv, ethhdr->h_dest) &&
!batadv_is_my_mac(bat_priv, coded_packet->second_dest))
- return NET_RX_DROP;
+ goto free_skb;
/* Update stat counter */
if (batadv_is_my_mac(bat_priv, coded_packet->second_dest))
@@ -1841,7 +1841,7 @@ static int batadv_nc_recv_coded_packet(struct sk_buff *skb,
coded_packet);
if (!nc_packet) {
batadv_inc_counter(bat_priv, BATADV_CNT_NC_DECODE_FAILED);
- return NET_RX_DROP;
+ goto free_skb;
}
/* Make skb's linear, because decoding accesses the entire buffer */
@@ -1867,6 +1867,9 @@ static int batadv_nc_recv_coded_packet(struct sk_buff *skb,
free_nc_packet:
batadv_nc_packet_free(nc_packet, true);
+free_skb:
+ kfree_skb(skb);
+
return NET_RX_DROP;
}
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 4d2679a..caf1866 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -262,8 +262,11 @@ static int batadv_recv_my_icmp_packet(struct batadv_priv *bat_priv,
icmph->ttl = BATADV_TTL;
res = batadv_send_skb_to_orig(skb, orig_node, NULL);
- ret = NET_RX_SUCCESS;
+ if (res == NET_XMIT_SUCCESS)
+ ret = NET_RX_SUCCESS;
+ /* skb was consumed */
+ skb = NULL;
break;
case BATADV_TP:
if (!pskb_may_pull(skb, sizeof(struct batadv_icmp_tp_packet)))
@@ -271,6 +274,8 @@ static int batadv_recv_my_icmp_packet(struct batadv_priv *bat_priv,
batadv_tp_meter_recv(bat_priv, skb);
ret = NET_RX_SUCCESS;
+ /* skb was consumed */
+ skb = NULL;
goto out;
default:
/* drop unknown type */
@@ -281,6 +286,9 @@ static int batadv_recv_my_icmp_packet(struct batadv_priv *bat_priv,
batadv_hardif_put(primary_if);
if (orig_node)
batadv_orig_node_put(orig_node);
+
+ kfree_skb(skb);
+
return ret;
}
@@ -322,13 +330,20 @@ static int batadv_recv_icmp_ttl_exceeded(struct batadv_priv *bat_priv,
icmp_packet->ttl = BATADV_TTL;
res = batadv_send_skb_to_orig(skb, orig_node, NULL);
- ret = NET_RX_SUCCESS;
+ if (res == NET_RX_SUCCESS)
+ ret = NET_XMIT_SUCCESS;
+
+ /* skb was consumed */
+ skb = NULL;
out:
if (primary_if)
batadv_hardif_put(primary_if);
if (orig_node)
batadv_orig_node_put(orig_node);
+
+ kfree_skb(skb);
+
return ret;
}
@@ -345,21 +360,21 @@ int batadv_recv_icmp_packet(struct sk_buff *skb,
/* drop packet if it has not necessary minimum size */
if (unlikely(!pskb_may_pull(skb, hdr_size)))
- goto out;
+ goto free_skb;
ethhdr = eth_hdr(skb);
/* packet with unicast indication but broadcast recipient */
if (is_broadcast_ether_addr(ethhdr->h_dest))
- goto out;
+ goto free_skb;
/* packet with broadcast sender address */
if (is_broadcast_ether_addr(ethhdr->h_source))
- goto out;
+ goto free_skb;
/* not for me */
if (!batadv_is_my_mac(bat_priv, ethhdr->h_dest))
- goto out;
+ goto free_skb;
icmph = (struct batadv_icmp_header *)skb->data;
@@ -368,17 +383,17 @@ int batadv_recv_icmp_packet(struct sk_buff *skb,
icmph->msg_type == BATADV_ECHO_REQUEST) &&
(skb->len >= sizeof(struct batadv_icmp_packet_rr))) {
if (skb_linearize(skb) < 0)
- goto out;
+ goto free_skb;
/* create a copy of the skb, if needed, to modify it. */
if (skb_cow(skb, ETH_HLEN) < 0)
- goto out;
+ goto free_skb;
ethhdr = eth_hdr(skb);
icmph = (struct batadv_icmp_header *)skb->data;
icmp_packet_rr = (struct batadv_icmp_packet_rr *)icmph;
if (icmp_packet_rr->rr_cur >= BATADV_RR_LEN)
- goto out;
+ goto free_skb;
ether_addr_copy(icmp_packet_rr->rr[icmp_packet_rr->rr_cur],
ethhdr->h_dest);
@@ -396,11 +411,11 @@ int batadv_recv_icmp_packet(struct sk_buff *skb,
/* get routing information */
orig_node = batadv_orig_hash_find(bat_priv, icmph->dst);
if (!orig_node)
- goto out;
+ goto free_skb;
/* create a copy of the skb, if needed, to modify it. */
if (skb_cow(skb, ETH_HLEN) < 0)
- goto out;
+ goto put_orig_node;
icmph = (struct batadv_icmp_header *)skb->data;
@@ -409,11 +424,18 @@ int batadv_recv_icmp_packet(struct sk_buff *skb,
/* route it */
res = batadv_send_skb_to_orig(skb, orig_node, recv_if);
- ret = NET_RX_SUCCESS;
+ if (res == NET_XMIT_SUCCESS)
+ ret = NET_RX_SUCCESS;
-out:
+ /* skb was consumed */
+ skb = NULL;
+
+put_orig_node:
if (orig_node)
batadv_orig_node_put(orig_node);
+free_skb:
+ kfree_skb(skb);
+
return ret;
}
@@ -662,18 +684,18 @@ static int batadv_route_unicast_packet(struct sk_buff *skb,
if (unicast_packet->ttl < 2) {
pr_debug("Warning - can't forward unicast packet from %pM to %pM: ttl exceeded\n",
ethhdr->h_source, unicast_packet->dest);
- goto out;
+ goto free_skb;
}
/* get routing information */
orig_node = batadv_orig_hash_find(bat_priv, unicast_packet->dest);
if (!orig_node)
- goto out;
+ goto free_skb;
/* create a copy of the skb, if needed, to modify it. */
if (skb_cow(skb, ETH_HLEN) < 0)
- goto out;
+ goto put_orig_node;
/* decrement ttl */
unicast_packet = (struct batadv_unicast_packet *)skb->data;
@@ -697,6 +719,11 @@ static int batadv_route_unicast_packet(struct sk_buff *skb,
len = skb->len;
res = batadv_send_skb_to_orig(skb, orig_node, recv_if);
+ if (res == NET_XMIT_SUCCESS)
+ ret = NET_RX_SUCCESS;
+
+ /* skb was consumed */
+ skb = NULL;
/* translate transmit result into receive result */
if (res == NET_XMIT_SUCCESS) {
@@ -706,11 +733,11 @@ static int batadv_route_unicast_packet(struct sk_buff *skb,
len + ETH_HLEN);
}
- ret = NET_RX_SUCCESS;
+put_orig_node:
+ batadv_orig_node_put(orig_node);
+free_skb:
+ kfree_skb(skb);
-out:
- if (orig_node)
- batadv_orig_node_put(orig_node);
return ret;
}
@@ -895,14 +922,18 @@ int batadv_recv_unhandled_unicast_packet(struct sk_buff *skb,
check = batadv_check_unicast_packet(bat_priv, skb, hdr_size);
if (check < 0)
- return NET_RX_DROP;
+ goto free_skb;
/* we don't know about this type, drop it. */
unicast_packet = (struct batadv_unicast_packet *)skb->data;
if (batadv_is_my_mac(bat_priv, unicast_packet->dest))
- return NET_RX_DROP;
+ goto free_skb;
return batadv_route_unicast_packet(skb, recv_if);
+
+free_skb:
+ kfree_skb(skb);
+ return NET_RX_DROP;
}
int batadv_recv_unicast_packet(struct sk_buff *skb,
@@ -916,6 +947,7 @@ int batadv_recv_unicast_packet(struct sk_buff *skb,
int check, hdr_size = sizeof(*unicast_packet);
enum batadv_subtype subtype;
bool is4addr;
+ int ret = NET_RX_DROP;
unicast_packet = (struct batadv_unicast_packet *)skb->data;
unicast_4addr_packet = (struct batadv_unicast_4addr_packet *)skb->data;
@@ -935,9 +967,9 @@ int batadv_recv_unicast_packet(struct sk_buff *skb,
batadv_nc_skb_store_sniffed_unicast(bat_priv, skb);
if (check < 0)
- return NET_RX_DROP;
+ goto free_skb;
if (!batadv_check_unicast_ttvn(bat_priv, skb, hdr_size))
- return NET_RX_DROP;
+ goto free_skb;
/* packet for me */
if (batadv_is_my_mac(bat_priv, unicast_packet->dest)) {
@@ -975,7 +1007,14 @@ int batadv_recv_unicast_packet(struct sk_buff *skb,
return NET_RX_SUCCESS;
}
- return batadv_route_unicast_packet(skb, recv_if);
+ ret = batadv_route_unicast_packet(skb, recv_if);
+ /* skb was consumed */
+ skb = NULL;
+
+free_skb:
+ kfree_skb(skb);
+
+ return ret;
}
/**
@@ -997,15 +1036,15 @@ int batadv_recv_unicast_tvlv(struct sk_buff *skb,
int ret = NET_RX_DROP;
if (batadv_check_unicast_packet(bat_priv, skb, hdr_size) < 0)
- return NET_RX_DROP;
+ goto free_skb;
/* the header is likely to be modified while forwarding */
if (skb_cow(skb, hdr_size) < 0)
- return NET_RX_DROP;
+ goto free_skb;
/* packet needs to be linearized to access the tvlv content */
if (skb_linearize(skb) < 0)
- return NET_RX_DROP;
+ goto free_skb;
unicast_tvlv_packet = (struct batadv_unicast_tvlv_packet *)skb->data;
@@ -1013,17 +1052,21 @@ int batadv_recv_unicast_tvlv(struct sk_buff *skb,
tvlv_buff_len = ntohs(unicast_tvlv_packet->tvlv_len);
if (tvlv_buff_len > skb->len - hdr_size)
- return NET_RX_DROP;
+ goto free_skb;
ret = batadv_tvlv_containers_process(bat_priv, false, NULL,
unicast_tvlv_packet->src,
unicast_tvlv_packet->dst,
tvlv_buff, tvlv_buff_len);
- if (ret != NET_RX_SUCCESS)
+ if (ret != NET_RX_SUCCESS) {
ret = batadv_route_unicast_packet(skb, recv_if);
- else
- consume_skb(skb);
+ /* skb was consumed */
+ skb = NULL;
+ }
+
+free_skb:
+ consume_skb(skb);
return ret;
}
@@ -1049,20 +1092,22 @@ int batadv_recv_frag_packet(struct sk_buff *skb,
if (batadv_check_unicast_packet(bat_priv, skb,
sizeof(*frag_packet)) < 0)
- goto out;
+ goto free_skb;
frag_packet = (struct batadv_frag_packet *)skb->data;
orig_node_src = batadv_orig_hash_find(bat_priv, frag_packet->orig);
if (!orig_node_src)
- goto out;
+ goto free_skb;
skb->priority = frag_packet->priority + 256;
/* Route the fragment if it is not for us and too big to be merged. */
if (!batadv_is_my_mac(bat_priv, frag_packet->dest) &&
batadv_frag_skb_fwd(skb, recv_if, orig_node_src)) {
+ /* skb was consumed */
+ skb = NULL;
ret = NET_RX_SUCCESS;
- goto out;
+ goto put_orig_node;
}
batadv_inc_counter(bat_priv, BATADV_CNT_FRAG_RX);
@@ -1070,20 +1115,24 @@ int batadv_recv_frag_packet(struct sk_buff *skb,
/* Add fragment to buffer and merge if possible. */
if (!batadv_frag_skb_buffer(&skb, orig_node_src))
- goto out;
+ goto put_orig_node;
/* Deliver merged packet to the appropriate handler, if it was
* merged
*/
- if (skb)
+ if (skb) {
batadv_batman_skb_recv(skb, recv_if->net_dev,
&recv_if->batman_adv_ptype, NULL);
+ /* skb was consumed */
+ skb = NULL;
+ }
ret = NET_RX_SUCCESS;
-out:
- if (orig_node_src)
- batadv_orig_node_put(orig_node_src);
+put_orig_node:
+ batadv_orig_node_put(orig_node_src);
+free_skb:
+ kfree_skb(skb);
return ret;
}
@@ -1102,35 +1151,35 @@ int batadv_recv_bcast_packet(struct sk_buff *skb,
/* drop packet if it has not necessary minimum size */
if (unlikely(!pskb_may_pull(skb, hdr_size)))
- goto out;
+ goto free_skb;
ethhdr = eth_hdr(skb);
/* packet with broadcast indication but unicast recipient */
if (!is_broadcast_ether_addr(ethhdr->h_dest))
- goto out;
+ goto free_skb;
/* packet with broadcast sender address */
if (is_broadcast_ether_addr(ethhdr->h_source))
- goto out;
+ goto free_skb;
/* ignore broadcasts sent by myself */
if (batadv_is_my_mac(bat_priv, ethhdr->h_source))
- goto out;
+ goto free_skb;
bcast_packet = (struct batadv_bcast_packet *)skb->data;
/* ignore broadcasts originated by myself */
if (batadv_is_my_mac(bat_priv, bcast_packet->orig))
- goto out;
+ goto free_skb;
if (bcast_packet->ttl < 2)
- goto out;
+ goto free_skb;
orig_node = batadv_orig_hash_find(bat_priv, bcast_packet->orig);
if (!orig_node)
- goto out;
+ goto free_skb;
spin_lock_bh(&orig_node->bcast_seqno_lock);
@@ -1158,7 +1207,7 @@ int batadv_recv_bcast_packet(struct sk_buff *skb,
/* check whether this has been sent by another originator before */
if (batadv_bla_check_bcast_duplist(bat_priv, skb))
- goto out;
+ goto free_skb;
batadv_skb_set_priority(skb, sizeof(struct batadv_bcast_packet));
@@ -1169,7 +1218,7 @@ int batadv_recv_bcast_packet(struct sk_buff *skb,
* from the same backbone.
*/
if (batadv_bla_is_backbone_gw(skb, orig_node, hdr_size))
- goto out;
+ goto free_skb;
if (batadv_dat_snoop_incoming_arp_request(bat_priv, skb, hdr_size))
goto rx_success;
@@ -1185,6 +1234,8 @@ int batadv_recv_bcast_packet(struct sk_buff *skb,
spin_unlock:
spin_unlock_bh(&orig_node->bcast_seqno_lock);
+free_skb:
+ kfree_skb(skb);
out:
if (orig_node)
batadv_orig_node_put(orig_node);
--
2.10.1
^ permalink raw reply related
* [PATCH 10/17] batman-adv: Count all non-success TX packets as dropped
From: Simon Wunderlich @ 2016-11-08 16:45 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Simon Wunderlich
In-Reply-To: <20161108164526.7518-1-sw@simonwunderlich.de>
From: Sven Eckelmann <sven@narfation.org>
A failure during the submission also causes dropped packets.
batadv_interface_tx should therefore also increase the DROPPED counter for
these returns.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/soft-interface.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 2f0304e..7b3494a 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -386,7 +386,7 @@ static int batadv_interface_tx(struct sk_buff *skb,
ret = batadv_send_skb_via_tt(bat_priv, skb, dst_hint,
vid);
}
- if (ret == NET_XMIT_DROP)
+ if (ret != NET_XMIT_SUCCESS)
goto dropped_freed;
}
--
2.10.1
^ permalink raw reply related
* [PATCH 09/17] batman-adv: use consume_skb for non-dropped packets
From: Simon Wunderlich @ 2016-11-08 16:45 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Simon Wunderlich
In-Reply-To: <20161108164526.7518-1-sw@simonwunderlich.de>
From: Sven Eckelmann <sven@narfation.org>
kfree_skb assumes that an skb is dropped after an failure and notes that.
consume_skb should be used in non-failure situations. Such information is
important for dropmonitor netlink which tells how many packets were dropped
and where this drop happened.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/bat_iv_ogm.c | 13 ++++++++-----
net/batman-adv/fragmentation.c | 20 ++++++++++++++------
net/batman-adv/network-coding.c | 24 +++++++++++++++---------
net/batman-adv/send.c | 27 +++++++++++++++++++--------
net/batman-adv/send.h | 3 ++-
net/batman-adv/soft-interface.c | 2 +-
6 files changed, 59 insertions(+), 30 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 0b9be62..310f391 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -698,7 +698,7 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff,
forw_packet_aggr->skb = netdev_alloc_skb_ip_align(NULL, skb_size);
if (!forw_packet_aggr->skb) {
- batadv_forw_packet_free(forw_packet_aggr);
+ batadv_forw_packet_free(forw_packet_aggr, true);
return;
}
@@ -1611,7 +1611,7 @@ batadv_iv_ogm_process_per_outif(const struct sk_buff *skb, int ogm_offset,
if (hardif_neigh)
batadv_hardif_neigh_put(hardif_neigh);
- kfree_skb(skb_priv);
+ consume_skb(skb_priv);
}
/**
@@ -1783,6 +1783,7 @@ static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
struct delayed_work *delayed_work;
struct batadv_forw_packet *forw_packet;
struct batadv_priv *bat_priv;
+ bool dropped = false;
delayed_work = to_delayed_work(work);
forw_packet = container_of(delayed_work, struct batadv_forw_packet,
@@ -1792,8 +1793,10 @@ static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
hlist_del(&forw_packet->list);
spin_unlock_bh(&bat_priv->forw_bat_list_lock);
- if (atomic_read(&bat_priv->mesh_state) == BATADV_MESH_DEACTIVATING)
+ if (atomic_read(&bat_priv->mesh_state) == BATADV_MESH_DEACTIVATING) {
+ dropped = true;
goto out;
+ }
batadv_iv_ogm_emit(forw_packet);
@@ -1810,7 +1813,7 @@ static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
batadv_iv_ogm_schedule(forw_packet->if_incoming);
out:
- batadv_forw_packet_free(forw_packet);
+ batadv_forw_packet_free(forw_packet, dropped);
}
static int batadv_iv_ogm_receive(struct sk_buff *skb,
@@ -1851,7 +1854,7 @@ static int batadv_iv_ogm_receive(struct sk_buff *skb,
ogm_packet = (struct batadv_ogm_packet *)packet_pos;
}
- kfree_skb(skb);
+ consume_skb(skb);
return NET_RX_SUCCESS;
}
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c
index 2b967a3..a2e28a1 100644
--- a/net/batman-adv/fragmentation.c
+++ b/net/batman-adv/fragmentation.c
@@ -42,17 +42,23 @@
/**
* batadv_frag_clear_chain - delete entries in the fragment buffer chain
* @head: head of chain with entries.
+ * @dropped: whether the chain is cleared because all fragments are dropped
*
* Free fragments in the passed hlist. Should be called with appropriate lock.
*/
-static void batadv_frag_clear_chain(struct hlist_head *head)
+static void batadv_frag_clear_chain(struct hlist_head *head, bool dropped)
{
struct batadv_frag_list_entry *entry;
struct hlist_node *node;
hlist_for_each_entry_safe(entry, node, head, list) {
hlist_del(&entry->list);
- kfree_skb(entry->skb);
+
+ if (dropped)
+ kfree_skb(entry->skb);
+ else
+ consume_skb(entry->skb);
+
kfree(entry);
}
}
@@ -73,7 +79,7 @@ void batadv_frag_purge_orig(struct batadv_orig_node *orig_node,
spin_lock_bh(&chain->lock);
if (!check_cb || check_cb(chain)) {
- batadv_frag_clear_chain(&chain->fragment_list);
+ batadv_frag_clear_chain(&chain->fragment_list, true);
chain->size = 0;
}
@@ -118,7 +124,7 @@ static bool batadv_frag_init_chain(struct batadv_frag_table_entry *chain,
return false;
if (!hlist_empty(&chain->fragment_list))
- batadv_frag_clear_chain(&chain->fragment_list);
+ batadv_frag_clear_chain(&chain->fragment_list, true);
chain->size = 0;
chain->seqno = seqno;
@@ -220,7 +226,7 @@ static bool batadv_frag_insert_packet(struct batadv_orig_node *orig_node,
* exceeds the maximum size of one merged packet. Don't allow
* packets to have different total_size.
*/
- batadv_frag_clear_chain(&chain->fragment_list);
+ batadv_frag_clear_chain(&chain->fragment_list, true);
chain->size = 0;
} else if (ntohs(frag_packet->total_size) == chain->size) {
/* All fragments received. Hand over chain to caller. */
@@ -254,6 +260,7 @@ batadv_frag_merge_packets(struct hlist_head *chain)
struct batadv_frag_list_entry *entry;
struct sk_buff *skb_out;
int size, hdr_size = sizeof(struct batadv_frag_packet);
+ bool dropped = false;
/* Remove first entry, as this is the destination for the rest of the
* fragments.
@@ -270,6 +277,7 @@ batadv_frag_merge_packets(struct hlist_head *chain)
if (pskb_expand_head(skb_out, 0, size - skb_out->len, GFP_ATOMIC) < 0) {
kfree_skb(skb_out);
skb_out = NULL;
+ dropped = true;
goto free;
}
@@ -291,7 +299,7 @@ batadv_frag_merge_packets(struct hlist_head *chain)
free:
/* Locking is not needed, because 'chain' is not part of any orig. */
- batadv_frag_clear_chain(chain);
+ batadv_frag_clear_chain(chain, dropped);
return skb_out;
}
diff --git a/net/batman-adv/network-coding.c b/net/batman-adv/network-coding.c
index c213dde..3af66d3 100644
--- a/net/batman-adv/network-coding.c
+++ b/net/batman-adv/network-coding.c
@@ -260,10 +260,16 @@ static void batadv_nc_path_put(struct batadv_nc_path *nc_path)
/**
* batadv_nc_packet_free - frees nc packet
* @nc_packet: the nc packet to free
+ * @dropped: whether the packet is freed because is is dropped
*/
-static void batadv_nc_packet_free(struct batadv_nc_packet *nc_packet)
+static void batadv_nc_packet_free(struct batadv_nc_packet *nc_packet,
+ bool dropped)
{
- kfree_skb(nc_packet->skb);
+ if (dropped)
+ kfree_skb(nc_packet->skb);
+ else
+ consume_skb(nc_packet->skb);
+
batadv_nc_path_put(nc_packet->nc_path);
kfree(nc_packet);
}
@@ -576,7 +582,7 @@ static void batadv_nc_send_packet(struct batadv_nc_packet *nc_packet)
{
batadv_send_unicast_skb(nc_packet->skb, nc_packet->neigh_node);
nc_packet->skb = NULL;
- batadv_nc_packet_free(nc_packet);
+ batadv_nc_packet_free(nc_packet, false);
}
/**
@@ -610,7 +616,7 @@ static bool batadv_nc_sniffed_purge(struct batadv_priv *bat_priv,
/* purge nc packet */
list_del(&nc_packet->list);
- batadv_nc_packet_free(nc_packet);
+ batadv_nc_packet_free(nc_packet, true);
res = true;
@@ -1208,11 +1214,11 @@ static bool batadv_nc_code_packets(struct batadv_priv *bat_priv,
}
/* skb_src is now coded into skb_dest, so free it */
- kfree_skb(skb_src);
+ consume_skb(skb_src);
/* avoid duplicate free of skb from nc_packet */
nc_packet->skb = NULL;
- batadv_nc_packet_free(nc_packet);
+ batadv_nc_packet_free(nc_packet, false);
/* Send the coded packet and return true */
batadv_send_unicast_skb(skb_dest, first_dest);
@@ -1399,7 +1405,7 @@ static void batadv_nc_skb_store_before_coding(struct batadv_priv *bat_priv,
/* batadv_nc_skb_store_for_decoding() clones the skb, so we must free
* our ref
*/
- kfree_skb(skb);
+ consume_skb(skb);
}
/**
@@ -1723,7 +1729,7 @@ batadv_nc_skb_decode_packet(struct batadv_priv *bat_priv, struct sk_buff *skb,
ether_addr_copy(unicast_packet->dest, orig_dest);
unicast_packet->ttvn = ttvn;
- batadv_nc_packet_free(nc_packet);
+ batadv_nc_packet_free(nc_packet, false);
return unicast_packet;
}
@@ -1860,7 +1866,7 @@ static int batadv_nc_recv_coded_packet(struct sk_buff *skb,
return batadv_recv_unicast_packet(skb, recv_if);
free_nc_packet:
- batadv_nc_packet_free(nc_packet);
+ batadv_nc_packet_free(nc_packet, true);
return NET_RX_DROP;
}
diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c
index d9bbb00..0f86293 100644
--- a/net/batman-adv/send.c
+++ b/net/batman-adv/send.c
@@ -451,13 +451,19 @@ int batadv_send_skb_via_gw(struct batadv_priv *bat_priv, struct sk_buff *skb,
/**
* batadv_forw_packet_free - free a forwarding packet
* @forw_packet: The packet to free
+ * @dropped: whether the packet is freed because is is dropped
*
* This frees a forwarding packet and releases any resources it might
* have claimed.
*/
-void batadv_forw_packet_free(struct batadv_forw_packet *forw_packet)
+void batadv_forw_packet_free(struct batadv_forw_packet *forw_packet,
+ bool dropped)
{
- kfree_skb(forw_packet->skb);
+ if (dropped)
+ kfree_skb(forw_packet->skb);
+ else
+ consume_skb(forw_packet->skb);
+
if (forw_packet->if_incoming)
batadv_hardif_put(forw_packet->if_incoming);
if (forw_packet->if_outgoing)
@@ -598,7 +604,7 @@ int batadv_add_bcast_packet_to_list(struct batadv_priv *bat_priv,
return NETDEV_TX_OK;
err_packet_free:
- batadv_forw_packet_free(forw_packet);
+ batadv_forw_packet_free(forw_packet, true);
err:
return NETDEV_TX_BUSY;
}
@@ -613,6 +619,7 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work)
struct sk_buff *skb1;
struct net_device *soft_iface;
struct batadv_priv *bat_priv;
+ bool dropped = false;
u8 *neigh_addr;
u8 *orig_neigh;
int ret = 0;
@@ -627,11 +634,15 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work)
hlist_del(&forw_packet->list);
spin_unlock_bh(&bat_priv->forw_bcast_list_lock);
- if (atomic_read(&bat_priv->mesh_state) == BATADV_MESH_DEACTIVATING)
+ if (atomic_read(&bat_priv->mesh_state) == BATADV_MESH_DEACTIVATING) {
+ dropped = true;
goto out;
+ }
- if (batadv_dat_drop_broadcast_packet(bat_priv, forw_packet))
+ if (batadv_dat_drop_broadcast_packet(bat_priv, forw_packet)) {
+ dropped = true;
goto out;
+ }
bcast_packet = (struct batadv_bcast_packet *)forw_packet->skb->data;
@@ -709,7 +720,7 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work)
}
out:
- batadv_forw_packet_free(forw_packet);
+ batadv_forw_packet_free(forw_packet, dropped);
}
void
@@ -750,7 +761,7 @@ batadv_purge_outstanding_packets(struct batadv_priv *bat_priv,
if (pending) {
hlist_del(&forw_packet->list);
- batadv_forw_packet_free(forw_packet);
+ batadv_forw_packet_free(forw_packet, true);
}
}
spin_unlock_bh(&bat_priv->forw_bcast_list_lock);
@@ -777,7 +788,7 @@ batadv_purge_outstanding_packets(struct batadv_priv *bat_priv,
if (pending) {
hlist_del(&forw_packet->list);
- batadv_forw_packet_free(forw_packet);
+ batadv_forw_packet_free(forw_packet, true);
}
}
spin_unlock_bh(&bat_priv->forw_bat_list_lock);
diff --git a/net/batman-adv/send.h b/net/batman-adv/send.h
index 86009b5..c580194 100644
--- a/net/batman-adv/send.h
+++ b/net/batman-adv/send.h
@@ -27,7 +27,8 @@
struct sk_buff;
-void batadv_forw_packet_free(struct batadv_forw_packet *forw_packet);
+void batadv_forw_packet_free(struct batadv_forw_packet *forw_packet,
+ bool dropped);
struct batadv_forw_packet *
batadv_forw_packet_alloc(struct batadv_hard_iface *if_incoming,
struct batadv_hard_iface *if_outgoing,
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 109f53b..2f0304e 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -362,7 +362,7 @@ static int batadv_interface_tx(struct sk_buff *skb,
/* a copy is stored in the bcast list, therefore removing
* the original skb.
*/
- kfree_skb(skb);
+ consume_skb(skb);
/* unicast packet */
} else {
--
2.10.1
^ permalink raw reply related
* [PATCH 06/17] batman-adv: Remove unused skb_reset_mac_header()
From: Simon Wunderlich @ 2016-11-08 16:45 UTC (permalink / raw)
To: davem
Cc: netdev, b.a.t.m.a.n, Linus Lüssing, Sven Eckelmann,
Simon Wunderlich
In-Reply-To: <20161108164526.7518-1-sw@simonwunderlich.de>
From: Linus Lüssing <linus.luessing@c0d3.blue>
During broadcast queueing, the skb_reset_mac_header() sets the skb
to a place invalid for a MAC header, pointing right into the
batman-adv broadcast packet. Luckily, no one seems to actually use
eth_hdr(skb) afterwards until batadv_send_skb_packet() resets the
header to a valid position again.
Therefore removing this unnecessary, weird skb_reset_mac_header()
call.
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/send.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c
index e1e9136..be3f6d7 100644
--- a/net/batman-adv/send.c
+++ b/net/batman-adv/send.c
@@ -586,8 +586,6 @@ int batadv_add_bcast_packet_to_list(struct batadv_priv *bat_priv,
bcast_packet = (struct batadv_bcast_packet *)newskb->data;
bcast_packet->ttl--;
- skb_reset_mac_header(newskb);
-
forw_packet->skb = newskb;
INIT_DELAYED_WORK(&forw_packet->delayed_work,
--
2.10.1
^ permalink raw reply related
* [PATCH 08/17] batman-adv: Simple (re)broadcast avoidance
From: Simon Wunderlich @ 2016-11-08 16:45 UTC (permalink / raw)
To: davem
Cc: netdev, b.a.t.m.a.n, Linus Lüssing, Sven Eckelmann,
Simon Wunderlich
In-Reply-To: <20161108164526.7518-1-sw@simonwunderlich.de>
From: Linus Lüssing <linus.luessing@c0d3.blue>
With this patch, (re)broadcasting on a specific interfaces is avoided:
* No neighbor: There is no need to broadcast on an interface if there
is no node behind it.
* Single neighbor is source: If there is just one neighbor on an
interface and if this neighbor is the one we actually got this
broadcast packet from, then we do not need to echo it back.
* Single neighbor is originator: If there is just one neighbor on
an interface and if this neighbor is the originator of this
broadcast packet, then we do not need to echo it back.
Goodies for BATMAN V:
("Upgrade your BATMAN IV network to V now to get these for free!")
Thanks to the split of OGMv1 into two packet types, OGMv2 and ELP
that is, we can now apply the same optimizations stated above to OGMv2
packets, too.
Furthermore, with BATMAN V, rebroadcasts can be reduced in certain
multi interface cases, too, where BATMAN IV cannot. This is thanks to
the removal of the "secondary interface originator" concept in BATMAN V.
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/bat_v_ogm.c | 56 +++++++++++++++++++++++++++++++++++++++++
net/batman-adv/hard-interface.c | 52 ++++++++++++++++++++++++++++++++++++++
net/batman-adv/hard-interface.h | 16 ++++++++++++
net/batman-adv/originator.c | 13 +++++++---
net/batman-adv/routing.c | 2 +-
net/batman-adv/send.c | 55 +++++++++++++++++++++++++++++++++++++++-
net/batman-adv/send.h | 3 ++-
net/batman-adv/soft-interface.c | 2 +-
net/batman-adv/types.h | 2 ++
9 files changed, 193 insertions(+), 8 deletions(-)
diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c
index 61ff5f8..9922ccd 100644
--- a/net/batman-adv/bat_v_ogm.c
+++ b/net/batman-adv/bat_v_ogm.c
@@ -140,6 +140,7 @@ static void batadv_v_ogm_send(struct work_struct *work)
unsigned char *ogm_buff, *pkt_buff;
int ogm_buff_len;
u16 tvlv_len = 0;
+ int ret;
bat_v = container_of(work, struct batadv_priv_bat_v, ogm_wq.work);
bat_priv = container_of(bat_v, struct batadv_priv, bat_v);
@@ -182,6 +183,31 @@ static void batadv_v_ogm_send(struct work_struct *work)
if (!kref_get_unless_zero(&hard_iface->refcount))
continue;
+ ret = batadv_hardif_no_broadcast(hard_iface, NULL, NULL);
+ if (ret) {
+ char *type;
+
+ switch (ret) {
+ case BATADV_HARDIF_BCAST_NORECIPIENT:
+ type = "no neighbor";
+ break;
+ case BATADV_HARDIF_BCAST_DUPFWD:
+ type = "single neighbor is source";
+ break;
+ case BATADV_HARDIF_BCAST_DUPORIG:
+ type = "single neighbor is originator";
+ break;
+ default:
+ type = "unknown";
+ }
+
+ batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "OGM2 from ourselve on %s surpressed: %s\n",
+ hard_iface->net_dev->name, type);
+
+ batadv_hardif_put(hard_iface);
+ continue;
+ }
+
batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
"Sending own OGM2 packet (originator %pM, seqno %u, throughput %u, TTL %d) on interface %s [%pM]\n",
ogm_packet->orig, ntohl(ogm_packet->seqno),
@@ -651,6 +677,7 @@ static void batadv_v_ogm_process(const struct sk_buff *skb, int ogm_offset,
struct batadv_hard_iface *hard_iface;
struct batadv_ogm2_packet *ogm_packet;
u32 ogm_throughput, link_throughput, path_throughput;
+ int ret;
ethhdr = eth_hdr(skb);
ogm_packet = (struct batadv_ogm2_packet *)(skb->data + ogm_offset);
@@ -716,6 +743,35 @@ static void batadv_v_ogm_process(const struct sk_buff *skb, int ogm_offset,
if (!kref_get_unless_zero(&hard_iface->refcount))
continue;
+ ret = batadv_hardif_no_broadcast(hard_iface,
+ ogm_packet->orig,
+ hardif_neigh->orig);
+
+ if (ret) {
+ char *type;
+
+ switch (ret) {
+ case BATADV_HARDIF_BCAST_NORECIPIENT:
+ type = "no neighbor";
+ break;
+ case BATADV_HARDIF_BCAST_DUPFWD:
+ type = "single neighbor is source";
+ break;
+ case BATADV_HARDIF_BCAST_DUPORIG:
+ type = "single neighbor is originator";
+ break;
+ default:
+ type = "unknown";
+ }
+
+ batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "OGM2 packet from %pM on %s surpressed: %s\n",
+ ogm_packet->orig, hard_iface->net_dev->name,
+ type);
+
+ batadv_hardif_put(hard_iface);
+ continue;
+ }
+
batadv_v_ogm_process_per_outif(bat_priv, ethhdr, ogm_packet,
orig_node, neigh_node,
if_incoming, hard_iface);
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 08ce361..a7a462e 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -228,6 +228,58 @@ bool batadv_is_wifi_netdev(struct net_device *net_device)
return false;
}
+/**
+ * batadv_hardif_no_broadcast - check whether (re)broadcast is necessary
+ * @if_outgoing: the outgoing interface checked and considered for (re)broadcast
+ * @orig_addr: the originator of this packet
+ * @orig_neigh: originator address of the forwarder we just got the packet from
+ * (NULL if we originated)
+ *
+ * Checks whether a packet needs to be (re)broadcasted on the given interface.
+ *
+ * Return:
+ * BATADV_HARDIF_BCAST_NORECIPIENT: No neighbor on interface
+ * BATADV_HARDIF_BCAST_DUPFWD: Just one neighbor, but it is the forwarder
+ * BATADV_HARDIF_BCAST_DUPORIG: Just one neighbor, but it is the originator
+ * BATADV_HARDIF_BCAST_OK: Several neighbors, must broadcast
+ */
+int batadv_hardif_no_broadcast(struct batadv_hard_iface *if_outgoing,
+ u8 *orig_addr, u8 *orig_neigh)
+{
+ struct batadv_hardif_neigh_node *hardif_neigh;
+ struct hlist_node *first;
+ int ret = BATADV_HARDIF_BCAST_OK;
+
+ rcu_read_lock();
+
+ /* 0 neighbors -> no (re)broadcast */
+ first = rcu_dereference(hlist_first_rcu(&if_outgoing->neigh_list));
+ if (!first) {
+ ret = BATADV_HARDIF_BCAST_NORECIPIENT;
+ goto out;
+ }
+
+ /* >1 neighbors -> (re)brodcast */
+ if (rcu_dereference(hlist_next_rcu(first)))
+ goto out;
+
+ hardif_neigh = hlist_entry(first, struct batadv_hardif_neigh_node,
+ list);
+
+ /* 1 neighbor, is the originator -> no rebroadcast */
+ if (orig_addr && batadv_compare_eth(hardif_neigh->orig, orig_addr)) {
+ ret = BATADV_HARDIF_BCAST_DUPORIG;
+ /* 1 neighbor, is the one we received from -> no rebroadcast */
+ } else if (orig_neigh &&
+ batadv_compare_eth(hardif_neigh->orig, orig_neigh)) {
+ ret = BATADV_HARDIF_BCAST_DUPFWD;
+ }
+
+out:
+ rcu_read_unlock();
+ return ret;
+}
+
static struct batadv_hard_iface *
batadv_hardif_get_active(const struct net_device *soft_iface)
{
diff --git a/net/batman-adv/hard-interface.h b/net/batman-adv/hard-interface.h
index a76724d..a043182 100644
--- a/net/batman-adv/hard-interface.h
+++ b/net/batman-adv/hard-interface.h
@@ -40,6 +40,20 @@ enum batadv_hard_if_state {
};
/**
+ * enum batadv_hard_if_bcast - broadcast avoidance options
+ * @BATADV_HARDIF_BCAST_OK: Do broadcast on according hard interface
+ * @BATADV_HARDIF_BCAST_NORECIPIENT: Broadcast not needed, there is no recipient
+ * @BATADV_HARDIF_BCAST_DUPFWD: There is just the neighbor we got it from
+ * @BATADV_HARDIF_BCAST_DUPORIG: There is just the originator
+ */
+enum batadv_hard_if_bcast {
+ BATADV_HARDIF_BCAST_OK = 0,
+ BATADV_HARDIF_BCAST_NORECIPIENT,
+ BATADV_HARDIF_BCAST_DUPFWD,
+ BATADV_HARDIF_BCAST_DUPORIG,
+};
+
+/**
* enum batadv_hard_if_cleanup - Cleanup modi for soft_iface after slave removal
* @BATADV_IF_CLEANUP_KEEP: Don't automatically delete soft-interface
* @BATADV_IF_CLEANUP_AUTO: Delete soft-interface after last slave was removed
@@ -63,6 +77,8 @@ void batadv_hardif_remove_interfaces(void);
int batadv_hardif_min_mtu(struct net_device *soft_iface);
void batadv_update_min_mtu(struct net_device *soft_iface);
void batadv_hardif_release(struct kref *ref);
+int batadv_hardif_no_broadcast(struct batadv_hard_iface *if_outgoing,
+ u8 *orig_addr, u8 *orig_neigh);
/**
* batadv_hardif_put - decrement the hard interface refcounter and possibly
diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c
index 518b1ed..ad26559 100644
--- a/net/batman-adv/originator.c
+++ b/net/batman-adv/originator.c
@@ -512,12 +512,14 @@ batadv_neigh_node_get(const struct batadv_orig_node *orig_node,
* batadv_hardif_neigh_create - create a hardif neighbour node
* @hard_iface: the interface this neighbour is connected to
* @neigh_addr: the interface address of the neighbour to retrieve
+ * @orig_node: originator object representing the neighbour
*
* Return: the hardif neighbour node if found or created or NULL otherwise.
*/
static struct batadv_hardif_neigh_node *
batadv_hardif_neigh_create(struct batadv_hard_iface *hard_iface,
- const u8 *neigh_addr)
+ const u8 *neigh_addr,
+ struct batadv_orig_node *orig_node)
{
struct batadv_priv *bat_priv = netdev_priv(hard_iface->soft_iface);
struct batadv_hardif_neigh_node *hardif_neigh;
@@ -536,6 +538,7 @@ batadv_hardif_neigh_create(struct batadv_hard_iface *hard_iface,
kref_get(&hard_iface->refcount);
INIT_HLIST_NODE(&hardif_neigh->list);
ether_addr_copy(hardif_neigh->addr, neigh_addr);
+ ether_addr_copy(hardif_neigh->orig, orig_node->orig);
hardif_neigh->if_incoming = hard_iface;
hardif_neigh->last_seen = jiffies;
@@ -556,12 +559,14 @@ batadv_hardif_neigh_create(struct batadv_hard_iface *hard_iface,
* node
* @hard_iface: the interface this neighbour is connected to
* @neigh_addr: the interface address of the neighbour to retrieve
+ * @orig_node: originator object representing the neighbour
*
* Return: the hardif neighbour node if found or created or NULL otherwise.
*/
static struct batadv_hardif_neigh_node *
batadv_hardif_neigh_get_or_create(struct batadv_hard_iface *hard_iface,
- const u8 *neigh_addr)
+ const u8 *neigh_addr,
+ struct batadv_orig_node *orig_node)
{
struct batadv_hardif_neigh_node *hardif_neigh;
@@ -570,7 +575,7 @@ batadv_hardif_neigh_get_or_create(struct batadv_hard_iface *hard_iface,
if (hardif_neigh)
return hardif_neigh;
- return batadv_hardif_neigh_create(hard_iface, neigh_addr);
+ return batadv_hardif_neigh_create(hard_iface, neigh_addr, orig_node);
}
/**
@@ -630,7 +635,7 @@ batadv_neigh_node_create(struct batadv_orig_node *orig_node,
goto out;
hardif_neigh = batadv_hardif_neigh_get_or_create(hard_iface,
- neigh_addr);
+ neigh_addr, orig_node);
if (!hardif_neigh)
goto out;
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 7e8dc64..a4cb157 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -1170,7 +1170,7 @@ int batadv_recv_bcast_packet(struct sk_buff *skb,
batadv_skb_set_priority(skb, sizeof(struct batadv_bcast_packet));
/* rebroadcast packet */
- batadv_add_bcast_packet_to_list(bat_priv, skb, 1);
+ batadv_add_bcast_packet_to_list(bat_priv, skb, 1, false);
/* don't hand the broadcast up if it is from an originator
* from the same backbone.
diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c
index be3f6d7..d9bbb00 100644
--- a/net/batman-adv/send.c
+++ b/net/batman-adv/send.c
@@ -549,6 +549,7 @@ _batadv_add_bcast_packet_to_list(struct batadv_priv *bat_priv,
* @bat_priv: the bat priv with all the soft interface information
* @skb: broadcast packet to add
* @delay: number of jiffies to wait before sending
+ * @own_packet: true if it is a self-generated broadcast packet
*
* add a broadcast packet to the queue and setup timers. broadcast packets
* are sent multiple times to increase probability for being received.
@@ -560,7 +561,8 @@ _batadv_add_bcast_packet_to_list(struct batadv_priv *bat_priv,
*/
int batadv_add_bcast_packet_to_list(struct batadv_priv *bat_priv,
const struct sk_buff *skb,
- unsigned long delay)
+ unsigned long delay,
+ bool own_packet)
{
struct batadv_hard_iface *primary_if;
struct batadv_forw_packet *forw_packet;
@@ -587,6 +589,7 @@ int batadv_add_bcast_packet_to_list(struct batadv_priv *bat_priv,
bcast_packet->ttl--;
forw_packet->skb = newskb;
+ forw_packet->own = own_packet;
INIT_DELAYED_WORK(&forw_packet->delayed_work,
batadv_send_outstanding_bcast_packet);
@@ -603,11 +606,16 @@ int batadv_add_bcast_packet_to_list(struct batadv_priv *bat_priv,
static void batadv_send_outstanding_bcast_packet(struct work_struct *work)
{
struct batadv_hard_iface *hard_iface;
+ struct batadv_hardif_neigh_node *neigh_node;
struct delayed_work *delayed_work;
struct batadv_forw_packet *forw_packet;
+ struct batadv_bcast_packet *bcast_packet;
struct sk_buff *skb1;
struct net_device *soft_iface;
struct batadv_priv *bat_priv;
+ u8 *neigh_addr;
+ u8 *orig_neigh;
+ int ret = 0;
delayed_work = to_delayed_work(work);
forw_packet = container_of(delayed_work, struct batadv_forw_packet,
@@ -625,6 +633,8 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work)
if (batadv_dat_drop_broadcast_packet(bat_priv, forw_packet))
goto out;
+ bcast_packet = (struct batadv_bcast_packet *)forw_packet->skb->data;
+
/* rebroadcast packet */
rcu_read_lock();
list_for_each_entry_rcu(hard_iface, &batadv_hardif_list, list) {
@@ -634,6 +644,49 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work)
if (forw_packet->num_packets >= hard_iface->num_bcasts)
continue;
+ if (forw_packet->own) {
+ neigh_node = NULL;
+ } else {
+ neigh_addr = eth_hdr(forw_packet->skb)->h_source;
+ neigh_node = batadv_hardif_neigh_get(hard_iface,
+ neigh_addr);
+ }
+
+ orig_neigh = neigh_node ? neigh_node->orig : NULL;
+
+ ret = batadv_hardif_no_broadcast(hard_iface, bcast_packet->orig,
+ orig_neigh);
+
+ if (ret) {
+ char *type;
+
+ switch (ret) {
+ case BATADV_HARDIF_BCAST_NORECIPIENT:
+ type = "no neighbor";
+ break;
+ case BATADV_HARDIF_BCAST_DUPFWD:
+ type = "single neighbor is source";
+ break;
+ case BATADV_HARDIF_BCAST_DUPORIG:
+ type = "single neighbor is originator";
+ break;
+ default:
+ type = "unknown";
+ }
+
+ batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "BCAST packet from orig %pM on %s surpressed: %s\n",
+ bcast_packet->orig,
+ hard_iface->net_dev->name, type);
+
+ if (neigh_node)
+ batadv_hardif_neigh_put(neigh_node);
+
+ continue;
+ }
+
+ if (neigh_node)
+ batadv_hardif_neigh_put(neigh_node);
+
if (!kref_get_unless_zero(&hard_iface->refcount))
continue;
diff --git a/net/batman-adv/send.h b/net/batman-adv/send.h
index 999f786..86009b5 100644
--- a/net/batman-adv/send.h
+++ b/net/batman-adv/send.h
@@ -46,7 +46,8 @@ int batadv_send_unicast_skb(struct sk_buff *skb,
struct batadv_neigh_node *neigh_node);
int batadv_add_bcast_packet_to_list(struct batadv_priv *bat_priv,
const struct sk_buff *skb,
- unsigned long delay);
+ unsigned long delay,
+ bool own_packet);
void
batadv_purge_outstanding_packets(struct batadv_priv *bat_priv,
const struct batadv_hard_iface *hard_iface);
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index f37c1c7..109f53b 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -357,7 +357,7 @@ static int batadv_interface_tx(struct sk_buff *skb,
seqno = atomic_inc_return(&bat_priv->bcast_seqno);
bcast_packet->seqno = htonl(seqno);
- batadv_add_bcast_packet_to_list(bat_priv, skb, brd_delay);
+ batadv_add_bcast_packet_to_list(bat_priv, skb, brd_delay, true);
/* a copy is stored in the bcast list, therefore removing
* the original skb.
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 0d46aea..98ebac0 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -408,6 +408,7 @@ struct batadv_hardif_neigh_node_bat_v {
* struct batadv_hardif_neigh_node - unique neighbor per hard-interface
* @list: list node for batadv_hard_iface::neigh_list
* @addr: the MAC address of the neighboring interface
+ * @orig: the address of the originator this neighbor node belongs to
* @if_incoming: pointer to incoming hard-interface
* @last_seen: when last packet via this neighbor was received
* @bat_v: B.A.T.M.A.N. V private data
@@ -417,6 +418,7 @@ struct batadv_hardif_neigh_node_bat_v {
struct batadv_hardif_neigh_node {
struct hlist_node list;
u8 addr[ETH_ALEN];
+ u8 orig[ETH_ALEN];
struct batadv_hard_iface *if_incoming;
unsigned long last_seen;
#ifdef CONFIG_BATMAN_ADV_BATMAN_V
--
2.10.1
^ permalink raw reply related
* [PATCH 07/17] batman-adv: Use own timer for multicast TT and TVLV updates
From: Simon Wunderlich @ 2016-11-08 16:45 UTC (permalink / raw)
To: davem
Cc: netdev, b.a.t.m.a.n, Linus Lüssing, Sven Eckelmann,
Simon Wunderlich
In-Reply-To: <20161108164526.7518-1-sw@simonwunderlich.de>
From: Linus Lüssing <linus.luessing@c0d3.blue>
Instead of latching onto the OGM period, this patch introduces a worker
dedicated to multicast TT and TVLV updates.
The reasoning is, that upon roaming especially the translation table
should be updated timely to minimize connectivity issues.
With BATMAN V, the idea is to greatly increase the OGM interval to
reduce overhead. Unfortunately, right now this could lead to
a bad user experience if multicast traffic is involved.
Therefore this patch introduces a fixed 500ms update interval for
multicast TT entries and the multicast TVLV.
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/main.h | 1 +
net/batman-adv/multicast.c | 62 ++++++++++++++++++++++++++++++++++----
net/batman-adv/multicast.h | 6 ----
net/batman-adv/translation-table.c | 4 ---
net/batman-adv/types.h | 4 ++-
5 files changed, 60 insertions(+), 17 deletions(-)
diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h
index daddca9..a6cc804 100644
--- a/net/batman-adv/main.h
+++ b/net/batman-adv/main.h
@@ -48,6 +48,7 @@
#define BATADV_TT_CLIENT_TEMP_TIMEOUT 600000 /* in milliseconds */
#define BATADV_TT_WORK_PERIOD 5000 /* 5 seconds */
#define BATADV_ORIG_WORK_PERIOD 1000 /* 1 second */
+#define BATADV_MCAST_WORK_PERIOD 500 /* 0.5 seconds */
#define BATADV_DAT_ENTRY_TIMEOUT (5 * 60000) /* 5 mins in milliseconds */
/* sliding packet range of received originator messages in sequence numbers
* (should be a multiple of our word size)
diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c
index 45757fa..090a69f 100644
--- a/net/batman-adv/multicast.c
+++ b/net/batman-adv/multicast.c
@@ -33,6 +33,7 @@
#include <linux/in6.h>
#include <linux/ip.h>
#include <linux/ipv6.h>
+#include <linux/jiffies.h>
#include <linux/kernel.h>
#include <linux/kref.h>
#include <linux/list.h>
@@ -48,6 +49,7 @@
#include <linux/stddef.h>
#include <linux/string.h>
#include <linux/types.h>
+#include <linux/workqueue.h>
#include <net/addrconf.h>
#include <net/if_inet6.h>
#include <net/ip.h>
@@ -60,6 +62,18 @@
#include "translation-table.h"
#include "tvlv.h"
+static void batadv_mcast_mla_update(struct work_struct *work);
+
+/**
+ * batadv_mcast_start_timer - schedule the multicast periodic worker
+ * @bat_priv: the bat priv with all the soft interface information
+ */
+static void batadv_mcast_start_timer(struct batadv_priv *bat_priv)
+{
+ queue_delayed_work(batadv_event_workqueue, &bat_priv->mcast.work,
+ msecs_to_jiffies(BATADV_MCAST_WORK_PERIOD));
+}
+
/**
* batadv_mcast_get_bridge - get the bridge on top of the softif if it exists
* @soft_iface: netdev struct of the mesh interface
@@ -255,6 +269,8 @@ static void batadv_mcast_mla_list_free(struct hlist_head *mcast_list)
* translation table except the ones listed in the given mcast_list.
*
* If mcast_list is NULL then all are retracted.
+ *
+ * Do not call outside of the mcast worker! (or cancel mcast worker first)
*/
static void batadv_mcast_mla_tt_retract(struct batadv_priv *bat_priv,
struct hlist_head *mcast_list)
@@ -262,7 +278,7 @@ static void batadv_mcast_mla_tt_retract(struct batadv_priv *bat_priv,
struct batadv_hw_addr *mcast_entry;
struct hlist_node *tmp;
- lockdep_assert_held(&bat_priv->tt.commit_lock);
+ WARN_ON(delayed_work_pending(&bat_priv->mcast.work));
hlist_for_each_entry_safe(mcast_entry, tmp, &bat_priv->mcast.mla_list,
list) {
@@ -287,6 +303,8 @@ static void batadv_mcast_mla_tt_retract(struct batadv_priv *bat_priv,
*
* Adds multicast listener announcements from the given mcast_list to the
* translation table if they have not been added yet.
+ *
+ * Do not call outside of the mcast worker! (or cancel mcast worker first)
*/
static void batadv_mcast_mla_tt_add(struct batadv_priv *bat_priv,
struct hlist_head *mcast_list)
@@ -294,7 +312,7 @@ static void batadv_mcast_mla_tt_add(struct batadv_priv *bat_priv,
struct batadv_hw_addr *mcast_entry;
struct hlist_node *tmp;
- lockdep_assert_held(&bat_priv->tt.commit_lock);
+ WARN_ON(delayed_work_pending(&bat_priv->mcast.work));
if (!mcast_list)
return;
@@ -528,13 +546,18 @@ static bool batadv_mcast_mla_tvlv_update(struct batadv_priv *bat_priv)
}
/**
- * batadv_mcast_mla_update - update the own MLAs
+ * __batadv_mcast_mla_update - update the own MLAs
* @bat_priv: the bat priv with all the soft interface information
*
* Updates the own multicast listener announcements in the translation
* table as well as the own, announced multicast tvlv container.
+ *
+ * Note that non-conflicting reads and writes to bat_priv->mcast.mla_list
+ * in batadv_mcast_mla_tt_retract() and batadv_mcast_mla_tt_add() are
+ * ensured by the non-parallel execution of the worker this function
+ * belongs to.
*/
-void batadv_mcast_mla_update(struct batadv_priv *bat_priv)
+static void __batadv_mcast_mla_update(struct batadv_priv *bat_priv)
{
struct net_device *soft_iface = bat_priv->soft_iface;
struct hlist_head mcast_list = HLIST_HEAD_INIT;
@@ -560,6 +583,29 @@ void batadv_mcast_mla_update(struct batadv_priv *bat_priv)
}
/**
+ * batadv_mcast_mla_update - update the own MLAs
+ * @work: kernel work struct
+ *
+ * Updates the own multicast listener announcements in the translation
+ * table as well as the own, announced multicast tvlv container.
+ *
+ * In the end, reschedules the work timer.
+ */
+static void batadv_mcast_mla_update(struct work_struct *work)
+{
+ struct delayed_work *delayed_work;
+ struct batadv_priv_mcast *priv_mcast;
+ struct batadv_priv *bat_priv;
+
+ delayed_work = to_delayed_work(work);
+ priv_mcast = container_of(delayed_work, struct batadv_priv_mcast, work);
+ bat_priv = container_of(priv_mcast, struct batadv_priv, mcast);
+
+ __batadv_mcast_mla_update(bat_priv);
+ batadv_mcast_start_timer(bat_priv);
+}
+
+/**
* batadv_mcast_is_report_ipv4 - check for IGMP reports
* @skb: the ethernet frame destined for the mesh
*
@@ -1128,6 +1174,9 @@ void batadv_mcast_init(struct batadv_priv *bat_priv)
batadv_tvlv_handler_register(bat_priv, batadv_mcast_tvlv_ogm_handler,
NULL, BATADV_TVLV_MCAST, 2,
BATADV_TVLV_HANDLER_OGM_CIFNOTFND);
+
+ INIT_DELAYED_WORK(&bat_priv->mcast.work, batadv_mcast_mla_update);
+ batadv_mcast_start_timer(bat_priv);
}
#ifdef CONFIG_BATMAN_ADV_DEBUGFS
@@ -1239,12 +1288,13 @@ int batadv_mcast_flags_seq_print_text(struct seq_file *seq, void *offset)
*/
void batadv_mcast_free(struct batadv_priv *bat_priv)
{
+ cancel_delayed_work_sync(&bat_priv->mcast.work);
+
batadv_tvlv_container_unregister(bat_priv, BATADV_TVLV_MCAST, 2);
batadv_tvlv_handler_unregister(bat_priv, BATADV_TVLV_MCAST, 2);
- spin_lock_bh(&bat_priv->tt.commit_lock);
+ /* safely calling outside of worker, as worker was canceled above */
batadv_mcast_mla_tt_retract(bat_priv, NULL);
- spin_unlock_bh(&bat_priv->tt.commit_lock);
}
/**
diff --git a/net/batman-adv/multicast.h b/net/batman-adv/multicast.h
index 1fb00ba..2cddaf5 100644
--- a/net/batman-adv/multicast.h
+++ b/net/batman-adv/multicast.h
@@ -39,8 +39,6 @@ enum batadv_forw_mode {
#ifdef CONFIG_BATMAN_ADV_MCAST
-void batadv_mcast_mla_update(struct batadv_priv *bat_priv);
-
enum batadv_forw_mode
batadv_mcast_forw_mode(struct batadv_priv *bat_priv, struct sk_buff *skb,
struct batadv_orig_node **mcast_single_orig);
@@ -55,10 +53,6 @@ void batadv_mcast_purge_orig(struct batadv_orig_node *orig_node);
#else
-static inline void batadv_mcast_mla_update(struct batadv_priv *bat_priv)
-{
-}
-
static inline enum batadv_forw_mode
batadv_mcast_forw_mode(struct batadv_priv *bat_priv, struct sk_buff *skb,
struct batadv_orig_node **mcast_single_orig)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index ad1e3bc..3cae8f4f 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -56,7 +56,6 @@
#include "hard-interface.h"
#include "hash.h"
#include "log.h"
-#include "multicast.h"
#include "netlink.h"
#include "originator.h"
#include "packet.h"
@@ -3795,9 +3794,6 @@ static void batadv_tt_local_commit_changes_nolock(struct batadv_priv *bat_priv)
{
lockdep_assert_held(&bat_priv->tt.commit_lock);
- /* Update multicast addresses in local translation table */
- batadv_mcast_mla_update(bat_priv);
-
if (atomic_read(&bat_priv->tt.local_changes) < 1) {
if (!batadv_atomic_dec_not_zero(&bat_priv->tt.ogm_append_cnt))
batadv_tt_tvlv_container_update(bat_priv);
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index c9db184..0d46aea 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -785,9 +785,10 @@ struct batadv_mcast_querier_state {
* @num_want_all_ipv6: counter for items in want_all_ipv6_list
* @want_lists_lock: lock for protecting modifications to mcast want lists
* (traversals are rcu-locked)
+ * @work: work queue callback item for multicast TT and TVLV updates
*/
struct batadv_priv_mcast {
- struct hlist_head mla_list;
+ struct hlist_head mla_list; /* see __batadv_mcast_mla_update() */
struct hlist_head want_all_unsnoopables_list;
struct hlist_head want_all_ipv4_list;
struct hlist_head want_all_ipv6_list;
@@ -802,6 +803,7 @@ struct batadv_priv_mcast {
atomic_t num_want_all_ipv6;
/* protects want_all_{unsnoopables,ipv4,ipv6}_list */
spinlock_t want_lists_lock;
+ struct delayed_work work;
};
#endif
--
2.10.1
^ permalink raw reply related
* [PATCH 12/17] batman-adv: Consume skb in batadv_send_skb_to_orig
From: Simon Wunderlich @ 2016-11-08 16:45 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Simon Wunderlich
In-Reply-To: <20161108164526.7518-1-sw@simonwunderlich.de>
From: Sven Eckelmann <sven@narfation.org>
Sending functions in Linux consume the supplied skbuff. Doing the same in
batadv_send_skb_to_orig avoids the hack of returning -1 (-EPERM) to signal
the caller that he is responsible for cleaning up the skb.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/routing.c | 11 ++---------
net/batman-adv/send.c | 39 ++++++++++++++++++++++-----------------
net/batman-adv/tp_meter.c | 6 ------
net/batman-adv/tvlv.c | 5 +----
4 files changed, 25 insertions(+), 36 deletions(-)
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index a4cb157..4d2679a 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -262,9 +262,6 @@ static int batadv_recv_my_icmp_packet(struct batadv_priv *bat_priv,
icmph->ttl = BATADV_TTL;
res = batadv_send_skb_to_orig(skb, orig_node, NULL);
- if (res == -1)
- goto out;
-
ret = NET_RX_SUCCESS;
break;
@@ -325,8 +322,7 @@ static int batadv_recv_icmp_ttl_exceeded(struct batadv_priv *bat_priv,
icmp_packet->ttl = BATADV_TTL;
res = batadv_send_skb_to_orig(skb, orig_node, NULL);
- if (res != -1)
- ret = NET_RX_SUCCESS;
+ ret = NET_RX_SUCCESS;
out:
if (primary_if)
@@ -413,8 +409,7 @@ int batadv_recv_icmp_packet(struct sk_buff *skb,
/* route it */
res = batadv_send_skb_to_orig(skb, orig_node, recv_if);
- if (res != -1)
- ret = NET_RX_SUCCESS;
+ ret = NET_RX_SUCCESS;
out:
if (orig_node)
@@ -702,8 +697,6 @@ static int batadv_route_unicast_packet(struct sk_buff *skb,
len = skb->len;
res = batadv_send_skb_to_orig(skb, orig_node, recv_if);
- if (res == -1)
- goto out;
/* translate transmit result into receive result */
if (res == NET_XMIT_SUCCESS) {
diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c
index 0f86293..b00aac7 100644
--- a/net/batman-adv/send.c
+++ b/net/batman-adv/send.c
@@ -165,11 +165,9 @@ int batadv_send_unicast_skb(struct sk_buff *skb,
* host, NULL can be passed as recv_if and no interface alternating is
* attempted.
*
- * Return: -1 on failure (and the skb is not consumed), -EINPROGRESS if the
- * skb is buffered for later transmit or the NET_XMIT status returned by the
+ * Return: negative errno code on a failure, -EINPROGRESS if the skb is
+ * buffered for later transmit or the NET_XMIT status returned by the
* lower routine if the packet has been passed down.
- *
- * If the returning value is not -1 the skb has been consumed.
*/
int batadv_send_skb_to_orig(struct sk_buff *skb,
struct batadv_orig_node *orig_node,
@@ -177,12 +175,14 @@ int batadv_send_skb_to_orig(struct sk_buff *skb,
{
struct batadv_priv *bat_priv = orig_node->bat_priv;
struct batadv_neigh_node *neigh_node;
- int ret = -1;
+ int ret;
/* batadv_find_router() increases neigh_nodes refcount if found. */
neigh_node = batadv_find_router(bat_priv, orig_node, recv_if);
- if (!neigh_node)
- goto out;
+ if (!neigh_node) {
+ ret = -EINVAL;
+ goto free_skb;
+ }
/* Check if the skb is too large to send in one piece and fragment
* it if needed.
@@ -191,8 +191,10 @@ int batadv_send_skb_to_orig(struct sk_buff *skb,
skb->len > neigh_node->if_incoming->net_dev->mtu) {
/* Fragment and send packet. */
ret = batadv_frag_send_packet(skb, orig_node, neigh_node);
+ /* skb was consumed */
+ skb = NULL;
- goto out;
+ goto put_neigh_node;
}
/* try to network code the packet, if it is received on an interface
@@ -204,9 +206,13 @@ int batadv_send_skb_to_orig(struct sk_buff *skb,
else
ret = batadv_send_unicast_skb(skb, neigh_node);
-out:
- if (neigh_node)
- batadv_neigh_node_put(neigh_node);
+ /* skb was consumed */
+ skb = NULL;
+
+put_neigh_node:
+ batadv_neigh_node_put(neigh_node);
+free_skb:
+ kfree_skb(skb);
return ret;
}
@@ -327,7 +333,7 @@ int batadv_send_skb_unicast(struct batadv_priv *bat_priv,
{
struct batadv_unicast_packet *unicast_packet;
struct ethhdr *ethhdr;
- int res, ret = NET_XMIT_DROP;
+ int ret = NET_XMIT_DROP;
if (!orig_node)
goto out;
@@ -364,13 +370,12 @@ int batadv_send_skb_unicast(struct batadv_priv *bat_priv,
if (batadv_tt_global_client_is_roaming(bat_priv, ethhdr->h_dest, vid))
unicast_packet->ttvn = unicast_packet->ttvn - 1;
- res = batadv_send_skb_to_orig(skb, orig_node, NULL);
- if (res != -1)
- ret = NET_XMIT_SUCCESS;
+ ret = batadv_send_skb_to_orig(skb, orig_node, NULL);
+ /* skb was consumed */
+ skb = NULL;
out:
- if (ret == NET_XMIT_DROP)
- kfree_skb(skb);
+ kfree_skb(skb);
return ret;
}
diff --git a/net/batman-adv/tp_meter.c b/net/batman-adv/tp_meter.c
index 2333777..f156452 100644
--- a/net/batman-adv/tp_meter.c
+++ b/net/batman-adv/tp_meter.c
@@ -615,9 +615,6 @@ static int batadv_tp_send_msg(struct batadv_tp_vars *tp_vars, const u8 *src,
batadv_tp_fill_prerandom(tp_vars, data, data_len);
r = batadv_send_skb_to_orig(skb, orig_node, NULL);
- if (r == -1)
- kfree_skb(skb);
-
if (r == NET_XMIT_SUCCESS)
return 0;
@@ -1206,9 +1203,6 @@ static int batadv_tp_send_ack(struct batadv_priv *bat_priv, const u8 *dst,
/* send the ack */
r = batadv_send_skb_to_orig(skb, orig_node, NULL);
- if (r == -1)
- kfree_skb(skb);
-
if (unlikely(r < 0) || (r == NET_XMIT_DROP)) {
ret = BATADV_TP_REASON_DST_UNREACHABLE;
goto out;
diff --git a/net/batman-adv/tvlv.c b/net/batman-adv/tvlv.c
index 77654f0..a783420 100644
--- a/net/batman-adv/tvlv.c
+++ b/net/batman-adv/tvlv.c
@@ -600,7 +600,6 @@ void batadv_tvlv_unicast_send(struct batadv_priv *bat_priv, u8 *src,
unsigned char *tvlv_buff;
unsigned int tvlv_len;
ssize_t hdr_len = sizeof(*unicast_tvlv_packet);
- int res;
orig_node = batadv_orig_hash_find(bat_priv, dst);
if (!orig_node)
@@ -633,9 +632,7 @@ void batadv_tvlv_unicast_send(struct batadv_priv *bat_priv, u8 *src,
tvlv_buff += sizeof(*tvlv_hdr);
memcpy(tvlv_buff, tvlv_value, tvlv_value_len);
- res = batadv_send_skb_to_orig(skb, orig_node, NULL);
- if (res == -1)
- kfree_skb(skb);
+ batadv_send_skb_to_orig(skb, orig_node, NULL);
out:
batadv_orig_node_put(orig_node);
}
--
2.10.1
^ permalink raw reply related
* [PATCH 05/17] batman-adv: Remove unnecessary lockdep in batadv_mcast_mla_list_free
From: Simon Wunderlich @ 2016-11-08 16:45 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r
In-Reply-To: <20161108164526.7518-1-sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX@public.gmane.org>
From: Linus Lüssing <linus.luessing-djzkFPsfvsizQB+pC5nmwQ@public.gmane.org>
batadv_mcast_mla_list_free() just frees some leftovers of a local feast
in batadv_mcast_mla_update(). No lockdep needed as it has nothing to do
with bat_priv->mcast.mla_list.
Signed-off-by: Linus Lüssing <linus.luessing-djzkFPsfvsizQB+pC5nmwQ@public.gmane.org>
Signed-off-by: Sven Eckelmann <sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org>
Signed-off-by: Simon Wunderlich <sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX@public.gmane.org>
---
net/batman-adv/multicast.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c
index 13661f4..45757fa 100644
--- a/net/batman-adv/multicast.c
+++ b/net/batman-adv/multicast.c
@@ -231,19 +231,15 @@ static int batadv_mcast_mla_bridge_get(struct net_device *dev,
/**
* batadv_mcast_mla_list_free - free a list of multicast addresses
- * @bat_priv: the bat priv with all the soft interface information
* @mcast_list: the list to free
*
* Removes and frees all items in the given mcast_list.
*/
-static void batadv_mcast_mla_list_free(struct batadv_priv *bat_priv,
- struct hlist_head *mcast_list)
+static void batadv_mcast_mla_list_free(struct hlist_head *mcast_list)
{
struct batadv_hw_addr *mcast_entry;
struct hlist_node *tmp;
- lockdep_assert_held(&bat_priv->tt.commit_lock);
-
hlist_for_each_entry_safe(mcast_entry, tmp, mcast_list, list) {
hlist_del(&mcast_entry->list);
kfree(mcast_entry);
@@ -560,7 +556,7 @@ void batadv_mcast_mla_update(struct batadv_priv *bat_priv)
batadv_mcast_mla_tt_add(bat_priv, &mcast_list);
out:
- batadv_mcast_mla_list_free(bat_priv, &mcast_list);
+ batadv_mcast_mla_list_free(&mcast_list);
}
/**
--
2.10.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox