Re: [devel] [PATCH 1/1] mbc: fix mbcsv loop forever while it is being dispatch ALL [#2899]
Hi Canh,
Ack. See my minor misspell comment below
Thanks
Lennart
> -Original Message-
> From: Canh Van Truong
> Sent: den 20 augusti 2018 10:48
> To: Minh Hon Chau ; Gary Lee
> ; Hans Nordebäck
> ; Lennart Lund
>
> Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
>
> Subject: [PATCH 1/1] mbc: fix mbcsv loop forever while it is being dispatch
> ALL [#2899]
>
> When processing "MBCSV_PEER_UP_MSG" msg in case dispatch all from
> user,
> the msg may be too old. The msg may be come from the node that already
> rebooted.
> This reason cause mbcsv loop forever in WHILE loop, because the active node
> cannot
> find the adest of peer msg.
>
> The solution is that find entity in peer list and compare if the peer node id
> that
> already exist. If the peer node id exist in the entity (get node id from peer
> anchor),
> this mean that mbcsv already processed the "MBCSV_PEER_UP_MSG" for
> that peer node.
> ---
> src/mbc/mbcsv_env.h | 2 --
> src/mbc/mbcsv_peer.c | 90
> ++--
> 2 files changed, 80 insertions(+), 12 deletions(-)
>
> diff --git a/src/mbc/mbcsv_env.h b/src/mbc/mbcsv_env.h
> index 350655bed..4d1f24310 100644
> --- a/src/mbc/mbcsv_env.h
> +++ b/src/mbc/mbcsv_env.h
> @@ -519,8 +519,6 @@ uint32_t mbcsv_hdl_dispatch_block(uint32_t
> mbcsv_hdl, SYSF_MBX mbx);
> /*
> * Peer discovery function prototypes.
> */
> -PEER_INST *mbcsv_search_and_return_peer(PEER_INST *peer_list,
> -MBCSV_ANCHOR anchor);
> PEER_INST *mbcsv_add_new_peer(CKPT_INST *ckpt, MBCSV_ANCHOR
> anchor);
> uint32_t mbcsv_shutdown_peer(PEER_INST *peer_ptr);
> uint32_t mbcsv_rmv_peer(CKPT_INST *ckpt, MBCSV_ANCHOR anchor);
> diff --git a/src/mbc/mbcsv_peer.c b/src/mbc/mbcsv_peer.c
> index f863591c4..a429ee9f8 100644
> --- a/src/mbc/mbcsv_peer.c
> +++ b/src/mbc/mbcsv_peer.c
> @@ -56,6 +56,73 @@ static const char *disc_trace[] = {"Peer UP msg", "Peer
> DOWN msg",
> "Peer INFO msg", "Peer INFO resp msg",
> "Peer Role change msg"
> "Invalid peer discovery msg"};
> +typedef enum {ANCHOR_SEARCH, NODE_ID_SEARCH} SearchMode;
> +
> +
> +/*
> *\
> +* PROCEDURE: search_peer_list
> +*
> +* Purpose: This function search MBCA peer list for a match of complete
> +* anchor value (all 64 bits) or Node Id (most significant 32 bits
> of
> +* anchor value)
> +*
> +* Input:peer_list - MBCSv peer list.
> +* anchor - Anchor value of the peer to be searched in the list.
> +* search_mode - See enum SearchMode for search alternatives
> +*
> +* Returns: Pointer to found peer instance or NULL if no instance found
> +*
> +* Notes:
> +*
> +\*
> */
> +static PEER_INST *search_peer_list(PEER_INST *peer_list,
> +MBCSV_ANCHOR anchor,
> +SearchMode search_mode)
> +{
> + PEER_INST *peer = NULL, *found_peer = NULL;
> + uint32_t node_id;
> + uint32_t peer_node_id =
> m_NCS_NODE_ID_FROM_MDS_DEST(anchor);
> +
> + for (peer = peer_list; peer != NULL; peer = peer->next) {
> + if (search_mode == ANCHOR_SEARCH) {
> + if (peer->peer_anchor == anchor) {
> + found_peer = peer;
> + break;
> + }
> + } else if (search_mode == NODE_ID_SEARCH) {
> + node_id =
> + m_NCS_NODE_ID_FROM_MDS_DEST(peer-
> >peer_anchor);
> + if (node_id == peer_node_id) {
> + found_peer = peer;
> + break;
> + }
> + } else {
> + TRACE("Unsupport search mode");
[Lennart] Misspelling. Should be TRACE("Unsupported search mode");
> + }
> + }
> + return found_peer;
> +}
> +
> +/*
> *\
> +* PROCEDURE: mbcsv_check_if_peer_node_id_exist
> +*
> +* Purpose: This function searches entire MBCA peer list for the peer entity
> +* and checking if the peer nodeId is exist in peer entity
> +*
> +* Input:peer_list - MBCSv peer list.
> +* anchor - Anchor value of the peer to be searched in the list.
> +*
> +* Returns: True/False
> +*
> +* Notes:
> +*
> +\*
> */
> +static bool check_if_peer_node_id_exist(PEER_INST *peer_list,
> + MBCSV_ANCHOR anchor)
> +{
> + PEER_INST *peer = search_peer_list(peer_list, anchor,
> NODE_ID_SEARCH);
> + return (peer != NULL);
> +}
>
>
> /**
>
Re: [devel] [PATCH 1/1] mbc: fix mbcsv loop forever while it is being dispatch ALL [#2899]
Hi Canh,
ack, code review only. /Thanks HansN
On 08/20/2018 10:48 AM, Canh Van Truong wrote:
When processing "MBCSV_PEER_UP_MSG" msg in case dispatch all from user,
the msg may be too old. The msg may be come from the node that already rebooted.
This reason cause mbcsv loop forever in WHILE loop, because the active node
cannot
find the adest of peer msg.
The solution is that find entity in peer list and compare if the peer node id
that
already exist. If the peer node id exist in the entity (get node id from peer
anchor),
this mean that mbcsv already processed the "MBCSV_PEER_UP_MSG" for that peer
node.
---
src/mbc/mbcsv_env.h | 2 --
src/mbc/mbcsv_peer.c | 90 ++--
2 files changed, 80 insertions(+), 12 deletions(-)
diff --git a/src/mbc/mbcsv_env.h b/src/mbc/mbcsv_env.h
index 350655bed..4d1f24310 100644
--- a/src/mbc/mbcsv_env.h
+++ b/src/mbc/mbcsv_env.h
@@ -519,8 +519,6 @@ uint32_t mbcsv_hdl_dispatch_block(uint32_t mbcsv_hdl,
SYSF_MBX mbx);
/*
* Peer discovery function prototypes.
*/
-PEER_INST *mbcsv_search_and_return_peer(PEER_INST *peer_list,
-MBCSV_ANCHOR anchor);
PEER_INST *mbcsv_add_new_peer(CKPT_INST *ckpt, MBCSV_ANCHOR anchor);
uint32_t mbcsv_shutdown_peer(PEER_INST *peer_ptr);
uint32_t mbcsv_rmv_peer(CKPT_INST *ckpt, MBCSV_ANCHOR anchor);
diff --git a/src/mbc/mbcsv_peer.c b/src/mbc/mbcsv_peer.c
index f863591c4..a429ee9f8 100644
--- a/src/mbc/mbcsv_peer.c
+++ b/src/mbc/mbcsv_peer.c
@@ -56,6 +56,73 @@ static const char *disc_trace[] = {"Peer UP msg", "Peer DOWN
msg",
"Peer INFO msg", "Peer INFO resp msg",
"Peer Role change msg"
"Invalid peer discovery msg"};
+typedef enum {ANCHOR_SEARCH, NODE_ID_SEARCH} SearchMode;
+
+
+/**\
+* PROCEDURE: search_peer_list
+*
+* Purpose: This function search MBCA peer list for a match of complete
+* anchor value (all 64 bits) or Node Id (most significant 32 bits of
+* anchor value)
+*
+* Input:peer_list - MBCSv peer list.
+* anchor - Anchor value of the peer to be searched in the list.
+* search_mode - See enum SearchMode for search alternatives
+*
+* Returns: Pointer to found peer instance or NULL if no instance found
+*
+* Notes:
+*
+\**/
+static PEER_INST *search_peer_list(PEER_INST *peer_list,
+ MBCSV_ANCHOR anchor,
+ SearchMode search_mode)
+{
+ PEER_INST *peer = NULL, *found_peer = NULL;
+ uint32_t node_id;
+ uint32_t peer_node_id = m_NCS_NODE_ID_FROM_MDS_DEST(anchor);
+
+ for (peer = peer_list; peer != NULL; peer = peer->next) {
+ if (search_mode == ANCHOR_SEARCH) {
+ if (peer->peer_anchor == anchor) {
+ found_peer = peer;
+ break;
+ }
+ } else if (search_mode == NODE_ID_SEARCH) {
+ node_id =
+ m_NCS_NODE_ID_FROM_MDS_DEST(peer->peer_anchor);
+ if (node_id == peer_node_id) {
+ found_peer = peer;
+ break;
+ }
+ } else {
+ TRACE("Unsupport search mode");
+ }
+ }
+ return found_peer;
+}
+
+/**\
+* PROCEDURE: mbcsv_check_if_peer_node_id_exist
+*
+* Purpose: This function searches entire MBCA peer list for the peer entity
+* and checking if the peer nodeId is exist in peer entity
+*
+* Input:peer_list - MBCSv peer list.
+* anchor - Anchor value of the peer to be searched in the list.
+*
+* Returns: True/False
+*
+* Notes:
+*
+\**/
+static bool check_if_peer_node_id_exist(PEER_INST *peer_list,
+ MBCSV_ANCHOR anchor)
+{
+ PEER_INST *peer = search_peer_list(peer_list, anchor, NODE_ID_SEARCH);
+ return (peer != NULL);
+}
/**\
* PROCEDURE: mbcsv_search_and_return_peer
@@ -64,23 +131,17 @@ static const char *disc_trace[] = {"Peer UP msg", "Peer DOWN
msg",
* peer entity and returns peer pointer.
*
* Input:peer_list - MBCSv peer list.
-* anchor - Anchor value of the peer to be serched in the list.
+* anchor - Anchor value of the peer to be searched in the list.
*
* Returns: Peer instance pointer.
*
* Notes:
*
\**/
Re: [devel] [PATCH 1/1] mbc: fix mbcsv loop forever while it is being dispatch ALL [#2899]
Hi Canh,
ack, review only. One minor comment, instead of adding a similar function
mbcsv_check_if_peer_node_id_exist
reuse
mbcsv_search_and_return_peer
and add a new argument to this function (upper/lower/all part of
archword) instead. It is not that many place that
needs to be updated and code redundancy is avoided.
/Thanks HansN
On 07/30/2018 06:16 AM, Canh Van Truong wrote:
When processing "MBCSV_PEER_UP_MSG" msg in case dispatch all from user,
the msg may be too old. The msg may be come from the node that already rebooted.
This reason cause mbcsv loop forever in WHILE loop, because the active node
cannot
find the adest of peer msg.
The solution is that find entity in peer list and compare if the peer node id
that
already exist. If the peer node id exist in the entity (get node id from peer
anchor),
this mean that mbcsv already processed the "MBCSV_PEER_UP_MSG" for that peer
node.
---
src/mbc/mbcsv_peer.c | 41 -
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/src/mbc/mbcsv_peer.c b/src/mbc/mbcsv_peer.c
index f863591c4..736c015c2 100644
--- a/src/mbc/mbcsv_peer.c
+++ b/src/mbc/mbcsv_peer.c
@@ -64,7 +64,7 @@ static const char *disc_trace[] = {"Peer UP msg", "Peer DOWN
msg",
* peer entity and returns peer pointer.
*
* Input:peer_list - MBCSv peer list.
-* anchor - Anchor value of the peer to be serched in the list.
+* anchor - Anchor value of the peer to be searched in the list.
*
* Returns: Peer instance pointer.
*
@@ -83,6 +83,36 @@ PEER_INST *mbcsv_search_and_return_peer(PEER_INST *peer_list,
return NULL;
}
+/**\
+* PROCEDURE: mbcsv_check_if_peer_node_id_exist
+*
+* Purpose: This function searches entire MBCA peer list for the peer entity
+* and checking if the peer nodeId is exist in peer entity
+*
+* Input:peer_list - MBCSv peer list.
+* anchor - Anchor value of the peer to be searched in the list.
+*
+* Returns: True/False
+*
+* Notes:
+*
+\**/
+bool mbcsv_check_if_peer_node_id_exist(PEER_INST *peer_list,
+ MBCSV_ANCHOR anchor)
+{
+ PEER_INST *peer;
+ uint32_t node_id;
+ uint32_t peer_node_id = anchor >> 32;
+
+ for (peer = peer_list; peer != NULL; peer = peer->next) {
+ node_id = peer->peer_anchor >> 32;
+ if (node_id == peer_node_id)
+ return true;
+ }
+
+ return false;
+}
+
/**\
* PROCEDURE: mbcsv_add_new_peer
*
@@ -733,6 +763,15 @@ uint32_t mbcsv_process_peer_up_info(MBCSV_EVT *msg,
CKPT_INST *ckpt,
}
}
+ // If the node id exist in any peer entity of the peer list, we already
+ // got peer up message for that peer node. So This peer up message
+ // is invalid and just ignore it. Fixed bug #2899
+ if (mbcsv_check_if_peer_node_id_exist(
+ ckpt->peer_list, msg->rcvr_peer_key.peer_anchor) == true) {
+ TRACE_LEAVE2("Peer up message is too old. Just ignore this");
+ return NCSCC_RC_SUCCESS;
+ }
+
if (0 != (mbx = mbcsv_get_mbx(msg->rcvr_peer_key.pwe_hdl,
msg->rcvr_peer_key.svc_id))) {
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/1] mbc: fix mbcsv loop forever while it is being dispatch ALL [#2899]
Hi Canh,
Ack from me for code review. A minor comment.
Thanks
Minh
On 30/07/18 14:16, Canh Van Truong wrote:
When processing "MBCSV_PEER_UP_MSG" msg in case dispatch all from user,
the msg may be too old. The msg may be come from the node that already rebooted.
This reason cause mbcsv loop forever in WHILE loop, because the active node
cannot
find the adest of peer msg.
The solution is that find entity in peer list and compare if the peer node id
that
already exist. If the peer node id exist in the entity (get node id from peer
anchor),
this mean that mbcsv already processed the "MBCSV_PEER_UP_MSG" for that peer
node.
---
src/mbc/mbcsv_peer.c | 41 -
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/src/mbc/mbcsv_peer.c b/src/mbc/mbcsv_peer.c
index f863591c4..736c015c2 100644
--- a/src/mbc/mbcsv_peer.c
+++ b/src/mbc/mbcsv_peer.c
@@ -64,7 +64,7 @@ static const char *disc_trace[] = {"Peer UP msg", "Peer DOWN
msg",
* peer entity and returns peer pointer.
*
* Input:peer_list - MBCSv peer list.
-* anchor - Anchor value of the peer to be serched in the list.
+* anchor - Anchor value of the peer to be searched in the list.
*
* Returns: Peer instance pointer.
*
@@ -83,6 +83,36 @@ PEER_INST *mbcsv_search_and_return_peer(PEER_INST *peer_list,
return NULL;
}
+/**\
+* PROCEDURE: mbcsv_check_if_peer_node_id_exist
+*
+* Purpose: This function searches entire MBCA peer list for the peer entity
+* and checking if the peer nodeId is exist in peer entity
+*
+* Input:peer_list - MBCSv peer list.
+* anchor - Anchor value of the peer to be searched in the list.
+*
+* Returns: True/False
+*
+* Notes:
+*
+\**/
+bool mbcsv_check_if_peer_node_id_exist(PEER_INST *peer_list,
+ MBCSV_ANCHOR anchor)
+{
+ PEER_INST *peer;
+ uint32_t node_id;
+ uint32_t peer_node_id = anchor >> 32;
[Minh]: Maybe we can use m_NCS_NODE_ID_FROM_MDS_DEST?
+
+ for (peer = peer_list; peer != NULL; peer = peer->next) {
+ node_id = peer->peer_anchor >> 32;
+ if (node_id == peer_node_id)
+ return true;
+ }
+
+ return false;
+}
+
/**\
* PROCEDURE: mbcsv_add_new_peer
*
@@ -733,6 +763,15 @@ uint32_t mbcsv_process_peer_up_info(MBCSV_EVT *msg,
CKPT_INST *ckpt,
}
}
+ // If the node id exist in any peer entity of the peer list, we already
+ // got peer up message for that peer node. So This peer up message
+ // is invalid and just ignore it. Fixed bug #2899
+ if (mbcsv_check_if_peer_node_id_exist(
+ ckpt->peer_list, msg->rcvr_peer_key.peer_anchor) == true) {
+ TRACE_LEAVE2("Peer up message is too old. Just ignore this");
+ return NCSCC_RC_SUCCESS;
+ }
+
if (0 != (mbx = mbcsv_get_mbx(msg->rcvr_peer_key.pwe_hdl,
msg->rcvr_peer_key.svc_id))) {
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
