[v2] ethdev: refine doxygen for add UDP tunnel port API

Message ID 20210119031905.518082-1-qi.z.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v2] ethdev: refine doxygen for add UDP tunnel port API |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Functional success Functional Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing warning Testing issues

Commit Message

Qi Zhang Jan. 19, 2021, 3:19 a.m. UTC
  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(+)
  

Comments

Ferruh Yigit Jan. 27, 2021, 11:34 a.m. UTC | #1
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?
  
Thomas Monjalon Jan. 27, 2021, 10:46 p.m. UTC | #2
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.
  

Patch

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