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 <[email protected]>
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.
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.
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.
Thanks,
Ben.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev