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.
|
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 :-)
|
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!
|
Wed 06 Oct 2010 11:40:49 AM UTC, comment #9:
... and fixed again with the code proposed in the last comment.
|
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).
|
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.
|
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?
|
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?
|
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.
|
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.
|
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.
|
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;);
|
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,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;
}
|