[ovs-dev] [Single DP 06/15] ofproto: Add initialization function.

Ben Pfaff blp at nicira.com
Fri Oct 19 17:53:58 UTC 2012


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 <jpettit at nicira.com>


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.



More information about the dev mailing list