diff mbox series

[ovs-dev,v2,2/2] ofproto-dpif: avoid unneccesary backer revalidation

Message ID bf88bc5133ee8ef0e62c84967760628deaf4b26a.1653574451.git.lic121@chinatelecom.cn
State Accepted
Commit 29a2f183543b2d5bdcc958952afdcc55f520ad2d
Headers show
Series fix revalidation triggers | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Cheng Li May 26, 2022, 2:25 p.m. UTC 
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 <lic121@chinatelecom.cn>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Co-authored-by: Eelco Chaudron <echaudro@redhat.com>
---
 lib/ovs-lldp.c         |  8 ++++++++
 lib/ovs-lldp.h         |  1 +
 ofproto/ofproto-dpif.c |  7 +++++--
 tests/ofproto-dpif.at  | 33 +++++++++++++++++++++++++++++++++
 4 files changed, 47 insertions(+), 2 deletions(-)

Comments

Paolo Valerio June 8, 2022, 5:14 p.m. UTC | #1 
lic121 <lic121@chinatelecom.cn> writes:

> 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 <lic121@chinatelecom.cn>
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> Co-authored-by: Eelco Chaudron <echaudro@redhat.com>
> ---

LGTM,

Acked-by: Paolo Valerio <pvalerio@redhat.com>
Ilya Maximets June 28, 2022, 4:15 p.m. UTC | #2 
On 6/8/22 19:14, Paolo Valerio wrote:
> lic121 <lic121@chinatelecom.cn> writes:
> 
>> 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 <lic121@chinatelecom.cn>
>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>> Co-authored-by: Eelco Chaudron <echaudro@redhat.com>
>> ---
> 
> LGTM,
> 
> Acked-by: Paolo Valerio <pvalerio@redhat.com>

Thanks!  Applied to master and 2.17.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c
index a9d205e..2d13e97 100644
--- 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
index 0e536e8..661ac4e 100644
--- 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
index 7a1bb0d..3ae44f0 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2490,11 +2490,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);
         }
 
@@ -2506,6 +2506,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
index dbb3b6d..1c9a94c 100644
--- 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