On 12/2/22 9:39 AM, Ji Rongfeng wrote:
Returning -EINVAL almost all the time when error occurs is not very helpful for the bpf prog to figure out what is wrong. This patch upgrades some return values so that they will be much more helpful. * return -ENOPROTOOPT when optname is unsupported The same as {g,s}etsockopt() syscall does. Before this patch, bpf_setsockopt(TCP_SAVED_SYN) already returns -ENOPROTOOPT, which may confuse the user, as -EINVAL is returned on other unsupported optnames. This patch also rejects TCP_SAVED_SYN right in sol_tcp_sockopt() when getopt is false, since do_tcp_setsockopt() is just the executor and it's not its duty to discover such error in bpf. We should maintain a precise allowlist to control whether an optname is supported and allowed to enter the executor or not. Functions like do_tcp_setsockopt(), their behaviour are not fully controllable by bpf. Imagine we let an optname pass, expecting -ENOPROTOOPT will be returned, but someday that optname is actually processed and unfortunately causes deadlock when calling from bpf. Thus, precise access control is essential.
Please leave the current -EINVAL to distinguish between optnames rejected by bpf and optnames rejected by the do_*_{get,set}sockopt().
* return -EOPNOTSUPP on level-related errors In do_ip_getsockopt(), -EOPNOTSUPP will be returned if level != SOL_IP. In ipv6_getsockopt(), -ENOPROTOOPT will be returned if level != SOL_IPV6. To be distinguishable, the former is chosen.
I would leave this one as is also. Are you sure the do_ip_*sockopt cannot handle sk_family == AF_INET6? afaict, bpf is rejecting those optnames instead.
* return -EBADFD when sk is not a full socket -EPERM or -EBUSY was an option, but in many cases one of them will be returned, especially under level SOL_TCP. -EBADFD is the better choice, since it is hardly returned in all cases. The bpf prog will be able to recognize it and decide what to do next.
This one makes sense and is useful.