Hi, On 2020/01/18 12:33, Jason Thorpe wrote:
Yes, the historical meaning is “busy transmitting, don’t send me any more stuff” — output active. It was a nice short cut back in the day when an interface could transmit one packet at a time (well, more accurately, had no interface-managed queue). The world is a little different now (well, for the last couple of decades or so, I guess)... the limitation isn’t necessarily even on the number of packets that the device’s queue can manage, but rather the number of DMA segments. As a result, many drivers need to handle being asked to transmit even if they don’t actually have the space because they don’t know until they’ve done at least some base amount of work. It is still true that if_start won’t be called if IFF_OACTIVE is set, but because drivers have to handle the “oops, can’t actually transmit that!” case anyway, it’s value is somewhat diminished. So we have 2 basic options ... we either do something similar to what OpenBSD did, or we just don’t bother and let drivers manage their device queue resources 100%. Modern drivers already do this (and multi-output-queue devices render OACTIVE essentially meaningless in those cases). Older drivers that really make use of OACTIVE could replace that logic with a bool field in their softc quite easily, and just short-circuit if_start if their own “busy” field is set. Many drivers already have a provision for checking OACTIVE in if_start anyway, so it’s not a big leap.
I agree. I think the OACTIVE bool field can also be moved to struct ifaltq and protected by ifaltq::ifq_lock. Using ifaltq::ifq_lock to protect the bool field would not increase lock contention, as IFQ_ENQUEUE and IFQ_DEQUEUE already uses ifaltq::ifq_lock. Thanks,
-- thorpej Sent from my iPhone.On Jan 17, 2020, at 4:58 PM, Greg Troxel <gdt%lexort.com@localhost> wrote: Jason Thorpe <thorpej%me.com@localhost> writes:IFF_OACTIVE is problematic for NET_MPSAFE because it lives in ifnet::if_flags, but needs to be fiddled with when ifnet::if_ioctl_lock is not held. In some ways, I question the utility of IFF_OACTIVE .. at best, it avoids calling (*if_start)() if there are no transmit slots available... but that situation is something that (*if_start)() routines need to handle anyway. OpenBSD addressed the issue by making the "output active" indicator an independent atomically settable field in the interface queue structure, and replacing direct manipulation of IFF_OACTIVE in ifnet::if_flags with accessor / mutator functions. Reporting of IFF_OACTIVE to userspace is maintained by returning the traditional flag in the ifreq. I'm not opposed to adopting OpenBSD's approach to fixing this problem, but if we can agree that IFF_OACTIVE is not useful in the first place, then I'd prefer to just rip it out completely. Thoughts?My impression is that IFF_OACTIVE has always been an implementation detail within the network stack. Traditionally, I would say that if_start could reliably expect not to be called when IFF_OACTIVE is true, expecting ether_output to add packet to queue if ! IFF_OACTIVE if_start and then in the tx complete routine, either queue another packet, or turn off IFF_OACTIVE. This is partly from the 4.3/4.4BSD books, and partly from memory 2.9BSD/4.2BSD era code. Given all that, it was part of the if_ethersubr/if_foo interface, but if if_start can't rely on not being called if it's set, then I don't see the point. As for exposing it to userland e.g. via flags shown by ifconfig, I would think that could be removed at any time. I can see why OpenBSD wanted to preserve traditional behavior; I am sympathethic to that myself. But I wonder if there was a functional reason?
-- ////////////////////////////////////////////////////////////////////// Internet Initiative Japan Inc. Device Engineering Section, Product Development Department, Product Division, Technology Unit Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>