[PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Firmware version query is supported on the PFs. Due to this
following kernel warning log is observed:

[  188.590344] mlx5_core 0000:08:00.2: mlx5_fw_version_query:816:(pid 1453): fw query isn't supported by the FW

Fix it by restricting the query and devlink info to the PF.

Fixes: 8338d9378895 ("net/mlx5: Added devlink info callback")
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/devlink.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index 98d4306929f3..a2cf3e79693d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -46,6 +46,9 @@ mlx5_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	u32 running_fw, stored_fw;
 	int err;
 
+	if (!mlx5_core_is_pf(dev))
+		return 0;
+
 	err = devlink_info_version_fixed_put(req, "fw.psid", dev->board_id);
 	if (err)
 		return err;
-- 
2.48.1

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Kalesh Anakkur Purayil]

[-- Attachment #1: Type: text/plain, Size: 614 bytes --]

On Mon, Mar 3, 2025 at 7:07 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
> From: Jiri Pirko <jiri@nvidia.com>
>
> Firmware version query is supported on the PFs. Due to this
> following kernel warning log is observed:
>
> [  188.590344] mlx5_core 0000:08:00.2: mlx5_fw_version_query:816:(pid 1453): fw query isn't supported by the FW
>
> Fix it by restricting the query and devlink info to the PF.
>
> Fixes: 8338d9378895 ("net/mlx5: Added devlink info callback")
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>


-- 
Regards,
Kalesh AP

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4239 bytes --]

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Tariq Toukan]

On 03/03/2025 15:32, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Firmware version query is supported on the PFs. Due to this
> following kernel warning log is observed:
> 
> [  188.590344] mlx5_core 0000:08:00.2: mlx5_fw_version_query:816:(pid 1453): fw query isn't supported by the FW
> 
> Fix it by restricting the query and devlink info to the PF.
> 
> Fixes: 8338d9378895 ("net/mlx5: Added devlink info callback")
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
>   drivers/net/ethernet/mellanox/mlx5/core/devlink.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
> index 98d4306929f3..a2cf3e79693d 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
> @@ -46,6 +46,9 @@ mlx5_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
>   	u32 running_fw, stored_fw;
>   	int err;
>   
> +	if (!mlx5_core_is_pf(dev))
> +		return 0;
> +
>   	err = devlink_info_version_fixed_put(req, "fw.psid", dev->board_id);
>   	if (err)
>   		return err;

Reviewed-by: Tariq Toukan <tariqt@nvidia.com>

Thanks.

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Jakub Kicinski]

On Wed, 5 Mar 2025 20:55:15 +0200 Tariq Toukan wrote:
> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>

Too late, take it via your tree, please. 
You need to respond within 24h or take the patches.

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Jiri Pirko]

Thu, Mar 06, 2025 at 03:30:16AM +0100, kuba@kernel.org wrote:
>On Wed, 5 Mar 2025 20:55:15 +0200 Tariq Toukan wrote:
>> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
>
>Too late, take it via your tree, please. 
>You need to respond within 24h or take the patches.

Can I repost with Tariq's tag (I was not aware it is needed). I have a
net-next patchset based on top of this I would like to push.

Thanks!

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Jakub Kicinski]

On Thu, 6 Mar 2025 13:47:27 +0100 Jiri Pirko wrote:
> Thu, Mar 06, 2025 at 03:30:16AM +0100, kuba@kernel.org wrote:
> >On Wed, 5 Mar 2025 20:55:15 +0200 Tariq Toukan wrote:  
> >> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>  
> >
> >Too late, take it via your tree, please. 
> >You need to respond within 24h or take the patches.  
> 
> Can I repost with Tariq's tag (I was not aware it is needed). I have a
> net-next patchset based on top of this I would like to push.

My knee jerk reaction would be to say yes, but we really need
to protect the sanity of core maintainers. You're both working
for the same company, you should be able to figure this out
between each other. We expect main stream of mlx5 patches to
come from Tariq.

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Tariq Toukan]

On 06/03/2025 4:30, Jakub Kicinski wrote:
> On Wed, 5 Mar 2025 20:55:15 +0200 Tariq Toukan wrote:
>> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
> 
> Too late, take it via your tree, please.
> You need to respond within 24h or take the patches.

Never heard of a 24h rule. Not clear to me what rule you're talking 
about, what's the rationale behind it, and where it's coming from.

It's pretty obvious for everyone that responding within 24h cannot be 
committed, and is not always achievable.

Moreover, this contradicts with maintainer-netdev.rst, which explicitly 
aligns the expected review timeline to be 48h for triage, also to give 
the opportunity for more reviewers to share their thoughts.

Tariq.

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Jakub Kicinski]

On Thu, 6 Mar 2025 21:20:58 +0200 Tariq Toukan wrote:
> On 06/03/2025 4:30, Jakub Kicinski wrote:
> > On Wed, 5 Mar 2025 20:55:15 +0200 Tariq Toukan wrote:  
> >> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>  
> > 
> > Too late, take it via your tree, please.
> > You need to respond within 24h or take the patches.  
> 
> Never heard of a 24h rule. Not clear to me what rule you're talking 
> about, what's the rationale behind it, and where it's coming from.
> 
> It's pretty obvious for everyone that responding within 24h cannot be 
> committed, and is not always achievable.
> 
> Moreover, this contradicts with maintainer-netdev.rst, which explicitly 
> aligns the expected review timeline to be 48h for triage, also to give 
> the opportunity for more reviewers to share their thoughts.

Quoting documentation:

  Responsibilities
  ================
  
  The amount of maintenance work is usually proportional to the size
  and popularity of the code base. Small features and drivers should
  require relatively small amount of care and feeding. Nonetheless
  when the work does arrive (in form of patches which need review,
  user bug reports etc.) it has to be acted upon promptly.
  Even when a particular driver only sees one patch a month, or a quarter,
  a subsystem could well have a hundred such drivers. Subsystem
  maintainers cannot afford to wait a long time to hear from reviewers.
  
  The exact expectations on the response time will vary by subsystem.
  The patch review SLA the subsystem had set for itself can sometimes
  be found in the subsystem documentation. Failing that as a rule of thumb
  reviewers should try to respond quicker than what is the usual patch
  review delay of the subsystem maintainer. The resulting expectations
  may range from two working days for fast-paced subsystems (e.g. networking)
  to as long as a few weeks in slower moving parts of the kernel.
  
See: https://www.kernel.org/doc/html/next/maintainer/feature-and-driver-maintainers.html#responsibilities

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Tariq Toukan]

On 06/03/2025 21:39, Jakub Kicinski wrote:
> On Thu, 6 Mar 2025 21:20:58 +0200 Tariq Toukan wrote:
>> On 06/03/2025 4:30, Jakub Kicinski wrote:
>>> On Wed, 5 Mar 2025 20:55:15 +0200 Tariq Toukan wrote:
>>>> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
>>>
>>> Too late, take it via your tree, please.
>>> You need to respond within 24h or take the patches.
>>
>> Never heard of a 24h rule. Not clear to me what rule you're talking
>> about, what's the rationale behind it, and where it's coming from.
>>
>> It's pretty obvious for everyone that responding within 24h cannot be
>> committed, and is not always achievable.
>>
>> Moreover, this contradicts with maintainer-netdev.rst, which explicitly
>> aligns the expected review timeline to be 48h for triage, also to give
>> the opportunity for more reviewers to share their thoughts.
> 
> Quoting documentation:
> 

Thanks for the pointer.

>    Responsibilities
>    ================
>    
>    The amount of maintenance work is usually proportional to the size
>    and popularity of the code base. Small features and drivers should
>    require relatively small amount of care and feeding. Nonetheless
>    when the work does arrive (in form of patches which need review,
>    user bug reports etc.) it has to be acted upon promptly.
>    Even when a particular driver only sees one patch a month, or a quarter,
>    a subsystem could well have a hundred such drivers. Subsystem
>    maintainers cannot afford to wait a long time to hear from reviewers.
>    
>    The exact expectations on the response time will vary by subsystem.
>    The patch review SLA the subsystem had set for itself can sometimes
>    be found in the subsystem documentation. Failing that as a rule of thumb
>    reviewers should try to respond quicker than what is the usual patch
>    review delay of the subsystem maintainer. The resulting expectations
>    may range from two working days for fast-paced subsystems (e.g. networking)

So no less than two working days for any subsystem.
Okay, now this makes more sense.

>    to as long as a few weeks in slower moving parts of the kernel.
>    
> See: https://www.kernel.org/doc/html/next/maintainer/feature-and-driver-maintainers.html#responsibilities

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Jakub Kicinski]

On Thu, 6 Mar 2025 22:12:52 +0200 Tariq Toukan wrote:
> >    The exact expectations on the response time will vary by subsystem.
> >    The patch review SLA the subsystem had set for itself can sometimes
> >    be found in the subsystem documentation. Failing that as a rule of thumb
> >    reviewers should try to respond quicker than what is the usual patch
> >    review delay of the subsystem maintainer. The resulting expectations
> >    may range from two working days for fast-paced subsystems (e.g. networking)  
> 
> So no less than two working days for any subsystem.
> Okay, now this makes more sense.

Could you explain to me why this is a problem for you?
You're obviously capable of opening your email client once a day.
Are you waiting for QA results or some such?

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Tariq Toukan]

On 06/03/2025 22:34, Jakub Kicinski wrote:
> On Thu, 6 Mar 2025 22:12:52 +0200 Tariq Toukan wrote:
>>>     The exact expectations on the response time will vary by subsystem.
>>>     The patch review SLA the subsystem had set for itself can sometimes
>>>     be found in the subsystem documentation. Failing that as a rule of thumb
>>>     reviewers should try to respond quicker than what is the usual patch
>>>     review delay of the subsystem maintainer. The resulting expectations
>>>     may range from two working days for fast-paced subsystems (e.g. networking)
>>
>> So no less than two working days for any subsystem.
>> Okay, now this makes more sense.
> 
> Could you explain to me why this is a problem for you?

Nothing special about "me" here.

I thought it's obvious. As patches are coming asynchronously, it is not 
rare to won't be able to respond within a single working day.

This becomes significantly rarer with two consecutive working days.

> You're obviously capable of opening your email client once a day.
> Are you waiting for QA results or some such?

No. I update when I do.

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Jakub Kicinski]

On Thu, 6 Mar 2025 23:13:33 +0200 Tariq Toukan wrote:
> >> So no less than two working days for any subsystem.
> >> Okay, now this makes more sense.  
> > 
> > Could you explain to me why this is a problem for you?  
> 
> Nothing special about "me" here.
> 
> I thought it's obvious. As patches are coming asynchronously, it is not 
> rare to won't be able to respond within a single working day.
> 
> This becomes significantly rarer with two consecutive working days.

On one thread Saeed is claiming that nVidia is a major netdev
contributor, and pillar of the community.

On another thread it's too hard for the main mlx5 maintainer
to open their inbox once a day to look at patches for their
own driver, that they were CCed on.

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Saeed Mahameed]

On 06 Mar 13:46, Jakub Kicinski wrote:
>On Thu, 6 Mar 2025 23:13:33 +0200 Tariq Toukan wrote:
>> >> So no less than two working days for any subsystem.
>> >> Okay, now this makes more sense.
>> >
>> > Could you explain to me why this is a problem for you?
>>
>> Nothing special about "me" here.
>>
>> I thought it's obvious. As patches are coming asynchronously, it is not
>> rare to won't be able to respond within a single working day.
>>
>> This becomes significantly rarer with two consecutive working days.
>
>On one thread Saeed is claiming that nVidia is a major netdev
>contributor, and pillar of the community.
>
>On another thread it's too hard for the main mlx5 maintainer
>to open their inbox once a day to look at patches for their
>own driver, that they were CCed on.
>

People have personal circumstances and constraints it has nothing to do
with the employer or the fact that they are a maintainer or a major
contributor.
24hr is a bit too tight for some people which is understandable,
48hr response time works well for everyone.

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Gal Pressman]

On 06/03/2025 23:46, Jakub Kicinski wrote:
> On Thu, 6 Mar 2025 23:13:33 +0200 Tariq Toukan wrote:
>>>> So no less than two working days for any subsystem.
>>>> Okay, now this makes more sense.  
>>>
>>> Could you explain to me why this is a problem for you?  
>>
>> Nothing special about "me" here.
>>
>> I thought it's obvious. As patches are coming asynchronously, it is not 
>> rare to won't be able to respond within a single working day.
>>
>> This becomes significantly rarer with two consecutive working days.
> 
> On one thread Saeed is claiming that nVidia is a major netdev
> contributor, and pillar of the community.

Do you not agree?

> 
> On another thread it's too hard for the main mlx5 maintainer
> to open their inbox once a day to look at patches for their
> own driver, that they were CCed on.

Is this new rule mlx5 specific? How come the driver maintainers are not
aware of it?

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Jakub Kicinski]

On Thu, 6 Mar 2025 21:20:58 +0200 Tariq Toukan wrote:
> It's pretty obvious for everyone that responding within 24h cannot be 
> committed, and is not always achievable.

On the occasion that you are too busy to look at the list on a given
day you can just take the patch via your own tree. It's not like we're
marking the patch as Rejected, we're marking it as Awaiting Upstream.

nvidia's employees patches should go via your tree, in the first place.
What are you even arguing about. Seriously.

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Tariq Toukan]

On 06/03/2025 21:43, Jakub Kicinski wrote:
> On Thu, 6 Mar 2025 21:20:58 +0200 Tariq Toukan wrote:
>> It's pretty obvious for everyone that responding within 24h cannot be
>> committed, and is not always achievable.
> 
> On the occasion that you are too busy to look at the list on a given
> day you can just take the patch via your own tree. It's not like we're
> marking the patch as Rejected, we're marking it as Awaiting Upstream.
> 
> nvidia's employees patches should go via your tree, in the first place.
> What are you even arguing about. Seriously.

Now this is a different argument.
For this, I'll resend the patch now.

Regarding the other argument, I still didn't see a single place stating 
24h (or a single working day).

Thanks.

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Jiri Pirko]

Thu, Mar 06, 2025 at 08:43:55PM +0100, kuba@kernel.org wrote:

[...]


>nvidia's employees patches should go via your tree, in the first place.

Why? I've been in Mellanox/Nvidia for almost 10 years and this is
actually the first time I hit this. I'm used to send patches directly,
I've been doing that for almost 15+ years and this was never issue.
Where this rule is written down? What changed?

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Jakub Kicinski]

On Fri, 7 Mar 2025 13:35:28 +0100 Jiri Pirko wrote:
> >nvidia's employees patches should go via your tree, in the first place.  
> 
> Why? I've been in Mellanox/Nvidia for almost 10 years and this is
> actually the first time I hit this. I'm used to send patches directly,
> I've been doing that for almost 15+ years and this was never issue.

You probably mostly change the driver together with core, like devlink.
In those cases you can post outside the main stream.

> Where this rule is written down? What changed?

It was always like this, since before I became a maintainer.
I think what happened is Saeed handed the maintainership over
without "writing down" all the rules that we established over
the years.

Obviously he will now probably disagree with me. Because y'all
apparently have no time to review patches, but playing victim
you have all the time in the world for.

There are only two vendors big enough to warrant a special process
(Intel and nVidia), and we make reasonable accommodations (like
the one above) so it's impractical to write all the rules down.

Not to mention the fact that you should perhaps, in an ideal 
world, just try to be a good community member, instead acting
as if we're signing a business contract.

Have a nice weekend.

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Jiri Pirko]

Fri, Mar 07, 2025 at 05:20:53PM +0100, kuba@kernel.org wrote:
>On Fri, 7 Mar 2025 13:35:28 +0100 Jiri Pirko wrote:
>> >nvidia's employees patches should go via your tree, in the first place.  
>> 
>> Why? I've been in Mellanox/Nvidia for almost 10 years and this is
>> actually the first time I hit this. I'm used to send patches directly,
>> I've been doing that for almost 15+ years and this was never issue.
>
>You probably mostly change the driver together with core, like devlink.
>In those cases you can post outside the main stream.

Okay, that is mostly what I do, correct.


>
>> Where this rule is written down? What changed?
>
>It was always like this, since before I became a maintainer.
>I think what happened is Saeed handed the maintainership over
>without "writing down" all the rules that we established over
>the years.
>
>Obviously he will now probably disagree with me. Because y'all
>apparently have no time to review patches, but playing victim
>you have all the time in the world for.
>
>There are only two vendors big enough to warrant a special process
>(Intel and nVidia), and we make reasonable accommodations (like
>the one above) so it's impractical to write all the rules down.
>
>Not to mention the fact that you should perhaps, in an ideal 
>world, just try to be a good community member, instead acting
>as if we're signing a business contract.

I'm always trying to be a good community member. I just can't behave in
cases I have no clue how, that's all. :)


>
>Have a nice weekend.

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Leon Romanovsky]

On Fri, Mar 07, 2025 at 01:35:28PM +0100, Jiri Pirko wrote:
> Thu, Mar 06, 2025 at 08:43:55PM +0100, kuba@kernel.org wrote:
> 
> [...]
> 
> 
> >nvidia's employees patches should go via your tree, in the first place.
> 
> Why? I've been in Mellanox/Nvidia for almost 10 years and this is
> actually the first time I hit this. I'm used to send patches directly,
> I've been doing that for almost 15+ years and this was never issue.
> Where this rule is written down? What changed?

Maintainer?

I'm second to your feelings.

Long standing contributors sent patches mostly directly. It was definitely
encouraged to send long series through Saeed, but simple, one line fixes were
accepted without extra hop.

It was easy and convenient for everyone.

Thanks

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Saeed Mahameed]

On 05 Mar 18:30, Jakub Kicinski wrote:
>On Wed, 5 Mar 2025 20:55:15 +0200 Tariq Toukan wrote:
>> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
>
>Too late, take it via your tree, please.
>You need to respond within 24h or take the patches.
>

I thought it was 48h.. I remember the rule since David Miller's era.
Maintainers should provide feedback within 48hrs of posting.

I agree with the policy but 24hr is really pushing it..

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Jakub Kicinski]

On Thu, 6 Mar 2025 11:32:52 -0800 Saeed Mahameed wrote:
> I thought it was 48h

Always happy to hear your thoughts Saeed. When you have a sec, could
you do the math between the following two dates for me, and tell me 
if it's less or more than 48 hours.

Date: Mon,  3 Mar 2025 14:32:00 +0100
Date: Wed,  5 Mar 2025 20:55:15 +0200

Thank you!

Re: [PATCH net] net/mlx5: Fill out devlink dev info only for PFs

[Saeed Mahameed]

On 06 Mar 11:45, Jakub Kicinski wrote:
>On Thu, 6 Mar 2025 11:32:52 -0800 Saeed Mahameed wrote:
>> I thought it was 48h
>
>Always happy to hear your thoughts Saeed. When you have a sec, could
>you do the math between the following two dates for me, and tell me
>if it's less or more than 48 hours.

I was talking about the 24h expectation, it is too tight.
48hr was always the expectation in netdev mailing list, let's keep it that
way, If any of the driver maintainers is late to respond then they will
have to take it to own queue.

>
>Date: Mon,  3 Mar 2025 14:32:00 +0100
>Date: Wed,  5 Mar 2025 20:55:15 +0200
>

Sure I wasn't arguing about what happened here, I agree with the policy,
I just didn't understand where the 24hr thing came from, it is 48h.

>Thank you!