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

From: Mingbao Sun <[email-masked]>
Date: Fri, 11 Mar 2022 16:47:48 +0800

On Fri, 11 Mar 2022 08:15:18 +0100
Christoph Hellwig <[email-masked]> 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?