batman-adv: Add missing source-if info in vis raw
Commit Message
With this patch, a line for each _source interface_ for every node will
be used in the vis raw output instead of one line per originator. This
stops discarding valuable information about secondary interfaces.
Signed-off-by: Linus Lüssing <linus.luessing@web.de>
---
proc.c | 33 ++++++++++++++++++++++++++-------
vis.c | 25 +++++++++----------------
vis.h | 7 +++++--
3 files changed, 40 insertions(+), 25 deletions(-)
Comments
On Tue, Mar 09, 2010 at 08:15:17AM +0100, Linus L??ssing wrote:
> With this patch, a line for each _source interface_ for every node will
> be used in the vis raw output instead of one line per originator. This
> stops discarding valuable information about secondary interfaces.
Hi Linus
Thanks for working on this bug. Since this is a bug which we want to
fix outside of the merge window, i guess the change log entry needs to
sound more bug like and not development work. From your comment it is
hard to tell it is a bug. How about this which uses the word bug and
fix.
Staging: batman-adv: Fix VIS output bug for secondary interfaces
TQ and HNA records for originators on secondary interfaces were
wrongly being included on the primary interface. Ensure we output a
line for each source interface on every node, so we correctly separate
primary and secondary interface records.
Since we have never submitted a bug fix out of the merge window, i've
no idea if this is really necessary...
Andrew
>
> Signed-off-by: Linus L??ssing <linus.luessing@web.de>
> ---
> proc.c | 33 ++++++++++++++++++++++++++-------
> vis.c | 25 +++++++++----------------
> vis.h | 7 +++++--
> 3 files changed, 40 insertions(+), 25 deletions(-)
>
> diff --git a/proc.c b/proc.c
> index 1c12613..ab5047f 100644
> --- a/proc.c
> +++ b/proc.c
> @@ -370,6 +370,8 @@ static int proc_vis_data_read(struct seq_file *seq, void *offset)
> struct vis_info *info;
> struct vis_info_entry *entries;
> HLIST_HEAD(vis_if_list);
> + struct if_list_entry *entry;
> + struct hlist_node *pos, *n;
> int i;
> char tmp_addr_str[ETH_STR_LEN];
> unsigned long flags;
> @@ -388,17 +390,34 @@ static int proc_vis_data_read(struct seq_file *seq, void *offset)
> info = hashit.bucket->data;
> entries = (struct vis_info_entry *)
> ((char *)info + sizeof(struct vis_info));
> - addr_to_string(tmp_addr_str, info->packet.vis_orig);
> - seq_printf(seq, "%s,", tmp_addr_str);
>
> for (i = 0; i < info->packet.entries; i++) {
> - proc_vis_read_entry(seq, &entries[i], &vis_if_list,
> - info->packet.vis_orig);
> + if (entries[i].quality == 0)
> + continue;
> + proc_vis_insert_interface(entries[i].src, &vis_if_list,
> + compare_orig(entries[i].src,
> + info->packet.vis_orig));
> }
>
> - /* add primary/secondary records */
> - proc_vis_read_prim_sec(seq, &vis_if_list);
> - seq_printf(seq, "\n");
> + hlist_for_each_entry(entry, pos, &vis_if_list, list) {
> + addr_to_string(tmp_addr_str, entry->addr);
> + seq_printf(seq, "%s,", tmp_addr_str);
> +
> + for (i = 0; i < info->packet.entries; i++)
> + proc_vis_read_entry(seq, &entries[i],
> + entry->addr, entry->primary);
> +
> + /* add primary/secondary records */
> + if (compare_orig(entry->addr, info->packet.vis_orig))
> + proc_vis_read_prim_sec(seq, &vis_if_list);
> +
> + seq_printf(seq, "\n");
> + }
> +
> + hlist_for_each_entry_safe(entry, pos, n, &vis_if_list, list) {
> + hlist_del(&entry->list);
> + kfree(entry);
> + }
> }
> spin_unlock_irqrestore(&vis_hash_lock, flags);
>
> diff --git a/vis.c b/vis.c
> index 4f1339a..fa8a487 100644
> --- a/vis.c
> +++ b/vis.c
> @@ -87,7 +87,7 @@ static int vis_info_choose(void *data, int size)
>
> /* insert interface to the list of interfaces of one originator, if it
> * does not already exist in the list */
> -static void proc_vis_insert_interface(const uint8_t *interface,
> +void proc_vis_insert_interface(const uint8_t *interface,
> struct hlist_head *if_list,
> bool primary)
> {
> @@ -112,39 +112,32 @@ void proc_vis_read_prim_sec(struct seq_file *seq,
> struct hlist_head *if_list)
> {
> struct if_list_entry *entry;
> - struct hlist_node *pos, *n;
> + struct hlist_node *pos;
> char tmp_addr_str[ETH_STR_LEN];
>
> - hlist_for_each_entry_safe(entry, pos, n, if_list, list) {
> - if (entry->primary) {
> + hlist_for_each_entry(entry, pos, if_list, list) {
> + if (entry->primary)
> seq_printf(seq, "PRIMARY, ");
> - } else {
> + else {
> addr_to_string(tmp_addr_str, entry->addr);
> seq_printf(seq, "SEC %s, ", tmp_addr_str);
> }
> -
> - hlist_del(&entry->list);
> - kfree(entry);
> }
> }
>
> /* read an entry */
> void proc_vis_read_entry(struct seq_file *seq,
> struct vis_info_entry *entry,
> - struct hlist_head *if_list,
> - uint8_t *vis_orig)
> + uint8_t *src,
> + bool primary)
> {
> char to[40];
>
> addr_to_string(to, entry->dest);
> - if (entry->quality == 0) {
> - proc_vis_insert_interface(vis_orig, if_list, true);
> + if (primary && entry->quality == 0)
> seq_printf(seq, "HNA %s, ", to);
> - } else {
> - proc_vis_insert_interface(entry->src, if_list,
> - compare_orig(entry->src, vis_orig));
> + else if (compare_orig(entry->src, src))
> seq_printf(seq, "TQ %s %d, ", to, entry->quality);
> - }
> }
>
> /* add the info packet to the send list, if it was not
> diff --git a/vis.h b/vis.h
> index 465da47..a1f92a4 100644
> --- a/vis.h
> +++ b/vis.h
> @@ -49,10 +49,13 @@ struct recvlist_node {
> extern struct hashtable_t *vis_hash;
> extern spinlock_t vis_hash_lock;
>
> +void proc_vis_insert_interface(const uint8_t *interface,
> + struct hlist_head *if_list,
> + bool primary);
> void proc_vis_read_entry(struct seq_file *seq,
> struct vis_info_entry *entry,
> - struct hlist_head *if_list,
> - uint8_t *vis_orig);
> + uint8_t *src,
> + bool primary);
> void proc_vis_read_prim_sec(struct seq_file *seq,
> struct hlist_head *if_list);
> void receive_server_sync_packet(struct vis_packet *vis_packet,
> --
> 1.7.0
>
Hi Andrew,
Your adjusted commit message sounds good to me, sure, we can
use that one instead :).
Cheers, Linus
On Wed, Mar 10, 2010 at 07:35:08AM +0100, Andrew Lunn wrote:
> On Tue, Mar 09, 2010 at 08:15:17AM +0100, Linus L??ssing wrote:
> > With this patch, a line for each _source interface_ for every node will
> > be used in the vis raw output instead of one line per originator. This
> > stops discarding valuable information about secondary interfaces.
>
> Hi Linus
>
> Thanks for working on this bug. Since this is a bug which we want to
> fix outside of the merge window, i guess the change log entry needs to
> sound more bug like and not development work. From your comment it is
> hard to tell it is a bug. How about this which uses the word bug and
> fix.
>
> Staging: batman-adv: Fix VIS output bug for secondary interfaces
>
> TQ and HNA records for originators on secondary interfaces were
> wrongly being included on the primary interface. Ensure we output a
> line for each source interface on every node, so we correctly separate
> primary and secondary interface records.
>
> Since we have never submitted a bug fix out of the merge window, i've
> no idea if this is really necessary...
>
> Andrew
>
> >
> > Signed-off-by: Linus L??ssing <linus.luessing@web.de>
> > ---
> > proc.c | 33 ++++++++++++++++++++++++++-------
> > vis.c | 25 +++++++++----------------
> > vis.h | 7 +++++--
> > 3 files changed, 40 insertions(+), 25 deletions(-)
> >
> > diff --git a/proc.c b/proc.c
> > index 1c12613..ab5047f 100644
> > --- a/proc.c
> > +++ b/proc.c
> > @@ -370,6 +370,8 @@ static int proc_vis_data_read(struct seq_file *seq, void *offset)
> > struct vis_info *info;
> > struct vis_info_entry *entries;
> > HLIST_HEAD(vis_if_list);
> > + struct if_list_entry *entry;
> > + struct hlist_node *pos, *n;
> > int i;
> > char tmp_addr_str[ETH_STR_LEN];
> > unsigned long flags;
> > @@ -388,17 +390,34 @@ static int proc_vis_data_read(struct seq_file *seq, void *offset)
> > info = hashit.bucket->data;
> > entries = (struct vis_info_entry *)
> > ((char *)info + sizeof(struct vis_info));
> > - addr_to_string(tmp_addr_str, info->packet.vis_orig);
> > - seq_printf(seq, "%s,", tmp_addr_str);
> >
> > for (i = 0; i < info->packet.entries; i++) {
> > - proc_vis_read_entry(seq, &entries[i], &vis_if_list,
> > - info->packet.vis_orig);
> > + if (entries[i].quality == 0)
> > + continue;
> > + proc_vis_insert_interface(entries[i].src, &vis_if_list,
> > + compare_orig(entries[i].src,
> > + info->packet.vis_orig));
> > }
> >
> > - /* add primary/secondary records */
> > - proc_vis_read_prim_sec(seq, &vis_if_list);
> > - seq_printf(seq, "\n");
> > + hlist_for_each_entry(entry, pos, &vis_if_list, list) {
> > + addr_to_string(tmp_addr_str, entry->addr);
> > + seq_printf(seq, "%s,", tmp_addr_str);
> > +
> > + for (i = 0; i < info->packet.entries; i++)
> > + proc_vis_read_entry(seq, &entries[i],
> > + entry->addr, entry->primary);
> > +
> > + /* add primary/secondary records */
> > + if (compare_orig(entry->addr, info->packet.vis_orig))
> > + proc_vis_read_prim_sec(seq, &vis_if_list);
> > +
> > + seq_printf(seq, "\n");
> > + }
> > +
> > + hlist_for_each_entry_safe(entry, pos, n, &vis_if_list, list) {
> > + hlist_del(&entry->list);
> > + kfree(entry);
> > + }
> > }
> > spin_unlock_irqrestore(&vis_hash_lock, flags);
> >
> > diff --git a/vis.c b/vis.c
> > index 4f1339a..fa8a487 100644
> > --- a/vis.c
> > +++ b/vis.c
> > @@ -87,7 +87,7 @@ static int vis_info_choose(void *data, int size)
> >
> > /* insert interface to the list of interfaces of one originator, if it
> > * does not already exist in the list */
> > -static void proc_vis_insert_interface(const uint8_t *interface,
> > +void proc_vis_insert_interface(const uint8_t *interface,
> > struct hlist_head *if_list,
> > bool primary)
> > {
> > @@ -112,39 +112,32 @@ void proc_vis_read_prim_sec(struct seq_file *seq,
> > struct hlist_head *if_list)
> > {
> > struct if_list_entry *entry;
> > - struct hlist_node *pos, *n;
> > + struct hlist_node *pos;
> > char tmp_addr_str[ETH_STR_LEN];
> >
> > - hlist_for_each_entry_safe(entry, pos, n, if_list, list) {
> > - if (entry->primary) {
> > + hlist_for_each_entry(entry, pos, if_list, list) {
> > + if (entry->primary)
> > seq_printf(seq, "PRIMARY, ");
> > - } else {
> > + else {
> > addr_to_string(tmp_addr_str, entry->addr);
> > seq_printf(seq, "SEC %s, ", tmp_addr_str);
> > }
> > -
> > - hlist_del(&entry->list);
> > - kfree(entry);
> > }
> > }
> >
> > /* read an entry */
> > void proc_vis_read_entry(struct seq_file *seq,
> > struct vis_info_entry *entry,
> > - struct hlist_head *if_list,
> > - uint8_t *vis_orig)
> > + uint8_t *src,
> > + bool primary)
> > {
> > char to[40];
> >
> > addr_to_string(to, entry->dest);
> > - if (entry->quality == 0) {
> > - proc_vis_insert_interface(vis_orig, if_list, true);
> > + if (primary && entry->quality == 0)
> > seq_printf(seq, "HNA %s, ", to);
> > - } else {
> > - proc_vis_insert_interface(entry->src, if_list,
> > - compare_orig(entry->src, vis_orig));
> > + else if (compare_orig(entry->src, src))
> > seq_printf(seq, "TQ %s %d, ", to, entry->quality);
> > - }
> > }
> >
> > /* add the info packet to the send list, if it was not
> > diff --git a/vis.h b/vis.h
> > index 465da47..a1f92a4 100644
> > --- a/vis.h
> > +++ b/vis.h
> > @@ -49,10 +49,13 @@ struct recvlist_node {
> > extern struct hashtable_t *vis_hash;
> > extern spinlock_t vis_hash_lock;
> >
> > +void proc_vis_insert_interface(const uint8_t *interface,
> > + struct hlist_head *if_list,
> > + bool primary);
> > void proc_vis_read_entry(struct seq_file *seq,
> > struct vis_info_entry *entry,
> > - struct hlist_head *if_list,
> > - uint8_t *vis_orig);
> > + uint8_t *src,
> > + bool primary);
> > void proc_vis_read_prim_sec(struct seq_file *seq,
> > struct hlist_head *if_list);
> > void receive_server_sync_packet(struct vis_packet *vis_packet,
> > --
> > 1.7.0
> >
>
@@ -370,6 +370,8 @@ static int proc_vis_data_read(struct seq_file *seq, void *offset)
struct vis_info *info;
struct vis_info_entry *entries;
HLIST_HEAD(vis_if_list);
+ struct if_list_entry *entry;
+ struct hlist_node *pos, *n;
int i;
char tmp_addr_str[ETH_STR_LEN];
unsigned long flags;
@@ -388,17 +390,34 @@ static int proc_vis_data_read(struct seq_file *seq, void *offset)
info = hashit.bucket->data;
entries = (struct vis_info_entry *)
((char *)info + sizeof(struct vis_info));
- addr_to_string(tmp_addr_str, info->packet.vis_orig);
- seq_printf(seq, "%s,", tmp_addr_str);
for (i = 0; i < info->packet.entries; i++) {
- proc_vis_read_entry(seq, &entries[i], &vis_if_list,
- info->packet.vis_orig);
+ if (entries[i].quality == 0)
+ continue;
+ proc_vis_insert_interface(entries[i].src, &vis_if_list,
+ compare_orig(entries[i].src,
+ info->packet.vis_orig));
}
- /* add primary/secondary records */
- proc_vis_read_prim_sec(seq, &vis_if_list);
- seq_printf(seq, "\n");
+ hlist_for_each_entry(entry, pos, &vis_if_list, list) {
+ addr_to_string(tmp_addr_str, entry->addr);
+ seq_printf(seq, "%s,", tmp_addr_str);
+
+ for (i = 0; i < info->packet.entries; i++)
+ proc_vis_read_entry(seq, &entries[i],
+ entry->addr, entry->primary);
+
+ /* add primary/secondary records */
+ if (compare_orig(entry->addr, info->packet.vis_orig))
+ proc_vis_read_prim_sec(seq, &vis_if_list);
+
+ seq_printf(seq, "\n");
+ }
+
+ hlist_for_each_entry_safe(entry, pos, n, &vis_if_list, list) {
+ hlist_del(&entry->list);
+ kfree(entry);
+ }
}
spin_unlock_irqrestore(&vis_hash_lock, flags);
@@ -87,7 +87,7 @@ static int vis_info_choose(void *data, int size)
/* insert interface to the list of interfaces of one originator, if it
* does not already exist in the list */
-static void proc_vis_insert_interface(const uint8_t *interface,
+void proc_vis_insert_interface(const uint8_t *interface,
struct hlist_head *if_list,
bool primary)
{
@@ -112,39 +112,32 @@ void proc_vis_read_prim_sec(struct seq_file *seq,
struct hlist_head *if_list)
{
struct if_list_entry *entry;
- struct hlist_node *pos, *n;
+ struct hlist_node *pos;
char tmp_addr_str[ETH_STR_LEN];
- hlist_for_each_entry_safe(entry, pos, n, if_list, list) {
- if (entry->primary) {
+ hlist_for_each_entry(entry, pos, if_list, list) {
+ if (entry->primary)
seq_printf(seq, "PRIMARY, ");
- } else {
+ else {
addr_to_string(tmp_addr_str, entry->addr);
seq_printf(seq, "SEC %s, ", tmp_addr_str);
}
-
- hlist_del(&entry->list);
- kfree(entry);
}
}
/* read an entry */
void proc_vis_read_entry(struct seq_file *seq,
struct vis_info_entry *entry,
- struct hlist_head *if_list,
- uint8_t *vis_orig)
+ uint8_t *src,
+ bool primary)
{
char to[40];
addr_to_string(to, entry->dest);
- if (entry->quality == 0) {
- proc_vis_insert_interface(vis_orig, if_list, true);
+ if (primary && entry->quality == 0)
seq_printf(seq, "HNA %s, ", to);
- } else {
- proc_vis_insert_interface(entry->src, if_list,
- compare_orig(entry->src, vis_orig));
+ else if (compare_orig(entry->src, src))
seq_printf(seq, "TQ %s %d, ", to, entry->quality);
- }
}
/* add the info packet to the send list, if it was not
@@ -49,10 +49,13 @@ struct recvlist_node {
extern struct hashtable_t *vis_hash;
extern spinlock_t vis_hash_lock;
+void proc_vis_insert_interface(const uint8_t *interface,
+ struct hlist_head *if_list,
+ bool primary);
void proc_vis_read_entry(struct seq_file *seq,
struct vis_info_entry *entry,
- struct hlist_head *if_list,
- uint8_t *vis_orig);
+ uint8_t *src,
+ bool primary);
void proc_vis_read_prim_sec(struct seq_file *seq,
struct hlist_head *if_list);
void receive_server_sync_packet(struct vis_packet *vis_packet,