From: Vladimir Lavor <vlavor@cisco.com>
Date: Thu, 3 Dec 2020 13:40:09 +0000 (+0100)
Subject: Decode message context using the message type only
X-Git-Tag: v0.4.0~29
X-Git-Url: https://gerrit.fd.io/r/gitweb?p=govpp.git;a=commitdiff_plain;h=8d3131f90f71271835e5fed91831565797894614

Decode message context using the message type only

In order to prevent potential future issues, the method
returning message based on its ID but ignoring its package
was optimized.

Change-Id: I12aa2b243f32f38cb3dbc7731613c7ed9fc66539
Signed-off-by: Vladimir Lavor <vlavor@cisco.com>
---

diff --git a/codec/msg_codec.go b/codec/msg_codec.go
index 6534e7d..227764d 100644
--- a/codec/msg_codec.go
+++ b/codec/msg_codec.go
@@ -30,8 +30,8 @@ func EncodeMsg(msg api.Message, msgID uint16) (data []byte, err error) {
 func DecodeMsg(data []byte, msg api.Message) (err error) {
 	return DefaultCodec.DecodeMsg(data, msg)
 }
-func DecodeMsgContext(data []byte, msg api.Message) (context uint32, err error) {
-	return DefaultCodec.DecodeMsgContext(data, msg)
+func DecodeMsgContext(data []byte, msgType api.MessageType) (context uint32, err error) {
+	return DefaultCodec.DecodeMsgContext(data, msgType)
 }
 
 // MsgCodec provides encoding and decoding functionality of `api.Message` structs into/from
@@ -106,12 +106,8 @@ func (*MsgCodec) DecodeMsg(data []byte, msg api.Message) (err error) {
 	return nil
 }
 
-func (*MsgCodec) DecodeMsgContext(data []byte, msg api.Message) (context uint32, err error) {
-	if msg == nil {
-		return 0, errors.New("nil message passed in")
-	}
-
-	switch msg.GetMessageType() {
+func (*MsgCodec) DecodeMsgContext(data []byte, msgType api.MessageType) (context uint32, err error) {
+	switch msgType {
 	case api.RequestMessage:
 		return binary.BigEndian.Uint32(data[6:10]), nil
 	case api.ReplyMessage:
diff --git a/core/channel.go b/core/channel.go
index fbb3e59..4cb5761 100644
--- a/core/channel.go
+++ b/core/channel.go
@@ -37,8 +37,8 @@ type MessageCodec interface {
 	EncodeMsg(msg api.Message, msgID uint16) ([]byte, error)
 	// DecodeMsg decodes binary-encoded data of a message into provided Message structure.
 	DecodeMsg(data []byte, msg api.Message) error
-	// DecodeMsgContext decodes context from message data.
-	DecodeMsgContext(data []byte, msg api.Message) (context uint32, err error)
+	// DecodeMsgContext decodes context from message data and type.
+	DecodeMsgContext(data []byte, msgType api.MessageType) (context uint32, err error)
 }
 
 // MessageIdentifier provides identification of generated API messages.
diff --git a/core/request_handler.go b/core/request_handler.go
index f9d972a..29685f6 100644
--- a/core/request_handler.go
+++ b/core/request_handler.go
@@ -17,7 +17,6 @@ package core
 import (
 	"errors"
 	"fmt"
-	"reflect"
 	"sync/atomic"
 	"time"
 
@@ -210,7 +209,7 @@ func (c *Connection) msgCallback(msgID uint16, data []byte) {
 		return
 	}
 
-	msg, err := c.getMessageByID(msgID)
+	msgType, name, crc, err := c.getMessageDataByID(msgID)
 	if err != nil {
 		log.Warnln(err)
 		return
@@ -221,7 +220,7 @@ func (c *Connection) msgCallback(msgID uint16, data []byte) {
 	// - replies that don't have context as first field (comes as zero)
 	// - events that don't have context at all (comes as non zero)
 	//
-	context, err := c.codec.DecodeMsgContext(data, msg)
+	context, err := c.codec.DecodeMsgContext(data, msgType)
 	if err != nil {
 		log.WithField("msg_id", msgID).Warnf("Unable to decode message context: %v", err)
 		return
@@ -230,14 +229,6 @@ func (c *Connection) msgCallback(msgID uint16, data []byte) {
 	chanID, isMulti, seqNum := unpackRequestContext(context)
 
 	if log.Level == logger.DebugLevel { // for performance reasons - logrus does some processing even if debugs are disabled
-		msg = reflect.New(reflect.TypeOf(msg).Elem()).Interface().(api.Message)
-
-		// decode the message
-		if err = c.codec.DecodeMsg(data, msg); err != nil {
-			err = fmt.Errorf("decoding message failed: %w", err)
-			return
-		}
-
 		log.WithFields(logger.Fields{
 			"context":  context,
 			"msg_id":   msgID,
@@ -245,8 +236,8 @@ func (c *Connection) msgCallback(msgID uint16, data []byte) {
 			"channel":  chanID,
 			"is_multi": isMulti,
 			"seq_num":  seqNum,
-			"msg_crc":  msg.GetCrcString(),
-		}).Debugf("<-- govpp RECEIVE: %s %+v", msg.GetMessageName(), msg)
+			"msg_crc":  crc,
+		}).Debugf("<-- govpp RECEIVE: %s %+v", name)
 	}
 
 	if context == 0 || c.isNotificationMessage(msgID) {
@@ -420,18 +411,12 @@ func compareSeqNumbers(seqNum1, seqNum2 uint16) int {
 	return 1
 }
 
-// Returns first message from any package where the message ID matches
-// Note: the msg is further used only for its MessageType which is not
-// affected by the message's package
-func (c *Connection) getMessageByID(msgID uint16) (msg api.Message, err error) {
-	var ok bool
+// Returns message data based on the message ID not depending on the message path
+func (c *Connection) getMessageDataByID(msgID uint16) (typ api.MessageType, name, crc string, err error) {
 	for _, msgs := range c.msgMapByPath {
-		if msg, ok = msgs[msgID]; ok {
-			break
+		if msg, ok := msgs[msgID]; ok {
+			return msg.GetMessageType(), msg.GetMessageName(), msg.GetCrcString(), nil
 		}
 	}
-	if !ok {
-		return nil, fmt.Errorf("unknown message received, ID: %d", msgID)
-	}
-	return msg, nil
+	return typ, name, crc, fmt.Errorf("unknown message received, ID: %d", msgID)
 }