chore(agw): MME retun type fix in nas #12973

vikramgreddy · 2022-06-13T13:32:50Z

Fixes for #11199
Signed-off-by: Vikram G vikram.gurrappagaru@radisys.com

Summary

In order to use the protobuf map, it's required to migrate all c files to cpp. Doing so will result in huge changes. So we planned to commit a small chunk of changes. This PR returns the value as expected by the function prototype.

Test Plan

make test_oai

Additional Information

Signed-off-by: Vikram G <vikram.gurrappagaru@radisys.com>

github-actions · 2022-06-13T13:33:13Z

Thanks for opening a PR! 💯

A couple initial guidelines

All commits must be signed off. This is enforced by DCO check.
All PR titles must follow the semantic commits format. This is enforced by Semantic PR.

Howto

Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
Checks. All required CI checks must pass before merge.
Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

Contributing Conventions for norms around contributed code

If this is your first Magma PR, also consider reading

Developer Onboarding for onboarding as a new Magma developer
Development Workflow for guidance on your first pull request
CI Checks for points of contact for failing or flaky CI checks
GitHub-to-Slack mappings for Magma maintainers for guidance on how to contact maintainers on Slack

github-actions · 2022-06-13T13:41:13Z

feg-workflow

    2 files 203 suites 39s ⏱️
374 tests 374 ✔️ 0 💤 0 ❌
388 runs 388 ✔️ 0 💤 0 ❌

Results for commit 0d474d8.

♻️ This comment has been updated with latest results.

github-actions · 2022-06-13T14:01:06Z

dp-workflow

14 tests 14 ✔️ 3m 33s ⏱️
  1 suites   0 💤
  1 files   0 ❌

Results for commit 0d474d8.

♻️ This comment has been updated with latest results.

github-actions · 2022-06-13T14:36:43Z

agw-workflow

615 tests 611 ✔️ 3m 37s ⏱️
    2 suites     4 💤
    2 files     0 ❌

Results for commit 0d474d8.

♻️ This comment has been updated with latest results.

rsarwad

Left some minor comments

rsarwad · 2022-06-14T06:36:44Z

lte/gateway/c/core/oai/tasks/nas/emm/Detach.c

@@ -307,7 +307,7 @@ status_code_e emm_proc_detach_request(mme_ue_s1ap_id_t ue_id,

 {
  OAILOG_FUNC_IN(LOG_NAS_EMM);
-  int rc;


Generic comment to initialize rc with appropriate value, wherever it is not initialized

rsarwad · 2022-06-14T06:39:46Z

lte/gateway/c/core/oai/tasks/nas/emm/NasTransportHdl.c

@@ -99,7 +99,7 @@
 ***************************************************************************/
 status_code_e emm_proc_uplink_nas_transport(mme_ue_s1ap_id_t ue_id,
                                            bstring nas_msg_pP) {
-  int rc = RETURNok;


rc is used at the time of initializing and returning. You can collect the return value of of fun, emm_proc_nw_initiated_detach_request

rsarwad · 2022-06-14T06:45:47Z

lte/gateway/c/core/oai/tasks/nas/emm/ServiceRequestHdl.c

@@ -70,7 +70,7 @@ static int check_paging_received_without_lai(mme_ue_s1ap_id_t ue_id);
 /****************************************************************************/
 status_code_e emm_proc_service_reject(const mme_ue_s1ap_id_t ue_id,
                                      const uint8_t emm_cause) {
-  int rc = RETURNok;
+  status_code_e rc = RETURNok;


rc is not required here, we can return as OAILOG_FUNC_RETURN(LOG_NAS_EMM, emm_service_reject(ue_id, emm_cause));

@ssanadhya, With addressing this comment, make_testoai was failing because OAILOG_FUNC_RETURN is defined as below
#define OAILOG_FUNC_RETURN(pRoTo, rEtUrNcOdE)
do {
log_func_return(pRoTo, FILE, LINE, FUNCTION,
(long)rEtUrNcOdE);
return rEtUrNcOdE;
} while (0) /*!< \brief informational */
#endif
So the second argument gets replaced twice and if it's function then it gets called twice. So we should always have variable for second argument.
So shall raise new issue to replace function call with variable wherever we are using macro, OAILOG_FUNC_RETURN.

Tracked in issue, #13255

rsarwad · 2022-06-14T06:46:59Z

lte/gateway/c/core/oai/tasks/nas/emm/ServiceRequestHdl.c

@@ -141,7 +141,7 @@ static int emm_service_reject(mme_ue_s1ap_id_t ue_id, uint8_t emm_cause)
 status_code_e emm_proc_extended_service_request(
    const mme_ue_s1ap_id_t ue_id, const extended_service_request_msg* msg) {
  OAILOG_FUNC_IN(LOG_NAS_EMM);
-  int rc = RETURNok;
+  status_code_e rc = RETURNok;


Above logic can be followed here as well

rsarwad · 2022-06-14T06:48:48Z

lte/gateway/c/core/oai/tasks/nas/emm/TrackingAreaUpdate.c

@@ -550,7 +550,7 @@ status_code_e emm_proc_tracking_area_update_request(
 ***************************************************************************/
 status_code_e emm_proc_tracking_area_update_reject(const mme_ue_s1ap_id_t ue_id,
                                                   const int emm_cause) {
-  int rc = RETURNerror;
+  status_code_e rc = RETURNerror;


rc not required here. We can directly return the status of function

rsarwad · 2022-06-14T09:24:18Z

lte/gateway/c/core/oai/tasks/nas/nas_proc.c

@@ -538,7 +538,7 @@ status_code_e nas_proc_implicit_detach_ue_ind(mme_ue_s1ap_id_t ue_id) {
 //------------------------------------------------------------------------------
 status_code_e nas_proc_nw_initiated_detach_ue_request(
    emm_cn_nw_initiated_detach_ue_t* const nw_initiated_detach_p) {
-  int rc = RETURNerror;
+  status_code_e rc = RETURNerror;


rc can be removed and handle as mentioned earlier

rsarwad · 2022-06-14T09:26:01Z

lte/gateway/c/core/oai/tasks/nas/nas_proc.c

@@ -882,7 +882,7 @@ status_code_e nas_proc_cs_domain_mm_information_request(
 status_code_e nas_proc_pdn_disconnect_rsp(
    emm_cn_pdn_disconnect_rsp_t* emm_cn_pdn_disconnect_rsp) {
  OAILOG_FUNC_IN(LOG_NAS_EMM);
-  int rc = RETURNerror;
+  status_code_e rc = RETURNerror;


rc can be removed as mentioned earlier

rsarwad · 2022-06-14T09:26:22Z

lte/gateway/c/core/oai/tasks/nas/nas_proc.c

@@ -856,7 +856,7 @@ static nas_cause_t s6a_error_2_nas_cause(uint32_t s6a_error, int experimental) {

 status_code_e nas_proc_cs_domain_mm_information_request(
    itti_sgsap_mm_information_req_t* const mm_information_req_pP) {
-  int rc = RETURNerror;
+  status_code_e rc = RETURNerror;


same comment as below

lionelgo · 2022-06-14T11:22:54Z

lte/gateway/c/core/oai/tasks/nas/emm/sap/emm_send.c

@@ -130,8 +129,8 @@ status_code_e emm_send_status(const emm_as_status_t* msg,
 **      Others:    None                                       **
 **                                                                        **
 ***************************************************************************/
-status_code_e emm_send_detach_accept(const emm_as_data_t* msg,
-                                     detach_accept_msg* emm_msg) {
+int emm_send_detach_accept(const emm_as_data_t* msg,


we could return something like ssize_t ?

Calling function is expecting int. We need to change in so many places. Can be taken as a part of separate PR

lionelgo · 2022-06-14T12:29:36Z

lte/gateway/c/core/oai/tasks/nas/esm/esm_ebr.c

@@ -112,7 +112,7 @@ void esm_ebr_initialize(void) {
 **      Others:    _esm_ebr_data                                          **
 **                                                                        **
 ***************************************************************************/
-status_code_e esm_ebr_assign(emm_context_t* emm_context) {
+int esm_ebr_assign(emm_context_t* emm_context) {


return type could be ebi_t

Signed-off-by: Vikram G <vikram.gurrappagaru@radisys.com>

ssanadhya · 2022-06-20T18:38:24Z

lte/gateway/c/core/oai/tasks/nas/emm/ServiceRequestHdl.c

@@ -70,10 +70,8 @@ static int check_paging_received_without_lai(mme_ue_s1ap_id_t ue_id);
 /****************************************************************************/
 status_code_e emm_proc_service_reject(const mme_ue_s1ap_id_t ue_id,
                                      const uint8_t emm_cause) {
-  status_code_e rc = RETURNok;
  OAILOG_FUNC_IN(LOG_NAS_EMM);


The emm_proc_service_reject function is just a wrapper around emm_service_reject. Better to rename emm_service_reject to emm_proc_service_reject and get rid of this wrapper.

I will raise as a part of separate PR.?

Tracked in #13134

ssanadhya

Lgtm, can be merged once @rsarwad approves

Signed-off-by: Vikram G <vikram.gurrappagaru@radisys.com>

rsarwad

LGTM

* chore(agw): MME retun type fix in nas Signed-off-by: Vikram G <vikram.gurrappagaru@radisys.com> * chore(agw): Review comments Signed-off-by: Vikram G <vikram.gurrappagaru@radisys.com>

chore(agw): MME retun type fix in nas

14dedcd

Signed-off-by: Vikram G <vikram.gurrappagaru@radisys.com>

vikramgreddy requested review from a team and rsarwad June 13, 2022 13:32

pull-request-size bot added the size/L Denotes a Pull Request that changes 100-499 lines. label Jun 13, 2022

github-actions bot added the component: agw Access gateway-related issue label Jun 13, 2022

rsarwad reviewed Jun 14, 2022

View reviewed changes

lionelgo reviewed Jun 14, 2022

View reviewed changes

chore(agw): Review comments

dce2513

Signed-off-by: Vikram G <vikram.gurrappagaru@radisys.com>

vikramgreddy force-pushed the issue_nas_11199 branch from bb004a3 to dce2513 Compare June 20, 2022 13:31

ssanadhya requested changes Jun 20, 2022

View reviewed changes

lionelgo approved these changes Jun 21, 2022

View reviewed changes

This was referenced Jun 29, 2022

[MME] BE : Rewove wrapper function emm_proc_service_reject function #13134

Closed

[MME] BE : Change return type to ssize_t in emm_send_detach_accept #13135

Closed

ssanadhya approved these changes Jun 29, 2022

View reviewed changes

vikramgreddy mentioned this pull request Jul 6, 2022

Return the status of function without rc #13205

Closed

Merge remote-tracking branch 'upstream/master' into issue_nas_11199

0d474d8

Signed-off-by: Vikram G <vikram.gurrappagaru@radisys.com>

rsarwad approved these changes Jul 11, 2022

View reviewed changes

rsarwad merged commit 98725b7 into magma:master Jul 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(agw): MME retun type fix in nas #12973

chore(agw): MME retun type fix in nas #12973

vikramgreddy commented Jun 13, 2022

github-actions bot commented Jun 13, 2022

github-actions bot commented Jun 13, 2022 •

edited

github-actions bot commented Jun 13, 2022 •

edited

github-actions bot commented Jun 13, 2022 •

edited

rsarwad left a comment

rsarwad Jun 14, 2022

rsarwad Jun 14, 2022

rsarwad Jun 14, 2022

rsarwad Jul 7, 2022

rsarwad Jul 11, 2022

rsarwad Jun 14, 2022

rsarwad Jun 14, 2022

rsarwad Jun 14, 2022

rsarwad Jun 14, 2022

rsarwad Jun 14, 2022

lionelgo Jun 14, 2022

vikramgreddy Jun 20, 2022

lionelgo Jun 14, 2022

ssanadhya Jun 20, 2022

vikramgreddy Jun 29, 2022

ssanadhya Jun 29, 2022

ssanadhya left a comment

rsarwad left a comment

chore(agw): MME retun type fix in nas #12973

chore(agw): MME retun type fix in nas #12973

Conversation

vikramgreddy commented Jun 13, 2022

Summary

Test Plan

Additional Information

github-actions bot commented Jun 13, 2022

Howto

More info

github-actions bot commented Jun 13, 2022 • edited

feg-workflow

github-actions bot commented Jun 13, 2022 • edited

dp-workflow

github-actions bot commented Jun 13, 2022 • edited

agw-workflow

rsarwad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssanadhya left a comment

Choose a reason for hiding this comment

rsarwad left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 13, 2022 •

edited

github-actions bot commented Jun 13, 2022 •

edited

github-actions bot commented Jun 13, 2022 •

edited