On Thu, Sep 10, 2026 at 9:51 AM Han Zhou <[email protected]> wrote:
> The kernel tells us that it could not tell us what changed by reporting a
> change with no content, which happens when the receive buffer of the
> netlink socket overflows or when a message cannot be parsed. Since the
> notifiers share one socket, this is reported for every group.
>
> All three change handlers dropped that report, so the tables the notifiers
> track kept whatever state they had built from the messages that did make
> it through. Route and neighbor changes in a watched table were silently
> never acted upon, and the tracked kernel nexthop table kept entries that
> may no longer exist. What saved us most of the time is that any later
> recompute reads the tables again, but nothing guarantees one happens.
>
> Record that notifications were missed and let each user of a notifier
> resync: the route and neighbor notify nodes report an update whatever
> their watches say, and nexthop_exchange dumps the nexthop table again
> instead of applying the messages it has.
>
> Fixes: 673d90f1173f ("controller: Watch for route changes.")
> Assisted-by: Claude Opus 5, Cursor
> Signed-off-by: Han Zhou <[email protected]>
> ---
>
Hi Han,
thank you for the patch, just few small comments down below.
> controller/ovn-controller.c | 47 +++++++++++++++++++++++--------
> controller/ovn-netlink-notifier.c | 42 +++++++++++++++++++++++++--
> controller/ovn-netlink-notifier.h | 1 +
> 3 files changed, 75 insertions(+), 15 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index be02d28646ba..700158eef53a 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -5821,11 +5821,20 @@ en_route_table_notify_run(struct engine_node *node
> OVS_UNUSED, void *data)
> struct vector *msgs;
> uint32_t *table_id;
>
> - msgs = ovn_netlink_get_msgs(OVN_NL_NOTIFIER_ROUTE_V4);
> - VECTOR_FOR_EACH_PTR (msgs, table_id) {
> - if (vector_bsearch(&rtn->watches, table_id, table_id_cmp)) {
> - state = EN_UPDATED;
> - break;
> + /* We cannot tell whether a table we watch was among the changes we
> + * missed, so assume it was. */
>
I would leave out this comment.
> + if (ovn_netlink_notifier_lost(OVN_NL_NOTIFIER_ROUTE_V4) ||
> + ovn_netlink_notifier_lost(OVN_NL_NOTIFIER_ROUTE_V6)) {
> + state = EN_UPDATED;
> + }
> +
> + if (state != EN_UPDATED) {
> + msgs = ovn_netlink_get_msgs(OVN_NL_NOTIFIER_ROUTE_V4);
> + VECTOR_FOR_EACH_PTR (msgs, table_id) {
> + if (vector_bsearch(&rtn->watches, table_id, table_id_cmp)) {
> + state = EN_UPDATED;
> + break;
> + }
> }
> }
>
> @@ -6465,13 +6474,21 @@ en_neighbor_table_notify_run(struct engine_node
> *node OVS_UNUSED,
> struct vector *msgs;
> struct ne_table_msg *ne_msg;
>
> - msgs = ovn_netlink_get_msgs(OVN_NL_NOTIFIER_NEIGHBOR);
> - VECTOR_FOR_EACH_PTR (msgs, ne_msg) {
> - if (vector_bsearch(&ntn->watches,
> - &ne_msg->nd.if_index,
> - if_index_cmp)) {
> - state = EN_UPDATED;
> - break;
> + /* We cannot tell whether an interface we watch was among the changes
> we
> + * missed, so assume it was. */
>
Same here.
> + if (ovn_netlink_notifier_lost(OVN_NL_NOTIFIER_NEIGHBOR)) {
> + state = EN_UPDATED;
> + }
> +
> + if (state != EN_UPDATED) {
> + msgs = ovn_netlink_get_msgs(OVN_NL_NOTIFIER_NEIGHBOR);
> + VECTOR_FOR_EACH_PTR (msgs, ne_msg) {
> + if (vector_bsearch(&ntn->watches,
> + &ne_msg->nd.if_index,
> + if_index_cmp)) {
> + state = EN_UPDATED;
> + break;
> + }
> }
> }
>
> @@ -6521,6 +6538,12 @@ en_nexthop_exchange_run(struct engine_node *node
> OVS_UNUSED, void *data)
> return EN_UNCHANGED;
> }
>
> + /* The messages we did get do not describe every change, so the table
> has
> + * to be read again to find out what it looks like now. */
> + if (ovn_netlink_notifier_lost(OVN_NL_NOTIFIER_NEXTHOP)) {
> + nhe_data->recompute = true;
> + }
> +
> if (nhe_data->recompute) {
> nexthops_destroy(&nhe_data->nexthops);
> nexthops_sync(&nhe_data->nexthops);
> diff --git a/controller/ovn-netlink-notifier.c
> b/controller/ovn-netlink-notifier.c
> index ff0d01ca7550..04db42d0e109 100644
> --- a/controller/ovn-netlink-notifier.c
> +++ b/controller/ovn-netlink-notifier.c
> @@ -40,6 +40,10 @@ struct ovn_netlink_notifier {
> struct nln_notifier *nln_notifier;
> /* Messages received by given notifier. */
> struct vector msgs;
> + /* Set when the kernel reported a change we could not read, in which
> case
> + * 'msgs' does not describe everything that happened and the state
> derived
> + * from it has to be built again from scratch. */
> + bool lost;
> /* Notifier change handler. */
> nln_notify_func *change_handler;
> /* Name of the notifier. */
> @@ -57,6 +61,7 @@ static void ovn_netlink_neighbor_change_handler(const
> void *change_,
> void *aux);
> static void ovn_netlink_nexthop_change_handler(const void *change_,
> void *aux);
> +static void ovn_netlink_notifier_report_lost(struct ovn_netlink_notifier
> *);
>
> static struct ovn_netlink_notifier notifiers[OVN_NL_NOTIFIER_MAX] = {
> [OVN_NL_NOTIFIER_ROUTE_V4] = {
> @@ -115,14 +120,30 @@ ovn_netlink_notifier_parse(struct ofpbuf *buf, void
> *change_)
> return 0;
> }
>
> +/* Records that the kernel told us that something changed without us being
> + * able to tell what, which happens when the receive buffer overflows or
> when
> + * a message cannot be parsed. It is reported for every group, since the
> + * notifiers share a single socket. */
> +static void
> +ovn_netlink_notifier_report_lost(struct ovn_netlink_notifier *notifier)
> +{
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +
> + VLOG_INFO_RL(&rl, "Missed %s table notifications, resyncing.",
> + notifier->name);
> + notifier->lost = true;
> +}
> +
> static void
> ovn_netlink_route_change_handler(const void *change_, void *aux)
> {
> + struct ovn_netlink_notifier *notifier = aux;
> +
> if (!change_) {
> + ovn_netlink_notifier_report_lost(notifier);
> return;
> }
>
> - struct ovn_netlink_notifier *notifier = aux;
> union ovn_notifier_msg_change *change =
> CONST_CAST(union ovn_notifier_msg_change *, change_);
>
> @@ -139,11 +160,13 @@ ovn_netlink_route_change_handler(const void
> *change_, void *aux)
> static void
> ovn_netlink_neighbor_change_handler(const void *change_, void *aux)
> {
> + struct ovn_netlink_notifier *notifier = aux;
> +
> if (!change_) {
> + ovn_netlink_notifier_report_lost(notifier);
> return;
> }
>
> - struct ovn_netlink_notifier *notifier = aux;
> const union ovn_notifier_msg_change *change = change_;
>
> if (!ne_is_ovn_owned(&change->neighbor.nd)) {
> @@ -154,11 +177,13 @@ ovn_netlink_neighbor_change_handler(const void
> *change_, void *aux)
> static void
> ovn_netlink_nexthop_change_handler(const void *change_, void *aux)
> {
> + struct ovn_netlink_notifier *notifier = aux;
> +
> if (!change_) {
> + ovn_netlink_notifier_report_lost(notifier);
> return;
> }
>
> - struct ovn_netlink_notifier *notifier = aux;
> const union ovn_notifier_msg_change *change = change_;
> vector_push(¬ifier->msgs, &change->nexthop);
> }
> @@ -238,6 +263,16 @@ ovn_netlink_get_msgs(enum ovn_netlink_notifier_type
> type)
> return ¬ifiers[type].msgs;
> }
>
> +/* Returns true if notifications were missed since the last flush, in
> which
> + * case the messages of 'type' do not describe every change and the
> caller has
> + * to read the table it tracks again. */
> +bool
> +ovn_netlink_notifier_lost(enum ovn_netlink_notifier_type type)
> +{
> + ovs_assert(type < OVN_NL_NOTIFIER_MAX);
> + return notifiers[type].lost;
> +}
> +
> void
> ovn_netlink_notifier_flush(enum ovn_netlink_notifier_type type)
> {
> @@ -260,6 +295,7 @@ ovn_netlink_notifier_flush(enum
> ovn_netlink_notifier_type type)
> }
>
> vector_clear(¬ifier->msgs);
> + notifier->lost = false;
> }
>
> void
> diff --git a/controller/ovn-netlink-notifier.h
> b/controller/ovn-netlink-notifier.h
> index 208a28d9968e..08a8380e8924 100644
> --- a/controller/ovn-netlink-notifier.h
> +++ b/controller/ovn-netlink-notifier.h
> @@ -31,6 +31,7 @@ enum ovn_netlink_notifier_type {
> void ovn_netlink_update_notifier(enum ovn_netlink_notifier_type type,
> bool enabled);
> struct vector *ovn_netlink_get_msgs(enum ovn_netlink_notifier_type type);
> +bool ovn_netlink_notifier_lost(enum ovn_netlink_notifier_type type);
>
This is missing ovn-netlink-notifier-stub.c implementation.
> void ovn_netlink_notifier_flush(enum ovn_netlink_notifier_type type);
> void ovn_netlink_notifiers_run(void);
> void ovn_netlink_notifiers_wait(void);
> --
> 2.38.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
With that addressed:
Acked-by: Ales Musil <[email protected]>
Regards,
Ales
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
