Message-ID: <20091006133406.GB14360@hmsreliant.think-freely.org>
Date:	Tue, 6 Oct 2009 09:34:06 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	Oliver Hartkopp <socketcan@...tkopp.net>,
	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH] af_packet: add interframe drop cmsg (v6)

On Tue, Oct 06, 2009 at 02:23:13PM +0200, Eric Dumazet wrote:
> Oliver Hartkopp a écrit :
> > Eric Dumazet wrote:
> 
> >> Given that datagrams are usually lost by network before hitting your socket, I am not sure
> >> it is useful at all. 
> > 
> > It is :-)
> > 
> > Usually the datagrams ('CAN frames') do not get lost on the media. The problem
> > is, that userspace apps are scheduled. An on high CAN traffic load this may
> > lead to drops in the receive path on socket level.
> > 
> > It helps to increase the sockets receivebufsize with
> > 
> >   setsockopt(s, SOL_SOCKET, SO_RCVBUF, ...)
> > 
> > but it never gives an answer whether any frames have been dropped.
> 
> OK
> 
> > 
> >> But yes, we could do the following for free :
> >>
> >> Use existing sock->sk_drops total counter, and to store its value at time datagram 
> >> is received and stored into socket queue (store sock->sk_drops into skb->mark)
> >>
> >> At recv() time, copy this saved value into a CMSG item if user set a given SOL_SOCKET option.
> >>
> >> (It would not be a relative gap, but absolute. Your application would have to compute
> >> deltas itself)
> >>
> > 
> > But this leads to two questions:
> > 
> > 1. AFAICS we would add a cmsg to *any* received datagram then. Would this have
> > any remarkable impact to the performance?
> 
> I said "if user set a given SOL_SOCKET option". Cmsg addition should be avoided for
> applications not caring about this drop count.
> 
> > 
> > 2. If it is that easy: Does it still make sense for PF_PACKET to support a
> > separate PACKET_GAPDATA interface to the user?
> > 
> 
> No, since generic would be enough to handle Neil problem, we could revert Neil patch.
> 
Actually, no, I don't think this is sufficient.  Looking at how the
implementation would work, we would query the sk_drop value in the recvmsg path,
but we wouldn't record it during frame enqueue, which would skew the data I need
to record.  Consider a queue of holding frames with sequence 1 2 3 4 101 102
110.  Assume that missing sequence numbers were dropped during enqueue to the
sk_recieve_queue for lack of space.  Using the proposed above implementation,
the frames the gap gets reported with is determined by the scheduling of the
user space app, rather than the actual order in which they were lost.  I.e if
frames 5-100 were received & dropped immediately after the user process called
recvmsg for the first time, we would get the cmsg notification when we called
recvmsg and read frame 2, but if we lost those frames after the process called
recvmsg twice, we'd recognize the loss when we read frame 3.  With my current
implementation we recognize the gap on the next enqueued frame after we
encountered the loss, keeping everything in order, which is important.  We would
need to do that with whatever socket level option we'd consider implementing.
Not saying its not doable, but we need to take it into account, and its not a
straightforward as simply reading sk_drops on recvmsg.  We'd have to store the
sk_drop value in skb->mask universally or some such, and then teach sock_recvmsg
to check that field.

Regards
Neil

> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists