On Sun, Sep 01, 2013 at 12:48:44PM +0800, Marek Lindner wrote:
On Sunday, September 01, 2013 02:38:29 Antonio Quartulli wrote:
@@ -717,12 +762,6 @@ static void batadv_tt_tvlv_container_update(struct batadv_priv *bat_priv)
tt_diff_entries_num = atomic_read(&bat_priv->tt.local_changes); tt_diff_len = batadv_tt_len(tt_diff_entries_num);
- /* if we have too many changes for one packet don't send any
* and wait for the tt table request which will be fragmented*/- if (tt_diff_len > bat_priv->soft_iface->mtu)
tt_diff_len = 0;Why are you removing this? We are not going to fragment OGMs and in this function we are preparing the TVLV to send within one of those.
Upon reading this function a realized this "tt_diff_len = 0" mechanism is buggy. For instance, the changes_list list is not purged but keeps growing until it magically fits the ogm diff again. Or checking bat_priv->soft_iface-
mtu isn't terribly accurate either. The more I looked the more my head
started to spin. Suddenly, I was convinced we don't need the check anymore because we have the fragmentation but you are right - it won't help here. I'll add the check again but be aware that it is broken.
Right, there is a bug. We should purge the list in any case, even if tt_diff_len is 0.
Yes, checking tt_diff_len against bat_priv->soft_iface->mtu is probably not correct..shall we check it against the mtu of the primary interface? Anyway, this is outside the scope of this patch.
Cheers,