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

Jesse Gross jesse at nicira.com
Thu Dec 15 00:37:02 UTC 2011


On Wed, Dec 14, 2011 at 3:47 PM, Pravin Shelar <pshelar at nicira.com> wrote:
> On Wed, Dec 14, 2011 at 3:34 PM, Jesse Gross <jesse at nicira.com> wrote:
>> On Wed, Dec 14, 2011 at 3:07 PM, Pravin Shelar <pshelar at nicira.com> wrote:
>>> 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:
>>>>> +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.
>>
>> Isn't all of this executing synchronously anyways, so the callback
>> actually is protected by genl_exec_lock and the completion stuff isn't
>> necessary?
> Its not same across kernel that we care.
> callbacks are executed synchronously from 2.6.24 onward.

Are you sure?  I think it just takes a slightly more convoluted path
to get there.  If doesn't execute in this context, what context does
it execute in?

On 2.6.18, for example, I think this is the call stack:
 - genl_exec
 - genlmsg_unicast
 - nlmsg_unicast
 - netlink_unicast
 - netlink_sendskb (skb gets enqueued to socket buffer)
 - netlink_data_ready AKA sk->sk_data_ready
 - genl_rcv AKA nlk->data_ready
 - netlink_run_queue (skb gets dequeued from socket buffer)
 - netlink_rcv_skb
 - genl_rcv_mesg
 - genl_exec_cmd AKA doit

So there's a lot of useless enqueueing, dequeueing, waking up
processes, etc. that are really only needed when sending data to
userspace.  That was shortcut in later kernels but it doesn't really
change anything.



More information about the dev mailing list