Packets for a tunnel with no outbound SA were only counted in 'netdev_push_header_drops', which all tunnel types share, and 'esp/show' looked healthy, so the cause was hard to see.
- Count these packets in a new 'esp_tx_no_sa' coverage counter. - 'esp/show' now says when an IKE-keyed tunnel has no outbound SA. - 'esp/show' without a port also lists SAs whose interface ID no tunnel has, which were invisible. - 'esp/sa-flush' now logs, like 'esp/sa-del'. The tunnel-push-pop test checks all of these. Assisted-by: claude-opus-5-5, Claude Code Signed-off-by: Thilak Raj Surendra Babu <[email protected]> --- lib/esp-ctl.c | 49 +++++++++++++++++++++++++++++++++ lib/esp-ctl.h | 3 ++ lib/netdev-native-tnl.c | 2 ++ lib/netdev-vport.c | 16 +++++++++-- ofproto/ofproto-tnl-unixctl.man | 3 ++ tests/tunnel-push-pop.at | 18 ++++++++++++ 6 files changed, 89 insertions(+), 2 deletions(-) diff --git a/lib/esp-ctl.c b/lib/esp-ctl.c index 7c2fce543..a30c6a70b 100644 --- a/lib/esp-ctl.c +++ b/lib/esp-ctl.c @@ -492,11 +492,13 @@ static void esp_ctl_sa_flush(struct unixctl_conn *conn, int argc OVS_UNUSED, const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED) { + size_t n = hmap_count(&ike_sas); struct ike_sa *ike; HMAP_FOR_EACH_SAFE (ike, node, &ike_sas) { ike_sa_delete(ike); } + VLOG_INFO("flushed %"PRIuSIZE" SAs", n); unixctl_command_reply(conn, NULL); } @@ -521,15 +523,21 @@ esp_ctl_format(struct ds *ds, uint32_t if_id) { struct ike_sa **sas = xmalloc(hmap_count(&ike_sas) * sizeof *sas); struct ike_sa *ike; + bool has_outbound = false; size_t n = 0; HMAP_FOR_EACH (ike, node, &ike_sas) { if (ike->if_id == if_id) { sas[n++] = ike; + has_outbound = has_outbound || !ike->inbound; } } qsort(sas, n, sizeof *sas, compare_ike_sas); + if (!has_outbound) { + ds_put_cstr(ds, " no outbound SA\n"); + } + for (size_t i = 0; i < n; i++) { const struct esp_sa_params *p = esp_sa_get_params(sas[i]->sa); bool active = esp_sad_lookup(p->sad_id, &p->dst) == sas[i]->sa; @@ -541,6 +549,47 @@ esp_ctl_format(struct ds *ds, uint32_t if_id) free(sas); } +static int +compare_u32(const void *a_, const void *b_) +{ + uint32_t a = *(const uint32_t *) a_; + uint32_t b = *(const uint32_t *) b_; + + return a < b ? -1 : a > b; +} + +/* Appends to 'ds' the SAs whose interface ID is not among the 'n_known' in + * 'known', which have no tunnel, so their packets cannot flow. */ +void +esp_ctl_format_unbound(struct ds *ds, const uint32_t known[], size_t n_known) +{ + uint32_t *ids = xmalloc(hmap_count(&ike_sas) * sizeof *ids); + struct ike_sa *ike; + size_t n = 0; + + HMAP_FOR_EACH (ike, node, &ike_sas) { + bool found = false; + + for (size_t i = 0; i < n_known; i++) { + found = found || known[i] == ike->if_id; + } + for (size_t i = 0; i < n; i++) { + found = found || ids[i] == ike->if_id; + } + if (!found) { + ids[n++] = ike->if_id; + } + } + qsort(ids, n, sizeof *ids, compare_u32); + + for (size_t i = 0; i < n; i++) { + ds_put_format(ds, "if_id %"PRIu32": no tunnel with this interface " + "ID\n", ids[i]); + esp_ctl_format(ds, ids[i]); + } + free(ids); +} + void esp_ctl_init(void) { diff --git a/lib/esp-ctl.h b/lib/esp-ctl.h index ff0afb9a7..25e7de1b6 100644 --- a/lib/esp-ctl.h +++ b/lib/esp-ctl.h @@ -21,11 +21,14 @@ * "kernel-ovs" plugin, to install the security associations of 'esp' * tunnels with "esp_keying=ike" through unixctl commands. */ +#include <stddef.h> #include <stdint.h> struct ds; void esp_ctl_init(void); void esp_ctl_format(struct ds *, uint32_t if_id); +void esp_ctl_format_unbound(struct ds *, const uint32_t known[], + size_t n_known); #endif /* esp-ctl.h */ diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c index 756438823..eced915ee 100644 --- a/lib/netdev-native-tnl.c +++ b/lib/netdev-native-tnl.c @@ -58,6 +58,7 @@ COVERAGE_DEFINE(native_tnl_l4csum_checked); COVERAGE_DEFINE(native_tnl_l4csum_err); COVERAGE_DEFINE(native_tnl_esp_natt_keepalive); COVERAGE_DEFINE(native_tnl_esp_natt_non_esp); +COVERAGE_DEFINE(esp_tx_no_sa); #define VXLAN_HLEN (sizeof(struct udp_header) + \ sizeof(struct vxlanhdr)) @@ -1220,6 +1221,7 @@ netdev_esp_push_header(const struct netdev *netdev, sa = esp_sad_lookup(get_16aligned_be32(&esp->spi), &dst); if (OVS_UNLIKELY(!sa)) { + COVERAGE_INC(esp_tx_no_sa); return ENOENT; } diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 9f8183929..9bff79455 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -985,22 +985,29 @@ netdev_vport_esp_show(struct unixctl_conn *conn, int argc, struct ds ds = DS_EMPTY_INITIALIZER; struct netdev **vports; size_t n_vports; + uint32_t *if_ids; + size_t n_if_ids = 0; bool found = false; vports = netdev_get_vports(&n_vports); + if_ids = xmalloc(n_vports * sizeof *if_ids); qsort(vports, n_vports, sizeof *vports, compare_netdev_names); for (size_t i = 0; i < n_vports; i++) { struct netdev_vport *dev = netdev_vport_cast(vports[i]); const char *name = netdev_get_name(vports[i]); - if (strcmp(netdev_get_type(vports[i]), "esp") - || (argc > 1 && strcmp(argv[1], name))) { + if (strcmp(netdev_get_type(vports[i]), "esp")) { continue; } ovs_mutex_lock(&dev->mutex); const struct netdev_tunnel_config *tnl_cfg = vport_tunnel_config(dev); if (tnl_cfg->esp_ike) { + if_ids[n_if_ids++] = tnl_cfg->esp_if_id; + } + if (argc > 1 && strcmp(argv[1], name)) { + /* Not the one asked for. */ + } else if (tnl_cfg->esp_ike) { ds_put_format(&ds, "%s: remote ", name); ipv6_format_mapped(&tnl_cfg->ipv6_dst, &ds); ds_put_format(&ds, ", keyed by IKE, if_id %"PRIu32"\n", @@ -1022,6 +1029,11 @@ netdev_vport_esp_show(struct unixctl_conn *conn, int argc, } free(vports); + if (argc == 1) { + esp_ctl_format_unbound(&ds, if_ids, n_if_ids); + } + free(if_ids); + if (argc > 1 && !found) { unixctl_command_reply_error(conn, "no such esp tunnel"); } else { diff --git a/ofproto/ofproto-tnl-unixctl.man b/ofproto/ofproto-tnl-unixctl.man index 22f677d34..bf84f1aba 100644 --- a/ofproto/ofproto-tnl-unixctl.man +++ b/ofproto/ofproto-tnl-unixctl.man @@ -85,6 +85,9 @@ the number of packets and bytes sent or received through each. For inbound security associations, it also shows the number of packets dropped as replays, because they failed authentication, or because they were malformed. The byte counts are of the inner packets. +For \fBesp_keying=ike\fR tunnels, it also says if a tunnel has no outbound +security association. Without \fIport\fR, it also lists security +associations whose interface ID no tunnel has. . .IP "\fBesp/spi-alloc\fR" .IQ "\fBesp/sa-add dir=\fR{\fBin\fR|\fBout\fR} \fBspi=\fIspi\fR \fBsrc=\fIip\fR \fBdst=\fIip\fR \fBif_id=\fIid\fR \fBkey=\fIkey\fR [\fBesn=yes\fR] [\fBreplay=\fIn\fR] [\fBencap=yes\fR]" diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at index 0568cc465..14f95db7c 100644 --- a/tests/tunnel-push-pop.at +++ b/tests/tunnel-push-pop.at @@ -2088,7 +2088,25 @@ AT_CHECK([ovs-appctl esp/sa-add dir=out spi=0x6000 src=1.1.2.88 dnl AT_CHECK([ovs-appctl esp/sa-flush]) AT_CHECK([ovs-appctl esp/show t1], [0], [dnl t1: remote 1.1.2.92, keyed by IKE, if_id 7 + no outbound SA ]) +AT_CHECK([grep -c 'flushed' ovs-vswitchd.log], [0], [1 +]) + +dnl Packets with no outbound SA are counted. +n=$(ovs-appctl coverage/read-counter esp_tx_no_sa) +AT_CHECK([ovs-appctl netdev-dummy/receive p1 'INNER_FLOW']) +OVS_WAIT_UNTIL([test $(ovs-appctl coverage/read-counter esp_tx_no_sa) -gt $n]) + +dnl An SA with no tunnel for its interface ID is still listed. +AT_CHECK([ovs-appctl esp/sa-add dir=out spi=0x7000 src=1.1.2.88 dnl + dst=1.1.2.93 if_id=8 key=KEY_OUT]) +AT_CHECK([ovs-appctl esp/show | grep -E '^[[a-z]]|spi'], [0], [dnl +t1: remote 1.1.2.92, keyed by IKE, if_id 7 +if_id 8: no tunnel with this interface ID + outbound: spi 0x00007000, aes128-gcm16, esn off +]) +AT_CHECK([ovs-appctl esp/sa-del dir=out spi=0x7000 dst=1.1.2.93]) dnl Configuration errors. AT_CHECK([ovs-vsctl set Interface t1 options:esp_in_key=KEY_IN], [0], [], -- 2.43.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
