Hi Jan and Ben,
Please find the inline responses.
-----Ben Pfaff <b...@ovn.org> wrote: -----
To: Jan Scheurich <jan.scheur...@ericsson.com>
From: Ben Pfaff <b...@ovn.org>
Date: 01/05/2018 02:35AM
Cc: SatyaValli <satyavalli.r...@gmail.com>, "d...@openvswitch.org"
<d...@openvswitch.org>, Manasa Cherukupally <manasa.cherukupa...@tcs.com>,
Pavani Panthagada <p.pava...@tcs.com>, Lavanya Harivelam
<harivelam.lava...@tcs.com>, Surya Muttamsetty <muttamsetty.su...@tcs.com>,
SatyaValli <satyavalli.r...@tcs.com>
Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry
Statistics Support
On Wed, Jan 03, 2018 at 04:24:06PM +0000, Jan Scheurich wrote:
> > > >
> > > > 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 <bridge> idle_time
> > > > ovs-ofctl dump-flows -O OpenFlow15 <bridge> packet_count
> > > > ovs-ofctl dump-flows -O OpenFlow15 <bridge> byte_count
> > >
> > > This would break backward compatibility for one of the most frequently
> > > used OVS CLI commands. Why don't you introduce a new
> > command such as "ovs-ofctl dump-flow-stats" for the new OXS stats?
> >
> > I think you might be misinterpreting the meaning here. It doesn't
> > appear to break compatibility, at least not in a major way, since it
> > doesn't do a lot of updates to the tests that would otherwise be
> > required.
>
> Perhaps I am missing the point of some of these changes. I understand that
> OVS needs to support the new extensible OXS flow stats syntax in OpenFlow 1.5
> and the differentiated MP request/reply pairs OFPMP_FLOW_DESC (replacing the
> former OFPMP_FLOW) and OFPMP_FLOW_STATS (just fetching flow stats per flow
> w/o the rest of the flow data).
>
> But I don't understand why this should have any impact on the existing CLI
> command "ovs-ofctl dump-flows" and its output. This tool expressly fetches
> and displays the complete flow dump from OVS, including match,
> instructions/actions and statistics. When using OF 1.5 it should
> transparently apply OFPMP_FLOW_DESC MP request/reply to fetch the data, up to
> OF 1.4 it should use the original OFPMP_FLOW.
>
> I can't see any ovs-ofctl use case that would justify the use of the new
> OFPMP_FLOW_STATS request/reply. The removed data in the reply compared to the
> full flow description are mainly the instructions, the full match is still
> there to identify each flow. So cutting down the transferred data volume can
> hardly be the reason (Note, this may still be different for real OF 1.5
> controllers).
>
> If you believe we should have an ovs-ofctl command anyhow, e.g. for testing
> purposes, I suggest to introduce a new command or add an option to dump-flows
> to force use of this particular MP message. The output would be limited to
> flow match and stats in that case.
>
As per our understanding and from previous review comments we treated OF1.5+
has two different ways to request and get replies for Flow Stats: FLOW_DESC and
FLOW_STATS (which will be even used for Flow Stats Trigger). And we've
supported this with the help of two commands
OFPMP_FLOW_DESC - ovs-ofctl dump-flow-desc -O OpenFlow15
OFPMP_FLOW_STATS - ovs-ofctl dump-flows -O OpenFlow15
> The new command to dump aggregate flow stats is of course a welcome addition.
> It should work irrespectively of the used OpenFlow version with the
> OFPMP_AGGREGATE_STATS primitive using classic flow stats prior to OF 1.5 and
> OXS flow stats in OF 1.5.
>
> All in all I am NACK-ing this patch as it stands.
>
> Regards, Jan
>
> BTW: I have played a bit with the patch. The existing ovs-ofctl test cases
> appear to not break because the changes described in the commit message and
> the documentation are not effective. The legacy command format "ovs-ofctl
> dump-flows -O OpenFlow15 <bridge> [<match>]" still produces the complete flow
> dump including instructions/actions:
>
We cross checked and tested the patch again on the latest OVS GIT and we
haven't observed any of these issues. Could you please share your topology and
any other configurations for better understanding of the issue:
Please find logs for the same with the below configuration / flow-add rules:
# git log --after="2018-01-03"
commit b02bf23fe3caa011a842f22fc802d77f989dd81b
Author: SatyaValli <satyavalli.r...@tcs.com>
Date: Thu Jan 4 16:59:03 2018 +0530
OF_Extensible_Statistics
commit 77b7d23230fecf774c2a707b7b42fdb8a65a8892
Author: Ben Pfaff <b...@ovn.org>
Date: Wed Jan 3 16:11:02 2018 -0800
ofp-actions: Log version, vendor, and type of unknown actions being parsed.
This may help debugging difficult controller problems.
Reported-by: Su Wang <suw...@vmware.com>
Signed-off-by: Ben Pfaff <b...@ovn.org>
Acked-by: Justin Pettit <jpet...@ovn.org>
# ovs-vsctl --version
ovs-vsctl (Open vSwitch) 2.8.90
DB Schema 7.15.1
# ovs-ofctl add-flow -O OpenFlow15 br0
icmp,reset_counts,check_overlap,actions=normal
# ovs-ofctl add-flow -O OpenFlow15 br0 udp,tp_dst=200,actions=normal
# ovs-ofctl add-flow -O OpenFlow15 br0 tcp,tp_src=100,actions=normal
> # ovs-ofctl -Oopenflow15 dump-flow-desc br0
> OFPST_FLOW_DESC reply (OF1.5) (xid=0x2):
> cookie=0x0, duration=90.515s, table=0, n_packets=0, n_bytes=0, reset_counts
> idle_age=840, hard_age=32766, in_port="br0-ns1" actions=output:"br0-ns2"
> cookie=0x0, duration=90.507s, table=0, n_packets=0, n_bytes=0, reset_counts
> idle_age=840, hard_age=32766, in_port="br0-ns2" actions=output:"br0-ns1"
> cookie=0x0, duration=850.125s, table=0, n_packets=2, n_bytes=180,
> idle_age=849, hard_age=32766, priority=0 actions=NORMAL
>
##Flow Description Dump:
## ovs-ofctl dump-flow-desc -O OpenFlow15 br0
OFPST_FLOW_DESC reply (OF1.5) (xid=0x2):
cookie=0x0, duration=446.251s, table=0, n_packets=0, n_bytes=0, check_overlap
reset_counts idle_age=0, hard_age=0, icmp actions=NORMAL
cookie=0x0, duration=440.523s, table=0, n_packets=0, n_bytes=0, idle_age=0,
hard_age=0, udp,tp_dst=200 actions=NORMAL
cookie=0x0, duration=436.579s, table=0, n_packets=0, n_bytes=0, idle_age=0,
hard_age=0, tcp,tp_src=100 actions=NORMAL
cookie=0x0, duration=3451.715s, table=0, n_packets=0, n_bytes=0, idle_age=0,
hard_age=0, priority=0 actions=NORMAL
## ovs-ofctl dump-flow-desc -O OpenFlow15 br0 packet_count
OFPST_FLOW_DESC reply (OF1.5) (xid=0x4):
cookie=0x0, duration=338.579s, table=0, n_packets=32918, n_bytes=0,
check_overlap reset_counts idle_age=0, hard_age=0, icmp actions=NORMAL
cookie=0x0, duration=332.851s, table=0, n_packets=16417, n_bytes=0,
idle_age=0, hard_age=0, udp,tp_dst=200 actions=NORMAL
cookie=0x0, duration=328.907s, table=0, n_packets=12309, n_bytes=0,
idle_age=0, hard_age=0, tcp,tp_src=100 actions=NORMAL
cookie=0x0, duration=3344.043s, table=0, n_packets=53604586, n_bytes=0,
idle_age=0, hard_age=0, priority=0 actions=NORMAL
> # ovs-ofctl -Oopenflow15 dump-flows br0
> cookie=0x0, duration=94.634s, table=0, n_packets=0, n_bytes=0, idle_age=844,
> hard_age=0, in_port="br0-ns1" actions=output:"br0-ns2"
> cookie=0x0, duration=94.626s, table=0, n_packets=0, n_bytes=0, idle_age=844,
> hard_age=0, in_port="br0-ns2" actions=output:"br0-ns1"
> cookie=0x0, duration=854.244s, table=0, n_packets=2, n_bytes=180,
> idle_age=853, hard_age=0, priority=0 actions=NORMAL
>
> The only difference appears to be that the flow property reset_counts is
> missing and that hard_age seems corrupt in the new ovs-ofctl dump-flow-desc.
>
> Furthermore, specifying any OXS field at the end of ovs-ofctl dump-flows
> <bridge> is without effect:
>
> # ovs-ofctl -Oopenflow15 dump-flows br0 duration
> cookie=0x0, duration=173.536s, table=0, n_packets=0, n_bytes=0,
> idle_age=923, hard_age=0, in_port="br0-ns1" actions=output:"br0-ns2"
> cookie=0x0, duration=173.528s, table=0, n_packets=0, n_bytes=0,
> idle_age=923, hard_age=0, in_port="br0-ns2" actions=output:"br0-ns1"
> cookie=0x0, duration=933.146s, table=0, n_packets=2, n_bytes=180,
> idle_age=932, hard_age=0, priority=0 actions=NORMAL
>
> # ovs-ofctl -Oopenflow15 dump-flows br0 packet_count
> cookie=0x0, duration=184.600s, table=0, n_packets=0, n_bytes=0,
> idle_age=934, hard_age=0, in_port="br0-ns1" actions=output:"br0-ns2"
> cookie=0x0, duration=184.593s, table=0, n_packets=0, n_bytes=0,
> idle_age=934, hard_age=0, in_port="br0-ns2" actions=output:"br0-ns1"
> cookie=0x0, duration=944.211s, table=0, n_packets=2, n_bytes=180,
> idle_age=943, hard_age=0, priority=0 actions=NORMAL
>
As OXS_OF_DURATION field is mandatory as per OF 1.5+ Spec for tracking the age
of flow entry in the switch, we've kept it as a default field. The other fields
OXS_OF_PACKET_COUNT and OXS_OF_BYTE_COUNT are kept as optional.
##Flow Status Dump:
## ovs-ofctl dump-flows -O OpenFlow15 br0 duration
cookie=0x0, duration=235.211s, table=0, n_packets=0, n_bytes=0, idle_age=0,
hard_age=0, icmp actions=NORMAL
cookie=0x0, duration=229.483s, table=0, n_packets=0, n_bytes=0, idle_age=0,
hard_age=0, udp,tp_dst=200 actions=NORMAL
cookie=0x0, duration=225.539s, table=0, n_packets=0, n_bytes=0, idle_age=0,
hard_age=0, tcp,tp_src=100 actions=NORMAL
cookie=0x0, duration=3240.675s, table=0, n_packets=0, n_bytes=0, idle_age=0,
hard_age=0, priority=0 actions=NORMAL
The OXS field value reflection is based on the flow match
## ovs-ofctl dump-flows -O OpenFlow15 br0 packet_count
cookie=0x0, duration=176.872s, table=0, n_packets=1102, n_bytes=0, idle_age=0,
hard_age=0, icmp actions=NORMAL
cookie=0x0, duration=171.144s, table=0, n_packets=544, n_bytes=0, idle_age=0,
hard_age=0, udp,tp_dst=200 actions=NORMAL
cookie=0x0, duration=167.200s, table=0, n_packets=408, n_bytes=0, idle_age=0,
hard_age=0, tcp,tp_src=100 actions=NORMAL
cookie=0x0, duration=3182.336s, table=0, n_packets=53592657, n_bytes=0,
idle_age=0, hard_age=0, priority=0 actions=NORMAL
## ovs-ofctl dump-flows -O OpenFlow15 br0 packet_count,idle_time
cookie=0x0, duration=143.448s, table=0, n_packets=1102, n_bytes=0, idle_age=5,
hard_age=0, icmp actions=NORMAL
cookie=0x0, duration=137.720s, table=0, n_packets=544, n_bytes=0, idle_age=5,
hard_age=0, udp,tp_dst=200 actions=NORMAL
cookie=0x0, duration=133.776s, table=0, n_packets=408, n_bytes=0, idle_age=5,
hard_age=0, tcp,tp_src=100 actions=NORMAL
cookie=0x0, duration=3148.912s, table=0, n_packets=53592655, n_bytes=0,
idle_age=0, hard_age=0, priority=0 actions=NORMAL
> When specifying it in addition to some filter match it is rejected:
>
> # ovs-ofctl -Oopenflow15 dump-flows br0 in_port="br0-ns1" packet_count
> ovs-ofctl: 'dump-flows' command takes at most 2 arguments
There is a slight syntax mismatch, please find the below dumps with match field:
##Dumps with Match Field:
## ovs-ofctl dump-flows -O OpenFlow15 br0 tcp,packet_count
cookie=0x0, duration=549s, table=0, n_packets=42146, n_bytes=0, idle_age=0,
hard_age=0, tcp,tp_src=100 actions=NORMAL
## ovs-ofctl dump-flows -O OpenFlow15 br0 tcp,byte_count
cookie=0x0, duration=565.577s, table=0, n_packets=0, n_bytes=2275884,
idle_age=0, hard_age=0, tcp,tp_src=100 actions=NORMAL
## ovs-ofctl dump-flows -O OpenFlow15 br0 udp,packet_count
cookie=0x0, duration=600.984s, table=0, n_packets=56200, n_bytes=0,
idle_age=0, hard_age=0, udp,tp_dst=200 actions=NORMAL
## ovs-ofctl dump-flows -O OpenFlow15 br0 icmp,packet_count
cookie=0x0, duration=614.544s, table=0, n_packets=112611, n_bytes=0,
idle_age=0, hard_age=0, icmp actions=NORMAL
## ovs-ofctl dump-flow-desc -O OpenFlow15 br0 icmp,packet_count
OFPST_FLOW_DESC reply (OF1.5) (xid=0x4):
cookie=0x0, duration=625.217s, table=0, n_packets=112611, n_bytes=0,
check_overlap reset_counts idle_age=0, hard_age=0, icmp actions=NORMAL
## ovs-ofctl dump-aggregate -O OpenFlow15 br0 icmp,packet_count
OFPST_AGGREGATE reply (OF1.5) (xid=0x4): packet_count=112611 byte_count=0
flow_count=1
SatyaValli, can you respond to this?
Thanks & Regards
Satya Valli
Tata Consultancy Services
Mailto: satyavalli.r...@tcs.com
Website: http://www.tcs.com
____________________________________________
Experience certainty. IT Services
Business Solutions
Consulting
____________________________________________
=====-----=====-----=====
Notice: The information contained in this e-mail
message and/or attachments to it may contain
confidential or privileged information. If you are
not the intended recipient, any dissemination, use,
review, distribution, printing or copying of the
information contained in this e-mail message
and/or attachments to it are strictly prohibited. If
you have received this communication in error,
please notify us by reply e-mail or telephone and
immediately and permanently delete the message
and any attachments. Thank you
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
