On 8/1/22 12:28 PM, Vincent MAILHOL wrote:
On Tue. 2 Aug. 2022 at 02:48, Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> wrote:
On 6/30/22 8:10 PM, Shuah Khan wrote:
On 6/27/22 7:35 PM, Alan Stern wrote:
On Mon, Jun 27, 2022 at 04:54:17PM -0600, Shuah Khan wrote:
On 6/24/22 12:07 PM, Alan Stern wrote:
In the future people will want to make other changes to
include/linux/usb.h and they will not be aware that those changes will
adversely affect usbip, because there is no documentation saying that
the values defined in usb.h are part of a user API. That will be a
problem, because those changes may be serious and important ones, not
just decorative or stylistic as in this case.
How often do these values change based on our past experience with these
fields?
I don't know. You could check the git history to find out for certain.
My guess would be every eight or ten years.
I agree with Hongren that values defined in include/linux/ should not be
part of a user API. There are two choices:
I agree with this in general. I don't think this is an explicit decision
to make them part of API. It is a consequence of simply copying the
transfer_flags. I am with you both on not being able to recognize the
impact until as this is rather obscure usage hidden away in the packets.
These defines aren't directly referenced.
Move the definitions into include/uapi/linux/, or
Wouldn't this be easier way to handle the change? With this option
the uapi will be well documented.
Add code to translate the values between the numbers used in
userspace and the numbers used in the kernel. (This is what
was done for urb->transfer_flags in devio.c:proc_do_submiturb()
near line 1862.)
I looked at the code and looks simple enough. I am okay going this route
if we see issues with the option 1.
It's up to you; either approach is okay with me. However, I do think
that the second option is a little better; I don't see any good reason
why the kernel should be forced to use the same numeric values for these
flags forever. Especially since the only user program that needs to
know them is usbip, which is fairly closely tied to the kernel; if there
were more programs using those values then they would constitute a good
reason for choosing the first option.
Thank you Alan and Hongren for your help with this problem. Since there
are no changes to the flags for the time being, I am comfortable going
with the second option.
I will send a patch soon.
Patch is almost ready to be sent out. Changes aren't bad at all. Hoping to
get this done sooner - summer vacations didn't cooperate.
Just an update that I haven't forgotten and it will taken care of.
thanks,
Thanks for keeping this under your radar. I also have on my TODO list
to send a new version of my patch to add the `URB_FREE_COHERENT' flag
but this time adding an `allocated_length' field to struct urb. I will
wait for your patch to go first. By the way, I will be out for summer
holiday for the next couple of weeks so I wasn't planning to submit
anything soon regardless.
Sounds good. I now have the patch ready to be sent out. I will wait for
the merge window to close before I send it out.
thanks,
-- Shuah
