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

> > [much ado about the RFC's real meaning snipped]

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

Quite so.

I also realized that it kind of misused the renewal timeout; read the
ChangeLog for more explanation.

It still doesn't quite implement this bit of the RFC:

   In both RENEWING and REBINDING states, if the client receives no
   response to its DHCPREQUEST message, the client SHOULD wait one-half
   of the remaining time until T2 (in RENEWING state) and one-half of
   the remaining lease time (in REBINDING state), down to a minimum of
   60 seconds, before retransmitting the DHCPREQUEST message.

(instead we use the same timeout scheme as other states) ...but it's only a
SHOULD not a MUST, to do with the timeouts used for retrying.  The previous
version didn't either, of course, but the new one is better.  Now your
lease lasts the whole time even if the server disappears!

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

Thank you (and the ASCOMites also) for thinking clearly about this ;-)

Here we go.

	- Huge

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Index: ChangeLog
===================================================================
RCS file: /home/cvs/ecc/ecc/net/tcpip/current/ChangeLog,v
retrieving revision 1.1
diff -u -5 -p -r1.1 ChangeLog
--- ChangeLog	2002/01/29 02:16:58	1.1
+++ ChangeLog	2002/01/31 15:26:06
@@ -1,5 +1,33 @@
+2002-01-31  Hugo Tyson  <hmt@redhat.com>
+
+	* src/dhcp_prot.c (do_dhcp): Generate a new XID (transaction ID)
+	every time this routine is entered.  Use the ESA and a random
+	source to avoid clashes with other net presences.  (The ESA was
+	used uninitialized before this change.)  Also use new macro
+	NEW_XID to increment the XID when we move to a new phase of the
+	protocol - one XID covers a question (identically transmitted
+	several times if necessary) and its answer.  A new question => a
+	new XID.
+
+	Also fixed a gedankenbug about the timeouts.  Old version tried
+	RENEWING the lease when T1 timed out as it should; but if that
+	failed (after the normal retries), it went straight into REBINDING
+	whether or not T2 had already timed out.  Likewise if REBINDING
+	failed, it terminated the lease, whether or not the EX time had
+	timed out.  This is wrong - it meant that a lease of 600 seconds
+	would actually shutdown the interface after about 340 (T1 + some)
+	if the server was gone.
+
+	The fix is to return to state BOUND from either RENEWING or
+	REBINDING (without resetting the lease timeouts) even if the
+	attempt failed; the rest of the lease machinery will take care of
+	things as and when the right time ticks by.  It's still the case
+	that if a timeout occurs whilst trying on part or another, the
+	machine will be forced to the next state - all that is in order as
+	it always was.
+
 2002-01-28  Gary Thomas  <gthomas@redhat.com>
 
 	* tests/udp_lo_test.c: 
 	* tests/tftp_server_test.c: 
 	* tests/tftp_client_test.c: 
Index: src/dhcp_prot.c
===================================================================
RCS file: /home/cvs/ecc/ecc/net/tcpip/current/src/dhcp_prot.c,v
retrieving revision 1.1
diff -u -5 -p -r1.1 dhcp_prot.c
--- src/dhcp_prot.c	2002/01/29 02:16:58	1.1
+++ src/dhcp_prot.c	2002/01/31 15:26:06
@@ -577,21 +577,27 @@ do_dhcp(const char *intf, struct bootp *
     // be the neatest way to do it; it returns from within the switch arms
     // when all is well, or utterly failed.
 
     reset_timeout( &tv, &timeout_scratch );
 
-    xid = res->bp_xid; // default to what's there already;
-
-    // generates a new XID
-    {
-      unsigned char* xp = (unsigned char*)&xid;
-      
-      *xp++ = ifr.ifr_hwaddr.sa_data[5];
-      *xp++ = ifr.ifr_hwaddr.sa_data[4];
-      *((cyg_uint16*)xp) = (cyg_uint16)(arc4random() & 0xffff);
+    // Choose a new XID: first get the ESA as a basis:
+    strcpy(&ifr.ifr_name[0], intf);
+    if (ioctl(s, SIOCGIFHWADDR, &ifr) < 0) {
+        perror("SIOCGIFHWADDR 2");
+        return false;
     }
-      
+
+    // Choose from scratch depending on ifr_hwaddr...[]
+    xid = ifr.ifr_hwaddr.sa_data[5];
+    xid |= (ifr.ifr_hwaddr.sa_data[4]) << 8;
+    xid |= (ifr.ifr_hwaddr.sa_data[3]) << 16;
+    xid |= (ifr.ifr_hwaddr.sa_data[2]) << 24;
+    xid ^= (arc4random() & 0xffff0000);
+
+    // Avoid adjacent ESAs colliding by increment
+#define NEW_XID(_xid) CYG_MACRO_START (_xid)+= 0x10000; CYG_MACRO_END
+
     while ( 1 ) {
 
         // If we are active rather than in the process of shutting down,
         // check for any lease expiry every time round, so that alarms
         // *can* change the course of events even when already renewing,
@@ -626,15 +632,10 @@ do_dhcp(const char *intf, struct bootp *
 
         case DHCPSTATE_INIT:
 
             // Send the DHCPDISCOVER packet
 
-            if (ioctl(s, SIOCGIFHWADDR, &ifr) < 0) { /* get MAC address */
-                perror("SIOCGIFHWADDR 2");
-                return false;
-            }
-
             // Fill in the BOOTP request - DHCPDISCOVER packet
             bzero(xmit, sizeof(*xmit));
             xmit->bp_op = BOOTREQUEST;
             xmit->bp_htype = HTYPE_ETHERNET;
             xmit->bp_hlen = IFHWADDRLEN;
@@ -677,10 +678,11 @@ do_dhcp(const char *intf, struct bootp *
                 // No packet arrived (this time)
                 if ( seen_bootp_reply ) { // then already have a bootp reply
                     // Save the good packet in *xmit
                     bcopy( received, xmit, dhcp_size(received) );
                     *pstate = DHCPSTATE_BOOTP_FALLBACK;
+                    NEW_XID( xid ); // Happy to advance, so new XID
                     reset_timeout( &tv, &timeout_scratch );
                     break;
                 }       
                 // go to the next larger timeout and re-send:
                 if ( ! next_timeout( &tv, &timeout_scratch ) ) {
@@ -719,10 +721,11 @@ do_dhcp(const char *intf, struct bootp *
                     // Save the good packet in *xmit
                     bcopy( received, xmit, dhcp_size(received) );
                     // we like the packet, so reset the timeout for next time
                     reset_timeout( &tv, &timeout_scratch );
                     *pstate = DHCPSTATE_REQUESTING;
+                    NEW_XID( xid ); // Happy to advance, so new XID
                 }
             }
             else // No TAG_DHCP_MESS_TYPE entry so it's a bootp reply
                 seen_bootp_reply = 1; // (keep the bootp packet in received)
                 
@@ -816,10 +819,11 @@ do_dhcp(const char *intf, struct bootp *
                 }
                 if ( DHCPNAK == msgtype // Same server?
                      && received->bp_siaddr.s_addr == xmit->bp_siaddr.s_addr) {
                     // we're bounced!
                     *pstate = DHCPSTATE_INIT;  // So back the start of the rigmarole.
+                    NEW_XID( xid ); // Unhappy to advance, so new XID
                     reset_timeout( &tv, &timeout_scratch );
                     break;
                 }
                 // otherwise it's something else, maybe another offer, or a bogus
                 // NAK from someone we are not asking!
@@ -893,21 +897,25 @@ do_dhcp(const char *intf, struct bootp *
             break;
 
         case DHCPSTATE_RENEW_RECV:
             // wait for an ACK or a NACK - retry by going back to
             // DHCPSTATE_RENEWING; NACK means go to NOTBOUND.
+            // No answer means just wait for T2, to broadcast.
 
             setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
 
             addrlen = sizeof(rx_addr);
             if (recvfrom(s, received, sizeof(struct bootp), 0,
                          (struct sockaddr *)&rx_addr, &addrlen) < 0) {
                 // No packet arrived
                 // go to the next larger timeout and re-send:
                 if ( ! next_timeout( &tv, &timeout_scratch ) ) {
-                    reset_timeout( &tv, &timeout_scratch );
-                    *pstate = DHCPSTATE_REBINDING;
+                    // If we timed out completely, just give up until T2
+                    // expires - retain the lease meanwhile.  The normal
+                    // lease mechanism will invoke REBINDING as and when
+                    // necessary.
+                    *pstate = DHCPSTATE_BOUND;
                     break;
                 }
                 *pstate = DHCPSTATE_RENEWING;
                 break;
             }
@@ -946,11 +954,10 @@ do_dhcp(const char *intf, struct bootp *
                     *pstate = DHCPSTATE_BOUND;
                     break;
                 }
                 if ( DHCPNAK == msgtype ) { // we're bounced!
                     *pstate = DHCPSTATE_NOTBOUND;  // So quit out.
-                    reset_timeout( &tv, &timeout_scratch );
                     break;
                 }
                 // otherwise it's something else, maybe another offer.
                 // Just listen again, which implicitly discards it.
             }
@@ -987,21 +994,25 @@ do_dhcp(const char *intf, struct bootp *
             break;
 
         case DHCPSTATE_REBIND_RECV:
             // wait for an ACK or a NACK - retry by going back to
             // DHCPSTATE_REBINDING; NACK means go to NOTBOUND.
+            // No answer means just wait for expiry; we tried!
 
             setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
 
             addrlen = sizeof(rx_addr);
             if (recvfrom(s, received, sizeof(struct bootp), 0,
                          (struct sockaddr *)&rx_addr, &addrlen) < 0) {
                 // No packet arrived
                 // go to the next larger timeout and re-send:
                 if ( ! next_timeout( &tv, &timeout_scratch ) ) {
-                    reset_timeout( &tv, &timeout_scratch );
-                    *pstate = DHCPSTATE_FAILED;
+                    // If we timed out completely, just give up until EX
+                    // expires - retain the lease meanwhile.  The normal
+                    // lease mechanism will invoke NOTBOUND state as and
+                    // when necessary.
+                    *pstate = DHCPSTATE_BOUND;
                     break;
                 }
                 *pstate = DHCPSTATE_REBINDING;
                 break;
             }
@@ -1040,11 +1051,10 @@ do_dhcp(const char *intf, struct bootp *
                     *pstate = DHCPSTATE_BOUND;
                     break;
                 }
                 else if ( DHCPNAK == msgtype ) { // we're bounced!
                     *pstate = DHCPSTATE_NOTBOUND;  // So back the start of the rigmarole.
-                    reset_timeout( &tv, &timeout_scratch );
                     break;
                 }
                 // otherwise it's something else, maybe another offer.
                 // Just listen again, which implicitly discards it.
             }
@@ -1095,10 +1105,11 @@ do_dhcp(const char *intf, struct bootp *
             // lease for graceful shutdown.
 
             // Just send what you got with a DHCPRELEASE in the message
             // type UNICAST straight to the server.  No ACK.  Then go to
             // NOTBOUND state.
+            NEW_XID( xid );
             xmit->bp_xid = xid;
             xmit->bp_op = BOOTREQUEST;
             xmit->bp_flags = htons(0); // no BROADCAST FLAG
             // Use the *client* address here:
             xmit->bp_ciaddr.s_addr = xmit->bp_yiaddr.s_addr;


%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%


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