On Thu, Aug 7, 2014 at 2:58 PM, Jarno Rajahalme <jrajaha...@nicira.com> wrote:
> Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
>
I pushed it to master.
Thanks.
> On Aug 7, 2014, at 12:51 PM, Pravin B Shelar <pshe...@nicira.com> wrote:
>
>> There are two separate API to allocate and copy actions list. Anytime
>> OVS needs to copy action list, it needs to call both functions.
>> Following patch moves action allocation to copy function to avoid
>> code duplication.
>>
>> Signed-off-by: Pravin B Shelar <pshe...@nicira.com>
>> ---
>> datapath/datapath.c | 49
>> +++++++++----------------------------------------
>> datapath/flow_netlink.c | 24 +++++++++++++++++-------
>> datapath/flow_netlink.h | 1 -
>> 3 files changed, 26 insertions(+), 48 deletions(-)
>>
>> diff --git a/datapath/datapath.c b/datapath/datapath.c
>> index 19d41c8..62e3c26 100644
>> --- a/datapath/datapath.c
>> +++ b/datapath/datapath.c
>> @@ -569,17 +569,12 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb,
>> struct genl_info *info)
>> if (err)
>> goto err_flow_free;
>>
>> - acts = ovs_nla_alloc_flow_actions(nla_len(a[OVS_PACKET_ATTR_ACTIONS]));
>> - err = PTR_ERR(acts);
>> - if (IS_ERR(acts))
>> - goto err_flow_free;
>> -
>> err = ovs_nla_copy_actions(a[OVS_PACKET_ATTR_ACTIONS],
>> &flow->key, &acts);
>> - rcu_assign_pointer(flow->sf_acts, acts);
>> if (err)
>> goto err_flow_free;
>>
>> + rcu_assign_pointer(flow->sf_acts, acts);
>> OVS_CB(packet)->flow = flow;
>> OVS_CB(packet)->pkt_key = &flow->key;
>> OVS_CB(skb)->egress_tun_info = NULL;
>> @@ -899,11 +894,6 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct
>> genl_info *info)
>> ovs_flow_mask_key(&new_flow->key, &new_flow->unmasked_key, &mask);
>>
>> /* Validate actions. */
>> - acts = ovs_nla_alloc_flow_actions(nla_len(a[OVS_FLOW_ATTR_ACTIONS]));
>> - error = PTR_ERR(acts);
>> - if (IS_ERR(acts))
>> - goto err_kfree_flow;
>> -
>> error = ovs_nla_copy_actions(a[OVS_FLOW_ATTR_ACTIONS], &new_flow->key,
>> &acts);
>> if (error) {
>> @@ -1000,29 +990,6 @@ error:
>> return error;
>> }
>>
>> -static struct sw_flow_actions *get_flow_actions(const struct nlattr *a,
>> - const struct sw_flow_key *key,
>> - const struct sw_flow_mask
>> *mask)
>> -{
>> - struct sw_flow_actions *acts;
>> - struct sw_flow_key masked_key;
>> - int error;
>> -
>> - acts = ovs_nla_alloc_flow_actions(nla_len(a));
>> - if (IS_ERR(acts))
>> - return acts;
>> -
>> - ovs_flow_mask_key(&masked_key, key, mask);
>> - error = ovs_nla_copy_actions(a, &masked_key, &acts);
>> - if (error) {
>> - OVS_NLERR("Flow actions may not be safe on all matching
>> packets.\n");
>> - kfree(acts);
>> - return ERR_PTR(error);
>> - }
>> -
>> - return acts;
>> -}
>> -
>> static int ovs_flow_cmd_set(struct sk_buff *skb, struct genl_info *info)
>> {
>> struct nlattr **a = info->attrs;
>> @@ -1051,15 +1018,17 @@ static int ovs_flow_cmd_set(struct sk_buff *skb,
>> struct genl_info *info)
>>
>> /* Validate actions. */
>> if (a[OVS_FLOW_ATTR_ACTIONS]) {
>> - acts = get_flow_actions(a[OVS_FLOW_ATTR_ACTIONS], &key, &mask);
>> - if (IS_ERR(acts)) {
>> - error = PTR_ERR(acts);
>> + struct sw_flow_key masked_key;
>> +
>> + ovs_flow_mask_key(&masked_key, &key, &mask);
>> + error = ovs_nla_copy_actions(a[OVS_FLOW_ATTR_ACTIONS],
>> + &masked_key, &acts);
>> + if (error) {
>> + OVS_NLERR("Flow actions may not be safe on all
>> matching packets.\n");
>> goto error;
>> }
>> - }
>>
>> - /* Can allocate before locking if have acts. */
>> - if (acts) {
>> + /* Can allocate before locking if have acts. */
>> reply = ovs_flow_cmd_alloc_info(acts, info, false);
>> if (IS_ERR(reply)) {
>> error = PTR_ERR(reply);
>> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
>> index 75172de..e4cf535 100644
>> --- a/datapath/flow_netlink.c
>> +++ b/datapath/flow_netlink.c
>> @@ -1239,7 +1239,7 @@ nla_put_failure:
>>
>> #define MAX_ACTIONS_BUFSIZE (32 * 1024)
>>
>> -struct sw_flow_actions *ovs_nla_alloc_flow_actions(int size)
>> +static struct sw_flow_actions *nla_alloc_flow_actions(int size)
>> {
>> struct sw_flow_actions *sfa;
>>
>> @@ -1293,7 +1293,7 @@ static struct nlattr *reserve_sfa_size(struct
>> sw_flow_actions **sfa,
>> new_acts_size = MAX_ACTIONS_BUFSIZE;
>> }
>>
>> - acts = ovs_nla_alloc_flow_actions(new_acts_size);
>> + acts = nla_alloc_flow_actions(new_acts_size);
>> if (IS_ERR(acts))
>> return (void *)acts;
>>
>> @@ -1360,7 +1360,7 @@ static inline void add_nested_action_end(struct
>> sw_flow_actions *sfa,
>> a->nla_len = sfa->actions_len - st_offset;
>> }
>>
>> -static int ovs_nla_copy_actions__(const struct nlattr *attr,
>> +static int __ovs_nla_copy_actions(const struct nlattr *attr,
>> const struct sw_flow_key *key,
>> int depth, struct sw_flow_actions **sfa,
>> __be16 eth_type, __be16 vlan_tci);
>> @@ -1405,7 +1405,7 @@ static int validate_and_copy_sample(const struct
>> nlattr *attr,
>> if (st_acts < 0)
>> return st_acts;
>>
>> - err = ovs_nla_copy_actions__(actions, key, depth + 1, sfa,
>> + err = __ovs_nla_copy_actions(actions, key, depth + 1, sfa,
>> eth_type, vlan_tci);
>> if (err)
>> return err;
>> @@ -1645,7 +1645,7 @@ static int copy_action(const struct nlattr *from,
>> return 0;
>> }
>>
>> -static int ovs_nla_copy_actions__(const struct nlattr *attr,
>> +static int __ovs_nla_copy_actions(const struct nlattr *attr,
>> const struct sw_flow_key *key,
>> int depth, struct sw_flow_actions **sfa,
>> __be16 eth_type, __be16 vlan_tci)
>> @@ -1794,8 +1794,18 @@ int ovs_nla_copy_actions(const struct nlattr *attr,
>> const struct sw_flow_key *key,
>> struct sw_flow_actions **sfa)
>> {
>> - return ovs_nla_copy_actions__(attr, key, 0, sfa, key->eth.type,
>> - key->eth.tci);
>> + int err;
>> +
>> + *sfa = nla_alloc_flow_actions(nla_len(attr));
>> + if (IS_ERR(*sfa))
>> + return PTR_ERR(*sfa);
>> +
>> + err = __ovs_nla_copy_actions(attr, key, 0, sfa, key->eth.type,
>> + key->eth.tci);
>> + if (err)
>> + kfree(*sfa);
>> +
>> + return err;
>> }
>>
>> static int sample_action_to_attr(const struct nlattr *attr, struct sk_buff
>> *skb)
>> diff --git a/datapath/flow_netlink.h b/datapath/flow_netlink.h
>> index 16da264..8ac40b5 100644
>> --- a/datapath/flow_netlink.h
>> +++ b/datapath/flow_netlink.h
>> @@ -54,7 +54,6 @@ int ovs_nla_copy_actions(const struct nlattr *attr,
>> int ovs_nla_put_actions(const struct nlattr *attr,
>> int len, struct sk_buff *skb);
>>
>> -struct sw_flow_actions *ovs_nla_alloc_flow_actions(int actions_len);
>> void ovs_nla_free_flow_actions(struct sw_flow_actions *);
>>
>> #endif /* flow_netlink.h */
>> --
>> 1.9.3
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
