OS-4525 lxbrand implement getsockopt(TCP_INFO) #459

cneira · 2023-11-22T21:41:26Z

Hi,

As discussed in the mailing list, this patch adds the TCP_INFO getsockopt to the LX brand.
Tests where done in using LX image: 0bf06d4d-b62f-4b3b-b560-3cd258df2070.
Applications tested where nginx , iperf and a simple C program exercising this TCP option.
I used as reference the notes in https://smartos.org/bugview/OS-4525

Bests

usr/src/uts/common/brand/lx/sys/lx_socket.h

usr/src/uts/common/brand/lx/syscall/lx_socket.c

danmcd

Sorry I wasn't clear... you don't just need a new macro, you need a new TABLE as well. I showed the entirety of what I had in mind.

I'll also note there's no illumos version of Linux's NEW_SYN_RECV either.

usr/src/uts/common/brand/lx/sys/lx_socket.h

usr/src/uts/common/brand/lx/syscall/lx_socket.c

usr/src/uts/common/brand/lx/sys/lx_socket.h

danmcd

Mostly nits left, and one thing that's probably my fault with merge hell. This is looking much better.

usr/src/uts/common/brand/lx/sys/lx_socket.h

usr/src/uts/common/brand/lx/syscall/lx_socket.c

danmcd

One more nit: Please remove the ':' from your summary. It should read:

"OS-4525 lxbrand implement getsockopt(TCP_INFO)"

This should allow our Jira to synch up with this PR.

usr/src/uts/common/brand/lx/sys/lx_socket.h

danmcd · 2024-01-16T15:00:04Z

Oh damn, one last thing... please update your copyrights to 2024.

danmcd

Found some more nits. Pity LX doesn't have stricter compilation flags; some of this might've been caught. If this was caused by me pressing "update branch", I apologize.

usr/src/uts/common/brand/lx/syscall/lx_socket.c

cneira · 2024-01-18T22:57:11Z

Found some more nits. Pity LX doesn't have stricter compilation flags; some of this might've been caught. If this was caused by me pressing "update branch", I apologize.

Done

cneira · 2024-01-18T22:58:50Z

Copyrights updated

danmcd

You're going to need to put comments on every field from our tcp_t you assign to lx_tcp_info_t in the actual function. I'm seeing enough here to make me scratch my head.

usr/src/uts/common/brand/lx/syscall/lx_socket.c

cneira · 2024-01-19T18:29:03Z

You're going to need to put comments on every field from our tcp_t you assign to lx_tcp_info_t in the actual function. I'm seeing enough here to make me scratch my head.

I'll add comments on how I arrived to map those fields and remove the ones that don't have the same meaning as the ones expected by Linux conventions.

danmcd

Fix the cstyle nits and you're good to go. git pbchk is your friend as long as /opt/onbld or some version of it in your illumos build is available.

usr/src/uts/common/brand/lx/syscall/lx_socket.c

danmcd · 2024-01-23T05:20:20Z

Please make sure you re-run your tests and update any result changes here. Between that and maybe one other reviewer I'll mark IA and pull this in. Thank you for your efforts, time, and patience here.

cneira · 2024-01-24T18:41:30Z

Please make sure you re-run your tests and update any result changes here. Between that and maybe one other reviewer I'll mark IA and pull this in. Thank you for your efforts, time, and patience here.

Here are the tests I have done after the latest changes :

Linux Branded Zone

test_connect[SUCCESS] TCP state: LX_TCP_ESTABLISHED
test_connect[SUCCESS] tcpi_rto 1125000
test_connect[SUCCESS] tcpi_last_data_recv 0
test_connect[SUCCESS] tcpi_rtt 4000000
test_connect[SUCCESS] tcpi_rttvar 500000
test_connect[SUCCESS] tcpi_snd_ssthresh 0
test_connect[SUCCESS] tcpi_snd_cwnd 16360
test_connect[SUCCESS] tcpi_snd_mss 8180
test_connect[SUCCESS] tcpi_unacked -74323462
test_connect[SUCCESS] tcpi_sacked 0
test_connect[SUCCESS] tcpi_pmtu 0
test_connect[SUCCESS] tcpi_total_retrans 0
test_connect[SUCCESS] tcpi_unacked -74323462
test_connect[SUCCESS] tcpi_sacked 0
test_connect[SUCCESS] tcpi_rcv_space 1056768
test_connect[SUCCESS]:SOCK_STREAM NOT_CONNECTED TCP state: LX_TCP_ESTABLISHED
test_connect[SUCCESS] tcpi_rto 1125000
test_connect[SUCCESS] tcpi_last_data_recv 0
test_connect[SUCCESS] tcpi_rtt 4000000
test_connect[SUCCESS] tcpi_rttvar 500000
test_connect[SUCCESS] tcpi_snd_ssthresh 0
test_connect[SUCCESS] tcpi_snd_cwnd 0
test_connect[SUCCESS] tcpi_snd_mss 536
test_connect[SUCCESS] tcpi_unacked 0
test_connect[SUCCESS] tcpi_sacked 0
test_connect[SUCCESS] tcpi_pmtu 0
test_connect[SUCCESS] tcpi_total_retrans 0
test_connect[SUCCESS] tcpi_unacked 0
test_connect[SUCCESS] tcpi_sacked 0
test_connect[SUCCESS] tcpi_rcv_space 1048576
test_connect[SUCCESS]:SOCK_STREAM CONNECTED TCP state: LX_TCP_ESTABLISHED
test_connect[SUCCESS] tcpi_rto 1125000
test_connect[SUCCESS] tcpi_last_data_recv 0
test_connect[SUCCESS] tcpi_rtt 4000000
test_connect[SUCCESS] tcpi_rttvar 500000
test_connect[SUCCESS] tcpi_snd_ssthresh 0
test_connect[SUCCESS] tcpi_snd_cwnd 16360
test_connect[SUCCESS] tcpi_snd_mss 8180
test_connect[SUCCESS] tcpi_unacked -1767823445
test_connect[SUCCESS] tcpi_sacked 0
test_connect[SUCCESS] tcpi_pmtu 0
test_connect[SUCCESS] tcpi_total_retrans 0
test_connect[SUCCESS] tcpi_unacked -1767823445
test_connect[SUCCESS] tcpi_sacked 0
test_connect[SUCCESS] tcpi_rcv_space 1056768
test_bound[SUCCESS] BOUND tcp state: LX_TCP_LISTEN
test_bound[SUCCESS] tcpi_rto 1125000
test_bound[SUCCESS] tcpi_last_data_recv 0
test_bound[SUCCESS] tcpi_rtt 4000000
test_bound[SUCCESS] tcpi_rttvar 500000
test_bound[SUCCESS] tcpi_snd_ssthresh 0
test_bound[SUCCESS] tcpi_snd_cwnd 0
test_bound[SUCCESS] tcpi_snd_mss 536
test_bound[SUCCESS] tcpi_unacked 0
test_bound[SUCCESS] tcpi_sacked 0
test_bound[SUCCESS] tcpi_pmtu 0
test_bound[SUCCESS] tcpi_total_retrans 0
test_bound[SUCCESS] tcpi_unacked 0
test_bound[SUCCESS] tcpi_sacked 0
test_bound[SUCCESS] tcpi_rcv_space 1048576
test_bound[SUCCESS] LISTEN tcp state: LX_TCP_LISTEN
test_bound[SUCCESS] tcpi_rto 1125000
test_bound[SUCCESS] tcpi_last_data_recv 0
test_bound[SUCCESS] tcpi_rtt 4000000
test_bound[SUCCESS] tcpi_rttvar 500000
test_bound[SUCCESS] tcpi_snd_ssthresh 0
test_bound[SUCCESS] tcpi_snd_cwnd 0
test_bound[SUCCESS] tcpi_snd_mss 536
test_bound[SUCCESS] tcpi_unacked 0
test_bound[SUCCESS] tcpi_sacked 0
test_bound[SUCCESS] tcpi_pmtu 0
test_bound[SUCCESS] tcpi_total_retrans 0
test_bound[SUCCESS] tcpi_unacked 0
test_bound[SUCCESS] tcpi_sacked 0
test_bound[SUCCESS] tcpi_rcv_space 1048576

Linux Native VM

test_connect[SUCCESS] TCP state: LX_TCP_ESTABLISHED
test_connect[SUCCESS] tcpi_rto 204000
test_connect[SUCCESS] tcpi_last_data_recv 0
test_connect[SUCCESS] tcpi_rtt 50
test_connect[SUCCESS] tcpi_rttvar 25
test_connect[SUCCESS] tcpi_snd_ssthresh 2147483647
test_connect[SUCCESS] tcpi_snd_cwnd 10
test_connect[SUCCESS] tcpi_snd_mss 32732
test_connect[SUCCESS] tcpi_unacked 0
test_connect[SUCCESS] tcpi_sacked 0
test_connect[SUCCESS] tcpi_pmtu 65536
test_connect[SUCCESS] tcpi_total_retrans 0
test_connect[SUCCESS] tcpi_unacked 0
test_connect[SUCCESS] tcpi_sacked 0
test_connect[SUCCESS] tcpi_rcv_space 65476
test_connect[SUCCESS]:SOCK_STREAM NOT_CONNECTED TCP state: LX_TCP_CLOSE
test_connect[SUCCESS] tcpi_rto 1000000
test_connect[SUCCESS] tcpi_last_data_recv 16758308
test_connect[SUCCESS] tcpi_rtt 0
test_connect[SUCCESS] tcpi_rttvar 250000
test_connect[SUCCESS] tcpi_snd_ssthresh 2147483647
test_connect[SUCCESS] tcpi_snd_cwnd 10
test_connect[SUCCESS] tcpi_snd_mss 536
test_connect[SUCCESS] tcpi_unacked 0
test_connect[SUCCESS] tcpi_sacked 0
test_connect[SUCCESS] tcpi_pmtu 0
test_connect[SUCCESS] tcpi_total_retrans 0
test_connect[SUCCESS] tcpi_unacked 0
test_connect[SUCCESS] tcpi_sacked 0
test_connect[SUCCESS] tcpi_rcv_space 0
test_connect[SUCCESS]:SOCK_STREAM CONNECTED TCP state: LX_TCP_ESTABLISHED
test_connect[SUCCESS] tcpi_rto 204000
test_connect[SUCCESS] tcpi_last_data_recv 0
test_connect[SUCCESS] tcpi_rtt 40
test_connect[SUCCESS] tcpi_rttvar 20
test_connect[SUCCESS] tcpi_snd_ssthresh 2147483647
test_connect[SUCCESS] tcpi_snd_cwnd 10
test_connect[SUCCESS] tcpi_snd_mss 32741
test_connect[SUCCESS] tcpi_unacked 0
test_connect[SUCCESS] tcpi_sacked 0
test_connect[SUCCESS] tcpi_pmtu 65535
test_connect[SUCCESS] tcpi_total_retrans 0
test_connect[SUCCESS] tcpi_unacked 0
test_connect[SUCCESS] tcpi_sacked 0
test_connect[SUCCESS] tcpi_rcv_space 65495
test_bound[SUCCESS] BOUND tcp state: LX_TCP_CLOSE
test_bound[SUCCESS] tcpi_rto 1000000
test_bound[SUCCESS] tcpi_last_data_recv 16758308
test_bound[SUCCESS] tcpi_rtt 0
test_bound[SUCCESS] tcpi_rttvar 250000
test_bound[SUCCESS] tcpi_snd_ssthresh 2147483647
test_bound[SUCCESS] tcpi_snd_cwnd 10
test_bound[SUCCESS] tcpi_snd_mss 536
test_bound[SUCCESS] tcpi_unacked 0
test_bound[SUCCESS] tcpi_sacked 0
test_bound[SUCCESS] tcpi_pmtu 0
test_bound[SUCCESS] tcpi_total_retrans 0
test_bound[SUCCESS] tcpi_unacked 0
test_bound[SUCCESS] tcpi_sacked 0
test_bound[SUCCESS] tcpi_rcv_space 0
test_bound[SUCCESS] LISTEN tcp state: LX_TCP_LISTEN
test_bound[SUCCESS] tcpi_rto 0
test_bound[SUCCESS] tcpi_last_data_recv 0
test_bound[SUCCESS] tcpi_rtt 0
test_bound[SUCCESS] tcpi_rttvar 0
test_bound[SUCCESS] tcpi_snd_ssthresh 0
test_bound[SUCCESS] tcpi_snd_cwnd 10
test_bound[SUCCESS] tcpi_snd_mss 0
test_bound[SUCCESS] tcpi_unacked 0
test_bound[SUCCESS] tcpi_sacked 10
test_bound[SUCCESS] tcpi_pmtu 0
test_bound[SUCCESS] tcpi_total_retrans 0
test_bound[SUCCESS] tcpi_unacked 0
test_bound[SUCCESS] tcpi_sacked 10
test_bound[SUCCESS] tcpi_rcv_space 0

There are still differences for example in tcpi_unacked we return a negative value, Linux returns 0.

cneira mentioned this pull request Nov 26, 2023

OS-4525: lxbrand implement getsockopt(TCP_INFO) omniosorg/illumos-omnios#1396

Open

danmcd reviewed Nov 27, 2023

View reviewed changes

usr/src/uts/common/brand/lx/sys/lx_socket.h Show resolved Hide resolved

usr/src/uts/common/brand/lx/syscall/lx_socket.c Outdated Show resolved Hide resolved

usr/src/uts/common/brand/lx/syscall/lx_socket.c Show resolved Hide resolved

cneira and others added 3 commits December 1, 2023 23:05

OS-4525: lxbrand implement getsockopt(TCP_INFO)

54f44a2

Merge branch 'master' into OS-4525

bd83a53

Merge branch 'master' into OS-4525

3820e2a

danmcd reviewed Dec 4, 2023

View reviewed changes

cneira and others added 3 commits December 14, 2023 21:11

OS-4525: lxbrand implement getsockopt(TCP_INFO)

1d06dfa

Merge branch 'OS-4525' of github.com:cneira/illumos-joyent into OS-4525

ea78f83

Merge branch 'master' into OS-4525

88080af

danmcd reviewed Dec 14, 2023

View reviewed changes

cneira and others added 3 commits December 18, 2023 20:02

Merge branch 'OS-4525' of github.com:cneira/illumos-joyent into OS-4525

ed10cb7

OS-4525 Add TCP_INFO option to LX Brand

f7cdf31

Merge branch 'master' into OS-4525

3672c8f

danmcd reviewed Jan 16, 2024

View reviewed changes

usr/src/uts/common/brand/lx/sys/lx_socket.h Show resolved Hide resolved

danmcd reviewed Jan 16, 2024

View reviewed changes

usr/src/uts/common/brand/lx/syscall/lx_socket.c Outdated Show resolved Hide resolved

usr/src/uts/common/brand/lx/syscall/lx_socket.c Outdated Show resolved Hide resolved

usr/src/uts/common/brand/lx/syscall/lx_socket.c Outdated Show resolved Hide resolved

cneira changed the title ~~OS-4525: lxbrand implement getsockopt(TCP_INFO)~~ OS-4525 lxbrand implement getsockopt(TCP_INFO) Jan 18, 2024

danmcd and others added 4 commits January 18, 2024 22:34

Merge branch 'master' into OS-4525

fc78a77

Merge branch 'OS-4525' of github.com:cneira/illumos-joyent into OS-4525

aa99267

Update lx_socket.h

eb08267

Update lx_socket.c

b882824

danmcd reviewed Jan 19, 2024

View reviewed changes

cneira added 2 commits January 19, 2024 23:33

Merge branch 'OS-4525' of github.com:cneira/illumos-joyent into OS-4525

b13d69d

Merge branch 'OS-4525' of github.com:cneira/illumos-joyent into OS-4525

a74ebc9

danmcd reviewed Jan 22, 2024

View reviewed changes

usr/src/uts/common/brand/lx/syscall/lx_socket.c Outdated Show resolved Hide resolved

usr/src/uts/common/brand/lx/syscall/lx_socket.c Outdated Show resolved Hide resolved

cneira added 2 commits January 22, 2024 22:42

Merge branch 'OS-4525' of github.com:cneira/illumos-joyent into OS-4525

4dd762b

Merge branch 'OS-4525' of github.com:cneira/illumos-joyent into OS-4525

2560401

danmcd approved these changes Jan 23, 2024

View reviewed changes

Merge branch 'master' into OS-4525

eb97143

danmcd added 2 commits January 23, 2024 23:34

Merge branch 'master' into OS-4525

6927091

Merge branch 'master' into OS-4525

fffeb6b

danmcd added the integration-approval label Jan 24, 2024

danmcd merged commit 80cb3c4 into TritonDataCenter:master Jan 24, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OS-4525 lxbrand implement getsockopt(TCP_INFO) #459

OS-4525 lxbrand implement getsockopt(TCP_INFO) #459

cneira commented Nov 22, 2023

danmcd left a comment

danmcd left a comment

danmcd left a comment

danmcd commented Jan 16, 2024

danmcd left a comment

cneira commented Jan 18, 2024

cneira commented Jan 18, 2024

danmcd left a comment

cneira commented Jan 19, 2024

danmcd left a comment

danmcd commented Jan 23, 2024

cneira commented Jan 24, 2024

OS-4525 lxbrand implement getsockopt(TCP_INFO) #459

OS-4525 lxbrand implement getsockopt(TCP_INFO) #459

Conversation

cneira commented Nov 22, 2023

danmcd left a comment

Choose a reason for hiding this comment

danmcd left a comment

Choose a reason for hiding this comment

danmcd left a comment

Choose a reason for hiding this comment

danmcd commented Jan 16, 2024

danmcd left a comment

Choose a reason for hiding this comment

cneira commented Jan 18, 2024

cneira commented Jan 18, 2024

danmcd left a comment

Choose a reason for hiding this comment

cneira commented Jan 19, 2024

danmcd left a comment

Choose a reason for hiding this comment

danmcd commented Jan 23, 2024

cneira commented Jan 24, 2024

Linux Branded Zone

Linux Native VM