| Message ID | 20230606071219.483255-14-saeed@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [net-next,01/15] RDMA/mlx5: Free second uplink ib port | expand |
On Tue, 6 Jun 2023 00:12:17 -0700 Saeed Mahameed wrote: > Fixes: bffaa916588e ("net/mlx5: E-Switch, Add control for inline mode") > Fixes: 8c98ee77d911 ("net/mlx5e: E-Switch, Add extack messages to devlink callbacks") The combination of net-next and Fixes is always odd. Why? Either it's important enough to be a fix or its not important and can go to net-next...
On Tue, Jun 06, 2023 at 10:01:17PM -0700, Jakub Kicinski wrote: > On Tue, 6 Jun 2023 00:12:17 -0700 Saeed Mahameed wrote: > > Fixes: bffaa916588e ("net/mlx5: E-Switch, Add control for inline mode") > > Fixes: 8c98ee77d911 ("net/mlx5e: E-Switch, Add extack messages to devlink callbacks") > > The combination of net-next and Fixes is always odd. > Why? > Either it's important enough to be a fix or its not important > and can go to net-next... Generally I tell people to mark things as Fixes if it is a fix, regardless of how small, minor or unimportant. It helps backporters because they can suck in the original patch and all the touchups then test that result. If people try to predict if it is "important" or not they get it wrong quite often. Fixes is not supposed to mean "this is important" or "send this to -rc" or "apply it to -stable" If it is really important add a 'cc: stable'. If it is sort of important then send it to the -rc tree. Otherwise dump it in the merge window. But mark it with Fixes regardless Jason
Wed, Jun 07, 2023 at 07:01:17AM CEST, kuba@kernel.org wrote: >On Tue, 6 Jun 2023 00:12:17 -0700 Saeed Mahameed wrote: >> Fixes: bffaa916588e ("net/mlx5: E-Switch, Add control for inline mode") >> Fixes: 8c98ee77d911 ("net/mlx5e: E-Switch, Add extack messages to devlink callbacks") > >The combination of net-next and Fixes is always odd. >Why? >Either it's important enough to be a fix or its not important >and can go to net-next... > As Jason wrote, this is a fix, but not -net worthy. I believe that "Fixes" tag should be there regardless of the target tree, it makes things easier to follow.
On Wed, 7 Jun 2023 11:10:42 -0300 Jason Gunthorpe wrote: > On Tue, Jun 06, 2023 at 10:01:17PM -0700, Jakub Kicinski wrote: > > On Tue, 6 Jun 2023 00:12:17 -0700 Saeed Mahameed wrote: > > > Fixes: bffaa916588e ("net/mlx5: E-Switch, Add control for inline mode") > > > Fixes: 8c98ee77d911 ("net/mlx5e: E-Switch, Add extack messages to devlink callbacks") > > > > The combination of net-next and Fixes is always odd. > > Why? > > Either it's important enough to be a fix or its not important > > and can go to net-next... > > Generally I tell people to mark things as Fixes if it is a fix, > regardless of how small, minor or unimportant. Yes, exactly, we do the same, but also to send them all to net. > It helps backporters because they can suck in the original patch and > all the touchups then test that result. If people try to predict if it > is "important" or not they get it wrong quite often. > > Fixes is not supposed to mean "this is important" or "send this to > -rc" or "apply it to -stable" Agreed with the distinction that we consider every fix -rc worthy. We'll obviously apply our own judgment but submitter should send all fixes against net. > If it is really important add a 'cc: stable'. > > If it is sort of important then send it to the -rc tree. > > Otherwise dump it in the merge window. You just said that people can't predict the importance of their fixes and yet you draw categories. > But mark it with Fixes regardless Every subsystem can make their own rules. In netdev Fixes go to net.
On Wed, 7 Jun 2023 17:46:09 +0200 Jiri Pirko wrote: > >The combination of net-next and Fixes is always odd. > >Why? > >Either it's important enough to be a fix or its not important > >and can go to net-next... > > As Jason wrote, this is a fix, but not -net worthy. I believe that > "Fixes" tag should be there regardless of the target tree, > it makes things easier to follow. No it doesn't. Both as a maintainer and a person doing backports for a production kernel I'm telling you that it doesn't. Fishing a gazillion patches with random Fixes tags during the merge window, 2 months after they had been merged is *not* helping anyone. And as it usually happens fixes people consider "not important enough" are also usually trivial so very low risk of regression. Maybe it makes it easier for you to stack patches but that's secondary..
On Wed, Jun 07, 2023 at 09:19:09AM -0700, Jakub Kicinski wrote: > > If it is really important add a 'cc: stable'. > > > > If it is sort of important then send it to the -rc tree. > > > > Otherwise dump it in the merge window. > > You just said that people can't predict the importance of their fixes > and yet you draw categories. The categories exist in our workflow - we have two patch streams to Linus and the cc stable mechanism for Greg and others. This is what Greg/Linus defined. Maintainers have to funnel patches into these streams. I said people have trouble categorizing their own work into each stream. Many people legitimately disagree what should go in each stream. If patch comes to the list with the best guess then the community/maintainer can help and create some consistency. If people self-censor their Fixes lines this is harder. Further, there is a legitimate disagreement on what should and should not be backported. Keeping the Fixes lines allows everyone to make their own choices. If something doesn't go to -rc because it doesn't meet Linus's threshold it may still need to be backported. > > But mark it with Fixes regardless > > Every subsystem can make their own rules. In netdev Fixes go to net. I don't really like this position that every maintainer and every subsystem can do whatever they want. Jason
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c index 29de4e759f4f..eafb098db6b0 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c @@ -2178,6 +2178,7 @@ static int esw_offloads_start(struct mlx5_eswitch *esw, "Failed setting eswitch to offloads"); esw->mode = MLX5_ESWITCH_LEGACY; mlx5_rescan_drivers(esw->dev); + return err; } if (esw->offloads.inline_mode == MLX5_INLINE_MODE_NONE) { if (mlx5_eswitch_inline_mode_get(esw, @@ -2187,7 +2188,7 @@ static int esw_offloads_start(struct mlx5_eswitch *esw, "Inline mode is different between vports"); } } - return err; + return 0; } static void mlx5_esw_offloads_rep_mark_set(struct mlx5_eswitch *esw,