> On Dec 8, 2015, at 5:08 PM, Ben Pfaff <b...@ovn.org> wrote:
> 
> @@ -31,10 +32,11 @@ struct action_params {
>      * expr_parse()). */
>     const struct shash *symtab;
> 
> -     /* 'ports' must be a map from strings (presumably names of ports) to
> -      * integers (as one would provide to expr_to_matches()).  Strings used 
> in
> -      * the actions that are not in 'ports' are translated to zero. */
> -    const struct simap *ports;
> +    /* Looks up logical port 'name'.  If found, stores its port number in
> +     * '*portp' and returns true; otherwise, returns false. */
> +    bool (*lookup_port)(const void *aux, const char *name,
> +                        unsigned int *portp);

lookup_port_cb() refers to the second argument as "port_name" instead of 
"name".  It might be nice to use the same argument names.  If you change the 
name to "port_name", there are quite a few other protoypes defined in "expr.c" 
and "expr.h" that use "name", too.

> @@ -2443,19 +2452,21 @@ add_cmp_flow(const struct expr *cmp, const struct 
> simap *ports,
>  * (This treatment of string fields might be too simplistic in general, but it
>  * seems reasonable for now when string fields are used only for ports.) */
> uint32_t
> -expr_to_matches(const struct expr *expr, const struct simap *ports,
> -                struct hmap *matches)
> +expr_to_matches(const struct expr *expr,
> +                bool (*lookup_port)(const void *aux, const char *name,
> +                                    unsigned int *portp),
> +                const void *aux, struct hmap *matches)
> {

I think the comment describing this function needs to be updated, since "ports" 
is no longer an argument.

Acked-by: Justin Pettit <jpet...@ovn.org>

--Justin


_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to