fix(agw): Modified code to replace state_teid_hash list to protobuf map in spgw task #13843

rsarwad · 2022-09-05T12:37:39Z

Signed-off-by: Rashmi rashmi.sarwad@radisys.com

Summary

fix(agw): Modified code to replace state_teid_hash list to protobuf map in spgw task

The test case, TestUEContextConversion was failing because in function, proto_to_ue() after fetching ue and bearer details, it inserts bearer contexts to map with unique teid. While simulating this test case, bearer contexts were created with same teid. Due to which test case was failing. Fixed the issue by assigning unique teid to bearer contexts

Test Plan

Executed s1ap sanity test suite and unit test cases.

…ap in spgw task Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>

github-actions · 2022-09-05T12:38:00Z

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-09-05T12:40:13Z

lte/gateway/c/core/oai/tasks/sgw/pgw_procedures.cpp

@@ -54,7 +48,7 @@ void delete_pending_procedures(
      base_proc1 = base_proc2;
    }
    LIST_INIT(ctx_p->pending_procedures);
-    free_wrapper((void**)&ctx_p->pending_procedures);
+    free_cpp_wrapper((void**)&ctx_p->pending_procedures);


[cpplint] _{reported by reviewdog 🐶}
Using C-style cast. Use reinterpret_cast<void**>(...) instead [readability/casting] [4]

github-actions · 2022-09-05T12:40:13Z

lte/gateway/c/core/oai/tasks/sgw/spgw_state.cpp

+      delete_pending_procedures(
+          &(context_p->sgw_eps_bearer_context_information));
+
+      free_cpp_wrapper((void**)ptr);


[cpplint] _{reported by reviewdog 🐶}
Using C-style cast. Use reinterpret_cast<void**>(...) instead [readability/casting] [4]

Please fix lint warning

github-actions · 2022-09-05T12:40:13Z

lte/gateway/c/core/oai/tasks/sgw_s8/sgw_s8_handlers.cpp

@@ -1475,7 +1475,7 @@ void sgw_s8_proc_s11_create_bearer_rsp(
  if (pgw_ni_cbr_proc && (LIST_EMPTY(pgw_ni_cbr_proc->pending_eps_bearers))) {
    pgw_base_proc_t* base_proc1 = LIST_FIRST(sgw_context_p->pending_procedures);
    LIST_REMOVE(base_proc1, entries);
-    free_wrapper((void**)&sgw_context_p->pending_procedures);
+    free_cpp_wrapper((void**)&sgw_context_p->pending_procedures);


[cpplint] _{reported by reviewdog 🐶}
Using C-style cast. Use reinterpret_cast<void**>(...) instead [readability/casting] [4]

Please fix lint warning

github-actions · 2022-09-05T12:47:47Z

feg-workflow

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

Results for commit 3526b94.

♻️ This comment has been updated with latest results.

github-actions · 2022-09-05T13:09:43Z

dp-workflow

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

Results for commit 3526b94.

♻️ This comment has been updated with latest results.

github-actions · 2022-09-05T13:39:11Z

agw-workflow

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

Results for commit 3526b94.

♻️ This comment has been updated with latest results.

pruthvihebbani

Left a minor comment

pruthvihebbani · 2022-09-07T12:32:13Z

lte/gateway/c/core/oai/tasks/sgw/sgw_context_manager.cpp

-                              (void (*)(void**))pgw_lite_cm_free_apn, b);
-  bdestroy_wrapper(&b);
-
-  if (new_bearer_context_information->pgw_eps_bearer_context_information.apns ==


Where is APN collection getting created now?

Hi @pruthvihebbani, this removed code was not used so I cleaned it up. The apn information is available within pdn session, https://github.com/magma/magma/blob/master/lte/gateway/c/core/oai/lib/3gpp/3gpp_23.401.h#L122

…w_teid_hl Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>

ssanadhya · 2022-10-06T04:15:12Z

lte/gateway/c/core/oai/tasks/sgw/pgw_handlers.cpp

  status_code_e rc = RETURNok;
-  hash_table_ts_t* hashtblP = NULL;
-  uint32_t num_elements = 0;
+  state_teid_map_t* state_teid_map = nullptr;
  s_plus_p_gw_eps_bearer_context_information_t* spgw_ctxt_p = NULL;


Suggested change

s_plus_p_gw_eps_bearer_context_information_t* spgw_ctxt_p = NULL;

s_plus_p_gw_eps_bearer_context_information_t* spgw_ctxt_p = nullptr;

ssanadhya · 2022-10-06T04:15:26Z

lte/gateway/c/core/oai/tasks/sgw/pgw_handlers.cpp

+  for (auto itr = state_teid_map->map->begin();
+       itr != state_teid_map->map->end(); itr++) {
+    if (!is_lbi_found) {
+      state_teid_map->get(itr->first, &spgw_ctxt_p);
      if (spgw_ctxt_p != NULL) {


Suggested change

if (spgw_ctxt_p != NULL) {

if (spgw_ctxt_p != nullptr) {

ssanadhya · 2022-10-06T04:16:05Z

lte/gateway/c/core/oai/tasks/sgw/pgw_handlers.cpp

+  for (auto itr = state_teid_map->map->begin();
+       itr != state_teid_map->map->end(); itr++) {
+    if (!is_lbi_found) {
+      state_teid_map->get(itr->first, &spgw_ctxt_p);
      if (spgw_ctxt_p != NULL) {


Suggested change

if (spgw_ctxt_p != NULL) {

if (spgw_ctxt_p != nullptr) {

ssanadhya · 2022-10-06T04:21:05Z

lte/gateway/c/core/oai/tasks/sgw/spgw_state.cpp

+      delete_pending_procedures(
+          &(context_p->sgw_eps_bearer_context_information));
+
+      free_cpp_wrapper((void**)ptr);


Please fix lint warning

lte/gateway/c/core/oai/tasks/sgw/spgw_state_converter.cpp

ssanadhya · 2022-10-06T04:24:37Z

lte/gateway/c/core/oai/tasks/sgw_s8/sgw_s8_handlers.cpp

@@ -1475,7 +1475,7 @@ void sgw_s8_proc_s11_create_bearer_rsp(
  if (pgw_ni_cbr_proc && (LIST_EMPTY(pgw_ni_cbr_proc->pending_eps_bearers))) {
    pgw_base_proc_t* base_proc1 = LIST_FIRST(sgw_context_p->pending_procedures);
    LIST_REMOVE(base_proc1, entries);
-    free_wrapper((void**)&sgw_context_p->pending_procedures);
+    free_cpp_wrapper((void**)&sgw_context_p->pending_procedures);


Please fix lint warning

lte/gateway/c/core/oai/test/spgw_task/state_creators.cpp

…w_teid_hl Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>

…w_teid_hl Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>

ssanadhya · 2022-10-11T16:47:23Z

lte/gateway/c/core/oai/tasks/sgw/sgw_handlers.cpp

@@ -1635,8 +1635,9 @@ status_code_e sgw_handle_nw_initiated_actv_bearer_rsp(
    pgw_base_proc_t* base_proc1 = LIST_FIRST(
        spgw_context->sgw_eps_bearer_context_information.pending_procedures);
    LIST_REMOVE(base_proc1, entries);
-    free_wrapper((void**)&spgw_context->sgw_eps_bearer_context_information
-                     .pending_procedures);
+    delete spgw_context->sgw_eps_bearer_context_information.pending_procedures;


Can't we use free_cpp_wrapper here?

It's possible to use free_cpp_wrapper and corrected it now

ssanadhya · 2022-10-11T16:48:00Z

lte/gateway/c/core/oai/tasks/sgw/sgw_handlers.cpp

@@ -1983,7 +1984,8 @@ void handle_failed_create_bearer_response(
        pgw_base_proc_t* base_proc1 =
            LIST_FIRST(sgw_context_p->pending_procedures);
        LIST_REMOVE(base_proc1, entries);
-        free_wrapper((void**)&sgw_context_p->pending_procedures);
+        delete sgw_context_p->pending_procedures;


Use free_cpp_wrapper

ssanadhya · 2022-10-11T16:49:31Z

lte/gateway/c/core/oai/tasks/sgw_s8/sgw_s8_handlers.cpp

-    free_wrapper((void**)&pgw_ni_cbr_proc->pending_eps_bearers);
+    free_cpp_wrapper(
+        reinterpret_cast<void**>(&sgw_context_p->pending_procedures));
+    free_wrapper(


Shouldn't this be free_cpp_wrapper?

In this PR, for pending_eps_bearers memory is allocated with calloc. But in next PR I have changed to new()

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>

ssanadhya

Lgtm

fix(agw): Modified code to replace state_teid_hash list to protobuf m…

e474e3b

…ap in spgw task Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>

rsarwad requested review from a team and ssanadhya September 5, 2022 12:37

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

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

github-actions bot reviewed Sep 5, 2022

View reviewed changes

pruthvihebbani reviewed Sep 7, 2022

View reviewed changes

Merge remote-tracking branch 'upstream/master' into rsarwad_11190_spg…

a544448

…w_teid_hl Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>

ssanadhya requested changes Oct 6, 2022

View reviewed changes

rsarwad added 2 commits October 6, 2022 14:34

Merge remote-tracking branch 'upstream/master' into rsarwad_11190_spg…

2212ad1

…w_teid_hl Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>

Addressed review comments

d3bc939

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>

rsarwad force-pushed the rsarwad_11190_spgw_teid_hl branch from e4bf0cd to d3bc939 Compare October 7, 2022 07:06

Merge remote-tracking branch 'upstream/master' into rsarwad_11190_spg…

5dafbe6

…w_teid_hl Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>

rsarwad requested a review from ssanadhya October 11, 2022 06:14

ssanadhya requested changes Oct 11, 2022

View reviewed changes

Addressed review comments

3526b94

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>

rsarwad requested a review from ssanadhya October 12, 2022 04:51

ssanadhya approved these changes Oct 12, 2022

View reviewed changes

ssanadhya enabled auto-merge (squash) October 12, 2022 05:17

ssanadhya merged commit 634f31c into magma:master Oct 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(agw): Modified code to replace state_teid_hash list to protobuf map in spgw task #13843

fix(agw): Modified code to replace state_teid_hash list to protobuf map in spgw task #13843

rsarwad commented Sep 5, 2022 •

edited

github-actions bot commented Sep 5, 2022

github-actions bot Sep 5, 2022

github-actions bot Sep 5, 2022

ssanadhya Oct 6, 2022

github-actions bot Sep 5, 2022

ssanadhya Oct 6, 2022

github-actions bot commented Sep 5, 2022 •

edited

github-actions bot commented Sep 5, 2022 •

edited

github-actions bot commented Sep 5, 2022 •

edited

pruthvihebbani left a comment

pruthvihebbani Sep 7, 2022

rsarwad Sep 13, 2022

ssanadhya Oct 6, 2022

ssanadhya Oct 6, 2022

ssanadhya Oct 6, 2022

ssanadhya Oct 6, 2022

ssanadhya Oct 6, 2022

ssanadhya Oct 11, 2022

rsarwad Oct 12, 2022

ssanadhya Oct 11, 2022

ssanadhya Oct 11, 2022

rsarwad Oct 12, 2022

ssanadhya left a comment

	s_plus_p_gw_eps_bearer_context_information_t* spgw_ctxt_p = NULL;
	s_plus_p_gw_eps_bearer_context_information_t* spgw_ctxt_p = nullptr;

fix(agw): Modified code to replace state_teid_hash list to protobuf map in spgw task #13843

fix(agw): Modified code to replace state_teid_hash list to protobuf map in spgw task #13843

Conversation

rsarwad commented Sep 5, 2022 • edited

Summary

Test Plan

github-actions bot commented Sep 5, 2022

Howto

More info

github-actions bot Sep 5, 2022

Choose a reason for hiding this comment

github-actions bot Sep 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot Sep 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 5, 2022 • edited

feg-workflow

github-actions bot commented Sep 5, 2022 • edited

dp-workflow

github-actions bot commented Sep 5, 2022 • edited

agw-workflow

pruthvihebbani 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

ssanadhya left a comment

Choose a reason for hiding this comment

rsarwad commented Sep 5, 2022 •

edited

github-actions bot commented Sep 5, 2022 •

edited

github-actions bot commented Sep 5, 2022 •

edited

github-actions bot commented Sep 5, 2022 •

edited