Kea #3357: Merge DHCPv4-over-DHCPv6 into Kea repo
Kea Development
do-not-reply at isc.org
Tue Apr 8 18:51:21 UTC 2014
#3357: Merge DHCPv4-over-DHCPv6 into Kea repo
-------------------------------------+-------------------------------------
Reporter: tomek | Owner:
Type: enhancement | Status: new
Priority: medium | Milestone: DHCP-
Component: dhcp | Kea0.9-beta
Keywords: | Resolution:
Sensitive: 0 | CVSS Scoring:
Sub-Project: DHCP | Defect Severity: N/A
Estimated Difficulty: 0 | Feature Depending on Ticket:
Total Hours: 0 | Add Hours to Ticket: 0
| Internal?: 0
-------------------------------------+-------------------------------------
Comment (by marcin):
I've sent quite extensive review to the implementers. The review follows.
Overall, I think that the approach is good. I realize that it will require
more work. In particular, configuration knobs for DHCPv4 over DHCPv6. The
lack of configuration knobs doesn't however preclude the code from being
included as "experimental" in the Kea tree. The one configuration knob
that MUST be present is the one that enables/disables the DHCPv4 over
DHCPv6 function. By default, the server should ignore DHCPv4-query packets
so as the inclusion of this experimental implementation doesn't affect the
operation of the "normal" DHCPv6 server, unless someone specifically
enables it. So this knob should be on our todo list.
The code you provided works but we will need to work on refinement of its
structure, given the fact that this implementation will evolve (and will
require more data to be passed back and forth), and we would like the
communication between processes to be more generic, so as it is easier to
implement similar protocols in the future.
A couple of suggestions below.
'''Socket for Inter process communication (IPC)'''
I think it is a good idea to use UNIX sockets to pass the packets back and
forth between the Kea4 and Kea6 processes. I think we will stick to this.
However, the IPC can be possibly used for other types of traffic (not only
DHCPv4 over DHCPv6 traffic) so the code should be more generic.
Note that for any type of communication between two processes there will
be always a pair of UNIX sockets needed. Therefore, the code that opens
sockets will be common for any type of communication.
What I want to propose here is creation of the simple C++ class (let's
call it BaseIPC). This class should be used by both servers Kea4 and Kea6
to create a UNIX socket used to communicate with other entity (perhaps it
could have an openSocket function). Constructor of this class could take
source and destination FILENAMEs or addresses as arguments.
So, instead of duplicating the code that opens socket (in
IfaceMgr::openSockets4 and IfaceMgr::openSockets6), there could be
something similar to this:
BaseIPC ipc(FILENAME1);
int sock4 = ipc.openSocket();
and in the other server:
BaseIPC ipc(FILENAME2);
int sock6 = ipc.openSocket();
'''Communication between processes'''
Two processes communicating over the UNIX socket may typically exchange
any type of information. The DHCPv4 over DHCPv6 protocol implementation
may certainly need to exchange some more information beyond the DHCPv4
packet decapsulated from the DHCPv4-query. Some examples of extra
information that needs to be carried from DHCPv6 server to the DHCPv4
server:
- flags field from the DHCPv6 message
- IPv6 enabled interface or IPv6 subnet associated with the DHCPv6 packet
received, as the DHCPv4 may need to make allocation decisions based on the
v6 subnet or interface data, because the received v4 packet lacks this
kind of information (as it has been received over IPv6 network and no IPv4
may be in place).
Conversely, the DHCPv4 server may need to include some additional data
beyond DHCPv4 Offer/Reply too. For example, instead of keeping a state of
the DHCPv4-query packet in the map (map4o6) we could choose to send some
information (e.g. id of the v6 enabled interface on which we received the
DHCPv4-query) to the DHCPv4 server. The DHCPv4 would always send it back
to the DHCPv6 server. Perhaps, some other information could be sent by the
DHCPv6 server to DHCPv4 server and be reflected back when the DHCPv4
packet processing is finished. This would eliminate the need for keeping
DHCPv6 packet state in the map.
Elimination of the map is actually an important point, because there are
certain issues that arise when the map is in use. For example, if the
packets are not responded by the DHCPv4 server, the map grows as the new
packets arrive, effectively leading to the leak of memory, unless purged.
Having said that, if we take into account that the extra data have to be
exchanged between the servers it makes me think that we need some
expandable API for the communication between processes. Therefore we
should probably remove DHCPv4-over-DHCPv6 specific functions like send4to6
from the IfaceMgr and put them into the new class derived from the BaseIPC
(see below).
'''Specific communication between DHCPv4 and DHCPv6'''
So far, the proposed BaseIPC class implements the functionality to open a
UNIX socket. At this point, it should probably be extended to be able to
send and receive raw data. Note, that IfaceMgr::send4to6 and
Dhcpv6Srv::processDHCPv4Query in your implementation are rather agnostic
to the Pkt4 and Pkt6 objects, except for the initial step to retrieve the
raw buffer from these objects. The rest of the code (with some
modifications), could be moved from these functions to the new function
BaseIPC::send(const std::vector<uint8_t>& buf). This would be a generic
function used to opaque data between processes.
Similarly, there could be a BaseIPC::receive function to receive raw data
on the socket.
Having a generic class opening sockets and sending/receiving data through
a unix socket, it will be possible to create a new classes (derived from
BaseIPC) DHCPv6to4IPC and DHCPv4to6IPC which would customize the
communication specifically for the sake of DHCPv4 over DHCPv6 protocol
implementation. For example the class DHCPv6to4IPC could have a new "send"
method:
{{{
DHCPv6to4IPC::send(const Pkt4Ptr& pkt4, const Metadata& metadata) {
create buffer;
write length of the pkt4 to the buffer;
write pkt4 as binary;
write length of the metadata;
write metadata as binary;
BaseIPC::send(buffer);
}
}}}
The data on the other end would be reconstructed by the
DHCPv4to6IPC::receive().
Please note that by abstracting out the BaseIPC and derived classes, if we
ever decide to change the format of the serialized data from binary to
something else (e.g. text based) the change is limited to the DHCPv6to4IPC
and DHCPv4to6IPC.
'''Taking server specific code out of the libdhcp++'''
The libdhcp++ is now meant to be a low level library for operation on
sockets but it is not really dedicated to solve server-side problems. The
inter process communication, inclusion of the metadata like subnet
information is more a server side problem.
Therefore, the code that is preparing these data to send to the other
process should be taken out of libdhcp++. What I am proposing here is that
IPC sockets are not opened in the IfaceMgr, that IfaceMgr::send and
IfaceMgr::receive don't call send4to6, receive6to4 etc.
The servers (either dhcpv6_srv or dhcpv4_srv) should rather call
DHCPvXtoYIPC::send and DHCPvXtoYIPC::receive directly (or via something in
libdhcpsrv).
Also, please note that there should be a way to disable/enable the DHCPv4
over DHCPv6 function. If this is disabled (and it should actually be the
default setting), the server knowing about it, would not open a UNIX
socket and would simply ignore the DHCPv4-query packets. Note, that
current code doesn't really allow this behavior, because the logic that
opens UNIX socket is buried under IfaceMgr::openSockets4 and
IfaceMgr::openSocket6 so the UNIX socket is always opened.
'''Coding style'''
Each code that is committed to Kea needs to conform to the Kea coding
guidelines: http://kea.isc.org/wiki/CodingGuidelines. This is a part of
our review process to check that.
'''Additional comments'''
The IfaceMgr already opens unix socket for IPC, there is no need to open a
new socket to send the packet (see processDHCPv4Query). Simply use the
socket that is bound. That could be solved by the new classes I proposed
above.
The processDHCPv4Response calls these functions:
- copyDefaultOptions
- appendDefaultOptions
- appendRequestedOptions
to append a bunch of options to the DHCPv4-response packet. This shouldn't
be the case. The client should not get the v6 client id, and other
configured options, such as NTP servers etc. Note that the client has got
his configuration already in the initial DHCPv6 exchange with the DHCPv6
server.
Every code must be unit tested. This code doesn't include unit tests.
The latest Kea code (precisely IfaceMgr) allows for registration of the
"external" sockets and associated callback function. The callback function
is invoked when the external socket has the data. The server code may
provide a callback function which will call DHCPvXoYIPC::receive to
receive the data over the UNIX socket. Your code should be updated to the
latest version of Kea.
--
Ticket URL: <http://kea.isc.org/ticket/3357#comment:4>
Kea Development <http://kea.isc.org>
Kea Development
More information about the bind10-tickets
mailing list