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

["lihuisong (C)" <lihuisong@huawei.com>]

在 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>