comments, leftovers, small fixes

more minor

acceptance/sig_short_exp_time/test

@@ -10,6 +10,7 @@
 # short lifetime expires, the first path is taken down and a ping sent out. If
 # the failover was correct, this ping should not be lost.
 
+# TODO(JordiSubira): Remove dispatcher from schema.
 # Docker topology:
 #
 # +---------------------------+     +---------------------------+

gateway/gateway.go

@@ -106,26 +106,6 @@ func (pcf PacketConnFactory) New() (net.PacketConn, error) {
 	return conn, nil
 }
 
-type ProbeConnFactory struct {
-	LocalIA addr.IA
-	LocalIP net.IP
-}
-
-func (f ProbeConnFactory) New(ctx context.Context) (net.PacketConn, error) {
-	pathMonitorConnection, err := (&snet.SCIONNetwork{
-		LocalIA: f.LocalIA,
-		Connector: &snet.DefaultConnector{
-			SCMPHandler: ignoreSCMP{},
-		},
-	}).Listen(ctx, "udp", &net.UDPAddr{IP: f.LocalIP}, addr.SvcNone)
-	if err != nil {
-		return nil, serrors.WrapStr("unable to open control socket", err)
-	}
-	log.FromCtx(ctx).Debug("Path monitor connection opened on Raw UDP/SCION",
-		"local_addr", pathMonitorConnection.LocalAddr())
-	return pathMonitorConnection, nil
-}
-
 type RoutingTableFactory struct {
 	RoutePublisherFactory control.PublisherFactory
 }

gateway/pathhealth/pathwatcher.go

@@ -37,12 +37,6 @@ const (
 	defaultProbeInterval = 500 * time.Millisecond
 )
 
-// ProbeConnFactory is used to construct net.PacketConn objects for sending and
-// receiving probes.
-type ProbeConnFactory interface {
-	New(context.Context) (net.PacketConn, error)
-}
-
 // DefaultPathWatcherFactory creates PathWatchers.
 type DefaultPathWatcherFactory struct {
 	// LocalIA is the ID of the local AS.
@@ -51,7 +45,6 @@ type DefaultPathWatcherFactory struct {
 	LocalIP net.IP
 	// RevocationHandler is the revocation handler.
 	RevocationHandler snet.RevocationHandler
-	// ConnFactory is used to create probe connections.
 	// Probeinterval defines the interval at which probes are sent. If it is not
 	// set a default is used.
 	ProbeInterval time.Duration
@@ -86,6 +79,8 @@ func (f *DefaultPathWatcherFactory) New(
 		}
 		return create(remote)
 	}
+	// FIXME(JordiSubira): Keep or change the listening port, once we decide
+	// how SCMP are addressed.
 	nc, err := (&snet.DefaultConnector{
 		SCMPHandler: scmpHandler{
 			wrappedHandler: snet.DefaultSCMPHandler{

pkg/snet/BUILD.bazel

@@ -5,14 +5,14 @@ go_library(
     srcs = [
         "base.go",
         "conn.go",
-        "dispatcher.go",
         "interface.go",
         "packet.go",
         "packet_conn.go",
         "path.go",
         "reader.go",
         "reply_pather.go",
         "router.go",
+        "scmp.go",
         "snet.go",
         "svcaddr.go",
         "udpaddr.go",

pkg/snet/snet.go

@@ -127,19 +127,6 @@ func (n *SCIONNetwork) Listen(ctx context.Context, network string, listen *net.U
 		return nil, serrors.New("Unknown network", "network", network)
 	}
 
-	// FIXME(JordiSubira): Without the dispatcher if the host in the address parameter is nil
-	// or a literal unspecified IP address we can listen on all available IP addresses, except
-	// multicast. Port and address can be exposed using LocalAddr() on the returned Conn.
-	// if listen == nil {
-	// 	return nil, serrors.New("nil listen addr not supported")
-	// }
-	// if listen.IP == nil {
-	// 	return nil, serrors.New("nil listen IP not supported")
-	// }
-	// if listen.IP.IsUnspecified() {
-	// 	return nil, serrors.New("unspecified listen IP not supported")
-	// }
-
 	packetConn, err := n.Connector.OpenUDP(listen)
 	if err != nil {
 		return nil, err

private/app/appnet/infraenv.go

@@ -203,8 +203,8 @@ func GenerateTLSConfig() (*tls.Config, error) {
 
 // AddressRewriter initializes path and svc resolvers for infra servers.
 //
-// The connection factory is used to open sockets for SVC resolution requests.
-// If the connection factory is nil, the default connection factory is used.
+// The connector is used to open sockets for SVC resolution requests.
+// If the connector is nil, the default connection factory is used.
 func (nc *NetworkConfig) AddressRewriter(
 	connector snet.Connector) *AddressRewriter {
 
@@ -291,6 +291,9 @@ func (nc *NetworkConfig) initQUICSockets() (net.PacketConn, net.PacketConn, erro
 	serverNet := &snet.SCIONNetwork{
 		LocalIA: nc.IA,
 		Connector: &snet.DefaultConnector{
+			// XXX(roosd): This is essential, the server must not read SCMP
+			// errors. Otherwise, the accept loop will always return that error
+			// on every subsequent call to accept.
 			SCMPHandler: ignoreSCMP{},
 			Metrics:     nc.SCIONPacketConnMetrics,
 		},

private/app/flag/env.go

@@ -75,8 +75,7 @@ func (v *ipVal) Type() string   { return "ip" }
 func (v *ipVal) String() string { return netip.Addr(*v).String() }
 
 // SCIONEnvironment can be used to access the common SCION configuration values,
-// like the SCION daemon address and the local IP
-// as well as the local ISD-AS.
+// like the SCION daemon address and the local IP as well as the local ISD-AS.
 type SCIONEnvironment struct {
 	sciondFlag *pflag.Flag
 	sciondEnv  *string

private/svc/svc.go

@@ -50,9 +50,9 @@ const (
 type ResolverPacketConnector struct {
 	// Connector opens a PacketConn to receive and send packets.
 	Connector snet.Connector
-	// source contains the address from which packets should be sent.
+	// LocalIA contains the address from which packets should be sent.
 	LocalIA addr.IA
-	// handler handles packets for SVC destinations.
+	// Handler handles packets for SVC destinations.
 	Handler RequestHandler
 }
 
@@ -76,9 +76,9 @@ func (c *ResolverPacketConnector) OpenUDP(u *net.UDPAddr) (snet.PacketConn, erro
 type ResolverPacketConn struct {
 	// PacketConn is the conn to receive and send packets.
 	snet.PacketConn
-	// source contains the address from which packets should be sent.
+	// Source contains the address from which packets should be sent.
 	Source snet.SCIONAddress
-	// handler handles packets for SVC destinations.
+	// Handler handles packets for SVC destinations.
 	Handler RequestHandler
 }
 

private/topology/topology.go

@@ -33,7 +33,8 @@ import (
 
 const (
 	// EndhostPort is the underlay port that SCION binds to on non-routers.
-	EndhostPort       = underlay.EndhostPort
+	EndhostPort = underlay.EndhostPort
+	// TODO(JordiSubira): Yet to be defined.
 	HostPortRangeLow  = 0
 	HostPortRangeHigh = 1<<16 - 1
 )

private/underlay/conn/BUILD.bazel

@@ -1,4 +1,4 @@
-load("//tools/lint:go.bzl", "go_library")
+load("//tools/lint:go.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
@@ -23,3 +23,14 @@ go_library(
         "//conditions:default": [],
     }),
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["conn_test.go"],
+    embed = [":go_default_library"],
+    deps = [
+        "//pkg/private/xtest:go_default_library",
+        "@com_github_stretchr_testify//assert:go_default_library",
+        "@com_github_stretchr_testify//require:go_default_library",
+    ],
+)

private/underlay/conn/conn.go

@@ -32,6 +32,13 @@ import (
 	"github.com/scionproto/scion/private/underlay/sockctrl"
 )
 
+const (
+	// ReceiveBufferSize is the size of receive buffers used by the dispatcher.
+	ReceiveBufferSize = 1 << 20
+	// SendBufferSize is the size of the send buffers used by the dispatcher.
+	SendBufferSize = 1 << 20
+)
+
 // Messages is a list of ipX.Messages. It is necessary to hide the type alias
 // between ipv4.Message, ipv6.Message and socket.Message.
 type Messages []ipv4.Message
@@ -88,6 +95,11 @@ func OpenConn(addr *net.UDPAddr) (net.PacketConn, error) {
 	// because the socket options are specific to only one socket type, so we
 	// degrade udp to only udp4.
 	listeningAddr := copyUDPAddr(addr)
+	if listeningAddr == nil {
+		listeningAddr = &net.UDPAddr{
+			IP: net.IPv4zero,
+		}
+	}
 	if (listeningAddr.Network() == "udp" || listeningAddr.Network() == "udp4") &&
 		listeningAddr.IP == nil {
 		listeningAddr.IP = net.IPv4zero
@@ -96,9 +108,11 @@ func OpenConn(addr *net.UDPAddr) (net.PacketConn, error) {
 		listeningAddr.IP = net.IPv6zero
 	}
 
+	// TODO(JordiSubira): Should we keep a default config or use the passed-through
+	// configuration.
 	c, err := New(listeningAddr, nil, &Config{
-		SendBufferSize:    0,
-		ReceiveBufferSize: 0,
+		SendBufferSize:    SendBufferSize,
+		ReceiveBufferSize: ReceiveBufferSize,
 	})
 	if err != nil {
 		return nil, serrors.WrapStr("unable to open conn", err)
@@ -193,9 +207,6 @@ type connUDPBase struct {
 func (cc *connUDPBase) initConnUDP(network string, laddr, raddr *net.UDPAddr, cfg *Config) error {
 	var c *net.UDPConn
 	var err error
-	if laddr == nil {
-		return serrors.New("listen address must be specified")
-	}
 	if raddr == nil {
 		if c, err = net.ListenUDP(network, laddr); err != nil {
 			return serrors.WrapStr("Error listening on socket", err,

private/underlay/conn/conn_test.go

@@ -0,0 +1,78 @@
+// Copyright 2023 ETH Zurich
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package conn
+
+import (
+	"net"
+	"testing"
+
+	"github.com/scionproto/scion/pkg/private/xtest"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestOpenConn(t *testing.T) {
+	testCases := map[string]struct {
+		addr *net.UDPAddr
+	}{
+		"nil": {
+			addr: nil,
+		},
+		"undefined_addr": {
+			addr: xtest.MustParseUDPAddr(t, "0.0.0.0:0"),
+		},
+		"undefined_port": {
+			addr: xtest.MustParseUDPAddr(t, "127.0.0.1:0"),
+		},
+	}
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			sc, err := OpenConn(tc.addr)
+			require.NoError(t, err)
+			defer sc.Close()
+			lAddr := sc.LocalAddr().(*net.UDPAddr)
+
+			if tc.addr != nil && !tc.addr.IP.IsUnspecified() {
+				assert.Equal(t, tc.addr.IP, lAddr.IP)
+			}
+
+			// Client
+			cc, err := New(nil, lAddr, &Config{
+				SendBufferSize:    0,
+				ReceiveBufferSize: 0,
+			})
+			require.NoError(t, err)
+			defer cc.Close()
+
+			exchangeMessages := func(sc net.PacketConn, cc Conn) {
+				msg := []byte("hello")
+
+				go func() {
+					_, err := cc.Write(msg)
+					require.NoError(t, err)
+				}()
+
+				buf := make([]byte, 100)
+				n, cAddr, err := sc.ReadFrom(buf)
+				require.NoError(t, err)
+				assert.Equal(t, msg, buf[:n])
+				assert.Equal(t, cc.LocalAddr(), cAddr)
+			}
+
+			exchangeMessages(sc, cc)
+		})
+	}
+
+}

router/dataplane.go

@@ -624,14 +624,6 @@ func (p *scionPacketProcessor) processPkt(rawPkt []byte,
 		return processResult{}, err
 	}
 	pld := p.lastLayer.LayerPayload()
-	srcHost, _ := p.scionLayer.SrcAddr()
-	dstHost, _ := p.scionLayer.DstAddr()
-	log.Debug("packet",
-		"src IA", p.scionLayer.SrcIA,
-		"dst IA", p.scionLayer.DstIA,
-		"src addr", srcHost.String(),
-		"dst addr", dstHost.String(),
-	)
 
 	pathType := p.scionLayer.PathType
 	switch pathType {
@@ -1466,9 +1458,9 @@ func addEndhostPort(lastLayer gopacket.DecodingLayer, dst *net.IPAddr) (*net.UDP
 		if port < topology.HostPortRangeLow || port > topology.HostPortRangeHigh {
 			port = topology.EndhostPort
 		}
-		log.Debug("TBR XXXJ:", "udp port that will be rewritten", port)
 		return &net.UDPAddr{IP: dst.IP, Port: int(port)}, nil
 	case slayers.L4SCMP:
+		// TODO(JordiSubira): On-going discussion regarding SCMP dst port
 		return &net.UDPAddr{IP: dst.IP, Port: topology.EndhostPort}, nil
 	default:
 		panic(fmt.Sprintf("Port rewriting not supported for protcol number %v", l4Type))

tools/end2end/main.go

@@ -203,7 +203,6 @@ func (s server) handlePing(conn snet.PacketConn) error {
 		))
 	}
 	log.Info(fmt.Sprintf("Ping received from %s:%d, sending pong.", p.Source, udp.SrcPort))
-	log.Info((fmt.Sprintf("%v %d", p.Source, udp.SrcPort)))
 	raw, err := json.Marshal(Pong{
 		Client:  p.Source.IA,
 		Server:  integration.Local.IA,