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

fix: ULCL not working; close #141 #147

Merged
merged 1 commit into from
Feb 28, 2025
Merged

Conversation

louisroyer
Copy link
Contributor

No description provided.

@louisroyer louisroyer mentioned this pull request Feb 4, 2025
@ianchen0119
Copy link
Contributor

@lyz508
Please help to review the PR.

Thanks!

@@ -587,7 +605,7 @@ func (c *SMContext) SelectDefaultDataPath() error {
c.Log.Infof("Has pre-config default path")
uePreConfigPaths := GetUEPreConfigPaths(c.Supi, c.SelectedUPF.Name)
for _, dp := range uePreConfigPaths.DataPathPool {
if !dp.IsDefaultPath {
if dp.IsDefaultPath {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, @louisroyer,
If we add the default path created by the ULCL config here,
SMF will create a PDR without a decision from the PCF (e.g., additional charging URRs).
Maybe we could directly use the default path created by PCC rules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @lyz508,
how can we use this path? According to

// SelectDefaultDataPath() will create a default data path if default data path is not found.
SelectDefaultDataPath is called to create the default datapath when it is not found…

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, @louisroyer,
Sorry for the late reply.
Maybe we could add a check before adding the ULCL default path to SMContext?

defaultPath = uePreConfigPaths.DataPathPool.GetDefaultPath()
if c.Tunnel.DataPathPool.GetDefaultPath() == nil {
    c.Tunnel.AddDataPath(defaultPath)
}

Since the default path is added to SMContext when handling PCCRules creation, this approach would help prevent duplicate creation of the default path.

createdDataPath.IsDefaultPath = true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lyz508 Is eab6e3c what you had in mind ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@louisroyer Yes, thanks for the update!

@lyz508
Copy link
Contributor

lyz508 commented Feb 28, 2025

@ianchen0119 LGTM.
I also tested the ULCL and charging functionality, and they work fine.

@ianchen0119 ianchen0119 changed the base branch from main to next February 28, 2025 17:03
@ianchen0119
Copy link
Contributor

Hi @Alonza0314

Please test the PR on next branch by using CI-TEST.

Thanks.

@louisroyer
Copy link
Contributor Author

I rebased on next.

@Alonza0314
Copy link
Contributor

Hi @Alonza0314

Please test the PR on next branch by using CI-TEST.

Thanks.

It passed the tests. 🎉

@ianchen0119 ianchen0119 merged commit 724b44e into free5gc:next Feb 28, 2025
3 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.

4 participants