The ethernet header is 14 bytes long. Therefore, the data after it is not 4 byte aligned and may cause problems on systems without unaligned data access. Reserving NET_IP_ALIGN more bytes can fix the misalignment of the ethernet header.
Signed-off-by: Sven Eckelmann sven@narfation.org --- bat_iv_ogm.c | 2 +- icmp_socket.c | 2 +- translation-table.c | 10 +++++----- vis.c | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 75403a4..5be9a3e 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -422,7 +422,7 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff, kfree(forw_packet_aggr); goto out; } - skb_reserve(forw_packet_aggr->skb, ETH_HLEN); + skb_reserve(forw_packet_aggr->skb, ETH_HLEN + NET_IP_ALIGN);
INIT_HLIST_NODE(&forw_packet_aggr->list);
diff --git a/icmp_socket.c b/icmp_socket.c index 5874c0e..f64e9c8 100644 --- a/icmp_socket.c +++ b/icmp_socket.c @@ -183,7 +183,7 @@ static ssize_t batadv_socket_write(struct file *file, const char __user *buff, goto out; }
- skb_reserve(skb, ETH_HLEN); + skb_reserve(skb, ETH_HLEN + NET_IP_ALIGN); icmp_packet = (struct batadv_icmp_packet_rr *)skb_put(skb, packet_len);
if (copy_from_user(icmp_packet, buff, packet_len)) { diff --git a/translation-table.c b/translation-table.c index 6fecb94..bb3941c 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1598,7 +1598,7 @@ batadv_tt_response_fill_table(uint16_t tt_len, uint8_t ttvn, if (!skb) goto out;
- skb_reserve(skb, ETH_HLEN); + skb_reserve(skb, ETH_HLEN + NET_IP_ALIGN); tt_response = (struct batadv_tt_query_packet *)skb_put(skb, len); tt_response->ttvn = ttvn;
@@ -1663,7 +1663,7 @@ static int batadv_send_tt_request(struct batadv_priv *bat_priv, if (!skb) goto out;
- skb_reserve(skb, ETH_HLEN); + skb_reserve(skb, ETH_HLEN + NET_IP_ALIGN);
tt_req_len = sizeof(*tt_request); tt_request = (struct batadv_tt_query_packet *)skb_put(skb, tt_req_len); @@ -1765,7 +1765,7 @@ batadv_send_other_tt_response(struct batadv_priv *bat_priv, if (!skb) goto unlock;
- skb_reserve(skb, ETH_HLEN); + skb_reserve(skb, ETH_HLEN + NET_IP_ALIGN); packet_pos = skb_put(skb, len); tt_response = (struct batadv_tt_query_packet *)packet_pos; tt_response->ttvn = req_ttvn; @@ -1884,7 +1884,7 @@ batadv_send_my_tt_response(struct batadv_priv *bat_priv, if (!skb) goto unlock;
- skb_reserve(skb, ETH_HLEN); + skb_reserve(skb, ETH_HLEN + NET_IP_ALIGN); packet_pos = skb_put(skb, len); tt_response = (struct batadv_tt_query_packet *)packet_pos; tt_response->ttvn = req_ttvn; @@ -2215,7 +2215,7 @@ static void batadv_send_roam_adv(struct batadv_priv *bat_priv, uint8_t *client, if (!skb) goto out;
- skb_reserve(skb, ETH_HLEN); + skb_reserve(skb, ETH_HLEN + NET_IP_ALIGN);
roam_adv_packet = (struct batadv_roam_adv_packet *)skb_put(skb, len);
diff --git a/vis.c b/vis.c index 79589a3..b004560 100644 --- a/vis.c +++ b/vis.c @@ -401,7 +401,7 @@ batadv_add_packet(struct batadv_priv *bat_priv, kfree(info); return NULL; } - skb_reserve(info->skb_packet, ETH_HLEN); + skb_reserve(info->skb_packet, ETH_HLEN + NET_IP_ALIGN); packet = (struct batadv_vis_packet *)skb_put(info->skb_packet, len);
kref_init(&info->refcount); @@ -861,7 +861,7 @@ int batadv_vis_init(struct batadv_priv *bat_priv) if (!bat_priv->vis.my_info->skb_packet) goto free_info;
- skb_reserve(bat_priv->vis.my_info->skb_packet, ETH_HLEN); + skb_reserve(bat_priv->vis.my_info->skb_packet, ETH_HLEN + NET_IP_ALIGN); tmp_skb = bat_priv->vis.my_info->skb_packet; packet = (struct batadv_vis_packet *)skb_put(tmp_skb, sizeof(*packet));
Headers which are already perfectly aligned and create a 4 byte boundary non-ethernet header payload can have the __packed attribute removed. The __packed attribute doesn't change the appeareance of the packet for these headers because no extra padding is necessary to align the data members. The compiler will also create slightly faster code for loads of multi-byte members.
The definition of the batadv_bcast_packet has to change slightly because the 32 bit sequence number would cause this packet to be padded with two extra bytes at the end. These "missing" two bytes are used to fix the alignment of the payload after the following ethernet header. Splitting the sequence number in two 16 bit values avoids it without changing the actual packet.
Signed-off-by: Sven Eckelmann sven@narfation.org --- Newer tested - but at least it builds ;)
packet.h | 25 +++++++++++++++++-------- routing.c | 21 ++++++++++++++++++--- soft-interface.c | 16 +++++++++++++++- 3 files changed, 50 insertions(+), 12 deletions(-)
diff --git a/packet.h b/packet.h index aa3e63a..43c863c 100644 --- a/packet.h +++ b/packet.h @@ -121,13 +121,15 @@ struct batadv_bla_claim_dst { uint8_t magic[3]; /* FF:43:05 */ uint8_t type; /* bla_claimframe */ __be16 group; /* group id */ -} __packed; +};
struct batadv_header { uint8_t packet_type; uint8_t version; /* batman version field */ uint8_t ttl; -} __packed; + /* the parent struct has to add a byte after the header to make + * everything 4 bytes aligned again */ +};
struct batadv_ogm_packet { struct batadv_header header; @@ -152,7 +154,7 @@ struct batadv_icmp_packet { __be16 seqno; uint8_t uid; uint8_t reserved; -} __packed; +};
#define BATADV_RR_LEN 16
@@ -168,13 +170,16 @@ struct batadv_icmp_packet_rr { uint8_t uid; uint8_t rr_cur; uint8_t rr[BATADV_RR_LEN][ETH_ALEN]; -} __packed; +};
struct batadv_unicast_packet { struct batadv_header header; uint8_t ttvn; /* destination translation table version number */ uint8_t dest[ETH_ALEN]; -} __packed; + /* "4 bytes boundary + 2 bytes" long to make the payload after the + * following ethernet header again 4 bytes boundary aligned + */ +};
/** * struct batadv_unicast_4addr_packet - extended unicast packet @@ -201,9 +206,13 @@ struct batadv_unicast_frag_packet { struct batadv_bcast_packet { struct batadv_header header; uint8_t reserved; - __be32 seqno; + __be16 seqno_hi; + __be16 seqno_lo; uint8_t orig[ETH_ALEN]; -} __packed; + /* "4 bytes boundary + 2 bytes" long to make the payload after the + * following ethernet header again 4 bytes boundary aligned + */ +};
struct batadv_vis_packet { struct batadv_header header; @@ -214,7 +223,7 @@ struct batadv_vis_packet { uint8_t vis_orig[ETH_ALEN]; /* originator reporting its neighbors */ uint8_t target_orig[ETH_ALEN]; /* who should receive this packet */ uint8_t sender_orig[ETH_ALEN]; /* who sent or forwarded this packet */ -} __packed; +};
struct batadv_tt_query_packet { struct batadv_header header; diff --git a/routing.c b/routing.c index d5a01d1..f7d50d0 100644 --- a/routing.c +++ b/routing.c @@ -1115,6 +1115,17 @@ rx_success: return batadv_route_unicast_packet(skb, recv_if); }
+/** + * batadv_ntoh_seqno32 - Convert 2 seqno parts to a single host byte order seqno + * @seqno_hi: Upper 16 bit part of the sequence number + * @seqno_lo: Lower 16 bit part of the sequence number + * + * Returns the complete host byte order sequence number + */ +static uint32_t batadv_ntoh_seqno32(__be16 seqno_hi, __be16 seqno_lo) +{ + return ntohs(seqno_hi) << 16 | ntohs(seqno_lo); +}
int batadv_recv_bcast_packet(struct sk_buff *skb, struct batadv_hard_iface *recv_if) @@ -1126,6 +1137,7 @@ int batadv_recv_bcast_packet(struct sk_buff *skb, int hdr_size = sizeof(*bcast_packet); int ret = NET_RX_DROP; int32_t seq_diff; + uint32_t seqno;
/* drop packet if it has not necessary minimum size */ if (unlikely(!pskb_may_pull(skb, hdr_size))) @@ -1161,12 +1173,15 @@ int batadv_recv_bcast_packet(struct sk_buff *skb,
spin_lock_bh(&orig_node->bcast_seqno_lock);
+ seqno = batadv_ntoh_seqno32(bcast_packet->seqno_hi, + bcast_packet->seqno_lo); + /* check whether the packet is a duplicate */ if (batadv_test_bit(orig_node->bcast_bits, orig_node->last_bcast_seqno, - ntohl(bcast_packet->seqno))) + seqno)) goto spin_unlock;
- seq_diff = ntohl(bcast_packet->seqno) - orig_node->last_bcast_seqno; + seq_diff = seqno - orig_node->last_bcast_seqno;
/* check whether the packet is old and the host just restarted. */ if (batadv_window_protected(bat_priv, seq_diff, @@ -1177,7 +1192,7 @@ int batadv_recv_bcast_packet(struct sk_buff *skb, * if required. */ if (batadv_bit_get_packet(bat_priv, orig_node->bcast_bits, seq_diff, 1)) - orig_node->last_bcast_seqno = ntohl(bcast_packet->seqno); + orig_node->last_bcast_seqno = seqno;
spin_unlock_bh(&orig_node->bcast_seqno_lock);
diff --git a/soft-interface.c b/soft-interface.c index 2d1f895..7d1b0ce 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -138,6 +138,19 @@ static int batadv_interface_change_mtu(struct net_device *dev, int new_mtu) return 0; }
+/** + * batadv_hton_seqno32 - Convert seqno in two sequence number parts + * @seqno: Sequence number to be converted + * @seqno_hi: Pointer to upper 16 bit part of the sequence number + * @seqno_lo: Pointer to ulower 16 bit part of the sequence number + */ +static void batadv_hton_seqno32(uint32_t seqno, + __be16 *seqno_hi, __be16 *seqno_lo) +{ + *seqno_hi = htons(seqno >> 16); + *seqno_lo = htons(seqno & 0xffff); +} + static int batadv_interface_tx(struct sk_buff *skb, struct net_device *soft_iface) { @@ -252,7 +265,8 @@ static int batadv_interface_tx(struct sk_buff *skb,
/* set broadcast sequence number */ seqno = atomic_inc_return(&bat_priv->bcast_seqno); - bcast_packet->seqno = htonl(seqno); + batadv_hton_seqno32(seqno, &bcast_packet->seqno_hi, + &bcast_packet->seqno_lo);
batadv_add_bcast_packet_to_list(bat_priv, skb, brd_delay);
The packets sent by batman-adv in COMPAT_VERSION 14 were badly aligned and resulted in unnecessary extra cycles for architectures without load/store operations on unaligned memory addresses. This also affects the payload like the IP headers following after the unicast/broadcast headers and the ethernet header.
COMPAT_VERSION 15 tries to fix this by adding extra reserved fields to ensure that either the packet length is either a multiple of 4 bytes long or the header plus the ethernet frame is a multiple of 4 bytes long. Also address fields are now 2 bytes boundary aligned.
Signed-off-by: Sven Eckelmann sven@narfation.org --- Did I told you that I don't intend to test this stuff?
bat_iv_ogm.c | 1 + packet.h | 26 +++++++++++++++++++------- translation-table.c | 6 ++++++ types.h | 5 +++-- unicast.c | 4 ++++ vis.c | 2 ++ 6 files changed, 35 insertions(+), 9 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 5be9a3e..9cc7a81 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -80,6 +80,7 @@ static int batadv_iv_ogm_iface_enable(struct batadv_hard_iface *hard_iface) batadv_ogm_packet->tq = BATADV_TQ_MAX_VALUE; batadv_ogm_packet->tt_num_changes = 0; batadv_ogm_packet->ttvn = 0; + batadv_ogm_packet->reserved = 0;
res = 0;
diff --git a/packet.h b/packet.h index 43c863c..c94ff5e 100644 --- a/packet.h +++ b/packet.h @@ -49,7 +49,7 @@ enum batadv_subtype { };
/* this file is included by batctl which needs these defines */ -#define BATADV_COMPAT_VERSION 14 +#define BATADV_COMPAT_VERSION 15
enum batadv_iv_flags { BATADV_NOT_BEST_NEXT_HOP = BIT(3), @@ -142,7 +142,8 @@ struct batadv_ogm_packet { uint8_t tt_num_changes; uint8_t ttvn; /* translation table version number */ __be16 tt_crc; -} __packed; + __be16 reserved; +};
#define BATADV_OGM_HLEN sizeof(struct batadv_ogm_packet)
@@ -191,7 +192,11 @@ struct batadv_unicast_4addr_packet { struct batadv_unicast_packet u; uint8_t src[ETH_ALEN]; uint8_t subtype; -} __packed; + uint8_t reserved; + /* "4 bytes boundary + 2 bytes" long to make the payload after the + * following ethernet header again 4 bytes boundary aligned + */ +};
struct batadv_unicast_frag_packet { struct batadv_header header; @@ -201,7 +206,11 @@ struct batadv_unicast_frag_packet { uint8_t align; uint8_t orig[ETH_ALEN]; __be16 seqno; -} __packed; + __be16 reserved; + /* "4 bytes boundary + 2 bytes" long to make the payload after the + * following ethernet header again 4 bytes boundary aligned + */ +};
struct batadv_bcast_packet { struct batadv_header header; @@ -241,13 +250,14 @@ struct batadv_tt_query_packet { * orig_node */ uint8_t ttvn; + uint8_t reserved; /* tt_data field is: * if TT_REQUEST: crc associated with the * ttvn * if TT_RESPONSE: table_size */ __be16 tt_data; -} __packed; +};
struct batadv_roam_adv_packet { struct batadv_header header; @@ -255,11 +265,13 @@ struct batadv_roam_adv_packet { uint8_t dst[ETH_ALEN]; uint8_t src[ETH_ALEN]; uint8_t client[ETH_ALEN]; -} __packed; + __be16 reserved2; +};
struct batadv_tt_change { uint8_t flags; + uint8_t reserved; uint8_t addr[ETH_ALEN]; -} __packed; +};
#endif /* _NET_BATMAN_ADV_PACKET_H_ */ diff --git a/translation-table.c b/translation-table.c index bb3941c..668766c 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1601,6 +1601,7 @@ batadv_tt_response_fill_table(uint16_t tt_len, uint8_t ttvn, skb_reserve(skb, ETH_HLEN + NET_IP_ALIGN); tt_response = (struct batadv_tt_query_packet *)skb_put(skb, len); tt_response->ttvn = ttvn; + tt_response->reserved = 0;
tt_change = (struct batadv_tt_change *)(skb->data + tt_query_size); tt_count = 0; @@ -1620,6 +1621,7 @@ batadv_tt_response_fill_table(uint16_t tt_len, uint8_t ttvn, memcpy(tt_change->addr, tt_common_entry->addr, ETH_ALEN); tt_change->flags = BATADV_NO_FLAGS; + tt_change->reserved = 0;
tt_count++; tt_change++; @@ -1676,6 +1678,7 @@ static int batadv_send_tt_request(struct batadv_priv *bat_priv, tt_request->ttvn = ttvn; tt_request->tt_data = htons(tt_crc); tt_request->flags = BATADV_TT_REQUEST; + tt_request->reserved = 0;
if (full_table) tt_request->flags |= BATADV_TT_FULL_TABLE; @@ -1799,6 +1802,7 @@ batadv_send_other_tt_response(struct batadv_priv *bat_priv, memcpy(tt_response->src, req_dst_orig_node->orig, ETH_ALEN); memcpy(tt_response->dst, tt_request->src, ETH_ALEN); tt_response->flags = BATADV_TT_RESPONSE; + tt_response->reserved = 0;
if (full_table) tt_response->flags |= BATADV_TT_FULL_TABLE; @@ -1916,6 +1920,7 @@ batadv_send_my_tt_response(struct batadv_priv *bat_priv, memcpy(tt_response->src, primary_if->net_dev->dev_addr, ETH_ALEN); memcpy(tt_response->dst, tt_request->src, ETH_ALEN); tt_response->flags = BATADV_TT_RESPONSE; + tt_response->reserved = 0;
if (full_table) tt_response->flags |= BATADV_TT_FULL_TABLE; @@ -2223,6 +2228,7 @@ static void batadv_send_roam_adv(struct batadv_priv *bat_priv, uint8_t *client, roam_adv_packet->header.version = BATADV_COMPAT_VERSION; roam_adv_packet->header.ttl = BATADV_TTL; roam_adv_packet->reserved = 0; + roam_adv_packet->reserved2 = 0; primary_if = batadv_primary_if_get_selected(bat_priv); if (!primary_if) goto out; diff --git a/types.h b/types.h index ae9ac9a..4bfb91a 100644 --- a/types.h +++ b/types.h @@ -449,13 +449,14 @@ struct batadv_vis_info { /* this packet might be part of the vis send queue. */ struct sk_buff *skb_packet; /* vis_info may follow here */ -} __packed; +};
struct batadv_vis_info_entry { uint8_t src[ETH_ALEN]; uint8_t dest[ETH_ALEN]; uint8_t quality; /* quality = 0 client */ -} __packed; + uint8_t reserved; +};
struct batadv_recvlist_node { struct list_head list; diff --git a/unicast.c b/unicast.c index 9416136..fbbc4c8 100644 --- a/unicast.c +++ b/unicast.c @@ -276,6 +276,9 @@ int batadv_frag_send_skb(struct sk_buff *skb, struct batadv_priv *bat_priv, frag1->seqno = htons(seqno - 1); frag2->seqno = htons(seqno);
+ frag1->reserved = 0; + frag2->reserved = 0; + batadv_send_skb_packet(skb, hard_iface, dstaddr); batadv_send_skb_packet(frag_skb, hard_iface, dstaddr); ret = NET_RX_SUCCESS; @@ -374,6 +377,7 @@ bool batadv_unicast_4addr_prepare_skb(struct batadv_priv *bat_priv, memcpy(unicast_4addr_packet->src, primary_if->net_dev->dev_addr, ETH_ALEN); unicast_4addr_packet->subtype = packet_subtype; + unicast_4addr_packet->reserved = 0;
ret = true; out: diff --git a/vis.c b/vis.c index b004560..e08a204 100644 --- a/vis.c +++ b/vis.c @@ -626,6 +626,7 @@ static int batadv_generate_vis_packet(struct batadv_priv *bat_priv) ETH_ALEN); memcpy(entry->dest, orig_node->orig, ETH_ALEN); entry->quality = router->tq_avg; + entry->reserved = 0; packet->entries++;
next: @@ -650,6 +651,7 @@ next: memset(entry->src, 0, ETH_ALEN); memcpy(entry->dest, tt_common_entry->addr, ETH_ALEN); entry->quality = 0; /* 0 means TT */ + entry->reserved = 0; packet->entries++;
if (batadv_vis_packet_full(info))
COMPAT_VERSION 15 provides correctly (2 bytes boundary) aligned address fields. This allows the usage of compare_ether_addr instead of the memcmp based batadv_compare_eth.
Signed-off-by: Sven Eckelmann sven@narfation.org --- Btw. this one also builds...
bat_iv_ogm.c | 24 ++++++++++++------------ bridge_loop_avoidance.c | 28 ++++++++++++++-------------- distributed-arp-table.c | 4 ++-- hard-interface.c | 2 +- main.c | 2 +- main.h | 9 --------- originator.h | 2 +- routing.c | 10 +++++----- soft-interface.c | 4 ++-- translation-table.c | 15 +++++++++------ vis.c | 14 +++++++------- 11 files changed, 54 insertions(+), 60 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 9cc7a81..8978a5f 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -670,7 +670,7 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv, hlist_for_each_entry_rcu(tmp_neigh_node, node, &orig_node->neigh_list, list) { neigh_addr = tmp_neigh_node->addr; - if (batadv_compare_eth(neigh_addr, ethhdr->h_source) && + if (compare_ether_addr(neigh_addr, ethhdr->h_source) && tmp_neigh_node->if_incoming == if_incoming && atomic_inc_not_zero(&tmp_neigh_node->refcount)) { if (neigh_node) @@ -815,7 +815,7 @@ static int batadv_iv_ogm_calc_tq(struct batadv_orig_node *orig_node, hlist_for_each_entry_rcu(tmp_neigh_node, node, &orig_neigh_node->neigh_list, list) {
- if (!batadv_compare_eth(tmp_neigh_node->addr, + if (!compare_ether_addr(tmp_neigh_node->addr, orig_neigh_node->orig)) continue;
@@ -954,7 +954,7 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, seqno);
neigh_addr = tmp_neigh_node->addr; - if (batadv_compare_eth(neigh_addr, ethhdr->h_source) && + if (compare_ether_addr(neigh_addr, ethhdr->h_source) && tmp_neigh_node->if_incoming == if_incoming) set_mark = 1; else @@ -1028,7 +1028,7 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr, else has_directlink_flag = 0;
- if (batadv_compare_eth(ethhdr->h_source, batadv_ogm_packet->orig)) + if (compare_ether_addr(ethhdr->h_source, batadv_ogm_packet->orig)) is_single_hop_neigh = true;
batadv_dbg(BATADV_DBG_BATMAN, bat_priv, @@ -1050,15 +1050,15 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr, if (hard_iface->soft_iface != if_incoming->soft_iface) continue;
- if (batadv_compare_eth(ethhdr->h_source, + if (compare_ether_addr(ethhdr->h_source, hard_iface->net_dev->dev_addr)) is_my_addr = 1;
- if (batadv_compare_eth(batadv_ogm_packet->orig, + if (compare_ether_addr(batadv_ogm_packet->orig, hard_iface->net_dev->dev_addr)) is_my_orig = 1;
- if (batadv_compare_eth(batadv_ogm_packet->prev_sender, + if (compare_ether_addr(batadv_ogm_packet->prev_sender, hard_iface->net_dev->dev_addr)) is_my_oldorig = 1;
@@ -1105,7 +1105,7 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr, * save packet seqno for bidirectional check */ if (has_directlink_flag && - batadv_compare_eth(if_incoming->net_dev->dev_addr, + compare_ether_addr(if_incoming->net_dev->dev_addr, batadv_ogm_packet->orig)) { if_num = if_incoming->if_num; offset = if_num * BATADV_NUM_WORDS; @@ -1166,15 +1166,15 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr, router_router = batadv_orig_node_get_router(router->orig_node);
if ((router && router->tq_avg != 0) && - (batadv_compare_eth(router->addr, ethhdr->h_source))) + (compare_ether_addr(router->addr, ethhdr->h_source))) is_from_best_next_hop = true;
prev_sender = batadv_ogm_packet->prev_sender; /* avoid temporary routing loops */ if (router && router_router && - (batadv_compare_eth(router->addr, prev_sender)) && - !(batadv_compare_eth(batadv_ogm_packet->orig, prev_sender)) && - (batadv_compare_eth(router->addr, router_router->addr))) { + (compare_ether_addr(router->addr, prev_sender)) && + !(compare_ether_addr(batadv_ogm_packet->orig, prev_sender)) && + (compare_ether_addr(router->addr, router_router->addr))) { batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "Drop packet: ignoring all rebroadcast packets that may make me loop (sender: %pM)\n", ethhdr->h_source); diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c index b6da6ae..fd34954 100644 --- a/bridge_loop_avoidance.c +++ b/bridge_loop_avoidance.c @@ -692,13 +692,13 @@ static int batadv_handle_request(struct batadv_priv *bat_priv, struct ethhdr *ethhdr, short vid) { /* check for REQUEST frame */ - if (!batadv_compare_eth(backbone_addr, ethhdr->h_dest)) + if (!compare_ether_addr(backbone_addr, ethhdr->h_dest)) return 0;
/* sanity check, this should not happen on a normal switch, * we ignore it in this case. */ - if (!batadv_compare_eth(ethhdr->h_dest, primary_if->net_dev->dev_addr)) + if (!compare_ether_addr(ethhdr->h_dest, primary_if->net_dev->dev_addr)) return 1;
batadv_dbg(BATADV_DBG_BLA, bat_priv, @@ -718,7 +718,7 @@ static int batadv_handle_unclaim(struct batadv_priv *bat_priv, struct batadv_backbone_gw *backbone_gw;
/* unclaim in any case if it is our own */ - if (primary_if && batadv_compare_eth(backbone_addr, + if (primary_if && compare_ether_addr(backbone_addr, primary_if->net_dev->dev_addr)) batadv_bla_send_claim(bat_priv, claim_addr, vid, BATADV_CLAIM_TYPE_UNCLAIM); @@ -756,7 +756,7 @@ static int batadv_handle_claim(struct batadv_priv *bat_priv,
/* this must be a CLAIM frame */ batadv_bla_add_claim(bat_priv, claim_addr, vid, backbone_gw); - if (batadv_compare_eth(backbone_addr, primary_if->net_dev->dev_addr)) + if (compare_ether_addr(backbone_addr, primary_if->net_dev->dev_addr)) batadv_bla_send_claim(bat_priv, claim_addr, vid, BATADV_CLAIM_TYPE_CLAIM);
@@ -816,7 +816,7 @@ static int batadv_check_claim_group(struct batadv_priv *bat_priv, }
/* don't accept claim frames from ourselves */ - if (batadv_compare_eth(backbone_addr, primary_if->net_dev->dev_addr)) + if (compare_ether_addr(backbone_addr, primary_if->net_dev->dev_addr)) return 0;
/* if its already the same group, it is fine. */ @@ -1030,7 +1030,7 @@ static void batadv_bla_purge_claims(struct batadv_priv *bat_priv, hlist_for_each_entry_rcu(claim, node, head, hash_entry) { if (now) goto purge_now; - if (!batadv_compare_eth(claim->backbone_gw->orig, + if (!compare_ether_addr(claim->backbone_gw->orig, primary_if->net_dev->dev_addr)) continue; if (!batadv_has_timed_out(claim->lasttime, @@ -1089,7 +1089,7 @@ void batadv_bla_update_orig_address(struct batadv_priv *bat_priv, rcu_read_lock(); hlist_for_each_entry_rcu(backbone_gw, node, head, hash_entry) { /* own orig still holds the old value. */ - if (!batadv_compare_eth(backbone_gw->orig, + if (!compare_ether_addr(backbone_gw->orig, oldif->net_dev->dev_addr)) continue;
@@ -1152,7 +1152,7 @@ static void batadv_bla_periodic_work(struct work_struct *work)
rcu_read_lock(); hlist_for_each_entry_rcu(backbone_gw, node, head, hash_entry) { - if (!batadv_compare_eth(backbone_gw->orig, + if (!compare_ether_addr(backbone_gw->orig, primary_if->net_dev->dev_addr)) continue;
@@ -1290,7 +1290,7 @@ int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, if (entry->crc != crc) continue;
- if (batadv_compare_eth(entry->orig, bcast_packet->orig)) + if (compare_ether_addr(entry->orig, bcast_packet->orig)) continue;
/* this entry seems to match: same crc, not too old, @@ -1344,7 +1344,7 @@ int batadv_bla_is_backbone_gw_orig(struct batadv_priv *bat_priv, uint8_t *orig)
rcu_read_lock(); hlist_for_each_entry_rcu(backbone_gw, node, head, hash_entry) { - if (batadv_compare_eth(backbone_gw->orig, orig)) { + if (compare_ether_addr(backbone_gw->orig, orig)) { rcu_read_unlock(); return 1; } @@ -1476,7 +1476,7 @@ int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, short vid, }
/* if it is our own claim ... */ - if (batadv_compare_eth(claim->backbone_gw->orig, + if (compare_ether_addr(claim->backbone_gw->orig, primary_if->net_dev->dev_addr)) { /* ... allow it in any case */ claim->lasttime = jiffies; @@ -1570,7 +1570,7 @@ int batadv_bla_tx(struct batadv_priv *bat_priv, struct sk_buff *skb, short vid) goto allow;
/* check if we are responsible. */ - if (batadv_compare_eth(claim->backbone_gw->orig, + if (compare_ether_addr(claim->backbone_gw->orig, primary_if->net_dev->dev_addr)) { /* if yes, the client has roamed and we have * to unclaim it. @@ -1636,7 +1636,7 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset)
rcu_read_lock(); hlist_for_each_entry_rcu(claim, node, head, hash_entry) { - is_own = batadv_compare_eth(claim->backbone_gw->orig, + is_own = compare_ether_addr(claim->backbone_gw->orig, primary_addr); seq_printf(seq, " * %pM on % 5d by %pM [%c] (%04x)\n", claim->addr, claim->vid, @@ -1687,7 +1687,7 @@ int batadv_bla_backbone_table_seq_print_text(struct seq_file *seq, void *offset) secs = msecs / 1000; msecs = msecs % 1000;
- is_own = batadv_compare_eth(backbone_gw->orig, + is_own = compare_ether_addr(backbone_gw->orig, primary_addr); if (is_own) continue; diff --git a/distributed-arp-table.c b/distributed-arp-table.c index 8e1d89d..43b1fa2 100644 --- a/distributed-arp-table.c +++ b/distributed-arp-table.c @@ -277,7 +277,7 @@ static void batadv_dat_entry_add(struct batadv_priv *bat_priv, __be32 ip, dat_entry = batadv_dat_entry_hash_find(bat_priv, ip); /* if this entry is already known, just update it */ if (dat_entry) { - if (!batadv_compare_eth(dat_entry->mac_addr, mac_addr)) + if (!compare_ether_addr(dat_entry->mac_addr, mac_addr)) memcpy(dat_entry->mac_addr, mac_addr, ETH_ALEN); dat_entry->last_update = jiffies; batadv_dbg(BATADV_DBG_DAT, bat_priv, @@ -441,7 +441,7 @@ static bool batadv_is_orig_node_eligible(struct batadv_dat_candidate *res, * the one with the lowest address */ if ((tmp_max == max) && - (batadv_compare_eth(candidate->orig, max_orig_node->orig) > 0)) + (compare_ether_addr(candidate->orig, max_orig_node->orig) > 0)) goto out;
ret = true; diff --git a/hard-interface.c b/hard-interface.c index 365ed74..a4c6f28 100644 --- a/hard-interface.c +++ b/hard-interface.c @@ -209,7 +209,7 @@ static void batadv_check_known_mac_addr(const struct net_device *net_dev) if (hard_iface->net_dev == net_dev) continue;
- if (!batadv_compare_eth(hard_iface->net_dev->dev_addr, + if (!compare_ether_addr(hard_iface->net_dev->dev_addr, net_dev->dev_addr)) continue;
diff --git a/main.c b/main.c index 253e240..003e9f4 100644 --- a/main.c +++ b/main.c @@ -178,7 +178,7 @@ int batadv_is_my_mac(const uint8_t *addr) if (hard_iface->if_status != BATADV_IF_ACTIVE) continue;
- if (batadv_compare_eth(hard_iface->net_dev->dev_addr, addr)) { + if (compare_ether_addr(hard_iface->net_dev->dev_addr, addr)) { rcu_read_unlock(); return 1; } diff --git a/main.h b/main.h index ec9c5ad..5a5ff17 100644 --- a/main.h +++ b/main.h @@ -231,15 +231,6 @@ static inline void batadv_dbg(int type __always_unused, pr_err("%s: " fmt, _netdev->name, ## arg); \ } while (0)
-/* returns 1 if they are the same ethernet addr - * - * note: can't use compare_ether_addr() as it requires aligned memory - */ -static inline int batadv_compare_eth(const void *data1, const void *data2) -{ - return (memcmp(data1, data2, ETH_ALEN) == 0 ? 1 : 0); -} - /** * has_timed_out - compares current time (jiffies) and timestamp + timeout * @timestamp: base value to compare with (in jiffies) diff --git a/originator.h b/originator.h index 9778e65..91f4277 100644 --- a/originator.h +++ b/originator.h @@ -80,7 +80,7 @@ batadv_orig_hash_find(struct batadv_priv *bat_priv, const void *data)
rcu_read_lock(); hlist_for_each_entry_rcu(orig_node, node, head, hash_entry) { - if (!batadv_compare_eth(orig_node, data)) + if (!compare_ether_addr(orig_node->orig, data)) continue;
if (!atomic_inc_not_zero(&orig_node->refcount)) diff --git a/routing.c b/routing.c index f7d50d0..3c0c70b 100644 --- a/routing.c +++ b/routing.c @@ -154,7 +154,7 @@ void batadv_bonding_candidate_add(struct batadv_orig_node *orig_node, spin_lock_bh(&orig_node->neigh_list_lock);
/* only consider if it has the same primary address ... */ - if (!batadv_compare_eth(orig_node->orig, + if (!compare_ether_addr(orig_node->orig, neigh_node->orig_node->primary_addr)) goto candidate_del;
@@ -183,7 +183,7 @@ void batadv_bonding_candidate_add(struct batadv_orig_node *orig_node, continue;
if ((neigh_node->if_incoming == tmp_neigh_node->if_incoming) || - (batadv_compare_eth(neigh_node->addr, + (compare_ether_addr(neigh_node->addr, tmp_neigh_node->addr))) { interference_candidate = 1; break; @@ -738,13 +738,13 @@ batadv_find_router(struct batadv_priv *bat_priv, /* if we have something in the primary_addr, we can search * for a potential bonding candidate. */ - if (batadv_compare_eth(primary_addr, zero_mac)) + if (compare_ether_addr(primary_addr, zero_mac)) goto return_router;
/* find the orig_node which has the primary interface. might * even be the same as our router_orig in many cases */ - if (batadv_compare_eth(primary_addr, router_orig->orig)) { + if (compare_ether_addr(primary_addr, router_orig->orig)) { primary_orig_node = router_orig; } else { primary_orig_node = batadv_orig_hash_find(bat_priv, @@ -897,7 +897,7 @@ batadv_reroute_unicast_packet(struct batadv_priv *bat_priv, if (!orig_node) goto out;
- if (batadv_compare_eth(orig_node->orig, unicast_packet->dest)) + if (compare_ether_addr(orig_node->orig, unicast_packet->dest)) goto out;
/* update the packet header */ diff --git a/soft-interface.c b/soft-interface.c index 7d1b0ce..13310a0 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -201,10 +201,10 @@ static int batadv_interface_tx(struct sk_buff *skb, * The same goes for ECTP sent at least by some Cisco Switches, * it might confuse the mesh when used with bridge loop avoidance. */ - if (batadv_compare_eth(ethhdr->h_dest, stp_addr)) + if (compare_ether_addr(ethhdr->h_dest, stp_addr)) goto dropped;
- if (batadv_compare_eth(ethhdr->h_dest, ectp_addr)) + if (compare_ether_addr(ethhdr->h_dest, ectp_addr)) goto dropped;
if (is_multicast_ether_addr(ethhdr->h_dest)) { diff --git a/translation-table.c b/translation-table.c index 668766c..eac3102 100644 --- a/translation-table.c +++ b/translation-table.c @@ -72,7 +72,7 @@ batadv_tt_hash_find(struct batadv_hashtable *hash, const void *data)
rcu_read_lock(); hlist_for_each_entry_rcu(tt_common_entry, node, head, hash_entry) { - if (!batadv_compare_eth(tt_common_entry, data)) + if (!compare_ether_addr(tt_common_entry->addr, data)) continue;
if (!atomic_inc_not_zero(&tt_common_entry->refcount)) @@ -184,7 +184,7 @@ static void batadv_tt_local_event(struct batadv_priv *bat_priv, spin_lock_bh(&bat_priv->tt.changes_list_lock); list_for_each_entry_safe(entry, safe, &bat_priv->tt.changes_list, list) { - if (!batadv_compare_eth(entry->change.addr, addr)) + if (!compare_ether_addr(entry->change.addr, addr)) continue;
/* DEL+ADD in the same orig interval have no effect and can be @@ -314,7 +314,7 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr, tt_local->common.added_at = tt_local->last_seen;
/* the batman interface mac address should never be purged */ - if (batadv_compare_eth(addr, soft_iface->dev_addr)) + if (compare_ether_addr(addr, soft_iface->dev_addr)) tt_local->common.flags |= BATADV_TT_CLIENT_NOPURGE;
/* The local entry has to be marked as NEW to avoid to send it in @@ -1518,10 +1518,13 @@ batadv_new_tt_req_node(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node) { struct batadv_tt_req_node *tt_req_node_tmp, *tt_req_node = NULL; + unsigned same_addr;
spin_lock_bh(&bat_priv->tt.req_list_lock); list_for_each_entry(tt_req_node_tmp, &bat_priv->tt.req_list, list) { - if (batadv_compare_eth(tt_req_node_tmp, orig_node) && + same_addr = compare_ether_addr(tt_req_node_tmp->addr, + orig_node->orig); + if (same_addr && !batadv_has_timed_out(tt_req_node_tmp->issued_at, BATADV_TT_REQUEST_TIMEOUT)) goto unlock; @@ -2090,7 +2093,7 @@ void batadv_handle_tt_response(struct batadv_priv *bat_priv, /* Delete the tt_req_node from pending tt_requests list */ spin_lock_bh(&bat_priv->tt.req_list_lock); list_for_each_entry_safe(node, safe, &bat_priv->tt.req_list, list) { - if (!batadv_compare_eth(node->addr, tt_response->src)) + if (!compare_ether_addr(node->addr, tt_response->src)) continue; list_del(&node->list); kfree(node); @@ -2168,7 +2171,7 @@ static bool batadv_tt_check_roam_count(struct batadv_priv *bat_priv, * reply from the same orig_node yet */ list_for_each_entry(tt_roam_node, &bat_priv->tt.roam_list, list) { - if (!batadv_compare_eth(tt_roam_node->addr, client)) + if (!compare_ether_addr(tt_roam_node->addr, client)) continue;
if (batadv_has_timed_out(tt_roam_node->first_time, diff --git a/vis.c b/vis.c index e08a204..b5dceb2 100644 --- a/vis.c +++ b/vis.c @@ -62,7 +62,7 @@ static int batadv_vis_info_cmp(const struct hlist_node *node, const void *data2) d2 = data2; p1 = (struct batadv_vis_packet *)d1->skb_packet->data; p2 = (struct batadv_vis_packet *)d2->skb_packet->data; - return batadv_compare_eth(p1->vis_orig, p2->vis_orig); + return compare_ether_addr(p1->vis_orig, p2->vis_orig); }
/* hash function to choose an entry in a hash table of given size @@ -130,7 +130,7 @@ static void batadv_vis_data_insert_interface(const uint8_t *interface, struct hlist_node *pos;
hlist_for_each_entry(entry, pos, if_list, list) { - if (batadv_compare_eth(entry->addr, interface)) + if (compare_ether_addr(entry->addr, interface)) return; }
@@ -165,7 +165,7 @@ batadv_vis_data_read_entry(struct seq_file *seq, { if (primary && entry->quality == 0) return seq_printf(seq, "TT %pM, ", entry->dest); - else if (batadv_compare_eth(entry->src, src)) + else if (compare_ether_addr(entry->src, src)) return seq_printf(seq, "TQ %pM %d, ", entry->dest, entry->quality);
@@ -183,7 +183,7 @@ batadv_vis_data_insert_interfaces(struct hlist_head *list, if (entries[i].quality == 0) continue;
- if (batadv_compare_eth(entries[i].src, packet->vis_orig)) + if (compare_ether_addr(entries[i].src, packet->vis_orig)) continue;
batadv_vis_data_insert_interface(entries[i].src, list, false); @@ -207,7 +207,7 @@ static void batadv_vis_data_read_entries(struct seq_file *seq, entry->addr, entry->primary);
/* add primary/secondary records */ - if (batadv_compare_eth(entry->addr, packet->vis_orig)) + if (compare_ether_addr(entry->addr, packet->vis_orig)) batadv_vis_data_read_prim_sec(seq, list);
seq_printf(seq, "\n"); @@ -325,7 +325,7 @@ static int batadv_recv_list_is_in(struct batadv_priv *bat_priv,
spin_lock_bh(&bat_priv->vis.list_lock); list_for_each_entry(entry, recv_list, list) { - if (batadv_compare_eth(entry->mac, mac)) { + if (compare_ether_addr(entry->mac, mac)) { spin_unlock_bh(&bat_priv->vis.list_lock); return 1; } @@ -609,7 +609,7 @@ static int batadv_generate_vis_packet(struct batadv_priv *bat_priv) if (!router) continue;
- if (!batadv_compare_eth(router->addr, orig_node->orig)) + if (!compare_ether_addr(router->addr, orig_node->orig)) goto next;
if (router->if_incoming->if_status != BATADV_IF_ACTIVE)
The ethernet header is 14 bytes long. Therefore, the data after it is not 4 byte aligned and may cause problems on systems without unaligned data access. Reserving NET_IP_ALIGN more byes can fix the misalignment of the ethernet header.
Signed-off-by: Sven Eckelmann sven@narfation.org --- Oops, the allocation of the skb also has to be changed
bat_iv_ogm.c | 8 +++++--- icmp_socket.c | 4 ++-- translation-table.c | 20 ++++++++++---------- vis.c | 9 +++++---- 4 files changed, 22 insertions(+), 19 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 75403a4..9f3925a 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -411,9 +411,11 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff,
if ((atomic_read(&bat_priv->aggregated_ogms)) && (packet_len < BATADV_MAX_AGGREGATION_BYTES)) - skb_size = BATADV_MAX_AGGREGATION_BYTES + ETH_HLEN; + skb_size = BATADV_MAX_AGGREGATION_BYTES; else - skb_size = packet_len + ETH_HLEN; + skb_size = packet_len; + + skb_size += ETH_HLEN + NET_IP_ALIGN;
forw_packet_aggr->skb = dev_alloc_skb(skb_size); if (!forw_packet_aggr->skb) { @@ -422,7 +424,7 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff, kfree(forw_packet_aggr); goto out; } - skb_reserve(forw_packet_aggr->skb, ETH_HLEN); + skb_reserve(forw_packet_aggr->skb, ETH_HLEN + NET_IP_ALIGN);
INIT_HLIST_NODE(&forw_packet_aggr->list);
diff --git a/icmp_socket.c b/icmp_socket.c index 5874c0e..87ca809 100644 --- a/icmp_socket.c +++ b/icmp_socket.c @@ -177,13 +177,13 @@ static ssize_t batadv_socket_write(struct file *file, const char __user *buff, if (len >= sizeof(struct batadv_icmp_packet_rr)) packet_len = sizeof(struct batadv_icmp_packet_rr);
- skb = dev_alloc_skb(packet_len + ETH_HLEN); + skb = dev_alloc_skb(packet_len + ETH_HLEN + NET_IP_ALIGN); if (!skb) { len = -ENOMEM; goto out; }
- skb_reserve(skb, ETH_HLEN); + skb_reserve(skb, ETH_HLEN + NET_IP_ALIGN); icmp_packet = (struct batadv_icmp_packet_rr *)skb_put(skb, packet_len);
if (copy_from_user(icmp_packet, buff, packet_len)) { diff --git a/translation-table.c b/translation-table.c index 6fecb94..c731050 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1594,11 +1594,11 @@ batadv_tt_response_fill_table(uint16_t tt_len, uint8_t ttvn, tt_tot = tt_len / sizeof(struct batadv_tt_change);
len = tt_query_size + tt_len; - skb = dev_alloc_skb(len + ETH_HLEN); + skb = dev_alloc_skb(len + ETH_HLEN + NET_IP_ALIGN); if (!skb) goto out;
- skb_reserve(skb, ETH_HLEN); + skb_reserve(skb, ETH_HLEN + NET_IP_ALIGN); tt_response = (struct batadv_tt_query_packet *)skb_put(skb, len); tt_response->ttvn = ttvn;
@@ -1659,11 +1659,11 @@ static int batadv_send_tt_request(struct batadv_priv *bat_priv, if (!tt_req_node) goto out;
- skb = dev_alloc_skb(sizeof(*tt_request) + ETH_HLEN); + skb = dev_alloc_skb(sizeof(*tt_request) + ETH_HLEN + NET_IP_ALIGN); if (!skb) goto out;
- skb_reserve(skb, ETH_HLEN); + skb_reserve(skb, ETH_HLEN + NET_IP_ALIGN);
tt_req_len = sizeof(*tt_request); tt_request = (struct batadv_tt_query_packet *)skb_put(skb, tt_req_len); @@ -1761,11 +1761,11 @@ batadv_send_other_tt_response(struct batadv_priv *bat_priv, tt_tot = tt_len / sizeof(struct batadv_tt_change);
len = sizeof(*tt_response) + tt_len; - skb = dev_alloc_skb(len + ETH_HLEN); + skb = dev_alloc_skb(len + ETH_HLEN + NET_IP_ALIGN); if (!skb) goto unlock;
- skb_reserve(skb, ETH_HLEN); + skb_reserve(skb, ETH_HLEN + NET_IP_ALIGN); packet_pos = skb_put(skb, len); tt_response = (struct batadv_tt_query_packet *)packet_pos; tt_response->ttvn = req_ttvn; @@ -1880,11 +1880,11 @@ batadv_send_my_tt_response(struct batadv_priv *bat_priv, tt_tot = tt_len / sizeof(struct batadv_tt_change);
len = sizeof(*tt_response) + tt_len; - skb = dev_alloc_skb(len + ETH_HLEN); + skb = dev_alloc_skb(len + ETH_HLEN + NET_IP_ALIGN); if (!skb) goto unlock;
- skb_reserve(skb, ETH_HLEN); + skb_reserve(skb, ETH_HLEN + NET_IP_ALIGN); packet_pos = skb_put(skb, len); tt_response = (struct batadv_tt_query_packet *)packet_pos; tt_response->ttvn = req_ttvn; @@ -2211,11 +2211,11 @@ static void batadv_send_roam_adv(struct batadv_priv *bat_priv, uint8_t *client, if (!batadv_tt_check_roam_count(bat_priv, client)) goto out;
- skb = dev_alloc_skb(sizeof(*roam_adv_packet) + ETH_HLEN); + skb = dev_alloc_skb(sizeof(*roam_adv_packet) + ETH_HLEN + NET_IP_ALIGN); if (!skb) goto out;
- skb_reserve(skb, ETH_HLEN); + skb_reserve(skb, ETH_HLEN + NET_IP_ALIGN);
roam_adv_packet = (struct batadv_roam_adv_packet *)skb_put(skb, len);
diff --git a/vis.c b/vis.c index 79589a3..0f65a9d 100644 --- a/vis.c +++ b/vis.c @@ -396,12 +396,12 @@ batadv_add_packet(struct batadv_priv *bat_priv, return NULL;
len = sizeof(*packet) + vis_info_len; - info->skb_packet = dev_alloc_skb(len + ETH_HLEN); + info->skb_packet = dev_alloc_skb(len + ETH_HLEN + NET_IP_ALIGN); if (!info->skb_packet) { kfree(info); return NULL; } - skb_reserve(info->skb_packet, ETH_HLEN); + skb_reserve(info->skb_packet, ETH_HLEN + NET_IP_ALIGN); packet = (struct batadv_vis_packet *)skb_put(info->skb_packet, len);
kref_init(&info->refcount); @@ -856,12 +856,13 @@ int batadv_vis_init(struct batadv_priv *bat_priv) if (!bat_priv->vis.my_info) goto err;
- len = sizeof(*packet) + BATADV_MAX_VIS_PACKET_SIZE + ETH_HLEN; + len = sizeof(*packet) + BATADV_MAX_VIS_PACKET_SIZE; + len += ETH_HLEN + NET_IP_ALIGN; bat_priv->vis.my_info->skb_packet = dev_alloc_skb(len); if (!bat_priv->vis.my_info->skb_packet) goto free_info;
- skb_reserve(bat_priv->vis.my_info->skb_packet, ETH_HLEN); + skb_reserve(bat_priv->vis.my_info->skb_packet, ETH_HLEN + NET_IP_ALIGN); tmp_skb = bat_priv->vis.my_info->skb_packet; packet = (struct batadv_vis_packet *)skb_put(tmp_skb, sizeof(*packet));
batadv_unicast_4addr_packet created an odd aligned ethernet header and a not 4 bytes boundary aligned IP header. Adding an extra reserved bytes avoids this problem.
This problem was introduced in 78fc6bbe0aca868b65b92723b1e259e7ef7b35c0 ("batman-adv: add UNICAST_4ADDR packet type")
Signed-off-by: Sven Eckelmann sven@narfation.org --- This is an alternative version of the third patch.
DONT APPLY PATCH 4 WHEN NOT APPLYING THE INITIAL VERSION OF THE THIRD PATCH
packet.h | 6 +++++- unicast.c | 1 + 2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/packet.h b/packet.h index 43c863c..0a9886c 100644 --- a/packet.h +++ b/packet.h @@ -191,7 +191,11 @@ struct batadv_unicast_4addr_packet { struct batadv_unicast_packet u; uint8_t src[ETH_ALEN]; uint8_t subtype; -} __packed; + uint8_t reserved; + /* "4 bytes boundary + 2 bytes" long to make the payload after the + * following ethernet header again 4 bytes boundary aligned + */ +};
struct batadv_unicast_frag_packet { struct batadv_header header; diff --git a/unicast.c b/unicast.c index 9416136..10aff49 100644 --- a/unicast.c +++ b/unicast.c @@ -374,6 +374,7 @@ bool batadv_unicast_4addr_prepare_skb(struct batadv_priv *bat_priv, memcpy(unicast_4addr_packet->src, primary_if->net_dev->dev_addr, ETH_ALEN); unicast_4addr_packet->subtype = packet_subtype; + unicast_4addr_packet->reserved = 0;
ret = true; out:
Headers which are already perfectly aligned and create a 4 byte boundary non-ethernet header payload can have the __packed attribute removed. The __packed attribute doesn't change the appeareance of the packet for these headers because no extra padding is necessary to align the data members. The compiler will also create slightly faster code for loads of multi-byte members.
Signed-off-by: Sven Eckelmann sven@narfation.org --- Because we had protests against the approach of splitting the 32 bit seqno... here is a different approach. Now get your dices to find out what David may will like.
packet.h | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/packet.h b/packet.h index aa3e63a..28699f1 100644 --- a/packet.h +++ b/packet.h @@ -121,13 +121,15 @@ struct batadv_bla_claim_dst { uint8_t magic[3]; /* FF:43:05 */ uint8_t type; /* bla_claimframe */ __be16 group; /* group id */ -} __packed; +};
struct batadv_header { uint8_t packet_type; uint8_t version; /* batman version field */ uint8_t ttl; -} __packed; + /* the parent struct has to add a byte after the header to make + * everything 4 bytes aligned again */ +};
struct batadv_ogm_packet { struct batadv_header header; @@ -152,7 +154,7 @@ struct batadv_icmp_packet { __be16 seqno; uint8_t uid; uint8_t reserved; -} __packed; +};
#define BATADV_RR_LEN 16
@@ -168,13 +170,16 @@ struct batadv_icmp_packet_rr { uint8_t uid; uint8_t rr_cur; uint8_t rr[BATADV_RR_LEN][ETH_ALEN]; -} __packed; +};
struct batadv_unicast_packet { struct batadv_header header; uint8_t ttvn; /* destination translation table version number */ uint8_t dest[ETH_ALEN]; -} __packed; + /* "4 bytes boundary + 2 bytes" long to make the payload after the + * following ethernet header again 4 bytes boundary aligned + */ +};
/** * struct batadv_unicast_4addr_packet - extended unicast packet @@ -203,6 +208,9 @@ struct batadv_bcast_packet { uint8_t reserved; __be32 seqno; uint8_t orig[ETH_ALEN]; + /* "4 bytes boundary + 2 bytes" long to make the payload after the + * following ethernet header again 4 bytes boundary aligned + */ } __packed;
struct batadv_vis_packet { @@ -214,7 +222,7 @@ struct batadv_vis_packet { uint8_t vis_orig[ETH_ALEN]; /* originator reporting its neighbors */ uint8_t target_orig[ETH_ALEN]; /* who should receive this packet */ uint8_t sender_orig[ETH_ALEN]; /* who sent or forwarded this packet */ -} __packed; +};
struct batadv_tt_query_packet { struct batadv_header header;
All packet headers in front of an ethernet header have to be completely divisible by 2 but not by 4 to make the payload after the ethernet header again 4 bytes boundary aligned.
A packing of 2 is necessary to avoid extra padding at the end of the struct caused by a structure member which is larger than two bytes. Otherwise the structure would not fulfill the previously mentioned rule to avoid the misalignment of the payload after the ethernet header. It may also lead to leakage of information when the padding it not initialized before sending.
Signed-off-by: Sven Eckelmann sven@narfation.org --- Because we had protests against the approach of splitting the 32 bit seqno... here is a different approach. Now get your dices to find out what David may will like.
packet.h | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/packet.h b/packet.h index 28699f1..a211916 100644 --- a/packet.h +++ b/packet.h @@ -172,6 +172,18 @@ struct batadv_icmp_packet_rr { uint8_t rr[BATADV_RR_LEN][ETH_ALEN]; };
+/* All packet headers in front of an ethernet header have to be completely + * divisible by 2 but not by 4 to make the payload after the ethernet + * header again 4 bytes boundary aligned. + * + * A packing of 2 is necessary to avoid extra padding at the end of the struct + * caused by a structure member which is larger than two bytes. Otherwise + * the structure would not fulfill the previously mentioned rule to avoid the + * misalignment of the payload after the ethernet header. It may also lead to + * leakage of information when the padding it not initialized before sending. + */ +#pragma pack(2) + struct batadv_unicast_packet { struct batadv_header header; uint8_t ttvn; /* destination translation table version number */ @@ -211,7 +223,9 @@ struct batadv_bcast_packet { /* "4 bytes boundary + 2 bytes" long to make the payload after the * following ethernet header again 4 bytes boundary aligned */ -} __packed; +}; + +#pragma pack()
struct batadv_vis_packet { struct batadv_header header;
On Monday, November 05, 2012 00:11:45 Sven Eckelmann wrote:
The ethernet header is 14 bytes long. Therefore, the data after it is not 4 byte aligned and may cause problems on systems without unaligned data access. Reserving NET_IP_ALIGN more byes can fix the misalignment of the ethernet header.
Signed-off-by: Sven Eckelmann sven@narfation.org
Oops, the allocation of the skb also has to be changed
bat_iv_ogm.c | 8 +++++--- icmp_socket.c | 4 ++-- translation-table.c | 20 ++++++++++---------- vis.c | 9 +++++---- 4 files changed, 22 insertions(+), 19 deletions(-)
Applied in revision 7f91790.
Thanks, Marek
On Tuesday, November 06, 2012 04:25:26 Sven Eckelmann wrote:
Headers which are already perfectly aligned and create a 4 byte boundary non-ethernet header payload can have the __packed attribute removed. The __packed attribute doesn't change the appeareance of the packet for these headers because no extra padding is necessary to align the data members. The compiler will also create slightly faster code for loads of multi-byte members.
Signed-off-by: Sven Eckelmann sven@narfation.org
Because we had protests against the approach of splitting the 32 bit seqno... here is a different approach. Now get your dices to find out what David may will like.
packet.h | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
Applied in revision e754ea0.
Thanks, Marek
On Tuesday, November 06, 2012 04:25:27 Sven Eckelmann wrote:
All packet headers in front of an ethernet header have to be completely divisible by 2 but not by 4 to make the payload after the ethernet header again 4 bytes boundary aligned.
A packing of 2 is necessary to avoid extra padding at the end of the struct caused by a structure member which is larger than two bytes. Otherwise the structure would not fulfill the previously mentioned rule to avoid the misalignment of the payload after the ethernet header. It may also lead to leakage of information when the padding it not initialized before sending.
Signed-off-by: Sven Eckelmann sven@narfation.org
Because we had protests against the approach of splitting the 32 bit seqno... here is a different approach. Now get your dices to find out what David may will like.
packet.h | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
Applied in revision 52f79c4.
Thanks, Marek
On Monday, November 05, 2012 17:15:24 Sven Eckelmann wrote:
batadv_unicast_4addr_packet created an odd aligned ethernet header and a not 4 bytes boundary aligned IP header. Adding an extra reserved bytes avoids this problem.
This problem was introduced in 78fc6bbe0aca868b65b92723b1e259e7ef7b35c0 ("batman-adv: add UNICAST_4ADDR packet type")
Signed-off-by: Sven Eckelmann sven@narfation.org
This is an alternative version of the third patch.
DONT APPLY PATCH 4 WHEN NOT APPLYING THE INITIAL VERSION OF THE THIRD PATCH
packet.h | 6 +++++- unicast.c | 1 + 2 files changed, 6 insertions(+), 1 deletion(-)
Applied in revision cc9fb91.
Thanks, Marek
b.a.t.m.a.n@lists.open-mesh.org
-
Marek Lindner -
Sven Eckelmann