Re: [lwip-devel] Extended udp_recv callback

[Kieran Mansley]

On Wed, 2009-01-14 at 11:36 +0100, Jakob Stoklund Olesen wrote:
> Kieran Mansley wrote:

> > We wouldn't be able to do the full version of that, no, but we might be
> > able to do a reduced set, or something similar.  I think it's worthy of
> > further investigation, even if only to make the extended API that is
> > being discussed a bit more like the recvmsg and cmsg API.
> 
> Something like recvmsg() would have to be implemented at the socket
> layer, right? 

Yes.  I'm keen that anything we do at the raw API layer though be
mindful of what the layers above might need.

> 
> Here is what I propose:
> 
> I will add this to opt.h:
> 
> /**
>  * UDP_RAW_RECV_CALLBACK==1: Enable the udp_set_raw_recv_callback()
>  * API function. This makes it possible to register UDP receive
>  * callbacks with access to the IP header and receiving interface.
>  */
> #ifndef UDP_RAW_RECV_CALLBACK
> #define UDP_RAW_RECV_CALLBACK           0
> #endif
> 
> 
> I will create an extra UDP callback prototype in udp.h:
> 
> #if UDP_RAW_RECV_CALLBACK
> /** UDP raw receive callback function.
>  *
>  * This is an alternative UDP callback for applications that need
>  * detailed information from the IP layer such as destination IP
>  * address, TTL etc. A udp_raw_recv_callback_t is set by calling
>  * udp_set_raw_recv_callback() instead of udp_recv().
>  *
>  * port is in host byte order, iphdr is untouched, and all fields are
>  * in network byte order. The IP source address is available as
>  * iphdr->src.
>  *
>  * The callback is responsible for freeing the pbuf if it's not used
>  * any more.
>  *
>  * @param arg user supplied argument (udp_pcb.recv_arg)
>  * @param pcb the udp_pcb which received data
>  * @param p the packet buffer that was received
>  * @param port the remote port from which the packet was received
>  * @param iphdr the raw IP header of the packet
>  * @param netif the interface that received the packet
>  */
> typedef void (*udp_raw_recv_callback_t)(
>     void *arg,
>     struct udp_pcb *pcb,
>     struct pbuf *p,
>     u16_t port,
>     const struct ip_hdr *iphdr,
>     struct netif *netif);
> #endif /* UDP_RAW_RECV_CALLBACK */
> 
> 
> And the following implementation in udp_input():
> 
> 
> #if UDP_RAW_RECV_CALLBACK
>         if (pcb->flags & UDP_FLAGS_RAW_RECV_CALLBACK) {
>           ((udp_raw_recv_callback_t)pcb->recv)(
>             pcb->recv_arg, pcb, p, src, iphdr, inp);
>         } else
> #endif /* UDP_RAW_RECV_CALLBACK */
>         {
>           pcb->recv(pcb->recv_arg, pcb, p, &(iphdr->src), src);
>         }

Looks fine to me.

Couple of style points:
 - let's try and keep our naming consistent.  At the moment we have
udp_recv() to set the recv callback (note no "callback" in that function
name), so try to add something that logically follows from that.  
 - I'm not a big fan of using "raw" again: it has lots of other meanings
already (raw API, raw socket, etc) and so having another use will only
confuse things.

> I am not very familiar with the lwIP socket layer -- I am not using it.
> I believe you could implement most of the recvmsg() functionality by
> extending struct netbuf to also hold ip_hdr and netif pointers. The
> ip_hdr wouĺd give you access to IP_TOS, IP_TTL, IP_OPTIONS, and
> IP_RECVSTADDR.

Would anyone with an interest in future changes/improvements to the
sockets API care to comment on this? 

> Unfortunately I cannot offer to write recvmsg() for you, but I believe
> the udp_raw_recv_callback_t provides the proper foundation to do so.

No need to write recvmsg()!  I just want to make sure that if someone
else did want to do that (and other people have talked at length about a
new sockets API) the raw API would be able to support them, so saving
some work in the long run.

Kieran