Thread (33 messages) 33 messages, 6 authors, 2025-10-24

Re: [PATCH net-next v2 02/14] i40e: support generic devlink param "max_mac_per_vf"

From: mohammad heib <hidden>
Date: 2025-10-24 08:07:30
Also in: linux-doc, lkml

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:
quoted
On Tue, 21 Oct 2025 13:39:27 -0700 Jacob Keller wrote:
quoted
On 10/20/2025 6:25 PM, Jakub Kicinski wrote:
quoted
On Thu, 16 Oct 2025 23:08:31 -0700 Jacob Keller wrote:
quoted
- 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.
quoted
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,
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help