| Message ID | 3803c301564bf21d397030e82d57df81fe5c5a91.1557922974.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] OVN: grab pinctrl_mutex before runningpinctrl_handle_buffered_packets | expand |
On Wed, May 15, 2019 at 6:34 AM Lorenzo Bianconi < lorenzo.bianconi@redhat.com> wrote: > > pinctrl_handle_buffered_packets can insert new elements in > buffered_packets_map hasmap and it runs concurrently with pinctrl_run > starting from commit 3594ffab6b4b. Fix possible races grabbing > pinctrl_mutex before running pinctrl_handle_buffered_packets > > Fixes: 3594ffab6b4b ("ovn-controller: Add a new thread in pinctrl module to handle packet-ins.") > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > ovn/controller/pinctrl.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c > index 8ae1f9bd6..c10693301 100644 > --- a/ovn/controller/pinctrl.c > +++ b/ovn/controller/pinctrl.c > @@ -464,6 +464,7 @@ static int > pinctrl_handle_buffered_packets(const struct flow *ip_flow, > struct dp_packet *pkt_in, > const struct match *md, bool is_arp) > + OVS_REQUIRES(pinctrl_mutex) > { > struct buffered_packets *bp; > struct dp_packet *clone; > @@ -512,7 +513,9 @@ pinctrl_handle_arp(struct rconn *swconn, const struct flow *ip_flow, > return; > } > > + ovs_mutex_lock(&pinctrl_mutex); > pinctrl_handle_buffered_packets(ip_flow, pkt_in, md, true); > + ovs_mutex_unlock(&pinctrl_mutex); > > /* Compose an ARP packet. */ > uint64_t packet_stub[128 / 8]; > @@ -3136,7 +3139,9 @@ pinctrl_handle_nd_ns(struct rconn *swconn, const struct flow *ip_flow, > return; > } > > + ovs_mutex_lock(&pinctrl_mutex); > pinctrl_handle_buffered_packets(ip_flow, pkt_in, md, false); > + ovs_mutex_unlock(&pinctrl_mutex); > > uint64_t packet_stub[128 / 8]; > struct dp_packet packet; > -- > 2.20.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev Thanks for the fix! Acked-by: Han Zhou <hzhou8@ebay.com>
On Wed, May 15, 2019 at 12:16:42PM -0700, Han Zhou wrote: > On Wed, May 15, 2019 at 6:34 AM Lorenzo Bianconi < > lorenzo.bianconi@redhat.com> wrote: > > > > pinctrl_handle_buffered_packets can insert new elements in > > buffered_packets_map hasmap and it runs concurrently with pinctrl_run > > starting from commit 3594ffab6b4b. Fix possible races grabbing > > pinctrl_mutex before running pinctrl_handle_buffered_packets > > > > Fixes: 3594ffab6b4b ("ovn-controller: Add a new thread in pinctrl module > to handle packet-ins.") > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > --- > > ovn/controller/pinctrl.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c > > index 8ae1f9bd6..c10693301 100644 > > --- a/ovn/controller/pinctrl.c > > +++ b/ovn/controller/pinctrl.c > > @@ -464,6 +464,7 @@ static int > > pinctrl_handle_buffered_packets(const struct flow *ip_flow, > > struct dp_packet *pkt_in, > > const struct match *md, bool is_arp) > > + OVS_REQUIRES(pinctrl_mutex) > > { > > struct buffered_packets *bp; > > struct dp_packet *clone; > > @@ -512,7 +513,9 @@ pinctrl_handle_arp(struct rconn *swconn, const struct > flow *ip_flow, > > return; > > } > > > > + ovs_mutex_lock(&pinctrl_mutex); > > pinctrl_handle_buffered_packets(ip_flow, pkt_in, md, true); > > + ovs_mutex_unlock(&pinctrl_mutex); > > > > /* Compose an ARP packet. */ > > uint64_t packet_stub[128 / 8]; > > @@ -3136,7 +3139,9 @@ pinctrl_handle_nd_ns(struct rconn *swconn, const > struct flow *ip_flow, > > return; > > } > > > > + ovs_mutex_lock(&pinctrl_mutex); > > pinctrl_handle_buffered_packets(ip_flow, pkt_in, md, false); > > + ovs_mutex_unlock(&pinctrl_mutex); > > > > uint64_t packet_stub[128 / 8]; > > struct dp_packet packet; > > -- > > 2.20.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks for the fix! > > Acked-by: Han Zhou <hzhou8@ebay.com> Thanks Lorenzo (and Han). I applied this to master.
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c index 8ae1f9bd6..c10693301 100644 --- a/ovn/controller/pinctrl.c +++ b/ovn/controller/pinctrl.c @@ -464,6 +464,7 @@ static int pinctrl_handle_buffered_packets(const struct flow *ip_flow, struct dp_packet *pkt_in, const struct match *md, bool is_arp) + OVS_REQUIRES(pinctrl_mutex) { struct buffered_packets *bp; struct dp_packet *clone; @@ -512,7 +513,9 @@ pinctrl_handle_arp(struct rconn *swconn, const struct flow *ip_flow, return; } + ovs_mutex_lock(&pinctrl_mutex); pinctrl_handle_buffered_packets(ip_flow, pkt_in, md, true); + ovs_mutex_unlock(&pinctrl_mutex); /* Compose an ARP packet. */ uint64_t packet_stub[128 / 8]; @@ -3136,7 +3139,9 @@ pinctrl_handle_nd_ns(struct rconn *swconn, const struct flow *ip_flow, return; } + ovs_mutex_lock(&pinctrl_mutex); pinctrl_handle_buffered_packets(ip_flow, pkt_in, md, false); + ovs_mutex_unlock(&pinctrl_mutex); uint64_t packet_stub[128 / 8]; struct dp_packet packet;
pinctrl_handle_buffered_packets can insert new elements in buffered_packets_map hasmap and it runs concurrently with pinctrl_run starting from commit 3594ffab6b4b. Fix possible races grabbing pinctrl_mutex before running pinctrl_handle_buffered_packets Fixes: 3594ffab6b4b ("ovn-controller: Add a new thread in pinctrl module to handle packet-ins.") Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- ovn/controller/pinctrl.c | 5 +++++ 1 file changed, 5 insertions(+)