From a56e3450651dab054c3d4a5277ba2b007f375549 Mon Sep 17 00:00:00 2001 From: Louis Royer Date: Thu, 5 Sep 2024 13:41:38 +0200 Subject: [PATCH 1/5] Fix crash --- pfcp/entity.go | 1 - pfcp/far_map.go | 24 +++++------------------- 2 files changed, 5 insertions(+), 20 deletions(-) diff --git a/pfcp/entity.go b/pfcp/entity.go index 38b0004..fa29782 100644 --- a/pfcp/entity.go +++ b/pfcp/entity.go @@ -401,7 +401,6 @@ func (e *PFCPEntity) LogPFCPRules() { ForwardingParametersIe, err := far.ForwardingParameters() if err != nil { isFP = false - } OuterHeaderCreationLabel := "No" if isFP { diff --git a/pfcp/far_map.go b/pfcp/far_map.go index 386973e..f7ec902 100644 --- a/pfcp/far_map.go +++ b/pfcp/far_map.go @@ -162,22 +162,11 @@ func NewFARMap(fars []*ie.IE) (farmap *FARMap, err error, cause uint8, offending mustHaveFP = true } fp, err := far.ForwardingParameters() - - if err != nil && mustHaveFP { - fp, err = far.ForwardingParameters() - if err != nil { - //XXX: workaround for a free5gc-smf bug: Forwarding Parameters are missing sometimes - fp = make([]*ie.IE, 0) - hasFP = true - // if err == io.ErrUnexpectedEOF { - // return nil, err, ie.CauseInvalidLength, ie.ForwardingParameters - // } - // if ie.NewApplyAction(aa).HasFORW() && err == ie.ErrIENotFound { - // return nil, err, ie.CauseConditionalIEMissing, ie.ForwardingParameters - // } - } else if err == nil { - hasFP = true - } + if err == nil { + hasFP = true + } + if mustHaveFP && !hasFP { + return nil, err, ie.CauseMandatoryIEIncorrect, ie.CreateFAR } if !hasFP { @@ -185,9 +174,6 @@ func NewFARMap(fars []*ie.IE) (farmap *FARMap, err error, cause uint8, offending } else { err = f.Add(NewFAR(ie.NewFARID(id), ie.NewApplyAction(aa...), ie.NewForwardingParameters(fp...))) } - if err != nil { - return nil, err, ie.CauseMandatoryIEIncorrect, ie.CreateFAR - } } return &f, nil, 0, 0 From 31dedf1128311d0fdb2e0c4f502ab511c5aede1d Mon Sep 17 00:00:00 2001 From: Louis Royer Date: Thu, 5 Sep 2024 14:46:26 +0200 Subject: [PATCH 2/5] Update far --- pfcp/api/far_interface.go | 2 ++ pfcp/far.go | 10 +++++++++ pfcp/far_map.go | 44 +++++++++++++++++++++++++++++++++++---- pfcp/handlers.go | 2 +- 4 files changed, 53 insertions(+), 5 deletions(-) diff --git a/pfcp/api/far_interface.go b/pfcp/api/far_interface.go index bbe8702..bfd8ef9 100644 --- a/pfcp/api/far_interface.go +++ b/pfcp/api/far_interface.go @@ -12,6 +12,8 @@ type FARID = uint32 type FARInterface interface { ID() (FARID, error) ApplyAction() *ie.IE + SetApplyAction(*ie.IE) error ForwardingParameters() (*ie.IE, error) + SetForwardingParameters(*ie.IE) error NewCreateFAR() *ie.IE } diff --git a/pfcp/far.go b/pfcp/far.go index f603774..c01bd08 100644 --- a/pfcp/far.go +++ b/pfcp/far.go @@ -33,6 +33,11 @@ func (far *FAR) ApplyAction() *ie.IE { return far.applyAction } +func (far *FAR) SetApplyAction(aa *ie.IE) error { + far.applyAction = aa + return nil +} + func (far *FAR) ForwardingParameters() (*ie.IE, error) { // This IE shall be present when the Apply Action requests // the packets to be forwarded. It may be present otherwise. @@ -43,6 +48,11 @@ func (far *FAR) ForwardingParameters() (*ie.IE, error) { } +func (far *FAR) SetForwardingParameters(fp *ie.IE) error { + far.forwardingParameters = fp + return nil +} + func (far *FAR) NewCreateFAR() *ie.IE { ies := make([]*ie.IE, 0) ies = append(ies, far.id) diff --git a/pfcp/far_map.go b/pfcp/far_map.go index f7ec902..22ef4ea 100644 --- a/pfcp/far_map.go +++ b/pfcp/far_map.go @@ -68,8 +68,7 @@ func (m *FARMap) SimulateAdd(far api.FARInterface) error { } func (m *FARMap) Update(far api.FARInterface) error { - // XXX: instead of replacing old FAR with new one, - // only present fields should be replaced + // only present fields are replaced id, err := far.ID() if err != nil { return err @@ -79,8 +78,12 @@ func (m *FARMap) Update(far api.FARInterface) error { if _, exists := m.farmap[id]; !exists { return fmt.Errorf("FAR %d does not exist.", id) } else { - delete(m.farmap, id) - m.farmap[id] = far + if far.ApplyAction() != nil { + m.farmap[id].SetApplyAction(far.ApplyAction()) + } + if fp, err := far.ForwardingParameters(); err == nil { + m.farmap[id].SetForwardingParameters(fp) + } return nil } } @@ -178,3 +181,36 @@ func NewFARMap(fars []*ie.IE) (farmap *FARMap, err error, cause uint8, offending return &f, nil, 0, 0 } + +func NewFARMapUpdate(fars []*ie.IE) (*FARMap, error, uint8, uint16) { + f := FARMap{ + farmap: make(farmapInternal), + mu: sync.RWMutex{}, + } + for _, far := range fars { + id, err := far.FARID() + if err != nil { + switch err { + case io.ErrUnexpectedEOF: + return nil, err, ie.CauseInvalidLength, ie.FARID + case ie.ErrIENotFound: + return nil, err, ie.CauseMandatoryIEMissing, ie.FARID + default: + return nil, err, ie.CauseMandatoryIEIncorrect, ie.CreateFAR + } + } + var ieaa *ie.IE = nil + aa, err := far.ApplyAction() + if err == nil { + ieaa = ie.NewApplyAction(aa...) + } + var iefp *ie.IE = nil + fp, err := far.ForwardingParameters() + if err == nil { + iefp = ie.NewForwardingParameters(fp...) + } + f.Update(NewFAR(ie.NewFARID(id), ieaa, iefp)) + } + return &f, nil, 0, 0 + +} diff --git a/pfcp/handlers.go b/pfcp/handlers.go index 0819ccf..98a81c2 100644 --- a/pfcp/handlers.go +++ b/pfcp/handlers.go @@ -240,7 +240,7 @@ func DefaultSessionModificationRequestHandler(ctx context.Context, msg ReceivedM } // update FARs - updatefars, err, cause, offendingie := NewFARMap(m.UpdateFAR) + updatefars, err, cause, offendingie := NewFARMapUpdate(m.UpdateFAR) if err != nil { res := message.NewSessionEstablishmentResponse(0, 0, rseid, msg.Sequence(), 0, msg.Entity.NodeID(), ie.NewCause(cause), ie.NewOffendingIE(offendingie)) return msg.NewResponse(res) From dc2a1f267a8b28006cc641d10e9d2b2f4f83e5fa Mon Sep 17 00:00:00 2001 From: Louis Royer Date: Thu, 5 Sep 2024 15:00:13 +0200 Subject: [PATCH 3/5] update-fp --- pfcp/far_map.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pfcp/far_map.go b/pfcp/far_map.go index 22ef4ea..62d237d 100644 --- a/pfcp/far_map.go +++ b/pfcp/far_map.go @@ -205,7 +205,7 @@ func NewFARMapUpdate(fars []*ie.IE) (*FARMap, error, uint8, uint16) { ieaa = ie.NewApplyAction(aa...) } var iefp *ie.IE = nil - fp, err := far.ForwardingParameters() + fp, err := far.UpdateForwardingParameters() if err == nil { iefp = ie.NewForwardingParameters(fp...) } From 683837d9ae6c9e8f27f69d00df1e3280ba062d42 Mon Sep 17 00:00:00 2001 From: Louis Royer Date: Thu, 5 Sep 2024 15:22:07 +0200 Subject: [PATCH 4/5] update instead of create --- pfcp/far_map.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pfcp/far_map.go b/pfcp/far_map.go index 62d237d..53e0446 100644 --- a/pfcp/far_map.go +++ b/pfcp/far_map.go @@ -81,6 +81,7 @@ func (m *FARMap) Update(far api.FARInterface) error { if far.ApplyAction() != nil { m.farmap[id].SetApplyAction(far.ApplyAction()) } + // XXX: update fields in forwarding paramaters instead of replacing if fp, err := far.ForwardingParameters(); err == nil { m.farmap[id].SetForwardingParameters(fp) } @@ -207,7 +208,7 @@ func NewFARMapUpdate(fars []*ie.IE) (*FARMap, error, uint8, uint16) { var iefp *ie.IE = nil fp, err := far.UpdateForwardingParameters() if err == nil { - iefp = ie.NewForwardingParameters(fp...) + iefp = ie.NewUpdateForwardingParameters(fp...) } f.Update(NewFAR(ie.NewFARID(id), ieaa, iefp)) } From b6135a55d9ee7c51e18f1cdc908adbcdfd6e55d2 Mon Sep 17 00:00:00 2001 From: Louis Royer Date: Thu, 5 Sep 2024 15:29:18 +0200 Subject: [PATCH 5/5] Add warning --- pfcp/far_map.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/pfcp/far_map.go b/pfcp/far_map.go index 53e0446..a89b859 100644 --- a/pfcp/far_map.go +++ b/pfcp/far_map.go @@ -11,6 +11,7 @@ import ( "sync" "github.com/nextmn/go-pfcp-networking/pfcp/api" + "github.com/sirupsen/logrus" "github.com/wmnsk/go-pfcp/ie" ) @@ -68,6 +69,7 @@ func (m *FARMap) SimulateAdd(far api.FARInterface) error { } func (m *FARMap) Update(far api.FARInterface) error { + logrus.Trace("Inside farmap.Update()") // only present fields are replaced id, err := far.ID() if err != nil { @@ -76,20 +78,32 @@ func (m *FARMap) Update(far api.FARInterface) error { m.mu.Lock() defer m.mu.Unlock() if _, exists := m.farmap[id]; !exists { + logrus.WithFields(logrus.Fields{"far-id": id, "current_map": m.farmap}).Trace("Updating FAR: this FAR id does not exist") return fmt.Errorf("FAR %d does not exist.", id) } else { + logrus.WithFields(logrus.Fields{"far-id": id}).Trace("Updating FAR") if far.ApplyAction() != nil { m.farmap[id].SetApplyAction(far.ApplyAction()) + logrus.WithFields(logrus.Fields{"far-id": id}).Trace("Updating FAR Apply Action") } // XXX: update fields in forwarding paramaters instead of replacing if fp, err := far.ForwardingParameters(); err == nil { + if fp == nil { + logrus.Warn("Removing forwarding parameters. aborting") + return nil + } m.farmap[id].SetForwardingParameters(fp) + logrus.WithFields(logrus.Fields{"far-id": id}).Trace("Updating FAR Forwarding Parameters") + } else { + logrus.WithFields(logrus.Fields{"far-id": id}).Trace("Updating FAR but not Forwarding Parameters") } + return nil } } func (m *FARMap) SimulateUpdate(far api.FARInterface) error { + logrus.Trace("Inside farmap.SimulateUpdate()") id, err := far.ID() if err != nil { return err @@ -97,8 +111,10 @@ func (m *FARMap) SimulateUpdate(far api.FARInterface) error { m.mu.RLock() defer m.mu.RUnlock() if _, exists := m.farmap[id]; !exists { + logrus.WithFields(logrus.Fields{"far-id": id, "current_map": m.farmap}).Trace("Simulate Updating FAR: this FAR id does not exist") return fmt.Errorf("FAR %d does not exist.", id) } + logrus.WithFields(logrus.Fields{"far-id": id, "current_map": m.farmap}).Trace("Simulate Updating FAR: exist") return nil } func (m *FARMap) Remove(key api.FARID) error { @@ -208,9 +224,9 @@ func NewFARMapUpdate(fars []*ie.IE) (*FARMap, error, uint8, uint16) { var iefp *ie.IE = nil fp, err := far.UpdateForwardingParameters() if err == nil { - iefp = ie.NewUpdateForwardingParameters(fp...) + iefp = ie.NewForwardingParameters(fp...) } - f.Update(NewFAR(ie.NewFARID(id), ieaa, iefp)) + f.Add(NewFAR(ie.NewFARID(id), ieaa, iefp)) } return &f, nil, 0, 0