IFF_OACTIVE -- is it still relevant?

[Jason Thorpe]

Hey folks --

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?

-- thorpej


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[David Young]

Post by Jason Thorpe
Hey folks --
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.

Right, it's not really useful. In multiple ring/queue/processor
situations, you end up maintaining multiple output-active indications,
each one independent from IFF_OACTIVE.

Dave

[Greg Troxel]

Post by Jason Thorpe
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?



--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Jason Thorpe]

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.

-- thorpej
Sent from my iPhone.

Post by Greg Troxel

Post by Jason Thorpe
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?

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Kengo NAKAHARA]

Hi,

Post by Jason Thorpe
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,

Post by Jason Thorpe
-- thorpej
Sent from my iPhone.

Post by Greg Troxel

Post by Jason Thorpe
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-***@iij.ad.jp>

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Jason Thorpe]

Post by Kengo NAKAHARA
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.

Ok, fair enough... However, I won't be using any lock for it... if anything, the lock that covers it would be the lock protecting the driver's hardware-managed transmit queue (which is managed independently of the output queue between the network interface and the upper layers of the stack); it's an indicator of those hardware queue resources being exhausted, after all. It's really just an optimization, and so I think I'll use a 32-bit word and manipulate it with atomic_{load,store}_relaxed().

-- thorpej


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Kengo NAKAHARA]

Hi,

Post by Jason Thorpe

Post by Kengo NAKAHARA
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.

Ok, fair enough... However, I won't be using any lock for it... if anything, the lock that covers it would be the lock protecting the driver's hardware-managed transmit queue (which is managed independently of the output queue between the network interface and the upper layers of the stack); it's an indicator of those hardware queue resources being exhausted, after all. It's really just an optimization, and so I think I'll use a 32-bit word and manipulate it with atomic_{load,store}_relaxed().

Ah, you are right. The bool field is not required any lock, sorry.
Indeed, atomic_{load,store}_relaxd() are sufficient for it.


Thanks,
--
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

Device Engineering Section,
Product Development Department,
Product Division,
Technology Unit

Kengo NAKAHARA <k-***@iij.ad.jp>

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Michael van Elst]

Post by Greg Troxel
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.

It's an optimization. Checking a flag is cheaper than possibily
checking hardware and acquiring access locks. The network layer
doesn't necessarily know how efficient a network driver is.

Of course you can replicate that optimization into every driver
(most already have something almost as efficient) and of course it
doesn't model interfaces with multiple queues correctly. But
as an interface it's better handled by something in the network
layer.

[Joerg Sonnenberger]

Post by Jason Thorpe
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.

That would be better served by an estimate of the number of open and
in-flight transmit slots, something a queueing discipline might care
for. As is, I see little reason for preserving IFF_OACTIVE in the
current form and would suggest to just drop it.

Joerg

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Jason Thorpe]

Bumping this thread...

Post by Joerg Sonnenberger

Post by Jason Thorpe
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.

That would be better served by an estimate of the number of open and
in-flight transmit slots, something a queueing discipline might care
for. As is, I see little reason for preserving IFF_OACTIVE in the
current form and would suggest to just drop it.

Yes, this is the direction I plan to go forward with.

I will start by G/C'ing IFF_OACTIVE usage in individual drivers that are easy to handle, and then slowly move on to the more difficult tiers.

-- thorpej


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de