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

[lte][agw] Support nonconsecutive PLMNS #6697

Merged
merged 4 commits into from May 5, 2021
Merged

Conversation

ulaskozat
Copy link
Contributor

@ulaskozat ulaskozat commented May 4, 2021

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

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
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
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
Signed-off-by: Ulas Kozat <kozat@fb.com>
Signed-off-by: Ulas Kozat <kozat@fb.com>
Signed-off-by: Ulas Kozat <kozat@fb.com>
Signed-off-by: Ulas Kozat <kozat@fb.com>
themarwhal pushed a commit that referenced this pull request May 6, 2021
* 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
themarwhal pushed a commit that referenced this pull request May 7, 2021
* 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
* 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
* 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
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
Development

Successfully merging this pull request may close these issues.

Non-consecutive TAI list support
5 participants