Re: [PATCH] RDMA/RXE: Add send_common_ack() helper

Zhu Yanjun
2022-08-01T07:11:59Z

On Mon, Aug 1, 2022 at 2:16 PM Li Zhijian <lizhijian@fujitsu.com> wrote:
>
> Most code in send_ack() and send_atomic_ack() are duplicate, move them
> to a new helper send_common_ack().
>
> In newer IBA SPEC, some opcodes require acknowledge with a zero-length read
> response, with this new helper, we can easily implement it later.
>
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> drivers/infiniband/sw/rxe/rxe_resp.c | 43 ++++++++++++++----------------------
> 1 file changed, 17 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index b36ec5c4d5e0..4c398fa220fa 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -1028,50 +1028,41 @@ static enum resp_states do_complete(struct rxe_qp *qp,
> return RESPST_CLEANUP;
> }
>
> -static int send_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
> +
> +static int send_common_ack(struct rxe_qp *qp, u8 syndrome, u32 psn,

The function is better with rxe_send_common_ack?
So when debug, rxe_ prefix can help us.

> + int opcode, const char *msg)
> {
> - int err = 0;
> + int err;
> struct rxe_pkt_info ack_pkt;
> struct sk_buff *skb;
>
> - skb = prepare_ack_packet(qp, &ack_pkt, IB_OPCODE_RC_ACKNOWLEDGE,
> - 0, psn, syndrome);
> - if (!skb) {
> - err = -ENOMEM;
> - goto err1;
> - }
> + skb = prepare_ack_packet(qp, &ack_pkt, opcode, 0, psn, syndrome);
> + if (!skb)
> + return -ENOMEM;
>
> err = rxe_xmit_packet(qp, &ack_pkt, skb);
> if (err)
> - pr_err_ratelimited("Failed sending ack\n");
> + pr_err_ratelimited("Failed sending %s\n", msg);
>
> -err1:
> return err;
> }
>
> -static int send_atomic_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
> +static int send_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)

rxe_send_ack

> {
> - int err = 0;
> - struct rxe_pkt_info ack_pkt;
> - struct sk_buff *skb;
> -
> - skb = prepare_ack_packet(qp, &ack_pkt, IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE,
> - 0, psn, syndrome);
> - if (!skb) {
> - err = -ENOMEM;
> - goto out;
> - }
> + return send_common_ack(qp, syndrome, psn,
> + IB_OPCODE_RC_ACKNOWLEDGE, "ACK");
> +}
>
> - err = rxe_xmit_packet(qp, &ack_pkt, skb);
> - if (err)
> - pr_err_ratelimited("Failed sending atomic ack\n");
> +static int send_atomic_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)

rxe_send_atomic_ack

Thanks and Regards,
Zhu Yanjun
> +{
> + int ret = send_common_ack(qp, syndrome, psn,
> + IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE, "ATOMIC ACK");
>
> /* have to clear this since it is used to trigger
> * long read replies
> */
> qp->resp.res = NULL;
> -out:
> - return err;
> + return ret;
> }
>
> static enum resp_states acknowledge(struct rxe_qp *qp,
> --
> 1.8.3.1
>