On Thu, Oct 18, 2012 at 12:51:51PM -0700, Justin Pettit wrote:
> A future commit will make all bridges of a particular dpif share a
> single backing datapath. In order to handle restart, the datapath will
> need to have some idea of what the initial state looks like. Otherwise,
> it won't know which ports belong to which bridges and orphaned ports may
> never be cleaned up.
>
> This commit introduces an initialization method to ofproto, which takes
> as an argument a high-level description of the bridges and ports. An
> ofproto provider can then use this information to initialize its state.
>
> Signed-off-by: Justin Pettit <[email protected]>
Hmm, putting "return;" in an otherwise empty function isn't our usual
practice:
> static void
> +init(const struct shash *iface_hints OVS_UNUSED)
> +{
> + return;
> +}
...
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index a7ebc09..afcfe8d 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -319,6 +319,15 @@ struct ofproto_class {
> /* ## ----------------- ## */
> /* ## Factory Functions ## */
> /* ## ----------------- ## */
It'd be nice to have a blank line here.
> + /* Iinitializes provider. The caller may pass in 'iface_hints',
s/Iinitializes/Initializes/.
> + * which contains an shash of "iface_hint" elements indexed by the
Would you mind saying "struct iface_hint" elements? It took me a minute
to understand that was what you meant.
> + * interface's name. The provider may use these hints to describe
> + * the startup configuration in order to reinitialize its state.
> + * The caller owns the provided data, so a provider must make copies
> + * of anything required. An ofproto provider must remove any
> + * existing state that is not described by the hint, and may choose
> + * to remove it all. */
> + void (*init)(const struct shash *iface_hints);
Otherwise this looks good, thank you.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
