Skip to content

Commit

Permalink
Fix device number and update table name the device index
Browse files Browse the repository at this point in the history
  • Loading branch information
Claes Mogren committed Jul 17, 2020
1 parent 0da7491 commit 85dcb40
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 45 deletions.
28 changes: 14 additions & 14 deletions cmd/routed-eni-cni-plugin/driver/driver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ const (

// NetworkAPIs defines network API calls
type NetworkAPIs interface {
SetupNS(hostVethName string, contVethName string, netnsPath string, addr *net.IPNet, table int, vpcCIDRs []string, useExternalSNAT bool, mtu int, log logger.Logger) error
TeardownNS(addr *net.IPNet, table int, log logger.Logger) error
SetupNS(hostVethName string, contVethName string, netnsPath string, addr *net.IPNet, deviceNumber int, vpcCIDRs []string, useExternalSNAT bool, mtu int, log logger.Logger) error
TeardownNS(addr *net.IPNet, deviceNumber int, log logger.Logger) error
}

type linuxNetwork struct {
Expand Down Expand Up @@ -168,12 +168,12 @@ func (createVethContext *createVethPairContext) run(hostNS ns.NetNS) error {
}

// SetupNS wires up linux networking for a pod's network
func (os *linuxNetwork) SetupNS(hostVethName string, contVethName string, netnsPath string, addr *net.IPNet, table int, vpcCIDRs []string, useExternalSNAT bool, mtu int, log logger.Logger) error {
log.Debugf("SetupNS: hostVethName=%s, contVethName=%s, netnsPath=%s, table=%d, mtu=%d", hostVethName, contVethName, netnsPath, table, mtu)
return setupNS(hostVethName, contVethName, netnsPath, addr, table, vpcCIDRs, useExternalSNAT, os.netLink, os.ns, mtu, log, os.procSys)
func (os *linuxNetwork) SetupNS(hostVethName string, contVethName string, netnsPath string, addr *net.IPNet, deviceNumber int, vpcCIDRs []string, useExternalSNAT bool, mtu int, log logger.Logger) error {
log.Debugf("SetupNS: hostVethName=%s, contVethName=%s, netnsPath=%s, deviceNumber=%d, mtu=%d", hostVethName, contVethName, netnsPath, deviceNumber, mtu)
return setupNS(hostVethName, contVethName, netnsPath, addr, deviceNumber, vpcCIDRs, useExternalSNAT, os.netLink, os.ns, mtu, log, os.procSys)
}

func setupNS(hostVethName string, contVethName string, netnsPath string, addr *net.IPNet, table int, vpcCIDRs []string, useExternalSNAT bool,
func setupNS(hostVethName string, contVethName string, netnsPath string, addr *net.IPNet, deviceNumber int, vpcCIDRs []string, useExternalSNAT bool,
netLink netlinkwrapper.NetLink, ns nswrapper.NS, mtu int, log logger.Logger, procSys procsyswrapper.ProcSys) error {
// Clean up if hostVeth exists.
if oldHostVeth, err := netLink.LinkByName(hostVethName); err == nil {
Expand Down Expand Up @@ -246,19 +246,19 @@ func setupNS(hostVethName string, contVethName string, netnsPath string, addr *n
if table > 0 {
if useExternalSNAT {
// add rule: 1536: from <podIP> use table <table>
err = addContainerRule(netLink, false, addr, table)
err = addContainerRule(netLink, false, addr, deviceNumber)
if err != nil {
log.Errorf("Failed to add fromContainer rule for %s err: %v", addr.String(), err)
return errors.Wrap(err, "add NS network: failed to add fromContainer rule")
}
log.Infof("Added rule priority %d from %s table %d", fromContainerRulePriority, addr.String(), table)
log.Infof("Added rule priority %d from %s table %d", fromContainerRulePriority, addr.String(), deviceNumber)
} else {
// add rule: 1536: list of from <podIP> to <vpcCIDR> use table <table>
for _, cidr := range vpcCIDRs {
podRule := netLink.NewRule()
_, podRule.Dst, _ = net.ParseCIDR(cidr)
podRule.Src = addr
podRule.Table = table
podRule.Table = deviceNumber
podRule.Priority = fromContainerRulePriority

err = netLink.RuleAdd(podRule)
Expand Down Expand Up @@ -311,12 +311,12 @@ func addContainerRule(netLink netlinkwrapper.NetLink, isToContainer bool, addr *
}

// TeardownPodNetwork cleanup ip rules
func (os *linuxNetwork) TeardownNS(addr *net.IPNet, table int, log logger.Logger) error {
log.Debugf("TeardownNS: addr %s, table %d", addr.String(), table)
return tearDownNS(addr, table, os.netLink, log)
func (os *linuxNetwork) TeardownNS(addr *net.IPNet, deviceNumber int, log logger.Logger) error {
log.Debugf("TeardownNS: addr %s, deviceNumber %d", addr.String(), deviceNumber)
return tearDownNS(addr, deviceNumber, os.netLink, log)
}

func tearDownNS(addr *net.IPNet, table int, netLink netlinkwrapper.NetLink, log logger.Logger) error {
func tearDownNS(addr *net.IPNet, deviceNumber int, netLink netlinkwrapper.NetLink, log logger.Logger) error {
if addr == nil {
return errors.New("can't tear down network namespace with no IP address")
}
Expand All @@ -339,7 +339,7 @@ func tearDownNS(addr *net.IPNet, table int, netLink netlinkwrapper.NetLink, log
log.Errorf("Failed to delete fromContainer for %s %v", addr.String(), err)
return errors.Wrapf(err, "delete NS network: failed to delete fromContainer rule for %s", addr.String())
}
log.Infof("Delete fromContainer rule for %s in table %d", addr.String(), table)
log.Infof("Delete fromContainer rule for %s in table %d", addr.String(), deviceNumber)
}

addrHostAddr := &net.IPNet{
Expand Down
26 changes: 14 additions & 12 deletions pkg/awsutils/awsutils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -588,38 +588,40 @@ func (cache *EC2InstanceMetadataCache) getIPsAndCIDR(eniMAC string) ([]*ec2.Netw
}

// getENIDeviceNumber returns ENI ID, device number, error
func (cache *EC2InstanceMetadataCache) getENIDeviceNumber(eniMAC string) (string, int, error) {
func (cache *EC2InstanceMetadataCache) getENIDeviceNumber(eniMAC string) (eniID string, deviceNumber int, err error) {
// get device-number
start := time.Now()
device, err := cache.ec2Metadata.GetMetadata(metadataMACPath + eniMAC + metadataDeviceNum)
awsAPILatency.WithLabelValues("GetMetadata", fmt.Sprint(err != nil)).Observe(msSince(start))
if err != nil {
awsAPIErrInc("GetMetadata", err)
log.Errorf("Failed to retrieve the device-number of ENI %s, %v", eniMAC, err)
return "", 0, errors.Wrapf(err, "failed to retrieve device-number for ENI %s",
eniMAC)
return "", -1, errors.Wrapf(err, "failed to retrieve device-number for ENI %s", eniMAC)
}

deviceNum, err := strconv.ParseInt(device, 0, 32)
deviceNumber, err = strconv.Atoi(device)
if err != nil {
return "", 0, errors.Wrapf(err, "invalid device %s for ENI %s", device, eniMAC)
return "", -1, errors.Wrapf(err, "invalid device %s for ENI %s", device, eniMAC)
}

start = time.Now()
eni, err := cache.ec2Metadata.GetMetadata(metadataMACPath + eniMAC + metadataInterface)
eniID, err = cache.ec2Metadata.GetMetadata(metadataMACPath + eniMAC + metadataInterface)
awsAPILatency.WithLabelValues("GetMetadata", fmt.Sprint(err != nil)).Observe(msSince(start))
if err != nil {
awsAPIErrInc("GetMetadata", err)
log.Errorf("Failed to retrieve the interface-id data from instance metadata service, %v", err)
return "", 0, errors.Wrapf(err, "get attached ENIs: failed to retrieve interface-id for ENI %s", eniMAC)
return "", -1, errors.Wrapf(err, "get attached ENIs: failed to retrieve interface-id for ENI %s", eniMAC)
}

if cache.primaryENI == eni {
log.Debugf("Using device number 0 for primary eni: %s", eni)
return eni, 0, nil
if cache.primaryENI == eniID {
log.Debugf("Using device number 0 for primary ENI: %s", eniID)
if deviceNumber != 0 {
log.Warnf("Device number of primary ENI is %d! It was expected to be 0", deviceNumber)
}
return eniID, deviceNumber, nil
}
// 0 is reserved for primary ENI, the rest of them has to +1 to avoid collision at 0
return eni, int(deviceNum + 1), nil
// 0 is reserved for primary ENI
return eniID, deviceNumber, nil
}

// awsGetFreeDeviceNumber calls EC2 API DescribeInstances to get the next free device index
Expand Down
8 changes: 4 additions & 4 deletions pkg/ipamd/datastore/data_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ type ENI struct {
createTime time.Time
// IsPrimary indicates whether ENI is a primary ENI
IsPrimary bool
// DeviceNumber is the device number of ENI (0 means the primary ENI)
// DeviceNumber is the device number of the ENI
DeviceNumber int
// IPv4Addresses shows whether each address is assigned, the key is IP address, which must
// be in dot-decimal notation with no leading zeros and no whitespace(eg: "10.1.0.253")
Expand Down Expand Up @@ -440,7 +440,7 @@ func (ds *DataStore) DelIPv4AddressFromStore(eniID string, ipv4 string, force bo

// AssignPodIPv4Address assigns an IPv4 address to pod
// It returns the assigned IPv4 address, device number, error
func (ds *DataStore) AssignPodIPv4Address(ipamKey IPAMKey) (string, int, error) {
func (ds *DataStore) AssignPodIPv4Address(ipamKey IPAMKey) (ipv4address string, deviceNumber int, err error) {
ds.lock.Lock()
defer ds.lock.Unlock()

Expand All @@ -458,7 +458,7 @@ func (ds *DataStore) AssignPodIPv4Address(ipamKey IPAMKey) (string, int, error)
ds.log.Warnf("Failed to update backing store: %v", err)
// Important! Unwind assignment
ds.unassignPodIPv4AddressUnsafe(eni, addr)
return "", 0, err
return "", -1, err
}

return addr.Address, eni.DeviceNumber, nil
Expand All @@ -467,7 +467,7 @@ func (ds *DataStore) AssignPodIPv4Address(ipamKey IPAMKey) (string, int, error)
ds.log.Debugf("AssignPodIPv4Address: ENI %s does not have available addresses", eni.ID)
}
ds.log.Errorf("DataStore has no available IP addresses")
return "", 0, errors.New("assignPodIPv4AddressUnsafe: no available IP addresses")
return "", -1, errors.New("assignPodIPv4AddressUnsafe: no available IP addresses")
}

// It returns the assigned IPv4 address, device number
Expand Down
30 changes: 15 additions & 15 deletions pkg/networkutils/network.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ var log = logger.Get()
type NetworkAPIs interface {
// SetupNodeNetwork performs node level network configuration
SetupHostNetwork(vpcCIDR *net.IPNet, vpcCIDRs []string, primaryMAC string, primaryAddr *net.IP) error
// SetupENINetwork performs eni level network configuration
SetupENINetwork(eniIP string, mac string, table int, subnetCIDR string) error
// SetupENINetwork performs ENI level network configuration
SetupENINetwork(eniIP string, mac string, deviceNumber int, subnetCIDR string) error
UseExternalSNAT() bool
GetExcludeSNATCIDRs() []string
GetRuleList() ([]netlink.Rule, error)
Expand Down Expand Up @@ -662,20 +662,21 @@ func LinkByMac(mac string, netLink netlinkwrapper.NetLink, retryInterval time.Du
}

// SetupENINetwork adds default route to route table (eni-<eni_table>)
func (n *linuxNetwork) SetupENINetwork(eniIP string, eniMAC string, eniTable int, eniSubnetCIDR string) error {
return setupENINetwork(eniIP, eniMAC, eniTable, eniSubnetCIDR, n.netLink, retryLinkByMacInterval, retryRouteAddInterval, n.mtu)
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, eniTable int, eniSubnetCIDR string, netLink netlinkwrapper.NetLink,
func setupENINetwork(eniIP string, eniMAC string, deviceNumber int, eniSubnetCIDR string, netLink netlinkwrapper.NetLink,
retryLinkByMacInterval time.Duration, retryRouteAddInterval time.Duration, mtu int) error {

if eniTable == 0 {
// TODO: Check primary through metadata instead
if deviceNumber == 0 {
log.Debugf("Skipping set up ENI network for primary interface")
return nil
}

log.Infof("Setting up network for an ENI with IP address %s, MAC address %s, CIDR %s and route table %d",
eniIP, eniMAC, eniSubnetCIDR, eniTable)
eniIP, eniMAC, eniSubnetCIDR, deviceNumber)
link, err := LinkByMac(eniMAC, netLink, retryLinkByMacInterval)
if err != nil {
return errors.Wrapf(err, "setupENINetwork: failed to find the link which uses MAC address %s", eniMAC)
Expand All @@ -689,8 +690,6 @@ func setupENINetwork(eniIP string, eniMAC string, eniTable int, eniSubnetCIDR st
return errors.Wrapf(err, "setupENINetwork: failed to bring up ENI %s", eniIP)
}

deviceNumber := link.Attrs().Index

_, ipnet, err := net.ParseCIDR(eniSubnetCIDR)
if err != nil {
return errors.Wrapf(err, "setupENINetwork: invalid IPv4 CIDR block %s", eniSubnetCIDR)
Expand Down Expand Up @@ -727,22 +726,23 @@ func setupENINetwork(eniIP string, eniMAC string, eniTable int, eniSubnetCIDR st
return errors.Wrap(err, "setupENINetwork: failed to add IP addr to ENI")
}

log.Debugf("Setting up ENI's default gateway %v", gw)
linkIndex := link.Attrs().Index
log.Debugf("Setting up ENI's default gateway %v, table %d, linkIndex %d", gw, deviceNumber, linkIndex)
routes := []netlink.Route{
// Add a direct link route for the host's ENI IP only
{
LinkIndex: deviceNumber,
LinkIndex: linkIndex,
Dst: &net.IPNet{IP: gw, Mask: net.CIDRMask(32, 32)},
Scope: netlink.SCOPE_LINK,
Table: eniTable,
Table: deviceNumber,
},
// Route all other traffic via the host's ENI IP
{
LinkIndex: deviceNumber,
LinkIndex: linkIndex,
Dst: &net.IPNet{IP: net.IPv4zero, Mask: net.CIDRMask(0, 32)},
Scope: netlink.SCOPE_UNIVERSE,
Gw: gw,
Table: eniTable,
Table: deviceNumber,
},
}
for _, r := range routes {
Expand All @@ -754,7 +754,7 @@ func setupENINetwork(eniIP string, eniMAC string, eniTable int, eniSubnetCIDR st
err = retry.RetryNWithBackoff(retry.NewSimpleBackoff(500*time.Millisecond, retryRouteAddInterval, 0.15, 2.0), maxRetryRouteAdd, func() error {
if err := netLink.RouteReplace(&r); err != nil {
log.Debugf("Not able to set route %s/0 via %s table %d",
r.Dst.IP.String(), gw.String(), eniTable)
r.Dst.IP.String(), gw.String(), deviceNumber)
return errors.Wrapf(err, "setupENINetwork: unable to replace route entry %s", r.Dst.IP.String())
}

Expand Down

0 comments on commit 85dcb40

Please sign in to comment.