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 port hash list, temporary_create_session_procedure_id to protobuf map #14005

Merged
merged 7 commits into from
Nov 10, 2022

Conversation

rsarwad
Copy link
Contributor

@rsarwad rsarwad commented Sep 23, 2022

Summary

fix(agw): Modified sgw_s8 code to port hash list, temporary_create_session_procedure_id to protobuf map

Test Plan

Executed only unit test suite. For in-bound roaming, End2End scenarios used to be tested in Tera VM setup. Since we are not using the Tera VM setup. We couldn't validate End2End scenario.

Additional Information

Once federated integ_test supports mocked external PGW, we can validate the End2End scenarios for inbound roaming UE

…edure_id to protobuf map

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
@rsarwad rsarwad requested review from a team and ssanadhya September 23, 2022 13:30
@pull-request-size pull-request-size bot added the size/L Denotes a Pull Request that changes 100-499 lines. label Sep 23, 2022
@github-actions
Copy link
Contributor

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 the component: agw Access gateway-related issue label Sep 23, 2022
}
free_wrapper((void**)sgw_eps_context);
free_wrapper((void**)ptr);
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]

@github-actions
Copy link
Contributor

github-actions bot commented Sep 23, 2022

feg-workflow

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

Results for commit 6fafce5.

♻️ This comment has been updated with latest results.

}
}
state_cache_p->temporary_create_session_procedure_id_map.map = nullptr;
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]

@github-actions
Copy link
Contributor

github-actions bot commented Sep 23, 2022

dp-workflow

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

Results for commit 6fafce5.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 23, 2022

agw-workflow

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

Results for commit 6fafce5.

♻️ This comment has been updated with latest results.

@@ -170,15 +170,15 @@ struct proto_map_s {
** the map. If key does not exists returns error **
** **
***************************************************************************/
proto_map_rc_t remove(const keyT key) {
proto_map_rc_t remove(const keyT key, bool free_an_entry = true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add the second parameter in the description?

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

@pruthvihebbani pruthvihebbani left a comment

Choose a reason for hiding this comment

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

LGTM

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

@ssanadhya ssanadhya left a comment

Choose a reason for hiding this comment

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

LGTM, with minor comments

** the map. If key does not exists returns error **
** Description: Takes key parameter.Looks up the corresponding entry from **
** the map. **
** By default the argument, free_an_entry is set to true. **
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
** By default the argument, free_an_entry is set to true. **
** By default, the argument free_an_entry is set to true. **

Comment on lines 104 to 105
// Map- Key: csr_proc_id of uint32_t , Data:
// sgw_eps_bearer_context_information_s*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Map- Key: csr_proc_id of uint32_t , Data:
// sgw_eps_bearer_context_information_s*
// Map with Key: csr_proc_id of uint32_t
// Data: sgw_eps_bearer_context_information_s*

Comment on lines 92 to 98
if (!(state_cache_p->temporary_create_session_procedure_id_map.map)) {
OAILOG_CRITICAL(
LOG_SGW_S8,
"Failed to create temporary_create_session_procedure_id_htbl for "
"Failed to create temporary_create_session_procedure_id_map for "
"SGW_S8 task \n");
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can get rid of this check since 'new' would raise the bad_alloc exception if memory allocation fails, terminating the process.

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
@rsarwad rsarwad merged commit f274cf5 into magma:master Nov 10, 2022
lucasgonze pushed a commit to lucasgonze/magma that referenced this pull request Feb 29, 2024
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 size/L Denotes a Pull Request that changes 100-499 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants