Re: [devel] [PATCH 1/1] dtm: Fix dtm close socket due to duplication of adding node IP info [#2984]
Hi Gary and Hans,
Thanks for your review.
I updated Gary comment and running patch test again.
@Hans. Because there is just small changes that does not related to the ticket.
(the changes in dtm_node_delete() and dtm_node_add() with replace number by
enum) so I don’t separate the patch.
I will send the new after patch test ran.
Regards
Canh
-Original Message-
From: Gary Lee
Sent: Thursday, March 7, 2019 6:58 AM
To: Hans Nordebäck ; Canh Van Truong
; Anders Widell ;
Minh Hon Chau
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] dtm: Fix dtm close socket due to duplication of adding
node IP info [#2984]
Hi Canh
One minor comment, KEY_TYPES should probably be called KeyTypes. Also,
can you make it an enum class, rather than plain enum?
Thanks
Gary
On 7/3/19 12:53 am, Hans Nordebäck wrote:
> Hi Canh,
>
> ack, review only. I think it would be good to separate the re-factoring
> part in a separate ticket though.
>
> /BR Hans
>
> On 12/18/18 08:25, Canh Van Truong wrote:
>> During cluster start, one node (node 1) broadcast up msg to other node. The
>> remote node (node 2) get this msg and send the connection to node 1
>> (connect()).
>> Similarly node 1 send the connection to node 2 after node 2 broadcast up
>> msg to.
>> Beside of node 2 connect() to node 1, node 2 also add the IP and ID info of
>> node 1 to database.
>> But before of that, node 2 may also accept the connection that come from
>> node 1. The
>> acception is also add node ID of node 1. So there is 2 times adding the node
>> ID
>> info of node 1 to database in node 2. This causes the socket connection is
>> closed
>> and node is restart again.
>>
>> The patch change to retrieve node from database by node IP instead node ID in
>> processing connection. This will reject the double of establishing connection
>> between 2 nodes and also double of adding node IP to database.
>> ---
>>src/dtm/dtmnd/dtm.h | 11 --
>>src/dtm/dtmnd/dtm_inter_trans.cc | 3 +-
>>src/dtm/dtmnd/dtm_node.cc | 2 +-
>>src/dtm/dtmnd/dtm_node_db.cc | 79
>> ---
>>src/dtm/dtmnd/dtm_node_sockets.cc | 20 ++
>>5 files changed, 72 insertions(+), 43 deletions(-)
>>
>> diff --git a/src/dtm/dtmnd/dtm.h b/src/dtm/dtmnd/dtm.h
>> index 28c811e65..a06b8f503 100644
>> --- a/src/dtm/dtmnd/dtm.h
>> +++ b/src/dtm/dtmnd/dtm.h
>> @@ -45,6 +45,11 @@ typedef enum {
>> DTM_MBX_MSG_TYPE = 5,
>>} MBX_POST_TYPES;
>>
>> +typedef enum {
>> + DTM_NODE_ID_KEY_TYPE = 0,
>> + DTM_NODE_IP_KEY_TYPE = 2,
>> +} KEY_TYPES;
>> +
>>typedef struct dtm_rcv_msg_elem {
>> void *next;
>> MBX_POST_TYPES type;
>> @@ -99,10 +104,10 @@ typedef struct dtm_snd_msg_elem {
>>
>>extern void node_discovery_process(void *arg);
>>extern uint32_t dtm_cb_init(DTM_INTERNODE_CB *dtms_cb);
>> -extern DTM_NODE_DB *dtm_node_get_by_id(uint32_t nodeid);
>> +extern DTM_NODE_DB *dtm_node_get(uint8_t *key, KEY_TYPES type);
>>extern DTM_NODE_DB *dtm_node_getnext_by_id(uint32_t node_id);
>> -extern uint32_t dtm_node_add(DTM_NODE_DB *node, int i);
>> -extern uint32_t dtm_node_delete(DTM_NODE_DB *nnode, int i);
>> +extern uint32_t dtm_node_add(DTM_NODE_DB *node, KEY_TYPES type);
>> +extern uint32_t dtm_node_delete(DTM_NODE_DB *nnode, KEY_TYPES type);
>>extern DTM_NODE_DB *dtm_node_new(const DTM_NODE_DB *new_node);
>>extern void dtm_print_config(DTM_INTERNODE_CB *config);
>>extern int dtm_read_config(DTM_INTERNODE_CB *config,
>> diff --git a/src/dtm/dtmnd/dtm_inter_trans.cc
>> b/src/dtm/dtmnd/dtm_inter_trans.cc
>> index 9d8335466..9b4194614 100644
>> --- a/src/dtm/dtmnd/dtm_inter_trans.cc
>> +++ b/src/dtm/dtmnd/dtm_inter_trans.cc
>> @@ -235,9 +235,10 @@ static uint32_t
>> dtm_internode_snd_msg_common(DTM_NODE_DB *node, uint8_t *buffer,
>>uint32_t dtm_internode_snd_msg_to_node(uint8_t *buffer, uint16_t len,
>> NODE_ID node_id) {
>> DTM_NODE_DB *node = nullptr;
>> + uint8_t *key = reinterpret_cast(_id);
>>
>> TRACE_ENTER();
>> - node = dtm_node_get_by_id(node_id);
>> + node = dtm_node_get(key, DTM_NODE_ID_KEY_TYPE);
>>
>> if (nullptr != node) {
>>if (NCSCC_RC_SUCCESS != dtm_internode_snd_msg_common(node, buffer,
>> len)) {
>> diff --git a/src/dtm/dtmnd/dtm_node.cc b/src/dtm/dtmnd/dtm_node.cc
>> index de2f94738..72506f262 100644
>> --- a/src/dtm/dtmnd/dtm_node.cc
>> +++ b/src/dtm/dtmnd/dtm_node.cc
>> @@ -125,7 +125,7 @@ uint32_t dtm_process_node_info(DTM_INTERNODE_CB
>> *dtms_cb, DTM_NODE_DB *node,
>> memcpy(node->node_name, data, nodename_len);
>> node->node_name[nodename_len] = '\0';
>> node->comm_status = true;
>> - if (dtm_node_add(node, 0) != NCSCC_RC_SUCCESS) {
>> + if (dtm_node_add(node, DTM_NODE_ID_KEY_TYPE) != NCSCC_RC_SUCCESS) {
>>LOG_ER(
>>"DTM: A node already exists in the cluster with
Re: [devel] [PATCH 1/1] log: Fix the last value of logRecordDestinationConfiguration is not deleted [#3014]
Hi Canh,
Ack. Thanks.
Regards, Vu
> -Original Message-
> From: Canh Van Truong
> Sent: Friday, March 1, 2019 5:36 PM
> To: lennart.l...@ericsson.com; vu.m.ngu...@dektech.com.au
> Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
>
> Subject: [PATCH 1/1] log: Fix the last value of
> logRecordDestinationConfiguration is not deleted [#3014]
>
> When using the command "immcfg logConfig=1,safApp=safLogService -a
> logRecordDestinationConfiguration-="
> If there is just one value in the multi attribute and we delete last one,
the
> value
> will be deleted in imm database but it is not deleted in LGD.
>
> The patch update the attribute "logRecordDestinationConfiguration" = empty
> in lgd
> after the last value is deleted.
> ---
> src/log/logd/lgs_config.cc | 14 ++
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/src/log/logd/lgs_config.cc b/src/log/logd/lgs_config.cc
> index f096b040f..44e10b84d 100644
> --- a/src/log/logd/lgs_config.cc
> +++ b/src/log/logd/lgs_config.cc
> @@ -306,10 +306,16 @@ void lgs_cfgupd_multival_delete(const std::string
> _name,
> }
>}
>
> - // Add this new list to the config data list
> - for (const auto : result_list) {
> -lgs_cfgupd_list_create(attribute_name.c_str(),
> - const_cast(value.c_str()),
config_data);
> + if (result_list.empty()) {
> +// Delete the last value. Create config_data with empty value
> +lgs_cfgupd_list_create(attribute_name.c_str(), const_cast(""),
> + config_data);
> + } else {
> +// Add this new list to the config data list
> +for (const auto : result_list) {
> + lgs_cfgupd_list_create(attribute_name.c_str(),
> + const_cast(value.c_str()),
config_data);
> +}
>}
>TRACE_LEAVE();
> }
> --
> 2.15.1
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/1] dtm: Fix dtm close socket due to duplication of adding node IP info [#2984]
Hi Canh
One minor comment, KEY_TYPES should probably be called KeyTypes. Also,
can you make it an enum class, rather than plain enum?
Thanks
Gary
On 7/3/19 12:53 am, Hans Nordebäck wrote:
Hi Canh,
ack, review only. I think it would be good to separate the re-factoring
part in a separate ticket though.
/BR Hans
On 12/18/18 08:25, Canh Van Truong wrote:
During cluster start, one node (node 1) broadcast up msg to other node. The
remote node (node 2) get this msg and send the connection to node 1 (connect()).
Similarly node 1 send the connection to node 2 after node 2 broadcast up msg
to.
Beside of node 2 connect() to node 1, node 2 also add the IP and ID info of
node 1 to database.
But before of that, node 2 may also accept the connection that come from node
1. The
acception is also add node ID of node 1. So there is 2 times adding the node ID
info of node 1 to database in node 2. This causes the socket connection is
closed
and node is restart again.
The patch change to retrieve node from database by node IP instead node ID in
processing connection. This will reject the double of establishing connection
between 2 nodes and also double of adding node IP to database.
---
src/dtm/dtmnd/dtm.h | 11 --
src/dtm/dtmnd/dtm_inter_trans.cc | 3 +-
src/dtm/dtmnd/dtm_node.cc | 2 +-
src/dtm/dtmnd/dtm_node_db.cc | 79
---
src/dtm/dtmnd/dtm_node_sockets.cc | 20 ++
5 files changed, 72 insertions(+), 43 deletions(-)
diff --git a/src/dtm/dtmnd/dtm.h b/src/dtm/dtmnd/dtm.h
index 28c811e65..a06b8f503 100644
--- a/src/dtm/dtmnd/dtm.h
+++ b/src/dtm/dtmnd/dtm.h
@@ -45,6 +45,11 @@ typedef enum {
DTM_MBX_MSG_TYPE = 5,
} MBX_POST_TYPES;
+typedef enum {
+ DTM_NODE_ID_KEY_TYPE = 0,
+ DTM_NODE_IP_KEY_TYPE = 2,
+} KEY_TYPES;
+
typedef struct dtm_rcv_msg_elem {
void *next;
MBX_POST_TYPES type;
@@ -99,10 +104,10 @@ typedef struct dtm_snd_msg_elem {
extern void node_discovery_process(void *arg);
extern uint32_t dtm_cb_init(DTM_INTERNODE_CB *dtms_cb);
-extern DTM_NODE_DB *dtm_node_get_by_id(uint32_t nodeid);
+extern DTM_NODE_DB *dtm_node_get(uint8_t *key, KEY_TYPES type);
extern DTM_NODE_DB *dtm_node_getnext_by_id(uint32_t node_id);
-extern uint32_t dtm_node_add(DTM_NODE_DB *node, int i);
-extern uint32_t dtm_node_delete(DTM_NODE_DB *nnode, int i);
+extern uint32_t dtm_node_add(DTM_NODE_DB *node, KEY_TYPES type);
+extern uint32_t dtm_node_delete(DTM_NODE_DB *nnode, KEY_TYPES type);
extern DTM_NODE_DB *dtm_node_new(const DTM_NODE_DB *new_node);
extern void dtm_print_config(DTM_INTERNODE_CB *config);
extern int dtm_read_config(DTM_INTERNODE_CB *config,
diff --git a/src/dtm/dtmnd/dtm_inter_trans.cc b/src/dtm/dtmnd/dtm_inter_trans.cc
index 9d8335466..9b4194614 100644
--- a/src/dtm/dtmnd/dtm_inter_trans.cc
+++ b/src/dtm/dtmnd/dtm_inter_trans.cc
@@ -235,9 +235,10 @@ static uint32_t dtm_internode_snd_msg_common(DTM_NODE_DB
*node, uint8_t *buffer,
uint32_t dtm_internode_snd_msg_to_node(uint8_t *buffer, uint16_t len,
NODE_ID node_id) {
DTM_NODE_DB *node = nullptr;
+ uint8_t *key = reinterpret_cast(_id);
TRACE_ENTER();
- node = dtm_node_get_by_id(node_id);
+ node = dtm_node_get(key, DTM_NODE_ID_KEY_TYPE);
if (nullptr != node) {
if (NCSCC_RC_SUCCESS != dtm_internode_snd_msg_common(node, buffer, len))
{
diff --git a/src/dtm/dtmnd/dtm_node.cc b/src/dtm/dtmnd/dtm_node.cc
index de2f94738..72506f262 100644
--- a/src/dtm/dtmnd/dtm_node.cc
+++ b/src/dtm/dtmnd/dtm_node.cc
@@ -125,7 +125,7 @@ uint32_t dtm_process_node_info(DTM_INTERNODE_CB *dtms_cb,
DTM_NODE_DB *node,
memcpy(node->node_name, data, nodename_len);
node->node_name[nodename_len] = '\0';
node->comm_status = true;
- if (dtm_node_add(node, 0) != NCSCC_RC_SUCCESS) {
+ if (dtm_node_add(node, DTM_NODE_ID_KEY_TYPE) != NCSCC_RC_SUCCESS) {
LOG_ER(
"DTM: A node already exists in the cluster with similar "
"configuration (possible duplicate IP address and/or node id), please
"
diff --git a/src/dtm/dtmnd/dtm_node_db.cc b/src/dtm/dtmnd/dtm_node_db.cc
index 1c9da4dac..1038f0918 100644
--- a/src/dtm/dtmnd/dtm_node_db.cc
+++ b/src/dtm/dtmnd/dtm_node_db.cc
@@ -123,24 +123,49 @@ uint32_t dtm_cb_init(DTM_INTERNODE_CB *dtms_cb) {
}
/**
- * Retrieve node from node db by nodeid
+ * Retrieve node from node db
*
- * @param nodeid
+ * @param key
+ * @param i
*
- * @return NCSCC_RC_SUCCESS
- * @return NCSCC_RC_FAILURE
+ * @return node
*
*/
-DTM_NODE_DB *dtm_node_get_by_id(uint32_t nodeid) {
+DTM_NODE_DB *dtm_node_get(uint8_t *key, KEY_TYPES type) {
TRACE_ENTER();
DTM_INTERNODE_CB *dtms_cb = dtms_gl_cb;
+ DTM_NODE_DB *node = nullptr;
- DTM_NODE_DB *node = reinterpret_cast(ncs_patricia_tree_get(
- _cb->nodeid_tree, reinterpret_cast()));
- if (node !=
Re: [devel] [PATCH 1/1] dtm: Fix dtm close socket due to duplication of adding node IP info [#2984]
Hi Canh,
ack, review only. I think it would be good to separate the re-factoring
part in a separate ticket though.
/BR Hans
On 12/18/18 08:25, Canh Van Truong wrote:
> During cluster start, one node (node 1) broadcast up msg to other node. The
> remote node (node 2) get this msg and send the connection to node 1
> (connect()).
> Similarly node 1 send the connection to node 2 after node 2 broadcast up msg
> to.
> Beside of node 2 connect() to node 1, node 2 also add the IP and ID info of
> node 1 to database.
> But before of that, node 2 may also accept the connection that come from node
> 1. The
> acception is also add node ID of node 1. So there is 2 times adding the node
> ID
> info of node 1 to database in node 2. This causes the socket connection is
> closed
> and node is restart again.
>
> The patch change to retrieve node from database by node IP instead node ID in
> processing connection. This will reject the double of establishing connection
> between 2 nodes and also double of adding node IP to database.
> ---
> src/dtm/dtmnd/dtm.h | 11 --
> src/dtm/dtmnd/dtm_inter_trans.cc | 3 +-
> src/dtm/dtmnd/dtm_node.cc | 2 +-
> src/dtm/dtmnd/dtm_node_db.cc | 79
> ---
> src/dtm/dtmnd/dtm_node_sockets.cc | 20 ++
> 5 files changed, 72 insertions(+), 43 deletions(-)
>
> diff --git a/src/dtm/dtmnd/dtm.h b/src/dtm/dtmnd/dtm.h
> index 28c811e65..a06b8f503 100644
> --- a/src/dtm/dtmnd/dtm.h
> +++ b/src/dtm/dtmnd/dtm.h
> @@ -45,6 +45,11 @@ typedef enum {
> DTM_MBX_MSG_TYPE = 5,
> } MBX_POST_TYPES;
>
> +typedef enum {
> + DTM_NODE_ID_KEY_TYPE = 0,
> + DTM_NODE_IP_KEY_TYPE = 2,
> +} KEY_TYPES;
> +
> typedef struct dtm_rcv_msg_elem {
> void *next;
> MBX_POST_TYPES type;
> @@ -99,10 +104,10 @@ typedef struct dtm_snd_msg_elem {
>
> extern void node_discovery_process(void *arg);
> extern uint32_t dtm_cb_init(DTM_INTERNODE_CB *dtms_cb);
> -extern DTM_NODE_DB *dtm_node_get_by_id(uint32_t nodeid);
> +extern DTM_NODE_DB *dtm_node_get(uint8_t *key, KEY_TYPES type);
> extern DTM_NODE_DB *dtm_node_getnext_by_id(uint32_t node_id);
> -extern uint32_t dtm_node_add(DTM_NODE_DB *node, int i);
> -extern uint32_t dtm_node_delete(DTM_NODE_DB *nnode, int i);
> +extern uint32_t dtm_node_add(DTM_NODE_DB *node, KEY_TYPES type);
> +extern uint32_t dtm_node_delete(DTM_NODE_DB *nnode, KEY_TYPES type);
> extern DTM_NODE_DB *dtm_node_new(const DTM_NODE_DB *new_node);
> extern void dtm_print_config(DTM_INTERNODE_CB *config);
> extern int dtm_read_config(DTM_INTERNODE_CB *config,
> diff --git a/src/dtm/dtmnd/dtm_inter_trans.cc
> b/src/dtm/dtmnd/dtm_inter_trans.cc
> index 9d8335466..9b4194614 100644
> --- a/src/dtm/dtmnd/dtm_inter_trans.cc
> +++ b/src/dtm/dtmnd/dtm_inter_trans.cc
> @@ -235,9 +235,10 @@ static uint32_t dtm_internode_snd_msg_common(DTM_NODE_DB
> *node, uint8_t *buffer,
> uint32_t dtm_internode_snd_msg_to_node(uint8_t *buffer, uint16_t len,
> NODE_ID node_id) {
> DTM_NODE_DB *node = nullptr;
> + uint8_t *key = reinterpret_cast(_id);
>
> TRACE_ENTER();
> - node = dtm_node_get_by_id(node_id);
> + node = dtm_node_get(key, DTM_NODE_ID_KEY_TYPE);
>
> if (nullptr != node) {
> if (NCSCC_RC_SUCCESS != dtm_internode_snd_msg_common(node, buffer,
> len)) {
> diff --git a/src/dtm/dtmnd/dtm_node.cc b/src/dtm/dtmnd/dtm_node.cc
> index de2f94738..72506f262 100644
> --- a/src/dtm/dtmnd/dtm_node.cc
> +++ b/src/dtm/dtmnd/dtm_node.cc
> @@ -125,7 +125,7 @@ uint32_t dtm_process_node_info(DTM_INTERNODE_CB *dtms_cb,
> DTM_NODE_DB *node,
> memcpy(node->node_name, data, nodename_len);
> node->node_name[nodename_len] = '\0';
> node->comm_status = true;
> - if (dtm_node_add(node, 0) != NCSCC_RC_SUCCESS) {
> + if (dtm_node_add(node, DTM_NODE_ID_KEY_TYPE) != NCSCC_RC_SUCCESS) {
> LOG_ER(
> "DTM: A node already exists in the cluster with similar "
> "configuration (possible duplicate IP address and/or node id),
> please "
> diff --git a/src/dtm/dtmnd/dtm_node_db.cc b/src/dtm/dtmnd/dtm_node_db.cc
> index 1c9da4dac..1038f0918 100644
> --- a/src/dtm/dtmnd/dtm_node_db.cc
> +++ b/src/dtm/dtmnd/dtm_node_db.cc
> @@ -123,24 +123,49 @@ uint32_t dtm_cb_init(DTM_INTERNODE_CB *dtms_cb) {
> }
>
> /**
> - * Retrieve node from node db by nodeid
> + * Retrieve node from node db
>*
> - * @param nodeid
> + * @param key
> + * @param i
>*
> - * @return NCSCC_RC_SUCCESS
> - * @return NCSCC_RC_FAILURE
> + * @return node
>*
>*/
> -DTM_NODE_DB *dtm_node_get_by_id(uint32_t nodeid) {
> +DTM_NODE_DB *dtm_node_get(uint8_t *key, KEY_TYPES type) {
> TRACE_ENTER();
> DTM_INTERNODE_CB *dtms_cb = dtms_gl_cb;
> + DTM_NODE_DB *node = nullptr;
>
> - DTM_NODE_DB *node = reinterpret_cast(ncs_patricia_tree_get(
> - _cb->nodeid_tree, reinterpret_cast()));
> - if (node != nullptr)
