DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Loftus, Ciara" <ciara.loftus@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>,
	"Richardson, Bruce" <bruce.richardson@intel.com>
Cc: "techboard@dpdk.org" <techboard@dpdk.org>,
	"dev@dpdk.org" <dev@dpdk.org>,
	 "Miskell, Timothy" <timothy.miskell@intel.com>
Subject: RE: [PATCH 1/2] net/iavf: support VF initiated resets
Date: Thu, 9 Oct 2025 10:30:55 +0000	[thread overview]
Message-ID: <PH3PPF7DB70F9CE21D69B81237AB58593BD8EEEA@PH3PPF7DB70F9CE.namprd11.prod.outlook.com> (raw)
In-Reply-To: <3977268.0vhOF50zNu@thomas>

> 
> 03/10/2025 12:29, Loftus, Ciara:
> > > > > Introduce a function that allows a VF to request the PF to reset itself.
> > > > > This is useful for example when the application detects that one of the
> > > > > queues have hung or any event where a reset is required and the PF is
> > > > > unlikely to trigger it.
> > > > >
> [...]
> > > >
> > > > In general, I'm not a huge fan of adding driver-specific functions and I
> > > > feel like this should fit under the existing reset APIs in some way. That
> > > > should avoid the need to update (or such a big update) to testpmd, for
> > > > example.
> > > > Some thoughts here:
> > > >
> > > > 1. we could add a devarg to the driver to adjust whether reset does a
> > > >    "softer" reset of the VF just resetting itself, or a "hard" reset where the
> > > >    PF does a fuller reset of the VF.
> > > > 2. rather than a devarg, would do use a driver-specific function to adjust
> > > >    this behaviour. The difference here would be that the driver-specific
> > > >    function would be an init-time one rather than runtime, so the runtime
> of
> > > >    the app, like testpmd, would be generic.
> > > > 3. the most generic solution would be to add an additional parameter to
> the
> > > >    reset() function itself to specify a hard or soft reset. This would mean
> > > >    updating all drivers to handle the new parameter (shouldn't be hard,
> since
> > > >    it would be __rte_unused in all cases by default). This also opens up
> > > >    the possibility of other drivers - especially VFs - using it in the same
> > > >    way. We could actually document that the "hard" option "may be used
> by
> > > VF
> > > >    drivers to request a full reset of the VF by the PF".
> > >
> > > Hi Bruce,
> > >
> > > I did not make it clear in my commit messages the full purpose of this new
> API.
> > > Along with resetting the VF, the VF is also reconfigured and restarted
> > > transparently, using the existing iavf_handle_hw_reset implementation.
> > > While there is probably merit in extending the reset API to include a
> soft/hard
> > > reset flag, I would still need this new API to fulfil the purpose described
> above.
> > > If we wanted to make this generic I see two options:
> > > 1. extend the reset API to optionally reconfigure and restart
> > > 2. introduce a new generic API that performs a reset followed by a
> reconfigure
> > > and restart.
> >
> > I've submitted a v2 and renamed the function to "restore" instead of "reset"
> to
> > better describe the behaviour of the function - not just resetting, but also
> > restoring configuration afterwards and restarting the device.
> >
> > I agree that it would be best to avoid a driver-specific function like this. After
> some
> > thought I don't think we can extend the reset API to include this behaviour,
> the
> > reset API should probably leave the device in a reset state.
> >
> > I'm wondering if the community would be in favour of creating a new ethdev
> API
> > for restoring/reinitialising a device. It would essentially comprise of reset,
> > reconfigure and queue setup, and optionally device start?
> 
> It is confusing to add an API if it is a perfect overlap
> of what exists already.
> If you want to add a new API, you need to explain how it is different
> of a combination of reset/configure/start.

Apologies for any confusion.
The new API would perform only the necessary reconfiguration
and setup steps under-the-hood and use the existing device
configuration. This would differ from a combination of
reset/reconfigure/start in two ways:
1. In some cases (at least in the case of the iavf driver), a
full rte_eth_dev_configure is not required after an rte_eth_dev_reset.
So the new API could just call the device callbacks for those functions,
rather than forcing the user to call the ethdev functions and perform
some unnecessary reconfiguration steps.
2. It takes the burden off the user to build up the device configuration
again after the reset. Any pertinent configuration can be stored before
the reset and used to reconfigure the device transparently. All the user
needs to supply is the port id.

Anyway, I've submitted a v3 of the non-generic version of this for the
iavf driver, so if there is no appetite for the generic approach I'll
continue to pursue the driver-specific one.

Thanks,
Ciara


  reply	other threads:[~2025-10-09 10:31 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-29 14:30 Ciara Loftus
2025-09-29 14:30 ` [PATCH 2/2] net/iavf: add reset VF command to testpmd Ciara Loftus
2025-09-30  9:07 ` [PATCH 1/2] net/iavf: support VF initiated resets Bruce Richardson
2025-09-30 12:30   ` Stephen Hemminger
2025-10-01  8:37   ` Loftus, Ciara
2025-10-03 10:29     ` Loftus, Ciara
2025-10-08 10:13       ` Thomas Monjalon
2025-10-09 10:30         ` Loftus, Ciara [this message]
2025-10-09 14:58           ` Thomas Monjalon
2025-10-10 13:48             ` Loftus, Ciara
2025-10-03 10:23 ` [PATCH v2 " Ciara Loftus
2025-10-03 10:23   ` [PATCH v2 2/2] net/iavf: add restore command to testpmd Ciara Loftus
2025-10-09 10:28   ` [PATCH v3 1/2] net/iavf: support VF initiated resets Ciara Loftus
2025-10-09 10:28     ` [PATCH v3 2/2] net/iavf: add restore command to testpmd Ciara Loftus
2025-10-10 16:36       ` Bruce Richardson
2025-10-10 16:29     ` [PATCH v3 1/2] net/iavf: support VF initiated resets Bruce Richardson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=PH3PPF7DB70F9CE21D69B81237AB58593BD8EEEA@PH3PPF7DB70F9CE.namprd11.prod.outlook.com \
    --to=ciara.loftus@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=techboard@dpdk.org \
    --cc=thomas@monjalon.net \
    --cc=timothy.miskell@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).