3 reasons why it is better to use rx_handler_private than adding fastnet_mode

Yes, it’s not in my corrected code on gitlab, but well, do what I say, not what I do.

– Modifying netdevice.h will force to rebuild and reinstall all modules that include it. If you don’t you’ll have errors on boot/when loading module for the old header.
– The pointer + private pointer is method is found in a lot of place in the kernel. Usually, you want to do some stuffs (a function) when something is done. That is called a callback. But usually you need some information, some context with this. When you ask de disk for some data, and you finally get back the block (after an IRQ), having just the block you asked is not enough… You’ll need to remember what you wanted to do with that block. You’ll find “private” structure in a lot of places. But pay attention : is it private for you, or private for someone else? In the case of the rxhandler, it’s pretty obvious as rx_handler_data (the private pointer) is set via register_handler : the owner is the one using rxhandler_register_data. But you also have a private space in netdevice for example (accessed through netdev_priv). For who it is private? Its intended user is the device owner, so the driver. If you modify it, you will corrupt the driver… So if you want to retain some per-device data but you’re not the creator of this net_device struct, this is absolutely not the way to go…
– If you add a variable, you have to initialize it, and add again some code.

I didn’t formalize about it in step 4 and 5, because I didn’t tried it… And because the kernel developers are really bastards some time (http://lxr.free-electrons.com/source/include/linux/netdevice.h?v=4.1#L1428 seriously? It would be faster to describe it than writing that…).

For Step 6, it’s more arguable if you need to add a fastnet field or not, as you also access a buffer of skbuff from the file operations. I didn’t specify what to do when the ioctl is called to go back to 0, but there is still packets to read… What should not happen is to leak the memory.

Note the __rcu macro before rx_handler_private definition in net_device.c… Some of you forgot to use rcu-specific accessors for dereferencing. I didn’t penalized either the usage of rcu or not, but for step 6, if you use a rcu pointer, you’ll have to do it right. Note that you shouldn’t play with RCU in your fastnet syscall/ioctl as netdev_rx_handler_register do it for you. Same for netdev_rx_handler_unregister. What you need to do is to use rcu_dereference in your rx_handler code to access the private data.

So next time, use RCU correctly, and try to use diverses private data when you register things. File also have some space for their backing file_operations 😉

Leave a Reply

Your email address will not be published. Required fields are marked *

Time limit is exhausted. Please reload the CAPTCHA.