mbox series

[v3,0/5] batman-adv: TT change events fixes and improvements

Message ID cover.1732124716.git.repk@triplefau.lt (mailing list archive)
Headers
Series batman-adv: TT change events fixes and improvements |

Message

Remi Pommarel Nov. 20, 2024, 5:47 p.m. UTC 
  The first three patches are actual fixes.

The first two try to avoid sending uninitialized data that could be
interpreted as invalid TT change events in both TT change response and
OGM.  Following invalid entries could be seen when that happen with
batctl o:

 * 00:00:00:00:00:00   -1 [....] (  0) 88:12:4e:ad:7e:ba (179) (0x45845380)
 * 00:00:00:00:78:79 4092 [.W..] (  0) 88:12:4e:ad:7e:3c (145) (0x8ebadb8b)

The third one fixes an issue that happened when a TT change event list
is too big for the MTU, the list was never actually sent nor free and
continued to grow indefinitely from this point. That also caused the
OGM TTVN to increase at each OGM interval without any changes being ever
visible to other nodes. This ever growing TT change event list could be
observed by looking at /sys/kernel/slab/batadv_tt_change_cache/objects
that sometimes showed unusal high value even after issuing a memcache
shrink.

The next two patches are more cleanup / potential slight improvements.
While patch 4 is mainly cosmetic (having negative tt.local_changes
values is not exactly an issue), patch 5 is here to keep the TT changes
list as short as possible (reducing network overhead).

V3:
  - Fix commit message wording
  - Update outdated comments

V2:
  - This has been tested enough to not be in RFC state anymore
  - Add one more uninitialize TT change fix for full table TT responses

Remi Pommarel (5):
  batman-adv: Do not send uninitialized TT changes
  batman-adv: Remove uninitialized data in full table TT response
  batman-adv: Do not let TT changes list grows indefinitely
  batman-adv: Remove atomic usage for tt.local_changes
  batman-adv: Don't keep redundant TT change events

 net/batman-adv/soft-interface.c    |  2 +-
 net/batman-adv/translation-table.c | 92 ++++++++++++++++++------------
 net/batman-adv/types.h             |  4 +-
 3 files changed, 60 insertions(+), 38 deletions(-)

Comments

Sven Eckelmann Nov. 20, 2024, 7:46 p.m. UTC | #1 
On Wednesday, 20 November 2024 18:47:13 CET Remi Pommarel wrote:
> The first three patches are actual fixes.
[...]
> 
> V3:
>   - Fix commit message wording
>   - Update outdated comments

Antonio, can you please review the patches? It is important that we get the 
first three out early (to avoid merge conflicts when sending the second part). 
So maybe you can have at least a look at the first three.

Kind regards,
	Sven
Remi Pommarel Nov. 20, 2024, 7:54 p.m. UTC | #2 
On Wed, Nov 20, 2024 at 08:46:12PM +0100, Sven Eckelmann wrote:
> On Wednesday, 20 November 2024 18:47:13 CET Remi Pommarel wrote:
> > The first three patches are actual fixes.
> [...]
> > 
> > V3:
> >   - Fix commit message wording
> >   - Update outdated comments
> 
> Antonio, can you please review the patches? It is important that we get the 
> first three out early (to avoid merge conflicts when sending the second part). 
> So maybe you can have at least a look at the first three.

If it is more convenient for you, I sure can split the serie in two ?

Also just saw that I forgot to remove the "a" from "a creating" in the
commit log, sorry about that. Do you need re-spin for that ?

Thanks,
Antonio Quartulli Nov. 20, 2024, 8:29 p.m. UTC | #3 
On 20/11/2024 20:54, Remi Pommarel wrote:
> On Wed, Nov 20, 2024 at 08:46:12PM +0100, Sven Eckelmann wrote:
>> On Wednesday, 20 November 2024 18:47:13 CET Remi Pommarel wrote:
>>> The first three patches are actual fixes.
>> [...]
>>>
>>> V3:
>>>    - Fix commit message wording
>>>    - Update outdated comments
>>
>> Antonio, can you please review the patches? It is important that we get the
>> first three out early (to avoid merge conflicts when sending the second part).
>> So maybe you can have at least a look at the first three.
> 
> If it is more convenient for you, I sure can split the serie in two ?

No problem for me. I'll allocate some hours tomorrow morning.
Thanks for the ping, Sven.

Regards,

> 
> Also just saw that I forgot to remove the "a" from "a creating" in the
> commit log, sorry about that. Do you need re-spin for that ?
> 
> Thanks,
>
Sven Eckelmann Nov. 20, 2024, 9:04 p.m. UTC | #4 
On Wednesday, 20 November 2024 20:54:49 CET Remi Pommarel wrote:
> Also just saw that I forgot to remove the "a" from "a creating" in the
> commit log, sorry about that. Do you need re-spin for that ?

No, I will try to fix this directly when applying the patch.

Kind regards,
	Sven