martinezjavier / ldd3

Linux Device Drivers 3 examples updated to work in recent kernels
http://examples.oreilly.com/9780596005900/
Other
2.35k stars 905 forks source link

`dev_addr` in `struct net_device` has now been made `const` #82

Open pauljherring opened 1 year ago

pauljherring commented 1 year ago

dev_addr in struct net_device has now been made const and cannot be directly written to, e.g.:

https://patchwork.kernel.org/project/linux-media/patch/20211026175250.3197558-1-kuba@kernel.org/#24558423

The change I made to get snull to compile (but maybe not the best way):

        /*
         * Assign the hardware address of the board: use "\0SNULx", where
         * x is 0 or 1. The first byte is '\0' to avoid being a multicast
         * address (the first byte of multicast addrs is odd).
         */
-       memcpy(dev->dev_addr, "\0SNUL0", ETH_ALEN);
        if (dev == snull_devs[1])
-               dev->dev_addr[ETH_ALEN-1]++; /* \0SNUL1 */
+               eth_hw_addr_set(dev, "\0SNUL1");
+       else
+               eth_hw_addr_set(dev, "\0SNUL0");

(Not included the full diff, since I've changed other stuff in there while messing with it so the line numbers won't match; but between looking at it today and the last time I was messing with it, this const problem came up.)

May be an issue elsewhere? Not sure - haven't checked the other modules; just raising this as something that may need looking at.

pauljherring commented 1 year ago

Had to revert my changes (because, reasons,) but still needed apply the use of eth_hw_addr_set - the diff as a result:

diff --git a/snull/snull.c b/snull/snull.c
index 2efc2b5..adc5899 100644
--- a/snull/snull.c
+++ b/snull/snull.c
@@ -213,9 +213,10 @@ int snull_open(struct net_device *dev)
         * x is 0 or 1. The first byte is '\0' to avoid being a multicast
         * address (the first byte of multicast addrs is odd).
         */
-       memcpy(dev->dev_addr, "\0SNUL0", ETH_ALEN);
-       if (dev == snull_devs[1])
-               dev->dev_addr[ETH_ALEN-1]++; /* \0SNUL1 */
+       if (dev == snull_devs[0])
+               eth_hw_addr_set(dev, "\0SNUL0");
+       else
+               eth_hw_addr_set(dev, "\0SNUL1");
        if (use_napi) {
                struct snull_priv *priv = netdev_priv(dev);
                napi_enable(&priv->napi);
dwalkes commented 1 year ago

Thanks @pauljherring do you want to create a pull request for this? Should probably check for kernel 5.15 and later based on https://github.com/torvalds/linux/commit/48eab831ae8b9f7002a533fa4235eed63ea1f1a3

See https://github.com/martinezjavier/ldd3/blob/bf49bb48ab3d16e81406953a0c3ae32a1faf534e/sbull/sbull.c#L93 for an example use of KERNEL_VERSION

pauljherring commented 1 year ago

.. I'm not at all sure that I'm the right person to be doing a pull request on this project, or for this in particular: 1) it's not clear it's sufficient - even after compiling, the driver doesn't "work" as expected for me (pings go to the first interface, but don't make it out the other end, for example); this merely "gets it to compile" 2) whether anything else in the repo needs looking at; I'm only concentrating on the network driver because I need to deal with an in-house network driver and this is part of my self-education on such matters.