On 19 Nov 2022, at 1:46, Peng He wrote:

> Eelco Chaudron <echau...@redhat.com> 于2022年11月18日周五 15:38写道:
>
>>
>>
>> On 18 Nov 2022, at 2:57, Peng He wrote:
>>
>>> Since there are possible race conditions (between the kernel (内核)
>> datapath and
>>> userspace datapath),
>>> I guess this patch (补丁) is now needed again? But two datapath is really
>> rare in
>>> the real deployment.
>>> So I am not sure if we should pay attention here.
>>
>> I still think we should add this, as there seem to be a decent amount of
>> times people intermix a kernel (内核) interface with a DPDK one. For example,
>> the bridge interface, which would be up to get routing (溃败) information for
>> tunnels.
>
>
> In this case, bridge interfaces are attached to the userspace datapath, it
> will be "polled" by the main thread, and it's pmd-id is NON_PMD_CORE_ID.
>
> The case that race could happen is that mix using of userspace datapath and
> kernel datapath. When the kernel datapath receives a upcall, it will set
> the pmd-id to PMD_ID_NULL. Checking the code of dpif_netdev_flow_put, only
> the megaflow with pmd-id equals to PMD_ID_NULL will be installed
> into all the PMD threads.

Agreed, I think this is the only case it could still happen. I could not find 
any other paths.

>> //Eelco
>>
>>
>>> Eelco Chaudron <echau...@redhat.com> 于2022年10月19日周三 18:50写道:
>>>
>>>>
>>>>
>>>> On 10 Oct 2022, at 9:12, Eelco Chaudron wrote:
>>>>
>>>>> On 8 Oct 2022, at 5:27, Peng He wrote:
>>>>>
>>>>>> Hi,Eelco
>>>>>>
>>>>>> after a second thought, I think this patch (补丁) is not needed neither,
>>>>>> the code (代码) here is trying to find a rule which cover the packet,
>>>>>> it does not mean (意味着) the match and action of rule equals to the ones
>>>>>> of the ukey.
>>>>>>
>>>>>> So the code (代码) here is just a prevention, no need to make it
>> consistent
>>>>>> with ukey.
>>>>>>
>>>>>> but the comments above are really misleading, so I sent a new patch
>> (补丁)
>>>> fixing
>>>>>> it.
>>>>>
>>>>> Ack, will wait for the v5, and review.
>>>>
>>>> As I did not see a v5, I reviewed the v4, and assume (假设) this patch
>> (补丁) can be
>>>> ignored (忽略) .
>>>>
>>>> //Eelco
>>>>
>>>>>> Peng He <xnhp0...@gmail.com> 于2022年10月3日周一 20:41写道:
>>>>>>
>>>>>>> When PMDs perform upcalls, the newly generated (生成) ukey will replace
>>>>>>> the old, however, the newly generated (生成) mageflow will be discard
>>>>>>> to reuse the old one without checking if the actions of new and
>>>>>>> old are equal.
>>>>>>>
>>>>>>> This code (代码) prevents in case someone runs dpctl/add-flow to add
>>>>>>> a dp flow with inconsistent actions with the actions of ukey,
>>>>>>> and causes more (更多) confusion (混乱) .
>>>>>>>
>>>>>>> Signed-off-by: Peng He <hepeng.0...@bytedance.com>
>>>>>>> ---
>>>>>>>  lib/dpif-netdev.c | 17 ++++++++++++++++-
>>>>>>>  1 file (文件) changed, 16 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>>>>>> index a45b46014..b316e59ef 100644
>>>>>>> --- a/lib/dpif-netdev.c
>>>>>>> +++ b/lib/dpif-netdev.c
>>>>>>> @@ -8304,7 +8304,22 @@ handle_packet_upcall(struct
>> dp_netdev_pmd_thread
>>>>>>> *pmd,
>>>>>>>           * to be locking revalidators out of making flow
>>>> modifications. */
>>>>>>>          ovs_mutex_lock(&pmd->flow_mutex);
>>>>>>>          netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
>>>>>>> -        if (OVS_LIKELY(!netdev_flow)) {
>>>>>>> +        if (OVS_UNLIKELY(netdev_flow)) {
>>>>>>> +            struct dp_netdev_actions *old_act =
>>>>>>> +                dp_netdev_flow_get_actions(netdev_flow);
>>>>>>> +
>>>>>>> +            if ((add_actions->size != old_act->size) ||
>>>>>>> +                    memcmp(old_act->actions, add_actions->data,
>>>>>>> +                                             add_actions->size)) {
>>>>>>> +
>>>>>>> +               struct dp_netdev_actions *new_act =
>>>>>>> +                   dp_netdev_actions_create(add_actions->data,
>>>>>>> +                                            add_actions->size);
>>>>>>> +
>>>>>>> +               ovsrcu_set(&netdev_flow->actions, new_act);
>>>>>>> +               ovsrcu_postpone(dp_netdev_actions_free, old_act);
>>>>>>> +            }
>>>>>>> +        } else {
>>>>>>>              netdev_flow = dp_netdev_flow_add(pmd, &match, &ufid,
>>>>>>>                                               add_actions->data,
>>>>>>>                                               add_actions->size,
>>>>>>> orig_in_port);
>>>>>>> --
>>>>>>> 2.25.1
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> hepeng
>>>>
>>>>
>>>
>>> --
>>> hepeng
>>
>>
>
> -- 
> hepeng

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to