Re: [RFC PATCH bpf-next 2/4] libbpf: check for channels.max_{t,r}x in xsk_get_max_queues

["Björn Töpel" <bjorn.topel@intel.com>]

On 2019-06-03 15:19, Maciej Fijalkowski wrote:
> When it comes down to ethtool's get channels API, various drivers are
> reporting the queue count in two ways - they are setting max_combined or
> max_tx/max_rx fields. When creating the eBPF maps for xsk socket, this
> API is used so that we have an entries in maps per each queue.
> In case where driver (mlx4, ice) reports queues in max_tx/max_rx, we end
> up with eBPF maps with single entries, so it's not possible to attach an
> AF_XDP socket onto queue other than 0 - xsk_set_bpf_maps() would try to
> call bpf_map_update_elem() with key set to xsk->queue_id.
> 
> To fix this, let's look for channels.max_{t,r}x as well in
> xsk_get_max_queues.
> 
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
>   tools/lib/bpf/xsk.c | 18 ++++++++++--------
>   1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> index 57dda1389870..514ab3fb06f4 100644
> --- a/tools/lib/bpf/xsk.c
> +++ b/tools/lib/bpf/xsk.c
> @@ -339,21 +339,23 @@ static int xsk_get_max_queues(struct xsk_socket *xsk)
>   	ifr.ifr_data = (void *)&channels;
>   	strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ);
>   	err = ioctl(fd, SIOCETHTOOL, &ifr);
> -	if (err && errno != EOPNOTSUPP) {
> -		ret = -errno;
> -		goto out;
> -	}
> +	close(fd);
> +
> +	if (err && errno != EOPNOTSUPP)
> +		return -errno;
>   
> -	if (channels.max_combined == 0 || errno == EOPNOTSUPP)
> +	if (channels.max_combined)
> +		ret = channels.max_combined;
> +	else if (channels.max_rx && channels.max_tx)
> +		ret = min(channels.max_rx, channels.max_tx);

Hmm, do we really need to look at max_tx? For each Rx, there's (usually)
an XDP ring.

OTOH, when AF_XDP ZC is not implemented, it uses the skb path...

> +	else if (channels.max_combined == 0 || errno == EOPNOTSUPP)
>   		/* If the device says it has no channels, then all traffic
>   		 * is sent to a single stream, so max queues = 1.
>   		 */
>   		ret = 1;
>   	else
> -		ret = channels.max_combined;
> +		ret = -1;
>   
> -out:
> -	close(fd);
>   	return ret;
>   }
>   
>