Re: [PATCH net v2 2/3] amt: make pre-query report drops visible

[netdev-bot+sashiko]

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 commit message's protocol explanation is wrong.

--

amt: make pre-query report drops visible

In gateway mode, amt_dev_xmit() drops an IGMP or MLD report when no
Membership Query has arrived yet for that family. This commit adds a
family-specific netdev_dbg() message before that drop. The drop path
itself is unchanged and still increments tx_dropped.

> A gateway cannot forward an IGMP or MLD report until it has received the
> relay's Membership Query for that family. The query supplies the nonce and
> interval echoed by the Membership Update, so dropping an early report is
> required, but doing so silently leaves operators with a dark multicast path
> and no indication why the join never happened.

[Severity: Low]
Is "The query supplies the nonce and interval echoed by the Membership
Update" accurate? The code doesn't seem to match it.

The gateway generates the nonce itself in amt_event_send_request():

drivers/net/amt.c:amt_event_send_request() {
    ...
		get_random_bytes(&amt->nonce, sizeof(__be32));
    ...
}

amt_membership_query_handler() only checks the nonce against that value.
>From the Query it stores the response MAC and the query interval:

drivers/net/amt.c:amt_membership_query_handler() {
    ...
	if (amtmq->nonce != amt->nonce)
		return true;

	response_mac = amtmq->response_mac;
    ...
		WRITE_ONCE(amt->ready4, true);
		amt->mac = response_mac;
		amt->req_cnt = 0;
		amt->qi = ihv3->qqic;
    ...
}

amt_send_membership_update() echoes only the nonce and the response MAC.
amt->qi is not part of the Update:

drivers/net/amt.c:amt_send_membership_update() {
    ...
	amtmu->nonce		= amt->nonce;
	amtmu->response_mac	= amt->mac;
    ...
}

So the state the Update needs from the relay's Query is the response
MAC, not the nonce or the interval. Could the commit message say that
instead?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261009201455.1904698-1-omar%40blockcast.net