[PATCH v3] babel: Rework seqno request handling
Ondrej Zajicek
santiago at crfreenet.org
Mon Dec 19 04:11:11 CET 2022
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."
More information about the Bird-users
mailing list