Patrick McHardy wrote:

quoted

quoted

(..)
Qdiscs might change skb->cb. Maybe use skb->sk?

      

The loopback functionality in CAN is a bit tricky (maybe you can take a
look into the Documentation patch [7/7] at chapter 3.2 and 4.1.4).

The problem is, that we need a per socket(!) option that enables the
loopback for the sent CAN-Frames or not, so the information about
loopback a skb content or not is contained inside the skb sent down to
the CAN netdevice. If the CAN networkdevice is not capable to perform a
loopback itself (see dev->flags, chapter 6) the CAN core has to do the
loopback as a fallback.
    

skb->sk should work fine for a per socket option. In fact the
CAN core patch simply sets skb->cb to skb->sk.

  

quoted

The use of skb->cb is also needed in the receive path to recognize and
trash loopback'ed CAN-frames inside the *originating* socket (see
chapter 4.1.4).
    

skb->sk should work here as well since you detect these frames
before queueing to the receiving socket.
  

Hm - this would indeed be much nicer than using skb->cb.

I think we just had some concerns to use skb->sk for our own
functionality, as it might be used and dereferenced by other layers
between the socket and the network driver ... so setting skb->sk to NULL
(to indicate 'no loopback') smells bad if you don't really know who ever
dereferences skb->sk.

Btw. it's really worth to look at it again. I just had the idea to
access the loopback flag via skb->sk->opt->loopback (argpfl!) on the TX
path and skb->sk on the RX path. This would avoid skb->sk to be set to
NULL but need to have this loopback flag to be implemented in each CAN
socket opt.

I'll discuss this with Urs tomorrow morning, which change would be the
best and also the 'safest' solution here.

Thanks for your contribution!

Oliver