On Tue, Feb 04, 2020 at 04:41:31PM +0100, Björn Töpel wrote: > 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(). So to summarize that: - return EAGAIN instead of ENETDOWN within ndo_xsk_wakeup implementations in i40e/ixgbe, - check for ENETDOWN in kick_tx() as well Do you guys agree? What about timeout and syscall repetition? That's the last question. > > > Björn
|