> -----Original Message-----
> From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
> Sent: Friday, May 26, 2017 5:04 PM
> To: Peltonen, Janne (Nokia - FI/Espoo) <janne.pelto...@nokia.com>
> Cc: lng-odp-forward <lng-odp@lists.linaro.org>
> Subject: Re: [lng-odp] Summary of Expiration Discussion
>
> On Wed, May 24, 2017 at 8:15 AM, Peltonen, Janne (Nokia - FI/Espoo)
> <janne.pelto...@nokia.com> wrote:
> > Bill Fischofer wrote:
> >>
> >> The following data items MUST be in the SAD:
> >
> > But that does not necessarily mean that they need to be in the ODP SA.
> >
> >> 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.
> >
> > Regardless of how complete the life time support is in ODP, there
> > are other things that the application needs to do for IPsec RFC
> > compliancy. For example, ODP does not do any policy enforcement
> > but leaves it to the application (which is good since it adds
> > flexibility).
> >
> >> Note also that the RFC says
> >> nothing about counting packets, only bytes, so perhaps we shouldn't be
> >> offering packet counts?
>
> Packets are certainly a more convenient approximation for data-based
> limits as these are not intended to be precise. I don't believe anyone
> ever tests compliance based on exact byte counts (e.g., the limit is
> 43210495 and you better not allow that 43210496th byte to be
> transmitted). Cryptographic degradation is gradual and statistical,
> not a step function.
>
> >
> > I suppose packet based life times could be added later as a backward
> > compatible API change if they are not included now. But I think some
> > other IPsec implementations provide them.
> >
> >> 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.
> >
> > Even if all life time expirations are signaled as status events,
> > a normal operation completion event for a packet is needed when
> > IPsec processing is attempted but fails due to hard life time
> > (or byte or packet limit) expiration. So those error bits are
> > still needed in the op_result.
>
> From a worker perspective, an SA whose hard limit has been reached is
> no different than one that has been disabled. Effectively, when the
> limit is reached odp_ipsec_sa_disable() is called by the
> implementation. The SA is still visible until it is destroyed, but any
> further attempt to use that SA will fail. Saying that the operation
> failed because the SA is disabled is sufficient.
Given how odp_ipsec_sa_disable() has now been defined, it is not
quite the same as reaching a hard limit. When an application calls
SA disable it promises that it will not initiate any new operations
that explicitly refer to the SA. An application cannot promise
the same (not using the SA in an IPsec operation) after reaching
a hard byte/packet/time limit (unless the application itself keeps
track of the limits and synchronizes the state between threads).
So from application perspective, it would be acceptable to have
IPsec operations on a disabled SA result in undefined behavior (as
currently defined) but not so for an SA whose hard limit has been
reached. But maybe there does not have to be hard limit specific
error bits in the result (or per packet metadata, if we switch to
that) but some generic operation error could be used for this case
too.
Another difference is that when an SA has been disabled, inbound
lookups will not match it anymore and incoming packets end up in
the default queue (in async mode) or in pktin (in inline mode).
Currently, after reaching a hard limit, the SA still matches
and the error result goes to the SA specific queue.
>
> >
> > I am not sure OFP has any preference one way or the other.
> >
> >> 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;
> >
> > There are some status events (at least the disable SA completion
> > event) that need to be ordered with operation completion events.
> > When an application gets the disable completion event, it must
> > be able to count on that it has seen all operation completion
> > events for that SA or else it becomes much more difficult to
> > tear down SAs safely. To get that ordering, disable completion
> > needs to come through the same queue as the normal IPsec
> > operation events.
>
> You already proposed that the implementation simply flush each queue
> associated with the SA with a disabled marker, which seems sufficient
> and extensible to however many queues may be in play. The application
> presumably knows how many queues it has associated with the SA and can
> simply wait until it receives that many marker events before
> proceeding to destroy the SA. The protocol is: Mark each queue to not
> accept further events and add a special marker event to the queue. Any
> further enq attempts after this fail.
Yes, if we define the API so that events for the same SA can come
through multiple queues, I think we need an end marker in all of
the queues.
>
> >
> > This ordering is not currently explicitly specified in the API
> > but we have discussed the need to clarify the API with Petri.
> > I think the ordering becomes even more important if we change
> > operation result events to packet events with IPsec specific
> > packet metadata.
>
> This is a general problem of queues needing a "quiesce" function that
> prohibits further enqueues while still allowing dequeues to permit
> them to be drained gracefully. Perhaps this use case is motivation to
> add that general capability?
>
Perhaps so.
Janne
