Skip to content

Commit

Permalink
Merge pull request #664 from nirmoy/fix_xid_gen_1
Browse files Browse the repository at this point in the history 
dhcp4: xid generation and usage fixes
  • Loading branch information
@mtomaschewski
mtomaschewski committed Sep 8, 2016
2 parents 7930bde + 505f6bd commit a745443
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 22 deletions.
43 changes: 29 additions & 14 deletions src/dhcp4/device.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
static unsigned int ni_dhcp4_do_bits(unsigned int);
static const char * __ni_dhcp4_print_doflags(unsigned int);

static uint32_t ni_dhcp4_xid;
ni_dhcp4_device_t * ni_dhcp4_active;

/*
Expand Down Expand Up @@ -487,7 +486,7 @@ ni_dhcp4_release(ni_dhcp4_device_t *dev, const ni_uuid_t *lease_uuid)
* server's reply. We just keep our fingers crossed that it's
* getting out. If it doesn't, it's rather likely the network
* is hosed anyway, so there's little point in delaying. */
ni_dhcp4_fsm_release(dev);
ni_dhcp4_fsm_release_init(dev);

ni_dhcp4_device_stop(dev);
return 0;
Expand Down Expand Up @@ -617,20 +616,17 @@ ni_dhcp4_device_send_message(ni_dhcp4_device_t *dev, unsigned int msg_code, cons
ni_timeout_param_t timeout;
int rv;

/* Assign a new XID to this message */
if (ni_dhcp4_xid == 0)
ni_dhcp4_xid = random();
dev->dhcp4.xid = ni_dhcp4_xid++;

dev->transmit.msg_code = msg_code;
dev->transmit.lease = lease;

if (ni_dhcp4_socket_open(dev) < 0) {
ni_error("unable to open capture socket");
ni_error("%s: unable to open capture socket", dev->ifname);
goto transient_failure;
}

ni_debug_dhcp("sending %s with xid 0x%x", ni_dhcp4_message_name(msg_code), htonl(dev->dhcp4.xid));
ni_debug_dhcp("%s: sending %s with xid 0x%x in state %s", dev->ifname,
ni_dhcp4_message_name(msg_code), htonl(dev->dhcp4.xid),
ni_dhcp4_fsm_state_name(dev->fsm.state));

if ((rv = ni_dhcp4_device_prepare_message(dev)) < 0)
return -1;
Expand Down Expand Up @@ -682,11 +678,6 @@ ni_dhcp4_device_send_message_unicast(ni_dhcp4_device_t *dev, unsigned int msg_co
.sin_port = htons(DHCP4_SERVER_PORT),
};

/* Assign a new XID to this message */
if (ni_dhcp4_xid == 0)
ni_dhcp4_xid = random();
dev->dhcp4.xid = ni_dhcp4_xid++;

dev->transmit.msg_code = msg_code;
dev->transmit.lease = lease;

Expand Down Expand Up @@ -901,3 +892,27 @@ ni_dhcp4_supported(const ni_netdev_t *ifp)
return TRUE;
}

void
ni_dhcp4_new_xid(ni_dhcp4_device_t *cur)
{
ni_dhcp4_device_t *dev;
unsigned int xid;

if (!cur)
return;

do {
do {
xid = random();
} while (!xid);

for (dev = ni_dhcp4_active; dev; dev = dev->next) {
if (xid == dev->dhcp4.xid) {
xid = 0;
break;
}
}
} while (!xid);

cur->dhcp4.xid = xid;
}
4 changes: 3 additions & 1 deletion src/dhcp4/dhcp4.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,9 @@ extern int ni_dhcp4_acquire(ni_dhcp4_device_t *, const ni_dhcp4_request_t *);
extern int ni_dhcp4_release(ni_dhcp4_device_t *, const ni_uuid_t *);
extern void ni_dhcp4_restart_leases(void);

extern const char * ni_dhcp4_fsm_state_name(enum fsm_state);
extern void ni_dhcp4_fsm_init_device(ni_dhcp4_device_t *);
extern void ni_dhcp4_fsm_release(ni_dhcp4_device_t *);
extern void ni_dhcp4_fsm_release_init(ni_dhcp4_device_t *);
extern int ni_dhcp4_fsm_process_dhcp4_packet(ni_dhcp4_device_t *, ni_buffer_t *);
extern int ni_dhcp4_fsm_commit_lease(ni_dhcp4_device_t *, ni_addrconf_lease_t *);
extern int ni_dhcp4_recover_lease(ni_dhcp4_device_t *);
Expand Down Expand Up @@ -258,6 +259,7 @@ extern void ni_dhcp4_device_force_retransmit(ni_dhcp4_device_t *, unsigned int)
extern void ni_dhcp4_device_arp_close(ni_dhcp4_device_t *);
extern ni_bool_t ni_dhcp4_parse_client_id(ni_opaque_t *, unsigned short, const char *);
extern ni_bool_t ni_dhcp4_set_client_id(ni_opaque_t *, const ni_hwaddr_t *);
extern void ni_dhcp4_new_xid(ni_dhcp4_device_t *);
extern void ni_dhcp4_device_set_best_offer(ni_dhcp4_device_t *, ni_addrconf_lease_t *, int);
extern void ni_dhcp4_device_drop_best_offer(ni_dhcp4_device_t *);

Expand Down
25 changes: 18 additions & 7 deletions src/dhcp4/fsm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#define NAK_BACKOFF_MAX 60 /* seconds */

static int ni_dhcp4_fsm_arp_validate(ni_dhcp4_device_t *);
static const char * ni_dhcp4_fsm_state_name(enum fsm_state);

static int ni_dhcp4_process_offer(ni_dhcp4_device_t *, ni_addrconf_lease_t *);
static int ni_dhcp4_process_ack(ni_dhcp4_device_t *, ni_addrconf_lease_t *);
Expand Down Expand Up @@ -220,7 +219,6 @@ ni_dhcp4_fsm_process_dhcp4_packet(ni_dhcp4_device_t *dev, ni_buffer_t *msgbuf)
* waiting for additional packets.
*/
ni_dhcp4_device_disarm_retransmit(dev);
dev->dhcp4.xid = 0;

/* move to next stage of protocol */
switch (msg_code) {
Expand Down Expand Up @@ -393,8 +391,10 @@ __ni_dhcp4_fsm_discover(ni_dhcp4_device_t *dev, int scan_offers)
}

static void
ni_dhcp4_fsm_discover(ni_dhcp4_device_t *dev)
ni_dhcp4_fsm_discover_init(ni_dhcp4_device_t *dev)
{
dev->fsm.state = NI_DHCP4_STATE_SELECTING;
ni_dhcp4_new_xid(dev);
dev->start_time = time(NULL);
dev->config->elapsed_timeout = 0;
__ni_dhcp4_fsm_discover(dev, 1);
Expand Down Expand Up @@ -436,6 +436,7 @@ static void
ni_dhcp4_fsm_renewal_init(ni_dhcp4_device_t *dev)
{
dev->fsm.state = NI_DHCP4_STATE_RENEWING;
ni_dhcp4_new_xid(dev);
dev->start_time = time(NULL);
/* Send renewal request at least once */
ni_dhcp4_fsm_renewal(dev, TRUE);
Expand Down Expand Up @@ -466,6 +467,7 @@ static void
ni_dhcp4_fsm_rebind_init(ni_dhcp4_device_t *dev)
{
dev->fsm.state = NI_DHCP4_STATE_REBINDING;
ni_dhcp4_new_xid(dev);
dev->start_time = time(NULL);
dev->lease->dhcp4.server_id.s_addr = 0;
/* Send rebind request at least once */
Expand All @@ -481,6 +483,7 @@ ni_dhcp4_fsm_reboot(ni_dhcp4_device_t *dev)
/* RFC 2131, 3.2 (see also 3.1) */
ni_debug_dhcp("trying to confirm lease for %s", dev->ifname);

ni_dhcp4_new_xid(dev);
dev->config->elapsed_timeout = 0;
dev->start_time = time(NULL);
dev->fsm.state = NI_DHCP4_STATE_REBOOT;
Expand Down Expand Up @@ -528,6 +531,13 @@ ni_dhcp4_fsm_release(ni_dhcp4_device_t *dev)
}
}

void
ni_dhcp4_fsm_release_init(ni_dhcp4_device_t *dev)
{
ni_dhcp4_new_xid(dev);
ni_dhcp4_fsm_release(dev);
}

/*
* We never received any response. Deal with the traumatic rejection.
*/
Expand All @@ -545,7 +555,7 @@ ni_dhcp4_fsm_timeout(ni_dhcp4_device_t *dev)
* started to back off, or if we declined a lease because
* the address was already in use. */
ni_dhcp4_device_drop_lease(dev);
ni_dhcp4_fsm_discover(dev);
ni_dhcp4_fsm_discover_init(dev);
break;

case NI_DHCP4_STATE_SELECTING:
Expand Down Expand Up @@ -594,7 +604,7 @@ ni_dhcp4_fsm_timeout(ni_dhcp4_device_t *dev)

/* Now decide whether we should keep trying */
if (dev->config->acquire_timeout == 0)
ni_dhcp4_fsm_discover(dev);
ni_dhcp4_fsm_discover_init(dev);
break;

case NI_DHCP4_STATE_VALIDATING:
Expand Down Expand Up @@ -659,7 +669,7 @@ ni_dhcp4_fsm_link_up(ni_dhcp4_device_t *dev)
switch (dev->fsm.state) {
case NI_DHCP4_STATE_INIT:
/* We get here if we aborted a discovery operation. */
ni_dhcp4_fsm_discover(dev);
ni_dhcp4_fsm_discover_init(dev);
break;

case NI_DHCP4_STATE_BOUND:
Expand All @@ -674,7 +684,7 @@ ni_dhcp4_fsm_link_up(ni_dhcp4_device_t *dev)
if (dev->lease)
ni_dhcp4_fsm_reboot(dev);
else
ni_dhcp4_fsm_discover(dev);
ni_dhcp4_fsm_discover_init(dev);
break;
case NI_DHCP4_STATE_SELECTING:
case NI_DHCP4_STATE_REQUESTING:
Expand Down Expand Up @@ -741,6 +751,7 @@ ni_dhcp4_process_offer(ni_dhcp4_device_t *dev, ni_addrconf_lease_t *lease)
} else {
ni_info("%s: Requesting DHCPv4 lease with timeout %u sec",
dev->ifname, dev->config->acquire_timeout);
ni_dhcp4_new_xid(dev);
dev->start_time = time(NULL);
dev->config->elapsed_timeout = 0;
ni_dhcp4_fsm_request(dev, lease);
Expand Down

0 comments on commit a745443

Please sign in to comment.