[Devel] [PATCH VZ9 2/3] net: zerocopy over unix sockets

[Alexey Kuznetsov]

Hello!

On Tue, Jan 16, 2024 at 1:13 PM Pavel Tikhomirov
<ptikhomirov at virtuozzo.com> wrote:
> > --- a/net/core/sock.c
> > +++ b/net/core/sock.c
> > @@ -1405,6 +1405,9 @@ int sk_setsockopt(struct sock *sk, int level, int optname,
> >                             (sk->sk_type == SOCK_DGRAM &&
> >                              sk->sk_protocol == IPPROTO_UDP)))
> >                               ret = -EOPNOTSUPP;
> > +             } else if (sk->sk_family == PF_UNIX) {
> > +                     if (sk->sk_type == SOCK_DGRAM)
> > +                             ret = -EOPNOTSUPP;
>
> Likely we don't want to enable zerocopy for SOCK_SEQPACKET. Maybe change
> to: "if (sk->sk_type != SOCK_STREAM)"

Indeed. Thanks.


> > +     if (unlikely(flags & MSG_ERRQUEUE))
> > +             return unix_recv_error(sk, msg, size);
>
> In case of MSG_PEEK it might be not what user wants to receive zero-copy
> notification error message.

MSG_ERRQUEUE means user requested error messages, them and only them.
If MSG_PEEK (or another flag) is also set it means either peeking from
error queue or this flag
is ignored (likeMSG_DONTWAIT) or rejected, whatever is appropriate. At
least it is how I originally designed
it. But in case it was messed up with time, I have to recheck current semantics
in tcp and align to it, whatever it is.


> > +     int err = skb_orphan_frags_rx(skb, GFP_KERNEL);
> > +
>
> I prefer not to put function call on the same line with variable

Ok-ok. If my old pants are old-fashioned granpa's style, we replace them.

But why that creepy checkpatch.pl does not complain? Is it broken and
already not haute couture? :-)