[VOL-2799] : Fix ONU delete issue on openolt agent
Change-Id: I87605da5f4081aeef0c8d779a49531375c471bde
diff --git a/.gitignore b/.gitignore
index 3db5238..2ebaf57 100644
--- a/.gitignore
+++ b/.gitignore
@@ -33,3 +33,6 @@
# IntelliJ Files
.idea
+
+# vscode
+.vscode
diff --git a/VERSION b/VERSION
index a724a9c..197c4d5 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-2.4.0-dev
+2.4.0
diff --git a/agent/src/core_api_handler.cc b/agent/src/core_api_handler.cc
index c993c48..c6ced48 100644
--- a/agent/src/core_api_handler.cc
+++ b/agent/src/core_api_handler.cc
@@ -770,7 +770,7 @@
BCMOLT_MSG_FIELD_SET(&interface_obj, discovery.control, BCMOLT_CONTROL_STATE_ENABLE);
BCMOLT_MSG_FIELD_SET(&interface_obj, discovery.interval, 5000);
BCMOLT_MSG_FIELD_SET(&interface_obj, discovery.onu_post_discovery_mode,
- BCMOLT_ONU_POST_DISCOVERY_MODE_ACTIVATE);
+ BCMOLT_ONU_POST_DISCOVERY_MODE_NONE);
BCMOLT_MSG_FIELD_SET(&interface_obj, itu.automatic_onu_deactivation.los, true);
BCMOLT_MSG_FIELD_SET(&interface_obj, itu.automatic_onu_deactivation.onu_alarms, true);
BCMOLT_MSG_FIELD_SET(&interface_obj, itu.automatic_onu_deactivation.tiwi, true);
@@ -981,50 +981,85 @@
bcmolt_serial_number serial_number; /**< ONU serial number */
bcmolt_bin_str_36 registration_id; /**< ONU registration ID */
+ bcmolt_onu_set_onu_state onu_oper; /* declare main API struct */
+ bcmolt_onu_state onu_state;
+
onu_key.onu_id = onu_id;
onu_key.pon_ni = intf_id;
BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
BCMOLT_FIELD_SET_PRESENT(&onu_cfg.data, onu_cfg_data, onu_state);
- #ifdef TEST_MODE
+#ifdef TEST_MODE
// It is impossible to mock the setting of onu_cfg.data.onu_state because
// the actual bcmolt_cfg_get passes the address of onu_cfg.hdr and we cannot
// set the onu_cfg.data.onu_state. So a new stub function is created and address
// of onu_cfg is passed. This is one-of case where we need to add test specific
// code in production code.
err = bcmolt_cfg_get__onu_state_stub(dev_id, &onu_cfg);
- #else
+#else
err = bcmolt_cfg_get(dev_id, &onu_cfg.hdr);
- #endif
+#endif
+ OPENOLT_LOG(INFO, openolt_log_id, "Activate ONU : old state = %d, current state = %d\n",
+ onu_cfg.data.onu_old_state, onu_cfg.data.onu_state);
if (err == BCM_ERR_OK) {
- if ((onu_cfg.data.onu_state == BCMOLT_ONU_STATE_PROCESSING ||
- onu_cfg.data.onu_state == BCMOLT_ONU_STATE_ACTIVE) ||
- (onu_cfg.data.onu_state == BCMOLT_ONU_STATE_INACTIVE &&
- onu_cfg.data.onu_old_state == BCMOLT_ONU_STATE_NOT_CONFIGURED))
+ if (onu_cfg.data.onu_state == BCMOLT_ONU_STATE_ACTIVE) {
+ OPENOLT_LOG(INFO, openolt_log_id, "ONU is already in ACTIVE state, \
+not processing this request for pon_intf=%d onu_id=%d\n", intf_id, onu_id);
return Status::OK;
+ } else if (onu_cfg.data.onu_state != BCMOLT_ONU_STATE_NOT_CONFIGURED &&
+ onu_cfg.data.onu_state != BCMOLT_ONU_STATE_INACTIVE) {
+ // We need the ONU to be in NOT_CONFIGURED or INACTIVE state to further process the request
+ OPENOLT_LOG(ERROR, openolt_log_id, "ONU in an invalid state to process the request, \
+state=%d pon_intf=%d onu_id=%d\n", onu_cfg.data.onu_state, intf_id, onu_id);
+ return bcm_to_grpc_err(err, "Failed to activate ONU, invalid ONU state");
+ }
+ } else {
+ // This should never happen. BAL GET should succeed for non-existant ONUs too. The state of such ONUs will be NOT_CONFIGURED
+ OPENOLT_LOG(ERROR, openolt_log_id, "ONU state query failed pon_intf=%d onu_id=%d\n", intf_id, onu_id);
+ return bcm_to_grpc_err(err, "onu get failed");
}
- OPENOLT_LOG(INFO, openolt_log_id, "Enabling ONU %d on PON %d : vendor id %s, \
+ // If the ONU is not configured at all we need to first configure it
+ if (onu_cfg.data.onu_state == BCMOLT_ONU_STATE_NOT_CONFIGURED) {
+ OPENOLT_LOG(INFO, openolt_log_id, "Configuring ONU %d on PON %d : vendor id %s, \
vendor specific %s, pir %d\n", onu_id, intf_id, vendor_id,
- vendor_specific_to_str(vendor_specific).c_str(), pir);
+ vendor_specific_to_str(vendor_specific).c_str(), pir);
- memcpy(serial_number.vendor_id.arr, vendor_id, 4);
- memcpy(serial_number.vendor_specific.arr, vendor_specific, 4);
- BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
- BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.serial_number, serial_number);
- BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.auto_learning, BCMOS_TRUE);
- /*set burst and data profiles to fec disabled*/
- if (board_technology == "XGS-PON") {
- BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.xgpon.ranging_burst_profile, 2);
- BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.xgpon.data_burst_profile, 1);
- } else if (board_technology == "GPON") {
- BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.gpon.ds_ber_reporting_interval, 1000000);
- BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.gpon.omci_port_id, onu_id);
+ memcpy(serial_number.vendor_id.arr, vendor_id, 4);
+ memcpy(serial_number.vendor_specific.arr, vendor_specific, 4);
+ BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
+ BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.serial_number, serial_number);
+ BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.auto_learning, BCMOS_TRUE);
+ /*set burst and data profiles to fec disabled*/
+ if (board_technology == "XGS-PON") {
+ BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.xgpon.ranging_burst_profile, 2);
+ BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.xgpon.data_burst_profile, 1);
+ } else if (board_technology == "GPON") {
+ BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.gpon.ds_ber_reporting_interval, 1000000);
+ BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.gpon.omci_port_id, onu_id);
+ }
+ err = bcmolt_cfg_set(dev_id, &onu_cfg.hdr);
+ if (err != BCM_ERR_OK) {
+ OPENOLT_LOG(ERROR, openolt_log_id, "Failed to configure ONU %d on PON %d, err = %s\n", onu_id, intf_id, bcmos_strerror(err));
+ return bcm_to_grpc_err(err, "Failed to configure ONU");
+ }
}
- err = bcmolt_cfg_set(dev_id, &onu_cfg.hdr);
+
+ // Now that the ONU is configured, move the ONU to ACTIVE state
+ memset(&onu_cfg, 0, sizeof(bcmolt_onu_cfg));
+ BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
+ BCMOLT_FIELD_SET_PRESENT(&onu_cfg.data, onu_cfg_data, onu_state);
+ BCMOLT_OPER_INIT(&onu_oper, onu, set_onu_state, onu_key);
+ BCMOLT_FIELD_SET(&onu_oper.data, onu_set_onu_state_data,
+ onu_state, BCMOLT_ONU_OPERATION_ACTIVE);
+ err = bcmolt_oper_submit(dev_id, &onu_oper.hdr);
if (err != BCM_ERR_OK) {
- OPENOLT_LOG(ERROR, openolt_log_id, "Failed to set activate ONU %d on PON %d, err = %s\n", onu_id, intf_id, bcmos_strerror(err));
+ OPENOLT_LOG(ERROR, openolt_log_id, "Failed to activate ONU %d on PON %d, err = %s\n", onu_id, intf_id, bcmos_strerror(err));
return bcm_to_grpc_err(err, "Failed to activate ONU");
}
+ // ONU will eventually get activated after we have submitted the operation request. The adapter will receive an asynchronous
+ // ONU_ACTIVATION_COMPLETED_INDICATION
+
+ OPENOLT_LOG(INFO, openolt_log_id, "Activated ONU, onu_id %d on PON %d\n", onu_id, intf_id);
return Status::OK;
}
@@ -1084,9 +1119,9 @@
err = get_onu_status((bcmolt_interface)intf_id, onu_id, &onu_state);
if (err == BCM_ERR_OK) {
- if (onu_state == BCMOLT_ONU_STATE_ACTIVE) {
- OPENOLT_LOG(INFO, openolt_log_id, "Onu is Active, onu_id: %d, waiting for onu deactivate complete response\n",
- intf_id);
+ if (onu_state != BCMOLT_ONU_STATE_INACTIVE) {
+ OPENOLT_LOG(INFO, openolt_log_id, "waiting for onu deactivate complete response: intf_id=%d, onu_id=%d\n",
+ intf_id, onu_id);
err = wait_for_onu_deactivate_complete(intf_id, onu_id);
if (err) {
OPENOLT_LOG(ERROR, openolt_log_id, "failed to delete onu intf_id %d, onu_id %d\n",
@@ -1094,7 +1129,7 @@
return bcm_to_grpc_err(err, "Failed to delete ONU");
}
}
- else if (onu_state == BCMOLT_ONU_STATE_INACTIVE) {
+ else {
OPENOLT_LOG(INFO, openolt_log_id, "Onu is Inactive, onu_id: %d, not waiting for onu deactivate complete response\n",
intf_id);
}
diff --git a/agent/test/src/test_core.cc b/agent/test/src/test_core.cc
index b08b92e..7f9926f 100644
--- a/agent/test/src/test_core.cc
+++ b/agent/test/src/test_core.cc
@@ -838,48 +838,30 @@
}
};
-// Test 1 - ActivateOnu success case
-TEST_F(TestActivateOnu, ActivateOnuSuccess) {
- bcmos_errno onu_cfg_get_res = BCM_ERR_INTERNAL;
- bcmos_errno onu_cfg_get_stub_res = BCM_ERR_INTERNAL;
+// Test 1 - ActivateOnu success case - ONU in BCMOLT_ONU_STATE_NOT_CONFIGURED state
+TEST_F(TestActivateOnu, ActivateOnuSuccessOnuNotConfigured) {
+ bcmos_errno onu_cfg_get_res = BCM_ERR_OK;
+ bcmos_errno onu_cfg_get_stub_res = BCM_ERR_OK;
bcmos_errno onu_cfg_set_res = BCM_ERR_OK;
+ bcmos_errno onu_oper_submit_res = BCM_ERR_OK;
bcmolt_onu_cfg onu_cfg;
bcmolt_onu_key onu_key;
BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
- onu_cfg.data.onu_state = BCMOLT_ONU_STATE_ACTIVE;
+ onu_cfg.data.onu_state = BCMOLT_ONU_STATE_NOT_CONFIGURED;
EXPECT_GLOBAL_CALL(bcmolt_cfg_get__onu_state_stub, bcmolt_cfg_get__onu_state_stub(_, _))
.WillOnce(DoAll(SetArg1ToBcmOltOnuCfg(onu_cfg), Return(onu_cfg_get_stub_res)));
ON_CALL(balMock, bcmolt_cfg_get(_, _)).WillByDefault(Return(onu_cfg_get_res));
ON_CALL(balMock, bcmolt_cfg_set(_, _)).WillByDefault(Return(onu_cfg_set_res));
+ ON_CALL(balMock, bcmolt_oper_submit(_, _)).WillByDefault(Return(onu_oper_submit_res));
Status status = ActivateOnu_(pon_id, onu_id, vendor_id.c_str(), vendor_specific.c_str(), pir);
ASSERT_TRUE( status.error_message() == Status::OK.error_message() );
}
-// Test 2 - ActivateOnu failure case
-TEST_F(TestActivateOnu, ActivateOnuFailure) {
- bcmos_errno onu_cfg_get_res = BCM_ERR_INTERNAL;
- bcmos_errno onu_cfg_get_stub_res = BCM_ERR_INTERNAL;
- bcmos_errno onu_cfg_set_res = BCM_ERR_INTERNAL;
-
- bcmolt_onu_cfg onu_cfg;
- bcmolt_onu_key onu_key;
- BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
- onu_cfg.data.onu_state = BCMOLT_ONU_STATE_ACTIVE;
- EXPECT_GLOBAL_CALL(bcmolt_cfg_get__onu_state_stub, bcmolt_cfg_get__onu_state_stub(_, _))
- .WillOnce(DoAll(SetArg1ToBcmOltOnuCfg(onu_cfg), Return(onu_cfg_get_stub_res)));
-
- ON_CALL(balMock, bcmolt_cfg_get(_, _)).WillByDefault(Return(onu_cfg_get_res));
- ON_CALL(balMock, bcmolt_cfg_set(_, _)).WillByDefault(Return(onu_cfg_set_res));
-
- Status status = ActivateOnu_(pon_id, onu_id, vendor_id.c_str(), vendor_specific.c_str(), pir);
- ASSERT_FALSE( status.error_message() == Status::OK.error_message() );
-}
-
-// Test 3 - ActivateOnu - Onu already under processing case
-TEST_F(TestActivateOnu, ActivateOnuProcessing) {
+// Test 2 - ActivateOnu success case - ONU already in BCMOLT_ONU_STATE_ACTIVE state
+TEST_F(TestActivateOnu, ActivateOnuSuccessOnuAlreadyActive) {
bcmos_errno onu_cfg_get_res = BCM_ERR_OK;
bcmos_errno onu_cfg_get_stub_res = BCM_ERR_OK;
@@ -889,12 +871,89 @@
onu_cfg.data.onu_state = BCMOLT_ONU_STATE_ACTIVE;
EXPECT_GLOBAL_CALL(bcmolt_cfg_get__onu_state_stub, bcmolt_cfg_get__onu_state_stub(_, _))
.WillOnce(DoAll(SetArg1ToBcmOltOnuCfg(onu_cfg), Return(onu_cfg_get_stub_res)));
+
ON_CALL(balMock, bcmolt_cfg_get(_, _)).WillByDefault(Return(onu_cfg_get_res));
Status status = ActivateOnu_(pon_id, onu_id, vendor_id.c_str(), vendor_specific.c_str(), pir);
ASSERT_TRUE( status.error_message() == Status::OK.error_message() );
}
+// Test 3 - ActivateOnu success case - ONU in BCMOLT_ONU_STATE_INACTIVE state
+TEST_F(TestActivateOnu, ActivateOnuSuccessOnuInactive) {
+ bcmos_errno onu_cfg_get_res = BCM_ERR_OK;
+ bcmos_errno onu_cfg_get_stub_res = BCM_ERR_OK;
+ bcmos_errno onu_oper_submit_res = BCM_ERR_OK;
+
+ bcmolt_onu_cfg onu_cfg;
+ bcmolt_onu_key onu_key;
+ BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
+ onu_cfg.data.onu_state = BCMOLT_ONU_STATE_INACTIVE;
+ EXPECT_GLOBAL_CALL(bcmolt_cfg_get__onu_state_stub, bcmolt_cfg_get__onu_state_stub(_, _))
+ .WillOnce(DoAll(SetArg1ToBcmOltOnuCfg(onu_cfg), Return(onu_cfg_get_stub_res)));
+
+ ON_CALL(balMock, bcmolt_cfg_get(_, _)).WillByDefault(Return(onu_cfg_get_res));
+ ON_CALL(balMock, bcmolt_oper_submit(_, _)).WillByDefault(Return(onu_oper_submit_res));
+
+
+ Status status = ActivateOnu_(pon_id, onu_id, vendor_id.c_str(), vendor_specific.c_str(), pir);
+ ASSERT_TRUE( status.error_message() == Status::OK.error_message() );
+}
+
+// Test 4 - ActivateOnu failure case - ONU in invalid state (for this ex: BCMOLT_ONU_STATE_LOW_POWER_DOZE)
+TEST_F(TestActivateOnu, ActivateOnuFailOnuInvalidState) {
+ bcmos_errno onu_cfg_get_res = BCM_ERR_OK;
+ bcmos_errno onu_cfg_get_stub_res = BCM_ERR_OK;
+
+ bcmolt_onu_cfg onu_cfg;
+ bcmolt_onu_key onu_key;
+ BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
+ onu_cfg.data.onu_state = BCMOLT_ONU_STATE_LOW_POWER_DOZE; // some invalid state which we dont recognize or process
+ EXPECT_GLOBAL_CALL(bcmolt_cfg_get__onu_state_stub, bcmolt_cfg_get__onu_state_stub(_, _))
+ .WillOnce(DoAll(SetArg1ToBcmOltOnuCfg(onu_cfg), Return(onu_cfg_get_stub_res)));
+
+ ON_CALL(balMock, bcmolt_cfg_get(_, _)).WillByDefault(Return(onu_cfg_get_res));
+
+ Status status = ActivateOnu_(pon_id, onu_id, vendor_id.c_str(), vendor_specific.c_str(), pir);
+ ASSERT_FALSE( status.error_message() == Status::OK.error_message() );
+}
+
+// Test 5 - ActivateOnu failure case - cfg_get failure
+TEST_F(TestActivateOnu, ActivateOnuFailCfgGetFail) {
+ bcmos_errno onu_cfg_get_stub_res = BCM_ERR_INTERNAL;// return cfg_get failure
+
+ bcmolt_onu_cfg onu_cfg;
+ bcmolt_onu_key onu_key;
+ BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
+ onu_cfg.data.onu_state = BCMOLT_ONU_STATE_ACTIVE;
+ EXPECT_GLOBAL_CALL(bcmolt_cfg_get__onu_state_stub, bcmolt_cfg_get__onu_state_stub(_, _))
+ .WillOnce(DoAll(SetArg1ToBcmOltOnuCfg(onu_cfg), Return(onu_cfg_get_stub_res)));
+
+ Status status = ActivateOnu_(pon_id, onu_id, vendor_id.c_str(), vendor_specific.c_str(), pir);
+ ASSERT_FALSE( status.error_message() == Status::OK.error_message() );
+}
+
+// Test 6 - ActivateOnu failure case - oper_submit failure
+TEST_F(TestActivateOnu, ActivateOnuFailOperSubmitFail) {
+ bcmos_errno onu_cfg_get_res = BCM_ERR_OK;
+ bcmos_errno onu_cfg_get_stub_res = BCM_ERR_OK;
+ bcmos_errno onu_cfg_set_res = BCM_ERR_OK;
+ bcmos_errno onu_oper_submit_res = BCM_ERR_INTERNAL; // return oper_submit failure
+
+ bcmolt_onu_cfg onu_cfg;
+ bcmolt_onu_key onu_key;
+ BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
+ onu_cfg.data.onu_state = BCMOLT_ONU_STATE_NOT_CONFIGURED;
+ EXPECT_GLOBAL_CALL(bcmolt_cfg_get__onu_state_stub, bcmolt_cfg_get__onu_state_stub(_, _))
+ .WillOnce(DoAll(SetArg1ToBcmOltOnuCfg(onu_cfg), Return(onu_cfg_get_stub_res)));
+
+ ON_CALL(balMock, bcmolt_cfg_get(_, _)).WillByDefault(Return(onu_cfg_get_res));
+ ON_CALL(balMock, bcmolt_cfg_set(_, _)).WillByDefault(Return(onu_cfg_set_res));
+ ON_CALL(balMock, bcmolt_oper_submit(_, _)).WillByDefault(Return(onu_oper_submit_res));
+
+ Status status = ActivateOnu_(pon_id, onu_id, vendor_id.c_str(), vendor_specific.c_str(), pir);
+ ASSERT_FALSE( status.error_message() == Status::OK.error_message() );
+}
+
////////////////////////////////////////////////////////////////////////////
// For testing DeactivateOnu functionality
////////////////////////////////////////////////////////////////////////////