Attachment 1454715 Details for Bug 1526306 – beta patch for the kernel data path

[patch] beta patch for the kernel data path

0001-openvswitch-send-upcalls-in-round-robin.patch (text/plain), 10.92 KB, created by Matteo Croce on 2018-06-26 16:32:58 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Matteo Croce

Created: 2018-06-26 16:32:58 UTC

Size: 10.92 KB

patch

obsolete

>From 2d9b713ffe0783ade9b0cf629dcb57ee6a838d36 Mon Sep 17 00:00:00 2001
>From: Matteo Croce <mcroce@redhat.com>
>Date: Fri, 1 Jun 2018 16:37:12 +0200
>Subject: [PATCH] openvswitch: send upcalls in round robin
>
>Open vSwitch sends to the userspace all received packets that don't have
>any associated flow (thus doing an "upcall"). Then, the userspace program
>creates a new flow and determines the actions to apply based on its
>configuration.
>
>When a single port is generating a lot of upcalls, it can prevent other
>ports from dispatching their own upcalls. vswitchd overcomes this problem
>by creating many netlink sockets per port, but this creates problems when
>dealing with huge amounts of ports.
>
>This patch queues all the upcall packets into a per-port list, and runs a
>task which sends a single upcall for each port in a round-robin fashion.
>This ensures fairness among ports under load and with few netlink sockets.
>
>Signed-off-by: Matteo Croce <mcroce@redhat.com>
>---
> net/openvswitch/datapath.c | 185 +++++++++++++++++++++++++++++--------
> net/openvswitch/datapath.h |  23 +++++
> net/openvswitch/vport.c    |  14 +++
> net/openvswitch/vport.h    |   7 ++
> 4 files changed, 188 insertions(+), 41 deletions(-)
>
>diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
>index 0f5ce77460d4..39645a8f98ae 100644
>--- a/net/openvswitch/datapath.c
>+++ b/net/openvswitch/datapath.c
>@@ -59,6 +59,8 @@
> #include "vport-internal_dev.h"
> #include "vport-netdev.h"
> 
>+#define UPCALL_QUEUE_LIMIT 16
>+
> unsigned int ovs_net_id __read_mostly;
> 
> static struct genl_family dp_packet_genl_family;
>@@ -199,6 +201,15 @@ struct vport *ovs_lookup_vport(const struct datapath *dp, u16 port_no)
> 	return NULL;
> }
> 
>+static void stats_lost_inc(struct datapath *dp)
>+{
>+	struct dp_stats_percpu *stats = this_cpu_ptr(dp->stats_percpu);
>+
>+	u64_stats_update_begin(&stats->syncp);
>+	stats->n_lost++;
>+	u64_stats_update_end(&stats->syncp);
>+}
>+
> /* Called with ovs_mutex. */
> static struct vport *new_vport(const struct vport_parms *parms)
> {
>@@ -225,6 +236,63 @@ void ovs_dp_detach_port(struct vport *p)
> 	ovs_vport_del(p);
> }
> 
>+int ovs_dp_upcall(struct datapath *dp, struct sk_buff *skb,
>+		  const struct sw_flow_key *key,
>+		  const struct dp_upcall_info *upcall_info,
>+		  uint32_t cutlen)
>+{
>+	int ret = 0;
>+
>+	if (upcall_info->portid == 0) {
>+		stats_lost_inc(dp);
>+		return -ENOTCONN;
>+	}
>+
>+	if (!skb_is_gso(skb))
>+		ret = queue_userspace_packet(dp, skb, key, upcall_info, cutlen);
>+	else
>+		ret = queue_gso_packets(dp, skb, key, upcall_info, cutlen);
>+
>+	if (ret)
>+		stats_lost_inc(dp);
>+
>+	return ret;
>+}
>+
>+static int ovs_dp_deferred_upcall(struct upcall_packet *upcall_pkt)
>+{
>+	struct vport *vport = OVS_CB(upcall_pkt->skb)->input_vport;
>+	int ret = 0;
>+
>+	if (upcall_pkt->upcall_info.portid == 0) {
>+		ret = -ENOTCONN;
>+		goto err;
>+	}
>+
>+	spin_lock_bh(&vport->upcall_lock);
>+	if (vport->upcall_len >= UPCALL_QUEUE_LIMIT) {
>+		spin_unlock_bh(&vport->upcall_lock);
>+		ret = -ENOSPC;
>+		goto err;
>+	}
>+	list_add_tail(&upcall_pkt->list, &vport->upcall_list);
>+	vport->upcall_len++;
>+	spin_unlock_bh(&vport->upcall_lock);
>+
>+	vport->dp->pending_upcall = true;
>+	schedule_work(&vport->dp->upcall_rr);
>+
>+	return 0;
>+
>+err:
>+	kfree_skb(upcall_pkt->skb);
>+	kfree(upcall_pkt);
>+
>+	stats_lost_inc(vport->dp);
>+
>+	return ret;
>+}
>+
> /* Must be called with rcu_read_lock. */
> void ovs_dp_process_packet(struct sk_buff *skb, struct sw_flow_key *key)
> {
>@@ -241,27 +309,31 @@ void ovs_dp_process_packet(struct sk_buff *skb, struct sw_flow_key *key)
> 	/* Look up flow. */
> 	flow = ovs_flow_tbl_lookup_stats(&dp->table, key, &n_mask_hit);
> 	if (unlikely(!flow)) {
>-		struct dp_upcall_info upcall;
>-		int error;
>-
>-		memset(&upcall, 0, sizeof(upcall));
>-		upcall.cmd = OVS_PACKET_CMD_MISS;
>-		upcall.portid = ovs_vport_find_upcall_portid(p, skb);
>-		upcall.mru = OVS_CB(skb)->mru;
>-		error = ovs_dp_upcall(dp, skb, key, &upcall, 0);
>-		if (unlikely(error))
>-			kfree_skb(skb);
>-		else
>-			consume_skb(skb);
>+		struct upcall_packet *upcall_pkt;
>+
> 		stats_counter = &stats->n_missed;
>-		goto out;
>-	}
> 
>-	ovs_flow_stats_update(flow, key->tp.flags, skb);
>-	sf_acts = rcu_dereference(flow->sf_acts);
>-	ovs_execute_actions(dp, skb, sf_acts, key);
>+		upcall_pkt = kzalloc(sizeof(*upcall_pkt), GFP_ATOMIC);
>+		if (!upcall_pkt) {
>+			kfree_skb(skb);
>+			goto out;
>+		}
>+
>+		upcall_pkt->upcall_info.cmd = OVS_PACKET_CMD_MISS;
>+		upcall_pkt->upcall_info.portid =
>+			ovs_vport_find_upcall_portid(p, skb);
>+		upcall_pkt->upcall_info.mru = OVS_CB(skb)->mru;
>+		upcall_pkt->skb = skb;
>+		upcall_pkt->key = *key;
> 
>-	stats_counter = &stats->n_hit;
>+		ovs_dp_deferred_upcall(upcall_pkt);
>+	} else {
>+		stats_counter = &stats->n_hit;
>+
>+		ovs_flow_stats_update(flow, key->tp.flags, skb);
>+		sf_acts = rcu_dereference(flow->sf_acts);
>+		ovs_execute_actions(dp, skb, sf_acts, key);
>+	}
> 
> out:
> 	/* Update datapath statistics. */
>@@ -271,36 +343,63 @@ void ovs_dp_process_packet(struct sk_buff *skb, struct sw_flow_key *key)
> 	u64_stats_update_end(&stats->syncp);
> }
> 
>-int ovs_dp_upcall(struct datapath *dp, struct sk_buff *skb,
>-		  const struct sw_flow_key *key,
>-		  const struct dp_upcall_info *upcall_info,
>-		  uint32_t cutlen)
>+static int send_upcall(struct upcall_packet *upcall)
> {
>-	struct dp_stats_percpu *stats;
>-	int err;
>+	struct vport *vport = OVS_CB(upcall->skb)->input_vport;
> 
>-	if (upcall_info->portid == 0) {
>-		err = -ENOTCONN;
>-		goto err;
>-	}
>+	if (!skb_is_gso(upcall->skb))
>+		return queue_userspace_packet(vport->dp, upcall->skb,
>+					      &upcall->key,
>+					      &upcall->upcall_info,
>+					      upcall->cutlen);
> 
>-	if (!skb_is_gso(skb))
>-		err = queue_userspace_packet(dp, skb, key, upcall_info, cutlen);
>-	else
>-		err = queue_gso_packets(dp, skb, key, upcall_info, cutlen);
>-	if (err)
>-		goto err;
>+	return queue_gso_packets(vport->dp, upcall->skb, &upcall->key,
>+				 &upcall->upcall_info, upcall->cutlen);
>+}
> 
>-	return 0;
>+static void vport_send_upcall(struct vport *vport)
>+{
>+	struct upcall_packet *upcall;
> 
>-err:
>-	stats = this_cpu_ptr(dp->stats_percpu);
>+	spin_lock_bh(&vport->upcall_lock);
>+	upcall = list_first_entry_or_null(&vport->upcall_list,
>+					  struct upcall_packet,
>+					  list);
>+	if (!upcall) {
>+		spin_unlock_bh(&vport->upcall_lock);
>+		return;
>+	}
>+	list_del(&upcall->list);
>+	if (--vport->upcall_len)
>+		vport->dp->pending_upcall = true;
>+	spin_unlock_bh(&vport->upcall_lock);
> 
>-	u64_stats_update_begin(&stats->syncp);
>-	stats->n_lost++;
>-	u64_stats_update_end(&stats->syncp);
>+	if (unlikely(send_upcall(upcall))) {
>+		stats_lost_inc(vport->dp);
>+		kfree_skb(upcall->skb);
>+	} else {
>+		consume_skb(upcall->skb);
>+	}
> 
>-	return err;
>+	kfree(upcall);
>+}
>+
>+static void upcall_rr(struct work_struct *work)
>+{
>+	struct datapath *dp = container_of(work, struct datapath, upcall_rr);
>+	struct vport *vport;
>+	int i;
>+
>+	rcu_read_lock();
>+	do {
>+		dp->pending_upcall = false;
>+		for (i = 0; i < DP_VPORT_HASH_BUCKETS; i++) {
>+			hlist_for_each_entry_rcu(vport, &dp->ports[i],
>+						 dp_hash_node)
>+				vport_send_upcall(vport);
>+		}
>+	} while (dp->pending_upcall);
>+	rcu_read_unlock();
> }
> 
> static int queue_gso_packets(struct datapath *dp, struct sk_buff *skb,
>@@ -1628,6 +1727,8 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
> 				   info->snd_seq, 0, OVS_DP_CMD_NEW);
> 	BUG_ON(err < 0);
> 
>+	INIT_WORK(&dp->upcall_rr, upcall_rr);
>+
> 	ovs_net = net_generic(ovs_dp_get_net(dp), ovs_net_id);
> 	list_add_tail_rcu(&dp->list_node, &ovs_net->dps);
> 
>@@ -1694,6 +1795,8 @@ static int ovs_dp_cmd_del(struct sk_buff *skb, struct genl_info *info)
> 	if (IS_ERR(dp))
> 		goto err_unlock_free;
> 
>+	cancel_work_sync(&dp->upcall_rr);
>+
> 	err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
> 				   info->snd_seq, 0, OVS_DP_CMD_DEL);
> 	BUG_ON(err < 0);
>diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
>index c9eb267c6f7e..c194d4754269 100644
>--- a/net/openvswitch/datapath.h
>+++ b/net/openvswitch/datapath.h
>@@ -24,6 +24,7 @@
> #include <linux/mutex.h>
> #include <linux/netdevice.h>
> #include <linux/skbuff.h>
>+#include <linux/workqueue.h>
> #include <linux/u64_stats_sync.h>
> #include <net/ip_tunnels.h>
> 
>@@ -70,6 +71,8 @@ struct dp_stats_percpu {
>  * @net: Reference to net namespace.
>  * @max_headroom: the maximum headroom of all vports in this datapath; it will
>  * be used by all the internal vports in this dp.
>+ * @upcall_rr: sends deferred upcalls to userspace.
>+ * @pending_upcall: there are upcalls ready to be sent.
>  *
>  * Context: See the comment on locking at the top of datapath.c for additional
>  * locking information.
>@@ -96,6 +99,10 @@ struct datapath {
> 
> 	/* Switch meters. */
> 	struct hlist_head *meters;
>+
>+	/* upcalls round robin sender */
>+	struct work_struct upcall_rr;
>+	bool pending_upcall;
> };
> 
> /**
>@@ -136,6 +143,22 @@ struct dp_upcall_info {
> 	u16 mru;
> };
> 
>+/**
>+ * struct upcall_packet - represent an upcall to be sent
>+ * @list: list within vport.
>+ * @skb: the sk_buff which generated the upcall.
>+ * @key: flow key.
>+ * @upcall_info: struct dp_upcall with metadata to send to userspace.
>+ * @cutlen: needed by queue_userspace_packet().
>+ */
>+struct upcall_packet {
>+	struct list_head list;
>+	struct sk_buff *skb;
>+	struct sw_flow_key key;
>+	struct dp_upcall_info upcall_info;
>+	u32 cutlen;
>+};
>+
> /**
>  * struct ovs_net - Per net-namespace data for ovs.
>  * @dps: List of datapaths to enable dumping them all out.
>diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
>index 19f6765566e7..ec8fe8757f6c 100644
>--- a/net/openvswitch/vport.c
>+++ b/net/openvswitch/vport.c
>@@ -148,6 +148,8 @@ struct vport *ovs_vport_alloc(int priv_size, const struct vport_ops *ops,
> 	vport->port_no = parms->port_no;
> 	vport->ops = ops;
> 	INIT_HLIST_NODE(&vport->dp_hash_node);
>+	INIT_LIST_HEAD(&vport->upcall_list);
>+	spin_lock_init(&vport->upcall_lock);
> 
> 	if (ovs_vport_set_upcall_portids(vport, parms->upcall_portids)) {
> 		kfree(vport);
>@@ -170,6 +172,18 @@ EXPORT_SYMBOL_GPL(ovs_vport_alloc);
>  */
> void ovs_vport_free(struct vport *vport)
> {
>+	struct list_head *q;
>+
>+	spin_lock_bh(&vport->upcall_lock);
>+	list_for_each(q, &vport->upcall_list) {
>+		struct upcall_packet *upcall;
>+
>+		upcall = list_entry(q, struct upcall_packet, list);
>+		kfree_skb(upcall->skb);
>+		kfree(upcall);
>+	}
>+	spin_unlock_bh(&vport->upcall_lock);
>+
> 	/* vport is freed from RCU callback or error path, Therefore
> 	 * it is safe to use raw dereference.
> 	 */
>diff --git a/net/openvswitch/vport.h b/net/openvswitch/vport.h
>index cda66c26ad08..48eb027ea5a5 100644
>--- a/net/openvswitch/vport.h
>+++ b/net/openvswitch/vport.h
>@@ -79,6 +79,9 @@ struct vport_portids {
>  * @ops: Class structure.
>  * @detach_list: list used for detaching vport in net-exit call.
>  * @rcu: RCU callback head for deferred destruction.
>+ * @upcall_list: list of queued upcalls.
>+ * @upcall_lock: lock for the upcall list.
>+ * @upcall_len: number of queued upcalls.
>  */
> struct vport {
> 	struct net_device *dev;
>@@ -92,6 +95,10 @@ struct vport {
> 
> 	struct list_head detach_list;
> 	struct rcu_head rcu;
>+
>+	struct list_head upcall_list;
>+	spinlock_t upcall_lock; /* protects the upcall list */
>+	int upcall_len;
> };
> 
> /**
>-- 
>2.17.1
>

Actions: View | Diff

Attachments on bug 1526306: 1383546 | 1452875 | 1454715 | 1477839 | 1477840 | 1483331