On Mon, 21 Jul 2014 10:47:49 +0300 Horia Geantă <horia.geanta@xxxxxxxxxxxxx> wrote: > On 7/19/2014 4:23 AM, Kim Phillips wrote: > > On Sat, 19 Jul 2014 02:51:30 +0300 > > Horia Geantă <horia.geanta@xxxxxxxxxxxxx> wrote: > > > >> On 7/19/2014 1:13 AM, Kim Phillips wrote: > >>> On Fri, 18 Jul 2014 19:37:17 +0300 > >>> Horia Geanta <horia.geanta@xxxxxxxxxxxxx> wrote: > >>> > >>>> This patch set adds Run Time Assembler (RTA) SEC descriptor library. > >>>> > >>> which can only mean it's slower and more susceptible to bugs - and > >>> AFAICT for no superior technical advantage: NACK from me. > >> > >> The fact that the code size is bigger doesn't necessarily mean a bad thing: > >> 1-code is better documented - cloc reports ~ 1000 more lines of > >> comments; patch 09 even adds support for generating a docbook > >> 2-pure code (i.e. no comments, white spaces) - cloc reports ~ 5000 more > >> lines; this reflects two things, AFAICT: > >> 2.1-more features: options (for e.g. new SEC instructions, little endian > >> env. support), platform support includes Era 7 and Era 8, i.e. > >> Layerscape LS1 and LS2; this is important to note, since plans are to > >> run the very same CAAM driver on ARM platforms > > > > um, *those* features should not cost any driver *that many* lines of > > code! > > You are invited to comment on the code at hand. I am pretty sure it's > not perfect. I can see RTA is responsible for the code size increase, not the features. And just because RTA has - or has plans for - those features don't justify the kernel driver adopting RTA over inline-append. > >> 2.2-more error-checking - from this perspective, I'd say driver is less > >> susceptible to bugs, especially subtle ones in CAAM descriptors that are > >> hard to identify / debug; RTA will complain when generating descriptors > >> using features (say a new bit in an instruction opcode) that are not > >> supported on the SEC on device > > > > ? The hardware does the error checking. This just tells me RTA is > > slow, inflexible, and requires unnecessary maintenance by design: > > all the more reason to keep it out of the kernel :) > > This is just like saying a toolchain isn't performing any checks and > lets the user generate invalid machine code and deal with HW errors. > > Beside this, there are (quite a few) cases when SEC won't emit any > error, but still the results are different than expected. > SEC HW is complex enough to deserve descriptor error checking. if part of RTA's objective is to cater to new SEC programmers, great, but that doesn't mean it belongs in the crypto API driver's limited input parameter set, and fixed descriptors operating environment: it's not the place to host an entire SEC toolchain. > >> RTA currently runs on: > >> -QorIQ platforms - userspace (USDPAA) > >> -Layerscape platforms - AIOP accelerator > >> (obviously, plans are to run also on QorIQ/PowerPC and LS/ARM kernels) > > > > inline append runs elsewhere, too, but I don't see how this is > > related to the subject I'm bringing up. > > This is relevant, since having a piece of SW running in multiple > environments should lead to better testing, more exposure, finding bugs > faster. that doesn't defeat the fact that more lines of code to do the same thing is always going to be a more bug-prone way of doing it. > inline append *could run* elsewhere , but it doesn't AFAICT. Last time > I checked, USDPAA and AIOP use RTA. inline append runs in ASF, and has been available for all upstream for years. > >> Combined with: > >> -comprehensive unit testing suite > >> -RTA kernel port is bit-exact in terms of SEC descriptors hex dumps with > >> inline append; besides this, it was tested with tcrypt and in IPsec > >> scenarios > >> I would say that RTA is tested more than inline append. In the end, this > >> is a side effect of having a single code base. > > > > inline append has been proven stable for years now. RTA just adds > > redundant code and violates CodingStyle AFAICT. > > New platform support is not redundant. No, RTA is. > Error checking is not redundant, as explained above. It is: the kernel has fixed descriptors. > kernel-doc is always helpful. it doesn't matter how much you decorate it. > Coding Style can be fixed. inline append isn't broken. Kim -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
