On Thu, Jul 23, 2015 at 02:36:56PM -0700, Jarno Rajahalme wrote: > Something bugged me about this, so I looked again. > > > On Jul 23, 2015, at 1:23 PM, Ben Pfaff <[email protected]> wrote: > > > > Since commit e672ff9b4d2 (ofproto-dpif: Restore metadata and registers > > on recirculation.), xlate_actions() has first obtained the input port: > > > > /* The in_port of the original packet before recirculation. */ > > struct xport *in_port = get_ofp_port(xbridge, flow->in_port.ofp_port); > > > > then updated it to restore the pre-recirculation state: > > > > if (xin->recirc) { > > ... > > /* Restore pipeline metadata. May change flow's in_port and other > > * metadata to the values that existed when recirculation was > > * triggered. */ > > recirc_metadata_to_flow(&recirc->metadata, flow); > > ... > > } > > > > > However I'm not sure that this order makes sense. We use 'in_port' > > later for checks like may_receive() and xport_stp_forward_state() where > > I'd think that the original input port is the appropriate one to check, > > not the one that happened to be used by the recirculated packet. Am I > > missing anything? > > > > ‘in_port’ IS the original input port, it is not changed by the > metadata restoration. And it seems to me that we actually DO check > may_receive() again on the original input port after > recirculation. The bottom half of xlate_actions() is littered with > checks on xin->recirc to figure out what checks (and stats) should not > be done after recirculation. Maybe it could be refactored to make it > clearer, but at the time I did want to keep changes minimal.
For what it's worth, my goal right now is to add some code to xlate_actions() to support testing in OVN. When I started in on that, I realized that I didn't really understand what it did anymore. So as a first step I'm going through xlate_actions() trying to refactor it to the point where I understand it again, then I'll add my feature. So this information is very helpful, thank you! _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
