kernel.git/net/tipc/link.c, branch linux-5.3.y

tipc: fix unlimited bundling of small messages

2019-10-07T17:01:50Z

[ Upstream commit e95584a889e1902fdf1ded9712e2c3c3083baf96 ] We have identified a problem with the "oversubscription" policy in the link transmission code. When small messages are transmitted, and the sending link has reached the transmit window limit, those messages will be bundled and put into the link backlog queue. However, bundles of data messages are counted at the 'CRITICAL' level, so that the counter for that level, instead of the counter for the real, bundled message's level is the one being increased. Subsequent, to-be-bundled data messages at non-CRITICAL levels continue to be tested against the unchanged counter for their own level, while contributing to an unrestrained increase at the CRITICAL backlog level. This leaves a gap in congestion control algorithm for small messages that can result in starvation for other users or a "real" CRITICAL user. Even that eventually can lead to buffer exhaustion & link reset. We fix this by keeping a 'target_bskb' buffer pointer at each levels, then when bundling, we only bundle messages at the same importance level only. This way, we know exactly how many slots a certain level have occupied in the queue, so can manage level congestion accurately. By bundling messages at the same level, we even have more benefits. Let consider this: - One socket sends 64-byte messages at the 'CRITICAL' level; - Another sends 4096-byte messages at the 'LOW' level; When a 64-byte message comes and is bundled the first time, we put the overhead of message bundle to it (+ 40-byte header, data copy, etc.) for later use, but the next message can be a 4096-byte one that cannot be bundled to the previous one. This means the last bundle carries only one payload message which is totally inefficient, as for the receiver also! Later on, another 64-byte message comes, now we make a new bundle and the same story repeats... With the new bundling algorithm, this will not happen, the 64-byte messages will be bundled together even when the 4096-byte message(s) comes in between. However, if the 4096-byte messages are sent at the same level i.e. 'CRITICAL', the bundling algorithm will again cause the same overhead. Also, the same will happen even with only one socket sending small messages at a rate close to the link transmit's one, so that, when one message is bundled, it's transmitted shortly. Then, another message comes, a new bundle is created and so on... We will solve this issue radically by another patch. Fixes: 365ad353c256 ("tipc: reduce risk of user starvation during link congestion") Reported-by: Hoang Le Acked-by: Jon Maloy Signed-off-by: Tuong Lien Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman

tipc: fix false detection of retransmit failures

2019-08-16T23:27:13Z

This commit eliminates the use of the link 'stale_limit' & 'prev_from' (besides the already removed - 'stale_cnt') variables in the detection of repeated retransmit failures as there is no proper way to initialize them to avoid a false detection, i.e. it is not really a retransmission failure but due to a garbage values in the variables. Instead, a jiffies variable will be added to individual skbs (like the way we restrict the skb retransmissions) in order to mark the first skb retransmit time. Later on, at the next retransmissions, the timestamp will be checked to see if the skb in the link transmq is "too stale", that is, the link tolerance time has passed, so that a link reset will be ordered. Note, just checking on the first skb in the queue is fine enough since it must be the oldest one. A counter is also added to keep track the actual skb retransmissions' number for later checking when the failure happens. The downside of this approach is that the skb->cb[] buffer is about to be exhausted, however it is always able to allocate another memory area and keep a reference to it when needed. Fixes: 77cf8edbc0e7 ("tipc: simplify stale link failure criteria") Reported-by: Hoang Le Acked-by: Ying Xue Acked-by: Jon Maloy Signed-off-by: Tuong Lien Signed-off-by: David S. Miller

tipc: embed jiffies in macro TIPC_BC_RETR_LIM

2019-07-02T02:10:57Z

The macro TIPC_BC_RETR_LIM is always used in combination with 'jiffies', so we can just as well perform the addition in the macro itself. This way, we get a few shorter code lines and one less line break. Signed-off-by: Jon Maloy Acked-by: Ying Xue Signed-off-by: David S. Miller

tipc: rename function msg_get_wrapped() to msg_inner_hdr()

2019-06-25T20:42:54Z

We rename the inline function msg_get_wrapped() to the more comprehensible msg_inner_hdr(). Signed-off-by: Jon Maloy Signed-off-by: David S. Miller

tipc: eliminate unnecessary skb expansion during retransmission

2019-06-25T20:40:23Z

We increase the allocated headroom for the buffer copies to be retransmitted. This eliminates the need for the lower stack levels (UDP/IP/L2) to expand the headroom in order to add their own headers. Signed-off-by: Jon Maloy Signed-off-by: David S. Miller

tipc: simplify stale link failure criteria

2019-06-25T20:28:57Z

In commit a4dc70d46cf1 ("tipc: extend link reset criteria for stale packet retransmission") we made link retransmission failure events dependent on the link tolerance, and not only of the number of failed retransmission attempts, as we did earlier. This works well. However, keeping the original, additional criteria of 99 failed retransmissions is now redundant, and may in some cases lead to failure detection times in the order of minutes instead of the expected 1.5 sec link tolerance value. We now remove this criteria altogether. Acked-by: Ying Xue Signed-off-by: Jon Maloy Signed-off-by: David S. Miller

Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net

2019-06-22T12:59:24Z

Minor SPDX change conflict. Signed-off-by: David S. Miller

tipc: fix issues with early FAILOVER_MSG from peer

2019-06-18T17:03:37Z

It appears that a FAILOVER_MSG can come from peer even when the failure link is resetting (i.e. just after the 'node_write_unlock()'...). This means the failover procedure on the node has not been started yet. The situation is as follows: node1 node2 linkb linka linka linkb | | | | | | x failure | | | RESETTING | | | | | | x failure RESET | | RESETTING FAILINGOVER | | | (FAILOVER_MSG) | | |<-------------------------------------------------| | *FAILINGOVER | | | | | (dummy FAILOVER_MSG) | | |------------------------------------------------->| | RESET | | FAILOVER_END | FAILINGOVER RESET | . . . . . . . . . . . . Once this happens, the link failover procedure will be triggered wrongly on the receiving node since the node isn't in FAILINGOVER state but then another link failover will be carried out. The consequences are: 1) A peer might get stuck in FAILINGOVER state because the 'sync_point' was set, reset and set incorrectly, the criteria to end the failover would not be met, it could keep waiting for a message that has already received. 2) The early FAILOVER_MSG(s) could be queued in the link failover deferdq but would be purged or not pulled out because the 'drop_point' was not set correctly. 3) The early FAILOVER_MSG(s) could be dropped too. 4) The dummy FAILOVER_MSG could make the peer leaving FAILINGOVER state shortly, but later on it would be restarted. The same situation can also happen when the link is in PEER_RESET state and a FAILOVER_MSG arrives. The commit resolves the issues by forcing the link down immediately, so the failover procedure will be started normally (which is the same as when receiving a FAILOVER_MSG and the link is in up state). Also, the function "tipc_node_link_failover()" is toughen to avoid such a situation from happening. Acked-by: Jon Maloy Signed-off-by: Tuong Lien Signed-off-by: David S. Miller

tipc: include retrans failure detection for unicast

2019-06-17T20:27:32Z

In patch series, commit 9195948fbf34 ("tipc: improve TIPC throughput by Gap ACK blocks"), as for simplicity, the repeated retransmit failures' detection in the function - "tipc_link_retrans()" was kept there for broadcast retransmissions only. This commit now reapplies this feature for link unicast retransmissions that has been done via the function - "tipc_link_advance_transmq()". Also, the "tipc_link_retrans()" is renamed to "tipc_link_bc_retrans()" as it is used only for broadcast. Acked-by: Jon Maloy Signed-off-by: Tuong Lien Signed-off-by: David S. Miller

tipc: fix missing Name entries due to half-failover

2019-05-04T04:59:51Z

TIPC link can temporarily fall into "half-establish" that only one of the link endpoints is ESTABLISHED and starts to send traffic, PROTOCOL messages, whereas the other link endpoint is not up (e.g. immediately when the endpoint receives ACTIVATE_MSG, the network interface goes down...). This is a normal situation and will be settled because the link endpoint will be eventually brought down after the link tolerance time. However, the situation will become worse when the second link is established before the first link endpoint goes down, For example: 1. Both links <1A-2A>, <1B-2B> down 2. Link endpoint 2A up, but 1A still down (e.g. due to network disturbance, wrong session, etc.) 3. Link <1B-2B> up 4. Link endpoint 2A down (e.g. due to link tolerance timeout) 5. Node B starts failover onto link <1B-2B> ==> Node A does never start link failover. When the "half-failover" situation happens, two consequences have been observed: a) Peer link/node gets stuck in FAILINGOVER state; b) Traffic or user messages that peer node is trying to failover onto the second link can be partially or completely dropped by this node. The consequence a) was actually solved by commit c140eb166d68 ("tipc: fix failover problem"), but that commit didn't cover the b). It's due to the fact that the tunnel link endpoint has never been prepared for a failover, so the 'l->drop_point' (and the other data...) is not set correctly. When a TUNNEL_MSG from peer node arrives on the link, depending on the inner message's seqno and the current 'l->drop_point' value, the message can be dropped (- treated as a duplicate message) or processed. At this early stage, the traffic messages from peer are likely to be NAME_DISTRIBUTORs, this means some name table entries will be missed on the node forever! The commit resolves the issue by starting the FAILOVER process on this node as well. Another benefit from this solution is that we ensure the link will not be re-established until the failover ends. Acked-by: Jon Maloy Signed-off-by: Tuong Lien Signed-off-by: David S. Miller