ipsec: Atomically upgrade XFRM states with new output-mark

Commit def1770 ("ipsec: Clear node ID and SPI with output-mark")
updates the output-mark of all our XFRM states. Without special logic to
handle the upgrade, these new states can however conflict with the
existing states. In that case, the agent will remove the existing state
and add the new one. It works fine, but can lead to a very short time
where packets are dropped because there isn't any matching XFRM state.
It also causes an error log for the conflicting states.

This commit adds the special upgrade logic to handle that situation. We
have two cases to handle, depending on where we upgrade from.

If upgrading from a version after commit 3e59b68 ("ipsec: Per-node
XFRM states & policies for EKS & AKS"), then the existing XFRM states
differ only by their output-marks. In that case, we can simply update
the state in place atomically using the appropriate netlink call.

If upgrading from a version that doesn't include commit 3e59b68, then
the XFRM states may differ significantly. In that case, we already have
some logic in place to remove the old conflicting states and add the new
ones (the netlink API doesn't let us perform an atomic update in that
case [1]). To handle the additional difference in output-marks, we
simply need to update the existing logic to not assume the output-marks
are the same. This is also the correct way to detect conflicting states
anyway, given the kernel doesn't consider the output-marks when looking
for conflicts.

This change was tested with upgrades to v1.13.7 with this patch
backported to it, on:
- IPAM=ENI, upgrading from v1.13.0 (before commit 3e59b68).
- IPAM=ENI, upgrading from v1.13.7.
- IPAM=cluster-pool, upgrading from v1.13.0.

In all three cases, I ran the full connectivity tests successfully and
checked that the logs didn't have any errors or warnings after the
upgrade. I also deployed the migrate-svc app before the upgrade to
ensure the upgrade doesn't cause any transient packet drops [2].

1 - c0d9b8c ("ipsec: Allow old and new XFRM OUT states to coexist for upgrade")
2 - Note that the second case isn't atomically updating the XFRM states
    so can in principle cause packet drops. The window for such drops
    is however so small (time between two netlink calls) that they
    haven't been reproduced yet (neither now nor when testing
    c0d9b8c).
Fixes: def1770 ("ipsec: Clear node ID and SPI with output-mark")
Reported-by: Hart Hoover <hart@isovalent.com>
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>

pkg/datapath/linux/ipsec/ipsec_linux.go

@@ -200,9 +200,14 @@ func xfrmStateReplace(new *netlink.XfrmState) error {
 	// Check if the XFRM state already exists
 	for _, s := range states {
 		if xfrmIPEqual(s.Src, new.Src) && xfrmIPEqual(s.Dst, new.Dst) &&
-			xfrmMarkEqual(s.OutputMark, new.OutputMark) &&
 			xfrmMarkEqual(s.Mark, new.Mark) && s.Spi == new.Spi {
-			return nil
+			if xfrmMarkEqual(s.OutputMark, new.OutputMark) {
+				return nil
+			} else {
+				// If only the output-marks differ, then we should be able
+				// to simply update the XFRM state atomically.
+				return netlink.XfrmStateUpdate(new)
+			}
 		}
 	}
 
@@ -228,7 +233,6 @@ func xfrmStateReplace(new *netlink.XfrmState) error {
 		// and can be removed in v1.15. Finally, this shouldn't happen with ENI
 		// and Azure IPAM modes because they don't have such conflicting states.
 		if xfrmIPEqual(s.Src, new.Src) && xfrmIPEqual(s.Dst, new.Dst) &&
-			xfrmMarkEqual(s.OutputMark, new.OutputMark) &&
 			xfrmMarkEqual(s.Mark, oldXFRMMark) && s.Spi == new.Spi {
 			err := netlink.XfrmStateDel(&s)
 			if err != nil {