Replace writev by sendmsg

[bosilca]

This allows the TCP BTL to avoid raising SIGPIPE on OSes that do not support SO_NOSIGPIPE.

[bosilca]

running out of disk space for the rocm build.

[jsquyres]

running out of disk space for the rocm build.

Hey @edgargabriel this came up again. Is there some kind of disk space limit in Github Actions, and we just happen to be running up against it in the RoCM build action for some reason?

[jsquyres]

@bosilca @abouteiller Is this worth bringing to v5.0.x?

[bosilca]

It helps deal with SIGPIPE on some odd systems. Worth it !

[abouteiller]

I think it would be beneficial yes

[jsquyres]

Why are there changes to group.h in this PR -- was that an accidental oversight?

I think the change is harmless, but it doesn't seem to be part of "Replace writev by sendmsg".

[bosilca]

Correct, the change in group.h is a performance improvement change that is not related to the sendmsg change. They ended up together because they were discovered in the same debugging session.

[jsquyres]

@bosilca @abouteiller Got a Coverity alert about this new code:

*** CID 1545093:  Uninitialized variables  (UNINIT)
/opal/mca/btl/tcp/btl_tcp_frag.c: 123 in mca_btl_tcp_frag_send()
117    
118         /* non-blocking write, continue if interrupted */
119         do {
120             /* Use sendmsg to avoid issues with SIGPIPE as described in
121              * https://blog.erratasec.com/2018/10/tcpip-sockets-and-sigpipe.html#
122              */
>>>     CID 1545093:  Uninitialized variables  (UNINIT)
>>>     Using uninitialized value "msg". Field "msg.msg_flags" is uninitialized when calling "sendmsg".
123             cnt = sendmsg(sd, &msg, msg_flags);
124             if (cnt < 0) {
125                 switch (opal_socket_errno) {
126                 case EINTR:
127                     continue;
128                 case EWOULDBLOCK:

[bosilca]

According to the documentation msg.msg_flags is not used for sendmsg, it is only used for recvmsg as a mean to get information about the received message.

[jsquyres]

If we use the C99 initialization, wouldn't that initialize all non-specified members to the zero value? E.g.:

 struct msghdr msg = {
    .msg_iov = frag->iov_ptr,
    .msg_iovlen = frag->iov_cnt,
};

I realize that you're saying we don't need to care about msg.msg_flags for sending, but it would be nice to obviate this warning and not have to explain it every time it comes up. 😄

[bosilca]

The aggregate initialisation will set the not-explicitly initialised members to their default values (NULL or zero). But this is not really a correctness issue, it is writing code in an unnecessary complicated way to make a code-analysis tool happy.

[jsquyres]

PR #11940 removed 5 lines of code -- what's not to love? 😜