On Tue, Aug 29, 2017 at 06:54:48AM -0700, Doug Ledford wrote:
> 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.
Thanks Leon, Doug. Well, we were only keeping it consistent with what we
do for our other device <-> driver interactions. We can probably remove the
enum and directly assign the network_hdr_type from our cqe to the IB stack's
WC.
Doug, let me know if you prefer a v1 of this patch series or a patch that just
removes the enum.
--
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
