Re: [dpdk-dev] [PATCH v7 1/5] lib/librte_ether: change function name of tunnel port config

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2016-03-04 10:35, Wenzhuo Lu:
> The names of function for tunnel port configuration are not
> accurate. They're tunnel_add/del, better change them to
> tunnel_port_add/del.

As a lot of ethdev API, it is really badly documented.

Please explain why this renaming and let's try to reword the
doxygen:
 * Add UDP tunneling port of an Ethernet device for filtering a specific
 * tunneling packet by UDP port number.

Please what are the values of
struct rte_eth_udp_tunnel {                                                                                      
    uint16_t udp_port;
    uint8_t prot_type;
};
When I see an API struct without any comment, I feel it must be dropped.

By the way, it is yet another filtering API, so it must be totally reworked.

> As it may be an ABI change if change the names directly, the
> new functions are added but not remove the old ones. The old
> ones will be removed in the next release after an ABI change
> announcement.

Please make the announce in this patch.

> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -3403,6 +3415,9 @@ rte_eth_dev_rss_hash_conf_get(uint8_t port_id,
>  int
>  rte_eth_dev_udp_tunnel_add(uint8_t port_id,
>  			   struct rte_eth_udp_tunnel *tunnel_udp);

You must deprecate this one and put a comment above
with something like @see rte_eth_dev_udp_tunnel_port_add.

> +int
> +rte_eth_dev_udp_tunnel_port_add(uint8_t port_id,
> +				struct rte_eth_udp_tunnel *tunnel_udp);

You must move it below the doxygen comment.
>  
>   /**
>   * Detete UDP tunneling port configuration of Ethernet device
> @@ -3420,6 +3435,9 @@ rte_eth_dev_udp_tunnel_add(uint8_t port_id,
>  int
>  rte_eth_dev_udp_tunnel_delete(uint8_t port_id,
>  			      struct rte_eth_udp_tunnel *tunnel_udp);
> +int
> +rte_eth_dev_udp_tunnel_port_delete(uint8_t port_id,
> +				   struct rte_eth_udp_tunnel *tunnel_udp);

idem

> --- a/lib/librte_ether/rte_ether_version.map
> +++ b/lib/librte_ether/rte_ether_version.map
> @@ -114,6 +114,8 @@ DPDK_2.2 {
>  	rte_eth_tx_queue_setup;
>  	rte_eth_xstats_get;
>  	rte_eth_xstats_reset;
> +	rte_eth_dev_udp_tunnel_port_add;
> +	rte_eth_dev_udp_tunnel_port_delete;
>  
>  	local: *;
>  };

Panu already made a comment about adding a new section for 16.04.