Hi Konstantin,

Subject: [EXT] RE: [PATCH 2/2] lib/security: add SA lifetime configuration

Hi Anoob,

quoted

quoted

quoted

Now that we have an agreement on bitfields (hoping no one else has
an objection), I would like to discuss one more topic. It is more
related to

checksum offload, but it's better that we discuss along with other
similar items (like soft expiry).

quoted

L3 & L4 checksum can be tristate (CSUM_OK, CSUM_ERROR,

CSUM_UNKOWN)

quoted

1. Application didn't request. Nothing computed.
2. Application requested. Checksum verification success.
3. Application requested. Checksum verification failed.
4. Application requested. Checksum could not be computed (PMD

limitations etc).

quoted

How would we indicate each case?

My proposal would be, let's call the field that we called
"warning" as

"aux_flags" (auxiliary or secondary information from the operation).

quoted

Sequence in the application would be,

if (op.status != SUCCESS) {
    /* handle errors */
}

#define RTE_SEC_IPSEC_AUX_FLAGS_L4_CHECKSUM_COMPUTED (1

<< 0)

quoted

quoted

#define

quoted

RTE_SEC_IPSEC_AUX_FLAGS_L4_CHECSUM_GOOD (1 << 1)

if (op.aux_flags &

RTE_SEC_IPSEC_AUX_FLAGS_L4_CHECKSUM_COMPUTED) {

quoted

	if (op.aux_flags &

RTE_SEC_IPSEC_AUX_FLAGS_L4_CHECSUM_GOOD)

quoted

		mbuf->l4_checksum_good = 1;
	else
		mbuf->l4_checksum_good = 0;
} else {
	if (verify_l4_checksum(mbuf) == SUCCESS) {
		mbuf->l4_checksum_good = 1;
	else
		mbuf->l4_checksum_good = 0;
}

For an application not worried about aux_flags (ex: ipsec-secgw),
additional checks are not required. For applications not
interested in checksum, a blind check on op.aux_flags would be enough

to bail out early.

quoted

quoted

For applications interested in checksum, it can follow above
sequence (kinds, for demonstration purpose only).

quoted

Would something like above fine? Or if we want to restrict
additional fields for just warnings, (L4_CHECKSUM_ERROR), how
would application differentiate between checksum good & checksum
not computed? In that

case, what should be PMDs treatment of "could not compute" v/s
"computed and wrong".

I am ok with what you suggest.
My only thought - we already have CSUM flags in mbuf itself, so why
not to use them instead to pass this information from crypto PMD to

user?

quoted

quoted

That way it would be compliant with ethdev CSUM approach and no need
to spend
2 bits in 'aux_flags'.
Konstantin

[Anoob] You are right. We do have CSUM flags in mbuf and that would fully

suite our requirement here.

quoted

Our problem was, it's called PKT_RX_ and the description text refers to RX.

/**
 * Mask of bits used to determine the status of RX IP checksum.
 * - PKT_RX_IP_CKSUM_UNKNOWN: no information about the RX IP

checksum

quoted

 * - PKT_RX_IP_CKSUM_BAD: the IP checksum in the packet is wrong
 * - PKT_RX_IP_CKSUM_GOOD: the IP checksum in the packet is valid
 * - PKT_RX_IP_CKSUM_NONE: the IP checksum is not correct in the packet
 *   data, but the integrity of the IP header is verified.
 */

But if we overlook that (& may be update documentation), it's a rather
great idea. We could use similar PKT_TX_* flags for requesting checksum

generation with outbound operations (checksum generation for plain packet
before IPsec processing).

quoted

/**
 * Offload the IP checksum in the hardware. The flag PKT_TX_IPV4
should
 * also be set by the application, although a PMD will only check
 * PKT_TX_IP_CKSUM.
 *  - fill the mbuf offload information: l2_len, l3_len  */
#define PKT_TX_IP_CKSUM      (1ULL << 54)

/**
 * Packet is IPv4. This flag must be set when using any offload
feature
 * (TSO, L3 or L4 checksum) to tell the NIC that the packet is an IPv4
 * packet. If the packet is a tunneled packet, this flag is related to
 * the inner headers.
 */
#define PKT_TX_IPV4          (1ULL << 55)

Do you think above might require some modifications to document

behavior with lookaside IPsec?

quoted

Also, these flags are probably the best way for checksum for inner
packet with inline IPsec. So this looks like overall better idea. Do you agree?

Not sure I understand your proposal fully.
Yes, right now inside mbuf we have different set of flags for checksum
offloads: RX and TX.
RX flags - indicate was checksum calculated/checked for incoming packet and
what was the result, While TX flags define which CSUM calculations have to
be done by HW.
Yes, I suppose same flags can be reused by crypto-dev, if it capable to
implement these HW offloads.
Though not sure what changes do you think will be required inside mbuf?

[Anoob] My concern was regarding "RX" & "TX" in the comments, which are more applicable for ethdev than cryptodev. But then, I guess it's fine this way itself.

Will submit a new version for all the proposals with some unit tests so that we can discuss if there is any ambiguity in the usage.

Appreciate your feedback and suggestions.

Thanks,
Anoob