Lockless changes for CSFB related changes for combined attach, TAU and detach procedure #1035

rsarwad · 2019-12-18T02:32:57Z

Summary: Lock less changes for CSFB related changes for combined attach, TAU and detach procedure
Type: Refactor
Test Plan: Executed s1ap tester test suite

…and detach procedure

rsarwad · 2019-12-19T04:45:05Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgsap_location_update.c

+      ue_context_p->mme_ue_s1ap_id);
+    OAILOG_FUNC_RETURN(LOG_MME_APP, RETURNerror);
+  }
+  /*Initialize SGS context to default values*/


rsarwad · 2019-12-19T04:46:37Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgsap_location_update.c

+  ue_context_p->sgs_context->neaf = false;
+  ue_context_p->sgs_context->ts6_1_timer.id = MME_APP_TIMER_INACTIVE_ID;
+  ue_context_p->sgs_context->ts8_timer.id = MME_APP_TIMER_INACTIVE_ID;
+  ue_context_p->sgs_context->ts9_timer.id = MME_APP_TIMER_INACTIVE_ID;


ts9_timer, ts10_timer, ts13_timer and ts8_timer remove
Remove duplicates

rsarwad · 2019-12-19T05:03:53Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgsap_location_update.c

-  //LAI
-  itti_nas_location_update_acc_p->laicsfb.mccdigit2 =
+  //Store LAI to be sent in Attach Accept/TAU Accept
+  emm_ctx_p->csfbparams.presencemask |= LAI_CSFB;


Keep in order for LAI copy

rsarwad · 2019-12-19T05:04:13Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgsap_location_update.c

+    itti_sgsap_location_update_acc->presencemask);

-  // Mobile Identity
+  //Store Mobile Identity to be sent in Attach Accept/TAU Accept


rsarwad · 2019-12-19T05:21:49Z

lte/gateway/c/oai/tasks/nas/emm/sap/emm_cn.c

+   * If fsm_state is DE-REGISTERED,Location Update Accept is received for
+   * Attach procedure
+   */
  if (EMM_REGISTERED == fsm_state) {


Use variable on left and constant on right

rsarwad · 2019-12-19T05:33:58Z

lte/gateway/c/oai/tasks/nas/emm/sap/emm_cn.c


 //------------------------------------------------------------------------------
 static int _is_csfb_enabled(struct emm_context_s *emm_ctx_p, bstring esm_data)
 {


Rename fun is_csfb_enabled() to emm_proc_combined_attach_req( )

rsarwad · 2019-12-19T05:39:23Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgsap_location_update.c

  }
-  rc = itti_send_msg_to_task(TASK_NAS_MME, INSTANCE_DEFAULT, message_p);
+  //Send Attach Accept/TAU Accept
+  rc = handle_cs_domain_loc_updt_acc(ue_context_p);


send_cs_domain_attach_tau_accept()()

Change the current fun name to handle_cs_domain_loc_updt_acc()

rsarwad · 2019-12-19T05:52:03Z

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

+ ** Outputs:                                                                 **
+ **      Return:    Selected emm cause                                       **
+*******************************************************************************/
+


Remove line

rsarwad · 2019-12-19T05:54:18Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgsap_location_update.c

-  OAILOG_FUNC_RETURN(LOG_MME_APP, rc);
-}
-
-/**********************************************************************************


Move the map_sgs_emm_cause() to mme_app_sgsap_location_update.c file

ssanadhya · 2019-12-20T05:07:46Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgs_detach.c

-           TASK_S1AP, INSTANCE_DEFAULT, ue_context_p->sgs_context->message_p);
-         ue_context_p->sgs_context->message_p = NULL;
-       }
+      if (NULL == ue_context_p->sgs_context->message_p) {


Change to "if (!ue_context_p->sgs_context->message_p)"

ssanadhya · 2019-12-20T05:14:23Z

lte/gateway/c/oai/include/nas_messages_def.h

-  MESSAGE_PRIORITY_MED,
-  itti_nas_cs_domain_location_update_fail_t,
-  nas_cs_domain_location_update_fail)
+


Remove space here to be consistent with rest of the file

ssanadhya · 2019-12-20T05:19:16Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgsap_location_update.c

-  const char *imsi, const unsigned int imsi_len)
+#include "nas_proc.h"
+
+static uint8_t _get_eps_attach_type(uint8_t emm_attach_type);


Move definition of _get_eps_attach_type, _mme_app_update_granted_service_for_ue and _mme_app_compare_tmsi here

ssanadhya · 2019-12-20T05:30:38Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgsap_location_update.c

-  itti_nas_location_update_acc_p->laicsfb.mncdigit3 =
+  emm_ctx_p->csfbparams.lai.mccdigit2 =
+    itti_sgsap_location_update_acc->laicsfb.mccdigit2;
+  emm_ctx_p->csfbparams.lai.mncdigit3 =


Swap line 230 and 236 so all MCC and MNC digits are in order

ssanadhya · 2019-12-20T05:41:01Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgsap_location_update.c

      MOBILE_IDENTITY_TMSI) {
-      tmsi_mobile_identity_t *tmsi_p =
-        &itti_nas_location_update_acc_p->mobileid.tmsi;
+      tmsi_mobile_identity_t received_tmsi = {0}, *received_tmsi_p = NULL;


We don't need two variables here, just "received_tmsi" is enough. Replace calls to "received_tmsi_p" below with "&received_tmsi"

ssanadhya · 2019-12-20T05:41:21Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgsap_location_update.c

-    itti_nas_location_update_acc_p->mobileid.imsi.typeofidentity =
-      itti_sgsap_location_update_acc->mobileid.typeofidentity;
  }
+  /* Store the status of Location Update procedure(success/failure) to send appropriate cause


Line longer than 80 characters

ssanadhya · 2019-12-20T06:26:06Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgsap_location_update.c

  sgsap_location_update_req->presencemask |= SGSAP_E_CGI;
  sgsap_location_update_req->ecgi.plmn.mcc_digit2 =
-    ue_context->e_utran_cgi.plmn.mcc_digit2;
+    ue_context_p->e_utran_cgi.plmn.mcc_digit2;


Re-order the statements so that all MCC and MNC digits are together

ssanadhya · 2019-12-20T06:41:02Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgsap_location_update.c

-      RETURNerror ==
-      (_send_cs_domain_loc_updt_acc_to_nas(
-        itti_sgsap_location_update_acc_p, ue_context_p, SGS_ASSOC_INACTIVE))) {
+      RETURNerror == (_handle_cs_domain_loc_updt_acc(


Move Macro to right side of check

ssanadhya · 2020-01-13T05:19:32Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgs_detach.c

-        */
-        case SGS_DETACH_TYPE_UE_INITIATED_EPS: {
+        /*
+         * Handle Ue initiated EPS detach towards SGS


Convert to single line comment

ssanadhya · 2020-01-13T05:19:43Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgs_detach.c

-        */
-        case SGS_DETACH_TYPE_UE_INITIATED_EXPLICIT_NONEPS: {
+        /*
+         * Handle Ue initiated IMSI detach towards SGS


Convert to single line comment

ssanadhya · 2020-01-13T05:19:57Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgs_detach.c

-        */
-        case SGS_DETACH_TYPE_UE_INITIATED_COMBINED: {
+        /*
+         * Handle Ue initiated Combined EPS/IMSI detach towards SGS


Convert to single line comment

ssanadhya · 2020-01-13T05:20:31Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgs_detach.c

-    * and update the SGS State based on event 
-    */
+       * Call the SGS FSM process function to
+       * process the respective message in different state


Comment can be wrapped in fewer lines

ssanadhya · 2020-01-13T05:21:53Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgs_detach.c

-#include "nas_messages_types.h"
 #include "s1ap_messages_types.h"
 #include "sgs_messages_types.h"
+#include "emm_proc.h"


Do we need this header here?

Yes removed

ssanadhya · 2020-01-13T05:24:18Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgsap_location_update.c

+  OAILOG_FUNC_IN(LOG_MME_APP);
+  additional_updt_t additional_update_type = -1;
+
+  additional_update_type =


Can be merged with line 73

ssanadhya · 2020-01-13T05:26:26Z

lte/gateway/c/oai/tasks/mme_app/mme_app_sgsap_location_update.c

+ **                      else RETURNerror                                     **
+ **                                                                           **
+********************************************************************************/
+int _mme_app_compare_tmsi(


Can this be converted into a macro?

facebook-github-bot

@ssanadhya has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

…d detach procedure (#1035) Summary: Lock less changes for CSFB related changes for combined attach, TAU and detach procedure Type: Refactor Pull Request resolved: #1035 Test Plan: Executed s1ap tester test suite Reviewed By: ardzoht Differential Revision: D19393873 Pulled By: ssanadhya fbshipit-source-id: 36ff5bff75f040c3fb6cede0454d40d7bd55b29a

rsarwad added 4 commits December 12, 2019 09:47

Lockless changes for dedicated procedure

4d15f57

Fixed clang errors

4e9785f

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

8263135

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

3cf23ab

facebook-github-bot added the CLA Signed label Dec 18, 2019

Added lockless changes for CSFB related code of combined attach, TAU …

21572e1

…and detach procedure

rsarwad force-pushed the rsarwad_merge_csfb branch from 6828405 to 21572e1 Compare December 18, 2019 03:22

rsarwad added 2 commits December 19, 2019 09:03

Resolved merge conflicts

172a90b

Fixed clang errors

99f7aeb

rsarwad commented Dec 19, 2019

View reviewed changes

rsarwad added 2 commits December 19, 2019 20:23

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

fbb181a

Incorporated review comments

2444cb2

ssanadhya reviewed Dec 20, 2019

View reviewed changes

rsarwad added 7 commits December 20, 2019 22:01

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

d3393bc

Incorporated review comments

74f0f50

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

14bb465

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

e7f0a7a

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

f1d00af

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

a77716b

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

55af133

rsarwad force-pushed the rsarwad_merge_csfb branch from ca5ee50 to a77716b Compare January 1, 2020 05:11

rsarwad added 2 commits January 2, 2020 09:56

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

b41cab7

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

0c37262

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

8b37b57

ssanadhya suggested changes Jan 13, 2020

View reviewed changes

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

f87adfd

rsarwad force-pushed the rsarwad_merge_csfb branch from 9419de0 to 67dd562 Compare January 13, 2020 11:59

Addressed review comments

ec6d70a

rsarwad force-pushed the rsarwad_merge_csfb branch from 67dd562 to ec6d70a Compare January 14, 2020 02:49

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

e15490c

rsarwad force-pushed the rsarwad_merge_csfb branch from de84313 to e15490c Compare January 14, 2020 04:41

facebook-github-bot reviewed Jan 14, 2020

View reviewed changes

facebook-github-bot closed this Jan 14, 2020

Lockless changes for CSFB related changes for combined attach, TAU and detach procedure #1035

Lockless changes for CSFB related changes for combined attach, TAU and detach procedure #1035

Uh oh!

Conversation

rsarwad commented Dec 18, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ssanadhya Dec 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ssanadhya Dec 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ssanadhya Dec 20, 2019 •

edited

Loading

ssanadhya Dec 20, 2019 •

edited

Loading