This ‘status_queue’ looks fine to me. /Bogdan
On 24 May 2017 at 02:20, Bill Fischofer <bill.fischo...@linaro.org> wrote: > This is a quick summary of the expiration discussion held during > Monday's ARCH call. > > The current odp_ipsec_capability_t defines two fields: > > /** Soft expiry limit in seconds support > * > * 0: Limit is not supported > * 1: Limit is supported > */ > uint8_t soft_limit_sec; > > /** Hard expiry limit in seconds support > * > * 0: Limit is not supported > * 1: Limit is supported > */ > uint8_t hard_limit_sec; > > Presumably the uint8_t's here should be changed to odp_support_t's, or > removed entirely. On the odp_ipsec_sa_create() call passes an > odp_ipsec_param_t struct and one of the sub-structures in that is an > odp_ipsec_lifetime_t struct that specifies hard and soft expiry limits > for bytes, packets, and or time (in seconds). These lifetimes are > defined by RFC 4301 which states (Section 4.4.2.1): > > The following data items MUST be in the SAD: > ... > > - Lifetime of this SA: a time interval after which an SA must be > replaced with a new SA (and new SPI) or terminated, plus an > indication of which of these actions should occur. This may be > expressed as a time or byte count, or a simultaneous use of both > with the first lifetime to expire taking precedence. A compliant > implementation MUST support both types of lifetimes, and MUST > support a simultaneous use of both. > ... > > Note: The details of how to handle the refreshing > of keys when SAs expire is a local matter. However, one > reasonable approach is: > > (a) If byte count is used, then the implementation SHOULD count the > number of bytes to which the IPsec cryptographic algorithm is > applied. For ESP, this is the encryption algorithm (including > Null encryption) and for AH, this is the authentication > algorithm. This includes pad bytes, etc. Note that > implementations MUST be able to handle having the counters at > the ends of an SA get out of synch, e.g., because of packet > loss or because the implementations at each end of the SA > aren't doing things the same way. > > (b) There SHOULD be two kinds of lifetime -- a soft lifetime that > warns the implementation to initiate action such as setting up > a replacement SA, and a hard lifetime when the current SA ends > and is destroyed. > > (c) If the entire packet does not get delivered during the SA's > lifetime, the packet SHOULD be discarded. > > With this background, it's clear that should we choose to remove > time-based expiration from the ODP IPsec API spec we would have an > incomplete IPsec specification that would require any ODP > implementation that wishes to be IPsec compliant to "fill in" the spec > on its own. That would not be good. Note also that the RFC says > nothing about counting packets, only bytes, so perhaps we shouldn't be > offering packet counts? > > Given that we want to keep these, the question is then how are they to > be handled? The argument I would make is that these are control > signals as they indicate that the control plane should begin rekeying > on an SA (soft expiration) or that an SA's lifetime has ended (hard > expiration). As such they should be odp_ipsec_status_t events, which > seems to be what IPsec users like OFP would prefer. > > Currently the odp_ipsec_param_t includes the fields: > > /** Destination queue for IPSEC events > * > * Operations in asynchronous or inline mode enqueue resulting events > * into this queue. > */ > odp_queue_t dest_queue; > > /** Classifier destination CoS for IPSEC result events > * > * Result events for successfully decapsulated packets are sent to > * classification through this CoS. Other result events are sent to > * 'dest_queue'. This field is considered only when 'pipeline' is > * ODP_IPSEC_PIPELINE_CLS. The CoS must not be shared between any pktio > * interface default CoS. > */ > odp_cos_t dest_cos; > > But perhaps this is too coarse. Having a dest_cos makes sense for > PIPELINE_CLS output but the current dest_queue is used for both > odp_ipsec_result_t and odp_ipsec_status_t events, which inherently > have different "consumers". So might it make more sense to split this > into: > > /** > * Output queue for IPSEC results > * > * Operations in asynchronous or inline mode enqueue result events are placed > * onto this queue. > */ > odp_queue_t result_queue; > > /** > * Output queue for IPSEC status > * > * Status events relating to this SA are placed onto this queue. > */ > odp_queue_t status_queue; > > This would also solve the "sync mode" problem that Dmitry mentioned. > An application that only intends to use IPsec lookaside operations and > then only in sync mode may set result_queue to ODP_QUEUE_INVALID. > However, if it wishes to specify a lifetime associated with this SA > then it MUST also specify a status_queue. Failure to do so would fail > the odp_ipsec_sa_create() call. > > With these in place IPsec operation seems well-defined and > RFC-compliant. Results are returned synchronously for sync operations > or asynchronously via the result_queue for async operations. Inline > operations go to either the result_queue or dest_cos, depending on > whether PIPELINE_CLS is in place. And any status events associated > with SA state changes go to the status_queue.