Re: [PATCH 3/3] usbip: Make the driver's match function specific

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 18/09/2020 18.49, Shuah Khan wrote:
On 9/18/20 8:31 AM, M. Vefa Bicakci wrote:
Hello Shuah,

I have just cleaned up the patches and run usbip_test.sh with a kernel without
the patches in this series and with a kernel in this series.

I noticed that there is a change in behaviour due to the fact that the new
match function (usbip_match) does not always return true. This causes the
stub device driver's probe() function to not get called at all, as the new
more selective match function will prevent the stub device driver from being
considered as a potential driver for the device under consideration.


Yes. This is the behavior I am concerned about and hence the reason
to use the usbip test to verify this doesn't happen.

With the patch you have the usbip match behavior becomes restrictive
which isn't desirable.

All of this results in the following difference in the logs of the usbip_test.sh,
where the expected kernel log message "usbip-host 2-6: 2-6 is not in match_busid table... skip!"
is not printed by a kernel that includes the patches in this series.

--- unpatched_kernel_log.txt  2020-09-18 17:12:10.654000000 +0300
+++ patched_kernel_log.txt  2020-09-18 17:12:10.654000000 +0300
@@ -213,70 +213,69 @@
      |__ Port 1: Dev 2, If 0, Class=Human Interface Device, Driver=usbhid, 480M
  ==============================================================
  modprobe usbip_host - does it work?
  Should see -busid- is not in match_busid table... skip! dmesg
  ==============================================================
-usbip-host 2-6: 2-6 is not in match_busid table... skip!
  ==============================================================

Do you find this change in behaviour unacceptable?

Yeah. This behavior isn't acceptable.

If no, I can remove this
test case from usbip_test.sh with the same patch. If yes, then there is a need
for a different solution to resolve the unexpected negative interaction between
Bastien's work on generic/specific USB device driver selection and usbip
functionality.


I would recommend finding a different solution. Now that you have the
usbip test handy, you can verify and test for regressions.

thanks,
-- Shuah

Thanks for the feedback, Shuah. I spent some time looking at and instrumenting
the code in an attempt to find another solution, but have not found one.

If the generic/specific USB driver selection functionality that Bastien Nocera
introduced is desired to stay in the kernel, then making usbip_match more
specific appears to be the only option for usbip to be functional without
negatively affecting other device drivers.

Should Bastien's work be reverted until a solution to this issue is found?
Would you (or anyone) have any suggestions? I would be happy to work further
on resolving this issue.

I am including a summary of my findings below my signature.

Thank you,

Vefa

=== === ===

(1) If usbip_match stays as is, then the issue reported by Andrey Konovalov at

      https://lore.kernel.org/linux-usb/CAAeHK+zOrHnxjRFs=OE8T=O9208B9HP_oo8RZpyVOZ9AJ54pAA@xxxxxxxxxxxxxx/

    will remain unresolved and will trickle down to Linux 5.9 due to the following
    commit's interaction with the return-true usbip_match function:

      USB: Simplify USB ID table match
      https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=master&id=0ed9498f9ecfde50c93f3f3dd64b4cd5414d9944

    This commit was mis-classified as a simplification rather than a bug-fix
    and was not backported to linux-5.8.y. (I can explain my reasoning if beneficial.)
    After cherry-picking this commit onto 5.8.10, I can reproduce Andrey's issue
    with Andrey's "keyboard" program at: https://github.com/xairy/raw-gadget.git

(2) If the following patches are also included in the kernel to fix two bugs in the
    USB driver selection logic, then a return-true usbip_match function will cause
    usbip's stub driver to be selected as the more specific device driver for all
    USB devices when the usbip_host module is loaded. (This makes all USB devices
    unavailable on my system, until I unload usbip_host and reload {x,e}hci-pci
    modules.)

      usbcore/driver: Fix specific driver selection
      https://marc.info/?l=linux-usb&m=160037262607960&q=mbox

      usbcore/driver: Fix incorrect downcast
      https://lore.kernel.org/linux-usb/20200917144151.355848-2-m.v.b@xxxxxxxxxx/

    I realize that this impact statement does not match my previous description at

       https://lore.kernel.org/linux-usb/363eab9a-c32a-4c60-4d6b-14ae8d873c52@xxxxxxxxxx/

    which was because I had forgotten to cherry pick 0ed9498f9e ("USB: Simplify
    USB ID table match"; i.e., the commit discussed in the previous bullet point)
    onto my v5.8.10-based test kernel.



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux