> >> +
> >> +       /** Support of pipelined classification
> (ODP_IPSEC_PIPELINE_CLS) of
> >> +        *  resulting inbound packets.
> >
> > Since the operating mode here is INLINE, why not simply
> ODP_IPSEC_INLINE_CLS?


Classifier can be used also in async mode, which would make term "inline" 
overloaded. Term inline is used to make difference between IPSEC inline and 
async modes. Pipeline is used for defining further processing for successfully 
ipsec transformed packets. Exception (not transformed) packets do not go into 
the pipeline.


> >
> >> +        *
> >> +        *  0: Classification of resulting packets is not supported
> >> +        *  1: Classification of resulting packets is supported
> >> +        *  2: Classification of resulting packets is supported and
> preferred
> >> +        */
> >> +       uint8_t pipeline_cls;
> >
> > inline_cls seems clearer here since this section is configuring inline
> > processing options.


Pipeline config is for inline and async modes. It's documented under SA param.


> >>  /**
> >> + * Result event pipeline configuration
> >> + */
> >> +typedef enum odp_ipsec_pipeline_t {
> >> +       /** Do not pipeline */
> >> +       ODP_IPSEC_PIPELINE_NONE = 0,
> >> +
> >> +       /** Send IPSEC result events to the classifier.
> >> +        *
> >> +        *  IPSEC capability 'pipeline_cls' determines if pipelined
> >> +        *  classification is supported. */
> >> +       ODP_IPSEC_PIPELINE_CLS
> >> +
> >> +} odp_ipsec_pipeline_t;
> >
> > I'm not sure that calling this odp_ipsec_pipeline_t is better than the
> > previous odp_ipsec_dest_mode_t. But since this is all about
> > configuring INLINE processing, how about odp_ipsec_inline_config_t and
> > the values are ODP_IPSEC_INLINE_DONE and ODP_IPSEC_INLINE_CLS. DONE
> > simply indicates that inline processing is finished and the result is
> > delivered back to the application.
> >
> > Since this is a per-SA configuration, the question is whether it
> > should be one setting or separate in and out configs since we want a
> > structure that will accommodate an ODP_IPSEC_INLINE_TM for sending
> > IPsec output directly to TM. Whether we want to introduce that now, we
> > want a structure that will accommodate this extension without needing
> > redesign in the next iteration.

It's also for async mode.

Pipeline enum may grow in the future (e.g. with TM). I don't see a problem that 
some pipeline commands are for inbound and some for outbound SAs, a single SA 
cannot be both in and out. So, CLS would a valid option for inbound and TM for 
outbound.


> >> +       /** Select pipelined destination for IPSEC result events
> >> +        *
> >> +        *  Asynchronous and inline modes generate result events.
> Select where
> >> +        *  those events are sent. Inbound SAs may choose to use
> pipelined
> >> +        *  classification. The default value is
> ODP_IPSEC_PIPELINE_NONE.
> >> +        */
> >> +       odp_ipsec_pipeline_t pipeline;
> >> +
> >>         /** Destination queue for IPSEC events
> >>          *
> >> -        *  Operations in asynchronous mode enqueue resulting events
> into
> >> -        *  this queue.
> >> +        *  Operations in asynchronous or inline mode enqueue resulting
> events
> >> +        *  into this queue.
> >>          */
> >>         odp_queue_t dest_queue;
> >
> > It seems confusing to have both pipeline and queue here since both are
> > referring to what should happen next after IPsec processing is
> > finished. The cases are:
> >
> > Input processing: Following IPsec processing the packet should either
> > be sent to the classifier or delivered to a destination event queue.

Exception (not decrypted) packets go to the SA queue in both cases (with or 
without CLS).

> >
> > Output processing: Following IPsec processing the packet should either
> > be sent back to the application via a destination event queue, or
> > forwarded to a TM queue or directly to a pktio for transmission.

Again failures will go to SA queue in all cases.


> >
> > Having two separate controls means that there are invalid combinations
> > that have to be dealt with, which seems messy.

Pipeline controls the next (additional) step for successfully IPsec'ed packets, 
others go SA 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;
> >
> > Having multiple pieces of information needed for the same config
> > suggests opportunities for simplification. Perhaps a scheme along the
> > following lines:
> >
> enum odp_ipsec_next_t {
>        ODP_IPSEC_NEXT_QUEUE = 0,
>        ODP_IPSEC_NEXT_COS,
>        ODP_IPSEC_NEXT_PKTIO,
>        ODP_IPSEC_NEXT_TM,
> } odp_ipsec_next_t;
> 
> > typedef struct odp_ipsec_next_config_t {
>         odp_ipsec_next_t next;
>         union {
>             odp_queue_t queue;
>             odp_cos_t cos;
>             odp_pktio_t pktio;
>             odp_tm_queue_t tm_q;
>         };


Queue is needed always (in inline and async). Pktio and tm queue handles are 
per packet, not per SA. Outer header is per packet and depends on output 
interface. Output interface may change rapidly due to a route change.

-Petri




Reply via email to