Re: CFP: 4th RDMA Mini-Summit at LPC 2019

[Yuval Shaia]

On Wed, May 15, 2019 at 07:36:26PM +0300, Leon Romanovsky wrote:
> On Wed, May 15, 2019 at 06:30:51PM +0300, Yuval Shaia wrote:
> > On Tue, May 14, 2019 at 03:23:21PM +0300, Leon Romanovsky wrote:
> > > This is a call for proposals for the 4th RDMA mini-summit at the Linux
> > > Plumbers Conference in Lisbon, Portugal, which will be happening on
> > > September 9-11h, 2019.
> > >
> > > We are looking for topics with focus on active audience discussions
> > > and problem solving. The preferable topic is up to 30 minutes with
> > > 3-5 slides maximum.
> >
> > Abstract: Expand the virtio portfolio with RDMA
> >
> > Description:
> > Data center backends use more and more RDMA or RoCE devices and more and
> > more software runs in virtualized environment.
> > There is a need for a standard to enable RDMA/RoCE on Virtual Machines.
> > Virtio is the optimal solution since is the de-facto para-virtualizaton
> > technology and also because the Virtio specification allows Hardware
> > Vendors to support Virtio protocol natively in order to achieve bare metal
> > performance.
> > This talk addresses challenges in defining the RDMA/RoCE Virtio
> > Specification and a look forward on possible implementation techniques.
> 
> Yuval,
> 
> Who is going to implement it?
> 
> Thanks

It is going to be an open source effort by an open source contributors.
Probably as with qemu-pvrdma it would be me and Marcel and i have an
unofficial approval from extra person that gave promise to join (can't say
his name but since he is also on this list then he welcome to raise a
hand).
I also recall once someone from Mellanox wanted to join but not sure about
his availability now.

> 
> >
> > >
> > > This year, the LPC will include netdev track too and it is
> > > collocated with Kernel Summit, such timing makes an excellent
> > > opportunity to drive cross-tree solutions.
> > >
> > > BTW, RDMA is not accepted yet as a track in LPC, but let's think
> > > positive and start collect topics.
> > >
> > > Thanks

Re: CFP: 4th RDMA Mini-Summit at LPC 2019

[Yuval Shaia]

On Tue, May 14, 2019 at 03:23:21PM +0300, Leon Romanovsky wrote:
> This is a call for proposals for the 4th RDMA mini-summit at the Linux
> Plumbers Conference in Lisbon, Portugal, which will be happening on
> September 9-11h, 2019.
> 
> We are looking for topics with focus on active audience discussions
> and problem solving. The preferable topic is up to 30 minutes with
> 3-5 slides maximum.

Abstract: Expand the virtio portfolio with RDMA 

Description:
Data center backends use more and more RDMA or RoCE devices and more and
more software runs in virtualized environment.
There is a need for a standard to enable RDMA/RoCE on Virtual Machines.
Virtio is the optimal solution since is the de-facto para-virtualizaton
technology and also because the Virtio specification allows Hardware
Vendors to support Virtio protocol natively in order to achieve bare metal
performance.
This talk addresses challenges in defining the RDMA/RoCE Virtio
Specification and a look forward on possible implementation techniques.

> 
> This year, the LPC will include netdev track too and it is
> collocated with Kernel Summit, such timing makes an excellent
> opportunity to drive cross-tree solutions.
> 
> BTW, RDMA is not accepted yet as a track in LPC, but let's think
> positive and start collect topics.
> 
> Thanks

[PATCH] net: Remove inclusion of pci.h

[Yuval Shaia]

This header is not in use - remove it.

Signed-off-by: Yuval Shaia 
---
 net/core/dev.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 2b67f2aa59dd..21a8205a7896 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -131,7 +131,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
-- 
2.19.1

[PATCH v3] net/mlx5e: Delete unneeded function argument

[Yuval Shaia]

priv argument is not used by the function, delete it.

Fixes: a89842811ea98 ("net/mlx5e: Merge per priority stats groups")
Signed-off-by: Yuval Shaia 
---
v1 -> v2:
* Remove blank line as pointed by Leon.

v2 -> v3:
* Change prefix to mlx5e
---
 drivers/net/ethernet/mellanox/mlx5/core/en_stats.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c 
b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
index 1646859974ce..4c4d779dafa8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
@@ -813,7 +813,7 @@ static const struct counter_desc 
pport_per_prio_traffic_stats_desc[] = {
 
 #define NUM_PPORT_PER_PRIO_TRAFFIC_COUNTERS
ARRAY_SIZE(pport_per_prio_traffic_stats_desc)
 
-static int mlx5e_grp_per_prio_traffic_get_num_stats(struct mlx5e_priv *priv)
+static int mlx5e_grp_per_prio_traffic_get_num_stats(void)
 {
return NUM_PPORT_PER_PRIO_TRAFFIC_COUNTERS * NUM_PPORT_PRIO;
 }
@@ -971,7 +971,7 @@ static int mlx5e_grp_per_prio_pfc_fill_stats(struct 
mlx5e_priv *priv,
 
 static int mlx5e_grp_per_prio_get_num_stats(struct mlx5e_priv *priv)
 {
-   return mlx5e_grp_per_prio_traffic_get_num_stats(priv) +
+   return mlx5e_grp_per_prio_traffic_get_num_stats() +
mlx5e_grp_per_prio_pfc_get_num_stats(priv);
 }
 
-- 
2.17.1

Re: [PATCH v2] net/mlx5: Delete unneeded function argument

[Yuval Shaia]

On Thu, Aug 16, 2018 at 09:41:34AM +0300, Gal Pressman wrote:
> On 15-Aug-18 18:08, Yuval Shaia wrote:
> > priv argument is not used by the function, delete it.
> > 
> > Fixes: a89842811ea98 ("net/mlx5e: Merge per priority stats groups")
> > Signed-off-by: Yuval Shaia 
> 
> nit: prefix should be net/mlx5e.

1. Will do.
2. Why? (asking since dir name is mlx5)

> 
> > ---
> > v1 -> v2:
> > * Remove blank line as pointed by Leon.
> > ---
> >  drivers/net/ethernet/mellanox/mlx5/core/en_stats.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c 
> > b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> > index 1646859974ce..4c4d779dafa8 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> > @@ -813,7 +813,7 @@ static const struct counter_desc 
> > pport_per_prio_traffic_stats_desc[] = {
> >  
> >  #define NUM_PPORT_PER_PRIO_TRAFFIC_COUNTERS
> > ARRAY_SIZE(pport_per_prio_traffic_stats_desc)
> >  
> > -static int mlx5e_grp_per_prio_traffic_get_num_stats(struct mlx5e_priv 
> > *priv)
> > +static int mlx5e_grp_per_prio_traffic_get_num_stats(void)
> >  {
> > return NUM_PPORT_PER_PRIO_TRAFFIC_COUNTERS * NUM_PPORT_PRIO;
> >  }
> > @@ -971,7 +971,7 @@ static int mlx5e_grp_per_prio_pfc_fill_stats(struct 
> > mlx5e_priv *priv,
> >  
> >  static int mlx5e_grp_per_prio_get_num_stats(struct mlx5e_priv *priv)
> >  {
> > -   return mlx5e_grp_per_prio_traffic_get_num_stats(priv) +
> > +   return mlx5e_grp_per_prio_traffic_get_num_stats() +
> > mlx5e_grp_per_prio_pfc_get_num_stats(priv);
> >  }
> >  
> > 
> 

[PATCH v2] net/mlx5: Delete unneeded function argument

[Yuval Shaia]

priv argument is not used by the function, delete it.

Fixes: a89842811ea98 ("net/mlx5e: Merge per priority stats groups")
Signed-off-by: Yuval Shaia 
---
v1 -> v2:
* Remove blank line as pointed by Leon.
---
 drivers/net/ethernet/mellanox/mlx5/core/en_stats.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c 
b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
index 1646859974ce..4c4d779dafa8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
@@ -813,7 +813,7 @@ static const struct counter_desc 
pport_per_prio_traffic_stats_desc[] = {
 
 #define NUM_PPORT_PER_PRIO_TRAFFIC_COUNTERS
ARRAY_SIZE(pport_per_prio_traffic_stats_desc)
 
-static int mlx5e_grp_per_prio_traffic_get_num_stats(struct mlx5e_priv *priv)
+static int mlx5e_grp_per_prio_traffic_get_num_stats(void)
 {
return NUM_PPORT_PER_PRIO_TRAFFIC_COUNTERS * NUM_PPORT_PRIO;
 }
@@ -971,7 +971,7 @@ static int mlx5e_grp_per_prio_pfc_fill_stats(struct 
mlx5e_priv *priv,
 
 static int mlx5e_grp_per_prio_get_num_stats(struct mlx5e_priv *priv)
 {
-   return mlx5e_grp_per_prio_traffic_get_num_stats(priv) +
+   return mlx5e_grp_per_prio_traffic_get_num_stats() +
mlx5e_grp_per_prio_pfc_get_num_stats(priv);
 }
 
-- 
2.17.1

[PATCH] net/mlx5: Delete unneeded function argument

[Yuval Shaia]

priv argument is not used by the function, delete it.

Fixes: a89842811ea98 ("net/mlx5e: Merge per priority stats groups")

Signed-off-by: Yuval Shaia 
---
 drivers/net/ethernet/mellanox/mlx5/core/en_stats.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c 
b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
index 1646859974ce..4c4d779dafa8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
@@ -813,7 +813,7 @@ static const struct counter_desc 
pport_per_prio_traffic_stats_desc[] = {
 
 #define NUM_PPORT_PER_PRIO_TRAFFIC_COUNTERS
ARRAY_SIZE(pport_per_prio_traffic_stats_desc)
 
-static int mlx5e_grp_per_prio_traffic_get_num_stats(struct mlx5e_priv *priv)
+static int mlx5e_grp_per_prio_traffic_get_num_stats(void)
 {
return NUM_PPORT_PER_PRIO_TRAFFIC_COUNTERS * NUM_PPORT_PRIO;
 }
@@ -971,7 +971,7 @@ static int mlx5e_grp_per_prio_pfc_fill_stats(struct 
mlx5e_priv *priv,
 
 static int mlx5e_grp_per_prio_get_num_stats(struct mlx5e_priv *priv)
 {
-   return mlx5e_grp_per_prio_traffic_get_num_stats(priv) +
+   return mlx5e_grp_per_prio_traffic_get_num_stats() +
mlx5e_grp_per_prio_pfc_get_num_stats(priv);
 }
 
-- 
2.17.1

Re: [PATCH net 2/2] net/mlx5e: Cleanup of dcbnl related fields

[Yuval Shaia]

On Wed, Aug 08, 2018 at 03:48:08PM -0700, Saeed Mahameed wrote:
> From: Huy Nguyen 
> 
> Remove unused netdev_registered_init/remove in en.h
> Return ENOSUPPORT if the check MLX5_DSCP_SUPPORTED fails.

s/ENOSUPPORT/EOPNOTSUPP
(noted by Haakon)

> Remove extra white space
> 
> Fixes: 2a5e7a1344f4 ("net/mlx5e: Add dcbnl dscp to priority support")
> Signed-off-by: Huy Nguyen 
> Cc: Yuval Shaia 
> Reviewed-by: Parav Pandit 
> Signed-off-by: Saeed Mahameed 
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en.h  |  2 --
>  .../ethernet/mellanox/mlx5/core/en_dcbnl.c| 30 +++
>  2 files changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h 
> b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> index eb9eb7aa953a..405236cf0b04 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -858,8 +858,6 @@ struct mlx5e_profile {
>   mlx5e_fp_handle_rx_cqe handle_rx_cqe;
>   mlx5e_fp_handle_rx_cqe handle_rx_cqe_mpwqe;
>   } rx_handlers;
> - void(*netdev_registered_init)(struct mlx5e_priv *priv);
> - void(*netdev_registered_remove)(struct mlx5e_priv *priv);
>   int max_tc;
>  };
>  
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c 
> b/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
> index e33afa8d2417..722998d68564 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
> @@ -443,16 +443,12 @@ static int mlx5e_dcbnl_ieee_setapp(struct net_device 
> *dev, struct dcb_app *app)
>   bool is_new;
>   int err;
>  
> - if (app->selector != IEEE_8021QAZ_APP_SEL_DSCP)
> - return -EINVAL;
> -
> - if (!MLX5_CAP_GEN(priv->mdev, vport_group_manager))
> - return -EINVAL;
> -
> - if (!MLX5_DSCP_SUPPORTED(priv->mdev))
> - return -EINVAL;
> + if (!MLX5_CAP_GEN(priv->mdev, vport_group_manager) ||
> + !MLX5_DSCP_SUPPORTED(priv->mdev))
> + return -EOPNOTSUPP;
>  
> - if (app->protocol >= MLX5E_MAX_DSCP)
> + if ((app->selector != IEEE_8021QAZ_APP_SEL_DSCP) ||
> + (app->protocol >= MLX5E_MAX_DSCP))
>   return -EINVAL;
>  
>   /* Save the old entry info */
> @@ -500,16 +496,12 @@ static int mlx5e_dcbnl_ieee_delapp(struct net_device 
> *dev, struct dcb_app *app)
>   struct mlx5e_priv *priv = netdev_priv(dev);
>   int err;
>  
> - if (app->selector != IEEE_8021QAZ_APP_SEL_DSCP)
> - return -EINVAL;
> -
> - if (!MLX5_CAP_GEN(priv->mdev, vport_group_manager))
> - return -EINVAL;
> -
> - if (!MLX5_DSCP_SUPPORTED(priv->mdev))
> - return -EINVAL;
> + if  (!MLX5_CAP_GEN(priv->mdev, vport_group_manager) ||
> +  !MLX5_DSCP_SUPPORTED(priv->mdev))
> + return -EOPNOTSUPP;
>  
> - if (app->protocol >= MLX5E_MAX_DSCP)
> + if ((app->selector != IEEE_8021QAZ_APP_SEL_DSCP) ||
> + (app->protocol >= MLX5E_MAX_DSCP))
>   return -EINVAL;
>  
>   /* Skip if no dscp app entry */
> @@ -1146,7 +1138,7 @@ static int mlx5e_set_trust_state(struct mlx5e_priv 
> *priv, u8 trust_state)
>  {
>   int err;
>  
> - err =  mlx5_set_trust_state(priv->mdev, trust_state);
> + err = mlx5_set_trust_state(priv->mdev, trust_state);
>   if (err)
>   return err;
>   priv->dcbx_dp.trust_state = trust_state;
> -- 
> 2.17.0
> 

Re: [PATCH 2/3] IB/ipoib: Stop using dev_id to expose port numbers

[Yuval Shaia]

On Mon, Aug 13, 2018 at 02:42:23PM +0300, Arseny Maslennikov wrote:
> Some InfiniBand network devices have multiple ports on the same PCI
> function. Prior to this the kernel erroneously used the `dev_id' sysfs
> field of those network interfaces to convey the port number to userspace.
> 
> `dev_id' is currently reserved for distinguishing stacked ifaces
> (e.g: VLANs) with the same hardware address as their parent device.
> 
> Similar fixes to net/mlx4_en and many other drivers, which started
> exporting this information through `dev_id' before 3.15, were accepted
> into the kernel 4 years ago.
> See 76a066f2a2a0268b565459c417b59724b5a3197b, commit message:
> `net/mlx4_en: Expose port number through sysfs'.
> 
> I would be OK with this commit not being backported to stable, since
> it might break admin-supplied udev rules and the likes.
> 
> Signed-off-by: Arseny Maslennikov 
> ---
>  drivers/infiniband/ulp/ipoib/ipoib_main.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
> b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> index 6eb0594fffec..f64535038147 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -2252,7 +2252,6 @@ static struct net_device *ipoib_add_port(const char 
> *format,
>   }
>  
>   SET_NETDEV_DEV(priv->dev, hca->dev.parent);
> - priv->dev->dev_id = port - 1;

Correct me if i'm wrong here but besides some changes in commit message
looks like patch 1/3 is the same as 2/3, isn't it?

Yuval

>   priv->dev->dev_port = port - 1;
>  
>   result = ib_query_port(hca, port, &attr);
> -- 
> 2.18.0
> 

Re: [PATCH net 2/2] net/mlx5e: Cleanup of dcbnl related fields

[Yuval Shaia]

On Wed, Aug 08, 2018 at 03:48:08PM -0700, Saeed Mahameed wrote:
> From: Huy Nguyen 
> 
> Remove unused netdev_registered_init/remove in en.h
> Return ENOSUPPORT if the check MLX5_DSCP_SUPPORTED fails.
> Remove extra white space
> 
> Fixes: 2a5e7a1344f4 ("net/mlx5e: Add dcbnl dscp to priority support")
> Signed-off-by: Huy Nguyen 
> Cc: Yuval Shaia 
> Reviewed-by: Parav Pandit 
> Signed-off-by: Saeed Mahameed 

FWIW:

Reviewed-by: Yuval Shaia 
Tested-by: Yuval Shaia 

> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en.h  |  2 --
>  .../ethernet/mellanox/mlx5/core/en_dcbnl.c| 30 +++
>  2 files changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h 
> b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> index eb9eb7aa953a..405236cf0b04 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -858,8 +858,6 @@ struct mlx5e_profile {
>   mlx5e_fp_handle_rx_cqe handle_rx_cqe;
>   mlx5e_fp_handle_rx_cqe handle_rx_cqe_mpwqe;
>   } rx_handlers;
> - void(*netdev_registered_init)(struct mlx5e_priv *priv);
> - void(*netdev_registered_remove)(struct mlx5e_priv *priv);
>   int max_tc;
>  };
>  
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c 
> b/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
> index e33afa8d2417..722998d68564 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
> @@ -443,16 +443,12 @@ static int mlx5e_dcbnl_ieee_setapp(struct net_device 
> *dev, struct dcb_app *app)
>   bool is_new;
>   int err;
>  
> - if (app->selector != IEEE_8021QAZ_APP_SEL_DSCP)
> - return -EINVAL;
> -
> - if (!MLX5_CAP_GEN(priv->mdev, vport_group_manager))
> - return -EINVAL;
> -
> - if (!MLX5_DSCP_SUPPORTED(priv->mdev))
> - return -EINVAL;
> + if (!MLX5_CAP_GEN(priv->mdev, vport_group_manager) ||
> + !MLX5_DSCP_SUPPORTED(priv->mdev))
> + return -EOPNOTSUPP;
>  
> - if (app->protocol >= MLX5E_MAX_DSCP)
> + if ((app->selector != IEEE_8021QAZ_APP_SEL_DSCP) ||
> + (app->protocol >= MLX5E_MAX_DSCP))
>   return -EINVAL;
>  
>   /* Save the old entry info */
> @@ -500,16 +496,12 @@ static int mlx5e_dcbnl_ieee_delapp(struct net_device 
> *dev, struct dcb_app *app)
>   struct mlx5e_priv *priv = netdev_priv(dev);
>   int err;
>  
> - if (app->selector != IEEE_8021QAZ_APP_SEL_DSCP)
> - return -EINVAL;
> -
> - if (!MLX5_CAP_GEN(priv->mdev, vport_group_manager))
> - return -EINVAL;
> -
> - if (!MLX5_DSCP_SUPPORTED(priv->mdev))
> - return -EINVAL;
> + if  (!MLX5_CAP_GEN(priv->mdev, vport_group_manager) ||
> +  !MLX5_DSCP_SUPPORTED(priv->mdev))
> + return -EOPNOTSUPP;
>  
> - if (app->protocol >= MLX5E_MAX_DSCP)
> + if ((app->selector != IEEE_8021QAZ_APP_SEL_DSCP) ||
> + (app->protocol >= MLX5E_MAX_DSCP))
>   return -EINVAL;
>  
>   /* Skip if no dscp app entry */
> @@ -1146,7 +1138,7 @@ static int mlx5e_set_trust_state(struct mlx5e_priv 
> *priv, u8 trust_state)
>  {
>   int err;
>  
> - err =  mlx5_set_trust_state(priv->mdev, trust_state);
> + err = mlx5_set_trust_state(priv->mdev, trust_state);
>   if (err)
>   return err;
>   priv->dcbx_dp.trust_state = trust_state;
> -- 
> 2.17.0
> 

Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'

[Yuval Shaia]

On Thu, May 10, 2018 at 09:02:26AM +0200, Christophe JAILLET wrote:
> If an error occurs, 'mlx4_en_destroy_netdev()' is called.
> It then calls 'mlx4_en_free_resources()' which does the needed resources
> cleanup.
> 
> So, doing some explicit kfree in the error handling path would lead to
> some double kfree.

Patch make sense but what's bothering me is that mlx4_en_free_resources
loops on the entire array, assuming !priv->tx_ring[t] means entry is
allocated but the existing code does not assume that, see [1]. So i looked
to see where tx_ring array is zeroed and didn't find it.

Am i missing something here.

> 
> Simplify code to avoid such a case.
> 
> Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
> Signed-off-by: Christophe JAILLET 
> ---
>  drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 8 +---
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c 
> b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index e0adac4a9a19..9670b33fc9b1 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -3324,12 +3324,11 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int 
> port,
>  MAX_TX_RINGS, GFP_KERNEL);
>   if (!priv->tx_ring[t]) {
>   err = -ENOMEM;
> - goto err_free_tx;
> + goto out;
>   }
>   priv->tx_cq[t] = kzalloc(sizeof(struct mlx4_en_cq *) *
>MAX_TX_RINGS, GFP_KERNEL);
>   if (!priv->tx_cq[t]) {
> - kfree(priv->tx_ring[t]);
>   err = -ENOMEM;
>   goto out;
>   }
> @@ -3582,11 +3581,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int 
> port,
>  
>   return 0;
>  
> -err_free_tx:
> - while (t--) {

[1]

> - kfree(priv->tx_ring[t]);
> - kfree(priv->tx_cq[t]);
> - }
>  out:
>   mlx4_en_destroy_netdev(dev);
>   return err;
> -- 
> 2.17.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: ks8851: fix ks_start_xmit()'s return type

[Yuval Shaia]

On Tue, Apr 24, 2018 at 03:17:02PM +0200, Luc Van Oostenryck wrote:
> The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
> which is a typedef for an enum type, but the implementation in this
> driver returns an 'int'.
> 
> Fix this by returning 'netdev_tx_t' in this driver too.
> 
> Signed-off-by: Luc Van Oostenryck 

Reviewed-by: Yuval Shaia 

> ---
>  drivers/net/ethernet/micrel/ks8851_mll.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/micrel/ks8851_mll.c 
> b/drivers/net/ethernet/micrel/ks8851_mll.c
> index f3e9dd47b..a732fdeb5 100644
> --- a/drivers/net/ethernet/micrel/ks8851_mll.c
> +++ b/drivers/net/ethernet/micrel/ks8851_mll.c
> @@ -1020,7 +1020,7 @@ static void ks_write_qmu(struct ks_net *ks, u8 *pdata, 
> u16 len)
>   * spin_lock_irqsave is required because tx and rx should be mutual 
> exclusive.
>   * So while tx is in-progress, prevent IRQ interrupt from happenning.
>   */
> -static int ks_start_xmit(struct sk_buff *skb, struct net_device *netdev)
> +static netdev_tx_t ks_start_xmit(struct sk_buff *skb, struct net_device 
> *netdev)
>  {
>   int retv = NETDEV_TX_OK;
>   struct ks_net *ks = netdev_priv(netdev);
> -- 
> 2.17.0
> 

Re: [PATCH] net/mlx5/core/fpga/ipsec: Fix use-after-free

[Yuval Shaia]

On Thu, Mar 22, 2018 at 01:03:42PM -0500, Gustavo A. R. Silva wrote:
> _rule_ is being freed and then dereferenced by accessing rule->ctx
> 
> Fix this by copying the value returned by PTR_ERR(rule->ctx) into a local
> variable for its safe use after freeing _rule_
> 
> Addresses-Coverity-ID: 1466041 ("Read from pointer after free")
> Fixes: 05564d0ae075 ("net/mlx5: Add flow-steering commands for FPGA IPSec 
> implementation")
> Signed-off-by: Gustavo A. R. Silva 

Prefix should not be that long, a short one as this is enough.

net/mlx5: Fix use-after-free

Besides that - lgtm.

Reviewed-by: Yuval Shaia 

> ---
>  drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c 
> b/drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c
> index 4f15685..0f5da49 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c
> @@ -1061,8 +1061,9 @@ static int fpga_ipsec_fs_create_fte(struct 
> mlx5_core_dev *dev,
>  
>   rule->ctx = mlx5_fpga_ipsec_fs_create_sa_ctx(dev, fte, is_egress);
>   if (IS_ERR(rule->ctx)) {
> + int err = PTR_ERR(rule->ctx);
>   kfree(rule);
> - return PTR_ERR(rule->ctx);
> + return err;
>   }
>  
>   rule->fte = fte;
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

[Yuval Shaia]

On Tue, Mar 13, 2018 at 06:13:45PM +0200, Yuval Shaia wrote:
> On Tue, Mar 13, 2018 at 05:07:22PM +0200, Liran Alon wrote:
> > Before this commit, dev_forward_skb() always cleared packet's
> > per-network-namespace info. Even if the packet doesn't cross
> > network namespaces.
> > 
> > The comment above dev_forward_skb() describes that this is done
> > because the receiving device may be in another network namespace.
> > However, this case can easily be tested for and therefore we can
> > scrub packet's per-network-namespace info only when receiving device
> > is indeed in another network namespace.
> > 
> > Therefore, this commit changes dev_forward_skb() to tell
> > skb_scrub_packet() that skb has crossed network-namespace only in case
> > transmitting device (skb->dev) network namespace is different then
> > receiving device (dev) network namespace.
> > 
> > An example of a netdev that use skb_forward_skb() is veth.
> > Thus, before this commit a packet transmitted from one veth peer to
> > another when both veth peers are on same network namespace will lose
> > it's skb->mark. The bug could easily be demonstrated by the following:
> > 
> > ip netns add test
> > ip netns exec test bash
> > ip link add veth-a type veth peer name veth-b
> > ip link set veth-a up
> > ip link set veth-b up
> > ip addr add dev veth-a 12.0.0.1/24
> > tc qdisc add dev veth-a root handle 1 prio
> > tc qdisc add dev veth-b ingress
> > tc filter add dev veth-a parent 1: u32 match u32 0 0 action skbedit mark 
> > 1337
> > tc filter add dev veth-b parent : basic match 'meta(nf_mark eq 1337)' 
> > action simple "skb->mark 1337!"
> > dmesg -C
> > ping 12.0.0.2
> > dmesg
> > 
> > Before this change, the above will print nothing to dmesg.
> > After this change, "skb->mark 1337!" will be printed as necessary.
> 
> Hi Liran,
> 
> > 
> > Signed-off-by: Liran Alon 
> > Reviewed-by: Yuval Shaia 
> > Signed-off-by: Yuval Shaia 
> 
> I did not earned the credits for SOB, only r-b.

Had an offlist conversation with Liran,
Turns out that this SOB is ok.

Yuval

> 
> Yuval
> 
> > ---
> >  include/linux/netdevice.h | 2 +-
> >  net/core/dev.c| 6 +++---
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index 5eef6c8e2741..5908f1e31ee2 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -3371,7 +3371,7 @@ static __always_inline int dev_forward_skb(struct 
> > net_device *dev,
> > return NET_RX_DROP;
> > }
> >  
> > -   skb_scrub_packet(skb, true);
> > +   skb_scrub_packet(skb, !net_eq(dev_net(dev), dev_net(skb->dev)));
> > skb->priority = 0;
> > return 0;
> >  }
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 2cedf520cb28..087787dd0a50 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -1877,9 +1877,9 @@ int __dev_forward_skb(struct net_device *dev, struct 
> > sk_buff *skb)
> >   * start_xmit function of one device into the receive queue
> >   * of another device.
> >   *
> > - * The receiving device may be in another namespace, so
> > - * we have to clear all information in the skb that could
> > - * impact namespace isolation.
> > + * The receiving device may be in another namespace.
> > + * In that case, we have to clear all information in the
> > + * skb that could impact namespace isolation.
> >   */
> >  int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
> >  {
> > -- 
> > 1.9.1
> > 

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

[Yuval Shaia]

On Tue, Mar 13, 2018 at 05:07:22PM +0200, Liran Alon wrote:
> Before this commit, dev_forward_skb() always cleared packet's
> per-network-namespace info. Even if the packet doesn't cross
> network namespaces.
> 
> The comment above dev_forward_skb() describes that this is done
> because the receiving device may be in another network namespace.
> However, this case can easily be tested for and therefore we can
> scrub packet's per-network-namespace info only when receiving device
> is indeed in another network namespace.
> 
> Therefore, this commit changes dev_forward_skb() to tell
> skb_scrub_packet() that skb has crossed network-namespace only in case
> transmitting device (skb->dev) network namespace is different then
> receiving device (dev) network namespace.
> 
> An example of a netdev that use skb_forward_skb() is veth.
> Thus, before this commit a packet transmitted from one veth peer to
> another when both veth peers are on same network namespace will lose
> it's skb->mark. The bug could easily be demonstrated by the following:
> 
> ip netns add test
> ip netns exec test bash
> ip link add veth-a type veth peer name veth-b
> ip link set veth-a up
> ip link set veth-b up
> ip addr add dev veth-a 12.0.0.1/24
> tc qdisc add dev veth-a root handle 1 prio
> tc qdisc add dev veth-b ingress
> tc filter add dev veth-a parent 1: u32 match u32 0 0 action skbedit mark 1337
> tc filter add dev veth-b parent : basic match 'meta(nf_mark eq 1337)' 
> action simple "skb->mark 1337!"
> dmesg -C
> ping 12.0.0.2
> dmesg
> 
> Before this change, the above will print nothing to dmesg.
> After this change, "skb->mark 1337!" will be printed as necessary.

Hi Liran,

> 
> Signed-off-by: Liran Alon 
> Reviewed-by: Yuval Shaia 
> Signed-off-by: Yuval Shaia 

I did not earned the credits for SOB, only r-b.

Yuval

> ---
>  include/linux/netdevice.h | 2 +-
>  net/core/dev.c| 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 5eef6c8e2741..5908f1e31ee2 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3371,7 +3371,7 @@ static __always_inline int dev_forward_skb(struct 
> net_device *dev,
>   return NET_RX_DROP;
>   }
>  
> - skb_scrub_packet(skb, true);
> + skb_scrub_packet(skb, !net_eq(dev_net(dev), dev_net(skb->dev)));
>   skb->priority = 0;
>   return 0;
>  }
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 2cedf520cb28..087787dd0a50 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1877,9 +1877,9 @@ int __dev_forward_skb(struct net_device *dev, struct 
> sk_buff *skb)
>   * start_xmit function of one device into the receive queue
>   * of another device.
>   *
> - * The receiving device may be in another namespace, so
> - * we have to clear all information in the skb that could
> - * impact namespace isolation.
> + * The receiving device may be in another namespace.
> + * In that case, we have to clear all information in the
> + * skb that could impact namespace isolation.
>   */
>  int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
>  {
> -- 
> 1.9.1
> 

Re: [PATCH net] RDS: Check cmsg_len before dereferencing CMSG_DATA

[Yuval Shaia]

On Thu, Dec 21, 2017 at 08:17:04PM -0800, Avinash Repaka wrote:
> RDS currently doesn't check if the length of the control message is
> large enough to hold the required data, before dereferencing the control
> message data. This results in following crash:
> 
> BUG: KASAN: stack-out-of-bounds in rds_rdma_bytes net/rds/send.c:1013
> [inline]
> BUG: KASAN: stack-out-of-bounds in rds_sendmsg+0x1f02/0x1f90
> net/rds/send.c:1066
> Read of size 8 at addr 8801c928fb70 by task syzkaller455006/3157
> 
> CPU: 0 PID: 3157 Comm: syzkaller455006 Not tainted 4.15.0-rc3+ #161
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:17 [inline]
>  dump_stack+0x194/0x257 lib/dump_stack.c:53
>  print_address_description+0x73/0x250 mm/kasan/report.c:252
>  kasan_report_error mm/kasan/report.c:351 [inline]
>  kasan_report+0x25b/0x340 mm/kasan/report.c:409
>  __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:430
>  rds_rdma_bytes net/rds/send.c:1013 [inline]
>  rds_sendmsg+0x1f02/0x1f90 net/rds/send.c:1066
>  sock_sendmsg_nosec net/socket.c:628 [inline]
>  sock_sendmsg+0xca/0x110 net/socket.c:638
>  ___sys_sendmsg+0x320/0x8b0 net/socket.c:2018
>  __sys_sendmmsg+0x1ee/0x620 net/socket.c:2108
>  SYSC_sendmmsg net/socket.c:2139 [inline]
>  SyS_sendmmsg+0x35/0x60 net/socket.c:2134
>  entry_SYSCALL_64_fastpath+0x1f/0x96
> RIP: 0033:0x43fe49
> RSP: 002b:7fffbe244ad8 EFLAGS: 0217 ORIG_RAX: 0133
> RAX: ffda RBX: 004002c8 RCX: 0043fe49
> RDX: 0001 RSI: 2020c000 RDI: 0003
> RBP: 006ca018 R08:  R09: 
> R10:  R11: 0217 R12: 004017b0
> R13: 00401840 R14:  R15: 
> 
> To fix this, we verify that the cmsg_len is large enough to hold the
> data to be read, before proceeding further.
> 
> Reported-by: syzbot 
> Signed-off-by: Avinash Repaka 
> ---
>  net/rds/send.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/rds/send.c b/net/rds/send.c
> index b52cdc8..f72466c 100644
> --- a/net/rds/send.c
> +++ b/net/rds/send.c
> @@ -1009,6 +1009,9 @@ static int rds_rdma_bytes(struct msghdr *msg, size_t 
> *rdma_bytes)
>   continue;
>  
>   if (cmsg->cmsg_type == RDS_CMSG_RDMA_ARGS) {
> + if (cmsg->cmsg_len <
> + CMSG_LEN(sizeof(struct rds_rdma_args)))
> +     return -EINVAL;
>   args = CMSG_DATA(cmsg);
>   *rdma_bytes += args->remote_vec.bytes;
>   }

Reviewed-by: Yuval Shaia 

> -- 
> 2.4.11
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 net] net: hns3: Updates MSI/MSI-X alloc/free APIs(depricated) to new APIs

[Yuval Shaia]

On Thu, Nov 09, 2017 at 04:38:13PM +, Salil Mehta wrote:
> This patch migrates the HNS3 driver code from use of depricated PCI
> MSI/MSI-X interrupt vector allocation/free APIs to new common APIs.
> 
> Signed-off-by: Salil Mehta 
> Suggested-by: Christoph Hellwig 
> ---
> PATCH V2: Yuval Shaia 
>   Link -> https://lkml.org/lkml/2017/11/9/138
> PATCH V1: Initial Submit
> ---
>  .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c| 107 
> +++--
>  .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h|  15 ++-
>  2 files changed, 42 insertions(+), 80 deletions(-)

Reviewed-by: Yuval Shaia 

> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c 
> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> index c1cdbfd..d65c599 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> @@ -885,14 +885,14 @@ static int hclge_query_pf_resource(struct hclge_dev 
> *hdev)
>   hdev->pkt_buf_size = __le16_to_cpu(req->buf_size) << HCLGE_BUF_UNIT_S;
>  
>   if (hnae3_dev_roce_supported(hdev)) {
> - hdev->num_roce_msix =
> + hdev->num_roce_msi =
>   hnae_get_field(__le16_to_cpu(req->pf_intr_vector_number),
>  HCLGE_PF_VEC_NUM_M, HCLGE_PF_VEC_NUM_S);
>  
>   /* PF should have NIC vectors and Roce vectors,
>* NIC vectors are queued before Roce vectors.
>*/
> - hdev->num_msi = hdev->num_roce_msix  + HCLGE_ROCE_VECTOR_OFFSET;
> + hdev->num_msi = hdev->num_roce_msi  + HCLGE_ROCE_VECTOR_OFFSET;
>   } else {
>   hdev->num_msi =
>   hnae_get_field(__le16_to_cpu(req->pf_intr_vector_number),
> @@ -1835,7 +1835,7 @@ static int hclge_init_roce_base_info(struct hclge_vport 
> *vport)
>   struct hnae3_handle *roce = &vport->roce;
>   struct hnae3_handle *nic = &vport->nic;
>  
> - roce->rinfo.num_vectors = vport->back->num_roce_msix;
> + roce->rinfo.num_vectors = vport->back->num_roce_msi;
>  
>   if (vport->back->num_msi_left < vport->roce.rinfo.num_vectors ||
>   vport->back->num_msi_left == 0)
> @@ -1853,67 +1853,47 @@ static int hclge_init_roce_base_info(struct 
> hclge_vport *vport)
>   return 0;
>  }
>  
> -static int hclge_init_msix(struct hclge_dev *hdev)
> +static int hclge_init_msi(struct hclge_dev *hdev)
>  {
>   struct pci_dev *pdev = hdev->pdev;
> - int ret, i;
> -
> - hdev->msix_entries = devm_kcalloc(&pdev->dev, hdev->num_msi,
> -   sizeof(struct msix_entry),
> -   GFP_KERNEL);
> - if (!hdev->msix_entries)
> - return -ENOMEM;
> -
> - hdev->vector_status = devm_kcalloc(&pdev->dev, hdev->num_msi,
> -sizeof(u16), GFP_KERNEL);
> - if (!hdev->vector_status)
> - return -ENOMEM;
> + int vectors;
> + int i;
>  
> - for (i = 0; i < hdev->num_msi; i++) {
> - hdev->msix_entries[i].entry = i;
> - hdev->vector_status[i] = HCLGE_INVALID_VPORT;
> + vectors = pci_alloc_irq_vectors(pdev, 1, hdev->num_msi,
> + PCI_IRQ_MSI | PCI_IRQ_MSIX);
> + if (vectors < 0) {
> + dev_err(&pdev->dev,
> + "failed(%d) to allocate MSI/MSI-X vectors\n",
> + vectors);
> + return vectors;
>   }
> + if (vectors < hdev->num_msi)
> + dev_warn(&hdev->pdev->dev,
> +  "requested %d MSI/MSI-X, but allocated %d MSI/MSI-X\n",
> +  hdev->num_msi, vectors);
>  
> - hdev->num_msi_left = hdev->num_msi;
> - hdev->base_msi_vector = hdev->pdev->irq;
> + hdev->num_msi = vectors;
> + hdev->num_msi_left = vectors;
> + hdev->base_msi_vector = pdev->irq;
>   hdev->roce_base_vector = hdev->base_msi_vector +
>   HCLGE_ROCE_VECTOR_OFFSET;
>  
> - ret = pci_enable_msix_range(hdev->pdev, hdev->msix_entries,
> - hdev->num_msi, hdev->num_msi);
> - if (ret < 0) {
> - dev_info(&hdev->pdev->dev,
> -  "MSI-X vector alloc failed: %d\n", ret);
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> -static int 

Re: [PATCH net] net: hns3: Updates MSI/MSI-X alloc/free APIs(depricated) to new APIs

[Yuval Shaia]

On Wed, Nov 08, 2017 at 11:56:06AM +, Salil Mehta wrote:
> This patch migrates the HNS3 driver code from use of depricated PCI
> MSI/MSI-X interrupt vector allocation/free APIs to new common APIs.
> 
> Signed-off-by: Salil Mehta 
> Suggested-by: Christoph Hellwig 
> ---
>  .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c| 108 
> +++--
>  .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h|  15 ++-
>  2 files changed, 42 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c 
> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> index c1cdbfd..09fa068 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> @@ -885,14 +885,14 @@ static int hclge_query_pf_resource(struct hclge_dev 
> *hdev)
>   hdev->pkt_buf_size = __le16_to_cpu(req->buf_size) << HCLGE_BUF_UNIT_S;
>  
>   if (hnae3_dev_roce_supported(hdev)) {
> - hdev->num_roce_msix =
> + hdev->num_roce_msi =
>   hnae_get_field(__le16_to_cpu(req->pf_intr_vector_number),
>  HCLGE_PF_VEC_NUM_M, HCLGE_PF_VEC_NUM_S);
>  
>   /* PF should have NIC vectors and Roce vectors,
>* NIC vectors are queued before Roce vectors.
>*/
> - hdev->num_msi = hdev->num_roce_msix  + HCLGE_ROCE_VECTOR_OFFSET;
> + hdev->num_msi = hdev->num_roce_msi  + HCLGE_ROCE_VECTOR_OFFSET;
>   } else {
>   hdev->num_msi =
>   hnae_get_field(__le16_to_cpu(req->pf_intr_vector_number),
> @@ -1835,7 +1835,7 @@ static int hclge_init_roce_base_info(struct hclge_vport 
> *vport)
>   struct hnae3_handle *roce = &vport->roce;
>   struct hnae3_handle *nic = &vport->nic;
>  
> - roce->rinfo.num_vectors = vport->back->num_roce_msix;
> + roce->rinfo.num_vectors = vport->back->num_roce_msi;
>  
>   if (vport->back->num_msi_left < vport->roce.rinfo.num_vectors ||
>   vport->back->num_msi_left == 0)
> @@ -1853,67 +1853,46 @@ static int hclge_init_roce_base_info(struct 
> hclge_vport *vport)
>   return 0;
>  }
>  
> -static int hclge_init_msix(struct hclge_dev *hdev)
> +static int hclge_init_msi(struct hclge_dev *hdev)
>  {
>   struct pci_dev *pdev = hdev->pdev;
> - int ret, i;
> -
> - hdev->msix_entries = devm_kcalloc(&pdev->dev, hdev->num_msi,
> -   sizeof(struct msix_entry),
> -   GFP_KERNEL);
> - if (!hdev->msix_entries)
> - return -ENOMEM;
> -
> - hdev->vector_status = devm_kcalloc(&pdev->dev, hdev->num_msi,
> -sizeof(u16), GFP_KERNEL);
> - if (!hdev->vector_status)
> - return -ENOMEM;
> + int vectors;
> + int i;
>  
> - for (i = 0; i < hdev->num_msi; i++) {
> - hdev->msix_entries[i].entry = i;
> - hdev->vector_status[i] = HCLGE_INVALID_VPORT;
> + vectors = pci_alloc_irq_vectors(pdev, 1, hdev->num_msi,
> + PCI_IRQ_MSI | PCI_IRQ_MSIX);
> + if (vectors < 0) {
> + dev_err(&pdev->dev, "failed to allocate MSI/MSI-X vectors %d\n",

Suggesting:
dev_err(&pdev->dev, "failed to allocate MSI/MSI-X %d vecrors, 
error=%d",
hdev->num_msi, vectors);

> + vectors);
> + return vectors;
>   }
> + if (vectors < hdev->num_msi)
> + dev_warn(&hdev->pdev->dev,
> +  "could not alloc(=%d) all requested(=%d) MSI/MSI-X\n",

Suggesting something like:
"Requested %d, allocated %d"

> +  hdev->num_msi, vectors);
>  
> - hdev->num_msi_left = hdev->num_msi;
> - hdev->base_msi_vector = hdev->pdev->irq;
> + hdev->num_msi = vectors;
> + hdev->num_msi_left = vectors;
> + hdev->base_msi_vector = pdev->irq;
>   hdev->roce_base_vector = hdev->base_msi_vector +
> - HCLGE_ROCE_VECTOR_OFFSET;
> -
> - ret = pci_enable_msix_range(hdev->pdev, hdev->msix_entries,
> - hdev->num_msi, hdev->num_msi);
> - if (ret < 0) {
> - dev_info(&hdev->pdev->dev,
> -  "MSI-X vector alloc failed: %d\n", ret);
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> -static int hclge_init_msi(struct hclge_dev *hdev)
> -{
> - struct pci_dev *pdev = hdev->pdev;
> - int vectors;
> - int i;
> +  HCLGE_ROCE_VECTOR_OFFSET;

Any reason for this  

>  
>   hdev->vector_status = devm_kcalloc(&pdev->dev, hdev->num_msi,
>  sizeof(u16), GFP_KERNEL);
> - if (!hdev->vector_status)
> + if (!hdev->vector_status) {
> + pci_free_irq_vectors(pdev);
> 

Re: [PATCH] net/mlx4: fix spelling mistake: "availible" -> "available"

[Yuval Shaia]

 u8 port, u8 *tc_tx_bw,
>   u8 *pg, u16 *ratelimit);
>  /**
> - * mlx4_ALLOCATE_VPP_get - Query port VPP availible resources and allocation.
> - * Before distribution of VPPs to priorities, only availible_vpp is returned.
> + * mlx4_ALLOCATE_VPP_get - Query port VPP available resources and allocation.
> + * Before distribution of VPPs to priorities, only available_vpp is returned.
>   * After initialization it returns the distribution of VPPs among priorities.
>   *
>   * @dev: mlx4_dev.
>   * @port: Physical port number.
> - * @availible_vpp: Pointer to variable where number of availible VPPs is 
> stored
> + * @available_vpp: Pointer to variable where number of available VPPs is 
> stored
>   * @vpp_p_up: Distribution of VPPs to priorities is stored in this array
>   *
>   * Returns 0 on success or a negative mlx4_core errno code.
>   **/
>  int mlx4_ALLOCATE_VPP_get(struct mlx4_dev *dev, u8 port,
> -   u16 *availible_vpp, u8 *vpp_p_up);
> +   u16 *available_vpp, u8 *vpp_p_up);
>  /**
>   * mlx4_ALLOCATE_VPP_set - Distribution of VPPs among differnt priorities.
>   * The total number of VPPs assigned to all for a port must not exceed
> - * the value reported by availible_vpp in mlx4_ALLOCATE_VPP_get.
> + * the value reported by available_vpp in mlx4_ALLOCATE_VPP_get.
>   * VPP allocation is allowed only after the port type has been set,
>   * and while no QPs are open for this port.
>   *

Reviewed-by: Yuval Shaia 

(I'm using setlocal spell spelllang=en to avoid such spelling mistakes)

> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] mlx4_en: remove unnecessary returned value check

[Yuval Shaia]

On Wed, Jul 12, 2017 at 02:44:33AM -0400, Zhu Yanjun wrote:
> The function __mlx4_zone_remove_one_entry always returns zero. So
> it is not necessary to check it.
> 
> Cc: Joe Jin 
> Cc: Junxiao Bi 
> Signed-off-by: Zhu Yanjun 
> ---
>  drivers/net/ethernet/mellanox/mlx4/alloc.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/alloc.c 
> b/drivers/net/ethernet/mellanox/mlx4/alloc.c
> index 249a458..bfb185d 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/alloc.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/alloc.c
> @@ -283,7 +283,7 @@ int mlx4_zone_add_one(struct mlx4_zone_allocator 
> *zone_alloc,
>  }
>  
>  /* Should be called under a lock */
> -static int __mlx4_zone_remove_one_entry(struct mlx4_zone_entry *entry)
> +static void __mlx4_zone_remove_one_entry(struct mlx4_zone_entry *entry)
>  {
>   struct mlx4_zone_allocator *zone_alloc = entry->allocator;
>  
> @@ -315,8 +315,6 @@ static int __mlx4_zone_remove_one_entry(struct 
> mlx4_zone_entry *entry)
>   }
>   zone_alloc->mask = mask;
>   }
> -
> - return 0;
>  }
>  
>  void mlx4_zone_allocator_destroy(struct mlx4_zone_allocator *zone_alloc)
> @@ -468,7 +466,8 @@ int mlx4_zone_remove_one(struct mlx4_zone_allocator 
> *zones, u32 uid)
>   goto out;
>   }
>  
> - res = __mlx4_zone_remove_one_entry(zone);
> + __mlx4_zone_remove_one_entry(zone);
> +     res = 0;

Will look better if initialization will move to variable declaration.

Besides this minor thing - lgtm.

Reviewed-by: Yuval Shaia 

>  
>  out:
>   spin_unlock(&zones->lock);
> -- 
> 2.7.4
> 

Re: [PATCH iproute2 V2 1/4] rdma: Add basic infrastructure for RDMA tool

[Yuval Shaia]

On Mon, Jul 03, 2017 at 05:06:55PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky 
> 
> RDMA devices are cross-functional devices from one side,
> but very tailored for the specific markets from another.
> 
> Such diversity caused to spread of RDMA related configuration
> across various tools, e.g. devlink, ip, ethtool, ib specific and
> vendor specific solutions.
> 
> This patch adds ability to fill device and port information
> by reading RDMA netlink.
> 
> Signed-off-by: Leon Romanovsky 
> ---
>  Makefile|   2 +-
>  rdma/.gitignore |   1 +
>  rdma/Makefile   |  22 ++
>  rdma/rdma.c | 116 
>  rdma/rdma.h |  71 +
>  rdma/utils.c| 232 
> 
>  6 files changed, 443 insertions(+), 1 deletion(-)
>  create mode 100644 rdma/.gitignore
>  create mode 100644 rdma/Makefile
>  create mode 100644 rdma/rdma.c
>  create mode 100644 rdma/rdma.h
>  create mode 100644 rdma/utils.c
> 
> diff --git a/Makefile b/Makefile
> index 18de7dcb..c255063b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -52,7 +52,7 @@ WFLAGS += -Wmissing-declarations -Wold-style-definition 
> -Wformat=2
>  CFLAGS := $(WFLAGS) $(CCOPTS) -I../include $(DEFINES) $(CFLAGS)
>  YACCFLAGS = -d -t -v
>  
> -SUBDIRS=lib ip tc bridge misc netem genl tipc devlink man
> +SUBDIRS=lib ip tc bridge misc netem genl tipc devlink rdma man
>  
>  LIBNETLINK=../lib/libnetlink.a ../lib/libutil.a
>  LDLIBS += $(LIBNETLINK)
> diff --git a/rdma/.gitignore b/rdma/.gitignore
> new file mode 100644
> index ..51fb172b
> --- /dev/null
> +++ b/rdma/.gitignore
> @@ -0,0 +1 @@
> +rdma
> diff --git a/rdma/Makefile b/rdma/Makefile
> new file mode 100644
> index ..64da2142
> --- /dev/null
> +++ b/rdma/Makefile
> @@ -0,0 +1,22 @@
> +include ../Config
> +
> +ifeq ($(HAVE_MNL),y)
> +
> +RDMA_OBJ = rdma.o utils.o
> +
> +TARGETS=rdma
> +CFLAGS += $(shell $(PKG_CONFIG) libmnl --cflags)
> +LDLIBS += $(shell $(PKG_CONFIG) libmnl --libs)
> +
> +endif
> +
> +all: $(TARGETS) $(LIBS)
> +
> +rdma:$(RDMA_OBJ) $(LIBS)
> + $(QUIET_LINK)$(CC) $^ $(LDFLAGS) $(LDLIBS) -o $@
> +
> +install: all
> + install -m 0755 $(TARGETS) $(DESTDIR)$(SBINDIR)
> +
> +clean:
> + rm -f $(RDMA_OBJ) $(TARGETS)
> diff --git a/rdma/rdma.c b/rdma/rdma.c
> new file mode 100644
> index ..29273839
> --- /dev/null
> +++ b/rdma/rdma.c
> @@ -0,0 +1,116 @@
> +/*
> + * rdma.cRDMA tool
> + *
> + *  This program is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU General Public License
> + *  as published by the Free Software Foundation; either version
> + *  2 of the License, or (at your option) any later version.
> + *
> + * Authors: Leon Romanovsky 
> + */
> +
> +#include 
> +#include 
> +
> +#include "rdma.h"
> +#include "SNAPSHOT.h"
> +
> +static void help(char *name)
> +{
> + pr_out("Usage: %s [ OPTIONS ] OBJECT { COMMAND | help }\n"
> +"where  OBJECT := { help }\n"
> +"   OPTIONS := { -V[ersion] | -d[etails]}\n", name);
> +}
> +
> +static int cmd_help(struct rdma *rd)
> +{
> + help(rd->filename);
> + return 0;

Can we change it to void?

> +}
> +
> +static int rd_cmd(struct rdma *rd)
> +{
> + const struct rdma_cmd cmds[] = {
> + { NULL, cmd_help },
> + { "help",   cmd_help },
> + { 0 }
> + };
> +
> + return rdma_exec_cmd(rd, cmds, "object");
> +}
> +
> +static int rd_init(struct rdma *rd, int argc, char **argv, char *filename)
> +{
> + uint32_t seq;
> + int ret;
> +
> + rd->filename = filename;
> + rd->argc = argc;
> + rd->argv = argv;
> + INIT_LIST_HEAD(&rd->dev_map_list);
> + rd->buff = malloc(MNL_SOCKET_BUFFER_SIZE);
> + if (!rd->buff)
> + return -ENOMEM;
> +
> + rdma_prepare_msg(rd, RDMA_NLDEV_CMD_GET, &seq, (NLM_F_REQUEST | 
> NLM_F_ACK | NLM_F_DUMP));
> + if ((ret = rdma_send_msg(rd)))
> + return ret;

Maybe it is only my perspective but as i see it - if some init function
fails at one of the steps it needs to rollback to starting point before
returning an error. A caller expect that if init fails then no reason to
call the free.

Caller here calls free when init fails so we are fine but just razing a
point here.

> +
> + return rdma_recv_msg(rd, rd_dev_init_cb, rd, seq);
> +}
> +
> +static void rd_free(struct rdma *rd)
> +{
> + free(rd->buff);
> + rdma_free_devmap(rd);
> +}
> +int main(int argc, char **argv)
> +{
> + static const struct option long_options[] = {
> + { "version",no_argument,NULL, 'V' },
> + { "help",   no_argument,NULL, 'h' },
> + { "details",no_argument,NULL, 'd' },
> + { NULL, 0, NULL, 0 }
> + };
> + bool show_details = false;
> + cha

Re: [PATCH 1/1] mlx4_en: make mlx4_log_num_mgm_entry_size static

[Yuval Shaia]

On Mon, Jul 03, 2017 at 01:35:19AM -0400, Zhu Yanjun wrote:
> The variable mlx4_log_num_mgm_entry_size is only called in main.c.
> 
> CC: Joe Jin 
> CC: Junxiao Bi 
> Signed-off-by: Zhu Yanjun 
> ---
>  drivers/net/ethernet/mellanox/mlx4/main.c | 2 +-
>  drivers/net/ethernet/mellanox/mlx4/mlx4.h | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c 
> b/drivers/net/ethernet/mellanox/mlx4/main.c
> index 83aab1e..4b67277 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/main.c
> @@ -91,7 +91,7 @@ module_param_array(probe_vf, byte, &probe_vfs_argc, 0444);
>  MODULE_PARM_DESC(probe_vf, "number of vfs to probe by pf driver (num_vfs > 
> 0)\n"
>  "probe_vf=port1,port2,port1+2");
>  
> -int mlx4_log_num_mgm_entry_size = MLX4_DEFAULT_MGM_LOG_ENTRY_SIZE;
> +static int mlx4_log_num_mgm_entry_size = MLX4_DEFAULT_MGM_LOG_ENTRY_SIZE;
>  module_param_named(log_num_mgm_entry_size,
>   mlx4_log_num_mgm_entry_size, int, 0444);
>  MODULE_PARM_DESC(log_num_mgm_entry_size, "log mgm size, that defines the num"
> diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h 
> b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
> index b4f1bc5..a3f5edb 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
> +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
> @@ -231,7 +231,6 @@ do {  
> \
>  #define mlx4_warn(mdev, format, ...) \
>   dev_warn(&(mdev)->persist->pdev->dev, format, ##__VA_ARGS__)
>  
> -extern int mlx4_log_num_mgm_entry_size;

Reviewed-by: Yuval Shaia 

>  extern int log_mtts_per_seg;
>  extern int mlx4_internal_err_reset;
>  
> -- 
> 2.7.4
> 

Re: [Patch net] net_sched: move tcf_lock down after gen_replace_estimator()

[Yuval Shaia]

On Tue, Jun 13, 2017 at 01:36:24PM -0700, Cong Wang wrote:
> Laura reported a sleep-in-atomic kernel warning inside

Since you added a Reported-by tag below i don't see a reason to
specifically mention it in commit log message.

> tcf_act_police_init() which calls gen_replace_estimator() with
> spinlock protection.
> 
> It is not necessary in this case, we already have RTNL lock here
> so it is enough to protect concurrent writers. For the reader,
> i.e. tcf_act_police(), it needs to make decision based on this
> rate estimator, in the worst case we drop more/less packets than
> necessary while changing the rate in parallel, it is still acceptable.
> 
> Reported-by: Laura Abbott 
> Reported-by: Nick Huber 
> Cc: Jamal Hadi Salim 
> Signed-off-by: Cong Wang 
> ---
>  net/sched/act_police.c | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/net/sched/act_police.c b/net/sched/act_police.c
> index f42008b..b062bc8 100644
> --- a/net/sched/act_police.c
> +++ b/net/sched/act_police.c
> @@ -132,21 +132,21 @@ static int tcf_act_police_init(struct net *net, struct 
> nlattr *nla,
>   }
>   }
>  
> - spin_lock_bh(&police->tcf_lock);
>   if (est) {
>   err = gen_replace_estimator(&police->tcf_bstats, NULL,
>   &police->tcf_rate_est,
>   &police->tcf_lock,
>   NULL, est);
>   if (err)
> - goto failure_unlock;
> + goto failure;
>   } else if (tb[TCA_POLICE_AVRATE] &&
>  (ret == ACT_P_CREATED ||
>   !gen_estimator_active(&police->tcf_rate_est))) {
>   err = -EINVAL;
> - goto failure_unlock;
> + goto failure;
>   }
>  
> + spin_lock_bh(&police->tcf_lock);
>   /* No failure allowed after this point */
>   police->tcfp_mtu = parm->mtu;
>   if (police->tcfp_mtu == 0) {
> @@ -192,8 +192,6 @@ static int tcf_act_police_init(struct net *net, struct 
> nlattr *nla,
>  
>   return ret;
>  
> -failure_unlock:
> - spin_unlock_bh(&police->tcf_lock);
>  failure:
>   qdisc_put_rtab(P_tab);
>   qdisc_put_rtab(R_tab);
> -- 
> 2.5.5
> 

Re: [net-next] macvlan: propagate the mac address change status for lowerdev

[Yuval Shaia]

On Tue, Jun 13, 2017 at 10:45:11PM +0800, Zhang Shengju wrote:
> The macvlan dev should propagate the return value of mac address change for
> lower device in the passthru mode, instead of always return 0.
> 
> Signed-off-by: Zhang Shengju 
> ---
>  drivers/net/macvlan.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index 346ad2f..ade1213 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -703,10 +703,8 @@ static int macvlan_set_mac_address(struct net_device 
> *dev, void *p)
>   if (!is_valid_ether_addr(addr->sa_data))
>   return -EADDRNOTAVAIL;
>  
> - if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
> - dev_set_mac_address(vlan->lowerdev, addr);
> - return 0;
> - }
> + if (vlan->mode == MACVLAN_MODE_PASSTHRU)
> + return dev_set_mac_address(vlan->lowerdev, addr);

Do you think the following functions needs this fix as well?
- alb_set_mac_address
- bond_alb_handle_active_change
- bond_enslave
- __bond_release_one
- macvlan_set_mac_address

Yuval

>  
>   return macvlan_sync_address(dev, addr->sa_data);
>  }
> -- 
> 1.8.3.1
> 
> 
> 

[PATCH v3] net: phy: Make phy_ethtool_ksettings_get return void

[Yuval Shaia]

Make return value void since function never return meaningfull value

Signed-off-by: Yuval Shaia 
Acked-by: Sergei Shtylyov 
---
v0 ->v1:
* These files were missing in v0
* drivers/net/ethernet/renesas/ravb_main.c
* drivers/net/ethernet/renesas/sh_eth.c
* drivers/net/ethernet/ti/netcp_ethss.c
* Add Acked-by: Sergei Shtylyov

v1 -> v2:
* Adjust to net-next tree

v2 -> v3:
* These files were missing in v1
* drivers/net/ethernet/apm/xgene-v2/ethtool.c
* drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
* drivers/net/ethernet/broadcom/b44.c
* drivers/net/ethernet/broadcom/bcm63xx_enet.c
* drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
* drivers/net/ethernet/mediatek/mtk_eth_soc.c
* drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
* drivers/net/ethernet/ti/cpsw.c
* drivers/staging/netlogic/xlr_net.c
---
 drivers/net/ethernet/apm/xgene-v2/ethtool.c  |  4 +++-
 drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c  |  8 ++--
 drivers/net/ethernet/broadcom/b44.c  |  4 +++-
 drivers/net/ethernet/broadcom/bcm63xx_enet.c |  5 -
 drivers/net/ethernet/broadcom/genet/bcmgenet.c   |  4 +++-
 drivers/net/ethernet/broadcom/tg3.c  |  4 +++-
 drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c   |  6 ++
 drivers/net/ethernet/freescale/ucc_geth_ethtool.c|  4 +++-
 drivers/net/ethernet/hisilicon/hns/hns_ethtool.c |  2 +-
 drivers/net/ethernet/marvell/mv643xx_eth.c   |  5 ++---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c  |  4 +++-
 drivers/net/ethernet/renesas/ravb_main.c | 14 +++---
 drivers/net/ethernet/renesas/sh_eth.c|  5 ++---
 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c |  5 ++---
 drivers/net/ethernet/ti/cpsw.c   |  8 
 drivers/net/ethernet/ti/netcp_ethss.c|  8 +++-
 drivers/net/phy/phy.c| 10 +-
 drivers/net/usb/lan78xx.c|  2 +-
 drivers/staging/netlogic/xlr_net.c   |  5 -
 include/linux/phy.h  |  4 ++--
 net/dsa/slave.c  |  9 +
 21 files changed, 68 insertions(+), 52 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene-v2/ethtool.c 
b/drivers/net/ethernet/apm/xgene-v2/ethtool.c
index be4..d31ad82 100644
--- a/drivers/net/ethernet/apm/xgene-v2/ethtool.c
+++ b/drivers/net/ethernet/apm/xgene-v2/ethtool.c
@@ -157,7 +157,9 @@ static int xge_get_link_ksettings(struct net_device *ndev,
if (!phydev)
return -ENODEV;
 
-   return phy_ethtool_ksettings_get(phydev, cmd);
+   phy_ethtool_ksettings_get(phydev, cmd);
+
+   return 0;
 }
 
 static int xge_set_link_ksettings(struct net_device *ndev,
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c 
b/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
index 559963b..4f50f11 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
@@ -131,13 +131,17 @@ static int xgene_get_link_ksettings(struct net_device 
*ndev,
if (phydev == NULL)
return -ENODEV;
 
-   return phy_ethtool_ksettings_get(phydev, cmd);
+   phy_ethtool_ksettings_get(phydev, cmd);
+
+   return 0;
} else if (pdata->phy_mode == PHY_INTERFACE_MODE_SGMII) {
if (pdata->mdio_driver) {
if (!phydev)
return -ENODEV;
 
-   return phy_ethtool_ksettings_get(phydev, cmd);
+   phy_ethtool_ksettings_get(phydev, cmd);
+
+   return 0;
}
 
supported = SUPPORTED_1000baseT_Full | SUPPORTED_Autoneg |
diff --git a/drivers/net/ethernet/broadcom/b44.c 
b/drivers/net/ethernet/broadcom/b44.c
index 5b95bb4..f411936 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -1836,7 +1836,9 @@ static int b44_get_link_ksettings(struct net_device *dev,
 
if (bp->flags & B44_FLAG_EXTERNAL_PHY) {
BUG_ON(!dev->phydev);
-   return phy_ethtool_ksettings_get(dev->phydev, cmd);
+   phy_ethtool_ksettings_get(dev->phydev, cmd);
+
+   return 0;
}
 
supported = (SUPPORTED_Autoneg);
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c 
b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index 50d88d3..ea3c906 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -1453,7 +1453,10 @@ static int bcm_enet_get_link_ksettings(struct net

[PATCH v2] net: phy: Make phy_ethtool_ksettings_get return void

[Yuval Shaia]

Make return value void since function never return meaningfull value

Signed-off-by: Yuval Shaia 
Acked-by: Sergei Shtylyov 
---
v0 ->v1:
* These files were missing in v0
* drivers/net/ethernet/renesas/ravb_main.c
* drivers/net/ethernet/renesas/sh_eth.c
* drivers/net/ethernet/ti/netcp_ethss.c
* Add Acked-by: Sergei Shtylyov
v1 -> v2:
* Adjust to net-next tree
---
 drivers/net/ethernet/broadcom/b44.c|  3 ++-
 drivers/net/ethernet/broadcom/bcm63xx_enet.c   |  3 ++-
 drivers/net/ethernet/broadcom/genet/bcmgenet.c |  4 +++-
 drivers/net/ethernet/broadcom/tg3.c|  4 +++-
 drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c |  6 ++
 drivers/net/ethernet/freescale/ucc_geth_ethtool.c  |  4 +++-
 drivers/net/ethernet/marvell/mv643xx_eth.c |  5 ++---
 drivers/net/ethernet/renesas/ravb_main.c   | 14 +++---
 drivers/net/ethernet/renesas/sh_eth.c  |  5 ++---
 drivers/net/ethernet/ti/cpsw.c |  9 +
 drivers/net/ethernet/ti/netcp_ethss.c  |  8 +++-
 drivers/net/phy/phy.c  | 10 +-
 drivers/net/usb/lan78xx.c  |  2 +-
 include/linux/phy.h|  4 ++--
 net/dsa/slave.c|  9 +
 15 files changed, 47 insertions(+), 43 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c 
b/drivers/net/ethernet/broadcom/b44.c
index 5b95bb4..9873d2d 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -1836,7 +1836,8 @@ static int b44_get_link_ksettings(struct net_device *dev,
 
if (bp->flags & B44_FLAG_EXTERNAL_PHY) {
BUG_ON(!dev->phydev);
-   return phy_ethtool_ksettings_get(dev->phydev, cmd);
+   phy_ethtool_ksettings_get(dev->phydev, cmd);
+   return 0;
}
 
supported = (SUPPORTED_Autoneg);
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c 
b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index 50d88d3..34ebb40 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -1453,7 +1453,8 @@ static int bcm_enet_get_link_ksettings(struct net_device 
*dev,
if (priv->has_phy) {
if (!dev->phydev)
return -ENODEV;
-   return phy_ethtool_ksettings_get(dev->phydev, cmd);
+   phy_ethtool_ksettings_get(dev->phydev, cmd);
+   return 0;
} else {
cmd->base.autoneg = 0;
cmd->base.speed = (priv->force_speed_100) ?
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index a205a9f..daca1c9 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -477,7 +477,9 @@ static int bcmgenet_get_link_ksettings(struct net_device 
*dev,
if (!priv->phydev)
return -ENODEV;
 
-   return phy_ethtool_ksettings_get(priv->phydev, cmd);
+   phy_ethtool_ksettings_get(priv->phydev, cmd);
+
+   return 0;
 }
 
 static int bcmgenet_set_link_ksettings(struct net_device *dev,
diff --git a/drivers/net/ethernet/broadcom/tg3.c 
b/drivers/net/ethernet/broadcom/tg3.c
index 537d571..d600c41 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -12097,7 +12097,9 @@ static int tg3_get_link_ksettings(struct net_device 
*dev,
if (!(tp->phy_flags & TG3_PHYFLG_IS_CONNECTED))
return -EAGAIN;
phydev = mdiobus_get_phy(tp->mdio_bus, tp->phy_addr);
-   return phy_ethtool_ksettings_get(phydev, cmd);
+   phy_ethtool_ksettings_get(phydev, cmd);
+
+   return 0;
}
 
supported = (SUPPORTED_Autoneg);
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c 
b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
index 15571e2..aad825088 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
@@ -75,16 +75,14 @@ static char dpaa_stats_global[][ETH_GSTRING_LEN] = {
 static int dpaa_get_link_ksettings(struct net_device *net_dev,
   struct ethtool_link_ksettings *cmd)
 {
-   int err;
-
if (!net_dev->phydev) {
netdev_dbg(net_dev, "phy device not initialized\n");
return 0;
}
 
-   err = phy_ethtool_ksettings_get(net_dev->phydev, cmd);
+   phy_ethtool_ksettings_get(net_dev->phydev, cmd);
 
-   return err;
+   return 0;
 }
 
 static int dpaa_set_link_ksettings(struct net_device *net_dev,
diff --git a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c 
b/d

[PATCH v1] net/phy: Make phy_ethtool_ksettings_get return void

[Yuval Shaia]

Make return value void since function never return meaningfull value

Signed-off-by: Yuval Shaia 
Acked-by: Sergei Shtylyov 
---
Re-sending since last time forgot to add netdev-list
v0 ->v1:
* These files were missing in v0
* drivers/net/ethernet/renesas/ravb_main.c
* drivers/net/ethernet/renesas/sh_eth.c
* drivers/net/ethernet/ti/netcp_ethss.c
* Add Acked-by: Sergei Shtylyov
---
 drivers/net/ethernet/broadcom/b44.c|  3 ++-
 drivers/net/ethernet/broadcom/bcm63xx_enet.c   |  3 ++-
 drivers/net/ethernet/broadcom/genet/bcmgenet.c |  4 +++-
 drivers/net/ethernet/broadcom/tg3.c|  4 +++-
 drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c |  6 ++
 drivers/net/ethernet/freescale/ucc_geth_ethtool.c  |  4 +++-
 drivers/net/ethernet/marvell/mv643xx_eth.c |  5 ++---
 drivers/net/ethernet/renesas/ravb_main.c   | 14 +++---
 drivers/net/ethernet/renesas/sh_eth.c  |  5 ++---
 drivers/net/ethernet/ti/cpsw.c |  9 +
 drivers/net/ethernet/ti/netcp_ethss.c  |  8 +++-
 drivers/net/phy/phy.c  | 10 +-
 drivers/net/usb/lan78xx.c  |  2 +-
 include/linux/phy.h|  4 ++--
 net/dsa/slave.c|  9 +
 15 files changed, 47 insertions(+), 43 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c 
b/drivers/net/ethernet/broadcom/b44.c
index 5b95bb4..9873d2d 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -1836,7 +1836,8 @@ static int b44_get_link_ksettings(struct net_device *dev,
 
if (bp->flags & B44_FLAG_EXTERNAL_PHY) {
BUG_ON(!dev->phydev);
-   return phy_ethtool_ksettings_get(dev->phydev, cmd);
+   phy_ethtool_ksettings_get(dev->phydev, cmd);
+   return 0;
}
 
supported = (SUPPORTED_Autoneg);
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c 
b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index 50d88d3..34ebb40 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -1453,7 +1453,8 @@ static int bcm_enet_get_link_ksettings(struct net_device 
*dev,
if (priv->has_phy) {
if (!dev->phydev)
return -ENODEV;
-   return phy_ethtool_ksettings_get(dev->phydev, cmd);
+   phy_ethtool_ksettings_get(dev->phydev, cmd);
+   return 0;
} else {
cmd->base.autoneg = 0;
cmd->base.speed = (priv->force_speed_100) ?
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index a205a9f..daca1c9 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -477,7 +477,9 @@ static int bcmgenet_get_link_ksettings(struct net_device 
*dev,
if (!priv->phydev)
return -ENODEV;
 
-   return phy_ethtool_ksettings_get(priv->phydev, cmd);
+   phy_ethtool_ksettings_get(priv->phydev, cmd);
+
+   return 0;
 }
 
 static int bcmgenet_set_link_ksettings(struct net_device *dev,
diff --git a/drivers/net/ethernet/broadcom/tg3.c 
b/drivers/net/ethernet/broadcom/tg3.c
index 537d571..d600c41 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -12097,7 +12097,9 @@ static int tg3_get_link_ksettings(struct net_device 
*dev,
if (!(tp->phy_flags & TG3_PHYFLG_IS_CONNECTED))
return -EAGAIN;
phydev = mdiobus_get_phy(tp->mdio_bus, tp->phy_addr);
-   return phy_ethtool_ksettings_get(phydev, cmd);
+   phy_ethtool_ksettings_get(phydev, cmd);
+
+   return 0;
}
 
supported = (SUPPORTED_Autoneg);
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c 
b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
index 15571e2..aad825088 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
@@ -75,16 +75,14 @@ static char dpaa_stats_global[][ETH_GSTRING_LEN] = {
 static int dpaa_get_link_ksettings(struct net_device *net_dev,
   struct ethtool_link_ksettings *cmd)
 {
-   int err;
-
if (!net_dev->phydev) {
netdev_dbg(net_dev, "phy device not initialized\n");
return 0;
}
 
-   err = phy_ethtool_ksettings_get(net_dev->phydev, cmd);
+   phy_ethtool_ksettings_get(net_dev->phydev, cmd);
 
-   return err;
+   return 0;
 }
 
 static int dpaa_set_link_ksettings(struct net_device *net_dev,
diff --git a/drivers/net/ethernet/freescale/ucc_geth_etht

[no subject]

[Yuval Shaia]

subscribe netdev

[PATCH] net/ethtool: Replace memset with compiler directive

[Yuval Shaia]

Take advantage of compiler variable zero-ing and remove calls to memset.

Signed-off-by: Yuval Shaia 
---
 net/core/ethtool.c | 21 +++--
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 03111a2..051af09 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -559,7 +559,7 @@ int __ethtool_get_link_ksettings(struct net_device *dev,
 struct ethtool_link_ksettings *link_ksettings)
 {
int err;
-   struct ethtool_cmd cmd;
+   struct ethtool_cmd cmd = {0};
 
ASSERT_RTNL();
 
@@ -576,7 +576,6 @@ int __ethtool_get_link_ksettings(struct net_device *dev,
if (!dev->ethtool_ops->get_settings)
return -EOPNOTSUPP;
 
-   memset(&cmd, 0, sizeof(cmd));
cmd.cmd = ETHTOOL_GSET;
err = dev->ethtool_ops->get_settings(dev, &cmd);
if (err < 0)
@@ -777,7 +776,7 @@ warn_incomplete_ethtool_legacy_settings_conversion(const 
char *details)
  */
 static int ethtool_get_settings(struct net_device *dev, void __user *useraddr)
 {
-   struct ethtool_cmd cmd;
+   struct ethtool_cmd cmd = {0};
 
ASSERT_RTNL();
 
@@ -808,7 +807,6 @@ static int ethtool_get_settings(struct net_device *dev, 
void __user *useraddr)
if (!dev->ethtool_ops->get_settings)
return -EOPNOTSUPP;
 
-   memset(&cmd, 0, sizeof(cmd));
cmd.cmd = ETHTOOL_GSET;
err = dev->ethtool_ops->get_settings(dev, &cmd);
if (err < 0)
@@ -870,10 +868,9 @@ static int ethtool_set_settings(struct net_device *dev, 
void __user *useraddr)
 static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
  void __user *useraddr)
 {
-   struct ethtool_drvinfo info;
+   struct ethtool_drvinfo info = {0};
const struct ethtool_ops *ops = dev->ethtool_ops;
 
-   memset(&info, 0, sizeof(info));
info.cmd = ETHTOOL_GDRVINFO;
if (ops->get_drvinfo) {
ops->get_drvinfo(dev, &info);
@@ -916,7 +913,7 @@ static noinline_for_stack int ethtool_get_drvinfo(struct 
net_device *dev,
 static noinline_for_stack int ethtool_get_sset_info(struct net_device *dev,
void __user *useraddr)
 {
-   struct ethtool_sset_info info;
+   struct ethtool_sset_info info = {0};
u64 sset_mask;
int i, idx = 0, n_bits = 0, ret, rc;
u32 *info_buf = NULL;
@@ -932,7 +929,6 @@ static noinline_for_stack int ethtool_get_sset_info(struct 
net_device *dev,
/* calculate size of return buffer */
n_bits = hweight64(sset_mask);
 
-   memset(&info, 0, sizeof(info));
info.cmd = ETHTOOL_GSSET_INFO;
 
info_buf = kzalloc(n_bits * sizeof(u32), GFP_USER);
@@ -1479,13 +1475,12 @@ static int ethtool_set_wol(struct net_device *dev, char 
__user *useraddr)
 
 static int ethtool_get_eee(struct net_device *dev, char __user *useraddr)
 {
-   struct ethtool_eee edata;
+   struct ethtool_eee edata = {0};
int rc;
 
if (!dev->ethtool_ops->get_eee)
return -EOPNOTSUPP;
 
-   memset(&edata, 0, sizeof(struct ethtool_eee));
edata.cmd = ETHTOOL_GEEE;
rc = dev->ethtool_ops->get_eee(dev, &edata);
 
@@ -2109,7 +2104,7 @@ static int ethtool_get_dump_data(struct net_device *dev,
 {
int ret;
__u32 len;
-   struct ethtool_dump dump, tmp;
+   struct ethtool_dump dump, tmp = {0};
const struct ethtool_ops *ops = dev->ethtool_ops;
void *data = NULL;
 
@@ -2119,7 +2114,6 @@ static int ethtool_get_dump_data(struct net_device *dev,
if (copy_from_user(&dump, useraddr, sizeof(dump)))
return -EFAULT;
 
-   memset(&tmp, 0, sizeof(tmp));
tmp.cmd = ETHTOOL_GET_DUMP_FLAG;
ret = ops->get_dump_flag(dev, &tmp);
if (ret)
@@ -2170,11 +2164,10 @@ static int ethtool_get_dump_data(struct net_device *dev,
 static int ethtool_get_ts_info(struct net_device *dev, void __user *useraddr)
 {
int err = 0;
-   struct ethtool_ts_info info;
+   struct ethtool_ts_info info = {0};
const struct ethtool_ops *ops = dev->ethtool_ops;
struct phy_device *phydev = dev->phydev;
 
-   memset(&info, 0, sizeof(info));
info.cmd = ETHTOOL_GET_TS_INFO;
 
if (phydev && phydev->drv && phydev->drv->ts_info) {
-- 
2.9.4

[PATCH v1] net: phy: Delete unused function phy_ethtool_gset

[Yuval Shaia]

It's unused, so remove it.

Signed-off-by: Yuval Shaia 
---
v0 -> v1:
* Add commit message
* Update Documentation/networking/phy.txt
* Modify commit header message
---
 Documentation/networking/phy.txt |  1 -
 drivers/net/phy/phy.c| 24 
 include/linux/phy.h  |  1 -
 3 files changed, 26 deletions(-)

diff --git a/Documentation/networking/phy.txt b/Documentation/networking/phy.txt
index 16f90d8..bdec0f7 100644
--- a/Documentation/networking/phy.txt
+++ b/Documentation/networking/phy.txt
@@ -295,7 +295,6 @@ Doing it all yourself
settings in the PHY.
 
  int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd);
- int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
 
Ethtool convenience functions.
 
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 9c372bf..8e26af5 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -484,30 +484,6 @@ int phy_ethtool_ksettings_set(struct phy_device *phydev,
 }
 EXPORT_SYMBOL(phy_ethtool_ksettings_set);
 
-int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd)
-{
-   cmd->supported = phydev->supported;
-
-   cmd->advertising = phydev->advertising;
-   cmd->lp_advertising = phydev->lp_advertising;
-
-   ethtool_cmd_speed_set(cmd, phydev->speed);
-   cmd->duplex = phydev->duplex;
-   if (phydev->interface == PHY_INTERFACE_MODE_MOCA)
-   cmd->port = PORT_BNC;
-   else
-   cmd->port = PORT_MII;
-   cmd->phy_address = phydev->mdio.addr;
-   cmd->transceiver = phy_is_internal(phydev) ?
-   XCVR_INTERNAL : XCVR_EXTERNAL;
-   cmd->autoneg = phydev->autoneg;
-   cmd->eth_tp_mdix_ctrl = phydev->mdix_ctrl;
-   cmd->eth_tp_mdix = phydev->mdix;
-
-   return 0;
-}
-EXPORT_SYMBOL(phy_ethtool_gset);
-
 void phy_ethtool_ksettings_get(struct phy_device *phydev,
   struct ethtool_link_ksettings *cmd)
 {
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 4ec07a6..804851c 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -843,7 +843,6 @@ void phy_start_machine(struct phy_device *phydev);
 void phy_stop_machine(struct phy_device *phydev);
 void phy_trigger_machine(struct phy_device *phydev, bool sync);
 int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd);
-int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
 void phy_ethtool_ksettings_get(struct phy_device *phydev,
   struct ethtool_link_ksettings *cmd);
 int phy_ethtool_ksettings_set(struct phy_device *phydev,
-- 
2.9.4

Re: [PATCH] net/{mii,smsc}: Make mii_ethtool_get_link_ksettings and smc_netdev_get_ecmd return void

[Yuval Shaia]

On Sun, Jun 04, 2017 at 09:01:33PM +0200, Andrew Lunn wrote:
> > diff --git a/drivers/net/cris/eth_v10.c b/drivers/net/cris/eth_v10.c
> > index da02041..017f48c 100644
> > --- a/drivers/net/cris/eth_v10.c
> > +++ b/drivers/net/cris/eth_v10.c
> > @@ -1417,10 +1417,9 @@ static int e100_get_link_ksettings(struct net_device 
> > *dev,
> >  {
> > struct net_local *np = netdev_priv(dev);
> > u32 supported;
> > -   int err;
> >  
> > spin_lock_irq(&np->lock);
> > -   err = mii_ethtool_get_link_ksettings(&np->mii_if, cmd);
> > +   mii_ethtool_get_link_ksettings(&np->mii_if, cmd);
> > spin_unlock_irq(&np->lock);
> >  
> > /* The PHY may support 1000baseT, but the Etrax100 does not.  */
> > @@ -1432,7 +1431,7 @@ static int e100_get_link_ksettings(struct net_device 
> > *dev,
> > ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
> > supported);
> >  
> > -   return err;
> > +   return 0;
> >  }
> 
> How far are going planning on going? It seems like
> *_get_link_ksettings() now all return a useless 0. Do you plan to
> change ethtool_ops and make if void all the way up?

It is not always correct, see for example how xgene_get_link_ksettings
returns non-zero value so i assume that ethtool_ops should remain as it is.
Also, looking at ethtool_get_settings it seems that returned value is
checked.

> 
>Andrew

Re: [PATCH] net/dec: Make __de_get_link_ksettings return void

[Yuval Shaia]

On Sun, Jun 04, 2017 at 08:22:05PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 6/4/2017 8:08 PM, Yuval Shaia wrote:
> 
> > Make return value void since function never return meaningfull value
> 
>   You only make 1 of 2 functions void. It looks like there should be 2 
> patches.

And what would be the second function?

> 
> > 
> > Signed-off-by: Yuval Shaia 
> > ---
> >  drivers/net/ethernet/dec/tulip/de2104x.c | 11 ---
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c 
> > b/drivers/net/ethernet/dec/tulip/de2104x.c
> > index 91b8f6f..c87b8cc 100644
> > --- a/drivers/net/ethernet/dec/tulip/de2104x.c
> > +++ b/drivers/net/ethernet/dec/tulip/de2104x.c
> > @@ -1483,8 +1483,8 @@ static void __de_get_regs(struct de_private *de, u8 
> > *buf)
> > de_rx_missed(de, rbuf[8]);
> >  }
> > 
> > -static int __de_get_link_ksettings(struct de_private *de,
> > -  struct ethtool_link_ksettings *cmd)
> > +static void __de_get_link_ksettings(struct de_private *de,
> > +   struct ethtool_link_ksettings *cmd)
> >  {
> > ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
> > de->media_supported);
> > @@ -1517,8 +1517,6 @@ static int __de_get_link_ksettings(struct de_private 
> > *de,
> > cmd->base.autoneg = AUTONEG_ENABLE;
> > 
> > /* ignore maxtxpkt, maxrxpkt for now */
> > -
> > -   return 0;
> >  }
> > 
> >  static int __de_set_link_ksettings(struct de_private *de,
> > @@ -1615,13 +1613,12 @@ static int de_get_link_ksettings(struct net_device 
> > *dev,
> >  struct ethtool_link_ksettings *cmd)
> >  {
> > struct de_private *de = netdev_priv(dev);
> > -   int rc;
> > 
> > spin_lock_irq(&de->lock);
> > -   rc = __de_get_link_ksettings(de, cmd);
> > +   __de_get_link_ksettings(de, cmd);
> > spin_unlock_irq(&de->lock);
> > 
> > -   return rc;
> > +   return 0;
> >  }
> > 
> >  static int de_set_link_ksettings(struct net_device *dev,
> > 
> 
> MBR, Sergei
> 

[PATCH] net/{mii,smsc}: Make mii_ethtool_get_link_ksettings and smc_netdev_get_ecmd return void

[Yuval Shaia]

Make return value void since functions never returns meaningfull value.

Signed-off-by: Yuval Shaia 
---
 drivers/net/cris/eth_v10.c  |  5 ++---
 drivers/net/ethernet/3com/3c59x.c   |  4 +++-
 drivers/net/ethernet/amd/pcnet32.c  |  5 +
 drivers/net/ethernet/cirrus/ep93xx_eth.c|  5 -
 drivers/net/ethernet/dec/tulip/winbond-840.c|  5 ++---
 drivers/net/ethernet/faraday/ftmac100.c |  5 -
 drivers/net/ethernet/fealnx.c   |  5 ++---
 drivers/net/ethernet/intel/e100.c   |  5 -
 drivers/net/ethernet/jme.c  |  5 ++---
 drivers/net/ethernet/korina.c   |  5 ++---
 drivers/net/ethernet/micrel/ks8851.c|  5 -
 drivers/net/ethernet/micrel/ks8851_mll.c|  5 -
 drivers/net/ethernet/nuvoton/w90p910_ether.c|  5 -
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c |  6 +++---
 drivers/net/ethernet/realtek/8139cp.c   |  5 ++---
 drivers/net/ethernet/realtek/r8169.c|  4 +++-
 drivers/net/ethernet/sgi/ioc3-eth.c |  5 ++---
 drivers/net/ethernet/sis/sis190.c   |  4 +++-
 drivers/net/ethernet/smsc/epic100.c |  5 ++---
 drivers/net/ethernet/smsc/smc911x.c |  7 +++
 drivers/net/ethernet/smsc/smc91c92_cs.c | 13 +
 drivers/net/ethernet/smsc/smc91x.c  |  7 ++-
 drivers/net/ethernet/tundra/tsi108_eth.c|  5 ++---
 drivers/net/ethernet/via/via-rhine.c|  5 ++---
 drivers/net/mii.c   |  8 ++--
 drivers/net/usb/ax88179_178a.c  |  5 -
 drivers/net/usb/r8152.c |  2 +-
 drivers/net/usb/usbnet.c|  4 +++-
 include/linux/mii.h |  2 +-
 29 files changed, 78 insertions(+), 73 deletions(-)

diff --git a/drivers/net/cris/eth_v10.c b/drivers/net/cris/eth_v10.c
index da02041..017f48c 100644
--- a/drivers/net/cris/eth_v10.c
+++ b/drivers/net/cris/eth_v10.c
@@ -1417,10 +1417,9 @@ static int e100_get_link_ksettings(struct net_device 
*dev,
 {
struct net_local *np = netdev_priv(dev);
u32 supported;
-   int err;
 
spin_lock_irq(&np->lock);
-   err = mii_ethtool_get_link_ksettings(&np->mii_if, cmd);
+   mii_ethtool_get_link_ksettings(&np->mii_if, cmd);
spin_unlock_irq(&np->lock);
 
/* The PHY may support 1000baseT, but the Etrax100 does not.  */
@@ -1432,7 +1431,7 @@ static int e100_get_link_ksettings(struct net_device *dev,
ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
supported);
 
-   return err;
+   return 0;
 }
 
 static int e100_set_link_ksettings(struct net_device *dev,
diff --git a/drivers/net/ethernet/3com/3c59x.c 
b/drivers/net/ethernet/3com/3c59x.c
index e41245a..14cff60 100644
--- a/drivers/net/ethernet/3com/3c59x.c
+++ b/drivers/net/ethernet/3com/3c59x.c
@@ -2912,7 +2912,9 @@ static int vortex_get_link_ksettings(struct net_device 
*dev,
 {
struct vortex_private *vp = netdev_priv(dev);
 
-   return mii_ethtool_get_link_ksettings(&vp->mii, cmd);
+   mii_ethtool_get_link_ksettings(&vp->mii, cmd);
+
+   return 0;
 }
 
 static int vortex_set_link_ksettings(struct net_device *dev,
diff --git a/drivers/net/ethernet/amd/pcnet32.c 
b/drivers/net/ethernet/amd/pcnet32.c
index 86369d7..7f60d17 100644
--- a/drivers/net/ethernet/amd/pcnet32.c
+++ b/drivers/net/ethernet/amd/pcnet32.c
@@ -731,12 +731,10 @@ static int pcnet32_get_link_ksettings(struct net_device 
*dev,
 {
struct pcnet32_private *lp = netdev_priv(dev);
unsigned long flags;
-   int r = -EOPNOTSUPP;
 
spin_lock_irqsave(&lp->lock, flags);
if (lp->mii) {
mii_ethtool_get_link_ksettings(&lp->mii_if, cmd);
-   r = 0;
} else if (lp->chip_version == PCNET32_79C970A) {
if (lp->autoneg) {
cmd->base.autoneg = AUTONEG_ENABLE;
@@ -753,10 +751,9 @@ static int pcnet32_get_link_ksettings(struct net_device 
*dev,
ethtool_convert_legacy_u32_to_link_mode(
cmd->link_modes.supported,
SUPPORTED_TP | SUPPORTED_AUI);
-   r = 0;
}
spin_unlock_irqrestore(&lp->lock, flags);
-   return r;
+   return 0;
 }
 
 static int pcnet32_set_link_ksettings(struct net_device *dev,
diff --git a/drivers/net/ethernet/cirrus/ep93xx_eth.c 
b/drivers/net/ethernet/cirrus/ep93xx_eth.c
index 7a7c02f

[PATCH] net/3com: Make el3_netdev_get_ecmd return void

[Yuval Shaia]

Make return value void since function never returns meaningfull value.

Signed-off-by: Yuval Shaia 
---
 drivers/net/ethernet/3com/3c509.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/3com/3c509.c 
b/drivers/net/ethernet/3com/3c509.c
index db8592d..f66c971 100644
--- a/drivers/net/ethernet/3com/3c509.c
+++ b/drivers/net/ethernet/3com/3c509.c
@@ -1039,7 +1039,7 @@ el3_link_ok(struct net_device *dev)
return tmp & (1<<11);
 }
 
-static int
+static void
 el3_netdev_get_ecmd(struct net_device *dev, struct ethtool_link_ksettings *cmd)
 {
u16 tmp;
@@ -1082,7 +1082,6 @@ el3_netdev_get_ecmd(struct net_device *dev, struct 
ethtool_link_ksettings *cmd)
supported);
cmd->base.speed = SPEED_10;
EL3WINDOW(1);
-   return 0;
 }
 
 static int
@@ -1151,12 +1150,11 @@ static int el3_get_link_ksettings(struct net_device 
*dev,
  struct ethtool_link_ksettings *cmd)
 {
struct el3_private *lp = netdev_priv(dev);
-   int ret;
 
spin_lock_irq(&lp->lock);
-   ret = el3_netdev_get_ecmd(dev, cmd);
+   el3_netdev_get_ecmd(dev, cmd);
spin_unlock_irq(&lp->lock);
-   return ret;
+   return 0;
 }
 
 static int el3_set_link_ksettings(struct net_device *dev,
-- 
2.9.4

[PATCH] net/phy: Make phy_ethtool_ksettings_get return void

[Yuval Shaia]

Make return value void since function never return meaningfull value

Signed-off-by: Yuval Shaia 
---
 drivers/net/ethernet/broadcom/b44.c|  3 ++-
 drivers/net/ethernet/broadcom/bcm63xx_enet.c   |  3 ++-
 drivers/net/ethernet/broadcom/genet/bcmgenet.c |  4 +++-
 drivers/net/ethernet/broadcom/tg3.c|  4 +++-
 drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c |  6 ++
 drivers/net/ethernet/freescale/ucc_geth_ethtool.c  |  4 +++-
 drivers/net/ethernet/marvell/mv643xx_eth.c |  5 ++---
 drivers/net/ethernet/ti/cpsw.c |  9 +
 drivers/net/phy/phy.c  | 10 +-
 drivers/net/usb/lan78xx.c  |  2 +-
 include/linux/phy.h|  4 ++--
 net/dsa/slave.c|  9 +
 12 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c 
b/drivers/net/ethernet/broadcom/b44.c
index 5b95bb4..9873d2d 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -1836,7 +1836,8 @@ static int b44_get_link_ksettings(struct net_device *dev,
 
if (bp->flags & B44_FLAG_EXTERNAL_PHY) {
BUG_ON(!dev->phydev);
-   return phy_ethtool_ksettings_get(dev->phydev, cmd);
+   phy_ethtool_ksettings_get(dev->phydev, cmd);
+   return 0;
}
 
supported = (SUPPORTED_Autoneg);
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c 
b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index 50d88d3..34ebb40 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -1453,7 +1453,8 @@ static int bcm_enet_get_link_ksettings(struct net_device 
*dev,
if (priv->has_phy) {
if (!dev->phydev)
return -ENODEV;
-   return phy_ethtool_ksettings_get(dev->phydev, cmd);
+   phy_ethtool_ksettings_get(dev->phydev, cmd);
+   return 0;
} else {
cmd->base.autoneg = 0;
cmd->base.speed = (priv->force_speed_100) ?
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index a205a9f..daca1c9 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -477,7 +477,9 @@ static int bcmgenet_get_link_ksettings(struct net_device 
*dev,
if (!priv->phydev)
return -ENODEV;
 
-   return phy_ethtool_ksettings_get(priv->phydev, cmd);
+   phy_ethtool_ksettings_get(priv->phydev, cmd);
+
+   return 0;
 }
 
 static int bcmgenet_set_link_ksettings(struct net_device *dev,
diff --git a/drivers/net/ethernet/broadcom/tg3.c 
b/drivers/net/ethernet/broadcom/tg3.c
index 537d571..d600c41 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -12097,7 +12097,9 @@ static int tg3_get_link_ksettings(struct net_device 
*dev,
if (!(tp->phy_flags & TG3_PHYFLG_IS_CONNECTED))
return -EAGAIN;
phydev = mdiobus_get_phy(tp->mdio_bus, tp->phy_addr);
-   return phy_ethtool_ksettings_get(phydev, cmd);
+   phy_ethtool_ksettings_get(phydev, cmd);
+
+   return 0;
}
 
supported = (SUPPORTED_Autoneg);
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c 
b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
index 15571e2..aad825088 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
@@ -75,16 +75,14 @@ static char dpaa_stats_global[][ETH_GSTRING_LEN] = {
 static int dpaa_get_link_ksettings(struct net_device *net_dev,
   struct ethtool_link_ksettings *cmd)
 {
-   int err;
-
if (!net_dev->phydev) {
netdev_dbg(net_dev, "phy device not initialized\n");
return 0;
}
 
-   err = phy_ethtool_ksettings_get(net_dev->phydev, cmd);
+   phy_ethtool_ksettings_get(net_dev->phydev, cmd);
 
-   return err;
+   return 0;
 }
 
 static int dpaa_set_link_ksettings(struct net_device *net_dev,
diff --git a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c 
b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
index b642990..4df282e 100644
--- a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
+++ b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
@@ -113,7 +113,9 @@ uec_get_ksettings(struct net_device *netdev, struct 
ethtool_link_ksettings *cmd)
if (!phydev)
return -ENODEV;
 
-   return phy_ethtool_ksettings_get(phydev, cmd);
+   phy_ethtool_ksettings_get(phydev, cmd);
+
+   return 0;
 }
 
 static int
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth

[PATCH] net/phy: Delete unused function phy_ethtool_gset

[Yuval Shaia]

Signed-off-by: Yuval Shaia 
---
 drivers/net/phy/phy.c | 24 
 include/linux/phy.h   |  1 -
 2 files changed, 25 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 9c372bf..8e26af5 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -484,30 +484,6 @@ int phy_ethtool_ksettings_set(struct phy_device *phydev,
 }
 EXPORT_SYMBOL(phy_ethtool_ksettings_set);
 
-int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd)
-{
-   cmd->supported = phydev->supported;
-
-   cmd->advertising = phydev->advertising;
-   cmd->lp_advertising = phydev->lp_advertising;
-
-   ethtool_cmd_speed_set(cmd, phydev->speed);
-   cmd->duplex = phydev->duplex;
-   if (phydev->interface == PHY_INTERFACE_MODE_MOCA)
-   cmd->port = PORT_BNC;
-   else
-   cmd->port = PORT_MII;
-   cmd->phy_address = phydev->mdio.addr;
-   cmd->transceiver = phy_is_internal(phydev) ?
-   XCVR_INTERNAL : XCVR_EXTERNAL;
-   cmd->autoneg = phydev->autoneg;
-   cmd->eth_tp_mdix_ctrl = phydev->mdix_ctrl;
-   cmd->eth_tp_mdix = phydev->mdix;
-
-   return 0;
-}
-EXPORT_SYMBOL(phy_ethtool_gset);
-
 void phy_ethtool_ksettings_get(struct phy_device *phydev,
   struct ethtool_link_ksettings *cmd)
 {
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 4ec07a6..804851c 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -843,7 +843,6 @@ void phy_start_machine(struct phy_device *phydev);
 void phy_stop_machine(struct phy_device *phydev);
 void phy_trigger_machine(struct phy_device *phydev, bool sync);
 int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd);
-int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
 void phy_ethtool_ksettings_get(struct phy_device *phydev,
   struct ethtool_link_ksettings *cmd);
 int phy_ethtool_ksettings_set(struct phy_device *phydev,
-- 
2.9.4

[PATCH] net/dec: Make __de_get_link_ksettings return void

[Yuval Shaia]

Make return value void since function never return meaningfull value

Signed-off-by: Yuval Shaia 
---
 drivers/net/ethernet/dec/tulip/de2104x.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c 
b/drivers/net/ethernet/dec/tulip/de2104x.c
index 91b8f6f..c87b8cc 100644
--- a/drivers/net/ethernet/dec/tulip/de2104x.c
+++ b/drivers/net/ethernet/dec/tulip/de2104x.c
@@ -1483,8 +1483,8 @@ static void __de_get_regs(struct de_private *de, u8 *buf)
de_rx_missed(de, rbuf[8]);
 }
 
-static int __de_get_link_ksettings(struct de_private *de,
-  struct ethtool_link_ksettings *cmd)
+static void __de_get_link_ksettings(struct de_private *de,
+   struct ethtool_link_ksettings *cmd)
 {
ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
de->media_supported);
@@ -1517,8 +1517,6 @@ static int __de_get_link_ksettings(struct de_private *de,
cmd->base.autoneg = AUTONEG_ENABLE;
 
/* ignore maxtxpkt, maxrxpkt for now */
-
-   return 0;
 }
 
 static int __de_set_link_ksettings(struct de_private *de,
@@ -1615,13 +1613,12 @@ static int de_get_link_ksettings(struct net_device *dev,
 struct ethtool_link_ksettings *cmd)
 {
struct de_private *de = netdev_priv(dev);
-   int rc;
 
spin_lock_irq(&de->lock);
-   rc = __de_get_link_ksettings(de, cmd);
+   __de_get_link_ksettings(de, cmd);
spin_unlock_irq(&de->lock);
 
-   return rc;
+   return 0;
 }
 
 static int de_set_link_ksettings(struct net_device *dev,
-- 
2.9.4

Re: [PATCH net-next] net/mlx5e: Fix possible memory leak

[Yuval Shaia]

On Thu, May 18, 2017 at 03:34:41PM +, Wei Yongjun wrote:
> From: Wei Yongjun 
> 
> 'encap_header' is malloced and should be freed before leaving from
> the error handling cases, otherwise it will cause memory leak.
> 
> Fixes: 232c001398ae ("net/mlx5e: Add support to neighbour update flow")
> Signed-off-by: Wei Yongjun 
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c 
> b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index 11c27e4..a72ecbc 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -1404,8 +1404,8 @@ static int mlx5e_create_encap_header_ipv4(struct 
> mlx5e_priv *priv,
>  
>   if (!(nud_state & NUD_VALID)) {
>   neigh_event_send(n, NULL);
> - neigh_release(n);
> - return -EAGAIN;
> + err = -EAGAIN;
> + goto out;
>   }
>  
>   err = mlx5_encap_alloc(priv->mdev, e->tunnel_type,
> @@ -1510,8 +1510,8 @@ static int mlx5e_create_encap_header_ipv6(struct 
> mlx5e_priv *priv,
>  
>   if (!(nud_state & NUD_VALID)) {
>   neigh_event_send(n, NULL);
> - neigh_release(n);
> - return -EAGAIN;
> + err = -EAGAIN;
> + goto out;
>   }

Reviewed-by: Yuval Shaia 

>  
>   err = mlx5_encap_alloc(priv->mdev, e->tunnel_type,
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] net/mlx4_core: Use min3 to select number of MSI-X vectors

[Yuval Shaia]

On Mon, May 15, 2017 at 07:43:04AM +0300, Leon Romanovsky wrote:
> On Sun, May 14, 2017 at 10:01:34PM +0300, Yuval Shaia wrote:
> > On Fri, May 12, 2017 at 09:10:51AM +0300, Yuval Shaia wrote:
> > > Signed-off-by: Yuval Shaia 
> > > ---
> > > v0 -> v1:
> > >   * s/"min_t("/"min_t(int"
> > > v1 -> v2:
> > >   * Use min3 instead of min_t twice
> > > v2 -> v3:
> > >   * Change commit log header message to reflect the changes made in
> > > v2
> > > v3 -> v4:
> > >   * Cast return value from num_online_cpus to int to avoid
> > > compilation errors from "sparse"
> >
> > Hi Leon,
> > Got your r-b for v3, can you please review v4?
> 
> What was the sparse error?
> 
> num_online_cpus() is declared as "unsigned int", so you don't need to do 
> casting.
> 
> ➜  linux-rdma git:(master) git grep num_online_cpus include/
> include/linux/cpumask.h:#define num_online_cpus()   
> cpumask_weight(cpu_online_mask)
> include/linux/cpumask.h:#define num_online_cpus()   1U
> 
> ➜  linux-rdma git:(master) git grep cpumask_weight include/
> include/linux/cpumask.h:static inline unsigned int cpumask_weight(const 
> struct cpumask *srcp)

But num_eqs and reserved_eqs are ints so assuming the error is that i
asked min3 to compare unsigned int with int.

This casting cannot do harm because anyway the result is stored in int
variable.

> 
> Thanks
> 
> >
> > Thanks,
> >
> > > ---
> > >  drivers/net/ethernet/mellanox/mlx4/main.c | 10 --
> > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c 
> > > b/drivers/net/ethernet/mellanox/mlx4/main.c
> > > index 7032054..83aab1e 100644
> > > --- a/drivers/net/ethernet/mellanox/mlx4/main.c
> > > +++ b/drivers/net/ethernet/mellanox/mlx4/main.c
> > > @@ -2862,12 +2862,10 @@ static void mlx4_enable_msi_x(struct mlx4_dev 
> > > *dev)
> > >   int port = 0;
> > >
> > >   if (msi_x) {
> > > - int nreq = dev->caps.num_ports * num_online_cpus() + 1;
> > > -
> > > - nreq = min_t(int, dev->caps.num_eqs - dev->caps.reserved_eqs,
> > > -  nreq);
> > > - if (nreq > MAX_MSIX)
> > > - nreq = MAX_MSIX;
> > > + int nreq = min3(dev->caps.num_ports *
> > > + (int)num_online_cpus() + 1,
> > > + dev->caps.num_eqs - dev->caps.reserved_eqs,
> > > + MAX_MSIX);
> > >
> > >   entries = kcalloc(nreq, sizeof *entries, GFP_KERNEL);
> > >   if (!entries)
> > > --
> > > 2.7.4
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > > the body of a message to majord...@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] net/mlx4_core: Use min3 to select number of MSI-X vectors

[Yuval Shaia]

On Fri, May 12, 2017 at 09:10:51AM +0300, Yuval Shaia wrote:
> Signed-off-by: Yuval Shaia 
> ---
> v0 -> v1:
>   * s/"min_t("/"min_t(int"
> v1 -> v2:
>   * Use min3 instead of min_t twice
> v2 -> v3:
>   * Change commit log header message to reflect the changes made in
> v2
> v3 -> v4:
>   * Cast return value from num_online_cpus to int to avoid
> compilation errors from "sparse"

Hi Leon,
Got your r-b for v3, can you please review v4?

Thanks,

> ---
>  drivers/net/ethernet/mellanox/mlx4/main.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c 
> b/drivers/net/ethernet/mellanox/mlx4/main.c
> index 7032054..83aab1e 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/main.c
> @@ -2862,12 +2862,10 @@ static void mlx4_enable_msi_x(struct mlx4_dev *dev)
>   int port = 0;
>  
>   if (msi_x) {
> - int nreq = dev->caps.num_ports * num_online_cpus() + 1;
> -
> - nreq = min_t(int, dev->caps.num_eqs - dev->caps.reserved_eqs,
> -  nreq);
> - if (nreq > MAX_MSIX)
> - nreq = MAX_MSIX;
> + int nreq = min3(dev->caps.num_ports *
> + (int)num_online_cpus() + 1,
> + dev->caps.num_eqs - dev->caps.reserved_eqs,
> + MAX_MSIX);
>  
>   entries = kcalloc(nreq, sizeof *entries, GFP_KERNEL);
>   if (!entries)
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] net/mlx4_core: Use min3 to select number of MSI-X vectors

[Yuval Shaia]

Signed-off-by: Yuval Shaia 
---
v0 -> v1:
* s/"min_t("/"min_t(int"
v1 -> v2:
* Use min3 instead of min_t twice
v2 -> v3:
* Change commit log header message to reflect the changes made in
  v2
v3 -> v4:
* Cast return value from num_online_cpus to int to avoid
  compilation errors from "sparse"
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c 
b/drivers/net/ethernet/mellanox/mlx4/main.c
index 7032054..83aab1e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2862,12 +2862,10 @@ static void mlx4_enable_msi_x(struct mlx4_dev *dev)
int port = 0;
 
if (msi_x) {
-   int nreq = dev->caps.num_ports * num_online_cpus() + 1;
-
-   nreq = min_t(int, dev->caps.num_eqs - dev->caps.reserved_eqs,
-nreq);
-   if (nreq > MAX_MSIX)
-   nreq = MAX_MSIX;
+   int nreq = min3(dev->caps.num_ports *
+   (int)num_online_cpus() + 1,
+   dev->caps.num_eqs - dev->caps.reserved_eqs,
+   MAX_MSIX);
 
entries = kcalloc(nreq, sizeof *entries, GFP_KERNEL);
if (!entries)
-- 
2.7.4

[PATCH v3] net/mlx4_core: Use min3 to select number of MSI-X vectors

[Yuval Shaia]

Signed-off-by: Yuval Shaia 
---
v0 -> v1:
* s/"min_t("/"min_t(int"
v1 -> v2:
* Use min3 instead of min_t twice
v2 -> v3:
* Change commit log header message to reflect the changes made in
  v2
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c 
b/drivers/net/ethernet/mellanox/mlx4/main.c
index 7032054..2afa340 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2862,12 +2862,9 @@ static void mlx4_enable_msi_x(struct mlx4_dev *dev)
int port = 0;
 
if (msi_x) {
-   int nreq = dev->caps.num_ports * num_online_cpus() + 1;
-
-   nreq = min_t(int, dev->caps.num_eqs - dev->caps.reserved_eqs,
-nreq);
-   if (nreq > MAX_MSIX)
-   nreq = MAX_MSIX;
+   int nreq = min3(dev->caps.num_ports * num_online_cpus() + 1,
+   dev->caps.num_eqs - dev->caps.reserved_eqs,
+   MAX_MSIX);
 
entries = kcalloc(nreq, sizeof *entries, GFP_KERNEL);
if (!entries)
-- 
2.7.4

[PATCH v2] net/mlx4_core: Use min_t instead of if for consistency

[Yuval Shaia]

Signed-off-by: Yuval Shaia 
---
v0 -> v1:
* s/"min_t("/"min_t(int"
v1 -> v2:
* Use min3 instead of min_t twice
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c 
b/drivers/net/ethernet/mellanox/mlx4/main.c
index 7032054..2afa340 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2862,12 +2862,9 @@ static void mlx4_enable_msi_x(struct mlx4_dev *dev)
int port = 0;
 
if (msi_x) {
-   int nreq = dev->caps.num_ports * num_online_cpus() + 1;
-
-   nreq = min_t(int, dev->caps.num_eqs - dev->caps.reserved_eqs,
-nreq);
-   if (nreq > MAX_MSIX)
-   nreq = MAX_MSIX;
+   int nreq = min3(dev->caps.num_ports * num_online_cpus() + 1,
+   dev->caps.num_eqs - dev->caps.reserved_eqs,
+   MAX_MSIX);
 
entries = kcalloc(nreq, sizeof *entries, GFP_KERNEL);
if (!entries)
-- 
2.7.4

Re: [PATCH v1] net/mlx4_core: Use min_t instead of if for consistency

[Yuval Shaia]

On Thu, May 11, 2017 at 01:23:29PM +0300, Leon Romanovsky wrote:
> On Thu, May 11, 2017 at 11:46:29AM +0300, Yuval Shaia wrote:
> > Signed-off-by: Yuval Shaia 
> > ---
> > v0 -> v1:
> > * s/"min_t("/"min_t(int"
> > ---
> >  drivers/net/ethernet/mellanox/mlx4/main.c | 9 -
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c 
> > b/drivers/net/ethernet/mellanox/mlx4/main.c
> > index 7032054..7bb377e 100644
> > --- a/drivers/net/ethernet/mellanox/mlx4/main.c
> > +++ b/drivers/net/ethernet/mellanox/mlx4/main.c
> > @@ -2862,12 +2862,11 @@ static void mlx4_enable_msi_x(struct mlx4_dev *dev)
> > int port = 0;
> >
> > if (msi_x) {
> > -   int nreq = dev->caps.num_ports * num_online_cpus() + 1;
> > +   int nreq = min_t(int,
> > +dev->caps.num_ports * num_online_cpus() + 1,
> > +dev->caps.num_eqs - dev->caps.reserved_eqs);
> >
> > -   nreq = min_t(int, dev->caps.num_eqs - dev->caps.reserved_eqs,
> > -nreq);
> > -   if (nreq > MAX_MSIX)
> > -   nreq = MAX_MSIX;
> > +   nreq = min_t(int, nreq, MAX_MSIX);
> 
> You don't need type checking for these variables, they are all int and
> you can use directly min3(..).

Ha ha ha, i can't explain how this macro slipped out from my sight when was
looking for it :).
Hope you can review v2.

> 
> Thanks
> 
> >
> > entries = kcalloc(nreq, sizeof *entries, GFP_KERNEL);
> > if (!entries)
> > --
> > 2.7.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1] net/mlx4_core: Use min_t instead of if for consistency

[Yuval Shaia]

Signed-off-by: Yuval Shaia 
---
v0 -> v1:
* s/"min_t("/"min_t(int"
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c 
b/drivers/net/ethernet/mellanox/mlx4/main.c
index 7032054..7bb377e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2862,12 +2862,11 @@ static void mlx4_enable_msi_x(struct mlx4_dev *dev)
int port = 0;
 
if (msi_x) {
-   int nreq = dev->caps.num_ports * num_online_cpus() + 1;
+   int nreq = min_t(int,
+dev->caps.num_ports * num_online_cpus() + 1,
+dev->caps.num_eqs - dev->caps.reserved_eqs);
 
-   nreq = min_t(int, dev->caps.num_eqs - dev->caps.reserved_eqs,
-nreq);
-   if (nreq > MAX_MSIX)
-   nreq = MAX_MSIX;
+   nreq = min_t(int, nreq, MAX_MSIX);
 
entries = kcalloc(nreq, sizeof *entries, GFP_KERNEL);
if (!entries)
-- 
2.7.4

[PATCH] net/mlx4_core: Use min_t instead of if for consistency

[Yuval Shaia]

Signed-off-by: Yuval Shaia 
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c 
b/drivers/net/ethernet/mellanox/mlx4/main.c
index 7032054..a58b15a 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2862,12 +2862,11 @@ static void mlx4_enable_msi_x(struct mlx4_dev *dev)
int port = 0;
 
if (msi_x) {
-   int nreq = dev->caps.num_ports * num_online_cpus() + 1;
+   int nreq = min_t(int,
+dev->caps.num_ports * num_online_cpus() + 1,
+dev->caps.num_eqs - dev->caps.reserved_eqs);
 
-   nreq = min_t(int, dev->caps.num_eqs - dev->caps.reserved_eqs,
-nreq);
-   if (nreq > MAX_MSIX)
-   nreq = MAX_MSIX;
+   nreq = min_t(nreq, MAX_MSIX);
 
entries = kcalloc(nreq, sizeof *entries, GFP_KERNEL);
if (!entries)
-- 
2.7.4

Re: [PATCH] IB/IPoIB: Check the headroom size

[Yuval Shaia]

8:c900085e38e0  EFLAGS: 00010246
> [  124.023578] RAX: 0085 RBX: 880816a72500 RCX: 
> 
> [  124.023579] RDX:  RSI: 0296 RDI: 
> 0296
> [  124.023580] RBP: c900085e3900 R08: 0085 R09: 
> 82012ce5
> [  124.023581] R10: 03ed R11:  R12: 
> 8808198b7368
> [  124.023581] R13: 0608 R14: 0701de0a R15: 
> 8808198b7000
> [  124.023583] FS:  2b3922409b00() GS:88083fc8() 
> knlGS:
> [  124.023584] CS:  0010 DS:  ES:  CR0: 80050033
> [  124.023584] CR2: 2ac965af0072 CR3: 000814472000 CR4: 
> 06e0
> [  124.023585] Stack:
> [  124.023588]  8808179dabf8 0048 00c0 
> 8808198b7000
> [  124.023590]  c900085e3910 815dcb5d c900085e3938 
> a040f6a9
> [  124.023592]  8808179dac10 8808198b7368 0601de0a 
> c900085e3990
> [  124.023592] Call Trace:
> [  124.023598]  [] skb_push+0x3d/0x40
> [  124.023607]  [] ipoib_hard_header+0x69/0x90 [ib_ipoib]
> [  124.023611]  [] arp_create+0x2ae/0x3e0
> [  124.023613]  [] arp_send_dst.part.19+0x28/0x50
> [  124.023615]  [] arp_solicit+0x115/0x290
> [  124.023618]  [] ? skb_clone+0x4c/0xa0
> [  124.023619]  [] ? __skb_clone+0x2e/0x140
> [  124.023622]  [] neigh_probe+0x45/0x60
> [  124.023624]  [] __neigh_event_send+0xa7/0x230
> [  124.023625]  [] neigh_resolve_output+0x12e/0x1c0
> [  124.023628]  [] ip_finish_output2+0x14b/0x370
> [  124.023630]  [] ip_finish_output+0x136/0x1e0
> [  124.023632]  [] ip_output+0x6e/0xf0
> [  124.023633]  [] ? __ip_local_out+0x72/0x120
> [  124.023635]  [] ? ip_fragment.constprop.49+0x80/0x80
> [  124.023636]  [] ip_local_out+0x35/0x40
> [  124.023638]  [] ip_send_skb+0x19/0x40
> [  124.023640]  [] ip_push_pending_frames+0x33/0x40
> [  124.023641]  [] raw_sendmsg+0x77a/0xb00
> [  124.023644]  [] ? skb_recv_datagram+0x41/0x60
> [  124.023645]  [] ? raw_recvmsg+0x94/0x1d0
> [  124.023650]  [] ? sock_has_perm+0x70/0x90
> [  124.023653]  [] ? ___sys_recvmsg+0xf2/0x1f0
> [  124.023655]  [] inet_sendmsg+0x67/0xa0
> [  124.023657]  [] sock_sendmsg+0x38/0x50
> [  124.023659]  [] SYSC_sendto+0x102/0x190
> [  124.023662]  [] ? __audit_syscall_entry+0xaf/0x100
> [  124.023665]  [] ? syscall_trace_enter+0x1d0/0x2b0
> [  124.023667]  [] ? __audit_syscall_exit+0x1db/0x260
> [  124.023669]  [] SyS_sendto+0xe/0x10
> [  124.023670]  [] do_syscall_64+0x67/0x180
> [  124.023673]  [] entry_SYSCALL64_slow_path+0x25/0x25
> [  124.023688] Code: 00 00 48 89 44 24 10 8b 87 c8 00 00 00 48 89 44 24 08 48 
> 8b 87 d8 00 00 00 48 c7 c7 50 83 ab 81 48 89 04 24 31 c0 e8 5f e6 a9 ff <0f> 
> 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 0f 1f 44 00 00 55 48
> [  124.023690] RIP  [] skb_panic+0x66/0x68
> [  124.023691]  RSP 
> [  124.023696] ---[ end trace 95c238901cb322be ]---
> [  124.026071] Kernel panic - not syncing: Fatal exception in interrupt
> [  124.026368] Kernel Offset: disabled
> [  124.644414] ---[ end Kernel panic - not syncing: Fatal exception in 
> interrupt
> 
> Fixes: fc791b633515 ('IB/ipoib: move back IB LL address into the hard header')
> Reported-by: Norbert P 
> Signed-off-by: Honggang Li 
> ---
>  drivers/infiniband/ulp/ipoib/ipoib_main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
> b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> index d1d3fb7..3668e1e 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -1161,6 +1161,9 @@ static int ipoib_hard_header(struct sk_buff *skb,
>  {
>   struct ipoib_header *header;
>  
> + if (unlikely(skb_headroom(skb) < IPOIB_HARD_LEN))
> + return -EINVAL;
> +
>   header = (struct ipoib_header *) skb_push(skb, sizeof *header);
>  
>   header->proto = htons(type);
> -- 
> 1.8.3.1

Reviewed-by: Yuval Shaia 

> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] {net,IB}/{rxe,usnic}: Utilize generic mac to eui32 function

[Yuval Shaia]

On Tue, Mar 14, 2017 at 07:58:43PM +0200, Leon Romanovsky wrote:
> On Tue, Mar 14, 2017 at 04:01:57PM +0200, Yuval Shaia wrote:
> > This logic seems to be duplicated in (at least) three separate files.
> > Move it to one place so code can be re-use.
> >
> > Signed-off-by: Yuval Shaia 
> > ---
> > v0 -> v1:
> > * Add missing #include
> > * Rename to genaddrconf_ifid_eui48
> > v1 -> v2:
> > * Reset eui[0] to default if dev_id is used
> > v2 -> v3:
> > * Add helper function to avoid re-setting eui[0] to default if
> >   dev_id is used
> > v3 -> v4:
> > * Remove RXE wrappers
> > * Remove addrconf_addr_eui48_xor and do the eui[0] ^= 2 in the
> >   basic implementation
> > ---
> >  drivers/infiniband/hw/usnic/usnic_common_util.h | 11 +++---
> >  drivers/infiniband/sw/rxe/rxe.c |  4 +++-
> >  drivers/infiniband/sw/rxe/rxe_loc.h |  2 --
> >  drivers/infiniband/sw/rxe/rxe_net.c | 28 
> > -
> >  drivers/infiniband/sw/rxe/rxe_verbs.c   |  4 +++-
> >  include/net/addrconf.h  | 22 +++
> >  6 files changed, 27 insertions(+), 44 deletions(-)
> >
> 
> Thanks, Yuval.
> Reviewed-by: Leon Romanovsky 

Hi Doug,
If no more comments on this one can you consider taking it?

Thanks,
Yuval

[PATCH v4] {net,IB}/{rxe,usnic}: Utilize generic mac to eui32 function

[Yuval Shaia]

This logic seems to be duplicated in (at least) three separate files.
Move it to one place so code can be re-use.

Signed-off-by: Yuval Shaia 
---
v0 -> v1:
* Add missing #include
* Rename to genaddrconf_ifid_eui48
v1 -> v2:
* Reset eui[0] to default if dev_id is used
v2 -> v3:
* Add helper function to avoid re-setting eui[0] to default if
  dev_id is used
v3 -> v4:
* Remove RXE wrappers
* Remove addrconf_addr_eui48_xor and do the eui[0] ^= 2 in the
  basic implementation
---
 drivers/infiniband/hw/usnic/usnic_common_util.h | 11 +++---
 drivers/infiniband/sw/rxe/rxe.c |  4 +++-
 drivers/infiniband/sw/rxe/rxe_loc.h |  2 --
 drivers/infiniband/sw/rxe/rxe_net.c | 28 -
 drivers/infiniband/sw/rxe/rxe_verbs.c   |  4 +++-
 include/net/addrconf.h  | 22 +++
 6 files changed, 27 insertions(+), 44 deletions(-)

diff --git a/drivers/infiniband/hw/usnic/usnic_common_util.h 
b/drivers/infiniband/hw/usnic/usnic_common_util.h
index b54986d..d91b035 100644
--- a/drivers/infiniband/hw/usnic/usnic_common_util.h
+++ b/drivers/infiniband/hw/usnic/usnic_common_util.h
@@ -34,6 +34,8 @@
 #ifndef USNIC_CMN_UTIL_H
 #define USNIC_CMN_UTIL_H
 
+#include 
+
 static inline void
 usnic_mac_to_gid(const char *const mac, char *raw_gid)
 {
@@ -57,14 +59,7 @@ usnic_mac_ip_to_gid(const char *const mac, const __be32 
inaddr, char *raw_gid)
raw_gid[1] = 0x80;
memset(&raw_gid[2], 0, 2);
memcpy(&raw_gid[4], &inaddr, 4);
-   raw_gid[8] = mac[0]^2;
-   raw_gid[9] = mac[1];
-   raw_gid[10] = mac[2];
-   raw_gid[11] = 0xff;
-   raw_gid[12] = 0xfe;
-   raw_gid[13] = mac[3];
-   raw_gid[14] = mac[4];
-   raw_gid[15] = mac[5];
+   addrconf_addr_eui48(&raw_gid[8], mac);
 }
 
 static inline void
diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
index b12dd9b..e93f81f 100644
--- a/drivers/infiniband/sw/rxe/rxe.c
+++ b/drivers/infiniband/sw/rxe/rxe.c
@@ -31,6 +31,7 @@
  * SOFTWARE.
  */
 
+#include 
 #include "rxe.h"
 #include "rxe_loc.h"
 
@@ -178,7 +179,8 @@ static int rxe_init_ports(struct rxe_dev *rxe)
return -ENOMEM;
 
port->pkey_tbl[0] = 0x;
-   port->port_guid = rxe_port_guid(rxe);
+   addrconf_addr_eui48((unsigned char *)&port->port_guid,
+   rxe->ndev->dev_addr);
 
spin_lock_init(&port->port_lock);
 
diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h 
b/drivers/infiniband/sw/rxe/rxe_loc.h
index 272337e..13ed8b5 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -145,7 +145,6 @@ int advance_dma_data(struct rxe_dma_info *dma, unsigned int 
length);
 int rxe_loopback(struct sk_buff *skb);
 int rxe_send(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
 struct sk_buff *skb);
-__be64 rxe_port_guid(struct rxe_dev *rxe);
 struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
int paylen, struct rxe_pkt_info *pkt);
 int rxe_prepare(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
@@ -153,7 +152,6 @@ int rxe_prepare(struct rxe_dev *rxe, struct rxe_pkt_info 
*pkt,
 enum rdma_link_layer rxe_link_layer(struct rxe_dev *rxe, unsigned int 
port_num);
 const char *rxe_parent_name(struct rxe_dev *rxe, unsigned int port_num);
 struct device *rxe_dma_device(struct rxe_dev *rxe);
-__be64 rxe_node_guid(struct rxe_dev *rxe);
 int rxe_mcast_add(struct rxe_dev *rxe, union ib_gid *mgid);
 int rxe_mcast_delete(struct rxe_dev *rxe, union ib_gid *mgid);
 
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c 
b/drivers/infiniband/sw/rxe/rxe_net.c
index d8610960..43b1a0c 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -84,34 +84,6 @@ struct rxe_dev *get_rxe_by_name(const char *name)
 
 struct rxe_recv_sockets recv_sockets;
 
-static __be64 rxe_mac_to_eui64(struct net_device *ndev)
-{
-   unsigned char *mac_addr = ndev->dev_addr;
-   __be64 eui64;
-   unsigned char *dst = (unsigned char *)&eui64;
-
-   dst[0] = mac_addr[0] ^ 2;
-   dst[1] = mac_addr[1];
-   dst[2] = mac_addr[2];
-   dst[3] = 0xff;
-   dst[4] = 0xfe;
-   dst[5] = mac_addr[3];
-   dst[6] = mac_addr[4];
-   dst[7] = mac_addr[5];
-
-   return eui64;
-}
-
-__be64 rxe_node_guid(struct rxe_dev *rxe)
-{
-   return rxe_mac_to_eui64(rxe->ndev);
-}
-
-__be64 rxe_port_guid(struct rxe_dev *rxe)
-{
-   return rxe_mac_to_eui64(rxe->ndev);
-}
-
 struct device *rxe_dma_device(struct rxe_dev *rxe)
 {
struct net_device *ndev;
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c 
b/drivers/infiniband/sw/rxe/rxe_verbs.c
index d2e2eff..09f1bec 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniban

Re: [PATCHv2 3/5] rds: ib: remove redundant ib_dealloc_fmr

[Yuval Shaia]

On Thu, Mar 09, 2017 at 04:20:43AM -0500, Zhu Yanjun wrote:
> The function ib_dealloc_fmr will never be called. As such, it should
> be removed.
> 
> Cc: Joe Jin 
> Cc: Junxiao Bi 
> Signed-off-by: Zhu Yanjun 
> ---
> Change from v1 to v2:
>   remove ibmr NULL test.
> 
>  net/rds/ib_fmr.c | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/net/rds/ib_fmr.c b/net/rds/ib_fmr.c
> index 4fe8f4f..249ae1c 100644
> --- a/net/rds/ib_fmr.c
> +++ b/net/rds/ib_fmr.c
> @@ -78,12 +78,9 @@ struct rds_ib_mr *rds_ib_alloc_fmr(struct rds_ib_device 
> *rds_ibdev, int npages)
>   return ibmr;
>  
>  out_no_cigar:
> - if (ibmr) {
> - if (fmr->fmr)
> - ib_dealloc_fmr(fmr->fmr);
> - kfree(ibmr);
> -     }
> + kfree(ibmr);
>   atomic_dec(&pool->item_count);
> +

Reviewed-by: Yuval Shaia 

>   return ERR_PTR(err);
>  }
>  
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] {net,IB}/{rxe,usnic}: Utilize generic mac to eui32 function

[Yuval Shaia]

On Wed, Mar 08, 2017 at 09:40:41AM +0200, Leon Romanovsky wrote:
> On Tue, Mar 07, 2017 at 09:31:58PM +0200, Yuval Shaia wrote:
> > This logic seems to be duplicated in (at least) three separate files.
> > Move it to one place so code can be re-use.
> >
> > Signed-off-by: Yuval Shaia 
> > ---
> > v0 -> v1:
> > * Add missing #include
> > * Rename to genaddrconf_ifid_eui48
> > v1 -> v2:
> > * Reset eui[0] to default if dev_id is used
> > v2 -> v3:
> > * Add helper function to avoid re-setting eui[0] to default if
> >   dev_id is used
> > ---
> >  drivers/infiniband/hw/usnic/usnic_common_util.h | 11 +++
> >  drivers/infiniband/sw/rxe/rxe_net.c | 11 ++-
> >  include/net/addrconf.h  | 25 
> > +++--
> >  3 files changed, 24 insertions(+), 23 deletions(-)
> 
> Not promising statistics :)

It is not fair, i also added 4 new blank lines :)

> 
> >
> > diff --git a/drivers/infiniband/hw/usnic/usnic_common_util.h 
> > b/drivers/infiniband/hw/usnic/usnic_common_util.h
> > index b54986d..d91b035 100644
> > --- a/drivers/infiniband/hw/usnic/usnic_common_util.h
> > +++ b/drivers/infiniband/hw/usnic/usnic_common_util.h
> > @@ -34,6 +34,8 @@
> >  #ifndef USNIC_CMN_UTIL_H
> >  #define USNIC_CMN_UTIL_H
> >
> > +#include 
> > +
> >  static inline void
> >  usnic_mac_to_gid(const char *const mac, char *raw_gid)
> >  {
> > @@ -57,14 +59,7 @@ usnic_mac_ip_to_gid(const char *const mac, const __be32 
> > inaddr, char *raw_gid)
> > raw_gid[1] = 0x80;
> > memset(&raw_gid[2], 0, 2);
> > memcpy(&raw_gid[4], &inaddr, 4);
> > -   raw_gid[8] = mac[0]^2;
> > -   raw_gid[9] = mac[1];
> > -   raw_gid[10] = mac[2];
> > -   raw_gid[11] = 0xff;
> > -   raw_gid[12] = 0xfe;
> > -   raw_gid[13] = mac[3];
> > -   raw_gid[14] = mac[4];
> > -   raw_gid[15] = mac[5];
> > +   addrconf_addr_eui48(&raw_gid[8], mac);
> >  }
> >
> >  static inline void
> > diff --git a/drivers/infiniband/sw/rxe/rxe_net.c 
> > b/drivers/infiniband/sw/rxe/rxe_net.c
> > index d8610960..ab8ea23 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_net.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> > @@ -38,6 +38,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >
> > @@ -86,18 +87,10 @@ struct rxe_recv_sockets recv_sockets;
> >
> >  static __be64 rxe_mac_to_eui64(struct net_device *ndev)
> >  {
> 
> It is worth to drop this wrapper completely. The rxe_mac_to_eui64 is
> called twice in the same file.

hmmm, this is a classic wrapper to adapt an existing API signature to
usage. And in this case to return __be64 from existing function which
returns void.

What we can do is the opposite - get rid of rxe_node_guid and rxe_port_guid
and call directly to rxe_mac_to_eui64. This is based on the assumption that
node_guid and port_guid are the same in Soft RoCE.
What do you think?

> 
> > -   unsigned char *mac_addr = ndev->dev_addr;
> > __be64 eui64;
> > unsigned char *dst = (unsigned char *)&eui64;
> >
> > -   dst[0] = mac_addr[0] ^ 2;
> > -   dst[1] = mac_addr[1];
> > -   dst[2] = mac_addr[2];
> > -   dst[3] = 0xff;
> > -   dst[4] = 0xfe;
> > -   dst[5] = mac_addr[3];
> > -   dst[6] = mac_addr[4];
> > -   dst[7] = mac_addr[5];
> > +   addrconf_addr_eui48(dst, ndev->dev_addr);
> >
> > return eui64;
> >  }
> > diff --git a/include/net/addrconf.h b/include/net/addrconf.h
> > index 17c6fd8..28274ed 100644
> > --- a/include/net/addrconf.h
> > +++ b/include/net/addrconf.h
> > @@ -103,12 +103,25 @@ int addrconf_prefix_rcv_add_addr(struct net *net, 
> > struct net_device *dev,
> >  u32 addr_flags, bool sllao, bool tokenized,
> >  __u32 valid_lft, u32 prefered_lft);
> >
> > +static inline void addrconf_addr_eui48_xor(u8 *eui, const char *const 
> > addr, bool xor)
> > +{
> > +   memcpy(eui, addr, 3);
> > +   if (xor)
> > +   eui[0] ^= 2;
> > +   eui[3] = 0xFF;
> > +   eui[4] = 0xFE;
> > +   memcpy(eui + 5, addr + 3, 3);
> > +}
> > +
> > +static inline void addrconf_addr_eui48(u8 *eui, const char *const addr)
> > +{
> > +   addrconf_addr_eui48_xor(eui, addr, true);
> 
> Just put your "eui[0] ^= 2" here and remove redundant "if (xor)".
> > +}
> >

[PATCH v3] {net,IB}/{rxe,usnic}: Utilize generic mac to eui32 function

[Yuval Shaia]

This logic seems to be duplicated in (at least) three separate files.
Move it to one place so code can be re-use.

Signed-off-by: Yuval Shaia 
---
v0 -> v1:
* Add missing #include
* Rename to genaddrconf_ifid_eui48
v1 -> v2:
* Reset eui[0] to default if dev_id is used
v2 -> v3:
* Add helper function to avoid re-setting eui[0] to default if
  dev_id is used
---
 drivers/infiniband/hw/usnic/usnic_common_util.h | 11 +++
 drivers/infiniband/sw/rxe/rxe_net.c | 11 ++-
 include/net/addrconf.h  | 25 +++--
 3 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/infiniband/hw/usnic/usnic_common_util.h 
b/drivers/infiniband/hw/usnic/usnic_common_util.h
index b54986d..d91b035 100644
--- a/drivers/infiniband/hw/usnic/usnic_common_util.h
+++ b/drivers/infiniband/hw/usnic/usnic_common_util.h
@@ -34,6 +34,8 @@
 #ifndef USNIC_CMN_UTIL_H
 #define USNIC_CMN_UTIL_H
 
+#include 
+
 static inline void
 usnic_mac_to_gid(const char *const mac, char *raw_gid)
 {
@@ -57,14 +59,7 @@ usnic_mac_ip_to_gid(const char *const mac, const __be32 
inaddr, char *raw_gid)
raw_gid[1] = 0x80;
memset(&raw_gid[2], 0, 2);
memcpy(&raw_gid[4], &inaddr, 4);
-   raw_gid[8] = mac[0]^2;
-   raw_gid[9] = mac[1];
-   raw_gid[10] = mac[2];
-   raw_gid[11] = 0xff;
-   raw_gid[12] = 0xfe;
-   raw_gid[13] = mac[3];
-   raw_gid[14] = mac[4];
-   raw_gid[15] = mac[5];
+   addrconf_addr_eui48(&raw_gid[8], mac);
 }
 
 static inline void
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c 
b/drivers/infiniband/sw/rxe/rxe_net.c
index d8610960..ab8ea23 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -86,18 +87,10 @@ struct rxe_recv_sockets recv_sockets;
 
 static __be64 rxe_mac_to_eui64(struct net_device *ndev)
 {
-   unsigned char *mac_addr = ndev->dev_addr;
__be64 eui64;
unsigned char *dst = (unsigned char *)&eui64;
 
-   dst[0] = mac_addr[0] ^ 2;
-   dst[1] = mac_addr[1];
-   dst[2] = mac_addr[2];
-   dst[3] = 0xff;
-   dst[4] = 0xfe;
-   dst[5] = mac_addr[3];
-   dst[6] = mac_addr[4];
-   dst[7] = mac_addr[5];
+   addrconf_addr_eui48(dst, ndev->dev_addr);
 
return eui64;
 }
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 17c6fd8..28274ed 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -103,12 +103,25 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct 
net_device *dev,
 u32 addr_flags, bool sllao, bool tokenized,
 __u32 valid_lft, u32 prefered_lft);
 
+static inline void addrconf_addr_eui48_xor(u8 *eui, const char *const addr, 
bool xor)
+{
+   memcpy(eui, addr, 3);
+   if (xor)
+   eui[0] ^= 2;
+   eui[3] = 0xFF;
+   eui[4] = 0xFE;
+   memcpy(eui + 5, addr + 3, 3);
+}
+
+static inline void addrconf_addr_eui48(u8 *eui, const char *const addr)
+{
+   addrconf_addr_eui48_xor(eui, addr, true);
+}
+
 static inline int addrconf_ifid_eui48(u8 *eui, struct net_device *dev)
 {
if (dev->addr_len != ETH_ALEN)
return -1;
-   memcpy(eui, dev->dev_addr, 3);
-   memcpy(eui + 5, dev->dev_addr + 3, 3);
 
/*
 * The zSeries OSA network cards can be shared among various
@@ -123,14 +136,14 @@ static inline int addrconf_ifid_eui48(u8 *eui, struct 
net_device *dev)
 * case.  Hence the resulting interface identifier has local
 * scope according to RFC2373.
 */
+
+   addrconf_addr_eui48_xor(eui, dev->dev_addr, !dev->dev_id);
+
if (dev->dev_id) {
eui[3] = (dev->dev_id >> 8) & 0xFF;
eui[4] = dev->dev_id & 0xFF;
-   } else {
-   eui[3] = 0xFF;
-   eui[4] = 0xFE;
-   eui[0] ^= 2;
}
+
return 0;
 }
 
-- 
2.7.4

Re: [PATCH v2] {net,IB}/{rxe,usnic}: Utilize generic mac to eui32 function

[Yuval Shaia]

On Tue, Mar 07, 2017 at 04:27:11PM +0200, Leon Romanovsky wrote:
> On Mon, Mar 06, 2017 at 08:54:06PM +0200, Yuval Shaia wrote:
> > This logic seems to be duplicated in (at least) three separate files.
> > Move it to one place so code can be re-use.
> >
> > Signed-off-by: Yuval Shaia 
> > ---
> > v0 -> v1:
> > * Add missing #include
> > * Rename to genaddrconf_ifid_eui48
> > v1 -> v2:
> > * Reset eui[0] to default if dev_id is used
> > ---
> >  drivers/infiniband/hw/usnic/usnic_common_util.h | 11 +++
> >  drivers/infiniband/sw/rxe/rxe_net.c | 11 ++-
> >  include/net/addrconf.h  | 19 +--
> >  3 files changed, 18 insertions(+), 23 deletions(-)
> >
> >  * scope according to RFC2373.
> >  */
> > if (dev->dev_id) {
> > +   eui[0] = dev->dev_addr[0];
> > eui[3] = (dev->dev_id >> 8) & 0xFF;
> > eui[4] = dev->dev_id & 0xFF;
> > -   } else {
> > -   eui[3] = 0xFF;
> > -   eui[4] = 0xFE;
> > -   eui[0] ^= 2;
> > }
> > +
> > return 0;
> >  }
> 
> Technically, the code is correct now, but it doesn't look right
> to set the value and restore it right after that.

Agree.
I will soon send v3 that offers an alternative by adding new helper
function so this re-assignment can be avoid.

> 
> Thanks
> 
> >
> > --
> > 2.7.4
> >

[PATCH v2] {net,IB}/{rxe,usnic}: Utilize generic mac to eui32 function

[Yuval Shaia]

This logic seems to be duplicated in (at least) three separate files.
Move it to one place so code can be re-use.

Signed-off-by: Yuval Shaia 
---
v0 -> v1:
* Add missing #include
* Rename to genaddrconf_ifid_eui48
v1 -> v2:
* Reset eui[0] to default if dev_id is used
---
 drivers/infiniband/hw/usnic/usnic_common_util.h | 11 +++
 drivers/infiniband/sw/rxe/rxe_net.c | 11 ++-
 include/net/addrconf.h  | 19 +--
 3 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/infiniband/hw/usnic/usnic_common_util.h 
b/drivers/infiniband/hw/usnic/usnic_common_util.h
index b54986d..09871da 100644
--- a/drivers/infiniband/hw/usnic/usnic_common_util.h
+++ b/drivers/infiniband/hw/usnic/usnic_common_util.h
@@ -34,6 +34,8 @@
 #ifndef USNIC_CMN_UTIL_H
 #define USNIC_CMN_UTIL_H
 
+#include 
+
 static inline void
 usnic_mac_to_gid(const char *const mac, char *raw_gid)
 {
@@ -57,14 +59,7 @@ usnic_mac_ip_to_gid(const char *const mac, const __be32 
inaddr, char *raw_gid)
raw_gid[1] = 0x80;
memset(&raw_gid[2], 0, 2);
memcpy(&raw_gid[4], &inaddr, 4);
-   raw_gid[8] = mac[0]^2;
-   raw_gid[9] = mac[1];
-   raw_gid[10] = mac[2];
-   raw_gid[11] = 0xff;
-   raw_gid[12] = 0xfe;
-   raw_gid[13] = mac[3];
-   raw_gid[14] = mac[4];
-   raw_gid[15] = mac[5];
+   genaddrconf_ifid_eui48(&raw_gid[8], mac);
 }
 
 static inline void
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c 
b/drivers/infiniband/sw/rxe/rxe_net.c
index d8610960..90285c8 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -86,18 +87,10 @@ struct rxe_recv_sockets recv_sockets;
 
 static __be64 rxe_mac_to_eui64(struct net_device *ndev)
 {
-   unsigned char *mac_addr = ndev->dev_addr;
__be64 eui64;
unsigned char *dst = (unsigned char *)&eui64;
 
-   dst[0] = mac_addr[0] ^ 2;
-   dst[1] = mac_addr[1];
-   dst[2] = mac_addr[2];
-   dst[3] = 0xff;
-   dst[4] = 0xfe;
-   dst[5] = mac_addr[3];
-   dst[6] = mac_addr[4];
-   dst[7] = mac_addr[5];
+   genaddrconf_ifid_eui48(dst, ndev->dev_addr);
 
return eui64;
 }
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 17c6fd8..cdfa73f 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -103,12 +103,21 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct 
net_device *dev,
 u32 addr_flags, bool sllao, bool tokenized,
 __u32 valid_lft, u32 prefered_lft);
 
+static inline void genaddrconf_ifid_eui48(u8 *eui, const char *const addr)
+{
+   memcpy(eui, addr, 3);
+   eui[0] ^= 2;
+   eui[3] = 0xFF;
+   eui[4] = 0xFE;
+   memcpy(eui + 5, addr + 3, 3);
+}
+
 static inline int addrconf_ifid_eui48(u8 *eui, struct net_device *dev)
 {
if (dev->addr_len != ETH_ALEN)
return -1;
-   memcpy(eui, dev->dev_addr, 3);
-   memcpy(eui + 5, dev->dev_addr + 3, 3);
+
+   genaddrconf_ifid_eui48(eui, dev->dev_addr);
 
/*
 * The zSeries OSA network cards can be shared among various
@@ -124,13 +133,11 @@ static inline int addrconf_ifid_eui48(u8 *eui, struct 
net_device *dev)
 * scope according to RFC2373.
 */
if (dev->dev_id) {
+   eui[0] = dev->dev_addr[0];
eui[3] = (dev->dev_id >> 8) & 0xFF;
eui[4] = dev->dev_id & 0xFF;
-   } else {
-   eui[3] = 0xFF;
-   eui[4] = 0xFE;
-   eui[0] ^= 2;
}
+
return 0;
 }
 
-- 
2.7.4

Re: [PATCH 1/1] rds: remove unnecessary returned value check

[Yuval Shaia]

On Fri, Mar 03, 2017 at 12:44:26AM -0500, Zhu Yanjun wrote:
> The function rds_trans_register always returns 0. As such, it is not
> necessary to check the returned value.
> 
> Cc: Joe Jin 
> Cc: Junxiao Bi 
> Signed-off-by: Zhu Yanjun 
> ---
>  net/rds/ib.c| 6 +-
>  net/rds/rds.h   | 2 +-
>  net/rds/tcp.c   | 6 +-
>  net/rds/transport.c | 4 +---
>  4 files changed, 4 insertions(+), 14 deletions(-)
> 
> diff --git a/net/rds/ib.c b/net/rds/ib.c
> index 0f557b2..7a64c8d 100644
> --- a/net/rds/ib.c
> +++ b/net/rds/ib.c
> @@ -438,16 +438,12 @@ int rds_ib_init(void)
>   if (ret)
>   goto out_sysctl;
>  
> - ret = rds_trans_register(&rds_ib_transport);
> - if (ret)
> - goto out_recv;
> + rds_trans_register(&rds_ib_transport);
>  
>   rds_info_register_func(RDS_INFO_IB_CONNECTIONS, rds_ib_ic_info);
>  
>   goto out;
>  
> -out_recv:
> - rds_ib_recv_exit();
>  out_sysctl:
>   rds_ib_sysctl_exit();
>  out_ibreg:
> diff --git a/net/rds/rds.h b/net/rds/rds.h
> index 966d2ee..39518ef 100644
> --- a/net/rds/rds.h
> +++ b/net/rds/rds.h
> @@ -903,7 +903,7 @@ void rds_connect_path_complete(struct rds_conn_path 
> *conn, int curr);
>  void rds_connect_complete(struct rds_connection *conn);
>  
>  /* transport.c */
> -int rds_trans_register(struct rds_transport *trans);
> +void rds_trans_register(struct rds_transport *trans);
>  void rds_trans_unregister(struct rds_transport *trans);
>  struct rds_transport *rds_trans_get_preferred(struct net *net, __be32 addr);
>  void rds_trans_put(struct rds_transport *trans);
> diff --git a/net/rds/tcp.c b/net/rds/tcp.c
> index 5438f67..a973d3b 100644
> --- a/net/rds/tcp.c
> +++ b/net/rds/tcp.c
> @@ -652,16 +652,12 @@ static int rds_tcp_init(void)
>   if (ret)
>   goto out_pernet;
>  
> - ret = rds_trans_register(&rds_tcp_transport);
> - if (ret)
> - goto out_recv;
> + rds_trans_register(&rds_tcp_transport);
>  
>   rds_info_register_func(RDS_INFO_TCP_SOCKETS, rds_tcp_tc_info);
>  
>   goto out;
>  
> -out_recv:
> - rds_tcp_recv_exit();
>  out_pernet:
>   unregister_pernet_subsys(&rds_tcp_net_ops);
>  out_notifier:
> diff --git a/net/rds/transport.c b/net/rds/transport.c
> index 2ffd3e3..0b188dd 100644
> --- a/net/rds/transport.c
> +++ b/net/rds/transport.c
> @@ -40,7 +40,7 @@
>  static struct rds_transport *transports[RDS_TRANS_COUNT];
>  static DECLARE_RWSEM(rds_trans_sem);
>  
> -int rds_trans_register(struct rds_transport *trans)
> +void rds_trans_register(struct rds_transport *trans)
>  {
>   BUG_ON(strlen(trans->t_name) + 1 > TRANSNAMSIZ);
>  
> @@ -55,8 +55,6 @@ int rds_trans_register(struct rds_transport *trans)
>   }
>  
>   up_write(&rds_trans_sem);
> -
> - return 0;
>  }
>  EXPORT_SYMBOL_GPL(rds_trans_register);

Reviewed-by: Yuval Shaia 

>  
> -- 
> 2.7.4
> 

Re: [PATCH 1/1] rds: ib: add the static type to the variables

[Yuval Shaia]

On Tue, Feb 28, 2017 at 01:45:40AM -0500, Zhu Yanjun wrote:
> The variables rds_ib_mr_1m_pool_size and rds_ib_mr_8k_pool_size
> are used only in the ib.c file. As such, the static type is
> added to limit them in this file.
> 
> Cc: Joe Jin 
> Cc: Junxiao Bi 
> Signed-off-by: Zhu Yanjun 
> ---
>  net/rds/ib.c| 4 ++--
>  net/rds/ib_mr.h | 2 --
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/net/rds/ib.c b/net/rds/ib.c
> index 8d70884..ecff01b 100644
> --- a/net/rds/ib.c
> +++ b/net/rds/ib.c
> @@ -45,8 +45,8 @@
>  #include "ib.h"
>  #include "ib_mr.h"
>  
> -unsigned int rds_ib_mr_1m_pool_size = RDS_MR_1M_POOL_SIZE;
> -unsigned int rds_ib_mr_8k_pool_size = RDS_MR_8K_POOL_SIZE;
> +static unsigned int rds_ib_mr_1m_pool_size = RDS_MR_1M_POOL_SIZE;
> +static unsigned int rds_ib_mr_8k_pool_size = RDS_MR_8K_POOL_SIZE;
>  unsigned int rds_ib_retry_count = RDS_IB_DEFAULT_RETRY_COUNT;
>  
>  module_param(rds_ib_mr_1m_pool_size, int, 0444);
> diff --git a/net/rds/ib_mr.h b/net/rds/ib_mr.h
> index 24c086d..5d6e98a 100644
> --- a/net/rds/ib_mr.h
> +++ b/net/rds/ib_mr.h
> @@ -107,8 +107,6 @@ struct rds_ib_mr_pool {
>  };
>  
>  extern struct workqueue_struct *rds_ib_mr_wq;
> -extern unsigned int rds_ib_mr_1m_pool_size;
> -extern unsigned int rds_ib_mr_8k_pool_size;
>  extern bool prefer_frmr;

Reviewed-by: Yuval Shaia 

>  
>  struct rds_ib_mr_pool *rds_ib_create_mr_pool(struct rds_ib_device *rds_dev,
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net 1/3] qed: Release CQ resource under lock on failure

[Yuval Shaia]

On Tue, Jan 24, 2017 at 11:15:21PM +0200, Yuval Mintz wrote:
> From: Ram Amrani 
> 
> The CQ resource pool is protected by a spin lock. When a CQ creation
> fails it now deallocates under that lock as well.
> 
> Signed-off-by: Ram Amrani 
> Signed-off-by: Yuval Mintz 
> ---
>  drivers/net/ethernet/qlogic/qed/qed_roce.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_roce.c 
> b/drivers/net/ethernet/qlogic/qed/qed_roce.c
> index bd4cad2..7ab6d4e 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_roce.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_roce.c
> @@ -948,7 +948,9 @@ static int qed_rdma_create_cq(void *rdma_cxt,
>  
>  err:
>   /* release allocated icid */
> + spin_lock_bh(&p_info->lock);
>   qed_bmap_release_id(p_hwfn, &p_info->cq_map, returned_id);
> + spin_unlock_bh(&p_info->lock);
>   DP_NOTICE(p_hwfn, "Create CQ failed, rc = %d\n", rc);

Minor suggestion.
Can you consider embedding the lock and unlock inside qed_bmap_release_id?
There are two places where this is bad idea as driver needs to release two
IDs but still one is in error flow and second is when destroying QP so for
most cases code may look a bit better.

>  
>   return rc;
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [for-next V2 06/10] net/mlx5: Add interface to get reference to a UAR

[Yuval Shaia]

On Sun, Jan 08, 2017 at 05:54:47PM +0200, Saeed Mahameed wrote:
> From: Eli Cohen 
> 
> A reference to a UAR is required to generate CQ or EQ doorbells. Since
> CQ or EQ doorbells can all be generated using the same UAR area without
> any effect on performance, we are just getting a reference to any
> available UAR, If one is not available we allocate it but we don't waste
> the blue flame registers it can provide and we will use them for
> subsequent allocations.
> We get a reference to such UAR and put in mlx5_priv so any kernel
> consumer can make use of it.
> 
> Signed-off-by: Eli Cohen 
> Reviewed-by: Matan Barak 
> Signed-off-by: Leon Romanovsky 
> Signed-off-by: Saeed Mahameed 
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/eq.c   | 14 ---
>  drivers/net/ethernet/mellanox/mlx5/core/main.c | 22 ++
>  drivers/net/ethernet/mellanox/mlx5/core/uar.c  | 32 
> ++
>  include/linux/mlx5/driver.h|  5 +++-
>  4 files changed, 59 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eq.c 
> b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
> index 11a8d63..9849ee9 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eq.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
> @@ -512,7 +512,7 @@ static void init_eq_buf(struct mlx5_eq *eq)
>  
>  int mlx5_create_map_eq(struct mlx5_core_dev *dev, struct mlx5_eq *eq, u8 
> vecidx,
>  int nent, u64 mask, const char *name,
> -struct mlx5_uar *uar, enum mlx5_eq_type type)
> +enum mlx5_eq_type type)
>  {
>   u32 out[MLX5_ST_SZ_DW(create_eq_out)] = {0};
>   struct mlx5_priv *priv = &dev->priv;
> @@ -556,7 +556,7 @@ int mlx5_create_map_eq(struct mlx5_core_dev *dev, struct 
> mlx5_eq *eq, u8 vecidx,
>  
>   eqc = MLX5_ADDR_OF(create_eq_in, in, eq_context_entry);
>   MLX5_SET(eqc, eqc, log_eq_size, ilog2(eq->nent));
> - MLX5_SET(eqc, eqc, uar_page, uar->index);
> + MLX5_SET(eqc, eqc, uar_page, priv->uar->index);
>   MLX5_SET(eqc, eqc, intr, vecidx);
>   MLX5_SET(eqc, eqc, log_page_size,
>eq->buf.page_shift - MLX5_ADAPTER_PAGE_SHIFT);
> @@ -571,7 +571,7 @@ int mlx5_create_map_eq(struct mlx5_core_dev *dev, struct 
> mlx5_eq *eq, u8 vecidx,
>   eq->eqn = MLX5_GET(create_eq_out, out, eq_number);
>   eq->irqn = priv->msix_arr[vecidx].vector;
>   eq->dev = dev;
> - eq->doorbell = uar->map + MLX5_EQ_DOORBEL_OFFSET;
> + eq->doorbell = priv->uar->map + MLX5_EQ_DOORBEL_OFFSET;
>   err = request_irq(eq->irqn, handler, 0,
> priv->irq_info[vecidx].name, eq);
>   if (err)
> @@ -686,8 +686,7 @@ int mlx5_start_eqs(struct mlx5_core_dev *dev)
>  
>   err = mlx5_create_map_eq(dev, &table->cmd_eq, MLX5_EQ_VEC_CMD,
>MLX5_NUM_CMD_EQE, 1ull << MLX5_EVENT_TYPE_CMD,
> -  "mlx5_cmd_eq", &dev->priv.bfregi.uars[0],
> -  MLX5_EQ_TYPE_ASYNC);
> +  "mlx5_cmd_eq",  MLX5_EQ_TYPE_ASYNC);

Remove extra space

>   if (err) {
>   mlx5_core_warn(dev, "failed to create cmd EQ %d\n", err);
>   return err;
> @@ -697,8 +696,7 @@ int mlx5_start_eqs(struct mlx5_core_dev *dev)
>  
>   err = mlx5_create_map_eq(dev, &table->async_eq, MLX5_EQ_VEC_ASYNC,
>MLX5_NUM_ASYNC_EQE, async_event_mask,
> -  "mlx5_async_eq", &dev->priv.bfregi.uars[0],
> -  MLX5_EQ_TYPE_ASYNC);
> +  "mlx5_async_eq", MLX5_EQ_TYPE_ASYNC);
>   if (err) {
>   mlx5_core_warn(dev, "failed to create async EQ %d\n", err);
>   goto err1;
> @@ -708,7 +706,6 @@ int mlx5_start_eqs(struct mlx5_core_dev *dev)
>MLX5_EQ_VEC_PAGES,
>/* TODO: sriov max_vf + */ 1,
>1 << MLX5_EVENT_TYPE_PAGE_REQUEST, 
> "mlx5_pages_eq",
> -  &dev->priv.bfregi.uars[0],
>MLX5_EQ_TYPE_ASYNC);
>   if (err) {
>   mlx5_core_warn(dev, "failed to create pages EQ %d\n", err);
> @@ -722,7 +719,6 @@ int mlx5_start_eqs(struct mlx5_core_dev *dev)
>MLX5_NUM_ASYNC_EQE,
>1 << MLX5_EVENT_TYPE_PAGE_FAULT,
>"mlx5_page_fault_eq",
> -  &dev->priv.bfregi.uars[0],
>MLX5_EQ_TYPE_PF);
>   if (err) {
>   mlx5_core_warn(dev, "failed to create page fault EQ 
> %d\n",
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c 
> b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> index 634e96a..2882d04 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> +++ b

Re: [PATCH] net: return value of skb_linearize should be handled in Linux kernel

[Yuval Shaia]

On Tue, Dec 06, 2016 at 03:10:33PM +0800, Zhouyi Zhou wrote:
> kmalloc_reserve may fail to allocate memory inside skb_linearize, 
> which means skb_linearize's return value should not be ignored. 
> Following patch correct the uses of skb_linearize.
> 
> Compiled in x86_64

FWIW compiled also on SPARC

Reviewed-by: Yuval Shaia 

> 
> Signed-off-by: Zhouyi Zhou 
> ---
>  drivers/infiniband/hw/nes/nes_nic.c   | 5 +++--
>  drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +--
>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 7 +--
>  drivers/scsi/fcoe/fcoe.c  | 5 -
>  net/tipc/link.c   | 3 ++-
>  net/tipc/name_distr.c | 5 -
>  7 files changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/nes/nes_nic.c 
> b/drivers/infiniband/hw/nes/nes_nic.c
> index 2b27d13..69372ea 100644
> --- a/drivers/infiniband/hw/nes/nes_nic.c
> +++ b/drivers/infiniband/hw/nes/nes_nic.c
> @@ -662,10 +662,11 @@ static int nes_netdev_start_xmit(struct sk_buff *skb, 
> struct net_device *netdev)
>   nesnic->sq_head &= nesnic->sq_size-1;
>   }
>   } else {
> - nesvnic->linearized_skbs++;
>   hoffset = skb_transport_header(skb) - skb->data;
>   nhoffset = skb_network_header(skb) - skb->data;
> - skb_linearize(skb);
> + if (skb_linearize(skb))
> + return NETDEV_TX_BUSY;
> + nesvnic->linearized_skbs++;
>   skb_set_transport_header(skb, hoffset);
>   skb_set_network_header(skb, nhoffset);
>   if (!nes_nic_send(skb, netdev))
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c 
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> index 2a653ec..ab787cb 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> @@ -490,7 +490,11 @@ int ixgbe_fcoe_ddp(struct ixgbe_adapter *adapter,
>*/
>   if ((fh->fh_r_ctl == FC_RCTL_DD_SOL_DATA) &&
>   (fctl & FC_FC_END_SEQ)) {
> - skb_linearize(skb);
> + int err = 0;
> +
> + err = skb_linearize(skb);
> + if (err)
> + return err;
>   crc = (struct fcoe_crc_eof *)skb_put(skb, sizeof(*crc));
>   crc->fcoe_eof = FC_EOF_T;
>   }
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c 
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index fee1f29..4926d48 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -2173,8 +2173,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector 
> *q_vector,
>   total_rx_bytes += ddp_bytes;
>   total_rx_packets += DIV_ROUND_UP(ddp_bytes,
>mss);
> - }
> - if (!ddp_bytes) {
> + } else {
>   dev_kfree_skb_any(skb);
>   continue;
>   }
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c 
> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> index f9ddb61..197d02e 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> @@ -542,8 +542,11 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
>   return;
>   }
>  
> - if (skb_is_nonlinear(skb))
> - skb_linearize(skb);
> + if (skb_linearize(skb)) {
> + kfree_skb(skb);
> + return;
> + }
> +
>   mac = eth_hdr(skb)->h_source;
>   dest_mac = eth_hdr(skb)->h_dest;
>  
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index 9bd41a3..f691b97 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -1685,7 +1685,10 @@ static void fcoe_recv_frame(struct sk_buff *skb)
>   skb->dev ? skb->dev->name : "");
>  
>   port = lport_priv(lport);
> - skb_linearize(skb); /* check for skb_is_nonlinear is within 
> skb_linearize */
> + if (skb_linearize(skb)) {
> + kfree_skb(skb);
> + return;
> + }
>  
>   /*
>* Frame length checks and setting up the header pointers
> diff --git a/net/tipc/link.c b/net/tipc/link.c
> ind

Re: [PATCH v3] ethernet :mellanox :mlx4: Replace pci_pool_alloc by pci_pool_zalloc

[Yuval Shaia]

On Wed, Nov 30, 2016 at 01:16:12AM +0530, Souptick Joarder wrote:
> In mlx4_alloc_cmd_mailbox(), pci_pool_alloc() followed by memset will be
> replaced by pci_pool_zalloc()
> 
> Signed-off-by: Souptick joarder 
> ---
> v3:
>   - Fixed alignment issues

You mean remove extra empty line?

Reviewed-by: Yuval Shaia 

> 
> v2:
>   - Address comment from sergei
> Alignment was not proper
> 
>  drivers/net/ethernet/mellanox/mlx4/cmd.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c 
> b/drivers/net/ethernet/mellanox/mlx4/cmd.c
> index e36bebc..a49072b4 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
> @@ -2679,15 +2679,13 @@ struct mlx4_cmd_mailbox 
> *mlx4_alloc_cmd_mailbox(struct mlx4_dev *dev)
>   if (!mailbox)
>   return ERR_PTR(-ENOMEM);
> 
> - mailbox->buf = pci_pool_alloc(mlx4_priv(dev)->cmd.pool, GFP_KERNEL,
> -   &mailbox->dma);
> + mailbox->buf = pci_pool_zalloc(mlx4_priv(dev)->cmd.pool, GFP_KERNEL,
> +&mailbox->dma);
>   if (!mailbox->buf) {
>   kfree(mailbox);
>   return ERR_PTR(-ENOMEM);
>   }
> 
> - memset(mailbox->buf, 0, MLX4_MAILBOX_SIZE);
> -
>   return mailbox;
>  }
>  EXPORT_SYMBOL_GPL(mlx4_alloc_cmd_mailbox);
> --
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 13/16] IB/pvrdma: Add the main driver module for PVRDMA

[Yuval Shaia]

On Sat, Sep 24, 2016 at 04:21:37PM -0700, Adit Ranadive wrote:
> +
> + /* Currently, the driver only supports RoCE mode. */
> + if (dev->dsr->caps.mode != PVRDMA_DEVICE_MODE_ROCE) {
> + dev_err(&pdev->dev, "unsupported transport %d\n",
> + dev->dsr->caps.mode);
> + ret = -EINVAL;

This is some fatal error with the device, not that something wrong with the
function's argument.
Suggesting to replace with -EFAULT.

> + goto err_free_cq_ring;
> + }
> +
> + /* Currently, the driver only supports RoCE V1. */
> + if (!(dev->dsr->caps.gid_types & PVRDMA_GID_TYPE_FLAG_ROCE_V1)) {
> + dev_err(&pdev->dev, "driver needs RoCE v1 support\n");
> + ret = -EINVAL;

Ditto.

> + goto err_free_cq_ring;
> + }
> +
> + /* Paired vmxnet3 will have same bus, slot. But func will be 0 */
> + pdev_net = pci_get_slot(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 0));
> + if (!pdev_net) {
> + dev_err(&pdev->dev, "failed to find paired net device\n");
> + ret = -ENODEV;
> + goto err_free_cq_ring;
> + }
> +
> + if (pdev_net->vendor != PCI_VENDOR_ID_VMWARE ||
> + pdev_net->device != PCI_DEVICE_ID_VMWARE_VMXNET3) {
> + dev_err(&pdev->dev, "failed to find paired vmxnet3 device\n");
> + pci_dev_put(pdev_net);
> + ret = -ENODEV;
> + goto err_free_cq_ring;
> + }
> +
> + dev->netdev = pci_get_drvdata(pdev_net);
> + pci_dev_put(pdev_net);
> + if (!dev->netdev) {
> + dev_err(&pdev->dev, "failed to get vmxnet3 device\n");
> + ret = -ENODEV;
> + goto err_free_cq_ring;
> + }
> +
> + dev_info(&pdev->dev, "paired device to %s\n", dev->netdev->name);
> +
> + /* Interrupt setup */
> + ret = pvrdma_alloc_intrs(dev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to allocate interrupts\n");
> + ret = -ENOMEM;
> + goto err_netdevice;
> + }
> +
> + /* Allocate UAR table. */
> + ret = pvrdma_uar_table_init(dev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to allocate UAR table\n");
> + ret = -ENOMEM;
> + goto err_free_intrs;
> + }
> +
> + /* Allocate GID table */
> + dev->sgid_tbl = kcalloc(dev->dsr->caps.gid_tbl_len,
> + sizeof(union ib_gid), GFP_KERNEL);
> + if (!dev->sgid_tbl) {
> + ret = -ENOMEM;
> + goto err_free_uar_table;
> + }
> + dev_dbg(&pdev->dev, "gid table len %d\n", dev->dsr->caps.gid_tbl_len);
> +
> + pvrdma_enable_intrs(dev);
> +
> + /* Activate pvrdma device */
> + pvrdma_write_reg(dev, PVRDMA_REG_CTL, PVRDMA_DEVICE_CTL_ACTIVATE);
> +
> + /* Make sure the write is complete before reading status. */
> + mb();
> +
> + /* Check if device was successfully activated */
> + ret = pvrdma_read_reg(dev, PVRDMA_REG_ERR);
> + if (ret != 0) {
> + dev_err(&pdev->dev, "failed to activate device\n");
> + ret = -EINVAL;

Suggesting -EFAULT.

> + goto err_disable_intr;
> + }
> +
> + /* Register IB device */
> + ret = pvrdma_register_device(dev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register IB device\n");
> + goto err_disable_intr;
> + }
> +
> + dev->nb_netdev.notifier_call = pvrdma_netdevice_event;
> + ret = register_netdevice_notifier(&dev->nb_netdev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register netdevice events\n");
> + goto err_unreg_ibdev;
> + }
> +
> + dev_info(&pdev->dev, "attached to device\n");
> + return 0;
> +
> +err_unreg_ibdev:
> + ib_unregister_device(&dev->ib_dev);
> +err_disable_intr:
> + pvrdma_disable_intrs(dev);
> + kfree(dev->sgid_tbl);
> +err_free_uar_table:
> + pvrdma_uar_table_cleanup(dev);
> +err_free_intrs:
> + pvrdma_free_irq(dev);
> + pvrdma_disable_msi_all(dev);
> +err_netdevice:
> + unregister_netdevice_notifier(&dev->nb_netdev);
> +err_free_cq_ring:
> + pvrdma_page_dir_cleanup(dev, &dev->cq_pdir);
> +err_free_async_ring:
> + pvrdma_page_dir_cleanup(dev, &dev->async_pdir);
> +err_free_slots:
> + pvrdma_free_slots(dev);
> +err_free_dsr:
> + dma_free_coherent(&pdev->dev, sizeof(*dev->dsr), dev->dsr,
> +   dev->dsrbase);
> +err_uar_unmap:
> + iounmap(dev->driver_uar.map);
> +err_unmap_regs:
> + iounmap(dev->regs);
> +err_free_resource:
> + pci_release_regions(pdev);
> +err_disable_pdev:
> + pci_disable_device(pdev);
> + pci_set_drvdata(pdev, NULL);
> +err_free_device:
> + mutex_lock(&pvrdma_device_list_lock);
> + list_del(&dev->device_link);
> + mutex_unlock(&pvrdma_device_list_lock);
> + ib_dealloc_device(&dev->ib_dev);
> + return ret;
> +}

Re: [PATCH v5 08/16] IB/pvrdma: Add device command support

[Yuval Shaia]

Minor question/suggestion inline.
(sorry for missing it till now).

Yuval

On Sat, Sep 24, 2016 at 04:21:32PM -0700, Adit Ranadive wrote:
> This patch enables posting Verb requests and receiving responses to/from
> the backend PVRDMA emulation layer.
> 
> Reviewed-by: Yuval Shaia 
> Reviewed-by: Jorgen Hansen 
> Reviewed-by: George Zhang 
> Reviewed-by: Aditya Sarwade 
> Reviewed-by: Bryan Tan 
> Signed-off-by: Adit Ranadive 
> ---
> Changes v4->v5:
>  - Moved the timeout to pvrdma_cmd_recv.
>  - Added additional response code parameter to pvrdma_cmd_post.
> 
> Changes v3->v4:
>  - Removed the min check and added a BUILD_BUG_ON check for size.
> 
> Changes v2->v3:
>  - Converted pvrdma_cmd_recv to inline.
>  - Added a min check in the memcpy to cmd_slot.
>  - Removed the boolean from pvrdma_cmd_post.
> ---
>  drivers/infiniband/hw/pvrdma/pvrdma_cmd.c | 117 
> ++
>  1 file changed, 117 insertions(+)
>  create mode 100644 drivers/infiniband/hw/pvrdma/pvrdma_cmd.c
> 
> diff --git a/drivers/infiniband/hw/pvrdma/pvrdma_cmd.c 
> b/drivers/infiniband/hw/pvrdma/pvrdma_cmd.c
> new file mode 100644
> index 000..21f1af8
> --- /dev/null
> +++ b/drivers/infiniband/hw/pvrdma/pvrdma_cmd.c
> @@ -0,0 +1,117 @@
> +/*
> + * Copyright (c) 2012-2016 VMware, Inc.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of EITHER the GNU General Public License
> + * version 2 as published by the Free Software Foundation or the BSD
> + * 2-Clause License. This program is distributed in the hope that it
> + * will be useful, but WITHOUT ANY WARRANTY; WITHOUT EVEN THE IMPLIED
> + * WARRANTY OF MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE.
> + * See the GNU General Public License version 2 for more details at
> + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program available in the file COPYING in the main
> + * directory of this source tree.
> + *
> + * The BSD 2-Clause License
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + *  - Redistributions of source code must retain the above
> + *copyright notice, this list of conditions and the following
> + *disclaimer.
> + *
> + *  - Redistributions in binary form must reproduce the above
> + *copyright notice, this list of conditions and the following
> + *disclaimer in the documentation and/or other materials
> + *provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> + * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
> + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
> + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
> + * OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include 
> +
> +#include "pvrdma.h"
> +
> +#define PVRDMA_CMD_TIMEOUT   1 /* ms */
> +
> +static inline int pvrdma_cmd_recv(struct pvrdma_dev *dev,
> +   union pvrdma_cmd_resp *resp,
> +   unsigned resp_code)
> +{
> + int err;
> +
> + dev_dbg(&dev->pdev->dev, "receive response from device\n");
> +
> + err = wait_for_completion_interruptible_timeout(&dev->cmd_done,
> + msecs_to_jiffies(PVRDMA_CMD_TIMEOUT));
> + if (err == 0 || err == -ERESTARTSYS) {
> + dev_warn(&dev->pdev->dev,
> +  "completion timeout or interrupted\n");
> + return -ETIMEDOUT;
> + }
> +
> + spin_lock(&dev->cmd_lock);
> + memcpy(resp, dev->resp_slot, sizeof(*resp));
> + spin_unlock(&dev->cmd_lock);
> +
> + if (resp->hdr.ack != resp_code) {
> + dev_warn(&dev->pdev->dev,
> +  "unknown response %#x expected %#x\n",
&g

Re: [PATCH v4 01/16] vmxnet3: Move PCI Id to pci_ids.h

[Yuval Shaia]

Besides that no more comments.

Reviewed-by: Yuval Shaia 

On Wed, Sep 14, 2016 at 07:36:34PM +, Adit Ranadive wrote:
> On Wed, Sep 14, 2016 at 09:25:18 -0700, Yuval Shaia wrote:
> > On Wed, Sep 14, 2016 at 04:00:25PM +, Adit Ranadive wrote:
> > > On Wed, Sep 14, 2016 at 04:09:12 -0700, Yuval Shaia wrote:
> > > > Please update vmxnet3_drv.c accordingly.
> > >
> > > Any reason why? I don't think we need to. Vmxnet3 should just pick up
> > > the moved PCI device id from pci_ids.h file.
> > 
> > So now you need to include it from vmxnet3_drv.c.
> > Same with pvrdma_main.c
> 
> If you're asking me to include pci_ids.h in our drivers we already do that
> by including pci.h in both the drivers. 
> pci.h already includes pci_ids.h - 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/pci.h#n35
> 
> If that's going to change maybe someone from the PCI group can comment on.
> 
> Thanks,
> Adit
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 09/16] IB/pvrdma: Add support for Completion Queues

[Yuval Shaia]

Hi Adit,
Please see my comments inline.

Besides that I have no more comment for this patch.

Reviewed-by: Yuval Shaia 

Yuval

On Thu, Sep 15, 2016 at 12:07:29AM +, Adit Ranadive wrote:
> On Wed, Sep 14, 2016 at 05:43:37 -0700, Yuval Shaia wrote:
> > On Sun, Sep 11, 2016 at 09:49:19PM -0700, Adit Ranadive wrote:
> > > +
> > > +static int pvrdma_poll_one(struct pvrdma_cq *cq, struct pvrdma_qp
> > **cur_qp,
> > > +struct ib_wc *wc)
> > > +{
> > > + struct pvrdma_dev *dev = to_vdev(cq->ibcq.device);
> > > + int has_data;
> > > + unsigned int head;
> > > + bool tried = false;
> > > + struct pvrdma_cqe *cqe;
> > > +
> > > +retry:
> > > + has_data = pvrdma_idx_ring_has_data(&cq->ring_state->rx,
> > > + cq->ibcq.cqe, &head);
> > > + if (has_data == 0) {
> > > + if (tried)
> > > + return -EAGAIN;
> > > +
> > > + /* Pass down POLL to give physical HCA a chance to poll. */
> > > + pvrdma_write_uar_cq(dev, cq->cq_handle |
> > PVRDMA_UAR_CQ_POLL);
> > > +
> > > + tried = true;
> > > + goto retry;
> > > + } else if (has_data == PVRDMA_INVALID_IDX) {
> > 
> > I didn't went throw the entire life cycle of RX-ring's head and tail but you
> > need to make sure that PVRDMA_INVALID_IDX error is recoverable one, i.e
> > there is probability that in the next call to pvrdma_poll_one it will be 
> > fine.
> > Otherwise it is an endless loop.
> 
> We have never run into this issue internally but I don't think we can recover 
> here 

I briefly reviewed the life cycle of RX-ring's head and tail and didn't
caught any suspicious place that might corrupt it.
So glad to see that you never encountered this case.

> in the driver. The only way to recover would be to destroy and recreate the 
> CQ 
> which we shouldn't do since it could be used by multiple QPs. 

Agree.
But don't they hit the same problem too?

> We don't have a way yet to recover in the device. Once we add that this check
> should go away.

To be honest i have no idea how to do that - i was expecting driver's vendors
to come up with an ideas :)
I once came up with an idea to force restart of the driver but it was
rejected.

> 
> The reason I returned an error value from poll_cq in v3 was to break the 
> possible 
> loop so that it might give clients a chance to recover. But since poll_cq is 
> not expected
> to fail I just log the device error here. I can revert to that version if you 
> want to break
> the possible loop.

Clients (ULPs) cannot recover from this case. They even do not check the
reason of the error and treats any error as -EAGAIN.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 13/16] IB/pvrdma: Add the main driver module for PVRDMA

[Yuval Shaia]

On Sun, Sep 11, 2016 at 09:49:23PM -0700, Adit Ranadive wrote:
+
> +static void pvrdma_netdevice_event_handle(struct pvrdma_dev *dev,
> +   unsigned long event)
> +{
> + struct net_device *netdev;
> +
> + netdev = dev->netdev;

Please remove the above two lines.

> + switch (event) {
> + case NETDEV_REBOOT:
> + case NETDEV_DOWN:
> + pvrdma_dispatch_event(dev, 1, IB_EVENT_PORT_ERR);
> + break;
> + case NETDEV_UP:
> + pvrdma_dispatch_event(dev, 1, IB_EVENT_PORT_ACTIVE);
> + break;
> + default:
> + dev_dbg(&dev->pdev->dev, "ignore netdevice event %ld on %s\n",
> + event, dev->ib_dev.name);
> + break;
> + }
> +}

Re: [PATCH v4 12/16] IB/pvrdma: Add Queue Pair support

[Yuval Shaia]

No more comments.
Reviewed-by: Yuval Shaia 

On Sun, Sep 11, 2016 at 09:49:22PM -0700, Adit Ranadive wrote:
> This patch adds the ability to create, modify, query and destroy QPs. The
> PVRDMA device supports RC, UD and GSI QPs.
> 
> Reviewed-by: Jorgen Hansen 
> Reviewed-by: George Zhang 
> Reviewed-by: Aditya Sarwade 
> Reviewed-by: Bryan Tan 
> Signed-off-by: Adit Ranadive 
> ---
> Changes v3->v4:
>  - Removed an unnecessary switch case.
>  - Unified the returns in pvrdma_create_qp to use one exit point.
>  - Renamed pvrdma_flush_cqe to _pvrdma_flush_cqe since we need a lock to
>  be held when calling this.
>  - Updated to use wrapper for UAR write for QP.
>  - Updated conversion function to func_name(dst, src) format.
>  - Renamed max_gs to max_sg.
>  - Renamed cap variable to req_cap in pvrdma_set_sq/rq_size.
>  - Changed dev_warn to dev_warn_ratelimited in pvrdma_post_send/recv.
>  - Added nesting locking for flushing CQs when destroying/resetting a QP.
>  - Added missing ret value.
> 
> Changes v2->v3:
>  - Removed boolean in pvrdma_cmd_post.
> ---
>  drivers/infiniband/hw/pvrdma/pvrdma_qp.c | 980 
> +++
>  1 file changed, 980 insertions(+)
>  create mode 100644 drivers/infiniband/hw/pvrdma/pvrdma_qp.c
> 
> diff --git a/drivers/infiniband/hw/pvrdma/pvrdma_qp.c 
> b/drivers/infiniband/hw/pvrdma/pvrdma_qp.c
> new file mode 100644
> index 000..4163186
> --- /dev/null
> +++ b/drivers/infiniband/hw/pvrdma/pvrdma_qp.c
> @@ -0,0 +1,980 @@
> +/*
> + * Copyright (c) 2012-2016 VMware, Inc.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of EITHER the GNU General Public License
> + * version 2 as published by the Free Software Foundation or the BSD
> + * 2-Clause License. This program is distributed in the hope that it
> + * will be useful, but WITHOUT ANY WARRANTY; WITHOUT EVEN THE IMPLIED
> + * WARRANTY OF MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE.
> + * See the GNU General Public License version 2 for more details at
> + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program available in the file COPYING in the main
> + * directory of this source tree.
> + *
> + * The BSD 2-Clause License
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + *  - Redistributions of source code must retain the above
> + *copyright notice, this list of conditions and the following
> + *disclaimer.
> + *
> + *  - Redistributions in binary form must reproduce the above
> + *copyright notice, this list of conditions and the following
> + *disclaimer in the documentation and/or other materials
> + *provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> + * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
> + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
> + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
> + * OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "pvrdma.h"
> +#include "pvrdma_user.h"
> +
> +static inline void get_cqs(struct pvrdma_qp *qp, struct pvrdma_cq **send_cq,
> +struct pvrdma_cq **recv_cq)
> +{
> + *send_cq = to_vcq(qp->ibqp.send_cq);
> + *recv_cq = to_vcq(qp->ibqp.recv_cq);
> +}
> +
> +static void pvrdma_lock_cqs(struct pvrdma_cq *scq, struct pvrdma_cq *rcq,
> + unsigned long *scq_flags,
> + unsigned long *rcq_flags)
> + __acquires(scq->cq_lock) __acquires(rcq->cq_lock)
> +{
> + if (scq == rcq) {
> + spin_lock_irqsave(&scq->cq_lock, *scq_flags);
> + __acquire(rcq->cq_lock);
> + } e

Re: [PATCH v4 01/16] vmxnet3: Move PCI Id to pci_ids.h

[Yuval Shaia]

On Wed, Sep 14, 2016 at 04:00:25PM +, Adit Ranadive wrote:
> On Wed, Sep 14, 2016 at 04:09:12 -0700, Yuval Shaia wrote:
> > Please update vmxnet3_drv.c accordingly.
> 
> Any reason why? I don't think we need to. Vmxnet3 should just pick up the 
> moved
> PCI device id from pci_ids.h file.

So now you need to include it from vmxnet3_drv.c.
Same with pvrdma_main.c

> 
> - Adit
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 11/16] IB/pvrdma: Add support for memory regions

[Yuval Shaia]

No more comments.
Reviewed-by: Yuval Shaia 

On Sun, Sep 11, 2016 at 09:49:21PM -0700, Adit Ranadive wrote:
> This patch adds support for creating and destroying memory regions. The
> PVRDMA device supports User MRs, DMA MRs (no Remote Read/Write support),
> Fast Register MRs.
> 
> Reviewed-by: Jorgen Hansen 
> Reviewed-by: George Zhang 
> Reviewed-by: Aditya Sarwade 
> Reviewed-by: Bryan Tan 
> Signed-off-by: Adit Ranadive 
> ---
> Changes v3->v4:
>  - Changed access flag check for DMA MR to using bit operation.
>  - Removed some local variables.
> 
> Changes v2->v3:
>  - Removed boolean in pvrdma_cmd_post.
> ---
>  drivers/infiniband/hw/pvrdma/pvrdma_mr.c | 332 
> +++
>  1 file changed, 332 insertions(+)
>  create mode 100644 drivers/infiniband/hw/pvrdma/pvrdma_mr.c
> 
> diff --git a/drivers/infiniband/hw/pvrdma/pvrdma_mr.c 
> b/drivers/infiniband/hw/pvrdma/pvrdma_mr.c
> new file mode 100644
> index 000..6163f17
> --- /dev/null
> +++ b/drivers/infiniband/hw/pvrdma/pvrdma_mr.c
> @@ -0,0 +1,332 @@
> +/*
> + * Copyright (c) 2012-2016 VMware, Inc.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of EITHER the GNU General Public License
> + * version 2 as published by the Free Software Foundation or the BSD
> + * 2-Clause License. This program is distributed in the hope that it
> + * will be useful, but WITHOUT ANY WARRANTY; WITHOUT EVEN THE IMPLIED
> + * WARRANTY OF MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE.
> + * See the GNU General Public License version 2 for more details at
> + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program available in the file COPYING in the main
> + * directory of this source tree.
> + *
> + * The BSD 2-Clause License
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + *  - Redistributions of source code must retain the above
> + *copyright notice, this list of conditions and the following
> + *disclaimer.
> + *
> + *  - Redistributions in binary form must reproduce the above
> + *copyright notice, this list of conditions and the following
> + *disclaimer in the documentation and/or other materials
> + *provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> + * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
> + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
> + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
> + * OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include 
> +#include 
> +
> +#include "pvrdma.h"
> +
> +/**
> + * pvrdma_get_dma_mr - get a DMA memory region
> + * @pd: protection domain
> + * @acc: access flags
> + *
> + * @return: ib_mr pointer on success, otherwise returns an errno.
> + */
> +struct ib_mr *pvrdma_get_dma_mr(struct ib_pd *pd, int acc)
> +{
> + struct pvrdma_dev *dev = to_vdev(pd->device);
> + struct pvrdma_user_mr *mr;
> + union pvrdma_cmd_req req;
> + union pvrdma_cmd_resp rsp;
> + struct pvrdma_cmd_create_mr *cmd = &req.create_mr;
> + struct pvrdma_cmd_create_mr_resp *resp = &rsp.create_mr_resp;
> + int ret;
> +
> + if (!(acc & IB_ACCESS_LOCAL_WRITE)) {
> + dev_warn(&dev->pdev->dev,
> +  "unsupported dma mr access flags %#x\n", acc);
> + return ERR_PTR(-EOPNOTSUPP);
> + }
> +
> + mr = kzalloc(sizeof(*mr), GFP_KERNEL);
> + if (!mr)
> + return ERR_PTR(-ENOMEM);
> +
> + memset(cmd, 0, sizeof(*cmd));
> + cmd->hdr.cmd = PVRDMA_CMD_CREATE_MR;
> + cmd->pd_handle = to_vpd(pd)->pd_handle;
> + cmd->access_flags = acc;
> + cmd->flags = PVRDMA_MR_FLAG_DMA;
> +
> + ret = pvrdma_cmd_post(dev, &req, 

Re: [PATCH v4 09/16] IB/pvrdma: Add support for Completion Queues

[Yuval Shaia]

On Sun, Sep 11, 2016 at 09:49:19PM -0700, Adit Ranadive wrote:
> +
> +static int pvrdma_poll_one(struct pvrdma_cq *cq, struct pvrdma_qp **cur_qp,
> +struct ib_wc *wc)
> +{
> + struct pvrdma_dev *dev = to_vdev(cq->ibcq.device);
> + int has_data;
> + unsigned int head;
> + bool tried = false;
> + struct pvrdma_cqe *cqe;
> +
> +retry:
> + has_data = pvrdma_idx_ring_has_data(&cq->ring_state->rx,
> + cq->ibcq.cqe, &head);
> + if (has_data == 0) {
> + if (tried)
> + return -EAGAIN;
> +
> + /* Pass down POLL to give physical HCA a chance to poll. */
> + pvrdma_write_uar_cq(dev, cq->cq_handle | PVRDMA_UAR_CQ_POLL);
> +
> + tried = true;
> + goto retry;
> + } else if (has_data == PVRDMA_INVALID_IDX) {

I didn't went throw the entire life cycle of RX-ring's head and tail but
you need to make sure that PVRDMA_INVALID_IDX error is recoverable one, i.e
there is probability that in the next call to pvrdma_poll_one it will be
fine. Otherwise it is an endless loop.

> + dev_err(&dev->pdev->dev, "CQ ring state invalid\n");
> + return -EAGAIN;
> + }
> +
> + cqe = get_cqe(cq, head);
> +
> + /* Ensure cqe is valid. */
> + rmb();
> + if (dev->qp_tbl[cqe->qp & 0x])
> + *cur_qp = (struct pvrdma_qp *)dev->qp_tbl[cqe->qp & 0x];
> + else
> + return -EAGAIN;
> +
> + wc->opcode = pvrdma_wc_opcode_to_ib(cqe->opcode);
> + wc->status = pvrdma_wc_status_to_ib(cqe->status);
> + wc->wr_id = cqe->wr_id;
> + wc->qp = &(*cur_qp)->ibqp;
> + wc->byte_len = cqe->byte_len;
> + wc->ex.imm_data = cqe->imm_data;
> + wc->src_qp = cqe->src_qp;
> + wc->wc_flags = pvrdma_wc_flags_to_ib(cqe->wc_flags);
> + wc->pkey_index = cqe->pkey_index;
> + wc->slid = cqe->slid;
> + wc->sl = cqe->sl;
> + wc->dlid_path_bits = cqe->dlid_path_bits;
> + wc->port_num = cqe->port_num;
> + wc->vendor_err = 0;
> +
> + /* Update shared ring state */
> + pvrdma_idx_ring_inc(&cq->ring_state->rx.cons_head, cq->ibcq.cqe);
> +
> + return 0;
> +}
> +
> +/**
> + * pvrdma_poll_cq - poll for work completion queue entries
> + * @ibcq: completion queue
> + * @num_entries: the maximum number of entries
> + * @entry: pointer to work completion array
> + *
> + * @return: number of polled completion entries
> + */
> +int pvrdma_poll_cq(struct ib_cq *ibcq, int num_entries, struct ib_wc *wc)
> +{
> + struct pvrdma_cq *cq = to_vcq(ibcq);
> + struct pvrdma_qp *cur_qp = NULL;
> + unsigned long flags;
> + int npolled;
> +
> + if (num_entries < 1 || wc == NULL)
> + return 0;
> +
> + spin_lock_irqsave(&cq->cq_lock, flags);
> + for (npolled = 0; npolled < num_entries; ++npolled) {
> + if (pvrdma_poll_one(cq, &cur_qp, wc + npolled))
> + break;
> + }
> +
> + spin_unlock_irqrestore(&cq->cq_lock, flags);
> +
> + /* Ensure we do not return errors from poll_cq */
> + return npolled;
> +}
> +
> +/**
> + * pvrdma_resize_cq - resize CQ
> + * @ibcq: the completion queue
> + * @entries: CQ entries
> + * @udata: user data
> + *
> + * @return: -EOPNOTSUPP as CQ resize is not supported.
> + */
> +int pvrdma_resize_cq(struct ib_cq *ibcq, int entries, struct ib_udata *udata)
> +{
> + return -EOPNOTSUPP;
> +}
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 05/16] IB/pvrdma: Add functions for Verbs support

[Yuval Shaia]

On Sun, Sep 11, 2016 at 09:49:15PM -0700, Adit Ranadive wrote:
> +
> +/**
> + * pvrdma_alloc_pd - allocate protection domain
> + * @ibdev: the IB device
> + * @context: user context
> + * @udata: user data
> + *
> + * @return: the ib_pd protection domain pointer on success, otherwise errno.
> + */
> +struct ib_pd *pvrdma_alloc_pd(struct ib_device *ibdev,
> +   struct ib_ucontext *context,
> +   struct ib_udata *udata)
> +{
> + struct pvrdma_pd *pd;
> + struct pvrdma_dev *dev = to_vdev(ibdev);
> + union pvrdma_cmd_req req;
> + union pvrdma_cmd_resp rsp;
> + struct pvrdma_cmd_create_pd *cmd = &req.create_pd;
> + struct pvrdma_cmd_create_pd_resp *resp = &rsp.create_pd_resp;
> + int ret;
> + void *ptr;
> +
> + /* Check allowed max pds */
> + if (!atomic_add_unless(&dev->num_pds, 1, dev->dsr->caps.max_pd))
> + return ERR_PTR(-EINVAL);
> +
> + memset(cmd, 0, sizeof(*cmd));
> + cmd->hdr.cmd = PVRDMA_CMD_CREATE_PD;
> + cmd->ctx_handle = (context) ? to_vucontext(context)->ctx_handle : 0;
> + ret = pvrdma_cmd_post(dev, &req, &rsp);
> + if (ret < 0) {
> + dev_warn(&dev->pdev->dev,
> +  "failed to allocate protection domain, error: %d\n",
> +  ret);
> + ptr = ERR_PTR(ret);
> + goto err;
> + } else if (resp->hdr.ack != PVRDMA_CMD_CREATE_PD_RESP) {
> + dev_warn(&dev->pdev->dev,
> +  "unknown response for allocate protection domain\n");
> + ptr = ERR_PTR(-EFAULT);
> + goto err;
> + }
> +
> + pd = kmalloc(sizeof(*pd), GFP_KERNEL);
> + if (!pd) {
> + ptr = ERR_PTR(-ENOMEM);
> + goto err;
> + }

I know that this was my suggestion but also remember that you raised a
(correct) argument that it is preferred to first do other allocation and
free them if command fails then the other way around where failure of
memory allocation (like here) will force us to do the opposite command
(pvrdma_dealloc_pd in this case).

So either accept your way (better) or call pvrdma_dealloc_pd when kmalloc
fails.

> +
> + pd->privileged = !context;
> + pd->pd_handle = resp->pd_handle;
> + pd->pdn = resp->pd_handle;
> +
> + if (context) {
> + if (ib_copy_to_udata(udata, &pd->pdn, sizeof(__u32))) {
> + dev_warn(&dev->pdev->dev,
> +  "failed to copy back protection domain\n");
> + pvrdma_dealloc_pd(&pd->ibpd);
> + return ERR_PTR(-EFAULT);
> + }
> + }
> +
> + /* u32 pd handle */
> + return  &pd->ibpd;
> +
> +err:
> + atomic_dec(&dev->num_pds);
> + return ptr;
> +}

Re: [PATCH v4 07/16] IB/pvrdma: Add helper functions

[Yuval Shaia]

No more comments.
Reviewed-by: Yuval Shaia 

On Sun, Sep 11, 2016 at 09:49:17PM -0700, Adit Ranadive wrote:
> This patch adds helper functions to store guest page addresses in a page
> directory structure. The page directory pointer is passed down to the
> backend which then maps the entire memory for the RDMA object by
> traversing the directory. We add some more helper functions for converting
> to/from RDMA stack address handles from/to PVRDMA ones.
> 
> Reviewed-by: Jorgen Hansen 
> Reviewed-by: George Zhang 
> Reviewed-by: Aditya Sarwade 
> Reviewed-by: Bryan Tan 
> Signed-off-by: Adit Ranadive 
> ---
> Changes v3->v4:
>  - Updated conversion functions to func_name(dst, src) format.
>  - Removed unneeded local variables.
> ---
>  drivers/infiniband/hw/pvrdma/pvrdma_misc.c | 303 
> +
>  1 file changed, 303 insertions(+)
>  create mode 100644 drivers/infiniband/hw/pvrdma/pvrdma_misc.c
> 
> diff --git a/drivers/infiniband/hw/pvrdma/pvrdma_misc.c 
> b/drivers/infiniband/hw/pvrdma/pvrdma_misc.c
> new file mode 100644
> index 000..1f12cd6
> --- /dev/null
> +++ b/drivers/infiniband/hw/pvrdma/pvrdma_misc.c
> @@ -0,0 +1,303 @@
> +/*
> + * Copyright (c) 2012-2016 VMware, Inc.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of EITHER the GNU General Public License
> + * version 2 as published by the Free Software Foundation or the BSD
> + * 2-Clause License. This program is distributed in the hope that it
> + * will be useful, but WITHOUT ANY WARRANTY; WITHOUT EVEN THE IMPLIED
> + * WARRANTY OF MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE.
> + * See the GNU General Public License version 2 for more details at
> + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program available in the file COPYING in the main
> + * directory of this source tree.
> + *
> + * The BSD 2-Clause License
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + *  - Redistributions of source code must retain the above
> + *copyright notice, this list of conditions and the following
> + *disclaimer.
> + *
> + *  - Redistributions in binary form must reproduce the above
> + *copyright notice, this list of conditions and the following
> + *disclaimer in the documentation and/or other materials
> + *provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> + * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
> + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
> + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
> + * OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "pvrdma.h"
> +
> +int pvrdma_page_dir_init(struct pvrdma_dev *dev, struct pvrdma_page_dir 
> *pdir,
> +  u64 npages, bool alloc_pages)
> +{
> + u64 i;
> +
> + if (npages > PVRDMA_PAGE_DIR_MAX_PAGES)
> + return -EINVAL;
> +
> + memset(pdir, 0, sizeof(*pdir));
> +
> + pdir->dir = dma_alloc_coherent(&dev->pdev->dev, PAGE_SIZE,
> +&pdir->dir_dma, GFP_KERNEL);
> + if (!pdir->dir)
> + goto err;
> +
> + pdir->ntables = PVRDMA_PAGE_DIR_TABLE(npages - 1) + 1;
> + pdir->tables = kcalloc(pdir->ntables, sizeof(*pdir->tables),
> +GFP_KERNEL);
> + if (!pdir->tables)
> + goto err;
> +
> + for (i = 0; i < pdir->ntables; i++) {
> + pdir->tables[i] = dma_alloc_coherent(&dev->pdev->dev, PAGE_SIZE,
> +  &pdir->dir[i], GFP_KERNEL);
> + if (!pdir->tables[i])
> + goto err;
> + }
> +
> +  

Re: [PATCH v4 06/16] IB/pvrdma: Add paravirtual rdma device

[Yuval Shaia]

No more comments.
Reviewed-by: Yuval Shaia 

On Sun, Sep 11, 2016 at 09:49:16PM -0700, Adit Ranadive wrote:
> This patch adds the main device-level structures and functions to be used
> to provide RDMA functionality. Also, we define conversion functions from
> the IB core stack structures to the device-specific ones.
> 
> Reviewed-by: Jorgen Hansen 
> Reviewed-by: George Zhang 
> Reviewed-by: Aditya Sarwade 
> Reviewed-by: Bryan Tan 
> Signed-off-by: Adit Ranadive 
> ---
> Changes v3->v4:
>  - Renamed pvrdma_flush_cqe to _pvrdma_flush_cqe since we hold a lock
>  to call it.
>  - Added wrapper functions for writing to UARs for CQ/QP.
>  - The conversion functions are updated as func_name(dst, src) format.
>  - Renamed max_gs to max_sg.
>  - Added work struct for net device events.
>  - priviledged -> privileged.
> 
> Changes v2->v3:
>  - Removed VMware vendor id redefinition.
>  - Removed the boolean in pvrdma_cmd_post.
> ---
>  drivers/infiniband/hw/pvrdma/pvrdma.h | 473 
> ++
>  1 file changed, 473 insertions(+)
>  create mode 100644 drivers/infiniband/hw/pvrdma/pvrdma.h
> 
> diff --git a/drivers/infiniband/hw/pvrdma/pvrdma.h 
> b/drivers/infiniband/hw/pvrdma/pvrdma.h
> new file mode 100644
> index 000..fedd7cb
> --- /dev/null
> +++ b/drivers/infiniband/hw/pvrdma/pvrdma.h
> @@ -0,0 +1,473 @@
> +/*
> + * Copyright (c) 2012-2016 VMware, Inc.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of EITHER the GNU General Public License
> + * version 2 as published by the Free Software Foundation or the BSD
> + * 2-Clause License. This program is distributed in the hope that it
> + * will be useful, but WITHOUT ANY WARRANTY; WITHOUT EVEN THE IMPLIED
> + * WARRANTY OF MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE.
> + * See the GNU General Public License version 2 for more details at
> + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program available in the file COPYING in the main
> + * directory of this source tree.
> + *
> + * The BSD 2-Clause License
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + *  - Redistributions of source code must retain the above
> + *copyright notice, this list of conditions and the following
> + *disclaimer.
> + *
> + *  - Redistributions in binary form must reproduce the above
> + *copyright notice, this list of conditions and the following
> + *disclaimer in the documentation and/or other materials
> + *provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> + * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
> + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
> + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
> + * OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef __PVRDMA_H__
> +#define __PVRDMA_H__
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "pvrdma_defs.h"
> +#include "pvrdma_dev_api.h"
> +#include "pvrdma_verbs.h"
> +
> +/* NOT the same as BIT_MASK(). */
> +#define PVRDMA_MASK(n) ((n << 1) - 1)
> +
> +/*
> + * VMware PVRDMA PCI device id.
> + */
> +#define PCI_DEVICE_ID_VMWARE_PVRDMA  0x0820
> +
> +struct pvrdma_dev;
> +
> +struct pvrdma_page_dir {
> + dma_addr_t dir_dma;
> + u64 *dir;
> + int ntables;
> + u64 **tables;
> + u64 npages;
> + void **pages;
> +};
> +
> +struct pvrdma_cq {
> + struct ib_cq ibcq;
> + int offset;
> + spinlock_t cq_lock; /* Poll lock. */
> + struct pvrdma_uar_map *uar;
> + struct ib_umem *umem;
> + struct pvrdma_ring_state *ring_state;
> + struct pvrdma_pa

Re: [PATCH v4 01/16] vmxnet3: Move PCI Id to pci_ids.h

[Yuval Shaia]

Please update vmxnet3_drv.c accordingly.

Yuval

On Sun, Sep 11, 2016 at 09:49:11PM -0700, Adit Ranadive wrote:
> The VMXNet3 PCI Id will be shared with our paravirtual RDMA driver.
> Moved it to the shared location in pci_ids.h.
> 
> Suggested-by: Leon Romanovsky 
> Signed-off-by: Adit Ranadive 
> ---
> ---
>  drivers/net/vmxnet3/vmxnet3_int.h | 3 +--
>  include/linux/pci_ids.h   | 1 +
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/vmxnet3/vmxnet3_int.h 
> b/drivers/net/vmxnet3/vmxnet3_int.h
> index 74fc030..2bd6bf8 100644
> --- a/drivers/net/vmxnet3/vmxnet3_int.h
> +++ b/drivers/net/vmxnet3/vmxnet3_int.h
> @@ -119,9 +119,8 @@ enum {
>  };
>  
>  /*
> - * PCI vendor and device IDs.
> + * Maximum devices supported.
>   */
> -#define PCI_DEVICE_ID_VMWARE_VMXNET30x07B0
>  #define MAX_ETHERNET_CARDS   10
>  #define MAX_PCI_PASSTHRU_DEVICE  6
>  
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index c58752f..98bb455 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2251,6 +2251,7 @@
>  #define PCI_DEVICE_ID_RASTEL_2PORT   0x2000
>  
>  #define PCI_VENDOR_ID_VMWARE 0x15ad
> +#define PCI_DEVICE_ID_VMWARE_VMXNET3 0x07b0
>  
>  #define PCI_VENDOR_ID_ZOLTRIX0x15b0
>  #define PCI_DEVICE_ID_ZOLTRIX_2BD0   0x2bd0
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] net/mlx4_core: Fix backward compatibility on VFs

[Yuval Shaia]

On Fri, Mar 18, 2016 at 11:11:06PM -0400, David Miller wrote:
> From: Eli Cohen 
> Date: Thu, 17 Mar 2016 18:49:42 +0200
> 
> > Commit 85743f1eb345 ("net/mlx4_core: Set UAR page size to 4KB regardless
> > of system page size") introduced dependency where old VF drivers without
> > this fix fail to load if the PF driver runs with this commit.
> > 
> > To resolve this add a module parameter which disables that functionality
> > by default.  If both the PF and VFs are running with a driver with that
> > commit the administrator may set the module param to true.
> > 
> > The module parameter is called enable_4k_uar.
Can you consider passing this via comm-channel and save us all from new
module parameter?
Suggesting this from sys-admin perspective where (1) making this consist in
VF and **all** guests would me a nightmare and also (2) take into account
in public cloud that hypervisor sys-admin is not necessary the same person
as guest sys-admin.
> > 
> > Fixes: 85743f1eb345 ('net/mlx4_core: Set UAR page size to 4KB ...')
> > Signed-off-by: Eli Cohen 
> 
> Applied.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html