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

feat: add F-TEID allocation support #840

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

gruyaume
Copy link
Contributor

@gruyaume gruyaume commented Aug 5, 2024

Description

As of 3GPP Release 16, F-TEID allocation is done in the UPF instead of the SMF. This PR implements F-TEID allocation as well as the necessary PFCP communication changes.

Summary

  1. The UPF will set the FTUP feature flag in its PFCP Association Setup Response to the SMF
  2. The SMF must set the CHOOSE flag in its request to establishment a new PDR in the PFCP Session Establishment Request
  3. When this CHOOSE flag is set, the UPF allocates a F-TEID using a TEID generator
  4. The UPF sets the F-TEID in its PFCP Session Establishment Response

Changes in the SMF and UPF are intertwined

This change and the SMF are intertwined and have to be merged close together so that the network keeps on working on main:

Reference

3GPP Release 16

Relevant information from the 3GPP Release 16 29.244 document:

F-TEID shall be only allocated by the UPF. The UPF shall set the FTUP feature flag in the UP Function Features IE and the SMF shall request the UPF to allocate the F-TEID. The UPF shall reject a request to establish a new PDR with
an F-TEID allocation in the SMF option, with the cause "Invalid F-TEID allocation option". As an exception, the
UPF shall accept the PFCP Session Establishment Request message with a PDR including an existing F-TEID
to re-establish a PFCP session during a restoration procedure.

The SMF shall request the UPF to allocate the F-TEID by setting the CHOOSE flag in the Local F-TEID. And if the PDR(s) is created successfully, the UPF shall return the F-TEID(s) it has assigned to the PDR(s) in the PFCP Session Establishment Response or PFCP Session Modification Response.

@omecproject
Copy link

Can one of the admins verify this patch?

@gruyaume gruyaume changed the title feat: return F-TEID in PFCP session establishment response feat: add f-teid allocation support Aug 5, 2024
@gruyaume gruyaume changed the title feat: add f-teid allocation support feat: add F-TEID allocation support Aug 5, 2024
@thakurajayL thakurajayL self-assigned this Oct 5, 2024
@gab-arrobo
Copy link
Collaborator

@gruyaume please rebase your PR.

@gab-arrobo
Copy link
Collaborator

@gruyaume,
FYI, the 2 GHAs that fail are because of the same reason:

=== RUN   TestUPFBasedUeIPAllocation
    basic_test.go:102: 
        	Error Trace:	/home/runner/work/upf/upf/test/integration/basic_test.go:102
        	Error:      	"[0xc000de20c0 0xc000de2180]" should have 1 item(s), but has 2
        	Test:       	TestUPFBasedUeIPAllocation
--- FAIL: TestUPFBasedUeIPAllocation (5.18s)

@gruyaume
Copy link
Contributor Author

@gruyaume, FYI, the 2 GHAs that fail are because of the same reason:

=== RUN   TestUPFBasedUeIPAllocation
    basic_test.go:102: 
        	Error Trace:	/home/runner/work/upf/upf/test/integration/basic_test.go:102
        	Error:      	"[0xc000de20c0 0xc000de2180]" should have 1 item(s), but has 2
        	Test:       	TestUPFBasedUeIPAllocation
--- FAIL: TestUPFBasedUeIPAllocation (5.18s)

Yeah this is still a draft, I'm trying to figure out why the Created PDR's aren't returned in the PFCP Session Establishment Response.

@gruyaume gruyaume marked this pull request as ready for review November 12, 2024 11:39
Copy link
Collaborator

@gab-arrobo gab-arrobo left a comment

Choose a reason for hiding this comment

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

Overall, it looks good to me. However, when running an E2E test using this PR along with SMF #334, the test fails, as shown below. Did you get a chance to run an E2E test with these changes?

TASK [simulator : debug] ***********************************************************************************************
fatal: [node1]: FAILED! => {
    "msg": [
        "Profile Name: profile2, Profile Type: pdusessest",
        "Ue's Passed: 0, Ue's Failed: 1",
        "Profile Errors:",
        "imsi:imsi-208930100007510, profile timeout",
        "Profile Status: FAIL"
    ]
}

docs/configuration-guide.md Outdated Show resolved Hide resolved
pfcpiface/fteid.go Outdated Show resolved Hide resolved
@gruyaume
Copy link
Contributor Author

Overall, it looks good to me. However, when running an E2E test using this PR along with SMF #334, the test fails, as shown below. Did you get a chance to run an E2E test with these changes?

TASK [simulator : debug] ***********************************************************************************************
fatal: [node1]: FAILED! => {
    "msg": [
        "Profile Name: profile2, Profile Type: pdusessest",
        "Ue's Passed: 0, Ue's Failed: 1",
        "Profile Errors:",
        "imsi:imsi-208930100007510, profile timeout",
        "Profile Status: FAIL"
    ]
}

Yes I've tested this end to end and it worked fine with all the 5 profiles I tested with. I'll try again to make sure.

@gruyaume
Copy link
Contributor Author

Overall, it looks good to me. However, when running an E2E test using this PR along with SMF #334, the test fails, as shown below. Did you get a chance to run an E2E test with these changes?

TASK [simulator : debug] ***********************************************************************************************
fatal: [node1]: FAILED! => {
    "msg": [
        "Profile Name: profile2, Profile Type: pdusessest",
        "Ue's Passed: 0, Ue's Failed: 1",
        "Profile Errors:",
        "imsi:imsi-208930100007510, profile timeout",
        "Profile Status: FAIL"
    ]
}

Yes I've tested this end to end and it worked fine with all the 5 profiles I tested with. I'll try again to make sure.

I tested it again and it still works on my side, I attached the gnbsim logs.
gnbsim_logs.txt

Let me know if you have more information about failures. Logs and/or captures would be useful.

@gab-arrobo
Copy link
Collaborator

I tested it again and it still works on my side, I attached the gnbsim logs. gnbsim_logs.txt

Let me know if you have more information about failures. Logs and/or captures would be useful.

My E2E tests still fail, except Profile 1 (Registration request: no UP involvement). Let me take another look into the PR and will share logs later today/tomorrow. Also, I asked @sureshmarikkannu to test these PRs

        "Profile Name: profile1, Profile Type: register",
        "Ue's Passed: 5, Ue's Failed: 0",
        "Profile Status: PASS",
        "Profile Name: profile5, Profile Type: deregister",
        "Ue's Passed: 0, Ue's Failed: 5",
        "Profile Errors:",
        "imsi:imsi-208930100007543, profile timeout",
        "imsi:imsi-208930100007544, profile timeout",
        "imsi:imsi-208930100007542, profile timeout",
        "imsi:imsi-208930100007540, profile timeout",
        "imsi:imsi-208930100007541, profile timeout",
        "Profile Status: FAIL",
        "Profile Name: profile3, Profile Type: anrelease",
        "Ue's Passed: 0, Ue's Failed: 5",
        "Profile Errors:",
        "imsi:imsi-208930100007524, profile timeout",
        "imsi:imsi-208930100007523, profile timeout",
        "imsi:imsi-208930100007522, profile timeout",
        "imsi:imsi-208930100007521, profile timeout",
        "imsi:imsi-208930100007520, profile timeout",
        "Profile Status: FAIL",
        "Profile Name: profile7, Profile Type: uereqpdusessrelease",
        "Ue's Passed: 0, Ue's Failed: 5",
        "Profile Errors:",
        "imsi:imsi-208930100007562, profile timeout",
        "imsi:imsi-208930100007564, profile timeout",
        "imsi:imsi-208930100007561, profile timeout",
        "imsi:imsi-208930100007563, profile timeout",
        "imsi:imsi-208930100007560, profile timeout",
        "Profile Status: FAIL",
        "Profile Name: profile2, Profile Type: pdusessest",
        "Ue's Passed: 0, Ue's Failed: 5",
        "Profile Errors:",
        "imsi:imsi-208930100007513, profile timeout",
        "imsi:imsi-208930100007512, profile timeout",
        "imsi:imsi-208930100007511, profile timeout",
        "imsi:imsi-208930100007514, profile timeout",
        "imsi:imsi-208930100007510, profile timeout",
        "Profile Status: FAIL",
        "Profile Name: profile4, Profile Type: uetriggservicereq",
        "Ue's Passed: 0, Ue's Failed: 5",
        "Profile Errors:",
        "imsi:imsi-208930100007534, profile timeout",
        "imsi:imsi-208930100007532, profile timeout",
        "imsi:imsi-208930100007530, profile timeout",
        "imsi:imsi-208930100007533, profile timeout",
        "imsi:imsi-208930100007531, profile timeout",
        "Profile Status: FAIL"

@gruyaume
Copy link
Contributor Author

today

@gab-arrobo did you get time to find more details about the problem you are experimenting? Any help here would be useful in driving this one to completion.

@gab-arrobo
Copy link
Collaborator

today

@gab-arrobo did you get time to find more details about the problem you are experimenting? Any help here would be useful in driving this one to completion.

Reviewing it again this moment and I found the reason why the PRs fail in my setup... I use the upf-adapter by default and it looks like changes are required for the upf-adapter to handle the new way TEID is allocated. As soon as I disabled the upf-adapter, all my tests were successful. Can you please address this breaking change in the upf-adapter?

Also, I am moving the codebase to GO 1.23. This is because the latest GRPC version (1.68.0) requires any of the latest 2 major GO versions (i.e., 1.22 and/or 1.23). So, I am making this change across all of our repos such that all of them use the same Go version. Can you please make this change in as part of your 2 PRs (UPF and SMF)? or would you like me to open a PR for that and then you rebase your PRs? The key piece would be to have/use GO 1.23 as part of release images v2.0.0

@gruyaume
Copy link
Contributor Author

today

@gab-arrobo did you get time to find more details about the problem you are experimenting? Any help here would be useful in driving this one to completion.

Reviewing it again this moment and I found the reason why the PRs fail in my setup... I use the upf-adapter by default and it looks like changes are required for the upf-adapter to handle the new way TEID is allocated. As soon as I disabled the upf-adapter, all my tests were successful. Can you please address this breaking change in the upf-adapter?

Also, I am moving the codebase to GO 1.23. This is because the latest GRPC version (1.68.0) requires any of the latest 2 major GO versions (i.e., 1.22 and/or 1.23). So, I am making this change across all of our repos such that all of them use the same Go version. Can you please make this change in as part of your 2 PRs (UPF and SMF)? or would you like me to open a PR for that and then you rebase your PRs? The key piece would be to have/use GO 1.23 as part of release images v2.0.0

No problem, I just bumped it in the last commit.

@gab-arrobo
Copy link
Collaborator

@gruyaume, you need to "redo" the commit Update pfcpiface/session_pdr.go because it is not signed

@gab-arrobo
Copy link
Collaborator

No problem, I just bumped it in the last commit.

Thanks!

Signed-off-by: Guillaume Belanger <[email protected]>
pfcpiface/upf.go Outdated Show resolved Hide resolved
}

// Allocate and return an id in range [minValue, maxValue]
func (idGenerator *FTEIDGenerator) Allocate() (uint32, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not efficient.. In case we establish few hundred or thousand tunnels then this will become slow. Anything better we can do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is using the exact same algorithm as was used in the SMF. While we may want to change it for something efficient, I would decouple this performance improvement from this change here which is focused on moving f-teid allocation from the SMF to the UPF.

Reference:

@thakurajayL
Copy link
Contributor

Could you please share pcap when you establish 1 call and also PFCP association. Thanks

p.tunnelTEIDMask = 0xFFFFFFFF
p.tunnelIP4Dst = ip2int(tunnelIPv4Address)
p.tunnelIP4DstMask = 0xFFFFFFFF
if fteid.HasCh() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you pls help with this. What does this do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the SMF wants the UPF to allocate the F-TEID, which is the case in 3GPP release 16, the SMF sets the CHOOSE flag in the Local F-TEID of its PFCP association setup request. Here, the UPF checks if this flag is set. If it is, it sets UPAllocateFteid to True which will result in the UPF allocating an F-TEID.

From the 3GPP spec:

The SMF shall request the UPF to allocate the F-TEID by setting the CHOOSE flag in the Local F-TEID. And if the PDR(s) is created successfully, the UPF shall return the F-TEID(s) it has assigned to the PDR(s) in the PFCP Session Establishment Response or PFCP Session Modification Response.

pfcpiface/parse_pdr.go Outdated Show resolved Hide resolved
@gruyaume
Copy link
Contributor Author

Could you please share pcap when you establish 1 call and also PFCP association. Thanks

Ok will do.

@gruyaume
Copy link
Contributor Author

Could you please share pcap when you establish 1 call and also PFCP association. Thanks

Ok will do.

@thakurajayL

Here are the captures taken from the SMF and UPF respectively. You likely will want to filter in only pfcp packets.

smf.tar.gz
upf.tar.gz

I had to tar them because github does not accept pcap files.

@gruyaume
Copy link
Contributor Author

@thakurajayL @gab-arrobo can we complete the review here and in the SMF for this feature?

@gab-arrobo
Copy link
Collaborator

@thakurajayL @gab-arrobo can we complete the review here and in the SMF for this feature?

@thakurajayL, any further comment/question about this PR (and omec-project/smf#334). Everything looks good to me and these PRs are working fine when having the upfadapter in between them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants