Fix resizing of BPF maps.

Maps were being correctly resized but the program attachment
metadata wasn't updated so the programs failed to attach.

- Fix the test to actually verify basic workload connectivity.
- Add metadta updates to each of the attach points.

felix/bpf/maps/maps.go

@@ -576,6 +576,7 @@ func (b *PinnedMap) Open() error {
 }
 
 func (b *PinnedMap) repinAt(fd int, from, to string) error {
+	log.Infof("Repining BPF map from %s to %s", from, to)
 	err := libbpf.ObjPin(fd, to)
 	if err != nil {
 		return fmt.Errorf("error repinning %s to %s: %w", from, to, err)
@@ -604,19 +605,27 @@ func (b *PinnedMap) EnsureExists() error {
 		return nil
 	}
 
-	// In case felix restarts in the middle of migration, we might end up with
-	// old map. Repin the old map and let the map creation continue.
+	// In case felix restarts in the middle of migration, the in-use map
+	// will be pinned with suffix "_old" and the map in the normal place
+	// wil be a partially-migrated map.  Clean up the partial map and move
+	// the old map back into its normal place.
 	if b.oldMapExists() {
-		log.WithField("name", b.Name).Debug("Old map exists")
+		log.WithField("name", b.Name).Info("Old map exists (from previous migration attempt?)")
 		if _, err := os.Stat(b.Path()); err == nil {
-			os.Remove(b.Path())
+			err := os.Remove(b.Path())
+			if err != nil {
+				log.WithError(err).Warning("Failed to remove partially-migrated map.  Ignoring...")
+			}
 		}
 		fd, err := libbpf.ObjGet(oldMapPath)
 		if err != nil {
 			return fmt.Errorf("cannot get old map at %s: %w", oldMapPath, err)
 		}
 		err = b.repinAt(fd, oldMapPath, b.Path())
-		syscall.Close(fd)
+		closeErr := syscall.Close(fd)
+		if closeErr != nil {
+			log.WithError(err).Warn("Error from fd.Close().  Ignoring.")
+		}
 		if err != nil {
 			return fmt.Errorf("error repinning old map %s to %s, err=%w", oldMapPath, b.Path(), err)
 		}
@@ -641,23 +650,27 @@ func (b *PinnedMap) EnsureExists() error {
 			}).Warn("Map with same name but different parameters exists. Deleting it")
 		} else {
 			if b.MaxEntries == mapInfo.MaxEntries {
+				log.WithField("name", b.Name).Info("Map already exists with correct parameters.")
 				return nil
 			}
-			log.WithField("name", b.Name).Debugf("Size changed %d -> %d", mapInfo.MaxEntries, b.MaxEntries)
+			log.WithField("name", b.Name).Infof("BPF map size changed; need to migrate %d -> %d", mapInfo.MaxEntries, b.MaxEntries)
 
 			// store the old fd
 			b.oldfd = b.MapFD()
 			b.oldSize = mapInfo.MaxEntries
 
 			err = b.repinAt(int(b.MapFD()), b.Path(), oldMapPath)
 			if err != nil {
-				return fmt.Errorf("error migrating the old map %w", err)
+				return fmt.Errorf("error repinning the old map %w", err)
 			}
 			copyData = true
 			// Do not close the oldfd if the map is updated by the BPF programs.
 			if !b.UpdatedByBPF {
 				defer func() {
-					b.oldfd.Close()
+					err := b.oldfd.Close()
+					if err != nil {
+						log.WithError(err).Warn("Error closing old map fd. Ignoring.")
+					}
 					b.oldfd = 0
 				}()
 			}
@@ -690,10 +703,13 @@ func (b *PinnedMap) EnsureExists() error {
 			// same version but of different size.
 			err := b.copyFromOldMap()
 			if err != nil {
-				b.fd.Close()
+				log.WithError(err).Error("error copying data from old map")
+				closeErr := b.fd.Close()
+				if closeErr != nil {
+					log.WithError(closeErr).Warn("Error when closing FD, ignoring...")
+				}
 				b.fd = 0
 				b.fdLoaded = false
-				log.WithError(err).Error("error copying data from old map")
 				return err
 			}
 			// Delete the old pin if the map is not updated by BPF programs.
@@ -707,7 +723,10 @@ func (b *PinnedMap) EnsureExists() error {
 		// Handle map upgrade.
 		err = b.upgrade()
 		if err != nil {
-			b.fd.Close()
+			closeErr := b.fd.Close()
+			if closeErr != nil {
+				log.WithError(closeErr).Warn("Error when closing FD, ignoring...")
+			}
 			b.fd = 0
 			b.fdLoaded = false
 			return err

felix/bpf/tc/attach.go

@@ -30,6 +30,7 @@ import (
 	"github.com/projectcalico/calico/felix/bpf/bpfdefs"
 	"github.com/projectcalico/calico/felix/bpf/hook"
 	"github.com/projectcalico/calico/felix/bpf/libbpf"
+	"github.com/projectcalico/calico/felix/bpf/maps"
 	tcdefs "github.com/projectcalico/calico/felix/bpf/tc/defs"
 )
 
@@ -99,6 +100,12 @@ func (ap *AttachPoint) loadObject(file string) (*libbpf.Obj, error) {
 			continue
 		}
 
+		if size := maps.Size(mapName); size != 0 {
+			if err := m.SetSize(size); err != nil {
+				return nil, fmt.Errorf("error resizing map %s: %w", mapName, err)
+			}
+		}
+
 		log.Debugf("Pinning map %s k %d v %d", mapName, m.KeySize(), m.ValueSize())
 		pinDir := bpf.MapPinDir(m.Type(), mapName, ap.Iface, ap.Hook)
 		if err := m.SetPinPath(path.Join(pinDir, mapName)); err != nil {

felix/bpf/xdp/attach.go

@@ -26,6 +26,7 @@ import (
 	"github.com/projectcalico/calico/felix/bpf/bpfdefs"
 	"github.com/projectcalico/calico/felix/bpf/hook"
 	"github.com/projectcalico/calico/felix/bpf/libbpf"
+	"github.com/projectcalico/calico/felix/bpf/maps"
 	tcdefs "github.com/projectcalico/calico/felix/bpf/tc/defs"
 )
 
@@ -166,7 +167,13 @@ func (ap *AttachPoint) AttachProgram() (bpf.AttachResult, error) {
 			}
 			continue
 		}
-		// TODO: We need to set map size here like tc.
+
+		if size := maps.Size(mapName); size != 0 {
+			if err := m.SetSize(size); err != nil {
+				return nil, fmt.Errorf("error resizing map %s: %w", mapName, err)
+			}
+		}
+
 		pinDir := bpf.MapPinDir(m.Type(), mapName, ap.Iface, hook.XDP)
 		if err := m.SetPinPath(path.Join(pinDir, mapName)); err != nil {
 			return nil, fmt.Errorf("error pinning map %s: %w", mapName, err)

felix/fv/bpf_map_resize_test.go

@@ -20,22 +20,23 @@ import (
 	"context"
 	"encoding/base64"
 	"encoding/json"
+	"fmt"
 	"net"
-	"os"
 	"strings"
 	"time"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-
 	api "github.com/projectcalico/api/pkg/apis/projectcalico/v3"
 
 	"github.com/projectcalico/calico/felix/bpf/conntrack"
 	"github.com/projectcalico/calico/felix/bpf/ipsets"
 	"github.com/projectcalico/calico/felix/bpf/maps"
 	"github.com/projectcalico/calico/felix/bpf/nat"
 	"github.com/projectcalico/calico/felix/bpf/routes"
+	"github.com/projectcalico/calico/felix/fv/connectivity"
 	"github.com/projectcalico/calico/felix/fv/infrastructure"
+	"github.com/projectcalico/calico/felix/fv/workload"
 	"github.com/projectcalico/calico/felix/timeshim"
 	"github.com/projectcalico/calico/libcalico-go/lib/apiconfig"
 	client "github.com/projectcalico/calico/libcalico-go/lib/clientv3"
@@ -45,7 +46,7 @@ import (
 
 var _ = infrastructure.DatastoreDescribe("_BPF-SAFE_ Felix bpf test configurable map size", []apiconfig.DatastoreType{apiconfig.EtcdV3}, func(getInfra infrastructure.InfraFactory) {
 
-	if os.Getenv("FELIX_FV_ENABLE_BPF") != "true" {
+	if !BPFMode() {
 		// Non-BPF run.
 		return
 	}
@@ -54,19 +55,26 @@ var _ = infrastructure.DatastoreDescribe("_BPF-SAFE_ Felix bpf test configurable
 		infra  infrastructure.DatastoreInfra
 		tc     infrastructure.TopologyContainers
 		client client.Interface
+
+		w  [2]*workload.Workload
+		cc *connectivity.Checker
 	)
 
 	BeforeEach(func() {
 		infra = getInfra()
 		opts := infrastructure.DefaultTopologyOptions()
 		tc, client = infrastructure.StartNNodeTopology(1, opts, infra)
+
+		infra.AddDefaultAllow()
 	})
 
 	AfterEach(func() {
 		if CurrentGinkgoTestDescription().Failed {
 			infra.DumpErrorData()
 		}
-
+		for _, wl := range w {
+			wl.Stop()
+		}
 		tc.Stop()
 		infra.Stop()
 	})
@@ -115,7 +123,6 @@ var _ = infrastructure.DatastoreDescribe("_BPF-SAFE_ Felix bpf test configurable
 		out, err = tc.Felixes[0].ExecOutput("calico-bpf", "conntrack", "dump")
 		Expect(err).NotTo(HaveOccurred())
 		Expect(strings.Count(out, srcIP.String())).To(Equal(1), "entry not found in conntrack map")
-
 	})
 
 	It("should program new map sizes", func() {
@@ -155,6 +162,24 @@ var _ = infrastructure.DatastoreDescribe("_BPF-SAFE_ Felix bpf test configurable
 		Eventually(getMapSizeFn(felix, affMap), "10s", "200ms").Should(Equal(newNATAffSize))
 		Eventually(getMapSizeFn(felix, ipsMap), "10s", "200ms").Should(Equal(newIpSetMapSize))
 		Eventually(getMapSizeFn(felix, ctMap), "10s", "200ms").Should(Equal(newCtMapSize))
+
+		// Add some workloads after resize to verify that provisioning is working with the new map sizes.
+		for i := range w {
+			w[i] = workload.Run(
+				tc.Felixes[0],
+				fmt.Sprintf("w%d", i),
+				"default",
+				fmt.Sprintf("10.65.0.%d", i+2),
+				"8080",
+				"tcp",
+			)
+			w[i].ConfigureInInfra(infra)
+		}
+		cc = &connectivity.Checker{}
+
+		cc.Expect(connectivity.Some, w[0], w[1])
+		cc.Expect(connectivity.Some, w[1], w[0])
+		cc.CheckConnectivity()
 	})
 })