Skip to content

Commit 1724cd2

Browse files
beaconing: fix diversity calculation (scionproto#4854)
Currently the beacon diversity calculation was wrong. We only considered ingress interface in construction direction. However the first ASEntry will always have a zero ingress interface. Furthermore with this approach we did not really consider the last ingress interface, which was the ingress interface into the AS, which is not yet represented by an ASEntry (as the termination/extension only happens after selection). By using the egress interfaces interfaces instead we solve both problems. Co-authored-by: Roman Sharkov <roman.scharkov@gmail.com>
1 parent 8e4f8b1 commit 1724cd2

File tree

3 files changed

+44
-7
lines changed

3 files changed

+44
-7
lines changed

control/beacon/beacon.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,5 +60,5 @@ func (b Beacon) String() string {
6060
}
6161

6262
func link(entry seg.ASEntry) (addr.IA, uint16) {
63-
return entry.Local, entry.HopEntry.HopField.ConsIngress
63+
return entry.Local, entry.HopEntry.HopField.ConsEgress
6464
}

control/beacon/beacon_test.go

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,19 @@ import (
2020
"github.com/golang/mock/gomock"
2121
"github.com/stretchr/testify/assert"
2222

23+
"github.com/scionproto/scion/control/beacon"
24+
"github.com/scionproto/scion/pkg/addr"
2325
"github.com/scionproto/scion/pkg/private/xtest/graph"
26+
"github.com/scionproto/scion/pkg/segment"
2427
)
2528

2629
// TestBeaconDiversity tests that diversity is calculated correctly.
2730
func TestBeaconDiversity(t *testing.T) {
2831
var tests = []struct {
29-
name string
30-
beacon []uint16
32+
name string
33+
beacon []uint16
34+
// mBeacon allows to manually specify the other beacon.
35+
mBeacon beacon.Beacon
3136
diversity int
3237
}{
3338
{
@@ -51,14 +56,47 @@ func TestBeaconDiversity(t *testing.T) {
5156
},
5257
diversity: 2,
5358
},
59+
{
60+
name: "Last link distinct",
61+
mBeacon: beacon.Beacon{
62+
Segment: &segment.PathSegment{
63+
ASEntries: []segment.ASEntry{
64+
{
65+
Local: addr.MustParseIA("1-ff00:0:130"),
66+
Next: addr.MustParseIA("1-ff00:0:110"),
67+
HopEntry: segment.HopEntry{
68+
HopField: segment.HopField{
69+
ConsEgress: graph.If_130_A_110_X,
70+
},
71+
},
72+
},
73+
{
74+
Local: addr.MustParseIA("1-ff00:0:110"),
75+
Next: addr.MustParseIA("2-ff00:0:210"),
76+
HopEntry: segment.HopEntry{
77+
HopField: segment.HopField{
78+
ConsIngress: graph.If_110_X_130_A,
79+
ConsEgress: 2321,
80+
},
81+
},
82+
},
83+
},
84+
},
85+
InIfID: 2123,
86+
},
87+
diversity: 1,
88+
},
5489
}
5590
mctrl := gomock.NewController(t)
5691

5792
g := graph.NewDefaultGraph(mctrl)
5893
bseg := testBeacon(g, tests[0].beacon...)
5994
for _, test := range tests {
6095
t.Run(test.name, func(t *testing.T) {
61-
other := testBeacon(g, test.beacon...)
96+
other := test.mBeacon
97+
if len(test.beacon) > 0 {
98+
other = testBeacon(g, test.beacon...)
99+
}
62100
diversity := bseg.Diversity(other)
63101
assert.Equal(t, test.diversity, diversity)
64102
})

control/beacon/store_test.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,10 +274,9 @@ func testCoreStoreSelection(t *testing.T,
274274
}
275275

276276
func testBeacon(g *graph.Graph, desc ...uint16) beacon.Beacon {
277-
pseg := testSegment(g, desc)
278-
asEntry := pseg.ASEntries[pseg.MaxIdx()]
277+
pseg := testSegment(g, desc[:len(desc)-1])
279278
return beacon.Beacon{
280-
InIfID: asEntry.HopEntry.HopField.ConsIngress,
279+
InIfID: desc[len(desc)-1],
281280
Segment: pseg,
282281
}
283282
}

0 commit comments

Comments
 (0)