Looks cleaner, thanks.
Ethan
On Fri, Jan 13, 2012 at 16:43, Ben Pfaff <b...@nicira.com> wrote:
> If the ->construct() function initializes the OpenFlow tables itself,
> then it can conveniently do implementation-specific configuration of
> those tables afterward. There isn't any such configuration to do yet;
> an upcoming commit will add some.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> ofproto/ofproto-dpif.c | 7 ++++---
> ofproto/ofproto-provider.h | 17 ++++++++---------
> ofproto/ofproto.c | 26 +++++++++++++++++---------
> 3 files changed, 29 insertions(+), 21 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index c4ededa..5acd349 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -620,7 +620,7 @@ dealloc(struct ofproto *ofproto_)
> }
>
> static int
> -construct(struct ofproto *ofproto_, int *n_tablesp)
> +construct(struct ofproto *ofproto_)
> {
> struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
> const char *name = ofproto->up.name;
> @@ -682,9 +682,10 @@ construct(struct ofproto *ofproto_, int *n_tablesp)
>
> hmap_insert(&all_ofproto_dpifs, &ofproto->all_ofproto_dpifs_node,
> hash_string(ofproto->up.name, 0));
> -
> - *n_tablesp = N_TABLES;
> memset(&ofproto->stats, 0, sizeof ofproto->stats);
> +
> + ofproto_init_tables(ofproto_, N_TABLES);
> +
> return 0;
> }
>
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 33c75df..f79c41e 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -81,6 +81,8 @@ struct ofproto {
> bool vlans_changed; /* True if new VLANs are in use. */
> };
>
> +void ofproto_init_tables(struct ofproto *, int n_tables);
> +
> struct ofproto *ofproto_lookup(const char *name);
> struct ofport *ofproto_get_port(const struct ofproto *, uint16_t ofp_port);
>
> @@ -295,14 +297,11 @@ struct ofproto_class {
> *
> * When ->construct() is called, the client does not yet know how many
> flow
> * tables the datapath supports, so ofproto->n_tables will be 0 and
> - * ofproto->tables will be NULL. ->construct() should store the number
> of
> - * flow tables supported by the datapath (between 1 and 255, inclusive)
> - * into '*n_tables'. After a successful return, the client will
> initialize
> - * the base 'n_tables' member to '*n_tables' and allocate and initialize
> - * the base 'tables' member as the specified number of empty flow tables.
> - * Each flow table will be initially empty, so ->construct() should
> delete
> - * flows from the underlying datapath, if necessary, rather than
> populating
> - * the tables.
> + * ofproto->tables will be NULL. ->construct() should call
> + * ofproto_init_tables() to allocate and initialize ofproto->n_tables and
> + * ofproto->tables. Each flow table will be initially empty, so
> + * ->construct() should delete flows from the underlying datapath, if
> + * necessary, rather than populating the tables.
> *
> * Only one ofproto instance needs to be supported for any given datapath.
> * If a datapath is already open as part of one "ofproto", then another
> @@ -325,7 +324,7 @@ struct ofproto_class {
> * returns.
> */
> struct ofproto *(*alloc)(void);
> - int (*construct)(struct ofproto *ofproto, int *n_tables);
> + int (*construct)(struct ofproto *ofproto);
> void (*destruct)(struct ofproto *ofproto);
> void (*dealloc)(struct ofproto *ofproto);
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index f7a3027..7eff4a0 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -298,8 +298,6 @@ ofproto_create(const char *datapath_name, const char
> *datapath_type,
> {
> const struct ofproto_class *class;
> struct ofproto *ofproto;
> - struct oftable *table;
> - int n_tables;
> int error;
>
> *ofprotop = NULL;
> @@ -352,7 +350,7 @@ ofproto_create(const char *datapath_name, const char
> *datapath_type,
> ofproto->vlan_bitmap = NULL;
> ofproto->vlans_changed = false;
>
> - error = ofproto->ofproto_class->construct(ofproto, &n_tables);
> + error = ofproto->ofproto_class->construct(ofproto);
> if (error) {
> VLOG_ERR("failed to open datapath %s: %s",
> datapath_name, strerror(error));
> @@ -360,12 +358,7 @@ ofproto_create(const char *datapath_name, const char
> *datapath_type,
> return error;
> }
>
> - assert(n_tables >= 1 && n_tables <= 255);
> - ofproto->n_tables = n_tables;
> - ofproto->tables = xmalloc(n_tables * sizeof *ofproto->tables);
> - OFPROTO_FOR_EACH_TABLE (table, ofproto) {
> - oftable_init(table);
> - }
> + assert(ofproto->n_tables);
>
> ofproto->datapath_id = pick_datapath_id(ofproto);
> VLOG_INFO("using datapath ID %016"PRIx64, ofproto->datapath_id);
> @@ -376,6 +369,21 @@ ofproto_create(const char *datapath_name, const char
> *datapath_type,
> }
>
> void
> +ofproto_init_tables(struct ofproto *ofproto, int n_tables)
> +{
> + struct oftable *table;
> +
> + assert(!ofproto->n_tables);
> + assert(n_tables >= 1 && n_tables <= 255);
> +
> + ofproto->n_tables = n_tables;
> + ofproto->tables = xmalloc(n_tables * sizeof *ofproto->tables);
> + OFPROTO_FOR_EACH_TABLE (table, ofproto) {
> + oftable_init(table);
> + }
> +}
> +
> +void
> ofproto_set_datapath_id(struct ofproto *p, uint64_t datapath_id)
> {
> uint64_t old_dpid = p->datapath_id;
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
