> On Jul 12, 2016, at 8:23 PM, Ben Pfaff <b...@ovn.org> wrote:
> 
> On Mon, Jul 11, 2016 at 11:56:43PM -0700, Justin Pettit wrote:
>> Extract port security and logical switch port addresses once and store
>> them as part of the ovn_port structure.  Use the string representations
>> from the extracted addresses.
>> 
>> Signed-off-by: Justin Pettit <jpet...@ovn.org>
> 
> In build_port_security_l2(), you can combine the following two calls
> into one:
>> +        ds_put_format(match, "%s", ps_addrs[i].ea_s);
>> +        ds_put_char(match, ' ');

Yes.  :-)

> In build_port_security_nd(), this can use ds_put_cstr():
> +                        ds_put_format(&match, "%s", 
> ps->ipv4_addrs[i].addr_s);

Done.

> This bit of build_lswitch_flows() would probably benefit from {...}
> notation:
>                if (op->lsp_addrs[i].n_ipv6_addrs == 1) {
>                    ds_put_format(&match, "nd.target == %s",
>                                  op->lsp_addrs[i].ipv6_addrs[0].addr_s);
>                } else {
>                    ds_put_cstr(&match, "(");
>                    for (size_t j = 0; j < op->lsp_addrs[i].n_ipv6_addrs; j++) 
> {
>                        ds_put_format(&match, "nd.target == %s || ",
>                                      op->lsp_addrs[i].ipv6_addrs[j].addr_s);
>                    }
>                    ds_chomp(&match, ' ');
>                    ds_chomp(&match, '|');
>                    ds_chomp(&match, '|');
>                    ds_chomp(&match, ' ');
>                    ds_put_cstr(&match, ")");
>                }

Yes, it's cleaner that way.  Done.

> Does anything in this series rename 'json_key' to 'key_s'?  It may be
> the consistent thing to do.

No.  I've added it to my todo list.

> Acked-by: Ben Pfaff <b...@ovn.org>

Thanks,

--Justin


_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to