| Message ID | 20250303133200.1505-1-jiri@resnulli.us (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net/mlx5: Fill out devlink dev info only for PFs | expand |
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>
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.
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.
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!
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.
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.
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..
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
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.
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!
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
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?
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.
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.
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.
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!
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.
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?
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?
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.
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
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.
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;