patchlwIP - A Lightweight TCP/IP stack - Patches: patch #9165, Allowing udp src port to be 0 in...

 
 

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

patch #9165: Allowing udp src port to be 0 in cases when we don't care about outgoing port

Submitter:  Alex Moran <alex081293>
Submitted:  Thu 17 Nov 2016 10:25:35 PM UTC
   
 
Category:  UDP Priority:  5 - Normal
Status:  Invalid Privacy:  Public
Assigned to:  None Open/Closed:  Closed
Planned Release:  None

Jump to the original submission

Tue 22 Nov 2016 09:42:55 PM UTC, comment #8: 

I believe I realize why it's a bad idea. I was not considering that from a successful udp_connect, you are assuming that communication to/from that address:port are eligible. By using connect with port 0, you would not able to send (with normal udp_send) to the remote IP, only receive. You would need to use udp_sendto and specify a different port. For this reason, I can see why it'd be a bad solution, although I do think the problem still exists. I appreciate you all entertaining the idea.

Alex Moran <alex081293>
Tue 22 Nov 2016 09:01:36 PM UTC, comment #7: 


> As I stated before, it is impossible to connect a pcb to an ip address
> without knowing the senders outgoing port.


Receiving without knowing the sender outgoing port is the udp_bind() behavior. What you need is a filter on source IP address which can currently only be done in your udp_recv callback.

udp_connect(pcb, &cb, 0); means for me that you want to connect/send a packet to a random remote port, it does not make sense indeed but a 0 port is special and is globally known as "chose for me" and never as "don't filter input packet on source port".

What you could ask is a udp_filter_source_ip() function but it will then never be available for the Socket APIā€¦

Sylvain

Sylvain Rochet <gradator>
Group Member
Tue 22 Nov 2016 08:41:48 PM UTC, comment #6: 


> It seems unnecessary to allow every Ip address to reach the
> application layer and require the application layer to do the > filtering.


That's true.

To me, it's not only the thought about how many people actually use this. What's even more important is that everything we do in the core API is done for the socket API as well, and we have to keep compatibility there. In my opinion, we can't just allow connecting to port 0 there.

For such changes, I'm open to implement something broadly used (give me examples from other stacks if there are no RFCs to follow), but I'm not fond of implementing our own thing: if we're lucky and it doesn't conflict whith other people's ideas, we still have the problem that nobody knows this shortcut exists (unless they deeply study lwIP), so not many people can take advantage of this.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 22 Nov 2016 08:31:52 PM UTC, comment #5: 

I apologize if this is the wrong area for this discussion. I didn't think it was a bug because I was presenting a solution to a problem. I would be happy to discuss this in the proper location.

While the end goal of my application was to receive multicast traffic, the patch was first testing unicast traffic, and is valid for all types of udp communication.

As I stated before, it is impossible to connect a pcb to an ip address without knowing the senders outgoing port. If the packet is being received and processed at the udp layer and then dropped over the senders outgoing port, why not make it an optional requirement to be passed up the pcb layer? Port 0 is already an invalid port to connect to, it is intended to a be a wildcard. It seems unnecessary to allow every Ip address to reach the application layer and require the application layer to do the filtering.

Is the argument that the extra check on if the remote port is 0 is an over unnecessary check of the library and it is correct to place the burden on the application to maintain the philosophy of being as barebones and light-weight as possible? Since, for example, only 5% of users may benefit for this, it is hurting the performance the 95% of all other users?  If so, I respect that can can see how additional checks and configuration to the library can add up and eventually hurt the integrity of LWIP of being as light weight as possible.

Alex Moran <alex081293>
Tue 22 Nov 2016 07:41:53 PM UTC, comment #4: 

Me neither.

To OP: if you really filed this because you were trying to find a way how to only receive datagrams sent to a specific multicast IP, it would probably have been better to ask for solutions to this problem than to post a patch like this.

"udp_connect(this->pcb, &RecvIPAddress), 0);" should have made us suspicious enough ;-)

Simon Goldschmidt <goldsimon>
Group administrator
Tue 22 Nov 2016 02:07:01 PM UTC, comment #3: 

Ah!, I didn't understand why that was merged in and I was too lazy to argue about it :-)

Anyway, I fully agree, this is what PPPoL2TP is doing in lwIP:
http://git.savannah.gnu.org/cgit/lwip.git/tree/src/netif/ppp/pppol2tp.c#n353

Sylvain

Sylvain Rochet <gradator>
Group Member
Tue 22 Nov 2016 01:37:19 PM UTC, comment #2: 

Reverted, same behavior can be achieved by NOT calling udp_connect() and comparing source address to be RecvIPAddress.

Not worth changing UDP PCB semantics.

Dirk Ziegelmeier <dziegel>
Group administrator
Fri 18 Nov 2016 07:19:46 AM UTC, comment #1: 

Thanks for the improvement!

Dirk Ziegelmeier <dziegel>
Group administrator
Thu 17 Nov 2016 10:25:35 PM UTC, original submission:  

In the case of knowing the senders IP address, the host running LWIP should be able to open a socket on their known local port and be able to receive packets from the known sender without caring about its source port.

With the current implementation of the UDP stack, it is impossible to receiving a packet from only the intended source without first knowing it's outgoing port. I believe you should be able to specify if you do not care about the outgoing port of the sender but only want to receive packets from that address like so:

udp_bind(this->pcb, IP_ADDR_ANY, LocalPort);
udp_connect(this->pcb, &RecvIPAddress), 0);

The patch I'd like to add is to change line 278 of the current /src/core/udp.c file to:
((pcb->remote_port == src) || (pcb->remote_port == 0)) &&
     

Without this change, the packet gets to the udp layer but is dropped. This allows the packet to be passed up to the application layer regardless of what the remote port is.

Alex Moran <alex081293>

 

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

    Date Changed by Updated Field Previous Value => Replaced by
    2016-11-22 dziegel StatusDone Invalid
    2016-11-18 dziegel StatusNone Done
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code