All of lore.kernel.org
 help / color / mirror / Atom feed
* Add support for Geneve udp port offload
@ 2015-12-08 18:12 Anjali Singhai Jain
  2015-12-08 18:12 ` [PATCH v3 1/4] geneve: Add geneve udp port offload for ethernet devices Anjali Singhai Jain
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Anjali Singhai Jain @ 2015-12-08 18:12 UTC (permalink / raw)
  To: netdev; +Cc: jeffrey.t.kirsher

This patch series adds new ndo ops for Geneve add/del port, so as
to help offload Geneve tunnel functionalities such as RX checksum, 
RSS, filters etc.

i40e driver has been tested with the changes to make sure the offloads
happen.

We do understand that this is not the ideal solution and most likely
will be redone with a more generic offload framework.
But this certainly will enable us to start seeing benefits of the
accelerations for Geneve tunnels.

As a side note, we did find an existing issue in i40e driver where a
service task can modify tunnel data structures with no locks held to
help linearize access. A separate patch will be taking care of that issue.

A question out to the community is regarding the driver Kconfig parameters
for VxLAN and Geneve, it would be ideal to drop those if there is a way
to help resolve vxlan/geneve_get_rx_port symbols while the tunnel modules
are not loaded.

Performance numbers
With the offloads enable on X722 devices with remote checksum enabled
and no other tuning in terms of cpu governer etc.

With offload
Throughput: 5527Mbits/sec with a single thread
%cpu: ~43% per core with 4 threads

Without offload
Throughput: 2364Mbits/sec with a single thread
%cpu: ~99% per core with 4 threads

These numbers will get better for X722 as it is being worked. But
this does bring out the delta in terms of when the stack is notified
with csum_level 1 and CHECKSUM_UNNECESSARY vs not without the RX offload.

Anjali Singhai Jain (4):
  [RFC PATCH v2 1/4] geneve: Add geneve udp port offload for ethernet
  [RFC PATCH v2 2/4] i40e: geneve tunnel offload support
  [RFC PATCH v2 3/4] i40e: Kernel dependency update for i40e to support
  [RFC PATCH v2 4/4] geneve: Add geneve_get_rx_port support

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3 1/4] geneve: Add geneve udp port offload for ethernet devices
  2015-12-08 18:12 Add support for Geneve udp port offload Anjali Singhai Jain
@ 2015-12-08 18:12 ` Anjali Singhai Jain
  2015-12-12  0:58   ` David Miller
  2015-12-12  3:11   ` Tom Herbert
  2015-12-08 18:12 ` [PATCH v3 2/4] i40e: geneve tunnel offload support Anjali Singhai Jain
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 17+ messages in thread
From: Anjali Singhai Jain @ 2015-12-08 18:12 UTC (permalink / raw)
  To: netdev; +Cc: jeffrey.t.kirsher, Anjali Singhai Jain, kiran.patil

Add ndo_ops to add/del UDP ports to a device that supports geneve
offload.

v3: Add some more comments about the use of the new ndo ops.

Signed-off-by: Anjali Singhai Jain <anjali.singhai at intel.com>
Signed-off-by: Kiran Patil <kiran.patil at intel.com>
---
 drivers/net/geneve.c      | 23 +++++++++++++++++++++++
 include/linux/netdevice.h | 21 ++++++++++++++++++++-
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index de5c30c..b43fd56 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -371,8 +371,11 @@ static struct socket *geneve_create_sock(struct net *net, bool ipv6,
 
 static void geneve_notify_add_rx_port(struct geneve_sock *gs)
 {
+	struct net_device *dev;
 	struct sock *sk = gs->sock->sk;
+	struct net *net = sock_net(sk);
 	sa_family_t sa_family = sk->sk_family;
+	__be16 port = inet_sk(sk)->inet_sport;
 	int err;
 
 	if (sa_family == AF_INET) {
@@ -381,6 +384,14 @@ static void geneve_notify_add_rx_port(struct geneve_sock *gs)
 			pr_warn("geneve: udp_add_offload failed with status %d\n",
 				err);
 	}
+
+	rcu_read_lock();
+	for_each_netdev_rcu(net, dev) {
+		if (dev->netdev_ops->ndo_add_geneve_port)
+			dev->netdev_ops->ndo_add_geneve_port(dev, sa_family,
+							     port);
+	}
+	rcu_read_unlock();
 }
 
 static int geneve_hlen(struct genevehdr *gh)
@@ -521,8 +532,20 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
 
 static void geneve_notify_del_rx_port(struct geneve_sock *gs)
 {
+	struct net_device *dev;
 	struct sock *sk = gs->sock->sk;
+	struct net *net = sock_net(sk);
 	sa_family_t sa_family = sk->sk_family;
+	__be16 port = inet_sk(sk)->inet_sport;
+
+	rcu_read_lock();
+	for_each_netdev_rcu(net, dev) {
+		if (dev->netdev_ops->ndo_del_geneve_port)
+			dev->netdev_ops->ndo_del_geneve_port(dev, sa_family,
+							     port);
+	}
+
+	rcu_read_unlock();
 
 	if (sa_family == AF_INET)
 		udp_del_offload(&gs->udp_offloads);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 1bb21ff..ff91007 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1013,6 +1013,20 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
  *	a new port starts listening. The operation is protected by the
  *	vxlan_net->sock_lock.
  *
+ * void (*ndo_add_geneve_port)(struct net_device *dev,
+ *			      sa_family_t sa_family, __be16 port);
+ *	Called by geneve to notiy a driver about the UDP port and socket
+ *	address family that geneve is listnening to. It is called only when
+ *	a new port starts listening. The operation is protected by the
+ *	geneve_net->sock_lock. This should be strictly used by the driver
+ *	when setting up the device for RX side offloads only.
+ *
+ * void (*ndo_del_geneve_port)(struct net_device *dev,
+ *			      sa_family_t sa_family, __be16 port);
+ *	Called by geneve to notify the driver about a UDP port and socket
+ *	address family that geneve is not listening to anymore. The operation
+ *	is protected by the geneve_net->sock_lock.
+ *
  * void (*ndo_del_vxlan_port)(struct  net_device *dev,
  *			      sa_family_t sa_family, __be16 port);
  *	Called by vxlan to notify the driver about a UDP port and socket
@@ -1217,7 +1231,12 @@ struct net_device_ops {
 	void			(*ndo_del_vxlan_port)(struct  net_device *dev,
 						      sa_family_t sa_family,
 						      __be16 port);
-
+	void			(*ndo_add_geneve_port)(struct  net_device *dev,
+						       sa_family_t sa_family,
+						       __be16 port);
+	void			(*ndo_del_geneve_port)(struct  net_device *dev,
+						       sa_family_t sa_family,
+						       __be16 port);
 	void*			(*ndo_dfwd_add_station)(struct net_device *pdev,
 							struct net_device *dev);
 	void			(*ndo_dfwd_del_station)(struct net_device *pdev,
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 2/4] i40e: geneve tunnel offload support
  2015-12-08 18:12 Add support for Geneve udp port offload Anjali Singhai Jain
  2015-12-08 18:12 ` [PATCH v3 1/4] geneve: Add geneve udp port offload for ethernet devices Anjali Singhai Jain
@ 2015-12-08 18:12 ` Anjali Singhai Jain
  2015-12-08 18:20   ` Alexei Starovoitov
                     ` (2 more replies)
  2015-12-08 18:12 ` [PATCH v3 3/4] i40e: Kernel dependency update for i40e to support geneve offload Anjali Singhai Jain
  2015-12-08 18:12 ` [PATCH v3 4/4] geneve: Add geneve_get_rx_port support Anjali Singhai Jain
  3 siblings, 3 replies; 17+ messages in thread
From: Anjali Singhai Jain @ 2015-12-08 18:12 UTC (permalink / raw)
  To: netdev; +Cc: jeffrey.t.kirsher, Anjali Singhai Jain, Kiran Patil

This patch adds driver hooks to implement ndo_ops to add/del
udp port in the HW to identify GENEVE tunnels.

Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
Signed-off-by: Kiran Patil <kiran.patil@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h      |  16 +--
 drivers/net/ethernet/intel/i40e/i40e_main.c | 167 ++++++++++++++++++++++------
 drivers/net/ethernet/intel/i40e/i40e_txrx.c |   8 +-
 drivers/net/ethernet/intel/i40e/i40e_txrx.h |   2 +-
 4 files changed, 150 insertions(+), 43 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 9b1dbbc..68f2204 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -245,6 +245,11 @@ struct i40e_tc_configuration {
 	struct i40e_tc_info tc_info[I40E_MAX_TRAFFIC_CLASS];
 };
 
+struct i40e_udp_port_config {
+	__be16 index;
+	u8 type;
+};
+
 /* struct that defines the Ethernet device */
 struct i40e_pf {
 	struct pci_dev *pdev;
@@ -281,11 +286,9 @@ struct i40e_pf {
 	u32 fd_atr_cnt;
 	u32 fd_tcp_rule;
 
-#ifdef CONFIG_I40E_VXLAN
-	__be16  vxlan_ports[I40E_MAX_PF_UDP_OFFLOAD_PORTS];
-	u16 pending_vxlan_bitmap;
+	struct i40e_udp_port_config udp_ports[I40E_MAX_PF_UDP_OFFLOAD_PORTS];
+	u16 pending_udp_bitmap;
 
-#endif
 	enum i40e_interrupt_policy int_policy;
 	u16 rx_itr_default;
 	u16 tx_itr_default;
@@ -322,9 +325,7 @@ struct i40e_pf {
 #define I40E_FLAG_FD_ATR_ENABLED		BIT_ULL(22)
 #define I40E_FLAG_PTP				BIT_ULL(25)
 #define I40E_FLAG_MFP_ENABLED			BIT_ULL(26)
-#ifdef CONFIG_I40E_VXLAN
-#define I40E_FLAG_VXLAN_FILTER_SYNC		BIT_ULL(27)
-#endif
+#define I40E_FLAG_UDP_FILTER_SYNC		BIT_ULL(27)
 #define I40E_FLAG_PORT_ID_VALID			BIT_ULL(28)
 #define I40E_FLAG_DCB_CAPABLE			BIT_ULL(29)
 #define I40E_FLAG_RSS_AQ_CAPABLE		BIT_ULL(31)
@@ -336,6 +337,7 @@ struct i40e_pf {
 #define I40E_FLAG_MULTIPLE_TCP_UDP_RSS_PCTYPE	BIT_ULL(38)
 #define I40E_FLAG_LINK_POLLING_ENABLED		BIT_ULL(39)
 #define I40E_FLAG_VEB_MODE_ENABLED		BIT_ULL(40)
+#define I40E_FLAG_GENEVE_OFFLOAD_CAPABLE	BIT_ULL(41)
 #define I40E_FLAG_NO_PCI_LINK_CHECK		BIT_ULL(42)
 #define I40E_FLAG_PF_MAC			BIT_ULL(50)
 
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index e2ca6e0..08dc7d6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -36,9 +36,12 @@
 /* Local includes */
 #include "i40e.h"
 #include "i40e_diag.h"
-#ifdef CONFIG_I40E_VXLAN
+#if IS_ENABLED(CONFIG_VXLAN)
 #include <net/vxlan.h>
 #endif
+#if IS_ENABLED(CONFIG_GENEVE)
+#include <net/geneve.h>
+#endif
 
 const char i40e_driver_name[] = "i40e";
 static const char i40e_driver_string[] =
@@ -7044,30 +7047,30 @@ static void i40e_handle_mdd_event(struct i40e_pf *pf)
 	i40e_flush(hw);
 }
 
-#ifdef CONFIG_I40E_VXLAN
 /**
- * i40e_sync_vxlan_filters_subtask - Sync the VSI filter list with HW
+ * i40e_sync_udp_filters_subtask - Sync the VSI filter list with HW
  * @pf: board private structure
  **/
-static void i40e_sync_vxlan_filters_subtask(struct i40e_pf *pf)
+static void i40e_sync_udp_filters_subtask(struct i40e_pf *pf)
 {
+#if IS_ENABLED(CONFIG_VXLAN) || IS_ENABLED(CONFIG_GENEVE)
 	struct i40e_hw *hw = &pf->hw;
 	i40e_status ret;
 	__be16 port;
 	int i;
 
-	if (!(pf->flags & I40E_FLAG_VXLAN_FILTER_SYNC))
+	if (!(pf->flags & I40E_FLAG_UDP_FILTER_SYNC))
 		return;
 
-	pf->flags &= ~I40E_FLAG_VXLAN_FILTER_SYNC;
+	pf->flags &= ~I40E_FLAG_UDP_FILTER_SYNC;
 
 	for (i = 0; i < I40E_MAX_PF_UDP_OFFLOAD_PORTS; i++) {
-		if (pf->pending_vxlan_bitmap & BIT_ULL(i)) {
-			pf->pending_vxlan_bitmap &= ~BIT_ULL(i);
-			port = pf->vxlan_ports[i];
+		if (pf->pending_udp_bitmap & BIT_ULL(i)) {
+			pf->pending_udp_bitmap &= ~BIT_ULL(i);
+			port = pf->udp_ports[i].index;
 			if (port)
 				ret = i40e_aq_add_udp_tunnel(hw, ntohs(port),
-						     I40E_AQC_TUNNEL_TYPE_VXLAN,
+						     pf->udp_ports[i].type,
 						     NULL, NULL);
 			else
 				ret = i40e_aq_del_udp_tunnel(hw, i, NULL);
@@ -7080,13 +7083,13 @@ static void i40e_sync_vxlan_filters_subtask(struct i40e_pf *pf)
 					 i40e_stat_str(&pf->hw, ret),
 					 i40e_aq_str(&pf->hw,
 						    pf->hw.aq.asq_last_status));
-				pf->vxlan_ports[i] = 0;
+				pf->udp_ports[i].index = 0;
 			}
 		}
 	}
+#endif
 }
 
-#endif
 /**
  * i40e_service_task - Run the driver's async subtasks
  * @work: pointer to work_struct containing our data
@@ -7111,8 +7114,8 @@ static void i40e_service_task(struct work_struct *work)
 	i40e_watchdog_subtask(pf);
 	i40e_fdir_reinit_subtask(pf);
 	i40e_sync_filters_subtask(pf);
-#ifdef CONFIG_I40E_VXLAN
-	i40e_sync_vxlan_filters_subtask(pf);
+#if IS_ENABLED(CONFIG_VXLAN) || IS_ENABLED(CONFIG_GENEVE)
+	i40e_sync_udp_filters_subtask(pf);
 #endif
 	i40e_clean_adminq_subtask(pf);
 
@@ -8388,7 +8391,8 @@ static int i40e_sw_init(struct i40e_pf *pf)
 			     I40E_FLAG_HW_ATR_EVICT_CAPABLE |
 			     I40E_FLAG_OUTER_UDP_CSUM_CAPABLE |
 			     I40E_FLAG_WB_ON_ITR_CAPABLE |
-			     I40E_FLAG_MULTIPLE_TCP_UDP_RSS_PCTYPE;
+			     I40E_FLAG_MULTIPLE_TCP_UDP_RSS_PCTYPE |
+			     I40E_FLAG_GENEVE_OFFLOAD_CAPABLE;
 	}
 	pf->eeprom_version = 0xDEAD;
 	pf->lan_veb = I40E_NO_VEB;
@@ -8487,26 +8491,27 @@ static int i40e_set_features(struct net_device *netdev,
 	return 0;
 }
 
-#ifdef CONFIG_I40E_VXLAN
+#if IS_ENABLED(CONFIG_VXLAN) || IS_ENABLED(CONFIG_GENEVE)
 /**
- * i40e_get_vxlan_port_idx - Lookup a possibly offloaded for Rx UDP port
+ * i40e_get_udp_port_idx - Lookup a possibly offloaded for Rx UDP port
  * @pf: board private structure
  * @port: The UDP port to look up
  *
  * Returns the index number or I40E_MAX_PF_UDP_OFFLOAD_PORTS if port not found
  **/
-static u8 i40e_get_vxlan_port_idx(struct i40e_pf *pf, __be16 port)
+static u8 i40e_get_udp_port_idx(struct i40e_pf *pf, __be16 port)
 {
 	u8 i;
 
 	for (i = 0; i < I40E_MAX_PF_UDP_OFFLOAD_PORTS; i++) {
-		if (pf->vxlan_ports[i] == port)
+		if (pf->udp_ports[i].index == port)
 			return i;
 	}
 
 	return i;
 }
 
+#endif
 /**
  * i40e_add_vxlan_port - Get notifications about VXLAN ports that come up
  * @netdev: This physical port's netdev
@@ -8516,6 +8521,7 @@ static u8 i40e_get_vxlan_port_idx(struct i40e_pf *pf, __be16 port)
 static void i40e_add_vxlan_port(struct net_device *netdev,
 				sa_family_t sa_family, __be16 port)
 {
+#if IS_ENABLED(CONFIG_VXLAN)
 	struct i40e_netdev_priv *np = netdev_priv(netdev);
 	struct i40e_vsi *vsi = np->vsi;
 	struct i40e_pf *pf = vsi->back;
@@ -8525,7 +8531,7 @@ static void i40e_add_vxlan_port(struct net_device *netdev,
 	if (sa_family == AF_INET6)
 		return;
 
-	idx = i40e_get_vxlan_port_idx(pf, port);
+	idx = i40e_get_udp_port_idx(pf, port);
 
 	/* Check if port already exists */
 	if (idx < I40E_MAX_PF_UDP_OFFLOAD_PORTS) {
@@ -8535,7 +8541,7 @@ static void i40e_add_vxlan_port(struct net_device *netdev,
 	}
 
 	/* Now check if there is space to add the new port */
-	next_idx = i40e_get_vxlan_port_idx(pf, 0);
+	next_idx = i40e_get_udp_port_idx(pf, 0);
 
 	if (next_idx == I40E_MAX_PF_UDP_OFFLOAD_PORTS) {
 		netdev_info(netdev, "maximum number of vxlan UDP ports reached, not adding port %d\n",
@@ -8544,9 +8550,11 @@ static void i40e_add_vxlan_port(struct net_device *netdev,
 	}
 
 	/* New port: add it and mark its index in the bitmap */
-	pf->vxlan_ports[next_idx] = port;
-	pf->pending_vxlan_bitmap |= BIT_ULL(next_idx);
-	pf->flags |= I40E_FLAG_VXLAN_FILTER_SYNC;
+	pf->udp_ports[next_idx].index = port;
+	pf->udp_ports[next_idx].type = I40E_AQC_TUNNEL_TYPE_VXLAN;
+	pf->pending_udp_bitmap |= BIT_ULL(next_idx);
+	pf->flags |= I40E_FLAG_UDP_FILTER_SYNC;
+#endif
 }
 
 /**
@@ -8558,6 +8566,7 @@ static void i40e_add_vxlan_port(struct net_device *netdev,
 static void i40e_del_vxlan_port(struct net_device *netdev,
 				sa_family_t sa_family, __be16 port)
 {
+#if IS_ENABLED(CONFIG_VXLAN)
 	struct i40e_netdev_priv *np = netdev_priv(netdev);
 	struct i40e_vsi *vsi = np->vsi;
 	struct i40e_pf *pf = vsi->back;
@@ -8566,23 +8575,108 @@ static void i40e_del_vxlan_port(struct net_device *netdev,
 	if (sa_family == AF_INET6)
 		return;
 
-	idx = i40e_get_vxlan_port_idx(pf, port);
+	idx = i40e_get_udp_port_idx(pf, port);
 
 	/* Check if port already exists */
 	if (idx < I40E_MAX_PF_UDP_OFFLOAD_PORTS) {
 		/* if port exists, set it to 0 (mark for deletion)
 		 * and make it pending
 		 */
-		pf->vxlan_ports[idx] = 0;
-		pf->pending_vxlan_bitmap |= BIT_ULL(idx);
-		pf->flags |= I40E_FLAG_VXLAN_FILTER_SYNC;
+		pf->udp_ports[idx].index = 0;
+		pf->pending_udp_bitmap |= BIT_ULL(idx);
+		pf->flags |= I40E_FLAG_UDP_FILTER_SYNC;
 	} else {
 		netdev_warn(netdev, "vxlan port %d was not found, not deleting\n",
 			    ntohs(port));
 	}
+#endif
+}
+
+/**
+ * i40e_add_geneve_port - Get notifications about GENEVE ports that come up
+ * @netdev: This physical port's netdev
+ * @sa_family: Socket Family that GENEVE is notifying us about
+ * @port: New UDP port number that GENEVE started listening to
+ **/
+static void i40e_add_geneve_port(struct net_device *netdev,
+				 sa_family_t sa_family, __be16 port)
+{
+#if IS_ENABLED(CONFIG_GENEVE)
+	struct i40e_netdev_priv *np = netdev_priv(netdev);
+	struct i40e_vsi *vsi = np->vsi;
+	struct i40e_pf *pf = vsi->back;
+	u8 next_idx;
+	u8 idx;
+
+	if (sa_family == AF_INET6)
+		return;
+
+	idx = i40e_get_udp_port_idx(pf, port);
+
+	/* Check if port already exists */
+	if (idx < I40E_MAX_PF_UDP_OFFLOAD_PORTS) {
+		netdev_info(netdev, "udp port %d already offloaded\n",
+			    ntohs(port));
+		return;
+	}
+
+	/* Now check if there is space to add the new port */
+	next_idx = i40e_get_udp_port_idx(pf, 0);
+
+	if (next_idx == I40E_MAX_PF_UDP_OFFLOAD_PORTS) {
+		netdev_info(netdev, "maximum number of UDP ports reached, not adding port %d\n",
+			    ntohs(port));
+		return;
+	}
+
+	/* New port: add it and mark its index in the bitmap */
+	pf->udp_ports[next_idx].index = port;
+	pf->udp_ports[next_idx].type = I40E_AQC_TUNNEL_TYPE_NGE;
+	pf->pending_udp_bitmap |= BIT_ULL(next_idx);
+	pf->flags |= I40E_FLAG_UDP_FILTER_SYNC;
+
+	dev_info(&pf->pdev->dev, "adding geneve port %d\n", ntohs(port));
+#endif
 }
 
+/**
+ * i40e_del_geneve_port - Get notifications about GENEVE ports that go away
+ * @netdev: This physical port's netdev
+ * @sa_family: Socket Family that GENEVE is notifying us about
+ * @port: UDP port number that GENEVE stopped listening to
+ **/
+static void i40e_del_geneve_port(struct net_device *netdev,
+				 sa_family_t sa_family, __be16 port)
+{
+#if IS_ENABLED(CONFIG_GENEVE)
+	struct i40e_netdev_priv *np = netdev_priv(netdev);
+	struct i40e_vsi *vsi = np->vsi;
+	struct i40e_pf *pf = vsi->back;
+	u8 idx;
+
+	if (sa_family == AF_INET6)
+		return;
+
+	idx = i40e_get_udp_port_idx(pf, port);
+
+	/* Check if port already exists */
+	if (idx < I40E_MAX_PF_UDP_OFFLOAD_PORTS) {
+		/* if port exists, set it to 0 (mark for deletion)
+		 * and make it pending
+		 */
+		pf->udp_ports[idx].index = 0;
+		pf->pending_udp_bitmap |= BIT_ULL(idx);
+		pf->flags |= I40E_FLAG_UDP_FILTER_SYNC;
+
+		dev_info(&pf->pdev->dev, "deleting geneve port %d\n",
+			 ntohs(port));
+	} else {
+		netdev_warn(netdev, "geneve port %d was not found, not deleting\n",
+			    ntohs(port));
+	}
 #endif
+}
+
 static int i40e_get_phys_port_id(struct net_device *netdev,
 				 struct netdev_phys_item_id *ppid)
 {
@@ -8760,7 +8854,10 @@ static int i40e_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 				       nlflags, 0, 0, filter_mask, NULL);
 }
 
-#define I40E_MAX_TUNNEL_HDR_LEN 80
+/* Hardware supports L4 tunnel length of 128B (=2^7) which includes
+ * inner mac plus all inner ethertypes.
+ */
+#define I40E_MAX_TUNNEL_HDR_LEN 128
 /**
  * i40e_features_check - Validate encapsulated packet conforms to limits
  * @skb: skb buff
@@ -8772,7 +8869,7 @@ static netdev_features_t i40e_features_check(struct sk_buff *skb,
 					     netdev_features_t features)
 {
 	if (skb->encapsulation &&
-	    (skb_inner_mac_header(skb) - skb_transport_header(skb) >
+	    ((skb_inner_network_header(skb) - skb_transport_header(skb)) >
 	     I40E_MAX_TUNNEL_HDR_LEN))
 		return features & ~(NETIF_F_ALL_CSUM | NETIF_F_GSO_MASK);
 
@@ -8807,10 +8904,14 @@ static const struct net_device_ops i40e_netdev_ops = {
 	.ndo_get_vf_config	= i40e_ndo_get_vf_config,
 	.ndo_set_vf_link_state	= i40e_ndo_set_vf_link_state,
 	.ndo_set_vf_spoofchk	= i40e_ndo_set_vf_spoofchk,
-#ifdef CONFIG_I40E_VXLAN
+#if IS_ENABLED(CONFIG_VXLAN)
 	.ndo_add_vxlan_port	= i40e_add_vxlan_port,
 	.ndo_del_vxlan_port	= i40e_del_vxlan_port,
 #endif
+#if IS_ENABLED(CONFIG_GENEVE)
+	.ndo_add_geneve_port	= i40e_add_geneve_port,
+	.ndo_del_geneve_port	= i40e_del_geneve_port,
+#endif
 	.ndo_get_phys_port_id	= i40e_get_phys_port_id,
 	.ndo_fdb_add		= i40e_ndo_fdb_add,
 	.ndo_features_check	= i40e_features_check,
@@ -8844,6 +8945,7 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
 	np->vsi = vsi;
 
 	netdev->hw_enc_features |= NETIF_F_IP_CSUM	 |
+				  NETIF_F_RXCSUM	 |
 				  NETIF_F_GSO_UDP_TUNNEL |
 				  NETIF_F_GSO_GRE	 |
 				  NETIF_F_TSO;
@@ -10406,6 +10508,9 @@ static void i40e_print_features(struct i40e_pf *pf)
 #if IS_ENABLED(CONFIG_VXLAN)
 	i += snprintf(&buf[i], REMAIN(i), " VxLAN");
 #endif
+#if IS_ENABLED(CONFIG_GENEVE)
+	i += snprintf(&buf[i], REMAIN(i), "Geneve ");
+#endif
 	if (pf->flags & I40E_FLAG_PTP)
 		i += snprintf(&buf[i], REMAIN(i), " PTP");
 #ifdef I40E_FCOE
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index e2ab7a6..5261372 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1380,7 +1380,7 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
 	if (rx_error & BIT(I40E_RX_DESC_ERROR_PPRS_SHIFT))
 		return;
 
-	/* If VXLAN traffic has an outer UDPv4 checksum we need to check
+	/* If VXLAN/GENEVE traffic has an outer UDPv4 checksum we need to check
 	 * it in the driver, hardware does not do it for us.
 	 * Since L3L4P bit was set we assume a valid IHL value (>=5)
 	 * so the total length of IPv4 header is IHL*4 bytes
@@ -2001,7 +2001,7 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
 	if (!(tx_flags & (I40E_TX_FLAGS_IPV4 | I40E_TX_FLAGS_IPV6)))
 		return;
 
-	if (!(tx_flags & I40E_TX_FLAGS_VXLAN_TUNNEL)) {
+	if (!(tx_flags & I40E_TX_FLAGS_UDP_TUNNEL)) {
 		/* snag network header to get L4 type and address */
 		hdr.network = skb_network_header(skb);
 
@@ -2086,7 +2086,7 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
 		     I40E_TXD_FLTR_QW1_FD_STATUS_SHIFT;
 
 	dtype_cmd |= I40E_TXD_FLTR_QW1_CNT_ENA_MASK;
-	if (!(tx_flags & I40E_TX_FLAGS_VXLAN_TUNNEL))
+	if (!(tx_flags & I40E_TX_FLAGS_UDP_TUNNEL))
 		dtype_cmd |=
 			((u32)I40E_FD_ATR_STAT_IDX(pf->hw.pf_id) <<
 			I40E_TXD_FLTR_QW1_CNTINDEX_SHIFT) &
@@ -2319,7 +2319,7 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 			oudph = udp_hdr(skb);
 			oiph = ip_hdr(skb);
 			l4_tunnel = I40E_TXD_CTX_UDP_TUNNELING;
-			*tx_flags |= I40E_TX_FLAGS_VXLAN_TUNNEL;
+			*tx_flags |= I40E_TX_FLAGS_UDP_TUNNEL;
 			break;
 		case IPPROTO_GRE:
 			l4_tunnel = I40E_TXD_CTX_GRE_TUNNELING;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index dccc1eb..3f081e2 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -163,7 +163,7 @@ enum i40e_dyn_idx_t {
 #define I40E_TX_FLAGS_FSO		BIT(7)
 #define I40E_TX_FLAGS_TSYN		BIT(8)
 #define I40E_TX_FLAGS_FD_SB		BIT(9)
-#define I40E_TX_FLAGS_VXLAN_TUNNEL	BIT(10)
+#define I40E_TX_FLAGS_UDP_TUNNEL	BIT(10)
 #define I40E_TX_FLAGS_VLAN_MASK		0xffff0000
 #define I40E_TX_FLAGS_VLAN_PRIO_MASK	0xe0000000
 #define I40E_TX_FLAGS_VLAN_PRIO_SHIFT	29
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 3/4] i40e: Kernel dependency update for i40e to support geneve offload
  2015-12-08 18:12 Add support for Geneve udp port offload Anjali Singhai Jain
  2015-12-08 18:12 ` [PATCH v3 1/4] geneve: Add geneve udp port offload for ethernet devices Anjali Singhai Jain
  2015-12-08 18:12 ` [PATCH v3 2/4] i40e: geneve tunnel offload support Anjali Singhai Jain
@ 2015-12-08 18:12 ` Anjali Singhai Jain
  2015-12-11  5:45   ` Jeff Kirsher
  2015-12-08 18:12 ` [PATCH v3 4/4] geneve: Add geneve_get_rx_port support Anjali Singhai Jain
  3 siblings, 1 reply; 17+ messages in thread
From: Anjali Singhai Jain @ 2015-12-08 18:12 UTC (permalink / raw)
  To: netdev; +Cc: jeffrey.t.kirsher, Anjali Singhai Jain, Kiran Patil

Update the Kconfig file with dependency for supporting GENEVE tunnel
offloads.

Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
Signed-off-by: Kiran Patil <kiran.patil@intel.com>
---
 drivers/net/ethernet/intel/Kconfig | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
index 4163b16..fa593dd 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -280,6 +280,16 @@ config I40E_VXLAN
 	  Say Y here if you want to use Virtual eXtensible Local Area Network
 	  (VXLAN) in the driver.
 
+config I40E_GENEVE
+	bool "Generic Network Virtualization Encapsulation (GENEVE) Support"
+	depends on I40E && GENEVE && !(I40E=y && GENEVE=m)
+	default n
+	---help---
+	  This allows one to create GENEVE virtual interfaces that provide
+	  Layer 2 Networks over Layer 3 Networks. GENEVE is often used
+	  to tunnel virtual network infrastructure in virtualized environments.
+	  Say Y here if you want to use GENEVE in the driver.
+
 config I40E_DCB
 	bool "Data Center Bridging (DCB) Support"
 	default n
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 4/4] geneve: Add geneve_get_rx_port support
  2015-12-08 18:12 Add support for Geneve udp port offload Anjali Singhai Jain
                   ` (2 preceding siblings ...)
  2015-12-08 18:12 ` [PATCH v3 3/4] i40e: Kernel dependency update for i40e to support geneve offload Anjali Singhai Jain
@ 2015-12-08 18:12 ` Anjali Singhai Jain
  2015-12-12  1:00   ` David Miller
  3 siblings, 1 reply; 17+ messages in thread
From: Anjali Singhai Jain @ 2015-12-08 18:12 UTC (permalink / raw)
  To: netdev; +Cc: jeffrey.t.kirsher, Anjali Singhai Jain

This patch adds an op that the drivers can call into to get existing
geneve ports.

Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c |  3 +++
 drivers/net/geneve.c                        | 24 ++++++++++++++++++++++++
 include/net/geneve.h                        |  8 ++++++++
 3 files changed, 35 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 08dc7d6..338d5eb 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -5347,6 +5347,9 @@ int i40e_open(struct net_device *netdev)
 #ifdef CONFIG_I40E_VXLAN
 	vxlan_get_rx_port(netdev);
 #endif
+#ifdef CONFIG_I40E_GENEVE
+	geneve_get_rx_port(netdev);
+#endif
 
 	return 0;
 }
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index b43fd56..bcf2be0 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1090,6 +1090,30 @@ static struct device_type geneve_type = {
 	.name = "geneve",
 };
 
+/* Calls the ndo_add_geneve_port of the caller in order to
+ * supply the listening GENEVE udp ports. Callers are expected
+ * to implement the ndo_add_geneve_port.
+ */
+void geneve_get_rx_port(struct net_device *dev)
+{
+	struct net *net = dev_net(dev);
+	struct geneve_net *gn = net_generic(net, geneve_net_id);
+	struct geneve_sock *gs;
+	sa_family_t sa_family;
+	struct sock *sk;
+	__be16 port;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(gs, &gn->sock_list, list) {
+		sk = gs->sock->sk;
+		sa_family = sk->sk_family;
+		port = inet_sk(sk)->inet_sport;
+		dev->netdev_ops->ndo_add_geneve_port(dev, sa_family, port);
+	}
+	rcu_read_unlock();
+}
+EXPORT_SYMBOL_GPL(geneve_get_rx_port);
+
 /* Initialize the device structure. */
 static void geneve_setup(struct net_device *dev)
 {
diff --git a/include/net/geneve.h b/include/net/geneve.h
index 3106ed6..e6c23dc 100644
--- a/include/net/geneve.h
+++ b/include/net/geneve.h
@@ -62,6 +62,14 @@ struct genevehdr {
 	struct geneve_opt options[];
 };
 
+#if IS_ENABLED(CONFIG_GENEVE)
+void geneve_get_rx_port(struct net_device *netdev);
+#else
+static inline void geneve_get_rx_port(struct net_device *netdev)
+{
+}
+#endif
+
 #ifdef CONFIG_INET
 struct net_device *geneve_dev_create_fb(struct net *net, const char *name,
 					u8 name_assign_type, u16 dst_port);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 2/4] i40e: geneve tunnel offload support
  2015-12-08 18:12 ` [PATCH v3 2/4] i40e: geneve tunnel offload support Anjali Singhai Jain
@ 2015-12-08 18:20   ` Alexei Starovoitov
  2015-12-08 18:36     ` Jesse Gross
  2015-12-08 18:22   ` John W. Linville
  2015-12-11  5:44   ` Jeff Kirsher
  2 siblings, 1 reply; 17+ messages in thread
From: Alexei Starovoitov @ 2015-12-08 18:20 UTC (permalink / raw)
  To: Anjali Singhai Jain; +Cc: netdev, jeffrey.t.kirsher, Kiran Patil, tom

On Tue, Dec 08, 2015 at 10:12:12AM -0800, Anjali Singhai Jain wrote:
> +/**
> + * i40e_add_geneve_port - Get notifications about GENEVE ports that come up
> + * @netdev: This physical port's netdev
> + * @sa_family: Socket Family that GENEVE is notifying us about
> + * @port: New UDP port number that GENEVE started listening to
> + **/
> +static void i40e_add_geneve_port(struct net_device *netdev,
> +				 sa_family_t sa_family, __be16 port)
> +{
> +#if IS_ENABLED(CONFIG_GENEVE)
...
> +	/* New port: add it and mark its index in the bitmap */
> +	pf->udp_ports[next_idx].index = port;
> +	pf->udp_ports[next_idx].type = I40E_AQC_TUNNEL_TYPE_NGE;

the function suppose to deal with geneve but tunnel type is NGE ?!

> -#define I40E_MAX_TUNNEL_HDR_LEN 80
> +/* Hardware supports L4 tunnel length of 128B (=2^7) which includes
> + * inner mac plus all inner ethertypes.
> + */
> +#define I40E_MAX_TUNNEL_HDR_LEN 128

so the driver lied about actual hw capabilities earlier
or it needs firmware update to work this way?

> -	if (!(tx_flags & I40E_TX_FLAGS_VXLAN_TUNNEL)) {
> +	if (!(tx_flags & I40E_TX_FLAGS_UDP_TUNNEL)) {
...
> @@ -163,7 +163,7 @@ enum i40e_dyn_idx_t {
>  #define I40E_TX_FLAGS_FSO		BIT(7)
>  #define I40E_TX_FLAGS_TSYN		BIT(8)
>  #define I40E_TX_FLAGS_FD_SB		BIT(9)
> -#define I40E_TX_FLAGS_VXLAN_TUNNEL	BIT(10)
> +#define I40E_TX_FLAGS_UDP_TUNNEL	BIT(10)

these changes implying that HW actually doesn't have special 'geneve or vxlan'
hard coded logic and it's generic enough to understand most of udp tunnels.
Then why you cannot generalize this whole things as generic udp tunnel offload
and do not add any protocol specific hooks and ndos.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 2/4] i40e: geneve tunnel offload support
  2015-12-08 18:12 ` [PATCH v3 2/4] i40e: geneve tunnel offload support Anjali Singhai Jain
  2015-12-08 18:20   ` Alexei Starovoitov
@ 2015-12-08 18:22   ` John W. Linville
  2015-12-11  5:44   ` Jeff Kirsher
  2 siblings, 0 replies; 17+ messages in thread
From: John W. Linville @ 2015-12-08 18:22 UTC (permalink / raw)
  To: Anjali Singhai Jain; +Cc: netdev, jeffrey.t.kirsher, Kiran Patil

On Tue, Dec 08, 2015 at 10:12:12AM -0800, Anjali Singhai Jain wrote:
> This patch adds driver hooks to implement ndo_ops to add/del
> udp port in the HW to identify GENEVE tunnels.
> 
> Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
> Signed-off-by: Kiran Patil <kiran.patil@intel.com>

> @@ -10406,6 +10508,9 @@ static void i40e_print_features(struct i40e_pf *pf)
>  #if IS_ENABLED(CONFIG_VXLAN)
>  	i += snprintf(&buf[i], REMAIN(i), " VxLAN");
>  #endif
> +#if IS_ENABLED(CONFIG_GENEVE)
> +	i += snprintf(&buf[i], REMAIN(i), "Geneve ");
> +#endif
>  	if (pf->flags & I40E_FLAG_PTP)
>  		i += snprintf(&buf[i], REMAIN(i), " PTP");
>  #ifdef I40E_FCOE

It seems like it should be " Geneve" rather than "Geneve "?

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 2/4] i40e: geneve tunnel offload support
  2015-12-08 18:20   ` Alexei Starovoitov
@ 2015-12-08 18:36     ` Jesse Gross
  2015-12-08 23:08       ` Singhai, Anjali
  0 siblings, 1 reply; 17+ messages in thread
From: Jesse Gross @ 2015-12-08 18:36 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Anjali Singhai Jain, Linux Kernel Network Developers,
	jeffrey.t.kirsher, Kiran Patil, Tom Herbert

On Tue, Dec 8, 2015 at 10:20 AM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Tue, Dec 08, 2015 at 10:12:12AM -0800, Anjali Singhai Jain wrote:
>> +/**
>> + * i40e_add_geneve_port - Get notifications about GENEVE ports that come up
>> + * @netdev: This physical port's netdev
>> + * @sa_family: Socket Family that GENEVE is notifying us about
>> + * @port: New UDP port number that GENEVE started listening to
>> + **/
>> +static void i40e_add_geneve_port(struct net_device *netdev,
>> +                              sa_family_t sa_family, __be16 port)
>> +{
>> +#if IS_ENABLED(CONFIG_GENEVE)
> ...
>> +     /* New port: add it and mark its index in the bitmap */
>> +     pf->udp_ports[next_idx].index = port;
>> +     pf->udp_ports[next_idx].type = I40E_AQC_TUNNEL_TYPE_NGE;
>
> the function suppose to deal with geneve but tunnel type is NGE ?!

NGE is an old name for Geneve: "Next Generation Encapsulation"

>> -#define I40E_MAX_TUNNEL_HDR_LEN 80
>> +/* Hardware supports L4 tunnel length of 128B (=2^7) which includes
>> + * inner mac plus all inner ethertypes.
>> + */
>> +#define I40E_MAX_TUNNEL_HDR_LEN 128
>
> so the driver lied about actual hw capabilities earlier
> or it needs firmware update to work this way?

I'm pretty sure that this is just making the calculation match the
hardware more accurately. If you look at the code below it, it is now
calculating the length from a different place.

>> -     if (!(tx_flags & I40E_TX_FLAGS_VXLAN_TUNNEL)) {
>> +     if (!(tx_flags & I40E_TX_FLAGS_UDP_TUNNEL)) {
> ...
>> @@ -163,7 +163,7 @@ enum i40e_dyn_idx_t {
>>  #define I40E_TX_FLAGS_FSO            BIT(7)
>>  #define I40E_TX_FLAGS_TSYN           BIT(8)
>>  #define I40E_TX_FLAGS_FD_SB          BIT(9)
>> -#define I40E_TX_FLAGS_VXLAN_TUNNEL   BIT(10)
>> +#define I40E_TX_FLAGS_UDP_TUNNEL     BIT(10)
>
> these changes implying that HW actually doesn't have special 'geneve or vxlan'
> hard coded logic and it's generic enough to understand most of udp tunnels.
> Then why you cannot generalize this whole things as generic udp tunnel offload
> and do not add any protocol specific hooks and ndos.

These are transmit flags but the issue of specialization relates to receive.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 2/4] i40e: geneve tunnel offload support
  2015-12-08 18:36     ` Jesse Gross
@ 2015-12-08 23:08       ` Singhai, Anjali
  0 siblings, 0 replies; 17+ messages in thread
From: Singhai, Anjali @ 2015-12-08 23:08 UTC (permalink / raw)
  To: Jesse Gross, Alexei Starovoitov
  Cc: Linux Kernel Network Developers, jeffrey.t.kirsher, Kiran Patil,
	Tom Herbert



On 12/8/2015 10:36 AM, Jesse Gross wrote:
> On Tue, Dec 8, 2015 at 10:20 AM, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>> On Tue, Dec 08, 2015 at 10:12:12AM -0800, Anjali Singhai Jain wrote:
>>> +/**
>>> + * i40e_add_geneve_port - Get notifications about GENEVE ports that come up
>>> + * @netdev: This physical port's netdev
>>> + * @sa_family: Socket Family that GENEVE is notifying us about
>>> + * @port: New UDP port number that GENEVE started listening to
>>> + **/
>>> +static void i40e_add_geneve_port(struct net_device *netdev,
>>> +                              sa_family_t sa_family, __be16 port)
>>> +{
>>> +#if IS_ENABLED(CONFIG_GENEVE)
>> ...
>>> +     /* New port: add it and mark its index in the bitmap */
>>> +     pf->udp_ports[next_idx].index = port;
>>> +     pf->udp_ports[next_idx].type = I40E_AQC_TUNNEL_TYPE_NGE;
>> the function suppose to deal with geneve but tunnel type is NGE ?!
> NGE is an old name for Geneve: "Next Generation Encapsulation"
Yes and that is why the old name in the SW/FW header files.
>>> -#define I40E_MAX_TUNNEL_HDR_LEN 80
>>> +/* Hardware supports L4 tunnel length of 128B (=2^7) which includes
>>> + * inner mac plus all inner ethertypes.
>>> + */
>>> +#define I40E_MAX_TUNNEL_HDR_LEN 128
>> so the driver lied about actual hw capabilities earlier
>> or it needs firmware update to work this way?
> I'm pretty sure that this is just making the calculation match the
> hardware more accurately. If you look at the code below it, it is now
> calculating the length from a different place.
It is making the code match the HW more accurately, which did support 
the 128 size
all along but till we enabled geneve support in the SW and FW,  the 
earlier setting was the right value.
>
>>> -     if (!(tx_flags & I40E_TX_FLAGS_VXLAN_TUNNEL)) {
>>> +     if (!(tx_flags & I40E_TX_FLAGS_UDP_TUNNEL)) {
>> ...
>>> @@ -163,7 +163,7 @@ enum i40e_dyn_idx_t {
>>>   #define I40E_TX_FLAGS_FSO            BIT(7)
>>>   #define I40E_TX_FLAGS_TSYN           BIT(8)
>>>   #define I40E_TX_FLAGS_FD_SB          BIT(9)
>>> -#define I40E_TX_FLAGS_VXLAN_TUNNEL   BIT(10)
>>> +#define I40E_TX_FLAGS_UDP_TUNNEL     BIT(10)
>> these changes implying that HW actually doesn't have special 'geneve or vxlan'
>> hard coded logic and it's generic enough to understand most of udp tunnels.
>> Then why you cannot generalize this whole things as generic udp tunnel offload
>> and do not add any protocol specific hooks and ndos.
> These are transmit flags but the issue of specialization relates to receive.
Second what Jesse said.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 2/4] i40e: geneve tunnel offload support
  2015-12-08 18:12 ` [PATCH v3 2/4] i40e: geneve tunnel offload support Anjali Singhai Jain
  2015-12-08 18:20   ` Alexei Starovoitov
  2015-12-08 18:22   ` John W. Linville
@ 2015-12-11  5:44   ` Jeff Kirsher
  2 siblings, 0 replies; 17+ messages in thread
From: Jeff Kirsher @ 2015-12-11  5:44 UTC (permalink / raw)
  To: Anjali Singhai Jain, netdev; +Cc: jeffrey.t.kirsher, Kiran Patil

[-- Attachment #1: Type: text/plain, Size: 694 bytes --]

On Tue, 2015-12-08 at 10:12 -0800, Anjali Singhai Jain wrote:
> This patch adds driver hooks to implement ndo_ops to add/del
> udp port in the HW to identify GENEVE tunnels.
> 
> Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
> Signed-off-by: Kiran Patil <kiran.patil@intel.com>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

> ---
>  drivers/net/ethernet/intel/i40e/i40e.h      |  16 +--
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 167
> ++++++++++++++++++++++------
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c |   8 +-
>  drivers/net/ethernet/intel/i40e/i40e_txrx.h |   2 +-
>  4 files changed, 150 insertions(+), 43 deletions(-)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 3/4] i40e: Kernel dependency update for i40e to support geneve offload
  2015-12-08 18:12 ` [PATCH v3 3/4] i40e: Kernel dependency update for i40e to support geneve offload Anjali Singhai Jain
@ 2015-12-11  5:45   ` Jeff Kirsher
  0 siblings, 0 replies; 17+ messages in thread
From: Jeff Kirsher @ 2015-12-11  5:45 UTC (permalink / raw)
  To: Anjali Singhai Jain, netdev; +Cc: jeffrey.t.kirsher, Kiran Patil

[-- Attachment #1: Type: text/plain, Size: 430 bytes --]

On Tue, 2015-12-08 at 10:12 -0800, Anjali Singhai Jain wrote:
> Update the Kconfig file with dependency for supporting GENEVE tunnel
> offloads.
> 
> Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
> Signed-off-by: Kiran Patil <kiran.patil@intel.com>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

> ---
>  drivers/net/ethernet/intel/Kconfig | 10 ++++++++++
>  1 file changed, 10 insertions(+)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/4] geneve: Add geneve udp port offload for ethernet devices
  2015-12-08 18:12 ` [PATCH v3 1/4] geneve: Add geneve udp port offload for ethernet devices Anjali Singhai Jain
@ 2015-12-12  0:58   ` David Miller
  2015-12-12  3:11   ` Tom Herbert
  1 sibling, 0 replies; 17+ messages in thread
From: David Miller @ 2015-12-12  0:58 UTC (permalink / raw)
  To: anjali.singhai; +Cc: netdev, jeffrey.t.kirsher, kiran.patil

From: Anjali Singhai Jain <anjali.singhai@intel.com>
Date: Tue,  8 Dec 2015 10:12:11 -0800

> @@ -1013,6 +1013,20 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>   *	a new port starts listening. The operation is protected by the
>   *	vxlan_net->sock_lock.
>   *
> + * void (*ndo_add_geneve_port)(struct net_device *dev,
> + *			      sa_family_t sa_family, __be16 port);
> + *	Called by geneve to notiy a driver about the UDP port and socket

"notify"

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 4/4] geneve: Add geneve_get_rx_port support
  2015-12-08 18:12 ` [PATCH v3 4/4] geneve: Add geneve_get_rx_port support Anjali Singhai Jain
@ 2015-12-12  1:00   ` David Miller
  0 siblings, 0 replies; 17+ messages in thread
From: David Miller @ 2015-12-12  1:00 UTC (permalink / raw)
  To: anjali.singhai; +Cc: netdev, jeffrey.t.kirsher

From: Anjali Singhai Jain <anjali.singhai@intel.com>
Date: Tue,  8 Dec 2015 10:12:14 -0800

> This patch adds an op that the drivers can call into to get existing
> geneve ports.
> 
> Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>

Add the new geneve infrastructure in one patch, then add the
call from the driver in another.

This looks like just a geneve change from the Subject line,
but what's hidden is that i40e is changed too.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/4] geneve: Add geneve udp port offload for ethernet devices
  2015-12-08 18:12 ` [PATCH v3 1/4] geneve: Add geneve udp port offload for ethernet devices Anjali Singhai Jain
  2015-12-12  0:58   ` David Miller
@ 2015-12-12  3:11   ` Tom Herbert
  2015-12-14 16:03     ` Singhai, Anjali
  1 sibling, 1 reply; 17+ messages in thread
From: Tom Herbert @ 2015-12-12  3:11 UTC (permalink / raw)
  To: Anjali Singhai Jain
  Cc: Linux Kernel Network Developers, jeffrey.t.kirsher, Kiran Patil

On Tue, Dec 8, 2015 at 10:12 AM, Anjali Singhai Jain
<anjali.singhai@intel.com> wrote:
> Add ndo_ops to add/del UDP ports to a device that supports geneve
> offload.
>
> v3: Add some more comments about the use of the new ndo ops.
>
> Signed-off-by: Anjali Singhai Jain <anjali.singhai at intel.com>
> Signed-off-by: Kiran Patil <kiran.patil at intel.com>
> ---
>  drivers/net/geneve.c      | 23 +++++++++++++++++++++++
>  include/linux/netdevice.h | 21 ++++++++++++++++++++-
>  2 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index de5c30c..b43fd56 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -371,8 +371,11 @@ static struct socket *geneve_create_sock(struct net *net, bool ipv6,
>
>  static void geneve_notify_add_rx_port(struct geneve_sock *gs)
>  {
> +       struct net_device *dev;
>         struct sock *sk = gs->sock->sk;
> +       struct net *net = sock_net(sk);
>         sa_family_t sa_family = sk->sk_family;
> +       __be16 port = inet_sk(sk)->inet_sport;
>         int err;
>
>         if (sa_family == AF_INET) {
> @@ -381,6 +384,14 @@ static void geneve_notify_add_rx_port(struct geneve_sock *gs)
>                         pr_warn("geneve: udp_add_offload failed with status %d\n",
>                                 err);
>         }
> +
> +       rcu_read_lock();
> +       for_each_netdev_rcu(net, dev) {
> +               if (dev->netdev_ops->ndo_add_geneve_port)
> +                       dev->netdev_ops->ndo_add_geneve_port(dev, sa_family,
> +                                                            port);
> +       }
> +       rcu_read_unlock();

What about IPv6 case?

>  }
>
>  static int geneve_hlen(struct genevehdr *gh)
> @@ -521,8 +532,20 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
>
>  static void geneve_notify_del_rx_port(struct geneve_sock *gs)
>  {
> +       struct net_device *dev;
>         struct sock *sk = gs->sock->sk;
> +       struct net *net = sock_net(sk);
>         sa_family_t sa_family = sk->sk_family;
> +       __be16 port = inet_sk(sk)->inet_sport;
> +
> +       rcu_read_lock();
> +       for_each_netdev_rcu(net, dev) {
> +               if (dev->netdev_ops->ndo_del_geneve_port)
> +                       dev->netdev_ops->ndo_del_geneve_port(dev, sa_family,
> +                                                            port);
> +       }
> +
> +       rcu_read_unlock();
>
>         if (sa_family == AF_INET)
>                 udp_del_offload(&gs->udp_offloads);
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 1bb21ff..ff91007 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1013,6 +1013,20 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>   *     a new port starts listening. The operation is protected by the
>   *     vxlan_net->sock_lock.
>   *
> + * void (*ndo_add_geneve_port)(struct net_device *dev,
> + *                           sa_family_t sa_family, __be16 port);
> + *     Called by geneve to notiy a driver about the UDP port and socket
> + *     address family that geneve is listnening to. It is called only when
> + *     a new port starts listening. The operation is protected by the
> + *     geneve_net->sock_lock. This should be strictly used by the driver
> + *     when setting up the device for RX side offloads only.
> + *
> + * void (*ndo_del_geneve_port)(struct net_device *dev,
> + *                           sa_family_t sa_family, __be16 port);
> + *     Called by geneve to notify the driver about a UDP port and socket
> + *     address family that geneve is not listening to anymore. The operation
> + *     is protected by the geneve_net->sock_lock.
> + *
>   * void (*ndo_del_vxlan_port)(struct  net_device *dev,
>   *                           sa_family_t sa_family, __be16 port);
>   *     Called by vxlan to notify the driver about a UDP port and socket
> @@ -1217,7 +1231,12 @@ struct net_device_ops {
>         void                    (*ndo_del_vxlan_port)(struct  net_device *dev,
>                                                       sa_family_t sa_family,
>                                                       __be16 port);
> -
> +       void                    (*ndo_add_geneve_port)(struct  net_device *dev,
> +                                                      sa_family_t sa_family,
> +                                                      __be16 port);
> +       void                    (*ndo_del_geneve_port)(struct  net_device *dev,
> +                                                      sa_family_t sa_family,
> +                                                      __be16 port);
>         void*                   (*ndo_dfwd_add_station)(struct net_device *pdev,
>                                                         struct net_device *dev);
>         void                    (*ndo_dfwd_del_station)(struct net_device *pdev,
> --
> 1.8.1.4
>
> --
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/4] geneve: Add geneve udp port offload for ethernet devices
  2015-12-12  3:11   ` Tom Herbert
@ 2015-12-14 16:03     ` Singhai, Anjali
  0 siblings, 0 replies; 17+ messages in thread
From: Singhai, Anjali @ 2015-12-14 16:03 UTC (permalink / raw)
  To: Tom Herbert
  Cc: Linux Kernel Network Developers, jeffrey.t.kirsher, Kiran Patil



On 12/11/2015 7:11 PM, Tom Herbert wrote:
> On Tue, Dec 8, 2015 at 10:12 AM, Anjali Singhai Jain
> <anjali.singhai@intel.com> wrote:
>> Add ndo_ops to add/del UDP ports to a device that supports geneve
>> offload.
>>
>> v3: Add some more comments about the use of the new ndo ops.
>>
>> Signed-off-by: Anjali Singhai Jain <anjali.singhai at intel.com>
>> Signed-off-by: Kiran Patil <kiran.patil at intel.com>
>> ---
>>   drivers/net/geneve.c      | 23 +++++++++++++++++++++++
>>   include/linux/netdevice.h | 21 ++++++++++++++++++++-
>>   2 files changed, 43 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
>> index de5c30c..b43fd56 100644
>> --- a/drivers/net/geneve.c
>> +++ b/drivers/net/geneve.c
>> @@ -371,8 +371,11 @@ static struct socket *geneve_create_sock(struct net *net, bool ipv6,
>>
>>   static void geneve_notify_add_rx_port(struct geneve_sock *gs)
>>   {
>> +       struct net_device *dev;
>>          struct sock *sk = gs->sock->sk;
>> +       struct net *net = sock_net(sk);
>>          sa_family_t sa_family = sk->sk_family;
>> +       __be16 port = inet_sk(sk)->inet_sport;
>>          int err;
>>
>>          if (sa_family == AF_INET) {
>> @@ -381,6 +384,14 @@ static void geneve_notify_add_rx_port(struct geneve_sock *gs)
>>                          pr_warn("geneve: udp_add_offload failed with status %d\n",
>>                                  err);
>>          }
>> +
>> +       rcu_read_lock();
>> +       for_each_netdev_rcu(net, dev) {
>> +               if (dev->netdev_ops->ndo_add_geneve_port)
>> +                       dev->netdev_ops->ndo_add_geneve_port(dev, sa_family,
>> +                                                            port);
>> +       }
>> +       rcu_read_unlock();
> What about IPv6 case?

The driver still gets add port calls for IPv6 and can decide to offload 
L4 RX checksum if the HW is capable.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Add support for Geneve udp port offload
  2015-12-04  8:19 Add support for Geneve udp port offload Anjali Singhai Jain
@ 2015-12-04 16:31 ` Tom Herbert
  0 siblings, 0 replies; 17+ messages in thread
From: Tom Herbert @ 2015-12-04 16:31 UTC (permalink / raw)
  To: Anjali Singhai Jain; +Cc: Linux Kernel Network Developers

On Fri, Dec 4, 2015 at 12:19 AM, Anjali Singhai Jain
<anjali.singhai@intel.com> wrote:
> This patch series adds new ndo ops for Geneve add/del port, so as
> to help offload Geneve tunnel functionalities such as checksum, TSO
> RSS, filters etc.
>
> i40e driver has been tested with the changes to make sure the offloads
> happen.
>
> We do understand that this is not the ideal solution and most likely
> will be redone with a more generic offload framework.
> But this certainly will enable us to start seeing benefits of the
> accelerations for Geneve tunnels.
>
Anjali,

This is a lot of complexity being added to i40e driver and kernel. It
needs to be justified. Please:

1) Quantify the effects of the offloads. Please provide a set of
performance numbers for various benchmarks showing tps, CPU, latency,
etc. You can peruse some of the patches I have done for GUE or VXLAN
for ideas on reporting performance.
2) The baselines for the above testing should include UDP checksum
enabled with Remote Checksum Offload and GRO/GSO in full effect. I
don't know if RCO has been implemented for Geneve, but it should be
pretty straightforward to add if it's not.
3) The ndo function needs to be clearly documented. Precisely what is
a driver allowed to do with this port information? We already have at
least one case in VXLAN where a driver is incorrectly using the
provide port in the transmit path, this should be explicitly forbidden
in the description.

Thanks,
Tom

> As a side note, we did find an existing issue in i40e driver where a
> service task can modify tunnel data structures with no locks held to
> help linearize access. A separate patch will be taking care of that issue.
>
> A question out to the community is regarding the driver Kconfig parameters
> for VxLAN and Geneve, it would be ideal to drop those if there is a way
> to help resolve vxlan/geneve_get_rx_port symbols while the tunnel modules
> are not loaded.
>
> Anjali Singhai Jain (4):
>   [RFC PATCH v2 1/4] geneve: Add geneve udp port offload for ethernet
>   [RFC PATCH v2 2/4] i40e: geneve tunnel offload support
>   [RFC PATCH v2 3/4] i40e: Kernel dependency update for i40e to support
>   [RFC PATCH v2 4/4] geneve: Add geneve_get_rx_port support
> --
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Add support for Geneve udp port offload
@ 2015-12-04  8:19 Anjali Singhai Jain
  2015-12-04 16:31 ` Tom Herbert
  0 siblings, 1 reply; 17+ messages in thread
From: Anjali Singhai Jain @ 2015-12-04  8:19 UTC (permalink / raw)
  To: netdev

This patch series adds new ndo ops for Geneve add/del port, so as
to help offload Geneve tunnel functionalities such as checksum, TSO
RSS, filters etc.

i40e driver has been tested with the changes to make sure the offloads
happen.

We do understand that this is not the ideal solution and most likely
will be redone with a more generic offload framework.
But this certainly will enable us to start seeing benefits of the
accelerations for Geneve tunnels.

As a side note, we did find an existing issue in i40e driver where a
service task can modify tunnel data structures with no locks held to
help linearize access. A separate patch will be taking care of that issue.

A question out to the community is regarding the driver Kconfig parameters
for VxLAN and Geneve, it would be ideal to drop those if there is a way
to help resolve vxlan/geneve_get_rx_port symbols while the tunnel modules
are not loaded.

Anjali Singhai Jain (4):
  [RFC PATCH v2 1/4] geneve: Add geneve udp port offload for ethernet
  [RFC PATCH v2 2/4] i40e: geneve tunnel offload support
  [RFC PATCH v2 3/4] i40e: Kernel dependency update for i40e to support
  [RFC PATCH v2 4/4] geneve: Add geneve_get_rx_port support

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-12-14 16:04 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-08 18:12 Add support for Geneve udp port offload Anjali Singhai Jain
2015-12-08 18:12 ` [PATCH v3 1/4] geneve: Add geneve udp port offload for ethernet devices Anjali Singhai Jain
2015-12-12  0:58   ` David Miller
2015-12-12  3:11   ` Tom Herbert
2015-12-14 16:03     ` Singhai, Anjali
2015-12-08 18:12 ` [PATCH v3 2/4] i40e: geneve tunnel offload support Anjali Singhai Jain
2015-12-08 18:20   ` Alexei Starovoitov
2015-12-08 18:36     ` Jesse Gross
2015-12-08 23:08       ` Singhai, Anjali
2015-12-08 18:22   ` John W. Linville
2015-12-11  5:44   ` Jeff Kirsher
2015-12-08 18:12 ` [PATCH v3 3/4] i40e: Kernel dependency update for i40e to support geneve offload Anjali Singhai Jain
2015-12-11  5:45   ` Jeff Kirsher
2015-12-08 18:12 ` [PATCH v3 4/4] geneve: Add geneve_get_rx_port support Anjali Singhai Jain
2015-12-12  1:00   ` David Miller
  -- strict thread matches above, loose matches on Subject: below --
2015-12-04  8:19 Add support for Geneve udp port offload Anjali Singhai Jain
2015-12-04 16:31 ` Tom Herbert

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.