[dpdk-dev] [PATCH v2 2/7] lib/librte_ether: support l2 tunnel config

2016-02-03 Thread Stephen Hemminger
On Tue,  2 Feb 2016 14:57:00 +0800
Wenzhuo Lu  wrote:

> +/**
> + * l2 tunnel configuration.
> + */
> +struct rte_eth_l2_tunnel {
> + enum rte_eth_l2_tunnel_type l2_tunnel_type;
> + uint16_t ether_type;
> +};

Building an API with an indirect (structure) parameter with only two
elements seems like overkill. If you are building it to support future
features, that won't work because it will break ABI.

Why not just?

int
rte_eth_dev_etag_enable(uint8_t port_id, uint16_t ether_type)


[dpdk-dev] [PATCH v2 2/7] lib/librte_ether: support l2 tunnel config

2016-02-03 Thread Lu, Wenzhuo
Hi Stephen,

> -Original Message-
> From: Stephen Hemminger [mailto:stephen at networkplumber.org]
> Sent: Wednesday, February 3, 2016 11:36 AM
> To: Lu, Wenzhuo
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 2/7] lib/librte_ether: support l2 tunnel 
> config
> 
> On Tue,  2 Feb 2016 14:57:00 +0800
> Wenzhuo Lu  wrote:
> 
> > +/**
> > + * l2 tunnel configuration.
> > + */
> > +struct rte_eth_l2_tunnel {
> > +   enum rte_eth_l2_tunnel_type l2_tunnel_type;
> > +   uint16_t ether_type;
> > +};
> 
> Building an API with an indirect (structure) parameter with only two elements
> seems like overkill. If you are building it to support future features, that 
> won't
> work because it will break ABI.
> 
> Why not just?
> 
> int
> rte_eth_dev_etag_enable(uint8_t port_id, uint16_t ether_type)
I'm trying to make the rte ops can support future features, because there're 
some similar technology 802.1Qbg, 802.1Qbh, 802.1BR, VN-tag. 
And after applying all the patches in this patch set, the  struct 
rte_eth_l2_tunnel will be like this,
struct rte_eth_l2_tunnel {
enum rte_eth_l2_tunnel_type l2_tunnel_type;
uint16_t ether_type;
   uint32_t tunnel_id; /* port tag id for e-tag */
 };
If we want to support a new type of tunnel which can be described by ether type 
and tunnel id, we need not to extend this structure. We
only need to add new value for l2_tunnel_type. I think it's not an ABI change.
And I think using a l2 tunnel to cover all these tunnels can make interface 
simple :)



[dpdk-dev] [PATCH v2 2/7] lib/librte_ether: support l2 tunnel config

2016-02-03 Thread Lu, Wenzhuo
Hi Bruce,

> -Original Message-
> From: Richardson, Bruce
> Sent: Tuesday, February 2, 2016 8:03 PM
> To: Lu, Wenzhuo
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 2/7] lib/librte_ether: support l2 tunnel 
> config
> 
> On Tue, Feb 02, 2016 at 02:57:00PM +0800, Wenzhuo Lu wrote:
> > Add functions to support l2 tunnel configuration.
> > The support includes ether type modification and the tunnel support
> > enabling/disabling.
> > Ether type modification means modifying the ether type of a specific
> > type of tunnel. So the packet with this ether type will be parsed as
> > this type of tunnel.
> > Enabling/disabling a tunnel support means enabling/disabling the
> > ability of parsing the specific type of tunnel. This ability should be
> > enabled before we enable filtering, forwarding, offloading for this
> > specific type of tunnel.
> > Only support e-tag tunnel now.
> >
> > E-tag introduction,
> > E-tag means external tag which is defined in I 802.1BR
> > specification.
> > E-tag is a kind of l2 tunnel. It means a tag will be inserted in the
> > l2 header. Like below,
> >|3124|23   16|15 8|7   0|
> >   0|   Destination MAC address |
> >   4| Dest MAC address(cont.)| Src MAC address  |
> >   8|  Source MAC address(cont.)|
> >  12| E-tag Etherenet type (0x893f)  |  E-tag header|
> >  16|E-tag header(cont.)|
> >  20|   VLAN Ethertype(optional) |   VLAN header(optional)  |
> >  24| Original type  | ..   |
> > ...|  ..   |
> > The E-tag format is like below,
> >|015|16   18|19 |20   31|
> >|   Ethertype - 0x893f  | E-PCP |DEI|   Ingress E-CID_base  |
> >
> >|32  33|34 35|36  47|48 55|56 63|
> >|  RSV | GRP |E-CID_base|Ingress_E-CID_ext|E-CID_ext|
> >
> > The Ingess_E-CID_ext and E-CID_ext are always zero for endpoints and
> > are effectively reserved.
> >
> > The more details of E-tag is in IEEE 802.1BR. 802.1BR is used to
> > replace 802.1Qbh. 802.1BR is a standard for Bridge Port Extension.
> > It specifies the operation of Bridge Port Extenders, including
> > management, protocols, and algorithms. Bridge Port Extenders operate
> > in support of the MAC Service by Extended Bridges.
> > The E-tag is added to l2 header to identify the VM channel and the
> > virtual port.
> >
> > Signed-off-by: Wenzhuo Lu 
> 
> Your introduction to e-tag needs to be in patch 1 rather than 2, and in the 
> cover
> letter, since both those earlier mails reference it.
> Furthermore, it would be better if you could link to a web description of the 
> e-
> tag rather than try and explain it all in a commit message. Diagrams also work
> better on a web-page.
O, my bad. I thought the patch 1 doesn't have something to do with E-tag 
directly.
But I do mention E-tag in the patch V1.
Thanks for the comments. I'll create  a V3.

> 
> /Bruce



[dpdk-dev] [PATCH v2 2/7] lib/librte_ether: support l2 tunnel config

2016-02-02 Thread Wenzhuo Lu
Add functions to support l2 tunnel configuration.
The support includes ether type modification and the tunnel support
enabling/disabling.
Ether type modification means modifying the ether type of a specific
type of tunnel. So the packet with this ether type will be parsed as
this type of tunnel.
Enabling/disabling a tunnel support means enabling/disabling the
ability of parsing the specific type of tunnel. This ability should
be enabled before we enable filtering, forwarding, offloading for
this specific type of tunnel.
Only support e-tag tunnel now.

E-tag introduction,
E-tag means external tag which is defined in I 802.1BR
specification.
E-tag is a kind of l2 tunnel. It means a tag will be inserted in the
l2 header. Like below,
   |3124|23   16|15 8|7   0|
  0|   Destination MAC address |
  4| Dest MAC address(cont.)| Src MAC address  |
  8|  Source MAC address(cont.)|
 12| E-tag Etherenet type (0x893f)  |  E-tag header|
 16|E-tag header(cont.)|
 20|   VLAN Ethertype(optional) |   VLAN header(optional)  |
 24| Original type  | ..   |
...|  ..   |
The E-tag format is like below,
   |015|16   18|19 |20   31|
   |   Ethertype - 0x893f  | E-PCP |DEI|   Ingress E-CID_base  |

   |32  33|34 35|36  47|48 55|56 63|
   |  RSV | GRP |E-CID_base|Ingress_E-CID_ext|E-CID_ext|

The Ingess_E-CID_ext and E-CID_ext are always zero for endpoints
and are effectively reserved.

The more details of E-tag is in IEEE 802.1BR. 802.1BR is used to
replace 802.1Qbh. 802.1BR is a standard for Bridge Port Extension.
It specifies the operation of Bridge Port Extenders, including
management, protocols, and algorithms. Bridge Port Extenders
operate in support of the MAC Service by Extended Bridges.
The E-tag is added to l2 header to identify the VM channel and
the virtual port.

Signed-off-by: Wenzhuo Lu 
---
 lib/librte_ether/rte_eth_ctrl.h |  9 +
 lib/librte_ether/rte_ethdev.c   | 61 ++
 lib/librte_ether/rte_ethdev.h   | 84 +
 3 files changed, 154 insertions(+)

diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
index ce224ad..09af6fb 100644
--- a/lib/librte_ether/rte_eth_ctrl.h
+++ b/lib/librte_ether/rte_eth_ctrl.h
@@ -804,6 +804,15 @@ struct rte_eth_hash_filter_info {
} info;
 };

+/**
+ * l2 tunnel type.
+ */
+enum rte_eth_l2_tunnel_type {
+   RTE_L2_TUNNEL_TYPE_NONE = 0,
+   RTE_L2_TUNNEL_TYPE_E_TAG,
+   RTE_L2_TUNNEL_TYPE_MAX,
+};
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index ed971b4..1b90e09 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3239,3 +3239,64 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev, 
struct rte_pci_device *pci_de
eth_dev->data->numa_node = pci_dev->numa_node;
eth_dev->data->drv_name = pci_dev->driver->name;
 }
+
+int
+rte_eth_dev_l2_tunnel_eth_type_conf(uint8_t port_id,
+   struct rte_eth_l2_tunnel *l2_tunnel)
+{
+   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 = _eth_devices[port_id];
+   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->l2_tunnel_eth_type_conf,
+   -ENOTSUP);
+   return (*dev->dev_ops->l2_tunnel_eth_type_conf)(dev, l2_tunnel);
+}
+
+int
+rte_eth_dev_l2_tunnel_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 = _eth_devices[port_id];
+   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->l2_tunnel_enable,
+   -ENOTSUP);
+   return (*dev->dev_ops->l2_tunnel_enable)(dev, l2_tunnel_type);
+}
+
+int
+rte_eth_dev_l2_tunnel_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");
+   

[dpdk-dev] [PATCH v2 2/7] lib/librte_ether: support l2 tunnel config

2016-02-02 Thread Bruce Richardson
On Tue, Feb 02, 2016 at 02:57:00PM +0800, Wenzhuo Lu wrote:
> Add functions to support l2 tunnel configuration.
> The support includes ether type modification and the tunnel support
> enabling/disabling.
> Ether type modification means modifying the ether type of a specific
> type of tunnel. So the packet with this ether type will be parsed as
> this type of tunnel.
> Enabling/disabling a tunnel support means enabling/disabling the
> ability of parsing the specific type of tunnel. This ability should
> be enabled before we enable filtering, forwarding, offloading for
> this specific type of tunnel.
> Only support e-tag tunnel now.
> 
> E-tag introduction,
> E-tag means external tag which is defined in I 802.1BR
> specification.
> E-tag is a kind of l2 tunnel. It means a tag will be inserted in the
> l2 header. Like below,
>|3124|23   16|15 8|7   0|
>   0|   Destination MAC address |
>   4| Dest MAC address(cont.)| Src MAC address  |
>   8|  Source MAC address(cont.)|
>  12| E-tag Etherenet type (0x893f)  |  E-tag header|
>  16|E-tag header(cont.)|
>  20|   VLAN Ethertype(optional) |   VLAN header(optional)  |
>  24| Original type  | ..   |
> ...|  ..   |
> The E-tag format is like below,
>|015|16   18|19 |20   31|
>|   Ethertype - 0x893f  | E-PCP |DEI|   Ingress E-CID_base  |
> 
>|32  33|34 35|36  47|48 55|56 63|
>|  RSV | GRP |E-CID_base|Ingress_E-CID_ext|E-CID_ext|
> 
> The Ingess_E-CID_ext and E-CID_ext are always zero for endpoints
> and are effectively reserved.
> 
> The more details of E-tag is in IEEE 802.1BR. 802.1BR is used to
> replace 802.1Qbh. 802.1BR is a standard for Bridge Port Extension.
> It specifies the operation of Bridge Port Extenders, including
> management, protocols, and algorithms. Bridge Port Extenders
> operate in support of the MAC Service by Extended Bridges.
> The E-tag is added to l2 header to identify the VM channel and
> the virtual port.
> 
> Signed-off-by: Wenzhuo Lu 

Your introduction to e-tag needs to be in patch 1 rather than 2, and in the
cover letter, since both those earlier mails reference it.
Furthermore, it would be better if you could link to a web description of the
e-tag rather than try and explain it all in a commit message. Diagrams also work
better on a web-page.

/Bruce