| Message ID | 20210408081720.23314-3-ktejasree@marvell.com (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | akhil goyal |
| Headers | show |
| Series | add lookaside IPsec UDP encapsulation and transport mode | expand |
| Context | Check | Description |
|---|---|---|
| ci/checkpatch | success | coding style OK |
Hi Olivier, Andrew Could you please review the patch? Thanks Tejasree > -----Original Message----- > From: Tejasree Kondoj <ktejasree@marvell.com> > Sent: Thursday, April 8, 2021 1:47 PM > To: Akhil Goyal <gakhil@marvell.com>; Radu Nicolau > <radu.nicolau@intel.com>; Konstantin Ananyev > <konstantin.ananyev@intel.com> > Cc: Tejasree Kondoj <ktejasree@marvell.com>; Anoob Joseph > <anoobj@marvell.com>; Ankur Dwivedi <adwivedi@marvell.com>; Jerin > Jacob Kollanukkaran <jerinj@marvell.com>; dev@dpdk.org > Subject: [PATCH v3 2/4] mbuf: add packet type for UDP-ESP tunnel packets > > Adding new mbuf packet type for UDP encapsulated ESP packets. > > Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com> > --- > doc/guides/rel_notes/release_21_05.rst | 5 +++++ > lib/librte_mbuf/rte_mbuf_ptype.h | 21 +++++++++++++++++++++ > 2 files changed, 26 insertions(+) > > diff --git a/doc/guides/rel_notes/release_21_05.rst > b/doc/guides/rel_notes/release_21_05.rst > index 5565c7637c..c9e9e2ec22 100644 > --- a/doc/guides/rel_notes/release_21_05.rst > +++ b/doc/guides/rel_notes/release_21_05.rst > @@ -55,6 +55,11 @@ New Features > Also, make sure to start the actual text at the margin. > ======================================================= > > +* **Added new packet type for UDP-ESP packets in mbuf.** > + > + Added new packet type ``RTE_PTYPE_TUNNEL_ESP_IN_UDP`` which can be > + used to identify UDP encapsulated ESP packets. > + > * **Enhanced ethdev representor syntax.** > > * Introduced representor type of VF, SF and PF. > diff --git a/lib/librte_mbuf/rte_mbuf_ptype.h > b/lib/librte_mbuf/rte_mbuf_ptype.h > index 17a2dd3576..bf92ce0c1a 100644 > --- a/lib/librte_mbuf/rte_mbuf_ptype.h > +++ b/lib/librte_mbuf/rte_mbuf_ptype.h > @@ -491,6 +491,27 @@ extern "C" { > * | 'destination port'=6635> > */ > #define RTE_PTYPE_TUNNEL_MPLS_IN_UDP 0x0000d000 > +/** > + * ESP-in-UDP tunneling packet type (RFC 3948). > + * > + * Packet format: > + * <'ether type'=0x0800 > + * | 'version'=4, 'protocol'=17 > + * | 'destination port'=4500> > + * or, > + * <'ether type'=0x86DD > + * | 'version'=6, 'next header'=17 > + * | 'destination port'=4500> > + * or, > + * <'ether type'=0x0800 > + * | 'version'=4, 'protocol'=17 > + * | 'source port'=4500> > + * or, > + * <'ether type'=0x86DD > + * | 'version'=6, 'next header'=17 > + * | 'source port'=4500> > + */ > +#define RTE_PTYPE_TUNNEL_ESP_IN_UDP 0x0000e000 > /** > * Mask of tunneling packet types. > */ > -- > 2.27.0
On Thu, Apr 08, 2021 at 01:47:18PM +0530, Tejasree Kondoj wrote: > Adding new mbuf packet type for UDP encapsulated > ESP packets. > > Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com> > --- > doc/guides/rel_notes/release_21_05.rst | 5 +++++ > lib/librte_mbuf/rte_mbuf_ptype.h | 21 +++++++++++++++++++++ > 2 files changed, 26 insertions(+) > > diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst > index 5565c7637c..c9e9e2ec22 100644 > --- a/doc/guides/rel_notes/release_21_05.rst > +++ b/doc/guides/rel_notes/release_21_05.rst > @@ -55,6 +55,11 @@ New Features > Also, make sure to start the actual text at the margin. > ======================================================= > > +* **Added new packet type for UDP-ESP packets in mbuf.** > + > + Added new packet type ``RTE_PTYPE_TUNNEL_ESP_IN_UDP`` which can be > + used to identify UDP encapsulated ESP packets. > + > * **Enhanced ethdev representor syntax.** > > * Introduced representor type of VF, SF and PF. > diff --git a/lib/librte_mbuf/rte_mbuf_ptype.h b/lib/librte_mbuf/rte_mbuf_ptype.h > index 17a2dd3576..bf92ce0c1a 100644 > --- a/lib/librte_mbuf/rte_mbuf_ptype.h > +++ b/lib/librte_mbuf/rte_mbuf_ptype.h > @@ -491,6 +491,27 @@ extern "C" { > * | 'destination port'=6635> > */ > #define RTE_PTYPE_TUNNEL_MPLS_IN_UDP 0x0000d000 > +/** > + * ESP-in-UDP tunneling packet type (RFC 3948). > + * > + * Packet format: > + * <'ether type'=0x0800 > + * | 'version'=4, 'protocol'=17 > + * | 'destination port'=4500> > + * or, > + * <'ether type'=0x86DD > + * | 'version'=6, 'next header'=17 > + * | 'destination port'=4500> > + * or, > + * <'ether type'=0x0800 > + * | 'version'=4, 'protocol'=17 > + * | 'source port'=4500> > + * or, > + * <'ether type'=0x86DD > + * | 'version'=6, 'next header'=17 > + * | 'source port'=4500> > + */ > +#define RTE_PTYPE_TUNNEL_ESP_IN_UDP 0x0000e000 > /** > * Mask of tunneling packet types. > */ We arrive at the end of the values in packet type tunnel types, and there is another pending patch that needs another tunnel type. As there is already a RTE_PTYPE_TUNNEL_ESP, what would you think about trying to reuse it, and differentiate IP/ESP from IP/UDP/ESP by using the L4 layer type (unknown vs udp)? Or maybe add RTE_PTYPE_L4_NONE. It is sensible, because it can be considered as an API change for current users of RTE_PTYPE_TUNNEL_ESP. I don't really know how this type is used by applications. I think it is time to start thinking about how the packet_type mbuf API can evolve to solve this issue. By the way, the update of *rte_get_ptype_tunnel_name() is missing.
Hi Olivier, > On Thu, Apr 08, 2021 at 01:47:18PM +0530, Tejasree Kondoj wrote: > > Adding new mbuf packet type for UDP encapsulated > > ESP packets. > > > > Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com> > > --- > > doc/guides/rel_notes/release_21_05.rst | 5 +++++ > > lib/librte_mbuf/rte_mbuf_ptype.h | 21 +++++++++++++++++++++ > > 2 files changed, 26 insertions(+) > > > > diff --git a/doc/guides/rel_notes/release_21_05.rst > b/doc/guides/rel_notes/release_21_05.rst > > index 5565c7637c..c9e9e2ec22 100644 > > --- a/doc/guides/rel_notes/release_21_05.rst > > +++ b/doc/guides/rel_notes/release_21_05.rst > > @@ -55,6 +55,11 @@ New Features > > Also, make sure to start the actual text at the margin. > > ======================================================= > > > > +* **Added new packet type for UDP-ESP packets in mbuf.** > > + > > + Added new packet type ``RTE_PTYPE_TUNNEL_ESP_IN_UDP`` which can > be > > + used to identify UDP encapsulated ESP packets. > > + > > * **Enhanced ethdev representor syntax.** > > > > * Introduced representor type of VF, SF and PF. > > diff --git a/lib/librte_mbuf/rte_mbuf_ptype.h > b/lib/librte_mbuf/rte_mbuf_ptype.h > > index 17a2dd3576..bf92ce0c1a 100644 > > --- a/lib/librte_mbuf/rte_mbuf_ptype.h > > +++ b/lib/librte_mbuf/rte_mbuf_ptype.h > > @@ -491,6 +491,27 @@ extern "C" { > > * | 'destination port'=6635> > > */ > > #define RTE_PTYPE_TUNNEL_MPLS_IN_UDP 0x0000d000 > > +/** > > + * ESP-in-UDP tunneling packet type (RFC 3948). > > + * > > + * Packet format: > > + * <'ether type'=0x0800 > > + * | 'version'=4, 'protocol'=17 > > + * | 'destination port'=4500> > > + * or, > > + * <'ether type'=0x86DD > > + * | 'version'=6, 'next header'=17 > > + * | 'destination port'=4500> > > + * or, > > + * <'ether type'=0x0800 > > + * | 'version'=4, 'protocol'=17 > > + * | 'source port'=4500> > > + * or, > > + * <'ether type'=0x86DD > > + * | 'version'=6, 'next header'=17 > > + * | 'source port'=4500> > > + */ > > +#define RTE_PTYPE_TUNNEL_ESP_IN_UDP 0x0000e000 > > /** > > * Mask of tunneling packet types. > > */ > > We arrive at the end of the values in packet type tunnel types, > and there is another pending patch that needs another tunnel type. > > As there is already a RTE_PTYPE_TUNNEL_ESP, what would you think about > trying to reuse it, and differentiate IP/ESP from IP/UDP/ESP by using > the L4 layer type (unknown vs udp)? Or maybe add RTE_PTYPE_L4_NONE. > > It is sensible, because it can be considered as an API change for > current users of RTE_PTYPE_TUNNEL_ESP. I don't really know how this > type is used by applications. It is OK to use combination of these two but with an assumption that a normal - IP-UDP packet when encrypted will be an IP-ESP packet And L4 types are reset from the mbuf->packet_type by the driver. @Konstantin Ananyev: Are you OK with this assumption? And, if we choose this path, then also we may need a macro in this file, So that application doesn't have to combine that explicitly for a standard use case. #define RTE_PTYPE_TUNNEL_ESP_IN_UDP RTE_PTYPE_TUNNEL_ESP | RTE_PTYPE_L4_UDP Will this be fine? > > I think it is time to start thinking about how the packet_type > mbuf API can evolve to solve this issue. > > By the way, the update of *rte_get_ptype_tunnel_name() is missing.
Hi Olivier/ Konstantin, > > On Thu, Apr 08, 2021 at 01:47:18PM +0530, Tejasree Kondoj wrote: > > > Adding new mbuf packet type for UDP encapsulated > > > ESP packets. > > > > > > Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com> > > > --- > > > doc/guides/rel_notes/release_21_05.rst | 5 +++++ > > > lib/librte_mbuf/rte_mbuf_ptype.h | 21 +++++++++++++++++++++ > > > 2 files changed, 26 insertions(+) > > > > > > diff --git a/doc/guides/rel_notes/release_21_05.rst > > b/doc/guides/rel_notes/release_21_05.rst > > > index 5565c7637c..c9e9e2ec22 100644 > > > --- a/doc/guides/rel_notes/release_21_05.rst > > > +++ b/doc/guides/rel_notes/release_21_05.rst > > > @@ -55,6 +55,11 @@ New Features > > > Also, make sure to start the actual text at the margin. > > > ======================================================= > > > > > > +* **Added new packet type for UDP-ESP packets in mbuf.** > > > + > > > + Added new packet type ``RTE_PTYPE_TUNNEL_ESP_IN_UDP`` which can > > be > > > + used to identify UDP encapsulated ESP packets. > > > + > > > * **Enhanced ethdev representor syntax.** > > > > > > * Introduced representor type of VF, SF and PF. > > > diff --git a/lib/librte_mbuf/rte_mbuf_ptype.h > > b/lib/librte_mbuf/rte_mbuf_ptype.h > > > index 17a2dd3576..bf92ce0c1a 100644 > > > --- a/lib/librte_mbuf/rte_mbuf_ptype.h > > > +++ b/lib/librte_mbuf/rte_mbuf_ptype.h > > > @@ -491,6 +491,27 @@ extern "C" { > > > * | 'destination port'=6635> > > > */ > > > #define RTE_PTYPE_TUNNEL_MPLS_IN_UDP 0x0000d000 > > > +/** > > > + * ESP-in-UDP tunneling packet type (RFC 3948). > > > + * > > > + * Packet format: > > > + * <'ether type'=0x0800 > > > + * | 'version'=4, 'protocol'=17 > > > + * | 'destination port'=4500> > > > + * or, > > > + * <'ether type'=0x86DD > > > + * | 'version'=6, 'next header'=17 > > > + * | 'destination port'=4500> > > > + * or, > > > + * <'ether type'=0x0800 > > > + * | 'version'=4, 'protocol'=17 > > > + * | 'source port'=4500> > > > + * or, > > > + * <'ether type'=0x86DD > > > + * | 'version'=6, 'next header'=17 > > > + * | 'source port'=4500> > > > + */ > > > +#define RTE_PTYPE_TUNNEL_ESP_IN_UDP 0x0000e000 > > > /** > > > * Mask of tunneling packet types. > > > */ > > > > We arrive at the end of the values in packet type tunnel types, > > and there is another pending patch that needs another tunnel type. > > > > As there is already a RTE_PTYPE_TUNNEL_ESP, what would you think about > > trying to reuse it, and differentiate IP/ESP from IP/UDP/ESP by using > > the L4 layer type (unknown vs udp)? Or maybe add RTE_PTYPE_L4_NONE. > > > > It is sensible, because it can be considered as an API change for > > current users of RTE_PTYPE_TUNNEL_ESP. I don't really know how this > > type is used by applications. > > It is OK to use combination of these two but with an assumption > that a normal - IP-UDP packet when encrypted will be an IP-ESP packet > And L4 types are reset from the mbuf->packet_type by the driver. > @Konstantin Ananyev: Are you OK with this assumption? > > And, if we choose this path, then also we may need a macro in this file, > So that application doesn't have to combine that explicitly for a standard use > case. > #define RTE_PTYPE_TUNNEL_ESP_IN_UDP RTE_PTYPE_TUNNEL_ESP | > RTE_PTYPE_L4_UDP > > Will this be fine? > Can we proceed with this approach? Regards, Akhil > > > > I think it is time to start thinking about how the packet_type > > mbuf API can evolve to solve this issue. > > > > By the way, the update of *rte_get_ptype_tunnel_name() is missing.
Hi Akhil, > > Hi Olivier/ Konstantin, > > > On Thu, Apr 08, 2021 at 01:47:18PM +0530, Tejasree Kondoj wrote: > > > > Adding new mbuf packet type for UDP encapsulated > > > > ESP packets. > > > > > > > > Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com> > > > > --- > > > > doc/guides/rel_notes/release_21_05.rst | 5 +++++ > > > > lib/librte_mbuf/rte_mbuf_ptype.h | 21 +++++++++++++++++++++ > > > > 2 files changed, 26 insertions(+) > > > > > > > > diff --git a/doc/guides/rel_notes/release_21_05.rst > > > b/doc/guides/rel_notes/release_21_05.rst > > > > index 5565c7637c..c9e9e2ec22 100644 > > > > --- a/doc/guides/rel_notes/release_21_05.rst > > > > +++ b/doc/guides/rel_notes/release_21_05.rst > > > > @@ -55,6 +55,11 @@ New Features > > > > Also, make sure to start the actual text at the margin. > > > > ======================================================= > > > > > > > > +* **Added new packet type for UDP-ESP packets in mbuf.** > > > > + > > > > + Added new packet type ``RTE_PTYPE_TUNNEL_ESP_IN_UDP`` which can > > > be > > > > + used to identify UDP encapsulated ESP packets. > > > > + > > > > * **Enhanced ethdev representor syntax.** > > > > > > > > * Introduced representor type of VF, SF and PF. > > > > diff --git a/lib/librte_mbuf/rte_mbuf_ptype.h > > > b/lib/librte_mbuf/rte_mbuf_ptype.h > > > > index 17a2dd3576..bf92ce0c1a 100644 > > > > --- a/lib/librte_mbuf/rte_mbuf_ptype.h > > > > +++ b/lib/librte_mbuf/rte_mbuf_ptype.h > > > > @@ -491,6 +491,27 @@ extern "C" { > > > > * | 'destination port'=6635> > > > > */ > > > > #define RTE_PTYPE_TUNNEL_MPLS_IN_UDP 0x0000d000 > > > > +/** > > > > + * ESP-in-UDP tunneling packet type (RFC 3948). > > > > + * > > > > + * Packet format: > > > > + * <'ether type'=0x0800 > > > > + * | 'version'=4, 'protocol'=17 > > > > + * | 'destination port'=4500> > > > > + * or, > > > > + * <'ether type'=0x86DD > > > > + * | 'version'=6, 'next header'=17 > > > > + * | 'destination port'=4500> > > > > + * or, > > > > + * <'ether type'=0x0800 > > > > + * | 'version'=4, 'protocol'=17 > > > > + * | 'source port'=4500> > > > > + * or, > > > > + * <'ether type'=0x86DD > > > > + * | 'version'=6, 'next header'=17 > > > > + * | 'source port'=4500> > > > > + */ > > > > +#define RTE_PTYPE_TUNNEL_ESP_IN_UDP 0x0000e000 > > > > /** > > > > * Mask of tunneling packet types. > > > > */ > > > > > > We arrive at the end of the values in packet type tunnel types, > > > and there is another pending patch that needs another tunnel type. > > > > > > As there is already a RTE_PTYPE_TUNNEL_ESP, what would you think about > > > trying to reuse it, and differentiate IP/ESP from IP/UDP/ESP by using > > > the L4 layer type (unknown vs udp)? Or maybe add RTE_PTYPE_L4_NONE. > > > > > > It is sensible, because it can be considered as an API change for > > > current users of RTE_PTYPE_TUNNEL_ESP. I don't really know how this > > > type is used by applications. > > > > It is OK to use combination of these two but with an assumption > > that a normal - IP-UDP packet when encrypted will be an IP-ESP packet > > And L4 types are reset from the mbuf->packet_type by the driver. > > @Konstantin Ananyev: Are you OK with this assumption? > > > > And, if we choose this path, then also we may need a macro in this file, > > So that application doesn't have to combine that explicitly for a standard use > > case. > > #define RTE_PTYPE_TUNNEL_ESP_IN_UDP RTE_PTYPE_TUNNEL_ESP | > > RTE_PTYPE_L4_UDP > > > > Will this be fine? > > > Can we proceed with this approach? I think we can safely use such combination inside ipsec-secgw app. About making it a new generic type - I am not so sure. As Olivier already pointed out - it looks like an API/behaviour breakage to me. > Regards, > Akhil > > > > > > > I think it is time to start thinking about how the packet_type > > > mbuf API can evolve to solve this issue. +1 Might be it needs to be reworked completely. > > > > > > By the way, the update of *rte_get_ptype_tunnel_name() is missing.
diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst index 5565c7637c..c9e9e2ec22 100644 --- a/doc/guides/rel_notes/release_21_05.rst +++ b/doc/guides/rel_notes/release_21_05.rst @@ -55,6 +55,11 @@ New Features Also, make sure to start the actual text at the margin. ======================================================= +* **Added new packet type for UDP-ESP packets in mbuf.** + + Added new packet type ``RTE_PTYPE_TUNNEL_ESP_IN_UDP`` which can be + used to identify UDP encapsulated ESP packets. + * **Enhanced ethdev representor syntax.** * Introduced representor type of VF, SF and PF. diff --git a/lib/librte_mbuf/rte_mbuf_ptype.h b/lib/librte_mbuf/rte_mbuf_ptype.h index 17a2dd3576..bf92ce0c1a 100644 --- a/lib/librte_mbuf/rte_mbuf_ptype.h +++ b/lib/librte_mbuf/rte_mbuf_ptype.h @@ -491,6 +491,27 @@ extern "C" { * | 'destination port'=6635> */ #define RTE_PTYPE_TUNNEL_MPLS_IN_UDP 0x0000d000 +/** + * ESP-in-UDP tunneling packet type (RFC 3948). + * + * Packet format: + * <'ether type'=0x0800 + * | 'version'=4, 'protocol'=17 + * | 'destination port'=4500> + * or, + * <'ether type'=0x86DD + * | 'version'=6, 'next header'=17 + * | 'destination port'=4500> + * or, + * <'ether type'=0x0800 + * | 'version'=4, 'protocol'=17 + * | 'source port'=4500> + * or, + * <'ether type'=0x86DD + * | 'version'=6, 'next header'=17 + * | 'source port'=4500> + */ +#define RTE_PTYPE_TUNNEL_ESP_IN_UDP 0x0000e000 /** * Mask of tunneling packet types. */
Adding new mbuf packet type for UDP encapsulated ESP packets. Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com> --- doc/guides/rel_notes/release_21_05.rst | 5 +++++ lib/librte_mbuf/rte_mbuf_ptype.h | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+)