linux-kernel - Re: [PATCH net-next v2 04/14] 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20251020182235.7db743ee@kernel.org>
Date: Mon, 20 Oct 2025 18:22:35 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Jacob Keller <jacob.e.keller@...el.com>
Cc: Jiri Pirko <jiri@...nulli.us>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Simon
 Horman <horms@...nel.org>, Jonathan Corbet <corbet@....net>, Tony Nguyen
 <anthony.l.nguyen@...el.com>, Przemek Kitszel
 <przemyslaw.kitszel@...el.com>, Andrew Lunn <andrew+netdev@...n.ch>,
 Alexander Lobakin <aleksander.lobakin@...el.com>, netdev@...r.kernel.org,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org, Aleksandr
 Loktionov <aleksandr.loktionov@...el.com>, Dan Nowlin
 <dan.nowlin@...el.com>, Jie Wang <jie1x.wang@...el.com>, Junfeng Guo
 <junfeng.guo@...el.com>, Qi Zhang <qi.z.zhang@...el.com>, Ting Xu
 <ting.xu@...el.com>, Rafal Romanowski <rafal.romanowski@...el.com>
Subject: Re: [PATCH net-next v2 04/14] ice: add virtchnl and VF context
 support for GTP RSS

On Thu, 16 Oct 2025 23:08:33 -0700 Jacob Keller wrote:
>  void ice_parser_destroy(struct ice_parser *psr)
>  {
> +	if (!psr)
> +		return;

questionable

> +	{VIRTCHNL_PROTO_HDR_L2TPV2,
> +		FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_L2TPV2_SESS_ID),
> +		BIT_ULL(ICE_FLOW_FIELD_IDX_L2TPV2_SESS_ID)},
> +	{VIRTCHNL_PROTO_HDR_L2TPV2,
> +		FIELD_SELECTOR(VIRTCHNL_PROTO_HDR_L2TPV2_LEN_SESS_ID),
> +		BIT_ULL(ICE_FLOW_FIELD_IDX_L2TPV2_LEN_SESS_ID)},

consider moving out all the static data and define changes to a
separate commit for ease of review?

>  };
>  
> +static enum virtchnl_status_code
> +ice_vc_rss_hash_update(struct ice_hw *hw, struct ice_vsi *vsi, u8 hash_type)
> +{
> +	enum virtchnl_status_code v_ret = VIRTCHNL_STATUS_SUCCESS;
> +	struct ice_vsi_ctx *ctx;
> +	int ret;
> +
> +	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return VIRTCHNL_STATUS_ERR_NO_MEMORY;

I feel like the VIRTCHNL_* error codes are spreading, we've been over
this. 

> +static int
> +ice_hash_remove(struct ice_vf *vf, struct ice_rss_hash_cfg *cfg)
> +{
> +	int ret;
> +
> +	ret = ice_hash_moveout(vf, cfg);
> +	if (ret && (ret != -ENOENT))

spurious brackets

> +/**
> + * ice_add_rss_cfg_pre_ip - Pre-process IP-layer RSS configuration
> + * @vf: VF pointer
> + * @ctx: IP L4 hash context (ESP/UDP-ESP/AH/PFCP and UDP/TCP/SCTP)
> + *
> + * Remove covered/recorded IP RSS configurations prior to adding a new one.
> + *
> + * Return: 0 on success; negative error code on failure.
> + */
> +static int
> +ice_add_rss_cfg_pre_ip(struct ice_vf *vf, struct ice_vf_hash_ip_ctx *ctx)
> +{
> +	int i, ret;
> +
> +	for (i = 1; i < ICE_HASH_IP_CTX_MAX; i++)
> +		if (ice_is_hash_cfg_valid(&ctx->ctx[i])) {
> +			ret = ice_hash_remove(vf, &ctx->ctx[i]);
> +

spurious new line

> +			if (ret)
> +				return ret;
> +		}
> +
> +	return 0;
> +}

> +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;

goto free_alloc, surely, parser creation has failed, there's nothing to 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;
> +}