[dpdk-dev] net/i40e: do not turn on flexible payload on driver init

Message ID 1511960148-128836-1-git-send-email-kirill.rybalchenko@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Helin Zhang
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Rybalchenko, Kirill Nov. 29, 2017, 12:55 p.m. UTC
  Function i40e_GLQF_reg_init() overwrites global register for
flexible payload, forcing extraction of first 16 bytes of L2/L3/L4
payload to the field vector even if flexible payload is not used by
an application. Such unconditional turn on of flexible payload
effectively disables ability to use outer IP Destination address
for RSS/FDIR for tunnelled packets, as flexible payload
overwrites outer IP destination address on the field vector.

Now flexible payload turned on only when flow director is
enabled and configured.

Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 11 -----------
 drivers/net/i40e/i40e_fdir.c   |  8 ++++++++
 2 files changed, 8 insertions(+), 11 deletions(-)
  

Comments

Xing, Beilei Nov. 30, 2017, 9:17 a.m. UTC | #1
> -----Original Message-----
> From: Rybalchenko, Kirill
> Sent: Wednesday, November 29, 2017 8:56 PM
> To: dev@dpdk.org
> Cc: Rybalchenko, Kirill <kirill.rybalchenko@intel.com>; Chilikin, Andrey
> <andrey.chilikin@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>
> Subject: [PATCH] net/i40e: do not turn on flexible payload on driver init
> 
> Function i40e_GLQF_reg_init() overwrites global register for flexible payload,
> forcing extraction of first 16 bytes of L2/L3/L4 payload to the field vector
> even if flexible payload is not used by an application. Such unconditional turn
> on of flexible payload effectively disables ability to use outer IP Destination
> address for RSS/FDIR for tunnelled packets, as flexible payload overwrites
> outer IP destination address on the field vector.
> 
> Now flexible payload turned on only when flow director is enabled and
> configured.
> 
> Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 11 -----------
>  drivers/net/i40e/i40e_fdir.c   |  8 ++++++++
>  2 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 811cc9f..de5e2a3 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -701,17 +701,6 @@ RTE_PMD_REGISTER_KMOD_DEP(net_i40e, "*
> igb_uio | uio_pci_generic | vfio-pci");  static inline void
> i40e_GLQF_reg_init(struct i40e_hw *hw)  {
>  	/*
> -	 * Force global configuration for flexible payload
> -	 * to the first 16 bytes of the corresponding L2/L3/L4 paylod.
> -	 * This should be removed from code once proper
> -	 * configuration API is added to avoid configuration conflicts
> -	 * between ports of the same device.
> -	 */
> -	I40E_WRITE_REG(hw, I40E_GLQF_ORT(33), 0x000000E0);
> -	I40E_WRITE_REG(hw, I40E_GLQF_ORT(34), 0x000000E3);
> -	I40E_WRITE_REG(hw, I40E_GLQF_ORT(35), 0x000000E6);
> -
> -	/*
>  	 * Initialize registers for parsing packet type of QinQ
>  	 * This should be removed from code once proper
>  	 * configuration API is added to avoid configuration conflicts diff --git
> a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c index
> 3d7170d..8191f72 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -342,6 +342,14 @@ i40e_init_flx_pld(struct i40e_pf *pf)
>  	uint16_t flow_type;
> 
>  	/*
> +	 * Force global configuration for flexible payload
> +	 * to the first 16 bytes of the corresponding L2/L3/L4 paylod.
> +	 */
> +	I40E_WRITE_REG(hw, I40E_GLQF_ORT(33), 0x000000E0);
> +	I40E_WRITE_REG(hw, I40E_GLQF_ORT(34), 0x000000E3);
> +	I40E_WRITE_REG(hw, I40E_GLQF_ORT(35), 0x000000E6);
> +

In this case, the global registers will be set when enabling flow director, but it won't
be reset to default value unless core reset. So need an alternative mechanism to
configure the global registers.

> +	/*
>  	 * Define the bytes stream extracted as flexible payload in
>  	 * field vector. By default, select 8 words from the beginning
>  	 * of payload as flexible payload.
> --
> 2.5.5
  

Patch

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 811cc9f..de5e2a3 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -701,17 +701,6 @@  RTE_PMD_REGISTER_KMOD_DEP(net_i40e, "* igb_uio | uio_pci_generic | vfio-pci");
 static inline void i40e_GLQF_reg_init(struct i40e_hw *hw)
 {
 	/*
-	 * Force global configuration for flexible payload
-	 * to the first 16 bytes of the corresponding L2/L3/L4 paylod.
-	 * This should be removed from code once proper
-	 * configuration API is added to avoid configuration conflicts
-	 * between ports of the same device.
-	 */
-	I40E_WRITE_REG(hw, I40E_GLQF_ORT(33), 0x000000E0);
-	I40E_WRITE_REG(hw, I40E_GLQF_ORT(34), 0x000000E3);
-	I40E_WRITE_REG(hw, I40E_GLQF_ORT(35), 0x000000E6);
-
-	/*
 	 * Initialize registers for parsing packet type of QinQ
 	 * This should be removed from code once proper
 	 * configuration API is added to avoid configuration conflicts
diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 3d7170d..8191f72 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -342,6 +342,14 @@  i40e_init_flx_pld(struct i40e_pf *pf)
 	uint16_t flow_type;
 
 	/*
+	 * Force global configuration for flexible payload
+	 * to the first 16 bytes of the corresponding L2/L3/L4 paylod.
+	 */
+	I40E_WRITE_REG(hw, I40E_GLQF_ORT(33), 0x000000E0);
+	I40E_WRITE_REG(hw, I40E_GLQF_ORT(34), 0x000000E3);
+	I40E_WRITE_REG(hw, I40E_GLQF_ORT(35), 0x000000E6);
+
+	/*
 	 * Define the bytes stream extracted as flexible payload in
 	 * field vector. By default, select 8 words from the beginning
 	 * of payload as flexible payload.