On Thu, May 25, 2023 at 11:34 AM David Ahern [off-list ref] wrote:

On 5/25/23 9:05 AM, Willem de Bruijn wrote:

quoted

quoted

+/* A wrapper around sock ioctls, which copies the data from userspace
+ * (depending on the protocol/ioctl), and copies back the result to userspace.
+ * The main motivation for this function is to pass kernel memory to the
+ * protocol ioctl callbacks, instead of userspace memory.
+ */
+int sk_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
+{
+       int rc = 1;
+
+       if (ipmr_is_sk(sk))
+               rc = ipmr_sk_ioctl(sk, cmd, arg);
+       else if (ip6mr_is_sk(sk))
+               rc = ip6mr_sk_ioctl(sk, cmd, arg);
+       else if (phonet_is_sk(sk))
+               rc = phonet_sk_ioctl(sk, cmd, arg);

I don't understand what this buys us vs testing the sk_family,
sk_protocol and cmd here.

To keep protocol specific code out of core files is the reason I
suggested it.

I guess you object to demultiplexing based on per-family
protocol and ioctl cmd constants directly in this file?

That only requires including the smaller uapi headers.

But now net/core/sock.h now still has to add includes
linux/mroute.h, linux/mroute6.h and net/phonet/phonet.h.

Aside on phonet_is_sk, if we're keeping this: this should be
sk_is_phonet? Analogous to sk_is_tcp and such. And, it should suffice
to  demultiplex based on the protocol family, without testing the
type or protocol. The family is defined in protocol-independent header
linux/socket.h. The differences between
PN_PROTO_PHONET and PN_PROTO_PIPE should be handled inside the family
code. So I think it is cleaner just to open-coded as `if
(sk->sk_family == PF_PHONET)`