On Tue, Mar 15, 2011 at 04:03:23PM -0700, Ethan Jackson wrote:
> When a slave may no longer participate in a LACP bond, it is polite
> to transmit a notification indicating so.  This will allow the
> remote partner to adapt more quickly.  The specification does not
> indicate what to do in this situation.  This patch emulates the
> Linux bonding stack.

Now that I look at the paths along which lacp_slave_unregister() is
called more carefully, I don't think that it is safe to assume that
slave_lookup() will always return nonnull here.  I think that it
should just return immediately in that case.  (That needs to be
applied to patch 7, I think.)

The slave_lookup() at the end of lacp_slave_unregister() is now
redundant, I think.  I think that the last line could just be
slave_destroy(slave);

I don't think that it's safe to call the callback unconditionally
here.  Just after the call to lacp_slave_unregister() in
iface_destroy() is an 'if' statement that checks whether netdev is
nonnull.  But lacp_send_pdu_cb() does not check whether netdev is
nonnull.

It might be better to just have lacp_slave_unregister() initialize a
pdu passed in by the caller, who can then do with it whatever it
wants.

Thanks,

Ben.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to