NO HTML mails to the ODP list please. Indentation is lost due to HTML.

 /**
+ * Protocol layers in IPSEC configuration
+ */
+typedef enum odp_ipsec_proto_layer_t {
+       /** No layers */
+       ODP_IPSEC_LAYER_NONE = 0,
+
+       /** Layer L2 protocols (Ethernet, VLAN, etc) */
+       ODP_IPSEC_LAYER_L2,
+
+       /** Layer L3 protocols (IPv4, IPv6, ICMP, IPSec, etc) */

Nit: Correct spelling is IPsec, not IPSec, per RFC 4301.

+       /** Parse packet headers in IPSEC payload
+        *
+        *  Select header parsing level after inbound processing. Packet headers
+        *  in IPSEC payload must be parsed (at least) up to this level.
+        *  Default value is ODP_IPSEC_LAYER_NONE.
+        *
+        *  Note that IPSec payload is never a L2 packet (ODP_IPSEC_LAYER_L2

IPsec


OK. I'll correct it to IPSEC, which is used throughout this file.

" All other capitalizations of IPsec (e.g., IPSEC, IPSec, ipsec) are 
deprecated. However, any capitalization of the  sequence of letters "IPsec" 
should be understood to refer to the IPsec protocols. "

 
+/**
  * IPSEC Security Association (SA) parameters
  */
 typedef struct odp_ipsec_sa_param_t {
@@ -426,6 +610,17 @@ typedef struct odp_ipsec_sa_param_t {
        /** SPI value */
        uint32_t spi;

+       /** Additional inbound SA lookup parameters. Values are considered
+        *  only in ODP_IPSEC_LOOKUP_IN_DSTADDR_UNIQUE_SPI lookup mode. */
+       struct {
+               /* v4 or v6 */
+               uint8_t ip_version;

Is this an enum? If not perhaps doc this as 4 = IPv4, 6 = IPv6.

OK. I'll document it.

 

 /**
+ * Outbound inline IPSEC operation parameters
+ */
+typedef struct odp_ipsec_inline_op_param_t {
+       /** Packet output interface for inline output operation
+        *
+        *  Outbound inline IPSEC operation uses this packet IO interface to
+        *  output the packet after a successful IPSEC transformation. The pktio
+        *  must have been configured to operate in inline IPSEC mode.
+        */
+       odp_pktio_t pktio;

Shouldn't we have an option for this to be output to an odp_tm_queue_t instead 
of a pktio? 
Also, for output are there any controls over which odp_pktout_queue_t is used, 
or is this undefined?


TM is not supported at this stage. Destination TM queue will be added here when 
TM support is added.

Pktout queues are for application use, there's no benefit from those to an 
accelerator (IPSEC may use whatever  implementation specific way to deliver 
packet to pktio).

 
+
+       /** Outer headers for inline output operation
+        *
+        *  Outbound inline IPSEC operation uses this information to prepend
+        *  outer headers to the IPSEC packet before sending it out.
+        */
+       struct {
+               /** Points to first byte of outer headers to be copied in
+                *  front of the outgoing IPSEC packet. Implementation copies
+                *  the headers during odp_ipsec_out_inline() call. */
+               uint8_t *ptr;

Should this be an odp_packet_t rather than a raw set of bytes? The rationale 
for making this an odp_packet_t is that this could then be a packet reference 
which can be used for (re)transmit tracking. For input having this be raw bytes 
makes sense since the ODP implementation is controlling their allocation and 
placement, however for output how is the application going to allocate these? 
Presumably it would create a header via odp_packet_alloc() and then use 
odp_packet_data() to get this address, but that seems cumbersome compared to 
simply passing the odp_packet_t directly. The alternative would seem to require 
that the application use some sort of odp_shm_reserve() call, but that gets 
awkward and is far less efficient than packet allocation, which is a fastpath 
operation.
 

Packet is not needed for l2 hdr storage. Application will store headers to e.g. 
SA context, etc convenient memory location and pass only a pointer to there. 
Implementation copies those (few) bytes during odp_ipsec_out_inline() call. 
Since headers are copied, there's no special requirement for the memory - also 
stack can be used. Most likely application would not store the headers into a 
packet - since there's no benefit on doing so. The payload packet would be more 
likely target for reference counting (than l2 hdr bytes).


 /**
@@ -773,18 +1045,14 @@ typedef struct odp_ipsec_op_result_t {
         *  at least 'num_pkt' elements.
         *
         *  Each successfully transformed packet has a valid value for these
-        *  meta-data:
+        *  meta-data regardless of the inner packet parse configuration.

Vestigial typo. It's metadata, not meta-data. Might as well clean up here.

OK.

+
+       /** Outbound IPSEC inlined with packet output
+        *
+        *  Enable/disable inline outbound IPSEC operation. When enabled IPSEC
+        *  outbound processing can send outgoing IPSEC packets directly
+        *  to the pktio interface for output. IPSEC configuration is done
+        *  through the IPSEC API.
+        *
+        *  Outbound IPSEC inline operation cannot be combined with traffic
+        *  manager (ODP_PKTOUT_MODE_TM).

We need to fix this since TM is likely to be the preferred TX mode, just as the 
classifier is the preferred RX mode. There's no reason why IPsec traffic 
shouldn't also be subject to TM policies. 


Me, Bala and Janne think that it's better to leave TM integration as a next 
step. Classifier integration is much more important, since it enables load 
balancing even for a single, fat IPSEC tunnel. TM is an additional feature that 
may, or may not be used (IPSEC does not need it). Application may still shape 
traffic in SW before calling IPSEC output.


-Petri

Reply via email to