Thanks for the detailed answer. Comments below. Jarno Rajahalme <ja...@ovn.org> escreveu no dia quarta, 17/02/2016 às 17:59:
> > > 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? > Good question, will have to look at this. It should allow a packet_out in that "new" port. > 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. > > If this is the case, should a new validation function be made that does not involve validating group and meters? > > 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. > That's good to know. If it comes out that splitting the function is a better solution, I may be able to do it. > > > 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. > Right, I just thought it would make sense in this case, but it cannot be done. > > 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. > So, summing up, the most straightforward solution is to assume that after the first validation, a packet-out never fails being sent, right? > > 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
