> > > > > > 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. 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: # 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 # 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 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
