> > >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? Good catch, lldp_configure returns true even the cfg has eabled=false Will address this in the next version >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
