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 <ma...@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 <stall...@netapp.com> > AuthorDate: 2021-05-03 17:45:00 +0000 > Commit: Mark Johnston <ma...@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@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main > To unsubscribe, send any mail to " > dev-commits-src-main-unsubscr...@freebsd.org" > _______________________________________________ dev-commits-src-main@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main To unsubscribe, send any mail to "dev-commits-src-main-unsubscr...@freebsd.org"
