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.moro...@arknetworks.am>
Reviewed-by: Ivan Malov <ivan.ma...@arknetworks.am>
Reviewed-by: Andy Moreton <amore...@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;
