On 20 Oct 2022, at 17:06, [email protected] wrote:
> From: Lin Huang <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Lin Huang <[email protected]>
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 <[email protected]>
> ---
> 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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
