| Message ID | 20210514193309.80837-1-vdasari@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v3] ofproto-dpif: APIs and CLI option to add/delete static fdb entry | expand |
On 14 May 2021, at 21:33, 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. ovs-appctl command looks like this: > > 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 > > 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. > Another check as part of mac-update process, when a packet with same source mac as this > entry arrives on the configured port will not modify the expires field > > 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); > > Signed-off-by: Vasu Dasari <vdasari@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 I did some testing and found the issues below, once fixed, I’ll do a full code review. When you do an FDB flush, it also clears the static FDB entries. I think this is wrong, as all hardware vendors I know will retain the static FDB entries. When you create a static entry for lets say Port A, when a packet with the same MAC comes from Port B the entry will be updated with Port B. This should not happen for static entries. When you add a static MAC entry, the command just returns "OK". Other commands do not return anything on a successful addition. You should either follow the same behavior or be more verbose (Static FDB successfully added?) on your return. Also, it might be nice to be more verbose when you replace an existing static or dynamic FDB entry, i.e. especially if the physical port is different (mac move case). Cheers, Eelco
Thank you Eelco for testing the patch. My responses are inline: *Vasu Dasari* On Thu, May 20, 2021 at 5:20 AM Eelco Chaudron <echaudro@redhat.com> wrote: > > > On 14 May 2021, at 21:33, 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. ovs-appctl command looks like > this: > > > > 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 > > > > 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. > > Another check as part of mac-update process, when a packet with same > source mac as this > > entry arrives on the configured port will not modify the expires field > > > > 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); > > > > Signed-off-by: Vasu Dasari <vdasari@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 > > I did some testing and found the issues below, once fixed, I’ll do a full > code review. When you do an FDB flush, it also clears the static FDB entries. I think > this is wrong, as all hardware vendors I know will retain the static FDB > entries. > I took the liberty of having fdb/flush clear static entries as well. I do not mind changing the code to delete only dynamic ones. Will take care of this. > When you create a static entry for lets say Port A, when a packet with the > same MAC comes from Port B the entry will be updated with Port B. This > should not happen for static entries. > > I agree. I tested this case too. It fails. Will fix it. > When you add a static MAC entry, the command just returns "OK". Other > commands do not return anything on a successful addition. You should either > follow the same behavior or be more verbose (Static FDB successfully > added?) on your return. > > Sure, will make the command return error string only if it fails, otherwise will be silent. > Also, it might be nice to be more verbose when you replace an existing > static or dynamic FDB entry, i.e. especially if the physical port is > different (mac move case). > > Sure, will add more detailed output. > Cheers, > > > Eelco > >
On 20 May 2021, at 19:09, Vasu Dasari wrote: > Thank you Eelco for testing the patch. > > My responses are inline: Thanks, looking forward for your next revision. Would be good if you can add a test case for the mac move and flush change. //Eelco > *Vasu Dasari* > > > On Thu, May 20, 2021 at 5:20 AM Eelco Chaudron <echaudro@redhat.com> wrote: > >> >> >> On 14 May 2021, at 21:33, 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. ovs-appctl command looks like >> this: >>> >>> 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 >>> >>> 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. >>> Another check as part of mac-update process, when a packet with same >> source mac as this >>> entry arrives on the configured port will not modify the expires field >>> >>> 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); >>> >>> Signed-off-by: Vasu Dasari <vdasari@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 >> >> I did some testing and found the issues below, once fixed, I’ll do a full >> code review. > > When you do an FDB flush, it also clears the static FDB entries. I think >> this is wrong, as all hardware vendors I know will retain the static FDB >> entries. >> > > I took the liberty of having fdb/flush clear static entries as well. I do > not mind changing the code to delete only dynamic ones. Will take care of > this. > > >> When you create a static entry for lets say Port A, when a packet with the >> same MAC comes from Port B the entry will be updated with Port B. This >> should not happen for static entries. >> >> I agree. I tested this case too. It fails. Will fix it. > >> When you add a static MAC entry, the command just returns "OK". Other >> commands do not return anything on a successful addition. You should either >> follow the same behavior or be more verbose (Static FDB successfully >> added?) on your return. >> >> Sure, will make the command return error string only if it fails, > otherwise will be silent. > >> Also, it might be nice to be more verbose when you replace an existing >> static or dynamic FDB entry, i.e. especially if the physical port is >> different (mac move case). >> >> > Sure, will add more detailed output. > >> Cheers, >> >> >> Eelco >> >>
Hi Eelco, I addressed your comments and also there was a bug uncovered with unit tests. Here is the PATCH v4 <http://patchwork.ozlabs.org/project/openvswitch/patch/20210522181211.48976-1-vdasari@gmail.com/> Thanks *Vasu Dasari* On Fri, May 21, 2021 at 4:41 AM Eelco Chaudron <echaudro@redhat.com> wrote: > > > On 20 May 2021, at 19:09, Vasu Dasari wrote: > > > Thank you Eelco for testing the patch. > > > > My responses are inline: > > > Thanks, looking forward for your next revision. Would be good if you can > add a test case for the mac move and flush change. > > //Eelco > > > > *Vasu Dasari* > > > > > > On Thu, May 20, 2021 at 5:20 AM Eelco Chaudron <echaudro@redhat.com> > wrote: > > > >> > >> > >> On 14 May 2021, at 21:33, 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. ovs-appctl command looks like > >> this: > >>> > >>> 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 > >>> > >>> 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. > >>> Another check as part of mac-update process, when a packet with same > >> source mac as this > >>> entry arrives on the configured port will not modify the expires field > >>> > >>> 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); > >>> > >>> Signed-off-by: Vasu Dasari <vdasari@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 > >> > >> I did some testing and found the issues below, once fixed, I’ll do a > full > >> code review. > > > > When you do an FDB flush, it also clears the static FDB entries. I think > >> this is wrong, as all hardware vendors I know will retain the static FDB > >> entries. > >> > > > > I took the liberty of having fdb/flush clear static entries as well. I do > > not mind changing the code to delete only dynamic ones. Will take care of > > this. > > > > > >> When you create a static entry for lets say Port A, when a packet with > the > >> same MAC comes from Port B the entry will be updated with Port B. This > >> should not happen for static entries. > >> > >> I agree. I tested this case too. It fails. Will fix it. > > > >> When you add a static MAC entry, the command just returns "OK". Other > >> commands do not return anything on a successful addition. You should > either > >> follow the same behavior or be more verbose (Static FDB successfully > >> added?) on your return. > >> > >> Sure, will make the command return error string only if it fails, > > otherwise will be silent. > > > >> Also, it might be nice to be more verbose when you replace an existing > >> static or dynamic FDB entry, i.e. especially if the physical port is > >> different (mac move case). > >> > >> > > Sure, will add more detailed output. > > > >> Cheers, > >> > >> > >> Eelco > >> > >> > >
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..f7c6ef538 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,7 +405,8 @@ 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 or if it is a static entry, then just return */ + if (!mac || (mac_entry_age(ml, mac) == MAC_ENTRY_AGE_STATIC_ENTRY)) { return true; } 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..8580b39f4 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -8011,6 +8011,32 @@ 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_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..d0f030e87 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -5852,18 +5852,66 @@ 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 *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; + + 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, 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")) { + xlate_add_static_mac_entry(ofproto, in_port, mac, vlan); + unixctl_command_reply(conn, "OK"); + } else if (!strcmp(op, "del")) { + xlate_delete_static_mac_entry(ofproto, mac, vlan); + unixctl_command_reply(conn, "OK"); + } else { + unixctl_command_reply_error(conn, "software error, unknown op"); + } +} + static void ofproto_unixctl_fdb_stats_clear(struct unixctl_conn *conn, int argc, const char *argv[], void *aux OVS_UNUSED) @@ -6415,6 +6463,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 24bbd884c..fcec75999 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -6753,6 +6753,61 @@ PORTNAME portName=p2 ])]) +AT_SETUP([ofproto-dpif - static MAC programming]) +OVS_VSWITCHD_START([set bridge br0 fail-mode=standalone]) +add_of_ports br0 1 2 + +arp='eth_type(0x0806),arp(sip=192.168.0.1,tip=192.168.0.2,op=1,sha=50:54:00:00:00:05,tha=00:00:00:00:00:00)' + +dnl Trace an ARP packet arriving on p1 +OFPROTO_TRACE( + [ovs-dummy], + [in_port(1),eth(src=50:54:00:00:00:05,dst=ff:ff:ff:ff:ff:ff),$arp], + [-generate], + [2,100]) + +dnl Make sure dynamic learning happens +AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/ *[[0-9]]\{1,\}$//' | sort], + [0], [dnl + 1 0 50:54:00:00:00:05 + port VLAN MAC Age +]) + +dnl Now add convert same entry to a static entry +AT_CHECK([ovs-appctl fdb/add br0 p1 0 50:54:00:00:00:05], [0], [OK +]) + +dnl Check for the static MAC entry +AT_CHECK([ovs-appctl fdb/show br0], [0], [dnl + port VLAN MAC Age + 1 0 50:54:00:00:00:05 static +]) + +dnl Trace an ARP packet arriving on p1 +OFPROTO_TRACE( + [ovs-dummy], + [in_port(1),eth(src=50:54:00:00:00:05,dst=ff:ff:ff:ff:ff:ff),$arp], + [-generate], + [2,100]) + +dnl Check that age on static MAC entry is not altered because of above learning event +AT_CHECK([ovs-appctl fdb/show br0], [0], [dnl + port VLAN MAC Age + 1 0 50:54:00:00:00:05 static +]) + +dnl Remove static mac entry +AT_CHECK([ovs-appctl fdb/del br0 p1 0 50:54:00:00:00:05], [0], [OK +]) + +dnl Check that entry is removed +AT_CHECK([ovs-appctl fdb/show br0], [0], [dnl + port VLAN MAC Age +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([ofproto-dpif - basic truncate action]) OVS_VSWITCHD_START add_of_ports br0 1 2 3 4 5
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. ovs-appctl command looks like this: 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 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. Another check as part of mac-update process, when a packet with same source mac as this entry arrives on the configured port will not modify the expires field 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); Signed-off-by: Vasu Dasari <vdasari@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 --- 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 --- NEWS | 2 ++ lib/mac-learning.c | 38 ++++++++++++++++++++--- lib/mac-learning.h | 11 +++++++ ofproto/ofproto-dpif-xlate.c | 26 ++++++++++++++++ ofproto/ofproto-dpif-xlate.h | 5 +++ ofproto/ofproto-dpif.c | 60 +++++++++++++++++++++++++++++++++--- tests/ofproto-dpif.at | 55 +++++++++++++++++++++++++++++++++ 7 files changed, 188 insertions(+), 9 deletions(-)