[VOL-3914] Not forwarding OMCI messages if the ONU is disabled
Change-Id: Ic826368642e510e9dc39766569ed5682e5910b5d
diff --git a/internal/bbsim/devices/olt.go b/internal/bbsim/devices/olt.go
index 87b9da4..b2480e7 100644
--- a/internal/bbsim/devices/olt.go
+++ b/internal/bbsim/devices/olt.go
@@ -22,6 +22,7 @@
"fmt"
"github.com/opencord/bbsim/internal/bbsim/responders/dhcp"
"github.com/opencord/bbsim/internal/bbsim/types"
+ omcilib "github.com/opencord/bbsim/internal/common/omci"
"github.com/opencord/voltha-protos/v4/go/ext/config"
"net"
"strconv"
@@ -1335,15 +1336,37 @@
"OnuId": onu.ID,
"OnuSn": onu.Sn(),
}).Tracef("Received OmciMsgOut")
- msg := types.Message{
- Type: types.OMCI,
- Data: types.OmciMessage{
- OnuSN: onu.SerialNumber,
- OnuID: onu.ID,
- OmciMsg: omci_msg,
- },
+ omciPkt, omciMsg, err := omcilib.ParseOpenOltOmciPacket(omci_msg.Pkt)
+ if err != nil {
+ log.WithFields(log.Fields{
+ "IntfId": onu.PonPortID,
+ "SerialNumber": onu.Sn(),
+ "omciPacket": omcilib.HexDecode(omci_msg.Pkt),
+ "err": err.Error(),
+ }).Error("cannot-parse-OMCI-packet")
+ return nil, fmt.Errorf("olt-received-malformed-omci-packet")
}
- onu.Channel <- msg
+ if onu.InternalState.Current() == OnuStateDisabled {
+ // if the ONU is disabled just drop the message
+ log.WithFields(log.Fields{
+ "IntfId": onu.PonPortID,
+ "SerialNumber": onu.Sn(),
+ "omciBytes": hex.EncodeToString(omciPkt.Data()),
+ "omciPkt": omciPkt,
+ "omciMsgType": omciMsg.MessageType,
+ }).Warn("dropping-omci-message")
+ } else {
+ msg := types.Message{
+ Type: types.OMCI,
+ Data: types.OmciMessage{
+ OnuSN: onu.SerialNumber,
+ OnuID: onu.ID,
+ OmciMsg: omciMsg,
+ OmciPkt: omciPkt,
+ },
+ }
+ onu.Channel <- msg
+ }
return new(openolt.Empty), nil
}
diff --git a/internal/bbsim/devices/olt_test.go b/internal/bbsim/devices/olt_test.go
index acbe7a7..994902e 100644
--- a/internal/bbsim/devices/olt_test.go
+++ b/internal/bbsim/devices/olt_test.go
@@ -17,7 +17,10 @@
package devices
import (
+ "context"
+ "github.com/looplab/fsm"
"github.com/opencord/bbsim/internal/bbsim/types"
+ bbsim "github.com/opencord/bbsim/internal/bbsim/types"
"github.com/opencord/bbsim/internal/common"
"net"
"testing"
@@ -54,6 +57,17 @@
ID: onuId,
PonPort: &pon,
PonPortID: pon.ID,
+ InternalState: fsm.NewFSM(
+ OnuStateCreated,
+ // this is fake state machine, we don't care about transition in the OLT
+ // unit tests, we'll use SetState to emulate cases
+ fsm.Events{
+ {Name: OnuTxEnable, Src: []string{}, Dst: OnuStateEnabled},
+ {Name: OnuTxDisable, Src: []string{}, Dst: OnuStateDisabled},
+ },
+ fsm.Callbacks{},
+ ),
+ Channel: make(chan bbsim.Message, 2048),
}
for k, s := range services {
@@ -445,3 +459,54 @@
err = olt.validateFlow(invalidAllocDifferentPonFlow)
assert.NilError(t, err)
}
+
+func Test_Olt_OmciMsgOut(t *testing.T) {
+ numPon := 4
+ numOnu := 4
+
+ olt := createMockOlt(numPon, numOnu, []ServiceIf{})
+
+ // a malformed packet should return an error
+ msg := &openolt.OmciMsg{
+ IntfId: 1,
+ OnuId: 1,
+ Pkt: []byte{},
+ }
+ ctx := context.TODO()
+ _, err := olt.OmciMsgOut(ctx, msg)
+ assert.Error(t, err, "olt-received-malformed-omci-packet")
+
+ // a correct packet for a non exiting ONU should throw an error
+ msg = &openolt.OmciMsg{
+ IntfId: 10,
+ OnuId: 25,
+ Pkt: makeOmciSetRequest(t),
+ }
+ _, err = olt.OmciMsgOut(ctx, msg)
+ assert.Error(t, err, "Cannot find PonPort with id 10 in OLT 0")
+
+ // a correct packet for a disabled ONU should be dropped
+ // note that an error is not returned, this is valid in BBsim
+ const (
+ ponId = 1
+ onuId = 1
+ )
+ pon, _ := olt.GetPonById(ponId)
+ onu, _ := pon.GetOnuById(onuId)
+ onu.InternalState.SetState(OnuStateDisabled)
+ msg = &openolt.OmciMsg{
+ IntfId: ponId,
+ OnuId: onuId,
+ Pkt: makeOmciSetRequest(t),
+ }
+ _, err = olt.OmciMsgOut(ctx, msg)
+ assert.NilError(t, err)
+ assert.Equal(t, len(onu.Channel), 0) // check that no messages have been sent
+
+ // test that the ONU receives a valid packet
+ onu.InternalState.SetState(OnuStateEnabled)
+ _, err = olt.OmciMsgOut(ctx, msg)
+ assert.NilError(t, err)
+ assert.Equal(t, len(onu.Channel), 1) // check that one message have been sent
+
+}
diff --git a/internal/bbsim/devices/onu.go b/internal/bbsim/devices/onu.go
index 6c1b5b9..5e74f46 100644
--- a/internal/bbsim/devices/onu.go
+++ b/internal/bbsim/devices/onu.go
@@ -660,7 +660,7 @@
func (o *Onu) publishOmciEvent(msg bbsim.OmciMessage) {
if olt.PublishEvents {
- _, omciMsg, err := omcilib.ParseOpenOltOmciPacket(msg.OmciMsg.Pkt)
+ _, omciMsg, err := omcilib.ParseOpenOltOmciPacket(msg.OmciPkt.Data())
if err != nil {
log.Errorf("error in getting msgType %v", err)
return
@@ -679,7 +679,7 @@
// Create a TestResponse packet and send it
func (o *Onu) sendTestResult(msg bbsim.OmciMessage, stream openolt.Openolt_EnableIndicationServer) error {
- resp, err := omcilib.BuildTestResult(msg.OmciMsg.Pkt)
+ resp, err := omcilib.BuildTestResult(msg.OmciPkt.Data())
if err != nil {
return err
}
@@ -706,44 +706,35 @@
// and generate the appropriate response to it
func (o *Onu) handleOmciRequest(msg bbsim.OmciMessage, stream openolt.Openolt_EnableIndicationServer) {
- omciPkt, omciMsg, err := omcilib.ParseOpenOltOmciPacket(msg.OmciMsg.Pkt)
- if err != nil {
- log.WithFields(log.Fields{
- "IntfId": o.PonPortID,
- "SerialNumber": o.Sn(),
- "omciPacket": omcilib.HexDecode(msg.OmciMsg.Pkt),
- }).Error("cannot-parse-OMCI-packet")
- }
-
onuLogger.WithFields(log.Fields{
- "omciMsgType": omciMsg.MessageType,
- "transCorrId": strconv.FormatInt(int64(omciMsg.TransactionID), 16),
- "DeviceIdent": omciMsg.DeviceIdentifier,
+ "omciMsgType": msg.OmciMsg.MessageType,
+ "transCorrId": strconv.FormatInt(int64(msg.OmciMsg.TransactionID), 16),
+ "DeviceIdent": msg.OmciMsg.DeviceIdentifier,
"IntfId": o.PonPortID,
"SerialNumber": o.Sn(),
}).Trace("omci-message-decoded")
var responsePkt []byte
var errResp error
- switch omciMsg.MessageType {
+ switch msg.OmciMsg.MessageType {
case omci.MibResetRequestType:
onuLogger.WithFields(log.Fields{
"IntfId": o.PonPortID,
"OnuId": o.ID,
"SerialNumber": o.Sn(),
}).Debug("received-mib-reset-request-resetting-mds")
- if responsePkt, errResp = omcilib.CreateMibResetResponse(omciMsg.TransactionID); errResp == nil {
+ if responsePkt, errResp = omcilib.CreateMibResetResponse(msg.OmciMsg.TransactionID); errResp == nil {
o.MibDataSync = 0
}
case omci.MibUploadRequestType:
- responsePkt, _ = omcilib.CreateMibUploadResponse(omciMsg.TransactionID)
+ responsePkt, _ = omcilib.CreateMibUploadResponse(msg.OmciMsg.TransactionID)
case omci.MibUploadNextRequestType:
- responsePkt, _ = omcilib.CreateMibUploadNextResponse(omciPkt, omciMsg, o.MibDataSync)
+ responsePkt, _ = omcilib.CreateMibUploadNextResponse(msg.OmciPkt, msg.OmciMsg, o.MibDataSync)
case omci.GetRequestType:
- responsePkt, _ = omcilib.CreateGetResponse(omciPkt, omciMsg, o.SerialNumber, o.MibDataSync, o.ActiveImageEntityId, o.CommittedImageEntityId)
+ responsePkt, _ = omcilib.CreateGetResponse(msg.OmciPkt, msg.OmciMsg, o.SerialNumber, o.MibDataSync, o.ActiveImageEntityId, o.CommittedImageEntityId)
case omci.SetRequestType:
success := true
- msgObj, _ := omcilib.ParseSetRequest(omciPkt)
+ msgObj, _ := omcilib.ParseSetRequest(msg.OmciPkt)
switch msgObj.EntityClass {
case me.PhysicalPathTerminationPointEthernetUniClassID:
// if we're Setting a PPTP state
@@ -809,17 +800,17 @@
}
if success {
- if responsePkt, errResp = omcilib.CreateSetResponse(omciPkt, omciMsg, me.Success); errResp == nil {
+ if responsePkt, errResp = omcilib.CreateSetResponse(msg.OmciPkt, msg.OmciMsg, me.Success); errResp == nil {
o.MibDataSync++
}
} else {
- responsePkt, _ = omcilib.CreateSetResponse(omciPkt, omciMsg, me.AttributeFailure)
+ responsePkt, _ = omcilib.CreateSetResponse(msg.OmciPkt, msg.OmciMsg, me.AttributeFailure)
}
case omci.CreateRequestType:
// check for GemPortNetworkCtp and make sure there are no duplicates on the same PON
var used bool
var sn *openolt.SerialNumber
- msgObj, err := omcilib.ParseCreateRequest(omciPkt)
+ msgObj, err := omcilib.ParseCreateRequest(msg.OmciPkt)
if err == nil {
if msgObj.EntityClass == me.GemPortNetworkCtpClassID {
if used, sn = o.PonPort.isGemPortAllocated(msgObj.EntityInstance); used {
@@ -846,14 +837,14 @@
// for now the CreateRequeste for the gemPort is the only one that can fail, if we start supporting multiple
// validation this check will need to be rewritten
if !used {
- if responsePkt, errResp = omcilib.CreateCreateResponse(omciPkt, omciMsg, me.Success); errResp == nil {
+ if responsePkt, errResp = omcilib.CreateCreateResponse(msg.OmciPkt, msg.OmciMsg, me.Success); errResp == nil {
o.MibDataSync++
}
} else {
- responsePkt, _ = omcilib.CreateCreateResponse(omciPkt, omciMsg, me.ProcessingError)
+ responsePkt, _ = omcilib.CreateCreateResponse(msg.OmciPkt, msg.OmciMsg, me.ProcessingError)
}
case omci.DeleteRequestType:
- msgObj, err := omcilib.ParseDeleteRequest(omciPkt)
+ msgObj, err := omcilib.ParseDeleteRequest(msg.OmciPkt)
if err == nil {
if msgObj.EntityClass == me.GemPortNetworkCtpClassID {
onuLogger.WithFields(log.Fields{
@@ -866,12 +857,12 @@
}
}
- if responsePkt, errResp = omcilib.CreateDeleteResponse(omciPkt, omciMsg); errResp == nil {
+ if responsePkt, errResp = omcilib.CreateDeleteResponse(msg.OmciPkt, msg.OmciMsg); errResp == nil {
o.MibDataSync++
}
case omci.RebootRequestType:
- responsePkt, _ = omcilib.CreateRebootResponse(omciPkt, omciMsg)
+ responsePkt, _ = omcilib.CreateRebootResponse(msg.OmciPkt, msg.OmciMsg)
// powercycle the ONU
// we run this in a separate goroutine so that
@@ -893,14 +884,14 @@
// second packet, TestResult, reports the result of running the self-test
// TestResult can come some time after a TestResponse
// TODO: Implement some delay between the TestResponse and the TestResult
- isTest, err := omcilib.IsTestRequest(msg.OmciMsg.Pkt)
+ isTest, err := omcilib.IsTestRequest(msg.OmciPkt.Data())
if (err == nil) && (isTest) {
if sendErr := o.sendTestResult(msg, stream); sendErr != nil {
onuLogger.WithFields(log.Fields{
"IntfId": o.PonPortID,
"OnuId": o.ID,
"SerialNumber": o.Sn(),
- "omciPacket": msg.OmciMsg.Pkt,
+ "omciPacket": msg.OmciPkt.Data(),
"msg": msg,
"err": sendErr,
}).Error("send-TestResult-indication-failed")
@@ -908,14 +899,14 @@
}
case omci.SynchronizeTimeRequestType:
// MDS counter increment is not required for this message type
- responsePkt, _ = omcilib.CreateSyncTimeResponse(omciPkt, omciMsg)
+ responsePkt, _ = omcilib.CreateSyncTimeResponse(msg.OmciPkt, msg.OmciMsg)
case omci.StartSoftwareDownloadRequestType:
o.ImageSoftwareReceivedSections = 0
- o.ImageSoftwareExpectedSections = omcilib.ComputeDownloadSectionsCount(omciPkt)
+ o.ImageSoftwareExpectedSections = omcilib.ComputeDownloadSectionsCount(msg.OmciPkt)
- if responsePkt, errResp = omcilib.CreateStartSoftwareDownloadResponse(omciPkt, omciMsg); errResp == nil {
+ if responsePkt, errResp = omcilib.CreateStartSoftwareDownloadResponse(msg.OmciPkt, msg.OmciMsg); errResp == nil {
o.MibDataSync++
if err := o.InternalState.Event(OnuTxStartImageDownload); err != nil {
onuLogger.WithFields(log.Fields{
@@ -927,18 +918,18 @@
}
} else {
onuLogger.WithFields(log.Fields{
- "OmciMsgType": omciMsg.MessageType,
- "TransCorrId": omciMsg.TransactionID,
- "Err": err.Error(),
+ "OmciMsgType": msg.OmciMsg.MessageType,
+ "TransCorrId": msg.OmciMsg.TransactionID,
+ "Err": errResp.Error(),
"IntfId": o.PonPortID,
"SerialNumber": o.Sn(),
}).Error("error-while-processing-start-software-download-request")
}
case omci.DownloadSectionRequestType:
- if msgObj, err := omcilib.ParseDownloadSectionRequest(omciPkt); err == nil {
+ if msgObj, err := omcilib.ParseDownloadSectionRequest(msg.OmciPkt); err == nil {
onuLogger.WithFields(log.Fields{
- "OmciMsgType": omciMsg.MessageType,
- "TransCorrId": omciMsg.TransactionID,
+ "OmciMsgType": msg.OmciMsg.MessageType,
+ "TransCorrId": msg.OmciMsg.TransactionID,
"EntityInstance": msgObj.EntityInstance,
"SectionNumber": msgObj.SectionNumber,
"SectionData": msgObj.SectionData,
@@ -957,12 +948,12 @@
}
case omci.DownloadSectionRequestWithResponseType:
// NOTE we only need to respond if an ACK is requested
- responsePkt, err = omcilib.CreateDownloadSectionResponse(omciPkt, omciMsg)
- if err != nil {
+ responsePkt, errResp = omcilib.CreateDownloadSectionResponse(msg.OmciPkt, msg.OmciMsg)
+ if errResp != nil {
onuLogger.WithFields(log.Fields{
- "OmciMsgType": omciMsg.MessageType,
- "TransCorrId": omciMsg.TransactionID,
- "Err": err.Error(),
+ "OmciMsgType": msg.OmciMsg.MessageType,
+ "TransCorrId": msg.OmciMsg.TransactionID,
+ "Err": errResp.Error(),
"IntfId": o.PonPortID,
"SerialNumber": o.Sn(),
}).Error("error-while-processing-create-download-section-response")
@@ -988,7 +979,7 @@
}
if success {
- if responsePkt, errResp = omcilib.CreateEndSoftwareDownloadResponse(omciPkt, omciMsg, me.Success); errResp == nil {
+ if responsePkt, errResp = omcilib.CreateEndSoftwareDownloadResponse(msg.OmciPkt, msg.OmciMsg, me.Success); errResp == nil {
o.MibDataSync++
if err := o.InternalState.Event(OnuTxCompleteImageDownload); err != nil {
onuLogger.WithFields(log.Fields{
@@ -1000,15 +991,15 @@
}
} else {
onuLogger.WithFields(log.Fields{
- "OmciMsgType": omciMsg.MessageType,
- "TransCorrId": omciMsg.TransactionID,
- "Err": err.Error(),
+ "OmciMsgType": msg.OmciMsg.MessageType,
+ "TransCorrId": msg.OmciMsg.TransactionID,
+ "Err": errResp.Error(),
"IntfId": o.PonPortID,
"SerialNumber": o.Sn(),
}).Error("error-while-processing-end-software-download-request")
}
} else {
- if responsePkt, errResp = omcilib.CreateEndSoftwareDownloadResponse(omciPkt, omciMsg, me.ProcessingError); errResp == nil {
+ if responsePkt, errResp = omcilib.CreateEndSoftwareDownloadResponse(msg.OmciPkt, msg.OmciMsg, me.ProcessingError); errResp == nil {
if err := o.InternalState.Event(OnuTxFailImageDownload); err != nil {
onuLogger.WithFields(log.Fields{
"OnuId": o.ID,
@@ -1021,7 +1012,7 @@
}
case omci.ActivateSoftwareRequestType:
- if responsePkt, errResp = omcilib.CreateActivateSoftwareResponse(omciPkt, omciMsg); errResp == nil {
+ if responsePkt, errResp = omcilib.CreateActivateSoftwareResponse(msg.OmciPkt, msg.OmciMsg); errResp == nil {
o.MibDataSync++
if err := o.InternalState.Event(OnuTxActivateImage); err != nil {
onuLogger.WithFields(log.Fields{
@@ -1031,7 +1022,7 @@
"Err": err.Error(),
}).Errorf("cannot-change-onu-internal-state-to-%s", OnuStateImageActivated)
}
- if msgObj, err := omcilib.ParseActivateSoftwareRequest(omciPkt); err == nil {
+ if msgObj, err := omcilib.ParseActivateSoftwareRequest(msg.OmciPkt); err == nil {
o.ActiveImageEntityId = msgObj.EntityInstance
} else {
onuLogger.Errorf("something-went-wrong-while-activating: %s", err)
@@ -1060,9 +1051,9 @@
}()
}
case omci.CommitSoftwareRequestType:
- if responsePkt, errResp = omcilib.CreateCommitSoftwareResponse(omciPkt, omciMsg); errResp == nil {
+ if responsePkt, errResp = omcilib.CreateCommitSoftwareResponse(msg.OmciPkt, msg.OmciMsg); errResp == nil {
o.MibDataSync++
- if msgObj, err := omcilib.ParseCommitSoftwareRequest(omciPkt); err == nil {
+ if msgObj, err := omcilib.ParseCommitSoftwareRequest(msg.OmciPkt); err == nil {
// TODO validate that the image to commit is:
// - active
// - not already committed
@@ -1095,30 +1086,30 @@
o.onuAlarmsInfo[key] = alarmInfo
}
o.onuAlarmsInfoLock.Unlock()
- responsePkt, _ = omcilib.CreateGetAllAlarmsResponse(omciMsg.TransactionID, o.onuAlarmsInfo)
+ responsePkt, _ = omcilib.CreateGetAllAlarmsResponse(msg.OmciMsg.TransactionID, o.onuAlarmsInfo)
case omci.GetAllAlarmsNextRequestType:
- if responsePkt, errResp = omcilib.CreateGetAllAlarmsNextResponse(omciPkt, omciMsg, o.onuAlarmsInfo); errResp != nil {
+ if responsePkt, errResp = omcilib.CreateGetAllAlarmsNextResponse(msg.OmciPkt, msg.OmciMsg, o.onuAlarmsInfo); errResp != nil {
responsePkt = nil //Do not send any response for error case
}
default:
onuLogger.WithFields(log.Fields{
- "omciBytes": hex.EncodeToString(omciPkt.Data()),
- "omciPkt": omciPkt,
- "omciMsgType": omciMsg.MessageType,
- "transCorrId": omciMsg.TransactionID,
+ "omciBytes": hex.EncodeToString(msg.OmciPkt.Data()),
+ "omciPkt": msg.OmciPkt,
+ "omciMsgType": msg.OmciMsg.MessageType,
+ "transCorrId": msg.OmciMsg.TransactionID,
"IntfId": o.PonPortID,
"SerialNumber": o.Sn(),
}).Warnf("OMCI-message-not-supported")
}
if responsePkt != nil {
- if err := o.sendOmciIndication(responsePkt, omciMsg.TransactionID, stream); err != nil {
+ if err := o.sendOmciIndication(responsePkt, msg.OmciMsg.TransactionID, stream); err != nil {
onuLogger.WithFields(log.Fields{
- "IntfId": o.PonPortID,
- "SerialNumber": o.Sn(),
- "omciPacket": responsePkt,
- "omciMsgType": omciMsg.MessageType,
- "transCorrId": omciMsg.TransactionID,
+ "IntfId": o.PonPortID,
+ "SerialNumber": o.Sn(),
+ "omciPacket": responsePkt,
+ "msg.OmciMsgType": msg.OmciMsg.MessageType,
+ "transCorrId": msg.OmciMsg.TransactionID,
}).Errorf("failed-to-send-omci-message: %v", err)
}
}
diff --git a/internal/bbsim/devices/onu_omci_test.go b/internal/bbsim/devices/onu_omci_test.go
index 4d706b9..246514b 100644
--- a/internal/bbsim/devices/onu_omci_test.go
+++ b/internal/bbsim/devices/onu_omci_test.go
@@ -109,14 +109,15 @@
}
func makeOmciMessage(t *testing.T, onu *Onu, pkt []byte) bbsim.OmciMessage {
+ omciPkt, omciMsg, err := omcilib.ParseOpenOltOmciPacket(pkt)
+ if err != nil {
+ t.Fatal(err.Error())
+ }
return bbsim.OmciMessage{
- OnuSN: onu.SerialNumber,
- OnuID: onu.ID,
- OmciMsg: &openolt.OmciMsg{
- IntfId: onu.PonPortID,
- OnuId: onu.ID,
- Pkt: pkt,
- },
+ OnuSN: onu.SerialNumber,
+ OnuID: onu.ID,
+ OmciPkt: omciPkt,
+ OmciMsg: omciMsg,
}
}
diff --git a/internal/bbsim/types/messageTypes.go b/internal/bbsim/types/messageTypes.go
index 4068dfa..1723009 100644
--- a/internal/bbsim/types/messageTypes.go
+++ b/internal/bbsim/types/messageTypes.go
@@ -19,6 +19,7 @@
import (
"github.com/google/gopacket"
"github.com/opencord/bbsim/internal/bbsim/packetHandlers"
+ "github.com/opencord/omci-lib-go"
"github.com/opencord/voltha-protos/v4/go/openolt"
"net"
)
@@ -113,7 +114,8 @@
type OmciMessage struct {
OnuSN *openolt.SerialNumber
OnuID uint32
- OmciMsg *openolt.OmciMsg
+ OmciMsg *omci.OMCI
+ OmciPkt gopacket.Packet
}
type UniStatusAlarmMessage struct {