netdev - Re: [iwl-next v2 4/7] ice, irdma: move interrupts code to irdma

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZrBbUfqMwniZF1wv@mev-dev.igk.intel.com>
Date: Mon, 5 Aug 2024 06:55:45 +0200
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: Przemek Kitszel <przemyslaw.kitszel@...el.com>
Cc: netdev@...r.kernel.org, pawel.chmielewski@...el.com,
	sridhar.samudrala@...el.com, jacob.e.keller@...el.com,
	pio.raczynski@...il.com, konrad.knitter@...el.com,
	marcin.szycik@...el.com, wojciech.drewek@...el.com,
	nex.sw.ncis.nat.hpm.dev@...el.com, jiri@...nulli.us,
	intel-wired-lan@...ts.osuosl.org
Subject: Re: [iwl-next v2 4/7] ice, irdma: move interrupts code to irdma

On Fri, Aug 02, 2024 at 02:49:08PM +0200, Przemek Kitszel wrote:
> On 8/1/24 11:31, Michal Swiatkowski wrote:
> > Move responsibility of MSI-X requesting for RDMA feature from ice driver
> > to irdma driver. It is done to allow simple fallback when there is not
> > enough MSI-X available.
> > 
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> > ---
> >   drivers/infiniband/hw/irdma/hw.c         |  2 -
> >   drivers/infiniband/hw/irdma/main.c       | 46 ++++++++++++++++-
> >   drivers/infiniband/hw/irdma/main.h       |  3 ++
> >   drivers/net/ethernet/intel/ice/ice.h     |  2 -
> >   drivers/net/ethernet/intel/ice/ice_idc.c | 64 ++++++------------------
> >   include/linux/net/intel/iidc.h           |  2 +
> >   6 files changed, 63 insertions(+), 56 deletions(-)
> > 
> > diff --git a/drivers/infiniband/hw/irdma/hw.c b/drivers/infiniband/hw/irdma/hw.c
> > index ad50b77282f8..69ce1862eabe 100644
> > --- a/drivers/infiniband/hw/irdma/hw.c
> > +++ b/drivers/infiniband/hw/irdma/hw.c
> > @@ -498,8 +498,6 @@ static int irdma_save_msix_info(struct irdma_pci_f *rf)
> >   	iw_qvlist->num_vectors = rf->msix_count;
> >   	if (rf->msix_count <= num_online_cpus())
> >   		rf->msix_shared = true;
> > -	else if (rf->msix_count > num_online_cpus() + 1)
> > -		rf->msix_count = num_online_cpus() + 1;
> >   	pmsix = rf->msix_entries;
> >   	for (i = 0, ceq_idx = 0; i < rf->msix_count; i++, iw_qvinfo++) {
> > diff --git a/drivers/infiniband/hw/irdma/main.c b/drivers/infiniband/hw/irdma/main.c
> > index 3f13200ff71b..69ad137be7aa 100644
> > --- a/drivers/infiniband/hw/irdma/main.c
> > +++ b/drivers/infiniband/hw/irdma/main.c
> > @@ -206,6 +206,43 @@ static void irdma_lan_unregister_qset(struct irdma_sc_vsi *vsi,
> >   		ibdev_dbg(&iwdev->ibdev, "WS: LAN free_res for rdma qset failed.\n");
> >   }
> > +static int irdma_init_interrupts(struct irdma_pci_f *rf, struct ice_pf *pf)
> > +{
> > +	int i;
> > +
> > +	rf->msix_count = num_online_cpus() + IRDMA_NUM_AEQ_MSIX;
> > +	rf->msix_entries = kcalloc(rf->msix_count, sizeof(*rf->msix_entries),
> > +				   GFP_KERNEL);
> > +	if (!rf->msix_entries)
> > +		return -ENOMEM;
> > +
> > +	for (i = 0; i < rf->msix_count; i++)
> > +		if (ice_alloc_rdma_qvector(pf, &rf->msix_entries[i]))
> > +			break;
> > +
> > +	if (i < IRDMA_MIN_MSIX) {
> > +		for (; i >= 0; i--)
> > +			ice_free_rdma_qvector(pf, &rf->msix_entries[i]);
> 
> you call ice_free_rdma_qvector() for i=0 even if the very first alloc
> attempt has failed
> 

Good point, I will fix it, thanks

> > +
> > +		kfree(rf->msix_entries);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	rf->msix_count = i;
> > +
> > +	return 0;
> > +}
> 
> [...]
> 
> > --- a/drivers/infiniband/hw/irdma/main.h
> > +++ b/drivers/infiniband/hw/irdma/main.h
> > @@ -117,6 +117,9 @@ extern struct auxiliary_driver i40iw_auxiliary_drv;
> >   #define IRDMA_IRQ_NAME_STR_LEN (64)
> > +#define IRDMA_NUM_AEQ_MSIX	1
> > +#define IRDMA_MIN_MSIX		2
> > +
> >   enum init_completion_state {
> >   	INVALID_STATE = 0,
> >   	INITIAL_STATE,
> > diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> > index 8247d27541b0..1311be1d2c30 100644
> > --- a/drivers/net/ethernet/intel/ice/ice.h
> > +++ b/drivers/net/ethernet/intel/ice/ice.h
> > @@ -97,8 +97,6 @@
> >   #define ICE_MIN_MSIX		(ICE_MIN_LAN_TXRX_MSIX + ICE_MIN_LAN_OICR_MSIX)
> >   #define ICE_MAX_MSIX		256
> >   #define ICE_FDIR_MSIX		2
> > -#define ICE_RDMA_NUM_AEQ_MSIX	4
> 
> you have to extend commit message to tell why there is a 4 -> 1 change
>

Ok, I will

> > -#define ICE_MIN_RDMA_MSIX	2
> >   #define ICE_ESWITCH_MSIX	1
> >   #define ICE_NO_VSI		0xffff
> >   #define ICE_VSI_MAP_CONTIG	0
> 
>