tasklwIP - A Lightweight TCP/IP stack - Tasks: task #13508, Implement IPv4 Address Conflict...

 
 

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

task #13508: Implement IPv4 Address Conflict Detection

Submitter:  Simon Goldschmidt <goldsimon>
Submitted:  Wed 04 Mar 2015 09:11:40 PM UTC
   
 
Category:  None Should Start On:  Wed 04 Mar 2015 12:00:00 AM UTC
Should be Finished on:  Wed 04 Mar 2015 12:00:00 AM UTC Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Percent Complete:  100%
Open/Closed:  Closed Planned Release:  2.2.0
Effort:  0.00

Jump to the original submission

Fri 05 Oct 2018 06:58:09 AM UTC, comment #21: 

You're welcome! :)

Jasper Verschueren <jasperv>
Thu 04 Oct 2018 03:35:56 PM UTC, comment #20: 

Pushed Jasper's patch. Thank you very much for your contribution, Jasper!

Dirk Ziegelmeier <dziegel>
Group administrator
Thu 04 Oct 2018 08:26:52 AM UTC, comment #19: 

Steps to compile using CMake in the current git master:

- cd contrib/ports/<your_os>/example_app
- mkdir build
- cd build
- cmake ..
- cmake --build

The lwIP default warning settings can be found:

contrib/ports/CMakeCommon.cmake
or
contrib/ports/Common.allports.mk

I'm currently also creating a VS Code workspace (only helpful if you use VS Code, of course)

Dirk Ziegelmeier <dziegel>
Group administrator
Thu 04 Oct 2018 08:22:42 AM UTC, comment #18: 

The patch looks good. I found one more error in the /prot/acd.h in the enums. I attached a patch with your changes and the enum correction. I also removed the @param netif from acd_stop comments.

We are not using IPv6 so I did not check if it's dual stack safe.
I have enabled IPv6 on my system and made it compiling, but did not setup any IPv6 stuff. We are not planning on using IPv6. The ACD code is still functional but I never aqcuired a IPv6 address so I could not check how the ip_addr_isany handles this. Does this need more testing?

It took a while to find out how I should compile the lwip code with the lwip cmake in contrib. Maybe adding a small overview in a readme would be helpful :). Or is it somewhere and did I not find it?

We would like to use the same compile warning options as you, were can I find them?


(file #45137)

Jasper Verschueren <jasperv>
Wed 03 Oct 2018 07:34:43 AM UTC, comment #17: 

Hello Jasper,

I applied your patch locally, but it does not compile. I attached a patch that makes it work. Issues are:

- You did not write the code "dual-stack-safe" - please enable IPv6 support when compiling
- Please try to compile with lwIP GCC warning settings (See Makefiles). acd_stop() has an unused parameter "netif" and some "default:" statements are missing in switch/case.

Can you please review my patch and test your code with it, especially when IPv6 is enabled on your system?


(file #45129)

Dirk Ziegelmeier <dziegel>
Group administrator
Wed 03 Oct 2018 06:58:58 AM UTC, comment #16: 

Yes of course you can use the netif_set_addr functions and without ACD you don't need more then that for fixed IP.
Let me know if you stumble upon something else.

Jasper Verschueren <jasperv>
Tue 02 Oct 2018 08:09:29 PM UTC, comment #15: 

Ok, I guess most of comment #14 makes sense, except for:

>  lwIP does not have any support for fixed IP


Did you miss netif_set_ipaddr()/netif_set_netmask()? With DHCP disabled, this is your support for fixed IPs.

I know that this doesn't really integrate with ACD, because a callback on conflict is not supported.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 02 Oct 2018 04:31:06 PM UTC, comment #14: 

- it seems like autoip is never stopped in coop mode, why did you remove the coop state?

Autoip was stopped by dhcp in following cases:

  • Link goes up -> autoip now has it's own functions for link up and down. dhcp should not bother with the autoip link up down behavior.
  • dhcp_bind -> autoip should stay active when a valid routable address is acquired. This was already the case (more or less) because autoip_accept_packet stayed active. So actually the module was disabled while it was still receiving packets on that address. This is not the good approach. Now when a routable address is set on the netif the acd module of autoip will be put in passive mode. This means that conflict detection and autoip stays active but when a conflict occurs it's shut off.
  • dhcp_release_and_stop -> If you want to stop dhcp, their is no need to turn off autoip.


Generally I removed the coop state because dhcp should not influence the autoip behavior. To keep a state variable of autoip in the dhcp struct seems looking for trouble to me. The autoip state can change without dhcp knowing it, should we then make functions to update the state variable from autoip in the dhcp struct... The only thing that dhcp should do is start autoip if dhcp is failing. Also keeping the the address open for packets but shutting down the autoip module is not nice.

- it seems like autoip is disabled on link loss and never re-enabled. Why is that?

Autoip is always disabled on link down. This because their is no need to keep it enabled. When the link goes up, you need to restart it anyway. Autoip is (re)started on link up if the LWIP_DHCP_AUTOIP_COOP is off. In that situation autoip is on it's own. If LWIP_DHCP_AUTOIP_COOP is on, dhcp will need to restart it again if it fails to find a dhcp server. The link local address however is kept the same until their is a conflict found.

I would like (later on) to add a kind of address api that handles these kinds of priority levels in how the address should be acquired. It would also have fixed IP in their. I don't like when things are too much entangled. I don't think that dhcp should start autoip in the first place. My perference would be that the dhcp module signals to the address api that it is failing to find a server. Then the address api can decide what to do with that information. I do not have time to work on the address api for now but that is on my list.

- how is the new ACD module integrated with user-supplied static IPs? Your patch disables the single gratuitous ARP for static IPs, so that's another regression.

There is not really an integration with the static IPs because there are no static IP functions. If a user want's to use static IPs it can of course use the ACD module.
I see that the single gratuitous ARP is indeed disabled in the case you have ACD enabled and are using fixed ip without ACD. If you would be using it with ACD their is no problem of course. If you are using autoip or dhcp with ACD then why would you not use it on fixed ip? I did not think of that case because lwIP does not have any support for fixed IP. You would need to write your own management functions.
Could we leave it as in my patch? or do you have a suggestion? Also if autoip is enabled but ACD is not enabled on dhcp, it will not send this gratuitous arp for a dhcp address.
I do not directly see a solution for the fixed ip situation except adding the just mentioned address api. A fixed IP looks like a routable so we cannot do an easy check on the netif address. Using defines in the lwipopts.h also does not work, because it can be changed at run time.
For the dhcp situation this can simply be solved with an extra check on the LWIP_DOES_ACD_CHECK and the type of the new address.
If we have a solution for the fixed ip situation, I will create a patch combining both dhcp and fixed ip solutions.

Jasper Verschueren <jasperv>
Tue 02 Oct 2018 12:30:26 PM UTC, comment #13: 

Reviewing this, I do have some comments. Overall, the code looks good, however:
- it seems like autoip is never stopped in coop mode, why did you remove the coop state?
- it seems like autoip is disabled on link loss and never re-enabled. Why is that?
- how is the new ACD module integrated with user-supplied static IPs? Your patch disables the single gratuitous ARP for static IPs, so that's another regression.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 14 Sep 2018 08:12:37 AM UTC, comment #12: 

Attached you can find the complete ACD module patch. So all patches previously send are included in this patch.

I updated the code as in the proposal from comment #9.
You can find some of my thoughts in the commit messages or comments.

Please let me know if you have any comments on the code. I'm open to discuss them and update were needed.

I hope we can finish this topic soon.

(file #45008)

Jasper Verschueren <jasperv>
Tue 11 Sep 2018 06:47:26 AM UTC, comment #11: 

:) Together we are stronger :)

The format patch is, if I remember correctly, made with the last version of the lwIP master. I will combine all patches previously sent in one patch when I finish the last part.

Jasper Verschueren <jasperv>
Mon 10 Sep 2018 02:30:10 PM UTC, comment #10: 

Did I say 'must' anywhere? But for lwIP, I guess you can say those 'should' and 'may' are 'must' ;-)

But anyway, I'm glad your Ethernet test consultant seems to do a good job ;-) Your proposals from comment #9 are good I think!

The patches look good overall, although I can't apply file #44909. Can you make sure patches you send apply to known revisions of the files? I had problems before doing this on tortoise-git, so it could be my problem here, as well...

Simon Goldschmidt <goldsimon>
Group administrator
Mon 10 Sep 2018 01:50:37 PM UTC, comment #9: 

First of all are the patches I sent good? Or do you have any comments?

We discussed topic 4 from comment #5 with our Ethernet test consultant.

The acd module should indeed be running on both autoip and dhcp but we cannot announce the LL address once a valid dhcp address is present. In the implementation from before my patches, the ongoing conflict detection was just stopped once a routable address was obtained. But the netif kept listening to this unmaintained address. That is not acceptable. In the comments it's said that we MUST continue to listen to the LL address and that we MUST keep open connections, this is actually not true. These are a should and a may, not musts. I do think they are a good practice so they should be implemented in a good way.
I would like to make the module so that it acts logically and as proposed in the RFC (see comment #6).

My proposal:

  • Add a function to etharp so we can send a probe or announce message independent of netif->ip_addr. This will enable us to send a probe message for a new routable address while the Link Local address is still active. We will not need to put the netif->ip_addr to ANY.
  • As Simon already asked we will indeed need to add the ACD module to both autoip and dhcp but some changes will need to be done to the acd module so only the address used by the netif is announced/defended.
  • We need to update the acd module to accommodate following user case: so the interface was previously configured with an IPv4 LL address and now has a routable address. We want the LL address still active and it should keep doing conflict detection. But once a conflict is seen, it should not defend it's address it should just stop listening to the LL address and stop all open tcp pcb's. We will change the behavior of the autoip_accept_packet function. Once a conflict is found on the LL address and we have a valid routable address, we will not select a new LL address.


I will start with the changes today. When these changes are implemented the acd module should be perfectly in accordance with the RFC and should be working nicely with autoip and dhcp.

If you have any comments to my proposal or the already sent patches please let me know.

Jasper Verschueren <jasperv>
Mon 03 Sep 2018 11:42:42 AM UTC, comment #8: 

Attached you can find a patch for the cleanup of the autoip.c file.
The unused random define (was used for timing) is removed + a small commenting error is solved.

(file #44926)

Jasper Verschueren <jasperv>
Fri 31 Aug 2018 03:06:03 PM UTC, comment #7: 

I finished the ACD module.
DHCP check is replaced with the ACD module and should work as stated in RFC5227.
Some small changes are done the the ACD to coop with the differences between the autoip and dhcp (autoip is simpler). The ACD module itself is kept as generic as possible.



(file #44909)

Jasper Verschueren <jasperv>
Mon 27 Aug 2018 02:46:28 PM UTC, comment #6: 

Thanks for the feedback. I solved the first 3 comments, you can find the patches attached.

As for the 4th issue I do not agree. We would contaminate the caches of other hosts in the network when we defend the LL address while we obtained a routable address. I believe this was a bug in the previous version of autoip.
My opinion is of course not that important. But it is stated in the RFC 3927 about link local addresses. See section 1.9 page 9 point 2.

>If a host finds that an interface that was previously        configured with an IPv4 Link-Local address now has an operable          routable address available, the host MUST use the routable          address when initiating new communications, and MUST cease          advertising the availability of the IPv4 Link-Local address          through whatever mechanisms that address had been made known to          others.  The host SHOULD continue to use the IPv4 Link-Local          address for communications already underway, and MAY continue          to accept new communications addressed to the IPv4 Link-Local          address.


So since defending the address is advertising it, we do not need the ACD module running on more then one address.

I will start implementing the ACD module in dhcp this week.

(file #44875, file #44876)

Jasper Verschueren <jasperv>
Mon 13 Aug 2018 07:29:54 PM UTC, comment #5: 

OK, from reading the code, it looks mostly ok. Here are my thoughts:
- etharp_input: don't respond to "from_use" -> how do we defend? This needs a description in etharp_input
- some C++ comments slipped in
- drop the //--- lines
- we need 2 instances when using DHCP and AutoIP or else defend/retreat won't work on an AutoIP which is in passive use after getting a DHCP address
  -> e.g. embed the ACD struct into struct AutoIP and DHCP and don't call directly into the AutoIP module from etharp_input.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 06 Aug 2018 08:29:43 PM UTC, comment #4: 

Ok, wait, I'll apply all your patches and make the diff myself.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 06 Aug 2018 08:09:11 PM UTC, comment #3: 

Ok, your files are 10 days old and i must admit they slipped through.

Could you please provide a smaller set of files? To look at your work, I don't think we need the actual history of how you got there.

Also, please upload patch files, not ZIP files.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 06 Aug 2018 11:47:05 AM UTC, comment #2: 

I would like to continue with the integration of ACD in DHCP.
Does anyone have comments?

Jasper Verschueren <jasperv>
Wed 25 Jul 2018 08:54:09 AM UTC, comment #1: 

Hi All,

I started with the implementation of ACD in preparation to make lwIP 'zero configuration networking' compatible.
RFC 5227 on ACD is an extraction from the RFC 3927 on auto ip. The method is extracted out of the auto ip RFC because it's not only applicable on link local addresses. So they wanted to make it a bit more generic. The ACD module can be used for an LL address, a dhcp address or even a fixed address.

As previously suggested by Simon (in email), I'm making a stand alone module for ACD. Because auto ip is already implemented in lwIP, I used that module a lot as basis for the ACD module. I added the copyright text and kept the auto ip developer in it because it's partially his code. I have no experience with copyright texts, so do not know if it's the correct approach.

The patches that you can find attached follow following steps:
- create "empty" ACD module
- start pulling ACD parts from auto ip
- improve / correct the auto ip code used in ACD
- change auto ip so it uses the ACD module
- remove all ACD functions, variables and defines from auto ip
(Not every patch can be compiled.)

The code is written so clients that are using auto ip at the moment do no need to do anything. The ACD module will be activated when auto ip is.

At the moment their is one ACD module per netif. When an LL address is configured and a dhcp server comes available, the ACD module should start running for the dhcp address. Although the LL address stays available for incoming messages, it will not be used to open new connections so that is why I believe its not needed to keep running the ACD on the LL address. The RFC's are a bit unclear on this point, input is welcome.

The code isn't finished yet, I need to embed it with the dhcp client. At this moment it only works with auto ip. But I first would like some input on the written code.

The ACD module is completely independent, so not tied up into other modules. It is made so you can start the ACD module from auto ip, dhcp or from your home made fixed ip client. You call acd_start() with the netif, the address you would like to check and a callback function. This callback function is used by ACD to keep the client up to date on conflict information. This can be that after probing no conflict occurred -> address is good to use. Or it can be that a conflict occurred.

Enough talk :)...
Let me know if you have any questions.

(file #44619)

Jasper Verschueren <jasperv>
Wed 04 Mar 2015 09:11:40 PM UTC, original submission:  

See RFC 5227. We currently send one gratuitous ARP request only.

This could be integrated into etharp.c.

ACD is e.g. required by Ethernet/IP, so not an Apple-only thing.

Simon Goldschmidt <goldsimon>
Group administrator

 

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

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dziegel (Updated the item)
  • -email is unavailable- added by jasperv (Updated the item)
  • -email is unavailable- added by goldsimon (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 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-10-04 dziegel StatusNone Done
        Percent Complete0% 100%
        Open/ClosedOpen Closed
    2018-10-04 jasperv Attached File- Added 0001-ACD-AUTOIP-fix-compile-warnings.patch, #45137
    2018-10-03 dziegel Attached File- Added acd_fix.patch, #45129
    2018-10-02 dziegel Planned ReleaseNone 2.2.0
    2018-09-14 jasperv Attached File- Added 0001-ACD-module-added-update-and-improve-DHCP-AUTOIP-beha.patch, #45008
    2018-09-03 jasperv Attached File- Added 0001-autoip-unused-random-function-removed-small-comment-.patch, #44926
    2018-08-31 jasperv Attached File- Added 0001-DHCP-add-ACD-module-in-DHCP-squached-commits.patch, #44909
    2018-08-27 jasperv Attached File- Added 0033-Issue-1-from-Comment-5-task-13508-solved.patch, #44875
        Attached File- Added 0034-Issue-2-3-from-Comment-5-task-13508-solved.patch, #44876
    2018-07-25 jasperv Attached File- Added 2018-07-25_ACD_patches.zip, #44619

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code