On Fri, Apr 14, 2017 at 2:17 PM, Ben Pfaff <b...@ovn.org> wrote: > On Fri, Apr 14, 2017 at 10:20:10AM -0700, Yi-Hung Wei wrote: >> This patch adds support of OFPR_PACKET_OUT as the packet-in reason. >> This packet-in reason is a required feature for OF1.4+, and it indicates >> that the associated packet-in message to the controller is triggered when >> the switch is processing a packet-out message. This reason code is enabled >> by default when OF1.4+ is used. >> >> Signed-off-by: Yi-Hung Wei <yihung....@gmail.com> > > Thank you for working on this! I would very much like to say that OVS > supports everything that OpenFlow 1.4 requires (so that we can enable it > by default), so it's nice to see some steps in that direction. > > In openflow.rst, you mention that "All required features are now > supported" for packet-in reasons. Are there optional packet-in reasons > that are still not supported? If all packet-in reasons are now > supported, then I would prefer to delete the whole item, because these > items are supposed to list areas where there is still potential work to > do. Thanks for review. Yes, all packet-in reasons are now supported. I will delete the whole item in v2.
> > I don't think that the change to connmgr_send_async_msg() makes sense. > The 'reason' code passed to ofconn_receives_async_msg() is supposed to > be one of the bit indexes into the members of struct ofputil_async_cfg. > Those members are not well documented, but if you look around, for > example at ofputil_async_cfg_default(), you can see that they are > supposed to have OpenFlow version independent meanings. Can you explain > this change? By doing a version-dependent mapping before calling > ofconn_receives_async_msg(), we will lose some nuances; for example, we > will lose the difference between explicit and implicit misses. Yes, thanks for pointing out this problem. Actually, it is due to another issue that should be addressed by a separate patch. I just sent it out as in v2. > > In xlate_actions(), I believe that you can write: > .in_packet_out = xin->in_packet_out ? true : false, > as > .in_packet_out = xin->in_packet_out, > since both are booleans. Sure. > > Thanks, > > Ben. Thanks, -Yi-Hung _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
