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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to