Repository: scionproto/scion
Pull request #3832 (2020-07-14 07:40:01)

cs: don't verify RemoteOutIf in beaconing
Also for v2 don't set it.

Since we want to get rid of keepalives, which also detect remote
interfaces we have to stop using remote interface IDs. Except for
peering links we don't really need to know the remote interface ID. For
peering links we should add the remote ID to the topology file once we
implement support for peering links.

<!-- 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/3832)
<!-- Reviewable:end -->



Patch:
diff --git a/go/cs/beacon/beacon.go b/go/cs/beacon/beacon.go
index 851114fa5..145be15a1 100644
--- a/go/cs/beacon/beacon.go
+++ b/go/cs/beacon/beacon.go
@@ -56,7 +56,7 @@ func (b Beacon) Diversity(other Beacon) int {
 }
 
 func link(entry *seg.ASEntry) (addr.IA, common.IFIDType) {
-	return entry.IA(), entry.HopEntries[0].RemoteOutIF
+	return entry.IA(), common.IFIDType(entry.HopEntries[0].HopField.ConsIngress)
 }
 
 // BeaconOrErr contains a read-only beacon or an error.
diff --git a/go/cs/beaconing/extender.go b/go/cs/beaconing/extender.go
index 3e1fb4e02..2256940b3 100644
--- a/go/cs/beaconing/extender.go
+++ b/go/cs/beaconing/extender.go
@@ -263,7 +263,7 @@ func (s *DefaultExtender) createHopEntries(ingress, egress common.IFIDType, peer
 	thisBeta := beta ^ binary.BigEndian.Uint16(hopEntry.HopField.MAC[:2])
 	hopEntries := []*seg.HopEntry{hopEntry}
 	for _, peer := range peers {
-		peerEntry, err := s.createHopEntry(peer, egress, ts, thisBeta)
+		peerEntry, err := s.createPeerEntry(peer, egress, ts, thisBeta)
 		if err != nil {
 			log.Debug("Ignoring peer link upon error", "task", s.Task, "ifid", peer, "err", err)
 			continue
@@ -276,11 +276,43 @@ func (s *DefaultExtender) createHopEntries(ingress, egress common.IFIDType, peer
 func (s *DefaultExtender) createHopEntry(ingress, egress common.IFIDType, ts time.Time,
 	beta uint16) (*seg.HopEntry, error) {
 
+	remoteInIA, err := s.remoteIA(ingress)
+	if err != nil {
+		return nil, serrors.WrapStr("checking remote ingress interface", err, "ifid", ingress)
+	}
+	remoteInMTU, err := s.remoteMTU(ingress)
+	if err != nil {
+		return nil, serrors.WrapStr("checking remote ingress interface (mtu)", err, "ifid", ingress)
+	}
+	remoteOutIA, err := s.remoteIA(egress)
+	if err != nil {
+		return nil, serrors.WrapStr("checking remote egress interface", err, "ifid", egress)
+	}
+	hopF := s.createHopF(uint16(ingress), uint16(egress), ts, beta)
+	hop := &seg.HopEntry{
+		RawInIA:     remoteInIA,
+		RemoteInIF:  0,
+		InMTU:       uint16(remoteInMTU),
+		RawOutIA:    remoteOutIA,
+		RemoteOutIF: 0,
+		HopField: seg.HopField{
+			ConsIngress: hopF.ConsIngress,
+			ConsEgress:  hopF.ConsEgress,
+			ExpTime:     hopF.ExpTime,
+			MAC:         hopF.Mac,
+		},
+	}
+	return hop, nil
+}
+
+func (s *DefaultExtender) createPeerEntry(ingress, egress common.IFIDType, ts time.Time,
+	beta uint16) (*seg.HopEntry, error) {
+
 	remoteInIA, remoteInIfID, remoteInMTU, err := s.remoteInfo(ingress)
 	if err != nil {
 		return nil, serrors.WrapStr("checking remote ingress interface", err, "ifid", ingress)
 	}
-	remoteOutIA, remoteOutIfid, _, err := s.remoteInfo(egress)
+	remoteOutIA, err := s.remoteIA(egress)
 	if err != nil {
 		return nil, serrors.WrapStr("checking remote egress interface", err, "ifid", egress)
 	}
@@ -290,7 +322,7 @@ func (s *DefaultExtender) createHopEntry(ingress, egress common.IFIDType, ts tim
 		RemoteInIF:  remoteInIfID,
 		InMTU:       uint16(remoteInMTU),
 		RawOutIA:    remoteOutIA,
-		RemoteOutIF: remoteOutIfid,
+		RemoteOutIF: 0,
 		HopField: seg.HopField{
 			ConsIngress: hopF.ConsIngress,
 			ConsEgress:  hopF.ConsEgress,
@@ -301,6 +333,33 @@ func (s *DefaultExtender) createHopEntry(ingress, egress common.IFIDType, ts tim
 	return hop, nil
 }
 
+func (s *DefaultExtender) remoteIA(ifID common.IFIDType) (addr.IAInt, error) {
+	if ifID == 0 {
+		return 0, nil
+	}
+	intf := s.Intfs.Get(ifID)
+	if intf == nil {
+		return 0, serrors.New("interface not found")
+	}
+	topoInfo := intf.TopoInfo()
+	if topoInfo.IA.IsWildcard() {
+		return 0, serrors.New("remote is wildcard", "isd_as", topoInfo.IA)
+	}
+	return topoInfo.IA.IAInt(), nil
+}
+
+func (s *DefaultExtender) remoteMTU(ifID common.IFIDType) (uint16, error) {
+	if ifID == 0 {
+		return 0, nil
+	}
+	intf := s.Intfs.Get(ifID)
+	if intf == nil {
+		return 0, serrors.New("interface not found")
+	}
+	topoInfo := intf.TopoInfo()
+	return uint16(topoInfo.MTU), nil
+}
+
 func (s *DefaultExtender) remoteInfo(ifid common.IFIDType) (
 	addr.IAInt, common.IFIDType, uint16, error) {
 
diff --git a/go/cs/beaconing/handler.go b/go/cs/beaconing/handler.go
index 24082e01c..2f6df0780 100644
--- a/go/cs/beaconing/handler.go
+++ b/go/cs/beaconing/handler.go
@@ -187,10 +187,6 @@ func (h *handler) validateASEntry(b beacon.Beacon) error {
 			return common.NewBasicError("Out IA of hop entry does not match local IA", nil,
 				"index", i, "expected", h.ia, "actual", hopEntry.OutIA())
 		}
-		if hopEntry.RemoteOutIF != b.InIfId {
-			return common.NewBasicError("RemoteOutIF of hop entry does not match ingress interface",
-				nil, "expected", b.InIfId, "actual", hopEntry.RemoteOutIF)
-		}
 	}
 	return nil
 }


Test patch:
diff --git a/go/cs/beaconing/extender_test.go b/go/cs/beaconing/extender_test.go
index 8df110032..30243e957 100644
--- a/go/cs/beaconing/extender_test.go
+++ b/go/cs/beaconing/extender_test.go
@@ -450,9 +450,7 @@ func TestDefaultExtenderExtend(t *testing.T) {
 			t.Run("hop entry check", func(t *testing.T) {
 				intf := intfs.Get(tc.egress)
 				ia := intf.TopoInfo().IA
-				remote := intf.TopoInfo().RemoteIFID
 				assert.Equal(t, ia, entry.HopEntries[0].OutIA())
-				assert.Equal(t, remote, entry.HopEntries[0].RemoteOutIF)
 
 				assert.Equal(t, uint16(tc.ingress), entry.HopEntries[0].HopField.ConsIngress)
 				assert.Equal(t, uint16(tc.egress), entry.HopEntries[0].HopField.ConsEgress)
@@ -462,9 +460,7 @@ func TestDefaultExtenderExtend(t *testing.T) {
 			t.Run("peer entry check", func(t *testing.T) {
 				intf := intfs.Get(tc.egress)
 				ia := intf.TopoInfo().IA
-				remote := intf.TopoInfo().RemoteIFID
 				assert.Equal(t, ia, entry.HopEntries[1].OutIA())
-				assert.Equal(t, remote, entry.HopEntries[1].RemoteOutIF)
 
 				assert.Equal(t, uint16(peer), entry.HopEntries[1].HopField.ConsIngress)
 				assert.Equal(t, uint16(tc.egress), entry.HopEntries[1].HopField.ConsEgress)
diff --git a/go/cs/beaconing/handler_test.go b/go/cs/beaconing/handler_test.go
index 0894ff1a7..f66583e5f 100644
--- a/go/cs/beaconing/handler_test.go
+++ b/go/cs/beaconing/handler_test.go
@@ -156,17 +156,6 @@ func TestNewHandler(t *testing.T) {
 		res := handler.Handle(defaultTestReq(rw, pseg))
 		assert.Equal(t, res, infra.MetricsErrInvalid)
 	})
-	t.Run("Invalid remote out interface", func(t *testing.T) {
-		pseg := testBeacon(g, []common.IFIDType{graph.If_120_A_110_X}).Segment
-		asEntry := pseg.ASEntries[pseg.MaxAEIdx()]
-		asEntry.HopEntries[0].RemoteOutIF = 42
-		raw, err := asEntry.Pack()
-		require.NoError(t, err)
-		pseg.RawASEntries[pseg.MaxAEIdx()].Blob = raw
-		res := handler.Handle(defaultTestReq(rw, pseg))
-		assert.Equal(t, res, infra.MetricsErrInvalid)
-	})
-
 	t.Run("Verification error", func(t *testing.T) {
 		verifier := mock_infra.NewMockVerifier(mctrl)
 		verifier.EXPECT().WithIA(gomock.Any()).Return(verifier)