diff --git a/acceptance/router_newbenchmark/BUILD.bazel b/acceptance/router_newbenchmark/BUILD.bazel index 10a6f42aca..e12d0b233f 100644 --- a/acceptance/router_newbenchmark/BUILD.bazel +++ b/acceptance/router_newbenchmark/BUILD.bazel @@ -6,14 +6,11 @@ load("//:scion.bzl", "scion_go_binary") go_library( name = "go_default_library", - srcs = [ - "cases.go", - "main.go", - "topo.go", - ], + srcs = ["main.go"], importpath = "github.com/scionproto/scion/acceptance/router_newbenchmark", visibility = ["//visibility:private"], deps = [ + "//acceptance/router_newbenchmark/cases:go_default_library", "//pkg/addr:go_default_library", "//pkg/log:go_default_library", "//pkg/private/serrors:go_default_library", @@ -39,14 +36,11 @@ scion_go_binary( exports_files([ "conf", "test.py", - "pause.tar", ]) args = [ "--executable", "brload:$(location //acceptance/router_newbenchmark:brload)", - "--container-loader", - "posix-router:latest#$(location //acceptance/router_newbenchmark:router)", ] data = [ @@ -69,9 +63,3 @@ container_image( name = "router", base = "//docker:posix_router", ) - -go_binary( - name = "router_newbenchmark", - embed = [":go_default_library"], - visibility = ["//visibility:public"], -) diff --git a/acceptance/router_newbenchmark/cases/BUILD.bazel b/acceptance/router_newbenchmark/cases/BUILD.bazel new file mode 100644 index 0000000000..3adaf30d07 --- /dev/null +++ b/acceptance/router_newbenchmark/cases/BUILD.bazel @@ -0,0 +1,22 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_binary") +load("//tools/lint:go.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = [ + "br_transit.go", + "topo.go", + ], + visibility = ["//acceptance/router_newbenchmark:__pkg__"], + importpath = "github.com/scionproto/scion/acceptance/router_newbenchmark/cases", + deps = [ + "//pkg/addr:go_default_library", + "//pkg/private/util:go_default_library", + "//pkg/slayers:go_default_library", + "//pkg/slayers/path:go_default_library", + "//pkg/slayers/path/scion:go_default_library", + "//pkg/private/xtest:go_default_library", + "@com_github_google_gopacket//:go_default_library", + "@com_github_google_gopacket//layers:go_default_library", + ], +) diff --git a/acceptance/router_newbenchmark/cases.go b/acceptance/router_newbenchmark/cases/br_transit.go similarity index 99% rename from acceptance/router_newbenchmark/cases.go rename to acceptance/router_newbenchmark/cases/br_transit.go index 1b147e1172..7bfb665bcc 100644 --- a/acceptance/router_newbenchmark/cases.go +++ b/acceptance/router_newbenchmark/cases/br_transit.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package main +package cases import ( "hash" diff --git a/acceptance/router_newbenchmark/topo.go b/acceptance/router_newbenchmark/cases/topo.go similarity index 97% rename from acceptance/router_newbenchmark/topo.go rename to acceptance/router_newbenchmark/cases/topo.go index 6bf0652212..a994de4dd5 100644 --- a/acceptance/router_newbenchmark/topo.go +++ b/acceptance/router_newbenchmark/cases/topo.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package main +package cases import ( "fmt" @@ -107,10 +107,10 @@ var ( macAddrs map[netip.Addr]net.HardwareAddr = map[netip.Addr]net.HardwareAddr{} ) -// initInterfaces collects the names and mac addresses for the interfaces setup by the invoker +// InitInterfaces collects the names and mac addresses for the interfaces setup by the invoker // according to instructions given via listInterfaces(). // This information is indexed by our own interface labels. -func initInterfaces(pairs []string) { +func InitInterfaces(pairs []string) { for _, pair := range pairs { p := strings.Split(pair, "=") label := p[0] @@ -160,7 +160,7 @@ func hostAddr(ip netip.Addr) addr.Host { // to set up the network accordingly before executing the test without that option. // We do not choose interface names or mac addresses those will be provided by the invoker // via the --interfaces options. -func listInterfaces() string { +func ListInterfaces() string { var sb strings.Builder for l, i := range intfMap { sb.WriteString(l) @@ -172,6 +172,7 @@ func listInterfaces() string { sb.WriteString(i.peerIP.String()) sb.WriteString("\n") } + // "our_label,24,,\n" return sb.String() } diff --git a/acceptance/router_newbenchmark/main.go b/acceptance/router_newbenchmark/main.go index 107c73a183..ddf96d446f 100644 --- a/acceptance/router_newbenchmark/main.go +++ b/acceptance/router_newbenchmark/main.go @@ -29,6 +29,7 @@ import ( "github.com/google/gopacket/afpacket" "github.com/google/gopacket/layers" + "github.com/scionproto/scion/acceptance/router_newbenchmark/cases" "github.com/scionproto/scion/pkg/log" "github.com/scionproto/scion/pkg/private/serrors" "github.com/scionproto/scion/pkg/scrypto" @@ -52,7 +53,7 @@ type Case func(payload string, mac hash.Hash, numDistinct int) (string, string, var ( allCases = map[string]Case{ - "br_transit": BrTransit, + "br_transit": cases.BrTransit, } logConsole = flag.String("log.console", "debug", "Console logging level: debug|info|error") dir = flag.String("artifacts", "", "Artifacts directory") @@ -63,29 +64,31 @@ var ( reflect.ValueOf(allCases).MapKeys())) showIntf = flag.Bool("show_interfaces", false, "Show interfaces needed by the test") interfaces = arrayFlags{} - handles = make(map[string]*afpacket.TPacket) ) // initDevices inventories the available network interfaces, picks the ones that a case may inject -// traffic into, and associates them with a AF Packet interface. -func initDevices() error { +// traffic into, and associates them with a AF Packet interface. It returns the packet interfaces +// corresponding to each network interface. +func openDevices() (map[string]*afpacket.TPacket, error) { devs, err := net.Interfaces() if err != nil { - return serrors.WrapStr("listing network interfaces", err) + return nil, serrors.WrapStr("listing network interfaces", err) } + handles := make(map[string]*afpacket.TPacket) + for _, dev := range devs { if !strings.HasPrefix(dev.Name, "veth_") || !strings.HasSuffix(dev.Name, "_host") { continue } handle, err := afpacket.NewTPacket(afpacket.OptInterface(dev.Name)) if err != nil { - return serrors.WrapStr("creating TPacket", err) + return nil, serrors.WrapStr("creating TPacket", err) } handles[dev.Name] = handle } - return nil + return handles, nil } func main() { @@ -100,7 +103,7 @@ func realMain() int { peer_mac: the mac address of `) flag.Parse() if *showIntf { - fmt.Println(listInterfaces()) + fmt.Println(cases.ListInterfaces()) return 0 } @@ -119,25 +122,24 @@ func realMain() int { return 1 } - artifactsDir, err := os.MkdirTemp("", "brload_") - if err != nil { - log.Error("Cannot create tmp dir", "err", err) - return 1 - } - if *dir != "" { - artifactsDir = *dir - } + artifactsDir := *dir if v := os.Getenv("TEST_ARTIFACTS_DIR"); v != "" { artifactsDir = v } + + if artifactsDir == "" { + log.Error("Artifacts directory not configured") + return 1 + } + hfMAC, err := loadKey(artifactsDir) if err != nil { log.Error("Loading keys failed", "err", err) return 1 } - initInterfaces(interfaces) - err = initDevices() + cases.InitInterfaces(interfaces) + handles, err := openDevices() if err != nil { log.Error("Loading devices failed", "err", err) return 1 @@ -170,34 +172,7 @@ func realMain() int { go func() { defer log.HandlePanic() - numRcv := 0 - - defer func() { - listenerChan <- numRcv - close(listenerChan) - }() - - for { - got, ok := <-packetChan - if !ok { - // No more packets - log.Info("No more Packets") - return - } - if err := got.ErrorLayer(); err != nil { - log.Error("error decoding packet", "err", err) - continue - } - layer := got.Layer(gopacket.LayerTypePayload) - if layer == nil { - log.Error("error fetching packet payload: no PayLoad") - continue - } - if string(layer.LayerContents()) == payloadString { - numRcv++ - return - } - } + receivePackets(packetChan, payloadString, listenerChan) }() // We started everything that could be started. So the best window for perf mertics @@ -213,23 +188,65 @@ func realMain() int { // The test harness looks for this output. fmt.Printf("metricsBegin: %d metricsEnd: %d\n", metricsBegin, metricsEnd) - // In short tests (<1M packets), we finish sending before the first packets arrive. - time.Sleep(time.Second * time.Duration(1)) - - // If our listener is still stuck there, unstick it. Closing the device doesn't cause the - // packet channel to close (presumably a bug). Close the channel ourselves. - close(packetChan) - - outcome := <-listenerChan - if outcome == 0 { - log.Error("Listener never saw a valid packet being forwarded") - return 1 + // Get the results from the packet listener. + // Give it one second as in very short tests (<1M pkts) we get here before the first packet. + outcome := 0 + timeout := time.After(1 * time.Second) + for outcome == 0 { + select { + case outcome = <-listenerChan: + if outcome == 0 { + log.Error("Listener never saw a valid packet being forwarded") + return 1 + } + case <-timeout: + // If our listener is still stuck there, unstick it. Closing the device doesn't cause the + // packet channel to close (presumably a bug). Close the channel ourselves. + // After this, the next loop is guaranteed an outcome. + close(packetChan) + } } fmt.Printf("Listener results: %d\n", outcome) return 0 } +// receivePkts consume some or all (at least one if it arrives) of the packets +// arriving on the given handle and checks that they contain the given payload. +// The number of consumed packets is returned via the given outcome channel. +// Currently we are content with receiving a single correct packet and we terminate after +// that. +func receivePackets(packetChan chan gopacket.Packet, payload string, outcome chan int) { + numRcv := 0 + + defer func() { + outcome <- numRcv + close(outcome) + }() + + for { + got, ok := <-packetChan + if !ok { + // No more packets + log.Info("No more Packets") + return + } + if err := got.ErrorLayer(); err != nil { + log.Error("error decoding packet", "err", err) + continue + } + layer := got.Layer(gopacket.LayerTypePayload) + if layer == nil { + log.Error("error fetching packet payload: no PayLoad") + continue + } + if string(layer.LayerContents()) == payload { + numRcv++ + return + } + } +} + // loadKey loads the keys that the router under test uses to sign hop fields. func loadKey(artifactsDir string) (hash.Hash, error) { keysDir := filepath.Join(artifactsDir, "conf", "keys") diff --git a/acceptance/router_newbenchmark/test.py b/acceptance/router_newbenchmark/test.py index 2beeea125e..47d4ccb7d1 100644 --- a/acceptance/router_newbenchmark/test.py +++ b/acceptance/router_newbenchmark/test.py @@ -103,7 +103,7 @@ class RouterBMTest(base.TestBase): # Accepts an IntfReq records names and mac addresses into an Intf and associates it with # the request label. - def create_interface(self, req: IntfReq, ns: str) -> Intf: + def create_interface(self, req: IntfReq, ns: str): hostIntf = f"veth_{req.label}_host" brIntf = f"veth_{req.label}" peerMac = mac_for_ip(req.peerIp) @@ -161,8 +161,7 @@ def setup_prepare(self): # We supply the label->(host-side-name,mac,peermac) mapping to brload when we start it. self.intfMap = {} brload = self.get_executable("brload") - output = exec_sudo(f"{brload.executable} -artifacts {self.artifacts} " - "-show_interfaces") + output = brload("-artifacts", f"{self.artifacts}", "-show_interfaces") for line in output.splitlines(): elems = line.split(",")