[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