[systemd-devel] [PATCH 24/28] dhcp: Process DHCP Ack/Nak message
Zbigniew Jędrzejewski-Szmek
zbyszek at in.waw.pl
Wed Nov 13 17:40:29 PST 2013
On Wed, Nov 13, 2013 at 11:22:52PM +0200, Patrik Flykt wrote:
> Process a DHCP Ack/Nak in much the same way as an DHCP Offer. Factor
> out header verification and process options sent. Add notification
> functionality with discrete values for the outcome of the DHCP Ack/
> Nak processing.
> ---
> src/dhcp/client.c | 145 +++++++++++++++++++++++++++++++++++++++++++++--------
> src/dhcp/client.h | 4 ++
> 2 files changed, 128 insertions(+), 21 deletions(-)
>
> diff --git a/src/dhcp/client.c b/src/dhcp/client.c
> index 45c62f3..669804a 100644
> --- a/src/dhcp/client.c
> +++ b/src/dhcp/client.c
> @@ -151,6 +151,11 @@ int dhcp_client_set_mac(DHCPClient *client, struct ether_addr *addr)
> return 0;
> }
>
> +static int client_notify(DHCPClient *client, int event)
> +{
> + return 0;
> +}
> +
> static int client_stop(DHCPClient *client, int error)
> {
> if (!client)
> @@ -178,6 +183,7 @@ static int client_stop(DHCPClient *client, int error)
>
> case DHCP_STATE_INIT:
> case DHCP_STATE_SELECTING:
> + case DHCP_STATE_REQUESTING:
>
> client->start_time = 0;
> client->state = DHCP_STATE_INIT;
> @@ -185,7 +191,6 @@ static int client_stop(DHCPClient *client, int error)
>
> case DHCP_STATE_INIT_REBOOT:
> case DHCP_STATE_REBOOTING:
> - case DHCP_STATE_REQUESTING:
> case DHCP_STATE_BOUND:
> case DHCP_STATE_RENEWING:
> case DHCP_STATE_REBINDING:
> @@ -499,41 +504,52 @@ static int client_parse_offer(uint8_t code, uint8_t len, uint8_t *option,
> return 0;
> }
>
> -static int client_receive_offer(DHCPClient *client,
> - DHCPPacket *offer, int len)
> +static int client_verify_headers(DHCPClient *client,
> + DHCPPacket *message, int len)
> {
> int hdrlen;
> - DHCPLease *lease;
>
> if (len < (DHCP_IP_UDP_SIZE + DHCP_MESSAGE_SIZE))
> return -EINVAL;
>
> - hdrlen = offer->ip.ihl * 4;
> - if (hdrlen < 20 || hdrlen > len || client_checksum(&offer->ip,
> + hdrlen = message->ip.ihl * 4;
> + if (hdrlen < 20 || hdrlen > len || client_checksum(&message->ip,
> hdrlen))
> return -EINVAL;
>
> - offer->ip.check = offer->udp.len;
> - offer->ip.ttl = 0;
> + message->ip.check = message->udp.len;
> + message->ip.ttl = 0;
>
> - if (hdrlen + ntohs(offer->udp.len) > len ||
> - client_checksum(&offer->ip.ttl, ntohs(offer->udp.len) + 12))
> + if (hdrlen + ntohs(message->udp.len) > len ||
> + client_checksum(&message->ip.ttl, ntohs(message->udp.len) + 12))
> return -EINVAL;
>
> - if (ntohs(offer->udp.source) != DHCP_PORT_SERVER ||
> - ntohs(offer->udp.dest) != DHCP_PORT_CLIENT)
> + if (ntohs(message->udp.source) != DHCP_PORT_SERVER ||
> + ntohs(message->udp.dest) != DHCP_PORT_CLIENT)
> return -EINVAL;
>
> - if (offer->dhcp.op != BOOTREPLY)
> + if (message->dhcp.op != BOOTREPLY)
> return -EINVAL;
>
> - if (ntohl(offer->dhcp.xid) != client->xid)
> + if (ntohl(message->dhcp.xid) != client->xid)
> return -EINVAL;
>
> - if (memcmp(&offer->dhcp.chaddr[0], &client->mac_addr.ether_addr_octet,
> + if (memcmp(&message->dhcp.chaddr[0], &client->mac_addr.ether_addr_octet,
> ETHER_ADDR_LEN))
> return -EINVAL;
>
> + return 0;
> +}
> +
> +static int client_receive_offer(DHCPClient *client, DHCPPacket *offer, int len)
> +{
> + int err;
> + DHCPLease *lease;
> +
> + err = client_verify_headers(client, offer, len);
> + if (err < 0)
> + return err;
> +
> lease = new0(DHCPLease, 1);
> if (!lease)
> return -ENOBUFS;
> @@ -561,6 +577,64 @@ error:
> return -ENOMSG;
> }
>
> +static int client_receive_ack(DHCPClient *client, DHCPPacket *offer, int len)
> +{
> + int err = -ENOMSG;
Since err is not an error always, but a sucess return value too,
it would be nicer to call it r. Also this value is override right below.
You might consider the following pattern:
_cleanup_free_ DHCPLease *lease = NULL;
and ...
> + DHCPLease *lease;
> + int type;
> +
> + err = client_verify_headers(client, offer, len);
> + if (err < 0)
> + return err;
> +
> + lease = new0(DHCPLease, 1);
> + if (!lease)
> + return -ENOBUFS;
> +
> + len = len - DHCP_IP_UDP_SIZE;
> + type = __dhcp_option_parse(&offer->dhcp, len, client_parse_offer,
> + lease);
> +
> + if (type == DHCP_NAK) {
> + err = DHCP_EVENT_NAK;
> + goto error;
... and
return DHCP_EVENT_NAK;
and ...
> + }
> +
> + if (type != DHCP_ACK)
> + goto error;
Is err set correctly here?
> +
> + lease->address.s_addr = offer->dhcp.yiaddr;
> +
> + if (lease->address.s_addr == INADDR_ANY ||
> + lease->server_address.s_addr == INADDR_ANY ||
> + lease->subnet_mask.s_addr == INADDR_ANY ||
> + lease->lifetime == 0) {
> + err = -ENOMSG;
> + goto error;
... and just return here ...
> + }
> +
> + err = 0;
> +
> + if (client->lease) {
> + if (client->lease->address.s_addr != lease->address.s_addr ||
> + client->lease->subnet_mask.s_addr !=
> + lease->subnet_mask.s_addr ||
> + client->lease->router.s_addr != lease->router.s_addr) {
> + err = DHCP_EVENT_IP_CHANGE;
> + }
> + free(client->lease);
> + }
> +
> + client->lease = lease;
... and here
lease = NULL;
and ...
> +
> + return err;
... and remove the part below.
> +
> +error:
> + free(lease);
> +
> + return err;
> +}
> +
> static int client_receive_raw_message(sd_event_source *s, int fd,
> uint32_t revents, void *userdata)
> {
> @@ -568,6 +642,7 @@ static int client_receive_raw_message(sd_event_source *s, int fd,
> int err = 0;
> int len, buflen;
> uint8_t *buf;
> + char tmp;
> DHCPPacket *message;
>
> buflen = sizeof(DHCPPacket) + DHCP_CLIENT_MIN_OPTIONS_SIZE;
> @@ -602,10 +677,36 @@ static int client_receive_raw_message(sd_event_source *s, int fd,
>
> break;
>
> + case DHCP_STATE_REQUESTING:
> +
> + err = client_receive_ack(client, message, len);
> + if (err == DHCP_EVENT_NAK)
> + goto error;
> +
> + if (err >= 0) {
> + client->timeout_resend =
> + sd_event_source_unref(client->timeout_resend);
> +
> + client->state = DHCP_STATE_BOUND;
> + client->attempt = 1;
> +
> + if (!client->last_addr)
> + client->last_addr =
> + malloc(sizeof(struct in_addr));
> + if (client->last_addr)
> + client->last_addr->s_addr =
> + client->lease->address.s_addr;
> +
> + client_notify(client, DHCP_EVENT_IP_ACQUIRE);
> +
> + close(client->fd);
> + client->fd = 0;
-1 not 0.
> + }
> + break;
> +
> case DHCP_STATE_INIT:
> case DHCP_STATE_INIT_REBOOT:
> case DHCP_STATE_REBOOTING:
> - case DHCP_STATE_REQUESTING:
> case DHCP_STATE_BOUND:
> case DHCP_STATE_RENEWING:
> case DHCP_STATE_REBINDING:
> @@ -618,13 +719,15 @@ static int client_receive_raw_message(sd_event_source *s, int fd,
> return 0;
>
> error:
> - if (err < 0)
> - return client_stop(client, err);
> + if (buf)
> + free(buf);
> + else
> + read(fd, &tmp, 1);
>
> - if (!err)
> - read(fd, buf, 1);
> + if (err != 0)
> + return client_stop(client, err);
>
> - return 0;
> + return err;
> }
>
> int dhcp_client_start(DHCPClient *client)
> diff --git a/src/dhcp/client.h b/src/dhcp/client.h
> index 568e70a..5ce7fb4 100644
> --- a/src/dhcp/client.h
> +++ b/src/dhcp/client.h
> @@ -26,6 +26,10 @@
>
> #include "sd-event.h"
>
> +#define DHCP_EVENT_NAK 1
> +#define DHCP_EVENT_IP_ACQUIRE 2
> +#define DHCP_EVENT_IP_CHANGE 3
> +
> struct DHCPClient;
> typedef struct DHCPClient DHCPClient;
>
Zbyszek
More information about the systemd-devel
mailing list