mbox series

[liburing,0/5] multishot recvmsg docs and example

Message ID 20220726121502.1958288-1-dylany@fb.com (mailing list archive)
Headers show
Series multishot recvmsg docs and example | expand

Message

Dylan Yudaken July 26, 2022, 12:14 p.m. UTC
Some multishot recvmsg patches for liburing:

Patches 1-3  cleanup the API a little while we're doing this.
Patch 4 adds docs for the new API
Patch 5 adds an example UDP echo server that uses the API


Dylan Yudaken (5):
  more consistent multishot recvmsg API parameter names
  order like functions together in liburing.h
  change io_uring_recvmsg_payload_length return type
  add documentation for multishot recvmsg
  add an example for a UDP server

 .gitignore                            |   1 +
 examples/Makefile                     |   1 +
 examples/io_uring-udp.c               | 388 ++++++++++++++++++++++++++
 man/io_uring_prep_recvmsg.3           |  30 ++
 man/io_uring_prep_recvmsg_multishot.3 |   1 +
 man/io_uring_recvmsg_cmsg_firsthdr.3  |   1 +
 man/io_uring_recvmsg_cmsg_nexthdr.3   |   1 +
 man/io_uring_recvmsg_name.3           |   1 +
 man/io_uring_recvmsg_out.3            |  78 ++++++
 man/io_uring_recvmsg_payload.3        |   1 +
 man/io_uring_recvmsg_payload_length.3 |   1 +
 man/io_uring_recvmsg_validate.3       |   1 +
 src/include/liburing.h                |  48 ++--
 13 files changed, 529 insertions(+), 24 deletions(-)
 create mode 100644 examples/io_uring-udp.c
 create mode 120000 man/io_uring_prep_recvmsg_multishot.3
 create mode 120000 man/io_uring_recvmsg_cmsg_firsthdr.3
 create mode 120000 man/io_uring_recvmsg_cmsg_nexthdr.3
 create mode 120000 man/io_uring_recvmsg_name.3
 create mode 100644 man/io_uring_recvmsg_out.3
 create mode 120000 man/io_uring_recvmsg_payload.3
 create mode 120000 man/io_uring_recvmsg_payload_length.3
 create mode 120000 man/io_uring_recvmsg_validate.3


base-commit: 30a20795d7e4f300c606c6a2aa0a4c9492882d1d

Comments

Jens Axboe July 26, 2022, 4:23 p.m. UTC | #1
On Tue, 26 Jul 2022 05:14:57 -0700, Dylan Yudaken wrote:
> Some multishot recvmsg patches for liburing:
> 
> Patches 1-3  cleanup the API a little while we're doing this.
> Patch 4 adds docs for the new API
> Patch 5 adds an example UDP echo server that uses the API
> 
> 
> [...]

Applied, thanks!

[1/5] more consistent multishot recvmsg API parameter names
      commit: c025f206a8d7337109f148a738bf5df75aeed494
[2/5] order like functions together in liburing.h
      commit: b569b365c3c7d4ac78e8c2f482d5a227a980977f
[3/5] change io_uring_recvmsg_payload_length return type
      commit: 464ed8e15b3dbec2b44e087c4620dcffdc28a753
[4/5] add documentation for multishot recvmsg
      commit: 06f979baaae2c5a17f6b4233cb1f1d6720378149
[5/5] add an example for a UDP server
      commit: 61d472b51e761e61cbf46caea40aaf40d8ed1484

Best regards,
Ammar Faizi July 26, 2022, 4:32 p.m. UTC | #2
On 7/26/22 11:23 PM, Jens Axboe wrote:
> [5/5] add an example for a UDP server
>        commit: 61d472b51e761e61cbf46caea40aaf40d8ed1484

This one breaks clang-13 build, I'll send a patch.
Jens Axboe July 26, 2022, 4:40 p.m. UTC | #3
On 7/26/22 10:32 AM, Ammar Faizi wrote:
> 
> 
> On 7/26/22 11:23 PM, Jens Axboe wrote:
>> [5/5] add an example for a UDP server
>>        commit: 61d472b51e761e61cbf46caea40aaf40d8ed1484
> 
> This one breaks clang-13 build, I'll send a patch.

Hmm, built fine with clang-13/14 here?
Ammar Faizi July 26, 2022, 4:48 p.m. UTC | #4
On 7/26/22 11:40 PM, Jens Axboe wrote:
> On 7/26/22 10:32 AM, Ammar Faizi wrote:
>> On 7/26/22 11:23 PM, Jens Axboe wrote:
>>> [5/5] add an example for a UDP server
>>>         commit: 61d472b51e761e61cbf46caea40aaf40d8ed1484
>>
>> This one breaks clang-13 build, I'll send a patch.
> 
> Hmm, built fine with clang-13/14 here?

Not sure what is going on, but clang-13 on my machine is not happy:

     io_uring-udp.c:134:18: error: incompatible pointer types passing \
     'struct sockaddr_in6 *' to parameter of type 'const struct sockaddr *' \
     [-Werror,-Wincompatible-pointer-types

     io_uring-udp.c:142:18: error: incompatible pointer types passing \
     'struct sockaddr_in *' to parameter of type 'const struct sockaddr *' \
     [-Werror,-Wincompatible-pointer-types]

Changing the compiler to GCC builds just fine. I have fixed something like
this more than once. Strange indeed.
Jens Axboe July 26, 2022, 4:49 p.m. UTC | #5
On 7/26/22 10:48 AM, Ammar Faizi wrote:
> On 7/26/22 11:40 PM, Jens Axboe wrote:
>> On 7/26/22 10:32 AM, Ammar Faizi wrote:
>>> On 7/26/22 11:23 PM, Jens Axboe wrote:
>>>> [5/5] add an example for a UDP server
>>>>         commit: 61d472b51e761e61cbf46caea40aaf40d8ed1484
>>>
>>> This one breaks clang-13 build, I'll send a patch.
>>
>> Hmm, built fine with clang-13/14 here?
> 
> Not sure what is going on, but clang-13 on my machine is not happy:
> 
>     io_uring-udp.c:134:18: error: incompatible pointer types passing \
>     'struct sockaddr_in6 *' to parameter of type 'const struct sockaddr *' \
>     [-Werror,-Wincompatible-pointer-types
> 
>     io_uring-udp.c:142:18: error: incompatible pointer types passing \
>     'struct sockaddr_in *' to parameter of type 'const struct sockaddr *' \
>     [-Werror,-Wincompatible-pointer-types]
> 
> Changing the compiler to GCC builds just fine. I have fixed something like
> this more than once. Strange indeed.

Regardless, the patch is sane!
Dylan Yudaken July 27, 2022, 7:57 a.m. UTC | #6
On Tue, 2022-07-26 at 10:49 -0600, Jens Axboe wrote:
> 
> On 7/26/22 10:48 AM, Ammar Faizi wrote:
> > On 7/26/22 11:40 PM, Jens Axboe wrote:
> > > On 7/26/22 10:32 AM, Ammar Faizi wrote:
> > > > On 7/26/22 11:23 PM, Jens Axboe wrote:
> > > > > [5/5] add an example for a UDP server
> > > > >         commit: 61d472b51e761e61cbf46caea40aaf40d8ed1484
> > > > 
> > > > This one breaks clang-13 build, I'll send a patch.
> > > 
> > > Hmm, built fine with clang-13/14 here?
> > 
> > Not sure what is going on, but clang-13 on my machine is not happy:
> > 
> >     io_uring-udp.c:134:18: error: incompatible pointer types
> > passing \
> >     'struct sockaddr_in6 *' to parameter of type 'const struct
> > sockaddr *' \
> >     [-Werror,-Wincompatible-pointer-types
> > 
> >     io_uring-udp.c:142:18: error: incompatible pointer types
> > passing \
> >     'struct sockaddr_in *' to parameter of type 'const struct
> > sockaddr *' \
> >     [-Werror,-Wincompatible-pointer-types]
> > 
> > Changing the compiler to GCC builds just fine. I have fixed
> > something like
> > this more than once. Strange indeed.


Interestingly it did not show up on the Github CI either. What flags
are you setting for this? Maybe the CI can be expanded to include those
flags.
As you say its not the first time you've fixed this, or that I've done
this.

Dylan
Ammar Faizi July 27, 2022, 9:59 a.m. UTC | #7
On 7/27/22 2:57 PM, Dylan Yudaken wrote:
> Interestingly it did not show up on the Github CI either. What flags
> are you setting for this? Maybe the CI can be expanded to include those
> flags.
> As you say its not the first time you've fixed this, or that I've done
> this.

I use the same flag with the GitHub CI. Just a small experiment here...

I compile this with default compilation flags:

#include <arpa/inet.h>
#include <sys/socket.h>
#include <sys/types.h>

int main(void)
{
         struct sockaddr_in addr = { };
         return bind(0, &addr, sizeof(addr));
}

===============================================================================

ammarfaizi2@integral2:/tmp$ gcc test.c -o test
test.c: In function ‘main’:
test.c:9:24: warning: passing argument 2 of ‘bind’ from incompatible pointer type [-Wincompatible-pointer-types]
     9 |         return bind(0, &addr, sizeof(addr));
       |                        ^~~~~
       |                        |
       |                        struct sockaddr_in *
In file included from /usr/include/netinet/in.h:23,
                  from /usr/include/arpa/inet.h:22,
                  from test.c:2:
/usr/include/x86_64-linux-gnu/sys/socket.h:112:49: note: expected ‘const struct sockaddr *’ but argument is of type ‘struct sockaddr_in *’
   112 | extern int bind (int __fd, __CONST_SOCKADDR_ARG __addr, socklen_t __len)
       |

===============================================================================

ammarfaizi2@integral2:/tmp$ clang test.c -o test
test.c:9:17: warning: incompatible pointer types passing 'struct sockaddr_in *' to parameter of type 'const struct sockaddr *' [-Wincompatible-pointer-types]
         return bind(0, &addr, sizeof(addr));
                        ^~~~~
/usr/include/x86_64-linux-gnu/sys/socket.h:112:49: note: passing argument to parameter '__addr' here
extern int bind (int __fd, __CONST_SOCKADDR_ARG __addr, socklen_t __len)
                                                 ^
1 warning generated.

===============================================================================

Interestingly GCC also complains here, but it doesn't complain when
compiling your code. Your code only breaks my clang-13.

What is the magic behind this?

We never disable -Wincompatible-pointer-types in liburing either.
It's enabled by default.
Ammar Faizi July 27, 2022, 10:11 a.m. UTC | #8
On 7/27/22 4:59 PM, Ammar Faizi wrote:
> Interestingly GCC also complains here, but it doesn't complain when
> compiling your code. Your code only breaks my clang-13.
> 
> What is the magic behind this?

OK, I figured it out.

This work:

    gcc -D_GNU_SOURCE test.c -o test;

These 3 break:

    clang -D_GNU_SOURCE test.c -o test;
    clang test.c -o test;
    gcc test.c -o test;


So -D_GNU_SOURCE is the culprit. It seems to be unavoidable as
the warn seems to be compiler specific or something. Maybe that
_GNU_SOURCE patches the definition of bind().
Ammar Faizi July 27, 2022, 10:19 a.m. UTC | #9
On 7/27/22 5:11 PM, Ammar Faizi wrote:
> So -D_GNU_SOURCE is the culprit. It seems to be unavoidable as
> the warn seems to be compiler specific or something. Maybe that
> _GNU_SOURCE patches the definition of bind().

I did:

   gcc -E -D_GNU_SOURCE test.c -o xtest.c

and examined the xtest.c output.

So basically when we use _GNU_SOURCE, sometimes the declaration of
bind() is like this:

extern int bind (int __fd, __CONST_SOCKADDR_ARG __addr, socklen_t __len)
      __attribute__ ((__nothrow__ , __leaf__));

With __CONST_SOCKADDR_ARG being a union:

typedef union {
     const struct sockaddr *__restrict __sockaddr__;
     const struct sockaddr_at *__restrict __sockaddr_at__;
     const struct sockaddr_ax25 *__restrict __sockaddr_ax25__;
     const struct sockaddr_dl *__restrict __sockaddr_dl__;
     const struct sockaddr_eon *__restrict __sockaddr_eon__;
     const struct sockaddr_in *__restrict __sockaddr_in__;
     const struct sockaddr_in6 *__restrict __sockaddr_in6__;
     const struct sockaddr_inarp *__restrict __sockaddr_inarp__;
     const struct sockaddr_ipx *__restrict __sockaddr_ipx__;
     const struct sockaddr_iso *__restrict __sockaddr_iso__;
     const struct sockaddr_ns *__restrict __sockaddr_ns__;
     const struct sockaddr_un *__restrict __sockaddr_un__;
     const struct sockaddr_x25 *__restrict __sockaddr_x25__;
} __CONST_SOCKADDR_ARG __attribute__ ((__transparent_union__));

But not all header file included by the compiler has this union stuff.
When it doesn't, it will throw a warning like that.