* [Intel-wired-lan] [PATCH iwl-next 1/4] ice: pass VSI pointer into ice_vc_isvalid_q_id
2024-02-16 22:06 [Intel-wired-lan] [PATCH iwl-next 0/4] ice: use relative VSI index for VFs VSIs Jacob Keller
@ 2024-02-16 22:06 ` Jacob Keller
2024-03-01 15:19 ` Romanowski, Rafal
2024-02-16 22:06 ` [Intel-wired-lan] [PATCH iwl-next 2/4] ice: remove unnecessary duplicate checks for VF VSI ID Jacob Keller
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Jacob Keller @ 2024-02-16 22:06 UTC (permalink / raw)
To: Anthony Nguyen, Intel Wired LAN; +Cc: Jacob Keller, Przemek Kitszel
The ice_vc_isvalid_q_id() function takes a VSI index and a queue ID. It
looks up the VSI from its index, and then validates that the queue number
is valid for that VSI.
The VSI ID passed is typically a VSI index from the VF. This VSI number is
validated by the PF to ensure that it matches the VSI associated with the
VF already.
In every flow where ice_vc_isvalid_q_id() is called, the PF driver already
has a pointer to the VSI associated with the VF. This pointer is obtained
using ice_get_vf_vsi(), rather than looking up the VSI using the index sent
by the VF.
Since we already know which VSI to operate on, we can modify
ice_vc_isvalid_q_id() to take a VSI pointer instead of a VSI index. Pass
the VSI we found from ice_get_vf_vsi() instead of re-doing the lookup. This
removes some unnecessary computation and scanning of the VSI list.
It also removes the last place where the driver directly used the VSI
number from the VF. This will pave the way for refactoring to communicate
relative VSI numbers to the VF instead of absolute numbers from the PF
space.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
drivers/net/ethernet/intel/ice/ice_virtchnl.c | 22 +++++++++----------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
index 6f2328a049bf..29449030174f 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
@@ -555,17 +555,15 @@ bool ice_vc_isvalid_vsi_id(struct ice_vf *vf, u16 vsi_id)
/**
* ice_vc_isvalid_q_id
- * @vf: pointer to the VF info
- * @vsi_id: VSI ID
+ * @vsi: VSI to check queue ID against
* @qid: VSI relative queue ID
*
* check for the valid queue ID
*/
-static bool ice_vc_isvalid_q_id(struct ice_vf *vf, u16 vsi_id, u8 qid)
+static bool ice_vc_isvalid_q_id(struct ice_vsi *vsi, u8 qid)
{
- struct ice_vsi *vsi = ice_find_vsi(vf->pf, vsi_id);
/* allocated Tx and Rx queues should be always equal for VF VSI */
- return (vsi && (qid < vsi->alloc_txq));
+ return qid < vsi->alloc_txq;
}
/**
@@ -1323,7 +1321,7 @@ static int ice_vc_ena_qs_msg(struct ice_vf *vf, u8 *msg)
*/
q_map = vqs->rx_queues;
for_each_set_bit(vf_q_id, &q_map, ICE_MAX_RSS_QS_PER_VF) {
- if (!ice_vc_isvalid_q_id(vf, vqs->vsi_id, vf_q_id)) {
+ if (!ice_vc_isvalid_q_id(vsi, vf_q_id)) {
v_ret = VIRTCHNL_STATUS_ERR_PARAM;
goto error_param;
}
@@ -1345,7 +1343,7 @@ static int ice_vc_ena_qs_msg(struct ice_vf *vf, u8 *msg)
q_map = vqs->tx_queues;
for_each_set_bit(vf_q_id, &q_map, ICE_MAX_RSS_QS_PER_VF) {
- if (!ice_vc_isvalid_q_id(vf, vqs->vsi_id, vf_q_id)) {
+ if (!ice_vc_isvalid_q_id(vsi, vf_q_id)) {
v_ret = VIRTCHNL_STATUS_ERR_PARAM;
goto error_param;
}
@@ -1450,7 +1448,7 @@ static int ice_vc_dis_qs_msg(struct ice_vf *vf, u8 *msg)
q_map = vqs->tx_queues;
for_each_set_bit(vf_q_id, &q_map, ICE_MAX_RSS_QS_PER_VF) {
- if (!ice_vc_isvalid_q_id(vf, vqs->vsi_id, vf_q_id)) {
+ if (!ice_vc_isvalid_q_id(vsi, vf_q_id)) {
v_ret = VIRTCHNL_STATUS_ERR_PARAM;
goto error_param;
}
@@ -1476,7 +1474,7 @@ static int ice_vc_dis_qs_msg(struct ice_vf *vf, u8 *msg)
bitmap_zero(vf->rxq_ena, ICE_MAX_RSS_QS_PER_VF);
} else if (q_map) {
for_each_set_bit(vf_q_id, &q_map, ICE_MAX_RSS_QS_PER_VF) {
- if (!ice_vc_isvalid_q_id(vf, vqs->vsi_id, vf_q_id)) {
+ if (!ice_vc_isvalid_q_id(vsi, vf_q_id)) {
v_ret = VIRTCHNL_STATUS_ERR_PARAM;
goto error_param;
}
@@ -1532,7 +1530,7 @@ ice_cfg_interrupt(struct ice_vf *vf, struct ice_vsi *vsi, u16 vector_id,
for_each_set_bit(vsi_q_id_idx, &qmap, ICE_MAX_RSS_QS_PER_VF) {
vsi_q_id = vsi_q_id_idx;
- if (!ice_vc_isvalid_q_id(vf, vsi->vsi_num, vsi_q_id))
+ if (!ice_vc_isvalid_q_id(vsi, vsi_q_id))
return VIRTCHNL_STATUS_ERR_PARAM;
q_vector->num_ring_rx++;
@@ -1546,7 +1544,7 @@ ice_cfg_interrupt(struct ice_vf *vf, struct ice_vsi *vsi, u16 vector_id,
for_each_set_bit(vsi_q_id_idx, &qmap, ICE_MAX_RSS_QS_PER_VF) {
vsi_q_id = vsi_q_id_idx;
- if (!ice_vc_isvalid_q_id(vf, vsi->vsi_num, vsi_q_id))
+ if (!ice_vc_isvalid_q_id(vsi, vsi_q_id))
return VIRTCHNL_STATUS_ERR_PARAM;
q_vector->num_ring_tx++;
@@ -1703,7 +1701,7 @@ static int ice_vc_cfg_qs_msg(struct ice_vf *vf, u8 *msg)
qpi->txq.headwb_enabled ||
!ice_vc_isvalid_ring_len(qpi->txq.ring_len) ||
!ice_vc_isvalid_ring_len(qpi->rxq.ring_len) ||
- !ice_vc_isvalid_q_id(vf, qci->vsi_id, qpi->txq.queue_id)) {
+ !ice_vc_isvalid_q_id(vsi, qpi->txq.queue_id)) {
goto error_param;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Intel-wired-lan] [PATCH iwl-next 1/4] ice: pass VSI pointer into ice_vc_isvalid_q_id
2024-02-16 22:06 ` [Intel-wired-lan] [PATCH iwl-next 1/4] ice: pass VSI pointer into ice_vc_isvalid_q_id Jacob Keller
@ 2024-03-01 15:19 ` Romanowski, Rafal
0 siblings, 0 replies; 10+ messages in thread
From: Romanowski, Rafal @ 2024-03-01 15:19 UTC (permalink / raw)
To: Keller, Jacob E, Nguyen, Anthony L, Intel Wired LAN
Cc: Keller, Jacob E, Kitszel, Przemyslaw
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: Friday, February 16, 2024 11:07 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Intel Wired LAN
> <intel-wired-lan@lists.osuosl.org>
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next 1/4] ice: pass VSI pointer into
> ice_vc_isvalid_q_id
>
> The ice_vc_isvalid_q_id() function takes a VSI index and a queue ID. It looks up
> the VSI from its index, and then validates that the queue number is valid for
> that VSI.
>
> The VSI ID passed is typically a VSI index from the VF. This VSI number is
> validated by the PF to ensure that it matches the VSI associated with the VF
> already.
>
> In every flow where ice_vc_isvalid_q_id() is called, the PF driver already has a
> pointer to the VSI associated with the VF. This pointer is obtained using
> ice_get_vf_vsi(), rather than looking up the VSI using the index sent by the VF.
>
> Since we already know which VSI to operate on, we can modify
> ice_vc_isvalid_q_id() to take a VSI pointer instead of a VSI index. Pass the VSI
> we found from ice_get_vf_vsi() instead of re-doing the lookup. This removes
> some unnecessary computation and scanning of the VSI list.
>
> It also removes the last place where the driver directly used the VSI number
> from the VF. This will pave the way for refactoring to communicate relative VSI
> numbers to the VF instead of absolute numbers from the PF space.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_virtchnl.c | 22 +++++++++----------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> index 6f2328a049bf..29449030174f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> @@ -555,17 +555,15 @@ bool ice_vc_isvalid_vsi_id(struct ice_vf *vf, u16
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Intel-wired-lan] [PATCH iwl-next 2/4] ice: remove unnecessary duplicate checks for VF VSI ID
2024-02-16 22:06 [Intel-wired-lan] [PATCH iwl-next 0/4] ice: use relative VSI index for VFs VSIs Jacob Keller
2024-02-16 22:06 ` [Intel-wired-lan] [PATCH iwl-next 1/4] ice: pass VSI pointer into ice_vc_isvalid_q_id Jacob Keller
@ 2024-02-16 22:06 ` Jacob Keller
2024-03-01 15:20 ` Romanowski, Rafal
2024-02-16 22:06 ` [Intel-wired-lan] [PATCH iwl-next 3/4] ice: use relative VSI index for VFs instead of PF VSI number Jacob Keller
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Jacob Keller @ 2024-02-16 22:06 UTC (permalink / raw)
To: Anthony Nguyen, Intel Wired LAN; +Cc: Jacob Keller, Przemek Kitszel
The ice_vc_fdir_param_check() function validates that the VSI ID of the
virtchnl flow director command matches the VSI number of the VF. This is
already checked by the call to ice_vc_isvalid_vsi_id() immediately
following this.
This check is unnecessary since ice_vc_isvalid_vsi_id() already confirms
this by checking that the VSI ID can locate the VSI associated with the VF
structure.
Furthermore, a following change is going to refactor the ice driver to
report VSI IDs using a relative index for each VF instead of reporting the
PF VSI number. This additional check would break that logic since it
enforces that the VSI ID matches the VSI number.
Since this check duplicates the logic in ice_vc_isvalid_vsi_id() and gets
in the way of refactoring that logic, remove it.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
index f001553e1a1a..8e4ff3af86c6 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
@@ -94,9 +94,6 @@ ice_vc_fdir_param_check(struct ice_vf *vf, u16 vsi_id)
if (!(vf->driver_caps & VIRTCHNL_VF_OFFLOAD_FDIR_PF))
return -EINVAL;
- if (vsi_id != vf->lan_vsi_num)
- return -EINVAL;
-
if (!ice_vc_isvalid_vsi_id(vf, vsi_id))
return -EINVAL;
--
2.41.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Intel-wired-lan] [PATCH iwl-next 2/4] ice: remove unnecessary duplicate checks for VF VSI ID
2024-02-16 22:06 ` [Intel-wired-lan] [PATCH iwl-next 2/4] ice: remove unnecessary duplicate checks for VF VSI ID Jacob Keller
@ 2024-03-01 15:20 ` Romanowski, Rafal
0 siblings, 0 replies; 10+ messages in thread
From: Romanowski, Rafal @ 2024-03-01 15:20 UTC (permalink / raw)
To: Keller, Jacob E, Nguyen, Anthony L, Intel Wired LAN
Cc: Keller, Jacob E, Kitszel, Przemyslaw
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: Friday, February 16, 2024 11:07 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Intel Wired LAN
> <intel-wired-lan@lists.osuosl.org>
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next 2/4] ice: remove unnecessary
> duplicate checks for VF VSI ID
>
> The ice_vc_fdir_param_check() function validates that the VSI ID of the
> virtchnl flow director command matches the VSI number of the VF. This is
> already checked by the call to ice_vc_isvalid_vsi_id() immediately following
> this.
>
> This check is unnecessary since ice_vc_isvalid_vsi_id() already confirms this by
> checking that the VSI ID can locate the VSI associated with the VF structure.
>
> Furthermore, a following change is going to refactor the ice driver to report VSI
> IDs using a relative index for each VF instead of reporting the PF VSI number.
> This additional check would break that logic since it enforces that the VSI ID
> matches the VSI number.
>
> Since this check duplicates the logic in ice_vc_isvalid_vsi_id() and gets in the
> way of refactoring that logic, remove it.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
> b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
> index f001553e1a1a..8e4ff3af86c6 100644
> --- a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
> +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
> @@ -94,9 +94,6 @@ ice_vc_fdir_param_check(struct ice_vf *vf, u16 vsi_id)
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Intel-wired-lan] [PATCH iwl-next 3/4] ice: use relative VSI index for VFs instead of PF VSI number
2024-02-16 22:06 [Intel-wired-lan] [PATCH iwl-next 0/4] ice: use relative VSI index for VFs VSIs Jacob Keller
2024-02-16 22:06 ` [Intel-wired-lan] [PATCH iwl-next 1/4] ice: pass VSI pointer into ice_vc_isvalid_q_id Jacob Keller
2024-02-16 22:06 ` [Intel-wired-lan] [PATCH iwl-next 2/4] ice: remove unnecessary duplicate checks for VF VSI ID Jacob Keller
@ 2024-02-16 22:06 ` Jacob Keller
2024-03-01 15:21 ` Romanowski, Rafal
2024-02-16 22:06 ` [Intel-wired-lan] [PATCH iwl-next 4/4] ice: remove vf->lan_vsi_num field Jacob Keller
2024-03-01 15:25 ` [Intel-wired-lan] [PATCH iwl-next 0/4] ice: use relative VSI index for VFs VSIs Romanowski, Rafal
4 siblings, 1 reply; 10+ messages in thread
From: Jacob Keller @ 2024-02-16 22:06 UTC (permalink / raw)
To: Anthony Nguyen, Intel Wired LAN; +Cc: Jacob Keller, Przemek Kitszel
When initializing over virtchnl, the PF is required to pass a VSI ID to the
VF as part of its capabilities exchange. The VF driver reports this value
back to the PF in a variety of commands. The PF driver validates that this
value matches the value it sent to the VF.
Some hardware families such as the E700 series could use this value when
reading RSS registers or communicating directly with firmware over the
Admin Queue.
However, E800 series hardware does not support any of these interfaces and
the VF's only use for this value is to report it back to the PF. Thus,
there is no requirement that this value be an actual VSI ID value of any
kind.
The PF driver already does not trust that the VF sends it a real VSI ID.
The VSI structure is always looked up from the VF structure. The PF does
validate that the VSI ID provided matches a VSI associated with the VF, but
otherwise does not use the VSI ID for any purpose.
Instead of reporting the VSI number relative to the PF space, report a
fixed value of 1. When communicating with the VF over virtchnl, validate
that the VSI number is returned appropriately.
This avoids leaking information about the firmware of the PF state.
Currently the ice driver only supplies a VF with a single VSI. However, it
appears that virtchnl has some support for allowing multiple VSIs. I did
not attempt to implement this. However, space is left open to allow further
relative indexes if additional VSIs are provided in future feature
development. For this reason, keep the ice_vc_isvalid_vsi_id function in
place to allow extending it for multiple VSIs in the future.
This change will also simplify handling of live migration in a future
series. Since we no longer will provide a real VSI number to the VF, there
will be no need to keep track of this number when migrating to a new host.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
drivers/net/ethernet/intel/ice/ice_virtchnl.c | 9 ++-------
drivers/net/ethernet/intel/ice/ice_virtchnl.h | 9 +++++++++
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
index 29449030174f..1ff9818b4c84 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
@@ -499,7 +499,7 @@ static int ice_vc_get_vf_res_msg(struct ice_vf *vf, u8 *msg)
vfres->rss_lut_size = ICE_LUT_VSI_SIZE;
vfres->max_mtu = ice_vc_get_max_frame_size(vf);
- vfres->vsi_res[0].vsi_id = vf->lan_vsi_num;
+ vfres->vsi_res[0].vsi_id = ICE_VF_VSI_ID;
vfres->vsi_res[0].vsi_type = VIRTCHNL_VSI_SRIOV;
vfres->vsi_res[0].num_queue_pairs = vsi->num_txq;
ether_addr_copy(vfres->vsi_res[0].default_mac_addr,
@@ -545,12 +545,7 @@ static void ice_vc_reset_vf_msg(struct ice_vf *vf)
*/
bool ice_vc_isvalid_vsi_id(struct ice_vf *vf, u16 vsi_id)
{
- struct ice_pf *pf = vf->pf;
- struct ice_vsi *vsi;
-
- vsi = ice_find_vsi(pf, vsi_id);
-
- return (vsi && (vsi->vf == vf));
+ return vsi_id == ICE_VF_VSI_ID;
}
/**
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.h b/drivers/net/ethernet/intel/ice/ice_virtchnl.h
index 60dfbe05980a..3a4115869153 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl.h
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.h
@@ -19,6 +19,15 @@
#define ICE_MAX_MACADDR_PER_VF 18
#define ICE_FLEX_DESC_RXDID_MAX_NUM 64
+/* VFs only get a single VSI. For ice hardware, the VF does not need to know
+ * its VSI index. However, the virtchnl interface requires a VSI number,
+ * mainly due to legacy hardware.
+ *
+ * Since the VF doesn't need this information, report a static value to the VF
+ * instead of leaking any information about the PF or hardware setup.
+ */
+#define ICE_VF_VSI_ID 1
+
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);
--
2.41.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Intel-wired-lan] [PATCH iwl-next 3/4] ice: use relative VSI index for VFs instead of PF VSI number
2024-02-16 22:06 ` [Intel-wired-lan] [PATCH iwl-next 3/4] ice: use relative VSI index for VFs instead of PF VSI number Jacob Keller
@ 2024-03-01 15:21 ` Romanowski, Rafal
0 siblings, 0 replies; 10+ messages in thread
From: Romanowski, Rafal @ 2024-03-01 15:21 UTC (permalink / raw)
To: Keller, Jacob E, Nguyen, Anthony L, Intel Wired LAN
Cc: Keller, Jacob E, Kitszel, Przemyslaw
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: Friday, February 16, 2024 11:07 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Intel Wired LAN
> <intel-wired-lan@lists.osuosl.org>
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next 3/4] ice: use relative VSI index for
> VFs instead of PF VSI number
>
> When initializing over virtchnl, the PF is required to pass a VSI ID to the VF as
> part of its capabilities exchange. The VF driver reports this value back to the PF
> in a variety of commands. The PF driver validates that this value matches the
> value it sent to the VF.
>
> Some hardware families such as the E700 series could use this value when
> reading RSS registers or communicating directly with firmware over the Admin
> Queue.
>
> However, E800 series hardware does not support any of these interfaces and
> the VF's only use for this value is to report it back to the PF. Thus, there is no
> requirement that this value be an actual VSI ID value of any kind.
>
> The PF driver already does not trust that the VF sends it a real VSI ID.
> The VSI structure is always looked up from the VF structure. The PF does
> validate that the VSI ID provided matches a VSI associated with the VF, but
> otherwise does not use the VSI ID for any purpose.
>
> Instead of reporting the VSI number relative to the PF space, report a fixed
> value of 1. When communicating with the VF over virtchnl, validate that the
> VSI number is returned appropriately.
>
> This avoids leaking information about the firmware of the PF state.
> Currently the ice driver only supplies a VF with a single VSI. However, it
> appears that virtchnl has some support for allowing multiple VSIs. I did not
> attempt to implement this. However, space is left open to allow further relative
> indexes if additional VSIs are provided in future feature development. For this
> reason, keep the ice_vc_isvalid_vsi_id function in place to allow extending it for
> multiple VSIs in the future.
>
> This change will also simplify handling of live migration in a future series. Since
> we no longer will provide a real VSI number to the VF, there will be no need to
> keep track of this number when migrating to a new host.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_virtchnl.c | 9 ++-------
> drivers/net/ethernet/intel/ice/ice_virtchnl.h | 9 +++++++++
> 2 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> index 29449030174f..1ff9818b4c84 100644
> --- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> @@ -499,7 +499,7 @@ static int ice_vc_get_vf_res_msg(struct ice_vf *vf, u8
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Intel-wired-lan] [PATCH iwl-next 4/4] ice: remove vf->lan_vsi_num field
2024-02-16 22:06 [Intel-wired-lan] [PATCH iwl-next 0/4] ice: use relative VSI index for VFs VSIs Jacob Keller
` (2 preceding siblings ...)
2024-02-16 22:06 ` [Intel-wired-lan] [PATCH iwl-next 3/4] ice: use relative VSI index for VFs instead of PF VSI number Jacob Keller
@ 2024-02-16 22:06 ` Jacob Keller
2024-03-01 15:21 ` Romanowski, Rafal
2024-03-01 15:25 ` [Intel-wired-lan] [PATCH iwl-next 0/4] ice: use relative VSI index for VFs VSIs Romanowski, Rafal
4 siblings, 1 reply; 10+ messages in thread
From: Jacob Keller @ 2024-02-16 22:06 UTC (permalink / raw)
To: Anthony Nguyen, Intel Wired LAN; +Cc: Jacob Keller, Przemek Kitszel
The lan_vsi_num field of the VF structure is no longer used for any
purpose. Remove it.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
drivers/net/ethernet/intel/ice/ice_sriov.c | 1 -
drivers/net/ethernet/intel/ice/ice_vf_lib.c | 10 +---------
drivers/net/ethernet/intel/ice/ice_vf_lib.h | 5 -----
3 files changed, 1 insertion(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index 706b5ee8ec89..2485abd95672 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -238,7 +238,6 @@ static struct ice_vsi *ice_vf_vsi_setup(struct ice_vf *vf)
}
vf->lan_vsi_idx = vsi->idx;
- vf->lan_vsi_num = vsi->vsi_num;
return vsi;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
index 2ffdae9a82df..21d26e19338a 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
@@ -280,12 +280,6 @@ int ice_vf_reconfig_vsi(struct ice_vf *vf)
return err;
}
- /* Update the lan_vsi_num field since it might have been changed. The
- * PF lan_vsi_idx number remains the same so we don't need to change
- * that.
- */
- vf->lan_vsi_num = vsi->vsi_num;
-
return 0;
}
@@ -315,7 +309,6 @@ static int ice_vf_rebuild_vsi(struct ice_vf *vf)
* vf->lan_vsi_idx
*/
vsi->vsi_num = ice_get_hw_vsi_num(&pf->hw, vsi->idx);
- vf->lan_vsi_num = vsi->vsi_num;
return 0;
}
@@ -1315,13 +1308,12 @@ int ice_vf_init_host_cfg(struct ice_vf *vf, struct ice_vsi *vsi)
}
/**
- * ice_vf_invalidate_vsi - invalidate vsi_idx/vsi_num to remove VSI access
+ * ice_vf_invalidate_vsi - invalidate vsi_idx to remove VSI access
* @vf: VF to remove access to VSI for
*/
void ice_vf_invalidate_vsi(struct ice_vf *vf)
{
vf->lan_vsi_idx = ICE_NO_VSI;
- vf->lan_vsi_num = ICE_NO_VSI;
}
/**
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.h b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
index 0cc9034065c5..fec16919ec19 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
@@ -109,11 +109,6 @@ struct ice_vf {
u8 spoofchk:1;
u8 link_forced:1;
u8 link_up:1; /* only valid if VF link is forced */
- /* VSI indices - actual VSI pointers are maintained in the PF structure
- * When assigned, these will be non-zero, because VSI 0 is always
- * the main LAN VSI for the PF.
- */
- u16 lan_vsi_num; /* ID as used by firmware */
unsigned int min_tx_rate; /* Minimum Tx bandwidth limit in Mbps */
unsigned int max_tx_rate; /* Maximum Tx bandwidth limit in Mbps */
DECLARE_BITMAP(vf_states, ICE_VF_STATES_NBITS); /* VF runtime states */
--
2.41.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Intel-wired-lan] [PATCH iwl-next 4/4] ice: remove vf->lan_vsi_num field
2024-02-16 22:06 ` [Intel-wired-lan] [PATCH iwl-next 4/4] ice: remove vf->lan_vsi_num field Jacob Keller
@ 2024-03-01 15:21 ` Romanowski, Rafal
0 siblings, 0 replies; 10+ messages in thread
From: Romanowski, Rafal @ 2024-03-01 15:21 UTC (permalink / raw)
To: Keller, Jacob E, Nguyen, Anthony L, Intel Wired LAN
Cc: Keller, Jacob E, Kitszel, Przemyslaw
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: Friday, February 16, 2024 11:07 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Intel Wired LAN
> <intel-wired-lan@lists.osuosl.org>
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next 4/4] ice: remove vf->lan_vsi_num
> field
>
> The lan_vsi_num field of the VF structure is no longer used for any purpose.
> Remove it.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_sriov.c | 1 -
> drivers/net/ethernet/intel/ice/ice_vf_lib.c | 10 +---------
> drivers/net/ethernet/intel/ice/ice_vf_lib.h | 5 -----
> 3 files changed, 1 insertion(+), 15 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index 706b5ee8ec89..2485abd95672 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> @@ -238,7 +238,6 @@ static struct ice_vsi *ice_vf_vsi_setup(struct ice_vf
> *vf)
> }
>
> vf->lan_vsi_idx = vsi->idx;
> - vf->lan_vsi_num = vsi->vsi_num;
>
> return vsi;
> }
> diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> index 2ffdae9a82df..21d26e19338a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> @@ -280,12 +280,6 @@ int ice_vf_reconfig_vsi(struct ice_vf *vf)
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-wired-lan] [PATCH iwl-next 0/4] ice: use relative VSI index for VFs VSIs
2024-02-16 22:06 [Intel-wired-lan] [PATCH iwl-next 0/4] ice: use relative VSI index for VFs VSIs Jacob Keller
` (3 preceding siblings ...)
2024-02-16 22:06 ` [Intel-wired-lan] [PATCH iwl-next 4/4] ice: remove vf->lan_vsi_num field Jacob Keller
@ 2024-03-01 15:25 ` Romanowski, Rafal
4 siblings, 0 replies; 10+ messages in thread
From: Romanowski, Rafal @ 2024-03-01 15:25 UTC (permalink / raw)
To: Intel Wired LAN; +Cc: Keller, Jacob E, Kitszel, Przemyslaw, Nguyen, Anthony L
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: Friday, February 16, 2024 11:07 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Intel Wired LAN
> <intel-wired-lan@lists.osuosl.org>
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next 0/4] ice: use relative VSI index for
> VFs VSIs
>
> The ice driver currently communicates firmware VSI numbers to its virtual
> functions over virtchnl. For E800 series hardware, the VF driver has no direct
> use of the VSI number.
>
> Some older legacy hardware could use the actual VSI number when
> communicating directly to firmware via the AdminQ. The E800 hardware does
> not allow this, and all communication happens over mailbox to the PF. The VFs
> do not have a direct access to the firmware. Additionally, none of the registers
> exposed to the VF depend on the VSI number.
>
> Further, the PF is able to lookup the VSI for the VF without using the number
> provided by the VF over virtchnl. Thus, there is no reason that the number
> provided to the VF must actually be a real VSI number, nor does it need to be
> distinct across multiple VFs.
>
> This series modifies the ice driver to send a relative VSI number to the VF
> instead of sending the firmware values. This simplifies the interface with the
> VF, as the PF can simply validate this relative number. Currently, only a single
> VSI is provided to each VF. Thus, a simple static value of 1 is used. We can
> easily extend this to use a proper relative index if we enable multiple VSIs for a
> VF in the future.
>
> First, a couple of patches cleanup a few places in the code which still use the
> VF VSI IDs. Then, the VSI ID logic over virtchnl is changed to use the static
> values. Finally, the vf->lan_vsi_num field is no longer used and only set, so we
> can simplify the driver further by removing this entirely.
>
> This eliminates a path for leaking information about the PF state to the VF, and
> simplifies the PF driver logic. Several of the removed code flows required an
> iterated scan over the VSI list to locate the VSI with the reported VSI number.
>
> Finally, this has significant value for a future series implementing VF live
> migration. Now that the PF always passes relative VSI indexes, migration will
> no longer need to worry about migrating the absolute VSI numbers sent
> previously, which will simplify both the migration process as well as continued
> handling of a migrated VF after a migration event completes.
>
> Jacob Keller (4):
> ice: pass VSI pointer into ice_vc_isvalid_q_id
> ice: remove unnecessary duplicate checks for VF VSI ID
> ice: use relative VSI index for VFs instead of PF VSI number
> ice: remove vf->lan_vsi_num field
>
> drivers/net/ethernet/intel/ice/ice_sriov.c | 1 -
> drivers/net/ethernet/intel/ice/ice_vf_lib.c | 10 +-----
> drivers/net/ethernet/intel/ice/ice_vf_lib.h | 5 ---
> drivers/net/ethernet/intel/ice/ice_virtchnl.c | 31 +++++++------------
> drivers/net/ethernet/intel/ice/ice_virtchnl.h | 9 ++++++
> .../ethernet/intel/ice/ice_virtchnl_fdir.c | 3 --
> 6 files changed, 22 insertions(+), 37 deletions(-)
>
>
> base-commit: 6cffde791c4f1c276fdfcf068554c3c77de35f40
> --
> 2.41.0
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
^ permalink raw reply [flat|nested] 10+ messages in thread