lwIP - A Lightweight TCP/IP stack - Bugs: bug #57706, dhcp/acd integration: acd is not...
You are not allowed to post comments on this tracker with your current authentication level.
bug #57706: dhcp/acd integration: acd is not stopped
Submitter: | Simon Goldschmidt <goldsimon> | ||
Submitted: | Fri 31 Jan 2020 11:30:20 AM UTC | ||
Category: | DHCP | Severity: | 3 - Normal |
Item Group: | Faulty Behaviour | Status: | Fixed |
Privacy: | Public | Assigned to: | goldsimon |
Open/Closed: | Closed | Planned Release: | 2.2.0 |
lwIP version: | git head |
( Jump to the original submission )
Fri 14 May 2021 07:29:43 AM UTC, comment #8: |
Simon Goldschmidt <goldsimon>![]() ![]() |
Fri 14 May 2021 05:53:00 AM UTC, comment #7: Fix updating prev pointer by
|
Axel Lin <axellin>![]() |
Thu 13 May 2021 12:31:46 AM UTC, comment #6: Seems missing the code to update *prev.
|
Axel Lin <axellin>![]() |
Wed 12 May 2021 07:08:45 PM UTC, comment #5: Fixed by adding 'adc_remove()' and calling it when stopping DHCP. |
Simon Goldschmidt <goldsimon>![]() ![]() |
Thu 04 Mar 2021 09:35:05 PM UTC, comment #4: Jasper,
I'm still not convinced acd_remove() is not required in dhcp_stop(): the next cll to acd_add() again on the next call to dhcp_start() succeeds because of special check for adding existing acd struct, but
a) we often execute some code just to see that the state is OFF and
b) I think that the counterpart to dhcp_start() is dhcp_stop(). As such, the logical counterpart to calling acd_add() from dhcp_start() should be calling acd_remove() from dhcp_stop().
I admit there's no memory issue when using dhcp_set_struct() (as I do it, too).
As a side note (for the 2.2.0 release): the autoip module does not seem to stop acd, too, so that will have to be fixed before the relase as well :-( |
Simon Goldschmidt <goldsimon>![]() ![]() |
Wed 06 May 2020 03:15:19 PM UTC, comment #3: I agree with the patch made and I applied it on top of the latest master. The attached patch does apply cleanly.
I'm not convinced that we should adc_remove() from dhcp_stop().
However the functions were we do unallocate memory should include something like adc_remove(). I'm thinking of dhcp_cleanup().
I do not use dhcp_cleanup() as I use dhcp_set_struct().
What do you think? |
Jasper Verschueren <jasperv> |
Sun 16 Feb 2020 08:16:14 PM UTC, comment #2: This is a blocker for 2.2.0, as ACD is one of the major changes between the 2.1.x branch and master. |
Simon Goldschmidt <goldsimon>![]() ![]() |
Sun 16 Feb 2020 08:15:27 PM UTC, comment #1: Also, acd_add() has no counterpart acd_remove() -> after calling dhcp_stop(), dhcp->acd is still on the netif->acd_list (which now points to possibly invalid memory) |
Simon Goldschmidt <goldsimon>![]() ![]() |
Fri 31 Jan 2020 11:30:20 AM UTC, original submission:
acd_stop() is completely missing in dhcp.c.
- if dhcp_stop() is called during ACD probing, the dhcp_conflict_callback() binds the netif to 0.0.0.0
- if dhcp_conflict_callback() is called again later because of ongoing defend, strange things might happen
--> at least acd_stop() must be called from dhcp_release_and_stop() (see attached patch which does not cleanly apply) |
Simon Goldschmidt <goldsimon>![]() ![]() |
Depends on the following items: None found
Items that depend on this one: None found
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 7 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2021-05-12 | goldsimon | Status | In Progress | ![]() |
Fixed |
Open/Closed | Open | ![]() |
Closed | ||
2021-03-04 | goldsimon | Status | None | ![]() |
In Progress |
Assigned to | None | ![]() |
goldsimon | ||
2020-05-06 | jasperv | Attached File | - | ![]() |
Added 0001-dhcp-stop-acd-module-when-stopping-dhcp.patch, #49013 |
2020-02-16 | goldsimon | Planned Release | None | ![]() |
2.2.0 |
2020-01-31 | goldsimon | Attached File | - | ![]() |
Added dhcp_stop_acd.patch, #48311 |
Yes, thanks for catching that!