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.
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
