Re: [PATCH net 02/10] batman-adv: Close OGM aggregation before transmission
flat view
From: netdev-bot+sashiko@kernel.org
Date: 2026-10-27 08:55:22
Also in:
batman
Thank you for your contribution! Sashiko AI review found more severe issue(s) in other patches of this series. This mail is only an FYI, in case you want to address the nit picks below when you respin. Low priority / nit picks for your consideration: - [Low] The patch gives `struct batadv_forw_packet::num_packets` a second meaning without updating its kernel-doc. -- batman-adv: Close OGM aggregation before transmission This patch makes the OGM send worker mark the forwarding packet as a full aggregate under forw_bat_list_lock before it calls batadv_iv_ogm_emit(). After that, no other context can append OGMs while the packet is being emitted and cloned. The packet stays on forw_bat_list so interface purging can still find it.
quoted hunk ↗ jump to hunk
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index 158d2b8f04036..85e99506a53f5 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c@@ -1909,6 +1909,11 @@ static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work) goto out; } + /* Mark aggregate as full before forcing emit. */ + spin_lock_bh(&bat_priv->forw_bat_list_lock); + forw_packet->num_packets = BATADV_MAX_AGGREGATION_PACKETS; + spin_unlock_bh(&bat_priv->forw_bat_list_lock);
[Severity: Low] This isn't a bug, but should the kernel-doc for num_packets be updated to match this new use? In net/batman-adv/types.h the field is still documented as a plain counter: /** @num_packets: counter for aggregated OGMv1 packets */ u8 num_packets; After this change, the worker in batadv_iv_send_outstanding_bat_ogm_packet() replaces the real count with BATADV_MAX_AGGREGATION_PACKETS, whatever the number of queued OGMs is. For example, an aggregate holding 3 OGMs reports 32 from then on. The field is now both a counter and a "closed for aggregation" marker, and only the comment at this call site says so. As far as I can tell, nothing reads the inflated value today: - batadv_iv_ogm_can_aggregate() rejects packet_num >= BATADV_MAX_AGGREGATION_PACKETS under the same lock. - batadv_iv_ogm_aggregate() is only reached after that check passes, so set_bit(num_packets, direct_link_flags) cannot index past the bitmap. - batadv_iv_ogm_send_to_if() walks the OGMs using packet_len and a local counter. A short note in the struct documentation would help later readers who rely on the field description.
batadv_iv_ogm_emit(forw_packet); /* we have to have at least one packet in the queue to determine the
-- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261026085309.1074911-2-sw%40simonwunderlich.de