[PATCHv2 net-next 0/4] macvtap: locking and offload control

[PATCHv2 net-next 0/4] macvtap: locking and offload control

[Vlad Yasevich]

This series of patches changes the macvtap locking to use rtnl instead
of a private lock and adds the ability for user to specify TAP offload
features.

The macvtap_lock protects the relationship between macvlan_dev and
macvtap_queue structures.  This relationship is modified in 3 places:
open(), release(), and dellinks().  dellinks() is already protected
by rtnl through the rtnetlink message handling.  If we add rtnl protection
during the open() and relase() calls, we can remove the private lock.

We can also stop using the _bh veriants of rcu since in most cases
either regular rcu is already taken or we don't need to disable bh.

Macvtap does not allow tap user to control of offload functionality via
TUNSETOFFLOAD ioctl.  This is the ioctl that qemu uses when
attempting to enable or disable offload features that it expects on the
tap socket.  The 3rd patch adds this support, but there is a small
wrinkle.  Typcally, tap offloads are reversed.  When the user disables
TSO to the tap, it does not expect to receive TSO packets.  To do this
here, we reverse the sence such that when TSO is disabled, we actuall
disalbe GRO on the macvtap interface, but also save the user expected
offloads so that we can properly perform segmentation when passing
data to the user (patch 4).

Changes since v1:
 - use rtnl locks.
 - reverse the sence of offload features to make them more consitent
   with user of tap.

Vlad Yasevich (4):
  macvtap: Convert to using rtnl lock
  macvtap: Consistently use rcu functions
  macvtap: Let TUNSETOFFLOAD actually controll offload features.
  macvtap: Perform GSO on forwarding path.

 drivers/net/macvlan.c      |  10 +++
 drivers/net/macvtap.c      | 176 ++++++++++++++++++++++++++++++++-------------
 include/linux/if_macvlan.h |   2 +
 3 files changed, 140 insertions(+), 48 deletions(-)

-- 
1.8.1.4

[PATCHv2 net-next 1/4] macvtap: Convert to using rtnl lock

[Vlad Yasevich]

Macvtap uses a private lock to protect the relationship between
macvtap_queue and macvlan_dev.  The private lock is not needed
since the relationship is managed by user via open(), release(),
and dellink() calls.  dellink() already happens under rtnl, so
we can safely convert open() and release(), and use it in ioctl()
as well.

Suggested by Eric Dumazet.

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
 drivers/net/macvtap.c | 62 +++++++++++++++++++++------------------------------
 1 file changed, 25 insertions(+), 37 deletions(-)

diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 5a76f20..efbf2eb 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -69,7 +69,7 @@ static const struct proto_ops macvtap_socket_ops;
  * RCU usage:
  * The macvtap_queue and the macvlan_dev are loosely coupled, the
  * pointers from one to the other can only be read while rcu_read_lock
- * or macvtap_lock is held.
+ * or rtnl is held.
  *
  * Both the file and the macvlan_dev hold a reference on the macvtap_queue
  * through sock_hold(&q->sk). When the macvlan_dev goes away first,
@@ -81,7 +81,6 @@ static const struct proto_ops macvtap_socket_ops;
  * file or the dev. The data structure is freed through __sk_free
  * when both our references and any pending SKBs are gone.
  */
-static DEFINE_SPINLOCK(macvtap_lock);
 
 static int macvtap_enable_queue(struct net_device *dev, struct file *file,
 				struct macvtap_queue *q)
@@ -89,7 +88,7 @@ static int macvtap_enable_queue(struct net_device *dev, struct file *file,
 	struct macvlan_dev *vlan = netdev_priv(dev);
 	int err = -EINVAL;
 
-	spin_lock(&macvtap_lock);
+	ASSERT_RTNL();
 
 	if (q->enabled)
 		goto out;
@@ -101,7 +100,6 @@ static int macvtap_enable_queue(struct net_device *dev, struct file *file,
 
 	vlan->numvtaps++;
 out:
-	spin_unlock(&macvtap_lock);
 	return err;
 }
 
@@ -111,7 +109,7 @@ static int macvtap_set_queue(struct net_device *dev, struct file *file,
 	struct macvlan_dev *vlan = netdev_priv(dev);
 	int err = -EBUSY;
 
-	spin_lock(&macvtap_lock);
+	rtnl_lock();
 	if (vlan->numqueues == MAX_MACVTAP_QUEUES)
 		goto out;
 
@@ -130,26 +128,25 @@ static int macvtap_set_queue(struct net_device *dev, struct file *file,
 	vlan->numqueues++;
 
 out:
-	spin_unlock(&macvtap_lock);
+	rtnl_unlock();
 	return err;
 }
 
-static int __macvtap_disable_queue(struct macvtap_queue *q)
+static int macvtap_disable_queue(struct macvtap_queue *q)
 {
 	struct macvlan_dev *vlan;
 	struct macvtap_queue *nq;
 
-	vlan = rcu_dereference_protected(q->vlan,
-					 lockdep_is_held(&macvtap_lock));
-
+	ASSERT_RTNL();
 	if (!q->enabled)
 		return -EINVAL;
 
+	vlan = rtnl_dereference(q->vlan);
+
 	if (vlan) {
 		int index = q->queue_index;
 		BUG_ON(index >= vlan->numvtaps);
-		nq = rcu_dereference_protected(vlan->taps[vlan->numvtaps - 1],
-					       lockdep_is_held(&macvtap_lock));
+		nq = rtnl_dereference(vlan->taps[vlan->numvtaps - 1]);
 		nq->queue_index = index;
 
 		rcu_assign_pointer(vlan->taps[index], nq);
@@ -162,17 +159,6 @@ static int __macvtap_disable_queue(struct macvtap_queue *q)
 	return 0;
 }
 
-static int macvtap_disable_queue(struct macvtap_queue *q)
-{
-	int err;
-
-	spin_lock(&macvtap_lock);
-	err = __macvtap_disable_queue(q);
-	spin_unlock(&macvtap_lock);
-
-	return err;
-}
-
 /*
  * The file owning the queue got closed, give up both
  * the reference that the files holds as well as the
@@ -185,12 +171,12 @@ static void macvtap_put_queue(struct macvtap_queue *q)
 {
 	struct macvlan_dev *vlan;
 
-	spin_lock(&macvtap_lock);
-	vlan = rcu_dereference_protected(q->vlan,
-					 lockdep_is_held(&macvtap_lock));
+	rtnl_lock();
+	vlan = rtnl_dereference(q->vlan);
+
 	if (vlan) {
 		if (q->enabled)
-			BUG_ON(__macvtap_disable_queue(q));
+			BUG_ON(macvtap_disable_queue(q));
 
 		vlan->numqueues--;
 		RCU_INIT_POINTER(q->vlan, NULL);
@@ -198,7 +184,7 @@ static void macvtap_put_queue(struct macvtap_queue *q)
 		list_del_init(&q->next);
 	}
 
-	spin_unlock(&macvtap_lock);
+	rtnl_unlock();
 
 	synchronize_rcu();
 	sock_put(&q->sk);
@@ -260,7 +246,7 @@ static void macvtap_del_queues(struct net_device *dev)
 	struct macvtap_queue *q, *tmp, *qlist[MAX_MACVTAP_QUEUES];
 	int i, j = 0;
 
-	spin_lock(&macvtap_lock);
+	ASSERT_RTNL();
 	list_for_each_entry_safe(q, tmp, &vlan->queue_list, next) {
 		list_del_init(&q->next);
 		qlist[j++] = q;
@@ -275,9 +261,6 @@ static void macvtap_del_queues(struct net_device *dev)
 	BUG_ON(vlan->numqueues);
 	/* guarantee that any future macvtap_set_queue will fail */
 	vlan->numvtaps = MAX_MACVTAP_QUEUES;
-	spin_unlock(&macvtap_lock);
-
-	synchronize_rcu();
 
 	for (--j; j >= 0; j--)
 		sock_put(&qlist[j]->sk);
@@ -941,11 +924,10 @@ static struct macvlan_dev *macvtap_get_vlan(struct macvtap_queue *q)
 {
 	struct macvlan_dev *vlan;
 
-	rcu_read_lock_bh();
-	vlan = rcu_dereference_bh(q->vlan);
+	ASSERT_RTNL();
+	vlan = rtnl_dereference(q->vlan);
 	if (vlan)
 		dev_hold(vlan->dev);
-	rcu_read_unlock_bh();
 
 	return vlan;
 }
@@ -1008,21 +990,27 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
 		return ret;
 
 	case TUNGETIFF:
+		rtnl_lock();
 		vlan = macvtap_get_vlan(q);
-		if (!vlan)
+		if (!vlan) {
+			rtnl_unlock();
 			return -ENOLINK;
+		}
 
 		ret = 0;
 		if (copy_to_user(&ifr->ifr_name, vlan->dev->name, IFNAMSIZ) ||
 		    put_user(q->flags, &ifr->ifr_flags))
 			ret = -EFAULT;
 		macvtap_put_vlan(vlan);
+		rtnl_unlock();
 		return ret;
 
 	case TUNSETQUEUE:
 		if (get_user(u, &ifr->ifr_flags))
 			return -EFAULT;
-		return macvtap_ioctl_set_queue(file, u);
+		rtnl_lock();
+		ret = macvtap_ioctl_set_queue(file, u);
+		rtnl_unlock();
 
 	case TUNGETFEATURES:
 		if (put_user(IFF_TAP | IFF_NO_PI | IFF_VNET_HDR |
-- 
1.8.1.4

Re: [PATCHv2 net-next 1/4] macvtap: Convert to using rtnl lock

[Eric Dumazet]

On Tue, 2013-06-25 at 16:04 -0400, Vlad Yasevich wrote:
> Macvtap uses a private lock to protect the relationship between
> macvtap_queue and macvlan_dev.  The private lock is not needed
> since the relationship is managed by user via open(), release(),
> and dellink() calls.  dellink() already happens under rtnl, so
> we can safely convert open() and release(), and use it in ioctl()
> as well.
> 
> Suggested by Eric Dumazet.
> 
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
>  drivers/net/macvtap.c | 62 +++++++++++++++++++++------------------------------
>  1 file changed, 25 insertions(+), 37 deletions(-)

Looks fine to me, thanks !

Reviewed-by: Eric Dumazet <edumazet@google.com>

[PATCHv2 net-next 2/4] macvtap: Consistently use rcu functions

[Vlad Yasevich]

Currently macvtap uses rcu_bh functions in its
user facing fuction macvtap_get_user() and macvtap_put_user().
However, its packet handlers use normal rcu as the rcu_read_lock()
is taken in netif_receive_skb().  We can safely discontinue
the usage or rcu with bh disabled.

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
 drivers/net/macvtap.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index efbf2eb..d7856a8 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -754,8 +754,8 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
 
 	skb_probe_transport_header(skb, ETH_HLEN);
 
-	rcu_read_lock_bh();
-	vlan = rcu_dereference_bh(q->vlan);
+	rcu_read_lock();
+	vlan = rcu_dereference(q->vlan);
 	/* copy skb_ubuf_info for callback when skb has no error */
 	if (zerocopy) {
 		skb_shinfo(skb)->destructor_arg = m->msg_control;
@@ -766,7 +766,7 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
 		macvlan_start_xmit(skb, vlan->dev);
 	else
 		kfree_skb(skb);
-	rcu_read_unlock_bh();
+	rcu_read_unlock();
 
 	return total_len;
 
@@ -774,11 +774,11 @@ err_kfree:
 	kfree_skb(skb);
 
 err:
-	rcu_read_lock_bh();
-	vlan = rcu_dereference_bh(q->vlan);
+	rcu_read_lock();
+	vlan = rcu_dereference(q->vlan);
 	if (vlan)
 		vlan->dev->stats.tx_dropped++;
-	rcu_read_unlock_bh();
+	rcu_read_unlock();
 
 	return err;
 }
@@ -854,11 +854,11 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
 	copied += len;
 
 done:
-	rcu_read_lock_bh();
-	vlan = rcu_dereference_bh(q->vlan);
+	rcu_read_lock();
+	vlan = rcu_dereference(q->vlan);
 	if (vlan)
 		macvlan_count_rx(vlan, copied - vnet_hdr_len, ret == 0, 0);
-	rcu_read_unlock_bh();
+	rcu_read_unlock();
 
 	return ret ? ret : copied;
 }
-- 
1.8.1.4

Re: [PATCHv2 net-next 2/4] macvtap: Consistently use rcu functions

[Eric Dumazet]

On Tue, 2013-06-25 at 16:04 -0400, Vlad Yasevich wrote:
> Currently macvtap uses rcu_bh functions in its
> user facing fuction macvtap_get_user() and macvtap_put_user().
> However, its packet handlers use normal rcu as the rcu_read_lock()
> is taken in netif_receive_skb().  We can safely discontinue
> the usage or rcu with bh disabled.
> 
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
>  drivers/net/macvtap.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)

Reviewed-by: Eric Dumazet <edumazet@google.com>

[PATCHv2 net-next 3/4] macvtap: Let TUNSETOFFLOAD actually controll offload features.

[Vlad Yasevich]

When the user issues TUNSETOFFLOAD ioctl, macvtap does not do
anything other then to verify arguments.  This patch adds
functionality to allow users to actually control offload features.
NETIF_F_GSO and NETIF_F_GRO are always on, but the rest of the
features can be controlled.

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
 drivers/net/macvlan.c      | 10 +++++++
 drivers/net/macvtap.c      | 65 +++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/if_macvlan.h |  2 ++
 3 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index edfddc5..61c53a8 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -638,6 +638,14 @@ static int macvlan_ethtool_get_settings(struct net_device *dev,
 	return __ethtool_get_settings(vlan->lowerdev, cmd);
 }
 
+static netdev_features_t macvlan_fix_features(struct net_device *dev,
+					      netdev_features_t features)
+{
+	struct macvlan_dev *vlan = netdev_priv(dev);
+
+	return features & (vlan->set_features | ~MACVLAN_FEATURES);
+}
+
 static const struct ethtool_ops macvlan_ethtool_ops = {
 	.get_link		= ethtool_op_get_link,
 	.get_settings		= macvlan_ethtool_get_settings,
@@ -651,6 +659,7 @@ static const struct net_device_ops macvlan_netdev_ops = {
 	.ndo_stop		= macvlan_stop,
 	.ndo_start_xmit		= macvlan_start_xmit,
 	.ndo_change_mtu		= macvlan_change_mtu,
+	.ndo_fix_features	= macvlan_fix_features,
 	.ndo_change_rx_flags	= macvlan_change_rx_flags,
 	.ndo_set_mac_address	= macvlan_set_mac_address,
 	.ndo_set_rx_mode	= macvlan_set_mac_lists,
@@ -791,6 +800,7 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
 	vlan->port     = port;
 	vlan->receive  = receive;
 	vlan->forward  = forward;
+	vlan->set_features = MACVLAN_FEATURES;
 
 	vlan->mode     = MACVLAN_MODE_VEPA;
 	if (data && data[IFLA_MACVLAN_MODE])
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index d7856a8..7eab019 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -65,6 +65,9 @@ static struct cdev macvtap_cdev;
 
 static const struct proto_ops macvtap_socket_ops;
 
+#define TUN_OFFLOADS (NETIF_F_HW_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
+		      NETIF_F_TSO6 | NETIF_F_UFO)
+#define RX_OFFLOADS (NETIF_F_GRO | NETIF_F_LRO)
 /*
  * RCU usage:
  * The macvtap_queue and the macvlan_dev are loosely coupled, the
@@ -349,6 +352,11 @@ static int macvtap_newlink(struct net *src_net,
 	struct macvlan_dev *vlan = netdev_priv(dev);
 	INIT_LIST_HEAD(&vlan->queue_list);
 
+	/* Since macvlan supports all offloads by default, make
+	 * tap support all offloads also.
+	 */
+	vlan->tap_features = TUN_OFFLOADS;
+
 	/* Don't put anything that may fail after macvlan_common_newlink
 	 * because we can't undo what it does.
 	 */
@@ -958,6 +966,58 @@ static int macvtap_ioctl_set_queue(struct file *file, unsigned int flags)
 	return ret;
 }
 
+static int set_offload(struct macvtap_queue *q, unsigned long arg)
+{
+	struct macvlan_dev *vlan;
+	netdev_features_t features;
+	netdev_features_t feature_mask = 0;
+
+	vlan = rtnl_dereference(q->vlan);
+	if (!vlan)
+		return -ENOLINK;
+
+	features = vlan->dev->features;
+
+	if (arg & TUN_F_CSUM) {
+		feature_mask = NETIF_F_HW_CSUM;
+
+		if (arg & (TUN_F_TSO4 | TUN_F_TSO6)) {
+			if (arg & TUN_F_TSO_ECN)
+				feature_mask |= NETIF_F_TSO_ECN;
+			if (arg & TUN_F_TSO4)
+				feature_mask |= NETIF_F_TSO;
+			if (arg & TUN_F_TSO6)
+				feature_mask |= NETIF_F_TSO6;
+		}
+
+		if (arg & TUN_F_UFO)
+			feature_mask |= NETIF_F_UFO;
+	}
+
+	/* tun/tap driver inverts the usage for TSO offloads, where
+	 * setting the TSO bit means that the userspace wants to
+	 * accept TSO frames and turning it off means that user space
+	 * does not support TSO.
+	 * For macvtap, we have to invert it to mean the same thing.
+	 * When user space turns off TSO, we turn off GSO/LRO so that
+	 * user-space will not receive TSO frames.
+	 */
+	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_UFO))
+		features |= RX_OFFLOADS;
+	else
+		features &= ~RX_OFFLOADS;
+
+	/* tap_features are the same as features on tun/tap and
+	 * reflect user expectations.
+	 */
+	vlan->tap_features = vlan->dev->features &
+			    (feature_mask | ~TUN_OFFLOADS);
+	vlan->set_features = features;
+	netdev_update_features(vlan->dev);
+
+	return 0;
+}
+
 /*
  * provide compatibility with generic tun/tap interface
  */
@@ -1050,7 +1110,10 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
 			 got enabled for forwarded frames */
 		if (!(q->flags & IFF_VNET_HDR))
 			return  -EINVAL;
-		return 0;
+		rtnl_lock();
+		ret = set_offload(q, arg);
+		rtnl_unlock();
+		return ret;
 
 	default:
 		return -EINVAL;
diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
index f49a9f6..ddd33fd 100644
--- a/include/linux/if_macvlan.h
+++ b/include/linux/if_macvlan.h
@@ -65,6 +65,7 @@ struct macvlan_dev {
 
 	DECLARE_BITMAP(mc_filter, MACVLAN_MC_FILTER_SZ);
 
+	netdev_features_t	set_features;
 	enum macvlan_mode	mode;
 	u16			flags;
 	int (*receive)(struct sk_buff *skb);
@@ -75,6 +76,7 @@ struct macvlan_dev {
 	struct list_head	queue_list;
 	int			numvtaps;
 	int			numqueues;
+	netdev_features_t	tap_features;
 	int			minor;
 };
 
-- 
1.8.1.4

[PATCHv2 net-next 4/4] macvtap: Perform GSO on forwarding path.

[Vlad Yasevich]

When macvtap forwards skb to its tap, it needs to check
if GSO needs to be performed.  This is sometimes necessary
when the HW device performed GRO, but the guest reading
from the tap does not support it (ex: Windows 7).

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
 drivers/net/macvtap.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 7eab019..e370b79 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -276,14 +276,43 @@ static void macvtap_del_queues(struct net_device *dev)
  */
 static int macvtap_forward(struct net_device *dev, struct sk_buff *skb)
 {
+	struct macvlan_dev *vlan = netdev_priv(dev);
 	struct macvtap_queue *q = macvtap_get_queue(dev, skb);
+	netdev_features_t features;
 	if (!q)
 		goto drop;
 
 	if (skb_queue_len(&q->sk.sk_receive_queue) >= dev->tx_queue_len)
 		goto drop;
 
-	skb_queue_tail(&q->sk.sk_receive_queue, skb);
+	skb->dev = dev;
+	/* Apply the forward feature mask so that we perform segmentation
+	 * according to users wishes.
+	 */
+	features = netif_skb_features(skb) & vlan->tap_features;
+	if (netif_needs_gso(skb, features)) {
+		struct sk_buff *segs = __skb_gso_segment(skb, features, false);
+
+		if (IS_ERR(segs))
+			goto drop;
+
+		if (!segs) {
+			skb_queue_tail(&q->sk.sk_receive_queue, skb);
+			goto wake_up;
+		}
+
+		kfree_skb(skb);
+		while (segs) {
+			struct sk_buff *nskb = segs->next;
+
+			segs->next = NULL;
+			skb_queue_tail(&q->sk.sk_receive_queue, segs);
+			segs = nskb;
+		}
+	} else
+		skb_queue_tail(&q->sk.sk_receive_queue, skb);
+
+wake_up:
 	wake_up_interruptible_poll(sk_sleep(&q->sk), POLLIN | POLLRDNORM | POLLRDBAND);
 	return NET_RX_SUCCESS;
 
-- 
1.8.1.4

Re: [PATCHv2 net-next 4/4] macvtap: Perform GSO on forwarding path.

[Sergei Shtylyov]

Hello.

On 06/26/2013 12:04 AM, Vlad Yasevich wrote:

> When macvtap forwards skb to its tap, it needs to check
> if GSO needs to be performed.  This is sometimes necessary
> when the HW device performed GRO, but the guest reading
> from the tap does not support it (ex: Windows 7).

> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
>   drivers/net/macvtap.c | 31 ++++++++++++++++++++++++++++++-
>   1 file changed, 30 insertions(+), 1 deletion(-)

> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 7eab019..e370b79 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -276,14 +276,43 @@ static void macvtap_del_queues(struct net_device *dev)
>    */
>   static int macvtap_forward(struct net_device *dev, struct sk_buff *skb)
>   {
> +	struct macvlan_dev *vlan = netdev_priv(dev);
>   	struct macvtap_queue *q = macvtap_get_queue(dev, skb);
> +	netdev_features_t features;
>   	if (!q)
>   		goto drop;
>
>   	if (skb_queue_len(&q->sk.sk_receive_queue) >= dev->tx_queue_len)
>   		goto drop;
>
> -	skb_queue_tail(&q->sk.sk_receive_queue, skb);
> +	skb->dev = dev;
> +	/* Apply the forward feature mask so that we perform segmentation
> +	 * according to users wishes.
> +	 */
> +	features = netif_skb_features(skb) & vlan->tap_features;
> +	if (netif_needs_gso(skb, features)) {
> +		struct sk_buff *segs = __skb_gso_segment(skb, features, false);
> +
> +		if (IS_ERR(segs))
> +			goto drop;
> +
> +		if (!segs) {
> +			skb_queue_tail(&q->sk.sk_receive_queue, skb);
> +			goto wake_up;
> +		}
> +
> +		kfree_skb(skb);
> +		while (segs) {
> +			struct sk_buff *nskb = segs->next;
> +
> +			segs->next = NULL;
> +			skb_queue_tail(&q->sk.sk_receive_queue, segs);
> +			segs = nskb;
> +		}
> +	} else
> +		skb_queue_tail(&q->sk.sk_receive_queue, skb);

    According to Documentation/CodingStyle chapter 3, you should have {} 
on both branches of *if* statement, if one branch has it.

WBR, Sergei

Re: [PATCHv2 net-next 4/4] macvtap: Perform GSO on forwarding path.

[David Miller]

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Wed, 26 Jun 2013 00:25:28 +0400

> On 06/26/2013 12:04 AM, Vlad Yasevich wrote:
> 
>> +	} else
>> +		skb_queue_tail(&q->sk.sk_receive_queue, skb);
> 
>    According to Documentation/CodingStyle chapter 3, you should have {}
>    on both branches of *if* statement, if one branch has it.

I took care of this when applying the series.

Re: [PATCHv2 net-next 0/4] macvtap: locking and offload control

[David Miller]

From: Vlad Yasevich <vyasevic@redhat.com>
Date: Tue, 25 Jun 2013 16:04:18 -0400

> This series of patches changes the macvtap locking to use rtnl instead
> of a private lock and adds the ability for user to specify TAP offload
> features.

Series applied, thanks Vlad.