braccept: more br_parent tests

[sgmonroy]

Add test for the following cases:

• revoked parent interface
• svc tests
• revoked child (not owned) interface
• one-hop-path parent interface tests

Also fix some braccept issues when cloning packets.

---

This change is 

[lukedirtwalker]

Reviewed 16 of 16 files at r1.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @sgmonroy)

---

go/border/braccept/svc_tests.go, line 1 at r1 (raw file):

// Copyright 2018 ETH Zurich

2019?

---

go/border/braccept/svc_tests.go, line 17 at r1 (raw file):

package main

func svc_anycast_parent_to_internal_host() int {

In general it's not really go style to use underlines for names. But I guess all tests are already this way so we can also leave it.
svcAnycastParentToInternalHost would be the go way.

---

go/border/braccept/svc_tests.go, line 18 at r1 (raw file):

func svc_anycast_parent_to_internal_host() int {
	pkt0 := AllocatePacket()

General note. All tests do pktX := AllocatePacket(); pktX.ParsePacket why isn't there a single ParsePacket function that returns the parsed packet?

---

go/border/braccept/layers/scnpath.go, line 30 at r1 (raw file):

}

func (p *ScnPath) Clone() ScnPath {

this isn't used anywhere? why is it needed?
Also why doesn't it return a pointer object, since the receiver is pointer?

---

go/border/braccept/parser/scion.go, line 350 at r1 (raw file):

}

func newScionTags() scionTags {

I don't see why the slice is needed wouldn't a map[string]interface{} provide everything you need?

And at this point you could simply use type scionTags map[string]interface{}

---

go/border/braccept/parser/scion.go, line 354 at r1 (raw file):

	tags.fields = []interface{}{}
	tags.toIndex = make(map[string]int)
	return tags

could be simpler:

return scionTags{
    fields: []interface{}{},
    toIndex: make(map[string]int),
}

[sgmonroy]

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @lukedirtwalker)

---

go/border/braccept/svc_tests.go, line 1 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

2019?

Done.

---

go/border/braccept/svc_tests.go, line 17 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

In general it's not really go style to use underlines for names. But I guess all tests are already this way so we can also leave it.
svcAnycastParentToInternalHost would be the go way.

yeah, it should be a different PR anyway.

---

go/border/braccept/svc_tests.go, line 18 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

General note. All tests do pktX := AllocatePacket(); pktX.ParsePacket why isn't there a single ParsePacket function that returns the parsed packet?

No special reason, it could be done, but it wasn't done.

---

go/border/braccept/layers/scnpath.go, line 30 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

this isn't used anywhere? why is it needed?
Also why doesn't it return a pointer object, since the receiver is pointer?

It is used in go/border/braccept/parser/scion.go .
done.

---

go/border/braccept/parser/scion.go, line 350 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

I don't see why the slice is needed wouldn't a map[string]interface{} provide everything you need?

And at this point you could simply use type scionTags map[string]interface{}

Reworked to something IMO more straight forward. Single map as you suggested but providing a reverse lookup to retrieve a tag from a pointer to an object.

---

go/border/braccept/parser/scion.go, line 354 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

could be simpler:

return scionTags{
    fields: []interface{}{},
    toIndex: make(map[string]int),
}

Done.

[lukedirtwalker]

Reviewed 3 of 3 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved