Re: [ovs-dev] [PATCH 1/2] OF1.5/EXT-334 OXS/Individal Flow Statistics

2017-05-12 Thread Satyavalli Rama 
Surely Ben.

Aaron thanks for the initial feedback will definitely address them.

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 <b...@ovn.org> wrote: -
To: Satyavalli Rama <satyavalli.r...@tcs.com>
From: Ben Pfaff <b...@ovn.org>
Date: 05/12/2017 07:28PM
Cc: d...@openvswitch.org, Aaron Conole <acon...@redhat.com>
Subject: Re: [ovs-dev] [PATCH 1/2] OF1.5/EXT-334 OXS/Individal Flow Statistics

Even if you are sending fresh patches, I hope you will consider Aaron's 
comments.

On May 12, 2017 9:56 AM, Satyavalli Rama <satyavalli.r...@tcs.com> wrote:
Hi Aaron

Please ignore the current patch.  
Due to some proxy issues we were unable to send the complete patches.   
Soon, we wil submitt fresh patches.

Thanks & Regards
Satya Valli
Tata Consultancy Services
Mailto: satyavalli.r...@tcs.com
Website: http://www.tcs.com

Experience certainty.   IT Services
Business Solutions
Consulting



-Aaron Conole <acon...@redhat.com> wrote: -
To: Ben Pfaff <b...@ovn.org>, Satya Valli <satyavalli.r...@tcs.com>
From: Aaron Conole <acon...@redhat.com>
Date: 05/11/2017 11:23PM
Cc: d...@openvswitch.org,
Subject: Re: [ovs-dev] [PATCH 1/2] OF1.5/EXT-334 OXS/Individal Flow Statistics

Hi Satya,

I haven't checked the OF1.5 spec with this, yet.  Just saw a few things.

Thanks for the contribution!

Ben Pfaff <b...@ovn.org> writes:

> From: Satya Valli <satyavalli.r...@tcs.com>
>

Missing signed-off-by line.  Also, it would be good to describe exactly
which flow stat types are being provided.

> ---
>  include/openflow/openflow-1.5.h | 48 
> +
>  include/openvswitch/ofp-msgs.h  |  6 ++
>  lib/automake.mk |  2 ++
>  lib/ofp-parse.c | 45 +-
>  lib/ofp-print.c | 10 +
>  lib/rconn.c |  2 ++
>  ofproto/ofproto.c   |  4 
>  7 files changed, 116 insertions(+), 1 deletion(-)
>
> diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
> index 3649e6c29e63..ff5fa13fae4a 100644
> --- a/include/openflow/openflow-1.5.h
> +++ b/include/openflow/openflow-1.5.h
> @@ -150,4 +150,52 @@ struct ofp15_group_desc_stats {
>  };
>  OFP_ASSERT(sizeof(struct ofp15_group_desc_stats) == 16);
>  
> +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. */

Minor nit - please put a space at the beginning of the comment line
here.

> +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

Re: [ovs-dev] [PATCH 1/2] OF1.5/EXT-334 OXS/Individal Flow Statistics

2017-05-12 Thread Satyavalli Rama 
Hi Aaron

Please ignore the current patch.  
Due to some proxy issues we were unable to send the complete patches.   
Soon, we wil submitt fresh patches.

Thanks & Regards
Satya Valli
Tata Consultancy Services
Mailto: satyavalli.r...@tcs.com
Website: http://www.tcs.com

Experience certainty.   IT Services
Business Solutions
Consulting



-Aaron Conole <acon...@redhat.com> wrote: -
To: Ben Pfaff <b...@ovn.org>, Satya Valli <satyavalli.r...@tcs.com>
From: Aaron Conole <acon...@redhat.com>
Date: 05/11/2017 11:23PM
Cc: d...@openvswitch.org,
Subject: Re: [ovs-dev] [PATCH 1/2] OF1.5/EXT-334 OXS/Individal Flow Statistics

Hi Satya,

I haven't checked the OF1.5 spec with this, yet.  Just saw a few things.

Thanks for the contribution!

Ben Pfaff <b...@ovn.org> writes:

> From: Satya Valli <satyavalli.r...@tcs.com>
>

Missing signed-off-by line.  Also, it would be good to describe exactly
which flow stat types are being provided.

> ---
>  include/openflow/openflow-1.5.h | 48 
> +
>  include/openvswitch/ofp-msgs.h  |  6 ++
>  lib/automake.mk |  2 ++
>  lib/ofp-parse.c | 45 +-
>  lib/ofp-print.c | 10 +
>  lib/rconn.c |  2 ++
>  ofproto/ofproto.c   |  4 
>  7 files changed, 116 insertions(+), 1 deletion(-)
>
> diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
> index 3649e6c29e63..ff5fa13fae4a 100644
> --- a/include/openflow/openflow-1.5.h
> +++ b/include/openflow/openflow-1.5.h
> @@ -150,4 +150,52 @@ struct ofp15_group_desc_stats {
>  };
>  OFP_ASSERT(sizeof(struct ofp15_group_desc_stats) == 16);
>  
> +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. */

Minor nit - please put a space at the beginning of the comment line
here.

> +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. */
> +};
> +
>  #endif /* openflow/openflow-1.5.h */
> diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
> index 34708f3bd846..49b0f7a2cfa8 100644
> --- a/include/openvswitc

Re: [ovs-dev] [PATCH 1/2] OF1.5/EXT-334 OXS/Individal Flow Statistics

2017-05-11 Thread Aaron Conole 
Hi Satya,

I haven't checked the OF1.5 spec with this, yet.  Just saw a few things.

Thanks for the contribution!

Ben Pfaff  writes:

> From: Satya Valli 
>

Missing signed-off-by line.  Also, it would be good to describe exactly
which flow stat types are being provided.

> ---
>  include/openflow/openflow-1.5.h | 48 
> +
>  include/openvswitch/ofp-msgs.h  |  6 ++
>  lib/automake.mk |  2 ++
>  lib/ofp-parse.c | 45 +-
>  lib/ofp-print.c | 10 +
>  lib/rconn.c |  2 ++
>  ofproto/ofproto.c   |  4 
>  7 files changed, 116 insertions(+), 1 deletion(-)
>
> diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
> index 3649e6c29e63..ff5fa13fae4a 100644
> --- a/include/openflow/openflow-1.5.h
> +++ b/include/openflow/openflow-1.5.h
> @@ -150,4 +150,52 @@ struct ofp15_group_desc_stats {
>  };
>  OFP_ASSERT(sizeof(struct ofp15_group_desc_stats) == 16);
>  
> +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. */

Minor nit - please put a space at the beginning of the comment line
here.

> +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. */
> +};
> +
>  #endif /* openflow/openflow-1.5.h */
> diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
> index 34708f3bd846..49b0f7a2cfa8 100644
> --- a/include/openvswitch/ofp-msgs.h
> +++ b/include/openvswitch/ofp-msgs.h
> @@ -287,6 +287,8 @@ enum ofpraw {
>  OFPRAW_OFPST10_FLOW_REQUEST,
>  /* OFPST 1.1+ (1): struct ofp11_flow_stats_request, uint8_t[8][]. */
>  OFPRAW_OFPST11_FLOW_REQUEST,
> +/* OFPST 1.5+ (17): struct ofp15_oxs_flow_stats_request, uint8_t[8][]. */
> +OFPRAW_OFPST15_OXS_FLOW_REQUEST,
>  /* NXST 1.0 (0): struct nx_flow_stats_request, uint8_t[8][]. */
>  OFPRAW_NXST_FLOW_REQUEST,
>  
> @@ -296,6 +298,8 @@ enum ofpraw {
>  OFPRAW_OFPST11_FLOW_REPLY,
>  /* OFPST 1.3+ (1): uint8_t[]. */
>  OFPRAW_OFPST13_FLOW_REPLY,
> +/* OFPST 1.5+ (17): uint8_t[]. */
> +OFPRAW_OFPST15_OXS_FLOW_REPLY,
>  /* NXST 1.0 (0): uint8_t[]. */
>  OFPRAW_NXST_FLOW_REPLY,
>  
> @@ -628,10 +632,12 @@ enum ofptype {
>  OFPTYPE_FLOW_STATS_REQUEST,  /* OFPRAW_OFPST10_FLOW_REQUEST.
>* OFPRAW_OFPST11_FLOW_REQUEST.
>*

[ovs-dev] [PATCH 1/2] OF1.5/EXT-334 OXS/Individal Flow Statistics

2017-05-11 Thread Ben Pfaff 
From: Satya Valli 

---
 include/openflow/openflow-1.5.h | 48 +
 include/openvswitch/ofp-msgs.h  |  6 ++
 lib/automake.mk |  2 ++
 lib/ofp-parse.c | 45 +-
 lib/ofp-print.c | 10 +
 lib/rconn.c |  2 ++
 ofproto/ofproto.c   |  4 
 7 files changed, 116 insertions(+), 1 deletion(-)

diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
index 3649e6c29e63..ff5fa13fae4a 100644
--- a/include/openflow/openflow-1.5.h
+++ b/include/openflow/openflow-1.5.h
@@ -150,4 +150,52 @@ struct ofp15_group_desc_stats {
 };
 OFP_ASSERT(sizeof(struct ofp15_group_desc_stats) == 16);
 
+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. */
+};
+
 #endif /* openflow/openflow-1.5.h */
diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
index 34708f3bd846..49b0f7a2cfa8 100644
--- a/include/openvswitch/ofp-msgs.h
+++ b/include/openvswitch/ofp-msgs.h
@@ -287,6 +287,8 @@ enum ofpraw {
 OFPRAW_OFPST10_FLOW_REQUEST,
 /* OFPST 1.1+ (1): struct ofp11_flow_stats_request, uint8_t[8][]. */
 OFPRAW_OFPST11_FLOW_REQUEST,
+/* OFPST 1.5+ (17): struct ofp15_oxs_flow_stats_request, uint8_t[8][]. */
+OFPRAW_OFPST15_OXS_FLOW_REQUEST,
 /* NXST 1.0 (0): struct nx_flow_stats_request, uint8_t[8][]. */
 OFPRAW_NXST_FLOW_REQUEST,
 
@@ -296,6 +298,8 @@ enum ofpraw {
 OFPRAW_OFPST11_FLOW_REPLY,
 /* OFPST 1.3+ (1): uint8_t[]. */
 OFPRAW_OFPST13_FLOW_REPLY,
+/* OFPST 1.5+ (17): uint8_t[]. */
+OFPRAW_OFPST15_OXS_FLOW_REPLY,
 /* NXST 1.0 (0): uint8_t[]. */
 OFPRAW_NXST_FLOW_REPLY,
 
@@ -628,10 +632,12 @@ enum ofptype {
 OFPTYPE_FLOW_STATS_REQUEST,  /* OFPRAW_OFPST10_FLOW_REQUEST.
   * OFPRAW_OFPST11_FLOW_REQUEST.
   * OFPRAW_NXST_FLOW_REQUEST. */
+OFPTYPE_OXS_FLOW_STATS_REQUEST,  /* OFPRAW_OFPST15_OXS_FLOW_REQUEST. */
 OFPTYPE_FLOW_STATS_REPLY,/* OFPRAW_OFPST10_FLOW_REPLY.
   * OFPRAW_OFPST11_FLOW_REPLY.
   * OFPRAW_OFPST13_FLOW_REPLY.
   * OFPRAW_NXST_FLOW_REPLY. */
+OFPTYPE_OXS_FLOW_STATS_REPLY,/* OFPRAW_OFPST15_OXS_FLOW_REPLY. */
 OFPTYPE_AGGREGATE_STATS_REQUEST, /* OFPRAW_OFPST10_AGGREGATE_REQUEST.