netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Simon Horman <horms@verge.net.au>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>,
	fengguang.wu@intel.com, yuanhan.liu@linux.intel.com,
	netdev@vger.kernel.org, lvs-devel@vger.kernel.org,
	netfilter-devel@vger.kernel.org,
	Hans Schillstrom <hans.schillstrom@ericsson.com>
Subject: Re: [nf-next PATCH v2] ipvs: fix build errors related to config option combinations
Date: Tue, 23 Oct 2012 10:42:41 +0200	[thread overview]
Message-ID: <20121023084241.GA7079@1984> (raw)
In-Reply-To: <20121023003314.GC5015@verge.net.au>

On Tue, Oct 23, 2012 at 09:33:15AM +0900, Simon Horman wrote:
> On Mon, Oct 22, 2012 at 09:22:45PM +0200, Jesper Dangaard Brouer wrote:
> > Fix two build error introduced by commit 63dca2c0:
> >  "ipvs: Fix faulty IPv6 extension header handling in IPVS"
> > 
> > First build error was fairly trivial and can occur, when
> > CONFIG_IP_VS_IPV6 is disabled.
> > 
> > The second build error was tricky, and can occur when deselecting
> > both all Netfilter and IPVS, but selecting CONFIG_IPV6.  This is
> > caused by "kernel/sysctl_binary.c" including "net/ip_vs.h", which
> > includes "linux/netfilter_ipv6/ip6_tables.h" causing include
> > of "include/linux/netfilter/x_tables.h" which then cannot find
> > the typedef nf_hookfn.
> > 
> > Fix this by only including "linux/netfilter_ipv6/ip6_tables.h" in
> > case of CONFIG_IP_VS_IPV6 as its already used to guard the usage
> > of ipv6_find_hdr().
> 
> Thanks, I have verified both of these fixes and I will send a pull
> request ASAP.
> 
> I do wonder how we might get earlier test coverage of these kinds of problems.

David already mentioned that we (Netfilter/IPVS) should aim to reduce
the amount of ifdef pollution in the code. Sometimes it is possible in
a nice way by encapsulating code that depends on the feature in one
single file.

Sometimes it also requires some more thinking to make it that way (not
as easy as adding ifdef).

I think if we aim to that, we can avoid this sort of problems.

  reply	other threads:[~2012-10-23  8:42 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-22 19:22 [nf-next PATCH v2] ipvs: fix build errors related to config option combinations Jesper Dangaard Brouer
2012-10-23  0:33 ` Simon Horman
2012-10-23  8:42   ` Pablo Neira Ayuso [this message]
2012-10-23  8:53     ` Pablo Neira Ayuso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121023084241.GA7079@1984 \
    --to=pablo@netfilter.org \
    --cc=brouer@redhat.com \
    --cc=fengguang.wu@intel.com \
    --cc=hans.schillstrom@ericsson.com \
    --cc=horms@verge.net.au \
    --cc=lvs-devel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=yuanhan.liu@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).