Thanks for v2.

I often hear of LLDP used in connection with OpenFlow controllers and
switches.  Until now, Open vSwitch has had no built-in support for
LLDP, although support could be implemented through an OpenFlow
controller.  This patch series does start adding built-in LLDP
support.  I am not sure how this will interact with what users are
doing through OpenFlow already.  Do you have any thoughts?  I would
know a little more, if the LLDP support were documented, but
vswitch.xml in the final patch is incomplete: it does not document the
new "lldp" column (or "enabled" or "authentication_key" or
"mappings").

It seems to me that most of the lldp code added in the first patch is
really quite separate from the OVS code and accessed through a narrow
interface (ovs-lldp.[ch]).  I think maybe the library code should go
into a subdirectory of lib, e.g. lib/lldp, with only ovs-lldp.[ch]
inside lib/ directly.  (The existing sflow code is another good
candidate for this.)

Some of the files in patches 2 and 3 see to have inconsistent
indentation.  I think that they are using tabs to indent some lines
and spaces to indent others.  Open vSwitch source files should be
indented with spaces.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to