From d56c98986adb8aa9cb7f7d34f9f8c0a3e05efbbb Mon Sep 17 00:00:00 2001 From: Flavio Crisciani Date: Mon, 14 Aug 2017 09:20:55 -0700 Subject: [PATCH 1/3] Remove useless flags on operations In the peerDelete the updateDB flag was always true In the peerAdd the updateDB flag was always true except for the initSandbox case. But now the initSandbox is handled by the go routing of the peer operations, so we can move that flag down and remove it from the top level functions Signed-off-by: Flavio Crisciani (cherry picked from commit 396db359ba0a34a2c52cbb1681ae380d72ed3d3d) --- drivers/overlay/joinleave.go | 6 +++--- drivers/overlay/ov_network.go | 2 +- drivers/overlay/ov_serf.go | 5 ++--- drivers/overlay/overlay.go | 2 +- drivers/overlay/peerdb.go | 24 +++++++++--------------- 5 files changed, 16 insertions(+), 23 deletions(-) diff --git a/drivers/overlay/joinleave.go b/drivers/overlay/joinleave.go index 31c311f4fc..d50162f0d0 100644 --- a/drivers/overlay/joinleave.go +++ b/drivers/overlay/joinleave.go @@ -120,7 +120,7 @@ func (d *driver) Join(nid, eid string, sboxKey string, jinfo driverapi.JoinInfo, } } - d.peerAdd(nid, eid, ep.addr.IP, ep.addr.Mask, ep.mac, net.ParseIP(d.advertiseAddress), true, false, false, true) + d.peerAdd(nid, eid, ep.addr.IP, ep.addr.Mask, ep.mac, net.ParseIP(d.advertiseAddress), false, false, true) if err := d.checkEncryption(nid, nil, n.vxlanID(s), true, true); err != nil { logrus.Warn(err) @@ -200,11 +200,11 @@ func (d *driver) EventNotify(etype driverapi.EventType, nid, tableName, key stri } if etype == driverapi.Delete { - d.peerDelete(nid, eid, addr.IP, addr.Mask, mac, vtep, true) + d.peerDelete(nid, eid, addr.IP, addr.Mask, mac, vtep) return } - d.peerAdd(nid, eid, addr.IP, addr.Mask, mac, vtep, true, false, false, false) + d.peerAdd(nid, eid, addr.IP, addr.Mask, mac, vtep, false, false, false) } // Leave method is invoked when a Sandbox detaches from an endpoint. diff --git a/drivers/overlay/ov_network.go b/drivers/overlay/ov_network.go index f23edd850e..3ca4e36080 100644 --- a/drivers/overlay/ov_network.go +++ b/drivers/overlay/ov_network.go @@ -772,7 +772,7 @@ func (n *network) watchMiss(nlSock *nl.NetlinkSocket) { logrus.Errorf("could not resolve peer %q: %v", ip, err) continue } - n.driver.peerAdd(n.id, "dummy", ip, IPmask, mac, vtep, true, l2Miss, l3Miss, false) + n.driver.peerAdd(n.id, "dummy", ip, IPmask, mac, vtep, l2Miss, l3Miss, false) } else { // If the gc_thresh values are lower kernel might knock off the neighor entries. // When we get a L3 miss check if its a valid peer and reprogram the neighbor diff --git a/drivers/overlay/ov_serf.go b/drivers/overlay/ov_serf.go index 20954ef237..0bc1d12421 100644 --- a/drivers/overlay/ov_serf.go +++ b/drivers/overlay/ov_serf.go @@ -120,10 +120,9 @@ func (d *driver) processEvent(u serf.UserEvent) { switch action { case "join": - d.peerAdd(nid, eid, net.ParseIP(ipStr), net.IPMask(net.ParseIP(maskStr).To4()), mac, net.ParseIP(vtepStr), - true, false, false, false) + d.peerAdd(nid, eid, net.ParseIP(ipStr), net.IPMask(net.ParseIP(maskStr).To4()), mac, net.ParseIP(vtepStr), false, false, false) case "leave": - d.peerDelete(nid, eid, net.ParseIP(ipStr), net.IPMask(net.ParseIP(maskStr).To4()), mac, net.ParseIP(vtepStr), true) + d.peerDelete(nid, eid, net.ParseIP(ipStr), net.IPMask(net.ParseIP(maskStr).To4()), mac, net.ParseIP(vtepStr)) } } diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index 11eda6781b..6c63888561 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -170,7 +170,7 @@ func (d *driver) restoreEndpoints() error { } n.incEndpointCount() - d.peerAdd(ep.nid, ep.id, ep.addr.IP, ep.addr.Mask, ep.mac, net.ParseIP(d.advertiseAddress), true, false, false, true) + d.peerAdd(ep.nid, ep.id, ep.addr.IP, ep.addr.Mask, ep.mac, net.ParseIP(d.advertiseAddress), false, false, true) } return nil } diff --git a/drivers/overlay/peerdb.go b/drivers/overlay/peerdb.go index b22f93570d..7dda761693 100644 --- a/drivers/overlay/peerdb.go +++ b/drivers/overlay/peerdb.go @@ -234,7 +234,6 @@ type peerOperation struct { peerIPMask net.IPMask peerMac net.HardwareAddr vtepIP net.IP - updateDB bool l2Miss bool l3Miss bool localPeer bool @@ -252,9 +251,9 @@ func (d *driver) peerOpRoutine(ctx context.Context, ch chan *peerOperation) { case peerOperationINIT: err = d.peerInitOp(op.networkID) case peerOperationADD: - err = d.peerAddOp(op.networkID, op.endpointID, op.peerIP, op.peerIPMask, op.peerMac, op.vtepIP, op.updateDB, op.l2Miss, op.l3Miss, op.localPeer) + err = d.peerAddOp(op.networkID, op.endpointID, op.peerIP, op.peerIPMask, op.peerMac, op.vtepIP, op.l2Miss, op.l3Miss, true, op.localPeer) case peerOperationDELETE: - err = d.peerDeleteOp(op.networkID, op.endpointID, op.peerIP, op.peerIPMask, op.peerMac, op.vtepIP, op.updateDB) + err = d.peerDeleteOp(op.networkID, op.endpointID, op.peerIP, op.peerIPMask, op.peerMac, op.vtepIP) } if err != nil { logrus.Warnf("Peer operation failed:%s op:%v", err, op) @@ -279,14 +278,14 @@ func (d *driver) peerInitOp(nid string) error { return false } - d.peerAddOp(nid, pEntry.eid, pKey.peerIP, pEntry.peerIPMask, pKey.peerMac, pEntry.vtep, false, false, false, false) + d.peerAddOp(nid, pEntry.eid, pKey.peerIP, pEntry.peerIPMask, pKey.peerMac, pEntry.vtep, false, false, false, pEntry.isLocal) // return false to loop on all entries return false }) } func (d *driver) peerAdd(nid, eid string, peerIP net.IP, peerIPMask net.IPMask, - peerMac net.HardwareAddr, vtep net.IP, updateDb, l2Miss, l3Miss, localPeer bool) { + peerMac net.HardwareAddr, vtep net.IP, l2Miss, l3Miss, localPeer bool) { callerName := common.CallerName(1) d.peerOpCh <- &peerOperation{ opType: peerOperationADD, @@ -296,7 +295,6 @@ func (d *driver) peerAdd(nid, eid string, peerIP net.IP, peerIPMask net.IPMask, peerIPMask: peerIPMask, peerMac: peerMac, vtepIP: vtep, - updateDB: updateDb, l2Miss: l2Miss, l3Miss: l3Miss, localPeer: localPeer, @@ -305,13 +303,13 @@ func (d *driver) peerAdd(nid, eid string, peerIP net.IP, peerIPMask net.IPMask, } func (d *driver) peerAddOp(nid, eid string, peerIP net.IP, peerIPMask net.IPMask, - peerMac net.HardwareAddr, vtep net.IP, updateDb, l2Miss, l3Miss, updateOnlyDB bool) error { + peerMac net.HardwareAddr, vtep net.IP, l2Miss, l3Miss, updateDB, updateOnlyDB bool) error { if err := validateID(nid, eid); err != nil { return err } - if updateDb { + if updateDB { d.peerDbAdd(nid, eid, peerIP, peerIPMask, peerMac, vtep, false) if updateOnlyDB { return nil @@ -368,7 +366,7 @@ func (d *driver) peerAddOp(nid, eid string, peerIP net.IP, peerIPMask net.IPMask } func (d *driver) peerDelete(nid, eid string, peerIP net.IP, peerIPMask net.IPMask, - peerMac net.HardwareAddr, vtep net.IP, updateDb bool) { + peerMac net.HardwareAddr, vtep net.IP) { callerName := common.CallerName(1) d.peerOpCh <- &peerOperation{ opType: peerOperationDELETE, @@ -378,22 +376,18 @@ func (d *driver) peerDelete(nid, eid string, peerIP net.IP, peerIPMask net.IPMas peerIPMask: peerIPMask, peerMac: peerMac, vtepIP: vtep, - updateDB: updateDb, callerName: callerName, } } func (d *driver) peerDeleteOp(nid, eid string, peerIP net.IP, peerIPMask net.IPMask, - peerMac net.HardwareAddr, vtep net.IP, updateDb bool) error { + peerMac net.HardwareAddr, vtep net.IP) error { if err := validateID(nid, eid); err != nil { return err } - var pEntry peerEntry - if updateDb { - pEntry = d.peerDbDelete(nid, eid, peerIP, peerIPMask, peerMac, vtep) - } + pEntry := d.peerDbDelete(nid, eid, peerIP, peerIPMask, peerMac, vtep) n := d.network(nid) if n == nil { From 4a1d62f9498fb2ef8ed657de9aabc286db8ddcb8 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Tue, 9 May 2017 16:07:09 -0700 Subject: [PATCH 2/3] all: Avoid trivial uses of Sprintf Use the string concatenation operator instead of using Sprintf for simple string concatenation. This is usually easier to read, and allows the compiler to detect problems with the type or number of operands, which would be runtime errors with Sprintf. Signed-off-by: Aaron Lehmann (cherry picked from commit dbd29259e6c2a4b9e17f87a87e5eee5c6eeb315d) --- drivers/bridge/bridge_test.go | 2 +- drivers/ipvlan/ipvlan_setup.go | 2 +- drivers/macvlan/macvlan_setup.go | 2 +- drivers/overlay/ov_network.go | 4 ++-- drivers/overlay/overlay.go | 4 ++-- drivers/solaris/overlay/ov_network.go | 4 ++-- drivers/solaris/overlay/overlay.go | 4 ++-- ipam/allocator.go | 4 ++-- types/types.go | 4 ++-- 9 files changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/bridge/bridge_test.go b/drivers/bridge/bridge_test.go index 78ecfcbd40..3fbddc6e63 100644 --- a/drivers/bridge/bridge_test.go +++ b/drivers/bridge/bridge_test.go @@ -476,7 +476,7 @@ func verifyV4INCEntries(networks map[string]*bridgeNetwork, numEntries int, t *t if x == y { continue } - re := regexp.MustCompile(fmt.Sprintf("%s %s", x.config.BridgeName, y.config.BridgeName)) + re := regexp.MustCompile(x.config.BridgeName + " " + y.config.BridgeName) matches := re.FindAllString(string(out[:]), -1) if len(matches) != 1 { t.Fatalf("Cannot find expected inter-network isolation rules in IP Tables:\n%s", string(out[:])) diff --git a/drivers/ipvlan/ipvlan_setup.go b/drivers/ipvlan/ipvlan_setup.go index 75f08210fb..51afa0f35f 100644 --- a/drivers/ipvlan/ipvlan_setup.go +++ b/drivers/ipvlan/ipvlan_setup.go @@ -201,5 +201,5 @@ func delDummyLink(linkName string) error { // getDummyName returns the name of a dummy parent with truncated net ID and driver prefix func getDummyName(netID string) string { - return fmt.Sprintf("%s%s", dummyPrefix, netID) + return dummyPrefix + netID } diff --git a/drivers/macvlan/macvlan_setup.go b/drivers/macvlan/macvlan_setup.go index b5b4be3499..1cf5ba8772 100644 --- a/drivers/macvlan/macvlan_setup.go +++ b/drivers/macvlan/macvlan_setup.go @@ -205,5 +205,5 @@ func delDummyLink(linkName string) error { // getDummyName returns the name of a dummy parent with truncated net ID and driver prefix func getDummyName(netID string) string { - return fmt.Sprintf("%s%s", dummyPrefix, netID) + return dummyPrefix + netID } diff --git a/drivers/overlay/ov_network.go b/drivers/overlay/ov_network.go index 3ca4e36080..0e8131ee76 100644 --- a/drivers/overlay/ov_network.go +++ b/drivers/overlay/ov_network.go @@ -494,7 +494,7 @@ func (n *network) restoreSubnetSandbox(s *subnet, brName, vxlanName string) erro brIfaceOption := make([]osl.IfaceOption, 2) brIfaceOption = append(brIfaceOption, sbox.InterfaceOptions().Address(s.gwIP)) brIfaceOption = append(brIfaceOption, sbox.InterfaceOptions().Bridge(true)) - Ifaces[fmt.Sprintf("%s+%s", brName, "br")] = brIfaceOption + Ifaces[brName+"+br"] = brIfaceOption err := sbox.Restore(Ifaces, nil, nil, nil) if err != nil { @@ -504,7 +504,7 @@ func (n *network) restoreSubnetSandbox(s *subnet, brName, vxlanName string) erro Ifaces = make(map[string][]osl.IfaceOption) vxlanIfaceOption := make([]osl.IfaceOption, 1) vxlanIfaceOption = append(vxlanIfaceOption, sbox.InterfaceOptions().Master(brName)) - Ifaces[fmt.Sprintf("%s+%s", vxlanName, "vxlan")] = vxlanIfaceOption + Ifaces[vxlanName+"+vxlan"] = vxlanIfaceOption err = sbox.Restore(Ifaces, nil, nil, nil) if err != nil { return err diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index 6c63888561..398843a1a4 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -162,7 +162,7 @@ func (d *driver) restoreEndpoints() error { Ifaces := make(map[string][]osl.IfaceOption) vethIfaceOption := make([]osl.IfaceOption, 1) vethIfaceOption = append(vethIfaceOption, n.sbox.InterfaceOptions().Master(s.brName)) - Ifaces[fmt.Sprintf("%s+%s", "veth", "veth")] = vethIfaceOption + Ifaces["veth+veth"] = vethIfaceOption err := n.sbox.Restore(Ifaces, nil, nil, nil) if err != nil { @@ -270,7 +270,7 @@ func (d *driver) nodeJoin(advertiseAddress, bindAddress string, self bool) { // If there is no cluster store there is no need to start serf. if d.store != nil { if err := validateSelf(advertiseAddress); err != nil { - logrus.Warnf("%s", err.Error()) + logrus.Warn(err.Error()) } err := d.serfInit() if err != nil { diff --git a/drivers/solaris/overlay/ov_network.go b/drivers/solaris/overlay/ov_network.go index 5e3dd5abe1..f90574eb69 100644 --- a/drivers/solaris/overlay/ov_network.go +++ b/drivers/solaris/overlay/ov_network.go @@ -331,7 +331,7 @@ func (n *network) restoreSubnetSandbox(s *subnet, brName, vxlanName string) erro brIfaceOption := make([]osl.IfaceOption, 2) brIfaceOption = append(brIfaceOption, sbox.InterfaceOptions().Address(s.gwIP)) brIfaceOption = append(brIfaceOption, sbox.InterfaceOptions().Bridge(true)) - Ifaces[fmt.Sprintf("%s+%s", brName, "br")] = brIfaceOption + Ifaces[brName+"+br"] = brIfaceOption err := sbox.Restore(Ifaces, nil, nil, nil) if err != nil { @@ -341,7 +341,7 @@ func (n *network) restoreSubnetSandbox(s *subnet, brName, vxlanName string) erro Ifaces = make(map[string][]osl.IfaceOption) vxlanIfaceOption := make([]osl.IfaceOption, 1) vxlanIfaceOption = append(vxlanIfaceOption, sbox.InterfaceOptions().Master(brName)) - Ifaces[fmt.Sprintf("%s+%s", vxlanName, "vxlan")] = vxlanIfaceOption + Ifaces[vxlanName+"+vxlan"] = vxlanIfaceOption err = sbox.Restore(Ifaces, nil, nil, nil) if err != nil { return err diff --git a/drivers/solaris/overlay/overlay.go b/drivers/solaris/overlay/overlay.go index 0a5a1bfdce..9a560679eb 100644 --- a/drivers/solaris/overlay/overlay.go +++ b/drivers/solaris/overlay/overlay.go @@ -141,7 +141,7 @@ func (d *driver) restoreEndpoints() error { Ifaces := make(map[string][]osl.IfaceOption) vethIfaceOption := make([]osl.IfaceOption, 1) vethIfaceOption = append(vethIfaceOption, n.sbox.InterfaceOptions().Master(s.brName)) - Ifaces[fmt.Sprintf("%s+%s", "veth", "veth")] = vethIfaceOption + Ifaces["veth+veth"] = vethIfaceOption err := n.sbox.Restore(Ifaces, nil, nil, nil) if err != nil { @@ -234,7 +234,7 @@ func (d *driver) nodeJoin(advertiseAddress, bindAddress string, self bool) { // If there is no cluster store there is no need to start serf. if d.store != nil { if err := validateSelf(advertiseAddress); err != nil { - logrus.Warnf("%s", err.Error()) + logrus.Warn(err.Error()) } err := d.serfInit() if err != nil { diff --git a/ipam/allocator.go b/ipam/allocator.go index 4243d57a74..69298ffebe 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -579,7 +579,7 @@ func (a *Allocator) DumpDatabase() string { s = fmt.Sprintf("\n\n%s Config", as) aSpace.Lock() for k, config := range aSpace.subnets { - s = fmt.Sprintf("%s%s", s, fmt.Sprintf("\n%v: %v", k, config)) + s += fmt.Sprintf("\n%v: %v", k, config) if config.Range == nil { a.retrieveBitmask(k, config.Pool) } @@ -589,7 +589,7 @@ func (a *Allocator) DumpDatabase() string { s = fmt.Sprintf("%s\n\nBitmasks", s) for k, bm := range a.addresses { - s = fmt.Sprintf("%s%s", s, fmt.Sprintf("\n%s: %s", k, bm)) + s += fmt.Sprintf("\n%s: %s", k, bm) } return s diff --git a/types/types.go b/types/types.go index da113e24cd..164b18096c 100644 --- a/types/types.go +++ b/types/types.go @@ -129,11 +129,11 @@ func (p *PortBinding) GetCopy() PortBinding { func (p *PortBinding) String() string { ret := fmt.Sprintf("%s/", p.Proto) if p.IP != nil { - ret = fmt.Sprintf("%s%s", ret, p.IP.String()) + ret += p.IP.String() } ret = fmt.Sprintf("%s:%d/", ret, p.Port) if p.HostIP != nil { - ret = fmt.Sprintf("%s%s", ret, p.HostIP.String()) + ret += p.HostIP.String() } ret = fmt.Sprintf("%s:%d", ret, p.HostPort) return ret From 1d211a06d681ba09db6e9baf83919ab7f390f3dd Mon Sep 17 00:00:00 2001 From: Flavio Crisciani Date: Tue, 5 Sep 2017 10:43:20 -0700 Subject: [PATCH 3/3] Backport squashed commits - Handle IP reuse in overlay (cherry picked from commit 49200cbd8405988bdc55cfd62abc06ab16f33750) - flush peerdb entries on network delete (cherry picked from commit 2ec096ace3e55967de909408a512f1fb01505577) - log for miss notification (cherry picked from commit 097b363e6f9f1f65338aa26e8ef16a0c335d3550) - Changed ipMask to string (cherry picked from commit d93b9b086a7dba24e76d36b9f770d95cad9bc789) Signed-off-by: Flavio Crisciani --- drivers/overlay/encryption.go | 1 - drivers/overlay/joinleave.go | 18 ++- drivers/overlay/ov_network.go | 66 +++------- drivers/overlay/ov_serf.go | 8 +- drivers/overlay/overlay.go | 2 +- drivers/overlay/overlay.proto | 2 +- drivers/overlay/peerdb.go | 233 ++++++++++++++++++++++------------ networkdb/networkdb.go | 5 +- osl/neigh_linux.go | 37 ++++-- 9 files changed, 216 insertions(+), 156 deletions(-) diff --git a/drivers/overlay/encryption.go b/drivers/overlay/encryption.go index 1d59f238b0..e95609bcb8 100644 --- a/drivers/overlay/encryption.go +++ b/drivers/overlay/encryption.go @@ -21,7 +21,6 @@ import ( const ( r = 0xD0C4E3 - timeout = 30 pktExpansion = 26 // SPI(4) + SeqN(4) + IV(8) + PadLength(1) + NextHeader(1) + ICV(8) ) diff --git a/drivers/overlay/joinleave.go b/drivers/overlay/joinleave.go index d50162f0d0..7cabd9f40a 100644 --- a/drivers/overlay/joinleave.go +++ b/drivers/overlay/joinleave.go @@ -68,7 +68,7 @@ func (d *driver) Join(nid, eid string, sboxKey string, jinfo driverapi.JoinInfo, ep.ifName = containerIfName - if err := d.writeEndpointToStore(ep); err != nil { + if err = d.writeEndpointToStore(ep); err != nil { return fmt.Errorf("failed to update overlay endpoint %s to local data store: %v", ep.id[0:7], err) } @@ -86,7 +86,7 @@ func (d *driver) Join(nid, eid string, sboxKey string, jinfo driverapi.JoinInfo, return err } - if err := sbox.AddInterface(overlayIfName, "veth", + if err = sbox.AddInterface(overlayIfName, "veth", sbox.InterfaceOptions().Master(s.brName)); err != nil { return fmt.Errorf("could not add veth pair inside the network sandbox: %v", err) } @@ -100,7 +100,7 @@ func (d *driver) Join(nid, eid string, sboxKey string, jinfo driverapi.JoinInfo, return err } - if err := nlh.LinkSetHardwareAddr(veth, ep.mac); err != nil { + if err = nlh.LinkSetHardwareAddr(veth, ep.mac); err != nil { return fmt.Errorf("could not set mac address (%v) to the container interface: %v", ep.mac, err) } @@ -108,7 +108,7 @@ func (d *driver) Join(nid, eid string, sboxKey string, jinfo driverapi.JoinInfo, if sub == s { continue } - if err := jinfo.AddStaticRoute(sub.subnetIP, types.NEXTHOP, s.gwIP.IP); err != nil { + if err = jinfo.AddStaticRoute(sub.subnetIP, types.NEXTHOP, s.gwIP.IP); err != nil { logrus.Errorf("Adding subnet %s static route in network %q failed\n", s.subnetIP, n.id) } } @@ -122,7 +122,7 @@ func (d *driver) Join(nid, eid string, sboxKey string, jinfo driverapi.JoinInfo, d.peerAdd(nid, eid, ep.addr.IP, ep.addr.Mask, ep.mac, net.ParseIP(d.advertiseAddress), false, false, true) - if err := d.checkEncryption(nid, nil, n.vxlanID(s), true, true); err != nil { + if err = d.checkEncryption(nid, nil, n.vxlanID(s), true, true); err != nil { logrus.Warn(err) } @@ -200,7 +200,7 @@ func (d *driver) EventNotify(etype driverapi.EventType, nid, tableName, key stri } if etype == driverapi.Delete { - d.peerDelete(nid, eid, addr.IP, addr.Mask, mac, vtep) + d.peerDelete(nid, eid, addr.IP, addr.Mask, mac, vtep, false) return } @@ -232,11 +232,9 @@ func (d *driver) Leave(nid, eid string) error { } } - n.leaveSandbox() + d.peerDelete(nid, eid, ep.addr.IP, ep.addr.Mask, ep.mac, net.ParseIP(d.advertiseAddress), true) - if err := d.checkEncryption(nid, nil, 0, true, false); err != nil { - logrus.Warn(err) - } + n.leaveSandbox() return nil } diff --git a/drivers/overlay/ov_network.go b/drivers/overlay/ov_network.go index 0e8131ee76..67dfc10c35 100644 --- a/drivers/overlay/ov_network.go +++ b/drivers/overlay/ov_network.go @@ -251,8 +251,9 @@ func (d *driver) DeleteNetwork(nid string) error { if err := d.deleteEndpointFromStore(ep); err != nil { logrus.Warnf("Failed to delete overlay endpoint %s from local store: %v", ep.id[0:7], err) } - } + // flush the peerDB entries + d.peerFlush(nid) d.deleteNetwork(nid) vnis, err := n.releaseVxlanID() @@ -505,11 +506,7 @@ func (n *network) restoreSubnetSandbox(s *subnet, brName, vxlanName string) erro vxlanIfaceOption := make([]osl.IfaceOption, 1) vxlanIfaceOption = append(vxlanIfaceOption, sbox.InterfaceOptions().Master(brName)) Ifaces[vxlanName+"+vxlan"] = vxlanIfaceOption - err = sbox.Restore(Ifaces, nil, nil, nil) - if err != nil { - return err - } - return nil + return sbox.Restore(Ifaces, nil, nil, nil) } func (n *network) setupSubnetSandbox(s *subnet, brName, vxlanName string) error { @@ -760,58 +757,38 @@ func (n *network) watchMiss(nlSock *nl.NetlinkSocket) { continue } - logrus.Debugf("miss notification: dest IP %v, dest MAC %v", ip, mac) - if neigh.State&(netlink.NUD_STALE|netlink.NUD_INCOMPLETE) == 0 { continue } if n.driver.isSerfAlive() { + logrus.Debugf("miss notification: dest IP %v, dest MAC %v", ip, mac) mac, IPmask, vtep, err := n.driver.resolvePeer(n.id, ip) if err != nil { logrus.Errorf("could not resolve peer %q: %v", ip, err) continue } n.driver.peerAdd(n.id, "dummy", ip, IPmask, mac, vtep, l2Miss, l3Miss, false) - } else { - // If the gc_thresh values are lower kernel might knock off the neighor entries. - // When we get a L3 miss check if its a valid peer and reprogram the neighbor - // entry again. Rate limit it to once attempt every 500ms, just in case a faulty - // container sends a flood of packets to invalid peers - if !l3Miss { - continue - } - if time.Since(t) > 500*time.Millisecond { + } else if l3Miss && time.Since(t) > time.Second { + // All the local peers will trigger a miss notification but this one is expected and the local container will reply + // autonomously to the ARP request + // In case the gc_thresh3 values is low kernel might reject new entries during peerAdd. This will trigger the following + // extra logs that will inform of the possible issue. + // Entries created would not be deleted see documentation http://man7.org/linux/man-pages/man7/arp.7.html: + // Entries which are marked as permanent are never deleted by the garbage-collector. + // The time limit here is to guarantee that the dbSearch is not + // done too frequently causing a stall of the peerDB operations. + pKey, pEntry, err := n.driver.peerDbSearch(n.id, ip) + if err == nil && !pEntry.isLocal { t = time.Now() - n.programNeighbor(ip) + logrus.Warnf("miss notification for peer:%+v l3Miss:%t l2Miss:%t, if the problem persist check the gc_thresh on the host pKey:%+v pEntry:%+v err:%v", + neigh, l3Miss, l2Miss, *pKey, *pEntry, err) } } } } } -func (n *network) programNeighbor(ip net.IP) { - peerMac, _, _, err := n.driver.peerDbSearch(n.id, ip) - if err != nil { - logrus.Errorf("Reprogramming on L3 miss failed for %s, no peer entry", ip) - return - } - s := n.getSubnetforIPAddr(ip) - if s == nil { - logrus.Errorf("Reprogramming on L3 miss failed for %s, not a valid subnet", ip) - return - } - sbox := n.sandbox() - if sbox == nil { - logrus.Errorf("Reprogramming on L3 miss failed for %s, overlay sandbox missing", ip) - return - } - if err := sbox.AddNeighbor(ip, peerMac, true, sbox.NeighborOptions().LinkName(s.vxlanName)); err != nil { - logrus.Errorf("Reprogramming on L3 miss failed for %s: %v", ip, err) - return - } -} - func (d *driver) addNetwork(n *network) { d.Lock() d.networks[n.id] = n @@ -1095,15 +1072,6 @@ func (n *network) contains(ip net.IP) bool { return false } -func (n *network) getSubnetforIPAddr(ip net.IP) *subnet { - for _, s := range n.subnets { - if s.subnetIP.Contains(ip) { - return s - } - } - return nil -} - // getSubnetforIP returns the subnet to which the given IP belongs func (n *network) getSubnetforIP(ip *net.IPNet) *subnet { for _, s := range n.subnets { diff --git a/drivers/overlay/ov_serf.go b/drivers/overlay/ov_serf.go index 0bc1d12421..be5e398a6e 100644 --- a/drivers/overlay/ov_serf.go +++ b/drivers/overlay/ov_serf.go @@ -122,7 +122,7 @@ func (d *driver) processEvent(u serf.UserEvent) { case "join": d.peerAdd(nid, eid, net.ParseIP(ipStr), net.IPMask(net.ParseIP(maskStr).To4()), mac, net.ParseIP(vtepStr), false, false, false) case "leave": - d.peerDelete(nid, eid, net.ParseIP(ipStr), net.IPMask(net.ParseIP(maskStr).To4()), mac, net.ParseIP(vtepStr)) + d.peerDelete(nid, eid, net.ParseIP(ipStr), net.IPMask(net.ParseIP(maskStr).To4()), mac, net.ParseIP(vtepStr), false) } } @@ -135,13 +135,13 @@ func (d *driver) processQuery(q *serf.Query) { fmt.Printf("Failed to scan query payload string: %v\n", err) } - peerMac, peerIPMask, vtep, err := d.peerDbSearch(nid, net.ParseIP(ipStr)) + pKey, pEntry, err := d.peerDbSearch(nid, net.ParseIP(ipStr)) if err != nil { return } - logrus.Debugf("Sending peer query resp mac %s, mask %s, vtep %s", peerMac, net.IP(peerIPMask), vtep) - q.Respond([]byte(fmt.Sprintf("%s %s %s", peerMac.String(), net.IP(peerIPMask).String(), vtep.String()))) + logrus.Debugf("Sending peer query resp mac %v, mask %s, vtep %s", pKey.peerMac, net.IP(pEntry.peerIPMask).String(), pEntry.vtep) + q.Respond([]byte(fmt.Sprintf("%s %s %s", pKey.peerMac.String(), net.IP(pEntry.peerIPMask).String(), pEntry.vtep.String()))) } func (d *driver) resolvePeer(nid string, peerIP net.IP) (net.HardwareAddr, net.IPMask, net.IP, error) { diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index 398843a1a4..02beb30292 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -262,7 +262,7 @@ func (d *driver) nodeJoin(advertiseAddress, bindAddress string, self bool) { d.Unlock() // If containers are already running on this network update the - // advertiseaddress in the peerDB + // advertise address in the peerDB d.localJoinOnce.Do(func() { d.peerDBUpdateSelf() }) diff --git a/drivers/overlay/overlay.proto b/drivers/overlay/overlay.proto index 45b8c9de7e..3133386e03 100644 --- a/drivers/overlay/overlay.proto +++ b/drivers/overlay/overlay.proto @@ -24,4 +24,4 @@ message PeerRecord { // which this container is running and can be reached by // building a tunnel to that host IP. string tunnel_endpoint_ip = 3 [(gogoproto.customname) = "TunnelEndpointIP"]; -} \ No newline at end of file +} diff --git a/drivers/overlay/peerdb.go b/drivers/overlay/peerdb.go index 7dda761693..9b5af87cb7 100644 --- a/drivers/overlay/peerdb.go +++ b/drivers/overlay/peerdb.go @@ -9,6 +9,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/libnetwork/common" + "github.com/docker/libnetwork/osl" ) const ovPeerTable = "overlay_peer_table" @@ -22,16 +23,45 @@ type peerEntry struct { eid string vtep net.IP peerIPMask net.IPMask - inSandbox bool isLocal bool } +func (p *peerEntry) MarshalDB() peerEntryDB { + return peerEntryDB{ + eid: p.eid, + vtep: p.vtep.String(), + peerIPMask: p.peerIPMask.String(), + isLocal: p.isLocal, + } +} + +// This the structure saved into the set (SetMatrix), due to the implementation of it +// the value inserted in the set has to be Hashable so the []byte had to be converted into +// strings +type peerEntryDB struct { + eid string + vtep string + peerIPMask string + isLocal bool +} + +func (p *peerEntryDB) UnMarshalDB() peerEntry { + return peerEntry{ + eid: p.eid, + vtep: net.ParseIP(p.vtep), + peerIPMask: net.IPMask(net.ParseIP(p.peerIPMask)), + isLocal: p.isLocal, + } +} + type peerMap struct { - mp map[string]peerEntry + // set of peerEntry, note they have to be objects and not pointers to maintain the proper equality checks + mp common.SetMatrix sync.Mutex } type peerNetworkMap struct { + // map with key peerKey mp map[string]*peerMap sync.Mutex } @@ -54,11 +84,7 @@ func (pKey *peerKey) Scan(state fmt.ScanState, verb rune) error { } pKey.peerMac, err = net.ParseMAC(string(macB)) - if err != nil { - return err - } - - return nil + return err } func (d *driver) peerDbWalk(f func(string, *peerKey, *peerEntry) bool) error { @@ -87,10 +113,13 @@ func (d *driver) peerDbNetworkWalk(nid string, f func(*peerKey, *peerEntry) bool } mp := map[string]peerEntry{} - pMap.Lock() - for pKeyStr, pEntry := range pMap.mp { - mp[pKeyStr] = pEntry + for _, pKeyStr := range pMap.mp.Keys() { + entryDBList, ok := pMap.mp.Get(pKeyStr) + if ok { + peerEntryDB := entryDBList[0].(peerEntryDB) + mp[pKeyStr] = peerEntryDB.UnMarshalDB() + } } pMap.Unlock() @@ -107,45 +136,38 @@ func (d *driver) peerDbNetworkWalk(nid string, f func(*peerKey, *peerEntry) bool return nil } -func (d *driver) peerDbSearch(nid string, peerIP net.IP) (net.HardwareAddr, net.IPMask, net.IP, error) { - var ( - peerMac net.HardwareAddr - vtep net.IP - peerIPMask net.IPMask - found bool - ) - +func (d *driver) peerDbSearch(nid string, peerIP net.IP) (*peerKey, *peerEntry, error) { + var pKeyMatched *peerKey + var pEntryMatched *peerEntry err := d.peerDbNetworkWalk(nid, func(pKey *peerKey, pEntry *peerEntry) bool { if pKey.peerIP.Equal(peerIP) { - peerMac = pKey.peerMac - peerIPMask = pEntry.peerIPMask - vtep = pEntry.vtep - found = true - return found + pKeyMatched = pKey + pEntryMatched = pEntry + return true } - return found + return false }) if err != nil { - return nil, nil, nil, fmt.Errorf("peerdb search for peer ip %q failed: %v", peerIP, err) + return nil, nil, fmt.Errorf("peerdb search for peer ip %q failed: %v", peerIP, err) } - if !found { - return nil, nil, nil, fmt.Errorf("peer ip %q not found in peerdb", peerIP) + if pKeyMatched == nil || pEntryMatched == nil { + return nil, nil, fmt.Errorf("peer ip %q not found in peerdb", peerIP) } - return peerMac, peerIPMask, vtep, nil + return pKeyMatched, pEntryMatched, nil } func (d *driver) peerDbAdd(nid, eid string, peerIP net.IP, peerIPMask net.IPMask, - peerMac net.HardwareAddr, vtep net.IP, isLocal bool) { + peerMac net.HardwareAddr, vtep net.IP, isLocal bool) (bool, int) { d.peerDb.Lock() pMap, ok := d.peerDb.mp[nid] if !ok { d.peerDb.mp[nid] = &peerMap{ - mp: make(map[string]peerEntry), + mp: common.NewSetMatrix(), } pMap = d.peerDb.mp[nid] @@ -165,18 +187,24 @@ func (d *driver) peerDbAdd(nid, eid string, peerIP net.IP, peerIPMask net.IPMask } pMap.Lock() - pMap.mp[pKey.String()] = pEntry - pMap.Unlock() + defer pMap.Unlock() + b, i := pMap.mp.Insert(pKey.String(), pEntry.MarshalDB()) + if i != 1 { + // Transient case, there is more than one endpoint that is using the same IP,MAC pair + s, _ := pMap.mp.String(pKey.String()) + logrus.Warnf("peerDbAdd transient condition - Key:%s cardinality:%d db state:%s", pKey.String(), i, s) + } + return b, i } func (d *driver) peerDbDelete(nid, eid string, peerIP net.IP, peerIPMask net.IPMask, - peerMac net.HardwareAddr, vtep net.IP) peerEntry { + peerMac net.HardwareAddr, vtep net.IP, isLocal bool) (bool, int) { d.peerDb.Lock() pMap, ok := d.peerDb.mp[nid] if !ok { d.peerDb.Unlock() - return peerEntry{} + return false, 0 } d.peerDb.Unlock() @@ -185,22 +213,22 @@ func (d *driver) peerDbDelete(nid, eid string, peerIP net.IP, peerIPMask net.IPM peerMac: peerMac, } - pMap.Lock() - - pEntry, ok := pMap.mp[pKey.String()] - if ok { - // Mismatched endpoint ID(possibly outdated). Do not - // delete peerdb - if pEntry.eid != eid { - pMap.Unlock() - return pEntry - } + pEntry := peerEntry{ + eid: eid, + vtep: vtep, + peerIPMask: peerIPMask, + isLocal: isLocal, } - delete(pMap.mp, pKey.String()) - pMap.Unlock() - - return pEntry + pMap.Lock() + defer pMap.Unlock() + b, i := pMap.mp.Remove(pKey.String(), pEntry.MarshalDB()) + if i != 0 { + // Transient case, there is more than one endpoint that is using the same IP,MAC pair + s, _ := pMap.mp.String(pKey.String()) + logrus.Warnf("peerDbDelete transient condition - Key:%s cardinality:%d db state:%s", pKey.String(), i, s) + } + return b, i } // The overlay uses a lazy initialization approach, this means that when a network is created @@ -224,6 +252,7 @@ const ( peerOperationINIT peerOperationType = iota peerOperationADD peerOperationDELETE + peerOperationFLUSH ) type peerOperation struct { @@ -253,7 +282,9 @@ func (d *driver) peerOpRoutine(ctx context.Context, ch chan *peerOperation) { case peerOperationADD: err = d.peerAddOp(op.networkID, op.endpointID, op.peerIP, op.peerIPMask, op.peerMac, op.vtepIP, op.l2Miss, op.l3Miss, true, op.localPeer) case peerOperationDELETE: - err = d.peerDeleteOp(op.networkID, op.endpointID, op.peerIP, op.peerIPMask, op.peerMac, op.vtepIP) + err = d.peerDeleteOp(op.networkID, op.endpointID, op.peerIP, op.peerIPMask, op.peerMac, op.vtepIP, op.localPeer) + case peerOperationFLUSH: + err = d.peerFlushOp(op.networkID) } if err != nil { logrus.Warnf("Peer operation failed:%s op:%v", err, op) @@ -286,7 +317,6 @@ func (d *driver) peerInitOp(nid string) error { func (d *driver) peerAdd(nid, eid string, peerIP net.IP, peerIPMask net.IPMask, peerMac net.HardwareAddr, vtep net.IP, l2Miss, l3Miss, localPeer bool) { - callerName := common.CallerName(1) d.peerOpCh <- &peerOperation{ opType: peerOperationADD, networkID: nid, @@ -298,24 +328,32 @@ func (d *driver) peerAdd(nid, eid string, peerIP net.IP, peerIPMask net.IPMask, l2Miss: l2Miss, l3Miss: l3Miss, localPeer: localPeer, - callerName: callerName, + callerName: common.CallerName(1), } } func (d *driver) peerAddOp(nid, eid string, peerIP net.IP, peerIPMask net.IPMask, - peerMac net.HardwareAddr, vtep net.IP, l2Miss, l3Miss, updateDB, updateOnlyDB bool) error { + peerMac net.HardwareAddr, vtep net.IP, l2Miss, l3Miss, updateDB, localPeer bool) error { if err := validateID(nid, eid); err != nil { return err } + var dbEntries int + var inserted bool if updateDB { - d.peerDbAdd(nid, eid, peerIP, peerIPMask, peerMac, vtep, false) - if updateOnlyDB { - return nil + inserted, dbEntries = d.peerDbAdd(nid, eid, peerIP, peerIPMask, peerMac, vtep, localPeer) + if !inserted { + logrus.Warnf("Entry already present in db: nid:%s eid:%s peerIP:%v peerMac:%v isLocal:%t vtep:%v", + nid, eid, peerIP, peerMac, localPeer, vtep) } } + // Local peers do not need any further configuration + if localPeer { + return nil + } + n := d.network(nid) if n == nil { return nil @@ -353,21 +391,26 @@ func (d *driver) peerAddOp(nid, eid string, peerIP net.IP, peerIPMask net.IPMask // Add neighbor entry for the peer IP if err := sbox.AddNeighbor(peerIP, peerMac, l3Miss, sbox.NeighborOptions().LinkName(s.vxlanName)); err != nil { - return fmt.Errorf("could not add neighbor entry into the sandbox: %v", err) + if _, ok := err.(osl.NeighborSearchError); ok && dbEntries > 1 { + // We are in the transient case so only the first configuration is programmed into the kernel + // Upon deletion if the active configuration is deleted the next one from the database will be restored + // Note we are skipping also the next configuration + return nil + } + return fmt.Errorf("could not add neighbor entry for nid:%s eid:%s into the sandbox:%v", nid, eid, err) } // Add fdb entry to the bridge for the peer mac if err := sbox.AddNeighbor(vtep, peerMac, l2Miss, sbox.NeighborOptions().LinkName(s.vxlanName), sbox.NeighborOptions().Family(syscall.AF_BRIDGE)); err != nil { - return fmt.Errorf("could not add fdb entry into the sandbox: %v", err) + return fmt.Errorf("could not add fdb entry for nid:%s eid:%s into the sandbox:%v", nid, eid, err) } return nil } func (d *driver) peerDelete(nid, eid string, peerIP net.IP, peerIPMask net.IPMask, - peerMac net.HardwareAddr, vtep net.IP) { - callerName := common.CallerName(1) + peerMac net.HardwareAddr, vtep net.IP, localPeer bool) { d.peerOpCh <- &peerOperation{ opType: peerOperationDELETE, networkID: nid, @@ -376,18 +419,23 @@ func (d *driver) peerDelete(nid, eid string, peerIP net.IP, peerIPMask net.IPMas peerIPMask: peerIPMask, peerMac: peerMac, vtepIP: vtep, - callerName: callerName, + callerName: common.CallerName(1), + localPeer: localPeer, } } func (d *driver) peerDeleteOp(nid, eid string, peerIP net.IP, peerIPMask net.IPMask, - peerMac net.HardwareAddr, vtep net.IP) error { + peerMac net.HardwareAddr, vtep net.IP, localPeer bool) error { if err := validateID(nid, eid); err != nil { return err } - pEntry := d.peerDbDelete(nid, eid, peerIP, peerIPMask, peerMac, vtep) + deleted, dbEntries := d.peerDbDelete(nid, eid, peerIP, peerIPMask, peerMac, vtep, localPeer) + if !deleted { + logrus.Warnf("Entry was not in db: nid:%s eid:%s peerIP:%v peerMac:%v isLocal:%t vtep:%v", + nid, eid, peerIP, peerMac, localPeer, vtep) + } n := d.network(nid) if n == nil { @@ -399,30 +447,59 @@ func (d *driver) peerDeleteOp(nid, eid string, peerIP net.IP, peerIPMask net.IPM return nil } - // Delete fdb entry to the bridge for the peer mac only if the - // entry existed in local peerdb. If it is a stale delete - // request, still call DeleteNeighbor but only to cleanup any - // leftover sandbox neighbor cache and not actually delete the - // kernel state. - if (eid == pEntry.eid && vtep.Equal(pEntry.vtep)) || - (eid != pEntry.eid && !vtep.Equal(pEntry.vtep)) { - if err := sbox.DeleteNeighbor(vtep, peerMac, - eid == pEntry.eid && vtep.Equal(pEntry.vtep)); err != nil { - return fmt.Errorf("could not delete fdb entry into the sandbox: %v", err) - } + if err := d.checkEncryption(nid, vtep, 0, localPeer, false); err != nil { + logrus.Warn(err) } - // Delete neighbor entry for the peer IP - if eid == pEntry.eid { + // Local peers do not have any local configuration to delete + if !localPeer { + // Remove fdb entry to the bridge for the peer mac + if err := sbox.DeleteNeighbor(vtep, peerMac, true); err != nil { + if _, ok := err.(osl.NeighborSearchError); ok && dbEntries > 0 { + // We fall in here if there is a transient state and if the neighbor that is being deleted + // was never been configured into the kernel (we allow only 1 configuration at the time per mapping) + return nil + } + return fmt.Errorf("could not delete fdb entry for nid:%s eid:%s into the sandbox:%v", nid, eid, err) + } + + // Delete neighbor entry for the peer IP if err := sbox.DeleteNeighbor(peerIP, peerMac, true); err != nil { - return fmt.Errorf("could not delete neighbor entry into the sandbox: %v", err) + return fmt.Errorf("could not delete neighbor entry for nid:%s eid:%s into the sandbox:%v", nid, eid, err) } } - if err := d.checkEncryption(nid, vtep, 0, false, false); err != nil { - logrus.Warn(err) + if dbEntries == 0 { + return nil + } + + // If there is still an entry into the database and the deletion went through without errors means that there is now no + // configuration active in the kernel. + // Restore one configuration for the directly from the database, note that is guaranteed that there is one + peerKey, peerEntry, err := d.peerDbSearch(nid, peerIP) + if err != nil { + logrus.Errorf("peerDeleteOp unable to restore a configuration for nid:%s ip:%v mac:%v err:%s", nid, peerIP, peerMac, err) + return err } + return d.peerAddOp(nid, peerEntry.eid, peerIP, peerEntry.peerIPMask, peerKey.peerMac, peerEntry.vtep, false, false, false, peerEntry.isLocal) +} +func (d *driver) peerFlush(nid string) { + d.peerOpCh <- &peerOperation{ + opType: peerOperationFLUSH, + networkID: nid, + callerName: common.CallerName(1), + } +} + +func (d *driver) peerFlushOp(nid string) error { + d.peerDb.Lock() + defer d.peerDb.Unlock() + _, ok := d.peerDb.mp[nid] + if !ok { + return fmt.Errorf("Unable to find the peerDB for nid:%s", nid) + } + delete(d.peerDb.mp, nid) return nil } diff --git a/networkdb/networkdb.go b/networkdb/networkdb.go index e0d1c1f980..007a03b89a 100644 --- a/networkdb/networkdb.go +++ b/networkdb/networkdb.go @@ -496,7 +496,10 @@ func (nDB *NetworkDB) deleteNodeNetworkEntries(nid, node string) { nDB.deleteEntry(nid, tname, key) } - nDB.broadcaster.Write(makeEvent(opDelete, tname, nid, key, entry.value)) + // Notify to the upper layer only entries not already marked for deletion + if !oldEntry.deleting { + nDB.broadcaster.Write(makeEvent(opDelete, tname, nid, key, entry.value)) + } return false }) } diff --git a/osl/neigh_linux.go b/osl/neigh_linux.go index 161ffa7beb..134d641893 100644 --- a/osl/neigh_linux.go +++ b/osl/neigh_linux.go @@ -9,6 +9,17 @@ import ( "github.com/vishvananda/netlink" ) +// NeighborSearchError indicates that the neighbor is already present +type NeighborSearchError struct { + ip net.IP + mac net.HardwareAddr + present bool +} + +func (n NeighborSearchError) Error() string { + return fmt.Sprintf("Search neighbor failed for IP %v, mac %v, present in db:%t", n.ip, n.mac, n.present) +} + // NeighOption is a function option type to set interface options type NeighOption func(nh *neigh) @@ -41,7 +52,7 @@ func (n *networkNamespace) DeleteNeighbor(dstIP net.IP, dstMac net.HardwareAddr, nh := n.findNeighbor(dstIP, dstMac) if nh == nil { - return fmt.Errorf("could not find the neighbor entry to delete") + return NeighborSearchError{dstIP, dstMac, false} } if osDelete { @@ -105,26 +116,27 @@ func (n *networkNamespace) DeleteNeighbor(dstIP net.IP, dstMac net.HardwareAddr, } } n.Unlock() - logrus.Debugf("Neighbor entry deleted for IP %v, mac %v", dstIP, dstMac) + logrus.Debugf("Neighbor entry deleted for IP %v, mac %v osDelete:%t", dstIP, dstMac, osDelete) return nil } func (n *networkNamespace) AddNeighbor(dstIP net.IP, dstMac net.HardwareAddr, force bool, options ...NeighOption) error { var ( - iface netlink.Link - err error + iface netlink.Link + err error + neighborAlreadyPresent bool ) // If the namespace already has the neighbor entry but the AddNeighbor is called // because of a miss notification (force flag) program the kernel anyway. nh := n.findNeighbor(dstIP, dstMac) if nh != nil { + neighborAlreadyPresent = true + logrus.Warnf("Neighbor entry already present for IP %v, mac %v neighbor:%+v forceUpdate:%t", dstIP, dstMac, nh, force) if !force { - logrus.Warnf("Neighbor entry already present for IP %v, mac %v", dstIP, dstMac) - return nil + return NeighborSearchError{dstIP, dstMac, true} } - logrus.Warnf("Force kernel update, Neighbor entry already present for IP %v, mac %v", dstIP, dstMac) } nh = &neigh{ @@ -148,8 +160,7 @@ func (n *networkNamespace) AddNeighbor(dstIP net.IP, dstMac net.HardwareAddr, fo if nh.linkDst != "" { iface, err = nlh.LinkByName(nh.linkDst) if err != nil { - return fmt.Errorf("could not find interface with destination name %s: %v", - nh.linkDst, err) + return fmt.Errorf("could not find interface with destination name %s: %v", nh.linkDst, err) } } @@ -169,13 +180,17 @@ func (n *networkNamespace) AddNeighbor(dstIP net.IP, dstMac net.HardwareAddr, fo } if err := nlh.NeighSet(nlnh); err != nil { - return fmt.Errorf("could not add neighbor entry: %v", err) + return fmt.Errorf("could not add neighbor entry:%+v error:%v", nlnh, err) + } + + if neighborAlreadyPresent { + return nil } n.Lock() n.neighbors = append(n.neighbors, nh) n.Unlock() - logrus.Debugf("Neighbor entry added for IP %v, mac %v", dstIP, dstMac) + logrus.Debugf("Neighbor entry added for IP:%v, mac:%v on ifc:%s", dstIP, dstMac, nh.linkName) return nil }