WIP: add an async dhcp client

[etene]

Here is my async DHCP client implementation, still a work in progress

closes #1243

[ffourcot]

this one is not sorted?

[etene]

i added it because at some point it was missing when i was testing something, but this is still a draft, these are not even used at this point

[svinota]

Wow, c'est magnifique. It will take some time to read & digest, be patient :)

[etene]

Wow, c'est magnifique. It will take some time to read & digest, be patient :)

Thanks ! Take your time, it's still a draft and lots of things are going to change, i'm not yet proud with everything

[svinota]

@etene what would you say — could it be a reasonable idea to make a socket wrapper that would use composition, and proxy most of the properties, but fake .type to make it work with loop.create_datagram_endpoint() ? and then use asyncio.Protocol

[etene]

I think it's an interesting idea.

I have tried at some point to make raw sockets work with asyncio.Protocol but changed my mind when i saw how a lot of code assumed TCP, UDP or Unix sockets iirc. But I hadn't thought about faking the socket type, and it might just work given how similar raw & UDP sockets are.

I'm willing to try if I find the time to. However, the python stdlib code might be too tightly coupled with UDP/TCP/unix sockets, so it might turn out to be dirty or impossible without making a PR to python itself, which is a whole other can of worms...

[svinota]

@etene JFYI: I plan to keep Python compatibility back to version 3.9 as for now. Will be there any issues with that — pls let me know.

[etene]

@etene JFYI: I plan to keep Python compatibility back to version 3.9 as for now. Will be there any issues with that — pls let me know.

@svinota I'm pretty sure i can make it work with 3.9 upwards, my initial draft was based on 3.12 out of habit and comfort but the really important asyncio parts have been there since 3.7.

[svinota]

The reason behind 3.9 (FIXME: I should document it) is socket.recv_fds() that is not supported in earlier versions — and which is used in the core functionality to init netns-enabled sockets.

[svinota]

@etene regarding the tests I would propose this thing:

1. save binary packets (optionally, store them as hex dumps, see [1])
2. use config.mock_dhcp in a way like [2][3]
3. having this, just mock the traffic through the socket, and assert every state transition
4. bonus: now code snippets in the docs can be tested as well, to keep the docs up to date automatically, like in [4][5]

[1]

pyroute2/tests/test_core/test_ipr/test_route_dump_async.py

Lines 7 to 23 in 899fc58

	from pyroute2.common import load_dump
	test_dump_data = '''
	# 10.1.2.0/24 via 127.0.0.8 dev lo table 100
	# 10.1.3.0/24 via 127.0.0.8 dev lo table 100
	24:12:31:45 3c:00:00:00 18:00:22:00 dc:d0:86:67
	7b:68:00:00 02:18:00:00 64:03:00:01 00:00:00:00
	08:00:0f:00 64:00:00:00 08:00:01:00 0a:01:02:00
	08:00:05:00 7f:00:00:08 08:00:04:00 01:00:00:00
	3c:00:00:00 18:00:22:00 dc:d0:86:67 7b:68:00:00
	02:18:00:00 64:03:00:01 00:00:00:00 08:00:0f:00
	64:00:00:00 08:00:01:00 0a:01:03:00 08:00:05:00
	7f:00:00:08 08:00:04:00 01:00:00:00
	'''

[2]

pyroute2/pyroute2/netlink/rtnl/iprsocket.py

Lines 93 to 95 in 899fc58

	if config.mock_iproute:
	use_socket = IPEngine()
	netns = None

[3]

pyroute2/docs/iproute_linux.rst

Lines 3 to 11 in 19a309b

	.. testsetup:: *
	from pyroute2 import config, IPRoute
	config.mock_netlink = True
	ipr = IPRoute()
	.. testcleanup:: *
	ipr.close()

[4] source:

pyroute2/pyroute2/iproute/linux.py

Lines 254 to 267 in 19a309b

	.. testcode:: fm01
	# get all links with names starting with eth:
	#
	for link in ipr.filter_messages(
	lambda x: x.get("ifname").startswith("eth"),
	ipr.link("dump"),
	):
	print(link.get("ifname"))
	.. testoutput:: fm01
	eth0

[5] rendered: https://pyroute2.org/pyroute2-0.8.1-109-g19a309be/iproute_linux.html#pyroute2.iproute.linux.RTNL_API.filter_messages

[svinota]

use busybox udhcpd instead, on Fedora there is no separate link for that, unlike Ubuntu; but running the plugin this way as busybox udhcpd works on both Fedora and Ubuntu.

[etene]

It is actually what's used, when building the command line the fixture uses BINARY_PATH and the rest (from get_cmdline_options) is just its args.

[etene]

@svinota I read your comment about using hex dumps for tests, that is a great idea for properly unit testing everything. Actually I was thinking about going as far as to store these test packets in pcap files, which might be easier for debugging.

However if you don't mind i would still like to have some real integration tests with a couple of real DHCP servers.

[svinota]

@etene sure thing, leave the integration in place, it isn't mutually exclusive.

[BrianBaboch]

Suggested change

	return self._seconds_til_timer('lease')
	return self._seconds_til_timer('expiration')

[svinota]

I believe it's about this option, see line 47:

pyroute2/pyroute2/dhcp/dhcp4msg.py

Lines 35 to 56 in c5679a0

	options = (
	(0, 'pad', 'none'),
	(1, 'subnet_mask', 'ip4addr'),
	(2, 'time_offset', 'be32'),
	(3, 'router', 'ip4list'),
	(4, 'time_server', 'ip4list'),
	(5, 'ien_name_server', 'ip4list'),
	(6, 'name_server', 'ip4list'),
	(7, 'log_server', 'ip4list'),
	(8, 'cookie_server', 'ip4list'),
	(9, 'lpr_server', 'ip4list'),
	(50, 'requested_ip', 'ip4addr'),
	(51, 'lease_time', 'be32'),
	(53, 'message_type', 'message_type'),
	(54, 'server_id', 'ip4addr'),
	(55, 'parameter_list', 'array8'),
	(57, 'message_size', 'be16'),
	(58, 'renewal_time', 'be32'),
	(59, 'rebinding_time', 'be32'),
	(60, 'vendor_id', 'string'),
	(61, 'client_id', 'client_id'),
	(255, 'end', 'none'),

[BrianBaboch]

Suggested change

	the lease. The leases's IP is expected to be assigned to the client's
	the lease. The lease's IP is expected to be assigned to the client's

[BrianBaboch]

Suggested change

	In both cases, the bootp client IP (ciaddr) is set to the leases's IP.
	In both cases, the bootp client IP (ciaddr) is set to the lease's IP.

[svinota]

Rebasing and merging into p10-dhcp-protocol branch.

Lets continue in one repo.