Hi Ben, After looking more carefully at the logic that parses the LLDP_TLV_MGMT_ADDR TLV I agree there is a bug here. This is actually code from the open source LLDPD project.
I have opened an issue within that project describing this problem. In the mean time I have modified this code within OvS to fix this bug and am testing that now. Also I am familiarizing myself with the OvS unit test environment (i.e. Autotest) with the intent of adding new tests for Auto-Attach (AA). At this point I can provide my fix to the LLDP_TLV_MGMT_ADDR problem so that we can move forward with the AA submission. From your perspective do you require that the new AA unit tests be in place before accepting the AA code? Do you have any further insight regarding the "Code Complete" date for OvS 2.4? Regards, Dennis -----Original Message----- From: Flynn, Dennis R (Dennis) Sent: Wednesday, January 14, 2015 11:14 AM To: 'Ben Pfaff' Cc: Keene, Carl (Carl); dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH 0/3] auto-attach: Add support for IETF Auto Attach standard (v4) Ben, Thanks for your continued review of the auto-attach patches and yes, including ovs-dev is fine. We will take a closer look at the logic which handles LLDP_TLV_MGMT_ADDR. We developed unit tests for the new Auto-Attach TLVs in the context of the open source LLDP project on which our code is based. We will look at porting those unit tests to the OvS unit test framework. Dennis -----Original Message----- From: Ben Pfaff [mailto:b...@nicira.com] Sent: Tuesday, January 13, 2015 6:36 PM To: Flynn, Dennis R (Dennis) Cc: Keene, Carl (Carl); dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH 0/3] auto-attach: Add support for IETF Auto Attach standard (v4) [adding ovs-dev back, hope that's OK] On Tue, Jan 13, 2015 at 09:19:53PM +0000, Flynn, Dennis R (Dennis) wrote: > Just wondering if you have been able to make any progress on this. > We are very excited about the prospects of this being included in OvS > 2.4 One place I've become a little hung up is parsing LLDP_TLV_MGMT_ADDR. I think that there's a bug in the length checking: I believe that CHECK_TLV_SIZE checks the total size of the attribute's value, but I think that the code here is passing the length of only the part of the TLV that it hasn't already passed through. If you could take a look at that then I'd appreciate it. Another thing that worries me a little is that there aren't any unit tests for LLDP decoding. Protocol parsing is always a little tricky to get right and it's always reassuring to have some tests for it. Thanks, Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
