[...]
> >
> > This code is wrong... but then the original in write_maxblksize is wrong
> > to, so you can't be blamed.
> > nfsd_max_blksize applies to nfsd in ALL network namespaces. So if we
> > need to check there are no active services in one namespace, we need to
> > check the same for *all* namespaces.
>
> Yes, the original code does look strange and is probably incorrect
> with regard to its handling of the mutex. Shall we explore and fix
> that issue in the nfsctl code first so that it can be backported to
> stable kernels?
>
>
> > I think we should make nfsd_max_blksize a per-namespace value.
>
> That is a different conversation.
>
> First, the current name of this tunable is incongruent with its
> actual function, which is to specify the maximum network buffer size
> that is allocated when the NFSD service pool is created. We should
> find a more descriptive and specific name for this element in the
> netlink protocol.
>
> Second, it does seem like a candidate for becoming namespace-
> specific, but TBH I'm not familiar enough with its current user
> space consumers to know if that change would be welcome or fraught.
>
> Since more discussion, research, and possibly a fix are needed, we
> might drop max_blksize from this round and look for one or two
> other tunables to convert for the first round.
ack. Are write_unlock_ip() and/or write_unlock_fs() good candidates?
Regards,
Lorenzo
>
>
> > > +
> > > + nfsd_max_blksize = nla_get_u32(attr);
> > > + nfsd_max_blksize = max_t(int, nfsd_max_blksize, 1024);
> > > + nfsd_max_blksize = min_t(int, nfsd_max_blksize, NFSSVC_MAXBLKSIZE);
> > > + nfsd_max_blksize &= ~1023;
> > > +out:
> > > + mutex_unlock(&nfsd_mutex);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +/**
> > > + * nfsd_nl_max_blksize_get_dumpit - dump the nfs block size
> > > + * @skb: reply buffer
> > > + * @cb: netlink metadata and command arguments
> > > + *
> > > + * Returns the size of the reply or a negative errno.
> > > + */
> > > +int nfsd_nl_max_blksize_get_dumpit(struct sk_buff *skb,
> > > + struct netlink_callback *cb)
> > > +{
> > > + return nfsd_nl_get_dump(skb, cb, NFSD_CMD_MAX_BLKSIZE_GET,
> > > + NFSD_A_CONTROL_PLANE_MAX_BLKSIZE,
> > > + nfsd_max_blksize);
> > > +}
> > > +
> > > +/**
> > > + * nfsd_nl_max_conn_set_doit - set the max number of connections
> > > + * @skb: reply buffer
> > > + * @info: netlink metadata and command arguments
> > > + *
> > > + * Return 0 on success or a negative errno.
> > > + */
> > > +int nfsd_nl_max_conn_set_doit(struct sk_buff *skb, struct genl_info *info)
> > > +{
> > > + struct nfsd_net *nn = net_generic(genl_info_net(info), nfsd_net_id);
> > > + struct nlattr *attr = info->attrs[NFSD_A_CONTROL_PLANE_MAX_CONN];
> > > +
> > > + if (!attr)
> > > + return -EINVAL;
> > > +
> > > + nn->max_connections = nla_get_u32(attr);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > > + * nfsd_nl_max_conn_get_dumpit - dump the max number of connections
> > > + * @skb: reply buffer
> > > + * @cb: netlink metadata and command arguments
> > > + *
> > > + * Returns the size of the reply or a negative errno.
> > > + */
> > > +int nfsd_nl_max_conn_get_dumpit(struct sk_buff *skb,
> > > + struct netlink_callback *cb)
> > > +{
> > > + struct nfsd_net *nn = net_generic(sock_net(cb->skb->sk), nfsd_net_id);
> > > +
> > > + return nfsd_nl_get_dump(skb, cb, NFSD_CMD_MAX_CONN_GET,
> > > + NFSD_A_CONTROL_PLANE_MAX_CONN,
> > > + nn->max_connections);
> > > +}
> > > +
> > > /**
> > > * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
> > > * @net: a freshly-created network namespace
Attachment:
signature.asc
Description: PGP signature
