[SEBA-918][VOL-2984] Allowing DHCP without completing eapol
For DT and TT workflow is required that DHCP is triggered without doing
anything for EAPOL. If you start BBSim with only the "-dhcp" option that
is now possible.
Change-Id: Iacfb75e70c47a2f7cfa64af58d6d848881f54974
diff --git a/VERSION b/VERSION
index 7179039..1610241 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1,2 @@
-0.2.3
+0.2.3-dev
+
diff --git a/docs/source/index.rst b/docs/source/index.rst
index 5c2ebf2..296d42f 100644
--- a/docs/source/index.rst
+++ b/docs/source/index.rst
@@ -247,43 +247,45 @@
.. code:: json
{
- "id": "BBSM00000003-1",
- "nasPortId": "BBSM00000003-1",
- "circuitId": "BBSM00000003-1",
- "remoteId": "BBSM00000003-1",
- "uniTagList": [
- {
- "DownstreamBandwidthProfile": "User_Bandwidth1",
- "IsDhcpRequired": true,
- "IsIgmpRequired": true,
- "PonCTag": 903,
- "PonSTag": 900,
- "TechnologyProfileID": 64,
- "UpstreamBandwidthProfile": "Default"
- }
- ]
-}
+ "id": "BBSM00000003-1",
+ "nasPortId": "BBSM00000003-1",
+ "circuitId": "BBSM00000003-1",
+ "remoteId": "BBSM00000003-1",
+ "uniTagList": [
+ {
+ "DownstreamBandwidthProfile": "User_Bandwidth1",
+ "IsDhcpRequired": true,
+ "IsIgmpRequired": true,
+ "PonCTag": 903,
+ "PonSTag": 900,
+ "TechnologyProfileID": 64,
+ "UpstreamBandwidthProfile": "Default"
+ }
+ ]
+ }
+
**DT**
.. code:: json
{
- "id": "BBSM00000003-1",
- "nasPortId": "BBSM00000003-1",
- "circuitId": "BBSM00000003-1",
- "remoteId": "BBSM00000003-1",
- "uniTagList": [
- {
- "DownstreamBandwidthProfile": "User_Bandwidth1",
- "PonCTag": 4096,
- "PonSTag": 903,
- "TechnologyProfileID": 64,
- "UniTagMatch": 4096,
- "UpstreamBandwidthProfile": "Default"
- }
- ]
-}
+ "id": "BBSM00000003-1",
+ "nasPortId": "BBSM00000003-1",
+ "circuitId": "BBSM00000003-1",
+ "remoteId": "BBSM00000003-1",
+ "uniTagList": [
+ {
+ "DownstreamBandwidthProfile": "User_Bandwidth1",
+ "PonCTag": 4096,
+ "PonSTag": 903,
+ "TechnologyProfileID": 64,
+ "UniTagMatch": 4096,
+ "UpstreamBandwidthProfile": "Default"
+ }
+ ]
+ }
+
**TT**
diff --git a/docs/source/onu-state-machine.rst b/docs/source/onu-state-machine.rst
index 7117e57..a7adb9e 100644
--- a/docs/source/onu-state-machine.rst
+++ b/docs/source/onu-state-machine.rst
@@ -32,18 +32,10 @@
- discovered, disabled, pon_disabled
- enabled
-
- * - receive_eapol_flow
- - enabled, gem_port_added
- - eapol_flow_received
- -
- * - add_gem_port
- - enabled, eapol_flow_received
- - gem_port_added
- - We need to wait for both the flow and the gem port to come before moving to ``auth_started``
* - start_auth
- - eapol_flow_received, gem_port_added, eap_start_sent, eap_response_identity_sent, eap_response_challenge_sent, eap_response_success_received, auth_failed, dhcp_ack_received, dhcp_failed
+ - enabled, eap_start_sent, eap_response_identity_sent, eap_response_challenge_sent, eap_response_success_received, auth_failed, dhcp_ack_received, dhcp_failed
- auth_started
- -
+ - Requires that both the EAPOL flow has been received and the GemPort added
* - eap_start_sent
- auth_started
- eap_start_sent
@@ -65,9 +57,9 @@
- auth_failed
-
* - start_dhcp
- - eap_response_success_received, dhcp_discovery_sent, dhcp_request_sent, dhcp_ack_received, dhcp_failed
+ - enabled, eap_response_success_received, dhcp_discovery_sent, dhcp_request_sent, dhcp_ack_received, dhcp_failed
- dhcp_started
- -
+ - Requires that both the DHCP flow has been received and the GemPort added. In addition the transtition from the ``enabled`` state is only allowed if Auth is set to false
* - dhcp_discovery_sent
- dhcp_started
- dhcp_discovery_sent
@@ -169,7 +161,6 @@
enabled
disabled [fillcolor="#f9d6ff"]
}
- gem_port_added
{created, disabled} -> initialized -> discovered -> enabled
}
@@ -179,7 +170,6 @@
style=dotted
node [fillcolor="#e6ffc2"]
- eapol_flow_received
auth_started
eap_start_sent
eap_response_identity_sent
@@ -229,8 +219,8 @@
dhcp_ack_received -> dhcp_started
dhcp_failed -> dhcp_started
}
- enabled -> gem_port_added -> eapol_flow_received -> auth_started
- enabled -> eapol_flow_received -> gem_port_added -> auth_started
+ enabled -> auth_started
+ enabled -> dhcp_started
{dhcp_ack_received, dhcp_failed} -> auth_started
diff --git a/internal/bbsim/devices/olt.go b/internal/bbsim/devices/olt.go
index fae3dd5..a2e3229 100644
--- a/internal/bbsim/devices/olt.go
+++ b/internal/bbsim/devices/olt.go
@@ -680,26 +680,26 @@
if err := pon.OperState.Event("enable"); err != nil {
oltLogger.WithFields(log.Fields{
"IntfId": msg.PonPortID,
- "Err": err,
+ "Err": err,
}).Error("Can't Enable Oper state for PON Port")
}
if err := pon.InternalState.Event("enable"); err != nil {
oltLogger.WithFields(log.Fields{
"IntfId": msg.PonPortID,
- "Err": err,
+ "Err": err,
}).Error("Can't Enable Internal state for PON Port")
}
} else if msg.OperState == DOWN {
if err := pon.OperState.Event("disable"); err != nil {
oltLogger.WithFields(log.Fields{
"IntfId": msg.PonPortID,
- "Err": err,
+ "Err": err,
}).Error("Can't Disable Oper state for PON Port")
}
if err := pon.InternalState.Event("disable"); err != nil {
oltLogger.WithFields(log.Fields{
"IntfId": msg.PonPortID,
- "Err": err,
+ "Err": err,
}).Error("Can't Disable Internal state for PON Port")
}
}
diff --git a/internal/bbsim/devices/onu.go b/internal/bbsim/devices/onu.go
index ecb91f8..6b879cc 100644
--- a/internal/bbsim/devices/onu.go
+++ b/internal/bbsim/devices/onu.go
@@ -65,14 +65,17 @@
// PortNo comes with flows and it's used when sending packetIndications,
// There is one PortNo per UNI Port, for now we're only storing the first one
// FIXME add support for multiple UNIs (each UNI has a different PortNo)
- PortNo uint32
- DhcpFlowReceived bool
- Flows []FlowKey
+ PortNo uint32
+ GemPortAdded bool
+ EapolFlowReceived bool
+ DhcpFlowReceived bool
+ Flows []FlowKey
OperState *fsm.FSM
SerialNumber *openolt.SerialNumber
- Channel chan Message // this Channel is to track state changes OMCI messages, EAPOL and DHCP packets
+ Channel chan Message // this Channel is to track state changes OMCI messages, EAPOL and DHCP packets
+ GemPortChannels []chan bool // this channels are used to notify everyone that is interested that a GemPort has been added
// OMCI params
tid uint16
@@ -88,6 +91,12 @@
return common.OnuSnToString(o.SerialNumber)
}
+func (o *Onu) GetGemPortChan() chan bool {
+ listener := make(chan bool, 1)
+ o.GemPortChannels = append(o.GemPortChannels, listener)
+ return listener
+}
+
func CreateONU(olt *OltDevice, pon PonPort, id uint32, sTag int, cTag int, auth bool, dhcp bool, delay time.Duration, isMock bool) *Onu {
o := Onu{
@@ -105,6 +114,8 @@
seqNumber: 0,
DoneChannel: make(chan bool, 1),
DhcpFlowReceived: false,
+ EapolFlowReceived: false,
+ GemPortAdded: false,
DiscoveryRetryDelay: 60 * time.Second, // this is used to send OnuDiscoveryIndications until an activate call is received
Flows: []FlowKey{},
DiscoveryDelay: delay,
@@ -130,18 +141,18 @@
{Name: "receive_eapol_flow", Src: []string{"enabled", "gem_port_added"}, Dst: "eapol_flow_received"},
{Name: "add_gem_port", Src: []string{"enabled", "eapol_flow_received"}, Dst: "gem_port_added"},
// NOTE should disabled state be different for oper_disabled (emulating an error) and admin_disabled (received a disabled call via VOLTHA)?
- {Name: "disable", Src: []string{"enabled", "eapol_flow_received", "gem_port_added", "eap_response_success_received", "auth_failed", "dhcp_ack_received", "dhcp_failed", "pon_disabled"}, Dst: "disabled"},
+ {Name: "disable", Src: []string{"enabled", "eap_response_success_received", "auth_failed", "dhcp_ack_received", "dhcp_failed", "pon_disabled"}, Dst: "disabled"},
// ONU state when PON port is disabled but ONU is power ON(more states should be added in src?)
- {Name: "pon_disabled", Src: []string{"enabled", "gem_port_added", "eapol_flow_received", "eap_response_success_received", "auth_failed", "dhcp_ack_received", "dhcp_failed"}, Dst: "pon_disabled"},
+ {Name: "pon_disabled", Src: []string{"enabled", "eap_response_success_received", "auth_failed", "dhcp_ack_received", "dhcp_failed"}, Dst: "pon_disabled"},
// EAPOL
- {Name: "start_auth", Src: []string{"eapol_flow_received", "gem_port_added", "eap_start_sent", "eap_response_identity_sent", "eap_response_challenge_sent", "eap_response_success_received", "auth_failed", "dhcp_ack_received", "dhcp_failed", "igmp_join_started", "igmp_left", "igmp_join_error"}, Dst: "auth_started"},
+ {Name: "start_auth", Src: []string{"enabled", "eap_start_sent", "eap_response_identity_sent", "eap_response_challenge_sent", "eap_response_success_received", "auth_failed", "dhcp_ack_received", "dhcp_failed", "igmp_join_started", "igmp_left", "igmp_join_error"}, Dst: "auth_started"},
{Name: "eap_start_sent", Src: []string{"auth_started"}, Dst: "eap_start_sent"},
{Name: "eap_response_identity_sent", Src: []string{"eap_start_sent"}, Dst: "eap_response_identity_sent"},
{Name: "eap_response_challenge_sent", Src: []string{"eap_response_identity_sent"}, Dst: "eap_response_challenge_sent"},
{Name: "eap_response_success_received", Src: []string{"eap_response_challenge_sent"}, Dst: "eap_response_success_received"},
{Name: "auth_failed", Src: []string{"auth_started", "eap_start_sent", "eap_response_identity_sent", "eap_response_challenge_sent"}, Dst: "auth_failed"},
// DHCP
- {Name: "start_dhcp", Src: []string{"eap_response_success_received", "dhcp_discovery_sent", "dhcp_request_sent", "dhcp_ack_received", "dhcp_failed", "igmp_join_started", "igmp_left", "igmp_join_error"}, Dst: "dhcp_started"},
+ {Name: "start_dhcp", Src: []string{"enabled", "eap_response_success_received", "dhcp_discovery_sent", "dhcp_request_sent", "dhcp_ack_received", "dhcp_failed", "igmp_join_started", "igmp_left", "igmp_join_error"}, Dst: "dhcp_started"},
{Name: "dhcp_discovery_sent", Src: []string{"dhcp_started"}, Dst: "dhcp_discovery_sent"},
{Name: "dhcp_request_sent", Src: []string{"dhcp_discovery_sent"}, Dst: "dhcp_request_sent"},
{Name: "dhcp_ack_received", Src: []string{"dhcp_request_sent"}, Dst: "dhcp_ack_received"},
@@ -151,10 +162,10 @@
{Name: "send_eapol_flow", Src: []string{"initialized"}, Dst: "eapol_flow_sent"},
{Name: "send_dhcp_flow", Src: []string{"eapol_flow_sent"}, Dst: "dhcp_flow_sent"},
// IGMP
- {Name: "igmp_join_start", Src: []string{"eap_response_success_received", "gem_port_added", "eapol_flow_received", "dhcp_ack_received", "igmp_left", "igmp_join_error", "igmp_join_started"}, Dst: "igmp_join_started"},
- {Name: "igmp_join_startv3", Src: []string{"eap_response_success_received", "gem_port_added", "eapol_flow_received", "dhcp_ack_received", "igmp_left", "igmp_join_error", "igmp_join_started"}, Dst: "igmp_join_started"},
+ {Name: "igmp_join_start", Src: []string{"eap_response_success_received", "dhcp_ack_received", "igmp_left", "igmp_join_error", "igmp_join_started"}, Dst: "igmp_join_started"},
+ {Name: "igmp_join_startv3", Src: []string{"eap_response_success_received", "dhcp_ack_received", "igmp_left", "igmp_join_error", "igmp_join_started"}, Dst: "igmp_join_started"},
{Name: "igmp_join_error", Src: []string{"igmp_join_started"}, Dst: "igmp_join_error"},
- {Name: "igmp_leave", Src: []string{"igmp_join_started", "gem_port_added", "eapol_flow_received", "eap_response_success_received", "dhcp_ack_received"}, Dst: "igmp_left"},
+ {Name: "igmp_leave", Src: []string{"igmp_join_started", "eap_response_success_received", "dhcp_ack_received"}, Dst: "igmp_left"},
},
fsm.Callbacks{
"enter_state": func(e *fsm.Event) {
@@ -227,6 +238,16 @@
// terminate the ONU's ProcessOnuMessages Go routine
close(o.Channel)
},
+ "before_start_auth": func(e *fsm.Event) {
+ if o.EapolFlowReceived == false {
+ e.Cancel(errors.New("cannot-go-to-auth-started-as-eapol-flow-is-missing"))
+ return
+ }
+ if o.GemPortAdded == false {
+ e.Cancel(errors.New("cannot-go-to-auth-started-as-gemport-is-missing"))
+ return
+ }
+ },
"enter_auth_started": func(e *fsm.Event) {
o.logStateChange(e.Src, e.Dst)
msg := Message{
@@ -249,8 +270,21 @@
}).Errorf("ONU failed to authenticate!")
},
"before_start_dhcp": func(e *fsm.Event) {
+
+ // we allow transition from eanbled to dhcp_started only if auth was set to false
+ if o.InternalState.Current() == "enabled" && o.Auth {
+ e.Cancel(errors.New("cannot-go-to-dhcp-started-as-authentication-is-required"))
+ return
+ }
+
if o.DhcpFlowReceived == false {
e.Cancel(errors.New("cannot-go-to-dhcp-started-as-dhcp-flow-is-missing"))
+ return
+ }
+
+ if o.GemPortAdded == false {
+ e.Cancel(errors.New("cannot-go-to-dhcp-started-as-gemport-is-missing"))
+ return
}
},
"enter_dhcp_started": func(e *fsm.Event) {
@@ -471,25 +505,16 @@
"OnuSn": o.Sn(),
}).Infof("GemPort Added")
- // NOTE if we receive the GemPort but we don't have EAPOL flows
- // go an intermediate state, otherwise start auth
- if o.InternalState.Is("enabled") {
- if err := o.InternalState.Event("add_gem_port"); err != nil {
- log.Errorf("Can't go to gem_port_added: %v", err)
- }
- } else if o.InternalState.Is("eapol_flow_received") {
- if o.Auth == true {
- if err := o.InternalState.Event("start_auth"); err != nil {
- log.Warnf("Can't go to auth_started: %v", err)
- }
- } else {
- onuLogger.WithFields(log.Fields{
- "IntfId": o.PonPortID,
- "OnuId": o.ID,
- "SerialNumber": o.Sn(),
- }).Warn("Not starting authentication as Auth bit is not set in CLI parameters")
- }
+ o.GemPortAdded = true
+
+ // broadcast the change to all listeners
+ // and close the channels as once the GemPort is set
+ // it won't change anymore
+ for _, ch := range o.GemPortChannels {
+ ch <- true
+ close(ch)
}
+ o.GemPortChannels = []chan bool{}
}
}
@@ -721,34 +746,41 @@
if msg.Flow.Classifier.EthType == uint32(layers.EthernetTypeEAPOL) && msg.Flow.Classifier.OVid == 4091 {
// NOTE storing the PortNO, it's needed when sending PacketIndications
o.storePortNumber(uint32(msg.Flow.PortNo))
-
- // NOTE if we receive the EAPOL flows but we don't have GemPorts
- // go an intermediate state, otherwise start auth
- if o.InternalState.Is("enabled") {
- if err := o.InternalState.Event("receive_eapol_flow"); err != nil {
- log.Warnf("Can't go to eapol_flow_received: %v", err)
- }
- } else if o.InternalState.Is("gem_port_added") {
-
- if o.Auth == true {
- if err := o.InternalState.Event("start_auth"); err != nil {
- log.Warnf("Can't go to auth_started: %v", err)
- }
+ o.EapolFlowReceived = true
+ // if authentication is not enabled, do nothing
+ if o.Auth {
+ // NOTE if we receive the EAPOL flows but we don't have GemPorts
+ // wait for it before starting auth
+ if !o.GemPortAdded {
+ // wait for Gem and then start auth
+ go func() {
+ for v := range o.GetGemPortChan() {
+ if v == true {
+ if err := o.InternalState.Event("start_auth"); err != nil {
+ onuLogger.Warnf("Can't go to auth_started: %v", err)
+ }
+ }
+ }
+ onuLogger.Trace("GemPortChannel closed")
+ }()
} else {
- onuLogger.WithFields(log.Fields{
- "IntfId": o.PonPortID,
- "OnuId": o.ID,
- "SerialNumber": o.Sn(),
- }).Warn("Not starting authentication as Auth bit is not set in CLI parameters")
+ // start the EAPOL state machine
+ if err := o.InternalState.Event("start_auth"); err != nil {
+ onuLogger.Warnf("Can't go to auth_started: %v", err)
+ }
}
-
+ } else {
+ onuLogger.WithFields(log.Fields{
+ "IntfId": o.PonPortID,
+ "OnuId": o.ID,
+ "SerialNumber": o.Sn(),
+ }).Warn("Not starting authentication as Auth bit is not set in CLI parameters")
}
} else if msg.Flow.Classifier.EthType == uint32(layers.EthernetTypeIPv4) &&
msg.Flow.Classifier.SrcPort == uint32(68) &&
msg.Flow.Classifier.DstPort == uint32(67) &&
(msg.Flow.Classifier.OPbits == 0 || msg.Flow.Classifier.OPbits == 255) {
-
if o.Dhcp == true {
if o.DhcpFlowReceived == false {
// keep track that we received the DHCP Flows
@@ -756,9 +788,22 @@
// this is needed as a check in case someone trigger DHCP from the CLI
o.DhcpFlowReceived = true
- // now start the DHCP state machine
- if err := o.InternalState.Event("start_dhcp"); err != nil {
- log.Errorf("Can't go to dhcp_started: %v", err)
+ if !o.GemPortAdded {
+ // wait for Gem and then start DHCP
+ go func() {
+ for v := range o.GetGemPortChan() {
+ if v == true {
+ if err := o.InternalState.Event("start_dhcp"); err != nil {
+ log.Errorf("Can't go to dhcp_started: %v", err)
+ }
+ }
+ }
+ }()
+ } else {
+ // start the DHCP state machine
+ if err := o.InternalState.Event("start_dhcp"); err != nil {
+ log.Errorf("Can't go to dhcp_started: %v", err)
+ }
}
} else {
onuLogger.WithFields(log.Fields{
diff --git a/internal/bbsim/devices/onu_flow_test.go b/internal/bbsim/devices/onu_flow_test.go
index e8fa451..9ab8abb 100644
--- a/internal/bbsim/devices/onu_flow_test.go
+++ b/internal/bbsim/devices/onu_flow_test.go
@@ -21,7 +21,9 @@
"github.com/looplab/fsm"
"github.com/opencord/voltha-protos/v2/go/openolt"
"gotest.tools/assert"
+ "sync"
"testing"
+ "time"
)
func Test_Onu_SendEapolFlow(t *testing.T) {
@@ -46,15 +48,16 @@
}
// validates that when an ONU receives an EAPOL flow for UNI 0
+// and the GemPort has already been configured
// it transition to auth_started state
-func Test_HandleFlowUpdateEapolFromGem(t *testing.T) {
+func Test_HandleFlowUpdateEapolWithGem(t *testing.T) {
onu := createMockOnu(1, 1, 900, 900, true, false)
onu.InternalState = fsm.NewFSM(
- "gem_port_added",
+ "enabled",
fsm.Events{
- {Name: "start_auth", Src: []string{"eapol_flow_received", "gem_port_added"}, Dst: "auth_started"},
+ {Name: "start_auth", Src: []string{"enabled"}, Dst: "auth_started"},
},
fsm.Callbacks{},
)
@@ -87,97 +90,15 @@
}
// validates that when an ONU receives an EAPOL flow for UNI that is not 0
-// no action is taken
-func Test_HandleFlowUpdateEapolFromGemIgnore(t *testing.T) {
+// no action is taken (this is independent of GemPort status
+func Test_HandleFlowUpdateEapolWrongUNI(t *testing.T) {
- onu := createMockOnu(1, 1, 900, 900, false, false)
-
- onu.InternalState = fsm.NewFSM(
- "gem_port_added",
- fsm.Events{
- {Name: "start_auth", Src: []string{"eapol_flow_received", "gem_port_added"}, Dst: "auth_started"},
- },
- fsm.Callbacks{},
- )
-
- flow := openolt.Flow{
- AccessIntfId: int32(onu.PonPortID),
- OnuId: int32(onu.ID),
- UniId: int32(1),
- FlowId: uint32(onu.ID),
- FlowType: "downstream",
- AllocId: int32(0),
- NetworkIntfId: int32(0),
- Classifier: &openolt.Classifier{
- EthType: uint32(layers.EthernetTypeEAPOL),
- OVid: 4091,
- },
- Action: &openolt.Action{},
- Priority: int32(100),
- PortNo: uint32(onu.ID), // NOTE we are using this to map an incoming packetIndication to an ONU
- }
-
- msg := OnuFlowUpdateMessage{
- PonPortID: 1,
- OnuID: 1,
- Flow: &flow,
- }
-
- onu.handleFlowUpdate(msg)
- assert.Equal(t, onu.InternalState.Current(), "gem_port_added")
-}
-
-// validates that when an ONU receives an EAPOL flow for UNI 0
-// it transition to auth_started state
-func Test_HandleFlowUpdateEapolFromEnabled(t *testing.T) {
-
- onu := createMockOnu(1, 1, 900, 900, false, false)
+ onu := createMockOnu(1, 1, 900, 900, true, false)
onu.InternalState = fsm.NewFSM(
"enabled",
fsm.Events{
- {Name: "receive_eapol_flow", Src: []string{"enabled", "gem_port_added"}, Dst: "eapol_flow_received"},
- },
- fsm.Callbacks{},
- )
-
- flow := openolt.Flow{
- AccessIntfId: int32(onu.PonPortID),
- OnuId: int32(onu.ID),
- UniId: int32(0),
- FlowId: uint32(onu.ID),
- FlowType: "downstream",
- AllocId: int32(0),
- NetworkIntfId: int32(0),
- Classifier: &openolt.Classifier{
- EthType: uint32(layers.EthernetTypeEAPOL),
- OVid: 4091,
- },
- Action: &openolt.Action{},
- Priority: int32(100),
- PortNo: uint32(onu.ID), // NOTE we are using this to map an incoming packetIndication to an ONU
- }
-
- msg := OnuFlowUpdateMessage{
- PonPortID: 1,
- OnuID: 1,
- Flow: &flow,
- }
-
- onu.handleFlowUpdate(msg)
- assert.Equal(t, onu.InternalState.Current(), "eapol_flow_received")
-}
-
-// validates that when an ONU receives an EAPOL flow for UNI that is not 0
-// no action is taken
-func Test_HandleFlowUpdateEapolFromEnabledIgnore(t *testing.T) {
-
- onu := createMockOnu(1, 1, 900, 900, false, false)
-
- onu.InternalState = fsm.NewFSM(
- "enabled",
- fsm.Events{
- {Name: "receive_eapol_flow", Src: []string{"enabled", "gem_port_added"}, Dst: "eapol_flow_received"},
+ {Name: "start_auth", Src: []string{"enabled"}, Dst: "auth_started"},
},
fsm.Callbacks{},
)
@@ -210,14 +131,17 @@
}
// validates that when an ONU receives an EAPOL flow for UNI 0
-// but the noAuth bit is set no action is taken
-func Test_HandleFlowUpdateEapolNoAuth(t *testing.T) {
- onu := createMockOnu(1, 1, 900, 900, false, false)
+// and the GemPort has not yet been configured
+// it transition to auth_started state
+func Test_HandleFlowUpdateEapolWithoutGem(t *testing.T) {
+
+ onu := createMockOnu(1, 1, 900, 900, true, false)
+ onu.GemPortAdded = false
onu.InternalState = fsm.NewFSM(
- "gem_port_added",
+ "enabled",
fsm.Events{
- {Name: "start_auth", Src: []string{"eapol_flow_received", "gem_port_added"}, Dst: "auth_started"},
+ {Name: "start_auth", Src: []string{"enabled"}, Dst: "auth_started"},
},
fsm.Callbacks{},
)
@@ -246,9 +170,68 @@
}
onu.handleFlowUpdate(msg)
- assert.Equal(t, onu.InternalState.Current(), "gem_port_added")
+
+ wg := sync.WaitGroup{}
+ wg.Add(1)
+ go func(wg *sync.WaitGroup) {
+ defer wg.Done()
+ time.Sleep(100 * time.Millisecond)
+
+ // emulate the addition of a GemPort
+ for _, ch := range onu.GemPortChannels {
+ ch <- true
+ }
+
+ time.Sleep(100 * time.Millisecond)
+ assert.Equal(t, onu.InternalState.Current(), "auth_started")
+ }(&wg)
+ wg.Wait()
+
}
+// validates that when an ONU receives an EAPOL flow for UNI 0
+// but the noAuth bit is set no action is taken
+func Test_HandleFlowUpdateEapolNoAuth(t *testing.T) {
+ onu := createMockOnu(1, 1, 900, 900, false, false)
+
+ onu.InternalState = fsm.NewFSM(
+ "enabled",
+ fsm.Events{
+ {Name: "start_auth", Src: []string{"enabled"}, Dst: "auth_started"},
+ },
+ fsm.Callbacks{},
+ )
+
+ flow := openolt.Flow{
+ AccessIntfId: int32(onu.PonPortID),
+ OnuId: int32(onu.ID),
+ UniId: int32(0),
+ FlowId: uint32(onu.ID),
+ FlowType: "downstream",
+ AllocId: int32(0),
+ NetworkIntfId: int32(0),
+ Classifier: &openolt.Classifier{
+ EthType: uint32(layers.EthernetTypeEAPOL),
+ OVid: 4091,
+ },
+ Action: &openolt.Action{},
+ Priority: int32(100),
+ PortNo: uint32(onu.ID), // NOTE we are using this to map an incoming packetIndication to an ONU
+ }
+
+ msg := OnuFlowUpdateMessage{
+ PonPortID: 1,
+ OnuID: 1,
+ Flow: &flow,
+ }
+
+ onu.handleFlowUpdate(msg)
+ assert.Equal(t, onu.InternalState.Current(), "enabled")
+}
+
+// validates that when an ONU receives a DHCP flow for UNI 0 and pbit 0
+// and the GemPort has already been configured
+// it transition to dhcp_started state
func Test_HandleFlowUpdateDhcp(t *testing.T) {
onu := createMockOnu(1, 1, 900, 900, false, true)
@@ -272,7 +255,7 @@
EthType: uint32(layers.EthernetTypeIPv4),
SrcPort: uint32(68),
DstPort: uint32(67),
- OPbits: 0,
+ OPbits: 0,
},
Action: &openolt.Action{},
Priority: int32(100),
@@ -290,6 +273,9 @@
assert.Equal(t, onu.DhcpFlowReceived, true)
}
+// validates that when an ONU receives a DHCP flow for UNI 0 and pbit 255
+// and the GemPort has already been configured
+// it transition to dhcp_started state
func Test_HandleFlowUpdateDhcpPBit255(t *testing.T) {
onu := createMockOnu(1, 1, 900, 900, false, true)
@@ -313,7 +299,7 @@
EthType: uint32(layers.EthernetTypeIPv4),
SrcPort: uint32(68),
DstPort: uint32(67),
- OPbits: 0,
+ OPbits: 255,
},
Action: &openolt.Action{},
Priority: int32(100),
@@ -331,6 +317,9 @@
assert.Equal(t, onu.DhcpFlowReceived, true)
}
+// validates that when an ONU receives a DHCP flow for UNI 0 and pbit not 0 or 255
+// and the GemPort has already been configured
+// it ignores the message
func Test_HandleFlowUpdateDhcpIgnoreByPbit(t *testing.T) {
onu := createMockOnu(1, 1, 900, 900, false, true)
@@ -354,7 +343,7 @@
EthType: uint32(layers.EthernetTypeIPv4),
SrcPort: uint32(68),
DstPort: uint32(67),
- OPbits: 1,
+ OPbits: 1,
},
Action: &openolt.Action{},
Priority: int32(100),
@@ -372,6 +361,8 @@
assert.Equal(t, onu.DhcpFlowReceived, false)
}
+// validates that when an ONU receives a DHCP flow for UNI 0
+// but the noDchp bit is set no action is taken
func Test_HandleFlowUpdateDhcpNoDhcp(t *testing.T) {
onu := createMockOnu(1, 1, 900, 900, false, false)
@@ -411,3 +402,66 @@
assert.Equal(t, onu.InternalState.Current(), "eap_response_success_received")
assert.Equal(t, onu.DhcpFlowReceived, false)
}
+
+// validates that when an ONU receives a DHCP flow for UNI 0 and pbit not 0 or 255
+// and the GemPort has not already been configured
+// it transition to dhcp_started state
+func Test_HandleFlowUpdateDhcpWithoutGem(t *testing.T) {
+ // NOTE that this feature is required when we do DHCP with no eapol
+ // as the DHCP flow will be the first one received
+ onu := createMockOnu(1, 1, 900, 900, false, true)
+
+ onu.GemPortAdded = false
+
+ onu.InternalState = fsm.NewFSM(
+ "enabled",
+ fsm.Events{
+ {Name: "start_dhcp", Src: []string{"enabled"}, Dst: "dhcp_started"},
+ },
+ fsm.Callbacks{},
+ )
+
+ flow := openolt.Flow{
+ AccessIntfId: int32(onu.PonPortID),
+ OnuId: int32(onu.ID),
+ UniId: int32(0),
+ FlowId: uint32(onu.ID),
+ FlowType: "downstream",
+ AllocId: int32(0),
+ NetworkIntfId: int32(0),
+ Classifier: &openolt.Classifier{
+ EthType: uint32(layers.EthernetTypeIPv4),
+ SrcPort: uint32(68),
+ DstPort: uint32(67),
+ OPbits: 0,
+ },
+ Action: &openolt.Action{},
+ Priority: int32(100),
+ PortNo: uint32(onu.ID), // NOTE we are using this to map an incoming packetIndication to an ONU
+ }
+
+ msg := OnuFlowUpdateMessage{
+ PonPortID: 1,
+ OnuID: 1,
+ Flow: &flow,
+ }
+
+ onu.handleFlowUpdate(msg)
+
+ wg := sync.WaitGroup{}
+ wg.Add(1)
+ go func(wg *sync.WaitGroup) {
+ defer wg.Done()
+ time.Sleep(100 * time.Millisecond)
+
+ // emulate the addition of a GemPort
+ for _, ch := range onu.GemPortChannels {
+ ch <- true
+ }
+
+ time.Sleep(100 * time.Millisecond)
+ assert.Equal(t, onu.InternalState.Current(), "dhcp_started")
+ assert.Equal(t, onu.DhcpFlowReceived, true)
+ }(&wg)
+ wg.Wait()
+}
diff --git a/internal/bbsim/devices/onu_state_machine_test.go b/internal/bbsim/devices/onu_state_machine_test.go
index 0bf4bed..81bf330 100644
--- a/internal/bbsim/devices/onu_state_machine_test.go
+++ b/internal/bbsim/devices/onu_state_machine_test.go
@@ -39,8 +39,8 @@
onu.PortNo = 16
onu.DhcpFlowReceived = true
onu.Flows = []FlowKey{
- FlowKey{ID:1, Direction:"upstream"},
- FlowKey{ID:2, Direction:"downstream"},
+ FlowKey{ID: 1, Direction: "upstream"},
+ FlowKey{ID: 2, Direction: "downstream"},
}
onu.InternalState.Event("disable")
@@ -51,38 +51,51 @@
assert.Equal(t, len(onu.Flows), 0)
}
-func Test_Onu_StateMachine_eapol_start_eap_flow(t *testing.T) {
+// check that I can go to auth_started only if
+// - the GemPort is set
+// - the eapolFlow is received
+func Test_Onu_StateMachine_eapol_no_flow(t *testing.T) {
onu := createTestOnu()
onu.InternalState.SetState("enabled")
-
- // TODO we need to add a check so that you can't go from eapol_flow_received
- // to auth_started without passing through gem_port_added
- // (see start_dhcp for an example)
-
assert.Equal(t, onu.InternalState.Current(), "enabled")
- onu.InternalState.Event("receive_eapol_flow")
- assert.Equal(t, onu.InternalState.Current(), "eapol_flow_received")
- onu.InternalState.Event("add_gem_port")
- assert.Equal(t, onu.InternalState.Current(), "gem_port_added")
- onu.InternalState.Event("start_auth")
- assert.Equal(t, onu.InternalState.Current(), "auth_started")
+
+ // fail as no EapolFlow has been received
+ err := onu.InternalState.Event("start_auth")
+ if err == nil {
+ t.Fatal("can't start EAPOL without EapolFlow")
+ }
+ assert.Equal(t, onu.InternalState.Current(), "enabled")
+ assert.Equal(t, err.Error(), "transition canceled with error: cannot-go-to-auth-started-as-eapol-flow-is-missing")
}
-func Test_Onu_StateMachine_eapol_start_gem_port(t *testing.T) {
+func Test_Onu_StateMachine_eapol_no_gem(t *testing.T) {
onu := createTestOnu()
onu.InternalState.SetState("enabled")
-
- // TODO we need to add a check so that you can't go from gem_port_added
- // to auth_started without passing through eapol_flow_received
- // (see start_dhcp for an example)
-
assert.Equal(t, onu.InternalState.Current(), "enabled")
- onu.InternalState.Event("add_gem_port")
- assert.Equal(t, onu.InternalState.Current(), "gem_port_added")
- onu.InternalState.Event("receive_eapol_flow")
- assert.Equal(t, onu.InternalState.Current(), "eapol_flow_received")
+ // fail has no GemPort has been set
+ onu.EapolFlowReceived = true
+
+ err := onu.InternalState.Event("start_auth")
+ if err == nil {
+ t.Fatal("can't start EAPOL without GemPort")
+ }
+ assert.Equal(t, onu.InternalState.Current(), "enabled")
+ assert.Equal(t, err.Error(), "transition canceled with error: cannot-go-to-auth-started-as-gemport-is-missing")
+
+}
+
+func Test_Onu_StateMachine_eapol_start(t *testing.T) {
+
+ onu := createTestOnu()
+
+ onu.InternalState.SetState("enabled")
+ assert.Equal(t, onu.InternalState.Current(), "enabled")
+
+ // succeed
+ onu.EapolFlowReceived = true
+ onu.GemPortAdded = true
onu.InternalState.Event("start_auth")
assert.Equal(t, onu.InternalState.Current(), "auth_started")
}
@@ -90,6 +103,9 @@
func Test_Onu_StateMachine_eapol_states(t *testing.T) {
onu := createTestOnu()
+ onu.GemPortAdded = true
+ onu.EapolFlowReceived = true
+
onu.InternalState.SetState("auth_started")
assert.Equal(t, onu.InternalState.Current(), "auth_started")
@@ -112,34 +128,77 @@
}
}
+// if auth is set to true we can't go from enabled to dhcp_started
+func Test_Onu_StateMachine_dhcp_no_auth(t *testing.T) {
+ onu := createTestOnu()
+
+ onu.InternalState.SetState("enabled")
+ assert.Equal(t, onu.InternalState.Current(), "enabled")
+
+ onu.Auth = true
+
+ err := onu.InternalState.Event("start_dhcp")
+ if err == nil {
+ t.Fail()
+ }
+ assert.Equal(t, onu.InternalState.Current(), "enabled")
+ assert.Equal(t, err.Error(), "transition canceled with error: cannot-go-to-dhcp-started-as-authentication-is-required")
+}
+
+// if the DHCP flow has not been received we can't start authentication
+func Test_Onu_StateMachine_dhcp_no_flow(t *testing.T) {
+ onu := createTestOnu()
+
+ onu.InternalState.SetState("eap_response_success_received")
+ assert.Equal(t, onu.InternalState.Current(), "eap_response_success_received")
+
+ onu.DhcpFlowReceived = false
+
+ err := onu.InternalState.Event("start_dhcp")
+ if err == nil {
+ t.Fail()
+ }
+ assert.Equal(t, onu.InternalState.Current(), "eap_response_success_received")
+ assert.Equal(t, err.Error(), "transition canceled with error: cannot-go-to-dhcp-started-as-dhcp-flow-is-missing")
+}
+
+// if the ONU does not have a GemPort we can't start DHCP
+func Test_Onu_StateMachine_dhcp_no_gem(t *testing.T) {
+ onu := createTestOnu()
+
+ onu.InternalState.SetState("eap_response_success_received")
+ assert.Equal(t, onu.InternalState.Current(), "eap_response_success_received")
+
+ onu.DhcpFlowReceived = true
+ onu.GemPortAdded = false
+
+ err := onu.InternalState.Event("start_dhcp")
+ if err == nil {
+ t.Fail()
+ }
+ assert.Equal(t, onu.InternalState.Current(), "eap_response_success_received")
+ assert.Equal(t, err.Error(), "transition canceled with error: cannot-go-to-dhcp-started-as-gemport-is-missing")
+}
+
func Test_Onu_StateMachine_dhcp_start(t *testing.T) {
onu := createTestOnu()
onu.DhcpFlowReceived = true
+ onu.GemPortAdded = true
+ onu.Auth = true
onu.InternalState.SetState("eap_response_success_received")
assert.Equal(t, onu.InternalState.Current(), "eap_response_success_received")
+ // default transition
onu.InternalState.Event("start_dhcp")
-
assert.Equal(t, onu.InternalState.Current(), "dhcp_started")
}
-func Test_Onu_StateMachine_dhcp_start_error(t *testing.T) {
- onu := createTestOnu()
-
- onu.InternalState.SetState("eap_response_success_received")
- assert.Equal(t, onu.InternalState.Current(), "eap_response_success_received")
-
- err := onu.InternalState.Event("start_dhcp")
-
- assert.Equal(t, onu.InternalState.Current(), "eap_response_success_received")
- assert.Equal(t, err.Error(), "transition canceled with error: cannot-go-to-dhcp-started-as-dhcp-flow-is-missing")
-}
-
func Test_Onu_StateMachine_dhcp_states(t *testing.T) {
onu := createTestOnu()
- onu.DhcpFlowReceived = false
+ onu.DhcpFlowReceived = true
+ onu.GemPortAdded = true
onu.InternalState.SetState("dhcp_started")
@@ -152,7 +211,6 @@
assert.Equal(t, onu.InternalState.Current(), "dhcp_ack_received")
// test that we can retrigger DHCP
- onu.DhcpFlowReceived = true
states := []string{"eap_response_success_received", "dhcp_discovery_sent", "dhcp_request_sent", "dhcp_ack_received", "dhcp_failed"}
for _, state := range states {
onu.InternalState.SetState(state)
diff --git a/internal/bbsim/devices/onu_test.go b/internal/bbsim/devices/onu_test.go
index 6d4804c..35b75e8 100644
--- a/internal/bbsim/devices/onu_test.go
+++ b/internal/bbsim/devices/onu_test.go
@@ -17,6 +17,7 @@
package devices
import (
+ omcisim "github.com/opencord/omci-sim"
"gotest.tools/assert"
"testing"
)
@@ -31,7 +32,7 @@
Olt: &olt,
}
- onu := CreateONU(&olt, pon, 1, 900, 900, true, false,0,false)
+ onu := CreateONU(&olt, pon, 1, 900, 900, true, false, 0, false)
assert.Equal(t, onu.Sn(), "BBSM00000101")
assert.Equal(t, onu.STag, 900)
@@ -39,4 +40,45 @@
assert.Equal(t, onu.Auth, true)
assert.Equal(t, onu.Dhcp, false)
assert.Equal(t, onu.HwAddress.String(), "2e:60:70:00:01:01")
-}
\ No newline at end of file
+}
+
+func TestOnu_processOmciMessage_GemPortAdded(t *testing.T) {
+
+ receivedValues := []bool{}
+
+ checker := func(ch chan bool, done chan int) {
+ for v := range ch {
+ receivedValues = append(receivedValues, v)
+ }
+ done <- 0
+ }
+
+ onu := createTestOnu()
+
+ // create two listeners on the GemPortAdded event
+ ch1 := onu.GetGemPortChan()
+ ch2 := onu.GetGemPortChan()
+
+ msg := omcisim.OmciChMessage{
+ Type: omcisim.GemPortAdded,
+ Data: omcisim.OmciChMessageData{
+ IntfId: 1,
+ OnuId: 1,
+ },
+ }
+
+ onu.processOmciMessage(msg, nil)
+
+ done := make(chan int)
+
+ go checker(ch1, done)
+ go checker(ch2, done)
+
+ // wait for the messages to be received on the "done" channel
+ <-done
+ <-done
+
+ // make sure all channel are closed and removed
+ assert.Equal(t, len(onu.GemPortChannels), 0)
+ assert.Equal(t, len(receivedValues), 2)
+}
diff --git a/internal/bbsim/devices/onu_test_helpers.go b/internal/bbsim/devices/onu_test_helpers.go
index 1f70786..5a991d6 100644
--- a/internal/bbsim/devices/onu_test_helpers.go
+++ b/internal/bbsim/devices/onu_test_helpers.go
@@ -106,19 +106,20 @@
}
// this method creates a fake ONU used in the tests
-func createMockOnu(id uint32, ponPortId uint32, sTag int, cTag int, auth bool, dhcp bool) Onu {
+func createMockOnu(id uint32, ponPortId uint32, sTag int, cTag int, auth bool, dhcp bool) *Onu {
o := Onu{
- ID: id,
- PonPortID: ponPortId,
- STag: sTag,
- CTag: cTag,
- HwAddress: net.HardwareAddr{0x2e, 0x60, 0x70, 0x13, byte(ponPortId), byte(id)},
- PortNo: 0,
- Auth: auth,
- Dhcp: dhcp,
+ ID: id,
+ PonPortID: ponPortId,
+ STag: sTag,
+ CTag: cTag,
+ HwAddress: net.HardwareAddr{0x2e, 0x60, 0x70, 0x13, byte(ponPortId), byte(id)},
+ PortNo: 0,
+ Auth: auth,
+ Dhcp: dhcp,
+ GemPortAdded: true,
}
o.SerialNumber = o.NewSN(0, ponPortId, o.ID)
- return o
+ return &o
}
// this method creates a real ONU to be used in the tests
diff --git a/internal/bbsim/devices/pon.go b/internal/bbsim/devices/pon.go
index d1f7d2a..225a45f 100644
--- a/internal/bbsim/devices/pon.go
+++ b/internal/bbsim/devices/pon.go
@@ -80,40 +80,40 @@
if onu.InternalState.Current() == "pon_disabled" {
if err := onu.InternalState.Event("enable"); err != nil {
log.WithFields(log.Fields{
- "Err": err,
- "OnuSn": onu.Sn(),
+ "Err": err,
+ "OnuSn": onu.Sn(),
"IntfId": onu.PonPortID,
}).Error("Error enabling ONU")
}
} else if onu.InternalState.Current() == "disabled" {
if err := onu.InternalState.Event("initialize"); err != nil {
log.WithFields(log.Fields{
- "Err": err,
- "OnuSn": onu.Sn(),
+ "Err": err,
+ "OnuSn": onu.Sn(),
"IntfId": onu.PonPortID,
}).Error("Error initializing ONU")
continue
}
if err := onu.InternalState.Event("discover"); err != nil {
log.WithFields(log.Fields{
- "Err": err,
- "OnuSn": onu.Sn(),
+ "Err": err,
+ "OnuSn": onu.Sn(),
"IntfId": onu.PonPortID,
}).Error("Error discovering ONU")
}
} else if onu.InternalState.Current() == "initialized" {
if err := onu.InternalState.Event("discover"); err != nil {
log.WithFields(log.Fields{
- "Err": err,
- "OnuSn": onu.Sn(),
+ "Err": err,
+ "OnuSn": onu.Sn(),
"IntfId": onu.PonPortID,
}).Error("Error discovering ONU")
}
} else {
// this is to loudly report unexpected states in order to address them
log.WithFields(log.Fields{
- "OnuSn": onu.Sn(),
- "IntfId": onu.PonPortID,
+ "OnuSn": onu.Sn(),
+ "IntfId": onu.PonPortID,
"InternalState": onu.InternalState.Current(),
}).Error("Unexpected ONU state in PON enabling")
}