Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1683932 > unrolled thread

[PATCH 1/2] netdevice: add netdev_pub helper function

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/2] netdevice: add netdev_pub helper function "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-07-10 05:30 +0200
    Re: [PATCH 1/2] netdevice: add netdev_pub helper function David Miller <davem@davemloft.net> - 2017-07-10 10:10 +0200
      Re: [PATCH 1/2] netdevice: add netdev_pub helper function "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-07-10 14:00 +0200

#1683932 — [PATCH 1/2] netdevice: add netdev_pub helper function

Being able to utilize this makes code a lot simpler and cleaner. It's
easier in many cases for drivers to pass around their private data
structure, while occationally needing to dip into net_device, rather
than the other way around, which results in tons of calls to netdev_priv
in the top of every single function, which makes everything confusing
and less clear. Additionally, this enables a "correct" way of doing such
a thing, instead of having drivers attempt to reinvent the wheel and
screw it up.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 include/linux/netdevice.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 779b23595596..83d58504e5c4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2030,26 +2030,37 @@ void dev_net_set(struct net_device *dev, struct net *net)
 }
 
 /**
  *	netdev_priv - access network device private data
  *	@dev: network device
  *
  * Get network device private data
  */
 static inline void *netdev_priv(const struct net_device *dev)
 {
 	return (char *)dev + ALIGN(sizeof(struct net_device), NETDEV_ALIGN);
 }
 
+/**
+ * 	netdev_pub - access network device from private pointer
+ * 	@priv: private data pointer of network device
+ *
+ * Get network device from a network device private data pointer
+ */
+static inline struct net_device *netdev_pub(void *priv)
+{
+	return (struct net_device *)((char *)priv - ALIGN(sizeof(struct net_device), NETDEV_ALIGN));
+}
+
 /* Set the sysfs physical device reference for the network logical device
  * if set prior to registration will cause a symlink during initialization.
  */
 #define SET_NETDEV_DEV(net, pdev)	((net)->dev.parent = (pdev))
 
 /* Set the sysfs device type for the network logical device to allow
  * fine-grained identification of different network device types. For
  * example Ethernet, Wireless LAN, Bluetooth, WiMAX etc.
  */
 #define SET_NETDEV_DEVTYPE(net, devtype)	((net)->dev.type = (devtype))
 
 /* Default NAPI poll() weight
  * Device drivers are strongly advised to not use bigger value
-- 
2.13.2

[toc] | [next] | [standalone]

#1684034

From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: Mon, 10 Jul 2017 05:19:58 +0200

> Being able to utilize this makes code a lot simpler and cleaner. It's
> easier in many cases for drivers to pass around their private data
> structure, while occationally needing to dip into net_device, rather
> than the other way around, which results in tons of calls to netdev_priv
> in the top of every single function, which makes everything confusing
> and less clear. Additionally, this enables a "correct" way of doing such
> a thing, instead of having drivers attempt to reinvent the wheel and
> screw it up.
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

I disagree.  Assuming one can go from the driver private to the netdev
object trivially is a worse assumption than the other way around, and
locks us into the current implementation of how the netdev and driver
private memory is allocated.

If you want to style your driver such that the private is passed
around instead of the netdev, put a pointer back to the netdev object
in your private data structure.

Which is exactly what the ioc3-eth driver ought to be doing.

[toc] | [prev] | [next] | [standalone]

#1684169

On Mon, Jul 10, 2017 at 10:04 AM, David Miller <davem@davemloft.net> wrote:
> I disagree.  Assuming one can go from the driver private to the netdev
> object trivially is a worse assumption than the other way around, and
> locks us into the current implementation of how the netdev and driver
> private memory is allocated.
>
> If you want to style your driver such that the private is passed
> around instead of the netdev, put a pointer back to the netdev object
> in your private data structure.

I'm surprised you're okay with the memory waste of that, but you bring
up the ability to change the interface later, which is a great point.
I'll submit a patch for that random driver, and I'll also refactor
WireGuard to do the same. Thanks for the guidance.

Jason

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web