Hi Eelco, I addressed your comments and also added a counter to track number of static entries in the switch.
Here is the new Patch v5 <https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/383731.html>. Please take a look. Thanks -Vasu *Vasu Dasari* On Wed, May 26, 2021 at 4:52 AM Eelco Chaudron <[email protected]> wrote: > > > 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 <[email protected]> > 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 <[email protected]> > >>> 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 <[email protected]> > >>> --- > >>> 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 > >> > >> > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
