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)
For branch-2.3, I want to use this fix. Do you think it is okay?
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
