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.

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

[lte][agw] Support nonconsecutive PLMNS #6697

Merged
merged 4 commits into from
May 5, 2021
Merged

[lte][agw] Support nonconsecutive PLMNS #6697

merged 4 commits into from
May 5, 2021

Conversation

ulaskozat
Copy link
Contributor

@ulaskozat ulaskozat commented May 4, 2021 •
edited
Loading

Signed-off-by: Ulas Kozat kozat@fb.com

Summary

  • Fixed bug in handling of non-consecutive TACs in TAI list, where the partial lists were not encoded correctly.
  • Refactored the code for readability and simplicity.
  • Fixed a bug that was not swapping the MNC length during sorting TAIs.

PR limits the TAI list communicated to UE to one partial list that can have up to 16 TAIs. As per the spec TS 124.301 Table 9.9.3.33.1: Tracking area identity list information element:
"The value part of the Tracking area identity list information element consists of one or several partial tracking area identity lists. The length of each partial tracking area identity list can be determined from the 'type of list' field and the 'number of elements'
field in the first octet of the partial tracking area identity list. The UE shall store the complete list received. If more than 16 TAIs are included in this information element, the UE shall store the first 16 TAIs and ignore the remaining
octets of the information element."

Test Plan

Integ tests.

Tested with different entries for consecutive and non-consecutive tacs (by modifying attachedEnodebTacs in gateway.mconfig and restarting the services) and multi-plmn inputs (by modifying the TAC_LIST and TAI_LIST in mme.conf.template file). Checked the pcap entries.

E.g., for
"attachedEnodebTacs": [10181,10183,10185,10187,10188,10283,10284,10287,10288,10383,10384,10387,10388,10483,10487,10488,10583,10584,10587,10588,10683,10684,10687,10783,10784,10787,10883,10884,10887,10983,10984,10987,11083,11084,11183,11184,11283,11288,12888,12988,13088,13188,13288,13388,13488,13588,13684,13688,13784,13788,13884,13888,13984,13988,14084,14088,14184,14188,14284,14384,14484,14584,14684,14784]

Tracking area identity list - TAI list
    Length: 36
    0... .... = Spare bit(s): 0x00
    .00. .... = Type of list: list of TACs belonging to one PLMN, with non-consecutive TAC values (0)
    ...0 1111 = Number of elements: 15
    Mobile Country Code (MCC): Unknown (1)
    Mobile Network Code (MNC): Unknown (01)
    Tracking area code(TAC): 10181
    Tracking area code(TAC): 10183
    Tracking area code(TAC): 10185
    Tracking area code(TAC): 10187
    Tracking area code(TAC): 10188
    Tracking area code(TAC): 10283
    Tracking area code(TAC): 10284
    Tracking area code(TAC): 10287
    Tracking area code(TAC): 10288
    Tracking area code(TAC): 10383
    Tracking area code(TAC): 10384
    Tracking area code(TAC): 10387
    Tracking area code(TAC): 10388
    Tracking area code(TAC): 10483
    Tracking area code(TAC): 10487
    Tracking area code(TAC): 10488

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 May 4, 2021
@magmabot magmabot added the component: agw Access gateway-related issue label May 4, 2021
@codecov
Copy link

codecov bot commented May 4, 2021 •
edited
Loading

Codecov Report

Merging #6697 (ecec62c) into master (4d5b4c2) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

❗ Current head ecec62c differs from pull request most recent head 15a4948. Consider uploading reports for the commit 15a4948 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6697      +/-   ##
==========================================
- Coverage   33.13%   33.11%   -0.02%     
==========================================
  Files        1150     1150              
  Lines      103233   103229       -4     
  Branches     1318     1318              
==========================================
- Hits        34205    34184      -21     
- Misses      65656    65674      +18     
+ Partials     3372     3371       -1
Flag Coverage Δ
c_cpp 7.96% <0.00%> (-0.03%) ⬇️
cloud_lint 65.68% <ø> (ø)
feg-lint 56.43% <ø> (-0.13%) ⬇️
lte-test 72.67% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lte/gateway/c/oai/tasks/nas/api/mme/mme_api.c 0.00% <0.00%> (ø)
lte/gateway/c/oai/tasks/nas/emm/sap/emm_send.c 0.00% <ø> (ø)
...y/services/s8_proxy/servicers/mock_pgw/mock_pgw.go 72.41% <0.00%> (-11.59%) ⬇️
feg/gateway/services/s8_proxy/servicers/senders.go 73.52% <0.00%> (-7.24%) ⬇️
...ateway/services/s8_proxy/servicers/gtp_handlers.go 78.94% <0.00%> (-6.77%) ⬇️
feg/gateway/gtp/gtp_client.go 56.89% <0.00%> (-1.08%) ⬇️
lte/gateway/c/session_manager/StoredState.cpp 93.31% <0.00%> (-0.17%) ⬇️
lte/gateway/c/session_manager/SessionState.cpp 79.63% <0.00%> (-0.08%) ⬇️
lte/gateway/c/session_manager/Types.h 100.00% <0.00%> (ø)
feg/gateway/services/csfb/servicers/csfb.go 42.15% <0.00%> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d5b4c2...15a4948. Read the comment docs.

@themarwhal themarwhal requested review from a team, ardzoht and lionelgo and removed request for ssanadhya, ardzoht, lionelgo and a team May 4, 2021 14:52
ardzoht
ardzoht approved these changes May 4, 2021
View reviewed changes
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.

Overall lgtm, with a few comments

lte/gateway/c/oai/tasks/nas/api/mme/mme_api.c Outdated Show resolved Hide resolved
lte/gateway/c/oai/tasks/nas/api/mme/mme_api.c Outdated Show resolved Hide resolved
lte/gateway/c/oai/tasks/nas/api/mme/mme_api.c Outdated Show resolved Hide resolved
lionelgo
lionelgo approved these changes May 4, 2021
View reviewed changes
lte/gateway/c/oai/tasks/nas/api/mme/mme_api.c Outdated Show resolved Hide resolved
lte/gateway/c/oai/tasks/nas/api/mme/mme_api.c Outdated Show resolved Hide resolved
@ulaskozat ulaskozat added apply-v1.3 Needs to be applied to v1.3 release branch as well apply-v1.4 Needs to be applied to v1.4 release branch as well apply-v1.5 Apply this commit to the v1.5 release branch as well. labels May 5, 2021
@ulaskozat
Support nonconsecutive PLMNS
fb3bdf8 
Signed-off-by: Ulas Kozat <kozat@fb.com>
@ulaskozat
Addressed review comments
b2159ef 
Signed-off-by: Ulas Kozat <kozat@fb.com>
@ulaskozat
Addressed review comments
cb45391 
Signed-off-by: Ulas Kozat <kozat@fb.com>
@ulaskozat
Fix bug in TAI sorting, limit the TAI list for UE
a8478ae 
Signed-off-by: Ulas Kozat <kozat@fb.com>
@ulaskozat ulaskozat merged commit 1106092 into magma:master May 5, 2021
themarwhal pushed a commit that referenced this pull request May 6, 2021
@ulaskozat @themarwhal
[lte][agw] Support nonconsecutive PLMNS (#6697)
918b39f 
* Support nonconsecutive PLMNS

Signed-off-by: Ulas Kozat <kozat@fb.com>
@ulaskozat ulaskozat linked an issue May 6, 2021 that may be closed by this pull request
Non-consecutive TAI list support #6686
Closed
@ulaskozat ulaskozat mentioned this pull request May 6, 2021
Closed
themarwhal pushed a commit that referenced this pull request May 7, 2021
@ulaskozat @themarwhal
[lte][agw] Support nonconsecutive PLMNS (#6697)
155e33e 
* Support nonconsecutive PLMNS

Signed-off-by: Ulas Kozat <kozat@fb.com>
@themarwhal themarwhal added the backported-v1.4 Has been backported to v1.4 release branch label May 7, 2021
m-trojanowski pushed a commit to openEPC/magma that referenced this pull request Oct 20, 2021
@ulaskozat @themarwhal
[lte][agw] Support nonconsecutive PLMNS (magma#6697)
5ebe687 
* Support nonconsecutive PLMNS

Signed-off-by: Ulas Kozat <kozat@fb.com>
m-trojanowski pushed a commit to openEPC/magma that referenced this pull request Oct 20, 2021
@ulaskozat @themarwhal
[lte][agw] Support nonconsecutive PLMNS (magma#6697)
cf0af54 
* Support nonconsecutive PLMNS

Signed-off-by: Ulas Kozat <kozat@fb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lionelgo lionelgo lionelgo approved these changes

@ardzoht ardzoht ardzoht approved these changes

Assignees
No one assigned
Labels
apply-v1.3 Needs to be applied to v1.3 release branch as well apply-v1.4 Needs to be applied to v1.4 release branch as well apply-v1.5 Apply this commit to the v1.5 release branch as well. backported-v1.4 Has been backported to v1.4 release branch backported-v1.5 component: agw Access gateway-related issue size/L Denotes a Pull Request that changes 100-499 lines.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Non-consecutive TAI list support
5 participants
@ulaskozat @ardzoht @lionelgo @themarwhal @magmabot