On Tue, Jul 29, 2014 at 2:09 PM, Alex Wang <al...@nicira.com> wrote:
> Thx for the suggestion, I think it will make the connmgr module safer
> if we annotate the ofproto->connmgr. With a quick look, I think it will
> take more checking and efforts, (for functions we know that are only called
> by the main thread, there is no lock protection)
Is this safe if ofproto->connmgr can be accessed by other threads?
>
> For branch-2.3, I want to use this fix. Do you think it is okay?
I suppose so, as long as the fix is safe. Your call.
>
> Thanks,
> Alex Wang,
>
>
> On Tue, Jul 29, 2014 at 1:23 PM, Andy Zhou <az...@nicira.com> wrote:
>>
>> Should we also consider lock annotate the ofproto->connmgr variable?
>>
>> Also, it seems to me the lock can be per ofproto, instead of a global
>> lock, but I am not sure this use case will benefit from a finer grain
>> lock.
>>
>>
>> On Tue, Jul 29, 2014 at 11:20 AM, Alex Wang <al...@nicira.com> wrote:
>> > connmgr_wants_packet_in_on_miss() is called by multiple threads
>> > and thusly should be protected by the mutex.
>> >
>> > Signed-off-by: Alex Wang <al...@nicira.com>
>> > ---
>> > ofproto/connmgr.c | 6 +++++-
>> > 1 file changed, 5 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
>> > index c6432bf..89af6b6 100644
>> > --- a/ofproto/connmgr.c
>> > +++ b/ofproto/connmgr.c
>> > @@ -1506,19 +1506,23 @@ ofconn_wants_packet_in_on_miss(struct ofconn
>> > *ofconn,
>> > * This logic assumes that "table-miss" packet_in messages
>> > * are always sent to controller_id 0. */
>> > bool
>> > -connmgr_wants_packet_in_on_miss(struct connmgr *mgr)
>> > +connmgr_wants_packet_in_on_miss(struct connmgr *mgr)
>> > OVS_EXCLUDED(ofproto_mutex)
>> > {
>> > struct ofconn *ofconn;
>> >
>> > + ovs_mutex_lock(&ofproto_mutex);
>> > LIST_FOR_EACH (ofconn, node, &mgr->all_conns) {
>> > enum ofputil_protocol protocol = ofconn_get_protocol(ofconn);
>> >
>> > if (ofconn->controller_id == 0 &&
>> > (protocol == OFPUTIL_P_NONE ||
>> > ofputil_protocol_to_ofp_version(protocol) <
>> > OFP13_VERSION)) {
>> > + ovs_mutex_unlock(&ofproto_mutex);
>> > return true;
>> > }
>> > }
>> > + ovs_mutex_unlock(&ofproto_mutex);
>> > +
>> > return false;
>> > }
>> >
>> > --
>> > 1.7.9.5
>> >
>> > _______________________________________________
>> > dev mailing list
>> > dev@openvswitch.org
>> > http://openvswitch.org/mailman/listinfo/dev
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
