On 9 Jan 2022, at 14:44, lic121 wrote:
> If lldp didn't change, we are not supposed to trigger backer
> revalidation.
>
> Signed-off-by: lic121 <[email protected]>
> ---
> ofproto/ofproto-dpif.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 1cdef18..1bc9ec7 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -2461,10 +2461,10 @@ set_lldp(struct ofport *ofport_,
> struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
> struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofport->up.ofproto);
> int error = 0;
> + struct lldp *old_lldp = ofport->lldp;
>
> if (cfg) {
> if (!ofport->lldp) {
> - ofproto->backer->need_revalidate = REV_RECONFIGURE;
> ofport->lldp = lldp_create(ofport->up.netdev, ofport_->mtu, cfg);
> }
>
> @@ -2476,6 +2476,8 @@ set_lldp(struct ofport *ofport_,
> } else if (ofport->lldp) {
> lldp_unref(ofport->lldp);
> ofport->lldp = NULL;
> + }
> + if (old_lldp != ofport->lldp) {
Same as for your first patch in the series. Here you only check if the pointer
changes, so if you enable and then disable LLDP it will not trigger a
REV_RECONFIGURE?
Maybe something like this will work (not tested):
diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c
index 162311fa4..0f374df67 100644
--- a/lib/ovs-lldp.c
+++ b/lib/ovs-lldp.c
@@ -761,6 +761,14 @@ lldp_configure(struct lldp *lldp, const struct smap *cfg)
OVS_EXCLUDED(mutex)
return true;
}
+/* Is LLDP enabled?
+ */
+bool
+lldp_is_enabled(struct lldp *lldp)
+{
+ return lldp ? lldp->enabled : false;
+}
+
/* Create an LLDP stack instance. At the moment there is one per bridge port.
*/
struct lldp *
diff --git a/lib/ovs-lldp.h b/lib/ovs-lldp.h
index 0e536e8c2..661ac4e18 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 1bc9ec7e4..edfe7e123 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2461,7 +2461,7 @@ 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;
- struct lldp *old_lldp = ofport->lldp;
if (cfg) {
if (!ofport->lldp) {
@@ -2477,7 +2477,8 @@ set_lldp(struct ofport *ofport_,
lldp_unref(ofport->lldp);
ofport->lldp = NULL;
}
- if (old_lldp != ofport->lldp) {
+
+ if (lldp_is_enabled(ofport->lldp) != old_enable) {
ofproto->backer->need_revalidate = REV_RECONFIGURE;
}
Maybe you could also add some test cases to make sure this works?
> ofproto->backer->need_revalidate = REV_RECONFIGURE;
> }
>
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
