Subject: [dhcwg] A few comments about draft-farrer-dhc-shared-address-lease-00

From: Marcin Siodelski <[email-masked]>
Date: Mon, 21 Oct 2013 13:25:29 +0200

Ian,

I apologize for doing a review so late before IETF. I should have done that
a few weeks ago. A few (hopefully useful) comments...

Abstract

With respect to this:
"This memo describes an update to [RFC2131], which enables the dynamic
leasing of shared IPv4 address and layer 4 source ports to DHCPv4 over
DHCPv6 clients [I-D. etf-dhc-dhcpv4-over-dhcpv6]."

Is it intended to restrict shared addresses functionality to
DHCPv4-over-DHCPv6 clients? It is ok that the draft addresses a specific,
real life use cases, but if it is possible to implement it as a more
generic approach, the document should strive to make it sound generic.

It may be useful to provide a definition of DHCPv4-over-DHCPv6 client
(which is a term used multiple times throughout the document). Although,
there is a reference to the DHCPv4-over-DHCPv6 draft, this draft doesn't
provide a definition for it. Some form of the glossary may be useful.

Introduction:
"[I-D.ietf-dhcp-dhcpv4-over-dhcpv6] introduces a Unified Server..."

After one of the reviews we have changed the terminology in the DHCPv4 over
DHCPv6 draft and we refer to the server processing DHCPv4 requests over
DHCPv6 protocol as "4o6 Server".

3.1 Allocation a Shared, Dynamic IPv4 Address
"The process described below detail...". Should it be "The process
described below details...." ?

Since, the draft is intended to update RFC2131, it should be mentioned what
the server which doesn't support the Dynamic Shared addresses should do
when it receives a message with the OPTION_PORTPARAMSV4. Should it ignore
the message? What should do the server which supports this extension (and
is configured to use it) but haven't received the OPTION_PORTPARAMSV4
option from the client.

3.1.2.
"... within the BOOTPRELAYV6 message  may respond with a DHCPOFFER ...".

Suggest to use MAY instead of "may".

3.1.3. It may be implicitly assumed, because RFC2131 describes the message
exchange process in detail, but IMHO it should be mentioned that client
MUST include yiaddr in DHCPREQUEST to indicate the desired configuration
(besides PSID). Also, the DHCPREQUEST message MUST contain the Client
Identifier (the same that was used to create DHCPDISCOVER).

3.1.5.
In the RFC2131 sections: 2.2 and 3.1 it is said that server SHOULD or MAY
probe that the address being assigned is already in use. The draft is
intended to update RFC2131 so it should be mentioned that server MUST NOT
probe that the address being assigned is in use using ICMP Echo. The text
about client not performing final check is fine obviously.

3.2. Reusing a Previously Allocated Shared, Dynamic IPv4 address

"OPTION_PORTPARAMSV4 MUST be included in the message flow, with the
client's requested port set being included in the DHCPDISCOVER message".

Note that section 3.2 of RFC2131 doesn't mention DISCOVER. The whole
communication is based on 2-way exchange DHCPREQUEST/DHCPACK. Did you mean
DHCPREQUEST?

Although, I support the idea to refer to the draft being updated rather
than rewriting whole sections, but in this case it may be unavoidable. For
example:
- With the standard DHCP, the client will broadcast REQUEST which is not
the case for the Shared Dynamic allocation.
- normally server would broadcast the DHCPNAK if requested binding is
invalid (expired or whatever), it is not the case here.
- Client MUST not perform final check on the assigned address when Shared
Dynamic allocation is used (RFC2131 section 3.2. says it does).

What I mean here is that there are two many disparities between the bare
DHCP protocol described in the RFC2131 and extension described in your
draft to be just able to say: "The RFC2131 is followed with the exception
that OPTION_PORTPARAMSV4 must be included everywhere."

5. Additional Changes to RFC2131 Behvaiour
Neither should server probe IPv4 addresses assigned.

6. Typo: MUST use apply. Should be MUST apply.

7. Specific Behaviour for Softwire Clients
I would rather see this section in the different draft and leave this one a
more generic extension to DHCPv4 to simply allocate IPv4 address + PSID.

8. DHCPv4 over DHCPv6 Source Address Option
The format of the option is wrong. The fields holding lengths of the
variable length fields should be laid before the variable length fields as
typically the server would parse the option from the beginning to the end.

cipv6-prefix-hint is not described

What are the minimal lengths of the prefixes (and minimal length of the
option)? Blank prefix means length=0?

Marcin

---

From: "Bernie Volz (volz)" <[email-masked]>
Date: Mon, 21 Oct 2013 19:57:32 +0000

Marcin:

"Since, the draft is intended to update RFC2131, it should be mentioned what the server which doesn't support the Dynamic Shared addresses should do when it receives a message with the OPTION_PORTPARAMSV4."

These servers would ignore the option as they don't support it. We can't make them do anything else as they already exist.

Also, I think it is WRONG for this draft to update 2131. This isn't a change to all DHCPv4 clients, relays, and servers, so I think we need to be careful here. This extends 2131 with changes into a new space, but that is far different than "updating" 2131. We might need to discuss this some depending on where this work goes - if the intent really is to uplift all clients (and servers) to support this, then perhaps the updates is more appropriate - but I think this is intended to be used only when it is detected that ietf-dhc-dhcpv4-over-dhcpv6 applies?

"Although, I support the idea to refer to the draft being updated rather than rewriting whole sections, but in this case it may be unavoidable."

I don't look forward to a new RFC 2131! Be nice if the differences could be clearly enumerated. I think this would be a win anyway because most likely someone implementing this draft would want to know what to change in an existing client implementation rather than implementing a new one from scratch? Also, we'd have to be careful that people don't mistake the a new 2131 like document for a 2131bis.

-          Bernie

From: [email-masked] [mailto:[email-masked]] On Behalf Of Marcin Siodelski
Sent: Monday, October 21, 2013 7:25 AM
To: [email-masked]
Cc: <[email-masked]>
Subject: [dhcwg] A few comments about draft-farrer-dhc-shared-address-lease-00

Ian,

I apologize for doing a review so late before IETF. I should have done that a few weeks ago. A few (hopefully useful) comments...

Abstract

With respect to this:
"This memo describes an update to [RFC2131], which enables the dynamic leasing of shared IPv4 address and layer 4 source ports to DHCPv4 over DHCPv6 clients [I-D. etf-dhc-dhcpv4-over-dhcpv6]."

Is it intended to restrict shared addresses functionality to DHCPv4-over-DHCPv6 clients? It is ok that the draft addresses a specific, real life use cases, but if it is possible to implement it as a more generic approach, the document should strive to make it sound generic.

It may be useful to provide a definition of DHCPv4-over-DHCPv6 client (which is a term used multiple times throughout the document). Although, there is a reference to the DHCPv4-over-DHCPv6 draft, this draft doesn't provide a definition for it. Some form of the glossary may be useful.

Introduction:

After one of the reviews we have changed the terminology in the DHCPv4 over DHCPv6 draft and we refer to the server processing DHCPv4 requests over DHCPv6 protocol as "4o6 Server".

3.1 Allocation a Shared, Dynamic IPv4 Address
"The process described below detail...". Should it be "The process described below details...." ?

Since, the draft is intended to update RFC2131, it should be mentioned what the server which doesn't support the Dynamic Shared addresses should do when it receives a message with the OPTION_PORTPARAMSV4. Should it ignore the message? What should do the server which supports this extension (and is configured to use it) but haven't received the OPTION_PORTPARAMSV4 option from the client.

3.1.2.

3.1.3. It may be implicitly assumed, because RFC2131 describes the message exchange process in detail, but IMHO it should be mentioned that client MUST include yiaddr in DHCPREQUEST to indicate the desired configuration (besides PSID). Also, the DHCPREQUEST message MUST contain the Client Identifier (the same that was used to create DHCPDISCOVER).

3.1.5.
In the RFC2131 sections: 2.2 and 3.1 it is said that server SHOULD or MAY probe that the address being assigned is already in use. The draft is intended to update RFC2131 so it should be mentioned that server MUST NOT probe that the address being assigned is in use using ICMP Echo. The text about client not performing final check is fine obviously.

"OPTION_PORTPARAMSV4 MUST be included in the message flow, with the client's requested port set being included in the DHCPDISCOVER message".

Note that section 3.2 of RFC2131 doesn't mention DISCOVER. The whole communication is based on 2-way exchange DHCPREQUEST/DHCPACK. Did you mean DHCPREQUEST?

Although, I support the idea to refer to the draft being updated rather than rewriting whole sections, but in this case it may be unavoidable. For example:
- With the standard DHCP, the client will broadcast REQUEST which is not the case for the Shared Dynamic allocation.
- normally server would broadcast the DHCPNAK if requested binding is invalid (expired or whatever), it is not the case here.
- Client MUST not perform final check on the assigned address when Shared Dynamic allocation is used (RFC2131 section 3.2. says it does).

What I mean here is that there are two many disparities between the bare DHCP protocol described in the RFC2131 and extension described in your draft to be just able to say: "The RFC2131 is followed with the exception that OPTION_PORTPARAMSV4 must be included everywhere."

7. Specific Behaviour for Softwire Clients
I would rather see this section in the different draft and leave this one a more generic extension to DHCPv4 to simply allocate IPv4 address + PSID.

8. DHCPv4 over DHCPv6 Source Address Option
The format of the option is wrong. The fields holding lengths of the variable length fields should be laid before the variable length fields as typically the server would parse the option from the beginning to the end.

What are the minimal lengths of the prefixes (and minimal length of the option)? Blank prefix means length=0?

Marcin