[ovs-dev] [generic tci mask 4/8] flow: Move functions for dealing with wildcard bit counts to ofp-util.

Justin Pettit jpettit at nicira.com
Fri Nov 19 22:55:00 UTC 2010


On Nov 10, 2010, at 3:37 PM, Ben Pfaff wrote:

> +/* Given the wildcard bit count in bits 0...5 (inclusive) of 'wildcards',
> + * returns an IP netmask with a 1 in each bit that must match and a 0 in each
> + * bit that is wildcarded.
> + *
> + * The bits in 'wildcards' are in the format used in enum ofp_flow_wildcards: 0
> + * is exact match, 1 ignores the LSB, 2 ignores the 2 least-significant bits,
> + * ..., 32 and higher wildcard the entire field.  This is the *opposite* of the
> + * usual convention where e.g. /24 indicates that 8 bits (not 24 bits) are
> + * wildcarded. */

The comment references "wildcards", but the argument was changed to "wcbits".

> +ovs_be32
> +ofputil_wcbits_to_netmask(int wcbits)
> +{
> +    wcbits &= wcbits;

I think you can safely remove this line.  :-)

> +/* Given the IP netmask 'netmask', returns the number of bits of the IP address
> + * that it wildcards.  The return value is only accurate if 'netmask' is a CIDR
> + * netmask (see ip_is_cidr()). */
> +int
> +ofputil_netmask_to_wcbits(ovs_be32 netmask)
> +{

Do you think it's worth making an assertion that ip_is_cidr() is true?

> +/* Returns true if 'netmask' is a CIDR netmask, that is, if it consists of N
> + * high-order 1-bits and 32-N low-order 0-bits. */
> +static inline bool
> +ip_is_cidr(ovs_be32 netmask)
> +{
> +    uint32_t x = ~ntohl(netmask);
> +    return !(x & (x + 1));
> +}

Clever.  :-)

> @@ -200,9 +201,12 @@ match(const struct cls_rule *wild, const struct flow *fixed)
>         if (wild->wc.wildcards & f->wildcards) {
>             uint32_t test = get_unaligned_u32(wild_field);
>             uint32_t ip = get_unaligned_u32(fixed_field);
> -            int shift = (f_idx == CLS_F_IDX_NW_SRC
> +            uint32_t mask;
> +            int shift;
> +
> +            shift = (f_idx == CLS_F_IDX_NW_SRC
>                          ? OFPFW_NW_SRC_SHIFT : OFPFW_NW_DST_SHIFT);
> -            uint32_t mask = flow_nw_bits_to_mask(wild->wc.wildcards, shift);
> +            mask = ofputil_wcbits_to_netmask(wild->wc.wildcards) >> shift;

Should that shift be occurring within the parentheses?

--Justin






More information about the dev mailing list