fix(agw): Modifed code to port hash list of ue contexts to protobuf map #13895

rsarwad · 2022-09-12T14:33:51Z

Summary

fix(agw): Modifed code to port hash list of ue contexts to protobuf map

Test Plan

Executed s1ap sanity test suite and unit test cases.
Because of DCO check failure, the changes have been duplicated PR, #14342

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

github-actions · 2022-09-12T14:34:10Z

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-12T14:36:59Z

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-12T14:36:59Z

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

  if (ue_context_p) {
    sgw_s11_teid_t* p1 = LIST_FIRST(&(ue_context_p->sgw_s11_teid_list));
    while (p1) {
      if (p1->sgw_s11_teid == teid) {
        LIST_REMOVE(p1, entries);
-        free_wrapper((void**)&p1);
+        free_cpp_wrapper((void**)&p1);


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

github-actions · 2022-09-12T14:36:59Z

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 warnings

Fix the warning

github-actions · 2022-09-12T14:36:59Z

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

    sgw_s11_teid_t* p2 = nullptr;
    while (p1) {
      p2 = LIST_NEXT(p1, entries);
      LIST_REMOVE(p1, entries);
-      free_wrapper((void**)&p1);
+      free_cpp_wrapper((void**)&p1);


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

Fix the warning

github-actions · 2022-09-12T14:37:00Z

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

@@ -97,7 +97,7 @@ void sgw_remove_sgw_bearer_context_information(sgw_state_t* sgw_state,
    while (p1) {
      if (p1->sgw_s11_teid == teid) {
        LIST_REMOVE(p1, entries);
-        free_wrapper((void**)&p1);
+        free_cpp_wrapper((void**)&p1);


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

Please fix lint warning

Fix cpplint warning

github-actions · 2022-09-12T14:37:00Z

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

@@ -1475,7 +1474,7 @@
  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]

Fix the warning

github-actions · 2022-09-12T14:37:00Z

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

@@ -119,6 +118,7 @@ TEST(SPGWStateConverterTest, TestUEContextConversion) {
    EXPECT_TRUE(!memcmp(&want_sgw.last_known_cell_Id,
                        &got_sgw.last_known_cell_Id,
                        sizeof(want_sgw.last_known_cell_Id)));
+    sgw_free_ue_context((void**)&final_ctx);


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

Please fix lint warning

Fix the warning

github-actions · 2022-09-12T14:43:42Z

feg-workflow

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

Results for commit 0f7be0e.

♻️ This comment has been updated with latest results.

github-actions · 2022-09-12T15:05:14Z

agw-workflow

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

Results for commit 0f7be0e.

♻️ This comment has been updated with latest results.

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

github-actions · 2022-09-13T05:30:12Z

Oops! Looks like you failed the DCO check. Be sure to sign all your commits.

Howto

♻️ Updated: ❌ The check is still failing the DCO check after the last commit.

github-actions · 2022-09-13T05:54:35Z

dp-workflow

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

Results for commit 0f7be0e.

♻️ This comment has been updated with latest results.

ajahl · 2022-09-26T12:42:55Z

@rsarwad Can you fix the conflicts and the linter warnings?

ssanadhya · 2022-10-10T16:12:54Z

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

-    spgw_free_s11_bearer_context_information(&new_bearer_context_information);
-    return NULL;
-  }
-
  /*
   * Trying to insert the new tunnel into the tree.


Please update the comment

Suggested change

* Trying to insert the new tunnel into the tree.

* Trying to insert the new tunnel into the map.

ssanadhya · 2022-10-10T16:17:25Z

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

-                    "Failed to free teid from state_teid_ht \n");
-    return temp;
+                    "Failed to free teid from state_teid_map \n");
+    return magma::PROTO_MAP_KEY_ALREADY_EXISTS;


This should be an explicit error, not that key already exists. Can we add an error code for PROTO_MAP_REMOVE_KEY_FAILED

The above comment is addressed at https://github.com/magma/magma/pull/13895/files#diff-8dbb5820f4aef5a4b6295c1040d13d405728e409346d63b37cfac400b8004312R189

ssanadhya · 2022-10-10T16:20:48Z

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

+  state_teid_map_t* state_teid_map = get_spgw_teid_state();
+  if (!state_teid_map) {
+    OAILOG_ERROR(LOG_SPGW_APP, "Failed to get state_teid_map");
+    return spgw_bearer_context_info;


Better to just return a nullptr (which is the value of spgw_bearer_context_info at this point) since this is an error scenario.

Suggested change

return spgw_bearer_context_info;

return nullptr;

ssanadhya · 2022-10-10T16:21:21Z

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

+  map_uint64_spgw_ue_context_t* state_ue_map = get_spgw_ue_state();
+  if (!state_ue_map) {
+    OAILOG_ERROR(LOG_SPGW_APP, "Failed to find spgw_ue_state");
+    OAILOG_FUNC_RETURN(LOG_SPGW_APP, ue_context_p);


Same comment as above

Suggested change

OAILOG_FUNC_RETURN(LOG_SPGW_APP, ue_context_p);

OAILOG_FUNC_RETURN(LOG_SPGW_APP, nullptr);

ssanadhya · 2022-10-10T16:22:13Z

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

+    OAILOG_ERROR(LOG_SPGW_APP, "Failed to find spgw_ue_state");
+    OAILOG_FUNC_RETURN(LOG_SPGW_APP, ue_context_p);
+  }
+  state_ue_map->get(imsi64, &ue_context_p);
  OAILOG_FUNC_RETURN(LOG_SPGW_APP, ue_context_p);
 }

 spgw_ue_context_t* spgw_create_or_get_ue_context(imsi64_t imsi64) {
  OAILOG_FUNC_IN(LOG_SPGW_APP);
  spgw_ue_context_t* ue_context_p = NULL;


Suggested change

spgw_ue_context_t* ue_context_p = NULL;

spgw_ue_context_t* ue_context_p = nullptr;

ssanadhya · 2022-10-10T16:22:28Z

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

+  map_uint64_spgw_ue_context_t* state_ue_map = get_spgw_ue_state();
+  if (!state_ue_map) {
+    OAILOG_ERROR(LOG_SPGW_APP, "Failed to find spgw_ue_state");
+    OAILOG_FUNC_RETURN(LOG_SPGW_APP, ue_context_p);


Suggested change

OAILOG_FUNC_RETURN(LOG_SPGW_APP, ue_context_p);

OAILOG_FUNC_RETURN(LOG_SPGW_APP, nullptr);

ssanadhya · 2022-10-10T16:26:12Z

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

@@ -1984,7 +1989,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;


Please use free_cpp_wrapper

ssanadhya · 2022-10-10T16:28:12Z

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 warnings

ssanadhya · 2022-10-10T16:29:28Z

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

@@ -215,8 +215,10 @@ void SpgwStateConverter::sgw_pdn_connection_to_proto(

 void SpgwStateConverter::proto_to_sgw_pdn_connection(
    const oai::SgwPdnConnection& proto, sgw_pdn_connection_t* state_pdn) {
-  state_pdn->apn_in_use =
-      strndup(proto.apn_in_use().c_str(), proto.apn_in_use().length());
+  if ((proto.apn_in_use().length())) {


Can we use proto.has_apn_in_use() here?

apn_in_use is of string type. For string data type, protobuf doesn't generate api, has_<field_name>;
It's in https://developers.google.com/protocol-buffers/docs/reference/cpp-generated#proto3_string
If field is not set then it returns empty string, but again in code we need to check empty string using memcmp; So I thought to use length of string.

ssanadhya · 2022-10-10T16:31:41Z

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

@@ -97,7 +97,7 @@ void sgw_remove_sgw_bearer_context_information(sgw_state_t* sgw_state,
    while (p1) {
      if (p1->sgw_s11_teid == teid) {
        LIST_REMOVE(p1, entries);
-        free_wrapper((void**)&p1);
+        free_cpp_wrapper((void**)&p1);


Please fix lint warning

ssanadhya · 2022-10-10T16:32:36Z

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

@@ -119,6 +118,7 @@ TEST(SPGWStateConverterTest, TestUEContextConversion) {
    EXPECT_TRUE(!memcmp(&want_sgw.last_known_cell_Id,
                        &got_sgw.last_known_cell_Id,
                        sizeof(want_sgw.last_known_cell_Id)));
+    sgw_free_ue_context((void**)&final_ctx);


Please fix lint warning

pruthvihebbani · 2022-10-10T15:31:32Z

lte/gateway/c/core/oai/include/spgw_state.hpp

@@ -44,9 +44,9 @@ void put_spgw_state(void);
 * Returns pointer to SPGW UE state
 * @return hashtable_ts_t struct with SPGW UE context structs as data


Update the return type in comment

pruthvihebbani · 2022-10-10T15:35:32Z

lte/gateway/c/core/oai/tasks/sgw/sgw.hpp

@@ -1,70 +0,0 @@
-/*


Is this not used anywhere?

sgw.hpp is nowhere used

pruthvihebbani · 2022-10-11T11:57:44Z

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

-        "An error occurred while destroying SGW s11_bearer_context_information "
-        "hashtable");
+  if (state_ue_map.map && state_ue_map.destroy_map() != PROTO_MAP_OK) {
+    OAI_FPRINTF_ERR("An error occurred while destroying SPGW's state ue map ");


What doe FPRINTF do? Can we replace this with OAILOG_ERROR

I think we need to keep "OAI_FPRINTF_ERR" because free_state() would be called when tasks exits. So can't rely on logging task as it might have already exited.
Let me change at other places in this function.

pruthvihebbani · 2022-10-11T12:00:25Z

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

@@ -97,7 +97,7 @@ void sgw_remove_sgw_bearer_context_information(sgw_state_t* sgw_state,
    while (p1) {
      if (p1->sgw_s11_teid == teid) {
        LIST_REMOVE(p1, entries);
-        free_wrapper((void**)&p1);
+        free_cpp_wrapper((void**)&p1);


Fix cpplint warning

pruthvihebbani · 2022-10-11T12:00:44Z

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

@@ -1475,7 +1474,7 @@
  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);


Fix the warning

pruthvihebbani · 2022-10-11T12:02:53Z

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

@@ -119,6 +118,7 @@ TEST(SPGWStateConverterTest, TestUEContextConversion) {
    EXPECT_TRUE(!memcmp(&want_sgw.last_known_cell_Id,
                        &got_sgw.last_known_cell_Id,
                        sizeof(want_sgw.last_known_cell_Id)));
+    sgw_free_ue_context((void**)&final_ctx);


Fix the warning

pruthvihebbani · 2022-10-11T12:03:39Z

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

    sgw_s11_teid_t* p2 = nullptr;
    while (p1) {
      p2 = LIST_NEXT(p1, entries);
      LIST_REMOVE(p1, entries);
-      free_wrapper((void**)&p1);
+      free_cpp_wrapper((void**)&p1);


Fix the warning

pruthvihebbani · 2022-10-11T12:03:51Z

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);


Fix the warning

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

magma#14139) otherwise hanging containers take up to 20s to restart and tests run into timeouts. Signed-off-by: Marco Pfirrmann <marco.pfirrmann@tngtech.com> Co-authored-by: Marco Pfirrmann <christian.kraemer@tngtech.com> Addressed review comment Signed-off-by: Rashmi <rashmi.sarwad@radisys.com> feat(agw): time before killing containers during restart is reduced as (magma#14139) otherwise hanging containers take up to 20s to restart and tests run into timeouts. Signed-off-by: Marco Pfirrmann <marco.pfirrmann@tngtech.com> Co-authored-by: Marco Pfirrmann <christian.kraemer@tngtech.com> Addressed review comment Signed-off-by: Rashmi <rashmi.sarwad@radisys.com> feat(agw): time before killing containers during restart is reduced as (magma#14139) otherwise hanging containers take up to 20s to restart and tests run into timeouts. Signed-off-by: Marco Pfirrmann <marco.pfirrmann@tngtech.com> Co-authored-by: Marco Pfirrmann <christian.kraemer@tngtech.com> fix(ci): there was a bit too much deleted in magma#14050 (magma#14143) Signed-off-by: Fritz Lehnert <13189449+Neudrino@users.noreply.github.com>

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

pruthvihebbani · 2022-10-14T07:11:51Z

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

@@ -112,8 +114,7 @@ status_code_e SpgwStateManager::read_ue_state_from_db() {
      return RETURNerror;
    }
    OAILOG_DEBUG(log_task, "Reading UE state from db for key %s", key.c_str());
-    spgw_ue_context_t* ue_context_p =
-        (spgw_ue_context_t*)calloc(1, sizeof(spgw_ue_context_t));
+    spgw_ue_context_t* ue_context_p = new spgw_ue_context_t();


Null check for ue_context_p

In C++ the new operator already throws an exception (bad_alloc) if it is not able to allocate memory I guess. Why we should need the Null check?

@ajahl should we catch the exception?

Only if we can handle the error, maybe to log the error. However, the application should be stopped.

Then it's good to catch the exception to understand where the application is going wrong. @ssanadhya May I know your thoughts on this?

@rsarwad , since this would be a system error, it is best to let the process terminate as a result of bad_alloc exception. No need to catch that exception.

pruthvihebbani · 2022-10-14T07:12:45Z

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

@@ -55,8 +55,7 @@ status_code_e mock_read_spgw_ue_state_db(
      return RETURNerror;
    }

-    spgw_ue_context_t* ue_context_p =
-        (spgw_ue_context_t*)calloc(1, sizeof(spgw_ue_context_t));
+    spgw_ue_context_t* ue_context_p = new spgw_ue_context_t();


Null check missing

pruthvihebbani · 2022-10-14T07:13:20Z

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

@@ -76,8 +76,7 @@ TEST(SPGWStateConverterTest, TestUEContextConversion) {
    auto want_teid = LIST_FIRST(&initial_ctx->sgw_s11_teid_list)->sgw_s11_teid;

    oai::SpgwUeContext proto_ctx;
-    spgw_ue_context_t* final_ctx =
-        (spgw_ue_context_t*)calloc(1, sizeof(spgw_ue_context_t));
+    spgw_ue_context_t* final_ctx = new spgw_ue_context_t();


Null check missing

pruthvihebbani · 2022-10-14T07:14:26Z

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

  LIST_INIT(s11_proc_create_bearer->pending_eps_bearers);
  for (auto& eps_bearer_proto : proto.pending_eps_bearers()) {
-    sgw_eps_bearer_ctxt_t* eps_bearer =
-        (sgw_eps_bearer_ctxt_t*)calloc(1, sizeof(sgw_eps_bearer_ctxt_t));
+    sgw_eps_bearer_ctxt_t* eps_bearer = new sgw_eps_bearer_ctxt_t();


NULL check missing

pruthvihebbani · 2022-10-14T07:14:48Z

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

@@ -937,26 +936,23 @@ void SpgwStateConverter::insert_proc_into_sgw_pending_procedures(
    const oai::PgwCbrProcedure& proto,
    sgw_eps_bearer_context_information_t::pending_procedures_s*
        pending_procedures) {
-  pgw_ni_cbr_proc_t* s11_proc_create_bearer =
-      (pgw_ni_cbr_proc_t*)calloc(1, sizeof(pgw_ni_cbr_proc_t));
+  pgw_ni_cbr_proc_t* s11_proc_create_bearer = new pgw_ni_cbr_proc_t();


Null check missing

pruthvihebbani · 2022-10-14T07:15:08Z

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

@@ -233,8 +233,7 @@ void SpgwStateConverter::proto_to_sgw_pdn_connection(

  for (int i = 0; i < BEARERS_PER_UE; i++) {
    if (proto.eps_bearer_list(i).eps_bearer_id()) {
-      auto* eps_bearer_entry =
-          (sgw_eps_bearer_ctxt_t*)calloc(1, sizeof(sgw_eps_bearer_ctxt_t));
+      auto* eps_bearer_entry = new sgw_eps_bearer_ctxt_t();


Null check missing

pruthvihebbani · 2022-10-14T07:17:57Z

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

@@ -72,8 +70,7 @@ pgw_ni_cbr_proc_t* pgw_create_procedure_create_bearer(
  LIST_INSERT_HEAD((ctx_p->pending_procedures), base_proc, entries);

  s11_proc_create_bearer->pending_eps_bearers =
-      (pgw_ni_cbr_proc_s::pending_eps_bearers_s*)calloc(
-          1, sizeof(s11_proc_create_bearer->pending_eps_bearers));
+      new pgw_ni_cbr_proc_s::pending_eps_bearers_s();


Null check missing

pruthvihebbani · 2022-10-14T07:18:09Z

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

-  pgw_ni_cbr_proc_t* s11_proc_create_bearer =
-      reinterpret_cast<pgw_ni_cbr_proc_t*>(
-          calloc(1, sizeof(pgw_ni_cbr_proc_t)));
+  pgw_ni_cbr_proc_t* s11_proc_create_bearer = new pgw_ni_cbr_proc_t();


Null check missing

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

github-actions · 2022-10-14T09:22:30Z

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

    sgw_eps_bearer_entry_wrapper->sgw_eps_bearer_entry = eps_bearer;
    LIST_INSERT_HEAD((s11_proc_create_bearer->pending_eps_bearers),
                     sgw_eps_bearer_entry_wrapper, entries);
  }
+  if (failed_to_allocate) {
+    pgw_free_procedure_create_bearer(
+        (pgw_ni_cbr_proc_t**)&s11_proc_create_bearer);


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

ajahl · 2022-10-24T14:29:40Z

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

-      hashtable_ts_create(SGW_STATE_CONTEXT_HT_MAX_SIZE, nullptr,
-                          (void (*)(void**))sgw_free_ue_context, nullptr);
+  state_ue_map.map = new google::protobuf::Map<uint64_t, spgw_ue_context_s*>();
+  if (!(state_ue_map.map)) {


If you use new to instantiate an object in C++ the Null check is always needless because of the bad_alloc exception.

ajahl · 2022-10-24T14:33:57Z

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

-  sgw_eps_bearer_ctxt_t* eps_bearer_ctxt_p =
-      reinterpret_cast<sgw_eps_bearer_ctxt_t*>(
-          calloc(1, sizeof(sgw_eps_bearer_ctxt_t)));
+  sgw_eps_bearer_ctxt_t* eps_bearer_ctxt_p = new sgw_eps_bearer_ctxt_t();

  if (!eps_bearer_ctxt_p) {


The Null check is always needless because of C++ throws a bad_alloc exception.

ajahl · 2022-10-24T14:34:07Z

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

@@ -660,8 +657,7 @@ status_code_e create_temporary_dedicated_bearer_context(
    }
  }
  struct sgw_eps_bearer_entry_wrapper_s* sgw_eps_bearer_entry_p =
-      reinterpret_cast<sgw_eps_bearer_entry_wrapper_s*>(
-          calloc(1, sizeof(*sgw_eps_bearer_entry_p)));
+      new sgw_eps_bearer_entry_wrapper_s();
  if (!sgw_eps_bearer_entry_p) {


The Null check is always needless because of C++ throws a bad_alloc exception.

ajahl · 2022-10-24T14:34:35Z

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

-      reinterpret_cast<pgw_ni_cbr_proc_t*>(
-          calloc(1, sizeof(pgw_ni_cbr_proc_t)));
+  pgw_ni_cbr_proc_t* s11_proc_create_bearer = new pgw_ni_cbr_proc_t();
+  if (!s11_proc_create_bearer) {


The Null check is always needless because of C++ throws a bad_alloc exception.

ajahl · 2022-10-24T14:34:51Z

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

  s11_proc_create_bearer->proc.type =
      PGW_BASE_PROC_TYPE_NETWORK_INITATED_CREATE_BEARER_REQUEST;
  pgw_base_proc_t* base_proc = (pgw_base_proc_t*)s11_proc_create_bearer;

  if (!ctx_p->pending_procedures) {
    ctx_p->pending_procedures =
        new sgw_eps_bearer_context_information_s::pending_procedures_s();
+    if (!ctx_p->pending_procedures) {


The Null check is always needless because of C++ throws a bad_alloc exception.

ajahl · 2022-10-24T14:35:27Z

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

-     * Malloc failed, may be ENOMEM error
-     */
+  new_tunnel = new mme_sgw_tunnel_t();
+  if (new_tunnel == nullptr) {


The Null check after call the new operator is always needless because of C++ throws a bad_alloc exception.

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

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

github-actions · 2022-10-28T12:27:49Z

Oops! Looks like you failed the PR Check DCO. Be sure to sign all your commits.

Howto

♻️ Updated: ❌ The check is still failing the PR Check DCO after the last commit.

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

pruthvihebbani

LGTM

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

rsarwad added 8 commits August 29, 2022 11:54

Modified code to fix compilation errors

618e413

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

Removed files and structures that are no more required

640a511

Ported heid hash list protobuf map

451e87c

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

Merge remote-tracking branch 'upstream/master' into 11190_spgw_teid_hl

1a8aaf6

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

Resolved merge conflicts

8f7a2e7

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

Addressed self comments

f4760bf

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

fix(agw): Modiifed code to port hash list of ue contexts to protobuf map

10b1a0c

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

fix(agw): Modiifed code to port hash list of ue contexts to protobuf map

62bec19

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

rsarwad requested review from a team and panyogesh September 12, 2022 14:33

pull-request-size bot added the size/XL Denotes a Pull Request that changes 500-999 lines. label Sep 12, 2022

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

github-actions bot reviewed Sep 12, 2022

View reviewed changes

Merge remote-tracking branch 'upstream/master' into 11190_spgw_ue_con…

b621678

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

ssanadhya reviewed Oct 10, 2022

View reviewed changes

ssanadhya requested changes Oct 10, 2022

View reviewed changes

pruthvihebbani requested changes Oct 11, 2022

View reviewed changes

rsarwad added 2 commits October 12, 2022 16:53

Addressed review comments

8ca0674

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

Resolved merge conflicts

314c059

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

pull-request-size bot added size/L Denotes a Pull Request that changes 100-499 lines. and removed size/XL Denotes a Pull Request that changes 500-999 lines. labels Oct 12, 2022

rsarwad force-pushed the 11190_spgw_ue_context_ht branch from 6a85922 to 8eafe02 Compare October 13, 2022 04:59

rsarwad requested review from a team and mpfirrmann October 13, 2022 04:59

rsarwad force-pushed the 11190_spgw_ue_context_ht branch from e4b4fc0 to bff7cc9 Compare October 13, 2022 05:12

rsarwad requested a review from a team as a code owner October 13, 2022 05:12

rsarwad requested a review from koolzz October 13, 2022 05:12

Merge remote-tracking branch 'upstream/master' into 11190_spgw_ue_con…

46810d1

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

pruthvihebbani requested changes Oct 14, 2022

View reviewed changes

Addressed review comment

2bffe54

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

github-actions bot reviewed Oct 14, 2022

View reviewed changes

ajahl reviewed Oct 24, 2022

View reviewed changes

rsarwad added 2 commits October 27, 2022 12:19

Merge remote-tracking branch 'upstream/master' into 11190_spgw_ue_con…

a6d1fe3

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

Catching exception on failing to allocate memory

0f7be0e

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

rsarwad force-pushed the 11190_spgw_ue_context_ht branch from 1a21c6c to 0f7be0e Compare October 27, 2022 09:39

rsarwad requested review from ssanadhya and pruthvihebbani October 28, 2022 12:22

Removed the null check

a35732f

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

rsarwad force-pushed the 11190_spgw_ue_context_ht branch from 0754a91 to a35732f Compare October 28, 2022 13:22

pruthvihebbani approved these changes Nov 2, 2022

View reviewed changes

Merge remote-tracking branch 'upstream/master' into 11190_spgw_ue_con…

a149ec6

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

rsarwad force-pushed the 11190_spgw_ue_context_ht branch from cd8575e to a149ec6 Compare November 2, 2022 10:36

rsarwad mentioned this pull request Nov 2, 2022

fix(agw): Modifed code to port hash list of ue contexts to protobuf map #14342

Merged

rsarwad closed this Nov 4, 2022

	* Trying to insert the new tunnel into the tree.
	* Trying to insert the new tunnel into the map.

	OAILOG_FUNC_RETURN(LOG_SPGW_APP, ue_context_p);
	OAILOG_FUNC_RETURN(LOG_SPGW_APP, nullptr);

	spgw_ue_context_t* ue_context_p = NULL;
	spgw_ue_context_t* ue_context_p = nullptr;

		@@ -44,9 +44,9 @@ void put_spgw_state(void);
		* Returns pointer to SPGW UE state
		* @return hashtable_ts_t struct with SPGW UE context structs as data

fix(agw): Modifed code to port hash list of ue contexts to protobuf map #13895

fix(agw): Modifed code to port hash list of ue contexts to protobuf map #13895

Conversation

rsarwad commented Sep 12, 2022 • edited

Summary

Test Plan

github-actions bot commented Sep 12, 2022

Howto

More info

github-actions bot Sep 12, 2022

Choose a reason for hiding this comment

github-actions bot Sep 12, 2022

Choose a reason for hiding this comment

github-actions bot Sep 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot Sep 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot Sep 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot Sep 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot Sep 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 12, 2022 • edited

feg-workflow

github-actions bot commented Sep 12, 2022 • edited

agw-workflow

github-actions bot commented Sep 13, 2022 • edited

Howto

github-actions bot commented Sep 13, 2022 • edited

dp-workflow

ajahl commented Sep 26, 2022 • edited

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

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

ajahl Oct 14, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajahl Oct 24, 2022 • edited

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

github-actions bot Oct 14, 2022

Choose a reason for hiding this comment

rsarwad commented Sep 12, 2022 •

edited

github-actions bot commented Sep 12, 2022 •

edited

github-actions bot commented Sep 12, 2022 •

edited

github-actions bot commented Sep 13, 2022 •

edited

github-actions bot commented Sep 13, 2022 •

edited

ajahl commented Sep 26, 2022 •

edited

ajahl Oct 14, 2022 •

edited

ajahl Oct 24, 2022 •

edited

ajahl Oct 24, 2022 •

edited

github-actions bot commented Oct 28, 2022 •

edited