Repository: scionproto/scion
Pull request #3437 (2019-11-28 15:21:25)

segfetcher: Add LocalInfo to resolver
This is to make sure that we never try to fetch segments that are locally cached.
This happened some times in the past.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/scionproto/scion/3437)
<!-- Reviewable:end -->



Patch:
diff --git a/go/lib/infra/modules/segfetcher/fetcher.go b/go/lib/infra/modules/segfetcher/fetcher.go
index 6013bfd9e..22e9d1f05 100644
--- a/go/lib/infra/modules/segfetcher/fetcher.go
+++ b/go/lib/infra/modules/segfetcher/fetcher.go
@@ -75,8 +75,10 @@ type FetcherConfig struct {
 	// SciondMode enables sciond mode, this means it uses the local CS to fetch
 	// crypto material and considers revocations in the path lookup.
 	SciondMode bool
-	// The namespace used for metrics.
+	// MetricsNamespace is the namespace used for metrics.
 	MetricsNamespace string
+	// LocalInfo provides information about local segments.
+	LocalInfo LocalInfo
 }
 
 // New creates a new fetcher from the configuration.
@@ -84,7 +86,7 @@ func (cfg FetcherConfig) New() *Fetcher {
 	return &Fetcher{
 		Validator: cfg.Validator,
 		Splitter:  cfg.Splitter,
-		Resolver:  NewResolver(cfg.PathDB, cfg.RevCache),
+		Resolver:  NewResolver(cfg.PathDB, cfg.RevCache, cfg.LocalInfo),
 		Requester: &DefaultRequester{API: cfg.RequestAPI, DstProvider: cfg.DstProvider},
 		ReplyHandler: &seghandler.Handler{
 			Verifier: &seghandler.DefaultVerifier{Verifier: cfg.VerificationFactory.NewVerifier()},
diff --git a/go/lib/infra/modules/segfetcher/resolver.go b/go/lib/infra/modules/segfetcher/resolver.go
index 8bf28f4b0..31146e5e5 100644
--- a/go/lib/infra/modules/segfetcher/resolver.go
+++ b/go/lib/infra/modules/segfetcher/resolver.go
@@ -37,21 +37,30 @@ type Resolver interface {
 	Resolve(ctx context.Context, segs Segments, req RequestSet) (Segments, RequestSet, error)
 }
 
+// LocalInfo provides information about which segments are always locally
+// stored.
+type LocalInfo interface {
+	// IsSegLocal returns whether this segment should always be locally cached.
+	IsSegLocal(ctx context.Context, src, dst addr.IA) (bool, error)
+}
+
 // NewResolver creates a new resolver with the given DB. The DB might be
 // customized. E.g. a PS could inject a wrapper around GetNextQuery so that it
 // always returns that the cache is up to date for segments that should be
 // available local.
-func NewResolver(DB pathdb.Read, revCache revcache.RevCache) *DefaultResolver {
+func NewResolver(DB pathdb.Read, revCache revcache.RevCache, localInfo LocalInfo) *DefaultResolver {
 	return &DefaultResolver{
-		DB:       DB,
-		RevCache: revCache,
+		DB:        DB,
+		RevCache:  revCache,
+		LocalInfo: localInfo,
 	}
 }
 
 // DefaultResolver is the default resolver implementation.
 type DefaultResolver struct {
-	DB       pathdb.Read
-	RevCache revcache.RevCache
+	DB        pathdb.Read
+	RevCache  revcache.RevCache
+	LocalInfo LocalInfo
 }
 
 // Resolve resolves a request set. It returns the segments that are locally
@@ -93,6 +102,11 @@ func (r *DefaultResolver) Resolve(ctx context.Context, segs Segments,
 		req.Cores = nil
 	}
 	for i, coreReq := range req.Cores {
+		if local, err := r.LocalInfo.IsSegLocal(ctx, coreReq.Src, coreReq.Dst); err != nil {
+			return segs, req, err
+		} else if local {
+			coreReq.State = Cached
+		}
 		if coreReq.State != Cached && coreReq.State != Fetched {
 			continue
 		}
@@ -145,6 +159,11 @@ func (r *DefaultResolver) resolveDownSegs(ctx context.Context, segs Segments,
 func (r *DefaultResolver) resolveSegment(ctx context.Context,
 	req Request, consDir bool) (seg.Segments, Request, error) {
 
+	if local, err := r.LocalInfo.IsSegLocal(ctx, req.Src, req.Dst); err != nil {
+		return nil, req, err
+	} else if local {
+		req.State = Cached
+	}
 	if req.State == Unresolved {
 		fetch, err := r.needsFetching(ctx, req)
 		if err != nil || fetch {
diff --git a/go/path_srv/internal/segreq/handler.go b/go/path_srv/internal/segreq/handler.go
index c1d55edec..8dfaba5aa 100644
--- a/go/path_srv/internal/segreq/handler.go
+++ b/go/path_srv/internal/segreq/handler.go
@@ -37,7 +37,8 @@ type handler struct {
 
 func NewHandler(args handlers.HandlerArgs) infra.Handler {
 	core := args.TopoProvider.Get().Core()
-	args.PathDB = createPathDB(args, core)
+	localInfo := createLocalInfo(args, core)
+	args.PathDB = createPathDB(args.PathDB, localInfo)
 	return &handler{
 		fetcher: segfetcher.FetcherConfig{
 			QueryInterval:       args.QueryInterval,
@@ -50,6 +51,7 @@ func NewHandler(args handlers.HandlerArgs) infra.Handler {
 			DstProvider:         createDstProvider(args, core),
 			Splitter:            &Splitter{ASInspector: args.ASInspector},
 			MetricsNamespace:    metrics.Namespace,
+			LocalInfo:           localInfo,
 		}.New(),
 		revCache: args.RevCache,
 	}
@@ -125,20 +127,21 @@ func createValidator(args handlers.HandlerArgs, core bool) segfetcher.Validator
 	return &CoreValidator{BaseValidator: base}
 }
 
-func createPathDB(args handlers.HandlerArgs, core bool) pathdb.PathDB {
-	var localInfo LocalInfo
+func createLocalInfo(args handlers.HandlerArgs, core bool) LocalInfo {
 	if core {
-		localInfo = &CoreLocalInfo{
+		return &CoreLocalInfo{
 			CoreChecker: CoreChecker{Inspector: args.ASInspector},
 			LocalIA:     args.IA,
 		}
-	} else {
-		localInfo = &NonCoreLocalInfo{
-			LocalIA: args.IA,
-		}
 	}
+	return &NonCoreLocalInfo{
+		LocalIA: args.IA,
+	}
+}
+
+func createPathDB(db pathdb.PathDB, localInfo LocalInfo) pathdb.PathDB {
 	return &PathDB{
-		PathDB:     args.PathDB,
+		PathDB:     db,
 		LocalInfo:  localInfo,
 		RetrySleep: 500 * time.Millisecond,
 	}
diff --git a/go/sciond/internal/fetcher/fetcher.go b/go/sciond/internal/fetcher/fetcher.go
index 19c4ecab2..b6993c39a 100644
--- a/go/sciond/internal/fetcher/fetcher.go
+++ b/go/sciond/internal/fetcher/fetcher.go
@@ -80,6 +80,7 @@ func NewFetcher(messenger infra.Messenger, pathDB pathdb.PathDB, trustStore Trus
 			Splitter:            NewRequestSplitter(localIA, trustStore),
 			SciondMode:          true,
 			MetricsNamespace:    metrics.Namespace,
+			LocalInfo:           neverLocal{},
 		}.New(),
 	}
 }
@@ -323,3 +324,7 @@ type dstProvider struct {
 func (r *dstProvider) Dst(_ context.Context, _ segfetcher.Request) (net.Addr, error) {
 	return &snet.Addr{IA: r.IA, Host: addr.NewSVCUDPAppAddr(addr.SvcPS)}, nil
 }
+
+type neverLocal struct{}
+
+func (neverLocal) IsSegLocal(_ context.Context, _, _ addr.IA) (bool, error) { return false, nil }


Test patch:
diff --git a/go/lib/infra/modules/segfetcher/resolver_test.go b/go/lib/infra/modules/segfetcher/resolver_test.go
index b4025cd38..10e362b0d 100644
--- a/go/lib/infra/modules/segfetcher/resolver_test.go
+++ b/go/lib/infra/modules/segfetcher/resolver_test.go
@@ -94,7 +94,7 @@ func (rt resolverTest) run(t *testing.T) {
 	} else {
 		revCache.EXPECT().Get(gomock.Any(), gomock.Any()).AnyTimes()
 	}
-	resolver := segfetcher.NewResolver(db, revCache)
+	resolver := segfetcher.NewResolver(db, revCache, neverLocal{})
 	segs, remainingReqs, err := resolver.Resolve(context.Background(), rt.Segs, rt.Req)
 	assert.Equal(t, rt.ExpectedSegments, segs)
 	assert.Equal(t, rt.ExpectedReqSet, remainingReqs)
@@ -875,3 +875,7 @@ func (m keySetContains) Matches(other interface{}) bool {
 func (m keySetContains) String() string {
 	return fmt.Sprintf("revcache.KeySet containing %v", m.keys)
 }
+
+type neverLocal struct{}
+
+func (neverLocal) IsSegLocal(_ context.Context, _, _ addr.IA) (bool, error) { return false, nil }