Eelco Chaudron <[email protected]> 于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. > //Eelco > > > Eelco Chaudron <[email protected]> 于2022年11月23日周三 23:54写道： > > > >> > >> > >> On 19 Nov 2022, at 1:46, Peng He wrote: > >> > >>> Eelco Chaudron <[email protected]> 于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 <[email protected]> 于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 <[email protected]> 于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 <[email protected]> > >>>>>>>>> --- > >>>>>>>>> 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
