On 22 August 2017 at 08:26, Greg Rose <[email protected]> wrote:
> On 08/18/2017 02:41 PM, Joe Stringer wrote:
>>
>> On 16 August 2017 at 15:48, Greg Rose <[email protected]> wrote:
>> > Upstream commit:
>> >      commit 494bea39f3201776cdfddc232705f54a0bd210c4
>> >      Author: Liping Zhang <[email protected]>
>> >      Date:   Wed Aug 16 13:30:07 2017 +0800
>> >
>> >      For sw_flow_actions, the actions_len only represents the kernel
>> > part's
>> >      size, and when we dump the actions to the userspace, we will do the
>> >      convertions, so it's true size may become bigger than the
>> > actions_len.
>> >
>> >      But unfortunately, for OVS_PACKET_ATTR_ACTIONS, we use the
>> > actions_len
>> >      to alloc the skbuff, so the user_skb's size may become insufficient
>> > and
>> >      oops will happen like this:
>> >        skbuff: skb_over_panic: text:ffffffff8148fabf len:1749 put:157
>> > head:
>> >        ffff881300f39000 data:ffff881300f39000 tail:0x6d5 end:0x6c0
>> > dev:<NULL>
>> >        ------------[ cut here ]------------
>> >        kernel BUG at net/core/skbuff.c:129!
>> >        [...]
>> >        Call Trace:
>> >         <IRQ>
>> >         [<ffffffff8148be82>] skb_put+0x43/0x44
>> >         [<ffffffff8148fabf>] skb_zerocopy+0x6c/0x1f4
>> >         [<ffffffffa0290d36>] queue_userspace_packet+0x3a3/0x448
>> > [openvswitch]
>> >         [<ffffffffa0292023>] ovs_dp_upcall+0x30/0x5c [openvswitch]
>> >         [<ffffffffa028d435>] output_userspace+0x132/0x158 [openvswitch]
>> >         [<ffffffffa01e6890>] ? ip6_rcv_finish+0x74/0x77 [ipv6]
>> >         [<ffffffffa028e277>] do_execute_actions+0xcc1/0xdc8
>> > [openvswitch]
>> >         [<ffffffffa028e3f2>] ovs_execute_actions+0x74/0x106
>> > [openvswitch]
>> >         [<ffffffffa0292130>] ovs_dp_process_packet+0xe1/0xfd
>> > [openvswitch]
>> >         [<ffffffffa0292b77>] ? key_extract+0x63c/0x8d5 [openvswitch]
>> >         [<ffffffffa029848b>] ovs_vport_receive+0xa1/0xc3 [openvswitch]
>> >        [...]
>> >
>> >      Also we can find that the actions_len is much little than the
>> > orig_len:
>> >        crash> struct sw_flow_actions 0xffff8812f539d000
>> >        struct sw_flow_actions {
>> >          rcu = {
>> >            next = 0xffff8812f5398800,
>> >            func = 0xffffe3b00035db32
>> >          },
>> >          orig_len = 1384,
>> >          actions_len = 592,
>> >          actions = 0xffff8812f539d01c
>> >        }
>> >
>> >      So as a quick fix, use the orig_len instead of the actions_len to
>> > alloc
>> >      the user_skb.
>> >
>> >      Last, this oops happened on our system running a relative old
>> > kernel, but
>> >      the same risk still exists on the mainline, since we use the wrong
>> >      actions_len from the beginning.
>> >
>> >      Fixes: 0e469d3b380c ("datapath: include datapath actions with
>> > sampled-"...)
>> >      Cc: Neil McKee <[email protected]>
>> >      Signed-off-by: Liping Zhang <[email protected]>
>> >      Acked-by: Pravin B Shelar <[email protected]>
>> >      Signed-off-by: David S. Miller <[email protected]>
>> >
>> > Signed-off-by: Greg Rose <[email protected]>
>> > ---
>>
>> Thanks for the backport.
>>
>> It seems like we're a bit diverged from the latest net-next, if I
>> follow correctly these patches haven't been backported yet. Would you
>> mind preparing these, too?
>>
>> 0ed80da518a1 openvswitch: Remove unnecessary newlines from OVS_NLERR uses
>> c4b2bf6b4a35 openvswitch: Optimize operations for OvS flow_stats.
>> c57c054eb5b1 openvswitch: Optimize updating for OvS flow_stats.
>> 880388aa3c07 net: Remove all references to SKB_GSO_UDP.
>>
>> I realise that this particular patch is a long-standing bug that we
>> can fix (and will need backporting in our tree), but it's nice if we
>> can keep the patches applied to master here in the same order as
>> upstream net-next so that it's easier to tell how out of sync we are
>> by a side-by-side comparison of "git log --oneline" for datapath/ or
>> net/openvswitch in the corresponding trees. I also realise that this
>> patch didn't land in net-next yet since it's so new, but it's fairly
>> safe to assume it'll be applied there next.
>>
>> Cheers,
>> Joe
>>
> OK - I had been waiting for patches to hit the net tree before backporting.
> I'll get the ones in net-next too and submit a patch series.

Thanks! I'll keep an eye out for them.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to