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): Fix mobility updating registration #15403

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

brunohcfaria
Copy link
Collaborator

Summary

This PR fixes the Mobility Updating Registration Request reported in #1576 PR. which is marked as Draft and still had problems with duplicate sessions.

In summary, the AMF was not handling the Registration Request with Mobility Updating Type. That issue was observed in laboratory with SIMCOM devices. The test case was moving out of coverage and going back while running an iperf session. It was observed that when UE restablished the RRC Connection with the gNB it was resuming the NAS signaling by sending a Registration Request with type Mobility Updating. The AMF was unable to handle the procedure correctly leading to disconnection of the user.

Test Plan

The fix was tested with the same test-case presented above. The UE was moved out of coverage and it comes back which triggered the Mobility Updating procedure. The iperf session and pings where reestablished every time confirming the data session was kept.

fix_mobility_updating_registration_type_screenshot

Additional Information

  • This change is backwards-breaking

Security Considerations

@brunohcfaria brunohcfaria requested a review from a team as a code owner April 8, 2024 14:24
@brunohcfaria brunohcfaria requested a review from maxhbr April 8, 2024 14:24
@pull-request-size pull-request-size bot added the size/L Denotes a Pull Request that changes 100-499 lines. label Apr 8, 2024
Copy link
Contributor

github-actions bot commented Apr 8, 2024

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 Apr 8, 2024
msg->m5gs_mobile_identity.mobile_identity.imsi.type_of_identity);
if (msg->m5gs_mobile_identity.mobile_identity.guti.type_of_identity ==
M5GSMobileIdentityMsg_GUTI && !(is_amf_ctx_new)) {

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 🐶
Redundant blank line at the start of a code block should be deleted. [whitespace/blank_line] [2]


OAILOG_DEBUG(LOG_NAS_AMF,
"In process of registration mobility update: "
"GUAMFI: " PLMN_FMT".%u.%u.%u - "
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 🐶
Line ends in whitespace. Consider deleting these extra spaces. [whitespace/end_of_line] [4]

@@ -927,6 +927,8 @@ int ngap_amf_handle_ue_context_release_request(ngap_state_t* state,
asn_INTEGER2ulong(&ie->value.choice.AMF_UE_NGAP_ID,
(uint64_t*)&amf_ue_ngap_id);
} else {
OAILOG_ERROR(LOG_NGAP,
"UE CONTEXT RELEASE REQUEST invoked with missing Ngap_UEContextReleaseRequest_IEs (AMF_UE_NGAP_ID). Unable to process the message.\n");
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 🐶
Lines should be <= 120 characters long [whitespace/line_length] [2]

lte/gateway/c/core/oai/tasks/ngap/ngap_amf_handlers.c Outdated Show resolved Hide resolved
lte/gateway/c/core/oai/tasks/ngap/ngap_amf_handlers.c Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Apr 8, 2024

FeG Lint & Test

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

Results for commit 56b5fe4.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Apr 8, 2024

DP Lint & Test

14 tests   14 ✔️  2m 12s ⏱️
  1 suites    0 💤
  1 files      0

Results for commit 56b5fe4.

♻️ This comment has been updated with latest results.

@panyogesh
Copy link
Contributor

@brunohcfaria: Can you please fix the lint errors. Fixes are given in - https://github.com/magma/magma/actions/runs/8601794520/job/23569853806?pr=15403

@brunohcfaria brunohcfaria force-pushed the fix_mobility_updating_registration branch 2 times, most recently from b10d0b9 to e35f8ed Compare April 12, 2024 13:04
@Krishnavamsi-wavelabs
Copy link
Contributor

Hi @brunohcfaria, It appears that the required CI jobs are failing, primarily due to a failing unit test case. Could you please address the issues with the test case.
Screenshot from 2024-04-17 12-06-42

@brunohcfaria brunohcfaria requested review from a team as code owners April 22, 2024 21:09
@pull-request-size pull-request-size bot added size/XL Denotes a Pull Request that changes 500-999 lines. and removed size/L Denotes a Pull Request that changes 100-499 lines. labels Apr 22, 2024
@github-actions github-actions bot added component: ci All updates on CI (Jenkins/CircleCi/Github Action) component: nms NMS-related issue labels Apr 22, 2024
Copy link
Contributor

github-actions bot commented Apr 22, 2024

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

Howto

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

@brunohcfaria brunohcfaria force-pushed the fix_mobility_updating_registration branch from 346fe7e to 34d5fb0 Compare April 22, 2024 21:11
@pull-request-size pull-request-size bot added size/L Denotes a Pull Request that changes 100-499 lines. and removed size/XL Denotes a Pull Request that changes 500-999 lines. labels Apr 22, 2024
@github-actions github-actions bot removed component: nms NMS-related issue component: ci All updates on CI (Jenkins/CircleCi/Github Action) labels Apr 22, 2024
    Fixes a bug where the Registration Request with Mobility
    Registration Updating type was handled as a new registration,
    causing duplicate sessions in AMF and SMF.

Signed-off-by: Bruno Faria <bruno.faria@ektrum.com>
    Fixes a bug where the Registration Request with Mobility
    Registration Updating type was handled as a new registration,
    causing duplicate sessions in AMF and SMF.

Signed-off-by: Bruno Faria <bruno.faria@ektrum.com>
…Periodic

Registration

Modified the tests to not expect Identity Procedure when the TMSI
is matched in guti hashtable.

Signed-off-by: Bruno Faria <bruno.faria@ektrum.com>
@panyogesh panyogesh force-pushed the fix_mobility_updating_registration branch from 34d5fb0 to 56b5fe4 Compare May 6, 2024 04:13
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

3 participants