Re: [PATCH 0/3] macvlan: support for guest vm direct rx/tx

Re: [PATCH 0/3] macvlan: support for guest vm direct rx/tx

[Patrick Mullaney]

On Fri, 2009-11-13 at 13:27 -0800, Stephen Hemminger wrote:
> On Fri, 13 Nov 2009 14:55:06 -0500
> Patrick Mullaney <pmullaney@novell.com> wrote:
> 
> > These patches allow other modules to override the receive
> > path of a macvlan. This is being done to support guest
> > VMs operating directly over a macvlan. Routines to allow
> > creation and deletion of macvlans from in-kernel modules
> > were also exposed/added.
> 
> Which guest VM, how will it use it? The kernel is not in the business
> of providing infrastructure for out of tree patches.

Actually, any guest vm or container. macvtap was the first to suggest
a patch like this to provide this functionality. This infrastructure
is generic enough to allow others to use it. My interest is in
supporting kvm guests via vbus drivers which are being integrated
in the alacrityvm tree.

> 
> Also, macvlan should really being calling netif_receive_skb()
> not going through another queue/softirq cycle.
I understand but you are talking about the current behavior of
macvlans - my goal wasn't to change the current behavior
of macvlans, as I didn't want to disturb what the original author
intended, just provide the ability to override the rx path and
provide for management of macvlans from other kernel modules(not
just via rtnl).

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

[Patrick McHardy]

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.

> -static void macvlan_transfer_operstate(struct net_device *dev)
> +void macvlan_transfer_operstate(struct net_device *dev)
>  {
>  	struct macvlan_dev *vlan = netdev_priv(dev);
>  	const struct net_device *lowerdev = vlan->lowerdev;
> @@ -458,6 +458,7 @@ static void macvlan_transfer_operstate(struct net_device *dev)
>  			netif_carrier_off(dev);
>  	}
>  }
> +EXPORT_SYMBOL_GPL(macvlan_transfer_operstate);

I think this function could be moved to net/core/dev.c or
net/core/link_watch.c. The VLAN code has an identical copy.

> -int macvlan_newlink(struct net_device *dev,
> -		    struct nlattr *tb[], struct nlattr *data[])
> +int macvlan_link_lowerdev(struct net_device *dev,
> +						  struct net_device *lowerdev)

Please indent this more cleanly.

>  {
>  	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);

> @@ -502,23 +539,14 @@ 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)
>  		return err;

You've already added the device to the port->vlans list, so you
need to remove it again when register_netdevice() fails.

> -	list_add_tail(&vlan->list, &port->vlans);
>  	macvlan_transfer_operstate(dev);
>  	return 0;
>  }
> @@ -526,14 +554,8 @@ EXPORT_SYMBOL_GPL(macvlan_newlink);

[PATCH 0/3] macvlan: support for guest vm direct rx/tx

[Patrick Mullaney]

(Applies to net-2.6.git/master:1dfc5827)

These patches allow other modules to override the receive
path of a macvlan. This is being done to support guest
VMs operating directly over a macvlan. Routines to allow
creation and deletion of macvlans from in-kernel modules
were also exposed/added.

---

Patrick Mullaney (3):
      macvlan: allow in-kernel modules to create and manage macvlan devices
      macvlan:  derived from Arnd Bergmann's patch for macvtap
      netdevice: provide common routine for macvlan and vlan operstate management


 drivers/net/macvlan.c     |  135 ++++++++++++++++++++++-----------------------
 include/linux/macvlan.h   |   41 ++++++++++++++
 include/linux/netdevice.h |    3 +
 net/8021q/vlan.c          |   29 +---------
 net/core/dev.c            |   27 +++++++++
 5 files changed, 142 insertions(+), 93 deletions(-)
 create mode 100644 include/linux/macvlan.h

Re: [PATCH 0/3] macvlan: support for guest vm direct rx/tx

[Stephen Hemminger]

On Fri, 13 Nov 2009 14:55:06 -0500
Patrick Mullaney <pmullaney@novell.com> wrote:

> These patches allow other modules to override the receive
> path of a macvlan. This is being done to support guest
> VMs operating directly over a macvlan. Routines to allow
> creation and deletion of macvlans from in-kernel modules
> were also exposed/added.

Which guest VM, how will it use it? The kernel is not in the business
of providing infrastructure for out of tree patches.

Also, macvlan should really being calling netif_receive_skb()
not going through another queue/softirq cycle.

Re: [PATCH 0/3] macvlan: support for guest vm direct rx/tx

[Arnd Bergmann]

On Friday 13 November 2009, Stephen Hemminger wrote:
> Also, macvlan should really being calling netif_receive_skb()
> not going through another queue/softirq cycle.

I've added a patch for this in my experimental queue now.
When I last tried this, I saw a kernel stack overflow
but it seems fine now.

I wonder if we can also return from the macvlan hook with
skb->dev changed to netif_receive_skb rather than calling it
recursively.

	Arnd <><

Re: [PATCH 0/3] macvlan: support for guest vm direct rx/tx

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Sat, 28 Nov 2009 00:43:58 +0100

> On Friday 13 November 2009, Stephen Hemminger wrote:
>> Also, macvlan should really being calling netif_receive_skb()
>> not going through another queue/softirq cycle.
> 
> I've added a patch for this in my experimental queue now.
> When I last tried this, I saw a kernel stack overflow
> but it seems fine now.

I think it is unwise for any virtual device layer to use netif_receive_skb().
Just like tunnels they should always use netif_rx().

Otherwise stack overflow is a very real concern.

Re: [PATCH 0/3] macvlan: support for guest vm direct rx/tx

[Stephen Hemminger]

On Fri, 27 Nov 2009 16:19:57 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Arnd Bergmann <arnd@arndb.de>
> Date: Sat, 28 Nov 2009 00:43:58 +0100
> 
> > On Friday 13 November 2009, Stephen Hemminger wrote:
> >> Also, macvlan should really being calling netif_receive_skb()
> >> not going through another queue/softirq cycle.
> > 
> > I've added a patch for this in my experimental queue now.
> > When I last tried this, I saw a kernel stack overflow
> > but it seems fine now.
> 
> I think it is unwise for any virtual device layer to use netif_receive_skb().
> Just like tunnels they should always use netif_rx().
> 
> Otherwise stack overflow is a very real concern.

Maybe we should figure out a way for protocols to return new skb in netif_receive_skb
to avoid extra softirq, but avoid stack overflow?

-- 

Re: [PATCH 0/3] macvlan: support for guest vm direct rx/tx

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Fri, 27 Nov 2009 21:38:24 -0800

> Maybe we should figure out a way for protocols to return new skb in netif_receive_skb
> to avoid extra softirq, but avoid stack overflow?

Eric Dumazet and I tried to find ways to handle this, please see the
archives.  It's not an easy problem to solve and none of the patches
we came up with avoided crashes under high stress scenarios.