On Thu, Jun 11, 2015 at 03:41:53PM -0700, Jarno Rajahalme wrote: > > > On Jun 11, 2015, at 3:00 PM, Gurucharan Shetty <shet...@nicira.com> wrote: > > > > On Tue, Jun 9, 2015 at 5:24 PM, Jarno Rajahalme <jrajaha...@nicira.com > > <mailto:jrajaha...@nicira.com>> wrote: > >> This patch allows classifier rules to become visible and invisible in > >> specific versions. A 'version' is defined as a positive monotonically > >> increasing integer, which never wraps around. > >> > >> The new 'visibility' attribute replaces the prior 'to_be_removed' and > >> 'visible' attributes. > >> > >> When versioning is not used, the 'version' parameter should be passed > >> as 'CLS_MIN_VERSION' when creating rules, and 'CLS_MAX_VERSION' when > >> looking up flows. > >> > >> This feature enables the support for atomic OpenFlow bundles without > >> significant performance penalty on 64-bit systems. There is a > >> performance decrease in 32-bit systems due to 64-bit atomics used. > >> > >> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com> > > > > Looks like this patch causes ovs-vswitchd to crash in unit tests of > > Windows with the following backtrace: > > > >> ovs-vswitchd.exe!abort() Line 88 C > > ovs-vswitchd.exe!ofproto_dpif_add_internal_flow(ofproto_dpif * > > ofproto, const match * match, int priority, unsigned short > > idle_timeout, const ofpbuf * ofpacts, rule * * rulep) Line 5454 C > > On master line ofproto-dpif.c:5452 is the OVS_NOT_REACHED(); > > I’ve seen this happen earlier while coding this, but this should not happen > on master. The cause was that the internal rule was added for a next version, > but the lookup was done with the old version, so the new rule was invisible. > But on master the rule is added in version CLS_MIN_VERSION (= 1) and the > lookup (that seems to fail) is done in CLS_MAX_VERSION (LLONG_MAX), so it > should work. > > You could try to modify the classifier_lookup line in ofproto-dpif.c to use a > much lower number, in case something goes wrong with the atomics used within > the classifier: > > - cls_rule = classifier_lookup(cls, CLS_MAX_VERSION, flow, wc); > + cls_rule = classifier_lookup(cls, 10, flow, wc); > > In master the version number is not yet incremented, so this should work.
In C I think that enums always have type "int", but CLS_MAX_VERSION is bigger than int: enum { CLS_MIN_VERSION = 1, /* Default version number to use. */ CLS_MAX_VERSION = LLONG_MAX, /* Last possible version number. */ CLS_MAX_INDICES = 3, /* Maximum number of lookup indices per subtable. */ CLS_MAX_TRIES = 3 /* Maximum number of prefix trees per classifier. */ }; Does it make any difference if you do something like this instead: enum { CLS_MIN_VERSION = 1, /* Default version number to use. */ #define CLS_MAX_VERSION LLONG_MAX /* Last possible version number. */ CLS_MAX_INDICES = 3, /* Maximum number of lookup indices per subtable. */ CLS_MAX_TRIES = 3 /* Maximum number of prefix trees per classifier. */ }; _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev