On Thu, Nov 01, 2012 at 12:32:29AM -0700, Justin Pettit wrote: > This commit switches to using a single backing datapath (called > "ovs-datapath") for all bridges of that datapath's type. Previously, > resources couldn't be shared across bridges, since each was in its own > datapath. This change will allow sharing of tunnels and cheaper patch > ports to be added in the future. > > Since bridges share a common datapath, the ovs-dpctl commands won't > provide bridge-specific information. Users wishing to have that > information should use the new "ovs-appctl dpif/*" commands as > documented in ovs-vswitchd(8). > > Signed-off-by: Justin Pettit <jpet...@nicira.com>
... > diff --git a/NEWS b/NEWS > index 5c66b5e..41cf9e7 100644 > --- a/NEWS > +++ b/NEWS > @@ -47,6 +47,11 @@ v1.9.0 - xx xxx xxxx > - The ofproto library is now responsible for assigning OpenFlow port > numbers. An ofproto implementation should assign them when > port_construct() is called. > + - All dpif-based bridges of a particular type share a common > + datapath called "ovs-<type>", where "<type>" is the type of dpif. It seems likely to me that many users aren't really aware of types. So perhaps substitute an example, like this: > + - All dpif-based bridges of a particular type share a common > + datapath called "ovs-<type>", e.g. "ovs-system". I think that the new #include "svec.h" is not necessary, because this version does not use svecs. It looks like "dpif-provider.h" is necessary only to access ->full_name and ->base_name in struct dpif. I think it would be better to use dpif_name() and dpif_base_name() instead, to avoid a minor layering violation. The update to set_sflow() made me wonder what the dpif_sflow code does with the dpif. The answer is that it does nothing with the dpif, it just stores away a pointer and never references it again. So it would be better to update ofproto-dpif-sflow.c to just remove that parameter from dpif_sflow_create(), probably in a preparatory patch. I think that the code in port_dump_next() should be made a little more robust. Currently, I believe that if there is some discrepancy between ofproto->ports and what is in the kernel datapath (e.g. some admin meddling with "ovs-dpctl del-port"), then that discrepancy will be returned as an error to the caller, who will interpret it as meaning that there are no more ports and thus not call back to find out any more. So I think that port_dump_next() probably should have a retry loop for when port_query_by_name() fails. update_stats() ends up calling odp_port_to_ofport() twice, once via odp_port_to_ofproto_dpif(), once via odp_port_to_ofp_port(). It would be more efficient to do this just once, directly. I don't think the code would look any worse for it, and perhaps even a little better. Similarly, odp_port_to_ofproto_dpif() does the same thing, through odp_port_to_ofproto_dpif() and ofproto_dpif_vsp_adjust(). This one is more important because it is on the flow setup fast path. I think that doing this lookup twice is likely to show up in "ovs-benchmark rate" results. Thanks, Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
