Commit a34f8291 authored Jul 09, 2020 by Hamish Martin Committed by David S. Miller Jul 08, 2020

tipc: fix retransmission on unicast links



A scenario has been observed where a 'bc_init' message for a link is not
retransmitted if it fails to be received by the peer. This leads to the
peer never establishing the link fully and it discarding all other data
received on the link. In this scenario the message is lost in transit to
the peer.

The issue is traced to the 'nxt_retr' field of the skb not being
initialised for links that aren't a bc_sndlink. This leads to the
comparison in tipc_link_advance_transmq() that gates whether to attempt
retransmission of a message performing in an undesirable way.
Depending on the relative value of 'jiffies', this comparison:
    time_before(jiffies, TIPC_SKB_CB(skb)->nxt_retr)
may return true or false given that 'nxt_retr' remains at the
uninitialised value of 0 for non bc_sndlinks.

This is most noticeable shortly after boot when jiffies is initialised
to a high value (to flush out rollover bugs) and we compare a jiffies of,
say, 4294940189 to zero. In that case time_before returns 'true' leading
to the skb not being retransmitted.

The fix is to ensure that all skbs have a valid 'nxt_retr' time set for
them and this is achieved by refactoring the setting of this value into
a central function.
With this fix, transmission losses of 'bc_init' messages do not stall
the link establishment forever because the 'bc_init' message is
retransmitted and the link eventually establishes correctly.

Fixes: 382f598f ("tipc: reduce duplicate packets for unicast traffic")
Acked-by: Jon Maloy <jmaloy@redhat.com>
Signed-off-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz>
Signed-off-by: David S. Miller <davem@davemloft.net>

parent 27d53323

net/tipc/link.c

+18 −8

Original line number	Diff line number	Diff line
		@@ -921,6 +921,21 @@ static void link_prepare_wakeup(struct tipc_link *l)

		}

		/**
		* tipc_link_set_skb_retransmit_time - set the time at which retransmission of
		* the given skb should be next attempted
		* @skb: skb to set a future retransmission time for
		* @l: link the skb will be transmitted on
		*/
		static void tipc_link_set_skb_retransmit_time(struct sk_buff *skb,
		struct tipc_link *l)
		{
		if (link_is_bc_sndlink(l))
		TIPC_SKB_CB(skb)->nxt_retr = TIPC_BC_RETR_LIM;
		else
		TIPC_SKB_CB(skb)->nxt_retr = TIPC_UC_RETR_TIME;
		}

		void tipc_link_reset(struct tipc_link *l)
		{
		struct sk_buff_head list;
		@@ -1036,9 +1051,7 @@ int tipc_link_xmit(struct tipc_link l, struct sk_buff_head list,
		return -ENOBUFS;
		}
		__skb_queue_tail(transmq, skb);
		/* next retransmit attempt */
		if (link_is_bc_sndlink(l))
		TIPC_SKB_CB(skb)->nxt_retr = TIPC_BC_RETR_LIM;
		tipc_link_set_skb_retransmit_time(skb, l);
		__skb_queue_tail(xmitq, _skb);
		TIPC_SKB_CB(skb)->ackers = l->ackers;
		l->rcv_unacked = 0;
		@@ -1139,9 +1152,7 @@ static void tipc_link_advance_backlog(struct tipc_link *l,
		if (unlikely(skb == l->backlog[imp].target_bskb))
		l->backlog[imp].target_bskb = NULL;
		__skb_queue_tail(&l->transmq, skb);
		/* next retransmit attempt */
		if (link_is_bc_sndlink(l))
		TIPC_SKB_CB(skb)->nxt_retr = TIPC_BC_RETR_LIM;
		tipc_link_set_skb_retransmit_time(skb, l);

		__skb_queue_tail(xmitq, _skb);
		TIPC_SKB_CB(skb)->ackers = l->ackers;
		@@ -1584,8 +1595,7 @@ release:
		/* retransmit skb if unrestricted*/
		if (time_before(jiffies, TIPC_SKB_CB(skb)->nxt_retr))
		continue;
		TIPC_SKB_CB(skb)->nxt_retr = (is_uc) ?
		TIPC_UC_RETR_TIME : TIPC_BC_RETR_LIM;
		tipc_link_set_skb_retransmit_time(skb, l);
		_skb = pskb_copy(skb, GFP_ATOMIC);
		if (!_skb)
		continue;

Admin message