From e45d8802fd8de3602db630d75b370ff804000da9 Mon Sep 17 00:00:00 2001 From: Ondrej Fabry Date: Fri, 15 Nov 2019 12:33:28 +0100 Subject: Improve compatibility checking - added CompatibilityError to api package to represent error with list of incompatible messages - added UnknownMsgError to adapter package to represent error for unknown message - added list of registered message types Change-Id: I2623b45135521d52612ba91e4605fc064a7fc76e Signed-off-by: Ondrej Fabry --- adapter/socketclient/socketclient.go | 2 +- adapter/vpp_api.go | 12 +++++++++++ adapter/vppapiclient/vppapiclient.go | 2 +- api/binapi.go | 36 +++++++++++++++++++++++++++------ core/channel.go | 14 +++++++++++-- core/channel_test.go | 12 +++++------ examples/rpc-service/rpc_service.go | 4 ++-- examples/simple-client/simple_client.go | 4 ++++ 8 files changed, 68 insertions(+), 18 deletions(-) diff --git a/adapter/socketclient/socketclient.go b/adapter/socketclient/socketclient.go index 043d253..366163f 100644 --- a/adapter/socketclient/socketclient.go +++ b/adapter/socketclient/socketclient.go @@ -419,7 +419,7 @@ func (c *vppClient) GetMsgID(msgName string, msgCrc string) (uint16, error) { msg := msgName + "_" + msgCrc msgID, ok := c.msgTable[msg] if !ok { - return 0, fmt.Errorf("unknown message: %q", msg) + return 0, &adapter.UnknownMsgError{msgName, msgCrc} } return msgID, nil } diff --git a/adapter/vpp_api.go b/adapter/vpp_api.go index 71bf7c2..b32f975 100644 --- a/adapter/vpp_api.go +++ b/adapter/vpp_api.go @@ -16,6 +16,7 @@ package adapter import ( "errors" + "fmt" ) const ( @@ -52,3 +53,14 @@ type VppAPI interface { // WaitReady waits until adapter is ready. WaitReady() error } + +// UnknownMsgError is the error type usually returned by GetMsgID +// method of VppAPI. It describes the name and CRC for the unknown message. +type UnknownMsgError struct { + MsgName string + MsgCrc string +} + +func (u *UnknownMsgError) Error() string { + return fmt.Sprintf("unknown message: %s_%s", u.MsgName, u.MsgCrc) +} diff --git a/adapter/vppapiclient/vppapiclient.go b/adapter/vppapiclient/vppapiclient.go index fcd3f3f..977f32d 100644 --- a/adapter/vppapiclient/vppapiclient.go +++ b/adapter/vppapiclient/vppapiclient.go @@ -116,7 +116,7 @@ func (a *vppClient) GetMsgID(msgName string, msgCrc string) (uint16, error) { msgID := uint16(C.govpp_get_msg_index(nameAndCrc)) if msgID == ^uint16(0) { // VPP does not know this message - return msgID, fmt.Errorf("unknown message: %v (crc: %v)", msgName, msgCrc) + return msgID, &adapter.UnknownMsgError{msgName, msgCrc} } return msgID, nil diff --git a/api/binapi.go b/api/binapi.go index 6495d0d..d933612 100644 --- a/api/binapi.go +++ b/api/binapi.go @@ -15,6 +15,8 @@ package api import ( + "fmt" + "reflect" "time" ) @@ -115,15 +117,32 @@ type SubscriptionCtx interface { Unsubscribe() error } -var registeredMessages = make(map[string]Message) +// CompatibilityError is the error type usually returned by CheckCompatibility +// method of Channel. It describes all of the incompatible messages. +type CompatibilityError struct { + // IncompatibleMessages is the list of all messages + // that failed compatibility check. + IncompatibleMessages []string +} + +func (c *CompatibilityError) Error() string { + return fmt.Sprintf("%d incompatible messages: %v", len(c.IncompatibleMessages), c.IncompatibleMessages) +} + +var ( + registeredMessageTypes = make(map[reflect.Type]string) + registeredMessages = make(map[string]Message) +) // RegisterMessage is called from generated code to register message. func RegisterMessage(x Message, name string) { - name = x.GetMessageName() + "_" + x.GetCrcString() - /*if _, ok := registeredMessages[name]; ok { - panic(fmt.Errorf("govpp: duplicate message registered: %s (%s)", name, x.GetCrcString())) - }*/ - registeredMessages[name] = x + typ := reflect.TypeOf(x) + namecrc := x.GetMessageName() + "_" + x.GetCrcString() + if _, ok := registeredMessageTypes[typ]; ok { + panic(fmt.Errorf("govpp: message type %v already registered as %s (%s)", typ, name, namecrc)) + } + registeredMessages[namecrc] = x + registeredMessageTypes[typ] = name } // GetRegisteredMessages returns list of all registered messages. @@ -131,6 +150,11 @@ func GetRegisteredMessages() map[string]Message { return registeredMessages } +// GetRegisteredMessageTypes returns list of all registered message types. +func GetRegisteredMessageTypes() map[reflect.Type]string { + return registeredMessageTypes +} + // GoVppAPIPackageIsVersion1 is referenced from generated binapi files // to assert that that code is compatible with this version of the GoVPP api package. const GoVppAPIPackageIsVersion1 = true diff --git a/core/channel.go b/core/channel.go index 5b513e3..363a267 100644 --- a/core/channel.go +++ b/core/channel.go @@ -23,6 +23,7 @@ import ( "github.com/sirupsen/logrus" + "git.fd.io/govpp.git/adapter" "git.fd.io/govpp.git/api" ) @@ -144,14 +145,23 @@ func (ch *Channel) SendMultiRequest(msg api.Message) api.MultiRequestCtx { } func (ch *Channel) CheckCompatiblity(msgs ...api.Message) error { + var comperr api.CompatibilityError for _, msg := range msgs { - // TODO: collect all incompatible messages and return summarized error _, err := ch.msgIdentifier.GetMessageID(msg) if err != nil { + if uerr, ok := err.(*adapter.UnknownMsgError); ok { + m := fmt.Sprintf("%s_%s", uerr.MsgName, uerr.MsgCrc) + comperr.IncompatibleMessages = append(comperr.IncompatibleMessages, m) + continue + } + // other errors return immediatelly return err } } - return nil + if len(comperr.IncompatibleMessages) == 0 { + return nil + } + return &comperr } func (ch *Channel) SubscribeNotification(notifChan chan api.Message, event api.Message) (api.SubscriptionCtx, error) { diff --git a/core/channel_test.go b/core/channel_test.go index 7e721cd..849255a 100644 --- a/core/channel_test.go +++ b/core/channel_test.go @@ -213,15 +213,15 @@ func TestSetReplyTimeoutMultiRequest(t *testing.T) { ctx.mockVpp.MockReply( &interfaces.SwInterfaceDetails{ SwIfIndex: 1, - InterfaceName: []byte("if-name-test"), + InterfaceName: "if-name-test", }, &interfaces.SwInterfaceDetails{ SwIfIndex: 2, - InterfaceName: []byte("if-name-test"), + InterfaceName: "if-name-test", }, &interfaces.SwInterfaceDetails{ SwIfIndex: 3, - InterfaceName: []byte("if-name-test"), + InterfaceName: "if-name-test", }, ) ctx.mockVpp.MockReply(&ControlPingReply{}) @@ -288,7 +288,7 @@ func TestMultiRequestDouble(t *testing.T) { msgs = append(msgs, mock.MsgWithContext{ Msg: &interfaces.SwInterfaceDetails{ SwIfIndex: uint32(i), - InterfaceName: []byte("if-name-test"), + InterfaceName: "if-name-test", }, Multipart: true, SeqNum: 1, @@ -301,7 +301,7 @@ func TestMultiRequestDouble(t *testing.T) { mock.MsgWithContext{ Msg: &interfaces.SwInterfaceDetails{ SwIfIndex: uint32(i), - InterfaceName: []byte("if-name-test"), + InterfaceName: "if-name-test", }, Multipart: true, SeqNum: 2, @@ -427,7 +427,7 @@ func TestReceiveReplyAfterTimeoutMultiRequest(t *testing.T) { msgs = append(msgs, mock.MsgWithContext{ Msg: &interfaces.SwInterfaceDetails{ SwIfIndex: uint32(i), - InterfaceName: []byte("if-name-test"), + InterfaceName: "if-name-test", }, Multipart: true, SeqNum: 2, diff --git a/examples/rpc-service/rpc_service.go b/examples/rpc-service/rpc_service.go index ca0c295..8ff6c08 100644 --- a/examples/rpc-service/rpc_service.go +++ b/examples/rpc-service/rpc_service.go @@ -17,12 +17,12 @@ package main import ( - "bytes" "context" "flag" "fmt" "io" "log" + "strings" "git.fd.io/govpp.git" "git.fd.io/govpp.git/adapter/socketclient" @@ -88,6 +88,6 @@ func interfaceDump(ch api.Channel) { if err != nil { log.Fatalln("ERROR: DumpSwInterface failed:", err) } - fmt.Printf("- interface: %s\n", bytes.Trim(iface.InterfaceName, "\x00")) + fmt.Printf("- interface: %s\n", strings.Trim(iface.InterfaceName, "\x00")) } } diff --git a/examples/simple-client/simple_client.go b/examples/simple-client/simple_client.go index 3a24e6a..3ed811d 100644 --- a/examples/simple-client/simple_client.go +++ b/examples/simple-client/simple_client.go @@ -65,6 +65,10 @@ func main() { vppVersion(ch) + if err := ch.CheckCompatiblity(interfaces.AllMessages()...); err != nil { + log.Fatal(err) + } + createLoopback(ch) createLoopback(ch) interfaceDump(ch) -- cgit 1.2.3-korg