[ovs-dev] [PATCH net-next 6/8] openvswitch: Allow update of dp with OVS_DP_CMD_NEW if NLM_F_REPLACE is set

[Jesse Gross]

On Fri, Nov 22, 2013 at 8:56 AM, Thomas Graf <tgraf at suug.ch> wrote:
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 95d4424..3f1fb87 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c

I'm a little worried that this introduces some quirks to the interface. Such as:

> @@ -1190,6 +1185,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
>         struct vport *vport;
>         struct ovs_net *ovs_net;
>         int err, i;
> +       bool allocated = false;
>
>         err = -EINVAL;
>         if (!a[OVS_DP_ATTR_NAME] || !a[OVS_DP_ATTR_UPCALL_PID])

This requires that DP_CMD_SET supply an OVS_DP_ATTR_UPCALL_PID
although I don't think that it's really necessary. In fact, we used to
completely ignore that field on SET since it's really only useful on
datapath creation and can otherwise be done more naturally through the
vport interface.

> @@ -1197,11 +1193,26 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
>
>         ovs_lock();
>
> +       dp = lookup_datapath(sock_net(skb->sk), info->userhdr, info->attrs);
> +       if (!IS_ERR(dp)) {
> +               if (info->nlhdr->nlmsg_flags & NLM_F_REPLACE)
> +                       goto update;

Conversely, this won't respect the UPCALL_PID field, which I would
expect it to since I think NLM_F_REPLACE should be roughly equivalent
to a delete and create. I believe that's the only field that is
missing although it seems easy to have the same problem as others are
added in the future.