Message ID | SY4PR01MB84380CF341B689C71F3570B7CD2A9@SY4PR01MB8438.ausprd01.prod.outlook.com |
---|---|
State | Superseded |
Headers | show |
Series | [ovs-dev] mac-learning: Fix learned fdb entries not age out issue. | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
ovsrobot/intel-ovs-compilation | success | test: success |
On 20 Oct 2022, at 17:06, miterv@outlook.com wrote: > From: Lin Huang <linhuang@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. > > Fixes: ccc24fc88d59 ("ofproto-dpif: APIs and CLI option to add/delete static fdb entry.") > Tested-by: Zhang Yuhuang <zhangyuhuang@ruijie.com.cn> > Signed-off-by: Lin Huang <linhuang@ruijie.com.cn> Hi Lin, Thanks for fixing the comments. There is one small nit which, maybe Ilya can fix at commit time. Also, next time, please add a version reference to the subject line if you send out an updated version (and some version history). Thanks, Eelco Acked-by: Eelco Chaudron <echaudro@redhat.com> > --- > lib/mac-learning.c | 36 +++++++++++++----------------------- > tests/ofproto-dpif.at | 23 +++++++++++++++++++++++ > 2 files changed, 36 insertions(+), 23 deletions(-) > > diff --git a/lib/mac-learning.c b/lib/mac-learning.c > index a60794fb2..e8ead51af 100644 > --- a/lib/mac-learning.c > +++ b/lib/mac-learning.c > @@ -176,12 +176,17 @@ get_lru(struct mac_learning *ml, struct mac_entry **e) > OVS_REQ_RDLOCK(ml->rwlock) > { > if (!ovs_list_is_empty(&ml->lrus)) { > - *e = mac_entry_from_lru_node(ml->lrus.next); > - return true; > - } else { > - *e = NULL; > - return false; > + struct mac_entry *entry; Please add cr/lf after definitions and beginning of code. > + 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 +623,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 8e993c585..eb4cd1896 100644 > --- a/tests/ofproto-dpif.at > +++ b/tests/ofproto-dpif.at > @@ -7287,6 +7287,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]) > +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=60:54:00:00:00:01)], [-generate], [100,2]) > +OFPROTO_TRACE([ovs-dummy], [in_port(2),eth(src=60:54:00:00:00:02)], [-generate], [100,1]) > + > +dnl Waiting for aging out. > +ovs-appctl time/warp 20000 > + > +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.27.0
diff --git a/lib/mac-learning.c b/lib/mac-learning.c index a60794fb2..e8ead51af 100644 --- a/lib/mac-learning.c +++ b/lib/mac-learning.c @@ -176,12 +176,17 @@ get_lru(struct mac_learning *ml, struct mac_entry **e) OVS_REQ_RDLOCK(ml->rwlock) { if (!ovs_list_is_empty(&ml->lrus)) { - *e = mac_entry_from_lru_node(ml->lrus.next); - return true; - } else { - *e = NULL; - return false; + struct mac_entry *entry; + 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 +623,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 8e993c585..eb4cd1896 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -7287,6 +7287,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]) +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=60:54:00:00:00:01)], [-generate], [100,2]) +OFPROTO_TRACE([ovs-dummy], [in_port(2),eth(src=60:54:00:00:00:02)], [-generate], [100,1]) + +dnl Waiting for aging out. +ovs-appctl time/warp 20000 + +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