Subject: Re: [dhcwg] WGLC for draft-ietf-dhc-access-network-identifier-02 - respond by April 27, 2014

From: Tomek Mrugalski <[email-masked]>
Date: Fri, 02 May 2014 15:51:25 +0200

On 12.04.2014 16:04, Bernie Volz (volz) wrote:
> Folks, the authors of draft-ietf-dhc-access-network-identifier
> (http://tools.ietf.org/html/draft-ietf-dhc-access-network-identifier-02)
> 
> feel it's ready for work group last call. Please review this draft and
> indicate whether or not you feel it is ready to be published.
While I agree with the reasons behind the draft and I think the idea is
sound, I'm afraid that it is not ready in my opinion.

With my co-chair hat off, I must say that the text is confusing to read
and allows very bad things, like sending DHCPv6 options in DHCPv4
messages (and vice versa), inserting sub-options as regular options and
is very imprecise in many cases (it is not clear which options to insert
where, what to ignore: just a sub-option, whole option or the whole
message etc).

Sorry for sending another long list of comments. I tried to split them
into substantial and editorial.

Substantial comments:

Section 2 (motivation) the text in 3rd paragraph explains that APs
insert acess network identifiers in DHCP messages. It is unclear whether
this is:
a) AP acting as a DHCP client and configures itself
b) AP acting as a DHCP relay agent and relays messages from connected
clients.

I assume you meant b), but this is text is ambiguous and should be
clarified.

I'm uncomfortable with treating both protocols in the same way. That
notion is introduced in section 3 and then continued throughout the
document. That is reflected in many of my comments below.

Section 4 (DHCPv4 Access-Network-Identifier Option) actually redefines
Relay Agent Information option! You can't do that! Please define this
option just for the client (i.e. remove the text "code: if added by
relay agnet: Relay Agent Information Option (82)".

Please consider adding the following text at the end of section 4:
"Client inserts Access Network Identifier option that contains one or
more suboptions, as defined in Section 4.1. Relay agent that wants to
include Access Network Indentifer information includes one or more
sub-options (see Section 4.1) in Relay Agent Information option [RFC3046]."

Please move the paragraph starting with "ANI Sub-options" from section
4. to 4.1 where it belongs.

Please rearrange the text, so it follows this pattern:

4. DHCPv4 options
4.1 DHCPv4 option 1
4.2 DHCPv4 sub-option 1
4.3 DHCPv4 sub-option 2
...

5. DHCPv6 options
5.1 DHCPv6 option 1
5.2 DHCPv6 option 2
5.3 DHCPv6 option 3
...

This layout is better than what you have for several reasons:
1. People interested in only one protocol family can read only one
continuous chunk of the text.
2. It is easier to reference options (by giving their subsection numbers).
3. It is easier to read without constant context switching (in v4 most
defined things are sub-options while in v6 they are regular options).
4. You will need to add extra text in each section, something along the
lines of "DHCPv4 clients insert this sub-option into
Access-Network-Identifier Option. DHCPv4 Relay Agents insert this
sub-option into Relay Agent Information option.". This is actually very
important to NOT have v4 and v6 options mixed here. The current text
allows crazy things, like v4 relay agent inserting v6 options.

I understand that it will require a bit of text duplication, but is much
less of an issue than allowing mixing up v4 and v6 options. If you don't
want to repeat it, use something like "ATT field. For definition of ATT
field, see section X.Y".

Section 7 is completely broken. Please be explicit about protocol
family. In essence, you need to write everything twice.

"DHCPv4 clients MAY include DHCPv4 ANI Option (see Section X.Y)
including one or more sub-options, as defined in sections X.Y through
Z.W. DHCPv4 clients MAY include DHCPv6 ANI Option, as defined in Section
A.B."

Please pay special attention to the wording here. Your text must
distinguish between options and sub-options. Please do not use pronouns.

Let me give you and example how badly broken the current text is: "DHCP
Relay Agents MAY include these options". What does it mean "these" here?
All options? My DHCPv4 relay agent can insert DHCPv6 options or insert
DHCPv4 sub-options directly as options. Your text says that it's ok. It
clearly is not.

It is fine (and actually a good idea) to have separate
client/relay/server behavior sections for DHCPv4 and DHCPv6. It will be
especially useful for relay agents, as those work differently for v4 and v6.

I strongly suggest to take a look at draft-ietf-dhc-option-guidelines
(to be published as RFC7227), section 21. It gives examples on how to
write such client/server/relay agent behavior text.

Section 9 is also very confusing. "If DHCP Server is unable to
understand these options it MUST be ignored." What does the "it" means
in this context? The message or the option? Strictly adhering to English
grammar, "it" would mean "the DHCP server"... Anyway, the server ignores
options it doesn't understand. This is the standard behavior. No need to
use normative language here.

There is no text to explain what to do when both client and relay
include ANI options. If the relay sees that client included ANI options,
should it include them as well? I'm not an expert on mobility or access
networks, but as an operator, I would trust more the relay agent than
the values sent by client. Client can spoof its options in an attempt to
gain better service.

Since you define a new option with sub-options, please make sure to say
what's the server's behavior when receiving an option with some
sub-options not being recognized. Should the server ignore just that one
unrecognized sub-option or the whole option? If it's up to server
administrative policy, please say so in the text.

IANA considerations section uses wrong registry names.
"Bootp and DHCP options" =>
"BOOTP Vendor Extensions and DHCP Options"

"DHCPv6 and DHCPv6 options" =>
I think it should be "Option Codes registry for DHCPv6".

Security considerations section should mention that client can fake its
ANI option values to gain better service.

Many normative references should be informative.

Idnits reports several issues: missing and unused references, too long
lines, non-RFC2606 FQDNs and others. Please make sure that those are
resolved before you upload next version. Idnits is available here:
http://tools.ietf.org/tools/idnits/ you can check your unpublished version.

Editorial comments:

Section 1:
"range of application" => "range of applications".

"For e.g. The local mobility anchor..." => "For example the local
mobility anchor..."

The official protocol name is "Dynamic Host Configuration Protocol for
IPv6" or "DHCPv6", not "Dynamic Host Configuration Protocol v6". That
mistake is repeated in section 3.

Dot is missing at the last sentence in section 1.

Section 2
"various type of network deployments" =>
"various types of network deployments"

Section 3:
"DHCP refers to both DHCPv4 and DHCPv6 messages...". DHCP is a name of
the protocol, not the message. Did you want to say
"DHCP message refers to both DHCPv4 and DHCPv6 messages..." ?