Re: [PATCH bpf v2 3/4] bpf: Use EOPNOTSUPP in bpf_tcp_check_syncookie

[Maxim Mikityanskiy <maximmi@nvidia.com>]

On 2022-01-25 09:06, John Fastabend wrote:
> Maxim Mikityanskiy wrote:
>> When CONFIG_SYN_COOKIES is off, bpf_tcp_check_syncookie returns
>> ENOTSUPP. It's a non-standard and deprecated code. The related function
>> bpf_tcp_gen_syncookie and most of the other functions use EOPNOTSUPP if
>> some feature is not available. This patch changes ENOTSUPP to EOPNOTSUPP
>> in bpf_tcp_check_syncookie.
>>
>> Fixes: 399040847084 ("bpf: add helper to check for a valid SYN cookie")
>> Signed-off-by: Maxim Mikityanskiy <maximmi@nvidia.com>
>> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
> 
> This came up in another thread? Or was it the same and we lost the context
> in the commit msg. Either way I don't think we should start one-off
> changing these user facing error codes. Its not the only spot we do this
> and its been this way for sometime.
> 
> Is it causing a real problem?

I'm not aware of anyone complaining about it. It's just a cleanup to use 
the proper error code, since ENOTSUPP is a non-standard one (used in 
NFS?), for example, strerror() returns "Unknown error 524" instead of 
"Operation not supported".

Source: Documentation/dev-tools/checkpatch.rst:

 > ENOTSUPP is not a standard error code and should be avoided in new
 > patches. EOPNOTSUPP should be used instead.
 >
 > See: https://lore.kernel.org/netdev/20200510182252.GA411829@lunn.ch/

>> ---
>>   net/core/filter.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 780e635fb52a..2c9106704821 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -6814,7 +6814,7 @@ BPF_CALL_5(bpf_tcp_check_syncookie, struct sock *, sk, void *, iph, u32, iph_len
>>   
>>   	return -ENOENT;
>>   #else
>> -	return -ENOTSUPP;
>> +	return -EOPNOTSUPP;
>>   #endif
>>   }
>>   
>> -- 
>> 2.30.2
>>