>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]>
>Co-authored-by: Eelco Chaudron <[email protected]>
>---
> lib/ovs-lldp.c         |  8 ++++++++
> lib/ovs-lldp.h         |  1 +
> ofproto/ofproto-dpif.c |  5 ++++-
> tests/ofproto-dpif.at  | 33 +++++++++++++++++++++++++++++++++
> 4 files changed, 46 insertions(+), 1 deletion(-)
>
>diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c
>index 162311f..bfc69a3 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 ced67b0..c2f5b12 100644
>--- a/ofproto/ofproto-dpif.c
>+++ b/ofproto/ofproto-dpif.c
>@@ -2499,11 +2499,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 (!ofport->lldp) {
>-            ofproto->backer->need_revalidate = REV_RECONFIGURE;
>             ofport->lldp = lldp_create(ofport->up.netdev, ofport_->mtu, cfg);
>         }
>
>@@ -2515,6 +2515,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 7f273e5..edb6aa1 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])

CI test shows that setting lldp cause memory leaks, will try to find out the 
places.

>+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
>--
>1.8.3.1
>

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to