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): Replaced the key for guti hashtable, currently the key is GUTI in string format #12653

Merged
merged 3 commits into from Jun 1, 2022

Conversation

rsarwad
Copy link
Contributor

@rsarwad rsarwad commented May 9, 2022

Summary

Guti hash table is of object hash table, for which earlier key was pointer to object of structure, guti_t. As part of this PR, we changed the key as string format of GUTI.
Partially fixes the issue, #11189
The other part would be fixed by PR, #12573

Test Plan

  1. Executed s1ap sanity test suite and manually verified the GUTI values with mme restart, so that GUTI is correctly written to DB and fetched from DB
  2. Executed unit test cases.

@rsarwad rsarwad requested review from a team and ssanadhya May 9, 2022 11:31
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines. label May 9, 2022
@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2022

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 May 9, 2022
@@ -757,4 +757,5 @@ bstring paa_to_bstring(const paa_t* paa);
// Return the hex representation of a char array
char* bytes_to_hex(char* byte_array, int length, char* hex_array);

void convert_guti_to_string(guti_t* guti_p, char (*guti_str)[]);
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 🐶
Extra space before ( in function call [whitespace/parens] [4]

(*key_array_p)[i], *((*key_list)[i]));
hashtable_rc_t ht_rc = obj_hashtable_uint64_ts_get(
guti_htbl, (const void*)(*key_array_p)[i], sizeof(guti_t), &mme_ue_id);
std::string guti_str((char*)(*key_array_p)[i], (GUTI_STRING_LEN - 1));
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<char*>(...) instead [readability/casting] [4]

@@ -180,9 +179,10 @@ void MmeNasStateConverter::proto_to_guti_table(
std::unique_ptr<guti_t> guti = std::make_unique<guti_t>();
memset(guti.get(), 0, sizeof(guti_t));

mme_app_convert_string_to_guti(guti.get(), kv.first);
char guti_str[GUTI_STRING_LEN] = {};
strcpy(guti_str, (char*)(kv.first.c_str()));
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<char*>(...) instead [readability/casting] [4]

@@ -180,9 +179,10 @@ void MmeNasStateConverter::proto_to_guti_table(
std::unique_ptr<guti_t> guti = std::make_unique<guti_t>();
memset(guti.get(), 0, sizeof(guti_t));

mme_app_convert_string_to_guti(guti.get(), kv.first);
char guti_str[GUTI_STRING_LEN] = {};
strcpy(guti_str, (char*)(kv.first.c_str()));
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 🐶
Almost always, snprintf is better than strcpy [runtime/printf] [4]

@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2022

feg-workflow

    2 files  202 suites   38s ⏱️
371 tests 371 ✔️ 0 💤 0
385 runs  385 ✔️ 0 💤 0

Results for commit 7a6e558.

♻️ This comment has been updated with latest results.

@rsarwad rsarwad force-pushed the rsarwad_lte_guti_hashtable_conversion branch from f2dbcf1 to 9b89ba4 Compare May 9, 2022 12:02
@pull-request-size 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 May 9, 2022
@rsarwad rsarwad force-pushed the rsarwad_lte_guti_hashtable_conversion branch from 9b89ba4 to 26044c2 Compare May 9, 2022 12:43
@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2022

agw-workflow

     77 files     122 suites   7m 28s ⏱️
1 151 tests 1 147 ✔️ 4 💤 0
1 152 runs  1 148 ✔️ 4 💤 0

Results for commit 7a6e558.

♻️ This comment has been updated with latest results.

@rsarwad rsarwad force-pushed the rsarwad_lte_guti_hashtable_conversion branch from 26044c2 to 971cc36 Compare May 10, 2022 06:33
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 change

@@ -367,3 +367,12 @@ char* bytes_to_hex(char* byte_array, int length, char* hex_array) {
}
return hex_array;
}

void convert_guti_to_string(guti_t* guti_p, char (*guti_str)[]) {
#define GUTI_STRING_LEN 21
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is already defined in conversions.h below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you.

@@ -541,8 +541,6 @@ struct emm_context_s* emm_context_get(emm_data_t* emm_data,
const mme_ue_s1ap_id_t ue_id);
struct emm_context_s* emm_context_get_by_imsi(emm_data_t* emm_data,
imsi64_t imsi64);
struct emm_context_s* emm_context_get_by_guti(emm_data_t* emm_data,
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@@ -662,26 +662,6 @@ struct emm_context_s* emm_context_get_by_imsi(
return emm_context_p;
}

//------------------------------------------------------------------------------
struct emm_context_s* emm_context_get_by_guti(emm_data_t* emm_data,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the clean up!

@rsarwad rsarwad force-pushed the rsarwad_lte_guti_hashtable_conversion branch from 971cc36 to 9ea686e Compare May 27, 2022 10:35
@pull-request-size pull-request-size bot added size/XS Denotes a PR that changes 0-9 lines. and removed size/L Denotes a Pull Request that changes 100-499 lines. labels May 27, 2022
@github-actions
Copy link
Contributor

github-actions bot commented May 27, 2022

Oops! Looks like you failed the Semantic PR check.

Howto

♻️ Updated: ✅ The check is passing the Semantic PR after the last commit.

@magma magma deleted a comment from github-actions bot May 27, 2022
@github-actions
Copy link
Contributor

github-actions bot commented May 27, 2022

dp-workflow

  2 files    2 suites   3m 15s ⏱️
15 tests 15 ✔️ 0 💤 0

Results for commit 7a6e558.

♻️ This comment has been updated with latest results.

@rsarwad rsarwad force-pushed the rsarwad_lte_guti_hashtable_conversion branch from 9ea686e to 369e46e Compare May 27, 2022 14:59
@pull-request-size pull-request-size bot added size/L Denotes a Pull Request that changes 100-499 lines. and removed size/XS Denotes a PR that changes 0-9 lines. labels May 27, 2022
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
@@ -367,3 +367,11 @@ char* bytes_to_hex(char* byte_array, int length, char* hex_array) {
}
return hex_array;
}

void convert_guti_to_string(guti_t* guti_p, char (*guti_str)[]) {
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 🐶
Extra space before ( in function call [whitespace/parens] [4]

…hashtable_conversion

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
@rsarwad rsarwad merged commit 2fda7a8 into magma:master Jun 1, 2022
emakeev pushed a commit to emakeev/magma that referenced this pull request Aug 5, 2022
…UTI in string format (magma#12653)

* Modified code to use guti in string format

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

* Fixed OAI build failure

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
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

2 participants