On Fri, 14 Oct 2016 17:50:40 +0300 Andriy Gapon <a...@freebsd.org> wrote:
> On 14/10/2016 00:39, Lewis Donzis wrote: > > After upgrading to FreeBSD 11.0 and changing source code to use the > > new version of “struct smbcmd”, some commands are not working as > > documented, specifically those that read data. > > > > As an example, SMB_READW is documented as returning the word read > > from the device in rdata.word. However, this doesn’t happen, I > > think because the ioctl request value is defined using _IOW(), so > > the kernel doesn’t copy the data it read back out. > > > > In prior versions, the structure had only a pointer to the data, > > and the smb.c code used copyout() to transfer the data back to > > userland. > > > > As a temporary work-around, we added code to set rbuf to point to > > rdata.word and rcount to two. > > Lewis, > > thank you for the report. This is a bug indeed and your analysis is > correct. Could you please open a bugzilla issue for the bug? > https://bugs.freebsd.org/bugzilla/ > > Looking at ports commit 385155 > https://svnweb.freebsd.org/ports/head/sysutils/xmbmon/files/patch-getMB-smb_ioctl.c?r1=385155&r2=385154&pathrev=385155 > I see that it also used the approach that you use as a workaround. > And that port commit is by Michael Gmelin who made the change to > smb.h in r281985 > https://svnweb.freebsd.org/base?view=revision&revision=281985 So, I > am not sure if the documented approach was known to not work. > > The src change is described as "Expand SMBUS API ...", but in fact it > also _changed_ the existing ioctls. And both binary compatibility > and programming compatibility were broken because of how struct > smbcmd was changed. In FreeBSD we try to not do that without a very > strong reason, but alas. And, as you report, the change was not done > entirely correctly. > > I see several possibilities now. > > Option 1. Change the documentation to reflect the actual behavior. > In this case data.rdata will remain unusable and unused. No > interface changes. > > Option 2. Redefine SMB_READB, SMB_READW and SMB_PCALL ioctls using > _IOWR, so that data.rdata could be returned from kernel. This seems > like a proper fix, but it is another binary level incompatibility. > > Option 3. Use a horrible hack to discover a userland address of > smbcmd and explicitly copyout to data.rdata. No interface > incompatibilities, but it will be a horrible hack. Besides, not sure > how feasible it is. > > Option 4. Revert smb ioctl changes to what they used to be before > r281985. Personally, I would prefer this approach. But now that the > new interface is in 11.0, it means another interface change just like > Option 2. > > I would like to hear other developers' opinions about this situation. > > P.S. > Two changes that I want to do no matter which course of action we > select are: > - revert SMB_MAXBLOCKSIZE to 32 > - remove SMB_TRANS as it does not map to anything defined by the SMBus > specification and it can not be implemented for most, if not all, > SMBus controllers > For some history on these changes, please see also  and  (there were a few discussions and the revision was bumped, I also tried to get some attention, but not enough it seems). Given your recent changes to iicbus in HEAD, I think it would be best to MFC those and go with Option 4 or, if that's to drastic, go with Option 1. Thanks for cleaning after me. - Michael https://lists.freebsd.org/pipermail/freebsd-arch/2015-March/016972.html https://lists.freebsd.org/pipermail/freebsd-arch/2015-May/017157.html -- Michael Gmelin _______________________________________________ email@example.com mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"