Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#3576 closed enhancement (fixed)

Investigate PXE options, define any missing

Reported by: tomek Owned by: sar
Priority: high Milestone: Kea1.0-beta
Component: dhcp Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

As of 2014-09-05, there is no PXE string in src/lib/dhcp/std_option_defs.h. We need to better understand which options are needed for PXE booting and add definitions for them.

This covers both DHCPv4 and DHCPv6.

Subtickets

Change History (20)

comment:1 Changed 5 years ago by tomek

  • Milestone changed from Kea0.9.1 to Kea0.9.2

Moving to 0.9.2 as discussed on 2014-12-10 meeting.

comment:2 Changed 5 years ago by hschempf

  • Milestone changed from Kea0.9.2 to Kea1.0

comment:3 Changed 5 years ago by marcin

  • Priority changed from medium to high

Move to 'high' priority as discussed during the 1.0 tickets scrub.

comment:4 Changed 4 years ago by fdupont

Related to #4015 for DHCPv6 (#4015 defines all IANA options).

comment:5 Changed 4 years ago by sar

  • Owner set to sar
  • Status changed from new to assigned

comment:6 Changed 4 years ago by sar

It appears that the PXE requires the following options in addition to standard ones already defined:

V4 - RFC4578

93 - CLIENT_SYSTEM (arch type) - array of 2byte arch types
94 - CLIENT_NDI (network interface id) - type, major, minor all 1 byte each
97 - UUID/GUID (client identifier) - type 1byte, machine id variable, the length and format depends on the type. As of 4578 only one type is defined - 0 which indicates a 16 byte GUID

It can make use of option 43 (vendor options) and it may be convenient to try and specify the PXE versions.

Options 128-135 have been used by PXE boot but they can also be used for other things and I haven't seen definitions for them.

V6 - RFC5970

59 BOOTFILE_URL - non-null terminated string, variable length
60 BOOTFILE_PARAM - length1 2bytes, param1 length1 bytes,..., lengthN 2bytes, paramN lengthN bytes
61 CLIENT_ARCH_TYPE - array of 2byte arch types
62 NII - type, major, minor all 1 byte each

comment:7 Changed 4 years ago by fdupont

Cf the #4015 discussion about new IANA code points applied to options...

comment:8 Changed 4 years ago by fdupont

ISC DHCP defines incorrectly in a comment the option 93 (S -> SA).
For option 60 the simplest (and also the ISC DHCP choice) is to defined is as a binary (so the application will parse it).
It should take a low amount of time to implement and review this.
BTW we can take the occasion to revisit/update LibDHCP::isStandardOption (definition and unit tests).

comment:9 Changed 4 years ago by sar

For this ticket option 60 will be defined similar to v4 option 124 - it will get a class to parse a byte string into the proper strings. The configuration will still be done via a byte string.

I have created ticket 4070 to enhance the configuration code to allow a user to specify a number of strings which will then be put together into the proper byte string for the option.

comment:10 Changed 4 years ago by sar

  • Owner changed from sar to UnAssigned
  • Status changed from assigned to reviewing

Ready for review

I have added a new class OptionOpaqueDataTuples? which handles an option of the form
<length1><data><length2><data>...
Currently this is only used for v6 option 60 but I've written it to be usable for other options in the future.

I also made a small change to OptionVendorClass? to adjust the minimal length of an option. Previously this was 6 which seems incorrect. The option includes 2 bytes each for the code and length and 4 bytes for the vendor id, so I think the minimum is 8.

comment:11 Changed 4 years ago by fdupont

  • Owner changed from UnAssigned to fdupont

comment:12 follow-up: Changed 4 years ago by fdupont

  • Owner changed from fdupont to sar

The option 118 disappeared from the DHCPv4 guide. Is it an edit error?

comment:13 Changed 4 years ago by fdupont

fileds -> fields (in std_option_defs.h)
I still have to read the new files...

comment:14 follow-up: Changed 4 years ago by fdupont

Please tabify libdhcp++ unit tests (cf LibDhcpTest::testStdOptionDefs6(D6O_BOOTFILE_URL)

.h: wrong comment for param at in getTuple()

,h spurious space in 'object holds' at hasTuple()

unit test: add a space in OptionBuffer? buf(buf_data,buf_data + sizeof(buf_data)); (after ',').

I am not sure there are enough unit tests: I propose to go with this and to add new tests if coverage is too low. (BTW Jenkins has a coverage setup only in Kea (vs Kea-developers) build farm...)

And of course we need a ChangeLog proposal...

comment:15 in reply to: ↑ 12 Changed 4 years ago by sar

Replying to fdupont:

The option 118 disappeared from the DHCPv4 guide. Is it an edit error?

yes, I've re-added the line for 118

comment:16 in reply to: ↑ 14 Changed 4 years ago by sar

  • Owner changed from sar to fdupont

Replying to fdupont:

Please tabify libdhcp++ unit tests (cf LibDhcpTest::testStdOptionDefs6(D6O_BOOTFILE_URL)

Sigh, I had done that but it didn't get saved and pushed, done

.h: wrong comment for param at in getTuple()

fixed, also fixed same comment in option_vendor_class.h

,h spurious space in 'object holds' at hasTuple()

fixed

unit test: add a space in OptionBuffer? buf(buf_data,buf_data + sizeof(buf_data)); (after ',').

fixed

I am not sure there are enough unit tests: I propose to go with this and to add new tests if coverage is too low. (BTW Jenkins has a coverage setup only in Kea (vs Kea-developers) build farm...)

And of course we need a ChangeLog proposal...

For the change log I propose (also now in the ChangeLog? file in the repo branch).

101x.	[func]		sar
		Added support for several options for use by PXE.
		From RFC4578 (for DHCPv4) these are: 93 - client-system,
		94 - client-ndi, 97 - uuid-guid.
		From RFC5970 (for  DHCPv6) these ae: 59 - bootfile-url,
		60 - bootfile-param, 61 - client-arch-type, 62 - nii.

comment:17 Changed 4 years ago by fdupont

  • Owner changed from fdupont to sar

Patch OK. For the ChangeLog there is an extra (and very visible) space in '(for DHCPv6)' and I notice you believe there is no need to talk about the new opaque tuples option.

comment:18 Changed 4 years ago by tomek

ChangeLog? entry has small typo for DHCPv6: these ae => these are.

comment:19 Changed 4 years ago by sar

  • Resolution set to fixed
  • Status changed from reviewing to closed

Merged with commit tag fdcc73afe7e26bd427817fd771567b1c44713b06

comment:20 Changed 4 years ago by tomek

  • Milestone changed from Kea1.0 to Kea1.0-beta

Milestone renamed

Note: See TracTickets for help on using tickets.