> On Feb 17, 2016, at 2:39 AM, André Mantas <andremant...@gmail.com> wrote: > > Hi. I'm currently adding support for packet-out messages in bundles. > > I started to look at *handle_packet_out*, *handle_bundle_add *and > *do_bundle_commit > *functions to understand how things work. Here is what I did in each > function: > > *handle_bundle_add:* added if case for OFPTYPE_PACKET_OUT which does all > the validations performed in handle_packet_out as well as saving the > packet_out info in the bundle_entry. For this I added a new
You’d have to make sure that the validations work as expected in the context of the bundle. I haven’t checked the details, but if an earlier message in the bundle adds a port (that has not been committed yet), will the in_port of the packet_out using that port validate correctly? Group and meter validation is part of the action validation for packet_out. While the bundles can’t currently include group or meter mods, I’d like to see that adding packet_out to a bundle does not unnecessarily complicate adding group and meter mods to bundles. > ofproto_packet_out struct to the union in ofp_bundle_entry. This new struct > (created in the same way as ofproto_flow_mod and ofproto_port_mod) contains > an ofputil_packet_out, a dp_packet *payload and a flow. If I understand > correctly, this three structs are needed later to send the packet_out in > the commit phase. > > *do_bundle_commit: *added if case for OFPTYPE_PACKET_OUT in phase 3 > (finish) which calls the ofproto->ofproto_class->packet_out function using > the data saved in the bundle entry and then deletes de payload (as done in > handle_packet_out). > In phase 1 (begin) I only added prev_is_port_mod = false; to the packet-out > "if case" because all the validations were done before. > > Is there a way to verify if the packet can be sent without actually sending > it? Currently, no. The packet_out execution can fail either in action translation (which may call into arbitrarily complex set of flow tables via resubmit action), or in datapath action execution. Datapaths (such as the openvswitch Linux kernel module) currently perform action validation as an integral part of packet execution, and it may be impossible to create a call that would only exercise the validation part. > If not, the ofproto->ofproto_class->packet_out can still return errors, but > this will be in the finish phase, which cannot be rolled back. To make packet_out behave correctly in a bundle, the ofproto_class->packet_out should be split into separate validation and execution pieces. If you decide to do this, it may be possible to perform the action translation piece before the commit phase, leaving only the datapath action execution to the ofproto_class->packet_out. However, it may be costly to split the datapath action execution to separate validation and execution steps. Generally we do a good job not trying to execute something that the datapath then rejects, so maybe that is not necessary. > I think that in this case the bundle should still be committed (because it > does not affect other modifications like flow-mods and port-mods) and > errors can be returned to the controller. Would this be a good option? > As per the OpenFlow bundle spec, the controller does not expect any errors for a successful bundle commit. So this would not be a good option. However, contrary to the documentation in ofproto-provider.h, our current implementation of packet_out in ofproto-dpif.c does not pass any errors back to the caller. If we decide to keep it that way, then all of the above is kind of moot, as we pretend that the packet_out succeeded even when it did not. Jarno > I would also like to know if I'm going in the right direction or if I did > something wrong. > > Thanks in advance. > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
