-
Notifications
You must be signed in to change notification settings - Fork 3k
mbed-os/LwIP changes and fixes in auto-IP for Bonjour Conformance Test #11339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Changes: 1. Following issues are fixed in LwIP for AutoIP. a) Fixed bug in max conflict rate limitting. - According to RFC section RFC 3927 Section 2.2.1 conflict probe interval should be increased to 60 seconds, once conflict count reaches after MAX_CONFLICTS (i.e., 10) counts. - The initial value of 'autoip->tried_llipaddr' is 0. Hence the probe interval (i.e., autoip->ttw) should be increased to 60 secs when 'autoip->tried_llipaddr >= MAX_CONFLICTS' b) Added code to free 'autoip' client in autoip_stop() API. - New 'autoip' client is allocated in autoip_start() API, and the client is not freed during autoip_stop(). This would result in memory leak if not freed. - Updated autoip_stop() API to take care of releasing the memory allocated for 'autoip' client. 2. Introduced a configurable macro "MBED_CONF_LWIP_DHCP_TIMEOUT" in "lwipopts.h" to configure DHCP timeout based on the usecase requirement. For example: bonjour conformance test would need a DHCP timeout value which is grater than 320 secs to run mDNS probing test to verify protocol compilance of the implementation.
@AnttiKauppila for review. Please add more reviewers as required. Thanks. |
@sathishm6, thank you for your changes. |
Do changes in features/lwipstack/lwip/src/core/ipv4/lwip_autoip.c also live in the upstream repo or we are fixing them here and upstreaming later ? |
@0xc0170 - Hello Martin - I'm pushing the fixes first here in mbed-os. Following this, I plan to push the fix in LwIP repo as well. But the current code base in LwIP repo is little different than what we have here in mbed-os today. However, I am planning to apply these fixes in the new code base on LwIP in it's repo. Thanks. |
@@ -94,6 +94,9 @@ | |||
static err_t autoip_arp_announce(struct netif *netif); | |||
static void autoip_start_probing(struct netif *netif); | |||
|
|||
/* static variables */ | |||
static u8_t is_autoip_alloced = 0; /* Set when 'struct autoip' is allocated dynamically and assigned to 'netif' */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed? Isn't if (autoip == NULL)
enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LwIP provides a way for the caller to assign the static autoip client (structure). This can be achieved by calling autoip_set_struct() API. When the caller allocates the autoip client and assigns it to the netif interface by calling autoip_set_struct() API, LwIP doesn't allocate the autoip client in autoip_start(). Hence, if the autoip client (structure) is allocated by the caller and assigned it to netif interface by calling autoip_set_struct() API, then autoip structure shouldn't be freed in autoip_stop(), otherwise it should be freed during stop. This flag is used to differentiate the above case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks for the answer, could you fix the naming to ..._allocated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I was trying to keep the name shorter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AnttiKauppila - Renamed the flag. Committed in SHA#a4aeee941d263e0e5d05c561ca6689e581af214b
This PR is to fix the issues in LwIP for AutoIP which is required for passing Bonjour Conformance Test for mDNS. Following gives the summary of the changes/fixes added. Changes: 1. Following issues are fixed in LwIP for AutoIP. - Fixed bug in max conflict rate limiting: According to RFC section RFC 3927 Section 2.2.1 conflict probe interval should be increased to 60 seconds, once conflict count reaches after MAX_CONFLICTS (i.e., 10) counts. The initial value of 'autoip->tried_llipaddr' is 0. Hence the probe interval (i.e., autoip->ttw) should be increased to 60 secs when 'autoip->tried_llipaddr >= MAX_CONFLICTS' - Added code to free 'autoip' client in autoip_stop() API: New 'autoip' client is allocated in autoip_start() API, and the client is not freed during autoip_stop(). This would result in memory leak, if not freed. Updated autoip_stop() API to take care of releasing the memory allocated for 'autoip' client. 2. Introduced a configurable macro "MBED_CONF_LWIP_DHCP_TIMEOUT" in "lwipopts.h" to configure DHCP timeout based on the usecase requirement. For example: bonjour conformance test would need a DHCP timeout value which is grater than 320 secs to run mDNS probing test to verify protocol compilance of the implementation. Tested the fixes using Bonjour Conformance Test tool Version 1.5.0 for IPv4. It has successfully passed Bonjour Conformance Test.
CI started |
Test run: SUCCESSSummary: 11 of 11 test jobs passed |
Description
This PR is to fix the issues in LwIP for AutoIP which is required for passing Bonjour Conformance Test for mDNS. Following gives the summary of the changes/fixes added.
Changes:
Following issues are fixed in LwIP for AutoIP.
Introduced a configurable macro "MBED_CONF_LWIP_DHCP_TIMEOUT" in "lwipopts.h" to configure DHCP timeout based on the usecase requirement. For example: bonjour conformance test would need a DHCP timeout value which is grater than 320 secs to run mDNS probing test to verify protocol compilance of the implementation.
Tested the fixes using Bonjour Conformance Test tool Version 1.5.0 for IPv4. It has successfully passed Bonjour Conformance Test.
Pull request type
Reviewers
@AnttiKauppila
Release Notes