[dpdk-dev] [PATCH] ethdev: refine API description

[dpdk-dev] [PATCH] ethdev: refine API description

[Qi Zhang]

Refine the description for rte_eth_dev_udp_tunnel_port_add.
Claim this is an API for device (or port) level configuration.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 lib/librte_ethdev/rte_ethdev.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index f5f891918..a7bb16d45 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -4030,6 +4030,16 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
  * to change or add more UDP port for the tunnel. So the offloading function
  * can take effect on the packets with the specific UDP port.
  *
+ * Due to different requirements from different use cases, NICs may have a
+ * different way to identify a UDP port as a tunnel type. Some NIC takes this
+ * as a device (or port) level configure while some NIC takes this as a flow
+ * based configure.
+ *
+ * This API is for the first case and typically it will only be implemented
+ * on a PF driver or a VF driver which have privilege right to configure for
+ * other VFs. For the second case, a tunnel configure could be embedded in a
+ * rte_flow rule.
+ *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param tunnel_udp
-- 
2.26.2

Re: [dpdk-dev] [PATCH] ethdev: refine API description

[Thomas Monjalon]

Hi,

It seems we need to clarify how the API for UDP tunnel works
with rte_flow. Thanks for starting this patch.
I ask some questions below for writing a clear and exact API definition.

12/01/2021 12:47, Qi Zhang:
> Refine the description for rte_eth_dev_udp_tunnel_port_add.
> Claim this is an API for device (or port) level configuration.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -4030,6 +4030,16 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
>   * to change or add more UDP port for the tunnel. So the offloading function
>   * can take effect on the packets with the specific UDP port.
>   *
> + * Due to different requirements from different use cases, NICs may have a
> + * different way to identify a UDP port as a tunnel type. Some NIC takes this
> + * as a device (or port) level configure while some NIC takes this as a flow
> + * based configure.

I think this assumption is too vague to be useful.
It brings more confusion than it explains.

> + *
> + * This API is for the first case and typically it will only be implemented
> + * on a PF driver or a VF driver which have privilege right to configure for

What is a privileged VF exactly?

> + * other VFs. For the second case, a tunnel configure could be embedded in a
> + * rte_flow rule.

I suggest we make the explanation more API-oriented.

First thing to explain: this API will have effect on rte_flow rules only,
am I right?

What does it mean for a flow rule matching on a specific tunnel?
Let's take an example config:
	rte_eth_dev_udp_tunnel_port_add [UDP X] [tunnel T]
	rte_flow_create [tunnel T] [action A]
	rte_flow_create [UDP Y] [tunnel T] [action B]
Then action for these packets:
	1/ [UDP X] - no tunnel header
-> A (rte_eth_udp_tunnel skips the tunnel header check)
-> or none, tunnel header is checked according to rte_flow?
	2/ [UDP Y] - no tunnel header
-> none (flow rule requires a tunnel header)
	3/ [UDP X] [tunnel T]
-> A
	4/ [UDP Y] [tunnel T]
-> B
	5/ [UDP X] [tunnel U]
-> A (rte_eth_udp_tunnel skips the tunnel header check)
-> or none, tunnel header is checked according to rte_flow?
	6/ [UDP Y] [tunnel U]
-> none

Last question, how it plays with rte_flow_tunnel_match?
Should we replace rte_eth_udp_tunnel with rte_flow_tunnel_match?

Re: [dpdk-dev] [PATCH] ethdev: refine API description

[Zhang, Qi Z]

Hi Thomas:
	Thanks for review, comment in line

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Friday, January 15, 2021 11:52 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; orika@nvidia.com;
> getelson@nvidia.com; andrew.rybchenko@oktetlabs.ru;
> ajit.khaparde@broadcom.com; jerinj@marvell.com
> Subject: Re: [dpdk-dev] [PATCH] ethdev: refine API description
> 
> Hi,
> 
> It seems we need to clarify how the API for UDP tunnel works with rte_flow.
> Thanks for starting this patch.
> I ask some questions below for writing a clear and exact API definition.
> 
> 12/01/2021 12:47, Qi Zhang:
> > Refine the description for rte_eth_dev_udp_tunnel_port_add.
> > Claim this is an API for device (or port) level configuration.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > ---
> > --- a/lib/librte_ethdev/rte_ethdev.h
> > +++ b/lib/librte_ethdev/rte_ethdev.h
> > @@ -4030,6 +4030,16 @@ rte_eth_dev_rss_hash_conf_get(uint16_t
> port_id,
> >   * to change or add more UDP port for the tunnel. So the offloading
> function
> >   * can take effect on the packets with the specific UDP port.
> >   *
> > + * Due to different requirements from different use cases, NICs may
> > + have a
> > + * different way to identify a UDP port as a tunnel type. Some NIC
> > + takes this
> > + * as a device (or port) level configure while some NIC takes this as
> > + a flow
> > + * based configure.
> 
> I think this assumption is too vague to be useful.
> It brings more confusion than it explains.

OK, let's focus on the API impact as you suggested.
> 
> > + *
> > + * This API is for the first case and typically it will only be
> > + implemented
> > + * on a PF driver or a VF driver which have privilege right to
> > + configure for
> 
> What is a privileged VF exactly?

Yes, looks like "a privileged VF" is not generic enough here, actually it should be a driver that implement DPDK ethdev API and able to perform device/port level configure, 
for example use rte_flow to steer traffic to specific VF, configure UDP tunnel port .... it is something like a "host driver"
It could be a pure user pace DPDK PF driver, or a vdev / SR-IOV driver back ended by a kernel driver but be granted to have privilege to access more device resource through some sw/hw channel.

> 
> > + * other VFs. For the second case, a tunnel configure could be
> > + embedded in a
> > + * rte_flow rule.
> 
> I suggest we make the explanation more API-oriented.
> 
> First thing to explain: this API will have effect on rte_flow rules only, am I right?

I think it should not only impact the rte_flow, but also may impact the packet type that extract from Rx Descriptor to mbuf on some devices
Because without this configure, the parser is not able to recognize the specific tunnel type

> 
> What does it mean for a flow rule matching on a specific tunnel?
> Let's take an example config:
> 	rte_eth_dev_udp_tunnel_port_add [UDP X] [tunnel T]
> 	rte_flow_create [tunnel T] [action A]
> 	rte_flow_create [UDP Y] [tunnel T] [action B] Then action for these

Some driver may not allow to specific a tunnel port in rte_flow as rte_eth_dev_udp_tunnel_port_add is prefer API to handle this.

> packets:
> 	1/ [UDP X] - no tunnel header
> -> A (rte_eth_udp_tunnel skips the tunnel header check) or none, tunnel
> -> header is checked according to rte_flow?

If a packet only have udp tunnel port but don't have tunnel header, so the packet will still not be recognized as a an tunnel packet by the parser, the pattern is not matched and action A should not be executed.

> 	2/ [UDP Y] - no tunnel header
> -> none (flow rule requires a tunnel header)

Yes, expected result

> 	3/ [UDP X] [tunnel T]
> -> A

Yes, expected result

> 	4/ [UDP Y] [tunnel T]
> -> B

Yes, expected result, if the rule #2 is allowed to create

> 	5/ [UDP X] [tunnel U]
> -> A (rte_eth_udp_tunnel skips the tunnel header check) or none, tunnel
> -> header is checked according to rte_flow?

Should be none.

> 	6/ [UDP Y] [tunnel U]
> -> none

Yes expected result.
> 
> Last question, how it plays with rte_flow_tunnel_match?
> Should we replace rte_eth_udp_tunnel with rte_flow_tunnel_match?

My understanding is rte_flow_tunnel_match is just a help function, it help to generate a set of rte_flow_items to feed a new rule creation, so looks like it is not expected to have any interaction with hardware? So I didn't figure out how can we use it to replace,  But maybe we can introduce something new like rte_flow_tunnel_create as we already have a rte_flow_tunnel structure which looks more generic than the existing API that only take udp port for tunnel configure.
> 

Re: [dpdk-dev] [PATCH] ethdev: refine API description

[Thomas Monjalon]

18/01/2021 05:01, Zhang, Qi Z:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 12/01/2021 12:47, Qi Zhang:
> > > + * This API is for the first case and typically it will only be
> > > + implemented
> > > + * on a PF driver or a VF driver which have privilege right to
> > > + configure for
> > 
> > What is a privileged VF exactly?
> 
> Yes, looks like "a privileged VF" is not generic enough here, actually it should be a driver that implement DPDK ethdev API and able to perform device/port level configure, 
> for example use rte_flow to steer traffic to specific VF, configure UDP tunnel port .... it is something like a "host driver"
> It could be a pure user pace DPDK PF driver, or a vdev / SR-IOV driver back ended by a kernel driver but be granted to have privilege to access more device resource through some sw/hw channel.

How such device is granted privilege?


> > > + * other VFs. For the second case, a tunnel configure could be
> > > + embedded in a
> > > + * rte_flow rule.
> > 
> > I suggest we make the explanation more API-oriented.
> > 
> > First thing to explain: this API will have effect on rte_flow rules only, am I right?
> 
> I think it should not only impact the rte_flow, but also may impact the packet type that extract from Rx Descriptor to mbuf on some devices
> Because without this configure, the parser is not able to recognize the specific tunnel type

Oh right.
This impact on mbuf classification should be part of the doxygen explanation.


> > What does it mean for a flow rule matching on a specific tunnel?
> > Let's take an example config:
> > 	rte_eth_dev_udp_tunnel_port_add [UDP X] [tunnel T]
> > 	rte_flow_create [tunnel T] [action A]
> > 	rte_flow_create [UDP Y] [tunnel T] [action B] Then action for these
> 
> Some driver may not allow to specific a tunnel port in rte_flow as rte_eth_dev_udp_tunnel_port_add is prefer API to handle this.

Every rte_flow items and actions have limitations in drivers, that's OK.

> > packets:
> > 	1/ [UDP X] - no tunnel header
> > -> A (rte_eth_udp_tunnel skips the tunnel header check) or none, tunnel
> > -> header is checked according to rte_flow?
> 
> If a packet only have udp tunnel port but don't have tunnel header, so the packet will still not be recognized as a an tunnel packet by the parser, the pattern is not matched and action A should not be executed.

OK I like it. Please make this behaviour clear in the doxygen.


> > 	2/ [UDP Y] - no tunnel header
> > -> none (flow rule requires a tunnel header)
> 
> Yes, expected result
> 
> > 	3/ [UDP X] [tunnel T]
> > -> A
> 
> Yes, expected result
> 
> > 	4/ [UDP Y] [tunnel T]
> > -> B
> 
> Yes, expected result, if the rule #2 is allowed to create

Yes of course, if the rule cannot be created, the driver will reject it
with an appropriate error code and a log mentioning the limitation.

> > 	5/ [UDP X] [tunnel U]
> > -> A (rte_eth_udp_tunnel skips the tunnel header check) or none, tunnel
> > -> header is checked according to rte_flow?
> 
> Should be none.

OK makes sense.

> > 	6/ [UDP Y] [tunnel U]
> > -> none
> 
> Yes expected result.
> > 
> > Last question, how it plays with rte_flow_tunnel_match?
> > Should we replace rte_eth_udp_tunnel with rte_flow_tunnel_match?
> 
> My understanding is rte_flow_tunnel_match is just a help function, it help to generate a set of rte_flow_items to feed a new rule creation, so looks like it is not expected to have any interaction with hardware? So I didn't figure out how can we use it to replace,  But maybe we can introduce something new like rte_flow_tunnel_create as we already have a rte_flow_tunnel structure which looks more generic than the existing API that only take udp port for tunnel configure.

Yes we need to think about it for future more generic API.
Ori, any thoughts?

Re: [dpdk-dev] [PATCH] ethdev: refine API description

[Zhang, Qi Z]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, January 18, 2021 5:50 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; orika@nvidia.com;
> getelson@nvidia.com; andrew.rybchenko@oktetlabs.ru;
> ajit.khaparde@broadcom.com; jerinj@marvell.com
> Subject: Re: [dpdk-dev] [PATCH] ethdev: refine API description
> 
> 18/01/2021 05:01, Zhang, Qi Z:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 12/01/2021 12:47, Qi Zhang:
> > > > + * This API is for the first case and typically it will only be
> > > > + implemented
> > > > + * on a PF driver or a VF driver which have privilege right to
> > > > + configure for
> > >
> > > What is a privileged VF exactly?
> >
> > Yes, looks like "a privileged VF" is not generic enough here, actually
> > it should be a driver that implement DPDK ethdev API and able to perform
> device/port level configure, for example use rte_flow to steer traffic to specific
> VF, configure UDP tunnel port .... it is something like a "host driver"
> > It could be a pure user pace DPDK PF driver, or a vdev / SR-IOV driver back
> ended by a kernel driver but be granted to have privilege to access more device
> resource through some sw/hw channel.
> 
> How such device is granted privilege?

There is no unified way currently as I know, but typically it should be configured from the host by an administrator through tools like ip / devlink ...

> 
> 
> > > > + * other VFs. For the second case, a tunnel configure could be
> > > > + embedded in a
> > > > + * rte_flow rule.
> > >
> > > I suggest we make the explanation more API-oriented.
> > >
> > > First thing to explain: this API will have effect on rte_flow rules only, am I
> right?
> >
> > I think it should not only impact the rte_flow, but also may impact
> > the packet type that extract from Rx Descriptor to mbuf on some
> > devices Because without this configure, the parser is not able to
> > recognize the specific tunnel type
> 
> Oh right.
> This impact on mbuf classification should be part of the doxygen explanation.
> 
> 
> > > What does it mean for a flow rule matching on a specific tunnel?
> > > Let's take an example config:
> > > 	rte_eth_dev_udp_tunnel_port_add [UDP X] [tunnel T]
> > > 	rte_flow_create [tunnel T] [action A]
> > > 	rte_flow_create [UDP Y] [tunnel T] [action B] Then action for these
> >
> > Some driver may not allow to specific a tunnel port in rte_flow as
> rte_eth_dev_udp_tunnel_port_add is prefer API to handle this.
> 
> Every rte_flow items and actions have limitations in drivers, that's OK.
> 
> > > packets:
> > > 	1/ [UDP X] - no tunnel header
> > > -> A (rte_eth_udp_tunnel skips the tunnel header check) or none,
> > > -> tunnel header is checked according to rte_flow?
> >
> > If a packet only have udp tunnel port but don't have tunnel header, so the
> packet will still not be recognized as a an tunnel packet by the parser, the pattern
> is not matched and action A should not be executed.
> 
> OK I like it. Please make this behaviour clear in the doxygen.
> 
> 
> > > 	2/ [UDP Y] - no tunnel header
> > > -> none (flow rule requires a tunnel header)
> >
> > Yes, expected result
> >
> > > 	3/ [UDP X] [tunnel T]
> > > -> A
> >
> > Yes, expected result
> >
> > > 	4/ [UDP Y] [tunnel T]
> > > -> B
> >
> > Yes, expected result, if the rule #2 is allowed to create
> 
> Yes of course, if the rule cannot be created, the driver will reject it with an
> appropriate error code and a log mentioning the limitation.
> 
> > > 	5/ [UDP X] [tunnel U]
> > > -> A (rte_eth_udp_tunnel skips the tunnel header check) or none,
> > > -> tunnel header is checked according to rte_flow?
> >
> > Should be none.
> 
> OK makes sense.
> 
> > > 	6/ [UDP Y] [tunnel U]
> > > -> none
> >
> > Yes expected result.
> > >
> > > Last question, how it plays with rte_flow_tunnel_match?
> > > Should we replace rte_eth_udp_tunnel with rte_flow_tunnel_match?
> >
> > My understanding is rte_flow_tunnel_match is just a help function, it help to
> generate a set of rte_flow_items to feed a new rule creation, so looks like it is
> not expected to have any interaction with hardware? So I didn't figure out how
> can we use it to replace,  But maybe we can introduce something new like
> rte_flow_tunnel_create as we already have a rte_flow_tunnel structure which
> looks more generic than the existing API that only take udp port for tunnel
> configure.
> 
> Yes we need to think about it for future more generic API.
> Ori, any thoughts?
> 

[dpdk-dev] [PATCH v2] ethdev: refine doxygen for add UDP tunnel port API

[Qi Zhang]

Refine the doxygen for rte_eth_dev_udp_tunnel_port_add.
Add more detail description of the impacted offload functions.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---

v2:
- reword doxygen that focus on API impact base on previous discussion. 

 lib/librte_ethdev/rte_ethdev.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index f758ec837..ab50a7039 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -4031,6 +4031,17 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
  * to change or add more UDP port for the tunnel. So the offloading function
  * can take effect on the packets with the specific UDP port.
  *
+ * The impacted offloading functions include:
+ *
+ * - A specific tunnel type in mbuf->packet_type
+ *
+ * - A rte_flow rule that matches on specific tunnel header
+ *
+ * NOTE: If a packet only has a matched UDP port but don't have a legal tunnel
+ *       header, the packet may still not be recognized as a tunnel packet by
+ *       the device parser, then the related offloading function will not take
+ *       effect.
+ *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param tunnel_udp
-- 
2.26.2

Re: [dpdk-dev] [PATCH v2] ethdev: refine doxygen for add UDP tunnel port API

[Ferruh Yigit]

On 1/19/2021 3:19 AM, Qi Zhang wrote:
> Refine the doxygen for rte_eth_dev_udp_tunnel_port_add.
> Add more detail description of the impacted offload functions.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
> 
> v2:
> - reword doxygen that focus on API impact base on previous discussion.
> 
>   lib/librte_ethdev/rte_ethdev.h | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index f758ec837..ab50a7039 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -4031,6 +4031,17 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
>    * to change or add more UDP port for the tunnel. So the offloading function
>    * can take effect on the packets with the specific UDP port.
>    *
> + * The impacted offloading functions include:
> + *
> + * - A specific tunnel type in mbuf->packet_type
> + *
> + * - A rte_flow rule that matches on specific tunnel header
> + *
> + * NOTE: If a packet only has a matched UDP port but don't have a legal tunnel
> + *       header, the packet may still not be recognized as a tunnel packet by
> + *       the device parser, then the related offloading function will not take
> + *       effect.
> + *
>    * @param port_id
>    *   The port identifier of the Ethernet device.
>    * @param tunnel_udp
> 

Hi Thomas, is the v2 good to go?

Re: [dpdk-dev] [PATCH v2] ethdev: refine doxygen for add UDP tunnel port API

[Thomas Monjalon]

27/01/2021 12:34, Ferruh Yigit:
> On 1/19/2021 3:19 AM, Qi Zhang wrote:
> > Refine the doxygen for rte_eth_dev_udp_tunnel_port_add.
> > Add more detail description of the impacted offload functions.
> > 
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > ---
> > 
> > v2:
> > - reword doxygen that focus on API impact base on previous discussion.
> > 
> >   lib/librte_ethdev/rte_ethdev.h | 11 +++++++++++
> >   1 file changed, 11 insertions(+)
> > 
> > diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> > index f758ec837..ab50a7039 100644
> > --- a/lib/librte_ethdev/rte_ethdev.h
> > +++ b/lib/librte_ethdev/rte_ethdev.h
> > @@ -4031,6 +4031,17 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
> >    * to change or add more UDP port for the tunnel. So the offloading function
> >    * can take effect on the packets with the specific UDP port.
> >    *
> > + * The impacted offloading functions include:
> > + *
> > + * - A specific tunnel type in mbuf->packet_type
> > + *
> > + * - A rte_flow rule that matches on specific tunnel header
> > + *
> > + * NOTE: If a packet only has a matched UDP port but don't have a legal tunnel
> > + *       header, the packet may still not be recognized as a tunnel packet by
> > + *       the device parser, then the related offloading function will not take
> > + *       effect.
> > + *
> >    * @param port_id
> >    *   The port identifier of the Ethernet device.
> >    * @param tunnel_udp
> > 
> 
> Hi Thomas, is the v2 good to go?

Sorry I didn't take time to work on the wording.
I think we can make the explanation a bit more precise
with few more updates in the existing lines for this function and
for struct rte_eth_udp_tunnel and enum rte_eth_tunnel_type.
I agree with the idea and would like to propose a v3 for -rc3 integration
if possible.

The ethdev API documentation is not an easy task.
As we are improving it, let's not miss some corners.

[dpdk-dev] [PATCH v3 1/1] ethdev: refine doxygen comment of UDP tunnel API

[Thomas Monjalon]

From: Qi Zhang <qi.z.zhang@intel.com>

Clarify what is the scope and impact of the UDP port tunnel API.

There are still missing infos to be improved in future:
	- no capability flag
	- dependency between ports of the same device
	- required privilege

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
v2 (Qi): focus on API impact based on previous discussion
v3 (Thomas): reword more lines, in functions, struct and enum
---
 lib/librte_ethdev/rte_ethdev.h | 57 ++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 24 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 667373f06f..059a061072 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1214,7 +1214,8 @@ struct rte_eth_pfc_conf {
 };
 
 /**
- * Tunneled type.
+ * Tunnel type for device-specific classifier configuration.
+ * @see rte_eth_udp_tunnel
  */
 enum rte_eth_tunnel_type {
 	RTE_TUNNEL_TYPE_NONE = 0,
@@ -1270,14 +1271,16 @@ struct rte_fdir_conf {
 
 /**
  * UDP tunneling configuration.
- * Used to config the UDP port for a type of tunnel.
- * NICs need the UDP port to identify the tunnel type.
- * Normally a type of tunnel has a default UDP port, this structure can be used
- * in case if the users want to change or support more UDP port.
+ *
+ * Used to configure the classifier of a device,
+ * associating an UDP port with a type of tunnel.
+ *
+ * Some NICs may need such configuration to properly parse a tunnel
+ * with any standard or custom UDP port.
  */
 struct rte_eth_udp_tunnel {
 	uint16_t udp_port; /**< UDP port used for the tunnel. */
-	uint8_t prot_type; /**< Tunnel type. Defined in rte_eth_tunnel_type. */
+	uint8_t prot_type; /**< Tunnel type. @see rte_eth_tunnel_type */
 };
 
 /**
@@ -3868,7 +3871,7 @@ int rte_eth_dev_rss_reta_update(uint16_t port_id,
 				struct rte_eth_rss_reta_entry64 *reta_conf,
 				uint16_t reta_size);
 
- /**
+/**
  * Query Redirection Table(RETA) of Receive Side Scaling of Ethernet device.
  *
  * @param port_id
@@ -3890,7 +3893,7 @@ int rte_eth_dev_rss_reta_query(uint16_t port_id,
 			       struct rte_eth_rss_reta_entry64 *reta_conf,
 			       uint16_t reta_size);
 
- /**
+/**
  * Updates unicast hash table for receiving packet with the given destination
  * MAC address, and the packet is routed to all VFs for which the RX mode is
  * accept packets that match the unicast hash table.
@@ -3912,7 +3915,7 @@ int rte_eth_dev_rss_reta_query(uint16_t port_id,
 int rte_eth_dev_uc_hash_table_set(uint16_t port_id, struct rte_ether_addr *addr,
 				  uint8_t on);
 
- /**
+/**
  * Updates all unicast hash bitmaps for receiving packet with any Unicast
  * Ethernet MAC addresses,the packet is routed to all VFs for which the RX
  * mode is accept packets that match the unicast hash table.
@@ -3995,7 +3998,7 @@ int rte_eth_mirror_rule_reset(uint16_t port_id,
 int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 			uint16_t tx_rate);
 
- /**
+/**
  * Configuration of Receive Side Scaling hash computation of Ethernet device.
  *
  * @param port_id
@@ -4012,7 +4015,7 @@ int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 int rte_eth_dev_rss_hash_update(uint16_t port_id,
 				struct rte_eth_rss_conf *rss_conf);
 
- /**
+/**
  * Retrieve current configuration of Receive Side Scaling hash computation
  * of Ethernet device.
  *
@@ -4030,12 +4033,18 @@ int
 rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
 			      struct rte_eth_rss_conf *rss_conf);
 
- /**
- * Add UDP tunneling port for a specific type of tunnel.
- * The packets with this UDP port will be identified as this type of tunnel.
- * Before enabling any offloading function for a tunnel, users can call this API
- * to change or add more UDP port for the tunnel. So the offloading function
- * can take effect on the packets with the specific UDP port.
+/**
+ * Add UDP tunneling port for a type of tunnel.
+ *
+ * Some NICs may require such configuration to properly parse a tunnel
+ * with any standard or custom UDP port.
+ * The packets with this UDP port will be parsed for this type of tunnel.
+ * The device parser will also check the rest of the tunnel headers
+ * before classifying the packet.
+ *
+ * With some devices, this API will affect packet classification, i.e.:
+ *     - mbuf.packet_type reported on Rx
+ *     - rte_flow rules with tunnel items
  *
  * @param port_id
  *   The port identifier of the Ethernet device.
@@ -4052,13 +4061,13 @@ int
 rte_eth_dev_udp_tunnel_port_add(uint16_t port_id,
 				struct rte_eth_udp_tunnel *tunnel_udp);
 
- /**
- * Delete UDP tunneling port a specific type of tunnel.
- * The packets with this UDP port will not be identified as this type of tunnel
- * any more.
- * Before enabling any offloading function for a tunnel, users can call this API
- * to delete a UDP port for the tunnel. So the offloading function will not take
- * effect on the packets with the specific UDP port.
+/**
+ * Delete UDP tunneling port for a type of tunnel.
+ *
+ * The packets with this UDP port will not be classified as this type of tunnel
+ * anymore if the device use such mapping for tunnel packet classification.
+ *
+ * @see rte_eth_dev_udp_tunnel_port_add
  *
  * @param port_id
  *   The port identifier of the Ethernet device.
-- 
2.30.0

Re: [dpdk-dev] [PATCH v3 1/1] ethdev: refine doxygen comment of UDP tunnel API

[Andrew Rybchenko]

On 2/3/21 11:02 PM, Thomas Monjalon wrote:
> From: Qi Zhang <qi.z.zhang@intel.com>
> 
> Clarify what is the scope and impact of the UDP port tunnel API.
> 
> There are still missing infos to be improved in future:
> 	- no capability flag
> 	- dependency between ports of the same device
> 	- required privilege
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

Re: [dpdk-dev] [PATCH v3 1/1] ethdev: refine doxygen comment of UDP tunnel API

[Ferruh Yigit]

On 2/6/2021 10:40 AM, Andrew Rybchenko wrote:
> On 2/3/21 11:02 PM, Thomas Monjalon wrote:
>> From: Qi Zhang <qi.z.zhang@intel.com>
>>
>> Clarify what is the scope and impact of the UDP port tunnel API.
>>
>> There are still missing infos to be improved in future:
>> 	- no capability flag
>> 	- dependency between ports of the same device
>> 	- required privilege
>>
>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH v3 1/1] ethdev: refine doxygen comment of UDP tunnel API

[Thomas Monjalon]

10/02/2021 12:20, Ferruh Yigit:
> On 2/6/2021 10:40 AM, Andrew Rybchenko wrote:
> > On 2/3/21 11:02 PM, Thomas Monjalon wrote:
> >> From: Qi Zhang <qi.z.zhang@intel.com>
> >>
> >> Clarify what is the scope and impact of the UDP port tunnel API.
> >>
> >> There are still missing infos to be improved in future:
> >> 	- no capability flag
> >> 	- dependency between ports of the same device
> >> 	- required privilege
> >>
> >> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> >> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > 
> > Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied