Marek Lindner wrote:
> +static void softif_neigh_hold(struct softif_neigh *softif_neigh)
> +{
> + if (!softif_neigh)
> + return;
> +
> + atomic_inc(&softif_neigh->refcnt);
> +}
> +
> +static void softif_neigh_put(struct softif_neigh *softif_neigh)
> +{
> + if (!softif_neigh)
> + return;
> +
> + if (atomic_dec_and_test(&softif_neigh->refcnt))
> + kfree(softif_neigh);
> +}
Do you want to change the other _hold and _put functions to make it consistent
in batman-adv regarding the NULL pointer checks?
> +void softif_neigh_purge(struct bat_priv *bat_priv)
> +{
> + struct softif_neigh *softif_neigh;
> + struct hlist_node *node, *node_tmp;
> +
> + hlist_for_each_entry_safe(softif_neigh, node, node_tmp,
> + &bat_priv->softif_neigh_list, list) {
> +
> + if (!time_after(jiffies, softif_neigh->last_seen +
> + msecs_to_jiffies(SOFTIF_NEIGH_TIMEOUT)))
> + continue;
> +
> + spin_lock(&bat_priv->softif_neigh_lock);
> + hlist_del_rcu(&softif_neigh->list);
> + spin_unlock(&bat_priv->softif_neigh_lock);
> +
> + if (bat_priv->softif_neigh == softif_neigh) {
> + bat_priv->softif_neigh = NULL;
> + bat_dbg(DBG_ROUTES, bat_priv,
> + "Current mesh exit point '%pM' vanished "
> + "(vid: %d).\n",
> + softif_neigh->addr, softif_neigh->vid);
> + softif_neigh_put(bat_priv->softif_neigh);
> + }
> +
> + synchronize_rcu();
> + softif_neigh_put(softif_neigh);
> + }
You must protect the whole loop and not only the hlist_del_rcu with the
bat_priv->softif_neigh_lock, And it must be with spin_lock_irqsave because you
use it in context were irq is disabled and in some with enabled irq
contexts... or am I wrong?
> +void softif_batman_recv(struct sk_buff *skb, struct net_device *dev, short
> vid) +{
> + struct bat_priv *bat_priv = netdev_priv(dev);
> + struct ethhdr *ethhdr = (struct ethhdr *)skb->data;
> + struct batman_packet *batman_packet;
> + struct softif_neigh *softif_neigh;
> +
> + if (ntohs(ethhdr->h_proto) == ETH_P_8021Q)
> + batman_packet = (struct batman_packet *)
> + (skb->data + ETH_HLEN + VLAN_HLEN);
> + else
> + batman_packet = (struct batman_packet *)(skb->data + ETH_HLEN);
> +
> + if (batman_packet->version != COMPAT_VERSION)
> + goto err;
> +
> + if (batman_packet->packet_type != BAT_PACKET)
> + goto err;
> +
> + if (!(batman_packet->flags & PRIMARIES_FIRST_HOP))
> + goto err;
> +
> + if (is_my_mac(batman_packet->orig))
> + goto err;
> +
> + softif_neigh = softif_neigh_get(bat_priv, batman_packet->orig, vid);
> +
> + if (!softif_neigh)
> + goto err;
> +
> + if (bat_priv->softif_neigh == softif_neigh)
> + goto out;
> +
> + /* we got a neighbor but its mac is 'bigger' than ours */
> + if (memcmp(bat_priv->primary_if->net_dev->dev_addr,
> + softif_neigh->addr, ETH_ALEN) < 0)
> + goto out;
> +
> + /* switch to new 'smallest neighbor' */
> + if ((bat_priv->softif_neigh) &&
> + (memcmp(softif_neigh->addr, bat_priv->softif_neigh->addr,
> + ETH_ALEN) < 0)) {
> + bat_dbg(DBG_ROUTES, bat_priv,
> + "Changing mesh exit point from %pM (vid: %d) "
> + "to %pM (vid: %d).\n",
> + bat_priv->softif_neigh->addr,
> + bat_priv->softif_neigh->vid,
> + softif_neigh->addr, softif_neigh->vid);
> + softif_neigh_put(bat_priv->softif_neigh);
> + bat_priv->softif_neigh = softif_neigh;
> + /* we need to hold the additional reference */
> + goto err;
> + }
> +
> + /* close own batX device and use softif_neigh as exit node */
> + if ((!bat_priv->softif_neigh) &&
> + (memcmp(softif_neigh->addr,
> + bat_priv->primary_if->net_dev->dev_addr, ETH_ALEN) < 0)) {
> + bat_dbg(DBG_ROUTES, bat_priv,
> + "Setting mesh exit point to %pM (vid: %d).\n",
> + softif_neigh->addr, softif_neigh->vid);
> + bat_priv->softif_neigh = softif_neigh;
> + /* we need to hold the additional reference */
> + goto err;
> + }
> +
> +out:
> + softif_neigh_put(softif_neigh);
> +err:
> + kfree_skb(skb);
> + return;
> +}
Can you mark that function as static?
Best regards,
Sven
signature.asc
Description: This is a digitally signed message part.
