Hello Pablo,
On Fri, 2013-12-20 at 10:50 +0100, Pablo Neira Ayuso wrote:
> Hi Eric,
>
> Some comments on this patch.
>
> On Thu, Dec 12, 2013 at 08:44:40AM +0100, Eric Leblond wrote:
> > This patch prepares the addition of TCP reset support in
> > the nft_reject module by moving reusable code into a header
> > file.
> >
> > Signed-off-by: Eric Leblond <eric@xxxxxxxxx>
> > ---
> > include/net/netfilter/nf_reject.h | 297 ++++++++++++++++++++++++++++++++++++++
> > net/ipv4/netfilter/ipt_REJECT.c | 124 +---------------
> > net/ipv6/netfilter/ip6t_REJECT.c | 177 +----------------------
> > 3 files changed, 307 insertions(+), 291 deletions(-)
> > create mode 100644 include/net/netfilter/nf_reject.h
> >
> > diff --git a/include/net/netfilter/nf_reject.h b/include/net/netfilter/nf_reject.h
> > new file mode 100644
> > index 0000000..0bb089a
> > --- /dev/null
> > +++ b/include/net/netfilter/nf_reject.h
> > @@ -0,0 +1,297 @@
> > +#ifndef _NF_REJECT_H
> > +#define _NF_REJECT_H
> > +
> > +#include <net/ip.h>
> > +#include <net/tcp.h>
> > +#include <net/route.h>
> > +#include <net/dst.h>
> > +
> > +#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
> > +#include <net/ipv6.h>
> > +#include <net/ip6_route.h>
> > +#include <linux/netfilter_ipv6.h>
> > +#endif
> > +
> > +static inline void send_unreach(struct sk_buff *skb_in, int code)
> > +{
> > + icmp_send(skb_in, ICMP_DEST_UNREACH, code, 0);
> > +}
> > +
> > +/* Send RST reply */
> > +static inline void send_reset(struct sk_buff *oldskb, int hook)
>
> I think that this function is too large to use the inline approach
> that we have used in nft_queue. I've been considering moving this
> common code to a different module and export these symbols but I
> don't find a nice way to make it.
>
> So my suggestion is to have two different include files:
>
> include/net/netfilter/ipv4/nf_reject.h, that contains:
>
> static inline void send_unreach(...
> static void send_reset(...
>
> that you can include from ipt_REJECT and nft_reject.
>
> And include/net/netfilter/ipv6/nf_reject.h, that contains:
>
> static inline void send_unreach6(...
> static void send_reset6(...
>
> that you can include from ip6t_REJECT and nft_reject.
>
> While at it, it is probably good to prepend the prefix nf_ to those
> functions.
>
> Let me know if you have any concern with this approach.
I've implemented this but there is a corner case that I don't like: if
IPv6 is build as module then nft reject should be build as module too.
Putting a dependency on IPv6 to nft reject is not working as IPv4 only
setup will not be possible.
So I only see two solutions:
1. We make IPv6 to be dependent of nft reject IPv6 (I don"t think
this is a good choice)
2. We keep two separate versions of the nft reject module (IPv4 and
IPv6) to avoid this problem (not satisfying solution).
Do you have a better idea ?
I'm in favor of 2. as I think that it is a bit too much to act on IPv6
configuration value.
FYI the build message is:
net/built-in.o: In function `nft_reject_eval':
:(.text+0x2774a): undefined reference to `nf_ip6_checksum'
:(.text+0x27792): undefined reference to `ip6_route_output'
BR,
--
Eric Leblond <eric@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
