netdev - Re: [patch 5/7] CAN: Add virtual CAN netdevice driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 03 Jun 2007 20:27:17 +0200
From:	Patrick McHardy <kaber@...sh.net>
To:	Urs Thuermann <urs@...ogud.escape.de>
CC:	David Miller <davem@...emloft.net>,
	Thomas Gleixner <tglx@...utronix.de>,
	Oliver Hartkopp <oliver.hartkopp@...kswagen.de>,
	netdev@...r.kernel.org
Subject: Re: [patch 5/7] CAN: Add virtual CAN netdevice driver

Urs Thuermann wrote:
> Patrick McHardy <kaber@...sh.net> writes:
> 
>>And it allows you have both loopback and non-loopback devices
>>in case that would be useful.
> 
> 
> That sounds very promising.  I also was unhappy with the fixed number
> of vcan devices at module load time and have thought about ioctl's to
> add/delete devices and to change the loopback flag.  A more general
> approach like the one you seem to be working on is preferable of
> course.


I got a bit delayed, but I hope to have the patches in postable state
tommorrow.

>>Qdiscs might change skb->cb. Maybe use skb->sk?
> 
> 
> When we decided to use skb->cb it seemed the only possible option.
> We need a field that we can set to zero to indicate we don't want the
> driver to loop back the packet and the value in that field must
> survive the path
> 
>         can_send()
>         dev_queue_xmit()
>         ...
>         dev->hard_start_xmit()
>         
>         netif_rx()
>         can_rcv()
> 
> I think I misread the comment
> 
>  *      @cb: Control buffer. Free for use by every layer. Put private vars here
> 
> to mean I can use it for this purpose and since it worked as intended
> we felt ok with this.  Now I see, it states exactly the opposite that
> I can't count on the value being preserved across layers.


Yes, that comment is a bit misleading.

> skb->sk can't be used since we shouldn't set it to zero before
> dev_queue_xmit() as Oliver already pointed out.  IIRC, skb->sk
> couldn't also be used in rx half of that path, since it was set to
> null somewhere in netif_rx() but now, reading the src, I can't see
> where this would happen.  Maybe this has changed or my memory has some
> bit errors.  I'll look at it again.  Even if it turns out skb->sk can
> be used in rx path, the need remains to pass down a flag from
> can_send() to dev->hard_start_xmit() indicating whether to loop back
> or not.


That flag is a socket flag acording to Oliver, so you can use the flag
itself. You could keep skb->sk for the RX path (don't call skb_orphan
in your hard_start_xmit function), if that doesn't work worst case would
be to use a flag in the skb (we still have 2 unused bits) until you've
figured out a better way.
-
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