fix(agw): c++ migration of mme_app files #14211

pruthvihebbani · 2022-10-18T06:32:26Z

fix(agw): c++ migration of mme_app files

Summary

Migrated all the mme_app files to cpp/hpp and fixed all the compilation errors and warnings

Test Plan

Executed s1ap integ_tests, bazel build and mme unit test

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

…_emm_part2

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

…_app

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

…_app

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

…_app

github-actions · 2022-10-18T06:32:46Z

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

lte/gateway/c/core/oai/tasks/mme_app/mme_app_context.cpp

lte/gateway/c/core/oai/tasks/mme_app/mme_app_edns_emulation.cpp

lte/gateway/c/core/oai/tasks/nas/emm/sap/emm_fsm.cpp

github-actions · 2022-10-18T06:44:13Z

feg-workflow

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

Results for commit 3149868.

♻️ This comment has been updated with latest results.

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

github-actions · 2022-10-18T07:17:56Z

dp-workflow

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

Results for commit 3149868.

♻️ This comment has been updated with latest results.

github-actions · 2022-10-18T08:02:13Z

agw-workflow

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

Results for commit 1fb9b3c.

♻️ This comment has been updated with latest results.

LKreutzer

Bazel changes lgtm!

Please rebase and resolve conflicts

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

rsarwad · 2022-10-20T05:01:24Z

lte/gateway/c/core/oai/include/mme_app_statistics.h

 void update_mme_app_stats_s1u_bearer_add(void);
 #ifdef __cplusplus
 extern "C" {
 #endif
+void update_mme_app_stats_connected_ue_sub(void);


I think this should not be moved within extern C

This gives undefined reference if not under extern C as it is declared in .h file and used in cpp files

rsarwad · 2022-10-20T05:03:12Z

lte/gateway/c/core/oai/include/mme_app_statistics.h

@@ -21,11 +21,11 @@
 /*********************************** Utility Functions to update
 * Statistics**************************************/
 void update_mme_app_stats_connected_ue_add(void);
-void update_mme_app_stats_connected_ue_sub(void);
 void update_mme_app_stats_s1u_bearer_add(void);
 #ifdef __cplusplus
 extern "C" {
 #endif


mme_app_statistics.h, mme_app_ue_context.h and mme_config.h are part of mme_app module, so move these files to hpp

Migration of files under include folder will be taken up as a separate activity as these files will be included in many .c/.cpp files

Separate PR is fine, but it will good if we could do before porting hash table to protobuf map. Because on inclusion of c++ provided features we get lot of errors as some piece of code is in 'C'

rsarwad · 2022-10-20T05:03:42Z

lte/gateway/c/core/oai/include/mme_app_ue_context.h

@@ -41,10 +41,16 @@
 #include "lte/gateway/c/core/oai/include/s6a_messages_types.h"
 #include "lte/gateway/c/core/oai/include/sgw_ie_defs.h"
 #include "lte/gateway/c/core/oai/lib/bstr/bstrlib.h"
+#ifdef __cplusplus


rsarwad · 2022-10-20T05:03:50Z

lte/gateway/c/core/oai/include/mme_app_ue_context.h

+#include "lte/gateway/c/core/oai/tasks/mme_app/mme_app_sgs_fsm.hpp"
+#ifdef __cplusplus
+}
+#endif


rsarwad · 2022-10-20T05:04:32Z

lte/gateway/c/core/oai/include/mme_app_ue_context.h

 #include "lte/gateway/c/core/oai/lib/hashtable/hashtable.h"
 #include "lte/gateway/c/core/oai/lib/hashtable/obj_hashtable.h"
+#include "lte/gateway/c/core/oai/tasks/mme_app/mme_app_sgs_fsm.hpp"


This file should not under extern C

mme_app_ue_context.h is included in many other .hpp/.cpp files which is causing undefined reference to many functions in mme_app_sgs_fsm.hpp. Instead of tracing all the files in which mme_app_ue_context.h is included and then wrapping it under extern C, I have added mme_app_sgs_fsm.hpp under extern C

Could you add comment with mentioning github issue id i.e #13096

rsarwad · 2022-10-20T05:56:16Z

lte/gateway/c/core/oai/tasks/mme_app/mme_app_timer.hpp

 #include "lte/gateway/c/core/oai/tasks/nas/util/nas_timer.hpp"

 #define MME_APP_TIMER_INACTIVE_ID (-1)

+#ifdef __cplusplus
+extern "C" {
+#endif
 int mme_app_start_timer_arg(size_t msec, timer_repeat_t repeat,


Below funs() need not be under extern C

rsarwad · 2022-10-20T06:25:25Z

lte/gateway/c/core/oai/tasks/nas/api/mme/mme_api.hpp

@@ -169,7 +169,11 @@ status_code_e mme_api_notify_new_guti(const mme_ue_s1ap_id_t ueid,
 status_code_e mme_api_new_guti(const imsi_t* const imsi,
                               const guti_t* const old_guti, guti_t* const guti,
                               const tai_t* const originating_tai,
-                               tai_list_t* const tai_list);*/
+                               tai_list_t* const tai_list);


Remove extern from this file because most of the functions would have already defined in cpp files

This is not under extern, it is temporarily moved to emm_headers.hpp file as some functions were throwing undefined reference even though they are declared/defined and used in hpp/cpp files. emm_headers.hpp file will be removed after all the files are moved to c++

Could you add comment with mentioning github issue id i.e #13096

Comment with issue id is already added

rsarwad · 2022-10-20T06:33:03Z

lte/gateway/c/core/oai/tasks/nas/emm/emm_data_ctx.cpp

@@ -48,7 +48,7 @@ extern "C" {
 #include "lte/gateway/c/core/oai/lib/3gpp/3gpp_33.401.h"
 #include "lte/gateway/c/core/oai/lib/3gpp/3gpp_36.401.h"
 #include "lte/gateway/c/core/oai/lib/secu/secu_defs.h"
-#include "lte/gateway/c/core/oai/tasks/mme_app/mme_app_timer.h"
+#include "lte/gateway/c/core/oai/tasks/mme_app/mme_app_timer.hpp"


In this file, emm_ctx_clear_ue_nw_cap() is defined under extern C. Now extern C can be removed

This is not under extern, it is temporarily moved to emm_headers.hpp file as some functions were throwing undefined reference even though they are declared/defined and used in hpp/cpp files. emm_headers.hpp file will be removed after all the files are moved to c++

Could you add comment with mentioning github issue id i.e #13096

rsarwad · 2022-10-20T06:38:15Z

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

+#include "lte/gateway/c/core/oai/tasks/mme_app/mme_app_apn_selection.hpp"
+#include "lte/gateway/c/core/oai/tasks/mme_app/mme_app_defs.hpp"
+#include "lte/gateway/c/core/oai/tasks/mme_app/mme_app_itti_messaging.hpp"
+#include "lte/gateway/c/core/oai/tasks/mme_app/mme_app_sgs_fsm.hpp"


Remove extern C from this file

rsarwad · 2022-10-20T06:41:10Z

lte/gateway/c/core/oai/tasks/nas/esm/DefaultEpsBearerContextActivation.cpp

-#include "lte/gateway/c/core/oai/tasks/mme_app/mme_app_defs.h"
-#include "lte/gateway/c/core/oai/tasks/mme_app/mme_app_timer.h"
+#include "lte/gateway/c/core/oai/tasks/mme_app/mme_app_defs.hpp"
+#include "lte/gateway/c/core/oai/tasks/mme_app/mme_app_timer.hpp"


Remove extern C from this file.
General comment, if function calling and function definitions are in cpp files, then remove from extern C

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

LKreutzer

Could it be that you forgot to rename mme_app_authentication.cpp? Bazel build is failing since it is not finding the file.

Answer:
I removed this file. I will update Bazel.build

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

LKreutzer

Bazel changes lgtm!

…_app

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

…_app

Converted mme_app files

pruthvihebbani added 29 commits August 29, 2022 11:56

Renamed files to cpp/hpp

883755d

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Renamed files to cpp/hpp

99ea210

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

resolved compilation errors

750327e

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Fixed compilation errors

ed68944

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Fixed compilation errors

58ce846

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Fixed compilation errors

2deb6ad

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Resolved undefined reference errors

01840c4

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Resolved undefined references

d8fe5f2

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

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

ad45c74

…_emm_part2

Converted nas/util files to cpp

6a51b14

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Addressed self review comments

4bffab2

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Fixed clang errors

445c96c

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Adding emm_headers.hpp

ba160be

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Migrated files in ies to cpp

55e7907

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Migrated files under ies to cpp

2aaa22c

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Fixed formatting errors

8fe8589

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Converted mme_app files to cpp

e52f7b1

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Resolved compilation errors

e116cb9

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Fixed compilation errors

2187666

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Fixed compilation errors

3306f0d

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Fixed compilation errors

d46a8e4

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Fixing compilation errord

c191cae

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Resolved merge conflicts

d321dd9

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Fixed compilation errors

c65f349

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

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

25c49d6

…_app

Fixed compilation errors

0db54a2

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

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

ff5e7e6

…_app

clang formatting

83445b7

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

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

07aa1e8

…_app

pruthvihebbani requested a review from a team October 18, 2022 06:32

pruthvihebbani requested review from a team, electronjoe and panyogesh October 18, 2022 06:32

pull-request-size bot added the size/XXL Denotes a Pull Request that changes 1000+ lines. label Oct 18, 2022

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

github-actions bot reviewed Oct 18, 2022

View reviewed changes

Fixed cpplint errors

2230442

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

LKreutzer requested review from LKreutzer and removed request for electronjoe October 19, 2022 11:08

LKreutzer requested changes Oct 19, 2022

View reviewed changes

Resolved merge conflicts

886f8d1

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

rsarwad reviewed Oct 20, 2022

View reviewed changes

pruthvihebbani added 2 commits October 27, 2022 16:27

Addressed review comments

291e31f

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

Resolved merge conflicts

cedb64f

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

LKreutzer requested changes Oct 27, 2022

View reviewed changes

Updated bazel build

3149868

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

pruthvihebbani requested a review from LKreutzer October 28, 2022 04:56

LKreutzer approved these changes Oct 28, 2022

View reviewed changes

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

7220290

…_app

pruthvihebbani requested a review from rsarwad October 28, 2022 11:12

pruthvihebbani added 3 commits October 28, 2022 16:52

Addressed review comments

1fb9b3c

Signed-off-by: Pruthvi Hebbani <pruthvi.hebbani@radisys.com>

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

7ba9550

…_app

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

3e04300

…_app

rsarwad approved these changes Nov 3, 2022

View reviewed changes

rsarwad merged commit 2debe31 into magma:master Nov 3, 2022

lucasgonze pushed a commit to lucasgonze/magma that referenced this pull request Feb 29, 2024

fix(agw): c++ migration of mme_app files (magma#14211)

2756591

Converted mme_app files

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(agw): c++ migration of mme_app files #14211

fix(agw): c++ migration of mme_app files #14211

pruthvihebbani commented Oct 18, 2022

github-actions bot commented Oct 18, 2022

github-actions bot commented Oct 18, 2022 •

edited

github-actions bot commented Oct 18, 2022 •

edited

github-actions bot commented Oct 18, 2022 •

edited

LKreutzer left a comment

rsarwad Oct 20, 2022

pruthvihebbani Oct 27, 2022

rsarwad Oct 20, 2022

pruthvihebbani Oct 27, 2022

rsarwad Oct 28, 2022

rsarwad Oct 20, 2022

rsarwad Oct 20, 2022

rsarwad Oct 20, 2022

pruthvihebbani Oct 27, 2022

rsarwad Oct 28, 2022

rsarwad Oct 20, 2022

rsarwad Oct 20, 2022

pruthvihebbani Oct 27, 2022 •

edited

rsarwad Oct 28, 2022

pruthvihebbani Oct 28, 2022

rsarwad Oct 20, 2022

pruthvihebbani Oct 27, 2022

rsarwad Oct 28, 2022

rsarwad Oct 20, 2022

rsarwad Oct 20, 2022

LKreutzer left a comment •

edited

LKreutzer left a comment

fix(agw): c++ migration of mme_app files #14211

fix(agw): c++ migration of mme_app files #14211

Conversation

pruthvihebbani commented Oct 18, 2022

Summary

Test Plan

github-actions bot commented Oct 18, 2022

Howto

More info

github-actions bot commented Oct 18, 2022 • edited

feg-workflow

github-actions bot commented Oct 18, 2022 • edited

dp-workflow

github-actions bot commented Oct 18, 2022 • edited

agw-workflow

LKreutzer 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

pruthvihebbani Oct 27, 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

LKreutzer left a comment • edited

Choose a reason for hiding this comment

LKreutzer left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 18, 2022 •

edited

github-actions bot commented Oct 18, 2022 •

edited

github-actions bot commented Oct 18, 2022 •

edited

pruthvihebbani Oct 27, 2022 •

edited

LKreutzer left a comment •

edited