On 24 Nov 2022, at 10:04, Peng He wrote:
> 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. If you are sure we can drop this patch. >> //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
