Page MenuHomeFreeBSD

tcp: Prefer EINVAL in case function not found
AcceptedPublic

Authored by zlei on Nov 17 2023, 8:44 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jul 21, 11:36 AM
Unknown Object (File)
Sat, Jul 5, 10:25 PM
Unknown Object (File)
Thu, Jul 3, 11:39 AM
Unknown Object (File)
Jul 2 2025, 5:18 AM
Unknown Object (File)
Jun 29 2025, 7:43 AM
Unknown Object (File)
Jun 26 2025, 8:51 PM
Unknown Object (File)
Jun 22 2025, 7:01 PM
Unknown Object (File)
Jun 16 2025, 1:41 AM

Details

Reviewers
rrs
rscheff
tuexen
Group Reviewers
transport
Summary

Suggested by: emaste
PR: 275146

Test Plan
# sysctl net.inet.tcp.functions_default=rack
net.inet.tcp.functions_default: freebsd
sysctl: net.inet.tcp.functions_default=rack: Invalid argument

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

zlei requested review of this revision.Nov 17 2023, 8:44 AM
This revision is now accepted and ready to land.Nov 17 2023, 9:41 AM

Looks like I lost the vote :) 3 people prefer EINVAL to ENOENT. I guess you dislike ENOENT cause it mentions files and directories and there are no files around. Agreed. Let me tell why I hate EINVAL. Cause it seems as a valid reply to any kind of problem. Developers tend to put it everywhere in the code. If a syscall returns me EINVAL, I can't find the returning line just with grep and my eyes. I end with long dtrace session instead. Numbers:

glebius@morannon:~:|>grep -r EINVAL /usr/src/FreeBSD/sys/ | wc -l
   16261
glebius@morannon:~:|>grep -r ENOENT /usr/src/FreeBSD/sys/ | wc -l
    2663

Didn't check other errors, but my bet nobody beats 16k EINVALs.

For average user EINVAL usually means that their input isn't correct. It is equally misleading as ENOENT. For more experienced user (I speculate) it is known that "file not found" actually means "entity no found".

Let's see what Randall thinks.

Looks like I lost the vote :) 3 people prefer EINVAL to ENOENT. I guess you dislike ENOENT cause it mentions files and

I wouldn't say so. I don't have any hard preference. My approval just means: "If EINVAL is preferred to ENOENT, the change is correct." I actually think ENOENT describes the situation better, just the textual description "No such file or directory" does not describe the situation as well as ENOENT.

I guess I don't have a strong opinion on the kernel errno, just that sysctl(1) should give a useful error message to the user.

I know that No such file or directory means that ENOENT was returned somewhere, but it's completely meaningless as an error string returned to an end user.

For average user EINVAL usually means that their input isn't correct. It is equally misleading as ENOENT. For more experienced user (I speculate) it is known that "file not found" actually means "entity no found".

Generally speaking, a more precise error number is valuable. But I think it is case by case.

In this context, to select a default TCP functions, then we firstly want to know the options. We ( experienced or not ) can refer to net.inet.tcp.functions_available to get all available functions. Then EINVAL from sysctl functions_default is meaningful. It is clear enough user entered the wrong value (probably typo).
Incase user blindly try to change functions_default then EINVAL is still meaningful, that means "your change is not invalid (for whatever reason)". So user will recheck his / her input.

I know that No such file or directory means that ENOENT was returned somewhere, but it's completely meaningless as an error string returned to an end user.

I agree with @emaste ' opinion . The sysctl functions_default is not to load the kernel module ( if not loaded ) for requested functions. It is only to choose a possible option from available TCP functions. So IMO either ENOENT or No such file or directory is misleading.

That's an interesting but not so important change. I think that the current "No such file or directory" is far better than the proposed "Invalid argument". When I saw "No such file or directory" for the first time it was clear to me that the module has to be loaded. "Invalid argument" will rather not bring any useful clues.