[ovs-dev] [PATCH V1] bridge.c: Change variable "ofport" type in "struct if_cfg" and "struct iface"

Ben Pfaff blp at nicira.com
Tue Jun 4 17:17:48 UTC 2013


On Fri, May 31, 2013 at 04:04:45PM -0700, Alex Wang wrote:
> This patch changes the variable type of "ofport" in "struct if_cfg" and
> "struct iface" from int64_t to uint16_t. This is more consistent with
> the OpenFlow-1.0 port definition.
> 
> Also, before this patch, -1 is used to indicate an unknown port. This
> patch uses OFPP_NONE, since "ofport" becomes uint16_t.
> 
> Signed-off-by: Alex Wang <alexw at nicira.com>

Thank you.  I agree that this helps make the code a little cleaner.  I
have a couple of comments.

First, what is the purpose of the casts here:

> +static uint16_t
>  iface_pick_ofport(const struct ovsrec_interface *cfg)
>  {
> -    int64_t ofport = cfg->n_ofport ? *cfg->ofport : OFPP_NONE;
> -    return cfg->n_ofport_request ? *cfg->ofport_request : ofport;
> +    uint16_t ofport = cfg->n_ofport ? (uint16_t) *cfg->ofport : OFPP_NONE;
> +    return cfg->n_ofport_request ? (uint16_t) *cfg->ofport_request : ofport;

Second, iface_configure_qos() tests ->ofp_port for >= 0, which no
longer makes sense.  I guess it should test for ->ofp_port !=
OFPP_NONE now.

Thanks,

Ben.



More information about the dev mailing list