On Mon, Sep 30, 2013 at 2:09 PM, Ethan Jackson <et...@nicira.com> wrote:
> On the right track, mostly nits at this point.
>
> I'd prefer we simply copied the hw_addr around instead of maintaining
> a pointer to the same data in multiple threads. It's only 6 bytes, so
> it shouldn't be too expensive.
>
I see, I'll memcpy it,
> Let's rename ofproto_dpif_monitor_mport_update() =>
> ofproto_dpif_monitor_port_update(). The rest of the code shouldn't
> need to know anything about mports.
>
That's good point, thanks.
> In xlate_ofport_set() could you just get the hw_addr from the netdev
> instead of passing it as another argument?
>
Yes, I'll modify accordingly,
> We could ditch the update_monitor bool pretty easily by doing
> something like the following:
>
> if (xport->bfd != bfd || xport->cfm != cfm) {
> bfd_unref()
> cfm_unref()
> xport->bfd = bfd
> xport->cfm = cfm
> port_update()
> }
>
> Less variables means less wrong variables.
>
I still think that will make the code ckear. So, I'm keeping it in my
following V3 patch and wait for your feedback.
I think it'd be cleaner if the stats mutex change was pulled into it's
> own separate patch before this one and the previous.
>
I'll use a separate patch,
I don't totally get the unit test changes. Could you explain them?
> Is there some simpler way to achieve what you're going for? Why are
> they needed?
>
As we discussed off list, I'll add explicit explanation, to this.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
