gobgp: fix client.AddPath to use api.Path instead of table.Path

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

8 files changed, 85 insertions, 70 deletions

diff --git a/api/grpc_server.go b/api/grpc_server.go
index 3fa65ee5..f6f5913f 100644
--- a/api/grpc_server.go
+++ b/api/grpc_server.go
@@ -473,23 +473,6 @@ func toPathAPI(binNlri []byte, binPattrs [][]byte, anyNlri *any.Any, anyPattrs [
 	return p
 }
 
-func ToPathApiInBin(path *table.Path, v *table.Validation) *Path {
-	nlri := path.GetNlri()
-	binNlri, _ := nlri.Serialize()
-	if path.IsWithdraw {
-		return toPathAPI(binNlri, nil, nil, nil, path, v)
-	}
-	binPattrs := func(attrs []bgp.PathAttributeInterface) [][]byte {
-		bufList := make([][]byte, 0, len(attrs))
-		for _, a := range attrs {
-			buf, _ := a.Serialize()
-			bufList = append(bufList, buf)
-		}
-		return bufList
-	}(path.GetPathAttrs())
-	return toPathAPI(binNlri, binPattrs, nil, nil, path, v)
-}
-
 func ToPathApi(path *table.Path, v *table.Validation) *Path {
 	nlri := path.GetNlri()
 	anyNlri := MarshalNLRI(nlri)
diff --git a/api/util.go b/api/util.go
index 721909c5..9b7a389e 100644
--- a/api/util.go
+++ b/api/util.go
@@ -18,6 +18,7 @@ package gobgpapi
 import (
 	"encoding/json"
 	"net"
+	"time"
 
 	"github.com/osrg/gobgp/config"
 	"github.com/osrg/gobgp/packet/bgp"
@@ -39,9 +40,27 @@ func (d *Destination) MarshalJSON() ([]byte, error) {
 	return json.Marshal(d.Paths)
 }
 
+func NewPath(nlri bgp.AddrPrefixInterface, isWithdraw bool, attrs []bgp.PathAttributeInterface, age time.Time) *Path {
+	return &Path{
+		AnyNlri:    MarshalNLRI(nlri),
+		AnyPattrs:  MarshalPathAttributes(attrs),
+		Age:        age.Unix(),
+		IsWithdraw: isWithdraw,
+		Family:     uint32(bgp.AfiSafiToRouteFamily(nlri.AFI(), nlri.SAFI())),
+		Identifier: nlri.PathIdentifier(),
+	}
+}
+
 func (p *Path) MarshalJSON() ([]byte, error) {
-	nlri, _ := p.GetNativeNlri()
-	attrs, _ := p.GetNativePathAttributes()
+	nlri, err := p.GetNativeNlri()
+	if err != nil {
+		return nil, err
+	}
+	attrs, err := p.GetNativePathAttributes()
+	if err != nil {
+		return nil, err
+	}
+
 	return json.Marshal(struct {
 		Nlri       bgp.AddrPrefixInterface      `json:"nlri"`
 		Age        int64                        `json:"age"`
diff --git a/client/client.go b/client/client.go
index 04f148c0..c62310a3 100644
--- a/client/client.go
+++ b/client/client.go
@@ -339,14 +339,10 @@ type AddPathByStreamClient struct {
 	stream api.GobgpApi_InjectMrtClient
 }
 
-func (c *AddPathByStreamClient) Send(paths ...*table.Path) error {
-	ps := make([]*api.Path, 0, len(paths))
-	for _, p := range paths {
-		ps = append(ps, api.ToPathApiInBin(p, nil))
-	}
+func (c *AddPathByStreamClient) Send(paths ...*api.Path) error {
 	return c.stream.Send(&api.InjectMrtRequest{
 		Resource: api.Resource_GLOBAL,
-		Paths:    ps,
+		Paths:    paths,
 	})
 }
 
@@ -363,7 +359,7 @@ func (cli *Client) AddPathByStream() (*AddPathByStreamClient, error) {
 	return &AddPathByStreamClient{stream}, nil
 }
 
-func (cli *Client) addPath(vrfID string, pathList []*table.Path) ([]byte, error) {
+func (cli *Client) addPath(vrfID string, pathList []*api.Path) ([]byte, error) {
 	resource := api.Resource_GLOBAL
 	if vrfID != "" {
 		resource = api.Resource_VRF
@@ -373,7 +369,7 @@ func (cli *Client) addPath(vrfID string, pathList []*table.Path) ([]byte, error)
 		r, err := cli.cli.AddPath(context.Background(), &api.AddPathRequest{
 			Resource: resource,
 			VrfId:    vrfID,
-			Path:     api.ToPathApi(path, nil),
+			Path:     path,
 		})
 		if err != nil {
 			return nil, err
@@ -383,18 +379,18 @@ func (cli *Client) addPath(vrfID string, pathList []*table.Path) ([]byte, error)
 	return uuid, nil
 }
 
-func (cli *Client) AddPath(pathList []*table.Path) ([]byte, error) {
+func (cli *Client) AddPath(pathList []*api.Path) ([]byte, error) {
 	return cli.addPath("", pathList)
 }
 
-func (cli *Client) AddVRFPath(vrfID string, pathList []*table.Path) ([]byte, error) {
+func (cli *Client) AddVRFPath(vrfID string, pathList []*api.Path) ([]byte, error) {
 	if vrfID == "" {
 		return nil, fmt.Errorf("VRF ID is empty")
 	}
 	return cli.addPath(vrfID, pathList)
 }
 
-func (cli *Client) deletePath(uuid []byte, f bgp.RouteFamily, vrfID string, pathList []*table.Path) error {
+func (cli *Client) deletePath(uuid []byte, f bgp.RouteFamily, vrfID string, pathList []*api.Path) error {
 	var reqs []*api.DeletePathRequest
 
 	resource := api.Resource_GLOBAL
@@ -407,7 +403,7 @@ func (cli *Client) deletePath(uuid []byte, f bgp.RouteFamily, vrfID string, path
 			reqs = append(reqs, &api.DeletePathRequest{
 				Resource: resource,
 				VrfId:    vrfID,
-				Path:     api.ToPathApi(path, nil),
+				Path:     path,
 			})
 		}
 	default:
@@ -427,11 +423,11 @@ func (cli *Client) deletePath(uuid []byte, f bgp.RouteFamily, vrfID string, path
 	return nil
 }
 
-func (cli *Client) DeletePath(pathList []*table.Path) error {
+func (cli *Client) DeletePath(pathList []*api.Path) error {
 	return cli.deletePath(nil, bgp.RouteFamily(0), "", pathList)
 }
 
-func (cli *Client) DeleteVRFPath(vrfID string, pathList []*table.Path) error {
+func (cli *Client) DeleteVRFPath(vrfID string, pathList []*api.Path) error {
 	if vrfID == "" {
 		return fmt.Errorf("VRF ID is empty")
 	}
diff --git a/gobgp/cmd/global.go b/gobgp/cmd/global.go
index 91c71ff5..809a4b2e 100644
--- a/gobgp/cmd/global.go
+++ b/gobgp/cmd/global.go
@@ -27,6 +27,7 @@ import (
 
 	"github.com/spf13/cobra"
 
+	api "github.com/osrg/gobgp/api"
 	"github.com/osrg/gobgp/config"
 	"github.com/osrg/gobgp/packet/bgp"
 	"github.com/osrg/gobgp/table"
@@ -1074,11 +1075,11 @@ func extractAggregator(args []string) ([]string, bgp.PathAttributeInterface, err
 	return args, nil, nil
 }
 
-func ParsePath(rf bgp.RouteFamily, args []string) (*table.Path, error) {
+func ParsePath(rf bgp.RouteFamily, args []string) (*api.Path, error) {
 	var nlri bgp.AddrPrefixInterface
 	var extcomms []string
 	var err error
-	attrs := table.PathAttrs(make([]bgp.PathAttributeInterface, 0, 1))
+	attrs := make([]bgp.PathAttributeInterface, 0, 1)
 
 	fns := []func([]string) ([]string, bgp.PathAttributeInterface, error){
 		extractOrigin,         // 1 ORIGIN
@@ -1261,10 +1262,9 @@ func ParsePath(rf bgp.RouteFamily, args []string) (*table.Path, error) {
 			attrs = append(attrs, ip6p)
 		}
 	}
+	sort.Slice(attrs, func(i, j int) bool { return attrs[i].GetType() < attrs[j].GetType() })
 
-	sort.Sort(attrs)
-
-	return table.NewPath(nil, nlri, false, attrs, time.Now(), false), nil
+	return api.NewPath(nlri, false, attrs, time.Now()), nil
 }
 
 func showGlobalRib(args []string) error {
@@ -1447,15 +1447,15 @@ usage: %s rib %s key <KEY> [value <VALUE>]`,
 
 	if modtype == CMD_ADD {
 		if resource == CMD_VRF {
-			_, err = client.AddVRFPath(name, []*table.Path{path})
+			_, err = client.AddVRFPath(name, []*api.Path{path})
 		} else {
-			_, err = client.AddPath([]*table.Path{path})
+			_, err = client.AddPath([]*api.Path{path})
 		}
 	} else {
 		if resource == CMD_VRF {
-			err = client.DeleteVRFPath(name, []*table.Path{path})
+			err = client.DeleteVRFPath(name, []*api.Path{path})
 		} else {
-			err = client.DeletePath([]*table.Path{path})
+			err = client.DeletePath([]*api.Path{path})
 		}
 	}
 	return err
diff --git a/gobgp/cmd/global_test.go b/gobgp/cmd/global_test.go
index 15c633a2..1be0a561 100644
--- a/gobgp/cmd/global_test.go
+++ b/gobgp/cmd/global_test.go
@@ -31,7 +31,8 @@ func Test_ParsePath(t *testing.T) {
 	path, err := ParsePath(bgp.RF_IPv4_UC, strings.Split(buf, " "))
 	assert.Nil(err)
 	i := 0
-	for _, a := range path.GetPathAttrs() {
+	attrs, _ := path.GetNativePathAttributes()
+	for _, a := range attrs {
 		assert.True(i < int(a.GetType()))
 		i = int(a.GetType())
 	}
diff --git a/gobgp/cmd/mrt.go b/gobgp/cmd/mrt.go
index f8a2d568..cef4f1a0 100644
--- a/gobgp/cmd/mrt.go
+++ b/gobgp/cmd/mrt.go
@@ -24,9 +24,9 @@ import (
 
 	"github.com/spf13/cobra"
 
+	api "github.com/osrg/gobgp/api"
 	"github.com/osrg/gobgp/packet/bgp"
 	"github.com/osrg/gobgp/packet/mrt"
-	"github.com/osrg/gobgp/table"
 )
 
 func injectMrt() error {
@@ -45,7 +45,7 @@ func injectMrt() error {
 		return fmt.Errorf("Specified queue size is smaller than 1, refusing to run with unbounded memory usage")
 	}
 
-	ch := make(chan []*table.Path, mrtOpts.QueueSize)
+	ch := make(chan []*api.Path, mrtOpts.QueueSize)
 	go func() {
 
 		var peers []*mrt.Peer
@@ -107,47 +107,53 @@ func injectMrt() error {
 				rib := msg.Body.(*mrt.Rib)
 				nlri := rib.Prefix
 
-				paths := make([]*table.Path, 0, len(rib.Entries))
+				paths := make([]*api.Path, 0, len(rib.Entries))
 
 				for _, e := range rib.Entries {
 					if len(peers) < int(e.PeerIndex) {
 						exitWithError(fmt.Errorf("invalid peer index: %d (PEER_INDEX_TABLE has only %d peers)\n", e.PeerIndex, len(peers)))
 					}
-					source := &table.PeerInfo{
-						AS: peers[e.PeerIndex].AS,
-						ID: peers[e.PeerIndex].BgpId,
-					}
-					t := time.Unix(int64(e.OriginatedTime), 0)
+					//t := time.Unix(int64(e.OriginatedTime), 0)
 
+					var attrs []bgp.PathAttributeInterface
 					switch subType {
 					case mrt.RIB_IPV4_UNICAST, mrt.RIB_IPV4_UNICAST_ADDPATH:
-						paths = append(paths, table.NewPath(source, nlri, false, e.PathAttributes, t, false))
+						if mrtOpts.NextHop != nil {
+							for i, attr := range e.PathAttributes {
+								if attr.GetType() != bgp.BGP_ATTR_TYPE_NEXT_HOP {
+									e.PathAttributes[i] = bgp.NewPathAttributeNextHop(mrtOpts.NextHop.String())
+									break
+								}
+							}
+						}
+						attrs = e.PathAttributes
 					default:
-						attrs := make([]bgp.PathAttributeInterface, 0, len(e.PathAttributes))
+						attrs = make([]bgp.PathAttributeInterface, 0, len(e.PathAttributes))
 						for _, attr := range e.PathAttributes {
 							if attr.GetType() != bgp.BGP_ATTR_TYPE_MP_REACH_NLRI {
 								attrs = append(attrs, attr)
 							} else {
 								a := attr.(*bgp.PathAttributeMpReachNLRI)
-								attrs = append(attrs, bgp.NewPathAttributeMpReachNLRI(a.Nexthop.String(), []bgp.AddrPrefixInterface{nlri}))
+								nexthop := a.Nexthop.String()
+								if mrtOpts.NextHop != nil {
+									nexthop = mrtOpts.NextHop.String()
+								}
+								attrs = append(attrs, bgp.NewPathAttributeMpReachNLRI(nexthop, []bgp.AddrPrefixInterface{nlri}))
 							}
 						}
-						paths = append(paths, table.NewPath(source, nlri, false, attrs, t, false))
-					}
-				}
-				if mrtOpts.NextHop != nil {
-					for _, p := range paths {
-						p.SetNexthop(mrtOpts.NextHop)
 					}
+
+					path := api.NewPath(nlri, false, attrs, time.Unix(int64(e.OriginatedTime), 0))
+					path.SourceAsn = peers[e.PeerIndex].AS
+					path.SourceId = peers[e.PeerIndex].BgpId.String()
+
+					// TODO: compare here if mrtOpts.Best is enabled
+					paths = append(paths, path)
 				}
 
+				// TODO: calculate properly if necessary.
 				if mrtOpts.Best {
-					dst := table.NewDestination(nlri, 0, paths[1:]...)
-					best, _, _ := dst.Calculate(paths[0]).GetChanges(table.GLOBAL_RIB_NAME, 0, false)
-					if best == nil {
-						exitWithError(fmt.Errorf("Can't find the best %v", nlri))
-					}
-					paths = []*table.Path{best}
+					paths = []*api.Path{paths[0]}
 				}
 
 				if idx >= mrtOpts.RecordSkip {
diff --git a/gobgp/lib/path.go b/gobgp/lib/path.go
index da06bb93..863a2595 100644
--- a/gobgp/lib/path.go
+++ b/gobgp/lib/path.go
@@ -57,13 +57,22 @@ func serialize_path(rf C.int, input *C.char) *C.path {
 		return nil
 	}
 	path := C.new_path()
-	if nlri := p.GetNlri(); nlri != nil {
+	if nlri, err := p.GetNativeNlri(); err != nil {
+		return nil
+	} else {
 		buf, _ := nlri.Serialize()
 		path.nlri.len = C.int(len(buf))
 		path.nlri.value = C.CString(string(buf))
 	}
-	for _, attr := range p.GetPathAttrs() {
-		buf, _ := attr.Serialize()
+	attrs, err := p.GetNativePathAttributes()
+	if err != nil {
+		return nil
+	}
+	for _, attr := range attrs {
+		buf, err := attr.Serialize()
+		if err != nil {
+			return nil
+		}
 		C.append_path_attribute(path, C.int(len(buf)), C.CString(string(buf)))
 	}
 	return path
diff --git a/packet/mrt/mrt.go b/packet/mrt/mrt.go
index 0c009e80..9c6fef6d 100644
--- a/packet/mrt/mrt.go
+++ b/packet/mrt/mrt.go
@@ -19,10 +19,11 @@ import (
 	"bytes"
 	"encoding/binary"
 	"fmt"
-	"github.com/osrg/gobgp/packet/bgp"
 	"math"
 	"net"
 	"time"
+
+	"github.com/osrg/gobgp/packet/bgp"
 )
 
 const (