[ovs-dev] [PATCH] datapath: fix lock up issue in ovs_vport_cmd_set()

[Ansis Atteka]

On Thu, Mar 22, 2012 at 10:37 AM, Jesse Gross <jesse at nicira.com> wrote:

> On Wed, Mar 21, 2012 at 5:13 PM, Ansis Atteka <aatteka at nicira.com> wrote:
> > diff --git a/datapath/datapath.c b/datapath/datapath.c
> > index d64fc32..aa5be89 100644
> > --- a/datapath/datapath.c
> > +++ b/datapath/datapath.c
> > @@ -1885,7 +1885,7 @@ static int ovs_vport_cmd_set(struct sk_buff *skb,
> struct genl_info *info)
> >                err = PTR_ERR(reply);
> >                netlink_set_err(GENL_SOCK(sock_net(skb->sk)), 0,
> >                                ovs_dp_vport_multicast_group.id, err);
> > -               return 0;
> > +               goto exit_unlock;
>
> This has a side effect of changing the error code from 0 to -ENOMEM
> (presumably that's why building the message failed), which I don't
> think is right because the actual command succeeded.  This is
> obviously related to the other problem that you noticed with sending
> messages on failure but we should try to avoid introducing side
> effects in this patch.
>

You are right. But what would be the correct behavior if message
building in ovs_vport_cmd_build_info() failed:
1. revert the altered config to the previous state and return error
code (similarly as ovs_vport_cmd_new() does)
2. do not revert the altered config, but reply with 0 (the same behavior
as before my patch)?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20120322/af2d5c6b/attachment-0003.html>