Re: [dpdk-dev] [PATCH] driver: i40evf device initialization

["Xueming(Steven) Li" <xuemingl@nvidia.com>]

On Wed, 2021-10-13 at 09:21 -0400, Ben Magistro wrote:
> Hello,
> 
> Replying here as I'm a little stuck and hoping someone has some
> advice for what the next steps should be.
> 
> Going from the list above of how to get this noticed by the LTS
> maintainer(s), the patch, well commit message + subject were revised
> and resent to the list
> (https://patches.dpdk.org/project/dpdk/patch/20211012141752.6376-1-
> koncept1@gmail.com/) but the i40evf has since been removed from main
> already so options 1 & 2 seem to no longer apply.  This seems to put
> us into option 3 of a backported patch?  Is it just a subject line
> change then or can this be pulled out of the "not applicable" pile
> still?

Hi Ben,

Since it doesn't apply to upstream, option 3 should be good enough,
just add [20.11] or [19.11] as prefix and send to stable@dpdk.org.

Regards,
Xueming Li


> 
> Thanks and appreciate the advice,
> 
> Ben Magistro
> 
> On Mon, Sep 13, 2021 at 10:52 PM Ben Magistro <koncept1@gmail.com>
> wrote:
> > +cc: stable@dpdk.org
> > 
> > Per discussions here, cc'ing stable for fix to be applied to LTS as
> > i40evf is being removed from next.
> > 
> > On Thu, Sep 2, 2021 at 8:37 AM Xueming(Steven) Li
> > <xuemingl@nvidia.com> wrote:
> > > 
> > > 
> > > 
> > > > -----Original Message-----
> > > > From: Ferruh Yigit <ferruh.yigit@intel.com>
> > > > Sent: Monday, August 30, 2021 5:43 PM
> > > > To: Xueming(Steven) Li <xuemingl@nvidia.com>; Kevin Traynor
> > > > <ktraynor@redhat.com>; Ben Magistro <koncept1@gmail.com>;
> > > > dev@dpdk.org; Beilei Xing <beilei.xing@intel.com>; Luca
> > > > Boccassi <bluca@debian.org>; Christian Ehrhardt
> > > > <christian.ehrhardt@canonical.com>
> > > > Cc: ben.magistro@trinitycyber.com;
> > > > stefan.baranoff@trinitycyber.com; Qi Zhang
> > > > <qi.z.zhang@intel.com>
> > > > Subject: Re: [dpdk-dev] [PATCH] driver: i40evf device
> > > > initialization
> > > > 
> > > > On 8/27/2021 7:28 AM, Xueming(Steven) Li wrote:
> > > > > 
> > > > > 
> > > > > > -----Original Message-----
> > > > > > From: Kevin Traynor <ktraynor@redhat.com>
> > > > > > Sent: Thursday, August 26, 2021 6:46 PM
> > > > > > To: Ferruh Yigit <ferruh.yigit@intel.com>; Ben Magistro
> > > > > > <koncept1@gmail.com>; dev@dpdk.org; Beilei Xing
> > > > > > <beilei.xing@intel.com>; Luca Boccassi <bluca@debian.org>;
> > > > > > Christian
> > > > > > Ehrhardt <christian.ehrhardt@canonical.com>;
> > > > > > Xueming(Steven) Li
> > > > > > <xuemingl@nvidia.com>
> > > > > > Cc: ben.magistro@trinitycyber.com;
> > > > > > stefan.baranoff@trinitycyber.com;
> > > > > > Qi Zhang <qi.z.zhang@intel.com>
> > > > > > Subject: Re: [dpdk-dev] [PATCH] driver: i40evf device
> > > > > > initialization
> > > > > > 
> > > > > > + Christian and Xueming
> > > > > > 
> > > > > > On 26/08/2021 11:25, Ferruh Yigit wrote:
> > > > > > > On 8/25/2021 8:45 PM, Ben Magistro wrote:
> > > > > > > > The i40evf driver is not initializing the eth_dev
> > > > > > > > attribute which
> > > > > > > > can result in a nullptr dereference. Changes were
> > > > > > > > modeled after the
> > > > > > > > iavf_dev_init() per suggestion from the mailing
> > > > > > > > list[1].
> > > > > > > > 
> > > > > > > > [1] https://mails.dpdk.org/archives/dev/2021-
> > > > > > > > August/217251.html
> > > > > > > > 
> > > > > > > > Signed-off-by: Ben Magistro <koncept1@gmail.com>
> > > > > > > 
> > > > > > > i40evf will be removed in this release. But I guess it
> > > > > > > helps for
> > > > > > > stable releases to first merge the fixes and later
> > > > > > > removed it, not sure.
> > > > > > > 
> > > > > > > @Luca, @Kevin, do you prefer this patch directly to
> > > > > > > stable repos, or
> > > > > > > through the main repo?
> > > > > > 
> > > > > > I'll leave to Luca/Xueming and Christian to say if they
> > > > > > have a
> > > > > > preference, but I'd guess either way is fine from stable
> > > > > > view once it has fixes/stable tags or LTS patch prefix (it
> > > > > > doesn't have any of
> > > > these at present).
> > > > > 
> > > > > Yes, any option will make it being noticed by LTS maintainer:
> > > > > 1. patches accepted by main with "fix" in subject 2. patches
> > > > > accepted
> > > > > by main with "cc: stable@dpdk.org" in commit message 3.
> > > > > patches
> > > > > backported to LTS, sent to stable maillist with LTS prefix,
> > > > > for example "[20.11]"
> > > > > 
> > > > 
> > > > Thanks Xueming,
> > > > 
> > > > But is there a preferences for this case?
> > > > 
> > > > The i40evf will be removed from main repo, is it better
> > > > 1- first apply the fix and remove the component from main (I
> > > > assume fix still will be bacported to LTS in this case) or
> > > > 2- remove the i40evf from main (without fix), apply the fix
> > > > directly to the LTS.
> > > 
> > > Both options will work, the first is more easy and common I
> > > guess, both 19.11 LTS and 20.11 LTS maintainer can find it.
> > > 
> > > > 
> > > > Thanks,
> > > > ferruh
> > > > 
> > > > > > 
> > > > > > > i40evf won't be tested in the main anyway, since it would
> > > > > > > be removed
> > > > > > > before -rc1 testing, so it looks like there won't be any
> > > > > > > difference from testing point of view.
> > > > > > > 
> > > > > > > 
> > > > > > > > ---
> > > > > > > >   drivers/net/i40e/i40e_ethdev_vf.c | 8 ++++++--
> > > > > > > >   1 file changed, 6 insertions(+), 2 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > > > > > > > b/drivers/net/i40e/i40e_ethdev_vf.c
> > > > > > > > index 0cfe13b7b2..ccdce9a16a 100644
> > > > > > > > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > > > > > > > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > > > > > > > @@ -1564,8 +1564,9 @@ i40evf_dev_alarm_handler(void
> > > > > > > > *param)  static
> > > > > > > > int  i40evf_dev_init(struct rte_eth_dev *eth_dev)  {
> > > > > > > > -        struct i40e_hw *hw
> > > > > > > > -                = I40E_DEV_PRIVATE_TO_HW(eth_dev-
> > > > > > > > >data->dev_private);
> > > > > > > > +        struct i40e_adapter *adapter =
> > > > > > > > +                I40E_DEV_PRIVATE_TO_ADAPTER(eth_dev-
> > > > > > > > >data->dev_private);
> > > > > > > > +        struct i40e_hw *hw =
> > > > > > > > I40E_DEV_PRIVATE_TO_HW(adapter);
> > > > > > > >           struct rte_pci_device *pci_dev =
> > > > > > > > RTE_ETH_DEV_TO_PCI(eth_dev);
> > > > > > > > 
> > > > > > > >           PMD_INIT_FUNC_TRACE();
> > > > > > > > @@ -1596,11 +1597,14 @@ i40evf_dev_init(struct
> > > > > > > > rte_eth_dev *eth_dev)
> > > > > > > >           hw->device_id = pci_dev->id.device_id;
> > > > > > > >           hw->subsystem_vendor_id = pci_dev-
> > > > > > > > >id.subsystem_vendor_id;
> > > > > > > >           hw->subsystem_device_id = pci_dev-
> > > > > > > > >id.subsystem_device_id;
> > > > > > > > +        hw->bus.bus_id = pci_dev->addr.bus;
> > > > > > > >           hw->bus.device = pci_dev->addr.devid;
> > > > > > > >           hw->bus.func = pci_dev->addr.function;
> > > > > > > >           hw->hw_addr = (void *)pci_dev-
> > > > > > > > >mem_resource[0].addr;
> > > > > > > >           hw->adapter_stopped = 1;
> > > > > > > >           hw->adapter_closed = 0;
> > > > > > > > +        hw->back =
> > > > > > > > I40E_DEV_PRIVATE_TO_ADAPTER(eth_dev->data-
> > > > > > > > >dev_private);
> > > > > > > > +        adapter->eth_dev = eth_dev;
> > > > > > > > 
> > > > > > > >           if(i40evf_init_vf(eth_dev) != 0) {
> > > > > > > >                   PMD_INIT_LOG(ERR, "Init vf failed");
> > > > > > > > 
> > > > > > > 
> > > > > 
> > >