> On Jul 29, 2016, at 1:32 PM, Ben Pfaff <b...@ovn.org> wrote:
>
> On Thu, Jul 28, 2016 at 05:56:04PM -0700, Jarno Rajahalme wrote:
>> It is possible that a bundle add message fails, but the following
>> commit succeeds, since the message was not added to the bundle. Make
>> ovs-ofctl fail also in these cases.
>>
>> Also, the commit should not be sent if any of the bundled messages
>> failed. To make sure all the errors are received before the commit is
>> sent, a barrier is required before sending the commit message.
>>
>> Finally, make vconn collect bundle errors into a list instead of
>> calling a callback. This makes bundle error management simpler.
>>
>> Signed-off-by: Jarno Rajahalme <ja...@ovn.org>
>
> Glad to see the callback function go away. Die, callbacks, die!
>
> The array bound on ofp_msg[] is puzzling. Do we really want an array of
> "struct ofp_header"s? If we do, why is its bound specified in terms of
> the size of an ofp_header? Maybe a union of a struct ofp_header and a
> uint8_t[64] is a better formulation?
Changed. Someone tried to be too clever.
>> +struct vconn_bundle_error {
>> + struct ovs_list list_node;
>> +
>> + /* OpenFlow header and some of the message contents for error
>> reporting. */
>> + struct ofp_header ofp_msg[DIV_ROUND_UP(64, sizeof(struct ofp_header))];
>> +};
>
> s/reelasing/releasing/ in the comment on
> vconn_bundle_control_transact().
>
Done.
> This is very careful and thorough error reporting. I like it.
>
:-)
> Acked-by: Ben Pfaff <b...@ovn.org>
>
> Thanks,
>
> Ben.
Thanks for the review!
Jarno
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
