netdev - Re: [PATCH iwl-next v4 2/5] ice: add virtchnl and VF context support for GTP RSS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250912090237.GU30363@horms.kernel.org>
Date: Fri, 12 Sep 2025 10:02:37 +0100
From: Simon Horman <horms@...nel.org>
To: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
Cc: intel-wired-lan@...ts.osuosl.org, anthony.l.nguyen@...el.com,
	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 v4 2/5] ice: add virtchnl and VF context support
 for GTP RSS

On Fri, Aug 29, 2025 at 10:18:05AM +0000, 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 virt/rss.c to handle advanced RSS
>    configuration via virtchnl messages.
> 
> VFs can 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>

Aside from the minor comment below, this looks good to me.
So with that addressed, feel free to add:

Reviewed-by: Simon Horman <horms@...nel.org>

> diff --git a/drivers/net/ethernet/intel/ice/virt/rss.c b/drivers/net/ethernet/intel/ice/virt/rss.c

...

> +/**
> + * ice_parse_raw_rss_pattern - Parse raw pattern spec and mask for RSS
> + * @vf: pointer to the VF info
> + * @proto: pointer to the virtchnl protocol header
> + * @raw_cfg: pointer to the RSS raw pattern configuration
> + *
> + * Parser function to get spec and mask from virtchnl message, and parse
> + * them to get the corresponding profile and offset. The profile is used
> + * to add RSS configuration.
> + *
> + * Return: 0 on success; negative error code on failure.
> + */
> +static int
> +ice_parse_raw_rss_pattern(struct ice_vf *vf, struct virtchnl_proto_hdrs *proto,
> +			  struct ice_rss_raw_cfg *raw_cfg)
> +{
> +	struct ice_parser_result pkt_parsed;
> +	struct ice_hw *hw = &vf->pf->hw;
> +	struct ice_parser_profile prof;
> +	u16 pkt_len;
> +	struct ice_parser *psr;
> +	u8 *pkt_buf, *msk_buf;
> +	int ret = 0;
> +
> +	pkt_len = proto->raw.pkt_len;
> +	if (!pkt_len)
> +		return -EINVAL;
> +	if (pkt_len > VIRTCHNL_MAX_SIZE_RAW_PACKET)
> +		pkt_len = VIRTCHNL_MAX_SIZE_RAW_PACKET;
> +
> +	pkt_buf = kzalloc(pkt_len, GFP_KERNEL);
> +	msk_buf = kzalloc(pkt_len, GFP_KERNEL);
> +	if (!pkt_buf || !msk_buf) {
> +		ret = -ENOMEM;
> +		goto free_alloc;
> +	}
> +
> +	memcpy(pkt_buf, proto->raw.spec, pkt_len);
> +	memcpy(msk_buf, proto->raw.mask, pkt_len);
> +
> +	psr = ice_parser_create(hw);
> +	if (IS_ERR(psr)) {
> +		ret = PTR_ERR(psr);
> +		goto parser_destroy;

I think this one should be goto free_alloc.
Else there will be a NULL pointer dereference in ice_parser_destroy().

> +	}
> +
> +	ret = ice_parser_run(psr, pkt_buf, pkt_len, &pkt_parsed);
> +	if (ret)
> +		goto parser_destroy;
> +
> +	ret = ice_parser_profile_init(&pkt_parsed, pkt_buf, msk_buf,
> +				      pkt_len, ICE_BLK_RSS, &prof);
> +	if (ret)
> +		goto parser_destroy;
> +
> +	memcpy(&raw_cfg->prof, &prof, sizeof(prof));
> +
> +parser_destroy:
> +	ice_parser_destroy(psr);
> +free_alloc:
> +	kfree(pkt_buf);
> +	kfree(msk_buf);
> +	return ret;
> +}