| 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: <31709a8a-f314-4522-830d-ec81b9435f10@redhat.com>
Date: Thu, 28 Aug 2025 13:19:39 +0300
From: mohammad heib <mheib@...hat.com>
To: Tony Nguyen <anthony.l.nguyen@...el.com>, intel-wired-lan@...ts.osuosl.org
Cc: przemyslawx.patynowski@...el.com, jiri@...nulli.us,
netdev@...r.kernel.org, horms@...nel.org, jacob.e.keller@...el.com,
aleksandr.loktionov@...el.com, przemyslaw.kitszel@...el.com
Subject: Re: [PATCH net v2] i40e: add devlink param to control VF MAC address
limit
Hi Tony,
On 8/28/25 12:14 AM, Tony Nguyen wrote:
>
>
> On 8/23/2025 2:49 AM, 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.
>
> Are you intending for this to be for all VFs or only trusted? The
> changes look to be only for trusted VFs, but it's not clear to me from
> the documentation/comments.
>
>> + 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 filters are a shared resource. This will allow over-subscription;
> other VFs could be starved
> of filters due to this. Since the user/PF is controlling, this is
> probably ok but should be documented. Also, since these are shared,
> this value acts as a theoretical max value, the hardware will start
> returning errors when its limit are reached.
>
>> + 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,
>> +};
>> +
>> +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;
>
> RCT; declarations should order from longest to shortest.
>
>> + 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);
>> + }
>
> Braces not needed here.
>
> Thanks,
> Tony
>
Thank you for your review.
yes agree this need to be more documented i updated the devlink param
documentation to include your comment.
and fixed the minor code issue all in v3.
thanks.
Powered by blists - more mailing lists