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(amf): Re-Attempt Authentication during subscriberdb lock #11454

Conversation

RahulKalsangra
Copy link
Contributor

@RahulKalsangra RahulKalsangra commented Feb 8, 2022

Signed-off-by: RahulKalsangra rahul.kalsangra@wavelabs.ai

Summary

AMF should re-attempt authentication when subscriberdb returns Server too busy message

Fix

  • Added configurable parameter for re-trying the authentication during lock
  • Introduced a new function amf_authentication_request_sent which can be called from
    regular and re-attempted authentication
  • After configurable attempts authentication is rejected.

Test Plan

Added new UT case : TestAuthFailureFromSubscribeDbLock

Tested on ueransim with help of # 11438
Screenshot from 2022-02-21 12-17-54

Additional Information

  • This change is backwards-breaking

@pull-request-size pull-request-size bot added the size/L Denotes a Pull Request that changes 100-499 lines. label Feb 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 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 Feb 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2022

feg-workflow

    2 files  202 suites   32s ⏱️
362 tests 362 ✔️ 0 💤 0
376 runs  376 ✔️ 0 💤 0

Results for commit ac46c8f.

♻️ This comment has been updated with latest results.

@RahulKalsangra RahulKalsangra force-pushed the RahulKalsangra/tpc/authenticationprocedure branch from fb0d54c to 1f3b25d Compare February 8, 2022 08:09
@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2022

agw-workflow

  60 files    87 suites   4m 47s ⏱️
970 tests 961 ✔️ 9 💤 0
971 runs  962 ✔️ 9 💤 0

Results for commit ac46c8f.

♻️ This comment has been updated with latest results.

@RahulKalsangra RahulKalsangra force-pushed the RahulKalsangra/tpc/authenticationprocedure branch from 1f3b25d to 45e464c Compare February 8, 2022 10:51
@RahulKalsangra RahulKalsangra force-pushed the RahulKalsangra/tpc/authenticationprocedure branch 4 times, most recently from 9e07ed1 to d592bd4 Compare February 14, 2022 14:27
@github-actions
Copy link
Contributor

github-actions bot commented Feb 14, 2022

Oops! Looks like you failed the DCO check. Be sure to sign all your commits.

Howto

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

@RahulKalsangra RahulKalsangra force-pushed the RahulKalsangra/tpc/authenticationprocedure branch 2 times, most recently from d592bd4 to 05ebfdc Compare February 16, 2022 17:21
@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2022

Oops! Looks like you failed the Python Format Check.

Howto

♻️ Updated: ✅ The check is passing the Python Format Check after the last commit.

@RahulKalsangra RahulKalsangra force-pushed the RahulKalsangra/tpc/authenticationprocedure branch from 05ebfdc to d89f6aa Compare February 16, 2022 17:32
@RahulKalsangra RahulKalsangra marked this pull request as ready for review February 17, 2022 04:39
@RahulKalsangra RahulKalsangra requested review from a team February 17, 2022 04:39
@RahulKalsangra RahulKalsangra requested a review from a team as a code owner February 17, 2022 04:39
@sebathomas
Copy link
Contributor

You can run ./precommit.py --format --diff in lte/gateway/python to fix the Python formatting errors.

@RahulKalsangra RahulKalsangra force-pushed the RahulKalsangra/tpc/authenticationprocedure branch from d89f6aa to 469227b Compare February 21, 2022 06:51
@RahulKalsangra RahulKalsangra force-pushed the RahulKalsangra/tpc/authenticationprocedure branch from 469227b to db7f562 Compare February 22, 2022 11:05
@RahulKalsangra RahulKalsangra force-pushed the RahulKalsangra/tpc/authenticationprocedure branch from db7f562 to 0ee2500 Compare March 2, 2022 09:09
@RahulKalsangra RahulKalsangra force-pushed the RahulKalsangra/tpc/authenticationprocedure branch 2 times, most recently from 17ab7d0 to 1b098d0 Compare March 16, 2022 16:18
Copy link
Contributor

@sebathomas sebathomas left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes, Python code looks good to me. 👍

Copy link
Contributor

@ardzoht ardzoht left a comment

Choose a reason for hiding this comment

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

lgtm

@RahulKalsangra RahulKalsangra force-pushed the RahulKalsangra/tpc/authenticationprocedure branch from 1b098d0 to b4bd50c Compare March 17, 2022 04:00
@RahulKalsangra RahulKalsangra added ready2merge This PR is ready to be merged (is approved and passes all required checks) product: 5g sa labels Mar 17, 2022
@sebathomas
Copy link
Contributor

sebathomas commented Mar 17, 2022

I think the semantic PR check does not know "amf". This will probably be ready to merge if you rename it to "fix(mme)" or "fix(agw)".

@RahulKalsangra RahulKalsangra removed the ready2merge This PR is ready to be merged (is approved and passes all required checks) label Mar 17, 2022
@sebathomas
Copy link
Contributor

Sorry, I may have been wrong about that. Seems the semantic PR check is broken in general at the moment, see:
#12172

@RahulKalsangra
Copy link
Contributor Author

Sorry, I may have been wrong about that. Seems the semantic PR check is broken in general at the moment, see: #12172

thanks for the information.

@sebathomas
Copy link
Contributor

I think the problem is fixed now, you just have to rebase on the current master.

@RahulKalsangra RahulKalsangra force-pushed the RahulKalsangra/tpc/authenticationprocedure branch from b4bd50c to ac0cab8 Compare March 17, 2022 14:34
… during authentication

Signed-off-by: RahulKalsangra <rahul.kalsangra@wavelabs.ai>
Signed-off-by: RahulKalsangra <rahul.kalsangra@wavelabs.ai>
Signed-off-by: RahulKalsangra <rahul.kalsangra@wavelabs.ai>
@RahulKalsangra RahulKalsangra force-pushed the RahulKalsangra/tpc/authenticationprocedure branch from ac0cab8 to cfe23f2 Compare March 17, 2022 15:52
@RahulKalsangra RahulKalsangra added the ready2merge This PR is ready to be merged (is approved and passes all required checks) label Mar 17, 2022
@sebathomas sebathomas merged commit 767eaab into magma:master Mar 18, 2022
vikramgreddy pushed a commit to vikramgreddy/magma that referenced this pull request Mar 23, 2022
…1454)

* fix(amf): handle and proper response the error code from subscriberdb during authentication

Signed-off-by: RahulKalsangra <rahul.kalsangra@wavelabs.ai>

* Addressed review comment (R1)

Signed-off-by: RahulKalsangra <rahul.kalsangra@wavelabs.ai>

* Addressed review comment (R2)

Signed-off-by: RahulKalsangra <rahul.kalsangra@wavelabs.ai>
@RahulKalsangra RahulKalsangra deleted the RahulKalsangra/tpc/authenticationprocedure branch March 25, 2022 06:27
@RahulKalsangra RahulKalsangra restored the RahulKalsangra/tpc/authenticationprocedure branch March 25, 2022 06:29
ardzoht pushed a commit that referenced this pull request Mar 30, 2022
* fix(amf): handle and proper response the error code from subscriberdb during authentication

Signed-off-by: RahulKalsangra <rahul.kalsangra@wavelabs.ai>

* Addressed review comment (R1)

Signed-off-by: RahulKalsangra <rahul.kalsangra@wavelabs.ai>

* Addressed review comment (R2)

Signed-off-by: RahulKalsangra <rahul.kalsangra@wavelabs.ai>
sreedharkumartn added a commit to sreedharkumartn/magma that referenced this pull request Apr 12, 2022
…1454)

Signed-off-by: sreedharkumartn <sreedhar.kumar@wavelabs.ai>
@panyogesh panyogesh added the backport-v1.7 MME fixes to be backported for 1.7 label Apr 25, 2022
emakeev pushed a commit to emakeev/magma that referenced this pull request Aug 5, 2022
…1454)

* fix(amf): handle and proper response the error code from subscriberdb during authentication

Signed-off-by: RahulKalsangra <rahul.kalsangra@wavelabs.ai>

* Addressed review comment (R1)

Signed-off-by: RahulKalsangra <rahul.kalsangra@wavelabs.ai>

* Addressed review comment (R2)

Signed-off-by: RahulKalsangra <rahul.kalsangra@wavelabs.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v1.7 MME fixes to be backported for 1.7 component: agw Access gateway-related issue product: 5g sa ready2merge This PR is ready to be merged (is approved and passes all required checks) 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

7 participants