Hi Pablo, thank you for the comments
> -----Original Message-----
> From: De Lara Guarch, Pablo <pablo.de.lara.gua...@intel.com>
> Sent: Tuesday, June 23, 2020 6:57 PM
>
> > +static inline void
> > +verify_docsis_sec_crc(JOB_AES_HMAC *job, uint16_t crc_len, uint8_t
> > +*status) {
> > + uint16_t crc_offset;
> > + uint8_t *crc;
> > +
> > + if (!job->msg_len_to_hash_in_bytes)
> > + return;
> > +
> > + crc_offset = job->hash_start_src_offset_in_bytes +
> > + job->msg_len_to_hash_in_bytes -
> > + job->cipher_start_src_offset_in_bytes;
> > + crc = job->dst + crc_offset;
> > +
> > + /* Verify CRC (at the end of the message) */
> > + if (memcmp(job->auth_tag_output, crc, crc_len) != 0)
>
> I'd say we can use direct RTE_ETHER_CRC_LEN here, as there is no other
> possible case, right?
> It should perform better.
[DC] You are correct - I have changed this to use RTE_ETHER_CRC_LEN.
I had been thinking about removing the crc_size from the
rte_security_docsis_xform
and the docsis capabilities completely and your comment here has made me realize
I should do this, as there is only 1 CRC length that can be used for DOCSIS.
So these have been removed.
These changes will be in v3 early next week
>
> > + *status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED; } #endif
> > +
> > static inline void
> > verify_digest(JOB_AES_HMAC *job, void *digest, uint16_t len, uint8_t
> > *status) { @@ -1196,9 +1452,27 @@ static inline struct rte_crypto_op
> > * post_process_mb_job(struct aesni_mb_qp *qp, JOB_AES_HMAC *job) {
> > struct rte_crypto_op *op = (struct rte_crypto_op *)job->user_data;
> > - struct aesni_mb_session *sess = get_sym_session_private_data(
> > - op->sym->session,
> > - cryptodev_driver_id);
> > + struct aesni_mb_session *sess = NULL;
> > +
> > +#ifdef AESNI_MB_DOCSIS_SEC_ENABLED
> > + struct rte_security_op *sec_op = NULL;
> > +
> > + if (unlikely(op->type == RTE_CRYPTO_OP_TYPE_SECURITY)) {
>
> Not sure if this unlikely is actually needed. I don't expect to have multiple
> types enqueued in the same queue, so this or the other branch will always
> be taken.
[DC] That's a fair point - I have removed the unlikely
Again, the change will be available in v3
>
> > + /*
> > + * Assuming at this point that if it's a security type op, that
> > + * this is for DOCSIS
> > + */
> > + sec_op = (struct rte_security_op *)&op->security;
> > + struct rte_crypto_sym_op *crypto_sym =
> > + &sec_op-
> >docsis.crypto_sym;
> > + sess = get_sec_session_private_data(crypto_sym-
> > >sec_session);
>
> ...
>
> > - retval = set_mb_job_params(job, qp, op, &digest_idx);
> > +#ifdef AESNI_MB_DOCSIS_SEC_ENABLED
> > + if (unlikely(op->type == RTE_CRYPTO_OP_TYPE_SECURITY))
>
> Same comment as above.
[DC] Same reply as above. :)
>
> > + retval = set_sec_mb_job_params(job, qp, op,
> > + &digest_idx);
> > + else
> > +#endif
