On 30 Sep 2022, at 17:55, Peng He wrote:
> Eelco Chaudron <[email protected]>于2022年9月30日 周五23:50写道： > >> >> >> On 23 Sep 2022, at 18:29, Peng He wrote: >> >>> 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); >> >> Do we really need this patch? Asking as this is in the fastpath code? >> And it only happens if someone is messing with dpctl commands, which they >> should not. > > > Yes, I just point out there is inconsistent here，for most cases the code > will never run into here. Ok, please add a comment explain this in the next version. >> >> I we want it, we should add a good comment on why it’s there. > > > > >> >>> + } >>> + } else { >>> netdev_flow = dp_netdev_flow_add(pmd, &match, &ufid, >>> add_actions->data, >>> add_actions->size, >> orig_in_port); >>> -- >>> 2.25.1 >>> >>> _______________________________________________ >>> dev mailing list >>> [email protected] >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> -- > hepeng _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
