Re: [dpdk-dev] [PATCH 2/2] lib/security: add SA lifetime configuration

[Anoob Joseph <anoobj@marvell.com>]

Hi Konstantin,

> Subject: [EXT] RE: [PATCH 2/2] lib/security: add SA lifetime configuration
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Anoob,
> 
> > 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).
> >
> > L3 & L4 checksum can be tristate (CSUM_OK, CSUM_ERROR,
> CSUM_UNKOWN)
> >
> > 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).
> >
> > 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).
> >
> > Sequence in the application would be,
> >
> > if (op.status != SUCCESS) {
> >     /* handle errors */
> > }
> >
> > #define RTE_SEC_IPSEC_AUX_FLAGS_L4_CHECKSUM_COMPUTED (1 << 0)
> #define
> > RTE_SEC_IPSEC_AUX_FLAGS_L4_CHECSUM_GOOD (1 << 1)
> >
> > if (op.aux_flags &
> RTE_SEC_IPSEC_AUX_FLAGS_L4_CHECKSUM_COMPUTED) {
> > 	if (op.aux_flags &
> RTE_SEC_IPSEC_AUX_FLAGS_L4_CHECSUM_GOOD)
> > 		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.
> For applications interested in checksum, it can follow above sequence (kinds,
> for demonstration purpose only).
> >
> > 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?
> 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. 

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
 * - 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).

/**
 * 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?

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?

Thanks,
Anoob