[devel] [PATCH 0/1] Review Request for smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]

2019-10-21 Thread phuc.h.chau
Summary: smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]
Review request for Ticket(s): 3104
Peer Reviewer(s): Thang, Thuan
Pull request to: Thang
Affected branch(es): develop
Development branch: ticket-3104
Base revision: a6781e424c8963d03009e555bffbb323a4e290c4
Personal repository: git://git.code.sf.net/u/zchxphc/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 08707e9f71f4cdf48e33c3ba1413d517cebf2488
Author: phuc.h.chau 
Date:   Mon, 21 Oct 2019 17:12:19 +0700

smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]

SW upgrade testing, if found that if a service unit is in INSTANTIATION_FAILED,
one_step upgrade will not continue with the software installation.



Complete diffstat:
--
 src/smf/smfd/SmfAdminState.cc | 2 ++
 1 file changed, 2 insertions(+)


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


[devel] [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]

2019-10-21 Thread phuc.h.chau
SW upgrade testing, if found that if a service unit is in INSTANTIATION_FAILED,
one_step upgrade will not continue with the software installation.
---
 src/smf/smfd/SmfAdminState.cc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/smf/smfd/SmfAdminState.cc b/src/smf/smfd/SmfAdminState.cc
index 076f9f0..a54f47f 100644
--- a/src/smf/smfd/SmfAdminState.cc
+++ b/src/smf/smfd/SmfAdminState.cc
@@ -900,11 +900,13 @@ bool SmfAdminStateHandler::nodeGroupAdminOperation(
 LOG_NO(
 "%s: saImmOmAdminOperationInvoke_2 Fail %s",
 __FUNCTION__, saf_error(imm_rc));
+method_rc = false;
 errno_ = imm_rc;
 break;
   } else if (oi_rc != SA_AIS_OK) {
 LOG_NO("%s: SaAmfAdminOperationId %d Fail %s", __FUNCTION__, adminOp,
saf_error(oi_rc));
+method_rc = false;
 errno_ = oi_rc;
 break;
   } else {
-- 
2.7.4



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


Re: [devel] [PATCH 2/2] msg: fully support SC absence [#3083]

2019-10-21 Thread Mathi N P
It becomes trickier to handle timeout scenarios within the agent library
threads.
The application designer may not like asserts from OpenSAF library.
Returning an error code (SA_AIS_ERR_NO_RESOURCES) could be an option, but
the error code doesn't accurately reflects the situation if the timeout
occurred because of a transient situation even though the user an decide to
reInitialize the library at a later point of time.
As you mentioned, we should perhaps just revisit this topic if such a
timeout situation arises(at all)...

Ack,
Mathi.

On Tue, Oct 15, 2019 at 8:59 PM Jones, Alex  wrote:

> Hi Mathi,
>
> See my comments inline.
>
> Alex
>
> On 10/15/19 12:45 PM, Mathi N P wrote:
> 
> NOTICE: This email was received from an EXTERNAL sender
> 
>
>
> Hi Alex,
>
> I only have a couple of things to discuss, the rest of the changes are
> fine.
>
> 1) Performing mqa_mds_reregister_queues() in a separate thread(inside the
> application process), iam not yet sure about
> synchronization issues this might throw with the MDS thread
> [Alex] I tried this originally, but this is currently on the mds thread.
> So, because we are using a sync call we can never get the return because we
> are calling this from the mds thread. That's why I created a separate
> thread. I did create a sync with msgnd, so that API calls can only continue
> when we know that msgnd has synced all of its queues with msgd.
> 2) In the below call, there is likelihood of the MDS send timing out
> (extreme case?). Should we handle the timeout error code from the MDS send?
> + /* send the request to the MQND */
> + uint32_t rc(mqa_mds_msg_sync_send(mqa_cb->mqa_mds_hdl,
> + &mqa_cb->mqnd_mds_dest,
> + &cap_evt, &out_evt, MQSV_WAIT_TIME));
> +
> + mqa_cb = m_MQSV_MQA_RETRIEVE_MQA_CB;
> +
> + if (rc != NCSCC_RC_SUCCESS)
> [Alex] How should we handle a timeout? Retry a few times? Assert? I put
> the LOG_ER in there, so we can see if it ever happens, and deal with it at
> that point. I haven't seen this in my testing.
>
> Thanks,
> Mathi.
>
>
> On Thu, Oct 10, 2019 at 6:19 PM Jones, Alex  ajo...@rbbn.com>> wrote:
> Fix whitespace issues
> ---
> src/msg/apitest/test_CapacityThresholds.cc | 2 +-
> src/msg/msgd/mqd_saf.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/msg/apitest/test_CapacityThresholds.cc
> b/src/msg/apitest/test_CapacityThresholds.cc
> index eb5dda1dc..971c8ae90 100644
> --- a/src/msg/apitest/test_CapacityThresholds.cc
> +++ b/src/msg/apitest/test_CapacityThresholds.cc
> @@ -50,7 +50,7 @@ static SaAisErrorT msgInitialize(SaMsgHandleT *msgHandle,
> SaVersionT *version)
> {
> SaAisErrorT rc(SA_AIS_OK);
> -
> +
> while (true) {
> rc = saMsgInitialize(msgHandle, 0, version);
>
> diff --git a/src/msg/msgd/mqd_saf.c b/src/msg/msgd/mqd_saf.c
> index 846755ba7..895a4852d 100644
> --- a/src/msg/msgd/mqd_saf.c
> +++ b/src/msg/msgd/mqd_saf.c
> @@ -64,7 +64,7 @@ static void get_q_groups_from_imm(MQD_CB *pMqd)
> SaVersionT version = { 'A', 2, 15 };
>
> error = immutil_saImmOmInitialize(&immHandle, 0, &version);
> -
> +
> if (error != SA_AIS_OK) {
> LOG_ER("saImmOmInitialize failed %u", error);
> break;
> --
> 2.20.1
>
>
> 
> Notice: This e-mail together with any attachments may contain information
> of Ribbon Communications Inc. that is confidential and/or proprietary for
> the sole use of the intended recipient. Any review, disclosure, reliance or
> distribution by others or forwarding without express permission is strictly
> prohibited. If you are not the intended recipient, please notify the sender
> immediately and then delete all copies, including any attachments.
> 
>

___
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: not waste 1.5s in waiting Adest already down to send response message [#3102] V2

2019-10-21 Thread thuan.tran
Summary: mds: not waste 1.5s in waiting Adest already down to send response 
message [#3102]
Review request for Ticket(s): 3102
Peer Reviewer(s): Minh, Vu, Gary, Hans
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-3102
Base revision: e668475b78f3e91db037607755c5c9d92842b445
Personal repository: git://git.code.sf.net/u/thuantr/review


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesn
 OpenSAF servicesn
 Core libraries  y
 Samples n
 Tests   n
 Other   n

NOTE: Patch(es) contain lines longer than 80 characers

Comments (indicate scope for each "y" above):
-
N/A

revision 38f10bb8bb5db407ac2c151df9eb439aab0dc6d8
Author: thuan.tran 
Date:   Tue, 22 Oct 2019 07:05:56 +0700

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 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...



Complete diffstat:
--
 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(-)


Testing Commands:
-
N/A

Testing, Expected Results:
--
N/A

Conditions of Submission:
-
ACK by reviewers

Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  y
powerpc n  n
powerpc64   n  n


Reviewer Checklist:
---
[Submitters: make sure that your review doesn't trigger any checkmarks!]


Your checkin has not passed review because (see checked entries):

___ Your RR template is generally incomplete; it has too many blank entries
that need proper data filled in.

___ You have failed to nominate the proper persons for review and push.

___ Your patches do not have proper short+long header

___ You have grammar/spelling in your header that is unacceptable.

___ You have exceeded a sensible line length in your headers/comments/text.

___ You have failed to put in a proper Trac Ticket # into your commits.

___ You have incorrectly put/left internal data in your comments/files
(i.e. internal bug tracking tool IDs, product names etc)

___ You have not given any evidence of testing beyond basic build tests.
Demonstrate some level of runtime or other sanity testing.

___ You have ^M present in some of your files. These have to be removed.

___ You have needlessly changed whitespace or added whitespace crimes
like trailing spaces, or spaces before tabs.

___ You have mixed real technical changes with whitespace and other
cosmetic code cleanup changes. These have to be separate commits.

___ You need to refactor your submission into logical chunks; there is
too much content into a single commit.

___ You have extraneous garbage in your review (merge commits etc)

___ You have giant attachments which should never have been sent;
Instead you should place your content in a public tree to be pulled.

___ You have too many commits attached to an e-mail; resend as threaded
commits, or place in a public tree for a pull.

___ You have resent this content multiple times without a clear indication
of what has changed between each re-send.

___ You have failed to adequately and individually address all of the
comments and change requests that were proposed in the initial review.

___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you don't present any results
for in-service upgradability test.

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.



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


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

2019-10-21 Thread thuan.tran
- 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(
+   &gl_mds_mcm_cb->adest_list,
+   (uint8_t *)&adest);
+   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 *)&adest_info->adest;
+   adest_info->svc_cnt = 1;
+   adest_info->tmr_start = false;
+   ncs_patricia_tree_add(
+   &gl_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, &log_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(
+   &gl_mds_mcm_cb->adest_list,
+   (uint8_t *)&adest);
+   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(&pat_tree_params, 0, sizeof(NCS_PATRICIA_PARAMS));
+   pat_tree_params.key_size = sizeof(MDS_DEST);
+   if (NCSCC_RC_SUCCESS !=
+   ncs_patricia_tree_init(&gl_mds_mcm_cb->adest_list,
+  &pat_tree_params)) {
+   m_MDS_LOG_ERR(
+   "MCM:API: patricia_tree_init: adest :failure, L 
mds_mcm_init");
+   return NCSCC_RC_FAILURE;
+   }
+
/* SERVICE TREE */
memset(&pat_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(&gl_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 :failure, L 
mds_mcm_init");
+   

[devel] [PATCH 0/1] Review Request for base: add serial number arithmetic (RFC1982) [#3074]

2019-10-21 Thread thuan.tran
Summary: base: add serial number arithmetic (RFC1982) [#3074]
Review request for Ticket(s): 3074
Peer Reviewer(s): Minh, Gary
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-3074
Base revision: 9b1c588a4f244cb34a304b03c7568e04fa6cd8a7
Personal repository: git://git.code.sf.net/u/thuantr/review


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesy
 OpenSAF servicesn
 Core libraries  n
 Samples n
 Tests   n
 Other   n


Comments (indicate scope for each "y" above):
-
N/A

revision a2a6c6c19d758b5652e0729f0ab839a3444fe09e
Author: thuan.tran 
Date:   Tue, 22 Oct 2019 09:29:01 +0700

base: add serial number arithmetic (RFC1982) [#3074]

- Adapt MDS with this SNA implementation.



Added Files:

 src/base/sna.h
 src/base/tests/sna_test.cc


Complete diffstat:
--
 src/base/Makefile.am |   6 +-
 src/base/sna.h   | 126 +++
 src/base/tests/sna_test.cc   | 117 
 src/mds/mds_tipc_fctrl_portid.cc |  45 +++---
 src/mds/mds_tipc_fctrl_portid.h  |  79 +++-
 5 files changed, 278 insertions(+), 95 deletions(-)


Testing Commands:
-
N/A

Testing, Expected Results:
--
N/A

Conditions of Submission:
-
ACK by reviewers

Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  y
powerpc n  n
powerpc64   n  n


Reviewer Checklist:
---
[Submitters: make sure that your review doesn't trigger any checkmarks!]


Your checkin has not passed review because (see checked entries):

___ Your RR template is generally incomplete; it has too many blank entries
that need proper data filled in.

___ You have failed to nominate the proper persons for review and push.

___ Your patches do not have proper short+long header

___ You have grammar/spelling in your header that is unacceptable.

___ You have exceeded a sensible line length in your headers/comments/text.

___ You have failed to put in a proper Trac Ticket # into your commits.

___ You have incorrectly put/left internal data in your comments/files
(i.e. internal bug tracking tool IDs, product names etc)

___ You have not given any evidence of testing beyond basic build tests.
Demonstrate some level of runtime or other sanity testing.

___ You have ^M present in some of your files. These have to be removed.

___ You have needlessly changed whitespace or added whitespace crimes
like trailing spaces, or spaces before tabs.

___ You have mixed real technical changes with whitespace and other
cosmetic code cleanup changes. These have to be separate commits.

___ You need to refactor your submission into logical chunks; there is
too much content into a single commit.

___ You have extraneous garbage in your review (merge commits etc)

___ You have giant attachments which should never have been sent;
Instead you should place your content in a public tree to be pulled.

___ You have too many commits attached to an e-mail; resend as threaded
commits, or place in a public tree for a pull.

___ You have resent this content multiple times without a clear indication
of what has changed between each re-send.

___ You have failed to adequately and individually address all of the
comments and change requests that were proposed in the initial review.

___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you don't present any results
for in-service upgradability test.

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.



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


[devel] [PATCH 1/1] base: add serial number arithmetic (RFC1982) [#3074]

2019-10-21 Thread thuan.tran
- Adapt MDS with this SNA implementation.
---
 src/base/Makefile.am |   6 +-
 src/base/sna.h   | 126 +++
 src/base/tests/sna_test.cc   | 117 
 src/mds/mds_tipc_fctrl_portid.cc |  45 ++-
 src/mds/mds_tipc_fctrl_portid.h  |  79 +++
 5 files changed, 278 insertions(+), 95 deletions(-)
 create mode 100644 src/base/sna.h
 create mode 100644 src/base/tests/sna_test.cc

diff --git a/src/base/Makefile.am b/src/base/Makefile.am
index 025fb86a2..5082175cf 100644
--- a/src/base/Makefile.am
+++ b/src/base/Makefile.am
@@ -173,7 +173,8 @@ noinst_HEADERS += \
src/base/unix_client_socket.h \
src/base/unix_server_socket.h \
src/base/unix_socket.h \
-   src/base/usrbuf.h
+   src/base/usrbuf.h \
+   src/base/sna.h
 
 TESTS += bin/testleap bin/libbase_test bin/core_common_test
 
@@ -237,7 +238,8 @@ bin_libbase_test_SOURCES = \
src/base/tests/time_compare_test.cc \
src/base/tests/time_convert_test.cc \
src/base/tests/time_subtract_test.cc \
-   src/base/tests/unix_socket_test.cc
+   src/base/tests/unix_socket_test.cc \
+   src/base/tests/sna_test.cc
 
 bin_libbase_test_LDADD = \
$(GTEST_DIR)/lib/libgtest.la \
diff --git a/src/base/sna.h b/src/base/sna.h
new file mode 100644
index 0..b231fb134
--- /dev/null
+++ b/src/base/sna.h
@@ -0,0 +1,126 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2019 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ * Reference: Serial Number Arithmetic from RFC1982
+ *
+ */
+
+#ifndef BASE_SNA_H_
+#define BASE_SNA_H_
+
+#include 
+#include 
+
+#define MAX_16BITS 65536  // 2^16
+#define MAX_32BITS 4294967296  // 2^32
+
+template 
+class _sna {
+ private:
+  T i;
+  uint64_t max() {
+if (typeid(T) == typeid(uint64_t)) {
+  return MAX_32BITS;
+} else if (typeid(T) == typeid(uint32_t)) {
+  return MAX_16BITS;
+} else {
+  printf("Type:%s\n", typeid(T).name());
+  throw std::out_of_range("Invalid type");
+}
+  }
+
+ public:
+  _sna(): i(0) {}
+  _sna(const _sna &t) {
+i = t.i;
+  }
+  explicit _sna(const uint64_t &n) {
+if ((n < 0) || (n > (max()-1)))
+  throw std::out_of_range("SNA assign with invalid value");
+i = n;
+  }
+  _sna& operator=(const _sna &t) {
+// check for self-assignment
+if (&t == this)
+  return *this;
+i = t.i;
+return *this;
+  }
+  T v() const {
+return i;
+  }
+  _sna& operator+=(const uint64_t& n) {
+if ((n < 0) || (n > (max()/2 - 1)))
+  throw std::out_of_range("SNA received invalid addition value");
+i = (i + n) % max();
+return *this;
+  }
+  friend _sna operator+(_sna m, const uint64_t& n) {
+m += n;
+return m;
+  }
+  // prefix ++
+  _sna& operator++() {
+*this += 1;
+return *this;
+  }
+  // postfix ++
+  _sna operator++(int) {
+_sna tmp(*this);
+operator++();
+return tmp;
+  }
+  bool operator==(const _sna& rhs) {
+return i == rhs.i;
+  }
+  bool operator==(const uint32_t val) {
+return i == val;
+  }
+  bool operator!=(const _sna& rhs) {
+return i != rhs.i;
+  }
+  bool operator<(const _sna& rhs) {
+return (i < rhs.i && rhs.i - i < max()/2) || \
+   (i > rhs.i && i - rhs.i > max()/2);
+  }
+  bool operator>=(const _sna& rhs) {
+return !(*this < rhs);
+  }
+  bool operator>(const _sna& rhs) {
+return (i < rhs.i && rhs.i - i > max()/2) || \
+   (i > rhs.i && i - rhs.i < max()/2);
+  }
+  bool operator<=(const _sna& rhs) {
+return !(*this > rhs);
+  }
+  int64_t operator-(const _sna& rhs) {
+if (*this >= rhs) {
+  if (i >= rhs.v()) {
+return i - rhs.v();
+  } else {
+return (i + max()) - rhs.v();
+  }
+} else {
+  if (i < rhs.v()) {
+return i - rhs.v();
+  } else {
+return i - (rhs.v() + max());
+  }
+}
+  }
+};
+
+typedef _sna sna16_t;
+typedef _sna sna32_t;
+
+#endif  // BASE_SNA_H_
diff --git a/src/base/tests/sna_test.cc b/src/base/tests/sna_test.cc
new file mode 100644
index 0..4b7eb83e3
--- /dev/null
+++ b/src/base/tests/sna_test.cc
@@ -0,0 +1,117 @@
+/*  -*- OpenSAF  -*-
+ *
+ * Copyright Ericsson AB 2019 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and 

Re: [devel] [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if Fail [#3104]

2019-10-21 Thread Tran Thuan
Hi Phuc,

Instead of add more code, you can change default value to reduce code.
And releaseAdminOwnerOf() should be called base on "admset_rc" check, I
think.

Example:

--- a/src/smf/smfd/SmfAdminState.cc
+++ b/src/smf/smfd/SmfAdminState.cc
@@ -858,7 +858,7 @@ bool SmfAdminStateHandler::deleteNodeGroup() {
 bool SmfAdminStateHandler::nodeGroupAdminOperation(
 SaAmfAdminOperationIdT adminOp) {
 
-  bool method_rc = true;
+  bool method_rc = false;
 
   TRACE_ENTER();
 
@@ -920,20 +920,17 @@ bool SmfAdminStateHandler::nodeGroupAdminOperation(
   } else if (imm_rc != SA_AIS_OK) {
 LOG_NO("%s adminOpTimeout Fail %s", __FUNCTION__,
saf_error(imm_rc));
 errno_ = imm_rc;
-method_rc = false;
   } else {
 LOG_NO("%s adminOpTimeout Fail %s", __FUNCTION__,
saf_error(oi_rc));
 errno_ = oi_rc;
-method_rc = false;
   }
 }
   } else {
 LOG_NO("%s: becomeAdminOwnerOf(%s) Fail", __FUNCTION__,
nodeGroupName_s.c_str());
-method_rc = false;
   }
 
-  if (method_rc == true) {
+  if (admset_rc == true) {
 TRACE("%s Admin operation is done. Release ownership if nodegroup",
   __FUNCTION__);
 if (releaseAdminOwnerOf(nodeGroupName_s) == false) {


Best Regards,
ThuanTr

-Original Message-
From: phuc.h.chau  
Sent: Monday, October 21, 2019 6:23 PM
To: thuan.t...@dektech.com.au; thang.d.ngu...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; phuc.h.chau

Subject: [PATCH 1/1] smf: Improve SmfAdminStateHandler() Return false if
Fail [#3104]

SW upgrade testing, if found that if a service unit is in
INSTANTIATION_FAILED,
one_step upgrade will not continue with the software installation.
---
 src/smf/smfd/SmfAdminState.cc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/smf/smfd/SmfAdminState.cc b/src/smf/smfd/SmfAdminState.cc
index 076f9f0..a54f47f 100644
--- a/src/smf/smfd/SmfAdminState.cc
+++ b/src/smf/smfd/SmfAdminState.cc
@@ -900,11 +900,13 @@ bool SmfAdminStateHandler::nodeGroupAdminOperation(
 LOG_NO(
 "%s: saImmOmAdminOperationInvoke_2 Fail %s",
 __FUNCTION__, saf_error(imm_rc));
+method_rc = false;
 errno_ = imm_rc;
 break;
   } else if (oi_rc != SA_AIS_OK) {
 LOG_NO("%s: SaAmfAdminOperationId %d Fail %s", __FUNCTION__,
adminOp,
saf_error(oi_rc));
+method_rc = false;
 errno_ = oi_rc;
 break;
   } else {
-- 
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: 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(
+   &gl_mds_mcm_cb->adest_list,
+   (uint8_t *)&adest);
+   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 *)&adest_info->adest;
+   adest_info->svc_cnt = 1;
+   adest_info->tmr_start = false;
+   ncs_patricia_tree_add(
+   &gl_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, &log_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(
+   &gl_mds_mcm_cb->adest_list,
+   (uint8_t *)&adest);
+   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(&pat_tree_params, 0, sizeof(NCS_PATRICIA_PARAMS));
+   pat_tree_params.key_size = sizeof(MDS_DEST);
+   if (NCSCC_RC_SUCCESS !=
+   ncs_patricia_tree_init(&gl_mds_mcm_cb->adest_list,
+  &pat_tree_params)) {
+   m_MDS_LOG_ERR(
+   "MCM:API: patricia_tree_init: adest :failure, L 
mds_mcm_init");
+   return NCSCC_RC_FAILURE;
+   }
+
/* SERVICE TREE */
memset(&pat_tree_params, 0, sizeof(NCS_PATRICIA_PARAMS));
pat_tree_params.key_size = sizeof(MDS_SVC_HDL);
@@ -4966,7 +5021,12 @@ uint3

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

2019-10-21 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(
> + &gl_mds_mcm_cb->adest_list,
> + (uint8_t *)&adest);
> + 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 *)&adest_info->adest;
> + adest_info->svc_cnt = 1;
> + adest_info->tmr_start = false;
> + ncs_patricia_tree_add(
> + &gl_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, &log_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(
> + &gl_mds_mcm_cb->adest_list,
> + (uint8_t *)&adest);
> + 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_VDE