Re: [ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
On Mon, May 07, 2018 at 10:11:44PM +0530, SatyaValli wrote:
> From: SatyaValli
>
> This Patch provides implementation Existing flow entry statistics are
> redefined as standard OXS(OpenFlow Extensible Statistics) fields for
> displaying the arbitrary flow stats.
Thanks for the revision.
I see that you used "LW" to stand for "lightweight", but I don't see
anything that explains that.
I don't see anything that actually implements the lightweight flow
requests and replies; for example, ofp-flow.[ch] doesn't seem to have
any way to decode or encode them. (If you don't intend to implement
them here, then it may not be useful to define OFPTYPE_* and OFPRAW_*
for them.)
I don't see a reason to separate OFPRAW_OFPST11_FLOW_REQUEST and
OFPRAW_OFPST15_FLOW_REQUEST. They have the same format and the same
code is used to process both.
In lib/automake.mk, please indent with a tab to match the rest of the
file.
ofputil_decode_flow_removed() must honor the error return value from
oxs_flow_removed_stat_pull().
This does not use any OVS acceptable style:
if (ofputil_pull_ofp11_match
(msg, NULL, NULL, &fs->match, &padded_match_len)) {
The following is one acceptable variation:
if (ofputil_pull_ofp11_match(msg, NULL, NULL, &fs->match,
&padded_match_len)) {
The use of oxs_field_set in ofputil_append_flow_stats_reply() puzzles
me. It is initialized using a loop, which is odd given that it's really
a static value with all of the relevant bits set to 1, and then that's
passed to oxs_put_stat() (this function is the only caller), and then
that passes it to ox_put_raw() (again, this function is the only
caller), and then ox_put_raw() checks that each bit is set. Is there
some big plan here? Why can't ox_put_raw() just append all the fields
without needing this oxs_field_set parameter?
Same thing for aggregate stats.
Why do ox_put_raw() and ox_put_agg_raw() check for null stats
parameters? I don't think they're ever called with nulls.
I don't see value in ox_put_raw() and ox_put_agg_raw() returning byte
counts. It doesn't seem particularly useful here.
The !false test in oxs_flow_rem_stat_fields_pull() doesn't make sense:
if (error == OFPERR_OFPBMC_BAD_FIELD && !false) {
oxs_pull_flow_rem_stat_entry() doesn't seem to check that payload
lengths are correct.
In oxs_pull_flow_rem_stat_entry(), the following:
fr->duration_sec = ((uint32_t) ((duration_ &
0xLL) >> 32));
fr->duration_nsec = ((uint32_t) (duration_ & 0xLL));
can be written as:
fr->duration_sec = duration >> 32;
fr->duration_nsec = duration;
Thanks,
Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
From: SatyaValli
This Patch provides implementation Existing flow entry statistics are
redefined as standard OXS(OpenFlow Extensible Statistics) fields for
displaying the arbitrary flow stats.
To support this implementation below messages are newly added
OFPRAW_OFPT15_FLOW_REMOVED,
OFPRAW_OFPST15_FLOW_REQUEST,
OFPRAW_OFPST15_LW_FLOW_REQUEST,
OFPRAW_OFPST15_AGGREGATE_REQUEST,
OFPRAW_OFPST15_FLOW_REPLY,
OFPRAW_OFPST15_LW_FLOW_REPLY,
OFPRAW_OFPST15_AGGREGATE_REPLY,
The current commit adds support for the new feature in flow statistics
multipart messages,aggregate multipart messages and OXS support for flow
removal message, individual flow description messages.
"ovs-ofctl dump-flows" & "ovs-ofctl dump-aggregate" now accepts a new option
"--oxs-stats" provided with the arbitrary OXS fields for displaying the
desired flow stats.
Below are Commands to display OXS stats field wise
Flow Statistics Multipart
ovs-ofctl dump-flows -O OpenFlow15 --oxs-stats=idle_time
ovs-ofctl dump-flows -O OpenFlow15 --oxs-stats=packet_count
ovs-ofctl dump-flows -O OpenFlow15 --oxs-stats=byte_count
ovs-ofctl dump-flows -O OpenFlow15 --oxs-stats=duration
Flow Statistics Multipart
ovs-ofctl dump-flows -O OpenFlow15 --oxs-stats=idle_time
ovs-ofctl dump-flows -O OpenFlow15 --oxs-stats=packet_count
ovs-ofctl dump-flows -O OpenFlow15 --oxs-stats=byte_count
ovs-ofctl dump-flows -O OpenFlow15 --oxs-stats=duration
Aggregate Flow Statistics Multipart
ovs-ofctl dump-aggregate -O OpenFlow15 --oxs-stats=packet_count
ovs-ofctl dump-aggregate -O OpenFlow15 --oxs-stats=byte_count
ovs-ofctl dump-aggregate -O OpenFlow15 --oxs-stats=flow_count
Signed-off-by: Satya Valli
Co-authored-by: Lavanya Harivelam
Signed-off-by: Lavanya Harivelam
Co-authored-by: Surya Muttamsetty
Signed-off-by: Surya Muttamsetty
Co-authored-by: Manasa Cherukupally
Signed-off-by: Manasa Cherukupally
Co-authored-by: Pavani Panthagada
Signed-off-by: Pavani Panthagada
---
NEWS|6 +
include/openflow/openflow-1.5.h | 59 +++
include/openvswitch/ofp-msgs.h | 31 +-
include/openvswitch/ofp-print.h |7 +
lib/automake.mk |2 +
lib/ofp-bundle.c|2 +
lib/ofp-flow.c | 194 +++-
lib/ofp-monitor.c | 46 +-
lib/ofp-print.c | 55 +++
lib/ox-stat.c | 1015 +++
lib/ox-stat.h | 54 +++
lib/rconn.c |2 +
lib/vconn.c |3 +-
ofproto/ofproto.c |2 +
tests/ofp-print.at | 80 +++
tests/ofproto-dpif.at | 85
tests/ofproto.at|1 +
utilities/ovs-ofctl.8.in| 31 +-
utilities/ovs-ofctl.c | 127 -
19 files changed, 1759 insertions(+), 43 deletions(-)
create mode 100644 lib/ox-stat.c
create mode 100644 lib/ox-stat.h
diff --git a/NEWS b/NEWS
index d22ad14..7132b0d 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,12 @@ Post-v2.9.0
* Add minimum network namespace support for Linux.
* New command "lacp/show-stats"
- ovs-ofctl:
+ * Existing flow entry statistics are redefined as standard OXS(OpenFlow
+ Extensible Statistics) fields for displaying the arbitrary flow stats.
+ * Now "ovs-ofctl dump-flows" and "ovs-ofctl dump-aggregate" accepts a new
+ option --oxs-stats provided with the arbitrary OXS fields i.e
+ flow duration, flow count, packet count, byte count for displaying
+ the desired flow stats. See ovs-ofctl(8) for details.
* ovs-ofctl now accepts and display table names in place of numbers. By
default it always accepts names and in interactive use it displays them;
use --names or --no-names to override. See ovs-ofctl(8) for details.
diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
index 73b76d8..6a5e369 100644
--- a/include/openflow/openflow-1.5.h
+++ b/include/openflow/openflow-1.5.h
@@ -163,4 +163,63 @@ struct ofp15_packet_out {
};
OFP_ASSERT(sizeof(struct ofp15_packet_out) == 8);
+struct ofp_oxs_stat {
+ovs_be16 reserved; /* Reserved for future use,
+ * currently zeroed. */
+ovs_be16 length;/* Stats Length */
+};
+
+OFP_ASSERT(sizeof(struct ofp_oxs_stat) == 4);
+
+/* Body of reply to OFPMP_FLOW_DESC request. */
+struct ofp15_flow_desc {
+ovs_be16 length; /* Length of this entry. */
+uint8_t pad2[2]; /* Align to 64 bits. */
+uint8_t table_id; /* ID of table flow came from. */
+uint8_t pad;
+ovs_be16 priority;/* Priority of the entry. */
+ovs_be16 idle_timeout;/* Number of seconds
+ idle before expiration. */
+ovs_be16 hard_timeout;/* Number of seconds
+ before expiration. */
+ovs_be16 flags; /* Bitmap of OFPFF_*. flags. */
+
Re: [ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
On Wed, Feb 28, 2018 at 08:05:56PM +0530, SatyaValli wrote: > From: SatyaValli > > This Patch provides implementation Existing flow entry statistics are > redefined as standard OXS(OpenFlow Extensible Statistics) fields for > displaying the arbitrary flow stats.The existing Flow Stats were renamed > as Flow Description. Thanks for working on this. I have some comments. A big issue here is naming. OF1.5 renamed "flow statistics" to "flow description" and introduced something new named "flow statistics" that is different from the previous meaning. This is really confusing. In the past, when this sort of renaming has happened, we've usually taken one of two different approaches: 1. Use the old name everywhere and invent a new name for whatever OF newly invented. For example, in this case we would continue to use "flow statistics" for its old pre-OF1.5 meaning and invent some new name for OF1.5 flow statistics (from the OF1.5 release notes I suggest "lightweight flow statistics"). 2. Rename everything in the tree to match the new names. For example, in this case we would rename everything involved with flow statistics, regardless of OpenFlow version, to flow description, and then use the new names. We generally find that using a mix of names according to OpenFlow version is more confusing than either of these choices. In this case, I recommend choice #1 above, since the name "flow statistics" is pretty ingrained in the OVS source code and among OpenFlow and OVS users. It's going to be less confusing. So I'd go through and replace OF1.5 FLOW_STATS by something new like LIGHTWEIGHT_FLOW_STATS (that's pretty long so maybe some shorter name can be invented) and then replace FLOW_DESC by FLOW_STATS. Overall, it is likely to be less confusing. There is another way that the approach here is a little different from the ones that we normally take. Usually, when some new version of OpenFlow offers increased granularity of some functionality, or a new feature on top of functionality, we make an attempt to provide some uniformity in the interface, to the extent that we can. For example, existing OpenFlow versions already provide all of the kinds of statistics that the predefined OXS constants provide. It is possible therefore, at some level, to simply pretend that previous OpenFlow versions have OXS support, just that the OXS fields that they support is fixed. In this viewpoint, for example, one might make ofp_print_flow_stats() take an 'oxs' argument instead of a 'show_stats' (or in addition, since the latter controls more than what OXS considers statistics) argument and just print the requested statistics. Usually this "more uniform" view of OpenFlow works out better in the long term. It looks to me that ofp15_flow_stats_request is identical to ofp11_flow_stats_request. If so, please do not define it at all: use the existing structure. I think the new block in ofputil_encode_flow_stats_request() is identical to the existing OXM block except for the raw value in the non-aggregate case. Please avoid so much cut-and-paste for such a trivial change. There is some funny indentation in ofputil_decode_flow_stats_reply(). Please adjust it. ofputil_decode_flow_removed() must honor the error return value from oxs_flow_removed_stat_pull(). The only change to ofp-parse.c is a new #include, which is probably unneeded. I don't understand why ofp_print_flow_oxs_stats() takes its 'oxs' argument and does nothing if 'oxs' is true. Why call the function at all in that case? And in fact the only caller appears to never call it with oxs == true. Same question for ofp_print_flow_oxs_agg_stats(). The oxs_bitmap_*() functions don't look to me like they provide a useful abstraction. I would drop them. I think that this patch is making a lot of progress and I'll look forward to the next version. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
From: SatyaValli
This Patch provides implementation Existing flow entry statistics are
redefined as standard OXS(OpenFlow Extensible Statistics) fields for
displaying the arbitrary flow stats.The existing Flow Stats were renamed
as Flow Description.
To support this implementation below messages are newly added
OFPRAW_OFPT15_FLOW_REMOVED,
OFPRAW_OFPST15_FLOW_REQUEST,
OFPRAW_OFPST15_FLOW_DESC_REQUEST,
OFPRAW_OFPST15_AGGREGATE_REQUEST,
OFPRAW_OFPST15_FLOW_REPLY,
OFPRAW_OFPST15_FLOW_DESC_REPLY,
OFPRAW_OFPST15_AGGREGATE_REPLY,
The current commit adds support for the new feature in flow statistics
multipart messages,aggregate multipart messages and OXS support for flow
removal message, individual flow description messages.
"ovs-ofctl dump-flows" & "ovs-ofctl dump-aggregate" now accepts a new option
"--oxs-stats" provided with the arbitrary OXS fields for displaying the
desired flow stats.
Below are Commands to display OXS stats field wise
Flow Statistics Multipart
ovs-ofctl dump-flows -O OpenFlow15 --oxs-stats=idle_time
ovs-ofctl dump-flows -O OpenFlow15 --oxs-stats=packet_count
ovs-ofctl dump-flows -O OpenFlow15 --oxs-stats=byte_count
ovs-ofctl dump-flows -O OpenFlow15 --oxs-stats=duration
Aggregate Flow Statistics Multipart
ovs-ofctl dump-aggregate -O OpenFlow15 --oxs-stats=packet_count
ovs-ofctl dump-aggregate -O OpenFlow15 --oxs-stats=byte_count
ovs-ofctl dump-aggregate -O OpenFlow15 --oxs-stats=flow_count
Signed-off-by: Satya Valli
Co-authored-by: Lavanya Harivelam
Signed-off-by: Lavanya Harivelam
Co-authored-by: Surya Muttamsetty
Signed-off-by: Surya Muttamsetty
Co-authored-by: Manasa Cherukupally
Signed-off-by: Manasa Cherukupally
Co-authored-by: Pavani Panthagada
Signed-off-by: Pavani Panthagada
---
NEWS|7 +
include/openflow/openflow-1.5.h | 81 +++
include/openvswitch/ofp-msgs.h | 31 +-
include/openvswitch/ofp-print.h |9 +-
lib/automake.mk |2 +
lib/ofp-bundle.c|2 +
lib/ofp-flow.c | 198 +++-
lib/ofp-monitor.c | 46 +-
lib/ofp-parse.c |1 +
lib/ofp-print.c | 52 ++
lib/ox-stat.c | 1040 +++
lib/ox-stat.h | 60 +++
lib/rconn.c |2 +
lib/vconn.c |7 +-
ofproto/ofproto.c |3 +-
tests/ofp-print.at | 80 +++
tests/ofproto-dpif.at | 85
tests/ofproto.at|3 +-
utilities/ovs-ofctl.8.in| 31 +-
utilities/ovs-ofctl.c | 123 -
20 files changed, 1815 insertions(+), 48 deletions(-)
create mode 100644 lib/ox-stat.c
create mode 100644 lib/ox-stat.h
diff --git a/NEWS b/NEWS
index 4230189..0704960 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,13 @@ Post-v2.9.0
"table#". These are not helpful names for the purpose of accepting
and displaying table names, so now tables by default have no names.
- ovs-ofctl:
+ * Existing flow entry statistics are redefined as standard OXS(OpenFlow
+ Extensible Statistics) fields for displaying the arbitrary flow stats.
+ * The existing flow statistics are renamed as Flow Description.
+ * Now "ovs-ofctl dump-flows" and "ovs-ofctl dump-aggregate" accepts a new
+ option --oxs-stats provided with the arbitrary OXS fields i.e
+ flow duration, flow count, packet count, byte count for displaying
+ the desired flow stats. See ovs-ofctl(8) for details.
* ovs-ofctl now accepts and display table names in place of numbers. By
default it always accepts names and in interactive use it displays them;
use --names or --no-names to override. See ovs-ofctl(8) for details.
diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
index 73b76d8..d1870ce 100644
--- a/include/openflow/openflow-1.5.h
+++ b/include/openflow/openflow-1.5.h
@@ -163,4 +163,85 @@ struct ofp15_packet_out {
};
OFP_ASSERT(sizeof(struct ofp15_packet_out) == 8);
+struct ofp_oxs_stat {
+ovs_be16 reserved; /* Reserved for future use,
+ * currently zeroed. */
+ovs_be16 length;/* Stats Length */
+};
+
+OFP_ASSERT(sizeof(struct ofp_oxs_stat) == 4);
+
+/* Body for ofp_multipart_request of type
+ * OFPMP_FLOW_DESC & OFPMP_FLOW_STATS. */
+struct ofp15_flow_stats_request {
+uint8_t table_id; /* ID of table to read (from ofp_table_desc),
+ * OFPTT_ALL for all tables. */
+uint8_t pad[3]; /* Align to 32 bits. */
+ovs_be32 out_port; /* Require matching entries to include this as
+ * an output port. A value of OFP_ANY
+ * indicates no restriction. */
+ovs_be32 out_group; /* Require matching entries to include this as
+
Re: [ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
On Wed, Sep 13, 2017 at 05:12:11PM +0530, SatyaValli wrote: > From: SatyaValli > > This Patch provides implementation Existing flow entry statistics are > redefined as standard OXS(OpenFlow Extensible Statistics) fields for > displaying the arbitrary flow stats.The existing Flow Stats were renamed > as Flow Description. Thank you for doing all this work. I have some comments. I think that the comment on 'reserved' in struct ofp_oxs_stat is wrong. This field should always be zero, not "One of OFPST_*.". Why does "struct ox_fields" have a plural name? It appear to represent a single field. The meaning of the new 'oxs_fields' and 'flow_desc' members in ofputil_flow_stats_request isn't clear from the comments: - It looks like oxs_fields is used to append some stats to flow stats request messages, but I can't find a description of this ability in the OF1.5.1 spec. I see a mention of stats in *replies* to these messages, but not requests. Can you help me understand? - I guess that 'flow_desc' distinguishes OFPMP_FLOW_DESC from OFPMP_FLOW_STATS. That really isn't clear from the comments. Please edit the top-level comment for this struct to explain the three kinds of messages it can represent and how they are distinguished. Names like "oxs-idle_time" are going to be hard to remember. Do you think that the "oxs-" prefix is really needed? Do you think that we could consistently use either _ or -, not a combination of both? I don't understand this comment. The grammar is a little funny: +/* ofputil_flow_stats structure is used to encode/decode oxs stats + * fields since no stat fields are necessary in request NULL was + * passed */ There is a little funny indentation in ofputil_decode_flow_stats_reply(). Please check it. ox-stat.c seems to use a function name be64toh() on uint64_ts that actually contain big-endian data. Please instead use ovs_be64 and ntohll(). Possibly a helper function or two would help. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
Hi Ben, We have addressed all the review comments and submitted the updated patch. Kindly review it once and provide your inputs for the same. Thanks & Regards Satya Valli Tata Consultancy Services Mailto: satyavalli.r...@tcs.com Website: http://www.tcs.com Experience certainty. IT Services Business Solutions Consulting From: Satyavalli Rama/HYD/TCS To: Ben Pfaff Cc: d...@openvswitch.org, Surya Muttamsetty , SatyaValli , harivelam.lava...@tcs.com Date: 08/04/2017 06:48 PM Subject:Re: [ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support Hi Ben, Much Thanks for the Review. We will address the comments and will submit the updated patch soon. Thanks & Regards Satya Valli Tata Consultancy Services Mailto: satyavalli.r...@tcs.com Website: http://www.tcs.com Experience certainty. IT Services Business Solutions Consulting From: Ben Pfaff To: SatyaValli Cc: d...@openvswitch.org, Surya Muttamsetty , SatyaValli Date: 08/03/2017 01:41 AM Subject: Re: [ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support Thanks for the revision. I have some more comments. On Mon, Jun 19, 2017 at 01:14:40PM +0530, SatyaValli wrote: > commit 49a3592b2878a33033e5fd2e6e5ce82ebccef780 > Author: Satya Valli > Co-authored-by: Lavanya Harivelam > Co-authored-by: Surya Muttamsetty None of the above should be at the top of the commit message. Coauthors go with the sign-offs at the end. > OpenVswitch: OF1.5/EXT-334 Extensible Flow Entry Statistics Please don't repeat the subject again in the body. > OpenFlow 1.5 introduces the Extensible Statistics (OXS) by redefining the existing > flow entry statistics with OXS Fields. > This Patch provides implementation for OXS fields encoding in TLV format. > > To support this implementation below two messages are newly added > > OFPST_OXS_FLOW_STATS_REQUEST > OFPST_OXS_FLOW_STATS_REPLY > OFPST_OXS_AGGREGATE_STATS_REQUEST > OFPST_OXS_AGGREGATE_STATS_REPLY > OFPST_FLOW_REMOVED > As per the openflow specification-1.5, this enhancement should take place > on the existing flow entry statistics with the OXS fields on all the messages > that carries flow entry statistics. > > The current commit adds support for the new feature in flow statistics multipart messages, > aggregate multipart messages and OXS flow statistics support for flow removal message. > > Some more fields are added to ovs-ofctl dump-flows command to support OpenFlow15 OXS stats. > Below are Commands to display OXS stats field wise > > Flow Statistics Multipart > ovs-ofctl dump-flows -O OpenFlow15 oxs-duration > ovs-ofctl dump-flows -O OpenFlow15 oxs-idle_time > ovs-ofctl dump-flows -O OpenFlow15 oxs-packet_count > ovs-ofctl dump-flows -O OpenFlow15 oxs-byte_count > > Aggregate Flow Statistics Multipart > ovs-ofctl dump-aggregate -O OpenFlow15 oxs-packet_count > ovs-ofctl dump-aggregate -O OpenFlow15 oxs-byte_count > ovs-ofctl dump-aggregate -O OpenFlow15 oxs-flow_count > > Make Check Changes in - ofproto.at and ofproto-dpif.at: > For 1.5 version "flags" variable has been removed from flow_stats_reply structure. > For avoiding the make check failures for OF1.5 version add-flow with flags, > below test cases has been modified for 1.5 version in respective .at files > > Files testcase > ofproto.at 0884 > ofproto-dpif1126 Please don't refer to tests by number. The numbers are not meaningful and change often. Use the name of the test. > In ofproto.at there is one test case numbered 0884 and 1126 in ofproto-dpif.at > which is validating flagsfor OF1.5 this testcase is not valid, because > in stats reply flags are not explicitly communicated,I've removed this test case. > Before removing this test case is failing for 1.5 version. > > Since FLOW_REMOVED is not supported in OF1.5 version, > testing has been done by adding test case in ofproto.at. > While doing make check after adding test case we are able to see > OFPT_FLOW_REMOVED (OF1.5) message has sent successfully. > > But make check is failing because of Signal 15 termination, > so we've removed that test case from the patch. Why isn't FLOW_REMOVED supported in OF1.5? Support is needed. Signal 15 is SIGTERM, meaning that something actually used "kill" to kill it intentionally. Please investigate and fix the problem rather than removing a test. This adds features that users can use via ovs-ofctl, but it does not document
[ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
From: SatyaValli
This Patch provides implementation Existing flow entry statistics are
redefined as standard OXS(OpenFlow Extensible Statistics) fields for
displaying the arbitrary flow stats.The existing Flow Stats were renamed
as Flow Description.
To support this implementation below messages are newly added
OFPRAW_OFPT15_FLOW_REMOVED,
OFPRAW_OFPST15_FLOW_REQUEST,
OFPRAW_OFPST15_FLOW_DESC_REQUEST,
OFPRAW_OFPST15_AGGREGATE_REQUEST,
OFPRAW_OFPST15_FLOW_REPLY,
OFPRAW_OFPST15_FLOW_DESC_REPLY,
OFPRAW_OFPST15_AGGREGATE_REPLY,
The current commit adds support for the new feature in flow statistics
multipart messages,aggregate multipart messages and OXS support for flow
removal message, individual flow description messages.
"ovs-ofctl dump-flows" needs to be provided with the arbitrary OXS fields
for displaying the desired flow stats.
Below are Commands to display OXS stats field wise
Flow Statistics Multipart
ovs-ofctl dump-flows -O OpenFlow15 oxs-idle_time
ovs-ofctl dump-flows -O OpenFlow15 oxs-packet_count
ovs-ofctl dump-flows -O OpenFlow15 oxs-byte_count
Aggregate Flow Statistics Multipart
ovs-ofctl dump-aggregate -O OpenFlow15 oxs-packet_count
ovs-ofctl dump-aggregate -O OpenFlow15 oxs-byte_count
Flow Descritption
ovs-ofctl dump-flow-desc -O OpenFlow15 oxs-idle_time
ovs-ofctl dump-flow-desc -O OpenFlow15 oxs-packet_count
ovs-ofctl dump-flow-desc -O OpenFlow15 oxs-byte_count
Signed-off-by: Satya Valli
Co-authored-by: Lavanya Harivelam
Signed-off-by: Lavanya Harivelam
Co-authored-by: Surya Muttamsetty
Signed-off-by: Surya Muttamsetty
Co-authored-by: Manasa Cherukupally
Signed-off-by: Manasa Cherukupally
Co-authored-by: Pavani Panthagada
Signed-off-by: Pavani Panthagada
---
NEWS| 9 +
include/openflow/openflow-1.5.h | 80
include/openvswitch/ofp-msgs.h | 31 +-
include/openvswitch/ofp-parse.h | 6 +-
include/openvswitch/ofp-util.h | 7 +-
lib/automake.mk | 2 +
lib/ofp-parse.c | 71 ++-
lib/ofp-print.c | 2 +
lib/ofp-util.c | 285 +++-
lib/ox-stat.c | 987
lib/ox-stat.h | 53 +++
lib/rconn.c | 2 +
lib/vconn.c | 1 -
ofproto/ofproto.c | 9 +-
tests/ofp-print.at | 127 ++
tests/ofproto-dpif.at | 84
tests/ofproto.at| 5 +
utilities/ovs-ofctl.8.in| 48 +-
utilities/ovs-ofctl.c | 33 +-
19 files changed, 1785 insertions(+), 57 deletions(-)
create mode 100644 lib/ox-stat.c
create mode 100644 lib/ox-stat.h
diff --git a/NEWS b/NEWS
index 6a5d2bf..e76b2fe 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,15 @@ Post-v2.8.0
v2.8.0 - xx xxx
-
- ovs-ofctl:
+ * Existing flow entry statistics are redefined as standard OXS(OpenFlow
+ Extensible Statistics) fields for displaying the arbitrary flow stats.
+ * Now "ovs-ofctl dump-flows" needs to be provided with the arbitrary OXS
+ fields i.e flow duration, flow count, packet count, byte count or all
+ for displaying the desired flow stats.By default with "ovs-ofctl dump-
+ flows" displays only flow duration. See ovs-ofctl(8) for details.
+ * The existing flow statistics are renamed as Flow Description. Now the
+ information about individual flow entries will be displayed with the
+ help of ovs-ofctl dump-flow-desc. See ovs-ofctl(8) for details.
* ovs-ofctl can now accept and display port names in place of numbers. By
default it always accepts names and in interactive use it displays them;
use --names or --no-names to override. See ovs-ofctl(8) for details.
diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
index 73b76d8..0698232 100644
--- a/include/openflow/openflow-1.5.h
+++ b/include/openflow/openflow-1.5.h
@@ -163,4 +163,84 @@ struct ofp15_packet_out {
};
OFP_ASSERT(sizeof(struct ofp15_packet_out) == 8);
+struct ofp_oxs_stat {
+ovs_be16 reserved; /* One of OFPST_*. */
+ovs_be16 length;/* Stats Length */
+};
+
+OFP_ASSERT(sizeof(struct ofp_oxs_stat) == 4);
+
+/* Body for ofp_multipart_request of type
+* OFPMP_FLOW_DESC & OFPMP_FLOW_STATS. */
+struct ofp15_flow_stats_request {
+uint8_t table_id; /* ID of table to read (from ofp_table_desc),
+ * OFPTT_ALL for all tables. */
+uint8_t pad[3]; /* Align to 32 bits. */
+ovs_be32 out_port; /* Require matching entries to include this as
+ * an output port. A value of OFP_ANY
+ * indicates no restriction. */
+ovs_be32 out_group; /* Require matching entries to include this as
+ * an output group. A value of OFPG_ANY
+
Re: [ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
Hi Ben,
Much Thanks for the Review.
We will address the comments and will submit the updated patch soon.
Thanks & Regards
Satya Valli
Tata Consultancy Services
Mailto: satyavalli.r...@tcs.com
Website: http://www.tcs.com
Experience certainty. IT Services
Business Solutions
Consulting
From: Ben Pfaff
To: SatyaValli
Cc: d...@openvswitch.org, Surya Muttamsetty
, SatyaValli
Date: 08/03/2017 01:41 AM
Subject:Re: [ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow
Entry Statistics Support
Thanks for the revision. I have some more comments.
On Mon, Jun 19, 2017 at 01:14:40PM +0530, SatyaValli wrote:
> commit 49a3592b2878a33033e5fd2e6e5ce82ebccef780
> Author: Satya Valli
> Co-authored-by: Lavanya Harivelam
> Co-authored-by: Surya Muttamsetty
None of the above should be at the top of the commit message. Coauthors
go with the sign-offs at the end.
> OpenVswitch: OF1.5/EXT-334 Extensible Flow Entry Statistics
Please don't repeat the subject again in the body.
> OpenFlow 1.5 introduces the Extensible Statistics (OXS) by redefining
the existing
> flow entry statistics with OXS Fields.
> This Patch provides implementation for OXS fields encoding in TLV
format.
>
> To support this implementation below two messages are newly added
>
> OFPST_OXS_FLOW_STATS_REQUEST
> OFPST_OXS_FLOW_STATS_REPLY
> OFPST_OXS_AGGREGATE_STATS_REQUEST
> OFPST_OXS_AGGREGATE_STATS_REPLY
> OFPST_FLOW_REMOVED
> As per the openflow specification-1.5, this enhancement should take
place
> on the existing flow entry statistics with the OXS fields on all the
messages
> that carries flow entry statistics.
>
> The current commit adds support for the new feature in flow statistics
multipart messages,
> aggregate multipart messages and OXS flow statistics support for flow
removal message.
>
> Some more fields are added to ovs-ofctl dump-flows command to support
OpenFlow15 OXS stats.
> Below are Commands to display OXS stats field wise
>
> Flow Statistics Multipart
> ovs-ofctl dump-flows -O OpenFlow15 oxs-duration
> ovs-ofctl dump-flows -O OpenFlow15 oxs-idle_time
> ovs-ofctl dump-flows -O OpenFlow15 oxs-packet_count
> ovs-ofctl dump-flows -O OpenFlow15 oxs-byte_count
>
> Aggregate Flow Statistics Multipart
> ovs-ofctl dump-aggregate -O OpenFlow15 oxs-packet_count
> ovs-ofctl dump-aggregate -O OpenFlow15 oxs-byte_count
> ovs-ofctl dump-aggregate -O OpenFlow15 oxs-flow_count
>
> Make Check Changes in - ofproto.at and ofproto-dpif.at:
> For 1.5 version "flags" variable has been removed from flow_stats_reply
structure.
> For avoiding the make check failures for OF1.5 version add-flow with
flags,
> below test cases has been modified for 1.5 version in respective .at
files
>
> Files testcase
> ofproto.at 0884
> ofproto-dpif1126
Please don't refer to tests by number. The numbers are not meaningful
and change often. Use the name of the test.
> In ofproto.at there is one test case numbered 0884 and 1126 in
ofproto-dpif.at
> which is validating flagsfor OF1.5 this testcase is not valid, because
> in stats reply flags are not explicitly communicated,I've removed this
test case.
> Before removing this test case is failing for 1.5 version.
>
> Since FLOW_REMOVED is not supported in OF1.5 version,
> testing has been done by adding test case in ofproto.at.
> While doing make check after adding test case we are able to see
> OFPT_FLOW_REMOVED (OF1.5) message has sent successfully.
>
> But make check is failing because of Signal 15 termination,
> so we've removed that test case from the patch.
Why isn't FLOW_REMOVED supported in OF1.5? Support is needed. Signal
15 is SIGTERM, meaning that something actually used "kill" to kill it
intentionally. Please investigate and fix the problem rather than
removing a test.
This adds features that users can use via ovs-ofctl, but it does not
document them. Please document them in ovs-ofctl(8). Please also add
appropriate NEWS items to explain the new features.
This adds support for new OpenFlow messages, but it does not add any
tests for printing them in ofp-print.at. Please add at least one
representative test there for every new message.
struct ofp15_flow_removed doesn't seem to be the same as
ofp_flow_removed in OpenFlow 1.5.
This adds a global variable oxs_field_set. This is inappropriate. Do
not use global variables.
The variable oxs_field_set has a bunch of unnamed magic numbers. This
is inappropriate. Do not use unnamed magic numbers.
This code is weird. Fix it please:
+if (parse_oxs_field(name, &f)) {
+if (f->fl_t
Re: [ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
Thanks for the revision. I have some more comments.
On Mon, Jun 19, 2017 at 01:14:40PM +0530, SatyaValli wrote:
> commit 49a3592b2878a33033e5fd2e6e5ce82ebccef780
> Author: Satya Valli
> Co-authored-by: Lavanya Harivelam
> Co-authored-by: Surya Muttamsetty
None of the above should be at the top of the commit message. Coauthors
go with the sign-offs at the end.
> OpenVswitch: OF1.5/EXT-334 Extensible Flow Entry Statistics
Please don't repeat the subject again in the body.
> OpenFlow 1.5 introduces the Extensible Statistics (OXS) by redefining the
> existing
> flow entry statistics with OXS Fields.
> This Patch provides implementation for OXS fields encoding in TLV format.
>
> To support this implementation below two messages are newly added
>
> OFPST_OXS_FLOW_STATS_REQUEST
> OFPST_OXS_FLOW_STATS_REPLY
> OFPST_OXS_AGGREGATE_STATS_REQUEST
> OFPST_OXS_AGGREGATE_STATS_REPLY
> OFPST_FLOW_REMOVED
> As per the openflow specification-1.5, this enhancement should take place
> on the existing flow entry statistics with the OXS fields on all the messages
> that carries flow entry statistics.
>
> The current commit adds support for the new feature in flow statistics
> multipart messages,
> aggregate multipart messages and OXS flow statistics support for flow removal
> message.
>
> Some more fields are added to ovs-ofctl dump-flows command to support
> OpenFlow15 OXS stats.
> Below are Commands to display OXS stats field wise
>
> Flow Statistics Multipart
> ovs-ofctl dump-flows -O OpenFlow15 oxs-duration
> ovs-ofctl dump-flows -O OpenFlow15 oxs-idle_time
> ovs-ofctl dump-flows -O OpenFlow15 oxs-packet_count
> ovs-ofctl dump-flows -O OpenFlow15 oxs-byte_count
>
> Aggregate Flow Statistics Multipart
> ovs-ofctl dump-aggregate -O OpenFlow15 oxs-packet_count
> ovs-ofctl dump-aggregate -O OpenFlow15 oxs-byte_count
> ovs-ofctl dump-aggregate -O OpenFlow15 oxs-flow_count
>
> Make Check Changes in - ofproto.at and ofproto-dpif.at:
> For 1.5 version "flags" variable has been removed from flow_stats_reply
> structure.
> For avoiding the make check failures for OF1.5 version add-flow with flags,
> below test cases has been modified for 1.5 version in respective .at files
>
> Files testcase
> ofproto.at 0884
> ofproto-dpif1126
Please don't refer to tests by number. The numbers are not meaningful
and change often. Use the name of the test.
> In ofproto.at there is one test case numbered 0884 and 1126 in ofproto-dpif.at
> which is validating flagsfor OF1.5 this testcase is not valid, because
> in stats reply flags are not explicitly communicated,I've removed this test
> case.
> Before removing this test case is failing for 1.5 version.
>
> Since FLOW_REMOVED is not supported in OF1.5 version,
> testing has been done by adding test case in ofproto.at.
> While doing make check after adding test case we are able to see
> OFPT_FLOW_REMOVED (OF1.5) message has sent successfully.
>
> But make check is failing because of Signal 15 termination,
> so we've removed that test case from the patch.
Why isn't FLOW_REMOVED supported in OF1.5? Support is needed. Signal
15 is SIGTERM, meaning that something actually used "kill" to kill it
intentionally. Please investigate and fix the problem rather than
removing a test.
This adds features that users can use via ovs-ofctl, but it does not
document them. Please document them in ovs-ofctl(8). Please also add
appropriate NEWS items to explain the new features.
This adds support for new OpenFlow messages, but it does not add any
tests for printing them in ofp-print.at. Please add at least one
representative test there for every new message.
struct ofp15_flow_removed doesn't seem to be the same as
ofp_flow_removed in OpenFlow 1.5.
This adds a global variable oxs_field_set. This is inappropriate. Do
not use global variables.
The variable oxs_field_set has a bunch of unnamed magic numbers. This
is inappropriate. Do not use unnamed magic numbers.
This code is weird. Fix it please:
+if (parse_oxs_field(name, &f)) {
+if (f->fl_type == OFPXST_OFB_DURATION) {
+oxs_field_set |= (1 << f->fl_type);
+} else if (f->fl_type == OFPXST_OFB_IDLE_TIME) {
+oxs_field_set |= (1 << f->fl_type);
+} else if (f->fl_type == OFPXST_OFB_FLOW_COUNT) {
+oxs_field_set |= (1 << f->fl_type);
+} else if (f->fl_type == OFPXST_OFB_PACKET_COUNT) {
+oxs_field_set |= (1 << f->fl_type);
+} else if (f->fl_type == OFPXST_OFB_BYTE_COUNT) {
+oxs_field_set |= (1 << f->fl_type);
+}
This patch seems not to recognize that OF1.5+ has two different ways to
request and get replies for flow stats: FLOW_DESC and FLOW_STATS. It
looks like it only supports the latter. We must support both.
Thanks,
Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailma
Re: [ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
On Mon, Jun 19, 2017 at 01:26:54PM +0530, Satyavalli Rama wrote: > Hi Ben, > > Much Thanks for your initial reveiw comments. > We have addressed almost all problems and re-submitted the patch except the > below two lines. > > "checkpatch" reports: > > warning: 1 line adds whitespace errors. > WARNING: Line length is >79-characters long > #137 FILE: include/openvswitch/ofp-msgs.h:656: > OFPTYPE_OXS_AGGREGATE_STATS_REQUEST, /* > OFPRAW_OFPST15_OXS_AGGREGATE_REQUEST. */ > > WARNING: Line length is >79-characters long > #138 FILE: include/openvswitch/ofp-msgs.h:657: > OFPTYPE_OXS_AGGREGATE_STATS_REPLY, /* > OFPRAW_OFPST15_OXS_AGGREGATE_REPLY. */ > > We tried to address the above two lines also, but we are getting "unexpected > syntax within OFPTYPE_ definition" error as below, if we are trying to adjust > them. OK. You can ignore those reports from checkpatch. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
Hi Ben,
Much Thanks for your initial reveiw comments.
We have addressed almost all problems and re-submitted the patch except the
below two lines.
"checkpatch" reports:
warning: 1 line adds whitespace errors.
WARNING: Line length is >79-characters long
#137 FILE: include/openvswitch/ofp-msgs.h:656:
OFPTYPE_OXS_AGGREGATE_STATS_REQUEST, /*
OFPRAW_OFPST15_OXS_AGGREGATE_REQUEST. */
WARNING: Line length is >79-characters long
#138 FILE: include/openvswitch/ofp-msgs.h:657:
OFPTYPE_OXS_AGGREGATE_STATS_REPLY, /*
OFPRAW_OFPST15_OXS_AGGREGATE_REPLY. */
We tried to address the above two lines also, but we are getting "unexpected
syntax within OFPTYPE_ definition" error as below, if we are trying to adjust
them.
PYTHONPATH=./python":"$PYTHONPATH PYTHONDONTWRITEBYTECODE=yes /usr/bin/python
./build-aux/extract-ofp-msgs \
./include/openvswitch/ofp-msgs.h lib/ofp-msgs.inc >
lib/ofp-msgs.inc.tmp && mv lib/ofp-msgs.inc.tmp lib/ofp-msgs.inc
./include/openvswitch/ofp-msgs.h:659: unexpected syntax within OFPTYPE_
definition
make[2]: *** [lib/ofp-msgs.inc] Error 1
make[2]: Leaving directory `/home/tcs/freshgitjune15/ovsfinalpatch/ovs'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/tcs/freshgitjune15/ovsfinalpatch/ovs'
make: *** [all] Error 2
Thanks & Regards
Satya Valli
Tata Consultancy Services
Mailto: satyavalli.r...@tcs.com
Website: http://www.tcs.com
Experience certainty. IT Services
Business Solutions
Consulting
-Ben Pfaff wrote: -
To: SatyaValli
From: Ben Pfaff
Date: 06/15/2017 02:38AM
Cc: d...@openvswitch.org, SatyaValli
Subject: Re: [ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry
Statistics Support
On Fri, Jun 09, 2017 at 01:26:51PM +0530, SatyaValli wrote:
> commit 49a3592b2878a33033e5fd2e6e5ce82ebccef780
> Author: Satya Valli
>
> OpenVswitch: OF1.5/EXT-334 Extensible Flow Entry Statistics
>
> OpenFlow 1.5 introduces the Extensible Statistics (OXS) by redefining the
> existing
> flow entry statistics with OXS Fields.
> This Patch provides implementation for OXS fields encoding in TLV format.
>
> To support this implementation below two messages are newly added
>
> OFPST_OXS_FLOW_STATS_REQUEST
> OFPST_OXS_FLOW_STATS_REPLY
> OFPST_OXS_AGGREGATE_STATS_REQUEST
> OFPST_OXS_AGGREGATE_STATS_REPLY
> OFPST_FLOW_REMOVED
>
> As per the openflow specification-1.5, this enhancement should take place
> on the existing flow entry statistics with the OXS fields on all the messages
> that carries flow entry statistics.
>
> The current commit adds support for the new feature in flow statistics
> multipart messages,
> aggregate multipart messages and OXS flow statistics support for flow removal
> message.
>
> Some more fields are added to ovs-ofctl dump-flows command to support
> OpenFlow15 OXS stats.
> Below are Commands to display OXS stats field wise
>
> Flow Statistics Multipart
> ovs-ofctl dump-flows -O OpenFlow15 oxs-duration
> ovs-ofctl dump-flows -O OpenFlow15 oxs-idle_time
> ovs-ofctl dump-flows -O OpenFlow15 oxs-packet_count
> ovs-ofctl dump-flows -O OpenFlow15 oxs-byte_count
>
> Aggregate Flow Statistics Multipart
> ovs-ofctl dump-aggregate -O OpenFlow15 oxs-packet_count
> ovs-ofctl dump-aggregate -O OpenFlow15 oxs-byte_count
> ovs-ofctl dump-aggregate -O OpenFlow15 oxs-flow_count
>
> Signed-off-by: Satya Valli
> Co-authored-by: Lavanya Harivelam and Surya
> Muttamsetty
Thank you for working on Open vSwitch.
"Co-authored-by" takes only one name. Please use multiple
"Co-authored-by" tags if you have more than one coauthor.
"checkpatch" reports:
warning: 1 line adds whitespace errors.
WARNING: Line length is >79-characters long
#137 FILE: include/openvswitch/ofp-msgs.h:656:
OFPTYPE_OXS_AGGREGATE_STATS_REQUEST, /*
OFPRAW_OFPST15_OXS_AGGREGATE_REQUEST. */
WARNING: Line length is >79-characters long
#138 FILE: include/openvswitch/ofp-msgs.h:657:
OFPTYPE_OXS_AGGREGATE_STATS_REPLY, /*
OFPRAW_OFPST15_OXS_AGGREGATE_REPLY. */
ERROR: Inappropriate bracing around statement
#483 FILE: lib/ofp-util.c:3331:
if (error)
ERROR: Improper whitespace around control block
#1114 FILE: lib/ox-stat.c:534:
HMAP_FOR_EACH_IN_BUCKET(oxfs, header_node, hash_int(header_no_len, 0),
ERROR: Improper whitespace around control block
#1135 FILE: lib/ox-stat.c:555:
LIST_FOR_EACH(oxfs, ox_node, &oxs_ox_map[id]) {
ERROR: Improper whitespace around control block
#1298 FILE: lib/ox-stat.c:718:
if(fs) {
ERROR: Improper whitespace around control block
#1308 FILE: lib/ox-stat
[ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
commit 49a3592b2878a33033e5fd2e6e5ce82ebccef780
Author: Satya Valli
Co-authored-by: Lavanya Harivelam
Co-authored-by: Surya Muttamsetty
OpenVswitch: OF1.5/EXT-334 Extensible Flow Entry Statistics
OpenFlow 1.5 introduces the Extensible Statistics (OXS) by redefining the
existing
flow entry statistics with OXS Fields.
This Patch provides implementation for OXS fields encoding in TLV format.
To support this implementation below two messages are newly added
OFPST_OXS_FLOW_STATS_REQUEST
OFPST_OXS_FLOW_STATS_REPLY
OFPST_OXS_AGGREGATE_STATS_REQUEST
OFPST_OXS_AGGREGATE_STATS_REPLY
OFPST_FLOW_REMOVED
As per the openflow specification-1.5, this enhancement should take place
on the existing flow entry statistics with the OXS fields on all the messages
that carries flow entry statistics.
The current commit adds support for the new feature in flow statistics
multipart messages,
aggregate multipart messages and OXS flow statistics support for flow removal
message.
Some more fields are added to ovs-ofctl dump-flows command to support
OpenFlow15 OXS stats.
Below are Commands to display OXS stats field wise
Flow Statistics Multipart
ovs-ofctl dump-flows -O OpenFlow15 oxs-duration
ovs-ofctl dump-flows -O OpenFlow15 oxs-idle_time
ovs-ofctl dump-flows -O OpenFlow15 oxs-packet_count
ovs-ofctl dump-flows -O OpenFlow15 oxs-byte_count
Aggregate Flow Statistics Multipart
ovs-ofctl dump-aggregate -O OpenFlow15 oxs-packet_count
ovs-ofctl dump-aggregate -O OpenFlow15 oxs-byte_count
ovs-ofctl dump-aggregate -O OpenFlow15 oxs-flow_count
Make Check Changes in - ofproto.at and ofproto-dpif.at:
For 1.5 version "flags" variable has been removed from flow_stats_reply
structure.
For avoiding the make check failures for OF1.5 version add-flow with flags,
below test cases has been modified for 1.5 version in respective .at files
Files testcase
ofproto.at 0884
ofproto-dpif1126
In ofproto.at there is one test case numbered 0884 and 1126 in ofproto-dpif.at
which is validating flagsfor OF1.5 this testcase is not valid, because
in stats reply flags are not explicitly communicated,I've removed this test
case.
Before removing this test case is failing for 1.5 version.
Since FLOW_REMOVED is not supported in OF1.5 version,
testing has been done by adding test case in ofproto.at.
While doing make check after adding test case we are able to see
OFPT_FLOW_REMOVED (OF1.5) message has sent successfully.
But make check is failing because of Signal 15 termination,
so we've removed that test case from the patch.
Signed-off-by: Satya Valli
Signed-off-by: Lavanya Harivelam
Signed-off-by: Surya Muttamsetty
---
include/openflow/openflow-1.5.h | 61 +++
include/openvswitch/ofp-msgs.h | 15 +
lib/automake.mk | 2 +
lib/ofp-parse.c | 44 +-
lib/ofp-print.c | 16 +
lib/ofp-util.c | 194 +++-
lib/ox-stat.c | 980
lib/ox-stat.h | 44 ++
lib/rconn.c | 4 +
lib/vconn.c | 3 +-
ofproto/ofproto.c | 8 +
tests/ofproto-dpif.at | 1 -
tests/ofproto.at| 4 -
13 files changed, 1360 insertions(+), 16 deletions(-)
create mode 100644 lib/ox-stat.c
create mode 100644 lib/ox-stat.h
diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
index 73b76d8..a59eda9 100644
--- a/include/openflow/openflow-1.5.h
+++ b/include/openflow/openflow-1.5.h
@@ -163,4 +163,65 @@ struct ofp15_packet_out {
};
OFP_ASSERT(sizeof(struct ofp15_packet_out) == 8);
+struct ofp_oxs_stat {
+ovs_be16 reserved; /* One of OFPST_* */
+ovs_be16 length;/* Stats Length */
+};
+
+OFP_ASSERT(sizeof(struct ofp_oxs_stat) == 4);
+
+/* Body for ofp_multipart_request of type
+* OFPMP_FLOW_DESC & OFPMP_FLOW_STATS. */
+struct ofp15_oxs_flow_stats_request {
+uint8_t table_id; /* ID of table to read (from ofp_table_desc),
+ * OFPTT_ALL for all tables. */
+uint8_t pad[3]; /* Align to 32 bits. */
+ovs_be32 out_port; /* Require matching entries to include this as
+ * an output port. A value of OFP_ANY
+ * indicates no restriction. */
+ovs_be32 out_group; /* Require matching entries to include this as
+ * an output group. A value of OFPG_ANY
+ * indicates no restriction. */
+uint8_t pad2[4];/* Align to 64 bits. */
+ovs_be64 cookie;/* Require matching entries to contain this
+ * cookie value */
+ovs_be64 cookie_mask; /* Mask used to restrict the cookie bits that
+ * must match. A value of 0 indicates no
+ * restriction. */
+};
+
+OFP_A
Re: [ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
On Fri, Jun 09, 2017 at 01:26:51PM +0530, SatyaValli wrote:
> commit 49a3592b2878a33033e5fd2e6e5ce82ebccef780
> Author: Satya Valli
>
> OpenVswitch: OF1.5/EXT-334 Extensible Flow Entry Statistics
>
> OpenFlow 1.5 introduces the Extensible Statistics (OXS) by redefining the
> existing
> flow entry statistics with OXS Fields.
> This Patch provides implementation for OXS fields encoding in TLV format.
>
> To support this implementation below two messages are newly added
>
> OFPST_OXS_FLOW_STATS_REQUEST
> OFPST_OXS_FLOW_STATS_REPLY
> OFPST_OXS_AGGREGATE_STATS_REQUEST
> OFPST_OXS_AGGREGATE_STATS_REPLY
> OFPST_FLOW_REMOVED
>
> As per the openflow specification-1.5, this enhancement should take place
> on the existing flow entry statistics with the OXS fields on all the messages
> that carries flow entry statistics.
>
> The current commit adds support for the new feature in flow statistics
> multipart messages,
> aggregate multipart messages and OXS flow statistics support for flow removal
> message.
>
> Some more fields are added to ovs-ofctl dump-flows command to support
> OpenFlow15 OXS stats.
> Below are Commands to display OXS stats field wise
>
> Flow Statistics Multipart
> ovs-ofctl dump-flows -O OpenFlow15 oxs-duration
> ovs-ofctl dump-flows -O OpenFlow15 oxs-idle_time
> ovs-ofctl dump-flows -O OpenFlow15 oxs-packet_count
> ovs-ofctl dump-flows -O OpenFlow15 oxs-byte_count
>
> Aggregate Flow Statistics Multipart
> ovs-ofctl dump-aggregate -O OpenFlow15 oxs-packet_count
> ovs-ofctl dump-aggregate -O OpenFlow15 oxs-byte_count
> ovs-ofctl dump-aggregate -O OpenFlow15 oxs-flow_count
>
> Signed-off-by: Satya Valli
> Co-authored-by: Lavanya Harivelam and Surya
> Muttamsetty
Thank you for working on Open vSwitch.
"Co-authored-by" takes only one name. Please use multiple
"Co-authored-by" tags if you have more than one coauthor.
"checkpatch" reports:
warning: 1 line adds whitespace errors.
WARNING: Line length is >79-characters long
#137 FILE: include/openvswitch/ofp-msgs.h:656:
OFPTYPE_OXS_AGGREGATE_STATS_REQUEST, /*
OFPRAW_OFPST15_OXS_AGGREGATE_REQUEST. */
WARNING: Line length is >79-characters long
#138 FILE: include/openvswitch/ofp-msgs.h:657:
OFPTYPE_OXS_AGGREGATE_STATS_REPLY, /*
OFPRAW_OFPST15_OXS_AGGREGATE_REPLY. */
ERROR: Inappropriate bracing around statement
#483 FILE: lib/ofp-util.c:3331:
if (error)
ERROR: Improper whitespace around control block
#1114 FILE: lib/ox-stat.c:534:
HMAP_FOR_EACH_IN_BUCKET(oxfs, header_node, hash_int(header_no_len, 0),
ERROR: Improper whitespace around control block
#1135 FILE: lib/ox-stat.c:555:
LIST_FOR_EACH(oxfs, ox_node, &oxs_ox_map[id]) {
ERROR: Improper whitespace around control block
#1298 FILE: lib/ox-stat.c:718:
if(fs) {
ERROR: Improper whitespace around control block
#1308 FILE: lib/ox-stat.c:728:
if(fs) {
ERROR: Improper whitespace around control block
#1318 FILE: lib/ox-stat.c:738:
if(fs) {
Clang reports:
../lib/ofp-util.c:3466:42: error: implicit conversion from enumeration type
'enum ofputil_protocol' to different enumeration type 'enum ofp_version'
[-Werror,-Wenum-conversion]
"sparse" reports:
../lib/ox-stat.c:282:45: warning: constant 0x is so big it
is unsigned long long
../lib/ox-stat.c:291:42: warning: constant 0x is so big it
is unsigned long long
../lib/ox-stat.c:127:9: warning: symbol 'oxs_field_set' was not declared.
Should it be static?
../lib/ox-stat.c:280:31: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:280:31:expected restricted ovs_be64 [usertype]
../lib/ox-stat.c:280:31:got unsigned long long [unsigned] [addressable]
[usertype] duration
../lib/ox-stat.c:290:32: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:290:32:expected restricted ovs_be64 [usertype]
../lib/ox-stat.c:290:32:got unsigned long long [unsigned] [addressable]
[usertype] idle_time
../lib/ox-stat.c:298:39: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:298:39:expected restricted ovs_be64 [usertype]
../lib/ox-stat.c:298:39:got unsigned long long [unsigned] [addressable]
[usertype] packet_count
../lib/ox-stat.c:305:37: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:305:37:expected restricted ovs_be64 [usertype]
../lib/ox-stat.c:305:37:got unsigned long long [unsigned] [addressable]
[usertype] byte_count
../lib/ox-stat.c:443:40: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:443:40:expected restricted ovs_be32 [usertype] x
../lib/ox-stat.c:443:40:got unsigned int [unsigned] [addressable]
[usertype] flow_count
../lib/ox-stat.c:450:43: warning: incorrect type in argument 1 (different
[ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
commit 49a3592b2878a33033e5fd2e6e5ce82ebccef780
Author: Satya Valli
OpenVswitch: OF1.5/EXT-334 Extensible Flow Entry Statistics
OpenFlow 1.5 introduces the Extensible Statistics (OXS) by redefining the
existing
flow entry statistics with OXS Fields.
This Patch provides implementation for OXS fields encoding in TLV format.
To support this implementation below two messages are newly added
OFPST_OXS_FLOW_STATS_REQUEST
OFPST_OXS_FLOW_STATS_REPLY
OFPST_OXS_AGGREGATE_STATS_REQUEST
OFPST_OXS_AGGREGATE_STATS_REPLY
OFPST_FLOW_REMOVED
As per the openflow specification-1.5, this enhancement should take place
on the existing flow entry statistics with the OXS fields on all the messages
that carries flow entry statistics.
The current commit adds support for the new feature in flow statistics
multipart messages,
aggregate multipart messages and OXS flow statistics support for flow removal
message.
Some more fields are added to ovs-ofctl dump-flows command to support
OpenFlow15 OXS stats.
Below are Commands to display OXS stats field wise
Flow Statistics Multipart
ovs-ofctl dump-flows -O OpenFlow15 oxs-duration
ovs-ofctl dump-flows -O OpenFlow15 oxs-idle_time
ovs-ofctl dump-flows -O OpenFlow15 oxs-packet_count
ovs-ofctl dump-flows -O OpenFlow15 oxs-byte_count
Aggregate Flow Statistics Multipart
ovs-ofctl dump-aggregate -O OpenFlow15 oxs-packet_count
ovs-ofctl dump-aggregate -O OpenFlow15 oxs-byte_count
ovs-ofctl dump-aggregate -O OpenFlow15 oxs-flow_count
Signed-off-by: Satya Valli
Co-authored-by: Lavanya Harivelam and Surya
Muttamsetty
---
include/openflow/openflow-1.5.h | 61 +++
include/openvswitch/ofp-msgs.h | 15 +
lib/automake.mk | 2 +
lib/ofp-parse.c | 45 +-
lib/ofp-print.c | 16 +
lib/ofp-util.c | 192 +++-
lib/ox-stat.c | 982
lib/ox-stat.h | 43 ++
lib/rconn.c | 4 +
lib/vconn.c | 3 +-
ofproto/ofproto.c | 8 +
tests/ofproto-dpif.at | 1 -
tests/ofproto.at| 4 -
13 files changed, 1360 insertions(+), 16 deletions(-)
create mode 100644 lib/ox-stat.c
create mode 100644 lib/ox-stat.h
diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
index 73b76d8..a59eda9 100644
--- a/include/openflow/openflow-1.5.h
+++ b/include/openflow/openflow-1.5.h
@@ -163,4 +163,65 @@ struct ofp15_packet_out {
};
OFP_ASSERT(sizeof(struct ofp15_packet_out) == 8);
+struct ofp_oxs_stat {
+ovs_be16 reserved; /* One of OFPST_* */
+ovs_be16 length;/* Stats Length */
+};
+
+OFP_ASSERT(sizeof(struct ofp_oxs_stat) == 4);
+
+/* Body for ofp_multipart_request of type
+* OFPMP_FLOW_DESC & OFPMP_FLOW_STATS. */
+struct ofp15_oxs_flow_stats_request {
+uint8_t table_id; /* ID of table to read (from ofp_table_desc),
+ * OFPTT_ALL for all tables. */
+uint8_t pad[3]; /* Align to 32 bits. */
+ovs_be32 out_port; /* Require matching entries to include this as
+ * an output port. A value of OFP_ANY
+ * indicates no restriction. */
+ovs_be32 out_group; /* Require matching entries to include this as
+ * an output group. A value of OFPG_ANY
+ * indicates no restriction. */
+uint8_t pad2[4];/* Align to 64 bits. */
+ovs_be64 cookie;/* Require matching entries to contain this
+ * cookie value */
+ovs_be64 cookie_mask; /* Mask used to restrict the cookie bits that
+ * must match. A value of 0 indicates no
+ * restriction. */
+};
+
+OFP_ASSERT(sizeof(struct ofp15_oxs_flow_stats_request) == 32);
+
+/* Body of reply to OFPMP_FLOW_STATS request
+* and body for OFPIT_STAT_TRIGGER generated status. */
+struct ofp15_oxs_flow_stats_reply {
+ovs_be16 length;/* Length of this entry. */
+uint8_t pad2[2];/* Align to 64-bits. */
+uint8_t table_id; /* ID of table flow came from. */
+uint8_t reason; /* One of OFPFSR_*. */
+ovs_be16 priority; /* Priority of the entry. */
+};
+
+OFP_ASSERT(sizeof(struct ofp15_oxs_flow_stats_reply) == 8);
+
+/* OXS flow stat field types for OpenFlow basic class. */
+enum oxs_ofb_stat_fields {
+OFPXST_OFB_DURATION = 0, /* Time flow entry has been alive. */
+OFPXST_OFB_IDLE_TIME = 1,/* Time flow entry has been idle. */
+OFPXST_OFB_FLOW_COUNT = 2, /* Number of aggregated flow entries. */
+OFPXST_OFB_PACKET_COUNT = 3, /* Number of packets in flow entry. */
+OFPXST_OFB_BYTE_COUNT = 4, /* Number of bytes in flow entry. */
+};
+
+/* Flow removed (datapath -> controller).
