| Message ID | 163587314267.3737810.12870765406848931871.stgit@fed.void |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Native tunnel: Update neigh entries in tnl termination. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On Tue, Nov 2, 2021, at 18:12, Paolo Valerio wrote: > with the command is now possible to change the ageing time of the > cache entries. > > For the existing entries the ageing time is updated only if the > current expiration is greater than the new one. In any case, the next > refresh will set it to the new value. > > This is intended mostly for debugging purpose. > > Signed-off-by: Paolo Valerio <pvalerio@redhat.com> > --- > NEWS | 3 ++ > lib/tnl-neigh-cache.c | 77 ++++++++++++++++++++++++++++++++++----- > ofproto/ofproto-tnl-unixctl.man | 9 +++++ > tests/tunnel-push-pop-ipv6.at | 30 +++++++++++++++ > tests/tunnel-push-pop.at | 30 +++++++++++++++ > 5 files changed, 140 insertions(+), 9 deletions(-) > > diff --git a/NEWS b/NEWS > index 90f4b1590..148dd5d61 100644 > --- a/NEWS > +++ b/NEWS > @@ -10,6 +10,9 @@ Post-v2.16.0 > limiting behavior. > * Add hardware offload support for matching IPv4/IPv6 frag types > (experimental). > + - Native tunnel: > + * Added new ovs-appctl tnl/neigh/ageing to read/write the neigh ageing > + time. > > > v2.16.0 - 16 Aug 2021 > diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c > index 2769e5c3d..8eacaccd8 100644 > --- a/lib/tnl-neigh-cache.c > +++ b/lib/tnl-neigh-cache.c > @@ -47,6 +47,7 @@ > > /* In milliseconds */ > #define NEIGH_ENTRY_DEFAULT_IDLE_TIME (15 * 60 * 1000) > +#define NEIGH_ENTRY_MAX_AGEING_TIME (3600 * 1000) Same remark, using _MS suffix would be nice. > > struct tnl_neigh_entry { > struct cmap_node cmap_node; > @@ -58,6 +59,7 @@ struct tnl_neigh_entry { > > static struct cmap table = CMAP_INITIALIZER; > static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER; > +static atomic_uint32_t neigh_ageing; > > static uint32_t > tnl_neigh_hash(const struct in6_addr *ip) > @@ -75,6 +77,15 @@ tnl_neigh_expired(struct tnl_neigh_entry *neigh) > return expired <= time_msec(); > } > > +static uint32_t > +tnl_neigh_get_ageing(void) > +{ > + unsigned int ageing; > + > + atomic_read_relaxed(&neigh_ageing, &ageing); > + return ageing; > +} > + > static struct tnl_neigh_entry * > tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const struct in6_addr > *dst) > { > @@ -89,7 +100,7 @@ tnl_neigh_lookup__(const char br_name[IFNAMSIZ], > const struct in6_addr *dst) > } > > atomic_store_relaxed(&neigh->expires, time_msec() + > - NEIGH_ENTRY_DEFAULT_IDLE_TIME); > + tnl_neigh_get_ageing()); > return neigh; > } > } > @@ -134,7 +145,7 @@ tnl_neigh_set__(const char name[IFNAMSIZ], const > struct in6_addr *dst, > if (neigh) { > if (eth_addr_equals(neigh->mac, mac)) { > atomic_store_relaxed(&neigh->expires, time_msec() + > - NEIGH_ENTRY_DEFAULT_IDLE_TIME); > + tnl_neigh_get_ageing()); > ovs_mutex_unlock(&mutex); > return; > } > @@ -147,7 +158,7 @@ tnl_neigh_set__(const char name[IFNAMSIZ], const > struct in6_addr *dst, > neigh->ip = *dst; > neigh->mac = mac; > atomic_store_relaxed(&neigh->expires, time_msec() + > - NEIGH_ENTRY_DEFAULT_IDLE_TIME); > + tnl_neigh_get_ageing()); > 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); > @@ -273,6 +284,43 @@ tnl_neigh_cache_flush(struct unixctl_conn *conn, > int argc OVS_UNUSED, > unixctl_command_reply(conn, "OK"); > } > > +static void > +tnl_neigh_cache_ageing(struct unixctl_conn *conn, int argc, > + const char *argv[], void *aux OVS_UNUSED) > +{ > + long long int new_exp, curr_exp; > + struct tnl_neigh_entry *neigh; > + uint32_t ageing; > + > + if (argc == 1) { > + struct ds ds = DS_EMPTY_INITIALIZER; > + ds_put_format(&ds, "%"PRIu32, tnl_neigh_get_ageing() / 1000); > + unixctl_command_reply(conn, ds_cstr(&ds)); > + ds_destroy(&ds); > + > + return; > + } > + > + if (!ovs_scan(argv[1], "%"SCNu32, &ageing) || > + !ageing || ageing > NEIGH_ENTRY_MAX_AGEING_TIME) { ageing is provided in seconds, while NEIGH_ENTRY_MAX_AGEING_TIME is in milliseconds right? > + unixctl_command_reply_error(conn, "bad ageing value"); > + return; > + } > + > + ageing *= 1000; > + atomic_store_relaxed(&neigh_ageing, ageing); Release here with acquire in tnl_neigh_get_ageing() might be needed. > + new_exp = time_msec() + ageing; > + > + CMAP_FOR_EACH (neigh, cmap_node, &table) { > + atomic_read_relaxed(&neigh->expires, &curr_exp); > + if (new_exp < curr_exp) { > + atomic_store_relaxed(&neigh->expires, new_exp); > + } Potential re-ordering issue between read and write seems more acute here. The explicit acquire / release ordering should probably be used. > + } > + > + unixctl_command_reply(conn, "OK"); > +} > + > static int > lookup_any(const char *host_name, struct in6_addr *address) > { > @@ -347,10 +395,21 @@ tnl_neigh_cache_show(struct unixctl_conn *conn, > int argc OVS_UNUSED, > void > tnl_neigh_cache_init(void) > { > - unixctl_command_register("tnl/arp/show", "", 0, 0, > tnl_neigh_cache_show, NULL); > - unixctl_command_register("tnl/arp/set", "BRIDGE IP MAC", 3, 3, > tnl_neigh_cache_add, NULL); > - unixctl_command_register("tnl/arp/flush", "", 0, 0, > tnl_neigh_cache_flush, NULL); > - unixctl_command_register("tnl/neigh/show", "", 0, 0, > tnl_neigh_cache_show, NULL); > - unixctl_command_register("tnl/neigh/set", "BRIDGE IP MAC", 3, 3, > tnl_neigh_cache_add, NULL); > - unixctl_command_register("tnl/neigh/flush", "", 0, 0, > tnl_neigh_cache_flush, NULL); > + atomic_init(&neigh_ageing, NEIGH_ENTRY_DEFAULT_IDLE_TIME); > + unixctl_command_register("tnl/arp/show", "", 0, 0, > + tnl_neigh_cache_show, NULL); > + unixctl_command_register("tnl/arp/set", "BRIDGE IP MAC", 3, 3, > + tnl_neigh_cache_add, NULL); > + unixctl_command_register("tnl/arp/flush", "", 0, 0, > + tnl_neigh_cache_flush, NULL); > + unixctl_command_register("tnl/arp/ageing", "[SECS]", 0, 1, > + tnl_neigh_cache_ageing, NULL); > + unixctl_command_register("tnl/neigh/show", "", 0, 0, > + tnl_neigh_cache_show, NULL); > + unixctl_command_register("tnl/neigh/set", "BRIDGE IP MAC", 3, 3, > + tnl_neigh_cache_add, NULL); > + unixctl_command_register("tnl/neigh/flush", "", 0, 0, > + tnl_neigh_cache_flush, NULL); > + unixctl_command_register("tnl/neigh/ageing", "[SECS]", 0, 1, > + tnl_neigh_cache_ageing, NULL); > } > diff --git a/ofproto/ofproto-tnl-unixctl.man > b/ofproto/ofproto-tnl-unixctl.man > index c70cca539..5c2ad4843 100644 > --- a/ofproto/ofproto-tnl-unixctl.man > +++ b/ofproto/ofproto-tnl-unixctl.man > @@ -27,6 +27,15 @@ to \fImac\fR. > .IP "\fBtnl/arp/flush\fR" > Flush ARP table. > . > +.IP "\fBtnl/neigh/ageing [\fIseconds\fB]\fR" > +.IP "\fBtnl/arp/ageing [\fIseconds\fB]\fR" > +Changes the ageing time. The accepted values of \fIseconds\fR are > +between 1 and 3600. The new entries will get the value as specified in > +\fIseconds\fR. For the existing entries, the ageing time is updated > +only if the current expiration is greater than \fIseconds\fR. > +.IP > +If used without arguments, it prints the current ageing value. > +. > .IP "\fBtnl/egress_port_range [num1] [num2]\fR" > Set range for UDP source port used for UDP based Tunnels. For > example VxLAN. If case of zero arguments this command prints > diff --git a/tests/tunnel-push-pop-ipv6.at > b/tests/tunnel-push-pop-ipv6.at > index 59723e63b..766a4bcf8 100644 > --- a/tests/tunnel-push-pop-ipv6.at > +++ b/tests/tunnel-push-pop-ipv6.at > @@ -255,6 +255,36 @@ AT_CHECK([cat p0.pcap.txt | grep > 93aa55aa55000086dd6000000000203aff2001cafe | un > > 3333ff000093aa55aa55000086dd6000000000203aff2001cafe000000000000000000000088ff0200000000000000000001ff00009387004d46000000002001cafe0000000000000000000000930101aa55aa550000 > ]) > > +dnl Set the ageing time to 5 seconds > +AT_CHECK([ovs-appctl tnl/neigh/ageing 5], [0], [OK > +]) > + > +dnl Read the current ageing time > +AT_CHECK([ovs-appctl tnl/neigh/ageing], [0], [5 > +]) > + > +dnl Add an entry > +AT_CHECK([ovs-appctl tnl/neigh/set br0 2001:cafe::92 > aa:bb:cc:00:00:01], [0], [OK > +]) > + > +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl > +2001:cafe::92 aa:bb:cc:00:00:01 br0 > +]) > + > +ovs-appctl time/warp 5000 > + > +dnl Check the entry has been removed > +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl > +]) > + > +dnl Restore the ageing time to 900s (default) > +AT_CHECK([ovs-appctl tnl/neigh/ageing 900], [0], [OK > +]) > + > +dnl Read the current ageing time > +AT_CHECK([ovs-appctl tnl/neigh/ageing], [0], [900 > +]) > + > dnl Check ARP Snoop > AT_CHECK([ovs-appctl netdev-dummy/receive p0 > 'in_port(1),eth(src=f8:bc:12:44:34:c8,dst=aa:55:aa:55:00:00),eth_type(0x86dd),ipv6(src=2001:cafe::92,dst=2001:cafe::88,label=0,proto=58,tclass=0,hlimit=255,frag=no),icmpv6(type=136,code=0),nd(target=2001:cafe::92,sll=00:00:00:00:00:00,tll=f8:bc:12:44:34:c8)']) > > diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at > index 12fc1ef91..385d14ab3 100644 > --- a/tests/tunnel-push-pop.at > +++ b/tests/tunnel-push-pop.at > @@ -270,6 +270,36 @@ AT_CHECK([cat p0.pcap.txt | grep 101025d | uniq], > [0], [dnl > > ffffffffffffaa55aa55000008060001080006040001aa55aa550000010102580000000000000101025d > ]) > > +dnl Set the ageing time to 5 seconds > +AT_CHECK([ovs-appctl tnl/neigh/ageing 5], [0], [OK > +]) > + > +dnl Read the current ageing time > +AT_CHECK([ovs-appctl tnl/neigh/ageing], [0], [5 > +]) > + > +dnl Add an entry > +AT_CHECK([ovs-appctl tnl/neigh/set br0 1.1.2.92 aa:bb:cc:00:00:01], > [0], [OK > +]) > + > +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl > +1.1.2.92 aa:bb:cc:00:00:01 br0 > +]) > + > +ovs-appctl time/warp 5000 > + > +dnl Check the entry has been removed > +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl > +]) > + > +dnl Restore the ageing time to 900s (default) > +AT_CHECK([ovs-appctl tnl/neigh/ageing 900], [0], [OK > +]) > + > +dnl Read the current ageing time > +AT_CHECK([ovs-appctl tnl/neigh/ageing], [0], [900 > +]) > + > dnl Check ARP Snoop > AT_CHECK([ovs-appctl netdev-dummy/receive p0 > 'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:c8,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:c8,tha=00:00:00:00:00:00)']) > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Gaëtan Rivet <grive@u256.net> writes: > On Tue, Nov 2, 2021, at 18:12, Paolo Valerio wrote: >> with the command is now possible to change the ageing time of the >> cache entries. >> >> For the existing entries the ageing time is updated only if the >> current expiration is greater than the new one. In any case, the next >> refresh will set it to the new value. >> >> This is intended mostly for debugging purpose. >> >> Signed-off-by: Paolo Valerio <pvalerio@redhat.com> >> --- >> NEWS | 3 ++ >> lib/tnl-neigh-cache.c | 77 ++++++++++++++++++++++++++++++++++----- >> ofproto/ofproto-tnl-unixctl.man | 9 +++++ >> tests/tunnel-push-pop-ipv6.at | 30 +++++++++++++++ >> tests/tunnel-push-pop.at | 30 +++++++++++++++ >> 5 files changed, 140 insertions(+), 9 deletions(-) >> >> diff --git a/NEWS b/NEWS >> index 90f4b1590..148dd5d61 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -10,6 +10,9 @@ Post-v2.16.0 >> limiting behavior. >> * Add hardware offload support for matching IPv4/IPv6 frag types >> (experimental). >> + - Native tunnel: >> + * Added new ovs-appctl tnl/neigh/ageing to read/write the neigh ageing >> + time. >> >> >> v2.16.0 - 16 Aug 2021 >> diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c >> index 2769e5c3d..8eacaccd8 100644 >> --- a/lib/tnl-neigh-cache.c >> +++ b/lib/tnl-neigh-cache.c >> @@ -47,6 +47,7 @@ >> >> /* In milliseconds */ >> #define NEIGH_ENTRY_DEFAULT_IDLE_TIME (15 * 60 * 1000) >> +#define NEIGH_ENTRY_MAX_AGEING_TIME (3600 * 1000) > > Same remark, using _MS suffix would be nice. > >> >> struct tnl_neigh_entry { >> struct cmap_node cmap_node; >> @@ -58,6 +59,7 @@ struct tnl_neigh_entry { >> >> static struct cmap table = CMAP_INITIALIZER; >> static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER; >> +static atomic_uint32_t neigh_ageing; >> >> static uint32_t >> tnl_neigh_hash(const struct in6_addr *ip) >> @@ -75,6 +77,15 @@ tnl_neigh_expired(struct tnl_neigh_entry *neigh) >> return expired <= time_msec(); >> } >> >> +static uint32_t >> +tnl_neigh_get_ageing(void) >> +{ >> + unsigned int ageing; >> + >> + atomic_read_relaxed(&neigh_ageing, &ageing); >> + return ageing; >> +} >> + >> static struct tnl_neigh_entry * >> tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const struct in6_addr >> *dst) >> { >> @@ -89,7 +100,7 @@ tnl_neigh_lookup__(const char br_name[IFNAMSIZ], >> const struct in6_addr *dst) >> } >> >> atomic_store_relaxed(&neigh->expires, time_msec() + >> - NEIGH_ENTRY_DEFAULT_IDLE_TIME); >> + tnl_neigh_get_ageing()); >> return neigh; >> } >> } >> @@ -134,7 +145,7 @@ tnl_neigh_set__(const char name[IFNAMSIZ], const >> struct in6_addr *dst, >> if (neigh) { >> if (eth_addr_equals(neigh->mac, mac)) { >> atomic_store_relaxed(&neigh->expires, time_msec() + >> - NEIGH_ENTRY_DEFAULT_IDLE_TIME); >> + tnl_neigh_get_ageing()); >> ovs_mutex_unlock(&mutex); >> return; >> } >> @@ -147,7 +158,7 @@ tnl_neigh_set__(const char name[IFNAMSIZ], const >> struct in6_addr *dst, >> neigh->ip = *dst; >> neigh->mac = mac; >> atomic_store_relaxed(&neigh->expires, time_msec() + >> - NEIGH_ENTRY_DEFAULT_IDLE_TIME); >> + tnl_neigh_get_ageing()); >> 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); >> @@ -273,6 +284,43 @@ tnl_neigh_cache_flush(struct unixctl_conn *conn, >> int argc OVS_UNUSED, >> unixctl_command_reply(conn, "OK"); >> } >> >> +static void >> +tnl_neigh_cache_ageing(struct unixctl_conn *conn, int argc, >> + const char *argv[], void *aux OVS_UNUSED) >> +{ >> + long long int new_exp, curr_exp; >> + struct tnl_neigh_entry *neigh; >> + uint32_t ageing; >> + >> + if (argc == 1) { >> + struct ds ds = DS_EMPTY_INITIALIZER; >> + ds_put_format(&ds, "%"PRIu32, tnl_neigh_get_ageing() / 1000); >> + unixctl_command_reply(conn, ds_cstr(&ds)); >> + ds_destroy(&ds); >> + >> + return; >> + } >> + >> + if (!ovs_scan(argv[1], "%"SCNu32, &ageing) || >> + !ageing || ageing > NEIGH_ENTRY_MAX_AGEING_TIME) { > > ageing is provided in seconds, while NEIGH_ENTRY_MAX_AGEING_TIME is > in milliseconds right? > that's a leftover of a move around/revert back. It should really be in seconds as it was. Thanks for spotting this. While at it I will add a range check in the test. >> + unixctl_command_reply_error(conn, "bad ageing value"); >> + return; >> + } >> + >> + ageing *= 1000; >> + atomic_store_relaxed(&neigh_ageing, ageing); > > Release here with acquire in tnl_neigh_get_ageing() might be needed. > ACK >> + new_exp = time_msec() + ageing; >> + >> + CMAP_FOR_EACH (neigh, cmap_node, &table) { >> + atomic_read_relaxed(&neigh->expires, &curr_exp); >> + if (new_exp < curr_exp) { >> + atomic_store_relaxed(&neigh->expires, new_exp); >> + } > > Potential re-ordering issue between read and write seems more acute here. > The explicit acquire / release ordering should probably be used. > Same remark as in patch 1. A store after a conditional based on the outcome of the load. If control dependency is not enough, guess we could use: atomic_read_explicit(&neigh->expires, &curr_exp, memory_order_acquire) atomic_store_explicit(&neigh->expires, new_exp, memory_order_release) >> + } >> + >> + unixctl_command_reply(conn, "OK"); >> +} >> + >> static int >> lookup_any(const char *host_name, struct in6_addr *address) >> { >> @@ -347,10 +395,21 @@ tnl_neigh_cache_show(struct unixctl_conn *conn, >> int argc OVS_UNUSED, >> void >> tnl_neigh_cache_init(void) >> { >> - unixctl_command_register("tnl/arp/show", "", 0, 0, >> tnl_neigh_cache_show, NULL); >> - unixctl_command_register("tnl/arp/set", "BRIDGE IP MAC", 3, 3, >> tnl_neigh_cache_add, NULL); >> - unixctl_command_register("tnl/arp/flush", "", 0, 0, >> tnl_neigh_cache_flush, NULL); >> - unixctl_command_register("tnl/neigh/show", "", 0, 0, >> tnl_neigh_cache_show, NULL); >> - unixctl_command_register("tnl/neigh/set", "BRIDGE IP MAC", 3, 3, >> tnl_neigh_cache_add, NULL); >> - unixctl_command_register("tnl/neigh/flush", "", 0, 0, >> tnl_neigh_cache_flush, NULL); >> + atomic_init(&neigh_ageing, NEIGH_ENTRY_DEFAULT_IDLE_TIME); >> + unixctl_command_register("tnl/arp/show", "", 0, 0, >> + tnl_neigh_cache_show, NULL); >> + unixctl_command_register("tnl/arp/set", "BRIDGE IP MAC", 3, 3, >> + tnl_neigh_cache_add, NULL); >> + unixctl_command_register("tnl/arp/flush", "", 0, 0, >> + tnl_neigh_cache_flush, NULL); >> + unixctl_command_register("tnl/arp/ageing", "[SECS]", 0, 1, >> + tnl_neigh_cache_ageing, NULL); >> + unixctl_command_register("tnl/neigh/show", "", 0, 0, >> + tnl_neigh_cache_show, NULL); >> + unixctl_command_register("tnl/neigh/set", "BRIDGE IP MAC", 3, 3, >> + tnl_neigh_cache_add, NULL); >> + unixctl_command_register("tnl/neigh/flush", "", 0, 0, >> + tnl_neigh_cache_flush, NULL); >> + unixctl_command_register("tnl/neigh/ageing", "[SECS]", 0, 1, >> + tnl_neigh_cache_ageing, NULL); >> } >> diff --git a/ofproto/ofproto-tnl-unixctl.man >> b/ofproto/ofproto-tnl-unixctl.man >> index c70cca539..5c2ad4843 100644 >> --- a/ofproto/ofproto-tnl-unixctl.man >> +++ b/ofproto/ofproto-tnl-unixctl.man >> @@ -27,6 +27,15 @@ to \fImac\fR. >> .IP "\fBtnl/arp/flush\fR" >> Flush ARP table. >> . >> +.IP "\fBtnl/neigh/ageing [\fIseconds\fB]\fR" >> +.IP "\fBtnl/arp/ageing [\fIseconds\fB]\fR" >> +Changes the ageing time. The accepted values of \fIseconds\fR are >> +between 1 and 3600. The new entries will get the value as specified in >> +\fIseconds\fR. For the existing entries, the ageing time is updated >> +only if the current expiration is greater than \fIseconds\fR. >> +.IP >> +If used without arguments, it prints the current ageing value. >> +. >> .IP "\fBtnl/egress_port_range [num1] [num2]\fR" >> Set range for UDP source port used for UDP based Tunnels. For >> example VxLAN. If case of zero arguments this command prints >> diff --git a/tests/tunnel-push-pop-ipv6.at >> b/tests/tunnel-push-pop-ipv6.at >> index 59723e63b..766a4bcf8 100644 >> --- a/tests/tunnel-push-pop-ipv6.at >> +++ b/tests/tunnel-push-pop-ipv6.at >> @@ -255,6 +255,36 @@ AT_CHECK([cat p0.pcap.txt | grep >> 93aa55aa55000086dd6000000000203aff2001cafe | un >> >> 3333ff000093aa55aa55000086dd6000000000203aff2001cafe000000000000000000000088ff0200000000000000000001ff00009387004d46000000002001cafe0000000000000000000000930101aa55aa550000 >> ]) >> >> +dnl Set the ageing time to 5 seconds >> +AT_CHECK([ovs-appctl tnl/neigh/ageing 5], [0], [OK >> +]) >> + >> +dnl Read the current ageing time >> +AT_CHECK([ovs-appctl tnl/neigh/ageing], [0], [5 >> +]) >> + >> +dnl Add an entry >> +AT_CHECK([ovs-appctl tnl/neigh/set br0 2001:cafe::92 >> aa:bb:cc:00:00:01], [0], [OK >> +]) >> + >> +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl >> +2001:cafe::92 aa:bb:cc:00:00:01 br0 >> +]) >> + >> +ovs-appctl time/warp 5000 >> + >> +dnl Check the entry has been removed >> +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl >> +]) >> + >> +dnl Restore the ageing time to 900s (default) >> +AT_CHECK([ovs-appctl tnl/neigh/ageing 900], [0], [OK >> +]) >> + >> +dnl Read the current ageing time >> +AT_CHECK([ovs-appctl tnl/neigh/ageing], [0], [900 >> +]) >> + >> dnl Check ARP Snoop >> AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> 'in_port(1),eth(src=f8:bc:12:44:34:c8,dst=aa:55:aa:55:00:00),eth_type(0x86dd),ipv6(src=2001:cafe::92,dst=2001:cafe::88,label=0,proto=58,tclass=0,hlimit=255,frag=no),icmpv6(type=136,code=0),nd(target=2001:cafe::92,sll=00:00:00:00:00:00,tll=f8:bc:12:44:34:c8)']) >> >> diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at >> index 12fc1ef91..385d14ab3 100644 >> --- a/tests/tunnel-push-pop.at >> +++ b/tests/tunnel-push-pop.at >> @@ -270,6 +270,36 @@ AT_CHECK([cat p0.pcap.txt | grep 101025d | uniq], >> [0], [dnl >> >> ffffffffffffaa55aa55000008060001080006040001aa55aa550000010102580000000000000101025d >> ]) >> >> +dnl Set the ageing time to 5 seconds >> +AT_CHECK([ovs-appctl tnl/neigh/ageing 5], [0], [OK >> +]) >> + >> +dnl Read the current ageing time >> +AT_CHECK([ovs-appctl tnl/neigh/ageing], [0], [5 >> +]) >> + >> +dnl Add an entry >> +AT_CHECK([ovs-appctl tnl/neigh/set br0 1.1.2.92 aa:bb:cc:00:00:01], >> [0], [OK >> +]) >> + >> +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl >> +1.1.2.92 aa:bb:cc:00:00:01 br0 >> +]) >> + >> +ovs-appctl time/warp 5000 >> + >> +dnl Check the entry has been removed >> +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl >> +]) >> + >> +dnl Restore the ageing time to 900s (default) >> +AT_CHECK([ovs-appctl tnl/neigh/ageing 900], [0], [OK >> +]) >> + >> +dnl Read the current ageing time >> +AT_CHECK([ovs-appctl tnl/neigh/ageing], [0], [900 >> +]) >> + >> dnl Check ARP Snoop >> AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> 'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:c8,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:c8,tha=00:00:00:00:00:00)']) >> >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > -- > Gaetan Rivet
On Mon, Nov 8, 2021, at 20:30, Paolo Valerio wrote: [...] >>> + new_exp = time_msec() + ageing; >>> + >>> + CMAP_FOR_EACH (neigh, cmap_node, &table) { >>> + atomic_read_relaxed(&neigh->expires, &curr_exp); >>> + if (new_exp < curr_exp) { >>> + atomic_store_relaxed(&neigh->expires, new_exp); >>> + } >> >> Potential re-ordering issue between read and write seems more acute here. >> The explicit acquire / release ordering should probably be used. >> > > Same remark as in patch 1. > A store after a conditional based on the outcome of the load. > If control dependency is not enough, guess we could use: > > atomic_read_explicit(&neigh->expires, &curr_exp, memory_order_acquire) > atomic_store_explicit(&neigh->expires, new_exp, memory_order_release) > Control dependency is sufficient for ARM (and x86 implements implicit acq_rel anyway). So it is probably fine as-is.
diff --git a/NEWS b/NEWS index 90f4b1590..148dd5d61 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,9 @@ Post-v2.16.0 limiting behavior. * Add hardware offload support for matching IPv4/IPv6 frag types (experimental). + - Native tunnel: + * Added new ovs-appctl tnl/neigh/ageing to read/write the neigh ageing + time. v2.16.0 - 16 Aug 2021 diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c index 2769e5c3d..8eacaccd8 100644 --- a/lib/tnl-neigh-cache.c +++ b/lib/tnl-neigh-cache.c @@ -47,6 +47,7 @@ /* In milliseconds */ #define NEIGH_ENTRY_DEFAULT_IDLE_TIME (15 * 60 * 1000) +#define NEIGH_ENTRY_MAX_AGEING_TIME (3600 * 1000) struct tnl_neigh_entry { struct cmap_node cmap_node; @@ -58,6 +59,7 @@ struct tnl_neigh_entry { static struct cmap table = CMAP_INITIALIZER; static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER; +static atomic_uint32_t neigh_ageing; static uint32_t tnl_neigh_hash(const struct in6_addr *ip) @@ -75,6 +77,15 @@ tnl_neigh_expired(struct tnl_neigh_entry *neigh) return expired <= time_msec(); } +static uint32_t +tnl_neigh_get_ageing(void) +{ + unsigned int ageing; + + atomic_read_relaxed(&neigh_ageing, &ageing); + return ageing; +} + static struct tnl_neigh_entry * tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const struct in6_addr *dst) { @@ -89,7 +100,7 @@ tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const struct in6_addr *dst) } atomic_store_relaxed(&neigh->expires, time_msec() + - NEIGH_ENTRY_DEFAULT_IDLE_TIME); + tnl_neigh_get_ageing()); return neigh; } } @@ -134,7 +145,7 @@ tnl_neigh_set__(const char name[IFNAMSIZ], const struct in6_addr *dst, if (neigh) { if (eth_addr_equals(neigh->mac, mac)) { atomic_store_relaxed(&neigh->expires, time_msec() + - NEIGH_ENTRY_DEFAULT_IDLE_TIME); + tnl_neigh_get_ageing()); ovs_mutex_unlock(&mutex); return; } @@ -147,7 +158,7 @@ tnl_neigh_set__(const char name[IFNAMSIZ], const struct in6_addr *dst, neigh->ip = *dst; neigh->mac = mac; atomic_store_relaxed(&neigh->expires, time_msec() + - NEIGH_ENTRY_DEFAULT_IDLE_TIME); + tnl_neigh_get_ageing()); 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); @@ -273,6 +284,43 @@ tnl_neigh_cache_flush(struct unixctl_conn *conn, int argc OVS_UNUSED, unixctl_command_reply(conn, "OK"); } +static void +tnl_neigh_cache_ageing(struct unixctl_conn *conn, int argc, + const char *argv[], void *aux OVS_UNUSED) +{ + long long int new_exp, curr_exp; + struct tnl_neigh_entry *neigh; + uint32_t ageing; + + if (argc == 1) { + struct ds ds = DS_EMPTY_INITIALIZER; + ds_put_format(&ds, "%"PRIu32, tnl_neigh_get_ageing() / 1000); + unixctl_command_reply(conn, ds_cstr(&ds)); + ds_destroy(&ds); + + return; + } + + if (!ovs_scan(argv[1], "%"SCNu32, &ageing) || + !ageing || ageing > NEIGH_ENTRY_MAX_AGEING_TIME) { + unixctl_command_reply_error(conn, "bad ageing value"); + return; + } + + ageing *= 1000; + atomic_store_relaxed(&neigh_ageing, ageing); + new_exp = time_msec() + ageing; + + CMAP_FOR_EACH (neigh, cmap_node, &table) { + atomic_read_relaxed(&neigh->expires, &curr_exp); + if (new_exp < curr_exp) { + atomic_store_relaxed(&neigh->expires, new_exp); + } + } + + unixctl_command_reply(conn, "OK"); +} + static int lookup_any(const char *host_name, struct in6_addr *address) { @@ -347,10 +395,21 @@ tnl_neigh_cache_show(struct unixctl_conn *conn, int argc OVS_UNUSED, void tnl_neigh_cache_init(void) { - unixctl_command_register("tnl/arp/show", "", 0, 0, tnl_neigh_cache_show, NULL); - unixctl_command_register("tnl/arp/set", "BRIDGE IP MAC", 3, 3, tnl_neigh_cache_add, NULL); - unixctl_command_register("tnl/arp/flush", "", 0, 0, tnl_neigh_cache_flush, NULL); - unixctl_command_register("tnl/neigh/show", "", 0, 0, tnl_neigh_cache_show, NULL); - unixctl_command_register("tnl/neigh/set", "BRIDGE IP MAC", 3, 3, tnl_neigh_cache_add, NULL); - unixctl_command_register("tnl/neigh/flush", "", 0, 0, tnl_neigh_cache_flush, NULL); + atomic_init(&neigh_ageing, NEIGH_ENTRY_DEFAULT_IDLE_TIME); + unixctl_command_register("tnl/arp/show", "", 0, 0, + tnl_neigh_cache_show, NULL); + unixctl_command_register("tnl/arp/set", "BRIDGE IP MAC", 3, 3, + tnl_neigh_cache_add, NULL); + unixctl_command_register("tnl/arp/flush", "", 0, 0, + tnl_neigh_cache_flush, NULL); + unixctl_command_register("tnl/arp/ageing", "[SECS]", 0, 1, + tnl_neigh_cache_ageing, NULL); + unixctl_command_register("tnl/neigh/show", "", 0, 0, + tnl_neigh_cache_show, NULL); + unixctl_command_register("tnl/neigh/set", "BRIDGE IP MAC", 3, 3, + tnl_neigh_cache_add, NULL); + unixctl_command_register("tnl/neigh/flush", "", 0, 0, + tnl_neigh_cache_flush, NULL); + unixctl_command_register("tnl/neigh/ageing", "[SECS]", 0, 1, + tnl_neigh_cache_ageing, NULL); } diff --git a/ofproto/ofproto-tnl-unixctl.man b/ofproto/ofproto-tnl-unixctl.man index c70cca539..5c2ad4843 100644 --- a/ofproto/ofproto-tnl-unixctl.man +++ b/ofproto/ofproto-tnl-unixctl.man @@ -27,6 +27,15 @@ to \fImac\fR. .IP "\fBtnl/arp/flush\fR" Flush ARP table. . +.IP "\fBtnl/neigh/ageing [\fIseconds\fB]\fR" +.IP "\fBtnl/arp/ageing [\fIseconds\fB]\fR" +Changes the ageing time. The accepted values of \fIseconds\fR are +between 1 and 3600. The new entries will get the value as specified in +\fIseconds\fR. For the existing entries, the ageing time is updated +only if the current expiration is greater than \fIseconds\fR. +.IP +If used without arguments, it prints the current ageing value. +. .IP "\fBtnl/egress_port_range [num1] [num2]\fR" Set range for UDP source port used for UDP based Tunnels. For example VxLAN. If case of zero arguments this command prints diff --git a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at index 59723e63b..766a4bcf8 100644 --- a/tests/tunnel-push-pop-ipv6.at +++ b/tests/tunnel-push-pop-ipv6.at @@ -255,6 +255,36 @@ AT_CHECK([cat p0.pcap.txt | grep 93aa55aa55000086dd6000000000203aff2001cafe | un 3333ff000093aa55aa55000086dd6000000000203aff2001cafe000000000000000000000088ff0200000000000000000001ff00009387004d46000000002001cafe0000000000000000000000930101aa55aa550000 ]) +dnl Set the ageing time to 5 seconds +AT_CHECK([ovs-appctl tnl/neigh/ageing 5], [0], [OK +]) + +dnl Read the current ageing time +AT_CHECK([ovs-appctl tnl/neigh/ageing], [0], [5 +]) + +dnl Add an entry +AT_CHECK([ovs-appctl tnl/neigh/set br0 2001:cafe::92 aa:bb:cc:00:00:01], [0], [OK +]) + +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl +2001:cafe::92 aa:bb:cc:00:00:01 br0 +]) + +ovs-appctl time/warp 5000 + +dnl Check the entry has been removed +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl +]) + +dnl Restore the ageing time to 900s (default) +AT_CHECK([ovs-appctl tnl/neigh/ageing 900], [0], [OK +]) + +dnl Read the current ageing time +AT_CHECK([ovs-appctl tnl/neigh/ageing], [0], [900 +]) + dnl Check ARP Snoop AT_CHECK([ovs-appctl netdev-dummy/receive p0 'in_port(1),eth(src=f8:bc:12:44:34:c8,dst=aa:55:aa:55:00:00),eth_type(0x86dd),ipv6(src=2001:cafe::92,dst=2001:cafe::88,label=0,proto=58,tclass=0,hlimit=255,frag=no),icmpv6(type=136,code=0),nd(target=2001:cafe::92,sll=00:00:00:00:00:00,tll=f8:bc:12:44:34:c8)']) diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at index 12fc1ef91..385d14ab3 100644 --- a/tests/tunnel-push-pop.at +++ b/tests/tunnel-push-pop.at @@ -270,6 +270,36 @@ AT_CHECK([cat p0.pcap.txt | grep 101025d | uniq], [0], [dnl ffffffffffffaa55aa55000008060001080006040001aa55aa550000010102580000000000000101025d ]) +dnl Set the ageing time to 5 seconds +AT_CHECK([ovs-appctl tnl/neigh/ageing 5], [0], [OK +]) + +dnl Read the current ageing time +AT_CHECK([ovs-appctl tnl/neigh/ageing], [0], [5 +]) + +dnl Add an entry +AT_CHECK([ovs-appctl tnl/neigh/set br0 1.1.2.92 aa:bb:cc:00:00:01], [0], [OK +]) + +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl +1.1.2.92 aa:bb:cc:00:00:01 br0 +]) + +ovs-appctl time/warp 5000 + +dnl Check the entry has been removed +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl +]) + +dnl Restore the ageing time to 900s (default) +AT_CHECK([ovs-appctl tnl/neigh/ageing 900], [0], [OK +]) + +dnl Read the current ageing time +AT_CHECK([ovs-appctl tnl/neigh/ageing], [0], [900 +]) + dnl Check ARP Snoop AT_CHECK([ovs-appctl netdev-dummy/receive p0 'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:c8,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:c8,tha=00:00:00:00:00:00)'])
with the command is now possible to change the ageing time of the cache entries. For the existing entries the ageing time is updated only if the current expiration is greater than the new one. In any case, the next refresh will set it to the new value. This is intended mostly for debugging purpose. Signed-off-by: Paolo Valerio <pvalerio@redhat.com> --- NEWS | 3 ++ lib/tnl-neigh-cache.c | 77 ++++++++++++++++++++++++++++++++++----- ofproto/ofproto-tnl-unixctl.man | 9 +++++ tests/tunnel-push-pop-ipv6.at | 30 +++++++++++++++ tests/tunnel-push-pop.at | 30 +++++++++++++++ 5 files changed, 140 insertions(+), 9 deletions(-)