On Thu, Dec 15, 2011 at 6:03 PM, Jesse Gross <je...@nicira.com> wrote:
> On Wed, Dec 14, 2011 at 6:09 PM, Pravin B Shelar <pshe...@nicira.com> wrote:
>> diff --git a/datapath/linux/compat/genetlink.inc
>> b/datapath/linux/compat/genetlink.inc
>> index bf96980..43c3227 100644
>> --- a/datapath/linux/compat/genetlink.inc
>> +++ b/datapath/linux/compat/genetlink.inc
>> +static int genl_exec_cmd(struct sk_buff *dummy, struct genl_info *dummy2)
>> +{
>> + if (genl_exec_function)
>> + genl_exec_function_ret = genl_exec_function(genl_exec_data);
>
> Can genl_exec_function ever be NULL (and is there a point to NULLing
> it out after use)?
>
right, i will drop it.
>> +enum genl_exec_attr {
>> + GENL_EXEC_UNSPEC,
>> + GENL_EXEC_RUN,
>> + GENL_EXEC_MAX,
>> +};
>
> This MAX is off by one, it should be the index of the highest element,
> not the one after that.
>
ok
>> +static struct genl_family genl_exec_family = {
>> + .id = GENL_ID_GENERATE,
>> + .hdrsize = 0,
>> + .name = "GENL EXEC FAMILY",
>
> The name should be prefixed with ovs like "ovs_genl_exec".
>
ok
>> + .version = 1,
>> + .maxattr = GENL_EXEC_MAX,
>
> This should be the highest attribute, of which there are none, but
> this is the highest command.
>
ok
>> +int genl_exec(genl_exec_func_t func, void *data)
>> +{
>> + struct sk_buff *skb;
>> + int ret;
>> +
>> + skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>> + if (!skb)
>> + return -ENOMEM;
>
> I'm worried about the consequences of this failing when it is used to
> cleanup namespaces.
>
ok , I am pre-allocating skb at init.
>> + genlmsg_put(skb, 0, 0, &genl_exec_family, NLM_F_REQUEST,
>> GENL_EXEC_RUN);
>> +
>> + mutex_lock(&genl_exec_lock);
>> + genl_exec_function = func;
>> + genl_exec_data = data;
>> + init_completion(&done);
>> + genlmsg_unicast(&init_net, skb, 0);
>
> genlmsg_unicast() can return an error if the socket buffer is full on
> kernels that actually add it to a queue but this will silently ignore
> it.
ok.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
