On Fri, Mar 21, 2014 at 09:17:28AM -0700, Ben Pfaff wrote: > On Fri, Mar 21, 2014 at 10:50:24AM +0200, Alexandru Copot wrote: > > On Tue, Mar 18, 2014 at 12:07 AM, Ben Pfaff <b...@nicira.com> wrote: > > >> > > > >> > The one bit that I'm concerned about is that it allows OpenFlow 1.4 to > > >> > be enabled even though there are unimplemented messages that will cause > > >> > Open vSwitch to abort if ovs-vswitchd receives one. > > >> > > >> This was my concern too after seeing that commit where you defined > > >> the protocol versions as macros. > > >> > > >> > We need to avoid that somehow; one way would be to not allow > > >> > OpenFlow 1.4 to be enabled as long as any of those messages exist. > > >> > > >> Do you mean to disable it until all those messages are implemented ? > > >> It will be a while to implement all of OF 1.4. Maybe if we make it a > > >> build > > >> configuration option, but not sure how to do that. > > > > > > I'd be happy with a command-line option to ovs-vswitchd and ovs-ofctl. > > > > For ovs-ofctl the user should explicitly set the OpenFlow 1.4 version with > > -O OpenFlow14 and this is useful for testing. We can add an additional > > parameter to ovs-ofctl (--alow-of14) which is by default set to false. > > When not set, this parameter will not allow ovs-ofctl -O OpenFlow14 to run. > > For ovs-ofctl, I'm happy with -O OpenFlow14, as long as "-O any" and > similar abbreviations don't yet include OF1.4. > > > Regarding ovs-vswitchd, we cannot have situations where the switch will > > receive an OF1.4 message. This is because, at version negotiation, > > OFPUTIL_DEFAULT_VERSIONS is 1 << OFP10_VERSION. > > > > We think that it's safe to enable OpenFlow1.4 because this will only be > > noticed when the user explicitly set it with ovs-ofctl and when the > > Protocols > > fields in the database is set to OpenFlow14. Also, this can speed up > > development for other OpenFlow 1.4 features. > > I really don't like the possibility of crashes without enabling it in > some nonstandard way. A command-line option is one way that would > make me happy. Enabling some remote administrator to crash the switch > just because he accidentally turned on a buggy protocol isn't good > enough.
I decided that the best thing to do here was to do it myself, so I committed your patch 1/5 (with a few adjustments) followed by a patch that disables OF1.4 unless one specifies --enable-of14 on the ovs-vswitchd command line. This should allow development work to proceed, and in particular it will make it possible for this week's hackathon to explore OF1.4 features. Please consider rebasing and reposting the remainder of the patch series. Thanks, Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
