mbox series

[net-next,00/15,pull,request] 100GbE Intel Wired LAN Driver Updates 2021-04-14

Message ID 20210415003013.19717-1-anthony.l.nguyen@intel.com
Headers show
Series 100GbE Intel Wired LAN Driver Updates 2021-04-14 | expand

Message

Tony Nguyen April 15, 2021, 12:29 a.m. UTC
This series contains updates to ice driver only.

Bruce changes and removes open coded values to instead use existing
kernel defines and suppresses false cppcheck issues.

Ani adds new VSI states to track netdev allocation and registration. He
also removes leading underscores in the ice_pf_state enum.

Jesse refactors ITR by introducing helpers to reduce duplicated code and
structures to simplify checking of ITR mode. He also triggers a software
interrupt when exiting napi poll or busy-poll to ensure all work is
processed. Modifies /proc/iomem to display driver name instead of PCI
address. He also changes the checks of vsi->type to use a local variable
in ice_vsi_rebuild() and removes an unneeded struct member.

Jake replaces the driver's adaptive interrupt moderation algorithm to
use the kernel's DIM library implementation.

Scott reworks module reads to reduce the number of reads needed and
remove excessive increment of QSFP page.

Brett sets the vsi->vf_id to invalid for non-VF VSIs.

Paul removes the return value from ice_vsi_manage_rss_lut() as it's not
communicating anything critical. He also reduces the scope of a
variable.

The following are changes since commit 3a1aa533f7f676aad68f8dbbbba10b9502903770:
  Merge tag 'linux-can-next-for-5.13-20210414' of git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue 100GbE

Anirudh Venkataramanan (2):
  ice: Drop leading underscores in enum ice_pf_state
  ice: Add new VSI states to track netdev alloc/registration

Brett Creeley (1):
  ice: Set vsi->vf_id as ICE_INVAL_VFID for non VF VSI types

Bruce Allan (2):
  ice: use kernel definitions for IANA protocol ports and ether-types
  ice: suppress false cppcheck issues

Jacob Keller (1):
  ice: replace custom AIM algorithm with kernel's DIM library

Jesse Brandeburg (6):
  ice: refactor interrupt moderation writes
  ice: manage interrupts during poll exit
  ice: refactor ITR data structures
  ice: print name in /proc/iomem
  ice: use local for consistency
  ice: remove unused struct member

Paul M Stillwell Jr (2):
  ice: remove return variable
  ice: reduce scope of variable

Scott W Taylor (1):
  ice: Reimplement module reads used by ethtool

 drivers/net/ethernet/intel/Kconfig            |   1 +
 drivers/net/ethernet/intel/ice/ice.h          |  78 ++--
 drivers/net/ethernet/intel/ice/ice_base.c     |  25 +-
 drivers/net/ethernet/intel/ice/ice_controlq.c |   6 +-
 drivers/net/ethernet/intel/ice/ice_controlq.h |   1 -
 drivers/net/ethernet/intel/ice/ice_dcb.c      |   8 +-
 drivers/net/ethernet/intel/ice/ice_dcb_lib.c  |   2 +-
 drivers/net/ethernet/intel/ice/ice_ethtool.c  | 105 ++++--
 .../net/ethernet/intel/ice/ice_ethtool_fdir.c |   2 +-
 .../net/ethernet/intel/ice/ice_flex_pipe.c    |   3 +
 .../net/ethernet/intel/ice/ice_hw_autogen.h   |   1 +
 drivers/net/ethernet/intel/ice/ice_lib.c      | 236 ++++++------
 drivers/net/ethernet/intel/ice/ice_lib.h      |   5 +-
 drivers/net/ethernet/intel/ice/ice_main.c     | 353 ++++++++++++------
 drivers/net/ethernet/intel/ice/ice_nvm.c      |   1 +
 drivers/net/ethernet/intel/ice/ice_sched.c    |   1 +
 drivers/net/ethernet/intel/ice/ice_txrx.c     | 314 +++-------------
 drivers/net/ethernet/intel/ice/ice_txrx.h     |  36 +-
 drivers/net/ethernet/intel/ice/ice_type.h     |   3 -
 .../ethernet/intel/ice/ice_virtchnl_fdir.c    |   6 +-
 .../net/ethernet/intel/ice/ice_virtchnl_pf.c  |  25 +-
 drivers/net/ethernet/intel/ice/ice_xsk.c      |   9 +-
 22 files changed, 597 insertions(+), 624 deletions(-)

Comments

Jakub Kicinski April 15, 2021, 4:46 p.m. UTC | #1
On Wed, 14 Apr 2021 17:30:03 -0700 Tony Nguyen wrote:
> +static void ice_tx_dim_work(struct work_struct *work)
> +{
> +	struct ice_ring_container *rc;
> +	struct ice_q_vector *q_vector;
> +	struct dim *dim;
> +	u16 itr, intrl;
> +
> +	dim = container_of(work, struct dim, work);
> +	rc = container_of(dim, struct ice_ring_container, dim);
> +	q_vector = container_of(rc, struct ice_q_vector, tx);
> +
> +	if (dim->profile_ix >= ARRAY_SIZE(tx_profile))
> +		dim->profile_ix = ARRAY_SIZE(tx_profile) - 1;
> +
> +	/* look up the values in our local table */
> +	itr = tx_profile[dim->profile_ix].itr;
> +	intrl = tx_profile[dim->profile_ix].intrl;
> +
> +	ice_write_itr(rc, itr);
> +	ice_write_intrl(q_vector, intrl);
> +
> +	dim->state = DIM_START_MEASURE;

Are you only doing register writes in ice_write_itr/intrl or talk to FW?
Scheduler is expensive so you can save real cycles if you don't have to
rely on a work to do the programming (not sure how hard that is with
DIM, but since you're already sorta poking at the internals I thought
I'd ask).
Jacob Keller April 15, 2021, 5:03 p.m. UTC | #2
> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Thursday, April 15, 2021 9:47 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Cc: davem@davemloft.net; Keller, Jacob E <jacob.e.keller@intel.com>;
> netdev@vger.kernel.org; sassmann@redhat.com; Brandeburg, Jesse
> <jesse.brandeburg@intel.com>; Brelinski, TonyX <tonyx.brelinski@intel.com>
> Subject: Re: [PATCH net-next 05/15] ice: replace custom AIM algorithm with
> kernel's DIM library
> 
> On Wed, 14 Apr 2021 17:30:03 -0700 Tony Nguyen wrote:
> > +static void ice_tx_dim_work(struct work_struct *work)
> > +{
> > +	struct ice_ring_container *rc;
> > +	struct ice_q_vector *q_vector;
> > +	struct dim *dim;
> > +	u16 itr, intrl;
> > +
> > +	dim = container_of(work, struct dim, work);
> > +	rc = container_of(dim, struct ice_ring_container, dim);
> > +	q_vector = container_of(rc, struct ice_q_vector, tx);
> > +
> > +	if (dim->profile_ix >= ARRAY_SIZE(tx_profile))
> > +		dim->profile_ix = ARRAY_SIZE(tx_profile) - 1;
> > +
> > +	/* look up the values in our local table */
> > +	itr = tx_profile[dim->profile_ix].itr;
> > +	intrl = tx_profile[dim->profile_ix].intrl;
> > +
> > +	ice_write_itr(rc, itr);
> > +	ice_write_intrl(q_vector, intrl);
> > +
> > +	dim->state = DIM_START_MEASURE;
> 
> Are you only doing register writes in ice_write_itr/intrl or talk to FW?
> Scheduler is expensive so you can save real cycles if you don't have to
> rely on a work to do the programming (not sure how hard that is with
> DIM, but since you're already sorta poking at the internals I thought
> I'd ask).

Hmm. I believe we only have to do register writes. If I recall, at least based on reading the other DIMLIB implementations, they seem to have mostly moved to a work item for apparently moving these changes out of the hot path.. but maybe that's not really an issue. Ofcourse the current dim implementation enforces use of the work queue, so I think it would require refactoring the library to support doing immediate application as opposed to using the work item..
Jakub Kicinski April 15, 2021, 5:07 p.m. UTC | #3
On Thu, 15 Apr 2021 17:03:23 +0000 Keller, Jacob E wrote:
> > On Wed, 14 Apr 2021 17:30:03 -0700 Tony Nguyen wrote:  
> > > +static void ice_tx_dim_work(struct work_struct *work)
> > > +{
> > > +	struct ice_ring_container *rc;
> > > +	struct ice_q_vector *q_vector;
> > > +	struct dim *dim;
> > > +	u16 itr, intrl;
> > > +
> > > +	dim = container_of(work, struct dim, work);
> > > +	rc = container_of(dim, struct ice_ring_container, dim);
> > > +	q_vector = container_of(rc, struct ice_q_vector, tx);
> > > +
> > > +	if (dim->profile_ix >= ARRAY_SIZE(tx_profile))
> > > +		dim->profile_ix = ARRAY_SIZE(tx_profile) - 1;
> > > +
> > > +	/* look up the values in our local table */
> > > +	itr = tx_profile[dim->profile_ix].itr;
> > > +	intrl = tx_profile[dim->profile_ix].intrl;
> > > +
> > > +	ice_write_itr(rc, itr);
> > > +	ice_write_intrl(q_vector, intrl);
> > > +
> > > +	dim->state = DIM_START_MEASURE;  
> > 
> > Are you only doing register writes in ice_write_itr/intrl or talk to FW?
> > Scheduler is expensive so you can save real cycles if you don't have to
> > rely on a work to do the programming (not sure how hard that is with
> > DIM, but since you're already sorta poking at the internals I thought
> > I'd ask).  
> 
> Hmm. I believe we only have to do register writes. If I recall, at
> least based on reading the other DIMLIB implementations, they seem to
> have mostly moved to a work item for apparently moving these changes
> out of the hot path.. but maybe that's not really an issue. Ofcourse
> the current dim implementation enforces use of the work queue, so I
> think it would require refactoring the library to support doing
> immediate application as opposed to using the work item..

I think it may be because of FW programming which needs to sleep. The
extra scheduler work because of this async mechanism does impact real
workloads (admittedly not more than 1%), but up to you.