NetBSD Problem Report #58168

From www@netbsd.org  Wed Apr 17 18:55:50 2024
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 1030F1A9238
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 17 Apr 2024 18:55:50 +0000 (UTC)
Message-Id: <20240417185548.559371A923C@mollari.NetBSD.org>
Date: Wed, 17 Apr 2024 18:55:48 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: IFF_OACTIVE semantics and replacement plan is unclear
X-Send-Pr-Version: www-1.0

>Number:         58168
>Category:       kern
>Synopsis:       IFF_OACTIVE semantics and replacement plan is unclear
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Apr 17 19:00:00 +0000 2024
>Last-Modified:  Sun Apr 21 21:40:01 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current
>Organization:
The NetIFF Oactivation
>Environment:
>Description:
I understand IFF_OACTIVE is deprecated, shouldn't be used in new drivers, and should be removed from old drivers.

But what is the semantics in old drivers?

What are the rules for setting, clearing, and testing IFF_OACTIVE?

What is its contract in relation to if_start, if_output, if_transmit?

What is the plan for replacing it?
>How-To-Repeat:
code inspection
>Fix:
Yes, please!

>Audit-Trail:
From: matthew green <mrg@eterna23.net>
To: gnats-bugs@netbsd.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
    netbsd-bugs@netbsd.org
Subject: re: kern/58168: IFF_OACTIVE semantics and replacement plan is unclear
Date: Thu, 18 Apr 2024 06:38:41 +1000

 in the old method, IFF_OACTIVE is an driver-internal thing to say
 "i'm sending a packet".

 how it's used is upto each driver.

 replacement is to put it into driver-internal data, if you need it,
 but it's not always wanted or useful.  as the TODO.smpnet says, it
 needs to stop being part of if_flags, but what you do instead is a
 driver specific answer.  (perhaps updating TODO.smpnet to say that
 the semantics are driver specific would suffice here?)


 .mrg.

From: matthew green <mrg@eterna23.net>
To: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
    netbsd-bugs@netbsd.org, gnats-bugs@netbsd.org
Cc: 
Subject: re: kern/58168: IFF_OACTIVE semantics and replacement plan is unclear
Date: Thu, 18 Apr 2024 16:01:06 +1000

 > in the old method, IFF_OACTIVE is an driver-internal thing to say
 > "i'm sending a packet".
 >
 > how it's used is upto each driver.
 >
 > replacement is to put it into driver-internal data, if you need it,
 > but it's not always wanted or useful.  as the TODO.smpnet says, it
 > needs to stop being part of if_flags, but what you do instead is a
 > driver specific answer.  (perhaps updating TODO.smpnet to say that
 > the semantics are driver specific would suffice here?)

 taylor pointed out that net/if.c does pay attention to this
 flag.  it won't call if_start if it is set, assuming that
 the device is already running the queue.

 that check, if needed, should also be moved into the device
 if_start() if not already there.


 .mrg.

From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/58168: IFF_OACTIVE semantics and replacement plan is unclear
Date: Thu, 18 Apr 2024 12:05:22 -0000 (UTC)

 mrg@eterna23.net (matthew green) writes:

 > that check, if needed, should also be moved into the device
 > if_start() if not already there.

 That check in net/if.c exists only to optimize away the call of the
 start routine. The common boilerplate for such a (legacy) start routine
 is:

 static void
 xxx_start(struct ifnet *ifp) {

         if ((ifp->if_flags & (IFF_RUNNING|IFF_OACTIVE)) != IFF_RUNNING)
                 return;

 	...
 }

 to skip the start when the interface is either unavailable or busy.

 But when you require to take extra locks for checking an mp-safe
 driver flag, the additional overhead of the call itself becomes
 neglible.



From: Jason Thorpe <thorpej@me.com>
To: matthew green <mrg@eterna23.net>
Cc: kern-bug-people@netbsd.org,
 gnats-admin@netbsd.org,
 netbsd-bugs@netbsd.org,
 gnats-bugs@netbsd.org
Subject: Re: kern/58168: IFF_OACTIVE semantics and replacement plan is unclear
Date: Thu, 18 Apr 2024 10:17:01 -0700

 > On Apr 17, 2024, at 11:01=E2=80=AFPM, matthew green <mrg@eterna23.net> =
 wrote:
 >=20
 > taylor pointed out that net/if.c does pay attention to this
 > flag.  it won't call if_start if it is set, assuming that
 > the device is already running the queue.
 >=20
 > that check, if needed, should also be moved into the device
 > if_start() if not already there.

 Good time to remind folks about the thorpej-ifq branch that specifically =
 addresses all of this stuff.  It=E2=80=99s still a W-I-P, of course, but =
 it=E2=80=99s designed to be able to co-exist with the old way as drivers =
 are transitioned.

 -- thorpej

From: Christoph Badura <bad@bsd.de>
To: gnats-bugs@netbsd.org
Cc: 
Subject: re: kern/58168: IFF_OACTIVE semantics and replacement plan is
 unclear
Date: Sun, 21 Apr 2024 23:38:27 +0200

 I remembered how the IFF_OACTIVE protocol works.

 The if_snd queue decouples the "upper half" of the networking stack from
 the "lower half" of the hardware drives that operates in (hardware)
 interrupt context.

 Basically the networking stack puts packets for transmission on the if_snd
 queue.  And the interrupt driven output side of driver removes them as
 send buffer space in the hardware becomes available.

 The driver sets IFF_OACTIVE whenever it is expecting an interrupt
 signalling the end of processing of a packet to be sent (successfully or
 unsuccessfully e.g. for excessive collisions).  Then it fetches the next
 packet(s) from the if_snd queue and hands it/them over to the hardware,
 expecting more interrupts.

 In this manner dequeing packets from the if_snd queue and transmitting
 them is self-clocked by the interrupts from the hardware.

 But if there are no more packets queued in the hardware for transmit and
 the if_snd queue is empty this self-clocking stops.  That's when
 IFF_OACTIVE is cleared so that the next call to if_transmit() calls the
 driver's if_start function which enqueues the packet(s) with the hardware
 starting the interrupt driven self-clocking again.

 In particular setting IFF_OACTIVE does NOT mean that the interface is
 either unavailable or busy.  And neither is it "up to the driver" to
 define the semantics of IFF_OACTIVE.  This is a generic protocol between
 the upper, interruptless half of the kernel and the lower,
 interrupt-driven half of the drivers.

 --chris

Home	PR Database Search