On 7/12/21 5:07 PM, Eli Britstein wrote: > Log flow modifications to help debugging. > > Signed-off-by: Eli Britstein <[email protected]> > Reviewed-by: Gaetan Rivet <[email protected]> > --- > lib/dpif-netdev.c | 101 +++++++++++++++++++++++++--------------------- > 1 file changed, 55 insertions(+), 46 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 9b2b8d6d9..caed3e7f2 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -2457,6 +2457,61 @@ queue_netdev_flow_put(struct dp_netdev_pmd_thread *pmd, > struct dp_flow_offload_item *offload; > int op; > > + if (OVS_UNLIKELY(!VLOG_DROP_DBG((&upcall_rl)))) { > + struct ds ds = DS_EMPTY_INITIALIZER; > + struct ofpbuf key_buf, mask_buf; > + struct odp_flow_key_parms odp_parms = { > + .flow = &match->flow, > + .mask = &match->wc.masks, > + .support = dp_netdev_support, > + }; > + > + ofpbuf_init(&key_buf, 0); > + ofpbuf_init(&mask_buf, 0); > + > + odp_flow_key_from_flow(&odp_parms, &key_buf); > + odp_parms.key_buf = &key_buf; > + odp_flow_key_from_mask(&odp_parms, &mask_buf); > + > + if (old_actions) { > + ds_put_cstr(&ds, "flow_mod: "); > + } else { > + ds_put_cstr(&ds, "flow_add: "); > + } > + odp_format_ufid(&flow->ufid, &ds); > + ds_put_cstr(&ds, " mega_"); > + odp_format_ufid(&flow->mega_ufid, &ds); > + ds_put_cstr(&ds, " "); > + odp_flow_format(key_buf.data, key_buf.size, > + mask_buf.data, mask_buf.size, > + NULL, &ds, false); > + if (old_actions) { > + ds_put_cstr(&ds, ", old_actions:"); > + format_odp_actions(&ds, old_actions->actions, old_actions->size, > + NULL); > + } > + ds_put_cstr(&ds, ", actions:"); > + format_odp_actions(&ds, actions, actions_len, NULL); > + > + VLOG_DBG("%s", ds_cstr(&ds)); > + > + ofpbuf_uninit(&key_buf); > + ofpbuf_uninit(&mask_buf); > + > + /* Add a printout of the actual match installed. */ > + struct match m; > + ds_clear(&ds); > + ds_put_cstr(&ds, "flow match: "); > + miniflow_expand(&flow->cr.flow.mf, &m.flow); > + miniflow_expand(&flow->cr.mask->mf, &m.wc.masks); > + memset(&m.tun_md, 0, sizeof m.tun_md); > + match_format(&m, NULL, &ds, OFP_DEFAULT_PRIORITY); > + > + VLOG_DBG("%s", ds_cstr(&ds)); > + > + ds_destroy(&ds); > + } > +
It make sense to print. However, I don't think that this generic log belongs to queue_netdev_flow_put() function. It shouldn't be there. Suggesting to create a separate function for logging and call it in two places after queue_netdev_flow_put(). Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
