Eelco Chaudron <echau...@redhat.com> 于2022年11月24日周四 17:08写道：
>
>
> On 24 Nov 2022, at 10:04, Peng He wrote:
>
> > Eelco Chaudron <echau...@redhat.com> 于2022年11月24日周四 16:34写道：
> >
> >>
> >>
> >> On 24 Nov 2022, at 1:46, Peng He wrote:
> >>
> >>> So do we need this patch (补丁) (补丁) or not??
> >>>
> >>> Guessing it's quite rare in the real production environment that we
> have
> >>> two datapaths at the same time ....
> >>> And I am more (更多) (更多) curious that even though we have 2 datapaths,
> should
> >> the port
> >>> id be different? Is one
> >>> port capable of being assigned to 2 datapaths at the same time ????
> >>>
> >>> Because only when a port is assigned to 2 datapaths at the same time,
> we
> >>> should worry about this race....
> >>
> >> I think we should still add this patch (补丁) (补丁) , as it’s common to
> have a
> >> single DPDK datapath bridge, but it could have kernel (内核) (内核) (the
> bridge
> >> itself for example) and DPDK ports. In this case when the actions of the
> >> OpenFlow rule change there could be different actions for existing rules
> >> not yet updated by the revalidator.
> >>
> >> Or do I miss the point here?
> >>
> >> On a single DPDK datapath, the kernel (内核) ports will be processed by
> the DPDK
> > datapath also (through using AF_SOCKET socket), so in this case, we only
> > have one datapath and we will not have this race.
>
> If you are sure we can drop this patch (补丁) .
>
I am sure that normally we will have only one datapath.
I am not sure that why kernel datatpath's megaflow should be added into the
userspace datapath ....
But I guess we can drop this patch.
Thanks!
>
> >> //Eelco
> >>
> >>> Eelco Chaudron <echau...@redhat.com> 于2022年11月23日周三 23:54写道：
> >>>
> >>>>
> >>>>
> >>>> 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
> >>>>
> >>>>
> >>>
> >>> --
> >>> hepeng
> >>
> >>
> >
> > --
> > hepeng
>
>
--
hepeng
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
