Thomas, is there a reason not to consider this patch as-is right now? It
seems like a straight-forward code cleanup with no direct association with
the later patches.
--Justin
On Fri, Sep 26, 2014 at 3:00 PM, Thomas Graf <tg...@noironetworks.com>
wrote:
> Signed-off-by: Thomas Graf <tg...@noironetworks.com>
> ---
> datapath/datapath.c | 8 ++++----
> datapath/flow_netlink.c | 14 ++++++++++----
> datapath/flow_netlink.h | 1 +
> datapath/flow_table.c | 3 ++-
> 4 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index 45e7c56..61d6c0f 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -966,7 +966,7 @@ static int ovs_flow_cmd_new(struct sk_buff *skb,
> struct genl_info *info)
> }
> ovs_unlock();
>
> - ovs_nla_free_flow_actions(old_acts);
> + ovs_nla_free_flow_actions_rcu(old_acts);
> ovs_flow_free(new_flow, false);
> }
>
> @@ -978,7 +978,7 @@ err_unlock_ovs:
> ovs_unlock();
> kfree_skb(reply);
> err_kfree_acts:
> - kfree(acts);
> + ovs_nla_free_flow_actions(acts);
> err_kfree_flow:
> ovs_flow_free(new_flow, false);
> error:
> @@ -1091,14 +1091,14 @@ static int ovs_flow_cmd_set(struct sk_buff *skb,
> struct genl_info *info)
> if (reply)
> ovs_notify(&dp_flow_genl_family,
> &ovs_dp_flow_multicast_group, reply, info);
> if (old_acts)
> - ovs_nla_free_flow_actions(old_acts);
> + ovs_nla_free_flow_actions_rcu(old_acts);
> return 0;
>
> err_unlock_ovs:
> ovs_unlock();
> kfree_skb(reply);
> err_kfree_acts:
> - kfree(acts);
> + ovs_nla_free_flow_actions(acts);
> error:
> return error;
> }
> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
> index a3f34f1..ecb6631 100644
> --- a/datapath/flow_netlink.c
> +++ b/datapath/flow_netlink.c
> @@ -1338,17 +1338,23 @@ static struct sw_flow_actions
> *nla_alloc_flow_actions(int size)
> return sfa;
> }
>
> -/* RCU callback used by ovs_nla_free_flow_actions. */
> +void ovs_nla_free_flow_actions(struct sw_flow_actions *sf_acts)
> +{
> + kfree(sf_acts);
> +}
> +
> +/* RCU callback used by ovs_nla_free_flow_actions_rcu. */
> static void rcu_free_acts_callback(struct rcu_head *rcu)
> {
> struct sw_flow_actions *sf_acts = container_of(rcu,
> struct sw_flow_actions, rcu);
> - kfree(sf_acts);
> +
> + ovs_nla_free_flow_actions(sf_acts);
> }
>
> /* Schedules 'sf_acts' to be freed after the next RCU grace period.
> * The caller must hold rcu_read_lock for this to be sensible. */
> -void ovs_nla_free_flow_actions(struct sw_flow_actions *sf_acts)
> +void ovs_nla_free_flow_actions_rcu(struct sw_flow_actions *sf_acts)
> {
> call_rcu(&sf_acts->rcu, rcu_free_acts_callback);
> }
> @@ -1885,7 +1891,7 @@ int ovs_nla_copy_actions(const struct nlattr *attr,
> err = __ovs_nla_copy_actions(attr, key, 0, sfa, key->eth.type,
> key->eth.tci);
> if (err)
> - kfree(*sfa);
> + ovs_nla_free_flow_actions(*sfa);
>
> return err;
> }
> diff --git a/datapath/flow_netlink.h b/datapath/flow_netlink.h
> index 90bbe37..d1802c4 100644
> --- a/datapath/flow_netlink.h
> +++ b/datapath/flow_netlink.h
> @@ -60,5 +60,6 @@ int ovs_nla_put_actions(const struct nlattr *attr,
> int len, struct sk_buff *skb);
>
> void ovs_nla_free_flow_actions(struct sw_flow_actions *);
> +void ovs_nla_free_flow_actions_rcu(struct sw_flow_actions *);
>
> #endif /* flow_netlink.h */
> diff --git a/datapath/flow_table.c b/datapath/flow_table.c
> index 4efef13..a5ce275 100644
> --- a/datapath/flow_table.c
> +++ b/datapath/flow_table.c
> @@ -45,6 +45,7 @@
> #include <net/ndisc.h>
>
> #include "vlan.h"
> +#include "flow_netlink.h"
>
> #define TBL_MIN_BUCKETS 1024
> #define MASK_ARRAY_SIZE_MIN 16
> @@ -146,7 +147,7 @@ static void flow_free(struct sw_flow *flow)
> {
> int node;
>
> - kfree((struct sw_flow_actions __force *)flow->sf_acts);
> + ovs_nla_free_flow_actions((struct sw_flow_actions __force
> *)flow->sf_acts);
> for_each_node(node)
> if (flow->stats[node])
> kmem_cache_free(flow_stats_cache,
> --
> 1.9.3
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
