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. > > 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
SatyaValli, can you respond to this? _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
