Re: [ovs-dev] [PATCH net v2] net: openvswitch: fix upcall counter access before allocation
Simon Horman writes:
> On Wed, Jun 07, 2023 at 11:09:58AM +0200, Eelco Chaudron wrote:
>
> ...
>
>> >> We moved the per cpu upcall counter allocation to the existing vport
>> >> alloc and free functions to solve this.
>> >>
>> >> Fixes: 95637d91fefd ("net: openvswitch: release vport resources on
>> >> failure")
>> >> Fixes: 1933ea365aa7 ("net: openvswitch: Add support to count upcall
>> >> packets")
>> >> Signed-off-by: Eelco Chaudron
>> >> ---
>> >
>> > Acked-by: Aaron Conole
>>
>> Were you intentionally ACKing this on Aaron’s behalf? Or just a cut/paste
>> error ;)
>
> I was wondering that too.
> But then I concluded it was an artifact of top-posting or some
> other behaviour of the mail client.
Thankfully, I did ack the patch, but yes, this is something to be
careful.
To wangchuanlei , please read
https://people.kernel.org/tglx/
Specifically, do not top-post for this and other reasons.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net v2] net: openvswitch: fix upcall counter access before allocation
On Wed, Jun 07, 2023 at 11:09:58AM +0200, Eelco Chaudron wrote:
...
> >> We moved the per cpu upcall counter allocation to the existing vport
> >> alloc and free functions to solve this.
> >>
> >> Fixes: 95637d91fefd ("net: openvswitch: release vport resources on
> >> failure")
> >> Fixes: 1933ea365aa7 ("net: openvswitch: Add support to count upcall
> >> packets")
> >> Signed-off-by: Eelco Chaudron
> >> ---
> >
> > Acked-by: Aaron Conole
>
> Were you intentionally ACKing this on Aaron’s behalf? Or just a cut/paste
> error ;)
I was wondering that too.
But then I concluded it was an artifact of top-posting or some
other behaviour of the mail client.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net v2] net: openvswitch: fix upcall counter access before allocation
Hello: This patch was applied to netdev/net.git (main) by David S. Miller : On Tue, 6 Jun 2023 13:56:35 +0200 you wrote: > Currently, the per cpu upcall counters are allocated after the vport is > created and inserted into the system. This could lead to the datapath > accessing the counters before they are allocated resulting in a kernel > Oops. > > Here is an example: > > [...] Here is the summary with links: - [net,v2] net: openvswitch: fix upcall counter access before allocation https://git.kernel.org/netdev/net/c/de9df6c6b27e You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net v2] net: openvswitch: fix upcall counter access before allocation
On 7 Jun 2023, at 3:05, wangchuanlei wrote:
> Thanks for fix this, in common enviroment, it's a
> small probability event.
Well, on ARM, they could replicate it a couple of times, but I guess the system
was under memory pressure and has a lot of cores.
>> Eelco Chaudron writes:
>
>> Currently, the per cpu upcall counters are allocated after the vport
>> is created and inserted into the system. This could lead to the
>> datapath accessing the counters before they are allocated resulting in
>> a kernel Oops.
>>
>> Here is an example:
>>
>> PID: 59693TASK: 0005f4f51500 CPU: 0COMMAND: "ovs-vswitchd"
>>#0 [8a39b5b0] __switch_to at b70f0629f2f4
>>#1 [8a39b5d0] __schedule at b70f0629f5cc
>>#2 [8a39b650] preempt_schedule_common at b70f0629fa60
>>#3 [8a39b670] dynamic_might_resched at b70f0629fb58
>>#4 [8a39b680] mutex_lock_killable at b70f062a1388
>>#5 [8a39b6a0] pcpu_alloc at b70f0594460c
>>#6 [8a39b750] __alloc_percpu_gfp at b70f05944e68
>>#7 [8a39b760] ovs_vport_cmd_new at b70ee6961b90 [openvswitch]
>>...
>>
>> PID: 58682TASK: 0005b2f0bf00 CPU: 0COMMAND: "kworker/0:3"
>>#0 [8a5d2f40] machine_kexec at b70f056a0758
>>#1 [8a5d2f70] __crash_kexec at b70f057e2994
>>#2 [8a5d3100] crash_kexec at b70f057e2ad8
>>#3 [8a5d3120] die at b70f0628234c
>>#4 [8a5d31e0] die_kernel_fault at b70f062828a8
>>#5 [8a5d3210] __do_kernel_fault at b70f056a31f4
>>#6 [8a5d3240] do_bad_area at b70f056a32a4
>>#7 [8a5d3260] do_translation_fault at b70f062a9710
>>#8 [8a5d3270] do_mem_abort at b70f056a2f74
>>#9 [8a5d32a0] el1_abort at b70f06297dac
>> #10 [8a5d32d0] el1h_64_sync_handler at b70f06299b24
>> #11 [8a5d3410] el1h_64_sync at b70f056812dc
>> #12 [8a5d3430] ovs_dp_upcall at b70ee6963c84 [openvswitch]
>> #13 [8a5d3470] ovs_dp_process_packet at b70ee6963fdc
>> [openvswitch]
>> #14 [8a5d34f0] ovs_vport_receive at b70ee6972c78 [openvswitch]
>> #15 [8a5d36f0] netdev_port_receive at b70ee6973948
>> [openvswitch]
>> #16 [8a5d3720] netdev_frame_hook at b70ee6973a28 [openvswitch]
>> #17 [8a5d3730] __netif_receive_skb_core.constprop.0 at
>> b70f06079f90
>>
>> We moved the per cpu upcall counter allocation to the existing vport
>> alloc and free functions to solve this.
>>
>> Fixes: 95637d91fefd ("net: openvswitch: release vport resources on
>> failure")
>> Fixes: 1933ea365aa7 ("net: openvswitch: Add support to count upcall
>> packets")
>> Signed-off-by: Eelco Chaudron
>> ---
>
> Acked-by: Aaron Conole
Were you intentionally ACKing this on Aaron’s behalf? Or just a cut/paste error
;)
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net v2] net: openvswitch: fix upcall counter access before allocation
Thanks for fix this, in common enviroment, it's a
small probability event.
> Eelco Chaudron writes:
> Currently, the per cpu upcall counters are allocated after the vport
> is created and inserted into the system. This could lead to the
> datapath accessing the counters before they are allocated resulting in
> a kernel Oops.
>
> Here is an example:
>
> PID: 59693TASK: 0005f4f51500 CPU: 0COMMAND: "ovs-vswitchd"
>#0 [8a39b5b0] __switch_to at b70f0629f2f4
>#1 [8a39b5d0] __schedule at b70f0629f5cc
>#2 [8a39b650] preempt_schedule_common at b70f0629fa60
>#3 [8a39b670] dynamic_might_resched at b70f0629fb58
>#4 [8a39b680] mutex_lock_killable at b70f062a1388
>#5 [8a39b6a0] pcpu_alloc at b70f0594460c
>#6 [8a39b750] __alloc_percpu_gfp at b70f05944e68
>#7 [8a39b760] ovs_vport_cmd_new at b70ee6961b90 [openvswitch]
>...
>
> PID: 58682TASK: 0005b2f0bf00 CPU: 0COMMAND: "kworker/0:3"
>#0 [8a5d2f40] machine_kexec at b70f056a0758
>#1 [8a5d2f70] __crash_kexec at b70f057e2994
>#2 [8a5d3100] crash_kexec at b70f057e2ad8
>#3 [8a5d3120] die at b70f0628234c
>#4 [8a5d31e0] die_kernel_fault at b70f062828a8
>#5 [8a5d3210] __do_kernel_fault at b70f056a31f4
>#6 [8a5d3240] do_bad_area at b70f056a32a4
>#7 [8a5d3260] do_translation_fault at b70f062a9710
>#8 [8a5d3270] do_mem_abort at b70f056a2f74
>#9 [8a5d32a0] el1_abort at b70f06297dac
> #10 [8a5d32d0] el1h_64_sync_handler at b70f06299b24
> #11 [8a5d3410] el1h_64_sync at b70f056812dc
> #12 [8a5d3430] ovs_dp_upcall at b70ee6963c84 [openvswitch]
> #13 [8a5d3470] ovs_dp_process_packet at b70ee6963fdc
> [openvswitch]
> #14 [8a5d34f0] ovs_vport_receive at b70ee6972c78 [openvswitch]
> #15 [8a5d36f0] netdev_port_receive at b70ee6973948 [openvswitch]
> #16 [8a5d3720] netdev_frame_hook at b70ee6973a28 [openvswitch]
> #17 [8a5d3730] __netif_receive_skb_core.constprop.0 at
> b70f06079f90
>
> We moved the per cpu upcall counter allocation to the existing vport
> alloc and free functions to solve this.
>
> Fixes: 95637d91fefd ("net: openvswitch: release vport resources on
> failure")
> Fixes: 1933ea365aa7 ("net: openvswitch: Add support to count upcall
> packets")
> Signed-off-by: Eelco Chaudron
> ---
Acked-by: Aaron Conole
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net v2] net: openvswitch: fix upcall counter access before allocation
Eelco Chaudron writes:
> Currently, the per cpu upcall counters are allocated after the vport is
> created and inserted into the system. This could lead to the datapath
> accessing the counters before they are allocated resulting in a kernel
> Oops.
>
> Here is an example:
>
> PID: 59693TASK: 0005f4f51500 CPU: 0COMMAND: "ovs-vswitchd"
>#0 [8a39b5b0] __switch_to at b70f0629f2f4
>#1 [8a39b5d0] __schedule at b70f0629f5cc
>#2 [8a39b650] preempt_schedule_common at b70f0629fa60
>#3 [8a39b670] dynamic_might_resched at b70f0629fb58
>#4 [8a39b680] mutex_lock_killable at b70f062a1388
>#5 [8a39b6a0] pcpu_alloc at b70f0594460c
>#6 [8a39b750] __alloc_percpu_gfp at b70f05944e68
>#7 [8a39b760] ovs_vport_cmd_new at b70ee6961b90 [openvswitch]
>...
>
> PID: 58682TASK: 0005b2f0bf00 CPU: 0COMMAND: "kworker/0:3"
>#0 [8a5d2f40] machine_kexec at b70f056a0758
>#1 [8a5d2f70] __crash_kexec at b70f057e2994
>#2 [8a5d3100] crash_kexec at b70f057e2ad8
>#3 [8a5d3120] die at b70f0628234c
>#4 [8a5d31e0] die_kernel_fault at b70f062828a8
>#5 [8a5d3210] __do_kernel_fault at b70f056a31f4
>#6 [8a5d3240] do_bad_area at b70f056a32a4
>#7 [8a5d3260] do_translation_fault at b70f062a9710
>#8 [8a5d3270] do_mem_abort at b70f056a2f74
>#9 [8a5d32a0] el1_abort at b70f06297dac
> #10 [8a5d32d0] el1h_64_sync_handler at b70f06299b24
> #11 [8a5d3410] el1h_64_sync at b70f056812dc
> #12 [8a5d3430] ovs_dp_upcall at b70ee6963c84 [openvswitch]
> #13 [8a5d3470] ovs_dp_process_packet at b70ee6963fdc
> [openvswitch]
> #14 [8a5d34f0] ovs_vport_receive at b70ee6972c78 [openvswitch]
> #15 [8a5d36f0] netdev_port_receive at b70ee6973948 [openvswitch]
> #16 [8a5d3720] netdev_frame_hook at b70ee6973a28 [openvswitch]
> #17 [8a5d3730] __netif_receive_skb_core.constprop.0 at
> b70f06079f90
>
> We moved the per cpu upcall counter allocation to the existing vport
> alloc and free functions to solve this.
>
> Fixes: 95637d91fefd ("net: openvswitch: release vport resources on failure")
> Fixes: 1933ea365aa7 ("net: openvswitch: Add support to count upcall packets")
> Signed-off-by: Eelco Chaudron
> ---
Acked-by: Aaron Conole
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net v2] net: openvswitch: fix upcall counter access before allocation
On Tue, Jun 06, 2023 at 01:56:35PM +0200, Eelco Chaudron wrote:
> Currently, the per cpu upcall counters are allocated after the vport is
> created and inserted into the system. This could lead to the datapath
> accessing the counters before they are allocated resulting in a kernel
> Oops.
>
> Here is an example:
>
> PID: 59693TASK: 0005f4f51500 CPU: 0COMMAND: "ovs-vswitchd"
>#0 [8a39b5b0] __switch_to at b70f0629f2f4
>#1 [8a39b5d0] __schedule at b70f0629f5cc
>#2 [8a39b650] preempt_schedule_common at b70f0629fa60
>#3 [8a39b670] dynamic_might_resched at b70f0629fb58
>#4 [8a39b680] mutex_lock_killable at b70f062a1388
>#5 [8a39b6a0] pcpu_alloc at b70f0594460c
>#6 [8a39b750] __alloc_percpu_gfp at b70f05944e68
>#7 [8a39b760] ovs_vport_cmd_new at b70ee6961b90 [openvswitch]
>...
>
> PID: 58682TASK: 0005b2f0bf00 CPU: 0COMMAND: "kworker/0:3"
>#0 [8a5d2f40] machine_kexec at b70f056a0758
>#1 [8a5d2f70] __crash_kexec at b70f057e2994
>#2 [8a5d3100] crash_kexec at b70f057e2ad8
>#3 [8a5d3120] die at b70f0628234c
>#4 [8a5d31e0] die_kernel_fault at b70f062828a8
>#5 [8a5d3210] __do_kernel_fault at b70f056a31f4
>#6 [8a5d3240] do_bad_area at b70f056a32a4
>#7 [8a5d3260] do_translation_fault at b70f062a9710
>#8 [8a5d3270] do_mem_abort at b70f056a2f74
>#9 [8a5d32a0] el1_abort at b70f06297dac
> #10 [8a5d32d0] el1h_64_sync_handler at b70f06299b24
> #11 [8a5d3410] el1h_64_sync at b70f056812dc
> #12 [8a5d3430] ovs_dp_upcall at b70ee6963c84 [openvswitch]
> #13 [8a5d3470] ovs_dp_process_packet at b70ee6963fdc
> [openvswitch]
> #14 [8a5d34f0] ovs_vport_receive at b70ee6972c78 [openvswitch]
> #15 [8a5d36f0] netdev_port_receive at b70ee6973948 [openvswitch]
> #16 [8a5d3720] netdev_frame_hook at b70ee6973a28 [openvswitch]
> #17 [8a5d3730] __netif_receive_skb_core.constprop.0 at
> b70f06079f90
>
> We moved the per cpu upcall counter allocation to the existing vport
> alloc and free functions to solve this.
>
> Fixes: 95637d91fefd ("net: openvswitch: release vport resources on failure")
> Fixes: 1933ea365aa7 ("net: openvswitch: Add support to count upcall packets")
> Signed-off-by: Eelco Chaudron
> ---
>
> v2: - Cleaned up error handling as Simon suggested.
Thanks!
Reviewed-by: Simon Horman
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
