buglwIP - A Lightweight TCP/IP stack - Bugs: bug #19927, DHCP NACK problem

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #19927: DHCP NACK problem

Submitter:  Cui hengbin <phant>
Submitted:  Tue 22 May 2007 07:08:33 AM UTC
   
 
Category:  None Severity:  2 - Minor
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Closed Planned Release:  None
lwIP version:  None

Jump to the original submission

Fri 25 Jan 2008 03:06:37 AM UTC, comment #16: 

Hi All:
     Sorry for response later.I check out the 1.30 rc1. find this problem(DHCP NAK) resolve well.it should be closed now.

     Thanks.

Cui hengbin <phant>
Fri 11 Jan 2008 10:02:23 AM UTC, comment #15: 

Fine by me.

Cui, since you've been able to reproduce DHCP NAKs, it would be nice if you could confirm it works too.

Jonathan Larmour <jifl>
Group Member
Thu 10 Jan 2008 10:19:07 PM UTC, comment #14: 

I tried to produce the NACK case, without any success on my platform. But I suppose it's good. Should we close it ?


Frédéric Bernon <fbernon>
Group Member
Sun 06 Jan 2008 09:06:45 PM UTC, comment #13: 

Yes, since Cui says it works for him/her this should be fine.

I've done more research into answering my question at the end of comment #8, and looking at the RFC I believe it is safe to just start discovery immediately, rather than waiting for the 10 second timeout.

RFC2131 does talk about using exponential backoffs, but only for retransmission, and this wouldn't be one. Theoretically this could also be part of an attempt to introduce a random delay, but then we don't try and introduce that delay at the first request at all, and that's when it's needed most, so that couldn't really have been the rationale (and the delay is fixed at 10secs, not random anyway). I've also looked at a different open source implementation of DHCP, and it doesn't have any introduced delay (not that that implementation is necessarily correct!).

So in the patch I've made dhcp_handle_nak() now call dhcp_discover() directly, and no longer wait 10 seconds, which is what I'll now check in.


(file #14757)

Jonathan Larmour <jifl>
Group Member
Sun 06 Jan 2008 08:01:06 PM UTC, comment #12: 

Is the suggestion in comment #8 check in ?

Frédéric Bernon <fbernon>
Group Member
Mon 03 Dec 2007 12:41:39 AM UTC, comment #11: 

yes,I use the Comment 8#,it works well for me,thanks.

Cui hengbin <phant>
Sun 02 Dec 2007 02:39:47 PM UTC, comment #10: 

I think the suggestion Jonathan made in comment #8 looks quite good.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 29 Aug 2007 10:46:41 AM UTC, comment #9: 


> The problem seems valid, but I'm not sure the fix is.


That was my feeling too.  As a result I think this should be postponed till post-1.3.0 when someone with better understanding of the DHCP code can look at it.

Kieran Mansley <kieranm>
Group Member
Wed 29 Aug 2007 10:41:56 AM UTC, comment #8: 

The problem seems valid, but I'm not sure the fix is.

From RFC2131 ( http://www.faqs.org/rfcs/rfc2131.html ) section 3.2.3: " If the client receives a DHCPNAK message, it cannot reuse its remembered network address."

That's not just for the DHCPDISCOVER (as per the original bug report) but also for a dhcp_release. We should simply stop using the address, not do a release. So I'm not sure, but I think dhcp_handle_nak should include:
  netif_set_down(netif);
  /* remove IP address from interface */
  netif_set_ipaddr(netif, IP_ADDR_ANY);
  netif_set_gw(netif, IP_ADDR_ANY);
  netif_set_netmask(netif, IP_ADDR_ANY);

Is there any reason the state machine is then only advanced in dhcp_timeout(). Couldn't we call dhcp_discover directly?

Jonathan Larmour <jifl>
Group Member
Wed 29 Aug 2007 09:37:00 AM UTC, comment #7: 

I'm agree with Cui comment #4: "I means when receiving nack dhcp client should release the old Ip information and discovery new one ,Are you agree?". It seems logical.

But, I just come back to the office after my vacation, and I don't take time yet to read this RFC.

Frédéric Bernon <fbernon>
Group Member
Wed 29 Aug 2007 09:18:07 AM UTC, comment #6: 

Frederic: did you look in the RFC?  Any opinions from others on this patch?  I have no objections to it in principle, but haven't looked at the details.

Kieran Mansley <kieranm>
Group Member
Tue 22 May 2007 07:46:21 AM UTC, comment #5: 

I have attach the patch file. About your remark, I will look inside the RFC to see the "normal" behavior expected...

(file #12823)

Frédéric Bernon <fbernon>
Group Member
Tue 22 May 2007 07:41:12 AM UTC, comment #4: 

Ok,I attack my changed dhcp.c .I means when receiving nack dhcp client should release the old Ip information and discovery new one ,Are you agree?

Thanks.

(file #12822)

Cui hengbin <phant>
Tue 22 May 2007 07:25:48 AM UTC, comment #3: 

So, attach your dhcp.c file, I will produce the patch file (but you can also download the diff tools ;) )

Frédéric Bernon <fbernon>
Group Member
Tue 22 May 2007 07:18:40 AM UTC, comment #2: 

Sorry,I can't genreate a patch file
:)

Cui hengbin <phant>
Tue 22 May 2007 07:12:49 AM UTC, comment #1: 

Can you send us a patch file ?

Frédéric Bernon <fbernon>
Group Member
Tue 22 May 2007 07:08:33 AM UTC, original submission:  

Hi All:
      I use dhcp in my project with a CableModem.when starting the TCP/IP ,CM give me a ip address through DHCP,But when CM is on line ,CM send NACK to me,so I must discover again .But In dhcp_hanfrl_nak() simple set state with DHCP_BACKING_OFF,let dhcp_timeout to discover,when sniffering the network,I find discover UDP broadcast with the old IP adress .is not "0.0.0.0",so dhcp server may not reply.
  so I change the dhcp_timeout follow:

static void dhcp_timeout(struct netif *netif)
{
  struct dhcp *dhcp = netif->dhcp;
  LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | 3, ("dhcp_timeout()\n"));
  /* back-off period has passed, or server selection timed out */
  if (dhcp->state == DHCP_BACKING_OFF) {
 dhcp_release(netif);
dhcp_discover(netif);
}else if(dhcp->state == DHCP_SELECTING) {
    LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE, ("dhcp_timeout(): restarting discovery\n"));
    dhcp_discover(netif);
  /* receiving the requested lease timed out */
  } else if (dhcp->state == DHCP_REQUESTING) {
    LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_STATE, ("dhcp_timeout(): REQUESTING, DHCP request timed out\n"));
    if (dhcp->tries <= 5) {
      dhcp_select(netif);
    } else {
      LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_STATE, ("dhcp_timeout(): REQUESTING, releasing, restarting\n"));
      dhcp_release(netif);
      dhcp_discover(netif);
    }
  /* received no ARP reply for the offered address (which is good) */
  }.........
.........
}

Can You agree?

Cui hengbin <phant>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #14757:  dhcp.nak.patch added by jifl (2KiB - text/x-patch)
file #12823:  dhcp.patch added by fbernon (916B - text/x-patch)
file #12822:  dhcp.c added by phant (55KiB - text/x-csrc)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by jifl (Posted a comment)
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by fbernon (Posted a comment)
  • -email is unavailable- added by phant (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-01-25 kieranm StatusPostponed Fixed
        Open/ClosedOpen Closed
    2008-01-10 fbernon Planned Release 1.3.0
    2008-01-06 jifl Attached File- Added dhcp.nak.patch, #14757
    2007-08-29 kieranm StatusNone Postponed
    2007-08-29 kieranm Severity3 - Normal 2 - Minor
    2007-05-22 fbernon Attached File- Added dhcp.patch, #12823
    2007-05-22 phant Attached File- Added dhcp.c, #12822

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code