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 to port UE context to protobuf structure at spgw task #14782

Closed
wants to merge 35 commits into from

Conversation

rsarwad
Copy link
Contributor

@rsarwad rsarwad commented Jan 2, 2023

Summary

agw(fix): Modified code to port UE context to protobuf structure at spgw task

Test Plan

Executed s1ap sanity test suite and unit test cases

… protobuf structure

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

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
… protobuf structure, S11BearerContext

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

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

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

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

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
…rwad/magma into 11191_spgw_teid_ctxt_changes

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

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

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

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

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

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
…rwad/magma into 11191_spgw_ue_ctxt_changes

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
@rsarwad rsarwad requested review from a team January 2, 2023 08:24
@github-actions
Copy link
Contributor

github-actions bot commented Jan 2, 2023

✔️ The Semantic PR check ended with status success. See instructions on formatting your commit and pull request titles.

pco_req.protocol_or_container_ids[0].id =
PCO_CI_IP_ADDRESS_ALLOCATION_VIA_NAS_SIGNALLING;
magma::lte::oai::PcoProtocol* pco_protocol = pco_req.add_pco_protocol();
pco_protocol->set_id(PCO_CI_IP_ADDRESS_ALLOCATION_VIA_NAS_SIGNALLING);
Copy link
Contributor

Choose a reason for hiding this comment

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

[misspell] reported by reviewdog 🐶
"SIGNALLING" is a misspelling of "SIGNALING"

typedef magma::proto_map_s<uint32_t,
struct s_plus_p_gw_eps_bearer_context_information_s*>
// Data: S11BearerContext*
// TODO (rsarwad): rename S11BearerContext to SpgwSessionContext and also
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 🐶
Missing username in TODO; it should look like "// TODO(my_username): Stuff." [readability/todo] [2]

(proto_ip->pdn_type() == IPv6 || proto_ip->pdn_type() == IPv4_AND_v6)) {
inet_pton(AF_INET6, proto_ip->ipv6_addr().c_str(), ipv6);
}
}
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 🐶
Closing brace should be aligned with beginning of struct in_addr [whitespace/indent] [3]

if (procedures != nullptr) {
pgw_base_proc_t* base_proc = nullptr;
proto->set_teid(session_request->teid);
proto->set_imsi((char*)session_request->imsi.digit);
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]

break;
default:
break;
};
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 🐶
You don't need a ; after a } [readability/braces] [4]

eps_bearer_ctxt->s_gw_teid_S5_S8_up);
}

// TODO Need to add handling on failing to delete s1-u tunnel rules from
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 🐶
Missing username in TODO; it should look like "// TODO(my_username): Stuff." [readability/todo] [2]

@rsarwad rsarwad changed the title agw(fix): Modified code to port UE context to protobuf structure at spgw task fix(agw): Modified code to port UE context to protobuf structure at spgw task Jan 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jan 2, 2023

FeG Lint & Test

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

Results for commit b73040c.

♻️ This comment has been updated with latest results.

@rsarwad rsarwad marked this pull request as draft January 2, 2023 08:33
@github-actions
Copy link
Contributor

github-actions bot commented Jan 2, 2023

DP Lint & Test

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

Results for commit b73040c.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 2, 2023

Orc8r Lint & Test

1 135 tests   1 135 ✔️  2m 13s ⏱️
   365 suites         0 💤
       7 files           0

Results for commit b73040c.

♻️ This comment has been updated with latest results.

…txt_changes

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

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
…rwad/magma into 11191_spgw_ue_ctxt_changes

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
…rwad/magma into 11191_spgw_ue_ctxt_changes

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
@rsarwad rsarwad marked this pull request as ready for review January 6, 2023 05:17
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
@nstng
Copy link
Contributor

nstng commented Jan 6, 2023

@rsarwad, what is the difference to #14810? The changes seem overlapping - what is the plan for merging here?

The Bazel changes look good in general, but it looks to me there are a couple of memory leaks detected in the tests - is this ready-for-review?

@rsarwad
Copy link
Contributor Author

rsarwad commented Jan 6, 2023

@rsarwad, what is the difference to #14810? The changes seem overlapping - what is the plan for merging here?

The Bazel changes look good in general, but it looks to me there are a couple of memory leaks detected in the tests - is this ready-for-review?

PR #14810 has been branched out from this PR and this PR has been branched out from #14717. But PR 14717 is blocked from getting approvals from magma/approvers-agw .
So currently the diff shows 3 PR changes. To view individual PR changes, I have created PRs in my forked repo, rsarwad#29 and rsarwad#30

Yes with restart test cases, memory leaks are observed. I shall move this PR to draft till memory leaks are fixed

@rsarwad rsarwad marked this pull request as draft January 6, 2023 15:41
…ad/magma into 11191_spgw_ue_ctxt_changes

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
@rsarwad rsarwad marked this pull request as ready for review January 10, 2023 07:20
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
@rdefosse
Copy link
Contributor

@rsarwad

I made a mistake on one of the OAI CI servers while your PR was running. Sorry. It should be OK.

@nstng
Copy link
Contributor

nstng commented Jan 10, 2023

//lte/gateway/c/core/oai/test/sgw_s8_task:sgw_dedicated_bearer_test was failing in CI - I recently noticed this one failing and succeeding after restarting the run. This is the second strike - if I see this happening again, then I will mark the test as flaky (will then be executed with a one-out-of-three policy).

Restarted the tests.

@rsarwad
Copy link
Contributor Author

rsarwad commented Jan 10, 2023

@rsarwad

I made a mistake on one of the OAI CI servers while your PR was running. Sorry. It should be OK.

No problem

@nstng
Copy link
Contributor

nstng commented Jan 12, 2023

Bazel and pipelined changes lgtm. Can you please,

  • argue here why test_spgw_state_converter.cpp was removed
  • rebase -> in order to re-trigger the oai jenkins tests

I will approve afterwards.

@panyogesh
Copy link
Contributor

@VinashakAnkitAman : I guess we can close the PR. Kindly confirm

@panyogesh
Copy link
Contributor

@VinashakAnkitAman : We are planning to close it next week. Hope it should be fine.

@Sathyaj27
Copy link
Contributor

Closing this PR

@Sathyaj27 Sathyaj27 closed this Oct 9, 2023
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 component: orc8r Orchestrator-related issue size/XXL Denotes a Pull Request that changes 1000+ lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants