Re: [devel] [PATCH 0/2] Review Request for mds: Add Nack message for MDS_TIPC_FCTRL_ENABLED [#3095] V2
Hi Minh, ACK from me. Best Regards, ThuanTr -Original Message- From: Minh Hon Chau Sent: Monday, October 7, 2019 7:12 AM To: hans.nordeb...@ericsson.com; vu.m.ngu...@dektech.com.au; gary@dektech.com.au; thuan.t...@dektech.com.au Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [PATCH 0/2] Review Request for mds: Add Nack message for MDS_TIPC_FCTRL_ENABLED [#3095] V2 Hi, I would like to push the patches today if no more comment for them. Thanks Minh On 4/10/19 3:20 pm, Minh Chau wrote: > Summary: mds: Add Nack message for MDS_TIPC_FCTRL_ENABLED [#3095] V2 > Review request for Ticket(s): 3095 Peer Reviewer(s): Hans, Vu, Gary, > Thuan Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** > Affected branch(es): develop Development branch: ticket-3095 Base > revision: 05064a1cfd0aeaf824dce7602d535654b3457e30 > Personal repository: git://git.code.sf.net/u/minh-chau/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): > - > *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** > > revision cbbeab8f2299620aa3eb9b0e29710a2b159b5a45 > Author: Minh Chau > Date: Fri, 4 Oct 2019 12:59:27 +1000 > > mds: Improve error log for MDS_TIPC_FCTRL_ENABLED [#3095] > > This commit as part of #3095 updates the error string with pattern > "FCTRL:*Error[*]", in order to help grep-ing the error in mds debug > log. > > > > revision cc666586717fa82df70471748d8766e8fe901460 > Author: Minh Chau > Date: Fri, 4 Oct 2019 12:59:16 +1000 > > mds: Add Nack message for MDS_TIPC_FCTRL_ENABLED [#3095] > > In the scenario of recovery from split-brain, where both active > director services may suffer mds message loss due to lost-contact tipc > link. If MDS_TIPC_FCTRL_ENABLED is set, the out-of-order message will > be dropped, and there is no mechanism to trigger the retransmission > from receiver side at this moment (the retransmission is only > triggered from sender as result of TIPC_ERR_OVERLOAD). > > In reception of disordered message, the receiver can send > not-acknowledgement to notify the sender for retransmission. > Therefore, the sender can trigger retransmisison in the same way as > receiving TIPC_ERR_OVERLOAD. > > This patch adds Nack message for retransmission of disordered message > detected from receiver side, and adds a missing call to > portid_map_mutex.unlock() in process_all_events(). > > > > Complete diffstat: > -- > src/mds/mds_c_api.c | 2 +- > src/mds/mds_dt_common.c | 2 +- > src/mds/mds_tipc_fctrl_intf.cc | 72 > +--- > src/mds/mds_tipc_fctrl_msg.cc| 35 ++- > src/mds/mds_tipc_fctrl_msg.h | 22 > src/mds/mds_tipc_fctrl_portid.cc | 42 --- > src/mds/mds_tipc_fctrl_portid.h | 3 +- > 7 files changed, 143 insertions(+), 35 deletions(-) > > > Testing Commands: > - > *** LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES *** > > > Testing, Expected Results: > -- > *** PASTE COMMAND OUTPUTS / TEST RESULTS *** > > > Conditions of Submission: > - > *** HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC *** > > > Arch Built StartedLinux distro > --- > mipsn n > mips64 n n > x86 n n > x86_64 n n > 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
Re: [devel] [PATCH 1/1] mds: Enhance decoding for mds flow control message [#3097]
Hi Thuan,
Please see comments inline.
Thanks
Minh
On 7/10/19 3:18 pm, Tran Thuan wrote:
Hi Minh,
Some minor comments from me, check [Thuan] inline.
Thanks.
Best Regards,
ThuanTr
-Original Message-
From: Minh Chau
Sent: Monday, October 7, 2019 7:12 AM
To: hans.nordeb...@ericsson.com; vu.m.ngu...@dektech.com.au;
gary@dektech.com.au; thuan.t...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; Minh Chau
Subject: [PATCH 1/1] mds: Enhance decoding for mds flow control message
[#3097]
mds currently uses MDS_PROT_FCTRL_ID 4 bytes value (0x00AC13F5) from octet11
to octet14 to identify the flow control message e.g., chunkack message. In
case of fragmentation from big message, the second fragment onwards will
start from the octet11, which may have arbitrary value and cause mds to
incorrectly decode as a flow control message if the fragment starts with
value of 0x00AC13F5.
This patch fixes this rare case by decoding flow control message only if the
oct2-5 (mds global sequence number) and oct6-7 (mds fragment number) are
non-zero. Change MDS_PROT_FCTRL_ID:0xFDAC13F5
[Thuan]: typo "non-zero" -> "zero"?
[Minh]: Yes, typo, it's "zero"
[Thuan] Can you give info in commit message about why change
MDS_PROT_FCTRL_ID to FDAC13F5?
[Minh]: It is only a random number for identifier, but 0x00AC will
occupy the oct11&12 which is msd header length, and may cause a higher
probability to be identical
---
src/mds/mds_dt.h | 2 +-
src/mds/mds_tipc_fctrl_msg.cc | 20 +---
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/src/mds/mds_dt.h b/src/mds/mds_dt.h index d9e8633..64da600
100644
--- a/src/mds/mds_dt.h
+++ b/src/mds/mds_dt.h
@@ -245,7 +245,7 @@ bool mdtm_mailbox_mbx_cleanup(NCSCONTEXT arg, NCSCONTEXT
msg);
/* MDS protocol/version for flow control */ #define MDS_PROT_FCTRL (0xB0 |
MDS_VERSION) -#define MDS_PROT_FCTRL_ID 0x00AC13F5
+#define MDS_PROT_FCTRL_ID 0xFDAC13F5
/* Added for the subscription changes */ #define MDS_NCS_CHASSIS_ID
(m_NCS_GET_NODE_ID & 0x00ff) diff --git a/src/mds/mds_tipc_fctrl_msg.cc
b/src/mds/mds_tipc_fctrl_msg.cc index 064d977..8375673 100644
--- a/src/mds/mds_tipc_fctrl_msg.cc
+++ b/src/mds/mds_tipc_fctrl_msg.cc
@@ -64,13 +64,19 @@ void HeaderMessage::Decode(uint8_t *msg) {
// decode flow control sequence number
ptr = [HeaderMessage::FieldIndex::kFlowControlSequenceNumber];
fseq_ = ncs_decode_16bit();
-// decode protocol identifier
-ptr = [ChunkAck::FieldIndex::kProtocolIdentifier];
-pro_id_ = ncs_decode_32bit();
-if (pro_id_ == MDS_PROT_FCTRL_ID) {
- // decode message type
- ptr = [ChunkAck::FieldIndex::kFlowControlMessageType];
- msg_type_ = ncs_decode_8bit();
+// decode protocol identifier if the mfrag_ and mseq_ are 0
+// otherwise it is always DataMessage within non-zero mseq_ and mfrag_
+if (mfrag_ == 0 && mseq_ == 0) {
+ ptr = [ChunkAck::FieldIndex::kProtocolIdentifier];
+ pro_id_ = ncs_decode_32bit();
+ if (pro_id_ == MDS_PROT_FCTRL_ID) {
+// decode message type
+ptr = [ChunkAck::FieldIndex::kFlowControlMessageType];
+msg_type_ = ncs_decode_8bit();
+ }
+} else {
+ pro_id_ = 0;
+ msg_type_ = 0;
[Thuan] Don't need ELSE as values 0 already?
[Minh]: I think we should explicitly set again, the variable header
might be reused to decode
}
} else {
if (mfrag_ != 0) {
--
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: Enhance decoding for mds flow control message [#3097]
Hi Minh,
Some minor comments from me, check [Thuan] inline.
Thanks.
Best Regards,
ThuanTr
-Original Message-
From: Minh Chau
Sent: Monday, October 7, 2019 7:12 AM
To: hans.nordeb...@ericsson.com; vu.m.ngu...@dektech.com.au;
gary@dektech.com.au; thuan.t...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; Minh Chau
Subject: [PATCH 1/1] mds: Enhance decoding for mds flow control message
[#3097]
mds currently uses MDS_PROT_FCTRL_ID 4 bytes value (0x00AC13F5) from octet11
to octet14 to identify the flow control message e.g., chunkack message. In
case of fragmentation from big message, the second fragment onwards will
start from the octet11, which may have arbitrary value and cause mds to
incorrectly decode as a flow control message if the fragment starts with
value of 0x00AC13F5.
This patch fixes this rare case by decoding flow control message only if the
oct2-5 (mds global sequence number) and oct6-7 (mds fragment number) are
non-zero. Change MDS_PROT_FCTRL_ID:0xFDAC13F5
[Thuan]: typo "non-zero" -> "zero"?
[Thuan] Can you give info in commit message about why change
MDS_PROT_FCTRL_ID to FDAC13F5?
---
src/mds/mds_dt.h | 2 +-
src/mds/mds_tipc_fctrl_msg.cc | 20 +---
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/src/mds/mds_dt.h b/src/mds/mds_dt.h index d9e8633..64da600
100644
--- a/src/mds/mds_dt.h
+++ b/src/mds/mds_dt.h
@@ -245,7 +245,7 @@ bool mdtm_mailbox_mbx_cleanup(NCSCONTEXT arg, NCSCONTEXT
msg);
/* MDS protocol/version for flow control */ #define MDS_PROT_FCTRL (0xB0 |
MDS_VERSION) -#define MDS_PROT_FCTRL_ID 0x00AC13F5
+#define MDS_PROT_FCTRL_ID 0xFDAC13F5
/* Added for the subscription changes */ #define MDS_NCS_CHASSIS_ID
(m_NCS_GET_NODE_ID & 0x00ff) diff --git a/src/mds/mds_tipc_fctrl_msg.cc
b/src/mds/mds_tipc_fctrl_msg.cc index 064d977..8375673 100644
--- a/src/mds/mds_tipc_fctrl_msg.cc
+++ b/src/mds/mds_tipc_fctrl_msg.cc
@@ -64,13 +64,19 @@ void HeaderMessage::Decode(uint8_t *msg) {
// decode flow control sequence number
ptr = [HeaderMessage::FieldIndex::kFlowControlSequenceNumber];
fseq_ = ncs_decode_16bit();
-// decode protocol identifier
-ptr = [ChunkAck::FieldIndex::kProtocolIdentifier];
-pro_id_ = ncs_decode_32bit();
-if (pro_id_ == MDS_PROT_FCTRL_ID) {
- // decode message type
- ptr = [ChunkAck::FieldIndex::kFlowControlMessageType];
- msg_type_ = ncs_decode_8bit();
+// decode protocol identifier if the mfrag_ and mseq_ are 0
+// otherwise it is always DataMessage within non-zero mseq_ and mfrag_
+if (mfrag_ == 0 && mseq_ == 0) {
+ ptr = [ChunkAck::FieldIndex::kProtocolIdentifier];
+ pro_id_ = ncs_decode_32bit();
+ if (pro_id_ == MDS_PROT_FCTRL_ID) {
+// decode message type
+ptr = [ChunkAck::FieldIndex::kFlowControlMessageType];
+msg_type_ = ncs_decode_8bit();
+ }
+} else {
+ pro_id_ = 0;
+ msg_type_ = 0;
[Thuan] Don't need ELSE as values 0 already?
}
} else {
if (mfrag_ != 0) {
--
2.7.4
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1/1] ntfd: Do not send response to client if client down [#3084]
Ntfd will not send response to a client when client already down.
This will avoid timeout when ntfd send via mds.
---
src/ntf/ntfd/NtfAdmin.cc | 61
src/ntf/ntfd/NtfAdmin.h | 1 +
src/ntf/ntfd/ntfs_cb.h | 6 +
src/ntf/ntfd/ntfs_com.c | 4
src/ntf/ntfd/ntfs_com.h | 1 +
src/ntf/ntfd/ntfs_evt.c | 1 +
src/ntf/ntfd/ntfs_mds.c | 2 +-
7 files changed, 75 insertions(+), 1 deletion(-)
diff --git a/src/ntf/ntfd/NtfAdmin.cc b/src/ntf/ntfd/NtfAdmin.cc
index 8bbee69..3a0e9f6 100644
--- a/src/ntf/ntfd/NtfAdmin.cc
+++ b/src/ntf/ntfd/NtfAdmin.cc
@@ -555,6 +555,28 @@ void NtfAdmin::SearchAndSetClientsDownFlag(MDS_DEST
mds_dest) {
client->SetClientDownFlag();
}
}
+
+ CLIENT_DOWN_LIST *client_down_rec = NULL;
+ if ((client_down_rec = reinterpret_cast(
+ malloc(sizeof(CLIENT_DOWN_LIST == NULL) {
+LOG_ER("memory allocation for the CLIENT_DOWN_LIST failed");
+return;
+ }
+ memset(client_down_rec, 0, sizeof(CLIENT_DOWN_LIST));
+ client_down_rec->mds_dest = mds_dest;
+ client_down_rec->next = NULL;
+
+ if (ntfs_cb->client_down_list_head == NULL) {
+ntfs_cb->client_down_list_head = client_down_rec;
+ } else {
+CLIENT_DOWN_LIST *p = ntfs_cb->client_down_list_head;
+while (p->next != NULL) {
+ p = p->next;
+}
+p->next = client_down_rec;
+ }
+ TRACE_1("MDS dest added: %" PRIx64, client_down_rec->mds_dest);
+
osaf_mutex_unlock_ordie(_map_mutex);
TRACE_LEAVE();
}
@@ -1096,6 +1118,41 @@ bool NtfAdmin::is_stale_client(unsigned int client_id) {
return false;
}
+/**
+ * @brief Checks client added to CLIENT_DOWN_LIST.
+ * Remove client out of list if existed.
+ * @param mds_dest
+ * @return true/false.
+ */
+bool NtfAdmin::checkAddedAndRemove(MDS_DEST mds_dest) {
+ bool found = false;
+ CLIENT_DOWN_LIST *client_down_rec = ntfs_cb->client_down_list_head;
+ CLIENT_DOWN_LIST *prev = NULL;
+ TRACE_ENTER();
+ while (client_down_rec != NULL) {
+if (mds_dest == client_down_rec->mds_dest) {
+ if (client_down_rec == ntfs_cb->client_down_list_head) {
+if (client_down_rec->next == NULL) {
+ ntfs_cb->client_down_list_head = NULL;
+} else {
+ ntfs_cb->client_down_list_head = client_down_rec->next;
+}
+ } else if (prev) {
+prev->next = client_down_rec->next;
+ }
+ TRACE("MDS dest %" PRIx64 " already delete", client_down_rec->mds_dest);
+ free(client_down_rec);
+ client_down_rec = NULL;
+ found = true;
+ break;
+}
+prev = client_down_rec;
+client_down_rec = client_down_rec->next;
+ }
+ TRACE_LEAVE();
+ return found;
+}
+
// C wrapper funcions start here
/**
@@ -1300,6 +1357,10 @@ uint32_t
send_clm_node_status_change(SaClmClusterChangesT cluster_change,
cluster_change, node_id));
}
+bool checkAddedAndRemove(MDS_DEST mds_dest) {
+ return (NtfAdmin::theNtfAdmin->checkAddedAndRemove(mds_dest));
+}
+
/**
* @brief Checks CLM membership status of a client.
* A0101 clients are always CLM member.
diff --git a/src/ntf/ntfd/NtfAdmin.h b/src/ntf/ntfd/NtfAdmin.h
index 4808ca9..d0c5528 100644
--- a/src/ntf/ntfd/NtfAdmin.h
+++ b/src/ntf/ntfd/NtfAdmin.h
@@ -109,6 +109,7 @@ class NtfAdmin {
uint32_t send_cluster_membership_msg_to_clients(
SaClmClusterChangesT cluster_change, NODE_ID node_id);
bool is_stale_client(unsigned int clientId);
+ bool checkAddedAndRemove(MDS_DEST mds_dest);
private:
void processNotification(unsigned int clientId,
diff --git a/src/ntf/ntfd/ntfs_cb.h b/src/ntf/ntfd/ntfs_cb.h
index 96eedc1..e09f8fb 100644
--- a/src/ntf/ntfd/ntfs_cb.h
+++ b/src/ntf/ntfd/ntfs_cb.h
@@ -38,6 +38,11 @@ typedef struct {
MDS_DEST mds_dest;
} ntf_client_t;
+typedef struct client_down_list {
+ MDS_DEST mds_dest;
+ struct client_down_list *next;
+} CLIENT_DOWN_LIST;
+
typedef struct ntfs_cb {
SYSF_MBX mbx; /* NTFS's mailbox */
MDS_HDL mds_hdl;/* PWE Handle for interacting with NTFAs */
@@ -71,6 +76,7 @@ typedef struct ntfs_cb {
NCS_SEL_OBJ usr2_sel_obj; /* Selection object for CLM initialization.*/
uint16_t peer_mbcsv_version; /*Remeber peer NTFS MBCSV version.*/
bool clm_initialized;// For CLM init status;
+ CLIENT_DOWN_LIST *client_down_list_head; /* client down reccords */
} ntfs_cb_t;
extern uint32_t ntfs_cb_init(ntfs_cb_t *);
diff --git a/src/ntf/ntfd/ntfs_com.c b/src/ntf/ntfd/ntfs_com.c
index 35435bb..ee282dc 100644
--- a/src/ntf/ntfd/ntfs_com.c
+++ b/src/ntf/ntfd/ntfs_com.c
@@ -44,6 +44,10 @@ void client_added_res_lib(SaAisErrorT error, unsigned int
clientId,
ntfsv_msg_t msg;
ntfsv_ckpt_msg_t ckpt;
TRACE_ENTER2("clientId: %u, rv: %u", clientId, error);
+ // Not respond when client already down
+ if (checkAddedAndRemove(mdsDest))
+ return;
+
msg.type = NTFSV_NTFA_API_RESP_MSG;
[devel] [PATCH 0/1] Review Request for ntfd: Do not send response to client if client down V2 [#3084]
Summary: ntfd: Do not send response to client if client down [#3083] Review request for Ticket(s): 3084 Peer Reviewer(s): Vu, Minh Pull request to: Vu Affected branch(es): develop Development branch: ticket-3084 Base revision: e699c22ddc1ca8530318b0dc0bde46794a224bd9 Personal repository: git://git.code.sf.net/u/thienhuynh/review Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesn OpenSAF servicesy Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** revision 4583fe8ec09727bb56ad703bdf10ccae81e5043f Author: thien.m.huynh Date: Mon, 7 Oct 2019 09:23:14 +0700 ntfd: Do not send response to client if client down [#3084] Ntfd will not send response to a client when client already down. This will avoid timeout when ntfd send via mds. Complete diffstat: -- src/ntf/ntfd/NtfAdmin.cc | 61 src/ntf/ntfd/NtfAdmin.h | 1 + src/ntf/ntfd/ntfs_cb.h | 6 + src/ntf/ntfd/ntfs_com.c | 4 src/ntf/ntfd/ntfs_com.h | 1 + src/ntf/ntfd/ntfs_evt.c | 1 + src/ntf/ntfd/ntfs_mds.c | 2 +- 7 files changed, 75 insertions(+), 1 deletion(-) Testing Commands: - N/A Testing, Expected Results: -- N/A Conditions of Submission: - ACK from 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 1/1] amf: add asserts to problematic areas identified by codechecker [#3077]
Hi Gary,
ACK from me.
Best Regards,
ThuanTr
-Original Message-
From: Gary Lee
Sent: Thursday, October 3, 2019 12:11 PM
To: thuan.t...@dektech.com.au; minh.c...@dektech.com.au;
hans.nordeb...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net; Gary Lee
Subject: [PATCH 1/1] amf: add asserts to problematic areas identified by
codechecker [#3077]
---
src/amf/amfd/sg_nway_fsm.cc | 2 ++
src/amf/amfd/sgtype.cc | 1 +
src/amf/amfnd/comp.cc | 2 ++
src/amf/amfnd/susm.cc | 1 +
4 files changed, 6 insertions(+)
diff --git a/src/amf/amfd/sg_nway_fsm.cc b/src/amf/amfd/sg_nway_fsm.cc index
f7ddc57..2c17b5a 100644
--- a/src/amf/amfd/sg_nway_fsm.cc
+++ b/src/amf/amfd/sg_nway_fsm.cc
@@ -2589,6 +2589,8 @@ static AVD_SU_SI_REL
*find_pref_standby_susi(AVD_SU_SI_REL *sisu) {
TRACE_ENTER();
+ osafassert(sisu != nullptr);
+ osafassert(sisu->si != nullptr);
curr_sisu = sisu->si->list_of_sisu;
while (curr_sisu) {
if ((SA_AMF_READINESS_IN_SERVICE ==
curr_sisu->su->saAmfSuReadinessState) && diff --git a/src/amf/amfd/sgtype.cc
b/src/amf/amfd/sgtype.cc index 15fae9c..64ebbd7 100644
--- a/src/amf/amfd/sgtype.cc
+++ b/src/amf/amfd/sgtype.cc
@@ -439,6 +439,7 @@ static void sgtype_ccb_apply_modify_hdlr(struct
CcbUtilOperationData *opdata) {
LOG_WA("SGT modify apply (STDBY): sgt does not exist");
return;
}
+ osafassert(sgt != nullptr);
while ((attr_mod = opdata->param.modify.attrMods[i++]) != nullptr) {
bool value_is_deleted;
diff --git a/src/amf/amfnd/comp.cc b/src/amf/amfnd/comp.cc index
b520550..a12171c 100644
--- a/src/amf/amfnd/comp.cc
+++ b/src/amf/amfnd/comp.cc
@@ -2785,6 +2785,7 @@ uint32_t comp_restart_initiate(AVND_COMP *comp) {
// reset contained comps for this container
AVND_COMP_CSI_REC *curr_csi(m_AVND_CSI_REC_FROM_COMP_DLL_NODE_GET(
m_NCS_DBLIST_FIND_FIRST(>csi_list)));
+osafassert(curr_csi != nullptr);
const std::string& containerCsi(curr_csi->name);
for (auto : cb->compdb) {
@@ -2837,6 +2838,7 @@ uint32_t comp_restart_initiate(AVND_COMP *comp) {
if (!m_AVND_COMP_TYPE_IS_PREINSTANTIABLE(comp)) {
AVND_COMP_CSI_REC *csi = m_AVND_CSI_REC_FROM_COMP_DLL_NODE_GET(
m_NCS_DBLIST_FIND_FIRST(>csi_list));
+osafassert(csi != nullptr);
if (m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_ASSIGNED(csi) ||
m_AVND_COMP_CSI_CURR_ASSIGN_STATE_IS_RESTARTING(csi)) {
m_AVND_COMP_CSI_CURR_ASSIGN_STATE_SET(
diff --git a/src/amf/amfnd/susm.cc b/src/amf/amfnd/susm.cc index
6376327..c1aa9e4 100644
--- a/src/amf/amfnd/susm.cc
+++ b/src/amf/amfnd/susm.cc
@@ -392,6 +392,7 @@ uint32_t avnd_su_si_msg_prc(AVND_CB *cb, AVND_SU *su,
AVND_SU_SI_PARAM *info) {
if (true == info->single_csi) {
AVND_COMP_CSI_PARAM *csi_param;
AVND_COMP_CSI_REC *csi_rec;
+osafassert(si != nullptr);
si->single_csi_add_rem_in_si = AVSV_SUSI_ACT_DEL;
osafassert((info->num_assigns == 1));
csi_param = info->list;
--
2.7.4
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 0/2] Review Request for mds: Add Nack message for MDS_TIPC_FCTRL_ENABLED [#3095] V2
Hi, I would like to push the patches today if no more comment for them. Thanks Minh On 4/10/19 3:20 pm, Minh Chau wrote: Summary: mds: Add Nack message for MDS_TIPC_FCTRL_ENABLED [#3095] V2 Review request for Ticket(s): 3095 Peer Reviewer(s): Hans, Vu, Gary, Thuan Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-3095 Base revision: 05064a1cfd0aeaf824dce7602d535654b3457e30 Personal repository: git://git.code.sf.net/u/minh-chau/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): - *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** revision cbbeab8f2299620aa3eb9b0e29710a2b159b5a45 Author: Minh Chau Date: Fri, 4 Oct 2019 12:59:27 +1000 mds: Improve error log for MDS_TIPC_FCTRL_ENABLED [#3095] This commit as part of #3095 updates the error string with pattern "FCTRL:*Error[*]", in order to help grep-ing the error in mds debug log. revision cc666586717fa82df70471748d8766e8fe901460 Author: Minh Chau Date: Fri, 4 Oct 2019 12:59:16 +1000 mds: Add Nack message for MDS_TIPC_FCTRL_ENABLED [#3095] In the scenario of recovery from split-brain, where both active director services may suffer mds message loss due to lost-contact tipc link. If MDS_TIPC_FCTRL_ENABLED is set, the out-of-order message will be dropped, and there is no mechanism to trigger the retransmission from receiver side at this moment (the retransmission is only triggered from sender as result of TIPC_ERR_OVERLOAD). In reception of disordered message, the receiver can send not-acknowledgement to notify the sender for retransmission. Therefore, the sender can trigger retransmisison in the same way as receiving TIPC_ERR_OVERLOAD. This patch adds Nack message for retransmission of disordered message detected from receiver side, and adds a missing call to portid_map_mutex.unlock() in process_all_events(). Complete diffstat: -- src/mds/mds_c_api.c | 2 +- src/mds/mds_dt_common.c | 2 +- src/mds/mds_tipc_fctrl_intf.cc | 72 +--- src/mds/mds_tipc_fctrl_msg.cc| 35 ++- src/mds/mds_tipc_fctrl_msg.h | 22 src/mds/mds_tipc_fctrl_portid.cc | 42 --- src/mds/mds_tipc_fctrl_portid.h | 3 +- 7 files changed, 143 insertions(+), 35 deletions(-) Testing Commands: - *** LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES *** Testing, Expected Results: -- *** PASTE COMMAND OUTPUTS / TEST RESULTS *** Conditions of Submission: - *** HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC *** Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 n n 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
[devel] [PATCH 1/1] mds: Enhance decoding for mds flow control message [#3097]
mds currently uses MDS_PROT_FCTRL_ID 4 bytes value (0x00AC13F5)
from octet11 to octet14 to identify the flow control message
e.g., chunkack message. In case of fragmentation from big
message, the second fragment onwards will start from the octet11,
which may have arbitrary value and cause mds to incorrectly
decode as a flow control message if the fragment starts with
value of 0x00AC13F5.
This patch fixes this rare case by decoding flow control message
only if the oct2-5 (mds global sequence number) and oct6-7 (mds
fragment number) are non-zero. Change MDS_PROT_FCTRL_ID:0xFDAC13F5
---
src/mds/mds_dt.h | 2 +-
src/mds/mds_tipc_fctrl_msg.cc | 20 +---
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/src/mds/mds_dt.h b/src/mds/mds_dt.h
index d9e8633..64da600 100644
--- a/src/mds/mds_dt.h
+++ b/src/mds/mds_dt.h
@@ -245,7 +245,7 @@ bool mdtm_mailbox_mbx_cleanup(NCSCONTEXT arg, NCSCONTEXT
msg);
/* MDS protocol/version for flow control */
#define MDS_PROT_FCTRL (0xB0 | MDS_VERSION)
-#define MDS_PROT_FCTRL_ID 0x00AC13F5
+#define MDS_PROT_FCTRL_ID 0xFDAC13F5
/* Added for the subscription changes */
#define MDS_NCS_CHASSIS_ID (m_NCS_GET_NODE_ID & 0x00ff)
diff --git a/src/mds/mds_tipc_fctrl_msg.cc b/src/mds/mds_tipc_fctrl_msg.cc
index 064d977..8375673 100644
--- a/src/mds/mds_tipc_fctrl_msg.cc
+++ b/src/mds/mds_tipc_fctrl_msg.cc
@@ -64,13 +64,19 @@ void HeaderMessage::Decode(uint8_t *msg) {
// decode flow control sequence number
ptr = [HeaderMessage::FieldIndex::kFlowControlSequenceNumber];
fseq_ = ncs_decode_16bit();
-// decode protocol identifier
-ptr = [ChunkAck::FieldIndex::kProtocolIdentifier];
-pro_id_ = ncs_decode_32bit();
-if (pro_id_ == MDS_PROT_FCTRL_ID) {
- // decode message type
- ptr = [ChunkAck::FieldIndex::kFlowControlMessageType];
- msg_type_ = ncs_decode_8bit();
+// decode protocol identifier if the mfrag_ and mseq_ are 0
+// otherwise it is always DataMessage within non-zero mseq_ and mfrag_
+if (mfrag_ == 0 && mseq_ == 0) {
+ ptr = [ChunkAck::FieldIndex::kProtocolIdentifier];
+ pro_id_ = ncs_decode_32bit();
+ if (pro_id_ == MDS_PROT_FCTRL_ID) {
+// decode message type
+ptr = [ChunkAck::FieldIndex::kFlowControlMessageType];
+msg_type_ = ncs_decode_8bit();
+ }
+} else {
+ pro_id_ = 0;
+ msg_type_ = 0;
}
} else {
if (mfrag_ != 0) {
--
2.7.4
___
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: Enhance decoding for mds flow control message [#3097]
Summary: mds: Enhance decoding for mds flow control message [#3097] Review request for Ticket(s): 3097 Peer Reviewer(s): Hans, Vu, Gary, Thuan Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-3097 Base revision: e699c22ddc1ca8530318b0dc0bde46794a224bd9 Personal repository: git://git.code.sf.net/u/minh-chau/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): - *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** revision b3a4d9597a6738bcad7a65be2a050382adb5b9ff Author: Minh Chau Date: Mon, 7 Oct 2019 11:04:35 +1100 mds: Enhance decoding for mds flow control message [#3097] mds currently uses MDS_PROT_FCTRL_ID 4 bytes value (0x00AC13F5) from octet11 to octet14 to identify the flow control message e.g., chunkack message. In case of fragmentation from big message, the second fragment onwards will start from the octet11, which may have arbitrary value and cause mds to incorrectly decode as a flow control message if the fragment starts with value of 0x00AC13F5. This patch fixes this rare case by decoding flow control message only if the oct2-5 (mds global sequence number) and oct6-7 (mds fragment number) are non-zero. Change MDS_PROT_FCTRL_ID:0xFDAC13F5 Complete diffstat: -- src/mds/mds_dt.h | 2 +- src/mds/mds_tipc_fctrl_msg.cc | 20 +--- 2 files changed, 14 insertions(+), 8 deletions(-) Testing Commands: - *** LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES *** Testing, Expected Results: -- *** PASTE COMMAND OUTPUTS / TEST RESULTS *** Conditions of Submission: - *** HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC *** 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
