On 6/30/2025 1:56 PM, Vladimir Oltean wrote: > > Ugh :-/ sorry for the trouble, and thanks for doing the hard work of > characterizing this. >
No worries. > Indeed, my first conversion of ixgbe was in August 2023, as this commit > can attest: > https://github.com/vladimiroltean/linux/commit/0351ebf1eee3381ccfba9d31a924d1dd887a316f > > At that time, Przemyslaw's commit fd5ef5203ce6 ("ixgbe: wrap > netdev_priv() usage") didn't exist, and "struct ixgbe_adapter *adapter = > netdev_priv(netdev);" was the de facto idiom in the driver, which I then > replicated two more times, in the new ixgbe_ptp_hwtstamp_set() and > ixgbe_ptp_hwtstamp_get() functions. Not only did I not notice that this > change took place, but it also compiled just fine, making me completely > unsuspecting... > It would be nice if we had a mechanism to provide type safety for the netdev_priv.. but with C I don't really know of a good way. If you provide your own macro you can do type checks on that.. but I'm not sure how else we could implement this to avoid the type issues. The use of such a wrapper private structure is essentially required either in devlink code or netdev code because both of the parent structs want flexible array private members, so they can't both be ixgbe_adapter. Of course validating with KASAN would have caught this faster.. I am actually quite surprised that we manage to get into ice_ptp_set_timestamp_mode() and invoke the register access functions without immediately crashing. At least its caught before this merged and caused memory corruption on production systems! > Tony, let me know how you would like to proceed.
OpenPGP_signature.asc
Description: OpenPGP digital signature
