Re: [PATCH mptcp-next 3/5] mptcp: add MPTCP_TCPINFO getsockopt support

[Florian Westphal <fw@strlen.de>]

Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:
> 
> 
> On Wed, 11 Aug 2021, Florian Westphal wrote:
> 
> > diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> > index eba294a071c8..ac8e6823db4f 100644
> > --- a/net/mptcp/sockopt.c
> > +++ b/net/mptcp/sockopt.c
> > @@ -723,6 +723,96 @@ static int mptcp_getsockopt_info(struct mptcp_sock *msk, char __user *optval, in
> > 
> > 	if (copy_to_user(optval, &m_info, len))
> > 		return -EFAULT;
> > +	return 0;
> > +}
> > +
> > +static int mptcp_get_subflow_data(struct mptcp_subflow_data *sfd,
> > +				  char __user *optval, int __user *_u_optlen)
> > +{
> > +#define MIN_INFO_OPTLEN_SIZE	16
> 
> This caught my eye on my first quick review pass, I'm not accustomed to
> seeing inline defines like this in the kernel unless it's part of some macro
> magic. The style guide / checkpatch don't say anything, so I'll just suggest
> it would fit better outside the function scope (but no big deal).

An alternative would be to use 'sizeof struct mptcp_subflow_data' here
and change that structure a bit.

Instead of storing the size, add a 'version/flags' field.

That would also allow to extend the structure later: kernel would
expose a 'struct mptcp_subflow_data_v2' or similar if the need would
arise and userspace would tell kernel the size in indirect fashion by
setting a special 'v2 enabled' field.