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

[mohammad heib <mheib@redhat.com>]

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 <mheib@redhat.com>
> *Sent:* Wednesday, September 3, 2025 12:01 PM
> *To:* Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; intel-wired- 
> lan@lists.osuosl.org
> *Cc:* przemyslawx.patynowski@intel.com; jiri@resnulli.us; 
> netdev@vger.kernel.org; horms@kernel.org; Keller, Jacob E 
> <jacob.e.keller@intel.com>; Nguyen, Anthony L 
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw 
> <przemyslaw.kitszel@intel.com>
> *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> <mheib@redhat.com> <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
> 
>         <jacob.e.keller@intel.com> <mailto:jacob.e.keller@intel.com>; Loktionov, Aleksandr
> 
>         <aleksandr.loktionov@intel.com> <mailto:aleksandr.loktionov@intel.com>; Nguyen, Anthony L
> 
>         <anthony.l.nguyen@intel.com> <mailto:anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> 
>         <przemyslaw.kitszel@intel.com> <mailto:przemyslaw.kitszel@intel.com>; Mohammad Heib<mheib@redhat.com> <mailto:mheib@redhat.com>
> 
>         Subject: [PATCH net-next,2/2] i40e: support generic devlink param
> 
>         "max_mac_per_vf"
> 
>         From: Mohammad Heib<mheib@redhat.com> <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.
> 
>     …
>