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.

Sign up for GitHub

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

Jump to bottom

fix(amf): PDU-Session accept message coming with extra buffer causing failure of PDU-Session #13103

Merged

panyogesh merged 4 commits into magma:master from priya-wavelabs:topic/priya/pdu_fix

Jul 7, 2022

Contributor

priya-wavelabs commented Jun 28, 2022 •

edited

Signed-off-by: priya-wavelabs priya.agrawal@wavelabs.ai

Summary

PDU-Session accept message coming with extra buffer causing failure of PDU-Session . With other gnb simulators we are not seeing this issue.

Test Plan

Verified with unit test:

Basic sanity with ueransim.
Pcap:

MME Log:

Additional Information

This change is backwards-breaking

priya-wavelabs self-assigned this

pull-request-size bot added the size/S label

Contributor

github-actions bot commented Jun 28, 2022

Thanks for opening a PR! 💯

A couple initial guidelines

All commits must be signed off. This is enforced by DCO check.
All PR titles must follow the semantic commits format. This is enforced by Semantic PR.

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

Contributing Conventions for norms around contributed code

If this is your first Magma PR, also consider reading

Developer Onboarding for onboarding as a new Magma developer
Development Workflow for guidance on your first pull request
CI Checks for points of contact for failing or flaky CI checks
GitHub-to-Slack mappings for Magma maintainers for guidance on how to contact maintainers on Slack

github-actions bot added the component: agw label

github-actions bot reviewed

View reviewed changes

lte/gateway/c/core/oai/tasks/ngap/ngap_amf_decoder.c Outdated

+                  }
+                  dec_ret = aper_decode(
+                      NULL, &asn_DEF_Ngap_NGAP_PDU, (void**) &pdu, bdata(raw), blength(raw),

Contributor

github-actions bot Jun 28, 2022

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

Contributor

github-actions bot commented Jun 28, 2022 •

edited

feg-workflow

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

Results for commit e81ad04.

♻️ This comment has been updated with latest results.

Contributor

github-actions bot commented Jun 28, 2022 •

edited

dp-workflow

18 tests 18 ✔️ 5m 29s ⏱️
  2 suites   0 💤
  2 files   0 ❌

Results for commit e81ad04.

♻️ This comment has been updated with latest results.

Contributor

github-actions bot commented Jun 28, 2022 •

edited

agw-workflow

580 tests 576 ✔️ 3m 50s ⏱️
    2 suites     4 💤
    2 files     0 ❌

Results for commit e81ad04.

♻️ This comment has been updated with latest results.

priya-wavelabs force-pushed the topic/priya/pdu_fix branch from 15f0815 to 6392f24 Compare

June 28, 2022 13:28

pull-request-size bot added size/M and removed size/S labels

github-actions bot reviewed

View reviewed changes

lte/gateway/c/core/oai/tasks/ngap/ngap_amf_nas_procedures.c Outdated

                   ngap_pdusession_setup_item_ies->pDUSessionResourceSetupRequestTransfer.buf =
-                      (uint8_t*)calloc(er.encoded, sizeof(uint8_t));
+                      (uint8_t*)calloc(((er.encoded + 7) >> 3), sizeof(uint8_t));

Contributor

github-actions bot Jun 28, 2022

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

lte/gateway/c/core/oai/tasks/ngap/ngap_amf_nas_procedures.c

                   memcpy((void*)ngap_pdusession_setup_item_ies
                              ->pDUSessionResourceSetupRequestTransfer.buf,
-                         (void*)transfer->data, er.encoded);
+                         (void*)transfer->data, blength(transfer));

Contributor

github-actions bot Jun 28, 2022

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

priya-wavelabs force-pushed the topic/priya/pdu_fix branch from 6392f24 to 1db1b40 Compare

June 28, 2022 14:14

pull-request-size bot added size/S and removed size/M labels

priya-wavelabs force-pushed the topic/priya/pdu_fix branch from 1db1b40 to 5681bee Compare

June 29, 2022 05:27

priya-wavelabs marked this pull request as ready for review

June 29, 2022 06:34

priya-wavelabs requested review from a team and panyogesh

June 29, 2022 06:34

ganeshg87 approved these changes

View reviewed changes

Contributor

ganeshg87 left a comment

lgtm

priya-wavelabs added type: bug priority: medium product: 5g sa labels

rsarwad reviewed

View reviewed changes

lte/gateway/c/core/oai/tasks/ngap/ngap_amf_nas_procedures.c Outdated Show resolved Hide resolved

rsarwad reviewed

View reviewed changes

lte/gateway/c/core/oai/tasks/ngap/ngap_amf_nas_procedures.c Outdated Show resolved Hide resolved

rsarwad reviewed

View reviewed changes

lte/gateway/c/core/oai/tasks/ngap/ngap_amf_nas_procedures.c Outdated Show resolved Hide resolved

priya-wavelabs force-pushed the topic/priya/pdu_fix branch from 5681bee to 524a718 Compare

June 30, 2022 06:29

ganeshg87 requested a review from rsarwad

June 30, 2022 06:40

priya-wavelabs force-pushed the topic/priya/pdu_fix branch from 524a718 to b85f96b Compare

July 5, 2022 10:07

rsarwad reviewed

View reviewed changes

lte/gateway/c/core/oai/tasks/ngap/ngap_amf_nas_procedures.c Outdated Show resolved Hide resolved

rsarwad reviewed

View reviewed changes

lte/gateway/c/core/oai/tasks/ngap/ngap_amf_nas_procedures.c Show resolved Hide resolved

priya-wavelabs added 3 commits

July 7, 2022 05:21


          fix(amf): PDU-Session accept message coming with extra buffer causing…

2c6e68d

… failure of PDU-Session

Signed-off-by: priya-wavelabs <priya.agrawal@wavelabs.ai>


          Addressed review comments (R1)

94ea839

Signed-off-by: priya-wavelabs <priya.agrawal@wavelabs.ai>


          Addressed review comments (R2)

24297b6

Signed-off-by: priya-wavelabs <priya.agrawal@wavelabs.ai>

priya-wavelabs force-pushed the topic/priya/pdu_fix branch from b85f96b to 5055f0c Compare

July 7, 2022 05:22

pull-request-size bot added size/M and removed size/S labels

github-actions bot reviewed

View reviewed changes

lte/gateway/c/core/oai/tasks/ngap/ngap_amf_nas_procedures.c Outdated

                     session_context->pDUSessionResourceSetupRequestTransfer.buf =
-                        (uint8_t*)calloc(er.encoded, sizeof(uint8_t));
+                        (uint8_t*)calloc(((er.encoded + 7) >> 3), sizeof(uint8_t));

Contributor

github-actions bot Jul 7, 2022

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

lte/gateway/c/core/oai/tasks/ngap/ngap_amf_nas_procedures.c

                     memcpy((void*)session_context->pDUSessionResourceSetupRequestTransfer.buf,
-                           (void*)transfer->data, er.encoded);
+                           (void*)transfer->data, blength(transfer));

Contributor

github-actions bot Jul 7, 2022

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

priya-wavelabs force-pushed the topic/priya/pdu_fix branch from 5055f0c to 0619ad6 Compare

July 7, 2022 05:41

priya-wavelabs requested a review from rsarwad

July 7, 2022 05:42

github-actions bot reviewed

View reviewed changes

lte/gateway/c/core/oai/tasks/ngap/ngap_amf_nas_procedures.c Outdated

-                    bstring transfer = blk2bstr(buffer, er.encoded);
+                               &pduSessionResourceSetupRequestTransferIEs);
+                    uint32_t encoded_bytes = NGAP_ASN_ENCODED_BYTES(er.encoded);

Contributor

github-actions bot Jul 7, 2022

[cpplint] _{reported by reviewdog 🐶}
Line ends in whitespace. Consider deleting these extra spaces. [whitespace/end_of_line] [4]

lte/gateway/c/core/oai/tasks/ngap/ngap_amf_nas_procedures.c

                     session_context->pDUSessionResourceSetupRequestTransfer.buf =
-                        (uint8_t*)calloc(er.encoded, sizeof(uint8_t));
+                        (uint8_t*)calloc(encoded_bytes, sizeof(uint8_t));

Contributor

github-actions bot Jul 7, 2022

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

lte/gateway/c/core/oai/tasks/ngap/ngap_amf_nas_procedures.c

                   ngap_pdusession_setup_item_ies->pDUSessionResourceSetupRequestTransfer.buf =
-                      (uint8_t*)calloc(er.encoded, sizeof(uint8_t));
+                      (uint8_t*)calloc(encoded_bytes, sizeof(uint8_t));

Contributor

github-actions bot Jul 7, 2022

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


          Addressed review comments (R3)

e81ad04

Signed-off-by: priya-wavelabs <priya.agrawal@wavelabs.ai>

priya-wavelabs force-pushed the topic/priya/pdu_fix branch from 0619ad6 to e81ad04 Compare

July 7, 2022 05:52

rsarwad reviewed

View reviewed changes

Contributor

rsarwad left a comment

Thanks for explanation.
lgtm

rsarwad approved these changes

View reviewed changes

Contributor

rsarwad left a comment

LGTM

panyogesh approved these changes

View reviewed changes

Contributor

panyogesh commented Jul 7, 2022

LGTM

panyogesh merged commit cac083b into magma:master

emakeev pushed a commit to emakeev/magma that referenced this pull request


          fix(amf): PDU-Session accept message coming with extra buffer causing…

e4faa5c

… failure of PDU-Session (magma#13103)

* fix(amf): PDU-Session accept message coming with extra buffer causing failure of PDU-Session

Signed-off-by: priya-wavelabs <priya.agrawal@wavelabs.ai>

* Addressed review comments (R1)

Signed-off-by: priya-wavelabs <priya.agrawal@wavelabs.ai>

* Addressed review comments (R2)

Signed-off-by: priya-wavelabs <priya.agrawal@wavelabs.ai>

* Addressed review comments (R3)

Signed-off-by: priya-wavelabs <priya.agrawal@wavelabs.ai>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment