On Tue, 2017-08-29 at 08:45 +0300, Leon Romanovsky wrote:
> On Mon, Aug 28, 2017 at 05:19:35PM -0700, Adit Ranadive wrote:
> > From: Aditya Sarwade <asarwade@xxxxxxxxxx>
> >
> > We should report the network header type in the work completion so
> > that
> > the kernel can infer the right RoCE type headers.
> >
> > Reviewed-by: Bryan Tan <bryantan@xxxxxxxxxx>
> > Signed-off-by: Aditya Sarwade <asarwade@xxxxxxxxxx>
> > Signed-off-by: Adit Ranadive <aditr@xxxxxxxxxx>
> > ---
> > drivers/infiniband/hw/vmw_pvrdma/pvrdma.h | 6 ++++++
> > drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c | 2 ++
> > include/uapi/rdma/vmw_pvrdma-abi.h | 13 +++++++++++--
> > 3 files changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> > b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> > index 663a0c3..99b2c97 100644
> > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> > @@ -426,6 +426,12 @@ static inline int pvrdma_wc_flags_to_ib(int
> > flags)
> > return flags;
> > }
> >
> > +static inline enum rdma_network_type pvrdma_wc_network_hdr_to_ib(
> > + enum pvrdma_network_type
> > type)
> > +{
> > + return (enum rdma_network_type)type;
> > +}
> > +
> > static inline int ib_send_flags_to_pvrdma(int flags)
> > {
> > return flags & PVRDMA_MASK(PVRDMA_SEND_FLAGS_MAX);
> > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c
> > b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c
> > index 90aa326..34727f6 100644
> > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c
> > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c
> > @@ -389,6 +389,8 @@ static int pvrdma_poll_one(struct pvrdma_cq
> > *cq, struct pvrdma_qp **cur_qp,
> > wc->dlid_path_bits = cqe->dlid_path_bits;
> > wc->port_num = cqe->port_num;
> > wc->vendor_err = cqe->vendor_err;
> > + wc->network_hdr_type =
> > + pvrdma_wc_network_hdr_to_ib(cqe-
> > >network_hdr_type);
> >
> > /* Update shared ring state */
> > pvrdma_idx_ring_inc(&cq->ring_state->rx.cons_head, cq-
> > >ibcq.cqe);
> > diff --git a/include/uapi/rdma/vmw_pvrdma-abi.h
> > b/include/uapi/rdma/vmw_pvrdma-abi.h
> > index c8c1d2d..6a87806 100644
> > --- a/include/uapi/rdma/vmw_pvrdma-abi.h
> > +++ b/include/uapi/rdma/vmw_pvrdma-abi.h
> > @@ -58,6 +58,13 @@
> > #define PVRDMA_UAR_CQ_ARM BIT(30) /*
> > Arm bit. */
> > #define PVRDMA_UAR_CQ_POLL BIT(31) /
> > * Poll bit. */
> >
> > +enum pvrdma_network_type {
> > + PVRDMA_NETWORK_IB,
> > + PVRDMA_NETWORK_ROCE_V1 = PVRDMA_NETWORK_IB,
> > + PVRDMA_NETWORK_IPV4,
> > + PVRDMA_NETWORK_IPV6
> > +};
>
> Doug,
>
> I see that you are already merged this patch, but it is problematic
> patch.
It's only gone to github, so it's not set in stone.
> They defined in uapi file, the new enum which is equal to already
> existed
> 128 enum rdma_network_type {
> 129 RDMA_NETWORK_IB,
> 130 RDMA_NETWORK_ROCE_V1 = RDMA_NETWORK_IB,
> 131 RDMA_NETWORK_IPV4,
> 132 RDMA_NETWORK_IPV6
> 133 };
>
> And the more important they are doing direct casting from
> rdma_network_type to
> pvrdma_network_type as is in the same patch.
>
> The proper way to do it is to return rdma_network_type directly
> without obfuscation
> and without creating new supported forever UAPI enum.
Generally, I would agree with you. I guess that's what I get for
trying to get everything done and working late last night :-/.
Adit, is there a reason for the duplicate enum and silly casting in
this patch? If there isn't a specific reason for it, then it would be
best to re-write it without this silliness.
--
Doug Ledford <dledford@xxxxxxxxxx>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
