[ovs-dev] [IPv6 v2 10/10] ovn: Add support for IPv6 dynamic bindings.

Justin Pettit jpettit at ovn.org
Mon Aug 1 05:23:19 UTC 2016


> On Aug 1, 2016, at 12:18 AM, Ben Pfaff <blp at ovn.org> wrote:
> 
> On Mon, Aug 01, 2016 at 12:13:45AM -0500, Justin Pettit wrote:
>> 
>>> On Jul 29, 2016, at 12:53 PM, Ben Pfaff <blp at ovn.org> wrote:
>>> 
>>> On Thu, Jul 28, 2016 at 11:26:20PM -0700, Justin Pettit wrote:
>>>> This commit also introduces "get_nd" and "put_nd" logical actions.
>>>> 
>>>> Signed-off-by: Justin Pettit <jpettit at ovn.org>
>>> 
>>> struct put_mac_binding might include a string buffer directly; not sure
>>> there's value in the extra allocation here.
>> 
>> Agreed.
>> 
>>> In pinctrl_handle_put_mac_binding(), the parentheses around the call
>>> to hash_2words() look funny to me here:
>>> +    uint32_t hash = hash_string(ip_s, (hash_2words(dp_key, port_key)));
>> 
>> Must be from all that Lisp programming I do on the side.
>> 
>>> s/Solictation/Solicitation/ in ovn-northd.8.xml.
>> 
>> Argh.  I knew I was going to do that somewhere.
>> 
>> I've added an incremental in case you're interested.
> 
> ...
> 
>> @@ -605,7 +605,7 @@ pinctrl_handle_put_mac_binding(const struct flow *md,
>>         hmap_insert(&put_mac_bindings, &pmb->hmap_node, hash);
>>         pmb->dp_key = dp_key;
>>         pmb->port_key = port_key;
>> -        pmb->ip_s = xstrdup(ip_s);
>> +        memcpy(pmb->ip_s, ip_s, sizeof pmb->ip_s);
> 
> This is a memcpy of a string?  Why not ovs_strlcpy?

Because I'm tired.  :-)

> Acked-by: Ben Pfaff <blp at ovn.org>

Thanks for all the reviews.  I'll merge this unless you spot something else bone-headed in the next few minutes.

--Justin


diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index f0faa85..bd685fe 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -605,7 +605,7 @@ pinctrl_handle_put_mac_binding(const struct flow *md,
         hmap_insert(&put_mac_bindings, &pmb->hmap_node, hash);
         pmb->dp_key = dp_key;
         pmb->port_key = port_key;
-        memcpy(pmb->ip_s, ip_s, sizeof pmb->ip_s);
+        ovs_strlcpy(pmb->ip_s, ip_s, sizeof pmb->ip_s);
     }
     pmb->timestamp = time_msec();
     pmb->mac = headers->dl_src;




More information about the dev mailing list