VOL-2632 Error propagation from HashFlowStats
Change-Id: Iec132540c8d1b474820d390fca3d4a37e7dc99c9
diff --git a/VERSION b/VERSION
index 9bbe651..e265a8c 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-3.0.14
+3.0.15
diff --git a/pkg/flows/flow_utils.go b/pkg/flows/flow_utils.go
index 4de929f..b2086cd 100644
--- a/pkg/flows/flow_utils.go
+++ b/pkg/flows/flow_utils.go
@@ -18,6 +18,7 @@
import (
"bytes"
"crypto/md5"
+ "errors"
"fmt"
"github.com/cevaris/ordered_map"
"github.com/gogo/protobuf/proto"
@@ -678,9 +679,9 @@
// Return unique 64-bit integer hash for flow covering the following attributes:
// 'table_id', 'priority', 'flags', 'cookie', 'match', '_instruction_string'
-func HashFlowStats(flow *ofp.OfpFlowStats) uint64 {
+func HashFlowStats(flow *ofp.OfpFlowStats) (uint64, error) {
if flow == nil { // Should never happen
- return 0
+ return 0, errors.New("hash-flow-stats-nil-flow")
}
// Create string with the instructions field first
var instructionString bytes.Buffer
@@ -690,19 +691,18 @@
var flowString = fmt.Sprintf("%d%d%d%d%s%s", flow.TableId, flow.Priority, flow.Flags, flow.Cookie, flow.Match.String(), instructionString.String())
h := md5.New()
if _, err := h.Write([]byte(flowString)); err != nil {
- logger.Errorw("hash-flow-status", log.Fields{"error": err})
- return 0
+ return 0, fmt.Errorf("hash-flow-stats-failed-hash: %v", err)
}
hash := big.NewInt(0)
hash.SetBytes(h.Sum(nil))
- return hash.Uint64()
+ return hash.Uint64(), nil
}
// flowStatsEntryFromFlowModMessage maps an ofp_flow_mod message to an ofp_flow_stats message
-func FlowStatsEntryFromFlowModMessage(mod *ofp.OfpFlowMod) *ofp.OfpFlowStats {
+func FlowStatsEntryFromFlowModMessage(mod *ofp.OfpFlowMod) (*ofp.OfpFlowStats, error) {
flow := &ofp.OfpFlowStats{}
if mod == nil {
- return flow
+ return flow, nil
}
flow.TableId = mod.TableId
flow.Priority = mod.Priority
@@ -712,8 +712,12 @@
flow.Cookie = mod.Cookie
flow.Match = mod.Match
flow.Instructions = mod.Instructions
- flow.Id = HashFlowStats(flow)
- return flow
+ var err error
+ if flow.Id, err = HashFlowStats(flow); err != nil {
+ return nil, err
+ }
+
+ return flow, nil
}
func GroupEntryFromGroupMod(mod *ofp.OfpGroupMod) *ofp.OfpGroupEntry {
@@ -913,7 +917,7 @@
}
// MkFlowStat is a helper method to build flows
-func MkFlowStat(fa *FlowArgs) *ofp.OfpFlowStats {
+func MkFlowStat(fa *FlowArgs) (*ofp.OfpFlowStats, error) {
//Build the match-fields
matchFields := make([]*ofp.OfpOxmField, 0)
for _, val := range fa.MatchFields {
diff --git a/pkg/flows/flow_utils_test.go b/pkg/flows/flow_utils_test.go
index 5219cf3..c4e481d 100644
--- a/pkg/flows/flow_utils_test.go
+++ b/pkg/flows/flow_utils_test.go
@@ -37,6 +37,11 @@
timeoutError = status.Errorf(codes.Aborted, "timeout")
}
+func TestHashFlowStatsNil(t *testing.T) {
+ _, err := HashFlowStats(nil)
+ assert.EqualError(t, err, "hash-flow-stats-nil-flow")
+}
+
func TestFlowsAndGroups_AddFlow(t *testing.T) {
fg := NewFlowsAndGroups()
allFlows := fg.ListFlows()
@@ -63,7 +68,8 @@
Output(uint32(ofp.OfpPortNo_OFPP_CONTROLLER)),
},
}
- flow := MkFlowStat(fa)
+ flow, err := MkFlowStat(fa)
+ assert.Nil(t, err)
fg.AddFlow(flow)
allFlows = fg.ListFlows()
@@ -115,7 +121,8 @@
Output(1),
},
}
- flow := MkFlowStat(fa)
+ flow, err := MkFlowStat(fa)
+ assert.Nil(t, err)
fg.AddFlow(flow)
ga = &GroupArgs{
@@ -166,7 +173,8 @@
PopVlan(),
},
}
- flow1 := MkFlowStat(fa1)
+ flow1, err := MkFlowStat(fa1)
+ assert.Nil(t, err)
fa2 = &FlowArgs{
KV: OfpFlowModArgs{"priority": 1500},
@@ -181,7 +189,8 @@
Output(2),
},
}
- flow2 := MkFlowStat(fa2)
+ flow2, err := MkFlowStat(fa2)
+ assert.Nil(t, err)
fg.AddFlow(flow1)
fg.AddFlow(flow2)
@@ -234,7 +243,8 @@
Group(10),
},
}
- flow := MkFlowStat(fa)
+ flow, err := MkFlowStat(fa)
+ assert.Nil(t, err)
fg.AddFlow(flow)
ga = &GroupArgs{
@@ -281,7 +291,8 @@
Output(1),
},
}
- flow := MkFlowStat(fa)
+ flow, err := MkFlowStat(fa)
+ assert.Nil(t, err)
fg.AddFlow(flow)
ga = &GroupArgs{
@@ -335,7 +346,8 @@
Output(1),
},
}
- flow := MkFlowStat(fa)
+ flow, err := MkFlowStat(fa)
+ assert.Nil(t, err)
dr.AddFlow("123456", flow)
rules = dr.GetRules()
assert.True(t, len(rules) == 1)
@@ -368,7 +380,8 @@
Output(1),
},
}
- flow := MkFlowStat(fa)
+ flow, err := MkFlowStat(fa)
+ assert.Nil(t, err)
fg.AddFlow(flow)
ga = &GroupArgs{
@@ -414,7 +427,8 @@
Output(1),
},
}
- flow = MkFlowStat(fa)
+ flow, err := MkFlowStat(fa)
+ assert.Nil(t, err)
assert.True(t, FlowHasOutPort(flow, 1))
assert.False(t, FlowHasOutPort(flow, 2))
@@ -425,7 +439,8 @@
VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0),
},
}
- flow = MkFlowStat(fa)
+ flow, err = MkFlowStat(fa)
+ assert.Nil(t, err)
assert.False(t, FlowHasOutPort(flow, 1))
}
@@ -446,7 +461,8 @@
Group(10),
},
}
- flow = MkFlowStat(fa)
+ flow, err := MkFlowStat(fa)
+ assert.Nil(t, err)
assert.True(t, FlowHasOutGroup(flow, 10))
assert.False(t, FlowHasOutGroup(flow, 11))
@@ -463,7 +479,8 @@
Output(1),
},
}
- flow = MkFlowStat(fa)
+ flow, err = MkFlowStat(fa)
+ assert.Nil(t, err)
assert.False(t, FlowHasOutGroup(flow, 1))
}
@@ -482,7 +499,8 @@
Group(10),
},
}
- flow1 := MkFlowStat(fa)
+ flow1, err := MkFlowStat(fa)
+ assert.Nil(t, err)
assert.False(t, FlowMatch(flow1, nil))
fa = &FlowArgs{
@@ -498,7 +516,8 @@
Group(10),
},
}
- flow2 := MkFlowStat(fa)
+ flow2, err := MkFlowStat(fa)
+ assert.Nil(t, err)
assert.False(t, FlowMatch(flow1, flow2))
assert.False(t, FlowMatch(nil, flow2))
@@ -512,7 +531,8 @@
Ipv4Dst(0xe00a0a0a),
},
}
- flow2 = MkFlowStat(fa)
+ flow2, err = MkFlowStat(fa)
+ assert.Nil(t, err)
assert.True(t, FlowMatch(flow1, flow2))
fa = &FlowArgs{
@@ -528,7 +548,8 @@
Group(10),
},
}
- flow2 = MkFlowStat(fa)
+ flow2, err = MkFlowStat(fa)
+ assert.Nil(t, err)
assert.False(t, FlowMatch(flow1, flow2))
fa = &FlowArgs{
@@ -541,7 +562,8 @@
Ipv4Dst(0xe00a0a0a),
},
}
- flow2 = MkFlowStat(fa)
+ flow2, err = MkFlowStat(fa)
+ assert.Nil(t, err)
assert.False(t, FlowMatch(flow1, flow2))
fa = &FlowArgs{
@@ -554,7 +576,8 @@
Ipv4Dst(0xe00a0a0a),
},
}
- flow2 = MkFlowStat(fa)
+ flow2, err = MkFlowStat(fa)
+ assert.Nil(t, err)
assert.False(t, FlowMatch(flow1, flow2))
fa = &FlowArgs{
@@ -567,7 +590,8 @@
Ipv4Dst(0xe00a0a0a),
},
}
- flow2 = MkFlowStat(fa)
+ flow2, err = MkFlowStat(fa)
+ assert.Nil(t, err)
assert.False(t, FlowMatch(flow1, flow2))
fa = &FlowArgs{
@@ -580,7 +604,8 @@
Ipv4Dst(0xe00a0a0a),
},
}
- flow2 = MkFlowStat(fa)
+ flow2, err = MkFlowStat(fa)
+ assert.Nil(t, err)
assert.False(t, FlowMatch(flow1, flow2))
fa = &FlowArgs{
@@ -592,7 +617,8 @@
EthType(0x800),
},
}
- flow2 = MkFlowStat(fa)
+ flow2, err = MkFlowStat(fa)
+ assert.Nil(t, err)
assert.False(t, FlowMatch(flow1, flow2))
fa = &FlowArgs{
@@ -609,7 +635,8 @@
Output(1),
},
}
- flow2 = MkFlowStat(fa)
+ flow2, err = MkFlowStat(fa)
+ assert.Nil(t, err)
assert.True(t, FlowMatch(flow1, flow2))
}
@@ -629,7 +656,8 @@
Group(10),
},
}
- flow := MkFlowStat(fa)
+ flow, err := MkFlowStat(fa)
+ assert.Nil(t, err)
assert.False(t, FlowMatchesMod(flow, nil))
fa = &FlowArgs{
@@ -649,7 +677,9 @@
flowMod := MkSimpleFlowMod(ToOfpOxmField(fa.MatchFields), fa.Actions, fa.Command, fa.KV)
assert.False(t, FlowMatchesMod(nil, flowMod))
assert.False(t, FlowMatchesMod(flow, flowMod))
- assert.True(t, FlowMatch(flow, FlowStatsEntryFromFlowModMessage(flowMod)))
+ entry, err := FlowStatsEntryFromFlowModMessage(flowMod)
+ assert.Nil(t, err)
+ assert.True(t, FlowMatch(flow, entry))
fa = &FlowArgs{
KV: OfpFlowModArgs{"table_id": uint64(ofp.OfpTable_OFPTT_ALL),