Hi,
I've started implementing DAT for IPv6 and I have a few questions regarding the design. I've already implemented a snooping mechanism for Network Solicitation package, but now I have to add it to the ipv6 distributed table. I'm also a beginner in kernel programming so any help would be useful.
From what I've seen there is a lot of code in distributed-arp-table.c
that could be used for ipv6 distributed table. A problem in using that code, from what I've seen, is the fact that <struct batadv_dat_entry > has a field for ip with only 32 bits (__be32).
So I was thinking to make it more general by: 1. transforming that into a pointer (unsigned char *ip) and add a field size which will represent the total size of the ip. The ip itself would be allocated dynamically. 2. use a union with both _be32 / struct in6_addr. In this case the problem is that if we use ipv4 then all the other space for struct in6_addr would be wasted.
The second problem from what I've seen in the code is that the batadv_priv->dat field is hardcoded. For ipv6 we would need a new field to memorize the table, but I don't think it's a really hard problem to solve in order to make it general.
This solution would be to avoid code duplicate. Off course, I could still write all the code in parallel just for IPv6, but I think that a common base would be useful.
Thanks, Mihail
On Fri, Mar 01, 2013 at 12:59:18PM +0200, Mihail Costea wrote:
Hi,
I've started implementing DAT for IPv6 and I have a few questions regarding the design. I've already implemented a snooping mechanism for Network Solicitation package, but now I have to add it to the ipv6 distributed table. I'm also a beginner in kernel programming so any help would be useful.
From what I've seen there is a lot of code in distributed-arp-table.c that could be used for ipv6 distributed table.
yeah, I think that the table handling and DHT primitives can be reused for IPv6 (they were designed on purpose).
A problem in using that code, from what I've seen, is the fact that <struct batadv_dat_entry > has a field for ip with only 32 bits (__be32).
Yes, here a way to generalise the structure is needed.
So I was thinking to make it more general by:
- transforming that into a pointer (unsigned char *ip) and add a
field size which will represent the total size of the ip. The ip itself would be allocated dynamically. 2. use a union with both _be32 / struct in6_addr. In this case the problem is that if we use ipv4 then all the other space for struct in6_addr would be wasted.
correct. and for point 2) I think we waste a not negligible amount of memory..
The second problem from what I've seen in the code is that the batadv_priv->dat field is hardcoded. For ipv6 we would need a new field to memorize the table, but I don't think it's a really hard problem to solve in order to make it general.
I did not get the problem with the dat field? Can't you use the same table to store both IPv4 and IPv6? The DHT is made to store general data, so I think (with the suggestions you wrote above) that you should be able to use one table only.
This solution would be to avoid code duplicate. Off course, I could still write all the code in parallel just for IPv6, but I think that a common base would be useful.
Re-use code is definitely the way to go :) easy maintenance later, one single point for bug-fixing and code has been tested already (so it should work[tm] as expected :-))
Thanks,
no worries :)
Cheers,
On 1 March 2013 13:08, Antonio Quartulli ordex@autistici.org wrote:
On Fri, Mar 01, 2013 at 12:59:18PM +0200, Mihail Costea wrote:
Hi,
I've started implementing DAT for IPv6 and I have a few questions regarding the design. I've already implemented a snooping mechanism for Network Solicitation package, but now I have to add it to the ipv6 distributed table. I'm also a beginner in kernel programming so any help would be useful.
From what I've seen there is a lot of code in distributed-arp-table.c that could be used for ipv6 distributed table.
yeah, I think that the table handling and DHT primitives can be reused for IPv6 (they were designed on purpose).
A problem in using that code, from what I've seen, is the fact that <struct batadv_dat_entry > has a field for ip with only 32 bits (__be32).
Yes, here a way to generalise the structure is needed.
So I was thinking to make it more general by:
- transforming that into a pointer (unsigned char *ip) and add a
field size which will represent the total size of the ip. The ip itself would be allocated dynamically. 2. use a union with both _be32 / struct in6_addr. In this case the problem is that if we use ipv4 then all the other space for struct in6_addr would be wasted.
correct. and for point 2) I think we waste a not negligible amount of memory..
The second problem from what I've seen in the code is that the batadv_priv->dat field is hardcoded. For ipv6 we would need a new field to memorize the table, but I don't think it's a really hard problem to solve in order to make it general.
I did not get the problem with the dat field? Can't you use the same table to store both IPv4 and IPv6? The DHT is made to store general data, so I think (with the suggestions you wrote above) that you should be able to use one table only.
I think it can be reuse. If I understood well the DHT has buckets for each entry, so it shouldn't be a problem if the IPv6 hash collides with the IPv4 hash. I just thought that they should be separated because because they are different concepts. This makes it a lot more easier :).
This solution would be to avoid code duplicate. Off course, I could still write all the code in parallel just for IPv6, but I think that a common base would be useful.
Re-use code is definitely the way to go :) easy maintenance later, one single point for bug-fixing and code has been tested already (so it should work[tm] as expected :-))
Thanks,
no worries :)
Cheers,
-- Antonio Quartulli
..each of us alone is worth nothing.. Ernesto "Che" Guevara
On Fri, Mar 01, 2013 at 01:38:43PM +0200, Mihail Costea wrote:
On 1 March 2013 13:08, Antonio Quartulli ordex@autistici.org wrote:
On Fri, Mar 01, 2013 at 12:59:18PM +0200, Mihail Costea wrote:
Hi,
I've started implementing DAT for IPv6 and I have a few questions regarding the design. I've already implemented a snooping mechanism for Network Solicitation package, but now I have to add it to the ipv6 distributed table. I'm also a beginner in kernel programming so any help would be useful.
From what I've seen there is a lot of code in distributed-arp-table.c that could be used for ipv6 distributed table.
yeah, I think that the table handling and DHT primitives can be reused for IPv6 (they were designed on purpose).
A problem in using that code, from what I've seen, is the fact that <struct batadv_dat_entry > has a field for ip with only 32 bits (__be32).
Yes, here a way to generalise the structure is needed.
So I was thinking to make it more general by:
- transforming that into a pointer (unsigned char *ip) and add a
field size which will represent the total size of the ip. The ip itself would be allocated dynamically. 2. use a union with both _be32 / struct in6_addr. In this case the problem is that if we use ipv4 then all the other space for struct in6_addr would be wasted.
correct. and for point 2) I think we waste a not negligible amount of memory..
The second problem from what I've seen in the code is that the batadv_priv->dat field is hardcoded. For ipv6 we would need a new field to memorize the table, but I don't think it's a really hard problem to solve in order to make it general.
I did not get the problem with the dat field? Can't you use the same table to store both IPv4 and IPv6? The DHT is made to store general data, so I think (with the suggestions you wrote above) that you should be able to use one table only.
I think it can be reuse. If I understood well the DHT has buckets for each entry, so it shouldn't be a problem if the IPv6 hash collides with the IPv4 hash.
yeah in general hash collisions are not a problem (well it is better to reduce them in order to provide a better distribution of the data).
I just thought that they should be separated because because they are different concepts.
yes, but in the end they are still separated at the logical level (the data size will make the difference). Maybe you can add a "type" field on the bucket? In this way we can easily differentiate one data type from another (this maybe helpful for future use of the DHT too) instead of replying only on the "addr_size" field.
This makes it a lot more easier :).
Yeah :)
Cheers,
On 1 March 2013 13:43, Antonio Quartulli ordex@autistici.org wrote:
On Fri, Mar 01, 2013 at 01:38:43PM +0200, Mihail Costea wrote:
On 1 March 2013 13:08, Antonio Quartulli ordex@autistici.org wrote:
On Fri, Mar 01, 2013 at 12:59:18PM +0200, Mihail Costea wrote:
Hi,
I've started implementing DAT for IPv6 and I have a few questions regarding the design. I've already implemented a snooping mechanism for Network Solicitation package, but now I have to add it to the ipv6 distributed table. I'm also a beginner in kernel programming so any help would be useful.
From what I've seen there is a lot of code in distributed-arp-table.c that could be used for ipv6 distributed table.
yeah, I think that the table handling and DHT primitives can be reused for IPv6 (they were designed on purpose).
A problem in using that code, from what I've seen, is the fact that <struct batadv_dat_entry > has a field for ip with only 32 bits (__be32).
Yes, here a way to generalise the structure is needed.
So I was thinking to make it more general by:
- transforming that into a pointer (unsigned char *ip) and add a
field size which will represent the total size of the ip. The ip itself would be allocated dynamically. 2. use a union with both _be32 / struct in6_addr. In this case the problem is that if we use ipv4 then all the other space for struct in6_addr would be wasted.
correct. and for point 2) I think we waste a not negligible amount of memory..
The second problem from what I've seen in the code is that the batadv_priv->dat field is hardcoded. For ipv6 we would need a new field to memorize the table, but I don't think it's a really hard problem to solve in order to make it general.
I did not get the problem with the dat field? Can't you use the same table to store both IPv4 and IPv6? The DHT is made to store general data, so I think (with the suggestions you wrote above) that you should be able to use one table only.
I think it can be reuse. If I understood well the DHT has buckets for each entry, so it shouldn't be a problem if the IPv6 hash collides with the IPv4 hash.
yeah in general hash collisions are not a problem (well it is better to reduce them in order to provide a better distribution of the data).
I just thought that they should be separated because because they are different concepts.
yes, but in the end they are still separated at the logical level (the data size will make the difference). Maybe you can add a "type" field on the bucket? In this way we can easily differentiate one data type from another (this maybe helpful for future use of the DHT too) instead of replying only on the "addr_size" field.
Yes, the type is a better idea. Then, back to coding :).
Cheers, Mihail
Hi,
I have created a patch for this, just that is quite big (~ 450 lines the whole patch, with signature, etc). It doesn't contain complex changes, just small changes from __be32 to (unsigned char *), a new parameter to functions in order to give the IP type and added comments.
I've tested it with the snooping mechanism I added for IPv6 and it works ok (the snooping mechanism is not in this patch).
What I wanted to ask is: it's ok to send a patch this big to the mailing list? I don't see how I could split it more because all I did is changed most of functions signatures and edit the code to support this changes.
Thx, Mihail
On Fri, Mar 08, 2013 at 11:22:37AM +0200, Mihail Costea wrote:
Hi,
I have created a patch for this, just that is quite big (~ 450 lines the whole patch, with signature, etc). It doesn't contain complex changes, just small changes from __be32 to (unsigned char *), a new parameter to functions in order to give the IP type and added comments.
I've tested it with the snooping mechanism I added for IPv6 and it works ok (the snooping mechanism is not in this patch).
What I wanted to ask is: it's ok to send a patch this big to the mailing list? I don't see how I could split it more because all I did is changed most of functions signatures and edit the code to support this changes.
The common rule is "one change per patch" meaning that if you can logically split it (e.g. 1) add this 2) use this there and there 3) improve that) it would be better to send more than one patch in one patchset. Remember that each and every patch must compile.
Cheers,
p.s. if you want you can send the patch as RFC and see the others say (but please try to think about splitting it, if really needed).
Thx, Mihail
On 8 March 2013 11:33, Antonio Quartulli ordex@autistici.org wrote:
On Fri, Mar 08, 2013 at 11:22:37AM +0200, Mihail Costea wrote:
Hi,
I have created a patch for this, just that is quite big (~ 450 lines the whole patch, with signature, etc). It doesn't contain complex changes, just small changes from __be32 to (unsigned char *), a new parameter to functions in order to give the IP type and added comments.
I've tested it with the snooping mechanism I added for IPv6 and it works ok (the snooping mechanism is not in this patch).
What I wanted to ask is: it's ok to send a patch this big to the mailing list? I don't see how I could split it more because all I did is changed most of functions signatures and edit the code to support this changes.
The common rule is "one change per patch" meaning that if you can logically split it (e.g. 1) add this 2) use this there and there 3) improve that) it would be better to send more than one patch in one patchset. Remember that each and every patch must compile.
I think I could split it into 2-3 parts logically: 1. Add the enum for IP types and add generic functions for comparing, calculating hash 2. Add generic debug message 3. Modify batadv_dat_entry and all other functions to support both IPv4/IPv6
Some functions from 1./2. might not be used from start, but only later, when 3. is done.
Cheers,
p.s. if you want you can send the patch as RFC and see the others say (but please try to think about splitting it, if really needed).
Thx, Mihail
-- Antonio Quartulli
..each of us alone is worth nothing.. Ernesto "Che" Guevara
On Fri, Mar 08, 2013 at 12:11:42PM +0200, Mihail Costea wrote:
On 8 March 2013 11:33, Antonio Quartulli ordex@autistici.org wrote:
On Fri, Mar 08, 2013 at 11:22:37AM +0200, Mihail Costea wrote:
Hi,
I have created a patch for this, just that is quite big (~ 450 lines the whole patch, with signature, etc). It doesn't contain complex changes, just small changes from __be32 to (unsigned char *), a new parameter to functions in order to give the IP type and added comments.
I've tested it with the snooping mechanism I added for IPv6 and it works ok (the snooping mechanism is not in this patch).
What I wanted to ask is: it's ok to send a patch this big to the mailing list? I don't see how I could split it more because all I did is changed most of functions signatures and edit the code to support this changes.
The common rule is "one change per patch" meaning that if you can logically split it (e.g. 1) add this 2) use this there and there 3) improve that) it would be better to send more than one patch in one patchset. Remember that each and every patch must compile.
I think I could split it into 2-3 parts logically:
- Add the enum for IP types and add generic functions for comparing,
calculating hash 2. Add generic debug message 3. Modify batadv_dat_entry and all other functions to support both IPv4/IPv6
Some functions from 1./2. might not be used from start, but only later, when 3. is done.
Well, looking at the code it is easier to judge :) Feel free to send your RFC/PATCH to the ml.
Cheers,
On Fri, Mar 01, 2013 at 12:59:18PM +0200, Mihail Costea wrote:
Hi,
I've started implementing DAT for IPv6 and I have a few questions regarding the design. I've already implemented a snooping mechanism for Network Solicitation package,
I forgot to ask here: packets generated by the IPv6 the Neigh Discovery Protocol are sent on a periodic basis, right? so what is the idea behind the snooping mechanism? There is not a request (like for ARP), right? So when should the data in distributed table be used?
Cheers,
On 1 March 2013 13:11, Antonio Quartulli ordex@autistici.org wrote:
On Fri, Mar 01, 2013 at 12:59:18PM +0200, Mihail Costea wrote:
Hi,
I've started implementing DAT for IPv6 and I have a few questions regarding the design. I've already implemented a snooping mechanism for Network Solicitation package,
I forgot to ask here: packets generated by the IPv6 the Neigh Discovery Protocol are sent on a periodic basis, right? so what is the idea behind the snooping mechanism? There is not a request (like for ARP), right? So when should the data in distributed table be used?
From what I understood the Neighbor Solicitation and Neighbor
Advertisement packets are the ARP equivalents. So if we receive a Neighbor Solicitation than we would use the distributed table and send the Neighbor Advertisement directly.
Cheers,
-- Antonio Quartulli
..each of us alone is worth nothing.. Ernesto "Che" Guevara
Thanks, Mihail
On Fri, Mar 01, 2013 at 01:41:28PM +0200, Mihail Costea wrote:
On 1 March 2013 13:11, Antonio Quartulli ordex@autistici.org wrote:
On Fri, Mar 01, 2013 at 12:59:18PM +0200, Mihail Costea wrote:
Hi,
I've started implementing DAT for IPv6 and I have a few questions regarding the design. I've already implemented a snooping mechanism for Network Solicitation package,
I forgot to ask here: packets generated by the IPv6 the Neigh Discovery Protocol are sent on a periodic basis, right? so what is the idea behind the snooping mechanism? There is not a request (like for ARP), right? So when should the data in distributed table be used?
From what I understood the Neighbor Solicitation and Neighbor Advertisement packets are the ARP equivalents. So if we receive a Neighbor Solicitation than we would use the distributed table and send the Neighbor Advertisement directly.
Oh ok, It was my fault. I quickly re-read part of the RFC4861: only router advertisement are sent on a periodic basis, while the neighbour advertisement is asynchronously sent either on address change or as a reply to a neighbour solicitation.
Thanks.
Cheers,
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
Mihail Costea