Update FARs instead of overwriting them

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
 }

pfcp/entity.go

@@ -401,7 +401,6 @@ func (e *PFCPEntity) LogPFCPRules() {
 			ForwardingParametersIe, err := far.ForwardingParameters()
 			if err != nil {
 				isFP = false
-
 			}
 			OuterHeaderCreationLabel := "No"
 			if isFP {

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)

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,33 +69,52 @@ 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
+	logrus.Trace("Inside farmap.Update()")
+	// only present fields are replaced
 	id, err := far.ID()
 	if err != nil {
 		return err
 	}
 	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 {
-		delete(m.farmap, id)
-		m.farmap[id] = far
+		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
 	}
 	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 {
@@ -162,32 +182,51 @@ 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 {
 			err = f.Add(NewFAR(ie.NewFARID(id), ie.NewApplyAction(aa...), nil))
 		} else {
 			err = f.Add(NewFAR(ie.NewFARID(id), ie.NewApplyAction(aa...), ie.NewForwardingParameters(fp...)))
 		}
+	}
+	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 {
-			return nil, err, ie.CauseMandatoryIEIncorrect, ie.CreateFAR
+			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.UpdateForwardingParameters()
+		if err == nil {
+			iefp = ie.NewForwardingParameters(fp...)
 		}
+		f.Add(NewFAR(ie.NewFARID(id), ieaa, iefp))
 	}
 	return &f, nil, 0, 0
 

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)