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: request UPF to allocate F-TEID #334

Merged
merged 19 commits into from
Nov 28, 2024
Merged

Conversation

gruyaume
Copy link
Contributor

@gruyaume gruyaume commented Oct 16, 2024

Description

As of 3GPP release 16, the F-TEID should be allocated by the UPF (and not by the SMF). Here we make the necessary changes for the SMF to request the UPF to allocate the F-TEID through PFCP and we remove the F-TEID allocation from the SMF.

⚠️ Warning: This is a breaking change. This PR goes hand in hand with the change in the UPF.

Context

From the 3GPP specification referenced below:

For EPC and 5GC, F-TEID shall be only allocated by the UP function, see clause 5.8.2.3 of 3GPP TS 23.501[28].
The UP function shall set the FTUP feature flag in the UP Function Features IE (see clause 8.2.25) and the CP function
shall request the UP function to allocate the F-TEID. The UP function shall reject a request to establish a new PDR with
an F-TEID allocation in the CP function option, with the cause "Invalid F-TEID allocation option". As an exception, the
UP Function 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 as specified in clause 4.3.2 of 3GPP TS 23.527 [40] and
clauses 16.1A.4 and 17.1A.4 of 3GPP TS 23.007 [24].

Reference

Signed-off-by: Guillaume Belanger <[email protected]>
@gruyaume gruyaume changed the title feat: request upf to allocate f-teid feat: request UPF to allocate F-TEID Oct 16, 2024
@thakurajayL
Copy link
Contributor

Not reviewing since its in draft state !

@gruyaume gruyaume marked this pull request as ready for review November 12, 2024 11:39
VERSION Outdated Show resolved Hide resolved
Signed-off-by: Guillaume Belanger <[email protected]>
Copy link
Contributor

@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, everything looks good

context/datapath_test.go Show resolved Hide resolved
context/upf.go Show resolved Hide resolved
@gruyaume
Copy link
Contributor Author

Overall, everything looks good

I believe I made the proper changes in the upf pfcp adapter here but I'm not very confident since I have no environment to validate this and the tests for this part of the code are non-existent.

@gab-arrobo
Copy link
Contributor

Overall, everything looks good

I believe I made the proper changes in the upf pfcp adapter here but I'm not very confident since I have no environment to validate this and the tests for this part of the code are non-existent.

I am not sure I understand this comment. You mentioned that you have made the proper changes in the upf adapter, but, I do not see any PR open in that repo (https://github.com/omec-project/upfadapter/pulls). Or are you referring to the changes in smf/pfcp/adapter.go? If so, the changes did not work when running a test that includes/deploys the upfadapter pod

context/datapath.go Show resolved Hide resolved
pfcp/adapter/adapter.go Show resolved Hide resolved
pfcp/adapter/adapter.go Outdated Show resolved Hide resolved
pfcp/handler/handler.go Show resolved Hide resolved
@gruyaume
Copy link
Contributor Author

Overall, everything looks good

I believe I made the proper changes in the upf pfcp adapter here but I'm not very confident since I have no environment to validate this and the tests for this part of the code are non-existent.

I am not sure I understand this comment. You mentioned that you have made the proper changes in the upf adapter, but, I do not see any PR open in that repo (https://github.com/omec-project/upfadapter/pulls). Or are you referring to the changes in smf/pfcp/adapter.go? If so, the changes did not work when running a test that includes/deploys the upfadapter pod

I was referring to the changes in this commit: 1f33efe

I haven't changed upfadapter indeed, I can do my best to make the changes over there but I have not proper way of testing them.

Signed-off-by: Guillaume Belanger <[email protected]>
@gruyaume
Copy link
Contributor Author

Overall, everything looks good

I believe I made the proper changes in the upf pfcp adapter here but I'm not very confident since I have no environment to validate this and the tests for this part of the code are non-existent.

I am not sure I understand this comment. You mentioned that you have made the proper changes in the upf adapter, but, I do not see any PR open in that repo (https://github.com/omec-project/upfadapter/pulls). Or are you referring to the changes in smf/pfcp/adapter.go? If so, the changes did not work when running a test that includes/deploys the upfadapter pod

I was referring to the changes in this commit: 1f33efe

I haven't changed upfadapter indeed, I can do my best to make the changes over there but I have not proper way of testing them.

I just looked at the upfadapter code and it's quite unclear to me what needs to be changed.

Note: This project has no README.md, which doesn't help in understanding its purpose.

go.mod Outdated Show resolved Hide resolved
@gruyaume
Copy link
Contributor Author

Overall, everything looks good

I believe I made the proper changes in the upf pfcp adapter here but I'm not very confident since I have no environment to validate this and the tests for this part of the code are non-existent.

I am not sure I understand this comment. You mentioned that you have made the proper changes in the upf adapter, but, I do not see any PR open in that repo (https://github.com/omec-project/upfadapter/pulls). Or are you referring to the changes in smf/pfcp/adapter.go? If so, the changes did not work when running a test that includes/deploys the upfadapter pod

I was referring to the changes in this commit: 1f33efe
I haven't changed upfadapter indeed, I can do my best to make the changes over there but I have not proper way of testing them.

I just looked at the upfadapter code and it's quite unclear to me what needs to be changed.

Note: This project has no README.md, which doesn't help in understanding its purpose.

@gab-arrobo were you able to re-test with the changes in the smf's pfcp adapter changes. I just looked at upfadapter and I can't find anything in there that would break the e2e tests. I believe the problem was in the SMF's pfcp adapter code. In any case, logs and/or captures would be necessary at this point.

@gab-arrobo
Copy link
Contributor

@gab-arrobo were you able to re-test with the changes in the smf's pfcp adapter changes. I just looked at upfadapter and I can't find anything in there that would break the e2e tests. I believe the problem was in the SMF's pfcp adapter code. In any case, logs and/or captures would be necessary at this point.

I think I did run a test, but it did not work. Let me run another test this afternoon and I can provide logs.

@gab-arrobo
Copy link
Contributor

@gab-arrobo were you able to re-test with the changes in the smf's pfcp adapter changes. I just looked at upfadapter and I can't find anything in there that would break the e2e tests. I believe the problem was in the SMF's pfcp adapter code. In any case, logs and/or captures would be necessary at this point.

I think I did run a test, but it did not work. Let me run another test this afternoon and I can provide logs.

@gruyaume the PRs are now working as expected when using the upfadapter

@thakurajayL, were all of your comments addressed? There is no more comments from my side

        "Profile Name: profile1, Profile Type: register",
        "Ue's Passed: 5, Ue's Failed: 0",
        "Profile Status: PASS",
        "Profile Name: profile2, Profile Type: pdusessest",
        "Ue's Passed: 5, Ue's Failed: 0",
        "Profile Status: PASS",
        "Profile Name: profile3, Profile Type: anrelease",
        "Ue's Passed: 5, Ue's Failed: 0",
        "Profile Status: PASS",
        "Profile Name: profile5, Profile Type: deregister",
        "Ue's Passed: 5, Ue's Failed: 0",
        "Profile Status: PASS",
        "Profile Name: profile7, Profile Type: uereqpdusessrelease",
        "Ue's Passed: 5, Ue's Failed: 0",
        "Profile Status: PASS",
        "Profile Name: profile4, Profile Type: uetriggservicereq",
        "Ue's Passed: 5, Ue's Failed: 0",
        "Profile Status: PASS"

image

@gruyaume
Copy link
Contributor Author

@gab-arrobo were you able to re-test with the changes in the smf's pfcp adapter changes. I just looked at upfadapter and I can't find anything in there that would break the e2e tests. I believe the problem was in the SMF's pfcp adapter code. In any case, logs and/or captures would be necessary at this point.

I think I did run a test, but it did not work. Let me run another test this afternoon and I can provide logs.

@gruyaume the PRs are now working as expected when using the upfadapter

@thakurajayL, were all of your comments addressed? There is no more comments from my side

        "Profile Name: profile1, Profile Type: register",
        "Ue's Passed: 5, Ue's Failed: 0",
        "Profile Status: PASS",
        "Profile Name: profile2, Profile Type: pdusessest",
        "Ue's Passed: 5, Ue's Failed: 0",
        "Profile Status: PASS",
        "Profile Name: profile3, Profile Type: anrelease",
        "Ue's Passed: 5, Ue's Failed: 0",
        "Profile Status: PASS",
        "Profile Name: profile5, Profile Type: deregister",
        "Ue's Passed: 5, Ue's Failed: 0",
        "Profile Status: PASS",
        "Profile Name: profile7, Profile Type: uereqpdusessrelease",
        "Ue's Passed: 5, Ue's Failed: 0",
        "Profile Status: PASS",
        "Profile Name: profile4, Profile Type: uetriggservicereq",
        "Ue's Passed: 5, Ue's Failed: 0",
        "Profile Status: PASS"

image

Awesome, the issues were in the smf's pfcp adapter code then/

@@ -534,37 +476,19 @@ func (dpNode *DataPathNode) ActivateUpLinkPdr(smContext *SMContext, defQER *QER,

curULTunnel := dpNode.UpLinkTunnel
for name, ULPDR := range curULTunnel.PDR {
ULDestUPF := curULTunnel.DestEndPoint.UPF
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this code is deleted?

ULPDR.QER = append(ULPDR.QER, defQER)

// Set Default precedence
if ULPDR.Precedence == 0 {
ULPDR.Precedence = defPrecedence
}

var iface *UPFInterfaceInfo
Copy link
Contributor

Choose a reason for hiding this comment

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

What was motivation to delete this code?

Copy link
Contributor Author

@gruyaume gruyaume Nov 26, 2024

Choose a reason for hiding this comment

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

This instance of iface was only used to get the user plane IP address and set the PDR F-TEID accordingly. Now that the F-TEID is set by the UPF, this is not needed anymore

Old code:

var iface *UPFInterfaceInfo
		if dpNode.IsANUPF() {
			iface = ULDestUPF.GetInterface(models.UpInterfaceType_N3, smContext.Dnn)
		} else {
			iface = ULDestUPF.GetInterface(models.UpInterfaceType_N9, smContext.Dnn)
		}

		if upIP, err := iface.IP(smContext.SelectedPDUSessionType); err != nil {
			logger.CtxLog.Errorf("activate UpLink PDR[%v] failed %v", name, err)
			return err
		} else {
			ULPDR.PDI.SourceInterface = SourceInterface{InterfaceValue: SourceInterfaceAccess}
			ULPDR.PDI.LocalFTeid = &FTEID{
				V4:          true,
				Ipv4Address: upIP,
				Teid:        curULTunnel.TEID,
			}

			ULPDR.PDI.UEIPAddress = &ueIpAddr

			ULPDR.PDI.NetworkInstance = util_3gpp.Dnn(smContext.Dnn)
		}

}

if upIP, err := iface.IP(smContext.SelectedPDUSessionType); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we are removing code which supports iUPF

Copy link
Contributor Author

@gruyaume gruyaume Nov 26, 2024

Choose a reason for hiding this comment

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

What's iUPF?

The code you are referring to is not needed anymore as the F-TEID is generated by the UPF.

Copy link
Contributor

Choose a reason for hiding this comment

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

@gruyaume, FYI, iUPF is the intermediate UPF (N3 and N9 interfaces)

pfcp/adapter/adapter.go Show resolved Hide resolved
pfcp/adapter/adapter.go Outdated Show resolved Hide resolved
Signed-off-by: Guillaume Belanger <[email protected]>
@gruyaume
Copy link
Contributor Author

@gab-arrobo @thakurajayL can you please review this PR?

@thakurajayL thakurajayL merged commit ec30f3e into omec-project:main Nov 28, 2024
8 checks passed
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.

5 participants