Re: [devel] [PATCH 1/1] osaf: return a help message if no parameter is specified [#3118]
Hi Gary,
ACK from me.
Best Regards,
ThuanTr
-Original Message-
From: Gary Lee
Sent: Wednesday, November 13, 2019 1:19 PM
To: minh.c...@dektech.com.au; thuan.t...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; Gary Lee
Subject: [PATCH 1/1] osaf: return a help message if no parameter is specified
[#3118]
---
src/osaf/consensus/plugins/tcp/tcp.plugin | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/osaf/consensus/plugins/tcp/tcp.plugin
b/src/osaf/consensus/plugins/tcp/tcp.plugin
index 1b5ddf5..0be20fc 100755
--- a/src/osaf/consensus/plugins/tcp/tcp.plugin
+++ b/src/osaf/consensus/plugins/tcp/tcp.plugin
@@ -149,7 +149,12 @@ class ArbitratorPlugin(object):
params = []
if args:
params.append(args)
-return getattr(self, command)(*params)
+if command:
+return getattr(self, command)(*params)
+else:
+ret = {'code': 0,
+ 'output': parser.format_help()}
+return ret
def get_node_name(self):
node_file = open(self.node_name_file)
--
2.7.4
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/1] mds: fix sender take very long time to send all messages [#3119]
Hi Thuan,
ack from me.
THanks
Minh
On 13/11/19 10:00 pm, thuan.tran wrote:
When overload happens, sender will wait for chunkAck to continue
sending more messages, it should send number of message equal chunkAck
size of receiver. If not, receiver don't receive enough messages to send
chunkAck and wait until timer timeout to send chunkAck to sender.
This loop will make sender take very long time to sending all messages.
---
src/mds/mds_tipc_fctrl_portid.cc | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/mds/mds_tipc_fctrl_portid.cc b/src/mds/mds_tipc_fctrl_portid.cc
index 3704baddb..bd1825446 100644
--- a/src/mds/mds_tipc_fctrl_portid.cc
+++ b/src/mds/mds_tipc_fctrl_portid.cc
@@ -190,6 +190,7 @@ uint32_t TipcPortId::Queue(const uint8_t* data, uint16_t
length,
sndwnd_.acked_.v(), sndwnd_.send_.v(), sndwnd_.nacked_space_);
} else {
++sndwnd_.send_;
+sndwnd_.nacked_space_ += length;
m_MDS_LOG_NOTIFY("FCTRL: [me] --> [node:%x, ref:%u], "
"QueData[mseq:%u, mfrag:%u, fseq:%u, len:%u], "
"sndwnd[acked:%u, send:%u, nacked:%" PRIu64 "]",
@@ -444,32 +445,29 @@ void TipcPortId::ReceiveChunkAck(uint16_t fseq, uint16_t
chksize) {
// the nacked_space_ of sender
uint64_t acked_bytes = sndqueue_.Erase(Seq16(fseq) - (chksize-1),
Seq16(fseq));
+assert(sndwnd_.nacked_space_ >= acked_bytes);
sndwnd_.nacked_space_ -= acked_bytes;
// try to send a few pending msg
DataMessage* msg = nullptr;
-uint64_t resend_bytes = 0;
-while (resend_bytes < acked_bytes) {
+uint16_t send_msg_cnt = 0;
+while (send_msg_cnt++ < chunk_size_) {
// find the lowest sequence unsent yet
msg = sndqueue_.FirstUnsent();
if (msg == nullptr) {
break;
} else {
-if (resend_bytes < acked_bytes) {
if (Send(msg->msg_data_, msg->header_.msg_len_) ==
NCSCC_RC_SUCCESS) {
-sndwnd_.nacked_space_ += msg->header_.msg_len_;
msg->is_sent_ = true;
-resend_bytes += msg->header_.msg_len_;
m_MDS_LOG_NOTIFY("FCTRL: [me] --> [node:%x, ref:%u], "
"SndQData[fseq:%u, len:%u], "
"sndwnd[acked:%u, send:%u, nacked:%" PRIu64 "]",
id_.node, id_.ref,
msg->header_.fseq_, msg->header_.msg_len_,
sndwnd_.acked_.v(), sndwnd_.send_.v(), sndwnd_.nacked_space_);
+ } else {
+break;
}
-} else {
- break;
-}
}
}
// no more unsent message, back to kEnabled
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/3] mds: Distinguish protocol version of fragment [#3111]
Hi Thuan,
Please see my reply inline.
Thanks
Minh
On 13/11/19 9:54 pm, Tran Thuan wrote:
Hi Minh,
See my comment inline.
Best Regards,
ThuanTr
-Original Message-
From: Minh Chau
Sent: Friday, November 8, 2019 5:33 PM
To: hans.nordeb...@ericsson.com; gary@dektech.com.au;
vu.m.ngu...@dektech.com.au; thuan.t...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; Minh Chau
Subject: [PATCH 1/3] mds: Distinguish protocol version of fragment [#3111]
The legacy mds encodes the protocol version in either non fragment
message or the first fragment only. Hence, the subsequent fragment
after the first one is not able for mds to determine the protocol
version.
The patch maintains the encoding of lengthcheck as same as the legacy
mds version. Also, the subsequent fragments needs to consult the
stateful portid to determine the protocol version, so that the
fragment will be skipped if it is sent from legacy mds, or inspected
the sequence if it is sent from new mds.
---
src/mds/mds_dt.h | 6 ++
src/mds/mds_dt_tipc.c| 11 ++-
src/mds/mds_tipc_fctrl_intf.cc | 154 ++-
src/mds/mds_tipc_fctrl_msg.cc| 86 +++---
src/mds/mds_tipc_fctrl_msg.h | 5 ++
src/mds/mds_tipc_fctrl_portid.cc | 23 ++
src/mds/mds_tipc_fctrl_portid.h | 1 +
7 files changed, 193 insertions(+), 93 deletions(-)
diff --git a/src/mds/mds_dt.h b/src/mds/mds_dt.h
index 64da600..007ff98 100644
--- a/src/mds/mds_dt.h
+++ b/src/mds/mds_dt.h
@@ -243,6 +243,12 @@ bool mdtm_mailbox_mbx_cleanup(NCSCONTEXT arg, NCSCONTEXT
msg);
#define MDS_PROT_VER_MASK 0xFC
#define MDTM_PRI_MASK 0x3
+/* Unknown or undefined MDS protocol/version */
+#define MDS_PROT_UNDEFINED 0x00
+
+/* MDS protocol/version for non flow control (legacy) */
+#define MDS_PROT_LEGACY (MDS_PROT | MDS_VERSION)
+
/* MDS protocol/version for flow control */
#define MDS_PROT_FCTRL (0xB0 | MDS_VERSION)
#define MDS_PROT_FCTRL_ID 0xFDAC13F5
diff --git a/src/mds/mds_dt_tipc.c b/src/mds/mds_dt_tipc.c
index e085de7..fdf0da7 100644
--- a/src/mds/mds_dt_tipc.c
+++ b/src/mds/mds_dt_tipc.c
@@ -166,7 +166,7 @@ NCS_PATRICIA_TREE mdtm_reassembly_list;
uint32_t mdtm_global_frag_num;
const unsigned int MAX_RECV_THRESHOLD = 30;
-static uint8_t gl_mds_pro_ver = MDS_PROT | MDS_VERSION;
+static uint8_t gl_mds_pro_ver = MDS_PROT_LEGACY;
static int gl_mds_fctrl_acksize = -1;
static int gl_mds_fctrl_ackto = -1;
@@ -381,7 +381,7 @@ uint32_t mdtm_tipc_init(NODE_ID nodeid, uint32_t *mds_tipc_ref)
"MDTM:TIPC Failed to unset
MDS_TIPC_FCTRL_ACKSIZE");
}
} else {
- gl_mds_pro_ver = MDS_PROT | MDS_VERSION;
+ gl_mds_pro_ver = MDS_PROT_LEGACY;
syslog(LOG_ERR, "MDTM:TIPC Invalid value of"
"MDS_TIPC_FCTRL_ENABLED");
}
@@ -3125,7 +3125,12 @@ uint32_t mdtm_add_frag_hdr(uint8_t *buf_ptr, uint16_t
len, uint32_t seq_num,
* hereafter, these 2 bytes will be used as sequence number in flow
control
* (per tipc portid)
* */
- ncs_encode_16bit(, fctrl_seq_num);
+ if (gl_mds_pro_ver == MDS_PROT_FCTRL) {
+ ncs_encode_16bit(, fctrl_seq_num);
+ } else {
+ ncs_encode_16bit(, len - MDTM_FRAG_HDR_LEN - 2);
+ }
+
#endif
return NCSCC_RC_SUCCESS;
}
diff --git a/src/mds/mds_tipc_fctrl_intf.cc b/src/mds/mds_tipc_fctrl_intf.cc
index c9073b2..3d92290 100644
--- a/src/mds/mds_tipc_fctrl_intf.cc
+++ b/src/mds/mds_tipc_fctrl_intf.cc
@@ -132,8 +132,16 @@ uint32_t process_flow_event(const Event& evt) {
portid = new TipcPortId(evt.id_, data_sock_fd,
chunk_ack_size, sock_buf_size);
portid_map[TipcPortId::GetUniqueId(evt.id_)] = portid;
- rc = portid->ReceiveData(evt.mseq_, evt.mfrag_,
-evt.fseq_, evt.svc_id_, evt.snd_type_, is_mcast_enabled);
+ if (evt.legacy_data_ == true) {
+// we create portid and set state kDisabled even though we know
+// this portid has no flow control. It is because the 2nd, 3rd fragment
+// could not reflect the protocol version, so need to keep this portid
+// remained stateful
+portid->ChangeState(TipcPortId::State::kDisabled);
+ } else {
+rc = portid->ReceiveData(evt.mseq_, evt.mfrag_,
+ evt.fseq_, evt.svc_id_, evt.snd_type_, is_mcast_enabled);
+ }
} else if (evt.type_ == Event::Type::kEvtRcvIntro) {
portid = new TipcPortId(evt.id_, data_sock_fd,
chunk_ack_size, sock_buf_size);
@@ -146,8 +154,12 @@ uint32_t process_flow_event(const Event& evt) {
}
} else {
if (evt.type_ == Event::Type::kEvtRcvData) {
- rc = portid->ReceiveData(evt.mseq_, evt.mfrag_,
- evt.fseq_, evt.svc_id_, evt.snd_type_, is_mcast_enabled);
+ if
[devel] [PATCH 1/1] mds: fix sender take very long time to send all messages [#3119]
When overload happens, sender will wait for chunkAck to continue
sending more messages, it should send number of message equal chunkAck
size of receiver. If not, receiver don't receive enough messages to send
chunkAck and wait until timer timeout to send chunkAck to sender.
This loop will make sender take very long time to sending all messages.
---
src/mds/mds_tipc_fctrl_portid.cc | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/mds/mds_tipc_fctrl_portid.cc b/src/mds/mds_tipc_fctrl_portid.cc
index 3704baddb..bd1825446 100644
--- a/src/mds/mds_tipc_fctrl_portid.cc
+++ b/src/mds/mds_tipc_fctrl_portid.cc
@@ -190,6 +190,7 @@ uint32_t TipcPortId::Queue(const uint8_t* data, uint16_t
length,
sndwnd_.acked_.v(), sndwnd_.send_.v(), sndwnd_.nacked_space_);
} else {
++sndwnd_.send_;
+sndwnd_.nacked_space_ += length;
m_MDS_LOG_NOTIFY("FCTRL: [me] --> [node:%x, ref:%u], "
"QueData[mseq:%u, mfrag:%u, fseq:%u, len:%u], "
"sndwnd[acked:%u, send:%u, nacked:%" PRIu64 "]",
@@ -444,32 +445,29 @@ void TipcPortId::ReceiveChunkAck(uint16_t fseq, uint16_t
chksize) {
// the nacked_space_ of sender
uint64_t acked_bytes = sndqueue_.Erase(Seq16(fseq) - (chksize-1),
Seq16(fseq));
+assert(sndwnd_.nacked_space_ >= acked_bytes);
sndwnd_.nacked_space_ -= acked_bytes;
// try to send a few pending msg
DataMessage* msg = nullptr;
-uint64_t resend_bytes = 0;
-while (resend_bytes < acked_bytes) {
+uint16_t send_msg_cnt = 0;
+while (send_msg_cnt++ < chunk_size_) {
// find the lowest sequence unsent yet
msg = sndqueue_.FirstUnsent();
if (msg == nullptr) {
break;
} else {
-if (resend_bytes < acked_bytes) {
if (Send(msg->msg_data_, msg->header_.msg_len_) == NCSCC_RC_SUCCESS)
{
-sndwnd_.nacked_space_ += msg->header_.msg_len_;
msg->is_sent_ = true;
-resend_bytes += msg->header_.msg_len_;
m_MDS_LOG_NOTIFY("FCTRL: [me] --> [node:%x, ref:%u], "
"SndQData[fseq:%u, len:%u], "
"sndwnd[acked:%u, send:%u, nacked:%" PRIu64 "]",
id_.node, id_.ref,
msg->header_.fseq_, msg->header_.msg_len_,
sndwnd_.acked_.v(), sndwnd_.send_.v(), sndwnd_.nacked_space_);
+ } else {
+break;
}
-} else {
- break;
-}
}
}
// no more unsent message, back to kEnabled
--
2.17.1
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 0/1] Review Request for mds: fix sender take very long time to send all messages [#3119] V2
Summary: mds: fix sender take very long time to send all messages [#3119] Review request for Ticket(s): 3119 Peer Reviewer(s): Minh, Vu, Gary Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-3119 Base revision: 2a7ec1f63710f9e8f679bbceb18032e0ebb1b46a Personal repository: git://git.code.sf.net/u/thuantr/review Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesn OpenSAF servicesn Core libraries y Samples n Tests n Other n Comments (indicate scope for each "y" above): - N/A revision acade1d9b368380e78c82901374b4528bbd89c7b Author: thuan.tran Date: Wed, 13 Nov 2019 17:55:09 +0700 mds: fix sender take very long time to send all messages [#3119] When overload happens, sender will wait for chunkAck to continue sending more messages, it should send number of message equal chunkAck size of receiver. If not, receiver don't receive enough messages to send chunkAck and wait until timer timeout to send chunkAck to sender. This loop will make sender take very long time to sending all messages. Complete diffstat: -- src/mds/mds_tipc_fctrl_portid.cc | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) Testing Commands: - N/A Testing, Expected Results: -- N/A Conditions of Submission: - ACK by reviewers Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 2/3] mds: Refactor logging [#3111]
Hi,
ACK from me.
Best Regards,
ThuanTr
-Original Message-
From: Minh Chau
Sent: Friday, November 8, 2019 5:33 PM
To: hans.nordeb...@ericsson.com; gary@dektech.com.au;
vu.m.ngu...@dektech.com.au; thuan.t...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; Minh Chau
Subject: [PATCH 2/3] mds: Refactor logging [#3111]
Since adding TipcPortId:ChangeState(), the patch refactors
logging to shorten the code.
---
src/mds/mds_tipc_fctrl_portid.cc | 71
1 file changed, 21 insertions(+), 50 deletions(-)
diff --git a/src/mds/mds_tipc_fctrl_portid.cc b/src/mds/mds_tipc_fctrl_portid.cc
index 9b87c74..df53d4d 100644
--- a/src/mds/mds_tipc_fctrl_portid.cc
+++ b/src/mds/mds_tipc_fctrl_portid.cc
@@ -208,17 +208,13 @@ bool TipcPortId::ReceiveCapable(uint16_t sending_len) {
if (state_ == State::kTxProb) {
// Too many msgs are not acked by receiver while in txprob state
// disable flow control
- state_ = State::kDisabled;
- m_MDS_LOG_ERR("FCTRL: me --> [node:%x, ref:%u], [nacked:%" PRIu64
- ", len:%u, rcv_buf_size:%" PRIu64 "], Warning[kTxProb -> kDisabled]",
- id_.node, id_.ref, sndwnd_.nacked_space_,
- sending_len, rcv_buf_size_);
+ m_MDS_LOG_ERR("FCTRL: me --> [node:%x, ref:%u], "
+ "Warning[Too many nacked in kTxProb]",
+ id_.node, id_.ref);
+ ChangeState(State::kDisabled);
return true;
} else if (state_ == State::kEnabled) {
- state_ = State::kRcvBuffOverflow;
- m_MDS_LOG_NOTIFY("FCTRL: [node:%x, ref:%u] --> Overflow, %" PRIu64
- ", %u, %" PRIu64, id_.node, id_.ref, sndwnd_.nacked_space_,
- sending_len, rcv_buf_size_);
+ ChangeState(State::kRcvBuffOverflow);
}
return false;
}
@@ -271,20 +267,18 @@ uint32_t TipcPortId::ReceiveData(uint32_t mseq, uint16_t
mfrag,
uint32_t rc = NCSCC_RC_SUCCESS;
if (state_ == State::kDisabled) {
m_MDS_LOG_ERR("FCTRL: [me] <-- [node:%x, ref:%u], "
-"RcvData, TxProb[retries:%u, state:%u], "
-"Error[receive fseq:%u in invalid state]",
+"RcvData[mseq:%u, mfrag:%u, fseq:%u], "
+"rcvwnd[acked:%u, rcv:%u, nacked:%" PRIu64 "], "
+"Warning[Invalid state:%u]",
id_.node, id_.ref,
-txprob_cnt_, (uint8_t)state_,
-fseq);
+mseq, mfrag, fseq,
+rcvwnd_.acked_.v(), rcvwnd_.rcv_.v(), rcvwnd_.nacked_space_,
+(uint8_t)state_);
return rc;
}
// update state
if (state_ == State::kTxProb || state_ == State::kStartup) {
-state_ = State::kEnabled;
-m_MDS_LOG_NOTIFY("FCTRL: [me] <-- [node:%x, ref:%u], "
-"RcvData, TxProb[retries:%u, state:%u]",
-id_.node, id_.ref,
-txprob_cnt_, (uint8_t)state_);
+ChangeState(State::kEnabled);
}
// if tipc multicast is enabled, receiver does not inspect sequence number
// for both fragment/unfragment multicast/broadcast message
@@ -398,12 +392,7 @@ void TipcPortId::ReceiveChunkAck(uint16_t fseq, uint16_t
chksize) {
}
// update state
if (state_ == State::kTxProb) {
-state_ = State::kEnabled;
-m_MDS_LOG_NOTIFY("FCTRL: [me] <-- [node:%x, ref:%u], "
-"RcvChkAck, "
-"TxProb[retries:%u, state:%u]",
-id_.node, id_.ref,
-txprob_cnt_, (uint8_t)state_);
+ChangeState(State::kEnabled);
}
// update sender sequence window
if (sndwnd_.acked_ < Seq16(fseq)) {
@@ -474,9 +463,7 @@ void TipcPortId::ReceiveChunkAck(uint16_t fseq, uint16_t
chksize) {
}
// no more unsent message, back to kEnabled
if (msg == nullptr && state_ == State::kRcvBuffOverflow) {
- state_ = State::kEnabled;
- m_MDS_LOG_NOTIFY("FCTRL: [node:%x, ref:%u] Overflow --> Enabled ",
- id_.node, id_.ref);
+ ChangeState(State::kEnabled);
}
} else {
m_MDS_LOG_ERR("FCTRL: [me] <-- [node:%x, ref:%u], "
@@ -517,9 +504,7 @@ void TipcPortId::ReceiveNack(uint32_t mseq, uint16_t mfrag,
}
}
if (state_ != State::kRcvBuffOverflow) {
-state_ = State::kRcvBuffOverflow;
-m_MDS_LOG_NOTIFY("FCTRL: [node:%x, ref:%u] --> Overflow ",
-id_.node, id_.ref);
+ChangeState(State::kRcvBuffOverflow);
sndqueue_.MarkUnsentFrom(Seq16(fseq));
}
DataMessage* msg = sndqueue_.Find(Seq16(fseq));
@@ -545,27 +530,15 @@ void TipcPortId::ReceiveNack(uint32_t mseq, uint16_t
mfrag,
bool TipcPortId::ReceiveTmrTxProb(uint8_t max_txprob) {
bool restart_txprob = false;
- if (state_ == State::kDisabled ||
- sndwnd_.acked_ > Seq16(1) ||
- rcvwnd_.rcv_ > Seq16(1)) return restart_txprob;
+ if (state_ == State::kDisabled) return restart_txprob;
if (state_ == State::kTxProb || state_ == State::kRcvBuffOverflow) {
txprob_cnt_++;
if (txprob_cnt_ >= max_txprob) {
- state_ = State::kDisabled;
+ ChangeState(State::kDisabled);
restart_txprob = false;
} else {
restart_txprob = true;
}
-
-// at kDisabled state,
Re: [devel] [PATCH 3/3] mds: Add backward compatibility mdstest for fragment [#3111]
Hi,
ACK from me.
Best Regards,
ThuanTr
-Original Message-
From: Minh Chau
Sent: Friday, November 8, 2019 5:33 PM
To: hans.nordeb...@ericsson.com; gary@dektech.com.au;
vu.m.ngu...@dektech.com.au; thuan.t...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; Minh Chau
Subject: [PATCH 3/3] mds: Add backward compatibility mdstest for fragment
[#3111]
---
src/mds/apitest/mdstipc_api.c | 83 ---
1 file changed, 78 insertions(+), 5 deletions(-)
diff --git a/src/mds/apitest/mdstipc_api.c b/src/mds/apitest/mdstipc_api.c
index 5c0e28a..651365e 100644
--- a/src/mds/apitest/mdstipc_api.c
+++ b/src/mds/apitest/mdstipc_api.c
@@ -13512,8 +13512,8 @@ void tet_mds_fctrl_compatibility_tp1(void)
uint32_t msg_num = 1000;
uint32_t msg_size = 500;
- printf("\nTest Case 5: Sender enable MDS FCTRL but Receiver disable\n");
- /**/
+ printf("\nTest Case 5: Sender enable MDS FCTRL, Receiver disable\n");
+ /*-*/
pid_t pid = fork();
if (pid == 0) {
/* child as sender */
@@ -13545,8 +13545,8 @@ void tet_mds_fctrl_compatibility_tp2(void)
uint32_t msg_num = 1000;
uint32_t msg_size = 500;
- printf("\nTest Case 5: Sender diable MDS FCTRL but Receiver enable\n");
- /**/
+ printf("\nTest Case 6: Sender disable MDS FCTRL, Receiver enable\n");
+ /*-*/
pid_t pid = fork();
if (pid == 0) {
/* child as sender */
@@ -13644,6 +13644,73 @@ void tet_mds_fctrl_with_sna_tp2(void)
test_validate(FAIL, 0);
}
+
+void tet_mds_fctrl_compatibility_tp3(void)
+{
+ int FAIL = 1;
+ uint32_t msg_num = 5;
+ uint32_t msg_size = 13;
+
+ printf("\nTest Case 9: Sender enable MDS FCTRL, Receiver disable\n");
+ /*-*/
+ pid_t pid = fork();
+ if (pid == 0) {
+ /* child as sender */
+ setenv("MDS_TIPC_FCTRL_ENABLED", "1", 1);
+ mds_startup();
+ MDS_SVC_ID to_svcids[] = {NCSMDS_SVC_ID_EXTERNAL_MIN};
+ MDS_SVC_ID svc_id = NCSMDS_SVC_ID_INTERNAL_MIN;
+ tet_sender(svc_id, msg_num, msg_size, 1, to_svcids);
+ mds_shutdown();
+ } else if (pid > 0) {
+ /* parent as receiver */
+ mds_startup();
+ MDS_SVC_ID fr_svcids[] = {NCSMDS_SVC_ID_INTERNAL_MIN};
+ MDS_SVC_ID svc_id = NCSMDS_SVC_ID_EXTERNAL_MIN;
+ FAIL = tet_receiver(svc_id, msg_num, msg_size, 1, fr_svcids);
+ printf("\nReceiver finish, kill Sender\n");
+ kill(pid, SIGKILL);
+ mds_shutdown();
+ } else {
+ printf("\nFAIL to fork()\n");
+ }
+
+ test_validate(FAIL, 0);
+}
+
+void tet_mds_fctrl_compatibility_tp4(void)
+{
+ int FAIL = 1;
+ uint32_t msg_num = 10;
+ uint32_t msg_size = 13;
+
+ printf("\nTest Case 10: Sender disable MDS FCTRL, Receiver enable\n");
+ /*--*/
+ pid_t pid = fork();
+ if (pid == 0) {
+ /* child as sender */
+ mds_startup();
+ MDS_SVC_ID to_svcids[] = {NCSMDS_SVC_ID_EXTERNAL_MIN};
+ MDS_SVC_ID svc_id = NCSMDS_SVC_ID_INTERNAL_MIN;
+ tet_sender(svc_id, msg_num, msg_size, 1, to_svcids);
+ mds_shutdown();
+ } else if (pid > 0) {
+ /* parent as receiver */
+ setenv("MDS_TIPC_FCTRL_ENABLED", "1", 1);
+ mds_startup();
+ MDS_SVC_ID fr_svcids[] = {NCSMDS_SVC_ID_INTERNAL_MIN};
+ MDS_SVC_ID svc_id = NCSMDS_SVC_ID_EXTERNAL_MIN;
+ FAIL = tet_receiver(svc_id, msg_num, msg_size, 1, fr_svcids);
+ printf("\nReceiver finish, kill Sender\n");
+ kill(pid, SIGKILL);
+ mds_shutdown();
+ } else {
+ printf("\nFAIL to fork()\n");
+ }
+ test_validate(FAIL, 0);
+}
+
+
void Print_return_status(uint32_t rs)
{
switch (rs) {
@@ -14384,7 +14451,7 @@ __attribute__((constructor)) static void
mdsTipcAPI_constructor(void)
"Sender enable MDS FCTRL but Receiver disable");
test_case_add(
27, tet_mds_fctrl_compatibility_tp2,
- "Sender diable MDS FCTRL but Receiver enable");
+ "Sender disable MDS FCTRL but Receiver enable");
test_case_add(
27, tet_mds_fctrl_with_sna_tp1,
"Sender gradually sends more than 65535"
@@ -14395,4 +14462,10 @@
Re: [devel] [PATCH 1/3] mds: Distinguish protocol version of fragment [#3111]
Hi Minh,
See my comment inline.
Best Regards,
ThuanTr
-Original Message-
From: Minh Chau
Sent: Friday, November 8, 2019 5:33 PM
To: hans.nordeb...@ericsson.com; gary@dektech.com.au;
vu.m.ngu...@dektech.com.au; thuan.t...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; Minh Chau
Subject: [PATCH 1/3] mds: Distinguish protocol version of fragment [#3111]
The legacy mds encodes the protocol version in either non fragment
message or the first fragment only. Hence, the subsequent fragment
after the first one is not able for mds to determine the protocol
version.
The patch maintains the encoding of lengthcheck as same as the legacy
mds version. Also, the subsequent fragments needs to consult the
stateful portid to determine the protocol version, so that the
fragment will be skipped if it is sent from legacy mds, or inspected
the sequence if it is sent from new mds.
---
src/mds/mds_dt.h | 6 ++
src/mds/mds_dt_tipc.c| 11 ++-
src/mds/mds_tipc_fctrl_intf.cc | 154 ++-
src/mds/mds_tipc_fctrl_msg.cc| 86 +++---
src/mds/mds_tipc_fctrl_msg.h | 5 ++
src/mds/mds_tipc_fctrl_portid.cc | 23 ++
src/mds/mds_tipc_fctrl_portid.h | 1 +
7 files changed, 193 insertions(+), 93 deletions(-)
diff --git a/src/mds/mds_dt.h b/src/mds/mds_dt.h
index 64da600..007ff98 100644
--- a/src/mds/mds_dt.h
+++ b/src/mds/mds_dt.h
@@ -243,6 +243,12 @@ bool mdtm_mailbox_mbx_cleanup(NCSCONTEXT arg, NCSCONTEXT
msg);
#define MDS_PROT_VER_MASK 0xFC
#define MDTM_PRI_MASK 0x3
+/* Unknown or undefined MDS protocol/version */
+#define MDS_PROT_UNDEFINED 0x00
+
+/* MDS protocol/version for non flow control (legacy) */
+#define MDS_PROT_LEGACY (MDS_PROT | MDS_VERSION)
+
/* MDS protocol/version for flow control */
#define MDS_PROT_FCTRL (0xB0 | MDS_VERSION)
#define MDS_PROT_FCTRL_ID 0xFDAC13F5
diff --git a/src/mds/mds_dt_tipc.c b/src/mds/mds_dt_tipc.c
index e085de7..fdf0da7 100644
--- a/src/mds/mds_dt_tipc.c
+++ b/src/mds/mds_dt_tipc.c
@@ -166,7 +166,7 @@ NCS_PATRICIA_TREE mdtm_reassembly_list;
uint32_t mdtm_global_frag_num;
const unsigned int MAX_RECV_THRESHOLD = 30;
-static uint8_t gl_mds_pro_ver = MDS_PROT | MDS_VERSION;
+static uint8_t gl_mds_pro_ver = MDS_PROT_LEGACY;
static int gl_mds_fctrl_acksize = -1;
static int gl_mds_fctrl_ackto = -1;
@@ -381,7 +381,7 @@ uint32_t mdtm_tipc_init(NODE_ID nodeid, uint32_t
*mds_tipc_ref)
"MDTM:TIPC Failed to unset
MDS_TIPC_FCTRL_ACKSIZE");
}
} else {
- gl_mds_pro_ver = MDS_PROT | MDS_VERSION;
+ gl_mds_pro_ver = MDS_PROT_LEGACY;
syslog(LOG_ERR, "MDTM:TIPC Invalid value of"
"MDS_TIPC_FCTRL_ENABLED");
}
@@ -3125,7 +3125,12 @@ uint32_t mdtm_add_frag_hdr(uint8_t *buf_ptr, uint16_t
len, uint32_t seq_num,
* hereafter, these 2 bytes will be used as sequence number in flow
control
* (per tipc portid)
* */
- ncs_encode_16bit(, fctrl_seq_num);
+ if (gl_mds_pro_ver == MDS_PROT_FCTRL) {
+ ncs_encode_16bit(, fctrl_seq_num);
+ } else {
+ ncs_encode_16bit(, len - MDTM_FRAG_HDR_LEN - 2);
+ }
+
#endif
return NCSCC_RC_SUCCESS;
}
diff --git a/src/mds/mds_tipc_fctrl_intf.cc b/src/mds/mds_tipc_fctrl_intf.cc
index c9073b2..3d92290 100644
--- a/src/mds/mds_tipc_fctrl_intf.cc
+++ b/src/mds/mds_tipc_fctrl_intf.cc
@@ -132,8 +132,16 @@ uint32_t process_flow_event(const Event& evt) {
portid = new TipcPortId(evt.id_, data_sock_fd,
chunk_ack_size, sock_buf_size);
portid_map[TipcPortId::GetUniqueId(evt.id_)] = portid;
- rc = portid->ReceiveData(evt.mseq_, evt.mfrag_,
-evt.fseq_, evt.svc_id_, evt.snd_type_, is_mcast_enabled);
+ if (evt.legacy_data_ == true) {
+// we create portid and set state kDisabled even though we know
+// this portid has no flow control. It is because the 2nd, 3rd fragment
+// could not reflect the protocol version, so need to keep this portid
+// remained stateful
+portid->ChangeState(TipcPortId::State::kDisabled);
+ } else {
+rc = portid->ReceiveData(evt.mseq_, evt.mfrag_,
+ evt.fseq_, evt.svc_id_, evt.snd_type_, is_mcast_enabled);
+ }
} else if (evt.type_ == Event::Type::kEvtRcvIntro) {
portid = new TipcPortId(evt.id_, data_sock_fd,
chunk_ack_size, sock_buf_size);
@@ -146,8 +154,12 @@ uint32_t process_flow_event(const Event& evt) {
}
} else {
if (evt.type_ == Event::Type::kEvtRcvData) {
- rc = portid->ReceiveData(evt.mseq_, evt.mfrag_,
- evt.fseq_, evt.svc_id_, evt.snd_type_, is_mcast_enabled);
+ if (evt.legacy_data_ == true) {
+portid->ChangeState(TipcPortId::State::kDisabled);
+ } else {
+rc =
Re: [devel] [PATCH 1/1] mds: fix sender take very long time to send all messages [#3119]
Hi Minh,
Agree with your latest comment. I will send out V2.
Best Regards,
ThuanTr
-Original Message-
From: Minh Hon Chau
Sent: Wednesday, November 13, 2019 11:15 AM
To: Tran Thuan ; 'Nguyen Minh Vu'
; gary@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] mds: fix sender take very long time to send all
messages [#3119]
Hi Thuan,
Please see comment inline
Thanks
Minh
On 13/11/19 2:24 pm, Tran Thuan wrote:
> Hi Minh,
>
> Please check replies inline. Thanks.
>
> Best Regards,
> ThuanTr
>
> -Original Message-
> From: Minh Hon Chau
> Sent: Wednesday, November 13, 2019 10:05 AM
> To: Tran Thuan ; 'Nguyen Minh Vu'
> ; gary@dektech.com.au
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1/1] mds: fix sender take very long time to send all
> messages [#3119]
>
> Hi Thuan,
>
> Please see comment inline.
>
> Thanks
>
> Minh
>
> On 13/11/19 1:11 pm, Tran Thuan wrote:
>> Hi Minh,
>>
>> Thanks for comments, please check my replies inline.
>>
>> Best Regards,
>> ThuanTr
>>
>> -Original Message-
>> From: Minh Hon Chau
>> Sent: Wednesday, November 13, 2019 7:47 AM
>> To: thuan.tran ; 'Nguyen Minh Vu'
>> ; gary@dektech.com.au
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: Re: [PATCH 1/1] mds: fix sender take very long time to send all
>> messages [#3119]
>>
>> Hi Thuan,
>>
>> Some comments inline.
>>
>> Thanks
>>
>> Minh
>>
>> On 12/11/19 5:04 pm, thuan.tran wrote:
>>> When overload happens, sender will wait for chunkAck to continue
>>> sending more messages, it should send number of message equal chunkAck
>>> size of receiver. If not, receiver don't receive enough messages to send
>>> chunkAck and wait until timer timeout to send chunkAck to sender.
>>> This loop will make sender take very long time to sending all messages.
>>> ---
>>> src/mds/mds_tipc_fctrl_portid.cc | 30 +++---
>>> 1 file changed, 7 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/src/mds/mds_tipc_fctrl_portid.cc
>>> b/src/mds/mds_tipc_fctrl_portid.cc
>>> index 3704baddb..1fff4c855 100644
>>> --- a/src/mds/mds_tipc_fctrl_portid.cc
>>> +++ b/src/mds/mds_tipc_fctrl_portid.cc
>>> @@ -190,6 +190,7 @@ uint32_t TipcPortId::Queue(const uint8_t* data,
>>> uint16_t length,
>>> sndwnd_.acked_.v(), sndwnd_.send_.v(), sndwnd_.nacked_space_);
>>> } else {
>>> ++sndwnd_.send_;
>>> +sndwnd_.nacked_space_ += length;
>> [Minh] We haven't sent the msg out to wait for ack, thus nacked_space_
>> should not be increased
>>> m_MDS_LOG_NOTIFY("FCTRL: [me] --> [node:%x, ref:%u], "
>>> "QueData[mseq:%u, mfrag:%u, fseq:%u, len:%u], "
>>> "sndwnd[acked:%u, send:%u, nacked:%" PRIu64 "]",
>>> @@ -444,32 +445,29 @@ void TipcPortId::ReceiveChunkAck(uint16_t fseq,
>>> uint16_t chksize) {
>>> // the nacked_space_ of sender
>>> uint64_t acked_bytes = sndqueue_.Erase(Seq16(fseq) - (chksize-1),
>>> Seq16(fseq));
>>> +assert(sndwnd_.nacked_space_ >= acked_bytes);
>>> sndwnd_.nacked_space_ -= acked_bytes;
>>>
>>> // try to send a few pending msg
>>> DataMessage* msg = nullptr;
>>> -uint64_t resend_bytes = 0;
>>> -while (resend_bytes < acked_bytes) {
>>> +uint16_t send_msg_cnt = 0;
>>> +while (send_msg_cnt++ < chunk_size_) {
>>> // find the lowest sequence unsent yet
>>> msg = sndqueue_.FirstUnsent();
>>> if (msg == nullptr) {
>>> break;
>>> } else {
>>> -if (resend_bytes < acked_bytes) {
>>> if (Send(msg->msg_data_, msg->header_.msg_len_) ==
>>> NCSCC_RC_SUCCESS) {
>>> -sndwnd_.nacked_space_ += msg->header_.msg_len_;
>> [Minh] We now send it out and wait for acked, thus the nacked_space_ is
>> increased here, so any reason moving the nacked_space_ from Queue() to here?
>> [Thuan] Because the message could be in sndwnd (resend) either in sndqueue
>> (send)
>> Cannot increase nacked_space with resend message.
>> I have tried another way to increase/decrease nacked_space dynamic
>> but it become complex with markUnsent() since sender may receiver Nack for
>> same msg > 2 times.
> [Minh] OK.
>>> msg->is_sent_ = true;
>>> -resend_bytes += msg->header_.msg_len_;
>>> m_MDS_LOG_NOTIFY("FCTRL: [me] --> [node:%x, ref:%u], "
>>> "SndQData[fseq:%u, len:%u], "
>>> "sndwnd[acked:%u, send:%u, nacked:%" PRIu64 "]",
>>> id_.node, id_.ref,
>>> msg->header_.fseq_, msg->header_.msg_len_,
>>> sndwnd_.acked_.v(), sndwnd_.send_.v(),
>>> sndwnd_.nacked_space_);
>>> + } else {
>>> +break;
>>> }
>>> -} else {
>>> - break;
>>> -}
>>> }
>>> }
>>> // no more unsent message, back to kEnabled
>> [Minh] Agree, the new strategy
