buglwIP - A Lightweight TCP/IP stack - Bugs: bug #46524, Device as dhcp server does not...

 
 

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

bug #46524: Device as dhcp server does not work after upgrade to lwip-1.4.1

Submitter:  Axel Lin <axellin>
Submitted:  Wed 25 Nov 2015 06:48:35 AM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  1.4.1

Jump to the original submission

Wed 24 Feb 2016 09:14:38 PM UTC, comment #7: 

Fixed by accepting local match for dst==255.255.255.255

Simon Goldschmidt <goldsimon>
Group administrator
Fri 19 Feb 2016 08:59:48 AM UTC, comment #6: 

The patch checks dst for correct subnet (subnet-broadcasts) and src for NULL. I'd instead check for correct subnet or dst==255.255.255.255. Additionally, for dst==255.255.255.255 but local_ip not ANY, a pcb matching the inp-subnet should be the best bet. This adds additional checks...

I think this is the correct fix. Having src==ANY is just a very special case of the DHCP protocol.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 19 Feb 2016 02:56:43 AM UTC, comment #5: 

Hi,
The patch actually already checks both dest and src.

diff --git a/src/core/udp.c b/src/core/udp.c
index 4a30010..63b3c74 100644
--- a/src/core/udp.c
+++ b/src/core/udp.c
@@ -269,7 +269,8 @@ udp_input(struct pbuf *p, struct netif *inp)
              ip_get_option(pcb, SOF_BROADCAST) &&
 #endif /* IP_SOF_BROADCAST_RECV */
              (ip_addr_isany(&pcb->local_ip) ||
-              ip_addr_netcmp(&pcb->local_ip, ip_current_dest_addr(), netif_ip4_netmask(inp))))))
+              ip_addr_netcmp(&pcb->local_ip, ip_current_dest_addr(), netif_ip4_netmask(inp)) ||
+              ip_addr_isany_val(*ip_current_src_addr())))))
 #endif /* LWIP_IPV4 */
               ) {
           local_match = 1;


Below is the snip of the code after apply the patches:
broadcast must be true before other checking:

            (broadcast &&
#if IP_SOF_BROADCAST_RECV
             ip_get_option(pcb, SOF_BROADCAST) &&
#endif /* IP_SOF_BROADCAST_RECV */
             (ip_addr_isany(&pcb->local_ip) ||
              ip_addr_netcmp(&pcb->local_ip, ip_current_dest_addr(), netif_ip4_netmask(inp)) ||
              ip_addr_isany_val(*ip_current_src_addr())))))
#endif /* LWIP_IPV4 */
              ) {
          local_match = 1

Axel Lin <axellin>
Group Member
Thu 18 Feb 2016 07:59:15 PM UTC, comment #4: 

Right. Sorry for comment #2 :-)

Anyway, I think the correct fix is to check for dst==255.255.255.255 instead of src==0.0.0.0.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 29 Jan 2016 02:52:50 PM UTC, comment #3: 

Actually, I already tried the master tree when I reported this issue.
The attached patches are against master tree, because I found v1.4.1 is not
updated for years, so I make the patch against master tree.

Axel Lin <axellin>
Group Member
Fri 29 Jan 2016 08:43:42 AM UTC, comment #2: 

I think I remember changes somewhere in this area since 1.4.1, could you try master? (or wait for a 1.5.0 release candidate?)

Simon Goldschmidt <goldsimon>
Group administrator
Thu 26 Nov 2015 01:56:14 AM UTC, comment #1: 

The device is running a dhcp server.

The dhcp client (PC) sends a "DHCP Request" to destination 255.255.255.255.
But the device's local_ip is 192.168.10.1 with netmask 255.255.255.0.
Current code fails on such case (local_match is 0) so netconn_recv() never get the dhcp request packet.

I think the problem is the ip_addr_netcmp() returns FALSE in this case.
ip_addr_netcmp(&pcb->local_ip, ip_current_dest_addr(), &inp->netmask)

comments?

Axel Lin <axellin>
Group Member
Wed 25 Nov 2015 06:48:35 AM UTC, original submission:  

hi,
My device's dhcp server does not work after upgrade from lwip-1.4.0rc1 to lwip-1.4.1.

In my case, the device is running as an AP and dhcp server.
So I use PC to connect this device and do some configuration.
After upgrading to lwip-1.4.1, the PC can associate the AP but fail to
get ip from the dhcp server.

I found the bad commit is:
commit 4849eb4c543ea4ddc9869ee51698d4760d107cb0
Author: Simon Goldschmidt <goldsimon@gmx.de>
Date:   Fri Sep 9 22:25:59 2011 +0200

    fixed bug #34072: UDP broadcast is received from wrong UDP pcb if udp port matches

Below is my run time variable status:
broadcast is true
ip_addr_isany(&pcb->local_ip) is false
ip_addr_netcmp(&pcb->local_ip, ip_current_dest_addr(), &inp->netmask) is false

&pcb->local_ip: 192 .168 .10 .1
ip_current_dest_addr(): 255 .255 .255 .255
&inp->netmask: 255 .255 .255 .0

It works if I did below changes:

diff --git a/src/core/udp.c b/src/core/udp.c
index 32c7d38..1c2733b 100644
--- a/src/core/udp.c
+++ b/src/core/udp.c
@@ -246,13 +246,13 @@ udp_input(struct pbuf *p, struct netif *inp)
            ip_addr_ismulticast(&current_iphdr_dest) ||
 #endif /* LWIP_IGMP */
 #if IP_SOF_BROADCAST_RECV
-            (broadcast && ip_get_option(pcb, SOF_BROADCAST) &&
+            (broadcast && ip_get_option(pcb, SOF_BROADCAST) /*&&
              (ip_addr_isany(&pcb->local_ip) ||
-              ip_addr_netcmp(&pcb->local_ip, ip_current_dest_addr(), &inp->netmask)))) {
+              ip_addr_netcmp(&pcb->local_ip, ip_current_dest_addr(), &inp->netmask))*/)) {
 #else /* IP_SOF_BROADCAST_RECV */
-            (broadcast &&
+            (broadcast /*&&
              (ip_addr_isany(&pcb->local_ip) ||
-              ip_addr_netcmp(&pcb->local_ip, ip_current_dest_addr(), &inp->netmask)))) {
+              ip_addr_netcmp(&pcb->local_ip, ip_current_dest_addr(), &inp->netmask))*/)) {
 #endif /* IP_SOF_BROADCAST_RECV */
           local_match = 1;
           if ((uncon_pcb == NULL) &&

Axel Lin <axellin>
Group Member

 

(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 goldsimon (Posted a comment)
  • -email is unavailable- added by axellin (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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-02-24 goldsimon StatusNone Fixed
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2016-02-18 goldsimon StatusNeed Info None
    2016-01-29 goldsimon StatusNone Need Info
    2015-11-27 axellin Attached File- Added 0001-udp-Simplify-the-if-IP_SOF_BROADCAST_RECV-guard-in-u.patch, #35570
        Attached File- Added 0002-udp-Allow-matching-0.0.0.0-source-address-for-UDP-br.patch, #35571

    Back to the top

    Powered by Savane 3.13-bb6a.
    Corresponding source code