* [Cluster-devel] [PATCHv3 dlm/next 0/8] fs: dlm: introduce dlm re-transmission layer
@ 2021-03-26 17:33 Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 1/8] fs: dlm: public header in out utility Alexander Aring
` (7 more replies)
0 siblings, 8 replies; 21+ messages in thread
From: Alexander Aring @ 2021-03-26 17:33 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
this is the final patch-series to make dlm reliable when re-connection
occurs. You can easily generate a couple of re-connections by running:
tcpkill -9 -i $IFACE port 21064
on your own to test these patches. At some time dlm will detect message
drops and will re-transmit messages if necessary. It introduces a new dlm
protocol behaviour and increases the dlm protocol version. I tested it
with SCTP as well and tried to be backwards compatible with dlm protocol
version 3.1. However I don't recommend at all to mix these versions
in a setup since dlm version 3.2 fixes long-term issues.
- Alex
changes since v3:
- add comment about why queues are unbound
- move rcu usage to version receive handler
changes since v2:
- make timer handling pending only if messages are on air, the sync
isn't quite correct there but doesn't need to be precise
- use before() from tcp to check if seq is before other seq with
respect of overflows
- change srcu handling to hold srcu in all places where nodes are
referencing - we should not get a disadvantage of holding that
lock. We should update also lowcomms regarding to that.
- add some WARN_ON() to check that nothing in send/recv is going
anymore otherwise it's likely an issue.
- add more future work regarding to fencing of nodes if over
cluster manager timeout/bad seq happens
- add note about missing length size check of tail payload
(resource name length) regarding to the receive buffer
- remove some include which isn't necessary in recoverd.c
Thanks to Paolo Abeni for his review and recommendations.
Alexander Aring (8):
fs: dlm: public header in out utility
fs: dlm: add more midcomms hooks
fs: dlm: make buffer handling per msg
fs: dlm: add functionality to re-transmit a message
fs: dlm: move out some hash functionality
fs: dlm: add union in dlm header for lockspace id
fs: dlm: add reliable connection if reconnect
fs: dlm: don't allow half transmitted messages
fs/dlm/config.c | 3 +-
fs/dlm/dlm_internal.h | 35 +-
fs/dlm/lock.c | 14 +-
fs/dlm/lockspace.c | 14 +-
fs/dlm/lowcomms.c | 197 +++++--
fs/dlm/lowcomms.h | 23 +-
fs/dlm/member.c | 12 +-
fs/dlm/midcomms.c | 1307 ++++++++++++++++++++++++++++++++++++++++-
fs/dlm/midcomms.h | 10 +
fs/dlm/rcom.c | 63 +-
fs/dlm/util.c | 10 +-
fs/dlm/util.h | 2 +
12 files changed, 1577 insertions(+), 113 deletions(-)
--
2.26.3
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 1/8] fs: dlm: public header in out utility
2021-03-26 17:33 [Cluster-devel] [PATCHv3 dlm/next 0/8] fs: dlm: introduce dlm re-transmission layer Alexander Aring
@ 2021-03-26 17:33 ` Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 2/8] fs: dlm: add more midcomms hooks Alexander Aring
` (6 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Alexander Aring @ 2021-03-26 17:33 UTC (permalink / raw)
To: cluster-devel.redhat.com
This patch allows to use header_out() and header_in() outside of dlm
util functionality.
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
fs/dlm/util.c | 4 ++--
fs/dlm/util.h | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/dlm/util.c b/fs/dlm/util.c
index cfd0d00b19ae..74a8c5bfe9b5 100644
--- a/fs/dlm/util.c
+++ b/fs/dlm/util.c
@@ -20,7 +20,7 @@
#define DLM_ERRNO_ETIMEDOUT 110
#define DLM_ERRNO_EINPROGRESS 115
-static void header_out(struct dlm_header *hd)
+void header_out(struct dlm_header *hd)
{
hd->h_version = cpu_to_le32(hd->h_version);
hd->h_lockspace = cpu_to_le32(hd->h_lockspace);
@@ -28,7 +28,7 @@ static void header_out(struct dlm_header *hd)
hd->h_length = cpu_to_le16(hd->h_length);
}
-static void header_in(struct dlm_header *hd)
+void header_in(struct dlm_header *hd)
{
hd->h_version = le32_to_cpu(hd->h_version);
hd->h_lockspace = le32_to_cpu(hd->h_lockspace);
diff --git a/fs/dlm/util.h b/fs/dlm/util.h
index cc719ca9397e..d46f23c7a6a0 100644
--- a/fs/dlm/util.h
+++ b/fs/dlm/util.h
@@ -15,6 +15,8 @@ void dlm_message_out(struct dlm_message *ms);
void dlm_message_in(struct dlm_message *ms);
void dlm_rcom_out(struct dlm_rcom *rc);
void dlm_rcom_in(struct dlm_rcom *rc);
+void header_out(struct dlm_header *hd);
+void header_in(struct dlm_header *hd);
#endif
--
2.26.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 2/8] fs: dlm: add more midcomms hooks
2021-03-26 17:33 [Cluster-devel] [PATCHv3 dlm/next 0/8] fs: dlm: introduce dlm re-transmission layer Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 1/8] fs: dlm: public header in out utility Alexander Aring
@ 2021-03-26 17:33 ` Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 3/8] fs: dlm: make buffer handling per msg Alexander Aring
` (5 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Alexander Aring @ 2021-03-26 17:33 UTC (permalink / raw)
To: cluster-devel.redhat.com
This patch prepares hooks to redirect to the midcomms layer which will
be used by the midcomms re-transmit handling.
There exists the new concept of stateless buffers allocation and
commits. This can be used to bypass the midcomms re-transmit handling. It
is used by RCOM_STATUS and RCOM_NAMES messages, because they have their
own ping-like re-transmit handling. As well these two messages will be
used to determine the DLM version per node, because these two messages
are per observation the first messages which are exchanged.
The midcomms_remove_member() hook should be called when there is nothing
to send to the other node and the other node is still capable to
transmit dlm messages to the other node which called
midcomms_remove_member(). I experienced that the dlm protocol has a lack
of support for synchronize this event on protocol level. The result was
that there was still something to transmit but the other node was already
gone. This hook can be used to provide such synchronization. Although I
am not totally sure about the placement of this hook, I did not observed
issues yet when providing such synchronization on protocol layer.
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
fs/dlm/config.c | 3 ++-
fs/dlm/lock.c | 6 ++---
fs/dlm/lockspace.c | 7 +++---
fs/dlm/member.c | 12 +++++++++-
fs/dlm/midcomms.c | 44 +++++++++++++++++++++++++++++++++++++
fs/dlm/midcomms.h | 10 +++++++++
fs/dlm/rcom.c | 55 +++++++++++++++++++++++++++-------------------
7 files changed, 107 insertions(+), 30 deletions(-)
diff --git a/fs/dlm/config.c b/fs/dlm/config.c
index 88d95d96e36c..01ae294743e9 100644
--- a/fs/dlm/config.c
+++ b/fs/dlm/config.c
@@ -20,6 +20,7 @@
#include <net/sock.h>
#include "config.h"
+#include "midcomms.h"
#include "lowcomms.h"
/*
@@ -532,7 +533,7 @@ static void drop_comm(struct config_group *g, struct config_item *i)
struct dlm_comm *cm = config_item_to_comm(i);
if (local_comm == cm)
local_comm = NULL;
- dlm_lowcomms_close(cm->nodeid);
+ dlm_midcomms_close(cm->nodeid);
while (cm->addr_count--)
kfree(cm->addr[cm->addr_count]);
config_item_put(i);
diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index b93df39d0915..b3fd823009f4 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -59,7 +59,7 @@
#include "dlm_internal.h"
#include <linux/dlm_device.h>
#include "memory.h"
-#include "lowcomms.h"
+#include "midcomms.h"
#include "requestqueue.h"
#include "util.h"
#include "dir.h"
@@ -3537,7 +3537,7 @@ static int _create_message(struct dlm_ls *ls, int mb_len,
pass into lowcomms_commit and a message buffer (mb) that we
write our data into */
- mh = dlm_lowcomms_get_buffer(to_nodeid, mb_len, GFP_NOFS, &mb);
+ mh = dlm_midcomms_get_buffer(to_nodeid, mb_len, GFP_NOFS, &mb);
if (!mh)
return -ENOBUFS;
@@ -3589,7 +3589,7 @@ static int create_message(struct dlm_rsb *r, struct dlm_lkb *lkb,
static int send_message(struct dlm_mhandle *mh, struct dlm_message *ms)
{
dlm_message_out(ms);
- dlm_lowcomms_commit_buffer(mh);
+ dlm_midcomms_commit_buffer(mh);
return 0;
}
diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c
index c14cf2b7faab..bf5c55ef9d0d 100644
--- a/fs/dlm/lockspace.c
+++ b/fs/dlm/lockspace.c
@@ -16,6 +16,7 @@
#include "member.h"
#include "recoverd.h"
#include "dir.h"
+#include "midcomms.h"
#include "lowcomms.h"
#include "config.h"
#include "memory.h"
@@ -390,7 +391,7 @@ static int threads_start(void)
}
/* Thread for sending/receiving messages for all lockspace's */
- error = dlm_lowcomms_start();
+ error = dlm_midcomms_start();
if (error) {
log_print("cannot start dlm lowcomms %d", error);
goto scand_fail;
@@ -698,7 +699,7 @@ int dlm_new_lockspace(const char *name, const char *cluster,
error = 0;
if (!ls_count) {
dlm_scand_stop();
- dlm_lowcomms_shutdown();
+ dlm_midcomms_shutdown();
dlm_lowcomms_stop();
}
out:
@@ -787,7 +788,7 @@ static int release_lockspace(struct dlm_ls *ls, int force)
if (ls_count == 1) {
dlm_scand_stop();
- dlm_lowcomms_shutdown();
+ dlm_midcomms_shutdown();
}
dlm_callback_stop(ls);
diff --git a/fs/dlm/member.c b/fs/dlm/member.c
index ceef3f2074ff..c018a995a8ab 100644
--- a/fs/dlm/member.c
+++ b/fs/dlm/member.c
@@ -15,6 +15,7 @@
#include "recover.h"
#include "rcom.h"
#include "config.h"
+#include "midcomms.h"
#include "lowcomms.h"
int dlm_slots_version(struct dlm_header *h)
@@ -378,7 +379,15 @@ void dlm_clear_members(struct dlm_ls *ls)
void dlm_clear_members_gone(struct dlm_ls *ls)
{
- clear_memb_list(&ls->ls_nodes_gone);
+ struct list_head *head = &ls->ls_nodes_gone;
+ struct dlm_member *memb;
+
+ while (!list_empty(head)) {
+ memb = list_entry(head->next, struct dlm_member, list);
+ midcomms_remove_member(memb->nodeid);
+ list_del(&memb->list);
+ kfree(memb);
+ }
}
static void make_member_array(struct dlm_ls *ls)
@@ -563,6 +572,7 @@ int dlm_recover_members(struct dlm_ls *ls, struct dlm_recover *rv, int *neg_out)
if (dlm_is_member(ls, node->nodeid))
continue;
dlm_add_member(ls, node);
+ midcomms_add_member(node->nodeid);
log_rinfo(ls, "add member %d", node->nodeid);
}
diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
index 1c6654a21ec4..bbcb242e6101 100644
--- a/fs/dlm/midcomms.c
+++ b/fs/dlm/midcomms.c
@@ -28,6 +28,50 @@
#include "lock.h"
#include "midcomms.h"
+void *dlm_midcomms_get_buffer(int nodeid, int len, gfp_t allocation, char **ppc)
+{
+ return dlm_lowcomms_get_buffer(nodeid, len, allocation, ppc);
+}
+
+void dlm_midcomms_commit_buffer(void *mh)
+{
+ dlm_lowcomms_commit_buffer(mh);
+}
+
+void *dlm_midcomms_stateless_get_buffer(int nodeid, int len, gfp_t allocation,
+ char **ppc)
+{
+ return dlm_lowcomms_get_buffer(nodeid, len, allocation, ppc);
+}
+
+void dlm_midcomms_stateless_commit_buffer(void *mh)
+{
+ dlm_lowcomms_commit_buffer(mh);
+}
+
+void midcomms_add_member(int nodeid)
+{
+}
+
+void midcomms_remove_member(int nodeid)
+{
+}
+
+int dlm_midcomms_close(int nodeid)
+{
+ return dlm_lowcomms_close(nodeid);
+}
+
+int dlm_midcomms_start(void)
+{
+ return dlm_lowcomms_start();
+}
+
+void dlm_midcomms_shutdown(void)
+{
+ dlm_lowcomms_shutdown();
+}
+
/*
* Called from the low-level comms layer to process a buffer of
* commands.
diff --git a/fs/dlm/midcomms.h b/fs/dlm/midcomms.h
index 61e90a921849..6ce2f2947475 100644
--- a/fs/dlm/midcomms.h
+++ b/fs/dlm/midcomms.h
@@ -13,6 +13,16 @@
#define __MIDCOMMS_DOT_H__
int dlm_process_incoming_buffer(int nodeid, unsigned char *buf, int buflen);
+void *dlm_midcomms_get_buffer(int nodeid, int len, gfp_t allocation, char **ppc);
+void dlm_midcomms_commit_buffer(void *mh);
+void *dlm_midcomms_stateless_get_buffer(int nodeid, int len, gfp_t allocation,
+ char **ppc);
+void dlm_midcomms_stateless_commit_buffer(void *mh);
+void midcomms_add_member(int nodeid);
+void midcomms_remove_member(int nodeid);
+int dlm_midcomms_close(int nodeid);
+int dlm_midcomms_start(void);
+void dlm_midcomms_shutdown(void);
#endif /* __MIDCOMMS_DOT_H__ */
diff --git a/fs/dlm/rcom.c b/fs/dlm/rcom.c
index f5b1bd65728d..7a7d4a8e4706 100644
--- a/fs/dlm/rcom.c
+++ b/fs/dlm/rcom.c
@@ -28,14 +28,18 @@ static int rcom_response(struct dlm_ls *ls)
}
static int create_rcom(struct dlm_ls *ls, int to_nodeid, int type, int len,
- struct dlm_rcom **rc_ret, struct dlm_mhandle **mh_ret)
+ struct dlm_rcom **rc_ret, struct dlm_mhandle **mh_ret,
+ bool stateless)
{
struct dlm_rcom *rc;
struct dlm_mhandle *mh;
char *mb;
int mb_len = sizeof(struct dlm_rcom) + len;
- mh = dlm_lowcomms_get_buffer(to_nodeid, mb_len, GFP_NOFS, &mb);
+ if (stateless)
+ mh = dlm_midcomms_stateless_get_buffer(to_nodeid, mb_len, GFP_NOFS, &mb);
+ else
+ mh = dlm_midcomms_get_buffer(to_nodeid, mb_len, GFP_NOFS, &mb);
if (!mh) {
log_print("create_rcom to %d type %d len %d ENOBUFS",
to_nodeid, type, len);
@@ -62,10 +66,13 @@ static int create_rcom(struct dlm_ls *ls, int to_nodeid, int type, int len,
}
static void send_rcom(struct dlm_ls *ls, struct dlm_mhandle *mh,
- struct dlm_rcom *rc)
+ struct dlm_rcom *rc, bool stateless)
{
dlm_rcom_out(rc);
- dlm_lowcomms_commit_buffer(mh);
+ if (stateless)
+ dlm_midcomms_stateless_commit_buffer(mh);
+ else
+ dlm_midcomms_commit_buffer(mh);
}
static void set_rcom_status(struct dlm_ls *ls, struct rcom_status *rs,
@@ -154,7 +161,7 @@ int dlm_rcom_status(struct dlm_ls *ls, int nodeid, uint32_t status_flags)
retry:
error = create_rcom(ls, nodeid, DLM_RCOM_STATUS,
- sizeof(struct rcom_status), &rc, &mh);
+ sizeof(struct rcom_status), &rc, &mh, true);
if (error)
goto out;
@@ -163,7 +170,7 @@ int dlm_rcom_status(struct dlm_ls *ls, int nodeid, uint32_t status_flags)
allow_sync_reply(ls, &rc->rc_id);
memset(ls->ls_recover_buf, 0, LOWCOMMS_MAX_TX_BUFFER_LEN);
- send_rcom(ls, mh, rc);
+ send_rcom(ls, mh, rc, true);
error = dlm_wait_function(ls, &rcom_response);
disallow_sync_reply(ls);
@@ -219,7 +226,7 @@ static void receive_rcom_status(struct dlm_ls *ls, struct dlm_rcom *rc_in)
do_create:
error = create_rcom(ls, nodeid, DLM_RCOM_STATUS_REPLY,
- len, &rc, &mh);
+ len, &rc, &mh, true);
if (error)
return;
@@ -246,7 +253,7 @@ static void receive_rcom_status(struct dlm_ls *ls, struct dlm_rcom *rc_in)
spin_unlock(&ls->ls_recover_lock);
do_send:
- send_rcom(ls, mh, rc);
+ send_rcom(ls, mh, rc, true);
}
static void receive_sync_reply(struct dlm_ls *ls, struct dlm_rcom *rc_in)
@@ -277,7 +284,8 @@ int dlm_rcom_names(struct dlm_ls *ls, int nodeid, char *last_name, int last_len)
ls->ls_recover_nodeid = nodeid;
retry:
- error = create_rcom(ls, nodeid, DLM_RCOM_NAMES, last_len, &rc, &mh);
+ error = create_rcom(ls, nodeid, DLM_RCOM_NAMES, last_len, &rc, &mh,
+ true);
if (error)
goto out;
memcpy(rc->rc_buf, last_name, last_len);
@@ -285,7 +293,7 @@ int dlm_rcom_names(struct dlm_ls *ls, int nodeid, char *last_name, int last_len)
allow_sync_reply(ls, &rc->rc_id);
memset(ls->ls_recover_buf, 0, LOWCOMMS_MAX_TX_BUFFER_LEN);
- send_rcom(ls, mh, rc);
+ send_rcom(ls, mh, rc, true);
error = dlm_wait_function(ls, &rcom_response);
disallow_sync_reply(ls);
@@ -305,7 +313,8 @@ static void receive_rcom_names(struct dlm_ls *ls, struct dlm_rcom *rc_in)
inlen = rc_in->rc_header.h_length - sizeof(struct dlm_rcom);
outlen = LOWCOMMS_MAX_TX_BUFFER_LEN - sizeof(struct dlm_rcom);
- error = create_rcom(ls, nodeid, DLM_RCOM_NAMES_REPLY, outlen, &rc, &mh);
+ error = create_rcom(ls, nodeid, DLM_RCOM_NAMES_REPLY, outlen, &rc, &mh,
+ true);
if (error)
return;
rc->rc_id = rc_in->rc_id;
@@ -313,7 +322,7 @@ static void receive_rcom_names(struct dlm_ls *ls, struct dlm_rcom *rc_in)
dlm_copy_master_names(ls, rc_in->rc_buf, inlen, rc->rc_buf, outlen,
nodeid);
- send_rcom(ls, mh, rc);
+ send_rcom(ls, mh, rc, true);
}
int dlm_send_rcom_lookup(struct dlm_rsb *r, int dir_nodeid)
@@ -324,13 +333,13 @@ int dlm_send_rcom_lookup(struct dlm_rsb *r, int dir_nodeid)
int error;
error = create_rcom(ls, dir_nodeid, DLM_RCOM_LOOKUP, r->res_length,
- &rc, &mh);
+ &rc, &mh, false);
if (error)
goto out;
memcpy(rc->rc_buf, r->res_name, r->res_length);
rc->rc_id = (unsigned long) r->res_id;
- send_rcom(ls, mh, rc);
+ send_rcom(ls, mh, rc, false);
out:
return error;
}
@@ -342,7 +351,8 @@ static void receive_rcom_lookup(struct dlm_ls *ls, struct dlm_rcom *rc_in)
int error, ret_nodeid, nodeid = rc_in->rc_header.h_nodeid;
int len = rc_in->rc_header.h_length - sizeof(struct dlm_rcom);
- error = create_rcom(ls, nodeid, DLM_RCOM_LOOKUP_REPLY, 0, &rc, &mh);
+ error = create_rcom(ls, nodeid, DLM_RCOM_LOOKUP_REPLY, 0, &rc, &mh,
+ false);
if (error)
return;
@@ -361,7 +371,7 @@ static void receive_rcom_lookup(struct dlm_ls *ls, struct dlm_rcom *rc_in)
rc->rc_id = rc_in->rc_id;
rc->rc_seq_reply = rc_in->rc_seq;
- send_rcom(ls, mh, rc);
+ send_rcom(ls, mh, rc, false);
}
static void receive_rcom_lookup_reply(struct dlm_ls *ls, struct dlm_rcom *rc_in)
@@ -410,7 +420,8 @@ int dlm_send_rcom_lock(struct dlm_rsb *r, struct dlm_lkb *lkb)
if (lkb->lkb_lvbptr)
len += ls->ls_lvblen;
- error = create_rcom(ls, r->res_nodeid, DLM_RCOM_LOCK, len, &rc, &mh);
+ error = create_rcom(ls, r->res_nodeid, DLM_RCOM_LOCK, len, &rc, &mh,
+ false);
if (error)
goto out;
@@ -418,7 +429,7 @@ int dlm_send_rcom_lock(struct dlm_rsb *r, struct dlm_lkb *lkb)
pack_rcom_lock(r, lkb, rl);
rc->rc_id = (unsigned long) r;
- send_rcom(ls, mh, rc);
+ send_rcom(ls, mh, rc, false);
out:
return error;
}
@@ -433,7 +444,7 @@ static void receive_rcom_lock(struct dlm_ls *ls, struct dlm_rcom *rc_in)
dlm_recover_master_copy(ls, rc_in);
error = create_rcom(ls, nodeid, DLM_RCOM_LOCK_REPLY,
- sizeof(struct rcom_lock), &rc, &mh);
+ sizeof(struct rcom_lock), &rc, &mh, false);
if (error)
return;
@@ -444,7 +455,7 @@ static void receive_rcom_lock(struct dlm_ls *ls, struct dlm_rcom *rc_in)
rc->rc_id = rc_in->rc_id;
rc->rc_seq_reply = rc_in->rc_seq;
- send_rcom(ls, mh, rc);
+ send_rcom(ls, mh, rc, false);
}
/* If the lockspace doesn't exist then still send a status message
@@ -458,7 +469,7 @@ int dlm_send_ls_not_ready(int nodeid, struct dlm_rcom *rc_in)
char *mb;
int mb_len = sizeof(struct dlm_rcom) + sizeof(struct rcom_config);
- mh = dlm_lowcomms_get_buffer(nodeid, mb_len, GFP_NOFS, &mb);
+ mh = dlm_midcomms_get_buffer(nodeid, mb_len, GFP_NOFS, &mb);
if (!mh)
return -ENOBUFS;
@@ -479,7 +490,7 @@ int dlm_send_ls_not_ready(int nodeid, struct dlm_rcom *rc_in)
rf->rf_lvblen = cpu_to_le32(~0U);
dlm_rcom_out(rc);
- dlm_lowcomms_commit_buffer(mh);
+ dlm_midcomms_commit_buffer(mh);
return 0;
}
--
2.26.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 3/8] fs: dlm: make buffer handling per msg
2021-03-26 17:33 [Cluster-devel] [PATCHv3 dlm/next 0/8] fs: dlm: introduce dlm re-transmission layer Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 1/8] fs: dlm: public header in out utility Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 2/8] fs: dlm: add more midcomms hooks Alexander Aring
@ 2021-03-26 17:33 ` Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 4/8] fs: dlm: add functionality to re-transmit a message Alexander Aring
` (4 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Alexander Aring @ 2021-03-26 17:33 UTC (permalink / raw)
To: cluster-devel.redhat.com
This patch makes the void pointer handle for lowcomms functionality per
message and not per page allocation entry. A refcount handling for the
handle was added to keep the message alive until the user doesn't need
it anymore.
There exists now a per message callback which will be called when
allocating a new buffer. This callback will be guaranteed to be called
according the order of the sending buffer, which can be used that the
caller increments a sequence number.
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
fs/dlm/lowcomms.c | 100 +++++++++++++++++++++++++++++++++++++++++-----
fs/dlm/lowcomms.h | 5 ++-
fs/dlm/midcomms.c | 8 +++-
3 files changed, 101 insertions(+), 12 deletions(-)
diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index 73cc1809050a..ba782ea84281 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -114,6 +114,17 @@ struct writequeue_entry {
int end;
int users;
struct connection *con;
+ struct list_head msgs;
+ struct kref ref;
+};
+
+struct dlm_msg {
+ struct writequeue_entry *entry;
+ void *ppc;
+ int len;
+
+ struct list_head list;
+ struct kref ref;
};
struct dlm_node_addr {
@@ -976,12 +987,36 @@ static int accept_from_sock(struct listen_connection *con)
return result;
}
-static void free_entry(struct writequeue_entry *e)
+static void dlm_page_release(struct kref *kref)
{
+ struct writequeue_entry *e = container_of(kref, struct writequeue_entry,
+ ref);
+
__free_page(e->page);
kfree(e);
}
+static void dlm_msg_release(struct kref *kref)
+{
+ struct dlm_msg *msg = container_of(kref, struct dlm_msg, ref);
+
+ kref_put(&msg->entry->ref, dlm_page_release);
+ kfree(msg);
+}
+
+static void free_entry(struct writequeue_entry *e)
+{
+ struct dlm_msg *msg, *tmp;
+
+ list_for_each_entry_safe(msg, tmp, &e->msgs, list) {
+ list_del(&msg->list);
+ kref_put(&msg->ref, dlm_msg_release);
+ }
+
+ list_del(&e->list);
+ kref_put(&e->ref, dlm_page_release);
+}
+
/*
* writequeue_entry_complete - try to delete and free write queue entry
* @e: write queue entry to try to delete
@@ -994,10 +1029,8 @@ static void writequeue_entry_complete(struct writequeue_entry *e, int completed)
e->offset += completed;
e->len -= completed;
- if (e->len == 0 && e->users == 0) {
- list_del(&e->list);
+ if (e->len == 0 && e->users == 0)
free_entry(e);
- }
}
/*
@@ -1363,12 +1396,16 @@ static struct writequeue_entry *new_writequeue_entry(struct connection *con,
entry->con = con;
entry->users = 1;
+ kref_init(&entry->ref);
+ INIT_LIST_HEAD(&entry->msgs);
return entry;
}
static struct writequeue_entry *new_wq_entry(struct connection *con, int len,
- gfp_t allocation, char **ppc)
+ gfp_t allocation, char **ppc,
+ void (*cb)(void *buf, void *priv),
+ void *priv)
{
struct writequeue_entry *e;
@@ -1376,7 +1413,12 @@ static struct writequeue_entry *new_wq_entry(struct connection *con, int len,
if (!list_empty(&con->writequeue)) {
e = list_last_entry(&con->writequeue, struct writequeue_entry, list);
if (DLM_WQ_REMAIN_BYTES(e) >= len) {
+ kref_get(&e->ref);
+
*ppc = page_address(e->page) + e->end;
+ if (cb)
+ cb(*ppc, priv);
+
e->end += len;
e->users++;
spin_unlock(&con->writequeue_lock);
@@ -1390,19 +1432,26 @@ static struct writequeue_entry *new_wq_entry(struct connection *con, int len,
if (!e)
return NULL;
+ kref_get(&e->ref);
*ppc = page_address(e->page);
e->end += len;
spin_lock(&con->writequeue_lock);
+ if (cb)
+ cb(*ppc, priv);
+
list_add_tail(&e->list, &con->writequeue);
spin_unlock(&con->writequeue_lock);
return e;
};
-void *dlm_lowcomms_get_buffer(int nodeid, int len, gfp_t allocation, char **ppc)
+void *dlm_lowcomms_new_buffer(int nodeid, int len, gfp_t allocation, char **ppc,
+ void (*cb)(void *buf, void *priv), void *priv)
{
+ struct writequeue_entry *e;
struct connection *con;
+ struct dlm_msg *msg;
if (len > DEFAULT_BUFFER_SIZE ||
len < sizeof(struct dlm_header)) {
@@ -1416,16 +1465,36 @@ void *dlm_lowcomms_get_buffer(int nodeid, int len, gfp_t allocation, char **ppc)
if (!con)
return NULL;
- return new_wq_entry(con, len, allocation, ppc);
+ msg = kzalloc(sizeof(*msg), allocation);
+ if (!msg)
+ return NULL;
+
+ kref_init(&msg->ref);
+
+ e = new_wq_entry(con, len, allocation, ppc, cb, priv);
+ if (!e) {
+ kfree(msg);
+ return NULL;
+ }
+
+ msg->ppc = *ppc;
+ msg->len = len;
+ msg->entry = e;
+
+ return msg;
}
void dlm_lowcomms_commit_buffer(void *mh)
{
- struct writequeue_entry *e = (struct writequeue_entry *)mh;
+ struct dlm_msg *msg = mh;
+ struct writequeue_entry *e = msg->entry;
struct connection *con = e->con;
int users;
spin_lock(&con->writequeue_lock);
+ list_add(&msg->list, &e->msgs);
+ kref_get(&msg->ref);
+
users = --e->users;
if (users)
goto out;
@@ -1441,6 +1510,20 @@ void dlm_lowcomms_commit_buffer(void *mh)
return;
}
+void dlm_lowcomms_put_buffer(void *mh)
+{
+ struct dlm_msg *msg = mh;
+
+ kref_put(&msg->ref, dlm_msg_release);
+}
+
+void dlm_lowcomms_get_buffer(void *mh)
+{
+ struct dlm_msg *msg = mh;
+
+ kref_get(&msg->ref);
+}
+
/* Send a message */
static void send_to_sock(struct connection *con)
{
@@ -1519,7 +1602,6 @@ static void clean_one_writequeue(struct connection *con)
spin_lock(&con->writequeue_lock);
list_for_each_entry_safe(e, safe, &con->writequeue, list) {
- list_del(&e->list);
free_entry(e);
}
spin_unlock(&con->writequeue_lock);
diff --git a/fs/dlm/lowcomms.h b/fs/dlm/lowcomms.h
index 48bbc4e18761..fa735497dad8 100644
--- a/fs/dlm/lowcomms.h
+++ b/fs/dlm/lowcomms.h
@@ -22,11 +22,14 @@ void dlm_lowcomms_shutdown(void);
void dlm_lowcomms_stop(void);
void dlm_lowcomms_exit(void);
int dlm_lowcomms_close(int nodeid);
-void *dlm_lowcomms_get_buffer(int nodeid, int len, gfp_t allocation, char **ppc);
+void *dlm_lowcomms_new_buffer(int nodeid, int len, gfp_t allocation, char **ppc,
+ void (*cb)(void *buf, void *priv), void *priv);
void dlm_lowcomms_commit_buffer(void *mh);
int dlm_lowcomms_connect_node(int nodeid);
int dlm_lowcomms_nodes_set_mark(int nodeid, unsigned int mark);
int dlm_lowcomms_addr(int nodeid, struct sockaddr_storage *addr, int len);
+void dlm_lowcomms_put_buffer(void *mh);
+void dlm_lowcomms_get_buffer(void *mh);
#endif /* __LOWCOMMS_DOT_H__ */
diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
index bbcb242e6101..2ea0449a82ab 100644
--- a/fs/dlm/midcomms.c
+++ b/fs/dlm/midcomms.c
@@ -30,23 +30,27 @@
void *dlm_midcomms_get_buffer(int nodeid, int len, gfp_t allocation, char **ppc)
{
- return dlm_lowcomms_get_buffer(nodeid, len, allocation, ppc);
+ return dlm_lowcomms_new_buffer(nodeid, len, allocation, ppc, NULL,
+ NULL);
}
void dlm_midcomms_commit_buffer(void *mh)
{
dlm_lowcomms_commit_buffer(mh);
+ dlm_lowcomms_put_buffer(mh);
}
void *dlm_midcomms_stateless_get_buffer(int nodeid, int len, gfp_t allocation,
char **ppc)
{
- return dlm_lowcomms_get_buffer(nodeid, len, allocation, ppc);
+ return dlm_lowcomms_new_buffer(nodeid, len, allocation, ppc, NULL,
+ NULL);
}
void dlm_midcomms_stateless_commit_buffer(void *mh)
{
dlm_lowcomms_commit_buffer(mh);
+ dlm_lowcomms_put_buffer(mh);
}
void midcomms_add_member(int nodeid)
--
2.26.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 4/8] fs: dlm: add functionality to re-transmit a message
2021-03-26 17:33 [Cluster-devel] [PATCHv3 dlm/next 0/8] fs: dlm: introduce dlm re-transmission layer Alexander Aring
` (2 preceding siblings ...)
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 3/8] fs: dlm: make buffer handling per msg Alexander Aring
@ 2021-03-26 17:33 ` Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 5/8] fs: dlm: move out some hash functionality Alexander Aring
` (3 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Alexander Aring @ 2021-03-26 17:33 UTC (permalink / raw)
To: cluster-devel.redhat.com
This patch introduces a irqsafe retransmit functionality for a lowcomms
message handle. It's just allocates a new buffer and transmit it again,
no special handling about prioritize it because keeping bytestream in
order.
To avoid another connection look some refactor was done to make a new
buffer allocation with a preexisting connection pointer.
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
fs/dlm/lowcomms.c | 84 +++++++++++++++++++++++++++++++----------------
fs/dlm/lowcomms.h | 1 +
2 files changed, 56 insertions(+), 29 deletions(-)
diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index ba782ea84281..d2be58496fd0 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -1409,7 +1409,7 @@ static struct writequeue_entry *new_wq_entry(struct connection *con, int len,
{
struct writequeue_entry *e;
- spin_lock(&con->writequeue_lock);
+ spin_lock_bh(&con->writequeue_lock);
if (!list_empty(&con->writequeue)) {
e = list_last_entry(&con->writequeue, struct writequeue_entry, list);
if (DLM_WQ_REMAIN_BYTES(e) >= len) {
@@ -1421,12 +1421,12 @@ static struct writequeue_entry *new_wq_entry(struct connection *con, int len,
e->end += len;
e->users++;
- spin_unlock(&con->writequeue_lock);
+ spin_unlock_bh(&con->writequeue_lock);
return e;
}
}
- spin_unlock(&con->writequeue_lock);
+ spin_unlock_bh(&con->writequeue_lock);
e = new_writequeue_entry(con, allocation);
if (!e)
@@ -1436,35 +1436,24 @@ static struct writequeue_entry *new_wq_entry(struct connection *con, int len,
*ppc = page_address(e->page);
e->end += len;
- spin_lock(&con->writequeue_lock);
+ spin_lock_bh(&con->writequeue_lock);
if (cb)
cb(*ppc, priv);
list_add_tail(&e->list, &con->writequeue);
- spin_unlock(&con->writequeue_lock);
+ spin_unlock_bh(&con->writequeue_lock);
return e;
};
-void *dlm_lowcomms_new_buffer(int nodeid, int len, gfp_t allocation, char **ppc,
- void (*cb)(void *buf, void *priv), void *priv)
+static void *dlm_lowcomms_new_buffer_con(struct connection *con, int len,
+ gfp_t allocation, char **ppc,
+ void (*cb)(void *buf, void *priv),
+ void *priv)
{
struct writequeue_entry *e;
- struct connection *con;
struct dlm_msg *msg;
- if (len > DEFAULT_BUFFER_SIZE ||
- len < sizeof(struct dlm_header)) {
- BUILD_BUG_ON(PAGE_SIZE < DEFAULT_BUFFER_SIZE);
- log_print("failed to allocate a buffer of size %d", len);
- WARN_ON(1);
- return NULL;
- }
-
- con = nodeid2con(nodeid, allocation);
- if (!con)
- return NULL;
-
msg = kzalloc(sizeof(*msg), allocation);
if (!msg)
return NULL;
@@ -1484,6 +1473,26 @@ void *dlm_lowcomms_new_buffer(int nodeid, int len, gfp_t allocation, char **ppc,
return msg;
}
+void *dlm_lowcomms_new_buffer(int nodeid, int len, gfp_t allocation, char **ppc,
+ void (*cb)(void *buf, void *priv), void *priv)
+{
+ struct connection *con;
+
+ if (len > DEFAULT_BUFFER_SIZE ||
+ len < sizeof(struct dlm_header)) {
+ BUILD_BUG_ON(PAGE_SIZE < DEFAULT_BUFFER_SIZE);
+ log_print("failed to allocate a buffer of size %d", len);
+ WARN_ON(1);
+ return NULL;
+ }
+
+ con = nodeid2con(nodeid, allocation);
+ if (!con)
+ return NULL;
+
+ return dlm_lowcomms_new_buffer_con(con, len, GFP_ATOMIC, ppc, cb, priv);
+}
+
void dlm_lowcomms_commit_buffer(void *mh)
{
struct dlm_msg *msg = mh;
@@ -1491,7 +1500,7 @@ void dlm_lowcomms_commit_buffer(void *mh)
struct connection *con = e->con;
int users;
- spin_lock(&con->writequeue_lock);
+ spin_lock_bh(&con->writequeue_lock);
list_add(&msg->list, &e->msgs);
kref_get(&msg->ref);
@@ -1500,13 +1509,13 @@ void dlm_lowcomms_commit_buffer(void *mh)
goto out;
e->len = DLM_WQ_LENGTH_BYTES(e);
- spin_unlock(&con->writequeue_lock);
+ spin_unlock_bh(&con->writequeue_lock);
queue_work(send_workqueue, &con->swork);
return;
out:
- spin_unlock(&con->writequeue_lock);
+ spin_unlock_bh(&con->writequeue_lock);
return;
}
@@ -1524,6 +1533,23 @@ void dlm_lowcomms_get_buffer(void *mh)
kref_get(&msg->ref);
}
+/* irqsafe */
+void dlm_lowcomms_resend_buffer(void *mh)
+{
+ struct dlm_msg *msg = mh;
+ void *mh_new;
+ char *ppc;
+
+ mh_new = dlm_lowcomms_new_buffer_con(msg->entry->con, msg->len, GFP_ATOMIC,
+ &ppc, NULL, NULL);
+ if (!mh_new)
+ return;
+
+ memcpy(ppc, msg->ppc, msg->len);
+ dlm_lowcomms_commit_buffer(mh_new);
+ dlm_lowcomms_put_buffer(mh_new);
+}
+
/* Send a message */
static void send_to_sock(struct connection *con)
{
@@ -1537,7 +1563,7 @@ static void send_to_sock(struct connection *con)
if (con->sock == NULL)
goto out_connect;
- spin_lock(&con->writequeue_lock);
+ spin_lock_bh(&con->writequeue_lock);
for (;;) {
if (list_empty(&con->writequeue))
break;
@@ -1546,7 +1572,7 @@ static void send_to_sock(struct connection *con)
len = e->len;
offset = e->offset;
BUG_ON(len == 0 && e->users == 0);
- spin_unlock(&con->writequeue_lock);
+ spin_unlock_bh(&con->writequeue_lock);
ret = 0;
if (len) {
@@ -1574,10 +1600,10 @@ static void send_to_sock(struct connection *con)
count = 0;
}
- spin_lock(&con->writequeue_lock);
+ spin_lock_bh(&con->writequeue_lock);
writequeue_entry_complete(e, ret);
}
- spin_unlock(&con->writequeue_lock);
+ spin_unlock_bh(&con->writequeue_lock);
out:
mutex_unlock(&con->sock_mutex);
return;
@@ -1600,11 +1626,11 @@ static void clean_one_writequeue(struct connection *con)
{
struct writequeue_entry *e, *safe;
- spin_lock(&con->writequeue_lock);
+ spin_lock_bh(&con->writequeue_lock);
list_for_each_entry_safe(e, safe, &con->writequeue, list) {
free_entry(e);
}
- spin_unlock(&con->writequeue_lock);
+ spin_unlock_bh(&con->writequeue_lock);
}
/* Called from recovery when it knows that a node has
diff --git a/fs/dlm/lowcomms.h b/fs/dlm/lowcomms.h
index fa735497dad8..345aed7e00cc 100644
--- a/fs/dlm/lowcomms.h
+++ b/fs/dlm/lowcomms.h
@@ -30,6 +30,7 @@ int dlm_lowcomms_nodes_set_mark(int nodeid, unsigned int mark);
int dlm_lowcomms_addr(int nodeid, struct sockaddr_storage *addr, int len);
void dlm_lowcomms_put_buffer(void *mh);
void dlm_lowcomms_get_buffer(void *mh);
+void dlm_lowcomms_resend_buffer(void *mh);
#endif /* __LOWCOMMS_DOT_H__ */
--
2.26.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 5/8] fs: dlm: move out some hash functionality
2021-03-26 17:33 [Cluster-devel] [PATCHv3 dlm/next 0/8] fs: dlm: introduce dlm re-transmission layer Alexander Aring
` (3 preceding siblings ...)
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 4/8] fs: dlm: add functionality to re-transmit a message Alexander Aring
@ 2021-03-26 17:33 ` Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 6/8] fs: dlm: add union in dlm header for lockspace id Alexander Aring
` (2 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Alexander Aring @ 2021-03-26 17:33 UTC (permalink / raw)
To: cluster-devel.redhat.com
This patch moves out some lowcomms hash functionality into lowcomms
header to provide them to other layers like midcomms as well.
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
fs/dlm/lowcomms.c | 9 ---------
fs/dlm/lowcomms.h | 10 ++++++++++
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index d2be58496fd0..5cb6d7c9ddc1 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -59,7 +59,6 @@
#include "config.h"
#define NEEDED_RMEM (4*1024*1024)
-#define CONN_HASH_SIZE 32
/* Number of messages to send before rescheduling */
#define MAX_SEND_MSG_COUNT 25
@@ -166,14 +165,6 @@ static void sctp_connect_to_sock(struct connection *con);
static void tcp_connect_to_sock(struct connection *con);
static void dlm_tcp_shutdown(struct connection *con);
-/* This is deliberately very simple because most clusters have simple
- sequential nodeids, so we should be able to go straight to a connection
- struct in the array */
-static inline int nodeid_hash(int nodeid)
-{
- return nodeid & (CONN_HASH_SIZE-1);
-}
-
static struct connection *__find_con(int nodeid)
{
int r, idx;
diff --git a/fs/dlm/lowcomms.h b/fs/dlm/lowcomms.h
index 345aed7e00cc..cacfc5620f54 100644
--- a/fs/dlm/lowcomms.h
+++ b/fs/dlm/lowcomms.h
@@ -13,6 +13,16 @@
#define __LOWCOMMS_DOT_H__
#define LOWCOMMS_MAX_TX_BUFFER_LEN 4096
+#define CONN_HASH_SIZE 32
+
+/* This is deliberately very simple because most clusters have simple
+ * sequential nodeids, so we should be able to go straight to a connection
+ * struct in the array
+ */
+static inline int nodeid_hash(int nodeid)
+{
+ return nodeid & (CONN_HASH_SIZE-1);
+}
/* switch to check if dlm is running */
extern int dlm_allow_conn;
--
2.26.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 6/8] fs: dlm: add union in dlm header for lockspace id
2021-03-26 17:33 [Cluster-devel] [PATCHv3 dlm/next 0/8] fs: dlm: introduce dlm re-transmission layer Alexander Aring
` (4 preceding siblings ...)
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 5/8] fs: dlm: move out some hash functionality Alexander Aring
@ 2021-03-26 17:33 ` Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 8/8] fs: dlm: don't allow half transmitted messages Alexander Aring
7 siblings, 0 replies; 21+ messages in thread
From: Alexander Aring @ 2021-03-26 17:33 UTC (permalink / raw)
To: cluster-devel.redhat.com
This patch adds union inside the lockspace id to handle it also for
another use case for a different dlm command.
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
fs/dlm/dlm_internal.h | 5 ++++-
fs/dlm/lock.c | 8 ++++----
fs/dlm/midcomms.c | 1 -
fs/dlm/rcom.c | 4 ++--
fs/dlm/util.c | 6 ++++--
5 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/fs/dlm/dlm_internal.h b/fs/dlm/dlm_internal.h
index 04fe9f525ac7..917de7367a32 100644
--- a/fs/dlm/dlm_internal.h
+++ b/fs/dlm/dlm_internal.h
@@ -377,7 +377,10 @@ static inline int rsb_flag(struct dlm_rsb *r, enum rsb_flags flag)
struct dlm_header {
uint32_t h_version;
- uint32_t h_lockspace;
+ union {
+ /* for DLM_MSG and DLM_RCOM */
+ uint32_t h_lockspace;
+ } u;
uint32_t h_nodeid; /* nodeid of sender */
uint16_t h_length;
uint8_t h_cmd; /* DLM_MSG, DLM_RCOM */
diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index b3fd823009f4..daa5747c8556 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -3544,7 +3544,7 @@ static int _create_message(struct dlm_ls *ls, int mb_len,
ms = (struct dlm_message *) mb;
ms->m_header.h_version = (DLM_HEADER_MAJOR | DLM_HEADER_MINOR);
- ms->m_header.h_lockspace = ls->ls_global_id;
+ ms->m_header.u.h_lockspace = ls->ls_global_id;
ms->m_header.h_nodeid = dlm_our_nodeid();
ms->m_header.h_length = mb_len;
ms->m_header.h_cmd = DLM_MSG;
@@ -5038,16 +5038,16 @@ void dlm_receive_buffer(union dlm_packet *p, int nodeid)
if (hd->h_nodeid != nodeid) {
log_print("invalid h_nodeid %d from %d lockspace %x",
- hd->h_nodeid, nodeid, hd->h_lockspace);
+ hd->h_nodeid, nodeid, hd->u.h_lockspace);
return;
}
- ls = dlm_find_lockspace_global(hd->h_lockspace);
+ ls = dlm_find_lockspace_global(hd->u.h_lockspace);
if (!ls) {
if (dlm_config.ci_log_debug) {
printk_ratelimited(KERN_DEBUG "dlm: invalid lockspace "
"%u from %d cmd %d type %d\n",
- hd->h_lockspace, nodeid, hd->h_cmd, type);
+ hd->u.h_lockspace, nodeid, hd->h_cmd, type);
}
if (hd->h_cmd == DLM_RCOM && type == DLM_RCOM_STATUS)
diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
index 2ea0449a82ab..d86dcc95f736 100644
--- a/fs/dlm/midcomms.c
+++ b/fs/dlm/midcomms.c
@@ -149,4 +149,3 @@ int dlm_process_incoming_buffer(int nodeid, unsigned char *buf, int len)
return ret;
}
-
diff --git a/fs/dlm/rcom.c b/fs/dlm/rcom.c
index 7a7d4a8e4706..06f7a5f1d99d 100644
--- a/fs/dlm/rcom.c
+++ b/fs/dlm/rcom.c
@@ -49,7 +49,7 @@ static int create_rcom(struct dlm_ls *ls, int to_nodeid, int type, int len,
rc = (struct dlm_rcom *) mb;
rc->rc_header.h_version = (DLM_HEADER_MAJOR | DLM_HEADER_MINOR);
- rc->rc_header.h_lockspace = ls->ls_global_id;
+ rc->rc_header.u.h_lockspace = ls->ls_global_id;
rc->rc_header.h_nodeid = dlm_our_nodeid();
rc->rc_header.h_length = mb_len;
rc->rc_header.h_cmd = DLM_RCOM;
@@ -476,7 +476,7 @@ int dlm_send_ls_not_ready(int nodeid, struct dlm_rcom *rc_in)
rc = (struct dlm_rcom *) mb;
rc->rc_header.h_version = (DLM_HEADER_MAJOR | DLM_HEADER_MINOR);
- rc->rc_header.h_lockspace = rc_in->rc_header.h_lockspace;
+ rc->rc_header.u.h_lockspace = rc_in->rc_header.u.h_lockspace;
rc->rc_header.h_nodeid = dlm_our_nodeid();
rc->rc_header.h_length = mb_len;
rc->rc_header.h_cmd = DLM_RCOM;
diff --git a/fs/dlm/util.c b/fs/dlm/util.c
index 74a8c5bfe9b5..58acbcc2081a 100644
--- a/fs/dlm/util.c
+++ b/fs/dlm/util.c
@@ -23,7 +23,8 @@
void header_out(struct dlm_header *hd)
{
hd->h_version = cpu_to_le32(hd->h_version);
- hd->h_lockspace = cpu_to_le32(hd->h_lockspace);
+ /* does it for others u32 in union as well */
+ hd->u.h_lockspace = cpu_to_le32(hd->u.h_lockspace);
hd->h_nodeid = cpu_to_le32(hd->h_nodeid);
hd->h_length = cpu_to_le16(hd->h_length);
}
@@ -31,7 +32,8 @@ void header_out(struct dlm_header *hd)
void header_in(struct dlm_header *hd)
{
hd->h_version = le32_to_cpu(hd->h_version);
- hd->h_lockspace = le32_to_cpu(hd->h_lockspace);
+ /* does it for others u32 in union as well */
+ hd->u.h_lockspace = le32_to_cpu(hd->u.h_lockspace);
hd->h_nodeid = le32_to_cpu(hd->h_nodeid);
hd->h_length = le16_to_cpu(hd->h_length);
}
--
2.26.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect
2021-03-26 17:33 [Cluster-devel] [PATCHv3 dlm/next 0/8] fs: dlm: introduce dlm re-transmission layer Alexander Aring
` (5 preceding siblings ...)
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 6/8] fs: dlm: add union in dlm header for lockspace id Alexander Aring
@ 2021-03-26 17:33 ` Alexander Aring
2021-04-02 20:53 ` Guillaume Nault
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 8/8] fs: dlm: don't allow half transmitted messages Alexander Aring
7 siblings, 1 reply; 21+ messages in thread
From: Alexander Aring @ 2021-03-26 17:33 UTC (permalink / raw)
To: cluster-devel.redhat.com
This patch introduce to make a tcp lowcomms connection reliable even if
reconnects occurs. This is done by an application layer re-transmission
handling and sequence numbers in dlm protocols. There are three new dlm
commands:
DLM_OPTS:
This will encapsulate an existing dlm message (and rcom message if they
don't have an own application side re-transmission handling). As optional
handling additional tlv's (type length fields) can be appended. This can
be for example a sequence number field. However because in DLM_OPTS the
lockspace field is unused and a sequence number is a mandatory field it
isn't made as a tlv and we put the sequence number inside the lockspace
id. The possibility to add optional options are still there for future
purposes.
DLM_ACK:
Just a dlm header to acknowledge the receive of a DLM_OPTS message to
it's sender.
DLM_FIN:
A new DLM message to synchronize pending message to the other dlm end if
the node want to disconnects. Each side waits until it receives this
message and disconnects. It's important that this message has nothing to
do with the application logic because it might run in a timeout if
acknowledge messages are dropped. The problem which we try to solve with
DLM_FIN is that the node membership is handled by corosync and not the
kernel dlm protocol itself, DLM_FIN tries to synchronize the kernel dlm
protocol with corosync join/leave membership callbacks.
To explain the basic functionality take a look into the
dlm_midcomms_receive_buffer() function. This function will take care
that dlm messages are delivered according to their sequence numbers and
request re-transmission via sending acknowledge messages. However there
exists three cases:
1. sequence number is the one which is expected. That means everything
is working fine. Additional there is always a check if the next
message was already queued for future, this will occur when there was
some messages drops before.
2. A sequence number is in the future, in this case we queue it for might
future delivery, see case 1.
3. A sequence number is in the past, in this case we drop this message
because it was already delivered.
To send acknowledge we always send the sequence number which is
expected, if the other node sends multiple acknowledge for the same
sequence numbers it will trigger a re-transmission. In case no acknowledge
is send back, a timer with a timeout handling is running and will trigger
a re-tranmission as well. Sending multiple ack's with the same sequence or
messages with the same sequence should not have any effects that breaks
dlm. Only messages in the far future can break dlm, that's why important
that the closing connection is right synchronized with DLM_FIN which
also resets the sequence numbers.
As RCOM_STATUS and RCOM_NAMES messages are the first messages which are
exchanged and they have they own re-transmission handling, there exists
logic that these messages must be first. If these messages arrives we
store the dlm version field. This handling is on DLM 3.1 and after this
patch 3.2 the same. A backwards compatibility handling has been added
which seems to work on tests without tcpkill, however it's not recommended
to use DLM 3.1 and 3.2 at the same time, because DLM 3.2 tries to fix long
term bugs in the DLM protocol.
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
fs/dlm/dlm_internal.h | 30 +-
fs/dlm/lockspace.c | 7 +-
fs/dlm/lowcomms.h | 7 +-
fs/dlm/midcomms.c | 1302 +++++++++++++++++++++++++++++++++++++++--
fs/dlm/rcom.c | 4 +-
5 files changed, 1295 insertions(+), 55 deletions(-)
diff --git a/fs/dlm/dlm_internal.h b/fs/dlm/dlm_internal.h
index 917de7367a32..da2297705713 100644
--- a/fs/dlm/dlm_internal.h
+++ b/fs/dlm/dlm_internal.h
@@ -368,18 +368,26 @@ static inline int rsb_flag(struct dlm_rsb *r, enum rsb_flags flag)
/* dlm_header is first element of all structs sent between nodes */
#define DLM_HEADER_MAJOR 0x00030000
-#define DLM_HEADER_MINOR 0x00000001
+#define DLM_HEADER_MINOR 0x00000002
+
+#define DLM_VERSION_3_1 0x00030001
+#define DLM_VERSION_3_2 0x00030002
#define DLM_HEADER_SLOTS 0x00000001
#define DLM_MSG 1
#define DLM_RCOM 2
+#define DLM_OPTS 3
+#define DLM_ACK 4
+#define DLM_FIN 5
struct dlm_header {
uint32_t h_version;
union {
/* for DLM_MSG and DLM_RCOM */
uint32_t h_lockspace;
+ /* for DLM_ACK and DLM_OPTS */
+ uint32_t h_seq;
} u;
uint32_t h_nodeid; /* nodeid of sender */
uint16_t h_length;
@@ -387,7 +395,6 @@ struct dlm_header {
uint8_t h_pad;
};
-
#define DLM_MSG_REQUEST 1
#define DLM_MSG_CONVERT 2
#define DLM_MSG_UNLOCK 3
@@ -455,10 +462,29 @@ struct dlm_rcom {
char rc_buf[];
};
+struct dlm_opt_header {
+ uint16_t t_type;
+ uint16_t t_length;
+ uint32_t o_pad;
+ /* need to be 8 byte aligned */
+ char t_value[];
+};
+
+/* encapsulation header */
+struct dlm_opts {
+ struct dlm_header o_header;
+ uint8_t o_nextcmd;
+ uint8_t o_pad;
+ uint16_t o_optlen;
+ uint32_t o_pad2;
+ char o_opts[];
+};
+
union dlm_packet {
struct dlm_header header; /* common to other two */
struct dlm_message message;
struct dlm_rcom rcom;
+ struct dlm_opts opts;
};
#define DLM_RSF_NEED_SLOTS 0x00000001
diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c
index bf5c55ef9d0d..2b738be8d7e4 100644
--- a/fs/dlm/lockspace.c
+++ b/fs/dlm/lockspace.c
@@ -567,7 +567,12 @@ static int new_lockspace(const char *name, const char *cluster,
mutex_init(&ls->ls_requestqueue_mutex);
mutex_init(&ls->ls_clear_proc_locks);
- ls->ls_recover_buf = kmalloc(LOWCOMMS_MAX_TX_BUFFER_LEN, GFP_NOFS);
+ /* Due backwards compatibility with 3.1 we need to use maximum
+ * possible dlm message size to be sure the message will fit and
+ * not having out of bounds issues. However on sending side 3.2
+ * might send less.
+ */
+ ls->ls_recover_buf = kmalloc(DEFAULT_BUFFER_SIZE, GFP_NOFS);
if (!ls->ls_recover_buf)
goto out_lkbidr;
diff --git a/fs/dlm/lowcomms.h b/fs/dlm/lowcomms.h
index cacfc5620f54..1554d8c078c9 100644
--- a/fs/dlm/lowcomms.h
+++ b/fs/dlm/lowcomms.h
@@ -12,7 +12,12 @@
#ifndef __LOWCOMMS_DOT_H__
#define __LOWCOMMS_DOT_H__
-#define LOWCOMMS_MAX_TX_BUFFER_LEN 4096
+#include "dlm_internal.h"
+
+#define DLM_MIDCOMMS_OPT_LEN sizeof(struct dlm_opts)
+#define LOWCOMMS_MAX_TX_BUFFER_LEN (DEFAULT_BUFFER_SIZE - \
+ DLM_MIDCOMMS_OPT_LEN)
+
#define CONN_HASH_SIZE 32
/* This is deliberately very simple because most clusters have simple
diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
index d86dcc95f736..879d1dba817f 100644
--- a/fs/dlm/midcomms.c
+++ b/fs/dlm/midcomms.c
@@ -3,7 +3,7 @@
*******************************************************************************
**
** Copyright (C) Sistina Software, Inc. 1997-2003 All rights reserved.
-** Copyright (C) 2004-2008 Red Hat, Inc. All rights reserved.
+** Copyright (C) 2004-2021 Red Hat, Inc. All rights reserved.
**
**
*******************************************************************************
@@ -12,68 +12,870 @@
/*
* midcomms.c
*
- * This is the appallingly named "mid-level" comms layer.
+ * This is the appallingly named "mid-level" comms layer. It takes care
+ * about deliver an on application layer "reliable" communication above
+ * the used lowcomms transport layer.
*
- * Its purpose is to take packets from the "real" comms layer,
- * split them up into packets and pass them to the interested
- * part of the locking mechanism.
+ * How it works:
*
- * It also takes messages from the locking layer, formats them
- * into packets and sends them to the comms layer.
+ * The basic implementation about the implemented re-transmission algorithm
+ * is based on:
+ *
+ * https://web.archive.org/web/20200813052233/https://www.isi.edu/nsnam/DIRECTED_RESEARCH/DR_WANIDA/DR/JavisInActionFastRetransmitFrame.html
+ *
+ * There are two cases when a re-transmission occurs, both are described in
+ * the link above:
+ *
+ * - Multiple acks if the same sequence number are received agan, known as
+ * "fast re-transmit".
+ * - Ack timeout, if we don't receive an ack in a time of "DLM_RTO". Note
+ * that the this timeout will only be checked in the re-transmission
+ * time which runs ever "DLM_RTO_TIMER".
+ *
+ * Each node has a send_queue and recv_queue. The send_queue contains a
+ * reference to each msg handle of lowcomms and keeps the message alive
+ * so far it has not been acked. This is necessary to keep the message
+ * for a possible re-transmission.
+ *
+ * The recv_queue is used to keep messages saved which are having a
+ * sequence number in the future of the expected sequence number. If
+ * the message with the expected sequence number arrives and we have
+ * the next sequence number inside this queue it will be delivered to
+ * the upper layer. This queue should have entries only if there was a
+ * drop.
+ *
+ * How version detection works:
+ *
+ * Due the fact that dlm has pre-configured node addresses on every side
+ * it is in it's nature that every side connects at starts to transmit
+ * dlm messages which ends in a race. However DLM_RCOM_NAMES, DLM_RCOM_STATUS
+ * and their replies are the first messages which are exchanges. Due backwards
+ * compatibility these messages are not covered by the midcomms re-transmission
+ * layer. These messages have their own re-transmission handling in the dlm
+ * application layer. The version field of every node will be set on these RCOM
+ * messages as soon as they arrived and the node isn't yet part of the nodes
+ * hash. There exists also logic to detect version mismatched if something weird
+ * going on or the first messages isn't an expected one.
+ *
+ * How ack handling basically works:
+ *
+ * We ack every message with the "expected sequence number". A connection
+ * starts with the sequence number of 0. Receiving an ack will ack all
+ * dlm messages below it's sequence number. It's possible that an ack
+ * message can ack multiple dlm messages as the other side will send an
+ * sequence number back of it's expected sequence number. If there are
+ * multiple ack's received with the same sequence number it means that
+ * there was a drop and the other side telling us which message was
+ * dropped. The parameter "DLM_RETRANS_ACK_COUNT" can be used to change
+ * the amount of how many ack we need to received to start a retransmit.
+ * It's 3 but 2 should be fine as well as it's ery unlikely that we have
+ * out of order issues as RFC 2001 says to use 3. I can only think about
+ * multiple re-connection in a row where with drops which we might get
+ * ordering issues.
+ *
+ * In cases where nobody transmits any messages anymore but there was
+ * a drop, the re-transmission timer will kick in to resend messages
+ * which have an ack timeout.
+ *
+ * Sequence number cases:
+ *
+ * When a message arrives and function "dlm_midcomms_receive_buffer()" is
+ * called, it will detect by it's sequence number if the message is:
+ *
+ * - as expected => everything is fine
+ * - in the future => there was a drop, queue msg in recv_queue queue
+ * - in the past => ignore that message, because already delivered
+ *
+ * However we will always send an ack message to the other node to signal
+ * which is the current expected sequence number. This either acks
+ * messages or re-transmit on the other side, see ack handling.
+ *
+ * Termination:
+ *
+ * As this layer was implemented there was a problem discovered during
+ * connection termination. At midcomms_stop() call which occurs at the
+ * last dlm lockspace release the other side was still sending dlm
+ * messages after the connection was terminated. Further investigations
+ * shows that the membership organization is done by the cluster manager
+ * and not the kernel dlm protocol itself. To make sure the node membership
+ * of a lockspace is synchronized with the kernel a DLM_FIN message was
+ * introduced. It is part of a synchronization mechanism to wait for
+ * the specific cluster manager events on both endpoints so a termination
+ * of sockets can be initiated. The "DLM_QUEUE_TIMEOUT" defines some
+ * timeout handling to wait for some synchronization points at
+ * connection termination.
+ *
+ * Future improvements:
+ *
+ * There exists some known issues/improvements of the dlm handling. Some
+ * of them should be done in a next major dlm version bump which makes
+ * it incompatible with previous versions.
+ *
+ * Unaligned memory access:
+ *
+ * There exists cases when the dlm message buffer length is not aligned
+ * to 8 byte. However seems nobody detected any problem with it. This
+ * can be fixed in the next major version bump of dlm.
+ *
+ * Version detection:
+ *
+ * The version detection and how it's done is related to backwards
+ * compatibility. There exists better ways to make a better handling.
+ * However this should be changed in the next major version bump of dlm.
+ *
+ * Ack handling:
+ *
+ * Currently we send an ack message for every dlm message. However we
+ * can ack multiple dlm messages with one ack by just delaying the ack
+ * message. Will reduce some traffic but makes the drop detection slower.
+ *
+ * Termination:
+ *
+ * Maybe handle nodes membership of lockspaces in dlm protocol itself,
+ * but that requires still synchronization with the cluster manager.
+ *
+ * Tail Size checking:
+ *
+ * There exists a message tail payload in e.g. DLM_MSG however we don't
+ * check it against the message length yet regarding to the receive buffer
+ * length. That need to be validated.
+ *
+ * Fencing bad nodes:
+ *
+ * At timeout places or weird sequence number behaviours we should send
+ * a fencing request to the cluster manager.
+ */
+
+/* Debug switch to enable a 5 seconds sleep waiting of a termination.
+ * This can be useful to test fencing while termination is running.
+ * This requires a setup with only gfs2 as dlm user, so that the
+ * last umount will terminate the connection.
+ *
+ * However it became useful to test, while the 5 seconds block in umount
+ * just press the reset button. In a lot of dropping the termination
+ * process can could take several seconds.
*/
+#define DLM_DEBUG_FENCE_TERMINATION 0
+
+#include <net/tcp.h>
#include "dlm_internal.h"
#include "lowcomms.h"
#include "config.h"
#include "lock.h"
+#include "util.h"
#include "midcomms.h"
-void *dlm_midcomms_get_buffer(int nodeid, int len, gfp_t allocation, char **ppc)
+/* init value for sequence numbers for testing purpose only e.g. overflows */
+#define DLM_SEQ_INIT 0
+/* if we get duplicates amount of acks we will start retransmit */
+#define DLM_RETRANS_ACK_COUNT 3
+/* timer timeout to start retransmit if we don't get an ack back */
+#define DLM_RTO msecs_to_jiffies(15 * 1000)
+#define DLM_RTO_TIMER msecs_to_jiffies(30 * 1000)
+/* 5 minutes wait to sync ending of dlm */
+#define DLM_QUEUE_TIMEOUT msecs_to_jiffies(5 * 60 * 1000)
+#define DLM_VERSION_NOT_SET 0
+
+struct midcomms_node {
+ int nodeid;
+ uint32_t version;
+ uint32_t seq_send;
+ uint32_t seq_next;
+ /* These list may not sorted according to their seq.
+ * These queues are unbound because we cannot drop any message in dlm.
+ * We could send a fence signal for a specific node to the cluster
+ * manager if queues hits some maximum value, however this handling
+ * not supported yet.
+ */
+ struct list_head send_queue;
+ struct list_head recv_queue;
+ spinlock_t send_queue_lock;
+ atomic_t send_queue_cnt;
+ atomic_t recv_queue_cnt;
+#define DLM_NODE_FLAG_FIN 1
+#define DLM_NODE_FLAG_CLOSE 2
+#define DLM_NODE_FLAG_STOP_TX 3
+#define DLM_NODE_FLAG_STOP_RX 4
+ unsigned long flags;
+ wait_queue_head_t fin_wait;
+ /* counts how many lockspaces are using this node
+ * this refcount is necessary to determine if the
+ * node wants to disconnect.
+ */
+ int users;
+
+ struct hlist_node hlist;
+ struct rcu_head rcu;
+};
+
+struct dlm_send_msg {
+ const struct dlm_header *inner_hd;
+ struct midcomms_node *node;
+ struct dlm_opts *opts;
+ unsigned long timeout;
+ int ack_count;
+ uint32_t seq;
+ void *mh;
+
+ /* get_buffer/commit srcu idx exchange */
+ int idx;
+
+ struct list_head list;
+ struct rcu_head rcu;
+};
+
+struct dlm_recv_msg {
+ union dlm_packet *p;
+ uint32_t seq;
+
+ struct list_head list;
+};
+
+static DEFINE_SPINLOCK(dlm_retransmit_timer_lock);
+static struct timer_list dlm_retransmit_timer;
+
+static struct hlist_head node_hash[CONN_HASH_SIZE];
+static DEFINE_SPINLOCK(nodes_lock);
+DEFINE_STATIC_SRCU(nodes_srcu);
+
+/* This mutex prevents that midcomms_close() is running while
+ * stop() or remove(). As I experienced invalid memory access
+ * behaviours when DLM_DEBUG_FENCE_TERMINATION is enabled and
+ * resetting machines. I will end in some double deletion in nodes
+ * datastructure.
+ */
+static DEFINE_MUTEX(close_lock);
+
+static struct midcomms_node *__find_node(int nodeid, int r)
{
- return dlm_lowcomms_new_buffer(nodeid, len, allocation, ppc, NULL,
- NULL);
+ struct midcomms_node *node;
+
+ hlist_for_each_entry_rcu(node, &node_hash[r], hlist) {
+ if (node->nodeid == nodeid)
+ return node;
+ }
+
+ return NULL;
}
-void dlm_midcomms_commit_buffer(void *mh)
+static struct midcomms_node *nodeid2node(int nodeid, gfp_t alloc)
{
- dlm_lowcomms_commit_buffer(mh);
- dlm_lowcomms_put_buffer(mh);
+ struct midcomms_node *node, *tmp;
+ int r = nodeid_hash(nodeid);
+
+ node = __find_node(nodeid, r);
+ if (node || !alloc)
+ return node;
+
+ node = kzalloc(sizeof(*node), alloc);
+ if (!node)
+ return NULL;
+
+ node->nodeid = nodeid;
+ node->seq_next = DLM_SEQ_INIT;
+ node->seq_send = DLM_SEQ_INIT;
+ spin_lock_init(&node->send_queue_lock);
+ atomic_set(&node->send_queue_cnt, 0);
+ INIT_LIST_HEAD(&node->send_queue);
+ atomic_set(&node->recv_queue_cnt, 0);
+ INIT_LIST_HEAD(&node->recv_queue);
+ init_waitqueue_head(&node->fin_wait);
+
+ spin_lock(&nodes_lock);
+ /* check again if there was somebody else
+ * earlier here to add the node
+ */
+ tmp = __find_node(nodeid, r);
+ if (tmp) {
+ spin_unlock(&nodes_lock);
+ kfree(node);
+ return tmp;
+ }
+
+ hlist_add_head_rcu(&node->hlist, &node_hash[r]);
+ spin_unlock(&nodes_lock);
+
+ return node;
}
-void *dlm_midcomms_stateless_get_buffer(int nodeid, int len, gfp_t allocation,
- char **ppc)
+static int dlm_send_ack(int nodeid, uint32_t seq)
{
- return dlm_lowcomms_new_buffer(nodeid, len, allocation, ppc, NULL,
- NULL);
+ int mb_len = sizeof(struct dlm_header);
+ struct dlm_header *m_header;
+ char *ppc;
+ void *mh;
+
+ mh = dlm_midcomms_stateless_get_buffer(nodeid, mb_len, GFP_NOFS, &ppc);
+ if (!mh)
+ return -ENOMEM;
+
+ m_header = (struct dlm_header *)ppc;
+
+ m_header->h_version = (DLM_HEADER_MAJOR | DLM_HEADER_MINOR);
+ m_header->h_nodeid = dlm_our_nodeid();
+ m_header->h_length = mb_len;
+ m_header->h_cmd = DLM_ACK;
+ m_header->u.h_seq = seq;
+
+ header_out(m_header);
+ dlm_midcomms_stateless_commit_buffer(mh);
+
+ return 0;
}
-void dlm_midcomms_stateless_commit_buffer(void *mh)
+static int dlm_send_fin(int nodeid)
{
- dlm_lowcomms_commit_buffer(mh);
- dlm_lowcomms_put_buffer(mh);
+ int mb_len = sizeof(struct dlm_header);
+ struct dlm_header *m_header;
+ char *ppc;
+ void *mh;
+
+ mh = dlm_midcomms_get_buffer(nodeid, mb_len, GFP_NOFS, &ppc);
+ if (!mh)
+ return -ENOMEM;
+
+ m_header = (struct dlm_header *)ppc;
+
+ m_header->h_version = (DLM_HEADER_MAJOR | DLM_HEADER_MINOR);
+ m_header->h_nodeid = dlm_our_nodeid();
+ m_header->h_length = mb_len;
+ m_header->h_cmd = DLM_FIN;
+
+ header_out(m_header);
+ dlm_midcomms_commit_buffer(mh);
+
+ return 0;
}
-void midcomms_add_member(int nodeid)
+static void dlm_send_msg_release(struct rcu_head *rcu)
{
+ struct dlm_send_msg *msg = container_of(rcu, struct dlm_send_msg, rcu);
+
+ dlm_lowcomms_put_buffer(msg->mh);
+ kfree(msg);
}
-void midcomms_remove_member(int nodeid)
+/* send_queue_lock must be held */
+static void dlm_send_msg_del(struct midcomms_node *node,
+ struct dlm_send_msg *msg)
{
+ list_del_rcu(&msg->list);
+ if (atomic_dec_and_test(&node->send_queue_cnt)) {
+ spin_lock(&dlm_retransmit_timer_lock);
+ del_timer(&dlm_retransmit_timer);
+ spin_unlock(&dlm_retransmit_timer_lock);
+
+ wake_up(&node->fin_wait);
+ }
+
+ call_rcu(&msg->rcu, dlm_send_msg_release);
}
-int dlm_midcomms_close(int nodeid)
+static void dlm_send_queue_flush(struct midcomms_node *node)
{
- return dlm_lowcomms_close(nodeid);
+ struct dlm_send_msg *msg;
+
+ rcu_read_lock();
+ spin_lock(&node->send_queue_lock);
+ list_for_each_entry_rcu(msg, &node->send_queue, list) {
+ dlm_send_msg_del(node, msg);
+ }
+ spin_unlock(&node->send_queue_lock);
+ rcu_read_unlock();
}
-int dlm_midcomms_start(void)
+static void dlm_recv_queue_flush(struct midcomms_node *node)
{
- return dlm_lowcomms_start();
+ struct dlm_recv_msg *msg, *tmp;
+
+ list_for_each_entry_safe(msg, tmp, &node->recv_queue, list) {
+ list_del(&msg->list);
+ if (atomic_dec_and_test(&node->recv_queue_cnt))
+ wake_up(&node->fin_wait);
+
+ kfree(msg->p);
+ kfree(msg);
+ }
}
-void dlm_midcomms_shutdown(void)
+static void dlm_midcomms_seq_receive_buffer(struct midcomms_node *node,
+ union dlm_packet *p)
{
- dlm_lowcomms_shutdown();
+ dlm_receive_buffer(p, node->nodeid);
+ node->seq_next++;
+}
+
+static void dlm_receive_ack(struct midcomms_node *node, uint32_t seq)
+{
+ struct dlm_send_msg *msg;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(msg, &node->send_queue, list) {
+ if (before(msg->seq, seq)) {
+ spin_lock(&node->send_queue_lock);
+ dlm_send_msg_del(node, msg);
+ spin_unlock(&node->send_queue_lock);
+ } else if (msg->seq == seq) {
+ msg->ack_count++;
+ if (msg->ack_count >= DLM_RETRANS_ACK_COUNT) {
+ log_print("retransmit because multiple acks received. message seq %u, nodeid: %d",
+ seq, node->nodeid);
+ dlm_lowcomms_resend_buffer(msg->mh);
+ }
+ }
+ }
+ rcu_read_unlock();
+}
+
+static struct dlm_recv_msg *dlm_recv_create_msg(const union dlm_packet *p,
+ uint32_t seq)
+{
+ int mb_len = le16_to_cpu(p->header.h_length);
+ struct dlm_recv_msg *msg;
+
+ msg = kzalloc(sizeof(*msg), GFP_NOFS);
+ if (!msg)
+ return NULL;
+
+ msg->p = kzalloc(mb_len, GFP_NOFS);
+ if (!msg)
+ return NULL;
+
+ memcpy(msg->p, p, mb_len);
+ msg->seq = seq;
+
+ return msg;
+}
+
+static struct dlm_recv_msg *dlm_recv_get_msg_by_seq(struct midcomms_node *node,
+ uint32_t seq)
+{
+ struct dlm_recv_msg *msg;
+
+ list_for_each_entry(msg, &node->recv_queue, list) {
+ if (msg->seq == seq)
+ return msg;
+ }
+
+ return NULL;
+}
+
+static int dlm_recv_queue_future_msg(struct midcomms_node *node,
+ const union dlm_packet *p,
+ uint32_t seq)
+{
+ struct dlm_recv_msg *msg;
+
+ /* check if we already got the message */
+ msg = dlm_recv_get_msg_by_seq(node, seq);
+ if (msg)
+ return 0;
+
+ log_print("received message for the future, will queue it. expected seq: %u, message seq: %u, nodeid: %d",
+ node->seq_next, seq, node->nodeid);
+
+ msg = dlm_recv_create_msg(p, seq);
+ if (!msg)
+ return -ENOMEM;
+
+ atomic_inc(&node->recv_queue_cnt);
+ list_add_tail(&msg->list, &node->recv_queue);
+
+ return 0;
+}
+
+static void dlm_recv_queue_lookup_and_deliver_next(struct midcomms_node *node)
+{
+ struct dlm_recv_msg *msg;
+
+ msg = dlm_recv_get_msg_by_seq(node, node->seq_next);
+ while (msg) {
+ list_del(&msg->list);
+
+ log_print("deliver message from the queue because it was received in the past. message seq: %u, nodeid: %d",
+ node->seq_next, node->nodeid);
+ dlm_midcomms_seq_receive_buffer(node, msg->p);
+
+ if (atomic_dec_and_test(&node->recv_queue_cnt))
+ wake_up(&node->fin_wait);
+
+ kfree(msg->p);
+ kfree(msg);
+
+ msg = dlm_recv_get_msg_by_seq(node, node->seq_next);
+ }
+}
+
+static void dlm_midcomms_receive_buffer(union dlm_packet *p,
+ struct midcomms_node *node,
+ uint32_t seq)
+{
+ if (seq == node->seq_next) {
+ /* case when we received the expected next message */
+ dlm_midcomms_seq_receive_buffer(node, p);
+
+ /* lookup if we have the next message in the queue beceause
+ * we might got some message for the future in the past as
+ * we dropped some message which arrived now.
+ */
+ dlm_recv_queue_lookup_and_deliver_next(node);
+ } else if (seq > node->seq_next) {
+ /* We got some message in the future, we will queue it so
+ * we can deliver the message when the next message arrived.
+ */
+ dlm_recv_queue_future_msg(node, p, seq);
+ } else {
+ log_print("ignore message because it was already delivered. message seq: %u, expected: %u, nodeid: %d",
+ seq, node->seq_next, node->nodeid);
+ }
+
+ dlm_send_ack(node->nodeid, node->seq_next);
+}
+
+static struct midcomms_node *
+dlm_midcomms_recv_node_lookup(int nodeid, const union dlm_packet *p,
+ uint16_t msglen, int (*cb)(struct midcomms_node *node))
+{
+ struct midcomms_node *node;
+ gfp_t allocation = 0;
+ int ret;
+
+ switch (p->header.h_cmd) {
+ case DLM_RCOM:
+ if (msglen < sizeof(struct dlm_rcom)) {
+ log_print("rcom msg too small: %u, will skip this message from node %d",
+ msglen, nodeid);
+ return NULL;
+ }
+
+ switch (le32_to_cpu(p->rcom.rc_type)) {
+ case DLM_RCOM_NAMES:
+ fallthrough;
+ case DLM_RCOM_NAMES_REPLY:
+ fallthrough;
+ case DLM_RCOM_STATUS:
+ fallthrough;
+ case DLM_RCOM_STATUS_REPLY:
+ allocation = GFP_NOFS;
+ break;
+ default:
+ break;
+ }
+
+ break;
+ default:
+ break;
+ }
+
+ node = nodeid2node(nodeid, allocation);
+ if (!node) {
+ switch (p->header.h_cmd) {
+ case DLM_OPTS:
+ if (msglen < sizeof(struct dlm_opts)) {
+ log_print("opts msg too small: %u, will skip this message from node %d",
+ msglen, nodeid);
+ return NULL;
+ }
+
+ /* we only alloc a new node@receiving for the above
+ * RCOM messages. It can be that the other side is
+ * already gone and we cannot ack FIN messages anymore,
+ * we ignore it until the other side runs into an
+ * timeout. FIN messages are application stateless and
+ * it's not imortant to be acked since it is the last
+ * message before disconnect.
+ *
+ * we don't print a warning in this case.
+ */
+ switch (p->opts.o_nextcmd) {
+ case DLM_FIN:
+ return NULL;
+ default:
+ break;
+ }
+
+ break;
+ default:
+ break;
+ }
+
+ log_print("received dlm message cmd %d from node %d in an invalid sequence",
+ p->opts.o_nextcmd, nodeid);
+ return NULL;
+ }
+
+ if (test_bit(DLM_NODE_FLAG_STOP_RX, &node->flags)) {
+ WARN_ON(1);
+ return NULL;
+ }
+
+ ret = cb(node);
+ if (ret < 0)
+ return NULL;
+
+ return node;
+}
+
+static int dlm_midcomms_version_check_3_2(struct midcomms_node *node)
+{
+ switch (node->version) {
+ case DLM_VERSION_NOT_SET:
+ node->version = DLM_VERSION_3_2;
+ log_print("version 0x%08x for node %d detected", DLM_VERSION_3_2,
+ node->nodeid);
+ break;
+ case DLM_VERSION_3_2:
+ break;
+ default:
+ log_print("version mismatch detected, assumed 0x%08x but node %d has 0x%08x",
+ DLM_VERSION_3_2, node->nodeid, node->version);
+ return -1;
+ }
+
+ return 0;
+}
+
+#if 0
+static int dlm_parse_opts(void *opts, uint16_t optlen)
+{
+ struct dlm_opt_header *hd;
+ void *ptr = opts;
+ uint16_t length;
+
+ while (optlen > 0) {
+ hd = ptr;
+ switch (le16_to_cpu(hd->t_type)) {
+ /* add options here */
+ default:
+ break;
+ }
+
+ length = le16_to_cpu(hd->t_length);
+ ptr += length;
+ optlen -= length;
+ }
+
+ return 0;
+}
+#endif
+
+static int dlm_opts_check_msglen(union dlm_packet *p, uint16_t msglen, int nodeid)
+{
+ int len = msglen;
+
+ /* we only trust outer header msglen because
+ * it's checked against receive buffer length.
+ */
+ if (len < sizeof(struct dlm_opts))
+ return -1;
+ len -= sizeof(struct dlm_opts);
+
+ if (len < le16_to_cpu(p->opts.o_optlen))
+ return -1;
+ len -= le16_to_cpu(p->opts.o_optlen);
+
+ switch (p->opts.o_nextcmd) {
+ case DLM_FIN:
+ if (len < sizeof(struct dlm_header)) {
+ log_print("fin too small: %d, will skip this message from node %d",
+ len, nodeid);
+ return -1;
+ }
+
+ break;
+ case DLM_MSG:
+ if (len < sizeof(struct dlm_message)) {
+ log_print("msg too small: %d, will skip this message from node %d",
+ msglen, nodeid);
+ return -1;
+ }
+
+ break;
+ case DLM_RCOM:
+ if (len < sizeof(struct dlm_rcom)) {
+ log_print("rcom msg too small: %d, will skip this message from node %d",
+ len, nodeid);
+ return -1;
+ }
+
+ break;
+ default:
+ log_print("unsupported o_nextcmd received: %u, will skip this message from node %d",
+ p->opts.o_nextcmd, nodeid);
+ return -1;
+ }
+
+ return 0;
+}
+
+static void dlm_midcomms_receive_buffer_3_2(union dlm_packet *p, int nodeid)
+{
+ uint16_t msglen = le16_to_cpu(p->header.h_length);
+ struct midcomms_node *node;
+ uint32_t seq;
+ int ret, idx;
+
+ idx = srcu_read_lock(&nodes_srcu);
+ node = dlm_midcomms_recv_node_lookup(nodeid, p, msglen,
+ dlm_midcomms_version_check_3_2);
+ if (!node)
+ goto out;
+
+ switch (p->header.h_cmd) {
+ case DLM_RCOM:
+ /* these rcom message we use to determine version.
+ * they have their own retransmission handling and
+ * are the first messages of dlm.
+ *
+ * length already checked.
+ */
+ switch (le32_to_cpu(p->rcom.rc_type)) {
+ case DLM_RCOM_NAMES:
+ fallthrough;
+ case DLM_RCOM_NAMES_REPLY:
+ fallthrough;
+ case DLM_RCOM_STATUS:
+ fallthrough;
+ case DLM_RCOM_STATUS_REPLY:
+ break;
+ default:
+ log_print("unsupported rcom type received: %u, will skip this message from node %d",
+ le32_to_cpu(p->rcom.rc_type), nodeid);
+ goto out;
+ }
+
+ dlm_receive_buffer(p, nodeid);
+ break;
+ case DLM_OPTS:
+ seq = le32_to_cpu(p->header.u.h_seq);
+
+ ret = dlm_opts_check_msglen(p, msglen, nodeid);
+ if (ret < 0) {
+ log_print("opts msg too small: %u, will skip this message from node %d",
+ msglen, nodeid);
+ goto out;
+ }
+#if 0
+ ret = dlm_parse_opts(p->opts.o_opts, p->opts.o_optlen);
+ if (ret < 0)
+ goto out;
+#endif
+
+ p = (union dlm_packet *)((unsigned char *)p->opts.o_opts +
+ le16_to_cpu(p->opts.o_optlen));
+
+ /* recheck inner msglen just if it's not garbage */
+ msglen = le16_to_cpu(p->header.h_length);
+ switch (p->header.h_cmd) {
+ case DLM_RCOM:
+ if (msglen < sizeof(struct dlm_rcom)) {
+ log_print("inner rcom msg too small: %u, will skip this message from node %d",
+ msglen, nodeid);
+ goto out;
+ }
+
+ break;
+ case DLM_MSG:
+ if (msglen < sizeof(struct dlm_message)) {
+ log_print("inner msg too small: %u, will skip this message from node %d",
+ msglen, nodeid);
+ goto out;
+ }
+
+ break;
+ case DLM_FIN:
+ if (msglen < sizeof(struct dlm_header)) {
+ log_print("inner fin too small: %u, will skip this message from node %d",
+ msglen, nodeid);
+ goto out;
+ }
+
+ if (seq != node->seq_next) {
+ dlm_send_ack(nodeid, node->seq_next);
+ goto out;
+ }
+
+ dlm_send_ack(nodeid, seq + 1);
+ set_bit(DLM_NODE_FLAG_FIN, &node->flags);
+ wake_up(&node->fin_wait);
+ /* midcomms only */
+ goto out;
+ default:
+ log_print("unsupported inner h_cmd received: %u, will skip this message from node %d",
+ msglen, nodeid);
+ goto out;
+ }
+
+ dlm_midcomms_receive_buffer(p, node, seq);
+ break;
+ case DLM_ACK:
+ seq = le32_to_cpu(p->header.u.h_seq);
+ dlm_receive_ack(node, seq);
+ break;
+ default:
+ log_print("unsupported h_cmd received: %u, will skip this message from node %d",
+ p->header.h_cmd, nodeid);
+ break;
+ }
+
+out:
+ srcu_read_unlock(&nodes_srcu, idx);
+}
+
+static int dlm_midcomms_version_check_3_1(struct midcomms_node *node)
+{
+ switch (node->version) {
+ case DLM_VERSION_NOT_SET:
+ node->version = DLM_VERSION_3_1;
+ log_print("version 0x%08x for node %d detected", DLM_VERSION_3_1,
+ node->nodeid);
+ break;
+ case DLM_VERSION_3_1:
+ break;
+ default:
+ log_print("version mismatch detected, assumed 0x%08x but node %d has 0x%08x",
+ DLM_VERSION_3_1, node->nodeid, node->version);
+ return -1;
+ }
+
+ return 0;
+}
+
+static void dlm_midcomms_receive_buffer_3_1(union dlm_packet *p, int nodeid)
+{
+ uint16_t msglen = le16_to_cpu(p->header.h_length);
+ struct midcomms_node *node;
+ int idx;
+
+ idx = srcu_read_lock(&nodes_srcu);
+ node = dlm_midcomms_recv_node_lookup(nodeid, p, msglen,
+ dlm_midcomms_version_check_3_1);
+ if (!node) {
+ srcu_read_unlock(&nodes_srcu, idx);
+ return;
+ }
+ srcu_read_unlock(&nodes_srcu, idx);
+
+ switch (p->header.h_cmd) {
+ case DLM_RCOM:
+ /* length already checked */
+ break;
+ case DLM_MSG:
+ if (msglen < sizeof(struct dlm_message)) {
+ log_print("msg too small: %u, will skip this message from node %d",
+ msglen, nodeid);
+ return;
+ }
+
+ break;
+ default:
+ log_print("unsupported h_cmd received: %u, will skip this message from node %d",
+ p->header.h_cmd, nodeid);
+ return;
+ }
+
+ dlm_receive_buffer(p, nodeid);
}
/*
@@ -116,32 +918,19 @@ int dlm_process_incoming_buffer(int nodeid, unsigned char *buf, int len)
if (msglen > len)
break;
- switch (hd->h_cmd) {
- case DLM_MSG:
- if (msglen < sizeof(struct dlm_message)) {
- log_print("dlm msg too small: %u, will skip this message",
- msglen);
- goto skip;
- }
-
+ switch (le32_to_cpu(hd->h_version)) {
+ case DLM_VERSION_3_1:
+ dlm_midcomms_receive_buffer_3_1((union dlm_packet *)ptr, nodeid);
break;
- case DLM_RCOM:
- if (msglen < sizeof(struct dlm_rcom)) {
- log_print("dlm rcom msg too small: %u, will skip this message",
- msglen);
- goto skip;
- }
-
+ case DLM_VERSION_3_2:
+ dlm_midcomms_receive_buffer_3_2((union dlm_packet *)ptr, nodeid);
break;
default:
- log_print("unsupported h_cmd received: %u, will skip this message",
- hd->h_cmd);
- goto skip;
+ log_print("received invalid version header: %u from node %d, will skip this message",
+ le32_to_cpu(hd->h_version), nodeid);
+ break;
}
- dlm_receive_buffer((union dlm_packet *)ptr, nodeid);
-
-skip:
ret += msglen;
len -= msglen;
ptr += msglen;
@@ -149,3 +938,418 @@ int dlm_process_incoming_buffer(int nodeid, unsigned char *buf, int len)
return ret;
}
+
+void *dlm_midcomms_stateless_get_buffer(int nodeid, int len, gfp_t allocation,
+ char **ppc)
+{
+ return dlm_lowcomms_new_buffer(nodeid, len, allocation,
+ ppc, NULL, NULL);
+}
+
+void dlm_midcomms_stateless_commit_buffer(void *mh)
+{
+ dlm_lowcomms_commit_buffer(mh);
+ dlm_lowcomms_put_buffer(mh);
+}
+
+static void dlm_fill_opts_header(struct dlm_opts *opts, uint16_t inner_len,
+ uint32_t seq)
+{
+ opts->o_header.h_cmd = DLM_OPTS;
+ opts->o_header.h_version = (DLM_HEADER_MAJOR | DLM_HEADER_MINOR);
+ opts->o_header.h_nodeid = dlm_our_nodeid();
+ opts->o_header.h_length = DLM_MIDCOMMS_OPT_LEN + inner_len;
+ opts->o_header.u.h_seq = seq;
+ header_out(&opts->o_header);
+}
+
+static void midcomms_get_buffer_cb(void *buf, void *priv)
+{
+ struct dlm_send_msg *msg = priv;
+
+ msg->seq = msg->node->seq_send++;
+}
+
+static void *dlm_midcomms_get_buffer_3_2(struct dlm_send_msg *msg, int nodeid,
+ int len, gfp_t allocation, char **ppc)
+{
+ struct dlm_opts *opts;
+ void *mh;
+
+ mh = dlm_lowcomms_new_buffer(nodeid, len + DLM_MIDCOMMS_OPT_LEN,
+ allocation, ppc, midcomms_get_buffer_cb, msg);
+ if (!mh)
+ return NULL;
+
+ opts = (struct dlm_opts *)*ppc;
+ msg->opts = opts;
+
+ /* add possible options here */
+ dlm_fill_opts_header(opts, len, msg->seq);
+
+ *ppc += sizeof(*opts);
+ msg->inner_hd = (const struct dlm_header *)*ppc;
+ return mh;
+}
+
+void *dlm_midcomms_get_buffer(int nodeid, int len, gfp_t allocation, char **ppc)
+{
+ struct midcomms_node *node;
+ struct dlm_send_msg *msg;
+ void *mh;
+ int idx;
+
+ idx = srcu_read_lock(&nodes_srcu);
+ node = nodeid2node(nodeid, 0);
+ if (!node) {
+ WARN_ON_ONCE(1);
+ goto err;
+ }
+
+ if (test_bit(DLM_NODE_FLAG_STOP_TX, &node->flags)) {
+ WARN_ON(1);
+ goto err;
+ }
+
+ msg = kzalloc(sizeof(*msg), GFP_NOFS);
+ if (!msg)
+ goto err;
+
+ msg->idx = idx;
+ msg->node = node;
+
+ switch (node->version) {
+ case DLM_VERSION_3_1:
+ mh = dlm_midcomms_stateless_get_buffer(nodeid, len, allocation,
+ ppc);
+ if (!mh) {
+ kfree(msg);
+ goto err;
+ }
+
+ break;
+ case DLM_VERSION_3_2:
+ mh = dlm_midcomms_get_buffer_3_2(msg, nodeid, len, allocation,
+ ppc);
+ if (!mh) {
+ kfree(msg);
+ goto err;
+ }
+
+ break;
+ default:
+ kfree(msg);
+ WARN_ON(1);
+ goto err;
+ }
+
+ msg->mh = mh;
+
+ /* keep in mind that is a must to call
+ * dlm_midcomms_commit_buffer() which releases
+ * nodes_srcu using msg->idx which is assumed
+ * here that the application will call it.
+ */
+ return msg;
+
+err:
+ srcu_read_unlock(&nodes_srcu, idx);
+ return NULL;
+}
+
+static void dlm_midcomms_commit_buffer_3_2(struct dlm_send_msg *msg,
+ struct midcomms_node *node)
+{
+ spin_lock(&dlm_retransmit_timer_lock);
+ if (!timer_pending(&dlm_retransmit_timer))
+ mod_timer(&dlm_retransmit_timer, jiffies + DLM_RTO_TIMER);
+ spin_unlock(&dlm_retransmit_timer_lock);
+
+ atomic_inc(&node->send_queue_cnt);
+
+ /* nexthdr chain for fast lookup */
+ msg->opts->o_nextcmd = msg->inner_hd->h_cmd;
+
+ /* required to set before list_add because timer */
+ msg->timeout = jiffies + DLM_RTO;
+
+ spin_lock(&node->send_queue_lock);
+ list_add_tail_rcu(&msg->list, &node->send_queue);
+ spin_unlock(&node->send_queue_lock);
+
+ dlm_lowcomms_commit_buffer(msg->mh);
+}
+
+static void dlm_midcomms_commit_buffer_3_1(void *mh)
+{
+ dlm_midcomms_stateless_commit_buffer(mh);
+}
+
+void dlm_midcomms_commit_buffer(void *mh)
+{
+ struct dlm_send_msg *msg = mh;
+
+ switch (msg->node->version) {
+ case DLM_VERSION_3_1:
+ dlm_midcomms_commit_buffer_3_1(msg->mh);
+ /* msg is not part of rcu list in this case */
+ kfree(msg);
+ break;
+ case DLM_VERSION_3_2:
+ dlm_midcomms_commit_buffer_3_2(msg, msg->node);
+ break;
+ default:
+ WARN_ON(1);
+ break;
+ }
+
+ /* release nodes_srcu from get_buffer() */
+ srcu_read_unlock(&nodes_srcu, msg->idx);
+}
+
+void midcomms_add_member(int nodeid)
+{
+ struct midcomms_node *node;
+ int idx;
+
+ if (nodeid == dlm_our_nodeid())
+ return;
+
+ idx = srcu_read_lock(&nodes_srcu);
+ node = nodeid2node(nodeid, GFP_NOFS);
+ if (!node) {
+ srcu_read_unlock(&nodes_srcu, idx);
+ return;
+ }
+
+ node->users++;
+ srcu_read_unlock(&nodes_srcu, idx);
+}
+
+static void midcomms_drain_and_fin(struct midcomms_node *node, bool active)
+{
+ int ret;
+
+ /* old protocol, we don't wait for pending operations */
+ switch (node->version) {
+ case DLM_VERSION_3_2:
+ break;
+ default:
+ return;
+ }
+
+ if (DLM_DEBUG_FENCE_TERMINATION)
+ msleep(5000);
+
+ ret = wait_event_timeout(node->fin_wait,
+ (!atomic_read(&node->send_queue_cnt) &&
+ !atomic_read(&node->recv_queue_cnt)) ||
+ test_bit(DLM_NODE_FLAG_CLOSE, &node->flags),
+ DLM_QUEUE_TIMEOUT);
+ if (!ret || test_bit(DLM_NODE_FLAG_CLOSE, &node->flags))
+ goto timed_out;
+
+ if (active) {
+ dlm_send_fin(node->nodeid);
+ set_bit(DLM_NODE_FLAG_STOP_TX, &node->flags);
+
+ /* wait for other side dlm + fin */
+ ret = wait_event_timeout(node->fin_wait,
+ (!atomic_read(&node->send_queue_cnt) &&
+ !atomic_read(&node->recv_queue_cnt) &&
+ test_bit(DLM_NODE_FLAG_FIN, &node->flags)) ||
+ test_bit(DLM_NODE_FLAG_CLOSE, &node->flags),
+ DLM_QUEUE_TIMEOUT);
+ if (!ret || test_bit(DLM_NODE_FLAG_CLOSE, &node->flags))
+ goto timed_out;
+ } else {
+ /* wait other side is finished by it's fin */
+ ret = wait_event_timeout(node->fin_wait,
+ test_bit(DLM_NODE_FLAG_FIN, &node->flags) ||
+ test_bit(DLM_NODE_FLAG_CLOSE, &node->flags),
+ DLM_QUEUE_TIMEOUT);
+ if (!ret || test_bit(DLM_NODE_FLAG_CLOSE, &node->flags))
+ goto timed_out;
+
+ dlm_send_fin(node->nodeid);
+ set_bit(DLM_NODE_FLAG_STOP_TX, &node->flags);
+
+ /* wait fin ack is back */
+ ret = wait_event_timeout(node->fin_wait,
+ (!atomic_read(&node->send_queue_cnt) &&
+ !atomic_read(&node->recv_queue_cnt)) ||
+ test_bit(DLM_NODE_FLAG_CLOSE, &node->flags),
+ DLM_QUEUE_TIMEOUT);
+ if (!ret || test_bit(DLM_NODE_FLAG_CLOSE, &node->flags))
+ goto timed_out;
+ }
+
+ set_bit(DLM_NODE_FLAG_STOP_RX, &node->flags);
+
+ return;
+
+timed_out:
+ if (!ret)
+ log_print("timed out from node %d", node->nodeid);
+
+ dlm_send_queue_flush(node);
+ synchronize_rcu();
+
+ dlm_recv_queue_flush(node);
+}
+
+static void midcomms_node_release(struct rcu_head *rcu)
+{
+ struct midcomms_node *node = container_of(rcu, struct midcomms_node, rcu);
+
+ /* warn if we have anything in queues which indicates a bug */
+ WARN_ON(atomic_read(&node->send_queue_cnt));
+ WARN_ON(atomic_read(&node->recv_queue_cnt));
+
+ kfree(node);
+}
+
+void midcomms_remove_member(int nodeid)
+{
+ struct midcomms_node *node;
+ int idx;
+
+ if (nodeid == dlm_our_nodeid())
+ return;
+
+ idx = srcu_read_lock(&nodes_srcu);
+ mutex_lock(&close_lock);
+ node = nodeid2node(nodeid, 0);
+ if (!node) {
+ mutex_unlock(&close_lock);
+ srcu_read_unlock(&nodes_srcu, idx);
+ return;
+ }
+
+ node->users--;
+
+ /* hitting users count to zero means the
+ * other side is running dlm_midcomms_stop()
+ * we meet us to have a clean disconnect.
+ */
+ if (!node->users) {
+ midcomms_drain_and_fin(node, false);
+
+ spin_lock(&nodes_lock);
+ hlist_del_rcu(&node->hlist);
+ spin_unlock(&nodes_lock);
+
+ call_srcu(&nodes_srcu, &node->rcu, midcomms_node_release);
+ }
+ mutex_unlock(&close_lock);
+ srcu_read_unlock(&nodes_srcu, idx);
+}
+
+static void dlm_retransmit_timer_expires(struct timer_list *timer)
+{
+ unsigned long now = jiffies;
+ struct midcomms_node *node;
+ struct dlm_send_msg *msg;
+ int idx, i;
+
+ rcu_read_lock();
+ idx = srcu_read_lock(&nodes_srcu);
+ for (i = 0; i < CONN_HASH_SIZE; i++) {
+ hlist_for_each_entry_rcu(node, &node_hash[i], hlist) {
+ list_for_each_entry_rcu(msg, &node->send_queue, list) {
+ if (time_before_eq(msg->timeout, now)) {
+ log_print("retransmit because ack timeout. message seq %u, nodeid %d",
+ msg->seq, node->nodeid);
+
+ dlm_lowcomms_resend_buffer(msg->mh);
+ }
+ }
+ }
+ }
+ srcu_read_unlock(&nodes_srcu, idx);
+ rcu_read_unlock();
+}
+
+int dlm_midcomms_start(void)
+{
+ int i, ret;
+
+ for (i = 0; i < CONN_HASH_SIZE; i++)
+ INIT_HLIST_HEAD(&node_hash[i]);
+
+ ret = dlm_lowcomms_start();
+ if (ret == 0)
+ timer_setup(&dlm_retransmit_timer,
+ dlm_retransmit_timer_expires, TIMER_DEFERRABLE);
+
+ return ret;
+}
+
+void dlm_midcomms_shutdown(void)
+{
+ struct midcomms_node *node;
+ int i, idx;
+
+ mutex_lock(&close_lock);
+ idx = srcu_read_lock(&nodes_srcu);
+ for (i = 0; i < CONN_HASH_SIZE; i++) {
+ hlist_for_each_entry_rcu(node, &node_hash[i], hlist) {
+ midcomms_drain_and_fin(node, true);
+ }
+ }
+
+ /* should not be the case */
+ WARN_ON(timer_pending(&dlm_retransmit_timer));
+ dlm_lowcomms_shutdown();
+
+ for (i = 0; i < CONN_HASH_SIZE; i++) {
+ hlist_for_each_entry_rcu(node, &node_hash[i], hlist) {
+ spin_lock(&nodes_lock);
+ hlist_del_rcu(&node->hlist);
+ spin_unlock(&nodes_lock);
+
+ call_srcu(&nodes_srcu, &node->rcu, midcomms_node_release);
+ }
+ }
+ srcu_read_unlock(&nodes_srcu, idx);
+ mutex_unlock(&close_lock);
+}
+
+int dlm_midcomms_close(int nodeid)
+{
+ struct midcomms_node *node;
+ int ret, idx;
+
+ if (nodeid == dlm_our_nodeid())
+ return 0;
+
+ idx = srcu_read_lock(&nodes_srcu);
+ /* Abort pending close/remove operation */
+ node = nodeid2node(nodeid, 0);
+ if (node) {
+ set_bit(DLM_NODE_FLAG_STOP_TX, &node->flags);
+ set_bit(DLM_NODE_FLAG_CLOSE, &node->flags);
+ wake_up(&node->fin_wait);
+ }
+
+ mutex_lock(&close_lock);
+ node = nodeid2node(nodeid, 0);
+ if (!node) {
+ mutex_unlock(&close_lock);
+ srcu_read_unlock(&nodes_srcu, idx);
+ return dlm_lowcomms_close(nodeid);
+ }
+
+ spin_lock(&nodes_lock);
+ hlist_del_rcu(&node->hlist);
+ spin_unlock(&nodes_lock);
+
+ srcu_read_unlock(&nodes_srcu, idx);
+ /* wait until there are all readers left e.g. timer */
+ mutex_unlock(&close_lock);
+
+ ret = dlm_lowcomms_close(nodeid);
+ set_bit(DLM_NODE_FLAG_STOP_RX, &node->flags);
+ call_srcu(&nodes_srcu, &node->rcu, midcomms_node_release);
+ return ret;
+}
diff --git a/fs/dlm/rcom.c b/fs/dlm/rcom.c
index 06f7a5f1d99d..d731d9e65c3b 100644
--- a/fs/dlm/rcom.c
+++ b/fs/dlm/rcom.c
@@ -168,7 +168,7 @@ int dlm_rcom_status(struct dlm_ls *ls, int nodeid, uint32_t status_flags)
set_rcom_status(ls, (struct rcom_status *)rc->rc_buf, status_flags);
allow_sync_reply(ls, &rc->rc_id);
- memset(ls->ls_recover_buf, 0, LOWCOMMS_MAX_TX_BUFFER_LEN);
+ memset(ls->ls_recover_buf, 0, DEFAULT_BUFFER_SIZE);
send_rcom(ls, mh, rc, true);
@@ -291,7 +291,7 @@ int dlm_rcom_names(struct dlm_ls *ls, int nodeid, char *last_name, int last_len)
memcpy(rc->rc_buf, last_name, last_len);
allow_sync_reply(ls, &rc->rc_id);
- memset(ls->ls_recover_buf, 0, LOWCOMMS_MAX_TX_BUFFER_LEN);
+ memset(ls->ls_recover_buf, 0, DEFAULT_BUFFER_SIZE);
send_rcom(ls, mh, rc, true);
--
2.26.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 8/8] fs: dlm: don't allow half transmitted messages
2021-03-26 17:33 [Cluster-devel] [PATCHv3 dlm/next 0/8] fs: dlm: introduce dlm re-transmission layer Alexander Aring
` (6 preceding siblings ...)
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect Alexander Aring
@ 2021-03-26 17:33 ` Alexander Aring
7 siblings, 0 replies; 21+ messages in thread
From: Alexander Aring @ 2021-03-26 17:33 UTC (permalink / raw)
To: cluster-devel.redhat.com
This patch will clean a dirty page buffer if a reconnect occurs. If a page
buffer was half transmitted we cannot start inside the middle of a dlm
message if a node connects again. I observed invalid length receptions
errors and was guessing that this behaviour occurs, after this patch I
never saw an invalid message length again. This patch might drops more
messages for dlm version 3.1 but 3.1 can't deal with half messages as
well, for 3.2 it might trigger more re-transmissions but will not leave dlm
in a broken state.
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
fs/dlm/lowcomms.c | 84 ++++++++++++++++++++++++++++++-----------------
1 file changed, 54 insertions(+), 30 deletions(-)
diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index 5cb6d7c9ddc1..d70a554c6b2b 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -112,6 +112,7 @@ struct writequeue_entry {
int len;
int end;
int users;
+ bool dirty;
struct connection *con;
struct list_head msgs;
struct kref ref;
@@ -671,6 +672,36 @@ static void make_sockaddr(struct sockaddr_storage *saddr, uint16_t port,
memset((char *)saddr + *addr_len, 0, sizeof(struct sockaddr_storage) - *addr_len);
}
+static void dlm_page_release(struct kref *kref)
+{
+ struct writequeue_entry *e = container_of(kref, struct writequeue_entry,
+ ref);
+
+ __free_page(e->page);
+ kfree(e);
+}
+
+static void dlm_msg_release(struct kref *kref)
+{
+ struct dlm_msg *msg = container_of(kref, struct dlm_msg, ref);
+
+ kref_put(&msg->entry->ref, dlm_page_release);
+ kfree(msg);
+}
+
+static void free_entry(struct writequeue_entry *e)
+{
+ struct dlm_msg *msg, *tmp;
+
+ list_for_each_entry_safe(msg, tmp, &e->msgs, list) {
+ list_del(&msg->list);
+ kref_put(&msg->ref, dlm_msg_release);
+ }
+
+ list_del(&e->list);
+ kref_put(&e->ref, dlm_page_release);
+}
+
static void dlm_close_sock(struct socket **sock)
{
if (*sock) {
@@ -685,6 +716,7 @@ static void close_connection(struct connection *con, bool and_other,
bool tx, bool rx)
{
bool closing = test_and_set_bit(CF_CLOSING, &con->flags);
+ struct writequeue_entry *e;
if (tx && !closing && cancel_work_sync(&con->swork)) {
log_print("canceled swork for node %d", con->nodeid);
@@ -703,6 +735,26 @@ static void close_connection(struct connection *con, bool and_other,
close_connection(con->othercon, false, true, true);
}
+ /* if we send a writequeue entry only a half way, we drop the
+ * whole entry because reconnection and that we not start of the
+ * middle of a msg which will confuse the other end.
+ *
+ * we can always drop messages because retransmits, but what we
+ * cannot allow is to transmit half messages which may be processed
+ * at the other side.
+ *
+ * our policy is to start on a clean state when disconnects, we don't
+ * know what's send/received on transport layer in this case.
+ */
+ spin_lock_bh(&con->writequeue_lock);
+ if (!list_empty(&con->writequeue)) {
+ e = list_first_entry(&con->writequeue, struct writequeue_entry,
+ list);
+ if (e->dirty)
+ free_entry(e);
+ }
+ spin_unlock_bh(&con->writequeue_lock);
+
con->rx_leftover = 0;
con->retries = 0;
clear_bit(CF_CONNECTED, &con->flags);
@@ -978,36 +1030,6 @@ static int accept_from_sock(struct listen_connection *con)
return result;
}
-static void dlm_page_release(struct kref *kref)
-{
- struct writequeue_entry *e = container_of(kref, struct writequeue_entry,
- ref);
-
- __free_page(e->page);
- kfree(e);
-}
-
-static void dlm_msg_release(struct kref *kref)
-{
- struct dlm_msg *msg = container_of(kref, struct dlm_msg, ref);
-
- kref_put(&msg->entry->ref, dlm_page_release);
- kfree(msg);
-}
-
-static void free_entry(struct writequeue_entry *e)
-{
- struct dlm_msg *msg, *tmp;
-
- list_for_each_entry_safe(msg, tmp, &e->msgs, list) {
- list_del(&msg->list);
- kref_put(&msg->ref, dlm_msg_release);
- }
-
- list_del(&e->list);
- kref_put(&e->ref, dlm_page_release);
-}
-
/*
* writequeue_entry_complete - try to delete and free write queue entry
* @e: write queue entry to try to delete
@@ -1019,6 +1041,8 @@ static void writequeue_entry_complete(struct writequeue_entry *e, int completed)
{
e->offset += completed;
e->len -= completed;
+ /* signal that page was half way transmitted */
+ e->dirty = true;
if (e->len == 0 && e->users == 0)
free_entry(e);
--
2.26.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect Alexander Aring
@ 2021-04-02 20:53 ` Guillaume Nault
2021-04-03 15:34 ` Alexander Ahring Oder Aring
0 siblings, 1 reply; 21+ messages in thread
From: Guillaume Nault @ 2021-04-02 20:53 UTC (permalink / raw)
To: cluster-devel.redhat.com
On Fri, Mar 26, 2021 at 01:33:36PM -0400, Alexander Aring wrote:
> This patch introduce to make a tcp lowcomms connection reliable even if
> reconnects occurs. This is done by an application layer re-transmission
> handling and sequence numbers in dlm protocols. There are three new dlm
> commands:
>
> DLM_OPTS:
>
> This will encapsulate an existing dlm message (and rcom message if they
> don't have an own application side re-transmission handling). As optional
> handling additional tlv's (type length fields) can be appended. This can
> be for example a sequence number field. However because in DLM_OPTS the
> lockspace field is unused and a sequence number is a mandatory field it
> isn't made as a tlv and we put the sequence number inside the lockspace
> id. The possibility to add optional options are still there for future
> purposes.
>
> DLM_ACK:
>
> Just a dlm header to acknowledge the receive of a DLM_OPTS message to
> it's sender.
>
> DLM_FIN:
>
> A new DLM message to synchronize pending message to the other dlm end if
> the node want to disconnects. Each side waits until it receives this
> message and disconnects. It's important that this message has nothing to
> do with the application logic because it might run in a timeout if
> acknowledge messages are dropped. The problem which we try to solve with
> DLM_FIN is that the node membership is handled by corosync and not the
> kernel dlm protocol itself, DLM_FIN tries to synchronize the kernel dlm
> protocol with corosync join/leave membership callbacks.
If I understand correctly, currently, when DLM faces a hard but
temporary connection failure (like receiving a stray TCP RST), it
automatically recreates a new connection. However, the in-flight data
of the previous connection are lost. The problem is that such data loss
can turn the participating nodes into inconsistent states.
Therefore this patch series implements sequence number tracking at the
application level, so that a new connection can retransmit
unacknowledged data from the previous, failed, connection.
Is that an accurate summary or is there anything I've missed?
I feel that this patch goes very far into re-implementing TCP-like
features. For example, why is fast-retransmit even needed? DLM seems to
always uses a transport protocol that guarantees reliable and in order
delivery. Therefore, duplicate DLM acknowledgements can't happen on an
established connection. Even when reconnecting, it'd be the sender's
responsibility to resend the last unackowledged data first. How could
this lead to holes in the DLM sequence numbers on the receiver's side?
It seems to me that the only time DLM might need to retransmit data, is
when recovering from a connection failure. So why can't we just resend
unacknowledged data at reconnection time? That'd probably simplify the
code a lot (no need to maintain a retransmission timeout on TX, no need
to handle sequence numbers that are in the future on RX).
Also, couldn't we set the DLM sequence numbers in
dlm_midcomms_commit_buffer_3_2() rather than using a callback function
in dlm_lowcomms_new_buffer()?
Finally, I wonder if we could avoid adding DLM sequence numbers
entirely. Have you considered using the TCP_REPAIR infrastructure to
retrieve the send queue of the failed socket and resend that data once
the new socket is connected?
Please correct me if the above suggestions don't make sense. I'm not
familiar with DLM so I can very well be missing important points.
Thanks,
> To explain the basic functionality take a look into the
> dlm_midcomms_receive_buffer() function. This function will take care
> that dlm messages are delivered according to their sequence numbers and
> request re-transmission via sending acknowledge messages. However there
> exists three cases:
>
> 1. sequence number is the one which is expected. That means everything
> is working fine. Additional there is always a check if the next
> message was already queued for future, this will occur when there was
> some messages drops before.
>
> 2. A sequence number is in the future, in this case we queue it for might
> future delivery, see case 1.
>
> 3. A sequence number is in the past, in this case we drop this message
> because it was already delivered.
>
> To send acknowledge we always send the sequence number which is
> expected, if the other node sends multiple acknowledge for the same
> sequence numbers it will trigger a re-transmission. In case no acknowledge
> is send back, a timer with a timeout handling is running and will trigger
> a re-tranmission as well. Sending multiple ack's with the same sequence or
> messages with the same sequence should not have any effects that breaks
> dlm. Only messages in the far future can break dlm, that's why important
> that the closing connection is right synchronized with DLM_FIN which
> also resets the sequence numbers.
>
> As RCOM_STATUS and RCOM_NAMES messages are the first messages which are
> exchanged and they have they own re-transmission handling, there exists
> logic that these messages must be first. If these messages arrives we
> store the dlm version field. This handling is on DLM 3.1 and after this
> patch 3.2 the same. A backwards compatibility handling has been added
> which seems to work on tests without tcpkill, however it's not recommended
> to use DLM 3.1 and 3.2 at the same time, because DLM 3.2 tries to fix long
> term bugs in the DLM protocol.
>
> Signed-off-by: Alexander Aring <aahringo@redhat.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect
2021-04-02 20:53 ` Guillaume Nault
@ 2021-04-03 15:34 ` Alexander Ahring Oder Aring
2021-04-05 17:33 ` Alexander Ahring Oder Aring
2021-04-09 20:32 ` Guillaume Nault
0 siblings, 2 replies; 21+ messages in thread
From: Alexander Ahring Oder Aring @ 2021-04-03 15:34 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi Guillaume,
On Fri, Apr 2, 2021 at 4:54 PM Guillaume Nault <gnault@redhat.com> wrote:
>
> On Fri, Mar 26, 2021 at 01:33:36PM -0400, Alexander Aring wrote:
> > This patch introduce to make a tcp lowcomms connection reliable even if
> > reconnects occurs. This is done by an application layer re-transmission
> > handling and sequence numbers in dlm protocols. There are three new dlm
> > commands:
> >
> > DLM_OPTS:
> >
> > This will encapsulate an existing dlm message (and rcom message if they
> > don't have an own application side re-transmission handling). As optional
> > handling additional tlv's (type length fields) can be appended. This can
> > be for example a sequence number field. However because in DLM_OPTS the
> > lockspace field is unused and a sequence number is a mandatory field it
> > isn't made as a tlv and we put the sequence number inside the lockspace
> > id. The possibility to add optional options are still there for future
> > purposes.
> >
> > DLM_ACK:
> >
> > Just a dlm header to acknowledge the receive of a DLM_OPTS message to
> > it's sender.
> >
> > DLM_FIN:
> >
> > A new DLM message to synchronize pending message to the other dlm end if
> > the node want to disconnects. Each side waits until it receives this
> > message and disconnects. It's important that this message has nothing to
> > do with the application logic because it might run in a timeout if
> > acknowledge messages are dropped. The problem which we try to solve with
> > DLM_FIN is that the node membership is handled by corosync and not the
> > kernel dlm protocol itself, DLM_FIN tries to synchronize the kernel dlm
> > protocol with corosync join/leave membership callbacks.
>
> If I understand correctly, currently, when DLM faces a hard but
> temporary connection failure (like receiving a stray TCP RST), it
> automatically recreates a new connection. However, the in-flight data
> of the previous connection are lost. The problem is that such data loss
> can turn the participating nodes into inconsistent states.
>
> Therefore this patch series implements sequence number tracking at the
> application level, so that a new connection can retransmit
> unacknowledged data from the previous, failed, connection.
>
> Is that an accurate summary or is there anything I've missed?
>
Yes, there is one thing missing. I detected a synchronization issue
because the node membership of a lockspace is done by a different
protocol handled in user space. This protocol declares the actual peer
of the DLM protocol. In short, there are two protocols here, whereas
one handles the membership of peers and the other handles the actual
operations. There is a missing synchronization between operations and
membership which I solved with a DLM_FIN message.
> I feel that this patch goes very far into re-implementing TCP-like
> features. For example, why is fast-retransmit even needed? DLM seems to
> always uses a transport protocol that guarantees reliable and in order
> delivery. Therefore, duplicate DLM acknowledgements can't happen on an
> established connection. Even when reconnecting, it'd be the sender's
> responsibility to resend the last unackowledged data first. How could
> this lead to holes in the DLM sequence numbers on the receiver's side?
>
I agree it's a TCP re-implementation on the application layer.
Fast-retransmit is not needed as it is not needed in TCP. However it
solves drops faster, in DLM and GFS2 you would experience a stuck in
some filesystem syscalls until the drop is resolved.
> It seems to me that the only time DLM might need to retransmit data, is
> when recovering from a connection failure. So why can't we just resend
> unacknowledged data at reconnection time? That'd probably simplify the
> code a lot (no need to maintain a retransmission timeout on TX, no need
> to handle sequence numbers that are in the future on RX).
>
I can try to remove the timer, timeout and do the above approach to
retransmit at reconnect. Then I test it again and I will report back
to see if it works or why we have other problems.
> Also, couldn't we set the DLM sequence numbers in
> dlm_midcomms_commit_buffer_3_2() rather than using a callback function
> in dlm_lowcomms_new_buffer()?
>
That has something to do with the internal buffer allocator. The order
is defined in dlm_lowcomms_new_buffer() and there is an internal lock
which needs to be held there. I can't do this without somehow making
the lock accessible in dlm_midcomms_commit_buffer_3_2(). The code is
also optimized that the lock isn't held during kmalloc().
> Finally, I wonder if we could avoid adding DLM sequence numbers
> entirely. Have you considered using the TCP_REPAIR infrastructure to
> retrieve the send queue of the failed socket and resend that data once
> the new socket is connected?
>
Yes, I looked into TCP_REPAIR at first and I agree it can be used to
solve this problem. However TCP_REPAIR can be used as a part of a more
generic solution, there needs to be something "additional handling"
done e.g. additional socket options to let the application layer save
states before receiving errors. I am also concerned how it would work
with haproxy, etc. It also has some advantages to restore window size,
etc. I was thinking about submitting a proposal to the next netdevconf
after doing some experiments with it.
We decided to solve this problem at the application layer first, then
look into how to make a more generic solution. For me, I am not sure
how cifs deals with reconnects or it just allows drops.
> Please correct me if the above suggestions don't make sense. I'm not
> familiar with DLM so I can very well be missing important points.
>
No problem, it does make sense for me.
- Alex
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect
2021-04-03 15:34 ` Alexander Ahring Oder Aring
@ 2021-04-05 17:33 ` Alexander Ahring Oder Aring
2021-04-05 20:29 ` Alexander Ahring Oder Aring
2021-04-09 20:44 ` Guillaume Nault
2021-04-09 20:32 ` Guillaume Nault
1 sibling, 2 replies; 21+ messages in thread
From: Alexander Ahring Oder Aring @ 2021-04-05 17:33 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
On Sat, Apr 3, 2021 at 11:34 AM Alexander Ahring Oder Aring
<aahringo@redhat.com> wrote:
>
...
>
> > It seems to me that the only time DLM might need to retransmit data, is
> > when recovering from a connection failure. So why can't we just resend
> > unacknowledged data at reconnection time? That'd probably simplify the
> > code a lot (no need to maintain a retransmission timeout on TX, no need
> > to handle sequence numbers that are in the future on RX).
> >
>
> I can try to remove the timer, timeout and do the above approach to
> retransmit at reconnect. Then I test it again and I will report back
> to see if it works or why we have other problems.
>
I have an implementation of this running and so far I don't see any problems.
> > Also, couldn't we set the DLM sequence numbers in
> > dlm_midcomms_commit_buffer_3_2() rather than using a callback function
> > in dlm_lowcomms_new_buffer()?
> >
...
>
> Yes, I looked into TCP_REPAIR at first and I agree it can be used to
> solve this problem. However TCP_REPAIR can be used as a part of a more
> generic solution, there needs to be something "additional handling"
> done e.g. additional socket options to let the application layer save
> states before receiving errors. I am also concerned how it would work
The code [0] is what I meant above. It will call
tcp_write_queue_purge(); before reporting the error over error
queue/callback. That need to be handled differently to allow dumping
the actual TCP state and restore at reconnect, at least that is what I
have in my mind.
- Alex
[0] https://elixir.bootlin.com/linux/v5.12-rc6/source/net/ipv4/tcp_input.c#L4239
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect
2021-04-05 17:33 ` Alexander Ahring Oder Aring
@ 2021-04-05 20:29 ` Alexander Ahring Oder Aring
2021-04-09 21:11 ` Guillaume Nault
2021-04-09 20:44 ` Guillaume Nault
1 sibling, 1 reply; 21+ messages in thread
From: Alexander Ahring Oder Aring @ 2021-04-05 20:29 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
On Mon, Apr 5, 2021 at 1:33 PM Alexander Ahring Oder Aring
<aahringo@redhat.com> wrote:
>
> Hi,
>
> On Sat, Apr 3, 2021 at 11:34 AM Alexander Ahring Oder Aring
> <aahringo@redhat.com> wrote:
> >
> ...
> >
> > > It seems to me that the only time DLM might need to retransmit data, is
> > > when recovering from a connection failure. So why can't we just resend
> > > unacknowledged data at reconnection time? That'd probably simplify the
> > > code a lot (no need to maintain a retransmission timeout on TX, no need
> > > to handle sequence numbers that are in the future on RX).
> > >
> >
> > I can try to remove the timer, timeout and do the above approach to
> > retransmit at reconnect. Then I test it again and I will report back
> > to see if it works or why we have other problems.
> >
>
> I have an implementation of this running and so far I don't see any problems.
There is a problem but it's related to the behaviour how reconnections
are triggered. The whole communication can be stuck because the send()
triggers a reconnection if not connected anymore. Before, the timer
was triggering some send() and this was triggering a reconnection in a
periodic way. Therefore we never had any stuck situation where nobody
was sending anything anymore. It's a rare case but I am currently
running into it. However I think I need to change how the
reconnections are triggered with some "forever periodic try" which
should solve this issue.
- Alex
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect
2021-04-03 15:34 ` Alexander Ahring Oder Aring
2021-04-05 17:33 ` Alexander Ahring Oder Aring
@ 2021-04-09 20:32 ` Guillaume Nault
2021-04-12 15:21 ` Alexander Ahring Oder Aring
1 sibling, 1 reply; 21+ messages in thread
From: Guillaume Nault @ 2021-04-09 20:32 UTC (permalink / raw)
To: cluster-devel.redhat.com
On Sat, Apr 03, 2021 at 11:34:33AM -0400, Alexander Ahring Oder Aring wrote:
> Hi Guillaume,
>
> On Fri, Apr 2, 2021 at 4:54 PM Guillaume Nault <gnault@redhat.com> wrote:
> >
> > On Fri, Mar 26, 2021 at 01:33:36PM -0400, Alexander Aring wrote:
> > > This patch introduce to make a tcp lowcomms connection reliable even if
> > > reconnects occurs. This is done by an application layer re-transmission
> > > handling and sequence numbers in dlm protocols. There are three new dlm
> > > commands:
> > >
> > > DLM_OPTS:
> > >
> > > This will encapsulate an existing dlm message (and rcom message if they
> > > don't have an own application side re-transmission handling). As optional
> > > handling additional tlv's (type length fields) can be appended. This can
> > > be for example a sequence number field. However because in DLM_OPTS the
> > > lockspace field is unused and a sequence number is a mandatory field it
> > > isn't made as a tlv and we put the sequence number inside the lockspace
> > > id. The possibility to add optional options are still there for future
> > > purposes.
> > >
> > > DLM_ACK:
> > >
> > > Just a dlm header to acknowledge the receive of a DLM_OPTS message to
> > > it's sender.
> > >
> > > DLM_FIN:
> > >
> > > A new DLM message to synchronize pending message to the other dlm end if
> > > the node want to disconnects. Each side waits until it receives this
> > > message and disconnects. It's important that this message has nothing to
> > > do with the application logic because it might run in a timeout if
> > > acknowledge messages are dropped. The problem which we try to solve with
> > > DLM_FIN is that the node membership is handled by corosync and not the
> > > kernel dlm protocol itself, DLM_FIN tries to synchronize the kernel dlm
> > > protocol with corosync join/leave membership callbacks.
> >
> > If I understand correctly, currently, when DLM faces a hard but
> > temporary connection failure (like receiving a stray TCP RST), it
> > automatically recreates a new connection. However, the in-flight data
> > of the previous connection are lost. The problem is that such data loss
> > can turn the participating nodes into inconsistent states.
> >
> > Therefore this patch series implements sequence number tracking at the
> > application level, so that a new connection can retransmit
> > unacknowledged data from the previous, failed, connection.
> >
> > Is that an accurate summary or is there anything I've missed?
> >
>
> Yes, there is one thing missing. I detected a synchronization issue
> because the node membership of a lockspace is done by a different
> protocol handled in user space. This protocol declares the actual peer
> of the DLM protocol. In short, there are two protocols here, whereas
> one handles the membership of peers and the other handles the actual
> operations. There is a missing synchronization between operations and
> membership which I solved with a DLM_FIN message.
Thanks, I'll re-read the parts about DLM_FIN with this explanation in
mind.
> > I feel that this patch goes very far into re-implementing TCP-like
> > features. For example, why is fast-retransmit even needed? DLM seems to
> > always uses a transport protocol that guarantees reliable and in order
> > delivery. Therefore, duplicate DLM acknowledgements can't happen on an
> > established connection. Even when reconnecting, it'd be the sender's
> > responsibility to resend the last unackowledged data first. How could
> > this lead to holes in the DLM sequence numbers on the receiver's side?
> >
>
> I agree it's a TCP re-implementation on the application layer.
>
> Fast-retransmit is not needed as it is not needed in TCP. However it
> solves drops faster, in DLM and GFS2 you would experience a stuck in
> some filesystem syscalls until the drop is resolved.
I was not talking about fast retransmits in general. My question was:
how could a DLM fast retransmit even be triggered? This can only happen
when some DLM messages get lost or are received out of order. But the
transport layer guarantees lossless and in order delivery. Even in case
of a reconnection, I can't see how the peer could get holes in its
receive queue. What am I missing?
> > It seems to me that the only time DLM might need to retransmit data, is
> > when recovering from a connection failure. So why can't we just resend
> > unacknowledged data at reconnection time? That'd probably simplify the
> > code a lot (no need to maintain a retransmission timeout on TX, no need
> > to handle sequence numbers that are in the future on RX).
> >
>
> I can try to remove the timer, timeout and do the above approach to
> retransmit at reconnect. Then I test it again and I will report back
> to see if it works or why we have other problems.
I see you've posted a new version of the patch series. I'll look at it
soon.
> > Also, couldn't we set the DLM sequence numbers in
> > dlm_midcomms_commit_buffer_3_2() rather than using a callback function
> > in dlm_lowcomms_new_buffer()?
> >
>
> That has something to do with the internal buffer allocator. The order
> is defined in dlm_lowcomms_new_buffer() and there is an internal lock
> which needs to be held there. I can't do this without somehow making
> the lock accessible in dlm_midcomms_commit_buffer_3_2(). The code is
> also optimized that the lock isn't held during kmalloc().
I missed that. Thanks for the explanation.
> > Finally, I wonder if we could avoid adding DLM sequence numbers
> > entirely. Have you considered using the TCP_REPAIR infrastructure to
> > retrieve the send queue of the failed socket and resend that data once
> > the new socket is connected?
> >
>
> Yes, I looked into TCP_REPAIR at first and I agree it can be used to
> solve this problem. However TCP_REPAIR can be used as a part of a more
> generic solution, there needs to be something "additional handling"
> done e.g. additional socket options to let the application layer save
> states before receiving errors. I am also concerned how it would work
> with haproxy, etc. It also has some advantages to restore window size,
> etc. I was thinking about submitting a proposal to the next netdevconf
> after doing some experiments with it.
Well, I was thinking of using TCP_REPAIR only to retrieve the send
queue. I didn't mean to avoid the hand-check like in the CRIU use case.
A TCP connection break is a sign that something went wrong between the
peers, so a full reconnection is in order IMHO.
> We decided to solve this problem at the application layer first, then
> look into how to make a more generic solution. For me, I am not sure
> how cifs deals with reconnects or it just allows drops.
Indeed, it'd be interesting to know how other protocols handle this
case.
> > Please correct me if the above suggestions don't make sense. I'm not
> > familiar with DLM so I can very well be missing important points.
> >
>
> No problem, it does make sense for me.
>
> - Alex
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect
2021-04-05 17:33 ` Alexander Ahring Oder Aring
2021-04-05 20:29 ` Alexander Ahring Oder Aring
@ 2021-04-09 20:44 ` Guillaume Nault
2021-04-12 15:30 ` Alexander Ahring Oder Aring
1 sibling, 1 reply; 21+ messages in thread
From: Guillaume Nault @ 2021-04-09 20:44 UTC (permalink / raw)
To: cluster-devel.redhat.com
On Mon, Apr 05, 2021 at 01:33:48PM -0400, Alexander Ahring Oder Aring wrote:
> Hi,
>
> On Sat, Apr 3, 2021 at 11:34 AM Alexander Ahring Oder Aring
> <aahringo@redhat.com> wrote:
> >
> ...
> >
> > > It seems to me that the only time DLM might need to retransmit data, is
> > > when recovering from a connection failure. So why can't we just resend
> > > unacknowledged data at reconnection time? That'd probably simplify the
> > > code a lot (no need to maintain a retransmission timeout on TX, no need
> > > to handle sequence numbers that are in the future on RX).
> > >
> >
> > I can try to remove the timer, timeout and do the above approach to
> > retransmit at reconnect. Then I test it again and I will report back
> > to see if it works or why we have other problems.
> >
>
> I have an implementation of this running and so far I don't see any problems.
>
> > > Also, couldn't we set the DLM sequence numbers in
> > > dlm_midcomms_commit_buffer_3_2() rather than using a callback function
> > > in dlm_lowcomms_new_buffer()?
> > >
> ...
> >
> > Yes, I looked into TCP_REPAIR at first and I agree it can be used to
> > solve this problem. However TCP_REPAIR can be used as a part of a more
> > generic solution, there needs to be something "additional handling"
> > done e.g. additional socket options to let the application layer save
> > states before receiving errors. I am also concerned how it would work
>
> The code [0] is what I meant above. It will call
> tcp_write_queue_purge(); before reporting the error over error
> queue/callback. That need to be handled differently to allow dumping
> the actual TCP state and restore at reconnect, at least that is what I
> have in my mind.
Thanks. That's not usable as is, indeed.
Also, by retransmitting data from the previous send-queue, we risk
resending messages that the peer already received (for example because
the previous connection didn't receive the latest ACKs). I guess that
receiving the same DLM messages twice is going to confuse the peer.
So it looks like we'll need application level sequence numbers anyway.
> - Alex
>
> [0] https://elixir.bootlin.com/linux/v5.12-rc6/source/net/ipv4/tcp_input.c#L4239
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect
2021-04-05 20:29 ` Alexander Ahring Oder Aring
@ 2021-04-09 21:11 ` Guillaume Nault
2021-04-12 15:35 ` Alexander Ahring Oder Aring
0 siblings, 1 reply; 21+ messages in thread
From: Guillaume Nault @ 2021-04-09 21:11 UTC (permalink / raw)
To: cluster-devel.redhat.com
On Mon, Apr 05, 2021 at 04:29:10PM -0400, Alexander Ahring Oder Aring wrote:
> Hi,
>
> On Mon, Apr 5, 2021 at 1:33 PM Alexander Ahring Oder Aring
> <aahringo@redhat.com> wrote:
> >
> > Hi,
> >
> > On Sat, Apr 3, 2021 at 11:34 AM Alexander Ahring Oder Aring
> > <aahringo@redhat.com> wrote:
> > >
> > ...
> > >
> > > > It seems to me that the only time DLM might need to retransmit data, is
> > > > when recovering from a connection failure. So why can't we just resend
> > > > unacknowledged data at reconnection time? That'd probably simplify the
> > > > code a lot (no need to maintain a retransmission timeout on TX, no need
> > > > to handle sequence numbers that are in the future on RX).
> > > >
> > >
> > > I can try to remove the timer, timeout and do the above approach to
> > > retransmit at reconnect. Then I test it again and I will report back
> > > to see if it works or why we have other problems.
> > >
> >
> > I have an implementation of this running and so far I don't see any problems.
>
> There is a problem but it's related to the behaviour how reconnections
> are triggered. The whole communication can be stuck because the send()
> triggers a reconnection if not connected anymore. Before, the timer
> was triggering some send() and this was triggering a reconnection in a
> periodic way. Therefore we never had any stuck situation where nobody
> was sending anything anymore. It's a rare case but I am currently
> running into it. However I think I need to change how the
> reconnections are triggered with some "forever periodic try" which
> should solve this issue.
Would it be sufficient to detect socket errors to avoid this problem?
For example by letting lowcomms_error_report() do the reconnection when
necessary?
>
> - Alex
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect
2021-04-09 20:32 ` Guillaume Nault
@ 2021-04-12 15:21 ` Alexander Ahring Oder Aring
2021-04-21 16:21 ` Alexander Ahring Oder Aring
0 siblings, 1 reply; 21+ messages in thread
From: Alexander Ahring Oder Aring @ 2021-04-12 15:21 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
On Fri, Apr 9, 2021 at 4:32 PM Guillaume Nault <gnault@redhat.com> wrote:
...
> >
> > Yes, there is one thing missing. I detected a synchronization issue
> > because the node membership of a lockspace is done by a different
> > protocol handled in user space. This protocol declares the actual peer
> > of the DLM protocol. In short, there are two protocols here, whereas
> > one handles the membership of peers and the other handles the actual
> > operations. There is a missing synchronization between operations and
> > membership which I solved with a DLM_FIN message.
>
> Thanks, I'll re-read the parts about DLM_FIN with this explanation in
> mind.
>
okay. Thank you.
> > > I feel that this patch goes very far into re-implementing TCP-like
> > > features. For example, why is fast-retransmit even needed? DLM seems to
> > > always uses a transport protocol that guarantees reliable and in order
> > > delivery. Therefore, duplicate DLM acknowledgements can't happen on an
> > > established connection. Even when reconnecting, it'd be the sender's
> > > responsibility to resend the last unackowledged data first. How could
> > > this lead to holes in the DLM sequence numbers on the receiver's side?
> > >
> >
> > I agree it's a TCP re-implementation on the application layer.
> >
> > Fast-retransmit is not needed as it is not needed in TCP. However it
> > solves drops faster, in DLM and GFS2 you would experience a stuck in
> > some filesystem syscalls until the drop is resolved.
>
> I was not talking about fast retransmits in general. My question was:
> how could a DLM fast retransmit even be triggered? This can only happen
> when some DLM messages get lost or are received out of order. But the
> transport layer guarantees lossless and in order delivery. Even in case
> of a reconnection, I can't see how the peer could get holes in its
> receive queue. What am I missing?
>
It can be triggered in cases that DLM just does some other operations
which triggers new messages after a reconnect and drop. The receiver
will send back multiple ack frames back with the same sequence number
which indicates for the sender a drop. If the ack count with multiple
times the same sequence number (the receiver expected sequence number)
a retransmission will be triggered. Just like in [0].
> > > It seems to me that the only time DLM might need to retransmit data, is
> > > when recovering from a connection failure. So why can't we just resend
> > > unacknowledged data at reconnection time? That'd probably simplify the
> > > code a lot (no need to maintain a retransmission timeout on TX, no need
> > > to handle sequence numbers that are in the future on RX).
> > >
> >
> > I can try to remove the timer, timeout and do the above approach to
> > retransmit at reconnect. Then I test it again and I will report back
> > to see if it works or why we have other problems.
>
> I see you've posted a new version of the patch series. I'll look at it
> soon.
>
ok.
> > > Also, couldn't we set the DLM sequence numbers in
> > > dlm_midcomms_commit_buffer_3_2() rather than using a callback function
> > > in dlm_lowcomms_new_buffer()?
> > >
> >
> > That has something to do with the internal buffer allocator. The order
> > is defined in dlm_lowcomms_new_buffer() and there is an internal lock
> > which needs to be held there. I can't do this without somehow making
> > the lock accessible in dlm_midcomms_commit_buffer_3_2(). The code is
> > also optimized that the lock isn't held during kmalloc().
>
> I missed that. Thanks for the explanation.
>
ok, no problem. It took some time for me to figure that out as well.
> > > Finally, I wonder if we could avoid adding DLM sequence numbers
> > > entirely. Have you considered using the TCP_REPAIR infrastructure to
> > > retrieve the send queue of the failed socket and resend that data once
> > > the new socket is connected?
> > >
> >
> > Yes, I looked into TCP_REPAIR at first and I agree it can be used to
> > solve this problem. However TCP_REPAIR can be used as a part of a more
> > generic solution, there needs to be something "additional handling"
> > done e.g. additional socket options to let the application layer save
> > states before receiving errors. I am also concerned how it would work
> > with haproxy, etc. It also has some advantages to restore window size,
> > etc. I was thinking about submitting a proposal to the next netdevconf
> > after doing some experiments with it.
>
> Well, I was thinking of using TCP_REPAIR only to retrieve the send
> queue. I didn't mean to avoid the hand-check like in the CRIU use case.
> A TCP connection break is a sign that something went wrong between the
> peers, so a full reconnection is in order IMHO.
>
I am somehow worried about what data-format they are in the "queues"
and even if we can figure out message boundaries? Yes "queues" so far
I remember/know there exists two queues for TCP, a write queue and
retransmit queue. I am not sure if the data payload is with TCP header
or not, something to figure out. Otherwise we would need to parse TCP
here?
> > We decided to solve this problem at the application layer first, then
> > look into how to make a more generic solution. For me, I am not sure
> > how cifs deals with reconnects or it just allows drops.
>
> Indeed, it'd be interesting to know how other protocols handle this
> case.
>
Yes, this protocol is very picky if there is any drop. Of course we
are talking about lock operations over the network, it can be very
fatal if drops occur. Pablo review mentions a lot of "security"
related things and disconnecting peers if they happen. We work here in
the cluster world which operates a little bit differently. What we can
do is send an event to the cluster manager to fence nodes. However I
am not aware that we currently support such handling if something runs
out-of-boundaries of queues, sequence numbers, etc. We need to trust
the network anyway because you can easily trigger deadlocks from
outside, I guess. Trusting network != things run crazy, for the crazy
case we could send events to fence a specific node.
- Alex
[0] https://www.isi.edu/nsnam/DIRECTED_RESEARCH/DR_WANIDA/DR/images/fast-retransmit-2.gif
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect
2021-04-09 20:44 ` Guillaume Nault
@ 2021-04-12 15:30 ` Alexander Ahring Oder Aring
2021-04-12 15:42 ` Alexander Ahring Oder Aring
0 siblings, 1 reply; 21+ messages in thread
From: Alexander Ahring Oder Aring @ 2021-04-12 15:30 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
On Fri, Apr 9, 2021 at 4:44 PM Guillaume Nault <gnault@redhat.com> wrote:
>
> On Mon, Apr 05, 2021 at 01:33:48PM -0400, Alexander Ahring Oder Aring wrote:
> > Hi,
> >
> > On Sat, Apr 3, 2021 at 11:34 AM Alexander Ahring Oder Aring
> > <aahringo@redhat.com> wrote:
> > >
> > ...
> > >
> > > > It seems to me that the only time DLM might need to retransmit data, is
> > > > when recovering from a connection failure. So why can't we just resend
> > > > unacknowledged data at reconnection time? That'd probably simplify the
> > > > code a lot (no need to maintain a retransmission timeout on TX, no need
> > > > to handle sequence numbers that are in the future on RX).
> > > >
> > >
> > > I can try to remove the timer, timeout and do the above approach to
> > > retransmit at reconnect. Then I test it again and I will report back
> > > to see if it works or why we have other problems.
> > >
> >
> > I have an implementation of this running and so far I don't see any problems.
> >
> > > > Also, couldn't we set the DLM sequence numbers in
> > > > dlm_midcomms_commit_buffer_3_2() rather than using a callback function
> > > > in dlm_lowcomms_new_buffer()?
> > > >
> > ...
> > >
> > > Yes, I looked into TCP_REPAIR at first and I agree it can be used to
> > > solve this problem. However TCP_REPAIR can be used as a part of a more
> > > generic solution, there needs to be something "additional handling"
> > > done e.g. additional socket options to let the application layer save
> > > states before receiving errors. I am also concerned how it would work
> >
> > The code [0] is what I meant above. It will call
> > tcp_write_queue_purge(); before reporting the error over error
> > queue/callback. That need to be handled differently to allow dumping
> > the actual TCP state and restore at reconnect, at least that is what I
> > have in my mind.
>
> Thanks. That's not usable as is, indeed.
> Also, by retransmitting data from the previous send-queue, we risk
> resending messages that the peer already received (for example because
> the previous connection didn't receive the latest ACKs). I guess that
> receiving the same DLM messages twice is going to confuse the peer.
> So it looks like we'll need application level sequence numbers anyway.
I agree, the new "retransmit all unacknowledged messages on reconnect"
method will filter at the receiving side the already received messages
because they have the sequence numbers, this case occurs a lot.
However I think there is still the possibility to use TCP_REPAIR here,
we need to restore states about all 3 queues, rx, tx (write,
retransmit) and sequence numbers. Window size is an optional
additional thing. On the application layer we need to be sure that we
don't drop anything if error occurs and start to transmit them after
restoring the state again. Of course both endpoints need to support it
and have been correctly configured.
- Alex
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect
2021-04-09 21:11 ` Guillaume Nault
@ 2021-04-12 15:35 ` Alexander Ahring Oder Aring
0 siblings, 0 replies; 21+ messages in thread
From: Alexander Ahring Oder Aring @ 2021-04-12 15:35 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
On Fri, Apr 9, 2021 at 5:11 PM Guillaume Nault <gnault@redhat.com> wrote:
>
> On Mon, Apr 05, 2021 at 04:29:10PM -0400, Alexander Ahring Oder Aring wrote:
> > Hi,
> >
> > On Mon, Apr 5, 2021 at 1:33 PM Alexander Ahring Oder Aring
> > <aahringo@redhat.com> wrote:
> > >
> > > Hi,
> > >
> > > On Sat, Apr 3, 2021 at 11:34 AM Alexander Ahring Oder Aring
> > > <aahringo@redhat.com> wrote:
> > > >
> > > ...
> > > >
> > > > > It seems to me that the only time DLM might need to retransmit data, is
> > > > > when recovering from a connection failure. So why can't we just resend
> > > > > unacknowledged data at reconnection time? That'd probably simplify the
> > > > > code a lot (no need to maintain a retransmission timeout on TX, no need
> > > > > to handle sequence numbers that are in the future on RX).
> > > > >
> > > >
> > > > I can try to remove the timer, timeout and do the above approach to
> > > > retransmit at reconnect. Then I test it again and I will report back
> > > > to see if it works or why we have other problems.
> > > >
> > >
> > > I have an implementation of this running and so far I don't see any problems.
> >
> > There is a problem but it's related to the behaviour how reconnections
> > are triggered. The whole communication can be stuck because the send()
> > triggers a reconnection if not connected anymore. Before, the timer
> > was triggering some send() and this was triggering a reconnection in a
> > periodic way. Therefore we never had any stuck situation where nobody
> > was sending anything anymore. It's a rare case but I am currently
> > running into it. However I think I need to change how the
> > reconnections are triggered with some "forever periodic try" which
> > should solve this issue.
>
> Would it be sufficient to detect socket errors to avoid this problem?
> For example by letting lowcomms_error_report() do the reconnection when
> necessary?
I have something like that as a patch for afterwards, it also contains
some change in the lowcomms workqueue handling and removal of the
"othercon" race paradigm. There are sometimes two connections
established because of a race which I mentioned in midcomms as well
for version detection. In short every node wants to connect
immediately after the cluster manager reports membership to the kernel
dlm implementation. However I ignored that problem of reconnection for
now as it occurs never/rarely but I think it's still there.
Thanks for your review.
- Alex
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect
2021-04-12 15:30 ` Alexander Ahring Oder Aring
@ 2021-04-12 15:42 ` Alexander Ahring Oder Aring
0 siblings, 0 replies; 21+ messages in thread
From: Alexander Ahring Oder Aring @ 2021-04-12 15:42 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
On Mon, Apr 12, 2021 at 11:30 AM Alexander Ahring Oder Aring
<aahringo@redhat.com> wrote:
>
> Hi,
>
> On Fri, Apr 9, 2021 at 4:44 PM Guillaume Nault <gnault@redhat.com> wrote:
> >
> > On Mon, Apr 05, 2021 at 01:33:48PM -0400, Alexander Ahring Oder Aring wrote:
> > > Hi,
> > >
> > > On Sat, Apr 3, 2021 at 11:34 AM Alexander Ahring Oder Aring
> > > <aahringo@redhat.com> wrote:
> > > >
> > > ...
> > > >
> > > > > It seems to me that the only time DLM might need to retransmit data, is
> > > > > when recovering from a connection failure. So why can't we just resend
> > > > > unacknowledged data at reconnection time? That'd probably simplify the
> > > > > code a lot (no need to maintain a retransmission timeout on TX, no need
> > > > > to handle sequence numbers that are in the future on RX).
> > > > >
> > > >
> > > > I can try to remove the timer, timeout and do the above approach to
> > > > retransmit at reconnect. Then I test it again and I will report back
> > > > to see if it works or why we have other problems.
> > > >
> > >
> > > I have an implementation of this running and so far I don't see any problems.
> > >
> > > > > Also, couldn't we set the DLM sequence numbers in
> > > > > dlm_midcomms_commit_buffer_3_2() rather than using a callback function
> > > > > in dlm_lowcomms_new_buffer()?
> > > > >
> > > ...
> > > >
> > > > Yes, I looked into TCP_REPAIR at first and I agree it can be used to
> > > > solve this problem. However TCP_REPAIR can be used as a part of a more
> > > > generic solution, there needs to be something "additional handling"
> > > > done e.g. additional socket options to let the application layer save
> > > > states before receiving errors. I am also concerned how it would work
> > >
> > > The code [0] is what I meant above. It will call
> > > tcp_write_queue_purge(); before reporting the error over error
> > > queue/callback. That need to be handled differently to allow dumping
> > > the actual TCP state and restore at reconnect, at least that is what I
> > > have in my mind.
> >
> > Thanks. That's not usable as is, indeed.
> > Also, by retransmitting data from the previous send-queue, we risk
> > resending messages that the peer already received (for example because
> > the previous connection didn't receive the latest ACKs). I guess that
> > receiving the same DLM messages twice is going to confuse the peer.
> > So it looks like we'll need application level sequence numbers anyway.
>
> I agree, the new "retransmit all unacknowledged messages on reconnect"
> method will filter at the receiving side the already received messages
> because they have the sequence numbers, this case occurs a lot.
>
> However I think there is still the possibility to use TCP_REPAIR here,
> we need to restore states about all 3 queues, rx, tx (write,
> retransmit) and sequence numbers. Window size is an optional
> additional thing. On the application layer we need to be sure that we
> don't drop anything if error occurs and start to transmit them after
> restoring the state again. Of course both endpoints need to support it
> and have been correctly configured.
I am not sure if this ends in something like "ignore some error cases
in TCP", at least TCP_RST is something which seems to be triggered
sometimes because "smart hardware" in the network e.g. but cable out
and in again (not sure about that one). I think restoring the state
might be work, but transparent proxies (haproxy, etc.) could be
confused? I am not sure here...
- Alex
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect
2021-04-12 15:21 ` Alexander Ahring Oder Aring
@ 2021-04-21 16:21 ` Alexander Ahring Oder Aring
0 siblings, 0 replies; 21+ messages in thread
From: Alexander Ahring Oder Aring @ 2021-04-21 16:21 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
On Mon, Apr 12, 2021 at 11:21 AM Alexander Ahring Oder Aring
<aahringo@redhat.com> wrote:
>
> Hi,
>
> On Fri, Apr 9, 2021 at 4:32 PM Guillaume Nault <gnault@redhat.com> wrote:
> ...
> > >
> > > Yes, there is one thing missing. I detected a synchronization issue
> > > because the node membership of a lockspace is done by a different
> > > protocol handled in user space. This protocol declares the actual peer
> > > of the DLM protocol. In short, there are two protocols here, whereas
> > > one handles the membership of peers and the other handles the actual
> > > operations. There is a missing synchronization between operations and
> > > membership which I solved with a DLM_FIN message.
> >
> > Thanks, I'll re-read the parts about DLM_FIN with this explanation in
> > mind.
> >
>
> okay. Thank you.
>
Additional note:
The DLM_FIN is like a 4 way close handshake with the possibility of
having a half-close socket. We probably can do that on TCP with
shutdown, but not on SCTP. However it's now on the application layer.
- Alex
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2021-04-21 16:21 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-26 17:33 [Cluster-devel] [PATCHv3 dlm/next 0/8] fs: dlm: introduce dlm re-transmission layer Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 1/8] fs: dlm: public header in out utility Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 2/8] fs: dlm: add more midcomms hooks Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 3/8] fs: dlm: make buffer handling per msg Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 4/8] fs: dlm: add functionality to re-transmit a message Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 5/8] fs: dlm: move out some hash functionality Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 6/8] fs: dlm: add union in dlm header for lockspace id Alexander Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 7/8] fs: dlm: add reliable connection if reconnect Alexander Aring
2021-04-02 20:53 ` Guillaume Nault
2021-04-03 15:34 ` Alexander Ahring Oder Aring
2021-04-05 17:33 ` Alexander Ahring Oder Aring
2021-04-05 20:29 ` Alexander Ahring Oder Aring
2021-04-09 21:11 ` Guillaume Nault
2021-04-12 15:35 ` Alexander Ahring Oder Aring
2021-04-09 20:44 ` Guillaume Nault
2021-04-12 15:30 ` Alexander Ahring Oder Aring
2021-04-12 15:42 ` Alexander Ahring Oder Aring
2021-04-09 20:32 ` Guillaume Nault
2021-04-12 15:21 ` Alexander Ahring Oder Aring
2021-04-21 16:21 ` Alexander Ahring Oder Aring
2021-03-26 17:33 ` [Cluster-devel] [PATCHv3 dlm/next 8/8] fs: dlm: don't allow half transmitted messages Alexander Aring
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.