On 3 Aug 2022, at 15:52, lin huang wrote:

> Hi Vasu & Eelco,
>
> Pls review my patch.

Just returned from PTO, will try to add this to my next week’s list of things 
to review.

//Eelco


>> -----Original Message-----
>> From: lin huang <[email protected]>
>> Sent: Friday, July 29, 2022 7:17 PM
>> To: [email protected]; [email protected]; [email protected];
>> [email protected]
>> Cc: Lin Huang <[email protected]>; Zhang Yuhuang
>> <[email protected]>
>> Subject: [PATCH] mac-learning: Fix learned fdb entries are not age out issue.
>>
>> 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]>
>> ---
>>  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;
>> +
>>      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])
>> +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
>> +
>> +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
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to