On Tue, 4 Jun 2019 10:06:57 +0200
Björn Töpel <bjorn.topel@xxxxxxxxx> wrote:
> 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@xxxxxxxxx>
> > ---
> > 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.
Probably we would be good to go with only max_rx, but in drivers during the
umem setup we also are comparing the queue id provided by user against the num
tx queues...so in theory, we could allocate the max_rx entries, but if the
current txq count is lower than reported max_rx, a bunch of map entries would
never be used, no?
>
> 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;
> > }
> >
> >
