Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] i40e: missing state check in devlink atr_sample_rate callback causes continuous flushes --
commit dbdc3fe26acc90d8b2942b0ba0ea5222fd02c43b Author: Mohammad Heib <[email protected]> i40e: add devlink parameter for Flow Director ATR sample rate This commit introduces a new devlink parameter, atr_sample_rate, to allow administrators to tune the update frequency of Flow Director ATR dynamically. It provides set and get callbacks to apply changes without rebuilding the driver or disabling ATR functionality entirely. > diff --git a/drivers/net/ethernet/intel/i40e/i40e_devlink.c > b/drivers/net/ethernet/intel/i40e/i40e_devlink.c > index 229179ccc1319..8af2487776f28 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_devlink.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_devlink.c [ ... ] > @@ -33,12 +33,58 @@ static int i40e_max_mac_per_vf_get(struct devlink > *devlink, > return 0; > } > > +static int i40e_atr_sample_rate_set(struct devlink *devlink, > + u32 id, > + struct devlink_param_gset_ctx *ctx, > + struct netlink_ext_ack *extack) > +{ > + struct i40e_pf *pf = devlink_priv(devlink); > + u32 sample_rate = ctx->val.vu32; > + > + WRITE_ONCE(pf->atr_sample_rate, sample_rate); > + > + /* Setting rate to 0 disables sampling but does not remove > + * existing ATR entries. Flush them so stale filters do not > + * persist on devices without HW ATR eviction. > + */ > + if (!sample_rate) { > + set_bit(__I40E_FD_ATR_AUTO_DISABLED, pf->state); > + set_bit(__I40E_FD_FLUSH_REQUESTED, pf->state); > + } [Severity: Low] Will this trigger unnecessary hardware flushes if a user or configuration tool repeatedly sets the atr_sample_rate to 0 when it is already 0? The devlink core in net/devlink/param.c:__devlink_nl_cmd_param_set_doit() does not filter out unchanged values before invoking the driver's set callback. As a result, i40e_atr_sample_rate_set() unconditionally sets the __I40E_FD_FLUSH_REQUESTED bit in pf->state, which then triggers a flush in the background service task. Would it be better to check if the new sample_rate differs from the existing value before applying the change and requesting a flush? > + > + return 0; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
