[task #13508] Implement IPv4 Address Conflict Detection

classic Classic list List threaded Threaded
23 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
URL:
  <http://savannah.nongnu.org/task/?13508>

                 Summary: Implement IPv4 Address Conflict Detection
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: goldsimon
            Submitted on: Mi 04 Mär 2015 21:11:40 GMT
                Category: None
         Should Start On: Mi 04 Mär 2015 00:00:00 GMT
   Should be Finished on: Mi 04 Mär 2015 00:00:00 GMT
                Priority: 5 - Normal
                  Status: None
                 Privacy: Public
        Percent Complete: 0%
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None
                  Effort: 0.00

    _______________________________________________________

Details:

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.




    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/task/?13508>

_______________________________________________
  Nachricht gesendet von/durch Savannah
  http://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #1, task #13508 (project lwip):

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

Additional Item Attachment:

File name: 2018-07-25_ACD_patches.zip     Size:41 KB


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #2, task #13508 (project lwip):

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

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #3, task #13508 (project lwip):

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.

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #4, task #13508 (project lwip):

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

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #5, task #13508 (project lwip):

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.


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #6, task #13508 (project lwip):

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

Additional Item Attachment:

File name: 0033-Issue-1-from-Comment-5-task-13508-solved.patch Size:1 KB
File name: 0034-Issue-2-3-from-Comment-5-task-13508-solved.patch Size:5 KB


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #7, task #13508 (project lwip):

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

Additional Item Attachment:

File name: 0001-DHCP-add-ACD-module-in-DHCP-squached-commits.patch Size:28 KB


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #8, task #13508 (project lwip):

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

Additional Item Attachment:

File name: 0001-autoip-unused-random-function-removed-small-comment-.patch
Size:1 KB


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #9, task #13508 (project lwip):

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.

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #10, task #13508 (project lwip):

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

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #11, task #13508 (project lwip):

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

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #12, task #13508 (project lwip):

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

Additional Item Attachment:

File name: 0001-ACD-module-added-update-and-improve-DHCP-AUTOIP-beha.patch
Size:92 KB


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Update of task #13508 (project lwip):

         Planned Release:                    None => 2.2.0                  


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #13, task #13508 (project lwip):

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.

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #14, task #13508 (project lwip):

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

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #15, task #13508 (project lwip):

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.

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #16, task #13508 (project lwip):

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.

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #17, task #13508 (project lwip):

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

Additional Item Attachment:

File name: acd_fix.patch                  Size:6 KB


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[task #13508] Implement IPv4 Address Conflict Detection

Simon Goldschmidt
Follow-up Comment #18, task #13508 (project lwip):

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

Additional Item Attachment:

File name: 0001-ACD-AUTOIP-fix-compile-warnings.patch Size:7 KB


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/task/?13508>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
12