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