* [PATCH net-next 06/11] openvswitch: Simplify mega-flow APIs.
From: Jesse Gross @ 2013-10-30 0:22 UTC (permalink / raw)
To: David Miller; +Cc: netdev, dev, Pravin B Shelar, Jesse Gross
In-Reply-To: <1383092544-50599-1-git-send-email-jesse@nicira.com>
From: Pravin B Shelar <pshelar@nicira.com>
Hides mega-flow implementation in flow_table.c rather than
datapath.c.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
---
net/openvswitch/datapath.c | 27 +++------
net/openvswitch/flow_table.c | 138 +++++++++++++++++++++++++------------------
net/openvswitch/flow_table.h | 12 +---
3 files changed, 89 insertions(+), 88 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 60b9be3..cf27097 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -161,7 +161,7 @@ static void destroy_dp_rcu(struct rcu_head *rcu)
{
struct datapath *dp = container_of(rcu, struct datapath, rcu);
- ovs_flow_tbl_destroy(&dp->table, false);
+ ovs_flow_tbl_destroy(&dp->table);
free_percpu(dp->stats_percpu);
release_net(ovs_dp_get_net(dp));
kfree(dp->ports);
@@ -795,8 +795,6 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
/* Check if this is a duplicate flow */
flow = ovs_flow_tbl_lookup(&dp->table, &key);
if (!flow) {
- struct sw_flow_mask *mask_p;
-
/* Bail out if we're not allowed to create a new flow. */
error = -ENOENT;
if (info->genlhdr->cmd == OVS_FLOW_CMD_SET)
@@ -812,25 +810,14 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
flow->key = masked_key;
flow->unmasked_key = key;
-
- /* Make sure mask is unique in the system */
- mask_p = ovs_sw_flow_mask_find(&dp->table, &mask);
- if (!mask_p) {
- /* Allocate a new mask if none exsits. */
- mask_p = ovs_sw_flow_mask_alloc();
- if (!mask_p)
- goto err_flow_free;
- mask_p->key = mask.key;
- mask_p->range = mask.range;
- ovs_sw_flow_mask_insert(&dp->table, mask_p);
- }
-
- ovs_sw_flow_mask_add_ref(mask_p);
- flow->mask = mask_p;
rcu_assign_pointer(flow->sf_acts, acts);
/* Put flow in bucket. */
- ovs_flow_tbl_insert(&dp->table, flow);
+ error = ovs_flow_tbl_insert(&dp->table, flow, &mask);
+ if (error) {
+ acts = NULL;
+ goto err_flow_free;
+ }
reply = ovs_flow_cmd_build_info(flow, dp, info->snd_portid,
info->snd_seq, OVS_FLOW_CMD_NEW);
@@ -1236,7 +1223,7 @@ err_destroy_ports_array:
err_destroy_percpu:
free_percpu(dp->stats_percpu);
err_destroy_table:
- ovs_flow_tbl_destroy(&dp->table, false);
+ ovs_flow_tbl_destroy(&dp->table);
err_free_dp:
release_net(ovs_dp_get_net(dp));
kfree(dp);
diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
index 1c7e773..036e019 100644
--- a/net/openvswitch/flow_table.c
+++ b/net/openvswitch/flow_table.c
@@ -128,12 +128,36 @@ static void rcu_free_flow_callback(struct rcu_head *rcu)
flow_free(flow);
}
+static void rcu_free_sw_flow_mask_cb(struct rcu_head *rcu)
+{
+ struct sw_flow_mask *mask = container_of(rcu, struct sw_flow_mask, rcu);
+
+ kfree(mask);
+}
+
+static void flow_mask_del_ref(struct sw_flow_mask *mask, bool deferred)
+{
+ if (!mask)
+ return;
+
+ BUG_ON(!mask->ref_count);
+ mask->ref_count--;
+
+ if (!mask->ref_count) {
+ list_del_rcu(&mask->list);
+ if (deferred)
+ call_rcu(&mask->rcu, rcu_free_sw_flow_mask_cb);
+ else
+ kfree(mask);
+ }
+}
+
void ovs_flow_free(struct sw_flow *flow, bool deferred)
{
if (!flow)
return;
- ovs_sw_flow_mask_del_ref(flow->mask, deferred);
+ flow_mask_del_ref(flow->mask, deferred);
if (deferred)
call_rcu(&flow->rcu, rcu_free_flow_callback);
@@ -225,11 +249,11 @@ static void table_instance_destroy(struct table_instance *ti, bool deferred)
__table_instance_destroy(ti);
}
-void ovs_flow_tbl_destroy(struct flow_table *table, bool deferred)
+void ovs_flow_tbl_destroy(struct flow_table *table)
{
struct table_instance *ti = ovsl_dereference(table->ti);
- table_instance_destroy(ti, deferred);
+ table_instance_destroy(ti, false);
}
struct sw_flow *ovs_flow_tbl_dump_next(struct table_instance *ti,
@@ -304,7 +328,7 @@ static struct table_instance *table_instance_rehash(struct table_instance *ti,
new_ti = table_instance_alloc(n_buckets);
if (!new_ti)
- return ERR_PTR(-ENOMEM);
+ return NULL;
flow_table_copy_flows(ti, new_ti);
@@ -425,32 +449,6 @@ static struct table_instance *table_instance_expand(struct table_instance *ti)
return table_instance_rehash(ti, ti->n_buckets * 2);
}
-void ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow)
-{
- struct table_instance *ti = NULL;
- struct table_instance *new_ti = NULL;
-
- ti = ovsl_dereference(table->ti);
-
- /* Expand table, if necessary, to make room. */
- if (table->count > ti->n_buckets)
- new_ti = table_instance_expand(ti);
- else if (time_after(jiffies, table->last_rehash + REHASH_INTERVAL))
- new_ti = table_instance_rehash(ti, ti->n_buckets);
-
- if (new_ti && !IS_ERR(new_ti)) {
- rcu_assign_pointer(table->ti, new_ti);
- ovs_flow_tbl_destroy(table, true);
- ti = ovsl_dereference(table->ti);
- table->last_rehash = jiffies;
- }
-
- flow->hash = flow_hash(&flow->key, flow->mask->range.start,
- flow->mask->range.end);
- table_instance_insert(ti, flow);
- table->count++;
-}
-
void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow)
{
struct table_instance *ti = ovsl_dereference(table->ti);
@@ -460,7 +458,7 @@ void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow)
table->count--;
}
-struct sw_flow_mask *ovs_sw_flow_mask_alloc(void)
+static struct sw_flow_mask *mask_alloc(void)
{
struct sw_flow_mask *mask;
@@ -471,35 +469,11 @@ struct sw_flow_mask *ovs_sw_flow_mask_alloc(void)
return mask;
}
-void ovs_sw_flow_mask_add_ref(struct sw_flow_mask *mask)
+static void mask_add_ref(struct sw_flow_mask *mask)
{
mask->ref_count++;
}
-static void rcu_free_sw_flow_mask_cb(struct rcu_head *rcu)
-{
- struct sw_flow_mask *mask = container_of(rcu, struct sw_flow_mask, rcu);
-
- kfree(mask);
-}
-
-void ovs_sw_flow_mask_del_ref(struct sw_flow_mask *mask, bool deferred)
-{
- if (!mask)
- return;
-
- BUG_ON(!mask->ref_count);
- mask->ref_count--;
-
- if (!mask->ref_count) {
- list_del_rcu(&mask->list);
- if (deferred)
- call_rcu(&mask->rcu, rcu_free_sw_flow_mask_cb);
- else
- kfree(mask);
- }
-}
-
static bool mask_equal(const struct sw_flow_mask *a,
const struct sw_flow_mask *b)
{
@@ -511,7 +485,7 @@ static bool mask_equal(const struct sw_flow_mask *a,
&& (memcmp(a_, b_, range_n_bytes(&a->range)) == 0);
}
-struct sw_flow_mask *ovs_sw_flow_mask_find(const struct flow_table *tbl,
+static struct sw_flow_mask *flow_mask_find(const struct flow_table *tbl,
const struct sw_flow_mask *mask)
{
struct list_head *ml;
@@ -531,9 +505,55 @@ struct sw_flow_mask *ovs_sw_flow_mask_find(const struct flow_table *tbl,
* The caller needs to make sure that 'mask' is not the same
* as any masks that are already on the list.
*/
-void ovs_sw_flow_mask_insert(struct flow_table *tbl, struct sw_flow_mask *mask)
+static int flow_mask_insert(struct flow_table *tbl, struct sw_flow *flow,
+ struct sw_flow_mask *new)
+{
+ struct sw_flow_mask *mask;
+ mask = flow_mask_find(tbl, new);
+ if (!mask) {
+ /* Allocate a new mask if none exsits. */
+ mask = mask_alloc();
+ if (!mask)
+ return -ENOMEM;
+ mask->key = new->key;
+ mask->range = new->range;
+ list_add_rcu(&mask->list, &tbl->mask_list);
+ }
+
+ mask_add_ref(mask);
+ flow->mask = mask;
+ return 0;
+}
+
+int ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow,
+ struct sw_flow_mask *mask)
{
- list_add_rcu(&mask->list, &tbl->mask_list);
+ struct table_instance *new_ti = NULL;
+ struct table_instance *ti;
+ int err;
+
+ err = flow_mask_insert(table, flow, mask);
+ if (err)
+ return err;
+
+ flow->hash = flow_hash(&flow->key, flow->mask->range.start,
+ flow->mask->range.end);
+ ti = ovsl_dereference(table->ti);
+ table_instance_insert(ti, flow);
+ table->count++;
+
+ /* Expand table, if necessary, to make room. */
+ if (table->count > ti->n_buckets)
+ new_ti = table_instance_expand(ti);
+ else if (time_after(jiffies, table->last_rehash + REHASH_INTERVAL))
+ new_ti = table_instance_rehash(ti, ti->n_buckets);
+
+ if (new_ti) {
+ rcu_assign_pointer(table->ti, new_ti);
+ table_instance_destroy(ti, true);
+ table->last_rehash = jiffies;
+ }
+ return 0;
}
/* Initializes the flow module.
diff --git a/net/openvswitch/flow_table.h b/net/openvswitch/flow_table.h
index 5d1abe5..4db5f78 100644
--- a/net/openvswitch/flow_table.h
+++ b/net/openvswitch/flow_table.h
@@ -60,10 +60,11 @@ void ovs_flow_free(struct sw_flow *, bool deferred);
int ovs_flow_tbl_init(struct flow_table *);
int ovs_flow_tbl_count(struct flow_table *table);
-void ovs_flow_tbl_destroy(struct flow_table *table, bool deferred);
+void ovs_flow_tbl_destroy(struct flow_table *table);
int ovs_flow_tbl_flush(struct flow_table *flow_table);
-void ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow);
+int ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow,
+ struct sw_flow_mask *mask);
void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow);
struct sw_flow *ovs_flow_tbl_dump_next(struct table_instance *table,
u32 *bucket, u32 *idx);
@@ -73,13 +74,6 @@ struct sw_flow *ovs_flow_tbl_lookup(struct flow_table *,
bool ovs_flow_cmp_unmasked_key(const struct sw_flow *flow,
struct sw_flow_match *match);
-struct sw_flow_mask *ovs_sw_flow_mask_alloc(void);
-void ovs_sw_flow_mask_add_ref(struct sw_flow_mask *);
-void ovs_sw_flow_mask_del_ref(struct sw_flow_mask *, bool deferred);
-void ovs_sw_flow_mask_insert(struct flow_table *, struct sw_flow_mask *);
-struct sw_flow_mask *ovs_sw_flow_mask_find(const struct flow_table *,
- const struct sw_flow_mask *);
void ovs_flow_mask_key(struct sw_flow_key *dst, const struct sw_flow_key *src,
const struct sw_flow_mask *mask);
-
#endif /* flow_table.h */
--
1.8.3.2
^ permalink raw reply related
* [PATCH net-next 08/11] openvswitch: Per cpu flow stats.
From: Jesse Gross @ 2013-10-30 0:22 UTC (permalink / raw)
To: David Miller; +Cc: netdev, dev, Pravin B Shelar, Jesse Gross
In-Reply-To: <1383092544-50599-1-git-send-email-jesse@nicira.com>
From: Pravin B Shelar <pshelar@nicira.com>
With mega flow implementation ovs flow can be shared between
multiple CPUs which makes stats updates highly contended
operation. Following patch allocates separate stats for each
CPU to make stats update scalable.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
---
net/openvswitch/datapath.c | 54 +++++++++++++----------------------
net/openvswitch/flow.c | 68 +++++++++++++++++++++++++++++++++++++++-----
net/openvswitch/flow.h | 20 ++++++++-----
net/openvswitch/flow_table.c | 14 +++++++--
4 files changed, 105 insertions(+), 51 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 5bc5a4e..8c8875b 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -251,9 +251,9 @@ void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb)
OVS_CB(skb)->flow = flow;
OVS_CB(skb)->pkt_key = &key;
- stats_counter = &stats->n_hit;
- ovs_flow_used(OVS_CB(skb)->flow, skb);
+ ovs_flow_stats_update(OVS_CB(skb)->flow, skb);
ovs_execute_actions(dp, skb);
+ stats_counter = &stats->n_hit;
out:
/* Update datapath statistics. */
@@ -453,14 +453,6 @@ out:
return err;
}
-static void clear_stats(struct sw_flow *flow)
-{
- flow->used = 0;
- flow->tcp_flags = 0;
- flow->packet_count = 0;
- flow->byte_count = 0;
-}
-
static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
{
struct ovs_header *ovs_header = info->userhdr;
@@ -634,11 +626,9 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow, struct datapath *dp,
{
const int skb_orig_len = skb->len;
struct nlattr *start;
- struct ovs_flow_stats stats;
+ struct sw_flow_stats flow_stats;
struct ovs_header *ovs_header;
struct nlattr *nla;
- unsigned long used;
- u8 tcp_flags;
int err;
ovs_header = genlmsg_put(skb, portid, seq, &dp_flow_genl_family, flags, cmd);
@@ -667,24 +657,24 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow, struct datapath *dp,
nla_nest_end(skb, nla);
- spin_lock_bh(&flow->lock);
- used = flow->used;
- stats.n_packets = flow->packet_count;
- stats.n_bytes = flow->byte_count;
- tcp_flags = flow->tcp_flags;
- spin_unlock_bh(&flow->lock);
-
- if (used &&
- nla_put_u64(skb, OVS_FLOW_ATTR_USED, ovs_flow_used_time(used)))
+ ovs_flow_stats_get(flow, &flow_stats);
+ if (flow_stats.used &&
+ nla_put_u64(skb, OVS_FLOW_ATTR_USED, ovs_flow_used_time(flow_stats.used)))
goto nla_put_failure;
- if (stats.n_packets &&
- nla_put(skb, OVS_FLOW_ATTR_STATS,
- sizeof(struct ovs_flow_stats), &stats))
- goto nla_put_failure;
+ if (flow_stats.packet_count) {
+ struct ovs_flow_stats stats = {
+ .n_packets = flow_stats.packet_count,
+ .n_bytes = flow_stats.byte_count,
+ };
- if (tcp_flags &&
- nla_put_u8(skb, OVS_FLOW_ATTR_TCP_FLAGS, tcp_flags))
+ if (nla_put(skb, OVS_FLOW_ATTR_STATS,
+ sizeof(struct ovs_flow_stats), &stats))
+ goto nla_put_failure;
+ }
+
+ if (flow_stats.tcp_flags &&
+ nla_put_u8(skb, OVS_FLOW_ATTR_TCP_FLAGS, flow_stats.tcp_flags))
goto nla_put_failure;
/* If OVS_FLOW_ATTR_ACTIONS doesn't fit, skip dumping the actions if
@@ -822,7 +812,6 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
error = PTR_ERR(flow);
goto err_unlock_ovs;
}
- clear_stats(flow);
flow->key = masked_key;
flow->unmasked_key = key;
@@ -868,11 +857,8 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
info->snd_seq, OVS_FLOW_CMD_NEW);
/* Clear stats. */
- if (a[OVS_FLOW_ATTR_CLEAR]) {
- spin_lock_bh(&flow->lock);
- clear_stats(flow);
- spin_unlock_bh(&flow->lock);
- }
+ if (a[OVS_FLOW_ATTR_CLEAR])
+ ovs_flow_stats_clear(flow);
}
ovs_unlock();
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 617810f..2d82995 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -35,6 +35,7 @@
#include <linux/ip.h>
#include <linux/ipv6.h>
#include <linux/sctp.h>
+#include <linux/smp.h>
#include <linux/tcp.h>
#include <linux/udp.h>
#include <linux/icmp.h>
@@ -61,8 +62,9 @@ u64 ovs_flow_used_time(unsigned long flow_jiffies)
#define TCP_FLAGS_OFFSET 13
#define TCP_FLAG_MASK 0x3f
-void ovs_flow_used(struct sw_flow *flow, struct sk_buff *skb)
+void ovs_flow_stats_update(struct sw_flow *flow, struct sk_buff *skb)
{
+ struct sw_flow_stats *stats = &flow->stats[smp_processor_id()];
u8 tcp_flags = 0;
if ((flow->key.eth.type == htons(ETH_P_IP) ||
@@ -73,12 +75,64 @@ void ovs_flow_used(struct sw_flow *flow, struct sk_buff *skb)
tcp_flags = *(tcp + TCP_FLAGS_OFFSET) & TCP_FLAG_MASK;
}
- spin_lock(&flow->lock);
- flow->used = jiffies;
- flow->packet_count++;
- flow->byte_count += skb->len;
- flow->tcp_flags |= tcp_flags;
- spin_unlock(&flow->lock);
+ spin_lock(&stats->lock);
+ stats->used = jiffies;
+ stats->packet_count++;
+ stats->byte_count += skb->len;
+ stats->tcp_flags |= tcp_flags;
+ spin_unlock(&stats->lock);
+}
+
+void ovs_flow_stats_get(struct sw_flow *flow, struct sw_flow_stats *res)
+{
+ int cpu, cur_cpu;
+
+ memset(res, 0, sizeof(*res));
+
+ cur_cpu = get_cpu();
+ for_each_possible_cpu(cpu) {
+ struct sw_flow_stats *stats = &flow->stats[cpu];
+
+ if (cpu == cur_cpu)
+ local_bh_disable();
+
+ spin_lock(&stats->lock);
+ if (time_after(stats->used, res->used))
+ res->used = stats->used;
+ res->packet_count += stats->packet_count;
+ res->byte_count += stats->byte_count;
+ res->tcp_flags |= stats->tcp_flags;
+ spin_unlock(&stats->lock);
+
+ if (cpu == cur_cpu)
+ local_bh_enable();
+
+ }
+ put_cpu();
+}
+
+void ovs_flow_stats_clear(struct sw_flow *flow)
+{
+ int cpu, cur_cpu;
+
+ cur_cpu = get_cpu();
+ for_each_possible_cpu(cpu) {
+ struct sw_flow_stats *stats = &flow->stats[cpu];
+
+ if (cpu == cur_cpu)
+ local_bh_disable();
+
+ spin_lock(&stats->lock);
+ stats->used = 0;
+ stats->packet_count = 0;
+ stats->byte_count = 0;
+ stats->tcp_flags = 0;
+ spin_unlock(&stats->lock);
+
+ if (cpu == cur_cpu)
+ local_bh_enable();
+ }
+ put_cpu();
}
static int check_header(struct sk_buff *skb, int len)
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index 098fd1d..b844252 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -19,6 +19,7 @@
#ifndef FLOW_H
#define FLOW_H 1
+#include <linux/cache.h>
#include <linux/kernel.h>
#include <linux/netlink.h>
#include <linux/openvswitch.h>
@@ -144,6 +145,14 @@ struct sw_flow_actions {
struct nlattr actions[];
};
+struct sw_flow_stats {
+ u64 packet_count; /* Number of packets matched. */
+ u64 byte_count; /* Number of bytes matched. */
+ unsigned long used; /* Last used time (in jiffies). */
+ spinlock_t lock; /* Lock for atomic stats update. */
+ u8 tcp_flags; /* Union of seen TCP flags. */
+} ____cacheline_aligned_in_smp;
+
struct sw_flow {
struct rcu_head rcu;
struct hlist_node hash_node[2];
@@ -153,12 +162,7 @@ struct sw_flow {
struct sw_flow_key unmasked_key;
struct sw_flow_mask *mask;
struct sw_flow_actions __rcu *sf_acts;
-
- spinlock_t lock; /* Lock for values below. */
- unsigned long used; /* Last used time (in jiffies). */
- u64 packet_count; /* Number of packets matched. */
- u64 byte_count; /* Number of bytes matched. */
- u8 tcp_flags; /* Union of seen TCP flags. */
+ struct sw_flow_stats stats[];
};
struct arp_eth_header {
@@ -175,7 +179,9 @@ struct arp_eth_header {
unsigned char ar_tip[4]; /* target IP address */
} __packed;
-void ovs_flow_used(struct sw_flow *, struct sk_buff *);
+void ovs_flow_stats_update(struct sw_flow *flow, struct sk_buff *skb);
+void ovs_flow_stats_get(struct sw_flow *flow, struct sw_flow_stats *res);
+void ovs_flow_stats_clear(struct sw_flow *flow);
u64 ovs_flow_used_time(unsigned long flow_jiffies);
int ovs_flow_extract(struct sk_buff *, u16 in_port, struct sw_flow_key *);
diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
index 536b4d2..feb1b9b 100644
--- a/net/openvswitch/flow_table.c
+++ b/net/openvswitch/flow_table.c
@@ -75,15 +75,19 @@ void ovs_flow_mask_key(struct sw_flow_key *dst, const struct sw_flow_key *src,
struct sw_flow *ovs_flow_alloc(void)
{
struct sw_flow *flow;
+ int cpu;
flow = kmem_cache_alloc(flow_cache, GFP_KERNEL);
if (!flow)
return ERR_PTR(-ENOMEM);
- spin_lock_init(&flow->lock);
flow->sf_acts = NULL;
flow->mask = NULL;
+ memset(flow->stats, 0, num_possible_cpus() * sizeof(struct sw_flow_stats));
+ for_each_possible_cpu(cpu)
+ spin_lock_init(&flow->stats[cpu].lock);
+
return flow;
}
@@ -574,11 +578,15 @@ int ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow,
* Returns zero if successful or a negative error code. */
int ovs_flow_init(void)
{
+ int flow_size;
+
BUILD_BUG_ON(__alignof__(struct sw_flow_key) % __alignof__(long));
BUILD_BUG_ON(sizeof(struct sw_flow_key) % sizeof(long));
- flow_cache = kmem_cache_create("sw_flow", sizeof(struct sw_flow), 0,
- 0, NULL);
+ flow_size = sizeof(struct sw_flow) +
+ (num_possible_cpus() * sizeof(struct sw_flow_stats));
+
+ flow_cache = kmem_cache_create("sw_flow", flow_size, 0, 0, NULL);
if (flow_cache == NULL)
return -ENOMEM;
--
1.8.3.2
^ permalink raw reply related
* [PATCH net-next 09/11] openvswitch: Enable all GSO features on internal port.
From: Jesse Gross @ 2013-10-30 0:22 UTC (permalink / raw)
To: David Miller; +Cc: netdev, dev, Pravin B Shelar, Jesse Gross
In-Reply-To: <1383092544-50599-1-git-send-email-jesse@nicira.com>
From: Pravin B Shelar <pshelar@nicira.com>
OVS already can handle all types of segmentation offloads that
are supported by the kernel.
Following patch specifically enables UDP and IPV6 segmentation
offloads.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
---
net/openvswitch/vport-internal_dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
index 98d3edb..729c687 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c
@@ -134,7 +134,7 @@ 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_TSO;
+ NETIF_F_HIGHDMA | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE;
netdev->vlan_features = netdev->features;
netdev->features |= NETIF_F_HW_VLAN_CTAG_TX;
--
1.8.3.2
^ permalink raw reply related
* [PATCH net-next 10/11] openvswitch: Widen TCP flags handling.
From: Jesse Gross @ 2013-10-30 0:22 UTC (permalink / raw)
To: David Miller; +Cc: netdev, dev, Jarno Rajahalme, Jesse Gross
In-Reply-To: <1383092544-50599-1-git-send-email-jesse@nicira.com>
From: Jarno Rajahalme <jrajahalme@nicira.com>
Widen TCP flags handling from 7 bits (uint8_t) to 12 bits (uint16_t).
The kernel interface remains at 8 bits, which makes no functional
difference now, as none of the higher bits is currently of interest
to the userspace.
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
---
net/openvswitch/datapath.c | 5 +++--
net/openvswitch/flow.c | 8 +++-----
net/openvswitch/flow.h | 2 +-
3 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 8c8875b..376b2ad 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -673,8 +673,9 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow, struct datapath *dp,
goto nla_put_failure;
}
- if (flow_stats.tcp_flags &&
- nla_put_u8(skb, OVS_FLOW_ATTR_TCP_FLAGS, flow_stats.tcp_flags))
+ if ((u8)ntohs(flow_stats.tcp_flags) &&
+ nla_put_u8(skb, OVS_FLOW_ATTR_TCP_FLAGS,
+ (u8)ntohs(flow_stats.tcp_flags)))
goto nla_put_failure;
/* If OVS_FLOW_ATTR_ACTIONS doesn't fit, skip dumping the actions if
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 2d82995..6ee7a41 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -59,20 +59,18 @@ u64 ovs_flow_used_time(unsigned long flow_jiffies)
return cur_ms - idle_ms;
}
-#define TCP_FLAGS_OFFSET 13
-#define TCP_FLAG_MASK 0x3f
+#define TCP_FLAGS_BE16(tp) (*(__be16 *)&tcp_flag_word(tp) & htons(0x0FFF))
void ovs_flow_stats_update(struct sw_flow *flow, struct sk_buff *skb)
{
struct sw_flow_stats *stats = &flow->stats[smp_processor_id()];
- u8 tcp_flags = 0;
+ __be16 tcp_flags = 0;
if ((flow->key.eth.type == htons(ETH_P_IP) ||
flow->key.eth.type == htons(ETH_P_IPV6)) &&
flow->key.ip.proto == IPPROTO_TCP &&
likely(skb->len >= skb_transport_offset(skb) + sizeof(struct tcphdr))) {
- u8 *tcp = (u8 *)tcp_hdr(skb);
- tcp_flags = *(tcp + TCP_FLAGS_OFFSET) & TCP_FLAG_MASK;
+ tcp_flags = TCP_FLAGS_BE16(tcp_hdr(skb));
}
spin_lock(&stats->lock);
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index b844252..a5504de 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -150,7 +150,7 @@ struct sw_flow_stats {
u64 byte_count; /* Number of bytes matched. */
unsigned long used; /* Last used time (in jiffies). */
spinlock_t lock; /* Lock for atomic stats update. */
- u8 tcp_flags; /* Union of seen TCP flags. */
+ __be16 tcp_flags; /* Union of seen TCP flags. */
} ____cacheline_aligned_in_smp;
struct sw_flow {
--
1.8.3.2
^ permalink raw reply related
* [PATCH net-next 07/11] openvswitch: collect mega flow mask stats
From: Jesse Gross @ 2013-10-30 0:22 UTC (permalink / raw)
To: David Miller; +Cc: netdev, dev, Andy Zhou, Jesse Gross
In-Reply-To: <1383092544-50599-1-git-send-email-jesse@nicira.com>
From: Andy Zhou <azhou@nicira.com>
Collect mega flow mask stats. ovs-dpctl show command can be used to
display them for debugging and performance tuning.
Signed-off-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
---
include/uapi/linux/openvswitch.h | 17 ++++++++++++++---
net/openvswitch/datapath.c | 38 +++++++++++++++++++++++++++++++-------
net/openvswitch/datapath.h | 4 ++++
net/openvswitch/flow_table.c | 16 +++++++++++++++-
net/openvswitch/flow_table.h | 4 +++-
5 files changed, 67 insertions(+), 12 deletions(-)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index a74d375..2cc4644 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -63,15 +63,18 @@ enum ovs_datapath_cmd {
* not be sent.
* @OVS_DP_ATTR_STATS: Statistics about packets that have passed through the
* datapath. Always present in notifications.
+ * @OVS_DP_ATTR_MEGAFLOW_STATS: Statistics about mega flow masks usage for the
+ * datapath. Always present in notifications.
*
* These attributes follow the &struct ovs_header within the Generic Netlink
* payload for %OVS_DP_* commands.
*/
enum ovs_datapath_attr {
OVS_DP_ATTR_UNSPEC,
- OVS_DP_ATTR_NAME, /* name of dp_ifindex netdev */
- OVS_DP_ATTR_UPCALL_PID, /* Netlink PID to receive upcalls */
- OVS_DP_ATTR_STATS, /* struct ovs_dp_stats */
+ OVS_DP_ATTR_NAME, /* name of dp_ifindex netdev */
+ OVS_DP_ATTR_UPCALL_PID, /* Netlink PID to receive upcalls */
+ OVS_DP_ATTR_STATS, /* struct ovs_dp_stats */
+ OVS_DP_ATTR_MEGAFLOW_STATS, /* struct ovs_dp_megaflow_stats */
__OVS_DP_ATTR_MAX
};
@@ -84,6 +87,14 @@ struct ovs_dp_stats {
__u64 n_flows; /* Number of flows present */
};
+struct ovs_dp_megaflow_stats {
+ __u64 n_mask_hit; /* Number of masks used for flow lookups. */
+ __u32 n_masks; /* Number of masks for the datapath. */
+ __u32 pad0; /* Pad for future expension. */
+ __u64 pad1; /* Pad for future expension. */
+ __u64 pad2; /* Pad for future expension. */
+};
+
struct ovs_vport_stats {
__u64 rx_packets; /* total packets received */
__u64 tx_packets; /* total packets transmitted */
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index cf27097..5bc5a4e 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -221,6 +221,7 @@ void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb)
struct dp_stats_percpu *stats;
struct sw_flow_key key;
u64 *stats_counter;
+ u32 n_mask_hit;
int error;
stats = this_cpu_ptr(dp->stats_percpu);
@@ -233,7 +234,7 @@ void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb)
}
/* Look up flow. */
- flow = ovs_flow_tbl_lookup(&dp->table, &key);
+ flow = ovs_flow_tbl_lookup(&dp->table, &key, &n_mask_hit);
if (unlikely(!flow)) {
struct dp_upcall_info upcall;
@@ -258,6 +259,7 @@ out:
/* Update datapath statistics. */
u64_stats_update_begin(&stats->sync);
(*stats_counter)++;
+ stats->n_mask_hit += n_mask_hit;
u64_stats_update_end(&stats->sync);
}
@@ -563,13 +565,18 @@ static struct genl_ops dp_packet_genl_ops[] = {
}
};
-static void get_dp_stats(struct datapath *dp, struct ovs_dp_stats *stats)
+static void get_dp_stats(struct datapath *dp, struct ovs_dp_stats *stats,
+ struct ovs_dp_megaflow_stats *mega_stats)
{
int i;
+ memset(mega_stats, 0, sizeof(*mega_stats));
+
stats->n_flows = ovs_flow_tbl_count(&dp->table);
+ mega_stats->n_masks = ovs_flow_tbl_num_masks(&dp->table);
stats->n_hit = stats->n_missed = stats->n_lost = 0;
+
for_each_possible_cpu(i) {
const struct dp_stats_percpu *percpu_stats;
struct dp_stats_percpu local_stats;
@@ -585,6 +592,7 @@ static void get_dp_stats(struct datapath *dp, struct ovs_dp_stats *stats)
stats->n_hit += local_stats.n_hit;
stats->n_missed += local_stats.n_missed;
stats->n_lost += local_stats.n_lost;
+ mega_stats->n_mask_hit += local_stats.n_mask_hit;
}
}
@@ -743,6 +751,14 @@ static struct sk_buff *ovs_flow_cmd_build_info(struct sw_flow *flow,
return skb;
}
+static struct sw_flow *__ovs_flow_tbl_lookup(struct flow_table *tbl,
+ const struct sw_flow_key *key)
+{
+ u32 __always_unused n_mask_hit;
+
+ return ovs_flow_tbl_lookup(tbl, key, &n_mask_hit);
+}
+
static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
{
struct nlattr **a = info->attrs;
@@ -793,7 +809,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
goto err_unlock_ovs;
/* Check if this is a duplicate flow */
- flow = ovs_flow_tbl_lookup(&dp->table, &key);
+ flow = __ovs_flow_tbl_lookup(&dp->table, &key);
if (!flow) {
/* Bail out if we're not allowed to create a new flow. */
error = -ENOENT;
@@ -905,7 +921,7 @@ static int ovs_flow_cmd_get(struct sk_buff *skb, struct genl_info *info)
goto unlock;
}
- flow = ovs_flow_tbl_lookup(&dp->table, &key);
+ flow = __ovs_flow_tbl_lookup(&dp->table, &key);
if (!flow || !ovs_flow_cmp_unmasked_key(flow, &match)) {
err = -ENOENT;
goto unlock;
@@ -953,7 +969,7 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct genl_info *info)
if (err)
goto unlock;
- flow = ovs_flow_tbl_lookup(&dp->table, &key);
+ flow = __ovs_flow_tbl_lookup(&dp->table, &key);
if (!flow || !ovs_flow_cmp_unmasked_key(flow, &match)) {
err = -ENOENT;
goto unlock;
@@ -1067,6 +1083,7 @@ static size_t ovs_dp_cmd_msg_size(void)
msgsize += nla_total_size(IFNAMSIZ);
msgsize += nla_total_size(sizeof(struct ovs_dp_stats));
+ msgsize += nla_total_size(sizeof(struct ovs_dp_megaflow_stats));
return msgsize;
}
@@ -1076,6 +1093,7 @@ static int ovs_dp_cmd_fill_info(struct datapath *dp, struct sk_buff *skb,
{
struct ovs_header *ovs_header;
struct ovs_dp_stats dp_stats;
+ struct ovs_dp_megaflow_stats dp_megaflow_stats;
int err;
ovs_header = genlmsg_put(skb, portid, seq, &dp_datapath_genl_family,
@@ -1091,8 +1109,14 @@ static int ovs_dp_cmd_fill_info(struct datapath *dp, struct sk_buff *skb,
if (err)
goto nla_put_failure;
- get_dp_stats(dp, &dp_stats);
- if (nla_put(skb, OVS_DP_ATTR_STATS, sizeof(struct ovs_dp_stats), &dp_stats))
+ get_dp_stats(dp, &dp_stats, &dp_megaflow_stats);
+ if (nla_put(skb, OVS_DP_ATTR_STATS, sizeof(struct ovs_dp_stats),
+ &dp_stats))
+ goto nla_put_failure;
+
+ if (nla_put(skb, OVS_DP_ATTR_MEGAFLOW_STATS,
+ sizeof(struct ovs_dp_megaflow_stats),
+ &dp_megaflow_stats))
goto nla_put_failure;
return genlmsg_end(skb, ovs_header);
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index acfd4af..d3d14a58 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -46,11 +46,15 @@
* @n_lost: Number of received packets that had no matching flow in the flow
* table that could not be sent to userspace (normally due to an overflow in
* one of the datapath's queues).
+ * @n_mask_hit: Number of masks looked up for flow match.
+ * @n_mask_hit / (@n_hit + @n_missed) will be the average masks looked
+ * up per packet.
*/
struct dp_stats_percpu {
u64 n_hit;
u64 n_missed;
u64 n_lost;
+ u64 n_mask_hit;
struct u64_stats_sync sync;
};
diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
index 036e019..536b4d2 100644
--- a/net/openvswitch/flow_table.c
+++ b/net/openvswitch/flow_table.c
@@ -430,13 +430,16 @@ static struct sw_flow *masked_flow_lookup(struct table_instance *ti,
}
struct sw_flow *ovs_flow_tbl_lookup(struct flow_table *tbl,
- const struct sw_flow_key *key)
+ const struct sw_flow_key *key,
+ u32 *n_mask_hit)
{
struct table_instance *ti = rcu_dereference(tbl->ti);
struct sw_flow_mask *mask;
struct sw_flow *flow;
+ *n_mask_hit = 0;
list_for_each_entry_rcu(mask, &tbl->mask_list, list) {
+ (*n_mask_hit)++;
flow = masked_flow_lookup(ti, key, mask);
if (flow) /* Found */
return flow;
@@ -444,6 +447,17 @@ struct sw_flow *ovs_flow_tbl_lookup(struct flow_table *tbl,
return NULL;
}
+int ovs_flow_tbl_num_masks(const struct flow_table *table)
+{
+ struct sw_flow_mask *mask;
+ int num = 0;
+
+ list_for_each_entry(mask, &table->mask_list, list)
+ num++;
+
+ return num;
+}
+
static struct table_instance *table_instance_expand(struct table_instance *ti)
{
return table_instance_rehash(ti, ti->n_buckets * 2);
diff --git a/net/openvswitch/flow_table.h b/net/openvswitch/flow_table.h
index 4db5f78..fbe45d5 100644
--- a/net/openvswitch/flow_table.h
+++ b/net/openvswitch/flow_table.h
@@ -66,10 +66,12 @@ int ovs_flow_tbl_flush(struct flow_table *flow_table);
int ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow,
struct sw_flow_mask *mask);
void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow);
+int ovs_flow_tbl_num_masks(const struct flow_table *table);
struct sw_flow *ovs_flow_tbl_dump_next(struct table_instance *table,
u32 *bucket, u32 *idx);
struct sw_flow *ovs_flow_tbl_lookup(struct flow_table *,
- const struct sw_flow_key *);
+ const struct sw_flow_key *,
+ u32 *n_mask_hit);
bool ovs_flow_cmp_unmasked_key(const struct sw_flow *flow,
struct sw_flow_match *match);
--
1.8.3.2
^ permalink raw reply related
* [PATCH net-next 11/11] openvswitch: TCP flags matching support.
From: Jesse Gross @ 2013-10-30 0:22 UTC (permalink / raw)
To: David Miller; +Cc: netdev, dev, Jarno Rajahalme, Jesse Gross
In-Reply-To: <1383092544-50599-1-git-send-email-jesse@nicira.com>
From: Jarno Rajahalme <jrajahalme@nicira.com>
tcp_flags=flags/mask
Bitwise match on TCP flags. The flags and mask are 16-bit num‐
bers written in decimal or in hexadecimal prefixed by 0x. Each
1-bit in mask requires that the corresponding bit in port must
match. Each 0-bit in mask causes the corresponding bit to be
ignored.
TCP protocol currently defines 9 flag bits, and additional 3
bits are reserved (must be transmitted as zero), see RFCs 793,
3168, and 3540. The flag bits are, numbering from the least
significant bit:
0: FIN No more data from sender.
1: SYN Synchronize sequence numbers.
2: RST Reset the connection.
3: PSH Push function.
4: ACK Acknowledgement field significant.
5: URG Urgent pointer field significant.
6: ECE ECN Echo.
7: CWR Congestion Windows Reduced.
8: NS Nonce Sum.
9-11: Reserved.
12-15: Not matchable, must be zero.
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
---
include/uapi/linux/openvswitch.h | 1 +
net/openvswitch/flow.c | 2 ++
net/openvswitch/flow.h | 2 ++
net/openvswitch/flow_netlink.c | 31 +++++++++++++++++++++++++++++--
4 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 2cc4644..d120f9f 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -271,6 +271,7 @@ enum ovs_key_attr {
OVS_KEY_ATTR_SKB_MARK, /* u32 skb mark */
OVS_KEY_ATTR_TUNNEL, /* Nested set of ovs_tunnel attributes */
OVS_KEY_ATTR_SCTP, /* struct ovs_key_sctp */
+ OVS_KEY_ATTR_TCP_FLAGS, /* be16 TCP flags. */
#ifdef __KERNEL__
OVS_KEY_ATTR_IPV4_TUNNEL, /* struct ovs_key_ipv4_tunnel */
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 6ee7a41..863418f 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -482,6 +482,7 @@ int ovs_flow_extract(struct sk_buff *skb, u16 in_port, struct sw_flow_key *key)
struct tcphdr *tcp = tcp_hdr(skb);
key->ipv4.tp.src = tcp->source;
key->ipv4.tp.dst = tcp->dest;
+ key->ipv4.tp.flags = TCP_FLAGS_BE16(tcp);
}
} else if (key->ip.proto == IPPROTO_UDP) {
if (udphdr_ok(skb)) {
@@ -550,6 +551,7 @@ int ovs_flow_extract(struct sk_buff *skb, u16 in_port, struct sw_flow_key *key)
struct tcphdr *tcp = tcp_hdr(skb);
key->ipv6.tp.src = tcp->source;
key->ipv6.tp.dst = tcp->dest;
+ key->ipv6.tp.flags = TCP_FLAGS_BE16(tcp);
}
} else if (key->ip.proto == NEXTHDR_UDP) {
if (udphdr_ok(skb)) {
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index a5504de..d2d12ad 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -94,6 +94,7 @@ struct sw_flow_key {
struct {
__be16 src; /* TCP/UDP/SCTP source port. */
__be16 dst; /* TCP/UDP/SCTP destination port. */
+ __be16 flags; /* TCP flags. */
} tp;
struct {
u8 sha[ETH_ALEN]; /* ARP source hardware address. */
@@ -110,6 +111,7 @@ struct sw_flow_key {
struct {
__be16 src; /* TCP/UDP/SCTP source port. */
__be16 dst; /* TCP/UDP/SCTP destination port. */
+ __be16 flags; /* TCP flags. */
} tp;
struct {
struct in6_addr target; /* ND target address. */
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index e04649c..2bc1bc1 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -114,6 +114,7 @@ static bool match_validate(const struct sw_flow_match *match,
mask_allowed &= ~((1 << OVS_KEY_ATTR_IPV4)
| (1 << OVS_KEY_ATTR_IPV6)
| (1 << OVS_KEY_ATTR_TCP)
+ | (1 << OVS_KEY_ATTR_TCP_FLAGS)
| (1 << OVS_KEY_ATTR_UDP)
| (1 << OVS_KEY_ATTR_SCTP)
| (1 << OVS_KEY_ATTR_ICMP)
@@ -154,8 +155,11 @@ static bool match_validate(const struct sw_flow_match *match,
if (match->key->ip.proto == IPPROTO_TCP) {
key_expected |= 1 << OVS_KEY_ATTR_TCP;
- if (match->mask && (match->mask->key.ip.proto == 0xff))
+ key_expected |= 1 << OVS_KEY_ATTR_TCP_FLAGS;
+ if (match->mask && (match->mask->key.ip.proto == 0xff)) {
mask_allowed |= 1 << OVS_KEY_ATTR_TCP;
+ mask_allowed |= 1 << OVS_KEY_ATTR_TCP_FLAGS;
+ }
}
if (match->key->ip.proto == IPPROTO_ICMP) {
@@ -186,8 +190,11 @@ static bool match_validate(const struct sw_flow_match *match,
if (match->key->ip.proto == IPPROTO_TCP) {
key_expected |= 1 << OVS_KEY_ATTR_TCP;
- if (match->mask && (match->mask->key.ip.proto == 0xff))
+ key_expected |= 1 << OVS_KEY_ATTR_TCP_FLAGS;
+ if (match->mask && (match->mask->key.ip.proto == 0xff)) {
mask_allowed |= 1 << OVS_KEY_ATTR_TCP;
+ mask_allowed |= 1 << OVS_KEY_ATTR_TCP_FLAGS;
+ }
}
if (match->key->ip.proto == IPPROTO_ICMPV6) {
@@ -235,6 +242,7 @@ static const int ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = {
[OVS_KEY_ATTR_IPV4] = sizeof(struct ovs_key_ipv4),
[OVS_KEY_ATTR_IPV6] = sizeof(struct ovs_key_ipv6),
[OVS_KEY_ATTR_TCP] = sizeof(struct ovs_key_tcp),
+ [OVS_KEY_ATTR_TCP_FLAGS] = sizeof(__be16),
[OVS_KEY_ATTR_UDP] = sizeof(struct ovs_key_udp),
[OVS_KEY_ATTR_SCTP] = sizeof(struct ovs_key_sctp),
[OVS_KEY_ATTR_ICMP] = sizeof(struct ovs_key_icmp),
@@ -634,6 +642,19 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
attrs &= ~(1 << OVS_KEY_ATTR_TCP);
}
+ if (attrs & (1 << OVS_KEY_ATTR_TCP_FLAGS)) {
+ if (orig_attrs & (1 << OVS_KEY_ATTR_IPV4)) {
+ SW_FLOW_KEY_PUT(match, ipv4.tp.flags,
+ nla_get_be16(a[OVS_KEY_ATTR_TCP_FLAGS]),
+ is_mask);
+ } else {
+ SW_FLOW_KEY_PUT(match, ipv6.tp.flags,
+ nla_get_be16(a[OVS_KEY_ATTR_TCP_FLAGS]),
+ is_mask);
+ }
+ attrs &= ~(1 << OVS_KEY_ATTR_TCP_FLAGS);
+ }
+
if (attrs & (1 << OVS_KEY_ATTR_UDP)) {
const struct ovs_key_udp *udp_key;
@@ -1004,9 +1025,15 @@ int ovs_nla_put_flow(const struct sw_flow_key *swkey,
if (swkey->eth.type == htons(ETH_P_IP)) {
tcp_key->tcp_src = output->ipv4.tp.src;
tcp_key->tcp_dst = output->ipv4.tp.dst;
+ if (nla_put_be16(skb, OVS_KEY_ATTR_TCP_FLAGS,
+ output->ipv4.tp.flags))
+ goto nla_put_failure;
} else if (swkey->eth.type == htons(ETH_P_IPV6)) {
tcp_key->tcp_src = output->ipv6.tp.src;
tcp_key->tcp_dst = output->ipv6.tp.dst;
+ if (nla_put_be16(skb, OVS_KEY_ATTR_TCP_FLAGS,
+ output->ipv6.tp.flags))
+ goto nla_put_failure;
}
} else if (swkey->ip.proto == IPPROTO_UDP) {
struct ovs_key_udp *udp_key;
--
1.8.3.2
^ permalink raw reply related
* [PATCH net-next RFC 0/5] xen-netback: TX grant mapping instead of copy
From: Zoltan Kiss @ 2013-10-30 0:50 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
A long known problem of the upstream netback implementation that on the TX
path (from guest to Dom0) it copies the whole packet from guest memory into
Dom0. That simply became a bottleneck with 10Gb NICs, and generally it's a
huge perfomance penalty. The classic kernel version of netback used grant
mapping, and to get notified when the page can be unmapped, it used page
destructors. Unfortunately that destructor is not an upstreamable solution.
Ian Campbell's skb fragment destructor patch series
(http://lwn.net/Articles/491522/) tried to solve this problem, however it
seems to be very invasive on the network stack's code, and therefore haven't
progressed very well.
This patch series use SKBTX_DEV_ZEROCOPY flags to tell the stack it needs to
know when the skb is freed up. That is the way KVM solved the same problem,
and based on my initial tests it can do the same for us. Avoiding the extra
copy boosted up TX throughput from 6.8 Gbps to 7.9 (I used a slower
Interlagos box, both Dom0 and guest on upstream kernel, on the same NUMA node,
running iperf 2.0.5, and the remote end was a bare metal box on the same 10Gb
switch)
Based on my investigations the packet get only copied if it is delivered to
Dom0 stack, which is due to this patch:
https://lkml.org/lkml/2012/7/20/363
That's a bit unfortunate, but as far as I know for the huge majority this use
case is not too important. There are a couple of things which need more
polishing, see the FIXME comments. I will run some more extensive tests, but
in the meantime I would like to hear comments about what I've done so far.
I've tried to broke it down to smaller patches, with mixed results, so I
welcome suggestions on that part as well:
1/5: Introduce TX grant map definitions
2/5: Change TX path from grant copy to mapping
3/5: Remove old TX grant copy definitons
4/5: Fix indentations
5/5: Change RX path for mapped SKB fragments
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
^ permalink raw reply
* [PATCH net-next RFC 1/5] xen-netback: Introduce TX grant map definitions
From: Zoltan Kiss @ 2013-10-30 0:50 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1383094220-14775-1-git-send-email-zoltan.kiss@citrix.com>
This patch contains the new definitions necessary for grant mapping.
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/common.h | 22 ++++++
drivers/net/xen-netback/interface.c | 1 +
drivers/net/xen-netback/netback.c | 134 +++++++++++++++++++++++++++++++++++
3 files changed, 157 insertions(+)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 55b8dec..36a3fda 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -79,6 +79,11 @@ struct pending_tx_info {
* if it is head of one or more tx
* reqs
*/
+ /* callback data for released SKBs. The callback is always
+ * xenvif_zerocopy_callback, ctx points to the next fragment, desc
+ * contains the pending_idx
+ */
+ struct ubuf_info callback_struct;
};
#define XEN_NETIF_TX_RING_SIZE __CONST_RING_SIZE(xen_netif_tx, PAGE_SIZE)
@@ -101,6 +106,8 @@ struct xenvif_rx_meta {
#define MAX_PENDING_REQS 256
+#define NETBACK_INVALID_HANDLE -1
+
struct xenvif {
/* Unique identifier for this interface. */
domid_t domid;
@@ -119,13 +126,22 @@ struct xenvif {
pending_ring_idx_t pending_cons;
u16 pending_ring[MAX_PENDING_REQS];
struct pending_tx_info pending_tx_info[MAX_PENDING_REQS];
+ grant_handle_t grant_tx_handle[MAX_PENDING_REQS];
/* Coalescing tx requests before copying makes number of grant
* copy ops greater or equal to number of slots required. In
* worst case a tx request consumes 2 gnttab_copy.
*/
struct gnttab_copy tx_copy_ops[2*MAX_PENDING_REQS];
+ struct gnttab_unmap_grant_ref tx_unmap_ops[MAX_PENDING_REQS];
+ struct gnttab_map_grant_ref tx_map_ops[MAX_PENDING_REQS];
+ /* passed to gnttab_[un]map_refs with pages under (un)mapping */
+ struct page *pages_to_gnt[MAX_PENDING_REQS];
+ spinlock_t dealloc_lock;
+ pending_ring_idx_t dealloc_prod;
+ pending_ring_idx_t dealloc_cons;
+ u16 dealloc_ring[MAX_PENDING_REQS];
/* Use kthread for guest RX */
struct task_struct *task;
@@ -226,6 +242,12 @@ void xenvif_rx_action(struct xenvif *vif);
int xenvif_kthread(void *data);
+/* Callback from stack when TX packet can be released */
+void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success);
+
+/* Unmap a pending page, usually has to be called before xenvif_idx_release */
+void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx);
+
extern bool separate_tx_rx_irq;
#endif /* __XEN_NETBACK__COMMON_H__ */
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index e4aa267..f5c3c57 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -37,6 +37,7 @@
#include <xen/events.h>
#include <asm/xen/hypercall.h>
+#include <xen/balloon.h>
#define XENVIF_QUEUE_LENGTH 32
#define XENVIF_NAPI_WEIGHT 64
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 828fdab..10470dc 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -869,6 +869,20 @@ static struct page *xenvif_alloc_page(struct xenvif *vif,
return page;
}
+static inline void xenvif_tx_create_gop(struct xenvif *vif, u16 pending_idx,
+ struct xen_netif_tx_request *txp,
+ struct gnttab_map_grant_ref *gop)
+{
+ vif->pages_to_gnt[gop-vif->tx_map_ops] = vif->mmap_pages[pending_idx];
+ gnttab_set_map_op(gop, idx_to_kaddr(vif, pending_idx),
+ GNTMAP_host_map | GNTMAP_readonly,
+ txp->gref, vif->domid);
+
+ memcpy(&vif->pending_tx_info[pending_idx].req, txp,
+ sizeof(*txp));
+
+}
+
static struct gnttab_copy *xenvif_get_requests(struct xenvif *vif,
struct sk_buff *skb,
struct xen_netif_tx_request *txp,
@@ -1652,6 +1666,106 @@ static int xenvif_tx_submit(struct xenvif *vif, int budget)
return work_done;
}
+void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success)
+{
+ unsigned long flags;
+ pending_ring_idx_t index;
+ u16 pending_idx = ubuf->desc;
+ struct pending_tx_info *temp =
+ container_of(ubuf, struct pending_tx_info, callback_struct);
+ struct xenvif *vif =
+ container_of(temp - pending_idx, struct xenvif,
+ pending_tx_info[0]);
+
+ spin_lock_irqsave(&vif->dealloc_lock, flags);
+ do {
+ pending_idx = ubuf->desc;
+ ubuf = (struct ubuf_info *) ubuf->ctx;
+ index = pending_index(vif->dealloc_prod);
+ vif->dealloc_ring[index] = pending_idx;
+ /* Sync with xenvif_tx_action_dealloc:
+ * insert idx then incr producer.
+ */
+ smp_wmb();
+ vif->dealloc_prod++;
+ napi_schedule(&vif->napi);
+ } while (ubuf);
+ spin_unlock_irqrestore(&vif->dealloc_lock, flags);
+}
+
+static inline void xenvif_tx_action_dealloc(struct xenvif *vif)
+{
+ struct gnttab_unmap_grant_ref *gop;
+ pending_ring_idx_t dc, dp;
+ u16 pending_idx, pending_idx_release[MAX_PENDING_REQS];
+ unsigned int i = 0;
+
+ dc = vif->dealloc_cons;
+ gop = vif->tx_unmap_ops;
+
+ /* Free up any grants we have finished using */
+ do {
+ dp = vif->dealloc_prod;
+
+ /* Ensure we see all indices enqueued by netif_idx_release(). */
+ smp_rmb();
+
+ while (dc != dp) {
+
+ pending_idx_release[gop-vif->tx_unmap_ops] =
+ pending_idx =
+ vif->dealloc_ring[pending_index(dc++)];
+
+ /* Already unmapped? */
+ if (vif->grant_tx_handle[pending_idx] ==
+ NETBACK_INVALID_HANDLE) {
+ netdev_err(vif->dev,
+ "Trying to unmap invalid handle! "
+ "pending_idx: %x\n", pending_idx);
+ continue;
+ }
+
+ vif->pages_to_gnt[gop-vif->tx_unmap_ops] =
+ vif->mmap_pages[pending_idx];
+ gnttab_set_unmap_op(gop,
+ idx_to_kaddr(vif, pending_idx),
+ GNTMAP_host_map,
+ vif->grant_tx_handle[pending_idx]);
+ vif->grant_tx_handle[pending_idx] =
+ NETBACK_INVALID_HANDLE;
+ ++gop;
+ }
+
+ } while (dp != vif->dealloc_prod);
+
+ vif->dealloc_cons = dc;
+
+ if (gop - vif->tx_unmap_ops > 0) {
+ int ret;
+ ret = gnttab_unmap_refs(vif->tx_unmap_ops,
+ NULL,
+ vif->pages_to_gnt,
+ gop - vif->tx_unmap_ops);
+ if (ret) {
+ netdev_err(vif->dev, "Unmap fail: nr_ops %x ret %d\n",
+ gop - vif->tx_unmap_ops, ret);
+ for (i = 0; i < gop - vif->tx_unmap_ops; ++i) {
+ netdev_err(vif->dev,
+ " host_addr: %llx handle: %x status: %d\n",
+ gop[i].host_addr,
+ gop[i].handle,
+ gop[i].status);
+ }
+ BUG();
+ }
+ }
+
+ for (i = 0; i < gop - vif->tx_unmap_ops; ++i)
+ xenvif_idx_release(vif, pending_idx_release[i],
+ XEN_NETIF_RSP_OKAY);
+}
+
+
/* Called after netfront has transmitted */
int xenvif_tx_action(struct xenvif *vif, int budget)
{
@@ -1718,6 +1832,26 @@ static void xenvif_idx_release(struct xenvif *vif, u16 pending_idx,
vif->mmap_pages[pending_idx] = NULL;
}
+void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx)
+{
+ int ret;
+ if (vif->grant_tx_handle[pending_idx] == NETBACK_INVALID_HANDLE) {
+ netdev_err(vif->dev,
+ "Trying to unmap invalid handle! pending_idx: %x\n",
+ pending_idx);
+ return;
+ }
+ gnttab_set_unmap_op(&vif->tx_unmap_ops[0],
+ idx_to_kaddr(vif, pending_idx),
+ GNTMAP_host_map,
+ vif->grant_tx_handle[pending_idx]);
+ ret = gnttab_unmap_refs(vif->tx_unmap_ops,
+ NULL,
+ &vif->mmap_pages[pending_idx],
+ 1);
+ BUG_ON(ret);
+ vif->grant_tx_handle[pending_idx] = NETBACK_INVALID_HANDLE;
+}
static void make_tx_response(struct xenvif *vif,
struct xen_netif_tx_request *txp,
^ permalink raw reply related
* [PATCH net-next RFC 2/5] xen-netback: Change TX path from grant copy to mapping
From: Zoltan Kiss @ 2013-10-30 0:50 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1383094220-14775-1-git-send-email-zoltan.kiss@citrix.com>
This patch changes the grant copy on the TX patch to grant mapping
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/interface.c | 39 +++++-
drivers/net/xen-netback/netback.c | 241 +++++++++++++----------------------
2 files changed, 126 insertions(+), 154 deletions(-)
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index f5c3c57..fb16ede 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -336,8 +336,20 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
vif->pending_prod = MAX_PENDING_REQS;
for (i = 0; i < MAX_PENDING_REQS; i++)
vif->pending_ring[i] = i;
- for (i = 0; i < MAX_PENDING_REQS; i++)
- vif->mmap_pages[i] = NULL;
+ err = alloc_xenballooned_pages(MAX_PENDING_REQS,
+ vif->mmap_pages,
+ false);
+ if (err) {
+ netdev_err(dev, "Could not reserve mmap_pages\n");
+ return NULL;
+ }
+ for (i = 0; i < MAX_PENDING_REQS; i++) {
+ vif->pending_tx_info[i].callback_struct = (struct ubuf_info)
+ { .callback = xenvif_zerocopy_callback,
+ .ctx = NULL,
+ .desc = i };
+ vif->grant_tx_handle[i] = NETBACK_INVALID_HANDLE;
+ }
/*
* Initialise a dummy MAC address. We choose the numerically
@@ -481,11 +493,34 @@ void xenvif_disconnect(struct xenvif *vif)
void xenvif_free(struct xenvif *vif)
{
+ int i;
+
netif_napi_del(&vif->napi);
unregister_netdev(vif->dev);
free_netdev(vif->dev);
+ /* FIXME: This is a workaround for 2 problems:
+ * - the guest sent a packet just before teardown, and it is still not
+ * delivered
+ * - the stack forgot to notify us that we can give back a slot
+ * For the first problem we shouldn't do this, as the skb might still
+ * access that page. I will come up with a more robust solution later.
+ * The second is definitely a bug, it leaks a slot from the ring
+ * forever.
+ * Classic kernel patchset has delayed copy for that, we might want to
+ * reuse that?
+ */
+ for (i = 0; i < MAX_PENDING_REQS; ++i) {
+ if (vif->grant_tx_handle[i] != NETBACK_INVALID_HANDLE) {
+ netdev_err(vif->dev,
+ "Page still granted! Index: %x\n", i);
+ xenvif_idx_unmap(vif, i);
+ }
+ }
+
+ free_xenballooned_pages(MAX_PENDING_REQS, vif->mmap_pages);
+
module_put(THIS_MODULE);
}
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 10470dc..e544e9f 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -883,10 +883,10 @@ static inline void xenvif_tx_create_gop(struct xenvif *vif, u16 pending_idx,
}
-static struct gnttab_copy *xenvif_get_requests(struct xenvif *vif,
+static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif,
struct sk_buff *skb,
struct xen_netif_tx_request *txp,
- struct gnttab_copy *gop)
+ struct gnttab_map_grant_ref *gop)
{
struct skb_shared_info *shinfo = skb_shinfo(skb);
skb_frag_t *frags = shinfo->frags;
@@ -907,83 +907,12 @@ static struct gnttab_copy *xenvif_get_requests(struct xenvif *vif,
/* Skip first skb fragment if it is on same page as header fragment. */
start = (frag_get_pending_idx(&shinfo->frags[0]) == pending_idx);
- /* Coalesce tx requests, at this point the packet passed in
- * should be <= 64K. Any packets larger than 64K have been
- * handled in xenvif_count_requests().
- */
- for (shinfo->nr_frags = slot = start; slot < nr_slots;
- shinfo->nr_frags++) {
- struct pending_tx_info *pending_tx_info =
- vif->pending_tx_info;
-
- page = alloc_page(GFP_ATOMIC|__GFP_COLD);
- if (!page)
- goto err;
-
- dst_offset = 0;
- first = NULL;
- while (dst_offset < PAGE_SIZE && slot < nr_slots) {
- gop->flags = GNTCOPY_source_gref;
-
- gop->source.u.ref = txp->gref;
- gop->source.domid = vif->domid;
- gop->source.offset = txp->offset;
-
- gop->dest.domid = DOMID_SELF;
-
- gop->dest.offset = dst_offset;
- gop->dest.u.gmfn = virt_to_mfn(page_address(page));
-
- if (dst_offset + txp->size > PAGE_SIZE) {
- /* This page can only merge a portion
- * of tx request. Do not increment any
- * pointer / counter here. The txp
- * will be dealt with in future
- * rounds, eventually hitting the
- * `else` branch.
- */
- gop->len = PAGE_SIZE - dst_offset;
- txp->offset += gop->len;
- txp->size -= gop->len;
- dst_offset += gop->len; /* quit loop */
- } else {
- /* This tx request can be merged in the page */
- gop->len = txp->size;
- dst_offset += gop->len;
-
+ for (shinfo->nr_frags = start; shinfo->nr_frags < nr_slots;
+ shinfo->nr_frags++, txp++, gop++) {
index = pending_index(vif->pending_cons++);
-
pending_idx = vif->pending_ring[index];
-
- memcpy(&pending_tx_info[pending_idx].req, txp,
- sizeof(*txp));
-
- /* Poison these fields, corresponding
- * fields for head tx req will be set
- * to correct values after the loop.
- */
- vif->mmap_pages[pending_idx] = (void *)(~0UL);
- pending_tx_info[pending_idx].head =
- INVALID_PENDING_RING_IDX;
-
- if (!first) {
- first = &pending_tx_info[pending_idx];
- start_idx = index;
- head_idx = pending_idx;
- }
-
- txp++;
- slot++;
- }
-
- gop++;
- }
-
- first->req.offset = 0;
- first->req.size = dst_offset;
- first->head = start_idx;
- vif->mmap_pages[head_idx] = page;
- frag_set_pending_idx(&frags[shinfo->nr_frags], head_idx);
+ xenvif_tx_create_gop(vif, pending_idx, txp, gop);
+ frag_set_pending_idx(&frags[shinfo->nr_frags], pending_idx);
}
BUG_ON(shinfo->nr_frags > MAX_SKB_FRAGS);
@@ -1005,9 +934,9 @@ err:
static int xenvif_tx_check_gop(struct xenvif *vif,
struct sk_buff *skb,
- struct gnttab_copy **gopp)
+ struct gnttab_map_grant_ref **gopp)
{
- struct gnttab_copy *gop = *gopp;
+ struct gnttab_map_grant_ref *gop = *gopp;
u16 pending_idx = *((u16 *)skb->data);
struct skb_shared_info *shinfo = skb_shinfo(skb);
struct pending_tx_info *tx_info;
@@ -1019,6 +948,16 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
err = gop->status;
if (unlikely(err))
xenvif_idx_release(vif, pending_idx, XEN_NETIF_RSP_ERROR);
+ else {
+ if (vif->grant_tx_handle[pending_idx] !=
+ NETBACK_INVALID_HANDLE) {
+ netdev_err(vif->dev,
+ "Stale mapped handle! pending_idx %x handle %x\n",
+ pending_idx, vif->grant_tx_handle[pending_idx]);
+ xenvif_fatal_tx_err(vif);
+ }
+ vif->grant_tx_handle[pending_idx] = gop->handle;
+ }
/* Skip first skb fragment if it is on same page as header fragment. */
start = (frag_get_pending_idx(&shinfo->frags[0]) == pending_idx);
@@ -1032,18 +971,24 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
head = tx_info->head;
/* Check error status: if okay then remember grant handle. */
- do {
newerr = (++gop)->status;
- if (newerr)
- break;
- peek = vif->pending_ring[pending_index(++head)];
- } while (!pending_tx_is_head(vif, peek));
if (likely(!newerr)) {
+ if (vif->grant_tx_handle[pending_idx] !=
+ NETBACK_INVALID_HANDLE) {
+ netdev_err(vif->dev,
+ "Stale mapped handle! pending_idx %x handle %x\n",
+ pending_idx,
+ vif->grant_tx_handle[pending_idx]);
+ xenvif_fatal_tx_err(vif);
+ }
+ vif->grant_tx_handle[pending_idx] = gop->handle;
/* Had a previous error? Invalidate this fragment. */
- if (unlikely(err))
+ if (unlikely(err)) {
+ xenvif_idx_unmap(vif, pending_idx);
xenvif_idx_release(vif, pending_idx,
XEN_NETIF_RSP_OKAY);
+ }
continue;
}
@@ -1056,9 +1001,11 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
/* First error: invalidate header and preceding fragments. */
pending_idx = *((u16 *)skb->data);
+ xenvif_idx_unmap(vif, pending_idx);
xenvif_idx_release(vif, pending_idx, XEN_NETIF_RSP_OKAY);
for (j = start; j < i; j++) {
pending_idx = frag_get_pending_idx(&shinfo->frags[j]);
+ xenvif_idx_unmap(vif, pending_idx);
xenvif_idx_release(vif, pending_idx,
XEN_NETIF_RSP_OKAY);
}
@@ -1071,7 +1018,8 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
return err;
}
-static void xenvif_fill_frags(struct xenvif *vif, struct sk_buff *skb)
+static void xenvif_fill_frags(struct xenvif *vif, struct sk_buff *skb,
+ u16 prev_pending_idx)
{
struct skb_shared_info *shinfo = skb_shinfo(skb);
int nr_frags = shinfo->nr_frags;
@@ -1085,6 +1033,15 @@ static void xenvif_fill_frags(struct xenvif *vif, struct sk_buff *skb)
pending_idx = frag_get_pending_idx(frag);
+ /* If this is not the first frag, chain it to the previous*/
+ vif->pending_tx_info[pending_idx].callback_struct.ctx = NULL;
+ if (pending_idx != prev_pending_idx) {
+ vif->pending_tx_info[prev_pending_idx].callback_struct.ctx =
+ &(vif->pending_tx_info[pending_idx].callback_struct);
+ prev_pending_idx = pending_idx;
+ }
+
+
txp = &vif->pending_tx_info[pending_idx].req;
page = virt_to_page(idx_to_kaddr(vif, pending_idx));
__skb_fill_page_desc(skb, i, page, txp->offset, txp->size);
@@ -1092,10 +1049,15 @@ static void xenvif_fill_frags(struct xenvif *vif, struct sk_buff *skb)
skb->data_len += txp->size;
skb->truesize += txp->size;
- /* Take an extra reference to offset xenvif_idx_release */
+ /* Take an extra reference to offset network stack's put_page */
get_page(vif->mmap_pages[pending_idx]);
- xenvif_idx_release(vif, pending_idx, XEN_NETIF_RSP_OKAY);
}
+ /* FIXME: __skb_fill_page_desc set this to true because page->pfmemalloc
+ * overlaps with "index", and "mapping" is not set. I think mapping
+ * should be set. If delivered to local stack, it would drop this
+ * skb in sk_filter unless the socket has the right to use it.
+ */
+ skb->pfmemalloc = false;
}
static int xenvif_get_extras(struct xenvif *vif,
@@ -1426,7 +1388,7 @@ static bool tx_credit_exceeded(struct xenvif *vif, unsigned size)
static unsigned xenvif_tx_build_gops(struct xenvif *vif)
{
- struct gnttab_copy *gop = vif->tx_copy_ops, *request_gop;
+ struct gnttab_map_grant_ref *gop = vif->tx_map_ops, *request_gop;
struct sk_buff *skb;
int ret;
@@ -1533,30 +1495,10 @@ static unsigned xenvif_tx_build_gops(struct xenvif *vif)
}
}
- /* XXX could copy straight to head */
- page = xenvif_alloc_page(vif, pending_idx);
- if (!page) {
- kfree_skb(skb);
- xenvif_tx_err(vif, &txreq, idx);
- break;
- }
-
- gop->source.u.ref = txreq.gref;
- gop->source.domid = vif->domid;
- gop->source.offset = txreq.offset;
-
- gop->dest.u.gmfn = virt_to_mfn(page_address(page));
- gop->dest.domid = DOMID_SELF;
- gop->dest.offset = txreq.offset;
-
- gop->len = txreq.size;
- gop->flags = GNTCOPY_source_gref;
+ xenvif_tx_create_gop(vif, pending_idx, &txreq, gop);
gop++;
- memcpy(&vif->pending_tx_info[pending_idx].req,
- &txreq, sizeof(txreq));
- vif->pending_tx_info[pending_idx].head = index;
*((u16 *)skb->data) = pending_idx;
__skb_put(skb, data_len);
@@ -1585,17 +1527,17 @@ static unsigned xenvif_tx_build_gops(struct xenvif *vif)
vif->tx.req_cons = idx;
- if ((gop-vif->tx_copy_ops) >= ARRAY_SIZE(vif->tx_copy_ops))
+ if ((gop-vif->tx_map_ops) >= ARRAY_SIZE(vif->tx_map_ops))
break;
}
- return gop - vif->tx_copy_ops;
+ return gop - vif->tx_map_ops;
}
static int xenvif_tx_submit(struct xenvif *vif, int budget)
{
- struct gnttab_copy *gop = vif->tx_copy_ops;
+ struct gnttab_map_grant_ref *gop = vif->tx_map_ops;
struct sk_buff *skb;
int work_done = 0;
@@ -1620,14 +1562,25 @@ static int xenvif_tx_submit(struct xenvif *vif, int budget)
memcpy(skb->data,
(void *)(idx_to_kaddr(vif, pending_idx)|txp->offset),
data_len);
+ vif->pending_tx_info[pending_idx].callback_struct.ctx = NULL;
if (data_len < txp->size) {
/* Append the packet payload as a fragment. */
txp->offset += data_len;
txp->size -= data_len;
- } else {
+ skb_shinfo(skb)->destructor_arg =
+ &vif->pending_tx_info[pending_idx].callback_struct;
+ } else if (!skb_shinfo(skb)->nr_frags) {
/* Schedule a response immediately. */
+ skb_shinfo(skb)->destructor_arg = NULL;
+ xenvif_idx_unmap(vif, pending_idx);
xenvif_idx_release(vif, pending_idx,
XEN_NETIF_RSP_OKAY);
+ } else {
+ /* FIXME: first request fits linear space, I don't know
+ * if any guest would do that, but I think it's possible
+ */
+ skb_shinfo(skb)->destructor_arg =
+ &vif->pending_tx_info[pending_idx].callback_struct;
}
if (txp->flags & XEN_NETTXF_csum_blank)
@@ -1635,13 +1588,19 @@ static int xenvif_tx_submit(struct xenvif *vif, int budget)
else if (txp->flags & XEN_NETTXF_data_validated)
skb->ip_summed = CHECKSUM_UNNECESSARY;
- xenvif_fill_frags(vif, skb);
+ xenvif_fill_frags(vif, skb, pending_idx);
if (skb_is_nonlinear(skb) && skb_headlen(skb) < PKT_PROT_LEN) {
int target = min_t(int, skb->len, PKT_PROT_LEN);
__pskb_pull_tail(skb, target - skb_headlen(skb));
}
+ /* Set this flag after __pskb_pull_tail, as it can trigger
+ * skb_copy_ubufs, while we are still in control of the skb
+ */
+ if (skb_shinfo(skb)->destructor_arg)
+ skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
+
skb->dev = vif->dev;
skb->protocol = eth_type_trans(skb, skb->dev);
skb_reset_network_header(skb);
@@ -1770,17 +1729,25 @@ static inline void xenvif_tx_action_dealloc(struct xenvif *vif)
int xenvif_tx_action(struct xenvif *vif, int budget)
{
unsigned nr_gops;
- int work_done;
+ int work_done, ret;
if (unlikely(!tx_work_todo(vif)))
return 0;
+ xenvif_tx_action_dealloc(vif);
+
nr_gops = xenvif_tx_build_gops(vif);
if (nr_gops == 0)
return 0;
- gnttab_batch_copy(vif->tx_copy_ops, nr_gops);
+ if (nr_gops) {
+ ret = gnttab_map_refs(vif->tx_map_ops,
+ NULL,
+ vif->pages_to_gnt,
+ nr_gops);
+ BUG_ON(ret);
+ }
work_done = xenvif_tx_submit(vif, nr_gops);
@@ -1791,45 +1758,13 @@ static void xenvif_idx_release(struct xenvif *vif, u16 pending_idx,
u8 status)
{
struct pending_tx_info *pending_tx_info;
- pending_ring_idx_t head;
+ pending_ring_idx_t index;
u16 peek; /* peek into next tx request */
- BUG_ON(vif->mmap_pages[pending_idx] == (void *)(~0UL));
-
- /* Already complete? */
- if (vif->mmap_pages[pending_idx] == NULL)
- return;
-
- pending_tx_info = &vif->pending_tx_info[pending_idx];
-
- head = pending_tx_info->head;
-
- BUG_ON(!pending_tx_is_head(vif, head));
- BUG_ON(vif->pending_ring[pending_index(head)] != pending_idx);
-
- do {
- pending_ring_idx_t index;
- pending_ring_idx_t idx = pending_index(head);
- u16 info_idx = vif->pending_ring[idx];
-
- pending_tx_info = &vif->pending_tx_info[info_idx];
+ pending_tx_info = &vif->pending_tx_info[pending_idx];
make_tx_response(vif, &pending_tx_info->req, status);
-
- /* Setting any number other than
- * INVALID_PENDING_RING_IDX indicates this slot is
- * starting a new packet / ending a previous packet.
- */
- pending_tx_info->head = 0;
-
index = pending_index(vif->pending_prod++);
- vif->pending_ring[index] = vif->pending_ring[info_idx];
-
- peek = vif->pending_ring[pending_index(++head)];
-
- } while (!pending_tx_is_head(vif, peek));
-
- put_page(vif->mmap_pages[pending_idx]);
- vif->mmap_pages[pending_idx] = NULL;
+ vif->pending_ring[index] = pending_idx;
}
void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx)
@@ -1904,6 +1839,8 @@ static inline int rx_work_todo(struct xenvif *vif)
static inline int tx_work_todo(struct xenvif *vif)
{
+ if (vif->dealloc_cons != vif->dealloc_prod)
+ return 1;
if (likely(RING_HAS_UNCONSUMED_REQUESTS(&vif->tx)) &&
(nr_pending_reqs(vif) + XEN_NETBK_LEGACY_SLOTS_MAX
^ permalink raw reply related
* [PATCH net-next RFC 3/5] xen-netback: Remove old TX grant copy definitons
From: Zoltan Kiss @ 2013-10-30 0:50 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1383094220-14775-1-git-send-email-zoltan.kiss@citrix.com>
These became obsolate with grant mapping.
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/common.h | 37 +---------------------------
drivers/net/xen-netback/netback.c | 48 ++-----------------------------------
2 files changed, 3 insertions(+), 82 deletions(-)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 36a3fda..e82c82c 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -46,39 +46,9 @@
#include <xen/xenbus.h>
typedef unsigned int pending_ring_idx_t;
-#define INVALID_PENDING_RING_IDX (~0U)
-/* For the head field in pending_tx_info: it is used to indicate
- * whether this tx info is the head of one or more coalesced requests.
- *
- * When head != INVALID_PENDING_RING_IDX, it means the start of a new
- * tx requests queue and the end of previous queue.
- *
- * An example sequence of head fields (I = INVALID_PENDING_RING_IDX):
- *
- * ...|0 I I I|5 I|9 I I I|...
- * -->|<-INUSE----------------
- *
- * After consuming the first slot(s) we have:
- *
- * ...|V V V V|5 I|9 I I I|...
- * -----FREE->|<-INUSE--------
- *
- * where V stands for "valid pending ring index". Any number other
- * than INVALID_PENDING_RING_IDX is OK. These entries are considered
- * free and can contain any number other than
- * INVALID_PENDING_RING_IDX. In practice we use 0.
- *
- * The in use non-INVALID_PENDING_RING_IDX (say 0, 5 and 9 in the
- * above example) number is the index into pending_tx_info and
- * mmap_pages arrays.
- */
struct pending_tx_info {
- struct xen_netif_tx_request req; /* coalesced tx request */
- pending_ring_idx_t head; /* head != INVALID_PENDING_RING_IDX
- * if it is head of one or more tx
- * reqs
- */
+ struct xen_netif_tx_request req; /* tx request */
/* callback data for released SKBs. The callback is always
* xenvif_zerocopy_callback, ctx points to the next fragment, desc
* contains the pending_idx
@@ -128,11 +98,6 @@ struct xenvif {
struct pending_tx_info pending_tx_info[MAX_PENDING_REQS];
grant_handle_t grant_tx_handle[MAX_PENDING_REQS];
- /* Coalescing tx requests before copying makes number of grant
- * copy ops greater or equal to number of slots required. In
- * worst case a tx request consumes 2 gnttab_copy.
- */
- struct gnttab_copy tx_copy_ops[2*MAX_PENDING_REQS];
struct gnttab_unmap_grant_ref tx_unmap_ops[MAX_PENDING_REQS];
struct gnttab_map_grant_ref tx_map_ops[MAX_PENDING_REQS];
/* passed to gnttab_[un]map_refs with pages under (un)mapping */
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index e544e9f..c91dd36 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -70,16 +70,6 @@ module_param(fatal_skb_slots, uint, 0444);
*/
#define XEN_NETBK_LEGACY_SLOTS_MAX XEN_NETIF_NR_SLOTS_MIN
-/*
- * If head != INVALID_PENDING_RING_IDX, it means this tx request is head of
- * one or more merged tx requests, otherwise it is the continuation of
- * previous tx request.
- */
-static inline int pending_tx_is_head(struct xenvif *vif, RING_IDX idx)
-{
- return vif->pending_tx_info[idx].head != INVALID_PENDING_RING_IDX;
-}
-
static void xenvif_idx_release(struct xenvif *vif, u16 pending_idx,
u8 status);
@@ -856,19 +846,6 @@ static int xenvif_count_requests(struct xenvif *vif,
return slots;
}
-static struct page *xenvif_alloc_page(struct xenvif *vif,
- u16 pending_idx)
-{
- struct page *page;
-
- page = alloc_page(GFP_ATOMIC|__GFP_COLD);
- if (!page)
- return NULL;
- vif->mmap_pages[pending_idx] = page;
-
- return page;
-}
-
static inline void xenvif_tx_create_gop(struct xenvif *vif, u16 pending_idx,
struct xen_netif_tx_request *txp,
struct gnttab_map_grant_ref *gop)
@@ -891,13 +868,9 @@ static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif,
struct skb_shared_info *shinfo = skb_shinfo(skb);
skb_frag_t *frags = shinfo->frags;
u16 pending_idx = *((u16 *)skb->data);
- u16 head_idx = 0;
- int slot, start;
- struct page *page;
- pending_ring_idx_t index, start_idx = 0;
- uint16_t dst_offset;
+ int start;
+ pending_ring_idx_t index;
unsigned int nr_slots;
- struct pending_tx_info *first = NULL;
/* At this point shinfo->nr_frags is in fact the number of
* slots, which can be as large as XEN_NETBK_LEGACY_SLOTS_MAX.
@@ -918,18 +891,6 @@ static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif,
BUG_ON(shinfo->nr_frags > MAX_SKB_FRAGS);
return gop;
-err:
- /* Unwind, freeing all pages and sending error responses. */
- while (shinfo->nr_frags-- > start) {
- xenvif_idx_release(vif,
- frag_get_pending_idx(&frags[shinfo->nr_frags]),
- XEN_NETIF_RSP_ERROR);
- }
- /* The head too, if necessary. */
- if (start)
- xenvif_idx_release(vif, pending_idx, XEN_NETIF_RSP_ERROR);
-
- return NULL;
}
static int xenvif_tx_check_gop(struct xenvif *vif,
@@ -942,7 +903,6 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
struct pending_tx_info *tx_info;
int nr_frags = shinfo->nr_frags;
int i, err, start;
- u16 peek; /* peek into next tx request */
/* Check status of header. */
err = gop->status;
@@ -964,11 +924,9 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
for (i = start; i < nr_frags; i++) {
int j, newerr;
- pending_ring_idx_t head;
pending_idx = frag_get_pending_idx(&shinfo->frags[i]);
tx_info = &vif->pending_tx_info[pending_idx];
- head = tx_info->head;
/* Check error status: if okay then remember grant handle. */
newerr = (++gop)->status;
@@ -1396,7 +1354,6 @@ static unsigned xenvif_tx_build_gops(struct xenvif *vif)
< MAX_PENDING_REQS)) {
struct xen_netif_tx_request txreq;
struct xen_netif_tx_request txfrags[XEN_NETBK_LEGACY_SLOTS_MAX];
- struct page *page;
struct xen_netif_extra_info extras[XEN_NETIF_EXTRA_TYPE_MAX-1];
u16 pending_idx;
RING_IDX idx;
@@ -1759,7 +1716,6 @@ static void xenvif_idx_release(struct xenvif *vif, u16 pending_idx,
{
struct pending_tx_info *pending_tx_info;
pending_ring_idx_t index;
- u16 peek; /* peek into next tx request */
pending_tx_info = &vif->pending_tx_info[pending_idx];
make_tx_response(vif, &pending_tx_info->req, status);
^ permalink raw reply related
* [PATCH net-next RFC 4/5] xen-netback: Fix indentations
From: Zoltan Kiss @ 2013-10-30 0:50 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1383094220-14775-1-git-send-email-zoltan.kiss@citrix.com>
I've left intentionally these indentations in this way, to improve readability of previous patches.
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/netback.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index c91dd36..204fa46 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -882,8 +882,8 @@ static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif,
for (shinfo->nr_frags = start; shinfo->nr_frags < nr_slots;
shinfo->nr_frags++, txp++, gop++) {
- index = pending_index(vif->pending_cons++);
- pending_idx = vif->pending_ring[index];
+ index = pending_index(vif->pending_cons++);
+ pending_idx = vif->pending_ring[index];
xenvif_tx_create_gop(vif, pending_idx, txp, gop);
frag_set_pending_idx(&frags[shinfo->nr_frags], pending_idx);
}
@@ -929,7 +929,7 @@ static int xenvif_tx_check_gop(struct xenvif *vif,
tx_info = &vif->pending_tx_info[pending_idx];
/* Check error status: if okay then remember grant handle. */
- newerr = (++gop)->status;
+ newerr = (++gop)->status;
if (likely(!newerr)) {
if (vif->grant_tx_handle[pending_idx] !=
@@ -1717,10 +1717,10 @@ static void xenvif_idx_release(struct xenvif *vif, u16 pending_idx,
struct pending_tx_info *pending_tx_info;
pending_ring_idx_t index;
- pending_tx_info = &vif->pending_tx_info[pending_idx];
- make_tx_response(vif, &pending_tx_info->req, status);
- index = pending_index(vif->pending_prod++);
- vif->pending_ring[index] = pending_idx;
+ pending_tx_info = &vif->pending_tx_info[pending_idx];
+ make_tx_response(vif, &pending_tx_info->req, status);
+ index = pending_index(vif->pending_prod++);
+ vif->pending_ring[index] = pending_idx;
}
void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx)
^ permalink raw reply related
* [PATCH net-next RFC 4/5] xen-netback: Change RX path for mapped SKB fragments
From: Zoltan Kiss @ 2013-10-30 0:50 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss
In-Reply-To: <1383094220-14775-1-git-send-email-zoltan.kiss@citrix.com>
RX path need to know if the SKB fragments are stored on pages from another
domain.
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/netback.c | 46 +++++++++++++++++++++++++++++++++----
1 file changed, 41 insertions(+), 5 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 204fa46..0ffa412 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -322,7 +322,9 @@ static struct xenvif_rx_meta *get_next_rx_buffer(struct xenvif *vif,
static void xenvif_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
struct netrx_pending_operations *npo,
struct page *page, unsigned long size,
- unsigned long offset, int *head)
+ unsigned long offset, int *head,
+ struct xenvif *foreign_vif,
+ grant_ref_t foreign_gref)
{
struct gnttab_copy *copy_gop;
struct xenvif_rx_meta *meta;
@@ -364,8 +366,15 @@ static void xenvif_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
copy_gop->flags = GNTCOPY_dest_gref;
copy_gop->len = bytes;
- copy_gop->source.domid = DOMID_SELF;
- copy_gop->source.u.gmfn = virt_to_mfn(page_address(page));
+ if (foreign_vif) {
+ copy_gop->source.domid = foreign_vif->domid;
+ copy_gop->source.u.ref = foreign_gref;
+ copy_gop->flags |= GNTCOPY_source_gref;
+ } else {
+ copy_gop->source.domid = DOMID_SELF;
+ copy_gop->source.u.gmfn =
+ virt_to_mfn(page_address(page));
+ }
copy_gop->source.offset = offset;
copy_gop->dest.domid = vif->domid;
@@ -426,6 +435,9 @@ static int xenvif_gop_skb(struct sk_buff *skb,
int old_meta_prod;
int gso_type;
int gso_size;
+ struct ubuf_info *ubuf = skb_shinfo(skb)->destructor_arg;
+ grant_ref_t foreign_grefs[MAX_SKB_FRAGS];
+ struct xenvif *foreign_vif = NULL;
old_meta_prod = npo->meta_prod;
@@ -466,6 +478,26 @@ static int xenvif_gop_skb(struct sk_buff *skb,
npo->copy_off = 0;
npo->copy_gref = req->gref;
+ if ((skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) &&
+ (ubuf->callback == &xenvif_zerocopy_callback)) {
+ u16 pending_idx = ubuf->desc;
+ int i = 0;
+ struct pending_tx_info *temp =
+ container_of(ubuf,
+ struct pending_tx_info,
+ callback_struct);
+ foreign_vif =
+ container_of(temp - pending_idx,
+ struct xenvif,
+ pending_tx_info[0]);
+ do {
+ pending_idx = ubuf->desc;
+ ubuf = (struct ubuf_info *) ubuf->ctx;
+ foreign_grefs[i++] =
+ foreign_vif->pending_tx_info[pending_idx].req.gref;
+ } while (ubuf);
+ }
+
data = skb->data;
while (data < skb_tail_pointer(skb)) {
unsigned int offset = offset_in_page(data);
@@ -475,7 +507,9 @@ static int xenvif_gop_skb(struct sk_buff *skb,
len = skb_tail_pointer(skb) - data;
xenvif_gop_frag_copy(vif, skb, npo,
- virt_to_page(data), len, offset, &head);
+ virt_to_page(data), len, offset, &head,
+ NULL,
+ 0);
data += len;
}
@@ -484,7 +518,9 @@ static int xenvif_gop_skb(struct sk_buff *skb,
skb_frag_page(&skb_shinfo(skb)->frags[i]),
skb_frag_size(&skb_shinfo(skb)->frags[i]),
skb_shinfo(skb)->frags[i].page_offset,
- &head);
+ &head,
+ foreign_vif,
+ foreign_grefs[i]);
}
return npo->meta_prod - old_meta_prod;
^ permalink raw reply related
* Re: [PATCH v2 net-next] net: introduce gro_frag_list_enable sysctl
From: Eric Dumazet @ 2013-10-30 0:53 UTC (permalink / raw)
To: David Miller; +Cc: christoph.paasch, herbert, netdev, hkchu, mwdalton
In-Reply-To: <20131029.194446.2215574000648693370.davem@davemloft.net>
On Tue, 2013-10-29 at 19:44 -0400, David Miller wrote:
> I do not like the idea of packet actions indirectly changing sysctl
> values, even if you document it sufficiently as you have here.
>
Fair enough.
> I would suggest instead making it change in response to changes to
> ip_forward, as we do with per-device LRO settings. This means that,
> like ip_forward, you should also make this sysctl a global + devinet
> per-device sysctl.
>
> You might even emit a pr_info() when this logic triggers, and if you
> are ambitious enough keep track of the previous GRO sysctl state so
> you can restore it if ip_forward is set back to zero.
Ok, but this might take some time.
So should we apply the first fix to avoid the BUG_ON() ?
^ permalink raw reply
* [GIT PULL] Second Round of IPVS updates for v3.13
From: Simon Horman @ 2013-10-30 1:11 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Daniel Borkmann, Simon Horman
Hi Pablo,
please consider this second round of updates or IPVS for v3.13,
based on nf-next.
I realise this is rather late in the game as the v3.13 merge window
is just round the corner. So please let me know if you would
like me to handle things a different way.
There are two SCTP changes by Daniel Borkman.
* Correct verdict assignments in sctp_conn_schedule
This is a bug fix for incorrect handling of the error case
where skb_header_pointer() fails.
It appears that this problem has been present since SCTP was added
to IPVS in v2.6.34.
I would like this change considered for -stable all the way back
to v2.6.34. I have checked and it cherry-picks cleanly on top
of v3.12-rc7 and v3.11. And seems to apply with some trivial merge conflicts
on top of v3.4, v3.2 and v3.6.34. I am happy to supply versions of
the patch that resolve those conflicts.
* Do not recalculate SCTP checksum if ports don't change
This is an enhancement which should lead to increased performance
in some cases.
The following changes since commit 6b8dbcf2c44fd7aa716560d04e9857c870bd510c:
bridge: netfilter: orphan skb before invoking ip netfilter hooks (2013-10-27 21:44:33 +0100)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-next.git tags/ipvs2-for-v3.13
for you to fetch changes up to 97203abe6bc41ee020f37c902bd1a761157f22c1:
net: ipvs: sctp: do not recalc sctp csum when ports didn't change (2013-10-30 09:48:16 +0900)
----------------------------------------------------------------
Second Round of IPVS updates for v3.13
Some SCTP changes by Daniel Borkman.
* Correct verdict assignments in sctp_conn_schedule
This is a bug fix for incorrect handling of the error case
where skb_header_pointer() fails.
It appears that this problem has been present since SCTP was added
to IPVS in v2.6.34.
* Do not recalculate SCTP checksum if ports don't change
This is an enhancement which should lead to increased performance
in some cases.
----------------------------------------------------------------
Daniel Borkmann (2):
net: ipvs: sctp: add missing verdict assignments in sctp_conn_schedule
net: ipvs: sctp: do not recalc sctp csum when ports didn't change
net/netfilter/ipvs/ip_vs_proto_sctp.c | 48 +++++++++++++++++++++++++++++------
1 file changed, 40 insertions(+), 8 deletions(-)
^ permalink raw reply
* [PATCH nf-next 1/2] net: ipvs: sctp: add missing verdict assignments in sctp_conn_schedule
From: Simon Horman @ 2013-10-30 1:11 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Daniel Borkmann, Simon Horman
In-Reply-To: <1383095486-5215-1-git-send-email-horms@verge.net.au>
From: Daniel Borkmann <dborkman@redhat.com>
If skb_header_pointer() fails, we need to assign a verdict, that is
NF_DROP in this case, otherwise, we would leave the verdict from
conn_schedule() uninitialized when returning.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_proto_sctp.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
index 23e596e..9ca7aa0 100644
--- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
@@ -20,13 +20,18 @@ sctp_conn_schedule(int af, struct sk_buff *skb, struct ip_vs_proto_data *pd,
sctp_sctphdr_t *sh, _sctph;
sh = skb_header_pointer(skb, iph->len, sizeof(_sctph), &_sctph);
- if (sh == NULL)
+ if (sh == NULL) {
+ *verdict = NF_DROP;
return 0;
+ }
sch = skb_header_pointer(skb, iph->len + sizeof(sctp_sctphdr_t),
sizeof(_schunkh), &_schunkh);
- if (sch == NULL)
+ if (sch == NULL) {
+ *verdict = NF_DROP;
return 0;
+ }
+
net = skb_net(skb);
ipvs = net_ipvs(net);
rcu_read_lock();
--
1.8.4
^ permalink raw reply related
* [PATCH nf-next 2/2] net: ipvs: sctp: do not recalc sctp csum when ports didn't change
From: Simon Horman @ 2013-10-30 1:11 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Daniel Borkmann, Simon Horman
In-Reply-To: <1383095486-5215-1-git-send-email-horms@verge.net.au>
From: Daniel Borkmann <dborkman@redhat.com>
Unlike UDP or TCP, we do not take the pseudo-header into
account in SCTP checksums. So in case port mapping is the
very same, we do not need to recalculate the whole SCTP
checksum in software, which is very expensive.
Also, similarly as in TCP, take into account when a private
helper mangled the packet. In that case, we also need to
recalculate the checksum even if ports might be same.
Thanks for feedback regarding skb->ip_summed checks from
Julian Anastasov; here's a discussion on these checks for
snat and dnat:
* For snat_handler(), we can see CHECKSUM_PARTIAL from
virtual devices, and from LOCAL_OUT, otherwise it
should be CHECKSUM_UNNECESSARY. In general, in snat it
is more complex. skb contains the original route and
ip_vs_route_me_harder() can change the route after
snat_handler. So, for locally generated replies from
local server we can not preserve the CHECKSUM_PARTIAL
mode. It is an chicken or egg dilemma: snat_handler
needs the device after rerouting (to check for
NETIF_F_SCTP_CSUM), while ip_route_me_harder() wants
the snat_handler() to put the new saddr for proper
rerouting.
* For dnat_handler(), we should not see CHECKSUM_COMPLETE
for SCTP, in fact the small set of drivers that support
SCTP offloading return CHECKSUM_UNNECESSARY on correctly
received SCTP csum. We can see CHECKSUM_PARTIAL from
local stack or received from virtual drivers. The idea is
that SCTP decides to avoid csum calculation if hardware
supports offloading. IPVS can change the device after
rerouting to real server but we can preserve the
CHECKSUM_PARTIAL mode if the new device supports
offloading too. This works because skb dst is changed
before dnat_handler and we see the new device. So, checks
in the 'if' part will decide whether it is ok to keep
CHECKSUM_PARTIAL for the output. If the packet was with
CHECKSUM_NONE, hence we deal with unknown checksum. As we
recalculate the sum for IP header in all cases, it should
be safe to use CHECKSUM_UNNECESSARY. We can forward wrong
checksum in this case (without cp->app). In case of
CHECKSUM_UNNECESSARY, the csum was valid on receive.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_proto_sctp.c | 39 +++++++++++++++++++++++++++++------
1 file changed, 33 insertions(+), 6 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
index 9ca7aa0..2f7ea75 100644
--- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
@@ -81,6 +81,7 @@ sctp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
{
sctp_sctphdr_t *sctph;
unsigned int sctphoff = iph->len;
+ bool payload_csum = false;
#ifdef CONFIG_IP_VS_IPV6
if (cp->af == AF_INET6 && iph->fragoffs)
@@ -92,19 +93,31 @@ sctp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
return 0;
if (unlikely(cp->app != NULL)) {
+ int ret;
+
/* Some checks before mangling */
if (pp->csum_check && !pp->csum_check(cp->af, skb, pp))
return 0;
/* Call application helper if needed */
- if (!ip_vs_app_pkt_out(cp, skb))
+ ret = ip_vs_app_pkt_out(cp, skb);
+ if (ret == 0)
return 0;
+ /* ret=2: csum update is needed after payload mangling */
+ if (ret == 2)
+ payload_csum = true;
}
sctph = (void *) skb_network_header(skb) + sctphoff;
- sctph->source = cp->vport;
- sctp_nat_csum(skb, sctph, sctphoff);
+ /* Only update csum if we really have to */
+ if (sctph->source != cp->vport || payload_csum ||
+ skb->ip_summed == CHECKSUM_PARTIAL) {
+ sctph->source = cp->vport;
+ sctp_nat_csum(skb, sctph, sctphoff);
+ } else {
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+ }
return 1;
}
@@ -115,6 +128,7 @@ sctp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
{
sctp_sctphdr_t *sctph;
unsigned int sctphoff = iph->len;
+ bool payload_csum = false;
#ifdef CONFIG_IP_VS_IPV6
if (cp->af == AF_INET6 && iph->fragoffs)
@@ -126,19 +140,32 @@ sctp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
return 0;
if (unlikely(cp->app != NULL)) {
+ int ret;
+
/* Some checks before mangling */
if (pp->csum_check && !pp->csum_check(cp->af, skb, pp))
return 0;
/* Call application helper if needed */
- if (!ip_vs_app_pkt_in(cp, skb))
+ ret = ip_vs_app_pkt_in(cp, skb);
+ if (ret == 0)
return 0;
+ /* ret=2: csum update is needed after payload mangling */
+ if (ret == 2)
+ payload_csum = true;
}
sctph = (void *) skb_network_header(skb) + sctphoff;
- sctph->dest = cp->dport;
- sctp_nat_csum(skb, sctph, sctphoff);
+ /* Only update csum if we really have to */
+ if (sctph->dest != cp->dport || payload_csum ||
+ (skb->ip_summed == CHECKSUM_PARTIAL &&
+ !(skb_dst(skb)->dev->features & NETIF_F_SCTP_CSUM))) {
+ sctph->dest = cp->dport;
+ sctp_nat_csum(skb, sctph, sctphoff);
+ } else if (skb->ip_summed != CHECKSUM_PARTIAL) {
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+ }
return 1;
}
--
1.8.4
^ permalink raw reply related
* Re: [PATCH net-next 04/11] openvswitch: Restructure datapath.c and flow.c
From: Joe Perches @ 2013-10-30 1:25 UTC (permalink / raw)
To: Jesse Gross; +Cc: David Miller, netdev, dev, Pravin B Shelar
In-Reply-To: <1383092544-50599-5-git-send-email-jesse@nicira.com>
On Tue, 2013-10-29 at 17:22 -0700, Jesse Gross wrote:
> Over the time datapath.c and flow.c has became pretty large files.
> Following patch restructures functionality of component into three
> different components:
[]
> net/openvswitch/Makefile | 2 +
> net/openvswitch/datapath.c | 528 +------------
> net/openvswitch/datapath.h | 1 +
> net/openvswitch/flow.c | 1605 +---------------------------------------
> net/openvswitch/flow.h | 128 +---
> net/openvswitch/flow_netlink.c | 1603 +++++++++++++++++++++++++++++++++++++++
Is this largish patch any different or easier to
read/verify when generated with git format-patch -M ?
^ permalink raw reply
* Re: [PATCH net-next 04/11] openvswitch: Restructure datapath.c and flow.c
From: Joe Perches @ 2013-10-30 1:32 UTC (permalink / raw)
To: Jesse Gross
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
David Miller
In-Reply-To: <1383096313.12439.44.camel@joe-AO722>
On Tue, 2013-10-29 at 18:25 -0700, Joe Perches wrote:
> On Tue, 2013-10-29 at 17:22 -0700, Jesse Gross wrote:
> > Over the time datapath.c and flow.c has became pretty large files.
> > Following patch restructures functionality of component into three
> > different components:
> []
> > net/openvswitch/Makefile | 2 +
> > net/openvswitch/datapath.c | 528 +------------
> > net/openvswitch/datapath.h | 1 +
> > net/openvswitch/flow.c | 1605 +---------------------------------------
> > net/openvswitch/flow.h | 128 +---
> > net/openvswitch/flow_netlink.c | 1603 +++++++++++++++++++++++++++++++++++++++
>
> Is this largish patch any different or easier to
> read/verify when generated with git format-patch -M ?
After a git pull, the short answer is no.
Oh well.
^ permalink raw reply
* Re: [PATCH net-next] tipc: remove two indentation levels in tipc_recv_msg routine
From: Ying Xue @ 2013-10-30 1:34 UTC (permalink / raw)
To: David Laight, Andreas Bofjäll, Erik Hugne
Cc: jon.maloy, netdev, tipc-discussion, davem, Paul.Gortmaker
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B73BD@saturn3.aculab.com>
On 10/29/2013 08:42 PM, David Laight wrote:
>> continue;
>> +
>> +unlock:
>> + tipc_node_unlock(n_ptr);
>> cont:
>> kfree_skb(buf);
>> }
>
> Might be better to call the labels 'unlock_discard' and 'discard'.
>
Thanks for your valuable suggestions. I will submit next version soon.
Regards,
Ying
> David
>
>
>
> --
> 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
>
>
------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
^ permalink raw reply
* Re: Bug in skb_segment: fskb->len != len
From: Herbert Xu @ 2013-10-30 1:50 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Christoph Paasch, netdev
In-Reply-To: <1383059293.5464.31.camel@edumazet-glaptop.roam.corp.google.com>
On Tue, Oct 29, 2013 at 08:08:13AM -0700, Eric Dumazet wrote:
>
> GRO layer was updated to be able to stack two or three sk_buff,
> fully populated with page frags.
>
> Thats quite mandatory to support line rate for 40Gb links.
Indeed I missed this. Which commit introduced this change?
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v2 net-next] net: introduce gro_frag_list_enable sysctl
From: David Miller @ 2013-10-30 2:02 UTC (permalink / raw)
To: eric.dumazet; +Cc: christoph.paasch, herbert, netdev, hkchu, mwdalton
In-Reply-To: <1383094428.4857.16.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 29 Oct 2013 17:53:48 -0700
> So should we apply the first fix to avoid the BUG_ON() ?
Please be more specific, are you talking about splitting up
this patch in some way?
^ permalink raw reply
* Re: [PATCH v2 net-next] net: introduce gro_frag_list_enable sysctl
From: Herbert Xu @ 2013-10-30 2:05 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, christoph.paasch, netdev, hkchu, mwdalton
In-Reply-To: <20131029.220253.1263087684709722001.davem@davemloft.net>
On Tue, Oct 29, 2013 at 10:02:53PM -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 29 Oct 2013 17:53:48 -0700
>
> > So should we apply the first fix to avoid the BUG_ON() ?
>
> Please be more specific, are you talking about splitting up
> this patch in some way?
I think Eric is referring to the patch that removes the BUG_ON
in skb_segment and deals with the new mega-GRO packets.
I think that's fine for stable, but for the long term we should
fix it properly as these new meag-GRO packets still retain the
existing packet boundaries and are trivially segmentable.
If we are indeed able to do that, I doubt we would even need
the sysctl patch since the GRO performance should be vastly
superior to the non-GRO case, even for a router/bridge.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH net-next 08/11] openvswitch: Per cpu flow stats.
From: David Miller @ 2013-10-30 2:11 UTC (permalink / raw)
To: jesse-l0M0P4e3n4LQT0dZR+AlfA
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1383092544-50599-9-git-send-email-jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
From: Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
Date: Tue, 29 Oct 2013 17:22:21 -0700
> From: Pravin B Shelar <pshelar-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
>
> With mega flow implementation ovs flow can be shared between
> multiple CPUs which makes stats updates highly contended
> operation. Following patch allocates separate stats for each
> CPU to make stats update scalable.
>
> Signed-off-by: Pravin B Shelar <pshelar-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
Please properly use __percpu memory for this.
Using arrays for this is very strongly discouraged, please instead
use the kernel facility designed exactly for this.
^ permalink raw reply
* Re: [PATCH v2 net-next] net: introduce gro_frag_list_enable sysctl
From: Jerry Chu @ 2013-10-30 2:13 UTC (permalink / raw)
To: Herbert Xu
Cc: David Miller, Eric Dumazet, Christoph Paasch,
netdev@vger.kernel.org, Michael Dalton
In-Reply-To: <20131030020543.GA1925@gondor.apana.org.au>
On Tue, Oct 29, 2013 at 7:05 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Tue, Oct 29, 2013 at 10:02:53PM -0400, David Miller wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Tue, 29 Oct 2013 17:53:48 -0700
>>
>> > So should we apply the first fix to avoid the BUG_ON() ?
>>
>> Please be more specific, are you talking about splitting up
>> this patch in some way?
>
> I think Eric is referring to the patch that removes the BUG_ON
> in skb_segment and deals with the new mega-GRO packets.
>
> I think that's fine for stable, but for the long term we should
> fix it properly as these new meag-GRO packets still retain the
> existing packet boundaries and are trivially segmentable.
>
> If we are indeed able to do that, I doubt we would even need
> the sysctl patch since the GRO performance should be vastly
> superior to the non-GRO case, even for a router/bridge.
Probably not the case for the simple forwarding case. See my
test result of some small (5-8%) CPU+throughput penalty from
GRO (over GRE tunnel) posted previously. But I can believe
the number may be very different if the forwarding path involves
more work (NAT, iptables filtering,...,etc) resulting in a higher per
pkt cost.
Best,
Jerry
>
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2013-10-30 2:14 UTC (permalink / raw)
To: David Miller, netdev
Cc: linux-next, linux-kernel, Nikolay Aleksandrov, Joe Perches
[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in
drivers/net/netconsole.c between commit c7c6effdeffc ("netconsole: fix
multiple race conditions") from the net tree and commit 22ded57729e6
("netconsole: Convert to pr_<level>") from the net-next tree.
(Thanks for removing that spare blank line :-))
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc drivers/net/netconsole.c
index c9a15925a1f7,a8ef4c4b94be..000000000000
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@@ -333,18 -336,14 +335,18 @@@ static ssize_t store_enabled(struct net
netpoll_print_options(&nt->np);
err = netpoll_setup(&nt->np);
- if (err) {
- mutex_unlock(&nt->mutex);
+ if (err)
return err;
- }
- printk(KERN_INFO "netconsole: network logging started\n");
+ pr_info("network logging started\n");
-
} else { /* 0 */
+ /* We need to disable the netconsole before cleaning it up
+ * otherwise we might end up in write_msg() with
+ * nt->np.dev == NULL and nt->enabled == 1
+ */
+ spin_lock_irqsave(&target_list_lock, flags);
+ nt->enabled = 0;
+ spin_unlock_irqrestore(&target_list_lock, flags);
netpoll_cleanup(&nt->np);
}
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox