[PATCH 2/2] openvswitch: Enable tunnel GSO for OVS bridge.

Signed-off-by: Pravin B Shelar <redacted>
---
 include/linux/netdev_features.h      | 4 ++++
 net/openvswitch/vport-internal_dev.c | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index e5a5894..12118a5 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h

@@ -152,6 +152,10 @@ enum {
 #define NETIF_F_ALL_FCOE	(NETIF_F_FCOE_CRC | NETIF_F_FCOE_MTU | \
 				 NETIF_F_FSO)
 
+#define NETIF_F_ALL_ENCAP_GSO   (NETIF_F_GSO_GRE | NETIF_F_GSO_GRE_CSUM | \
+				 NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT |     \
+				 NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_MPLS)
+
 /*
  * If one device supports one of these features, then enable them
  * for all in netdev_increment_features.

diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
index 789af92..b971610 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c

@@ -134,7 +134,8 @@ static void do_setup(struct net_device *netdev)
 	netdev->tx_queue_len = 0;
 
 	netdev->features = NETIF_F_LLTX | NETIF_F_SG | NETIF_F_FRAGLIST |
-			   NETIF_F_HIGHDMA | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE;
+			   NETIF_F_HIGHDMA | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE |
+			   NETIF_F_ALL_ENCAP_GSO;
 
 	netdev->vlan_features = netdev->features;
 	netdev->features |= NETIF_F_HW_VLAN_CTAG_TX;

-- 
1.9.1

On Sun, May 25, 2014 at 2:39 PM, Pravin B Shelar [off-list ref] wrote:

Signed-off-by: Pravin B Shelar <redacted>

Hi Pravin,

Can you please provide a non-empty change log for this patch... I
understand the patch
addresses some openvswitch use case where offloads are currently
non-coming into play and
you want to support that, right?

AFAIK OVS supports only VXLAN and GRE tunneling -- wouldn't it be
enough to add here only
NETIF_F_GSO_GRE and NETIF_F_GSO_UDP_TUNNEL?

On Sun, May 25, 2014 at 2:39 PM, Pravin B Shelar [off-list ref] wrote:

OK, I guess this commit is similar in spirit to 82d8189 "veth: extend
features to support tunneling"

So in that respect, don't you need to set also dev->hw_enc_features
for the NETIF_F_GSO_YYY to come into play?

On Thu, Jun 12, 2014 at 2:27 AM, Or Gerlitz [off-list ref] wrote:

On Sun, May 25, 2014 at 2:39 PM, Pravin B Shelar [off-list ref] wrote:

quoted

Signed-off-by: Pravin B Shelar <redacted>
---
 include/linux/netdev_features.h      | 4 ++++
 net/openvswitch/vport-internal_dev.c | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index e5a5894..12118a5 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h

@@ -152,6 +152,10 @@ enum {
 #define NETIF_F_ALL_FCOE       (NETIF_F_FCOE_CRC | NETIF_F_FCOE_MTU | \
                                 NETIF_F_FSO)

+#define NETIF_F_ALL_ENCAP_GSO   (NETIF_F_GSO_GRE | NETIF_F_GSO_GRE_CSUM | \
+                                NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT |     \
+                                NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_MPLS)
+
 /*
  * If one device supports one of these features, then enable them
  * for all in netdev_increment_features.

diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
index 789af92..b971610 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c

@@ -134,7 +134,8 @@ static void do_setup(struct net_device *netdev)
        netdev->tx_queue_len = 0;

        netdev->features = NETIF_F_LLTX | NETIF_F_SG | NETIF_F_FRAGLIST |
-                          NETIF_F_HIGHDMA | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE;
+                          NETIF_F_HIGHDMA | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE |
+                          NETIF_F_ALL_ENCAP_GSO;

        netdev->vlan_features = netdev->features;
        netdev->features |= NETIF_F_HW_VLAN_CTAG_TX;

OK, I guess this commit is similar in spirit to 82d8189 "veth: extend
features to support tunneling"

So in that respect, don't you need to set also dev->hw_enc_features
for the NETIF_F_GSO_YYY to come into play?

Right, I will send updated patch.

On Sun, May 25, 2014 at 2:39 PM, Pravin B Shelar [off-list ref] wrote:

Signed-off-by: Pravin B Shelar <redacted>

Hi Pravin, ping... this addresses a real life use case where offloads
didn't come into play, right? can you respin the patch with non-empty
change-log so people can review/ack it? --> and we'll have upstream
support for that case.

Or.

On Mon, Jun 30, 2014 at 2:53 PM, Or Gerlitz [off-list ref] wrote:

On Sun, May 25, 2014 at 2:39 PM, Pravin B Shelar [off-list ref] wrote:

quoted

Signed-off-by: Pravin B Shelar <redacted>

Hi Pravin, ping... this addresses a real life use case where offloads
didn't come into play, right? can you respin the patch with non-empty
change-log so people can review/ack it? --> and we'll have upstream
support for that case.

This patch is blocked by double encap issue. You can read mail thread
on first patch http://marc.info/?l=linux-netdev&m=140236173911474&w=2

Tom is going to fix GSO for packets with multiple encapsulations.
After that it would be safe to enable tunnel offloading features for
OVS.

Thanks,
Pravin.

Or.

quoted

---
 include/linux/netdev_features.h      | 4 ++++
 net/openvswitch/vport-internal_dev.c | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index e5a5894..12118a5 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h

@@ -152,6 +152,10 @@ enum {
 #define NETIF_F_ALL_FCOE       (NETIF_F_FCOE_CRC | NETIF_F_FCOE_MTU | \
                                 NETIF_F_FSO)

+#define NETIF_F_ALL_ENCAP_GSO   (NETIF_F_GSO_GRE | NETIF_F_GSO_GRE_CSUM | \
+                                NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT |     \
+                                NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_MPLS)
+
 /*
  * If one device supports one of these features, then enable them
  * for all in netdev_increment_features.

diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
index 789af92..b971610 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c

@@ -134,7 +134,8 @@ static void do_setup(struct net_device *netdev)
        netdev->tx_queue_len = 0;

        netdev->features = NETIF_F_LLTX | NETIF_F_SG | NETIF_F_FRAGLIST |
-                          NETIF_F_HIGHDMA | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE;
+                          NETIF_F_HIGHDMA | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE |
+                          NETIF_F_ALL_ENCAP_GSO;

        netdev->vlan_features = netdev->features;
        netdev->features |= NETIF_F_HW_VLAN_CTAG_TX;
--

1.9.1

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

On Mon, Jun 30, 2014 at 4:37 PM, Pravin Shelar [off-list ref] wrote:

On Mon, Jun 30, 2014 at 2:53 PM, Or Gerlitz [off-list ref] wrote:

quoted

On Sun, May 25, 2014 at 2:39 PM, Pravin B Shelar [off-list ref] wrote:

quoted

Signed-off-by: Pravin B Shelar <redacted>

Hi Pravin, ping... this addresses a real life use case where offloads
didn't come into play, right? can you respin the patch with non-empty
change-log so people can review/ack it? --> and we'll have upstream
support for that case.

This patch is blocked by double encap issue. You can read mail thread
on first patch http://marc.info/?l=linux-netdev&m=140236173911474&w=2

Tom is going to fix GSO for packets with multiple encapsulations.
After that it would be safe to enable tunnel offloading features for
OVS.

I have some working patches for that but would like to get checksum
changes finished first. The first patch is reasonable to keep things
moving and should be okay to undo.

Thanks,
Pravin.

quoted

Or.

quoted

---
 include/linux/netdev_features.h      | 4 ++++
 net/openvswitch/vport-internal_dev.c | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index e5a5894..12118a5 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h

@@ -152,6 +152,10 @@ enum {
 #define NETIF_F_ALL_FCOE       (NETIF_F_FCOE_CRC | NETIF_F_FCOE_MTU | \
                                 NETIF_F_FSO)

+#define NETIF_F_ALL_ENCAP_GSO   (NETIF_F_GSO_GRE | NETIF_F_GSO_GRE_CSUM | \
+                                NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT |     \
+                                NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_MPLS)
+
 /*
  * If one device supports one of these features, then enable them
  * for all in netdev_increment_features.

diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
index 789af92..b971610 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c

@@ -134,7 +134,8 @@ static void do_setup(struct net_device *netdev)
        netdev->tx_queue_len = 0;

        netdev->features = NETIF_F_LLTX | NETIF_F_SG | NETIF_F_FRAGLIST |
-                          NETIF_F_HIGHDMA | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE;
+                          NETIF_F_HIGHDMA | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE |
+                          NETIF_F_ALL_ENCAP_GSO;

        netdev->vlan_features = netdev->features;
        netdev->features |= NETIF_F_HW_VLAN_CTAG_TX;
--

1.9.1

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