ofproto-dpif: Avoid unneccesary backer revalidation.

If lldp didn't change, we are not supposed to trigger backer revalidation. Without this patch, bridge_reconfigure() always trigger udpif revalidator because of lldp. Signed-off-by: lic121 <[email protected]> Signed-off-by: Eelco Chaudron <[email protected]> Co-authored-by: Eelco Chaudron <[email protected]> Acked-by: Paolo Valerio <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>
nevalosa · Jun 28, 2022 · 29a2f18 · 29a2f18
1 parent 509c327
commit 29a2f18
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 2 deletions.
diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c
@@ -738,6 +738,14 @@ lldp_put_packet(struct lldp *lldp, struct dp_packet *packet,
     ovs_mutex_unlock(&mutex);
 }
 
+/* Is LLDP enabled?
+ */
+bool
+lldp_is_enabled(struct lldp *lldp)
+{
+    return lldp ? lldp->enabled : false;
+}
+
 /* Configures the LLDP stack.
  */
 bool

diff --git a/lib/ovs-lldp.h b/lib/ovs-lldp.h
@@ -86,6 +86,7 @@ void lldp_run(struct lldpd *cfg);
 bool lldp_should_send_packet(struct lldp *cfg);
 bool lldp_should_process_flow(struct lldp *lldp, const struct flow *flow);
 bool lldp_configure(struct lldp *lldp, const struct smap *cfg);
+bool lldp_is_enabled(struct lldp *lldp);
 void lldp_process_packet(struct lldp *cfg, const struct dp_packet *);
 void lldp_put_packet(struct lldp *lldp, struct dp_packet *packet,
                      const struct eth_addr eth_src);

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
@@ -2491,11 +2491,11 @@ set_lldp(struct ofport *ofport_,
 {
     struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofport->up.ofproto);
+    bool old_enable = lldp_is_enabled(ofport->lldp);
     int error = 0;
 
-    if (cfg) {
+    if (cfg && !smap_is_empty(cfg)) {
         if (!ofport->lldp) {
-            ofproto->backer->need_revalidate = REV_RECONFIGURE;
             ofport->lldp = lldp_create(ofport->up.netdev, ofport_->mtu, cfg);
         }
 
@@ -2507,6 +2507,9 @@ set_lldp(struct ofport *ofport_,
     } else if (ofport->lldp) {
         lldp_unref(ofport->lldp);
         ofport->lldp = NULL;
+    }
+
+    if (lldp_is_enabled(ofport->lldp) != old_enable) {
         ofproto->backer->need_revalidate = REV_RECONFIGURE;
     }
 

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
@@ -29,6 +29,39 @@ AT_CHECK([ovs-appctl revalidator/wait])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - lldp revalidator event(REV_RECONFIGURE)])
+OVS_VSWITCHD_START(
+    [add-port br0 p1 -- set interface p1 ofport_request=1 type=dummy]
+)
+dnl first revalidation triggered by add interface
+AT_CHECK([ovs-appctl coverage/read-counter rev_reconfigure], [0], [dnl
+1
+])
+
+dnl enable lldp
+AT_CHECK([ovs-vsctl set interface p1 lldp:enable=true])
+AT_CHECK([ovs-appctl revalidator/wait])
+AT_CHECK([ovs-appctl coverage/read-counter rev_reconfigure], [0], [dnl
+2
+])
+
+dnl disable lldp
+AT_CHECK([ovs-vsctl set interface p1 lldp:enable=false])
+AT_CHECK([ovs-appctl revalidator/wait])
+AT_CHECK([ovs-appctl coverage/read-counter rev_reconfigure], [0], [dnl
+3
+])
+
+dnl remove lldp, no revalidation as lldp was disabled
+AT_CHECK([ovs-vsctl remove interface p1 lldp enable])
+AT_CHECK([ovs-appctl revalidator/wait])
+AT_CHECK([ovs-appctl coverage/read-counter rev_reconfigure], [0], [dnl
+3
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto-dpif - active-backup bonding (with primary)])
 
 dnl Create br0 with members p1, p2 and p7, creating bond0 with p1 and