On Tue, 4 Feb 2020 at 14:58, Maciej Fijalkowski <maciej.fijalkowski@xxxxxxxxx> wrote: > > On Mon, Feb 03, 2020 at 01:49:11PM +0200, Maxim Mikityanskiy wrote: [...] > > Right, we shouldn't drop this check, at least it's needed to sync on > > xsk_wakeup when closing a socket. I think there are two issues here, and I > > suggest this way of solving them: > > > > 1. __I40E_CONFIG_BUSY is set while a single QP is being recreated, e.g., > > when opening/closing an XSK. I considered it as a flag for global > > configuration changes, but apparently it's also needed when a single XSK is > > being configured. Probably xsk_wakeup shouldn't return ENETDOWN when this > > flag is set, but something lighter like EAGAIN. xdpsock will need to repeat > > the syscall instead of bailing out. > > > Haven't yet checked the l2fwd case since I broke my hw setup, but do we > really need the syscall repetition? For tx only scenario skipping the > syscall would mean that we wouldn't post entries to completion queue and > in case tx ring gets full we would call complete_tx_only() again. So there > we have syscall being repeated? > I sort of like EAGAIN, as Max suggested, is better. Kind of nice for a user to know that "hey, for some reason the driver couldn't service you ATM, please try again". > > 2. Should xdpsock stop on ENETDOWN? Normal network sockets don't close as > > soon as the carrier is lost, so why should xdpsock fail on ENETDOWN? IMO, > > xdpsock should handle it more gracefully (wait with some timeout?). > > > > Does it sound like the right direction? > > It does! :) But in general I would allow ENETDOWN to the set of errnos > checked in kick_tx(). Not sure about repeating syscall though. > The fact that xdpsock die() on errors are a bit harsh, can't argue there. So, I'm all in for simplifying the sample error checking for kick_tx(). Björn
|