[ovs-dev,ovn-ipv6,20/26] packets: Add in6_is_lla() function.
diff mbox

Message ID FE079AEA-BD5C-4E61-93AC-8F3D22D7F97C@ovn.org
State Superseded
Headers show

Commit Message

Justin Pettit July 20, 2016, 5:16 a.m. UTC
> On Jul 13, 2016, at 12:57 PM, Ben Pfaff <blp@ovn.org> wrote:
> 
> On Mon, Jul 11, 2016 at 11:56:50PM -0700, Justin Pettit wrote:
>> This will have a caller in a future commit.
>> 
>> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> 
>> +static inline bool
>> +in6_is_lla(struct in6_addr *addr)
>> +{
>> +    return addr->s6_addr32[0] == htonl(0xfe800000) && !(addr->s6_addr32[1]);
>> +}
> 
> One more thing: not all implementations have s6_addr32, so we probably
> need a fallback.

Good point.  I've addressed that as well as your previous comments.

--Justin


-=-=-=-=-=-=-=-=-=-

Comments

Ben Pfaff July 20, 2016, 5:49 a.m. UTC | #1
On Tue, Jul 19, 2016 at 10:16:12PM -0700, Justin Pettit wrote:
> 
> > On Jul 13, 2016, at 12:57 PM, Ben Pfaff <blp@ovn.org> wrote:
> > 
> > On Mon, Jul 11, 2016 at 11:56:50PM -0700, Justin Pettit wrote:
> >> This will have a caller in a future commit.
> >> 
> >> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> > 
> >> +static inline bool
> >> +in6_is_lla(struct in6_addr *addr)
> >> +{
> >> +    return addr->s6_addr32[0] == htonl(0xfe800000) && !(addr->s6_addr32[1]);
> >> +}
> > 
> > One more thing: not all implementations have s6_addr32, so we probably
> > need a fallback.
> 
> Good point.  I've addressed that as well as your previous comments.
> 
> --Justin
> 
> 
> -=-=-=-=-=-=-=-=-=-
> 
> diff --git a/lib/packets.h b/lib/packets.h
> index 02028c9..8f11e2c 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -980,10 +980,16 @@ in6_generate_lla(struct eth_addr ea, struct in6_addr *lla)
>      taddr->be16[7] = ea.be16[2];
>  }
>  
> +/* Returns true if 'addr' is a link local address.  Otherwise, false. */
>  static inline bool
>  in6_is_lla(struct in6_addr *addr)
>  {
> +#ifdef s6_addr32
>      return addr->s6_addr32[0] == htonl(0xfe800000) && !(addr->s6_addr32[1]);
> +#else
> +    return addr->s6_addr[0] == htons(0xfe80) &&
> +         !(addr->s6_addr[1] | addr->s6_addr[2] | addr->s6_addr[3]);
> +#endif
>  }

Isn't s6_addr[] an array of bytes?  I don't think that comparing against
htons(...) is going to do anything sensible.
Justin Pettit July 20, 2016, 5:55 a.m. UTC | #2
> On Jul 19, 2016, at 10:49 PM, Ben Pfaff <blp@ovn.org> wrote:
> 
> On Tue, Jul 19, 2016 at 10:16:12PM -0700, Justin Pettit wrote:
>> 
>> diff --git a/lib/packets.h b/lib/packets.h
>> index 02028c9..8f11e2c 100644
>> --- a/lib/packets.h
>> +++ b/lib/packets.h
>> @@ -980,10 +980,16 @@ in6_generate_lla(struct eth_addr ea, struct in6_addr *lla)
>>     taddr->be16[7] = ea.be16[2];
>> }
>> 
>> +/* Returns true if 'addr' is a link local address.  Otherwise, false. */
>> static inline bool
>> in6_is_lla(struct in6_addr *addr)
>> {
>> +#ifdef s6_addr32
>>     return addr->s6_addr32[0] == htonl(0xfe800000) && !(addr->s6_addr32[1]);
>> +#else
>> +    return addr->s6_addr[0] == htons(0xfe80) &&
>> +         !(addr->s6_addr[1] | addr->s6_addr[2] | addr->s6_addr[3]);
>> +#endif
>> }
> 
> Isn't s6_addr[] an array of bytes?  I don't think that comparing against
> htons(...) is going to do anything sensible.

Ah, crap.  I made a bad assumption, and then it build cleanly when I forced it to take that code, so I thought I was good.

I'll send out a revised patch.

--Justin

Patch
diff mbox

diff --git a/lib/packets.h b/lib/packets.h
index 02028c9..8f11e2c 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -980,10 +980,16 @@  in6_generate_lla(struct eth_addr ea, struct in6_addr *lla)
     taddr->be16[7] = ea.be16[2];
 }
 
+/* Returns true if 'addr' is a link local address.  Otherwise, false. */
 static inline bool
 in6_is_lla(struct in6_addr *addr)
 {
+#ifdef s6_addr32
     return addr->s6_addr32[0] == htonl(0xfe800000) && !(addr->s6_addr32[1]);
+#else
+    return addr->s6_addr[0] == htons(0xfe80) &&
+         !(addr->s6_addr[1] | addr->s6_addr[2] | addr->s6_addr[3]);
+#endif
 }
 
 static inline void