Re: [PATCH v3] net/ice: support link flow control

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Sep 22, 2026 at 06:23:33PM +0000, Vladimir Medvedkin wrote:
> Add get and set operations for IEEE 802.3x link flow control.
> Link pause is advertised only when requested, retained across port
> stop/start and re-armed on link-up. The watermark and pause quanta
> programming is shared with priority flow control.
> 
> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> ---
> v3:
> - Address Bruce's comments.
> - Add documentation.
> - Retain LFC configuration across port stop/start.
> - Fix various issues.
> 
>  doc/guides/nics/features/ice.ini       |   1 +
>  doc/guides/nics/ice.rst                |  50 +++++
>  doc/guides/rel_notes/release_26_11.rst |   4 +
>  drivers/net/intel/ice/ice_ethdev.c     | 296 +++++++++++++++++++++----
>  drivers/net/intel/ice/ice_ethdev.h     |   1 +
>  5 files changed, 310 insertions(+), 42 deletions(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

One comment inline below. I can fix on apply if you agree with it.

> diff --git a/doc/guides/nics/features/ice.ini b/doc/guides/nics/features/ice.ini
> index 893d09e9ec..309f691f32 100644
> --- a/doc/guides/nics/features/ice.ini
> +++ b/doc/guides/nics/features/ice.ini
> @@ -30,6 +30,7 @@ RSS hash             = Y
>  RSS key update       = Y
>  RSS reta update      = Y
>  VLAN filter          = Y
> +Flow control         = Y
>  Traffic manager      = Y
>  CRC offload          = Y
>  VLAN offload         = Y
> diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
> index b671422ad7..065038a0d1 100644
> --- a/doc/guides/nics/ice.rst
> +++ b/doc/guides/nics/ice.rst
> @@ -516,6 +516,56 @@ Example PFC configuration using DPDK API:
>     ret = rte_eth_dev_priority_flow_ctrl_set(port_id, &pfc_conf);
>  
>  

<snip>

> +/*
> + * Program the Rx buffer watermarks, pause quanta and MAC control frame
> + * forwarding shared by link flow control and priority flow control.
> + * A tc_bitmap of 0 selects link flow control; a single bit selects a PFC
> + * traffic class. Watermark or quanta values of 0 leave the current setting.
> + * The only step that can fail is issued first so that on error nothing
> + * has been changed.
> + */
> +static int
> +ice_cfg_fc_params(struct rte_eth_dev *dev, uint8_t tc_bitmap,
> +		  struct rte_eth_fc_conf *fc_conf)
> +{
> +	struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> +	struct ice_hw *hw = ICE_PF_TO_HW(pf);
> +	struct ice_port_info *port_info = hw->port_info;
> +	uint8_t tc = tc_bitmap == 0 ? 0 : rte_ctz32(tc_bitmap);
> +	uint32_t high_water = fc_conf->high_water;
> +	uint32_t low_water = fc_conf->low_water;
> +	uint16_t max_frame_size;
> +	int cgd_idx;
> +	int ret;
> +
> +	if (high_water > ICE_MAC_TC_MAX_WATERMARK)
> +		high_water = ICE_MAC_TC_MAX_WATERMARK;
> +	if (low_water > ICE_MAC_TC_MAX_WATERMARK)
> +		low_water = ICE_MAC_TC_MAX_WATERMARK;
> +
> +	fc_conf->high_water = high_water;
> +	fc_conf->low_water = low_water;
> +
> +	cgd_idx = ice_get_cgd_idx(hw, tc);
> +	if (high_water)
> +		wr32(hw, GLRPB_TCHW(cgd_idx), high_water);
> +	if (low_water)
> +		wr32(hw, GLRPB_TCLW(cgd_idx), low_water);
> +
> +	/* Update forwarding of the non FC MAC control frames settings */
> +	if (hw->mac_type == ICE_MAC_E830) {
> +#define E830_MAC_COMMAND_CONFIG(pi) (((pi)->phy.link_info.link_speed == ICE_AQ_LINK_SPEED_200GB) ? \
> +		E830_PRTMAC_200G_COMMAND_CONFIG : E830_PRTMAC_COMMAND_CONFIG)
> +

I don't like this definition right in the middle of the function. I think
it should either be defined right before the function or else, at latest,
just inside the function opening brace at the top.
If you are ok with it, I'll move it on apply of the patch.

> +		u32 mac_config = rd32(hw, E830_MAC_COMMAND_CONFIG(port_info));
> +
> +		if (fc_conf->mac_ctrl_frame_fwd)
> +			mac_config |= E830_PRTMAC_COMMAND_CONFIG_CNTL_FRM_ENA_M;
> +		else
> +			mac_config &= ~E830_PRTMAC_COMMAND_CONFIG_CNTL_FRM_ENA_M;
> +
> +		wr32(hw, E830_MAC_COMMAND_CONFIG(port_info), mac_config);
> +	} else {
> +		fc_conf->mac_ctrl_frame_fwd = false;
> +	}
> +
> +	/* Update pause quanta and refresh threshold */
> +	max_frame_size = pf->dev_data->mtu ?
> +		pf->dev_data->mtu + ICE_ETH_OVERHEAD :
> +		ICE_FRAME_SIZE_MAX;
> +	ret = ice_aq_set_mac_pfc_cfg(hw, max_frame_size, tc_bitmap,
> +			fc_conf->pause_time, ((u32)fc_conf->pause_time + 1) / 2,
> +			false, NULL);
> +	if (ret)
> +		PMD_DRV_LOG(ERR, "Can not update MAC configuration");
> +
> +	return ret;
> +}

<snip>