Repository navigation
Conversation
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
270a9ee to
01b3b7e
Compare
|
Rebased onto AI-assisted (Claude Code). |
01b3b7e to
a91225e
Compare
a91225e to
e33b6a9
Compare
harisibrar1
left a comment
There was a problem hiding this comment.
Checked against main 522e32e and FreeBSD 11.4 sys/kern/uipc_usrreq.c, sys/netinet/udp_usrreq.c and sys/netinet/ip_output.c:
- Main still answers every level other than 0xffff/6/41 with ENOPROTOOPT (Udp.cpp:1148, 1160), and
git merge-treeagainst main is clean. orezraey's #10320 changes the same two lines with PS5 13.60 measurements (gist d9efe0da…,pinet4): level 99 gives EINVAL, IPPROTO_IP on AF_INET6 gives EINVAL, and an unknown IP option gives ENOPROTOOPT. Those agree with this PR, so they may be worth citing. One difference: #10320's blanket EINVAL would also cover level 17 on a datagram socket, whereudp_ctloutput(udp_usrreq.c:982) keeps ENOPROTOOPT as this PR does, so whichever lands second should keep that. - AF_UNIX, level 0:
uipc_ctloutput(uipc_usrreq.c:1237-1309) supports LOCAL_PEERCRED (1, get), LOCAL_CREDS (2) and LOCAL_CONNWAIT (4); only other options get EOPNOTSUPP. With this change (UnsupportedLevelError, Udp.cpp:729),setsockopt(unixfd, 0, 2, &one, 4)fails with EOPNOTSUPP where 11.4 returns 0. Could those three options throw (NotImplemented_nid_no_patch) and the rest return EOPNOTSUPP? - Small test note: option 1 at IPPROTO_UDP is
UDP_ENCAP(netinet/udp.h), which 11.4 handles on an IPSEC kernel (udp_usrreq.c:1004-1012). An option number with no case would test ENOPROTOOPT without depending on the kernel config.
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
Progress report (acb900c → 77c7232)📉 System libraries: 50.17% (-0.02%, -2 reverted functions)
|
| Library | Name |
|---|---|
| libkernel | getsockopt_nid_postfix |
| libkernel | setsockopt_nid_postfix |
Changed libraries (implemented exports)
| Library | Before | After | Change |
|---|---|---|---|
| libkernel | 683/721 (94.73%) | 681/721 (94.45%) | -0.28 pp |
|
@harisibrar1 thanks for the review. Pushed
AI-assisted (Claude Code). |
harisibrar1
left a comment
There was a problem hiding this comment.
Re-checked head b6ca1f3 against main b607bfd:
- The AF_UNIX change matches
uipc_ctloutput: on get,LOCAL_PEERCRED/LOCAL_CREDS/LOCAL_CONNWAITnow throw; on set, onlyLOCAL_CREDS/LOCAL_CONNWAITthrow (11.4'sSOPT_SETswitch has noLOCAL_PEERCREDcase, so EOPNOTSUPP there is right), and every other level-0 option gets EOPNOTSUPP. Option 77 at IPPROTO_UDP avoids the IPSEC dependency. Thanks. - #10320 has merged since, and it changed the same two lines: main now has
if (level != 0xffff && level != 0 && level != 6 && level != 41) return Fail(22);in bothsetsockoptandgetsockopt(3-waygit merge-treeshows two conflicts in Udp.cpp, plus GuestSocketOptions.cpp changed on both sides). Two things to keep in mind when resolving:- Level 0 is now routed to
SetIpLevelOption/GetIpLevelOption, which return EINVAL for anything that isn't IPv4 (SocketOptions.cpp:283, :320). So for the AF_UNIX handling to take effect, thesocket.family == GuestUnixcheck has to run before that dispatch; otherwise every AF_UNIX level-0 call becomes EINVAL. - Main's blanket
Fail(22)also covers IPPROTO_UDP (17) on a datagram socket, where your helper keepsudp_ctloutput's ENOPROTOOPT, so keepingUnsupportedLevelErrorfor the non-zero levels preserves that.
- Level 0 is now routed to
- With #10320's PS5 13.60 measurements now on main (level 99 and IPPROTO_IP on AF_INET6 → EINVAL), the remaining parts of this PR are the AF_UNIX and IPPROTO_UDP cases.
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
…INVAL like FreeBSD sosetopt and sogetopt hand a level other than SOL_SOCKET to the protocol. ip_ctloutput, ip6_ctloutput and tcp_ctloutput fail every level they do not own with EINVAL, and uipc_ctloutput does so for any level but 0. The call failed with ENOPROTOOPT for them; it still does for the IP level on an AF_INET socket and the UDP level on a datagram socket, which are handled levels with no option implemented. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…d with an error uipc_ctloutput handles LOCAL_PEERCRED (get), LOCAL_CREDS and LOCAL_CONNWAIT, so answering them with EOPNOTSUPP was wrong; they throw as unimplemented, every other option at level 0 still fails with EOPNOTSUPP. The UDP-level test uses an option number with no case in udp_ctloutput instead of UDP_ENCAP, which an IPSEC kernel handles. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
b6ca1f3 to
425ada0
Compare
|
Rebased onto AI-assisted (Claude Code). |
|
Tested head
One test note. AI-assisted: yes (Claude Code); results above were run on this machine. |
Option 1 at level 0 is IP_OPTIONS, which FreeBSD 11.4 accepts: ip_ctloutput passes the four bytes to ip_pcbopts (sys/netinet/ip_options.c:600), which reads a NOP and then an EOL and returns 0. The check asserted an answer FreeBSD does not give. Use 0x7777, the number the IPPROTO_IP getsockopt check in the same file already uses. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Thank you for the runs and for the note on the test. You are right about option 1: at level 0 it is AI-assisted (Claude Code). |
|
Re-checked head
AI-assisted: yes (Claude Code); results above were run on this machine. |
What
setsockopt_nid_postfixandgetsockopt_nid_postfix(Udp.cpp) answeredENOPROTOOPTfor every level exceptSOL_SOCKET,IPPROTO_TCPandIPPROTO_IPV6. FreeBSD 11.4 hands such a level to the protocol, andip_ctloutput(ip_output.c#L943-L944),ip6_ctloutput(ip6_output.c#L1459) anduipc_ctloutput(uipc_usrreq.c#L1228-L1229) fail every level they do not own withEINVAL. Now an unknown level fails withEINVAL. The levels a protocol owns keepENOPROTOOPTfor the options nothing here implements:IPPROTO_IPon anAF_INETsocket andIPPROTO_UDPon a datagram socket (udp_ctloutput); level 0 on anAF_UNIXsocket fails withEOPNOTSUPPfor the optionsuipc_ctloutputdoes not handle;LOCAL_PEERCRED(get),LOCAL_CREDSandLOCAL_CONNWAIT, which it handles, throw as unimplemented. The Technical debt entry is not touched:mainhas since split thegetsockopt, setsockoptentry into measured per-option entries, and its unmeasured sentence on theIPPROTO_IPlevel is now only exact forAF_INETsockets (this change answersEINVALonAF_INET6andEOPNOTSUPP/EINVALonAF_UNIX); that is left for a separate note.Fixes #8447
Overlap:
git merge-treeof this head with each of #7490, #7475, #7485, #7090, #9515 and #9578 (every one on the merge base git finds for the pair) is clean, and so is this head againstmain160f6592b5. #7511, which changed the same function bodies, was closed on 25 October; #8392, #7850 and #8312 are merged.pr_overlapwas not run. #10320 (merged) changed the same two lines and routes level 0 toSetIpLevelOption/GetIpLevelOption; here level 0 reachesUnsupportedLevelErroronly on anAF_UNIXsocket, so the PS5-measuredAF_INETandAF_INET6answers of #10320 are unchanged.Tested
First measurement, Windows 10 (19045), MinGW-w64 GCC 15.2.0, Release, on upstream/main 02b5a8e + the first commit:
guest_socket_options(the only test registered under that name): fails without the change toUdp.cpp(an unknown protocol level on a stream socket must fail with EINVAL (22), ip_ctloutput (errno 42), exit 0xc0000409) and passes with it.40 socket and network tests (
guest_socket_*,guest_sce_net,guest_udp,guest_tcp,guest_poll,guest_select*,guest_datagram_*,guest_recvmsg_memory) pass.python tools/check_conventions.py --base upstream/main: no finding.Rebased onto
mainb607bfdc8b(head425ada08af; conflict only inUdp.cpp, one line in each ofsetsockopt_nid_postfixandgetsockopt_nid_postfix, from fix(libkernel): keep IP_TTL, IP_TOS and the multicast options as measured on a PS5 #10320 routing level 0): Windows 10 19045, MinGW-w64 GCC 15.2.0, Release, both arms on the same base and rebuilt:guest_socket_optionsandguest_socket_stream_optionspass 4 of 4 runs; withUdp.cpprestored tomain,guest_socket_optionsfails 4 of 4 (exit code0xc0000409) andguest_socket_stream_optionspasses 4 of 4. Linux (WSL2 Ubuntu 24.04, virtualised kernel, not bare-metal), GCC 13.3.0, Release, one run per arm: both pass; withUdp.cpprestored tomain,guest_socket_optionsaborts andguest_socket_stream_optionspasses.Not tested: bare-metal Linux, macOS, a console, a title run. Not covered by the new test: level 17 on an
AF_INET6datagram socket and any console behaviour of the throwing options.Checklist
main; no other open PR implements the same functions (fix(libkernel): check socket address arguments before the descriptor #8392, merged, touched the same two functions for pointer and length checks, not the level).mdfiles or images added to the repositoryAI-assisted (Claude Code).
🤖 Generated with Claude Code