On Thu, Jun 19, 2014 at 1:31 PM, Pravin Shelar <pshe...@nicira.com> wrote:
> Git am warning.
>
> Applying: datapath: Factor out allocation and verification of actions.
>
> /home/pravin/ovs/w7/.git/rebase-apply/patch:64: trailing whitespace.
>
> warning: 1 line adds whitespace errors.
>
> On Tue, Jun 10, 2014 at 4:47 PM, Jesse Gross <je...@nicira.com> wrote:
>> As the size of the flow key grows, it can put some pressure on the
>> stack. This is particularly true in ovs_flow_cmd_set(), which needs several
>> copies of the key on the stack. One of those uses is logically separate,
>> so this factors it out to reduce stack pressure and improve readibility.
>>
>> Signed-off-by: Jesse Gross <je...@nicira.com>
>> ---
>> datapath/datapath.c | 38 ++++++++++++++++++++++++++------------
>> 1 file changed, 26 insertions(+), 12 deletions(-)
>>
>> diff --git a/datapath/datapath.c b/datapath/datapath.c
>> index 81ecc0f..305936a 100644
>> --- a/datapath/datapath.c
>> +++ b/datapath/datapath.c
>> @@ -952,11 +952,34 @@ 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, 0, &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;
>> struct ovs_header *ovs_header = info->userhdr;
>> - struct sw_flow_key key, masked_key;
>> + struct sw_flow_key key;
>> struct sw_flow *flow;
>> struct sw_flow_mask mask;
>> struct sk_buff *reply = NULL;
>> @@ -978,20 +1001,11 @@ static int ovs_flow_cmd_set(struct sk_buff *skb,
>> struct genl_info *info)
>>
>> /* Validate actions. */
>> if (a[OVS_FLOW_ATTR_ACTIONS]) {
>> - acts =
>> ovs_nla_alloc_flow_actions(nla_len(a[OVS_FLOW_ATTR_ACTIONS]));
>> - error = PTR_ERR(acts);
>> + acts = get_flow_actions(a[OVS_FLOW_ATTR_ACTIONS], &key,
>> &mask);
>
> Need to set error before jumping to error.
>> if (IS_ERR(acts))
>> goto error;
>> -
>> - ovs_flow_mask_key(&masked_key, &key, &mask);
>> - error = ovs_nla_copy_actions(a[OVS_FLOW_ATTR_ACTIONS],
>> - &masked_key, 0, &acts);
>> - if (error) {
>> - OVS_NLERR("Flow actions may not be safe on all
>> matching packets.\n");
>> - goto err_kfree_acts;
>> - }
>> }
>> -
>> +
>> /* Can allocate before locking if have acts. */
>> if (acts) {
>> reply = ovs_flow_cmd_alloc_info(acts, info, false);
>> --
>
> Otherwise Looks good.
>
> Acked-by: Pravin B Shelar <pshe...@nicira.com>
Both issues fixed, thanks.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
