recv_from() returns incorrect remote {ipaddr, port} tuple for udp rx packet data

[simonqhughes]

recv_from() returns incorrect remote {ipaddr, port} tuple for udp rx packet data

Problem:

• udp_echo_client_orig.cpp test sends/received udp packet data
• when receiving data, the recv_from() can be of the following form: err = api->recv_from(&s, (void*) ((uintptr_t) data + rx_bytes), &len, &rxaddr, &rxport);
• The serial console trace is stored in the following file: iotsfw_num_tbd_console_trace.txt
• During the test, the target sends a 2048 byte udp msg payload.
• On the send, the udp frame is fragmented into 2 IP frames as the udp message is larger than mtu (see wireshark trace).
• Host pc receives 2 IP packets, IP reassembles and UDP layer delivers to full udp msg to python udp server handler (see sal_udpserver.py)
• sal_udpserver.py sends 2048 bytes udp msg back to target. Host IP stack fragments into 2 IP packets. See pcap trace.

• Target received 2 IP packets but lwip layer doesnt reassemble the 2 ip packets into the udp msg but instead delivers 2 udp msg fragments via 2 calls to recv_from(). The first recv_from() correctly recovers the ipaddr:port of the remote host pc transmitter. However the second recv_from() call fails to report the correct ipaddr:port tuple, as shown in the following trace:

  :FIX_LINE:>>> MBED TARGET: len=1472 rxaddr:rxport=0.0.ffff0000.15cb020a:49192
  :FIX_LINE:Entered recv_from() do loop, timedout=0, client_rx_done=1
  :FIX_LINE:MBED: here1
  :FIX_LINE:MBED: here2
  :FIX_LINE:about to call recv_from()
  :FIX_LINE:>>> MBED TARGET: len=576 rxaddr:rxport=0.0.ffff0000.80805c8:2562
  :FIX_LINE:socket_api_test_echo_client_connected_udp_test:818 rc!=0 [FAIL]

  first line reports remote ipaddr:port as 15cb020a:49192, which is correct sixth line reports remote ipaddr:port as 80805c8:2562, which is incorrect

  This behaviour is a BUG (BUG01).

• The following describes behaviour of the lwip stack which developers will find confusing. UDP implementation "normally" preserve msg boundaries or discard large UDP datagrams if a component IP fragment is lost, for example. However, lwip appears to have a different UDP behaviour.

  The target received 2 IP packets but lwip doesnt reassemble the 2 ip packets into the udp msg. Instead lwip delivers 2 udp msg fragments via 2 calls to recv_from(). UDP implementations normally preserve message boundaries (see Comparison of Transport protocols here: https://en.wikipedia.org/wiki/Transport_layer# Comparison_of_transport_layer_protocols) but this is not a requirement of rfc1122, for example. Udp message boundaries will be preserved and the sockets layer will present the whole datagram in one recv_from() call, provided the received data buffer is less than the maximum datagram size for the system (see rfc 1122 Sec 4 and 3.4, GET_MAXSIZES()), where the minimum value of GET_MAXSIZES() is the min size of IP packets (576 bytes) less the size of the IP/UDP packet headers. i.e. However, rfc1122 does not state that UDP message boundaries will be preserved always, but rather says:

  "Therefore, an application program running over UDP must deal directly with end-to-end communication problems that a connection-oriented protocol would have handled -- e.g., retransmission for reliable delivery, packetization and reassembly, flow control, congestion avoidance, etc., when these are required. The fairly complex coupling between IP and TCP will be mirrored in the coupling between UDP and many applications using UDP."

  This is possibly one of those cases where e.g. the BSD reference implementations have become the defacto standard.

• The following describes the callback handler API which developers may find unsatisfactory.

  The provided udp_echo_client_orig.cpp/socket_abstract_test.cpp provide samples give an example of whats outlined below.

  socket_types.h defines this:

  typedef void (*socket_api_handler_t)(void);

  socket_api.h defines this which uses the handler:

    typedef socket_error_t (*socket_create)(struct socket *socket,
            const socket_address_family_t af, const socket_proto_family_t pf,
            socket_api_handler_t const handler);  

The problem is that the handler had no context argument (better to be void* ctx rather than void), so ctx can be cast to a type to access variable shared between the callback and the mainline code. The disadvantages of this are:

• developers have to use global variables throughout the code to share state variables e.g.

in socket_abstract_test.cpp:
        static struct socket *client_socket;
        static volatile bool client_event_done;
        static volatile bool client_rx_done;
        static volatile bool client_tx_done;
        static volatile struct socket_tx_info client_tx_info;
        static volatile struct socket_event client_event

• multiple handler functions following a single template have to be coded to use specific global variables relevant to the handlers client. Using a context promotes re-usability of a single template handler function whose behaviour changes depending on ctx->state variables.

Reproducing BUG01 Problem:

• get a clone of sal-stack-lwip
• get a clone of sal
• yt link sal into sal-stack-lwip
• replace socket_abstract_test.cpp and sal_api_test.h in sal with the files attached to this ticket
• add sal_udpserver.py to sal-stack-lwip/test/host_tests/
• add udp_echo_client_orig.cpp to sal-stack-lwip/test/host_tests/udp_echo_client_orig/udp_echo_client_orig.cpp
• yt build
• mbedhtrun -v -d E: -f ".\build\frdm-k64f-gcc\test\sal-stack-lwip-test-udp_echo_client_orig.bin" -p COM40:9600 -C 4 -c default -m K64F -e test/host_tests

Traces similar to the following will be captured:

• iotsfw_num_tbd_console_trace.txt
• iotsfw_num_tbd_sal_stack_lwip_recv_from_returns_incorrect_sockaddr_info_for_remote ipaddr_port_tuple_20160113.pcapng

defect_report.txt iotsfw_num_tbd_console_trace.txt

iotsfw_num_tbd_sal_stack_lwip_recv_from_returns_incorrect_sockaddr_info_for_remote ipaddr_port_tupl_20160113_pcapng.txt iotsfw_num_tbd__20160113_pcapng.txt

sal_test_api_h.txt sal_udpserver_py.txt socket_abstract_test_cpp.txt udp_echo_client_orig_cpp.txt

[ciarmcom]

ARM Internal Ref: IOTSFW-1788

[bremoran]

@niklas-arm, I believe you've seen something similar to this. Did you have a fix in mind?

[simonqhughes]

Hi A further observation is that if the udp socket create(), bind(), connect() idiom is used then after the connect the socket is not bound correctly to the interface, but rather the local address is still reported as INADDR_ANY, as shown in the following console trace:

MBED: remote udp server:10.2.203.21:49659
MBED: udp local after bind():0.0.0.0:49153
MBED: udp local after connect():0.0.0.0:49153
MBED: udp_ec_tx_rx_from_test:289 ret!=0 [FAIL]
HOST: Received 32 bytes of data
HOST: Sent 32 bytes of data
 MBED: [FAIL] connect() failed as connected local address (0.0.0.0) doesnt match ethernet i/f ip address 10.2.203.161)

How to reproduce this problem:

• use the udp_echo_client.cpp test which will be submitted in a PR soon.

Best Simon

[bremoran]

Fixed in #48