diff mbox

[01/10] batman-adv: fix soft-interface MTU computation

Message ID 1392670129-2498-2-git-send-email-antonio@meshcoding.com (mailing list archive)
State Not Applicable, archived
Headers

Commit Message

Antonio Quartulli Feb. 17, 2014, 8:48 p.m. UTC 
  The current MTU computation always returns a value
smaller than 1500bytes even if the real interfaces
have an MTU large enough to compensate the batman-adv
overhead.

Fix the computation by properly returning the highest
admitted value.

Introduced by a19d3d85e1b854e4a483a55d740a42458085560d
("batman-adv: limit local translation table max size")

Reported-by: Russell Senior <russell@personaltelco.net>
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
---
 net/batman-adv/hard-interface.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

Comments

David Miller Feb. 17, 2014, 9:13 p.m. UTC | #1 
From: Antonio Quartulli <antonio@meshcoding.com>
Date: Mon, 17 Feb 2014 21:48:40 +0100

> +	atomic_set(&bat_priv->packet_size_max, min_mtu);

Please fix this.

The only operations performed on packet_size_max are 'set' and
'read'.  This is not what one uses atomic_t's for.

The use of an atomic_t in this context is a NOP.  You aren't
getting any kind of synchronization at all.
Antonio Quartulli Feb. 18, 2014, 6:44 a.m. UTC | #2 
On 17/02/14 22:13, David Miller wrote:
> From: Antonio Quartulli <antonio@meshcoding.com>
> Date: Mon, 17 Feb 2014 21:48:40 +0100
> 
>> +	atomic_set(&bat_priv->packet_size_max, min_mtu);
> 
> Please fix this.
> 
> The only operations performed on packet_size_max are 'set' and
> 'read'.  This is not what one uses atomic_t's for.
> 
> The use of an atomic_t in this context is a NOP.  You aren't
> getting any kind of synchronization at all.

True. Thanks for the suggestion.
Unfortunately this is not the only "fake-atomic" variable we have.

We'll send a change for this later within our pull request for net-next, ok?
David Miller Feb. 18, 2014, 6:22 p.m. UTC | #3 
From: Antonio Quartulli <antonio@meshcoding.com>
Date: Tue, 18 Feb 2014 07:44:53 +0100

> Unfortunately this is not the only "fake-atomic" variable we have.

:-(

> We'll send a change for this later within our pull request for net-next, ok?

Fair enough.
David Miller Feb. 18, 2014, 8:41 p.m. UTC | #4 
From: Antonio Quartulli <antonio@meshcoding.com>
Date: Tue, 18 Feb 2014 07:44:53 +0100

> On 17/02/14 22:13, David Miller wrote:
>> From: Antonio Quartulli <antonio@meshcoding.com>
>> Date: Mon, 17 Feb 2014 21:48:40 +0100
>> 
>>> +	atomic_set(&bat_priv->packet_size_max, min_mtu);
>> 
>> Please fix this.
>> 
>> The only operations performed on packet_size_max are 'set' and
>> 'read'.  This is not what one uses atomic_t's for.
>> 
>> The use of an atomic_t in this context is a NOP.  You aren't
>> getting any kind of synchronization at all.
> 
> True. Thanks for the suggestion.
> Unfortunately this is not the only "fake-atomic" variable we have.

So I pulled this, but I just want you to know that you should really
start to bear down and minimize the amount of changes you are submitting
for 'net' as we're starting to get deep into -rc territory.

Thanks.
Antonio Quartulli Feb. 18, 2014, 8:57 p.m. UTC | #5 
On 18/02/14 21:41, David Miller wrote:
> So I pulled this, but I just want you to know that you should really
> start to bear down and minimize the amount of changes you are submitting
> for 'net' as we're starting to get deep into -rc territory.

I decided to wait until the last bugfix was ready before sending the
patchset....but I imagine a better choice is to send the big bunch as
soon as possible and then keep the small remaining bits for later.

I will do that next time.
Thanks!
diff mbox

Patch

diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 3d417d3..b851cc5 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -241,7 +241,7 @@  int batadv_hardif_min_mtu(struct net_device *soft_iface)
 {
 	struct batadv_priv *bat_priv = netdev_priv(soft_iface);
 	const struct batadv_hard_iface *hard_iface;
-	int min_mtu = ETH_DATA_LEN;
+	int min_mtu = INT_MAX;
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(hard_iface, &batadv_hardif_list, list) {
@@ -256,8 +256,6 @@  int batadv_hardif_min_mtu(struct net_device *soft_iface)
 	}
 	rcu_read_unlock();
 
-	atomic_set(&bat_priv->packet_size_max, min_mtu);
-
 	if (atomic_read(&bat_priv->fragmentation) == 0)
 		goto out;
 
@@ -268,13 +266,21 @@  int batadv_hardif_min_mtu(struct net_device *soft_iface)
 	min_mtu = min_t(int, min_mtu, BATADV_FRAG_MAX_FRAG_SIZE);
 	min_mtu -= sizeof(struct batadv_frag_packet);
 	min_mtu *= BATADV_FRAG_MAX_FRAGMENTS;
-	atomic_set(&bat_priv->packet_size_max, min_mtu);
-
-	/* with fragmentation enabled we can fragment external packets easily */
-	min_mtu = min_t(int, min_mtu, ETH_DATA_LEN);
 
 out:
-	return min_mtu - batadv_max_header_len();
+	/* report to the other components the maximum amount of bytes that
+	 * batman-adv can send over the wire (without considering the payload
+	 * overhead). For example, this value is used by TT to compute the
+	 * maximum local table table size
+	 */
+	atomic_set(&bat_priv->packet_size_max, min_mtu);
+
+	/* the real soft-interface MTU is computed by removing the payload
+	 * overhead from the maximum amount of bytes that was just computed.
+	 *
+	 * However batman-adv does not support MTUs bigger than ETH_DATA_LEN
+	 */
+	return min_t(int, min_mtu - batadv_max_header_len(), ETH_DATA_LEN);
 }
 
 /* adjusts the MTU if a new interface with a smaller MTU appeared. */