netdev - Re: [net-next] openvswitch: Simplify do_execute

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CABKoBm3=MGv+sdL6T3=2_=pTh+Wukp66ZdDUx99tD7t482nodA@mail.gmail.com>
Date:   Fri, 27 Jan 2017 13:25:52 -0800
From:   Andy Zhou <azhou@....org>
To:     Pravin Shelar <pshelar@....org>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [net-next] openvswitch: Simplify do_execute_actions().

On Fri, Jan 27, 2017 at 12:42 PM, Pravin Shelar <pshelar@....org> wrote:
> On Wed, Jan 25, 2017 at 9:24 PM, Andy Zhou <azhou@....org> wrote:
>> do_execute_actions() implements a worthwhile optimization: in case
>> an output action is the last action in an action list, skb_clone()
>> can be avoided by outputing the current skb. However, the
>> implementation is more complicated than necessary.  This patch
>> simplify this logic.
>>
>> Signed-off-by: Andy Zhou <azhou@....org>
>> ---
>>  net/openvswitch/actions.c | 40 +++++++++++++++++++---------------------
>>  1 file changed, 19 insertions(+), 21 deletions(-)
>>
>> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
>> index 514f7bc..3866608 100644
>> --- a/net/openvswitch/actions.c
>> +++ b/net/openvswitch/actions.c
>> @@ -830,6 +830,9 @@ static void do_output(struct datapath *dp, struct sk_buff *skb, int out_port,
>>  {
>>         struct vport *vport = ovs_vport_rcu(dp, out_port);
>>
>> +       if (unlikely(!skb))
>> +               return;
>> +
> Patch looks good to me. But I wanted to know if you considered moving
> this check to do_execute_actions() in case skb-clone is done? This way
> we can avoid this unlikely check from likely case :)
>
Good point.  O.K. I will repost a version without this check.  Thanks
for the review and comment.