Send IPv4 GARP and IPv6 Unsolicited NA in "cmdAdd"

In "cmdAdd", SRIOV-CNI would construct and send IPv4 Gratuitous ARP
and/or Unsolicited Neighbor Advertisement depending on the IP
addresses configured by IPAM. The reason why this change is needed
is for the scenario when an IP address is reused by IPAM with
different interfaces (with different link-layer addresses). This can
occur when pods are deleted and created. For performance reasons,
sending of GARP and/or Unsolicited NA would update invalid ARP/Neighbor
caches in other neighbors/nodes.

Also we set IPv4 ARP Notify and IPv6 Neighbor Discovery Notify in sysfs
for each interface. This will send GARP and/or Unsolicited NA when the
interface is either brought up or the link-layer address changes. This
is useful in cases where an application reenables the interface or the
MAC address configuration is changed.

Some new packages were added, thus go.mod and go.sum were modified
accordingly.

Mocked PciUtils for sriov tests since sriov.go would call PciUtils to
set IPv4 ARP Notify and IPv6 Neighbor Discovery.

Fixes #177

Signed-off-by: William Zhao <wizhao@redhat.com>

Developer.md

@@ -0,0 +1,23 @@
+# Developer Readme
+
+* [Using Mockery](#using-mockery)
+
+## Using Mockery
+
+Mockery (https://github.com/vektra/mockery) is used to auto-generate mock files for golang interfaces. The advantage of using Mockery is that there will be no need to manually write boilerplate code for mocking interfaces.
+
+Reading the readme file in Mockery is recommended to understand how to get started with Mockery.
+
+For each package, there may be a "mocks" folder with mock files generated by Mockery. To generate the mock for a particular interface the following command format should be used at the root project directory:
+
+```
+docker run -v "$PWD":/src -w /src vektra/mockery --recursive=true --name=<Interface Name> --output=./pkg/<Package name where the interface is defined>/mocks/ --filename=<Interface Name>_mock.go --exported
+```
+
+An example for mocking the "pciUtils" interface in the "sriov" package is as follows:
+
+```
+docker run -v "$PWD":/src -w /src vektra/mockery --recursive=true --name=pciUtils --output=./pkg/sriov/mocks/ --filename=pci_utils_mock.go --exported
+```
+
+This will create the "mocks" directory if not present and will auto-generate the mock file "pci_utils_mock.go" for the "pciUtils" interface.

cmd/sriov/main.go

@@ -135,7 +135,27 @@ func cmdAdd(args *skel.CmdArgs) error {
 
 		if !netConf.DPDKMode {
 			err = netns.Do(func(_ ns.NetNS) error {
-				return ipam.ConfigureIface(args.IfName, newResult)
+				err := ipam.ConfigureIface(args.IfName, newResult)
+				if err != nil {
+					return err
+				}
+
+				/* After IPAM configuration is done, the following needs to handle the case of an IP address being reused by a different pods.
+				 * This is achieved by sending Gratuitous ARPs and/or Unsolicited Neighbor Advertisements unconditionally.
+				 * Although we set arp_notify and ndisc_notify unconditionally on the interface (please see EnableArpAndNdiscNotify()), the kernel
+				 * only sends GARPs/Unsolicited NA when the interface goes from down to up, or when the link-layer address changes on the interfaces.
+				 * These scenarios are perfectly valid and recommended to be enabled for optimal network performance.
+				 * However for our specific case, which the kernel is unaware of, is the reuse of IP addresses across pods where each pod has a different
+				 * link-layer address for it's SRIOV interface. The ARP/Neighbor cache residing in neighbors would be invalid if an IP address is reused.
+				 * In order to update the cache, the GARP/Unsolicited NA packets should be sent for performance reasons. Otherwise, the neighbors
+				 * may be sending packets with the incorrect link-layer address. Eventually, most network stacks would send ARPs and/or Neighbor
+				 * Solicitation packets when the connection is unreachable. This would correct the invalid cache; however this may take a significant
+				 * amount of time to complete.
+				 *
+				 * The error is ignored here because enabling this feature is only a performance enhancement.
+				 */
+				_ = utils.AnnounceIPs(args.IfName, newResult.IPs)
+				return nil
 			})
 			if err != nil {
 				return err

go.mod

@@ -9,6 +9,7 @@ require (
 	github.com/onsi/gomega v0.0.0-20151007035656-2152b45fa28a
 	github.com/stretchr/testify v1.4.0
 	github.com/vishvananda/netlink v1.0.1-0.20190924205540-07ace697bea4
+	golang.org/x/net v0.0.0-20220802222814-0bcc04d9c69b
 )
 
 require (
@@ -18,6 +19,6 @@ require (
 	github.com/safchain/ethtool v0.0.0-20190326074333-42ed695e3de8 // indirect
 	github.com/stretchr/objx v0.1.0 // indirect
 	github.com/vishvananda/netns v0.0.0-20180720170159-13995c7128cc // indirect
-	golang.org/x/sys v0.0.0-20210510120138-977fb7262007 // indirect
+	golang.org/x/sys v0.0.0-20220728004956-3c1f35247d10 // indirect
 	gopkg.in/yaml.v2 v2.2.2 // indirect
 )

go.sum

@@ -43,9 +43,15 @@ github.com/vishvananda/netns v0.0.0-20180720170159-13995c7128cc h1:R83G5ikgLMxrB
 github.com/vishvananda/netns v0.0.0-20180720170159-13995c7128cc/go.mod h1:ZjcWmFBXmLKZu9Nxj3WKYEafiSqer2rnvPr0en9UNpI=
 golang.org/x/crypto v0.0.0-20181009213950-7c1a557ab941/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
 golang.org/x/net v0.0.0-20181011144130-49bb7cea24b1/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
+golang.org/x/net v0.0.0-20220802222814-0bcc04d9c69b h1:3ogNYyK4oIQdIKzTu68hQrr4iuVxF3AxKl9Aj/eDrw0=
+golang.org/x/net v0.0.0-20220802222814-0bcc04d9c69b/go.mod h1:YDH+HFinaLZZlnHAfSS6ZXJJ9M9t4Dl22yv3iI2vPwk=
 golang.org/x/sys v0.0.0-20190616124812-15dcb6c0061f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20210510120138-977fb7262007 h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE=
-golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20220728004956-3c1f35247d10 h1:WIoqL4EROvwiPdUtaip4VcDdpZ4kha7wBWZrbVKCIZg=
+golang.org/x/sys v0.0.0-20220728004956-3c1f35247d10/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
+golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
+golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 gopkg.in/airbrake/gobrake.v2 v2.0.9/go.mod h1:/h5ZAUhDkGaJfjzjKLSjv6zCL6O0LLBxU4K+aSYdM/U=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

pkg/sriov/sriov.go

@@ -11,116 +11,31 @@ import (
 	"github.com/vishvananda/netlink"
 )
 
-// mocked netlink interface
-// required for unit tests
-
-// NetlinkManager is an interface to mock nelink library
-type NetlinkManager interface {
-	LinkByName(string) (netlink.Link, error)
-	LinkSetVfVlan(netlink.Link, int, int) error
-	LinkSetVfVlanQos(netlink.Link, int, int, int) error
-	LinkSetVfHardwareAddr(netlink.Link, int, net.HardwareAddr) error
-	LinkSetHardwareAddr(netlink.Link, net.HardwareAddr) error
-	LinkSetUp(netlink.Link) error
-	LinkSetDown(netlink.Link) error
-	LinkSetNsFd(netlink.Link, int) error
-	LinkSetName(netlink.Link, string) error
-	LinkSetVfRate(netlink.Link, int, int, int) error
-	LinkSetVfSpoofchk(netlink.Link, int, bool) error
-	LinkSetVfTrust(netlink.Link, int, bool) error
-	LinkSetVfState(netlink.Link, int, uint32) error
-}
-
-// MyNetlink NetlinkManager
-type MyNetlink struct {
-	NetlinkManager
-}
-
-// LinkByName implements NetlinkManager
-func (n *MyNetlink) LinkByName(name string) (netlink.Link, error) {
-	return netlink.LinkByName(name)
-}
-
-// LinkSetVfVlan using NetlinkManager
-func (n *MyNetlink) LinkSetVfVlan(link netlink.Link, vf, vlan int) error {
-	return netlink.LinkSetVfVlan(link, vf, vlan)
-}
-
-// LinkSetVfVlanQos sets VLAN ID and QoS field for given VF using NetlinkManager
-func (n *MyNetlink) LinkSetVfVlanQos(link netlink.Link, vf, vlan, qos int) error {
-	return netlink.LinkSetVfVlanQos(link, vf, vlan, qos)
-}
-
-// LinkSetVfHardwareAddr using NetlinkManager
-func (n *MyNetlink) LinkSetVfHardwareAddr(link netlink.Link, vf int, hwaddr net.HardwareAddr) error {
-	return netlink.LinkSetVfHardwareAddr(link, vf, hwaddr)
-}
-
-// LinkSetHardwareAddr using NetlinkManager
-func (n *MyNetlink) LinkSetHardwareAddr(link netlink.Link, hwaddr net.HardwareAddr) error {
-	return netlink.LinkSetHardwareAddr(link, hwaddr)
-}
-
-// LinkSetUp using NetlinkManager
-func (n *MyNetlink) LinkSetUp(link netlink.Link) error {
-	return netlink.LinkSetUp(link)
-}
-
-// LinkSetDown using NetlinkManager
-func (n *MyNetlink) LinkSetDown(link netlink.Link) error {
-	return netlink.LinkSetDown(link)
-}
-
-// LinkSetNsFd using NetlinkManager
-func (n *MyNetlink) LinkSetNsFd(link netlink.Link, fd int) error {
-	return netlink.LinkSetNsFd(link, fd)
-}
-
-// LinkSetName using NetlinkManager
-func (n *MyNetlink) LinkSetName(link netlink.Link, name string) error {
-	return netlink.LinkSetName(link, name)
-}
-
-// LinkSetVfRate using NetlinkManager
-func (n *MyNetlink) LinkSetVfRate(link netlink.Link, vf int, minRate int, maxRate int) error {
-	return netlink.LinkSetVfRate(link, vf, minRate, maxRate)
-}
-
-// LinkSetVfSpoofchk using NetlinkManager
-func (n *MyNetlink) LinkSetVfSpoofchk(link netlink.Link, vf int, check bool) error {
-	return netlink.LinkSetVfSpoofchk(link, vf, check)
-}
-
-// LinkSetVfTrust using NetlinkManager
-func (n *MyNetlink) LinkSetVfTrust(link netlink.Link, vf int, state bool) error {
-	return netlink.LinkSetVfTrust(link, vf, state)
-}
-
-// LinkSetVfState using NetlinkManager
-func (n *MyNetlink) LinkSetVfState(link netlink.Link, vf int, state uint32) error {
-	return netlink.LinkSetVfState(link, vf, state)
-}
-
 type pciUtils interface {
-	getSriovNumVfs(ifName string) (int, error)
-	getVFLinkNamesFromVFID(pfName string, vfID int) ([]string, error)
-	getPciAddress(ifName string, vf int) (string, error)
+	GetSriovNumVfs(ifName string) (int, error)
+	GetVFLinkNamesFromVFID(pfName string, vfID int) ([]string, error)
+	GetPciAddress(ifName string, vf int) (string, error)
+	EnableArpAndNdiscNotify(ifName string) error
 }
 
 type pciUtilsImpl struct{}
 
-func (p *pciUtilsImpl) getSriovNumVfs(ifName string) (int, error) {
+func (p *pciUtilsImpl) GetSriovNumVfs(ifName string) (int, error) {
 	return utils.GetSriovNumVfs(ifName)
 }
 
-func (p *pciUtilsImpl) getVFLinkNamesFromVFID(pfName string, vfID int) ([]string, error) {
+func (p *pciUtilsImpl) GetVFLinkNamesFromVFID(pfName string, vfID int) ([]string, error) {
 	return utils.GetVFLinkNamesFromVFID(pfName, vfID)
 }
 
-func (p *pciUtilsImpl) getPciAddress(ifName string, vf int) (string, error) {
+func (p *pciUtilsImpl) GetPciAddress(ifName string, vf int) (string, error) {
 	return utils.GetPciAddress(ifName, vf)
 }
 
+func (p *pciUtilsImpl) EnableArpAndNdiscNotify(ifName string) error {
+	return utils.EnableArpAndNdiscNotify(ifName)
+}
+
 // Manager provides interface invoke sriov nic related operations
 type Manager interface {
 	SetupVF(conf *sriovtypes.NetConf, podifName string, cid string, netns ns.NetNS) (string, error)
@@ -130,14 +45,14 @@ type Manager interface {
 }
 
 type sriovManager struct {
-	nLink NetlinkManager
+	nLink utils.NetlinkManager
 	utils pciUtils
 }
 
 // NewSriovManager returns an instance of SriovManager
 func NewSriovManager() Manager {
 	return &sriovManager{
-		nLink: &MyNetlink{},
+		nLink: &utils.MyNetlink{},
 		utils: &pciUtilsImpl{},
 	}
 }
@@ -192,7 +107,11 @@ func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, cid s
 			return fmt.Errorf("error setting container interface name %s for %s", linkName, tempName)
 		}
 
-		// 6. Bring IF up in Pod netns
+		// 6. Enable IPv4 ARP notify and IPv6 Network Discovery notify
+		// Error is ignored here because enabling this feature is only a performance enhancement.
+		_ = s.utils.EnableArpAndNdiscNotify(podifName)
+
+		// 7. Bring IF up in Pod netns
 		if err := s.nLink.LinkSetUp(linkObj); err != nil {
 			return fmt.Errorf("error bringing interface up in container ns: %q", err)
 		}