From aea0bb4f8ee513537ad84b9f3f609f96e272d98e Mon Sep 17 00:00:00 2001 From: Thomas Graf Date: Tue, 14 Jan 2014 16:27:49 +0000 Subject: openvswitch: Pad OVS_PACKET_ATTR_PACKET if linear copy was performed While the zerocopy method is correctly omitted if user space does not support unaligned Netlink messages. The attribute is still not padded correctly as skb_zerocopy() will not ensure padding and the attribute size is no longer pre calculated though nla_reserve() which ensured padding previously. This patch applies appropriate padding if a linear data copy was performed in skb_zerocopy(). Signed-off-by: Thomas Graf Acked-by: Zoltan Kiss Signed-off-by: Jesse Gross --- net/openvswitch/datapath.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'net/openvswitch') diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c index df4692826ead..d1a73a6102f8 100644 --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -466,6 +466,14 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb, skb_zerocopy(user_skb, skb, skb->len, hlen); + /* Pad OVS_PACKET_ATTR_PACKET if linear copy was performed */ + if (!(dp->user_features & OVS_DP_F_UNALIGNED)) { + size_t plen = NLA_ALIGN(user_skb->len) - user_skb->len; + + if (plen > 0) + memset(skb_put(user_skb, plen), 0, plen); + } + ((struct nlmsghdr *) user_skb->data)->nlmsg_len = user_skb->len; err = genlmsg_unicast(ovs_dp_get_net(dp), user_skb, upcall_info->portid); -- cgit v1.2.3 From e80857cce82da31e41a6599fc888dfc92e0167cc Mon Sep 17 00:00:00 2001 From: Andy Zhou Date: Tue, 21 Jan 2014 09:31:04 -0800 Subject: openvswitch: Fix kernel panic on ovs_flow_free Both mega flow mask's reference counter and per flow table mask list should only be accessed when holding ovs_mutex() lock. However this is not true with ovs_flow_table_flush(). The patch fixes this bug. Reported-by: Joe Stringer Signed-off-by: Andy Zhou Signed-off-by: Jesse Gross --- net/openvswitch/datapath.c | 9 +++-- net/openvswitch/flow_table.c | 84 +++++++++++++++++++++----------------------- net/openvswitch/flow_table.h | 2 +- 3 files changed, 47 insertions(+), 48 deletions(-) (limited to 'net/openvswitch') diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c index d1a73a6102f8..e1b337e0bf4d 100644 --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -55,6 +55,7 @@ #include "datapath.h" #include "flow.h" +#include "flow_table.h" #include "flow_netlink.h" #include "vport-internal_dev.h" #include "vport-netdev.h" @@ -160,7 +161,6 @@ static void destroy_dp_rcu(struct rcu_head *rcu) { struct datapath *dp = container_of(rcu, struct datapath, rcu); - ovs_flow_tbl_destroy(&dp->table); free_percpu(dp->stats_percpu); release_net(ovs_dp_get_net(dp)); kfree(dp->ports); @@ -1287,7 +1287,7 @@ err_destroy_ports_array: err_destroy_percpu: free_percpu(dp->stats_percpu); err_destroy_table: - ovs_flow_tbl_destroy(&dp->table); + ovs_flow_tbl_destroy(&dp->table, false); err_free_dp: release_net(ovs_dp_get_net(dp)); kfree(dp); @@ -1314,10 +1314,13 @@ static void __dp_destroy(struct datapath *dp) list_del_rcu(&dp->list_node); /* OVSP_LOCAL is datapath internal port. We need to make sure that - * all port in datapath are destroyed first before freeing datapath. + * all ports in datapath are destroyed first before freeing datapath. */ ovs_dp_detach_port(ovs_vport_ovsl(dp, OVSP_LOCAL)); + /* RCU destroy the flow table */ + ovs_flow_tbl_destroy(&dp->table, true); + call_rcu(&dp->rcu, destroy_dp_rcu); } diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c index c58a0fe3c889..bd14052ed342 100644 --- a/net/openvswitch/flow_table.c +++ b/net/openvswitch/flow_table.c @@ -153,29 +153,27 @@ static void rcu_free_flow_callback(struct rcu_head *rcu) flow_free(flow); } -static void flow_mask_del_ref(struct sw_flow_mask *mask, bool deferred) +void ovs_flow_free(struct sw_flow *flow, bool deferred) { - if (!mask) + if (!flow) return; - BUG_ON(!mask->ref_count); - mask->ref_count--; + ASSERT_OVSL(); - if (!mask->ref_count) { - list_del_rcu(&mask->list); - if (deferred) - kfree_rcu(mask, rcu); - else - kfree(mask); - } -} + if (flow->mask) { + struct sw_flow_mask *mask = flow->mask; -void ovs_flow_free(struct sw_flow *flow, bool deferred) -{ - if (!flow) - return; + BUG_ON(!mask->ref_count); + mask->ref_count--; - flow_mask_del_ref(flow->mask, deferred); + if (!mask->ref_count) { + list_del_rcu(&mask->list); + if (deferred) + kfree_rcu(mask, rcu); + else + kfree(mask); + } + } if (deferred) call_rcu(&flow->rcu, rcu_free_flow_callback); @@ -188,26 +186,9 @@ static void free_buckets(struct flex_array *buckets) flex_array_free(buckets); } + static void __table_instance_destroy(struct table_instance *ti) { - int i; - - if (ti->keep_flows) - goto skip_flows; - - for (i = 0; i < ti->n_buckets; i++) { - struct sw_flow *flow; - struct hlist_head *head = flex_array_get(ti->buckets, i); - struct hlist_node *n; - int ver = ti->node_ver; - - hlist_for_each_entry_safe(flow, n, head, hash_node[ver]) { - hlist_del(&flow->hash_node[ver]); - ovs_flow_free(flow, false); - } - } - -skip_flows: free_buckets(ti->buckets); kfree(ti); } @@ -258,20 +239,38 @@ static void flow_tbl_destroy_rcu_cb(struct rcu_head *rcu) static void table_instance_destroy(struct table_instance *ti, bool deferred) { + int i; + if (!ti) return; + if (ti->keep_flows) + goto skip_flows; + + for (i = 0; i < ti->n_buckets; i++) { + struct sw_flow *flow; + struct hlist_head *head = flex_array_get(ti->buckets, i); + struct hlist_node *n; + int ver = ti->node_ver; + + hlist_for_each_entry_safe(flow, n, head, hash_node[ver]) { + hlist_del_rcu(&flow->hash_node[ver]); + ovs_flow_free(flow, deferred); + } + } + +skip_flows: if (deferred) call_rcu(&ti->rcu, flow_tbl_destroy_rcu_cb); else __table_instance_destroy(ti); } -void ovs_flow_tbl_destroy(struct flow_table *table) +void ovs_flow_tbl_destroy(struct flow_table *table, bool deferred) { struct table_instance *ti = ovsl_dereference(table->ti); - table_instance_destroy(ti, false); + table_instance_destroy(ti, deferred); } struct sw_flow *ovs_flow_tbl_dump_next(struct table_instance *ti, @@ -504,16 +503,11 @@ static struct sw_flow_mask *mask_alloc(void) mask = kmalloc(sizeof(*mask), GFP_KERNEL); if (mask) - mask->ref_count = 0; + mask->ref_count = 1; return mask; } -static void mask_add_ref(struct sw_flow_mask *mask) -{ - mask->ref_count++; -} - static bool mask_equal(const struct sw_flow_mask *a, const struct sw_flow_mask *b) { @@ -554,9 +548,11 @@ static int flow_mask_insert(struct flow_table *tbl, struct sw_flow *flow, mask->key = new->key; mask->range = new->range; list_add_rcu(&mask->list, &tbl->mask_list); + } else { + BUG_ON(!mask->ref_count); + mask->ref_count++; } - mask_add_ref(mask); flow->mask = mask; return 0; } diff --git a/net/openvswitch/flow_table.h b/net/openvswitch/flow_table.h index 1996e34c0fd8..baaeb101924d 100644 --- a/net/openvswitch/flow_table.h +++ b/net/openvswitch/flow_table.h @@ -60,7 +60,7 @@ 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); +void ovs_flow_tbl_destroy(struct flow_table *table, bool deferred); int ovs_flow_tbl_flush(struct flow_table *flow_table); int ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow, -- cgit v1.2.3 From 45fb9c35b27c9982e9a55d04ed0a5230a2d0b306 Mon Sep 17 00:00:00 2001 From: Daniele Di Proietto Date: Thu, 23 Jan 2014 10:47:35 -0800 Subject: openvswitch: Fix ovs_dp_cmd_msg_size() commit 43d4be9cb55f3bac5253e9289996fd9d735531db (openvswitch: Allow user space to announce ability to accept unaligned Netlink messages) introduced OVS_DP_ATTR_USER_FEATURES netlink attribute in datapath responses, but the attribute size was not taken into account in ovs_dp_cmd_msg_size(). Signed-off-by: Daniele Di Proietto Signed-off-by: Jesse Gross --- net/openvswitch/datapath.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net/openvswitch') diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c index e1b337e0bf4d..58689dda8377 100644 --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -1087,6 +1087,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)); + msgsize += nla_total_size(sizeof(u32)); /* OVS_DP_ATTR_USER_FEATURES */ return msgsize; } -- cgit v1.2.3 From e4c6d7595403e943a3afc334eb8c0efcd840043a Mon Sep 17 00:00:00 2001 From: Pravin B Shelar Date: Fri, 31 Jan 2014 09:43:23 -0800 Subject: openvswitch: Fix ovs_flow_free() ovs-lock assert. ovs_flow_free() is not called under ovs-lock during packet execute path (ovs_packet_cmd_execute()). Since packet execute does not touch flow->mask, there is no need to take that lock either. So move assert in case where flow->mask is checked. Found by code inspection. Signed-off-by: Pravin B Shelar Signed-off-by: Jesse Gross --- net/openvswitch/flow_table.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'net/openvswitch') diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c index bd14052ed342..3c268b3d71c3 100644 --- a/net/openvswitch/flow_table.c +++ b/net/openvswitch/flow_table.c @@ -158,11 +158,13 @@ void ovs_flow_free(struct sw_flow *flow, bool deferred) if (!flow) return; - ASSERT_OVSL(); - if (flow->mask) { struct sw_flow_mask *mask = flow->mask; + /* ovs-lock is required to protect mask-refcount and + * mask list. + */ + ASSERT_OVSL(); BUG_ON(!mask->ref_count); mask->ref_count--; -- cgit v1.2.3 From c14e0953ca51dbcb8d1ac92acbdcff23d0caa158 Mon Sep 17 00:00:00 2001 From: Andy Zhou Date: Sun, 2 Feb 2014 17:08:06 -0800 Subject: openvswitch: Suppress error messages on megaflow updates With subfacets, we'd expect megaflow updates message to carry the original micro flow. If not, EINVAL is returned and kernel logs an error message. Now that the user space subfacet layer is removed, it is expected that flow updates can arrive with a micro flow other than the original. Change the return code to EEXIST and remove the kernel error log message. Reported-by: Ben Pfaff Signed-off-by: Andy Zhou Signed-off-by: Jesse Gross --- net/openvswitch/datapath.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'net/openvswitch') diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c index 58689dda8377..e9a48baf8551 100644 --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -860,11 +860,8 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info) goto err_unlock_ovs; /* The unmasked key has to be the same for flow updates. */ - error = -EINVAL; - if (!ovs_flow_cmp_unmasked_key(flow, &match)) { - OVS_NLERR("Flow modification message rejected, unmasked key does not match.\n"); + if (!ovs_flow_cmp_unmasked_key(flow, &match)) goto err_unlock_ovs; - } /* Update actions. */ old_acts = ovsl_dereference(flow->sf_acts); -- cgit v1.2.3