Re: [devel] [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to send response message [#3102]

2019-10-23 Thread Tran Thuan 
Hi Minh,

 

Please see my responses inline.

In general, I am trying to implement new version to reuse exist database.

 

Best Regards,

ThuanTr

 

From: Minh Hon Chau  
Sent: Wednesday, October 23, 2019 12:23 PM
To: Tran Thuan ; 'Nguyen Minh Vu' 
; hans.nordeb...@ericsson.com; 
gary@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to 
send response message [#3102]

 

Hi Thuan,

Please see comment inline

Thanks

Minh

On 23/10/19 3:32 pm, Tran Thuan wrote:

Hi Minh,

 

Thanks for comments. See my response inline.

Btw, I am preparing to send out new patch, I think I found an issue in current 
patch.

 

Best Regards,

ThuanTr

 

-Original Message-
From: Minh Hon Chau   
 
Sent: Wednesday, October 23, 2019 5:52 AM
To: Tran Thuan   ; 
'Nguyen Minh Vu'   
; hans.nordeb...@ericsson.com 
 ; gary@dektech.com.au 
 
Cc: opensaf-devel@lists.sourceforge.net 
 
Subject: Re: [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to 
send response message [#3102]

 

Hi Thuan,

 

I wonder the patch would work in the same reproduced steps if the both 

adests have subscribed each other more than 2 services. The svc_cnt will 

be greater than 1 until it is the last service down event. I think 

that's why mds has the database @subtn_results, in which each item is an 

adest associated with a service id separately.

[Thuan] We can understand that adest still alive, then go with origin flow 
(wait 1.5s).

But can a process send SNDRSP then mds unregister? I think it cannot, because 
it’s in SNDRSP (blocking)

[M] Not unregister, it can be unsubscribe. Or do you mean a process can not 
send two SNDRSP at the same time on 2 different subscribed services? 
[T] I think process cannot do anything relate to MDS API because it is under 
send SNDRSP (blocking).

The scenario of this ticket happens for the process terminated/crash.

[M] Yes my doubt is in the context of this ticket - terminated/crash - you 
would get 2 service down event I think

[T] Yes, will get 2 service down events.

[M] I don't think adding a new database at the global scope for this specific 
case is a good idea, if we can reuse the existing database. Can you try to use 
MDS_SUBSCRIPTION_INFO, add a flag or something similar to indicate which case 
mds should wait 1.5 sec. It would isolate the bug fix in the scope of this 
problem.

[T] I don’t want to mess up current database and its logic. But anyway, I am 
trying to reuse that database. Please wait for next version.

 

The problem originally resides at the services code e.g ntf, imm... 

where the threads structure between mds receiving thread and main thread 

cause a race condition. Thus the service sends a message with a death 

adest which is removed from mds database, that confuses mds and hit 1.5 

secs wait time.

 

If I read the code correctly, the 1.5 wait time is for another case, it 

gives another chance to wait 1.5 when the subscription result is empty 

in @subtn_results because the service up has not arrived yet.

[Thuan] Yes, it will give a chance if adest not yet UP any.

My patch still keep that chance as origin code.

But I think I need reduce timeout for adest down timer, I am verifying this 
change.

 

mds  subscribe >

 

mds  sends message A x

 

mds wait 1.5 sec

 

mds <--- service up 

 

mds  sends message A >

 

So the 1.5 sec time is for early phase of waiting service up.

 

} else if (sub_info->tmr_flag != true) {

if ((MDS_SENDTYPE_RSP == req->i_sendtype) ||

(MDS_SENDTYPE_RRSP == req->i_sendtype)) {

time_wait = true;

m_MDS_LOG_INFO(

"MDS_SND_RCV:Disc queue: Subscr exists no timer 

running: Waiting for some time\n");

 

-> I think at this line, it means: the SUBSCRIPTION_TMR has timeout, and 

mds is sending RSP/RRSP which means mds should have received the 

*request* message (?), so mds wants to wait for another 1.5 second for 

service_up to create the subscription result in database.

 

The problem in this ticket hit 1.5 because the service down has already 

come and mds removed the subscription result item, now the ntf, imm... 

sends msg with a death adest, and mds now it thinks it is waiting for a 

service up to come as at the early phase, so it waits. Both two 

scenarios can be distinguished themselves to avoid to wait 1.5 secs for 

the latter case I think.

 

Thanks

 

Minh

 

On 22/10/19 9:50 pm, Tran Thuan wrote:

> Hi Vu,

> 

> Thanks for additional comments.

> I reply your concerns inline below.

> 

> Best Regards,

> ThuanTr

> 

> -Original Message-

> From: Nguyen Minh Vu <  
>

Re: [devel] [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to send response message [#3102]

2019-10-22 Thread Minh Hon Chau 

Hi Thuan,

Please see comment inline

Thanks

Minh

On 23/10/19 3:32 pm, Tran Thuan wrote:


Hi Minh,

Thanks for comments. See my response inline.

Btw, I am preparing to send out new patch, I think I found an issue in 
current patch.


Best Regards,

ThuanTr

-Original Message-
From: Minh Hon Chau 
Sent: Wednesday, October 23, 2019 5:52 AM
To: Tran Thuan ; 'Nguyen Minh Vu' 
; hans.nordeb...@ericsson.com; 
gary@dektech.com.au

Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] mds: not waste 1.5s in waiting Adest already 
down to send response message [#3102]


Hi Thuan,

I wonder the patch would work in the same reproduced steps if the both

adests have subscribed each other more than 2 services. The svc_cnt will

be greater than 1 until it is the last service down event. I think

that's why mds has the database @subtn_results, in which each item is an

adest associated with a service id separately.

[Thuan] We can understand that adest still alive, then go with origin 
flow (wait 1.5s).


But can a process send SNDRSP then mds unregister? I think it cannot, 
because it’s in SNDRSP (blocking)


[M] Not unregister, it can be unsubscribe. Or do you mean a process can 
not send two SNDRSP at the same time on 2 different subscribed services?


The scenario of this ticket happens for the process terminated/crash.

[M] Yes my doubt is in the context of this ticket - terminated/crash - 
you would get 2 service down event I think


[M] I don't think adding a new database at the global scope for this 
specific case is a good idea, if we can reuse the existing database. Can 
you try to use MDS_SUBSCRIPTION_INFO, add a flag or something similar to 
indicate which case mds should wait 1.5 sec. It would isolate the bug 
fix in the scope of this problem.



The problem originally resides at the services code e.g ntf, imm...

where the threads structure between mds receiving thread and main thread

cause a race condition. Thus the service sends a message with a death

adest which is removed from mds database, that confuses mds and hit 1.5

secs wait time.

If I read the code correctly, the 1.5 wait time is for another case, it

gives another chance to wait 1.5 when the subscription result is empty

in @subtn_results because the service up has not arrived yet.

[Thuan] Yes, it will give a chance if adest not yet UP any.

My patch still keep that chance as origin code.

But I think I need reduce timeout for adest down timer, I am verifying 
this change.


mds  subscribe >

mds  sends message A x

mds wait 1.5 sec

mds <--- service up 

mds  sends message A >

So the 1.5 sec time is for early phase of waiting service up.

    } else if (sub_info->tmr_flag != true) {

        if ((MDS_SENDTYPE_RSP == req->i_sendtype) ||

            (MDS_SENDTYPE_RRSP == req->i_sendtype)) {

            time_wait = true;

            m_MDS_LOG_INFO(

                "MDS_SND_RCV:Disc queue: Subscr exists no timer

running: Waiting for some time\n");

-> I think at this line, it means: the SUBSCRIPTION_TMR has timeout, and

mds is sending RSP/RRSP which means mds should have received the

*request* message (?), so mds wants to wait for another 1.5 second for

service_up to create the subscription result in database.

The problem in this ticket hit 1.5 because the service down has already

come and mds removed the subscription result item, now the ntf, imm...

sends msg with a death adest, and mds now it thinks it is waiting for a

service up to come as at the early phase, so it waits. Both two

scenarios can be distinguished themselves to avoid to wait 1.5 secs for

the latter case I think.

Thanks

Minh

On 22/10/19 9:50 pm, Tran Thuan wrote:

> Hi Vu,

>

> Thanks for additional comments.

> I reply your concerns inline below.

>

> Best Regards,

> ThuanTr

>

> -Original Message-

> From: Nguyen Minh Vu >


> Sent: Tuesday, October 22, 2019 5:28 PM

> To: thuan.tran >; 'Minh Chau' 
mailto:minh.c...@dektech.com.au>>; 
hans.nordeb...@ericsson.com ; 
gary@dektech.com.au 


> Cc: opensaf-devel@lists.sourceforge.net 



> Subject: Re: [PATCH 1/1] mds: not waste 1.5s in waiting Adest 
already down to send response message [#3102]


>

> Hi Thuan,

>

> I have additional comments below.

>

> Regards, Vu

>

> On 10/22/19 7:14 AM, thuan.tran wrote:

>> - When sending response message which Adest not exist (already down)

>> current MDS try to wait for 1.5 seconds before conclude no route to

>> send response message.

>>

>> - There are 2 scenarios may have:

>> UP -> DOWN -> receive SNDRSP -> response timeout after 1.5s

>> UP -> receive SNDRSP -> DOWN -> response timeout after 1.5s

>>

>> - With this change, MDS will not waste for 1.5s which can cause trouble

>> for higher layer services, e.g:

Re: [devel] [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to send response message [#3102]

2019-10-22 Thread Tran Thuan 
Hi Minh,

 

Thanks for comments. See my response inline.

Btw, I am preparing to send out new patch, I think I found an issue in current 
patch.

 

Best Regards,

ThuanTr

 

-Original Message-
From: Minh Hon Chau  
Sent: Wednesday, October 23, 2019 5:52 AM
To: Tran Thuan ; 'Nguyen Minh Vu' 
; hans.nordeb...@ericsson.com; 
gary@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to 
send response message [#3102]

 

Hi Thuan,

 

I wonder the patch would work in the same reproduced steps if the both 

adests have subscribed each other more than 2 services. The svc_cnt will 

be greater than 1 until it is the last service down event. I think 

that's why mds has the database @subtn_results, in which each item is an 

adest associated with a service id separately.

[Thuan] We can understand that adest still alive, then go with origin flow 
(wait 1.5s).

But can a process send SNDRSP then mds unregister? I think it cannot, because 
it’s in SNDRSP (blocking)

The scenario of this ticket happens for the process terminated/crash.

 

The problem originally resides at the services code e.g ntf, imm... 

where the threads structure between mds receiving thread and main thread 

cause a race condition. Thus the service sends a message with a death 

adest which is removed from mds database, that confuses mds and hit 1.5 

secs wait time.

 

If I read the code correctly, the 1.5 wait time is for another case, it 

gives another chance to wait 1.5 when the subscription result is empty 

in @subtn_results because the service up has not arrived yet.

[Thuan] Yes, it will give a chance if adest not yet UP any.

My patch still keep that chance as origin code.

But I think I need reduce timeout for adest down timer, I am verifying this 
change.

 

mds  subscribe >

 

mds  sends message A x

 

mds wait 1.5 sec

 

mds <--- service up 

 

mds  sends message A >

 

So the 1.5 sec time is for early phase of waiting service up.

 

} else if (sub_info->tmr_flag != true) {

if ((MDS_SENDTYPE_RSP == req->i_sendtype) ||

(MDS_SENDTYPE_RRSP == req->i_sendtype)) {

time_wait = true;

m_MDS_LOG_INFO(

"MDS_SND_RCV:Disc queue: Subscr exists no timer 

running: Waiting for some time\n");

 

-> I think at this line, it means: the SUBSCRIPTION_TMR has timeout, and 

mds is sending RSP/RRSP which means mds should have received the 

*request* message (?), so mds wants to wait for another 1.5 second for 

service_up to create the subscription result in database.

 

The problem in this ticket hit 1.5 because the service down has already 

come and mds removed the subscription result item, now the ntf, imm... 

sends msg with a death adest, and mds now it thinks it is waiting for a 

service up to come as at the early phase, so it waits. Both two 

scenarios can be distinguished themselves to avoid to wait 1.5 secs for 

the latter case I think.

 

Thanks

 

Minh

 

On 22/10/19 9:50 pm, Tran Thuan wrote:

> Hi Vu,

> 

> Thanks for additional comments.

> I reply your concerns inline below.

> 

> Best Regards,

> ThuanTr

> 

> -Original Message-

> From: Nguyen Minh Vu <  
> vu.m.ngu...@dektech.com.au>

> Sent: Tuesday, October 22, 2019 5:28 PM

> To: thuan.tran <  
> thuan.t...@dektech.com.au>; 'Minh Chau' <  
> minh.c...@dektech.com.au>;   
> hans.nordeb...@ericsson.com;   
> gary@dektech.com.au

> Cc:   
> opensaf-devel@lists.sourceforge.net

> Subject: Re: [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to 
> send response message [#3102]

> 

> Hi Thuan,

> 

> I have additional comments below.

> 

> Regards, Vu

> 

> On 10/22/19 7:14 AM, thuan.tran wrote:

>> - When sending response message which Adest not exist (already down)

>> current MDS try to wait for 1.5 seconds before conclude no route to

>> send response message.

>> 

>> - There are 2 scenarios may have:

>> UP -> DOWN -> receive SNDRSP -> response timeout after 1.5s

>> UP -> receive SNDRSP -> DOWN -> response timeout after 1.5s

>> 

>> - With this change, MDS will not waste for 1.5s which can cause trouble

>> for higher layer services, e.g: ntf, imm, etc...

>> ---

>>src/mds/mds_c_api.c | 70 -

>>src/mds/mds_c_sndrcv.c  | 52 --

>>src/mds/mds_core.h  | 25 +--

>>src/mds/mds_dt2c.h  |  2 +-

>>src/mds/mds_dt_common.c | 22 -

>>5 files changed, 162 insertions(+), 9 deletions(-)

>> 

>> diff --git a/src/mds/mds_c_api.c b/src/mds/mds_c_api.c

>> index 132555b8e..5dd30c536 100644

>> --- a/src/mds/mds_c_api.c

>>

Re: [devel] [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to send response message [#3102]

2019-10-22 Thread Minh Hon Chau 

Hi Thuan,

I wonder the patch would work in the same reproduced steps if the both 
adests have subscribed each other more than 2 services. The svc_cnt will 
be greater than 1 until it is the last service down event. I think 
that's why mds has the database @subtn_results, in which each item is an 
adest associated with a service id separately.


The problem originally resides at the services code e.g ntf, imm... 
where the threads structure between mds receiving thread and main thread 
cause a race condition. Thus the service sends a message with a death 
adest which is removed from mds database, that confuses mds and hit 1.5 
secs wait time.


If I read the code correctly, the 1.5 wait time is for another case, it 
gives another chance to wait 1.5 when the subscription result is empty 
in @subtn_results because the service up has not arrived yet.


mds  subscribe >

mds  sends message A x

mds wait 1.5 sec

mds <--- service up 

mds  sends message A >

So the 1.5 sec time is for early phase of waiting service up.

    } else if (sub_info->tmr_flag != true) {
        if ((MDS_SENDTYPE_RSP == req->i_sendtype) ||
            (MDS_SENDTYPE_RRSP == req->i_sendtype)) {
            time_wait = true;
            m_MDS_LOG_INFO(
                "MDS_SND_RCV:Disc queue: Subscr exists no timer 
running: Waiting for some time\n");


-> I think at this line, it means: the SUBSCRIPTION_TMR has timeout, and 
mds is sending RSP/RRSP which means mds should have received the 
*request* message (?), so mds wants to wait for another 1.5 second for 
service_up to create the subscription result in database.


The problem in this ticket hit 1.5 because the service down has already 
come and mds removed the subscription result item, now the ntf, imm... 
sends msg with a death adest, and mds now it thinks it is waiting for a 
service up to come as at the early phase, so it waits. Both two 
scenarios can be distinguished themselves to avoid to wait 1.5 secs for 
the latter case I think.


Thanks

Minh

On 22/10/19 9:50 pm, Tran Thuan wrote:

Hi Vu,

Thanks for additional comments.
I reply your concerns inline below.

Best Regards,
ThuanTr

-Original Message-
From: Nguyen Minh Vu 
Sent: Tuesday, October 22, 2019 5:28 PM
To: thuan.tran ; 'Minh Chau' 
; hans.nordeb...@ericsson.com; gary@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to 
send response message [#3102]

Hi Thuan,

I have additional comments below.

Regards, Vu

On 10/22/19 7:14 AM, thuan.tran wrote:

- When sending response message which Adest not exist (already down)
current MDS try to wait for 1.5 seconds before conclude no route to
send response message.

- There are 2 scenarios may have:
UP -> DOWN -> receive SNDRSP -> response timeout after 1.5s
UP -> receive SNDRSP -> DOWN -> response timeout after 1.5s

- With this change, MDS will not waste for 1.5s which can cause trouble
for higher layer services, e.g: ntf, imm, etc...
---
   src/mds/mds_c_api.c | 70 -
   src/mds/mds_c_sndrcv.c  | 52 --
   src/mds/mds_core.h  | 25 +--
   src/mds/mds_dt2c.h  |  2 +-
   src/mds/mds_dt_common.c | 22 -
   5 files changed, 162 insertions(+), 9 deletions(-)

diff --git a/src/mds/mds_c_api.c b/src/mds/mds_c_api.c
index 132555b8e..5dd30c536 100644
--- a/src/mds/mds_c_api.c
+++ b/src/mds/mds_c_api.c
@@ -1900,6 +1900,32 @@ uint32_t mds_mcm_svc_up(PW_ENV_ID pwe_id, MDS_SVC_ID 
svc_id, V_DEST_RL role,
   
   	/*** Validation for SCOPE **/
   
+	if (adest != m_MDS_GET_ADEST) {

+   MDS_ADEST_INFO *adest_info =
+   (MDS_ADEST_INFO *)ncs_patricia_tree_get(
+   _mds_mcm_cb->adest_list,
+   (uint8_t *));
+   if (!adest_info) {
+   /* Add adest to adest list */
+   adest_info = m_MMGR_ALLOC_ADEST_INFO;
+   memset(adest_info, 0, sizeof(MDS_ADEST_INFO));
+   adest_info->adest = adest;
+   adest_info->node.key_info =
+   (uint8_t *)_info->adest;
+   adest_info->svc_cnt = 1;
+   adest_info->tmr_start = false;
+   ncs_patricia_tree_add(
+   _mds_mcm_cb->adest_list,
+   (NCS_PATRICIA_NODE *)adest_info);
+   m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
+   " svc_cnt=%u", adest, adest_info->svc_cnt);
+   } else {
+   adest_info->svc_cnt++;
+   m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
+   " svc_cnt=%u", adest, adest_info->svc_cnt);
+   }
+   }
+
status =

Re: [devel] [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to send response message [#3102]

2019-10-22 Thread Tran Thuan 
Hi Vu,

Thanks for additional comments.
I reply your concerns inline below.

Best Regards,
ThuanTr

-Original Message-
From: Nguyen Minh Vu  
Sent: Tuesday, October 22, 2019 5:28 PM
To: thuan.tran ; 'Minh Chau' 
; hans.nordeb...@ericsson.com; gary@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to 
send response message [#3102]

Hi Thuan,

I have additional comments below.

Regards, Vu

On 10/22/19 7:14 AM, thuan.tran wrote:
> - When sending response message which Adest not exist (already down)
> current MDS try to wait for 1.5 seconds before conclude no route to
> send response message.
>
> - There are 2 scenarios may have:
> UP -> DOWN -> receive SNDRSP -> response timeout after 1.5s
> UP -> receive SNDRSP -> DOWN -> response timeout after 1.5s
>
> - With this change, MDS will not waste for 1.5s which can cause trouble
> for higher layer services, e.g: ntf, imm, etc...
> ---
>   src/mds/mds_c_api.c | 70 -
>   src/mds/mds_c_sndrcv.c  | 52 --
>   src/mds/mds_core.h  | 25 +--
>   src/mds/mds_dt2c.h  |  2 +-
>   src/mds/mds_dt_common.c | 22 -
>   5 files changed, 162 insertions(+), 9 deletions(-)
>
> diff --git a/src/mds/mds_c_api.c b/src/mds/mds_c_api.c
> index 132555b8e..5dd30c536 100644
> --- a/src/mds/mds_c_api.c
> +++ b/src/mds/mds_c_api.c
> @@ -1900,6 +1900,32 @@ uint32_t mds_mcm_svc_up(PW_ENV_ID pwe_id, MDS_SVC_ID 
> svc_id, V_DEST_RL role,
>   
>   /*** Validation for SCOPE **/
>   
> + if (adest != m_MDS_GET_ADEST) {
> + MDS_ADEST_INFO *adest_info =
> + (MDS_ADEST_INFO *)ncs_patricia_tree_get(
> + _mds_mcm_cb->adest_list,
> + (uint8_t *));
> + if (!adest_info) {
> + /* Add adest to adest list */
> + adest_info = m_MMGR_ALLOC_ADEST_INFO;
> + memset(adest_info, 0, sizeof(MDS_ADEST_INFO));
> + adest_info->adest = adest;
> + adest_info->node.key_info =
> + (uint8_t *)_info->adest;
> + adest_info->svc_cnt = 1;
> + adest_info->tmr_start = false;
> + ncs_patricia_tree_add(
> + _mds_mcm_cb->adest_list,
> + (NCS_PATRICIA_NODE *)adest_info);
> + m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
> + " svc_cnt=%u", adest, adest_info->svc_cnt);
> + } else {
> + adest_info->svc_cnt++;
> + m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
> + " svc_cnt=%u", adest, adest_info->svc_cnt);
> + }
> + }
> +
>   status = mds_get_subtn_res_tbl_by_adest(local_svc_hdl, svc_id, vdest_id,
>   adest, _subtn_result_info);
>   
> @@ -3571,6 +3597,24 @@ uint32_t mds_mcm_svc_down(PW_ENV_ID pwe_id, MDS_SVC_ID 
> svc_id, V_DEST_RL role,
>   /* Discard : Getting down before getting up */
>   } else { /* Entry exist in subscription result table */
>   
> + /* If adest exist and no sndrsp, start a timer */
> + MDS_ADEST_INFO *adest_info =
> + (MDS_ADEST_INFO *)ncs_patricia_tree_get(
> + _mds_mcm_cb->adest_list,
> + (uint8_t *));
> + if (adest_info) {
> + adest_info->svc_cnt--;
> + if (adest_info->svc_cnt == 0 &&
> + adest_info->sndrsp_cnt == 0) {
> + m_MDS_LOG_INFO("MCM:API: Adest=%" PRIu64
> + " down timer start", adest);
> + if (adest_info->tmr_start == false) {
> + adest_info->tmr_start = true;
> + start_mds_down_tmr(adest, svc_id);
> + }
> + }
> + }
> +
>   if (vdest_id == m_VDEST_ID_FOR_ADEST_ENTRY) {
>   status = mds_subtn_res_tbl_del(
>   local_svc_hdl, svc_id, vdest_id, adest,
> @@ -4956,6 +5000,17 @@ uint32_t mds_mcm_init(void)
>   return NCSCC_RC_FAILURE;
>   }
>   
> + /* ADEST TREE */
> + memset(_tree_params, 0, sizeof(NCS_PATRICIA_PARAMS));
> + pat_tree_params.key_size = sizeof(MDS_DEST);
> + if (NCSCC_RC_SUCCESS !=
> + ncs_patricia_tree_init(_mds_mcm_cb->adest_list,
> +_tree_params)) {
> + m_MDS_LOG_ERR(
> + "MCM:API: patricia_tree_init: adest :failure, L 
> mds_mcm_init");
> + return NCSCC_RC_FAILURE;
> + }
> +

Re: [devel] [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to send response message [#3102]

2019-10-22 Thread Nguyen Minh Vu 

Hi Thuan,

I have additional comments below.

Regards, Vu

On 10/22/19 7:14 AM, thuan.tran wrote:

- When sending response message which Adest not exist (already down)
current MDS try to wait for 1.5 seconds before conclude no route to
send response message.

- There are 2 scenarios may have:
UP -> DOWN -> receive SNDRSP -> response timeout after 1.5s
UP -> receive SNDRSP -> DOWN -> response timeout after 1.5s

- With this change, MDS will not waste for 1.5s which can cause trouble
for higher layer services, e.g: ntf, imm, etc...
---
  src/mds/mds_c_api.c | 70 -
  src/mds/mds_c_sndrcv.c  | 52 --
  src/mds/mds_core.h  | 25 +--
  src/mds/mds_dt2c.h  |  2 +-
  src/mds/mds_dt_common.c | 22 -
  5 files changed, 162 insertions(+), 9 deletions(-)

diff --git a/src/mds/mds_c_api.c b/src/mds/mds_c_api.c
index 132555b8e..5dd30c536 100644
--- a/src/mds/mds_c_api.c
+++ b/src/mds/mds_c_api.c
@@ -1900,6 +1900,32 @@ uint32_t mds_mcm_svc_up(PW_ENV_ID pwe_id, MDS_SVC_ID 
svc_id, V_DEST_RL role,
  
  	/*** Validation for SCOPE **/
  
+	if (adest != m_MDS_GET_ADEST) {

+   MDS_ADEST_INFO *adest_info =
+   (MDS_ADEST_INFO *)ncs_patricia_tree_get(
+   _mds_mcm_cb->adest_list,
+   (uint8_t *));
+   if (!adest_info) {
+   /* Add adest to adest list */
+   adest_info = m_MMGR_ALLOC_ADEST_INFO;
+   memset(adest_info, 0, sizeof(MDS_ADEST_INFO));
+   adest_info->adest = adest;
+   adest_info->node.key_info =
+   (uint8_t *)_info->adest;
+   adest_info->svc_cnt = 1;
+   adest_info->tmr_start = false;
+   ncs_patricia_tree_add(
+   _mds_mcm_cb->adest_list,
+   (NCS_PATRICIA_NODE *)adest_info);
+   m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
+   " svc_cnt=%u", adest, adest_info->svc_cnt);
+   } else {
+   adest_info->svc_cnt++;
+   m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
+   " svc_cnt=%u", adest, adest_info->svc_cnt);
+   }
+   }
+
status = mds_get_subtn_res_tbl_by_adest(local_svc_hdl, svc_id, vdest_id,
adest, _subtn_result_info);
  
@@ -3571,6 +3597,24 @@ uint32_t mds_mcm_svc_down(PW_ENV_ID pwe_id, MDS_SVC_ID svc_id, V_DEST_RL role,

/* Discard : Getting down before getting up */
} else { /* Entry exist in subscription result table */
  
+		/* If adest exist and no sndrsp, start a timer */

+   MDS_ADEST_INFO *adest_info =
+   (MDS_ADEST_INFO *)ncs_patricia_tree_get(
+   _mds_mcm_cb->adest_list,
+   (uint8_t *));
+   if (adest_info) {
+   adest_info->svc_cnt--;
+   if (adest_info->svc_cnt == 0 &&
+   adest_info->sndrsp_cnt == 0) {
+   m_MDS_LOG_INFO("MCM:API: Adest=%" PRIu64
+   " down timer start", adest);
+   if (adest_info->tmr_start == false) {
+   adest_info->tmr_start = true;
+   start_mds_down_tmr(adest, svc_id);
+   }
+   }
+   }
+
if (vdest_id == m_VDEST_ID_FOR_ADEST_ENTRY) {
status = mds_subtn_res_tbl_del(
local_svc_hdl, svc_id, vdest_id, adest,
@@ -4956,6 +5000,17 @@ uint32_t mds_mcm_init(void)
return NCSCC_RC_FAILURE;
}
  
+	/* ADEST TREE */

+   memset(_tree_params, 0, sizeof(NCS_PATRICIA_PARAMS));
+   pat_tree_params.key_size = sizeof(MDS_DEST);
+   if (NCSCC_RC_SUCCESS !=
+   ncs_patricia_tree_init(_mds_mcm_cb->adest_list,
+  _tree_params)) {
+   m_MDS_LOG_ERR(
+   "MCM:API: patricia_tree_init: adest :failure, L 
mds_mcm_init");
+   return NCSCC_RC_FAILURE;
+   }
+
/* SERVICE TREE */
memset(_tree_params, 0, sizeof(NCS_PATRICIA_PARAMS));
pat_tree_params.key_size = sizeof(MDS_SVC_HDL);
@@ -4966,7 +5021,12 @@ uint32_t mds_mcm_init(void)
if (NCSCC_RC_SUCCESS !=
ncs_patricia_tree_destroy(_mds_mcm_cb->vdest_list)) {
m_MDS_LOG_ERR(
-   "MCM:API: patricia_tree_destroy: service :failure, L 
mds_mcm_init");
+   "MCM:API: patricia_tree_destroy: vdest

Re: [devel] [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to send response message [#3102]

2019-10-22 Thread Tran Thuan 
Hi Vu,

Thanks for comments. I reply my answer inline.

Best Regards,
ThuanTr

-Original Message-
From: Nguyen Minh Vu  
Sent: Tuesday, October 22, 2019 4:42 PM
To: thuan.tran ; 'Minh Chau' 
; hans.nordeb...@ericsson.com; gary@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to 
send response message [#3102]

Hi Thuan,

I have few comments below.

Regards, Vu

On 10/22/19 7:14 AM, thuan.tran wrote:
> - When sending response message which Adest not exist (already down)
> current MDS try to wait for 1.5 seconds before conclude no route to
> send response message.
>
> - There are 2 scenarios may have:
> UP -> DOWN -> receive SNDRSP -> response timeout after 1.5s
> UP -> receive SNDRSP -> DOWN -> response timeout after 1.5s
>
> - With this change, MDS will not waste for 1.5s which can cause trouble
> for higher layer services, e.g: ntf, imm, etc...
> ---
>   src/mds/mds_c_api.c | 70 -
>   src/mds/mds_c_sndrcv.c  | 52 --
>   src/mds/mds_core.h  | 25 +--
>   src/mds/mds_dt2c.h  |  2 +-
>   src/mds/mds_dt_common.c | 22 -
>   5 files changed, 162 insertions(+), 9 deletions(-)
>
> diff --git a/src/mds/mds_c_api.c b/src/mds/mds_c_api.c
> index 132555b8e..5dd30c536 100644
> --- a/src/mds/mds_c_api.c
> +++ b/src/mds/mds_c_api.c
> @@ -1900,6 +1900,32 @@ uint32_t mds_mcm_svc_up(PW_ENV_ID pwe_id, MDS_SVC_ID 
> svc_id, V_DEST_RL role,
>   
>   /*** Validation for SCOPE **/
>   
> + if (adest != m_MDS_GET_ADEST) {
> + MDS_ADEST_INFO *adest_info =
> + (MDS_ADEST_INFO *)ncs_patricia_tree_get(
> + _mds_mcm_cb->adest_list,
> + (uint8_t *));
> + if (!adest_info) {
> + /* Add adest to adest list */
> + adest_info = m_MMGR_ALLOC_ADEST_INFO;
> + memset(adest_info, 0, sizeof(MDS_ADEST_INFO));
> + adest_info->adest = adest;
> + adest_info->node.key_info =
> + (uint8_t *)_info->adest;
> + adest_info->svc_cnt = 1;
> + adest_info->tmr_start = false;
> + ncs_patricia_tree_add(
> + _mds_mcm_cb->adest_list,
> + (NCS_PATRICIA_NODE *)adest_info);
> + m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
> + " svc_cnt=%u", adest, adest_info->svc_cnt);
> + } else {
> + adest_info->svc_cnt++;
> + m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
> + " svc_cnt=%u", adest, adest_info->svc_cnt);
> + }
> + }
[Vu] This new database, adest_list, is shared b/w internal osaf_mds 
thread and mds's user thread, hence should be protected by mutex.
[Thuan] It's protected by gl_mds_library_mutex
> +
>   status = mds_get_subtn_res_tbl_by_adest(local_svc_hdl, svc_id, vdest_id,
>   adest, _subtn_result_info);
>   
> @@ -3571,6 +3597,24 @@ uint32_t mds_mcm_svc_down(PW_ENV_ID pwe_id, MDS_SVC_ID 
> svc_id, V_DEST_RL role,
>   /* Discard : Getting down before getting up */
>   } else { /* Entry exist in subscription result table */
>   
> + /* If adest exist and no sndrsp, start a timer */
> + MDS_ADEST_INFO *adest_info =
> + (MDS_ADEST_INFO *)ncs_patricia_tree_get(
> + _mds_mcm_cb->adest_list,
> + (uint8_t *));
> + if (adest_info) {
> + adest_info->svc_cnt--;
> + if (adest_info->svc_cnt == 0 &&
> + adest_info->sndrsp_cnt == 0) {
> + m_MDS_LOG_INFO("MCM:API: Adest=%" PRIu64
> + " down timer start", adest);
> + if (adest_info->tmr_start == false) {
> + adest_info->tmr_start = true;
> + start_mds_down_tmr(adest, svc_id);
[Vu] Seems mds_down tmr is started twice. The first start is at the 
beginning of the function.
[Thuan] That timer is not same with this timer, that timer for current process. 
This timer for remote adest

[Vu] But what is the reason to start the mds down timer here? What if we 
won't start the tmr?
[Thuan] Timer for handle UP -> DOWN -> receive SNDRSP -> send RSP

 /* potentially clean up the process info database */
 MDS_PROCESS_INFO *info = mds_process_info_get(adest, svc_id);
 if (info != NULL) {
 /* Process info exist, delay the cleanup with a timeout to avoid
  * race */
 start_mds_down_tmr(adest, svc_id);
 }
> +

Re: [devel] [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to send response message [#3102]

2019-10-22 Thread Nguyen Minh Vu 

Hi Thuan,

I have few comments below.

Regards, Vu

On 10/22/19 7:14 AM, thuan.tran wrote:

- When sending response message which Adest not exist (already down)
current MDS try to wait for 1.5 seconds before conclude no route to
send response message.

- There are 2 scenarios may have:
UP -> DOWN -> receive SNDRSP -> response timeout after 1.5s
UP -> receive SNDRSP -> DOWN -> response timeout after 1.5s

- With this change, MDS will not waste for 1.5s which can cause trouble
for higher layer services, e.g: ntf, imm, etc...
---
  src/mds/mds_c_api.c | 70 -
  src/mds/mds_c_sndrcv.c  | 52 --
  src/mds/mds_core.h  | 25 +--
  src/mds/mds_dt2c.h  |  2 +-
  src/mds/mds_dt_common.c | 22 -
  5 files changed, 162 insertions(+), 9 deletions(-)

diff --git a/src/mds/mds_c_api.c b/src/mds/mds_c_api.c
index 132555b8e..5dd30c536 100644
--- a/src/mds/mds_c_api.c
+++ b/src/mds/mds_c_api.c
@@ -1900,6 +1900,32 @@ uint32_t mds_mcm_svc_up(PW_ENV_ID pwe_id, MDS_SVC_ID 
svc_id, V_DEST_RL role,
  
  	/*** Validation for SCOPE **/
  
+	if (adest != m_MDS_GET_ADEST) {

+   MDS_ADEST_INFO *adest_info =
+   (MDS_ADEST_INFO *)ncs_patricia_tree_get(
+   _mds_mcm_cb->adest_list,
+   (uint8_t *));
+   if (!adest_info) {
+   /* Add adest to adest list */
+   adest_info = m_MMGR_ALLOC_ADEST_INFO;
+   memset(adest_info, 0, sizeof(MDS_ADEST_INFO));
+   adest_info->adest = adest;
+   adest_info->node.key_info =
+   (uint8_t *)_info->adest;
+   adest_info->svc_cnt = 1;
+   adest_info->tmr_start = false;
+   ncs_patricia_tree_add(
+   _mds_mcm_cb->adest_list,
+   (NCS_PATRICIA_NODE *)adest_info);
+   m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
+   " svc_cnt=%u", adest, adest_info->svc_cnt);
+   } else {
+   adest_info->svc_cnt++;
+   m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
+   " svc_cnt=%u", adest, adest_info->svc_cnt);
+   }
+   }
[Vu] This new database, adest_list, is shared b/w internal osaf_mds 
thread and mds's user thread, hence should be protected by mutex.

+
status = mds_get_subtn_res_tbl_by_adest(local_svc_hdl, svc_id, vdest_id,
adest, _subtn_result_info);
  
@@ -3571,6 +3597,24 @@ uint32_t mds_mcm_svc_down(PW_ENV_ID pwe_id, MDS_SVC_ID svc_id, V_DEST_RL role,

/* Discard : Getting down before getting up */
} else { /* Entry exist in subscription result table */
  
+		/* If adest exist and no sndrsp, start a timer */

+   MDS_ADEST_INFO *adest_info =
+   (MDS_ADEST_INFO *)ncs_patricia_tree_get(
+   _mds_mcm_cb->adest_list,
+   (uint8_t *));
+   if (adest_info) {
+   adest_info->svc_cnt--;
+   if (adest_info->svc_cnt == 0 &&
+   adest_info->sndrsp_cnt == 0) {
+   m_MDS_LOG_INFO("MCM:API: Adest=%" PRIu64
+   " down timer start", adest);
+   if (adest_info->tmr_start == false) {
+   adest_info->tmr_start = true;
+   start_mds_down_tmr(adest, svc_id);
[Vu] Seems mds_down tmr is started twice. The first start is at the 
beginning of the function.
But what is the reason to start the mds down timer here? What if we 
won't start the tmr?


    /* potentially clean up the process info database */
    MDS_PROCESS_INFO *info = mds_process_info_get(adest, svc_id);
    if (info != NULL) {
        /* Process info exist, delay the cleanup with a timeout to avoid
         * race */
        start_mds_down_tmr(adest, svc_id);
    }

+   }
+   }
+   }
+
if (vdest_id == m_VDEST_ID_FOR_ADEST_ENTRY) {
status = mds_subtn_res_tbl_del(
local_svc_hdl, svc_id, vdest_id, adest,
@@ -4956,6 +5000,17 @@ uint32_t mds_mcm_init(void)
return NCSCC_RC_FAILURE;
}
  
+	/* ADEST TREE */

+   memset(_tree_params, 0, sizeof(NCS_PATRICIA_PARAMS));
+   pat_tree_params.key_size = sizeof(MDS_DEST);
+   if (NCSCC_RC_SUCCESS !=
+   ncs_patricia_tree_init(_mds_mcm_cb->adest_list,
+  _tree_params)) {
+   m_MDS_LOG_ERR(
+   "MCM:API: patricia_tree_init:

Re: [devel] [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to send response message [#3102]

2019-10-22 Thread Tran Thuan 
Hi Minh,

Thanks for your comments:

1-  The wait time 1.5 in this flow:
mds_mcm_process_disc_queue_checks_redundant()
mds_subtn_tbl_add_disc_queue()
if (true == time_wait) {
timeout_val = 150; /* This may need a tuning */
}

2- We should not touch to current adest db because it is used
everywhere with current logic, if we used it, we have to change logic
of many code places, which become more complex and risky.

Best Regards,
ThuanTr

-Original Message-
From: Minh Hon Chau  
Sent: Tuesday, October 22, 2019 11:31 AM
To: thuan.tran ; hans.nordeb...@ericsson.com; 
gary@dektech.com.au; vu.m.ngu...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to 
send response message [#3102]

Hi Thuan,

1- Can you point out where is the mds code that waits for 1.5 seconds, 
is it hard coded within 1.5 secs?

2- Is existing db (mds_c_db.c) in mds not enough so we need to introduce 
adest_list? I think mds must have a memory of adest, perhaps in another 
implicit form, so mds can validate an adest, a svc_id associated with adest.

thanks

Minh

On 22/10/19 11:14 am, thuan.tran wrote:
> - When sending response message which Adest not exist (already down)
> current MDS try to wait for 1.5 seconds before conclude no route to
> send response message.
>
> - There are 2 scenarios may have:
> UP -> DOWN -> receive SNDRSP -> response timeout after 1.5s
> UP -> receive SNDRSP -> DOWN -> response timeout after 1.5s
>
> - With this change, MDS will not waste for 1.5s which can cause trouble
> for higher layer services, e.g: ntf, imm, etc...
> ---
>   src/mds/mds_c_api.c | 70 -
>   src/mds/mds_c_sndrcv.c  | 52 --
>   src/mds/mds_core.h  | 25 +--
>   src/mds/mds_dt2c.h  |  2 +-
>   src/mds/mds_dt_common.c | 22 -
>   5 files changed, 162 insertions(+), 9 deletions(-)
>
> diff --git a/src/mds/mds_c_api.c b/src/mds/mds_c_api.c
> index 132555b8e..5dd30c536 100644
> --- a/src/mds/mds_c_api.c
> +++ b/src/mds/mds_c_api.c
> @@ -1900,6 +1900,32 @@ uint32_t mds_mcm_svc_up(PW_ENV_ID pwe_id, MDS_SVC_ID 
> svc_id, V_DEST_RL role,
>   
>   /*** Validation for SCOPE **/
>   
> + if (adest != m_MDS_GET_ADEST) {
> + MDS_ADEST_INFO *adest_info =
> + (MDS_ADEST_INFO *)ncs_patricia_tree_get(
> + _mds_mcm_cb->adest_list,
> + (uint8_t *));
> + if (!adest_info) {
> + /* Add adest to adest list */
> + adest_info = m_MMGR_ALLOC_ADEST_INFO;
> + memset(adest_info, 0, sizeof(MDS_ADEST_INFO));
> + adest_info->adest = adest;
> + adest_info->node.key_info =
> + (uint8_t *)_info->adest;
> + adest_info->svc_cnt = 1;
> + adest_info->tmr_start = false;
> + ncs_patricia_tree_add(
> + _mds_mcm_cb->adest_list,
> + (NCS_PATRICIA_NODE *)adest_info);
> + m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
> + " svc_cnt=%u", adest, adest_info->svc_cnt);
> + } else {
> + adest_info->svc_cnt++;
> + m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
> + " svc_cnt=%u", adest, adest_info->svc_cnt);
> + }
> + }
> +
>   status = mds_get_subtn_res_tbl_by_adest(local_svc_hdl, svc_id, vdest_id,
>   adest, _subtn_result_info);
>   
> @@ -3571,6 +3597,24 @@ uint32_t mds_mcm_svc_down(PW_ENV_ID pwe_id, MDS_SVC_ID 
> svc_id, V_DEST_RL role,
>   /* Discard : Getting down before getting up */
>   } else { /* Entry exist in subscription result table */
>   
> + /* If adest exist and no sndrsp, start a timer */
> + MDS_ADEST_INFO *adest_info =
> + (MDS_ADEST_INFO *)ncs_patricia_tree_get(
> + _mds_mcm_cb->adest_list,
> + (uint8_t *));
> + if (adest_info) {
> + adest_info->svc_cnt--;
> + if (adest_info->svc_cnt == 0 &&
> + adest_info->sndrsp_cnt == 0) {
> + m_MDS_LOG_INFO("MCM:API: Adest=%" PRIu64
> + " down timer start", adest);
> + if (adest_info->tmr_start == false) {
> + adest_info->tmr_start = true;
> + start_mds_down_tmr(adest, svc_id);
> + }
> + }
> + }
> +
>   if (vdest_id == m_VDEST_ID_FOR_ADEST_ENTRY) {
>

Re: [devel] [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to send response message [#3102]

2019-10-21 Thread Minh Hon Chau 

Hi Thuan,

1- Can you point out where is the mds code that waits for 1.5 seconds, 
is it hard coded within 1.5 secs?


2- Is existing db (mds_c_db.c) in mds not enough so we need to introduce 
adest_list? I think mds must have a memory of adest, perhaps in another 
implicit form, so mds can validate an adest, a svc_id associated with adest.


thanks

Minh

On 22/10/19 11:14 am, thuan.tran wrote:

- When sending response message which Adest not exist (already down)
current MDS try to wait for 1.5 seconds before conclude no route to
send response message.

- There are 2 scenarios may have:
UP -> DOWN -> receive SNDRSP -> response timeout after 1.5s
UP -> receive SNDRSP -> DOWN -> response timeout after 1.5s

- With this change, MDS will not waste for 1.5s which can cause trouble
for higher layer services, e.g: ntf, imm, etc...
---
  src/mds/mds_c_api.c | 70 -
  src/mds/mds_c_sndrcv.c  | 52 --
  src/mds/mds_core.h  | 25 +--
  src/mds/mds_dt2c.h  |  2 +-
  src/mds/mds_dt_common.c | 22 -
  5 files changed, 162 insertions(+), 9 deletions(-)

diff --git a/src/mds/mds_c_api.c b/src/mds/mds_c_api.c
index 132555b8e..5dd30c536 100644
--- a/src/mds/mds_c_api.c
+++ b/src/mds/mds_c_api.c
@@ -1900,6 +1900,32 @@ uint32_t mds_mcm_svc_up(PW_ENV_ID pwe_id, MDS_SVC_ID 
svc_id, V_DEST_RL role,
  
  	/*** Validation for SCOPE **/
  
+	if (adest != m_MDS_GET_ADEST) {

+   MDS_ADEST_INFO *adest_info =
+   (MDS_ADEST_INFO *)ncs_patricia_tree_get(
+   _mds_mcm_cb->adest_list,
+   (uint8_t *));
+   if (!adest_info) {
+   /* Add adest to adest list */
+   adest_info = m_MMGR_ALLOC_ADEST_INFO;
+   memset(adest_info, 0, sizeof(MDS_ADEST_INFO));
+   adest_info->adest = adest;
+   adest_info->node.key_info =
+   (uint8_t *)_info->adest;
+   adest_info->svc_cnt = 1;
+   adest_info->tmr_start = false;
+   ncs_patricia_tree_add(
+   _mds_mcm_cb->adest_list,
+   (NCS_PATRICIA_NODE *)adest_info);
+   m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
+   " svc_cnt=%u", adest, adest_info->svc_cnt);
+   } else {
+   adest_info->svc_cnt++;
+   m_MDS_LOG_DBG("MCM:API: Adest=%" PRIu64
+   " svc_cnt=%u", adest, adest_info->svc_cnt);
+   }
+   }
+
status = mds_get_subtn_res_tbl_by_adest(local_svc_hdl, svc_id, vdest_id,
adest, _subtn_result_info);
  
@@ -3571,6 +3597,24 @@ uint32_t mds_mcm_svc_down(PW_ENV_ID pwe_id, MDS_SVC_ID svc_id, V_DEST_RL role,

/* Discard : Getting down before getting up */
} else { /* Entry exist in subscription result table */
  
+		/* If adest exist and no sndrsp, start a timer */

+   MDS_ADEST_INFO *adest_info =
+   (MDS_ADEST_INFO *)ncs_patricia_tree_get(
+   _mds_mcm_cb->adest_list,
+   (uint8_t *));
+   if (adest_info) {
+   adest_info->svc_cnt--;
+   if (adest_info->svc_cnt == 0 &&
+   adest_info->sndrsp_cnt == 0) {
+   m_MDS_LOG_INFO("MCM:API: Adest=%" PRIu64
+   " down timer start", adest);
+   if (adest_info->tmr_start == false) {
+   adest_info->tmr_start = true;
+   start_mds_down_tmr(adest, svc_id);
+   }
+   }
+   }
+
if (vdest_id == m_VDEST_ID_FOR_ADEST_ENTRY) {
status = mds_subtn_res_tbl_del(
local_svc_hdl, svc_id, vdest_id, adest,
@@ -4956,6 +5000,17 @@ uint32_t mds_mcm_init(void)
return NCSCC_RC_FAILURE;
}
  
+	/* ADEST TREE */

+   memset(_tree_params, 0, sizeof(NCS_PATRICIA_PARAMS));
+   pat_tree_params.key_size = sizeof(MDS_DEST);
+   if (NCSCC_RC_SUCCESS !=
+   ncs_patricia_tree_init(_mds_mcm_cb->adest_list,
+  _tree_params)) {
+   m_MDS_LOG_ERR(
+   "MCM:API: patricia_tree_init: adest :failure, L 
mds_mcm_init");
+   return NCSCC_RC_FAILURE;
+   }
+
/* SERVICE TREE */
memset(_tree_params, 0, sizeof(NCS_PATRICIA_PARAMS));
pat_tree_params.key_size = sizeof(MDS_SVC_HDL);
@@ -4966,7 +5021,12 @@ uint32_t mds_mcm_init(void)
if

Re: [devel] [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to send response message [#3102]

2019-10-17 Thread Tran Thuan 
Hi,

There is a issue with this change, I need consider more.
Please ignore this review for now.

Best Regards,
ThuanTr

-Original Message-
From: thuan.tran  
Sent: Thursday, October 17, 2019 4:56 PM
To: 'Minh Chau' ; hans.nordeb...@ericsson.com;
gary@dektech.com.au; vu.m.ngu...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; thuan.tran

Subject: [PATCH 1/1] mds: not waste 1.5s in waiting Adest already down to
send response message [#3102]

- When sending response message which Adest not exist (already down) current
MDS try to waiting in 1.5 seconds before conclude no route to send response
message.
- With this change, MDS will not waste for 1.5s which can cause trouble for
higher layer services, e.g: ntf, imm, etc...
---
 src/mds/mds_c_sndrcv.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/mds/mds_c_sndrcv.c b/src/mds/mds_c_sndrcv.c index
7850ac714..f0c60a8b7 100644
--- a/src/mds/mds_c_sndrcv.c
+++ b/src/mds/mds_c_sndrcv.c
@@ -2713,17 +2713,17 @@ static uint32_t
mds_mcm_process_disc_queue_checks_redundant(
"MDS_SND_RCV: Subscription made but no pointer
available\n");
return NCSCC_RC_FAILURE;
}
-   } else if (sub_info->tmr_flag != true) {
-   if ((MDS_SENDTYPE_RSP == req->i_sendtype) ||
-   (MDS_SENDTYPE_RRSP == req->i_sendtype)) {
+   } else if (sub_info->tmr_flag == true) {
+   if (((MDS_SENDTYPE_RSP == req->i_sendtype) ||
+   (MDS_SENDTYPE_RRSP == req->i_sendtype))) {
time_wait = true;
m_MDS_LOG_INFO(
-   "MDS_SND_RCV:Disc queue red: Subscr exists no
timer running: Waiting for some time\n");
-   } else {
-   m_MDS_LOG_INFO(
-   "MDS_SND_RCV: Subscription exists but Timer has
expired\n");
-   return NCSCC_RC_FAILURE;
+   "MDS_SND_RCV:Disc queue red: Subscr exists timer
running: 
+Waiting for some time\n");
}
+   } else {
+   m_MDS_LOG_INFO(
+   "MDS_SND_RCV: Subscription exists but Timer has
expired\n");
+   return NCSCC_RC_FAILURE;
}
 
/* Add this message to the DISC Queue, One function call */
--
2.17.1




___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel