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.
The main reason of replacing incumbent "inline append" is
to have a single code base both for user space and kernel space.
that's orthogonal to what this patchseries is doing from the kernel
maintainer's perspective: it's polluting the driver with a
CodingStyle-violating (see, e.g., Chapter 12) 6000+ lines of code -
Regarding coding style - AFAICT that's basically:
ERROR: Macros with complex values should be enclosed in parenthesis
and I am wiling to find a different approach.
There's that, too.
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.
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.
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.
inline append *could run* elsewhere , but it doesn't AFAICT. Last time
I checked, USDPAA and AIOP use RTA.
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.
Error checking is not redundant, as explained above.
kernel-doc is always helpful.
Coding Style can be fixed.
Thanks,
Horia
--
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
