Re: [PATCH bpf-next v2 2/6] xsk: add support for need_wakeup flag in AF_XDP rings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2019-07-02 12:21, Magnus Karlsson wrote:
>   
> +/* XDP_RING flags */
> +#define XDP_RING_NEED_WAKEUP (1 << 0)
> +
>   struct xdp_ring_offset {
>   	__u64 producer;
>   	__u64 consumer;
>   	__u64 desc;
> +	__u64 flags;
>   };
>   
>   struct xdp_mmap_offsets {

<snip>

> @@ -621,9 +692,12 @@ static int xsk_getsockopt(struct socket *sock, int level, int optname,
>   	case XDP_MMAP_OFFSETS:
>   	{
>   		struct xdp_mmap_offsets off;
> +		bool flags_supported = true;
>   
> -		if (len < sizeof(off))
> +		if (len < sizeof(off) - sizeof(off.rx.flags))
>   			return -EINVAL;
> +		else if (len < sizeof(off))
> +			flags_supported = false;
>   
>   		off.rx.producer = offsetof(struct xdp_rxtx_ring, ptrs.producer);
>   		off.rx.consumer = offsetof(struct xdp_rxtx_ring, ptrs.consumer);
> @@ -638,6 +712,16 @@ static int xsk_getsockopt(struct socket *sock, int level, int optname,
>   		off.cr.producer = offsetof(struct xdp_umem_ring, ptrs.producer);
>   		off.cr.consumer = offsetof(struct xdp_umem_ring, ptrs.consumer);
>   		off.cr.desc	= offsetof(struct xdp_umem_ring, desc);
> +		if (flags_supported) {
> +			off.rx.flags = offsetof(struct xdp_rxtx_ring,
> +						ptrs.flags);
> +			off.tx.flags = offsetof(struct xdp_rxtx_ring,
> +						ptrs.flags);
> +			off.fr.flags = offsetof(struct xdp_umem_ring,
> +						ptrs.flags);
> +			off.cr.flags = offsetof(struct xdp_umem_ring,
> +						ptrs.flags);
> +		}

As far as I understood (correct me if I'm wrong), you are trying to 
preserve backward compatibility, so that if userspace doesn't support 
the flags field, you will determine that by looking at len and fall back 
to the old format.

However, two things are broken here:

1. The check `len < sizeof(off) - sizeof(off.rx.flags)` should be `len < 
sizeof(off) - 4 * sizeof(flags)`, because struct xdp_mmap_offsets 
consists of 4 structs xdp_ring_offset.

2. The old and new formats are not binary compatible, as flags are 
inserted in the middle of struct xdp_mmap_offsets.




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux