[ovs-dev] [PATCH v9] ofproto-dpif: APIs and CLI option to add/delete static fdb entry

Vasu Dasari vdasari at gmail.com
Fri Jun 25 20:53:18 UTC 2021


Hi Eelco. 

Thanks for the review. Will update with your suggestions. 

I am out of town. Will do this next week. 

Thanks
Vasu

> On Jun 25, 2021, at 8:55 AM, Eelco Chaudron <echaudro at redhat.com> wrote:
> 
> 
> This one looks good, however, I would make some small changes.
> If you do, you can add my acked by to the new revision.
> 
> //Eelco
> 
> On 24 Jun 2021, at 17:57, 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> <vlan> <Mac> 
> ovs-appctl fdb/del br0 0 50:54:00:00:00:05
> 
> Added two new APIs to provide convenient interface to add and delete static-macs. 
> bool xlate_add_static_mac_entry(const struct ofproto_dpif *, ofp_port_t in_port, 
> struct eth_addr dl_src, int vlan); 
> bool 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. 
> 4. Updated "ovs-appctl fdb/stats-show br0" to display numberof static entries in switch
> 
> 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 
> v5: 
> - Addressed code review comments 
> - Added new total_static counter to count number of static entries. 
> - Removed mac_entry_set_idle_time() 
> - Added mac_learning_add_static_entry() and mac_learning_del_static_entry() 
> - Modified APIs xlate_add_static_mac_entry() and xlate_delete_static_mac_entry() 
> return 0 on success else a failure code 
> v6: 
> - Fixed a probable bug with Eelco's code review comments in 
> is_mac_learning_update_needed() 
> v7: 
> - Added a ovs-vswitchd.8 man page entry for fdb add/del commands 
> v8: 
> - Updaed with code review comments from Eelco. 
> - Renamed total_static to static_entries 
> - Added coverage counter mac_learning_static_none_move 
> - Fixed a possible bug with static_entries getting cleared via 
> fdb/stats-clear command 
> - Initialize static_entries in mac_learning_create() 
> - Modified fdb/del command by removing option to specify port-name 
> - Breakup ofproto_unixctl_fdb_update into ofproto_unixctl_fdb_add 
> and ofproto_unixctl_fdb_delete 
> - Updated test "static-mac add/del/flush" to have interleaved mac 
> entries before fdb/flush 
> - Updated test "static-mac mac move" to check for newly added 
> coverage counter mac_learning_static_none_move 
> v9: 
> - Updated source code comments and addressed code review comments 
> --- 
> NEWS | 4 + 
> lib/mac-learning.c | 155 +++++++++++++++++++++++++++++++---- 
> lib/mac-learning.h | 17 ++++ 
> ofproto/ofproto-dpif-xlate.c | 48 +++++++++-- 
> ofproto/ofproto-dpif-xlate.h | 5 ++ 
> ofproto/ofproto-dpif.c | 117 +++++++++++++++++++++++++- 
> tests/ofproto-dpif.at | 99 ++++++++++++++++++++++ 
> vswitchd/ovs-vswitchd.8.in | 5 ++ 
> 8 files changed, 421 insertions(+), 29 deletions(-)
> 
> diff --git a/NEWS b/NEWS 
> index db3faf4cc..12fb40054 100644 
> --- a/NEWS 
> +++ b/NEWS 
> @@ -20,6 +20,10 @@ Post-v2.15.0 
> - ovsdb-tool: 
> * New option '--election-timer' to the 'create-cluster' command to set the 
> leader election timer during cluster creation. 
> + - ovs-appctl: 
> + * Added ability to add and delete static mac entries using: 
> + 'ovs-appctl fdb/add <bridge> <port> <vlan> <mac>' 
> + 'ovs-appctl fdb/del <bridge> <vlan> <mac>'
> 
> v2.15.0 - 15 Feb 2021 
> diff --git a/lib/mac-learning.c b/lib/mac-learning.c 
> index 3d5293d3b..dd3f46a8b 100644 
> --- a/lib/mac-learning.c 
> +++ b/lib/mac-learning.c 
> @@ -34,13 +34,25 @@ COVERAGE_DEFINE(mac_learning_learned); 
> COVERAGE_DEFINE(mac_learning_expired); 
> COVERAGE_DEFINE(mac_learning_evicted); 
> COVERAGE_DEFINE(mac_learning_moved); 
> +COVERAGE_DEFINE(mac_learning_static_none_move);
> 
> -/* 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 MAC_ENTRY_AGE_STATIC_ENTRY; 
> + } else { 
> + time_t remaining = e->expires - time_now(); 
> + return ml->idle_time - remaining; 
> + } 
> }
> 
> static uint32_t 
> @@ -214,6 +226,7 @@ mac_learning_create(unsigned int idle_time) 
> ovs_refcount_init(&ml->ref_cnt); 
> ovs_rwlock_init(&ml->rwlock); 
> mac_learning_clear_statistics(ml); 
> + ml->static_entries = 0; 
> return ml; 
> }
> 
> @@ -309,16 +322,19 @@ mac_learning_may_learn(const struct mac_learning *ml, 
> }
> 
> /* Searches 'ml' for and returns a MAC learning entry for 'src_mac' in 'vlan', 
> - * inserting a new entry if necessary. The caller must have already verified, 
> - * by calling mac_learning_may_learn(), that 'src_mac' and 'vlan' are 
> - * learnable. 
> + * inserting a new entry if necessary. If entry being added is a 
> + * 1. cache entry: caller must have already verified, by calling 
> + * mac_learning_may_learn(), that 'src_mac' and 'vlan' are learnable. 
> + * 2. static entry: new mac static fdb entry will be created or if one 
> + * exists already, converts that entry to a static fdb type. 
> * 
> * If the returned MAC entry is new (that is, if it has a NULL client-provided 
> * port, as returned by mac_entry_get_port()), then the caller must initialize 
> * the new entry's port to a nonnull value with mac_entry_set_port(). */ 
> -struct mac_entry * 
> -mac_learning_insert(struct mac_learning *ml, 
> - const struct eth_addr src_mac, uint16_t vlan) 
> +static struct mac_entry * 
> +mac_learning_insert__(struct mac_learning *ml, const struct eth_addr src_mac, 
> + uint16_t vlan, bool is_static) 
> + OVS_REQ_WRLOCK(ml->rwlock) 
> { 
> struct mac_entry *e;
> 
> @@ -336,8 +352,11 @@ mac_learning_insert(struct mac_learning *ml, 
> e->vlan = vlan; 
> e->grat_arp_lock = TIME_MIN; 
> e->mlport = NULL; 
> - COVERAGE_INC(mac_learning_learned); 
> - ml->total_learned++; 
> + e->expires = 0; 
> + if (!is_static) { 
> + COVERAGE_INC(mac_learning_learned); 
> + ml->total_learned++; 
> + } 
> } else { 
> ovs_list_remove(&e->lru_node); 
> } 
> @@ -348,11 +367,78 @@ 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; 
> + 
> + /* Update 'expires' for mac entry. */ 
> + if (is_static) { 
> + /* Increment static_entries only if entry is a new one or entry is 
> + * converted from cache to static type. */ 
> + if (e->expires != MAC_ENTRY_AGE_STATIC_ENTRY) { 
> + ml->static_entries++; 
> + } 
> + e->expires = MAC_ENTRY_AGE_STATIC_ENTRY; 
> + } else { 
> + e->expires = time_now() + ml->idle_time;
> + }
> 
> return e; 
> }
> 
> +/* Adds a new dynamic mac entry to fdb. */ 
> +struct mac_entry * 
> +mac_learning_insert(struct mac_learning *ml, 
> + const struct eth_addr src_mac, uint16_t vlan) 
> +{ 
> + return mac_learning_insert__(ml, src_mac, vlan, false); 
> +} 
> + 
> +/* Adds a new static mac entry to fdb. It returns pointer to mac_entry 
> + * pointing to the fdb entry. 
> + * 
> + * Returns 'true' if mac entry is inserted, 'false' otherwise. 
> + */ 
> +bool 
> +mac_learning_add_static_entry(struct mac_learning *ml, 
> + const struct eth_addr src_mac, uint16_t vlan, 
> + void *in_port) 
> + OVS_EXCLUDED(ml->rwlock) 
> +{ 
> + struct mac_entry *mac = NULL; 
> + bool inserted = false; 
> + 
> + ovs_rwlock_wrlock(&ml->rwlock); 
> + mac = mac_learning_insert__(ml, src_mac, vlan, true); 
> + if (mac) { 
> + mac_entry_set_port(ml, mac, in_port); 
> + inserted = true; 
> + } 
> + ovs_rwlock_unlock(&ml->rwlock); 
> + 
> + return inserted; 
> +} 
> + 
> +/* Delete a static mac entry from fdb if it exists. 
> + * 
> + * Returns 'true' if mac entry is found, 'false' otherwise. 
> + */ 
> +bool 
> +mac_learning_del_static_entry(struct mac_learning *ml, 
> + const struct eth_addr dl_src, uint16_t vlan) 
> +{ 
> + struct mac_entry *mac = NULL; 
> + bool deleted = false; 
> + 
> + ovs_rwlock_wrlock(&ml->rwlock); 
> + mac = mac_learning_lookup(ml, dl_src, vlan); 
> + if (mac && mac_entry_age(ml, mac) == MAC_ENTRY_AGE_STATIC_ENTRY) { 
> + mac_learning_expire(ml, mac); 
> + ml->static_entries--; 
> + deleted = true; 
> + } 
> + ovs_rwlock_unlock(&ml->rwlock); 
> + 
> + return deleted; 
> +} 
> + 
> /* Checks whether a MAC learning update is necessary for MAC learning table 
> * 'ml' given that a packet matching 'src' was received on 'in_port' in 'vlan', 
> * and given that the packet was gratuitous ARP if 'is_gratuitous_arp' is 
> @@ -372,13 +458,32 @@ is_mac_learning_update_needed(const struct mac_learning *ml, 
> OVS_REQ_RDLOCK(ml->rwlock) 
> { 
> struct mac_entry *mac; 
> + int age;
> 
> if (!mac_learning_may_learn(ml, src, vlan)) {
> return false; 
> }
> 
> 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; 
> + } 
> + 
> + age = mac_entry_age(ml, mac); 
> + /* if mac is a static entry, then there is no need to update. */ 
> + if (age == MAC_ENTRY_AGE_STATIC_ENTRY) { 
> + /* coverage counter to increment when a packet with same 
> + * static-mac appears on a different port. */ 
> + if (mac_entry_get_port(ml, mac) != in_port) { 
> + COVERAGE_INC(mac_learning_static_none_move); 
> + } 
> + return false; 
> + } 
> + 
> + /* If entry is still alive, just update the mac_entry so, that expires 
> + * gets updated. */ 
> + if (age > 0) { 
> return true; 
> }
> 
> @@ -513,13 +618,29 @@ 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; 
> + } 
> + 
> + /* 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); 
> + } 
> } 
> hmap_shrink(&ml->table); 
> } 
> diff --git a/lib/mac-learning.h b/lib/mac-learning.h 
> index 0ddab06cb..270fbd70d 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 an age of MAC_ENTRY_AGE_STATIC_ENTRY. 
> + * 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 
> @@ -156,6 +164,7 @@ struct mac_learning { 
> unsigned long *flood_vlans; /* Bitmap of learning disabled VLANs. */ 
> unsigned int idle_time; /* Max age before deleting an entry. */ 
> size_t max_entries; /* Max number of learned MACs. */ 
> + size_t static_entries; /* Current number of static MAC entries. */ 
> struct ovs_refcount ref_cnt; 
> struct ovs_rwlock rwlock; 
> bool need_revalidate; 
> @@ -218,6 +227,14 @@ bool mac_learning_update(struct mac_learning *ml, struct eth_addr src, 
> int vlan, bool is_gratuitous_arp, bool is_bond, 
> void *in_port) 
> OVS_EXCLUDED(ml->rwlock); 
> +bool mac_learning_add_static_entry(struct mac_learning *ml, 
> + const struct eth_addr src, 
> + uint16_t vlan, void *in_port) 
> + OVS_EXCLUDED(ml->rwlock); 
> +bool mac_learning_del_static_entry(struct mac_learning *ml, 
> + const struct eth_addr src, 
> + uint16_t vlan) 
> + OVS_EXCLUDED(ml->rwlock);
> 
> /* Lookup. */ 
> struct mac_entry *mac_learning_lookup(const struct mac_learning *ml, 
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c 
> index a6f4ea334..c65723283 100644 
> --- a/ofproto/ofproto-dpif-xlate.c 
> +++ b/ofproto/ofproto-dpif-xlate.c 
> @@ -7991,26 +7991,58 @@ xlate_send_packet(const struct ofport_dpif *ofport, bool oam, 
> ofpacts.data, ofpacts.size, 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) 
> +/* Get xbundle for a ofp_port in a ofproto datapath. */ 
> +static struct xbundle* 
> +ofp_port_to_xbundle(const struct ofproto_dpif *ofproto, ofp_port_t ofp_port) 
> { 
> struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp); 
> struct xbridge *xbridge; 
> - struct xbundle *xbundle;
> 
> xbridge = xbridge_lookup(xcfg, ofproto); 
> if (!xbridge) { 
> - return; 
> + return NULL; 
> }
> 
> - xbundle = lookup_input_bundle__(xbridge, in_port, NULL); 
> + return lookup_input_bundle__(xbridge, ofp_port, NULL); 
> +} 
> + 
> +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) 
> +{ 
> + struct xbundle *xbundle = NULL; 
> + 
> + xbundle = ofp_port_to_xbundle(ofproto, in_port); 
> if (!xbundle) { 
> return; 
> }
> 
> - update_learning_table__(xbridge, xbundle, dl_src, vlan, is_grat_arp); 
> + update_learning_table__(xbundle->xbridge, 
> + xbundle, dl_src, vlan, is_grat_arp); 
> +} 
> + 
> +bool 
> +xlate_add_static_mac_entry(const struct ofproto_dpif *ofproto, 
> + ofp_port_t in_port, 
> + struct eth_addr dl_src, int vlan) 
> +{ 
> + struct xbundle *xbundle = ofp_port_to_xbundle(ofproto, in_port); 
> + 
> + /* Return here if xbundle is NULL. */ 
> + if (!xbundle || (xbundle == &ofpp_none_bundle)) { 
> + return false; 
> + } 
> + 
> + return mac_learning_add_static_entry(ofproto->ml, dl_src, vlan, 
> + xbundle->ofbundle); 
> +} 
> + 
> +bool 
> +xlate_delete_static_mac_entry(const struct ofproto_dpif *ofproto, 
> + struct eth_addr dl_src, int vlan) 
> +{ 
> + return mac_learning_del_static_entry(ofproto->ml, dl_src, vlan); 
> }
> 
> void 
> diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h 
> index 3426a27b2..851088d79 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); 
> +bool xlate_add_static_mac_entry(const struct ofproto_dpif *, 
> + ofp_port_t in_port, 
> + struct eth_addr dl_src, int vlan); 
> +bool 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 47db9bb57..c2352b318 100644 
> --- a/ofproto/ofproto-dpif.c 
> +++ b/ofproto/ofproto-dpif.c 
> @@ -5854,18 +5854,120 @@ 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_add(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 struct ofproto_dpif *ofproto; 
> + ofp_port_t in_port = OFPP_NONE; 
> + struct ofproto_port ofproto_port; 
> + struct ds ds = DS_EMPTY_INITIALIZER; 
> + const struct mac_entry *mac_entry; 
> + const struct ofbundle *bundle = NULL; 
> + int age; 
> + 
> + 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); 
> + 
> + /* Give a bit more information if the entry being added is overriding 
> + * an existing entry. */ 
> + 
> + ovs_rwlock_rdlock(&ofproto->ml->rwlock); 
> + mac_entry = mac_learning_lookup(ofproto->ml, mac, vlan); 
> + if (mac_entry) { 
> + bundle = mac_entry_get_port(ofproto->ml, mac_entry); 
> + 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); 
> + } 
> + 
> + if (!xlate_add_static_mac_entry(ofproto, in_port, mac, vlan)) { 
> + ds_put_format(&ds, "could not add static mac entry\n"); 
> + unixctl_command_reply_error(conn, ds_cstr(&ds));
> 
> Think on error you do not need the potential warning, just the error message, so it could become:
> 
> unixctl_command_reply_error(conn, "could not add static mac entry\n");
> 
> + } else { 
> + unixctl_command_reply(conn, ds_cstr(&ds)); 
> + } 
> + 
> + ds_destroy(&ds); 
> +} 
> + 
> +static void 
> +ofproto_unixctl_fdb_delete(struct unixctl_conn *conn, int argc OVS_UNUSED,
> + const char *argv[], void *aux OVS_UNUSED) 
> +{ 
> + const char *br_name = argv[1]; 
> + struct eth_addr mac; 
> + uint16_t vlan = atoi(argv[2]); 
> + const struct ofproto_dpif *ofproto; 
> + struct ds ds = DS_EMPTY_INITIALIZER;
> 
> Delete the above line.
> 
> + 
> + 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[3], &mac)) { 
> + unixctl_command_reply_error(conn, "bad MAC address"); 
> + return; 
> + } 
> + if (!xlate_delete_static_mac_entry(ofproto, mac, vlan)) { 
> + ds_put_format(&ds, "could not find static mac entry\n"); 
> + unixctl_command_reply_error(conn, ds_cstr(&ds));
> 
> Replace the two above with:
> 
>        unixctl_command_reply_error(conn, “could not find static mac entry\n”);
> + } else { 
> + unixctl_command_reply(conn, ds_cstr(&ds));
> 
> Change the above with:
> 
> 		unixctl_command_reply(conn, NULL);
> + } 
> + 
> + ds_destroy(&ds);
> 
> Remove the above.
> 
> +} 
> + 
> static void 
> ofproto_unixctl_fdb_stats_clear(struct unixctl_conn *conn, int argc, 
> const char *argv[], void *aux OVS_UNUSED) 
> @@ -5911,6 +6013,9 @@ ofproto_unixctl_fdb_stats_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
> ds_put_format(&ds, " Current/maximum MAC entries in the table: %" 
> PRIuSIZE"/%"PRIuSIZE"\n", 
> hmap_count(&ofproto->ml->table), ofproto->ml->max_entries); 
> + ds_put_format(&ds, 
> + " Current static MAC entries in the table : %"PRIuSIZE"\n", 
> + ofproto->ml->static_entries); 
> ds_put_format(&ds, 
> " Total number of learned MAC entries : %"PRIu64"\n", 
> ofproto->ml->total_learned); 
> @@ -6417,6 +6522,10 @@ ofproto_unixctl_init(void) 
> } 
> registered = true;
> 
> + unixctl_command_register("fdb/add", "[bridge port vlan mac]", 4, 4, 
> + ofproto_unixctl_fdb_add, NULL); 
> + unixctl_command_register("fdb/del", "[bridge vlan mac]", 3, 3, 
> + ofproto_unixctl_fdb_delete, NULL); 
> 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..88035bd10 100644 
> --- a/tests/ofproto-dpif.at 
> +++ b/tests/ofproto-dpif.at 
> @@ -6753,6 +6753,105 @@ 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 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 
> +]) 
> + 
> +# Add some more cache and static entries, to test out flush operation 
> +for i in 0 1 2 3 4 5 6 7 8 9 a b c d e f; do 
> + ovs-appctl ofproto/trace ovs-dummy "in_port(1),eth(src=50:54:00:00:0$i:ff)" -generate 
> + ovs-appctl fdb/add br0 p2 0 50:54:00:0$i:00:ff 
> +done 
> + 
> +for i in 0 1 2 3 4 5 6 7 8 9 a b c d e f; do 
> + ovs-appctl fdb/add br0 p2 0 50:54:00:0$i:00:ff 
> +done 
> + 
> +dnl Flush mac entries, only dynamic ones should be evicted. 
> +AT_CHECK([ovs-appctl fdb/flush br0], [0], [dnl 
> +table successfully flushed 
> +]) 
> + 
> +dnl Count number of static entries remaining. 
> +AT_CHECK_UNQUOTED([ovs-appctl fdb/stats-show br0 | grep static], [0], [dnl 
> + Current static MAC entries in the table : 17
> +]) 
> + 
> +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 
> +]) 
> + 
> +dnl Check mac_move coverage counter mac_learning_static_none_move. 
> +AT_CHECK([ovs-appctl coverage/read-counter mac_learning_static_none_move], [0], [dnl 
> +1 
> +]) 
> + 
> +OVS_VSWITCHD_STOP 
> +AT_CLEANUP 
> + 
> AT_SETUP([ofproto-dpif - basic truncate action]) 
> OVS_VSWITCHD_START 
> add_of_ports br0 1 2 3 4 5 
> diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in 
> index 50dad7208..08920600e 100644 
> --- a/vswitchd/ovs-vswitchd.8.in 
> +++ b/vswitchd/ovs-vswitchd.8.in 
> @@ -159,6 +159,11 @@ If \fIbridge\fR is not specified, then displays detailed information about all 
> bridges with RSTP enabled. 
> .SS "BRIDGE COMMANDS" 
> These commands manage bridges. 
> +.IP "\fBfdb/add\fR \fIbridge\fR \fIport\fR \fIvlan\fR \fImac\fR" 
> +.IQ "\fBfdb/del\fR \fIbridge\fR \fIport\fR \fIvlan\fR \fImac\fR" 
> +Adds/deletes \fImac\fR address on a \fIbridge\fR to/from \fIport\fR and 
> +\fIvlan\fR. This utility is can be used to pre-populate fdb table without 
> +relying on dynamic mac learning. 
> .IP "\fBfdb/flush\fR [\fIbridge\fR]" 
> Flushes \fIbridge\fR MAC address learning table, or all learning tables 
> if no \fIbridge\fR is given. 
> -- 
> 2.29.2
> 


More information about the dev mailing list