Re: [ECOS] DHCP xid

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

Hi Hugo,

Thanks for the explanation and the ChangeLog extract, these entries do
not appear in my version (from eCos release 1.5.2).

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?

Kind regards,

Robin

On Tue, 2002-01-29 at 15:06, Hugo Tyson wrote:
> 
> Robin Farine <acnrf@dial.eunet.ch> writes:
> > While updating my local eCos sources tree, I noticed a strange change in
> > the tcpip package: file src/lib/dhcp_prot.c, RCS version 1.10, routine
> > do_dhcp(). This routine always regenerates the XID, even in the case of
> > a lease renewal. Moreover, it uses values from the interface request
> > which isn't initialized at this point. If some nodes boot at the same
> > time and run the same release of their software, they will probably
> > generate the same XID, which ultimately results into duplicated IP
> > addresses on the network ...
> 
> Hi Robin, 
> 
> I remember putting in a change to prevent exactly that.  That's the added
> arc4random() call.  But...
> 
> > Could someone please give the rationale of this change, I've searched
> > the ChangeLog without success (I may have missed it, though).
> 
> You'll have to ask the ASCOM Dudes, I'm afraid.  The patches came from
> Anssi and Anand via Andrew - I merged them in in good faith around
> 2001-12-03 along with that one from Andrew; I don't remember the analysis.
> 
> Hmmm, you're right that it uses the hwaddr before it's been set; that's
> bogus; the SIOCGIFHWADDR in case DHCPSTATE_INIT is what gets the ESA to use
> there.  That's a bug.
> 
> At a glance I would have thought one wants to keep the same xid for a
> particular station for as long as you retain the same IP address - which is
> why it took care only to initialize it in case DHCPSTATE_INIT.
> 
> Over to ASCOM to have a chat and decide what to do - tell me why a new xid
> was generated every time?  Or I'll just change it back?
> 
> Relevent ChangeLogs follow FYI.
> 
> 	- Huge
> 
> 
>    2001-12-04  Jonathan Larmour  <jlarmour@redhat.com>
>    
>    	   * src/lib/bootp_support.c: Check CYGINT_ISO_DNS (from
>    	   <pkgconf/isoinfra.h>) instead of CYGPKG_ISO_DNS.
>    
>    2001-12-03   Andrew Lunn  <Andrew.Lunn@ascom.ch>
>    
>    	   * src/lib/bootp_support.c: get_bootp_option(): Take the
>    	   max length of data we want out of the bootp record.
>    	   * src/lib/dhcp_prot.c: Pass the length we expect for an option.
>    
> >  2001-08-16  Anssi Pulkkinen <Anssi.Pulkkinen@ascom.ch>
> >    
> >	   * src/lib/dhcp_prot.c: Make sure we use the newly calculated xid.
> >    
> >  2001-08-02  Anand Srivastava <Anand.Srivastava@ascom.ch>
> >    
> > 	   * src/lib/dhcp_prot.c do_dhcp: ID calculation must be done only
> > 	   once. moved it out the loop. 
>    
>    2001-11-21  Hugo Tyson  <hmt@redhat.com>
> 
>