diff mbox

[ovs-dev] ofproto/bond: Make bond_may_recirc() private within bond.c

Message ID 1489107147-13940-1-git-send-email-azhou@ovn.org
State Accepted
Headers show

Commit Message

Andy Zhou March 10, 2017, 12:52 a.m. UTC 
Minor refactoring to make the bond code easier to read.

Signed-off-by: Andy Zhou <azhou@ovn.org>
---
 ofproto/bond.c               | 26 ++++++++++++++++++--------
 ofproto/bond.h               |  1 -
 ofproto/ofproto-dpif-xlate.c | 12 +++---------
 3 files changed, 21 insertions(+), 18 deletions(-)

Comments

Ben Pfaff April 14, 2017, 9:32 p.m. UTC | #1 
On Thu, Mar 09, 2017 at 04:52:27PM -0800, Andy Zhou wrote:
> Minor refactoring to make the bond code easier to read.
> 
> Signed-off-by: Andy Zhou <azhou@ovn.org>

Acked-by: Ben Pfaff <blp@ovn.org>
Andy Zhou April 17, 2017, 9:09 p.m. UTC | #2 
On Fri, Apr 14, 2017 at 2:32 PM, Ben Pfaff <blp@ovn.org> wrote:
> On Thu, Mar 09, 2017 at 04:52:27PM -0800, Andy Zhou wrote:
>> Minor refactoring to make the bond code easier to read.
>>
>> Signed-off-by: Andy Zhou <azhou@ovn.org>
>
> Acked-by: Ben Pfaff <blp@ovn.org>

Thanks for the review. Pushed to master.
diff mbox

Patch

diff --git a/ofproto/bond.c b/ofproto/bond.c
index ef65a2a3c1b4..d46b623f6ce8 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -926,7 +926,7 @@  bond_recirculation_account(struct bond *bond)
     }
 }
 
-bool
+static bool
 bond_may_recirc(const struct bond *bond)
 {
     return bond->balance == BM_TCP && bond->recirc_id;
@@ -960,15 +960,25 @@  void
 bond_update_post_recirc_rules(struct bond *bond, uint32_t *recirc_id,
                               uint32_t *hash_basis)
 {
-    ovs_rwlock_wrlock(&rwlock);
-    if (bond_may_recirc(bond)) {
-        *recirc_id = bond->recirc_id;
-        *hash_basis = bond->basis;
-        bond_update_post_recirc_rules__(bond, false);
-    } else {
+    bool may_recirc = bond_may_recirc(bond);
+
+    if (may_recirc) {
+        /* To avoid unnecessary locking, bond_may_recirc() is first
+         * called outside of the 'rwlock'. After acquiring the lock,
+         * check again to make sure bond configuration has not been changed. */
+        ovs_rwlock_wrlock(&rwlock);
+        may_recirc = bond_may_recirc(bond);
+        if (may_recirc) {
+            *recirc_id = bond->recirc_id;
+            *hash_basis = bond->basis;
+            bond_update_post_recirc_rules__(bond, false);
+        }
+        ovs_rwlock_unlock(&rwlock);
+    }
+
+    if (!may_recirc) {
         *recirc_id = *hash_basis = 0;
     }
-    ovs_rwlock_unlock(&rwlock);
 }
 
 
diff --git a/ofproto/bond.h b/ofproto/bond.h
index d3a2256402da..e7c3d9bc35dd 100644
--- a/ofproto/bond.h
+++ b/ofproto/bond.h
@@ -122,5 +122,4 @@  void bond_rebalance(struct bond *);
 */
 void bond_update_post_recirc_rules(struct bond *, uint32_t *recirc_id,
                                    uint32_t *hash_basis);
-bool bond_may_recirc(const struct bond *);
 #endif /* bond.h */
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 8ce6a5939df6..6dfbccc3b13e 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1987,15 +1987,9 @@  output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
         struct flow_wildcards *wc = ctx->wc;
         struct ofport_dpif *ofport;
 
-        if (ctx->xbridge->support.odp.recirc
-            && bond_may_recirc(out_xbundle->bond)) {
-            /* To avoid unnecessary locking, bond_may_recirc() is first
-             * called outside of the 'rwlock'. After acquiring the lock,
-             * bond_update_post_recirc_rules() will check again to make
-             * sure bond configuration has not been changed.
-             *
-             * In case recirculation is not actually in use, 'xr.recirc_id'
-             * will be set to '0', Since a valid 'recirc_id' can
+        if (ctx->xbridge->support.odp.recirc) {
+            /* In case recirculation is not actually in use, 'xr.recirc_id'
+             * will be set to '0', since a valid 'recirc_id' can
              * not be zero.  */
             bond_update_post_recirc_rules(out_xbundle->bond,
                                           &xr.recirc_id,