linux.git/net/tipc, branch v4.3-rc4

tipc: reinitialize pointer after skb linearize

2015-09-21T05:31:20+00:00

The msg pointer into header may change after skb linearization.
We must reinitialize it after calling skb_linearize to prevent
operating on a freed or invalid pointer.

Signed-off-by: Erik Hugne 
Reported-by: Tamás Végh 
Acked-by: Ying Xue 
Signed-off-by: David S. Miller

net: tipc: fix stall during bclink wakeup procedure

2015-09-09T05:50:26+00:00

If an attempt to wake up users of broadcast link is made when there is
no enough place in send queue than it may hang up inside the
tipc_sk_rcv() function since the loop breaks only after the wake up
queue becomes empty. This can lead to complete CPU stall with the
following message generated by RCU:

INFO: rcu_sched self-detected stall on CPU { 0}  (t=2101 jiffies
					g=54225 c=54224 q=11465)
Task dump for CPU 0:
tpch            R  running task        0 39949  39948 0x0000000a
 ffffffff818536c0 ffff88181fa037a0 ffffffff8106a4be 0000000000000000
 ffffffff818536c0 ffff88181fa037c0 ffffffff8106d8a8 ffff88181fa03800
 0000000000000001 ffff88181fa037f0 ffffffff81094a50 ffff88181fa15680
Call Trace:
   [] sched_show_task+0xae/0x120
 [] dump_cpu_task+0x38/0x40
 [] rcu_dump_cpu_stacks+0x90/0xd0
 [] rcu_check_callbacks+0x3eb/0x6e0
 [] ? account_system_time+0x7f/0x170
 [] update_process_times+0x34/0x60
 [] tick_sched_handle.isra.18+0x31/0x40
 [] tick_sched_timer+0x3c/0x70
 [] __run_hrtimer.isra.34+0x3d/0xc0
 [] hrtimer_interrupt+0xc5/0x1e0
 [] ? native_smp_send_reschedule+0x42/0x60
 [] local_apic_timer_interrupt+0x34/0x60
 [] smp_apic_timer_interrupt+0x3c/0x60
 [] apic_timer_interrupt+0x6b/0x70
 [] ? _raw_spin_unlock_irqrestore+0x9/0x10
 [] __wake_up_sync_key+0x4f/0x60
 [] tipc_write_space+0x31/0x40 [tipc]
 [] filter_rcv+0x31f/0x520 [tipc]
 [] ? tipc_sk_lookup+0xc9/0x110 [tipc]
 [] ? _raw_spin_lock_bh+0x19/0x30
 [] tipc_sk_rcv+0x2dc/0x3e0 [tipc]
 [] tipc_bclink_wakeup_users+0x2f/0x40 [tipc]
 [] tipc_node_unlock+0x186/0x190 [tipc]
 [] ? kfree_skb+0x2c/0x40
 [] tipc_rcv+0x2ac/0x8c0 [tipc]
 [] tipc_l2_rcv_msg+0x38/0x50 [tipc]
 [] __netif_receive_skb_core+0x5a3/0x950
 [] __netif_receive_skb+0x13/0x60
 [] netif_receive_skb_internal+0x1e/0x90
 [] napi_gro_receive+0x78/0xa0
 [] tg3_poll_work+0xc54/0xf40 [tg3]
 [] ? consume_skb+0x2c/0x40
 [] tg3_poll_msix+0x41/0x160 [tg3]
 [] net_rx_action+0xe2/0x290
 [] __do_softirq+0xda/0x1f0
 [] irq_exit+0x76/0xa0
 [] do_IRQ+0x55/0xf0
 [] common_interrupt+0x6b/0x6b
 

The issue occurs only when tipc_sk_rcv() is used to wake up postponed
senders:

	tipc_bclink_wakeup_users()
		// wakeupq - is a queue which consists of special
		// 		 messages with SOCK_WAKEUP type.
		tipc_sk_rcv(wakeupq)
			...
			while (skb_queue_len(inputq)) {
				filter_rcv(skb)
					// Here the type of message is checked
					// and if it is SOCK_WAKEUP then
					// it tries to wake up a sender.
					tipc_write_space(sk)
						wake_up_interruptible_sync_poll()
			}

After the sender thread is woke up it can gather control and perform
an attempt to send a message. But if there is no enough place in send
queue it will call link_schedule_user() function which puts a message
of type SOCK_WAKEUP to the wakeup queue and put the sender to sleep.
Thus the size of the queue actually is not changed and the while()
loop never exits.

The approach I proposed is to wake up only senders for which there is
enough place in send queue so the described issue can't occur.
Moreover the same approach is already used to wake up senders on
unicast links.

I have got into the issue on our product code but to reproduce the
issue I changed a benchmark test application (from
tipcutils/demos/benchmark) to perform the following scenario:
	1. Run 64 instances of test application (nodes). It can be done
	   on the one physical machine.
	2. Each application connects to all other using TIPC sockets in
	   RDM mode.
	3. When setup is done all nodes start simultaneously send
	   broadcast messages.
	4. Everything hangs up.

The issue is reproducible only when a congestion on broadcast link
occurs. For example, when there are only 8 nodes it works fine since
congestion doesn't occur. Send queue limit is 40 in my case (I use a
critical importance level) and when 64 nodes send a message at the
same moment a congestion occurs every time.

Signed-off-by: Dmitry S Kolmakov 
Reviewed-by: Jon Maloy 
Acked-by: Ying Xue 
Signed-off-by: David S. Miller

tipc: fix stale link problem during synchronization

2015-08-23T23:14:45+00:00

Recent changes to the link synchronization means that we can now just
drop packets arriving on the synchronizing link before the synch point
is reached. This has lead to significant simplifications to the
implementation, but also turns out to have a flip side that we need
to consider.

Under unlucky circumstances, the two endpoints may end up
repeatedly dropping each other's packets, while immediately
asking for retransmission of the same packets, just to drop
them once more. This pattern will eventually be broken when
the synch point is reached on the other link, but before that,
the endpoints may have arrived at the retransmission limit
(stale counter) that indicates that the link should be broken.
We see this happen at rare occasions.

The fix for this is to not ask for retransmissions when a link is in
state LINK_SYNCHING. The fact that the link has reached this state
means that it has already received the first SYNCH packet, and that it
knows the synch point. Hence, it doesn't need any more packets until the
other link has reached the synch point, whereafter it can go ahead and
ask for the missing packets.

However, because of the reduced traffic on the synching link that
follows this change, it may now take longer to discover that the
synch point has been reached. We compensate for this by letting all
packets, on any of the links, trig a check for synchronization
termination. This is possible because the packets themselves don't
contain any information that is needed for discovering this condition.

Reviewed-by: Ying Xue 
Signed-off-by: Jon Maloy 
Signed-off-by: David S. Miller

tipc: interrupt link synchronization when a link goes down

2015-08-23T23:14:45+00:00

When we introduced the new link failover/synch mechanism
in commit 6e498158a827fd515b514842e9a06bdf0f75ab86
("tipc: move link synch and failover to link aggregation level"),
we missed the case when the non-tunnel link goes down during the link
synchronization period. In this case the tunnel link will remain in
state LINK_SYNCHING, something leading to unpredictable behavior when
the failover procedure is initiated.

In this commit, we ensure that the node and remaining link goes
back to regular communication state (SELF_UP_PEER_UP/LINK_ESTABLISHED)
when one of the parallel links goes down. We also ensure that we don't
re-enter synch mode if subsequent SYNCH packets arrive on the remaining
link.

Reviewed-by: Ying Xue 
Signed-off-by: Jon Maloy 
Signed-off-by: David S. Miller

tipc: eliminate risk of premature link setup during failover

2015-08-23T23:14:45+00:00

When a link goes down, and there is still a working link towards its
destination node, a failover is initiated, and the failed link is not
allowed to re-establish until that procedure is finished. To ensure
this, the concerned link endpoints are set to state LINK_FAILINGOVER,
and the node endpoints to NODE_FAILINGOVER during the failover period.

However, if the link reset is due to a disabled bearer, the corres-
ponding link endpoint is deleted, and only the node endpoint knows
about the ongoing failover. Now, if the disabled bearer is re-enabled
during the failover period, the discovery mechanism may create a new
link endpoint that is ready to be established, despite that this is not
permitted. This situation may cause both the ongoing failover and any
subsequent link synchronization to fail.

In this commit, we ensure that a newly created link goes directly to
state LINK_FAILINGOVER if the corresponding node state is
NODE_FAILINGOVER. This eliminates the problem described above.

Furthermore, we tighten the criteria for which packets are allowed
to end a failover state in the function tipc_node_check_state().
By checking that the receiving link is up and running, instead of just
checking that it is not in failover mode, we eliminate the risk that
protocol packets from the re-created link may cause the failover to
be prematurely terminated.

Reviewed-by: Ying Xue 
Signed-off-by: Jon Maloy 
Signed-off-by: David S. Miller

tipc: don't sanity check non-existing TLV (NL compat)

2015-08-17T17:39:54+00:00

A zero length payload means that no TLV (Type Length Value) data has
been passed. Prior to this patch a non-existing TLV could be sanity
checked with TLV_OK() resulting in random behavior where a user
sending an empty message occasionally got a incorrect "operation not
supported" message back.

Signed-off-by: Richard Alpe 
Reviewed-by: Erik Hugne 
Signed-off-by: David S. Miller

ipv6: change ipv6_stub_impl.ipv6_dst_lookup to take net argument

2015-07-31T22:21:30+00:00

This patch adds net argument to ipv6_stub_impl.ipv6_dst_lookup
for use cases where sk is not available (like mpls).
sk appears to be needed to get the namespace 'net' and is optional
otherwise. This patch series changes ipv6_stub_impl.ipv6_dst_lookup
to take net argument. sk remains optional.

All callers of ipv6_stub_impl.ipv6_dst_lookup have been modified
to pass net. I have modified them to use already available
'net' in the scope of the call. I can change them to
sock_net(sk) to avoid any unintended change in behaviour if sock
namespace is different. They dont seem to be from code inspection.

Signed-off-by: Roopa Prabhu 
Signed-off-by: David S. Miller

tipc: clean up link creation

2015-07-31T00:25:15+00:00

We simplify the link creation function tipc_link_create() and the way
the link struct it is connected to the node struct. In particular, we
remove the duplicate initialization of some fields which are anyway set
in tipc_link_reset().

Tested-by: Ying Xue 
Signed-off-by: Jon Maloy 
Signed-off-by: David S. Miller

tipc: use temporary, non-protected skb queue for bundle reception

2015-07-31T00:25:15+00:00

Currently, when we extract small messages from a message bundle, or
when many messages have accumulated in the link arrival queue, those
messages are added one by one to the lock protected link input queue.
This may increase contention with the reader of that queue, in
the function tipc_sk_rcv().

This commit introduces a temporary, unprotected input queue in
tipc_link_rcv() for such cases. Only when the arrival queue has been
emptied, and the function is ready to return, does it splice the whole
temporary queue into the real input queue.

Tested-by: Ying Xue 
Signed-off-by: Jon Maloy 
Signed-off-by: David S. Miller

tipc: remove implicit message delivery in node_unlock()

2015-07-31T00:25:14+00:00

After the most recent changes, all access calls to a link which
may entail addition of messages to the link's input queue are
postpended by an explicit call to tipc_sk_rcv(), using a reference
to the correct queue.

This means that the potentially hazardous implicit delivery, using
tipc_node_unlock() in combination with a binary flag and a cached
queue pointer, now has become redundant.

This commit removes this implicit delivery mechanism both for regular
data messages and for binding table update messages.

Tested-by: Ying Xue 
Signed-off-by: Jon Maloy 
Signed-off-by: David S. Miller