Re: [PATCH 2/3] nvme-tcp: support specifying the congestion-control

From: Mingbao Sun
Date: Fri Mar 11 2022 - 03:48:03 EST


On Fri, 11 Mar 2022 08:15:18 +0100
Christoph Hellwig <hch@xxxxxx> wrote:

> On Fri, Mar 11, 2022 at 11:01:12AM +0800, Mingbao Sun wrote:
> > + case NVMF_OPT_TCP_CONGESTION:
> > + p = match_strdup(args);
> > + if (!p) {
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + kfree(opts->tcp_congestion);
> > + opts->tcp_congestion = p;
>
> We'll need to check that the string is no loner than TCP_CA_NAME_MAX
> somewhere.
>

accept.
will do that in the next version.
this would also be applied for the target side.

> >
> > + if (nctrl->opts->mask & NVMF_OPT_TCP_CONGESTION) {
> > + ret = tcp_set_congestion_control(queue->sock->sk,
> > + nctrl->opts->tcp_congestion,
> > + true, true);
>
> This needs to be called under lock_sock() protection. Maybe also
> add an assert to tcp_set_congestion_control to enforce that.

accept.
will handle it in the next version.
this would also be applied for the target side.
Many thanks for reminding.

as for the assertion, I failed to find a conventional way to do that.
would you like to give me a suggestion?