From 100c762ce875a1462a152202e7c1cf108a8db208 Mon Sep 17 00:00:00 2001 From: Laura Henning Date: Wed, 11 Sep 2024 14:45:20 +0000 Subject: [PATCH 1/9] refactor: Use Go context.Context for UPF association state, improve context checks throughout code --- internal/context/context.go | 4 +- internal/context/sm_context_policy_test.go | 51 ++++++----- internal/context/upf.go | 91 ++++++++++++------- internal/context/upf_test.go | 57 ++++++------ internal/context/user_plane_information.go | 4 +- .../context/user_plane_information_test.go | 69 +++++++------- internal/pfcp/handler/handler.go | 4 +- internal/pfcp/message/send.go | 12 ++- internal/pfcp/message/send_test.go | 5 +- internal/pfcp/udp/udp.go | 13 +-- internal/pfcp/udp/udp_test.go | 12 +-- internal/sbi/api_upi.go | 10 +- internal/sbi/processor/association.go | 85 +++++++---------- internal/sbi/processor/pdu_session_test.go | 7 +- pkg/service/init.go | 5 +- pkg/utils/pfcp_util.go | 23 +++-- 16 files changed, 233 insertions(+), 219 deletions(-) diff --git a/internal/context/context.go b/internal/context/context.go index f8ab9b63..503ee834 100644 --- a/internal/context/context.go +++ b/internal/context/context.go @@ -62,8 +62,8 @@ type SMFContext struct { OAuth2Required bool UserPlaneInformation *UserPlaneInformation - Ctx context.Context - PFCPCancelFunc context.CancelFunc + PfcpContext context.Context + PfcpCancelFunc context.CancelFunc PfcpHeartbeatInterval time.Duration // Now only "IPv4" supported diff --git a/internal/context/sm_context_policy_test.go b/internal/context/sm_context_policy_test.go index 8630c143..58225916 100644 --- a/internal/context/sm_context_policy_test.go +++ b/internal/context/sm_context_policy_test.go @@ -1,12 +1,13 @@ package context_test import ( + "context" "testing" "github.com/stretchr/testify/require" "github.com/free5gc/openapi/models" - "github.com/free5gc/smf/internal/context" + smf_context "github.com/free5gc/smf/internal/context" "github.com/free5gc/smf/pkg/factory" ) @@ -110,7 +111,7 @@ var testConfig = factory.Config{ } func initConfig() { - context.InitSmfContext(&testConfig) + smf_context.InitSmfContext(&testConfig) factory.SmfConfig = &testConfig } @@ -121,7 +122,7 @@ func TestApplySessionRules(t *testing.T) { name string decision *models.SmPolicyDecision noErr bool - expectedSessRules map[string]*context.SessionRule + expectedSessRules map[string]*smf_context.SessionRule }{ { name: "nil decision", @@ -147,7 +148,7 @@ func TestApplySessionRules(t *testing.T) { }, }, }, - expectedSessRules: map[string]*context.SessionRule{ + expectedSessRules: map[string]*smf_context.SessionRule{ "SessRuleId-1": { SessionRule: &models.SessionRule{ AuthSessAmbr: &models.Ambr{ @@ -188,7 +189,7 @@ func TestApplySessionRules(t *testing.T) { }, }, }, - expectedSessRules: map[string]*context.SessionRule{ + expectedSessRules: map[string]*smf_context.SessionRule{ "SessRuleId-1": { SessionRule: &models.SessionRule{ AuthSessAmbr: &models.Ambr{ @@ -246,7 +247,7 @@ func TestApplySessionRules(t *testing.T) { }, }, }, - expectedSessRules: map[string]*context.SessionRule{ + expectedSessRules: map[string]*smf_context.SessionRule{ "SessRuleId-1": { SessionRule: &models.SessionRule{ AuthSessAmbr: &models.Ambr{ @@ -291,7 +292,7 @@ func TestApplySessionRules(t *testing.T) { "SessRuleId-1": nil, }, }, - expectedSessRules: map[string]*context.SessionRule{ + expectedSessRules: map[string]*smf_context.SessionRule{ "SessRuleId-2": { SessionRule: &models.SessionRule{ AuthSessAmbr: &models.Ambr{ @@ -324,7 +325,7 @@ func TestApplySessionRules(t *testing.T) { }, } - smctx := context.NewSMContext("imsi-208930000000001", 10) + smctx := smf_context.NewSMContext("imsi-208930000000001", 10) for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -346,9 +347,9 @@ func TestApplyPccRules(t *testing.T) { name string decision *models.SmPolicyDecision noErr bool - expectedPCCRules map[string]*context.PCCRule + expectedPCCRules map[string]*smf_context.PCCRule expectedQosDatas map[string]*models.QosData - expectedTcDatas map[string]*context.TrafficControlData + expectedTcDatas map[string]*smf_context.TrafficControlData }{ { name: "nil decision", @@ -386,7 +387,7 @@ func TestApplyPccRules(t *testing.T) { }, }, }, - expectedPCCRules: map[string]*context.PCCRule{ + expectedPCCRules: map[string]*smf_context.PCCRule{ "PccRuleId-1": { PccRule: &models.PccRule{ FlowInfos: []models.FlowInformation{ @@ -406,7 +407,7 @@ func TestApplyPccRules(t *testing.T) { QosId: "QosId-1", }, }, - expectedTcDatas: map[string]*context.TrafficControlData{ + expectedTcDatas: map[string]*smf_context.TrafficControlData{ "TcId-1": { TrafficControlData: &models.TrafficControlData{ TcId: "TcId-1", @@ -442,7 +443,7 @@ func TestApplyPccRules(t *testing.T) { }, }, }, - expectedPCCRules: map[string]*context.PCCRule{ + expectedPCCRules: map[string]*smf_context.PCCRule{ "PccRuleId-1": { PccRule: &models.PccRule{ FlowInfos: []models.FlowInformation{ @@ -478,7 +479,7 @@ func TestApplyPccRules(t *testing.T) { QosId: "QosId-2", }, }, - expectedTcDatas: map[string]*context.TrafficControlData{ + expectedTcDatas: map[string]*smf_context.TrafficControlData{ "TcId-1": { TrafficControlData: &models.TrafficControlData{ TcId: "TcId-1", @@ -514,7 +515,7 @@ func TestApplyPccRules(t *testing.T) { }, }, }, - expectedPCCRules: map[string]*context.PCCRule{ + expectedPCCRules: map[string]*smf_context.PCCRule{ "PccRuleId-1": { PccRule: &models.PccRule{ FlowInfos: []models.FlowInformation{ @@ -550,7 +551,7 @@ func TestApplyPccRules(t *testing.T) { QosId: "QosId-3", }, }, - expectedTcDatas: map[string]*context.TrafficControlData{ + expectedTcDatas: map[string]*smf_context.TrafficControlData{ "TcId-1": { TrafficControlData: &models.TrafficControlData{ TcId: "TcId-1", @@ -571,7 +572,7 @@ func TestApplyPccRules(t *testing.T) { "PccRuleId-2": nil, }, }, - expectedPCCRules: map[string]*context.PCCRule{ + expectedPCCRules: map[string]*smf_context.PCCRule{ "PccRuleId-1": { PccRule: &models.PccRule{ FlowInfos: []models.FlowInformation{ @@ -591,7 +592,7 @@ func TestApplyPccRules(t *testing.T) { QosId: "QosId-3", }, }, - expectedTcDatas: map[string]*context.TrafficControlData{ + expectedTcDatas: map[string]*smf_context.TrafficControlData{ "TcId-1": { TrafficControlData: &models.TrafficControlData{ TcId: "TcId-1", @@ -612,20 +613,20 @@ func TestApplyPccRules(t *testing.T) { "PccRuleId-1": nil, }, }, - expectedPCCRules: map[string]*context.PCCRule{}, + expectedPCCRules: map[string]*smf_context.PCCRule{}, expectedQosDatas: map[string]*models.QosData{}, - expectedTcDatas: map[string]*context.TrafficControlData{}, + expectedTcDatas: map[string]*smf_context.TrafficControlData{}, noErr: true, }, } - smfContext := context.GetSelf() - smfContext.UserPlaneInformation = context.NewUserPlaneInformation(&userPlaneConfig) + smfContext := smf_context.GetSelf() + smfContext.UserPlaneInformation = smf_context.NewUserPlaneInformation(&userPlaneConfig) for _, n := range smfContext.UserPlaneInformation.UPFs { - n.UPF.UPFStatus = context.AssociatedSetUpSuccess + n.UPF.AssociationContext = context.Background() } - smctx := context.NewSMContext("imsi-208930000000002", 10) + smctx := smf_context.NewSMContext("imsi-208930000000002", 10) smctx.SMLock.Lock() defer smctx.SMLock.Unlock() @@ -655,7 +656,7 @@ func TestApplyPccRules(t *testing.T) { }, } smctx.SelectedPDUSessionType = 1 - smctx.SessionRules["SessRuleId-1"] = &context.SessionRule{ + smctx.SessionRules["SessRuleId-1"] = &smf_context.SessionRule{ SessionRule: &models.SessionRule{ AuthSessAmbr: &models.Ambr{ Uplink: "1000 Kbps", diff --git a/internal/context/upf.go b/internal/context/upf.go index d14a0d3f..ff25b28e 100644 --- a/internal/context/upf.go +++ b/internal/context/upf.go @@ -69,11 +69,10 @@ type UPF struct { NodeID pfcpType.NodeID Addr string UPIPInfo pfcpType.UserPlaneIPResourceInformation - UPFStatus UPFStatus RecoveryTimeStamp time.Time - Ctx context.Context - CancelFunc context.CancelFunc + AssociationContext context.Context + CancelAssociation context.CancelFunc SNssaiInfos []*SnssaiUPFInfo N3Interfaces []*UPFInterfaceInfo @@ -244,7 +243,9 @@ func NewUPF(nodeID *pfcpType.NodeID, ifaces []*factory.InterfaceUpfInfoItem) (up upfPool.Store(upf.UUID(), upf) // Initialize context - upf.UPFStatus = NotAssociated + upf.AssociationContext, upf.CancelAssociation = context.WithCancel(context.Background()) + upf.CancelAssociation() // necessary to avoid nil pointer for checks of AssociationContext before UPF is associated + upf.NodeID = *nodeID upf.pdrIDGenerator = idgenerator.NewGenerator(1, math.MaxUint16) upf.farIDGenerator = idgenerator.NewGenerator(1, math.MaxUint32) @@ -377,9 +378,11 @@ func (upf *UPF) GetUPFID() string { } func (upf *UPF) pdrID() (uint16, error) { - if upf.UPFStatus != AssociatedSetUpSuccess { - err := fmt.Errorf("UPF[%s] not Associate with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + select { + case <-upf.AssociationContext.Done(): + err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) return 0, err + default: } var pdrID uint16 @@ -393,9 +396,11 @@ func (upf *UPF) pdrID() (uint16, error) { } func (upf *UPF) farID() (uint32, error) { - if upf.UPFStatus != AssociatedSetUpSuccess { - err := fmt.Errorf("UPF[%s] not Associate with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + select { + case <-upf.AssociationContext.Done(): + err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) return 0, err + default: } var farID uint32 @@ -409,9 +414,11 @@ func (upf *UPF) farID() (uint32, error) { } func (upf *UPF) barID() (uint8, error) { - if upf.UPFStatus != AssociatedSetUpSuccess { - err := fmt.Errorf("UPF[%s] not Associate with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + select { + case <-upf.AssociationContext.Done(): + err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) return 0, err + default: } var barID uint8 @@ -425,9 +432,11 @@ func (upf *UPF) barID() (uint8, error) { } func (upf *UPF) qerID() (uint32, error) { - if upf.UPFStatus != AssociatedSetUpSuccess { - err := fmt.Errorf("UPF[%s] not Associate with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + select { + case <-upf.AssociationContext.Done(): + err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) return 0, err + default: } var qerID uint32 @@ -452,9 +461,11 @@ func (upf *UPF) urrID() (uint32, error) { } func (upf *UPF) AddPDR() (*PDR, error) { - if upf.UPFStatus != AssociatedSetUpSuccess { - err := fmt.Errorf("UPF[%s] not Associate with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + select { + case <-upf.AssociationContext.Done(): + err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) return nil, err + default: } pdr := new(PDR) @@ -475,9 +486,11 @@ func (upf *UPF) AddPDR() (*PDR, error) { } func (upf *UPF) AddFAR() (*FAR, error) { - if upf.UPFStatus != AssociatedSetUpSuccess { - err := fmt.Errorf("UPF[%s] not Associate with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + select { + case <-upf.AssociationContext.Done(): + err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) return nil, err + default: } far := new(FAR) @@ -492,9 +505,11 @@ func (upf *UPF) AddFAR() (*FAR, error) { } func (upf *UPF) AddBAR() (*BAR, error) { - if upf.UPFStatus != AssociatedSetUpSuccess { - err := fmt.Errorf("UPF[%s] not Associate with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + select { + case <-upf.AssociationContext.Done(): + err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) return nil, err + default: } bar := new(BAR) @@ -508,9 +523,11 @@ func (upf *UPF) AddBAR() (*BAR, error) { } func (upf *UPF) AddQER() (*QER, error) { - if upf.UPFStatus != AssociatedSetUpSuccess { - err := fmt.Errorf("UPF[%s] not Associate with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + select { + case <-upf.AssociationContext.Done(): + err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) return nil, err + default: } qer := new(QER) @@ -524,9 +541,11 @@ func (upf *UPF) AddQER() (*QER, error) { } func (upf *UPF) AddURR(urrId uint32, opts ...UrrOpt) (*URR, error) { - if upf.UPFStatus != AssociatedSetUpSuccess { - err := fmt.Errorf("UPF[%s] not Associate with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + select { + case <-upf.AssociationContext.Done(): + err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) return nil, err + default: } urr := new(URR) @@ -564,9 +583,10 @@ func (upf *UPF) GetQERById(qerId uint32) *QER { // *** add unit test ***// func (upf *UPF) RemovePDR(pdr *PDR) (err error) { - if upf.UPFStatus != AssociatedSetUpSuccess { - err = fmt.Errorf("UPF[%s] not Associate with SMF", upf.NodeID.ResolveNodeIdToIp().String()) - return err + select { + case <-upf.AssociationContext.Done(): + return fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + default: } upf.pdrIDGenerator.FreeID(int64(pdr.PDRID)) @@ -576,9 +596,10 @@ func (upf *UPF) RemovePDR(pdr *PDR) (err error) { // *** add unit test ***// func (upf *UPF) RemoveFAR(far *FAR) (err error) { - if upf.UPFStatus != AssociatedSetUpSuccess { - err = fmt.Errorf("UPF[%s] not Associate with SMF", upf.NodeID.ResolveNodeIdToIp().String()) - return err + select { + case <-upf.AssociationContext.Done(): + return fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + default: } upf.farIDGenerator.FreeID(int64(far.FARID)) @@ -588,9 +609,10 @@ func (upf *UPF) RemoveFAR(far *FAR) (err error) { // *** add unit test ***// func (upf *UPF) RemoveBAR(bar *BAR) (err error) { - if upf.UPFStatus != AssociatedSetUpSuccess { - err = fmt.Errorf("UPF[%s] not Associate with SMF", upf.NodeID.ResolveNodeIdToIp().String()) - return err + select { + case <-upf.AssociationContext.Done(): + return fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + default: } upf.barIDGenerator.FreeID(int64(bar.BARID)) @@ -600,9 +622,10 @@ func (upf *UPF) RemoveBAR(bar *BAR) (err error) { // *** add unit test ***// func (upf *UPF) RemoveQER(qer *QER) (err error) { - if upf.UPFStatus != AssociatedSetUpSuccess { - err = fmt.Errorf("UPF[%s] not Associate with SMF", upf.NodeID.ResolveNodeIdToIp().String()) - return err + select { + case <-upf.AssociationContext.Done(): + return fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + default: } upf.qerIDGenerator.FreeID(int64(qer.QERID)) diff --git a/internal/context/upf_test.go b/internal/context/upf_test.go index d64837f4..75c08d73 100644 --- a/internal/context/upf_test.go +++ b/internal/context/upf_test.go @@ -1,6 +1,7 @@ package context_test import ( + "context" "fmt" "net" "testing" @@ -9,7 +10,7 @@ import ( "github.com/free5gc/nas/nasMessage" "github.com/free5gc/pfcp/pfcpType" - "github.com/free5gc/smf/internal/context" + smf_context "github.com/free5gc/smf/internal/context" "github.com/free5gc/smf/pkg/factory" ) @@ -45,7 +46,7 @@ func convertPDUSessTypeToString(pduType uint8) string { func TestIP(t *testing.T) { testCases := []struct { - input *context.UPFInterfaceInfo + input *smf_context.UPFInterfaceInfo inputPDUSessionType uint8 paramStr string resultStr string @@ -53,7 +54,7 @@ func TestIP(t *testing.T) { expectedError error }{ { - input: &context.UPFInterfaceInfo{ + input: &smf_context.UPFInterfaceInfo{ NetworkInstances: []string{""}, IPv4EndPointAddresses: []net.IP{net.ParseIP("8.8.8.8")}, IPv6EndPointAddresses: []net.IP{net.ParseIP("2001:4860:4860::8888")}, @@ -65,7 +66,7 @@ func TestIP(t *testing.T) { expectedError: nil, }, { - input: &context.UPFInterfaceInfo{ + input: &smf_context.UPFInterfaceInfo{ NetworkInstances: []string{""}, IPv4EndPointAddresses: []net.IP{net.ParseIP("8.8.8.8")}, IPv6EndPointAddresses: []net.IP{net.ParseIP("2001:4860:4860::8888")}, @@ -101,14 +102,14 @@ func TestIP(t *testing.T) { func TestAddDataPath(t *testing.T) { // AddDataPath is simple, should only have one case testCases := []struct { - tunnel *context.UPTunnel - addedDataPath *context.DataPath + tunnel *smf_context.UPTunnel + addedDataPath *smf_context.DataPath resultStr string expectedExist bool }{ { - tunnel: context.NewUPTunnel(), - addedDataPath: context.NewDataPath(), + tunnel: smf_context.NewUPTunnel(), + addedDataPath: smf_context.NewDataPath(), resultStr: "Datapath should exist", expectedExist: true, }, @@ -138,23 +139,23 @@ func TestAddDataPath(t *testing.T) { func TestAddPDR(t *testing.T) { testCases := []struct { - upf *context.UPF + upf *smf_context.UPF resultStr string expectedError error }{ { - upf: context.NewUPF(mockIPv4NodeID, mockIfaces), + upf: smf_context.NewUPF(mockIPv4NodeID, mockIfaces), resultStr: "AddPDR should success", expectedError: nil, }, { - upf: context.NewUPF(mockIPv4NodeID, mockIfaces), + upf: smf_context.NewUPF(mockIPv4NodeID, mockIfaces), resultStr: "AddPDR should fail", - expectedError: fmt.Errorf("UPF[127.0.0.1] not Associate with SMF"), + expectedError: fmt.Errorf("UPF[127.0.0.1] not associated with SMF"), }, } - testCases[0].upf.UPFStatus = context.AssociatedSetUpSuccess + testCases[0].upf.AssociationContext = context.Background() Convey("AddPDR should indeed add PDR and report error appropiately", t, func() { for i, testcase := range testCases { @@ -181,23 +182,23 @@ func TestAddPDR(t *testing.T) { func TestAddFAR(t *testing.T) { testCases := []struct { - upf *context.UPF + upf *smf_context.UPF resultStr string expectedError error }{ { - upf: context.NewUPF(mockIPv4NodeID, mockIfaces), + upf: smf_context.NewUPF(mockIPv4NodeID, mockIfaces), resultStr: "AddFAR should success", expectedError: nil, }, { - upf: context.NewUPF(mockIPv4NodeID, mockIfaces), + upf: smf_context.NewUPF(mockIPv4NodeID, mockIfaces), resultStr: "AddFAR should fail", - expectedError: fmt.Errorf("UPF[127.0.0.1] not Associate with SMF"), + expectedError: fmt.Errorf("UPF[127.0.0.1] not associated with SMF"), }, } - testCases[0].upf.UPFStatus = context.AssociatedSetUpSuccess + testCases[0].upf.AssociationContext = context.Background() Convey("AddFAR should indeed add FAR and report error appropiately", t, func() { for i, testcase := range testCases { @@ -224,23 +225,23 @@ func TestAddFAR(t *testing.T) { func TestAddQER(t *testing.T) { testCases := []struct { - upf *context.UPF + upf *smf_context.UPF resultStr string expectedError error }{ { - upf: context.NewUPF(mockIPv4NodeID, mockIfaces), + upf: smf_context.NewUPF(mockIPv4NodeID, mockIfaces), resultStr: "AddQER should success", expectedError: nil, }, { - upf: context.NewUPF(mockIPv4NodeID, mockIfaces), + upf: smf_context.NewUPF(mockIPv4NodeID, mockIfaces), resultStr: "AddQER should fail", - expectedError: fmt.Errorf("UPF[127.0.0.1] not Associate with SMF"), + expectedError: fmt.Errorf("UPF[127.0.0.1] not associated with SMF"), }, } - testCases[0].upf.UPFStatus = context.AssociatedSetUpSuccess + testCases[0].upf.AssociationContext = context.Background() Convey("AddQER should indeed add QER and report error appropiately", t, func() { for i, testcase := range testCases { @@ -267,23 +268,23 @@ func TestAddQER(t *testing.T) { func TestAddBAR(t *testing.T) { testCases := []struct { - upf *context.UPF + upf *smf_context.UPF resultStr string expectedError error }{ { - upf: context.NewUPF(mockIPv4NodeID, mockIfaces), + upf: smf_context.NewUPF(mockIPv4NodeID, mockIfaces), resultStr: "AddBAR should success", expectedError: nil, }, { - upf: context.NewUPF(mockIPv4NodeID, mockIfaces), + upf: smf_context.NewUPF(mockIPv4NodeID, mockIfaces), resultStr: "AddBAR should fail", - expectedError: fmt.Errorf("UPF[127.0.0.1] not Associate with SMF"), + expectedError: fmt.Errorf("UPF[127.0.0.1] not associated with SMF"), }, } - testCases[0].upf.UPFStatus = context.AssociatedSetUpSuccess + testCases[0].upf.AssociationContext = context.Background() Convey("AddBAR should indeed add BAR and report error appropiately", t, func() { for i, testcase := range testCases { diff --git a/internal/context/user_plane_information.go b/internal/context/user_plane_information.go index 91f2bab3..4af26731 100644 --- a/internal/context/user_plane_information.go +++ b/internal/context/user_plane_information.go @@ -877,10 +877,12 @@ func (upi *UserPlaneInformation) SelectUPFAndAllocUEIP(selection *UPFSelectionPa for _, upf := range sortedUPFList { logger.CtxLog.Debugf("check start UPF: %s", upi.GetUPFNameByIp(upf.NodeID.ResolveNodeIdToIp().String())) - if upf.UPF.UPFStatus != AssociatedSetUpSuccess { + select { + case <-upf.UPF.AssociationContext.Done(): logger.CtxLog.Infof("PFCP Association not yet Established with: %s", upi.GetUPFNameByIp(upf.NodeID.ResolveNodeIdToIp().String())) continue + default: } pools, useStaticIPPool := getUEIPPool(upf, selection) if len(pools) == 0 { diff --git a/internal/context/user_plane_information_test.go b/internal/context/user_plane_information_test.go index d27158a8..2a07c983 100644 --- a/internal/context/user_plane_information_test.go +++ b/internal/context/user_plane_information_test.go @@ -1,6 +1,7 @@ package context_test import ( + "context" "fmt" "net" "testing" @@ -8,7 +9,7 @@ import ( "github.com/stretchr/testify/require" "github.com/free5gc/openapi/models" - "github.com/free5gc/smf/internal/context" + smf_context "github.com/free5gc/smf/internal/context" "github.com/free5gc/smf/pkg/factory" ) @@ -149,7 +150,7 @@ var configuration = &factory.UserPlaneInformation{ } func TestNewUserPlaneInformation(t *testing.T) { - userplaneInformation := context.NewUserPlaneInformation(configuration) + userplaneInformation := smf_context.NewUserPlaneInformation(configuration) require.NotNil(t, userplaneInformation.AccessNetwork["GNodeB"]) @@ -188,13 +189,13 @@ func TestGenerateDefaultPath(t *testing.T) { testCases := []struct { name string - param *context.UPFSelectionParams + param *smf_context.UPFSelectionParams expected bool }{ { "S-NSSAI 01112232 and DNN internet ok", - &context.UPFSelectionParams{ - SNssai: &context.SNssai{ + &smf_context.UPFSelectionParams{ + SNssai: &smf_context.SNssai{ Sst: 1, Sd: "112232", }, @@ -204,8 +205,8 @@ func TestGenerateDefaultPath(t *testing.T) { }, { "S-NSSAI 02112233 and DNN internet ok", - &context.UPFSelectionParams{ - SNssai: &context.SNssai{ + &smf_context.UPFSelectionParams{ + SNssai: &smf_context.SNssai{ Sst: 2, Sd: "112233", }, @@ -215,8 +216,8 @@ func TestGenerateDefaultPath(t *testing.T) { }, { "S-NSSAI 03112234 and DNN internet ok", - &context.UPFSelectionParams{ - SNssai: &context.SNssai{ + &smf_context.UPFSelectionParams{ + SNssai: &smf_context.SNssai{ Sst: 3, Sd: "112234", }, @@ -226,8 +227,8 @@ func TestGenerateDefaultPath(t *testing.T) { }, { "S-NSSAI 01112235 and DNN internet ok", - &context.UPFSelectionParams{ - SNssai: &context.SNssai{ + &smf_context.UPFSelectionParams{ + SNssai: &smf_context.SNssai{ Sst: 1, Sd: "112235", }, @@ -237,8 +238,8 @@ func TestGenerateDefaultPath(t *testing.T) { }, { "S-NSSAI 01010203 and DNN internet fail", - &context.UPFSelectionParams{ - SNssai: &context.SNssai{ + &smf_context.UPFSelectionParams{ + SNssai: &smf_context.SNssai{ Sst: 1, Sd: "010203", }, @@ -248,7 +249,7 @@ func TestGenerateDefaultPath(t *testing.T) { }, } - userplaneInformation := context.NewUserPlaneInformation(&config1) + userplaneInformation := smf_context.NewUserPlaneInformation(&config1) for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { pathExist := userplaneInformation.GenerateDefaultPath(tc.param) @@ -267,15 +268,15 @@ func TestSelectUPFAndAllocUEIP(t *testing.T) { expectedIPPool = append(expectedIPPool, net.ParseIP(fmt.Sprintf("10.60.0.%d", i)).To4()) } - userplaneInformation := context.NewUserPlaneInformation(configuration) + userplaneInformation := smf_context.NewUserPlaneInformation(configuration) for _, upf := range userplaneInformation.UPFs { - upf.UPF.UPFStatus = context.AssociatedSetUpSuccess + upf.UPF.AssociationContext = context.Background() } for i := 0; i <= 100; i++ { - upf, allocatedIP, _ := userplaneInformation.SelectUPFAndAllocUEIP(&context.UPFSelectionParams{ + upf, allocatedIP, _ := userplaneInformation.SelectUPFAndAllocUEIP(&smf_context.UPFSelectionParams{ Dnn: "internet", - SNssai: &context.SNssai{ + SNssai: &smf_context.SNssai{ Sst: 1, Sd: "112232", }, @@ -393,16 +394,16 @@ var configForIPPoolAllocate = &factory.UserPlaneInformation{ var testCasesOfGetUEIPPool = []struct { name string allocateTimes int - param *context.UPFSelectionParams + param *smf_context.UPFSelectionParams subnet uint8 useStaticIP bool }{ { name: "static IP not in dynamic pool or static pool", allocateTimes: 1, - param: &context.UPFSelectionParams{ + param: &smf_context.UPFSelectionParams{ Dnn: "internet", - SNssai: &context.SNssai{ + SNssai: &smf_context.SNssai{ Sst: 1, Sd: "111111", }, @@ -414,9 +415,9 @@ var testCasesOfGetUEIPPool = []struct { { name: "static IP not in static pool but in dynamic pool", allocateTimes: 1, - param: &context.UPFSelectionParams{ + param: &smf_context.UPFSelectionParams{ Dnn: "internet", - SNssai: &context.SNssai{ + SNssai: &smf_context.SNssai{ Sst: 2, Sd: "222222", }, @@ -428,9 +429,9 @@ var testCasesOfGetUEIPPool = []struct { { name: "dynamic pool is exhausted", allocateTimes: 2, - param: &context.UPFSelectionParams{ + param: &smf_context.UPFSelectionParams{ Dnn: "internet", - SNssai: &context.SNssai{ + SNssai: &smf_context.SNssai{ Sst: 2, Sd: "222222", }, @@ -442,9 +443,9 @@ var testCasesOfGetUEIPPool = []struct { { name: "static IP is in static pool", allocateTimes: 1, - param: &context.UPFSelectionParams{ + param: &smf_context.UPFSelectionParams{ Dnn: "internet", - SNssai: &context.SNssai{ + SNssai: &smf_context.SNssai{ Sst: 3, Sd: "333333", }, @@ -456,9 +457,9 @@ var testCasesOfGetUEIPPool = []struct { { name: "static pool is exhausted", allocateTimes: 2, - param: &context.UPFSelectionParams{ + param: &smf_context.UPFSelectionParams{ Dnn: "internet", - SNssai: &context.SNssai{ + SNssai: &smf_context.SNssai{ Sst: 3, Sd: "333333", }, @@ -470,9 +471,9 @@ var testCasesOfGetUEIPPool = []struct { { name: "static IP is in static pool, and dynamic pool is exhaust(allocate twice and not release)", allocateTimes: 2, - param: &context.UPFSelectionParams{ + param: &smf_context.UPFSelectionParams{ Dnn: "internet", - SNssai: &context.SNssai{ + SNssai: &smf_context.SNssai{ Sst: 3, Sd: "333333", }, @@ -484,9 +485,9 @@ var testCasesOfGetUEIPPool = []struct { } func TestGetUEIPPool(t *testing.T) { - userplaneInformation := context.NewUserPlaneInformation(configForIPPoolAllocate) + userplaneInformation := smf_context.NewUserPlaneInformation(configForIPPoolAllocate) for _, upf := range userplaneInformation.UPFs { - upf.UPF.UPFStatus = context.AssociatedSetUpSuccess + upf.UPF.AssociationContext = context.Background() } for ci, tc := range testCasesOfGetUEIPPool { @@ -498,7 +499,7 @@ func TestGetUEIPPool(t *testing.T) { } } - var upf *context.UPNode + var upf *smf_context.UPNode var allocatedIP net.IP var useStatic bool for times := 1; times <= tc.allocateTimes; times++ { diff --git a/internal/pfcp/handler/handler.go b/internal/pfcp/handler/handler.go index ea53c001..e8c086b5 100644 --- a/internal/pfcp/handler/handler.go +++ b/internal/pfcp/handler/handler.go @@ -124,12 +124,14 @@ func HandlePfcpSessionReportRequest(msg *pfcpUdp.Message) { pfcp_message.SendPfcpSessionReportResponse(msg.RemoteAddr, cause, seqFromUPF, 0) return } - if upf.UPFStatus != smf_context.AssociatedSetUpSuccess { + select { + case <-upf.AssociationContext.Done(): logger.PfcpLog.Warnf("PFCP Session Report Request : Not Associated with UPF[%s], Request Rejected", upfNodeIDtoIPStr) cause.CauseValue = pfcpType.CauseNoEstablishedPfcpAssociation pfcp_message.SendPfcpSessionReportResponse(msg.RemoteAddr, cause, seqFromUPF, 0) return + default: } if smContext.UpCnxState == models.UpCnxState_DEACTIVATED { diff --git a/internal/pfcp/message/send.go b/internal/pfcp/message/send.go index 6f9f71e2..d38fe5e2 100644 --- a/internal/pfcp/message/send.go +++ b/internal/pfcp/message/send.go @@ -140,8 +140,10 @@ func SendPfcpSessionEstablishmentRequest( urrList []*context.URR, ) (resMsg *pfcpUdp.Message, err error) { nodeIDtoIP := upf.NodeID.ResolveNodeIdToIp() - if upf.UPFStatus != context.AssociatedSetUpSuccess { + select { + case <-upf.AssociationContext.Done(): return nil, fmt.Errorf("Not Associated with UPF[%s]", nodeIDtoIP.String()) + default: } pfcpMsg, err := BuildPfcpSessionEstablishmentRequest(upf.NodeID, nodeIDtoIP.String(), @@ -223,8 +225,10 @@ func SendPfcpSessionModificationRequest( urrList []*context.URR, ) (resMsg *pfcpUdp.Message, err error) { nodeIDtoIP := upf.NodeID.ResolveNodeIdToIp() - if upf.UPFStatus != context.AssociatedSetUpSuccess { + select { + case <-upf.AssociationContext.Done(): return nil, fmt.Errorf("Not Associated with UPF[%s]", nodeIDtoIP.String()) + default: } pfcpMsg, err := BuildPfcpSessionModificationRequest(upf.NodeID, nodeIDtoIP.String(), @@ -298,8 +302,10 @@ func SendPfcpSessionModificationResponse(addr *net.UDPAddr) { func SendPfcpSessionDeletionRequest(upf *context.UPF, ctx *context.SMContext) (resMsg *pfcpUdp.Message, err error) { nodeIDtoIP := upf.NodeID.ResolveNodeIdToIp() - if upf.UPFStatus != context.AssociatedSetUpSuccess { + select { + case <-upf.AssociationContext.Done(): return nil, fmt.Errorf("Not Associated with UPF[%s]", nodeIDtoIP.String()) + default: } pfcpMsg, err := BuildPfcpSessionDeletionRequest() diff --git a/internal/pfcp/message/send_test.go b/internal/pfcp/message/send_test.go index cf6cd33c..a114d0c3 100644 --- a/internal/pfcp/message/send_test.go +++ b/internal/pfcp/message/send_test.go @@ -24,9 +24,8 @@ func TestSendPfcpSessionEstablishmentRequest(t *testing.T) { } func TestSendHeartbeatResponse(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - smf_context.GetSelf().Ctx = ctx - smf_context.GetSelf().PFCPCancelFunc = cancel + smfContext := smf_context.GetSelf() + smfContext.PfcpContext, smfContext.PfcpCancelFunc = context.WithCancel(context.Background()) udp.Run(smf_pfcp.Dispatch) udp.ServerStartTime = time.Now() diff --git a/internal/pfcp/udp/udp.go b/internal/pfcp/udp/udp.go index dcdf12ab..3e1e8685 100644 --- a/internal/pfcp/udp/udp.go +++ b/internal/pfcp/udp/udp.go @@ -1,7 +1,6 @@ package udp import ( - "context" "errors" "net" "runtime/debug" @@ -18,8 +17,6 @@ const MaxPfcpUdpDataSize = 1024 var Server *pfcpUdp.PfcpServer -var cancelFunc *context.CancelFunc - var ServerStartTime time.Time func Run(dispatch func(*pfcpUdp.Message)) { @@ -30,10 +27,9 @@ func Run(dispatch func(*pfcpUdp.Message)) { } }() - newCtx, newCancelFunc := context.WithCancel(smf_context.GetSelf().Ctx) - cancelFunc = &newCancelFunc + smfContext := smf_context.GetSelf() - serverIP := smf_context.GetSelf().ListenIP().To4() + serverIP := smfContext.ListenIP().To4() Server = pfcpUdp.NewPfcpServer(serverIP.String()) err := Server.Listen() @@ -61,7 +57,7 @@ func Run(dispatch func(*pfcpUdp.Message)) { } else { logger.PfcpLog.Warnf("Read PFCP error: %v, msg: [%v]", errReadFrom, msg) select { - case <-newCtx.Done(): + case <-smfContext.PfcpContext.Done(): // PFCP is closing return default: @@ -94,7 +90,8 @@ func SendPfcpRequest(sndMsg *pfcp.Message, addr *net.UDPAddr) (rsvMsg *pfcpUdp.M } func ClosePfcp() error { - (*cancelFunc)() + smf_context.GetSelf().PfcpCancelFunc() + closeErr := Server.Close() if closeErr != nil { logger.PfcpLog.Errorf("Pfcp close err: %+v", closeErr) diff --git a/internal/pfcp/udp/udp_test.go b/internal/pfcp/udp/udp_test.go index 5e98b8eb..07f084d5 100644 --- a/internal/pfcp/udp/udp_test.go +++ b/internal/pfcp/udp/udp_test.go @@ -21,16 +21,16 @@ const testPfcpClientPort = 12345 func TestRun(t *testing.T) { // Set SMF Node ID - smf_context.GetSelf().CPNodeID = pfcpType.NodeID{ + smfContext := smf_context.GetSelf() + + smfContext.CPNodeID = pfcpType.NodeID{ NodeIdType: pfcpType.NodeIdTypeIpv4Address, IP: net.ParseIP("127.0.0.1").To4(), } - smf_context.GetSelf().ExternalAddr = "127.0.0.1" - smf_context.GetSelf().ListenAddr = "127.0.0.1" + smfContext.ExternalAddr = "127.0.0.1" + smfContext.ListenAddr = "127.0.0.1" - ctx, cancel := context.WithCancel(context.Background()) - smf_context.GetSelf().Ctx = ctx - smf_context.GetSelf().PFCPCancelFunc = cancel + smfContext.PfcpContext, smfContext.PfcpCancelFunc = context.WithCancel(context.Background()) udp.Run(smf_pfcp.Dispatch) testPfcpReq := pfcp.Message{ diff --git a/internal/sbi/api_upi.go b/internal/sbi/api_upi.go index 05030383..61ff2ea8 100644 --- a/internal/sbi/api_upi.go +++ b/internal/sbi/api_upi.go @@ -1,7 +1,6 @@ package sbi import ( - "context" "net/http" "github.com/gin-gonic/gin" @@ -69,9 +68,10 @@ func (s *Server) PostUpNodesLinks(c *gin.Context) { for _, upf := range upi.UPFs { // only associate new ones - if upf.UPF.UPFStatus == smf_context.NotAssociated { - upf.UPF.Ctx, upf.UPF.CancelFunc = context.WithCancel(context.Background()) - go s.Processor().ToBeAssociatedWithUPF(smf_context.GetSelf().Ctx, upf.UPF) + select { + case <-upf.UPF.AssociationContext.Done(): + go s.Processor().ToBeAssociatedWithUPF(smf_context.GetSelf().PfcpContext, upf.UPF) + default: } } c.JSON(http.StatusOK, gin.H{"status": "OK"}) @@ -91,7 +91,7 @@ func (s *Server) DeleteUpNodeLink(c *gin.Context) { go s.Processor().ReleaseAllResourcesOfUPF(upNode.UPF) } upi.UpNodeDelete(upNodeRef) - upNode.UPF.CancelFunc() + upNode.UPF.CancelAssociation() c.JSON(http.StatusOK, gin.H{"status": "OK"}) } else { c.JSON(http.StatusNotFound, gin.H{}) diff --git a/internal/sbi/processor/association.go b/internal/sbi/processor/association.go index 10c2bf5f..02ec1962 100644 --- a/internal/sbi/processor/association.go +++ b/internal/sbi/processor/association.go @@ -15,7 +15,7 @@ import ( "github.com/free5gc/smf/internal/pfcp/message" ) -func (p *Processor) ToBeAssociatedWithUPF(ctx context.Context, upf *smf_context.UPF) { +func (p *Processor) ToBeAssociatedWithUPF(smfPfcpContext context.Context, upf *smf_context.UPF) { var upfStr string if upf.NodeID.NodeIdType == pfcpType.NodeIdTypeFqdn { upfStr = fmt.Sprintf("[%s](%s)", upf.NodeID.FQDN, upf.NodeID.ResolveNodeIdToIp().String()) @@ -24,24 +24,21 @@ func (p *Processor) ToBeAssociatedWithUPF(ctx context.Context, upf *smf_context. } for { - ensureSetupPfcpAssociation(ctx, upf, upfStr) - if isDone(ctx, upf) { - break - } - - if smf_context.GetSelf().PfcpHeartbeatInterval == 0 { + // check if SMF PFCP context (parent) was cancelled + // note: UPF AssociationContexts are children of smfPfcpContext + select { + case <-smfPfcpContext.Done(): + logger.MainLog.Infoln("Cancelled SMF PFCP context") return - } - - keepHeartbeatTo(ctx, upf, upfStr) - // return when UPF heartbeat lost is detected or association is canceled - if isDone(ctx, upf) { - break - } + default: + ensureSetupPfcpAssociation(smfPfcpContext, upf, upfStr) + if smf_context.GetSelf().PfcpHeartbeatInterval == 0 { + return + } + keepHeartbeatTo(upf, upfStr) + // returns when UPF heartbeat loss is detected or association is cancelled - p.releaseAllResourcesOfUPF(upf, upfStr) - if isDone(ctx, upf) { - break + p.releaseAllResourcesOfUPF(upf, upfStr) } } } @@ -56,41 +53,30 @@ func (p *Processor) ReleaseAllResourcesOfUPF(upf *smf_context.UPF) { p.releaseAllResourcesOfUPF(upf, upfStr) } -func isDone(ctx context.Context, upf *smf_context.UPF) bool { - select { - case <-ctx.Done(): - return true - case <-upf.Ctx.Done(): - return true - default: - return false - } -} - -func ensureSetupPfcpAssociation(ctx context.Context, upf *smf_context.UPF, upfStr string) { +func ensureSetupPfcpAssociation(parentContext context.Context, upf *smf_context.UPF, upfStr string) { alertTime := time.Now() alertInterval := smf_context.GetSelf().AssocFailAlertInterval retryInterval := smf_context.GetSelf().AssocFailRetryInterval for { - timer := time.After(retryInterval) err := setupPfcpAssociation(upf, upfStr) if err == nil { + // success + // assign UPF an AssociationContext, with SMF PFCP Context as parent + upf.AssociationContext, upf.CancelAssociation = context.WithCancel(parentContext) return } - logger.MainLog.Warnf("Failed to setup an association with UPF%s, error:%+v", upfStr, err) + logger.MainLog.Warnf("Failed to setup an association with UPF[%s], error:%+v", upfStr, err) now := time.Now() logger.MainLog.Debugf("now %+v, alertTime %+v", now, alertTime) if now.After(alertTime.Add(alertInterval)) { - logger.MainLog.Errorf("ALERT for UPF%s", upfStr) + logger.MainLog.Errorf("ALERT for UPF[%s]", upfStr) alertTime = now } - logger.MainLog.Debugf("Wait %+v (or less) until next retry attempt", retryInterval) - select { - case <-ctx.Done(): - logger.MainLog.Infof("Canceled association request to UPF%s", upfStr) - return - case <-upf.Ctx.Done(): - logger.MainLog.Infof("Canceled association request to this UPF%s only", upfStr) + logger.MainLog.Debugf("Wait %+v until next retry attempt", retryInterval) + timer := time.After(retryInterval) + select { // no default case, either case needs to be true to continue + case <-parentContext.Done(): + logger.MainLog.Infoln("Cancelled SMF PFCP context") return case <-timer: continue @@ -119,7 +105,7 @@ func setupPfcpAssociation(upf *smf_context.UPF, upfStr string) error { logger.MainLog.Infof("Received PFCP Association Setup Accepted Response from UPF%s", upfStr) - upf.UPFStatus = smf_context.AssociatedSetUpSuccess + //upf.UPFStatus = smf_context.AssociatedSetUpSuccess if rsp.UserPlaneIPResourceInformation != nil { upf.UPIPInfo = *rsp.UserPlaneIPResourceInformation @@ -131,7 +117,7 @@ func setupPfcpAssociation(upf *smf_context.UPF, upfStr string) error { return nil } -func keepHeartbeatTo(ctx context.Context, upf *smf_context.UPF, upfStr string) { +func keepHeartbeatTo(upf *smf_context.UPF, upfStr string) { for { err := doPfcpHeartbeat(upf, upfStr) if err != nil { @@ -141,11 +127,8 @@ func keepHeartbeatTo(ctx context.Context, upf *smf_context.UPF, upfStr string) { timer := time.After(smf_context.GetSelf().PfcpHeartbeatInterval) select { - case <-ctx.Done(): - logger.MainLog.Infof("Canceled Heartbeat with UPF%s", upfStr) - return - case <-upf.Ctx.Done(): - logger.MainLog.Infof("Canceled Heartbeat to this UPF%s only", upfStr) + case <-upf.AssociationContext.Done(): + logger.MainLog.Infof("Cancelled association to UPF[%s]", upfStr) return case <-timer: continue @@ -154,15 +137,17 @@ func keepHeartbeatTo(ctx context.Context, upf *smf_context.UPF, upfStr string) { } func doPfcpHeartbeat(upf *smf_context.UPF, upfStr string) error { - if upf.UPFStatus != smf_context.AssociatedSetUpSuccess { - return fmt.Errorf("invalid status of UPF%s: %d", upfStr, upf.UPFStatus) + select { + case <-upf.AssociationContext.Done(): + return fmt.Errorf("Cancel heartbeat, UPF[%s] is not associted", upfStr) + default: } logger.MainLog.Debugf("Sending PFCP Heartbeat Request to UPF%s", upfStr) resMsg, err := message.SendPfcpHeartbeatRequest(upf) if err != nil { - upf.UPFStatus = smf_context.NotAssociated + upf.CancelAssociation() upf.RecoveryTimeStamp = time.Time{} return fmt.Errorf("SendPfcpHeartbeatRequest error: %w", err) } @@ -179,7 +164,7 @@ func doPfcpHeartbeat(upf *smf_context.UPF, upfStr string) error { upf.RecoveryTimeStamp = rsp.RecoveryTimeStamp.RecoveryTimeStamp } else if upf.RecoveryTimeStamp.Before(rsp.RecoveryTimeStamp.RecoveryTimeStamp) { // received a newer recovery timestamp - upf.UPFStatus = smf_context.NotAssociated + upf.CancelAssociation() upf.RecoveryTimeStamp = time.Time{} return fmt.Errorf("received PFCP Heartbeat Response RecoveryTimeStamp has been updated") } diff --git a/internal/sbi/processor/pdu_session_test.go b/internal/sbi/processor/pdu_session_test.go index a8153c47..4e811910 100644 --- a/internal/sbi/processor/pdu_session_test.go +++ b/internal/sbi/processor/pdu_session_test.go @@ -366,9 +366,8 @@ func initDiscAMFStubNRF() { } func initStubPFCP() { - ctx, cancel := context.WithCancel(context.Background()) - smf_context.GetSelf().Ctx = ctx - smf_context.GetSelf().PFCPCancelFunc = cancel + smfContext := smf_context.GetSelf() + smfContext.PfcpContext, smfContext.PfcpCancelFunc = context.WithCancel(context.Background()) udp.Run(pfcp.Dispatch) } @@ -449,7 +448,7 @@ func TestHandlePDUSessionSMContextCreate(t *testing.T) { // modify associate setup status allUPFs := smf_context.GetSelf().UserPlaneInformation.UPFs for _, upfNode := range allUPFs { - upfNode.UPF.UPFStatus = smf_context.AssociatedSetUpSuccess + upfNode.UPF.AssociationContext = context.Background() } testCases := []struct { diff --git a/pkg/service/init.go b/pkg/service/init.go index 0a4341a1..00ec52cb 100644 --- a/pkg/service/init.go +++ b/pkg/service/init.go @@ -88,9 +88,8 @@ func NewApp( smf.ctx, smf.cancel = context.WithCancel(ctx) // for PFCP - ctx, cancel := context.WithCancel(smf.ctx) - smf_context.GetSelf().Ctx = ctx - smf_context.GetSelf().PFCPCancelFunc = cancel + smfContext := smf_context.GetSelf() + smfContext.PfcpContext, smfContext.PfcpCancelFunc = context.WithCancel(smf.ctx) SMF = smf diff --git a/pkg/utils/pfcp_util.go b/pkg/utils/pfcp_util.go index f047fdca..f7a66334 100644 --- a/pkg/utils/pfcp_util.go +++ b/pkg/utils/pfcp_util.go @@ -11,17 +11,17 @@ import ( "github.com/free5gc/smf/pkg/service" ) -var ( - pfcpStart func(a *service.SmfApp) - pfcpStop func() -) +//var ( +// pfcpStart func(a *service.SmfApp) +// pfcpStop func() +//) func InitPFCPFunc() (func(a *service.SmfApp), func()) { - pfcpStart = func(a *service.SmfApp) { + smfContext := smf_context.GetSelf() + + pfcpStart := func(a *service.SmfApp) { // Initialize PFCP server - ctx, cancel := context.WithCancel(context.Background()) - smf_context.GetSelf().Ctx = ctx - smf_context.GetSelf().PFCPCancelFunc = cancel + smfContext.PfcpContext, smfContext.PfcpCancelFunc = context.WithCancel(context.Background()) udp.Run(pfcp.Dispatch) @@ -29,13 +29,12 @@ func InitPFCPFunc() (func(a *service.SmfApp), func()) { time.Sleep(1000 * time.Millisecond) for _, upNode := range smf_context.GetSelf().UserPlaneInformation.UPFs { - upNode.UPF.Ctx, upNode.UPF.CancelFunc = context.WithCancel(ctx) - go a.Processor().ToBeAssociatedWithUPF(ctx, upNode.UPF) + go a.Processor().ToBeAssociatedWithUPF(smfContext.PfcpContext, upNode.UPF) } } - pfcpStop = func() { - smf_context.GetSelf().PFCPCancelFunc() + pfcpStop := func() { + smfContext.PfcpCancelFunc() err := udp.Server.Close() if err != nil { logger.Log.Errorf("udp server close failed %+v", err) From 697d2b3fffc48e9b2ae25fd8e24360bd35ec0331 Mon Sep 17 00:00:00 2001 From: Laura Henning Date: Wed, 18 Sep 2024 15:29:44 +0000 Subject: [PATCH 2/9] chore: linting --- internal/sbi/processor/association.go | 10 +++++----- pkg/utils/pfcp_util.go | 5 ----- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/internal/sbi/processor/association.go b/internal/sbi/processor/association.go index 02ec1962..424f5a15 100644 --- a/internal/sbi/processor/association.go +++ b/internal/sbi/processor/association.go @@ -24,11 +24,11 @@ func (p *Processor) ToBeAssociatedWithUPF(smfPfcpContext context.Context, upf *s } for { - // check if SMF PFCP context (parent) was cancelled + // check if SMF PFCP context (parent) was canceled // note: UPF AssociationContexts are children of smfPfcpContext select { case <-smfPfcpContext.Done(): - logger.MainLog.Infoln("Cancelled SMF PFCP context") + logger.MainLog.Infoln("Canceled SMF PFCP context") return default: ensureSetupPfcpAssociation(smfPfcpContext, upf, upfStr) @@ -36,7 +36,7 @@ func (p *Processor) ToBeAssociatedWithUPF(smfPfcpContext context.Context, upf *s return } keepHeartbeatTo(upf, upfStr) - // returns when UPF heartbeat loss is detected or association is cancelled + // returns when UPF heartbeat loss is detected or association is canceled p.releaseAllResourcesOfUPF(upf, upfStr) } @@ -76,7 +76,7 @@ func ensureSetupPfcpAssociation(parentContext context.Context, upf *smf_context. timer := time.After(retryInterval) select { // no default case, either case needs to be true to continue case <-parentContext.Done(): - logger.MainLog.Infoln("Cancelled SMF PFCP context") + logger.MainLog.Infoln("Canceled SMF PFCP context") return case <-timer: continue @@ -128,7 +128,7 @@ func keepHeartbeatTo(upf *smf_context.UPF, upfStr string) { timer := time.After(smf_context.GetSelf().PfcpHeartbeatInterval) select { case <-upf.AssociationContext.Done(): - logger.MainLog.Infof("Cancelled association to UPF[%s]", upfStr) + logger.MainLog.Infof("Canceled association to UPF[%s]", upfStr) return case <-timer: continue diff --git a/pkg/utils/pfcp_util.go b/pkg/utils/pfcp_util.go index f7a66334..01719b92 100644 --- a/pkg/utils/pfcp_util.go +++ b/pkg/utils/pfcp_util.go @@ -11,11 +11,6 @@ import ( "github.com/free5gc/smf/pkg/service" ) -//var ( -// pfcpStart func(a *service.SmfApp) -// pfcpStop func() -//) - func InitPFCPFunc() (func(a *service.SmfApp), func()) { smfContext := smf_context.GetSelf() From ca0bec038f9706dec225ad8ef1549c64ce4f59fa Mon Sep 17 00:00:00 2001 From: Laura Henning Date: Fri, 20 Sep 2024 11:08:30 +0000 Subject: [PATCH 3/9] chore: linting --- internal/sbi/processor/association.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/sbi/processor/association.go b/internal/sbi/processor/association.go index 424f5a15..20fad749 100644 --- a/internal/sbi/processor/association.go +++ b/internal/sbi/processor/association.go @@ -105,8 +105,6 @@ func setupPfcpAssociation(upf *smf_context.UPF, upfStr string) error { logger.MainLog.Infof("Received PFCP Association Setup Accepted Response from UPF%s", upfStr) - //upf.UPFStatus = smf_context.AssociatedSetUpSuccess - if rsp.UserPlaneIPResourceInformation != nil { upf.UPIPInfo = *rsp.UserPlaneIPResourceInformation From 0f052c77564c860d7bfa5b0f52ac0e5e042964a2 Mon Sep 17 00:00:00 2001 From: Laura Henning Date: Mon, 23 Sep 2024 08:26:14 +0000 Subject: [PATCH 4/9] refactor: add util function IsAssociated() to UPF --- internal/context/upf.go | 79 ++++++++-------------- internal/context/user_plane_information.go | 8 +-- internal/pfcp/handler/handler.go | 8 +-- internal/pfcp/message/send.go | 18 ++--- internal/sbi/api_upi.go | 4 +- internal/sbi/processor/association.go | 6 +- 6 files changed, 41 insertions(+), 82 deletions(-) diff --git a/internal/context/upf.go b/internal/context/upf.go index ff25b28e..21c8628e 100644 --- a/internal/context/upf.go +++ b/internal/context/upf.go @@ -378,11 +378,8 @@ func (upf *UPF) GetUPFID() string { } func (upf *UPF) pdrID() (uint16, error) { - select { - case <-upf.AssociationContext.Done(): - err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + if err := upf.IsAssociated(); err != nil { return 0, err - default: } var pdrID uint16 @@ -396,11 +393,8 @@ func (upf *UPF) pdrID() (uint16, error) { } func (upf *UPF) farID() (uint32, error) { - select { - case <-upf.AssociationContext.Done(): - err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + if err := upf.IsAssociated(); err != nil { return 0, err - default: } var farID uint32 @@ -414,11 +408,8 @@ func (upf *UPF) farID() (uint32, error) { } func (upf *UPF) barID() (uint8, error) { - select { - case <-upf.AssociationContext.Done(): - err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + if err := upf.IsAssociated(); err != nil { return 0, err - default: } var barID uint8 @@ -432,11 +423,8 @@ func (upf *UPF) barID() (uint8, error) { } func (upf *UPF) qerID() (uint32, error) { - select { - case <-upf.AssociationContext.Done(): - err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + if err := upf.IsAssociated(); err != nil { return 0, err - default: } var qerID uint32 @@ -461,11 +449,8 @@ func (upf *UPF) urrID() (uint32, error) { } func (upf *UPF) AddPDR() (*PDR, error) { - select { - case <-upf.AssociationContext.Done(): - err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + if err := upf.IsAssociated(); err != nil { return nil, err - default: } pdr := new(PDR) @@ -486,11 +471,8 @@ func (upf *UPF) AddPDR() (*PDR, error) { } func (upf *UPF) AddFAR() (*FAR, error) { - select { - case <-upf.AssociationContext.Done(): - err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + if err := upf.IsAssociated(); err != nil { return nil, err - default: } far := new(FAR) @@ -505,11 +487,8 @@ func (upf *UPF) AddFAR() (*FAR, error) { } func (upf *UPF) AddBAR() (*BAR, error) { - select { - case <-upf.AssociationContext.Done(): - err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + if err := upf.IsAssociated(); err != nil { return nil, err - default: } bar := new(BAR) @@ -523,11 +502,8 @@ func (upf *UPF) AddBAR() (*BAR, error) { } func (upf *UPF) AddQER() (*QER, error) { - select { - case <-upf.AssociationContext.Done(): - err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + if err := upf.IsAssociated(); err != nil { return nil, err - default: } qer := new(QER) @@ -541,11 +517,8 @@ func (upf *UPF) AddQER() (*QER, error) { } func (upf *UPF) AddURR(urrId uint32, opts ...UrrOpt) (*URR, error) { - select { - case <-upf.AssociationContext.Done(): - err := fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) + if err := upf.IsAssociated(); err != nil { return nil, err - default: } urr := new(URR) @@ -583,10 +556,8 @@ func (upf *UPF) GetQERById(qerId uint32) *QER { // *** add unit test ***// func (upf *UPF) RemovePDR(pdr *PDR) (err error) { - select { - case <-upf.AssociationContext.Done(): - return fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) - default: + if err := upf.IsAssociated(); err != nil { + return err } upf.pdrIDGenerator.FreeID(int64(pdr.PDRID)) @@ -596,10 +567,8 @@ func (upf *UPF) RemovePDR(pdr *PDR) (err error) { // *** add unit test ***// func (upf *UPF) RemoveFAR(far *FAR) (err error) { - select { - case <-upf.AssociationContext.Done(): - return fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) - default: + if err := upf.IsAssociated(); err != nil { + return err } upf.farIDGenerator.FreeID(int64(far.FARID)) @@ -609,10 +578,8 @@ func (upf *UPF) RemoveFAR(far *FAR) (err error) { // *** add unit test ***// func (upf *UPF) RemoveBAR(bar *BAR) (err error) { - select { - case <-upf.AssociationContext.Done(): - return fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) - default: + if err := upf.IsAssociated(); err != nil { + return err } upf.barIDGenerator.FreeID(int64(bar.BARID)) @@ -622,10 +589,8 @@ func (upf *UPF) RemoveBAR(bar *BAR) (err error) { // *** add unit test ***// func (upf *UPF) RemoveQER(qer *QER) (err error) { - select { - case <-upf.AssociationContext.Done(): - return fmt.Errorf("UPF[%s] not associated with SMF", upf.NodeID.ResolveNodeIdToIp().String()) - default: + if err := upf.IsAssociated(); err != nil { + return err } upf.qerIDGenerator.FreeID(int64(qer.QERID)) @@ -651,3 +616,13 @@ func (upf *UPF) ProcEachSMContext(procFunc func(*SMContext)) { return true }) } + +func (upf *UPF) IsAssociated() error { + select { + case <-upf.AssociationContext.Done(): + return fmt.Errorf("UPF[%s] not associated with SMF", + upf.NodeID.ResolveNodeIdToIp().String()) + default: + return nil + } +} diff --git a/internal/context/user_plane_information.go b/internal/context/user_plane_information.go index 4af26731..32e483a8 100644 --- a/internal/context/user_plane_information.go +++ b/internal/context/user_plane_information.go @@ -877,13 +877,11 @@ func (upi *UserPlaneInformation) SelectUPFAndAllocUEIP(selection *UPFSelectionPa for _, upf := range sortedUPFList { logger.CtxLog.Debugf("check start UPF: %s", upi.GetUPFNameByIp(upf.NodeID.ResolveNodeIdToIp().String())) - select { - case <-upf.UPF.AssociationContext.Done(): - logger.CtxLog.Infof("PFCP Association not yet Established with: %s", - upi.GetUPFNameByIp(upf.NodeID.ResolveNodeIdToIp().String())) + if err := upf.UPF.IsAssociated(); err != nil { + logger.CtxLog.Infoln(err) continue - default: } + pools, useStaticIPPool := getUEIPPool(upf, selection) if len(pools) == 0 { continue diff --git a/internal/pfcp/handler/handler.go b/internal/pfcp/handler/handler.go index e8c086b5..7448072c 100644 --- a/internal/pfcp/handler/handler.go +++ b/internal/pfcp/handler/handler.go @@ -124,14 +124,10 @@ func HandlePfcpSessionReportRequest(msg *pfcpUdp.Message) { pfcp_message.SendPfcpSessionReportResponse(msg.RemoteAddr, cause, seqFromUPF, 0) return } - select { - case <-upf.AssociationContext.Done(): - logger.PfcpLog.Warnf("PFCP Session Report Request : Not Associated with UPF[%s], Request Rejected", - upfNodeIDtoIPStr) + if err := upf.IsAssociated(); err != nil { + logger.PfcpLog.Warnf("PFCP Session Report Request rejected: %+v", err) cause.CauseValue = pfcpType.CauseNoEstablishedPfcpAssociation pfcp_message.SendPfcpSessionReportResponse(msg.RemoteAddr, cause, seqFromUPF, 0) - return - default: } if smContext.UpCnxState == models.UpCnxState_DEACTIVATED { diff --git a/internal/pfcp/message/send.go b/internal/pfcp/message/send.go index d38fe5e2..3d68e9d3 100644 --- a/internal/pfcp/message/send.go +++ b/internal/pfcp/message/send.go @@ -140,10 +140,8 @@ func SendPfcpSessionEstablishmentRequest( urrList []*context.URR, ) (resMsg *pfcpUdp.Message, err error) { nodeIDtoIP := upf.NodeID.ResolveNodeIdToIp() - select { - case <-upf.AssociationContext.Done(): - return nil, fmt.Errorf("Not Associated with UPF[%s]", nodeIDtoIP.String()) - default: + if err := upf.IsAssociated(); err != nil { + return nil, err } pfcpMsg, err := BuildPfcpSessionEstablishmentRequest(upf.NodeID, nodeIDtoIP.String(), @@ -225,10 +223,8 @@ func SendPfcpSessionModificationRequest( urrList []*context.URR, ) (resMsg *pfcpUdp.Message, err error) { nodeIDtoIP := upf.NodeID.ResolveNodeIdToIp() - select { - case <-upf.AssociationContext.Done(): - return nil, fmt.Errorf("Not Associated with UPF[%s]", nodeIDtoIP.String()) - default: + if err := upf.IsAssociated(); err != nil { + return nil, err } pfcpMsg, err := BuildPfcpSessionModificationRequest(upf.NodeID, nodeIDtoIP.String(), @@ -302,10 +298,8 @@ func SendPfcpSessionModificationResponse(addr *net.UDPAddr) { func SendPfcpSessionDeletionRequest(upf *context.UPF, ctx *context.SMContext) (resMsg *pfcpUdp.Message, err error) { nodeIDtoIP := upf.NodeID.ResolveNodeIdToIp() - select { - case <-upf.AssociationContext.Done(): - return nil, fmt.Errorf("Not Associated with UPF[%s]", nodeIDtoIP.String()) - default: + if err := upf.IsAssociated(); err != nil { + return nil, err } pfcpMsg, err := BuildPfcpSessionDeletionRequest() diff --git a/internal/sbi/api_upi.go b/internal/sbi/api_upi.go index 61ff2ea8..254cc78d 100644 --- a/internal/sbi/api_upi.go +++ b/internal/sbi/api_upi.go @@ -68,10 +68,8 @@ func (s *Server) PostUpNodesLinks(c *gin.Context) { for _, upf := range upi.UPFs { // only associate new ones - select { - case <-upf.UPF.AssociationContext.Done(): + if err := upf.UPF.IsAssociated(); err != nil { go s.Processor().ToBeAssociatedWithUPF(smf_context.GetSelf().PfcpContext, upf.UPF) - default: } } c.JSON(http.StatusOK, gin.H{"status": "OK"}) diff --git a/internal/sbi/processor/association.go b/internal/sbi/processor/association.go index 20fad749..7a0306f1 100644 --- a/internal/sbi/processor/association.go +++ b/internal/sbi/processor/association.go @@ -135,10 +135,8 @@ func keepHeartbeatTo(upf *smf_context.UPF, upfStr string) { } func doPfcpHeartbeat(upf *smf_context.UPF, upfStr string) error { - select { - case <-upf.AssociationContext.Done(): - return fmt.Errorf("Cancel heartbeat, UPF[%s] is not associted", upfStr) - default: + if err := upf.IsAssociated(); err != nil { + return fmt.Errorf("Cancel heartbeat: %+v", err) } logger.MainLog.Debugf("Sending PFCP Heartbeat Request to UPF%s", upfStr) From c8cf39ff463e6f7bbaeebf41091b6e03592fc77f Mon Sep 17 00:00:00 2001 From: Laura Henning Date: Mon, 23 Sep 2024 08:29:45 +0000 Subject: [PATCH 5/9] refactor: make PFCP context child of app context --- cmd/main.go | 2 +- pkg/utils/pfcp_util.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 74fb5ce1..db97fcb7 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -86,7 +86,7 @@ func action(cliCtx *cli.Context) error { } factory.UERoutingConfig = ueRoutingCfg - pfcpStart, pfcpTerminate := utils.InitPFCPFunc() + pfcpStart, pfcpTerminate := utils.InitPFCPFunc(ctx) smf, err := service.NewApp(ctx, cfg, tlsKeyLogPath, pfcpStart, pfcpTerminate) if err != nil { sigCh <- nil diff --git a/pkg/utils/pfcp_util.go b/pkg/utils/pfcp_util.go index 01719b92..761ade7e 100644 --- a/pkg/utils/pfcp_util.go +++ b/pkg/utils/pfcp_util.go @@ -11,12 +11,12 @@ import ( "github.com/free5gc/smf/pkg/service" ) -func InitPFCPFunc() (func(a *service.SmfApp), func()) { +func InitPFCPFunc(pCtx context.Context) (func(a *service.SmfApp), func()) { smfContext := smf_context.GetSelf() pfcpStart := func(a *service.SmfApp) { // Initialize PFCP server - smfContext.PfcpContext, smfContext.PfcpCancelFunc = context.WithCancel(context.Background()) + smfContext.PfcpContext, smfContext.PfcpCancelFunc = context.WithCancel(pCtx) udp.Run(pfcp.Dispatch) From 04e8e6dfef7c44be9f65b691bc17981f73608eed Mon Sep 17 00:00:00 2001 From: Laura Henning Date: Tue, 24 Sep 2024 08:56:35 +0000 Subject: [PATCH 6/9] refactor: improve conciseness and memory allocation --- internal/context/upf.go | 225 +++++++++++++++++++--------------------- 1 file changed, 109 insertions(+), 116 deletions(-) diff --git a/internal/context/upf.go b/internal/context/upf.go index 21c8628e..0de70d74 100644 --- a/internal/context/upf.go +++ b/internal/context/upf.go @@ -377,169 +377,162 @@ func (upf *UPF) GetUPFID() string { return upInfo.GetUPFIDByIP(upfIP) } -func (upf *UPF) pdrID() (uint16, error) { - if err := upf.IsAssociated(); err != nil { - return 0, err +func (upf *UPF) pdrID() (pdrID uint16, err error) { + if err = upf.IsAssociated(); err != nil { + return } - var pdrID uint16 - if tmpID, err := upf.pdrIDGenerator.Allocate(); err != nil { + tmpID, err := upf.pdrIDGenerator.Allocate() + if err != nil { return 0, err - } else { - pdrID = uint16(tmpID) } - - return pdrID, nil + pdrID = uint16(tmpID) + return } -func (upf *UPF) farID() (uint32, error) { - if err := upf.IsAssociated(); err != nil { - return 0, err +func (upf *UPF) farID() (farID uint32, err error) { + if err = upf.IsAssociated(); err != nil { + return } - var farID uint32 - if tmpID, err := upf.farIDGenerator.Allocate(); err != nil { + tmpID, err := upf.farIDGenerator.Allocate() + if err != nil { return 0, err - } else { - farID = uint32(tmpID) } - - return farID, nil + farID = uint32(tmpID) + return } -func (upf *UPF) barID() (uint8, error) { - if err := upf.IsAssociated(); err != nil { - return 0, err +func (upf *UPF) barID() (barID uint8, err error) { + if err = upf.IsAssociated(); err != nil { + return } - var barID uint8 - if tmpID, err := upf.barIDGenerator.Allocate(); err != nil { + tmpID, err := upf.barIDGenerator.Allocate() + if err != nil { return 0, err - } else { - barID = uint8(tmpID) } - - return barID, nil + barID = uint8(tmpID) + return } -func (upf *UPF) qerID() (uint32, error) { - if err := upf.IsAssociated(); err != nil { - return 0, err +func (upf *UPF) qerID() (qerID uint32, err error) { + if err = upf.IsAssociated(); err != nil { + return } - var qerID uint32 - if tmpID, err := upf.qerIDGenerator.Allocate(); err != nil { + tmpID, err := upf.qerIDGenerator.Allocate() + if err != nil { return 0, err - } else { - qerID = uint32(tmpID) } - - return qerID, nil + qerID = uint32(tmpID) + return } -func (upf *UPF) urrID() (uint32, error) { - var urrID uint32 - if tmpID, err := upf.urrIDGenerator.Allocate(); err != nil { +func (upf *UPF) urrID() (urrID uint32, err error) { + tmpID, err := upf.urrIDGenerator.Allocate() + if err != nil { return 0, err - } else { - urrID = uint32(tmpID) } - - return urrID, nil + urrID = uint32(tmpID) + return } -func (upf *UPF) AddPDR() (*PDR, error) { - if err := upf.IsAssociated(); err != nil { - return nil, err +func (upf *UPF) AddPDR() (pdr *PDR, err error) { + if err = upf.IsAssociated(); err != nil { + return } - pdr := new(PDR) - if PDRID, err := upf.pdrID(); err != nil { - return nil, err - } else { - pdr.PDRID = PDRID - upf.pdrPool.Store(pdr.PDRID, pdr) + pdrID, err := upf.pdrID() + if err != nil { + return } - if newFAR, err := upf.AddFAR(); err != nil { - return nil, err - } else { - pdr.FAR = newFAR + newFAR, err := upf.AddFAR() + if err != nil { + return } - return pdr, nil + pdr = &PDR{ + PDRID: pdrID, + FAR: newFAR, + } + upf.pdrPool.Store(pdr.PDRID, pdr) + return } -func (upf *UPF) AddFAR() (*FAR, error) { - if err := upf.IsAssociated(); err != nil { - return nil, err +func (upf *UPF) AddFAR() (far *FAR, err error) { + if err = upf.IsAssociated(); err != nil { + return } - far := new(FAR) - if FARID, err := upf.farID(); err != nil { - return nil, err - } else { - far.FARID = FARID - upf.farPool.Store(far.FARID, far) + farID, err := upf.farID() + if err != nil { + return } - + far = &FAR{ + FARID: farID, + } + upf.farPool.Store(far.FARID, far) return far, nil } -func (upf *UPF) AddBAR() (*BAR, error) { - if err := upf.IsAssociated(); err != nil { - return nil, err +func (upf *UPF) AddBAR() (bar *BAR, err error) { + if err = upf.IsAssociated(); err != nil { + return } - bar := new(BAR) - if BARID, err := upf.barID(); err != nil { - } else { - bar.BARID = BARID - upf.barPool.Store(bar.BARID, bar) + barID, err := upf.barID() + if err != nil { + return } - - return bar, nil + bar = &BAR{ + BARID: barID, + } + upf.barPool.Store(bar.BARID, bar) + return } -func (upf *UPF) AddQER() (*QER, error) { - if err := upf.IsAssociated(); err != nil { - return nil, err +func (upf *UPF) AddQER() (qer *QER, err error) { + if err = upf.IsAssociated(); err != nil { + return } - qer := new(QER) - if QERID, err := upf.qerID(); err != nil { - } else { - qer.QERID = QERID - upf.qerPool.Store(qer.QERID, qer) + qerID, err := upf.qerID() + if err != nil { + return } - - return qer, nil + qer = &QER{ + QERID: qerID, + } + upf.qerPool.Store(qer.QERID, qer) + return } -func (upf *UPF) AddURR(urrId uint32, opts ...UrrOpt) (*URR, error) { - if err := upf.IsAssociated(); err != nil { - return nil, err +func (upf *UPF) AddURR(urrID uint32, opts ...UrrOpt) (urr *URR, err error) { + if err = upf.IsAssociated(); err != nil { + return + } + + if urrID == 0 { + urrID, err = upf.urrID() + if err != nil { + return + } } - urr := new(URR) - urr.MeasureMethod = MesureMethodVol - urr.MeasurementInformation = MeasureInformation(true, false) + urr = &URR{ + URRID: urrID, + MeasureMethod: MesureMethodVol, + MeasurementInformation: MeasureInformation(true, false), + } for _, opt := range opts { opt(urr) } - if urrId == 0 { - if URRID, err := upf.urrID(); err != nil { - } else { - urr.URRID = URRID - upf.urrPool.Store(urr.URRID, urr) - } - } else { - urr.URRID = urrId - upf.urrPool.Store(urr.URRID, urr) - } - return urr, nil + upf.urrPool.Store(urr.URRID, urr) + return } func (upf *UPF) GetUUID() uuid.UUID { @@ -556,46 +549,46 @@ func (upf *UPF) GetQERById(qerId uint32) *QER { // *** add unit test ***// func (upf *UPF) RemovePDR(pdr *PDR) (err error) { - if err := upf.IsAssociated(); err != nil { - return err + if err = upf.IsAssociated(); err != nil { + return } upf.pdrIDGenerator.FreeID(int64(pdr.PDRID)) upf.pdrPool.Delete(pdr.PDRID) - return nil + return } // *** add unit test ***// func (upf *UPF) RemoveFAR(far *FAR) (err error) { - if err := upf.IsAssociated(); err != nil { - return err + if err = upf.IsAssociated(); err != nil { + return } upf.farIDGenerator.FreeID(int64(far.FARID)) upf.farPool.Delete(far.FARID) - return nil + return } // *** add unit test ***// func (upf *UPF) RemoveBAR(bar *BAR) (err error) { - if err := upf.IsAssociated(); err != nil { - return err + if err = upf.IsAssociated(); err != nil { + return } upf.barIDGenerator.FreeID(int64(bar.BARID)) upf.barPool.Delete(bar.BARID) - return nil + return } // *** add unit test ***// func (upf *UPF) RemoveQER(qer *QER) (err error) { - if err := upf.IsAssociated(); err != nil { - return err + if err = upf.IsAssociated(); err != nil { + return } upf.qerIDGenerator.FreeID(int64(qer.QERID)) upf.qerPool.Delete(qer.QERID) - return nil + return } func (upf *UPF) isSupportSnssai(snssai *SNssai) bool { From 463154269507ae9466e7392138864d7d9f6c5216 Mon Sep 17 00:00:00 2001 From: Laura Henning Date: Tue, 24 Sep 2024 08:56:47 +0000 Subject: [PATCH 7/9] fix: linting --- internal/context/user_plane_information.go | 2 +- internal/pfcp/message/send.go | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/internal/context/user_plane_information.go b/internal/context/user_plane_information.go index 32e483a8..5204399b 100644 --- a/internal/context/user_plane_information.go +++ b/internal/context/user_plane_information.go @@ -877,7 +877,7 @@ func (upi *UserPlaneInformation) SelectUPFAndAllocUEIP(selection *UPFSelectionPa for _, upf := range sortedUPFList { logger.CtxLog.Debugf("check start UPF: %s", upi.GetUPFNameByIp(upf.NodeID.ResolveNodeIdToIp().String())) - if err := upf.UPF.IsAssociated(); err != nil { + if err = upf.UPF.IsAssociated(); err != nil { logger.CtxLog.Infoln(err) continue } diff --git a/internal/pfcp/message/send.go b/internal/pfcp/message/send.go index 3d68e9d3..7905e3d4 100644 --- a/internal/pfcp/message/send.go +++ b/internal/pfcp/message/send.go @@ -140,7 +140,7 @@ func SendPfcpSessionEstablishmentRequest( urrList []*context.URR, ) (resMsg *pfcpUdp.Message, err error) { nodeIDtoIP := upf.NodeID.ResolveNodeIdToIp() - if err := upf.IsAssociated(); err != nil { + if err = upf.IsAssociated(); err != nil { return nil, err } @@ -223,7 +223,7 @@ func SendPfcpSessionModificationRequest( urrList []*context.URR, ) (resMsg *pfcpUdp.Message, err error) { nodeIDtoIP := upf.NodeID.ResolveNodeIdToIp() - if err := upf.IsAssociated(); err != nil { + if err = upf.IsAssociated(); err != nil { return nil, err } @@ -296,9 +296,12 @@ func SendPfcpSessionModificationResponse(addr *net.UDPAddr) { udp.SendPfcpResponse(message, addr) } -func SendPfcpSessionDeletionRequest(upf *context.UPF, ctx *context.SMContext) (resMsg *pfcpUdp.Message, err error) { +func SendPfcpSessionDeletionRequest( + upf *context.UPF, + ctx *context.SMContext, +) (resMsg *pfcpUdp.Message, err error) { nodeIDtoIP := upf.NodeID.ResolveNodeIdToIp() - if err := upf.IsAssociated(); err != nil { + if err = upf.IsAssociated(); err != nil { return nil, err } From 7d3d1809aa771974310c2022d7a360d51904bf5a Mon Sep 17 00:00:00 2001 From: yzlin Date: Mon, 30 Sep 2024 07:27:16 +0000 Subject: [PATCH 8/9] fix: revert UPF selection way to choose correct UPF --- internal/context/user_plane_information.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/internal/context/user_plane_information.go b/internal/context/user_plane_information.go index 5204399b..1978ba10 100644 --- a/internal/context/user_plane_information.go +++ b/internal/context/user_plane_information.go @@ -741,14 +741,10 @@ func (upi *UserPlaneInformation) selectMatchUPF(selection *UPFSelectionParams) [ if currentSnssai.Equal(targetSnssai) { for _, dnnInfo := range snssaiInfo.DnnList { - if dnnInfo.Dnn != selection.Dnn { - continue + if dnnInfo.Dnn == selection.Dnn && dnnInfo.ContainsDNAI(selection.Dnai) { + upList = append(upList, upNode) + break } - if selection.Dnai != "" && !dnnInfo.ContainsDNAI(selection.Dnai) { - continue - } - upList = append(upList, upNode) - break } } } From 736f3d1f3a2d8f4ef466fccc45a9aece7455bdca Mon Sep 17 00:00:00 2001 From: yzlin Date: Mon, 30 Sep 2024 07:27:44 +0000 Subject: [PATCH 9/9] fix: Missing URR creation will cause TI error --- internal/context/datapath.go | 2 +- internal/pfcp/message/build.go | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/context/datapath.go b/internal/context/datapath.go index 3b540813..66780904 100644 --- a/internal/context/datapath.go +++ b/internal/context/datapath.go @@ -831,7 +831,7 @@ func (p *DataPath) AddChargingRules(smContext *SMContext, chgLevel ChargingLevel // nolint nodeId, _ := node.GetUPFID() logger.PduSessLog.Tracef("DownLinkTunnel add URR for node %s %+v", - nodeId, node.UpLinkTunnel.PDR) + nodeId, node.DownLinkTunnel.PDR) } } } diff --git a/internal/pfcp/message/build.go b/internal/pfcp/message/build.go index 5a81fd80..d20af340 100644 --- a/internal/pfcp/message/build.go +++ b/internal/pfcp/message/build.go @@ -435,8 +435,9 @@ func BuildPfcpSessionEstablishmentRequest( urrMap[urr.URRID] = urr } for _, filteredURR := range urrMap { - if filteredURR.State == context.RULE_INITIAL { - msg.CreateURR = append(msg.CreateURR, urrToCreateURR(filteredURR)) + msg.CreateURR = append(msg.CreateURR, urrToCreateURR(filteredURR)) + if filteredURR.State == context.RULE_CREATE { + smContext.Log.Warn("Duplicate URR creation") } filteredURR.State = context.RULE_CREATE } @@ -564,6 +565,9 @@ func BuildPfcpSessionModificationRequest( for _, urr := range urrList { switch urr.State { + case context.RULE_CREATE: + smContext.Log.Warn("Duplicate URR creation") + fallthrough case context.RULE_INITIAL: msg.CreateURR = append(msg.CreateURR, urrToCreateURR(urr)) case context.RULE_UPDATE: