[radvd-devel-l] RDNSS Option

[Mickaël Guérin]

Hi

I'm still working on some modifications but i'll give you answers (in
the mail, that's bad ;) )

I'll put the new version of the patch later (when it's finished ;))

Mickaël.


Pekka Savola a écrit :

> Hi,
>
> Thanks!
>
> On Mon, 6 Mar 2006, Mickaël Guérin wrote:
>
>> I've written a page to summarize what has been done (still under
>> construction) : http://withinthebox.net/ipv6/
>>
>> We have developped a GNU/Linux client but the current version is still
>> 'dirty'.
>
>
> A few comments on the patch on the web page, without compile or
> run-testing at this point:
>
> Generic comments:
>  - at some point, radvdump parser should be included

i'm working on it

>  - you don't seem to implement Lifetime bounding
>    between {Min,Max}RtrAdvInterval.  The default value for
>    AdvRDNSSLifetime should maybe be taken from there.

ok.

>  - I hope the draft gets approved in the IETF before we ship the code,
>    but you never know how long it takes...
>
> A few specific ones:
>
> +/* pref/flag/reserved field : yyyyx00000000000 (big endian) -
> 00000000yyyyx000 (little indian); where yyyy = pref, x = flag */
> +#if BYTE_ORDER == BIG_ENDIAN
> +#define ND_OPT_RDNSSI_PREF_SHIFT    12
> +#else
> +#define ND_OPT_RDNSSI_PREF_SHIFT    4
> +#endif
> +#define ND_OPT_RDNSSI_PREF_MASK        (0xff <<
> ND_OPT_RDNSSI_PREF_SHIFT)
>
> ==> does this work OK in a little-endian machine if one of the 8
> high-order zero bits would, at some later date, get used?  Should you
> use '0xf' instead of '0xff' ?

you're right :)

>
> +            rdnss_init_defaults(rdnss, iface);
> +            rdnss->AdvRDNSSNumber = 1;
>
> ==> is there a reason for overwriting the number of RDNSS's?

no reason, it has been changed.

>
> +    rdnss->AdvSFlag = DFLT_AdvSFlag;
>
> ==> AdvSFlag needs to be better named, as it's specific to RDNSS,
> maybe AdvRDNSSSFlag or AdvRDNSSOpenFlag ?  Especially in the actual
> configuration syntax-- "AdvS" isn't good, maybe "AdvRDNSSOpen" or
> something....

renamed to "AdvRDNSSOpen"

>
> +RDNSS (Recursive DNS server) definitions are of the form:
> +
> +.nf
> +.BR "rdnss " "ip " {
> +    list of rdnss specific options
> +.B };
> +.fi
>
> ==> If I understand correctly, the syntax is:
>
> RDNSS ip1 [ip2] [ip3] {
>  ...
> }
>
> or something?  And there can be only one RDNSS definition under
> an interface's configuration.   Is this true?

Maybe should i explain this in the documentation :)
you can specify only 1 IP per rdnss section, but you could put many
rdnss sections. (dns servers are grouped during the parsing of the
configuration file depending if the options allow it).

>
> +.BR "AdvS " on | off;
> +
> +"Service Open" flag. When set, it indicates that RDNSS can be
> available for IPv6 hosts which are located in the different network
> with the RDNSS.
> +
> +Default: on
>
> ==> the default value should be disabled.

done.

>
> +# NOTE: This feature is still a draft has no IANA number type for the
> moment
> +#
> +    rdnss 2001::1
>
> ==> the option name should probably be in upper-case, should use
> addresses under 2001:db8::/32, and should include multiple servers.

the address example was bad, you're right. i let it in uppercase to do
like the other options, but as it's an abbreviation, maybe should it be
in uppercase.

>
> +        // TODO : regrouper en fonction de pref lifetime et S, au
> plus 3 RDNSS
>
> ==> excuse my french... :)

oups, i forgot to delete it :)

>
> +.PP
> +J. Jeong, L. Beloeil, and S. Madanapalli, "IPv6 Router Advertisement
> Option for DNS Configuration", Draft ietf.
>
> ==> please add the draft name, version, etc.
>
that's done.