Refactor SMF code for readability, conciseness, and modularity

[LaumiH]

Dear free5gc SMF team!

In the past months, I have been working with the SMF at university. I realized that the code base could be improved with regards to readability, conciseness, and modularity.

So while modifying the code so that it fits my use cases, I ended up refactoring it as well. With this PR, I want to propose my changes to you. Maybe you agree and find the changes worth merging!

There is no new functionality added (besides the session restoration procedure for UPFs, made possible through my refactoring).

The following is a non-complete list of the most important changes I made:

• general: make code more object-oriented, remove global variables and non-member functions (e.g., sm_context)
• move functions that work on Session Management Context (SMContext) to member functions of SMFContext
• move data path tunnel handling functions to DataPath (instead of UPF), I think they fit better here
• unify data path node/ UPNode ID handling; remove Addr variable from UPF (NodeID is sufficient); remove UPF name
• PDRs are handled in PFCPSessionContext and by UPF object with same references
• refine session rule handling during session management: rules (PDRs, FARs etc.) are no longer collected in slices and transferred from these slices to the target UPF; instead, PFCPSessionContext stores all PDRs in the state that should be sent to the UPF; when a UPF acked the PFCP message transfer, the rules are set to a new SMF-internal state RULE_SYNCED for this UPF; this avoids creating all those lists and looping them every time a rule is created/ updated/ deleted; aka make BuildPfcpSessionEstablishmentRequest and other request build methods more efficient by only passing PFCPSessionContext, which contains all session rules to apply
• new session rule state RULE_SYNCED RuleState = 5 -> SMF notes that UPF successfully received rule via PFCP, no need to send these rules again; introduce new function SetStateRecursive to set state of PDR and associated FAR etc. recursively
• store session rule state INITIAL directly when generating PDRs and other rules
• make PDR the parent of all other session rules
• RemovePDR() also frees associated FAR IDs etc.
• pass createData *models.SmContextCreateData to NewSMContext directly, handle all related functionality in this method
• unify search and selection of other NFs in sm_context
• remove AllocateLocalSEIDForUPPath function (unused)
• make naming of maps in UserPlaneInformation more intuitive
• store PFCPSessionContexts at UPF as well
• introduce session restoration procedure for UPF
• replace UPF state (Associated etc.) with Go Context and CancelFunc -> thread safe!
• move UPF.SNssaiInfos generation directly into NewUPF contructor (instead of adding it in user_plane_information)
• remove global variable upfPool in UPF and use maps in UserPlaneInformation instead
• remove unnecessary checks for UPF association status for PDR ID generation and others
• increase readability of NewUserPlaneInformation function and remove unnecessary intermediate maps
• implement heartbeat retry procedure
• make code in internal/sbi/producer/datapath.go more modular and reusable, avoid looping through session rules
• HandlePDUSessionSMContextCreate: encapsulate context creation in one function, bundle calls to other NFs, make code more readable and more concise
• "inheritance" for UPNodes: UPF and GNB are composite structs from UPNode and implement the same interface UPNodeInterface; use UPF struct directly in the code whenever specifically a UPF is addressed
• add String() methods to many objects to better print them in logs and for debugging purposes

I have tested the code with UERANSIM and performed a session establishment and release procedure with 2 UEs and one sessions each. Please help me to also test additional functionality like handover, UL-CL, ...

Cheers, LaumiH

[ianchen0119]

Hi @LaumiH

Thanks for your PR. We are currently working on refactoring, so we'll considering merge this patch after the refactoring work is done.

Thanks again.

[LaumiH]

Dear free5gc team,

I just wanted to let you know that I am working on the refactoring, I was on holiday for 3 weeks and I am back just now.

I am rebasing the branch to the main branch, which means that there are many complex conflicts, but I am confident that I will sort things out.

There is one question about the ULCL procedure: Currently, afaik it is started during session modification (in HandlePDUSessionSMContextUpdate -> UpdatePDUSessionAtANUPF), but then it also establishes the uplink tunnels and PDRs etc. for the second PSA and the ULCL. The uplink part could be moved to the session establishment code (in HandlePDUSessionSMContextCreate) to keep the code more aligned with the procedure for a single PSA. Here, only the downlink direction is established during session modification. Is this generally something you find good or bad? I could refactor the ULCL code so that it makes use of existing code instead of having everything in a separate file (internal/sbi/processor/ulcl_procedure.go).

Cheers, LaumiH

[andy89923]

@LaumiH

Hi,

Thanks for your contribution. We will discuss about this PR and the question you ask, and get back to you tomorrow.

Best, Andy

[LaumiH]

Hi all,

so the branch now contains a conflict-free version of my refactorings that is rebased to the current main branch.

There are a couple of TODOs in the code, mainly with questions I want to ask and SMF logic I am not 100% sure about. I also am not sure that I have tested the code properly, maybe we can cooperate in this?

Cheers, LaumiH

[andy89923]

Hi, sorry for the late reply. I think we should split this PR into multiple. This PR contains too many refactors. In this case, we can verify each functionality and don't have too many conflicts with this PR and other development/fixes on our site.

@ianchen0119

[LaumiH]

Hi @andy89923,

I totally agree, this is one huge blob of changes :smile: . However, many refactorings are dependent on each other. I will try to make smaller PRs in the following days, and hope that they can be merged with your help :)

Best, LaumiH