[ovs-dev] OVN: grab pinctrl_mutex before runningpinctrl_handle_buffered_packets
diff mbox series

Message ID 3803c301564bf21d397030e82d57df81fe5c5a91.1557922974.git.lorenzo.bianconi@redhat.com
State New
Headers show
Series
  • [ovs-dev] OVN: grab pinctrl_mutex before runningpinctrl_handle_buffered_packets
Related show

Commit Message

Lorenzo Bianconi May 15, 2019, 1:33 p.m. UTC
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(+)

Comments

Han Zhou May 15, 2019, 7:16 p.m. UTC | #1
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>
Ben Pfaff June 7, 2019, 8 p.m. UTC | #2
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.

Patch
diff mbox series

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;