On Wed, Oct 01, 2014 at 08:44:58PM +1300, Joe Stringer wrote: > On 30 September 2014 10:24, Ben Pfaff <b...@nicira.com> wrote: > > > I suspect that check_recirc() and check_uid() should pass > > DPIF_FP_MODIFY as well as DPIF_FP_CREATE, just in case a previous run > > of OVS was killed at just the right time to leave a straggler probe > > flow in the datapath. > > > > I checked on this, and it used to use DPIF_FP_MODIFY, but this was changed > in commit a7d1bbdcfe49e8c7a5575c9ab46b2ac9e5642ef1, log below. > > The probes already check for whether the flow exists, and count this as a > success case, then delete the flows, so I don't think this should be a > problem. > > > ofproto-dpif: Use DPIF_FP_CREATE but not DPIF_FP_MODIFY. > > A dpif reports EEXIST if a flow put operation that should create a new > flow > instead attempts to modify an existing flow, or ENOENT if a flow put > would > create a flow that overlaps some existing flow. The latter does not > always > indicate a bug in OVS userspace, because it can also mean that two > userspace OVS packet handler threads received packets that generated > the same megaflow for different microflows. Until now, userspace has > logged this, which confuses users by making them suspect a bug. We > could > simply not log ENOENT in userspace, but that would suppress logging for > genuine bugs too. Instead, this commit drops DPIF_FP_MODIFY from flow > put operations in ofproto-dpif, which transforms this particular > problem into EEXIST, which userspace already logs at "debug" level (see > flow_message_log_level()), effectively suppressing the logging in normal > circumstances. > > It appears that in practice ofproto-dpif doesn't actually ever need to > modify flows in the datapath, only create and delete them, so this > shouldn't cause problems.
OK, thanks. It's somewhat surprising, perhaps we should have a comment. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
