> On Jul 23, 2015, at 2:39 PM, Ben Pfaff <[email protected]> wrote:
> 
> 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!

Happy to be of help if something else seems cryptic,

  Jarno


_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to