Re: [dpdk-dev] [PATCH v5 5/7] lib/librte_ether: support new l2 tunnel operation

["Wu, Jingjing" <jingjing.wu@intel.com>]

Hi, wenzhuo

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Monday, March 07, 2016 10:43 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v5 5/7] lib/librte_ether: support new l2 tunnel
> operation
> 
> Add functions to support the new l2 tunnel operation.
> 1, Insertion and stripping for l2 tunnel tag.
> 2, Forwarding the packets to a pool based on l2 tunnel tag.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
>  lib/librte_ether/rte_ethdev.c | 183
> ++++++++++++++++++++++++++++++++++++
>  lib/librte_ether/rte_ethdev.h | 214
> ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 397 insertions(+)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index c5c12cb..6bc19d4 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -3298,3 +3298,186 @@ rte_eth_dev_l2_tunnel_disable(uint8_t port_id,
>  				-ENOTSUP);
>  	return (*dev->dev_ops->l2_tunnel_disable)(dev, l2_tunnel_type);  }
> +
> +int
> +rte_eth_dev_l2_tunnel_insertion_enable(uint8_t port_id,
> +				       struct rte_eth_l2_tunnel *l2_tunnel,
> +				       uint16_t vf_id)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	if (l2_tunnel == NULL) {
> +		RTE_PMD_DEBUG_TRACE("Invalid l2_tunnel parameter\n");
> +		return -EINVAL;
> +	}
> +
> +	if (l2_tunnel->l2_tunnel_type >= RTE_L2_TUNNEL_TYPE_MAX) {
> +		RTE_PMD_DEBUG_TRACE("Invalid l2 tunnel type\n");
> +		return -EINVAL;
> +	}
> +
> +	dev = &rte_eth_devices[port_id];
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >l2_tunnel_insertion_enable,
> +				-ENOTSUP);
> +	return (*dev->dev_ops->l2_tunnel_insertion_enable)(dev,
> +							   l2_tunnel,
> +							   vf_id);
> +}
> +
> +int
> +rte_eth_dev_l2_tunnel_insertion_disable(uint8_t port_id,
> +					enum rte_eth_l2_tunnel_type
> +						l2_tunnel_type,
> +					uint16_t vf_id)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +
> +	if (l2_tunnel_type >= RTE_L2_TUNNEL_TYPE_MAX) {
> +		RTE_PMD_DEBUG_TRACE("Invalid l2 tunnel type\n");
> +		return -EINVAL;
> +	}
> +
> +	dev = &rte_eth_devices[port_id];
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >l2_tunnel_insertion_disable,
> +				-ENOTSUP);
> +	return (*dev->dev_ops->l2_tunnel_insertion_disable)(dev,
> +							    l2_tunnel_type,
> +							    vf_id);
> +}
> +
> +int
> +rte_eth_dev_l2_tunnel_stripping_enable(uint8_t port_id,
> +				       enum rte_eth_l2_tunnel_type
> +					       l2_tunnel_type)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +
> +	if (l2_tunnel_type >= RTE_L2_TUNNEL_TYPE_MAX) {
> +		RTE_PMD_DEBUG_TRACE("Invalid l2 tunnel type\n");
> +		return -EINVAL;
> +	}
> +
> +	dev = &rte_eth_devices[port_id];
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >l2_tunnel_stripping_enable,
> +				-ENOTSUP);
> +	return (*dev->dev_ops->l2_tunnel_stripping_enable)(dev,
> +							   l2_tunnel_type);
> +}
> +
> +int
> +rte_eth_dev_l2_tunnel_stripping_disable(uint8_t port_id,
> +					 enum rte_eth_l2_tunnel_type
> +						 l2_tunnel_type)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +
> +	if (l2_tunnel_type >= RTE_L2_TUNNEL_TYPE_MAX) {
> +		RTE_PMD_DEBUG_TRACE("Invalid l2 tunnel type\n");
> +		return -EINVAL;
> +	}
> +
> +	dev = &rte_eth_devices[port_id];
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >l2_tunnel_stripping_disable,
> +				-ENOTSUP);
> +	return (*dev->dev_ops->l2_tunnel_stripping_disable)(dev,
> +							    l2_tunnel_type);
> +}
> +
> +int
> +rte_eth_dev_l2_tunnel_forwarding_enable(uint8_t port_id,
> +					 enum rte_eth_l2_tunnel_type
> +						 l2_tunnel_type)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +
> +	if (l2_tunnel_type >= RTE_L2_TUNNEL_TYPE_MAX) {
> +		RTE_PMD_DEBUG_TRACE("Invalid l2 tunnel type\n");
> +		return -EINVAL;
> +	}
> +
> +	dev = &rte_eth_devices[port_id];
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >l2_tunnel_forwarding_enable,
> +				-ENOTSUP);
> +	return (*dev->dev_ops->l2_tunnel_forwarding_enable)(dev,
> +							    l2_tunnel_type);
> +}
> +
> +int
> +rte_eth_dev_l2_tunnel_forwarding_disable(uint8_t port_id,
> +					 enum rte_eth_l2_tunnel_type
> +						 l2_tunnel_type)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +
> +	if (l2_tunnel_type >= RTE_L2_TUNNEL_TYPE_MAX) {
> +		RTE_PMD_DEBUG_TRACE("Invalid l2 tunnel type\n");
> +		return -EINVAL;
> +	}
> +
> +	dev = &rte_eth_devices[port_id];
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >l2_tunnel_forwarding_disable,
> +				-ENOTSUP);
> +	return (*dev->dev_ops->l2_tunnel_forwarding_disable)(dev,
> +							     l2_tunnel_type);
> +}
> +

How about to merge the above six APIs rte_eth_dev_l2_tunnel_XXX_enable/disable
to one, may call rte_eth_dev_l2_tunnel_offload_set?
Just like the rte_eth_dev_set_vlan_offload.


> +int
> +rte_eth_dev_l2_tunnel_filter_add(uint8_t port_id,
> +				 struct rte_eth_l2_tunnel *l2_tunnel,
> +				 uint32_t pool)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	if (l2_tunnel == NULL) {
> +		RTE_PMD_DEBUG_TRACE("Invalid l2_tunnel parameter\n");
> +		return -EINVAL;
> +	}
> +
> +	if (l2_tunnel->l2_tunnel_type >= RTE_L2_TUNNEL_TYPE_MAX) {
> +		RTE_PMD_DEBUG_TRACE("Invalid l2 tunnel type\n");
> +		return -EINVAL;
> +	}
> +
> +	dev = &rte_eth_devices[port_id];
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >l2_tunnel_filter_add,
> +				-ENOTSUP);
> +	return (*dev->dev_ops->l2_tunnel_filter_add)(dev,
> +						     l2_tunnel,
> +						     pool);
> +}

Pool is used here, but vf_id is used above. Suggest to put such arguments into structure, maybe in l2_tunnel.

Thanks
Jingjing