| Message ID | 1498114547-3372-2-git-send-email-bogdan.pricope@linaro.org |
|---|---|
| State | New |
| Headers | show |
Ping! On 22 June 2017 at 09:55, Bogdan Pricope <bogdan.pricope@linaro.org> wrote: > Signed-off-by: Bogdan Pricope <bogdan.pricope@linaro.org> > --- > include/odp/api/spec/ipsec.h | 114 ++++++++++++++++++++++++------------------- > 1 file changed, 63 insertions(+), 51 deletions(-) > > diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h > index e602e4b..5373ede 100644 > --- a/include/odp/api/spec/ipsec.h > +++ b/include/odp/api/spec/ipsec.h > @@ -604,8 +604,8 @@ typedef enum odp_ipsec_ip_version_t { > * IPSEC Security Association (SA) parameters > */ > typedef struct odp_ipsec_sa_param_t { > - /** IPSEC SA direction: inbound or outbound */ > - odp_ipsec_dir_t dir; > + /** SPI value */ > + uint32_t spi; > > /** IPSEC protocol: ESP or AH */ > odp_ipsec_protocol_t proto; > @@ -616,51 +616,12 @@ typedef struct odp_ipsec_sa_param_t { > /** Parameters for crypto and authentication algorithms */ > odp_ipsec_crypto_param_t crypto; > > - /** Parameters for tunnel mode */ > - odp_ipsec_tunnel_param_t tunnel; > - > - /** Fragmentation mode */ > - odp_ipsec_frag_mode_t frag_mode; > - > - /** Various SA option flags */ > - odp_ipsec_sa_opt_t opt; > - > /** SA lifetime parameters */ > odp_ipsec_lifetime_t lifetime; > > - /** SA lookup mode */ > - odp_ipsec_lookup_mode_t lookup_mode; > - > - /** Minimum anti-replay window size. Use 0 to disable anti-replay > - * service. */ > - uint32_t antireplay_ws; > - > /** Initial sequence number */ > uint64_t seq; > > - /** SPI value */ > - uint32_t spi; > - > - /** Additional inbound SA lookup parameters. Values are considered > - * only in ODP_IPSEC_LOOKUP_DSTADDR_SPI lookup mode. */ > - struct { > - /** Select IP version > - */ > - odp_ipsec_ip_version_t ip_version; > - > - /** IP destination address (NETWORK ENDIAN) */ > - void *dst_addr; > - > - } lookup_param; > - > - /** MTU for outbound IP fragmentation offload > - * > - * This is the maximum length of IP packets that outbound IPSEC > - * operations may produce. The value may be updated later with > - * odp_ipsec_mtu_update(). > - */ > - uint32_t mtu; > - > /** Select pipelined destination for resulting events > * > * Asynchronous and inline modes generate events. Select where > @@ -677,16 +638,67 @@ typedef struct odp_ipsec_sa_param_t { > */ > odp_queue_t dest_queue; > > - /** Classifier destination CoS for resulting packets > - * > - * Successfully decapsulated packets are sent to classification > - * through this CoS. Other resulting 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. The maximum number of different CoS supported > - * is defined by IPSEC capability max_cls_cos. > - */ > - odp_cos_t dest_cos; > + /** IPSEC SA direction: inbound or outbound */ > + odp_ipsec_dir_t dir; > + > + /** IPSEC SA direction dependent parameters */ > + union { > + /** Inbound specific parameters */ > + struct { > + /** SA lookup mode */ > + odp_ipsec_lookup_mode_t lookup_mode; > + > + /** Additional inbound SA lookup parameters. Values are > + * considered only in ODP_IPSEC_LOOKUP_DSTADDR_SPI > + * lookup mode. */ > + struct { > + /** Select IP version > + */ > + odp_ipsec_ip_version_t ip_version; > + > + /** IP destination address (NETWORK ENDIAN) */ > + void *dst_addr; > + > + } lookup_param; > + > + /** Minimum anti-replay window size. Use 0 to disable > + * anti-replay service. */ > + uint32_t antireplay_ws; > + > + /** Classifier destination CoS for resulting packets > + * > + * Successfully decapsulated packets are sent to > + * classification through this CoS. Other resulting > + * 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. The maximum > + * number of different CoS supported is defined by > + * IPSEC capability max_cls_cos. > + */ > + odp_cos_t dest_cos; > + } inbound; > + > + /** Outbound specific parameters */ > + struct { > + /** Parameters for tunnel mode */ > + odp_ipsec_tunnel_param_t tunnel; > + > + /** MTU for outbound IP fragmentation offload > + * > + * This is the maximum length of IP packets that > + * outbound IPSEC operations may produce. The value may > + * be updated later with odp_ipsec_mtu_update(). > + */ > + uint32_t mtu; > + > + /** Fragmentation mode */ > + odp_ipsec_frag_mode_t frag_mode; > + } outbound; > + }; > + > + /** Various SA option flags */ > + odp_ipsec_sa_opt_t opt; > > /** User defined SA context pointer > * > -- > 1.9.1 >
> -----Original Message----- > From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of > Bogdan Pricope > Sent: Thursday, June 22, 2017 9:56 AM > To: lng-odp@lists.linaro.org > Subject: [lng-odp] [API-NEXTv2] api: ipsec: reorganize > odp_ipsec_sa_param_t structure based on SA direction > > Signed-off-by: Bogdan Pricope <bogdan.pricope@linaro.org> > --- > include/odp/api/spec/ipsec.h | 114 ++++++++++++++++++++++++-------------- > ----- > 1 file changed, 63 insertions(+), 51 deletions(-) > > diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h > index e602e4b..5373ede 100644 > --- a/include/odp/api/spec/ipsec.h > +++ b/include/odp/api/spec/ipsec.h > @@ -604,8 +604,8 @@ typedef enum odp_ipsec_ip_version_t { > * IPSEC Security Association (SA) parameters > */ > typedef struct odp_ipsec_sa_param_t { > - /** IPSEC SA direction: inbound or outbound */ > - odp_ipsec_dir_t dir; Direction is important for selecting parameters. It should remain the first field of the struct. > + /** SPI value */ > + uint32_t spi; SPI is simple value and should remain after more complex configuration options, which define how the SA works. > > /** IPSEC protocol: ESP or AH */ > odp_ipsec_protocol_t proto; > @@ -616,51 +616,12 @@ typedef struct odp_ipsec_sa_param_t { > /** Parameters for crypto and authentication algorithms */ > odp_ipsec_crypto_param_t crypto; > > - /** Parameters for tunnel mode */ > - odp_ipsec_tunnel_param_t tunnel; > - > - /** Fragmentation mode */ > - odp_ipsec_frag_mode_t frag_mode; > - > - /** Various SA option flags */ > - odp_ipsec_sa_opt_t opt; > - Keep opt here before the union. The inbound/outbound union should be the last thing in the struct. Everything before it is common for both directions. > /** SA lifetime parameters */ > odp_ipsec_lifetime_t lifetime; > > - /** SA lookup mode */ > - odp_ipsec_lookup_mode_t lookup_mode; > - > - /** Minimum anti-replay window size. Use 0 to disable anti- > replay > - * service. */ > - uint32_t antireplay_ws; > - > /** Initial sequence number */ > uint64_t seq; This may be moved to outbound struct. > > - /** SPI value */ > - uint32_t spi; > - > - /** Additional inbound SA lookup parameters. Values are > considered > - * only in ODP_IPSEC_LOOKUP_DSTADDR_SPI lookup mode. */ > - struct { > - /** Select IP version > - */ > - odp_ipsec_ip_version_t ip_version; > - > - /** IP destination address (NETWORK ENDIAN) */ > - void *dst_addr; > - > - } lookup_param; > - > - /** MTU for outbound IP fragmentation offload > - * > - * This is the maximum length of IP packets that outbound > IPSEC > - * operations may produce. The value may be updated later with > - * odp_ipsec_mtu_update(). > - */ > - uint32_t mtu; > - > /** Select pipelined destination for resulting events > * > * Asynchronous and inline modes generate events. Select where > @@ -677,16 +638,67 @@ typedef struct odp_ipsec_sa_param_t { > */ > odp_queue_t dest_queue; > > - /** Classifier destination CoS for resulting packets > - * > - * Successfully decapsulated packets are sent to > classification > - * through this CoS. Other resulting 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. The maximum number of different CoS > supported > - * is defined by IPSEC capability max_cls_cos. > - */ > - odp_cos_t dest_cos; > + /** IPSEC SA direction: inbound or outbound */ > + odp_ipsec_dir_t dir; > + > + /** IPSEC SA direction dependent parameters */ > + union { > + /** Inbound specific parameters */ > + struct { > + /** SA lookup mode */ > + odp_ipsec_lookup_mode_t lookup_mode; > + > + /** Additional inbound SA lookup > parameters. Values are > + * considered only in > ODP_IPSEC_LOOKUP_DSTADDR_SPI > + * lookup mode. */ > + struct { > + /** Select IP version > + */ > + odp_ipsec_ip_version_t > ip_version; > + > + /** IP destination address > (NETWORK ENDIAN) */ > + void *dst_addr; > + > + } lookup_param; > + > + /** Minimum anti-replay window size. Use 0 > to disable > + * anti-replay service. */ > + uint32_t antireplay_ws; > + > + /** Classifier destination CoS for > resulting packets > + * > + * Successfully decapsulated packets are > sent to > + * classification through this CoS. Other > resulting > + * 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. The maximum > + * number of different CoS supported is > defined by > + * IPSEC capability max_cls_cos. > + */ > + odp_cos_t dest_cos; > + } inbound; > + > + /** Outbound specific parameters */ > + struct { > + /** Parameters for tunnel mode */ > + odp_ipsec_tunnel_param_t tunnel; > + > + /** MTU for outbound IP fragmentation > offload > + * > + * This is the maximum length of IP > packets that > + * outbound IPSEC operations may produce. > The value may > + * be updated later with > odp_ipsec_mtu_update(). > + */ > + uint32_t mtu; > + > + /** Fragmentation mode */ > + odp_ipsec_frag_mode_t frag_mode; > + } outbound; > + }; The union should be the last field on the SA struct. -Petri > + > + /** Various SA option flags */ > + odp_ipsec_sa_opt_t opt; > > /** User defined SA context pointer > * > -- > 1.9.1
Hi Petri, See my comments, inline. In my patch, member fields are arranged by importance (in my view) and highlighting direction dependency. Yet, "importance" is a subjective term and is clear that you have a different view on it. Do arrange them as you consider appropriate... if direction is highlighted (through union/struct or other mechanism) I am fine with it. Best regards, Bogdan On 29 June 2017 at 16:04, Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com> wrote: > >> -----Original Message----- >> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of >> Bogdan Pricope >> Sent: Thursday, June 22, 2017 9:56 AM >> To: lng-odp@lists.linaro.org >> Subject: [lng-odp] [API-NEXTv2] api: ipsec: reorganize >> odp_ipsec_sa_param_t structure based on SA direction >> >> Signed-off-by: Bogdan Pricope <bogdan.pricope@linaro.org> >> --- >> include/odp/api/spec/ipsec.h | 114 ++++++++++++++++++++++++-------------- >> ----- >> 1 file changed, 63 insertions(+), 51 deletions(-) >> >> diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h >> index e602e4b..5373ede 100644 >> --- a/include/odp/api/spec/ipsec.h >> +++ b/include/odp/api/spec/ipsec.h >> @@ -604,8 +604,8 @@ typedef enum odp_ipsec_ip_version_t { >> * IPSEC Security Association (SA) parameters >> */ >> typedef struct odp_ipsec_sa_param_t { >> - /** IPSEC SA direction: inbound or outbound */ >> - odp_ipsec_dir_t dir; > > Direction is important for selecting parameters. It should remain the first field of the struct. Is important, but should be close to "direction dependent parameters" to make sense. > > >> + /** SPI value */ >> + uint32_t spi; > > SPI is simple value and should remain after more complex configuration options, which define how the SA works. RFC 4301 - Security Architecture for IP "For an SA used to carry unicast traffic, the Security Parameters Index (SPI) by itself suffices to specify an SA. (For information on the SPI, see Appendix A and the AH and ESP specifications [Ken05b, Ken05a].) However, as a local matter, an implementation may choose to use the SPI in conjunction with the IPsec protocol type (AH or ESP) for SA identification." SPI (or SPI + protocol) is the single most important information to identify a SA. Should be first. > > >> >> /** IPSEC protocol: ESP or AH */ >> odp_ipsec_protocol_t proto; >> @@ -616,51 +616,12 @@ typedef struct odp_ipsec_sa_param_t { >> /** Parameters for crypto and authentication algorithms */ >> odp_ipsec_crypto_param_t crypto; >> >> - /** Parameters for tunnel mode */ >> - odp_ipsec_tunnel_param_t tunnel; >> - >> - /** Fragmentation mode */ >> - odp_ipsec_frag_mode_t frag_mode; >> - >> - /** Various SA option flags */ >> - odp_ipsec_sa_opt_t opt; >> - > > Keep opt here before the union. The inbound/outbound union should be the last thing in the struct. Everything before it is common for both directions. "Various SA option flags" ... does not seems to be that important to put it upper in the structure. > > >> /** SA lifetime parameters */ >> odp_ipsec_lifetime_t lifetime; >> >> - /** SA lookup mode */ >> - odp_ipsec_lookup_mode_t lookup_mode; >> - >> - /** Minimum anti-replay window size. Use 0 to disable anti- >> replay >> - * service. */ >> - uint32_t antireplay_ws; >> - >> /** Initial sequence number */ >> uint64_t seq; > > This may be moved to outbound struct. I thought the same.. yet, some say is needed on inbound as well ("to know from where to start expecting frames") a.k.a. antireplay mechanism. > > >> >> - /** SPI value */ >> - uint32_t spi; >> - >> - /** Additional inbound SA lookup parameters. Values are >> considered >> - * only in ODP_IPSEC_LOOKUP_DSTADDR_SPI lookup mode. */ >> - struct { >> - /** Select IP version >> - */ >> - odp_ipsec_ip_version_t ip_version; >> - >> - /** IP destination address (NETWORK ENDIAN) */ >> - void *dst_addr; >> - >> - } lookup_param; >> - >> - /** MTU for outbound IP fragmentation offload >> - * >> - * This is the maximum length of IP packets that outbound >> IPSEC >> - * operations may produce. The value may be updated later with >> - * odp_ipsec_mtu_update(). >> - */ >> - uint32_t mtu; >> - >> /** Select pipelined destination for resulting events >> * >> * Asynchronous and inline modes generate events. Select where >> @@ -677,16 +638,67 @@ typedef struct odp_ipsec_sa_param_t { >> */ >> odp_queue_t dest_queue; >> >> - /** Classifier destination CoS for resulting packets >> - * >> - * Successfully decapsulated packets are sent to >> classification >> - * through this CoS. Other resulting 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. The maximum number of different CoS >> supported >> - * is defined by IPSEC capability max_cls_cos. >> - */ >> - odp_cos_t dest_cos; >> + /** IPSEC SA direction: inbound or outbound */ >> + odp_ipsec_dir_t dir; >> + >> + /** IPSEC SA direction dependent parameters */ >> + union { >> + /** Inbound specific parameters */ >> + struct { >> + /** SA lookup mode */ >> + odp_ipsec_lookup_mode_t lookup_mode; >> + >> + /** Additional inbound SA lookup >> parameters. Values are >> + * considered only in >> ODP_IPSEC_LOOKUP_DSTADDR_SPI >> + * lookup mode. */ >> + struct { >> + /** Select IP version >> + */ >> + odp_ipsec_ip_version_t >> ip_version; >> + >> + /** IP destination address >> (NETWORK ENDIAN) */ >> + void *dst_addr; >> + >> + } lookup_param; >> + >> + /** Minimum anti-replay window size. Use 0 >> to disable >> + * anti-replay service. */ >> + uint32_t antireplay_ws; >> + >> + /** Classifier destination CoS for >> resulting packets >> + * >> + * Successfully decapsulated packets are >> sent to >> + * classification through this CoS. Other >> resulting >> + * 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. The maximum >> + * number of different CoS supported is >> defined by >> + * IPSEC capability max_cls_cos. >> + */ >> + odp_cos_t dest_cos; >> + } inbound; >> + >> + /** Outbound specific parameters */ >> + struct { >> + /** Parameters for tunnel mode */ >> + odp_ipsec_tunnel_param_t tunnel; >> + >> + /** MTU for outbound IP fragmentation >> offload >> + * >> + * This is the maximum length of IP >> packets that >> + * outbound IPSEC operations may produce. >> The value may >> + * be updated later with >> odp_ipsec_mtu_update(). >> + */ >> + uint32_t mtu; >> + >> + /** Fragmentation mode */ >> + odp_ipsec_frag_mode_t frag_mode; >> + } outbound; >> + }; > > > The union should be the last field on the SA struct. Maybe, but I think it should to be here with the important/defining parameters, before "Various SA options" and SA context. > > -Petri > >> + >> + /** Various SA option flags */ >> + odp_ipsec_sa_opt_t opt; >> >> /** User defined SA context pointer >> * >> -- >> 1.9.1 >
> >> /** Initial sequence number */ > >> uint64_t seq; > > > > This may be moved to outbound struct. > > I thought the same.. yet, some say is needed on inbound as well ("to > know from where to start expecting frames") a.k.a. antireplay > mechanism. It is not needed in inbound since the antireplay window will anyway slide to the correct position when the first packet is received. Actually, this field is probably not needed in outbound either since the ESP RFC says (but maybe does not strictly require it?) that the first ESP packet sent on an SA will have sequence number 1. So I suppose the initial sequence number field should just be removed from the param struct. Janne
diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h index e602e4b..5373ede 100644 --- a/include/odp/api/spec/ipsec.h +++ b/include/odp/api/spec/ipsec.h @@ -604,8 +604,8 @@ typedef enum odp_ipsec_ip_version_t { * IPSEC Security Association (SA) parameters */ typedef struct odp_ipsec_sa_param_t { - /** IPSEC SA direction: inbound or outbound */ - odp_ipsec_dir_t dir; + /** SPI value */ + uint32_t spi; /** IPSEC protocol: ESP or AH */ odp_ipsec_protocol_t proto; @@ -616,51 +616,12 @@ typedef struct odp_ipsec_sa_param_t { /** Parameters for crypto and authentication algorithms */ odp_ipsec_crypto_param_t crypto; - /** Parameters for tunnel mode */ - odp_ipsec_tunnel_param_t tunnel; - - /** Fragmentation mode */ - odp_ipsec_frag_mode_t frag_mode; - - /** Various SA option flags */ - odp_ipsec_sa_opt_t opt; - /** SA lifetime parameters */ odp_ipsec_lifetime_t lifetime; - /** SA lookup mode */ - odp_ipsec_lookup_mode_t lookup_mode; - - /** Minimum anti-replay window size. Use 0 to disable anti-replay - * service. */ - uint32_t antireplay_ws; - /** Initial sequence number */ uint64_t seq; - /** SPI value */ - uint32_t spi; - - /** Additional inbound SA lookup parameters. Values are considered - * only in ODP_IPSEC_LOOKUP_DSTADDR_SPI lookup mode. */ - struct { - /** Select IP version - */ - odp_ipsec_ip_version_t ip_version; - - /** IP destination address (NETWORK ENDIAN) */ - void *dst_addr; - - } lookup_param; - - /** MTU for outbound IP fragmentation offload - * - * This is the maximum length of IP packets that outbound IPSEC - * operations may produce. The value may be updated later with - * odp_ipsec_mtu_update(). - */ - uint32_t mtu; - /** Select pipelined destination for resulting events * * Asynchronous and inline modes generate events. Select where @@ -677,16 +638,67 @@ typedef struct odp_ipsec_sa_param_t { */ odp_queue_t dest_queue; - /** Classifier destination CoS for resulting packets - * - * Successfully decapsulated packets are sent to classification - * through this CoS. Other resulting 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. The maximum number of different CoS supported - * is defined by IPSEC capability max_cls_cos. - */ - odp_cos_t dest_cos; + /** IPSEC SA direction: inbound or outbound */ + odp_ipsec_dir_t dir; + + /** IPSEC SA direction dependent parameters */ + union { + /** Inbound specific parameters */ + struct { + /** SA lookup mode */ + odp_ipsec_lookup_mode_t lookup_mode; + + /** Additional inbound SA lookup parameters. Values are + * considered only in ODP_IPSEC_LOOKUP_DSTADDR_SPI + * lookup mode. */ + struct { + /** Select IP version + */ + odp_ipsec_ip_version_t ip_version; + + /** IP destination address (NETWORK ENDIAN) */ + void *dst_addr; + + } lookup_param; + + /** Minimum anti-replay window size. Use 0 to disable + * anti-replay service. */ + uint32_t antireplay_ws; + + /** Classifier destination CoS for resulting packets + * + * Successfully decapsulated packets are sent to + * classification through this CoS. Other resulting + * 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. The maximum + * number of different CoS supported is defined by + * IPSEC capability max_cls_cos. + */ + odp_cos_t dest_cos; + } inbound; + + /** Outbound specific parameters */ + struct { + /** Parameters for tunnel mode */ + odp_ipsec_tunnel_param_t tunnel; + + /** MTU for outbound IP fragmentation offload + * + * This is the maximum length of IP packets that + * outbound IPSEC operations may produce. The value may + * be updated later with odp_ipsec_mtu_update(). + */ + uint32_t mtu; + + /** Fragmentation mode */ + odp_ipsec_frag_mode_t frag_mode; + } outbound; + }; + + /** Various SA option flags */ + odp_ipsec_sa_opt_t opt; /** User defined SA context pointer *
Signed-off-by: Bogdan Pricope <bogdan.pricope@linaro.org> --- include/odp/api/spec/ipsec.h | 114 ++++++++++++++++++++++++------------------- 1 file changed, 63 insertions(+), 51 deletions(-) -- 1.9.1