Thanks Pravin for the review. I have pushed all 3 patches with the
suggested changes. Also merged the first patch into branch 2.3.
On Thu, Jul 10, 2014 at 2:04 PM, Pravin Shelar <pshe...@nicira.com> wrote:
> On Thu, Jul 10, 2014 at 1:16 AM, Andy Zhou <az...@nicira.com> wrote:
>> skb_clone() NULL check is implemented in do_output(), as past of the
>> common (fast) path. Refactoring so that NULL check is done in the
>> slow path, immediately after skb_clone() is called.
>>
>> Besides optimization, this change also improves code readability by
>> making the skb_clone() NULL check consistent within OVS datapath
>> module.
>>
>> Signed-off-by: Andy Zhou <az...@nicira.com>
>> ---
>> datapath/actions.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/datapath/actions.c b/datapath/actions.c
>> index daa7b43..6eb83b8 100644
>> --- a/datapath/actions.c
>> +++ b/datapath/actions.c
>> @@ -490,9 +490,6 @@ static int do_output(struct datapath *dp, struct sk_buff
>> *skb, int out_port)
>> {
>> struct vport *vport;
>>
>> - if (unlikely(!skb))
>> - return -ENOMEM;
>> -
> There is no check for do_output() return, Therefore we can make it return
> void.
>
>
>> vport = ovs_vport_rcu(dp, out_port);
>> if (unlikely(!vport)) {
>> kfree_skb(skb);
>> @@ -693,7 +690,12 @@ static int do_execute_actions(struct datapath *dp,
>> struct sk_buff *skb,
>> int err = 0;
>>
>> if (prev_port != -1) {
> This can be marked as unlikely
>
>> - do_output(dp, skb_clone(skb, GFP_ATOMIC), prev_port);
>> + struct sk_buff *out_skb = skb_clone(skb, GFP_ATOMIC);
>> +
>> + if (!out_skb)
>> + return -ENOMEM;
>> +
> This changes behavior, we can just skip do_output if it skb_clone() fails.
>
>> + do_output(dp, out_skb, prev_port);
>> prev_port = -1;
>> }
>>
> otherwise looks good.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
