From dd975718153ebfc8e29a0ff1dbfa9bc8886888c6 Mon Sep 17 00:00:00 2001 From: Matthias Frei Date: Sat, 12 Mar 2022 12:53:08 +0100 Subject: [PATCH] slayers: recycle path when decoding In `slayers.SCION.DecodeFromBytes`, we allocate a new path for the correct path type on every invocation. This negates the advantages of recycling the layer object. Introduce a "recycling bin" for paths that can be reused in the SCION layer's DecodeFromBytes. This is only used when explicitly enabled, as it only makes sense if the layer is really reused for parsing. This opt-in also avoids issues with tests that deep-compare the SCION layer with an expected value where this recycling bin is empty.. Closes #4147 GitOrigin-RevId: cd7c81e4e062822bcc948eae4363c9544721f4d0 --- go/lib/slayers/path/path.go | 5 +++++ go/lib/slayers/scion.go | 33 ++++++++++++++++++++++++++++++++- go/pkg/router/dataplane.go | 4 +++- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/go/lib/slayers/path/path.go b/go/lib/slayers/path/path.go index 9ed91d1096..95e0dd59b3 100644 --- a/go/lib/slayers/path/path.go +++ b/go/lib/slayers/path/path.go @@ -104,6 +104,11 @@ func NewPath(pathType Type) (Path, error) { return pm.New(), nil } +// NewRawPath returns a new raw path that can hold any path type. +func NewRawPath() Path { + return &rawPath{} +} + type rawPath struct { raw []byte pathType Type diff --git a/go/lib/slayers/scion.go b/go/lib/slayers/scion.go index 226ab57667..d9d1ad40e1 100644 --- a/go/lib/slayers/scion.go +++ b/go/lib/slayers/scion.go @@ -145,6 +145,9 @@ type SCION struct { // Path is the path contained in the SCION header. It depends on the PathType field. Path path.Path + + pathPool []path.Path + pathPoolRaw path.Path } func (s *SCION) LayerType() gopacket.LayerType { @@ -244,7 +247,7 @@ func (s *SCION) DecodeFromBytes(data []byte, df gopacket.DecodeFeedback) error { return serrors.New("provided buffer is too small", "expected", minLen, "actual", len(data)) } - s.Path, err = path.NewPath(s.PathType) + s.Path, err = s.getPath(s.PathType) if err != nil { return err } @@ -259,6 +262,34 @@ func (s *SCION) DecodeFromBytes(data []byte, df gopacket.DecodeFeedback) error { return nil } +// RecyclePaths enables recycling of paths used for DecodeFromBytes. This is +// only useful if the layer itself is reused. +// When this is enabled, the Path instance may be overwritten in +// DecodeFromBytes. No references to Path should be kept in use between +// invocations of DecodeFromBytes. +func (s *SCION) RecyclePaths() { + if s.pathPool == nil { + s.pathPool = []path.Path{ + empty.PathType: empty.Path{}, + onehop.PathType: &onehop.Path{}, + scion.PathType: &scion.Raw{}, + epic.PathType: &epic.Path{}, + } + s.pathPoolRaw = path.NewRawPath() + } +} + +// getPath returns a new or recycled path for pathType +func (s *SCION) getPath(pathType path.Type) (path.Path, error) { + if s.pathPool == nil { + return path.NewPath(pathType) + } + if int(pathType) < len(s.pathPool) { + return s.pathPool[pathType], nil + } + return s.pathPoolRaw, nil +} + func decodeSCION(data []byte, pb gopacket.PacketBuilder) error { scn := &SCION{} err := scn.DecodeFromBytes(data, pb) diff --git a/go/pkg/router/dataplane.go b/go/pkg/router/dataplane.go index 98f85a1555..787f925c81 100644 --- a/go/pkg/router/dataplane.go +++ b/go/pkg/router/dataplane.go @@ -553,7 +553,7 @@ type processResult struct { } func newPacketProcessor(d *DataPlane, ingressID uint16) *scionPacketProcessor { - return &scionPacketProcessor{ + p := &scionPacketProcessor{ d: d, ingressID: ingressID, buffer: gopacket.NewSerializeBuffer(), @@ -563,6 +563,8 @@ func newPacketProcessor(d *DataPlane, ingressID uint16) *scionPacketProcessor { epicInput: make([]byte, libepic.MACBufferSize), }, } + p.scionLayer.RecyclePaths() + return p } func (p *scionPacketProcessor) reset() error {