From 9fc22d29092933460fe00bdaccea179f29e9960d Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Fri, 2 Nov 2018 11:23:54 +0100 Subject: [PATCH] net: update zoneCache on cache misses to cover appearing interfaces performance differences are in measurement noise as per benchcmp: benchmark old ns/op new ns/op delta BenchmarkUDP6LinkLocalUnicast-12 5012 5009 -0.06% Fixes #28535 Change-Id: Id022e2ed089ce8388a2398e755848ec94e77e653 Reviewed-on: https://go-review.googlesource.com/c/146941 Run-TryBot: Mikio Hara Reviewed-by: Mikio Hara --- src/net/interface.go | 40 ++++++++++++++++++++++++--------- src/net/interface_bsd_test.go | 5 +++++ src/net/interface_linux_test.go | 25 +++++++++++++++++++++ src/net/interface_unix_test.go | 34 ++++++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 10 deletions(-) diff --git a/src/net/interface.go b/src/net/interface.go index 375a4568e3..46b0400f2f 100644 --- a/src/net/interface.go +++ b/src/net/interface.go @@ -102,7 +102,7 @@ func Interfaces() ([]Interface, error) { return nil, &OpError{Op: "route", Net: "ip+net", Source: nil, Addr: nil, Err: err} } if len(ift) != 0 { - zoneCache.update(ift) + zoneCache.update(ift, false) } return ift, nil } @@ -159,7 +159,7 @@ func InterfaceByName(name string) (*Interface, error) { return nil, &OpError{Op: "route", Net: "ip+net", Source: nil, Addr: nil, Err: err} } if len(ift) != 0 { - zoneCache.update(ift) + zoneCache.update(ift, false) } for _, ifi := range ift { if name == ifi.Name { @@ -187,18 +187,21 @@ var zoneCache = ipv6ZoneCache{ toName: make(map[int]string), } -func (zc *ipv6ZoneCache) update(ift []Interface) { +// update refreshes the network interface information if the cache was last +// updated more than 1 minute ago, or if force is set. It returns whether the +// cache was updated. +func (zc *ipv6ZoneCache) update(ift []Interface, force bool) (updated bool) { zc.Lock() defer zc.Unlock() now := time.Now() - if zc.lastFetched.After(now.Add(-60 * time.Second)) { - return + if !force && zc.lastFetched.After(now.Add(-60*time.Second)) { + return false } zc.lastFetched = now if len(ift) == 0 { var err error if ift, err = interfaceTable(0); err != nil { - return + return false } } zc.toIndex = make(map[string]int, len(ift)) @@ -209,16 +212,25 @@ func (zc *ipv6ZoneCache) update(ift []Interface) { zc.toName[ifi.Index] = ifi.Name } } + return true } func (zc *ipv6ZoneCache) name(index int) string { if index == 0 { return "" } - zoneCache.update(nil) + updated := zoneCache.update(nil, false) zoneCache.RLock() - defer zoneCache.RUnlock() name, ok := zoneCache.toName[index] + zoneCache.RUnlock() + if !ok { + if !updated { + zoneCache.update(nil, true) + zoneCache.RLock() + name, ok = zoneCache.toName[index] + zoneCache.RUnlock() + } + } if !ok { name = uitoa(uint(index)) } @@ -229,10 +241,18 @@ func (zc *ipv6ZoneCache) index(name string) int { if name == "" { return 0 } - zoneCache.update(nil) + updated := zoneCache.update(nil, false) zoneCache.RLock() - defer zoneCache.RUnlock() index, ok := zoneCache.toIndex[name] + zoneCache.RUnlock() + if !ok { + if !updated { + zoneCache.update(nil, true) + zoneCache.RLock() + index, ok = zoneCache.toIndex[name] + zoneCache.RUnlock() + } + } if !ok { index, _, _ = dtoi(name) } diff --git a/src/net/interface_bsd_test.go b/src/net/interface_bsd_test.go index 69b0fbcab3..947dde71e6 100644 --- a/src/net/interface_bsd_test.go +++ b/src/net/interface_bsd_test.go @@ -7,6 +7,7 @@ package net import ( + "errors" "fmt" "os/exec" "runtime" @@ -53,3 +54,7 @@ func (ti *testInterface) setPointToPoint(suffix int) error { }) return nil } + +func (ti *testInterface) setLinkLocal(suffix int) error { + return errors.New("not yet implemented for BSD") +} diff --git a/src/net/interface_linux_test.go b/src/net/interface_linux_test.go index 6959ddb3d9..0699fec636 100644 --- a/src/net/interface_linux_test.go +++ b/src/net/interface_linux_test.go @@ -35,6 +35,31 @@ func (ti *testInterface) setBroadcast(suffix int) error { return nil } +func (ti *testInterface) setLinkLocal(suffix int) error { + ti.name = fmt.Sprintf("gotest%d", suffix) + xname, err := exec.LookPath("ip") + if err != nil { + return err + } + ti.setupCmds = append(ti.setupCmds, &exec.Cmd{ + Path: xname, + Args: []string{"ip", "link", "add", ti.name, "type", "dummy"}, + }) + ti.setupCmds = append(ti.setupCmds, &exec.Cmd{ + Path: xname, + Args: []string{"ip", "address", "add", ti.local, "dev", ti.name}, + }) + ti.teardownCmds = append(ti.teardownCmds, &exec.Cmd{ + Path: xname, + Args: []string{"ip", "address", "del", ti.local, "dev", ti.name}, + }) + ti.teardownCmds = append(ti.teardownCmds, &exec.Cmd{ + Path: xname, + Args: []string{"ip", "link", "delete", ti.name, "type", "dummy"}, + }) + return nil +} + func (ti *testInterface) setPointToPoint(suffix int) error { ti.name = fmt.Sprintf("gotest%d", suffix) xname, err := exec.LookPath("ip") diff --git a/src/net/interface_unix_test.go b/src/net/interface_unix_test.go index c3d981dc5c..20e75cd036 100644 --- a/src/net/interface_unix_test.go +++ b/src/net/interface_unix_test.go @@ -176,3 +176,37 @@ func TestInterfaceArrivalAndDeparture(t *testing.T) { } } } + +func TestInterfaceArrivalAndDepartureZoneCache(t *testing.T) { + if testing.Short() { + t.Skip("avoid external network") + } + if os.Getuid() != 0 { + t.Skip("must be root") + } + + // Ensure zoneCache is filled: + _, _ = Listen("tcp", "[fe80::1%nonexistant]:0") + + ti := &testInterface{local: "fe80::1"} + if err := ti.setLinkLocal(0); err != nil { + t.Skipf("test requires external command: %v", err) + } + if err := ti.setup(); err != nil { + t.Fatal(err) + } + defer ti.teardown() + + time.Sleep(3 * time.Millisecond) + + // If Listen fails (on Linux with “bind: invalid argument”), zoneCache was + // not updated when encountering a nonexistant interface: + ln, err := Listen("tcp", "[fe80::1%"+ti.name+"]:0") + if err != nil { + t.Fatal(err) + } + ln.Close() + if err := ti.teardown(); err != nil { + t.Fatal(err) + } +} -- 2.48.1