Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(agw): Modified code to port spgw's state contexts to protobuf structure #14810

Closed
wants to merge 41 commits into from

Conversation

rsarwad
Copy link
Contributor

@rsarwad rsarwad commented Jan 6, 2023

Summary

fix(agw): Modified code to port spgw's state contexts to protobuf structure

Test Plan

Executed unit test cases and s1ap sanity test suite

… protobuf structure

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

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
… protobuf structure, S11BearerContext

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

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

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

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

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
…rwad/magma into 11191_spgw_teid_ctxt_changes

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

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

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

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

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

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
…rwad/magma into 11191_spgw_ue_ctxt_changes

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
…ad/magma into 11191_spgw_state_ctxt_changes

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

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
@pull-request-size pull-request-size bot added the size/XXL Denotes a Pull Request that changes 1000+ lines. label Jan 6, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2023

Thanks for opening a PR! 💯

A couple initial guidelines

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

If this is your first Magma PR, also consider reading

@github-actions github-actions bot added component: agw Access gateway-related issue component: orc8r Orchestrator-related issue labels Jan 6, 2023
pco_req.protocol_or_container_ids[0].id =
PCO_CI_IP_ADDRESS_ALLOCATION_VIA_NAS_SIGNALLING;
magma::lte::oai::PcoProtocol* pco_protocol = pco_req.add_pco_protocol();
pco_protocol->set_id(PCO_CI_IP_ADDRESS_ALLOCATION_VIA_NAS_SIGNALLING);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[misspell] reported by reviewdog 🐶
"SIGNALLING" is a misspelling of "SIGNALING"

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
lte/gateway/c/core/oai/include/spgw_types.hpp Show resolved Hide resolved
(proto_ip->pdn_type() == IPv6 || proto_ip->pdn_type() == IPv4_AND_v6)) {
inet_pton(AF_INET6, proto_ip->ipv6_addr().c_str(), ipv6);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[cpplint] reported by reviewdog 🐶
Closing brace should be aligned with beginning of struct in_addr [whitespace/indent] [3]

break;
default:
break;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[cpplint] reported by reviewdog 🐶
You don't need a ; after a } [readability/braces] [4]


free_wrapper((void**)&state_cache_p);
state_cache_p->Clear();
free_cpp_wrapper((void**)&state_cache_p);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

eps_bearer_ctxt->s_gw_teid_S5_S8_up);
}

// TODO Need to add handling on failing to delete s1-u tunnel rules from
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[cpplint] reported by reviewdog 🐶
Missing username in TODO; it should look like "// TODO(my_username): Stuff." [readability/todo] [2]

@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2023

FeG Lint & Test

370 tests   369 ✔️  35s ⏱️
187 suites      0 💤
    1 files        1

For more details on these failures, see this check.

Results for commit 8f24b24.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2023

DP Lint & Test

0 tests   0 ✔️  0s ⏱️
0 suites  0 💤
0 files    0

Results for commit 8f24b24.

♻️ This comment has been updated with latest results.

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2023

Orc8r Lint & Test

551 tests   550 ✔️  1m 36s ⏱️
204 suites      0 💤
    1 files        1

For more details on these failures, see this check.

Results for commit 8f24b24.

♻️ This comment has been updated with latest results.

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

@LKreutzer LKreutzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the spgw_state_converter_test need to be removed?

…ad/magma into 11191_spgw_ue_ctxt_changes

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
@rsarwad
Copy link
Contributor Author

rsarwad commented Jan 9, 2023

Why does the spgw_state_converter_test need to be removed?
The previous code used to convert C structure to proto structure while writing to DB and vice-versa while reading from DB. This PR will have instance of protobuf structure itself, so doesn't involve conversion between C structure to proto structure. Tests within file, spgw_state_converter_test used to validate the conversion logic.

Copy link
Contributor

@LKreutzer LKreutzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are LSan and ASan errors in the SPGW procedures tests:

 //lte/gateway/c/core/oai/test/spgw_task:spgw_procedures_dedicated_bearer_test FAILED in 18.6s
  /var/tmp/bazel/execroot/__main__/bazel-out/k8-fastbuild/testlogs/lte/gateway/c/core/oai/test/spgw_task/spgw_procedures_dedicated_bearer_test/test.log
//lte/gateway/c/core/oai/test/spgw_task:spgw_procedures_session_test     FAILED in 12.5s
  /var/tmp/bazel/execroot/__main__/bazel-out/k8-fastbuild/testlogs/lte/gateway/c/core/oai/test/spgw_task/spgw_procedures_session_test/test.log
//lte/gateway/c/core/oai/test/spgw_task:spgw_procedures_test             FAILED in 15.6s
  /var/tmp/bazel/execroot/__main__/bazel-out/k8-fastbuild/testlogs/lte/gateway/c/core/oai/test/spgw_task/spgw_procedures_test/test.log
//lte/gateway/c/core/oai/test/spgw_task:spgw_procedures_with_injected_state_test FAILED in 3 out of 3 in 21.6s
  Stats over 3 runs: max = 21.6s, min = 21.5s, avg = 21.6s, dev = 0.0s

Please make sure that they are passing with bazel test --config=asan and bazel test --config=production or --config=lsan.

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
…ad/magma into 11191_spgw_state_ctxt_changes

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

@LKreutzer LKreutzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bazel changes lgtm

@nstng
Copy link
Contributor

nstng commented Jan 12, 2023

Will review after #14782 is merged and PR is rebased.

@panyogesh
Copy link
Contributor

@rsarwad : I hope this PR is not required. Can we close it. Idea is to clean up list for upcoming release.

@panyogesh
Copy link
Contributor

@rsarwad : We are planning to mark this as closed. I hope it should be fine.

@Sathyaj27
Copy link
Contributor

Closing this PR

@Sathyaj27 Sathyaj27 closed this Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: agw Access gateway-related issue component: orc8r Orchestrator-related issue size/XXL Denotes a Pull Request that changes 1000+ lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants