Hi Ben, I have addressed code review comments and I think made better documentation at the new patch, v3.
Patch v3 <http://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/> . Thanks -Vasu *Vasu Dasari* On Mon, May 10, 2021 at 8:20 PM Vasu Dasari <[email protected]> wrote: > Yes. You are right, Ben. I can have mac_entry_age() return 0, in case of > static entry. Actually, this following code is preventing age from getting > overwritten. > > void@@ -336,6 +353,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 +366,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 != INT_MAX) {+ > e->expires = time_now() + ml->idle_time;+ } > > > I will modify the code in mac_entry_age() to return 0, that would prevent > extra processing. > > Thanks > > *Vasu Dasari* > > > On Mon, May 10, 2021 at 7:55 PM Ben Pfaff <[email protected]> wrote: > >> I do not see how that helps. mac_entry_age() returns -1 if the entry is >> static, and its caller uses mac_entry_age() like this: >> >> if (!mac || mac_entry_age(ml, mac)) { >> return true; >> } >> >> which means that -1 will cause learning to happen. >> >> On Mon, May 10, 2021 at 07:43:46PM -0400, Vasu Dasari wrote: >> > 2. In mac_entry_age(), added new code to protect the age of the static >> > entry being overwritten by a received packet. >> > >> > 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 initialized with INT_MAX */+ if (INT_MAX == e->expires) {+ >> > return -1;+ } else {+ time_t remaining = e->expires - >> > time_now();+ return ml->idle_time - remaining;+ } >> > } >> > >> > >> > Hope this answers your question. >> > >> > Thanks >> > -Vasu >> > >> > *Vasu Dasari* >> > >> > >> > On Mon, May 10, 2021 at 2:57 PM Ben Pfaff <[email protected]> wrote: >> > >> > > On Sat, May 08, 2021 at 06:18:41PM -0400, 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 CLI. >> > > > >> > > > 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 >> > > > >> > > > 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 INT_MAX. 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 convinient interfacde 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 <[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 >> > > >> > > Needs documentation. >> > > >> > > I would expect a static entry to be one that couldn't be changed by >> > > receiving a packet that causes MAC learning, but I don't see anything >> > > that protects from that in here. >> > > >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
