netdev - RE: [PATCH net-next v2 0/8] ice: support dynamic interrupt allocation

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <PH0PR11MB5095013B20B313877F36B35CD6869@PH0PR11MB5095.namprd11.prod.outlook.com>
Date:   Wed, 22 Mar 2023 19:03:48 +0000
From:   "Keller, Jacob E" <jacob.e.keller@...el.com>
To:     "Raczynski, Piotr" <piotr.raczynski@...el.com>
CC:     "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "Swiatkowski, Michal" <michal.swiatkowski@...el.com>,
        "Saleem, Shiraz" <shiraz.saleem@...el.com>,
        "Samudrala, Sridhar" <sridhar.samudrala@...el.com>,
        "Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
        "Lobakin, Aleksander" <aleksander.lobakin@...el.com>,
        "Czapnik, Lukasz" <lukasz.czapnik@...el.com>
Subject: RE: [PATCH net-next v2 0/8] ice: support dynamic interrupt allocation



> -----Original Message-----
> From: Raczynski, Piotr <piotr.raczynski@...el.com>
> Sent: Wednesday, March 22, 2023 11:41 AM
> To: Keller, Jacob E <jacob.e.keller@...el.com>
> Cc: intel-wired-lan@...ts.osuosl.org; netdev@...r.kernel.org; Swiatkowski,
> Michal <michal.swiatkowski@...el.com>; Saleem, Shiraz
> <shiraz.saleem@...el.com>; Samudrala, Sridhar
> <sridhar.samudrala@...el.com>; Brandeburg, Jesse
> <jesse.brandeburg@...el.com>; Lobakin, Aleksander
> <aleksander.lobakin@...el.com>; Czapnik, Lukasz <lukasz.czapnik@...el.com>
> Subject: Re: [PATCH net-next v2 0/8] ice: support dynamic interrupt allocation
> 
> On Wed, Mar 22, 2023 at 10:33:31AM -0700, Jacob Keller wrote:
> >
> >
> > On 3/22/2023 9:25 AM, Piotr Raczynski wrote:
> > > This patchset reimplements MSIX interrupt allocation logic to allow dynamic
> > > interrupt allocation after MSIX has been initially enabled. This allows
> > > current and future features to allocate and free interrupts as needed and
> > > will help to drastically decrease number of initially preallocated
> > > interrupts (even down to the API hard limit of 1). Although this patchset
> > > does not change behavior in terms of actual number of allocated interrupts
> > > during probe, it will be subject to change.
> > >
> > > First few patches prepares to introduce dynamic allocation by moving
> > > interrupt allocation code to separate file and update allocation API used
> > > in the driver to the currently preferred one.
> > >
> > > Due to the current contract between ice and irdma driver which is directly
> > > accessing msix entries allocated by ice driver, even after moving away from
> > > older pci_enable_msix_range function, still keep msix_entries array for
> > > irdma use.
> > >
> > > Next patches refactors and removes redundant code from SRIOV related logic
> > > as it also make it easier to move away from static allocation scheme.
> > >
> > > Last patches actually enables dynamic allocation of MSIX interrupts. First,
> > > introduce functions to allocate and free interrupts individually. This sets
> > > ground for the rest of the changes even if that patch still allocates the
> > > interrupts from the preallocated pool. Since this patch starts to keep
> > > interrupt details in ice_q_vector structure we can get rid of functions
> > > that calculates base vector number and register offset for the interrupt
> > > as it is equal to the interrupt index. Only keep separate register offset
> > > functions for the VF VSIs.
> > >
> > > Next, replace homegrown interrupt tracker with much simpler xarray based
> > > approach. As new API always allocate interrupts one by one, also track
> > > interrupts in the same manner.
> > >
> > > Lastly, extend the interrupt tracker to deal both with preallocated and
> > > dynamically allocated vectors and use pci_msix_alloc_irq_at and
> > > pci_msix_free_irq functions. Since not all architecture supports dynamic
> > > allocation, check it before trying to allocate a new interrupt.
> > >
> > > As previously mentioned, this patchset does not change number of initially
> > > allocated interrupts during init phase but now it can and will likely be
> > > changed.
> > >
> > > Patch 1-3 -> move code around and use newer API
> > > Patch 4-5 -> refactor and remove redundant SRIOV code
> > > Patch 6   -> allocate every interrupt individually
> > > Patch 7   -> replace homegrown interrupt tracker with xarray
> > > Patch 8   -> allow dynamic interrupt allocation
> > >
> > > Change history:
> > > v1 -> v2:
> > > - ice: refactor VF control VSI interrupt handling
> > >   - move ice_get_vf_ctrl_vsi to ice_lib.c (ice_vf_lib.c depends on
> > >     CONFIG_PCI_IOV)
> > >
> >
> > The other option would have been to make ice_vf_lib.h have a no-op
> > function that always returned NULL, since we generally would know that
> > there are no VF ctrl VSI if CONFIG_PCI_IOV is disabled.
> >
> > But I'm ok with it being in ice_lib.c too.
> >
> > Thanks,
> > Jake
> 
> Thanks, that makes more sense, a little bit too hasty here.

The main callers are in ice_lib.c so I think its ok to keep it there as well. I think we can go with v2 unless some other comments come up.

Thanks,
Jake