On Tue, Feb 05, 2019 at 04:26:56PM -0800, Ashish Varma wrote: > In the parse_flow_monitor_request(), usable_protocols is an out parameter. > (and is set in parse_flow_monitor_request() only if a match field is > provided. ) > allowed_versions on the other hand is used only in the ofctl_monitor > function. That is the reason for the check in ofctl_monitor.
The usual way we handle this sort of version dependency is a bitmap like 'usable_protocols'. It works well for flows, for example. You can see lots of examples in ovs-ofctl.c if you search for the identifier usable_protocols. Looking closer at this instance, the existing code is buggy. Currently, only OpenFlow 1.0 should be supported, because that's the only protocol where OVS actually supports flow monitoring. parse_flow_monitor_request() should be returning that consistently as the out parameter (as you noted). Or it should be returning 0 if it's impossible to request a flow monitor at all (in the case where OF1.0 can't support whatever field is specified when parse_flow_monitor_request__ parses a field). But it's buggy and nothing ever properly initializes it. That bug is masked by another bug: nothing ever checks it, either! And, finally, there is a third bug: ovs-ofctl calls ofputil_append_flow_monitor_request() and blindly always uses OF1.0, which might not be the protocol actually in use on the vconn. The manpage for the ovs-ofctl monitor command doesn't say that "watch:" is OF1.0 only or that it is an Open vSwitch extension, either, although it should. It would be for the best if we can fix all these bugs before we add support for OF1.4+ flow monitor, and then backport the bug fixes. Does my description of the problems make sense? Can you tackle these problems too? Thanks, Ben. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
