af_packet.c bug?
Subsystems (2)
What is the '13' doing here? Maybe it should be IFNAMSIZ?
/* * Find the device first to size check it */ saddr->spkt_device[13] = 0; dev = dev_get_by_name(saddr->spkt_device); err = -ENODEV; if (dev == NULL) goto out_unlock;
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
* Ben Greear <[email protected]> 2005-03-28 11:19
Quoted 12 lines
What is the '13' doing here? Maybe it should be IFNAMSIZ?
/* * Find the device first to size check it */ saddr->spkt_device[13] = 0; dev = dev_get_by_name(saddr->spkt_device); err = -ENODEV; if (dev == NULL) goto out_unlock;
Seems so, please adopt the size of spkt_device in struct sockaddr_pkt as well if you change it, it's currently hardcoded as 14.
Quoted 8 lines · Thomas Graf
* Ben Greear <[email protected]> 2005-03-28 11:19
Quoted 6 lines
What is the '13' doing here? Maybe it should be IFNAMSIZ?
/* * Find the device first to size check it */
Quoted 8 lines
Quoted 5 lines
saddr->spkt_device[13] = 0; dev = dev_get_by_name(saddr->spkt_device); err = -ENODEV; if (dev == NULL) goto out_unlock;Seems so, please adopt the size of
spkt_devicein structsockaddr_pkt
you mean adapt maybe?
Quoted 1 line
as well if you change it, it's currently hardcoded as 14.
I was also wondering why we couldn't hold a reference to the net-device instead of just it's ifindex when dealing with a bound raw socket.
That would save the device lookup for each packet sent.
I figure that we would need to listen for NETDEV_UNREGISTER events and do a dev_put whenever the device wants to go away.
Does that sound workable?
Ben
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
* Ben Greear <[email protected]> 2005-03-28 12:08
Quoted 3 lines
Quoted 1 line
Seems so, please adopt the size of
spkt_devicein structsockaddr_pktyou mean adapt maybe?
Of course, stupid typo. ;->
Quoted 2 lines
I was also wondering why we couldn't hold a reference to the net-device instead of just it's ifindex when dealing with a bound raw socket.
Saving a netdevice handle in the packet socket is not a problem but doing the same for the packet socket address seems to be non trivial.
Quoted 7 lines · Thomas Graf
* Ben Greear <[email protected]> 2005-03-28 12:08
Quoted 3 lines
Quoted 1 line
Seems so, please adopt the size of
spkt_devicein structsockaddr_pktyou mean adapt maybe?
Quoted 8 lines
Of course, stupid typo. ;->
Quoted 2 lines
I was also wondering why we couldn't hold a reference to the net-device instead of just it's ifindex when dealing with a bound raw socket.
Saving a netdevice handle in the packet socket is not a problem but doing the same for the packet socket address seems to be non trivial.
Yes. But assuming you bind the socket, you use the device saved in the packet socket. If you are sending to a particular device for each send(), then you'll just have to pay the lookup costs. I haven't looked recently, but didn't someone finally hash the device lookup so that it's not a linear walk of the device list now?
Ben
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
David S. Miller
maintainerQuoted 2 lines · On Mon, 28 Mar 2005 12:08:14 -0800 Ben Greear <[email protected]> wrote:
I was also wondering why we couldn't hold a reference to the net-device instead of just it's ifindex when dealing with a bound raw socket.
Because then raw sockets could make unloading of netdevices hang forever. That is, unless you add some netdev notifier to af_packet.c that walks all the raw sockets looking for netdev references.
I guess it also would make things like leaving a dump running while you quickly down/up and interface stop working.
Quoted 8 lines · David S. Miller
Quoted 2 lines · On Mon, 28 Mar 2005 12:08:14 -0800 Ben Greear <[email protected]> wrote:
I was also wondering why we couldn't hold a reference to the net-device instead of just it's ifindex when dealing with a bound raw socket.
Quoted 8 lines
Because then raw sockets could make unloading of netdevices hang forever. That is, unless you add some netdev notifier to
af_packet.cthat walks all the raw sockets looking for netdev references.I guess it also would make things like leaving a dump running while you quickly down/up and interface stop working.
I was thinking of removing the reference only when the NETDEV_UNREGISTER event was sent. That will allow sockets to work through UP and DOWN transitions of the device as it does currently.
We could also save the name of the interface in the socket struct and resolve the device in a lazy manner. That way, even if the device is truly gone, the worst case performance will be like it is today.
It would be nice to have a list of all of these sockets so we could get info on them similar to the 'netstat -an' output anyway...
Ben
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
David S. Miller
maintainerQuoted 2 lines · On Mon, 28 Mar 2005 12:30:25 -0800 Ben Greear <[email protected]> wrote:
It would be nice to have a list of all of these sockets so we could get info on them similar to the 'netstat -an' output anyway...
/proc/net/packet is insufficient?
David S. Miller
maintainerQuoted 8 lines · On Mon, 28 Mar 2005 21:55:57 +0200 Thomas Graf <[email protected]> wrote:
* Ben Greear <[email protected]> 2005-03-28 11:19
Quoted 7 lines
What is the '13' doing here? Maybe it should be IFNAMSIZ?
/* * Find the device first to size check it */
Quoted 8 lines
Quoted 5 lines
saddr->spkt_device[13] = 0; dev = dev_get_by_name(saddr->spkt_device); err = -ENODEV; if (dev == NULL) goto out_unlock;Seems so, please adopt the size of
spkt_devicein structsockaddr_pktas well if you change it, it's currently hardcoded as 14.
No can do, that structure is exported to userspace. Thus, changing it from 14 to IFNAMESZ (which is 16) will break stuff.