buglwIP - A Lightweight TCP/IP stack - Bugs: bug #30038, dhcp_network_changed doesn't reset...

 
 

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

bug #30038: dhcp_network_changed doesn't reset AUTOIP coop state

Submitter:  Bill Auerbach <billauerbach>
Submitted:  Wed 02 Jun 2010 02:09:30 PM UTC
   
 
Category:  IPv4 Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  CVS Head

Jump to the original submission

Wed 06 Oct 2010 05:31:38 PM UTC, comment #12: 


> BTW: I almost released a product with that, too :-)


Well, I did release it - to a trade show where this would not have been detected.  I'm not sure why I didn't see this still and I'm bothered by it.  I did a lot of testing of cable changes, DHCP to static to AutoIp changes all around and didn't see it.

Since you're in dhcp_network_changed - if you want to be really complete with it, the following should be added:

if(!dhcp) {
  dhcp_inform(netif);
  return;
}

If there is no DHCP server running (dhcp_start wasn't used - e.g. application using static IP) then a network change will inform the network of the static IP address.  If you do this, one benefit is Windows will detect duplicate IPs and "balloon" a warning.

Bill Auerbach <billauerbach>
Wed 06 Oct 2010 05:18:56 PM UTC, comment #11: 

No, i didn't spot that from reading the code :-) it happened always in my scenario and then, from reading the code, it got quite clear that, at the point where the coop-state was set to off without stopping autoip, the two (dhcp and autoip) got out of sync and could run concurrently...

BTW: I almost released a product with that, too :-)

Simon Goldschmidt <goldsimon>
Group administrator
Wed 06 Oct 2010 05:05:26 PM UTC, comment #10: 

Re: #8 - I've done a lot of testing and have this dhcp/autoip installed in 4 products (with the additional feature to support DHCP fallback to static IP).  I did not see this behavior.  What is the frequency of occurrence (if you know or can guess)?  Did you see this happen - I assume so - if you saw this by reviewing the code kudos to you for picking it up!

Bill Auerbach <billauerbach>
Wed 06 Oct 2010 11:40:49 AM UTC, comment #9: 

... and fixed again with the code proposed in the last comment.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 06 Oct 2010 11:34:22 AM UTC, comment #8: 

I just discovered the patch didn't fix the bug - or introduced another bug, rather:

- start the device with DHCP/AutoIP-coop without a DHCP server
- wait until it has an AutoIP assigned (bound)
- start/connect the DHCP server
- pull and plug the cable
-> At this time, DHCP starts to run but AutoIP is still running/is restarted, too. Since DHCP does not stop AutoIP when setting autoip_coop_state to OFF, both continue to run. Then, if DHCP gets an address first, AutoIP will "win" since it sets its address later and DHCP does not recognize this.

Proposed fix (which I'll check in) is:

if(dhcp->autoip_coop_state == DHCP_AUTOIP_COOP_STATE_ON) {
  autoip_stop(netif);
  dhcp->autoip_coop_state = DHCP_AUTOIP_COOP_STATE_OFF;
}

(instead of only setting autoip_coop_state to OFF).

Simon Goldschmidt <goldsimon>
Group administrator
Mon 14 Jun 2010 08:52:30 PM UTC, comment #7: 

I have it working now so I can change Ethernet cables from static, to DHCP, to DHCP/AutoIP and I get the proper address for each wire without restarting the device.  I had to do this because field support was adding up as more lwIP devices go out.  Technicians and customers expect to change cables as they can do with PCs and not have to reboot.  We required a reboot of the device, and one of them doesn't have a power switch.  They weren't doing this and then calling when the device wouldn't connect.

Bill Auerbach <billauerbach>
Mon 14 Jun 2010 08:41:20 PM UTC, comment #6: 

Ah, I see. I didn't try that, yet, but I'd love to get that fixed in the next weeks before I ship my product, hehe.

Wouldn't it be enough to look at the DHCP/AutoIP-coop state? If DHCP is active and AutoIP-coop is active, too then retry DHCP after changing networks?

Simon Goldschmidt <goldsimon>
Group administrator
Mon 14 Jun 2010 08:22:11 PM UTC, comment #5: 

Problem: netif.flags is a u8_t and #define's exist from 0x01 to 0x80.  How can we add a #define e.g. for DHCP_AUTOIP_ADDR?  Just make flags a u16_t?  I don't think the size of a netif is critical if it grows but no sense in me sending a patch if you disagree.  If you do, how else can we signify DHCP settled for an AutoIP address?

Bill Auerbach <billauerbach>
Mon 14 Jun 2010 07:20:08 PM UTC, comment #4: 

Yep, I never meant it to be like it is now, thanks for checking. I must have been confused after checking the code and switching programs ;-) It should be OK now.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 14 Jun 2010 04:38:37 PM UTC, comment #3: 

The checked-in change is not the same as the patch and doesn't fix the problem.  if dhcp_start is called and provides an AutoIP address and then is called again (e.g. due to loss of link) it will provide the AutoIP address instead of retrying to find a DHCP server which then would provide an AutoIP address if no DHCP server responds.

Bill Auerbach <billauerbach>
Sat 12 Jun 2010 05:13:40 PM UTC, comment #2: 

Both fixed, thanks for reporting.

Although from the code, I would have expected the code just wouldn't start AutoIP and not get an address at all (instead of using the old DHCP address), the old code wasn't correct since DHCP could be restarted with AutoIP being still active, which also means that AutoIP cannot be restarted.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 03 Jun 2010 07:02:59 PM UTC, comment #1: 

An additional bug in dhcp.c:

A memory fault will occur before LWIP_ERROR can indicate the problem:

  struct dhcp *dhcp = netif->dhcp;
  LWIP_ERROR("dhcp_stop: netif != NULL", (netif != NULL), return;);

Bill Auerbach <billauerbach>
Wed 02 Jun 2010 02:09:30 PM UTC, original submission:  

The AutoIP coop state is not reset in dhcp_network_changed.  It's required to do so or you cannot change connections from DHCP to AutoIP and back to DHCP and have it change accordingly.  Without this patch when a link without DHCP is connected after a link that had DHCP lwIP continues to use the DHCP address.

Patch follows:

Index: dhcp.c
===================================================================
RCS file: /sources/lwip/lwip/src/core/dhcp.c,v
retrieving revision 1.125
diff -c -r1.125 dhcp.c
* dhcp.c 16 May 2010 15:57:44 -0000 1.125
--- dhcp.c 2 Jun 2010 13:59:46 -0000
*************
* 761,766 **
--- 761,769 ----
      break;
    default:
      dhcp->tries = 0;
+ #if LWIP_DHCP_AUTOIP_COOP
+     dhcp->autoip_coop_state = DHCP_AUTOIP_COOP_STATE_OFF;
+ #endif
      dhcp_discover(netif);
      break;
    }

Bill Auerbach <billauerbach>

 

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

No files currently attached

 

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 billauerbach (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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-10-06 goldsimon StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2010-10-06 goldsimon Planned Release 1.4.0
    2010-10-06 goldsimon CategoryNone IPv4
        StatusFixed In Progress
        Assigned toNone goldsimon
        Open/ClosedClosed Open
    2010-06-12 goldsimon StatusNone Fixed
    2010-06-12 goldsimon Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code