Hi Eelco, Thanks for reviewing.
The latest patch will be update soon. > -----Original Message----- > From: Eelco Chaudron <echau...@redhat.com> > Sent: Wednesday, October 19, 2022 8:06 PM > To: mit...@outlook.com > Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org; vdas...@gmail.com; Lin > Huang <linhu...@ruijie.com.cn>; Zhang Yuhuang > <zhangyuhu...@ruijie.com.cn> > Subject: Re: [PATCH] mac-learning: Fix learned fdb entries are not age out > issue. > > > > On 29 Jul 2022, at 13:16, mit...@outlook.com wrote: > > > From: Lin Huang <linhu...@ruijie.com.cn> > > > > After user add a static fdb entry, the get_lru() function will always > > return the > > static fdb entry. That's normal fdb entries will not age out through > mac_learning_run(). > > > > Fix the issue by modify the get_lru() function to check the entry->expires > field and > > not return the entry which entry->expires is > MAC_ENTRY_AGE_STATIC_ENTRY. > > > > Adding a unit test for this. > > Sorry for the late response, but it was dropped off of my review list :( > > One small nit, and a suggested test case change. > > The rest looks good! > > //Eelco > > > > Fixes: ccc24fc88d59 ("ofproto-dpif: APIs and CLI option to add/delete static > fdb entry.") > > Tested-by: Zhang Yuhuang <zhangyuhu...@ruijie.com.cn> > > Signed-off-by: Lin Huang <linhu...@ruijie.com.cn> > > --- > > lib/mac-learning.c | 37 ++++++++++++++----------------------- > > tests/ofproto-dpif.at | 23 +++++++++++++++++++++++ > > 2 files changed, 37 insertions(+), 23 deletions(-) > > > > diff --git a/lib/mac-learning.c b/lib/mac-learning.c > > index a60794fb2..713f81e63 100644 > > --- a/lib/mac-learning.c > > +++ b/lib/mac-learning.c > > @@ -175,13 +175,19 @@ static bool > > get_lru(struct mac_learning *ml, struct mac_entry **e) > > OVS_REQ_RDLOCK(ml->rwlock) > > { > > + struct mac_entry *entry; > > nit: the entry declaration can be moved inside the if statement. > > > + > > if (!ovs_list_is_empty(&ml->lrus)) { > > - *e = mac_entry_from_lru_node(ml->lrus.next); > > - return true; > > - } else { > > - *e = NULL; > > - return false; > > + LIST_FOR_EACH (entry, lru_node, &ml->lrus) { > > + if (entry->expires != MAC_ENTRY_AGE_STATIC_ENTRY) { > > + *e = entry; > > + return true; > > + } > > + } > > } > > + > > + *e = NULL; > > + return false; > > } > > > > static unsigned int > > @@ -618,25 +624,10 @@ mac_learning_expire(struct mac_learning *ml, > struct mac_entry *e) > > void > > mac_learning_flush(struct mac_learning *ml) > > { > > - struct mac_entry *e, *first_static_mac = NULL; > > - > > - while (get_lru(ml, &e) && (e != first_static_mac)) { > > - > > - /* Static mac should not be evicted. */ > > - if (MAC_ENTRY_AGE_STATIC_ENTRY == e->expires) { > > - > > - /* Make note of first static-mac encountered, so that this > while > > - * loop will break on visting this mac again via get_lru(). */ > > - if (!first_static_mac) { > > - first_static_mac = e; > > - } > > + struct mac_entry *e; > > > > - /* Remove from lru head and append it to tail. */ > > - ovs_list_remove(&e->lru_node); > > - ovs_list_push_back(&ml->lrus, &e->lru_node); > > - } else { > > - mac_learning_expire(ml, e); > > - } > > + while (get_lru(ml, &e)) { > > + mac_learning_expire(ml, e); > > } > > hmap_shrink(&ml->table); > > } > > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at > > index 2e91ae1a1..4b455315d 100644 > > --- a/tests/ofproto-dpif.at > > +++ b/tests/ofproto-dpif.at > > @@ -7195,6 +7195,29 @@ AT_CHECK([ovs-appctl coverage/read-counter > mac_learning_static_none_move], [0], > > OVS_VSWITCHD_STOP > > AT_CLEANUP > > > > +AT_SETUP([ofproto-dpif - static-mac learned mac age out]) > > +OVS_VSWITCHD_START([set bridge br0 fail-mode=standalone -- set bridge > br0 other_config:mac-aging-time=5]) > > 15 is the minimum value, so you might as well use it here. With the warp > command below, you can even leave it at the default. > > > +add_of_ports br0 1 2 > > + > > +dnl Add some static mac entries. > > +AT_CHECK([ovs-appctl fdb/add br0 p1 0 50:54:00:00:01:01]) > > +AT_CHECK([ovs-appctl fdb/add br0 p2 0 50:54:00:00:02:02]) > > + > > +dnl Generate some dynamic fdb entries on some ports. > > +OFPROTO_TRACE([ovs-dummy], [in_port(1),eth(src=50:54:00:00:00:01)], > [-generate], [100,2]) > > +OFPROTO_TRACE([ovs-dummy], [in_port(2),eth(src=50:54:00:00:00:02)], > [-generate], [100,1]) > > + > > +dnl Waiting for aging out. > > +sleep 16 > > Don’t wait 16 seconds, this will only cause the test run to take longer. Use > the > “ovs-appctl time/warp 20000” command here. > > > +dnl Count number of static entries remaining. > > +AT_CHECK_UNQUOTED([ovs-appctl fdb/stats-show br0 | grep expired], [0], > [dnl > > + Total number of expired MAC entries : 2 > > +]) > > + > > +OVS_VSWITCHD_STOP > > +AT_CLEANUP > > + > > AT_SETUP([ofproto-dpif - basic truncate action]) > > OVS_VSWITCHD_START > > add_of_ports br0 1 2 3 4 5 > > -- > > 2.32.0.windows.2 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev