| Message ID | 1504786327-820-2-git-send-email-yishaih@mellanox.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, Sep 07, 2017 at 03:12:05PM +0300, Yishai Hadas wrote: > diff --git a/libibverbs/man/ibv_create_qp_ex.3 b/libibverbs/man/ibv_create_qp_ex.3 > index 99ae497..3272c09 100644 > +++ b/libibverbs/man/ibv_create_qp_ex.3 > @@ -38,6 +38,7 @@ enum ibv_qp_create_flags create_flags; /* Creation flags for this QP */ > uint16_t max_tso_header; /* Maximum TSO header size */ > struct ibv_rwq_ind_table *rwq_ind_tbl; /* Indirection table to be associated with the QP */ > struct ibv_rx_hash_conf rx_hash_conf; /* RX hash configuration to be used */ > +uint32_t source_qpn; /* Source QP number, creation flag IBV_QP_CREATE_SOURCE_QPN should be set */ I think more discussion in the man page is required for this feature. Eg that it is a SEND ONLY QP and only works for UD. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 7, 2017 at 6:16 PM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > On Thu, Sep 07, 2017 at 03:12:05PM +0300, Yishai Hadas wrote: >> +uint32_t source_qpn; /* Source QP number, creation flag IBV_QP_CREATE_SOURCE_QPN should be set */ > > I think more discussion in the man page is required for this > feature. Eg that it is a SEND ONLY QP and only works for UD. Verbs UD QP can handle receive of ingress packets sent with BTH.DestQPN = ibv_qp->qp_num. This is applicable also in this case. Also, future implementation might include RC source qpn use case. This can be accomplished by setting the HCA's QPN to be the same as the 'source qpn' value, and returning that to the user. We prefer not to limit the API definition in the man page. Alex -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 07, 2017 at 09:46:39PM +0300, Alex Rosenbaum wrote: > On Thu, Sep 7, 2017 at 6:16 PM, Jason Gunthorpe > <jgunthorpe@obsidianresearch.com> wrote: > > On Thu, Sep 07, 2017 at 03:12:05PM +0300, Yishai Hadas wrote: > >> +uint32_t source_qpn; /* Source QP number, creation flag IBV_QP_CREATE_SOURCE_QPN should be set */ > > > > I think more discussion in the man page is required for this > > feature. Eg that it is a SEND ONLY QP and only works for UD. > > Verbs UD QP can handle receive of ingress packets sent with > BTH.DestQPN = ibv_qp->qp_num. This is applicable also in this case. > > Also, future implementation might include RC source qpn use case. This > can be accomplished by setting the HCA's QPN to be the same as the > 'source qpn' value, and returning that to the user. > > We prefer not to limit the API definition in the man page. Well, you have to define what the API does, specifically. If you expect it to RX then you need to specify that. Does it steal the traffic? Does it get a copy? Do you need to use flow steering to do that? Same for RC. There is nothing wrong with specifying the API limit that exist today, it helps people understand what the feature is and how to use it. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 7, 2017 at 10:29 PM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > On Thu, Sep 07, 2017 at 09:46:39PM +0300, Alex Rosenbaum wrote: >> On Thu, Sep 7, 2017 at 6:16 PM, Jason Gunthorpe >> <jgunthorpe@obsidianresearch.com> wrote: >> > On Thu, Sep 07, 2017 at 03:12:05PM +0300, Yishai Hadas wrote: >> >> +uint32_t source_qpn; /* Source QP number, creation flag IBV_QP_CREATE_SOURCE_QPN should be set */ >> > >> > I think more discussion in the man page is required for this >> > feature. Eg that it is a SEND ONLY QP and only works for UD. >> >> Verbs UD QP can handle receive of ingress packets sent with >> BTH.DestQPN = ibv_qp->qp_num. This is applicable also in this case. >> >> Also, future implementation might include RC source qpn use case. This >> can be accomplished by setting the HCA's QPN to be the same as the >> 'source qpn' value, and returning that to the user. >> >> We prefer not to limit the API definition in the man page. > > Well, you have to define what the API does, specifically. > > If you expect it to RX then you need to specify that. Like any other QP, this too can have a max_recv_wr defined by user. I don't think we need to specifically mention it, because it is not a limition. > Does it steal > the traffic? Does it get a copy? Do you need to use flow steering to > do that? Yes, this QP can support flow steering based on IBV_DEVICE_MANAGED_FLOW_STEERING. > Same for RC. > > There is nothing wrong with specifying the API limit that exist today, > it helps people understand what the feature is and how to use it. RC is currently not supported. Do you prefer we update the man page with the UD limitation? and once RC or other limitation are removed we update it again? Alex -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Sep 10, 2017 at 09:54:45AM +0300, Alex Rosenbaum wrote: > On Thu, Sep 7, 2017 at 10:29 PM, Jason Gunthorpe > <jgunthorpe@obsidianresearch.com> wrote: > > On Thu, Sep 07, 2017 at 09:46:39PM +0300, Alex Rosenbaum wrote: > >> On Thu, Sep 7, 2017 at 6:16 PM, Jason Gunthorpe > >> <jgunthorpe@obsidianresearch.com> wrote: > >> > On Thu, Sep 07, 2017 at 03:12:05PM +0300, Yishai Hadas wrote: > >> >> +uint32_t source_qpn; /* Source QP number, creation flag IBV_QP_CREATE_SOURCE_QPN should be set */ > >> > > >> > I think more discussion in the man page is required for this > >> > feature. Eg that it is a SEND ONLY QP and only works for UD. > >> > >> Verbs UD QP can handle receive of ingress packets sent with > >> BTH.DestQPN = ibv_qp->qp_num. This is applicable also in this case. > >> > >> Also, future implementation might include RC source qpn use case. This > >> can be accomplished by setting the HCA's QPN to be the same as the > >> 'source qpn' value, and returning that to the user. > >> > >> We prefer not to limit the API definition in the man page. > > > > Well, you have to define what the API does, specifically. > > > > If you expect it to RX then you need to specify that. > > Like any other QP, this too can have a max_recv_wr defined by user. I > don't think we need to specifically mention it, because it is not a > limition. You need to mention what impact source_qpn has on the RX, assuming the user does nothing else. From what I can gather, without flow steering, RX should not be possible. This needs to be described. > > There is nothing wrong with specifying the API limit that exist today, > > it helps people understand what the feature is and how to use it. > > RC is currently not supported. Do you prefer we update the man page > with the UD limitation? and once RC or other limitation are removed we > update it again? Yes. Describe what it does. The man page is not a spec, it documents what the software actually does. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 9/11/2017 7:28 PM, Jason Gunthorpe wrote: > You need to mention what impact source_qpn has on the RX, assuming the > user does nothing else. > > From what I can gather, without flow steering, RX should not be > possible. > Correct. > Yes. Describe what it does. The man page is not a spec, it documents > what the software actually does. > Man page was updated in the PR to include above notes. PR: https://github.com/linux-rdma/rdma-core/pull/212 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/libibverbs/cmd.c b/libibverbs/cmd.c index 7a266ee..9d2140f 100644 --- a/libibverbs/cmd.c +++ b/libibverbs/cmd.c @@ -927,7 +927,8 @@ static void create_qp_handle_resp_common(struct ibv_context *context, enum { CREATE_QP_EX2_SUP_CREATE_FLAGS = IBV_QP_CREATE_BLOCK_SELF_MCAST_LB | IBV_QP_CREATE_SCATTER_FCS | - IBV_QP_CREATE_CVLAN_STRIPPING, + IBV_QP_CREATE_CVLAN_STRIPPING | + IBV_QP_CREATE_SOURCE_QPN, }; int ibv_cmd_create_qp_ex2(struct ibv_context *context, @@ -967,6 +968,9 @@ int ibv_cmd_create_qp_ex2(struct ibv_context *context, sizeof(qp_attr->create_flags)) return EINVAL; cmd->create_flags = qp_attr->create_flags; + + if (qp_attr->create_flags & IBV_QP_CREATE_SOURCE_QPN) + cmd->source_qpn = qp_attr->source_qpn; } if (qp_attr->comp_mask & IBV_QP_INIT_ATTR_IND_TABLE) { diff --git a/libibverbs/kern-abi.h b/libibverbs/kern-abi.h index 8d42faf..3a0e02a 100644 --- a/libibverbs/kern-abi.h +++ b/libibverbs/kern-abi.h @@ -679,7 +679,7 @@ struct ibv_create_qp_ex { __u32 comp_mask; __u32 create_flags; __u32 ind_tbl_handle; - __u32 reserved1; + __u32 source_qpn; }; struct ibv_create_qp_resp_ex { diff --git a/libibverbs/man/ibv_create_qp_ex.3 b/libibverbs/man/ibv_create_qp_ex.3 index 99ae497..3272c09 100644 --- a/libibverbs/man/ibv_create_qp_ex.3 +++ b/libibverbs/man/ibv_create_qp_ex.3 @@ -38,6 +38,7 @@ enum ibv_qp_create_flags create_flags; /* Creation flags for this QP */ uint16_t max_tso_header; /* Maximum TSO header size */ struct ibv_rwq_ind_table *rwq_ind_tbl; /* Indirection table to be associated with the QP */ struct ibv_rx_hash_conf rx_hash_conf; /* RX hash configuration to be used */ +uint32_t source_qpn; /* Source QP number, creation flag IBV_QP_CREATE_SOURCE_QPN should be set */ .in -8 }; .sp diff --git a/libibverbs/verbs.h b/libibverbs/verbs.h index 922a011..3c8b2c9 100644 --- a/libibverbs/verbs.h +++ b/libibverbs/verbs.h @@ -790,6 +790,7 @@ enum ibv_qp_create_flags { IBV_QP_CREATE_BLOCK_SELF_MCAST_LB = 1 << 1, IBV_QP_CREATE_SCATTER_FCS = 1 << 8, IBV_QP_CREATE_CVLAN_STRIPPING = 1 << 9, + IBV_QP_CREATE_SOURCE_QPN = 1 << 10, }; struct ibv_rx_hash_conf { @@ -817,6 +818,7 @@ struct ibv_qp_init_attr_ex { uint16_t max_tso_header; struct ibv_rwq_ind_table *rwq_ind_tbl; struct ibv_rx_hash_conf rx_hash_conf; + uint32_t source_qpn; }; enum ibv_qp_open_attr_mask {
Enable QP creation with a given source QP number. The created QP will use the source QPN as its wire QP number. This comes as a pre-patch for downstream patches in this series to allow user space applications to accelerate traffic which is typically handled by IPoIB ULP. In the IPoIB use case, the source QPN can be achieved by using some networking tool as of ifconfig, ip link show on the interface and use the value of 3 bytes from the hardware address starting after the first byte. Signed-off-by: Yishai Hadas <yishaih@mellanox.com> --- libibverbs/cmd.c | 6 +++++- libibverbs/kern-abi.h | 2 +- libibverbs/man/ibv_create_qp_ex.3 | 1 + libibverbs/verbs.h | 2 ++ 4 files changed, 9 insertions(+), 2 deletions(-)