chore(mme): Remove unused trace defines #12055

themarwhal · 2022-03-10T15:43:46Z

Signed-off-by: GitHub noreply@github.com

Summary

Address #11898

Remove the following defines marked as "to be removed"

S1AP_DEBUG_LIST (false logic)
SCTP_DUMP_LIST (false logic)
TRACE_HASHTABLE (false logic)
TRACE_3GPP_SPEC (true logic)

Test Plan

make build_oai
make test_oai

Additional Information

This change is backwards-breaking

github-actions · 2022-03-10T15:44:07Z

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-03-10T15:53:48Z

feg-workflow

    2 files 202 suites 33s ⏱️
363 tests 363 ✔️ 0 💤 0 ❌
377 runs 377 ✔️ 0 💤 0 ❌

Results for commit a3d30bc.

♻️ This comment has been updated with latest results.

themarwhal · 2022-03-10T17:12:40Z

@ssanadhya Please let me know if there are other flags to be removed / cleaned up as well. The GitHub issue also mentions TRACE_HASHTABLE and TRACE_3GPP_SPEC but I was not sure about those.

github-actions · 2022-03-10T17:17:25Z

agw-workflow

    2 files     2 suites 3m 28s ⏱️
554 tests 545 ✔️ 9 💤 0 ❌
555 runs 546 ✔️ 9 💤 0 ❌

Results for commit a3d30bc.

♻️ This comment has been updated with latest results.

ssanadhya · 2022-03-10T22:41:44Z

lte/gateway/c/core/oai/tasks/s1ap/s1ap_mme.c

-  UE_LIST_OUT("SCTP stream send: 0x%04x", ue_ref->sctp_stream_send);
-#endif
-}
+                          void __attribute__((unused)) * *unused_resultP) {}


What's the purpose of this empty function? This can be removed since it is only called in s1ap_dump_enb, which is also removed here.

ah didn't realize it is now empty! Will remove this too.

ssanadhya · 2022-03-10T22:57:03Z

@ssanadhya Please let me know if there are other flags to be removed / cleaned up as well. The GitHub issue also mentions TRACE_HASHTABLE and TRACE_3GPP_SPEC but I was not sure about those.

@themarwhal , we can get rid of TRACE_HASHTABLE since it is always set to false and leading to no-op.
For TRACE_3GPP_SPEC, it will be good to remove the flag but keep the current default behavior in the macro definition here https://github.com/magma/magma/blob/master/lte/gateway/c/core/oai/lib/3gpp/3gpp_requirements.h#L41

themarwhal · 2022-03-10T22:59:52Z

@ssanadhya Please let me know if there are other flags to be removed / cleaned up as well. The GitHub issue also mentions TRACE_HASHTABLE and TRACE_3GPP_SPEC but I was not sure about those.

@themarwhal , we can get rid of TRACE_HASHTABLE since it is always set to false and leading to no-op. For TRACE_3GPP_SPEC, it will be good to remove the flag but keep the current default behavior in the macro definition here https://github.com/magma/magma/blob/master/lte/gateway/c/core/oai/lib/3gpp/3gpp_requirements.h#L41

Thanks @ssanadhya ! Will clean those up as well.

lte/gateway/c/core/oai/lib/hashtable/obj_hashtable_uint64.c

lte/gateway/c/core/oai/tasks/s1ap/s1ap_mme.c

lte/gateway/c/core/oai/lib/3gpp/3gpp_requirements.h

Signed-off-by: GitHub <noreply@github.com>

lionelgo

You can also remove magma/lte/gateway/c/core/oai/include/3gpp_requirements_24.301.h

themarwhal · 2022-03-11T16:21:50Z

You can also remove magma/lte/gateway/c/core/oai/include/3gpp_requirements_24.301.h

Actually, I will move this cleanup to another PR because removing this would remove a lot more macros.

remove magma/lte/gateway/c/core/oai/include/3gpp_requirements_24.301.h #12090

Signed-off-by: GitHub <noreply@github.com>

ssanadhya

Lgtm, thanks for the cleanup!

* 'master' of github.com:magma/magma: chore: Mark nas_converter_test as manual until flakiness is addressed (magma#12130) chore(mme): address GH11898 (magma#12129) fix(agw): Update DNS resolvers for ec2 instance (magma#12045) feat(feg_relay): move session proxy to NH implementation (magma#11080) chore: Use per_file_copt for MME unit test flag (magma#12112) chore(mme): Remove unused trace defines (magma#12055) chore: experiment with only applying ASAN and LSAN to Magma C files (magma#12113) Revert "test(mme): Add injection of state loaded in S1AP state manager (magma#11456)" (magma#12121) chore: bump ssri (magma#12032) feat(dp): Add grant attempt count (magma#12101) fix(mme): Fix typing_extensions version dependency on magma_test (magma#12110)

* chore(mme): Remove unused trace defines Signed-off-by: GitHub <noreply@github.com> * deprecate SCTP_DUMP_LIST Signed-off-by: GitHub <noreply@github.com> * chore(mme): remove TRACE_HASHTABLE Signed-off-by: GitHub <noreply@github.com> * chore(mme): remove TRACE_3GPP_SPEC Signed-off-by: GitHub <noreply@github.com> * remove unused macros Signed-off-by: GitHub <noreply@github.com>

pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines. label Mar 10, 2022

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

themarwhal marked this pull request as ready for review March 10, 2022 17:05

themarwhal requested review from a team and ulaskozat March 10, 2022 17:05

ssanadhya requested changes Mar 10, 2022

View reviewed changes

themarwhal force-pushed the deprecate-trace branch from 74a359f to d8fb47b Compare March 10, 2022 23:10

pull-request-size bot added size/L Denotes a Pull Request that changes 100-499 lines. and removed size/M Denotes a PR that changes 30-99 lines. labels Mar 10, 2022

themarwhal commented Mar 10, 2022

View reviewed changes

lte/gateway/c/core/oai/lib/hashtable/obj_hashtable_uint64.c Show resolved Hide resolved

themarwhal commented Mar 10, 2022

View reviewed changes

lte/gateway/c/core/oai/tasks/s1ap/s1ap_mme.c Outdated Show resolved Hide resolved

themarwhal commented Mar 10, 2022

View reviewed changes

lte/gateway/c/core/oai/lib/3gpp/3gpp_requirements.h Show resolved Hide resolved

themarwhal requested a review from ssanadhya March 10, 2022 23:45

themarwhal marked this pull request as draft March 11, 2022 12:37

github-actions bot added the component: ci All updates on CI (Jenkins/CircleCi/Github Action) label Mar 11, 2022

themarwhal force-pushed the deprecate-trace branch 2 times, most recently from b695948 to be8ca40 Compare March 11, 2022 13:16

themarwhal added 4 commits March 11, 2022 14:12

chore(mme): Remove unused trace defines

6a49524

Signed-off-by: GitHub <noreply@github.com>

deprecate SCTP_DUMP_LIST

95465a7

Signed-off-by: GitHub <noreply@github.com>

chore(mme): remove TRACE_HASHTABLE

4e777ba

Signed-off-by: GitHub <noreply@github.com>

chore(mme): remove TRACE_3GPP_SPEC

20ad4e8

Signed-off-by: GitHub <noreply@github.com>

themarwhal force-pushed the deprecate-trace branch from be8ca40 to 479f71c Compare March 11, 2022 14:12

github-actions bot removed the component: ci All updates on CI (Jenkins/CircleCi/Github Action) label Mar 11, 2022

themarwhal marked this pull request as ready for review March 11, 2022 14:13

themarwhal requested a review from lionelgo March 11, 2022 14:13

lionelgo approved these changes Mar 11, 2022

View reviewed changes

pull-request-size bot added size/XXL Denotes a Pull Request that changes 1000+ lines. and removed size/L Denotes a Pull Request that changes 100-499 lines. labels Mar 11, 2022

remove unused macros

a3d30bc

Signed-off-by: GitHub <noreply@github.com>

themarwhal force-pushed the deprecate-trace branch from 2a4050d to a3d30bc Compare March 11, 2022 16:27

pull-request-size bot added size/L Denotes a Pull Request that changes 100-499 lines. and removed size/XXL Denotes a Pull Request that changes 1000+ lines. labels Mar 11, 2022

themarwhal linked an issue Mar 11, 2022 that may be closed by this pull request

Deprecate c/core/oai CMake precompiler flags for tracing #11898

Closed

themarwhal enabled auto-merge (squash) March 11, 2022 20:31

ssanadhya approved these changes Mar 15, 2022

View reviewed changes

themarwhal merged commit 42df8a8 into magma:master Mar 15, 2022

themarwhal deleted the deprecate-trace branch March 15, 2022 17:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(mme): Remove unused trace defines #12055

chore(mme): Remove unused trace defines #12055

themarwhal commented Mar 10, 2022 •

edited

github-actions bot commented Mar 10, 2022

github-actions bot commented Mar 10, 2022 •

edited

themarwhal commented Mar 10, 2022

github-actions bot commented Mar 10, 2022 •

edited

ssanadhya Mar 10, 2022

themarwhal Mar 10, 2022

ssanadhya commented Mar 10, 2022

themarwhal commented Mar 10, 2022

lionelgo left a comment

themarwhal commented Mar 11, 2022 •

edited

ssanadhya left a comment

chore(mme): Remove unused trace defines #12055

chore(mme): Remove unused trace defines #12055

Conversation

themarwhal commented Mar 10, 2022 • edited

Summary

Test Plan

Additional Information

github-actions bot commented Mar 10, 2022

Howto

More info

github-actions bot commented Mar 10, 2022 • edited

feg-workflow

themarwhal commented Mar 10, 2022

github-actions bot commented Mar 10, 2022 • edited

agw-workflow

ssanadhya Mar 10, 2022

Choose a reason for hiding this comment

themarwhal Mar 10, 2022

Choose a reason for hiding this comment

ssanadhya commented Mar 10, 2022

themarwhal commented Mar 10, 2022

lionelgo left a comment

Choose a reason for hiding this comment

themarwhal commented Mar 11, 2022 • edited

ssanadhya left a comment

Choose a reason for hiding this comment

themarwhal commented Mar 10, 2022 •

edited

github-actions bot commented Mar 10, 2022 •

edited

github-actions bot commented Mar 10, 2022 •

edited

themarwhal commented Mar 11, 2022 •

edited