ocfs2: o2net: fix connect expired

PATCH New Junxiao Bi 2 participants 4 messages Oct 30, 2014 OCFS2

Share & tools

Junxiao Bi

author

2014-10-30 01:41 UTC #0 lore raw

PATCH 1/1 ocfs2: o2net: fix connect expired

Set nn_persistent_error to -ENOTCONN will stop reconnect since the "stop" condition in o2net_start_connect() will be true.

stop = (nn->nn_sc ||

		(nn->nn_persistent_error &&
		(nn->nn_persistent_error != -ENOTCONN || timeout == 0)));

This will make connection never be established if the first connection request is lost.

Signed-off-by Junxiao Bi <[email protected]>

---
 fs/ocfs2/cluster/tcp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/ocfs2/cluster/tcp.c +1 -1

index 97de0fb..4d6b645 100644
@@ -1736,7 +1736,7 @@ static void o2net_connect_expired(struct work_struct *work)
 		     o2net_idle_timeout() / 1000,
 		     o2net_idle_timeout() % 1000);
 
-		o2net_set_nn_state(nn, NULL, 0, -ENOTCONN);
+		o2net_set_nn_state(nn, NULL, 0, 0);
 	}
 	spin_unlock(&nn->nn_lock);
 }

Signature · 1 line

1.7.9.5

Srinivas Eeda

2014-10-30 05:16 UTC #1 lore raw

PATCH 1/1 ocfs2: o2net: fix connect expired

Junxiao,

can you please describe under what circumstances you saw this problem? My understanding is o2net_connect_expired is only queued when connection actually broke and ENOTCONN is the right error in that case.

Thanks,
--Srini

Quoted 28 lines · Junxiao Bi

Set nn_persistent_error to -ENOTCONN will stop reconnect since the "stop" condition in o2net_start_connect() will be true.
stop = (nn->nn_sc ||
		(nn->nn_persistent_error &&
		(nn->nn_persistent_error != -ENOTCONN || timeout == 0)));
This will make connection never be established if the first connection request is lost.
Signed-off-by Junxiao Bi <[email protected]>
---
  fs/ocfs2/cluster/tcp.c |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
fs/ocfs2/cluster/tcp.c +1 -1
index 97de0fb..4d6b645 100644
@@ -1736,7 +1736,7 @@ static void o2net_connect_expired(struct work_struct *work)
  		     o2net_idle_timeout() / 1000,
  		     o2net_idle_timeout() % 1000);
 
-		o2net_set_nn_state(nn, NULL, 0, -ENOTCONN);
+		o2net_set_nn_state(nn, NULL, 0, 0);
  	}
  	spin_unlock(&nn->nn_lock);
  }

Junxiao Bi

author

2014-10-30 05:32 UTC #2 lore raw

PATCH 1/1 ocfs2: o2net: fix connect expired

Hi Srini,

Quoted 5 lines · Srinivas Eeda

Junxiao,
can you please describe under what circumstances you saw this problem? My understanding is o2net_connect_expired is only queued when connection actually broke and ENOTCONN is the right error in that case.

This happened when o2net was issuing the first connect request to some node, but the request packet is lost due to some error like network broken, then the connect would be expired, in o2net_connect_expired() that set nn->nn_persistent_error to -ENOTCONN but timeout was zero, so o2net_start_connect() would return without sending another connect request, connection to the node will never be built.

Thanks,
Junxiao.

Quoted 35 lines

Thanks,
--Srini
Quoted 29 lines · Junxiao Bi
Set nn_persistent_error to -ENOTCONN will stop reconnect since the "stop" condition in o2net_start_connect() will be true.
stop = (nn->nn_sc ||
        (nn->nn_persistent_error &&
        (nn->nn_persistent_error != -ENOTCONN || timeout == 0)));
This will make connection never be established if the first connection request
is lost.
Signed-off-by Junxiao Bi <[email protected]>
---
  fs/ocfs2/cluster/tcp.c |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
fs/ocfs2/cluster/tcp.c +1 -0
index 97de0fb..4d6b645 100644
@@ -1736,7 +1736,7 @@ static void o2net_connect_expired(struct
 work_struct *work)
               o2net_idle_timeout() / 1000,
               o2net_idle_timeout() % 1000);
  -        o2net_set_nn_state(nn, NULL, 0, -ENOTCONN);
+        o2net_set_nn_state(nn, NULL, 0, 0);
      }
      spin_unlock(&nn->nn_lock);
  }

Srinivas Eeda

2014-10-30 05:41 UTC #3 lore raw

PATCH 1/1 ocfs2: o2net: fix connect expired

Hi Junxiao,

thanks for explaining. For this case allowing a reconnect (setting "atomic_set(&nn->nn_timeout, 1);" ) in o2net_connect_expired should work ?

Thanks,
--Srini

Quoted 51 lines · Junxiao Bi

Hi Srini,
Quoted 5 lines · Srinivas Eeda
Junxiao,
can you please describe under what circumstances you saw this problem? My understanding is o2net_connect_expired is only queued when connection actually broke and ENOTCONN is the right error in that case.
This happened when o2net was issuing the first connect request to some node, but the request packet is lost due to some error like network broken, then the connect would be expired, in o2net_connect_expired() that set nn->nn_persistent_error to -ENOTCONN but timeout was zero, so o2net_start_connect() would return without sending another connect request, connection to the node will never be built.
Thanks,
Junxiao.
Quoted 33 lines
Thanks,
--Srini
Quoted 29 lines · Junxiao Bi
Set nn_persistent_error to -ENOTCONN will stop reconnect since the "stop" condition in o2net_start_connect() will be true.
stop = (nn->nn_sc ||
         (nn->nn_persistent_error &&
         (nn->nn_persistent_error != -ENOTCONN || timeout == 0)));
This will make connection never be established if the first connection request
is lost.
Signed-off-by Junxiao Bi <[email protected]>
---
   fs/ocfs2/cluster/tcp.c |    2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
fs/ocfs2/cluster/tcp.c +1 -0
index 97de0fb..4d6b645 100644
@@ -1736,7 +1736,7 @@ static void o2net_connect_expired(struct
 work_struct *work)
                o2net_idle_timeout() / 1000,
                o2net_idle_timeout() % 1000);
   -        o2net_set_nn_state(nn, NULL, 0, -ENOTCONN);
+        o2net_set_nn_state(nn, NULL, 0, 0);
       }
       spin_unlock(&nn->nn_lock);
   }