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]

Message-ID: <46643EB2.2060204@hartkopp.net>
Date:	Mon, 04 Jun 2007 18:32:50 +0200
From:	Oliver Hartkopp <socketcan@...tkopp.net>
To:	Urs Thuermann <urs@...ogud.escape.de>
CC:	Patrick McHardy <kaber@...sh.net>,
	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:
> Oliver Hartkopp <socketcan@...tkopp.net> writes:
>
>   
>> 2. The loopback indication is done by using the unused skb->protocol in
>> the tx path.
>>     
>
> I don't think we should (mis-)use skb->protocol as a loopback flag.
>   

Yep! After reading the comments from Patrick and Urs i definitely agree
to their concerns.

> IMO it would be better to skb->pkt_type.  This is used to indicate
> packet type to rcv functions registered by dev_add_pack().  It is set
> by netdevice drivers to PACKET_{MULTICAST,BROADCAST,HOST,OTHER} for
> received packets.  In the send path it is set to PACKET_OUTGOING on
> the copy of the skbuff that is delivered to the sockets registered on
> ptype_all (typically packet sockets from tcpdump or other sniffers).
> AFAICS, pkt_type is not used otherwise in the send path.
>
> We could set skb->pkt_type = PACKET_LOOPBACK to flag to the CAN
> netdevice driver whether to loop back the packet.
>   

I think, it goes into the right direction to use skb->pkt_type. The flag
should really be somewhere inside the skb as all back references into
the sk would become sticky in the implementation.

But regarding the use of skb->pkt_type i would suggest to take a closer
look on the definitions in include/linux/if_packet.h and how the
pkt_type is to be used inside the kernel. In my opinion we should use ...

* TX-Path:
PACKET_OTHERHOST: send the CAN frame without loopback
PACKET_BROADCAST : send the CAN frame with loopback (aka local broadcast)

See an example of this approach in drivers/net/arcnet/rfc1051.c :
http://www.linux-m32r.org/lxr/http/source/drivers/net/arcnet/rfc1051.c?a=i386#L99

* RX-Path:
PACKET_HOST : just an incoming CAN frame for this host

Any comments? ACKs?

Best 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