buglwIP - A Lightweight TCP/IP stack - Bugs: bug #21643, udp_send/raw_send don't fail if...

 
 

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

bug #21643: udp_send/raw_send don't fail if netif is down

Submitter:  Simon Goldschmidt <goldsimon>
Submitted:  Sun 25 Nov 2007 11:04:20 AM UTC
   
 
Category:  IPv4 Severity:  3 - Normal
Item Group:  Change Request Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  None

Jump to the original submission

Wed 28 Nov 2007 06:48:54 AM UTC, comment #17: 

An since task #6969 is nearly solved, I'll check in the patch to ip_route now, so netifs that are down aren't used for routing.

With that, this bug is fixed.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 27 Nov 2007 07:58:11 PM UTC, comment #16: 

I'll check it in, then:

- add udp_sendto_if: much like udp_sendto but takes a netif and doesn't call ip_route
- make dhcp use udp_sendto_if in all places

- ip_route: only return a netif that is up (I'll check this in later, first task #6969: 'Review usage of conn->err in netconn layer' must be solved; I'm working on it)

DHCP still has a problem with multiple interfaces (receiving DHCP is not solved correctly). I'll open a separate bug for that.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 27 Nov 2007 03:03:01 PM UTC, comment #15: 

Sounds like a plan to me.

Jonathan Larmour <jifl>
Group Member
Tue 27 Nov 2007 11:59:08 AM UTC, comment #14: 


>> We should check that ARP doesn't respond on ARP requests to its
>> own address until the netif is up, but my guess is this is
>> already implemented.
>
> It seems it is not implemented.


No it isn't. That's why I asked on lwip-devel (http://lists.nongnu.org/archive/html/lwip-devel/2007-11/msg00302.html)

> So I wonder if the real problem (and fix) is in ip_route().


Could be better...

> At the same time, if such a fix prevents DHCP, maybe DHCP
> should be allowed a way to send with a particular netif. Even
> now, I don't think DHCP could work reliably in lwIP if you have
> more than one netif, so identifying the netif to use seems
> wise.


Which means we would need a function like udp_sendto_if() which behaves like udp_sendto but gives a netif to be used? I think that would be the best solution after all.

Then we only would have to check for NETIF_FLAG_UP in ip_route() and everyting would be OK (as long as ERR_RTE is a minor error).

Simon Goldschmidt <goldsimon>
Group administrator
Tue 27 Nov 2007 04:40:33 AM UTC, comment #13: 


>For DHCP, other stacks have the concept of being half-up


It seems a good way to process such cases

>So I wonder if the real problem (and fix) is in ip_route().


Yes, perhaps do the filter in ip_route could be more efficient that  in ip_output_if

Frédéric Bernon <fbernon>
Group Member
Mon 26 Nov 2007 10:33:44 PM UTC, comment #12: 


> We should check that ARP doesn't respond on ARP requests to its
> own address until the netif is up, but my guess is this is
> already implemented.


It seems it is not implemented.

For DHCP, other stacks have the concept of being half-up. Usually the interface is in fact marked up at this point, but a user should not be using it yet as its state is indeterminate. Of course the routing table would prevent any normal packets using it.

So I wonder if the real problem (and fix) is in ip_route().

At the same time, if such a fix prevents DHCP, maybe DHCP should be allowed a way to send with a particular netif. Even now, I don't think DHCP could work reliably in lwIP if you have more than one netif, so identifying the netif to use seems wise.


Jonathan Larmour <jifl>
Group Member
Mon 26 Nov 2007 05:04:34 PM UTC, comment #11: 


>> This shouldn't impact things like DHCP because they're not IP packets
>
> Are you sure of that? I check with a DHCP capture, and they are UDP packets.


Yep, same here. All the DHCP packets I ever saw were IP packets ;-)

And ARP won't send on its own: it asks for MAC addresses only if the destination address is a unicast address. We should check that ARP doesn't respond on ARP requests to its own address until the netif is up, but my guess is this is already implemented.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 26 Nov 2007 04:20:45 PM UTC, comment #10: 


>This shouldn't impact things like DHCP because they're not IP packets


Are you sure of that? I check with a DHCP capture, and they are UDP packets.

Frédéric Bernon <fbernon>
Group Member
Mon 26 Nov 2007 04:01:02 PM UTC, comment #9: 

It should be illegal to send any IP packet until the interface is up, so we shouldn't need to special case broadcast or multicast.  This shouldn't impact things like DHCP because they're not IP packets.  ARP is also not IP, so we might need some way to prevent them as well, but are they really calling through ip_output_if()?  That function sounds like it should be for IP packets only.

Kieran Mansley <kieranm>
Group Member
Sun 25 Nov 2007 12:58:15 PM UTC, comment #8: 


>Dunno, not using that ;-) What would you suggest? Is it legal to send multicast packets without having an IP address?


as legal than send broadcast packets ? :)

>Anyway, before including anything like this, we first have to finish task #6969.


Agree.

Frédéric Bernon <fbernon>
Group Member
Sun 25 Nov 2007 12:55:59 PM UTC, comment #7: 

Dunno, not using that ;-) What would you suggest? Is it legal to send multicast packets without having an IP address?

Anyway, before including anything like this, we first have to finish task #6969.

Simon Goldschmidt <goldsimon>
Group administrator
Sun 25 Nov 2007 12:50:45 PM UTC, comment #6: 

And about multicast ?

Frédéric Bernon <fbernon>
Group Member
Sun 25 Nov 2007 12:49:30 PM UTC, comment #5: 


> Can post here your patch?


I'm not really sure what to check: netif->ip_addr != 0 or (netif->flags & NETIF_FLAG_UP) != 0 but I always allow sending when the destination ip address is BROADCAST.

I do this check right at the beginning of ip_output_if and return ERR_IF (maybe ERR_RTE is better) if it's not allowed.

Oh, the code size growth is minimal (2 or 3 if-checks):

if (!((dest->addr == INADDR_NONE) ||
      (((netif->flags & NETIF_FLAG_UP) != 0)) && (netif->ip_addr.addr != 0))) {
    return ERR_IF; /* or ERR_RTE */
}

Simon Goldschmidt <goldsimon>
Group administrator
Sun 25 Nov 2007 12:44:18 PM UTC, comment #4: 

BTW: adding a check in ip_output_if() to send only if dest==BROADCAST or the netif is up (or netif->ip_addr != 0), we can safely reproduce a bug that task #6969 (Review usage of conn->err in netconn layer) should fix :-)

-> udp_send doesn't work at all after it failed once, since conn->err is != ERR_OK

Simon Goldschmidt <goldsimon>
Group administrator
Sun 25 Nov 2007 12:39:58 PM UTC, comment #3: 


>I'm not concerned about lost packets but about ARP requests like "Who has 208.67.222.222? Tell 0.0.0.0". To me, this looks like an error!


I'm not sure it's an error (non-compliance with any RFC).

Can post here your patch?

Frédéric Bernon <fbernon>
Group Member
Sun 25 Nov 2007 12:24:56 PM UTC, comment #2: 

I'm not concerned about lost packets but about ARP requests like "Who has 208.67.222.222? Tell 0.0.0.0". To me, this looks like an error!

lwIP sends packets like this when dhcp is configured (and started) and dns tries to resolve something before dhcp is finished (which leads to an arp query in this case).

Simon Goldschmidt <goldsimon>
Group administrator
Sun 25 Nov 2007 11:59:09 AM UTC, comment #1: 

Since the packet could be also lost on the wire, do that will just increase the "lw"IP size.

I'm not sure to see the problem on the ping example. Can you give more informations ?

Frédéric Bernon <fbernon>
Group Member
Sun 25 Nov 2007 11:04:20 AM UTC, original submission:  

While this is required for dhcp, normal UDP pcbs (like the ping example) should fail.

Maybe only broadcast sends should be allowed while the local IP is 0.0.0.0 or while the netif is down?

Simon Goldschmidt <goldsimon>
Group administrator

 

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

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jifl (Posted a comment)
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by fbernon (Posted a comment)
  • -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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-11-28 goldsimon CategoryNone IPv4
    2007-11-28 goldsimon StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2007-11-27 goldsimon StatusNone In Progress
        Assigned toNone goldsimon

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code