Thu, Aug 28, 2025 at 02:49:26PM +0200, [email protected] wrote: >Hi Jiri > >On 8/28/25 2:43 PM, Jiri Pirko wrote: >> Sat, Aug 23, 2025 at 11:49:52AM +0200, [email protected] wrote: >> > From: Mohammad Heib <[email protected]> >> > >> > This patch introduces a new devlink runtime parameter that controls >> > the maximum number of MAC filters allowed per VF. >> > >> > The parameter is an integer value. If set to a non-zero number, it is >> > used as a strict per-VF cap. If left at zero, the driver falls back to >> > the default limit calculated from the number of allocated VFs and >> > ports. >> > >> > This makes the limit policy explicit and configurable by user space, >> > instead of being only driver internal logic. >> > >> > Example command to enable per-vf mac limit: >> > - devlink dev param set pci/0000:3b:00.0 name max_mac_per_vf \ >> > value 12 \ >> > cmode runtime >> > >> > - Previous discussion about this change: >> > https://lore.kernel.org/netdev/[email protected] >> > >> > Fixes: cfb1d572c986 ("i40e: Add ensurance of MacVlan resources for every >> > trusted VF") >> > Signed-off-by: Mohammad Heib <[email protected]> >> > Reviewed-by: Aleksandr Loktionov <[email protected]> >> > --- >> > Documentation/networking/devlink/i40e.rst | 22 ++++++++ >> > drivers/net/ethernet/intel/i40e/i40e.h | 4 ++ >> > .../net/ethernet/intel/i40e/i40e_devlink.c | 56 ++++++++++++++++++- >> > .../ethernet/intel/i40e/i40e_virtchnl_pf.c | 25 +++++---- >> > 4 files changed, 95 insertions(+), 12 deletions(-) >> > >> > diff --git a/Documentation/networking/devlink/i40e.rst >> > b/Documentation/networking/devlink/i40e.rst >> > index d3cb5bb5197e..f8d5b00bb51d 100644 >> > --- a/Documentation/networking/devlink/i40e.rst >> > +++ b/Documentation/networking/devlink/i40e.rst >> > @@ -7,6 +7,28 @@ i40e devlink support >> > This document describes the devlink features implemented by the ``i40e`` >> > device driver. >> > >> > +Parameters >> > +========== >> > + >> > +.. list-table:: Driver specific parameters implemented >> > + :widths: 5 5 90 >> > + >> > + * - Name >> > + - Mode >> > + - Description >> > + * - ``max_mac_per_vf`` >> > + - runtime >> > + - Controls the maximum number of MAC addresses a VF can use on i40e >> > devices. >> > + >> > + By default (``0``), the driver enforces its internally calculated >> > per-VF >> > + MAC filter limit, which is based on the number of allocated VFS. >> > + >> > + If set to a non-zero value, this parameter acts as a strict cap: >> > + the driver enforces the maximum of the user-provided value and >> > ignore >> > + internally calculated limit. >> > + >> > + The default value is ``0``. >> > + >> > Info versions >> > ============= >> > >> > diff --git a/drivers/net/ethernet/intel/i40e/i40e.h >> > b/drivers/net/ethernet/intel/i40e/i40e.h >> > index 801a57a925da..d2d03db2acec 100644 >> > --- a/drivers/net/ethernet/intel/i40e/i40e.h >> > +++ b/drivers/net/ethernet/intel/i40e/i40e.h >> > @@ -574,6 +574,10 @@ struct i40e_pf { >> > struct i40e_vf *vf; >> > int num_alloc_vfs; /* actual number of VFs allocated */ >> > u32 vf_aq_requests; >> > + /* If set to non-zero, the device uses this value >> > + * as maximum number of MAC filters per VF. >> > + */ >> > + u32 max_mac_per_vf; >> > u32 arq_overflows; /* Not fatal, possibly indicative of problems */ >> > struct ratelimit_state mdd_message_rate_limit; >> > /* DCBx/DCBNL capability for PF that indicates >> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_devlink.c >> > b/drivers/net/ethernet/intel/i40e/i40e_devlink.c >> > index cc4e9e2addb7..8532e40b5c7d 100644 >> > --- a/drivers/net/ethernet/intel/i40e/i40e_devlink.c >> > +++ b/drivers/net/ethernet/intel/i40e/i40e_devlink.c >> > @@ -5,6 +5,42 @@ >> > #include "i40e.h" >> > #include "i40e_devlink.h" >> > >> > +static int i40e_max_mac_per_vf_set(struct devlink *devlink, >> > + u32 id, >> > + struct devlink_param_gset_ctx *ctx, >> > + struct netlink_ext_ack *extack) >> > +{ >> > + struct i40e_pf *pf = devlink_priv(devlink); >> > + >> > + pf->max_mac_per_vf = ctx->val.vu32; >> > + return 0; >> > +} >> > + >> > +static int i40e_max_mac_per_vf_get(struct devlink *devlink, >> > + u32 id, >> > + struct devlink_param_gset_ctx *ctx) >> > +{ >> > + struct i40e_pf *pf = devlink_priv(devlink); >> > + >> > + ctx->val.vu32 = pf->max_mac_per_vf; >> > + return 0; >> > +} >> > + >> > +enum i40e_dl_param_id { >> > + I40E_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX, >> > + I40E_DEVLINK_PARAM_ID_MAX_MAC_PER_VF, >> What's so i40 specific about this? Sounds pretty generic to be. >> >> >> >> > +}; >> > + >> > +static const struct devlink_param i40e_dl_params[] = { >> > + DEVLINK_PARAM_DRIVER(I40E_DEVLINK_PARAM_ID_MAX_MAC_PER_VF, >> > + "max_mac_per_vf", >> > + DEVLINK_PARAM_TYPE_U32, >> > + BIT(DEVLINK_PARAM_CMODE_RUNTIME), >> > + i40e_max_mac_per_vf_get, >> > + i40e_max_mac_per_vf_set, >> > + NULL), >> > +}; >> > + >> > static void i40e_info_get_dsn(struct i40e_pf *pf, char *buf, size_t len) >> > { >> > u8 dsn[8]; >> > @@ -165,7 +201,19 @@ void i40e_free_pf(struct i40e_pf *pf) >> > **/ >> > void i40e_devlink_register(struct i40e_pf *pf) >> > { >> > - devlink_register(priv_to_devlink(pf)); >> > + int err; >> > + struct devlink *dl = priv_to_devlink(pf); >> > + struct device *dev = &pf->pdev->dev; >> > + >> > + err = devlink_params_register(dl, i40e_dl_params, >> > + ARRAY_SIZE(i40e_dl_params)); >> > + if (err) { >> > + dev_err(dev, >> > + "devlink params register failed with error %d", err); >> > + } >> > + >> > + devlink_register(dl); >> > + >> > } >> > >> > /** >> > @@ -176,7 +224,11 @@ void i40e_devlink_register(struct i40e_pf *pf) >> > **/ >> > void i40e_devlink_unregister(struct i40e_pf *pf) >> > { >> > - devlink_unregister(priv_to_devlink(pf)); >> > + struct devlink *dl = priv_to_devlink(pf); >> > + >> > + devlink_unregister(dl); >> > + devlink_params_unregister(dl, i40e_dl_params, >> > + ARRAY_SIZE(i40e_dl_params)); >> > } >> > >> > /** >> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c >> > b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c >> > index 081a4526a2f0..e7c0c791eed1 100644 >> > --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c >> > +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c >> > @@ -2935,19 +2935,23 @@ static inline int i40e_check_vf_permission(struct >> > i40e_vf *vf, >> > if (!f) >> > ++mac_add_cnt; >> > } >> > - >> > - /* If this VF is not privileged, then we can't add more than a limited >> > - * number of addresses. >> > + /* Determine the maximum number of MAC addresses this VF may use. >> > + * >> > + * - For untrusted VFs: use a fixed small limit. >> > + * >> > + * - For trusted VFs: limit is calculated by dividing total MAC >> > + * filter pool across all VFs/ports. >> > * >> > - * If this VF is trusted, it can use more resources than untrusted. >> > - * However to ensure that every trusted VF has appropriate number of >> > - * resources, divide whole pool of resources per port and then across >> > - * all VFs. >> > + * - User can override this by devlink param "max_mac_per_vf". >> > + * If set its value is used as a strict cap. >> > */ >> > - if (!vf_trusted) >> > + if (!vf_trusted) { >> > mac_add_max = I40E_VC_MAX_MAC_ADDR_PER_VF; >> > - else >> > + } else { >> > mac_add_max = >> > I40E_VC_MAX_MACVLAN_PER_TRUSTED_VF(pf->num_alloc_vfs, hw->num_ports); >> > + if (pf->max_mac_per_vf > 0) >> > + mac_add_max = pf->max_mac_per_vf; >> > + } >> > >> > /* VF can replace all its filters in one step, in this case mac_add_max >> > * will be added as active and another mac_add_max will be in >> > @@ -2961,7 +2965,8 @@ static inline int i40e_check_vf_permission(struct >> > i40e_vf *vf, >> > return -EPERM; >> > } else { >> > dev_err(&pf->pdev->dev, >> > - "Cannot add more MAC addresses, trusted VF >> > exhausted it's resources\n"); >> > + "Cannot add more MAC addresses: trusted VF >> > reached its maximum allowed limit (%d)\n", >> > + mac_add_max); >> > return -EPERM; >> > } >> > } >> > -- >> > 2.47.3 >> > >If i understand correctly, you’re asking whether this parameter could be >added as a generic devlink parameter >rather than a driver-specific one? > > >if that the case, Yes, it could be made generic, but I initially implemented >it as driver specific because I was targeting i40e only, >and I thought a generic approach might not be acceptable at this stage > > >I do plan to extend this to other drivers, as mentioned in the v1 patch here: > - >https://patchwork.ozlabs.org/project/intel-wired-lan/patch/[email protected]/ >For now, i'm pushing hard to get this patch into i40e since it affects a >customer. Once it's accepted i can extend it >to other drivers and convert it to the generic devlink parameter.
No, please do it as generic from start. Thanks! > > >Thanks, >
