* [PATCH net-next] net: cavium: use module_pci_driver to simplify the code
From: Wei Yongjun @ 2018-03-28 12:51 UTC (permalink / raw)
To: Radoslaw Biernacki, Aleksey Makarov, Philippe Ombredanne,
Jan Glauber
Cc: Wei Yongjun, netdev, kernel-janitors
Use the module_pci_driver() macro to make the code simpler
by eliminating module_init and module_exit calls.
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
drivers/net/ethernet/cavium/common/cavium_ptp.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/cavium/common/cavium_ptp.c b/drivers/net/ethernet/cavium/common/cavium_ptp.c
index d59497a..6aeb104 100644
--- a/drivers/net/ethernet/cavium/common/cavium_ptp.c
+++ b/drivers/net/ethernet/cavium/common/cavium_ptp.c
@@ -336,18 +336,7 @@ static void cavium_ptp_remove(struct pci_dev *pdev)
.remove = cavium_ptp_remove,
};
-static int __init cavium_ptp_init_module(void)
-{
- return pci_register_driver(&cavium_ptp_driver);
-}
-
-static void __exit cavium_ptp_cleanup_module(void)
-{
- pci_unregister_driver(&cavium_ptp_driver);
-}
-
-module_init(cavium_ptp_init_module);
-module_exit(cavium_ptp_cleanup_module);
+module_pci_driver(cavium_ptp_driver);
MODULE_DESCRIPTION(DRV_NAME);
MODULE_AUTHOR("Cavium Networks <support@cavium.com>");
^ permalink raw reply related
* Re: [RFC v3 net-next 13/18] net/sched: Introduce the TBS Qdisc
From: Henrik Austad @ 2018-03-28 13:07 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Jesus Sanchez-Palencia, netdev, jhs, xiyou.wangcong, jiri,
vinicius.gomes, richardcochran, anna-maria, John Stultz,
levi.pearson, edumazet, willemb, mlichvar
In-Reply-To: <alpine.DEB.2.21.1803280808490.3247@nanos.tec.linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 13445 bytes --]
On Wed, Mar 28, 2018 at 09:48:05AM +0200, Thomas Gleixner wrote:
> Jesus,
Thomas, Jesus,
> On Tue, 27 Mar 2018, Jesus Sanchez-Palencia wrote:
> > On 03/25/2018 04:46 AM, Thomas Gleixner wrote:
> > > This is missing right now and you want to get that right from the very
> > > beginning. Duct taping it on the interface later on is a bad idea.
> >
> > Agreed that this is needed. On the SO_TXTIME + tbs proposal, I believe it's been
> > covered by the (per-packet) SCM_DROP_IF_LATE. Do you think we need a different
> > mechanism for expressing that?
>
> Uuurgh. No. DROP_IF_LATE is just crap to be honest.
>
> There are two modes:
>
> 1) Send at the given TX time (Explicit mode)
>
> 2) Send before given TX time (Deadline mode)
>
> There is no need to specify 'drop if late' simply because if the message is
> handed in past the given TX time, it's too late by definition. What you are
> trying to implement is a hybrid of TSN and general purpose (not time aware)
> networking in one go. And you do that because your overall design is not
> looking at the big picture. You designed from a given use case assumption
> and tried to fit other things into it with duct tape.
Yes, +1 to this. The whole point of bandwidth reservation is to not drop
frames, you should never, ever miss a deadline, if you do, then your
admission tests are inadequate.
> > > So you really want a way for the application to query the timing
> > > constraints and perhaps other properties of the channel it connects
> > > to. And you want that now before the first application starts to use the
> > > new ABI. If the application developer does not use it, you still have to
> > > fix the application, but you have to fix it because the developer was a
> > > lazy bastard and not because the design was bad. That's a major
> > > difference.
> >
> > Ok, this is something that we have considered in the past, but then the feedback
> > here drove us onto a different direction. The overall input we got here was that
> > applications would have to be adjusted or that userspace would have to handle
> > the coordination between applications somehow (e.g.: a daemon could be developed
> > separately to accommodate the fully dynamic use-cases, etc).
>
> The only thing which will happen is that you get applications which require
> to control the full interface themself because they are so important and
> the only ones which get it right. Good luck with fixing them up.
>
> That extra daemon if it ever surfaces will be just a PITA. Think about
> 20khz control loops. Do you really want queueing, locking, several context
> switches and priority configuration nightmares in such a scenario?
> Definitely not! You want a fast channel directly to the root qdisc which
> takes care of getting it out at the right point, which might be immediate
> handover if the adapter supports hw scheduling.
>
> > This is a new requirement for the entire discussion.
> > If I'm not missing anything, however, underutilization of the time slots is only
> > a problem:
> >
> > 1) for the fully dynamic use-cases and;
> > 2) because now you are designing applications in terms of time slices, right?
>
> No. It's a general problem. I'm not designing applications in terms of time
> slices. Time slices are a fundamental property of TSN. Whether you use them
> for explicit scheduling or bandwidth reservation or make them flat does not
> matter.
>
> The application does not necessarily need to know about the time
> constraints at all. But if it wants to use timed scheduling then it better
> does know about them.
yep, +1 in a lot of A/V cases here, the application will have to know about
presentation_time, and the delay through the network stack should be "low
and deterministic", but apart from that, the application shouldn't have to
care about SO_TXTIME and what other applications may or may not do.
> > We have not thought of making any of the proposed qdiscs capable of (optionally)
> > adjusting the "time slices", but mainly because this is not a problem we had
> > here before. Our assumption was that per-port Tx schedules would only be used
> > for static systems. In other words, no, we didn't think that re-balancing the
> > slots was a requirement, not even for 'taprio'.
>
> Sigh. Utilization is not something entirely new in the network space. I'm
> not saying that this needs to be implemented right away, but designing it
> in a way which forces underutilization is just wrong.
>
> > > Coming back to the overall scheme. If you start upfront with a time slice
> > > manager which is designed to:
> > >
> > > - Handle multiple channels
> > >
> > > - Expose the time constraints, properties per channel
> > >
> > > then you can fit all kind of use cases, whether designed by committee or
> > > not. You can configure that thing per node or network wide. It does not
> > > make a difference. The only difference are the resulting constraints.
> >
> >
> > Ok, and I believe the above was covered by what we had proposed before, unless
> > what you meant by time constraints is beyond the configured port schedule.
> >
> > Are you suggesting that we'll need to have a kernel entity that is not only
> > aware of the current traffic classes 'schedule', but also of the resources that
> > are still available for new streams to be accommodated into the classes? Putting
> > it differently, is the TAS you envision just an entity that runs a schedule, or
> > is it a time-aware 'orchestrator'?
>
> In the first place its something which runs a defined schedule.
>
> The accomodation for new streams is required, but not necessarily at the
> root qdisc level. That might be a qdisc feeding into it.
>
> Assume you have a bandwidth reservation, aka time slot, for audio. If your
> audio related qdisc does deadline scheduling then you can add new streams
> to it up to the point where it's not longer able to fit.
>
> The only thing which might be needed at the root qdisc is the ability to
> utilize unused time slots for other purposes, but that's not required to be
> there in the first place as long as its designed in a way that it can be
> added later on.
>
> > > So lets look once more at the picture in an abstract way:
> > >
> > > [ NIC ]
> > > |
> > > [ Time slice manager ]
> > > | |
> > > [ Ch 0 ] ... [ Ch N ]
> > >
> > > So you have a bunch of properties here:
> > >
> > > 1) Number of Channels ranging from 1 to N
> > >
> > > 2) Start point, slice period and slice length per channel
> >
> > Ok, so we agree that a TAS entity is needed. Assuming that channels are traffic
> > classes, do you have something else in mind other than a new root qdisc?
>
> Whatever you call it, the important point is that it is the gate keeper to
> the network adapter and there is no way around it. It fully controls the
> timed schedule how simple or how complex it may be.
>
> > > 3) Queueing modes assigned per channel. Again that might be anything from
> > > 'feed through' over FIFO, PRIO to more complex things like EDF.
> > >
> > > The queueing mode can also influence properties like the meaning of the
> > > TX time, i.e. strict or deadline.
> >
> >
> > Ok, but how are the queueing modes assigned / configured per channel?
> >
> > Just to make sure we re-visit some ideas from the past:
> >
> > * TAS:
> >
> > The idea we are currently exploring is to add a "time-aware", priority based
> > qdisc, that also exposes the Tx queues available and provides a mechanism for
> > mapping priority <-> traffic class <-> Tx queues in a similar fashion as
> > mqprio. We are calling this qdisc 'taprio', and its 'tc' cmd line would be:
> >
> > $ $ tc qdisc add dev ens4 parent root handle 100 taprio num_tc 4 \
> > map 2 2 1 0 3 3 3 3 3 3 3 3 3 3 3 3 \
> > queues 0 1 2 3 \
> > sched-file gates.sched [base-time <interval>] \
> > [cycle-time <interval>] [extension-time <interval>]
> >
> > <file> is multi-line, with each line being of the following format:
> > <cmd> <gate mask> <interval in nanoseconds>
> >
> > Qbv only defines one <cmd>: "S" for 'SetGates'
> >
> > For example:
> >
> > S 0x01 300
> > S 0x03 500
> >
> > This means that there are two intervals, the first will have the gate
> > for traffic class 0 open for 300 nanoseconds, the second will have
> > both traffic classes open for 500 nanoseconds.
>
> To accomodate stuff like control systems you also need a base line, which
> is not expressed as interval. Otherwise you can't schedule network wide
> explicit plans. That's either an absolute network-time (TAI) time stamp or
> an offset to a well defined network-time (TAI) time stamp, e.g. start of
> epoch or something else which is agreed on. The actual schedule then fast
> forwards past now (TAI) and sets up the slots from there. That makes node
> hotplug possible as well.
Ok, so this is perhaps a bit of a sidetrack, but based on other discussions
in this patch-series, does it really make sense to discuss anything *but*
TAI?
If you have a TSN-stream (or any other time-sensitive way of prioritizing
frames based on time), then the network is going to be PTP synched anyway,
and all the rest of the network is going to operate on PTP-time. Why even
bother adding CLOCK_REALTIME and CLOCK_MONOTONIC to the discussion? Sure,
use CLOCK_REALTIME locally and sync that to TAI, but the kernel should
worry about ptp-time _for_that_adapter_, and we should make it pretty
obvious to userspace that if you want to specify tx-time, then there's this
thing called 'PTP' and it rules this domain. My $0.02 etc
> Btw, it's not only control systems. Think about complex multi source A/V
> streams. They are reality in recording and life mixing and looking at the
> timing constraints of such scenarios, collision avoidance is key there. So
> you want to be able to do network wide traffic orchestration.
Yep, and if are too bursty, the network is free to drop your frames, which
is not desired.
> > It would handle multiple channels and expose their constraints / properties.
> > Each channel also becomes a traffic class, so other qdiscs can be attached to
> > them separately.
>
> Right.
I don't think you need a separate qdisc for each channel, if you describe a
channel with
- period (what AVB calls observation interval)
- max data
- deadline
you should be able to keep a sorted rb-tree and handle that pretty
efficiently. Or perhaps I'm completely missing the mark here. If so, my
apologies
> > So, in summary, because our entire design is based on qdisc interfaces, what we
> > had proposed was a root qdisc (the time slice manager, as you put) that allows
> > for other qdiscs to be attached to each channel. The inner qdiscs define the
> > queueing modes for each channel, and tbs is just one of those modes. I
> > understand now that you want to allow for fully dynamic use-cases to be
> > supported as well, which we hadn't covered with our TAS proposal before because
> > we hadn't envisioned it being used for these systems' design.
>
> Yes, you have the root qdisc, which is in charge of the overall scheduling
> plan, how complex or not it is defined does not matter. It exposes traffic
> classes which have properties defined by the configuration.
>
> The qdiscs which are attached to those traffic classes can be anything
> including:
>
> - Simple feed through (Applications are time contraints aware and set the
> exact schedule). qdisc has admission control.
>
> - Deadline aware qdisc to handle e.g. A/V streams. Applications are aware
> of time constraints and provide the packet deadline. qdisc has admission
> control. This can be a simple first comes, first served scheduler or
> something like EDF which allows optimized utilization. The qdisc sets
> the TX time depending on the deadline and feeds into the root.
As a small nitpick, it would make more sense to do a laxity-approach here,
both for explicit mode and deadline-mode. We know the size of the frame to
send, we know the outgoing rate, so keep a ready-queue sorted based on
laxity
laxity = absolute_deadline - (size / outgoing_rate)
Also, given that we use a *single* tx-queue for time-triggered
transmission, this boils down to a uniprocessor equivalent and we have a
lot of func real-time scheduling academia to draw from.
This could then probably handle both of the above (Direct + deadline), but
that's implementatino specific I guess.
> - FIFO/PRIO/XXX for general traffic. Applications do not know anything
> about timing constraints. These qdiscs obviously have neither admission
> control nor do they set a TX time. The root qdisc just pulls from there
> when the assigned time slot is due or if it (optionally) decides to use
> underutilized time slots from other classes.
>
> - .... Add your favourite scheduling mode(s).
Just give it sub-qdiscs and offload enqueue/dequeue to those I suppose.
--
Henrik Austad
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH] test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()
From: Daniel Borkmann @ 2018-03-28 13:25 UTC (permalink / raw)
To: Dan Carpenter, Alexei Starovoitov, Yonghong Song; +Cc: netdev, kernel-janitors
In-Reply-To: <20180328114836.GD29050@mwanda>
On 03/28/2018 01:48 PM, Dan Carpenter wrote:
> The skb_segment() function returns error pointers on error. It never
> returns NULL.
>
> Fixes: 76db8087c4c9 ("net: bpf: add a test for skb_segment in test_bpf module")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Patch is directly for net-next, thanks!
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: 4.14.29 - tcp_push() - null skb's cb dereference
From: Eric Dumazet @ 2018-03-28 13:38 UTC (permalink / raw)
To: Krzysztof Blaszkowski, netdev; +Cc: Greg Kroah-Hartman, David S. Miller
In-Reply-To: <1522234312.25303.15.camel@home.com.pl>
On 03/28/2018 03:51 AM, Krzysztof Blaszkowski wrote:
> Hi,
>
> I noticed a kernel bug report like below:
>
> [95576.826393] BUG: unable to handle kernel NULL pointer dereference at
> 0000000000000038
> [95576.834296] IP: tcp_push+0x3d/0x110
> [95576.837829] PGD 2c8474067 P4D 2c8474067 PUD 1119cf067 PMD 0
> [95576.843536] Oops: 0002 [#1] SMP NOPTI
> [95576.847247] CPU: 3 PID: 1682 Comm: nginx Not tainted 4.14.29 #1
> [95576.854421] Hardware name: PC-FACTORY empty/Tyan Transport GT24-
> B3992-E, BIOS 'V1.06.B10 ' 06/23/2009
> [95576.863678] task: ffff9e4f150b2580 task.stack: ffffb21401a50000
> [95576.869641] RIP: 0010:tcp_push+0x3d/0x110
> [95576.873692] RSP: 0018:ffffb21401a53be0 EFLAGS: 00010246
> [95576.878959] RAX: 0000000000000000 RBX: 0000000000009310 RCX:
> 0000000000000000
> [95576.886133] RDX: 0000000000000001 RSI: 0000000000000040 RDI:
> ffff9e4e79871f00
> [95576.893306] RBP: ffffb21401a53c78 R08: 00000000000065d0 R09:
> ffff9e4e79872048
> [95576.900479] R10: 00000000000005a8 R11: 0000000000000000 R12:
> 00000000ffffffe0
> [95576.907652] R13: ffffb21401a53cf8 R14: ffff9e4e79871f00 R15:
> ffff9e4f68727100
> [95576.914824] FS: 00007f683b4e1700(0000) GS:ffff9e50732c0000(0000)
> knlGS:0000000000000000
> [95576.922953] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [95576.928737] CR2: 0000000000000038 CR3: 000000020aa0e000 CR4:
> 00000000000006e0
> [95576.935908] Call Trace:
> [95576.938403] ? tcp_sendmsg_locked+0x60a/0xd90
> [95576.942872] tcp_sendmsg+0x27/0x40
> [95576.946386] inet_sendmsg+0x2c/0xa0
> [95576.949987] sock_sendmsg+0x33/0x40
> [95576.953587] sock_write_iter+0x76/0xd0
> [95576.957448] do_iter_readv_writev+0x108/0x160
> [95576.961917] do_iter_write+0x82/0x190
> [95576.965690] vfs_writev+0xbb/0x120
> [95576.969203] ? ep_poll+0x240/0x3f0
> [95576.972714] do_writev+0x4d/0xd0
> [95576.976052] ? do_writev+0x4d/0xd0
> [95576.979564] SyS_writev+0xb/0x10
> [95576.982905] do_syscall_64+0x5a/0x110
> [95576.986678] entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> [95576.991838] RIP: 0033:0x7f6839e56170
> [95576.995523] RSP: 002b:00007ffe34c9cf48 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000014
> [95577.003265] RAX: ffffffffffffffda RBX: 0000000004b622c0 RCX:
> 00007f6839e56170
> [95577.010507] RDX: 0000000000000017 RSI: 00007ffe34c9cfd0 RDI:
> 0000000000000012
> [95577.017749] RBP: 00007ffe34c9cfb0 R08: 00000000004b1c2f R09:
> 00007ffe34c9d130
> [95577.024989] R10: 000000000001d6c2 R11: 0000000000000246 R12:
> 0000000002f3eec0
> [95577.032228] R13: 000000007fffefff R14: 0000000000000000 R15:
> 0000000002f3eec0
> [95577.039469] Code: 01 00 00 4c 8d 8f 48 01 00 00 41 89 d2 41 89 f3 89
> ca b9 00 00 00 00 49 39 c1 48 0f 44 c1 41 81 e3 00 80 00 00 0f 85 ac 00
> 00 00 <80> 48 38 08 8b 8f 64 06 00 00 89 8f 6c 06 00 00 83 e6 01 74 0c
> [95577.058617] RIP: tcp_push+0x3d/0x110 RSP: ffffb21401a53be0
> [95577.064208] CR2: 0000000000000038
> [95577.068056] ---[ end trace 19bfaf872fd3ef10 ]--
>
>
>
> further report analysis ended up in net/ipv4/tcp.c :
>
>
> (gdb) list *(tcp_push+0x3d)
> 0xfd is in tcp_push (/data/work/linux-4.14.29/net/ipv4/tcp.c:630).
> 625 }
> 626 EXPORT_SYMBOL(tcp_ioctl);
> 627
> 628 static inline void tcp_mark_push(struct tcp_sock *tp, struct
> sk_buff *skb)
> 629 {
> 630 TCP_SKB_CB(skb)->tcp_flags |= TCPHDR_PSH;
> 631 tp->pushed_seq = tp->write_seq;
> 632 }
>
>
> and tcp_mark_push() is inlined in tcp_push() indeed.
>
> Looking forward to seeing rationale for tcp_write_queue_tail(sk)
> returning null skb - assuming the above is correct.
>
> Thanks
This has been reported 5 or 6 times already :/
For some reason the fix is missing from 4.14 stable tree.
https://patchwork.ozlabs.org/patch/886324/
Note this patch was needed even before a27fid7a8ed38 (tcp: purge write queue upon RST)
since tcp disconnect would have caused the same issue.
Thanks.
^ permalink raw reply
* Re: [PATCH] samples/bpf: fix spelling mistake: "revieve" -> "receive"
From: Daniel Borkmann @ 2018-03-28 13:38 UTC (permalink / raw)
To: Colin King, Alexei Starovoitov, netdev; +Cc: kernel-janitors, linux-kernel
In-Reply-To: <20180328100701.13289-1-colin.king@canonical.com>
On 03/28/2018 12:07 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistake in error message text
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied to bpf-next, thanks Colin!
^ permalink raw reply
* Re: 4.14.29 - tcp_push() - null skb's cb dereference
From: Krzysztof Blaszkowski @ 2018-03-28 13:43 UTC (permalink / raw)
To: Eric Dumazet, netdev; +Cc: Greg Kroah-Hartman, David S. Miller
In-Reply-To: <14343877-a2a0-215e-e052-2dfb7e4c4b09@gmail.com>
On Wed, 2018-03-28 at 06:38 -0700, Eric Dumazet wrote:
>
> On 03/28/2018 03:51 AM, Krzysztof Blaszkowski wrote:
> >
> > Hi,
> >
> > I noticed a kernel bug report like below:
> >
> > [95576.826393] BUG: unable to handle kernel NULL pointer
> > dereference at
> > 0000000000000038
> > [95576.834296] IP: tcp_push+0x3d/0x110
> > [95576.837829] PGD 2c8474067 P4D 2c8474067 PUD 1119cf067 PMD 0
> > [95576.843536] Oops: 0002 [#1] SMP NOPTI
> > [95576.847247] CPU: 3 PID: 1682 Comm: nginx Not tainted 4.14.29 #1
> > [95576.854421] Hardware name: PC-FACTORY empty/Tyan Transport GT24-
> > B3992-E, BIOS 'V1.06.B10 ' 06/23/2009
> > [95576.863678] task: ffff9e4f150b2580 task.stack: ffffb21401a50000
> > [95576.869641] RIP: 0010:tcp_push+0x3d/0x110
> > [95576.873692] RSP: 0018:ffffb21401a53be0 EFLAGS: 00010246
> > [95576.878959] RAX: 0000000000000000 RBX: 0000000000009310 RCX:
> > 0000000000000000
> > [95576.886133] RDX: 0000000000000001 RSI: 0000000000000040 RDI:
> > ffff9e4e79871f00
> > [95576.893306] RBP: ffffb21401a53c78 R08: 00000000000065d0 R09:
> > ffff9e4e79872048
> > [95576.900479] R10: 00000000000005a8 R11: 0000000000000000 R12:
> > 00000000ffffffe0
> > [95576.907652] R13: ffffb21401a53cf8 R14: ffff9e4e79871f00 R15:
> > ffff9e4f68727100
> > [95576.914824] FS: 00007f683b4e1700(0000)
> > GS:ffff9e50732c0000(0000)
> > knlGS:0000000000000000
> > [95576.922953] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [95576.928737] CR2: 0000000000000038 CR3: 000000020aa0e000 CR4:
> > 00000000000006e0
> > [95576.935908] Call Trace:
> > [95576.938403] ? tcp_sendmsg_locked+0x60a/0xd90
> > [95576.942872] tcp_sendmsg+0x27/0x40
> > [95576.946386] inet_sendmsg+0x2c/0xa0
> > [95576.949987] sock_sendmsg+0x33/0x40
> > [95576.953587] sock_write_iter+0x76/0xd0
> > [95576.957448] do_iter_readv_writev+0x108/0x160
> > [95576.961917] do_iter_write+0x82/0x190
> > [95576.965690] vfs_writev+0xbb/0x120
> > [95576.969203] ? ep_poll+0x240/0x3f0
> > [95576.972714] do_writev+0x4d/0xd0
> > [95576.976052] ? do_writev+0x4d/0xd0
> > [95576.979564] SyS_writev+0xb/0x10
> > [95576.982905] do_syscall_64+0x5a/0x110
> > [95576.986678] entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> > [95576.991838] RIP: 0033:0x7f6839e56170
> > [95576.995523] RSP: 002b:00007ffe34c9cf48 EFLAGS: 00000246
> > ORIG_RAX:
> > 0000000000000014
> > [95577.003265] RAX: ffffffffffffffda RBX: 0000000004b622c0 RCX:
> > 00007f6839e56170
> > [95577.010507] RDX: 0000000000000017 RSI: 00007ffe34c9cfd0 RDI:
> > 0000000000000012
> > [95577.017749] RBP: 00007ffe34c9cfb0 R08: 00000000004b1c2f R09:
> > 00007ffe34c9d130
> > [95577.024989] R10: 000000000001d6c2 R11: 0000000000000246 R12:
> > 0000000002f3eec0
> > [95577.032228] R13: 000000007fffefff R14: 0000000000000000 R15:
> > 0000000002f3eec0
> > [95577.039469] Code: 01 00 00 4c 8d 8f 48 01 00 00 41 89 d2 41 89
> > f3 89
> > ca b9 00 00 00 00 49 39 c1 48 0f 44 c1 41 81 e3 00 80 00 00 0f 85
> > ac 00
> > 00 00 <80> 48 38 08 8b 8f 64 06 00 00 89 8f 6c 06 00 00 83 e6 01 74
> > 0c
> > [95577.058617] RIP: tcp_push+0x3d/0x110 RSP: ffffb21401a53be0
> > [95577.064208] CR2: 0000000000000038
> > [95577.068056] ---[ end trace 19bfaf872fd3ef10 ]--
> >
> >
> >
> > further report analysis ended up in net/ipv4/tcp.c :
> >
> >
> > (gdb) list *(tcp_push+0x3d)
> > 0xfd is in tcp_push (/data/work/linux-4.14.29/net/ipv4/tcp.c:630).
> > 625 }
> > 626 EXPORT_SYMBOL(tcp_ioctl);
> > 627
> > 628 static inline void tcp_mark_push(struct tcp_sock *tp,
> > struct
> > sk_buff *skb)
> > 629 {
> > 630 TCP_SKB_CB(skb)->tcp_flags |= TCPHDR_PSH;
> > 631 tp->pushed_seq = tp->write_seq;
> > 632 }
> >
> >
> > and tcp_mark_push() is inlined in tcp_push() indeed.
> >
> > Looking forward to seeing rationale for tcp_write_queue_tail(sk)
> > returning null skb - assuming the above is correct.
> >
> > Thanks
>
> This has been reported 5 or 6 times already :/
>
> For some reason the fix is missing from 4.14 stable tree.
>
> https://patchwork.ozlabs.org/patch/886324/
>
> Note this patch was needed even before a27fid7a8ed38 (tcp: purge
> write queue upon RST)
> since tcp disconnect would have caused the same issue.
>
>
>
> Thanks.
I see. Many thanks.
>
>
^ permalink raw reply
* Re: [PATCH bpf-next]: add sock_ops R/W access to ipv4 tos
From: Daniel Borkmann @ 2018-03-28 13:47 UTC (permalink / raw)
To: Nikita V. Shirokov, brakmo, ast, netdev; +Cc: kernel-team
In-Reply-To: <20180326153657.2229959-1-tehnerd@fb.com>
On 03/26/2018 05:36 PM, Nikita V. Shirokov wrote:
> bpf: Add sock_ops R/W access to ipv4 tos
>
> Sample usage for tos:
>
> bpf_getsockopt(skops, SOL_IP, IP_TOS, &v, sizeof(v))
>
> where skops is a pointer to the ctx (struct bpf_sock_ops).
>
> Signed-off-by: Nikita V. Shirokov <tehnerd@fb.com>
> ---
> net/core/filter.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 00c711c..afd8255 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3462,6 +3462,27 @@ BPF_CALL_5(bpf_setsockopt, struct bpf_sock_ops_kern *, bpf_sock,
> ret = -EINVAL;
> }
> #ifdef CONFIG_INET
> + } else if (level == SOL_IP) {
> + if (optlen != sizeof(int) || sk->sk_family != AF_INET)
> + return -EINVAL;
> +
> + val = *((int *)optval);
> + /* Only some options are supported */
> + switch (optname) {
> + case IP_TOS:
> + if (val < -1 || val > 0xff) {
> + ret = -EINVAL;
> + } else {
> + struct inet_sock *inet = inet_sk(sk);
> +
> + if (val == -1)
> + val = 0;
> + inet->tos = val;
Should this not have the exact same semantics given the helper resembles
the normal setsockopt? do_ip_setsockopt() does the following when setting
IP_TOS:
case IP_TOS: /* This sets both TOS and Precedence */
if (sk->sk_type == SOCK_STREAM) {
val &= ~INET_ECN_MASK;
val |= inet->tos & INET_ECN_MASK;
}
if (inet->tos != val) {
inet->tos = val;
sk->sk_priority = rt_tos2priority(val);
sk_dst_reset(sk);
}
break;
E.g. why we don't need to set sk->sk_priority as well or reset the dst
entry here?
> + }
> + break;
> + default:
> + ret = -EINVAL;
> + }
> #if IS_ENABLED(CONFIG_IPV6)
> } else if (level == SOL_IPV6) {
> if (optlen != sizeof(int) || sk->sk_family != AF_INET6)
> @@ -3561,6 +3582,20 @@ BPF_CALL_5(bpf_getsockopt, struct bpf_sock_ops_kern *, bpf_sock,
> } else {
> goto err_clear;
> }
> + } else if (level == SOL_IP) {
> + struct inet_sock *inet = inet_sk(sk);
> +
> + if (optlen != sizeof(int) || sk->sk_family != AF_INET)
> + goto err_clear;
> +
> + /* Only some options are supported */
> + switch (optname) {
> + case IP_TOS:
> + *((int *)optval) = (int)inet->tos;
This part is fine though, same as in do_ip_getsockopt().
> + break;
> + default:
> + goto err_clear;
> + }
> #if IS_ENABLED(CONFIG_IPV6)
> } else if (level == SOL_IPV6) {
> struct ipv6_pinfo *np = inet6_sk(sk);
>
^ permalink raw reply
* Re: [PATCH v7 bpf-next 06/10] tracepoint: compute num_args at build time
From: Mathieu Desnoyers @ 2018-03-28 13:49 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Daniel Borkmann, Linus Torvalds, Peter Zijlstra,
rostedt, netdev, kernel-team, linux-api
In-Reply-To: <20180328021105.4061744-7-ast@fb.com>
----- On Mar 27, 2018, at 10:11 PM, Alexei Starovoitov ast@fb.com wrote:
> From: Alexei Starovoitov <ast@kernel.org>
>
> compute number of arguments passed into tracepoint
> at compile time and store it as part of 'struct tracepoint'.
> The number is necessary to check safety of bpf program access that
> is coming in subsequent patch.
Hi Alexei,
Given that only eBPF needs this parameter count, we can move
it to the struct bpf_raw_event_map newly introduced by Steven,
right ? This would reduce bloat of struct tracepoint. For instance,
we don't need to keep this count around when eBPF is configured
out.
Thanks,
Mathieu
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> include/linux/tracepoint-defs.h | 1 +
> include/linux/tracepoint.h | 12 ++++++------
> include/trace/define_trace.h | 14 +++++++-------
> 3 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
> index 64ed7064f1fa..39a283c61c51 100644
> --- a/include/linux/tracepoint-defs.h
> +++ b/include/linux/tracepoint-defs.h
> @@ -33,6 +33,7 @@ struct tracepoint {
> int (*regfunc)(void);
> void (*unregfunc)(void);
> struct tracepoint_func __rcu *funcs;
> + u32 num_args;
> };
>
> #endif
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index c94f466d57ef..c92f4adbc0d7 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -230,18 +230,18 @@ extern void syscall_unregfunc(void);
> * structures, so we create an array of pointers that will be used for iteration
> * on the tracepoints.
> */
> -#define DEFINE_TRACE_FN(name, reg, unreg) \
> +#define DEFINE_TRACE_FN(name, reg, unreg, num_args) \
> static const char __tpstrtab_##name[] \
> __attribute__((section("__tracepoints_strings"))) = #name; \
> struct tracepoint __tracepoint_##name \
> __attribute__((section("__tracepoints"))) = \
> - { __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL };\
> + { __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL, num_args };\
> static struct tracepoint * const __tracepoint_ptr_##name __used \
> __attribute__((section("__tracepoints_ptrs"))) = \
> &__tracepoint_##name;
>
> -#define DEFINE_TRACE(name) \
> - DEFINE_TRACE_FN(name, NULL, NULL);
> +#define DEFINE_TRACE(name, num_args) \
> + DEFINE_TRACE_FN(name, NULL, NULL, num_args);
>
> #define EXPORT_TRACEPOINT_SYMBOL_GPL(name) \
> EXPORT_SYMBOL_GPL(__tracepoint_##name)
> @@ -275,8 +275,8 @@ extern void syscall_unregfunc(void);
> return false; \
> }
>
> -#define DEFINE_TRACE_FN(name, reg, unreg)
> -#define DEFINE_TRACE(name)
> +#define DEFINE_TRACE_FN(name, reg, unreg, num_args)
> +#define DEFINE_TRACE(name, num_args)
> #define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
> #define EXPORT_TRACEPOINT_SYMBOL(name)
>
> diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
> index d9e3d4aa3f6e..96b22ace9ae7 100644
> --- a/include/trace/define_trace.h
> +++ b/include/trace/define_trace.h
> @@ -25,7 +25,7 @@
>
> #undef TRACE_EVENT
> #define TRACE_EVENT(name, proto, args, tstruct, assign, print) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef TRACE_EVENT_CONDITION
> #define TRACE_EVENT_CONDITION(name, proto, args, cond, tstruct, assign, print) \
> @@ -39,24 +39,24 @@
> #undef TRACE_EVENT_FN
> #define TRACE_EVENT_FN(name, proto, args, tstruct, \
> assign, print, reg, unreg) \
> - DEFINE_TRACE_FN(name, reg, unreg)
> + DEFINE_TRACE_FN(name, reg, unreg, COUNT_ARGS(args))
>
> #undef TRACE_EVENT_FN_COND
> #define TRACE_EVENT_FN_COND(name, proto, args, cond, tstruct, \
> assign, print, reg, unreg) \
> - DEFINE_TRACE_FN(name, reg, unreg)
> + DEFINE_TRACE_FN(name, reg, unreg, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT
> #define DEFINE_EVENT(template, name, proto, args) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT_FN
> #define DEFINE_EVENT_FN(template, name, proto, args, reg, unreg) \
> - DEFINE_TRACE_FN(name, reg, unreg)
> + DEFINE_TRACE_FN(name, reg, unreg, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT_PRINT
> #define DEFINE_EVENT_PRINT(template, name, proto, args, print) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT_CONDITION
> #define DEFINE_EVENT_CONDITION(template, name, proto, args, cond) \
> @@ -64,7 +64,7 @@
>
> #undef DECLARE_TRACE
> #define DECLARE_TRACE(name, proto, args) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef TRACE_INCLUDE
> #undef __TRACE_INCLUDE
> --
> 2.9.5
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* [PATCH] tipc: avoid possible string overflow
From: Arnd Bergmann @ 2018-03-28 14:02 UTC (permalink / raw)
To: Jon Maloy, Ying Xue, David S. Miller
Cc: Arnd Bergmann, Parthasarathy Bhuvaragan, netdev, tipc-discussion,
linux-kernel
gcc points out that the combined length of the fixed-length inputs to
l->name is larger than the destination buffer size:
net/tipc/link.c: In function 'tipc_link_create':
net/tipc/link.c:465:26: error: '%s' directive writing up to 32 bytes into a region of size between 26 and 58 [-Werror=format-overflow=]
sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
^~ ~~~~~~~~
net/tipc/link.c:465:2: note: 'sprintf' output 11 or more bytes (assuming 75) into a destination of size 60
sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
Using snprintf() ensures that the destination is still a nul-terminated
string in all cases. It's still theoretically possible that the string
gets trunctated though, so this patch should be carefully reviewed to
ensure that either truncation is impossible in practice, or that we're
ok with the truncation.
Fixes: 25b0b9c4e835 ("tipc: handle collisions of 32-bit node address hash values")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/tipc/link.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 1289b4ba404f..c195ba036035 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -462,7 +462,8 @@ bool tipc_link_create(struct net *net, char *if_name, int bearer_id,
sprintf(peer_str, "%x", peer);
}
/* Peer i/f name will be completed by reset/activate message */
- sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
+ snprintf(l->name, sizeof(l->name), "%s:%s-%s:unknown",
+ self_str, if_name, peer_str);
strcpy(l->if_name, if_name);
l->addr = peer;
--
2.9.0
^ permalink raw reply related
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Steven Rostedt @ 2018-03-28 14:06 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Mathieu Desnoyers, David S. Miller, Daniel Borkmann,
Linus Torvalds, Peter Zijlstra, netdev, kernel-team, linux-api,
Kees Cook
In-Reply-To: <34b9892d-c091-7b83-d8e8-79c0ff813140@fb.com>
On Tue, 27 Mar 2018 17:51:55 -0700
Alexei Starovoitov <ast@fb.com> wrote:
> Turned out it was in init.data section and got poisoned.
> this fixes it:
> @@ -258,6 +258,7 @@
> LIKELY_PROFILE() \
> BRANCH_PROFILE() \
> TRACE_PRINTKS() \
> + BPF_RAW_TP() \
> TRACEPOINT_STR()
>
> /*
> @@ -585,7 +586,6 @@
> *(.init.rodata) \
> FTRACE_EVENTS() \
> TRACE_SYSCALLS() \
> - BPF_RAW_TP() \
> KPROBE_BLACKLIST() \
> ERROR_INJECT_WHITELIST() \
> MEM_DISCARD(init.rodata) \
>
> and it works :)
> I will clean few other nits I found while debugging and respin.
Getting it properly working was an exercise left to the reader ;-)
Sorry about that, I did a bit of copy and paste to get it working, and
copied from code that did things a bit differently, so I massaged it by
hand, and doing it quickly as I had other things to work on.
-- Steve
^ permalink raw reply
* Re: [PATCH v3 net 1/5] tcp: feed correct number of pkts acked to cc modules also in recovery
From: Yuchung Cheng @ 2018-03-28 14:14 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: netdev, Neal Cardwell, Eric Dumazet, Sergei Shtylyov
In-Reply-To: <alpine.DEB.2.20.1803272047260.8966@whs-18.cs.helsinki.fi>
On Wed, Mar 28, 2018 at 5:45 AM, Ilpo Järvinen
<ilpo.jarvinen@helsinki.fi> wrote:
> On Tue, 27 Mar 2018, Yuchung Cheng wrote:
>
>> On Tue, Mar 27, 2018 at 7:23 AM, Ilpo Järvinen
>> <ilpo.jarvinen@helsinki.fi> wrote:
>> > On Mon, 26 Mar 2018, Yuchung Cheng wrote:
>> >
>> >> On Tue, Mar 13, 2018 at 3:25 AM, Ilpo Järvinen
>> >> <ilpo.jarvinen@helsinki.fi> wrote:
>> >> >
>> >> > A miscalculation for the number of acknowledged packets occurs during
>> >> > RTO recovery whenever SACK is not enabled and a cumulative ACK covers
>> >> > any non-retransmitted skbs. The reason is that pkts_acked value
>> >> > calculated in tcp_clean_rtx_queue is not correct for slow start after
>> >> > RTO as it may include segments that were not lost and therefore did
>> >> > not need retransmissions in the slow start following the RTO. Then
>> >> > tcp_slow_start will add the excess into cwnd bloating it and
>> >> > triggering a burst.
>> >> >
>> >> > Instead, we want to pass only the number of retransmitted segments
>> >> > that were covered by the cumulative ACK (and potentially newly sent
>> >> > data segments too if the cumulative ACK covers that far).
>> >> >
>> >> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
>> >> > ---
>> >>
>> >> My understanding is there are two problems
>> >>
>> >> 1) your fix: the reordering logic in tcp-remove_reno_sacks requires
>> >> precise cumulatively acked count, not newly acked count?
>> >
>> > While I'm not entirely sure if you intented to say that my fix is broken
>> > or not, I thought this very difference alot while making the fix and I
>> > believe that this fix is needed because of the discontinuity at RTO
>> > (sacked_out is cleared as we set L-bits + lost_out). This is an artifact
>> > in the imitation of sacked_out for non-SACK but at RTO we can't keep that
>> > in sync because we set L-bits (and have no S-bits to guide us). Thus, we
>> > cannot anymore "use" those skbs with only L-bit for the reno_sacks logic.
>> >
>> > In tcp_remove_reno_sacks acked - sacked_out is being used to calculate
>> > tp->delivered, using plain cumulative acked causes congestion control
>> > breakage later as call to tcp_cong_control will directly use the
>> > difference in tp->delivered.
>> >
>> > This boils down the exact definition of tp->delivered (the one given in
>> > the header is not detailed enough). I guess you might have better idea
>> > what it exactly is since one of you has added it? There are subtle things
>> > in the defination that can make it entirely unsuitable for cc decisions.
>> > Should those segments that we (possibly) already counted into
>> > tp->delivered during (potentially preceeding) CA_Recovery be added to it
>> > for _second time_ or not? This fix avoids such double counting (the
>> Where is the double counting, assuming normal DUPACK behavior?
>>
>> In the non-sack case:
>>
>> 1. upon receiving a DUPACK, we assume one packet has been delivered by
>> incrementing tp->delivered in tcp_add_reno_sack()
>
> 1b. RTO here. We clear tp->sacked_out at RTO (i.e., the discontinuity
> I've tried to point out quite many times already)...
>
>> 2. upon receiving a partial ACK or an ACK that acks recovery point
>> (high_seq), tp->delivered is incremented by (cumulatively acked -
>> #dupacks) in tcp_remove_reno_sacks()
>
> ...and this won't happen correctly anymore after RTO (since non-SACK
> won't keep #dupacks due to the discontinuity). Thus we end up adding
> cumulatively acked - 0 to tp->delivered on those ACKs.
>
>> therefore tp->delivered is tracking the # of packets delivered
>> (sacked, acked, DUPACK'd) with the most information it could have
>> inferred.
>
> Since you didn't answer any of my questions about tp->delivered directly,
> let me rephrase them to this example (non-SACK, of course):
>
> 4 segments outstanding. RTO recovery underway (lost_out=4, sacked_out=0).
> Cwnd = 2 so the sender rexmits 2 out of 4. We get cumulative ACK for
> three segments. How much should tp->delivered be incremented? 2 or 3?
>
> ...I think 2 is the right answer.
>
>> From congestion control's perspective, it cares about the delivery
>> information (e.g. how much), not the sequences (what or how).
>
> I guess you must have missed my point. I'm talking about "how much"
> whole the time. It's just about when can we account for it (and when not).
>
>> I am pointing out that
>>
>> 1) your fix may fix one corner CC packet accounting issue in the
>> non-SACK and CA_Loss
>> 2) but it does not fix the other (major) CC packet accounting issue in
>> the SACK case
>
> You say major but it's major if and only if one is using one of the
> affected cc modules which were not that many and IMHO not very mainstream
> anyway (check my list from the previous email, not that I'd mind if they'd
> get fixed too). The rest of the cc modules do not seem to use the incorrect
> value even if some of them have the pkts_acked callback.
>
> Other CC packet accounting (regardless of SACK) is based on tp->delivered
> and tp->delivered is NOT similarly miscounted with SACK because the logic
> depend on !TCPCB_SACKED_ACKED (that fails only if we have very high ACK
> loss).
>
>> 3) it breaks the dynamic dupthresh / reordering in the non-SACK case
>> as tcp_check_reno_reordering requires strictly cum. ack.
>
> I think that the current non-SACK reordering detection logic is not that
> sound after RTO (but I'm yet to prove this). ...There seems to be some
> failure modes which is why I even thought of disabling the whole thing
> for non-SACK RTO recoveries and as it now seems you do care more about
> non-SACK than you initial claimed, I might even have motivation to fix
> more those corners rather than the worst bugs only ;-). ...But I'll make
> the tp->delivered fix only in this patch to avoid any change this part of
> the code.
>
>> Therefore I prefer
>> a) using tp->delivered to fix (1)(2)
>> b) perhaps improving tp->delivered SACK emulation code in the non-SACK case
>
> Somehow I get an impression that you might assume/say here that
[resending in plaintext]
That's wrong impression. Perhaps it's worth re-iterating what I agree
and disagree
1. [agree] there's accounting issue in non-SACK as you discovered
which causes CC misbehavior
2. [major disagree] adjusting pkts_acked for ca_ops->pkts_acked in non-sack
=> that field is not used by common C.C. (you said so too)
3. [disagree] adjusting pkts_acked may not affect reordering
accounting in non-sack
For cubic or reno, the main source is the "acked_sacked" passed into
tcp_cong_avoid(). that variable is derived from tp->delivered.
Therefore we need to fix that to address the problem in (1)
I have yet to read your code. Will read later today.
> tp->delivered is all correct for non-SACK? ...It isn't without a fix.
> Therefore a) won't help any for non-SACK. Tp->delivered for non-SACK is
> currently (mis!)calculated in tcp_remove_reno_sacks because the incorrect
> pkts_acked is fed to it. ...Thus, b) is an intermediate step in the
> miscalculation chain I'm fixing with this change. B) resolves also (1)
> without additional changes to logic!
>
> I've included below the updated change that only fixes tp->delivered
> calculation (not tested besides compiling). ...But I think it's worse than
> my previous version because tcp_remove_reno_sacks then assumes
> non-sensical L|S skbs (but there seem to be other, worse limitations in
> sacked_out imitation after RTO because we've all skbs marked with L-bit so
> it's not that big deal for me as most of the that imitation code is no-op
> anyway after RTO).
>
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 9a1b3c1..e11748d 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -1868,7 +1868,6 @@ static void tcp_remove_reno_sacks(struct sock *sk, int acked)
>
> if (acked > 0) {
> /* One ACK acked hole. The rest eat duplicate ACKs. */
> - tp->delivered += max_t(int, acked - tp->sacked_out, 1);
> if (acked - 1 >= tp->sacked_out)
> tp->sacked_out = 0;
> else
> @@ -1878,6 +1877,12 @@ static void tcp_remove_reno_sacks(struct sock *sk, int acked)
> tcp_verify_left_out(tp);
> }
>
> +static void tcp_update_reno_delivered(struct tcp_sock *tp, int acked)
> +{
> + if (acked > 0)
> + tp->delivered += max_t(int, acked - tp->sacked_out, 1);
> +}
> +
> static inline void tcp_reset_reno_sack(struct tcp_sock *tp)
> {
> tp->sacked_out = 0;
> @@ -3027,6 +3032,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> long seq_rtt_us = -1L;
> long ca_rtt_us = -1L;
> u32 pkts_acked = 0;
> + u32 rexmit_acked = 0;
> + u32 newdata_acked = 0;
> u32 last_in_flight = 0;
> bool rtt_update;
> int flag = 0;
> @@ -3056,8 +3063,10 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> }
>
> if (unlikely(sacked & TCPCB_RETRANS)) {
> - if (sacked & TCPCB_SACKED_RETRANS)
> + if (sacked & TCPCB_SACKED_RETRANS) {
> tp->retrans_out -= acked_pcount;
> + rexmit_acked += acked_pcount;
> + }
> flag |= FLAG_RETRANS_DATA_ACKED;
> } else if (!(sacked & TCPCB_SACKED_ACKED)) {
> last_ackt = skb->skb_mstamp;
> @@ -3070,6 +3079,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> reord = start_seq;
> if (!after(scb->end_seq, tp->high_seq))
> flag |= FLAG_ORIG_SACK_ACKED;
> + else
> + newdata_acked += acked_pcount;
> }
>
> if (sacked & TCPCB_SACKED_ACKED) {
> @@ -3151,6 +3162,9 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> }
>
> if (tcp_is_reno(tp)) {
> + tcp_update_reno_delivered(tp, icsk->icsk_ca_state != TCP_CA_Loss ?
> + pkts_acked :
> + rexmit_acked + newdata_acked);
> tcp_remove_reno_sacks(sk, pkts_acked);
> } else {
> int delta;
^ permalink raw reply
* [PATCH] [net-next] sctp: fix unused lable warning
From: Arnd Bergmann @ 2018-03-28 14:14 UTC (permalink / raw)
To: Vlad Yasevich, Neil Horman, David S. Miller
Cc: Arnd Bergmann, Al Viro, linux-sctp, netdev, linux-kernel
The proc file cleanup left a label possibly unused:
net/sctp/protocol.c: In function 'sctp_defaults_init':
net/sctp/protocol.c:1304:1: error: label 'err_init_proc' defined but not used [-Werror=unused-label]
This adds an #ifdef around it to match the respective 'goto'.
Fixes: d47d08c8ca05 ("sctp: use proc_remove_subtree()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/sctp/protocol.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index c1c64b78c39d..d685f8456762 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -1301,8 +1301,10 @@ static int __net_init sctp_defaults_init(struct net *net)
return 0;
+#ifdef CONFIG_PROC_FS
err_init_proc:
cleanup_sctp_mibs(net);
+#endif
err_init_mibs:
sctp_sysctl_net_unregister(net);
err_sysctl_register:
--
2.9.0
^ permalink raw reply related
* Re: Problems: network rx out-of-order issue
From: Eric Dumazet @ 2018-03-28 14:19 UTC (permalink / raw)
To: Anny Hu, netdev; +Cc: zhen.jiang
In-Reply-To: <1522229201.8705.44.camel@mbjswglx818>
On 03/28/2018 02:26 AM, Anny Hu wrote:
> Dears,
>
> Recently, we find the following patch will impact multi-core network throughput performance on kernel-4.9,
> for it will cause rx packet out-of-order.
> commit id: 4cd13c21b207e80ddb1144c576500098f2d5f882
>
> [kernel version]:
> kernel-4.9
>
> [repeat steps]
> 1. two our phones with load based on kernel-4.9, enable wifi and set up P2P connection;
> 2. two reference phones with load based on kernel-4.4, enable wifi and set up P2P connection;
> 3. do iperf UDP downlink test with the following commands;
> Client: adb shell iperf -c 192.168.49.xx -u -i 1 -t 60 -b 15M
> Server: adb shell iperf -s -u
> 4. out-of-order rate of our phones are obviously higher than reference phone:
> DUT Lost rate out of order
> Our phone 0.011% 164
> Reference phone 0.0091% 1
>
> [Code flow]
> Wi-Fi driver trigger rx_thread to receive rx packets and call netif_rx_ni() to push packets to tcp stack in thread context.
>
> [netlog]:
> No Time Source ip Dest ip Protocol Len Ip id Info
> 3811 2010-01-01 08:11:35.075522 192.168.43.1 192.168.43.90 UDP 144 0x25ee (9710) 60834 -> 5201 Len=100
> 3812 2010-01-01 08:11:35.075805 192.168.43.1 192.168.43.90 UDP 144 0x25f5 (9717) 60834 -> 5201 Len=100
> 3813 2010-01-01 08:11:35.075549 192.168.43.1 192.168.43.90 UDP 144 0x25ef (9711) 60834 -> 5201 Len=100
> 3814 2010-01-01 08:11:35.075944 192.168.43.1 192.168.43.90 UDP 144 0x25f6 (9718) 60834 -> 5201 Len=100
> 3815 2010-01-01 08:11:35.075567 192.168.43.1 192.168.43.90 UDP 144 0x25f0 (9712) 60834 -> 5201 Len=100
>
> out-of-order issue happens on:
> 1) ip_id=0x25f5 is received before ip_id=0x25ef, which are belong to the same downlink connection;
> 2) ip_id=0x25f6 is received before ip_id=0x25f0, which are belong to the same downlink connection.
>
>
> [kernel log]:
> We add debug log in rx kernel flow, such as netif_rx_ni() -> enqueue_to_backlog -> do_softirq() -> net_rx_action() -> process_backlog()
>
> kernel time
> | cpu id which process running on
> | | process name function name
> | | | |
> [ 598.837358] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25ee, pending=0x8a, backlog_state=1
>
> // wifi rx_thread is running on cpu7 to receive rx packets of ip_id 0x25ee ~ 0x25f4, which are only enqueued.
> [ 598.837366] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25ee, input_qlen=1, proc_qlen=0, pending=0x8a, backlog_state=1
>
> // because ksoftirqd is in running queue, do_softirq() exit and net_rx_action isn't executed actually.
>
> [ 598.837373] (7)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0, pending=0x8a, ksoftirqd_running=1
>
> [ 598.837383] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25ef,input_qlen=2, proc_qlen=0, pending=0x8a, backlog_state=1
> [ 598.837391] (7)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0 pending=0x8a, ksoftirqd_running=1
> [ 598.837400] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25f0,input_qlen=3, proc_qlen=0, pending=0x8a, backlog_state=1
> [ 598.837407] (7)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0, pending=0x8a, ksoftirqd_running=1
> [ 598.837417] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25f1,input_qlen=4, proc_qlen=0, pending=0x8a, backlog_state=1
> [ 598.837424] (7)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0, pending=0x8a, ksoftirqd_running=1
> [ 598.837434] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25f2,input_qlen=5, proc_qlen=0, pending=0x8a, backlog_state=1
> [ 598.837441] (7)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0, pending=0x8a, ksoftirqd_running=1
> [ 598.837450] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25f3,input_qlen=6, proc_qlen=0, pending=0x8a, backlog_state=1
> [ 598.837458] (7)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0, pending=0x8a, ksoftirqd_running=1
>
> // after ip_id(0x25f4), there is no rx packet in this wifi interrupt, so rx_thread exit.
> [ 598.837470] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25f4,input_qlen=7, proc_qlen=0, pending=0x8a, backlog_state=1
> [ 598.837479] (7)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0, pending=0x8a, ksoftirqd_running=1
>
> // ksoftirqd is scheduled to run and call do_softirq(), so net_rx_action() is executed.
> [ 598.837511] (7)[45:ksoftirqd/7]: [RX]net_rx_action: budget=300, time=4295042005
> // net_rx_action() call process_backlog() to handle rx packet of ip_id=0x25ee
>
> [ 598.837646] (7)[45:ksoftirqd/7]: [RX]process_backlog: ip_id=0x25ee, delay=126308, work=1, input_qlen=0, proc_qlen=6
>
> // wifi interrupt happens and trigger rx_thread to receive the next rx packet, which is running on cpu1 this time.
> [ 598.837660] (1)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25f5, input_qlen=1, proc_qlen=0, pending=0x8, backlog_state=1
> // because ksoftirqd is not running, net_rx softirq is executed immediately.
> [ 598.837666] (1)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0, pending=0x8, ksoftirqd_running =0
>
> // so ksoftirqd on cpu7 and rx_thread on cpu1 are running at the same time.
> [ 598.837679] (1)[3282:rx_thread]: [RX]net_rx_action: budget=300, time=4295042005
>
> // ip_id=0x25ef is handled on cpu7 by net_rx softirq in ksoftirqd() thread context.
> [ 598.837762] (7)[45:ksoftirqd/7]: [RX]process_backlog: ip_id=0x25ef, delay=70385, work=2, input_qlen=0, proc_qlen=5
>
> // ip_id=0x25f5 is handled on cpu1 by net_rx softirq in rx_thread context, out-of-order issue happens.
>
> [ 598.837768] (1)[3282:rx_thread]: [RX]process_backlog: ip_id=0x25f5, delay=82000, work=1, quota=64, input_qlen=0, proc_qlen=0
>
> // ip_id=0x25f0 is handled on cpu7 by net_rx softirq in ksoftirqd() thread context.
> [ 598.837878] (7)[45:ksoftirqd/7]: [RX]process_backlog:ip_id=0x25f0, delay=47539, work=3, input_qlen=0, proc_qlen=4
>
>
> Could you please help to check if this out-of-order issue is caused by kernel-4.9 patch above?
>
> Thanks very much!
>
A driver calling netif_rx_ni() can have out of order packets,
regardless of commit 4cd13c21b207e80ddb1144c57650
This patch has changed scheduling, and might have pointed out ancient bugs/issues.
'thread context' means cpu can change under us (preempt kernels)
Which driver are you using exactly ?
^ permalink raw reply
* Re: [bpf-next PATCH v2 3/4] bpf: sockmap, BPF_F_INGRESS flag for BPF_SK_SKB_STREAM_VERDICT:
From: Daniel Borkmann @ 2018-03-28 14:21 UTC (permalink / raw)
To: John Fastabend, ast; +Cc: netdev, davem
In-Reply-To: <20180327172322.11354.54016.stgit@john-Precision-Tower-5810>
On 03/27/2018 07:23 PM, John Fastabend wrote:
> Add support for the BPF_F_INGRESS flag in skb redirect helper. To
> do this convert skb into a scatterlist and push into ingress queue.
> This is the same logic that is used in the sk_msg redirect helper
> so it should feel familiar.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> include/linux/filter.h | 1 +
> kernel/bpf/sockmap.c | 94 +++++++++++++++++++++++++++++++++++++++---------
> net/core/filter.c | 2 +
> 3 files changed, 78 insertions(+), 19 deletions(-)
[...]
> if (!sg->length && md->sg_start == md->sg_end) {
> list_del(&md->list);
> + if (md->skb)
> + consume_skb(md->skb);
> kfree(md);
> }
> }
> @@ -1045,27 +1048,72 @@ static int smap_verdict_func(struct smap_psock *psock, struct sk_buff *skb)
> __SK_DROP;
> }
>
> +static int smap_do_ingress(struct smap_psock *psock, struct sk_buff *skb)
> +{
> + struct sock *sk = psock->sock;
> + int copied = 0, num_sg;
> + struct sk_msg_buff *r;
> +
> + r = kzalloc(sizeof(struct sk_msg_buff), __GFP_NOWARN | GFP_ATOMIC);
> + if (unlikely(!r))
> + return -EAGAIN;
> +
> + if (!sk_rmem_schedule(sk, skb, skb->len)) {
> + kfree(r);
> + return -EAGAIN;
> + }
> + sk_mem_charge(sk, skb->len);
Usually mem accounting is based on truesize. This is not done here since
you need the exact length of the skb for the sg list later on, right?
> + sg_init_table(r->sg_data, MAX_SKB_FRAGS);
> + num_sg = skb_to_sgvec(skb, r->sg_data, 0, skb->len);
> + if (unlikely(num_sg < 0)) {
> + kfree(r);
Don't we need to undo the mem charge here in case of error?
> + return num_sg;
> + }
> + copied = skb->len;
> + r->sg_start = 0;
> + r->sg_end = num_sg == MAX_SKB_FRAGS ? 0 : num_sg;
> + r->skb = skb;
> + list_add_tail(&r->list, &psock->ingress);
> + sk->sk_data_ready(sk);
> + return copied;
> +}
^ permalink raw reply
* Re: [RFC PATCH v2] net: phy: Added device tree binding for dev-addr and dev-addr code check-up
From: Rob Herring @ 2018-03-28 14:31 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vicenţiu Galanopulo, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, mark.rutland@arm.com,
davem@davemloft.net, marcel@holtmann.org,
devicetree@vger.kernel.org, Madalin-cristian Bucur,
Alexandru Marginean
In-Reply-To: <20180327142408.GL5862@lunn.ch>
On Tue, Mar 27, 2018 at 9:24 AM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > This is a 2nd MDIO address, right? Can't you just append this to reg property?
>
> Hi Rob
>
> It is a sub address.
>
> There are two different MDIO addressing schemes. Clause 22 allowed for
> 32 different addresses on an MDIO bus. Clause 45 extended that. You
> have the existing 32 addresses for a package. However, within a
> package, you can have 32 devices.
Sounds similar to functions in PCI land (which are part of reg address).
> You are supposed to be able to look are registers inside the package,
> and it will tell you which devices in the packages are in use. You can
> then look at those devices and figure out what they are using ID
> registers.
>
> However some vendors get this wrong, they don't fill in the devices in
> package information. So the generic probe code never finds them. We
> need to pass it a hint, go looking at this specific device in the
> package.
If this is a rare case and something future devices should get right,
then I'm more inclined to use 'dev-addr' rather than extending reg.
> You can mix Clause 22 and Clause 45 on the same bus. Does DT allow two
> different reg formats to be used at once? Can we have some reg
> properties with a single value, and some with two values? I thought
> #address-cells = <1> means there should be a single address in reg.
#address-cells is how many cells (aka u32) it takes to store an
address, not how many addresses you have. The number of addresses is
(sizeof(reg) / 4) / (#address-cells + #size-cells). So yes, you can
have different number of addresses for each device, but you can't have
different sizes of addresses (e.g. 32-bit and 64-bit) in one bus. But
I think in this case it would logically be 1 address with 2 cells
because it is the port and device together that make up the address.
If you did that, you'd have to define how to express a clause 22
device with 2 cells. You could either set a high bit in the first cell
to indicate clause 45 address or use an illegal device address in the
2nd cell.
However, the MDIO core would need to handle 2 address cells. If more
than 1 address cell is an error currently, that causes a compatibility
problem with new dtb and older kernels (but could be addressed with
stable updates).
Rob
^ permalink raw reply
* Re: 4.14.29 - tcp_push() - null skb's cb dereference
From: David Miller @ 2018-03-28 14:38 UTC (permalink / raw)
To: eric.dumazet; +Cc: krzysztof.blaszkowski, netdev, gregkh
In-Reply-To: <14343877-a2a0-215e-e052-2dfb7e4c4b09@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 28 Mar 2018 06:38:21 -0700
> https://patchwork.ozlabs.org/patch/886324/
I have this in my current -stable submission set, and I'm working
actively on this right now.
Thanks.
^ permalink raw reply
* RE: [PATCH] tipc: avoid possible string overflow
From: Jon Maloy @ 2018-03-28 14:41 UTC (permalink / raw)
To: Arnd Bergmann, Ying Xue, David S. Miller
Cc: Parthasarathy Bhuvaragan, netdev@vger.kernel.org,
tipc-discussion@lists.sourceforge.net,
linux-kernel@vger.kernel.org
In-Reply-To: <20180328140302.2594031-1-arnd@arndb.de>
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Wednesday, March 28, 2018 10:02
> To: Jon Maloy <jon.maloy@ericsson.com>; Ying Xue
> <ying.xue@windriver.com>; David S. Miller <davem@davemloft.net>
> Cc: Arnd Bergmann <arnd@arndb.de>; Parthasarathy Bhuvaragan
> <parthasarathy.bhuvaragan@ericsson.com>; netdev@vger.kernel.org; tipc-
> discussion@lists.sourceforge.net; linux-kernel@vger.kernel.org
> Subject: [PATCH] tipc: avoid possible string overflow
>
> gcc points out that the combined length of the fixed-length inputs to
> l->name is larger than the destination buffer size:
>
> net/tipc/link.c: In function 'tipc_link_create':
> net/tipc/link.c:465:26: error: '%s' directive writing up to 32 bytes into a region
> of size between 26 and 58 [-Werror=format-overflow=]
> sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
> ^~ ~~~~~~~~
> net/tipc/link.c:465:2: note: 'sprintf' output 11 or more bytes (assuming 75)
> into a destination of size 60
> sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
>
> Using snprintf() ensures that the destination is still a nul-terminated string in
> all cases. It's still theoretically possible that the string gets trunctated though,
> so this patch should be carefully reviewed to ensure that either truncation is
> impossible in practice, or that we're ok with the truncation.
Theoretically, maximum bearer name is MAX_BEARER_NAME - 3 = 29 (because if_name is only the part after the ":" in a bearer name, and is zero-terminated.
The lines just above in the code reveals that the maximum length of self_str and peer_str is 16.
This taken together means that the theoretically max length of a link name becomes:
16 + 1 + 29 + 1 + 16 + 1 + 29 = 93. Since we also need room for a terminating zero, we need to extend the tipc_link::name array to 96 bytes.
I'll fix that.
Thank you to for reporting this.
///jon
>
> Fixes: 25b0b9c4e835 ("tipc: handle collisions of 32-bit node address hash
> values")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> net/tipc/link.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/tipc/link.c b/net/tipc/link.c index 1289b4ba404f..c195ba036035
> 100644
> --- a/net/tipc/link.c
> +++ b/net/tipc/link.c
> @@ -462,7 +462,8 @@ bool tipc_link_create(struct net *net, char *if_name,
> int bearer_id,
> sprintf(peer_str, "%x", peer);
> }
> /* Peer i/f name will be completed by reset/activate message */
> - sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
> + snprintf(l->name, sizeof(l->name), "%s:%s-%s:unknown",
> + self_str, if_name, peer_str);
>
> strcpy(l->if_name, if_name);
> l->addr = peer;
> --
> 2.9.0
^ permalink raw reply
* [PATCH net-next] net: Call add/kill vid ndo on vlan filter feature toggling
From: Gal Pressman @ 2018-03-28 14:46 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Tariq Toukan, Gal Pressman
NETIF_F_HW_VLAN_[CS]TAG_FILTER features require more than just a bit
flip in dev->features in order to keep the driver in a consistent state.
These features notify the driver of each added/removed vlan, but toggling
of vlan-filter does not notify the driver accordingly for each of the
existing vlans.
This patch implements a similar solution to NETIF_F_RX_UDP_TUNNEL_PORT
behavior (which notifies the driver about UDP ports in the same manner
that vids are reported).
Each toggling of the features propagates to the 8021q module, which
iterates over the vlans and call add/kill ndo accordingly.
Signed-off-by: Gal Pressman <galp@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
---
include/linux/if_vlan.h | 24 +++++++++++
include/linux/netdevice.h | 4 ++
net/8021q/vlan.c | 21 ++++++++++
net/8021q/vlan.h | 3 ++
net/8021q/vlan_core.c | 101 ++++++++++++++++++++++++++++++++++------------
net/core/dev.c | 20 +++++++++
6 files changed, 148 insertions(+), 25 deletions(-)
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index c4a1cff..24d1976 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -83,6 +83,30 @@ static inline bool is_vlan_dev(const struct net_device *dev)
#define skb_vlan_tag_get_id(__skb) ((__skb)->vlan_tci & VLAN_VID_MASK)
#define skb_vlan_tag_get_prio(__skb) ((__skb)->vlan_tci & VLAN_PRIO_MASK)
+static inline int vlan_get_rx_ctag_filter_info(struct net_device *dev)
+{
+ ASSERT_RTNL();
+ return notifier_to_errno(call_netdevice_notifiers(NETDEV_CVLAN_FILTER_PUSH_INFO, dev));
+}
+
+static inline void vlan_drop_rx_ctag_filter_info(struct net_device *dev)
+{
+ ASSERT_RTNL();
+ call_netdevice_notifiers(NETDEV_CVLAN_FILTER_DROP_INFO, dev);
+}
+
+static inline int vlan_get_rx_stag_filter_info(struct net_device *dev)
+{
+ ASSERT_RTNL();
+ return notifier_to_errno(call_netdevice_notifiers(NETDEV_SVLAN_FILTER_PUSH_INFO, dev));
+}
+
+static inline void vlan_drop_rx_stag_filter_info(struct net_device *dev)
+{
+ ASSERT_RTNL();
+ call_netdevice_notifiers(NETDEV_SVLAN_FILTER_DROP_INFO, dev);
+}
+
/**
* struct vlan_pcpu_stats - VLAN percpu rx/tx stats
* @rx_packets: number of received packets
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 2a2d9cf..da44dab 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2349,6 +2349,10 @@ enum netdev_cmd {
NETDEV_UDP_TUNNEL_PUSH_INFO,
NETDEV_UDP_TUNNEL_DROP_INFO,
NETDEV_CHANGE_TX_QUEUE_LEN,
+ NETDEV_CVLAN_FILTER_PUSH_INFO,
+ NETDEV_CVLAN_FILTER_DROP_INFO,
+ NETDEV_SVLAN_FILTER_PUSH_INFO,
+ NETDEV_SVLAN_FILTER_DROP_INFO,
};
const char *netdev_cmd_to_name(enum netdev_cmd cmd);
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index bd0ed39..4b33728 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -360,6 +360,7 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
struct vlan_dev_priv *vlan;
bool last = false;
LIST_HEAD(list);
+ int err;
if (is_vlan_dev(dev)) {
int err = __vlan_device_event(dev, event);
@@ -489,6 +490,26 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
vlan_group_for_each_dev(grp, i, vlandev)
call_netdevice_notifiers(event, vlandev);
break;
+
+ case NETDEV_CVLAN_FILTER_PUSH_INFO:
+ err = vlan_filter_push_vids(vlan_info, htons(ETH_P_8021Q));
+ if (err)
+ return notifier_from_errno(err);
+ break;
+
+ case NETDEV_CVLAN_FILTER_DROP_INFO:
+ vlan_filter_drop_vids(vlan_info, htons(ETH_P_8021Q));
+ break;
+
+ case NETDEV_SVLAN_FILTER_PUSH_INFO:
+ err = vlan_filter_push_vids(vlan_info, htons(ETH_P_8021AD));
+ if (err)
+ return notifier_from_errno(err);
+ break;
+
+ case NETDEV_SVLAN_FILTER_DROP_INFO:
+ vlan_filter_drop_vids(vlan_info, htons(ETH_P_8021AD));
+ break;
}
out:
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index a8ba510..e23aac3 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -97,6 +97,9 @@ static inline struct net_device *vlan_find_dev(struct net_device *real_dev,
if (((dev) = __vlan_group_get_device((grp), (i) / VLAN_N_VID, \
(i) % VLAN_N_VID)))
+int vlan_filter_push_vids(struct vlan_info *vlan_info, __be16 proto);
+void vlan_filter_drop_vids(struct vlan_info *vlan_info, __be16 proto);
+
/* found in vlan_dev.c */
void vlan_dev_set_ingress_priority(const struct net_device *dev,
u32 skb_prio, u16 vlan_prio);
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 45c9bf5..c8d7abd 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -165,13 +165,12 @@ struct vlan_vid_info {
int refcount;
};
-static bool vlan_hw_filter_capable(const struct net_device *dev,
- const struct vlan_vid_info *vid_info)
+bool vlan_hw_filter_capable(const struct net_device *dev, __be16 proto)
{
- if (vid_info->proto == htons(ETH_P_8021Q) &&
+ if (proto == htons(ETH_P_8021Q) &&
dev->features & NETIF_F_HW_VLAN_CTAG_FILTER)
return true;
- if (vid_info->proto == htons(ETH_P_8021AD) &&
+ if (proto == htons(ETH_P_8021AD) &&
dev->features & NETIF_F_HW_VLAN_STAG_FILTER)
return true;
return false;
@@ -202,11 +201,73 @@ static struct vlan_vid_info *vlan_vid_info_alloc(__be16 proto, u16 vid)
return vid_info;
}
+static int vlan_add_rx_filter_info(struct net_device *dev, __be16 proto, u16 vid)
+{
+ if (!vlan_hw_filter_capable(dev, proto))
+ return 0;
+
+ if (netif_device_present(dev))
+ return dev->netdev_ops->ndo_vlan_rx_add_vid(dev, proto, vid);
+ else
+ return -ENODEV;
+}
+
+static int vlan_kill_rx_filter_info(struct net_device *dev, __be16 proto, u16 vid)
+{
+ if (!vlan_hw_filter_capable(dev, proto))
+ return 0;
+
+ if (netif_device_present(dev))
+ return dev->netdev_ops->ndo_vlan_rx_kill_vid(dev, proto, vid);
+ else
+ return -ENODEV;
+}
+
+int vlan_filter_push_vids(struct vlan_info *vlan_info, __be16 proto)
+{
+ struct net_device *real_dev = vlan_info->real_dev;
+ struct vlan_vid_info *vlan_vid_info;
+ int err;
+
+ list_for_each_entry(vlan_vid_info, &vlan_info->vid_list, list) {
+ if (vlan_vid_info->proto == proto) {
+ err = vlan_add_rx_filter_info(real_dev, proto,
+ vlan_vid_info->vid);
+ if (err)
+ goto unwind;
+ }
+ }
+
+ return 0;
+
+unwind:
+ list_for_each_entry_continue_reverse(vlan_vid_info,
+ &vlan_info->vid_list, list) {
+ if (vlan_vid_info->proto == proto)
+ vlan_kill_rx_filter_info(real_dev, proto,
+ vlan_vid_info->vid);
+ }
+
+ return err;
+}
+EXPORT_SYMBOL(vlan_filter_push_vids);
+
+void vlan_filter_drop_vids(struct vlan_info *vlan_info, __be16 proto)
+{
+ struct vlan_vid_info *vlan_vid_info;
+
+ list_for_each_entry(vlan_vid_info, &vlan_info->vid_list, list)
+ if (vlan_vid_info->proto == proto)
+ vlan_kill_rx_filter_info(vlan_info->real_dev,
+ vlan_vid_info->proto,
+ vlan_vid_info->vid);
+}
+EXPORT_SYMBOL(vlan_filter_drop_vids);
+
static int __vlan_vid_add(struct vlan_info *vlan_info, __be16 proto, u16 vid,
struct vlan_vid_info **pvid_info)
{
struct net_device *dev = vlan_info->real_dev;
- const struct net_device_ops *ops = dev->netdev_ops;
struct vlan_vid_info *vid_info;
int err;
@@ -214,16 +275,12 @@ static int __vlan_vid_add(struct vlan_info *vlan_info, __be16 proto, u16 vid,
if (!vid_info)
return -ENOMEM;
- if (vlan_hw_filter_capable(dev, vid_info)) {
- if (netif_device_present(dev))
- err = ops->ndo_vlan_rx_add_vid(dev, proto, vid);
- else
- err = -ENODEV;
- if (err) {
- kfree(vid_info);
- return err;
- }
+ err = vlan_add_rx_filter_info(dev, proto, vid);
+ if (err) {
+ kfree(vid_info);
+ return err;
}
+
list_add(&vid_info->list, &vlan_info->vid_list);
vlan_info->nr_vids++;
*pvid_info = vid_info;
@@ -270,21 +327,15 @@ static void __vlan_vid_del(struct vlan_info *vlan_info,
struct vlan_vid_info *vid_info)
{
struct net_device *dev = vlan_info->real_dev;
- const struct net_device_ops *ops = dev->netdev_ops;
__be16 proto = vid_info->proto;
u16 vid = vid_info->vid;
int err;
- if (vlan_hw_filter_capable(dev, vid_info)) {
- if (netif_device_present(dev))
- err = ops->ndo_vlan_rx_kill_vid(dev, proto, vid);
- else
- err = -ENODEV;
- if (err) {
- pr_warn("failed to kill vid %04x/%d for device %s\n",
- proto, vid, dev->name);
- }
- }
+ err = vlan_kill_rx_filter_info(dev, proto, vid);
+ if (err)
+ pr_warn("failed to kill vid %04x/%d for device %s\n",
+ proto, vid, dev->name);
+
list_del(&vid_info->list);
kfree(vid_info);
vlan_info->nr_vids--;
diff --git a/net/core/dev.c b/net/core/dev.c
index 97a96df..5793775 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1584,6 +1584,8 @@ const char *netdev_cmd_to_name(enum netdev_cmd cmd)
N(RESEND_IGMP) N(PRECHANGEMTU) N(CHANGEINFODATA) N(BONDING_INFO)
N(PRECHANGEUPPER) N(CHANGELOWERSTATE) N(UDP_TUNNEL_PUSH_INFO)
N(UDP_TUNNEL_DROP_INFO) N(CHANGE_TX_QUEUE_LEN)
+ N(CVLAN_FILTER_PUSH_INFO) N(CVLAN_FILTER_DROP_INFO)
+ N(SVLAN_FILTER_PUSH_INFO) N(SVLAN_FILTER_DROP_INFO)
};
#undef N
return "UNKNOWN_NETDEV_EVENT";
@@ -7660,6 +7662,24 @@ int __netdev_update_features(struct net_device *dev)
}
}
+ if (diff & NETIF_F_HW_VLAN_CTAG_FILTER) {
+ if (features & NETIF_F_HW_VLAN_CTAG_FILTER) {
+ dev->features = features;
+ err |= vlan_get_rx_ctag_filter_info(dev);
+ } else {
+ vlan_drop_rx_ctag_filter_info(dev);
+ }
+ }
+
+ if (diff & NETIF_F_HW_VLAN_STAG_FILTER) {
+ if (features & NETIF_F_HW_VLAN_STAG_FILTER) {
+ dev->features = features;
+ err |= vlan_get_rx_stag_filter_info(dev);
+ } else {
+ vlan_drop_rx_stag_filter_info(dev);
+ }
+ }
+
dev->features = features;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net v2] udp6: set dst cache for a connected sk before udp_v6_send_skb
From: Alexey Kodanev @ 2018-03-28 14:57 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: netdev, Eric Dumazet, David Miller
In-Reply-To: <20180327190002.bbrfik2pucgwepyu@kafai-mbp.dhcp.thefacebook.com>
On 27.03.2018 22:00, Martin KaFai Lau wrote:
> On Tue, Mar 27, 2018 at 04:27:30PM +0300, Alexey Kodanev wrote:
>> On 26.03.2018 20:02, Martin KaFai Lau wrote:
>>> On Mon, Mar 26, 2018 at 05:48:47PM +0300, Alexey Kodanev wrote:
>>>> After commit 33c162a980fe ("ipv6: datagram: Update dst cache of a
>>>> connected datagram sk during pmtu update"), when the error occurs on
>>>> sending datagram in udpv6_sendmsg() due to ICMPV6_PKT_TOOBIG type,
>>>> error handler can trigger the following path and call ip6_dst_store():
>>>>
>>>> udpv6_err()
>>>> ip6_sk_update_pmtu()
>>>> ip6_datagram_dst_update()
>>>> ip6_dst_lookup_flow(), can create a RTF_CACHE clone
>>> Instead of ip6_dst_lookup_flow(),
>>> you meant the RTF_CACHE route created in ip6_update_pmtu()
>>>
>>
>> Right, or even earlier... I was using vti tunnel and it invokes
>> skb_dst_update_pmtu() on this error, then sends ICMPv6_PKT_TOOBIG.
>>
>>>> ...
>>>> ip6_dst_store()
>>>>
>>>> It can happen before a connected UDP socket invokes ip6_dst_store()
>>>> in the end of udpv6_sendmsg(), on destination release, as a result,
>>>> the last one changes dst to the old one, preventing getting updated
>>>> dst cache on the next udpv6_sendmsg() call.
>>>>
>>>> This patch moves ip6_dst_store() in udpv6_sendmsg(), so that it is
>>>> invoked after ip6_sk_dst_lookup_flow() and before udp_v6_send_skb().
>>> After this patch, the above udpv6_err() path could not happen after
>>> ip6_sk_dst_lookup_flow() and before the ip6_dst_store() in udpv6_sendmsg()?
>>>
>>
>> May be we could minimize this if save it in ip6_sk_dst_lookup_flow()
>> for a connected UDP sockets only if we're not getting it from a cache
>> for some reason?
> I am just not clear how moving it earlier can completely stop the
> described issue. Beside, the next ICMPV6_PKT_TOOBIG will be
> received again and eventually rectify sk->sk_dst_cache?
With a request/response scenario in my environment, it always happens
after udp_v6_send_skb() and before the 'release_dst:' in udpv6_sendmsg().
So the socket there never gets updated cache when resending datagram
again after ICMPV6_PKT_TOOBIG... dst cache is always reset to the old
one in the end of udpv6_sendmsg().
>
>>
>> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
>> index a8a9195..0204f52 100644
>> --- a/net/ipv6/ip6_output.c
>> +++ b/net/ipv6/ip6_output.c
>> @@ -1115,13 +1115,30 @@ struct dst_entry *ip6_dst_lookup_flow(const struct sock *sk, struct flowi6 *fl6,
>> * error code.
>> */
>> struct dst_entry *ip6_sk_dst_lookup_flow(struct sock *sk, struct flowi6 *fl6,
>> - const struct in6_addr *final_dst)
>> + const struct in6_addr *final_dst,
>> + bool connected)
>> {
>> struct dst_entry *dst = sk_dst_check(sk, inet6_sk(sk)->dst_cookie);
>>
>> dst = ip6_sk_dst_check(sk, dst, fl6);
>> - if (!dst)
>> - dst = ip6_dst_lookup_flow(sk, fl6, final_dst);
>> + if (dst)
>> + return dst;
>> +
>> + dst = ip6_dst_lookup_flow(sk, fl6, final_dst);
>> +
>> + if (connected && !IS_ERR(dst))
>> + ip6_dst_store(sk, dst_clone(dst), ...);
> Right, I think doing dst_store() only if ip6_sk_dst_check()/
> sk_dst_check() returns NULL is a more sensible thing to
> do here.
OK, will prepare the patch.
Thanks,
Alexey
^ permalink raw reply
* [net-next 0/4] tipc: slim down name table
From: Jon Maloy @ 2018-03-28 14:58 UTC (permalink / raw)
To: davem, netdev; +Cc: tipc-discussion, mohan.krishna.ghanta.krishnamurthy
We clean up and improve the name binding table:
- Replace the memory consuming 'sub_sequence/service range' array with
an RB tree.
- Introduce support for overlapping service sequences/ranges
Jon Maloy (4):
tipc: replace name table service range array with rb tree
tipc: refactor name table translate function
tipc: permit overlapping service ranges in name table
tipc: tipc: rename address types in user api
include/uapi/linux/tipc.h | 32 +-
net/tipc/core.h | 1 +
net/tipc/link.c | 2 +-
net/tipc/name_distr.c | 90 +---
net/tipc/name_distr.h | 1 -
net/tipc/name_table.c | 1075 ++++++++++++++++++++-------------------------
net/tipc/name_table.h | 10 +-
net/tipc/net.c | 2 +-
net/tipc/node.c | 4 +-
net/tipc/socket.c | 4 +-
net/tipc/subscr.h | 4 +-
11 files changed, 538 insertions(+), 687 deletions(-)
--
2.1.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply
* [net-next 1/4] tipc: replace name table service range array with rb tree
From: Jon Maloy @ 2018-03-28 14:58 UTC (permalink / raw)
To: davem, netdev; +Cc: tipc-discussion, mohan.krishna.ghanta.krishnamurthy
In-Reply-To: <1522249103-32714-1-git-send-email-jon.maloy@ericsson.com>
The current design of the binding table has an unnecessary memory
consuming and complex data structure. It aggregates the service range
items into an array, which is expanded by a factor two every time it
becomes too small to hold a new item. Furthermore, the arrays never
shrink when the number of ranges diminishes.
We now replace this array with an RB tree that is holding the range
items as tree nodes, each range directly holding a list of bindings.
This, along with a few name changes, improves both readability and
volume of the code, as well as reducing memory consumption and hopefully
improving cache hit rate.
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/core.h | 1 +
net/tipc/link.c | 2 +-
net/tipc/name_table.c | 1032 ++++++++++++++++++++++---------------------------
net/tipc/name_table.h | 2 +-
net/tipc/node.c | 4 +-
net/tipc/subscr.h | 4 +-
6 files changed, 477 insertions(+), 568 deletions(-)
diff --git a/net/tipc/core.h b/net/tipc/core.h
index d0f64ca..8020a6c 100644
--- a/net/tipc/core.h
+++ b/net/tipc/core.h
@@ -58,6 +58,7 @@
#include <linux/etherdevice.h>
#include <net/netns/generic.h>
#include <linux/rhashtable.h>
+#include <net/genetlink.h>
struct tipc_node;
struct tipc_bearer;
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 1289b4b..8f2a949 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1810,7 +1810,7 @@ int tipc_link_bc_nack_rcv(struct tipc_link *l, struct sk_buff *skb,
void tipc_link_set_queue_limits(struct tipc_link *l, u32 win)
{
- int max_bulk = TIPC_MAX_PUBLICATIONS / (l->mtu / ITEM_SIZE);
+ int max_bulk = TIPC_MAX_PUBL / (l->mtu / ITEM_SIZE);
l->window = win;
l->backlog[TIPC_LOW_IMPORTANCE].limit = max_t(u16, 50, win);
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 4359605..c309402 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -44,52 +44,40 @@
#include "addr.h"
#include "node.h"
#include "group.h"
-#include <net/genetlink.h>
-
-#define TIPC_NAMETBL_SIZE 1024 /* must be a power of 2 */
/**
- * struct name_info - name sequence publication info
- * @node_list: list of publications on own node of this <type,lower,upper>
- * @all_publ: list of all publications of this <type,lower,upper>
+ * struct service_range - container for all bindings of a service range
+ * @lower: service range lower bound
+ * @upper: service range upper bound
+ * @tree_node: member of service range RB tree
+ * @local_publ: list of identical publications made from this node
+ * Used by closest_first lookup and multicast lookup algorithm
+ * @all_publ: all publications identical to this one, whatever node and scope
+ * Used by round-robin lookup algorithm
*/
-struct name_info {
- struct list_head local_publ;
- struct list_head all_publ;
-};
-
-/**
- * struct sub_seq - container for all published instances of a name sequence
- * @lower: name sequence lower bound
- * @upper: name sequence upper bound
- * @info: pointer to name sequence publication info
- */
-struct sub_seq {
+struct service_range {
u32 lower;
u32 upper;
- struct name_info *info;
+ struct rb_node tree_node;
+ struct list_head local_publ;
+ struct list_head all_publ;
};
/**
- * struct name_seq - container for all published instances of a name type
- * @type: 32 bit 'type' value for name sequence
- * @sseq: pointer to dynamically-sized array of sub-sequences of this 'type';
- * sub-sequences are sorted in ascending order
- * @alloc: number of sub-sequences currently in array
- * @first_free: array index of first unused sub-sequence entry
- * @ns_list: links to adjacent name sequences in hash chain
- * @subscriptions: list of subscriptions for this 'type'
- * @lock: spinlock controlling access to publication lists of all sub-sequences
+ * struct tipc_service - container for all published instances of a service type
+ * @type: 32 bit 'type' value for service
+ * @ranges: rb tree containing all service ranges for this service
+ * @service_list: links to adjacent name ranges in hash chain
+ * @subscriptions: list of subscriptions for this service type
+ * @lock: spinlock controlling access to pertaining service ranges/publications
* @rcu: RCU callback head used for deferred freeing
*/
-struct name_seq {
+struct tipc_service {
u32 type;
- struct sub_seq *sseqs;
- u32 alloc;
- u32 first_free;
- struct hlist_node ns_list;
+ struct rb_root ranges;
+ struct hlist_node service_list;
struct list_head subscriptions;
- spinlock_t lock;
+ spinlock_t lock; /* Covers service range list */
struct rcu_head rcu;
};
@@ -99,17 +87,16 @@ static int hash(int x)
}
/**
- * publ_create - create a publication structure
+ * tipc_publ_create - create a publication structure
*/
-static struct publication *publ_create(u32 type, u32 lower, u32 upper,
- u32 scope, u32 node, u32 port,
- u32 key)
+static struct publication *tipc_publ_create(u32 type, u32 lower, u32 upper,
+ u32 scope, u32 node, u32 port,
+ u32 key)
{
struct publication *publ = kzalloc(sizeof(*publ), GFP_ATOMIC);
- if (publ == NULL) {
- pr_warn("Publication creation failure, no memory\n");
+
+ if (!publ)
return NULL;
- }
publ->type = type;
publ->lower = lower;
@@ -119,372 +106,298 @@ static struct publication *publ_create(u32 type, u32 lower, u32 upper,
publ->port = port;
publ->key = key;
INIT_LIST_HEAD(&publ->binding_sock);
+ INIT_LIST_HEAD(&publ->binding_node);
+ INIT_LIST_HEAD(&publ->local_publ);
+ INIT_LIST_HEAD(&publ->all_publ);
return publ;
}
/**
- * tipc_subseq_alloc - allocate a specified number of sub-sequence structures
- */
-static struct sub_seq *tipc_subseq_alloc(u32 cnt)
-{
- return kcalloc(cnt, sizeof(struct sub_seq), GFP_ATOMIC);
-}
-
-/**
- * tipc_nameseq_create - create a name sequence structure for the specified 'type'
+ * tipc_service_create - create a service structure for the specified 'type'
*
- * Allocates a single sub-sequence structure and sets it to all 0's.
+ * Allocates a single range structure and sets it to all 0's.
*/
-static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head *seq_head)
+static struct tipc_service *tipc_service_create(u32 type, struct hlist_head *hd)
{
- struct name_seq *nseq = kzalloc(sizeof(*nseq), GFP_ATOMIC);
- struct sub_seq *sseq = tipc_subseq_alloc(1);
+ struct tipc_service *service = kzalloc(sizeof(*service), GFP_ATOMIC);
- if (!nseq || !sseq) {
- pr_warn("Name sequence creation failed, no memory\n");
- kfree(nseq);
- kfree(sseq);
+ if (!service) {
+ pr_warn("Service creation failed, no memory\n");
return NULL;
}
- spin_lock_init(&nseq->lock);
- nseq->type = type;
- nseq->sseqs = sseq;
- nseq->alloc = 1;
- INIT_HLIST_NODE(&nseq->ns_list);
- INIT_LIST_HEAD(&nseq->subscriptions);
- hlist_add_head_rcu(&nseq->ns_list, seq_head);
- return nseq;
+ spin_lock_init(&service->lock);
+ service->type = type;
+ service->ranges = RB_ROOT;
+ INIT_HLIST_NODE(&service->service_list);
+ INIT_LIST_HEAD(&service->subscriptions);
+ hlist_add_head_rcu(&service->service_list, hd);
+ return service;
}
/**
- * nameseq_find_subseq - find sub-sequence (if any) matching a name instance
+ * tipc_service_find_range - find service range matching a service instance
*
- * Very time-critical, so binary searches through sub-sequence array.
+ * Very time-critical, so binary search through range rb tree
*/
-static struct sub_seq *nameseq_find_subseq(struct name_seq *nseq,
- u32 instance)
+static struct service_range *tipc_service_find_range(struct tipc_service *sc,
+ u32 instance)
{
- struct sub_seq *sseqs = nseq->sseqs;
- int low = 0;
- int high = nseq->first_free - 1;
- int mid;
-
- while (low <= high) {
- mid = (low + high) / 2;
- if (instance < sseqs[mid].lower)
- high = mid - 1;
- else if (instance > sseqs[mid].upper)
- low = mid + 1;
+ struct rb_node *n = sc->ranges.rb_node;
+ struct service_range *sr;
+
+ while (n) {
+ sr = container_of(n, struct service_range, tree_node);
+ if (sr->lower > instance)
+ n = n->rb_left;
+ else if (sr->upper < instance)
+ n = n->rb_right;
else
- return &sseqs[mid];
+ return sr;
}
return NULL;
}
-/**
- * nameseq_locate_subseq - determine position of name instance in sub-sequence
- *
- * Returns index in sub-sequence array of the entry that contains the specified
- * instance value; if no entry contains that value, returns the position
- * where a new entry for it would be inserted in the array.
- *
- * Note: Similar to binary search code for locating a sub-sequence.
- */
-static u32 nameseq_locate_subseq(struct name_seq *nseq, u32 instance)
+static struct service_range *tipc_service_create_range(struct tipc_service *sc,
+ u32 lower, u32 upper)
{
- struct sub_seq *sseqs = nseq->sseqs;
- int low = 0;
- int high = nseq->first_free - 1;
- int mid;
-
- while (low <= high) {
- mid = (low + high) / 2;
- if (instance < sseqs[mid].lower)
- high = mid - 1;
- else if (instance > sseqs[mid].upper)
- low = mid + 1;
+ struct rb_node **n, *parent = NULL;
+ struct service_range *sr, *tmp;
+
+ n = &sc->ranges.rb_node;
+ while (*n) {
+ tmp = container_of(*n, struct service_range, tree_node);
+ parent = *n;
+ tmp = container_of(parent, struct service_range, tree_node);
+ if (lower < tmp->lower)
+ n = &(*n)->rb_left;
+ else if (upper > tmp->upper)
+ n = &(*n)->rb_right;
else
- return mid;
+ return NULL;
}
- return low;
+ sr = kzalloc(sizeof(*sr), GFP_ATOMIC);
+ if (!sr)
+ return NULL;
+ sr->lower = lower;
+ sr->upper = upper;
+ INIT_LIST_HEAD(&sr->local_publ);
+ INIT_LIST_HEAD(&sr->all_publ);
+ rb_link_node(&sr->tree_node, parent, n);
+ rb_insert_color(&sr->tree_node, &sc->ranges);
+ return sr;
}
-/**
- * tipc_nameseq_insert_publ
- */
-static struct publication *tipc_nameseq_insert_publ(struct net *net,
- struct name_seq *nseq,
+static struct publication *tipc_service_insert_publ(struct net *net,
+ struct tipc_service *sc,
u32 type, u32 lower,
u32 upper, u32 scope,
- u32 node, u32 port, u32 key)
+ u32 node, u32 port,
+ u32 key)
{
- struct tipc_subscription *s;
- struct tipc_subscription *st;
- struct publication *publ;
- struct sub_seq *sseq;
- struct name_info *info;
- int created_subseq = 0;
-
- sseq = nameseq_find_subseq(nseq, lower);
- if (sseq) {
-
- /* Lower end overlaps existing entry => need an exact match */
- if ((sseq->lower != lower) || (sseq->upper != upper)) {
- return NULL;
- }
-
- info = sseq->info;
-
- /* Check if an identical publication already exists */
- list_for_each_entry(publ, &info->all_publ, all_publ) {
- if (publ->port == port && publ->key == key &&
- (!publ->node || publ->node == node))
- return NULL;
- }
- } else {
- u32 inspos;
- struct sub_seq *freesseq;
-
- /* Find where lower end should be inserted */
- inspos = nameseq_locate_subseq(nseq, lower);
-
- /* Fail if upper end overlaps into an existing entry */
- if ((inspos < nseq->first_free) &&
- (upper >= nseq->sseqs[inspos].lower)) {
- return NULL;
- }
-
- /* Ensure there is space for new sub-sequence */
- if (nseq->first_free == nseq->alloc) {
- struct sub_seq *sseqs = tipc_subseq_alloc(nseq->alloc * 2);
-
- if (!sseqs) {
- pr_warn("Cannot publish {%u,%u,%u}, no memory\n",
- type, lower, upper);
- return NULL;
- }
- memcpy(sseqs, nseq->sseqs,
- nseq->alloc * sizeof(struct sub_seq));
- kfree(nseq->sseqs);
- nseq->sseqs = sseqs;
- nseq->alloc *= 2;
- }
-
- info = kzalloc(sizeof(*info), GFP_ATOMIC);
- if (!info) {
- pr_warn("Cannot publish {%u,%u,%u}, no memory\n",
- type, lower, upper);
- return NULL;
- }
-
- INIT_LIST_HEAD(&info->local_publ);
- INIT_LIST_HEAD(&info->all_publ);
-
- /* Insert new sub-sequence */
- sseq = &nseq->sseqs[inspos];
- freesseq = &nseq->sseqs[nseq->first_free];
- memmove(sseq + 1, sseq, (freesseq - sseq) * sizeof(*sseq));
- memset(sseq, 0, sizeof(*sseq));
- nseq->first_free++;
- sseq->lower = lower;
- sseq->upper = upper;
- sseq->info = info;
- created_subseq = 1;
+ struct tipc_subscription *sub, *tmp;
+ struct service_range *sr;
+ struct publication *p;
+ bool first = false;
+
+ sr = tipc_service_find_range(sc, lower);
+ if (!sr) {
+ sr = tipc_service_create_range(sc, lower, upper);
+ if (!sr)
+ goto err;
+ first = true;
}
- /* Insert a publication */
- publ = publ_create(type, lower, upper, scope, node, port, key);
- if (!publ)
+ /* Lower end overlaps existing entry, but we need an exact match */
+ if (sr->lower != lower || sr->upper != upper)
return NULL;
- list_add(&publ->all_publ, &info->all_publ);
+ /* Return if the publication already exists */
+ list_for_each_entry(p, &sr->all_publ, all_publ) {
+ if (p->key == key && (!p->node || p->node == node))
+ return NULL;
+ }
+ /* Create and insert publication */
+ p = tipc_publ_create(type, lower, upper, scope, node, port, key);
+ if (!p)
+ goto err;
if (in_own_node(net, node))
- list_add(&publ->local_publ, &info->local_publ);
+ list_add(&p->local_publ, &sr->local_publ);
+ list_add(&p->all_publ, &sr->all_publ);
/* Any subscriptions waiting for notification? */
- list_for_each_entry_safe(s, st, &nseq->subscriptions, nameseq_list) {
- tipc_sub_report_overlap(s, publ->lower, publ->upper,
- TIPC_PUBLISHED, publ->port,
- publ->node, publ->scope,
- created_subseq);
+ list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
+ tipc_sub_report_overlap(sub, p->lower, p->upper, TIPC_PUBLISHED,
+ p->port, p->node, p->scope, first);
}
- return publ;
+ return p;
+err:
+ pr_warn("Failed to bind to %u,%u,%u, no memory\n", type, lower, upper);
+ return NULL;
}
/**
- * tipc_nameseq_remove_publ
+ * tipc_service_remove_publ - remove a publication from a service
*
* NOTE: There may be cases where TIPC is asked to remove a publication
* that is not in the name table. For example, if another node issues a
- * publication for a name sequence that overlaps an existing name sequence
+ * publication for a name range that overlaps an existing name range
* the publication will not be recorded, which means the publication won't
- * be found when the name sequence is later withdrawn by that node.
+ * be found when the name range is later withdrawn by that node.
* A failed withdraw request simply returns a failure indication and lets the
* caller issue any error or warning messages associated with such a problem.
*/
-static struct publication *tipc_nameseq_remove_publ(struct net *net,
- struct name_seq *nseq,
+static struct publication *tipc_service_remove_publ(struct net *net,
+ struct tipc_service *sc,
u32 inst, u32 node,
u32 port, u32 key)
{
- struct publication *publ;
- struct sub_seq *sseq = nameseq_find_subseq(nseq, inst);
- struct name_info *info;
- struct sub_seq *free;
- struct tipc_subscription *s, *st;
- int removed_subseq = 0;
-
- if (!sseq)
- return NULL;
+ struct tipc_subscription *sub, *tmp;
+ struct service_range *sr;
+ struct publication *p;
+ bool found = false;
+ bool last = false;
- info = sseq->info;
+ sr = tipc_service_find_range(sc, inst);
+ if (!sr)
+ return NULL;
- /* Locate publication, if it exists */
- list_for_each_entry(publ, &info->all_publ, all_publ) {
- if (publ->key == key && publ->port == port &&
- (!publ->node || publ->node == node))
- goto found;
+ /* Find publication, if it exists */
+ list_for_each_entry(p, &sr->all_publ, all_publ) {
+ if (p->key != key || (node && node != p->node))
+ continue;
+ found = true;
+ break;
}
- return NULL;
+ if (!found)
+ return NULL;
-found:
- list_del(&publ->all_publ);
- if (in_own_node(net, node))
- list_del(&publ->local_publ);
-
- /* Contract subseq list if no more publications for that subseq */
- if (list_empty(&info->all_publ)) {
- kfree(info);
- free = &nseq->sseqs[nseq->first_free--];
- memmove(sseq, sseq + 1, (free - (sseq + 1)) * sizeof(*sseq));
- removed_subseq = 1;
+ list_del(&p->all_publ);
+ list_del(&p->local_publ);
+
+ /* Remove service range item if this was its last publication */
+ if (list_empty(&sr->all_publ)) {
+ last = true;
+ rb_erase(&sr->tree_node, &sc->ranges);
+ kfree(sr);
}
/* Notify any waiting subscriptions */
- list_for_each_entry_safe(s, st, &nseq->subscriptions, nameseq_list) {
- tipc_sub_report_overlap(s, publ->lower, publ->upper,
- TIPC_WITHDRAWN, publ->port,
- publ->node, publ->scope,
- removed_subseq);
+ list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
+ tipc_sub_report_overlap(sub, p->lower, p->upper, TIPC_WITHDRAWN,
+ p->port, p->node, p->scope, last);
}
-
- return publ;
+ return p;
}
/**
- * tipc_nameseq_subscribe - attach a subscription, and optionally
- * issue the prescribed number of events if there is any sub-
- * sequence overlapping with the requested sequence
+ * tipc_service_subscribe - attach a subscription, and optionally
+ * issue the prescribed number of events if there is any service
+ * range overlapping with the requested range
*/
-static void tipc_nameseq_subscribe(struct name_seq *nseq,
+static void tipc_service_subscribe(struct tipc_service *service,
struct tipc_subscription *sub)
{
- struct sub_seq *sseq = nseq->sseqs;
+ struct tipc_subscr *sb = &sub->evt.s;
+ struct service_range *sr;
struct tipc_name_seq ns;
- struct tipc_subscr *s = &sub->evt.s;
- bool no_status;
+ struct publication *p;
+ struct rb_node *n;
+ bool first;
- ns.type = tipc_sub_read(s, seq.type);
- ns.lower = tipc_sub_read(s, seq.lower);
- ns.upper = tipc_sub_read(s, seq.upper);
- no_status = tipc_sub_read(s, filter) & TIPC_SUB_NO_STATUS;
+ ns.type = tipc_sub_read(sb, seq.type);
+ ns.lower = tipc_sub_read(sb, seq.lower);
+ ns.upper = tipc_sub_read(sb, seq.upper);
tipc_sub_get(sub);
- list_add(&sub->nameseq_list, &nseq->subscriptions);
+ list_add(&sub->service_list, &service->subscriptions);
- if (no_status || !sseq)
+ if (tipc_sub_read(sb, filter) & TIPC_SUB_NO_STATUS)
return;
- while (sseq != &nseq->sseqs[nseq->first_free]) {
- if (tipc_sub_check_overlap(&ns, sseq->lower, sseq->upper)) {
- struct publication *crs;
- struct name_info *info = sseq->info;
- int must_report = 1;
-
- list_for_each_entry(crs, &info->all_publ, all_publ) {
- tipc_sub_report_overlap(sub, sseq->lower,
- sseq->upper,
- TIPC_PUBLISHED,
- crs->port,
- crs->node,
- crs->scope,
- must_report);
- must_report = 0;
- }
+ for (n = rb_first(&service->ranges); n; n = rb_next(n)) {
+ sr = container_of(n, struct service_range, tree_node);
+ if (sr->lower > ns.upper)
+ break;
+ if (!tipc_sub_check_overlap(&ns, sr->lower, sr->upper))
+ continue;
+ first = true;
+
+ list_for_each_entry(p, &sr->all_publ, all_publ) {
+ tipc_sub_report_overlap(sub, sr->lower, sr->upper,
+ TIPC_PUBLISHED, p->port,
+ p->node, p->scope, first);
+ first = false;
}
- sseq++;
}
}
-static struct name_seq *nametbl_find_seq(struct net *net, u32 type)
+static struct tipc_service *tipc_service_find(struct net *net, u32 type)
{
- struct tipc_net *tn = net_generic(net, tipc_net_id);
- struct hlist_head *seq_head;
- struct name_seq *ns;
-
- seq_head = &tn->nametbl->seq_hlist[hash(type)];
- hlist_for_each_entry_rcu(ns, seq_head, ns_list) {
- if (ns->type == type)
- return ns;
+ struct name_table *nt = tipc_name_table(net);
+ struct hlist_head *service_head;
+ struct tipc_service *service;
+
+ service_head = &nt->services[hash(type)];
+ hlist_for_each_entry_rcu(service, service_head, service_list) {
+ if (service->type == type)
+ return service;
}
-
return NULL;
};
struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
- u32 lower, u32 upper, u32 scope,
- u32 node, u32 port, u32 key)
+ u32 lower, u32 upper,
+ u32 scope, u32 node,
+ u32 port, u32 key)
{
- struct tipc_net *tn = net_generic(net, tipc_net_id);
- struct publication *publ;
- struct name_seq *seq = nametbl_find_seq(net, type);
- int index = hash(type);
+ struct name_table *nt = tipc_name_table(net);
+ struct tipc_service *sc;
+ struct publication *p;
if (scope > TIPC_NODE_SCOPE || lower > upper) {
- pr_debug("Failed to publish illegal {%u,%u,%u} with scope %u\n",
+ pr_debug("Failed to bind illegal {%u,%u,%u} with scope %u\n",
type, lower, upper, scope);
return NULL;
}
-
- if (!seq)
- seq = tipc_nameseq_create(type, &tn->nametbl->seq_hlist[index]);
- if (!seq)
+ sc = tipc_service_find(net, type);
+ if (!sc)
+ sc = tipc_service_create(type, &nt->services[hash(type)]);
+ if (!sc)
return NULL;
- spin_lock_bh(&seq->lock);
- publ = tipc_nameseq_insert_publ(net, seq, type, lower, upper,
- scope, node, port, key);
- spin_unlock_bh(&seq->lock);
- return publ;
+ spin_lock_bh(&sc->lock);
+ p = tipc_service_insert_publ(net, sc, type, lower, upper,
+ scope, node, port, key);
+ spin_unlock_bh(&sc->lock);
+ return p;
}
struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
- u32 lower, u32 node, u32 port,
- u32 key)
+ u32 lower, u32 node, u32 port,
+ u32 key)
{
- struct publication *publ;
- struct name_seq *seq = nametbl_find_seq(net, type);
+ struct tipc_service *sc = tipc_service_find(net, type);
+ struct publication *p = NULL;
- if (!seq)
+ if (!sc)
return NULL;
- spin_lock_bh(&seq->lock);
- publ = tipc_nameseq_remove_publ(net, seq, lower, node, port, key);
- if (!seq->first_free && list_empty(&seq->subscriptions)) {
- hlist_del_init_rcu(&seq->ns_list);
- kfree(seq->sseqs);
- spin_unlock_bh(&seq->lock);
- kfree_rcu(seq, rcu);
- return publ;
+ spin_lock_bh(&sc->lock);
+ p = tipc_service_remove_publ(net, sc, lower, node, port, key);
+
+ /* Delete service item if this no more publications and subscriptions */
+ if (RB_EMPTY_ROOT(&sc->ranges) && list_empty(&sc->subscriptions)) {
+ hlist_del_init_rcu(&sc->service_list);
+ kfree_rcu(sc, rcu);
}
- spin_unlock_bh(&seq->lock);
- return publ;
+ spin_unlock_bh(&sc->lock);
+ return p;
}
/**
- * tipc_nametbl_translate - perform name translation
+ * tipc_nametbl_translate - perform service instance to socket translation
*
* On entry, 'destnode' is the search domain used during translation.
*
@@ -492,7 +405,7 @@ struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
* - if name translation is deferred to another node/cluster/zone,
* leaves 'destnode' unchanged (will be non-zero) and returns 0
* - if name translation is attempted and succeeds, sets 'destnode'
- * to publishing node and returns port reference (will be non-zero)
+ * to publication node and returns port reference (will be non-zero)
* - if name translation is attempted and fails, sets 'destnode' to 0
* and returns 0
*/
@@ -502,10 +415,9 @@ u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance,
struct tipc_net *tn = tipc_net(net);
bool legacy = tn->legacy_addr_format;
u32 self = tipc_own_addr(net);
- struct sub_seq *sseq;
- struct name_info *info;
- struct publication *publ;
- struct name_seq *seq;
+ struct service_range *sr;
+ struct tipc_service *sc;
+ struct publication *p;
u32 port = 0;
u32 node = 0;
@@ -513,49 +425,49 @@ u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance,
return 0;
rcu_read_lock();
- seq = nametbl_find_seq(net, type);
- if (unlikely(!seq))
+ sc = tipc_service_find(net, type);
+ if (unlikely(!sc))
goto not_found;
- spin_lock_bh(&seq->lock);
- sseq = nameseq_find_subseq(seq, instance);
- if (unlikely(!sseq))
+
+ spin_lock_bh(&sc->lock);
+ sr = tipc_service_find_range(sc, instance);
+ if (unlikely(!sr))
goto no_match;
- info = sseq->info;
/* Closest-First Algorithm */
if (legacy && !*destnode) {
- if (!list_empty(&info->local_publ)) {
- publ = list_first_entry(&info->local_publ,
- struct publication,
- local_publ);
- list_move_tail(&publ->local_publ,
- &info->local_publ);
+ if (!list_empty(&sr->local_publ)) {
+ p = list_first_entry(&sr->local_publ,
+ struct publication,
+ local_publ);
+ list_move_tail(&p->local_publ,
+ &sr->local_publ);
} else {
- publ = list_first_entry(&info->all_publ,
- struct publication,
- all_publ);
- list_move_tail(&publ->all_publ,
- &info->all_publ);
+ p = list_first_entry(&sr->all_publ,
+ struct publication,
+ all_publ);
+ list_move_tail(&p->all_publ,
+ &sr->all_publ);
}
}
/* Round-Robin Algorithm */
- else if (*destnode == tipc_own_addr(net)) {
- if (list_empty(&info->local_publ))
+ else if (*destnode == self) {
+ if (list_empty(&sr->local_publ))
goto no_match;
- publ = list_first_entry(&info->local_publ, struct publication,
- local_publ);
- list_move_tail(&publ->local_publ, &info->local_publ);
+ p = list_first_entry(&sr->local_publ, struct publication,
+ local_publ);
+ list_move_tail(&p->local_publ, &sr->local_publ);
} else {
- publ = list_first_entry(&info->all_publ, struct publication,
- all_publ);
- list_move_tail(&publ->all_publ, &info->all_publ);
+ p = list_first_entry(&sr->all_publ, struct publication,
+ all_publ);
+ list_move_tail(&p->all_publ, &sr->all_publ);
}
- port = publ->port;
- node = publ->node;
+ port = p->port;
+ node = p->node;
no_match:
- spin_unlock_bh(&seq->lock);
+ spin_unlock_bh(&sc->lock);
not_found:
rcu_read_unlock();
*destnode = node;
@@ -567,34 +479,36 @@ bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 scope,
bool all)
{
u32 self = tipc_own_addr(net);
- struct publication *publ;
- struct name_info *info;
- struct name_seq *seq;
- struct sub_seq *sseq;
+ struct service_range *sr;
+ struct tipc_service *sc;
+ struct publication *p;
*dstcnt = 0;
rcu_read_lock();
- seq = nametbl_find_seq(net, type);
- if (unlikely(!seq))
+ sc = tipc_service_find(net, type);
+ if (unlikely(!sc))
goto exit;
- spin_lock_bh(&seq->lock);
- sseq = nameseq_find_subseq(seq, instance);
- if (likely(sseq)) {
- info = sseq->info;
- list_for_each_entry(publ, &info->all_publ, all_publ) {
- if (publ->scope != scope)
- continue;
- if (publ->port == exclude && publ->node == self)
- continue;
- tipc_dest_push(dsts, publ->node, publ->port);
- (*dstcnt)++;
- if (all)
- continue;
- list_move_tail(&publ->all_publ, &info->all_publ);
- break;
- }
+
+ spin_lock_bh(&sc->lock);
+
+ sr = tipc_service_find_range(sc, instance);
+ if (!sr)
+ goto no_match;
+
+ list_for_each_entry(p, &sr->all_publ, all_publ) {
+ if (p->scope != scope)
+ continue;
+ if (p->port == exclude && p->node == self)
+ continue;
+ tipc_dest_push(dsts, p->node, p->port);
+ (*dstcnt)++;
+ if (all)
+ continue;
+ list_move_tail(&p->all_publ, &sr->all_publ);
+ break;
}
- spin_unlock_bh(&seq->lock);
+no_match:
+ spin_unlock_bh(&sc->lock);
exit:
rcu_read_unlock();
return !list_empty(dsts);
@@ -603,61 +517,64 @@ bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 scope,
void tipc_nametbl_mc_lookup(struct net *net, u32 type, u32 lower, u32 upper,
u32 scope, bool exact, struct list_head *dports)
{
- struct sub_seq *sseq_stop;
- struct name_info *info;
+ struct service_range *sr;
+ struct tipc_service *sc;
struct publication *p;
- struct name_seq *seq;
- struct sub_seq *sseq;
+ struct rb_node *n;
rcu_read_lock();
- seq = nametbl_find_seq(net, type);
- if (!seq)
+ sc = tipc_service_find(net, type);
+ if (!sc)
goto exit;
- spin_lock_bh(&seq->lock);
- sseq = seq->sseqs + nameseq_locate_subseq(seq, lower);
- sseq_stop = seq->sseqs + seq->first_free;
- for (; sseq != sseq_stop; sseq++) {
- if (sseq->lower > upper)
+ spin_lock_bh(&sc->lock);
+
+ for (n = rb_first(&sc->ranges); n; n = rb_next(n)) {
+ sr = container_of(n, struct service_range, tree_node);
+ if (sr->upper < lower)
+ continue;
+ if (sr->lower > upper)
break;
- info = sseq->info;
- list_for_each_entry(p, &info->local_publ, local_publ) {
+ list_for_each_entry(p, &sr->local_publ, local_publ) {
if (p->scope == scope || (!exact && p->scope < scope))
tipc_dest_push(dports, 0, p->port);
}
}
- spin_unlock_bh(&seq->lock);
+ spin_unlock_bh(&sc->lock);
exit:
rcu_read_unlock();
}
/* tipc_nametbl_lookup_dst_nodes - find broadcast destination nodes
* - Creates list of nodes that overlap the given multicast address
- * - Determines if any node local ports overlap
+ * - Determines if any node local destinations overlap
*/
void tipc_nametbl_lookup_dst_nodes(struct net *net, u32 type, u32 lower,
u32 upper, struct tipc_nlist *nodes)
{
- struct sub_seq *sseq, *stop;
- struct publication *publ;
- struct name_info *info;
- struct name_seq *seq;
+ struct service_range *sr;
+ struct tipc_service *sc;
+ struct publication *p;
+ struct rb_node *n;
rcu_read_lock();
- seq = nametbl_find_seq(net, type);
- if (!seq)
+ sc = tipc_service_find(net, type);
+ if (!sc)
goto exit;
- spin_lock_bh(&seq->lock);
- sseq = seq->sseqs + nameseq_locate_subseq(seq, lower);
- stop = seq->sseqs + seq->first_free;
- for (; sseq != stop && sseq->lower <= upper; sseq++) {
- info = sseq->info;
- list_for_each_entry(publ, &info->all_publ, all_publ) {
- tipc_nlist_add(nodes, publ->node);
+ spin_lock_bh(&sc->lock);
+
+ for (n = rb_first(&sc->ranges); n; n = rb_next(n)) {
+ sr = container_of(n, struct service_range, tree_node);
+ if (sr->upper < lower)
+ continue;
+ if (sr->lower > upper)
+ break;
+ list_for_each_entry(p, &sr->all_publ, all_publ) {
+ tipc_nlist_add(nodes, p->node);
}
}
- spin_unlock_bh(&seq->lock);
+ spin_unlock_bh(&sc->lock);
exit:
rcu_read_unlock();
}
@@ -667,89 +584,88 @@ void tipc_nametbl_lookup_dst_nodes(struct net *net, u32 type, u32 lower,
void tipc_nametbl_build_group(struct net *net, struct tipc_group *grp,
u32 type, u32 scope)
{
- struct sub_seq *sseq, *stop;
- struct name_info *info;
+ struct service_range *sr;
+ struct tipc_service *sc;
struct publication *p;
- struct name_seq *seq;
+ struct rb_node *n;
rcu_read_lock();
- seq = nametbl_find_seq(net, type);
- if (!seq)
+ sc = tipc_service_find(net, type);
+ if (!sc)
goto exit;
- spin_lock_bh(&seq->lock);
- sseq = seq->sseqs;
- stop = seq->sseqs + seq->first_free;
- for (; sseq != stop; sseq++) {
- info = sseq->info;
- list_for_each_entry(p, &info->all_publ, all_publ) {
+ spin_lock_bh(&sc->lock);
+ for (n = rb_first(&sc->ranges); n; n = rb_next(n)) {
+ sr = container_of(n, struct service_range, tree_node);
+ list_for_each_entry(p, &sr->all_publ, all_publ) {
if (p->scope != scope)
continue;
tipc_group_add_member(grp, p->node, p->port, p->lower);
}
}
- spin_unlock_bh(&seq->lock);
+ spin_unlock_bh(&sc->lock);
exit:
rcu_read_unlock();
}
-/*
- * tipc_nametbl_publish - add name publication to network name tables
+/* tipc_nametbl_publish - add service binding to name table
*/
struct publication *tipc_nametbl_publish(struct net *net, u32 type, u32 lower,
- u32 upper, u32 scope, u32 port_ref,
+ u32 upper, u32 scope, u32 port,
u32 key)
{
- struct publication *publ;
- struct sk_buff *buf = NULL;
- struct tipc_net *tn = net_generic(net, tipc_net_id);
+ struct name_table *nt = tipc_name_table(net);
+ struct tipc_net *tn = tipc_net(net);
+ struct sk_buff *skb = NULL;
+ struct publication *p;
spin_lock_bh(&tn->nametbl_lock);
- if (tn->nametbl->local_publ_count >= TIPC_MAX_PUBLICATIONS) {
- pr_warn("Publication failed, local publication limit reached (%u)\n",
- TIPC_MAX_PUBLICATIONS);
- spin_unlock_bh(&tn->nametbl_lock);
- return NULL;
+
+ if (nt->local_publ_count >= TIPC_MAX_PUBL) {
+ pr_warn("Bind failed, max limit %u reached\n", TIPC_MAX_PUBL);
+ goto exit;
}
- publ = tipc_nametbl_insert_publ(net, type, lower, upper, scope,
- tipc_own_addr(net), port_ref, key);
- if (likely(publ)) {
- tn->nametbl->local_publ_count++;
- buf = tipc_named_publish(net, publ);
+ p = tipc_nametbl_insert_publ(net, type, lower, upper, scope,
+ tipc_own_addr(net), port, key);
+ if (p) {
+ nt->local_publ_count++;
+ skb = tipc_named_publish(net, p);
/* Any pending external events? */
tipc_named_process_backlog(net);
}
+exit:
spin_unlock_bh(&tn->nametbl_lock);
- if (buf)
- tipc_node_broadcast(net, buf);
- return publ;
+ if (skb)
+ tipc_node_broadcast(net, skb);
+ return p;
}
/**
- * tipc_nametbl_withdraw - withdraw name publication from network name tables
+ * tipc_nametbl_withdraw - withdraw a service binding
*/
-int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, u32 port,
- u32 key)
+int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower,
+ u32 port, u32 key)
{
- struct publication *publ;
+ struct name_table *nt = tipc_name_table(net);
+ struct tipc_net *tn = tipc_net(net);
+ u32 self = tipc_own_addr(net);
struct sk_buff *skb = NULL;
- struct tipc_net *tn = net_generic(net, tipc_net_id);
+ struct publication *p;
spin_lock_bh(&tn->nametbl_lock);
- publ = tipc_nametbl_remove_publ(net, type, lower, tipc_own_addr(net),
- port, key);
- if (likely(publ)) {
- tn->nametbl->local_publ_count--;
- skb = tipc_named_withdraw(net, publ);
+
+ p = tipc_nametbl_remove_publ(net, type, lower, self, port, key);
+ if (p) {
+ nt->local_publ_count--;
+ skb = tipc_named_withdraw(net, p);
/* Any pending external events? */
tipc_named_process_backlog(net);
- list_del_init(&publ->binding_sock);
- kfree_rcu(publ, rcu);
+ list_del_init(&p->binding_sock);
+ kfree_rcu(p, rcu);
} else {
- pr_err("Unable to remove local publication\n"
- "(type=%u, lower=%u, port=%u, key=%u)\n",
+ pr_err("Failed to remove local publication {%u,%u,%u}/%u\n",
type, lower, port, key);
}
spin_unlock_bh(&tn->nametbl_lock);
@@ -766,27 +682,24 @@ int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, u32 port,
*/
void tipc_nametbl_subscribe(struct tipc_subscription *sub)
{
+ struct name_table *nt = tipc_name_table(sub->net);
struct tipc_net *tn = tipc_net(sub->net);
struct tipc_subscr *s = &sub->evt.s;
u32 type = tipc_sub_read(s, seq.type);
- int index = hash(type);
- struct name_seq *seq;
- struct tipc_name_seq ns;
+ struct tipc_service *sc;
spin_lock_bh(&tn->nametbl_lock);
- seq = nametbl_find_seq(sub->net, type);
- if (!seq)
- seq = tipc_nameseq_create(type, &tn->nametbl->seq_hlist[index]);
- if (seq) {
- spin_lock_bh(&seq->lock);
- tipc_nameseq_subscribe(seq, sub);
- spin_unlock_bh(&seq->lock);
+ sc = tipc_service_find(sub->net, type);
+ if (!sc)
+ sc = tipc_service_create(type, &nt->services[hash(type)]);
+ if (sc) {
+ spin_lock_bh(&sc->lock);
+ tipc_service_subscribe(sc, sub);
+ spin_unlock_bh(&sc->lock);
} else {
- ns.type = tipc_sub_read(s, seq.type);
- ns.lower = tipc_sub_read(s, seq.lower);
- ns.upper = tipc_sub_read(s, seq.upper);
- pr_warn("Failed to create subscription for {%u,%u,%u}\n",
- ns.type, ns.lower, ns.upper);
+ pr_warn("Failed to subscribe for {%u,%u,%u}\n", type,
+ tipc_sub_read(s, seq.lower),
+ tipc_sub_read(s, seq.upper));
}
spin_unlock_bh(&tn->nametbl_lock);
}
@@ -796,124 +709,122 @@ void tipc_nametbl_subscribe(struct tipc_subscription *sub)
*/
void tipc_nametbl_unsubscribe(struct tipc_subscription *sub)
{
- struct tipc_subscr *s = &sub->evt.s;
struct tipc_net *tn = tipc_net(sub->net);
- struct name_seq *seq;
+ struct tipc_subscr *s = &sub->evt.s;
u32 type = tipc_sub_read(s, seq.type);
+ struct tipc_service *sc;
spin_lock_bh(&tn->nametbl_lock);
- seq = nametbl_find_seq(sub->net, type);
- if (seq != NULL) {
- spin_lock_bh(&seq->lock);
- list_del_init(&sub->nameseq_list);
- tipc_sub_put(sub);
- if (!seq->first_free && list_empty(&seq->subscriptions)) {
- hlist_del_init_rcu(&seq->ns_list);
- kfree(seq->sseqs);
- spin_unlock_bh(&seq->lock);
- kfree_rcu(seq, rcu);
- } else {
- spin_unlock_bh(&seq->lock);
- }
+ sc = tipc_service_find(sub->net, type);
+ if (!sc)
+ goto exit;
+
+ spin_lock_bh(&sc->lock);
+ list_del_init(&sub->service_list);
+ tipc_sub_put(sub);
+
+ /* Delete service item if no more publications and subscriptions */
+ if (RB_EMPTY_ROOT(&sc->ranges) && list_empty(&sc->subscriptions)) {
+ hlist_del_init_rcu(&sc->service_list);
+ kfree_rcu(sc, rcu);
}
+ spin_unlock_bh(&sc->lock);
+exit:
spin_unlock_bh(&tn->nametbl_lock);
}
int tipc_nametbl_init(struct net *net)
{
- struct tipc_net *tn = net_generic(net, tipc_net_id);
- struct name_table *tipc_nametbl;
+ struct tipc_net *tn = tipc_net(net);
+ struct name_table *nt;
int i;
- tipc_nametbl = kzalloc(sizeof(*tipc_nametbl), GFP_ATOMIC);
- if (!tipc_nametbl)
+ nt = kzalloc(sizeof(*nt), GFP_ATOMIC);
+ if (!nt)
return -ENOMEM;
for (i = 0; i < TIPC_NAMETBL_SIZE; i++)
- INIT_HLIST_HEAD(&tipc_nametbl->seq_hlist[i]);
+ INIT_HLIST_HEAD(&nt->services[i]);
- INIT_LIST_HEAD(&tipc_nametbl->node_scope);
- INIT_LIST_HEAD(&tipc_nametbl->cluster_scope);
- tn->nametbl = tipc_nametbl;
+ INIT_LIST_HEAD(&nt->node_scope);
+ INIT_LIST_HEAD(&nt->cluster_scope);
+ tn->nametbl = nt;
spin_lock_init(&tn->nametbl_lock);
return 0;
}
/**
- * tipc_purge_publications - remove all publications for a given type
- *
- * tipc_nametbl_lock must be held when calling this function
+ * tipc_service_delete - purge all publications for a service and delete it
*/
-static void tipc_purge_publications(struct net *net, struct name_seq *seq)
+static void tipc_service_delete(struct net *net, struct tipc_service *sc)
{
- struct publication *publ, *safe;
- struct sub_seq *sseq;
- struct name_info *info;
-
- spin_lock_bh(&seq->lock);
- sseq = seq->sseqs;
- info = sseq->info;
- list_for_each_entry_safe(publ, safe, &info->all_publ, all_publ) {
- tipc_nameseq_remove_publ(net, seq, publ->lower, publ->node,
- publ->port, publ->key);
- kfree_rcu(publ, rcu);
+ struct service_range *sr, *tmpr;
+ struct publication *p, *tmpb;
+
+ spin_lock_bh(&sc->lock);
+ rbtree_postorder_for_each_entry_safe(sr, tmpr, &sc->ranges, tree_node) {
+ list_for_each_entry_safe(p, tmpb,
+ &sr->all_publ, all_publ) {
+ tipc_service_remove_publ(net, sc, p->lower, p->node,
+ p->port, p->key);
+ kfree_rcu(p, rcu);
+ }
}
- hlist_del_init_rcu(&seq->ns_list);
- kfree(seq->sseqs);
- spin_unlock_bh(&seq->lock);
-
- kfree_rcu(seq, rcu);
+ hlist_del_init_rcu(&sc->service_list);
+ spin_unlock_bh(&sc->lock);
+ kfree_rcu(sc, rcu);
}
void tipc_nametbl_stop(struct net *net)
{
+ struct name_table *nt = tipc_name_table(net);
+ struct tipc_net *tn = tipc_net(net);
+ struct hlist_head *service_head;
+ struct tipc_service *service;
u32 i;
- struct name_seq *seq;
- struct hlist_head *seq_head;
- struct tipc_net *tn = net_generic(net, tipc_net_id);
- struct name_table *tipc_nametbl = tn->nametbl;
/* Verify name table is empty and purge any lingering
* publications, then release the name table
*/
spin_lock_bh(&tn->nametbl_lock);
for (i = 0; i < TIPC_NAMETBL_SIZE; i++) {
- if (hlist_empty(&tipc_nametbl->seq_hlist[i]))
+ if (hlist_empty(&nt->services[i]))
continue;
- seq_head = &tipc_nametbl->seq_hlist[i];
- hlist_for_each_entry_rcu(seq, seq_head, ns_list) {
- tipc_purge_publications(net, seq);
+ service_head = &nt->services[i];
+ hlist_for_each_entry_rcu(service, service_head, service_list) {
+ tipc_service_delete(net, service);
}
}
spin_unlock_bh(&tn->nametbl_lock);
synchronize_net();
- kfree(tipc_nametbl);
-
+ kfree(nt);
}
static int __tipc_nl_add_nametable_publ(struct tipc_nl_msg *msg,
- struct name_seq *seq,
- struct sub_seq *sseq, u32 *last_publ)
+ struct tipc_service *service,
+ struct service_range *sr,
+ u32 *last_key)
{
- void *hdr;
- struct nlattr *attrs;
- struct nlattr *publ;
struct publication *p;
+ struct nlattr *attrs;
+ struct nlattr *b;
+ void *hdr;
- if (*last_publ) {
- list_for_each_entry(p, &sseq->info->all_publ, all_publ)
- if (p->key == *last_publ)
+ if (*last_key) {
+ list_for_each_entry(p, &sr->all_publ, all_publ)
+ if (p->key == *last_key)
break;
- if (p->key != *last_publ)
+ if (p->key != *last_key)
return -EPIPE;
} else {
- p = list_first_entry(&sseq->info->all_publ, struct publication,
+ p = list_first_entry(&sr->all_publ,
+ struct publication,
all_publ);
}
- list_for_each_entry_from(p, &sseq->info->all_publ, all_publ) {
- *last_publ = p->key;
+ list_for_each_entry_from(p, &sr->all_publ, all_publ) {
+ *last_key = p->key;
hdr = genlmsg_put(msg->skb, msg->portid, msg->seq,
&tipc_genl_family, NLM_F_MULTI,
@@ -925,15 +836,15 @@ static int __tipc_nl_add_nametable_publ(struct tipc_nl_msg *msg,
if (!attrs)
goto msg_full;
- publ = nla_nest_start(msg->skb, TIPC_NLA_NAME_TABLE_PUBL);
- if (!publ)
+ b = nla_nest_start(msg->skb, TIPC_NLA_NAME_TABLE_PUBL);
+ if (!b)
goto attr_msg_full;
- if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_TYPE, seq->type))
+ if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_TYPE, service->type))
goto publ_msg_full;
- if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_LOWER, sseq->lower))
+ if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_LOWER, sr->lower))
goto publ_msg_full;
- if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_UPPER, sseq->upper))
+ if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_UPPER, sr->upper))
goto publ_msg_full;
if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_SCOPE, p->scope))
goto publ_msg_full;
@@ -944,16 +855,16 @@ static int __tipc_nl_add_nametable_publ(struct tipc_nl_msg *msg,
if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_KEY, p->key))
goto publ_msg_full;
- nla_nest_end(msg->skb, publ);
+ nla_nest_end(msg->skb, b);
nla_nest_end(msg->skb, attrs);
genlmsg_end(msg->skb, hdr);
}
- *last_publ = 0;
+ *last_key = 0;
return 0;
publ_msg_full:
- nla_nest_cancel(msg->skb, publ);
+ nla_nest_cancel(msg->skb, b);
attr_msg_full:
nla_nest_cancel(msg->skb, attrs);
msg_full:
@@ -962,39 +873,34 @@ static int __tipc_nl_add_nametable_publ(struct tipc_nl_msg *msg,
return -EMSGSIZE;
}
-static int __tipc_nl_subseq_list(struct tipc_nl_msg *msg, struct name_seq *seq,
- u32 *last_lower, u32 *last_publ)
+static int __tipc_nl_service_range_list(struct tipc_nl_msg *msg,
+ struct tipc_service *sc,
+ u32 *last_lower, u32 *last_key)
{
- struct sub_seq *sseq;
- struct sub_seq *sseq_start;
+ struct service_range *sr;
+ struct rb_node *n;
int err;
- if (*last_lower) {
- sseq_start = nameseq_find_subseq(seq, *last_lower);
- if (!sseq_start)
- return -EPIPE;
- } else {
- sseq_start = seq->sseqs;
- }
-
- for (sseq = sseq_start; sseq != &seq->sseqs[seq->first_free]; sseq++) {
- err = __tipc_nl_add_nametable_publ(msg, seq, sseq, last_publ);
+ for (n = rb_first(&sc->ranges); n; n = rb_next(n)) {
+ sr = container_of(n, struct service_range, tree_node);
+ if (sr->lower < *last_lower)
+ continue;
+ err = __tipc_nl_add_nametable_publ(msg, sc, sr, last_key);
if (err) {
- *last_lower = sseq->lower;
+ *last_lower = sr->lower;
return err;
}
}
*last_lower = 0;
-
return 0;
}
-static int tipc_nl_seq_list(struct net *net, struct tipc_nl_msg *msg,
- u32 *last_type, u32 *last_lower, u32 *last_publ)
+static int tipc_nl_service_list(struct net *net, struct tipc_nl_msg *msg,
+ u32 *last_type, u32 *last_lower, u32 *last_key)
{
- struct tipc_net *tn = net_generic(net, tipc_net_id);
- struct hlist_head *seq_head;
- struct name_seq *seq = NULL;
+ struct tipc_net *tn = tipc_net(net);
+ struct tipc_service *service = NULL;
+ struct hlist_head *head;
int err;
int i;
@@ -1004,30 +910,31 @@ static int tipc_nl_seq_list(struct net *net, struct tipc_nl_msg *msg,
i = 0;
for (; i < TIPC_NAMETBL_SIZE; i++) {
- seq_head = &tn->nametbl->seq_hlist[i];
+ head = &tn->nametbl->services[i];
if (*last_type) {
- seq = nametbl_find_seq(net, *last_type);
- if (!seq)
+ service = tipc_service_find(net, *last_type);
+ if (!service)
return -EPIPE;
} else {
- hlist_for_each_entry_rcu(seq, seq_head, ns_list)
+ hlist_for_each_entry_rcu(service, head, service_list)
break;
- if (!seq)
+ if (!service)
continue;
}
- hlist_for_each_entry_from_rcu(seq, ns_list) {
- spin_lock_bh(&seq->lock);
- err = __tipc_nl_subseq_list(msg, seq, last_lower,
- last_publ);
+ hlist_for_each_entry_from_rcu(service, service_list) {
+ spin_lock_bh(&service->lock);
+ err = __tipc_nl_service_range_list(msg, service,
+ last_lower,
+ last_key);
if (err) {
- *last_type = seq->type;
- spin_unlock_bh(&seq->lock);
+ *last_type = service->type;
+ spin_unlock_bh(&service->lock);
return err;
}
- spin_unlock_bh(&seq->lock);
+ spin_unlock_bh(&service->lock);
}
*last_type = 0;
}
@@ -1036,13 +943,13 @@ static int tipc_nl_seq_list(struct net *net, struct tipc_nl_msg *msg,
int tipc_nl_name_table_dump(struct sk_buff *skb, struct netlink_callback *cb)
{
- int err;
- int done = cb->args[3];
+ struct net *net = sock_net(skb->sk);
u32 last_type = cb->args[0];
u32 last_lower = cb->args[1];
- u32 last_publ = cb->args[2];
- struct net *net = sock_net(skb->sk);
+ u32 last_key = cb->args[2];
+ int done = cb->args[3];
struct tipc_nl_msg msg;
+ int err;
if (done)
return 0;
@@ -1052,7 +959,8 @@ int tipc_nl_name_table_dump(struct sk_buff *skb, struct netlink_callback *cb)
msg.seq = cb->nlh->nlmsg_seq;
rcu_read_lock();
- err = tipc_nl_seq_list(net, &msg, &last_type, &last_lower, &last_publ);
+ err = tipc_nl_service_list(net, &msg, &last_type,
+ &last_lower, &last_key);
if (!err) {
done = 1;
} else if (err != -EMSGSIZE) {
@@ -1068,7 +976,7 @@ int tipc_nl_name_table_dump(struct sk_buff *skb, struct netlink_callback *cb)
cb->args[0] = last_type;
cb->args[1] = last_lower;
- cb->args[2] = last_publ;
+ cb->args[2] = last_key;
cb->args[3] = done;
return skb->len;
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index 34a4ccb..1b03b87 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -97,7 +97,7 @@ struct publication {
* @local_publ_count: number of publications issued by this node
*/
struct name_table {
- struct hlist_head seq_hlist[TIPC_NAMETBL_SIZE];
+ struct hlist_head services[TIPC_NAMETBL_SIZE];
struct list_head node_scope;
struct list_head cluster_scope;
u32 local_publ_count;
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 4fb4327..85e777e 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -324,12 +324,12 @@ static void tipc_node_write_unlock(struct tipc_node *n)
if (flags & TIPC_NOTIFY_LINK_UP) {
tipc_mon_peer_up(net, addr, bearer_id);
tipc_nametbl_publish(net, TIPC_LINK_STATE, addr, addr,
- TIPC_NODE_SCOPE, link_id, addr);
+ TIPC_NODE_SCOPE, link_id, link_id);
}
if (flags & TIPC_NOTIFY_LINK_DOWN) {
tipc_mon_peer_down(net, addr, bearer_id);
tipc_nametbl_withdraw(net, TIPC_LINK_STATE, addr,
- link_id, addr);
+ link_id, link_id);
}
}
diff --git a/net/tipc/subscr.h b/net/tipc/subscr.h
index 8b2d22b..d793b43 100644
--- a/net/tipc/subscr.h
+++ b/net/tipc/subscr.h
@@ -40,7 +40,7 @@
#include "topsrv.h"
#define TIPC_MAX_SUBSCR 65535
-#define TIPC_MAX_PUBLICATIONS 65535
+#define TIPC_MAX_PUBL 65535
struct tipc_subscription;
struct tipc_conn;
@@ -58,7 +58,7 @@ struct tipc_subscription {
struct kref kref;
struct net *net;
struct timer_list timer;
- struct list_head nameseq_list;
+ struct list_head service_list;
struct list_head sub_list;
struct tipc_event evt;
int conid;
--
2.1.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related
* [net-next 4/4] tipc: tipc: rename address types in user api
From: Jon Maloy @ 2018-03-28 14:58 UTC (permalink / raw)
To: davem, netdev; +Cc: tipc-discussion, mohan.krishna.ghanta.krishnamurthy
In-Reply-To: <1522249103-32714-1-git-send-email-jon.maloy@ericsson.com>
The three address type structs in the user API have names that in
reality reflect the specific, non-Linux environment where they were
originally created.
We now give them more intuitive names, in accordance with how TIPC is
described in the current documentation.
struct tipc_portid -> struct tipc_socket_addr
struct tipc_name -> struct tipc_service_addr
struct tipc_name_seq -> struct tipc_service_range
For compatibility, we add macros that map the old names to the new ones.
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
include/uapi/linux/tipc.h | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/include/uapi/linux/tipc.h b/include/uapi/linux/tipc.h
index 4ac9f1f..254f6e3 100644
--- a/include/uapi/linux/tipc.h
+++ b/include/uapi/linux/tipc.h
@@ -45,17 +45,17 @@
* TIPC addressing primitives
*/
-struct tipc_portid {
+struct tipc_socket_addr {
__u32 ref;
__u32 node;
};
-struct tipc_name {
+struct tipc_service_addr {
__u32 type;
__u32 instance;
};
-struct tipc_name_seq {
+struct tipc_service_range {
__u32 type;
__u32 lower;
__u32 upper;
@@ -108,28 +108,28 @@ enum tipc_scope {
* TIPC topology subscription service definitions
*/
-#define TIPC_SUB_PORTS 0x01 /* filter for port availability */
-#define TIPC_SUB_SERVICE 0x02 /* filter for service availability */
-#define TIPC_SUB_CANCEL 0x04 /* cancel a subscription */
+#define TIPC_SUB_PORTS 0x01 /* filter: evt at each match */
+#define TIPC_SUB_SERVICE 0x02 /* filter: evt at first up/last down */
+#define TIPC_SUB_CANCEL 0x04 /* filter: cancel a subscription */
#define TIPC_WAIT_FOREVER (~0) /* timeout for permanent subscription */
struct tipc_subscr {
- struct tipc_name_seq seq; /* name sequence of interest */
+ struct tipc_service_range seq; /* range of interest */
__u32 timeout; /* subscription duration (in ms) */
__u32 filter; /* bitmask of filter options */
char usr_handle[8]; /* available for subscriber use */
};
#define TIPC_PUBLISHED 1 /* publication event */
-#define TIPC_WITHDRAWN 2 /* withdraw event */
+#define TIPC_WITHDRAWN 2 /* withdrawal event */
#define TIPC_SUBSCR_TIMEOUT 3 /* subscription timeout event */
struct tipc_event {
__u32 event; /* event type */
- __u32 found_lower; /* matching name seq instances */
- __u32 found_upper; /* " " " " */
- struct tipc_portid port; /* associated port */
+ __u32 found_lower; /* matching range */
+ __u32 found_upper; /* " " */
+ struct tipc_socket_addr port; /* associated socket */
struct tipc_subscr s; /* associated subscription */
};
@@ -159,10 +159,10 @@ struct sockaddr_tipc {
unsigned char addrtype;
signed char scope;
union {
- struct tipc_portid id;
- struct tipc_name_seq nameseq;
+ struct tipc_socket_addr id;
+ struct tipc_service_range nameseq;
struct {
- struct tipc_name name;
+ struct tipc_service_addr name;
__u32 domain;
} name;
} addr;
@@ -250,6 +250,10 @@ struct tipc_sioc_ln_req {
#define TIPC_ZONE_CLUSTER_MASK (TIPC_ZONE_MASK | TIPC_CLUSTER_MASK)
+#define tipc_portid tipc_socket_addr
+#define tipc_name tipc_service_addr
+#define tipc_name_seq tipc_service_range
+
static inline __u32 tipc_addr(unsigned int zone,
unsigned int cluster,
unsigned int node)
--
2.1.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related
* [net-next 2/4] tipc: refactor name table translate function
From: Jon Maloy @ 2018-03-28 14:58 UTC (permalink / raw)
To: davem, netdev
Cc: mohan.krishna.ghanta.krishnamurthy, tung.q.nguyen, hoang.h.le,
jon.maloy, canh.d.luu, ying.xue, tipc-discussion
In-Reply-To: <1522249103-32714-1-git-send-email-jon.maloy@ericsson.com>
The function tipc_nametbl_translate() function is ugly and hard to
follow. This can be improved somewhat by introducing a stack variable
for holding the publication list to be used and re-ordering the if-
clauses for selection of algorithm.
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/name_table.c | 61 +++++++++++++++++++++------------------------------
1 file changed, 25 insertions(+), 36 deletions(-)
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index c309402..9915be0 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -399,29 +399,32 @@ struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
/**
* tipc_nametbl_translate - perform service instance to socket translation
*
- * On entry, 'destnode' is the search domain used during translation.
+ * On entry, 'dnode' is the search domain used during translation.
*
* On exit:
- * - if name translation is deferred to another node/cluster/zone,
- * leaves 'destnode' unchanged (will be non-zero) and returns 0
- * - if name translation is attempted and succeeds, sets 'destnode'
- * to publication node and returns port reference (will be non-zero)
- * - if name translation is attempted and fails, sets 'destnode' to 0
- * and returns 0
+ * - if translation is deferred to another node, leave 'dnode' unchanged and
+ * return 0
+ * - if translation is attempted and succeeds, set 'dnode' to the publishing
+ * node and return the published (non-zero) port number
+ * - if translation is attempted and fails, set 'dnode' to 0 and return 0
+ *
+ * Note that for legacy users (node configured with Z.C.N address format) the
+ * 'closest-first' lookup algorithm must be maintained, i.e., if dnode is 0
+ * we must look in the local binding list first
*/
-u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance,
- u32 *destnode)
+u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance, u32 *dnode)
{
struct tipc_net *tn = tipc_net(net);
bool legacy = tn->legacy_addr_format;
u32 self = tipc_own_addr(net);
struct service_range *sr;
struct tipc_service *sc;
+ struct list_head *list;
struct publication *p;
u32 port = 0;
u32 node = 0;
- if (!tipc_in_scope(legacy, *destnode, self))
+ if (!tipc_in_scope(legacy, *dnode, self))
return 0;
rcu_read_lock();
@@ -434,43 +437,29 @@ u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance,
if (unlikely(!sr))
goto no_match;
- /* Closest-First Algorithm */
- if (legacy && !*destnode) {
- if (!list_empty(&sr->local_publ)) {
- p = list_first_entry(&sr->local_publ,
- struct publication,
- local_publ);
- list_move_tail(&p->local_publ,
- &sr->local_publ);
- } else {
- p = list_first_entry(&sr->all_publ,
- struct publication,
- all_publ);
- list_move_tail(&p->all_publ,
- &sr->all_publ);
- }
- }
-
- /* Round-Robin Algorithm */
- else if (*destnode == self) {
- if (list_empty(&sr->local_publ))
+ /* Select lookup algorithm: local, closest-first or round-robin */
+ if (*dnode == self) {
+ list = &sr->local_publ;
+ if (list_empty(list))
goto no_match;
- p = list_first_entry(&sr->local_publ, struct publication,
- local_publ);
+ p = list_first_entry(list, struct publication, local_publ);
+ list_move_tail(&p->local_publ, &sr->local_publ);
+ } else if (legacy && !*dnode && !list_empty(&sr->local_publ)) {
+ list = &sr->local_publ;
+ p = list_first_entry(list, struct publication, local_publ);
list_move_tail(&p->local_publ, &sr->local_publ);
} else {
- p = list_first_entry(&sr->all_publ, struct publication,
- all_publ);
+ list = &sr->all_publ;
+ p = list_first_entry(list, struct publication, all_publ);
list_move_tail(&p->all_publ, &sr->all_publ);
}
-
port = p->port;
node = p->node;
no_match:
spin_unlock_bh(&sc->lock);
not_found:
rcu_read_unlock();
- *destnode = node;
+ *dnode = node;
return port;
}
--
2.1.4
^ permalink raw reply related
* [net-next 3/4] tipc: permit overlapping service ranges in name table
From: Jon Maloy @ 2018-03-28 14:58 UTC (permalink / raw)
To: davem, netdev
Cc: mohan.krishna.ghanta.krishnamurthy, tung.q.nguyen, hoang.h.le,
jon.maloy, canh.d.luu, ying.xue, tipc-discussion
In-Reply-To: <1522249103-32714-1-git-send-email-jon.maloy@ericsson.com>
With the new RB tree structure for service ranges it becomes possible to
solve an old problem; - we can now allow overlapping service ranges in
the table.
When inserting a new service range to the tree, we use 'lower' as primary
key, and when necessary 'upper' as secondary key.
Since there may now be multiple service ranges matching an indicated
'lower' value, we must also add the 'upper' value to the functions
used for removing publications, so that the correct, corresponding
range item can be found.
These changes guarantee that a well-formed publication/withdrawal item
from a peer node never will be rejected, and make it possible to
eliminate the problematic backlog functionality we currently have for
handling such cases.
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/name_distr.c | 90 +++++++++++++--------------------------------------
net/tipc/name_distr.h | 1 -
net/tipc/name_table.c | 64 +++++++++++++++++-------------------
net/tipc/name_table.h | 8 ++---
net/tipc/net.c | 2 +-
net/tipc/node.c | 2 +-
net/tipc/socket.c | 4 +--
7 files changed, 60 insertions(+), 111 deletions(-)
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 8240a85..51b4b96 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -204,12 +204,12 @@ void tipc_named_node_up(struct net *net, u32 dnode)
*/
static void tipc_publ_purge(struct net *net, struct publication *publ, u32 addr)
{
- struct tipc_net *tn = net_generic(net, tipc_net_id);
+ struct tipc_net *tn = tipc_net(net);
struct publication *p;
spin_lock_bh(&tn->nametbl_lock);
- p = tipc_nametbl_remove_publ(net, publ->type, publ->lower,
- publ->node, publ->port, publ->key);
+ p = tipc_nametbl_remove_publ(net, publ->type, publ->lower, publ->upper,
+ publ->node, publ->key);
if (p)
tipc_node_unsubscribe(net, &p->binding_node, addr);
spin_unlock_bh(&tn->nametbl_lock);
@@ -261,28 +261,31 @@ void tipc_publ_notify(struct net *net, struct list_head *nsub_list, u32 addr)
static bool tipc_update_nametbl(struct net *net, struct distr_item *i,
u32 node, u32 dtype)
{
- struct publication *publ = NULL;
+ struct publication *p = NULL;
+ u32 lower = ntohl(i->lower);
+ u32 upper = ntohl(i->upper);
+ u32 type = ntohl(i->type);
+ u32 port = ntohl(i->port);
+ u32 key = ntohl(i->key);
if (dtype == PUBLICATION) {
- publ = tipc_nametbl_insert_publ(net, ntohl(i->type),
- ntohl(i->lower),
- ntohl(i->upper),
- TIPC_CLUSTER_SCOPE, node,
- ntohl(i->port), ntohl(i->key));
- if (publ) {
- tipc_node_subscribe(net, &publ->binding_node, node);
+ p = tipc_nametbl_insert_publ(net, type, lower, upper,
+ TIPC_CLUSTER_SCOPE, node,
+ port, key);
+ if (p) {
+ tipc_node_subscribe(net, &p->binding_node, node);
return true;
}
} else if (dtype == WITHDRAWAL) {
- publ = tipc_nametbl_remove_publ(net, ntohl(i->type),
- ntohl(i->lower),
- node, ntohl(i->port),
- ntohl(i->key));
- if (publ) {
- tipc_node_unsubscribe(net, &publ->binding_node, node);
- kfree_rcu(publ, rcu);
+ p = tipc_nametbl_remove_publ(net, type, lower,
+ upper, node, key);
+ if (p) {
+ tipc_node_unsubscribe(net, &p->binding_node, node);
+ kfree_rcu(p, rcu);
return true;
}
+ pr_warn_ratelimited("Failed to remove binding %u,%u from %x\n",
+ type, lower, node);
} else {
pr_warn("Unrecognized name table message received\n");
}
@@ -290,53 +293,6 @@ static bool tipc_update_nametbl(struct net *net, struct distr_item *i,
}
/**
- * tipc_named_add_backlog - add a failed name table update to the backlog
- *
- */
-static void tipc_named_add_backlog(struct net *net, struct distr_item *i,
- u32 type, u32 node)
-{
- struct distr_queue_item *e;
- struct tipc_net *tn = net_generic(net, tipc_net_id);
- unsigned long now = get_jiffies_64();
-
- e = kzalloc(sizeof(*e), GFP_ATOMIC);
- if (!e)
- return;
- e->dtype = type;
- e->node = node;
- e->expires = now + msecs_to_jiffies(sysctl_tipc_named_timeout);
- memcpy(e, i, sizeof(*i));
- list_add_tail(&e->next, &tn->dist_queue);
-}
-
-/**
- * tipc_named_process_backlog - try to process any pending name table updates
- * from the network.
- */
-void tipc_named_process_backlog(struct net *net)
-{
- struct distr_queue_item *e, *tmp;
- struct tipc_net *tn = net_generic(net, tipc_net_id);
- unsigned long now = get_jiffies_64();
-
- list_for_each_entry_safe(e, tmp, &tn->dist_queue, next) {
- if (time_after(e->expires, now)) {
- if (!tipc_update_nametbl(net, &e->i, e->node, e->dtype))
- continue;
- } else {
- pr_warn_ratelimited("Dropping name table update (%d) of {%u, %u, %u} from %x key=%u\n",
- e->dtype, ntohl(e->i.type),
- ntohl(e->i.lower),
- ntohl(e->i.upper),
- e->node, ntohl(e->i.key));
- }
- list_del(&e->next);
- kfree(e);
- }
-}
-
-/**
* tipc_named_rcv - process name table update messages sent by another node
*/
void tipc_named_rcv(struct net *net, struct sk_buff_head *inputq)
@@ -358,12 +314,10 @@ void tipc_named_rcv(struct net *net, struct sk_buff_head *inputq)
count = msg_data_sz(msg) / ITEM_SIZE;
node = msg_orignode(msg);
while (count--) {
- if (!tipc_update_nametbl(net, item, node, mtype))
- tipc_named_add_backlog(net, item, mtype, node);
+ tipc_update_nametbl(net, item, node, mtype);
item++;
}
kfree_skb(skb);
- tipc_named_process_backlog(net);
}
spin_unlock_bh(&tn->nametbl_lock);
}
diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h
index 4753e62..63fc73e 100644
--- a/net/tipc/name_distr.h
+++ b/net/tipc/name_distr.h
@@ -72,7 +72,6 @@ struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ);
void tipc_named_node_up(struct net *net, u32 dnode);
void tipc_named_rcv(struct net *net, struct sk_buff_head *msg_queue);
void tipc_named_reinit(struct net *net);
-void tipc_named_process_backlog(struct net *net);
void tipc_publ_notify(struct net *net, struct list_head *nsub_list, u32 addr);
#endif
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 9915be0..bba3e41 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -171,10 +171,14 @@ static struct service_range *tipc_service_create_range(struct tipc_service *sc,
tmp = container_of(parent, struct service_range, tree_node);
if (lower < tmp->lower)
n = &(*n)->rb_left;
+ else if (lower > tmp->lower)
+ n = &(*n)->rb_right;
+ else if (upper < tmp->upper)
+ n = &(*n)->rb_left;
else if (upper > tmp->upper)
n = &(*n)->rb_right;
else
- return NULL;
+ return tmp;
}
sr = kzalloc(sizeof(*sr), GFP_ATOMIC);
if (!sr)
@@ -200,17 +204,11 @@ static struct publication *tipc_service_insert_publ(struct net *net,
struct publication *p;
bool first = false;
- sr = tipc_service_find_range(sc, lower);
- if (!sr) {
- sr = tipc_service_create_range(sc, lower, upper);
- if (!sr)
- goto err;
- first = true;
- }
+ sr = tipc_service_create_range(sc, lower, upper);
+ if (!sr)
+ goto err;
- /* Lower end overlaps existing entry, but we need an exact match */
- if (sr->lower != lower || sr->upper != upper)
- return NULL;
+ first = list_empty(&sr->all_publ);
/* Return if the publication already exists */
list_for_each_entry(p, &sr->all_publ, all_publ) {
@@ -239,30 +237,32 @@ static struct publication *tipc_service_insert_publ(struct net *net,
/**
* tipc_service_remove_publ - remove a publication from a service
- *
- * NOTE: There may be cases where TIPC is asked to remove a publication
- * that is not in the name table. For example, if another node issues a
- * publication for a name range that overlaps an existing name range
- * the publication will not be recorded, which means the publication won't
- * be found when the name range is later withdrawn by that node.
- * A failed withdraw request simply returns a failure indication and lets the
- * caller issue any error or warning messages associated with such a problem.
*/
static struct publication *tipc_service_remove_publ(struct net *net,
struct tipc_service *sc,
- u32 inst, u32 node,
- u32 port, u32 key)
+ u32 lower, u32 upper,
+ u32 node, u32 key)
{
struct tipc_subscription *sub, *tmp;
struct service_range *sr;
struct publication *p;
bool found = false;
bool last = false;
+ struct rb_node *n;
- sr = tipc_service_find_range(sc, inst);
+ sr = tipc_service_find_range(sc, lower);
if (!sr)
return NULL;
+ /* Find exact matching service range */
+ for (n = &sr->tree_node; n; n = rb_next(n)) {
+ sr = container_of(n, struct service_range, tree_node);
+ if (sr->upper == upper)
+ break;
+ }
+ if (!n || sr->lower != lower || sr->upper != upper)
+ return NULL;
+
/* Find publication, if it exists */
list_for_each_entry(p, &sr->all_publ, all_publ) {
if (p->key != key || (node && node != p->node))
@@ -375,8 +375,8 @@ struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
}
struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
- u32 lower, u32 node, u32 port,
- u32 key)
+ u32 lower, u32 upper,
+ u32 node, u32 key)
{
struct tipc_service *sc = tipc_service_find(net, type);
struct publication *p = NULL;
@@ -385,7 +385,7 @@ struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
return NULL;
spin_lock_bh(&sc->lock);
- p = tipc_service_remove_publ(net, sc, lower, node, port, key);
+ p = tipc_service_remove_publ(net, sc, lower, upper, node, key);
/* Delete service item if this no more publications and subscriptions */
if (RB_EMPTY_ROOT(&sc->ranges) && list_empty(&sc->subscriptions)) {
@@ -620,8 +620,6 @@ struct publication *tipc_nametbl_publish(struct net *net, u32 type, u32 lower,
if (p) {
nt->local_publ_count++;
skb = tipc_named_publish(net, p);
- /* Any pending external events? */
- tipc_named_process_backlog(net);
}
exit:
spin_unlock_bh(&tn->nametbl_lock);
@@ -635,7 +633,7 @@ struct publication *tipc_nametbl_publish(struct net *net, u32 type, u32 lower,
* tipc_nametbl_withdraw - withdraw a service binding
*/
int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower,
- u32 port, u32 key)
+ u32 upper, u32 key)
{
struct name_table *nt = tipc_name_table(net);
struct tipc_net *tn = tipc_net(net);
@@ -645,17 +643,15 @@ int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower,
spin_lock_bh(&tn->nametbl_lock);
- p = tipc_nametbl_remove_publ(net, type, lower, self, port, key);
+ p = tipc_nametbl_remove_publ(net, type, lower, upper, self, key);
if (p) {
nt->local_publ_count--;
skb = tipc_named_withdraw(net, p);
- /* Any pending external events? */
- tipc_named_process_backlog(net);
list_del_init(&p->binding_sock);
kfree_rcu(p, rcu);
} else {
pr_err("Failed to remove local publication {%u,%u,%u}/%u\n",
- type, lower, port, key);
+ type, lower, upper, key);
}
spin_unlock_bh(&tn->nametbl_lock);
@@ -754,8 +750,8 @@ static void tipc_service_delete(struct net *net, struct tipc_service *sc)
rbtree_postorder_for_each_entry_safe(sr, tmpr, &sc->ranges, tree_node) {
list_for_each_entry_safe(p, tmpb,
&sr->all_publ, all_publ) {
- tipc_service_remove_publ(net, sc, p->lower, p->node,
- p->port, p->key);
+ tipc_service_remove_publ(net, sc, p->lower, p->upper,
+ p->node, p->key);
kfree_rcu(p, rcu);
}
}
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index 1b03b87..4b14fc2 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -116,16 +116,16 @@ bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 domain,
struct list_head *dsts, int *dstcnt, u32 exclude,
bool all);
struct publication *tipc_nametbl_publish(struct net *net, u32 type, u32 lower,
- u32 upper, u32 scope, u32 port_ref,
+ u32 upper, u32 scope, u32 port,
u32 key);
-int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, u32 ref,
+int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, u32 upper,
u32 key);
struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
u32 lower, u32 upper, u32 scope,
u32 node, u32 ref, u32 key);
struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
- u32 lower, u32 node, u32 ref,
- u32 key);
+ u32 lower, u32 upper,
+ u32 node, u32 key);
void tipc_nametbl_subscribe(struct tipc_subscription *s);
void tipc_nametbl_unsubscribe(struct tipc_subscription *s);
int tipc_nametbl_init(struct net *net);
diff --git a/net/tipc/net.c b/net/tipc/net.c
index 29538dc..856f9e9 100644
--- a/net/tipc/net.c
+++ b/net/tipc/net.c
@@ -136,7 +136,7 @@ void tipc_net_stop(struct net *net)
if (!self)
return;
- tipc_nametbl_withdraw(net, TIPC_CFG_SRV, self, 0, self);
+ tipc_nametbl_withdraw(net, TIPC_CFG_SRV, self, self, self);
rtnl_lock();
tipc_bearer_stop(net);
tipc_node_stop(net);
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 85e777e..c77dd2f 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -329,7 +329,7 @@ static void tipc_node_write_unlock(struct tipc_node *n)
if (flags & TIPC_NOTIFY_LINK_DOWN) {
tipc_mon_peer_down(net, addr, bearer_id);
tipc_nametbl_withdraw(net, TIPC_LINK_STATE, addr,
- link_id, link_id);
+ addr, link_id);
}
}
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 275b666..3e5eba3 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -2634,12 +2634,12 @@ static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
if (publ->upper != seq->upper)
break;
tipc_nametbl_withdraw(net, publ->type, publ->lower,
- publ->port, publ->key);
+ publ->upper, publ->key);
rc = 0;
break;
}
tipc_nametbl_withdraw(net, publ->type, publ->lower,
- publ->port, publ->key);
+ publ->upper, publ->key);
rc = 0;
}
if (list_empty(&tsk->publications))
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v3 net 1/5] tcp: feed correct number of pkts acked to cc modules also in recovery
From: Yuchung Cheng @ 2018-03-28 15:04 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: netdev, Neal Cardwell, Eric Dumazet, Sergei Shtylyov
In-Reply-To: <CAK6E8=f77S5VjSofC6v8EY2U+YQ0incyv7beKA-aP2p9c7ebUg@mail.gmail.com>
On Wed, Mar 28, 2018 at 7:14 AM, Yuchung Cheng <ycheng@google.com> wrote:
>
> On Wed, Mar 28, 2018 at 5:45 AM, Ilpo Järvinen
> <ilpo.jarvinen@helsinki.fi> wrote:
> > On Tue, 27 Mar 2018, Yuchung Cheng wrote:
> >
> >> On Tue, Mar 27, 2018 at 7:23 AM, Ilpo Järvinen
> >> <ilpo.jarvinen@helsinki.fi> wrote:
> >> > On Mon, 26 Mar 2018, Yuchung Cheng wrote:
> >> >
> >> >> On Tue, Mar 13, 2018 at 3:25 AM, Ilpo Järvinen
> >> >> <ilpo.jarvinen@helsinki.fi> wrote:
> >> >> >
> >> >> > A miscalculation for the number of acknowledged packets occurs during
> >> >> > RTO recovery whenever SACK is not enabled and a cumulative ACK covers
> >> >> > any non-retransmitted skbs. The reason is that pkts_acked value
> >> >> > calculated in tcp_clean_rtx_queue is not correct for slow start after
> >> >> > RTO as it may include segments that were not lost and therefore did
> >> >> > not need retransmissions in the slow start following the RTO. Then
> >> >> > tcp_slow_start will add the excess into cwnd bloating it and
> >> >> > triggering a burst.
> >> >> >
> >> >> > Instead, we want to pass only the number of retransmitted segments
> >> >> > that were covered by the cumulative ACK (and potentially newly sent
> >> >> > data segments too if the cumulative ACK covers that far).
> >> >> >
> >> >> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> >> >> > ---
> >> >>
> >> >> My understanding is there are two problems
> >> >>
> >> >> 1) your fix: the reordering logic in tcp-remove_reno_sacks requires
> >> >> precise cumulatively acked count, not newly acked count?
> >> >
> >> > While I'm not entirely sure if you intented to say that my fix is broken
> >> > or not, I thought this very difference alot while making the fix and I
> >> > believe that this fix is needed because of the discontinuity at RTO
> >> > (sacked_out is cleared as we set L-bits + lost_out). This is an artifact
> >> > in the imitation of sacked_out for non-SACK but at RTO we can't keep that
> >> > in sync because we set L-bits (and have no S-bits to guide us). Thus, we
> >> > cannot anymore "use" those skbs with only L-bit for the reno_sacks logic.
> >> >
> >> > In tcp_remove_reno_sacks acked - sacked_out is being used to calculate
> >> > tp->delivered, using plain cumulative acked causes congestion control
> >> > breakage later as call to tcp_cong_control will directly use the
> >> > difference in tp->delivered.
> >> >
> >> > This boils down the exact definition of tp->delivered (the one given in
> >> > the header is not detailed enough). I guess you might have better idea
> >> > what it exactly is since one of you has added it? There are subtle things
> >> > in the defination that can make it entirely unsuitable for cc decisions.
> >> > Should those segments that we (possibly) already counted into
> >> > tp->delivered during (potentially preceeding) CA_Recovery be added to it
> >> > for _second time_ or not? This fix avoids such double counting (the
> >> Where is the double counting, assuming normal DUPACK behavior?
> >>
> >> In the non-sack case:
> >>
> >> 1. upon receiving a DUPACK, we assume one packet has been delivered by
> >> incrementing tp->delivered in tcp_add_reno_sack()
> >
> > 1b. RTO here. We clear tp->sacked_out at RTO (i.e., the discontinuity
> > I've tried to point out quite many times already)...
> >
> >> 2. upon receiving a partial ACK or an ACK that acks recovery point
> >> (high_seq), tp->delivered is incremented by (cumulatively acked -
> >> #dupacks) in tcp_remove_reno_sacks()
> >
> > ...and this won't happen correctly anymore after RTO (since non-SACK
> > won't keep #dupacks due to the discontinuity). Thus we end up adding
> > cumulatively acked - 0 to tp->delivered on those ACKs.
> >
> >> therefore tp->delivered is tracking the # of packets delivered
> >> (sacked, acked, DUPACK'd) with the most information it could have
> >> inferred.
> >
> > Since you didn't answer any of my questions about tp->delivered directly,
> > let me rephrase them to this example (non-SACK, of course):
> >
> > 4 segments outstanding. RTO recovery underway (lost_out=4, sacked_out=0).
> > Cwnd = 2 so the sender rexmits 2 out of 4. We get cumulative ACK for
> > three segments. How much should tp->delivered be incremented? 2 or 3?
> >
> > ...I think 2 is the right answer.
> >
> >> From congestion control's perspective, it cares about the delivery
> >> information (e.g. how much), not the sequences (what or how).
> >
> > I guess you must have missed my point. I'm talking about "how much"
> > whole the time. It's just about when can we account for it (and when not).
> >
> >> I am pointing out that
> >>
> >> 1) your fix may fix one corner CC packet accounting issue in the
> >> non-SACK and CA_Loss
> >> 2) but it does not fix the other (major) CC packet accounting issue in
> >> the SACK case
> >
> > You say major but it's major if and only if one is using one of the
> > affected cc modules which were not that many and IMHO not very mainstream
> > anyway (check my list from the previous email, not that I'd mind if they'd
> > get fixed too). The rest of the cc modules do not seem to use the incorrect
> > value even if some of them have the pkts_acked callback.
> >
> > Other CC packet accounting (regardless of SACK) is based on tp->delivered
> > and tp->delivered is NOT similarly miscounted with SACK because the logic
> > depend on !TCPCB_SACKED_ACKED (that fails only if we have very high ACK
> > loss).
> >
> >> 3) it breaks the dynamic dupthresh / reordering in the non-SACK case
> >> as tcp_check_reno_reordering requires strictly cum. ack.
> >
> > I think that the current non-SACK reordering detection logic is not that
> > sound after RTO (but I'm yet to prove this). ...There seems to be some
> > failure modes which is why I even thought of disabling the whole thing
> > for non-SACK RTO recoveries and as it now seems you do care more about
> > non-SACK than you initial claimed, I might even have motivation to fix
> > more those corners rather than the worst bugs only ;-). ...But I'll make
> > the tp->delivered fix only in this patch to avoid any change this part of
> > the code.
> >
> >> Therefore I prefer
> >> a) using tp->delivered to fix (1)(2)
> >> b) perhaps improving tp->delivered SACK emulation code in the non-SACK case
> >
> > Somehow I get an impression that you might assume/say here that
> [resending in plaintext]
> That's wrong impression. Perhaps it's worth re-iterating what I agree
> and disagree
>
> 1. [agree] there's accounting issue in non-SACK as you discovered
> which causes CC misbehavior
>
> 2. [major disagree] adjusting pkts_acked for ca_ops->pkts_acked in non-sack
> => that field is not used by common C.C. (you said so too)
>
> 3. [disagree] adjusting pkts_acked may not affect reordering
> accounting in non-sack
>
>
> For cubic or reno, the main source is the "acked_sacked" passed into
> tcp_cong_avoid(). that variable is derived from tp->delivered.
> Therefore we need to fix that to address the problem in (1)
>
> I have yet to read your code. Will read later today.
Your patch looks good. Some questions / comments:
1. Why only apply to CA_Loss and not also CA_Recovery? this may
mitigate GRO issue I noted in other thread.
2. minor style nit: can we adjust tp->delivered directly in
tcp_clean_rtx_queue(). I am personally against calling "non-sack" reno
(e.g. tcp_*reno*()) because its really confusing w/ Reno congestion
control when SACK is used. One day I'd like to rename all these *reno*
to _nonsack_.
Thanks
>
> > tp->delivered is all correct for non-SACK? ...It isn't without a fix.
> > Therefore a) won't help any for non-SACK. Tp->delivered for non-SACK is
> > currently (mis!)calculated in tcp_remove_reno_sacks because the incorrect
> > pkts_acked is fed to it. ...Thus, b) is an intermediate step in the
> > miscalculation chain I'm fixing with this change. B) resolves also (1)
> > without additional changes to logic!
> >
> > I've included below the updated change that only fixes tp->delivered
> > calculation (not tested besides compiling). ...But I think it's worse than
> > my previous version because tcp_remove_reno_sacks then assumes
> > non-sensical L|S skbs (but there seem to be other, worse limitations in
> > sacked_out imitation after RTO because we've all skbs marked with L-bit so
> > it's not that big deal for me as most of the that imitation code is no-op
> > anyway after RTO).
> >
> >
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index 9a1b3c1..e11748d 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -1868,7 +1868,6 @@ static void tcp_remove_reno_sacks(struct sock *sk, int acked)
> >
> > if (acked > 0) {
> > /* One ACK acked hole. The rest eat duplicate ACKs. */
> > - tp->delivered += max_t(int, acked - tp->sacked_out, 1);
> > if (acked - 1 >= tp->sacked_out)
> > tp->sacked_out = 0;
> > else
> > @@ -1878,6 +1877,12 @@ static void tcp_remove_reno_sacks(struct sock *sk, int acked)
> > tcp_verify_left_out(tp);
> > }
> >
> > +static void tcp_update_reno_delivered(struct tcp_sock *tp, int acked)
> > +{
> > + if (acked > 0)
> > + tp->delivered += max_t(int, acked - tp->sacked_out, 1);
> > +}
> > +
> > static inline void tcp_reset_reno_sack(struct tcp_sock *tp)
> > {
> > tp->sacked_out = 0;
> > @@ -3027,6 +3032,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> > long seq_rtt_us = -1L;
> > long ca_rtt_us = -1L;
> > u32 pkts_acked = 0;
> > + u32 rexmit_acked = 0;
> > + u32 newdata_acked = 0;
> > u32 last_in_flight = 0;
> > bool rtt_update;
> > int flag = 0;
> > @@ -3056,8 +3063,10 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> > }
> >
> > if (unlikely(sacked & TCPCB_RETRANS)) {
> > - if (sacked & TCPCB_SACKED_RETRANS)
> > + if (sacked & TCPCB_SACKED_RETRANS) {
> > tp->retrans_out -= acked_pcount;
> > + rexmit_acked += acked_pcount;
> > + }
> > flag |= FLAG_RETRANS_DATA_ACKED;
> > } else if (!(sacked & TCPCB_SACKED_ACKED)) {
> > last_ackt = skb->skb_mstamp;
> > @@ -3070,6 +3079,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> > reord = start_seq;
> > if (!after(scb->end_seq, tp->high_seq))
> > flag |= FLAG_ORIG_SACK_ACKED;
> > + else
> > + newdata_acked += acked_pcount;
> > }
> >
> > if (sacked & TCPCB_SACKED_ACKED) {
> > @@ -3151,6 +3162,9 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> > }
> >
> > if (tcp_is_reno(tp)) {
> > + tcp_update_reno_delivered(tp, icsk->icsk_ca_state != TCP_CA_Loss ?
> > + pkts_acked :
> > + rexmit_acked + newdata_acked);
> > tcp_remove_reno_sacks(sk, pkts_acked);
> > } else {
> > int delta;
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox