NAT IPv6/IPv4 translator - request for review, plus two questions regarding code

NAT IPv6/IPv4 translator - request for review, plus two questions regarding code

[Łukasz Czyż]

Hello all

I have implemented IPv6/IPv4 NAT for Linux OS. It is written as kernel
module which registers its functions in netfilter hooks. I would like
some netfilter programming expert to review my code (it is first
kernel module written by me) and I am wondering what is the best form
of contributing source to make somebody willing to glance at it. I am
not sure if patch is the best way here, because I didn't make any
changes in any other netfilter-related code, just created new module
which only depends on netfilter hook register functions. Maybe tarball
with Makefile is enough?

Ttranslator is released under GPLv2 license - You can find more info
about it at <http://czyzu.pl/xlat.html>.


I would like to take occasion and ask two questions regarding my code here.

1. I have following structure defined:

/*
 * Helper struct used during packet translation
 */
struct xlat_translation_buffer {
    /* Following entries must not be splitted */
	struct iphdr		hdr4;
	struct icmphdr		i_hdr4;
    /* End of entries */

    /* Following entries must not be splitted */
	struct ipv6hdr		hdr6;
	union {
		struct frag_hdr		f_hdr;
		struct icmp6hdr		i_hdr6;
	} hdrs6;
    /* End of entries */

	u16			hdr_len;
	u16			hfrag_off;
	u16			frag;
	s16			hdr_diff;
	u16			payload_len;
};

As You can guess from comments, there are particular fields in that
structure which have to follow immediately, one after the other.
That's because sometimes they are copied at the same time, by using
one memcpy() call. Can I be sure that compiler won't put any void
bytes between them (for alignment purposes) on any platform? Or maybe
I should use substructures with 'packed' attribute to prevent such
situation?

2. Second question - I have already posted it on kernel-newbies
mailing list, but I am still not sure about it. Does every function
which is used during packet translation (so it may be called many
times every second) should be declared as inline? If no - what rules
should I follow when determining if function has to be inlined in
networking code.


Lukasz

Re: NAT IPv6/IPv4 translator - request for review, plus two questions regarding code

[Maciej Żenczykowski]

Disclaimer: I haven't looked at the code, and only gave a 5 minute
glance at your web page.

I would probably suggest implementing this not as netfilter hooks,
but rather as a virtual device (tun/tap/veth/dummy like).

You route v6 netblock 64:ff9b::/96 towards this device (and similarly
with the v4 subnet(s) it uses as source addresses for generated
traffic).
All translation packet mirroring is then confined to the transmit
function of this virtual point-to-point device.

From the web page it doesn't sound like there's any actual netfilter
interaction...

Furthermore, I would hazard a guess that requiring an IPv4 address for
every IPv6 host desiring to connect to an IPv4 address makes
this a little uninteresting.... to quote:

"For every IPv6 node communicating with IPv4 network one translator
address is reserved as node's representative. Because of this maximal
number of IPv6 nodes that are able to connect to IPv4 network at the
same time is limited by translator prefix size."

OTOH, Fixing this _might_ just require much deeper integration with netfilter.
Although netfilter isn't really setup to handle v4-v6 connection so it
might be a wash...

On Thu, Mar 15, 2012 at 11:58, Łukasz Czyż <lukasz.czyzz@gmail.com> wrote:
> Hello all
>
> I have implemented IPv6/IPv4 NAT for Linux OS. It is written as kernel
> module which registers its functions in netfilter hooks. I would like
> some netfilter programming expert to review my code (it is first
> kernel module written by me) and I am wondering what is the best form
> of contributing source to make somebody willing to glance at it. I am
> not sure if patch is the best way here, because I didn't make any
> changes in any other netfilter-related code, just created new module
> which only depends on netfilter hook register functions. Maybe tarball
> with Makefile is enough?
>
> Ttranslator is released under GPLv2 license - You can find more info
> about it at <http://czyzu.pl/xlat.html>.
>
>
> I would like to take occasion and ask two questions regarding my code here.
>
> 1. I have following structure defined:
>
> /*
>  * Helper struct used during packet translation
>  */
> struct xlat_translation_buffer {
>    /* Following entries must not be splitted */
>        struct iphdr            hdr4;
>        struct icmphdr          i_hdr4;
>    /* End of entries */
>
>    /* Following entries must not be splitted */
>        struct ipv6hdr          hdr6;
>        union {
>                struct frag_hdr         f_hdr;
>                struct icmp6hdr         i_hdr6;
>        } hdrs6;
>    /* End of entries */
>
>        u16                     hdr_len;
>        u16                     hfrag_off;
>        u16                     frag;
>        s16                     hdr_diff;
>        u16                     payload_len;
> };
>
> As You can guess from comments, there are particular fields in that
> structure which have to follow immediately, one after the other.
> That's because sometimes they are copied at the same time, by using
> one memcpy() call. Can I be sure that compiler won't put any void
> bytes between them (for alignment purposes) on any platform? Or maybe
> I should use substructures with 'packed' attribute to prevent such
> situation?
>
> 2. Second question - I have already posted it on kernel-newbies
> mailing list, but I am still not sure about it. Does every function
> which is used during packet translation (so it may be called many
> times every second) should be declared as inline? If no - what rules
> should I follow when determining if function has to be inlined in
> networking code.
>
>
> Lukasz
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: NAT IPv6/IPv4 translator - request for review, plus two questions regarding code

[Łukasz Czyż]

>
> I would probably suggest implementing this not as netfilter hooks,
> but rather as a virtual device (tun/tap/veth/dummy like).
>

This exactly how I implemented it in early version of my module. I
changed to netfilter-hook solution, because it seemed to me that
translation process gets faster by doing so. At the other hand
netfilter way generates some additional processing (really small) for
every received packet, even if it is not dedicated to translation - so
it is some kind of trade off.

>
> From the web page it doesn't sound like there's any actual netfilter
> interaction...
>

Yes , as I wrote in my previous message - my module only registers its
functions in netfilter hooks, it doesn't use any other netfilter
functionality.

> Furthermore, I would hazard a guess that requiring an IPv4 address for
> every IPv6 host desiring to connect to an IPv4 address makes
> this a little uninteresting.... to quote:
>

Yes, I agree that it is big constraint, despite that such
configuration can be useful in some environment, when one needs to
have IPv6 host which should be able to reach IPv4 network from static
IPv4 address, reserved to this host only, without port forwarding. It
is similar to one of Cisco NAT translation schemes, called Static NAT.

I am aware that I would have to implement port forwarding option to
make translator more functional. But it is big topic and I do not have
time to code it at this moment. I am just looking for review of my
kernel module.



Lukasz