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

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

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?

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?

Konstantin