Message ID | 1392670129-2498-2-git-send-email-antonio@meshcoding.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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?
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 --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. */