Hi,
This is the fifth itereation to increase the DAT DHT timeout to reduce the amount of broadcasted ARP Replies.
To increase the timeout only for DAT DHT entries added via DHT-PUT but not for any other entry in the DAT cache the DAT cache and DAT DHT concepts are first split into two separate hash tables (PATCH 1/2).
PATCH 2/2 then increases the timeout for DAT DHT entries from 5 to 30 minutes.
The motivation for this patchset is based on the observations made here: https://www.open-mesh.org/projects/batman-adv/wiki/DAT_DHCP_Snooping
In tests this year at Freifunk Lübeck with ~180 mesh nodes and Gluon this reduced the ARP broadcast overhead, measured over 7 days, as follows:
- Total: 6677.66 bits/s -> 677.26 bits/s => -89.86% 11.92 pkts/s -> 1.21 pkts/s => -89.85%
- from gateways: 5618.02 bits/s -> 212.28 => -96.22% 10.03 pkts/s -> 0.38 pkts/s => -96.21%
Also see graphics and a few more test details here: - https://www.open-mesh.org/projects/batman-adv/wiki/DAT_DHCP_Snooping#Result-...
These patches have been applied in this mesh network without issues for 3 months now.
Regards, Linus
---
Changelog v5: - rebased to current main branch -> removed now obsolete debugfs code
Changelog v4: - rebased to: acfc9a214d01695 ("batman-adv: genetlink: make policy common to family")
Changelog v3:
formerly: "batman-adv: Increase purge timeout on DAT DHT candidates" https://patchwork.open-mesh.org/patch/17728/ - fixed the potential jiffies overflow and jiffies initialization issues by replacing the last_dht_update timeout variable with a split of DAT cache and DAT DHT into two separate hash tables -> instead of maintaining two timeouts in one DAT entry two DAT entries are created and maintained in their respective DAT cache and DAT DHT hash tables
Changelog v2:
formerly: "batman-adv: Increase DHCP snooped DAT entry purge timeout in DHT" (https://patchwork.open-mesh.org/patch/17364/) - removed the extended timeouts flag in the DHT-PUT messages introduced in v1 again - removed DHCP dependency
Similar to the translation table split the (mostly) locally maintained DAT cache and foreignly maintained DAT DHT concepts into two separate tables.
This eases the monitoring and debugging regarding the origin of DAT entries. And allows to apply differing properties to entries in the DAT cache and DAT DHT in the future.
Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue --- include/uapi/linux/batadv_packet.h | 4 +- include/uapi/linux/batman_adv.h | 5 + net/batman-adv/distributed-arp-table.c | 360 ++++++++++++++++++------- net/batman-adv/distributed-arp-table.h | 13 +- net/batman-adv/netlink.c | 5 + net/batman-adv/routing.c | 8 +- net/batman-adv/types.h | 7 +- 7 files changed, 304 insertions(+), 98 deletions(-)
diff --git a/include/uapi/linux/batadv_packet.h b/include/uapi/linux/batadv_packet.h index 6e25753015df..0fc631593f49 100644 --- a/include/uapi/linux/batadv_packet.h +++ b/include/uapi/linux/batadv_packet.h @@ -60,13 +60,15 @@ enum batadv_packettype { * @BATADV_P_DATA: user payload * @BATADV_P_DAT_DHT_GET: DHT request message * @BATADV_P_DAT_DHT_PUT: DHT store message - * @BATADV_P_DAT_CACHE_REPLY: ARP reply generated by DAT + * @BATADV_P_DAT_CACHE_REPLY: deprecated: use BATADV_P_DAT_DHT_REPLY instead + * @BATADV_P_DAT_DHT_REPLY: ARP reply generated by DAT */ enum batadv_subtype { BATADV_P_DATA = 0x01, BATADV_P_DAT_DHT_GET = 0x02, BATADV_P_DAT_DHT_PUT = 0x03, BATADV_P_DAT_CACHE_REPLY = 0x04, + BATADV_P_DAT_DHT_REPLY = 0x04, };
/* this file is included by batctl which needs these defines */ diff --git a/include/uapi/linux/batman_adv.h b/include/uapi/linux/batman_adv.h index 35dc016c9bb4..9498ccb09d67 100644 --- a/include/uapi/linux/batman_adv.h +++ b/include/uapi/linux/batman_adv.h @@ -613,6 +613,11 @@ enum batadv_nl_commands { */ BATADV_CMD_SET_VLAN,
+ /** + * @BATADV_CMD_GET_DAT_DHT: Query list of DAT DHT entries + */ + BATADV_CMD_GET_DAT_DHT, + /* add new commands above here */
/** diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c index 4c7e85534324..8a054b0cf2a2 100644 --- a/net/batman-adv/distributed-arp-table.c +++ b/net/batman-adv/distributed-arp-table.c @@ -145,17 +145,17 @@ static bool batadv_dat_to_purge(struct batadv_dat_entry *dat_entry) }
/** - * __batadv_dat_purge() - delete entries from the DAT local storage - * @bat_priv: the bat priv with all the soft interface information + * __batadv_dat_purge() - delete entries from a DAT hash table + * @hash: the hash table to delete from * @to_purge: function in charge to decide whether an entry has to be purged or * not. This function takes the dat_entry as argument and has to * returns a boolean value: true is the entry has to be deleted, * false otherwise * - * Loops over each entry in the DAT local storage and deletes it if and only if + * Loops over each entry in a DAT hash table and deletes it if and only if * the to_purge function passed as argument returns true. */ -static void __batadv_dat_purge(struct batadv_priv *bat_priv, +static void __batadv_dat_purge(struct batadv_hashtable *hash, bool (*to_purge)(struct batadv_dat_entry *)) { spinlock_t *list_lock; /* protects write access to the hash lists */ @@ -164,12 +164,12 @@ static void __batadv_dat_purge(struct batadv_priv *bat_priv, struct hlist_head *head; u32 i;
- if (!bat_priv->dat.hash) + if (!hash) return;
- for (i = 0; i < bat_priv->dat.hash->size; i++) { - head = &bat_priv->dat.hash->table[i]; - list_lock = &bat_priv->dat.hash->list_locks[i]; + for (i = 0; i < hash->size; i++) { + head = &hash->table[i]; + list_lock = &hash->list_locks[i];
spin_lock_bh(list_lock); hlist_for_each_entry_safe(dat_entry, node_tmp, head, @@ -202,7 +202,8 @@ static void batadv_dat_purge(struct work_struct *work) priv_dat = container_of(delayed_work, struct batadv_priv_dat, work); bat_priv = container_of(priv_dat, struct batadv_priv, dat);
- __batadv_dat_purge(bat_priv, batadv_dat_to_purge); + __batadv_dat_purge(bat_priv->dat.cache_hash, batadv_dat_to_purge); + __batadv_dat_purge(bat_priv->dat.dht_hash, batadv_dat_to_purge); batadv_dat_start_timer(bat_priv); }
@@ -314,21 +315,19 @@ static u32 batadv_hash_dat(const void *data, u32 size) }
/** - * batadv_dat_entry_hash_find() - look for a given dat_entry in the local hash - * table - * @bat_priv: the bat priv with all the soft interface information + * batadv_dat_entry_hash_find() - look for a given dat_entry in a hash table + * @hash: the hash table to search in * @ip: search key * @vid: VLAN identifier * * Return: the dat_entry if found, NULL otherwise. */ static struct batadv_dat_entry * -batadv_dat_entry_hash_find(struct batadv_priv *bat_priv, __be32 ip, +batadv_dat_entry_hash_find(struct batadv_hashtable *hash, __be32 ip, unsigned short vid) { struct hlist_head *head; struct batadv_dat_entry to_find, *dat_entry, *dat_entry_tmp = NULL; - struct batadv_hashtable *hash = bat_priv->dat.hash; u32 index;
if (!hash) @@ -356,28 +355,63 @@ batadv_dat_entry_hash_find(struct batadv_priv *bat_priv, __be32 ip, return dat_entry_tmp; }
+/** + * batadv_dat_cache_entry_find() - look for a given dat_entry in the DAT cache + * @bat_priv: the bat priv with all the soft interface information + * @ip: search key + * @vid: VLAN identifier + * + * Return: the dat_entry if found, NULL otherwise. + */ +static inline struct batadv_dat_entry * +batadv_dat_cache_entry_find(struct batadv_priv *bat_priv, __be32 ip, + unsigned short vid) +{ + return batadv_dat_entry_hash_find(bat_priv->dat.cache_hash, ip, vid); +} + +/** + * batadv_dat_dht_entry_find() - look for a given dat_entry in the DAT DHT + * @bat_priv: the bat priv with all the soft interface information + * @ip: search key + * @vid: VLAN identifier + * + * Return: the dat_entry if found, NULL otherwise. + */ +static inline struct batadv_dat_entry * +batadv_dat_dht_entry_find(struct batadv_priv *bat_priv, __be32 ip, + unsigned short vid) +{ + return batadv_dat_entry_hash_find(bat_priv->dat.dht_hash, ip, vid); +} + /** * batadv_dat_entry_add() - add a new dat entry or update it if already exists * @bat_priv: the bat priv with all the soft interface information + * @hash: the hash table to add to * @ip: ipv4 to add/edit * @mac_addr: mac address to assign to the given ipv4 * @vid: VLAN identifier + * @tablename: the name of the hash table to add to */ -static void batadv_dat_entry_add(struct batadv_priv *bat_priv, __be32 ip, - u8 *mac_addr, unsigned short vid) +static void +batadv_dat_entry_add(struct batadv_priv *bat_priv, + struct batadv_hashtable *hash, __be32 ip, + u8 *mac_addr, unsigned short vid, + const char *tablename) { struct batadv_dat_entry *dat_entry; int hash_added;
- dat_entry = batadv_dat_entry_hash_find(bat_priv, ip, vid); + dat_entry = batadv_dat_entry_hash_find(hash, ip, vid); /* if this entry is already known, just update it */ if (dat_entry) { if (!batadv_compare_eth(dat_entry->mac_addr, mac_addr)) ether_addr_copy(dat_entry->mac_addr, mac_addr); dat_entry->last_update = jiffies; batadv_dbg(BATADV_DBG_DAT, bat_priv, - "Entry updated: %pI4 %pM (vid: %d)\n", - &dat_entry->ip, dat_entry->mac_addr, + "Entry updated in %s: %pI4 %pM (vid: %d)\n", + tablename, &dat_entry->ip, dat_entry->mac_addr, batadv_print_vid(vid)); goto out; } @@ -393,7 +427,7 @@ static void batadv_dat_entry_add(struct batadv_priv *bat_priv, __be32 ip, kref_init(&dat_entry->refcount);
kref_get(&dat_entry->refcount); - hash_added = batadv_hash_add(bat_priv->dat.hash, batadv_compare_dat, + hash_added = batadv_hash_add(hash, batadv_compare_dat, batadv_hash_dat, dat_entry, &dat_entry->hash_entry);
@@ -403,13 +437,45 @@ static void batadv_dat_entry_add(struct batadv_priv *bat_priv, __be32 ip, goto out; }
- batadv_dbg(BATADV_DBG_DAT, bat_priv, "New entry added: %pI4 %pM (vid: %d)\n", - &dat_entry->ip, dat_entry->mac_addr, batadv_print_vid(vid)); + batadv_dbg(BATADV_DBG_DAT, bat_priv, + "New entry added in %s: %pI4 %pM (vid: %d)\n", + tablename, &dat_entry->ip, dat_entry->mac_addr, + batadv_print_vid(vid));
out: batadv_dat_entry_put(dat_entry); }
+/** + * batadv_dat_cache_entry_add() - add or update dat_entry in the DAT cache + * @bat_priv: the bat priv with all the soft interface information + * @ip: ipv4 to add/edit + * @mac_addr: mac address to assign to the given ipv4 + * @vid: VLAN identifier + */ +static inline void +batadv_dat_cache_entry_add(struct batadv_priv *bat_priv, __be32 ip, + u8 *mac_addr, unsigned short vid) +{ + batadv_dat_entry_add(bat_priv, bat_priv->dat.cache_hash, ip, mac_addr, + vid, "cache"); +} + +/** + * batadv_dat_dht_entry_add() - add or update dat_entry in the DAT DHT + * @bat_priv: the bat priv with all the soft interface information + * @ip: ipv4 to add/edit + * @mac_addr: mac address to assign to the given ipv4 + * @vid: VLAN identifier + */ +static inline void +batadv_dat_dht_entry_add(struct batadv_priv *bat_priv, __be32 ip, + u8 *mac_addr, unsigned short vid) +{ + batadv_dat_entry_add(bat_priv, bat_priv->dat.dht_hash, ip, mac_addr, + vid, "dht"); +} + #ifdef CONFIG_BATMAN_ADV_DEBUG
/** @@ -459,9 +525,9 @@ static void batadv_dbg_arp(struct batadv_priv *bat_priv, struct sk_buff *skb, case BATADV_P_DAT_DHT_GET: batadv_dbg(BATADV_DBG_DAT, bat_priv, "* type: DAT_DHT_GET\n"); break; - case BATADV_P_DAT_CACHE_REPLY: + case BATADV_P_DAT_DHT_REPLY: batadv_dbg(BATADV_DBG_DAT, bat_priv, - "* type: DAT_CACHE_REPLY\n"); + "* type: DAT_DHT_REPLY\n"); break; case BATADV_P_DATA: batadv_dbg(BATADV_DBG_DAT, bat_priv, "* type: DATA\n"); @@ -786,19 +852,57 @@ static void batadv_dat_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv, }
/** - * batadv_dat_hash_free() - free the local DAT hash table + * batadv_dat_hash_free() - free a DAT hash table * @bat_priv: the bat priv with all the soft interface information */ -static void batadv_dat_hash_free(struct batadv_priv *bat_priv) +static void batadv_dat_hash_free(struct batadv_hashtable **hash) { - if (!bat_priv->dat.hash) + if (!(*hash)) return;
- __batadv_dat_purge(bat_priv, NULL); + __batadv_dat_purge(*hash, NULL);
- batadv_hash_destroy(bat_priv->dat.hash); + batadv_hash_destroy(*hash);
- bat_priv->dat.hash = NULL; + *hash = NULL; +} + +/** + * batadv_dat_cache_hash_init() - initialise the DAT cache hash table + * @bat_priv: the bat priv with all the soft interface information + * + * Return: 0 in case of success, a negative error code otherwise + */ +static int batadv_dat_cache_hash_init(struct batadv_priv *bat_priv) +{ + if (bat_priv->dat.cache_hash) + return 0; + + bat_priv->dat.cache_hash = batadv_hash_new(1024); + + if (!bat_priv->dat.cache_hash) + return -ENOMEM; + + return 0; +} + +/** + * batadv_dat_dht_hash_init() - initialise the DAT DHT hash table + * @bat_priv: the bat priv with all the soft interface information + * + * Return: 0 in case of success, a negative error code otherwise + */ +static int batadv_dat_dht_hash_init(struct batadv_priv *bat_priv) +{ + if (bat_priv->dat.dht_hash) + return 0; + + bat_priv->dat.dht_hash = batadv_hash_new(1024); + + if (!bat_priv->dat.dht_hash) + return -ENOMEM; + + return 0; }
/** @@ -809,13 +913,15 @@ static void batadv_dat_hash_free(struct batadv_priv *bat_priv) */ int batadv_dat_init(struct batadv_priv *bat_priv) { - if (bat_priv->dat.hash) - return 0; + int ret;
- bat_priv->dat.hash = batadv_hash_new(1024); + ret = batadv_dat_cache_hash_init(bat_priv); + if (ret < 0) + return ret;
- if (!bat_priv->dat.hash) - return -ENOMEM; + ret = batadv_dat_dht_hash_init(bat_priv); + if (ret < 0) + return ret;
INIT_DELAYED_WORK(&bat_priv->dat.work, batadv_dat_purge); batadv_dat_start_timer(bat_priv); @@ -838,30 +944,31 @@ void batadv_dat_free(struct batadv_priv *bat_priv)
cancel_delayed_work_sync(&bat_priv->dat.work);
- batadv_dat_hash_free(bat_priv); + batadv_dat_hash_free(&bat_priv->dat.cache_hash); + batadv_dat_hash_free(&bat_priv->dat.dht_hash); }
/** - * batadv_dat_cache_dump_entry() - dump one entry of the DAT cache table to a - * netlink socket + * batadv_dat_dump_entry() - dump one entry of a DAT hash table to a netlink + * socket * @msg: buffer for the message * @portid: netlink port * @cb: Control block containing additional options * @dat_entry: entry to dump + * @cmd: generic netlink command * * Return: 0 or error code. */ static int -batadv_dat_cache_dump_entry(struct sk_buff *msg, u32 portid, - struct netlink_callback *cb, - struct batadv_dat_entry *dat_entry) +batadv_dat_dump_entry(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, + struct batadv_dat_entry *dat_entry, u8 cmd) { int msecs; void *hdr;
hdr = genlmsg_put(msg, portid, cb->nlh->nlmsg_seq, - &batadv_netlink_family, NLM_F_MULTI, - BATADV_CMD_GET_DAT_CACHE); + &batadv_netlink_family, NLM_F_MULTI, cmd); if (!hdr) return -ENOBUFS;
@@ -884,22 +991,23 @@ batadv_dat_cache_dump_entry(struct sk_buff *msg, u32 portid, }
/** - * batadv_dat_cache_dump_bucket() - dump one bucket of the DAT cache table to - * a netlink socket + * batadv_dat_dump_bucket() - dump one bucket of a DAT hash table to a netlink + * socket * @msg: buffer for the message * @portid: netlink port * @cb: Control block containing additional options * @hash: hash to dump * @bucket: bucket index to dump * @idx_skip: How many entries to skip + * @cmd: generic netlink command * * Return: 0 or error code. */ static int -batadv_dat_cache_dump_bucket(struct sk_buff *msg, u32 portid, - struct netlink_callback *cb, - struct batadv_hashtable *hash, unsigned int bucket, - int *idx_skip) +batadv_dat_dump_bucket(struct sk_buff *msg, u32 portid, + struct netlink_callback *cb, + struct batadv_hashtable *hash, unsigned int bucket, + int *idx_skip, u8 cmd) { struct batadv_dat_entry *dat_entry; int idx = 0; @@ -911,7 +1019,7 @@ batadv_dat_cache_dump_bucket(struct sk_buff *msg, u32 portid, if (idx < *idx_skip) goto skip;
- if (batadv_dat_cache_dump_entry(msg, portid, cb, dat_entry)) { + if (batadv_dat_dump_entry(msg, portid, cb, dat_entry, cmd)) { spin_unlock_bh(&hash->list_locks[bucket]); *idx_skip = idx;
@@ -927,39 +1035,26 @@ batadv_dat_cache_dump_bucket(struct sk_buff *msg, u32 portid, }
/** - * batadv_dat_cache_dump() - dump DAT cache table to a netlink socket + * batadv_dat_dump() - dump a DAT hash table to a netlink socket + * @bat_priv: the bat priv with all the soft interface information * @msg: buffer for the message * @cb: callback structure containing arguments + * @hash: the hash table to dump from + * @cmd: generic netlink command * * Return: message length. */ -int batadv_dat_cache_dump(struct sk_buff *msg, struct netlink_callback *cb) +static int +batadv_dat_dump(struct batadv_priv *bat_priv, struct sk_buff *msg, + struct netlink_callback *cb, struct batadv_hashtable *hash, + u8 cmd) { - struct batadv_hard_iface *primary_if = NULL; int portid = NETLINK_CB(cb->skb).portid; - struct net *net = sock_net(cb->skb->sk); - struct net_device *soft_iface; - struct batadv_hashtable *hash; - struct batadv_priv *bat_priv; + struct batadv_hard_iface *primary_if; int bucket = cb->args[0]; int idx = cb->args[1]; - int ifindex; int ret = 0;
- ifindex = batadv_netlink_get_ifindex(cb->nlh, - BATADV_ATTR_MESH_IFINDEX); - if (!ifindex) - return -EINVAL; - - soft_iface = dev_get_by_index(net, ifindex); - if (!soft_iface || !batadv_softif_is_valid(soft_iface)) { - ret = -ENODEV; - goto out; - } - - bat_priv = netdev_priv(soft_iface); - hash = bat_priv->dat.hash; - primary_if = batadv_primary_if_get_selected(bat_priv); if (!primary_if || primary_if->if_status != BATADV_IF_ACTIVE) { ret = -ENOENT; @@ -967,8 +1062,8 @@ int batadv_dat_cache_dump(struct sk_buff *msg, struct netlink_callback *cb) }
while (bucket < hash->size) { - if (batadv_dat_cache_dump_bucket(msg, portid, cb, hash, bucket, - &idx)) + if (batadv_dat_dump_bucket(msg, portid, cb, hash, bucket, &idx, + cmd)) break;
bucket++; @@ -983,8 +1078,87 @@ int batadv_dat_cache_dump(struct sk_buff *msg, struct netlink_callback *cb) out: batadv_hardif_put(primary_if);
+ return ret; +} + +/** + * batadv_dat_get_softif() - get the soft interface from a netlink callback + * @cb: callback structure containing arguments + * + * Return: The soft interface on success or an error pointer otherwise. + */ +static struct net_device *batadv_dat_get_softif(struct netlink_callback *cb) +{ + struct net *net = sock_net(cb->skb->sk); + struct net_device *soft_iface; + int ifindex; + + ifindex = batadv_netlink_get_ifindex(cb->nlh, + BATADV_ATTR_MESH_IFINDEX); + if (!ifindex) + return ERR_PTR(-EINVAL); + + soft_iface = dev_get_by_index(net, ifindex); + if (!soft_iface) + return ERR_PTR(-ENODEV); + + if (!batadv_softif_is_valid(soft_iface)) { + dev_put(soft_iface); + return ERR_PTR(-ENODEV); + } + + return soft_iface; +} + +/** + * batadv_dat_cache_dump() - dump the DAT cache table to a netlink socket + * @msg: buffer for the message + * @cb: callback structure containing arguments + * + * Return: Message length on success or a negative error number otherwise. + */ +int batadv_dat_cache_dump(struct sk_buff *msg, struct netlink_callback *cb) +{ + struct net_device *soft_iface; + struct batadv_priv *bat_priv; + int ret; + + soft_iface = batadv_dat_get_softif(cb); + if (IS_ERR(soft_iface)) + return PTR_ERR(soft_iface); + + bat_priv = netdev_priv(soft_iface); + + ret = batadv_dat_dump(bat_priv, msg, cb, bat_priv->dat.cache_hash, + BATADV_CMD_GET_DAT_CACHE); + dev_put(soft_iface); + return ret; +} + +/** + * batadv_dat_dht_dump() - dump the DAT DHT table to a netlink socket + * @msg: buffer for the message + * @cb: callback structure containing arguments + * + * Return: Message length on success or a negative error number otherwise. + */ +int batadv_dat_dht_dump(struct sk_buff *msg, struct netlink_callback *cb) +{ + struct net_device *soft_iface; + struct batadv_priv *bat_priv; + int ret; + + soft_iface = batadv_dat_get_softif(cb); + if (IS_ERR(soft_iface)) + return PTR_ERR(soft_iface);
+ bat_priv = netdev_priv(soft_iface); + + ret = batadv_dat_dump(bat_priv, msg, cb, bat_priv->dat.dht_hash, + BATADV_CMD_GET_DAT_DHT); + + dev_put(soft_iface); return ret; }
@@ -1164,9 +1338,9 @@ bool batadv_dat_snoop_outgoing_arp_request(struct batadv_priv *bat_priv, hw_src = batadv_arp_hw_src(skb, hdr_size); ip_dst = batadv_arp_ip_dst(skb, hdr_size);
- batadv_dat_entry_add(bat_priv, ip_src, hw_src, vid); + batadv_dat_cache_entry_add(bat_priv, ip_src, hw_src, vid);
- dat_entry = batadv_dat_entry_hash_find(bat_priv, ip_dst, vid); + dat_entry = batadv_dat_cache_entry_find(bat_priv, ip_dst, vid); if (dat_entry) { /* If the ARP request is destined for a local client the local * client will answer itself. DAT would only generate a @@ -1256,9 +1430,9 @@ bool batadv_dat_snoop_incoming_arp_request(struct batadv_priv *bat_priv,
batadv_dbg_arp(bat_priv, skb, hdr_size, "Parsing incoming ARP REQUEST");
- batadv_dat_entry_add(bat_priv, ip_src, hw_src, vid); + batadv_dat_cache_entry_add(bat_priv, ip_src, hw_src, vid);
- dat_entry = batadv_dat_entry_hash_find(bat_priv, ip_dst, vid); + dat_entry = batadv_dat_dht_entry_find(bat_priv, ip_dst, vid); if (!dat_entry) goto out;
@@ -1273,7 +1447,7 @@ bool batadv_dat_snoop_incoming_arp_request(struct batadv_priv *bat_priv, */ if (hdr_size == sizeof(struct batadv_unicast_4addr_packet)) err = batadv_send_skb_via_tt_4addr(bat_priv, skb_new, - BATADV_P_DAT_CACHE_REPLY, + BATADV_P_DAT_DHT_REPLY, NULL, vid); else err = batadv_send_skb_via_tt(bat_priv, skb_new, NULL, vid); @@ -1319,8 +1493,8 @@ void batadv_dat_snoop_outgoing_arp_reply(struct batadv_priv *bat_priv, hw_dst = batadv_arp_hw_dst(skb, hdr_size); ip_dst = batadv_arp_ip_dst(skb, hdr_size);
- batadv_dat_entry_add(bat_priv, ip_src, hw_src, vid); - batadv_dat_entry_add(bat_priv, ip_dst, hw_dst, vid); + batadv_dat_cache_entry_add(bat_priv, ip_src, hw_src, vid); + batadv_dat_cache_entry_add(bat_priv, ip_dst, hw_dst, vid);
/* Send the ARP reply to the candidates for both the IP addresses that * the node obtained from the ARP reply @@ -1342,7 +1516,8 @@ void batadv_dat_snoop_outgoing_arp_reply(struct batadv_priv *bat_priv, * packet has to be delivered to the interface */ bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv, - struct sk_buff *skb, int hdr_size) + struct sk_buff *skb, int hdr_size, + bool is_dht_put) { struct batadv_dat_entry *dat_entry = NULL; u16 type; @@ -1367,13 +1542,18 @@ bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv, hw_dst = batadv_arp_hw_dst(skb, hdr_size); ip_dst = batadv_arp_ip_dst(skb, hdr_size);
+ if (is_dht_put) { + batadv_dat_dht_entry_add(bat_priv, ip_src, hw_src, vid); + batadv_dat_dht_entry_add(bat_priv, ip_dst, hw_dst, vid); + } + /* If ip_dst is already in cache and has the right mac address, * drop this frame if this ARP reply is destined for us because it's * most probably an ARP reply generated by another node of the DHT. * We have most probably received already a reply earlier. Delivering * this frame would lead to doubled receive of an ARP reply. */ - dat_entry = batadv_dat_entry_hash_find(bat_priv, ip_src, vid); + dat_entry = batadv_dat_cache_entry_find(bat_priv, ip_src, vid); if (dat_entry && batadv_compare_eth(hw_src, dat_entry->mac_addr)) { batadv_dbg(BATADV_DBG_DAT, bat_priv, "Doubled ARP reply removed: ARP MSG = [src: %pM-%pI4 dst: %pM-%pI4]; dat_entry: %pM-%pI4\n", hw_src, &ip_src, hw_dst, &ip_dst, @@ -1384,8 +1564,8 @@ bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv, /* Update our internal cache with both the IP addresses the node got * within the ARP reply */ - batadv_dat_entry_add(bat_priv, ip_src, hw_src, vid); - batadv_dat_entry_add(bat_priv, ip_dst, hw_dst, vid); + batadv_dat_cache_entry_add(bat_priv, ip_src, hw_src, vid); + batadv_dat_cache_entry_add(bat_priv, ip_dst, hw_dst, vid);
if (dropped) goto out; @@ -1641,8 +1821,8 @@ static void batadv_dat_put_dhcp(struct batadv_priv *bat_priv, u8 *chaddr,
skb_set_network_header(skb, ETH_HLEN);
- batadv_dat_entry_add(bat_priv, yiaddr, chaddr, vid); - batadv_dat_entry_add(bat_priv, ip_dst, hw_dst, vid); + batadv_dat_cache_entry_add(bat_priv, yiaddr, chaddr, vid); + batadv_dat_cache_entry_add(bat_priv, ip_dst, hw_dst, vid);
batadv_dat_forward_data(bat_priv, skb, yiaddr, vid, BATADV_P_DAT_DHT_PUT); @@ -1767,8 +1947,8 @@ void batadv_dat_snoop_incoming_dhcp_ack(struct batadv_priv *bat_priv, hw_src = ethhdr->h_source; vid = batadv_dat_get_vid(skb, &hdr_size);
- batadv_dat_entry_add(bat_priv, yiaddr, chaddr, vid); - batadv_dat_entry_add(bat_priv, ip_src, hw_src, vid); + batadv_dat_cache_entry_add(bat_priv, yiaddr, chaddr, vid); + batadv_dat_cache_entry_add(bat_priv, ip_src, hw_src, vid);
batadv_dbg(BATADV_DBG_DAT, bat_priv, "Snooped from incoming DHCPACK (server address): %pI4, %pM (vid: %i)\n", @@ -1812,7 +1992,7 @@ bool batadv_dat_drop_broadcast_packet(struct batadv_priv *bat_priv, goto out;
ip_dst = batadv_arp_ip_dst(forw_packet->skb, hdr_size); - dat_entry = batadv_dat_entry_hash_find(bat_priv, ip_dst, vid); + dat_entry = batadv_dat_cache_entry_find(bat_priv, ip_dst, vid); /* check if the node already got this entry */ if (!dat_entry) { batadv_dbg(BATADV_DBG_DAT, bat_priv, diff --git a/net/batman-adv/distributed-arp-table.h b/net/batman-adv/distributed-arp-table.h index bed7f3d20844..28ac93b0113d 100644 --- a/net/batman-adv/distributed-arp-table.h +++ b/net/batman-adv/distributed-arp-table.h @@ -31,7 +31,8 @@ bool batadv_dat_snoop_incoming_arp_request(struct batadv_priv *bat_priv, void batadv_dat_snoop_outgoing_arp_reply(struct batadv_priv *bat_priv, struct sk_buff *skb); bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv, - struct sk_buff *skb, int hdr_size); + struct sk_buff *skb, int hdr_size, + bool is_dht_put); void batadv_dat_snoop_outgoing_dhcp_ack(struct batadv_priv *bat_priv, struct sk_buff *skb, __be16 proto, @@ -74,6 +75,7 @@ batadv_dat_init_own_addr(struct batadv_priv *bat_priv, int batadv_dat_init(struct batadv_priv *bat_priv); void batadv_dat_free(struct batadv_priv *bat_priv); int batadv_dat_cache_dump(struct sk_buff *msg, struct netlink_callback *cb); +int batadv_dat_dht_dump(struct sk_buff *msg, struct netlink_callback *cb);
/** * batadv_dat_inc_counter() - increment the correct DAT packet counter @@ -126,7 +128,8 @@ batadv_dat_snoop_outgoing_arp_reply(struct batadv_priv *bat_priv,
static inline bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv, - struct sk_buff *skb, int hdr_size) + struct sk_buff *skb, int hdr_size, + bool is_dht_put) { return false; } @@ -176,6 +179,12 @@ batadv_dat_cache_dump(struct sk_buff *msg, struct netlink_callback *cb) return -EOPNOTSUPP; }
+static inline int +batadv_dat_dht_dump(struct sk_buff *msg, struct netlink_callback *cb) +{ + return -EOPNOTSUPP; +} + static inline void batadv_dat_inc_counter(struct batadv_priv *bat_priv, u8 subtype) { diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c index 3f0003fc502c..c3f8d3511236 100644 --- a/net/batman-adv/netlink.c +++ b/net/batman-adv/netlink.c @@ -1442,6 +1442,11 @@ static const struct genl_small_ops batadv_netlink_ops[] = { .flags = GENL_UNS_ADMIN_PERM, .dumpit = batadv_dat_cache_dump, }, + { + .cmd = BATADV_CMD_GET_DAT_DHT, + .flags = GENL_ADMIN_PERM, + .dumpit = batadv_dat_dht_dump, + }, { .cmd = BATADV_CMD_GET_MCAST_FLAGS, .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index f1061985149f..b3195a225ccc 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -943,10 +943,10 @@ int batadv_recv_unicast_packet(struct sk_buff *skb, struct batadv_unicast_4addr_packet *unicast_4addr_packet; u8 *orig_addr, *orig_addr_gw; struct batadv_orig_node *orig_node = NULL, *orig_node_gw = NULL; + bool is4addr, is_gw, is_dht_put = false; int check, hdr_size = sizeof(*unicast_packet); enum batadv_subtype subtype; int ret = NET_RX_DROP; - bool is4addr, is_gw;
unicast_packet = (struct batadv_unicast_packet *)skb->data; is4addr = unicast_packet->packet_type == BATADV_UNICAST_4ADDR; @@ -1005,6 +1005,8 @@ int batadv_recv_unicast_packet(struct sk_buff *skb, orig_addr = unicast_4addr_packet->src; orig_node = batadv_orig_hash_find(bat_priv, orig_addr); + } else if (subtype == BATADV_P_DAT_DHT_PUT) { + is_dht_put = true; } }
@@ -1012,7 +1014,7 @@ int batadv_recv_unicast_packet(struct sk_buff *skb, hdr_size)) goto rx_success; if (batadv_dat_snoop_incoming_arp_reply(bat_priv, skb, - hdr_size)) + hdr_size, is_dht_put)) goto rx_success;
batadv_dat_snoop_incoming_dhcp_ack(bat_priv, skb, hdr_size); @@ -1249,7 +1251,7 @@ int batadv_recv_bcast_packet(struct sk_buff *skb,
if (batadv_dat_snoop_incoming_arp_request(bat_priv, skb, hdr_size)) goto rx_success; - if (batadv_dat_snoop_incoming_arp_reply(bat_priv, skb, hdr_size)) + if (batadv_dat_snoop_incoming_arp_reply(bat_priv, skb, hdr_size, false)) goto rx_success;
batadv_dat_snoop_incoming_dhcp_ack(bat_priv, skb, hdr_size); diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index 00840d5784fe..fe0b9b229bff 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -1231,8 +1231,11 @@ struct batadv_priv_dat { /** @addr: node DAT address */ batadv_dat_addr_t addr;
- /** @hash: hashtable representing the local ARP cache */ - struct batadv_hashtable *hash; + /** @cache_hash: hashtable representing the local ARP cache */ + struct batadv_hashtable *cache_hash; + + /** @dht_hash: hashtable representing the local DAT DHT */ + struct batadv_hashtable *dht_hash;
/** @work: work queue callback item for cache purging */ struct delayed_work work;
Hi Linus,
On 11/09/2024 06:57, Linus Lüssing wrote: [...]
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index 00840d5784fe..fe0b9b229bff 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -1231,8 +1231,11 @@ struct batadv_priv_dat { /** @addr: node DAT address */ batadv_dat_addr_t addr;
- /** @hash: hashtable representing the local ARP cache */
- struct batadv_hashtable *hash;
- /** @cache_hash: hashtable representing the local ARP cache */
- struct batadv_hashtable *cache_hash;
- /** @dht_hash: hashtable representing the local DAT DHT */
- struct batadv_hashtable *dht_hash;
I haven't followed previous discussions, therefore my question may just sound redundant, but if "cache_hash" is for ARP what is the "DAT DHT"? (the A in DAT stands for ARP).
Moreover, it seems with this approach you are converting the DAT concept into something "type specific" - am I wrong?
Is it truly required just to have a different timeout? Wouldn't a mark on entries be enough to instruct the subsystem about what timeout value to use?
Cheers,
/** @work: work queue callback item for cache purging */ struct delayed_work work;
Hi Antonio,
Thanks for the feedback!
I haven't followed previous discussions, therefore my question may just sound redundant, but if "cache_hash" is for ARP what is the "DAT DHT"? (the A in DAT stands for ARP).
The dht_hash would just store what was pushed to it from remote nodes through the DAT DHT feature. So through a BATADV_P_DAT_DHT_PUT in a batman-adv unicast4addr packet.
And would only be used for responses to a BATADV_P_DAT_DHT_GET.
The cache_hash would store all the other cases. That is local additions learned through bat0. Or the responses this node got to a BATADV_P_DAT_DHT_GET.
Moreover, it seems with this approach you are converting the DAT concept into something "type specific" - am I wrong?
You mean to be able to use it for other types than ARP? Like the patchset we had for IPv6 a while ago? Hm, no, that at least wasn't the intention.
Is it truly required just to have a different timeout? Wouldn't a mark on entries be enough to instruct the subsystem about what timeout value to use?
That would work, too, right. I just found a split simpler and less error-prone. With flags I'm always worried about missing potential race conditions when flipping it back and forth. Especially with the two different timeouts involved here.
For instance if I had an entry through a remote node (BATADV_P_DAT_DHT_PUT) and therefore with a flag set. But now got an update for the same address/entry through bat0 (maybe the client device roamed to us? Or some collision / duplicate address usage?). Should I now always unset the flag? Or should I check if its remaining timeout is less than 5min and only if it is then unset the flag?
Should a response to a BATADV_P_DAT_DHT_GET only be to an entry with the flag set? Or independent to what the flag is set to?
.oO(Maybe a split could also be useful if one wanted to introduce individual limits for additions? Against remote attackers trying to DoS a node?)
Let me know if you'd still prefer a flag instead of a separate hash table and I could change that.
Regards, Linus
Hi,
On 12/09/2024 13:08, Linus Lüssing wrote:
Hi Antonio,
Thanks for the feedback!
I haven't followed previous discussions, therefore my question may just sound redundant, but if "cache_hash" is for ARP what is the "DAT DHT"? (the A in DAT stands for ARP).
The dht_hash would just store what was pushed to it from remote nodes through the DAT DHT feature. So through a BATADV_P_DAT_DHT_PUT in a batman-adv unicast4addr packet.
And would only be used for responses to a BATADV_P_DAT_DHT_GET.
The cache_hash would store all the other cases. That is local additions learned through bat0. Or the responses this node got to a BATADV_P_DAT_DHT_GET.
Ah I see, so you're entirely splitting the DHT from the local ARP cache. Naming indeed makes sense now.
Moreover, it seems with this approach you are converting the DAT concept into something "type specific" - am I wrong?
You mean to be able to use it for other types than ARP? Like the patchset we had for IPv6 a while ago? Hm, no, that at least wasn't the intention.
nah, I got it wrong. Your explanation above helped me clear the doubt.
Is it truly required just to have a different timeout? Wouldn't a mark on entries be enough to instruct the subsystem about what timeout value to use?
That would work, too, right. I just found a split simpler and less error-prone. With flags I'm always worried about missing potential race conditions when flipping it back and forth. Especially with the two different timeouts involved here.
From what you are saying and (what I understand) this splitting is not only about the timeout, but it's a clear cut between "what we have in the DHT" and what we are collecting locally.
So splitting data structure makes a lot of sense to me.
With a flag, you should assume that a specific entry can either have or not the flag set. With two tables, the same entry could technically exists in both and it'll be up to you decide how to handle the situation (without ambiguity in this case because you know exactly where each entry is coming from).
For instance if I had an entry through a remote node (BATADV_P_DAT_DHT_PUT) and therefore with a flag set. But now got an update for the same address/entry through bat0 (maybe the client device roamed to us? Or some collision / duplicate address usage?). Should I now always unset the flag? Or should I check if its remaining timeout is less than 5min and only if it is then unset the flag?
Should a response to a BATADV_P_DAT_DHT_GET only be to an entry with the flag set? Or independent to what the flag is set to?
I feel you need to answer the above questions even with two tables. No? The difference is that with two tables you have more information and don't need overwrite a DHT entry with a local one (and viceversa).
.oO(Maybe a split could also be useful if one wanted to introduce individual limits for additions? Against remote attackers trying to DoS a node?)
Let me know if you'd still prefer a flag instead of a separate hash table and I could change that.
I think splitting is a good idea, not only because of the timeout, but also because it makes the state more clear.
Regards,
Regards, Linus
On Thu, Sep 12, 2024 at 01:28:16PM +0200, Antonio Quartulli wrote:
I feel you need to answer the above questions even with two tables. No? The difference is that with two tables you have more information and don't need overwrite a DHT entry with a local one (and viceversa).
Right. Though the straight-forward/easy approach differs between the two, I guess:
Currently with the two table approach I'm not having any direct interaction between them. So in theory a node might have a DHT table entry which is not as up-to-date as a DAT local cache entry on the same node?
But I'm having a hard time constructing this scenario. I guess it would need some topology change that results in new/other DAT candidate nodes? But then that entry wouldn't be that relevant anymore anyway, if it's not on a DAT candidate anymore?
While the straight-forward / less lines of codes approach with flags would be to always overwrite and flip the flag, I guess.
I think splitting is a good idea, not only because of the timeout, but also because it makes the state more clear.
Right, and you can also see the two, distinct states clearly as a user with the new "batctl dd" vs. "batctl dc".
Regards, Linus
Hi,
On 11/09/2024 06:57, Linus Lüssing wrote:
Similar to the translation table split the (mostly) locally maintained DAT cache and foreignly maintained DAT DHT concepts into two separate tables.
If I got your reasoning right, the second table is not only for foreignly maintained entries, but it could also be for entries installed locally by a DHT_PUT, no?
Therefore I'd remove foreignly and just say that this is the actual DHT.
More important: wouldn't it make sense to call the first table just "ARP cache/table"? Am I wrong or it is not truly related to DAT as it just caches ARP entries passing by, without any logic linked to the DHT?
This eases the monitoring and debugging regarding the origin of DAT entries. And allows to apply differing properties to entries in the DAT cache and DAT DHT in the future.
Maybe you can add a line about why you are renaming the CACHE_REPLY to DHT_REPLY? Otherwise that change would just get through without enough enphasis.
Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue
include/uapi/linux/batadv_packet.h | 4 +- include/uapi/linux/batman_adv.h | 5 + net/batman-adv/distributed-arp-table.c | 360 ++++++++++++++++++------- net/batman-adv/distributed-arp-table.h | 13 +- net/batman-adv/netlink.c | 5 + net/batman-adv/routing.c | 8 +- net/batman-adv/types.h | 7 +- 7 files changed, 304 insertions(+), 98 deletions(-)
diff --git a/include/uapi/linux/batadv_packet.h b/include/uapi/linux/batadv_packet.h index 6e25753015df..0fc631593f49 100644 --- a/include/uapi/linux/batadv_packet.h +++ b/include/uapi/linux/batadv_packet.h @@ -60,13 +60,15 @@ enum batadv_packettype {
- @BATADV_P_DATA: user payload
- @BATADV_P_DAT_DHT_GET: DHT request message
- @BATADV_P_DAT_DHT_PUT: DHT store message
- @BATADV_P_DAT_CACHE_REPLY: ARP reply generated by DAT
- @BATADV_P_DAT_CACHE_REPLY: deprecated: use BATADV_P_DAT_DHT_REPLY instead
- @BATADV_P_DAT_DHT_REPLY: ARP reply generated by DAT
*/ enum batadv_subtype { BATADV_P_DATA = 0x01, BATADV_P_DAT_DHT_GET = 0x02, BATADV_P_DAT_DHT_PUT = 0x03, BATADV_P_DAT_CACHE_REPLY = 0x04,
BATADV_P_DAT_DHT_REPLY = 0x04, };
/* this file is included by batctl which needs these defines */
diff --git a/include/uapi/linux/batman_adv.h b/include/uapi/linux/batman_adv.h index 35dc016c9bb4..9498ccb09d67 100644 --- a/include/uapi/linux/batman_adv.h +++ b/include/uapi/linux/batman_adv.h @@ -613,6 +613,11 @@ enum batadv_nl_commands { */ BATADV_CMD_SET_VLAN,
/**
* @BATADV_CMD_GET_DAT_DHT: Query list of DAT DHT entries*/BATADV_CMD_GET_DAT_DHT,
/* add new commands above here */
/**
diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c index 4c7e85534324..8a054b0cf2a2 100644 --- a/net/batman-adv/distributed-arp-table.c +++ b/net/batman-adv/distributed-arp-table.c @@ -145,17 +145,17 @@ static bool batadv_dat_to_purge(struct batadv_dat_entry *dat_entry) }
/**
- __batadv_dat_purge() - delete entries from the DAT local storage
- @bat_priv: the bat priv with all the soft interface information
- __batadv_dat_purge() - delete entries from a DAT hash table
- @hash: the hash table to delete from
- @to_purge: function in charge to decide whether an entry has to be purged or
not. This function takes the dat_entry as argument and has toreturns a boolean value: true is the entry has to be deleted,false otherwise
- Loops over each entry in the DAT local storage and deletes it if and only if
*/
- Loops over each entry in a DAT hash table and deletes it if and only if
- the to_purge function passed as argument returns true.
-static void __batadv_dat_purge(struct batadv_priv *bat_priv, +static void __batadv_dat_purge(struct batadv_hashtable *hash, bool (*to_purge)(struct batadv_dat_entry *)) { spinlock_t *list_lock; /* protects write access to the hash lists */ @@ -164,12 +164,12 @@ static void __batadv_dat_purge(struct batadv_priv *bat_priv, struct hlist_head *head; u32 i;
- if (!bat_priv->dat.hash)
- if (!hash) return;
- for (i = 0; i < bat_priv->dat.hash->size; i++) {
head = &bat_priv->dat.hash->table[i];list_lock = &bat_priv->dat.hash->list_locks[i];
for (i = 0; i < hash->size; i++) {
head = &hash->table[i];list_lock = &hash->list_locks[i];spin_lock_bh(list_lock); hlist_for_each_entry_safe(dat_entry, node_tmp, head,
@@ -202,7 +202,8 @@ static void batadv_dat_purge(struct work_struct *work) priv_dat = container_of(delayed_work, struct batadv_priv_dat, work); bat_priv = container_of(priv_dat, struct batadv_priv, dat);
- __batadv_dat_purge(bat_priv, batadv_dat_to_purge);
- __batadv_dat_purge(bat_priv->dat.cache_hash, batadv_dat_to_purge);
- __batadv_dat_purge(bat_priv->dat.dht_hash, batadv_dat_to_purge); batadv_dat_start_timer(bat_priv); }
@@ -314,21 +315,19 @@ static u32 batadv_hash_dat(const void *data, u32 size) }
/**
- batadv_dat_entry_hash_find() - look for a given dat_entry in the local hash
- table
- @bat_priv: the bat priv with all the soft interface information
- batadv_dat_entry_hash_find() - look for a given dat_entry in a hash table
*/ static struct batadv_dat_entry *
- @hash: the hash table to search in
- @ip: search key
- @vid: VLAN identifier
- Return: the dat_entry if found, NULL otherwise.
-batadv_dat_entry_hash_find(struct batadv_priv *bat_priv, __be32 ip, +batadv_dat_entry_hash_find(struct batadv_hashtable *hash, __be32 ip, unsigned short vid) { struct hlist_head *head; struct batadv_dat_entry to_find, *dat_entry, *dat_entry_tmp = NULL;
struct batadv_hashtable *hash = bat_priv->dat.hash; u32 index;
if (!hash)
@@ -356,28 +355,63 @@ batadv_dat_entry_hash_find(struct batadv_priv *bat_priv, __be32 ip, return dat_entry_tmp; }
+/**
- batadv_dat_cache_entry_find() - look for a given dat_entry in the DAT cache
- @bat_priv: the bat priv with all the soft interface information
- @ip: search key
- @vid: VLAN identifier
- Return: the dat_entry if found, NULL otherwise.
- */
+static inline struct batadv_dat_entry * +batadv_dat_cache_entry_find(struct batadv_priv *bat_priv, __be32 ip,
unsigned short vid)+{
- return batadv_dat_entry_hash_find(bat_priv->dat.cache_hash, ip, vid);
+}
+/**
- batadv_dat_dht_entry_find() - look for a given dat_entry in the DAT DHT
- @bat_priv: the bat priv with all the soft interface information
- @ip: search key
- @vid: VLAN identifier
- Return: the dat_entry if found, NULL otherwise.
- */
+static inline struct batadv_dat_entry * +batadv_dat_dht_entry_find(struct batadv_priv *bat_priv, __be32 ip,
unsigned short vid)+{
- return batadv_dat_entry_hash_find(bat_priv->dat.dht_hash, ip, vid);
+}
- /**
- batadv_dat_entry_add() - add a new dat entry or update it if already exists
- @bat_priv: the bat priv with all the soft interface information
- @hash: the hash table to add to
- @ip: ipv4 to add/edit
- @mac_addr: mac address to assign to the given ipv4
- @vid: VLAN identifier
*/
- @tablename: the name of the hash table to add to
-static void batadv_dat_entry_add(struct batadv_priv *bat_priv, __be32 ip,
u8 *mac_addr, unsigned short vid)+static void +batadv_dat_entry_add(struct batadv_priv *bat_priv,
struct batadv_hashtable *hash, __be32 ip,u8 *mac_addr, unsigned short vid,const char *tablename)
- dat_entry = batadv_dat_entry_hash_find(bat_priv, ip, vid);
- dat_entry = batadv_dat_entry_hash_find(hash, ip, vid); /* if this entry is already known, just update it */ if (dat_entry) { if (!batadv_compare_eth(dat_entry->mac_addr, mac_addr)) ether_addr_copy(dat_entry->mac_addr, mac_addr); dat_entry->last_update = jiffies; batadv_dbg(BATADV_DBG_DAT, bat_priv,
"Entry updated: %pI4 %pM (vid: %d)\n",&dat_entry->ip, dat_entry->mac_addr,
"Entry updated in %s: %pI4 %pM (vid: %d)\n",tablename, &dat_entry->ip, dat_entry->mac_addr, batadv_print_vid(vid));@@ -393,7 +427,7 @@ static void batadv_dat_entry_add(struct batadv_priv *bat_priv, __be32 ip, kref_init(&dat_entry->refcount);
kref_get(&dat_entry->refcount);
- hash_added = batadv_hash_add(bat_priv->dat.hash, batadv_compare_dat,
- hash_added = batadv_hash_add(hash, batadv_compare_dat, batadv_hash_dat, dat_entry, &dat_entry->hash_entry);
@@ -403,13 +437,45 @@ static void batadv_dat_entry_add(struct batadv_priv *bat_priv, __be32 ip, goto out; }
- batadv_dbg(BATADV_DBG_DAT, bat_priv, "New entry added: %pI4 %pM (vid: %d)\n",
&dat_entry->ip, dat_entry->mac_addr, batadv_print_vid(vid));
batadv_dbg(BATADV_DBG_DAT, bat_priv,
"New entry added in %s: %pI4 %pM (vid: %d)\n",tablename, &dat_entry->ip, dat_entry->mac_addr,batadv_print_vid(vid));out: batadv_dat_entry_put(dat_entry); }
+/**
- batadv_dat_cache_entry_add() - add or update dat_entry in the DAT cache
- @bat_priv: the bat priv with all the soft interface information
- @ip: ipv4 to add/edit
- @mac_addr: mac address to assign to the given ipv4
- @vid: VLAN identifier
- */
+static inline void +batadv_dat_cache_entry_add(struct batadv_priv *bat_priv, __be32 ip,
u8 *mac_addr, unsigned short vid)+{
- batadv_dat_entry_add(bat_priv, bat_priv->dat.cache_hash, ip, mac_addr,
vid, "cache");+}
+/**
- batadv_dat_dht_entry_add() - add or update dat_entry in the DAT DHT
- @bat_priv: the bat priv with all the soft interface information
- @ip: ipv4 to add/edit
- @mac_addr: mac address to assign to the given ipv4
- @vid: VLAN identifier
- */
+static inline void +batadv_dat_dht_entry_add(struct batadv_priv *bat_priv, __be32 ip,
u8 *mac_addr, unsigned short vid)+{
- batadv_dat_entry_add(bat_priv, bat_priv->dat.dht_hash, ip, mac_addr,
vid, "dht");+}
#ifdef CONFIG_BATMAN_ADV_DEBUG
/**
@@ -459,9 +525,9 @@ static void batadv_dbg_arp(struct batadv_priv *bat_priv, struct sk_buff *skb, case BATADV_P_DAT_DHT_GET: batadv_dbg(BATADV_DBG_DAT, bat_priv, "* type: DAT_DHT_GET\n"); break;
case BATADV_P_DAT_CACHE_REPLY:
case BATADV_P_DAT_DHT_REPLY: batadv_dbg(BATADV_DBG_DAT, bat_priv,
"* type: DAT_CACHE_REPLY\n");
"* type: DAT_DHT_REPLY\n"); break;@@ -786,19 +852,57 @@ static void batadv_dat_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv, }
/**
- batadv_dat_hash_free() - free the local DAT hash table
*/
- batadv_dat_hash_free() - free a DAT hash table
- @bat_priv: the bat priv with all the soft interface information
-static void batadv_dat_hash_free(struct batadv_priv *bat_priv) +static void batadv_dat_hash_free(struct batadv_hashtable **hash) {
- if (!bat_priv->dat.hash)
- if (!(*hash)) return;
- __batadv_dat_purge(bat_priv, NULL);
- __batadv_dat_purge(*hash, NULL);
- batadv_hash_destroy(bat_priv->dat.hash);
- batadv_hash_destroy(*hash);
- bat_priv->dat.hash = NULL;
- *hash = NULL;
+}
+/**
- batadv_dat_cache_hash_init() - initialise the DAT cache hash table
- @bat_priv: the bat priv with all the soft interface information
- Return: 0 in case of success, a negative error code otherwise
- */
+static int batadv_dat_cache_hash_init(struct batadv_priv *bat_priv) +{
- if (bat_priv->dat.cache_hash)
return 0;- bat_priv->dat.cache_hash = batadv_hash_new(1024);
- if (!bat_priv->dat.cache_hash)
return -ENOMEM;- return 0;
+}
+/**
- batadv_dat_dht_hash_init() - initialise the DAT DHT hash table
- @bat_priv: the bat priv with all the soft interface information
- Return: 0 in case of success, a negative error code otherwise
- */
+static int batadv_dat_dht_hash_init(struct batadv_priv *bat_priv) +{
if (bat_priv->dat.dht_hash)
return 0;bat_priv->dat.dht_hash = batadv_hash_new(1024);
if (!bat_priv->dat.dht_hash)
return -ENOMEM;return 0; }
/**
@@ -809,13 +913,15 @@ static void batadv_dat_hash_free(struct batadv_priv *bat_priv) */ int batadv_dat_init(struct batadv_priv *bat_priv) {
- if (bat_priv->dat.hash)
return 0;
- int ret;
- bat_priv->dat.hash = batadv_hash_new(1024);
- ret = batadv_dat_cache_hash_init(bat_priv);
- if (ret < 0)
return ret;
- if (!bat_priv->dat.hash)
return -ENOMEM;
ret = batadv_dat_dht_hash_init(bat_priv);
if (ret < 0)
return ret;INIT_DELAYED_WORK(&bat_priv->dat.work, batadv_dat_purge); batadv_dat_start_timer(bat_priv);
@@ -838,30 +944,31 @@ void batadv_dat_free(struct batadv_priv *bat_priv)
cancel_delayed_work_sync(&bat_priv->dat.work);
- batadv_dat_hash_free(bat_priv);
batadv_dat_hash_free(&bat_priv->dat.cache_hash);
batadv_dat_hash_free(&bat_priv->dat.dht_hash); }
/**
- batadv_dat_cache_dump_entry() - dump one entry of the DAT cache table to a
- netlink socket
- batadv_dat_dump_entry() - dump one entry of a DAT hash table to a netlink
- socket
- @msg: buffer for the message
- @portid: netlink port
- @cb: Control block containing additional options
- @dat_entry: entry to dump
*/ static int
- @cmd: generic netlink command
- Return: 0 or error code.
-batadv_dat_cache_dump_entry(struct sk_buff *msg, u32 portid,
struct netlink_callback *cb,struct batadv_dat_entry *dat_entry)+batadv_dat_dump_entry(struct sk_buff *msg, u32 portid,
struct netlink_callback *cb,struct batadv_dat_entry *dat_entry, u8 cmd){ int msecs; void *hdr;
hdr = genlmsg_put(msg, portid, cb->nlh->nlmsg_seq,
&batadv_netlink_family, NLM_F_MULTI,BATADV_CMD_GET_DAT_CACHE);
&batadv_netlink_family, NLM_F_MULTI, cmd);@@ -884,22 +991,23 @@ batadv_dat_cache_dump_entry(struct sk_buff *msg, u32 portid, }
/**
- batadv_dat_cache_dump_bucket() - dump one bucket of the DAT cache table to
- a netlink socket
- batadv_dat_dump_bucket() - dump one bucket of a DAT hash table to a netlink
- socket
- @msg: buffer for the message
- @portid: netlink port
- @cb: Control block containing additional options
- @hash: hash to dump
- @bucket: bucket index to dump
- @idx_skip: How many entries to skip
*/ static int
- @cmd: generic netlink command
- Return: 0 or error code.
-batadv_dat_cache_dump_bucket(struct sk_buff *msg, u32 portid,
struct netlink_callback *cb,struct batadv_hashtable *hash, unsigned int bucket,int *idx_skip)+batadv_dat_dump_bucket(struct sk_buff *msg, u32 portid,
struct netlink_callback *cb,struct batadv_hashtable *hash, unsigned int bucket,int *idx_skip, u8 cmd)@@ -911,7 +1019,7 @@ batadv_dat_cache_dump_bucket(struct sk_buff *msg, u32 portid, if (idx < *idx_skip) goto skip;
if (batadv_dat_cache_dump_entry(msg, portid, cb, dat_entry)) {
if (batadv_dat_dump_entry(msg, portid, cb, dat_entry, cmd)) { spin_unlock_bh(&hash->list_locks[bucket]); *idx_skip = idx;@@ -927,39 +1035,26 @@ batadv_dat_cache_dump_bucket(struct sk_buff *msg, u32 portid, }
/**
- batadv_dat_cache_dump() - dump DAT cache table to a netlink socket
- batadv_dat_dump() - dump a DAT hash table to a netlink socket
- @bat_priv: the bat priv with all the soft interface information
- @msg: buffer for the message
- @cb: callback structure containing arguments
- @hash: the hash table to dump from
*/
- @cmd: generic netlink command
- Return: message length.
-int batadv_dat_cache_dump(struct sk_buff *msg, struct netlink_callback *cb) +static int +batadv_dat_dump(struct batadv_priv *bat_priv, struct sk_buff *msg,
struct netlink_callback *cb, struct batadv_hashtable *hash,u8 cmd)
- struct batadv_hard_iface *primary_if = NULL; int portid = NETLINK_CB(cb->skb).portid;
- struct net *net = sock_net(cb->skb->sk);
- struct net_device *soft_iface;
- struct batadv_hashtable *hash;
- struct batadv_priv *bat_priv;
- struct batadv_hard_iface *primary_if; int bucket = cb->args[0]; int idx = cb->args[1];
int ifindex; int ret = 0;
ifindex = batadv_netlink_get_ifindex(cb->nlh,
BATADV_ATTR_MESH_IFINDEX);if (!ifindex)
return -EINVAL;soft_iface = dev_get_by_index(net, ifindex);
if (!soft_iface || !batadv_softif_is_valid(soft_iface)) {
ret = -ENODEV;goto out;}
bat_priv = netdev_priv(soft_iface);
hash = bat_priv->dat.hash;
primary_if = batadv_primary_if_get_selected(bat_priv); if (!primary_if || primary_if->if_status != BATADV_IF_ACTIVE) { ret = -ENOENT;
@@ -967,8 +1062,8 @@ int batadv_dat_cache_dump(struct sk_buff *msg, struct netlink_callback *cb) }
while (bucket < hash->size) {
if (batadv_dat_cache_dump_bucket(msg, portid, cb, hash, bucket,&idx))
if (batadv_dat_dump_bucket(msg, portid, cb, hash, bucket, &idx,cmd)) break;bucket++;
@@ -983,8 +1078,87 @@ int batadv_dat_cache_dump(struct sk_buff *msg, struct netlink_callback *cb) out: batadv_hardif_put(primary_if);
- return ret;
+}
+/**
- batadv_dat_get_softif() - get the soft interface from a netlink callback
- @cb: callback structure containing arguments
- Return: The soft interface on success or an error pointer otherwise.
- */
+static struct net_device *batadv_dat_get_softif(struct netlink_callback *cb) +{
- struct net *net = sock_net(cb->skb->sk);
- struct net_device *soft_iface;
- int ifindex;
- ifindex = batadv_netlink_get_ifindex(cb->nlh,
BATADV_ATTR_MESH_IFINDEX);- if (!ifindex)
return ERR_PTR(-EINVAL);- soft_iface = dev_get_by_index(net, ifindex);
- if (!soft_iface)
return ERR_PTR(-ENODEV);- if (!batadv_softif_is_valid(soft_iface)) {
dev_put(soft_iface);return ERR_PTR(-ENODEV);- }
- return soft_iface;
+}
I don't think this function is DAT specific at all. Moreover, the very same code (which I think you are re-using here) appears in batadv_netlink_dump_hardif().
I think it'd make more sense to factor it out and create a helper out of it (place it in netlink.c?). This way we avoid code duplication.
[I might be wrong but 90% of the work already is in batadv_get_softif_from_info()]
+/**
- batadv_dat_cache_dump() - dump the DAT cache table to a netlink socket
- @msg: buffer for the message
- @cb: callback structure containing arguments
- Return: Message length on success or a negative error number otherwise.
- */
+int batadv_dat_cache_dump(struct sk_buff *msg, struct netlink_callback *cb) +{
- struct net_device *soft_iface;
- struct batadv_priv *bat_priv;
- int ret;
- soft_iface = batadv_dat_get_softif(cb);
- if (IS_ERR(soft_iface))
return PTR_ERR(soft_iface);- bat_priv = netdev_priv(soft_iface);
- ret = batadv_dat_dump(bat_priv, msg, cb, bat_priv->dat.cache_hash,
BATADV_CMD_GET_DAT_CACHE);- dev_put(soft_iface);
- return ret;
+}
+/**
- batadv_dat_dht_dump() - dump the DAT DHT table to a netlink socket
- @msg: buffer for the message
- @cb: callback structure containing arguments
- Return: Message length on success or a negative error number otherwise.
- */
+int batadv_dat_dht_dump(struct sk_buff *msg, struct netlink_callback *cb) +{
struct net_device *soft_iface;
struct batadv_priv *bat_priv;
int ret;
soft_iface = batadv_dat_get_softif(cb);
if (IS_ERR(soft_iface))
return PTR_ERR(soft_iface);bat_priv = netdev_priv(soft_iface);
ret = batadv_dat_dump(bat_priv, msg, cb, bat_priv->dat.dht_hash,
BATADV_CMD_GET_DAT_DHT);dev_put(soft_iface); return ret; }
@@ -1164,9 +1338,9 @@ bool batadv_dat_snoop_outgoing_arp_request(struct batadv_priv *bat_priv, hw_src = batadv_arp_hw_src(skb, hdr_size); ip_dst = batadv_arp_ip_dst(skb, hdr_size);
- batadv_dat_entry_add(bat_priv, ip_src, hw_src, vid);
- batadv_dat_cache_entry_add(bat_priv, ip_src, hw_src, vid);
- dat_entry = batadv_dat_entry_hash_find(bat_priv, ip_dst, vid);
- dat_entry = batadv_dat_cache_entry_find(bat_priv, ip_dst, vid); if (dat_entry) { /* If the ARP request is destined for a local client the local
- client will answer itself. DAT would only generate a
@@ -1256,9 +1430,9 @@ bool batadv_dat_snoop_incoming_arp_request(struct batadv_priv *bat_priv,
batadv_dbg_arp(bat_priv, skb, hdr_size, "Parsing incoming ARP REQUEST");
- batadv_dat_entry_add(bat_priv, ip_src, hw_src, vid);
- batadv_dat_cache_entry_add(bat_priv, ip_src, hw_src, vid);
- dat_entry = batadv_dat_entry_hash_find(bat_priv, ip_dst, vid);
- dat_entry = batadv_dat_dht_entry_find(bat_priv, ip_dst, vid); if (!dat_entry) goto out;
@@ -1273,7 +1447,7 @@ bool batadv_dat_snoop_incoming_arp_request(struct batadv_priv *bat_priv, */ if (hdr_size == sizeof(struct batadv_unicast_4addr_packet)) err = batadv_send_skb_via_tt_4addr(bat_priv, skb_new,
BATADV_P_DAT_CACHE_REPLY,
BATADV_P_DAT_DHT_REPLY, NULL, vid);@@ -1319,8 +1493,8 @@ void batadv_dat_snoop_outgoing_arp_reply(struct batadv_priv *bat_priv, hw_dst = batadv_arp_hw_dst(skb, hdr_size); ip_dst = batadv_arp_ip_dst(skb, hdr_size);
- batadv_dat_entry_add(bat_priv, ip_src, hw_src, vid);
- batadv_dat_entry_add(bat_priv, ip_dst, hw_dst, vid);
batadv_dat_cache_entry_add(bat_priv, ip_src, hw_src, vid);
batadv_dat_cache_entry_add(bat_priv, ip_dst, hw_dst, vid);
/* Send the ARP reply to the candidates for both the IP addresses that
- the node obtained from the ARP reply
@@ -1342,7 +1516,8 @@ void batadv_dat_snoop_outgoing_arp_reply(struct batadv_priv *bat_priv,
- packet has to be delivered to the interface
*/ bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv,
struct sk_buff *skb, int hdr_size)
struct sk_buff *skb, int hdr_size,bool is_dht_put)@@ -1367,13 +1542,18 @@ bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv, hw_dst = batadv_arp_hw_dst(skb, hdr_size); ip_dst = batadv_arp_ip_dst(skb, hdr_size);
- if (is_dht_put) {
batadv_dat_dht_entry_add(bat_priv, ip_src, hw_src, vid);batadv_dat_dht_entry_add(bat_priv, ip_dst, hw_dst, vid);- }
- /* If ip_dst is already in cache and has the right mac address,
*/
- drop this frame if this ARP reply is destined for us because it's
- most probably an ARP reply generated by another node of the DHT.
- We have most probably received already a reply earlier. Delivering
- this frame would lead to doubled receive of an ARP reply.
- dat_entry = batadv_dat_entry_hash_find(bat_priv, ip_src, vid);
- dat_entry = batadv_dat_cache_entry_find(bat_priv, ip_src, vid); if (dat_entry && batadv_compare_eth(hw_src, dat_entry->mac_addr)) { batadv_dbg(BATADV_DBG_DAT, bat_priv, "Doubled ARP reply removed: ARP MSG = [src: %pM-%pI4 dst: %pM-%pI4]; dat_entry: %pM-%pI4\n", hw_src, &ip_src, hw_dst, &ip_dst,
@@ -1384,8 +1564,8 @@ bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv, /* Update our internal cache with both the IP addresses the node got * within the ARP reply */
- batadv_dat_entry_add(bat_priv, ip_src, hw_src, vid);
- batadv_dat_entry_add(bat_priv, ip_dst, hw_dst, vid);
batadv_dat_cache_entry_add(bat_priv, ip_src, hw_src, vid);
batadv_dat_cache_entry_add(bat_priv, ip_dst, hw_dst, vid);
if (dropped) goto out;
@@ -1641,8 +1821,8 @@ static void batadv_dat_put_dhcp(struct batadv_priv *bat_priv, u8 *chaddr,
skb_set_network_header(skb, ETH_HLEN);
- batadv_dat_entry_add(bat_priv, yiaddr, chaddr, vid);
- batadv_dat_entry_add(bat_priv, ip_dst, hw_dst, vid);
batadv_dat_cache_entry_add(bat_priv, yiaddr, chaddr, vid);
batadv_dat_cache_entry_add(bat_priv, ip_dst, hw_dst, vid);
batadv_dat_forward_data(bat_priv, skb, yiaddr, vid, BATADV_P_DAT_DHT_PUT);
@@ -1767,8 +1947,8 @@ void batadv_dat_snoop_incoming_dhcp_ack(struct batadv_priv *bat_priv, hw_src = ethhdr->h_source; vid = batadv_dat_get_vid(skb, &hdr_size);
- batadv_dat_entry_add(bat_priv, yiaddr, chaddr, vid);
- batadv_dat_entry_add(bat_priv, ip_src, hw_src, vid);
batadv_dat_cache_entry_add(bat_priv, yiaddr, chaddr, vid);
batadv_dat_cache_entry_add(bat_priv, ip_src, hw_src, vid);
batadv_dbg(BATADV_DBG_DAT, bat_priv, "Snooped from incoming DHCPACK (server address): %pI4, %pM (vid: %i)\n",
@@ -1812,7 +1992,7 @@ bool batadv_dat_drop_broadcast_packet(struct batadv_priv *bat_priv, goto out;
ip_dst = batadv_arp_ip_dst(forw_packet->skb, hdr_size);
- dat_entry = batadv_dat_entry_hash_find(bat_priv, ip_dst, vid);
- dat_entry = batadv_dat_cache_entry_find(bat_priv, ip_dst, vid); /* check if the node already got this entry */ if (!dat_entry) { batadv_dbg(BATADV_DBG_DAT, bat_priv,
diff --git a/net/batman-adv/distributed-arp-table.h b/net/batman-adv/distributed-arp-table.h index bed7f3d20844..28ac93b0113d 100644 --- a/net/batman-adv/distributed-arp-table.h +++ b/net/batman-adv/distributed-arp-table.h @@ -31,7 +31,8 @@ bool batadv_dat_snoop_incoming_arp_request(struct batadv_priv *bat_priv, void batadv_dat_snoop_outgoing_arp_reply(struct batadv_priv *bat_priv, struct sk_buff *skb); bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv,
struct sk_buff *skb, int hdr_size);
struct sk_buff *skb, int hdr_size,bool is_dht_put);@@ -74,6 +75,7 @@ batadv_dat_init_own_addr(struct batadv_priv *bat_priv, int batadv_dat_init(struct batadv_priv *bat_priv); void batadv_dat_free(struct batadv_priv *bat_priv); int batadv_dat_cache_dump(struct sk_buff *msg, struct netlink_callback *cb); +int batadv_dat_dht_dump(struct sk_buff *msg, struct netlink_callback *cb);
/**
- batadv_dat_inc_counter() - increment the correct DAT packet counter
@@ -126,7 +128,8 @@ batadv_dat_snoop_outgoing_arp_reply(struct batadv_priv *bat_priv,
static inline bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv,
struct sk_buff *skb, int hdr_size)
struct sk_buff *skb, int hdr_size,bool is_dht_put)@@ -176,6 +179,12 @@ batadv_dat_cache_dump(struct sk_buff *msg, struct netlink_callback *cb) return -EOPNOTSUPP; }
+static inline int +batadv_dat_dht_dump(struct sk_buff *msg, struct netlink_callback *cb) +{
- return -EOPNOTSUPP;
+}
- static inline void batadv_dat_inc_counter(struct batadv_priv *bat_priv, u8 subtype) {
diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c index 3f0003fc502c..c3f8d3511236 100644 --- a/net/batman-adv/netlink.c +++ b/net/batman-adv/netlink.c @@ -1442,6 +1442,11 @@ static const struct genl_small_ops batadv_netlink_ops[] = { .flags = GENL_UNS_ADMIN_PERM, .dumpit = batadv_dat_cache_dump, },
- {
.cmd = BATADV_CMD_GET_DAT_DHT,.flags = GENL_ADMIN_PERM,.dumpit = batadv_dat_dht_dump,- }, { .cmd = BATADV_CMD_GET_MCAST_FLAGS, .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index f1061985149f..b3195a225ccc 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -943,10 +943,10 @@ int batadv_recv_unicast_packet(struct sk_buff *skb, struct batadv_unicast_4addr_packet *unicast_4addr_packet; u8 *orig_addr, *orig_addr_gw; struct batadv_orig_node *orig_node = NULL, *orig_node_gw = NULL;
- bool is4addr, is_gw, is_dht_put = false; int check, hdr_size = sizeof(*unicast_packet); enum batadv_subtype subtype; int ret = NET_RX_DROP;
bool is4addr, is_gw;
unicast_packet = (struct batadv_unicast_packet *)skb->data; is4addr = unicast_packet->packet_type == BATADV_UNICAST_4ADDR;
@@ -1005,6 +1005,8 @@ int batadv_recv_unicast_packet(struct sk_buff *skb, orig_addr = unicast_4addr_packet->src; orig_node = batadv_orig_hash_find(bat_priv, orig_addr);
} else if (subtype == BATADV_P_DAT_DHT_PUT) {is_dht_put = true; }@@ -1012,7 +1014,7 @@ int batadv_recv_unicast_packet(struct sk_buff *skb, hdr_size)) goto rx_success; if (batadv_dat_snoop_incoming_arp_reply(bat_priv, skb,
hdr_size))
hdr_size, is_dht_put)) goto rx_success;batadv_dat_snoop_incoming_dhcp_ack(bat_priv, skb, hdr_size);
@@ -1249,7 +1251,7 @@ int batadv_recv_bcast_packet(struct sk_buff *skb,
if (batadv_dat_snoop_incoming_arp_request(bat_priv, skb, hdr_size)) goto rx_success;
- if (batadv_dat_snoop_incoming_arp_reply(bat_priv, skb, hdr_size))
if (batadv_dat_snoop_incoming_arp_reply(bat_priv, skb, hdr_size, false)) goto rx_success;
batadv_dat_snoop_incoming_dhcp_ack(bat_priv, skb, hdr_size);
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index 00840d5784fe..fe0b9b229bff 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -1231,8 +1231,11 @@ struct batadv_priv_dat { /** @addr: node DAT address */ batadv_dat_addr_t addr;
- /** @hash: hashtable representing the local ARP cache */
- struct batadv_hashtable *hash;
/** @cache_hash: hashtable representing the local ARP cache */
struct batadv_hashtable *cache_hash;
/** @dht_hash: hashtable representing the local DAT DHT */
struct batadv_hashtable *dht_hash;
/** @work: work queue callback item for cache purging */ struct delayed_work work;
I can see that most of the code in this patch is about handling two tables (in a generic fashion) instead of one.
One functional change I am seeing is also that before this patch batman-adv would store/cache any ARP information coming from the mesh. While now this happens only for the DHT PUT. Am I right?
If that's the case, it means we may issue more DHT_GETs (and possibly ARP requests) because we lost a chance to cache a bit more that what the DHT stores. Does it make sense?
Regards,
On Monday, 28 October 2024 14:25:36 CET Antonio Quartulli wrote: [...]
+/**
- batadv_dat_get_softif() - get the soft interface from a netlink callback
- @cb: callback structure containing arguments
- Return: The soft interface on success or an error pointer otherwise.
- */
+static struct net_device *batadv_dat_get_softif(struct netlink_callback *cb) +{
- struct net *net = sock_net(cb->skb->sk);
- struct net_device *soft_iface;
- int ifindex;
- ifindex = batadv_netlink_get_ifindex(cb->nlh,
BATADV_ATTR_MESH_IFINDEX);- if (!ifindex)
return ERR_PTR(-EINVAL);- soft_iface = dev_get_by_index(net, ifindex);
- if (!soft_iface)
return ERR_PTR(-ENODEV);- if (!batadv_softif_is_valid(soft_iface)) {
dev_put(soft_iface);return ERR_PTR(-ENODEV);- }
- return soft_iface;
+}
I don't think this function is DAT specific at all. Moreover, the very same code (which I think you are re-using here) appears in batadv_netlink_dump_hardif().
I think it'd make more sense to factor it out and create a helper out of it (place it in netlink.c?). This way we avoid code duplication.
[I might be wrong but 90% of the work already is in batadv_get_softif_from_info()]
Looks like this was never answered and also not handled in the v6 version of the patch.
[...]
--- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -1231,8 +1231,11 @@ struct batadv_priv_dat { /** @addr: node DAT address */ batadv_dat_addr_t addr;
- /** @hash: hashtable representing the local ARP cache */
- struct batadv_hashtable *hash;
/** @cache_hash: hashtable representing the local ARP cache */
struct batadv_hashtable *cache_hash;
/** @dht_hash: hashtable representing the local DAT DHT */
struct batadv_hashtable *dht_hash;
/** @work: work queue callback item for cache purging */ struct delayed_work work;
I can see that most of the code in this patch is about handling two tables (in a generic fashion) instead of one.
One functional change I am seeing is also that before this patch batman-adv would store/cache any ARP information coming from the mesh. While now this happens only for the DHT PUT. Am I right?
If that's the case, it means we may issue more DHT_GETs (and possibly ARP requests) because we lost a chance to cache a bit more that what the DHT stores. Does it make sense?
Looks like this was never answered.
Kind regards, Sven
Sorry, had overlooked the inline comments in the code. Replies follow below.
On Mon, Oct 28, 2024 at 02:25:36PM +0100, Antonio Quartulli wrote:
[...]
+/**
- batadv_dat_get_softif() - get the soft interface from a netlink callback
- @cb: callback structure containing arguments
- Return: The soft interface on success or an error pointer otherwise.
- */
+static struct net_device *batadv_dat_get_softif(struct netlink_callback *cb) +{
- struct net *net = sock_net(cb->skb->sk);
- struct net_device *soft_iface;
- int ifindex;
- ifindex = batadv_netlink_get_ifindex(cb->nlh,
BATADV_ATTR_MESH_IFINDEX);- if (!ifindex)
return ERR_PTR(-EINVAL);- soft_iface = dev_get_by_index(net, ifindex);
- if (!soft_iface)
return ERR_PTR(-ENODEV);- if (!batadv_softif_is_valid(soft_iface)) {
dev_put(soft_iface);return ERR_PTR(-ENODEV);- }
- return soft_iface;
+}
I don't think this function is DAT specific at all. Moreover, the very same code (which I think you are re-using here) appears in batadv_netlink_dump_hardif().
I think it'd make more sense to factor it out and create a helper out of it (place it in netlink.c?). This way we avoid code duplication.
[I might be wrong but 90% of the work already is in batadv_get_softif_from_info()]
Good idea, will do in v7! There are actually many more places that have basically the same code... :D.
I can see that most of the code in this patch is about handling two tables (in a generic fashion) instead of one.
One functional change I am seeing is also that before this patch batman-adv would store/cache any ARP information coming from the mesh. While now this happens only for the DHT PUT. Am I right?
Partially. For the DAT Cache part, it should be the same. In the batadv_dat_snoop_incoming_* functions we still add to the DAT cache table like we did before, unless I'm missing something.
For the new DAT DHT part, correct, there are the following cases where we don't update this anymore (immediately):
* incoming ARP request: we don't add/update the ARP source to DAT DHT * incoming ARP reply: we only add/update the ARP source+destination to DAT DHT if it was via a DHT PUT * incoming DHCP ACK: we don't add/update to DAT DHT
However these packets must have come from somewhere. So their originator should have sent a DHT PUT, too? If they arrive a bit later, shouldn't be an issue as these packets at least updated the DHT Cache.
If the according DHT PUT got lost on the way and we didn't have these entries in our DHT yet, shouldn't be an issue as we have two more DAT DHT candidates.
The only case where I could see it making a (small?) difference in practice is if a DHT PUT got lost and it would have changed the MAC address for the DAT DHT entry. But I'm not sure if that would justify extra lines of code right now? (For that use-case we should probably also have a majority vote of DAT_CACHE_REPLY from DAT candidates, instead of updating our DAT cache with the first reply we get - but we didn't do that so far either, for simplicity's sake.)
If that's the case, it means we may issue more DHT_GETs (and possibly ARP requests) because we lost a chance to cache a bit more that what the DHT stores. Does it make sense?
I don't think we would issue more DHT_GETs, as we at least update the DAT cache table like we did before. And only if we don't have an entry in the DAT Cache then we would trigger DHT_GETs?
Hope that makes sense?
Regards, Linus
Currently, the DHT_GET messages of the DAT will likely be left unanswered due to the following issues:
When a node has a matching DAT Cache entry for a local ARP Request then this node will answer it directly with the information provided by the cache. This however, will likely lead to missing ARP Replies from the original host. Which in turn leads to the DAT DHT not being updated.
Then the local DAT cache entry will time out, triggering a unicasted DHT_GET. However, as the 5min. timeout has passed, the DAT DHT candidates will likely have purged their entry, too.
So basically this results in an ARP Request broadcast fallback every five minutes.
A second issue is that it is quite common that a host which has long gone offline will be tried to be contacted by another one at some remote period larger than the current 5min. timeout. This too leads to flooded ARP Requests.
With this patch the purge timeout for DAT DHT entries is increased to 30min to reduce the number of DAT ARP broadcast fallbacks.
Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue --- net/batman-adv/distributed-arp-table.c | 22 +++++++++++++++++----- net/batman-adv/main.h | 3 ++- 2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c index 8a054b0cf2a2..1368bba93169 100644 --- a/net/batman-adv/distributed-arp-table.c +++ b/net/batman-adv/distributed-arp-table.c @@ -133,15 +133,27 @@ static void batadv_dat_entry_put(struct batadv_dat_entry *dat_entry) }
/** - * batadv_dat_to_purge() - check whether a dat_entry has to be purged or not + * batadv_dat_cache_to_purge() - check if a cache entry has to be purged or not * @dat_entry: the entry to check * * Return: true if the entry has to be purged now, false otherwise. */ -static bool batadv_dat_to_purge(struct batadv_dat_entry *dat_entry) +static bool batadv_dat_cache_to_purge(struct batadv_dat_entry *dat_entry) { return batadv_has_timed_out(dat_entry->last_update, - BATADV_DAT_ENTRY_TIMEOUT); + BATADV_DAT_CACHE_ENTRY_TIMEOUT); +} + +/** + * batadv_dat_dht_to_purge() - check if a DHT entry has to be purged or not + * @dat_entry: the entry to check + * + * Return: true if the entry has to be purged now, false otherwise. + */ +static bool batadv_dat_dht_to_purge(struct batadv_dat_entry *dat_entry) +{ + return batadv_has_timed_out(dat_entry->last_update, + BATADV_DAT_DHT_ENTRY_TIMEOUT); }
/** @@ -202,8 +214,8 @@ static void batadv_dat_purge(struct work_struct *work) priv_dat = container_of(delayed_work, struct batadv_priv_dat, work); bat_priv = container_of(priv_dat, struct batadv_priv, dat);
- __batadv_dat_purge(bat_priv->dat.cache_hash, batadv_dat_to_purge); - __batadv_dat_purge(bat_priv->dat.dht_hash, batadv_dat_to_purge); + __batadv_dat_purge(bat_priv->dat.cache_hash, batadv_dat_cache_to_purge); + __batadv_dat_purge(bat_priv->dat.dht_hash, batadv_dat_dht_to_purge); batadv_dat_start_timer(bat_priv); }
diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h index 97ea71a052f8..f04bce4eaf8c 100644 --- a/net/batman-adv/main.h +++ b/net/batman-adv/main.h @@ -38,7 +38,8 @@ #define BATADV_TT_WORK_PERIOD 5000 /* 5 seconds */ #define BATADV_ORIG_WORK_PERIOD 1000 /* 1 second */ #define BATADV_MCAST_WORK_PERIOD 500 /* 0.5 seconds */ -#define BATADV_DAT_ENTRY_TIMEOUT (5 * 60000) /* 5 mins in milliseconds */ +#define BATADV_DAT_CACHE_ENTRY_TIMEOUT (5 * 60000) /* 5 mins in milliseconds */ +#define BATADV_DAT_DHT_ENTRY_TIMEOUT (30 * 60000) /* 30 mins in milliseconds */ /* sliding packet range of received originator messages in sequence numbers * (should be a multiple of our word size) */
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
Linus Lüssing -
Sven Eckelmann