[LibOS] Add dummy ancillary data support for sockets

[dimakuv]

Description of the changes

Previously, Gramine returned -ENOSYS on recvmsg()/sendmsg() and similar syscalls if it detected the use of struct msghdr ancillary data (msg_control field). This PR is the first step in adding proper ancillary data support.

Currently no ancillary data type is truly supported:

• on sendmsg():
  • unknown/unsupported types force an error,
  • SCM_RIGHTS/SCM_CREDENTIALS are ignored in TCP/UDP sockets;
• on recvmsg(): msg_controllen is set to zero to indicate there is no ancillary data added to the received network packet.

How to test this PR?

I added a LibOS regression test + I disabled one misbehaving LTP test.

One can try this example: gramineproject/examples#57. Without this PR, it doesn't work properly (see test 2 in the README there).

---

This change is 

[dimakuv]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel)

a discussion (no related file):
This was found and debugged on the swtpm software, when I tried to enable it in Gramine.

Here are the relevant code snippets:

• https://github.com/stefanberger/swtpm/blob/185832c8d4be95fa9305279240ca33b7403cfc91/src/swtpm/ctrlchannel.c#L500-L506
• https://github.com/stefanberger/swtpm/blob/185832c8d4be95fa9305279240ca33b7403cfc91/src/swtpm/ctrlchannel.c#L345
• https://github.com/stefanberger/swtpm/blob/185832c8d4be95fa9305279240ca33b7403cfc91/src/swtpm/ctrlchannel.c#L810-L822

If you look closely at this code, you'll notice two things:

• This recvmsg(...with ancillary data...) is called even on TCP/IP sockets, where it is simply ignored.
• The ancillary data is used only in one command case (CMD_SET_DATAFD), and not used in all other cases (that make sense in Gramine-SGX).

You can even notice that CMD_SET_DATAFD is described as useful only for Unix Domain Sockets in the man page: https://github.com/stefanberger/swtpm/blob/e5fdd1c181d0414ad27a0f4f72c49020fbc1ac6e/man/man3/swtpm_ioctls.pod

So to workaround such flows, I think it is reasonable to merge this PR.

---

[mkow]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This was found and debugged on the swtpm software, when I tried to enable it in Gramine.

Here are the relevant code snippets:

• https://github.com/stefanberger/swtpm/blob/185832c8d4be95fa9305279240ca33b7403cfc91/src/swtpm/ctrlchannel.c#L500-L506
• https://github.com/stefanberger/swtpm/blob/185832c8d4be95fa9305279240ca33b7403cfc91/src/swtpm/ctrlchannel.c#L345
• https://github.com/stefanberger/swtpm/blob/185832c8d4be95fa9305279240ca33b7403cfc91/src/swtpm/ctrlchannel.c#L810-L822

If you look closely at this code, you'll notice two things:

• This recvmsg(...with ancillary data...) is called even on TCP/IP sockets, where it is simply ignored.
• The ancillary data is used only in one command case (CMD_SET_DATAFD), and not used in all other cases (that make sense in Gramine-SGX).

You can even notice that CMD_SET_DATAFD is described as useful only for Unix Domain Sockets in the man page: https://github.com/stefanberger/swtpm/blob/e5fdd1c181d0414ad27a0f4f72c49020fbc1ac6e/man/man3/swtpm_ioctls.pod

So to workaround such flows, I think it is reasonable to merge this PR.

This doesn't sound correct, you are silently dropping the data which the user app sent? I don't like it, it will make the apps which actually use ancillary data super annoying to debug.

If swtpm is open source, isn't it easier to just fix it to not send the data, which it doesn't even use?

---

[mkow]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)

a discussion (no related file):

Previously, mkow (Michał Kowalczyk) wrote…

This doesn't sound correct, you are silently dropping the data which the user app sent? I don't like it, it will make the apps which actually use ancillary data super annoying to debug.

If swtpm is open source, isn't it easier to just fix it to not send the data, which it doesn't even use?

Ok, you have a warning, but still... I don't like making our emulation behave differently than Linux just to workaround one specific app.

---

[dimakuv]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

a discussion (no related file):

This doesn't sound correct, you are silently dropping the data which the user app sent? I don't like it, it will make the apps which actually use ancillary data super annoying to debug.

TCP/IP sockets cannot have ancillary data.

Well, but now I think -- why do we even need a warning message. It should be the same behavior as Linux, so doesn't require a warning.

---

[dimakuv]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

a discussion (no related file):

TCP/IP sockets cannot have ancillary data.

Well, that's not exactly true. Here are the ancillary data types that could be on the socket during sendmsg():

• https://elixir.bootlin.com/linux/latest/source/net/core/sock.c#L2757
• (so, SO_MARK, SO_TIMESTAMPING_OLD and SCM_TXTIME are applicable to TCP, it seems, but they are all clearly unsupported by Gramine so could be ignored)

Here are the ancillary data types that could be on the socket during recvmsg():

• https://elixir.bootlin.com/linux/latest/source/net/ipv4/tcp.c#L2684
• (so, SO_TIMESTAMPNS_NEW, TCP_CM_INQ are applicable to TCP, but they are unsupported by Gramine so could be ignored)
• (btw, found an interesting example of this: https://github.com/torvalds/linux/blob/master/tools/testing/selftests/net/tcp_inq.c)

TLDR: Yeah, looks like Linux added support for ancillary data for TCP/IP sockets in recent years, though Gramine has no mechanics to support these data. So what about this:

1. I'll create another PR that has more specific handling of ancillary data,
2. sendmsg will fail on any ancillary data
3. recmsg will not add any ancillary data

This will be kinda similar to the current PR, but more explicit. @mkow ?

---

[dimakuv]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

TCP/IP sockets cannot have ancillary data.

Well, that's not exactly true. Here are the ancillary data types that could be on the socket during sendmsg():

• https://elixir.bootlin.com/linux/latest/source/net/core/sock.c#L2757
• (so, SO_MARK, SO_TIMESTAMPING_OLD and SCM_TXTIME are applicable to TCP, it seems, but they are all clearly unsupported by Gramine so could be ignored)

Here are the ancillary data types that could be on the socket during recvmsg():

• https://elixir.bootlin.com/linux/latest/source/net/ipv4/tcp.c#L2684
• (so, SO_TIMESTAMPNS_NEW, TCP_CM_INQ are applicable to TCP, but they are unsupported by Gramine so could be ignored)
• (btw, found an interesting example of this: https://github.com/torvalds/linux/blob/master/tools/testing/selftests/net/tcp_inq.c)

TLDR: Yeah, looks like Linux added support for ancillary data for TCP/IP sockets in recent years, though Gramine has no mechanics to support these data. So what about this:

1. I'll create another PR that has more specific handling of ancillary data,
2. sendmsg will fail on any ancillary data
3. recmsg will not add any ancillary data

This will be kinda similar to the current PR, but more explicit. @mkow ?

I experimented with TCP and different ancillary data types:

sendmsg()

1. If msg_control is set to an empty buffer, then sendmsg() returns EINVAL, triggering this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2794
2. If msg_control is set to SCM_RIGHTS, then sendmsg() sends the packet and ignores ancillary data, triggering this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2792
3. If msg_control is set to some non-socket level (i.e. cmsg_level = IPPROTO_IP), then ignores this ancillary data; this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2810
4. If msg_control is set to socket-specific SO_TIMESTAMPING, then sendmsg() uses this ancillary data, triggering this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2780

recvmsg()

1. If sendmsg() sent no ancillary data, recvmsg() gets nothing (msg_controllen = 0)
2. If sendmsg() sent SCM_RIGHTS, recvmsg() gets nothing (msg_controllen = 0)
3. If sendmsg() sent IPPROTO_IP, recvmsg() gets nothing (msg_controllen = 0)
4. If client socket has setsockopt(SOL_SOCKET, SO_TIMESTAMPING...) with SOF_TIMESTAMPING_RX_SOFTWARE | SOF_TIMESTAMPING_SOFTWARE, recvmsg() gets SO_TIMESTAMPING_OLD ancillary data (with msg_controllen = 24)

Conclusion

TCP/IP sockets in Linux:

• can send and receive ancillary data (I experimented with SO_TIMESTAMPING)
• sendmsg() ignores UDS-specific ancillary data or non-socket-level data and fails with EINVAL on unrecognized data
• recvmsg() returns msg_controllen = 0 if no ancillary data was attached to this packet

Thus, I should implement the "ignore all ancillary data" in the similar manner in this PR.

---

[dimakuv]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I experimented with TCP and different ancillary data types:

sendmsg()

1. If msg_control is set to an empty buffer, then sendmsg() returns EINVAL, triggering this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2794
2. If msg_control is set to SCM_RIGHTS, then sendmsg() sends the packet and ignores ancillary data, triggering this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2792
3. If msg_control is set to some non-socket level (i.e. cmsg_level = IPPROTO_IP), then ignores this ancillary data; this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2810
4. If msg_control is set to socket-specific SO_TIMESTAMPING, then sendmsg() uses this ancillary data, triggering this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2780

recvmsg()

1. If sendmsg() sent no ancillary data, recvmsg() gets nothing (msg_controllen = 0)
2. If sendmsg() sent SCM_RIGHTS, recvmsg() gets nothing (msg_controllen = 0)
3. If sendmsg() sent IPPROTO_IP, recvmsg() gets nothing (msg_controllen = 0)
4. If client socket has setsockopt(SOL_SOCKET, SO_TIMESTAMPING...) with SOF_TIMESTAMPING_RX_SOFTWARE | SOF_TIMESTAMPING_SOFTWARE, recvmsg() gets SO_TIMESTAMPING_OLD ancillary data (with msg_controllen = 24)

Conclusion

TCP/IP sockets in Linux:

• can send and receive ancillary data (I experimented with SO_TIMESTAMPING)
• sendmsg() ignores UDS-specific ancillary data or non-socket-level data and fails with EINVAL on unrecognized data
• recvmsg() returns msg_controllen = 0 if no ancillary data was attached to this packet

Thus, I should implement the "ignore all ancillary data" in the similar manner in this PR.

Btw, a good resource for this timestamping functionality: https://www.kernel.org/doc/Documentation/networking/timestamping.txt

---

[dimakuv]

Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

a discussion (no related file):
FYI: This PR is completely rewritten now (as of revision 3). It adds proper support (though everything is still resulting in error or is ignored).

I went deep into the Linux sources, and imitated what Linux does. Here are the relevant sources:

• https://elixir.bootlin.com/linux/latest/source/include/linux/socket.h
• https://elixir.bootlin.com/linux/latest/source/include/uapi/asm-generic/socket.h
• https://elixir.bootlin.com/linux/latest/source/include/net/scm.h
• https://elixir.bootlin.com/linux/latest/source/net/ipv4/tcp.c
• https://elixir.bootlin.com/linux/latest/source/net/ipv4/udp.c
• https://elixir.bootlin.com/linux/latest/source/net/ipv4/ip_sockglue.c
• https://elixir.bootlin.com/linux/latest/source/net/unix/af_unix.c
• https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c
• https://elixir.bootlin.com/linux/latest/source/net/core/scm.c

---

[stefanberger]

Reviewed 11 of 11 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

[stefanberger]

Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

[dimakuv]

Reviewable status: 10 of 12 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow and @stefanberger)

a discussion (no related file):
FYI: This PR is ready for review. Because the PR was reworked completely for revision 3, please review from scratch (ignore revisions 1 and 2).

---

[dimakuv]

Reviewable status: 10 of 12 files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow and @stefanberger)

---

libos/test/ltp/ltp.cfg line 1666 at r4 (raw file):

# subtest 8 requires SCM_RIGHTS support (to send an FD from sender to receiver) and doesn't check
# for errors (Gramine fails with -ENOSYS), leading to a hang on the receiver side because it tries
# to receive something that was never sent.

Btw, I created an issue in the LTP github repo: linux-test-project/ltp#1020

Not sure though this is a "real enough" issue, and that LTP folks will fix it

[mkow]

Reviewed 10 of 11 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)

a discussion (no related file):
Heh, I see a lot happened in the meantime :)

Regarding my original comment: The message I got from this PR originally was that this is a hack to make swtpm work and it's not how Linux works, thus my original blocking comment.

I experimented with TCP and different ancillary data types:
[...]

Jeez, what an ugly interface...

---

---

-- commits line 12 at r4:

Suggestion:

    - SCM_RIGHTS/SCM_CREDENTIALS are ignored in TCP/UDP sockets (same as on Linux);

---

libos/src/fs/socket/fs.c line 39 at r4 (raw file):

    };
    unsigned int flags = 0;
    return do_recvmsg(handle, &iov, /*iov_len=*/1, /*msg_control=*/NULL, /*msg_controllen=*/NULL,

Could we rename it to msg_controllen_ptr or something in this style? It's quite confusing to see things like msg_controllen = 0 almost next to msg_controllen = NULL when reading the socket code, because it looks like a bug. This is especially visible in this flie.
And maybe the same with addrlen?

Code quote:

/*msg_controllen=*/NULL

---

libos/src/net/ip.c line 682 at r4 (raw file):

        if (cmsg->cmsg_level != SOL_SOCKET) {
            /*
             * Currently don't support:

(it's in imperative mood otherwise)

Suggestion:

We currently don't support:

---

libos/src/net/ip.c line 693 at r4 (raw file):

        switch (cmsg->cmsg_type) {
            /* currently don't support below SOL_SOCKET types */

ditto

---

libos/src/net/ip.c line 778 at r4 (raw file):

    if (msg_control && msg_controllen) {
        /*
         * Currently don't support:

ditto

---

libos/src/net/unix.c line 435 at r4 (raw file):

        rest_msg_controllen -= CMSG_ALIGN(cmsg->cmsg_len);
        cmsg = (struct cmsghdr*)((char*)cmsg + CMSG_ALIGN(cmsg->cmsg_len));

Purely theoretical (I think) trivia: This cast to struct cmsghdr* is an UB in C, because it's possible that the resulting pointer won't point to neither a struct cmsghdr object nor right behind any such object if the msg_control array is not padded (I'm not sure if it has to be, probably not).

You could fix it by checking if the buffer we got is big enough to reach the (char*)cmsg + CMSG_ALIGN(cmsg->cmsg_len) address and executing this line only if it is, but probably not worth the effort.

I think C has this rule because lines like this one would be problematic if msg_control was allocated near the end of address space, because the addition could overflow the pointer then.

---

libos/test/regression/tcp_ancillary.c line 21 at r4 (raw file):

#define PORT   11110

#define MSG_SPACE CMSG_SPACE(sizeof(int)) + CMSG_SPACE(sizeof(struct ucred))

Suggestion:

#define MSG_SPACE (CMSG_SPACE(sizeof(int)) + CMSG_SPACE(sizeof(struct ucred)))

---

libos/test/regression/tcp_ancillary.c line 42 at r4 (raw file):

    char c = 0;
    ssize_t x = CHECK(write(pipefd, &c, sizeof(c)));
    if (x != 1) {

Suggestion:

if (x != sizeof(c))

---

libos/test/regression/tcp_ancillary.c line 117 at r4 (raw file):

        .sin_port = htons(PORT),
        .sin_addr = {
            /* TODO: remove this once Ubuntu 18.04 is deprecated. */

What exactly? The whole assignment of .sin_addr? Why? Could you explain this a bit more in the comment?

[kailun-qin]

Reviewable status: all files reviewed, 10 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)

---

libos/src/net/ip.c line 677 at r4 (raw file):

    size_t rest_msg_controllen = msg_controllen;
    while (cmsg && rest_msg_controllen >= sizeof(struct cmsghdr)) {
        if (cmsg->cmsg_len < sizeof(struct cmsghdr) || cmsg->cmsg_len > rest_msg_controllen)

Do we need to check against CMSG_ALIGN(cmsg->cmsg_len)?

Code quote:

cmsg->cmsg_len

[dimakuv]

Reviewable status: 6 of 12 files reviewed, 10 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @kailun-qin, @mkow, and @stefanberger)

---

-- commits line 12 at r4:
Yep, I'll add during final rebase

---

libos/src/fs/socket/fs.c line 39 at r4 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

Could we rename it to msg_controllen_ptr or something in this style? It's quite confusing to see things like msg_controllen = 0 almost next to msg_controllen = NULL when reading the socket code, because it looks like a bug. This is especially visible in this flie.
And maybe the same with addrlen?

Done.

---

libos/src/net/ip.c line 677 at r4 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

Do we need to check against CMSG_ALIGN(cmsg->cmsg_len)?

Done. You're right, otherwise we could have a broken corner case: cmsg->cmsg_len == rest_msg_controllen but CMSG_ALIGN(cmsg->cmsg_len) > rest_msg_controllen (e.g. if cmsg->cmsg_len == 7).

In that case, rest_msg_controllen -= CMSG_ALIGN(cmsg->cmsg_len); could result in integer underflow. Fixed now (by changing only the second part of this IF).

---

libos/src/net/ip.c line 682 at r4 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

(it's in imperative mood otherwise)

Done.

---

libos/src/net/ip.c line 693 at r4 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

ditto

Done.

---

libos/src/net/ip.c line 778 at r4 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

ditto

Done.

---

libos/src/net/unix.c line 435 at r4 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

Purely theoretical (I think) trivia: This cast to struct cmsghdr* is an UB in C, because it's possible that the resulting pointer won't point to neither a struct cmsghdr object nor right behind any such object if the msg_control array is not padded (I'm not sure if it has to be, probably not).

You could fix it by checking if the buffer we got is big enough to reach the (char*)cmsg + CMSG_ALIGN(cmsg->cmsg_len) address and executing this line only if it is, but probably not worth the effort.

I think C has this rule because lines like this one would be problematic if msg_control was allocated near the end of address space, because the addition could overflow the pointer then.

I agree with your assessment. I also stopped for a bit because of this CMSG_ALIGN() that could "shift" the cmsg pointer past the msg_control object boundary, which is a UB.

But @kailun-qin noticed above that there is still potential for integer overflow, see that comment. I fixed it now.

---

libos/test/regression/tcp_ancillary.c line 21 at r4 (raw file):

#define PORT   11110

#define MSG_SPACE CMSG_SPACE(sizeof(int)) + CMSG_SPACE(sizeof(struct ucred))

Done.

---

libos/test/regression/tcp_ancillary.c line 42 at r4 (raw file):

    char c = 0;
    ssize_t x = CHECK(write(pipefd, &c, sizeof(c)));
    if (x != 1) {

Done. Wow, that's some pedantry :)

---

libos/test/regression/tcp_ancillary.c line 117 at r4 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

What exactly? The whole assignment of .sin_addr? Why? Could you explain this a bit more in the comment?

Yes, remove this whole assignment of .sin_addr (because it would be implicitly set to zero anyway).

There is some weird bug in Ubuntu 18.04, and nobody wanted to care about this corner case. See the discussion starting here: #684 (review)

(Note that I based this new test on the already-existing tcp_msg_peek.c test, where the same TODO can be seen.)

If you want to, I can change the text of the TODO, but I'm not sure to what.

[mkow]

Reviewed 6 of 6 files at r5, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @dimakuv and @kailun-qin)

---

libos/src/net/unix.c line 435 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I agree with your assessment. I also stopped for a bit because of this CMSG_ALIGN() that could "shift" the cmsg pointer past the msg_control object boundary, which is a UB.

But @kailun-qin noticed above that there is still potential for integer overflow, see that comment. I fixed it now.

Yup, seems to be the same issue, but I didn't notice that it actually leads to underflowing rest_msg_controllen. So, turns out it wasn't purely theoretical after all, although because of a different part of the code :)

---

libos/test/regression/tcp_ancillary.c line 42 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Done. Wow, that's some pedantry :)

I disagree ;) This is how I check such places for correctness when reviewing: I read such constructions and check if there's a comparison between the return value and the exact expression which was used as a size for I/O. And here I had to stop and think because it was different ;)

---

libos/test/regression/tcp_ancillary.c line 117 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Yes, remove this whole assignment of .sin_addr (because it would be implicitly set to zero anyway).

There is some weird bug in Ubuntu 18.04, and nobody wanted to care about this corner case. See the discussion starting here: #684 (review)

(Note that I based this new test on the already-existing tcp_msg_peek.c test, where the same TODO can be seen.)

If you want to, I can change the text of the TODO, but I'm not sure to what.

Oh, so it's about GCC, not Ubuntu. Actually, we already started dropping 18.04, maybe just remove this line now?

[dimakuv]

Reviewable status: all files reviewed, 4 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @dimakuv, @kailun-qin, and @mkow)

---

libos/test/regression/tcp_ancillary.c line 117 at r4 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

Oh, so it's about GCC, not Ubuntu. Actually, we already started dropping 18.04, maybe just remove this line now?

What do you mean by started dropping 18.04? It's in the CI (e.g. Jenkins-18.04), so the current PR will fail if I remove this line.

[mkow]

Reviewable status: all files reviewed, 2 unresolved discussions, "fixup! " found in commit messages' one-liners (waiting on @dimakuv and @kailun-qin)

---

libos/test/regression/tcp_ancillary.c line 117 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

What do you mean by started dropping 18.04? It's in the CI (e.g. Jenkins-18.04), so the current PR will fail if I remove this line.

We're after v1.4, which was the last version which supports it and I think we already started removing some compat. But yeah, we need to disable the CI pipelines first.

[kailun-qin]

Reviewable status: all files reviewed, 1 unresolved discussion, "fixup! " found in commit messages' one-liners (waiting on @dimakuv)

[dimakuv]

Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

---

-- commits line 12 at r4:

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Yep, I'll add during final rebase

Done

[mkow]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved