| 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
| ||
|
Message-ID: <f865bca4-40e3-4e65-9269-6c0f0ae9ad22@redhat.com>
Date: Thu, 28 Aug 2025 15:49:26 +0300
From: mohammad heib <mheib@...hat.com>
To: Jiri Pirko <jiri@...nulli.us>
Cc: intel-wired-lan@...ts.osuosl.org, przemyslawx.patynowski@...el.com,
netdev@...r.kernel.org, horms@...nel.org, jacob.e.keller@...el.com,
aleksandr.loktionov@...el.com, anthony.l.nguyen@...el.com,
przemyslaw.kitszel@...el.com
Subject: Re: [PATCH net v2] i40e: add devlink param to control VF MAC address
limit
Hi Jiri
On 8/28/25 2:43 PM, Jiri Pirko wrote:
> Sat, Aug 23, 2025 at 11:49:52AM +0200, mheib@...hat.com wrote:
>> From: Mohammad Heib <mheib@...hat.com>
>>
>> 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/20250805134042.2604897-1-dhill@redhat.com
>>
>> Fixes: cfb1d572c986 ("i40e: Add ensurance of MacVlan resources for every trusted VF")
>> Signed-off-by: Mohammad Heib <mheib@...hat.com>
>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
>> ---
>> 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/20250821233930.127420-1-mheib@redhat.com/
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.
Thanks,
Powered by blists - more mailing lists