Re: [RFC PATCH 5/5] geneve: add initial netdev driver for GENEVE tunnels

["John W. Linville" <linville@tuxdriver.com>]

On Fri, Apr 03, 2015 at 05:20:53PM +0200, Jiri Pirko wrote:
> Fri, Apr 03, 2015 at 05:07:44PM CEST, linville@tuxdriver.com wrote:
> >On Fri, Apr 03, 2015 at 10:57:12AM -0400, John W. Linville wrote:
> >> On Thu, Apr 02, 2015 at 10:20:02PM +0200, Jiri Pirko wrote:
> >> > Thu, Apr 02, 2015 at 09:17:06PM CEST, linville@tuxdriver.com wrote:
> >> > >This is an initial implementation of a netdev driver for GENEVE
> >> > >tunnels.  This implementation uses a fixed UDP port, and only supports
> >> > >a single tunnel (and therefore only a single VNI) per net namespace.
> >> > >Only IPv4 links are supported at this time.
> >> > 
> >> > 
> >> > Thanks for doing this John!
> >> > 
> >> > 
> >> > >
> >> > >Signed-off-by: John W. Linville <linville@tuxdriver.com>
> >> > >---
> >> > > drivers/net/Kconfig          |  14 ++
> >> > > drivers/net/Makefile         |   1 +
> >> > > drivers/net/geneve.c         | 451 +++++++++++++++++++++++++++++++++++++++++++
> >> > > include/uapi/linux/if_link.h |   9 +
> >> > > 4 files changed, 475 insertions(+)
> >> > > create mode 100644 drivers/net/geneve.c
> >> > >
> >> > 
> >> > ...
> >> > 
> >> > >+/* Initialize the device structure. */
> >> > >+static void geneve_setup(struct net_device *dev)
> >> > >+{
> >> > >+	struct geneve_dev *geneve = netdev_priv(dev);
> >> > >+
> >> > >+	ether_setup(dev);
> >> > >+
> >> > >+	dev->netdev_ops = &geneve_netdev_ops;
> >> > >+	dev->destructor = free_netdev;
> >> > >+	SET_NETDEV_DEVTYPE(dev, &geneve_type);
> >> > >+
> >> > >+	INIT_WORK(&geneve->sock_work, geneve_sock_work);
> >> > 
> >> > I would push work initialization into geneve_newlink. Seems odd to have
> >> > it here in setup.
> >> 
> >> Yes, that will probably work a lot better for multiple tunnels on a
> >> host... :-)
> >
> >Ignore that comment -- I was thinking...something else...need coffee... ;-)
> >
> >What makes newlink better for INIT_WORK than setup?
> 
> 
> Can be here for sure. I just thought that setup should setup netdev
> according to type. This is not it. This initializes a part of priv
> structure.

Ah -- I wasn't thinking about it that way.  I'll consider it. :-)

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.