[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