Flavio Leitner <[email protected]> writes: > Hi Paolo, > > The lookup does not change cmap, but it changes the entry which can > be used by multiple threads. In that case, we would need a mutex to > modify the entry. However, in this specific case only 'expires' is > required to change, and other fields are static. Therefore, going > with atomic op makes sense to me. > > Since you're using atomic op, it would be great to include > "ovs-atomic.h", though it is indirectly included by thread > or rcu headers. > > What do you think? >
ACK. Including it explicitly it's better to me too. Will do. > fbl > > > On Thu, Oct 07, 2021 at 02:35:15PM +0200, Paolo Valerio wrote: >> Signed-off-by: Paolo Valerio <[email protected]> >> --- >> lib/tnl-neigh-cache.c | 31 ++++++++++++++++++++++--------- >> 1 file changed, 22 insertions(+), 9 deletions(-) >> >> diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c >> index 5bda4af7e..a37456e6d 100644 >> --- a/lib/tnl-neigh-cache.c >> +++ b/lib/tnl-neigh-cache.c >> @@ -44,14 +44,14 @@ >> #include "openvswitch/vlog.h" >> >> >> -/* In seconds */ >> -#define NEIGH_ENTRY_DEFAULT_IDLE_TIME (15 * 60) >> +/* In milliseconds */ >> +#define NEIGH_ENTRY_DEFAULT_IDLE_TIME (15 * 60 * 1000) >> >> struct tnl_neigh_entry { >> struct cmap_node cmap_node; >> struct in6_addr ip; >> struct eth_addr mac; >> - time_t expires; /* Expiration time. */ >> + atomic_llong expires; /* Expiration time in ms. */ >> char br_name[IFNAMSIZ]; >> }; >> >> @@ -64,6 +64,16 @@ tnl_neigh_hash(const struct in6_addr *ip) >> return hash_bytes(ip->s6_addr, 16, 0); >> } >> >> +static bool >> +tnl_neigh_expired(struct tnl_neigh_entry *neigh) >> +{ >> + long long expired; >> + >> + atomic_read_relaxed(&neigh->expires, &expired); >> + >> + return expired <= time_msec(); >> +} >> + >> static struct tnl_neigh_entry * >> tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const struct in6_addr *dst) >> { >> @@ -73,11 +83,12 @@ tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const >> struct in6_addr *dst) >> hash = tnl_neigh_hash(dst); >> CMAP_FOR_EACH_WITH_HASH (neigh, cmap_node, hash, &table) { >> if (ipv6_addr_equals(&neigh->ip, dst) && !strcmp(neigh->br_name, >> br_name)) { >> - if (neigh->expires <= time_now()) { > yy> + if (tnl_neigh_expired(neigh)) { >> return NULL; >> } >> >> - neigh->expires = time_now() + NEIGH_ENTRY_DEFAULT_IDLE_TIME; >> + atomic_store_relaxed(&neigh->expires, time_msec() + >> + NEIGH_ENTRY_DEFAULT_IDLE_TIME); >> return neigh; >> } >> } >> @@ -121,7 +132,8 @@ tnl_neigh_set__(const char name[IFNAMSIZ], const struct >> in6_addr *dst, >> struct tnl_neigh_entry *neigh = tnl_neigh_lookup__(name, dst); >> if (neigh) { >> if (eth_addr_equals(neigh->mac, mac)) { >> - neigh->expires = time_now() + NEIGH_ENTRY_DEFAULT_IDLE_TIME; >> + atomic_store_relaxed(&neigh->expires, time_msec() + >> + NEIGH_ENTRY_DEFAULT_IDLE_TIME); >> ovs_mutex_unlock(&mutex); >> return; >> } >> @@ -133,7 +145,8 @@ tnl_neigh_set__(const char name[IFNAMSIZ], const struct >> in6_addr *dst, >> >> neigh->ip = *dst; >> neigh->mac = mac; >> - neigh->expires = time_now() + NEIGH_ENTRY_DEFAULT_IDLE_TIME; >> + atomic_store_relaxed(&neigh->expires, time_msec() + >> + NEIGH_ENTRY_DEFAULT_IDLE_TIME); >> ovs_strlcpy(neigh->br_name, name, sizeof neigh->br_name); >> cmap_insert(&table, &neigh->cmap_node, tnl_neigh_hash(&neigh->ip)); >> ovs_mutex_unlock(&mutex); >> @@ -208,7 +221,7 @@ tnl_neigh_cache_run(void) >> >> ovs_mutex_lock(&mutex); >> CMAP_FOR_EACH(neigh, cmap_node, &table) { >> - if (neigh->expires <= time_now()) { >> + if (tnl_neigh_expired(neigh)) { >> tnl_neigh_delete(neigh); >> changed = true; >> } >> @@ -319,7 +332,7 @@ tnl_neigh_cache_show(struct unixctl_conn *conn, int argc >> OVS_UNUSED, >> >> ds_put_format(&ds, ETH_ADDR_FMT" %s", >> ETH_ADDR_ARGS(neigh->mac), neigh->br_name); >> - if (neigh->expires <= time_now()) { >> + if (tnl_neigh_expired(neigh)) { >> ds_put_format(&ds, " STALE"); >> } >> ds_put_char(&ds, '\n'); >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > -- > fbl _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
