Fix memory leak during redirect info handling

travelping · Jul 14, 2021 · d650220 · d650220
1 parent 11774a2
commit d650220
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 10 deletions.
diff --git a/test/e2e/framework/sessionconfig.go b/test/e2e/framework/sessionconfig.go
@@ -207,11 +207,22 @@ func (cfg SessionConfig) reversePDR(pdrID uint16, farID, urrID, precedence uint3
 	return ie.NewCreatePDR(ies...)
 }
 
-func (cfg SessionConfig) SessionIEs() []*ie.IE {
-	ies := []*ie.IE{
+func (cfg SessionConfig) CreateFARs() []*ie.IE {
+	return []*ie.IE{
 		cfg.forwardFAR(1),
 		cfg.reverseFAR(2),
 	}
+}
+
+func (cfg SessionConfig) DeleteFARs() []*ie.IE {
+	return []*ie.IE{
+		ie.NewRemoveFAR(ie.NewFARID(uint32(cfg.IdBase))),
+		ie.NewRemoveFAR(ie.NewFARID(uint32(cfg.IdBase + 1))),
+	}
+}
+
+func (cfg SessionConfig) SessionIEs() []*ie.IE {
+	ies := cfg.CreateFARs()
 	if !cfg.NoURRs {
 		ies = append(ies,
 			ie.NewCreateURR(

diff --git a/test/e2e/upg_e2e.go b/test/e2e/upg_e2e.go
@@ -533,22 +533,52 @@ var _ = ginkgo.Describe("Multiple PFCP Sessions", func() {
 			}
 			for i := 0; i < leakTestNumIterations; i++ {
 				framework.Logf("creating %d sessions", leakTestNumSessions)
+				sessionCfgs := make([]*framework.SessionConfig, leakTestNumSessions)
 				specs := make([]pfcp.SessionOpSpec, leakTestNumSessions)
 				for j := 0; j < leakTestNumSessions; j++ {
-					sessionCfg := &framework.SessionConfig{
+					sessionCfgs[j] = &framework.SessionConfig{
 						IdBase:  1,
 						UEIP:    ueIPs[j],
 						Mode:    f.Mode,
 						AppName: framework.HTTPAppName,
+						// There was a bug in free_far() at some point
+						// so it was failing to free redirect information
+						Redirect: true,
 					}
-					specs[j].IEs = sessionCfg.SessionIEs()
+					specs[j].IEs = sessionCfgs[j].SessionIEs()
 				}
 
 				seids, errs := f.PFCP.EstablishSessions(f.Context, specs)
 				for _, err := range errs {
 					framework.ExpectNoError(err)
 				}
 
+				framework.Logf("disabling redirects")
+				for j := 0; j < leakTestNumSessions; j++ {
+					sessionCfgs[j].Redirect = false
+					specs[j].SEID = seids[j]
+					specs[j].IEs = append(
+						sessionCfgs[j].DeleteFARs(),
+						sessionCfgs[j].CreateFARs()...)
+				}
+				_, errs = f.PFCP.ModifySessions(f.Context, specs)
+				for _, err := range errs {
+					framework.ExpectNoError(err)
+				}
+
+				framework.Logf("enabling redirects")
+				for j := 0; j < leakTestNumSessions; j++ {
+					sessionCfgs[j].Redirect = true
+					specs[j].SEID = seids[j]
+					specs[j].IEs = append(
+						sessionCfgs[j].DeleteFARs(),
+						sessionCfgs[j].CreateFARs()...)
+				}
+				_, errs = f.PFCP.ModifySessions(f.Context, specs)
+				for _, err := range errs {
+					framework.ExpectNoError(err)
+				}
+
 				framework.Logf("deleting %d sessions", leakTestNumSessions)
 				deleteSessions(f, seids, false)
 			}

diff --git a/upf/upf_pfcp.c b/upf/upf_pfcp.c
@@ -761,6 +761,8 @@ static inline void
 pfcp_free_far (upf_far_t * far)
 {
   vec_free (far->forward.rewrite);
+  if (far->forward.flags & FAR_F_REDIRECT_INFORMATION)
+    free_redirect_information (&far->forward.redirect_information);
 }
 
 int
@@ -783,13 +785,17 @@ pfcp_make_pending_far (upf_session_t * sx)
 	upf_far_t *new = vec_elt_at_index (pending->far, i);
 
 	new->forward.rewrite = NULL;
-	if (!(old->apply_action & FAR_FORWARD)
-	    || old->forward.rewrite == NULL)
-	  {
-	    continue;
-	  }
+	clib_memset (&new->forward.redirect_information, 0,
+		     sizeof (new->forward.redirect_information));
+
+	if (!(old->apply_action & FAR_FORWARD))
+	  continue;
 
-	new->forward.rewrite = vec_dup (old->forward.rewrite);
+	if (old->forward.rewrite)
+	  new->forward.rewrite = vec_dup (old->forward.rewrite);
+	if (old->forward.flags & FAR_F_REDIRECT_INFORMATION)
+	  cpy_redirect_information (&new->forward.redirect_information,
+				    &old->forward.redirect_information);
       }
     }