Message ID | 1368793673-27539-2-git-send-email-mihail.costea2005@gmail.com |
---|---|
State | RFC, archived |
Headers |
Return-Path: <mihail.costea2005@gmail.com> Received: from mail-ee0-f41.google.com (mail-ee0-f41.google.com [74.125.83.41]) by open-mesh.org (Postfix) with ESMTPS id 8A72E601F19 for <b.a.t.m.a.n@lists.open-mesh.org>; Fri, 17 May 2013 14:28:48 +0200 (CEST) Received: by mail-ee0-f41.google.com with SMTP id d4so2443063eek.14 for <b.a.t.m.a.n@lists.open-mesh.org>; Fri, 17 May 2013 05:28:48 -0700 (PDT) X-Received: by 10.15.94.131 with SMTP id bb3mr77547250eeb.20.1368793728276; Fri, 17 May 2013 05:28:48 -0700 (PDT) Received: from cmihail-VirtualBox.labs.cs.pub.ro ([141.85.225.204]) by mx.google.com with ESMTPSA id m48sm18028623eeh.16.2013.05.17.05.28.46 for <multiple recipients> (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 17 May 2013 05:28:47 -0700 (PDT) From: Mihail <mihail.costea2005@gmail.com> To: b.a.t.m.a.n@lists.open-mesh.org Date: Fri, 17 May 2013 15:27:50 +0300 Message-Id: <1368793673-27539-2-git-send-email-mihail.costea2005@gmail.com> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1368793673-27539-1-git-send-email-mihail.costea2005@gmail.com> References: <1368793673-27539-1-git-send-email-mihail.costea2005@gmail.com> Subject: [B.A.T.M.A.N.] [RFC 2/4] batman-adv: added IPv6 to DAT and generic functions in distributed-arp-table.c X-BeenThere: b.a.t.m.a.n@lists.open-mesh.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: The list for a Better Approach To Mobile Ad-hoc Networking <b.a.t.m.a.n@lists.open-mesh.org> List-Id: The list for a Better Approach To Mobile Ad-hoc Networking <b.a.t.m.a.n.lists.open-mesh.org> List-Unsubscribe: <https://lists.open-mesh.org/mm/options/b.a.t.m.a.n>, <mailto:b.a.t.m.a.n-request@lists.open-mesh.org?subject=unsubscribe> List-Archive: <http://lists.open-mesh.org/pipermail/b.a.t.m.a.n/> List-Post: <mailto:b.a.t.m.a.n@lists.open-mesh.org> List-Help: <mailto:b.a.t.m.a.n-request@lists.open-mesh.org?subject=help> List-Subscribe: <https://lists.open-mesh.org/mm/listinfo/b.a.t.m.a.n>, <mailto:b.a.t.m.a.n-request@lists.open-mesh.org?subject=subscribe> X-List-Received-Date: Fri, 17 May 2013 12:28:48 -0000 |
Commit Message
YourName
May 17, 2013, 12:27 p.m. UTC
From: "mihail.costea90@gmail.com" <mihail.costea90@gmail.com> Added IPv6 functionality to generic functions implemented in the first patch. Signed-off-by: Mihail Costea <mihail.costea90@gmail.com> Signed-off-by: Stefan Popa <Stefan.A.Popa@intel.com> Reviewed-by: Stefan Popa <Stefan.A.Popa@intel.com> --- distributed-arp-table.c | 30 ++++++++++++++++++++++++++---- types.h | 6 +++++- 2 files changed, 31 insertions(+), 5 deletions(-)
Comments
On Fri, May 17, 2013 at 03:27:50PM +0300, Mihail wrote: > From: "mihail.costea90@gmail.com" <mihail.costea90@gmail.com> > > Added IPv6 functionality to generic functions implemented in the first > patch. avoid references to "first"/"second"/"third" patch. Once committed, they will just be some patches in the list. You can either explicitly mention the name of the patch ("writing the subject in this way") or just saying previously/whatever else. > > Signed-off-by: Mihail Costea <mihail.costea90@gmail.com> > Signed-off-by: Stefan Popa <Stefan.A.Popa@intel.com> > Reviewed-by: Stefan Popa <Stefan.A.Popa@intel.com> > > --- > distributed-arp-table.c | 30 ++++++++++++++++++++++++++---- > types.h | 6 +++++- > 2 files changed, 31 insertions(+), 5 deletions(-) > > diff --git a/distributed-arp-table.c b/distributed-arp-table.c > index b2ca7e0..42118be 100644 > --- a/distributed-arp-table.c > +++ b/distributed-arp-table.c > @@ -32,7 +32,7 @@ > #include "unicast.h" > > static char *batadv_dat_types_str_fmt[] = { > - "%pI4", > + "%pI4", "%pI6c", better going on a new line rather than putting both on the same one (like the array declared in the gateway code). > }; > > static void batadv_dat_purge(struct work_struct *work); > @@ -173,6 +173,10 @@ static size_t batadv_sizeof_dat_data(uint8_t data_type) > switch (data_type) { > case BATADV_DAT_IPV4: > return sizeof(__be32); > +#if IS_ENABLED(CONFIG_IPV6) > + case BATADV_DAT_IPV6: > + return sizeof(struct in6_addr); > +#endif > default: > return 0; > } > @@ -295,6 +299,12 @@ static uint32_t batadv_hash_dat_ipv4(const void *data, uint32_t size) > return batadv_hash_dat(data, BATADV_DAT_IPV4, size); > } > > +#if IS_ENABLED(CONFIG_IPV6) > +static uint32_t batadv_hash_dat_ipv6(const void *data, uint32_t size) > +{ > + return batadv_hash_dat(data, BATADV_DAT_IPV6, size); > +} > +#endif > > /** > * batadv_dat_entry_hash_find - look for a given dat_entry in the local hash > @@ -393,6 +403,11 @@ static void batadv_dat_entry_add(struct batadv_priv *bat_priv, void *data, > case BATADV_DAT_IPV4: > choose = batadv_hash_dat_ipv4; > break; > +#if IS_ENABLED(CONFIG_IPV6) > + case BATADV_DAT_IPV6: > + choose = batadv_hash_dat_ipv6; > + break; > +#endif mh...I was just wondering: for the hash functions it would be nice to re-use the same "array approach" that you used for the printing format. So we would have an array of function pointers to reference rather than having this switch block...What do you think? I think it would be possible to do this for many other "variable" parameters if required. > default: > goto out; > } > @@ -872,8 +887,8 @@ int batadv_dat_cache_seq_print_text(struct seq_file *seq, void *offset) > goto out; > > seq_printf(seq, "Distributed ARP Table (%s):\n", net_dev->name); > - seq_printf(seq, " %-7s %-13s %5s\n", "IPv4", "MAC", > - "last-seen"); > + seq_printf(seq, " %-26s %-15s %5s\n", > + "IPv4/IPv6", "MAC", "last-seen"); > > for (i = 0; i < hash->size; i++) { > head = &hash->table[i]; > @@ -888,10 +903,17 @@ int batadv_dat_cache_seq_print_text(struct seq_file *seq, void *offset) > > switch (dat_entry->type) { > case BATADV_DAT_IPV4: > - seq_printf(seq, " * %15pI4 %14pM %6i:%02i\n", > + seq_printf(seq, " * %-40pI4 %15pM %6i:%02i\n", > dat_entry->data, dat_entry->mac_addr, > last_seen_mins, last_seen_secs); > break; > +#if IS_ENABLED(CONFIG_IPV6) > + case BATADV_DAT_IPV6: > + seq_printf(seq, " * %-40pI6c %15pM %6i:%02i\n", can't you generalise this using the printing format array defined at the top of this patch? > + dat_entry->data, dat_entry->mac_addr, > + last_seen_mins, last_seen_secs); > + break; > +#endif > } > } > rcu_read_unlock(); > diff --git a/types.h b/types.h > index 284e3d2..74c7091 100644 > --- a/types.h > +++ b/types.h > @@ -931,10 +931,14 @@ struct batadv_dat_entry { > > /** > * batadv_dat_types - types used in batadv_dat_entry for IP > - * @BATADV_DAT_IPv4: IPv4 address type > + * @BATADV_DAT_IPV4: IPv4 address type I guess this is a mistake. In the last version of your last patch you are using IPV4 and therefore it is impossible that you have IPv4 here (or maybe you forgot to fix this comment...in this case, this fix should go direclty in the previous patch). > + * @BATADV_DAT_IPV4: IPv6 address type > */ > enum batadv_dat_types { > BATADV_DAT_IPV4 = 0, > +#if IS_ENABLED(CONFIG_IPV6) > + BATADV_DAT_IPV6 = 1, > +#endif > }; > > /** > -- > 1.7.10.4
On 26 May 2013 06:57, Antonio Quartulli <ordex@autistici.org> wrote: > On Fri, May 17, 2013 at 03:27:50PM +0300, Mihail wrote: >> From: "mihail.costea90@gmail.com" <mihail.costea90@gmail.com> >> >> Added IPv6 functionality to generic functions implemented in the first >> patch. > > avoid references to "first"/"second"/"third" patch. Once committed, they will > just be some patches in the list. You can either explicitly mention the name of > the patch ("writing the subject in this way") or just saying > previously/whatever else. > > >> >> Signed-off-by: Mihail Costea <mihail.costea90@gmail.com> >> Signed-off-by: Stefan Popa <Stefan.A.Popa@intel.com> >> Reviewed-by: Stefan Popa <Stefan.A.Popa@intel.com> >> >> --- >> distributed-arp-table.c | 30 ++++++++++++++++++++++++++---- >> types.h | 6 +++++- >> 2 files changed, 31 insertions(+), 5 deletions(-) >> >> diff --git a/distributed-arp-table.c b/distributed-arp-table.c >> index b2ca7e0..42118be 100644 >> --- a/distributed-arp-table.c >> +++ b/distributed-arp-table.c >> @@ -32,7 +32,7 @@ >> #include "unicast.h" >> >> static char *batadv_dat_types_str_fmt[] = { >> - "%pI4", >> + "%pI4", "%pI6c", > > better going on a new line rather than putting both on the same one (like the > array declared in the gateway code). > ACK >> }; >> >> static void batadv_dat_purge(struct work_struct *work); >> @@ -173,6 +173,10 @@ static size_t batadv_sizeof_dat_data(uint8_t data_type) >> switch (data_type) { >> case BATADV_DAT_IPV4: >> return sizeof(__be32); >> +#if IS_ENABLED(CONFIG_IPV6) >> + case BATADV_DAT_IPV6: >> + return sizeof(struct in6_addr); >> +#endif >> default: >> return 0; >> } >> @@ -295,6 +299,12 @@ static uint32_t batadv_hash_dat_ipv4(const void *data, uint32_t size) >> return batadv_hash_dat(data, BATADV_DAT_IPV4, size); >> } >> >> +#if IS_ENABLED(CONFIG_IPV6) >> +static uint32_t batadv_hash_dat_ipv6(const void *data, uint32_t size) >> +{ >> + return batadv_hash_dat(data, BATADV_DAT_IPV6, size); >> +} >> +#endif >> >> /** >> * batadv_dat_entry_hash_find - look for a given dat_entry in the local hash >> @@ -393,6 +403,11 @@ static void batadv_dat_entry_add(struct batadv_priv *bat_priv, void *data, >> case BATADV_DAT_IPV4: >> choose = batadv_hash_dat_ipv4; >> break; >> +#if IS_ENABLED(CONFIG_IPV6) >> + case BATADV_DAT_IPV6: >> + choose = batadv_hash_dat_ipv6; >> + break; >> +#endif > > mh...I was just wondering: for the hash functions it would be nice to re-use the > same "array approach" that you used for the printing format. > > So we would have an array of function pointers to reference rather than having > this switch block...What do you think? > > I think it would be possible to do this for many other "variable" parameters if > required. > Should there be a struct that contains all of them, like: struct batadv_dat_data { } >> default: >> goto out; >> } >> @@ -872,8 +887,8 @@ int batadv_dat_cache_seq_print_text(struct seq_file *seq, void *offset) >> goto out; >> >> seq_printf(seq, "Distributed ARP Table (%s):\n", net_dev->name); >> - seq_printf(seq, " %-7s %-13s %5s\n", "IPv4", "MAC", >> - "last-seen"); >> + seq_printf(seq, " %-26s %-15s %5s\n", >> + "IPv4/IPv6", "MAC", "last-seen"); >> >> for (i = 0; i < hash->size; i++) { >> head = &hash->table[i]; >> @@ -888,10 +903,17 @@ int batadv_dat_cache_seq_print_text(struct seq_file *seq, void *offset) >> >> switch (dat_entry->type) { >> case BATADV_DAT_IPV4: >> - seq_printf(seq, " * %15pI4 %14pM %6i:%02i\n", >> + seq_printf(seq, " * %-40pI4 %15pM %6i:%02i\n", >> dat_entry->data, dat_entry->mac_addr, >> last_seen_mins, last_seen_secs); >> break; >> +#if IS_ENABLED(CONFIG_IPV6) >> + case BATADV_DAT_IPV6: >> + seq_printf(seq, " * %-40pI6c %15pM %6i:%02i\n", > > can't you generalise this using the printing format array defined at the > top of this patch? > >> + dat_entry->data, dat_entry->mac_addr, >> + last_seen_mins, last_seen_secs); >> + break; >> +#endif >> } >> } >> rcu_read_unlock(); >> diff --git a/types.h b/types.h >> index 284e3d2..74c7091 100644 >> --- a/types.h >> +++ b/types.h >> @@ -931,10 +931,14 @@ struct batadv_dat_entry { >> >> /** >> * batadv_dat_types - types used in batadv_dat_entry for IP >> - * @BATADV_DAT_IPv4: IPv4 address type >> + * @BATADV_DAT_IPV4: IPv4 address type > > I guess this is a mistake. In the last version of your last patch you are using > IPV4 and therefore it is impossible that you have IPv4 here (or maybe you forgot > to fix this comment...in this case, this fix should go direclty in the previous > patch). > >> + * @BATADV_DAT_IPV4: IPv6 address type >> */ >> enum batadv_dat_types { >> BATADV_DAT_IPV4 = 0, >> +#if IS_ENABLED(CONFIG_IPV6) >> + BATADV_DAT_IPV6 = 1, >> +#endif >> }; >> >> /** >> -- >> 1.7.10.4 > > -- > Antonio Quartulli > > ..each of us alone is worth nothing.. > Ernesto "Che" Guevara -- Mihail Costea
On 26 May 2013 06:57, Antonio Quartulli <ordex@autistici.org> wrote: > On Fri, May 17, 2013 at 03:27:50PM +0300, Mihail wrote: >> From: "mihail.costea90@gmail.com" <mihail.costea90@gmail.com> >> >> Added IPv6 functionality to generic functions implemented in the first >> patch. > > avoid references to "first"/"second"/"third" patch. Once committed, they will > just be some patches in the list. You can either explicitly mention the name of > the patch ("writing the subject in this way") or just saying > previously/whatever else. > > ACK >> >> Signed-off-by: Mihail Costea <mihail.costea90@gmail.com> >> Signed-off-by: Stefan Popa <Stefan.A.Popa@intel.com> >> Reviewed-by: Stefan Popa <Stefan.A.Popa@intel.com> >> >> --- >> distributed-arp-table.c | 30 ++++++++++++++++++++++++++---- >> types.h | 6 +++++- >> 2 files changed, 31 insertions(+), 5 deletions(-) >> >> diff --git a/distributed-arp-table.c b/distributed-arp-table.c >> index b2ca7e0..42118be 100644 >> --- a/distributed-arp-table.c >> +++ b/distributed-arp-table.c >> @@ -32,7 +32,7 @@ >> #include "unicast.h" >> >> static char *batadv_dat_types_str_fmt[] = { >> - "%pI4", >> + "%pI4", "%pI6c", > > better going on a new line rather than putting both on the same one (like the > array declared in the gateway code). > ACK >> }; >> >> static void batadv_dat_purge(struct work_struct *work); >> @@ -173,6 +173,10 @@ static size_t batadv_sizeof_dat_data(uint8_t data_type) >> switch (data_type) { >> case BATADV_DAT_IPV4: >> return sizeof(__be32); >> +#if IS_ENABLED(CONFIG_IPV6) >> + case BATADV_DAT_IPV6: >> + return sizeof(struct in6_addr); >> +#endif >> default: >> return 0; >> } >> @@ -295,6 +299,12 @@ static uint32_t batadv_hash_dat_ipv4(const void *data, uint32_t size) >> return batadv_hash_dat(data, BATADV_DAT_IPV4, size); >> } >> >> +#if IS_ENABLED(CONFIG_IPV6) >> +static uint32_t batadv_hash_dat_ipv6(const void *data, uint32_t size) >> +{ >> + return batadv_hash_dat(data, BATADV_DAT_IPV6, size); >> +} >> +#endif >> >> /** >> * batadv_dat_entry_hash_find - look for a given dat_entry in the local hash >> @@ -393,6 +403,11 @@ static void batadv_dat_entry_add(struct batadv_priv *bat_priv, void *data, >> case BATADV_DAT_IPV4: >> choose = batadv_hash_dat_ipv4; >> break; >> +#if IS_ENABLED(CONFIG_IPV6) >> + case BATADV_DAT_IPV6: >> + choose = batadv_hash_dat_ipv6; >> + break; >> +#endif > > mh...I was just wondering: for the hash functions it would be nice to re-use the > same "array approach" that you used for the printing format. > > So we would have an array of function pointers to reference rather than having > this switch block...What do you think? > > I think it would be possible to do this for many other "variable" parameters if > required. > Should there be a struct that contains all of them, like: struct batadv_dat_data { char *format; signature *hash_function; ....... } And after that the array with the initialization for all types. Or should they be declared as arrays that have nothing in common? Like just adding a new array for the choose functions. >> default: >> goto out; >> } >> @@ -872,8 +887,8 @@ int batadv_dat_cache_seq_print_text(struct seq_file *seq, void *offset) >> goto out; >> >> seq_printf(seq, "Distributed ARP Table (%s):\n", net_dev->name); >> - seq_printf(seq, " %-7s %-13s %5s\n", "IPv4", "MAC", >> - "last-seen"); >> + seq_printf(seq, " %-26s %-15s %5s\n", >> + "IPv4/IPv6", "MAC", "last-seen"); >> >> for (i = 0; i < hash->size; i++) { >> head = &hash->table[i]; >> @@ -888,10 +903,17 @@ int batadv_dat_cache_seq_print_text(struct seq_file *seq, void *offset) >> >> switch (dat_entry->type) { >> case BATADV_DAT_IPV4: >> - seq_printf(seq, " * %15pI4 %14pM %6i:%02i\n", >> + seq_printf(seq, " * %-40pI4 %15pM %6i:%02i\n", >> dat_entry->data, dat_entry->mac_addr, >> last_seen_mins, last_seen_secs); >> break; >> +#if IS_ENABLED(CONFIG_IPV6) >> + case BATADV_DAT_IPV6: >> + seq_printf(seq, " * %-40pI6c %15pM %6i:%02i\n", > > can't you generalise this using the printing format array defined at the > top of this patch? > I forgot about that. >> + dat_entry->data, dat_entry->mac_addr, >> + last_seen_mins, last_seen_secs); >> + break; >> +#endif >> } >> } >> rcu_read_unlock(); >> diff --git a/types.h b/types.h >> index 284e3d2..74c7091 100644 >> --- a/types.h >> +++ b/types.h >> @@ -931,10 +931,14 @@ struct batadv_dat_entry { >> >> /** >> * batadv_dat_types - types used in batadv_dat_entry for IP >> - * @BATADV_DAT_IPv4: IPv4 address type >> + * @BATADV_DAT_IPV4: IPv4 address type > > I guess this is a mistake. In the last version of your last patch you are using > IPV4 and therefore it is impossible that you have IPv4 here (or maybe you forgot > to fix this comment...in this case, this fix should go direclty in the previous > patch). > Don't know what happened here. Maybe by mistake I introduces this when I merged it with the old code used for snooping. >> + * @BATADV_DAT_IPV4: IPv6 address type >> */ >> enum batadv_dat_types { >> BATADV_DAT_IPV4 = 0, >> +#if IS_ENABLED(CONFIG_IPV6) >> + BATADV_DAT_IPV6 = 1, >> +#endif >> }; >> >> /** >> -- >> 1.7.10.4 > > -- > Antonio Quartulli > > ..each of us alone is worth nothing.. > Ernesto "Che" Guevara Thanks, Mihail
On Wed, May 29, 2013 at 08:16:25AM -0700, Mihail Costea wrote: > >> /** > >> * batadv_dat_entry_hash_find - look for a given dat_entry in the local hash > >> @@ -393,6 +403,11 @@ static void batadv_dat_entry_add(struct batadv_priv *bat_priv, void *data, > >> case BATADV_DAT_IPV4: > >> choose = batadv_hash_dat_ipv4; > >> break; > >> +#if IS_ENABLED(CONFIG_IPV6) > >> + case BATADV_DAT_IPV6: > >> + choose = batadv_hash_dat_ipv6; > >> + break; > >> +#endif > > > > mh...I was just wondering: for the hash functions it would be nice to re-use the > > same "array approach" that you used for the printing format. > > > > So we would have an array of function pointers to reference rather than having > > this switch block...What do you think? > > > > I think it would be possible to do this for many other "variable" parameters if > > required. > > > > Should there be a struct that contains all of them, like: > > struct batadv_dat_data { > I was thinking of having many array, but I have to say that the "struct idea" is much cleaner and it can be seen like an API. Then we would only have one array of objects of this type. Cheers,
On Wed, May 29, 2013 at 08:21:37AM -0700, Mihail Costea wrote: > On 26 May 2013 06:57, Antonio Quartulli <ordex@autistici.org> wrote: > > On Fri, May 17, 2013 at 03:27:50PM +0300, Mihail wrote: > >> From: "mihail.costea90@gmail.com" <mihail.costea90@gmail.com> > >> > >> Added IPv6 functionality to generic functions implemented in the first > >> patch. > > > > avoid references to "first"/"second"/"third" patch. Once committed, they will > > just be some patches in the list. You can either explicitly mention the name of > > the patch ("writing the subject in this way") or just saying > > previously/whatever else. > > > > > > ACK > > >> > >> Signed-off-by: Mihail Costea <mihail.costea90@gmail.com> > >> Signed-off-by: Stefan Popa <Stefan.A.Popa@intel.com> > >> Reviewed-by: Stefan Popa <Stefan.A.Popa@intel.com> > >> > >> --- > >> distributed-arp-table.c | 30 ++++++++++++++++++++++++++---- > >> types.h | 6 +++++- > >> 2 files changed, 31 insertions(+), 5 deletions(-) > >> > >> diff --git a/distributed-arp-table.c b/distributed-arp-table.c > >> index b2ca7e0..42118be 100644 > >> --- a/distributed-arp-table.c > >> +++ b/distributed-arp-table.c > >> @@ -32,7 +32,7 @@ > >> #include "unicast.h" > >> > >> static char *batadv_dat_types_str_fmt[] = { > >> - "%pI4", > >> + "%pI4", "%pI6c", > > > > better going on a new line rather than putting both on the same one (like the > > array declared in the gateway code). > > > > ACK > > >> }; > >> > >> static void batadv_dat_purge(struct work_struct *work); > >> @@ -173,6 +173,10 @@ static size_t batadv_sizeof_dat_data(uint8_t data_type) > >> switch (data_type) { > >> case BATADV_DAT_IPV4: > >> return sizeof(__be32); > >> +#if IS_ENABLED(CONFIG_IPV6) > >> + case BATADV_DAT_IPV6: > >> + return sizeof(struct in6_addr); > >> +#endif > >> default: > >> return 0; > >> } > >> @@ -295,6 +299,12 @@ static uint32_t batadv_hash_dat_ipv4(const void *data, uint32_t size) > >> return batadv_hash_dat(data, BATADV_DAT_IPV4, size); > >> } > >> > >> +#if IS_ENABLED(CONFIG_IPV6) > >> +static uint32_t batadv_hash_dat_ipv6(const void *data, uint32_t size) > >> +{ > >> + return batadv_hash_dat(data, BATADV_DAT_IPV6, size); > >> +} > >> +#endif > >> > >> /** > >> * batadv_dat_entry_hash_find - look for a given dat_entry in the local hash > >> @@ -393,6 +403,11 @@ static void batadv_dat_entry_add(struct batadv_priv *bat_priv, void *data, > >> case BATADV_DAT_IPV4: > >> choose = batadv_hash_dat_ipv4; > >> break; > >> +#if IS_ENABLED(CONFIG_IPV6) > >> + case BATADV_DAT_IPV6: > >> + choose = batadv_hash_dat_ipv6; > >> + break; > >> +#endif > > > > mh...I was just wondering: for the hash functions it would be nice to re-use the > > same "array approach" that you used for the printing format. > > > > So we would have an array of function pointers to reference rather than having > > this switch block...What do you think? > > > > I think it would be possible to do this for many other "variable" parameters if > > required. > > > > Should there be a struct that contains all of them, like: > > struct batadv_dat_data { > char *format; > signature *hash_function; > ....... > } > > And after that the array with the initialization for all types. > Or should they be declared as arrays that have nothing in common? Like > just adding a new array for the choose functions. > I guess you replied to the same email twice. However, the idea of 1 array of structs is nice! I like it :)
On 29 May 2013 08:33, Antonio Quartulli <ordex@autistici.org> wrote: > On Wed, May 29, 2013 at 08:21:37AM -0700, Mihail Costea wrote: >> On 26 May 2013 06:57, Antonio Quartulli <ordex@autistici.org> wrote: >> > On Fri, May 17, 2013 at 03:27:50PM +0300, Mihail wrote: >> >> From: "mihail.costea90@gmail.com" <mihail.costea90@gmail.com> >> >> >> >> Added IPv6 functionality to generic functions implemented in the first >> >> patch. >> > >> > avoid references to "first"/"second"/"third" patch. Once committed, they will >> > just be some patches in the list. You can either explicitly mention the name of >> > the patch ("writing the subject in this way") or just saying >> > previously/whatever else. >> > >> > >> >> ACK >> >> >> >> >> Signed-off-by: Mihail Costea <mihail.costea90@gmail.com> >> >> Signed-off-by: Stefan Popa <Stefan.A.Popa@intel.com> >> >> Reviewed-by: Stefan Popa <Stefan.A.Popa@intel.com> >> >> >> >> --- >> >> distributed-arp-table.c | 30 ++++++++++++++++++++++++++---- >> >> types.h | 6 +++++- >> >> 2 files changed, 31 insertions(+), 5 deletions(-) >> >> >> >> diff --git a/distributed-arp-table.c b/distributed-arp-table.c >> >> index b2ca7e0..42118be 100644 >> >> --- a/distributed-arp-table.c >> >> +++ b/distributed-arp-table.c >> >> @@ -32,7 +32,7 @@ >> >> #include "unicast.h" >> >> >> >> static char *batadv_dat_types_str_fmt[] = { >> >> - "%pI4", >> >> + "%pI4", "%pI6c", >> > >> > better going on a new line rather than putting both on the same one (like the >> > array declared in the gateway code). >> > >> >> ACK >> >> >> }; >> >> >> >> static void batadv_dat_purge(struct work_struct *work); >> >> @@ -173,6 +173,10 @@ static size_t batadv_sizeof_dat_data(uint8_t data_type) >> >> switch (data_type) { >> >> case BATADV_DAT_IPV4: >> >> return sizeof(__be32); >> >> +#if IS_ENABLED(CONFIG_IPV6) >> >> + case BATADV_DAT_IPV6: >> >> + return sizeof(struct in6_addr); >> >> +#endif >> >> default: >> >> return 0; >> >> } >> >> @@ -295,6 +299,12 @@ static uint32_t batadv_hash_dat_ipv4(const void *data, uint32_t size) >> >> return batadv_hash_dat(data, BATADV_DAT_IPV4, size); >> >> } >> >> >> >> +#if IS_ENABLED(CONFIG_IPV6) >> >> +static uint32_t batadv_hash_dat_ipv6(const void *data, uint32_t size) >> >> +{ >> >> + return batadv_hash_dat(data, BATADV_DAT_IPV6, size); >> >> +} >> >> +#endif >> >> >> >> /** >> >> * batadv_dat_entry_hash_find - look for a given dat_entry in the local hash >> >> @@ -393,6 +403,11 @@ static void batadv_dat_entry_add(struct batadv_priv *bat_priv, void *data, >> >> case BATADV_DAT_IPV4: >> >> choose = batadv_hash_dat_ipv4; >> >> break; >> >> +#if IS_ENABLED(CONFIG_IPV6) >> >> + case BATADV_DAT_IPV6: >> >> + choose = batadv_hash_dat_ipv6; >> >> + break; >> >> +#endif >> > >> > mh...I was just wondering: for the hash functions it would be nice to re-use the >> > same "array approach" that you used for the printing format. >> > >> > So we would have an array of function pointers to reference rather than having >> > this switch block...What do you think? >> > >> > I think it would be possible to do this for many other "variable" parameters if >> > required. >> > >> >> Should there be a struct that contains all of them, like: >> >> struct batadv_dat_data { >> char *format; >> signature *hash_function; >> ....... >> } >> >> And after that the array with the initialization for all types. >> Or should they be declared as arrays that have nothing in common? Like >> just adding a new array for the choose functions. >> > > I guess you replied to the same email twice. > However, the idea of 1 array of structs is nice! I like it :) > Sorry about that. I thought it didn't send (I was messing with tab key). I'll implement it in the next version for a review :). > > -- > Antonio Quartulli > > ..each of us alone is worth nothing.. > Ernesto "Che" Guevara -- Mihail Costea
On Wed, May 29, 2013 at 09:12:49AM -0700, Mihail Costea wrote: > > > > I guess you replied to the same email twice. > > However, the idea of 1 array of structs is nice! I like it :) > > > > Sorry about that. I thought it didn't send (I was messing with tab key). no worries :) > > I'll implement it in the next version for a review :). > Great! Sounds cool :) Cheers,
diff --git a/distributed-arp-table.c b/distributed-arp-table.c index b2ca7e0..42118be 100644 --- a/distributed-arp-table.c +++ b/distributed-arp-table.c @@ -32,7 +32,7 @@ #include "unicast.h" static char *batadv_dat_types_str_fmt[] = { - "%pI4", + "%pI4", "%pI6c", }; static void batadv_dat_purge(struct work_struct *work); @@ -173,6 +173,10 @@ static size_t batadv_sizeof_dat_data(uint8_t data_type) switch (data_type) { case BATADV_DAT_IPV4: return sizeof(__be32); +#if IS_ENABLED(CONFIG_IPV6) + case BATADV_DAT_IPV6: + return sizeof(struct in6_addr); +#endif default: return 0; } @@ -295,6 +299,12 @@ static uint32_t batadv_hash_dat_ipv4(const void *data, uint32_t size) return batadv_hash_dat(data, BATADV_DAT_IPV4, size); } +#if IS_ENABLED(CONFIG_IPV6) +static uint32_t batadv_hash_dat_ipv6(const void *data, uint32_t size) +{ + return batadv_hash_dat(data, BATADV_DAT_IPV6, size); +} +#endif /** * batadv_dat_entry_hash_find - look for a given dat_entry in the local hash @@ -393,6 +403,11 @@ static void batadv_dat_entry_add(struct batadv_priv *bat_priv, void *data, case BATADV_DAT_IPV4: choose = batadv_hash_dat_ipv4; break; +#if IS_ENABLED(CONFIG_IPV6) + case BATADV_DAT_IPV6: + choose = batadv_hash_dat_ipv6; + break; +#endif default: goto out; } @@ -872,8 +887,8 @@ int batadv_dat_cache_seq_print_text(struct seq_file *seq, void *offset) goto out; seq_printf(seq, "Distributed ARP Table (%s):\n", net_dev->name); - seq_printf(seq, " %-7s %-13s %5s\n", "IPv4", "MAC", - "last-seen"); + seq_printf(seq, " %-26s %-15s %5s\n", + "IPv4/IPv6", "MAC", "last-seen"); for (i = 0; i < hash->size; i++) { head = &hash->table[i]; @@ -888,10 +903,17 @@ int batadv_dat_cache_seq_print_text(struct seq_file *seq, void *offset) switch (dat_entry->type) { case BATADV_DAT_IPV4: - seq_printf(seq, " * %15pI4 %14pM %6i:%02i\n", + seq_printf(seq, " * %-40pI4 %15pM %6i:%02i\n", dat_entry->data, dat_entry->mac_addr, last_seen_mins, last_seen_secs); break; +#if IS_ENABLED(CONFIG_IPV6) + case BATADV_DAT_IPV6: + seq_printf(seq, " * %-40pI6c %15pM %6i:%02i\n", + dat_entry->data, dat_entry->mac_addr, + last_seen_mins, last_seen_secs); + break; +#endif } } rcu_read_unlock(); diff --git a/types.h b/types.h index 284e3d2..74c7091 100644 --- a/types.h +++ b/types.h @@ -931,10 +931,14 @@ struct batadv_dat_entry { /** * batadv_dat_types - types used in batadv_dat_entry for IP - * @BATADV_DAT_IPv4: IPv4 address type + * @BATADV_DAT_IPV4: IPv4 address type + * @BATADV_DAT_IPV4: IPv6 address type */ enum batadv_dat_types { BATADV_DAT_IPV4 = 0, +#if IS_ENABLED(CONFIG_IPV6) + BATADV_DAT_IPV6 = 1, +#endif }; /**