[CENNSO-1947] fix: Error indication report crash (#3)

Report message could contain invalid session handler if send the same moment as deletion request for this session. This resulted in crash in session report response.

test/e2e/framework/gtpu.go

@@ -22,6 +22,8 @@ import (
 	"os"
 
 	"github.com/pkg/errors"
+	"github.com/wmnsk/go-gtp/gtpv1/ie"
+	"github.com/wmnsk/go-gtp/gtpv1/message"
 
 	"github.com/sirupsen/logrus"
 
@@ -148,6 +150,14 @@ func (gtpu *GTPU) Stop() error {
 	return gtpu.up.Stop()
 }
 
+func (gtpu *GTPU) SendErrorIndication(teid uint32, seq uint16, IEs ...*ie.IE) error {
+	dst := net.UDPAddr{
+		IP:   gtpu.cfg.PGWIP,
+		Port: 2152,
+	}
+	return gtpu.up.WriteTo(&dst, message.NewErrorIndication(teid, seq, IEs...))
+}
+
 func (gtpu *GTPU) Context(parent context.Context) context.Context {
 	return gtpu.up.Context(parent)
 }

test/e2e/pfcp/pfcp.go

@@ -918,8 +918,10 @@ func (pc *PFCPConnection) sendSessionReportResponse(req *message.SessionReportRe
 
 			up_seid = ses.up_seid
 		}
+	} else if sess := pc.sessions[SEID(req.SEID())]; sess != nil {
+		up_seid = sess.up_seid
 	} else {
-		up_seid = pc.sessions[SEID(req.SEID())].up_seid
+		return pc.send(message.NewSessionReportResponse(0, 0, 0, req.SequenceNumber, 0, ie.NewCause(ie.CauseSessionContextNotFound)))
 	}
 	ies = append(ies, ie.NewRecoveryTimeStamp(pc.timestamp))
 	return pc.send(message.NewSessionReportResponse(0, 0, uint64(up_seid), req.SequenceNumber, 0, ies...))

test/e2e/upg_e2e.go

@@ -33,6 +33,7 @@ import (
 	"github.com/onsi/ginkgo"
 	"github.com/onsi/gomega"
 	"github.com/pkg/errors"
+	gtpuie "github.com/wmnsk/go-gtp/gtpv1/ie"
 	gtpumessage "github.com/wmnsk/go-gtp/gtpv1/message"
 	"github.com/wmnsk/go-pfcp/ie"
 	"github.com/wmnsk/go-pfcp/message"
@@ -2218,6 +2219,74 @@ var _ = ginkgo.Describe("Error handling", func() {
 	})
 })
 
+var _ = ginkgo.Describe("PGW Error indication", func() {
+	f := framework.NewDefaultFramework(framework.UPGModePGW, framework.UPGIPModeV4)
+	ginkgo.It("Error indication for deleted session", func() {
+		ginkgo.By("Configuring session")
+
+		var sessionCfgs []framework.SessionConfig
+		var seids []pfcp.SEID
+
+		// Create another session to trigger crash
+		ueIPs := []net.IP{f.UEIP(), f.AddUEIP()}
+		for i, ueIP := range ueIPs {
+			sessionCfgs = append(sessionCfgs, framework.SessionConfig{
+				IdBase:      1,
+				UEIP:        ueIP,
+				Mode:        f.Mode,
+				VolumeQuota: 100000000,
+				TEIDPGWs5u:  framework.TEIDPGWs5u + uint32(i),
+				TEIDSGWs5u:  framework.TEIDSGWs5u + uint32(i),
+				PGWIP:       f.VPPCfg.GetVPPAddress("grx").IP,
+				SGWIP:       f.VPPCfg.GetNamespaceAddress("grx").IP,
+			})
+			seids = append(seids, f.PFCP.NewSEID())
+		}
+
+		for i := range sessionCfgs {
+			_, err := f.PFCP.EstablishSession(f.Context, seids[i], sessionCfgs[i].SessionIEs()...)
+			framework.ExpectNoError(err)
+		}
+
+		ginkgo.By("Starting some traffic")
+		tg, clientNS, serverNS := newTrafficGen(f, &traffic.UDPPingConfig{
+			PacketCount: 5,
+			Retry:       false,
+			Delay:       10 * time.Millisecond,
+		}, &traffic.SimpleTrafficRec{})
+		tg.Start(f.Context, clientNS, serverNS)
+
+		ginkgo.By("Sending error indication")
+
+		stopSpam := make(chan struct{}, 1)
+
+		spamErrorsIndications := func() {
+			for {
+				select {
+				case _, closed := <-stopSpam:
+					if closed {
+						return
+					}
+				default:
+					err := f.GTPUs[0].SendErrorIndication(0, 0,
+						gtpuie.NewTEIDDataI(sessionCfgs[0].TEIDSGWs5u),
+						gtpuie.NewGSNAddress(f.VPPCfg.GetNamespaceAddress("grx").IP.String()),
+					)
+					gomega.Expect(err).NotTo(gomega.HaveOccurred())
+					time.Sleep(time.Millisecond)
+				}
+			}
+		}
+		go spamErrorsIndications()
+
+		f.PFCP.DeleteSession(f.Context, seids[0])
+		time.Sleep(time.Second / 2)
+		f.PFCP.DeleteSession(f.Context, seids[1])
+		close(stopSpam)
+		time.Sleep(10 * time.Second)
+	})
+})
+
 var _ = ginkgo.Describe("Error handling", func() {
 	f := framework.NewDefaultFramework(framework.UPGModeTDF, framework.UPGIPModeV4)
 

upf/upf_pfcp_api.c

@@ -2953,6 +2953,13 @@ handle_session_report_response (pfcp_msg_t *msg, pfcp_decoded_msg_t *dmsg)
       return -1;
     }
 
+  if (pool_is_free_index (gtm->sessions, msg->session.idx))
+    {
+      /* Precaution against buggy code */
+      ASSERT (0);
+      return -1;
+    }
+
   upf_session_t *sx = pool_elt_at_index (gtm->sessions, msg->session.idx);
 
   if (msg->up_seid != sx->up_seid)

upf/upf_pfcp_server.c

@@ -1499,8 +1499,10 @@ pfcp_process (vlib_main_t *vm, vlib_node_runtime_t *rt, vlib_frame_t *f)
                 pfcp_msg_t *tx = (pfcp_msg_t *) event_data[i];
                 // Handle case when session was removed before
                 // receiving this event
-
-                if (!pool_is_free_index (gtm->nodes, tx->node))
+                if (!pool_is_free_index (gtm->nodes, tx->node) &&
+                    !pool_is_free_index (gtm->sessions, tx->session.idx) &&
+                    pool_elt_at_index (gtm->sessions, tx->session.idx)
+                        ->up_seid == tx->up_seid)
                   {
                     pfcp_msg_t *msg;
 
@@ -1509,6 +1511,7 @@ pfcp_process (vlib_main_t *vm, vlib_node_runtime_t *rt, vlib_frame_t *f)
                   }
                 else
                   {
+                    upf_debug ("ignored tx event because session deleted");
                     vec_free (tx->data);
                   }