Message-ID: <fd9bfe41-bff8-472a-a50b-e2089f3c6351@redhat.com>
Date: Fri, 24 Oct 2025 11:07:22 +0300
From: mohammad heib <mheib@...hat.com>
To: Jacob Keller <jacob.e.keller@...el.com>, Jakub Kicinski <kuba@...nel.org>
Cc: Jiri Pirko <jiri@...nulli.us>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
 Simon Horman <horms@...nel.org>, Jonathan Corbet <corbet@....net>,
 Tony Nguyen <anthony.l.nguyen@...el.com>,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 Andrew Lunn <andrew+netdev@...n.ch>,
 Alexander Lobakin <aleksander.lobakin@...el.com>, netdev@...r.kernel.org,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
 Aleksandr Loktionov <aleksandr.loktionov@...el.com>,
 Rafal Romanowski <rafal.romanowski@...el.com>
Subject: Re: [PATCH net-next v2 02/14] i40e: support generic devlink param
 "max_mac_per_vf"


Thanks Jakub and Jacob for the feedback.

On 10/23/25 1:11 AM, Jacob Keller wrote:
> 
> 
> On 10/21/2025 4:07 PM, Jakub Kicinski wrote:
>> On Tue, 21 Oct 2025 13:39:27 -0700 Jacob Keller wrote:
>>> On 10/20/2025 6:25 PM, Jakub Kicinski wrote:
>>>> On Thu, 16 Oct 2025 23:08:31 -0700 Jacob Keller wrote:
>>>>> - The configured value is a theoretical maximum. Hardware limits may
>>>>>    still prevent additional MAC addresses from being added, even if the
>>>>>    parameter allows it.
>>>>
>>>> Is "administrative policy" better than "theoretical max" ?
>>>
>>> That could be a bit more accurate.
>>>
>>>> Also -- should we be scanning the existing state to check if some VM
>>>> hasn't violated the new setting and error or at least return a extack
>>>> to the user to warn that the policy is not currently adhered to?
>>>
>>> My understanding here is that this enforces the VF to never go *above*
>>> this value, but its possible some other hardware restriction (i.e. out
>>> of filters) could prevent a VF from adding more filters even if the
>>> value is set higher.
>>>
>>> Basically, this sets the maximum allowed number of filters, but doesn't
>>> guarantee that many filters are actually available, at least on X710
>>> where filters are a shared resource and we do not have a good mechanism
>>> to coordinate across PFs to confirm how many have been made available or
>>> reserved already. (Until firmware rejects adding a filter because
>>> resources are capped)
>>>
>>> Thus, I don't think we need to scan to check anything here. VFs should
>>> be unable to exceed this limit, and thats checked on filter add.
>>
>> Sorry, just to be clear -- this comment is independent on the comment
>> about "policy" vs "theoretical".
>>
>> What if:
>>   - max is set to 4
>>   - VF 1 adds 4 filters
>>   - (some time later) user asks to decrease max to 2
>>
>> The devlink param is CMODE_RUNTIME so I'm assuming it can be tweaked
>> at any point in time.
>>
>> We probably don't want to prevent lowering the max as admin has no way
>> to flush the filters. Either we don't let the knob be turned when SRIOV
>> is enabled or we should warn if some VF has more filters than the new
>> max?
> 
> Ah, yes that makes sense to me. I think the best approach is just return
> -EBUSY if there are active VFs. We could implement warning logic
> instead, but I think most of the time the administrator should be
> expected to configure this once during setup (i.e. a boot up script or
> something), and not during runtime.

To make sure I understood correctly before sending the next version:
  - I need to update the parameter documentation to describe it as an 
   administrative policy limit instead of a “theoretical maximum.

  - I need to modify the set callback to return -EBUSY if VFs are 
already allocated, so the parameter can only be changed before enabling 
SR-IOV.

  - I need to mention this behavior explicitly in the i40e devlink 
parameter description.

Also, just to confirm — should I resend the updated patch directly to 
the upstream netdev mailing list, or should it go through the IWL list 
first?

Thanks,

Powered by blists - more mailing lists