[PATCH 3/3] macvlan: allow in-kernel modules to create and manage macvlan devices

[PATCH 3/3] macvlan: allow in-kernel modules to create and manage macvlan devices

[Patrick Mullaney]

The macvlan driver didn't allow for creation/deletion of devices
by other in-kernel modules. This patch provides common routines
for both in-kernel and netlink based management. This patch
also enables macvlan device support for gro for lower level
devices that support gro.

Signed-off-by: Patrick Mullaney <pmullaney@novell.com>
---

 drivers/net/macvlan.c   |   72 ++++++++++++++++++++++++++++++++---------------
 include/linux/macvlan.h |    4 +++
 2 files changed, 53 insertions(+), 23 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 3425e12..bb180d0 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -210,7 +210,7 @@ static const struct header_ops macvlan_hard_header_ops = {
 	.cache_update	= eth_header_cache_update,
 };
 
-static int macvlan_open(struct net_device *dev)
+int macvlan_open(struct net_device *dev)
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
 	struct net_device *lowerdev = vlan->lowerdev;
@@ -237,7 +237,7 @@ out:
 	return err;
 }
 
-static int macvlan_stop(struct net_device *dev)
+int macvlan_stop(struct net_device *dev)
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
 	struct net_device *lowerdev = vlan->lowerdev;
@@ -318,7 +318,7 @@ static struct lock_class_key macvlan_netdev_addr_lock_key;
 #define MACVLAN_FEATURES \
 	(NETIF_F_SG | NETIF_F_ALL_CSUM | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | \
 	 NETIF_F_GSO | NETIF_F_TSO | NETIF_F_UFO | NETIF_F_GSO_ROBUST | \
-	 NETIF_F_TSO_ECN | NETIF_F_TSO6)
+	 NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_GRO)
 
 #define MACVLAN_STATE_MASK \
 	((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT))
@@ -454,6 +454,44 @@ int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
 }
 EXPORT_SYMBOL_GPL(macvlan_validate);
 
+int macvlan_link_lowerdev(struct net_device *dev,
+			  struct net_device *lowerdev)
+{
+	struct macvlan_dev *vlan = netdev_priv(dev);
+	struct macvlan_port *port;
+	int err = 0;
+
+	if (lowerdev->macvlan_port == NULL) {
+		err = macvlan_port_create(lowerdev);
+		if (err < 0)
+			return err;
+	}
+	port = lowerdev->macvlan_port;
+
+	vlan->lowerdev = lowerdev;
+	vlan->dev      = dev;
+	vlan->port     = port;
+	vlan->receive  = netif_rx;
+
+	macvlan_init(dev);
+
+	list_add_tail(&vlan->list, &port->vlans);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(macvlan_link_lowerdev);
+
+void macvlan_unlink_lowerdev(struct net_device *dev)
+{
+	struct macvlan_dev *vlan = netdev_priv(dev);
+	struct macvlan_port *port = vlan->port;
+
+	list_del(&vlan->list);
+
+	if (list_empty(&port->vlans))
+		macvlan_port_destroy(port->dev);
+}
+EXPORT_SYMBOL_GPL(macvlan_unlink_lowerdev);
+
 static int macvlan_get_tx_queues(struct net *net,
 				 struct nlattr *tb[],
 				 unsigned int *num_tx_queues,
@@ -504,38 +542,26 @@ int macvlan_newlink(struct net_device *dev,
 	if (!tb[IFLA_ADDRESS])
 		random_ether_addr(dev->dev_addr);
 
-	if (lowerdev->macvlan_port == NULL) {
-		err = macvlan_port_create(lowerdev);
-		if (err < 0)
-			return err;
-	}
-	port = lowerdev->macvlan_port;
-
-	vlan->lowerdev = lowerdev;
-	vlan->dev      = dev;
-	vlan->port     = port;
-	vlan->receive  = netif_rx;
+	err = macvlan_link_lowerdev(dev, lowerdev);
+	if (err < 0)
+		return err;
 
 	err = register_netdevice(dev);
-	if (err < 0)
+	if (err < 0) {
+		macvlan_unlink_lowerdev(dev);
 		return err;
+	}
 
-	list_add_tail(&vlan->list, &port->vlans);
 	netif_stacked_transfer_operstate(dev, lowerdev);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(macvlan_newlink);
 
 void macvlan_dellink(struct net_device *dev)
 {
-	struct macvlan_dev *vlan = netdev_priv(dev);
-	struct macvlan_port *port = vlan->port;
-
-	list_del(&vlan->list);
+	macvlan_unlink_lowerdev(dev);
 	unregister_netdevice(dev);
-
-	if (list_empty(&port->vlans))
-		macvlan_port_destroy(port->dev);
 }
 EXPORT_SYMBOL_GPL(macvlan_dellink);
 
diff --git a/include/linux/macvlan.h b/include/linux/macvlan.h
index 3f3c6c3..27f56d9 100644
--- a/include/linux/macvlan.h
+++ b/include/linux/macvlan.h
@@ -24,6 +24,10 @@ struct macvlan_dev {
 };
 
 extern int macvlan_start_xmit(struct sk_buff *skb, struct net_device *dev);
+extern int macvlan_link_lowerdev(struct net_device *dev,
+				 struct net_device *lowerdev);
+
+extern void macvlan_unlink_lowerdev(struct net_device *dev);
 
 extern void macvlan_setup(struct net_device *dev);
 

Re: [PATCH 3/3] macvlan: allow in-kernel modules to create and manage macvlan devices

[Arnd Bergmann]

On Friday 13 November 2009, Patrick Mullaney wrote:
> 
> The macvlan driver didn't allow for creation/deletion of devices
> by other in-kernel modules. This patch provides common routines
> for both in-kernel and netlink based management. This patch
> also enables macvlan device support for gro for lower level
> devices that support gro.

I wonder if doing this way round is a good idea, why don't
you just use netlink to set up the endpoint device like
the current macvlan and macvtap do? I think doing it consistently
for all backends would be a significant advantage.

	Arnd <><

Re: [PATCH 3/3] macvlan: allow in-kernel modules to create and manage macvlan devices

[Arnd Bergmann]

On Friday 13 November 2009, Patrick Mullaney wrote:
> @@ -318,7 +318,7 @@ static struct lock_class_key macvlan_netdev_addr_lock_key;
>  #define MACVLAN_FEATURES \
>         (NETIF_F_SG | NETIF_F_ALL_CSUM | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | \
>          NETIF_F_GSO | NETIF_F_TSO | NETIF_F_UFO | NETIF_F_GSO_ROBUST | \
> -        NETIF_F_TSO_ECN | NETIF_F_TSO6)
> +        NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_GRO)
>  
>  #define MACVLAN_STATE_MASK \
>         ((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT))

This hunk looks like it should be a separate patch, because we will
want to have this independently of the rest. I have taken it into
a series I'm preparing for a new posting of macvtap based on the
current net-next tree with my bridge mode changes. I also have
your patch 1 (the fixed version) and 2 in there. It's currently
work in progress, but if you are interested, take a look at [1].

	Arnd <><

[1] http://git.kernel.org/?p=linux/kernel/git/arnd/playground.git;a=shortlog;h=refs/heads/macvlan

Re: [PATCH 3/3] macvlan: allow in-kernel modules to create and manage macvlan devices

[Patrick Mullaney]

On Fri, 2009-11-27 at 23:14 +0100, Arnd Bergmann wrote:
> On Friday 13 November 2009, Patrick Mullaney wrote:
> > 
> > The macvlan driver didn't allow for creation/deletion of devices
> > by other in-kernel modules. This patch provides common routines
> > for both in-kernel and netlink based management. This patch
> > also enables macvlan device support for gro for lower level
> > devices that support gro.
> 
> I wonder if doing this way round is a good idea, why don't
> you just use netlink to set up the endpoint device like
> the current macvlan and macvtap do? I think doing it consistently
> for all backends would be a significant advantage.

sorry for the late response - I'm thinking about re-implementing
this along the lines that you are talking about. Especially in light
of your new configuration options. The reason(probably short sighted)
for the previous approach was that the creation step was already being
handled in our venet driver(but it doesn't have to be).

Thanks for the suggestion.
Patrick

> 
> 	Arnd <><

Re: [PATCH 3/3] macvlan: allow in-kernel modules to create and manage macvlan devices

[Patrick Mullaney]

I hope I didn't confuse things by posting:

netdevice: provide common routine for macvlan and vlan operstate
management

again. I offered to send that out patched against net-next-2.6 last
week and I just got back to following up. I'm fine with you rolling
them into your series too.

Thanks.

On Fri, 2009-11-27 at 23:19 +0100, Arnd Bergmann wrote:
> On Friday 13 November 2009, Patrick Mullaney wrote:
> > @@ -318,7 +318,7 @@ static struct lock_class_key macvlan_netdev_addr_lock_key;
> >  #define MACVLAN_FEATURES \
> >         (NETIF_F_SG | NETIF_F_ALL_CSUM | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | \
> >          NETIF_F_GSO | NETIF_F_TSO | NETIF_F_UFO | NETIF_F_GSO_ROBUST | \
> > -        NETIF_F_TSO_ECN | NETIF_F_TSO6)
> > +        NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_GRO)
> >  
> >  #define MACVLAN_STATE_MASK \
> >         ((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT))
> 
> This hunk looks like it should be a separate patch, because we will
> want to have this independently of the rest. I have taken it into
> a series I'm preparing for a new posting of macvtap based on the
> current net-next tree with my bridge mode changes. I also have
> your patch 1 (the fixed version) and 2 in there. It's currently
> work in progress, but if you are interested, take a look at [1].
> 
> 	Arnd <><
> 
> [1] http://git.kernel.org/?p=linux/kernel/git/arnd/playground.git;a=shortlog;h=refs/heads/macvlan

Re: [PATCH 3/3] macvlan: allow in-kernel modules to create and manage macvlan devices

[Arnd Bergmann]

On Thursday 03 December 2009 20:45:43 Patrick Mullaney wrote:
> I hope I didn't confuse things by posting:
> 
> netdevice: provide common routine for macvlan and vlan operstate
> management
> 
> again. I offered to send that out patched against net-next-2.6 last
> week and I just got back to following up. I'm fine with you rolling
> them into your series too.

Not at all, thanks for sending that yourself! You did miss the Acked-by
lines from Patrick McHardy and myself though, they should have been
part of your resend. I'll add give you another one then.

	Arnd <><