I would be inclined to implement a "clz" function similar to what
log_2_floor() does in util. We could use it both for log_2_floor()
and ip_count_cidr_bits(). That way we would only have to deal with
the ugly GNUC ifdefing in one place.
Seems fine otherwise.
Ethan
On Tue, Aug 16, 2011 at 16:29, Ben Pfaff <b...@nicira.com> wrote:
> We had these functions scattered around the source tree anyway. packets.h
> is a good place to centralize them.
>
> I do plan to introduce some additional callers.
> ---
> lib/classifier.c | 22 ++---------------
> lib/ofp-util.c | 16 ++----------
> lib/packets.c | 65
> +++++++++++++++++++++++++++++++++++++++++++++++++-----
> lib/packets.h | 4 +++
> 4 files changed, 69 insertions(+), 38 deletions(-)
>
> diff --git a/lib/classifier.c b/lib/classifier.c
> index d1f9d5d..9c71b85 100644
> --- a/lib/classifier.c
> +++ b/lib/classifier.c
> @@ -421,15 +421,8 @@ format_ip_netmask(struct ds *s, const char *name,
> ovs_be32 ip,
> ovs_be32 netmask)
> {
> if (netmask) {
> - ds_put_format(s, "%s="IP_FMT, name, IP_ARGS(&ip));
> - if (netmask != htonl(UINT32_MAX)) {
> - if (ip_is_cidr(netmask)) {
> - int wcbits = ofputil_netmask_to_wcbits(netmask);
> - ds_put_format(s, "/%d", 32 - wcbits);
> - } else {
> - ds_put_format(s, "/"IP_FMT, IP_ARGS(&netmask));
> - }
> - }
> + ds_put_format(s, "%s=", name);
> + ip_format_masked(ip, netmask, s);
> ds_put_char(s, ',');
> }
> }
> @@ -441,16 +434,7 @@ format_ipv6_netmask(struct ds *s, const char *name,
> {
> if (!ipv6_mask_is_any(netmask)) {
> ds_put_format(s, "%s=", name);
> - print_ipv6_addr(s, addr);
> - if (!ipv6_mask_is_exact(netmask)) {
> - if (ipv6_is_cidr(netmask)) {
> - int cidr_bits = ipv6_count_cidr_bits(netmask);
> - ds_put_format(s, "/%d", cidr_bits);
> - } else {
> - ds_put_char(s, '/');
> - print_ipv6_addr(s, netmask);
> - }
> - }
> + print_ipv6_masked(s, addr, netmask);
> ds_put_char(s, ',');
> }
> }
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index b0e7405..cdb12e6 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -59,22 +59,12 @@ ofputil_wcbits_to_netmask(int wcbits)
> }
>
> /* Given the IP netmask 'netmask', returns the number of bits of the IP
> address
> - * that it wildcards. 'netmask' must be a CIDR netmask (see ip_is_cidr()).
> */
> + * that it wildcards, that is, the number of 0-bits in 'netmask'. 'netmask'
> + * must be a CIDR netmask (see ip_is_cidr()). */
> int
> ofputil_netmask_to_wcbits(ovs_be32 netmask)
> {
> - assert(ip_is_cidr(netmask));
> -#if __GNUC__ >= 4
> - return netmask == htonl(0) ? 32 : __builtin_ctz(ntohl(netmask));
> -#else
> - int wcbits;
> -
> - for (wcbits = 32; netmask; wcbits--) {
> - netmask &= netmask - 1;
> - }
> -
> - return wcbits;
> -#endif
> + return 32 - ip_count_cidr_bits(netmask);
> }
>
> /* A list of the FWW_* and OFPFW_ bits that have the same value, meaning, and
> diff --git a/lib/packets.c b/lib/packets.c
> index e05e3eb..e7c53a8 100644
> --- a/lib/packets.c
> +++ b/lib/packets.c
> @@ -105,6 +105,40 @@ eth_set_vlan_tci(struct ofpbuf *packet, ovs_be16 tci)
> packet->l2 = packet->data;
> }
>
> +/* Given the IP netmask 'netmask', returns the number of bits of the IP
> address
> + * that it specifies, that is, the number of 1-bits in 'netmask'. 'netmask'
> + * must be a CIDR netmask (see ip_is_cidr()). */
> +int
> +ip_count_cidr_bits(ovs_be32 netmask)
> +{
> + assert(ip_is_cidr(netmask));
> +#if __GNUC__ >= 4
> + return netmask == htonl(0) ? 0 : 32 - __builtin_ctz(ntohl(netmask));
> +#else
> + int one_bits;
> +
> + for (one_bits = 0; netmask; one_bits++) {
> + netmask &= netmask - 1;
> + }
> +
> + return one_bits;
> +#endif
> +}
> +
> +void
> +ip_format_masked(ovs_be32 ip, ovs_be32 mask, struct ds *s)
> +{
> + ds_put_format(s, IP_FMT, IP_ARGS(&ip));
> + if (mask != htonl(UINT32_MAX)) {
> + if (ip_is_cidr(mask)) {
> + ds_put_format(s, "/%d", ip_count_cidr_bits(mask));
> + } else {
> + ds_put_format(s, "/"IP_FMT, IP_ARGS(&mask));
> + }
> + }
> +}
> +
> +
> /* Stores the string representation of the IPv6 address 'addr' into the
> * character array 'addr_str', which must be at least INET6_ADDRSTRLEN
> * bytes long. */
> @@ -117,10 +151,29 @@ format_ipv6_addr(char *addr_str, const struct in6_addr
> *addr)
> void
> print_ipv6_addr(struct ds *string, const struct in6_addr *addr)
> {
> - char addr_str[INET6_ADDRSTRLEN];
> + char *dst;
> +
> + ds_reserve(string, string->length + INET6_ADDRSTRLEN);
> +
> + dst = string->string + string->length;
> + format_ipv6_addr(dst, addr);
> + string->length += strlen(dst);
> +}
>
> - format_ipv6_addr(addr_str, addr);
> - ds_put_format(string, "%s", addr_str);
> +void
> +print_ipv6_masked(struct ds *s, const struct in6_addr *addr,
> + const struct in6_addr *mask)
> +{
> + print_ipv6_addr(s, addr);
> + if (mask && !ipv6_mask_is_exact(mask)) {
> + if (ipv6_is_cidr(mask)) {
> + int cidr_bits = ipv6_count_cidr_bits(mask);
> + ds_put_format(s, "/%d", cidr_bits);
> + } else {
> + ds_put_char(s, '/');
> + print_ipv6_addr(s, mask);
> + }
> + }
> }
>
> struct in6_addr ipv6_addr_bitand(const struct in6_addr *a,
> @@ -164,9 +217,9 @@ ipv6_create_mask(int mask)
> return netmask;
> }
>
> -/* Given the IPv6 netmask 'netmask', returns the number of bits of the
> - * IPv6 address that it wildcards. 'netmask' must be a CIDR netmask (see
> - * ipv6_is_cidr()). */
> +/* Given the IPv6 netmask 'netmask', returns the number of bits of the IPv6
> + * address that it specifies, that is, the number of 1-bits in 'netmask'.
> + * 'netmask' must be a CIDR netmask (see ipv6_is_cidr()). */
> int
> ipv6_count_cidr_bits(const struct in6_addr *netmask)
> {
> diff --git a/lib/packets.h b/lib/packets.h
> index a389e6a..304cd9d 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -291,6 +291,8 @@ ip_is_cidr(ovs_be32 netmask)
> uint32_t x = ~ntohl(netmask);
> return !(x & (x + 1));
> }
> +int ip_count_cidr_bits(ovs_be32 netmask);
> +void ip_format_masked(ovs_be32 ip, ovs_be32 mask, struct ds *);
>
> #define IP_VER(ip_ihl_ver) ((ip_ihl_ver) >> 4)
> #define IP_IHL(ip_ihl_ver) ((ip_ihl_ver) & 15)
> @@ -423,6 +425,8 @@ static inline bool ipv6_mask_is_exact(const struct
> in6_addr *mask) {
>
> void format_ipv6_addr(char *addr_str, const struct in6_addr *addr);
> void print_ipv6_addr(struct ds *string, const struct in6_addr *addr);
> +void print_ipv6_masked(struct ds *string, const struct in6_addr *addr,
> + const struct in6_addr *mask);
> struct in6_addr ipv6_addr_bitand(const struct in6_addr *src,
> const struct in6_addr *mask);
> struct in6_addr ipv6_create_mask(int mask);
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
