* [Intel-wired-lan] [bug report] ice: Enable FDIR Configure for AVF
@ 2025-09-01 9:09 Dan Carpenter
2025-09-01 11:15 ` Przemek Kitszel
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2025-09-01 9:09 UTC (permalink / raw)
To: Qi Zhang; +Cc: intel-wired-lan
Hello Qi Zhang,
Commit 1f7ea1cd6a37 ("ice: Enable FDIR Configure for AVF") from Mar
9, 2021 (linux-next), leads to the following Smatch static checker
warning:
drivers/net/ethernet/intel/ice/virt/fdir.c:2339 ice_vc_del_fdir_fltr()
warn: missing error code here? 'kzalloc_noprof()' failed
drivers/net/ethernet/intel/ice/virt/fdir.c
2311 int ice_vc_del_fdir_fltr(struct ice_vf *vf, u8 *msg)
2312 {
2313 struct virtchnl_fdir_del *fltr = (struct virtchnl_fdir_del *)msg;
2314 struct virtchnl_fdir_del *stat = NULL;
2315 struct virtchnl_fdir_fltr_conf *conf;
2316 struct ice_vf_fdir *fdir = &vf->fdir;
2317 enum virtchnl_status_code v_ret;
2318 struct ice_fdir_fltr *input;
2319 enum ice_fltr_ptype flow;
2320 struct device *dev;
2321 struct ice_pf *pf;
2322 int is_tun = 0;
2323 int len = 0;
2324 int ret;
2325
2326 pf = vf->pf;
2327 dev = ice_pf_to_dev(pf);
2328 ret = ice_vc_fdir_param_check(vf, fltr->vsi_id);
2329 if (ret) {
2330 v_ret = VIRTCHNL_STATUS_ERR_PARAM;
2331 dev_dbg(dev, "Parameter check for VF %d failed\n", vf->vf_id);
2332 goto err_exit;
2333 }
2334
2335 stat = kzalloc(sizeof(*stat), GFP_KERNEL);
2336 if (!stat) {
2337 v_ret = VIRTCHNL_STATUS_ERR_NO_MEMORY;
It's not clear if this is deliberate or not. Maybe we could add a comment?
Here set v_ret.
2338 dev_dbg(dev, "Alloc stat for VF %d failed\n", vf->vf_id);
--> 2339 goto err_exit;
2340 }
2341
2342 len = sizeof(*stat);
2343
2344 conf = ice_vc_fdir_lookup_entry(vf, fltr->flow_id);
2345 if (!conf) {
2346 v_ret = VIRTCHNL_STATUS_SUCCESS;
2347 stat->status = VIRTCHNL_FDIR_FAILURE_RULE_NONEXIST;
2348 dev_dbg(dev, "VF %d: FDIR invalid flow_id:0x%X\n",
2349 vf->vf_id, fltr->flow_id);
2350 goto err_exit;
2351 }
2352
2353 /* Just return failure when ctrl_vsi idx is invalid */
2354 if (vf->ctrl_vsi_idx == ICE_NO_VSI) {
2355 v_ret = VIRTCHNL_STATUS_SUCCESS;
2356 stat->status = VIRTCHNL_FDIR_FAILURE_RULE_NORESOURCE;
2357 dev_err(dev, "Invalid FDIR ctrl_vsi for VF %d\n", vf->vf_id);
2358 goto err_exit;
2359 }
2360
2361 ret = ice_vc_fdir_set_irq_ctx(vf, conf, VIRTCHNL_OP_DEL_FDIR_FILTER);
2362 if (ret) {
2363 v_ret = VIRTCHNL_STATUS_SUCCESS;
2364 stat->status = VIRTCHNL_FDIR_FAILURE_RULE_NORESOURCE;
2365 dev_dbg(dev, "VF %d: set FDIR context failed\n", vf->vf_id);
2366 goto err_exit;
2367 }
2368
2369 /* For raw FDIR filters created by the parser */
2370 if (conf->parser_ena) {
2371 ret = ice_vc_del_fdir_raw(vf, conf, &v_ret, stat, len);
2372 if (ret)
2373 goto err_del_tmr;
2374 goto exit;
2375 }
2376
2377 is_tun = ice_fdir_is_tunnel(conf->ttype);
2378 ret = ice_vc_fdir_write_fltr(vf, conf, false, is_tun);
2379 if (ret) {
2380 v_ret = VIRTCHNL_STATUS_SUCCESS;
2381 stat->status = VIRTCHNL_FDIR_FAILURE_RULE_NORESOURCE;
2382 dev_err(dev, "VF %d: writing FDIR rule failed, ret:%d\n",
2383 vf->vf_id, ret);
2384 goto err_del_tmr;
2385 }
2386
2387 /* Remove unused profiles to avoid unexpected behaviors */
2388 input = &conf->input;
2389 flow = input->flow_type;
2390 if (fdir->fdir_fltr_cnt[flow][is_tun] == 1)
2391 ice_vc_fdir_rem_prof(vf, flow, is_tun);
2392
2393 exit:
2394 kfree(stat);
2395
2396 return ret;
2397
2398 err_del_tmr:
2399 ice_vc_fdir_clear_irq_ctx(vf);
2400 err_exit:
2401 ret = ice_vc_send_msg_to_vf(vf, VIRTCHNL_OP_DEL_FDIR_FILTER, v_ret,
2402 (u8 *)stat, len);
But then "ret" is generally going to be success.
2403 kfree(stat);
2404 return ret;
2405 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Intel-wired-lan] [bug report] ice: Enable FDIR Configure for AVF
2025-09-01 9:09 [Intel-wired-lan] [bug report] ice: Enable FDIR Configure for AVF Dan Carpenter
@ 2025-09-01 11:15 ` Przemek Kitszel
2025-09-01 11:52 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Przemek Kitszel @ 2025-09-01 11:15 UTC (permalink / raw)
To: Dan Carpenter; +Cc: intel-wired-lan, Qi Zhang
On 9/1/25 11:09, Dan Carpenter wrote:
> Hello Qi Zhang,
>
> Commit 1f7ea1cd6a37 ("ice: Enable FDIR Configure for AVF") from Mar
> 9, 2021 (linux-next), leads to the following Smatch static checker
> warning:
>
> drivers/net/ethernet/intel/ice/virt/fdir.c:2339 ice_vc_del_fdir_fltr()
> warn: missing error code here? 'kzalloc_noprof()' failed
>
> drivers/net/ethernet/intel/ice/virt/fdir.c
> 2311 int ice_vc_del_fdir_fltr(struct ice_vf *vf, u8 *msg)
> 2312 {
> 2313 struct virtchnl_fdir_del *fltr = (struct virtchnl_fdir_del *)msg;
> 2314 struct virtchnl_fdir_del *stat = NULL;
> 2315 struct virtchnl_fdir_fltr_conf *conf;
> 2316 struct ice_vf_fdir *fdir = &vf->fdir;
> 2317 enum virtchnl_status_code v_ret;
> 2318 struct ice_fdir_fltr *input;
> 2319 enum ice_fltr_ptype flow;
> 2320 struct device *dev;
> 2321 struct ice_pf *pf;
> 2322 int is_tun = 0;
> 2323 int len = 0;
> 2324 int ret;
> 2325
> 2326 pf = vf->pf;
> 2327 dev = ice_pf_to_dev(pf);
> 2328 ret = ice_vc_fdir_param_check(vf, fltr->vsi_id);
> 2329 if (ret) {
> 2330 v_ret = VIRTCHNL_STATUS_ERR_PARAM;
> 2331 dev_dbg(dev, "Parameter check for VF %d failed\n", vf->vf_id);
> 2332 goto err_exit;
> 2333 }
> 2334
> 2335 stat = kzalloc(sizeof(*stat), GFP_KERNEL);
> 2336 if (!stat) {
> 2337 v_ret = VIRTCHNL_STATUS_ERR_NO_MEMORY;
>
> It's not clear if this is deliberate or not. Maybe we could add a comment?
> Here set v_ret.
thank you for the report,
ice_vc_del_fdir_fltr() is only called from general virtchnl processing
handler, which returns void, and only logs errors on info level, there
is nothing to do about the error anyway
in this case failure at kzalloc() call is on ice/kernel side, not on VF
side, so in principle worth mentioning/blaming; but, from return code
(we don't have much variety of those in VC, but for memory alloc failure
there is one) VF side is knowing that error had happened exactly here
in case of FDIR filter DEL operation, so not much value added
>
>
> 2338 dev_dbg(dev, "Alloc stat for VF %d failed\n", vf->vf_id);
> --> 2339 goto err_exit;
> 2340 }
> 2341
> 2342 len = sizeof(*stat);
> 2343
> 2344 conf = ice_vc_fdir_lookup_entry(vf, fltr->flow_id);
> 2345 if (!conf) {
> 2346 v_ret = VIRTCHNL_STATUS_SUCCESS;
> 2347 stat->status = VIRTCHNL_FDIR_FAILURE_RULE_NONEXIST;
> 2348 dev_dbg(dev, "VF %d: FDIR invalid flow_id:0x%X\n",
> 2349 vf->vf_id, fltr->flow_id);
> 2350 goto err_exit;
> 2351 }
[...]
> 2393 exit:
> 2394 kfree(stat);
> 2395
> 2396 return ret;
> 2397
> 2398 err_del_tmr:
> 2399 ice_vc_fdir_clear_irq_ctx(vf);
> 2400 err_exit:
> 2401 ret = ice_vc_send_msg_to_vf(vf, VIRTCHNL_OP_DEL_FDIR_FILTER, v_ret,
> 2402 (u8 *)stat, len);
>
> But then "ret" is generally going to be success.
>
> 2403 kfree(stat);
> 2404 return ret;
> 2405 }
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Intel-wired-lan] [bug report] ice: Enable FDIR Configure for AVF
2025-09-01 11:15 ` Przemek Kitszel
@ 2025-09-01 11:52 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2025-09-01 11:52 UTC (permalink / raw)
To: Przemek Kitszel; +Cc: intel-wired-lan, Qi Zhang
On Mon, Sep 01, 2025 at 01:15:03PM +0200, Przemek Kitszel wrote:
> On 9/1/25 11:09, Dan Carpenter wrote:
> > Hello Qi Zhang,
> >
> > Commit 1f7ea1cd6a37 ("ice: Enable FDIR Configure for AVF") from Mar
> > 9, 2021 (linux-next), leads to the following Smatch static checker
> > warning:
> >
> > drivers/net/ethernet/intel/ice/virt/fdir.c:2339 ice_vc_del_fdir_fltr()
> > warn: missing error code here? 'kzalloc_noprof()' failed
> >
> > drivers/net/ethernet/intel/ice/virt/fdir.c
> > 2311 int ice_vc_del_fdir_fltr(struct ice_vf *vf, u8 *msg)
> > 2312 {
> > 2313 struct virtchnl_fdir_del *fltr = (struct virtchnl_fdir_del *)msg;
> > 2314 struct virtchnl_fdir_del *stat = NULL;
> > 2315 struct virtchnl_fdir_fltr_conf *conf;
> > 2316 struct ice_vf_fdir *fdir = &vf->fdir;
> > 2317 enum virtchnl_status_code v_ret;
> > 2318 struct ice_fdir_fltr *input;
> > 2319 enum ice_fltr_ptype flow;
> > 2320 struct device *dev;
> > 2321 struct ice_pf *pf;
> > 2322 int is_tun = 0;
> > 2323 int len = 0;
> > 2324 int ret;
> > 2325
> > 2326 pf = vf->pf;
> > 2327 dev = ice_pf_to_dev(pf);
> > 2328 ret = ice_vc_fdir_param_check(vf, fltr->vsi_id);
> > 2329 if (ret) {
> > 2330 v_ret = VIRTCHNL_STATUS_ERR_PARAM;
> > 2331 dev_dbg(dev, "Parameter check for VF %d failed\n", vf->vf_id);
> > 2332 goto err_exit;
> > 2333 }
> > 2334
> > 2335 stat = kzalloc(sizeof(*stat), GFP_KERNEL);
> > 2336 if (!stat) {
> > 2337 v_ret = VIRTCHNL_STATUS_ERR_NO_MEMORY;
> >
> > It's not clear if this is deliberate or not. Maybe we could add a comment?
> > Here set v_ret.
>
> thank you for the report,
> ice_vc_del_fdir_fltr() is only called from general virtchnl processing
> handler, which returns void, and only logs errors on info level, there
> is nothing to do about the error anyway
> in this case failure at kzalloc() call is on ice/kernel side, not on VF
> side, so in principle worth mentioning/blaming; but, from return code
> (we don't have much variety of those in VC, but for memory alloc failure
> there is one) VF side is knowing that error had happened exactly here
> in case of FDIR filter DEL operation, so not much value added
>
Thanks for the explanation!
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-09-01 11:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-01 9:09 [Intel-wired-lan] [bug report] ice: Enable FDIR Configure for AVF Dan Carpenter
2025-09-01 11:15 ` Przemek Kitszel
2025-09-01 11:52 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.