diff mbox series

[mlx5-next,14/15] {IB, net}/mlx5: E-Switch, Use index of rep for vport to IB port mapping

Message ID 20190617192247.25107-15-saeedm@mellanox.com
State Awaiting Upstream
Delegated to: David Miller
Headers show
Series Mellanox, mlx5 vport metadata matching | expand

Commit Message

Saeed Mahameed June 17, 2019, 7:23 p.m. UTC
From: Bodong Wang <bodong@mellanox.com>

In the single IB device mode, the mapping between vport number and
rep relies on a counter. However for dynamic vport allocation, it is
desired to keep consistent map of eswitch vport and IB port.

Hence, simplify code to remove the free running counter and instead
use the available vport index during load/unload sequence from the
eswitch.

Signed-off-by: Bodong Wang <bodong@mellanox.com>
Suggested-by: Parav Pandit <parav@mellanox.com>
Reviewed-by: Parav Pandit <parav@mellanox.com>
Reviewed-by: Mark Bloch <markb@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/infiniband/hw/mlx5/ib_rep.c                        | 4 ++--
 drivers/infiniband/hw/mlx5/mlx5_ib.h                       | 1 -
 drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c | 1 +
 include/linux/mlx5/eswitch.h                               | 2 ++
 4 files changed, 5 insertions(+), 3 deletions(-)

Comments

Leon Romanovsky June 18, 2019, 10:42 a.m. UTC | #1
On Mon, Jun 17, 2019 at 07:23:37PM +0000, Saeed Mahameed wrote:
> From: Bodong Wang <bodong@mellanox.com>
>
> In the single IB device mode, the mapping between vport number and
> rep relies on a counter. However for dynamic vport allocation, it is
> desired to keep consistent map of eswitch vport and IB port.
>
> Hence, simplify code to remove the free running counter and instead
> use the available vport index during load/unload sequence from the
> eswitch.
>
> Signed-off-by: Bodong Wang <bodong@mellanox.com>
> Suggested-by: Parav Pandit <parav@mellanox.com>
> Reviewed-by: Parav Pandit <parav@mellanox.com>

We are not adding multiple "*-by" for same user, please choose one.

Thanks
Parav Pandit June 18, 2019, 10:47 a.m. UTC | #2
Hi Leon,

> -----Original Message-----
> From: Leon Romanovsky
> Sent: Tuesday, June 18, 2019 4:12 PM
> To: Saeed Mahameed <saeedm@mellanox.com>
> Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; Bodong Wang
> <bodong@mellanox.com>; Parav Pandit <parav@mellanox.com>; Mark Bloch
> <markb@mellanox.com>
> Subject: Re: [PATCH mlx5-next 14/15] {IB, net}/mlx5: E-Switch, Use index of rep
> for vport to IB port mapping
> 
> On Mon, Jun 17, 2019 at 07:23:37PM +0000, Saeed Mahameed wrote:
> > From: Bodong Wang <bodong@mellanox.com>
> >
> > In the single IB device mode, the mapping between vport number and rep
> > relies on a counter. However for dynamic vport allocation, it is
> > desired to keep consistent map of eswitch vport and IB port.
> >
> > Hence, simplify code to remove the free running counter and instead
> > use the available vport index during load/unload sequence from the
> > eswitch.
> >
> > Signed-off-by: Bodong Wang <bodong@mellanox.com>
> > Suggested-by: Parav Pandit <parav@mellanox.com>
> > Reviewed-by: Parav Pandit <parav@mellanox.com>
> 
> We are not adding multiple "*-by" for same user, please choose one.
> 
Suggested-by was added by Bodong during our discussion. Later on when I did gerrit +1, RB tag got added.

> Thanks
Saeed Mahameed June 18, 2019, 6:25 p.m. UTC | #3
On Tue, 2019-06-18 at 10:47 +0000, Parav Pandit wrote:
> Hi Leon,
> 
> > -----Original Message-----
> > From: Leon Romanovsky
> > Sent: Tuesday, June 18, 2019 4:12 PM
> > To: Saeed Mahameed <saeedm@mellanox.com>
> > Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; Bodong Wang
> > <bodong@mellanox.com>; Parav Pandit <parav@mellanox.com>; Mark
> > Bloch
> > <markb@mellanox.com>
> > Subject: Re: [PATCH mlx5-next 14/15] {IB, net}/mlx5: E-Switch, Use
> > index of rep
> > for vport to IB port mapping
> > 
> > On Mon, Jun 17, 2019 at 07:23:37PM +0000, Saeed Mahameed wrote:
> > > From: Bodong Wang <bodong@mellanox.com>
> > > 
> > > In the single IB device mode, the mapping between vport number
> > > and rep
> > > relies on a counter. However for dynamic vport allocation, it is
> > > desired to keep consistent map of eswitch vport and IB port.
> > > 
> > > Hence, simplify code to remove the free running counter and
> > > instead
> > > use the available vport index during load/unload sequence from
> > > the
> > > eswitch.
> > > 
> > > Signed-off-by: Bodong Wang <bodong@mellanox.com>
> > > Suggested-by: Parav Pandit <parav@mellanox.com>
> > > Reviewed-by: Parav Pandit <parav@mellanox.com>
> > 
> > We are not adding multiple "*-by" for same user, please choose one.
> > 
> Suggested-by was added by Bodong during our discussion. Later on when
> I did gerrit +1, RB tag got added.
> 

Is there a rule against having multiple "*-by" ? i don't think so  and
there shouldn't be, users need to get the exact amount of recognition
as the amount of work they put into this patch, if they reviewed and
tested a patch they deserve two tags ..
Leon Romanovsky June 19, 2019, 5 a.m. UTC | #4
On Tue, Jun 18, 2019 at 06:25:46PM +0000, Saeed Mahameed wrote:
> On Tue, 2019-06-18 at 10:47 +0000, Parav Pandit wrote:
> > Hi Leon,
> >
> > > -----Original Message-----
> > > From: Leon Romanovsky
> > > Sent: Tuesday, June 18, 2019 4:12 PM
> > > To: Saeed Mahameed <saeedm@mellanox.com>
> > > Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; Bodong Wang
> > > <bodong@mellanox.com>; Parav Pandit <parav@mellanox.com>; Mark
> > > Bloch
> > > <markb@mellanox.com>
> > > Subject: Re: [PATCH mlx5-next 14/15] {IB, net}/mlx5: E-Switch, Use
> > > index of rep
> > > for vport to IB port mapping
> > >
> > > On Mon, Jun 17, 2019 at 07:23:37PM +0000, Saeed Mahameed wrote:
> > > > From: Bodong Wang <bodong@mellanox.com>
> > > >
> > > > In the single IB device mode, the mapping between vport number
> > > > and rep
> > > > relies on a counter. However for dynamic vport allocation, it is
> > > > desired to keep consistent map of eswitch vport and IB port.
> > > >
> > > > Hence, simplify code to remove the free running counter and
> > > > instead
> > > > use the available vport index during load/unload sequence from
> > > > the
> > > > eswitch.
> > > >
> > > > Signed-off-by: Bodong Wang <bodong@mellanox.com>
> > > > Suggested-by: Parav Pandit <parav@mellanox.com>
> > > > Reviewed-by: Parav Pandit <parav@mellanox.com>
> > >
> > > We are not adding multiple "*-by" for same user, please choose one.
> > >
> > Suggested-by was added by Bodong during our discussion. Later on when
> > I did gerrit +1, RB tag got added.
> >
>
> Is there a rule against having multiple "*-by" ? i don't think so  and
> there shouldn't be, users need to get the exact amount of recognition
> as the amount of work they put into this patch, if they reviewed and
> tested a patch they deserve two tags ..

Not everything in the world has and needs rules, sometimes common sense
is enough. It goes without saying that during internal review process,
developer suggested something. Recognition comes in many ways in the
kernel but definitely not by number of tags with specific developer
name on it, especially if this developer comes from same company
as patch author.

If we extend your claim, both you and me should add this type of
signature block for almost every patch which we submit:

Reviewed-by: ....
Tested-by:  ....
Suggested-by: ...
Signed-by: ...

Thanks

>
>
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/mlx5/ib_rep.c b/drivers/infiniband/hw/mlx5/ib_rep.c
index d4ed611de35d..da4b936b3219 100644
--- a/drivers/infiniband/hw/mlx5/ib_rep.c
+++ b/drivers/infiniband/hw/mlx5/ib_rep.c
@@ -14,7 +14,7 @@  mlx5_ib_set_vport_rep(struct mlx5_core_dev *dev, struct mlx5_eswitch_rep *rep)
 	int vport_index;
 
 	ibdev = mlx5_ib_get_uplink_ibdev(dev->priv.eswitch);
-	vport_index = ibdev->free_port++;
+	vport_index = rep->vport_index;
 
 	ibdev->port[vport_index].rep = rep;
 	write_lock(&ibdev->port[vport_index].roce.netdev_lock);
@@ -50,7 +50,7 @@  mlx5_ib_vport_rep_load(struct mlx5_core_dev *dev, struct mlx5_eswitch_rep *rep)
 	}
 
 	ibdev->is_rep = true;
-	vport_index = ibdev->free_port++;
+	vport_index = rep->vport_index;
 	ibdev->port[vport_index].rep = rep;
 	ibdev->port[vport_index].roce.netdev =
 		mlx5_ib_get_rep_netdev(dev->priv.eswitch, rep->vport);
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 1c205c2bd486..ee73dc122d28 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -978,7 +978,6 @@  struct mlx5_ib_dev {
 	u16			devx_whitelist_uid;
 	struct mlx5_srq_table   srq_table;
 	struct mlx5_async_ctx   async_ctx;
-	int			free_port;
 };
 
 static inline struct mlx5_ib_cq *to_mibcq(struct mlx5_core_cq *mcq)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index f29b9e1f49ae..7fdea5600383 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -1411,6 +1411,7 @@  int esw_offloads_init_reps(struct mlx5_eswitch *esw)
 
 	mlx5_esw_for_all_reps(esw, vport_index, rep) {
 		rep->vport = mlx5_eswitch_index_to_vport_num(esw, vport_index);
+		rep->vport_index = vport_index;
 		ether_addr_copy(rep->hw_id, hw_id);
 
 		for (rep_type = 0; rep_type < NUM_REP_TYPES; rep_type++)
diff --git a/include/linux/mlx5/eswitch.h b/include/linux/mlx5/eswitch.h
index d729f5e4d70a..d29abefa03c6 100644
--- a/include/linux/mlx5/eswitch.h
+++ b/include/linux/mlx5/eswitch.h
@@ -46,6 +46,8 @@  struct mlx5_eswitch_rep {
 	u16		       vport;
 	u8		       hw_id[ETH_ALEN];
 	u16		       vlan;
+	/* Only IB rep is using vport_index */
+	u16		       vport_index;
 	u32		       vlan_refcount;
 };