On 30 September 2014 10:15, Ben Pfaff <b...@nicira.com> wrote: > On Fri, Sep 26, 2014 at 09:28:16PM +1200, Joe Stringer wrote: > > This patch modifies the dpif interface to allow flows to be manipulated > > using a 128-bit identifier. This allows revalidator threads to perform > > datapath operations faster, as they do not need to serialise the entire > > flow key for operations like flow_get and flow_delete. In conjunction > > with a future patch to simplify the dump interface, this provides a > > significant performance benefit for revalidation. > > > > When handlers assemble flow_put operations, they specify a unique > > identifier (UID) for each flow as it is passed down to the datapath to > > be stored with the flow. The UID is currently provided to handlers > > by the dpif during upcall processing. > > > > When revalidators assemble flow_get or flow_del operations, they specify > > the UID for the flow along with the key, and a boolean for whether to > > send the request using only a UID or to send the request using the UID > > and flow key. The former is preferred for newer datapaths that support > > UID, while the latter is used for backwards compatibility. > > I don't think I understand 'terse'. I guess that the caller uses the > 'terse' flag if it knows that the datapath supports UIDs. But can't > the datapath know that on its own, and use the key or not based on > that knowledge? Then again, I see that some of the callers (e.g. the > feature probe functions) always pass false for 'terse'. Does this > mean that sometimes the key should always be included? I'm not quite
following the logic. > Perhaps my use of the word 'terse' was more snippy than succinct :-) The 'terse' flag in this case is saying "don't serialise flow keys during get/delete ops". I think that this could be hidden entirely inside dpif (or dpif-*) if the UID support availability is cached in the dpif layer (currently it's in the dpif backer in ofproto-dpif). The main thing I wanted to avoid is the UID support value affecting the other probes. The other callers in ofproto-dpif-upcall don't really care, would just prefer that the most optimal path is used (ie, if the datapath supports skipping keys then skip them). Otherwise it may help for this terse flag to be changeable at runtime for debugging or something. > One point that I want to make sure is covered is startup. We will > hash differently on every startup of Open vSwitch, because of the > random secret, so I want to make sure that the initial scan and dump > of the flow table from the revalidator thread will properly handle the > wrong UIDs on that initial scan. > Hmm. At the moment, this series has handlers installing udpif_keys, and revalidators never install them. Earlier versions of the patchset would cause all such flows to be deleted when they are dumped, the current version just leaves them alone. To handle this case, we probably need to go back to installing updif_key entries from revalidator threads if the key cannot be found. I tried to avoid this for simplicity and consistent ukey map access, but I guess it's a fairly rare occurrence that shouldn't cause much trouble. So long as the ofproto-dpif-upcall logic doesn't depend on the relationship between flow_key and UID, I think that this case should be fine. This is the case right now. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
