#5514 closed enhancement (complete)

Reinstantiate lw4over6 options

Reported by: tomek Owned by: tomek
Priority: medium Milestone: Kea1.4
Component: dhcp6 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 (last modified by tomek)

The support for lw4over6 options (options 89-96) was added in PR#24 (#5016) and then removed in #5191 for various reasons. Now it's time to add it back and test it properly.

This ticket effectively reverts #5191 with perhaps some additional work around testing.

Subtickets

Change History (9)

comment:1 Changed 12 months ago by tomek

  • Description modified (diff)

comment:2 Changed 12 months ago by fdupont

  • Milestone changed from Kea-proposed to Kea1.4

1.4 medium by 2018-01-18 conf call.

comment:3 Changed 12 months ago by tomek

  • Owner set to Unassigned
  • Status changed from new to reviewing

The code is now ready for review. This is a user visible change, so a ChangeLog? is required. Here's a proposal:

13XX.	[func]		tomek
	Lightweight 4over6 options reinstantiated. Definitions for DHCPv6
	options 89 through 96 were added back. DHCPv4 v4 Parameters Option
        159 has its definition tweaked slightly.
	(Trac #5514, git tbd)

comment:4 Changed 12 months ago by fdupont

  • Owner changed from Unassigned to fdupont

comment:5 Changed 12 months ago by fdupont

  • Owner changed from fdupont to tomek

If you have time a more realistic MAP-E option should have a BR too, another test with two sub options with the same code (e.g. MAP-T with 2 rules) and a sub sub option (e.g. a rule with a port params) could be fine.

comment:6 Changed 12 months ago by tomek

  • Owner changed from tomek to fdupont

This was a good idea to test complex options. Added new test and it found couple issues.
Fixed support for option spaces other than dhcp6. Added printing of psid.

I'm not sure if I got the psid printing right.

comment:7 Changed 12 months ago by fdupont

I remember that coding toElement() I was looking for (and did not find) a not system option which has both a body and sub options because toElement uses toBinary which includes sub options. So it will be nice to add a test for this particular case and fix the code (not now, 1-4-final seems fine).
I'll finish the review tomorrow.

comment:8 Changed 12 months ago by fdupont

  • Owner changed from fdupont to tomek

Code OK.

comment:9 Changed 12 months ago by tomek

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

Thanks for the review. Code merged. Closing ticket.

Note: See TracTickets for help on using tickets.