From 31c0202b830960f99ac4b1c096f37a3cfb3b61cb Mon Sep 17 00:00:00 2001 From: Daniel Grau Date: Thu, 14 Sep 2023 22:06:57 +0000 Subject: [PATCH 1/3] route entry and router interface --- dataplane/internal/engine/engine.go | 2 +- dataplane/standalone/sai/common.cc | 7 +- dataplane/standalone/saiserver/routing.go | 80 +++++++++++++++++++++++ 3 files changed, 83 insertions(+), 6 deletions(-) diff --git a/dataplane/internal/engine/engine.go b/dataplane/internal/engine/engine.go index 8f778b7a..b2989ede 100644 --- a/dataplane/internal/engine/engine.go +++ b/dataplane/internal/engine/engine.go @@ -358,7 +358,7 @@ func prefixToPrimitives(prefix *dpb.RoutePrefix) ([]byte, []byte, bool, uint64, case net.IPv6len: isIPv4 = false default: - return ip, mask, isIPv4, vrf, fmt.Errorf("invalid ip addr length") + return ip, mask, isIPv4, vrf, fmt.Errorf("invalid ip addr length: ip %v, mask %v", ip, mask) } default: return ip, mask, isIPv4, vrf, fmt.Errorf("invalid prefix type") diff --git a/dataplane/standalone/sai/common.cc b/dataplane/standalone/sai/common.cc index e6bb5c9c..7fb67f81 100644 --- a/dataplane/standalone/sai/common.cc +++ b/dataplane/standalone/sai/common.cc @@ -25,12 +25,9 @@ std::string convert_from_ip_addr(sai_ip_addr_family_t addr_family, const sai_ip_addr_t& addr) { if (addr_family == SAI_IP_ADDR_FAMILY_IPV4) { - sai_ip4_t ip = addr.ip4; - return reinterpret_cast(&ip); + return std::string(&addr.ip4, &addr.ip4 + 4); } - sai_ip6_t ip; - std::copy(addr.ip6, addr.ip6 + sizeof(sai_ip6_t), ip); - return reinterpret_cast(ip); + return std::string(addr.ip6, addr.ip6 + 16); } std::string convert_from_ip_address(const sai_ip_address_t& val) { diff --git a/dataplane/standalone/saiserver/routing.go b/dataplane/standalone/saiserver/routing.go index 7c24ce35..2c2406d3 100644 --- a/dataplane/standalone/saiserver/routing.go +++ b/dataplane/standalone/saiserver/routing.go @@ -175,6 +175,60 @@ func newRoute(mgr *attrmgr.AttrMgr, dataplane routingDataplaneAPI, s *grpc.Serve return r } +// CreateRouteEntry creates a new route entry. +func (r *route) CreateRouteEntry(ctx context.Context, req *saipb.CreateRouteEntryRequest) (*saipb.CreateRouteEntryResponse, error) { + rReq := &dpb.AddIPRouteRequest{ + Route: &dpb.Route{ + Prefix: &dpb.RoutePrefix{ + Prefix: &dpb.RoutePrefix_Mask{ + Mask: &dpb.IpMask{ + Addr: req.GetEntry().GetDestination().GetAddr(), + Mask: req.GetEntry().GetDestination().GetMask(), + }, + }, + VrfId: req.GetEntry().GetVrId(), + }, + }, + } + + // TODO(dgrau): Implement CPU actions. + switch req.GetPacketAction() { + case saipb.PacketAction_PACKET_ACTION_DROP: + fallthrough + case saipb.PacketAction_PACKET_ACTION_TRAP: // COPY and DROP + fallthrough + case saipb.PacketAction_PACKET_ACTION_DENY: // COPY_CANCEL and DROP + rReq.Route.Action = dpb.PacketAction_PACKET_ACTION_DROP + case saipb.PacketAction_PACKET_ACTION_LOG: + fallthrough + case saipb.PacketAction_PACKET_ACTION_TRANSIT: + rReq.Route.Action = dpb.PacketAction_PACKET_ACTION_FORWARD + } + nextType := r.mgr.GetType(fmt.Sprint(req.GetNextHopId())) + + // If the packet action is drop, then next hop is optional. + if rReq.Route.Action == dpb.PacketAction_PACKET_ACTION_FORWARD { + switch nextType { + case saipb.ObjectType_OBJECT_TYPE_NEXT_HOP: + rReq.Route.Hop = &dpb.Route_NextHopId{NextHopId: req.GetNextHopId()} + case saipb.ObjectType_OBJECT_TYPE_NEXT_HOP_GROUP: + rReq.Route.Hop = &dpb.Route_NextHopGroupId{NextHopGroupId: req.GetNextHopId()} + case saipb.ObjectType_OBJECT_TYPE_ROUTER_INTERFACE: + rReq.Route.Hop = &dpb.Route_InterfaceId{InterfaceId: fmt.Sprint(req.GetNextHopId())} + case saipb.ObjectType_OBJECT_TYPE_PORT: + rReq.Route.Hop = &dpb.Route_PortId{PortId: fmt.Sprint(req.GetNextHopId())} + default: + return nil, status.Errorf(codes.InvalidArgument, "unknown next hop type: %v", nextType) + } + } + + _, err := r.dataplane.AddIPRoute(ctx, rReq) + if err != nil { + return nil, err + } + return nil, nil +} + type routerInterface struct { saipb.UnimplementedRouterInterfaceServer mgr *attrmgr.AttrMgr @@ -189,3 +243,29 @@ func newRouterInterface(mgr *attrmgr.AttrMgr, dataplane routingDataplaneAPI, s * saipb.RegisterRouterInterfaceServer(s, r) return r } + +// CreateRouterInterfaces creates a new router interface. +func (ri *routerInterface) CreateRouterInterface(ctx context.Context, req *saipb.CreateRouterInterfaceRequest) (*saipb.CreateRouterInterfaceResponse, error) { + id := ri.mgr.NextID() + iReq := &dpb.AddInterfaceRequest{ + Id: fmt.Sprint(id), + VrfId: uint32(req.GetVirtualRouterId()), + Mtu: uint64(req.GetMtu()), + PortIds: []string{fmt.Sprint(req.GetPortId())}, + Mac: req.GetSrcMacAddress(), + } + switch req.GetType() { + case saipb.RouterInterfaceType_ROUTER_INTERFACE_TYPE_PORT: + iReq.Type = dpb.InterfaceType_INTERFACE_TYPE_PORT + + case saipb.RouterInterfaceType_ROUTER_INTERFACE_TYPE_LOOPBACK: // TODO: Support loopback interfaces + return &saipb.CreateRouterInterfaceResponse{Oid: id}, nil + default: + return nil, status.Errorf(codes.InvalidArgument, "unknown next hop type: %v", req.GetType()) + } + if _, err := ri.dataplane.AddInterface(ctx, iReq); err != nil { + return nil, err + } + + return &saipb.CreateRouterInterfaceResponse{Oid: id}, nil +} From 26c17122d73b8512899b86173226f227c3ee65ee Mon Sep 17 00:00:00 2001 From: Daniel Grau Date: Thu, 14 Sep 2023 23:32:41 +0000 Subject: [PATCH 2/3] Impleme route and router interface --- dataplane/standalone/saiserver/routing.go | 6 +- .../standalone/saiserver/routing_test.go | 152 +++++++++++++++++- 2 files changed, 152 insertions(+), 6 deletions(-) diff --git a/dataplane/standalone/saiserver/routing.go b/dataplane/standalone/saiserver/routing.go index 2c2406d3..55957a68 100644 --- a/dataplane/standalone/saiserver/routing.go +++ b/dataplane/standalone/saiserver/routing.go @@ -203,6 +203,8 @@ func (r *route) CreateRouteEntry(ctx context.Context, req *saipb.CreateRouteEntr fallthrough case saipb.PacketAction_PACKET_ACTION_TRANSIT: rReq.Route.Action = dpb.PacketAction_PACKET_ACTION_FORWARD + default: + return nil, status.Errorf(codes.InvalidArgument, "unknown action type: %v", req.GetPacketAction()) } nextType := r.mgr.GetType(fmt.Sprint(req.GetNextHopId())) @@ -226,7 +228,7 @@ func (r *route) CreateRouteEntry(ctx context.Context, req *saipb.CreateRouteEntr if err != nil { return nil, err } - return nil, nil + return &saipb.CreateRouteEntryResponse{}, nil } type routerInterface struct { @@ -261,7 +263,7 @@ func (ri *routerInterface) CreateRouterInterface(ctx context.Context, req *saipb case saipb.RouterInterfaceType_ROUTER_INTERFACE_TYPE_LOOPBACK: // TODO: Support loopback interfaces return &saipb.CreateRouterInterfaceResponse{Oid: id}, nil default: - return nil, status.Errorf(codes.InvalidArgument, "unknown next hop type: %v", req.GetType()) + return nil, status.Errorf(codes.InvalidArgument, "unknown interface type: %v", req.GetType()) } if _, err := ri.dataplane.AddInterface(ctx, iReq); err != nil { return nil, err diff --git a/dataplane/standalone/saiserver/routing_test.go b/dataplane/standalone/saiserver/routing_test.go index 526a8a3c..ae7d44f5 100644 --- a/dataplane/standalone/saiserver/routing_test.go +++ b/dataplane/standalone/saiserver/routing_test.go @@ -34,6 +34,8 @@ type fakeRoutingDataplaneAPI struct { gotAddNeighborReq []*dpb.AddNeighborRequest gotAddNextHopGroupReq []*dpb.AddNextHopGroupRequest gotAddNextHopReq []*dpb.AddNextHopRequest + gotAddIPRouteReq []*dpb.AddIPRouteRequest + gotAddInterfaceReq []*dpb.AddInterfaceRequest } func (f *fakeRoutingDataplaneAPI) AddNeighbor(_ context.Context, req *dpb.AddNeighborRequest) (*dpb.AddNeighborResponse, error) { @@ -51,12 +53,14 @@ func (f *fakeRoutingDataplaneAPI) AddNextHop(_ context.Context, req *dpb.AddNext return nil, nil } -func (f *fakeRoutingDataplaneAPI) AddIPRoute(context.Context, *dpb.AddIPRouteRequest) (*dpb.AddIPRouteResponse, error) { - panic("not implemented") // TODO: Implement +func (f *fakeRoutingDataplaneAPI) AddIPRoute(_ context.Context, req *dpb.AddIPRouteRequest) (*dpb.AddIPRouteResponse, error) { + f.gotAddIPRouteReq = append(f.gotAddIPRouteReq, req) + return nil, nil } -func (f *fakeRoutingDataplaneAPI) AddInterface(context.Context, *dpb.AddInterfaceRequest) (*dpb.AddInterfaceResponse, error) { - panic("not implemented") // TODO: Implement +func (f *fakeRoutingDataplaneAPI) AddInterface(_ context.Context, req *dpb.AddInterfaceRequest) (*dpb.AddInterfaceResponse, error) { + f.gotAddInterfaceReq = append(f.gotAddInterfaceReq, req) + return nil, nil } func TestCreateNeighborEntry(t *testing.T) { @@ -262,6 +266,132 @@ func TestCreateNextHop(t *testing.T) { } } +func TestCreateRouteEntry(t *testing.T) { + tests := []struct { + desc string + req *saipb.CreateRouteEntryRequest + wantReq *dpb.AddIPRouteRequest + types map[string]saipb.ObjectType + wantErr string + }{{ + desc: "unknown action", + req: &saipb.CreateRouteEntryRequest{}, + wantErr: "InvalidArgument", + }, { + desc: "drop action", + req: &saipb.CreateRouteEntryRequest{ + Entry: &saipb.RouteEntry{ + Destination: &saipb.IpPrefix{ + Addr: []byte{127, 0, 0, 1}, + Mask: []byte{255, 255, 255, 255}, + }, + }, + PacketAction: saipb.PacketAction_PACKET_ACTION_DROP.Enum(), + }, + wantReq: &dpb.AddIPRouteRequest{ + Route: &dpb.Route{ + Prefix: &dpb.RoutePrefix{ + Prefix: &dpb.RoutePrefix_Mask{ + Mask: &dpb.IpMask{ + Addr: []byte{127, 0, 0, 1}, + Mask: []byte{255, 255, 255, 255}, + }, + }, + }, + Action: dpb.PacketAction_PACKET_ACTION_DROP, + }, + }, + }, { + desc: "forward port action", + types: map[string]saipb.ObjectType{"100": saipb.ObjectType_OBJECT_TYPE_PORT}, + req: &saipb.CreateRouteEntryRequest{ + Entry: &saipb.RouteEntry{ + Destination: &saipb.IpPrefix{ + Addr: []byte{127, 0, 0, 1}, + Mask: []byte{255, 255, 255, 255}, + }, + }, + PacketAction: saipb.PacketAction_PACKET_ACTION_TRANSIT.Enum(), + NextHopId: proto.Uint64(100), + }, + wantReq: &dpb.AddIPRouteRequest{ + Route: &dpb.Route{ + Prefix: &dpb.RoutePrefix{ + Prefix: &dpb.RoutePrefix_Mask{ + Mask: &dpb.IpMask{ + Addr: []byte{127, 0, 0, 1}, + Mask: []byte{255, 255, 255, 255}, + }, + }, + }, + Hop: &dpb.Route_PortId{PortId: "100"}, + Action: dpb.PacketAction_PACKET_ACTION_FORWARD, + }, + }, + }} + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + dplane := &fakeRoutingDataplaneAPI{} + c, mgr, stopFn := newTestRoute(t, dplane) + defer stopFn() + for k, v := range tt.types { + mgr.SetType(k, v) + } + _, gotErr := c.CreateRouteEntry(context.TODO(), tt.req) + if diff := errdiff.Check(gotErr, tt.wantErr); diff != "" { + t.Fatalf("CreateRouteEntry() unexpected err: %s", diff) + } + if gotErr != nil { + return + } + if d := cmp.Diff(dplane.gotAddIPRouteReq[0], tt.wantReq, protocmp.Transform()); d != "" { + t.Errorf("CreateRouteEntry() failed: diff(-got,+want)\n:%s", d) + } + }) + } +} + +func TestCreateRouterInterface(t *testing.T) { + tests := []struct { + desc string + req *saipb.CreateRouterInterfaceRequest + wantReq *dpb.AddInterfaceRequest + wantErr string + }{{ + desc: "unknown type", + req: &saipb.CreateRouterInterfaceRequest{}, + wantErr: "InvalidArgument", + }, { + desc: "success port", + req: &saipb.CreateRouterInterfaceRequest{ + PortId: proto.Uint64(10), + Type: saipb.RouterInterfaceType_ROUTER_INTERFACE_TYPE_PORT.Enum(), + }, + wantReq: &dpb.AddInterfaceRequest{ + Id: "1", + PortIds: []string{"10"}, + Type: dpb.InterfaceType_INTERFACE_TYPE_PORT, + }, + }} + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + dplane := &fakeRoutingDataplaneAPI{} + c, _, stopFn := newTestRouterInterface(t, dplane) + defer stopFn() + _, gotErr := c.CreateRouterInterface(context.TODO(), tt.req) + if diff := errdiff.Check(gotErr, tt.wantErr); diff != "" { + t.Fatalf("CreateRouterInterface() unexpected err: %s", diff) + } + if gotErr != nil { + return + } + if d := cmp.Diff(dplane.gotAddInterfaceReq[0], tt.wantReq, protocmp.Transform()); d != "" { + t.Errorf("CreateRouterInterface() failed: diff(-got,+want)\n:%s", d) + } + }) + } +} + func newTestNeighbor(t testing.TB, api routingDataplaneAPI) (saipb.NeighborClient, *attrmgr.AttrMgr, func()) { conn, mgr, stopFn := newTestServer(t, func(mgr *attrmgr.AttrMgr, srv *grpc.Server) { newNeighbor(mgr, api, srv) @@ -282,3 +412,17 @@ func newTestNextHop(t testing.TB, api routingDataplaneAPI) (saipb.NextHopClient, }) return saipb.NewNextHopClient(conn), mgr, stopFn } + +func newTestRoute(t testing.TB, api routingDataplaneAPI) (saipb.RouteClient, *attrmgr.AttrMgr, func()) { + conn, mgr, stopFn := newTestServer(t, func(mgr *attrmgr.AttrMgr, srv *grpc.Server) { + newRoute(mgr, api, srv) + }) + return saipb.NewRouteClient(conn), mgr, stopFn +} + +func newTestRouterInterface(t testing.TB, api routingDataplaneAPI) (saipb.RouterInterfaceClient, *attrmgr.AttrMgr, func()) { + conn, mgr, stopFn := newTestServer(t, func(mgr *attrmgr.AttrMgr, srv *grpc.Server) { + newRouterInterface(mgr, api, srv) + }) + return saipb.NewRouterInterfaceClient(conn), mgr, stopFn +} From 09111058989d8119d195e9b668ff115b9fbd7ede Mon Sep 17 00:00:00 2001 From: Daniel Grau Date: Fri, 15 Sep 2023 20:10:41 +0000 Subject: [PATCH 3/3] feedback --- dataplane/standalone/saiserver/routing.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/dataplane/standalone/saiserver/routing.go b/dataplane/standalone/saiserver/routing.go index 55957a68..05ee0119 100644 --- a/dataplane/standalone/saiserver/routing.go +++ b/dataplane/standalone/saiserver/routing.go @@ -24,6 +24,8 @@ import ( "github.com/openconfig/lemming/dataplane/standalone/saiserver/attrmgr" + log "github.com/golang/glog" + saipb "github.com/openconfig/lemming/dataplane/standalone/proto" dpb "github.com/openconfig/lemming/proto/dataplane" ) @@ -193,15 +195,13 @@ func (r *route) CreateRouteEntry(ctx context.Context, req *saipb.CreateRouteEntr // TODO(dgrau): Implement CPU actions. switch req.GetPacketAction() { - case saipb.PacketAction_PACKET_ACTION_DROP: - fallthrough - case saipb.PacketAction_PACKET_ACTION_TRAP: // COPY and DROP - fallthrough - case saipb.PacketAction_PACKET_ACTION_DENY: // COPY_CANCEL and DROP + case saipb.PacketAction_PACKET_ACTION_DROP, + saipb.PacketAction_PACKET_ACTION_TRAP, // COPY and DROP + saipb.PacketAction_PACKET_ACTION_DENY: // COPY_CANCEL and DROP rReq.Route.Action = dpb.PacketAction_PACKET_ACTION_DROP - case saipb.PacketAction_PACKET_ACTION_LOG: - fallthrough - case saipb.PacketAction_PACKET_ACTION_TRANSIT: + case saipb.PacketAction_PACKET_ACTION_FORWARD, + saipb.PacketAction_PACKET_ACTION_LOG, // COPY and FORWARD + saipb.PacketAction_PACKET_ACTION_TRANSIT: // COPY_CANCEL and FORWARD rReq.Route.Action = dpb.PacketAction_PACKET_ACTION_FORWARD default: return nil, status.Errorf(codes.InvalidArgument, "unknown action type: %v", req.GetPacketAction()) @@ -261,6 +261,7 @@ func (ri *routerInterface) CreateRouterInterface(ctx context.Context, req *saipb iReq.Type = dpb.InterfaceType_INTERFACE_TYPE_PORT case saipb.RouterInterfaceType_ROUTER_INTERFACE_TYPE_LOOPBACK: // TODO: Support loopback interfaces + log.Warning("loopback interfaces not supported") return &saipb.CreateRouterInterfaceResponse{Oid: id}, nil default: return nil, status.Errorf(codes.InvalidArgument, "unknown interface type: %v", req.GetType())