On Wed, Apr 4, 2012 at 10:44 AM, Ansis Atteka <aatt...@nicira.com> wrote:
> On Mon, Mar 26, 2012 at 11:21 AM, Jesse Gross <je...@nicira.com> wrote:
>>
>> On Fri, Mar 23, 2012 at 6:04 PM, Ansis Atteka <aatt...@nicira.com> wrote:
>> > diff --git a/datapath/datapath.c b/datapath/datapath.c
>> > index d64fc32..daf7b69 100644
>> > --- a/datapath/datapath.c
>> > +++ b/datapath/datapath.c
>> > + if (!err) {
>>
>> The difficult thing about this function is that it's possible to fail
>> in the middle without being able to revert (the upstream version
>> doesn't have this problem because it doesn't allow you to set the
>> address of ports). This means that we could have changed something
>> but now won't send a notification, which isn't good.
>
> Would this be an improvement, if we would send notification only when
> ovs_vport_set_options() function succeeded (irrelevant whether
> change_vport() succeeded)? Because as far as I understand notifications
> should be sent only when something was actually changed (at least
> partially)?
Yes, if nothing could have changed then it's nice to not send a
notification. I don't think that it affects correctness but it
reduces the amount of updating that other processes need to do.
>> For the upstream version (and us when we change the tunneling code to
>> not need a MAC address) this solution makes sense. Even though I
>> don't believe that anyone other than OVS listens to OVS notifications,
>> it's good to get them correct.
>
> One more thing I spotted is that ovs_vport_cmd_set() function is using
> OVS_VPORT_CMD_NEW instead of OVS_VPORT_CMD_SET in their
> notifications. This does not seem correct to me? I believe I saw the
> same issue in some other places as well.
The usual convention with Netlink is to use NEW any time that an
object has a different configuration, even if it is an update to an
existing thing.
>>
>> I think that a good userspace implementation of Netlink should be able
>> to handle both the notification and error report. In fact I think
>> this is happening all the time on success because we pass both
>> NLM_F_ECHO and NLM_F_ACK. In that case though, both are interpreted
>> as success so there's no issue but it doesn't seem right to me.
>
> Would it be a more correct behavior for user-space to wait till the NETLINK
> reply where nlmsghdr.type==2 arrives, instead of relying on notifications to
> figure out whether request succeeded or not?
Yes, I think looking at the actual success/error notification is the
right thing to do. We just have to make sure that we also get the
information contained in the notification in cases where we actually
request it.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
