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.
This patch also fix lldp memory leak bug:
lldp_create() malloc memory for lldp->lldpd->g_hardware. lldp_unref
is supposed to free the memory, but it doesn't.
Signed-off-by: lic121
Signed-off-by: Eelco Chaudron
Co-authored-by: Eelco Chaudron
---
lib/lldp/lldpd.c | 10 +++---
lib/ovs-lldp.c | 8
lib/ovs-lldp.h | 1 +
ofproto/ofproto-dpif.c | 7 +--
tests/ofproto-dpif.at | 33 +
5 files changed, 50 insertions(+), 9 deletions(-)
diff --git a/lib/lldp/lldpd.c b/lib/lldp/lldpd.c
index 403f1f5..4bff7b0 100644
--- a/lib/lldp/lldpd.c
+++ b/lib/lldp/lldpd.c
@@ -140,13 +140,9 @@ lldpd_cleanup(struct lldpd *cfg)
VLOG_DBG("cleanup all ports");
LIST_FOR_EACH_SAFE (hw, h_entries, >g_hardware) {
-if (!hw->h_flags) {
-ovs_list_remove(>h_entries);
-lldpd_remote_cleanup(hw, NULL, true);
-lldpd_hardware_cleanup(cfg, hw);
-} else {
-lldpd_remote_cleanup(hw, NULL, false);
-}
+ovs_list_remove(>h_entries);
+lldpd_remote_cleanup(hw, NULL, true);
+lldpd_hardware_cleanup(cfg, hw);
}
VLOG_DBG("cleanup all chassis");
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();
}
+/* 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
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
