sciond-API: Only have a single Connect method

Instead of having one with timeout and one without just have one method with context.

Also the same request id counter for all connection, to make sure that no two connection can potentially send a request with the same ID.

go/lib/infra/infraenv/infraenv.go

@@ -18,6 +18,7 @@ package infraenv
 
 import (
 	"bytes"
+	"context"
 	"crypto/tls"
 	"net"
 	"time"
@@ -297,14 +298,16 @@ func (h *LegacyForwardingHandler) Handle(request *svc.Request) (svc.Result, erro
 func NewRouter(localIA addr.IA, sd env.SciondClient) (snet.Router, error) {
 	ticker := time.NewTicker(time.Second)
 	timer := time.NewTimer(sd.InitialConnectPeriod.Duration)
+	ctx, cancelF := context.WithTimeout(context.Background(), sd.InitialConnectPeriod.Duration)
+	defer cancelF()
 	defer ticker.Stop()
 	defer timer.Stop()
 	// XXX(roosd): Initial retrying is implemented here temporarily.
 	// In https://github.com/scionproto/scion/issues/1974 this will be
 	// done transparently and pushed to snet.NewNetwork.
 	var router snet.Router
 	for {
-		sciondConn, err := sciond.NewService(sd.Path).Connect()
+		sciondConn, err := sciond.NewService(sd.Path).Connect(ctx)
 		if err == nil {
 			router = &snet.BaseRouter{
 				IA: localIA,

go/lib/sciond/BUILD.bazel

@@ -17,6 +17,7 @@ go_library(
         "//go/lib/infra/disp:go_default_library",
         "//go/lib/log:go_default_library",
         "//go/lib/sciond/internal/metrics:go_default_library",
+        "//go/lib/scrypto:go_default_library",
         "//go/lib/serrors:go_default_library",
         "//go/lib/sock/reliable:go_default_library",
         "//go/lib/util:go_default_library",

go/lib/sciond/sciond.go

@@ -39,6 +39,7 @@ import (
 	"github.com/scionproto/scion/go/lib/infra/disp"
 	"github.com/scionproto/scion/go/lib/log"
 	"github.com/scionproto/scion/go/lib/sciond/internal/metrics"
+	"github.com/scionproto/scion/go/lib/scrypto"
 	"github.com/scionproto/scion/go/lib/serrors"
 	"github.com/scionproto/scion/go/lib/sock/reliable"
 	"github.com/scionproto/scion/go/proto"
@@ -49,6 +50,10 @@ var (
 	ErrUnableToConnect = serrors.New("unable to connect to SCIOND")
 )
 
+var (
+	requestID = scrypto.RandUint64()
+)
+
 const (
 	// DefaultSCIONDPath contains the system default for a SCIOND socket.
 	DefaultSCIONDPath = "/run/shm/sciond/default.sock"
@@ -57,39 +62,24 @@ const (
 )
 
 // Service describes a SCIOND endpoint. New connections to SCIOND can be
-// initialized via Connect and ConnectTimeout.
+// initialized via Connect.
 type Service interface {
 	// Connect connects to the SCIOND server described by Service. Future
 	// method calls on the returned Connector request information from SCIOND.
-	// The information is not guaranteed to be fresh, as the returned connector
-	// caches ASInfo replies for ASInfoTTL time, IFInfo replies for IFInfoTTL
-	// time and SVCInfo for SVCInfoTTL time.
-	Connect() (Connector, error)
-	// ConnectTimeout acts like Connect but takes a timeout.
-	//
-	// A timeout of 0 means infinite timeout.
-	//
-	// To check for timeout errors, type assert the returned error to
-	// *net.OpError and call method Timeout().
-	ConnectTimeout(timeout time.Duration) (Connector, error)
+	Connect(context.Context) (Connector, error)
 }
 
 type service struct {
-	path       string
-	reconnects bool
+	path string
 }
 
 // NewService returns a SCIOND API connection factory.
 func NewService(name string) Service {
 	return &service{path: name}
 }
 
-func (s *service) Connect() (Connector, error) {
-	return newConn(s.path, 0)
-}
-
-func (s *service) ConnectTimeout(timeout time.Duration) (Connector, error) {
-	return newConn(s.path, timeout)
+func (s *service) Connect(ctx context.Context) (Connector, error) {
+	return newConn(ctx, s.path)
 }
 
 // A Connector is used to query SCIOND. The connector maintains an internal
@@ -124,25 +114,18 @@ type conn struct {
 	path      string
 }
 
-func newConn(path string, initialCheckTimeout time.Duration) (*conn, error) {
+func newConn(ctx context.Context, path string) (*conn, error) {
 	c := &conn{path: path}
 	// Test during initialization that SCIOND is alive; this helps catch some
 	// unfixable issues (like bad socket name) while apps are still
 	// initializing their networking.
-	if err := c.checkForSciond(initialCheckTimeout); err != nil {
+	if err := c.checkForSciond(ctx); err != nil {
 		return nil, err
 	}
 	return c, nil
 }
 
-func (c *conn) checkForSciond(initialCheckTimeout time.Duration) error {
-	ctx := context.Background()
-	if initialCheckTimeout != 0 {
-		timeoutCtx, cancelF := context.WithTimeout(context.Background(), initialCheckTimeout)
-		defer cancelF()
-		ctx = timeoutCtx
-	}
-
+func (c *conn) checkForSciond(ctx context.Context) error {
 	dispatcher, err := c.ctxAwareConnect(ctx)
 	if err != nil {
 		return serrors.Wrap(ErrUnableToConnect, err)
@@ -202,7 +185,7 @@ func (c *conn) Paths(ctx context.Context, dst, src addr.IA, max uint16,
 	reply, err := roundTripper.Request(
 		ctx,
 		&Pld{
-			Id:    c.nextID(),
+			Id:    nextID(),
 			Which: proto.SCIONDMsg_Which_pathReq,
 			PathReq: &PathReq{
 				Dst:      dst.IAInt(),
@@ -231,7 +214,7 @@ func (c *conn) ASInfo(ctx context.Context, ia addr.IA) (*ASInfoReply, error) {
 	pld, err := roundTripper.Request(
 		ctx,
 		&Pld{
-			Id:    c.nextID(),
+			Id:    nextID(),
 			Which: proto.SCIONDMsg_Which_asInfoReq,
 			AsInfoReq: &ASInfoReq{
 				Isdas: ia.IAInt(),
@@ -257,7 +240,7 @@ func (c *conn) IFInfo(ctx context.Context, ifs []common.IFIDType) (*IFInfoReply,
 	pld, err := roundTripper.Request(
 		ctx,
 		&Pld{
-			Id:    c.nextID(),
+			Id:    nextID(),
 			Which: proto.SCIONDMsg_Which_ifInfoRequest,
 			IfInfoRequest: &IFInfoRequest{
 				IfIDs: ifs,
@@ -285,7 +268,7 @@ func (c *conn) SVCInfo(ctx context.Context,
 	pld, err := roundTripper.Request(
 		ctx,
 		&Pld{
-			Id:    c.nextID(),
+			Id:    nextID(),
 			Which: proto.SCIONDMsg_Which_serviceInfoRequest,
 			ServiceInfoRequest: &ServiceInfoRequest{
 				ServiceTypes: svcTypes,
@@ -322,7 +305,7 @@ func (c *conn) RevNotification(ctx context.Context,
 	reply, err := roundTripper.Request(
 		ctx,
 		&Pld{
-			Id:    c.nextID(),
+			Id:    nextID(),
 			Which: proto.SCIONDMsg_Which_revNotification,
 			RevNotification: &RevNotification{
 				SRevInfo: sRevInfo,
@@ -343,8 +326,8 @@ func (c *conn) Close(_ context.Context) error {
 }
 
 // nextID returns a unique value for identifiying SCIOND requests.
-func (c *conn) nextID() uint64 {
-	return atomic.AddUint64(&c.requestID, 1)
+func nextID() uint64 {
+	return atomic.AddUint64(&requestID, 1)
 }
 
 func connectTimeout(socketName string, timeout time.Duration) (*disp.Dispatcher, error) {

go/lib/snet/snet.go

@@ -48,6 +48,7 @@
 package snet
 
 import (
+	"context"
 	"time"
 
 	"github.com/scionproto/scion/go/lib/addr"
@@ -168,7 +169,7 @@ func NewCustomNetwork(ia addr.IA, sciondPath string,
 func getResolver(sciondPath string) (pathmgr.Resolver, error) {
 	var pathResolver pathmgr.Resolver
 	if sciondPath != "" {
-		sciondConn, err := sciond.NewService(sciondPath).Connect()
+		sciondConn, err := sciond.NewService(sciondPath).Connect(context.Background())
 		if err != nil {
 			return nil, common.NewBasicError("Unable to initialize SCIOND service", err)
 		}

go/tools/scmp/main.go

@@ -60,8 +60,10 @@ func main() {
 		*sciondPath = sciond.GetDefaultSCIONDPath(nil)
 	}
 	// Connect to sciond
+	ctx, cancelF := context.WithTimeout(context.Background(), time.Second)
+	defer cancelF()
 	sd := sciond.NewService(*sciondPath)
-	sdConn, err = sd.ConnectTimeout(1 * time.Second)
+	sdConn, err = sd.Connect(ctx)
 	if err != nil {
 		cmn.Fatal("Failed to connect to SCIOND: %v\n", err)
 	}

go/tools/showpaths/paths.go

@@ -64,9 +64,12 @@ func main() {
 	}
 	defer log.LogPanicAndExit()
 
+	ctx, cancelF := context.WithTimeout(context.Background(), *timeout)
+	defer cancelF()
+
 	sd := sciond.NewService(*sciondPath)
 	var err error
-	sdConn, err := sd.ConnectTimeout(*timeout)
+	sdConn, err := sd.Connect(ctx)
 	if err != nil {
 		LogFatal("Failed to connect to SCIOND", "err", err)
 	}
@@ -82,12 +85,10 @@ func main() {
 	fmt.Println("Available paths to", dstIA)
 	var pathStatuses map[string]pathprobe.Status
 	if *status {
-		ctx, cancelF := context.WithTimeout(context.Background(), *timeout)
 		pathStatuses, err = pathprobe.Prober{
 			Local: local,
 			DstIA: dstIA,
 		}.GetStatuses(ctx, reply.Entries)
-		cancelF()
 		if err != nil {
 			LogFatal("Failed to get status", "err", err)
 		}