[quagga-dev 10805] Re: RFC-6506(Supporting Authentication Trailer for OSPFv3) implementation in quagga-0.99.21 version

Renato Westphal renatowestphal at gmail.com
Fri Oct 11 16:45:30 BST 2013


2013/10/11 Jyotsna Priya1 <jyotsna.priya1 at tcs.com>:
> Hi Denis/ List,
>
> Thank you for the the last review.
> A new revision of the patch is attached with this mail. The keychain mechanism as done in RIP has been implemented in OSPFv3. As far as sequence number check is concerned, we think that what has been implemented is according to the specs and thus it is not changed in new revision. Other smaller issues like using a common function and using the __func__ macro are considered and resolved.
> We look forward to your response.

Hi Jyotsna,

Your patch has a lot of problems. First of all, RFC 6506 Section 2.1
says: "For OSPFv3 Hello and Database Description packets, the AT-bit
indicates the AT is present. For other OSPFv3 packet types, the OSPFv3
AT-bitsetting from the OSPFv3 Hello/Database Description setting is
preserved in the OSPFv3 neighbor data structure. OSPFv3 packet types
that don't include an OSPFv3 Options field will use the setting from
the neighbor data structure to determine whether or not the AT is
expected."

You clearly missed that part. You are looking for the AT bit in all
received OSPFv3 packets, even the ones that don't have the Options
field. You should look for the AT bit only on 'Hello' and 'Database
Description' packets and store this information on the
'ospf6_neighbor' structure. For other types of OSPFv3 packets, you
should check if authentication is in use by looking for the previously
stored information on the 'ospf6_neighbor' structure.

Other considerations:
* Your patch misses a lot of byte order macros, it won't work at all
on any little endian machine;
* All your references to IP_STR should not be there;
* The changes to the 'memtypes.h' and 'configure' files should not be
in the patch since these are auto-generated files;
* Your code has some stylistic issues like very long lines and
improper indentation.

Other than that the patch looks ok. I hope you can work on the
aforementioned issues, RFC 6506 support would be a great addition to
Quagga.

Next time please send the patch inline, not as an attachment.

[]s

-- 
Renato Westphal




More information about the Quagga-dev mailing list