>On 6 Feb 2022, at 4:58, lic121 wrote: > >>> 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. > > >Ack, I’ll wait for reviewing once you found/fix it.
v4 is uploaded > >PS: You can add me as a co-author and sign off in your next iteration for the >changes I made. > Sure, please let me know if I haven't done it in the right way. >>> +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
