endpoint: Only perform full synchronization periodically

Performing a full dump of the policy map on every policy map synchronization
is expensive and is only necessary to catch rare cases where the agent's view
of the policy map has diverged from the kernel's view which should only happen
either due to kernel or other bugs or some other application modifying the
endpoint policy map.

To reduce the cost of synchronizing large endpoint policy maps perform the
full synchronization only periodically. The full synchronization is defaults
to 15 minutes. Configurable with the hidden option
"--policy-map-full-reconciliation-interval".

Fixes: 9dc1350 ("endpoint: Enhance policy map sync")
Signed-off-by: Jussi Maki <jussi@isovalent.com>

daemon/cmd/daemon_main.go

@@ -840,6 +840,10 @@ func InitGlobalFlags(cmd *cobra.Command, vp *viper.Viper) {
 	flags.Int(option.PolicyMapEntriesName, policymap.MaxEntries, "Maximum number of entries in endpoint policy map (per endpoint)")
 	option.BindEnv(vp, option.PolicyMapEntriesName)
 
+	flags.Duration(option.PolicyMapFullReconciliationIntervalName, 15*time.Minute, "Interval for full reconciliation of endpoint policy map")
+	option.BindEnv(vp, option.PolicyMapFullReconciliationIntervalName)
+	flags.MarkHidden(option.PolicyMapFullReconciliationIntervalName)
+
 	flags.Int(option.SockRevNatEntriesName, option.SockRevNATMapEntriesDefault, "Maximum number of entries for the SockRevNAT BPF map")
 	option.BindEnv(vp, option.SockRevNatEntriesName)
 

pkg/endpoint/bpf.go

@@ -1277,20 +1277,26 @@ func (e *Endpoint) dumpPolicyMapToMapState() (policy.MapState, error) {
 	return currentMap, err
 }
 
-// syncPolicyMapWithDump attempts to synchronize the PolicyMap for this endpoint to
-// contain the set of PolicyKeys represented by the endpoint's desiredMapState.
-// It checks the current contents of the endpoint's PolicyMap and deletes any
-// PolicyKeys that are not present in the endpoint's desiredMapState. It then
-// adds any keys that are not present in the map. When a key from desiredMapState
-// is inserted successfully to the endpoint's BPF PolicyMap, it is added to the
-// endpoint's realizedMapState field. Returns an error if the endpoint's BPF
-// PolicyMap is unable to be dumped, or any update operation to the map fails.
+// syncPolicyMapWithDump is invoked periodically to perform a full reconciliation
+// of the endpoint's PolicyMap against the BPF maps to catch cases where either
+// due to kernel issue or user intervention the agent's view of the PolicyMap
+// state has diverged from the kernel. A warning is logged if this method finds
+// such an discrepancy.
+//
+// Returns an error if the endpoint's BPF PolicyMap is unable to be dumped,
+// or any update operation to the map fails.
 // Must be called with e.mutex Lock()ed.
 func (e *Endpoint) syncPolicyMapWithDump() error {
 	if e.policyMap == nil {
 		return fmt.Errorf("not syncing PolicyMap state for endpoint because PolicyMap is nil")
 	}
 
+	// Endpoint not yet fully initialized or currently regenerating. Skip the check
+	// this round.
+	if e.getState() != StateReady {
+		return nil
+	}
+
 	// Apply pending policy map changes first so that desired map is up-to-date before
 	// we diff the maps below.
 	err := e.applyPolicyMapChanges()
@@ -1339,12 +1345,18 @@ func (e *Endpoint) syncPolicyMapWithDump() error {
 	return err
 }
 
-func (e *Endpoint) syncPolicyMapController() {
+const (
+	// syncPolicyMapInterval is the interval for periodic full reconciliation of
+	// the policy map to catch unexpected discrepancies with agent and kernel state.
+	syncPolicyMapInterval = 15 * time.Minute
+)
+
+func (e *Endpoint) startSyncPolicyMapController() {
 	ctrlName := fmt.Sprintf("sync-policymap-%d", e.ID)
-	e.controllers.UpdateController(ctrlName,
+	e.controllers.CreateController(ctrlName,
 		controller.ControllerParams{
 			Group: syncPolicymapControllerGroup,
-			DoFunc: func(ctx context.Context) (reterr error) {
+			DoFunc: func(ctx context.Context) error {
 				// Failure to lock is not an error, it means
 				// that the endpoint was disconnected and we
 				// should exit gracefully.
@@ -1354,7 +1366,7 @@ func (e *Endpoint) syncPolicyMapController() {
 				defer e.unlock()
 				return e.syncPolicyMapWithDump()
 			},
-			RunInterval: 1 * time.Minute,
+			RunInterval: syncPolicyMapInterval,
 			Context:     e.aliveCtx,
 		},
 	)

pkg/endpoint/policy.go

@@ -526,9 +526,10 @@ func (e *Endpoint) updateRealizedState(stats *regenerationStatistics, origDir st
 		return fmt.Errorf("error synchronizing endpoint BPF program directories: %s", err)
 	}
 
-	// Keep PolicyMap for this endpoint in sync with desired / realized state.
+	// Start periodic background full reconciliation of the policy map.
+	// Does nothing if it has already been started.
 	if !option.Config.DryMode {
-		e.syncPolicyMapController()
+		e.startSyncPolicyMapController()
 	}
 
 	if e.desiredPolicy != e.realizedPolicy {

pkg/option/config.go

@@ -658,6 +658,10 @@ const (
 	// PolicyMapEntriesName configures max entries for BPF policymap.
 	PolicyMapEntriesName = "bpf-policy-map-max"
 
+	// PolicyMapFullReconciliationInterval sets the interval for performing the full
+	// reconciliation of the endpoint policy map.
+	PolicyMapFullReconciliationIntervalName = "bpf-policy-map-full-reconciliation-interval"
+
 	// SockRevNatEntriesName configures max entries for BPF sock reverse nat
 	// entries.
 	SockRevNatEntriesName = "bpf-sock-rev-map-max"
@@ -1582,6 +1586,10 @@ type DaemonConfig struct {
 	// endpoint may allow traffic to exchange traffic with.
 	PolicyMapEntries int
 
+	// PolicyMapFullReconciliationInterval is the interval at which to perform
+	// the full reconciliation of the endpoint policy map.
+	PolicyMapFullReconciliationInterval time.Duration
+
 	// SockRevNatEntries is the maximum number of sock rev nat mappings
 	// allowed in the BPF rev nat table
 	SockRevNatEntries int
@@ -3869,6 +3877,7 @@ func (c *DaemonConfig) calculateBPFMapSizes(vp *viper.Viper) error {
 	c.NATMapEntriesGlobal = vp.GetInt(NATMapEntriesGlobalName)
 	c.NeighMapEntriesGlobal = vp.GetInt(NeighMapEntriesGlobalName)
 	c.PolicyMapEntries = vp.GetInt(PolicyMapEntriesName)
+	c.PolicyMapFullReconciliationInterval = vp.GetDuration(PolicyMapFullReconciliationIntervalName)
 	c.SockRevNatEntries = vp.GetInt(SockRevNatEntriesName)
 	c.LBMapEntries = vp.GetInt(LBMapEntriesName)
 	c.LBServiceMapEntries = vp.GetInt(LBServiceMapMaxEntries)