git: 64881da47807 - main - ixgbe: Restore AIM support

[Kevin Bowling]

Should we enable this by default?  My primary concern in the review is not
making things like this require expert tuning.  If it is expected to be
necessary, which Gallatin@ makes a case for, let’s toggle it on by default.

On Mon, May 3, 2021 at 10:56 AM Mark Johnston <markj at freebsd.org> wrote:

> The branch main has been updated by markj:
>
> URL:
> https://cgit.FreeBSD.org/src/commit/?id=64881da478071431a2d9e62613997a5772c56cdf
>
> commit 64881da478071431a2d9e62613997a5772c56cdf
> Author:     Sai Rajesh Tallamraju <stallamr at netapp.com>
> AuthorDate: 2021-05-03 17:45:00 +0000
> Commit:     Mark Johnston <markj at FreeBSD.org>
> CommitDate: 2021-05-03 17:47:14 +0000
>
>     ixgbe: Restore AIM support
>
>     AIM (adaptive interrupt moderation) was part of BSD11 driver. Upon
> IFLIB
>     migration, AIM feature got lost. Re-introducing AIM back into IFLIB
>     based IXGBE driver.
>
>     One caveat is that in BSD11 driver, a queue comprises both Rx and Tx
>     ring. Starting from BSD12, Rx and Tx have their own queues and rings.
>     Also, IRQ is now only configured for Rx side. So, when AIM is
>     re-enabled, we should now consider only Rx stats for configuring EITR
>     register in contrast to BSD11 where Rx and Tx stats were considered to
>     manipulate EITR register.
>
>     Reviewed by:    gallatin, markj
>     Sponsored by:   NetApp, Inc.
>     MFC after:      2 weeks
>     Differential Revision:  https://reviews.freebsd.org/D27344
> ---
>  sys/dev/ixgbe/if_ix.c | 73
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  sys/dev/ixgbe/ixgbe.h |  1 +
>  2 files changed, 74 insertions(+)
>
> diff --git a/sys/dev/ixgbe/if_ix.c b/sys/dev/ixgbe/if_ix.c
> index 489e891cf509..fbe4f1df38ca 100644
> --- a/sys/dev/ixgbe/if_ix.c
> +++ b/sys/dev/ixgbe/if_ix.c
> @@ -347,6 +347,16 @@ static int ixgbe_enable_rss = 1;
>  SYSCTL_INT(_hw_ix, OID_AUTO, enable_rss, CTLFLAG_RDTUN,
> &ixgbe_enable_rss, 0,
>      "Enable Receive-Side Scaling (RSS)");
>
> +/*
> + * AIM: Adaptive Interrupt Moderation
> + * which means that the interrupt rate
> + * is varied over time based on the
> + * traffic for that interrupt vector
> + */
> +static int ixgbe_enable_aim = FALSE;
> +SYSCTL_INT(_hw_ix, OID_AUTO, enable_aim, CTLFLAG_RWTUN,
> &ixgbe_enable_aim, 0,
> +    "Enable adaptive interrupt moderation");
> +
>  #if 0
>  /* Keep running tab on them for sanity check */
>  static int ixgbe_total_ports;
> @@ -2100,6 +2110,60 @@ fail:
>         return (error);
>  } /* ixgbe_if_msix_intr_assign */
>
> +static inline void
> +ixgbe_perform_aim(struct adapter *adapter, struct ix_rx_queue *que)
> +{
> +       uint32_t newitr = 0;
> +       struct rx_ring *rxr = &que->rxr;
> +
> +       /*
> +        * Do Adaptive Interrupt Moderation:
> +        *  - Write out last calculated setting
> +        *  - Calculate based on average size over
> +        *    the last interval.
> +        */
> +       if (que->eitr_setting) {
> +               IXGBE_WRITE_REG(&adapter->hw, IXGBE_EITR(que->msix),
> +                   que->eitr_setting);
> +       }
> +
> +       que->eitr_setting = 0;
> +       /* Idle, do nothing */
> +       if (rxr->bytes == 0) {
> +               return;
> +       }
> +
> +       if ((rxr->bytes) && (rxr->packets)) {
> +               newitr = (rxr->bytes / rxr->packets);
> +       }
> +
> +       newitr += 24; /* account for hardware frame, crc */
> +       /* set an upper boundary */
> +       newitr = min(newitr, 3000);
> +
> +       /* Be nice to the mid range */
> +       if ((newitr > 300) && (newitr < 1200)) {
> +               newitr = (newitr / 3);
> +       } else {
> +               newitr = (newitr / 2);
> +       }
> +
> +       if (adapter->hw.mac.type == ixgbe_mac_82598EB) {
> +               newitr |= newitr << 16;
> +       } else {
> +               newitr |= IXGBE_EITR_CNT_WDIS;
> +       }
> +
> +       /* save for next interrupt */
> +       que->eitr_setting = newitr;
> +
> +       /* Reset state */
> +       rxr->bytes = 0;
> +       rxr->packets = 0;
> +
> +       return;
> +}
> +
>  /*********************************************************************
>   * ixgbe_msix_que - MSI-X Queue Interrupt Service routine
>   **********************************************************************/
> @@ -2117,6 +2181,11 @@ ixgbe_msix_que(void *arg)
>         ixgbe_disable_queue(adapter, que->msix);
>         ++que->irqs;
>
> +       /* Check for AIM */
> +       if (adapter->enable_aim) {
> +               ixgbe_perform_aim(adapter, que);
> +       }
> +
>         return (FILTER_SCHEDULE_THREAD);
>  } /* ixgbe_msix_que */
>
> @@ -2575,6 +2644,10 @@ ixgbe_add_device_sysctls(if_ctx_t ctx)
>             adapter, 0, ixgbe_sysctl_advertise, "I",
>             IXGBE_SYSCTL_DESC_ADV_SPEED);
>
> +       adapter->enable_aim = ixgbe_enable_aim;
> +       SYSCTL_ADD_INT(ctx_list, child, OID_AUTO, "enable_aim", CTLFLAG_RW,
> +           &adapter->enable_aim, 0, "Interrupt Moderation");
> +
>  #ifdef IXGBE_DEBUG
>         /* testing sysctls (for all devices) */
>         SYSCTL_ADD_PROC(ctx_list, child, OID_AUTO, "power_state",
> diff --git a/sys/dev/ixgbe/ixgbe.h b/sys/dev/ixgbe/ixgbe.h
> index 30dd1d5368fb..31d5cc41c066 100644
> --- a/sys/dev/ixgbe/ixgbe.h
> +++ b/sys/dev/ixgbe/ixgbe.h
> @@ -408,6 +408,7 @@ struct adapter {
>
>         /* Info about the interface */
>         int                     advertise;  /* link speeds */
> +       int                     enable_aim; /* adaptive interrupt
> moderation */
>         bool                    link_active;
>         u16                     num_segs;
>         u32                     link_speed;
> _______________________________________________
> dev-commits-src-main at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main
> To unsubscribe, send any mail to "
> dev-commits-src-main-unsubscribe at freebsd.org"
>