From b64fc3ebc2793ea7f726eac7f7f4706408f8601d Mon Sep 17 00:00:00 2001 From: Manmeet Singh Date: Mon, 14 Oct 2024 18:11:42 -0700 Subject: [PATCH 1/3] Implement stats fetch using dump-flows to provide an option to get statistics for all matching flows in one shot. Also, provide an option to report interface names instead of port numbers, if requested. --- AUTHORS | 1 + ovs/flow.go | 124 +++++++++++++++++++++++++++ ovs/flow_test.go | 70 ++++++++++++++++ ovs/openflow.go | 60 ++++++++++++- ovs/openflow_test.go | 196 +++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 449 insertions(+), 2 deletions(-) diff --git a/AUTHORS b/AUTHORS index 3d08687..eb57d88 100644 --- a/AUTHORS +++ b/AUTHORS @@ -15,3 +15,4 @@ Neal Shrader Sangeetha Srikanth Franck Rupin Adam Simeth +Manmeet Singh \ No newline at end of file diff --git a/ovs/flow.go b/ovs/flow.go index 0de9337..7e4ba64 100644 --- a/ovs/flow.go +++ b/ovs/flow.go @@ -82,6 +82,16 @@ type LearnedFlow struct { Limit int } +// A PerFlowStats is meant for fetching FlowStats per flow +type PerFlowStats struct { + Protocol Protocol + InPort int + Table int + Cookie uint64 + IfName string + Stats FlowStats +} + var _ error = &FlowError{} // A FlowError is an error encountered while marshaling or unmarshaling @@ -439,6 +449,120 @@ func (f *Flow) UnmarshalText(b []byte) error { return nil } +// UnmarshalText unmarshals flows text into a PerFlowStats. +func (f *PerFlowStats) UnmarshalText(b []byte) error { + // Make a copy per documentation for encoding.TextUnmarshaler. + // A string is easier to work with in this case. + s := string(b) + + // Must have one and only one actions=... field in the flow. + ss := strings.Split(s, keyActions+"=") + if len(ss) != 2 || ss[1] == "" { + return &FlowError{ + Err: errNoActions, + } + } + if len(ss) < 2 { + return &FlowError{ + Err: errNotEnoughElements, + } + } + matchers := strings.TrimSpace(ss[0]) + + // Handle matchers first. + ss = strings.Split(matchers, ",") + for i := 0; i < len(ss); i++ { + if !strings.Contains(ss[i], "=") { + // that means this will be a protocol field. + if ss[i] != "" { + f.Protocol = Protocol(ss[i]) + } + continue + } + + // All remaining comma-separated values should be in key=value format + kv := strings.Split(ss[i], "=") + if len(kv) != 2 { + continue + } + kv[1] = strings.TrimSpace(kv[1]) + + switch strings.TrimSpace(kv[0]) { + case cookie: + // Parse cookie into struct field. + cookie, err := strconv.ParseUint(kv[1], 0, 64) + if err != nil { + return &FlowError{ + Str: kv[1], + Err: err, + } + } + f.Cookie = cookie + continue + case inPort: + // Parse in_port into struct field. + s := kv[1] + if strings.TrimSpace(s) == portLOCAL { + f.InPort = PortLOCAL + continue + } + // Try to read as integer port numbers first + port, err := strconv.ParseInt(s, 10, 0) + if err != nil { + f.IfName = s + } else { + f.InPort = int(port) + } + continue + case table: + // Parse table into struct field. + table, err := strconv.ParseInt(kv[1], 10, 0) + if err != nil { + return &FlowError{ + Str: kv[1], + Err: err, + } + } + f.Table = int(table) + continue + case nPackets: + // Parse nPackets into struct field. + pktCount, err := strconv.ParseUint(kv[1], 0, 64) + if err != nil { + return &FlowError{ + Str: kv[1], + Err: err, + } + } + f.Stats.PacketCount = uint64(pktCount) + continue + case nBytes: + // Parse nBytes into struct field. + byteCount, err := strconv.ParseUint(kv[1], 0, 64) + if err != nil { + return &FlowError{ + Str: kv[1], + Err: err, + } + } + f.Stats.ByteCount = uint64(byteCount) + continue + case duration, hardAge, idleAge, priority, idleTimeout, keyActions: + // ignore those fields. + continue + } + + // All arbitrary key/value pairs that + // don't match the case above. + _, err := parseMatch(kv[0], kv[1]) + if err != nil { + return err + } + } + + return nil +} + // MatchFlow converts Flow into MatchFlow. func (f *Flow) MatchFlow() *MatchFlow { return &MatchFlow{ diff --git a/ovs/flow_test.go b/ovs/flow_test.go index 86f87f4..a72baf3 100644 --- a/ovs/flow_test.go +++ b/ovs/flow_test.go @@ -1313,3 +1313,73 @@ func flowErrorEqual(a error, b error) bool { return reflect.DeepEqual(fa, fb) } + +// perflowstatsEqual determines if two possible PerFlowStats are equal. +func perflowstatsEqual(a *PerFlowStats, b *PerFlowStats) bool { + // Special case: both nil is OK + if a == nil && b == nil { + return true + } + + return reflect.DeepEqual(a, b) +} + +func TestPerFlowStatsUnmarshalText(t *testing.T) { + var tests = []struct { + desc string + s string + f *PerFlowStats + err error + }{ + { + desc: "empty Flow string, need actions fields", + err: &FlowError{ + Err: errNoActions, + }, + }, + { + desc: "Flow string with interface name", + s: "priority=10,in_port=eth0,table=0,actions=drop", + f: &PerFlowStats{ + InPort: 0, + IfName: "eth0", + Table: 0, + }, + }, + { + desc: "Flow string with flow stats", + s: "n_packets=13256, n_bytes=1287188, priority=10,in_port=eth0,table=0,actions=drop", + f: &PerFlowStats{ + InPort: 0, + IfName: "eth0", + Table: 0, + Stats: FlowStats{ + PacketCount: 13256, + ByteCount: 1287188, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + f := new(PerFlowStats) + err := f.UnmarshalText([]byte(tt.s)) + + // Need temporary strings to avoid nil pointer dereference + // panics when checking Error method. + if want, got := tt.err, err; !flowErrorEqual(want, got) { + t.Fatalf("unexpected error:\n- want: %v\n- got: %v", + want, got) + } + if err != nil { + return + } + + if want, got := tt.f, f; !perflowstatsEqual(want, got) { + t.Fatalf("unexpected Flow:\n- want: %#v\n- got: %#v", + want, got) + } + }) + } +} diff --git a/ovs/openflow.go b/ovs/openflow.go index f431413..c831878 100644 --- a/ovs/openflow.go +++ b/ovs/openflow.go @@ -75,6 +75,11 @@ const ( dirDelete = "delete" ) +// Interface names option +const ( + interfaceNamesOption = "--names" +) + // Add pushes zero or more Flows on to the transaction, to be added by // Open vSwitch. If any of the flows are invalid, Add becomes a no-op // and the error will be surfaced when Commit is called. @@ -267,7 +272,7 @@ func (o *OpenFlowService) DumpTables(bridge string) ([]*Table, error) { return tables, err } -// DumpFlowsWithFlowArgs retrieves statistics about all flows for the specified bridge, +// DumpFlowsWithFlowArgs retrieves details about all flows for the specified bridge, // filtering on the specified flow(s), if provided. // If a table has no active flows and has not been used for a lookup or matched // by an incoming packet, it is filtered from the output. @@ -306,13 +311,64 @@ func (o *OpenFlowService) DumpFlowsWithFlowArgs(bridge string, flow *MatchFlow) return flows, err } -// DumpFlows retrieves statistics about all flows for the specified bridge. +// DumpFlows retrieves details about all flows for the specified bridge. // If a table has no active flows and has not been used for a lookup or matched // by an incoming packet, it is filtered from the output. func (o *OpenFlowService) DumpFlows(bridge string) ([]*Flow, error) { return o.DumpFlowsWithFlowArgs(bridge, nil) } +// DumpFlowStatsWithFlowArgs retrieves statistics about all flows for the specified bridge, +// filtering on the specified flow(s), if provided. +// If a table has no active flows and has not been used for a lookup or matched +// by an incoming packet, it is filtered from the output. +// We neeed to add a Matchflow to filter the dumpflow results. For example filter based on table, cookie. +// Report with interface names if useInterfaceNames is set. Port numbers otherwise +func (o *OpenFlowService) DumpFlowStatsWithFlowArgs(bridge string, flow *MatchFlow, useInterfaceNames bool) ([]*PerFlowStats, error) { + args := []string{"dump-flows", bridge} + if useInterfaceNames { + args = append(args, interfaceNamesOption) + } + args = append(args, o.c.ofctlFlags...) + if flow != nil { + fb, err := flow.MarshalText() + if err != nil { + return nil, err + } + args = append(args, string(fb)) + } + out, err := o.exec(args...) + if err != nil { + return nil, err + } + + var flows []*PerFlowStats + err = parseEachLine(out, dumpFlowsPrefix, func(b []byte) error { + // Do not attempt to parse ST_FLOW messages. + if bytes.Contains(b, dumpFlowsPrefix) { + return nil + } + + f := new(PerFlowStats) + if err := f.UnmarshalText(b); err != nil { + return err + } + + flows = append(flows, f) + return nil + }) + + return flows, err +} + +// DumpFlowStats retrieves statistics about all matching flows for the specified bridge. +// If a table has no active flows and has not been used for a lookup or matched +// by an incoming packet, it is filtered from the output. +// Use nil MatchFlow if no filtering is desired. +func (o *OpenFlowService) DumpFlowStats(bridge string, flow *MatchFlow, useInterfaceNames bool) ([]*PerFlowStats, error) { + return o.DumpFlowStatsWithFlowArgs(bridge, flow, useInterfaceNames) +} + // DumpAggregate retrieves statistics about the specified flow attached to the // specified bridge. func (o *OpenFlowService) DumpAggregate(bridge string, flow *MatchFlow) (*FlowStats, error) { diff --git a/ovs/openflow_test.go b/ovs/openflow_test.go index eaa408b..8c7f1f1 100644 --- a/ovs/openflow_test.go +++ b/ovs/openflow_test.go @@ -1343,3 +1343,199 @@ func mustVerifyFlowBundle(t *testing.T, stdin io.Reader, flows []*Flow, matchFlo } } } + +func TestClientOpenFlowDumpFlowStatsWithFlowArgsInterfaceNames(t *testing.T) { + tests := []struct { + name string + table string + cookie uint64 + cookieMask uint64 + input string + flows string + want []*PerFlowStats + err error + }{ + { + name: "test single flow", + input: "br0", + table: "45", + cookie: 0, + cookieMask: 0x0, + flows: `NXST_FLOW reply (xid=0x4): + cookie=0x01, duration=9215.748s, table=45, n_packets=6, n_bytes=480, idle_age=9206, priority=820,in_port=LOCAL actions=mod_vlan_vid:10,output:1 +`, + want: []*PerFlowStats{ + { + InPort: PortLOCAL, + Table: 45, + Cookie: 1, + Stats: FlowStats{ + PacketCount: 6, + ByteCount: 480, + }, + }, + }, + err: nil, + }, + { + name: "test multiple flows", + input: "br0", + table: "45", + cookie: 0, + cookieMask: 0x1, + flows: `NXST_FLOW reply (xid=0x4): + cookie=0x0, duration=9215.748s, table=45, n_packets=6, n_bytes=480, idle_age=9206, priority=820,in_port=LOCAL actions=mod_vlan_vid:10,output:1 + cookie=0x0, duration=1121991.329s, table=45, n_packets=0, n_bytes=0, priority=110,ip,dl_src=f1:f2:f3:f4:f5:f6 actions=ct(table=51) + cookie=0x0, duration=9215.748s, table=45, n_packets=56, n_bytes=1480, idle_age=9206, priority=820,in_port=eth0 actions=mod_vlan_vid:10,output:1 +`, + want: []*PerFlowStats{ + { + InPort: PortLOCAL, + Table: 45, + Cookie: 0, + Stats: FlowStats{ + PacketCount: 6, + ByteCount: 480, + }, + }, + { + Protocol: ProtocolIPv4, + Table: 45, + Stats: FlowStats{ + PacketCount: 0, + ByteCount: 0, + }, + }, + { + IfName: "eth0", + Table: 45, + Cookie: 0, + Stats: FlowStats{ + PacketCount: 56, + ByteCount: 1480, + }, + }, + }, + err: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, _ := testClient([]OptionFunc{Timeout(1)}, func(cmd string, args ...string) ([]byte, error) { + if want, got := "ovs-ofctl", cmd; want != got { + t.Fatalf("incorrect command:\n- want: %v\n- got: %v", + want, got) + } + filterArg := "cookie=0x0000000000000000/0xffffffffffffffff," + "table=" + tt.table + wantArgs := []string{ + "--timeout=1", + "dump-flows", + string(tt.input), + "--names", + filterArg, + } + if want, got := wantArgs, args; !reflect.DeepEqual(want, got) { + t.Fatalf("incorrect arguments\n- want: %v\n- got: %v", + want, got) + } + return []byte(tt.flows), tt.err + }).OpenFlow.DumpFlowStatsWithFlowArgs(tt.input, &MatchFlow{Cookie: 0, + CookieMask: 0xffffffffffffffff, + Table: 45}, true) + if len(tt.want) != len(got) { + t.Errorf("got %d", len(got)) + t.Errorf("want %d", len(tt.want)) + t.Fatal("expected return value to be equal") + } + for i := range tt.want { + if !perflowstatsEqual(tt.want[i], got[i]) { + t.Errorf("got %+v", got[i]) + t.Errorf("want %+v", tt.want[i]) + t.Fatal("expected return value to be equal") + } + } + }) + } +} + +func TestClientOpenFlowDumpFlowStatsWithFlowArgsPortNumbers(t *testing.T) { + tests := []struct { + name string + table string + cookie uint64 + cookieMask uint64 + input string + flows string + want []*PerFlowStats + err error + }{ + { + name: "test multiple flows", + input: "br0", + table: "45", + cookie: 0, + cookieMask: 0x1, + flows: `NXST_FLOW reply (xid=0x4): + cookie=0x0, duration=9215.748s, table=45, n_packets=6, n_bytes=480, idle_age=9206, priority=820,in_port=LOCAL actions=mod_vlan_vid:10,output:1 + cookie=0x0, duration=9215.748s, table=45, n_packets=56, n_bytes=1480, idle_age=9206, priority=820,in_port=20 actions=mod_vlan_vid:10,output:1 +`, + want: []*PerFlowStats{ + { + InPort: PortLOCAL, + Table: 45, + Cookie: 0, + Stats: FlowStats{ + PacketCount: 6, + ByteCount: 480, + }, + }, + { + InPort: 20, + Table: 45, + Cookie: 0, + Stats: FlowStats{ + PacketCount: 56, + ByteCount: 1480, + }, + }, + }, + err: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, _ := testClient([]OptionFunc{Timeout(1)}, func(cmd string, args ...string) ([]byte, error) { + if want, got := "ovs-ofctl", cmd; want != got { + t.Fatalf("incorrect command:\n- want: %v\n- got: %v", + want, got) + } + filterArg := "cookie=0x0000000000000000/0xffffffffffffffff," + "table=" + tt.table + wantArgs := []string{ + "--timeout=1", + "dump-flows", + string(tt.input), + filterArg, + } + if want, got := wantArgs, args; !reflect.DeepEqual(want, got) { + t.Fatalf("incorrect arguments\n- want: %v\n- got: %v", + want, got) + } + return []byte(tt.flows), tt.err + }).OpenFlow.DumpFlowStatsWithFlowArgs(tt.input, &MatchFlow{Cookie: 0, + CookieMask: 0xffffffffffffffff, + Table: 45}, false) + if len(tt.want) != len(got) { + t.Errorf("got %d", len(got)) + t.Errorf("want %d", len(tt.want)) + t.Fatal("expected return value to be equal") + } + for i := range tt.want { + if !perflowstatsEqual(tt.want[i], got[i]) { + t.Errorf("got %+v", got[i]) + t.Errorf("want %+v", tt.want[i]) + t.Fatal("expected return value to be equal") + } + } + }) + } +} From a49b72ae7285961b9c67bf0716f3d8443dd12fea Mon Sep 17 00:00:00 2001 From: Manmeet Singh Date: Fri, 18 Oct 2024 12:11:28 -0700 Subject: [PATCH 2/3] Extend existing Flow structure --- ovs/flow.go | 151 ++++-------------------- ovs/flow_test.go | 78 ++----------- ovs/openflow.go | 59 +--------- ovs/openflow_test.go | 268 ++++++++++++------------------------------- 4 files changed, 110 insertions(+), 446 deletions(-) diff --git a/ovs/flow.go b/ovs/flow.go index 7e4ba64..522429d 100644 --- a/ovs/flow.go +++ b/ovs/flow.go @@ -53,7 +53,7 @@ const ( ProtocolUDPv6 Protocol = "udp6" ) -// A Flow is an OpenFlow flow meant for adding flows to a software bridge. It can be marshaled +// A Flow is an OpenFlow flow meant for adding/fetching flows to a software bridge. It can be marshaled // to and from its textual form for use with Open vSwitch. type Flow struct { Priority int @@ -64,6 +64,7 @@ type Flow struct { IdleTimeout int Cookie uint64 Actions []Action + Stats FlowStats } // A LearnedFlow is defined as part of the Learn action. @@ -82,16 +83,6 @@ type LearnedFlow struct { Limit int } -// A PerFlowStats is meant for fetching FlowStats per flow -type PerFlowStats struct { - Protocol Protocol - InPort int - Table int - Cookie uint64 - IfName string - Stats FlowStats -} - var _ error = &FlowError{} // A FlowError is an error encountered while marshaling or unmarshaling @@ -406,7 +397,29 @@ func (f *Flow) UnmarshalText(b []byte) error { } f.Table = int(table) continue - case duration, nPackets, nBytes, hardAge, idleAge: + case nPackets: + // Parse nPackets into struct field. + pktCount, err := strconv.ParseUint(kv[1], 0, 64) + if err != nil { + return &FlowError{ + Str: kv[1], + Err: err, + } + } + f.Stats.PacketCount = uint64(pktCount) + continue + case nBytes: + // Parse nBytes into struct field. + byteCount, err := strconv.ParseUint(kv[1], 0, 64) + if err != nil { + return &FlowError{ + Str: kv[1], + Err: err, + } + } + f.Stats.ByteCount = uint64(byteCount) + continue + case duration, hardAge, idleAge: // ignore those fields. continue } @@ -449,120 +462,6 @@ func (f *Flow) UnmarshalText(b []byte) error { return nil } -// UnmarshalText unmarshals flows text into a PerFlowStats. -func (f *PerFlowStats) UnmarshalText(b []byte) error { - // Make a copy per documentation for encoding.TextUnmarshaler. - // A string is easier to work with in this case. - s := string(b) - - // Must have one and only one actions=... field in the flow. - ss := strings.Split(s, keyActions+"=") - if len(ss) != 2 || ss[1] == "" { - return &FlowError{ - Err: errNoActions, - } - } - if len(ss) < 2 { - return &FlowError{ - Err: errNotEnoughElements, - } - } - matchers := strings.TrimSpace(ss[0]) - - // Handle matchers first. - ss = strings.Split(matchers, ",") - for i := 0; i < len(ss); i++ { - if !strings.Contains(ss[i], "=") { - // that means this will be a protocol field. - if ss[i] != "" { - f.Protocol = Protocol(ss[i]) - } - continue - } - - // All remaining comma-separated values should be in key=value format - kv := strings.Split(ss[i], "=") - if len(kv) != 2 { - continue - } - kv[1] = strings.TrimSpace(kv[1]) - - switch strings.TrimSpace(kv[0]) { - case cookie: - // Parse cookie into struct field. - cookie, err := strconv.ParseUint(kv[1], 0, 64) - if err != nil { - return &FlowError{ - Str: kv[1], - Err: err, - } - } - f.Cookie = cookie - continue - case inPort: - // Parse in_port into struct field. - s := kv[1] - if strings.TrimSpace(s) == portLOCAL { - f.InPort = PortLOCAL - continue - } - // Try to read as integer port numbers first - port, err := strconv.ParseInt(s, 10, 0) - if err != nil { - f.IfName = s - } else { - f.InPort = int(port) - } - continue - case table: - // Parse table into struct field. - table, err := strconv.ParseInt(kv[1], 10, 0) - if err != nil { - return &FlowError{ - Str: kv[1], - Err: err, - } - } - f.Table = int(table) - continue - case nPackets: - // Parse nPackets into struct field. - pktCount, err := strconv.ParseUint(kv[1], 0, 64) - if err != nil { - return &FlowError{ - Str: kv[1], - Err: err, - } - } - f.Stats.PacketCount = uint64(pktCount) - continue - case nBytes: - // Parse nBytes into struct field. - byteCount, err := strconv.ParseUint(kv[1], 0, 64) - if err != nil { - return &FlowError{ - Str: kv[1], - Err: err, - } - } - f.Stats.ByteCount = uint64(byteCount) - continue - case duration, hardAge, idleAge, priority, idleTimeout, keyActions: - // ignore those fields. - continue - } - - // All arbitrary key/value pairs that - // don't match the case above. - _, err := parseMatch(kv[0], kv[1]) - if err != nil { - return err - } - } - - return nil -} - // MatchFlow converts Flow into MatchFlow. func (f *Flow) MatchFlow() *MatchFlow { return &MatchFlow{ diff --git a/ovs/flow_test.go b/ovs/flow_test.go index a72baf3..3b060d2 100644 --- a/ovs/flow_test.go +++ b/ovs/flow_test.go @@ -785,6 +785,10 @@ func TestFlowUnmarshalText(t *testing.T) { ModVLANVID(10), Output(1), }, + Stats: FlowStats{ + PacketCount: 6, + ByteCount: 480, + }, }, }, { @@ -819,6 +823,10 @@ func TestFlowUnmarshalText(t *testing.T) { Actions: []Action{ ConnectionTracking("commit,table=65"), }, + Stats: FlowStats{ + PacketCount: 3, + ByteCount: 234, + }, }, }, { @@ -1313,73 +1321,3 @@ func flowErrorEqual(a error, b error) bool { return reflect.DeepEqual(fa, fb) } - -// perflowstatsEqual determines if two possible PerFlowStats are equal. -func perflowstatsEqual(a *PerFlowStats, b *PerFlowStats) bool { - // Special case: both nil is OK - if a == nil && b == nil { - return true - } - - return reflect.DeepEqual(a, b) -} - -func TestPerFlowStatsUnmarshalText(t *testing.T) { - var tests = []struct { - desc string - s string - f *PerFlowStats - err error - }{ - { - desc: "empty Flow string, need actions fields", - err: &FlowError{ - Err: errNoActions, - }, - }, - { - desc: "Flow string with interface name", - s: "priority=10,in_port=eth0,table=0,actions=drop", - f: &PerFlowStats{ - InPort: 0, - IfName: "eth0", - Table: 0, - }, - }, - { - desc: "Flow string with flow stats", - s: "n_packets=13256, n_bytes=1287188, priority=10,in_port=eth0,table=0,actions=drop", - f: &PerFlowStats{ - InPort: 0, - IfName: "eth0", - Table: 0, - Stats: FlowStats{ - PacketCount: 13256, - ByteCount: 1287188, - }, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.desc, func(t *testing.T) { - f := new(PerFlowStats) - err := f.UnmarshalText([]byte(tt.s)) - - // Need temporary strings to avoid nil pointer dereference - // panics when checking Error method. - if want, got := tt.err, err; !flowErrorEqual(want, got) { - t.Fatalf("unexpected error:\n- want: %v\n- got: %v", - want, got) - } - if err != nil { - return - } - - if want, got := tt.f, f; !perflowstatsEqual(want, got) { - t.Fatalf("unexpected Flow:\n- want: %#v\n- got: %#v", - want, got) - } - }) - } -} diff --git a/ovs/openflow.go b/ovs/openflow.go index c831878..a3c6c32 100644 --- a/ovs/openflow.go +++ b/ovs/openflow.go @@ -75,11 +75,6 @@ const ( dirDelete = "delete" ) -// Interface names option -const ( - interfaceNamesOption = "--names" -) - // Add pushes zero or more Flows on to the transaction, to be added by // Open vSwitch. If any of the flows are invalid, Add becomes a no-op // and the error will be surfaced when Commit is called. @@ -272,7 +267,7 @@ func (o *OpenFlowService) DumpTables(bridge string) ([]*Table, error) { return tables, err } -// DumpFlowsWithFlowArgs retrieves details about all flows for the specified bridge, +// DumpFlowsWithFlowArgs retrieves statistics about all flows for the specified bridge, // filtering on the specified flow(s), if provided. // If a table has no active flows and has not been used for a lookup or matched // by an incoming packet, it is filtered from the output. @@ -311,62 +306,18 @@ func (o *OpenFlowService) DumpFlowsWithFlowArgs(bridge string, flow *MatchFlow) return flows, err } -// DumpFlows retrieves details about all flows for the specified bridge. +// DumpFlows retrieves statistics about all flows for the specified bridge. // If a table has no active flows and has not been used for a lookup or matched // by an incoming packet, it is filtered from the output. func (o *OpenFlowService) DumpFlows(bridge string) ([]*Flow, error) { return o.DumpFlowsWithFlowArgs(bridge, nil) } -// DumpFlowStatsWithFlowArgs retrieves statistics about all flows for the specified bridge, -// filtering on the specified flow(s), if provided. -// If a table has no active flows and has not been used for a lookup or matched -// by an incoming packet, it is filtered from the output. -// We neeed to add a Matchflow to filter the dumpflow results. For example filter based on table, cookie. -// Report with interface names if useInterfaceNames is set. Port numbers otherwise -func (o *OpenFlowService) DumpFlowStatsWithFlowArgs(bridge string, flow *MatchFlow, useInterfaceNames bool) ([]*PerFlowStats, error) { - args := []string{"dump-flows", bridge} - if useInterfaceNames { - args = append(args, interfaceNamesOption) - } - args = append(args, o.c.ofctlFlags...) - if flow != nil { - fb, err := flow.MarshalText() - if err != nil { - return nil, err - } - args = append(args, string(fb)) - } - out, err := o.exec(args...) - if err != nil { - return nil, err - } - - var flows []*PerFlowStats - err = parseEachLine(out, dumpFlowsPrefix, func(b []byte) error { - // Do not attempt to parse ST_FLOW messages. - if bytes.Contains(b, dumpFlowsPrefix) { - return nil - } - - f := new(PerFlowStats) - if err := f.UnmarshalText(b); err != nil { - return err - } - - flows = append(flows, f) - return nil - }) - - return flows, err -} - -// DumpFlowStats retrieves statistics about all matching flows for the specified bridge. +// DumpMatchingFlows retrieves statistics of all matching flows for the specified bridge. // If a table has no active flows and has not been used for a lookup or matched // by an incoming packet, it is filtered from the output. -// Use nil MatchFlow if no filtering is desired. -func (o *OpenFlowService) DumpFlowStats(bridge string, flow *MatchFlow, useInterfaceNames bool) ([]*PerFlowStats, error) { - return o.DumpFlowStatsWithFlowArgs(bridge, flow, useInterfaceNames) +func (o *OpenFlowService) DumpMatchingFlows(bridge string, flow *MatchFlow) ([]*Flow, error) { + return o.DumpFlowsWithFlowArgs(bridge, flow) } // DumpAggregate retrieves statistics about the specified flow attached to the diff --git a/ovs/openflow_test.go b/ovs/openflow_test.go index 8c7f1f1..aea427c 100644 --- a/ovs/openflow_test.go +++ b/ovs/openflow_test.go @@ -862,6 +862,10 @@ func TestClientOpenFlowDumpFlows(t *testing.T) { ModVLANVID(10), Output(1), }, + Stats: FlowStats{ + PacketCount: 6, + ByteCount: 480, + }, }, }, err: nil, @@ -886,6 +890,10 @@ func TestClientOpenFlowDumpFlows(t *testing.T) { ModVLANVID(10), Output(1), }, + Stats: FlowStats{ + PacketCount: 6, + ByteCount: 480, + }, }, { Priority: 110, @@ -897,6 +905,10 @@ func TestClientOpenFlowDumpFlows(t *testing.T) { Actions: []Action{ ConnectionTracking("table=51"), }, + Stats: FlowStats{ + PacketCount: 0, + ByteCount: 0, + }, }, { Priority: 101, @@ -912,6 +924,10 @@ func TestClientOpenFlowDumpFlows(t *testing.T) { Actions: []Action{ ConnectionTracking("commit,table=65"), }, + Stats: FlowStats{ + PacketCount: 3, + ByteCount: 234, + }, }, { Priority: 4040, @@ -925,6 +941,10 @@ func TestClientOpenFlowDumpFlows(t *testing.T) { Actions: []Action{ Output(19), }, + Stats: FlowStats{ + PacketCount: 0, + ByteCount: 0, + }, }, { Priority: 4321, @@ -940,6 +960,10 @@ func TestClientOpenFlowDumpFlows(t *testing.T) { Actions: []Action{ Resubmit(0, 13), }, + Stats: FlowStats{ + PacketCount: 0, + ByteCount: 0, + }, }, }, err: nil, @@ -965,6 +989,10 @@ NXST_FLOW reply (xid=0x4): ModVLANVID(10), Output(1), }, + Stats: FlowStats{ + PacketCount: 6, + ByteCount: 480, + }, }, { Priority: 110, @@ -976,6 +1004,10 @@ NXST_FLOW reply (xid=0x4): Actions: []Action{ ConnectionTracking("table=51"), }, + Stats: FlowStats{ + PacketCount: 0, + ByteCount: 0, + }, }, { Priority: 101, @@ -991,6 +1023,10 @@ NXST_FLOW reply (xid=0x4): Actions: []Action{ ConnectionTracking("commit,table=65"), }, + Stats: FlowStats{ + PacketCount: 3, + ByteCount: 234, + }, }, { Priority: 4040, @@ -1004,6 +1040,10 @@ NXST_FLOW reply (xid=0x4): Actions: []Action{ Output(19), }, + Stats: FlowStats{ + PacketCount: 0, + ByteCount: 0, + }, }, { Priority: 4321, @@ -1019,6 +1059,10 @@ NXST_FLOW reply (xid=0x4): Actions: []Action{ Resubmit(0, 13), }, + Stats: FlowStats{ + PacketCount: 0, + ByteCount: 0, + }, }, }, err: nil, @@ -1089,6 +1133,10 @@ func TestClientOpenFlowDumpFlowsWithFlowArgs(t *testing.T) { ModVLANVID(10), Output(1), }, + Stats: FlowStats{ + PacketCount: 6, + ByteCount: 480, + }, }, }, err: nil, @@ -1114,6 +1162,10 @@ func TestClientOpenFlowDumpFlowsWithFlowArgs(t *testing.T) { ModVLANVID(10), Output(1), }, + Stats: FlowStats{ + PacketCount: 6, + ByteCount: 480, + }, }, { Priority: 110, @@ -1125,6 +1177,10 @@ func TestClientOpenFlowDumpFlowsWithFlowArgs(t *testing.T) { Actions: []Action{ ConnectionTracking("table=51"), }, + Stats: FlowStats{ + PacketCount: 0, + ByteCount: 0, + }, }, }, err: nil, @@ -1193,6 +1249,10 @@ func TestClientOpenFlowDumpFlows15(t *testing.T) { Actions: []Action{ ConnectionTracking("table=1"), }, + Stats: FlowStats{ + PacketCount: 1127501, + ByteCount: 1595250938, + }, }, }, err: nil, @@ -1216,6 +1276,10 @@ func TestClientOpenFlowDumpFlows15(t *testing.T) { Actions: []Action{ ConnectionTracking("table=1"), }, + Stats: FlowStats{ + PacketCount: 1127501, + ByteCount: 1595250938, + }, }, { Priority: 0, @@ -1224,6 +1288,10 @@ func TestClientOpenFlowDumpFlows15(t *testing.T) { Actions: []Action{ Normal(), }, + Stats: FlowStats{ + PacketCount: 7370490, + ByteCount: 893401420, + }, }, { Priority: 1000, @@ -1236,6 +1304,10 @@ func TestClientOpenFlowDumpFlows15(t *testing.T) { ConnectionTracking("commit"), Normal(), }, + Stats: FlowStats{ + PacketCount: 1068, + ByteCount: 388186, + }, }, }, err: nil, @@ -1343,199 +1415,3 @@ func mustVerifyFlowBundle(t *testing.T, stdin io.Reader, flows []*Flow, matchFlo } } } - -func TestClientOpenFlowDumpFlowStatsWithFlowArgsInterfaceNames(t *testing.T) { - tests := []struct { - name string - table string - cookie uint64 - cookieMask uint64 - input string - flows string - want []*PerFlowStats - err error - }{ - { - name: "test single flow", - input: "br0", - table: "45", - cookie: 0, - cookieMask: 0x0, - flows: `NXST_FLOW reply (xid=0x4): - cookie=0x01, duration=9215.748s, table=45, n_packets=6, n_bytes=480, idle_age=9206, priority=820,in_port=LOCAL actions=mod_vlan_vid:10,output:1 -`, - want: []*PerFlowStats{ - { - InPort: PortLOCAL, - Table: 45, - Cookie: 1, - Stats: FlowStats{ - PacketCount: 6, - ByteCount: 480, - }, - }, - }, - err: nil, - }, - { - name: "test multiple flows", - input: "br0", - table: "45", - cookie: 0, - cookieMask: 0x1, - flows: `NXST_FLOW reply (xid=0x4): - cookie=0x0, duration=9215.748s, table=45, n_packets=6, n_bytes=480, idle_age=9206, priority=820,in_port=LOCAL actions=mod_vlan_vid:10,output:1 - cookie=0x0, duration=1121991.329s, table=45, n_packets=0, n_bytes=0, priority=110,ip,dl_src=f1:f2:f3:f4:f5:f6 actions=ct(table=51) - cookie=0x0, duration=9215.748s, table=45, n_packets=56, n_bytes=1480, idle_age=9206, priority=820,in_port=eth0 actions=mod_vlan_vid:10,output:1 -`, - want: []*PerFlowStats{ - { - InPort: PortLOCAL, - Table: 45, - Cookie: 0, - Stats: FlowStats{ - PacketCount: 6, - ByteCount: 480, - }, - }, - { - Protocol: ProtocolIPv4, - Table: 45, - Stats: FlowStats{ - PacketCount: 0, - ByteCount: 0, - }, - }, - { - IfName: "eth0", - Table: 45, - Cookie: 0, - Stats: FlowStats{ - PacketCount: 56, - ByteCount: 1480, - }, - }, - }, - err: nil, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, _ := testClient([]OptionFunc{Timeout(1)}, func(cmd string, args ...string) ([]byte, error) { - if want, got := "ovs-ofctl", cmd; want != got { - t.Fatalf("incorrect command:\n- want: %v\n- got: %v", - want, got) - } - filterArg := "cookie=0x0000000000000000/0xffffffffffffffff," + "table=" + tt.table - wantArgs := []string{ - "--timeout=1", - "dump-flows", - string(tt.input), - "--names", - filterArg, - } - if want, got := wantArgs, args; !reflect.DeepEqual(want, got) { - t.Fatalf("incorrect arguments\n- want: %v\n- got: %v", - want, got) - } - return []byte(tt.flows), tt.err - }).OpenFlow.DumpFlowStatsWithFlowArgs(tt.input, &MatchFlow{Cookie: 0, - CookieMask: 0xffffffffffffffff, - Table: 45}, true) - if len(tt.want) != len(got) { - t.Errorf("got %d", len(got)) - t.Errorf("want %d", len(tt.want)) - t.Fatal("expected return value to be equal") - } - for i := range tt.want { - if !perflowstatsEqual(tt.want[i], got[i]) { - t.Errorf("got %+v", got[i]) - t.Errorf("want %+v", tt.want[i]) - t.Fatal("expected return value to be equal") - } - } - }) - } -} - -func TestClientOpenFlowDumpFlowStatsWithFlowArgsPortNumbers(t *testing.T) { - tests := []struct { - name string - table string - cookie uint64 - cookieMask uint64 - input string - flows string - want []*PerFlowStats - err error - }{ - { - name: "test multiple flows", - input: "br0", - table: "45", - cookie: 0, - cookieMask: 0x1, - flows: `NXST_FLOW reply (xid=0x4): - cookie=0x0, duration=9215.748s, table=45, n_packets=6, n_bytes=480, idle_age=9206, priority=820,in_port=LOCAL actions=mod_vlan_vid:10,output:1 - cookie=0x0, duration=9215.748s, table=45, n_packets=56, n_bytes=1480, idle_age=9206, priority=820,in_port=20 actions=mod_vlan_vid:10,output:1 -`, - want: []*PerFlowStats{ - { - InPort: PortLOCAL, - Table: 45, - Cookie: 0, - Stats: FlowStats{ - PacketCount: 6, - ByteCount: 480, - }, - }, - { - InPort: 20, - Table: 45, - Cookie: 0, - Stats: FlowStats{ - PacketCount: 56, - ByteCount: 1480, - }, - }, - }, - err: nil, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, _ := testClient([]OptionFunc{Timeout(1)}, func(cmd string, args ...string) ([]byte, error) { - if want, got := "ovs-ofctl", cmd; want != got { - t.Fatalf("incorrect command:\n- want: %v\n- got: %v", - want, got) - } - filterArg := "cookie=0x0000000000000000/0xffffffffffffffff," + "table=" + tt.table - wantArgs := []string{ - "--timeout=1", - "dump-flows", - string(tt.input), - filterArg, - } - if want, got := wantArgs, args; !reflect.DeepEqual(want, got) { - t.Fatalf("incorrect arguments\n- want: %v\n- got: %v", - want, got) - } - return []byte(tt.flows), tt.err - }).OpenFlow.DumpFlowStatsWithFlowArgs(tt.input, &MatchFlow{Cookie: 0, - CookieMask: 0xffffffffffffffff, - Table: 45}, false) - if len(tt.want) != len(got) { - t.Errorf("got %d", len(got)) - t.Errorf("want %d", len(tt.want)) - t.Fatal("expected return value to be equal") - } - for i := range tt.want { - if !perflowstatsEqual(tt.want[i], got[i]) { - t.Errorf("got %+v", got[i]) - t.Errorf("want %+v", tt.want[i]) - t.Fatal("expected return value to be equal") - } - } - }) - } -} From 80974b0dc65cd4dd1000f412bf06730531704cb8 Mon Sep 17 00:00:00 2001 From: Manmeet Singh Date: Mon, 21 Oct 2024 10:35:02 -0700 Subject: [PATCH 3/3] Clean up --- ovs/openflow.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/ovs/openflow.go b/ovs/openflow.go index a3c6c32..f431413 100644 --- a/ovs/openflow.go +++ b/ovs/openflow.go @@ -313,13 +313,6 @@ func (o *OpenFlowService) DumpFlows(bridge string) ([]*Flow, error) { return o.DumpFlowsWithFlowArgs(bridge, nil) } -// DumpMatchingFlows retrieves statistics of all matching flows for the specified bridge. -// If a table has no active flows and has not been used for a lookup or matched -// by an incoming packet, it is filtered from the output. -func (o *OpenFlowService) DumpMatchingFlows(bridge string, flow *MatchFlow) ([]*Flow, error) { - return o.DumpFlowsWithFlowArgs(bridge, flow) -} - // DumpAggregate retrieves statistics about the specified flow attached to the // specified bridge. func (o *OpenFlowService) DumpAggregate(bridge string, flow *MatchFlow) (*FlowStats, error) {