[tipc-discussion] [net-next v3 0/3] tipc: improve interaction socket-link

[Jon Maloy]

We fix a very real starvation problem that may occur when a link
encounters send buffer congestion. At the same time we make the 
interaction between the socket and link layer simpler and more 
consistent.

v2: - Simplified link congestion check to only check against own
  importance limit. This reduces the risk of higher levels
  starving out lower levels.
v3: - Adding one sent message to to link backlog queue even if there is
  congestion, as suggested by Partha.
- Allowing link_wakeup() loop to continue adding messages to the
  backlog queue even if one or more levels are congested. This
  seems to have a positive effect on performance.

Jon Maloy (3):
  tipc: unify tipc_wait_for_sndpkt() and tipc_wait_for_sndmsg()
functions
  tipc: modify struct tipc_plist to be more versatile
  tipc: reduce risk of user starvation during link congestion

 net/tipc/bcast.c  |   2 +-
 net/tipc/link.c   |  66 
 net/tipc/msg.h|   2 -
 net/tipc/name_table.c | 100 +++
 net/tipc/name_table.h |  21 +--
 net/tipc/node.c   |   2 +-
 net/tipc/socket.c | 448 ++
 7 files changed, 309 insertions(+), 332 deletions(-)

-- 
2.7.4


--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion

[tipc-discussion] [net-next v3 3/3] tipc: reduce risk of user starvation during link congestion

[Jon Maloy]

The socket code currently handles link congestion by either blocking
and trying to send again when the congestion has abated, or just
returning to the user with -EAGAIN and let him re-try later.

This mechanism is prone to starvation, because the wakeup algorithm is
non-atomic. During the time the link issues a wakeup signal, until the
socket wakes up and re-attempts sending, other senders may have come
in between and occupied the free buffer space in the link. This in turn
may lead to a socket having to make many send attempts before it is
successful. In extremely loaded systems we have observed latency times
of several seconds before a low-priority socket is able to send out a
message.

In this commit, we simplify this mechanism and reduce the risk of the
described scenario happening. When a message is attempted sent via a
congested link, we now let it be added to the link's backlog queue
anyway, thus permitting an oversubscription of one message per source
socket. We still create a wakeup item and return an error code, hence
instructing the sender to block or stop sending. Only when enough space
has been freed up in the link's backlog queue do we issue a wakeup event
that allows the sender to continue with the next message, if any.

The fact that a socket now can consider a message sent even when the
link returns a congestion code means that the sending socket code can
be simplified. Also, since this is a good opportunity to get rid of the
obsolete 'mtu change' condition in the three socket send functions, we
now choose to refactor those functions completely.

Signed-off-by: Jon Maloy 
---
 net/tipc/bcast.c  |   2 +-
 net/tipc/link.c   |  66 +--
 net/tipc/msg.h|   2 -
 net/tipc/node.c   |   2 +-
 net/tipc/socket.c | 346 --
 5 files changed, 184 insertions(+), 234 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index aa1babb..1a56cab 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -174,7 +174,7 @@ static void tipc_bcbase_xmit(struct net *net, struct 
sk_buff_head *xmitq)
  *and to identified node local sockets
  * @net: the applicable net namespace
  * @list: chain of buffers containing message
- * Consumes the buffer chain, except when returning -ELINKCONG
+ * Consumes the buffer chain.
  * Returns 0 if success, otherwise errno: -ELINKCONG,-EHOSTUNREACH,-EMSGSIZE
  */
 int tipc_bcast_xmit(struct net *net, struct sk_buff_head *list)
diff --git a/net/tipc/link.c b/net/tipc/link.c
index bda89bf..5f2b478 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -776,60 +776,55 @@ int tipc_link_timeout(struct tipc_link *l, struct 
sk_buff_head *xmitq)
 
 /**
  * link_schedule_user - schedule a message sender for wakeup after congestion
- * @link: congested link
- * @list: message that was attempted sent
+ * @l: congested link
+ * @hdr: header of message that is being sent
  * Create pseudo msg to send back to user when congestion abates
- * Does not consume buffer list
+ * Consumes buffer list
  */
-static int link_schedule_user(struct tipc_link *link, struct sk_buff_head 
*list)
+static int link_schedule_user(struct tipc_link *l, struct tipc_msg *hdr)
 {
-   struct tipc_msg *msg = buf_msg(skb_peek(list));
-   int imp = msg_importance(msg);
-   u32 oport = msg_origport(msg);
-   u32 addr = tipc_own_addr(link->net);
+   int imp = msg_importance(hdr);
+   u32 dnode = tipc_own_addr(l->net);
+   u32 dport = msg_origport(hdr);
struct sk_buff *skb;
 
/* This really cannot happen...  */
if (unlikely(imp > TIPC_CRITICAL_IMPORTANCE)) {
-   pr_warn("%s<%s>, send queue full", link_rst_msg, link->name);
+   pr_warn("%s<%s>, send queue full", link_rst_msg, l->name);
return -ENOBUFS;
}
-   /* Non-blocking sender: */
-   if (TIPC_SKB_CB(skb_peek(list))->wakeup_pending)
-   return -ELINKCONG;
 
/* Create and schedule wakeup pseudo message */
skb = tipc_msg_create(SOCK_WAKEUP, 0, INT_H_SIZE, 0,
- addr, addr, oport, 0, 0);
+ dnode, l->addr, dport, 0, 0);
if (!skb)
return -ENOBUFS;
-   TIPC_SKB_CB(skb)->chain_sz = skb_queue_len(list);
+   msg_set_dest_droppable(buf_msg(skb), true);
TIPC_SKB_CB(skb)->chain_imp = imp;
-   skb_queue_tail(>wakeupq, skb);
-   link->stats.link_congs++;
+   skb_queue_tail(>wakeupq, skb);
+   l->stats.link_congs++;
return -ELINKCONG;
 }
 
 /**
  * link_prepare_wakeup - prepare users for wakeup after congestion
- * @link: congested link
- * Move a number of waiting users, as permitted by available space in
- * the send queue, from link wait queue to node wait queue for wakeup
+ * @l: congested link
+ * Wake up a number of waiting users, as permitted by available space
+ * in the send queue
  */
 void 

[tipc-discussion] [net-next v3 1/3] tipc: unify tipc_wait_for_sndpkt() and tipc_wait_for_sndmsg() functions

[Jon Maloy]

The functions tipc_wait_for_sndpkt() and tipc_wait_for_sndmsg() are very
similar. The latter function is also called from two locations, and
there will be more in the coming commits, which will all need to test on
different conditions.

Instead of making yet another duplicates of the function, we now
introduce a new macro tipc_wait_for_cond() where the wakeup condition
can be stated as an argument to the call. This macro replaces all
current and future uses of the two functions, which can now be
eliminated.

Signed-off-by: Jon Maloy 
---
 net/tipc/socket.c | 108 +-
 1 file changed, 49 insertions(+), 59 deletions(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 333c5da..8f3ab08 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -110,7 +110,6 @@ static void tipc_write_space(struct sock *sk);
 static void tipc_sock_destruct(struct sock *sk);
 static int tipc_release(struct socket *sock);
 static int tipc_accept(struct socket *sock, struct socket *new_sock, int 
flags);
-static int tipc_wait_for_sndmsg(struct socket *sock, long *timeo_p);
 static void tipc_sk_timeout(unsigned long data);
 static int tipc_sk_publish(struct tipc_sock *tsk, uint scope,
   struct tipc_name_seq const *seq);
@@ -334,6 +333,49 @@ static int tipc_set_sk_state(struct sock *sk, int state)
return res;
 }
 
+static int tipc_sk_sock_err(struct socket *sock, long *timeout)
+{
+   struct sock *sk = sock->sk;
+   int err = sock_error(sk);
+   int typ = sock->type;
+
+   if (err)
+   return err;
+   if (typ == SOCK_STREAM || typ == SOCK_SEQPACKET) {
+   if (sk->sk_state == TIPC_DISCONNECTING)
+   return -EPIPE;
+   else if (!tipc_sk_connected(sk))
+   return -ENOTCONN;
+   }
+   if (!*timeout)
+   return -EAGAIN;
+   if (signal_pending(current))
+   return sock_intr_errno(*timeout);
+
+   return 0;
+}
+
+#define tipc_wait_for_cond(sock_, timeout_, condition_)
\
+({ \
+   int rc_ = 0;\
+   int done_ = 0;  \
+   \
+   while (!(condition_) && !done_) {   \
+   struct sock *sk_ = sock->sk;\
+   DEFINE_WAIT_FUNC(wait_, woken_wake_function);   \
+   \
+   rc_ = tipc_sk_sock_err(sock_, timeout_);\
+   if (rc_)\
+   break;  \
+   prepare_to_wait(sk_sleep(sk_), _,  \
+   TASK_INTERRUPTIBLE);\
+   done_ = sk_wait_event(sk_, timeout_,\
+ (condition_), _);\
+   remove_wait_queue(sk_sleep(sk), _);\
+   }   \
+   rc_;\
+})
+
 /**
  * tipc_sk_create - create a TIPC socket
  * @net: network namespace (must be default network)
@@ -719,7 +761,7 @@ static int tipc_sendmcast(struct  socket *sock, struct 
tipc_name_seq *seq,
 
if (rc == -ELINKCONG) {
tsk->link_cong = 1;
-   rc = tipc_wait_for_sndmsg(sock, );
+   rc = tipc_wait_for_cond(sock, , !tsk->link_cong);
if (!rc)
continue;
}
@@ -828,31 +870,6 @@ static void tipc_sk_proto_rcv(struct tipc_sock *tsk, 
struct sk_buff *skb,
kfree_skb(skb);
 }
 
-static int tipc_wait_for_sndmsg(struct socket *sock, long *timeo_p)
-{
-   DEFINE_WAIT_FUNC(wait, woken_wake_function);
-   struct sock *sk = sock->sk;
-   struct tipc_sock *tsk = tipc_sk(sk);
-   int done;
-
-   do {
-   int err = sock_error(sk);
-   if (err)
-   return err;
-   if (sk->sk_shutdown & SEND_SHUTDOWN)
-   return -EPIPE;
-   if (!*timeo_p)
-   return -EAGAIN;
-   if (signal_pending(current))
-   return sock_intr_errno(*timeo_p);
-
-   add_wait_queue(sk_sleep(sk), );
-   done = sk_wait_event(sk, timeo_p, !tsk->link_cong, );
-   remove_wait_queue(sk_sleep(sk), );
-   } while (!done);
-   return 0;
-}
-
 /**
  * tipc_sendmsg - send message in connectionless manner
  * 

Re: [tipc-discussion] [PATCH net v2 2/2] tipc: fix nametbl_lock soft lockup at module exit

[John Thompson]

Hi Ying,

I tested with the 3 patches applied:
1/3: tipc: fix nametbl_lock soft lockup at node/link events
2/3: tipc: fix nametbl_lock soft lockup at module exit
3/3: tipc: move connection cleanup to a workqueue

In my case the soft lockup I was seeing was resolved by patch 3 ("tipc:
move connection cleanup to a workqueue").

I have not tested without patch 2 applied.

Regards,
John



On Tue, Dec 13, 2016 at 12:37 AM, Ying Xue  wrote:

> Hi Parth,
>
> Sorry for late response.
>
> As I could not find your v3 version, I just give comments based on the
> version.
>
> On 11/22/2016 12:27 AM, Parthasarathy Bhuvaragan wrote:
>
>> Commit 333f796235a527 ("tipc: fix a race condition leading to
>> subscriber refcnt bug") reveals a soft lockup while acquiring
>> nametbl_lock.
>>
>> Before commit 333f796235a527, we call tipc_conn_shutdown() from
>> tipc_close_conn() in the context of tipc_topsrv_stop(). In that
>> context, we are allowed to grab the nametbl_lock.
>>
>> In commit 333f796235a527, i moved the tipc_conn_release (renamed from
>> tipc_conn_shutdown) to the connection refcount cleanup.
>>
>
> Can you please confirm whether the soft lockup doesn't happen any more if
> we don't adjust the sequence of tipc_conn_release?
>
> If yes, I think we can propose other method to fix the issue solved by
> commit 333f796235a527, but in the new method it's unnecessary to adjust the
> order of tipc_conn_release.
>
> In fact the order of tipc_conn_shutdown, tipc_unregister_callbacks and
> tipc_conn_release is very important. When I wrote that code, I spent much
> time considering how to carefully close connection.
>
> In my opinion, the ideal order is still as belows:
>
> 1, Close connection;
> 2. Call tipc_unregister_callbacks to let sk->sk_user_data. As long as
> sk->sk_user_data is 0, no more data will be submitted to
> con->rwork/on->swork works.
> 3. Release socket.
>
> Regards,
> Ying
>
>
>  This allows
>
>> either tipc_nametbl_withdraw() or tipc_topsrv_stop() to perform
>> tipc_sock_release().
>>
>> Since tipc_exit_net() first calls tipc_topsrv_stop() and then
>> tipc_nametble_withdraw() increases the chances for the later to
>> perform the connection cleanup.
>>
>> The soft lockup occurs in the call chain of tipc_nametbl_withdraw(),
>> when it performs the tipc_conn_kref_release() as it tries to grab
>> nametbl_lock again while holding it already.
>> tipc_nametbl_withdraw() grabs nametbl_lock
>>   tipc_nametbl_remove_publ()
>> tipc_subscrp_report_overlap()
>>   tipc_subscrp_send_event()
>> tipc_conn_sendmsg()
>>   << if (con->flags != CF_CONNECTED) we do conn_put(),
>>  triggering the cleanup as refcount=0. >>
>>   tipc_conn_kref_release
>> tipc_sock_release
>>   tipc_conn_release
>> tipc_subscrb_delete
>>   tipc_subscrp_delete
>> tipc_nametbl_unsubscribe << Soft Lockup >>
>>
>> Until now, tipc_server_stop() grabs and releases the idr_lock twice
>> for every connection. Once to find the connection and second to unset
>> connection flag, remove it from list and decrement the refcount.
>> The above lockup occurs when tipc_nametbl_withdraw() grabs the
>> connection in between the two, thus owning the connection
>> and triggering the cleanup while decrementing the refcount later.
>>
>> In this commit, we perform:
>> - call tipc_nametbl_withdraw() before tipc_topsrv_stop() to avoid:
>>   1. soft lockup
>>   2. its too late to actually notify the subscribers, as the topology
>>  server might already have started shutting down.
>> - In tipc_server_stop(), we remove all the connections from connection
>> list in the scope of the idr_lock to prevent any other thread finding
>> a connection which has unset CF_CONNECTED in its flags.
>>
>> Fixes: 333f796235a52727 ("tipc: fix a race condition leading to
>> subscriber refcnt bug")
>> Signed-off-by: Parthasarathy Bhuvaragan > sson.com>
>> ---
>> v2: commit message update.
>> cleanup tipc_server_stop() as per Ying Xue.
>> ---
>>  net/tipc/core.c   |  4 
>>  net/tipc/net.c|  2 --
>>  net/tipc/server.c | 13 -
>>  3 files changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/net/tipc/core.c b/net/tipc/core.c
>> index 236b043a4156..3ea1452e2f06 100644
>> --- a/net/tipc/core.c
>> +++ b/net/tipc/core.c
>> @@ -93,6 +93,10 @@ static int __net_init tipc_init_net(struct net *net)
>>
>>  static void __net_exit tipc_exit_net(struct net *net)
>>  {
>> +   struct tipc_net *tn = net_generic(net, tipc_net_id);
>> +
>> +   tipc_nametbl_withdraw(net, TIPC_CFG_SRV, tn->own_addr, 0,
>> + tn->own_addr);
>> tipc_topsrv_stop(net);
>> tipc_net_stop(net);
>> tipc_bcast_stop(net);
>> diff --git a/net/tipc/net.c b/net/tipc/net.c
>> index 28bf4feeb81c..92696cc6e763 100644
>> --- a/net/tipc/net.c
>> +++ b/net/tipc/net.c
>> @@ -130,8 +130,6 @@ 

Re: [tipc-discussion] [PATCH net v2 2/2] tipc: fix nametbl_lock soft lockup at module exit

[Ying Xue]

Hi Parth,

Sorry for late response.

As I could not find your v3 version, I just give comments based on the 
version.

On 11/22/2016 12:27 AM, Parthasarathy Bhuvaragan wrote:
> Commit 333f796235a527 ("tipc: fix a race condition leading to
> subscriber refcnt bug") reveals a soft lockup while acquiring
> nametbl_lock.
>
> Before commit 333f796235a527, we call tipc_conn_shutdown() from
> tipc_close_conn() in the context of tipc_topsrv_stop(). In that
> context, we are allowed to grab the nametbl_lock.
>
> In commit 333f796235a527, i moved the tipc_conn_release (renamed from
> tipc_conn_shutdown) to the connection refcount cleanup.

Can you please confirm whether the soft lockup doesn't happen any more 
if we don't adjust the sequence of tipc_conn_release?

If yes, I think we can propose other method to fix the issue solved by 
commit 333f796235a527, but in the new method it's unnecessary to adjust 
the order of tipc_conn_release.

In fact the order of tipc_conn_shutdown, tipc_unregister_callbacks and 
tipc_conn_release is very important. When I wrote that code, I spent 
much time considering how to carefully close connection.

In my opinion, the ideal order is still as belows:

1, Close connection;
2. Call tipc_unregister_callbacks to let sk->sk_user_data. As long as 
sk->sk_user_data is 0, no more data will be submitted to 
con->rwork/on->swork works.
3. Release socket.

Regards,
Ying

  This allows
> either tipc_nametbl_withdraw() or tipc_topsrv_stop() to perform
> tipc_sock_release().
>
> Since tipc_exit_net() first calls tipc_topsrv_stop() and then
> tipc_nametble_withdraw() increases the chances for the later to
> perform the connection cleanup.
>
> The soft lockup occurs in the call chain of tipc_nametbl_withdraw(),
> when it performs the tipc_conn_kref_release() as it tries to grab
> nametbl_lock again while holding it already.
> tipc_nametbl_withdraw() grabs nametbl_lock
>   tipc_nametbl_remove_publ()
> tipc_subscrp_report_overlap()
>   tipc_subscrp_send_event()
> tipc_conn_sendmsg()
>   << if (con->flags != CF_CONNECTED) we do conn_put(),
>  triggering the cleanup as refcount=0. >>
>   tipc_conn_kref_release
> tipc_sock_release
>   tipc_conn_release
> tipc_subscrb_delete
>   tipc_subscrp_delete
> tipc_nametbl_unsubscribe << Soft Lockup >>
>
> Until now, tipc_server_stop() grabs and releases the idr_lock twice
> for every connection. Once to find the connection and second to unset
> connection flag, remove it from list and decrement the refcount.
> The above lockup occurs when tipc_nametbl_withdraw() grabs the
> connection in between the two, thus owning the connection
> and triggering the cleanup while decrementing the refcount later.
>
> In this commit, we perform:
> - call tipc_nametbl_withdraw() before tipc_topsrv_stop() to avoid:
>   1. soft lockup
>   2. its too late to actually notify the subscribers, as the topology
>  server might already have started shutting down.
> - In tipc_server_stop(), we remove all the connections from connection
> list in the scope of the idr_lock to prevent any other thread finding
> a connection which has unset CF_CONNECTED in its flags.
>
> Fixes: 333f796235a52727 ("tipc: fix a race condition leading to subscriber 
> refcnt bug")
> Signed-off-by: Parthasarathy Bhuvaragan 
> 
> ---
> v2: commit message update.
> cleanup tipc_server_stop() as per Ying Xue.
> ---
>  net/tipc/core.c   |  4 
>  net/tipc/net.c|  2 --
>  net/tipc/server.c | 13 -
>  3 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/net/tipc/core.c b/net/tipc/core.c
> index 236b043a4156..3ea1452e2f06 100644
> --- a/net/tipc/core.c
> +++ b/net/tipc/core.c
> @@ -93,6 +93,10 @@ static int __net_init tipc_init_net(struct net *net)
>
>  static void __net_exit tipc_exit_net(struct net *net)
>  {
> + struct tipc_net *tn = net_generic(net, tipc_net_id);
> +
> + tipc_nametbl_withdraw(net, TIPC_CFG_SRV, tn->own_addr, 0,
> +   tn->own_addr);
>   tipc_topsrv_stop(net);
>   tipc_net_stop(net);
>   tipc_bcast_stop(net);
> diff --git a/net/tipc/net.c b/net/tipc/net.c
> index 28bf4feeb81c..92696cc6e763 100644
> --- a/net/tipc/net.c
> +++ b/net/tipc/net.c
> @@ -130,8 +130,6 @@ void tipc_net_stop(struct net *net)
>   if (!tn->own_addr)
>   return;
>
> - tipc_nametbl_withdraw(net, TIPC_CFG_SRV, tn->own_addr, 0,
> -   tn->own_addr);
>   rtnl_lock();
>   tipc_bearer_stop(net);
>   tipc_node_stop(net);
> diff --git a/net/tipc/server.c b/net/tipc/server.c
> index 215849ce453d..54c23ecccd26 100644
> --- a/net/tipc/server.c
> +++ b/net/tipc/server.c
> @@ -623,16 +623,19 @@ int tipc_server_start(struct tipc_server *s)
>  void tipc_server_stop(struct tipc_server *s)
>  {
>   struct tipc_conn *con;
> - int total = 0;
>   

Re: [tipc-discussion] [PATCH 3.10 00/17] patches for xiang in 3.10

[Ying Xue]

On 12/12/2016 03:53 PM, XIANG Haiming wrote:
> Hi Ying,
>
> I try to convert your email([PATCH 3.10 04/17] tipc: don't use memcpy to copy 
> from user space)  to patch file 3.patch.
>
> I use command " patch -p0 < /root/3.patch" , there is the follow error:
>
> patching file net/tipc/msg.c
> Hunk #1 succeeded at 76 with fuzz 2.
> Hunk #2 FAILED at 92.
> 1 out of 2 hunks FAILED -- saving rejects to file net/tipc/msg.c.rej
>
> Do you know why the secondary change cannot be apply?

The good news is that your patch format is recognized by "patch" 
command, but the bad news is that you met one conflict between your 
patch and your Linux kernel source code, which means that you may still 
lack some necessary patches of TIPC. Of course, you can understand what 
conflict you encountered by analyzing net/tipc/msg.c.rej. Meanwhile, I 
strongly suggest you don't you "patch" to apply patches into your Linux 
source code because it's hard for us to revert a patch applied by 
"patch" command.

As I have no your environment on hand, it's hard for me to help you.

>
>
> -Original Message-
> From: Ying Xue [mailto:ying@windriver.com]
> Sent: 2016年12月9日 18:01
> To: XIANG Haiming; Jon Maloy; tipc-discussion@lists.sourceforge.net; 
> parthasarathy.bhuvara...@ericsson.com
> Cc: ma...@donjonn.com
> Subject: Re: [PATCH 3.10 00/17] patches for xiang in 3.10
>
> On 12/09/2016 03:30 PM, XIANG Haiming wrote:
>> Hi Ying,
>>
>> I have two question about these patches:
>>
>> 1. There is no " net/tipc/server.c" file in our Kernel
>> 3.10.0-327.18.2.el7.x86_64
>
> It's a bit strange. server.c was merged since v3.10-rc4-858-gc5fa7b3.
>
>>
>> 2. I have saved 17 patches email to xxx.msg(from 1.msg to 17.msg)
>> file. And then copy these msg file to one directory such as tipc-patch And 
>> then I use command "git am tipc-patch", But there is no file changed.
>>
>> If I use command "git am 1.msg", there is error " Patch format detection 
>> failed."
>
> Yes, this command is right. I often use the command to apply patch of email 
> format into kernel.
>
> Probably you use outlook email client. In fact I use Thunderbird, and this 
> way works fine with me.
>
>>
>> If I use command "git am 1.txt" (I save email to 1.txt), there is error " 
>> Patch format detection failed."
>>
>> Maybe there is some misunderstanding, please tell me the correct patch 
>> method. Thank you.
>>
>>
>>
>> -Original Message-
>> From: Ying Xue [mailto:ying@windriver.com]
>> Sent: 2016年12月7日 19:28
>> To: XIANG Haiming; Jon Maloy; tipc-discussion@lists.sourceforge.net;
>> parthasarathy.bhuvara...@ericsson.com
>> Cc: ma...@donjonn.com
>> Subject: Re: [PATCH 3.10 00/17] patches for xiang in 3.10
>>
>> Hi Xiang,
>>
>> You can save all patches in your email inbox as files one by one. When you 
>> enter the folder of Linux kernel source, please run the command below:
>>
>> git am patch file
>>
>> The patch files is just saved through email.
>>
>> Once all 17 patches are applied into kernel source tree, you then can
>> compile your kernel as well as TIPC module,
>>
>> Regards,
>> Ying
>>
>> On 12/07/2016 11:33 AM, XIANG Haiming wrote:
>>> Hi Jon,
>>>
>>> How to get these patches and how to patch these patches to our kernel 3.10?
>>>
>>>
>>> -Original Message-
>>> From: Jon Maloy [mailto:jon.ma...@ericsson.com]
>>> Sent: 2016年12月6日 19:22
>>> To: tipc-discussion@lists.sourceforge.net;
>>> parthasarathy.bhuvara...@ericsson.com; ying@windriver.com;
>>> jon.ma...@ericsson.com
>>> Cc: ma...@donjonn.com; XIANG Haiming
>>> Subject: [PATCH 3.10 00/17] patches for xiang in 3.10
>>>
>>> See subject.
>>> ///jon
>>>
>>> Erik Hugne (6):
>>>   tipc: set sk_err correctly when connection fails
>>>   tipc: simplify the link lookup routine
>>>   tipc: don't reroute message fragments
>>>   tipc: message reassembly using fragment chain
>>>   tipc: reassembly failures should cause link reset
>>>   tipc: remove interface state mirroring in bearer
>>>
>>> Joe Perches (1):
>>>   net: misc: Remove extern from function prototypes
>>>
>>> Ying Xue (9):
>>>   tipc: fix oops when creating server socket fails
>>>   tipc: don't use memcpy to copy from user space
>>>   tipc: remove iovec length parameter from all sending functions
>>>   tipc: silence sparse warnings
>>>   tipc: make bearer and media naming consistent
>>>   tipc: avoid unnecessary lookup for tipc bearer instance
>>>   tipc: correct return value of recv_msg routine
>>>   tipc: correct return value of link_cmd_set_value routine
>>>   tipc: remove two indentation levels in tipc_recv_msg routine
>>>
>>> dingtianhong (1):
>>>   tipc: avoid possible deadlock while enable and disable bearer
>>>
>>>  net/irda/irnet/irnet.h   |  15 +-
>>>  net/l2tp/l2tp_core.h |  57 +++--
>>>  net/mac80211/rate.h  |  12 +-
>>>  net/netfilter/nf_internals.h |  28 +--
>>>  net/rds/rds.h|   2 +-
>>>  net/rxrpc/ar-internal.h  | 150 ++--
>>>  net/tipc/bcast.c |  22 +-
>>>  

Re: [tipc-discussion] reproducible link failure scenario

[Butler, Peter]

Thanks - just testing it out now and so far all looks good.


From: Parthasarathy Bhuvaragan 
Sent: Monday, December 12, 2016 3:20:51 AM
To: tipc-discussion@lists.sourceforge.net
Subject: Re: [tipc-discussion] reproducible link failure scenario

On 12/09/2016 09:25 PM, Butler, Peter wrote:
> We can certainly do that for future upgrades of our customers.  However we 
> may need to just patch in the interim.
>
>
> Is the patch small enough (self-contained enough) that it would be easy 
> enough for me to port it into our 4.4.0 kernel?  Or does it make use of many 
> kernel constructs that have changed between 4.4 and 4.8?
>
Yes, the commit below should apply cleanly on 4.4.0.
https://www.spinics.net/lists/netdev/msg392988.html

/Partha
> 
> From: Jon Maloy 
> Sent: Friday, December 9, 2016 1:57:46 PM
> To: Butler, Peter; tipc-discussion@lists.sourceforge.net
> Subject: RE: reproducible link failure scenario
>
> Hi Peter,
> This is a known bug, fixed in commit d2f394dc4816 ("tipc: fix random link 
> resets while adding a second bearer") from Partha Bhuvaragan, and present in 
> Linux 4.8.
> Do you have any possibility to upgrade your kernel?
>
> BR
> ///jon
>
>> -Original Message-
>> From: Butler, Peter [mailto:pbut...@sonusnet.com]
>> Sent: Friday, 09 December, 2016 09:31
>> To: tipc-discussion@lists.sourceforge.net
>> Subject: [tipc-discussion] reproducible link failure scenario
>>
>> I have a reproducible failure scenario that results in the following kernel
>> messages being printed in succession (along with the associated link 
>> failing):
>>
>> Dec  8 12:10:33 [SEQ 617259] lab236slot6 kernel:  [44856.752261] 
>> Retransmission
>> failure on link <1.1.6:p19p1-1.1.8:p19p1>
>> Dec  8 12:10:33 [SEQ 617260] lab236slot6 kernel:  [44856.758633] Resetting 
>> link
>> Link <1.1.6:p19p1-1.1.8:p19p1> state e
>> Dec  8 12:10:33 [SEQ 617261] lab236slot6 kernel:  [44856.758635] XMTQ: 3 
>> [2-4],
>> BKLGQ: 0, SNDNX: 5, RCVNX: 4
>> Dec  8 12:10:33 [SEQ 617262] lab236slot6 kernel:  [44856.758637] Failed msg: 
>> usr
>> 10, typ 0, len 1540, err 0
>> Dec  8 12:10:33 [SEQ 617263] lab236slot6 kernel:  [44856.758638] sqno 2, 
>> prev:
>> 1001006, src: 1001006
>>
>> The issue occurs within 30 seconds after any node in the cluster is rebooted.
>> There are two 10Gb Ethernet fabrics in the cluster, so every node has two 
>> links to
>> every other node.  When the failure occurs, it is only ever one of the two 
>> links
>> that fails (although it appears to be random which of the two that will be 
>> on a
>> boot-to-boot basis).
>>
>> Important: links only fail to a common node in the mesh.  While all nodes in 
>> the
>> mesh are running the same kernel (Linux 4.4.0), the common node is the only
>> one that is also running DRBD.  Actually, there are two nodes running DRBD, 
>> but
>> at any given time only one of the two nodes is the 'active' DRBD manager, so 
>> to
>> speak, as they use a shared IP for the DRBD functionality, much akin the 
>> HA-Linux
>> heartbeat.  Again, the failure only ever occurs on TIPC links to the active 
>> DRBD
>> node, as the other one is invisible (insofar as DRBD is concerned) as a 
>> stand-by.
>>
>> So it would appear (on the surface at least) that there is some conflict 
>> between
>> running DRBD within the TIPC mesh.
>>
>> This failure scenario is 100% reproducible and only takse the time of a 
>> reboot + 30
>> seconds to trigger.  It should be noted that the issue is only triggered if 
>> a node is
>> rebooted after the DRBD node is already up and running.  In other words, if 
>> the
>> DRBD node is rebooted *after* all other nodes are up and running, the link
>> failures to the other nodes do not occur (unless, of course, one or more of 
>> those
>> nodes is then subsequently rebooted, in which case those nodes will 
>> experience
>> a link failure once up and running).
>>
>> One other useful piece of information.  When the TIPC link fails (again it 
>> is only
>> ever one of the two TIPC links to a node that ever fails), it can be 
>> recovered by
>> manually 'bouncing' the bearer on the DRBD card (i.e. disabling bearer 
>> followed
>> by enabling the bearer).  However the interesting point here is that if the 
>> link on
>> fabric A is the one that failed, it is the B bearer that must be 'bounced' 
>> to fix the
>> link on fabric A.  Sounds like something to do with the DRBD shared address
>> scheme...
>>
>> --
>> Developer Access Program for Intel Xeon Phi Processors
>> Access to Intel Xeon Phi processor-based developer platforms.
>> With one year of Intel Parallel Studio XE.
>> Training and support from Colfax.
>> Order your platform today.http://sdm.link/xeonphi
>> ___
>> tipc-discussion mailing list
>>