[ovs-dev] [packet_in v2 15/18] flow: Create new flow_metadata structure for packet_in messages.

[Ben Pfaff]

On Sat, Jan 07, 2012 at 11:11:56AM -0800, Ethan Jackson wrote:
> This will ease the implementation of future patches.
> 
> Signed-off-by: Ethan Jackson <ethan at nicira.com>

Looks good.  I think that this will become more useful over time.

In ofp_print_packet_in(), the use of ~ (twice) below makes me nervous.
Testing the result of ~ for a nonzero value is risky, because C's
promotion rules mean that it will always be true due if the expression's
type is narrower than "int".  That should not ever be the case here, but
I think I still would prefer explicit comparisons against
e.g. htonll(UINT64_MAX), because it requires less thinking to see that
it is correct:

> +    if (pin.fmd.tun_id_mask) {
> +        ds_put_format(string, " tun_id=0x%"PRIx64, ntohll(pin.fmd.tun_id));
> +        if (~pin.fmd.tun_id_mask) {
> +            ds_put_format(string, "/0x%"PRIx64, ntohll(pin.fmd.tun_id_mask));
> +        }
> +    }
> +
> +    for (i = 0; i < FLOW_N_REGS; i++) {
> +        if (pin.fmd.reg_masks[i]) {
> +            ds_put_format(string, " reg%d=0x%"PRIx32, i, pin.fmd.regs[i]);
> +            if (~pin.fmd.reg_masks[i]) {
> +                ds_put_format(string, "/0x%"PRIx32, pin.fmd.reg_masks[i]);
> +            }
> +        }
> +    }

Thanks,

Ben.