diff --git a/pkg/ipamd/ipamd.go b/pkg/ipamd/ipamd.go index 155cc4048d..4bb740b078 100644 --- a/pkg/ipamd/ipamd.go +++ b/pkg/ipamd/ipamd.go @@ -216,8 +216,6 @@ type IPAMContext struct { lastInsufficientCidrError time.Time enableManageUntaggedMode bool enablePodIPAnnotation bool - // For IPv6 Security Groups for Pods, the gateway address is cached in order to speedup CNI ADD - v6Gateway net.IP } // setUnmanagedENIs will rebuild the set of ENI IDs for ENIs tagged as "no_manage" @@ -1059,14 +1057,13 @@ func (c *IPAMContext) setupENI(eni string, eniMetadata awsutils.ENIMetadata, isT return errors.Wrapf(err, "Failed to allocate IPv6 Prefixes to Primary ENI") } } else { - var gw net.IP // For other ENIs, set up the network if eni != primaryENI { subnetCidr := eniMetadata.SubnetIPv4CIDR if c.enableIPv6 { subnetCidr = eniMetadata.SubnetIPv6CIDR } - gw, err = c.networkClient.SetupENINetwork(c.primaryIP[eni], eniMetadata.MAC, eniMetadata.DeviceNumber, subnetCidr) + err = c.networkClient.SetupENINetwork(c.primaryIP[eni], eniMetadata.MAC, eniMetadata.DeviceNumber, subnetCidr) if err != nil { // Failed to set up the ENI errRemove := c.dataStore.RemoveENIFromDataStore(eni, true) @@ -1083,14 +1080,6 @@ func (c *IPAMContext) setupENI(eni string, eniMetadata awsutils.ENIMetadata, isT c.addENIsecondaryIPsToDataStore(eniMetadata.IPv4Addresses, eni) c.addENIv4prefixesToDataStore(eniMetadata.IPv4Prefixes, eni) } else { - // Cache the IPv6 gateway address to speed up CNI ADD. In IPv4, the gateway address is the .1 address for the subnet. In IPv6, the gateway - // address is a link-local address that is fixed for the lifetime of the instance and is consistent across instances in the same subnet. - c.v6Gateway = gw - // In strict mode, install gateway rule for ICMPv6 traffic. Note that this is a global rule, but installing/removing here is acceptable as - // IPv6 only supports attaching a trunk ENI, and trunk ENI is only attached once. - if err := c.networkClient.UpdateIPv6GatewayRule(&gw); err != nil { - return errors.Wrapf(err, "failed to install IPv6 gateway rule") - } // This is a trunk ENI in IPv6 PD mode, so do not add IPs or prefixes to datastore log.Infof("Found IPv6 trunk ENI having %d secondary IPs and %d Prefixes", len(eniMetadata.IPv6Addresses), len(eniMetadata.IPv6Prefixes)) } diff --git a/pkg/ipamd/ipamd_test.go b/pkg/ipamd/ipamd_test.go index 0c68c9ed7e..1cf0386b66 100644 --- a/pkg/ipamd/ipamd_test.go +++ b/pkg/ipamd/ipamd_test.go @@ -1877,7 +1877,7 @@ func TestIPAMContext_setupENI(t *testing.T) { newENIMetadata := getSecondaryENIMetadata() m.awsutils.EXPECT().GetPrimaryENI().Return(primaryENIid) - m.network.EXPECT().SetupENINetwork(gomock.Any(), secMAC, secDevice, primarySubnet).Return(nil, errors.New("not able to set route 0.0.0.0/0 via 10.10.10.1 table 2")) + m.network.EXPECT().SetupENINetwork(gomock.Any(), secMAC, secDevice, primarySubnet).Return(errors.New("not able to set route 0.0.0.0/0 via 10.10.10.1 table 2")) err = mockContext.setupENI(newENIMetadata.ENIID, newENIMetadata, false, false) assert.Error(t, err) @@ -1923,7 +1923,7 @@ func TestIPAMContext_setupENIwithPDenabled(t *testing.T) { newENIMetadata := getSecondaryENIMetadata() m.awsutils.EXPECT().GetPrimaryENI().Return(primaryENIid) - m.network.EXPECT().SetupENINetwork(gomock.Any(), secMAC, secDevice, primarySubnet).Return(nil, errors.New("not able to set route 0.0.0.0/0 via 10.10.10.1 table 2")) + m.network.EXPECT().SetupENINetwork(gomock.Any(), secMAC, secDevice, primarySubnet).Return(errors.New("not able to set route 0.0.0.0/0 via 10.10.10.1 table 2")) err = mockContext.setupENI(newENIMetadata.ENIID, newENIMetadata, false, false) assert.Error(t, err) diff --git a/pkg/ipamd/rpc_handler.go b/pkg/ipamd/rpc_handler.go index be148f5a6b..0eca3e07bc 100644 --- a/pkg/ipamd/rpc_handler.go +++ b/pkg/ipamd/rpc_handler.go @@ -142,7 +142,7 @@ func (s *server) AddNetwork(ctx context.Context, in *rpc.AddNetworkRequest) (*rp // For IPv6, the gateway is derived from the RA route on the primary ENI. The primary ENI is always in the same subnet as the trunk and branch ENI. // For IPv4, the gateway is always the .1 address for the subnet CIDR. if s.ipamContext.enableIPv6 { - gw = s.ipamContext.v6Gateway + gw = networkutils.GetIPv6Gateway() } else { gw = networkutils.GetIPv4Gateway(subnetCIDR) } diff --git a/pkg/networkutils/mocks/network_mocks.go b/pkg/networkutils/mocks/network_mocks.go index fb1c64ce2e..a4c9016777 100644 --- a/pkg/networkutils/mocks/network_mocks.go +++ b/pkg/networkutils/mocks/network_mocks.go @@ -124,12 +124,11 @@ func (mr *MockNetworkAPIsMockRecorder) GetRuleListBySrc(arg0, arg1 interface{}) } // SetupENINetwork mocks base method. -func (m *MockNetworkAPIs) SetupENINetwork(arg0, arg1 string, arg2 int, arg3 string) (net.IP, error) { +func (m *MockNetworkAPIs) SetupENINetwork(arg0, arg1 string, arg2 int, arg3 string) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SetupENINetwork", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(net.IP) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret0, _ := ret[0].(error) + return ret0 } // SetupENINetwork indicates an expected call of SetupENINetwork. @@ -180,20 +179,6 @@ func (mr *MockNetworkAPIsMockRecorder) UpdateHostIptablesRules(arg0, arg1, arg2, return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateHostIptablesRules", reflect.TypeOf((*MockNetworkAPIs)(nil).UpdateHostIptablesRules), arg0, arg1, arg2, arg3, arg4) } -// UpdateIPv6GatewayRule mocks base method. -func (m *MockNetworkAPIs) UpdateIPv6GatewayRule(arg0 *net.IP) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateIPv6GatewayRule", arg0) - ret0, _ := ret[0].(error) - return ret0 -} - -// UpdateIPv6GatewayRule indicates an expected call of UpdateIPv6GatewayRule. -func (mr *MockNetworkAPIsMockRecorder) UpdateIPv6GatewayRule(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateIPv6GatewayRule", reflect.TypeOf((*MockNetworkAPIs)(nil).UpdateIPv6GatewayRule), arg0) -} - // UpdateRuleListBySrc mocks base method. func (m *MockNetworkAPIs) UpdateRuleListBySrc(arg0 []netlink.Rule, arg1 net.IPNet) error { m.ctrl.T.Helper() diff --git a/pkg/networkutils/network.go b/pkg/networkutils/network.go index 9888776dca..8ad8394025 100644 --- a/pkg/networkutils/network.go +++ b/pkg/networkutils/network.go @@ -149,7 +149,7 @@ type NetworkAPIs interface { SetupHostNetwork(vpcCIDRs []string, primaryMAC string, primaryAddr *net.IP, enablePodENI bool, v4Enabled bool, v6Enabled bool) error // SetupENINetwork performs ENI level network configuration. Not needed on the primary ENI - SetupENINetwork(eniIP string, mac string, deviceNumber int, subnetCIDR string) (net.IP, error) + SetupENINetwork(eniIP string, mac string, deviceNumber int, subnetCIDR string) error // UpdateHostIptablesRules updates the nat table iptables rules on the host UpdateHostIptablesRules(vpcCIDRs []string, primaryMAC string, primaryAddr *net.IP, v4Enabled bool, v6Enabled bool) error UseExternalSNAT() bool @@ -160,7 +160,6 @@ type NetworkAPIs interface { UpdateRuleListBySrc(ruleList []netlink.Rule, src net.IPNet) error UpdateExternalServiceIpRules(ruleList []netlink.Rule, externalIPs []string) error GetLinkByMac(mac string, retryInterval time.Duration) (netlink.Link, error) - UpdateIPv6GatewayRule(gw *net.IP) error } type linuxNetwork struct { @@ -359,6 +358,13 @@ func (n *linuxNetwork) SetupHostNetwork(vpcv4CIDRs []string, primaryMAC string, if err != nil && !containsNoSuchRule(err) { return errors.Wrap(err, "ChangeLocalRulePriority: failed to delete priority 0 local rule") } + + // In IPv6 strict mode, ICMPv6 packets from the gateway must lookup in the local routing table so that branch interfaces can resolve their gateway. + if v6Enabled { + if err := n.createIPv6GatewayRule(); err != nil { + return errors.Wrapf(err, "failed to install IPv6 gateway rule") + } + } } return n.updateHostIptablesRules(vpcv4CIDRs, primaryMAC, primaryAddr, v4Enabled, v6Enabled) @@ -954,23 +960,10 @@ func linkByMac(mac string, netLink netlinkwrapper.NetLink, retryInterval time.Du } } -// For IPv6, derive gateway address from primary ENI's RA route. Note that in IPv6, all ENIs must be in the same subnet. -func GetIPv6Gateway() (net.IP, error) { - primaryEni, err := netlink.LinkByName("eth0") - if err != nil { - return nil, errors.Wrapf(err, "GetIPv6Gateway failed to get primary ENI") - } - primaryEniRoutes, err := netlink.RouteList(primaryEni, unix.AF_INET6) - if err != nil { - return nil, errors.Wrapf(err, "GetIPv6Gateway failed to list primary ENI routes") - } - for _, route := range primaryEniRoutes { - if route.Table == mainRoutingTable && len(route.Gw) != 0 && route.Protocol.String() == "ra" { - log.Infof("Found IPv6 gateway: %s", route.Gw.String()) - return route.Gw, nil - } - } - return nil, errors.Wrapf(err, "GetIPv6Gateway failed to find eth0 ra route") +// On AWS/VPC, the subnet gateway can always be reached at FE80:EC2::1 +// https://aws.amazon.com/about-aws/whats-new/2022/11/ipv6-subnet-default-gateway-router-multiple-addresses/ +func GetIPv6Gateway() net.IP { + return net.IP{0xfe, 0x80, 0x0e, 0xc2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1} } func GetIPv4Gateway(eniSubnetCIDR *net.IPNet) net.IP { @@ -980,42 +973,40 @@ func GetIPv4Gateway(eniSubnetCIDR *net.IPNet) net.IP { } // SetupENINetwork adds default route to route table (eni-), so it does not need to be called on the primary ENI -func (n *linuxNetwork) SetupENINetwork(eniIP string, eniMAC string, deviceNumber int, eniSubnetCIDR string) (net.IP, error) { +func (n *linuxNetwork) SetupENINetwork(eniIP string, eniMAC string, deviceNumber int, eniSubnetCIDR string) error { return setupENINetwork(eniIP, eniMAC, deviceNumber, eniSubnetCIDR, n.netLink, retryLinkByMacInterval, retryRouteAddInterval, n.mtu) } func setupENINetwork(eniIP string, eniMAC string, deviceNumber int, eniSubnetCIDR string, netLink netlinkwrapper.NetLink, - retryLinkByMacInterval time.Duration, retryRouteAddInterval time.Duration, mtu int) (net.IP, error) { + retryLinkByMacInterval time.Duration, retryRouteAddInterval time.Duration, mtu int) error { if deviceNumber == 0 { - return nil, errors.New("setupENINetwork should never be called on the primary ENI") + return errors.New("setupENINetwork should never be called on the primary ENI") } tableNumber := deviceNumber + 1 log.Infof("Setting up network for an ENI with IP address %s, MAC address %s, CIDR %s and route table %d", eniIP, eniMAC, eniSubnetCIDR, tableNumber) link, err := linkByMac(eniMAC, netLink, retryLinkByMacInterval) if err != nil { - return nil, errors.Wrapf(err, "setupENINetwork: failed to find the link which uses MAC address %s", eniMAC) + return errors.Wrapf(err, "setupENINetwork: failed to find the link which uses MAC address %s", eniMAC) } if err = netLink.LinkSetMTU(link, mtu); err != nil { - return nil, errors.Wrapf(err, "setupENINetwork: failed to set MTU to %d for %s", mtu, eniIP) + return errors.Wrapf(err, "setupENINetwork: failed to set MTU to %d for %s", mtu, eniIP) } if err = netLink.LinkSetUp(link); err != nil { - return nil, errors.Wrapf(err, "setupENINetwork: failed to bring up ENI %s", eniIP) + return errors.Wrapf(err, "setupENINetwork: failed to bring up ENI %s", eniIP) } isV6 := strings.Contains(eniSubnetCIDR, ":") _, eniSubnetIPNet, err := net.ParseCIDR(eniSubnetCIDR) if err != nil { - return nil, errors.Wrapf(err, "setupENINetwork: invalid IP CIDR block %s", eniSubnetCIDR) + return errors.Wrapf(err, "setupENINetwork: invalid IP CIDR block %s", eniSubnetCIDR) } // Get gateway IP address for ENI var gw net.IP if isV6 { - if gw, err = GetIPv6Gateway(); err != nil { - return nil, errors.Wrapf(err, "setupENINetwork: unable to get IPv6 gateway") - } + gw = GetIPv6Gateway() } else { gw = GetIPv4Gateway(eniSubnetIPNet) } @@ -1032,14 +1023,14 @@ func setupENINetwork(eniIP string, eniMAC string, deviceNumber int, eniSubnetCID var addrs []netlink.Addr addrs, err = netLink.AddrList(link, family) if err != nil { - return nil, errors.Wrap(err, "setupENINetwork: failed to list IP address for ENI") + return errors.Wrap(err, "setupENINetwork: failed to list IP address for ENI") } for _, addr := range addrs { if addr.IP.IsGlobalUnicast() { log.Debugf("Deleting existing IP address %s", addr.String()) if err = netLink.AddrDel(link, &addr); err != nil { - return nil, errors.Wrap(err, "setupENINetwork: failed to delete IP addr from ENI") + return errors.Wrap(err, "setupENINetwork: failed to delete IP addr from ENI") } } } @@ -1051,7 +1042,7 @@ func setupENINetwork(eniIP string, eniMAC string, deviceNumber int, eniSubnetCID } log.Debugf("Adding IP address %s", eniAddr.String()) if err = netLink.AddrAdd(link, &netlink.Addr{IPNet: eniAddr}); err != nil { - return nil, errors.Wrap(err, "setupENINetwork: failed to add IP addr to ENI") + return errors.Wrap(err, "setupENINetwork: failed to add IP addr to ENI") } linkIndex := link.Attrs().Index @@ -1082,7 +1073,7 @@ func setupENINetwork(eniIP string, eniMAC string, deviceNumber int, eniSubnetCID for _, r := range routes { err := netLink.RouteDel(&r) if err != nil && !netlinkwrapper.IsNotExistsError(err) { - return nil, errors.Wrap(err, "setupENINetwork: failed to clean up old routes") + return errors.Wrap(err, "setupENINetwork: failed to clean up old routes") } err = retry.NWithBackoff(retry.NewSimpleBackoff(500*time.Millisecond, retryRouteAddInterval, 0.15, 2.0), maxRetryRouteAdd, func() error { @@ -1094,7 +1085,7 @@ func setupENINetwork(eniIP string, eniMAC string, deviceNumber int, eniSubnetCID return nil }) if err != nil { - return gw, err + return err } } @@ -1110,7 +1101,7 @@ func setupENINetwork(eniIP string, eniMAC string, deviceNumber int, eniSubnetCID // eniSubnetIPNet was modified by GetIPv4Gateway, so the string must be parsed again _, eniSubnetCIDRNet, err := net.ParseCIDR(eniSubnetCIDR) if err != nil { - return gw, errors.Wrapf(err, "setupENINetwork: invalid IPv4 CIDR block: %s", eniSubnetCIDR) + return errors.Wrapf(err, "setupENINetwork: invalid IPv4 CIDR block: %s", eniSubnetCIDR) } defaultRoute = netlink.Route{ Dst: eniSubnetCIDRNet, @@ -1121,16 +1112,16 @@ func setupENINetwork(eniIP string, eniMAC string, deviceNumber int, eniSubnetCID } if err := netLink.RouteDel(&defaultRoute); err != nil { if !netlinkwrapper.IsNotExistsError(err) { - return gw, errors.Wrapf(err, "setupENINetwork: unable to delete default route %s for source IP %s", eniSubnetIPNet.String(), eniIP) + return errors.Wrapf(err, "setupENINetwork: unable to delete default route %s for source IP %s", eniSubnetIPNet.String(), eniIP) } } - return gw, nil + return nil } // For IPv6 strict mode, ICMPv6 packets from the gateway must lookup in the local routing table so that branch interfaces can resolve their gateway. -func (n *linuxNetwork) UpdateIPv6GatewayRule(gw *net.IP) error { +func (n *linuxNetwork) createIPv6GatewayRule() error { gatewayRule := n.netLink.NewRule() - gatewayRule.Src = &net.IPNet{IP: *gw, Mask: net.CIDRMask(128, 128)} + gatewayRule.Src = &net.IPNet{IP: GetIPv6Gateway(), Mask: net.CIDRMask(128, 128)} gatewayRule.IPProto = unix.IPPROTO_ICMPV6 gatewayRule.Table = localRouteTable gatewayRule.Priority = 0 @@ -1138,13 +1129,13 @@ func (n *linuxNetwork) UpdateIPv6GatewayRule(gw *net.IP) error { if n.podSGEnforcingMode == sgpp.EnforcingModeStrict { err := n.netLink.RuleAdd(gatewayRule) if err != nil && !isRuleExistsError(err) { - return errors.Wrap(err, "UpdateIPv6GatewayRule: unable to create rule for IPv6 gateway") + return errors.Wrap(err, "createIPv6GatewayRule: unable to create rule for IPv6 gateway") } } else { // Rule must be deleted when not in strict mode to support transitions. err := n.netLink.RuleDel(gatewayRule) if !netlinkwrapper.IsNotExistsError(err) { - return errors.Wrap(err, "UpdateIPv6GatewayRule: unable to delete rule for IPv6 gateway") + return errors.Wrap(err, "createIPv6GatewayRule: unable to delete rule for IPv6 gateway") } } return nil diff --git a/pkg/networkutils/network_test.go b/pkg/networkutils/network_test.go index a1c280fa86..b16b018ca9 100644 --- a/pkg/networkutils/network_test.go +++ b/pkg/networkutils/network_test.go @@ -122,7 +122,7 @@ func TestSetupENINetwork(t *testing.T) { mockNetLink.EXPECT().RouteDel(gomock.Any()).Return(nil) - _, err = setupENINetwork(testEniIP, testMAC2, testTable, testEniSubnet, mockNetLink, 0*time.Second, 0*time.Second, testMTU) + err = setupENINetwork(testEniIP, testMAC2, testTable, testEniSubnet, mockNetLink, 0*time.Second, 0*time.Second, testMTU) assert.NoError(t, err) } @@ -171,7 +171,7 @@ func TestSetupENIV6Network(t *testing.T) { mockNetLink.EXPECT().RouteReplace(gomock.Any()).Return(nil) mockNetLink.EXPECT().RouteDel(gomock.Any()).Return(nil) - _, err = setupENINetwork(testEniIP6, testMAC2, testTable, testEniV6Subnet, mockNetLink, 0*time.Second, 0*time.Second, testMTU) + err = setupENINetwork(testEniIP6, testMAC2, testTable, testEniV6Subnet, mockNetLink, 0*time.Second, 0*time.Second, testMTU) assert.NoError(t, err) } @@ -185,7 +185,7 @@ func TestSetupENINetworkMACFail(t *testing.T) { mockNetLink.EXPECT().LinkList().Return(nil, fmt.Errorf("simulated failure")) } - _, err := setupENINetwork(testEniIP, testMAC2, testTable, testEniSubnet, mockNetLink, 0*time.Second, 0*time.Second, testMTU) + err := setupENINetwork(testEniIP, testMAC2, testTable, testEniSubnet, mockNetLink, 0*time.Second, 0*time.Second, testMTU) assert.Errorf(t, err, "simulated failure") } @@ -193,7 +193,7 @@ func TestSetupENINetworkErrorOnPrimaryENI(t *testing.T) { ctrl, mockNetLink, _, _, _ := setup(t) defer ctrl.Finish() deviceNumber := 0 - _, err := setupENINetwork(testEniIP, testMAC2, deviceNumber, testEniSubnet, mockNetLink, 0*time.Second, 0*time.Second, testMTU) + err := setupENINetwork(testEniIP, testMAC2, deviceNumber, testEniSubnet, mockNetLink, 0*time.Second, 0*time.Second, testMTU) assert.Error(t, err) } @@ -218,7 +218,7 @@ func TestUpdateIPv6GatewayRule(t *testing.T) { mockNetLink.EXPECT().NewRule().Return(&icmpRule) mockNetLink.EXPECT().RuleAdd(&icmpRule) - err := ln.UpdateIPv6GatewayRule(&testEniV6GatewayNet) + err := ln.createIPv6GatewayRule() assert.NoError(t, err) // Validate rule del in non-strict mode @@ -226,7 +226,7 @@ func TestUpdateIPv6GatewayRule(t *testing.T) { mockNetLink.EXPECT().NewRule().Return(&icmpRule) mockNetLink.EXPECT().RuleDel(&icmpRule) - err = ln.UpdateIPv6GatewayRule(&testEniV6GatewayNet) + err = ln.createIPv6GatewayRule() assert.NoError(t, err) }