[v4,1/3] common/sfc_efx/base: report VLAN stripping capability

Message ID 20230601153022.99634-2-artemii.morozov@arknetworks.am (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series net/sfc: support VLAN stripping offload |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing warning apply patch failure

Commit Message

Artemii Morozov June 1, 2023, 3:30 p.m. UTC
  These changes are necessary in order to add support for stripping
VLAN tags in the future.

Signed-off-by: Artemii Morozov <artemii.morozov@arknetworks.am>
Reviewed-by: Ivan Malov <ivan.malov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
---
 drivers/common/sfc_efx/base/ef10_nic.c  | 6 ++++++
 drivers/common/sfc_efx/base/efx.h       | 1 +
 drivers/common/sfc_efx/base/siena_nic.c | 1 +
 3 files changed, 8 insertions(+)
  

Comments

Andrew Rybchenko June 2, 2023, 7:22 a.m. UTC | #1
On 6/1/23 18:30, Artemii Morozov wrote:
> These changes are necessary in order to add support for stripping
> VLAN tags in the future.
> 
> Signed-off-by: Artemii Morozov <artemii.morozov@arknetworks.am>
> Reviewed-by: Ivan Malov <ivan.malov@arknetworks.am>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> ---
>   drivers/common/sfc_efx/base/ef10_nic.c  | 6 ++++++
>   drivers/common/sfc_efx/base/efx.h       | 1 +
>   drivers/common/sfc_efx/base/siena_nic.c | 1 +
>   3 files changed, 8 insertions(+)
> 
> diff --git a/drivers/common/sfc_efx/base/ef10_nic.c b/drivers/common/sfc_efx/base/ef10_nic.c
> index e1709d1200..501ce2e37c 100644
> --- a/drivers/common/sfc_efx/base/ef10_nic.c
> +++ b/drivers/common/sfc_efx/base/ef10_nic.c
> @@ -1227,6 +1227,12 @@ ef10_get_datapath_caps(
>   	else
>   		encp->enc_init_evq_extended_width_supported = B_FALSE;
>   
> +	/* Check if firmware supports VLAN stripping. */
> +	if (CAP_FLAGS1(req, RX_VLAN_STRIPPING))
> +		encp->enc_rx_vlan_stripping = B_TRUE;
> +	else
> +		encp->enc_rx_vlan_stripping = B_FALSE;
> +

I'd like to understand the logic how the place is chosen.
Here it is put between two EvQ related flags. Why?
Also as far as I can see it is not about alphabetical order.
It is not at the end of the list.
I perfectly realize that the order here is far from ideal,
but chosen place is very-very strange.
Since we have no requirement to put at the end of the list,
it should be nearby TX_VLAN_INSERTION since these flags are
close in MCDI header and logically related.

>   	/*
>   	 * Check if the NO_CONT_EV mode for RX events is supported.
>   	 */
> diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
> index 49e29dcc1c..ac89b418e0 100644
> --- a/drivers/common/sfc_efx/base/efx.h
> +++ b/drivers/common/sfc_efx/base/efx.h
> @@ -1638,6 +1638,7 @@ typedef struct efx_nic_cfg_s {
>   	boolean_t		enc_pm_and_rxdp_counters;
>   	boolean_t		enc_mac_stats_40g_tx_size_bins;
>   	uint32_t		enc_tunnel_encapsulations_supported;
> +	boolean_t		enc_rx_vlan_stripping;

Here it is put between two tunnel related flags. Why?
Basically above thoughts are applicable here as well.
Moreover there is a hole just after enc_hw_tx_insert_vlan_enabled.

Naming is a separate question. It is definitely inconsistent
vs naming of boolean flags in the structure. The majority of
flags are either _enabled or _supported. As I understand it
should be _supported in this case.

IMHO, _hw_ is redundant in enc_hw_tx_insert_vlan_enabled, so,
we don't need it here as well.

>   	/*
>   	 * NIC global maximum for unique UDP tunnel ports shared by all
>   	 * functions.
> diff --git a/drivers/common/sfc_efx/base/siena_nic.c b/drivers/common/sfc_efx/base/siena_nic.c
> index 9f14faf271..451ca81bd7 100644
> --- a/drivers/common/sfc_efx/base/siena_nic.c
> +++ b/drivers/common/sfc_efx/base/siena_nic.c
> @@ -189,6 +189,7 @@ siena_board_cfg(
>   	encp->enc_rx_var_packed_stream_supported = B_FALSE;
>   	encp->enc_rx_es_super_buffer_supported = B_FALSE;
>   	encp->enc_fw_subvariant_no_tx_csum_supported = B_FALSE;
> +	encp->enc_rx_vlan_stripping = B_FALSE;
>   
>   	/* Siena supports two 10G ports, and 8 lanes of PCIe Gen2 */
>   	encp->enc_required_pcie_bandwidth_mbps = 2 * 10000;
  

Patch

diff --git a/drivers/common/sfc_efx/base/ef10_nic.c b/drivers/common/sfc_efx/base/ef10_nic.c
index e1709d1200..501ce2e37c 100644
--- a/drivers/common/sfc_efx/base/ef10_nic.c
+++ b/drivers/common/sfc_efx/base/ef10_nic.c
@@ -1227,6 +1227,12 @@  ef10_get_datapath_caps(
 	else
 		encp->enc_init_evq_extended_width_supported = B_FALSE;
 
+	/* Check if firmware supports VLAN stripping. */
+	if (CAP_FLAGS1(req, RX_VLAN_STRIPPING))
+		encp->enc_rx_vlan_stripping = B_TRUE;
+	else
+		encp->enc_rx_vlan_stripping = B_FALSE;
+
 	/*
 	 * Check if the NO_CONT_EV mode for RX events is supported.
 	 */
diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
index 49e29dcc1c..ac89b418e0 100644
--- a/drivers/common/sfc_efx/base/efx.h
+++ b/drivers/common/sfc_efx/base/efx.h
@@ -1638,6 +1638,7 @@  typedef struct efx_nic_cfg_s {
 	boolean_t		enc_pm_and_rxdp_counters;
 	boolean_t		enc_mac_stats_40g_tx_size_bins;
 	uint32_t		enc_tunnel_encapsulations_supported;
+	boolean_t		enc_rx_vlan_stripping;
 	/*
 	 * NIC global maximum for unique UDP tunnel ports shared by all
 	 * functions.
diff --git a/drivers/common/sfc_efx/base/siena_nic.c b/drivers/common/sfc_efx/base/siena_nic.c
index 9f14faf271..451ca81bd7 100644
--- a/drivers/common/sfc_efx/base/siena_nic.c
+++ b/drivers/common/sfc_efx/base/siena_nic.c
@@ -189,6 +189,7 @@  siena_board_cfg(
 	encp->enc_rx_var_packed_stream_supported = B_FALSE;
 	encp->enc_rx_es_super_buffer_supported = B_FALSE;
 	encp->enc_fw_subvariant_no_tx_csum_supported = B_FALSE;
+	encp->enc_rx_vlan_stripping = B_FALSE;
 
 	/* Siena supports two 10G ports, and 8 lanes of PCIe Gen2 */
 	encp->enc_required_pcie_bandwidth_mbps = 2 * 10000;