Eelco Chaudron <[email protected]> 于2022年9月30日周五 23:09写道：
> > > On 30 Sep 2022, at 16:42, Peng He wrote: > > > Eelco Chaudron <[email protected]> 于2022年9月30日周五 22:20写道： > > > >> > >> > >> On 23 Sep 2022, at 18:29, Peng He wrote: > >> > >>> push_dp_ops only handles delete ops errors but ignores the modify > >>> ops results. It's better to handle all the dp operation errors in > >>> a consistent way. > >>> > >>> We observe in the production environment that sometimes a megaflow > >>> with wrong actions keep staying in datapath. The coverage command shows > >>> revalidators have dumped several times, however the correct > >>> actions are not set. This implies that the ukey's action does not > >>> equal to the meagaflow's, i.e. revalidators think the underlying > >>> megaflow's actions are correct however they are not. > >>> > >>> We also check the megaflow using the ofproto/trace command, and the > >>> actions are not matched with the ones in the actual magaflow. By > >>> performing a revalidator/purge command, the right actions are set. > >>> > >>> This patch prevents the inconsistency by considering modify failure > >>> in revalidators. > >>> > >>> Signed-off-by: Peng He <[email protected]> > >>> --- > >>> ofproto/ofproto-dpif-upcall.c | 23 +++++++++++++---------- > >>> 1 file changed, 13 insertions(+), 10 deletions(-) > >>> > >>> diff --git a/ofproto/ofproto-dpif-upcall.c > >> b/ofproto/ofproto-dpif-upcall.c > >>> index 7ad728adf..7ea2a30f5 100644 > >>> --- a/ofproto/ofproto-dpif-upcall.c > >>> +++ b/ofproto/ofproto-dpif-upcall.c > >>> @@ -2416,23 +2416,26 @@ push_dp_ops(struct udpif *udpif, struct ukey_op > >> *ops, size_t n_ops) > >>> > >>> for (i = 0; i < n_ops; i++) { > >>> struct ukey_op *op = &ops[i]; > >>> - struct dpif_flow_stats *push, *stats, push_buf; > >>> - > >>> - stats = op->dop.flow_del.stats; > >>> - push = &push_buf; > >>> - > >>> - if (op->dop.type != DPIF_OP_FLOW_DEL) { > >>> - /* Only deleted flows need their stats pushed. */ > >>> - continue; > >>> - } > >>> > >>> if (op->dop.error) { > >>> - /* flow_del error, 'stats' is unusable. */ > >>> if (op->ukey) { > >>> ovs_mutex_lock(&op->ukey->mutex); > >> > >> I think we need transition_ukey(ukey, UKEY_EVICTING) also for none > delete > >> actions, so the state machine logging is correct. > >> > > > > you mean for modify error, we should modify the ukey into UKEY_EVICTING > > instead of UKEY_EVICTED? > > If so, should we add a new op to delete such datapath flow? > > I mean that if the flow was not marked for deletion, we can not just > change the state to EVICTED, it should be according to the states > transitions in transition_ukey_at() or, they should be updated. > Because we go from UKEY_OPERATIONAL -> UKEY_EVICTED in the none delete > case. > yes, we need an extra delete operation here and transit it into UKEY_EVICTING state. > > >>> transition_ukey(op->ukey, UKEY_EVICTED); > >>> ovs_mutex_unlock(&op->ukey->mutex); > >>> } > >>> + /* if it's a flow_del error, 'stats' is unusable, it's ok > >>> + * to discard the stats. > >>> + */ > >>> + continue; > >>> + } > >>> + > >>> + struct dpif_flow_stats *push, *stats, push_buf; > >>> + > >>> + stats = op->dop.flow_del.stats; > >>> + push = &push_buf; > >>> + > >>> + if (op->dop.type != DPIF_OP_FLOW_DEL) { > >>> + /* Only deleted flows need their stats pushed. */ > >> > >> Can you explain why deleting the “valid” datapath rule does not need > >> updating the stats? > >> > > The comment is included in the original code, I just move it here. > > And the code is to only update stats for datapath flow deleting ? > > Guess you might want to research as we update the stats here because the > dpath flow will be deleted. > > And now we do the same, i.e. change failed, so the flow is gone!? So we > might need to update it. > Ok, I got it. yes it makes sense, but where do we add this extra delete operation? > >> > >>> continue; > >>> } > >>> > >>> -- > >>> 2.25.1 > >>> > >>> _______________________________________________ > >>> dev mailing list > >>> [email protected] > >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >> > >> > > > > -- > > hepeng > > -- hepeng _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
