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.
//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
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
