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

Add support for SDM Subscription and Unsubscription #95

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

saileshvvr
Copy link
Contributor

Add support for SDM Subscription during UE 1st PDU Session Establishment
Add support for SDM UnSubscription during release of last UE PDU Session
Code refactor to handle all SDM processing (SDM_Get and Subscription/Unsubscription) under consumer package

@andy89923
Copy link
Contributor

Hi!
Thanks for the PR. We tested and found out that there is unexpected behavior occurred. I'm finding out the error, so I am marking this PR as a draft.
The error occurred when using UERANSIM to test the whole 5gc functionalities.

@saileshvvr
Copy link
Contributor Author

Hi,
Thanks for sharing. Sure please.
Request to share the test scenario or logs or elaborate the details, it helps me too for finding the issue.
Earlier while adding the support for UECM registration i encountered crash with UDM due to missing pointer deference. I fixed the UDM issue separately while SMF PR initiated.

@andy89923
Copy link
Contributor

Hi,
I created a subscriber and tried to establish PDU sessions uses UERANSIM, the errors as follow:

2024-03-06T05:44:58.678704422Z [ERRO][SMF][GIN] panic: runtime error: invalid memory address or nil pointer dereference
goroutine 93 [running]:
runtime/debug.Stack()
        /usr/local/go/src/runtime/debug/stack.go:24 +0x65
github.com/free5gc/util/logger.ginRecover.func1.1()
        /home/ctfang/go/pkg/mod/github.com/free5gc/util@v1.0.5/logger/logger.go:284 +0x1a5
panic({0xb53480, 0x11c3170})
        /usr/local/go/src/runtime/panic.go:838 +0x207
github.com/free5gc/smf/internal/sbi/producer.updateAnUpfPfcpSession(0xc0005b2000, {0xc0000564d0, 0x2, 0x2}, {0xc0000564e0, 0x2, 0x2}, {0xc0000bd390, 0x0, 0x0}, ...)
        /home/ctfang/free5gc-dev/NFs/smf/internal/sbi/producer/datapath.go:368 +0xd9
github.com/free5gc/smf/internal/sbi/producer.HandlePDUSessionSMContextUpdate({0xc00068e380, 0x2d}, {0xc000782680, {0x0, 0x0, 0x0}, {0xc00067a800, 0xf, 0x3e8}})
        /home/ctfang/free5gc-dev/NFs/smf/internal/sbi/producer/pdu_session.go:768 +0x36fa
github.com/free5gc/smf/internal/sbi/pdusession.HTTPUpdateSmContext(0xc000162000)
        /home/ctfang/free5gc-dev/NFs/smf/internal/sbi/pdusession/api_individual_sm_context.go:83 +0x3bb
github.com/gin-gonic/gin.(*Context).Next(...)
        /home/ctfang/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/context.go:174
github.com/free5gc/util/logger.ginRecover.func1(0xc000162000)
        /home/ctfang/go/pkg/mod/github.com/free5gc/util@v1.0.5/logger/logger.go:316 +0x7b
github.com/gin-gonic/gin.(*Context).Next(...)
        /home/ctfang/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/context.go:174
github.com/free5gc/util/logger.ginToLogrus.func1(0xc000162000)
        /home/ctfang/go/pkg/mod/github.com/free5gc/util@v1.0.5/logger/logger.go:251 +0x87
github.com/gin-gonic/gin.(*Context).Next(...)
        /home/ctfang/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/context.go:174
github.com/gin-gonic/gin.(*Engine).handleHTTPRequest(0xc000498ea0, 0xc000162000)
        /home/ctfang/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/gin.go:620 +0x671
github.com/gin-gonic/gin.(*Engine).ServeHTTP(0xc000498ea0, {0xda3370?, 0xc00068c020}, 0xc0006a2400)
        /home/ctfang/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/gin.go:576 +0x1dd
golang.org/x/net/http2.(*serverConn).runHandler(0xc000056370?, 0xc00049c7d0?, 0x900666?, 0xc000083ba0?)
        /home/ctfang/go/pkg/mod/golang.org/x/net@v0.19.0/http2/server.go:2368 +0xc2
created by golang.org/x/net/http2.(*serverConn).scheduleHandler
        /home/ctfang/go/pkg/mod/golang.org/x/net@v0.19.0/http2/server.go:2303 +0x23b
2024-03-06T05:44:58.678955069Z [INFO][SMF][GIN] | 500 |       127.0.0.1 | POST    | /nsmf-pdusession/v1/sm-contexts/urn:uuid:7bc2f7cd-37e7-412b-8696-2fb115514b78/modify |

@saileshvvr
Copy link
Contributor Author

Thank you for sharing. I shall also check to analyze the issue. Thanks again :)

@saileshvvr
Copy link
Contributor Author

Hi,

The merge issue was resolved in the latest patch. Remove duplicated code that is causing the crash.

Thanks
Sailesh

@saileshvvr saileshvvr closed this Apr 6, 2024
@saileshvvr saileshvvr reopened this Apr 6, 2024
@saileshvvr
Copy link
Contributor Author

Sorry, wrongly clicked close with commit button. Reopening PR. Thanks!

@andy89923
Copy link
Contributor

Thanks, I will try this PR next week.

@andy89923 andy89923 self-requested a review April 6, 2024 14:48
@ming-hsien
Copy link
Contributor

Looks good for me !

@saileshvvr
Copy link
Contributor Author

Thanks @ming-hsien .
Hi @andy89923 , please let me know in case if anything missed/needed to be checked from my end. Thanks!

@andy89923 andy89923 marked this pull request as ready for review April 17, 2024 05:54
@@ -16,6 +16,26 @@ func RemoveSMContextFromAllNF(smContext *smf_context.SMContext, sendNotification
}
}

problemDetails, err := consumer.SDMUnSubscribe(smContext)
Copy link
Contributor

Choose a reason for hiding this comment

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

This was also called in some HandleFunction in pdu_session.go, leading to re-UnSubscribe.

Copy link
Contributor

Choose a reason for hiding this comment

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

截圖 2024-04-17 下午3 51 35

I used PacketRusher termination to send Deregistration and found the result.

@andy89923
Copy link
Contributor

Also, I found out that there are some problems in UDM when sending RegistrationSmfRegistrations.
I checked the code and found out that the implementation of the parsing request in UDM may not comply with openapi.

截圖 2024-04-17 下午3 47 28

@saileshvvr
Copy link
Contributor Author

Can the UDM complying with openapi be handled seperately

@andy89923
Copy link
Contributor

Can the UDM complying with openapi be handled seperately

Sure.

@saileshvvr
Copy link
Contributor Author

Could you please share the steps used with Packet rusher termination for recreating the issue.

@andy89923 andy89923 marked this pull request as draft May 2, 2024 02:52
@andy89923
Copy link
Contributor

Could you please share the steps used with Packet rusher termination for recreating the issue.

This article shows how to use PacketRusher. When using CRTL+c to terminate UE, packet rusher would send dereg.
https://free5gc.org/blog/20240110/20240110/

@saileshvvr
Copy link
Contributor Author

Hi @andy89923 , sorry for the delay as i was occupied in other work. I looked the issue today and identified the problem.

The issue is due to the return statement in line #162 in subscriber_data_management.go. Due to this the DeleteUe() is not triggered.

I will fix the issue and upload latest patch.

Thanks!

@andy89923
Copy link
Contributor

Hi, thanks for your contribution.

Some informations to let you know:

@saileshvvr
Copy link
Contributor Author

could you please share the commands so that i shall not make any mistake. Thanks!

@saileshvvr
Copy link
Contributor Author

I did rebase and followed ""Conventional commit message" in latest commit. However it looks like due to old commit message the commit check failure encountered.
Thanks!

@andy89923
Copy link
Contributor

could you please share the commands so that i shall not make any mistake. Thanks!

git rebase -i 

And change "pick" -> "reword", you can revise the commit message.

@saileshvvr saileshvvr force-pushed the sdm_subscription branch 2 times, most recently from c63b92c to ad3979f Compare June 8, 2024 13:16
tim-ywliu and others added 17 commits June 8, 2024 18:51
* support converged charging

* flow status

* add pdu level charging urr to default data path

* fix: only onlince charging need to request unit

* fix: release charging session in all pdu session release case

* fix: don't send charging request for non-charging urr

* rm untested code

* update openapi/pfcp hash

* add nil & error checking

* update go.sum

* add more error checking

* fix test fail

* fix ci error

* add comment for addPduLevelChargingRuleToFlow()

* add comment & fix potential synchronization issues

* fix typo & linter error

* handle assigned when IP in ipFilterRule is assigned

* fix typo

* fix typo & set default value of RequestedUnit

* fix NewSMContext

* fix: remove SwapSrcAndDst()

* fix: UL, DL flow description should be same

* remove SMF's default data path to reduce redundant PDRs

* fix: testcase's ip.dst should not be any

* style: fix naked return

* update util's hash

* fix: duplicated URRs in PDR

* fix: Volume Threshold IE should be added to PFCP Session Modification Request Update URR to prevent the flow rule be blocked

* remove SMContext.UrrIDGenerator

* fix: MBQE, MAQE's URR

* comment out these variable: N3N6, N3N9, N9N6

* remove FlowDescription judgement

* remove unused code and add comments

* Add back defalt urr rule

* Fix: Add GetTokenCtx() in CHF Selection

* Remove unused comments

---------

Co-authored-by: roy19991013 <80-ChienAn@users.noreply.gitlab.nems.cs.nctu.edu.tw>
Co-authored-by: Tim Liu <timliu@nctu.edu.tw>
Co-authored-by: Ian Chen <iancodinghtml@gmail.com>
Co-authored-by: Ian Chen <yi.chen@saviah.com>
Co-authored-by: ianchen0119 <ychen.desl@gmail.com>
Co-authored-by: brianchennn <ny40ny40ny.cs11@nycu.edu.tw>
Code refactoring to move all SDM Handling in one place
fix lint error

fix golangci-lint error for package import

fix golangci-lint error for package import

fix golangci-lint error for package import

fix golangci-lint error for package import

fix golangci-lint error for package import
Bumps google.golang.org/protobuf from 1.30.0 to 1.33.0.

---
updated-dependencies:
- dependency-name: google.golang.org/protobuf
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@saileshvvr
Copy link
Contributor Author

could you please share the commands so that i shall not make any mistake. Thanks!

git rebase -i 

And change "pick" -> "reword", you can revise the commit message.

I followed the recommended. However encountering issue due to multiple old commits. Thanks!

@andy89923
Copy link
Contributor

If the PR is ready, just let me know. I can help you fix the commit message.

@saileshvvr
Copy link
Contributor Author

Hi PR is ready. issues encountered due to rebase and commit messages. Thanks!

@andy89923
Copy link
Contributor

Ok!
After the refactor PR is merged, I think this PR would be more conflicts to fix.
We could discuss and fix then.

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.

None yet

5 participants