[ovs-dev] [PATCH 2/3] auto-attach: Add auto-attach support to ofproto layer

[Ben Pfaff]

On Fri, Nov 14, 2014 at 07:42:08AM -0500, drflynn at avaya.com wrote:
> From: Dennis Flynn <drflynn at avaya.com>
> 
> Signed-off-by: Ludovic Beliveau <ludovic.beliveau at windriver.com>
> Signed-off-by: Dennis Flynn <drflynn at avaya.com>

Thanks for the second version!

In monitor_mport_run(), I think it would be better to put the result
of each of the *_wake_time() calls into a temporary variable, rather
than to pass them directly to the MIN macro, because calling these
functions twice is expensive and because they don't necessarily return
exactly the same value two times in a row.  (I realize that this is
partly an existing problem but it's best not to make it worse.)

In ofproto-provider.h, the comment on get_lldp_status talks about a
'status->rmps' member that does not exist.

In ofproto-provider.h there is some inconsistent indentation:

+    /* If 's' is nonnull, this function registers a mapping associated with
+     * client data pointer 'aux' in 'ofproto'.  If 'aux' is already registered
+        * then this function updates its configuration to 's'.  Otherwise, this
+        * function registers a new mapping.
+     *
+     * An implementation that does not support mapping at all may set
+     * it to NULL or return EOPNOTSUPP.  An implementation that supports

and:

+    /* If 's' is nonnull, this function unregisters a mapping associated with
+     * client data pointer 'aux' in 'ofproto'.  If 'aux' is already registered
+        * then this function updates its configuration to 's'.  Otherwise, this
+        * function unregisters a new mapping.
+     *

The comments on aa_get_vlan_queued() and on the member function in
struct ofproto_class should explain what's in the output list.

Thanks,

Ben.