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] Remove hardwired APN APMBR over S1 #5886

Merged
merged 3 commits into from
Apr 2, 2021

Conversation

ulaskozat
Copy link
Contributor

Summary

Two changes are done:

  • Eliminate hardwired APN AMBR information in attach accept (which was hardwired to 200 and 100Mbps for DL and UL).
  • Fix bugs in mapping QoS values

Test Plan

Modified default APN AMBR values and dedicated QoS values in S1AP tester, run test_attach_detach_dedicated.py, capture pcap and check QoS parameters for APN and bearers.

E.g., for:

magma_default_apn = {
    "apn_name": "magma.ipv4",  # APN-name
    "qci": 9,  # qci
    "priority": 15,  # priority
    "pre_cap": 1,  # preemption-capability
    "pre_vul": 0,  # preemption-vulnerability
    "mbr_ul": 96000,  # MBR UL
    "mbr_dl": 1500000,  # MBR DL
}

pcap shows (discrepancy is due to specific way of quantizing the field):

APN aggregate maximum bit rate
    Element ID: 0x5e
    Length: 2
    APN-AMBR for downlink: 1472 kbps
    APN-AMBR for uplink: 96 kbps

and for:

qos=FlowQos(
    qci=qci_val,
    gbr_ul=48000,
    gbr_dl=64000,
    max_req_bw_ul=150000,
    max_req_bw_dl=168000,
    arp=QosArp(
        priority_level=1,
        pre_capability=1,
        pre_vulnerability=0,
    ),
),

pcap shows:

gbrQosInformation
    e-RAB-MaximumBitrateDL: 168000bits/s
    e-RAB-MaximumBitrateUL: 150000bits/s
    e-RAB-GuaranteedBitrateDL: 64000bits/s
    e-RAB-GuaranteedBitrateUL: 48000bits/s
EPS quality of service
    Length: 5
    Quality of Service Class Identifier (QCI): QCI 1 (1)
    Maximum bit rate for uplink: 144 kbps
    Maximum bit rate for downlink: 168 kbps
    Guaranteed bit rate for uplink: 48 kbps
    Guaranteed bit rate for downlink: 64 kbps

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 Apr 2, 2021
@magmabot magmabot added the component: agw Access gateway-related issue label Apr 2, 2021
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.

lgtm

@codecov
Copy link

codecov bot commented Apr 2, 2021

Codecov Report

Merging #5886 (792d2f7) into master (8faf116) will increase coverage by 3.14%.
The diff coverage is 0.00%.

❗ Current head 792d2f7 differs from pull request most recent head 2f82869. Consider uploading reports for the commit 2f82869 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5886      +/-   ##
==========================================
+ Coverage   33.93%   37.07%   +3.14%     
==========================================
  Files        1127      984     -143     
  Lines       99124    89443    -9681     
  Branches     1307     1305       -2     
==========================================
- Hits        33639    33164     -475     
+ Misses      62191    52984    -9207     
- Partials     3294     3295       +1     
Flag Coverage Δ
c_cpp 8.86% <0.00%> (+0.72%) ⬆️
cloud_lint 66.33% <ø> (+0.01%) ⬆️
feg-lint 56.18% <ø> (+0.01%) ⬆️
lte-test 72.97% <ø> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
.../gateway/c/oai/tasks/mme_app/mme_app_pdn_context.c 0.00% <0.00%> (ø)
lte/gateway/c/oai/tasks/nas/emm/sap/emm_cn.c 0.00% <0.00%> (ø)
lte/gateway/c/oai/tasks/nas/esm/sap/esm_send.c 0.00% <0.00%> (ø)
...y/c/oai/tasks/nas/ies/ApnAggregateMaximumBitRate.c 0.00% <0.00%> (ø)
.../gateway/c/oai/tasks/nas/ies/EpsQualityOfService.c 0.00% <0.00%> (ø)
...8r/cloud/go/services/configurator/storage/union.go 93.54% <0.00%> (-6.46%) ⬇️
.../gateway/python/magma/subscriberdb/rpc_servicer.py 77.77% <0.00%> (-4.77%) ⬇️
...oud/go/services/state/indexer/reindex/reindexer.go 66.00% <0.00%> (-2.00%) ⬇️
lte/gateway/c/session_manager/SessionState.cpp 77.91% <0.00%> (-1.79%) ⬇️
...gateway/services/s8_proxy/servicers/mock_pgw/cs.go 54.23% <0.00%> (-0.85%) ⬇️
... and 177 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 8faf116...2f82869. Read the comment docs.

@ulaskozat ulaskozat enabled auto-merge (squash) April 2, 2021 04:04
Signed-off-by: Ulas Kozat <kozat@fb.com>
Signed-off-by: Ulas Kozat <kozat@fb.com>
Signed-off-by: Ulas Kozat <kozat@fb.com>
@ulaskozat ulaskozat merged commit 3120ec5 into magma:master Apr 2, 2021
@ulaskozat ulaskozat deleted the apnambr branch April 2, 2021 18:36
chandra-77 pushed a commit to chandra-77/magma that referenced this pull request Apr 6, 2021
* Send actual configured APN AMBR to RAN

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
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.

3 participants