[ovs-dev] [PATCH 1/3] datapath: Add genl_exec().

Pravin Shelar pshelar at nicira.com
Wed Dec 14 23:07:01 UTC 2011


On Wed, Dec 14, 2011 at 2:47 PM, Ansis Atteka <aatteka at nicira.com> wrote:
>
> On Wed, Dec 14, 2011 at 2:08 PM, Pravin B Shelar <pshelar at nicira.com> wrote:
>>
>> genl_lock is not exported from older kernel. Following patch add
>> genl_exec() which can run any function (passed as arg) with
>> genl_lock held.
>>
>> Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
>> ---
>>  datapath/datapath.c                           |    9 ++-
>>  datapath/linux/compat/genetlink.inc           |  102
>> +++++++++++++++++++++++++
>>  datapath/linux/compat/include/net/genetlink.h |    6 ++
>>  3 files changed, 116 insertions(+), 1 deletions(-)
>>
>> diff --git a/datapath/datapath.c b/datapath/datapath.c
>> index c86c20b..025d932 100644
>> --- a/datapath/datapath.c
>> +++ b/datapath/datapath.c
>> @@ -2049,10 +2049,14 @@ static int __init dp_init(void)
>>        pr_info("Open vSwitch switching datapath %s, built "__DATE__"
>> "__TIME__"\n",
>>                VERSION BUILDNR);
>>
>> -       err = ovs_tnl_init();
>> +       err = genl_exec_init();
>>        if (err)
>>                goto error;
>>
>> +       err = ovs_tnl_init();
>> +       if (err)
>> +               goto error_genl_exec;
>> +
>>        err = ovs_flow_init();
>>        if (err)
>>                goto error_tnl_exit;
>> @@ -2079,6 +2083,8 @@ error_flow_exit:
>>        ovs_flow_exit();
>>  error_tnl_exit:
>>        ovs_tnl_exit();
>> +error_genl_exec:
>> +       genl_exec_exit();
>>  error:
>>        return err;
>>  }
>> @@ -2091,6 +2097,7 @@ static void dp_cleanup(void)
>>        ovs_vport_exit();
>>        ovs_flow_exit();
>>        ovs_tnl_exit();
>> +       genl_exec_exit();
>>  }
>>
>>  module_init(dp_init);
>> diff --git a/datapath/linux/compat/genetlink.inc
>> b/datapath/linux/compat/genetlink.inc
>> index bf96980..b4de70e 100644
>> --- a/datapath/linux/compat/genetlink.inc
>> +++ b/datapath/linux/compat/genetlink.inc
>> @@ -146,3 +146,105 @@ void netlink_set_err(struct sock *ssk, u32 pid, u32
>> group, int code)
>>  {
>>  }
>>  #endif
>> +
>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,35)
>> +
>> +static DEFINE_MUTEX(genl_exec_lock);
>> +
>> +static genl_exec_func_t         genl_exec_function;
>> +static int              genl_exec_function_ret;
>> +static void            *genl_exec_data;
>> +static struct completion done;
>> +
>> +
>> +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);
>> +       complete(&done);
>> +       return 0;
>> +}
>> +
>> +enum genl_exec_attr {
>> +       GENL_EXEC_UNSPEC,
>> +       GENL_EXEC_RUN,
>> +       GENL_EXEC_MAX,
>> +};
>> +
>> +static struct genl_family genl_exec_family = {
>> +       .id = GENL_ID_GENERATE,
>> +       .hdrsize = 0,
>> +       .name = "GENL EXEC FAMILY",
>> +       .version = 1,
>> +       .maxattr = GENL_EXEC_MAX,
>> +};
>> +
>> +static struct genl_ops genl_exec_ops[] = {
>> +       {
>> +        .cmd = GENL_EXEC_RUN,
>> +        .doit = genl_exec_cmd,
>> +        .flags = CAP_NET_ADMIN,
>> +       },
>> +};
>> +
>> +int genl_exec_init(void)
>> +{
>> +       int err;
>> +
>> +       err = genl_register_family_with_ops(&genl_exec_family,
>> +                                           genl_exec_ops,
>> +                                           ARRAY_SIZE(genl_exec_ops));
>> +       return err;
>> +}
>> +
>> +void genl_exec_exit(void)
>> +{
>> +       genl_unregister_family(&genl_exec_family);
>> +}
>> +
>> +/* genl_lock() is not exported from older kernel.
>> + * Following function allows any function to be executed with
>> + * genl_lock held. */
>
> You meant with "genl_exec_lock" instead of "genl_lock", right?

not really. this whole function is written to execute a function with
genl_lock() (genl_mutex) which is in kernel lock. only way to get that
lock is to execute code in genetlink callback.
I will update the comment to genl_mutex which is less confusing.

>>
>> +
>> +int genl_exec(genl_exec_func_t func, void *data)
>> +{
>> +       struct sk_buff *skb;
>> +
>> +       skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>> +       if (!skb)
>> +               return -ENOMEM;
>> +
>> +       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);
>> +       wait_for_completion(&done);
>> +       genl_exec_function = NULL;
>
> Shouldn't you prepare here a copy of genl_exec_function_ret before actually
> returning it? I sense a small-probability of a race condition here.
>>
right, I need to do it inside lock. Even with lock it is not
completely safe as callback is not taking any lock. But I think that
is not issue for now.

>> +       mutex_unlock(&genl_exec_lock);
>> +
>> +       return genl_exec_function_ret;
>> +}
>> +
>> +#else
>> +
>> +int genl_exec(genl_exec_func_t func, void *data)
>> +{
>> +       int ret;
>> +
>> +       genl_lock();
>> +       ret = func(data);
>> +       genl_unlock();
>> +       return ret;
>> +}
>> +int genl_exec_init(void)
>> +{
>> +       return 0;
>> +}
>> +void genl_exec_exit(void)
>> +{
>> +}
>> +
>> +#endif
>> diff --git a/datapath/linux/compat/include/net/genetlink.h
>> b/datapath/linux/compat/include/net/genetlink.h
>> index a1ff7c1..1bbb559 100644
>> --- a/datapath/linux/compat/include/net/genetlink.h
>> +++ b/datapath/linux/compat/include/net/genetlink.h
>> @@ -170,4 +170,10 @@ static inline struct net *genl_info_net(struct
>> genl_info *info)
>>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,32)
>>  #define genlmsg_unicast(ignore_net, skb, pid)   genlmsg_unicast(skb, pid)
>>  #endif
>> +
>> +typedef int (*genl_exec_func_t)(void *data);
>> +int genl_exec(genl_exec_func_t func, void *data);
>> +int genl_exec_init(void);
>> +void genl_exec_exit(void);
>> +
>>  #endif /* genetlink.h */
>> --
>> 1.7.1
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>
>



More information about the dev mailing list