[SEBA-820] Preventing the ONU from going to dhcp_started if we don't have the DHCP flow
Change-Id: Ia9c136830a6781fa69d7ead5406c3e354ad35a06
diff --git a/internal/bbsim/devices/onu.go b/internal/bbsim/devices/onu.go
index 0f93e0d..cc467b8 100644
--- a/internal/bbsim/devices/onu.go
+++ b/internal/bbsim/devices/onu.go
@@ -18,6 +18,7 @@
import (
"context"
+ "errors"
"fmt"
"github.com/cboling/omci"
"github.com/google/gopacket/layers"
@@ -46,10 +47,13 @@
// 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
- PortNo uint32
HwAddress net.HardwareAddr
InternalState *fsm.FSM
+ // ONU State
+ PortNo uint32
+ DhcpFlowReceived bool
+
OperState *fsm.FSM
SerialNumber *openolt.SerialNumber
@@ -64,25 +68,26 @@
DoneChannel chan bool // this channel is used to signal once the onu is complete (when the struct is used by BBR)
}
-func (o Onu) Sn() string {
+func (o *Onu) Sn() string {
return common.OnuSnToString(o.SerialNumber)
}
func CreateONU(olt OltDevice, pon PonPort, id uint32, sTag int, cTag int) *Onu {
o := Onu{
- ID: id,
- PonPortID: pon.ID,
- PonPort: pon,
- STag: sTag,
- CTag: cTag,
- HwAddress: net.HardwareAddr{0x2e, 0x60, 0x70, 0x13, byte(pon.ID), byte(id)},
- PortNo: 0,
- Channel: make(chan Message, 2048),
- tid: 0x1,
- hpTid: 0x8000,
- seqNumber: 0,
- DoneChannel: make(chan bool, 1),
+ ID: id,
+ PonPortID: pon.ID,
+ PonPort: pon,
+ STag: sTag,
+ CTag: cTag,
+ HwAddress: net.HardwareAddr{0x2e, 0x60, 0x70, 0x13, byte(pon.ID), byte(id)},
+ PortNo: 0,
+ Channel: make(chan Message, 2048),
+ tid: 0x1,
+ hpTid: 0x8000,
+ seqNumber: 0,
+ DoneChannel: make(chan bool, 1),
+ DhcpFlowReceived: false,
}
o.SerialNumber = o.NewSN(olt.ID, pon.ID, o.ID)
@@ -167,6 +172,11 @@
"OnuSn": o.Sn(),
}).Errorf("ONU failed to authenticate!")
},
+ "before_start_dhcp": func(e *fsm.Event) {
+ if o.DhcpFlowReceived == false {
+ e.Cancel(errors.New("cannot-go-to-dhcp-started-as-dhcp-flow-is-missing"))
+ }
+ },
"enter_dhcp_started": func(e *fsm.Event) {
msg := Message{
Type: StartDHCP,
@@ -209,7 +219,7 @@
}).Debugf("Changing ONU InternalState from %s to %s", src, dst)
}
-func (o Onu) ProcessOnuMessages(stream openolt.Openolt_EnableIndicationServer, client openolt.OpenoltClient) {
+func (o *Onu) ProcessOnuMessages(stream openolt.Openolt_EnableIndicationServer, client openolt.OpenoltClient) {
onuLogger.WithFields(log.Fields{
"onuID": o.ID,
"onuSN": o.Sn(),
@@ -358,6 +368,7 @@
if err := stream.Send(&openolt.Indication{Data: discoverData}); err != nil {
log.Errorf("Failed to send Indication_OnuDiscInd: %v", err)
+ return
}
if err := o.InternalState.Event("discover"); err != nil {
@@ -507,9 +518,12 @@
} else if msg.Flow.Classifier.EthType == uint32(layers.EthernetTypeIPv4) &&
msg.Flow.Classifier.SrcPort == uint32(68) &&
msg.Flow.Classifier.DstPort == uint32(67) {
+
+ // keep track that we reveived the DHCP Flows so that we can transition the state to dhcp_started
+ o.DhcpFlowReceived = true
// NOTE we are receiving mulitple DHCP flows but we shouldn't call the transition multiple times
if err := o.InternalState.Event("start_dhcp"); err != nil {
- log.Warnf("Can't go to dhcp_started: %v", err)
+ log.Errorf("Can't go to dhcp_started: %v", err)
}
}
}
diff --git a/internal/bbsim/devices/onu_flow_test.go b/internal/bbsim/devices/onu_flow_test.go
index 1e47034..4194aa5 100644
--- a/internal/bbsim/devices/onu_flow_test.go
+++ b/internal/bbsim/devices/onu_flow_test.go
@@ -17,110 +17,13 @@
package devices
import (
- "context"
- "errors"
"github.com/google/gopacket/layers"
"github.com/looplab/fsm"
"github.com/opencord/voltha-protos/go/openolt"
- "github.com/opencord/voltha-protos/go/tech_profile"
- "google.golang.org/grpc"
"gotest.tools/assert"
- "net"
"testing"
)
-type FlowAddSpy struct {
- CallCount int
- Calls map[int]*openolt.Flow
-}
-
-type mockClient struct {
- FlowAddSpy
- fail bool
-}
-
-func (s *mockClient) DisableOlt(ctx context.Context, in *openolt.Empty, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) ReenableOlt(ctx context.Context, in *openolt.Empty, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) ActivateOnu(ctx context.Context, in *openolt.Onu, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) DeactivateOnu(ctx context.Context, in *openolt.Onu, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) DeleteOnu(ctx context.Context, in *openolt.Onu, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) OmciMsgOut(ctx context.Context, in *openolt.OmciMsg, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) OnuPacketOut(ctx context.Context, in *openolt.OnuPacket, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) UplinkPacketOut(ctx context.Context, in *openolt.UplinkPacket, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) FlowAdd(ctx context.Context, in *openolt.Flow, opts ...grpc.CallOption) (*openolt.Empty, error) {
- s.FlowAddSpy.CallCount++
- if s.fail {
- return nil, errors.New("fake-error")
- }
- s.FlowAddSpy.Calls[s.FlowAddSpy.CallCount] = in
- return &openolt.Empty{}, nil
-}
-func (s *mockClient) FlowRemove(ctx context.Context, in *openolt.Flow, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) HeartbeatCheck(ctx context.Context, in *openolt.Empty, opts ...grpc.CallOption) (*openolt.Heartbeat, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) EnablePonIf(ctx context.Context, in *openolt.Interface, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) DisablePonIf(ctx context.Context, in *openolt.Interface, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) GetDeviceInfo(ctx context.Context, in *openolt.Empty, opts ...grpc.CallOption) (*openolt.DeviceInfo, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) Reboot(ctx context.Context, in *openolt.Empty, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) CollectStatistics(ctx context.Context, in *openolt.Empty, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) CreateTrafficSchedulers(ctx context.Context, in *tech_profile.TrafficSchedulers, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) RemoveTrafficSchedulers(ctx context.Context, in *tech_profile.TrafficSchedulers, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) CreateTrafficQueues(ctx context.Context, in *tech_profile.TrafficQueues, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) RemoveTrafficQueues(ctx context.Context, in *tech_profile.TrafficQueues, opts ...grpc.CallOption) (*openolt.Empty, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-func (s *mockClient) EnableIndication(ctx context.Context, in *openolt.Empty, opts ...grpc.CallOption) (openolt.Openolt_EnableIndicationClient, error) {
- return nil, errors.New("unimplemented-in-mock-client")
-}
-
-func createMockOnu(id uint32, ponPortId uint32, sTag int, cTag int) Onu {
- o := Onu{
- ID: id,
- PonPortID: ponPortId,
- STag: sTag,
- CTag: cTag,
- HwAddress: net.HardwareAddr{0x2e, 0x60, 0x70, 0x13, byte(ponPortId), byte(id)},
- PortNo: 0,
- }
- o.SerialNumber = o.NewSN(0, ponPortId, o.ID)
- return o
-}
-
func Test_Onu_SendEapolFlow(t *testing.T) {
onu := createMockOnu(1, 1, 900, 900)
diff --git a/internal/bbsim/devices/onu_state_machine_test.go b/internal/bbsim/devices/onu_state_machine_test.go
new file mode 100644
index 0000000..239869b
--- /dev/null
+++ b/internal/bbsim/devices/onu_state_machine_test.go
@@ -0,0 +1,166 @@
+/*
+ * Copyright 2018-present Open Networking Foundation
+
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+
+ * http://www.apache.org/licenses/LICENSE-2.0
+
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package devices
+
+import (
+ "gotest.tools/assert"
+ "testing"
+)
+
+func Test_Onu_StateMachine_enable(t *testing.T) {
+ olt := OltDevice{
+ ID: 0,
+ }
+ pon := PonPort{
+ ID: 1,
+ }
+ onu := CreateONU(olt, pon, 1, 900, 900)
+
+ assert.Equal(t, onu.InternalState.Current(), "created")
+ onu.InternalState.Event("discover")
+ assert.Equal(t, onu.InternalState.Current(), "discovered")
+ onu.InternalState.Event("enable")
+ assert.Equal(t, onu.InternalState.Current(), "enabled")
+}
+
+func Test_Onu_StateMachine_eapol_start_eap_flow(t *testing.T) {
+ olt := OltDevice{
+ ID: 0,
+ }
+ pon := PonPort{
+ ID: 1,
+ }
+ onu := CreateONU(olt, pon, 1, 900, 900)
+
+ 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")
+}
+
+func Test_Onu_StateMachine_eapol_start_gem_port(t *testing.T) {
+ olt := OltDevice{
+ ID: 0,
+ }
+ pon := PonPort{
+ ID: 1,
+ }
+ onu := CreateONU(olt, pon, 1, 900, 900)
+
+ 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")
+ onu.InternalState.Event("start_auth")
+ assert.Equal(t, onu.InternalState.Current(), "auth_started")
+}
+
+func Test_Onu_StateMachine_eapol_states(t *testing.T) {
+ olt := OltDevice{
+ ID: 0,
+ }
+ pon := PonPort{
+ ID: 1,
+ }
+ onu := CreateONU(olt, pon, 1, 900, 900)
+
+ onu.InternalState.SetState("auth_started")
+
+ assert.Equal(t, onu.InternalState.Current(), "auth_started")
+ onu.InternalState.Event("eap_start_sent")
+ assert.Equal(t, onu.InternalState.Current(), "eap_start_sent")
+ onu.InternalState.Event("eap_response_identity_sent")
+ assert.Equal(t, onu.InternalState.Current(), "eap_response_identity_sent")
+ onu.InternalState.Event("eap_response_challenge_sent")
+ assert.Equal(t, onu.InternalState.Current(), "eap_response_challenge_sent")
+ onu.InternalState.Event("eap_response_success_received")
+ assert.Equal(t, onu.InternalState.Current(), "eap_response_success_received")
+}
+
+func Test_Onu_StateMachine_dhcp_start(t *testing.T) {
+ olt := OltDevice{
+ ID: 0,
+ }
+ pon := PonPort{
+ ID: 1,
+ }
+ onu := CreateONU(olt, pon, 1, 900, 900)
+ onu.DhcpFlowReceived = true
+
+ onu.InternalState.SetState("eap_response_success_received")
+ assert.Equal(t, onu.InternalState.Current(), "eap_response_success_received")
+
+ onu.InternalState.Event("start_dhcp")
+
+ assert.Equal(t, onu.InternalState.Current(), "dhcp_started")
+}
+
+func Test_Onu_StateMachine_dhcp_start_error(t *testing.T) {
+ olt := OltDevice{
+ ID: 0,
+ }
+ pon := PonPort{
+ ID: 1,
+ }
+ onu := CreateONU(olt, pon, 1, 900, 900)
+
+ 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) {
+ olt := OltDevice{
+ ID: 0,
+ }
+ pon := PonPort{
+ ID: 1,
+ }
+ onu := CreateONU(olt, pon, 1, 900, 900)
+
+ onu.DhcpFlowReceived = false
+
+ onu.InternalState.SetState("dhcp_started")
+
+ assert.Equal(t, onu.InternalState.Current(), "dhcp_started")
+ onu.InternalState.Event("dhcp_discovery_sent")
+ assert.Equal(t, onu.InternalState.Current(), "dhcp_discovery_sent")
+ onu.InternalState.Event("dhcp_request_sent")
+ assert.Equal(t, onu.InternalState.Current(), "dhcp_request_sent")
+ onu.InternalState.Event("dhcp_ack_received")
+ assert.Equal(t, onu.InternalState.Current(), "dhcp_ack_received")
+}
diff --git a/internal/bbsim/devices/onu_test_helpers.go b/internal/bbsim/devices/onu_test_helpers.go
new file mode 100644
index 0000000..1044913
--- /dev/null
+++ b/internal/bbsim/devices/onu_test_helpers.go
@@ -0,0 +1,118 @@
+/*
+ * Copyright 2018-present Open Networking Foundation
+
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+
+ * http://www.apache.org/licenses/LICENSE-2.0
+
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package devices
+
+import (
+ "context"
+ "errors"
+ "github.com/opencord/voltha-protos/go/openolt"
+ "github.com/opencord/voltha-protos/go/tech_profile"
+ "google.golang.org/grpc"
+ "net"
+)
+
+type FlowAddSpy struct {
+ CallCount int
+ Calls map[int]*openolt.Flow
+}
+
+type mockClient struct {
+ FlowAddSpy
+ fail bool
+}
+
+func (s *mockClient) DisableOlt(ctx context.Context, in *openolt.Empty, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) ReenableOlt(ctx context.Context, in *openolt.Empty, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) ActivateOnu(ctx context.Context, in *openolt.Onu, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) DeactivateOnu(ctx context.Context, in *openolt.Onu, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) DeleteOnu(ctx context.Context, in *openolt.Onu, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) OmciMsgOut(ctx context.Context, in *openolt.OmciMsg, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) OnuPacketOut(ctx context.Context, in *openolt.OnuPacket, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) UplinkPacketOut(ctx context.Context, in *openolt.UplinkPacket, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) FlowAdd(ctx context.Context, in *openolt.Flow, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ s.FlowAddSpy.CallCount++
+ if s.fail {
+ return nil, errors.New("fake-error")
+ }
+ s.FlowAddSpy.Calls[s.FlowAddSpy.CallCount] = in
+ return &openolt.Empty{}, nil
+}
+func (s *mockClient) FlowRemove(ctx context.Context, in *openolt.Flow, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) HeartbeatCheck(ctx context.Context, in *openolt.Empty, opts ...grpc.CallOption) (*openolt.Heartbeat, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) EnablePonIf(ctx context.Context, in *openolt.Interface, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) DisablePonIf(ctx context.Context, in *openolt.Interface, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) GetDeviceInfo(ctx context.Context, in *openolt.Empty, opts ...grpc.CallOption) (*openolt.DeviceInfo, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) Reboot(ctx context.Context, in *openolt.Empty, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) CollectStatistics(ctx context.Context, in *openolt.Empty, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) CreateTrafficSchedulers(ctx context.Context, in *tech_profile.TrafficSchedulers, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) RemoveTrafficSchedulers(ctx context.Context, in *tech_profile.TrafficSchedulers, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) CreateTrafficQueues(ctx context.Context, in *tech_profile.TrafficQueues, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) RemoveTrafficQueues(ctx context.Context, in *tech_profile.TrafficQueues, opts ...grpc.CallOption) (*openolt.Empty, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+func (s *mockClient) EnableIndication(ctx context.Context, in *openolt.Empty, opts ...grpc.CallOption) (openolt.Openolt_EnableIndicationClient, error) {
+ return nil, errors.New("unimplemented-in-mock-client")
+}
+
+func createMockOnu(id uint32, ponPortId uint32, sTag int, cTag int) Onu {
+ o := Onu{
+ ID: id,
+ PonPortID: ponPortId,
+ STag: sTag,
+ CTag: cTag,
+ HwAddress: net.HardwareAddr{0x2e, 0x60, 0x70, 0x13, byte(ponPortId), byte(id)},
+ PortNo: 0,
+ }
+ o.SerialNumber = o.NewSN(0, ponPortId, o.ID)
+ return o
+}