Message-ID: <8dce1f98-2d7f-4997-a2a2-86f5b24587a6@intel.com>
Date: Fri, 22 Aug 2025 10:49:01 -0700
From: Tony Nguyen <anthony.l.nguyen@...el.com>
To: Aleksandr Loktionov <aleksandr.loktionov@...el.com>,
	<intel-wired-lan@...ts.osuosl.org>
CC: <netdev@...r.kernel.org>, <mschmidt@...hat.com>, Dan Nowlin
	<dan.nowlin@...el.com>, Qi Zhang <qi.z.zhang@...el.com>, Przemek Kitszel
	<przemyslaw.kitszel@...el.com>
Subject: Re: [PATCH iwl-next v2 2/5] ice: add virtchnl and VF context support
 for GTP RSS



On 8/18/2025 5:39 AM, Aleksandr Loktionov wrote:
> Introduce infrastructure to support GTP-specific RSS configuration
> in the ICE driver, enabling flexible and programmable flow hashing
> on virtual functions (VFs).
> 
>   - Define new virtchnl protocol header and field types for GTPC, GTPU,
>     L2TPv2, ECPRI, PPP, GRE, and IP fragment headers.
>   - Extend virtchnl.h to support additional RSS fields including checksums,
>     TEID, QFI, and IPv6 prefix matching.
>   - Add VF-side hash context structures for IPv4/IPv6 and GTPU flows.
>   - Implement context tracking and rule ordering logic for TCAM-based
>     RSS configuration.
>   - Introduce symmetric hashing support for raw and tunnel flows.
>   - Update ice_vf_lib.h and ice_virtchnl_rss.c to handle advanced RSS
>     configuration via virtchnl messages.
> 
> This patch enables VFs to express RSS configuration for GTP flows
> using ethtool and virtchnl, laying the foundation for tunnel-aware
> RSS offloads in virtualized environments.
> 
> Co-developed-by: Dan Nowlin <dan.nowlin@...el.com>
> Signed-off-by: Dan Nowlin <dan.nowlin@...el.com>
> Co-developed-by: Jie Wang <jie1x.wang@...el.com>
> Signed-off-by: Jie Wang <jie1x.wang@...el.com>
> Co-developed-by: Junfeng Guo <junfeng.guo@...el.com>
> Signed-off-by: Junfeng Guo <junfeng.guo@...el.com>
> Co-developed-by: Qi Zhang <qi.z.zhang@...el.com>
> Signed-off-by: Qi Zhang <qi.z.zhang@...el.com>
> Co-developed-by: Ting Xu <ting.xu@...el.com>
> Signed-off-by: Ting Xu <ting.xu@...el.com>
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_vf_lib.h   |   48 +
>   .../net/ethernet/intel/ice/ice_virtchnl_rss.c | 1297 ++++++++++++++++-

I don't believe you are based on Przemek's changes as this should be 
under virt folder i.e. series doesn't apply.

>   include/linux/avf/virtchnl.h                  |   50 +
>   3 files changed, 1340 insertions(+), 55 deletions(-)
> 

...

>   
> +/**
> + * ice_is_hash_cfg_valid - check if the hash context is valid
> + * @cfg: pointer to the RSS hash configuration
> + *
> + * This function will return true if the hash context is valid, otherwise
> + * return false.
> + */

Many of the kdocs are missing the 'Return:'. For the boiler plate kdocs, 
we should probably just remove the kdoc altogether.

Also, as "This patch..." is not liked, I feel we should avoid starting 
many of these with "This function..." Saves some chars too :)

Thanks,
Tony

> +static bool ice_is_hash_cfg_valid(struct ice_rss_hash_cfg *cfg)
> +{
> +	return cfg->hash_flds && cfg->addl_hdrs;
> +}
> +
> +/**
> + * ice_hash_cfg_reset - reset the hash context
> + * @cfg: pointer to the RSS hash configuration
> + *
> + * This function will reset the hash context which stores the valid rule info.
> + */
> +static void ice_hash_cfg_reset(struct ice_rss_hash_cfg *cfg)
> +{
> +	cfg->hash_flds = 0;
> +	cfg->addl_hdrs = 0;
> +	cfg->hdr_type = ICE_RSS_OUTER_HEADERS;
> +	cfg->symm = 0;
> +}
> +
> +/**
> + * ice_hash_cfg_record - record the hash context
> + * @ctx: pointer to the global RSS hash configuration
> + * @cfg: pointer to the RSS hash configuration to be recorded
> + *
> + * This function will record the hash context which stores the valid rule info.
> + */
> +static void ice_hash_cfg_record(struct ice_rss_hash_cfg *ctx,
> +				struct ice_rss_hash_cfg *cfg)
> +{
> +	ctx->hash_flds = cfg->hash_flds;
> +	ctx->addl_hdrs = cfg->addl_hdrs;
> +	ctx->hdr_type = cfg->hdr_type;
> +	ctx->symm = cfg->symm;
> +}
> +
> +/**
> + * ice_hash_moveout - delete a RSS configuration
> + * @vf: pointer to the VF info
> + * @cfg: pointer to the RSS hash configuration
> + *
> + * This function will delete an existing RSS hash configuration but not delete
> + * the hash context which stores the rule info.
> + */
> +static int
> +ice_hash_moveout(struct ice_vf *vf, struct ice_rss_hash_cfg *cfg)
> +{
> +	struct device *dev = ice_pf_to_dev(vf->pf);
> +	struct ice_vsi *vsi = ice_get_vf_vsi(vf);
> +	struct ice_hw *hw = &vf->pf->hw;
> +	int ret;
> +
> +	if (!ice_is_hash_cfg_valid(cfg) || !vsi)
> +		return -ENOENT;
> +
> +	ret = ice_rem_rss_cfg(hw, vsi->idx, cfg);
> +	if (ret && ret != -ENOENT) {
> +		dev_err(dev, "ice_rem_rss_cfg failed for VF %d, VSI %d, error:%d\n",
> +			vf->vf_id, vf->lan_vsi_idx, ret);
> +		return -EBUSY;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * ice_hash_moveback - Add an RSS hash configuration for a VF
> + * @vf: pointer to the VF structure
> + * @cfg: pointer to the RSS hash configuration to be applied
> + *
> + * Add an RSS hash configuration to the specified VF if the configuration
> + * context is valid and the associated VSI is available. This function
> + * attempts to apply the configuration via hardware programming.
> + *
> + * Return: 0 on success, -ENOENT if the configuration or VSI is invalid,
> + *         -EBUSY if hardware programming fails.
> + */
> +static int
> +ice_hash_moveback(struct ice_vf *vf, struct ice_rss_hash_cfg *cfg)
> +{
> +	struct device *dev = ice_pf_to_dev(vf->pf);
> +	struct ice_vsi *vsi = ice_get_vf_vsi(vf);
> +	struct ice_hw *hw = &vf->pf->hw;
> +	int ret;
> +
> +	if (!ice_is_hash_cfg_valid(cfg) || !vsi)
> +		return -ENOENT;
> +
> +	ret = ice_add_rss_cfg(hw, vsi, cfg);
> +	if (ret) {
> +		dev_err(dev, "ice_add_rss_cfg failed for VF %d, VSI %d, error:%d\n",
> +			vf->vf_id, vf->lan_vsi_idx, ret);
> +		return -EBUSY;
> +	}
> +
> +	return 0;
> +}
> +

Powered by blists - more mailing lists