Re: [PATCH RFC bpf-next 2/3] bpf: Fix bpf_sk_lookup.remote_port on big-endian

[Jakub Sitnicki <jakub@cloudflare.com>]

On Sat, Feb 26, 2022 at 06:44 PM -08, Alexei Starovoitov wrote:
> On Tue, Feb 22, 2022 at 07:25:58PM +0100, Ilya Leoshkevich wrote:
>> On big-endian, the port is available in the second __u16, not the first
>> one. Therefore, provide a big-endian-specific definition that reflects
>> that. Also, define remote_port_compat in order to have nicer
>> architecture-agnostic code in the verifier and in tests.
>> 
>> Fixes: 9a69e2b385f4 ("bpf: Make remote_port field in struct bpf_sk_lookup 16-bit wide")
>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>> ---
>>  include/uapi/linux/bpf.h       | 17 +++++++++++++++--
>>  net/core/filter.c              |  5 ++---
>>  tools/include/uapi/linux/bpf.h | 17 +++++++++++++++--
>>  3 files changed, 32 insertions(+), 7 deletions(-)
>> 
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index afe3d0d7f5f2..7b0e5efa58e0 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -10,6 +10,7 @@
>>  
>>  #include <linux/types.h>
>>  #include <linux/bpf_common.h>
>> +#include <asm/byteorder.h>
>>  
>>  /* Extended instruction set based on top of classic BPF */
>>  
>> @@ -6453,8 +6454,20 @@ struct bpf_sk_lookup {
>>  	__u32 protocol;		/* IP protocol (IPPROTO_TCP, IPPROTO_UDP) */
>>  	__u32 remote_ip4;	/* Network byte order */
>>  	__u32 remote_ip6[4];	/* Network byte order */
>> -	__be16 remote_port;	/* Network byte order */
>> -	__u16 :16;		/* Zero padding */
>> +	union {
>> +		struct {
>> +#if defined(__BYTE_ORDER) ? __BYTE_ORDER == __LITTLE_ENDIAN : defined(__LITTLE_ENDIAN)
>> +			__be16 remote_port;	/* Network byte order */
>> +			__u16 :16;		/* Zero padding */
>> +#elif defined(__BYTE_ORDER) ? __BYTE_ORDER == __BIG_ENDIAN : defined(__BIG_ENDIAN)
>> +			__u16 :16;		/* Zero padding */
>> +			__be16 remote_port;	/* Network byte order */
>> +#else
>> +#error unspecified endianness
>> +#endif
>> +		};
>> +		__u32 remote_port_compat;
>
> Sorry this hack is not an option.
> Don't have any suggestions at this point. Pls come up with something else.

I think we can keep the bpf_sk_lookup definition as is, if we leave the
4-byte load from remote_port offset quirky behavior on little-endian.

Please take a look at the test fix I've posted for 4-byte load from
bpf_sock dst_port that works for me on x86_64 and s390. It is exactly
the same case as we're dealing with here:

https://lore.kernel.org/bpf/20220227202757.519015-4-jakub@cloudflare.com/T/#u