Date:	Tue, 6 Oct 2009 08:50:11 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	Oliver Hartkopp <socketcan@...tkopp.net>
Cc:	Eric Dumazet <eric.dumazet@...il.com>,
	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 01:05:42PM +0200, Oliver Hartkopp wrote:
> Eric Dumazet wrote:
> > Oliver Hartkopp a écrit :
> > 
> >>> +	gap = check_packet_gap(skb);
> >>> +	if (gap)
> >>> +		put_cmsg(msg, SOL_PACKET, PACKET_GAPDATA, sizeof(__u32), &gap);
> >>> +
> >> I wonder, if using SOL_PACKET with a packet specific sockopt PACKET_GAPDATA is
> >> the right approach here, as this functionality should probably be made
> >> available to all DGRAM/RAW sockets ...
> >>
> >> E.g. getting the gap information is also interesting for CAN_RAW sockets as i
> >> polled from the SocketCAN ML.
> >>
> >> So i could easily implement something like this in net/can/raw.c :
> >>
> >> gap = check_packet_gap(skb);
> >> if (gap)
> >> 	put_cmsg(msg, SOL_CAN_RAW, CAN_RAW_GAPDATA, sizeof(__u32), &gap);
> >>
> >> Is this a good approach or should we better find a general handling within
> >> SOL_SOCKET here?
> >>
> > 
> > 
> > 
> > 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.
> 
> > 
> > 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?
> 
About the only impact I see with this approach would be the fact that sk->drops
is an atomic, which on some arches may be an expensive operation.  Theres also
the need to check if this new hypothetical socket option is enabled.  So it
might be a slight performance impact, but given that all this activity would
happen in the recevie path of the process context, thats not likely to be a huge
deal.

> 2. If it is that easy: Does it still make sense for PF_PACKET to support a
> separate PACKET_GAPDATA interface to the user?
> 
If this is truly useful, then I would say no, its not needed specifically at the
protocol level.  I implemented it there because it was a request specifically
for that protocol to support detailed drop data.

That said, I think Eric has a point regarding the loss locations of other
protocols.  Most protocols loose frames on the media, rather than at the socket
layer.  AF_PACKETS only real loss point is at the receive queue.  Thats not 100%
accurate, of course, since it snoops other protocols that do have other loss
points, but people using AF_PACKET understand how the receive model works there.
If we provide drop data for say, UDP, I wonder if that won't give people a false
sense of whats going on on the network.  I.e. for UDP, we could report
interframe gap data of zero frames lost, but still show lots of loss statistics
(like invalid udp checksums, or the like), leading to confusion.

Perhaps what would be worthwhile would be a distinct cmsg, that was relevant to
all sockets.  Something like Recieve Queue Overflows.  A socket level cmsg that
indicates to the receiving process the number of times the sk_receive_queue
overflowed between this call into the receive path and the immediately prior
call.  I know the difference is subtle, and in the case of AF_PACKET the
information would be identical (since that is AF_PACKETS only drop point).  But
I think that subtle change would help clarify the difference in meaning of that
cmsg from other loss statistics for other protocols.

Regards
Neil

> Regards,
> Oliver
> 
> 
--
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