patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH v2] doc: fix support table for ETH and VLAN flow items
@ 2022-10-13 10:48 Ilya Maximets
  2022-10-14  9:41 ` fengchengwen
  2022-10-26 15:34 ` Thomas Monjalon
  0 siblings, 2 replies; 8+ messages in thread
From: Ilya Maximets @ 2022-10-13 10:48 UTC (permalink / raw)
  To: dev, Thomas Monjalon
  Cc: Ajit Khaparde, Somnath Kotur, Rahul Lakkireddy, Hemant Agrawal,
	Sachin Saxena, Simei Su, Wenjun Wu, John Daley, Hyong Youb Kim,
	Ziyang Xuan, Xiaoyun Wang, Guoyang Zhou, Dongdong Liu,
	Yisen Zhuang, Yuying Zhang, Beilei Xing, Jingjing Wu,
	Qiming Yang, Qi Zhang, Junfeng Guo, Rosen Xu, Matan Azrad,
	Viacheslav Ovsiienko, Liron Himi, Jiawen Wu, Jian Wang,
	Dekel Peled, Ori Kam, Ilya Maximets, stable

'has_vlan' attribute is only supported by sfc, mlx5 and cnxk.
Other drivers doesn't support it.  Most of them (like i40e) just
ignore it silently.  Some drivers (like mlx4) never had a full
support of the eth item even before introduction of 'has_vlan'
(mlx4 allows to match on the destination MAC only).

Same for the 'has_more_vlan' flag of the vlan item.

'has_vlan' is part of 'rte_flow_item_eth', so changing 'eth'
field to 'partial support' in documentation for all such drivers.
'has_more_vlan' is part of 'rte_flow_item_vlan', so changing
'vlan' to 'partial support' as well.

This doesn't solve the issue, but at least marks the problematic
drivers.

Some details are available in:
  https://bugs.dpdk.org/show_bug.cgi?id=958

Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN items")
Cc: stable@dpdk.org

Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---

Version 2:
  - Rebased on a current main branch.
  - Added more clarifications to the commit message.

I added the stable in CC, but the patch should be extended while
backporting.  For 21.11 the cnxk driver should be also updated,
for 20.11, sfc driver should also be included.

 doc/guides/nics/features/bnxt.ini   | 4 ++--
 doc/guides/nics/features/cxgbe.ini  | 4 ++--
 doc/guides/nics/features/dpaa2.ini  | 4 ++--
 doc/guides/nics/features/e1000.ini  | 2 +-
 doc/guides/nics/features/enic.ini   | 4 ++--
 doc/guides/nics/features/hinic.ini  | 2 +-
 doc/guides/nics/features/hns3.ini   | 4 ++--
 doc/guides/nics/features/i40e.ini   | 4 ++--
 doc/guides/nics/features/iavf.ini   | 4 ++--
 doc/guides/nics/features/ice.ini    | 4 ++--
 doc/guides/nics/features/igc.ini    | 2 +-
 doc/guides/nics/features/ipn3ke.ini | 4 ++--
 doc/guides/nics/features/ixgbe.ini  | 4 ++--
 doc/guides/nics/features/mlx4.ini   | 4 ++--
 doc/guides/nics/features/mvpp2.ini  | 4 ++--
 doc/guides/nics/features/tap.ini    | 4 ++--
 doc/guides/nics/features/txgbe.ini  | 4 ++--
 17 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/doc/guides/nics/features/bnxt.ini b/doc/guides/nics/features/bnxt.ini
index b2d54f06aa..50a0b5bfa6 100644
--- a/doc/guides/nics/features/bnxt.ini
+++ b/doc/guides/nics/features/bnxt.ini
@@ -57,7 +57,7 @@ Perf doc             = Y
 
 [rte_flow items]
 any                  = Y
-eth                  = Y
+eth                  = P
 ipv4                 = Y
 ipv6                 = Y
 gre                  = Y
@@ -68,7 +68,7 @@ port_representor     = Y
 represented_port     = Y
 tcp                  = Y
 udp                  = Y
-vlan                 = Y
+vlan                 = P
 vxlan                = Y
 
 [rte_flow actions]
diff --git a/doc/guides/nics/features/cxgbe.ini b/doc/guides/nics/features/cxgbe.ini
index a9dbcd0573..0d67ca8720 100644
--- a/doc/guides/nics/features/cxgbe.ini
+++ b/doc/guides/nics/features/cxgbe.ini
@@ -36,12 +36,12 @@ x86-64               = Y
 Usage doc            = Y
 
 [rte_flow items]
-eth                  = Y
+eth                  = P
 ipv4                 = Y
 ipv6                 = Y
 tcp                  = Y
 udp                  = Y
-vlan                 = Y
+vlan                 = P
 
 [rte_flow actions]
 count                = Y
diff --git a/doc/guides/nics/features/dpaa2.ini b/doc/guides/nics/features/dpaa2.ini
index cedc234f26..26dc8c2178 100644
--- a/doc/guides/nics/features/dpaa2.ini
+++ b/doc/guides/nics/features/dpaa2.ini
@@ -31,7 +31,7 @@ ARMv8                = Y
 Usage doc            = Y
 
 [rte_flow items]
-eth                  = Y
+eth                  = P
 gre                  = Y
 icmp                 = Y
 ipv4                 = Y
@@ -41,7 +41,7 @@ raw                  = Y
 sctp                 = Y
 tcp                  = Y
 udp                  = Y
-vlan                 = Y
+vlan                 = P
 
 [rte_flow actions]
 drop                 = Y
diff --git a/doc/guides/nics/features/e1000.ini b/doc/guides/nics/features/e1000.ini
index e4bdef6da9..a9cbed1c3c 100644
--- a/doc/guides/nics/features/e1000.ini
+++ b/doc/guides/nics/features/e1000.ini
@@ -31,7 +31,7 @@ x86-32               = Y
 x86-64               = Y
 
 [rte_flow items]
-eth                  = Y
+eth                  = P
 ipv4                 = Y
 ipv6                 = Y
 raw                  = Y
diff --git a/doc/guides/nics/features/enic.ini b/doc/guides/nics/features/enic.ini
index 61bec4910e..6dbea9f36e 100644
--- a/doc/guides/nics/features/enic.ini
+++ b/doc/guides/nics/features/enic.ini
@@ -40,7 +40,7 @@ Usage doc            = Y
 
 [rte_flow items]
 ecpri                = Y
-eth                  = Y
+eth                  = P
 geneve               = Y
 geneve_opt           = Y
 gtp                  = Y
@@ -52,7 +52,7 @@ raw                  = Y
 sctp                 = Y
 tcp                  = Y
 udp                  = Y
-vlan                 = Y
+vlan                 = P
 vxlan                = Y
 
 [rte_flow actions]
diff --git a/doc/guides/nics/features/hinic.ini b/doc/guides/nics/features/hinic.ini
index 9f6f0ebf3a..ada6607fe9 100644
--- a/doc/guides/nics/features/hinic.ini
+++ b/doc/guides/nics/features/hinic.ini
@@ -40,7 +40,7 @@ ARMv8                = Y
 
 [rte_flow items]
 any                  = Y
-eth                  = Y
+eth                  = P
 icmp                 = Y
 icmp6                = Y
 ipv4                 = Y
diff --git a/doc/guides/nics/features/hns3.ini b/doc/guides/nics/features/hns3.ini
index 405b94f05c..338b4e6864 100644
--- a/doc/guides/nics/features/hns3.ini
+++ b/doc/guides/nics/features/hns3.ini
@@ -51,7 +51,7 @@ Linux                = Y
 ARMv8                = Y
 
 [rte_flow items]
-eth                  = Y
+eth                  = P
 geneve               = Y
 icmp                 = Y
 ipv4                 = Y
@@ -60,7 +60,7 @@ nvgre                = Y
 sctp                 = Y
 tcp                  = Y
 udp                  = Y
-vlan                 = Y
+vlan                 = P
 vxlan                = Y
 vxlan_gpe            = Y
 
diff --git a/doc/guides/nics/features/i40e.ini b/doc/guides/nics/features/i40e.ini
index 95e39aaba0..e241dad047 100644
--- a/doc/guides/nics/features/i40e.ini
+++ b/doc/guides/nics/features/i40e.ini
@@ -54,7 +54,7 @@ Power8               = Y
 [rte_flow items]
 ah                   = Y
 esp                  = Y
-eth                  = Y
+eth                  = P
 gre                  = Y
 gtpc                 = Y
 gtpu                 = Y
@@ -68,7 +68,7 @@ raw                  = Y
 sctp                 = Y
 tcp                  = Y
 udp                  = Y
-vlan                 = Y
+vlan                 = P
 vxlan                = Y
 
 [rte_flow actions]
diff --git a/doc/guides/nics/features/iavf.ini b/doc/guides/nics/features/iavf.ini
index eeda6b7210..9db2865b71 100644
--- a/doc/guides/nics/features/iavf.ini
+++ b/doc/guides/nics/features/iavf.ini
@@ -43,7 +43,7 @@ ah                   = Y
 arp_eth_ipv4         = Y
 ecpri                = Y
 esp                  = Y
-eth                  = Y
+eth                  = P
 gre                  = Y
 gtpc                 = Y
 gtpu                 = Y
@@ -61,7 +61,7 @@ raw                  = Y
 sctp                 = Y
 tcp                  = Y
 udp                  = Y
-vlan                 = Y
+vlan                 = P
 
 [rte_flow actions]
 count                = Y
diff --git a/doc/guides/nics/features/ice.ini b/doc/guides/nics/features/ice.ini
index 032da8e2e2..13f8871dcc 100644
--- a/doc/guides/nics/features/ice.ini
+++ b/doc/guides/nics/features/ice.ini
@@ -55,7 +55,7 @@ ah                   = Y
 any                  = Y
 arp_eth_ipv4         = Y
 esp                  = Y
-eth                  = Y
+eth                  = P
 gtpu                 = Y
 gtp_psc              = Y
 icmp                 = Y
@@ -73,7 +73,7 @@ raw                  = Y
 sctp                 = Y
 tcp                  = Y
 udp                  = Y
-vlan                 = Y
+vlan                 = P
 vxlan                = Y
 
 [rte_flow actions]
diff --git a/doc/guides/nics/features/igc.ini b/doc/guides/nics/features/igc.ini
index f2c6fa28ad..b5deea3f61 100644
--- a/doc/guides/nics/features/igc.ini
+++ b/doc/guides/nics/features/igc.ini
@@ -35,7 +35,7 @@ Linux                = Y
 x86-64               = Y
 
 [rte_flow items]
-eth                  = Y
+eth                  = P
 ipv4                 = Y
 ipv6                 = Y
 tcp                  = Y
diff --git a/doc/guides/nics/features/ipn3ke.ini b/doc/guides/nics/features/ipn3ke.ini
index defc39f525..1f6b780273 100644
--- a/doc/guides/nics/features/ipn3ke.ini
+++ b/doc/guides/nics/features/ipn3ke.ini
@@ -47,13 +47,13 @@ x86-32               = Y
 x86-64               = Y
 
 [rte_flow items]
-eth                  = Y
+eth                  = P
 ipv4                 = Y
 mpls                 = Y
 nvgre                = Y
 tcp                  = Y
 udp                  = Y
-vlan                 = Y
+vlan                 = P
 vxlan                = Y
 
 [rte_flow actions]
diff --git a/doc/guides/nics/features/ixgbe.ini b/doc/guides/nics/features/ixgbe.ini
index 97c0a6af9e..8590ac857f 100644
--- a/doc/guides/nics/features/ixgbe.ini
+++ b/doc/guides/nics/features/ixgbe.ini
@@ -58,7 +58,7 @@ x86-32               = Y
 x86-64               = Y
 
 [rte_flow items]
-eth                  = Y
+eth                  = P
 e_tag                = Y
 fuzzy                = Y
 ipv4                 = Y
@@ -68,7 +68,7 @@ raw                  = Y
 sctp                 = Y
 tcp                  = Y
 udp                  = Y
-vlan                 = Y
+vlan                 = P
 vxlan                = Y
 
 [rte_flow actions]
diff --git a/doc/guides/nics/features/mlx4.ini b/doc/guides/nics/features/mlx4.ini
index 82f6f0bc0b..03f59a5f8b 100644
--- a/doc/guides/nics/features/mlx4.ini
+++ b/doc/guides/nics/features/mlx4.ini
@@ -38,11 +38,11 @@ x86-64               = Y
 Usage doc            = Y
 
 [rte_flow items]
-eth                  = Y
+eth                  = P
 ipv4                 = Y
 tcp                  = Y
 udp                  = Y
-vlan                 = Y
+vlan                 = P
 
 [rte_flow actions]
 drop                 = Y
diff --git a/doc/guides/nics/features/mvpp2.ini b/doc/guides/nics/features/mvpp2.ini
index 1bcf74875e..653c9d08cb 100644
--- a/doc/guides/nics/features/mvpp2.ini
+++ b/doc/guides/nics/features/mvpp2.ini
@@ -24,13 +24,13 @@ ARMv8                = Y
 Usage doc            = Y
 
 [rte_flow items]
-eth                  = Y
+eth                  = P
 ipv4                 = Y
 ipv6                 = Y
 raw                  = Y
 tcp                  = Y
 udp                  = Y
-vlan                 = Y
+vlan                 = P
 
 [rte_flow actions]
 drop                 = Y
diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index b4a356e5d5..f26355e57f 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -27,12 +27,12 @@ x86-64               = Y
 Usage doc            = Y
 
 [rte_flow items]
-eth                  = Y
+eth                  = P
 ipv4                 = Y
 ipv6                 = Y
 tcp                  = Y
 udp                  = Y
-vlan                 = Y
+vlan                 = P
 
 [rte_flow actions]
 drop                 = Y
diff --git a/doc/guides/nics/features/txgbe.ini b/doc/guides/nics/features/txgbe.ini
index 22c74ba9e3..e21083052c 100644
--- a/doc/guides/nics/features/txgbe.ini
+++ b/doc/guides/nics/features/txgbe.ini
@@ -53,7 +53,7 @@ x86-32               = Y
 x86-64               = Y
 
 [rte_flow items]
-eth                  = Y
+eth                  = P
 e_tag                = Y
 fuzzy                = Y
 ipv4                 = Y
@@ -63,7 +63,7 @@ raw                  = Y
 sctp                 = Y
 tcp                  = Y
 udp                  = Y
-vlan                 = Y
+vlan                 = P
 vxlan                = Y
 
 [rte_flow actions]
-- 
2.37.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] doc: fix support table for ETH and VLAN flow items
  2022-10-13 10:48 [PATCH v2] doc: fix support table for ETH and VLAN flow items Ilya Maximets
@ 2022-10-14  9:41 ` fengchengwen
  2022-10-14 12:37   ` Ilya Maximets
  2022-10-26 15:34 ` Thomas Monjalon
  1 sibling, 1 reply; 8+ messages in thread
From: fengchengwen @ 2022-10-14  9:41 UTC (permalink / raw)
  To: Ilya Maximets, dev, Thomas Monjalon
  Cc: Ajit Khaparde, Somnath Kotur, Rahul Lakkireddy, Hemant Agrawal,
	Sachin Saxena, Simei Su, Wenjun Wu, John Daley, Hyong Youb Kim,
	Ziyang Xuan, Xiaoyun Wang, Guoyang Zhou, Dongdong Liu,
	Yisen Zhuang, Yuying Zhang, Beilei Xing, Jingjing Wu,
	Qiming Yang, Qi Zhang, Junfeng Guo, Rosen Xu, Matan Azrad,
	Viacheslav Ovsiienko, Liron Himi, Jiawen Wu, Jian Wang,
	Dekel Peled, Ori Kam, stable

Hi Ilya,

   I have some questions about has_vlan/has_more_vlan fields:

   a\ DPDK framework support cvlan-tag(0x8100) and svlan-tag(0x88A8), and also deprecated qinq-tag(eg. 0x9100)
   b\ If has_vlan is used, does it mean that all the VLAN tags(0x8100/88A8/9100) must be matched ?
      I think this is different from using type, which can only match one of them.
   c\ And has_more_vlan has the same function as has_vlan ?
   d\ What the problems are solved by the new two fields?


   If the above understanding is correct, and the hardware support identify there TPID(cvlan-0x8100, svlan-0x88A8, dqing-0x9100) as VLAN, then:
     Rule: eth has_vlan is 1 / vlan vid is 100 / ipv4 / end actions xxx
     Result: all ipv4 packets with at least one VLAN(the TPID can be one of the above) and the vid is 100 can be matched.

     Rule: eth type is 0x8100 / vlan vid is 100 / ipv4 / end actions xxx
     Result: all ipv4 packets with at lease one VLAN(which TPID must be 0x8100) and the vid is 100 can be matched.

     Rule: eth has_vlan is 1 / vlan vid is 100 has_more_vlan is 1 / vlan vid is 200 / ipv4 / end action xxx
     Result: all ipv4 packets with at least two VLAN(the TPID can be one of the above) and outer vid is 100 and the next vid is 200 can be matched.

     Rule: eth type is 0x88A8 / vlan vid is 100 inner_type is 0x8100 / vlan vid is 200 / ipv4 / end action xxx
     Result: all ipv4 packets with at least two VLAN(the first TPID is 0x88A8 and second TPID is 0x8100) and outer vid is 100 and the next vid is 200 can be matched.
   Is the above result correct ?

Thanks

On 2022/10/13 18:48, Ilya Maximets wrote:
> 'has_vlan' attribute is only supported by sfc, mlx5 and cnxk.
> Other drivers doesn't support it.  Most of them (like i40e) just
> ignore it silently.  Some drivers (like mlx4) never had a full
> support of the eth item even before introduction of 'has_vlan'
> (mlx4 allows to match on the destination MAC only).
> 
> Same for the 'has_more_vlan' flag of the vlan item.
> 
> 'has_vlan' is part of 'rte_flow_item_eth', so changing 'eth'
> field to 'partial support' in documentation for all such drivers.
> 'has_more_vlan' is part of 'rte_flow_item_vlan', so changing
> 'vlan' to 'partial support' as well.
> 
> This doesn't solve the issue, but at least marks the problematic
> drivers.
> 
> Some details are available in:
>   https://bugs.dpdk.org/show_bug.cgi?id=958
> 
> Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN items")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---
> 
> Version 2:
>   - Rebased on a current main branch.
>   - Added more clarifications to the commit message.
> 
> I added the stable in CC, but the patch should be extended while
> backporting.  For 21.11 the cnxk driver should be also updated,
> for 20.11, sfc driver should also be included.
> 
>  doc/guides/nics/features/bnxt.ini   | 4 ++--
>  doc/guides/nics/features/cxgbe.ini  | 4 ++--
>  doc/guides/nics/features/dpaa2.ini  | 4 ++--
>  doc/guides/nics/features/e1000.ini  | 2 +-
>  doc/guides/nics/features/enic.ini   | 4 ++--
>  doc/guides/nics/features/hinic.ini  | 2 +-
>  doc/guides/nics/features/hns3.ini   | 4 ++--
>  doc/guides/nics/features/i40e.ini   | 4 ++--
>  doc/guides/nics/features/iavf.ini   | 4 ++--
>  doc/guides/nics/features/ice.ini    | 4 ++--
>  doc/guides/nics/features/igc.ini    | 2 +-
>  doc/guides/nics/features/ipn3ke.ini | 4 ++--
>  doc/guides/nics/features/ixgbe.ini  | 4 ++--
>  doc/guides/nics/features/mlx4.ini   | 4 ++--
>  doc/guides/nics/features/mvpp2.ini  | 4 ++--
>  doc/guides/nics/features/tap.ini    | 4 ++--
>  doc/guides/nics/features/txgbe.ini  | 4 ++--
>  17 files changed, 31 insertions(+), 31 deletions(-)
> 
> diff --git a/doc/guides/nics/features/bnxt.ini b/doc/guides/nics/features/bnxt.ini
> index b2d54f06aa..50a0b5bfa6 100644
> --- a/doc/guides/nics/features/bnxt.ini
> +++ b/doc/guides/nics/features/bnxt.ini
> @@ -57,7 +57,7 @@ Perf doc             = Y
>  
>  [rte_flow items]
>  any                  = Y
> -eth                  = Y
> +eth                  = P
>  ipv4                 = Y
>  ipv6                 = Y
>  gre                  = Y
> @@ -68,7 +68,7 @@ port_representor     = Y
>  represented_port     = Y
>  tcp                  = Y
>  udp                  = Y
> -vlan                 = Y
> +vlan                 = P
>  vxlan                = Y
>  
>  [rte_flow actions]
> diff --git a/doc/guides/nics/features/cxgbe.ini b/doc/guides/nics/features/cxgbe.ini
> index a9dbcd0573..0d67ca8720 100644
> --- a/doc/guides/nics/features/cxgbe.ini
> +++ b/doc/guides/nics/features/cxgbe.ini
> @@ -36,12 +36,12 @@ x86-64               = Y
>  Usage doc            = Y
>  
>  [rte_flow items]
> -eth                  = Y
> +eth                  = P
>  ipv4                 = Y
>  ipv6                 = Y
>  tcp                  = Y
>  udp                  = Y
> -vlan                 = Y
> +vlan                 = P
>  
>  [rte_flow actions]
>  count                = Y
> diff --git a/doc/guides/nics/features/dpaa2.ini b/doc/guides/nics/features/dpaa2.ini
> index cedc234f26..26dc8c2178 100644
> --- a/doc/guides/nics/features/dpaa2.ini
> +++ b/doc/guides/nics/features/dpaa2.ini
> @@ -31,7 +31,7 @@ ARMv8                = Y
>  Usage doc            = Y
>  
>  [rte_flow items]
> -eth                  = Y
> +eth                  = P
>  gre                  = Y
>  icmp                 = Y
>  ipv4                 = Y
> @@ -41,7 +41,7 @@ raw                  = Y
>  sctp                 = Y
>  tcp                  = Y
>  udp                  = Y
> -vlan                 = Y
> +vlan                 = P
>  
>  [rte_flow actions]
>  drop                 = Y
> diff --git a/doc/guides/nics/features/e1000.ini b/doc/guides/nics/features/e1000.ini
> index e4bdef6da9..a9cbed1c3c 100644
> --- a/doc/guides/nics/features/e1000.ini
> +++ b/doc/guides/nics/features/e1000.ini
> @@ -31,7 +31,7 @@ x86-32               = Y
>  x86-64               = Y
>  
>  [rte_flow items]
> -eth                  = Y
> +eth                  = P
>  ipv4                 = Y
>  ipv6                 = Y
>  raw                  = Y
> diff --git a/doc/guides/nics/features/enic.ini b/doc/guides/nics/features/enic.ini
> index 61bec4910e..6dbea9f36e 100644
> --- a/doc/guides/nics/features/enic.ini
> +++ b/doc/guides/nics/features/enic.ini
> @@ -40,7 +40,7 @@ Usage doc            = Y
>  
>  [rte_flow items]
>  ecpri                = Y
> -eth                  = Y
> +eth                  = P
>  geneve               = Y
>  geneve_opt           = Y
>  gtp                  = Y
> @@ -52,7 +52,7 @@ raw                  = Y
>  sctp                 = Y
>  tcp                  = Y
>  udp                  = Y
> -vlan                 = Y
> +vlan                 = P
>  vxlan                = Y
>  
>  [rte_flow actions]
> diff --git a/doc/guides/nics/features/hinic.ini b/doc/guides/nics/features/hinic.ini
> index 9f6f0ebf3a..ada6607fe9 100644
> --- a/doc/guides/nics/features/hinic.ini
> +++ b/doc/guides/nics/features/hinic.ini
> @@ -40,7 +40,7 @@ ARMv8                = Y
>  
>  [rte_flow items]
>  any                  = Y
> -eth                  = Y
> +eth                  = P
>  icmp                 = Y
>  icmp6                = Y
>  ipv4                 = Y
> diff --git a/doc/guides/nics/features/hns3.ini b/doc/guides/nics/features/hns3.ini
> index 405b94f05c..338b4e6864 100644
> --- a/doc/guides/nics/features/hns3.ini
> +++ b/doc/guides/nics/features/hns3.ini
> @@ -51,7 +51,7 @@ Linux                = Y
>  ARMv8                = Y
>  
>  [rte_flow items]
> -eth                  = Y
> +eth                  = P
>  geneve               = Y
>  icmp                 = Y
>  ipv4                 = Y
> @@ -60,7 +60,7 @@ nvgre                = Y
>  sctp                 = Y
>  tcp                  = Y
>  udp                  = Y
> -vlan                 = Y
> +vlan                 = P
>  vxlan                = Y
>  vxlan_gpe            = Y
>  
> diff --git a/doc/guides/nics/features/i40e.ini b/doc/guides/nics/features/i40e.ini
> index 95e39aaba0..e241dad047 100644
> --- a/doc/guides/nics/features/i40e.ini
> +++ b/doc/guides/nics/features/i40e.ini
> @@ -54,7 +54,7 @@ Power8               = Y
>  [rte_flow items]
>  ah                   = Y
>  esp                  = Y
> -eth                  = Y
> +eth                  = P
>  gre                  = Y
>  gtpc                 = Y
>  gtpu                 = Y
> @@ -68,7 +68,7 @@ raw                  = Y
>  sctp                 = Y
>  tcp                  = Y
>  udp                  = Y
> -vlan                 = Y
> +vlan                 = P
>  vxlan                = Y
>  
>  [rte_flow actions]
> diff --git a/doc/guides/nics/features/iavf.ini b/doc/guides/nics/features/iavf.ini
> index eeda6b7210..9db2865b71 100644
> --- a/doc/guides/nics/features/iavf.ini
> +++ b/doc/guides/nics/features/iavf.ini
> @@ -43,7 +43,7 @@ ah                   = Y
>  arp_eth_ipv4         = Y
>  ecpri                = Y
>  esp                  = Y
> -eth                  = Y
> +eth                  = P
>  gre                  = Y
>  gtpc                 = Y
>  gtpu                 = Y
> @@ -61,7 +61,7 @@ raw                  = Y
>  sctp                 = Y
>  tcp                  = Y
>  udp                  = Y
> -vlan                 = Y
> +vlan                 = P
>  
>  [rte_flow actions]
>  count                = Y
> diff --git a/doc/guides/nics/features/ice.ini b/doc/guides/nics/features/ice.ini
> index 032da8e2e2..13f8871dcc 100644
> --- a/doc/guides/nics/features/ice.ini
> +++ b/doc/guides/nics/features/ice.ini
> @@ -55,7 +55,7 @@ ah                   = Y
>  any                  = Y
>  arp_eth_ipv4         = Y
>  esp                  = Y
> -eth                  = Y
> +eth                  = P
>  gtpu                 = Y
>  gtp_psc              = Y
>  icmp                 = Y
> @@ -73,7 +73,7 @@ raw                  = Y
>  sctp                 = Y
>  tcp                  = Y
>  udp                  = Y
> -vlan                 = Y
> +vlan                 = P
>  vxlan                = Y
>  
>  [rte_flow actions]
> diff --git a/doc/guides/nics/features/igc.ini b/doc/guides/nics/features/igc.ini
> index f2c6fa28ad..b5deea3f61 100644
> --- a/doc/guides/nics/features/igc.ini
> +++ b/doc/guides/nics/features/igc.ini
> @@ -35,7 +35,7 @@ Linux                = Y
>  x86-64               = Y
>  
>  [rte_flow items]
> -eth                  = Y
> +eth                  = P
>  ipv4                 = Y
>  ipv6                 = Y
>  tcp                  = Y
> diff --git a/doc/guides/nics/features/ipn3ke.ini b/doc/guides/nics/features/ipn3ke.ini
> index defc39f525..1f6b780273 100644
> --- a/doc/guides/nics/features/ipn3ke.ini
> +++ b/doc/guides/nics/features/ipn3ke.ini
> @@ -47,13 +47,13 @@ x86-32               = Y
>  x86-64               = Y
>  
>  [rte_flow items]
> -eth                  = Y
> +eth                  = P
>  ipv4                 = Y
>  mpls                 = Y
>  nvgre                = Y
>  tcp                  = Y
>  udp                  = Y
> -vlan                 = Y
> +vlan                 = P
>  vxlan                = Y
>  
>  [rte_flow actions]
> diff --git a/doc/guides/nics/features/ixgbe.ini b/doc/guides/nics/features/ixgbe.ini
> index 97c0a6af9e..8590ac857f 100644
> --- a/doc/guides/nics/features/ixgbe.ini
> +++ b/doc/guides/nics/features/ixgbe.ini
> @@ -58,7 +58,7 @@ x86-32               = Y
>  x86-64               = Y
>  
>  [rte_flow items]
> -eth                  = Y
> +eth                  = P
>  e_tag                = Y
>  fuzzy                = Y
>  ipv4                 = Y
> @@ -68,7 +68,7 @@ raw                  = Y
>  sctp                 = Y
>  tcp                  = Y
>  udp                  = Y
> -vlan                 = Y
> +vlan                 = P
>  vxlan                = Y
>  
>  [rte_flow actions]
> diff --git a/doc/guides/nics/features/mlx4.ini b/doc/guides/nics/features/mlx4.ini
> index 82f6f0bc0b..03f59a5f8b 100644
> --- a/doc/guides/nics/features/mlx4.ini
> +++ b/doc/guides/nics/features/mlx4.ini
> @@ -38,11 +38,11 @@ x86-64               = Y
>  Usage doc            = Y
>  
>  [rte_flow items]
> -eth                  = Y
> +eth                  = P
>  ipv4                 = Y
>  tcp                  = Y
>  udp                  = Y
> -vlan                 = Y
> +vlan                 = P
>  
>  [rte_flow actions]
>  drop                 = Y
> diff --git a/doc/guides/nics/features/mvpp2.ini b/doc/guides/nics/features/mvpp2.ini
> index 1bcf74875e..653c9d08cb 100644
> --- a/doc/guides/nics/features/mvpp2.ini
> +++ b/doc/guides/nics/features/mvpp2.ini
> @@ -24,13 +24,13 @@ ARMv8                = Y
>  Usage doc            = Y
>  
>  [rte_flow items]
> -eth                  = Y
> +eth                  = P
>  ipv4                 = Y
>  ipv6                 = Y
>  raw                  = Y
>  tcp                  = Y
>  udp                  = Y
> -vlan                 = Y
> +vlan                 = P
>  
>  [rte_flow actions]
>  drop                 = Y
> diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
> index b4a356e5d5..f26355e57f 100644
> --- a/doc/guides/nics/features/tap.ini
> +++ b/doc/guides/nics/features/tap.ini
> @@ -27,12 +27,12 @@ x86-64               = Y
>  Usage doc            = Y
>  
>  [rte_flow items]
> -eth                  = Y
> +eth                  = P
>  ipv4                 = Y
>  ipv6                 = Y
>  tcp                  = Y
>  udp                  = Y
> -vlan                 = Y
> +vlan                 = P
>  
>  [rte_flow actions]
>  drop                 = Y
> diff --git a/doc/guides/nics/features/txgbe.ini b/doc/guides/nics/features/txgbe.ini
> index 22c74ba9e3..e21083052c 100644
> --- a/doc/guides/nics/features/txgbe.ini
> +++ b/doc/guides/nics/features/txgbe.ini
> @@ -53,7 +53,7 @@ x86-32               = Y
>  x86-64               = Y
>  
>  [rte_flow items]
> -eth                  = Y
> +eth                  = P
>  e_tag                = Y
>  fuzzy                = Y
>  ipv4                 = Y
> @@ -63,7 +63,7 @@ raw                  = Y
>  sctp                 = Y
>  tcp                  = Y
>  udp                  = Y
> -vlan                 = Y
> +vlan                 = P
>  vxlan                = Y
>  
>  [rte_flow actions]
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] doc: fix support table for ETH and VLAN flow items
  2022-10-14  9:41 ` fengchengwen
@ 2022-10-14 12:37   ` Ilya Maximets
  2022-10-16  5:26     ` Eli Britstein
  0 siblings, 1 reply; 8+ messages in thread
From: Ilya Maximets @ 2022-10-14 12:37 UTC (permalink / raw)
  To: fengchengwen, dev, Ori Kam, Thomas Monjalon, Eli Britstein
  Cc: i.maximets, Ajit Khaparde, Somnath Kotur, Rahul Lakkireddy,
	Hemant Agrawal, Sachin Saxena, Simei Su, Wenjun Wu, John Daley,
	Hyong Youb Kim, Ziyang Xuan, Xiaoyun Wang, Guoyang Zhou,
	Dongdong Liu, Yisen Zhuang, Yuying Zhang, Beilei Xing,
	Jingjing Wu, Qiming Yang, Qi Zhang, Junfeng Guo, Rosen Xu,
	Matan Azrad, Viacheslav Ovsiienko, Liron Himi, Jiawen Wu,
	Jian Wang, Dekel Peled, stable

On 10/14/22 11:41, fengchengwen wrote:
> Hi Ilya,
> 
>    I have some questions about has_vlan/has_more_vlan fields:

I think, these questions are more for rte_flow maintainers,
but I'll try answer.  Maintainers can correct me if I'm wrong.

> 
>    a\ DPDK framework support cvlan-tag(0x8100) and svlan-tag(0x88A8), and also deprecated qinq-tag(eg. 0x9100)

Didn't check, but sounds about right.

>    b\ If has_vlan is used, does it mean that all the VLAN tags(0x8100/88A8/9100) must be matched ?
>       I think this is different from using type, which can only match one of them.

I think so.  has_vlan = 1 means that packet has some vlan
regardless of the actual type of the vlan header.

>    c\ And has_more_vlan has the same function as has_vlan ?

Yes, from my understanding, 'has_more_vlan' is the same as 'has_vlan',
but for the 'inner_type'. 

>    d\ What the problems are solved by the new two fields?

One of the problems we solved in OVS by using these fields is that
we need a way to match on the fact that there is a vlan, but we
do not care what this vlan tag is and at the same time we want to
match on the inner type for such packets.

Trying to workaround that situation will likely require breaking
the 1:1 mapping between OVS flows and rte_flow rules, so it is
not really possible.  In the end, we had to use 'has_vlan' field
to fix an incorrect packet matching in OVS.  Alternative, I guess,
would be to just not offload vlan flows, but doesn't make a lot
of sense.

Eli should know better what was the actual problem, I think.

> 
> 
>    If the above understanding is correct, and the hardware support identify there TPID(cvlan-0x8100, svlan-0x88A8, dqing-0x9100) as VLAN, then:
>      Rule: eth has_vlan is 1 / vlan vid is 100 / ipv4 / end actions xxx
>      Result: all ipv4 packets with at least one VLAN(the TPID can be one of the above) and the vid is 100 can be matched.
> 
>      Rule: eth type is 0x8100 / vlan vid is 100 / ipv4 / end actions xxx
>      Result: all ipv4 packets with at lease one VLAN(which TPID must be 0x8100) and the vid is 100 can be matched.
> 
>      Rule: eth has_vlan is 1 / vlan vid is 100 has_more_vlan is 1 / vlan vid is 200 / ipv4 / end action xxx
>      Result: all ipv4 packets with at least two VLAN(the TPID can be one of the above) and outer vid is 100 and the next vid is 200 can be matched.
> 
>      Rule: eth type is 0x88A8 / vlan vid is 100 inner_type is 0x8100 / vlan vid is 200 / ipv4 / end action xxx
>      Result: all ipv4 packets with at least two VLAN(the first TPID is 0x88A8 and second TPID is 0x8100) and outer vid is 100 and the next vid is 200 can be matched.
>    Is the above result correct ?

Seems correct, but I don't have much experience with rte_flow notations.

Ori, could you comment on this?

Best regards, Ilya Maximets.

> 
> Thanks
> 
> On 2022/10/13 18:48, Ilya Maximets wrote:
>> 'has_vlan' attribute is only supported by sfc, mlx5 and cnxk.
>> Other drivers doesn't support it.  Most of them (like i40e) just
>> ignore it silently.  Some drivers (like mlx4) never had a full
>> support of the eth item even before introduction of 'has_vlan'
>> (mlx4 allows to match on the destination MAC only).
>>
>> Same for the 'has_more_vlan' flag of the vlan item.
>>
>> 'has_vlan' is part of 'rte_flow_item_eth', so changing 'eth'
>> field to 'partial support' in documentation for all such drivers.
>> 'has_more_vlan' is part of 'rte_flow_item_vlan', so changing
>> 'vlan' to 'partial support' as well.
>>
>> This doesn't solve the issue, but at least marks the problematic
>> drivers.
>>
>> Some details are available in:
>>   https://bugs.dpdk.org/show_bug.cgi?id=958
>>
>> Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN items")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
>> ---
>>
>> Version 2:
>>   - Rebased on a current main branch.
>>   - Added more clarifications to the commit message.
>>
>> I added the stable in CC, but the patch should be extended while
>> backporting.  For 21.11 the cnxk driver should be also updated,
>> for 20.11, sfc driver should also be included.
>>
>>  doc/guides/nics/features/bnxt.ini   | 4 ++--
>>  doc/guides/nics/features/cxgbe.ini  | 4 ++--
>>  doc/guides/nics/features/dpaa2.ini  | 4 ++--
>>  doc/guides/nics/features/e1000.ini  | 2 +-
>>  doc/guides/nics/features/enic.ini   | 4 ++--
>>  doc/guides/nics/features/hinic.ini  | 2 +-
>>  doc/guides/nics/features/hns3.ini   | 4 ++--
>>  doc/guides/nics/features/i40e.ini   | 4 ++--
>>  doc/guides/nics/features/iavf.ini   | 4 ++--
>>  doc/guides/nics/features/ice.ini    | 4 ++--
>>  doc/guides/nics/features/igc.ini    | 2 +-
>>  doc/guides/nics/features/ipn3ke.ini | 4 ++--
>>  doc/guides/nics/features/ixgbe.ini  | 4 ++--
>>  doc/guides/nics/features/mlx4.ini   | 4 ++--
>>  doc/guides/nics/features/mvpp2.ini  | 4 ++--
>>  doc/guides/nics/features/tap.ini    | 4 ++--
>>  doc/guides/nics/features/txgbe.ini  | 4 ++--
>>  17 files changed, 31 insertions(+), 31 deletions(-)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v2] doc: fix support table for ETH and VLAN flow items
  2022-10-14 12:37   ` Ilya Maximets
@ 2022-10-16  5:26     ` Eli Britstein
  2022-10-17 11:54       ` fengchengwen
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Britstein @ 2022-10-16  5:26 UTC (permalink / raw)
  To: Ilya Maximets, fengchengwen, dev, Ori Kam,
	NBU-Contact-Thomas Monjalon (EXTERNAL)
  Cc: Ajit Khaparde, Somnath Kotur, Rahul Lakkireddy, Hemant Agrawal,
	Sachin Saxena, Simei Su, Wenjun Wu, John Daley, Hyong Youb Kim,
	Ziyang Xuan, Xiaoyun Wang, Guoyang Zhou, Dongdong Liu,
	Yisen Zhuang, Yuying Zhang, Beilei Xing, Jingjing Wu,
	Qiming Yang, Qi Zhang, Junfeng Guo, Rosen Xu, Matan Azrad,
	Slava Ovsiienko, Liron Himi, Jiawen Wu, Jian Wang, Dekel Peled,
	stable



>-----Original Message-----
>From: Ilya Maximets <i.maximets@ovn.org>
>Sent: Friday, October 14, 2022 3:37 PM
>To: fengchengwen <fengchengwen@huawei.com>; dev@dpdk.org; Ori Kam
><orika@nvidia.com>; NBU-Contact-Thomas Monjalon (EXTERNAL)
><thomas@monjalon.net>; Eli Britstein <elibr@nvidia.com>
>Cc: i.maximets@ovn.org; Ajit Khaparde <ajit.khaparde@broadcom.com>;
>Somnath Kotur <somnath.kotur@broadcom.com>; Rahul Lakkireddy
><rahul.lakkireddy@chelsio.com>; Hemant Agrawal
><hemant.agrawal@nxp.com>; Sachin Saxena <sachin.saxena@oss.nxp.com>;
>Simei Su <simei.su@intel.com>; Wenjun Wu <wenjun1.wu@intel.com>; John
>Daley <johndale@cisco.com>; Hyong Youb Kim <hyonkim@cisco.com>; Ziyang
>Xuan <xuanziyang2@huawei.com>; Xiaoyun Wang
><cloud.wangxiaoyun@huawei.com>; Guoyang Zhou
><zhouguoyang@huawei.com>; Dongdong Liu <liudongdong3@huawei.com>;
>Yisen Zhuang <yisen.zhuang@huawei.com>; Yuying Zhang
><Yuying.Zhang@intel.com>; Beilei Xing <beilei.xing@intel.com>; Jingjing Wu
><jingjing.wu@intel.com>; Qiming Yang <qiming.yang@intel.com>; Qi Zhang
><qi.z.zhang@intel.com>; Junfeng Guo <junfeng.guo@intel.com>; Rosen Xu
><rosen.xu@intel.com>; Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
><viacheslavo@nvidia.com>; Liron Himi <lironh@marvell.com>; Jiawen Wu
><jiawenwu@trustnetic.com>; Jian Wang <jianwang@trustnetic.com>; Dekel
>Peled <dekelp@nvidia.com>; stable@dpdk.org
>Subject: Re: [PATCH v2] doc: fix support table for ETH and VLAN flow items
>
>External email: Use caution opening links or attachments
>
>
>On 10/14/22 11:41, fengchengwen wrote:
>> Hi Ilya,
>>
>>    I have some questions about has_vlan/has_more_vlan fields:
>
>I think, these questions are more for rte_flow maintainers, but I'll try answer.
>Maintainers can correct me if I'm wrong.
>
>>
>>    a\ DPDK framework support cvlan-tag(0x8100) and svlan-tag(0x88A8),
>> and also deprecated qinq-tag(eg. 0x9100)
>
>Didn't check, but sounds about right.
It is not related to "DPDK framework". It is up to the HW to determine.
>
>>    b\ If has_vlan is used, does it mean that all the VLAN
>tags(0x8100/88A8/9100) must be matched ?
>>       I think this is different from using type, which can only match one of
>them.
>
>I think so.  has_vlan = 1 means that packet has some vlan regardless of the
>actual type of the vlan header.
Again, it is up to the HW.
>
>>    c\ And has_more_vlan has the same function as has_vlan ?
>
>Yes, from my understanding, 'has_more_vlan' is the same as 'has_vlan', but
>for the 'inner_type'.
>
>>    d\ What the problems are solved by the new two fields?
>
>One of the problems we solved in OVS by using these fields is that we need a
>way to match on the fact that there is a vlan, but we do not care what this vlan
>tag is and at the same time we want to match on the inner type for such
>packets.
>
>Trying to workaround that situation will likely require breaking the 1:1
>mapping between OVS flows and rte_flow rules, so it is not really possible.  In
>the end, we had to use 'has_vlan' field to fix an incorrect packet matching in
>OVS.  Alternative, I guess, would be to just not offload vlan flows, but doesn't
>make a lot of sense.
>
>Eli should know better what was the actual problem, I think.
OVS does not support offload of qinq, so "has_more_vlan" is still not in use.
For native (untagged) flows, there is a need to tell the HW "has_vlan is 0", otherwise the HW flow will hit both tagged/untagged traffic, which is wrong.
For tagged flows, OVS will always match on the VLAN properties, so "has_vlan is 1" can be deducted/implicit.
Before that field existed, it could be implicit to deduct "lack" of VLAN header (e.g. "eth / ipv4" for example) as "has_vlan is 0". However, other applications that would like both tagged/untagged traffic to hit needed to have 2 separated flows (with a probably slightly lower performance).
Also, DPDK rte-flow is to have things explicit.
>
>>
>>
>>    If the above understanding is correct, and the hardware support identify
>there TPID(cvlan-0x8100, svlan-0x88A8, dqing-0x9100) as VLAN, then:
>>      Rule: eth has_vlan is 1 / vlan vid is 100 / ipv4 / end actions xxx
>>      Result: all ipv4 packets with at least one VLAN(the TPID can be one of the
>above) and the vid is 100 can be matched.
>>
>>      Rule: eth type is 0x8100 / vlan vid is 100 / ipv4 / end actions xxx
>>      Result: all ipv4 packets with at lease one VLAN(which TPID must be
>0x8100) and the vid is 100 can be matched.
>>
>>      Rule: eth has_vlan is 1 / vlan vid is 100 has_more_vlan is 1 / vlan vid is 200
>/ ipv4 / end action xxx
>>      Result: all ipv4 packets with at least two VLAN(the TPID can be one of the
>above) and outer vid is 100 and the next vid is 200 can be matched.
>>
>>      Rule: eth type is 0x88A8 / vlan vid is 100 inner_type is 0x8100 / vlan vid is
>200 / ipv4 / end action xxx
>>      Result: all ipv4 packets with at least two VLAN(the first TPID is 0x88A8 and
>second TPID is 0x8100) and outer vid is 100 and the next vid is 200 can be
>matched.
>>    Is the above result correct ?
>
>Seems correct, but I don't have much experience with rte_flow notations.
>
>Ori, could you comment on this?
>
>Best regards, Ilya Maximets.
>
>>
>> Thanks
>>
>> On 2022/10/13 18:48, Ilya Maximets wrote:
>>> 'has_vlan' attribute is only supported by sfc, mlx5 and cnxk.
>>> Other drivers doesn't support it.  Most of them (like i40e) just
>>> ignore it silently.  Some drivers (like mlx4) never had a full
>>> support of the eth item even before introduction of 'has_vlan'
>>> (mlx4 allows to match on the destination MAC only).
>>>
>>> Same for the 'has_more_vlan' flag of the vlan item.
>>>
>>> 'has_vlan' is part of 'rte_flow_item_eth', so changing 'eth'
>>> field to 'partial support' in documentation for all such drivers.
>>> 'has_more_vlan' is part of 'rte_flow_item_vlan', so changing 'vlan'
>>> to 'partial support' as well.
>>>
>>> This doesn't solve the issue, but at least marks the problematic
>>> drivers.
>>>
>>> Some details are available in:
>>>   https://bugs.dpdk.org/show_bug.cgi?id=958
>>>
>>> Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and
>>> VLAN items")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
>>> ---
>>>
>>> Version 2:
>>>   - Rebased on a current main branch.
>>>   - Added more clarifications to the commit message.
>>>
>>> I added the stable in CC, but the patch should be extended while
>>> backporting.  For 21.11 the cnxk driver should be also updated, for
>>> 20.11, sfc driver should also be included.
>>>
>>>  doc/guides/nics/features/bnxt.ini   | 4 ++--
>>>  doc/guides/nics/features/cxgbe.ini  | 4 ++--
>>> doc/guides/nics/features/dpaa2.ini  | 4 ++--
>>> doc/guides/nics/features/e1000.ini  | 2 +-
>>>  doc/guides/nics/features/enic.ini   | 4 ++--
>>>  doc/guides/nics/features/hinic.ini  | 2 +-
>>>  doc/guides/nics/features/hns3.ini   | 4 ++--
>>>  doc/guides/nics/features/i40e.ini   | 4 ++--
>>>  doc/guides/nics/features/iavf.ini   | 4 ++--
>>>  doc/guides/nics/features/ice.ini    | 4 ++--
>>>  doc/guides/nics/features/igc.ini    | 2 +-
>>>  doc/guides/nics/features/ipn3ke.ini | 4 ++--
>>> doc/guides/nics/features/ixgbe.ini  | 4 ++--
>>>  doc/guides/nics/features/mlx4.ini   | 4 ++--
>>>  doc/guides/nics/features/mvpp2.ini  | 4 ++--
>>>  doc/guides/nics/features/tap.ini    | 4 ++--
>>>  doc/guides/nics/features/txgbe.ini  | 4 ++--
>>>  17 files changed, 31 insertions(+), 31 deletions(-)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] doc: fix support table for ETH and VLAN flow items
  2022-10-16  5:26     ` Eli Britstein
@ 2022-10-17 11:54       ` fengchengwen
  2022-10-18  7:18         ` Ori Kam
  0 siblings, 1 reply; 8+ messages in thread
From: fengchengwen @ 2022-10-17 11:54 UTC (permalink / raw)
  To: Eli Britstein, Ilya Maximets, dev, Ori Kam,
	NBU-Contact-Thomas Monjalon (EXTERNAL)
  Cc: Ajit Khaparde, Somnath Kotur, Rahul Lakkireddy, Hemant Agrawal,
	Sachin Saxena, Simei Su, Wenjun Wu, John Daley, Hyong Youb Kim,
	Ziyang Xuan, Xiaoyun Wang, Guoyang Zhou, Dongdong Liu,
	Yisen Zhuang, Yuying Zhang, Beilei Xing, Jingjing Wu,
	Qiming Yang, Qi Zhang, Junfeng Guo, Rosen Xu, Matan Azrad,
	Slava Ovsiienko, Liron Himi, Jiawen Wu, Jian Wang, Dekel Peled,
	stable

Thanks Ilya and Eli

On 2022/10/16 13:26, Eli Britstein wrote:
> 
> 
>> -----Original Message-----
>> From: Ilya Maximets <i.maximets@ovn.org>
>> Sent: Friday, October 14, 2022 3:37 PM
>> To: fengchengwen <fengchengwen@huawei.com>; dev@dpdk.org; Ori Kam
>> <orika@nvidia.com>; NBU-Contact-Thomas Monjalon (EXTERNAL)
>> <thomas@monjalon.net>; Eli Britstein <elibr@nvidia.com>
>> Cc: i.maximets@ovn.org; Ajit Khaparde <ajit.khaparde@broadcom.com>;
>> Somnath Kotur <somnath.kotur@broadcom.com>; Rahul Lakkireddy
>> <rahul.lakkireddy@chelsio.com>; Hemant Agrawal
>> <hemant.agrawal@nxp.com>; Sachin Saxena <sachin.saxena@oss.nxp.com>;
>> Simei Su <simei.su@intel.com>; Wenjun Wu <wenjun1.wu@intel.com>; John
>> Daley <johndale@cisco.com>; Hyong Youb Kim <hyonkim@cisco.com>; Ziyang
>> Xuan <xuanziyang2@huawei.com>; Xiaoyun Wang
>> <cloud.wangxiaoyun@huawei.com>; Guoyang Zhou
>> <zhouguoyang@huawei.com>; Dongdong Liu <liudongdong3@huawei.com>;
>> Yisen Zhuang <yisen.zhuang@huawei.com>; Yuying Zhang
>> <Yuying.Zhang@intel.com>; Beilei Xing <beilei.xing@intel.com>; Jingjing Wu
>> <jingjing.wu@intel.com>; Qiming Yang <qiming.yang@intel.com>; Qi Zhang
>> <qi.z.zhang@intel.com>; Junfeng Guo <junfeng.guo@intel.com>; Rosen Xu
>> <rosen.xu@intel.com>; Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
>> <viacheslavo@nvidia.com>; Liron Himi <lironh@marvell.com>; Jiawen Wu
>> <jiawenwu@trustnetic.com>; Jian Wang <jianwang@trustnetic.com>; Dekel
>> Peled <dekelp@nvidia.com>; stable@dpdk.org
>> Subject: Re: [PATCH v2] doc: fix support table for ETH and VLAN flow items
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On 10/14/22 11:41, fengchengwen wrote:
>>> Hi Ilya,
>>>
>>>    I have some questions about has_vlan/has_more_vlan fields:
>>
>> I think, these questions are more for rte_flow maintainers, but I'll try answer.
>> Maintainers can correct me if I'm wrong.
>>
>>>
>>>    a\ DPDK framework support cvlan-tag(0x8100) and svlan-tag(0x88A8),
>>> and also deprecated qinq-tag(eg. 0x9100)
>>
>> Didn't check, but sounds about right.
> It is not related to "DPDK framework". It is up to the HW to determine.
>>
>>>    b\ If has_vlan is used, does it mean that all the VLAN
>> tags(0x8100/88A8/9100) must be matched ?
>>>       I think this is different from using type, which can only match one of
>> them.
>>
>> I think so.  has_vlan = 1 means that packet has some vlan regardless of the
>> actual type of the vlan header.
> Again, it is up to the HW.
>>
>>>    c\ And has_more_vlan has the same function as has_vlan ?
>>
>> Yes, from my understanding, 'has_more_vlan' is the same as 'has_vlan', but
>> for the 'inner_type'.
>>
>>>    d\ What the problems are solved by the new two fields?
>>
>> One of the problems we solved in OVS by using these fields is that we need a
>> way to match on the fact that there is a vlan, but we do not care what this vlan
>> tag is and at the same time we want to match on the inner type for such
>> packets.
>>
>> Trying to workaround that situation will likely require breaking the 1:1
>> mapping between OVS flows and rte_flow rules, so it is not really possible.  In
>> the end, we had to use 'has_vlan' field to fix an incorrect packet matching in
>> OVS.  Alternative, I guess, would be to just not offload vlan flows, but doesn't
>> make a lot of sense.
>>
>> Eli should know better what was the actual problem, I think.
> OVS does not support offload of qinq, so "has_more_vlan" is still not in use.
> For native (untagged) flows, there is a need to tell the HW "has_vlan is 0", otherwise the HW flow will hit both tagged/untagged traffic, which is wrong.
> For tagged flows, OVS will always match on the VLAN properties, so "has_vlan is 1" can be deducted/implicit.
> Before that field existed, it could be implicit to deduct "lack" of VLAN header (e.g. "eth / ipv4" for example) as "has_vlan is 0". However, other applications that would like both tagged/untagged traffic to hit needed to have 2 separated flows (with a probably slightly lower performance).

Got it, Thanks.

> Also, DPDK rte-flow is to have things explicit.
>>
>>>
>>>
>>>    If the above understanding is correct, and the hardware support identify
>> there TPID(cvlan-0x8100, svlan-0x88A8, dqing-0x9100) as VLAN, then:
>>>      Rule: eth has_vlan is 1 / vlan vid is 100 / ipv4 / end actions xxx
>>>      Result: all ipv4 packets with at least one VLAN(the TPID can be one of the
>> above) and the vid is 100 can be matched.
>>>
>>>      Rule: eth type is 0x8100 / vlan vid is 100 / ipv4 / end actions xxx
>>>      Result: all ipv4 packets with at lease one VLAN(which TPID must be
>> 0x8100) and the vid is 100 can be matched.
>>>
>>>      Rule: eth has_vlan is 1 / vlan vid is 100 has_more_vlan is 1 / vlan vid is 200
>> / ipv4 / end action xxx
>>>      Result: all ipv4 packets with at least two VLAN(the TPID can be one of the
>> above) and outer vid is 100 and the next vid is 200 can be matched.
>>>
>>>      Rule: eth type is 0x88A8 / vlan vid is 100 inner_type is 0x8100 / vlan vid is
>> 200 / ipv4 / end action xxx
>>>      Result: all ipv4 packets with at least two VLAN(the first TPID is 0x88A8 and
>> second TPID is 0x8100) and outer vid is 100 and the next vid is 200 can be
>> matched.
>>>    Is the above result correct ?
>>
>> Seems correct, but I don't have much experience with rte_flow notations.
>>
>> Ori, could you comment on this?

Assuming that A is the number of VLANs by flow creation,
and B is the number of VLANs of real flow

What I'm concerned about is: Whether the matching is successful only when A is equal to B?

In addition, the maximum number of VLANs that can be parsed by hardware is limited,
For example, if the hardware supports a maximum of two VLAN tags, a rule with the number
of two VLAN tags is created for the RTE_Flow. However, the actual flow has more than two
VLAN tags. Can this situation be matched?

Hi Ori, Could you check on this?

>>
>> Best regards, Ilya Maximets.
>>
>>>
>>> Thanks
>>>
>>> On 2022/10/13 18:48, Ilya Maximets wrote:
>>>> 'has_vlan' attribute is only supported by sfc, mlx5 and cnxk.
>>>> Other drivers doesn't support it.  Most of them (like i40e) just
>>>> ignore it silently.  Some drivers (like mlx4) never had a full
>>>> support of the eth item even before introduction of 'has_vlan'
>>>> (mlx4 allows to match on the destination MAC only).
>>>>
>>>> Same for the 'has_more_vlan' flag of the vlan item.
>>>>
>>>> 'has_vlan' is part of 'rte_flow_item_eth', so changing 'eth'
>>>> field to 'partial support' in documentation for all such drivers.
>>>> 'has_more_vlan' is part of 'rte_flow_item_vlan', so changing 'vlan'
>>>> to 'partial support' as well.
>>>>
>>>> This doesn't solve the issue, but at least marks the problematic
>>>> drivers.
>>>>
>>>> Some details are available in:
>>>>   https://bugs.dpdk.org/show_bug.cgi?id=958
>>>>
>>>> Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and
>>>> VLAN items")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
>>>> ---
>>>>
>>>> Version 2:
>>>>   - Rebased on a current main branch.
>>>>   - Added more clarifications to the commit message.
>>>>
>>>> I added the stable in CC, but the patch should be extended while
>>>> backporting.  For 21.11 the cnxk driver should be also updated, for
>>>> 20.11, sfc driver should also be included.
>>>>
>>>>  doc/guides/nics/features/bnxt.ini   | 4 ++--
>>>>  doc/guides/nics/features/cxgbe.ini  | 4 ++--
>>>> doc/guides/nics/features/dpaa2.ini  | 4 ++--
>>>> doc/guides/nics/features/e1000.ini  | 2 +-
>>>>  doc/guides/nics/features/enic.ini   | 4 ++--
>>>>  doc/guides/nics/features/hinic.ini  | 2 +-
>>>>  doc/guides/nics/features/hns3.ini   | 4 ++--
>>>>  doc/guides/nics/features/i40e.ini   | 4 ++--
>>>>  doc/guides/nics/features/iavf.ini   | 4 ++--
>>>>  doc/guides/nics/features/ice.ini    | 4 ++--
>>>>  doc/guides/nics/features/igc.ini    | 2 +-
>>>>  doc/guides/nics/features/ipn3ke.ini | 4 ++--
>>>> doc/guides/nics/features/ixgbe.ini  | 4 ++--
>>>>  doc/guides/nics/features/mlx4.ini   | 4 ++--
>>>>  doc/guides/nics/features/mvpp2.ini  | 4 ++--
>>>>  doc/guides/nics/features/tap.ini    | 4 ++--
>>>>  doc/guides/nics/features/txgbe.ini  | 4 ++--
>>>>  17 files changed, 31 insertions(+), 31 deletions(-)
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v2] doc: fix support table for ETH and VLAN flow items
  2022-10-17 11:54       ` fengchengwen
@ 2022-10-18  7:18         ` Ori Kam
  0 siblings, 0 replies; 8+ messages in thread
From: Ori Kam @ 2022-10-18  7:18 UTC (permalink / raw)
  To: fengchengwen, Eli Britstein, Ilya Maximets, dev,
	NBU-Contact-Thomas Monjalon (EXTERNAL)
  Cc: Ajit Khaparde, Somnath Kotur, Rahul Lakkireddy, Hemant Agrawal,
	Sachin Saxena, Simei Su, Wenjun Wu, John Daley, Hyong Youb Kim,
	Ziyang Xuan, Xiaoyun Wang, Guoyang Zhou, Dongdong Liu,
	Yisen Zhuang, Yuying Zhang, Beilei Xing, Jingjing Wu,
	Qiming Yang, Qi Zhang, Junfeng Guo, Rosen Xu, Matan Azrad,
	Slava Ovsiienko, Liron Himi, Jiawen Wu, Jian Wang, Dekel Peled,
	stable



> -----Original Message-----
> From: fengchengwen <fengchengwen@huawei.com>
> Sent: Monday, 17 October 2022 14:55
> 
> Thanks Ilya and Eli
> 
> On 2022/10/16 13:26, Eli Britstein wrote:
> >
> >
> >> -----Original Message-----
> >> From: Ilya Maximets <i.maximets@ovn.org>
> >> Sent: Friday, October 14, 2022 3:37 PM
> >> To: fengchengwen <fengchengwen@huawei.com>; dev@dpdk.org; Ori
> Kam
> >> <orika@nvidia.com>; NBU-Contact-Thomas Monjalon (EXTERNAL)
> >> <thomas@monjalon.net>; Eli Britstein <elibr@nvidia.com>
> >> Cc: i.maximets@ovn.org; Ajit Khaparde <ajit.khaparde@broadcom.com>;
> >> Somnath Kotur <somnath.kotur@broadcom.com>; Rahul Lakkireddy
> >> <rahul.lakkireddy@chelsio.com>; Hemant Agrawal
> >> <hemant.agrawal@nxp.com>; Sachin Saxena
> <sachin.saxena@oss.nxp.com>;
> >> Simei Su <simei.su@intel.com>; Wenjun Wu <wenjun1.wu@intel.com>;
> John
> >> Daley <johndale@cisco.com>; Hyong Youb Kim <hyonkim@cisco.com>;
> Ziyang
> >> Xuan <xuanziyang2@huawei.com>; Xiaoyun Wang
> >> <cloud.wangxiaoyun@huawei.com>; Guoyang Zhou
> >> <zhouguoyang@huawei.com>; Dongdong Liu
> <liudongdong3@huawei.com>;
> >> Yisen Zhuang <yisen.zhuang@huawei.com>; Yuying Zhang
> >> <Yuying.Zhang@intel.com>; Beilei Xing <beilei.xing@intel.com>; Jingjing
> Wu
> >> <jingjing.wu@intel.com>; Qiming Yang <qiming.yang@intel.com>; Qi
> Zhang
> >> <qi.z.zhang@intel.com>; Junfeng Guo <junfeng.guo@intel.com>; Rosen
> Xu
> >> <rosen.xu@intel.com>; Matan Azrad <matan@nvidia.com>; Slava
> Ovsiienko
> >> <viacheslavo@nvidia.com>; Liron Himi <lironh@marvell.com>; Jiawen Wu
> >> <jiawenwu@trustnetic.com>; Jian Wang <jianwang@trustnetic.com>;
> Dekel
> >> Peled <dekelp@nvidia.com>; stable@dpdk.org
> >> Subject: Re: [PATCH v2] doc: fix support table for ETH and VLAN flow items
> >>
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> On 10/14/22 11:41, fengchengwen wrote:
> >>> Hi Ilya,
> >>>
> >>>    I have some questions about has_vlan/has_more_vlan fields:
> >>
> >> I think, these questions are more for rte_flow maintainers, but I'll try
> answer.
> >> Maintainers can correct me if I'm wrong.
> >>
> >>>
> >>>    a\ DPDK framework support cvlan-tag(0x8100) and svlan-tag(0x88A8),
> >>> and also deprecated qinq-tag(eg. 0x9100)
> >>
> >> Didn't check, but sounds about right.
> > It is not related to "DPDK framework". It is up to the HW to determine.
> >>
> >>>    b\ If has_vlan is used, does it mean that all the VLAN
> >> tags(0x8100/88A8/9100) must be matched ?
> >>>       I think this is different from using type, which can only match one of
> >> them.
> >>
> >> I think so.  has_vlan = 1 means that packet has some vlan regardless of the
> >> actual type of the vlan header.
> > Again, it is up to the HW.
> >>
> >>>    c\ And has_more_vlan has the same function as has_vlan ?
> >>
> >> Yes, from my understanding, 'has_more_vlan' is the same as 'has_vlan',
> but
> >> for the 'inner_type'.
> >>
> >>>    d\ What the problems are solved by the new two fields?
> >>
> >> One of the problems we solved in OVS by using these fields is that we
> need a
> >> way to match on the fact that there is a vlan, but we do not care what this
> vlan
> >> tag is and at the same time we want to match on the inner type for such
> >> packets.
> >>
> >> Trying to workaround that situation will likely require breaking the 1:1
> >> mapping between OVS flows and rte_flow rules, so it is not really
> possible.  In
> >> the end, we had to use 'has_vlan' field to fix an incorrect packet matching
> in
> >> OVS.  Alternative, I guess, would be to just not offload vlan flows, but
> doesn't
> >> make a lot of sense.
> >>
> >> Eli should know better what was the actual problem, I think.
> > OVS does not support offload of qinq, so "has_more_vlan" is still not in
> use.
> > For native (untagged) flows, there is a need to tell the HW "has_vlan is 0",
> otherwise the HW flow will hit both tagged/untagged traffic, which is wrong.
> > For tagged flows, OVS will always match on the VLAN properties, so
> "has_vlan is 1" can be deducted/implicit.
> > Before that field existed, it could be implicit to deduct "lack" of VLAN
> header (e.g. "eth / ipv4" for example) as "has_vlan is 0". However, other
> applications that would like both tagged/untagged traffic to hit needed to
> have 2 separated flows (with a probably slightly lower performance).
> 
> Got it, Thanks.
> 
> > Also, DPDK rte-flow is to have things explicit.
> >>
> >>>
> >>>
> >>>    If the above understanding is correct, and the hardware support
> identify
> >> there TPID(cvlan-0x8100, svlan-0x88A8, dqing-0x9100) as VLAN, then:
> >>>      Rule: eth has_vlan is 1 / vlan vid is 100 / ipv4 / end actions xxx
> >>>      Result: all ipv4 packets with at least one VLAN(the TPID can be one of
> the
> >> above) and the vid is 100 can be matched.
> >>>

Right, just to be extra clear the vid == 100 is for the outer most.
 
> >>>      Rule: eth type is 0x8100 / vlan vid is 100 / ipv4 / end actions xxx
> >>>      Result: all ipv4 packets with at lease one VLAN(which TPID must be
> >> 0x8100) and the vid is 100 can be matched.
> >>>

Right,

> >>>      Rule: eth has_vlan is 1 / vlan vid is 100 has_more_vlan is 1 / vlan vid is
> 200
> >> / ipv4 / end action xxx
> >>>      Result: all ipv4 packets with at least two VLAN(the TPID can be one of
> the
> >> above) and outer vid is 100 and the next vid is 200 can be matched.
> >>>

Right, but you don't need to use the has_vlan since vlan item is given in the pattern.

> >>>      Rule: eth type is 0x88A8 / vlan vid is 100 inner_type is 0x8100 / vlan
> vid is
> >> 200 / ipv4 / end action xxx
> >>>      Result: all ipv4 packets with at least two VLAN(the first TPID is 0x88A8
> and
> >> second TPID is 0x8100) and outer vid is 100 and the next vid is 200 can be
> >> matched.
> >>>    Is the above result correct ?
> >>
> >> Seems correct, but I don't have much experience with rte_flow notations.
> >>
> >> Ori, could you comment on this?
> 

Yes, everything looks good.

> Assuming that A is the number of VLANs by flow creation,
> and B is the number of VLANs of real flow
> 
> What I'm concerned about is: Whether the matching is successful only when
> A is equal to B?
> 
> In addition, the maximum number of VLANs that can be parsed by hardware
> is limited,
> For example, if the hardware supports a maximum of two VLAN tags, a rule
> with the number
> of two VLAN tags is created for the RTE_Flow. However, the actual flow has
> more than two
> VLAN tags. Can this situation be matched?
> 
> Hi Ori, Could you check on this?
> 

A must be smaller than B and the last  vlan has_more = 1
The matching is based on HW limitations, it depends on how the HW works.
> >>
> >> Best regards, Ilya Maximets.
> >>
> >>>
> >>> Thanks
> >>>
> >>> On 2022/10/13 18:48, Ilya Maximets wrote:
> >>>> 'has_vlan' attribute is only supported by sfc, mlx5 and cnxk.
> >>>> Other drivers doesn't support it.  Most of them (like i40e) just
> >>>> ignore it silently.  Some drivers (like mlx4) never had a full
> >>>> support of the eth item even before introduction of 'has_vlan'
> >>>> (mlx4 allows to match on the destination MAC only).
> >>>>
> >>>> Same for the 'has_more_vlan' flag of the vlan item.
> >>>>
> >>>> 'has_vlan' is part of 'rte_flow_item_eth', so changing 'eth'
> >>>> field to 'partial support' in documentation for all such drivers.
> >>>> 'has_more_vlan' is part of 'rte_flow_item_vlan', so changing 'vlan'
> >>>> to 'partial support' as well.
> >>>>
> >>>> This doesn't solve the issue, but at least marks the problematic
> >>>> drivers.
> >>>>
> >>>> Some details are available in:
> >>>>   https://bugs.dpdk.org/show_bug.cgi?id=958
> >>>>
> >>>> Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and
> >>>> VLAN items")
> >>>> Cc: stable@dpdk.org
> >>>>
> >>>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> >>>> ---
> >>>>
> >>>> Version 2:
> >>>>   - Rebased on a current main branch.
> >>>>   - Added more clarifications to the commit message.
> >>>>
> >>>> I added the stable in CC, but the patch should be extended while
> >>>> backporting.  For 21.11 the cnxk driver should be also updated, for
> >>>> 20.11, sfc driver should also be included.
> >>>>
> >>>>  doc/guides/nics/features/bnxt.ini   | 4 ++--
> >>>>  doc/guides/nics/features/cxgbe.ini  | 4 ++--
> >>>> doc/guides/nics/features/dpaa2.ini  | 4 ++--
> >>>> doc/guides/nics/features/e1000.ini  | 2 +-
> >>>>  doc/guides/nics/features/enic.ini   | 4 ++--
> >>>>  doc/guides/nics/features/hinic.ini  | 2 +-
> >>>>  doc/guides/nics/features/hns3.ini   | 4 ++--
> >>>>  doc/guides/nics/features/i40e.ini   | 4 ++--
> >>>>  doc/guides/nics/features/iavf.ini   | 4 ++--
> >>>>  doc/guides/nics/features/ice.ini    | 4 ++--
> >>>>  doc/guides/nics/features/igc.ini    | 2 +-
> >>>>  doc/guides/nics/features/ipn3ke.ini | 4 ++--
> >>>> doc/guides/nics/features/ixgbe.ini  | 4 ++--
> >>>>  doc/guides/nics/features/mlx4.ini   | 4 ++--
> >>>>  doc/guides/nics/features/mvpp2.ini  | 4 ++--
> >>>>  doc/guides/nics/features/tap.ini    | 4 ++--
> >>>>  doc/guides/nics/features/txgbe.ini  | 4 ++--
> >>>>  17 files changed, 31 insertions(+), 31 deletions(-)
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] doc: fix support table for ETH and VLAN flow items
  2022-10-13 10:48 [PATCH v2] doc: fix support table for ETH and VLAN flow items Ilya Maximets
  2022-10-14  9:41 ` fengchengwen
@ 2022-10-26 15:34 ` Thomas Monjalon
  2022-11-02 11:41   ` Ilya Maximets
  1 sibling, 1 reply; 8+ messages in thread
From: Thomas Monjalon @ 2022-10-26 15:34 UTC (permalink / raw)
  To: Ilya Maximets
  Cc: dev, Ajit Khaparde, Somnath Kotur, Rahul Lakkireddy,
	Hemant Agrawal, Sachin Saxena, Simei Su, Wenjun Wu, John Daley,
	Hyong Youb Kim, Ziyang Xuan, Xiaoyun Wang, Guoyang Zhou,
	Dongdong Liu, Yisen Zhuang, Yuying Zhang, Beilei Xing,
	Jingjing Wu, Qiming Yang, Qi Zhang, Junfeng Guo, Rosen Xu,
	Matan Azrad, Viacheslav Ovsiienko, Liron Himi, Jiawen Wu,
	Jian Wang, Dekel Peled, Ori Kam, stable, andrew.rybchenko,
	ferruh.yigit, bruce.richardson, david.marchand

13/10/2022 12:48, Ilya Maximets:
> 'has_vlan' attribute is only supported by sfc, mlx5 and cnxk.
> Other drivers doesn't support it.  Most of them (like i40e) just
> ignore it silently.  Some drivers (like mlx4) never had a full
> support of the eth item even before introduction of 'has_vlan'
> (mlx4 allows to match on the destination MAC only).
> 
> Same for the 'has_more_vlan' flag of the vlan item.
> 
> 'has_vlan' is part of 'rte_flow_item_eth', so changing 'eth'
> field to 'partial support' in documentation for all such drivers.
> 'has_more_vlan' is part of 'rte_flow_item_vlan', so changing
> 'vlan' to 'partial support' as well.
> 
> This doesn't solve the issue, but at least marks the problematic
> drivers.
> 
> Some details are available in:
>   https://bugs.dpdk.org/show_bug.cgi?id=958
> 
> Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN items")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

Applied, thanks.

How can we encourage fixing the problematic drivers?



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] doc: fix support table for ETH and VLAN flow items
  2022-10-26 15:34 ` Thomas Monjalon
@ 2022-11-02 11:41   ` Ilya Maximets
  0 siblings, 0 replies; 8+ messages in thread
From: Ilya Maximets @ 2022-11-02 11:41 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: i.maximets, dev, Ajit Khaparde, Somnath Kotur, Rahul Lakkireddy,
	Hemant Agrawal, Sachin Saxena, Simei Su, Wenjun Wu, John Daley,
	Hyong Youb Kim, Ziyang Xuan, Xiaoyun Wang, Guoyang Zhou,
	Dongdong Liu, Yisen Zhuang, Yuying Zhang, Beilei Xing,
	Jingjing Wu, Qiming Yang, Qi Zhang, Junfeng Guo, Rosen Xu,
	Matan Azrad, Viacheslav Ovsiienko, Liron Himi, Jiawen Wu,
	Jian Wang, Dekel Peled, Ori Kam, stable, andrew.rybchenko,
	ferruh.yigit, bruce.richardson, david.marchand

On 10/26/22 17:34, Thomas Monjalon wrote:
> 13/10/2022 12:48, Ilya Maximets:
>> 'has_vlan' attribute is only supported by sfc, mlx5 and cnxk.
>> Other drivers doesn't support it.  Most of them (like i40e) just
>> ignore it silently.  Some drivers (like mlx4) never had a full
>> support of the eth item even before introduction of 'has_vlan'
>> (mlx4 allows to match on the destination MAC only).
>>
>> Same for the 'has_more_vlan' flag of the vlan item.
>>
>> 'has_vlan' is part of 'rte_flow_item_eth', so changing 'eth'
>> field to 'partial support' in documentation for all such drivers.
>> 'has_more_vlan' is part of 'rte_flow_item_vlan', so changing
>> 'vlan' to 'partial support' as well.
>>
>> This doesn't solve the issue, but at least marks the problematic
>> drivers.
>>
>> Some details are available in:
>>   https://bugs.dpdk.org/show_bug.cgi?id=958
>>
>> Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN items")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> 
> Applied, thanks.

Thanks!

> 
> How can we encourage fixing the problematic drivers?

That's a tough question and I don't have a good answer.

One thought that I have is to make all drivers report a mask for
each ITEM.  That mask should represent all the bits in the ITEM
theoretically supported by the driver.  If some driver doesn't
report a mask for an ITEM, it is treated as all-zero mask.

While installing the flow or on flow validation, the generic
layer will check the rte_flow against masks reported by the
driver.  If there are no extra bits in the flow, it is passed
to the driver for further validation and installation.
If there are extra bits, flow is rejected at the high level.

Further validation is still required because only the driver
is able to check dependencies between fields.

Once such a checking is merged, it will break offloading for all
the drivers.  Maintainers will need to create appropriate masks
to unblock offloading.  memset() on the whole ITEM or similar
operations must not be allowed for these masks in the driver code.
Thorough review of these patches is necessary.

This will not help with encouraging to implement the features
per se, but will force maintainers to re-check their drivers.
And should eliminate issues where certain fields are silently
ignored, because until the driver is updated with a new mask
generic validation will reject offloading of flows that contain
matches on new fields.

At least applications will not fear to use certain features
knowing that they will be correctly rejected on validation.

Best regards, Ilya Maximets.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-11-02 11:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-13 10:48 [PATCH v2] doc: fix support table for ETH and VLAN flow items Ilya Maximets
2022-10-14  9:41 ` fengchengwen
2022-10-14 12:37   ` Ilya Maximets
2022-10-16  5:26     ` Eli Britstein
2022-10-17 11:54       ` fengchengwen
2022-10-18  7:18         ` Ori Kam
2022-10-26 15:34 ` Thomas Monjalon
2022-11-02 11:41   ` Ilya Maximets

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).