From: Sven Eckelmann Both OGMv1 (on the primary interface) and OGM2 unconditionally reallocated their packet buffer on every transmission cycle, regardless of whether the required size had changed. This meant a kfree/kmalloc pair even when the TVLV payload size was identical to the previous send. Introduce struct batadv_ogm_buf to encapsulate the OGM packet buffer together with its current length, allocated capacity, and fixed header length. This consolidates the separate buf/len arguments that were previously threaded through each call site. In batadv_tvlv_realloc_packet_buff(), the capacity is rounded up to the next power of two so that small growth or shrinkage in TVLV data does not trigger a reallocation. When kmalloc fails but the existing buffer is large enough to hold the new data, the oversized buffer is reused rather than returning an error. Signed-off-by: Sven Eckelmann Signed-off-by: Simon Wunderlich --- net/batman-adv/bat_iv_ogm.c | 35 ++++++++++++----------- net/batman-adv/bat_v_ogm.c | 30 ++++++++++---------- net/batman-adv/tvlv.c | 56 ++++++++++++++++++++++--------------- net/batman-adv/tvlv.h | 3 +- net/batman-adv/types.h | 31 +++++++++++++------- 5 files changed, 88 insertions(+), 67 deletions(-) diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index c436b77674a56..6f5a468c4084c 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -195,14 +195,17 @@ static int batadv_iv_ogm_iface_enable(struct batadv_hard_iface *hard_iface) get_random_bytes(&random_seqno, sizeof(random_seqno)); atomic_set(&hard_iface->bat_iv.ogm_seqno, random_seqno); - hard_iface->bat_iv.ogm_buff_len = BATADV_OGM_HLEN; - ogm_buff = kmalloc(hard_iface->bat_iv.ogm_buff_len, GFP_ATOMIC); + hard_iface->bat_iv.ogm_buff.len = BATADV_OGM_HLEN; + hard_iface->bat_iv.ogm_buff.capacity = BATADV_OGM_HLEN; + hard_iface->bat_iv.ogm_buff.header_length = BATADV_OGM_HLEN; + + ogm_buff = kmalloc(hard_iface->bat_iv.ogm_buff.capacity, GFP_ATOMIC); if (!ogm_buff) { mutex_unlock(&hard_iface->bat_iv.ogm_buff_mutex); return -ENOMEM; } - hard_iface->bat_iv.ogm_buff = ogm_buff; + hard_iface->bat_iv.ogm_buff.buf = ogm_buff; batadv_ogm_packet = (struct batadv_ogm_packet *)ogm_buff; batadv_ogm_packet->packet_type = BATADV_IV_OGM; @@ -221,8 +224,9 @@ static void batadv_iv_ogm_iface_disable(struct batadv_hard_iface *hard_iface) { mutex_lock(&hard_iface->bat_iv.ogm_buff_mutex); - kfree(hard_iface->bat_iv.ogm_buff); - hard_iface->bat_iv.ogm_buff = NULL; + kfree(hard_iface->bat_iv.ogm_buff.buf); + memset(&hard_iface->bat_iv.ogm_buff, 0, + sizeof(hard_iface->bat_iv.ogm_buff)); mutex_unlock(&hard_iface->bat_iv.ogm_buff_mutex); @@ -236,7 +240,7 @@ static void batadv_iv_ogm_iface_update_mac(struct batadv_hard_iface *hard_iface) mutex_lock(&hard_iface->bat_iv.ogm_buff_mutex); - ogm_buff = hard_iface->bat_iv.ogm_buff; + ogm_buff = hard_iface->bat_iv.ogm_buff.buf; if (!ogm_buff) goto unlock; @@ -258,7 +262,7 @@ batadv_iv_ogm_primary_iface_set(struct batadv_hard_iface *hard_iface) mutex_lock(&hard_iface->bat_iv.ogm_buff_mutex); - ogm_buff = hard_iface->bat_iv.ogm_buff; + ogm_buff = hard_iface->bat_iv.ogm_buff.buf; if (!ogm_buff) goto unlock; @@ -796,10 +800,9 @@ batadv_iv_ogm_slide_own_bcast_window(struct batadv_hard_iface *hard_iface) static void batadv_iv_ogm_schedule_buff(struct batadv_hard_iface *hard_iface) { struct batadv_priv *bat_priv = netdev_priv(hard_iface->mesh_iface); - unsigned char **ogm_buff = &hard_iface->bat_iv.ogm_buff; + struct batadv_ogm_buf *ogm_buff = &hard_iface->bat_iv.ogm_buff; struct batadv_ogm_packet *batadv_ogm_packet; struct batadv_hard_iface *primary_if, *tmp_hard_iface; - int *ogm_buff_len = &hard_iface->bat_iv.ogm_buff_len; struct list_head *iter; u32 seqno; u16 tvlv_len = 0; @@ -811,7 +814,7 @@ static void batadv_iv_ogm_schedule_buff(struct batadv_hard_iface *hard_iface) lockdep_assert_held(&hard_iface->bat_iv.ogm_buff_mutex); /* interface already disabled by batadv_iv_ogm_iface_disable */ - if (!*ogm_buff) + if (!ogm_buff->buf) return; /* the interface gets activated here to avoid race conditions between @@ -830,9 +833,7 @@ static void batadv_iv_ogm_schedule_buff(struct batadv_hard_iface *hard_iface) * appended as it may alter the tt tvlv container */ batadv_tt_local_commit_changes(bat_priv); - ret = batadv_tvlv_container_ogm_append(bat_priv, ogm_buff, - ogm_buff_len, - BATADV_OGM_HLEN); + ret = batadv_tvlv_container_ogm_append(bat_priv, ogm_buff); if (ret < 0) { reschedule = true; goto out; @@ -841,7 +842,7 @@ static void batadv_iv_ogm_schedule_buff(struct batadv_hard_iface *hard_iface) tvlv_len = ret; } - batadv_ogm_packet = (struct batadv_ogm_packet *)(*ogm_buff); + batadv_ogm_packet = ogm_buff->buf; batadv_ogm_packet->tvlv_len = htons(tvlv_len); /* change sequence number to network order */ @@ -857,7 +858,7 @@ static void batadv_iv_ogm_schedule_buff(struct batadv_hard_iface *hard_iface) /* OGMs from secondary interfaces are only scheduled on their * respective interfaces. */ - scheduled = batadv_iv_ogm_queue_add(bat_priv, *ogm_buff, *ogm_buff_len, + scheduled = batadv_iv_ogm_queue_add(bat_priv, ogm_buff->buf, ogm_buff->len, hard_iface, hard_iface, 1, send_time); if (!scheduled) reschedule = true; @@ -873,8 +874,8 @@ static void batadv_iv_ogm_schedule_buff(struct batadv_hard_iface *hard_iface) if (!kref_get_unless_zero(&tmp_hard_iface->refcount)) continue; - scheduled = batadv_iv_ogm_queue_add(bat_priv, *ogm_buff, - *ogm_buff_len, hard_iface, + scheduled = batadv_iv_ogm_queue_add(bat_priv, ogm_buff->buf, + ogm_buff->len, hard_iface, tmp_hard_iface, 1, send_time); batadv_hardif_put(tmp_hard_iface); diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c index b337bd8e58e7d..2c4dca639709c 100644 --- a/net/batman-adv/bat_v_ogm.c +++ b/net/batman-adv/bat_v_ogm.c @@ -270,10 +270,9 @@ static void batadv_v_ogm_send_meshif(struct batadv_priv *bat_priv) { struct batadv_hard_iface *hard_iface; struct batadv_ogm2_packet *ogm_packet; + struct batadv_ogm_buf *ogm_buff; struct sk_buff *skb, *skb_tmp; - unsigned char **ogm_buff; struct list_head *iter; - int *ogm_buff_len; u16 tvlv_len; int ret; @@ -283,26 +282,23 @@ static void batadv_v_ogm_send_meshif(struct batadv_priv *bat_priv) goto out; ogm_buff = &bat_priv->bat_v.ogm_buff; - ogm_buff_len = &bat_priv->bat_v.ogm_buff_len; /* tt changes have to be committed before the tvlv data is * appended as it may alter the tt tvlv container */ batadv_tt_local_commit_changes(bat_priv); - ret = batadv_tvlv_container_ogm_append(bat_priv, ogm_buff, - ogm_buff_len, - BATADV_OGM2_HLEN); + ret = batadv_tvlv_container_ogm_append(bat_priv, ogm_buff); if (ret < 0) goto reschedule; tvlv_len = ret; - skb = netdev_alloc_skb_ip_align(NULL, ETH_HLEN + *ogm_buff_len); + skb = netdev_alloc_skb_ip_align(NULL, ETH_HLEN + ogm_buff->len); if (!skb) goto reschedule; skb_reserve(skb, ETH_HLEN); - skb_put_data(skb, *ogm_buff, *ogm_buff_len); + skb_put_data(skb, ogm_buff->buf, ogm_buff->len); ogm_packet = (struct batadv_ogm2_packet *)skb->data; ogm_packet->seqno = htonl(atomic_read(&bat_priv->bat_v.ogm_seqno)); @@ -448,10 +444,10 @@ void batadv_v_ogm_primary_iface_set(struct batadv_hard_iface *primary_iface) struct batadv_ogm2_packet *ogm_packet; mutex_lock(&bat_priv->bat_v.ogm_buff_mutex); - if (!bat_priv->bat_v.ogm_buff) + if (!bat_priv->bat_v.ogm_buff.buf) goto unlock; - ogm_packet = (struct batadv_ogm2_packet *)bat_priv->bat_v.ogm_buff; + ogm_packet = bat_priv->bat_v.ogm_buff.buf; ether_addr_copy(ogm_packet->orig, primary_iface->net_dev->dev_addr); unlock: @@ -1052,12 +1048,15 @@ int batadv_v_ogm_init(struct batadv_priv *bat_priv) unsigned char *ogm_buff; u32 random_seqno; - bat_priv->bat_v.ogm_buff_len = BATADV_OGM2_HLEN; - ogm_buff = kzalloc(bat_priv->bat_v.ogm_buff_len, GFP_ATOMIC); + bat_priv->bat_v.ogm_buff.len = BATADV_OGM2_HLEN; + bat_priv->bat_v.ogm_buff.capacity = BATADV_OGM2_HLEN; + bat_priv->bat_v.ogm_buff.header_length = BATADV_OGM2_HLEN; + + ogm_buff = kzalloc(bat_priv->bat_v.ogm_buff.capacity, GFP_ATOMIC); if (!ogm_buff) return -ENOMEM; - bat_priv->bat_v.ogm_buff = ogm_buff; + bat_priv->bat_v.ogm_buff.buf = ogm_buff; ogm_packet = (struct batadv_ogm2_packet *)ogm_buff; ogm_packet->packet_type = BATADV_OGM2; ogm_packet->version = BATADV_COMPAT_VERSION; @@ -1085,9 +1084,8 @@ void batadv_v_ogm_free(struct batadv_priv *bat_priv) mutex_lock(&bat_priv->bat_v.ogm_buff_mutex); - kfree(bat_priv->bat_v.ogm_buff); - bat_priv->bat_v.ogm_buff = NULL; - bat_priv->bat_v.ogm_buff_len = 0; + kfree(bat_priv->bat_v.ogm_buff.buf); + memset(&bat_priv->bat_v.ogm_buff, 0, sizeof(bat_priv->bat_v.ogm_buff)); mutex_unlock(&bat_priv->bat_v.ogm_buff_mutex); } diff --git a/net/batman-adv/tvlv.c b/net/batman-adv/tvlv.c index 8e8b54e2172ef..cd75daea478c5 100644 --- a/net/batman-adv/tvlv.c +++ b/net/batman-adv/tvlv.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -267,32 +268,48 @@ void batadv_tvlv_container_register(struct batadv_priv *bat_priv, /** * batadv_tvlv_realloc_packet_buff() - reallocate packet buffer to accommodate * requested packet size - * @packet_buff: packet buffer - * @packet_buff_len: packet buffer size - * @min_packet_len: requested packet minimum size + * @ogm_buff: ogm packet buffer * @additional_packet_len: requested additional packet size on top of minimum * size * * Return: true of the packet buffer could be changed to the requested size, * false otherwise. */ -static bool batadv_tvlv_realloc_packet_buff(unsigned char **packet_buff, - int *packet_buff_len, - int min_packet_len, - int additional_packet_len) +static bool batadv_tvlv_realloc_packet_buff(struct batadv_ogm_buf *ogm_buff, + size_t additional_packet_len) { unsigned char *new_buff; + size_t newcapacity; + size_t newlen; - new_buff = kmalloc(min_packet_len + additional_packet_len, GFP_ATOMIC); + newlen = ogm_buff->header_length + additional_packet_len; + newcapacity = roundup_pow_of_two(newlen); + + /* nothing to reallocate */ + if (newcapacity == ogm_buff->capacity) { + ogm_buff->len = newlen; + return true; + } + + new_buff = kmalloc(newcapacity, GFP_ATOMIC); /* keep old buffer if kmalloc should fail */ - if (!new_buff) + if (!new_buff) { + /* continue to use oversize buffer if new data fits */ + if (newlen <= ogm_buff->capacity) { + ogm_buff->len = newlen; + return true; + } + return false; + } + + memcpy(new_buff, ogm_buff->buf, ogm_buff->header_length); + kfree(ogm_buff->buf); - memcpy(new_buff, *packet_buff, min_packet_len); - kfree(*packet_buff); - *packet_buff = new_buff; - *packet_buff_len = min_packet_len + additional_packet_len; + ogm_buff->buf = new_buff; + ogm_buff->len = newlen; + ogm_buff->capacity = newcapacity; return true; } @@ -301,10 +318,7 @@ static bool batadv_tvlv_realloc_packet_buff(unsigned char **packet_buff, * batadv_tvlv_container_ogm_append() - append tvlv container content to given * OGM packet buffer * @bat_priv: the bat priv with all the mesh interface information - * @packet_buff: ogm packet buffer - * @packet_buff_len: ogm packet buffer size including ogm header and tvlv - * content - * @packet_min_len: ogm header size to be preserved for the OGM itself + * @ogm_buff: ogm packet buffer * * The ogm packet might be enlarged or shrunk depending on the current size * and the size of the to-be-appended tvlv containers. @@ -313,8 +327,7 @@ static bool batadv_tvlv_realloc_packet_buff(unsigned char **packet_buff, * if operation failed */ int batadv_tvlv_container_ogm_append(struct batadv_priv *bat_priv, - unsigned char **packet_buff, - int *packet_buff_len, int packet_min_len) + struct batadv_ogm_buf *ogm_buff) { struct batadv_tvlv_container *tvlv; struct batadv_tvlv_hdr *tvlv_hdr; @@ -330,8 +343,7 @@ int batadv_tvlv_container_ogm_append(struct batadv_priv *bat_priv, goto end; } - ret = batadv_tvlv_realloc_packet_buff(packet_buff, packet_buff_len, - packet_min_len, tvlv_value_len); + ret = batadv_tvlv_realloc_packet_buff(ogm_buff, tvlv_value_len); if (!ret) { tvlv_len_ret = -ENOMEM; goto end; @@ -342,7 +354,7 @@ int batadv_tvlv_container_ogm_append(struct batadv_priv *bat_priv, if (!tvlv_value_len) goto end; - tvlv_value = (*packet_buff) + packet_min_len; + tvlv_value = (u8 *)ogm_buff->buf + ogm_buff->header_length; hlist_for_each_entry(tvlv, &bat_priv->tvlv.container_list, list) { tvlv_hdr = tvlv_value; diff --git a/net/batman-adv/tvlv.h b/net/batman-adv/tvlv.h index f96f6b3f44a00..fc97838504431 100644 --- a/net/batman-adv/tvlv.h +++ b/net/batman-adv/tvlv.h @@ -17,8 +17,7 @@ void batadv_tvlv_container_register(struct batadv_priv *bat_priv, u8 type, u8 version, void *tvlv_value, u16 tvlv_value_len); int batadv_tvlv_container_ogm_append(struct batadv_priv *bat_priv, - unsigned char **packet_buff, - int *packet_buff_len, int packet_min_len); + struct batadv_ogm_buf *ogm_buff); void batadv_tvlv_ogm_receive(struct batadv_priv *bat_priv, struct batadv_ogm_packet *batadv_ogm_packet, struct batadv_orig_node *orig_node); diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index 5d4ad35c19636..c3b6af399110b 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -71,15 +71,29 @@ enum batadv_dhcp_recipient { */ #define BATADV_TT_SYNC_MASK 0x00F0 +/** + * struct batadv_ogm_buf - Buffer to construct an OGM with TVLV + */ +struct batadv_ogm_buf { + /** @buf: buffer holding the OGM packet */ + void *buf; + + /** @len: length of the OGM packet buffer data */ + size_t len; + + /** @capacity: size of allocated buf */ + size_t capacity; + + /** @header_length: fixed size header length (must be <= len) */ + size_t header_length; +}; + /** * struct batadv_hard_iface_bat_iv - per hard-interface B.A.T.M.A.N. IV data */ struct batadv_hard_iface_bat_iv { /** @ogm_buff: buffer holding the OGM packet */ - unsigned char *ogm_buff; - - /** @ogm_buff_len: length of the OGM packet buffer */ - int ogm_buff_len; + struct batadv_ogm_buf ogm_buff; /** @ogm_seqno: OGM sequence number - used to identify each OGM */ atomic_t ogm_seqno; @@ -87,7 +101,7 @@ struct batadv_hard_iface_bat_iv { /** @reschedule_work: recover OGM schedule after schedule error */ struct delayed_work reschedule_work; - /** @ogm_buff_mutex: lock protecting ogm_buff and ogm_buff_len */ + /** @ogm_buff_mutex: lock protecting ogm_buff */ struct mutex ogm_buff_mutex; }; @@ -1481,15 +1495,12 @@ struct batadv_meshif_vlan { */ struct batadv_priv_bat_v { /** @ogm_buff: buffer holding the OGM packet */ - unsigned char *ogm_buff; - - /** @ogm_buff_len: length of the OGM packet buffer */ - int ogm_buff_len; + struct batadv_ogm_buf ogm_buff; /** @ogm_seqno: OGM sequence number - used to identify each OGM */ atomic_t ogm_seqno; - /** @ogm_buff_mutex: lock protecting ogm_buff and ogm_buff_len */ + /** @ogm_buff_mutex: lock protecting ogm_buff */ struct mutex ogm_buff_mutex; /** @ogm_wq: workqueue used to schedule OGM transmissions */ -- 2.47.3