> Sorry if I'm being dense...so, this one?

Yes

Michael,

No, the version from git master is the current development, which there is no release for. The 2.0.3 release is a branch from master before any of the features you needed were added.

Michael,

If you only need recvmsg and IP_PKTINFO (the IPv4 implementation), that is already complete in the current development sources (on git master).  You'll need to enable LWIP_NETBUF_RECVINFO for IP_PKTINFo to work

Michael,

Which part of the change are you inquiring about?

The remaining task I’d like to work on is an IPv6 version of IP_PKTINFO before closing this. I believe an implementation is available in patch #9554, but it needs separation from the large changeset that was posted there

No, it's fine like that. I'll just remove the "in progress" marker then. I'm trying to get a hold on what's missing for 2.1.0 and reading your post, it doesn't make sense to wait for this task to be finished.

This is marked as "in progress", any update here or should we remove the marker?

I've pushed a change adding a cast via void pointer (which is what Linux does :)

Travis CI for clang is showing the follow error:

../../../../lwip/src/../test/unit/api/test_sockets.c:530:13: error: cast from 'u8_t *' (aka 'unsigned char *') to 'struct in_pktinfo *' increases required alignment from 1 to 4 [-Werror,-Wcast-align]
  pktinfo = (struct in_pktinfo*)CMSG_DATA(cmsg);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../../lwip/src/../test/unit/api/test_sockets.c:536:10: error: cast from 'u8_t *' (aka 'unsigned char *') to 'struct cmsghdr *' increases required alignment from 1 to 4 [-Werror,-Wcast-align]
  cmsg = CMSG_NXTHDR(&msg, cmsg);
         ^~~~~~~~~~~~~~~~~~~~~~~
../../../../lwip/src/include/lwip/sockets.h:162:11: note: expanded from macro 'CMSG_NXTHDR'
          (struct cmsghdr *)((u8_t *)(cmsg) + \

I think we might have a problem with using MEM_ALIGNMENT for ALIGN_H and ALIGN_D.

I'm somewhat confused why the error is generated for clang on x86_64.  I think fno-sanitize=alignment is forcing no unaligned access on x86_64, right?  If so, why is MEM_ALIGNMENT set to 1 for unix port (ports\unix\lib\lwipopts.h or ports\unix\minimal\lwipopts.h, not sure which the unit test is using)?

Second, I could set ALIGN_H and ALIGN_D to 4 which would be correct for architectures with 4 byte alignment, but what about architectures with 2 byte alignment?  I don't have much experience with these architectures.  cmsg_len (type socklen_t) could end up being a u32 and in a 2 byte architecture, does this need to start on a 4 byte boundary?  If so, then using MEM_ALIGNMENT would not work.

If we ever add a cmsg data type that is 64-bits, we'll need to up the alignment to 8, even though MEM_ALIGNMENT would still be 4 on architectures like ARM.

> It seemed minor to me since it was only a single check of: if (msg->msg_contrllen).

I rather thought about the 2nd indrection we get in standard receive because we create an (unused) struct msghdr. It's not a big deal, but if we find a decent way to separate it...

> Do you want me to go ahead and commit?

Yes :-)

That looks much better. Although I'm still not convinced we need the msg_control code when calling lwip_recvfrom, but let's optimize that later...

Ahh, now I see what you meant about ' enable NETCONN_FLAG_PKTINFO for netconns (add a function/define for that!)' that way we could maintain the existing LWIP_NETBUF_RECVINFO behavior.

I'll work on a new patch!

Thanks for the suggestion, I had also thought of doing a socket/netconn level flag, let me make sure I understand all the suggestions

> leave LWIP_NETBUF_RECVINFO as it was

I would still need to utilize the logic in recv_udp to store the address and the toaddr storage in netbuf.  Are you suggesting that I enable the toaddr storage all the time?  If I have access to toaddr and flags, I wouldn't need to do anything with LWIP_NETBUF_RECVINFO

> enable NETCONN_FLAG_PKTINFO for netconns (add a function/define for that!)

Add API to enable/disable PKTINFO.  If enabled at the netconn layer, toaddr and port are filled out in the netbuf

> disable NETCONN_FLAG_PKTINFO for sockets

By this you just mean it's off by default for sockets?  I would also think it would be off by default for netconns

 >  thing that looks strange is accessing the netconn's pcb without locking. Even if it's only a read access, chances are that the pcb already got deallocated. If not now, this might happen in the future. Isn't it enough to check the flag in the netbuf in this case?

Thanks for reminding me that PCB could be deallocated.  I think I can remove the check because your suggestion of relying on buf->flags having NETBUF_FLAG_DESTADDR set is enough.

I was thinking down the road when we add the IPv6 version, NETBUF_FLAG_DESTADDR would be set regardless of which version of PKTINFO socket option was set, but maybe checking the IP type via IP_IS_V4 or _IS_V6 is enough to determine which cmsg to output.  Maybe I can also check the domain in setsockopt and error if IP_PKTINFO is mixed with AF_INET6 (and vice versa for AF_NET with IPv6 option)

 > Shouldn't the new feature work with LWIP_NETBUF_RECVINFO==1, too?

I could make it work with this configuration as well, but was limiting the scope of the change.  The netbuf flags field would need to be enabled this configuration (currently it's not).  Also in relation to the above point about not accessing the PCB and checking NETBUF_FLAG_DESTADDR, with NETBUF_RECVINFO == 1, the flag would always bet set regardless of the IP_PKTINFO option, which would mean if a cmsg was provided during recvmsg, but IP_PKTINFO was off, the stack would still output PKTINFO in the cmsg.  Maybe this isn't a big deal since LWIP_NETBUF_RECVINFO == 1 always outputs the address

Attached is patch implementing CMSG infrastructure (only for recvmsg) and IP_PKTINFO socket option (including unit test)

I have a couple of things to discuss about the change, patch contains current approach:

1. For struct in_pktinfo, I went with the netbsd usage rather than Linux.  My thoughts here are that Linux has the additional ipi_spec_dst, which I believe will be set to the source address of the interface that received the packet.  Since this was going to be an additional lookup once we are outside of the LwIP core context, it didn't seem necessary.  Applications can still get this address from the interface index (we can implement SIOCGIFADDR if needed)

2. I re-used the LWIP_NETBUF_RECVINFO feature (rather than create a new option) to record the destination when the datagram is received, but I made a modification to the option.  I added a new subset of behavior.  If LWIP_NETBUF_RECVINFO is 2, the destination and port are only recorded when IP_PKTINFO is enabled.  This preserves the old API behavior of LWIP_NETBUF_RECVINFO == 1 and doesn't suffer the performance hit of recording on every received datagram.  I also enabled the netbuf flags for this case so we can check that the address was recorded since it's now dependent on IP_PKTINFO being enabled

3. I added an IP level so_option for PKTINFO (called SOF_PKTINFO).  This differs a little from the other options because it doesn't directly map from the SO_ equvialent because IP_PKTINFO if an IP level option.  I didn't see a better way to record this setting without introducing a separate flag

4. I changed lwip_recvfrom_udp_raw() to take struct msghdr because I need msg_control, msg_controllen, msg_flags and almost all members of the msghdr were being passed individually.  This also means that lwip_recvfrom() is more or less just calling recvmsg because it sets up struct msghdr

Thanks :)

(file #40412)

Socket unit tests now utilize recvmsg for both UDP and TCP.  So far no bugs in the implementation :)

> Do we have any mechanisms for testing the API yet?

I just tried to get started with this, have a look at test/unit/api and test/unit/arch.

I'd be happy to see more tests there, feel free to add them. That would be a much better place than contrib/apps/socket_examples (where the remote host is missing)

That would be great. I haven't started and don't know when I would have...

The actual recvmsg function is implemented, however, IP_PKTINFO/in_pktinfo is still missing (msg_control is not touched at all).

Feel free to add patches here :-)

Glad to see this as a task, recvmsg has been on my backlog for a while :)

I believe there is no open group standards for the message types and it ends up being fairly platform dependent.

Linux, NetBSD, and Windows use IP_PKTINFO, but struct in_pktinfo is different on the platforms.

Linux
NetBSD
Windows

FreeBSD has IP_RECVDSTADDR

For LwIP, I guess we get to pick which option and if IP_PKTINFO, the definition of struct in_pktinfo