[ovs-dev] [netlink v4 49/52] datapath: Convert ODP_DP_* commands to use AF_NETLINK socket layer.

Jesse Gross jesse at nicira.com
Sat Jan 22 02:38:21 UTC 2011


On Tue, Jan 11, 2011 at 9:50 PM, Ben Pfaff <blp at nicira.com> wrote:
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index 5fea1af..5d4e1f2 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> +static int odpdc_new(struct sk_buff *skb, struct genl_info *info)
[...]
> -       return 0;
> +       return genlmsg_reply(reply, info);

Should we set an error on the socket if the reply/message creation
fails?  That's what we (and the bridge) do if new port notification
fail.

It's also not really clear to me when we should send a reply.  We have:
* New objects unicast a response back.
* Flows unicast a response on deletion.
* Ports multicast notifications on creation and deletion as the bridge
does through RTNL.

> -static int modify_datapath(unsigned int cmd, struct odp_datapath __user *uodp_datapath)
> +static int odpdc_modify(struct sk_buff *skb, struct genl_info *info)
>  {
> -       struct nlattr *a[ODPDT_MAX + 1];
>        struct datapath *dp;
> -       struct sk_buff *skb;
>        int err;
>
> -       skb = copy_datapath_from_user(uodp_datapath, a);
> -       err = PTR_ERR(skb);
> -       if (IS_ERR(skb))
> +       err = odpdc_validate(info->attrs);
> +       if (err)
>                goto exit;
>
>        rtnl_lock();
>        mutex_lock(&dp_mutex);
> -       dp = lookup_datapath((struct odp_datapath *)skb->data, a);
> +       dp = lookup_datapath(info->userhdr, info->attrs);
>        err = PTR_ERR(dp);
>        if (IS_ERR(dp))
> -               goto exit_free;
> +               goto exit_unlock;
>
> -       if (cmd == ODP_DP_DEL)
> +       /* XXX need to compose reply */

Do we need to send a response here?

> +static int odpdc_get(struct sk_buff *skb, struct genl_info *info)
>  {
> -       struct nlattr *a[ODPDT_MAX + 1];
> -       struct odp_datapath *odp_datapath;
> +       struct sk_buff *reply;
>        struct datapath *dp;
> -       struct sk_buff *skb;
>        int err;
>
> -       skb = copy_datapath_from_user(uodp_datapath, a);
> -       err = PTR_ERR(skb);
> -       if (IS_ERR(skb))
> -               goto exit;
> -       odp_datapath = (struct odp_datapath *)skb->data;
> -
>        mutex_lock(&dp_mutex);
> -       dp = lookup_datapath(odp_datapath, a);
> -
> +       dp = lookup_datapath(info->userhdr, info->attrs);

Don't we need to validate first?  Specifically, we need to check that
the name is NULL terminated on older kernels.




More information about the dev mailing list