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 <vdas...@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 <echau...@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;
+ } 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,
+ int vlan, unsigned int idle_time)
+{
+ struct mac_entry *e;
+ 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;
}
+ /* 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;
+ }
+ } 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.
+ * 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);
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);
+
+ ovs_rwlock_wrlock(&ofproto->ml->rwlock);
+ mac_entry_set_idle_time(ofproto->ml, dl_src, vlan, INT_MAX);
+ 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);
+ unixctl_command_reply(conn, ds_cstr(&ds));
+ } else if (!strcmp(op, "del")) {
+ xlate_delete_static_mac_entry(ofproto, mac, vlan);
+ unixctl_command_reply(conn, NULL);
+ } else {
+ unixctl_command_reply_error(conn, "software error, unknown op");
+ }
+ 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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
