linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH for-next] RDMA/rxe: Remove pkey table
@ 2020-07-21 10:16 Kamal Heib
       [not found] ` <AM6PR05MB6263CFB337190B1740CDF4B7D8780@AM6PR05MB6263.eurprd05.prod.outlook.com>
  2020-07-31 19:22 ` Jason Gunthorpe
  0 siblings, 2 replies; 16+ messages in thread
From: Kamal Heib @ 2020-07-21 10:16 UTC (permalink / raw)
  To: linux-rdma; +Cc: Zhu Yanjun, Doug Ledford, Jason Gunthorpe, Kamal Heib

The RoCE spec require from RoCE devices to support only the defualt pkey,
While the rxe driver maintain a 64 enties pkey table and use only the
first entry. With that said remove the maintaing of the pkey table and
used the default pkey when needed.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe.c       | 34 +++------------------------
 drivers/infiniband/sw/rxe/rxe_param.h |  4 ++--
 drivers/infiniband/sw/rxe/rxe_recv.c  | 29 ++++-------------------
 drivers/infiniband/sw/rxe/rxe_req.c   |  5 +---
 drivers/infiniband/sw/rxe/rxe_verbs.c | 17 +++-----------
 drivers/infiniband/sw/rxe/rxe_verbs.h |  1 -
 6 files changed, 13 insertions(+), 77 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
index efcb72c92be6..907203afbd99 100644
--- a/drivers/infiniband/sw/rxe/rxe.c
+++ b/drivers/infiniband/sw/rxe/rxe.c
@@ -40,14 +40,6 @@ MODULE_AUTHOR("Bob Pearson, Frank Zago, John Groves, Kamal Heib");
 MODULE_DESCRIPTION("Soft RDMA transport");
 MODULE_LICENSE("Dual BSD/GPL");
 
-/* free resources for all ports on a device */
-static void rxe_cleanup_ports(struct rxe_dev *rxe)
-{
-	kfree(rxe->port.pkey_tbl);
-	rxe->port.pkey_tbl = NULL;
-
-}
-
 /* free resources for a rxe device all objects created for this device must
  * have been destroyed
  */
@@ -66,8 +58,6 @@ void rxe_dealloc(struct ib_device *ib_dev)
 	rxe_pool_cleanup(&rxe->mc_grp_pool);
 	rxe_pool_cleanup(&rxe->mc_elem_pool);
 
-	rxe_cleanup_ports(rxe);
-
 	if (rxe->tfm)
 		crypto_free_shash(rxe->tfm);
 }
@@ -139,25 +129,14 @@ static void rxe_init_port_param(struct rxe_port *port)
 /* initialize port state, note IB convention that HCA ports are always
  * numbered from 1
  */
-static int rxe_init_ports(struct rxe_dev *rxe)
+static void rxe_init_ports(struct rxe_dev *rxe)
 {
 	struct rxe_port *port = &rxe->port;
 
 	rxe_init_port_param(port);
-
-	port->pkey_tbl = kcalloc(port->attr.pkey_tbl_len,
-			sizeof(*port->pkey_tbl), GFP_KERNEL);
-
-	if (!port->pkey_tbl)
-		return -ENOMEM;
-
-	port->pkey_tbl[0] = 0xffff;
 	addrconf_addr_eui48((unsigned char *)&port->port_guid,
 			    rxe->ndev->dev_addr);
-
 	spin_lock_init(&port->port_lock);
-
-	return 0;
 }
 
 /* init pools of managed objects */
@@ -247,13 +226,11 @@ static int rxe_init(struct rxe_dev *rxe)
 	/* init default device parameters */
 	rxe_init_device_param(rxe);
 
-	err = rxe_init_ports(rxe);
-	if (err)
-		goto err1;
+	rxe_init_ports(rxe);
 
 	err = rxe_init_pools(rxe);
 	if (err)
-		goto err2;
+		return err;
 
 	/* init pending mmap list */
 	spin_lock_init(&rxe->mmap_offset_lock);
@@ -263,11 +240,6 @@ static int rxe_init(struct rxe_dev *rxe)
 	mutex_init(&rxe->usdev_lock);
 
 	return 0;
-
-err2:
-	rxe_cleanup_ports(rxe);
-err1:
-	return err;
 }
 
 void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu)
diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
index 99e9d8ba9767..2f381aeafcb5 100644
--- a/drivers/infiniband/sw/rxe/rxe_param.h
+++ b/drivers/infiniband/sw/rxe/rxe_param.h
@@ -100,7 +100,7 @@ enum rxe_device_param {
 	RXE_MAX_SRQ_SGE			= 27,
 	RXE_MIN_SRQ_SGE			= 1,
 	RXE_MAX_FMR_PAGE_LIST_LEN	= 512,
-	RXE_MAX_PKEYS			= 64,
+	RXE_MAX_PKEYS			= 1,
 	RXE_LOCAL_CA_ACK_DELAY		= 15,
 
 	RXE_MAX_UCONTEXT		= 512,
@@ -148,7 +148,7 @@ enum rxe_port_param {
 	RXE_PORT_INIT_TYPE_REPLY	= 0,
 	RXE_PORT_ACTIVE_WIDTH		= IB_WIDTH_1X,
 	RXE_PORT_ACTIVE_SPEED		= 1,
-	RXE_PORT_PKEY_TBL_LEN		= 64,
+	RXE_PORT_PKEY_TBL_LEN		= 1,
 	RXE_PORT_PHYS_STATE		= IB_PORT_PHYS_STATE_POLLING,
 	RXE_PORT_SUBNET_PREFIX		= 0xfe80000000000000ULL,
 };
diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
index 46e111c218fd..7e123d3c4d09 100644
--- a/drivers/infiniband/sw/rxe/rxe_recv.c
+++ b/drivers/infiniband/sw/rxe/rxe_recv.c
@@ -101,36 +101,15 @@ static void set_qkey_viol_cntr(struct rxe_port *port)
 static int check_keys(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
 		      u32 qpn, struct rxe_qp *qp)
 {
-	int i;
-	int found_pkey = 0;
 	struct rxe_port *port = &rxe->port;
 	u16 pkey = bth_pkey(pkt);
 
 	pkt->pkey_index = 0;
 
-	if (qpn == 1) {
-		for (i = 0; i < port->attr.pkey_tbl_len; i++) {
-			if (pkey_match(pkey, port->pkey_tbl[i])) {
-				pkt->pkey_index = i;
-				found_pkey = 1;
-				break;
-			}
-		}
-
-		if (!found_pkey) {
-			pr_warn_ratelimited("bad pkey = 0x%x\n", pkey);
-			set_bad_pkey_cntr(port);
-			goto err1;
-		}
-	} else {
-		if (unlikely(!pkey_match(pkey,
-					 port->pkey_tbl[qp->attr.pkey_index]
-					))) {
-			pr_warn_ratelimited("bad pkey = 0x%0x\n", pkey);
-			set_bad_pkey_cntr(port);
-			goto err1;
-		}
-		pkt->pkey_index = qp->attr.pkey_index;
+	if (!pkey_match(pkey, IB_DEFAULT_PKEY_FULL)) {
+		pr_warn_ratelimited("bad pkey = 0x%x\n", pkey);
+		set_bad_pkey_cntr(port);
+		goto err1;
 	}
 
 	if ((qp_type(qp) == IB_QPT_UD || qp_type(qp) == IB_QPT_GSI) &&
diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index e5031172c019..34df2b55e650 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -381,7 +381,6 @@ static struct sk_buff *init_req_packet(struct rxe_qp *qp,
 				       struct rxe_pkt_info *pkt)
 {
 	struct rxe_dev		*rxe = to_rdev(qp->ibqp.device);
-	struct rxe_port		*port = &rxe->port;
 	struct sk_buff		*skb;
 	struct rxe_send_wr	*ibwr = &wqe->wr;
 	struct rxe_av		*av;
@@ -419,9 +418,7 @@ static struct sk_buff *init_req_packet(struct rxe_qp *qp,
 			(pkt->mask & (RXE_WRITE_MASK | RXE_IMMDT_MASK)) ==
 			(RXE_WRITE_MASK | RXE_IMMDT_MASK));
 
-	pkey = (qp_type(qp) == IB_QPT_GSI) ?
-		 port->pkey_tbl[ibwr->wr.ud.pkey_index] :
-		 port->pkey_tbl[qp->attr.pkey_index];
+	pkey = IB_DEFAULT_PKEY_FULL;
 
 	qp_num = (pkt->mask & RXE_DETH_MASK) ? ibwr->wr.ud.remote_qpn :
 					 qp->attr.dest_qp_num;
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index 74f071003690..779458ddd422 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -83,22 +83,11 @@ static int rxe_query_port(struct ib_device *dev,
 static int rxe_query_pkey(struct ib_device *device,
 			  u8 port_num, u16 index, u16 *pkey)
 {
-	struct rxe_dev *rxe = to_rdev(device);
-	struct rxe_port *port;
-
-	port = &rxe->port;
-
-	if (unlikely(index >= port->attr.pkey_tbl_len)) {
-		dev_warn(device->dev.parent, "invalid index = %d\n",
-			 index);
-		goto err1;
-	}
+	if (index > 0)
+		return -EINVAL;
 
-	*pkey = port->pkey_tbl[index];
+	*pkey = IB_DEFAULT_PKEY_FULL;
 	return 0;
-
-err1:
-	return -EINVAL;
 }
 
 static int rxe_modify_device(struct ib_device *dev,
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index 92de39c4a7c1..c664c7f36ab5 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -371,7 +371,6 @@ struct rxe_mc_elem {
 
 struct rxe_port {
 	struct ib_port_attr	attr;
-	u16			*pkey_tbl;
 	__be64			port_guid;
 	__be64			subnet_prefix;
 	spinlock_t		port_lock; /* guard port */
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: FW: [PATCH for-next] RDMA/rxe: Remove pkey table
       [not found] ` <AM6PR05MB6263CFB337190B1740CDF4B7D8780@AM6PR05MB6263.eurprd05.prod.outlook.com>
@ 2020-07-22  2:09   ` Zhu Yanjun
  2020-07-23  5:57     ` Kamal Heib
  0 siblings, 1 reply; 16+ messages in thread
From: Zhu Yanjun @ 2020-07-22  2:09 UTC (permalink / raw)
  To: Yanjun Zhu, linux-rdma, Doug Ledford, Kamal Heib, Jason Gunthorpe

On Tue, Jul 21, 2020 at 7:28 PM Yanjun Zhu <yanjunz@mellanox.com> wrote:
>
>
>
> -----Original Message-----
> From: Kamal Heib <kamalheib1@gmail.com>
> Sent: Tuesday, July 21, 2020 6:16 PM
> To: linux-rdma@vger.kernel.org
> Cc: Yanjun Zhu <yanjunz@mellanox.com>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; Kamal Heib <kamalheib1@gmail.com>
> Subject: [PATCH for-next] RDMA/rxe: Remove pkey table
>
> The RoCE spec require from RoCE devices to support only the defualt pkey, While the rxe driver maintain a 64 enties pkey table and use only the first entry. With that said remove the maintaing of the pkey table and used the default pkey when needed.
>

Hi Kamal

After this patch is applied, do you make tests with SoftRoCE and mlx hardware?

The SoftRoCE should work well with the mlx hardware.

Zhu Yanjun

> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
> ---
>  drivers/infiniband/sw/rxe/rxe.c       | 34 +++------------------------
>  drivers/infiniband/sw/rxe/rxe_param.h |  4 ++--  drivers/infiniband/sw/rxe/rxe_recv.c  | 29 ++++-------------------
>  drivers/infiniband/sw/rxe/rxe_req.c   |  5 +---
>  drivers/infiniband/sw/rxe/rxe_verbs.c | 17 +++-----------  drivers/infiniband/sw/rxe/rxe_verbs.h |  1 -
>  6 files changed, 13 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c index efcb72c92be6..907203afbd99 100644
> --- a/drivers/infiniband/sw/rxe/rxe.c
> +++ b/drivers/infiniband/sw/rxe/rxe.c
> @@ -40,14 +40,6 @@ MODULE_AUTHOR("Bob Pearson, Frank Zago, John Groves, Kamal Heib");  MODULE_DESCRIPTION("Soft RDMA transport");  MODULE_LICENSE("Dual BSD/GPL");
>
> -/* free resources for all ports on a device */ -static void rxe_cleanup_ports(struct rxe_dev *rxe) -{
> -       kfree(rxe->port.pkey_tbl);
> -       rxe->port.pkey_tbl = NULL;
> -
> -}
> -
>  /* free resources for a rxe device all objects created for this device must
>   * have been destroyed
>   */
> @@ -66,8 +58,6 @@ void rxe_dealloc(struct ib_device *ib_dev)
>         rxe_pool_cleanup(&rxe->mc_grp_pool);
>         rxe_pool_cleanup(&rxe->mc_elem_pool);
>
> -       rxe_cleanup_ports(rxe);
> -
>         if (rxe->tfm)
>                 crypto_free_shash(rxe->tfm);
>  }
> @@ -139,25 +129,14 @@ static void rxe_init_port_param(struct rxe_port *port)
>  /* initialize port state, note IB convention that HCA ports are always
>   * numbered from 1
>   */
> -static int rxe_init_ports(struct rxe_dev *rxe)
> +static void rxe_init_ports(struct rxe_dev *rxe)
>  {
>         struct rxe_port *port = &rxe->port;
>
>         rxe_init_port_param(port);
> -
> -       port->pkey_tbl = kcalloc(port->attr.pkey_tbl_len,
> -                       sizeof(*port->pkey_tbl), GFP_KERNEL);
> -
> -       if (!port->pkey_tbl)
> -               return -ENOMEM;
> -
> -       port->pkey_tbl[0] = 0xffff;
>         addrconf_addr_eui48((unsigned char *)&port->port_guid,
>                             rxe->ndev->dev_addr);
> -
>         spin_lock_init(&port->port_lock);
> -
> -       return 0;
>  }
>
>  /* init pools of managed objects */
> @@ -247,13 +226,11 @@ static int rxe_init(struct rxe_dev *rxe)
>         /* init default device parameters */
>         rxe_init_device_param(rxe);
>
> -       err = rxe_init_ports(rxe);
> -       if (err)
> -               goto err1;
> +       rxe_init_ports(rxe);
>
>         err = rxe_init_pools(rxe);
>         if (err)
> -               goto err2;
> +               return err;
>
>         /* init pending mmap list */
>         spin_lock_init(&rxe->mmap_offset_lock);
> @@ -263,11 +240,6 @@ static int rxe_init(struct rxe_dev *rxe)
>         mutex_init(&rxe->usdev_lock);
>
>         return 0;
> -
> -err2:
> -       rxe_cleanup_ports(rxe);
> -err1:
> -       return err;
>  }
>
>  void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
> index 99e9d8ba9767..2f381aeafcb5 100644
> --- a/drivers/infiniband/sw/rxe/rxe_param.h
> +++ b/drivers/infiniband/sw/rxe/rxe_param.h
> @@ -100,7 +100,7 @@ enum rxe_device_param {
>         RXE_MAX_SRQ_SGE                 = 27,
>         RXE_MIN_SRQ_SGE                 = 1,
>         RXE_MAX_FMR_PAGE_LIST_LEN       = 512,
> -       RXE_MAX_PKEYS                   = 64,
> +       RXE_MAX_PKEYS                   = 1,
>         RXE_LOCAL_CA_ACK_DELAY          = 15,
>
>         RXE_MAX_UCONTEXT                = 512,
> @@ -148,7 +148,7 @@ enum rxe_port_param {
>         RXE_PORT_INIT_TYPE_REPLY        = 0,
>         RXE_PORT_ACTIVE_WIDTH           = IB_WIDTH_1X,
>         RXE_PORT_ACTIVE_SPEED           = 1,
> -       RXE_PORT_PKEY_TBL_LEN           = 64,
> +       RXE_PORT_PKEY_TBL_LEN           = 1,
>         RXE_PORT_PHYS_STATE             = IB_PORT_PHYS_STATE_POLLING,
>         RXE_PORT_SUBNET_PREFIX          = 0xfe80000000000000ULL,
>  };
> diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
> index 46e111c218fd..7e123d3c4d09 100644
> --- a/drivers/infiniband/sw/rxe/rxe_recv.c
> +++ b/drivers/infiniband/sw/rxe/rxe_recv.c
> @@ -101,36 +101,15 @@ static void set_qkey_viol_cntr(struct rxe_port *port)  static int check_keys(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
>                       u32 qpn, struct rxe_qp *qp)
>  {
> -       int i;
> -       int found_pkey = 0;
>         struct rxe_port *port = &rxe->port;
>         u16 pkey = bth_pkey(pkt);
>
>         pkt->pkey_index = 0;
>
> -       if (qpn == 1) {
> -               for (i = 0; i < port->attr.pkey_tbl_len; i++) {
> -                       if (pkey_match(pkey, port->pkey_tbl[i])) {
> -                               pkt->pkey_index = i;
> -                               found_pkey = 1;
> -                               break;
> -                       }
> -               }
> -
> -               if (!found_pkey) {
> -                       pr_warn_ratelimited("bad pkey = 0x%x\n", pkey);
> -                       set_bad_pkey_cntr(port);
> -                       goto err1;
> -               }
> -       } else {
> -               if (unlikely(!pkey_match(pkey,
> -                                        port->pkey_tbl[qp->attr.pkey_index]
> -                                       ))) {
> -                       pr_warn_ratelimited("bad pkey = 0x%0x\n", pkey);
> -                       set_bad_pkey_cntr(port);
> -                       goto err1;
> -               }
> -               pkt->pkey_index = qp->attr.pkey_index;
> +       if (!pkey_match(pkey, IB_DEFAULT_PKEY_FULL)) {
> +               pr_warn_ratelimited("bad pkey = 0x%x\n", pkey);
> +               set_bad_pkey_cntr(port);
> +               goto err1;
>         }
>
>         if ((qp_type(qp) == IB_QPT_UD || qp_type(qp) == IB_QPT_GSI) && diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> index e5031172c019..34df2b55e650 100644
> --- a/drivers/infiniband/sw/rxe/rxe_req.c
> +++ b/drivers/infiniband/sw/rxe/rxe_req.c
> @@ -381,7 +381,6 @@ static struct sk_buff *init_req_packet(struct rxe_qp *qp,
>                                        struct rxe_pkt_info *pkt)
>  {
>         struct rxe_dev          *rxe = to_rdev(qp->ibqp.device);
> -       struct rxe_port         *port = &rxe->port;
>         struct sk_buff          *skb;
>         struct rxe_send_wr      *ibwr = &wqe->wr;
>         struct rxe_av           *av;
> @@ -419,9 +418,7 @@ static struct sk_buff *init_req_packet(struct rxe_qp *qp,
>                         (pkt->mask & (RXE_WRITE_MASK | RXE_IMMDT_MASK)) ==
>                         (RXE_WRITE_MASK | RXE_IMMDT_MASK));
>
> -       pkey = (qp_type(qp) == IB_QPT_GSI) ?
> -                port->pkey_tbl[ibwr->wr.ud.pkey_index] :
> -                port->pkey_tbl[qp->attr.pkey_index];
> +       pkey = IB_DEFAULT_PKEY_FULL;
>
>         qp_num = (pkt->mask & RXE_DETH_MASK) ? ibwr->wr.ud.remote_qpn :
>                                          qp->attr.dest_qp_num;
> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
> index 74f071003690..779458ddd422 100644
> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
> @@ -83,22 +83,11 @@ static int rxe_query_port(struct ib_device *dev,  static int rxe_query_pkey(struct ib_device *device,
>                           u8 port_num, u16 index, u16 *pkey)  {
> -       struct rxe_dev *rxe = to_rdev(device);
> -       struct rxe_port *port;
> -
> -       port = &rxe->port;
> -
> -       if (unlikely(index >= port->attr.pkey_tbl_len)) {
> -               dev_warn(device->dev.parent, "invalid index = %d\n",
> -                        index);
> -               goto err1;
> -       }
> +       if (index > 0)
> +               return -EINVAL;
>
> -       *pkey = port->pkey_tbl[index];
> +       *pkey = IB_DEFAULT_PKEY_FULL;
>         return 0;
> -
> -err1:
> -       return -EINVAL;
>  }
>
>  static int rxe_modify_device(struct ib_device *dev, diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
> index 92de39c4a7c1..c664c7f36ab5 100644
> --- a/drivers/infiniband/sw/rxe/rxe_verbs.h
> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
> @@ -371,7 +371,6 @@ struct rxe_mc_elem {
>
>  struct rxe_port {
>         struct ib_port_attr     attr;
> -       u16                     *pkey_tbl;
>         __be64                  port_guid;
>         __be64                  subnet_prefix;
>         spinlock_t              port_lock; /* guard port */
> --
> 2.25.4
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: FW: [PATCH for-next] RDMA/rxe: Remove pkey table
  2020-07-22  2:09   ` FW: " Zhu Yanjun
@ 2020-07-23  5:57     ` Kamal Heib
  2020-07-23  6:58       ` Zhu Yanjun
  0 siblings, 1 reply; 16+ messages in thread
From: Kamal Heib @ 2020-07-23  5:57 UTC (permalink / raw)
  To: Zhu Yanjun; +Cc: Yanjun Zhu, linux-rdma, Doug Ledford, Jason Gunthorpe

On Wed, Jul 22, 2020 at 10:09:04AM +0800, Zhu Yanjun wrote:
> On Tue, Jul 21, 2020 at 7:28 PM Yanjun Zhu <yanjunz@mellanox.com> wrote:
> >
> >
> >
> > -----Original Message-----
> > From: Kamal Heib <kamalheib1@gmail.com>
> > Sent: Tuesday, July 21, 2020 6:16 PM
> > To: linux-rdma@vger.kernel.org
> > Cc: Yanjun Zhu <yanjunz@mellanox.com>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; Kamal Heib <kamalheib1@gmail.com>
> > Subject: [PATCH for-next] RDMA/rxe: Remove pkey table
> >
> > The RoCE spec require from RoCE devices to support only the defualt pkey, While the rxe driver maintain a 64 enties pkey table and use only the first entry. With that said remove the maintaing of the pkey table and used the default pkey when needed.
> >
> 
> Hi Kamal
> 
> After this patch is applied, do you make tests with SoftRoCE and mlx hardware?
> 
> The SoftRoCE should work well with the mlx hardware.
> 
> Zhu Yanjun
> 

Hi Zhu,

Yes, please see below:

$ ibv_rc_pingpong -d mlx5_0 -g 11
  local address:  LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
  remote address: LID 0x0000, QPN 0x000011, PSN 0xd67210, GID ::ffff:172.31.40.122
8192000 bytes in 0.03 seconds = 2194.56 Mbit/sec
1000 iters in 0.03 seconds = 29.86 usec/iter

$ ibv_rc_pingpong -d rxe0 -g 1 rdma-dev-21
  local address:  LID 0x0000, QPN 0x000011, PSN 0xd67210, GID ::ffff:172.31.40.122
  remote address: LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
8192000 bytes in 0.03 seconds = 2192.72 Mbit/sec
1000 iters in 0.03 seconds = 29.89 usec/iter

Thanks,
Kamal

> > Fixes: 8700e3e7c485 ("Soft RoCE driver")
> > Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
> > ---
> >  drivers/infiniband/sw/rxe/rxe.c       | 34 +++------------------------
> >  drivers/infiniband/sw/rxe/rxe_param.h |  4 ++--  drivers/infiniband/sw/rxe/rxe_recv.c  | 29 ++++-------------------
> >  drivers/infiniband/sw/rxe/rxe_req.c   |  5 +---
> >  drivers/infiniband/sw/rxe/rxe_verbs.c | 17 +++-----------  drivers/infiniband/sw/rxe/rxe_verbs.h |  1 -
> >  6 files changed, 13 insertions(+), 77 deletions(-)
> >
> > diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c index efcb72c92be6..907203afbd99 100644
> > --- a/drivers/infiniband/sw/rxe/rxe.c
> > +++ b/drivers/infiniband/sw/rxe/rxe.c
> > @@ -40,14 +40,6 @@ MODULE_AUTHOR("Bob Pearson, Frank Zago, John Groves, Kamal Heib");  MODULE_DESCRIPTION("Soft RDMA transport");  MODULE_LICENSE("Dual BSD/GPL");
> >
> > -/* free resources for all ports on a device */ -static void rxe_cleanup_ports(struct rxe_dev *rxe) -{
> > -       kfree(rxe->port.pkey_tbl);
> > -       rxe->port.pkey_tbl = NULL;
> > -
> > -}
> > -
> >  /* free resources for a rxe device all objects created for this device must
> >   * have been destroyed
> >   */
> > @@ -66,8 +58,6 @@ void rxe_dealloc(struct ib_device *ib_dev)
> >         rxe_pool_cleanup(&rxe->mc_grp_pool);
> >         rxe_pool_cleanup(&rxe->mc_elem_pool);
> >
> > -       rxe_cleanup_ports(rxe);
> > -
> >         if (rxe->tfm)
> >                 crypto_free_shash(rxe->tfm);
> >  }
> > @@ -139,25 +129,14 @@ static void rxe_init_port_param(struct rxe_port *port)
> >  /* initialize port state, note IB convention that HCA ports are always
> >   * numbered from 1
> >   */
> > -static int rxe_init_ports(struct rxe_dev *rxe)
> > +static void rxe_init_ports(struct rxe_dev *rxe)
> >  {
> >         struct rxe_port *port = &rxe->port;
> >
> >         rxe_init_port_param(port);
> > -
> > -       port->pkey_tbl = kcalloc(port->attr.pkey_tbl_len,
> > -                       sizeof(*port->pkey_tbl), GFP_KERNEL);
> > -
> > -       if (!port->pkey_tbl)
> > -               return -ENOMEM;
> > -
> > -       port->pkey_tbl[0] = 0xffff;
> >         addrconf_addr_eui48((unsigned char *)&port->port_guid,
> >                             rxe->ndev->dev_addr);
> > -
> >         spin_lock_init(&port->port_lock);
> > -
> > -       return 0;
> >  }
> >
> >  /* init pools of managed objects */
> > @@ -247,13 +226,11 @@ static int rxe_init(struct rxe_dev *rxe)
> >         /* init default device parameters */
> >         rxe_init_device_param(rxe);
> >
> > -       err = rxe_init_ports(rxe);
> > -       if (err)
> > -               goto err1;
> > +       rxe_init_ports(rxe);
> >
> >         err = rxe_init_pools(rxe);
> >         if (err)
> > -               goto err2;
> > +               return err;
> >
> >         /* init pending mmap list */
> >         spin_lock_init(&rxe->mmap_offset_lock);
> > @@ -263,11 +240,6 @@ static int rxe_init(struct rxe_dev *rxe)
> >         mutex_init(&rxe->usdev_lock);
> >
> >         return 0;
> > -
> > -err2:
> > -       rxe_cleanup_ports(rxe);
> > -err1:
> > -       return err;
> >  }
> >
> >  void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
> > index 99e9d8ba9767..2f381aeafcb5 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_param.h
> > +++ b/drivers/infiniband/sw/rxe/rxe_param.h
> > @@ -100,7 +100,7 @@ enum rxe_device_param {
> >         RXE_MAX_SRQ_SGE                 = 27,
> >         RXE_MIN_SRQ_SGE                 = 1,
> >         RXE_MAX_FMR_PAGE_LIST_LEN       = 512,
> > -       RXE_MAX_PKEYS                   = 64,
> > +       RXE_MAX_PKEYS                   = 1,
> >         RXE_LOCAL_CA_ACK_DELAY          = 15,
> >
> >         RXE_MAX_UCONTEXT                = 512,
> > @@ -148,7 +148,7 @@ enum rxe_port_param {
> >         RXE_PORT_INIT_TYPE_REPLY        = 0,
> >         RXE_PORT_ACTIVE_WIDTH           = IB_WIDTH_1X,
> >         RXE_PORT_ACTIVE_SPEED           = 1,
> > -       RXE_PORT_PKEY_TBL_LEN           = 64,
> > +       RXE_PORT_PKEY_TBL_LEN           = 1,
> >         RXE_PORT_PHYS_STATE             = IB_PORT_PHYS_STATE_POLLING,
> >         RXE_PORT_SUBNET_PREFIX          = 0xfe80000000000000ULL,
> >  };
> > diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
> > index 46e111c218fd..7e123d3c4d09 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_recv.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_recv.c
> > @@ -101,36 +101,15 @@ static void set_qkey_viol_cntr(struct rxe_port *port)  static int check_keys(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
> >                       u32 qpn, struct rxe_qp *qp)
> >  {
> > -       int i;
> > -       int found_pkey = 0;
> >         struct rxe_port *port = &rxe->port;
> >         u16 pkey = bth_pkey(pkt);
> >
> >         pkt->pkey_index = 0;
> >
> > -       if (qpn == 1) {
> > -               for (i = 0; i < port->attr.pkey_tbl_len; i++) {
> > -                       if (pkey_match(pkey, port->pkey_tbl[i])) {
> > -                               pkt->pkey_index = i;
> > -                               found_pkey = 1;
> > -                               break;
> > -                       }
> > -               }
> > -
> > -               if (!found_pkey) {
> > -                       pr_warn_ratelimited("bad pkey = 0x%x\n", pkey);
> > -                       set_bad_pkey_cntr(port);
> > -                       goto err1;
> > -               }
> > -       } else {
> > -               if (unlikely(!pkey_match(pkey,
> > -                                        port->pkey_tbl[qp->attr.pkey_index]
> > -                                       ))) {
> > -                       pr_warn_ratelimited("bad pkey = 0x%0x\n", pkey);
> > -                       set_bad_pkey_cntr(port);
> > -                       goto err1;
> > -               }
> > -               pkt->pkey_index = qp->attr.pkey_index;
> > +       if (!pkey_match(pkey, IB_DEFAULT_PKEY_FULL)) {
> > +               pr_warn_ratelimited("bad pkey = 0x%x\n", pkey);
> > +               set_bad_pkey_cntr(port);
> > +               goto err1;
> >         }
> >
> >         if ((qp_type(qp) == IB_QPT_UD || qp_type(qp) == IB_QPT_GSI) && diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> > index e5031172c019..34df2b55e650 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_req.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_req.c
> > @@ -381,7 +381,6 @@ static struct sk_buff *init_req_packet(struct rxe_qp *qp,
> >                                        struct rxe_pkt_info *pkt)
> >  {
> >         struct rxe_dev          *rxe = to_rdev(qp->ibqp.device);
> > -       struct rxe_port         *port = &rxe->port;
> >         struct sk_buff          *skb;
> >         struct rxe_send_wr      *ibwr = &wqe->wr;
> >         struct rxe_av           *av;
> > @@ -419,9 +418,7 @@ static struct sk_buff *init_req_packet(struct rxe_qp *qp,
> >                         (pkt->mask & (RXE_WRITE_MASK | RXE_IMMDT_MASK)) ==
> >                         (RXE_WRITE_MASK | RXE_IMMDT_MASK));
> >
> > -       pkey = (qp_type(qp) == IB_QPT_GSI) ?
> > -                port->pkey_tbl[ibwr->wr.ud.pkey_index] :
> > -                port->pkey_tbl[qp->attr.pkey_index];
> > +       pkey = IB_DEFAULT_PKEY_FULL;
> >
> >         qp_num = (pkt->mask & RXE_DETH_MASK) ? ibwr->wr.ud.remote_qpn :
> >                                          qp->attr.dest_qp_num;
> > diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
> > index 74f071003690..779458ddd422 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
> > @@ -83,22 +83,11 @@ static int rxe_query_port(struct ib_device *dev,  static int rxe_query_pkey(struct ib_device *device,
> >                           u8 port_num, u16 index, u16 *pkey)  {
> > -       struct rxe_dev *rxe = to_rdev(device);
> > -       struct rxe_port *port;
> > -
> > -       port = &rxe->port;
> > -
> > -       if (unlikely(index >= port->attr.pkey_tbl_len)) {
> > -               dev_warn(device->dev.parent, "invalid index = %d\n",
> > -                        index);
> > -               goto err1;
> > -       }
> > +       if (index > 0)
> > +               return -EINVAL;
> >
> > -       *pkey = port->pkey_tbl[index];
> > +       *pkey = IB_DEFAULT_PKEY_FULL;
> >         return 0;
> > -
> > -err1:
> > -       return -EINVAL;
> >  }
> >
> >  static int rxe_modify_device(struct ib_device *dev, diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
> > index 92de39c4a7c1..c664c7f36ab5 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_verbs.h
> > +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
> > @@ -371,7 +371,6 @@ struct rxe_mc_elem {
> >
> >  struct rxe_port {
> >         struct ib_port_attr     attr;
> > -       u16                     *pkey_tbl;
> >         __be64                  port_guid;
> >         __be64                  subnet_prefix;
> >         spinlock_t              port_lock; /* guard port */
> > --
> > 2.25.4
> >

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: FW: [PATCH for-next] RDMA/rxe: Remove pkey table
  2020-07-23  5:57     ` Kamal Heib
@ 2020-07-23  6:58       ` Zhu Yanjun
  2020-07-23  7:25         ` Kamal Heib
  0 siblings, 1 reply; 16+ messages in thread
From: Zhu Yanjun @ 2020-07-23  6:58 UTC (permalink / raw)
  To: Kamal Heib; +Cc: Yanjun Zhu, linux-rdma, Doug Ledford, Jason Gunthorpe

On 7/23/2020 1:57 PM, Kamal Heib wrote:
> On Wed, Jul 22, 2020 at 10:09:04AM +0800, Zhu Yanjun wrote:
>> On Tue, Jul 21, 2020 at 7:28 PM Yanjun Zhu <yanjunz@mellanox.com> wrote:
>>>
>>>
>>> -----Original Message-----
>>> From: Kamal Heib <kamalheib1@gmail.com>
>>> Sent: Tuesday, July 21, 2020 6:16 PM
>>> To: linux-rdma@vger.kernel.org
>>> Cc: Yanjun Zhu <yanjunz@mellanox.com>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; Kamal Heib <kamalheib1@gmail.com>
>>> Subject: [PATCH for-next] RDMA/rxe: Remove pkey table
>>>
>>> The RoCE spec require from RoCE devices to support only the defualt pkey, While the rxe driver maintain a 64 enties pkey table and use only the first entry. With that said remove the maintaing of the pkey table and used the default pkey when needed.
>>>
>> Hi Kamal
>>
>> After this patch is applied, do you make tests with SoftRoCE and mlx hardware?
>>
>> The SoftRoCE should work well with the mlx hardware.
>>
>> Zhu Yanjun
>>
> Hi Zhu,
>
> Yes, please see below:
>
> $ ibv_rc_pingpong -d mlx5_0 -g 11
>    local address:  LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121

Can you make tests with GSI QP?

Zhu Yanjun

>    remote address: LID 0x0000, QPN 0x000011, PSN 0xd67210, GID ::ffff:172.31.40.122
> 8192000 bytes in 0.03 seconds = 2194.56 Mbit/sec
> 1000 iters in 0.03 seconds = 29.86 usec/iter
>
> $ ibv_rc_pingpong -d rxe0 -g 1 rdma-dev-21
>    local address:  LID 0x0000, QPN 0x000011, PSN 0xd67210, GID ::ffff:172.31.40.122
>    remote address: LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
> 8192000 bytes in 0.03 seconds = 2192.72 Mbit/sec
> 1000 iters in 0.03 seconds = 29.89 usec/iter
>
> Thanks,
> Kamal
>
>>> Fixes: 8700e3e7c485 ("Soft RoCE driver")
>>> Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
>>> ---
>>>   drivers/infiniband/sw/rxe/rxe.c       | 34 +++------------------------
>>>   drivers/infiniband/sw/rxe/rxe_param.h |  4 ++--  drivers/infiniband/sw/rxe/rxe_recv.c  | 29 ++++-------------------
>>>   drivers/infiniband/sw/rxe/rxe_req.c   |  5 +---
>>>   drivers/infiniband/sw/rxe/rxe_verbs.c | 17 +++-----------  drivers/infiniband/sw/rxe/rxe_verbs.h |  1 -
>>>   6 files changed, 13 insertions(+), 77 deletions(-)
>>>
>>> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c index efcb72c92be6..907203afbd99 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe.c
>>> +++ b/drivers/infiniband/sw/rxe/rxe.c
>>> @@ -40,14 +40,6 @@ MODULE_AUTHOR("Bob Pearson, Frank Zago, John Groves, Kamal Heib");  MODULE_DESCRIPTION("Soft RDMA transport");  MODULE_LICENSE("Dual BSD/GPL");
>>>
>>> -/* free resources for all ports on a device */ -static void rxe_cleanup_ports(struct rxe_dev *rxe) -{
>>> -       kfree(rxe->port.pkey_tbl);
>>> -       rxe->port.pkey_tbl = NULL;
>>> -
>>> -}
>>> -
>>>   /* free resources for a rxe device all objects created for this device must
>>>    * have been destroyed
>>>    */
>>> @@ -66,8 +58,6 @@ void rxe_dealloc(struct ib_device *ib_dev)
>>>          rxe_pool_cleanup(&rxe->mc_grp_pool);
>>>          rxe_pool_cleanup(&rxe->mc_elem_pool);
>>>
>>> -       rxe_cleanup_ports(rxe);
>>> -
>>>          if (rxe->tfm)
>>>                  crypto_free_shash(rxe->tfm);
>>>   }
>>> @@ -139,25 +129,14 @@ static void rxe_init_port_param(struct rxe_port *port)
>>>   /* initialize port state, note IB convention that HCA ports are always
>>>    * numbered from 1
>>>    */
>>> -static int rxe_init_ports(struct rxe_dev *rxe)
>>> +static void rxe_init_ports(struct rxe_dev *rxe)
>>>   {
>>>          struct rxe_port *port = &rxe->port;
>>>
>>>          rxe_init_port_param(port);
>>> -
>>> -       port->pkey_tbl = kcalloc(port->attr.pkey_tbl_len,
>>> -                       sizeof(*port->pkey_tbl), GFP_KERNEL);
>>> -
>>> -       if (!port->pkey_tbl)
>>> -               return -ENOMEM;
>>> -
>>> -       port->pkey_tbl[0] = 0xffff;
>>>          addrconf_addr_eui48((unsigned char *)&port->port_guid,
>>>                              rxe->ndev->dev_addr);
>>> -
>>>          spin_lock_init(&port->port_lock);
>>> -
>>> -       return 0;
>>>   }
>>>
>>>   /* init pools of managed objects */
>>> @@ -247,13 +226,11 @@ static int rxe_init(struct rxe_dev *rxe)
>>>          /* init default device parameters */
>>>          rxe_init_device_param(rxe);
>>>
>>> -       err = rxe_init_ports(rxe);
>>> -       if (err)
>>> -               goto err1;
>>> +       rxe_init_ports(rxe);
>>>
>>>          err = rxe_init_pools(rxe);
>>>          if (err)
>>> -               goto err2;
>>> +               return err;
>>>
>>>          /* init pending mmap list */
>>>          spin_lock_init(&rxe->mmap_offset_lock);
>>> @@ -263,11 +240,6 @@ static int rxe_init(struct rxe_dev *rxe)
>>>          mutex_init(&rxe->usdev_lock);
>>>
>>>          return 0;
>>> -
>>> -err2:
>>> -       rxe_cleanup_ports(rxe);
>>> -err1:
>>> -       return err;
>>>   }
>>>
>>>   void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
>>> index 99e9d8ba9767..2f381aeafcb5 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_param.h
>>> +++ b/drivers/infiniband/sw/rxe/rxe_param.h
>>> @@ -100,7 +100,7 @@ enum rxe_device_param {
>>>          RXE_MAX_SRQ_SGE                 = 27,
>>>          RXE_MIN_SRQ_SGE                 = 1,
>>>          RXE_MAX_FMR_PAGE_LIST_LEN       = 512,
>>> -       RXE_MAX_PKEYS                   = 64,
>>> +       RXE_MAX_PKEYS                   = 1,
>>>          RXE_LOCAL_CA_ACK_DELAY          = 15,
>>>
>>>          RXE_MAX_UCONTEXT                = 512,
>>> @@ -148,7 +148,7 @@ enum rxe_port_param {
>>>          RXE_PORT_INIT_TYPE_REPLY        = 0,
>>>          RXE_PORT_ACTIVE_WIDTH           = IB_WIDTH_1X,
>>>          RXE_PORT_ACTIVE_SPEED           = 1,
>>> -       RXE_PORT_PKEY_TBL_LEN           = 64,
>>> +       RXE_PORT_PKEY_TBL_LEN           = 1,
>>>          RXE_PORT_PHYS_STATE             = IB_PORT_PHYS_STATE_POLLING,
>>>          RXE_PORT_SUBNET_PREFIX          = 0xfe80000000000000ULL,
>>>   };
>>> diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
>>> index 46e111c218fd..7e123d3c4d09 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_recv.c
>>> +++ b/drivers/infiniband/sw/rxe/rxe_recv.c
>>> @@ -101,36 +101,15 @@ static void set_qkey_viol_cntr(struct rxe_port *port)  static int check_keys(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
>>>                        u32 qpn, struct rxe_qp *qp)
>>>   {
>>> -       int i;
>>> -       int found_pkey = 0;
>>>          struct rxe_port *port = &rxe->port;
>>>          u16 pkey = bth_pkey(pkt);
>>>
>>>          pkt->pkey_index = 0;
>>>
>>> -       if (qpn == 1) {
>>> -               for (i = 0; i < port->attr.pkey_tbl_len; i++) {
>>> -                       if (pkey_match(pkey, port->pkey_tbl[i])) {
>>> -                               pkt->pkey_index = i;
>>> -                               found_pkey = 1;
>>> -                               break;
>>> -                       }
>>> -               }
>>> -
>>> -               if (!found_pkey) {
>>> -                       pr_warn_ratelimited("bad pkey = 0x%x\n", pkey);
>>> -                       set_bad_pkey_cntr(port);
>>> -                       goto err1;
>>> -               }
>>> -       } else {
>>> -               if (unlikely(!pkey_match(pkey,
>>> -                                        port->pkey_tbl[qp->attr.pkey_index]
>>> -                                       ))) {
>>> -                       pr_warn_ratelimited("bad pkey = 0x%0x\n", pkey);
>>> -                       set_bad_pkey_cntr(port);
>>> -                       goto err1;
>>> -               }
>>> -               pkt->pkey_index = qp->attr.pkey_index;
>>> +       if (!pkey_match(pkey, IB_DEFAULT_PKEY_FULL)) {
>>> +               pr_warn_ratelimited("bad pkey = 0x%x\n", pkey);
>>> +               set_bad_pkey_cntr(port);
>>> +               goto err1;
>>>          }
>>>
>>>          if ((qp_type(qp) == IB_QPT_UD || qp_type(qp) == IB_QPT_GSI) && diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
>>> index e5031172c019..34df2b55e650 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_req.c
>>> +++ b/drivers/infiniband/sw/rxe/rxe_req.c
>>> @@ -381,7 +381,6 @@ static struct sk_buff *init_req_packet(struct rxe_qp *qp,
>>>                                         struct rxe_pkt_info *pkt)
>>>   {
>>>          struct rxe_dev          *rxe = to_rdev(qp->ibqp.device);
>>> -       struct rxe_port         *port = &rxe->port;
>>>          struct sk_buff          *skb;
>>>          struct rxe_send_wr      *ibwr = &wqe->wr;
>>>          struct rxe_av           *av;
>>> @@ -419,9 +418,7 @@ static struct sk_buff *init_req_packet(struct rxe_qp *qp,
>>>                          (pkt->mask & (RXE_WRITE_MASK | RXE_IMMDT_MASK)) ==
>>>                          (RXE_WRITE_MASK | RXE_IMMDT_MASK));
>>>
>>> -       pkey = (qp_type(qp) == IB_QPT_GSI) ?
>>> -                port->pkey_tbl[ibwr->wr.ud.pkey_index] :
>>> -                port->pkey_tbl[qp->attr.pkey_index];
>>> +       pkey = IB_DEFAULT_PKEY_FULL;
>>>
>>>          qp_num = (pkt->mask & RXE_DETH_MASK) ? ibwr->wr.ud.remote_qpn :
>>>                                           qp->attr.dest_qp_num;
>>> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
>>> index 74f071003690..779458ddd422 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
>>> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
>>> @@ -83,22 +83,11 @@ static int rxe_query_port(struct ib_device *dev,  static int rxe_query_pkey(struct ib_device *device,
>>>                            u8 port_num, u16 index, u16 *pkey)  {
>>> -       struct rxe_dev *rxe = to_rdev(device);
>>> -       struct rxe_port *port;
>>> -
>>> -       port = &rxe->port;
>>> -
>>> -       if (unlikely(index >= port->attr.pkey_tbl_len)) {
>>> -               dev_warn(device->dev.parent, "invalid index = %d\n",
>>> -                        index);
>>> -               goto err1;
>>> -       }
>>> +       if (index > 0)
>>> +               return -EINVAL;
>>>
>>> -       *pkey = port->pkey_tbl[index];
>>> +       *pkey = IB_DEFAULT_PKEY_FULL;
>>>          return 0;
>>> -
>>> -err1:
>>> -       return -EINVAL;
>>>   }
>>>
>>>   static int rxe_modify_device(struct ib_device *dev, diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
>>> index 92de39c4a7c1..c664c7f36ab5 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_verbs.h
>>> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
>>> @@ -371,7 +371,6 @@ struct rxe_mc_elem {
>>>
>>>   struct rxe_port {
>>>          struct ib_port_attr     attr;
>>> -       u16                     *pkey_tbl;
>>>          __be64                  port_guid;
>>>          __be64                  subnet_prefix;
>>>          spinlock_t              port_lock; /* guard port */
>>> --
>>> 2.25.4
>>>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: FW: [PATCH for-next] RDMA/rxe: Remove pkey table
  2020-07-23  6:58       ` Zhu Yanjun
@ 2020-07-23  7:25         ` Kamal Heib
  2020-07-23 13:08           ` Zhu Yanjun
  0 siblings, 1 reply; 16+ messages in thread
From: Kamal Heib @ 2020-07-23  7:25 UTC (permalink / raw)
  To: Zhu Yanjun; +Cc: Yanjun Zhu, linux-rdma, Doug Ledford, Jason Gunthorpe

On Thu, Jul 23, 2020 at 02:58:41PM +0800, Zhu Yanjun wrote:
> On 7/23/2020 1:57 PM, Kamal Heib wrote:
> > On Wed, Jul 22, 2020 at 10:09:04AM +0800, Zhu Yanjun wrote:
> > > On Tue, Jul 21, 2020 at 7:28 PM Yanjun Zhu <yanjunz@mellanox.com> wrote:
> > > > 
> > > > 
> > > > -----Original Message-----
> > > > From: Kamal Heib <kamalheib1@gmail.com>
> > > > Sent: Tuesday, July 21, 2020 6:16 PM
> > > > To: linux-rdma@vger.kernel.org
> > > > Cc: Yanjun Zhu <yanjunz@mellanox.com>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; Kamal Heib <kamalheib1@gmail.com>
> > > > Subject: [PATCH for-next] RDMA/rxe: Remove pkey table
> > > > 
> > > > The RoCE spec require from RoCE devices to support only the defualt pkey, While the rxe driver maintain a 64 enties pkey table and use only the first entry. With that said remove the maintaing of the pkey table and used the default pkey when needed.
> > > > 
> > > Hi Kamal
> > > 
> > > After this patch is applied, do you make tests with SoftRoCE and mlx hardware?
> > > 
> > > The SoftRoCE should work well with the mlx hardware.
> > > 
> > > Zhu Yanjun
> > > 
> > Hi Zhu,
> > 
> > Yes, please see below:
> > 
> > $ ibv_rc_pingpong -d mlx5_0 -g 11
> >    local address:  LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
> 
> Can you make tests with GSI QP?
> 
> Zhu Yanjun
> 

[root@rdma-dev-21 ~]$ rping -s -C 10 -a 172.31.40.121 -v 
server ping data: rdma-ping-0: ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqr
server ping data: rdma-ping-1: BCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrs
server ping data: rdma-ping-2: CDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrst
server ping data: rdma-ping-3: DEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstu
server ping data: rdma-ping-4: EFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuv
server ping data: rdma-ping-5: FGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvw
server ping data: rdma-ping-6: GHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwx
server ping data: rdma-ping-7: HIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxy
server ping data: rdma-ping-8: IJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz
server ping data: rdma-ping-9: JKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyzA
server DISCONNECT EVENT...
wait for RDMA_READ_ADV state 10
[root@rdma-dev-21 ~]$ ls /sys/class/infiniband/
mlx5_0

[root@rdma-dev-22 ~]$ rping -c -C 10 -a 172.31.40.121 -v
ping data: rdma-ping-0: ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqr
ping data: rdma-ping-1: BCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrs
ping data: rdma-ping-2: CDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrst
ping data: rdma-ping-3: DEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstu
ping data: rdma-ping-4: EFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuv
ping data: rdma-ping-5: FGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvw
ping data: rdma-ping-6: GHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwx
ping data: rdma-ping-7: HIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxy
ping data: rdma-ping-8: IJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz
ping data: rdma-ping-9: JKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyzA
[root@rdma-dev-22 ~]$ ls /sys/class/infiniband
rxe0

Thanks,
Kamal

> >    remote address: LID 0x0000, QPN 0x000011, PSN 0xd67210, GID ::ffff:172.31.40.122
> > 8192000 bytes in 0.03 seconds = 2194.56 Mbit/sec
> > 1000 iters in 0.03 seconds = 29.86 usec/iter
> > 
> > $ ibv_rc_pingpong -d rxe0 -g 1 rdma-dev-21
> >    local address:  LID 0x0000, QPN 0x000011, PSN 0xd67210, GID ::ffff:172.31.40.122
> >    remote address: LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
> > 8192000 bytes in 0.03 seconds = 2192.72 Mbit/sec
> > 1000 iters in 0.03 seconds = 29.89 usec/iter
> > 
> > Thanks,
> > Kamal
> > 
> > > > Fixes: 8700e3e7c485 ("Soft RoCE driver")
> > > > Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
> > > > ---
> > > >   drivers/infiniband/sw/rxe/rxe.c       | 34 +++------------------------
> > > >   drivers/infiniband/sw/rxe/rxe_param.h |  4 ++--  drivers/infiniband/sw/rxe/rxe_recv.c  | 29 ++++-------------------
> > > >   drivers/infiniband/sw/rxe/rxe_req.c   |  5 +---
> > > >   drivers/infiniband/sw/rxe/rxe_verbs.c | 17 +++-----------  drivers/infiniband/sw/rxe/rxe_verbs.h |  1 -
> > > >   6 files changed, 13 insertions(+), 77 deletions(-)
> > > > 
> > > > diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c index efcb72c92be6..907203afbd99 100644
> > > > --- a/drivers/infiniband/sw/rxe/rxe.c
> > > > +++ b/drivers/infiniband/sw/rxe/rxe.c
> > > > @@ -40,14 +40,6 @@ MODULE_AUTHOR("Bob Pearson, Frank Zago, John Groves, Kamal Heib");  MODULE_DESCRIPTION("Soft RDMA transport");  MODULE_LICENSE("Dual BSD/GPL");
> > > > 
> > > > -/* free resources for all ports on a device */ -static void rxe_cleanup_ports(struct rxe_dev *rxe) -{
> > > > -       kfree(rxe->port.pkey_tbl);
> > > > -       rxe->port.pkey_tbl = NULL;
> > > > -
> > > > -}
> > > > -
> > > >   /* free resources for a rxe device all objects created for this device must
> > > >    * have been destroyed
> > > >    */
> > > > @@ -66,8 +58,6 @@ void rxe_dealloc(struct ib_device *ib_dev)
> > > >          rxe_pool_cleanup(&rxe->mc_grp_pool);
> > > >          rxe_pool_cleanup(&rxe->mc_elem_pool);
> > > > 
> > > > -       rxe_cleanup_ports(rxe);
> > > > -
> > > >          if (rxe->tfm)
> > > >                  crypto_free_shash(rxe->tfm);
> > > >   }
> > > > @@ -139,25 +129,14 @@ static void rxe_init_port_param(struct rxe_port *port)
> > > >   /* initialize port state, note IB convention that HCA ports are always
> > > >    * numbered from 1
> > > >    */
> > > > -static int rxe_init_ports(struct rxe_dev *rxe)
> > > > +static void rxe_init_ports(struct rxe_dev *rxe)
> > > >   {
> > > >          struct rxe_port *port = &rxe->port;
> > > > 
> > > >          rxe_init_port_param(port);
> > > > -
> > > > -       port->pkey_tbl = kcalloc(port->attr.pkey_tbl_len,
> > > > -                       sizeof(*port->pkey_tbl), GFP_KERNEL);
> > > > -
> > > > -       if (!port->pkey_tbl)
> > > > -               return -ENOMEM;
> > > > -
> > > > -       port->pkey_tbl[0] = 0xffff;
> > > >          addrconf_addr_eui48((unsigned char *)&port->port_guid,
> > > >                              rxe->ndev->dev_addr);
> > > > -
> > > >          spin_lock_init(&port->port_lock);
> > > > -
> > > > -       return 0;
> > > >   }
> > > > 
> > > >   /* init pools of managed objects */
> > > > @@ -247,13 +226,11 @@ static int rxe_init(struct rxe_dev *rxe)
> > > >          /* init default device parameters */
> > > >          rxe_init_device_param(rxe);
> > > > 
> > > > -       err = rxe_init_ports(rxe);
> > > > -       if (err)
> > > > -               goto err1;
> > > > +       rxe_init_ports(rxe);
> > > > 
> > > >          err = rxe_init_pools(rxe);
> > > >          if (err)
> > > > -               goto err2;
> > > > +               return err;
> > > > 
> > > >          /* init pending mmap list */
> > > >          spin_lock_init(&rxe->mmap_offset_lock);
> > > > @@ -263,11 +240,6 @@ static int rxe_init(struct rxe_dev *rxe)
> > > >          mutex_init(&rxe->usdev_lock);
> > > > 
> > > >          return 0;
> > > > -
> > > > -err2:
> > > > -       rxe_cleanup_ports(rxe);
> > > > -err1:
> > > > -       return err;
> > > >   }
> > > > 
> > > >   void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
> > > > index 99e9d8ba9767..2f381aeafcb5 100644
> > > > --- a/drivers/infiniband/sw/rxe/rxe_param.h
> > > > +++ b/drivers/infiniband/sw/rxe/rxe_param.h
> > > > @@ -100,7 +100,7 @@ enum rxe_device_param {
> > > >          RXE_MAX_SRQ_SGE                 = 27,
> > > >          RXE_MIN_SRQ_SGE                 = 1,
> > > >          RXE_MAX_FMR_PAGE_LIST_LEN       = 512,
> > > > -       RXE_MAX_PKEYS                   = 64,
> > > > +       RXE_MAX_PKEYS                   = 1,
> > > >          RXE_LOCAL_CA_ACK_DELAY          = 15,
> > > > 
> > > >          RXE_MAX_UCONTEXT                = 512,
> > > > @@ -148,7 +148,7 @@ enum rxe_port_param {
> > > >          RXE_PORT_INIT_TYPE_REPLY        = 0,
> > > >          RXE_PORT_ACTIVE_WIDTH           = IB_WIDTH_1X,
> > > >          RXE_PORT_ACTIVE_SPEED           = 1,
> > > > -       RXE_PORT_PKEY_TBL_LEN           = 64,
> > > > +       RXE_PORT_PKEY_TBL_LEN           = 1,
> > > >          RXE_PORT_PHYS_STATE             = IB_PORT_PHYS_STATE_POLLING,
> > > >          RXE_PORT_SUBNET_PREFIX          = 0xfe80000000000000ULL,
> > > >   };
> > > > diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
> > > > index 46e111c218fd..7e123d3c4d09 100644
> > > > --- a/drivers/infiniband/sw/rxe/rxe_recv.c
> > > > +++ b/drivers/infiniband/sw/rxe/rxe_recv.c
> > > > @@ -101,36 +101,15 @@ static void set_qkey_viol_cntr(struct rxe_port *port)  static int check_keys(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
> > > >                        u32 qpn, struct rxe_qp *qp)
> > > >   {
> > > > -       int i;
> > > > -       int found_pkey = 0;
> > > >          struct rxe_port *port = &rxe->port;
> > > >          u16 pkey = bth_pkey(pkt);
> > > > 
> > > >          pkt->pkey_index = 0;
> > > > 
> > > > -       if (qpn == 1) {
> > > > -               for (i = 0; i < port->attr.pkey_tbl_len; i++) {
> > > > -                       if (pkey_match(pkey, port->pkey_tbl[i])) {
> > > > -                               pkt->pkey_index = i;
> > > > -                               found_pkey = 1;
> > > > -                               break;
> > > > -                       }
> > > > -               }
> > > > -
> > > > -               if (!found_pkey) {
> > > > -                       pr_warn_ratelimited("bad pkey = 0x%x\n", pkey);
> > > > -                       set_bad_pkey_cntr(port);
> > > > -                       goto err1;
> > > > -               }
> > > > -       } else {
> > > > -               if (unlikely(!pkey_match(pkey,
> > > > -                                        port->pkey_tbl[qp->attr.pkey_index]
> > > > -                                       ))) {
> > > > -                       pr_warn_ratelimited("bad pkey = 0x%0x\n", pkey);
> > > > -                       set_bad_pkey_cntr(port);
> > > > -                       goto err1;
> > > > -               }
> > > > -               pkt->pkey_index = qp->attr.pkey_index;
> > > > +       if (!pkey_match(pkey, IB_DEFAULT_PKEY_FULL)) {
> > > > +               pr_warn_ratelimited("bad pkey = 0x%x\n", pkey);
> > > > +               set_bad_pkey_cntr(port);
> > > > +               goto err1;
> > > >          }
> > > > 
> > > >          if ((qp_type(qp) == IB_QPT_UD || qp_type(qp) == IB_QPT_GSI) && diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> > > > index e5031172c019..34df2b55e650 100644
> > > > --- a/drivers/infiniband/sw/rxe/rxe_req.c
> > > > +++ b/drivers/infiniband/sw/rxe/rxe_req.c
> > > > @@ -381,7 +381,6 @@ static struct sk_buff *init_req_packet(struct rxe_qp *qp,
> > > >                                         struct rxe_pkt_info *pkt)
> > > >   {
> > > >          struct rxe_dev          *rxe = to_rdev(qp->ibqp.device);
> > > > -       struct rxe_port         *port = &rxe->port;
> > > >          struct sk_buff          *skb;
> > > >          struct rxe_send_wr      *ibwr = &wqe->wr;
> > > >          struct rxe_av           *av;
> > > > @@ -419,9 +418,7 @@ static struct sk_buff *init_req_packet(struct rxe_qp *qp,
> > > >                          (pkt->mask & (RXE_WRITE_MASK | RXE_IMMDT_MASK)) ==
> > > >                          (RXE_WRITE_MASK | RXE_IMMDT_MASK));
> > > > 
> > > > -       pkey = (qp_type(qp) == IB_QPT_GSI) ?
> > > > -                port->pkey_tbl[ibwr->wr.ud.pkey_index] :
> > > > -                port->pkey_tbl[qp->attr.pkey_index];
> > > > +       pkey = IB_DEFAULT_PKEY_FULL;
> > > > 
> > > >          qp_num = (pkt->mask & RXE_DETH_MASK) ? ibwr->wr.ud.remote_qpn :
> > > >                                           qp->attr.dest_qp_num;
> > > > diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
> > > > index 74f071003690..779458ddd422 100644
> > > > --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
> > > > +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
> > > > @@ -83,22 +83,11 @@ static int rxe_query_port(struct ib_device *dev,  static int rxe_query_pkey(struct ib_device *device,
> > > >                            u8 port_num, u16 index, u16 *pkey)  {
> > > > -       struct rxe_dev *rxe = to_rdev(device);
> > > > -       struct rxe_port *port;
> > > > -
> > > > -       port = &rxe->port;
> > > > -
> > > > -       if (unlikely(index >= port->attr.pkey_tbl_len)) {
> > > > -               dev_warn(device->dev.parent, "invalid index = %d\n",
> > > > -                        index);
> > > > -               goto err1;
> > > > -       }
> > > > +       if (index > 0)
> > > > +               return -EINVAL;
> > > > 
> > > > -       *pkey = port->pkey_tbl[index];
> > > > +       *pkey = IB_DEFAULT_PKEY_FULL;
> > > >          return 0;
> > > > -
> > > > -err1:
> > > > -       return -EINVAL;
> > > >   }
> > > > 
> > > >   static int rxe_modify_device(struct ib_device *dev, diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
> > > > index 92de39c4a7c1..c664c7f36ab5 100644
> > > > --- a/drivers/infiniband/sw/rxe/rxe_verbs.h
> > > > +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
> > > > @@ -371,7 +371,6 @@ struct rxe_mc_elem {
> > > > 
> > > >   struct rxe_port {
> > > >          struct ib_port_attr     attr;
> > > > -       u16                     *pkey_tbl;
> > > >          __be64                  port_guid;
> > > >          __be64                  subnet_prefix;
> > > >          spinlock_t              port_lock; /* guard port */
> > > > --
> > > > 2.25.4
> > > > 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: FW: [PATCH for-next] RDMA/rxe: Remove pkey table
  2020-07-23  7:25         ` Kamal Heib
@ 2020-07-23 13:08           ` Zhu Yanjun
  2020-07-23 13:15             ` Jason Gunthorpe
  0 siblings, 1 reply; 16+ messages in thread
From: Zhu Yanjun @ 2020-07-23 13:08 UTC (permalink / raw)
  To: Kamal Heib; +Cc: Yanjun Zhu, linux-rdma, Doug Ledford, Jason Gunthorpe

On 7/23/2020 3:25 PM, Kamal Heib wrote:
> On Thu, Jul 23, 2020 at 02:58:41PM +0800, Zhu Yanjun wrote:
>> On 7/23/2020 1:57 PM, Kamal Heib wrote:
>>> On Wed, Jul 22, 2020 at 10:09:04AM +0800, Zhu Yanjun wrote:
>>>> On Tue, Jul 21, 2020 at 7:28 PM Yanjun Zhu <yanjunz@mellanox.com> wrote:
>>>>>
>>>>> -----Original Message-----
>>>>> From: Kamal Heib <kamalheib1@gmail.com>
>>>>> Sent: Tuesday, July 21, 2020 6:16 PM
>>>>> To: linux-rdma@vger.kernel.org
>>>>> Cc: Yanjun Zhu <yanjunz@mellanox.com>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; Kamal Heib <kamalheib1@gmail.com>
>>>>> Subject: [PATCH for-next] RDMA/rxe: Remove pkey table
>>>>>
>>>>> The RoCE spec require from RoCE devices to support only the defualt pkey, While the rxe driver maintain a 64 enties pkey table and use only the first entry. With that said remove the maintaing of the pkey table and used the default pkey when needed.
>>>>>
>>>> Hi Kamal
>>>>
>>>> After this patch is applied, do you make tests with SoftRoCE and mlx hardware?
>>>>
>>>> The SoftRoCE should work well with the mlx hardware.
>>>>
>>>> Zhu Yanjun
>>>>
>>> Hi Zhu,
>>>
>>> Yes, please see below:
>>>
>>> $ ibv_rc_pingpong -d mlx5_0 -g 11
>>>     local address:  LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
>> Can you make tests with GSI QP?
>>
>> Zhu Yanjun
>>

Is this the GSI ?

Please check GSI in "InfiniBandTM Architecture Specification Volume 1 
Release 1.3"

Then make tests with GSI again.


Zhu Yanjun


> [root@rdma-dev-21 ~]$ rping -s -C 10 -a 172.31.40.121 -v
> server ping data: rdma-ping-0: ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqr
> server ping data: rdma-ping-1: BCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrs
> server ping data: rdma-ping-2: CDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrst
> server ping data: rdma-ping-3: DEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstu
> server ping data: rdma-ping-4: EFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuv
> server ping data: rdma-ping-5: FGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvw
> server ping data: rdma-ping-6: GHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwx
> server ping data: rdma-ping-7: HIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxy
> server ping data: rdma-ping-8: IJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz
> server ping data: rdma-ping-9: JKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyzA
> server DISCONNECT EVENT...
> wait for RDMA_READ_ADV state 10
> [root@rdma-dev-21 ~]$ ls /sys/class/infiniband/
> mlx5_0
>
> [root@rdma-dev-22 ~]$ rping -c -C 10 -a 172.31.40.121 -v
> ping data: rdma-ping-0: ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqr
> ping data: rdma-ping-1: BCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrs
> ping data: rdma-ping-2: CDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrst
> ping data: rdma-ping-3: DEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstu
> ping data: rdma-ping-4: EFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuv
> ping data: rdma-ping-5: FGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvw
> ping data: rdma-ping-6: GHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwx
> ping data: rdma-ping-7: HIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxy
> ping data: rdma-ping-8: IJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz
> ping data: rdma-ping-9: JKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyzA
> [root@rdma-dev-22 ~]$ ls /sys/class/infiniband
> rxe0
>
> Thanks,
> Kamal
>
>>>     remote address: LID 0x0000, QPN 0x000011, PSN 0xd67210, GID ::ffff:172.31.40.122
>>> 8192000 bytes in 0.03 seconds = 2194.56 Mbit/sec
>>> 1000 iters in 0.03 seconds = 29.86 usec/iter
>>>
>>> $ ibv_rc_pingpong -d rxe0 -g 1 rdma-dev-21
>>>     local address:  LID 0x0000, QPN 0x000011, PSN 0xd67210, GID ::ffff:172.31.40.122
>>>     remote address: LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
>>> 8192000 bytes in 0.03 seconds = 2192.72 Mbit/sec
>>> 1000 iters in 0.03 seconds = 29.89 usec/iter
>>>
>>> Thanks,
>>> Kamal
>>>
>>>>> Fixes: 8700e3e7c485 ("Soft RoCE driver")
>>>>> Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
>>>>> ---
>>>>>    drivers/infiniband/sw/rxe/rxe.c       | 34 +++------------------------
>>>>>    drivers/infiniband/sw/rxe/rxe_param.h |  4 ++--  drivers/infiniband/sw/rxe/rxe_recv.c  | 29 ++++-------------------
>>>>>    drivers/infiniband/sw/rxe/rxe_req.c   |  5 +---
>>>>>    drivers/infiniband/sw/rxe/rxe_verbs.c | 17 +++-----------  drivers/infiniband/sw/rxe/rxe_verbs.h |  1 -
>>>>>    6 files changed, 13 insertions(+), 77 deletions(-)
>>>>>
>>>>> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c index efcb72c92be6..907203afbd99 100644
>>>>> --- a/drivers/infiniband/sw/rxe/rxe.c
>>>>> +++ b/drivers/infiniband/sw/rxe/rxe.c
>>>>> @@ -40,14 +40,6 @@ MODULE_AUTHOR("Bob Pearson, Frank Zago, John Groves, Kamal Heib");  MODULE_DESCRIPTION("Soft RDMA transport");  MODULE_LICENSE("Dual BSD/GPL");
>>>>>
>>>>> -/* free resources for all ports on a device */ -static void rxe_cleanup_ports(struct rxe_dev *rxe) -{
>>>>> -       kfree(rxe->port.pkey_tbl);
>>>>> -       rxe->port.pkey_tbl = NULL;
>>>>> -
>>>>> -}
>>>>> -
>>>>>    /* free resources for a rxe device all objects created for this device must
>>>>>     * have been destroyed
>>>>>     */
>>>>> @@ -66,8 +58,6 @@ void rxe_dealloc(struct ib_device *ib_dev)
>>>>>           rxe_pool_cleanup(&rxe->mc_grp_pool);
>>>>>           rxe_pool_cleanup(&rxe->mc_elem_pool);
>>>>>
>>>>> -       rxe_cleanup_ports(rxe);
>>>>> -
>>>>>           if (rxe->tfm)
>>>>>                   crypto_free_shash(rxe->tfm);
>>>>>    }
>>>>> @@ -139,25 +129,14 @@ static void rxe_init_port_param(struct rxe_port *port)
>>>>>    /* initialize port state, note IB convention that HCA ports are always
>>>>>     * numbered from 1
>>>>>     */
>>>>> -static int rxe_init_ports(struct rxe_dev *rxe)
>>>>> +static void rxe_init_ports(struct rxe_dev *rxe)
>>>>>    {
>>>>>           struct rxe_port *port = &rxe->port;
>>>>>
>>>>>           rxe_init_port_param(port);
>>>>> -
>>>>> -       port->pkey_tbl = kcalloc(port->attr.pkey_tbl_len,
>>>>> -                       sizeof(*port->pkey_tbl), GFP_KERNEL);
>>>>> -
>>>>> -       if (!port->pkey_tbl)
>>>>> -               return -ENOMEM;
>>>>> -
>>>>> -       port->pkey_tbl[0] = 0xffff;
>>>>>           addrconf_addr_eui48((unsigned char *)&port->port_guid,
>>>>>                               rxe->ndev->dev_addr);
>>>>> -
>>>>>           spin_lock_init(&port->port_lock);
>>>>> -
>>>>> -       return 0;
>>>>>    }
>>>>>
>>>>>    /* init pools of managed objects */
>>>>> @@ -247,13 +226,11 @@ static int rxe_init(struct rxe_dev *rxe)
>>>>>           /* init default device parameters */
>>>>>           rxe_init_device_param(rxe);
>>>>>
>>>>> -       err = rxe_init_ports(rxe);
>>>>> -       if (err)
>>>>> -               goto err1;
>>>>> +       rxe_init_ports(rxe);
>>>>>
>>>>>           err = rxe_init_pools(rxe);
>>>>>           if (err)
>>>>> -               goto err2;
>>>>> +               return err;
>>>>>
>>>>>           /* init pending mmap list */
>>>>>           spin_lock_init(&rxe->mmap_offset_lock);
>>>>> @@ -263,11 +240,6 @@ static int rxe_init(struct rxe_dev *rxe)
>>>>>           mutex_init(&rxe->usdev_lock);
>>>>>
>>>>>           return 0;
>>>>> -
>>>>> -err2:
>>>>> -       rxe_cleanup_ports(rxe);
>>>>> -err1:
>>>>> -       return err;
>>>>>    }
>>>>>
>>>>>    void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
>>>>> index 99e9d8ba9767..2f381aeafcb5 100644
>>>>> --- a/drivers/infiniband/sw/rxe/rxe_param.h
>>>>> +++ b/drivers/infiniband/sw/rxe/rxe_param.h
>>>>> @@ -100,7 +100,7 @@ enum rxe_device_param {
>>>>>           RXE_MAX_SRQ_SGE                 = 27,
>>>>>           RXE_MIN_SRQ_SGE                 = 1,
>>>>>           RXE_MAX_FMR_PAGE_LIST_LEN       = 512,
>>>>> -       RXE_MAX_PKEYS                   = 64,
>>>>> +       RXE_MAX_PKEYS                   = 1,
>>>>>           RXE_LOCAL_CA_ACK_DELAY          = 15,
>>>>>
>>>>>           RXE_MAX_UCONTEXT                = 512,
>>>>> @@ -148,7 +148,7 @@ enum rxe_port_param {
>>>>>           RXE_PORT_INIT_TYPE_REPLY        = 0,
>>>>>           RXE_PORT_ACTIVE_WIDTH           = IB_WIDTH_1X,
>>>>>           RXE_PORT_ACTIVE_SPEED           = 1,
>>>>> -       RXE_PORT_PKEY_TBL_LEN           = 64,
>>>>> +       RXE_PORT_PKEY_TBL_LEN           = 1,
>>>>>           RXE_PORT_PHYS_STATE             = IB_PORT_PHYS_STATE_POLLING,
>>>>>           RXE_PORT_SUBNET_PREFIX          = 0xfe80000000000000ULL,
>>>>>    };
>>>>> diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
>>>>> index 46e111c218fd..7e123d3c4d09 100644
>>>>> --- a/drivers/infiniband/sw/rxe/rxe_recv.c
>>>>> +++ b/drivers/infiniband/sw/rxe/rxe_recv.c
>>>>> @@ -101,36 +101,15 @@ static void set_qkey_viol_cntr(struct rxe_port *port)  static int check_keys(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
>>>>>                         u32 qpn, struct rxe_qp *qp)
>>>>>    {
>>>>> -       int i;
>>>>> -       int found_pkey = 0;
>>>>>           struct rxe_port *port = &rxe->port;
>>>>>           u16 pkey = bth_pkey(pkt);
>>>>>
>>>>>           pkt->pkey_index = 0;
>>>>>
>>>>> -       if (qpn == 1) {
>>>>> -               for (i = 0; i < port->attr.pkey_tbl_len; i++) {
>>>>> -                       if (pkey_match(pkey, port->pkey_tbl[i])) {
>>>>> -                               pkt->pkey_index = i;
>>>>> -                               found_pkey = 1;
>>>>> -                               break;
>>>>> -                       }
>>>>> -               }
>>>>> -
>>>>> -               if (!found_pkey) {
>>>>> -                       pr_warn_ratelimited("bad pkey = 0x%x\n", pkey);
>>>>> -                       set_bad_pkey_cntr(port);
>>>>> -                       goto err1;
>>>>> -               }
>>>>> -       } else {
>>>>> -               if (unlikely(!pkey_match(pkey,
>>>>> -                                        port->pkey_tbl[qp->attr.pkey_index]
>>>>> -                                       ))) {
>>>>> -                       pr_warn_ratelimited("bad pkey = 0x%0x\n", pkey);
>>>>> -                       set_bad_pkey_cntr(port);
>>>>> -                       goto err1;
>>>>> -               }
>>>>> -               pkt->pkey_index = qp->attr.pkey_index;
>>>>> +       if (!pkey_match(pkey, IB_DEFAULT_PKEY_FULL)) {
>>>>> +               pr_warn_ratelimited("bad pkey = 0x%x\n", pkey);
>>>>> +               set_bad_pkey_cntr(port);
>>>>> +               goto err1;
>>>>>           }
>>>>>
>>>>>           if ((qp_type(qp) == IB_QPT_UD || qp_type(qp) == IB_QPT_GSI) && diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
>>>>> index e5031172c019..34df2b55e650 100644
>>>>> --- a/drivers/infiniband/sw/rxe/rxe_req.c
>>>>> +++ b/drivers/infiniband/sw/rxe/rxe_req.c
>>>>> @@ -381,7 +381,6 @@ static struct sk_buff *init_req_packet(struct rxe_qp *qp,
>>>>>                                          struct rxe_pkt_info *pkt)
>>>>>    {
>>>>>           struct rxe_dev          *rxe = to_rdev(qp->ibqp.device);
>>>>> -       struct rxe_port         *port = &rxe->port;
>>>>>           struct sk_buff          *skb;
>>>>>           struct rxe_send_wr      *ibwr = &wqe->wr;
>>>>>           struct rxe_av           *av;
>>>>> @@ -419,9 +418,7 @@ static struct sk_buff *init_req_packet(struct rxe_qp *qp,
>>>>>                           (pkt->mask & (RXE_WRITE_MASK | RXE_IMMDT_MASK)) ==
>>>>>                           (RXE_WRITE_MASK | RXE_IMMDT_MASK));
>>>>>
>>>>> -       pkey = (qp_type(qp) == IB_QPT_GSI) ?
>>>>> -                port->pkey_tbl[ibwr->wr.ud.pkey_index] :
>>>>> -                port->pkey_tbl[qp->attr.pkey_index];
>>>>> +       pkey = IB_DEFAULT_PKEY_FULL;
>>>>>
>>>>>           qp_num = (pkt->mask & RXE_DETH_MASK) ? ibwr->wr.ud.remote_qpn :
>>>>>                                            qp->attr.dest_qp_num;
>>>>> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
>>>>> index 74f071003690..779458ddd422 100644
>>>>> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
>>>>> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
>>>>> @@ -83,22 +83,11 @@ static int rxe_query_port(struct ib_device *dev,  static int rxe_query_pkey(struct ib_device *device,
>>>>>                             u8 port_num, u16 index, u16 *pkey)  {
>>>>> -       struct rxe_dev *rxe = to_rdev(device);
>>>>> -       struct rxe_port *port;
>>>>> -
>>>>> -       port = &rxe->port;
>>>>> -
>>>>> -       if (unlikely(index >= port->attr.pkey_tbl_len)) {
>>>>> -               dev_warn(device->dev.parent, "invalid index = %d\n",
>>>>> -                        index);
>>>>> -               goto err1;
>>>>> -       }
>>>>> +       if (index > 0)
>>>>> +               return -EINVAL;
>>>>>
>>>>> -       *pkey = port->pkey_tbl[index];
>>>>> +       *pkey = IB_DEFAULT_PKEY_FULL;
>>>>>           return 0;
>>>>> -
>>>>> -err1:
>>>>> -       return -EINVAL;
>>>>>    }
>>>>>
>>>>>    static int rxe_modify_device(struct ib_device *dev, diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
>>>>> index 92de39c4a7c1..c664c7f36ab5 100644
>>>>> --- a/drivers/infiniband/sw/rxe/rxe_verbs.h
>>>>> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
>>>>> @@ -371,7 +371,6 @@ struct rxe_mc_elem {
>>>>>
>>>>>    struct rxe_port {
>>>>>           struct ib_port_attr     attr;
>>>>> -       u16                     *pkey_tbl;
>>>>>           __be64                  port_guid;
>>>>>           __be64                  subnet_prefix;
>>>>>           spinlock_t              port_lock; /* guard port */
>>>>> --
>>>>> 2.25.4
>>>>>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: FW: [PATCH for-next] RDMA/rxe: Remove pkey table
  2020-07-23 13:08           ` Zhu Yanjun
@ 2020-07-23 13:15             ` Jason Gunthorpe
  2020-07-23 15:15               ` Zhu Yanjun
  0 siblings, 1 reply; 16+ messages in thread
From: Jason Gunthorpe @ 2020-07-23 13:15 UTC (permalink / raw)
  To: Zhu Yanjun; +Cc: Kamal Heib, Yanjun Zhu, linux-rdma, Doug Ledford

On Thu, Jul 23, 2020 at 09:08:39PM +0800, Zhu Yanjun wrote:
> On 7/23/2020 3:25 PM, Kamal Heib wrote:
> > On Thu, Jul 23, 2020 at 02:58:41PM +0800, Zhu Yanjun wrote:
> > > On 7/23/2020 1:57 PM, Kamal Heib wrote:
> > > > On Wed, Jul 22, 2020 at 10:09:04AM +0800, Zhu Yanjun wrote:
> > > > > On Tue, Jul 21, 2020 at 7:28 PM Yanjun Zhu <yanjunz@mellanox.com> wrote:
> > > > > > 
> > > > > > From: Kamal Heib <kamalheib1@gmail.com>
> > > > > > Sent: Tuesday, July 21, 2020 6:16 PM
> > > > > > To: linux-rdma@vger.kernel.org
> > > > > > Cc: Yanjun Zhu <yanjunz@mellanox.com>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; Kamal Heib <kamalheib1@gmail.com>
> > > > > > Subject: [PATCH for-next] RDMA/rxe: Remove pkey table
> > > > > > 
> > > > > > The RoCE spec require from RoCE devices to support only the defualt pkey, While the rxe driver maintain a 64 enties pkey table and use only the first entry. With that said remove the maintaing of the pkey table and used the default pkey when needed.
> > > > > > 
> > > > > Hi Kamal
> > > > > 
> > > > > After this patch is applied, do you make tests with SoftRoCE and mlx hardware?
> > > > > 
> > > > > The SoftRoCE should work well with the mlx hardware.
> > > > > 
> > > > > Zhu Yanjun
> > > > > 
> > > > Hi Zhu,
> > > > 
> > > > Yes, please see below:
> > > > 
> > > > $ ibv_rc_pingpong -d mlx5_0 -g 11
> > > >     local address:  LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
> > > Can you make tests with GSI QP?
> > > 
> > > Zhu Yanjun
> > > 
> 
> Is this the GSI ?
> 
> Please check GSI in "InfiniBandTM Architecture Specification Volume 1
> Release 1.3"
> 
> Then make tests with GSI again.

rping uses RDMA CM which goes over the GSI

Jason

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: FW: [PATCH for-next] RDMA/rxe: Remove pkey table
  2020-07-23 13:15             ` Jason Gunthorpe
@ 2020-07-23 15:15               ` Zhu Yanjun
  2020-07-28  8:35                 ` Kamal Heib
  0 siblings, 1 reply; 16+ messages in thread
From: Zhu Yanjun @ 2020-07-23 15:15 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: Kamal Heib, Yanjun Zhu, linux-rdma, Doug Ledford

On 7/23/2020 9:15 PM, Jason Gunthorpe wrote:
> On Thu, Jul 23, 2020 at 09:08:39PM +0800, Zhu Yanjun wrote:
>> On 7/23/2020 3:25 PM, Kamal Heib wrote:
>>> On Thu, Jul 23, 2020 at 02:58:41PM +0800, Zhu Yanjun wrote:
>>>> On 7/23/2020 1:57 PM, Kamal Heib wrote:
>>>>> On Wed, Jul 22, 2020 at 10:09:04AM +0800, Zhu Yanjun wrote:
>>>>>> On Tue, Jul 21, 2020 at 7:28 PM Yanjun Zhu <yanjunz@mellanox.com> wrote:
>>>>>>> From: Kamal Heib <kamalheib1@gmail.com>
>>>>>>> Sent: Tuesday, July 21, 2020 6:16 PM
>>>>>>> To: linux-rdma@vger.kernel.org
>>>>>>> Cc: Yanjun Zhu <yanjunz@mellanox.com>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; Kamal Heib <kamalheib1@gmail.com>
>>>>>>> Subject: [PATCH for-next] RDMA/rxe: Remove pkey table
>>>>>>>
>>>>>>> The RoCE spec require from RoCE devices to support only the defualt pkey, While the rxe driver maintain a 64 enties pkey table and use only the first entry. With that said remove the maintaing of the pkey table and used the default pkey when needed.
>>>>>>>
>>>>>> Hi Kamal
>>>>>>
>>>>>> After this patch is applied, do you make tests with SoftRoCE and mlx hardware?
>>>>>>
>>>>>> The SoftRoCE should work well with the mlx hardware.
>>>>>>
>>>>>> Zhu Yanjun
>>>>>>
>>>>> Hi Zhu,
>>>>>
>>>>> Yes, please see below:
>>>>>
>>>>> $ ibv_rc_pingpong -d mlx5_0 -g 11
>>>>>      local address:  LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
>>>> Can you make tests with GSI QP?
>>>>
>>>> Zhu Yanjun
>>>>
>> Is this the GSI ?
>>
>> Please check GSI in "InfiniBandTM Architecture Specification Volume 1
>> Release 1.3"
>>
>> Then make tests with GSI again.

The followings are also removed by this commit. Not sure if it is good.

"

C9-42: If the destination QP is QP1, the BTH:P_Key shall be compared to 
the set of P_Keys associated with the port on which the packet arrived. 
If the P_Key matches any of the keys associated with the port, it shall 
be considered valid.

"

> rping uses RDMA CM which goes over the GSI
>
> Jason



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: FW: [PATCH for-next] RDMA/rxe: Remove pkey table
  2020-07-23 15:15               ` Zhu Yanjun
@ 2020-07-28  8:35                 ` Kamal Heib
  2020-07-28 13:21                   ` Zhu Yanjun
  0 siblings, 1 reply; 16+ messages in thread
From: Kamal Heib @ 2020-07-28  8:35 UTC (permalink / raw)
  To: Zhu Yanjun; +Cc: Jason Gunthorpe, Yanjun Zhu, linux-rdma, Doug Ledford

On Thu, Jul 23, 2020 at 11:15:00PM +0800, Zhu Yanjun wrote:
> On 7/23/2020 9:15 PM, Jason Gunthorpe wrote:
> > On Thu, Jul 23, 2020 at 09:08:39PM +0800, Zhu Yanjun wrote:
> > > On 7/23/2020 3:25 PM, Kamal Heib wrote:
> > > > On Thu, Jul 23, 2020 at 02:58:41PM +0800, Zhu Yanjun wrote:
> > > > > On 7/23/2020 1:57 PM, Kamal Heib wrote:
> > > > > > On Wed, Jul 22, 2020 at 10:09:04AM +0800, Zhu Yanjun wrote:
> > > > > > > On Tue, Jul 21, 2020 at 7:28 PM Yanjun Zhu <yanjunz@mellanox.com> wrote:
> > > > > > > > From: Kamal Heib <kamalheib1@gmail.com>
> > > > > > > > Sent: Tuesday, July 21, 2020 6:16 PM
> > > > > > > > To: linux-rdma@vger.kernel.org
> > > > > > > > Cc: Yanjun Zhu <yanjunz@mellanox.com>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; Kamal Heib <kamalheib1@gmail.com>
> > > > > > > > Subject: [PATCH for-next] RDMA/rxe: Remove pkey table
> > > > > > > > 
> > > > > > > > The RoCE spec require from RoCE devices to support only the defualt pkey, While the rxe driver maintain a 64 enties pkey table and use only the first entry. With that said remove the maintaing of the pkey table and used the default pkey when needed.
> > > > > > > > 
> > > > > > > Hi Kamal
> > > > > > > 
> > > > > > > After this patch is applied, do you make tests with SoftRoCE and mlx hardware?
> > > > > > > 
> > > > > > > The SoftRoCE should work well with the mlx hardware.
> > > > > > > 
> > > > > > > Zhu Yanjun
> > > > > > > 
> > > > > > Hi Zhu,
> > > > > > 
> > > > > > Yes, please see below:
> > > > > > 
> > > > > > $ ibv_rc_pingpong -d mlx5_0 -g 11
> > > > > >      local address:  LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
> > > > > Can you make tests with GSI QP?
> > > > > 
> > > > > Zhu Yanjun
> > > > > 
> > > Is this the GSI ?
> > > 
> > > Please check GSI in "InfiniBandTM Architecture Specification Volume 1
> > > Release 1.3"
> > > 
> > > Then make tests with GSI again.
> 
> The followings are also removed by this commit. Not sure if it is good.
> 
> "
> 
> C9-42: If the destination QP is QP1, the BTH:P_Key shall be compared to the
> set of P_Keys associated with the port on which the packet arrived. If the
> P_Key matches any of the keys associated with the port, it shall be
> considered valid.
> 
> "
>

The above is correct for ports that configured to work in InfiniBand
mode, while in RoCEv2 mode only the default P_Key should be associated
with the port (Please see below from "ANNEX A17:   ROCEV2 (IP ROUTABLE
ROCE)):

"""
17.7.1 LOADING THE P_KEY TABLE

Compliance statement C17-7: on page 1193 describes requirements for
setting the P_Key table based on an assumption that the P_Key table is
set directly by a Subnet Manager. However, RoCEv2 ports do not support
InfiniBand Subnet Management. Therefore, compliance statement C17-7:
on page 1193 does not apply to RoCEv2 ports.

Methods for setting the P_Key table associated with a RoCEv2 port are
not defined in this specification, except for the requirements for a
default P_Key described elsewhere in this annex.
"""

Thanks,
Kamal


> > rping uses RDMA CM which goes over the GSI
> > 
> > Jason
> 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: FW: [PATCH for-next] RDMA/rxe: Remove pkey table
  2020-07-28  8:35                 ` Kamal Heib
@ 2020-07-28 13:21                   ` Zhu Yanjun
  2020-07-28 13:44                     ` Kamal Heib
  0 siblings, 1 reply; 16+ messages in thread
From: Zhu Yanjun @ 2020-07-28 13:21 UTC (permalink / raw)
  To: Kamal Heib; +Cc: Jason Gunthorpe, Yanjun Zhu, linux-rdma, Doug Ledford

On 7/28/2020 4:35 PM, Kamal Heib wrote:
> On Thu, Jul 23, 2020 at 11:15:00PM +0800, Zhu Yanjun wrote:
>> On 7/23/2020 9:15 PM, Jason Gunthorpe wrote:
>>> On Thu, Jul 23, 2020 at 09:08:39PM +0800, Zhu Yanjun wrote:
>>>> On 7/23/2020 3:25 PM, Kamal Heib wrote:
>>>>> On Thu, Jul 23, 2020 at 02:58:41PM +0800, Zhu Yanjun wrote:
>>>>>> On 7/23/2020 1:57 PM, Kamal Heib wrote:
>>>>>>> On Wed, Jul 22, 2020 at 10:09:04AM +0800, Zhu Yanjun wrote:
>>>>>>>> On Tue, Jul 21, 2020 at 7:28 PM Yanjun Zhu <yanjunz@mellanox.com> wrote:
>>>>>>>>> From: Kamal Heib <kamalheib1@gmail.com>
>>>>>>>>> Sent: Tuesday, July 21, 2020 6:16 PM
>>>>>>>>> To: linux-rdma@vger.kernel.org
>>>>>>>>> Cc: Yanjun Zhu <yanjunz@mellanox.com>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; Kamal Heib <kamalheib1@gmail.com>
>>>>>>>>> Subject: [PATCH for-next] RDMA/rxe: Remove pkey table
>>>>>>>>>
>>>>>>>>> The RoCE spec require from RoCE devices to support only the defualt pkey, While the rxe driver maintain a 64 enties pkey table and use only the first entry. With that said remove the maintaing of the pkey table and used the default pkey when needed.
>>>>>>>>>
>>>>>>>> Hi Kamal
>>>>>>>>
>>>>>>>> After this patch is applied, do you make tests with SoftRoCE and mlx hardware?
>>>>>>>>
>>>>>>>> The SoftRoCE should work well with the mlx hardware.
>>>>>>>>
>>>>>>>> Zhu Yanjun
>>>>>>>>
>>>>>>> Hi Zhu,
>>>>>>>
>>>>>>> Yes, please see below:
>>>>>>>
>>>>>>> $ ibv_rc_pingpong -d mlx5_0 -g 11
>>>>>>>       local address:  LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
>>>>>> Can you make tests with GSI QP?
>>>>>>
>>>>>> Zhu Yanjun
>>>>>>
>>>> Is this the GSI ?
>>>>
>>>> Please check GSI in "InfiniBandTM Architecture Specification Volume 1
>>>> Release 1.3"
>>>>
>>>> Then make tests with GSI again.
>> The followings are also removed by this commit. Not sure if it is good.
>>
>> "
>>
>> C9-42: If the destination QP is QP1, the BTH:P_Key shall be compared to the
>> set of P_Keys associated with the port on which the packet arrived. If the
>> P_Key matches any of the keys associated with the port, it shall be
>> considered valid.
>>
>> "
>>
> The above is correct for ports that configured to work in InfiniBand
> mode, while in RoCEv2 mode only the default P_Key should be associated
> with the port (Please see below from "ANNEX A17:   ROCEV2 (IP ROUTABLE
> ROCE)):
>
> """
> 17.7.1 LOADING THE P_KEY TABLE
>
> Compliance statement C17-7: on page 1193 describes requirements for
> setting the P_Key table based on an assumption that the P_Key table is
> set directly by a Subnet Manager. However, RoCEv2 ports do not support
> InfiniBand Subnet Management. Therefore, compliance statement C17-7:
> on page 1193 does not apply to RoCEv2 ports.

"

C17-7: An HCA shall require no OS involvement to set the P_Key table;

the P_Key table shall be set directly by Subnet Manager MADs.

"

In SoftRoCE, what set the P_Key table?

>
> Methods for setting the P_Key table associated with a RoCEv2 port are
> not defined in this specification, except for the requirements for a
> default P_Key described elsewhere in this annex.
> """
>
> Thanks,
> Kamal
>
>
>>> rping uses RDMA CM which goes over the GSI
>>>
>>> Jason
>>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: FW: [PATCH for-next] RDMA/rxe: Remove pkey table
  2020-07-28 13:21                   ` Zhu Yanjun
@ 2020-07-28 13:44                     ` Kamal Heib
  2020-07-28 15:46                       ` Zhu Yanjun
  0 siblings, 1 reply; 16+ messages in thread
From: Kamal Heib @ 2020-07-28 13:44 UTC (permalink / raw)
  To: Zhu Yanjun; +Cc: Jason Gunthorpe, Yanjun Zhu, linux-rdma, Doug Ledford

On Tue, Jul 28, 2020 at 09:21:06PM +0800, Zhu Yanjun wrote:
> On 7/28/2020 4:35 PM, Kamal Heib wrote:
> > On Thu, Jul 23, 2020 at 11:15:00PM +0800, Zhu Yanjun wrote:
> > > On 7/23/2020 9:15 PM, Jason Gunthorpe wrote:
> > > > On Thu, Jul 23, 2020 at 09:08:39PM +0800, Zhu Yanjun wrote:
> > > > > On 7/23/2020 3:25 PM, Kamal Heib wrote:
> > > > > > On Thu, Jul 23, 2020 at 02:58:41PM +0800, Zhu Yanjun wrote:
> > > > > > > On 7/23/2020 1:57 PM, Kamal Heib wrote:
> > > > > > > > On Wed, Jul 22, 2020 at 10:09:04AM +0800, Zhu Yanjun wrote:
> > > > > > > > > On Tue, Jul 21, 2020 at 7:28 PM Yanjun Zhu <yanjunz@mellanox.com> wrote:
> > > > > > > > > > From: Kamal Heib <kamalheib1@gmail.com>
> > > > > > > > > > Sent: Tuesday, July 21, 2020 6:16 PM
> > > > > > > > > > To: linux-rdma@vger.kernel.org
> > > > > > > > > > Cc: Yanjun Zhu <yanjunz@mellanox.com>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; Kamal Heib <kamalheib1@gmail.com>
> > > > > > > > > > Subject: [PATCH for-next] RDMA/rxe: Remove pkey table
> > > > > > > > > > 
> > > > > > > > > > The RoCE spec require from RoCE devices to support only the defualt pkey, While the rxe driver maintain a 64 enties pkey table and use only the first entry. With that said remove the maintaing of the pkey table and used the default pkey when needed.
> > > > > > > > > > 
> > > > > > > > > Hi Kamal
> > > > > > > > > 
> > > > > > > > > After this patch is applied, do you make tests with SoftRoCE and mlx hardware?
> > > > > > > > > 
> > > > > > > > > The SoftRoCE should work well with the mlx hardware.
> > > > > > > > > 
> > > > > > > > > Zhu Yanjun
> > > > > > > > > 
> > > > > > > > Hi Zhu,
> > > > > > > > 
> > > > > > > > Yes, please see below:
> > > > > > > > 
> > > > > > > > $ ibv_rc_pingpong -d mlx5_0 -g 11
> > > > > > > >       local address:  LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
> > > > > > > Can you make tests with GSI QP?
> > > > > > > 
> > > > > > > Zhu Yanjun
> > > > > > > 
> > > > > Is this the GSI ?
> > > > > 
> > > > > Please check GSI in "InfiniBandTM Architecture Specification Volume 1
> > > > > Release 1.3"
> > > > > 
> > > > > Then make tests with GSI again.
> > > The followings are also removed by this commit. Not sure if it is good.
> > > 
> > > "
> > > 
> > > C9-42: If the destination QP is QP1, the BTH:P_Key shall be compared to the
> > > set of P_Keys associated with the port on which the packet arrived. If the
> > > P_Key matches any of the keys associated with the port, it shall be
> > > considered valid.
> > > 
> > > "
> > > 
> > The above is correct for ports that configured to work in InfiniBand
> > mode, while in RoCEv2 mode only the default P_Key should be associated
> > with the port (Please see below from "ANNEX A17:   ROCEV2 (IP ROUTABLE
> > ROCE)):
> > 
> > """
> > 17.7.1 LOADING THE P_KEY TABLE
> > 
> > Compliance statement C17-7: on page 1193 describes requirements for
> > setting the P_Key table based on an assumption that the P_Key table is
> > set directly by a Subnet Manager. However, RoCEv2 ports do not support
> > InfiniBand Subnet Management. Therefore, compliance statement C17-7:
> > on page 1193 does not apply to RoCEv2 ports.
> 
> "
> 
> C17-7: An HCA shall require no OS involvement to set the P_Key table;
> 
> the P_Key table shall be set directly by Subnet Manager MADs.
> 
> "
> 
> In SoftRoCE, what set the P_Key table?
>

No one is setting the P_Key table in SoftRoCE, and no subnet manager in
the RoCE fabric.

Could you please tell me what is wrong with this patch?

Thanks,
Kamal

> > 
> > Methods for setting the P_Key table associated with a RoCEv2 port are
> > not defined in this specification, except for the requirements for a
> > default P_Key described elsewhere in this annex.
> > """
> > 
> > Thanks,
> > Kamal
> > 
> > 
> > > > rping uses RDMA CM which goes over the GSI
> > > > 
> > > > Jason
> > > 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: FW: [PATCH for-next] RDMA/rxe: Remove pkey table
  2020-07-28 13:44                     ` Kamal Heib
@ 2020-07-28 15:46                       ` Zhu Yanjun
  2020-07-28 17:42                         ` Kamal Heib
  0 siblings, 1 reply; 16+ messages in thread
From: Zhu Yanjun @ 2020-07-28 15:46 UTC (permalink / raw)
  To: Kamal Heib; +Cc: Jason Gunthorpe, Yanjun Zhu, linux-rdma, Doug Ledford

On 7/28/2020 9:44 PM, Kamal Heib wrote:
> On Tue, Jul 28, 2020 at 09:21:06PM +0800, Zhu Yanjun wrote:
>> On 7/28/2020 4:35 PM, Kamal Heib wrote:
>>> On Thu, Jul 23, 2020 at 11:15:00PM +0800, Zhu Yanjun wrote:
>>>> On 7/23/2020 9:15 PM, Jason Gunthorpe wrote:
>>>>> On Thu, Jul 23, 2020 at 09:08:39PM +0800, Zhu Yanjun wrote:
>>>>>> On 7/23/2020 3:25 PM, Kamal Heib wrote:
>>>>>>> On Thu, Jul 23, 2020 at 02:58:41PM +0800, Zhu Yanjun wrote:
>>>>>>>> On 7/23/2020 1:57 PM, Kamal Heib wrote:
>>>>>>>>> On Wed, Jul 22, 2020 at 10:09:04AM +0800, Zhu Yanjun wrote:
>>>>>>>>>> On Tue, Jul 21, 2020 at 7:28 PM Yanjun Zhu <yanjunz@mellanox.com> wrote:
>>>>>>>>>>> From: Kamal Heib <kamalheib1@gmail.com>
>>>>>>>>>>> Sent: Tuesday, July 21, 2020 6:16 PM
>>>>>>>>>>> To: linux-rdma@vger.kernel.org
>>>>>>>>>>> Cc: Yanjun Zhu <yanjunz@mellanox.com>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; Kamal Heib <kamalheib1@gmail.com>
>>>>>>>>>>> Subject: [PATCH for-next] RDMA/rxe: Remove pkey table
>>>>>>>>>>>
>>>>>>>>>>> The RoCE spec require from RoCE devices to support only the defualt pkey, While the rxe driver maintain a 64 enties pkey table and use only the first entry. With that said remove the maintaing of the pkey table and used the default pkey when needed.
>>>>>>>>>>>
>>>>>>>>>> Hi Kamal
>>>>>>>>>>
>>>>>>>>>> After this patch is applied, do you make tests with SoftRoCE and mlx hardware?
>>>>>>>>>>
>>>>>>>>>> The SoftRoCE should work well with the mlx hardware.
>>>>>>>>>>
>>>>>>>>>> Zhu Yanjun
>>>>>>>>>>
>>>>>>>>> Hi Zhu,
>>>>>>>>>
>>>>>>>>> Yes, please see below:
>>>>>>>>>
>>>>>>>>> $ ibv_rc_pingpong -d mlx5_0 -g 11
>>>>>>>>>        local address:  LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
>>>>>>>> Can you make tests with GSI QP?
>>>>>>>>
>>>>>>>> Zhu Yanjun
>>>>>>>>
>>>>>> Is this the GSI ?
>>>>>>
>>>>>> Please check GSI in "InfiniBandTM Architecture Specification Volume 1
>>>>>> Release 1.3"
>>>>>>
>>>>>> Then make tests with GSI again.
>>>> The followings are also removed by this commit. Not sure if it is good.
>>>>
>>>> "
>>>>
>>>> C9-42: If the destination QP is QP1, the BTH:P_Key shall be compared to the
>>>> set of P_Keys associated with the port on which the packet arrived. If the
>>>> P_Key matches any of the keys associated with the port, it shall be
>>>> considered valid.
>>>>
>>>> "
>>>>
>>> The above is correct for ports that configured to work in InfiniBand
>>> mode, while in RoCEv2 mode only the default P_Key should be associated
>>> with the port (Please see below from "ANNEX A17:   ROCEV2 (IP ROUTABLE
>>> ROCE)):
>>>
>>> """
>>> 17.7.1 LOADING THE P_KEY TABLE
>>>
>>> Compliance statement C17-7: on page 1193 describes requirements for
>>> setting the P_Key table based on an assumption that the P_Key table is
>>> set directly by a Subnet Manager. However, RoCEv2 ports do not support
>>> InfiniBand Subnet Management. Therefore, compliance statement C17-7:
>>> on page 1193 does not apply to RoCEv2 ports.
>> "
>>
>> C17-7: An HCA shall require no OS involvement to set the P_Key table;
>>
>> the P_Key table shall be set directly by Subnet Manager MADs.
>>
>> "
>>
>> In SoftRoCE, what set the P_Key table?
>>
> No one is setting the P_Key table in SoftRoCE, and no subnet manager in
> the RoCE fabric.
>
> Could you please tell me what is wrong with this patch?

Please read the mail thread again.

GSI QP number is 1. In your commits, the handle of qpn == 1 is removed.

It seems that it conflicts with IB specification.

Not sure if it is good.

>
> Thanks,
> Kamal
>
>>> Methods for setting the P_Key table associated with a RoCEv2 port are
>>> not defined in this specification, except for the requirements for a
>>> default P_Key described elsewhere in this annex.
>>> """
>>>
>>> Thanks,
>>> Kamal
>>>
>>>
>>>>> rping uses RDMA CM which goes over the GSI
>>>>>
>>>>> Jason



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: FW: [PATCH for-next] RDMA/rxe: Remove pkey table
  2020-07-28 15:46                       ` Zhu Yanjun
@ 2020-07-28 17:42                         ` Kamal Heib
  2020-07-28 23:45                           ` Zhu Yanjun
  0 siblings, 1 reply; 16+ messages in thread
From: Kamal Heib @ 2020-07-28 17:42 UTC (permalink / raw)
  To: Zhu Yanjun; +Cc: Jason Gunthorpe, Yanjun Zhu, linux-rdma, Doug Ledford

On Tue, Jul 28, 2020 at 11:46:36PM +0800, Zhu Yanjun wrote:
> On 7/28/2020 9:44 PM, Kamal Heib wrote:
> > On Tue, Jul 28, 2020 at 09:21:06PM +0800, Zhu Yanjun wrote:
> > > On 7/28/2020 4:35 PM, Kamal Heib wrote:
> > > > On Thu, Jul 23, 2020 at 11:15:00PM +0800, Zhu Yanjun wrote:
> > > > > On 7/23/2020 9:15 PM, Jason Gunthorpe wrote:
> > > > > > On Thu, Jul 23, 2020 at 09:08:39PM +0800, Zhu Yanjun wrote:
> > > > > > > On 7/23/2020 3:25 PM, Kamal Heib wrote:
> > > > > > > > On Thu, Jul 23, 2020 at 02:58:41PM +0800, Zhu Yanjun wrote:
> > > > > > > > > On 7/23/2020 1:57 PM, Kamal Heib wrote:
> > > > > > > > > > On Wed, Jul 22, 2020 at 10:09:04AM +0800, Zhu Yanjun wrote:
> > > > > > > > > > > On Tue, Jul 21, 2020 at 7:28 PM Yanjun Zhu <yanjunz@mellanox.com> wrote:
> > > > > > > > > > > > From: Kamal Heib <kamalheib1@gmail.com>
> > > > > > > > > > > > Sent: Tuesday, July 21, 2020 6:16 PM
> > > > > > > > > > > > To: linux-rdma@vger.kernel.org
> > > > > > > > > > > > Cc: Yanjun Zhu <yanjunz@mellanox.com>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; Kamal Heib <kamalheib1@gmail.com>
> > > > > > > > > > > > Subject: [PATCH for-next] RDMA/rxe: Remove pkey table
> > > > > > > > > > > > 
> > > > > > > > > > > > The RoCE spec require from RoCE devices to support only the defualt pkey, While the rxe driver maintain a 64 enties pkey table and use only the first entry. With that said remove the maintaing of the pkey table and used the default pkey when needed.
> > > > > > > > > > > > 
> > > > > > > > > > > Hi Kamal
> > > > > > > > > > > 
> > > > > > > > > > > After this patch is applied, do you make tests with SoftRoCE and mlx hardware?
> > > > > > > > > > > 
> > > > > > > > > > > The SoftRoCE should work well with the mlx hardware.
> > > > > > > > > > > 
> > > > > > > > > > > Zhu Yanjun
> > > > > > > > > > > 
> > > > > > > > > > Hi Zhu,
> > > > > > > > > > 
> > > > > > > > > > Yes, please see below:
> > > > > > > > > > 
> > > > > > > > > > $ ibv_rc_pingpong -d mlx5_0 -g 11
> > > > > > > > > >        local address:  LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
> > > > > > > > > Can you make tests with GSI QP?
> > > > > > > > > 
> > > > > > > > > Zhu Yanjun
> > > > > > > > > 
> > > > > > > Is this the GSI ?
> > > > > > > 
> > > > > > > Please check GSI in "InfiniBandTM Architecture Specification Volume 1
> > > > > > > Release 1.3"
> > > > > > > 
> > > > > > > Then make tests with GSI again.
> > > > > The followings are also removed by this commit. Not sure if it is good.
> > > > > 
> > > > > "
> > > > > 
> > > > > C9-42: If the destination QP is QP1, the BTH:P_Key shall be compared to the
> > > > > set of P_Keys associated with the port on which the packet arrived. If the
> > > > > P_Key matches any of the keys associated with the port, it shall be
> > > > > considered valid.
> > > > > 
> > > > > "
> > > > > 
> > > > The above is correct for ports that configured to work in InfiniBand
> > > > mode, while in RoCEv2 mode only the default P_Key should be associated
> > > > with the port (Please see below from "ANNEX A17:   ROCEV2 (IP ROUTABLE
> > > > ROCE)):
> > > > 
> > > > """
> > > > 17.7.1 LOADING THE P_KEY TABLE
> > > > 
> > > > Compliance statement C17-7: on page 1193 describes requirements for
> > > > setting the P_Key table based on an assumption that the P_Key table is
> > > > set directly by a Subnet Manager. However, RoCEv2 ports do not support
> > > > InfiniBand Subnet Management. Therefore, compliance statement C17-7:
> > > > on page 1193 does not apply to RoCEv2 ports.
> > > "
> > > 
> > > C17-7: An HCA shall require no OS involvement to set the P_Key table;
> > > 
> > > the P_Key table shall be set directly by Subnet Manager MADs.
> > > 
> > > "
> > > 
> > > In SoftRoCE, what set the P_Key table?
> > > 
> > No one is setting the P_Key table in SoftRoCE, and no subnet manager in
> > the RoCE fabric.
> > 
> > Could you please tell me what is wrong with this patch?
> 
> Please read the mail thread again.
> 
> GSI QP number is 1. In your commits, the handle of qpn == 1 is removed.
> 
> It seems that it conflicts with IB specification.
> 
> Not sure if it is good.
>

Could you please read my patch again and point to what do you think is
wrong?

What I did in this patch is to verify that the pkey value in the
received packet is the default P_Key regardless of the qpn, because RoCE
devices should maintain only the default P_Key.

Thanks,
Kamal

> > 
> > Thanks,
> > Kamal
> > 
> > > > Methods for setting the P_Key table associated with a RoCEv2 port are
> > > > not defined in this specification, except for the requirements for a
> > > > default P_Key described elsewhere in this annex.
> > > > """
> > > > 
> > > > Thanks,
> > > > Kamal
> > > > 
> > > > 
> > > > > > rping uses RDMA CM which goes over the GSI
> > > > > > 
> > > > > > Jason
> 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: FW: [PATCH for-next] RDMA/rxe: Remove pkey table
  2020-07-28 17:42                         ` Kamal Heib
@ 2020-07-28 23:45                           ` Zhu Yanjun
  2020-07-29  1:36                             ` Mark Bloch
  0 siblings, 1 reply; 16+ messages in thread
From: Zhu Yanjun @ 2020-07-28 23:45 UTC (permalink / raw)
  To: Kamal Heib; +Cc: Jason Gunthorpe, Yanjun Zhu, linux-rdma, Doug Ledford

On 7/29/2020 1:42 AM, Kamal Heib wrote:
> On Tue, Jul 28, 2020 at 11:46:36PM +0800, Zhu Yanjun wrote:
>> On 7/28/2020 9:44 PM, Kamal Heib wrote:
>>> On Tue, Jul 28, 2020 at 09:21:06PM +0800, Zhu Yanjun wrote:
>>>> On 7/28/2020 4:35 PM, Kamal Heib wrote:
>>>>> On Thu, Jul 23, 2020 at 11:15:00PM +0800, Zhu Yanjun wrote:
>>>>>> On 7/23/2020 9:15 PM, Jason Gunthorpe wrote:
>>>>>>> On Thu, Jul 23, 2020 at 09:08:39PM +0800, Zhu Yanjun wrote:
>>>>>>>> On 7/23/2020 3:25 PM, Kamal Heib wrote:
>>>>>>>>> On Thu, Jul 23, 2020 at 02:58:41PM +0800, Zhu Yanjun wrote:
>>>>>>>>>> On 7/23/2020 1:57 PM, Kamal Heib wrote:
>>>>>>>>>>> On Wed, Jul 22, 2020 at 10:09:04AM +0800, Zhu Yanjun wrote:
>>>>>>>>>>>> On Tue, Jul 21, 2020 at 7:28 PM Yanjun Zhu <yanjunz@mellanox.com> wrote:
>>>>>>>>>>>>> From: Kamal Heib <kamalheib1@gmail.com>
>>>>>>>>>>>>> Sent: Tuesday, July 21, 2020 6:16 PM
>>>>>>>>>>>>> To: linux-rdma@vger.kernel.org
>>>>>>>>>>>>> Cc: Yanjun Zhu <yanjunz@mellanox.com>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; Kamal Heib <kamalheib1@gmail.com>
>>>>>>>>>>>>> Subject: [PATCH for-next] RDMA/rxe: Remove pkey table
>>>>>>>>>>>>>
>>>>>>>>>>>>> The RoCE spec require from RoCE devices to support only the defualt pkey, While the rxe driver maintain a 64 enties pkey table and use only the first entry. With that said remove the maintaing of the pkey table and used the default pkey when needed.
>>>>>>>>>>>>>
>>>>>>>>>>>> Hi Kamal
>>>>>>>>>>>>
>>>>>>>>>>>> After this patch is applied, do you make tests with SoftRoCE and mlx hardware?
>>>>>>>>>>>>
>>>>>>>>>>>> The SoftRoCE should work well with the mlx hardware.
>>>>>>>>>>>>
>>>>>>>>>>>> Zhu Yanjun
>>>>>>>>>>>>
>>>>>>>>>>> Hi Zhu,
>>>>>>>>>>>
>>>>>>>>>>> Yes, please see below:
>>>>>>>>>>>
>>>>>>>>>>> $ ibv_rc_pingpong -d mlx5_0 -g 11
>>>>>>>>>>>         local address:  LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
>>>>>>>>>> Can you make tests with GSI QP?
>>>>>>>>>>
>>>>>>>>>> Zhu Yanjun
>>>>>>>>>>
>>>>>>>> Is this the GSI ?
>>>>>>>>
>>>>>>>> Please check GSI in "InfiniBandTM Architecture Specification Volume 1
>>>>>>>> Release 1.3"
>>>>>>>>
>>>>>>>> Then make tests with GSI again.
>>>>>> The followings are also removed by this commit. Not sure if it is good.
>>>>>>
>>>>>> "
>>>>>>
>>>>>> C9-42: If the destination QP is QP1, the BTH:P_Key shall be compared to the
>>>>>> set of P_Keys associated with the port on which the packet arrived. If the
>>>>>> P_Key matches any of the keys associated with the port, it shall be
>>>>>> considered valid.
>>>>>>
>>>>>> "
>>>>>>
>>>>> The above is correct for ports that configured to work in InfiniBand
>>>>> mode, while in RoCEv2 mode only the default P_Key should be associated
>>>>> with the port (Please see below from "ANNEX A17:   ROCEV2 (IP ROUTABLE
>>>>> ROCE)):
>>>>>
>>>>> """
>>>>> 17.7.1 LOADING THE P_KEY TABLE
>>>>>
>>>>> Compliance statement C17-7: on page 1193 describes requirements for
>>>>> setting the P_Key table based on an assumption that the P_Key table is
>>>>> set directly by a Subnet Manager. However, RoCEv2 ports do not support
>>>>> InfiniBand Subnet Management. Therefore, compliance statement C17-7:
>>>>> on page 1193 does not apply to RoCEv2 ports.
>>>> "
>>>>
>>>> C17-7: An HCA shall require no OS involvement to set the P_Key table;
>>>>
>>>> the P_Key table shall be set directly by Subnet Manager MADs.
>>>>
>>>> "
>>>>
>>>> In SoftRoCE, what set the P_Key table?
>>>>
>>> No one is setting the P_Key table in SoftRoCE, and no subnet manager in
>>> the RoCE fabric.
>>>
>>> Could you please tell me what is wrong with this patch?
>> Please read the mail thread again.
>>
>> GSI QP number is 1. In your commits, the handle of qpn == 1 is removed.
>>
>> It seems that it conflicts with IB specification.
>>
>> Not sure if it is good.
>>
> Could you please read my patch again and point to what do you think is
> wrong?

What I said is very clear. Good luck

Zhu Yanjun

>
> What I did in this patch is to verify that the pkey value in the
> received packet is the default P_Key regardless of the qpn, because RoCE
> devices should maintain only the default P_Key.
>
> Thanks,
> Kamal
>
>>> Thanks,
>>> Kamal
>>>
>>>>> Methods for setting the P_Key table associated with a RoCEv2 port are
>>>>> not defined in this specification, except for the requirements for a
>>>>> default P_Key described elsewhere in this annex.
>>>>> """
>>>>>
>>>>> Thanks,
>>>>> Kamal
>>>>>
>>>>>
>>>>>>> rping uses RDMA CM which goes over the GSI
>>>>>>>
>>>>>>> Jason
>>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: FW: [PATCH for-next] RDMA/rxe: Remove pkey table
  2020-07-28 23:45                           ` Zhu Yanjun
@ 2020-07-29  1:36                             ` Mark Bloch
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Bloch @ 2020-07-29  1:36 UTC (permalink / raw)
  To: Zhu Yanjun, Kamal Heib
  Cc: Jason Gunthorpe, Yanjun Zhu, linux-rdma, Doug Ledford



On 7/28/2020 16:45, Zhu Yanjun wrote:
> On 7/29/2020 1:42 AM, Kamal Heib wrote:
>> On Tue, Jul 28, 2020 at 11:46:36PM +0800, Zhu Yanjun wrote:
>>> On 7/28/2020 9:44 PM, Kamal Heib wrote:
>>>> On Tue, Jul 28, 2020 at 09:21:06PM +0800, Zhu Yanjun wrote:
>>>>> On 7/28/2020 4:35 PM, Kamal Heib wrote:
>>>>>> On Thu, Jul 23, 2020 at 11:15:00PM +0800, Zhu Yanjun wrote:
>>>>>>> On 7/23/2020 9:15 PM, Jason Gunthorpe wrote:
>>>>>>>> On Thu, Jul 23, 2020 at 09:08:39PM +0800, Zhu Yanjun wrote:
>>>>>>>>> On 7/23/2020 3:25 PM, Kamal Heib wrote:
>>>>>>>>>> On Thu, Jul 23, 2020 at 02:58:41PM +0800, Zhu Yanjun wrote:
>>>>>>>>>>> On 7/23/2020 1:57 PM, Kamal Heib wrote:
>>>>>>>>>>>> On Wed, Jul 22, 2020 at 10:09:04AM +0800, Zhu Yanjun wrote:
>>>>>>>>>>>>> On Tue, Jul 21, 2020 at 7:28 PM Yanjun Zhu <yanjunz@mellanox.com> wrote:
>>>>>>>>>>>>>> From: Kamal Heib <kamalheib1@gmail.com>
>>>>>>>>>>>>>> Sent: Tuesday, July 21, 2020 6:16 PM
>>>>>>>>>>>>>> To: linux-rdma@vger.kernel.org
>>>>>>>>>>>>>> Cc: Yanjun Zhu <yanjunz@mellanox.com>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@ziepe.ca>; Kamal Heib <kamalheib1@gmail.com>
>>>>>>>>>>>>>> Subject: [PATCH for-next] RDMA/rxe: Remove pkey table
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The RoCE spec require from RoCE devices to support only the defualt pkey, While the rxe driver maintain a 64 enties pkey table and use only the first entry. With that said remove the maintaing of the pkey table and used the default pkey when needed.
>>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Kamal
>>>>>>>>>>>>>
>>>>>>>>>>>>> After this patch is applied, do you make tests with SoftRoCE and mlx hardware?
>>>>>>>>>>>>>
>>>>>>>>>>>>> The SoftRoCE should work well with the mlx hardware.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Zhu Yanjun
>>>>>>>>>>>>>
>>>>>>>>>>>> Hi Zhu,
>>>>>>>>>>>>
>>>>>>>>>>>> Yes, please see below:
>>>>>>>>>>>>
>>>>>>>>>>>> $ ibv_rc_pingpong -d mlx5_0 -g 11
>>>>>>>>>>>>         local address:  LID 0x0000, QPN 0x0000e3, PSN 0x728a4f, GID ::ffff:172.31.40.121
>>>>>>>>>>> Can you make tests with GSI QP?
>>>>>>>>>>>
>>>>>>>>>>> Zhu Yanjun
>>>>>>>>>>>
>>>>>>>>> Is this the GSI ?
>>>>>>>>>
>>>>>>>>> Please check GSI in "InfiniBandTM Architecture Specification Volume 1
>>>>>>>>> Release 1.3"
>>>>>>>>>
>>>>>>>>> Then make tests with GSI again.
>>>>>>> The followings are also removed by this commit. Not sure if it is good.
>>>>>>>
>>>>>>> "
>>>>>>>
>>>>>>> C9-42: If the destination QP is QP1, the BTH:P_Key shall be compared to the
>>>>>>> set of P_Keys associated with the port on which the packet arrived. If the
>>>>>>> P_Key matches any of the keys associated with the port, it shall be
>>>>>>> considered valid.
>>>>>>>
>>>>>>> "
>>>>>>>
>>>>>> The above is correct for ports that configured to work in InfiniBand
>>>>>> mode, while in RoCEv2 mode only the default P_Key should be associated
>>>>>> with the port (Please see below from "ANNEX A17:   ROCEV2 (IP ROUTABLE
>>>>>> ROCE)):
>>>>>>
>>>>>> """
>>>>>> 17.7.1 LOADING THE P_KEY TABLE
>>>>>>
>>>>>> Compliance statement C17-7: on page 1193 describes requirements for
>>>>>> setting the P_Key table based on an assumption that the P_Key table is
>>>>>> set directly by a Subnet Manager. However, RoCEv2 ports do not support
>>>>>> InfiniBand Subnet Management. Therefore, compliance statement C17-7:
>>>>>> on page 1193 does not apply to RoCEv2 ports.
>>>>> "
>>>>>
>>>>> C17-7: An HCA shall require no OS involvement to set the P_Key table;
>>>>>
>>>>> the P_Key table shall be set directly by Subnet Manager MADs.
>>>>>
>>>>> "
>>>>>
>>>>> In SoftRoCE, what set the P_Key table?
>>>>>
>>>> No one is setting the P_Key table in SoftRoCE, and no subnet manager in
>>>> the RoCE fabric.
>>>>
>>>> Could you please tell me what is wrong with this patch?
>>> Please read the mail thread again.
>>>
>>> GSI QP number is 1. In your commits, the handle of qpn == 1 is removed.
>>>
>>> It seems that it conflicts with IB specification.
>>>
>>> Not sure if it is good.
>>>
>> Could you please read my patch again and point to what do you think is
>> wrong?
> 
> What I said is very clear. Good luck

qpn == 1, qpn == x it, qpn == i, it doesn't matter. Please read the RoCEv2 annex:

A17.4.7 INFINIBAND PARTITIONING
Methods to populate the P_Key table associated with a RoCEv2 port are
outside the scope of this annex. Note that this annex relies on the partition
table being initialized at power on time with at least the default P_Key as
described in Chapter 10 (Software Transport Interface) of the base specification. The P_Key contained in the BTH is validated for inbound packets
as required by the packet header validation protocols defined in Chapter
9 of the base specification.

A17.7.1 LOADING THE P_KEY TABLE
Compliance statement C17-7 describes requirements for setting the
P_Key table based on an assumption that the P_Key table is set directly
by a Subnet Manager. However, RoCEv2 ports do not support InfiniBand
Subnet Management. Therefore, compliance statement C17-7 does not
apply to RoCEv2 ports.
Methods for setting the P_Key table associated with a RoCEv2 port are
not defined in this specification, except for the requirements for a default
P_Key described elsewhere in this annex.

rxe =  Software RDMA over Ethernet, so we are dealing only with RoCE traffic (no IB).
We don't have an SM.
The spec requires that at least the default pkey is defined (and rxe defines only the default p_pkey).
Kamel is doing just that.

Mark

> 
> Zhu Yanjun
> 
>>
>> What I did in this patch is to verify that the pkey value in the
>> received packet is the default P_Key regardless of the qpn, because RoCE
>> devices should maintain only the default P_Key.
>>
>> Thanks,
>> Kamal
>>
>>>> Thanks,
>>>> Kamal
>>>>
>>>>>> Methods for setting the P_Key table associated with a RoCEv2 port are
>>>>>> not defined in this specification, except for the requirements for a
>>>>>> default P_Key described elsewhere in this annex.
>>>>>> """
>>>>>>
>>>>>> Thanks,
>>>>>> Kamal
>>>>>>
>>>>>>
>>>>>>>> rping uses RDMA CM which goes over the GSI
>>>>>>>>
>>>>>>>> Jason
>>>
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH for-next] RDMA/rxe: Remove pkey table
  2020-07-21 10:16 [PATCH for-next] RDMA/rxe: Remove pkey table Kamal Heib
       [not found] ` <AM6PR05MB6263CFB337190B1740CDF4B7D8780@AM6PR05MB6263.eurprd05.prod.outlook.com>
@ 2020-07-31 19:22 ` Jason Gunthorpe
  1 sibling, 0 replies; 16+ messages in thread
From: Jason Gunthorpe @ 2020-07-31 19:22 UTC (permalink / raw)
  To: Kamal Heib; +Cc: linux-rdma, Zhu Yanjun, Doug Ledford

On Tue, Jul 21, 2020 at 01:16:18PM +0300, Kamal Heib wrote:
> The RoCE spec require from RoCE devices to support only the defualt pkey,
> While the rxe driver maintain a 64 enties pkey table and use only the
> first entry. With that said remove the maintaing of the pkey table and
> used the default pkey when needed.
> 
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
> ---
>  drivers/infiniband/sw/rxe/rxe.c       | 34 +++------------------------
>  drivers/infiniband/sw/rxe/rxe_param.h |  4 ++--
>  drivers/infiniband/sw/rxe/rxe_recv.c  | 29 ++++-------------------
>  drivers/infiniband/sw/rxe/rxe_req.c   |  5 +---
>  drivers/infiniband/sw/rxe/rxe_verbs.c | 17 +++-----------
>  drivers/infiniband/sw/rxe/rxe_verbs.h |  1 -
>  6 files changed, 13 insertions(+), 77 deletions(-)

It looks OK to me, but this is not a 'fixes' so I dropped it.

There is no reason a rxe device should have anything other than a
single entry pkey table with the default value

Applied to for-next

Thanks,
Jason

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-07-31 19:22 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-21 10:16 [PATCH for-next] RDMA/rxe: Remove pkey table Kamal Heib
     [not found] ` <AM6PR05MB6263CFB337190B1740CDF4B7D8780@AM6PR05MB6263.eurprd05.prod.outlook.com>
2020-07-22  2:09   ` FW: " Zhu Yanjun
2020-07-23  5:57     ` Kamal Heib
2020-07-23  6:58       ` Zhu Yanjun
2020-07-23  7:25         ` Kamal Heib
2020-07-23 13:08           ` Zhu Yanjun
2020-07-23 13:15             ` Jason Gunthorpe
2020-07-23 15:15               ` Zhu Yanjun
2020-07-28  8:35                 ` Kamal Heib
2020-07-28 13:21                   ` Zhu Yanjun
2020-07-28 13:44                     ` Kamal Heib
2020-07-28 15:46                       ` Zhu Yanjun
2020-07-28 17:42                         ` Kamal Heib
2020-07-28 23:45                           ` Zhu Yanjun
2020-07-29  1:36                             ` Mark Bloch
2020-07-31 19:22 ` Jason Gunthorpe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).