[PATCH v4 0/3] Correctly perform offloads when VNET_HDR is disabled

[PATCH v4 0/3] Correctly perform offloads when VNET_HDR is disabled

[Vlad Yasevich]

Changes since v3:
 - The main change here is that instead of mucking around with checksum bits,
   we instead treat macvtap as macvlan+tap each with separate feature sets.
   tap_features identify the features of only the 'tap' part and we only use
   them when forwarding, essentially making the 'tap' part a pseudo-device.
   To properly handle GSO, we always set NETIF_F_SG | NETIF_F_GSO on the
   'tap' part.

 - The rest of the set is mostly the same in that we allow user to change
   tap offload features whether VNET_HDR is set or cleared.  
 - When VNET_HDR is disabled, we ignore user specified tap offloads.  This
   is now a separate patch so that if there are disagreements, it can
   be easily dropped.

Vlad Yasevich (3):
  macvtap: simplify usage of tap_features
  macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
  macvtap: Ignore tap features when VNET_HDR is off

 drivers/net/macvtap.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

-- 
1.8.1.4

[PATCH v4 1/3] macvtap: simplify usage of tap_features

[Vlad Yasevich]

In macvtap, tap_features specific the features of that the user
has specified via ioctl().  If we treat macvtap as a macvlan+tap
then we could all the tap a pseudo-device and give it other features
like SG and GSO.  Then we can stop using the features of lower
device (macvlan) when forwarding the traffic the tap.

This solves the issue of possible checksum offload mismatch between
tap feature and macvlan features.

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

diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index b51db2a..448f8a5 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -68,6 +68,8 @@ 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)
+#define TAP_FEATURES (NETIF_F_GSO | NETIF_F_SG)
+
 /*
  * RCU usage:
  * The macvtap_queue and the macvlan_dev are loosely coupled, the
@@ -278,7 +280,8 @@ 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;
+	netdev_features_t features = TAP_FEATURES;
+
 	if (!q)
 		goto drop;
 
@@ -289,7 +292,7 @@ static int macvtap_forward(struct net_device *dev, struct sk_buff *skb)
 	/* Apply the forward feature mask so that we perform segmentation
 	 * according to users wishes.
 	 */
-	features = netif_skb_features(skb) & vlan->tap_features;
+	features |= vlan->tap_features;
 	if (netif_needs_gso(skb, features)) {
 		struct sk_buff *segs = __skb_gso_segment(skb, features, false);
 
@@ -1064,8 +1067,7 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
 	/* 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->tap_features = feature_mask;
 	vlan->set_features = features;
 	netdev_update_features(vlan->dev);
 
-- 
1.8.1.4

Re: [PATCH v4 1/3] macvtap: simplify usage of tap_features

[David Miller]

From: Vlad Yasevich <vyasevic@redhat.com>
Date: Fri, 16 Aug 2013 15:25:00 -0400

> In macvtap, tap_features specific the features of that the user
> has specified via ioctl().  If we treat macvtap as a macvlan+tap
> then we could all the tap a pseudo-device and give it other features
> like SG and GSO.  Then we can stop using the features of lower
> device (macvlan) when forwarding the traffic the tap.
> 
> This solves the issue of possible checksum offload mismatch between
> tap feature and macvlan features.
> 
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>

Applied.

[PATCH v4 2/3] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.

[Vlad Yasevich]

When the user turns off IFF_VNET_HDR flag, attempts to change
offload features via TUNSETOFFLOAD do not work.  This could cause
GSO packets to be delivered to the user when the user is
not prepared to handle them.

To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
disabled.

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

diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 448f8a5..4acfbce 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -1163,10 +1163,6 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
 			    TUN_F_TSO_ECN | TUN_F_UFO))
 			return -EINVAL;
 
-		/* TODO: only accept frames with the features that
-			 got enabled for forwarded frames */
-		if (!(q->flags & IFF_VNET_HDR))
-			return  -EINVAL;
 		rtnl_lock();
 		ret = set_offload(q, arg);
 		rtnl_unlock();
-- 
1.8.1.4

Re: [PATCH v4 2/3] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.

[David Miller]

From: Vlad Yasevich <vyasevic@redhat.com>
Date: Fri, 16 Aug 2013 15:25:01 -0400

> When the user turns off IFF_VNET_HDR flag, attempts to change
> offload features via TUNSETOFFLOAD do not work.  This could cause
> GSO packets to be delivered to the user when the user is
> not prepared to handle them.
> 
> To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
> disabled.
> 
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>

Applied.

[PATCH v4 3/3] macvtap: Ignore tap features when VNET_HDR is off

[Vlad Yasevich]

When the user turns off VNET_HDR support on the
macvtap device, there is no way to provide any
offload information to the user.  So, it's safer
to ignore offload setting then depend on the user
setting them correctly.

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

diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 4acfbce..ea53abb 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -290,9 +290,11 @@ static int macvtap_forward(struct net_device *dev, struct sk_buff *skb)
 
 	skb->dev = dev;
 	/* Apply the forward feature mask so that we perform segmentation
-	 * according to users wishes.
+	 * according to users wishes.  This only works if VNET_HDR is
+	 * enabled.
 	 */
-	features |= vlan->tap_features;
+	if (q->flags & IFF_VNET_HDR)
+		features |= vlan->tap_features;
 	if (netif_needs_gso(skb, features)) {
 		struct sk_buff *segs = __skb_gso_segment(skb, features, false);
 
-- 
1.8.1.4

Re: [PATCH v4 3/3] macvtap: Ignore tap features when VNET_HDR is off

[David Miller]

From: Vlad Yasevich <vyasevic@redhat.com>
Date: Fri, 16 Aug 2013 15:25:02 -0400

> When the user turns off VNET_HDR support on the
> macvtap device, there is no way to provide any
> offload information to the user.  So, it's safer
> to ignore offload setting then depend on the user
> setting them correctly.
> 
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>

Applied.