router: reduce per-packet allocations

[matzf]

Avoid allocating scionPacketProcessor; re-organized packet processing
functions. The scionPacketProcessor is now the main entry point for
packet processing and contains all the pre-allocated per-packet state.

Avoid allocation of message buffer for writing with WriteBatch.
We could just pre-allocate a buffer, but as we always write a batch of
size one, just use WriteTo instead.
For writing single packets, this is both more convenient and faster.
Additional benefit: Read/WriteBatch make rather large allocations
internally (see golang/go#26838) which is
avoided by using WriteTo.

The largest remaining allocations when processing SCION packets are:

• ReadBatch allocates temporary storage internally (as mentioned above).
  This is by far the biggest culprit, it accounts for almost 80% (after
  the changes in this patch).
• slayers/path.NewPath called during slayers.SCION.DecodeFromBytes:
  Could be avoided by reusing Path objects, but requires larger changes.
• slayers/path/scion.Raw.GetCurrentHopField and GetCurrentInfoField, called during parsePath:
  Both return pointer types. Should be possible to change this to value
  types instead, but requires larger changes.
• DataPlane.resolveLocalDst: allocates IP and UDPAddr. Can only really
  be fixed with better address types.
• slayers/path.FullMAC. Allocates both input and output. The function
  interface could be changed to allow reuse of the input buffer.
  The output buffer cannot be reused with this CMAC library.

Depends on (or actually includes) #4029.

---

This change is 

[lukedirtwalker]

/rebase

[lukedirtwalker]

Reviewed 5 of 5 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @matzf)

---

go/pkg/router/dataplane.go, line 80 at r1 (raw file):

	ReadBatch(underlayconn.Messages) (int, error)
	WriteTo([]byte, *net.UDPAddr) (int, error)
	WriteBatch(underlayconn.Messages) (int, error)

this no longer seems to be used, please remove and update the mock.

---

go/pkg/router/dataplane.go, line 487 at r1 (raw file):

, _

Please check the cast succeeds and if it doesn't the packet should be discarded. I think this address comes from user input so we can not simply assume that the cast works.

[matzf]

Reviewable status: 3 of 5 files reviewed, 2 unresolved discussions (waiting on @lukedirtwalker)

---

go/pkg/router/dataplane.go, line 80 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

this no longer seems to be used, please remove and update the mock.

Done. I've not renamed it although the name doesn't seem very apt now. I cannot think of anything that's really better, both "Conn" or "UnderlayConn" would probably be net negative in clarity. The doc string is technically still correct: 'supports "batch reads" and "writes".' 😉

---

go/pkg/router/dataplane.go, line 487 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

, _

Please check the cast succeeds and if it doesn't the packet should be discarded. I think this address comes from user input so we can not simply assume that the cast works.

I've checked the different code paths setting this, the address is either nil (uninitialized) or UDPAddr. I've changed this to use a panic-y type assertion to make this expectation explicit.

[lukedirtwalker]

Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matzf)

---

go/pkg/router/dataplane.go, line 487 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

I've checked the different code paths setting this, the address is either nil (uninitialized) or UDPAddr. I've changed this to use a panic-y type assertion to make this expectation explicit.

Hm then it would be cleaner to change the type of result.OutAddr, if that isn't too much work.

[matzf]

Reviewable status: 2 of 5 files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker)

---

go/pkg/router/dataplane.go, line 487 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

Hm then it would be cleaner to change the type of result.OutAddr, if that isn't too much work.

Done. This goes through quite a few places, now UDPAddr is now ubuquitous. I quite like that, though. I think it makes the previously hidden assumptions more clearly visible.

[lukedirtwalker]

Reviewed 3 of 3 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @matzf)