[Intel-wired-lan] [PATCH iwl-next 4/4] ice: Fix VLAN and CRC strip co-existence
Tony Nguyen
anthony.l.nguyen at intel.com
Tue Aug 29 20:51:05 UTC 2023
On 8/24/2023 5:50 AM, Ahmed Zaki wrote:
> From: Haiyue Wang <haiyue.wang at intel.com>
>
> When VLAN strip is enabled, the CRC strip should not be allowed to be
> disabled. And when CRC strip is disabled, the VLAN strip should not be
> allowed to be enabled.
>
> It needs to check CRC strip disable setting parameter firstly before
> configuring the Rx/Tx queues, otherwise, in current error handling,
> the already set Tx queue context doesn't rollback correctly, it will
> cause the Tx queue setup failure next time:
> "Failed to set LAN Tx queue context, error: ICE_ERR_PARAM"
Would this go better before the iavf patch so that we have a properly
functioning PF before iavf tries to use it?
Also, where does this come from?
"Failed to set LAN Tx queue context, error: ICE_ERR_PARAM"
ICE_ERRs don't exist any more.
> Signed-off-by: Haiyue Wang <haiyue.wang at intel.com>
> Reviewed-by: Jesse Brandeburg <jesse.brandeburg at intel.com>
> Signed-off-by: Ahmed Zaki <ahmed.zaki at intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_vf_lib.h | 3 +
> drivers/net/ethernet/intel/ice/ice_virtchnl.c | 64 ++++++++++++++++---
> 2 files changed, 58 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.h b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
> index 48fea6fa0362..31a082e8a827 100644
> --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.h
> +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
> @@ -123,6 +123,9 @@ struct ice_vf {
> u8 num_req_qs; /* num of queue pairs requested by VF */
> u16 num_mac;
> u16 num_vf_qs; /* num of queue configured per VF */
> + u8 vlan_strip_ena; /* Outer and Inner VLAN strip enable */
> +#define ICE_INNER_VLAN_STRIP_ENA BIT(0)
> +#define ICE_OUTER_VLAN_STRIP_ENA BIT(1)
> struct ice_mdd_vf_events mdd_rx_events;
> struct ice_mdd_vf_events mdd_tx_events;
> DECLARE_BITMAP(opcodes_allowlist, VIRTCHNL_OP_MAX);
> diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> index c0c3e524c523..e1621aecd973 100644
> --- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> @@ -1623,6 +1623,15 @@ static int ice_vc_cfg_qs_msg(struct ice_vf *vf, u8 *msg)
> goto error_param;
> }
>
> + for (i = 0; i < qci->num_queue_pairs; i++) {
> + if (!qci->qpair[i].rxq.crc_disable)
> + continue;
> +
> + if (!(vf->driver_caps & VIRTCHNL_VF_OFFLOAD_CRC) ||
> + vf->vlan_strip_ena)
> + goto error_param;
> + }
> +
> for (i = 0; i < qci->num_queue_pairs; i++) {
> qpi = &qci->qpair[i];
> if (qpi->txq.vsi_id != qci->vsi_id ||
> @@ -1669,11 +1678,6 @@ static int ice_vc_cfg_qs_msg(struct ice_vf *vf, u8 *msg)
> vsi->rx_rings[i]->dma = qpi->rxq.dma_ring_addr;
> vsi->rx_rings[i]->count = qpi->rxq.ring_len;
>
> - if (qpi->rxq.crc_disable &&
> - !(vf->driver_caps & VIRTCHNL_VF_OFFLOAD_CRC)) {
> - goto error_param;
> - }
> -
> if (qpi->rxq.crc_disable)
> vsi->rx_rings[q_idx]->flags |=
> ICE_RX_FLAGS_CRC_STRIP_DIS;
> @@ -2425,6 +2429,21 @@ static int ice_vc_remove_vlan_msg(struct ice_vf *vf, u8 *msg)
> return ice_vc_process_vlan_msg(vf, msg, false);
> }
>
> +/**
> + * ice_vsi_is_rxq_crc_strip_dis - check if Rx queue CRC strip is disabled or not
> + * @vsi: pointer to the VF VSI info
> + */
> +static bool ice_vsi_is_rxq_crc_strip_dis(struct ice_vsi *vsi)
> +{
> + u16 i;
I believe preference is to put this in the for loop now.
> + for (i = 0; i < vsi->alloc_rxq; i++)
> + if (vsi->rx_rings[i]->flags & ICE_RX_FLAGS_CRC_STRIP_DIS)
> + return true;
> +
> + return false;
> +}
> +
> /**
> * ice_vc_ena_vlan_stripping
> * @vf: pointer to the VF info
> @@ -2454,6 +2473,8 @@ static int ice_vc_ena_vlan_stripping(struct ice_vf *vf)
>
> if (vsi->inner_vlan_ops.ena_stripping(vsi, ETH_P_8021Q))
> v_ret = VIRTCHNL_STATUS_ERR_PARAM;
> + else
> + vf->vlan_strip_ena |= ICE_INNER_VLAN_STRIP_ENA;
>
> error_param:
> return ice_vc_send_msg_to_vf(vf, VIRTCHNL_OP_ENABLE_VLAN_STRIPPING,
> @@ -2489,6 +2510,8 @@ static int ice_vc_dis_vlan_stripping(struct ice_vf *vf)
>
> if (vsi->inner_vlan_ops.dis_stripping(vsi))
> v_ret = VIRTCHNL_STATUS_ERR_PARAM;
> + else
> + vf->vlan_strip_ena &= ~ICE_INNER_VLAN_STRIP_ENA;
>
> error_param:
> return ice_vc_send_msg_to_vf(vf, VIRTCHNL_OP_DISABLE_VLAN_STRIPPING,
> @@ -2663,6 +2686,9 @@ static int ice_vc_query_rxdid(struct ice_vf *vf)
> static int ice_vf_init_vlan_stripping(struct ice_vf *vf)
> {
> struct ice_vsi *vsi = ice_get_vf_vsi(vf);
> + int err;
Seems like scope of this can be reduced.
> + vf->vlan_strip_ena = 0;
>
> if (!vsi)
> return -EINVAL;
> @@ -2673,10 +2699,15 @@ static int ice_vf_init_vlan_stripping(struct ice_vf *vf)
> if (ice_vf_is_port_vlan_ena(vf) && !ice_is_dvm_ena(&vsi->back->hw))
> return 0;
>
> - if (ice_vf_vlan_offload_ena(vf->driver_caps))
> - return vsi->inner_vlan_ops.ena_stripping(vsi, ETH_P_8021Q);
> - else
> - return vsi->inner_vlan_ops.dis_stripping(vsi);
> + if (ice_vf_vlan_offload_ena(vf->driver_caps)) {
> + err = vsi->inner_vlan_ops.ena_stripping(vsi, ETH_P_8021Q);
> +
> + if (!err)
No newline between function call and error check please.
> + vf->vlan_strip_ena |= ICE_INNER_VLAN_STRIP_ENA;
> + return err;
> + }
> +
> + return vsi->inner_vlan_ops.dis_stripping(vsi);
> }
>
> static u16 ice_vc_get_max_vlan_fltrs(struct ice_vf *vf)
> @@ -3450,6 +3481,11 @@ static int ice_vc_ena_vlan_stripping_v2_msg(struct ice_vf *vf, u8 *msg)
> goto out;
> }
>
> + if (ice_vsi_is_rxq_crc_strip_dis(vsi)) {
> + v_ret = VIRTCHNL_STATUS_ERR_NOT_SUPPORTED;
> + goto out;
> + }
> +
> ethertype_setting = strip_msg->outer_ethertype_setting;
> if (ethertype_setting) {
> if (ice_vc_ena_vlan_offload(vsi,
> @@ -3470,6 +3506,8 @@ static int ice_vc_ena_vlan_stripping_v2_msg(struct ice_vf *vf, u8 *msg)
> * enabled, is extracted in L2TAG1.
> */
> ice_vsi_update_l2tsel(vsi, l2tsel);
> +
> + vf->vlan_strip_ena |= ICE_OUTER_VLAN_STRIP_ENA;
> }
> }
>
> @@ -3481,6 +3519,9 @@ static int ice_vc_ena_vlan_stripping_v2_msg(struct ice_vf *vf, u8 *msg)
> goto out;
> }
>
> + if (ethertype_setting)
> + vf->vlan_strip_ena |= ICE_INNER_VLAN_STRIP_ENA;
> +
> out:
> return ice_vc_send_msg_to_vf(vf, VIRTCHNL_OP_ENABLE_VLAN_STRIPPING_V2,
> v_ret, NULL, 0);
> @@ -3542,6 +3583,8 @@ static int ice_vc_dis_vlan_stripping_v2_msg(struct ice_vf *vf, u8 *msg)
> * in L2TAG1.
> */
> ice_vsi_update_l2tsel(vsi, l2tsel);
> +
> + vf->vlan_strip_ena &= ~ICE_OUTER_VLAN_STRIP_ENA;
> }
> }
>
> @@ -3551,6 +3594,9 @@ static int ice_vc_dis_vlan_stripping_v2_msg(struct ice_vf *vf, u8 *msg)
> goto out;
> }
>
> + if (ethertype_setting)
> + vf->vlan_strip_ena &= ~ICE_INNER_VLAN_STRIP_ENA;
> +
> out:
> return ice_vc_send_msg_to_vf(vf, VIRTCHNL_OP_DISABLE_VLAN_STRIPPING_V2,
> v_ret, NULL, 0);
More information about the Intel-wired-lan
mailing list