[ovs-dev] Per-switch configuration in datapath actions

Valentine Sinitsyn valentine.sinitsyn at gmail.com
Wed Dec 14 19:39:54 UTC 2016


Hi Ben,

On 13.12.2016 00:31, Ben Pfaff wrote:
> On Mon, Dec 12, 2016 at 11:51:45PM +0500, Valentine Sinitsyn wrote:
>> On 12.12.2016 21:25, Ben Pfaff wrote:
>>> On Mon, Dec 12, 2016 at 04:56:56PM +0500, Valentine Sinitsyn wrote:
>>>> Suppose you are implementing a custom OpenFlow action, and you need some
>>>> per-bridge configuration to translate it into a datapath action.
>>>>
>>>> Which would be the architecturally correct way to promote this bit of
>>>> information from OVSDB to somewhere inside struct xlate_ctx?
>>>
>>> Usually you have to push it down from bridge.c to ofproto.c to
>>> ofproto-dpif.c to ofproto-dpif-xlate.c through the various levels of
>>> abstraction.  It's awkward, unfortunately.
>>>
>>> I keep thinking lately about getting rid of the abstraction between
>>> ofproto and ofproto-dpif, because no one has ever contributed a second
>>> implementation of ofproto.
>> Thanks for the quick answer. I traced the path from sources, and looks like
>> many ofproto_set_something() functions ultimately use mutexes to protect
>> data they change. What's the contract here; I mean, should ofproto methods
>> always be thread safe? The only place I call my function is from
>> bridge_reconfigure(). Besides, shall I introduce a separate mutex for the
>> data structure I have embedded in struct ofproto?
>
> Most of the ofproto_set_*() functions are called just from a single
> thread (the main thread of the ovs-vswitchd process), but many of them
> configure settings that are used during flow translation, which happens
> in separate threads.  Therefore, many of them ultimately need some kind
> of synchronization.  Usually it's better to use one of the existing
> mutexes (or other synchronization primitive), if there is an appropriate
> one, because it's conceptually difficult to understand the relationships
> of many classes of mutex.
Thanks for the explanation.

So, that's the reason behind set_ipfix() implementation (taking IPFIX as 
an example) not being thread-safe when creating or destroying 
ofproto->ipfix member? I mean:

     struct dpif_ipfix *di = ofproto->ipfix;

     if (has_options && !di) {
         di = ofproto->ipfix = dpif_ipfix_create();
         new_di = true;
     }

for instance. As this code is always coming from the main thread, no 
synchronization is required.

On the other hand, the data within ofproto->ipfix itself is protected by 
a mutex. Is my understanding correct?

--
Best regards,
Valentine Sinitsyn


More information about the dev mailing list