app/testpmd: add parsing for multiple VLAN headers

  When having multiple VLANs in the packet, parse_ethernet
is cabable of parsing only the first vlan.

add parsing for mutliple VLAN headers in the packet.

Fixes: 51f694dd40f5 ("app/testpmd: rework checksum forward engine")
Cc: stable@dpdk.org

Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
---
 app/test-pmd/csumonly.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
  

> -----Original Message-----
> From: Raslan Darawsheh <rasland@mellanox.com>
> Sent: Monday, April 20, 2020 4:32 PM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: [PATCH] app/testpmd: add parsing for multiple VLAN headers
> 
> When having multiple VLANs in the packet, parse_ethernet is cabable of
> parsing only the first vlan.
> 
> add parsing for mutliple VLAN headers in the packet.
> 
> Fixes: 51f694dd40f5 ("app/testpmd: rework checksum forward engine")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
  

On 4/20/2020 4:32 PM, Raslan Darawsheh wrote:
> When having multiple VLANs in the packet, parse_ethernet
> is cabable of parsing only the first vlan.
> 
> add parsing for mutliple VLAN headers in the packet.
> 
> Fixes: 51f694dd40f5 ("app/testpmd: rework checksum forward engine")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> ---
>  app/test-pmd/csumonly.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> index fe19615..b0665f7 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -139,22 +139,22 @@ parse_ipv6(struct rte_ipv6_hdr *ipv6_hdr, struct testpmd_offload_info *info)
>  
>  /*
>   * Parse an ethernet header to fill the ethertype, l2_len, l3_len and
> - * ipproto. This function is able to recognize IPv4/IPv6 with one optional vlan
> - * header. The l4_len argument is only set in case of TCP (useful for TSO).
> + * ipproto. This function is able to recognize IPv4/IPv6 with optional VLAN
> + * headers. The l4_len argument is only set in case of TCP (useful for TSO).
>   */
>  static void
>  parse_ethernet(struct rte_ether_hdr *eth_hdr, struct testpmd_offload_info *info)
>  {
>  	struct rte_ipv4_hdr *ipv4_hdr;
>  	struct rte_ipv6_hdr *ipv6_hdr;
> +	struct rte_vlan_hdr *vlan_hdr;
>  
>  	info->l2_len = sizeof(struct rte_ether_hdr);
>  	info->ethertype = eth_hdr->ether_type;
>  
> -	if (info->ethertype == _htons(RTE_ETHER_TYPE_VLAN)) {
> -		struct rte_vlan_hdr *vlan_hdr = (
> -			struct rte_vlan_hdr *)(eth_hdr + 1);
> -
> +	while (info->ethertype == _htons(RTE_ETHER_TYPE_VLAN)) {
> +		vlan_hdr = (struct rte_vlan_hdr *)
> +			((char *)eth_hdr + info->l2_len);
>  		info->l2_len  += sizeof(struct rte_vlan_hdr);
>  		info->ethertype = vlan_hdr->eth_proto;
>  	}
> 

Can an ethernet packet have multiple VLAN header, according IEEE 802.1Q there
can be only single VLAN header, if this is for QinQ will both TPID be same and
0x8100 (RTE_ETHER_TYPE_VLAN)?
  

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Raslan Darawsheh
> Sent: Monday, April 20, 2020 6:32 PM
> To: bernard.iremonger@intel.com; jingjing.wu@intel.com;
> wenzhuo.lu@intel.com
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH] app/testpmd: add parsing for multiple VLAN
> headers
> 
> When having multiple VLANs in the packet, parse_ethernet
> is cabable of parsing only the first vlan.
> 
> add parsing for mutliple VLAN headers in the packet.
> 
> Fixes: 51f694dd40f5 ("app/testpmd: rework checksum forward engine")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> ---
>  app/test-pmd/csumonly.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> index fe19615..b0665f7 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -139,22 +139,22 @@ parse_ipv6(struct rte_ipv6_hdr *ipv6_hdr, struct
> testpmd_offload_info *info)
> 
>  /*
>   * Parse an ethernet header to fill the ethertype, l2_len, l3_len and
> - * ipproto. This function is able to recognize IPv4/IPv6 with one optional vlan
> - * header. The l4_len argument is only set in case of TCP (useful for TSO).
> + * ipproto. This function is able to recognize IPv4/IPv6 with optional VLAN
> + * headers. The l4_len argument is only set in case of TCP (useful for TSO).
>   */
>  static void
>  parse_ethernet(struct rte_ether_hdr *eth_hdr, struct testpmd_offload_info
> *info)
>  {
>  	struct rte_ipv4_hdr *ipv4_hdr;
>  	struct rte_ipv6_hdr *ipv6_hdr;
> +	struct rte_vlan_hdr *vlan_hdr;
> 
>  	info->l2_len = sizeof(struct rte_ether_hdr);
>  	info->ethertype = eth_hdr->ether_type;
> 
> -	if (info->ethertype == _htons(RTE_ETHER_TYPE_VLAN)) {
> -		struct rte_vlan_hdr *vlan_hdr = (
> -			struct rte_vlan_hdr *)(eth_hdr + 1);
> -
> +	while (info->ethertype == _htons(RTE_ETHER_TYPE_VLAN)) {
> +		vlan_hdr = (struct rte_vlan_hdr *)
> +			((char *)eth_hdr + info->l2_len);
>  		info->l2_len  += sizeof(struct rte_vlan_hdr);
>  		info->ethertype = vlan_hdr->eth_proto;
>  	}
> --


Acked-by: Ori Kam <orika@mellanox.com>
Thanks,
Ori
> 2.7.4
  

Hi Ferruh, Raslan,

> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Tuesday, April 21, 2020 9:48 PM
> To: Raslan Darawsheh <rasland@mellanox.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: add parsing for multiple
> VLAN headers
> 
> On 4/20/2020 4:32 PM, Raslan Darawsheh wrote:
> > When having multiple VLANs in the packet, parse_ethernet is cabable of
> > parsing only the first vlan.
> >
> > add parsing for mutliple VLAN headers in the packet.
> >
> > Fixes: 51f694dd40f5 ("app/testpmd: rework checksum forward engine")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> > ---
> >  app/test-pmd/csumonly.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> > fe19615..b0665f7 100644
> > --- a/app/test-pmd/csumonly.c
> > +++ b/app/test-pmd/csumonly.c
> > @@ -139,22 +139,22 @@ parse_ipv6(struct rte_ipv6_hdr *ipv6_hdr, struct
> > testpmd_offload_info *info)
> >
> >  /*
> >   * Parse an ethernet header to fill the ethertype, l2_len, l3_len and
> > - * ipproto. This function is able to recognize IPv4/IPv6 with one
> > optional vlan
> > - * header. The l4_len argument is only set in case of TCP (useful for TSO).
> > + * ipproto. This function is able to recognize IPv4/IPv6 with
> > + optional VLAN
> > + * headers. The l4_len argument is only set in case of TCP (useful for TSO).
> >   */
> >  static void
> >  parse_ethernet(struct rte_ether_hdr *eth_hdr, struct
> > testpmd_offload_info *info)  {
> >  	struct rte_ipv4_hdr *ipv4_hdr;
> >  	struct rte_ipv6_hdr *ipv6_hdr;
> > +	struct rte_vlan_hdr *vlan_hdr;
> >
> >  	info->l2_len = sizeof(struct rte_ether_hdr);
> >  	info->ethertype = eth_hdr->ether_type;
> >
> > -	if (info->ethertype == _htons(RTE_ETHER_TYPE_VLAN)) {
> > -		struct rte_vlan_hdr *vlan_hdr = (
> > -			struct rte_vlan_hdr *)(eth_hdr + 1);
> > -
> > +	while (info->ethertype == _htons(RTE_ETHER_TYPE_VLAN)) {
> > +		vlan_hdr = (struct rte_vlan_hdr *)
> > +			((char *)eth_hdr + info->l2_len);
> >  		info->l2_len  += sizeof(struct rte_vlan_hdr);
> >  		info->ethertype = vlan_hdr->eth_proto;
> >  	}
> >
> 
> Can an ethernet packet have multiple VLAN header, according IEEE 802.1Q
> there can be only single VLAN header, if this is for QinQ will both TPID be
> same and
> 0x8100 (RTE_ETHER_TYPE_VLAN)?

There is also 0x88a8 (RTE_ETHER_TYPE_QINQ), IEEE 802.1ad QinQ tagging, both values should probably be checked.

Regards,

Bernard.
  

Hi,

> -----Original Message-----
> From: Iremonger, Bernard <bernard.iremonger@intel.com>
> Sent: Wednesday, April 22, 2020 2:05 PM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Raslan Darawsheh
> <rasland@mellanox.com>; Wu, Jingjing <jingjing.wu@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: add parsing for multiple VLAN
> headers
> 
> Hi Ferruh, Raslan,
> 
> > -----Original Message-----
> > From: Yigit, Ferruh <ferruh.yigit@intel.com>
> > Sent: Tuesday, April 21, 2020 9:48 PM
> > To: Raslan Darawsheh <rasland@mellanox.com>; Iremonger, Bernard
> > <bernard.iremonger@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Lu,
> > Wenzhuo <wenzhuo.lu@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: add parsing for multiple
> > VLAN headers
> >
> > On 4/20/2020 4:32 PM, Raslan Darawsheh wrote:
> > > When having multiple VLANs in the packet, parse_ethernet is cabable of
> > > parsing only the first vlan.
> > >
> > > add parsing for mutliple VLAN headers in the packet.
> > >
> > > Fixes: 51f694dd40f5 ("app/testpmd: rework checksum forward engine")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> > > ---
> > >  app/test-pmd/csumonly.c | 12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> > > fe19615..b0665f7 100644
> > > --- a/app/test-pmd/csumonly.c
> > > +++ b/app/test-pmd/csumonly.c
> > > @@ -139,22 +139,22 @@ parse_ipv6(struct rte_ipv6_hdr *ipv6_hdr,
> struct
> > > testpmd_offload_info *info)
> > >
> > >  /*
> > >   * Parse an ethernet header to fill the ethertype, l2_len, l3_len and
> > > - * ipproto. This function is able to recognize IPv4/IPv6 with one
> > > optional vlan
> > > - * header. The l4_len argument is only set in case of TCP (useful for TSO).
> > > + * ipproto. This function is able to recognize IPv4/IPv6 with
> > > + optional VLAN
> > > + * headers. The l4_len argument is only set in case of TCP (useful for
> TSO).
> > >   */
> > >  static void
> > >  parse_ethernet(struct rte_ether_hdr *eth_hdr, struct
> > > testpmd_offload_info *info)  {
> > >  	struct rte_ipv4_hdr *ipv4_hdr;
> > >  	struct rte_ipv6_hdr *ipv6_hdr;
> > > +	struct rte_vlan_hdr *vlan_hdr;
> > >
> > >  	info->l2_len = sizeof(struct rte_ether_hdr);
> > >  	info->ethertype = eth_hdr->ether_type;
> > >
> > > -	if (info->ethertype == _htons(RTE_ETHER_TYPE_VLAN)) {
> > > -		struct rte_vlan_hdr *vlan_hdr = (
> > > -			struct rte_vlan_hdr *)(eth_hdr + 1);
> > > -
> > > +	while (info->ethertype == _htons(RTE_ETHER_TYPE_VLAN)) {
> > > +		vlan_hdr = (struct rte_vlan_hdr *)
> > > +			((char *)eth_hdr + info->l2_len);
> > >  		info->l2_len  += sizeof(struct rte_vlan_hdr);
> > >  		info->ethertype = vlan_hdr->eth_proto;
> > >  	}
> > >
> >
> > Can an ethernet packet have multiple VLAN header, according IEEE 802.1Q
> > there can be only single VLAN header, if this is for QinQ will both TPID be
> > same and
> > 0x8100 (RTE_ETHER_TYPE_VLAN)?
@Yigit, Ferruh,
If we are talking about QinQ then the value that need to be checked is (RTE_ETHER_TYPE_QINQ) you are right,

> 
> There is also 0x88a8 (RTE_ETHER_TYPE_QINQ), IEEE 802.1ad QinQ tagging,
> both values should probably be checked.
Yes you are right, I'll update the patch to check for both of them, will send v2 shortly

> 
> Regards,
> 
> Bernard.

Kindest regards
Raslan Darawsheh