[PATCH v3] babel: Rework seqno request handling

[Ondrej Zajicek]

On Sat, Dec 17, 2022 at 09:45:59PM +0100, Toke Høiland-Jørgensen via Bird-users wrote:
> The seqno request retransmission handling was tracking the destination that
> a forwarded request was being sent to and always retransmitting to that
> same destination. This is unnecessary because we only need to retransmit
> requests we originate ourselves, not those we forward on behalf of
> others; in fact retransmitting on behalf of others can lead to exponential
> multiplication of requests, which would be bad.

Hi

Thanks for investigating the issue and the patch. It took me some time to
process it, so answering now. I have some notes / suggestions:

 - In babel_expire_requests() the check for sr->forwarded is inside
 sr->count check, so BABEL_SEQNO_REQUEST_RETRY is applied for forwarded
 info, which does not make sense, perhaps it should be like:
 if (!sr->forwarded && (sr->count < BABEL_SEQNO_REQUEST_RETRY)) ...

 - There is no need to check hop_count in babel_handle_seqno_request(),
 that is already done in babel_read_seqno_request().

 - Seems to me that as the focus of the patch changed completely from v1
 to v3, some changes thate were initially introduced now makes less sense.
 Namely i would keep meaning of the last argument of babel_add_seqno_request()
 to be the neighbor we forward the request to, and keep seqno request routing
 (i.e. babel_find_seqno_request_tgt()) outside. Either keep old code in
 babel_handle_seqno_request(), or call babel_find_seqno_request_tgt() from
 that. That makes more sense to me as it is part of section 3.8.1.2
 implemented there.

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santiago at crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."