[ovs-dev] [PATCH v4] ofproto-dpif: APIs and CLI option to add/delete static fdb entry
Eelco Chaudron
echaudro at redhat.com
Wed May 26 08:51:58 UTC 2021
On 25 May 2021, at 20:11, Vasu Dasari wrote:
> Eelco, Thank you for the detailed review. My comments inline and will have
> a new pull-request shortly:
Thanks, take your time. I’m rather busy this week and early next week.
//Eelco
> *Vasu Dasari*
>
>
> On Tue, May 25, 2021 at 10:32 AM Eelco Chaudron <echaudro at redhat.com> wrote:
>
>> I did an initial code review. See comments inline below.
>>
>> //Eelco
>>
>>
>> On 22 May 2021, at 20:12, Vasu Dasari wrote:
>>
>>> Currently there is an option to add/flush/show ARP/ND neighbor. This
>> covers L3
>>> side. For L2 side, there is only fdb show command. This patch gives an
>> option
>>> to add/del an fdb entry via ovs-appctl.
>>>
>>> CLI command looks like:
>>>
>>> To add:
>>> ovs-appctl fdb/add <bridge> <port> <vlan> <Mac>
>>> ovs-appctl fdb/add br0 p1 0 50:54:00:00:00:05
>>>
>>> To del:
>>> ovs-appctl fdb/del <bridge> <port> <vlan> <Mac>
>>> ovs-appctl fdb/del br0 p1 0 50:54:00:00:00:05
>>>
>>> Added two new APIs to provide convenient interface to add and delete
>> static-macs.
>>> void xlate_add_static_mac_entry(const struct ofproto_dpif *, ofp_port_t
>> in_port,
>>> struct eth_addr dl_src, int vlan);
>>> void xlate_delete_static_mac_entry(const struct ofproto_dpif *,
>>> struct eth_addr dl_src, int vlan);
>>>
>>> 1. Static entry should not age. To indicate that entry being programmed
>> is a static entry,
>>> 'expires' field in 'struct mac_entry' will be set to a
>> MAC_ENTRY_AGE_STATIC_ENTRY. A
>>> check for this value is made while deleting mac entry as part of regular
>> aging process.
>>> 2. Another change to of mac-update logic, when a packet with same dl_src
>> as that of a
>>> static-mac entry arrives on any port, the logic will not modify the
>> expires field.
>>> 3. While flushing fdb entries, made sure static ones are not evicted.
>>>
>>> Added following tests:
>>> ofproto-dpif - static-mac add/del/flush
>>> ofproto-dpif - static-mac mac moves
>>>
>>> Signed-off-by: Vasu Dasari <vdasari at gmail.com>
>>> Reported-at:
>> https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048894.html
>>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1597752
>>> Tested-by: Eelco Chaudron <echaudro at redhat.com>
>>> ---
>>> v1:
>>> - Fixed 0-day robot warnings
>>> v2:
>>> - Fix valgrind error in the modified code in mac_learning_insert()
>> where a read is
>>> is performed on e->expires which is not initialized
>>> v3:
>>> - Addressed code review comments
>>> - Added more documentation
>>> - Fixed mac_entry_age() and is_mac_learning_update_needed() to have
>> common
>>> understanding of return values when mac_entry is a static one.
>>> - Added NEWS item
>>> v4:
>>> - Addressed code review comments
>>> - Static entries will not be purged when fdb/flush is performed.
>>> - Static entries will not be overwritten when packet with same dl_src
>> arrives on
>>> any port of switch
>>> - Provided bit more detail while doing fdb/add, to indicate if
>> static-mac is
>>> overriding already present entry
>>> - Separated test cases for a bit more clarity
>>> ---
>>> NEWS | 2 +
>>> lib/mac-learning.c | 61 +++++++++++++++++++++----
>>> lib/mac-learning.h | 11 +++++
>>> ofproto/ofproto-dpif-xlate.c | 28 ++++++++++++
>>> ofproto/ofproto-dpif-xlate.h | 5 ++
>>> ofproto/ofproto-dpif.c | 88 ++++++++++++++++++++++++++++++++++--
>>> tests/ofproto-dpif.at | 84 ++++++++++++++++++++++++++++++++++
>>> 7 files changed, 266 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/NEWS b/NEWS
>>> index 402ce5969..61ab61462 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -5,6 +5,8 @@ Post-v2.15.0
>>> - Userspace datapath:
>>> * Auto load balancing of PMDs now partially supports cross-NUMA
>> polling
>>> cases, e.g if all PMD threads are running on the same NUMA node.
>>> + * Added ability to add and delete static mac entries using:
>>> + 'ovs-appctl fdb/{add,del} <bridge> <port> <vlan> <mac>'
>>> - ovs-ctl:
>>> * New option '--no-record-hostname' to disable hostname
>> configuration
>>> in ovsdb on startup.
>>> diff --git a/lib/mac-learning.c b/lib/mac-learning.c
>>> index 3d5293d3b..ab58e2ab6 100644
>>> --- a/lib/mac-learning.c
>>> +++ b/lib/mac-learning.c
>>> @@ -35,12 +35,23 @@ COVERAGE_DEFINE(mac_learning_expired);
>>> COVERAGE_DEFINE(mac_learning_evicted);
>>> COVERAGE_DEFINE(mac_learning_moved);
>>>
>>> -/* Returns the number of seconds since 'e' (within 'ml') was last
>> learned. */
>>> +/*
>>> + * This function will return age of mac entry in the fdb. It
>>> + * will return either one of the following:
>>> + * 1. Number of seconds since 'e' (within 'ml') was last learned.
>>> + * 2. If the mac entry is a static entry, it returns
>>> + * MAC_ENTRY_AGE_STATIC_ENTRY
>>> + */
>>> int
>>> mac_entry_age(const struct mac_learning *ml, const struct mac_entry *e)
>>> {
>>> - time_t remaining = e->expires - time_now();
>>> - return ml->idle_time - remaining;
>>> + /* For static fdb entries, expires would be
>> MAC_ENTRY_AGE_STATIC_ENTRY */
>>> + if (MAC_ENTRY_AGE_STATIC_ENTRY == e->expires) {
>>> + return e->expires;
>>
>> My preference here would be to do “return MAC_ENTRY_AGE_STATIC_ENTRY;” but
>> I guess this is personal.
>>
>
> I do not have any preference either. I think it would be more readable, so
> I will change it to MAC_ENTRY_AGE_STATIC_ENTRY.
>
>
>>
>>> + } else {
>>> + time_t remaining = e->expires - time_now();
>>> + return ml->idle_time - remaining;
>>> + }
>>> }
>>>
>>> static uint32_t
>>> @@ -282,6 +293,18 @@ mac_learning_set_idle_time(struct mac_learning *ml,
>> unsigned int idle_time)
>>> }
>>> }
>>>
>>> +/* Changes the MAC aging timeout of a mac_entry to 'idle_time' seconds.
>> */
>>> +void
>>> +mac_entry_set_idle_time(struct mac_learning *ml, struct eth_addr mac,
>>
>> mac_learning_xxx is the prefix for all mac_learning functions, so this
>> should be something like mac_learning_set_entry_idle_time().
>>
> This API is setting idle time on a mac_entry. I see the following APIs
> acting on mac_entry using prefix mac_entry_XXX. And hence took liberty of
> using this prefix.
>
> mac_entry_get_port()
> mac_entry_set_port()
> mac_entry_age()
> mac_entry_is_grat_arp_locked()
>
>>
>>> + int vlan, unsigned int idle_time)
>>> +{
>>> + struct mac_entry *e;
>>
>> We need to do some form of normalize_idle_time() here. Maybe update the
>> normalize_idle_time() function to allow MAC_ENTRY_AGE_STATIC_ENTR?
>>
>
> Will remove this API in place of mac_learning_add_static_entry()
>
>
>>
>>> + e = mac_entry_lookup(ml, mac, vlan);
>>> + if (e) {
>>> + e->expires = idle_time;
>>> + }
>>> +}
>>> +
>>> /* Sets the maximum number of entries in 'ml' to 'max_entries',
>> adjusting it
>>> * to be within a reasonable range. */
>>> void
>>> @@ -336,6 +359,7 @@ mac_learning_insert(struct mac_learning *ml,
>>> e->vlan = vlan;
>>> e->grat_arp_lock = TIME_MIN;
>>> e->mlport = NULL;
>>> + e->expires = 0;
>>> COVERAGE_INC(mac_learning_learned);
>>> ml->total_learned++;
>>> } else {
>>> @@ -348,7 +372,10 @@ mac_learning_insert(struct mac_learning *ml,
>>> ovs_list_remove(&e->port_lru_node);
>>> ovs_list_push_back(&e->mlport->port_lrus, &e->port_lru_node);
>>> }
>>> - e->expires = time_now() + ml->idle_time;
>>> + /* Do not update 'expires' for static mac entry */
>>> + if (e->expires != MAC_ENTRY_AGE_STATIC_ENTRY) {
>>> + e->expires = time_now() + ml->idle_time;
>>> + }
>>>
>>> return e;
>>> }
>>> @@ -378,10 +405,16 @@ is_mac_learning_update_needed(const struct
>> mac_learning *ml,
>>> }
>>>
>>> mac = mac_learning_lookup(ml, src, vlan);
>>> - if (!mac || mac_entry_age(ml, mac)) {
>>> + /* If mac entry is missing it needs to be added to fdb */
>>> + if (!mac) {
>>> return true;
>>> }
>>
>> What happened here to the “if mac_entry_age(ml, mac) return true” case?
>> Was this a previous existing error, or did you remove it for another reason?
>>
>> Trying to understand the use case in the existing code, I think it was
>> there to make sure the table gets updated if not yet timed out so that the
>> e->expires gets updated.
>> So you probably need to add a check like (after the ==
>> MAC_ENTRY_AGE_STATIC_ENTRY below):
>>
>> age = mac_entry_age(ml, mac);
>> if (age > 0) {
>> return true;
>> }
>>
>
> Yes. You are right. Will fix this.
>
>>
>>> + /* if mac is a static entry, then there is no need to update */
>>> + if (mac_entry_age(ml, mac) == MAC_ENTRY_AGE_STATIC_ENTRY) {
>>> + return false;
>>> + }
>>> +
>>> if (is_gratuitous_arp) {
>>> /* We don't want to learn from gratuitous ARP packets that are
>>> * reflected back over bond members so we lock the learning
>> table. For
>>> @@ -513,13 +546,23 @@ mac_learning_expire(struct mac_learning *ml,
>> struct mac_entry *e)
>>> free(e);
>>> }
>>>
>>> -/* Expires all the mac-learning entries in 'ml'. */
>>> +/* Expires all the dynamic mac-learning entries in 'ml'. */
>>> void
>>> mac_learning_flush(struct mac_learning *ml)
>>> {
>>> - struct mac_entry *e;
>>> - while (get_lru(ml, &e)){
>>> - mac_learning_expire(ml, e);
>>> + 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;
>>> + }
>>
>> Do not think this will work, the list is sorted on most recently used.
>> If you hit the first static entry, you will stop flushing?
>>
>
> I have added the "fdb/flush" unit test to test this logic. If the entry
> being removed is a static entry entry and if first_static_mac is not set,
> then set it to point to the the entry. If it is a cache entry it will be
> deleted. During the loop processing all cache entries will be deleted
> leaving out all static entries, in the same order they were present before.
> We just need to break out from processing the static-macs list in a loop
> again. And hence the first_static_mac variable is introduced to track the
> first static-entry the while loop sees.
>
>
>>> + } else {
>>> + mac_learning_expire(ml, e);
>>> + }
>>> }
>>> hmap_shrink(&ml->table);
>>> }
>>> diff --git a/lib/mac-learning.h b/lib/mac-learning.h
>>> index 0ddab06cb..d8ff3172b 100644
>>> --- a/lib/mac-learning.h
>>> +++ b/lib/mac-learning.h
>>> @@ -57,6 +57,11 @@
>>> * list starting from the LRU end, deleting each entry that has been
>> idle too
>>> * long.
>>> *
>>> + * Fourth, a mac entry can be configured statically via API or appctl
>> commands.
>>> + * Static entries are programmed to have a age of
>> MAC_ENTRY_AGE_STATIC_ENTRY.
>>
>> Should be “an age”
>>
> Yes.
>
>>
>>> + * Age of static entries will not be updated by a receiving packet as
>> part of
>>> + * regular packet processing.
>>> + *
>>> * Finally, the number of MAC learning table entries has a configurable
>> maximum
>>> * size to prevent memory exhaustion. When a new entry must be
>> inserted but
>>> * the table is already full, the implementation uses an eviction
>> strategy
>>> @@ -94,6 +99,9 @@ struct mac_learning;
>>> /* Time, in seconds, before expiring a mac_entry due to inactivity. */
>>> #define MAC_ENTRY_DEFAULT_IDLE_TIME 300
>>>
>>> +/* Age value to represent a static entry */
>>> +#define MAC_ENTRY_AGE_STATIC_ENTRY INT_MAX
>>> +
>>> /* Time, in seconds, to lock an entry updated by a gratuitous ARP to
>> avoid
>>> * relearning based on a reflection from a bond member. */
>>> #define MAC_GRAT_ARP_LOCK_TIME 5
>>> @@ -202,6 +210,9 @@ bool mac_learning_set_flood_vlans(struct
>> mac_learning *ml,
>>> void mac_learning_set_idle_time(struct mac_learning *ml,
>>> unsigned int idle_time)
>>> OVS_REQ_WRLOCK(ml->rwlock);
>>> +void mac_entry_set_idle_time(struct mac_learning *ml, struct eth_addr
>> src,
>>> + int vlan, unsigned int idle_time)
>>> + OVS_REQ_WRLOCK(ml->rwlock);
>>
>> Rather than have a mac_entry_set_idle_time() API, would it not be better
>> to have an API that hides the internal implementation of a setting time to
>> INT_MAX?
>> We should have something like
>>
>> mac_learning_add_static_entry() to hide all this?
>>
>> I had some design choices to chose from. Either one involved adding an API.
> 1. Add mac_entry_set_idle_time() API to modify the expires field, where I
> could existing functions to get the job done with minute tweaks.
> 2. Add mac_learning_add_static_entry(), where some code duplication will be
> there for insert operation. It might not be a bad idea to take this
> approach. I can generate a pull request with this approach as well.
>
>>
>>> void mac_learning_set_max_entries(struct mac_learning *ml, size_t
>> max_entries)
>>> OVS_REQ_WRLOCK(ml->rwlock);
>>>
>>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>>> index 7108c8a30..4392a38f4 100644
>>> --- a/ofproto/ofproto-dpif-xlate.c
>>> +++ b/ofproto/ofproto-dpif-xlate.c
>>> @@ -8011,6 +8011,34 @@ xlate_mac_learning_update(const struct
>> ofproto_dpif *ofproto,
>>> update_learning_table__(xbridge, xbundle, dl_src, vlan,
>> is_grat_arp);
>>> }
>>>
>>> +void
>>> +xlate_add_static_mac_entry(const struct ofproto_dpif *ofproto,
>>> + ofp_port_t in_port,
>>> + struct eth_addr dl_src, int vlan)
>>> +{
>>> + xlate_delete_static_mac_entry(ofproto, dl_src, vlan);
>>> +
>>> + xlate_mac_learning_update(ofproto, in_port, dl_src, vlan, false);
>>
>> So what happens here if the entry got added by the call above, but due to
>> us not having a lock, the source port could have changed, and now we make
>> it static!? Guess a new API should solve this (see above).
>>
>>> + ovs_rwlock_wrlock(&ofproto->ml->rwlock);
>>> + mac_entry_set_idle_time(ofproto->ml, dl_src, vlan, INT_MAX);
>>
>> Why INT_MAX, there should be a #define for this.
>>
> This code will go away with mac_learning_add_static_entry() approach
>
>>
>>> + ovs_rwlock_unlock(&ofproto->ml->rwlock);
>>> +}
>>> +
>>> +void
>>> +xlate_delete_static_mac_entry(const struct ofproto_dpif *ofproto,
>>> + struct eth_addr dl_src, int vlan)
>>> +{
>>> + struct mac_entry *e = NULL;
>>> +
>>> + ovs_rwlock_wrlock(&ofproto->ml->rwlock);
>>> + e = mac_learning_lookup(ofproto->ml, dl_src, vlan);
>>> + if (e) {
>>> + mac_learning_expire(ofproto->ml, e);
>>> + }
>>> + ovs_rwlock_unlock(&ofproto->ml->rwlock);
>>> +}
>>> +
>>> void
>>> xlate_set_support(const struct ofproto_dpif *ofproto,
>>> const struct dpif_backer_support *support)
>>> diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
>>> index 3426a27b2..9e6e95756 100644
>>> --- a/ofproto/ofproto-dpif-xlate.h
>>> +++ b/ofproto/ofproto-dpif-xlate.h
>>> @@ -225,6 +225,11 @@ int xlate_send_packet(const struct ofport_dpif *,
>> bool oam, struct dp_packet *);
>>> void xlate_mac_learning_update(const struct ofproto_dpif *ofproto,
>>> ofp_port_t in_port, struct eth_addr
>> dl_src,
>>> int vlan, bool is_grat_arp);
>>> +void xlate_add_static_mac_entry(const struct ofproto_dpif *,
>>> + ofp_port_t in_port,
>>> + struct eth_addr dl_src, int vlan);
>>> +void xlate_delete_static_mac_entry(const struct ofproto_dpif *,
>>> + struct eth_addr dl_src, int vlan);
>>>
>>> void xlate_set_support(const struct ofproto_dpif *,
>>> const struct dpif_backer_support *);
>>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>>> index fd0b2fdea..97f2ac475 100644
>>> --- a/ofproto/ofproto-dpif.c
>>> +++ b/ofproto/ofproto-dpif.c
>>> @@ -5852,18 +5852,94 @@ ofproto_unixctl_fdb_show(struct unixctl_conn
>> *conn, int argc OVS_UNUSED,
>>> LIST_FOR_EACH (e, lru_node, &ofproto->ml->lrus) {
>>> struct ofbundle *bundle = mac_entry_get_port(ofproto->ml, e);
>>> char name[OFP_MAX_PORT_NAME_LEN];
>>> + int age = mac_entry_age(ofproto->ml, e);
>>>
>>> ofputil_port_to_string(ofbundle_get_a_port(bundle)->up.ofp_port,
>>> - NULL, name, sizeof name);
>>> - ds_put_format(&ds, "%5s %4d "ETH_ADDR_FMT" %3d\n",
>>> - name, e->vlan, ETH_ADDR_ARGS(e->mac),
>>> - mac_entry_age(ofproto->ml, e));
>>> + NULL, name, sizeof name);
>>> + ds_put_format(&ds, "%5s %4d "ETH_ADDR_FMT" ",
>>> + name, e->vlan, ETH_ADDR_ARGS(e->mac));
>>> + if (MAC_ENTRY_AGE_STATIC_ENTRY == age) {
>>> + ds_put_format(&ds, "static\n");
>>> + } else {
>>> + ds_put_format(&ds, "%3d\n", age);
>>> + }
>>> }
>>> ovs_rwlock_unlock(&ofproto->ml->rwlock);
>>> unixctl_command_reply(conn, ds_cstr(&ds));
>>> ds_destroy(&ds);
>>> }
>>>
>>> +static void
>>> +ofproto_unixctl_fdb_update(struct unixctl_conn *conn, int argc
>> OVS_UNUSED,
>>> + const char *argv[], void *aux OVS_UNUSED)
>>> +{
>>> + const char *br_name = argv[1];
>>> + const char *port_name = argv[2];
>>> + struct eth_addr mac;
>>> + uint16_t vlan = atoi(argv[3]);
>>> + const char *op = (const char *) aux;
>>> + const struct ofproto_dpif *ofproto;
>>> + ofp_port_t in_port = OFPP_NONE;
>>> + struct ofproto_port ofproto_port;
>>> + struct ds ds = DS_EMPTY_INITIALIZER;
>>> +
>>> + ofproto = ofproto_dpif_lookup_by_name(br_name);
>>> + if (!ofproto) {
>>> + unixctl_command_reply_error(conn, "no such bridge");
>>> + return;
>>> + }
>>> +
>>> + if (!eth_addr_from_string(argv[4], &mac)) {
>>> + unixctl_command_reply_error(conn, "bad MAC address");
>>> + return;
>>> + }
>>> +
>>> + if (ofproto_port_query_by_name(&ofproto->up, port_name,
>> &ofproto_port)) {
>>> + unixctl_command_reply_error(conn,
>>> + "software error, odp port is present but no ofp port");
>>> + return;
>>> + }
>>> + in_port = ofproto_port.ofp_port;
>>> + ofproto_port_destroy(&ofproto_port);
>>> +
>>> + if (!strcmp(op, "add")) {
>>> + /* Give a bit more information if the entry being added is
>> overriding
>>> + * an existing entry */
>>> + const struct mac_entry *mac_entry;
>>> + const struct ofbundle *bundle = NULL;
>>> + int age;
>>> +
>>> + ovs_rwlock_rdlock(&ofproto->ml->rwlock);
>>> + mac_entry = mac_learning_lookup(ofproto->ml, mac, vlan);
>>> + if (mac_entry) {
>>> + bundle = mac_entry ?
>>> + mac_entry_get_port(ofproto->ml, mac_entry) : NULL;
>>> + age = mac_entry->expires;
>>> + }
>>> + ovs_rwlock_unlock(&ofproto->ml->rwlock);
>>> +
>>> + if (bundle && ((strcmp(bundle->name, port_name)) ||
>>> + (age != MAC_ENTRY_AGE_STATIC_ENTRY))) {
>>> + char old_port_name[OFP_MAX_PORT_NAME_LEN];
>>> +
>>> +
>> ofputil_port_to_string(ofbundle_get_a_port(bundle)->up.ofp_port,
>>> + NULL, old_port_name, sizeof old_port_name);
>>> + ds_put_format(&ds, "Overriding already existing %s entry on
>> %s\n",
>>> + (age == MAC_ENTRY_AGE_STATIC_ENTRY) ? "static" :
>> "dynamic",
>>> + old_port_name);
>>> + }
>>> +
>>> + xlate_add_static_mac_entry(ofproto, in_port, mac, vlan);
>>
>> Is it impossible for xlate_add_static_mac_entry() to fail?
>>
>>> + unixctl_command_reply(conn, ds_cstr(&ds));
>>> + } else if (!strcmp(op, "del")) {
>>> + xlate_delete_static_mac_entry(ofproto, mac, vlan);
>>
>> Maybe an error if we tried to delete a non existing entry?
>>
>>> + unixctl_command_reply(conn, NULL);
>>> + } else {
>>> + unixctl_command_reply_error(conn, "software error, unknown op");
>>
>> Guess “op” might be to short would just spell it out, operation.
>>> + }
>>> + ds_destroy(&ds);
>>> +}
>>> +
>>> static void
>>> ofproto_unixctl_fdb_stats_clear(struct unixctl_conn *conn, int argc,
>>> const char *argv[], void *aux
>> OVS_UNUSED)
>>> @@ -6415,6 +6491,10 @@ ofproto_unixctl_init(void)
>>> }
>>> registered = true;
>>>
>>> + unixctl_command_register("fdb/add", "[bridge port vlan mac]", 4, 4,
>>> + ofproto_unixctl_fdb_update, "add");
>>> + unixctl_command_register("fdb/del", "[bridge port vlan mac]", 4, 4,
>>> + ofproto_unixctl_fdb_update, "del");
>>> unixctl_command_register("fdb/flush", "[bridge]", 0, 1,
>>> ofproto_unixctl_fdb_flush, NULL);
>>> unixctl_command_register("fdb/show", "bridge", 1, 1,
>>> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
>>> index 31064ed95..a6105df1d 100644
>>> --- a/tests/ofproto-dpif.at
>>> +++ b/tests/ofproto-dpif.at
>>> @@ -6753,6 +6753,90 @@ PORTNAME
>>> portName=p2
>>> ])])
>>>
>>> +AT_SETUP([ofproto-dpif - static-mac add/del/flush])
>>> +OVS_VSWITCHD_START([set bridge br0 fail-mode=standalone])
>>> +add_of_ports br0 1 2
>>> +
>>> +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 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 Check initial fdb
>>> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/ *[[0-9]]\{1,\}$//'
>> | grep -v port | sort], [0], [dnl
>>> + 1 0 50:54:00:00:00:01
>>> + 1 0 50:54:00:00:01:01 static
>>> + 2 0 50:54:00:00:00:02
>>> + 2 0 50:54:00:00:02:02 static
>>> +])
>>> +
>>> +dnl Remove static mac entry
>>> +AT_CHECK([ovs-appctl fdb/del br0 p1 0 50:54:00:00:01:01])
>>> +
>>> +dnl Check that entry is removed
>>> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | grep "50:54:00:00:01:01"],
>> [1], [dnl
>>> +])
>>> +
>>> +dnl Flush mac entries, only dynamic ones should be evicted.
>>> +AT_CHECK([ovs-appctl fdb/flush br0], [0], [dnl
>>> +table successfully flushed
>>> +])
>>> +
>>> +dnl Check that entry is removed
>>> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/ *[[0-9]]\{1,\}$//'
>> | grep -v port | sort], [0], [dnl
>>> + 2 0 50:54:00:00:02:02 static
>>> +])
>>> +
>>> +OVS_VSWITCHD_STOP
>>> +AT_CLEANUP
>>> +
>>> +AT_SETUP([ofproto-dpif - static-mac mac moves])
>>> +OVS_VSWITCHD_START([set bridge br0 fail-mode=standalone])
>>> +add_of_ports br0 1 2
>>> +
>>> +dnl Generate a dynamic entry
>>> +OFPROTO_TRACE([ovs-dummy], [in_port(1),eth(src=50:54:00:00:00:00)],
>> [-generate], [100,2])
>>> +
>>> +dnl Convert dynamically learnt dl_src to a static-mac
>>> +AT_CHECK([ovs-appctl fdb/add br0 p1 0 50:54:00:00:00:00], [0], [dnl
>>> +Overriding already existing dynamic entry on 1
>>> +])
>>> +
>>> +dnl Check fdb
>>> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/ *[[0-9]]\{1,\}$//'
>> | grep -v port | sort], [0], [dnl
>>> + 1 0 50:54:00:00:00:00 static
>>> +])
>>> +
>>> +dnl Move the static mac to different port
>>> +AT_CHECK([ovs-appctl fdb/add br0 p2 0 50:54:00:00:00:00], [0], [dnl
>>> +Overriding already existing static entry on 1
>>> +])
>>> +
>>> +dnl Check fdb
>>> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/ *[[0-9]]\{1,\}$//'
>> | grep -v port | sort], [0], [dnl
>>> + 2 0 50:54:00:00:00:00 static
>>> +])
>>> +
>>> +dnl static-mac should not be converted to a dynamic one when a packet
>> with same dl_src
>>> +dnl arrives on any port of the switch
>>> +dnl Packet arriving on p1
>>> +OFPROTO_TRACE([ovs-dummy], [in_port(1),eth(src=50:54:00:00:00:00)],
>> [-generate], [100,2])
>>> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/ *[[0-9]]\{1,\}$//'
>> | grep -v port | sort], [0], [dnl
>>> + 2 0 50:54:00:00:00:00 static
>>> +])
>>> +
>>> +dnl Packet arriving on p2
>>> +OFPROTO_TRACE([ovs-dummy], [in_port(2),eth(src=50:54:00:00:00:00)],
>> [-generate], [100,1])
>>> +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/ *[[0-9]]\{1,\}$//'
>> | grep -v port | sort], [0], [dnl
>>> + 2 0 50:54:00:00:00:00 static
>>> +])
>>> +
>>> +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.29.2
>>
>>
More information about the dev
mailing list