On 08/25/2017 11:34 AM, Kurt Van Dijck wrote:

quoted

On 08/24/2017 04:11 PM, Kurt Van Dijck wrote:

quoted

quoted

quoted

This patch removes the struct raw_sock::ifindex from the CAN raw socket
and uses the already existing sock::sk_bound_dev_if instead.

I would like to omit this patch.

1. It does NOT increase the readability ;-)

ifindex points out that we are handling an interface index.
sk_bound_dev_if could be anything.

See http://elixir.free-electrons.com/linux/v4.12.8/source/net/core/sock.c#L617
sk_bound_dev_if may be a difficult name, but could not be anything.
It allow to use SO_BINDTODEVICE (man 7 socket) to work properly for can raw sockets too.

When ro->ifindex = 0 it can still be bound in the case of CAN sockets.
There is a different semantic as ifindex = zero means 'all CAN interfaces'
and not 'no interface'.

ro->ifindex = 0: accept traffic from all interfaces
ro->ifindex != 0: accept traffic only from 1 iface

sk_bound_dev_if = 0: accept traffic from all interfaces
sk_bound_dev_if != 0: accept traffic only from 1 iface

What is the semantic difference?

man 7 socket

SO_BINDTODEVICE

Bind this socket to a particular device like “eth0”, as specified in the 
passed interface name.  If the name  is  an empty  string  or the option 
length is zero, the socket device binding is removed.  The passed option 
is a variable-length null-terminated interface name string with the 
maximum size of IFNAMSIZ.  If a socket is bound to  an  interface,  only 
  packets received from that particular interface are processed by the 
socket. Note that this works only for some socket types, particularly 
AF_INET sockets.  It is not supported for packet  sockets  (use  normal 
bind(2) there).

Especially "If the name  is  an empty  string  or the option length is 
zero, the socket device binding is removed." is wrong.

A bound CAN_RAW socket can have ifindex = 0.

And "It is not supported for packet  sockets  (use  normal  bind(2) 
there)" - the CAN_RAW socket is based on PF_PACKET sockets with the 
device binding and addressing concept.

The more I look into this, the more I'm sure that we should leave it as-is.

Regards,
Oliver