Re: [PATCH net] datagram: When peeking datagrams with offset < 0 don't skip empty skbs

I'm actually surprised that only unix sockets can have negative values.  Is
there a reason for that?  I had assumed that sk_set_peek_off would allow
negative values as the code already has to support negative values due to what
the initial value is.

A negative initial value indicates that PEEK_OFF is disabled. It only
makes sense to peek from a positive offset from the start of the data.

quoted

I'm wondering adding an explicit SOCK_PEEK_OFF/MSG_PEEK_OFF socket flag
would help simplyifing the code:

The behavior needs to be bifurcated between peeking with
offset and without offset.

When peeking with offset is enabled by setting SO_PEEK_OFF,
subsequent reads do move the offset, so the observed behavior
is correct.

When sk->sk_peek_offset is negative, offset mode is disabled
and the same packet must be read twice.

An explicit boolean flag to discern between the two may make
the code simpler to understand, not sure whether that is logically
required.

quoted

no need for negative offset; set such
flag when SOL_SOCKET/SO_PEEK_OFF with a non negative value is called
(and clear it when a negative value is used), forward such flag to
__skb_try_recv_datagram/__skb_try_recv_from_queue and use it to select
the proper peek behaviour.

The negative value is documented and supported for unix sockets, so I don't
think we can just reject all negative values.

What is the documented behavior on those sockets? The original
patch that introduced it explicitly mentions peek mode as disabled
when the value is negative:

  https://www.spinics.net/lists/netdev/msg189589.html

However, I do see a way to
implement that while supporting user space sending negative values.  If that
is preferred let me know and I'll see what I can make.

I assume that would make this patch target net-next?

I suspect that we can fix this with a small enough patch to be able
to send to net.

Would it be possible to
pull this into net so it can fix this regression for the next kernel release,
while I work on getting the better solution finished?  I'm happy to to make
__skb_try_recv_datagram/__skb_try_recv_from_queue accept the extra parameter
so they don't see an offset less then 0 in this patch.

--
Matthew