From: Simon Wunderlich <simon.wunderlich@s2003.tu-chemnitz.de>
To: The list for a Better Approach To Mobile Ad-hoc Networking
<b.a.t.m.a.n@lists.open-mesh.org>
Subject: Re: [B.A.T.M.A.N.] [PATCH 07/19] batman-adv: Prepare and send own multicast tracker packets
Date: Sat, 22 Jan 2011 23:00:50 +0100 [thread overview]
Message-ID: <20110122220050.GA11480@pandem0nium> (raw)
In-Reply-To: <1295659302-7171-8-git-send-email-linus.luessing@saxnet.de>
[-- Attachment #1: Type: text/plain, Size: 4520 bytes --]
Hey Linus,
please see some comments inline.
On Sat, Jan 22, 2011 at 02:21:30AM +0100, Linus Lüssing wrote:
> [...]
>
> +static inline int find_mca_match(struct orig_node *orig_node,
> + int mca_pos, uint8_t *mc_addr_list, int num_mcast_entries)
> +{
> + int pos;
> +
> + for (pos = 0; pos < num_mcast_entries; pos++)
> + if (!memcmp(&mc_addr_list[pos*ETH_ALEN],
> + &orig_node->mca_buff[ETH_ALEN*mca_pos], ETH_ALEN))
> + return pos;
> + return -1;
> +}
A comment explaining this function find_mca_match() would be nice.
> +
> +/**
> + * Prepares a multicast tracker packet on a multicast member with all its
> + * groups and their members attached. Note, that the proactive tracking
> + * mode does not differentiate between multicast senders and receivers,
> + * resulting in tracker packets between each node.
> + *
> + * Returns NULL if this node is not a member of any group or if there are
> + * no other members in its groups.
> + *
> + * @bat_priv: bat_priv for the mesh we are preparing this packet
> + */
> +static struct mcast_tracker_packet *mcast_proact_tracker_prepare(
> + struct bat_priv *bat_priv, int *tracker_packet_len)
> +{
> + struct net_device *soft_iface = bat_priv->primary_if->soft_iface;
> + uint8_t *mc_addr_list;
> + MC_LIST *mc_entry;
> + struct element_t *bucket;
> + struct orig_node *orig_node;
> + struct hashtable_t *hash = bat_priv->orig_hash;
> + struct hlist_node *walk;
> + struct hlist_head *head;
> + int i;
> +
> + /* one dest_entries_list per multicast group,
> + * they'll collect dest_entries[x] */
> + int num_mcast_entries, used_mcast_entries = 0;
> + struct list_head *dest_entries_list;
> + struct dest_entries_list dest_entries[UINT8_MAX], *dest, *tmp;
This will reserve 256 * 18 = 4608 byte on the stack, which
is too much for 4k stacks. Please allocate this somewhere else.
> + int num_dest_entries, dest_entries_total = 0;
> +
> + uint8_t *dest_entry;
> + int pos, mca_pos;
> + struct mcast_tracker_packet *tracker_packet = NULL;
> + struct mcast_entry *mcast_entry;
> +
> + if (!hash)
> + goto out;
> +
> + /* Make a copy so we don't have to rush because of locking */
> + netif_addr_lock_bh(soft_iface);
> + num_mcast_entries = netdev_mc_count(soft_iface);
> + mc_addr_list = kmalloc(ETH_ALEN * num_mcast_entries, GFP_ATOMIC);
> + if (!mc_addr_list) {
> + netif_addr_unlock_bh(soft_iface);
> + goto out;
> + }
> + pos = 0;
> + netdev_for_each_mc_addr(mc_entry, soft_iface) {
> + memcpy(&mc_addr_list[pos * ETH_ALEN], mc_entry->MC_LIST_ADDR,
> + ETH_ALEN);
> + pos++;
> + }
> + netif_addr_unlock_bh(soft_iface);
> +
> + if (num_mcast_entries > UINT8_MAX)
> + num_mcast_entries = UINT8_MAX;
> + dest_entries_list = kmalloc(num_mcast_entries *
> + sizeof(struct list_head), GFP_ATOMIC);
> + if (!dest_entries_list)
> + goto free;
> +
> + for (pos = 0; pos < num_mcast_entries; pos++)
> + INIT_LIST_HEAD(&dest_entries_list[pos]);
> +
dest_entries[...].list should be initialized here too, shouldn't they?
BTW, the names a re a little bit confusing (dest_entries_list vs. dest_entries),
other names and an explanation would be helpful.
> + /* fill the lists and buffers */
> + for (i = 0; i < hash->size; i++) {
> + head = &hash->table[i];
> +
> + rcu_read_lock();
> + hlist_for_each_entry_rcu(bucket, walk, head, hlist) {
> + orig_node = bucket->data;
> + if (!orig_node->num_mca)
> + continue;
> +
> + num_dest_entries = 0;
> + for (mca_pos = 0; mca_pos < orig_node->num_mca &&
> + dest_entries_total != UINT8_MAX; mca_pos++) {
> + pos = find_mca_match(orig_node, mca_pos,
> + mc_addr_list, num_mcast_entries);
> + if (pos > UINT8_MAX || pos < 0)
Shouldn't this rather be
if (pos >= num_mcast_entries || pos < 0)
(it is used for dest_entries_list after all).
> + continue;
> + memcpy(dest_entries[dest_entries_total].dest,
> + orig_node->orig, ETH_ALEN);
> + list_add(
> + &dest_entries[dest_entries_total].list,
> + &dest_entries_list[pos]);
> +
> + num_dest_entries++;
num_dest_entries is obsolete IMHO, it is only increase but never used.
> + dest_entries_total++;
> + }
> + }
> + rcu_read_unlock();
> + }
> +
> + /* Any list left empty? */
> + for (pos = 0; pos < num_mcast_entries; pos++)
> + if (!list_empty(&dest_entries_list[pos]))
> + used_mcast_entries++;
> +
> + if (!used_mcast_entries)
>
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
next prev parent reply other threads:[~2011-01-22 22:00 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-22 1:21 [B.A.T.M.A.N.] B.A.T.M.A.N.-Advanced Multicast Optimizations, v2 Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 01/19] batman-adv: Add packet structures for multicast optimizations Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 02/19] batman-adv: Adding configurable variables " Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 03/19] batman-adv: compat macros/defines for local multicast group fetching Linus Lüssing
2011-01-23 0:50 ` Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 04/19] batman-adv: Attach local MCAs to OGMs Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 05/19] batman-adv: Add periodic multicast tracker timer Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 06/19] batman-adv: Buffer other originator's received MCA entries Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 07/19] batman-adv: Prepare and send own multicast tracker packets Linus Lüssing
2011-01-22 22:00 ` Simon Wunderlich [this message]
2011-01-30 4:49 ` Linus Lüssing
2011-01-23 0:52 ` Linus Lüssing
2011-01-23 2:52 ` Simon Wunderlich
2011-01-23 3:00 ` Simon Wunderlich
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 08/19] batman-adv: Add length check for (received) " Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 09/19] batman-adv: Route multicast " Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 10/19] batman-adv: Add/refresh entries to/in mcast forwarding table Linus Lüssing
2011-01-22 22:13 ` Simon Wunderlich
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 11/19] batman-adv: Output mcast forw table in debugfs Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 12/19] batman-adv: Purge timeouted entries in mcast forw table Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 13/19] batman-adv: Send own BAT_MCAST packets in proact_tracking multicast mode Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 14/19] batman-adv: Export broadcast packet ethernet header checks Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 15/19] batman-adv: Receive multicast data packets BAT_MCAST Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 16/19] batman-adv: Forward multicast data in proact_tracking mode Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 17/19] batman-adv: Add duplicate checks for multicast data Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 18/19] batman-adv: Still flood multicast packets we are not a receiver of Linus Lüssing
2011-01-22 1:21 ` [B.A.T.M.A.N.] [PATCH 19/19] batman-adv: Make number of (re)broadcasts configurable via sysfs Linus Lüssing
2011-01-22 16:16 ` [B.A.T.M.A.N.] B.A.T.M.A.N.-Advanced Multicast Optimizations, v2 Linus Lüssing
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110122220050.GA11480@pandem0nium \
--to=simon.wunderlich@s2003.tu-chemnitz.de \
--cc=b.a.t.m.a.n@lists.open-mesh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).