Skip to content

Commit

Permalink
Merge pull request #103 from floatingstatic/structoptimize
Browse files Browse the repository at this point in the history
Optimize VRP struct sizes. Use netip.Prefix instead of net.IPNet.
  • Loading branch information
job authored Dec 21, 2023
2 parents 925a672 + 236c4bf commit 3289b5d
Show file tree
Hide file tree
Showing 10 changed files with 307 additions and 186 deletions.
31 changes: 15 additions & 16 deletions cmd/stayrtr/stayrtr.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"fmt"
"net"
"net/http"
"net/netip"
"os"
"os/signal"
"sort"
Expand All @@ -27,6 +28,7 @@ import (
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promhttp"
log "github.com/sirupsen/logrus"
"go4.org/netipx"
"golang.org/x/crypto/ssh"
)

Expand Down Expand Up @@ -182,9 +184,8 @@ func decodeJSON(data []byte) (*prefixfile.VRPList, error) {
return &vrplistjson, err
}

func isValidPrefixLength(prefix *net.IPNet, maxLength uint8) bool {
plen, max := net.IPMask.Size(prefix.Mask)

func isValidPrefixLength(prefix netip.Prefix, maxLength uint8) bool {
plen, max := net.IPMask.Size(netipx.PrefixIPNet(prefix).Mask)
if plen == 0 || uint8(plen) > maxLength || maxLength > uint8(max) {
log.Errorf("%s Maxlength wrong: %d - %d", prefix, plen, maxLength)
return false
Expand All @@ -201,8 +202,7 @@ func isValidPrefixLength(prefix *net.IPNet, maxLength uint8) bool {
func processData(vrplistjson []prefixfile.VRPJson,
brklistjson []prefixfile.BgpSecKeyJson,
aspajson *prefixfile.ProviderAuthorizationsJson) /*Export*/ ([]rtr.VRP, []rtr.BgpsecKey, []rtr.VAP, int, int, int) {
//
filterDuplicates := make(map[string]bool)
filterDuplicates := make(map[string]struct{})

// It may be tempting to change this to a simple time.Since() but that will
// grab the current time every time it's invoked, time calls can be slow on
Expand Down Expand Up @@ -240,7 +240,7 @@ func processData(vrplistjson []prefixfile.VRPJson,
}
}

if prefix.IP.To4() != nil {
if prefix.Addr().Is4() {
countv4++
} else {
countv6++
Expand All @@ -251,10 +251,10 @@ func processData(vrplistjson []prefixfile.VRPJson,
if exists {
continue
}
filterDuplicates[key] = true
filterDuplicates[key] = struct{}{}

vrp := rtr.VRP{
Prefix: *prefix,
Prefix: prefix,
ASN: asn,
MaxLen: v.Length,
}
Expand All @@ -281,15 +281,14 @@ func processData(vrplistjson []prefixfile.VRPJson,
P1 is likely to mark a BGP prefix P1 invalid. Therefore, the cache SHOULD announce the
sub-prefix P1 before the covering prefix P0.
*/
CIDRSizei, _ := vrplist[i].Prefix.Mask.Size()
CIDRSizej, _ := vrplist[j].Prefix.Mask.Size()
if CIDRSizei == CIDRSizej {

if vrplist[i].Prefix.Bits() == vrplist[j].Prefix.Bits() {
if vrplist[i].MaxLen != vrplist[j].MaxLen {
return vrplist[i].MaxLen > vrplist[j].MaxLen
}
return bytes.Compare(vrplist[i].Prefix.IP, vrplist[j].Prefix.IP) < 1
return vrplist[i].Prefix.Addr().Compare(vrplist[j].Prefix.Addr()) < 1
} else {
return CIDRSizei > CIDRSizej
return vrplist[i].Prefix.Bits() > vrplist[j].Prefix.Bits()
}
})

Expand Down Expand Up @@ -452,7 +451,7 @@ func (s *state) applyUpdateFromNewState(vrps []rtr.VRP, brks []rtr.BgpsecKey, va
vrpsjson []prefixfile.VRPJson, brksjson []prefixfile.BgpSecKeyJson, aspajson *prefixfile.ProviderAuthorizationsJson,
countv4 int, countv6 int) error {

SDs := make([]rtr.SendableData, 0)
SDs := make([]rtr.SendableData, 0, len(vrps)+len(brks)+len(vaps))
for _, v := range vrps {
SDs = append(SDs, v.Copy())
}
Expand Down Expand Up @@ -488,9 +487,9 @@ func (s *state) applyUpdateFromNewState(vrps []rtr.VRP, brks []rtr.BgpsecKey, va
var countv4_dup int
var countv6_dup int
for _, vrp := range vrps {
if vrp.Prefix.IP.To4() != nil {
if vrp.Prefix.Addr().Is4() {
countv4_dup++
} else if vrp.Prefix.IP.To16() != nil {
} else if vrp.Prefix.Addr().Is6() {
countv6_dup++
}
}
Expand Down
25 changes: 10 additions & 15 deletions cmd/stayrtr/stayrtr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package main

import (
"fmt"
"net"
"net/netip"
"os"
"testing"
"time"
Expand Down Expand Up @@ -103,17 +103,17 @@ func TestProcessData(t *testing.T) {
got, _, _, count, v4count, v6count := processData(stuff, nil, nil)
want := []rtr.VRP{
{
Prefix: mustParseIPNet("2001:db8::/32"),
Prefix: netip.MustParsePrefix("2001:db8::/32"),
MaxLen: 33,
ASN: 123,
},
{
Prefix: mustParseIPNet("192.168.1.0/24"),
Prefix: netip.MustParsePrefix("192.168.1.0/24"),
MaxLen: 25,
ASN: 123,
},
{
Prefix: mustParseIPNet("192.168.0.0/24"),
Prefix: netip.MustParsePrefix("192.168.0.0/24"),
MaxLen: 24,
ASN: 123,
},
Expand All @@ -122,20 +122,15 @@ func TestProcessData(t *testing.T) {
t.Errorf("Wanted count = 3, v4count = 2, v6count = 1, but got %d, %d, %d", count, v4count, v6count)
}

if !cmp.Equal(got, want) {
t.Errorf("Want (%+v), Got (%+v)", want, got)
opts := []cmp.Option{
cmp.Comparer(func(x, y netip.Prefix) bool {
return x == y
}),
}
}

// mustParseIPNet is a test helper function to return a net.IPNet
// This should only be called in test code, and it'll panic on test set up
// if unable to parse.
func mustParseIPNet(prefix string) net.IPNet {
_, ipnet, err := net.ParseCIDR(prefix)
if err != nil {
panic(err)
if !cmp.Equal(got, want, opts...) {
t.Errorf("Want (%+v), Got (%+v)", want, got)
}
return *ipnet
}

func BenchmarkDecodeJSON(b *testing.B) {
Expand Down
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
module github.com/bgp/stayrtr

go 1.17
go 1.21

require (
github.com/google/go-cmp v0.5.6
github.com/prometheus/client_golang v1.11.1
github.com/sirupsen/logrus v1.8.1
github.com/stretchr/testify v1.4.0
go4.org/netipx v0.0.0-20231129151722-fdeea329fbba
golang.org/x/crypto v0.6.0
golang.org/x/sys v0.15.0
)
Expand Down
Loading

0 comments on commit 3289b5d

Please sign in to comment.