Re: [PATCH net-next,2/2] i40e: support generic devlink param "max_mac_per_vf"

Hi Aleksandr,

Thanks again for your review.
I’ve updated the documentation and commit message in v2 to address your 
feedback.

Appreciate your time!
On 9/3/25 3:35 PM, Loktionov, Aleksandr wrote:

*From:*mohammad heib [off-list ref]
*Sent:* Wednesday, September 3, 2025 12:01 PM
*To:* Loktionov, Aleksandr [off-list ref]; intel-wired- 
lan@lists.osuosl.org
*Cc:* przemyslawx.patynowski@intel.com; jiri@resnulli.us; 
netdev@vger.kernel.org; horms@kernel.org; Keller, Jacob E 
[off-list ref]; Nguyen, Anthony L 
[off-list ref]; Kitszel, Przemyslaw 
[off-list ref]
*Subject:* Re: [PATCH net-next,2/2] i40e: support generic devlink param 
"max_mac_per_vf"

Hello Aleksandr,

Thank you for your review.

On 9/3/25 12:07 PM, Loktionov, Aleksandr wrote:

        -----Original Message-----

        From:mheib@redhat.com <mailto:mheib@redhat.com> [off-list ref] <mailto:mheib@redhat.com>

        Sent: Wednesday, September 3, 2025 9:58 AM

        To:intel-wired-lan@lists.osuosl.org <mailto:intel-wired-lan@lists.osuosl.org>

        Cc:przemyslawx.patynowski@intel.com <mailto:przemyslawx.patynowski@intel.com>;jiri@resnulli.us <mailto:jiri@resnulli.us>;

        netdev@vger.kernel.org <mailto:netdev@vger.kernel.org>;horms@kernel.org <mailto:horms@kernel.org>; Keller, Jacob E

        [off-list ref] <mailto:jacob.e.keller@intel.com>; Loktionov, Aleksandr

        [off-list ref] <mailto:aleksandr.loktionov@intel.com>; Nguyen, Anthony L

        [off-list ref] <mailto:anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw

        [off-list ref] <mailto:przemyslaw.kitszel@intel.com>; Mohammad Heib[off-list ref] <mailto:mheib@redhat.com>

        Subject: [PATCH net-next,2/2] i40e: support generic devlink param

        "max_mac_per_vf"

        From: Mohammad Heib[off-list ref] <mailto:mheib@redhat.com>

        Add support for the new generic devlink runtime parameter

        "max_mac_per_vf", which controls the maximum number of MAC addresses a

        trusted VF can use.

    Good day Mohammad,

    Thanks for working on this and for the clear explanation in the commit message.

    I have a couple of questions and thoughts:

    1) Scope of the parameter

         The name max_mac_per_vf is a bit ambiguous. From the description,

         it seems to apply only to trusted VFs, but the name does not make that obvious.

         Would it make sense to either:

      - Make the name reflect that (e.g., max_mac_per_trusted_vf), or

      - Introduce two separate parameters for trusted and untrusted VFs if both cases need to be handled differently?

I agree that the name could be a bit confusing. Since this is a generic 
devlink parameter, different devices may handle trusted and untrusted 
VFs differently.
For i40e specifically, the device does treat trusted VFs differently 
from untrusted ones, and this is documented in devlink/i40e.rst.
However, I chose a more general name to avoid creating a separate 
devlink parameter for untrusted VFs, which likely wouldn’t be used.
On reflection, I should update the patch number 1 to remove the 
**trusted VF** wording from the description to avoid implying that the 
parameter only applies to trusted VFs.

    I believe the community generally aims for solutions that work
    consistently across different hardware. If this parameter behaves
    differently on i40e compared to mlx5 (or other drivers), it might be
    helpful to mention that explicitly in the documentation or commit
    message.

    2)Problem statement

         It would help to better understand the underlying problem this parameter is solving.

         Is the goal to enforce a global cap for all VFs, or to provide operators with a way

         to fine-tune per-VF limits? From my perspective, the most important part is

         clearly stating the problem and the use case.

My main goal here is to enforce a global cap for all VFs.
There was a long discussion [1] about this, and one of the ideas raised 
was to create fine-tuned per-VF limits using devlink resources instead 
of a parameter
However, currently in i40e, we only create a devlink port per PF and no 
devlink ports per VF.
Implementing the resource-per-VF approach would therefore require some 
extra work.
so i decided to go with this global cap for now.
[1] - https://patchwork.kernel.org/project/netdevbpf/ 
patch/20250805134042.2604897-2-dhill@redhat.com/ <https:// 
patchwork.kernel.org/project/netdevbpf/patch/20250805134042.2604897-2- 
dhill@redhat.com/>

Thank, you Mohammad

The https://patchwork.kernel.org/project/netdevbpf/ 
patch/20250805134042.2604897-2-dhill@redhat.com/ <https:// 
patchwork.kernel.org/project/netdevbpf/patch/20250805134042.2604897-2- 
dhill@redhat.com/> explains many things.

It might be helpful to include a brief description of the problem being 
solved directly in the commit message. This gives reviewers the 
necessary context and makes it easier to understand the motivation 
behind the change.

    …