[ovs-dev] [PATCH v2 12/26] vconn: Better bundle error management.

Jarno Rajahalme jarno at ovn.org
Fri Jul 29 22:49:31 UTC 2016


> On Jul 29, 2016, at 1:32 PM, Ben Pfaff <blp at 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 <jarno at 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 <blp at ovn.org>
> 
> Thanks,
> 
> Ben.

Thanks for the review!

  Jarno




More information about the dev mailing list