Decode message context using the message type only 51/30251/1
authorVladimir Lavor <vlavor@cisco.com>
Thu, 3 Dec 2020 13:40:09 +0000 (14:40 +0100)
committerVladimir Lavor <vlavor@cisco.com>
Thu, 3 Dec 2020 13:40:09 +0000 (14:40 +0100)
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>
codec/msg_codec.go
core/channel.go
core/request_handler.go

index 6534e7d..227764d 100644 (file)
@@ -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:
index fbb3e59..4cb5761 100644 (file)
@@ -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.
index f9d972a..29685f6 100644 (file)
@@ -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)
 }