| Message ID | 163654120292.1486166.3822003811574144112.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 Wed, Nov 10, 2021 at 11:46:42AM +0100, Paolo Valerio wrote: > with the command is now possible to change the aging time of the > cache entries. > > For the existing entries the aging 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> > --- > v2: > - fixed NEIGH_ENTRY_MAX_AGEING_TIME (turned to seconds) correcting a > leftover. > - turned relaxed atomics to acq/rel. > - added range checks to tunnel-push-pop.at. It was useless to > duplicate the test for both ipv6 and ipv4, so only the latter > includes it. > - slightly modified the NEWS entry. > --- > NEWS | 2 + > lib/tnl-neigh-cache.c | 79 +++++++++++++++++++++++++++++++++++---- > ofproto/ofproto-tnl-unixctl.man | 9 ++++ > tests/tunnel-push-pop-ipv6.at | 30 +++++++++++++++ > tests/tunnel-push-pop.at | 47 +++++++++++++++++++++++ > 5 files changed, 158 insertions(+), 9 deletions(-) > > diff --git a/NEWS b/NEWS > index 434ee570f..1aa233a0d 100644 > --- a/NEWS > +++ b/NEWS > @@ -16,6 +16,8 @@ Post-v2.16.0 > - ovs-dpctl and 'ovs-appctl dpctl/': > * New commands 'cache-get-size' and 'cache-set-size' that allows to > get or configure linux kernel datapath cache sizes. > + - ovs-appctl: > + * New command tnl/neigh/aging to read/write the neigh aging time. > > > v2.16.0 - 16 Aug 2021 > diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c > index 1e6cc31db..a4d56e4cc 100644 > --- a/lib/tnl-neigh-cache.c > +++ b/lib/tnl-neigh-cache.c > @@ -46,6 +46,7 @@ > > > #define NEIGH_ENTRY_DEFAULT_IDLE_TIME_MS (15 * 60 * 1000) > +#define NEIGH_ENTRY_MAX_AGING_TIME 3600 Shouldn't we include the unit suffix here too? fbl > > struct tnl_neigh_entry { > struct cmap_node cmap_node; > @@ -57,6 +58,7 @@ struct tnl_neigh_entry { > > static struct cmap table = CMAP_INITIALIZER; > static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER; > +static atomic_uint32_t neigh_aging; > > static uint32_t > tnl_neigh_hash(const struct in6_addr *ip) > @@ -74,6 +76,15 @@ tnl_neigh_expired(struct tnl_neigh_entry *neigh) > return expires <= time_msec(); > } > > +static uint32_t > +tnl_neigh_get_aging(void) > +{ > + unsigned int aging; > + > + atomic_read_explicit(&neigh_aging, &aging, memory_order_acquire); > + return aging; > +} > + > static struct tnl_neigh_entry * > tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const struct in6_addr *dst) > { > @@ -88,7 +99,7 @@ tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const struct in6_addr *dst) > } > > atomic_store_explicit(&neigh->expires, time_msec() + > - NEIGH_ENTRY_DEFAULT_IDLE_TIME_MS, > + tnl_neigh_get_aging(), > memory_order_release); > 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_MS); > + tnl_neigh_get_aging()); > 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_MS); > + tnl_neigh_get_aging()); > 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,45 @@ tnl_neigh_cache_flush(struct unixctl_conn *conn, int argc OVS_UNUSED, > unixctl_command_reply(conn, "OK"); > } > > +static void > +tnl_neigh_cache_aging(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 aging; > + > + if (argc == 1) { > + struct ds ds = DS_EMPTY_INITIALIZER; > + ds_put_format(&ds, "%"PRIu32, tnl_neigh_get_aging() / 1000); > + unixctl_command_reply(conn, ds_cstr(&ds)); > + ds_destroy(&ds); > + > + return; > + } > + > + if (!ovs_scan(argv[1], "%"SCNu32, &aging) || > + !aging || aging > NEIGH_ENTRY_MAX_AGING_TIME) { > + unixctl_command_reply_error(conn, "bad aging value"); > + return; > + } > + > + aging *= 1000; > + atomic_store_explicit(&neigh_aging, aging, memory_order_release); > + new_exp = time_msec() + aging; > + > + CMAP_FOR_EACH (neigh, cmap_node, &table) { > + atomic_read_explicit(&neigh->expires, &curr_exp, > + memory_order_acquire); > + if (new_exp < curr_exp) { > + 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 +397,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_aging, NEIGH_ENTRY_DEFAULT_IDLE_TIME_MS); > + 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/aging", "[SECS]", 0, 1, > + tnl_neigh_cache_aging, 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/aging", "[SECS]", 0, 1, > + tnl_neigh_cache_aging, NULL); > } > diff --git a/ofproto/ofproto-tnl-unixctl.man b/ofproto/ofproto-tnl-unixctl.man > index c70cca539..13a465119 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/aging [\fIseconds\fB]\fR" > +.IP "\fBtnl/arp/aging [\fIseconds\fB]\fR" > +Changes the aging 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 aging time is updated > +only if the current expiration is greater than \fIseconds\fR. > +.IP > +If used without arguments, it prints the current aging 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..327c0e61e 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 aging time to 5 seconds > +AT_CHECK([ovs-appctl tnl/neigh/aging 5], [0], [OK > +]) > + > +dnl Read the current aging time > +AT_CHECK([ovs-appctl tnl/neigh/aging], [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 aging time to 900s (default) > +AT_CHECK([ovs-appctl tnl/neigh/aging 900], [0], [OK > +]) > + > +dnl Read the current aging time > +AT_CHECK([ovs-appctl tnl/neigh/aging], [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 636465397..1f6249b20 100644 > --- a/tests/tunnel-push-pop.at > +++ b/tests/tunnel-push-pop.at > @@ -270,6 +270,53 @@ AT_CHECK([cat p0.pcap.txt | grep 101025d | uniq], [0], [dnl > ffffffffffffaa55aa55000008060001080006040001aa55aa550000010102580000000000000101025d > ]) > > +dnl Check input range > +AT_CHECK([ovs-appctl tnl/neigh/aging 0], [2], [], [dnl > +bad aging value > +ovs-appctl: ovs-vswitchd: server returned an error > +]) > + > +AT_CHECK([ovs-appctl tnl/neigh/aging 3601], [2], [], [dnl > +bad aging value > +ovs-appctl: ovs-vswitchd: server returned an error > +]) > + > +AT_CHECK([ovs-appctl tnl/neigh/aging 1], [0], [OK > +]) > + > +AT_CHECK([ovs-appctl tnl/neigh/aging 3600], [0], [OK > +]) > + > +dnl Set the aging time to 5 seconds > +AT_CHECK([ovs-appctl tnl/neigh/aging 5], [0], [OK > +]) > + > +dnl Read the current aging time > +AT_CHECK([ovs-appctl tnl/neigh/aging], [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 aging time to 900s (default) > +AT_CHECK([ovs-appctl tnl/neigh/aging 900], [0], [OK > +]) > + > +dnl Read the current aging time > +AT_CHECK([ovs-appctl tnl/neigh/aging], [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
Flavio Leitner <fbl@sysclose.org> writes: > On Wed, Nov 10, 2021 at 11:46:42AM +0100, Paolo Valerio wrote: >> with the command is now possible to change the aging time of the >> cache entries. >> >> For the existing entries the aging 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> >> --- >> v2: >> - fixed NEIGH_ENTRY_MAX_AGEING_TIME (turned to seconds) correcting a >> leftover. >> - turned relaxed atomics to acq/rel. >> - added range checks to tunnel-push-pop.at. It was useless to >> duplicate the test for both ipv6 and ipv4, so only the latter >> includes it. >> - slightly modified the NEWS entry. >> --- >> NEWS | 2 + >> lib/tnl-neigh-cache.c | 79 +++++++++++++++++++++++++++++++++++---- >> ofproto/ofproto-tnl-unixctl.man | 9 ++++ >> tests/tunnel-push-pop-ipv6.at | 30 +++++++++++++++ >> tests/tunnel-push-pop.at | 47 +++++++++++++++++++++++ >> 5 files changed, 158 insertions(+), 9 deletions(-) >> >> diff --git a/NEWS b/NEWS >> index 434ee570f..1aa233a0d 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -16,6 +16,8 @@ Post-v2.16.0 >> - ovs-dpctl and 'ovs-appctl dpctl/': >> * New commands 'cache-get-size' and 'cache-set-size' that allows to >> get or configure linux kernel datapath cache sizes. >> + - ovs-appctl: >> + * New command tnl/neigh/aging to read/write the neigh aging time. >> >> >> v2.16.0 - 16 Aug 2021 >> diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c >> index 1e6cc31db..a4d56e4cc 100644 >> --- a/lib/tnl-neigh-cache.c >> +++ b/lib/tnl-neigh-cache.c >> @@ -46,6 +46,7 @@ >> >> >> #define NEIGH_ENTRY_DEFAULT_IDLE_TIME_MS (15 * 60 * 1000) >> +#define NEIGH_ENTRY_MAX_AGING_TIME 3600 > > Shouldn't we include the unit suffix here too? > We could. For consistency, I think we should add "_S". Is it ok, or you prefer something else like _SEC? > >> >> struct tnl_neigh_entry { >> struct cmap_node cmap_node; >> @@ -57,6 +58,7 @@ struct tnl_neigh_entry { >> >> static struct cmap table = CMAP_INITIALIZER; >> static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER; >> +static atomic_uint32_t neigh_aging; >> >> static uint32_t >> tnl_neigh_hash(const struct in6_addr *ip) >> @@ -74,6 +76,15 @@ tnl_neigh_expired(struct tnl_neigh_entry *neigh) >> return expires <= time_msec(); >> } >> >> +static uint32_t >> +tnl_neigh_get_aging(void) >> +{ >> + unsigned int aging; >> + >> + atomic_read_explicit(&neigh_aging, &aging, memory_order_acquire); >> + return aging; >> +} >> + >> static struct tnl_neigh_entry * >> tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const struct in6_addr *dst) >> { >> @@ -88,7 +99,7 @@ tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const struct in6_addr *dst) >> } >> >> atomic_store_explicit(&neigh->expires, time_msec() + >> - NEIGH_ENTRY_DEFAULT_IDLE_TIME_MS, >> + tnl_neigh_get_aging(), >> memory_order_release); >> 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_MS); >> + tnl_neigh_get_aging()); >> 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_MS); >> + tnl_neigh_get_aging()); >> 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,45 @@ tnl_neigh_cache_flush(struct unixctl_conn *conn, int argc OVS_UNUSED, >> unixctl_command_reply(conn, "OK"); >> } >> >> +static void >> +tnl_neigh_cache_aging(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 aging; >> + >> + if (argc == 1) { >> + struct ds ds = DS_EMPTY_INITIALIZER; >> + ds_put_format(&ds, "%"PRIu32, tnl_neigh_get_aging() / 1000); >> + unixctl_command_reply(conn, ds_cstr(&ds)); >> + ds_destroy(&ds); >> + >> + return; >> + } >> + >> + if (!ovs_scan(argv[1], "%"SCNu32, &aging) || >> + !aging || aging > NEIGH_ENTRY_MAX_AGING_TIME) { >> + unixctl_command_reply_error(conn, "bad aging value"); >> + return; >> + } >> + >> + aging *= 1000; >> + atomic_store_explicit(&neigh_aging, aging, memory_order_release); >> + new_exp = time_msec() + aging; >> + >> + CMAP_FOR_EACH (neigh, cmap_node, &table) { >> + atomic_read_explicit(&neigh->expires, &curr_exp, >> + memory_order_acquire); >> + if (new_exp < curr_exp) { >> + 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 +397,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_aging, NEIGH_ENTRY_DEFAULT_IDLE_TIME_MS); >> + 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/aging", "[SECS]", 0, 1, >> + tnl_neigh_cache_aging, 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/aging", "[SECS]", 0, 1, >> + tnl_neigh_cache_aging, NULL); >> } >> diff --git a/ofproto/ofproto-tnl-unixctl.man b/ofproto/ofproto-tnl-unixctl.man >> index c70cca539..13a465119 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/aging [\fIseconds\fB]\fR" >> +.IP "\fBtnl/arp/aging [\fIseconds\fB]\fR" >> +Changes the aging 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 aging time is updated >> +only if the current expiration is greater than \fIseconds\fR. >> +.IP >> +If used without arguments, it prints the current aging 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..327c0e61e 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 aging time to 5 seconds >> +AT_CHECK([ovs-appctl tnl/neigh/aging 5], [0], [OK >> +]) >> + >> +dnl Read the current aging time >> +AT_CHECK([ovs-appctl tnl/neigh/aging], [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 aging time to 900s (default) >> +AT_CHECK([ovs-appctl tnl/neigh/aging 900], [0], [OK >> +]) >> + >> +dnl Read the current aging time >> +AT_CHECK([ovs-appctl tnl/neigh/aging], [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 636465397..1f6249b20 100644 >> --- a/tests/tunnel-push-pop.at >> +++ b/tests/tunnel-push-pop.at >> @@ -270,6 +270,53 @@ AT_CHECK([cat p0.pcap.txt | grep 101025d | uniq], [0], [dnl >> ffffffffffffaa55aa55000008060001080006040001aa55aa550000010102580000000000000101025d >> ]) >> >> +dnl Check input range >> +AT_CHECK([ovs-appctl tnl/neigh/aging 0], [2], [], [dnl >> +bad aging value >> +ovs-appctl: ovs-vswitchd: server returned an error >> +]) >> + >> +AT_CHECK([ovs-appctl tnl/neigh/aging 3601], [2], [], [dnl >> +bad aging value >> +ovs-appctl: ovs-vswitchd: server returned an error >> +]) >> + >> +AT_CHECK([ovs-appctl tnl/neigh/aging 1], [0], [OK >> +]) >> + >> +AT_CHECK([ovs-appctl tnl/neigh/aging 3600], [0], [OK >> +]) >> + >> +dnl Set the aging time to 5 seconds >> +AT_CHECK([ovs-appctl tnl/neigh/aging 5], [0], [OK >> +]) >> + >> +dnl Read the current aging time >> +AT_CHECK([ovs-appctl tnl/neigh/aging], [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 aging time to 900s (default) >> +AT_CHECK([ovs-appctl tnl/neigh/aging 900], [0], [OK >> +]) >> + >> +dnl Read the current aging time >> +AT_CHECK([ovs-appctl tnl/neigh/aging], [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 > > -- > fbl
On Thu, Nov 25, 2021 at 05:34:27PM +0100, Paolo Valerio wrote: > Flavio Leitner <fbl@sysclose.org> writes: > > > On Wed, Nov 10, 2021 at 11:46:42AM +0100, Paolo Valerio wrote: > >> with the command is now possible to change the aging time of the > >> cache entries. > >> > >> For the existing entries the aging 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> > >> --- > >> v2: > >> - fixed NEIGH_ENTRY_MAX_AGEING_TIME (turned to seconds) correcting a > >> leftover. > >> - turned relaxed atomics to acq/rel. > >> - added range checks to tunnel-push-pop.at. It was useless to > >> duplicate the test for both ipv6 and ipv4, so only the latter > >> includes it. > >> - slightly modified the NEWS entry. > >> --- > >> NEWS | 2 + > >> lib/tnl-neigh-cache.c | 79 +++++++++++++++++++++++++++++++++++---- > >> ofproto/ofproto-tnl-unixctl.man | 9 ++++ > >> tests/tunnel-push-pop-ipv6.at | 30 +++++++++++++++ > >> tests/tunnel-push-pop.at | 47 +++++++++++++++++++++++ > >> 5 files changed, 158 insertions(+), 9 deletions(-) > >> > >> diff --git a/NEWS b/NEWS > >> index 434ee570f..1aa233a0d 100644 > >> --- a/NEWS > >> +++ b/NEWS > >> @@ -16,6 +16,8 @@ Post-v2.16.0 > >> - ovs-dpctl and 'ovs-appctl dpctl/': > >> * New commands 'cache-get-size' and 'cache-set-size' that allows to > >> get or configure linux kernel datapath cache sizes. > >> + - ovs-appctl: > >> + * New command tnl/neigh/aging to read/write the neigh aging time. > >> > >> > >> v2.16.0 - 16 Aug 2021 > >> diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c > >> index 1e6cc31db..a4d56e4cc 100644 > >> --- a/lib/tnl-neigh-cache.c > >> +++ b/lib/tnl-neigh-cache.c > >> @@ -46,6 +46,7 @@ > >> > >> > >> #define NEIGH_ENTRY_DEFAULT_IDLE_TIME_MS (15 * 60 * 1000) > >> +#define NEIGH_ENTRY_MAX_AGING_TIME 3600 > > > > Shouldn't we include the unit suffix here too? > > > > We could. For consistency, I think we should add "_S". Yup, that's fine by me. > Is it ok, or you prefer something else like _SEC? Well, as you pointed out, the consistency matters, so in that case I think _MS should have been _MSEC as well. fbl
diff --git a/NEWS b/NEWS index 434ee570f..1aa233a0d 100644 --- a/NEWS +++ b/NEWS @@ -16,6 +16,8 @@ Post-v2.16.0 - ovs-dpctl and 'ovs-appctl dpctl/': * New commands 'cache-get-size' and 'cache-set-size' that allows to get or configure linux kernel datapath cache sizes. + - ovs-appctl: + * New command tnl/neigh/aging to read/write the neigh aging time. v2.16.0 - 16 Aug 2021 diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c index 1e6cc31db..a4d56e4cc 100644 --- a/lib/tnl-neigh-cache.c +++ b/lib/tnl-neigh-cache.c @@ -46,6 +46,7 @@ #define NEIGH_ENTRY_DEFAULT_IDLE_TIME_MS (15 * 60 * 1000) +#define NEIGH_ENTRY_MAX_AGING_TIME 3600 struct tnl_neigh_entry { struct cmap_node cmap_node; @@ -57,6 +58,7 @@ struct tnl_neigh_entry { static struct cmap table = CMAP_INITIALIZER; static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER; +static atomic_uint32_t neigh_aging; static uint32_t tnl_neigh_hash(const struct in6_addr *ip) @@ -74,6 +76,15 @@ tnl_neigh_expired(struct tnl_neigh_entry *neigh) return expires <= time_msec(); } +static uint32_t +tnl_neigh_get_aging(void) +{ + unsigned int aging; + + atomic_read_explicit(&neigh_aging, &aging, memory_order_acquire); + return aging; +} + static struct tnl_neigh_entry * tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const struct in6_addr *dst) { @@ -88,7 +99,7 @@ tnl_neigh_lookup__(const char br_name[IFNAMSIZ], const struct in6_addr *dst) } atomic_store_explicit(&neigh->expires, time_msec() + - NEIGH_ENTRY_DEFAULT_IDLE_TIME_MS, + tnl_neigh_get_aging(), memory_order_release); 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_MS); + tnl_neigh_get_aging()); 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_MS); + tnl_neigh_get_aging()); 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,45 @@ tnl_neigh_cache_flush(struct unixctl_conn *conn, int argc OVS_UNUSED, unixctl_command_reply(conn, "OK"); } +static void +tnl_neigh_cache_aging(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 aging; + + if (argc == 1) { + struct ds ds = DS_EMPTY_INITIALIZER; + ds_put_format(&ds, "%"PRIu32, tnl_neigh_get_aging() / 1000); + unixctl_command_reply(conn, ds_cstr(&ds)); + ds_destroy(&ds); + + return; + } + + if (!ovs_scan(argv[1], "%"SCNu32, &aging) || + !aging || aging > NEIGH_ENTRY_MAX_AGING_TIME) { + unixctl_command_reply_error(conn, "bad aging value"); + return; + } + + aging *= 1000; + atomic_store_explicit(&neigh_aging, aging, memory_order_release); + new_exp = time_msec() + aging; + + CMAP_FOR_EACH (neigh, cmap_node, &table) { + atomic_read_explicit(&neigh->expires, &curr_exp, + memory_order_acquire); + if (new_exp < curr_exp) { + 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 +397,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_aging, NEIGH_ENTRY_DEFAULT_IDLE_TIME_MS); + 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/aging", "[SECS]", 0, 1, + tnl_neigh_cache_aging, 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/aging", "[SECS]", 0, 1, + tnl_neigh_cache_aging, NULL); } diff --git a/ofproto/ofproto-tnl-unixctl.man b/ofproto/ofproto-tnl-unixctl.man index c70cca539..13a465119 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/aging [\fIseconds\fB]\fR" +.IP "\fBtnl/arp/aging [\fIseconds\fB]\fR" +Changes the aging 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 aging time is updated +only if the current expiration is greater than \fIseconds\fR. +.IP +If used without arguments, it prints the current aging 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..327c0e61e 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 aging time to 5 seconds +AT_CHECK([ovs-appctl tnl/neigh/aging 5], [0], [OK +]) + +dnl Read the current aging time +AT_CHECK([ovs-appctl tnl/neigh/aging], [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 aging time to 900s (default) +AT_CHECK([ovs-appctl tnl/neigh/aging 900], [0], [OK +]) + +dnl Read the current aging time +AT_CHECK([ovs-appctl tnl/neigh/aging], [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 636465397..1f6249b20 100644 --- a/tests/tunnel-push-pop.at +++ b/tests/tunnel-push-pop.at @@ -270,6 +270,53 @@ AT_CHECK([cat p0.pcap.txt | grep 101025d | uniq], [0], [dnl ffffffffffffaa55aa55000008060001080006040001aa55aa550000010102580000000000000101025d ]) +dnl Check input range +AT_CHECK([ovs-appctl tnl/neigh/aging 0], [2], [], [dnl +bad aging value +ovs-appctl: ovs-vswitchd: server returned an error +]) + +AT_CHECK([ovs-appctl tnl/neigh/aging 3601], [2], [], [dnl +bad aging value +ovs-appctl: ovs-vswitchd: server returned an error +]) + +AT_CHECK([ovs-appctl tnl/neigh/aging 1], [0], [OK +]) + +AT_CHECK([ovs-appctl tnl/neigh/aging 3600], [0], [OK +]) + +dnl Set the aging time to 5 seconds +AT_CHECK([ovs-appctl tnl/neigh/aging 5], [0], [OK +]) + +dnl Read the current aging time +AT_CHECK([ovs-appctl tnl/neigh/aging], [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 aging time to 900s (default) +AT_CHECK([ovs-appctl tnl/neigh/aging 900], [0], [OK +]) + +dnl Read the current aging time +AT_CHECK([ovs-appctl tnl/neigh/aging], [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 aging time of the cache entries. For the existing entries the aging 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> --- v2: - fixed NEIGH_ENTRY_MAX_AGEING_TIME (turned to seconds) correcting a leftover. - turned relaxed atomics to acq/rel. - added range checks to tunnel-push-pop.at. It was useless to duplicate the test for both ipv6 and ipv4, so only the latter includes it. - slightly modified the NEWS entry. --- NEWS | 2 + lib/tnl-neigh-cache.c | 79 +++++++++++++++++++++++++++++++++++---- ofproto/ofproto-tnl-unixctl.man | 9 ++++ tests/tunnel-push-pop-ipv6.at | 30 +++++++++++++++ tests/tunnel-push-pop.at | 47 +++++++++++++++++++++++ 5 files changed, 158 insertions(+), 9 deletions(-)