* Re: [patch] net/mlx5: missing error code in esw_create_offloads_fdb_table()
2016-07-13 10:08 [patch] net/mlx5: missing error code in esw_create_offloads_fdb_table() Dan Carpenter
@ 2016-07-13 11:19 ` Matan Barak
2016-07-13 11:40 ` Or Gerlitz
2016-07-13 11:29 ` Leon Romanovsky
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Matan Barak @ 2016-07-13 11:19 UTC (permalink / raw)
To: Dan Carpenter, Or Gerlitz
Cc: Leon Romanovsky, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA
On 13/07/2016 13:08, Dan Carpenter wrote:
> We accidentally return success when we had intended to return an error
> code.
>
> Fixes: 69697b6e2086 ('net/mlx5: E-Switch, Add support for the sriov offloads mode')
> Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> index 1842dfb..7d982cf 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> @@ -183,6 +183,7 @@ static int esw_create_offloads_fdb_table(struct mlx5_eswitch *esw, int nvports)
>
> root_ns = mlx5_get_flow_namespace(dev, MLX5_FLOW_NAMESPACE_FDB);
> if (!root_ns) {
> + err = -EINVAL;
> esw_warn(dev, "Failed to get FDB flow namespace\n");
> goto ns_err;
> }
>
Hi,
Thanks for the patch.
I'm not sure EINVAL is the right error here though.
Maybe -ENOTSUPP is a bit more appropriate here.
Regards,
Matan
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [patch] net/mlx5: missing error code in esw_create_offloads_fdb_table()
2016-07-13 11:19 ` Matan Barak
@ 2016-07-13 11:40 ` Or Gerlitz
2016-07-14 20:09 ` Leon Romanovsky
0 siblings, 1 reply; 10+ messages in thread
From: Or Gerlitz @ 2016-07-13 11:40 UTC (permalink / raw)
To: Matan Barak, Dan Carpenter
Cc: Leon Romanovsky, netdev, linux-rdma, kernel-janitors
On 7/13/2016 2:19 PM, Matan Barak wrote:
> I'm not sure EINVAL is the right error here though.
> Maybe -ENOTSUPP is a bit more appropriate here.
I agree, Dan, can you please change to be along Matan's suggestion?
Or.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] net/mlx5: missing error code in esw_create_offloads_fdb_table()
2016-07-13 11:40 ` Or Gerlitz
@ 2016-07-14 20:09 ` Leon Romanovsky
0 siblings, 0 replies; 10+ messages in thread
From: Leon Romanovsky @ 2016-07-14 20:09 UTC (permalink / raw)
To: Or Gerlitz
Cc: Matan Barak, Dan Carpenter, netdev, linux-rdma, kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 663 bytes --]
On Wed, Jul 13, 2016 at 02:40:26PM +0300, Or Gerlitz wrote:
> On 7/13/2016 2:19 PM, Matan Barak wrote:
> >I'm not sure EINVAL is the right error here though.
> >Maybe -ENOTSUPP is a bit more appropriate here.
>
> I agree, Dan, can you please change to be along Matan's suggestion?
Or,
Dan already did it before Matan's response and we have very vivid discussion about it [1].
[1] http://marc.info/?t=146841063000002&r=1&w=2
>
> Or.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] net/mlx5: missing error code in esw_create_offloads_fdb_table()
2016-07-13 10:08 [patch] net/mlx5: missing error code in esw_create_offloads_fdb_table() Dan Carpenter
2016-07-13 11:19 ` Matan Barak
@ 2016-07-13 11:29 ` Leon Romanovsky
2016-07-13 11:53 ` [patch v2] " Matan Barak
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Leon Romanovsky @ 2016-07-13 11:29 UTC (permalink / raw)
To: Dan Carpenter
Cc: Matan Barak, Or Gerlitz, netdev, linux-rdma, kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 364 bytes --]
On Wed, Jul 13, 2016 at 01:08:25PM +0300, Dan Carpenter wrote:
> We accidentally return success when we had intended to return an error
> code.
>
> Fixes: 69697b6e2086 ('net/mlx5: E-Switch, Add support for the sriov offloads mode')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Thanks Dan,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [patch v2] net/mlx5: missing error code in esw_create_offloads_fdb_table()
2016-07-13 10:08 [patch] net/mlx5: missing error code in esw_create_offloads_fdb_table() Dan Carpenter
2016-07-13 11:19 ` Matan Barak
2016-07-13 11:29 ` Leon Romanovsky
@ 2016-07-13 11:53 ` Matan Barak
2016-07-13 13:04 ` Leon Romanovsky
2016-07-13 17:54 ` Jason Gunthorpe
4 siblings, 0 replies; 10+ messages in thread
From: Matan Barak @ 2016-07-13 11:53 UTC (permalink / raw)
To: Dan Carpenter, Or Gerlitz
Cc: Leon Romanovsky, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA
On 13/07/2016 14:48, Dan Carpenter wrote:
> We accidentally return success when we had intended to return an error
> code.
>
> Fixes: 69697b6e2086 ('net/mlx5: E-Switch, Add support for the sriov offloads mode')
> Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
> v2: return -ENOTSUPP instead --EINVAL
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> index 1842dfb..7d982cf 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> @@ -183,6 +183,7 @@ static int esw_create_offloads_fdb_table(struct mlx5_eswitch *esw, int nvports)
>
> root_ns = mlx5_get_flow_namespace(dev, MLX5_FLOW_NAMESPACE_FDB);
> if (!root_ns) {
> + err = -ENOTSUPP;
> esw_warn(dev, "Failed to get FDB flow namespace\n");
> goto ns_err;
> }
>
Thanks.
Reviewed-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [patch v2] net/mlx5: missing error code in esw_create_offloads_fdb_table()
2016-07-13 10:08 [patch] net/mlx5: missing error code in esw_create_offloads_fdb_table() Dan Carpenter
` (2 preceding siblings ...)
2016-07-13 11:53 ` [patch v2] " Matan Barak
@ 2016-07-13 13:04 ` Leon Romanovsky
[not found] ` <20160713130459.GF10079-2ukJVAZIZ/Y@public.gmane.org>
2016-07-13 17:54 ` Jason Gunthorpe
4 siblings, 1 reply; 10+ messages in thread
From: Leon Romanovsky @ 2016-07-13 13:04 UTC (permalink / raw)
To: Dan Carpenter
Cc: Matan Barak, Or Gerlitz, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 460 bytes --]
On Wed, Jul 13, 2016 at 02:48:44PM +0300, Dan Carpenter wrote:
> We accidentally return success when we had intended to return an error
> code.
>
> Fixes: 69697b6e2086 ('net/mlx5: E-Switch, Add support for the sriov offloads mode')
> Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
> v2: return -ENOTSUPP instead --EINVAL
I'm a little bit confused. Why did you prefer ENOTSUPP over EOPNOTSUPP?
Thanks.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [patch v2] net/mlx5: missing error code in esw_create_offloads_fdb_table()
2016-07-13 10:08 [patch] net/mlx5: missing error code in esw_create_offloads_fdb_table() Dan Carpenter
` (3 preceding siblings ...)
2016-07-13 13:04 ` Leon Romanovsky
@ 2016-07-13 17:54 ` Jason Gunthorpe
2016-07-14 5:56 ` Leon Romanovsky
4 siblings, 1 reply; 10+ messages in thread
From: Jason Gunthorpe @ 2016-07-13 17:54 UTC (permalink / raw)
To: Dan Carpenter
Cc: Matan Barak, Or Gerlitz, Leon Romanovsky, netdev, linux-rdma,
kernel-janitors
On Wed, Jul 13, 2016 at 02:48:44PM +0300, Dan Carpenter wrote:
> We accidentally return success when we had intended to return an error
> code.
>
> Fixes: 69697b6e2086 ('net/mlx5: E-Switch, Add support for the sriov offloads mode')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> v2: return -ENOTSUPP instead --EINVAL
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> index 1842dfb..7d982cf 100644
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> @@ -183,6 +183,7 @@ static int esw_create_offloads_fdb_table(struct mlx5_eswitch *esw, int nvports)
>
> root_ns = mlx5_get_flow_namespace(dev, MLX5_FLOW_NAMESPACE_FDB);
> if (!root_ns) {
> + err = -ENOTSUPP;
Did you mean ENOTSUP?
I thought ENOTSUPP was not to be used outside NFS, and isn't properly
exported to userspace..
$ find /usr/include -name "*errno*" | xargs grep 524
Jason
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [patch v2] net/mlx5: missing error code in esw_create_offloads_fdb_table()
2016-07-13 17:54 ` Jason Gunthorpe
@ 2016-07-14 5:56 ` Leon Romanovsky
0 siblings, 0 replies; 10+ messages in thread
From: Leon Romanovsky @ 2016-07-14 5:56 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Dan Carpenter, Matan Barak, Or Gerlitz, netdev, linux-rdma,
kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1460 bytes --]
On Wed, Jul 13, 2016 at 11:54:54AM -0600, Jason Gunthorpe wrote:
> On Wed, Jul 13, 2016 at 02:48:44PM +0300, Dan Carpenter wrote:
> > We accidentally return success when we had intended to return an error
> > code.
> >
> > Fixes: 69697b6e2086 ('net/mlx5: E-Switch, Add support for the sriov offloads mode')
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > v2: return -ENOTSUPP instead --EINVAL
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> > index 1842dfb..7d982cf 100644
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> > @@ -183,6 +183,7 @@ static int esw_create_offloads_fdb_table(struct mlx5_eswitch *esw, int nvports)
> >
> > root_ns = mlx5_get_flow_namespace(dev, MLX5_FLOW_NAMESPACE_FDB);
> > if (!root_ns) {
> > + err = -ENOTSUPP;
>
> Did you mean ENOTSUP?
>
> I thought ENOTSUPP was not to be used outside NFS, and isn't properly
> exported to userspace..
>
> $ find /usr/include -name "*errno*" | xargs grep 524
I asked similar question [1] with different return value in reply
to this patch.
[1] http://marc.info/?l=linux-netdev&m=146843171508230&w=2
>
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread