Page MenuHomeFreeBSD

AES iov optimization

Authored by sef on Dec 12 2018, 3:57 AM.



Right now, aesni_cipher_alloc does a bit of special-casing for CRYPTO_F_IOV, to not do any allocation if the first uio is large enough for the requested size. While working on ZFS crypto port, I ran into horrible performance because the code uses scatter-gather, and many of the times the data to encrypt was in the second entry. This code looks through the list, and tries to see if there is a single uio that can contain the requested data, and, if so, uses that.

This has a slight impact on the current consumers, in that the check is a little more complicated for the ones that use CRYPTO_F_IOV -- but none of them meet the criteria for testing more than one.

Test Plan


Diff Detail

rS FreeBSD src repository - subversion
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

409–432 ↗(On Diff #51893)

Like I mentioned in the other review, I think this is more broadly useful to OCF consumers than just aesni. E.g., armv8_crypto_cipher_alloc was basically copy-pasted from aesni and has the same issue. And the identical optimization can be applied to mbuf chains.

Given the general trend of OCF crypto_* methods that are agnostic to type, maybe it makes sense to just make this an OCF subroutine that operates generically, like crypto_copydata?

static inline void *
m_contiguous_subsegment(struct mbuf *m, size_t skip, size_t len)
        int rel_off;

        MPASS(skip <= INT_MAX);

        m = m_getptr(m, (int)skip, &rel_off);
        if (m == NULL)
                return (NULL);

        MPASS(rel_off >= 0);
        skip = rel_off;
        if (skip + len > m->m_len)
                return (NULL);
        return (mtod(m, char *) + skip);

static void *
cuio_contiguous_subsegment(struct uio *uio, size_t skip, size_t len)
        int rel_off, idx;

        MPASS(skip <= INT_MAX);
        idx = cuio_getptr(uio, (int)skip, &rel_off);
        if (idx < 0)
                return (NULL);

        MPASS(rel_off >= 0);
        skip = rel_off;
        if (skip + len > uio->uio_iov[idx].iov_len)
                return (NULL);
        return ((char *)uio->uio_iov[idx].iov_base + skip);

void *
crypto_contiguous_subsegment(int crpflags, void *crpbuf, size_t skip, size_t len)
        if ((flags & CRYPTO_F_IMBUF) != 0)
                return (m_contiguous_subsegment(crpbuf, off, size, out));
        else if ((flags & CRYPTO_F_IOV) != 0)
                return (cuio_contiguous_subsegment(crpbuf, off, size, out));
        else {
                MPASS((crp_flags & (CRYPTO_F_IMBUF | CRYPTO_F_IOV)) !=
                    (CRYPTO_F_IMBUF | CRYPTO_F_IOV));
                return ((char *)crpbuf + skip);

(Either way, the poorly named cuio_getptr already does most of this logic for you.)

442–465 ↗(On Diff #51893)

This would be reduced to:

addr = crypto_contiguous_subsegment(crp->crp_flags, crp->crp_buf, enccrd->crd_skip, enccrd->crd_len);
if (addr != NULL) {
    *allocated = false;
    return (addr);

// was: alloc:
457 ↗(On Diff #51893)

false for bool false values

sef marked 2 inline comments as done.Dec 12 2018, 7:22 PM
sef added inline comments.
409–432 ↗(On Diff #51893)

I'm *perfectly* happy to have another version of it done. The existing one is a bit of a hack, and I just extended the hack for a case I cared about. I tried to touch the greater crypto code as minimally as possible, which means that I didn't refactor anything.

457 ↗(On Diff #51893)

Easily enough done.

Re-implemented per cem's feedback. Tested with cryptocheck.

This revision is now accepted and ready to land.Dec 13 2018, 3:17 AM
This revision was automatically updated to reflect the committed changes.