Skip to content

Commit

Permalink
Revert "Add EOR type to Table's filter on Watch method"
Browse files Browse the repository at this point in the history
This reverts commit aff055b.

This breaks WatchEvent API:
#2777
  • Loading branch information
fujita committed Mar 5, 2024
1 parent 84a264e commit 9d05544
Show file tree
Hide file tree
Showing 9 changed files with 1,713 additions and 1,936 deletions.
2 changes: 1 addition & 1 deletion api/attribute.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion api/capability.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3,418 changes: 1,707 additions & 1,711 deletions api/gobgp.pb.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion api/gobgp.proto
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ message WatchEventRequest {

message Table {
message Filter {
enum Type { BEST = 0; ADJIN = 1; POST_POLICY = 2; EOR = 3; }
enum Type { BEST = 0; ADJIN = 1; POST_POLICY = 2; }
Type type = 1;
bool init = 2;
string peer_address = 3;
Expand Down
4 changes: 0 additions & 4 deletions api/gobgp_grpc.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 0 additions & 6 deletions internal/pkg/table/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -1245,12 +1245,6 @@ func (p *Path) GetHash() uint32 {
return p.attrsHash
}

func (p *Path) SetSource(peerInfo *PeerInfo) {
if p.info != nil {
p.info.source = peerInfo
}
}

func nlriToIPNet(nlri bgp.AddrPrefixInterface) *net.IPNet {
switch T := nlri.(type) {
case *bgp.IPAddrPrefix:
Expand Down
15 changes: 0 additions & 15 deletions pkg/server/grpc_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,21 +192,6 @@ func toPathAPI(binNlri []byte, binPattrs [][]byte, anyNlri *apb.Any, anyPattrs [
return p
}

func eorToPathAPI(path *table.Path) *api.Path {
nlri := path.GetNlri()
p := &api.Path{
Age: tspb.New(path.GetTimestamp()),
IsWithdraw: path.IsWithdraw,
Family: &api.Family{Afi: api.Family_Afi(nlri.AFI()), Safi: api.Family_Safi(nlri.SAFI())},
}
if s := path.GetSource(); s != nil {
p.SourceAsn = s.AS
p.SourceId = s.ID.String()
p.NeighborIp = s.Address.String()
}
return p
}

func toPathApi(path *table.Path, v *table.Validation, onlyBinary, nlriBinary, attributeBinary bool) *api.Path {
var (
anyNlri *apb.Any
Expand Down
119 changes: 0 additions & 119 deletions pkg/server/grpc_server_test.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,9 @@
package server

import (
"net"
"testing"
"time"

api "github.com/osrg/gobgp/v3/api"
"github.com/osrg/gobgp/v3/internal/pkg/table"
"github.com/osrg/gobgp/v3/pkg/apiutil"
"github.com/osrg/gobgp/v3/pkg/packet/bgp"
"github.com/stretchr/testify/assert"
"google.golang.org/protobuf/types/known/anypb"
)

func TestParseHost(t *testing.T) {
Expand Down Expand Up @@ -48,115 +41,3 @@ func TestParseHost(t *testing.T) {
})
}
}

func TestToPathApi(t *testing.T) {
type args struct {
path *table.Path
v *table.Validation
onlyBinary bool
nlriBinary bool
attributeBinary bool
}
tests := []struct {
name string
args args
want *api.Path
}{
{
name: "ipv4 path",
args: args{
path: table.NewPath(&table.PeerInfo{
ID: net.IP{10, 10, 10, 10},
LocalID: net.IP{10, 11, 11, 11},
Address: net.IP{10, 12, 12, 12},
LocalAddress: net.IP{10, 13, 13, 13},
},
bgp.NewIPAddrPrefix(8, "10.0.0.0"),
false,
[]bgp.PathAttributeInterface{bgp.NewPathAttributeOrigin(0)},
time.Time{},
false),
},
want: &api.Path{
Nlri: anyNlri(bgp.NewIPAddrPrefix(8, "10.0.0.0")),
Pattrs: anyAttrs([]bgp.PathAttributeInterface{bgp.NewPathAttributeOrigin(0)}),
Family: &api.Family{
Afi: api.Family_AFI_IP,
Safi: api.Family_SAFI_UNICAST,
},
Validation: &api.Validation{},
NeighborIp: "10.12.12.12",
SourceId: "10.10.10.10",
},
},
{
name: "eor ipv4 path",
args: args{
path: eor(bgp.RF_IPv4_UC),
},
want: &api.Path{
Nlri: anyEorNlri(bgp.AFI_IP, bgp.SAFI_UNICAST),
Family: &api.Family{
Afi: api.Family_AFI_IP,
Safi: api.Family_SAFI_UNICAST,
},
Pattrs: []*anypb.Any{},
Validation: &api.Validation{},
NeighborIp: "10.12.12.12",
SourceId: "10.10.10.10",
},
},
{
name: "eor vpn path",
args: args{
path: eor(bgp.RF_IPv4_VPN),
},
want: &api.Path{
Nlri: anyEorNlri(bgp.AFI_IP, bgp.SAFI_MPLS_VPN),
Family: &api.Family{
Afi: api.Family_AFI_IP,
Safi: api.Family_SAFI_MPLS_VPN,
},
Pattrs: []*anypb.Any{},
Validation: &api.Validation{},
NeighborIp: "10.12.12.12",
SourceId: "10.10.10.10",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
apiPath := toPathApi(tt.args.path, tt.args.v, tt.args.onlyBinary, tt.args.nlriBinary, tt.args.attributeBinary)
assert.Equal(t, tt.want.Nlri, apiPath.Nlri, "not equal nlri")
assert.Equal(t, tt.want.Pattrs, apiPath.Pattrs, "not equal attrs")
assert.Equal(t, tt.want.Family, apiPath.Family, "not equal family")
assert.Equal(t, tt.want.NeighborIp, apiPath.NeighborIp, "not equal neighbor")
})
}
}

func eor(f bgp.RouteFamily) *table.Path {
p := table.NewEOR(f)
p.SetSource(&table.PeerInfo{
ID: net.IP{10, 10, 10, 10},
LocalID: net.IP{10, 11, 11, 11},
Address: net.IP{10, 12, 12, 12},
LocalAddress: net.IP{10, 13, 13, 13},
})
return p
}

func anyEorNlri(afi uint16, safi uint8) *anypb.Any {
n, _ := bgp.NewPrefixFromRouteFamily(afi, safi)
return anyNlri(n)
}

func anyNlri(nlri bgp.AddrPrefixInterface) *anypb.Any {
anyNlri, _ := apiutil.MarshalNLRI(nlri)
return anyNlri
}

func anyAttrs(attrs []bgp.PathAttributeInterface) []*anypb.Any {
anyPattrs, _ := apiutil.MarshalPathAttributes(attrs)
return anyPattrs
}
81 changes: 3 additions & 78 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1700,21 +1700,6 @@ func (s *BgpServer) handleFSMMessage(peer *peer, e *fsmMsg) {
if f == bgp.RF_RTC_UC {
rtc = true
}
peer.fsm.lock.RLock()
peerInfo := &table.PeerInfo{
AS: peer.fsm.peerInfo.AS,
ID: peer.fsm.peerInfo.ID,
LocalAS: peer.fsm.peerInfo.LocalAS,
LocalID: peer.fsm.peerInfo.LocalID,
Address: peer.fsm.peerInfo.Address,
LocalAddress: peer.fsm.peerInfo.LocalAddress,
}
peer.fsm.lock.RUnlock()
ev := &watchEventEor{
Family: f,
PeerInfo: peerInfo,
}
s.notifyWatcher(watchEventTypeEor, ev)
for i, a := range peerAfiSafis {
if a.State.Family == f {
peer.fsm.lock.Lock()
Expand Down Expand Up @@ -4139,8 +4124,6 @@ func (s *BgpServer) WatchEvent(ctx context.Context, r *api.WatchEventRequest, fn
opts = append(opts, watchUpdate(filter.Init, filter.PeerAddress, filter.PeerGroup))
case api.WatchEventRequest_Table_Filter_POST_POLICY:
opts = append(opts, watchPostUpdate(filter.Init, filter.PeerAddress, filter.PeerGroup))
case api.WatchEventRequest_Table_Filter_EOR:
opts = append(opts, watchEor(filter.Init))
}
}
}
Expand All @@ -4161,7 +4144,7 @@ func (s *BgpServer) WatchEvent(ctx context.Context, r *api.WatchEventRequest, fn
case *watchEventUpdate:
paths := make([]*api.Path, 0)
for _, path := range msg.PathList {
paths = append(paths, toPathApi(path, nil, true, false, false))
paths = append(paths, toPathApi(path, nil, false, false, false))
}

fn(&api.WatchEventResponse{
Expand All @@ -4179,11 +4162,11 @@ func (s *BgpServer) WatchEvent(ctx context.Context, r *api.WatchEventRequest, fn
l = append(l, p...)
}
for _, p := range l {
pl = append(pl, toPathApi(p, nil, true, false, false))
pl = append(pl, toPathApi(p, nil, false, false, false))
}
} else {
for _, p := range msg.PathList {
pl = append(pl, toPathApi(p, nil, true, false, false))
pl = append(pl, toPathApi(p, nil, false, false, false))
}
}
fn(&api.WatchEventResponse{
Expand All @@ -4193,19 +4176,6 @@ func (s *BgpServer) WatchEvent(ctx context.Context, r *api.WatchEventRequest, fn
},
},
})
case *watchEventEor:
eor := table.NewEOR(msg.Family)
eor.SetSource(msg.PeerInfo)
path := eorToPathAPI(eor)

fn(&api.WatchEventResponse{
Event: &api.WatchEventResponse_Table{
Table: &api.WatchEventResponse_TableEvent{
Paths: []*api.Path{path},
},
},
})

case *watchEventPeer:
fn(&api.WatchEventResponse{
Event: &api.WatchEventResponse_Peer{
Expand Down Expand Up @@ -4276,7 +4246,6 @@ const (
watchEventTypePeerState watchEventType = "peerstate"
watchEventTypeTable watchEventType = "table"
watchEventTypeRecvMsg watchEventType = "receivedmessage"
watchEventTypeEor watchEventType = "eor"
)

type watchEvent interface {
Expand Down Expand Up @@ -4354,11 +4323,6 @@ type watchEventMessage struct {
IsSent bool
}

type watchEventEor struct {
Family bgp.RouteFamily
PeerInfo *table.PeerInfo
}

type watchOptions struct {
bestPath bool
preUpdate bool
Expand All @@ -4372,8 +4336,6 @@ type watchOptions struct {
initPostUpdate bool
tableName string
recvMessage bool
initEor bool
eor bool
}

type watchOption func(*watchOptions)
Expand Down Expand Up @@ -4435,15 +4397,6 @@ func watchPostUpdate(current bool, peerAddress string, peerGroup string) watchOp
}
}

func watchEor(current bool) watchOption {
return func(o *watchOptions) {
o.eor = true
if current {
o.initEor = true
}
}
}

func watchPeer() watchOption {
return func(o *watchOptions) {
o.peerState = true
Expand Down Expand Up @@ -4615,9 +4568,6 @@ func (s *BgpServer) watch(opts ...watchOption) (w *watcher) {
}
register(watchEventTypePostUpdate, w)
}
if w.opts.eor {
register(watchEventTypeEor, w)
}
if w.opts.peerState {
for _, p := range s.neighborMap {
w.notify(newWatchEventPeer(p, nil, p.fsm.state, PEER_EVENT_INIT))
Expand All @@ -4633,31 +4583,6 @@ func (s *BgpServer) watch(opts ...watchOption) (w *watcher) {
MultiPathList: s.globalRib.GetBestMultiPathList(table.GLOBAL_RIB_NAME, nil),
})
}
if w.opts.initEor && s.active() == nil {
for _, p := range s.neighborMap {
func() {
p.fsm.lock.RLock()
defer p.fsm.lock.RUnlock()
for _, a := range p.fsm.pConf.AfiSafis {
if s := a.MpGracefulRestart.State; s.Enabled && s.EndOfRibReceived {
family := a.State.Family
peerInfo := &table.PeerInfo{
AS: p.fsm.peerInfo.AS,
ID: p.fsm.peerInfo.ID,
LocalAS: p.fsm.peerInfo.LocalAS,
LocalID: p.fsm.peerInfo.LocalID,
Address: p.fsm.peerInfo.Address,
LocalAddress: p.fsm.peerInfo.LocalAddress,
}
w.notify(&watchEventEor{
Family: family,
PeerInfo: peerInfo,
})
}
}
}()
}
}
if w.opts.initUpdate {
for _, peer := range s.neighborMap {
peer.fsm.lock.RLock()
Expand Down

0 comments on commit 9d05544

Please sign in to comment.