Re: [RFC PATCH] macvlan: Introduce a PASSTHRU mode to takeover the underlying device

2010-11-02 Thread Michael S. Tsirkin
On Tue, Nov 02, 2010 at 11:19:13AM -0700, Sridhar Samudrala wrote:
 On Mon, 2010-11-01 at 10:28 +0200, Michael S. Tsirkin wrote:
 
 On Tue, Oct 26, 2010 at 03:19:38PM -0700, Sridhar Samudrala wrote:
  With the current default macvtap mode, a KVM guest using virtio with
  macvtap backend has the following limitations.
  - cannot change/add a mac address on the guest virtio-net
  - cannot create a vlan device on the guest virtio-net
  - cannot enable promiscuous mode on guest virtio-net
 
  This patch introduces a new mode called 'passthru' when creating a
  macvlan device which allows takeover of the underlying device and
  passing it to a guest using virtio with macvtap backend.
 
  Only one macvlan device is allowed in passthru mode and it inherits
  the mac address from the underlying device and sets it in promiscuous
  mode to receive and forward all the packets.
 
  Thanks
  Sridhar
 
 One concern with promisc mode is that for the common case,
 which is a single mac and no vlans, we will be getting
 extra packets that will get dropped in userspace/guest
 as compared to the case where same mac is programmed
 in hardware and by guest.
 
 In the tap/bridge model also, the external i/f is put in promiscuous mode and
 the
 bridge does the filtering of extra packets.

Yes but
1. that is much cheaper than passing them all the way up to the guest.
2. it's pretty painful for management to have to decide between
   features and speed. Better give them both :)

 We could let userspace supply a list of mac/vlan addresses through
 an ioctl on macvtap, and then
 1. for a single mac, program it in hardware
 2. for other configurations, set promisc mode
 
 tun already has TUNSETTXFILTER which might come in handy here.
 We don't pass in vlans with the filter now but maybe we should.
 How does this sound?
 
 I guess this can be done. But i am not sure if we can extend the existing
 TUNSETTXFILTER
 to support vlans too. we may need a new ioctl.
 
 Thanks
 Sridhar

OK. Maybe add it to tap too.

-- 
MST
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH] macvlan: Introduce a PASSTHRU mode to takeover the underlying device

2010-11-01 Thread Michael S. Tsirkin
On Tue, Oct 26, 2010 at 03:19:38PM -0700, Sridhar Samudrala wrote:
 With the current default macvtap mode, a KVM guest using virtio with 
 macvtap backend has the following limitations.
 - cannot change/add a mac address on the guest virtio-net
 - cannot create a vlan device on the guest virtio-net
 - cannot enable promiscuous mode on guest virtio-net
 
 This patch introduces a new mode called 'passthru' when creating a 
 macvlan device which allows takeover of the underlying device and 
 passing it to a guest using virtio with macvtap backend.
 
 Only one macvlan device is allowed in passthru mode and it inherits
 the mac address from the underlying device and sets it in promiscuous 
 mode to receive and forward all the packets.
 
 Thanks
 Sridhar

One concern with promisc mode is that for the common case,
which is a single mac and no vlans, we will be getting
extra packets that will get dropped in userspace/guest
as compared to the case where same mac is programmed
in hardware and by guest.

We could let userspace supply a list of mac/vlan addresses through
an ioctl on macvtap, and then
1. for a single mac, program it in hardware
2. for other configurations, set promisc mode

tun already has TUNSETTXFILTER which might come in handy here.
We don't pass in vlans with the filter now but maybe we should.
How does this sound?

 diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
 index 0ef0eb0..bca3cb7 100644
 --- a/drivers/net/macvlan.c
 +++ b/drivers/net/macvlan.c
 @@ -38,6 +38,7 @@ struct macvlan_port {
   struct hlist_head   vlan_hash[MACVLAN_HASH_SIZE];
   struct list_headvlans;
   struct rcu_head rcu;
 + boolpassthru;
  };
  
  #define macvlan_port_get_rcu(dev) \
 @@ -169,6 +170,7 @@ static struct sk_buff *macvlan_handle_frame(struct 
 sk_buff *skb)
   macvlan_broadcast(skb, port, NULL,
 MACVLAN_MODE_PRIVATE |
 MACVLAN_MODE_VEPA|
 +   MACVLAN_MODE_PASSTHRU|
 MACVLAN_MODE_BRIDGE);
   else if (src-mode == MACVLAN_MODE_VEPA)
   /* flood to everyone except source */
 @@ -185,7 +187,10 @@ static struct sk_buff *macvlan_handle_frame(struct 
 sk_buff *skb)
   return skb;
   }
  
 - vlan = macvlan_hash_lookup(port, eth-h_dest);
 + if (port-passthru)
 + vlan = list_first_entry(port-vlans, struct macvlan_dev, list);
 + else
 + vlan = macvlan_hash_lookup(port, eth-h_dest);
   if (vlan == NULL)
   return skb;
  
 @@ -284,6 +289,11 @@ static int macvlan_open(struct net_device *dev)
   struct net_device *lowerdev = vlan-lowerdev;
   int err;
  
 + if (vlan-port-passthru) {
 + dev_set_promiscuity(lowerdev, 1);
 + goto hash_add;
 + }
 +
   err = -EBUSY;
   if (macvlan_addr_busy(vlan-port, dev-dev_addr))
   goto out;
 @@ -296,6 +306,8 @@ static int macvlan_open(struct net_device *dev)
   if (err  0)
   goto del_unicast;
   }
 +
 +hash_add:
   macvlan_hash_add(vlan);
   return 0;
  
 @@ -310,12 +322,18 @@ static int macvlan_stop(struct net_device *dev)
   struct macvlan_dev *vlan = netdev_priv(dev);
   struct net_device *lowerdev = vlan-lowerdev;
  
 + if (vlan-port-passthru) {
 + dev_set_promiscuity(lowerdev, -1);
 + goto hash_del;
 + }
 +
   dev_mc_unsync(lowerdev, dev);
   if (dev-flags  IFF_ALLMULTI)
   dev_set_allmulti(lowerdev, -1);
  
   dev_uc_del(lowerdev, dev-dev_addr);
  
 +hash_del:
   macvlan_hash_del(vlan);
   return 0;
  }
 @@ -549,6 +567,7 @@ static int macvlan_port_create(struct net_device *dev)
   if (port == NULL)
   return -ENOMEM;
  
 + port-passthru = false;
   port-dev = dev;
   INIT_LIST_HEAD(port-vlans);
   for (i = 0; i  MACVLAN_HASH_SIZE; i++)
 @@ -593,6 +612,7 @@ static int macvlan_validate(struct nlattr *tb[], struct 
 nlattr *data[])
   case MACVLAN_MODE_PRIVATE:
   case MACVLAN_MODE_VEPA:
   case MACVLAN_MODE_BRIDGE:
 + case MACVLAN_MODE_PASSTHRU:
   break;
   default:
   return -EINVAL;
 @@ -661,6 +681,10 @@ int macvlan_common_newlink(struct net *src_net, struct 
 net_device *dev,
   }
   port = macvlan_port_get(lowerdev);
  
 + /* Only 1 macvlan device can be created in passthru mode */
 + if (port-passthru)
 + return -EINVAL;
 +
   vlan-lowerdev = lowerdev;
   vlan-dev  = dev;
   vlan-port = port;
 @@ -671,6 +695,13 @@ int macvlan_common_newlink(struct net *src_net, struct 
 net_device *dev,
   if (data  data[IFLA_MACVLAN_MODE])
   vlan-mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
  
 

Re: [RFC PATCH] macvlan: Introduce a PASSTHRU mode to takeover the underlying device

2010-10-27 Thread Arnd Bergmann
On Wednesday 27 October 2010, Sridhar Samudrala wrote:
 With the current default macvtap mode, a KVM guest using virtio with 
 macvtap backend has the following limitations.
 - cannot change/add a mac address on the guest virtio-net
 - cannot create a vlan device on the guest virtio-net
 - cannot enable promiscuous mode on guest virtio-net
 
 This patch introduces a new mode called 'passthru' when creating a 
 macvlan device which allows takeover of the underlying device and 
 passing it to a guest using virtio with macvtap backend.
 
 Only one macvlan device is allowed in passthru mode and it inherits
 the mac address from the underlying device and sets it in promiscuous 
 mode to receive and forward all the packets.

Interesting approach. It somewhat stretches the definition of the
macvlan concept, but it does sound useful to have.

I was thinking about adding a new tap frontend driver that could
share some code with macvtap and do only the takeover but not
use macvlan as a base. I believe that would be a cleaner abstraction,
but your code has two advantages in that the implementation is much
simpler and that it can share a fair amount of the infrastructure
that we're putting into qemu/libvirt/etc.

Arnd

PS: Please add a Signed-off-by: line when sending a patch, even for
discussion.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCH] macvlan: Introduce a PASSTHRU mode to takeover the underlying device

2010-10-26 Thread Sridhar Samudrala
With the current default macvtap mode, a KVM guest using virtio with 
macvtap backend has the following limitations.
- cannot change/add a mac address on the guest virtio-net
- cannot create a vlan device on the guest virtio-net
- cannot enable promiscuous mode on guest virtio-net

This patch introduces a new mode called 'passthru' when creating a 
macvlan device which allows takeover of the underlying device and 
passing it to a guest using virtio with macvtap backend.

Only one macvlan device is allowed in passthru mode and it inherits
the mac address from the underlying device and sets it in promiscuous 
mode to receive and forward all the packets.

Thanks
Sridhar

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 0ef0eb0..bca3cb7 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -38,6 +38,7 @@ struct macvlan_port {
struct hlist_head   vlan_hash[MACVLAN_HASH_SIZE];
struct list_headvlans;
struct rcu_head rcu;
+   boolpassthru;
 };
 
 #define macvlan_port_get_rcu(dev) \
@@ -169,6 +170,7 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff 
*skb)
macvlan_broadcast(skb, port, NULL,
  MACVLAN_MODE_PRIVATE |
  MACVLAN_MODE_VEPA|
+ MACVLAN_MODE_PASSTHRU|
  MACVLAN_MODE_BRIDGE);
else if (src-mode == MACVLAN_MODE_VEPA)
/* flood to everyone except source */
@@ -185,7 +187,10 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff 
*skb)
return skb;
}
 
-   vlan = macvlan_hash_lookup(port, eth-h_dest);
+   if (port-passthru)
+   vlan = list_first_entry(port-vlans, struct macvlan_dev, list);
+   else
+   vlan = macvlan_hash_lookup(port, eth-h_dest);
if (vlan == NULL)
return skb;
 
@@ -284,6 +289,11 @@ static int macvlan_open(struct net_device *dev)
struct net_device *lowerdev = vlan-lowerdev;
int err;
 
+   if (vlan-port-passthru) {
+   dev_set_promiscuity(lowerdev, 1);
+   goto hash_add;
+   }
+
err = -EBUSY;
if (macvlan_addr_busy(vlan-port, dev-dev_addr))
goto out;
@@ -296,6 +306,8 @@ static int macvlan_open(struct net_device *dev)
if (err  0)
goto del_unicast;
}
+
+hash_add:
macvlan_hash_add(vlan);
return 0;
 
@@ -310,12 +322,18 @@ static int macvlan_stop(struct net_device *dev)
struct macvlan_dev *vlan = netdev_priv(dev);
struct net_device *lowerdev = vlan-lowerdev;
 
+   if (vlan-port-passthru) {
+   dev_set_promiscuity(lowerdev, -1);
+   goto hash_del;
+   }
+
dev_mc_unsync(lowerdev, dev);
if (dev-flags  IFF_ALLMULTI)
dev_set_allmulti(lowerdev, -1);
 
dev_uc_del(lowerdev, dev-dev_addr);
 
+hash_del:
macvlan_hash_del(vlan);
return 0;
 }
@@ -549,6 +567,7 @@ static int macvlan_port_create(struct net_device *dev)
if (port == NULL)
return -ENOMEM;
 
+   port-passthru = false;
port-dev = dev;
INIT_LIST_HEAD(port-vlans);
for (i = 0; i  MACVLAN_HASH_SIZE; i++)
@@ -593,6 +612,7 @@ static int macvlan_validate(struct nlattr *tb[], struct 
nlattr *data[])
case MACVLAN_MODE_PRIVATE:
case MACVLAN_MODE_VEPA:
case MACVLAN_MODE_BRIDGE:
+   case MACVLAN_MODE_PASSTHRU:
break;
default:
return -EINVAL;
@@ -661,6 +681,10 @@ int macvlan_common_newlink(struct net *src_net, struct 
net_device *dev,
}
port = macvlan_port_get(lowerdev);
 
+   /* Only 1 macvlan device can be created in passthru mode */
+   if (port-passthru)
+   return -EINVAL;
+
vlan-lowerdev = lowerdev;
vlan-dev  = dev;
vlan-port = port;
@@ -671,6 +695,13 @@ int macvlan_common_newlink(struct net *src_net, struct 
net_device *dev,
if (data  data[IFLA_MACVLAN_MODE])
vlan-mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
 
+   if (vlan-mode == MACVLAN_MODE_PASSTHRU) {
+   if (!list_empty(port-vlans))
+   return -EINVAL;
+   port-passthru = true;
+   memcpy(dev-dev_addr, lowerdev-dev_addr, ETH_ALEN);
+   }
+
err = register_netdevice(dev);
if (err  0)
goto destroy_port;
diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index 2fc66dd..8454805 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -232,6 +232,7 @@ enum macvlan_mode {
MACVLAN_MODE_PRIVATE = 1, /* don't talk to other macvlans */
MACVLAN_MODE_VEPA=