This is the mail archive of the ecos-discuss@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: DHCP xid



Robin Farine <acnrf@dial.eunet.ch> ecrit:
> If I understand correctly, the XID generation *moved* from the inside of
> the while loop to before the loop instead of being duplicated (lesson:
> double check a CVS merge).
> 
> Under the assumption that the ifr's hardware address field gets
> initialized *before* generating the XID, this should work as before but
> with the extra property of keeping the same XID even in the case where
> the client has to rebind from scratch, what Ascom probably wanted to
> achieve for any reason.
> 
> Now, just to clarify the intended usage of the XID (from rfc2131):
> 
>     "The 'xid' field is used by the client to match incoming DHCP
>     messages with pending request. A DHCP client MUST choose 'xid's in
>     such a way as to minimize the chance of using an 'xid' identical to
>     one used by another client. For example, a client may choose a
>     different, random initial 'xid' each time the client is rebooted,
>     and subsequently use sequential 'xid's until the next reboot."
> 
> Which implies among other that the DHCP server shouldn't assign any
> additional meaning to the XID. Moreover, by always keeping the same XID,
> two clients that unluckily generate the same XID would never be able to
> recover from this situation until one of them reboots.
> 
> To summarize, I think that do_dhcp() should in fact generate a new XID
> for each request instead of making the XID even more permanent than the
> former version did. What do you think?

Agreed, and thanks for looking into it for us.  I think I understand the
history and purpose of the changes - I spotted a comment in our bug
tracking system somewhere about it today which reminded me.

Originally it picked an XID using addresses of something on the stack.
This meant the same XID for all identical images on identical hardware:
this was bad!

I changed it to use the ESA plus arc4random() to prevent identical XIDs.
Problem with that is, setting it up was within the DHCPSTATE_INIT case,
which is fine except that case is entered repeatedly whilst retrying the
initial broadcast.  Changing the XID every time meant that this happened:

  * broadcast initial request, XID1; go to state DHCPSTATE_SELECTING
  * listen for a very short time, time out, loop back to DHCPSTATE_INIT
  * broadcast initial request, XID2 (DIFFERENT), go to DHCPSTATE_SELECTING
  * listen for a short time, get an answer to the first broadcast - and
        reject it because it contains XID1!  WRONG BEHAVIOUR!

so the bugfix for that was to move the calculation outside of that loop.

Now, I had always intended that we retain the same XID for the duration of
a particular lease - I wasn't totally clear on that detail you mention
above from the RFC.  But you're right that getting a new XID each time you
go for a new part of the transaction is a good thing - I mean retrying the
same packet should not get a new XID, but each new call to do_dhcp()
certainly should - and perhaps moving to the next phase of the protocol
should also.

So what I'm going to do is this: make the new XID generation at the entry
to the routine get the ESA correctly every time, and also additionally
change XID as we progress though the state machine (but NOT when we retry
sending and listening in the same part of the state machine).

I'll post a patch here when it's done and tested.

	- Huge


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]