[dpdk-dev,v6,2/9] librte_ether:add VxLAN packet identification API in librte_ether

Message ID 1413881168-20239-3-git-send-email-jijiang.liu@intel.com (mailing list archive)
State Changes Requested, archived
Headers

Commit Message

Jijiang Liu Oct. 21, 2014, 8:46 a.m. UTC
  There are "some" destination UDP port numbers that have unque meaning.
In terms of VxLAN, "IANA has assigned the value 4789 for the VXLAN UDP port, and this value
SHOULD be used by default as the destination UDP port. Some early implementations of VXLAN
have used other values for the destination port. To enable interoperability with these 
implementations, the destination port SHOULD be configurable."

Add two APIs in librte_ether for supporting UDP tunneling port configuration on i40e.
Currently, only VxLAN is implemented in this patch set.
 
Signed-off-by: Jijiang Liu <jijiang.liu@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
Acked-by: Jing Chen <jing.d.chen@intel.com>
---
 lib/librte_ether/rte_ethdev.c |   63 ++++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ethdev.h |   75 +++++++++++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ether.h  |    8 ++++
 3 files changed, 146 insertions(+), 0 deletions(-)
  

Comments

Thomas Monjalon Oct. 21, 2014, 10:51 a.m. UTC | #1
2014-10-21 16:46, Jijiang Liu:
> There are "some" destination UDP port numbers that have unque meaning.
> In terms of VxLAN, "IANA has assigned the value 4789 for the VXLAN UDP port, and this value
> SHOULD be used by default as the destination UDP port. Some early implementations of VXLAN
> have used other values for the destination port. To enable interoperability with these 
> implementations, the destination port SHOULD be configurable."
> 
> Add two APIs in librte_ether for supporting UDP tunneling port configuration on i40e.
> Currently, only VxLAN is implemented in this patch set.

Actually, there are 2 different things in this patch
- new tunnelling API
- VXLAN macros
Please split in 2 patches.

>  int
> +rte_eth_dev_udp_tunnel_add(uint8_t port_id,
> +			   struct rte_eth_udp_tunnel *udp_tunnel,
> +			   uint8_t count)
> +{
> +	uint8_t i;
> +	struct rte_eth_dev *dev;
> +	struct rte_eth_udp_tunnel *tunnel;
> +
> +	if (port_id >= nb_ports) {
> +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> +		return -ENODEV;
> +	}
> +
> +	if (udp_tunnel == NULL) {
> +		PMD_DEBUG_TRACE("Invalid udp_tunnel parameter\n");
> +		return -EINVAL;
> +	}
> +	tunnel = udp_tunnel;
> +
> +	for (i = 0; i < count; i++, tunnel++) {
> +		if (tunnel->prot_type >= RTE_TUNNEL_TYPE_MAX) {
> +			PMD_DEBUG_TRACE("Invalid tunnel type\n");
> +			return -EINVAL;
> +		}
> +	}

I'm not sure it's a good idea to provide a count parameter to iterate in a loop.
It's probably something that the application should do by itself.

> +
> +	dev = &rte_eth_devices[port_id];
> +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->udp_tunnel_add, -ENOTSUP);
> +	return (*dev->dev_ops->udp_tunnel_add)(dev, udp_tunnel, count);
> +}

[...]

> +/**
> + * Tunneled type.
> + */
> +enum rte_eth_tunnel_type {
> +	RTE_TUNNEL_TYPE_NONE = 0,
> +	RTE_TUNNEL_TYPE_VXLAN,
> +	RTE_TUNNEL_TYPE_GENEVE,
> +	RTE_TUNNEL_TYPE_TEREDO,
> +	RTE_TUNNEL_TYPE_NVGRE,
> +	RTE_TUNNEL_TYPE_MAX,
> +};

This is moved later from rte_ethdev.h to rte_eth_ctrl.h.
Please choose where is the right location in this patch.
By the way, I think ethdev is the right location because it's not directly
related to ctrl API, right?

>  struct rte_eth_conf {
> +	enum rte_eth_tunnel_type tunnel_type;
>  	uint16_t link_speed;
>  	/**< ETH_LINK_SPEED_10[0|00|000], or 0 for autonegotation */
>  	uint16_t link_duplex;

Please don't add this field as the first. It's more logical to start
port configuration with link speed, duplex, etc.
Then please add a comment to explain how it should be used.
But I doubt we should configure a tunnel type for a whole port.
There's something weird here.

> +/* VXLAN protocol header */

This comment should be doxygen'ed.

> +struct vxlan_hdr {
> +	uint32_t vx_flags; /**< VxLAN flag. */
> +	uint32_t vx_vni;   /**< VxLAN ID. */
> +} __attribute__((__packed__));
> +
[...]
>  #define ETHER_TYPE_VLAN 0x8100 /**< IEEE 802.1Q VLAN tagging. */
>  #define ETHER_TYPE_1588 0x88F7 /**< IEEE 802.1AS 1588 Precise Time Protocol. */
>  
> +#define ETHER_VXLAN_HLEN (sizeof(struct udp_hdr) + sizeof(struct vxlan_hdr))

Please add a doxygen comment.
  
Jijiang Liu Oct. 21, 2014, 1:48 p.m. UTC | #2
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, October 21, 2014 6:51 PM
> To: Liu, Jijiang
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet
> identification API in librte_ether
> 
> 2014-10-21 16:46, Jijiang Liu:
> > There are "some" destination UDP port numbers that have unque meaning.
> > In terms of VxLAN, "IANA has assigned the value 4789 for the VXLAN UDP
> > port, and this value SHOULD be used by default as the destination UDP
> > port. Some early implementations of VXLAN have used other values for
> > the destination port. To enable interoperability with these
> implementations, the destination port SHOULD be configurable."
> >
> > Add two APIs in librte_ether for supporting UDP tunneling port
> configuration on i40e.
> > Currently, only VxLAN is implemented in this patch set.
> 
> Actually, there are 2 different things in this patch
> - new tunnelling API
> - VXLAN macros
> Please split in 2 patches.
Ok
> >  int
> > +rte_eth_dev_udp_tunnel_add(uint8_t port_id,
> > +			   struct rte_eth_udp_tunnel *udp_tunnel,
> > +			   uint8_t count)
> > +{
> > +	uint8_t i;
> > +	struct rte_eth_dev *dev;
> > +	struct rte_eth_udp_tunnel *tunnel;
> > +
> > +	if (port_id >= nb_ports) {
> > +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > +		return -ENODEV;
> > +	}
> > +
> > +	if (udp_tunnel == NULL) {
> > +		PMD_DEBUG_TRACE("Invalid udp_tunnel parameter\n");
> > +		return -EINVAL;
> > +	}
> > +	tunnel = udp_tunnel;
> > +
> > +	for (i = 0; i < count; i++, tunnel++) {
> > +		if (tunnel->prot_type >= RTE_TUNNEL_TYPE_MAX) {
> > +			PMD_DEBUG_TRACE("Invalid tunnel type\n");
> > +			return -EINVAL;
> > +		}
> > +	}
> 
> I'm not sure it's a good idea to provide a count parameter to iterate in a
> loop.
> It's probably something that the application should do by itself.

It is necessary to check if this prot_type(tunnel type) is valid here in case applications don't do that. 

> > +
> > +	dev = &rte_eth_devices[port_id];
> > +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->udp_tunnel_add, -
> ENOTSUP);
> > +	return (*dev->dev_ops->udp_tunnel_add)(dev, udp_tunnel, count); }
> 
> [...]
> 
> > +/**
> > + * Tunneled type.
> > + */
> > +enum rte_eth_tunnel_type {
> > +	RTE_TUNNEL_TYPE_NONE = 0,
> > +	RTE_TUNNEL_TYPE_VXLAN,
> > +	RTE_TUNNEL_TYPE_GENEVE,
> > +	RTE_TUNNEL_TYPE_TEREDO,
> > +	RTE_TUNNEL_TYPE_NVGRE,
> > +	RTE_TUNNEL_TYPE_MAX,
> > +};
> 
> This is moved later from rte_ethdev.h to rte_eth_ctrl.h.
> Please choose where is the right location in this patch.
> By the way, I think ethdev is the right location because it's not directly
> related to ctrl API, right?

It is used in both rte_ethdev.h file and rte_eth_ctrl.h file.  
In rte_eth_ctrl.h file, it is used in rte_eth_tunnel_filter_conf structure.
+struct rte_eth_tunnel_filter_conf {
+	...
+	enum rte_eth_tunnel_type tunnel_type; /**< Tunnel Type. */
+	...
+};

I will put the rte_eth_tunnel_type definition in the rte_eth_ctrl.h file at the beginning of definition.


> >  struct rte_eth_conf {
> > +	enum rte_eth_tunnel_type tunnel_type;
> >  	uint16_t link_speed;
> >  	/**< ETH_LINK_SPEED_10[0|00|000], or 0 for autonegotation */
> >  	uint16_t link_duplex;
> 
> Please don't add this field as the first. It's more logical to start port
> configuration with link speed, duplex, etc.

ok
> Then please add a comment to explain how it should be used.
I will add more comment for it.
> But I doubt we should configure a tunnel type for a whole port.
Yes, your understanding is correct. It is for a whole port/PF, that's why we should add 
tunnel_type in  rte_eth_conf structure.

> There's something weird here.


> > +/* VXLAN protocol header */
> 
> This comment should be doxygen'ed.
> 
> > +struct vxlan_hdr {
> > +	uint32_t vx_flags; /**< VxLAN flag. */
> > +	uint32_t vx_vni;   /**< VxLAN ID. */
> > +} __attribute__((__packed__));
> > +
> [...]
> >  #define ETHER_TYPE_VLAN 0x8100 /**< IEEE 802.1Q VLAN tagging. */
> > #define ETHER_TYPE_1588 0x88F7 /**< IEEE 802.1AS 1588 Precise Time
> > Protocol. */
> >
> > +#define ETHER_VXLAN_HLEN (sizeof(struct udp_hdr) + sizeof(struct
> > +vxlan_hdr))
> 
> Please add a doxygen comment.
ok
> --
> Thomas
  
Thomas Monjalon Oct. 21, 2014, 9:19 p.m. UTC | #3
2014-10-21 13:48, Liu, Jijiang:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2014-10-21 16:46, Jijiang Liu:
> > >  int
> > > +rte_eth_dev_udp_tunnel_add(uint8_t port_id,
> > > +			   struct rte_eth_udp_tunnel *udp_tunnel,
> > > +			   uint8_t count)
> > > +{
> > > +	uint8_t i;
> > > +	struct rte_eth_dev *dev;
> > > +	struct rte_eth_udp_tunnel *tunnel;
> > > +
> > > +	if (port_id >= nb_ports) {
> > > +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > > +		return -ENODEV;
> > > +	}
> > > +
> > > +	if (udp_tunnel == NULL) {
> > > +		PMD_DEBUG_TRACE("Invalid udp_tunnel parameter\n");
> > > +		return -EINVAL;
> > > +	}
> > > +	tunnel = udp_tunnel;
> > > +
> > > +	for (i = 0; i < count; i++, tunnel++) {
> > > +		if (tunnel->prot_type >= RTE_TUNNEL_TYPE_MAX) {
> > > +			PMD_DEBUG_TRACE("Invalid tunnel type\n");
> > > +			return -EINVAL;
> > > +		}
> > > +	}
> > 
> > I'm not sure it's a good idea to provide a count parameter to iterate in a
> > loop.
> > It's probably something that the application should do by itself.
> 
> It is necessary to check if this prot_type(tunnel type) is valid here in case
> applications don't do that. 

Yes, you have to check prot_type but looping for several tunnels is not needed
at this level.

> > But I doubt we should configure a tunnel type for a whole port.
> 
> Yes, your understanding is correct. It is for a whole port/PF, that's why we
> should add tunnel_type in rte_eth_conf structure.

Please explain me why a tunnel type should be associated to a port.
This design looks really broken.

Thanks
  
Jijiang Liu Oct. 22, 2014, 1:46 a.m. UTC | #4
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, October 22, 2014 5:19 AM
> To: Liu, Jijiang
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet
> identification API in librte_ether
> 
> 2014-10-21 13:48, Liu, Jijiang:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-10-21 16:46, Jijiang Liu:
> > > >  int
> > > > +rte_eth_dev_udp_tunnel_add(uint8_t port_id,
> > > > +			   struct rte_eth_udp_tunnel *udp_tunnel,
> > > > +			   uint8_t count)
> > > > +{
> > > > +	uint8_t i;
> > > > +	struct rte_eth_dev *dev;
> > > > +	struct rte_eth_udp_tunnel *tunnel;
> > > > +
> > > > +	if (port_id >= nb_ports) {
> > > > +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > > > +		return -ENODEV;
> > > > +	}
> > > > +
> > > > +	if (udp_tunnel == NULL) {
> > > > +		PMD_DEBUG_TRACE("Invalid udp_tunnel parameter\n");
> > > > +		return -EINVAL;
> > > > +	}
> > > > +	tunnel = udp_tunnel;
> > > > +
> > > > +	for (i = 0; i < count; i++, tunnel++) {
> > > > +		if (tunnel->prot_type >= RTE_TUNNEL_TYPE_MAX) {
> > > > +			PMD_DEBUG_TRACE("Invalid tunnel type\n");
> > > > +			return -EINVAL;
> > > > +		}
> > > > +	}
> > >
> > > I'm not sure it's a good idea to provide a count parameter to
> > > iterate in a loop.
> > > It's probably something that the application should do by itself.
> >
> > It is necessary to check if this prot_type(tunnel type) is valid here
> > in case applications don't do that.
> 
> Yes, you have to check prot_type but looping for several tunnels is not
> needed at this level.
> 
> > > But I doubt we should configure a tunnel type for a whole port.
> >
> > Yes, your understanding is correct. It is for a whole port/PF, that's
> > why we should add tunnel_type in rte_eth_conf structure.
> 
> Please explain me why a tunnel type should be associated to a port.
> This design looks really broken.

I don't think this design looks really broken.

Currently, A PF  associated to a port, right? What tunnel type should be supported in a PF, which is required we configure it.
Tunneling packet is encapsulation packet, in terms of VxLAN, packet format is outer L2 header+ outer L3 header +outer L4 header + tunneling header+ inner L2 header + inner L3 header + inner L4 header +PAY4.
For a VM/VF, the  real useful packet data is "inner L2 header + inner L3 header + inner L4 header +PAY4".  

In NIC, A port/PF receive this kind of tunneling packet(outer L2+...PAY4),  software should be responsible for decapsulating the packet and deliver real data(innerL2 + PAY4) to VM/VF。

DPDK just provide API/mechanism to guarantee a PF/port to receive the tunneling packet data, the encapsulation/ decapsulation work should be done by user application.

Normally, the tunneling packet processing like below:
Tunneling packet ------>PF processing/receive ---------> application SW do decapsulation -------> VF/VM processing

> Thanks
> --
> Thomas
  
Jijiang Liu Oct. 22, 2014, 5:21 a.m. UTC | #5
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, October 22, 2014 5:19 AM
> To: Liu, Jijiang
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet
> identification API in librte_ether
> 
> 2014-10-21 13:48, Liu, Jijiang:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-10-21 16:46, Jijiang Liu:
> > > >  int
> > > > +rte_eth_dev_udp_tunnel_add(uint8_t port_id,
> > > > +			   struct rte_eth_udp_tunnel *udp_tunnel,
> > > > +			   uint8_t count)
> > > > +{
> > > > +	uint8_t i;
> > > > +	struct rte_eth_dev *dev;
> > > > +	struct rte_eth_udp_tunnel *tunnel;
> > > > +
> > > > +	if (port_id >= nb_ports) {
> > > > +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > > > +		return -ENODEV;
> > > > +	}
> > > > +
> > > > +	if (udp_tunnel == NULL) {
> > > > +		PMD_DEBUG_TRACE("Invalid udp_tunnel parameter\n");
> > > > +		return -EINVAL;
> > > > +	}
> > > > +	tunnel = udp_tunnel;
> > > > +
> > > > +	for (i = 0; i < count; i++, tunnel++) {
> > > > +		if (tunnel->prot_type >= RTE_TUNNEL_TYPE_MAX) {
> > > > +			PMD_DEBUG_TRACE("Invalid tunnel type\n");
> > > > +			return -EINVAL;
> > > > +		}
> > > > +	}
> > >
> > > I'm not sure it's a good idea to provide a count parameter to
> > > iterate in a loop.
> > > It's probably something that the application should do by itself.
> >
> > It is necessary to check if this prot_type(tunnel type) is valid here
> > in case applications don't do that.
> 
> Yes, you have to check prot_type but looping for several tunnels is not
> needed at this level.

Ok, remove loop check from here.
 
> Thanks
> --
> Thomas
  
Thomas Monjalon Oct. 22, 2014, 9:52 a.m. UTC | #6
2014-10-22 01:46, Liu, Jijiang:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2014-10-21 13:48, Liu, Jijiang:
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > But I doubt we should configure a tunnel type for a whole port.
> > >
> > > Yes, your understanding is correct. It is for a whole port/PF, that's
> > > why we should add tunnel_type in rte_eth_conf structure.
> > 
> > Please explain me why a tunnel type should be associated to a port.
> > This design looks really broken.
> 
> I don't think this design looks really broken.
> 
> Currently, A PF  associated to a port, right? What tunnel type should
> be supported in a PF, which is required we configure it.
> Tunneling packet is encapsulation packet, in terms of VxLAN, packet format
> is outer L2 header+ outer L3 header +outer L4 header + tunneling header+
> inner L2 header + inner L3 header + inner L4 header +PAY4.
> For a VM/VF, the  real useful packet data is "inner L2 header +
> inner L3 header + inner L4 header +PAY4".  
> 
> In NIC, A port/PF receive this kind of tunneling packet(outer L2+...PAY4),
> software should be responsible for decapsulating the packet and deliver
> real data(innerL2 + PAY4) to VM/VF。
> 
> DPDK just provide API/mechanism to guarantee a PF/port to receive the
> tunneling packet data, the encapsulation/ decapsulation work should be
> done by user application.

You mean that all packets received on the PF which doesn't match the configured
tunnel type, won't be received by the software?

Other question, why a port is associated with only one tunnel type?

> Normally, the tunneling packet processing like below:
> Tunneling packet ------>PF processing/receive ---------> application SW do decapsulation -------> VF/VM processing

I really try to understand what you have in mind. Thanks for explaining
  
Jijiang Liu Oct. 22, 2014, 12:47 p.m. UTC | #7
> -----Original Message-----

> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]

> Sent: Wednesday, October 22, 2014 5:52 PM

> To: Liu, Jijiang

> Cc: dev@dpdk.org

> Subject: Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet

> identification API in librte_ether

> 

> 2014-10-22 01:46, Liu, Jijiang:

> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]

> > > 2014-10-21 13:48, Liu, Jijiang:

> > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]

> > > > > But I doubt we should configure a tunnel type for a whole port.

> > > >

> > > > Yes, your understanding is correct. It is for a whole port/PF,

> > > > that's why we should add tunnel_type in rte_eth_conf structure.

> > >

> > > Please explain me why a tunnel type should be associated to a port.

> > > This design looks really broken.

> >

> > I don't think this design looks really broken.

> >

> > Currently, A PF  associated to a port, right? What tunnel type should

> > be supported in a PF, which is required we configure it.

> > Tunneling packet is encapsulation packet, in terms of VxLAN, packet

> > format is outer L2 header+ outer L3 header +outer L4 header +

> > tunneling header+ inner L2 header + inner L3 header + inner L4 header

> +PAY4.

> > For a VM/VF, the  real useful packet data is "inner L2 header + inner

> > L3 header + inner L4 header +PAY4".

> >

> > In NIC, A port/PF receive this kind of tunneling packet(outer

> > L2+...PAY4), software should be responsible for decapsulating the

> > packet and deliver real data(innerL2 + PAY4) to VM/VF。

> >

> > DPDK just provide API/mechanism to guarantee a PF/port to receive the

> > tunneling packet data, the encapsulation/ decapsulation work should be

> > done by user application.

> 

> You mean that all packets received on the PF which doesn't match the

> configured tunnel type, won't be received by the software?


No, it will be received by the software.
In terms of VxLAN packet, if tunnel type is not configured VXLAN type, and software can't use API to configure the UDP destination number. 
Even if the incoming packet format is VXLAN packet format, hardware and software don't think it is VXLAN packet because we didn't configure UDP Destination port. 

Now I want to remove this limitation,  even if the  tunnel type is not configured at PF initialization phase, user also can configure the VxLAN UDP destination number.
It is more flexible and reasonable.

> Other question, why a port is associated with only one tunnel type?


Good question. Now I think we had better remove this limitation because it is NIC related.

Two points are summarized here.
1. The tunnel types is for a whole port/PF, I have explained it a lots.
2. I will remove tunnel type configuration from rte_eth_conf structure.

Any comments?

> > Normally, the tunneling packet processing like below:

> > Tunneling packet ------>PF processing/receive ---------> application

> > SW do decapsulation -------> VF/VM processing

> 

> I really try to understand what you have in mind. Thanks for explaining

> --

> Thomas
  
Thomas Monjalon Oct. 22, 2014, 1:25 p.m. UTC | #8
2014-10-22 12:47, Liu, Jijiang:
> > > Currently, A PF  associated to a port, right? What tunnel type should
> > > be supported in a PF, which is required we configure it.
> > > Tunneling packet is encapsulation packet, in terms of VxLAN, packet
> > > format is outer L2 header+ outer L3 header +outer L4 header +
> > > tunneling header+ inner L2 header + inner L3 header + inner L4 header +PAY4.
> > > For a VM/VF, the  real useful packet data is "inner L2 header + inner
> > > L3 header + inner L4 header +PAY4".
> > >
> > > In NIC, A port/PF receive this kind of tunneling packet(outer
> > > L2+...PAY4), software should be responsible for decapsulating the
> > > packet and deliver real data(innerL2 + PAY4) to VM/VF。
> > >
> > > DPDK just provide API/mechanism to guarantee a PF/port to receive the
> > > tunneling packet data, the encapsulation/ decapsulation work should be
> > > done by user application.
> > 
> > You mean that all packets received on the PF which doesn't match the
> > configured tunnel type, won't be received by the software?
> 
> No, it will be received by the software.
> In terms of VxLAN packet, if tunnel type is not configured VXLAN type,
> and software can't use API to configure the UDP destination number. 
> Even if the incoming packet format is VXLAN packet format, hardware and
> software don't think it is VXLAN packet because we didn't configure UDP
> Destination port. 
> 
> Now I want to remove this limitation,  even if the  tunnel type is not
> configured at PF initialization phase, user also can configure the VxLAN
> UDP destination number.
> It is more flexible and reasonable.
> 
> > Other question, why a port is associated with only one tunnel type?
> 
> Good question. Now I think we had better remove this limitation because it is NIC related.
> 
> Two points are summarized here.
> 1. The tunnel types is for a whole port/PF, I have explained it a lots.
> 2. I will remove tunnel type configuration from rte_eth_conf structure.
> 
> Any comments?

Honestly, I haven't understood your explanation :)
I just understood that you want to remove tunnel type from rte_eth_conf
and I think it's a really good thing.

Thanks
  

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 50f10d9..9e111b6 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -2038,6 +2038,69 @@  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 *udp_tunnel,
+			   uint8_t count)
+{
+	uint8_t i;
+	struct rte_eth_dev *dev;
+	struct rte_eth_udp_tunnel *tunnel;
+
+	if (port_id >= nb_ports) {
+		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
+		return -ENODEV;
+	}
+
+	if (udp_tunnel == NULL) {
+		PMD_DEBUG_TRACE("Invalid udp_tunnel parameter\n");
+		return -EINVAL;
+	}
+	tunnel = udp_tunnel;
+
+	for (i = 0; i < count; i++, tunnel++) {
+		if (tunnel->prot_type >= RTE_TUNNEL_TYPE_MAX) {
+			PMD_DEBUG_TRACE("Invalid tunnel type\n");
+			return -EINVAL;
+		}
+	}
+
+	dev = &rte_eth_devices[port_id];
+	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->udp_tunnel_add, -ENOTSUP);
+	return (*dev->dev_ops->udp_tunnel_add)(dev, udp_tunnel, count);
+}
+
+int
+rte_eth_dev_udp_tunnel_delete(uint8_t port_id,
+			      struct rte_eth_udp_tunnel *udp_tunnel,
+			      uint8_t count)
+{
+	uint8_t i;
+	struct rte_eth_dev *dev;
+	struct rte_eth_udp_tunnel *tunnel;
+
+	if (port_id >= nb_ports) {
+		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
+		return -ENODEV;
+	}
+	dev = &rte_eth_devices[port_id];
+
+	if (udp_tunnel == NULL) {
+		PMD_DEBUG_TRACE("Invalid udp_tunnel parametr\n");
+		return -EINVAL;
+	}
+	tunnel = udp_tunnel;
+	for (i = 0; i < count; i++, tunnel++) {
+		if (tunnel->prot_type >= RTE_TUNNEL_TYPE_MAX) {
+			PMD_DEBUG_TRACE("Invalid tunnel type\n");
+			return -EINVAL;
+		}
+	}
+
+	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->udp_tunnel_del, -ENOTSUP);
+	return (*dev->dev_ops->udp_tunnel_del)(dev, udp_tunnel, count);
+}
+
+int
 rte_eth_led_on(uint8_t port_id)
 {
 	struct rte_eth_dev *dev;
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index b69a6af..9ad11ec 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -710,6 +710,26 @@  struct rte_fdir_conf {
 };
 
 /**
+ * UDP tunneling configuration.
+ */
+struct rte_eth_udp_tunnel {
+	uint16_t udp_port;
+	uint8_t prot_type;
+};
+
+/**
+ * Tunneled type.
+ */
+enum rte_eth_tunnel_type {
+	RTE_TUNNEL_TYPE_NONE = 0,
+	RTE_TUNNEL_TYPE_VXLAN,
+	RTE_TUNNEL_TYPE_GENEVE,
+	RTE_TUNNEL_TYPE_TEREDO,
+	RTE_TUNNEL_TYPE_NVGRE,
+	RTE_TUNNEL_TYPE_MAX,
+};
+
+/**
  *  Possible l4type of FDIR filters.
  */
 enum rte_l4type {
@@ -831,6 +851,7 @@  struct rte_intr_conf {
  * configuration settings may be needed.
  */
 struct rte_eth_conf {
+	enum rte_eth_tunnel_type tunnel_type;
 	uint16_t link_speed;
 	/**< ETH_LINK_SPEED_10[0|00|000], or 0 for autonegotation */
 	uint16_t link_duplex;
@@ -1266,6 +1287,17 @@  typedef int (*eth_mirror_rule_reset_t)(struct rte_eth_dev *dev,
 				  uint8_t rule_id);
 /**< @internal Remove a traffic mirroring rule on an Ethernet device */
 
+typedef int (*eth_udp_tunnel_add_t)(struct rte_eth_dev *dev,
+				    struct rte_eth_udp_tunnel *tunnel_udp,
+				    uint8_t count);
+/**< @internal Add tunneling UDP info */
+
+typedef int (*eth_udp_tunnel_del_t)(struct rte_eth_dev *dev,
+				    struct rte_eth_udp_tunnel *tunnel_udp,
+				    uint8_t count);
+/**< @internal Delete tunneling UDP info */
+
+
 #ifdef RTE_NIC_BYPASS
 
 enum {
@@ -1446,6 +1478,8 @@  struct eth_dev_ops {
 	eth_set_vf_rx_t            set_vf_rx;  /**< enable/disable a VF receive */
 	eth_set_vf_tx_t            set_vf_tx;  /**< enable/disable a VF transmit */
 	eth_set_vf_vlan_filter_t   set_vf_vlan_filter;  /**< Set VF VLAN filter */
+	eth_udp_tunnel_add_t       udp_tunnel_add;
+	eth_udp_tunnel_del_t       udp_tunnel_del;
 	eth_set_queue_rate_limit_t set_queue_rate_limit;   /**< Set queue rate limit */
 	eth_set_vf_rate_limit_t    set_vf_rate_limit;   /**< Set VF rate limit */
 
@@ -3342,6 +3376,47 @@  int
 rte_eth_dev_rss_hash_conf_get(uint8_t port_id,
 			      struct rte_eth_rss_conf *rss_conf);
 
+ /**
+ * Add UDP tunneling port of an Ethernet device for filtering a specific
+ * tunneling packet by UDP port number.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param tunnel_udp
+ *   UDP tunneling configuration.
+ * @param count
+ *   configuration count.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if port identifier is invalid.
+ *   - (-ENOTSUP) if hardware doesn't support tunnel type.
+ */
+int
+rte_eth_dev_udp_tunnel_add(uint8_t port_id,
+			   struct rte_eth_udp_tunnel *tunnel_udp,
+			   uint8_t count);
+
+ /**
+ * Detete UDP tunneling port configuration of Ethernet device
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param tunnel_udp
+ *   UDP tunneling configuration.
+ * @param count
+ *   configuration count.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if port identifier is invalid.
+ *   - (-ENOTSUP) if hardware doesn't support tunnel type.
+ */
+int
+rte_eth_dev_udp_tunnel_delete(uint8_t port_id,
+			      struct rte_eth_udp_tunnel *tunnel_udp,
+			      uint8_t count);
+
 /**
  * add syn filter
  *
diff --git a/lib/librte_ether/rte_ether.h b/lib/librte_ether/rte_ether.h
index 2e08f23..ddbdbb3 100644
--- a/lib/librte_ether/rte_ether.h
+++ b/lib/librte_ether/rte_ether.h
@@ -286,6 +286,12 @@  struct vlan_hdr {
 	uint16_t eth_proto;/**< Ethernet type of encapsulated frame. */
 } __attribute__((__packed__));
 
+/* VXLAN protocol header */
+struct vxlan_hdr {
+	uint32_t vx_flags; /**< VxLAN flag. */
+	uint32_t vx_vni;   /**< VxLAN ID. */
+} __attribute__((__packed__));
+
 /* Ethernet frame types */
 #define ETHER_TYPE_IPv4 0x0800 /**< IPv4 Protocol. */
 #define ETHER_TYPE_IPv6 0x86DD /**< IPv6 Protocol. */
@@ -294,6 +300,8 @@  struct vlan_hdr {
 #define ETHER_TYPE_VLAN 0x8100 /**< IEEE 802.1Q VLAN tagging. */
 #define ETHER_TYPE_1588 0x88F7 /**< IEEE 802.1AS 1588 Precise Time Protocol. */
 
+#define ETHER_VXLAN_HLEN (sizeof(struct udp_hdr) + sizeof(struct vxlan_hdr))
+
 #ifdef __cplusplus
 }
 #endif