Re: [PATCH net-next v2 04/14] ice: add virtchnl and VF context support for GTP RSS

[Jakub Kicinski <kuba@kernel.org>]

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