Bug 4273

Summary:	DHCP Client: "xid" would be better to change per "dialog" window
Product:	Busybox	Reporter:	Bill <bspitz>
Component:	Networking	Assignee:	unassigned
Status:	RESOLVED WONTFIX
Severity:	minor	CC:	busybox-cvs
Priority:	P4
Version:	1.15.x
Target Milestone:	---
Hardware:	PC
OS:	Linux
Host:		Target:
Build:

Description Bill 2011-09-29 05:54:04 UTC

In networking/udhcp/dhcpc.c:
  I can see that the "xid" is randomly created at discovery, but then we just use the same xid for ever. Ideally it would be better to increment the xid after each ACK or NAK, to make the next "dialog window" we use unquie from previous "dialog" window. RFC say this could actually be incremented every message we send, but i have seen most clients just increment this per dialog window.
I am using busybox 1.11.2, but do not see any note of change up to 1.19.2. This is 2 lines of code change, and keeps the xid more unquie per dialog window. Also once we have processed a ACK or NAK, then the moving xid would make sure we do not accpets a redundant ACK, NAK.
Also note that i belive should check that xid is never 0, not sure what RFC says about that, but ensuring non zero value is probably best.

Comment 1 Denys Vlasenko 2011-11-18 01:05:00 UTC

(In reply to comment #0)
> In networking/udhcp/dhcpc.c:
>   I can see that the "xid" is randomly created at discovery, but then we just
> use the same xid for ever. Ideally it would be better to increment the xid
> after each ACK or NAK, to make the next "dialog window" we use unquie from
> previous "dialog" window.

Why it would be good?

> This is 2 lines of code change

Thus makes code bigger. What problematic scenario are you trying to fix?

> Also once we have processed a ACK or NAK, then the moving xid would make sure
> we do not accpets a redundant ACK, NAK.

We stop waiting for answers after first ACK, so problem doesn't exist.
If we get NAK, we generate new xid already.

> Also note that i belive should check that xid is never 0, not sure what RFC
> says about that, but ensuring non zero value is probably best.

Sounds somewhat paranoid to me. Should we also check for 0xffffffff?

Comment 2 Bill 2011-11-18 02:27:55 UTC

I just double checked RFC2131, the udhcpc is compliant with xid usage, we may just internally make this change for cleared per transaction xid uniqueness. In RFC2131, it does actually mention this as suggested optional implementation.