From d775155a8661957676bb0a7b363bf1f0da4d8b20 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Tue, 22 Feb 2022 16:26:48 -0800 Subject: [PATCH 01/11] ice: rename ice_sriov.c to ice_vf_mbx.c The ice_sriov.c file primarily contains code which handles the logic for mailbox overflow detection and some other utility functions related to the virtualization mailbox. The bulk of the SR-IOV implementation is actually found in ice_virtchnl_pf.c, and this file isn't strictly SR-IOV specific. In the future, the ice driver will support an additional virtualization scheme known as Scalable IOV, and the code in this file will be used for this alternative implementation. Rename this file (and its associated header) to ice_vf_mbx.c, so that we can later re-use the ice_sriov.c file as the SR-IOV specific file. Signed-off-by: Jacob Keller Tested-by: Konrad Jankowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/Makefile | 2 +- drivers/net/ethernet/intel/ice/ice.h | 2 +- .../net/ethernet/intel/ice/{ice_sriov.c => ice_vf_mbx.c} | 2 +- .../net/ethernet/intel/ice/{ice_sriov.h => ice_vf_mbx.h} | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) rename drivers/net/ethernet/intel/ice/{ice_sriov.c => ice_vf_mbx.c} (99%) rename drivers/net/ethernet/intel/ice/{ice_sriov.h => ice_vf_mbx.h} (95%) diff --git a/drivers/net/ethernet/intel/ice/Makefile b/drivers/net/ethernet/intel/ice/Makefile index 44b8464b76630d..451098e250238f 100644 --- a/drivers/net/ethernet/intel/ice/Makefile +++ b/drivers/net/ethernet/intel/ice/Makefile @@ -36,7 +36,7 @@ ice-y := ice_main.o \ ice-$(CONFIG_PCI_IOV) += \ ice_virtchnl_allowlist.o \ ice_virtchnl_fdir.o \ - ice_sriov.o \ + ice_vf_mbx.o \ ice_vf_vsi_vlan_ops.o \ ice_virtchnl_pf.o ice-$(CONFIG_PTP_1588_CLOCK) += ice_ptp.o ice_ptp_hw.o diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h index 1a130ff562af8a..9449ee2322b7b6 100644 --- a/drivers/net/ethernet/intel/ice/ice.h +++ b/drivers/net/ethernet/intel/ice/ice.h @@ -65,7 +65,7 @@ #include "ice_sched.h" #include "ice_idc_int.h" #include "ice_virtchnl_pf.h" -#include "ice_sriov.h" +#include "ice_vf_mbx.h" #include "ice_ptp.h" #include "ice_fdir.h" #include "ice_xsk.h" diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c similarity index 99% rename from drivers/net/ethernet/intel/ice/ice_sriov.c rename to drivers/net/ethernet/intel/ice/ice_vf_mbx.c index 52c6bac41bf766..fc8c93fa4455e1 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c @@ -2,7 +2,7 @@ /* Copyright (c) 2018, Intel Corporation. */ #include "ice_common.h" -#include "ice_sriov.h" +#include "ice_vf_mbx.h" /** * ice_aq_send_msg_to_vf diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.h b/drivers/net/ethernet/intel/ice/ice_vf_mbx.h similarity index 95% rename from drivers/net/ethernet/intel/ice/ice_sriov.h rename to drivers/net/ethernet/intel/ice/ice_vf_mbx.h index 68686a3fd7e8c4..582716e6d5f98b 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.h +++ b/drivers/net/ethernet/intel/ice/ice_vf_mbx.h @@ -1,8 +1,8 @@ /* SPDX-License-Identifier: GPL-2.0 */ /* Copyright (c) 2018, Intel Corporation. */ -#ifndef _ICE_SRIOV_H_ -#define _ICE_SRIOV_H_ +#ifndef _ICE_VF_MBX_H_ +#define _ICE_VF_MBX_H_ #include "ice_type.h" #include "ice_controlq.h" @@ -49,4 +49,4 @@ ice_conv_link_speed_to_virtchnl(bool __always_unused adv_link_support, } #endif /* CONFIG_PCI_IOV */ -#endif /* _ICE_SRIOV_H_ */ +#endif /* _ICE_VF_MBX_H_ */ From 0deb0bf70c3f15e61d3b14febcde8b8b19ca71d1 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Tue, 22 Feb 2022 16:26:49 -0800 Subject: [PATCH 02/11] ice: rename ice_virtchnl_pf.c to ice_sriov.c The ice_virtchnl_pf.c and ice_virtchnl_pf.h files are where most of the code for implementing Single Root IOV virtualization resides. This code includes support for bringing up and tearing down VFs, hooks into the kernel SR-IOV netdev operations, and for handling virtchnl messages from VFs. In the future, we plan to support Scalable IOV in addition to Single Root IOV as an alternative virtualization scheme. This implementation will re-use some but not all of the code in ice_virtchnl_pf.c To prepare for this future, we want to refactor and split up the code in ice_virtchnl_pf.c into the following scheme: * ice_vf_lib.[ch] Basic VF structures and accessors. This is where scheme-independent code will reside. * ice_virtchnl.[ch] Virtchnl message handling. This is where the bulk of the logic for processing messages from VFs using the virtchnl messaging scheme will reside. This is separated from ice_vf_lib.c because it is distinct and has a bulk of the processing code. * ice_sriov.[ch] Single Root IOV implementation, including initialization and the routines for interacting with SR-IOV based netdev operations. * (future) ice_siov.[ch] Scalable IOV implementation. As a first step, lets assume that all of the code in ice_virtchnl_pf.[ch] is for Single Root IOV. Rename this file to ice_sriov.c and its header to ice_sriov.h Future changes will further split out the code in these files following the plan outlined here. Signed-off-by: Jacob Keller Tested-by: Konrad Jankowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/Makefile | 2 +- drivers/net/ethernet/intel/ice/ice.h | 2 +- drivers/net/ethernet/intel/ice/ice_base.c | 2 +- drivers/net/ethernet/intel/ice/ice_repr.c | 2 +- .../ethernet/intel/ice/{ice_virtchnl_pf.c => ice_sriov.c} | 0 .../ethernet/intel/ice/{ice_virtchnl_pf.h => ice_sriov.h} | 6 +++--- drivers/net/ethernet/intel/ice/ice_vf_vsi_vlan_ops.c | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) rename drivers/net/ethernet/intel/ice/{ice_virtchnl_pf.c => ice_sriov.c} (100%) rename drivers/net/ethernet/intel/ice/{ice_virtchnl_pf.h => ice_sriov.h} (99%) diff --git a/drivers/net/ethernet/intel/ice/Makefile b/drivers/net/ethernet/intel/ice/Makefile index 451098e250238f..816e81832b7f2b 100644 --- a/drivers/net/ethernet/intel/ice/Makefile +++ b/drivers/net/ethernet/intel/ice/Makefile @@ -38,7 +38,7 @@ ice-$(CONFIG_PCI_IOV) += \ ice_virtchnl_fdir.o \ ice_vf_mbx.o \ ice_vf_vsi_vlan_ops.o \ - ice_virtchnl_pf.o + ice_sriov.o ice-$(CONFIG_PTP_1588_CLOCK) += ice_ptp.o ice_ptp_hw.o ice-$(CONFIG_TTY) += ice_gnss.o ice-$(CONFIG_DCB) += ice_dcb.o ice_dcb_nl.o ice_dcb_lib.o diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h index 9449ee2322b7b6..24325df8dc7d4e 100644 --- a/drivers/net/ethernet/intel/ice/ice.h +++ b/drivers/net/ethernet/intel/ice/ice.h @@ -64,7 +64,7 @@ #include "ice_flow.h" #include "ice_sched.h" #include "ice_idc_int.h" -#include "ice_virtchnl_pf.h" +#include "ice_sriov.h" #include "ice_vf_mbx.h" #include "ice_ptp.h" #include "ice_fdir.h" diff --git a/drivers/net/ethernet/intel/ice/ice_base.c b/drivers/net/ethernet/intel/ice/ice_base.c index a3094470d31ded..136d7911adb489 100644 --- a/drivers/net/ethernet/intel/ice/ice_base.c +++ b/drivers/net/ethernet/intel/ice/ice_base.c @@ -5,7 +5,7 @@ #include "ice_base.h" #include "ice_lib.h" #include "ice_dcb_lib.h" -#include "ice_virtchnl_pf.h" +#include "ice_sriov.h" static bool ice_alloc_rx_buf_zc(struct ice_rx_ring *rx_ring) { diff --git a/drivers/net/ethernet/intel/ice/ice_repr.c b/drivers/net/ethernet/intel/ice/ice_repr.c index f8db3ca521da8f..e0be2765756972 100644 --- a/drivers/net/ethernet/intel/ice/ice_repr.c +++ b/drivers/net/ethernet/intel/ice/ice_repr.c @@ -4,7 +4,7 @@ #include "ice.h" #include "ice_eswitch.h" #include "ice_devlink.h" -#include "ice_virtchnl_pf.h" +#include "ice_sriov.h" #include "ice_tc_lib.h" /** diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c b/drivers/net/ethernet/intel/ice/ice_sriov.c similarity index 100% rename from drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c rename to drivers/net/ethernet/intel/ice/ice_sriov.c diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.h b/drivers/net/ethernet/intel/ice/ice_sriov.h similarity index 99% rename from drivers/net/ethernet/intel/ice/ice_virtchnl_pf.h rename to drivers/net/ethernet/intel/ice/ice_sriov.h index 7f16ed9c70d662..bf42f7792d6802 100644 --- a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.h +++ b/drivers/net/ethernet/intel/ice/ice_sriov.h @@ -1,8 +1,8 @@ /* SPDX-License-Identifier: GPL-2.0 */ /* Copyright (c) 2018, Intel Corporation. */ -#ifndef _ICE_VIRTCHNL_PF_H_ -#define _ICE_VIRTCHNL_PF_H_ +#ifndef _ICE_SRIOV_H_ +#define _ICE_SRIOV_H_ #include "ice.h" #include "ice_virtchnl_fdir.h" #include "ice_vsi_vlan_ops.h" @@ -434,4 +434,4 @@ static inline bool ice_vf_is_port_vlan_ena(struct ice_vf __always_unused *vf) return false; } #endif /* CONFIG_PCI_IOV */ -#endif /* _ICE_VIRTCHNL_PF_H_ */ +#endif /* _ICE_SRIOV_H_ */ diff --git a/drivers/net/ethernet/intel/ice/ice_vf_vsi_vlan_ops.c b/drivers/net/ethernet/intel/ice/ice_vf_vsi_vlan_ops.c index b16f946185f240..5ecc0ee9a78e0f 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_vsi_vlan_ops.c +++ b/drivers/net/ethernet/intel/ice/ice_vf_vsi_vlan_ops.c @@ -6,7 +6,7 @@ #include "ice_vlan_mode.h" #include "ice.h" #include "ice_vf_vsi_vlan_ops.h" -#include "ice_virtchnl_pf.h" +#include "ice_sriov.h" static int noop_vlan_arg(struct ice_vsi __always_unused *vsi, From 649c87c6ff52efccf7832a690c0959e012ecce73 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Tue, 22 Feb 2022 16:26:50 -0800 Subject: [PATCH 03/11] ice: remove circular header dependencies on ice.h Several headers in the ice driver include ice.h even though they are themselves included by that header. The most notable of these is ice_common.h, but several other headers also do this. Such a recursive inclusion is problematic as it forces headers to be included in a strict order, otherwise compilation errors can result. The circular inclusions do not trigger an endless loop due to standard header inclusion guards, however other errors can occur. For example, ice_flow.h defines ice_rss_hash_cfg, which is used by ice_sriov.h as part of the definition of ice_vf_hash_ip_ctx. ice_flow.h includes ice_acl.h, which includes ice_common.h, and which finally includes ice.h. Since ice.h itself includes ice_sriov.h, this creates a circular dependency. The definition in ice_sriov.h requires things from ice_flow.h, but ice_flow.h itself will lead to trying to load ice_sriov.h as part of its process for expanding ice.h. The current code avoids this issue by having an implicit dependency without the include of ice_flow.h. If we were to fix that so that ice_sriov.h explicitly depends on ice_flow.h the following pattern would occur: ice_flow.h -> ice_acl.h -> ice_common.h -> ice.h -> ice_sriov.h At this point, during the expansion of, the header guard for ice_flow.h is already set, so when ice_sriov.h attempts to load the ice_flow.h header it is skipped. Then, we go on to begin including the rest of ice_sriov.h, including structure definitions which depend on ice_rss_hash_cfg. This produces a compiler warning because ice_rss_hash_cfg hasn't yet been included. Remember, we're just at the start of ice_flow.h! If the order of headers is incorrect (ice_flow.h is not implicitly loaded first in all files which include ice_sriov.h) then we get the same failure. Removing this recursive inclusion requires fixing a few cases where some headers depended on the header inclusions from ice.h. In addition, a few other changes are also required. Most notably, ice_hw_to_dev is implemented as a macro in ice_osdep.h, which is the likely reason that ice_common.h includes ice.h at all. This macro implementation requires the full definition of ice_pf in order to properly compile. Fix this by moving it to a function declared in ice_main.c, so that we do not require all files to depend on the layout of the ice_pf structure. Note that this change only fixes circular dependencies, but it does not fully resolve all implicit dependencies where one header may depend on the inclusion of another. I tried to fix as many of the implicit dependencies as I noticed, but fixing them all requires a somewhat tedious analysis of each header and attempting to compile it separately. Signed-off-by: Jacob Keller Tested-by: Gurucharan G (A Contingent worker at Intel) Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice.h | 3 --- drivers/net/ethernet/intel/ice/ice_arfs.h | 3 +++ drivers/net/ethernet/intel/ice/ice_common.h | 4 ++-- drivers/net/ethernet/intel/ice/ice_dcb.h | 1 + drivers/net/ethernet/intel/ice/ice_flex_pipe.c | 1 + drivers/net/ethernet/intel/ice/ice_flow.c | 1 + drivers/net/ethernet/intel/ice/ice_flow.h | 2 ++ drivers/net/ethernet/intel/ice/ice_idc_int.h | 1 - drivers/net/ethernet/intel/ice/ice_main.c | 15 +++++++++++++++ drivers/net/ethernet/intel/ice/ice_osdep.h | 11 +++++++++-- drivers/net/ethernet/intel/ice/ice_repr.h | 1 - drivers/net/ethernet/intel/ice/ice_sriov.h | 1 - drivers/net/ethernet/intel/ice/ice_type.h | 1 + drivers/net/ethernet/intel/ice/ice_xsk.h | 1 - 14 files changed, 35 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h index 24325df8dc7d4e..e9aa1fb43c3a85 100644 --- a/drivers/net/ethernet/intel/ice/ice.h +++ b/drivers/net/ethernet/intel/ice/ice.h @@ -52,9 +52,6 @@ #include #include #include -#if IS_ENABLED(CONFIG_DCB) -#include -#endif /* CONFIG_DCB */ #include "ice_devids.h" #include "ice_type.h" #include "ice_txrx.h" diff --git a/drivers/net/ethernet/intel/ice/ice_arfs.h b/drivers/net/ethernet/intel/ice/ice_arfs.h index 80ed76f0cace71..9669ad9bf7b54b 100644 --- a/drivers/net/ethernet/intel/ice/ice_arfs.h +++ b/drivers/net/ethernet/intel/ice/ice_arfs.h @@ -3,6 +3,9 @@ #ifndef _ICE_ARFS_H_ #define _ICE_ARFS_H_ + +#include "ice_fdir.h" + enum ice_arfs_fltr_state { ICE_ARFS_INACTIVE, ICE_ARFS_ACTIVE, diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h index 1efe6b2c32f019..872ea7d2332d0a 100644 --- a/drivers/net/ethernet/intel/ice/ice_common.h +++ b/drivers/net/ethernet/intel/ice/ice_common.h @@ -6,12 +6,12 @@ #include -#include "ice.h" #include "ice_type.h" #include "ice_nvm.h" #include "ice_flex_pipe.h" -#include "ice_switch.h" #include +#include "ice_switch.h" +#include "ice_fdir.h" #define ICE_SQ_SEND_DELAY_TIME_MS 10 #define ICE_SQ_SEND_MAX_EXECUTE 3 diff --git a/drivers/net/ethernet/intel/ice/ice_dcb.h b/drivers/net/ethernet/intel/ice/ice_dcb.h index d73348f279f732..6abf28a142916f 100644 --- a/drivers/net/ethernet/intel/ice/ice_dcb.h +++ b/drivers/net/ethernet/intel/ice/ice_dcb.h @@ -5,6 +5,7 @@ #define _ICE_DCB_H_ #include "ice_type.h" +#include #define ICE_DCBX_STATUS_NOT_STARTED 0 #define ICE_DCBX_STATUS_IN_PROGRESS 1 diff --git a/drivers/net/ethernet/intel/ice/ice_flex_pipe.c b/drivers/net/ethernet/intel/ice/ice_flex_pipe.c index 6a336e8d4e4d6e..c73cdab44f70c9 100644 --- a/drivers/net/ethernet/intel/ice/ice_flex_pipe.c +++ b/drivers/net/ethernet/intel/ice/ice_flex_pipe.c @@ -4,6 +4,7 @@ #include "ice_common.h" #include "ice_flex_pipe.h" #include "ice_flow.h" +#include "ice.h" /* For supporting double VLAN mode, it is necessary to enable or disable certain * boost tcam entries. The metadata labels names that match the following diff --git a/drivers/net/ethernet/intel/ice/ice_flow.c b/drivers/net/ethernet/intel/ice/ice_flow.c index beed4838dcbe8f..ef103e47a8dc20 100644 --- a/drivers/net/ethernet/intel/ice/ice_flow.c +++ b/drivers/net/ethernet/intel/ice/ice_flow.c @@ -3,6 +3,7 @@ #include "ice_common.h" #include "ice_flow.h" +#include /* Describe properties of a protocol header field */ struct ice_flow_field_info { diff --git a/drivers/net/ethernet/intel/ice/ice_flow.h b/drivers/net/ethernet/intel/ice/ice_flow.h index 84b6e4464a2124..b465d27d9b801c 100644 --- a/drivers/net/ethernet/intel/ice/ice_flow.h +++ b/drivers/net/ethernet/intel/ice/ice_flow.h @@ -4,6 +4,8 @@ #ifndef _ICE_FLOW_H_ #define _ICE_FLOW_H_ +#include "ice_flex_type.h" + #define ICE_FLOW_ENTRY_HANDLE_INVAL 0 #define ICE_FLOW_FLD_OFF_INVAL 0xffff diff --git a/drivers/net/ethernet/intel/ice/ice_idc_int.h b/drivers/net/ethernet/intel/ice/ice_idc_int.h index b7796b8aecbde7..4b0c86757df92b 100644 --- a/drivers/net/ethernet/intel/ice/ice_idc_int.h +++ b/drivers/net/ethernet/intel/ice/ice_idc_int.h @@ -5,7 +5,6 @@ #define _ICE_IDC_INT_H_ #include -#include "ice.h" struct ice_pf; diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index 2694acb1aa01fe..61ea670c5cfe01 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -48,6 +48,21 @@ static DEFINE_IDA(ice_aux_ida); DEFINE_STATIC_KEY_FALSE(ice_xdp_locking_key); EXPORT_SYMBOL(ice_xdp_locking_key); +/** + * ice_hw_to_dev - Get device pointer from the hardware structure + * @hw: pointer to the device HW structure + * + * Used to access the device pointer from compilation units which can't easily + * include the definition of struct ice_pf without leading to circular header + * dependencies. + */ +struct device *ice_hw_to_dev(struct ice_hw *hw) +{ + struct ice_pf *pf = container_of(hw, struct ice_pf, hw); + + return &pf->pdev->dev; +} + static struct workqueue_struct *ice_wq; static const struct net_device_ops ice_netdev_safe_mode_ops; static const struct net_device_ops ice_netdev_ops; diff --git a/drivers/net/ethernet/intel/ice/ice_osdep.h b/drivers/net/ethernet/intel/ice/ice_osdep.h index 380e8ae94fc9d4..82bc54fec7f364 100644 --- a/drivers/net/ethernet/intel/ice/ice_osdep.h +++ b/drivers/net/ethernet/intel/ice/ice_osdep.h @@ -5,7 +5,14 @@ #define _ICE_OSDEP_H_ #include +#include +#include #include +#include +#include +#include +#include +#include #ifndef CONFIG_64BIT #include #endif @@ -25,8 +32,8 @@ struct ice_dma_mem { size_t size; }; -#define ice_hw_to_dev(ptr) \ - (&(container_of((ptr), struct ice_pf, hw))->pdev->dev) +struct ice_hw; +struct device *ice_hw_to_dev(struct ice_hw *hw); #ifdef CONFIG_DYNAMIC_DEBUG #define ice_debug(hw, type, fmt, args...) \ diff --git a/drivers/net/ethernet/intel/ice/ice_repr.h b/drivers/net/ethernet/intel/ice/ice_repr.h index 0c77ff050d15d7..378a45bfa25646 100644 --- a/drivers/net/ethernet/intel/ice/ice_repr.h +++ b/drivers/net/ethernet/intel/ice/ice_repr.h @@ -5,7 +5,6 @@ #define _ICE_REPR_H_ #include -#include "ice.h" struct ice_repr { struct ice_vsi *src_vsi; diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.h b/drivers/net/ethernet/intel/ice/ice_sriov.h index bf42f7792d6802..a5ef3c46953a91 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.h +++ b/drivers/net/ethernet/intel/ice/ice_sriov.h @@ -3,7 +3,6 @@ #ifndef _ICE_SRIOV_H_ #define _ICE_SRIOV_H_ -#include "ice.h" #include "ice_virtchnl_fdir.h" #include "ice_vsi_vlan_ops.h" diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h index 28fcab26b8686d..f2a518a1fd9454 100644 --- a/drivers/net/ethernet/intel/ice/ice_type.h +++ b/drivers/net/ethernet/intel/ice/ice_type.h @@ -9,6 +9,7 @@ #define ICE_CHNL_MAX_TC 16 #include "ice_hw_autogen.h" +#include "ice_devids.h" #include "ice_osdep.h" #include "ice_controlq.h" #include "ice_lan_tx_rx.h" diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.h b/drivers/net/ethernet/intel/ice/ice_xsk.h index 123bb98ebfbedf..21faec8e97db1c 100644 --- a/drivers/net/ethernet/intel/ice/ice_xsk.h +++ b/drivers/net/ethernet/intel/ice/ice_xsk.h @@ -4,7 +4,6 @@ #ifndef _ICE_XSK_H_ #define _ICE_XSK_H_ #include "ice_txrx.h" -#include "ice.h" #define PKTS_PER_BATCH 8 From a7e117109a25b381eacf3206178ed9f661a20e83 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Tue, 22 Feb 2022 16:26:51 -0800 Subject: [PATCH 04/11] ice: convert vf->vc_ops to a const pointer The vc_ops structure is used to allow different handlers for virtchnl commands when the driver is in representor mode. The current implementation uses a copy of the ops table in each VF, and modifies this copy dynamically. The usual practice in kernel code is to store the ops table in a constant structure and point to different versions. This has a number of advantages: 1. Reduced memory usage. Each VF merely points to the correct table, so they're able to re-use the same constant lookup table in memory. 2. Consistency. It becomes more difficult to accidentally update or edit only one op call. Instead, the code switches to the correct able by a single pointer write. In general this is atomic, either the pointer is updated or its not. 3. Code Layout. The VF structure can store a pointer to the table without needing to have the full structure definition defined prior to the VF structure definition. This will aid in future refactoring of code by allowing the VF pointer to be kept in ice_vf_lib.h while the virtchnl ops table can be maintained in ice_virtchnl.h There is one major downside in the case of the vc_ops structure. Most of the operations in the table are the same between the two current implementations. This can appear to lead to duplication since each implementation must now fill in the complete table. It could make spotting the differences in the representor mode more challenging. Unfortunately, methods to make this less error prone either add complexity overhead (macros using CPP token concatenation) or don't work on all compilers we support (constant initializer from another constant structure). The cost of maintaining two structures does not out weigh the benefits of the constant table model. While we're making these changes, go ahead and rename the structure and implementations with "virtchnl" instead of "vc_vf_". This will more closely align with the planned file renaming, and avoid similar names when we later introduce a "vf ops" table for separating Scalable IOV and Single Root IOV implementations. Leave the accessor/assignment functions in order to avoid issues with compiling with options disabled. The interface makes it easier to handle when CONFIG_PCI_IOV is disabled in the kernel. Signed-off-by: Jacob Keller Tested-by: Sandeep Penigalapati Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_repr.c | 4 +- drivers/net/ethernet/intel/ice/ice_sriov.c | 61 +++++++++++++++++----- drivers/net/ethernet/intel/ice/ice_sriov.h | 13 +++-- 3 files changed, 55 insertions(+), 23 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_repr.c b/drivers/net/ethernet/intel/ice/ice_repr.c index e0be2765756972..848f2adea563e9 100644 --- a/drivers/net/ethernet/intel/ice/ice_repr.c +++ b/drivers/net/ethernet/intel/ice/ice_repr.c @@ -339,7 +339,7 @@ static int ice_repr_add(struct ice_vf *vf) devlink_port_type_eth_set(&vf->devlink_port, repr->netdev); - ice_vc_change_ops_to_repr(&vf->vc_ops); + ice_virtchnl_set_repr_ops(vf); return 0; @@ -384,7 +384,7 @@ static void ice_repr_rem(struct ice_vf *vf) kfree(vf->repr); vf->repr = NULL; - ice_vc_set_dflt_vf_ops(&vf->vc_ops); + ice_virtchnl_set_dflt_ops(vf); } /** diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c index 45fe36db076aa9..8578317ceb8ac3 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c @@ -2023,7 +2023,7 @@ static int ice_create_vf_entries(struct ice_pf *pf, u16 num_vfs) ice_vf_ctrl_invalidate_vsi(vf); ice_vf_fdir_init(vf); - ice_vc_set_dflt_vf_ops(&vf->vc_ops); + ice_virtchnl_set_dflt_ops(vf); mutex_init(&vf->cfg_lock); @@ -5672,7 +5672,7 @@ static int ice_vc_dis_vlan_insertion_v2_msg(struct ice_vf *vf, u8 *msg) return ice_vc_send_msg_to_vf(vf, VIRTCHNL_OP_DISABLE_VLAN_INSERTION_V2, v_ret, NULL, 0); } -static struct ice_vc_vf_ops ice_vc_vf_dflt_ops = { +static const struct ice_virtchnl_ops ice_virtchnl_dflt_ops = { .get_ver_msg = ice_vc_get_ver_msg, .get_vf_res_msg = ice_vc_get_vf_res_msg, .reset_vf = ice_vc_reset_vf_msg, @@ -5703,9 +5703,13 @@ static struct ice_vc_vf_ops ice_vc_vf_dflt_ops = { .dis_vlan_insertion_v2_msg = ice_vc_dis_vlan_insertion_v2_msg, }; -void ice_vc_set_dflt_vf_ops(struct ice_vc_vf_ops *ops) +/** + * ice_virtchnl_set_dflt_ops - Switch to default virtchnl ops + * @vf: the VF to switch ops + */ +void ice_virtchnl_set_dflt_ops(struct ice_vf *vf) { - *ops = ice_vc_vf_dflt_ops; + vf->virtchnl_ops = &ice_virtchnl_dflt_ops; } /** @@ -5838,15 +5842,44 @@ ice_vc_repr_cfg_promiscuous_mode(struct ice_vf *vf, u8 __always_unused *msg) NULL, 0); } -void ice_vc_change_ops_to_repr(struct ice_vc_vf_ops *ops) +static const struct ice_virtchnl_ops ice_virtchnl_repr_ops = { + .get_ver_msg = ice_vc_get_ver_msg, + .get_vf_res_msg = ice_vc_get_vf_res_msg, + .reset_vf = ice_vc_reset_vf_msg, + .add_mac_addr_msg = ice_vc_repr_add_mac, + .del_mac_addr_msg = ice_vc_repr_del_mac, + .cfg_qs_msg = ice_vc_cfg_qs_msg, + .ena_qs_msg = ice_vc_ena_qs_msg, + .dis_qs_msg = ice_vc_dis_qs_msg, + .request_qs_msg = ice_vc_request_qs_msg, + .cfg_irq_map_msg = ice_vc_cfg_irq_map_msg, + .config_rss_key = ice_vc_config_rss_key, + .config_rss_lut = ice_vc_config_rss_lut, + .get_stats_msg = ice_vc_get_stats_msg, + .cfg_promiscuous_mode_msg = ice_vc_repr_cfg_promiscuous_mode, + .add_vlan_msg = ice_vc_repr_add_vlan, + .remove_vlan_msg = ice_vc_repr_del_vlan, + .ena_vlan_stripping = ice_vc_repr_ena_vlan_stripping, + .dis_vlan_stripping = ice_vc_repr_dis_vlan_stripping, + .handle_rss_cfg_msg = ice_vc_handle_rss_cfg, + .add_fdir_fltr_msg = ice_vc_add_fdir_fltr, + .del_fdir_fltr_msg = ice_vc_del_fdir_fltr, + .get_offload_vlan_v2_caps = ice_vc_get_offload_vlan_v2_caps, + .add_vlan_v2_msg = ice_vc_add_vlan_v2_msg, + .remove_vlan_v2_msg = ice_vc_remove_vlan_v2_msg, + .ena_vlan_stripping_v2_msg = ice_vc_ena_vlan_stripping_v2_msg, + .dis_vlan_stripping_v2_msg = ice_vc_dis_vlan_stripping_v2_msg, + .ena_vlan_insertion_v2_msg = ice_vc_ena_vlan_insertion_v2_msg, + .dis_vlan_insertion_v2_msg = ice_vc_dis_vlan_insertion_v2_msg, +}; + +/** + * ice_virtchnl_set_repr_ops - Switch to representor virtchnl ops + * @vf: the VF to switch ops + */ +void ice_virtchnl_set_repr_ops(struct ice_vf *vf) { - ops->add_mac_addr_msg = ice_vc_repr_add_mac; - ops->del_mac_addr_msg = ice_vc_repr_del_mac; - ops->add_vlan_msg = ice_vc_repr_add_vlan; - ops->remove_vlan_msg = ice_vc_repr_del_vlan; - ops->ena_vlan_stripping = ice_vc_repr_ena_vlan_stripping; - ops->dis_vlan_stripping = ice_vc_repr_dis_vlan_stripping; - ops->cfg_promiscuous_mode_msg = ice_vc_repr_cfg_promiscuous_mode; + vf->virtchnl_ops = &ice_virtchnl_repr_ops; } /** @@ -5861,8 +5894,8 @@ void ice_vc_process_vf_msg(struct ice_pf *pf, struct ice_rq_event_info *event) { u32 v_opcode = le32_to_cpu(event->desc.cookie_high); s16 vf_id = le16_to_cpu(event->desc.retval); + const struct ice_virtchnl_ops *ops; u16 msglen = event->msg_len; - struct ice_vc_vf_ops *ops; u8 *msg = event->msg_buf; struct ice_vf *vf = NULL; struct device *dev; @@ -5883,7 +5916,7 @@ void ice_vc_process_vf_msg(struct ice_pf *pf, struct ice_rq_event_info *event) goto error_handler; } - ops = &vf->vc_ops; + ops = vf->virtchnl_ops; /* Perform basic checks on the msg */ err = virtchnl_vc_validate_vf_msg(&vf->vf_ver, v_opcode, msg, msglen); diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.h b/drivers/net/ethernet/intel/ice/ice_sriov.h index a5ef3c46953a91..b6951d71859228 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.h +++ b/drivers/net/ethernet/intel/ice/ice_sriov.h @@ -113,7 +113,7 @@ struct ice_mdd_vf_events { struct ice_vf; -struct ice_vc_vf_ops { +struct ice_virtchnl_ops { int (*get_ver_msg)(struct ice_vf *vf, u8 *msg); int (*get_vf_res_msg)(struct ice_vf *vf, u8 *msg); void (*reset_vf)(struct ice_vf *vf); @@ -206,8 +206,7 @@ struct ice_vf { DECLARE_BITMAP(opcodes_allowlist, VIRTCHNL_OP_MAX); struct ice_repr *repr; - - struct ice_vc_vf_ops vc_ops; + const struct ice_virtchnl_ops *virtchnl_ops; /* devlink port data */ struct devlink_port devlink_port; @@ -230,8 +229,8 @@ void ice_vc_process_vf_msg(struct ice_pf *pf, struct ice_rq_event_info *event); void ice_vc_notify_link_state(struct ice_pf *pf); void ice_vc_notify_reset(struct ice_pf *pf); void ice_vc_notify_vf_link_state(struct ice_vf *vf); -void ice_vc_change_ops_to_repr(struct ice_vc_vf_ops *ops); -void ice_vc_set_dflt_vf_ops(struct ice_vc_vf_ops *ops); +void ice_virtchnl_set_repr_ops(struct ice_vf *vf); +void ice_virtchnl_set_dflt_ops(struct ice_vf *vf); bool ice_reset_all_vfs(struct ice_pf *pf, bool is_vflr); bool ice_reset_vf(struct ice_vf *vf, bool is_vflr); void ice_restore_all_vfs_msi_state(struct pci_dev *pdev); @@ -303,8 +302,8 @@ void ice_vc_process_vf_msg(struct ice_pf *pf, struct ice_rq_event_info *event) { static inline void ice_vc_notify_link_state(struct ice_pf *pf) { } static inline void ice_vc_notify_reset(struct ice_pf *pf) { } static inline void ice_vc_notify_vf_link_state(struct ice_vf *vf) { } -static inline void ice_vc_change_ops_to_repr(struct ice_vc_vf_ops *ops) { } -static inline void ice_vc_set_dflt_vf_ops(struct ice_vc_vf_ops *ops) { } +static inline void ice_virtchnl_set_repr_ops(struct ice_vf *vf) { } +static inline void ice_virtchnl_set_dflt_ops(struct ice_vf *vf) { } static inline void ice_set_vf_state_qs_dis(struct ice_vf *vf) { } static inline void ice_vf_lan_overflow_event(struct ice_pf *pf, struct ice_rq_event_info *event) { } From 00a57e2959bd74e87a0e21dce746762d2f9c1987 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Tue, 22 Feb 2022 16:26:52 -0800 Subject: [PATCH 05/11] ice: remove unused definitions from ice_sriov.h A few more macros exist in ice_sriov.h which are not used anywhere. These can be safely removed. Note that ICE_VIRTCHNL_VF_CAP_L2 capability is set but never checked anywhere in the driver. Thus it is also safe to remove. Signed-off-by: Jacob Keller Tested-by: Konrad Jankowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_sriov.c | 1 - drivers/net/ethernet/intel/ice/ice_sriov.h | 7 +------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c index 8578317ceb8ac3..205d7e5003d882 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c @@ -2012,7 +2012,6 @@ static int ice_create_vf_entries(struct ice_pf *pf, u16 num_vfs) vf->vf_sw_id = pf->first_sw; /* assign default capabilities */ - set_bit(ICE_VIRTCHNL_VF_CAP_L2, &vf->vf_caps); vf->spoofchk = true; vf->num_vf_qs = pf->vfs.num_qps_per; ice_vc_set_default_allowlist(vf); diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.h b/drivers/net/ethernet/intel/ice/ice_sriov.h index b6951d71859228..699690c1f6a0a7 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.h +++ b/drivers/net/ethernet/intel/ice/ice_sriov.h @@ -13,9 +13,6 @@ */ #define ICE_MAX_MACADDR_PER_VF 18 -/* Malicious Driver Detection */ -#define ICE_MDD_EVENTS_THRESHOLD 30 - /* Static VF transaction/status register def */ #define VF_DEVICE_STATUS 0xAA #define VF_TRANS_PENDING_M 0x20 @@ -28,7 +25,6 @@ #define ICE_MAX_VF_COUNT 256 #define ICE_MIN_QS_PER_VF 1 #define ICE_NONQ_VECS_VF 1 -#define ICE_MAX_SCATTER_QS_PER_VF 16 #define ICE_MAX_RSS_QS_PER_VF 16 #define ICE_NUM_VF_MSIX_MED 17 #define ICE_NUM_VF_MSIX_SMALL 5 @@ -95,8 +91,7 @@ enum ice_vf_states { /* VF capabilities */ enum ice_virtchnl_cap { - ICE_VIRTCHNL_VF_CAP_L2 = 0, - ICE_VIRTCHNL_VF_CAP_PRIVILEGE, + ICE_VIRTCHNL_VF_CAP_PRIVILEGE = 0, }; struct ice_time_mac { From dc36796eadcae0e239fc4f42cafd12f46290fbdd Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Tue, 22 Feb 2022 16:26:53 -0800 Subject: [PATCH 06/11] ice: rename ICE_MAX_VF_COUNT to avoid confusion The ICE_MAX_VF_COUNT field is defined in ice_sriov.h. This count is true for SR-IOV but will not be true for all VF implementations, such as when the ice driver supports Scalable IOV. Rename this definition to clearly indicate ICE_MAX_SRIOV_VFS. Signed-off-by: Jacob Keller Tested-by: Konrad Jankowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_main.c | 2 +- drivers/net/ethernet/intel/ice/ice_sriov.c | 8 ++++---- drivers/net/ethernet/intel/ice/ice_sriov.h | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index 61ea670c5cfe01..416914452ece4e 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -3754,7 +3754,7 @@ static void ice_set_pf_caps(struct ice_pf *pf) if (func_caps->common_cap.sr_iov_1_1) { set_bit(ICE_FLAG_SRIOV_CAPABLE, pf->flags); pf->vfs.num_supported = min_t(int, func_caps->num_allocd_vfs, - ICE_MAX_VF_COUNT); + ICE_MAX_SRIOV_VFS); } clear_bit(ICE_FLAG_RSS_ENA, pf->flags); if (func_caps->common_cap.rss_table_size) diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c index 205d7e5003d882..7cd910bb7a7a82 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c @@ -661,7 +661,7 @@ void ice_free_vfs(struct ice_pf *pf) /* clear malicious info since the VF is getting released */ if (ice_mbx_clear_malvf(&hw->mbx_snapshot, pf->vfs.malvfs, - ICE_MAX_VF_COUNT, vf->vf_id)) + ICE_MAX_SRIOV_VFS, vf->vf_id)) dev_dbg(dev, "failed to clear malicious VF state for VF %u\n", vf->vf_id); @@ -1591,7 +1591,7 @@ bool ice_reset_all_vfs(struct ice_pf *pf, bool is_vflr) /* clear all malicious info if the VFs are getting reset */ ice_for_each_vf(pf, bkt, vf) if (ice_mbx_clear_malvf(&hw->mbx_snapshot, pf->vfs.malvfs, - ICE_MAX_VF_COUNT, vf->vf_id)) + ICE_MAX_SRIOV_VFS, vf->vf_id)) dev_dbg(dev, "failed to clear malicious VF state for VF %u\n", vf->vf_id); @@ -1805,7 +1805,7 @@ bool ice_reset_vf(struct ice_vf *vf, bool is_vflr) /* if the VF has been reset allow it to come up again */ if (ice_mbx_clear_malvf(&hw->mbx_snapshot, pf->vfs.malvfs, - ICE_MAX_VF_COUNT, vf->vf_id)) + ICE_MAX_SRIOV_VFS, vf->vf_id)) dev_dbg(dev, "failed to clear malicious VF state for VF %u\n", i); return true; @@ -6624,7 +6624,7 @@ ice_is_malicious_vf(struct ice_pf *pf, struct ice_rq_event_info *event, * know about it, then let them know now */ status = ice_mbx_report_malvf(&pf->hw, pf->vfs.malvfs, - ICE_MAX_VF_COUNT, vf_id, + ICE_MAX_SRIOV_VFS, vf_id, &report_vf); if (status) dev_dbg(dev, "Error reporting malicious VF\n"); diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.h b/drivers/net/ethernet/intel/ice/ice_sriov.h index 699690c1f6a0a7..b40e74cfb69461 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.h +++ b/drivers/net/ethernet/intel/ice/ice_sriov.h @@ -22,7 +22,7 @@ #define ICE_PCI_CIAD_WAIT_DELAY_US 1 /* VF resource constraints */ -#define ICE_MAX_VF_COUNT 256 +#define ICE_MAX_SRIOV_VFS 256 #define ICE_MIN_QS_PER_VF 1 #define ICE_NONQ_VECS_VF 1 #define ICE_MAX_RSS_QS_PER_VF 16 @@ -147,7 +147,7 @@ struct ice_vfs { u16 num_qps_per; /* number of queue pairs per VF */ u16 num_msix_per; /* number of MSI-X vectors per VF */ unsigned long last_printed_mdd_jiffies; /* MDD message rate limit */ - DECLARE_BITMAP(malvfs, ICE_MAX_VF_COUNT); /* malicious VF indicator */ + DECLARE_BITMAP(malvfs, ICE_MAX_SRIOV_VFS); /* malicious VF indicator */ }; /* VF information structure */ From a8ea6d86bd9831bf77577900d5e324070f23387e Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Tue, 22 Feb 2022 16:26:54 -0800 Subject: [PATCH 07/11] ice: refactor spoofchk control code in ice_sriov.c The API to control the VSI spoof checking for a VF VSI has three functions: enable, disable, and set. The set function takes the VSI and the VF and decides whether to call enable or disable based on the vf->spoofchk field. In some flows, vf->spoofchk is not yet set, such as the function used to control the setting for a VF. (vf->spoofchk is only updated after a success). Simplify this API by refactoring ice_vf_set_spoofchk_cfg to be "ice_vsi_apply_spoofchk" which takes the boolean and allows all callers to avoid having to determine whether to call enable or disable themselves. This matches the expected callers better, and will prevent the need to export more than one function when this code must be called from another file. Signed-off-by: Jacob Keller Tested-by: Konrad Jankowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_sriov.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c index 7cd910bb7a7a82..8d22b5d947065f 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c @@ -985,16 +985,15 @@ static int ice_vsi_dis_spoofchk(struct ice_vsi *vsi) } /** - * ice_vf_set_spoofchk_cfg - apply Tx spoof checking setting - * @vf: VF set spoofchk for + * ice_vsi_apply_spoofchk - Apply Tx spoof checking setting to a VSI * @vsi: VSI associated to the VF + * @enable: whether to enable or disable the spoof checking */ -static int -ice_vf_set_spoofchk_cfg(struct ice_vf *vf, struct ice_vsi *vsi) +static int ice_vsi_apply_spoofchk(struct ice_vsi *vsi, bool enable) { int err; - if (vf->spoofchk) + if (enable) err = ice_vsi_ena_spoofchk(vsi); else err = ice_vsi_dis_spoofchk(vsi); @@ -1478,7 +1477,7 @@ static void ice_vf_rebuild_host_cfg(struct ice_vf *vf) dev_err(dev, "failed to rebuild Tx rate limiting configuration for VF %u\n", vf->vf_id); - if (ice_vf_set_spoofchk_cfg(vf, vsi)) + if (ice_vsi_apply_spoofchk(vsi, vf->spoofchk)) dev_err(dev, "failed to rebuild spoofchk configuration for VF %d\n", vf->vf_id); @@ -1915,7 +1914,7 @@ static int ice_init_vf_vsi_res(struct ice_vf *vf) goto release_vsi; } - err = ice_vf_set_spoofchk_cfg(vf, vsi); + err = ice_vsi_apply_spoofchk(vsi, vf->spoofchk); if (err) { dev_warn(dev, "Failed to initialize spoofchk setting for VF %d\n", vf->vf_id); @@ -3129,10 +3128,7 @@ int ice_set_vf_spoofchk(struct net_device *netdev, int vf_id, bool ena) goto out_put_vf; } - if (ena) - ret = ice_vsi_ena_spoofchk(vf_vsi); - else - ret = ice_vsi_dis_spoofchk(vf_vsi); + ret = ice_vsi_apply_spoofchk(vf_vsi, ena); if (ret) dev_err(dev, "Failed to set spoofchk %s for VF %d VSI %d\n error %d\n", ena ? "ON" : "OFF", vf->vf_id, vf_vsi->vsi_num, ret); From 346f7aa3c7733d5dcbfd31d439822a22fcfedac9 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Tue, 22 Feb 2022 16:26:55 -0800 Subject: [PATCH 08/11] ice: move ice_set_vf_port_vlan near other .ndo ops The ice_set_vf_port_vlan function is located in ice_sriov.c very far away from the other .ndo operations that it is similar to. Move this so that its located near the other .ndo operation definitions. Signed-off-by: Jacob Keller Tested-by: Konrad Jankowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_sriov.c | 192 ++++++++++----------- 1 file changed, 96 insertions(+), 96 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c index 8d22b5d947065f..eebff1824be229 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c @@ -4249,102 +4249,6 @@ static int ice_vc_request_qs_msg(struct ice_vf *vf, u8 *msg) v_ret, (u8 *)vfres, sizeof(*vfres)); } -/** - * ice_is_supported_port_vlan_proto - make sure the vlan_proto is supported - * @hw: hardware structure used to check the VLAN mode - * @vlan_proto: VLAN TPID being checked - * - * If the device is configured in Double VLAN Mode (DVM), then both ETH_P_8021Q - * and ETH_P_8021AD are supported. If the device is configured in Single VLAN - * Mode (SVM), then only ETH_P_8021Q is supported. - */ -static bool -ice_is_supported_port_vlan_proto(struct ice_hw *hw, u16 vlan_proto) -{ - bool is_supported = false; - - switch (vlan_proto) { - case ETH_P_8021Q: - is_supported = true; - break; - case ETH_P_8021AD: - if (ice_is_dvm_ena(hw)) - is_supported = true; - break; - } - - return is_supported; -} - -/** - * ice_set_vf_port_vlan - * @netdev: network interface device structure - * @vf_id: VF identifier - * @vlan_id: VLAN ID being set - * @qos: priority setting - * @vlan_proto: VLAN protocol - * - * program VF Port VLAN ID and/or QoS - */ -int -ice_set_vf_port_vlan(struct net_device *netdev, int vf_id, u16 vlan_id, u8 qos, - __be16 vlan_proto) -{ - struct ice_pf *pf = ice_netdev_to_pf(netdev); - u16 local_vlan_proto = ntohs(vlan_proto); - struct device *dev; - struct ice_vf *vf; - int ret; - - dev = ice_pf_to_dev(pf); - - if (vlan_id >= VLAN_N_VID || qos > 7) { - dev_err(dev, "Invalid Port VLAN parameters for VF %d, ID %d, QoS %d\n", - vf_id, vlan_id, qos); - return -EINVAL; - } - - if (!ice_is_supported_port_vlan_proto(&pf->hw, local_vlan_proto)) { - dev_err(dev, "VF VLAN protocol 0x%04x is not supported\n", - local_vlan_proto); - return -EPROTONOSUPPORT; - } - - vf = ice_get_vf_by_id(pf, vf_id); - if (!vf) - return -EINVAL; - - ret = ice_check_vf_ready_for_cfg(vf); - if (ret) - goto out_put_vf; - - if (ice_vf_get_port_vlan_prio(vf) == qos && - ice_vf_get_port_vlan_tpid(vf) == local_vlan_proto && - ice_vf_get_port_vlan_id(vf) == vlan_id) { - /* duplicate request, so just return success */ - dev_dbg(dev, "Duplicate port VLAN %u, QoS %u, TPID 0x%04x request\n", - vlan_id, qos, local_vlan_proto); - ret = 0; - goto out_put_vf; - } - - mutex_lock(&vf->cfg_lock); - - vf->port_vlan_info = ICE_VLAN(local_vlan_proto, vlan_id, qos); - if (ice_vf_is_port_vlan_ena(vf)) - dev_info(dev, "Setting VLAN %u, QoS %u, TPID 0x%04x on VF %d\n", - vlan_id, qos, local_vlan_proto, vf_id); - else - dev_info(dev, "Clearing port VLAN on VF %d\n", vf_id); - - ice_vc_reset_vf(vf); - mutex_unlock(&vf->cfg_lock); - -out_put_vf: - ice_put_vf(vf); - return ret; -} - /** * ice_vf_vlan_offload_ena - determine if capabilities support VLAN offloads * @caps: VF driver negotiated capabilities @@ -6483,6 +6387,102 @@ int ice_get_vf_stats(struct net_device *netdev, int vf_id, return ret; } +/** + * ice_is_supported_port_vlan_proto - make sure the vlan_proto is supported + * @hw: hardware structure used to check the VLAN mode + * @vlan_proto: VLAN TPID being checked + * + * If the device is configured in Double VLAN Mode (DVM), then both ETH_P_8021Q + * and ETH_P_8021AD are supported. If the device is configured in Single VLAN + * Mode (SVM), then only ETH_P_8021Q is supported. + */ +static bool +ice_is_supported_port_vlan_proto(struct ice_hw *hw, u16 vlan_proto) +{ + bool is_supported = false; + + switch (vlan_proto) { + case ETH_P_8021Q: + is_supported = true; + break; + case ETH_P_8021AD: + if (ice_is_dvm_ena(hw)) + is_supported = true; + break; + } + + return is_supported; +} + +/** + * ice_set_vf_port_vlan + * @netdev: network interface device structure + * @vf_id: VF identifier + * @vlan_id: VLAN ID being set + * @qos: priority setting + * @vlan_proto: VLAN protocol + * + * program VF Port VLAN ID and/or QoS + */ +int +ice_set_vf_port_vlan(struct net_device *netdev, int vf_id, u16 vlan_id, u8 qos, + __be16 vlan_proto) +{ + struct ice_pf *pf = ice_netdev_to_pf(netdev); + u16 local_vlan_proto = ntohs(vlan_proto); + struct device *dev; + struct ice_vf *vf; + int ret; + + dev = ice_pf_to_dev(pf); + + if (vlan_id >= VLAN_N_VID || qos > 7) { + dev_err(dev, "Invalid Port VLAN parameters for VF %d, ID %d, QoS %d\n", + vf_id, vlan_id, qos); + return -EINVAL; + } + + if (!ice_is_supported_port_vlan_proto(&pf->hw, local_vlan_proto)) { + dev_err(dev, "VF VLAN protocol 0x%04x is not supported\n", + local_vlan_proto); + return -EPROTONOSUPPORT; + } + + vf = ice_get_vf_by_id(pf, vf_id); + if (!vf) + return -EINVAL; + + ret = ice_check_vf_ready_for_cfg(vf); + if (ret) + goto out_put_vf; + + if (ice_vf_get_port_vlan_prio(vf) == qos && + ice_vf_get_port_vlan_tpid(vf) == local_vlan_proto && + ice_vf_get_port_vlan_id(vf) == vlan_id) { + /* duplicate request, so just return success */ + dev_dbg(dev, "Duplicate port VLAN %u, QoS %u, TPID 0x%04x request\n", + vlan_id, qos, local_vlan_proto); + ret = 0; + goto out_put_vf; + } + + mutex_lock(&vf->cfg_lock); + + vf->port_vlan_info = ICE_VLAN(local_vlan_proto, vlan_id, qos); + if (ice_vf_is_port_vlan_ena(vf)) + dev_info(dev, "Setting VLAN %u, QoS %u, TPID 0x%04x on VF %d\n", + vlan_id, qos, local_vlan_proto, vf_id); + else + dev_info(dev, "Clearing port VLAN on VF %d\n", vf_id); + + ice_vc_reset_vf(vf); + mutex_unlock(&vf->cfg_lock); + +out_put_vf: + ice_put_vf(vf); + return ret; +} + /** * ice_print_vf_rx_mdd_event - print VF Rx malicious driver detect event * @vf: pointer to the VF structure From 94ab2488d467e69d1216bad97a5839afb3b805c3 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Tue, 22 Feb 2022 16:26:56 -0800 Subject: [PATCH 09/11] ice: cleanup error logging for ice_ena_vfs The ice_ena_vfs function and some of its sub-functions like ice_set_per_vf_res use a "if () { ; }" flow. This flow discards specialized errors reported by the called function. This style is generally not preferred as it makes tracing error sources more difficult. It also means we cannot log the actual error received properly. Refactor several calls in the ice_ena_vfs function that do this to catch the error in the 'ret' variable. Report this in the messages, and then return the more precise error value. Doing this reveals that ice_set_per_vf_res returns -EINVAL or -EIO in places where -ENOSPC makes more sense. Fix these calls up to return the more appropriate value. Signed-off-by: Jacob Keller Tested-by: Konrad Jankowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_sriov.c | 32 +++++++++++++--------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c index eebff1824be229..b695d479dfb170 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c @@ -1275,12 +1275,16 @@ static int ice_set_per_vf_res(struct ice_pf *pf, u16 num_vfs) u16 num_msix_per_vf, num_txq, num_rxq, avail_qs; int msix_avail_per_vf, msix_avail_for_sriov; struct device *dev = ice_pf_to_dev(pf); + int err; lockdep_assert_held(&pf->vfs.table_lock); - if (!num_vfs || max_valid_res_idx < 0) + if (!num_vfs) return -EINVAL; + if (max_valid_res_idx < 0) + return -ENOSPC; + /* determine MSI-X resources per VF */ msix_avail_for_sriov = pf->hw.func_caps.common_cap.num_msix_vectors - pf->irq_tracker->num_entries; @@ -1297,7 +1301,7 @@ static int ice_set_per_vf_res(struct ice_pf *pf, u16 num_vfs) dev_err(dev, "Only %d MSI-X interrupts available for SR-IOV. Not enough to support minimum of %d MSI-X interrupts per VF for %d VFs\n", msix_avail_for_sriov, ICE_MIN_INTR_PER_VF, num_vfs); - return -EIO; + return -ENOSPC; } num_txq = min_t(u16, num_msix_per_vf - ICE_NONQ_VECS_VF, @@ -1319,13 +1323,14 @@ static int ice_set_per_vf_res(struct ice_pf *pf, u16 num_vfs) if (num_txq < ICE_MIN_QS_PER_VF || num_rxq < ICE_MIN_QS_PER_VF) { dev_err(dev, "Not enough queues to support minimum of %d queue pairs per VF for %d VFs\n", ICE_MIN_QS_PER_VF, num_vfs); - return -EIO; + return -ENOSPC; } - if (ice_sriov_set_msix_res(pf, num_msix_per_vf * num_vfs)) { - dev_err(dev, "Unable to set MSI-X resources for %d VFs\n", - num_vfs); - return -EINVAL; + err = ice_sriov_set_msix_res(pf, num_msix_per_vf * num_vfs); + if (err) { + dev_err(dev, "Unable to set MSI-X resources for %d VFs, err %d\n", + num_vfs, err); + return err; } /* only allow equal Tx/Rx queue count (i.e. queue pairs) */ @@ -2058,10 +2063,10 @@ static int ice_ena_vfs(struct ice_pf *pf, u16 num_vfs) mutex_lock(&pf->vfs.table_lock); - if (ice_set_per_vf_res(pf, num_vfs)) { - dev_err(dev, "Not enough resources for %d VFs, try with fewer number of VFs\n", - num_vfs); - ret = -ENOSPC; + ret = ice_set_per_vf_res(pf, num_vfs); + if (ret) { + dev_err(dev, "Not enough resources for %d VFs, err %d. Try with fewer number of VFs\n", + num_vfs, ret); goto err_unroll_sriov; } @@ -2072,8 +2077,9 @@ static int ice_ena_vfs(struct ice_pf *pf, u16 num_vfs) goto err_unroll_sriov; } - if (ice_start_vfs(pf)) { - dev_err(dev, "Failed to start VF(s)\n"); + ret = ice_start_vfs(pf); + if (ret) { + dev_err(dev, "Failed to start %d VFs, err %d\n", num_vfs, ret); ret = -EAGAIN; goto err_unroll_vf_entries; } From 2b36944810b2270e4af8524ab14d42250a902510 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Tue, 22 Feb 2022 16:26:57 -0800 Subject: [PATCH 10/11] ice: log an error message when eswitch fails to configure When ice_eswitch_configure fails, print an error message to make it more obvious why VF initialization did not succeed. Signed-off-by: Jacob Keller Tested-by: Sandeep Penigalapati Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_sriov.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c index b695d479dfb170..d41fce16ddfb83 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c @@ -2087,8 +2087,10 @@ static int ice_ena_vfs(struct ice_pf *pf, u16 num_vfs) clear_bit(ICE_VF_DIS, pf->state); ret = ice_eswitch_configure(pf); - if (ret) + if (ret) { + dev_err(dev, "Failed to configure eswitch, err %d\n", ret); goto err_unroll_sriov; + } /* rearm global interrupts */ if (test_and_clear_bit(ICE_OICR_INTR_DIS, pf->state)) From 1261691dda6b7c229a01cbcb5ea9070d62435722 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Tue, 22 Feb 2022 16:26:58 -0800 Subject: [PATCH 11/11] ice: use ice_is_vf_trusted helper function The ice_vc_cfg_promiscuous_mode_msg function directly checks ICE_VIRTCHNL_VF_CAP_PRIVILEGE, instead of using the existing helper function ice_is_vf_trusted. Switch this to use the helper function so that all trusted checks are consistent. This aids in any potential future refactor by ensuring consistent code. Signed-off-by: Jacob Keller Tested-by: Konrad Jankowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_sriov.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c index d41fce16ddfb83..432841ab43524f 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c @@ -3148,6 +3148,15 @@ int ice_set_vf_spoofchk(struct net_device *netdev, int vf_id, bool ena) return ret; } +/** + * ice_is_vf_trusted + * @vf: pointer to the VF info + */ +static bool ice_is_vf_trusted(struct ice_vf *vf) +{ + return test_bit(ICE_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps); +} + /** * ice_is_any_vf_in_promisc - check if any VF(s) are in promiscuous mode * @pf: PF structure for accessing VF(s) @@ -3212,7 +3221,7 @@ static int ice_vc_cfg_promiscuous_mode_msg(struct ice_vf *vf, u8 *msg) } dev = ice_pf_to_dev(pf); - if (!test_bit(ICE_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps)) { + if (!ice_is_vf_trusted(vf)) { dev_err(dev, "Unprivileged VF %d is attempting to configure promiscuous mode\n", vf->vf_id); /* Leave v_ret alone, lie to the VF on purpose. */ @@ -3862,15 +3871,6 @@ static int ice_vc_cfg_qs_msg(struct ice_vf *vf, u8 *msg) NULL, 0); } -/** - * ice_is_vf_trusted - * @vf: pointer to the VF info - */ -static bool ice_is_vf_trusted(struct ice_vf *vf) -{ - return test_bit(ICE_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps); -} - /** * ice_can_vf_change_mac * @vf: pointer to the VF info