Re: [REGRESSION] changes to driver_override parsing broke DPDK script

[Greg KH <gregkh@linuxfoundation.org>]

On Fri, Aug 12, 2022 at 10:54:38AM +0800, lihuisong (C) wrote:
> 
> 在 2022/8/12 9:48, Dongdong Liu 写道:
> > cc Huisong who found the issue.
> > 
> > On 2022/8/10 16:21, Greg KH wrote:
> > > On Wed, Aug 10, 2022 at 08:11:33AM +0200, Greg KH wrote:
> > > > On Wed, Aug 10, 2022 at 08:54:36AM +0300, Krzysztof Kozlowski wrote:
> > > > > On 09/08/2022 22:21, Bjorn Helgaas wrote:
> > > > > > [+cc regressions list]
> > > > > > 
> > > > > > 23d99baf9d72 appeared in v5.19-rc1.
> > > > > > 
> > > > > > On Tue, Aug 09, 2022 at 11:29:43AM -0700, Stephen Hemminger wrote:
> > > > > > > This commit broke the driver override script in DPDK.
> > > > > > > This is an API/ABI breakage, please revert or fix the commit.
> > > > > > > 
> > > > > > > Report of problem:
> > > > > > > http://mails.dpdk.org/archives/dev/2022-August/247794.html
> > > > > 
> > > > > Thanks for the report. I'll take a look.
> > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > commit 23d99baf9d729ca30b2fb6798a7b403a37bfb800
> > > > > > > Author: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > > > > Date:   Tue Apr 19 13:34:28 2022 +0200
> > > > > > > 
> > > > > > >     PCI: Use driver_set_override() instead of open-coding
> > > > > > > 
> > > > > > >     Use a helper to set driver_override to the
> > > > > > > reduce amount of duplicated
> > > > > > >     code.  Make the driver_override field const
> > > > > > > char, because it is not
> > > > > > >     modified by the core and it matches other subsystems.
> > > > > > > 
> > > > > > >     Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > > > >     Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> > > > > > >     Signed-off-by: Krzysztof Kozlowski
> > > > > > > <krzysztof.kozlowski@linaro.org>
> > > > > > >     Link: https://lore.kernel.org/r/20220419113435.246203-6-krzysztof.kozlowski@linaro.org
> > > > > > >     Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > > > > 
> > > > > > > 
> > > > > > > The script is sending single nul character to remove override
> > > > > > > and that no longer works.
> > > > > 
> > > > > The sysfs API clearly states:
> > > > > "and
> > > > >  may be cleared with an empty string (echo > driver_override)."
> > > > > Documentation/ABI/testing/sysfs-bus-pci
> > > > > 
> > > > > Sending other data and expecting the same result is not conforming to
> > > > > API. Therefore we have usual example of some undocumented
> > > > > behavior which
> > > > > user-space started relying on and instead using API, user-space expect
> > > > > that undocumented behavior to be back.
> > > > > 
> > > > > Yay! I wonder what is the point to even describe the ABI if user-space
> > > > > can simply ignore it?
> > > > 
> > > > One can argue that a string of just '\0' is an "empty string" and we
> > > > should be able to properly handle this in the kernel.  Heck,
> > > > "\0\0\0\0\0\0" is also an "empty string", right?
> > > > 
> > > > I don't have an issue with fixing the kernel up here, it should be able
> > > > to handle this.
> > > 
> > > Stephen, does the patch below fix this for you?
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > > 
> > > -----------------
> > > 
> > > diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> > > index 15a75afe6b84..676b6275d5b5 100644
> > > --- a/drivers/base/driver.c
> > > +++ b/drivers/base/driver.c
> > > @@ -63,6 +63,12 @@ int driver_set_override(struct device *dev, const
> > > char **override,
> > >      if (len >= (PAGE_SIZE - 1))
> > >          return -EINVAL;
> > > 
> > > +    /*
> > > +     * Compute the real length of the string in case userspace
> > > sends us a
> > > +     * bunch of \0 characters like python likes to do.
> > > +     */
> > > +    len = strlen(s);
> > > +
> > >      if (!len) {
> > >          /* Empty string passed - clear override */
> > >          device_lock(dev);
> > > .
> > > 
> > This patch looks good,  @huisong, please help to test the patch.
> > 
> > Thanks,
> > Dongdong
> > .
> Tested-by: Huisong Li <lihuisong@huawei.com>

Wonderful, thanks!  I'll queue this up to send to Linus after -rc1 is
out.

greg k-h