diff mbox

[ovs-dev,ovn-ipv6,17/26] packets: Introduce xor and is_zero functions on IPv6 addresses.

Message ID 1468306616-125783-18-git-send-email-jpettit@ovn.org
State Changes Requested
Headers show

Commit Message

Justin Pettit July 12, 2016, 6:56 a.m. UTC 
These will have callers later.

Signed-off-by: Justin Pettit <jpettit@ovn.org>
---
 lib/packets.c | 38 ++++++++++++++++++++++++++++++++++++++
 lib/packets.h |  3 +++
 2 files changed, 41 insertions(+)

Comments

Ben Pfaff July 13, 2016, 7:48 p.m. UTC | #1 
On Mon, Jul 11, 2016 at 11:56:47PM -0700, Justin Pettit wrote:
> These will have callers later.
> 
> Signed-off-by: Justin Pettit <jpettit@ovn.org>

>  lib/packets.c | 38 ++++++++++++++++++++++++++++++++++++++
>  lib/packets.h |  3 +++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/lib/packets.c b/lib/packets.c
> index 9e4d0e7..9b34961 100644
> --- a/lib/packets.c
> +++ b/lib/packets.c
> @@ -693,6 +693,44 @@ struct in6_addr ipv6_addr_bitand(const struct in6_addr *a,
>      return dst;
>  }
>  

We usually put the return type on a separate line (here and a second
time later);

> +struct in6_addr ipv6_addr_bitxor(const struct in6_addr *a,
> +                                 const struct in6_addr *b)
> +{

I tend to declare loop variables inside the loop these days:
> +    int i;
> +    struct in6_addr dst;
> +
> +#ifdef s6_addr32
> +    for (i=0; i<4; i++) {
> +        dst.s6_addr32[i] = a->s6_addr32[i] ^ b->s6_addr32[i];
> +    }
> +#else
> +    for (i=0; i<16; i++) {
> +        dst.s6_addr[i] = a->s6_addr[i] ^ b->s6_addr[i];
> +    }
> +#endif
> +
> +    return dst;
> +}
> +
> +bool ipv6_is_zero(const struct in6_addr *a)
> +{
> +#ifdef s6_addr32
> +    for (int i = 0; i < 4; i++) {
> +        if (a->s6_addr32[i]) {
> +            return false;
> +        }
> +    }
> +#else
> +    for (int i = 0; i < 16; i++) {
> +        if (a->s6_addr[i]) {
> +            return false;
> +        }
> +    }
> +#endif
> +
> +    return true;
> +}

But what if we do the whole thing this way?

#ifdef s6_addr32
#define s6_addrX s6_addr32
#define IPV6_FOR_EACH(VAR) for (int VAR = 0; VAR < 4; VAR++)
#else
#define s6_addrX s6_addr
#define IPV6_FOR_EACH(VAR) for (int VAR = 0; VAR < 16; VAR++)
#endif

struct in6_addr
ipv6_addr_bitand(const struct in6_addr *a, const struct in6_addr *b)
{
    struct in6_addr dst;
    IPV6_FOR_EACH (i) {
        dst.s6_addrX[i] = a->s6_addrX[i] & b->s6_addrX[i];
    }
    return dst;
}

struct in6_addr
ipv6_addr_bitxor(const struct in6_addr *a, const struct in6_addr *b)
{
    struct in6_addr dst;
    IPV6_FOR_EACH (i) {
        dst.s6_addrX[i] = a->s6_addrX[i] ^ b->s6_addrX[i];
    }
    return dst;
}

bool
ipv6_is_zero(const struct in6_addr *a)
{
    IPV6_FOR_EACH (i) {
        if (a->s6_addrX[i]) {
            return false;
        }
    }
    return true;
}

or

#ifdef s6_addr32
#define s6_addrX s6_addr32
#define s6_n_addrX 4
#else
#define s6_addrX s6_addr
#define s6_n_addrX 16
#endif

struct in6_addr
ipv6_addr_bitand(const struct in6_addr *a, const struct in6_addr *b)
{
    struct in6_addr dst;
    for (int i = 0; i < s6_n_addrX; i++) {
        dst.s6_addrX[i] = a->s6_addrX[i] & b->s6_addrX[i];
    }
    return dst;
}

struct in6_addr
ipv6_addr_bitxor(const struct in6_addr *a, const struct in6_addr *b)
{
    struct in6_addr dst;
    for (int i = 0; i < s6_n_addrX; i++) {
        dst.s6_addrX[i] = a->s6_addrX[i] ^ b->s6_addrX[i];
    }
    return dst;
}

bool
ipv6_is_zero(const struct in6_addr *a)
{
    for (int i = 0; i < s6_n_addrX; i++) {
        if (a->s6_addrX[i]) {
            return false;
        }
    }
    return true;
}
Ben Pfaff July 19, 2016, 11:52 p.m. UTC | #2 
On Wed, Jul 13, 2016 at 12:48:28PM -0700, Ben Pfaff wrote:
> On Mon, Jul 11, 2016 at 11:56:47PM -0700, Justin Pettit wrote:
> > These will have callers later.
> > 
> > Signed-off-by: Justin Pettit <jpettit@ovn.org>
> 
> >  lib/packets.c | 38 ++++++++++++++++++++++++++++++++++++++
> >  lib/packets.h |  3 +++
> >  2 files changed, 41 insertions(+)
> > 
> > diff --git a/lib/packets.c b/lib/packets.c
> > index 9e4d0e7..9b34961 100644
> > --- a/lib/packets.c
> > +++ b/lib/packets.c
> > @@ -693,6 +693,44 @@ struct in6_addr ipv6_addr_bitand(const struct in6_addr *a,
> >      return dst;
> >  }
> >  
> 
> We usually put the return type on a separate line (here and a second
> time later);
> 
> > +struct in6_addr ipv6_addr_bitxor(const struct in6_addr *a,
> > +                                 const struct in6_addr *b)
> > +{
> 
> I tend to declare loop variables inside the loop these days:
> > +    int i;
> > +    struct in6_addr dst;
> > +
> > +#ifdef s6_addr32
> > +    for (i=0; i<4; i++) {
> > +        dst.s6_addr32[i] = a->s6_addr32[i] ^ b->s6_addr32[i];
> > +    }
> > +#else
> > +    for (i=0; i<16; i++) {
> > +        dst.s6_addr[i] = a->s6_addr[i] ^ b->s6_addr[i];
> > +    }
> > +#endif
> > +
> > +    return dst;
> > +}
> > +
> > +bool ipv6_is_zero(const struct in6_addr *a)
> > +{
> > +#ifdef s6_addr32
> > +    for (int i = 0; i < 4; i++) {
> > +        if (a->s6_addr32[i]) {
> > +            return false;
> > +        }
> > +    }
> > +#else
> > +    for (int i = 0; i < 16; i++) {
> > +        if (a->s6_addr[i]) {
> > +            return false;
> > +        }
> > +    }
> > +#endif
> > +
> > +    return true;
> > +}
> 
> But what if we do the whole thing this way?
> 
> #ifdef s6_addr32
> #define s6_addrX s6_addr32
> #define IPV6_FOR_EACH(VAR) for (int VAR = 0; VAR < 4; VAR++)
> #else
> #define s6_addrX s6_addr
> #define IPV6_FOR_EACH(VAR) for (int VAR = 0; VAR < 16; VAR++)
> #endif
> 
> struct in6_addr
> ipv6_addr_bitand(const struct in6_addr *a, const struct in6_addr *b)
> {
>     struct in6_addr dst;
>     IPV6_FOR_EACH (i) {
>         dst.s6_addrX[i] = a->s6_addrX[i] & b->s6_addrX[i];
>     }
>     return dst;
> }
> 
> struct in6_addr
> ipv6_addr_bitxor(const struct in6_addr *a, const struct in6_addr *b)
> {
>     struct in6_addr dst;
>     IPV6_FOR_EACH (i) {
>         dst.s6_addrX[i] = a->s6_addrX[i] ^ b->s6_addrX[i];
>     }
>     return dst;
> }
> 
> bool
> ipv6_is_zero(const struct in6_addr *a)
> {
>     IPV6_FOR_EACH (i) {
>         if (a->s6_addrX[i]) {
>             return false;
>         }
>     }
>     return true;
> }
> 
> or
> 
> #ifdef s6_addr32
> #define s6_addrX s6_addr32
> #define s6_n_addrX 4
> #else
> #define s6_addrX s6_addr
> #define s6_n_addrX 16
> #endif
> 
> struct in6_addr
> ipv6_addr_bitand(const struct in6_addr *a, const struct in6_addr *b)
> {
>     struct in6_addr dst;
>     for (int i = 0; i < s6_n_addrX; i++) {
>         dst.s6_addrX[i] = a->s6_addrX[i] & b->s6_addrX[i];
>     }
>     return dst;
> }
> 
> struct in6_addr
> ipv6_addr_bitxor(const struct in6_addr *a, const struct in6_addr *b)
> {
>     struct in6_addr dst;
>     for (int i = 0; i < s6_n_addrX; i++) {
>         dst.s6_addrX[i] = a->s6_addrX[i] ^ b->s6_addrX[i];
>     }
>     return dst;
> }
> 
> bool
> ipv6_is_zero(const struct in6_addr *a)
> {
>     for (int i = 0; i < s6_n_addrX; i++) {
>         if (a->s6_addrX[i]) {
>             return false;
>         }
>     }
>     return true;
> }

Acked-by: Ben Pfaff <blp@ovn.org>
Justin Pettit July 19, 2016, 11:59 p.m. UTC | #3 
> On Jul 19, 2016, at 4:52 PM, Ben Pfaff <blp@ovn.org> wrote:
> 
>> But what if we do the whole thing this way?
>> 
>> #ifdef s6_addr32
>> #define s6_addrX s6_addr32
>> #define IPV6_FOR_EACH(VAR) for (int VAR = 0; VAR < 4; VAR++)
>> #else
>> #define s6_addrX s6_addr
>> #define IPV6_FOR_EACH(VAR) for (int VAR = 0; VAR < 16; VAR++)
>> #endif
>> 
>> struct in6_addr
>> ipv6_addr_bitand(const struct in6_addr *a, const struct in6_addr *b)
>> {
>>    struct in6_addr dst;
>>    IPV6_FOR_EACH (i) {
>>        dst.s6_addrX[i] = a->s6_addrX[i] & b->s6_addrX[i];
>>    }
>>    return dst;
>> }
>> 
>> struct in6_addr
>> ipv6_addr_bitxor(const struct in6_addr *a, const struct in6_addr *b)
>> {
>>    struct in6_addr dst;
>>    IPV6_FOR_EACH (i) {
>>        dst.s6_addrX[i] = a->s6_addrX[i] ^ b->s6_addrX[i];
>>    }
>>    return dst;
>> }
>> 
>> bool
>> ipv6_is_zero(const struct in6_addr *a)
>> {
>>    IPV6_FOR_EACH (i) {
>>        if (a->s6_addrX[i]) {
>>            return false;
>>        }
>>    }
>>    return true;
>> }
>> 
>> 
> 
> Acked-by: Ben Pfaff <blp@ovn.org>

I agree this is cleaner.  I've switched to the above method and added you to the author list.

Thanks!

--Justin
diff mbox

Patch

diff --git a/lib/packets.c b/lib/packets.c
index 9e4d0e7..9b34961 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -693,6 +693,44 @@  struct in6_addr ipv6_addr_bitand(const struct in6_addr *a,
     return dst;
 }
 
+struct in6_addr ipv6_addr_bitxor(const struct in6_addr *a,
+                                 const struct in6_addr *b)
+{
+    int i;
+    struct in6_addr dst;
+
+#ifdef s6_addr32
+    for (i=0; i<4; i++) {
+        dst.s6_addr32[i] = a->s6_addr32[i] ^ b->s6_addr32[i];
+    }
+#else
+    for (i=0; i<16; i++) {
+        dst.s6_addr[i] = a->s6_addr[i] ^ b->s6_addr[i];
+    }
+#endif
+
+    return dst;
+}
+
+bool ipv6_is_zero(const struct in6_addr *a)
+{
+#ifdef s6_addr32
+    for (int i = 0; i < 4; i++) {
+        if (a->s6_addr32[i]) {
+            return false;
+        }
+    }
+#else
+    for (int i = 0; i < 16; i++) {
+        if (a->s6_addr[i]) {
+            return false;
+        }
+    }
+#endif
+
+    return true;
+}
+
 /* Returns an in6_addr consisting of 'mask' high-order 1-bits and 128-N
  * low-order 0-bits. */
 struct in6_addr
diff --git a/lib/packets.h b/lib/packets.h
index 06166de..2043175 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1027,6 +1027,9 @@  void ipv6_format_masked(const struct in6_addr *addr,
 const char * ipv6_string_mapped(char *addr_str, const struct in6_addr *addr);
 struct in6_addr ipv6_addr_bitand(const struct in6_addr *src,
                                  const struct in6_addr *mask);
+struct in6_addr ipv6_addr_bitxor(const struct in6_addr *a,
+                                 const struct in6_addr *b);
+bool ipv6_is_zero(const struct in6_addr *a);
 struct in6_addr ipv6_create_mask(int mask);
 int ipv6_count_cidr_bits(const struct in6_addr *netmask);
 bool ipv6_is_cidr(const struct in6_addr *netmask);