Re: if_txtimer API to replace (*if_watchdog)()

[Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>]

Hi,

Sorry for my late reply.

On 2020/01/22 9:21, Jason Thorpe wrote:
> > On Jan 20, 2020, at 2:48 PM, Jason Thorpe <thorpej%me.com@localhost> wrote:
> >
> > Folks --
> >
> > The legacy (*if_watchdog)() interface has a couple of problems:
> >
> > 1- It does not have any way to represent multiple hardware-managed transmit queues.
> >
> > 2- It's not easy to make MP-safe because it relies on modifying the ifnet structure periodically outside of the normal locking mechanisms.
> >
> > The wm(4) driver solved the problem in a reasonable way, and to make it easier for the other drivers in the tree to adopt it's strategy, I re-factored it into a new if_txtimer structure and API.
> >
> > So save typing, I'll paste the relevant section of <net/if.h>:
>
> I spent some time thinking about this a little more, especially around making it easier to convert drivers that don't have a periodic tick already (this is mainly drivers that don't use MII), so I added some extra support for such drivers and as a proof of concept, converted the SGI "sq" driver.  You can see that it's a very easy mechanical conversion for legacy drivers, that still gives them a simple NET_MPSAFE migration path (there's a provision for providing a tx queue interlock to the timer expiration callback).
>
> If there is consensus that this is a reasonable direction, then I'll start migrating drivers and, when complete, remove the legacy (*if_watchdog)() and if_timer fields from struct ifnet.

Looks good for me.  Thank you for detailed comments.


Thanks,

> For completeness, here's the big block comment:
>
> /*
>   * if_txtimer --
>   *
>   * Transmission timer (to replace the legacy ifnet watchdog timer).
>   *
>   * The driver should allocate one if_txtimer per hardware-managed
>   * transmission queue.  There are two different ways to allocate
>   * the and use the timer, based on the driver's structure.
>   *
>   * DRIVERS THAT PROVIDE THEIR OWN PERIODIC CALLOUT
>   * ===============================================
>   *
>   * ==> Allocate timers using if_txtimer_alloc().
>   * ==> In the periodic callout, check for txtimer expiration using
>   *     if_txtimer_is_expired() or if_txtimer_is_expired_explicit()
>   *     (see below).
>   *
>   * DRIVERS THAT DO NOT PROVIDE THEIR OWN PERIODIC CALLOUT
>   * ======================================================
>   *
>   * ==> Allocate timers using if_txtimer_alloc_with_callback().
>   *     This variant allocates a callout and provides a facility
>   *     for the callout to invoke a driver-provided callack when
>   *     the timer expires, with an optional interlock (typically
>   *     the transmit queue mutex).
>   *
>   *     If an interlock is provided, the interlock will be acquired
>   *     before checking for timer expiration, and will invoke the
>   *     callback with the interlock held if the timer has expired.
>   *     NOTE: the callback is responsible for releasing the interlock.
>   *     If an interlock is NOT provided, then IPL will be raised to
>   *     splnet() before checking for timer expiration and invoking
>   *     the callback.  In this case, the IPL will be restored on
>   *     the callback's behalf when it returns.
>   * ==> In the driver's (*if_init)() routine, the timer's callout
>   *     should be started with if_txtimer_start().  In the driver's
>   *     (*if_stop)() routine, the timer's callout should be stopped
>   *     with if_txtimer_stop() or if_txtimer_halt() (see callout(9)
>   *     for the difference between stop and halt).
>   *
>   * In both cases, all locking of the if_txtimer is the responsibility
>   * of the driver.  The if_txtimer should be protected by the same lock
>   * that protects the associated transmission queue.  The queue
>   * associated with the timer should be locked when arming and disarming
>   * the timer and when checking the timer for expiration.
>   *
>   * When the driver gives packets to the hardware to transmit, it should
>   * arm the timer by calling if_txtimer_arm().  When it is sweeping up
>   * completed transmit jobs, it should disarm the timer by calling
>   * if_txtimer_disarm() if there are no outstanding jobs remaining.
>   *
>   * If a driver needs to check multiple transmission queues, an
>   * optimization is available that avoids repeated calls to fetch
>   * the compare time.  In this case, the driver can get the compare
>   * time by calling if_txtimer_now() and can check for timer expiration
>   * using if_txtimer_is_expired_explicit().
>   *
>   * The granularity of the if_txtimer is 1 second.
>   */
>
>
>
>
> -- thorpej

--
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

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

Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>