Re: [ECOS] DHCP xid

[Robin Farine <acnrf at dial dot eunet dot ch>]

On Wed, 2002-01-30 at 18:48, Hugo Tyson wrote:
> 
> 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.

Me too!

> 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).

Yup, this time it will perfectly implement what the rfc says about the
XID (pfuuii!!! hope it will also *work* perfectly, but let's trust a
quasi standard ...).

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

Thanks a LOT!

> 	- Huge

Robin