Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API: Preserve UUID in path table #2644

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
API: Preserve UUID in path table
Changes the AddPath API to add the UUID generated to the path object.
Previously this UUID was only usable for deleting routes, and
would be dropped when inserted into the table. That meant then if the
same route was passed to a table event watcher, the UUID was set to nil,
despite being made by the API.

With this change, the UUID is preserved and will be passed to callers of
ListPath, or when passed to an event watcher. To implement this an
additional UUID field has been added onto the table path object, and the
AddPath, api2Path functions modified to preserve them. The Marshaller
was also updated to return this value to end users as well.

Additionally the DeletePath call has been simplified when deleting paths
by UUID. Since each path now stores the UUID they are checked directly
rather than using a lookup map.
dawn-minion authored and Dawn Minion committed Mar 31, 2023
commit ef4d0ea182f6feadffbc0af8ac7942b4fad9908c
5 changes: 5 additions & 0 deletions internal/pkg/table/path.go
Original file line number Diff line number Diff line change
@@ -24,6 +24,8 @@ import (
"sort"
"time"

"github.com/google/uuid"

"github.com/osrg/gobgp/v3/internal/pkg/config"
"github.com/osrg/gobgp/v3/pkg/log"
"github.com/osrg/gobgp/v3/pkg/packet/bgp"
@@ -141,6 +143,9 @@ type Path struct {
// For BGP Nexthop Tracking, this field shows if nexthop is invalidated by IGP.
IsNexthopInvalid bool
IsWithdraw bool

// Only exists for paths added via AddPath API
Uuid *uuid.UUID
}

var localSource = &PeerInfo{}
19 changes: 19 additions & 0 deletions pkg/server/grpc_server.go
Original file line number Diff line number Diff line change
@@ -33,6 +33,7 @@ import (
apb "google.golang.org/protobuf/types/known/anypb"
"google.golang.org/protobuf/types/known/emptypb"
tspb "google.golang.org/protobuf/types/known/timestamppb"
"github.com/google/uuid"

api "github.com/osrg/gobgp/v3/api"
"github.com/osrg/gobgp/v3/internal/pkg/config"
@@ -162,6 +163,12 @@ func newValidationFromTableStruct(v *table.Validation) *api.Validation {

func toPathAPI(binNlri []byte, binPattrs [][]byte, anyNlri *apb.Any, anyPattrs []*apb.Any, path *table.Path, v *table.Validation) *api.Path {
nlri := path.GetNlri()
var uuidBytes []byte = nil

if path.Uuid != nil {
uuidBytes, _ = path.Uuid.MarshalBinary()
}

p := &api.Path{
Nlri: anyNlri,
Pattrs: anyPattrs,
@@ -177,6 +184,7 @@ func toPathAPI(binNlri []byte, binPattrs [][]byte, anyNlri *apb.Any, anyPattrs [
LocalIdentifier: nlri.PathLocalIdentifier(),
NlriBinary: binNlri,
PattrsBinary: binPattrs,
Uuid: uuidBytes,
}
if s := path.GetSource(); s != nil {
p.SourceAsn = s.AS
@@ -370,6 +378,17 @@ func api2Path(resource api.TableType, path *api.Path, isWithdraw bool) (*table.P
newPath.SetHash(farm.Hash32(total.Bytes()))
}
newPath.SetIsFromExternal(path.IsFromExternal)

if path.Uuid != nil {
uuid, err := uuid.FromBytes(path.Uuid)

if err != nil {
return nil, err
}

newPath.Uuid = &uuid
}

return newPath, nil
}

25 changes: 8 additions & 17 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
@@ -174,7 +174,6 @@ type BgpServer struct {
bmpManager *bmpClientManager
mrtManager *mrtManager
roaTable *table.ROATable
uuidMap map[string]uuid.UUID
logger log.Logger
}

@@ -195,7 +194,6 @@ func NewBgpServer(opt ...ServerOption) *BgpServer {
policy: table.NewRoutingPolicy(logger),
mgmtCh: make(chan *mgmtOp, 1),
watcherMap: make(map[watchEventType][]*watcher),
uuidMap: make(map[string]uuid.UUID),
roaManager: newROAManager(roaTable, logger),
roaTable: roaTable,
logger: logger,
@@ -2131,6 +2129,7 @@ func (s *BgpServer) AddPath(ctx context.Context, r *api.AddPathRequest) (*api.Ad
var uuidBytes []byte
err := s.mgmtOperation(func() error {
id, err := uuid.NewRandom()

if err != nil {
return err
}
@@ -2139,13 +2138,15 @@ func (s *BgpServer) AddPath(ctx context.Context, r *api.AddPathRequest) (*api.Ad
if err != nil {
return err
}

path.Uuid = &id

err = s.addPathList(r.VrfId, []*table.Path{path})
if err != nil {
return err
}

s.uuidMap[pathTokey(path)] = id
uuidBytes, _ = id.MarshalBinary()
uuidBytes, _ = path.Uuid.MarshalBinary()
return nil
}, true)
return &api.AddPathResponse{Uuid: uuidBytes}, err
@@ -2172,15 +2173,9 @@ func (s *BgpServer) DeletePath(ctx context.Context, r *api.DeletePathRequest) er
if len(r.Uuid) > 0 {
// Delete locally generated path which has the given UUID
path := func() *table.Path {
id, _ := uuid.FromBytes(r.Uuid)
for k, v := range s.uuidMap {
if v == id {
for _, path := range s.globalRib.GetPathList(table.GLOBAL_RIB_NAME, 0, s.globalRib.GetRFlist()) {
if path.IsLocal() && k == pathTokey(path) {
delete(s.uuidMap, k)
return path
}
}
for _, path := range s.globalRib.GetPathList(table.GLOBAL_RIB_NAME, 0, s.globalRib.GetRFlist()) {
if path.Uuid != nil && bytes.Equal(path.Uuid[:], r.Uuid) {
return path
}
}
return nil
@@ -2201,15 +2196,11 @@ func (s *BgpServer) DeletePath(ctx context.Context, r *api.DeletePathRequest) er
deletePathList = append(deletePathList, path.Clone(true))
}
}
s.uuidMap = make(map[string]uuid.UUID)
} else {
if err := s.fixupApiPath(r.VrfId, pathList); err != nil {
return err
}
deletePathList = pathList
for _, p := range deletePathList {
delete(s.uuidMap, pathTokey(p))
}
}
s.propagateUpdate(nil, deletePathList)
return nil