From 392a56b700fa6715d091e56c49f79bfe32613fc6 Mon Sep 17 00:00:00 2001
From: Ondrej Fabry <ofabry@cisco.com>
Date: Thu, 5 Apr 2018 13:46:54 +0200
Subject: Lookup message name by ID when receiving unexpected message

Change-Id: I693e8084b7e3f036dec5e557dc772857bb7d5f3d
Signed-off-by: Ondrej Fabry <ofabry@cisco.com>
---
 api/api.go      | 22 ++++++++++++++++++----
 api/api_test.go | 32 ++++++++++++++++++++++++--------
 2 files changed, 42 insertions(+), 12 deletions(-)

(limited to 'api')

diff --git a/api/api.go b/api/api.go
index 60508cd..eafdf22 100644
--- a/api/api.go
+++ b/api/api.go
@@ -78,6 +78,8 @@ type MessageDecoder interface {
 type MessageIdentifier interface {
 	// GetMessageID returns message identifier of given API message.
 	GetMessageID(msg Message) (uint16, error)
+	// LookupByID looks up message name and crc by ID
+	LookupByID(ID uint16) (string, error)
 }
 
 // Channel is the main communication interface with govpp core. It contains two Go channels, one for sending the requests
@@ -231,6 +233,7 @@ func (ch *Channel) receiveReplyInternal(msg Message) (LastReplyReceived bool, er
 				LastReplyReceived = true
 				return
 			}
+
 			// message checks
 			expMsgID, err := ch.MsgIdentifier.GetMessageID(msg)
 			if err != nil {
@@ -238,20 +241,31 @@ func (ch *Channel) receiveReplyInternal(msg Message) (LastReplyReceived bool, er
 					msg.GetMessageName(), msg.GetCrcString())
 				return false, err
 			}
+
 			if vppReply.MessageID != expMsgID {
+				var msgNameCrc string
+				if nameCrc, err := ch.MsgIdentifier.LookupByID(vppReply.MessageID); err != nil {
+					msgNameCrc = err.Error()
+				} else {
+					msgNameCrc = nameCrc
+				}
+
 				if ch.lastTimedOut {
-					logrus.Warnf("received invalid message ID, expected %d (%s), but got %d (probably timed out reply from previous request)",
-						expMsgID, msg.GetMessageName(), vppReply.MessageID)
+					logrus.Warnf("received invalid message ID, expected %d (%s), but got %d (%s) (probably timed out reply from previous request)",
+						expMsgID, msg.GetMessageName(), vppReply.MessageID, msgNameCrc)
 					continue
 				}
-				err = fmt.Errorf("received invalid message ID, expected %d (%s), but got %d (check if multiple goroutines are not sharing single GoVPP channel)",
-					expMsgID, msg.GetMessageName(), vppReply.MessageID)
+
+				err = fmt.Errorf("received invalid message ID, expected %d (%s), but got %d (%s) (check if multiple goroutines are not sharing single GoVPP channel)",
+					expMsgID, msg.GetMessageName(), vppReply.MessageID, msgNameCrc)
 				return false, err
 			}
+
 			ch.lastTimedOut = false
 			// decode the message
 			err = ch.MsgDecoder.DecodeMsg(vppReply.Data, msg)
 			return false, err
+
 		case <-timer.C:
 			ch.lastTimedOut = true
 			err = fmt.Errorf("no reply received within the timeout period %s", ch.replyTimeout)
diff --git a/api/api_test.go b/api/api_test.go
index 62541ab..a439986 100644
--- a/api/api_test.go
+++ b/api/api_test.go
@@ -321,7 +321,6 @@ func TestCheckMessageCompatibility(t *testing.T) {
 	err := ctx.ch.CheckMessageCompatibility(&interfaces.SwInterfaceSetFlags{})
 	Expect(err).ShouldNot(HaveOccurred())
 }
-
 func TestSetReplyTimeout(t *testing.T) {
 	ctx := setupTest(t)
 	defer ctx.teardownTest()
@@ -483,13 +482,15 @@ func TestReceiveReplyAfterTimeout(t *testing.T) {
 	Expect(err).ShouldNot(HaveOccurred())
 }
 
-/*
-	TODO: fix mock adapter
-	This test will fail because mock adapter will stop sending replies
-	when it encounters control_ping_reply from multi request,
-	thus never sending reply for next request
-
 func TestReceiveReplyAfterTimeoutMultiRequest(t *testing.T) {
+	/*
+		TODO: fix mock adapter
+		This test will fail because mock adapter will stop sending replies
+		when it encounters control_ping_reply from multi request,
+		thus never sending reply for next request
+	*/
+	t.Skip()
+
 	ctx := setupTest(t)
 	defer ctx.teardownTest()
 
@@ -544,4 +545,19 @@ func TestReceiveReplyAfterTimeoutMultiRequest(t *testing.T) {
 	err = ctx.ch.SendRequest(req).ReceiveReply(reply)
 	Expect(err).ShouldNot(HaveOccurred())
 }
-*/
+
+func TestInvalidMessageID(t *testing.T) {
+	ctx := setupTest(t)
+	defer ctx.teardownTest()
+
+	// first one request should work
+	ctx.mockVpp.MockReply(&vpe.ShowVersionReply{})
+	err := ctx.ch.SendRequest(&vpe.ShowVersion{}).ReceiveReply(&vpe.ShowVersionReply{})
+	Expect(err).ShouldNot(HaveOccurred())
+
+	// second should fail with error invalid message ID
+	ctx.mockVpp.MockReply(&vpe.ShowVersionReply{})
+	err = ctx.ch.SendRequest(&vpe.ControlPing{}).ReceiveReply(&vpe.ControlPingReply{})
+	Expect(err).Should(HaveOccurred())
+	Expect(err.Error()).To(ContainSubstring("invalid message ID"))
+}
-- 
cgit 1.2.3-korg