Re: [PATCH v6 02/25] rtrs: public interface header to establish RDMA connections

Bart Van Assche
2019-12-30T19:25:19Z

On 2019-12-30 02:29, Jack Wang wrote:
> +/*
> + * Here goes RTRS client API
> + */

A comment that explains what the abbreviation "RTRS" stands for would be
welcome here. Additionally, I think that "Here goes" can be left out.

> +/**
> + * rtrs_clt_open() - Open a session to an RTRS server
> + * @priv: User supplied private data.
> + * @link_ev: Event notification for connection state changes

Please mention that @link_ev is a callback function.

> + * @priv: User supplied data that was passed to rtrs_clt_open()
> + * @ev: Occurred event

Is this patch series W=1 clean? @link_ev arguments should be documented
above the link_clt_ev_fn typedef.

> + * @path_cnt: Number of elemnts in the @paths array

elemnts -> elements?

> + * Starts session establishment with the rtrs_server. The function can block
> + * up to ~2000ms until it returns.

until -> before?

> +struct rtrs_clt *rtrs_clt_open(void *priv, link_clt_ev_fn *link_ev,
> + const char *sessname,
> + const struct rtrs_addr *paths,
> + size_t path_cnt, short port,
> + size_t pdu_sz, u8 reconnect_delay_sec,
> + u16 max_segments,
> + s16 max_reconnect_attempts);

Since the range for port numbers is 1..65535, please change "short port"
into "u16 port".

> +/**
> + * enum rtrs_clt_con_type() type of ib connection to use with a given permit

What is a "permit"?

> + * @vec: Message that is send to server together with the request.

send -> sent?

> + * Sum of len of all @vec elements limited to <= IO_MSG_SIZE.
> + * Since the msg is copied internally it can be allocated on stack.
> + * @nr: Number of elements in @vec.
> + * @len: length of data send to/from server

send -> sent?

> +/**
> + * link_ev_fn(): Events about connective state changes

connective -> connection?

> +/**
> + * rtrs_srv_open() - open RTRS server context
> + * @ops: callback functions
> + *
> + * Creates server context with specified callbacks.
> + *
> + * Return a valid pointer on success otherwise PTR_ERR.
> + */
> +struct rtrs_srv_ctx *rtrs_srv_open(rdma_ev_fn *rdma_ev, link_ev_fn *link_ev,
> + unsigned int port);

Is this patch series W=1 clean? The documented argument does not match
the actual argument list.

Bart.