From df2f5ff5719123ceee6051e2bd76910f1f4e0d19 Mon Sep 17 00:00:00 2001 From: Hongliang Liu <75655411+hongliangl@users.noreply.github.com> Date: Thu, 6 Jun 2024 09:49:39 +0800 Subject: [PATCH] Fix antrea-agent crashing with proxyAll enabled in networkPolicyOnly mode (#6259) In networkPolicyOnly mode and proxyAll is enabled, the ifindex of antrea-gw0 in `nodeConfig` is uninitialized, resulting in the failure to install the ip neighbor to antrea-gw0 due to the fact that the ifindex of antrea-gw0 is wrong. Additionally, the ipsets storing the pairs of Node IP and NodePort are not initialized and periodically synced. Consequently, this results in the failure to sync the iptables rules that referring to the ipsets. Signed-off-by: Hongliang Liu --- pkg/agent/agent.go | 8 ++++-- pkg/agent/route/route_linux.go | 45 +++++++++++++++++++--------------- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index 350770574bc..ebbd12c9bb3 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -771,7 +771,12 @@ func (i *Initializer) configureGatewayInterface(gatewayIface *interfacestore.Int klog.ErrorS(err, "Failed to persist interface MAC address", "interface", gatewayIface.InterfaceName, "mac", gwMAC) } } - i.nodeConfig.GatewayConfig = &config.GatewayConfig{Name: i.hostGateway, MAC: gwMAC, OFPort: uint32(gatewayIface.OFPort)} + i.nodeConfig.GatewayConfig = &config.GatewayConfig{ + Name: i.hostGateway, + MAC: gwMAC, + LinkIndex: gwLinkIdx, + OFPort: uint32(gatewayIface.OFPort), + } gatewayIface.IPs = []net.IP{} if i.networkConfig.TrafficEncapMode.IsNetworkPolicyOnly() { // Assign IP to gw as required by SpoofGuard. @@ -787,7 +792,6 @@ func (i *Initializer) configureGatewayInterface(gatewayIface *interfacestore.Int return nil } - i.nodeConfig.GatewayConfig.LinkIndex = gwLinkIdx // Allocate the gateway IP address for each Pod CIDR allocated to the Node. For each CIDR, // the first address in the subnet is assigned to the host gateway interface. podCIDRs := []*net.IPNet{i.nodeConfig.PodIPv4CIDR, i.nodeConfig.PodIPv6CIDR} diff --git a/pkg/agent/route/route_linux.go b/pkg/agent/route/route_linux.go index 31890415aa5..9b7702330de 100644 --- a/pkg/agent/route/route_linux.go +++ b/pkg/agent/route/route_linux.go @@ -361,30 +361,31 @@ func (c *Client) syncRoute() error { return nil } -// syncIPSet ensures that the required ipset exists and it has the initial members. +// syncIPSet ensures that the required ipset exists, and it has the initial members. func (c *Client) syncIPSet() error { - // In policy-only mode, Node Pod CIDR is undefined. - if c.networkConfig.TrafficEncapMode.IsNetworkPolicyOnly() { - return nil - } - if err := c.ipset.CreateIPSet(antreaPodIPSet, ipset.HashNet, false); err != nil { - return err - } - if err := c.ipset.CreateIPSet(antreaPodIP6Set, ipset.HashNet, true); err != nil { - return err - } - - // Loop all valid PodCIDR and add into the corresponding ipset. - for _, podCIDR := range []*net.IPNet{c.nodeConfig.PodIPv4CIDR, c.nodeConfig.PodIPv6CIDR} { - if podCIDR != nil { - ipsetName := getIPSetName(podCIDR.IP) - if err := c.ipset.AddEntry(ipsetName, podCIDR.String()); err != nil { - return err + // Create the ipsets to store all Pod CIDRs for constructing full-mesh routing in encap/noEncap/hybrid modes. In + // networkPolicyOnly mode, Antrea is not responsible for IPAM, so CIDRs are not available and the ipsets should not + // be created. + if !c.networkConfig.TrafficEncapMode.IsNetworkPolicyOnly() { + if err := c.ipset.CreateIPSet(antreaPodIPSet, ipset.HashNet, false); err != nil { + return err + } + if err := c.ipset.CreateIPSet(antreaPodIP6Set, ipset.HashNet, true); err != nil { + return err + } + // Loop all valid Pod CIDRs and add them into the corresponding ipset. + for _, podCIDR := range []*net.IPNet{c.nodeConfig.PodIPv4CIDR, c.nodeConfig.PodIPv6CIDR} { + if podCIDR != nil { + ipsetName := getIPSetName(podCIDR.IP) + if err := c.ipset.AddEntry(ipsetName, podCIDR.String()); err != nil { + return err + } } } } - // If proxy full is enabled, create NodePort ipset. + // AntreaProxy proxyAll is available in all traffic modes. If proxyAll is enabled, create the ipsets to store the + // pairs of Node IP and NodePort. if c.proxyAll { if err := c.ipset.CreateIPSet(antreaNodePortIPSet, ipset.HashIPPort, false); err != nil { return err @@ -409,6 +410,8 @@ func (c *Client) syncIPSet() error { }) } + // AntreaIPAM is available in noEncap mode. There is a validation in Antrea configuration about this traffic mode + // when AntreaIPAM is enabled. if c.connectUplinkToBridge { if err := c.ipset.CreateIPSet(localAntreaFlexibleIPAMPodIPSet, ipset.HashIP, false); err != nil { return err @@ -418,6 +421,7 @@ func (c *Client) syncIPSet() error { } } + // Multicast is available in encap/noEncap/hybrid mode, and the ipsets are consumed in encap mode. if c.multicastEnabled && c.networkConfig.TrafficEncapMode.SupportsEncap() { if err := c.ipset.CreateIPSet(clusterNodeIPSet, ipset.HashIP, false); err != nil { return err @@ -441,6 +445,7 @@ func (c *Client) syncIPSet() error { }) } + // NodeNetworkPolicy is available in all traffic modes. if c.nodeNetworkPolicyEnabled { c.nodeNetworkPolicyIPSetsIPv4.Range(func(key, value any) bool { ipsetName := key.(string) @@ -1817,7 +1822,7 @@ func (c *Client) AddLocalAntreaFlexibleIPAMPodRule(podAddresses []net.IP) error return nil } -// DeletLocaleAntreaFlexibleIPAMPodRule is used to delete related IP set entries when an AntreaFlexibleIPAM Pod is deleted. +// DeleteLocalAntreaFlexibleIPAMPodRule is used to delete related IP set entries when an AntreaFlexibleIPAM Pod is deleted. func (c *Client) DeleteLocalAntreaFlexibleIPAMPodRule(podAddresses []net.IP) error { if !c.connectUplinkToBridge { return nil