Hi Praveen,
ack, code review only, minor comments below.
/Regards HansN
On 07/27/2017 07:36 AM, Praveen wrote:
SG attribute saAmfSGNumPrefAssignedSUs is applicable to N-Way and N-Way Active
model.
AMF is assigning more than saAmfSGNumPrefAssignedSUs in both N-Way and N-Way
Active model.
Patch fixes this problem.
---
src/amf/amfd/sg.cc | 49 ++++++++++++++++++++----------------------
src/amf/amfd/sg.h | 1 +
src/amf/amfd/sg_nway_fsm.cc | 39 +++++++++++++++++++++++++++++----
src/amf/amfd/sg_nwayact_fsm.cc | 29 ++++++++++++++++++++++++-
4 files changed, 87 insertions(+), 31 deletions(-)
diff --git a/src/amf/amfd/sg.cc b/src/amf/amfd/sg.cc
index 7bdf52a..8f35901 100644
--- a/src/amf/amfd/sg.cc
+++ b/src/amf/amfd/sg.cc
@@ -98,7 +98,7 @@ AVD_SG::AVD_SG()
saAmfSGAutoAdjust(SA_FALSE),
saAmfSGNumPrefActiveSUs(0),
saAmfSGNumPrefStandbySUs(0),
- saAmfSGNumPrefInserviceSUs(~0),
+ saAmfSGNumPrefInserviceSUs(0),
saAmfSGNumPrefAssignedSUs(0),
saAmfSGMaxActiveSIsperSU(0),
saAmfSGMaxStandbySIsperSU(0),
@@ -978,18 +978,18 @@ static void ccb_apply_modify_hdlr(CcbUtilOperationData_t
*opdata) {
sg->saAmfSGNumPrefStandbySUs);
} else if (!strcmp(attribute->attrName, "saAmfSGNumPrefInserviceSUs")) {
if (value_is_deleted)
- sg->saAmfSGNumPrefInserviceSUs = ~0;
+ sg->saAmfSGNumPrefInserviceSUs = 0; //default value for internal use.
else
sg->saAmfSGNumPrefInserviceSUs = *((SaUint32T *)value);
TRACE("Modified saAmfSGNumPrefInserviceSUs is '%u'",
- sg->saAmfSGNumPrefInserviceSUs);
+ sg->pref_inservice_sus());
} else if (!strcmp(attribute->attrName, "saAmfSGNumPrefAssignedSUs")) {
if (value_is_deleted)
- sg->saAmfSGNumPrefAssignedSUs = sg->saAmfSGNumPrefInserviceSUs;
[HansN] why change to default value 0? Default value is
saAmfSGNumPrefInserviceSUs isn't it clearer to use?
+ sg->saAmfSGNumPrefAssignedSUs = 0; //default value for internal use.
else
sg->saAmfSGNumPrefAssignedSUs = *((SaUint32T *)value);
TRACE("Modified saAmfSGNumPrefAssignedSUs is '%u'",
- sg->saAmfSGNumPrefAssignedSUs);
+ sg->pref_assigned_sus());
} else if (!strcmp(attribute->attrName, "saAmfSGMaxActiveSIsperSU")) {
if (value_is_deleted)
sg->saAmfSGMaxActiveSIsperSU = -1;
@@ -1091,11 +1091,11 @@ static void
ccb_apply_modify_hdlr(CcbUtilOperationData_t *opdata) {
if (!strcmp(attribute->attrName, "saAmfSGNumPrefInserviceSUs")) {
if (value_is_deleted)
- sg->saAmfSGNumPrefInserviceSUs = ~0;
+ sg->saAmfSGNumPrefInserviceSUs = 0;
else
sg->saAmfSGNumPrefInserviceSUs = *((SaUint32T *)value);
TRACE("Modified saAmfSGNumPrefInserviceSUs is '%u'",
- sg->saAmfSGNumPrefInserviceSUs);
+ sg->pref_inservice_sus());
if (avd_cb->avail_state_avd == SA_AMF_HA_ACTIVE) {
if (avd_sg_app_su_inst_func(avd_cb, sg) != NCSCC_RC_SUCCESS) {
@@ -1256,7 +1256,7 @@ static void sg_app_sg_admin_unlock_inst(AVD_CL_CB *cb,
AVD_SG *sg) {
(su->saAmfSUPresenceState == SA_AMF_PRESENCE_UNINSTANTIATED)) {
if (su->saAmfSUPreInstantiable == true) {
if (su->su_on_node->node_state == AVD_AVND_STATE_PRESENT) {
- if (su->sg_of_su->saAmfSGNumPrefInserviceSUs > su_try_inst) {
+ if (su->sg_of_su->pref_inservice_sus() > su_try_inst) {
if (avd_snd_presence_msg(cb, su, false) != NCSCC_RC_SUCCESS) {
LOG_NO("%s: Failed to send Instantiation order of '%s' to %x",
__FUNCTION__, su->name.c_str(),
@@ -1944,19 +1944,6 @@ void avd_sg_adjust_config(AVD_SG *sg) {
}
}
}
-
- /* adjust saAmfSGNumPrefAssignedSUs if not configured, only applicable for
- * the N-way and N-way active redundancy models
- */
- if ((sg->saAmfSGNumPrefAssignedSUs == 0) &&
- ((sg->sg_type->saAmfSgtRedundancyModel ==
- SA_AMF_N_WAY_REDUNDANCY_MODEL) ||
- (sg->sg_type->saAmfSgtRedundancyModel ==
- SA_AMF_N_WAY_ACTIVE_REDUNDANCY_MODEL))) {
- sg->saAmfSGNumPrefAssignedSUs = sg->saAmfSGNumPrefInserviceSUs;
- LOG_NO("'%s' saAmfSGNumPrefAssignedSUs adjusted to %u", sg->name.c_str(),
- sg->saAmfSGNumPrefAssignedSUs);
- }
}
/**
@@ -1972,7 +1959,7 @@ uint32_t sg_instantiated_su_count(const AVD_SG *sg) {
for (const auto &su : sg->list_of_su) {
TRACE_1("su'%s', pres state'%u', in_serv'%u', PrefIn'%u'",
su->name.c_str(),
su->saAmfSUPresenceState, su->saAmfSuReadinessState,
- sg->saAmfSGNumPrefInserviceSUs);
+ sg->pref_inservice_sus());
if (((su->saAmfSUPresenceState == SA_AMF_PRESENCE_INSTANTIATED) ||
(su->saAmfSUPresenceState == SA_AMF_PRESENCE_INSTANTIATING) ||
(su->saAmfSUPresenceState == SA_AMF_PRESENCE_RESTARTING))) {
@@ -2051,7 +2038,7 @@ bool sg_stable_after_lock_in_or_unlock_in(AVD_SG *sg) {
}
}
- if (instantiated_sus >= sg->saAmfSGNumPrefInserviceSUs)
+ if (instantiated_sus >= sg->pref_inservice_sus())
return true;
else {
if (to_be_instantiated_sus == 0)
@@ -2358,9 +2345,19 @@ bool AVD_SG::any_assignment_assigned() {
}
uint32_t AVD_SG::pref_assigned_sus() const {
- // If not configured, AMFD has already adjusted to default value in
- // avd_sg_adjust_config().
- return saAmfSGNumPrefAssignedSUs;
+ if (saAmfSGNumPrefAssignedSUs == 0)
+ //default value is saAmfSGNumPrefInserviceSUs.
+ return pref_inservice_sus();
+ else
+ return saAmfSGNumPrefAssignedSUs;
+}
+
+uint32_t AVD_SG::pref_inservice_sus() const {
+ if (saAmfSGNumPrefInserviceSUs == 0)
+ //default value is saAmfSGNumPrefInserviceSUs.
+ return list_of_su.size();
+ else
+ return saAmfSGNumPrefInserviceSUs;
}
/*
diff --git a/src/amf/amfd/sg.h b/src/amf/amfd/sg.h
index 56a43a1..96187ef 100644
--- a/src/amf/amfd/sg.h
+++ b/src/amf/amfd/sg.h
@@ -447,6 +447,7 @@ class AVD_SG {
uint32_t curr_non_instantiated_spare_sus() const;
bool is_middleware() const { return sg_ncs_spec ? true : false; }
uint32_t pref_assigned_sus() const;
+ uint32_t pref_inservice_sus() const;
// Checks if si_equal_distribution is configured for the SG.
bool is_equal() const;
diff --git a/src/amf/amfd/sg_nway_fsm.cc b/src/amf/amfd/sg_nway_fsm.cc
index 29ffd5d..88e7e1a 100644
--- a/src/amf/amfd/sg_nway_fsm.cc
+++ b/src/amf/amfd/sg_nway_fsm.cc
@@ -1149,6 +1149,15 @@ AVD_SU *avd_sg_nway_get_su_std_equal(AVD_SG *sg, AVD_SI
*curr_si) {
curr_su->sg_of_su->saAmfSGMaxStandbySIsperSU)))
continue;
+ if ((sg->pref_assigned_sus() == sg->curr_assigned_sus()) &&
+ (curr_su->list_of_susi == nullptr)) {
+ //PrefAssignedSU count reached so no assignment in fresh SU.
+ //Continue searching next already assigned SU.
+ TRACE_1("PrefAssignedSU count reached in '%s', so no fresh
assignments.",
+ sg->name.c_str());
+ continue;
+ }
+
l_flag = true;
/* Get the current no of Standby assignments on the su */
@@ -1373,10 +1382,8 @@ uint32_t avd_sg_nway_si_assign(AVD_CL_CB *cb, AVD_SG
*sg) {
if (sg->equal_ranked_su == true) {
/* first try to select an SU which has no assignments */
if ((iter->saAmfSUNumCurrActiveSIs == 0) &&
- (iter->saAmfSUNumCurrStandbySIs == 0)) {
- /* got an SU without any assignments select
- * it for this SI's active assignment
- */
+ (iter->saAmfSUNumCurrStandbySIs == 0) &&
+ (sg->pref_assigned_sus() > sg->curr_assigned_sus())) {
pref_su = iter;
TRACE("set %s as pref_su", pref_su->name.c_str());
break;
@@ -1415,6 +1422,14 @@ uint32_t avd_sg_nway_si_assign(AVD_CL_CB *cb, AVD_SG
*sg) {
/* if found, send active assignment */
if (curr_su) {
+ if ((sg->pref_assigned_sus() == sg->curr_assigned_sus()) &&
+ (curr_su->list_of_susi == nullptr)) {
+ //PrefAssignedSU count reached so no assignment in fresh SU.
+ //Next SI may get assgined in already assigned SU.
+ TRACE_1("PrefAssignedSU count reached in '%s', so no fresh
assignments.",
+ sg->name.c_str());
+ continue;
+ }
TRACE("send active assignment to %s", curr_su->name.c_str());
rc = avd_new_assgn_susi(cb, curr_su, curr_si, SA_AMF_HA_ACTIVE, false,
@@ -1481,6 +1496,15 @@ uint32_t avd_sg_nway_si_assign(AVD_CL_CB *cb, AVD_SG
*sg) {
if (avd_su_susi_find(cb, curr_su, curr_si->name) != AVD_SU_SI_REL_NULL)
continue;
[HansN] indentation is not correct below?
+ if ((sg->pref_assigned_sus() == sg->curr_assigned_sus()) &&
+ (curr_su->list_of_susi == nullptr)) {
+ //PrefAssignedSU count reached so no assignment in fresh SU.
+ //Still continue for next pref ranked SU.
+ TRACE_1("PrefAssignedSU count reached in '%s', so no fresh
assignments.",
+ sg->name.c_str());
+ continue;
+ }
+
/* send the standby assignment */
rc = avd_new_assgn_susi(cb, curr_su, curr_si, SA_AMF_HA_STANDBY, false,
&tmp_susi);
@@ -1549,6 +1573,13 @@ uint32_t avd_sg_nway_si_assign(AVD_CL_CB *cb, AVD_SG
*sg) {
curr_su->sg_of_su->saAmfSGMaxStandbySIsperSU)))
continue;
+ if ((sg->pref_assigned_sus() == sg->curr_assigned_sus())
+ && (curr_su->list_of_susi == nullptr)) {
+ //PrefAssignedSU count reached so no assignment in fresh SU.
+ TRACE_1("PrefAssignedSU count reached in '%s', so no fresh
assignments.",
+ sg->name.c_str());
+ continue;
+ }
su_found = true;
/* verify if this su does not have this assignment */
diff --git a/src/amf/amfd/sg_nwayact_fsm.cc b/src/amf/amfd/sg_nwayact_fsm.cc
index 76ff216..68b76f1 100644
--- a/src/amf/amfd/sg_nwayact_fsm.cc
+++ b/src/amf/amfd/sg_nwayact_fsm.cc
@@ -130,6 +130,15 @@ AVD_SU *avd_sg_nacvred_su_chose_asgn(AVD_CL_CB *cb, AVD_SG
*sg) {
continue;
}
+ if ((sg->pref_assigned_sus() == sg->curr_assigned_sus()) &&
+ (i_su->list_of_susi == nullptr)) {
+ //PrefAssignedSU count reached so no assignment in fresh SU.
+ //Check in next siranked su.
+ TRACE_1("PrefAssignedSU count reached in '%s', so no fresh
assignments.",
+ sg->name.c_str());
+ continue;
+ }
+
/* found the SU assign the SI to the SU as active */
if (avd_new_assgn_susi(cb, i_su, i_si, SA_AMF_HA_ACTIVE, false,
&tmp_rel) == NCSCC_RC_SUCCESS) {
@@ -167,6 +176,15 @@ AVD_SU *avd_sg_nacvred_su_chose_asgn(AVD_CL_CB *cb, AVD_SG
*sg) {
continue;
}
+ if ((sg->pref_assigned_sus() == sg->curr_assigned_sus()) &&
+ (i_su->list_of_susi == nullptr)) {
+ //PrefAssignedSU count reached so no assignment in fresh SU.
+ //search in already assigned SU.
+ TRACE_1("PrefAssignedSU count reached in '%s', so no fresh
assignments.",
+ sg->name.c_str());
+ continue;
+ }
+
l_flag = true;
if (i_si->pref_active_assignments() <= i_si->curr_active_assignments()) {
@@ -259,7 +277,6 @@ uint32_t SG_NACV::si_assign(AVD_CL_CB *cb, AVD_SI *si) {
if ((cb->init_state != AVD_APP_STATE) &&
(si->sg_of_si->sg_ncs_spec == false)) {
- LOG_ER("%s:%u: %u", __FILE__, __LINE__, si->sg_of_si->sg_ncs_spec);
return NCSCC_RC_SUCCESS;
}
@@ -1903,6 +1920,16 @@ static AVD_SU *avd_get_qualified_su(AVD_SG *sg, AVD_SI *i_si,
i_su->saAmfSUNumCurrActiveSIs))) {
continue;
}
+
+ if ((sg->pref_assigned_sus() == sg->curr_assigned_sus()) &&
+ (i_su->list_of_susi == nullptr)) {
+ //PrefAssignedSU count reached so no assigned in fresh SU.
+ //continue search in already assigned SU.
+ TRACE_1("PrefAssignedSU count reached in '%s', so no fresh assignments.",
+ sg->name.c_str());
+ continue;
+ }
+
l_flag = true;
if (avd_su_susi_find(avd_cb, i_su, i_si->name) != AVD_SU_SI_REL_NULL) {
/* This SU has already a assignment for this SI go to the
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
