RE: [PATCH v5 2/4] bus/pci: introduce the PCIe TLP Processing Hints API

[Wathsala Wathawana Vithanage <wathsala.vithanage@arm.com>]

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Thursday, June 5, 2025 5:19 AM
> To: Wathsala Wathawana Vithanage <wathsala.vithanage@arm.com>
> Cc: Chenbo Xia <chenbox@nvidia.com>; Nipun Gupta <nipun.gupta@amd.com>;
> Anatoly Burakov <anatoly.burakov@intel.com>; Gaetan Rivet <grive@u256.net>;
> dev@dpdk.org; nd <nd@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Dhruv Tripathi <Dhruv.Tripathi@arm.com>
> Subject: Re: [PATCH v5 2/4] bus/pci: introduce the PCIe TLP Processing Hints API
> 
> On Wed, Jun 04, 2025 at 10:52:24PM +0000, Wathsala Wathawana Vithanage
> wrote:
> > > > rte_pci_tph_st_{get, set} functions will return an error if
> > > > processing any of the rte_tph_info objects fails. The API does not
> > > > indicate which entry in the rte_tph_info array was executed
> > > > successfully and which caused an error. Therefore, in case of an
> > > > error, the caller should discard the output. If rte_pci_tph_set
> > > > returns an error, it should be treated as a partial error. Hence,
> > > > the steering-tag update on the device should be considered partial
> > > > and inconsistent with the expected
> > > outcome.
> > > > This should be resolved by resetting the endpoint device before
> > > > further attempts to set steering tags.
> > >
> > > This seems very clunky for the user. Is there a fundamental reason
> > > why we cannot report out what ones passed or failed?
> > >
> > > If it's a limitation of the kernel IOCTL, how about just making one
> > > ioctl for each individual op requested, one at a time. That way we
> > > will know what failed to report it?
> > >
> >
> > The V1 of the kernel patch had that feature, but it was frowned upon,
> > and I was asked to implement the IOCTL this way. Please find it here
> > (V1)
> > https://lore.kernel.org/kvm/20250221224638.1836909-1-wathsala.vithanag
> > e@arm.com/T/#me73cf9b9c87da97d7d9461dfb97863b78ca1755b
> >
> Read the thread. However, from my reading, there is nothing in there that
> mandates having an interface where the user won't know the state on error.
> We need some method to have userspace know what tags were applied or not on
> failure. Resetting the whole device is not a good solution. Whatever API is
> provided, if it is going to take multiple ops in one go it needs to either return the
> number applied on failure, or if just returning success/failure, it should rollback
> the successful ones to give an all-or-nothing interface.
> 
> /Bruce

I will bring this up in the V2 review. If there is a pushback we can alternatively change
the API to do one tag at a time.

Thanks.