Re: [bug] build failure in net/netfilter/nf_conntrack_sip.c, on latest -git

Re: [bug] build failure in net/netfilter/nf_conntrack_sip.c, on latest -git

[David Miller]

From: Ingo Molnar <mingo@elte.hu>
Date: Sat, 19 Apr 2008 12:00:35 +0200

[ netfilter-devel and netdev added to CC: ]

> randconfig testing found a netfilter build failure in latest -git:
> 
>   net/netfilter/nf_conntrack_sip.c: In function 'set_expected_rtp_rtcp':
>   net/netfilter/nf_conntrack_sip.c:786: error: 'struct nf_conntrack_expect' has no member named 'saved_ip'
> 
>   http://redhat.com/~mingo/misc/config-Sat_Apr_19_11_37_02_CEST_2008.bad
> 
> disabling CONFIG_NF_CONNTRACK_SIP works it around.

I think this will fix it.

Patrick, is this how we should do it?

diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index c1fc0f1..fd60b2b 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -245,7 +245,7 @@ config NF_CONNTRACK_SANE
 
 config NF_CONNTRACK_SIP
 	tristate "SIP protocol support"
-	depends on NF_CONNTRACK
+	depends on NF_CONNTRACK && NF_NAT
 	default m if NETFILTER_ADVANCED=n
 	help
 	  SIP is an application-layer control protocol that can establish,

Re: [bug] build failure in net/netfilter/nf_conntrack_sip.c, on latest -git

[Ingo Molnar]

* David Miller <davem@davemloft.net> wrote:

> From: Ingo Molnar <mingo@elte.hu>
> Date: Sat, 19 Apr 2008 12:00:35 +0200
> 
> [ netfilter-devel and netdev added to CC: ]
> 
> > randconfig testing found a netfilter build failure in latest -git:
> > 
> >   net/netfilter/nf_conntrack_sip.c: In function 'set_expected_rtp_rtcp':
> >   net/netfilter/nf_conntrack_sip.c:786: error: 'struct nf_conntrack_expect' has no member named 'saved_ip'
> > 
> >   http://redhat.com/~mingo/misc/config-Sat_Apr_19_11_37_02_CEST_2008.bad
> > 
> > disabling CONFIG_NF_CONNTRACK_SIP works it around.
> 
> I think this will fix it.
> 
> Patrick, is this how we should do it?
> 
> diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
> index c1fc0f1..fd60b2b 100644
> --- a/net/netfilter/Kconfig
> +++ b/net/netfilter/Kconfig
> @@ -245,7 +245,7 @@ config NF_CONNTRACK_SANE
>  
>  config NF_CONNTRACK_SIP
>  	tristate "SIP protocol support"
> -	depends on NF_CONNTRACK
> +	depends on NF_CONNTRACK && NF_NAT
>  	default m if NETFILTER_ADVANCED=n
>  	help
>  	  SIP is an application-layer control protocol that can establish,

thanks David, this solves the build failure here.

Tested-by: Ingo Molnar <mingo@elte.hu>

	Ingo

Re: [bug] build failure in net/netfilter/nf_conntrack_sip.c, on latest -git

[David Miller]

From: Ingo Molnar <mingo@elte.hu>
Date: Sat, 19 Apr 2008 12:41:52 +0200

> thanks David, this solves the build failure here.
> 
> Tested-by: Ingo Molnar <mingo@elte.hu>

Thanks for testing.

I guess if you had been able to reply to the merge request
thread, this would have been fixed faster, sorry about that.

Re: [bug] build failure in net/netfilter/nf_conntrack_sip.c, on latest -git

[Jan Engelhardt]

On Saturday 2008-04-19 12:07, David Miller wrote:
>> randconfig testing found a netfilter build failure in latest -git:
>> 
>>   net/netfilter/nf_conntrack_sip.c: In function 'set_expected_rtp_rtcp':
>>   net/netfilter/nf_conntrack_sip.c:786: error: 'struct nf_conntrack_expect' has no member named 'saved_ip'
>> 
>>   http://redhat.com/~mingo/misc/config-Sat_Apr_19_11_37_02_CEST_2008.bad
>> 
>> disabling CONFIG_NF_CONNTRACK_SIP works it around.
>
>I think this will fix it.
>
>Patrick, is this how we should do it?
>
> config NF_CONNTRACK_SIP
> 	tristate "SIP protocol support"
>-	depends on NF_CONNTRACK
>+	depends on NF_CONNTRACK && NF_NAT
> 	default m if NETFILTER_ADVANCED=n

Not really. One may want to run a system without NAT,
but still with connection tracking (for firewalling) -- think IPv6.

Re: [bug] build failure in net/netfilter/nf_conntrack_sip.c, on latest -git

[David Miller]

From: Jan Engelhardt <jengelh@computergmbh.de>
Date: Sat, 19 Apr 2008 12:44:53 +0200 (CEST)

> 
> On Saturday 2008-04-19 12:07, David Miller wrote:
> >Patrick, is this how we should do it?
> >
> > config NF_CONNTRACK_SIP
> > 	tristate "SIP protocol support"
> >-	depends on NF_CONNTRACK
> >+	depends on NF_CONNTRACK && NF_NAT
> > 	default m if NETFILTER_ADVANCED=n
> 
> Not really. One may want to run a system without NAT,
> but still with connection tracking (for firewalling) -- think IPv6.

But SIP needs the NAT layer in order to get at those
expectation structure members.

What is your alternative fix suggestion then?

Re: [bug] build failure in net/netfilter/nf_conntrack_sip.c, on latest -git

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

David Miller wrote:
> From: Ingo Molnar <mingo@elte.hu>
> Date: Sat, 19 Apr 2008 12:00:35 +0200
>
> [ netfilter-devel and netdev added to CC: ]
>
>   
>> randconfig testing found a netfilter build failure in latest -git:
>>
>>   net/netfilter/nf_conntrack_sip.c: In function 'set_expected_rtp_rtcp':
>>   net/netfilter/nf_conntrack_sip.c:786: error: 'struct nf_conntrack_expect' has no member named 'saved_ip'
>>
>>   http://redhat.com/~mingo/misc/config-Sat_Apr_19_11_37_02_CEST_2008.bad
>>
>> disabling CONFIG_NF_CONNTRACK_SIP works it around.
>>     
>
> I think this will fix it.
>
> Patrick, is this how we should do it?

No, the SIP helper is also useful without NAT. This patch adds
an ifdef around the RTP call optimization for NATed clients.

Signed-off-by: Patrick McHardy <kaber@trash.net>



[-- Attachment #2: x --]
[-- Type: text/plain, Size: 806 bytes --]

diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 65b3ba5..9f49000 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -781,7 +781,7 @@ static int set_expected_rtp_rtcp(struct sk_buff *skb,
 		    nfct_help(exp->master)->helper != nfct_help(ct)->helper ||
 		    exp->class != class)
 			break;
-
+#ifdef CONFIG_NF_NAT_NEEDED
 		if (exp->tuple.src.l3num == AF_INET && !direct_rtp &&
 		    (exp->saved_ip != exp->tuple.dst.u3.ip ||
 		     exp->saved_proto.udp.port != exp->tuple.dst.u.udp.port) &&
@@ -791,6 +791,7 @@ static int set_expected_rtp_rtcp(struct sk_buff *skb,
 			tuple.dst.u.udp.port	= exp->saved_proto.udp.port;
 			direct_rtp = 1;
 		} else
+#endif
 			skip_expect = 1;
 	} while (!skip_expect);
 	rcu_read_unlock();

Re: [bug] build failure in net/netfilter/nf_conntrack_sip.c, on latest -git

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Sat, 19 Apr 2008 18:38:41 +0200

> David Miller wrote:
> > From: Ingo Molnar <mingo@elte.hu>
> > Date: Sat, 19 Apr 2008 12:00:35 +0200
> >
> > [ netfilter-devel and netdev added to CC: ]
> >
> >   
> >> randconfig testing found a netfilter build failure in latest -git:
> >>
> >>   net/netfilter/nf_conntrack_sip.c: In function 'set_expected_rtp_rtcp':
> >>   net/netfilter/nf_conntrack_sip.c:786: error: 'struct nf_conntrack_expect' has no member named 'saved_ip'
> >>
> >>   http://redhat.com/~mingo/misc/config-Sat_Apr_19_11_37_02_CEST_2008.bad
> >>
> >> disabling CONFIG_NF_CONNTRACK_SIP works it around.
> >>     
> >
> > I think this will fix it.
> >
> > Patrick, is this how we should do it?
> 
> No, the SIP helper is also useful without NAT. This patch adds
> an ifdef around the RTP call optimization for NATed clients.
> 
> Signed-off-by: Patrick McHardy <kaber@trash.net>

Fair enough, applied, thanks Patrick.

Re: [bug] build failure in net/netfilter/nf_conntrack_sip.c, on latest -git

[Jeff Garzik]

Ingo Molnar wrote:
> * David Miller <davem@davemloft.net> wrote:
> 
>> From: Ingo Molnar <mingo@elte.hu>
>> Date: Sat, 19 Apr 2008 12:00:35 +0200
>>
>>> btw., i found no thread to reply to on lkml or elsewhere - arent all 
>>> git pull requests supposed to be Cc:-ed to lkml, with shortlog 
>>> included?
>> I have never once done this even going all the way back to the BK 
>> days.
> 
> do you find this practice of private pull requests important? If not, 
> would it be difficult to Cc: lkml to routine pull requests?
> 
>> I always send my pull requests directly to Linus, CC:'ing Andrew.
> 
> that makes bugs harder to report and makes the flow of patches harder to 
> follow as well. Often (like in this case) i see it when a bug comes in 
> via a specific group of commits but i have no time to do a bisection. In 
> that case i'd like to report it to that pull request so i'd like to 
> reply to that pull on lkml.
> 
> But in this case i first did an unsuccessful full-text search on lkml, 
> then i also opened up netdev and did a full text search there too to 
> find the originator pull request or the patches but the search turned up 
> nothing. As the number of subsystems increases, i suspect you agree with 
> me that this does not scale very well for bug reporters, correct?

Well, 'git log [$file]' is even more scalable and precise, if committer 
(as well as author) info and patch flow info is what a tester or bug 
reporter seeks.

But it sounds like you are making an assumption about development 
/style/, then complaining when reality doesn't match that assumption.


> i'm convinced that testers and bug reporters are the scarce resource 
> these days, not patch integrators and not maintainers which was the 
> scarce resource 3-4 years ago, before Git and before BK. It is testing 
> (and review) capacity that limits the growth of Linux today, not patch 
> writing and patch integration capacity.

We're all for more bug reports and testing, but the link you are trying 
to draw is more than a little overblown.  I'm not convinced LKML's lack 
of davem pull request visibility is a problem source limiting the growth 
of Linux today...  That's a bit much.  :)

AFAICT you and davem have different development styles, he doesn't work 
the way you work, but the tone of your emails recently has been "if you 
do not fit my preferred development worldview, you are creating problems 
for Linux."  Witness not just this thread, but the LKML-vs-netdev 
discussions.

	Jeff

Re: [bug] build failure in net/netfilter/nf_conntrack_sip.c, on latest -git

[Ingo Molnar]

* Jeff Garzik <jeff@garzik.org> wrote:

>> But in this case i first did an unsuccessful full-text search on 
>> lkml, then i also opened up netdev and did a full text search there 
>> too to find the originator pull request or the patches but the search 
>> turned up nothing. As the number of subsystems increases, i suspect 
>> you agree with me that this does not scale very well for bug 
>> reporters, correct?
>
> Well, 'git log [$file]' is even more scalable and precise, if 
> committer (as well as author) info and patch flow info is what a 
> tester or bug reporter seeks.

but the only information i had at that point was that 'something in that 
recent appearance of a large group of networking commits introduced the 
problem'. There was no commit log entry of even the merge.

> But it sounds like you are making an assumption about development 
> /style/, then complaining when reality doesn't match that assumption.

no. I simply failed to put this (trivial) bugreport into some sort of 
existing context of discussion, and made a brief and neutral(-looking) 
comment about that. Under no other development style could i have done 
that because the context was simply missing from the mailing lists.

I did not intend this to be a big deal comment. It was literally just 
this single side-question:

||  disabling CONFIG_NF_CONNTRACK_SIP works it around.
||
||  btw., i found no thread to reply to on lkml or elsewhere - arent all 
||  git pull requests supposed to be Cc:-ed to lkml, with shortlog 
||  included?

I was seriously surprised that large pull requests and shortlogs dont go 
to lkml. Perhaps some sort of negative sentiment was perceived in that 
single sentence of mine? If yes, how should i have expressed this 
comment otherwise?

and i mean, lost context of discussion happens all the time and i do it 
too - recently Andrew complained that ftrace commits were not on lkml 
and he was complaining rightfully. We dont notice it unless people 
mention it.

[ later on the tone of the discussion degenerated indeed. ]

	Ingo

Re: [bug] build failure in net/netfilter/nf_conntrack_sip.c, on latest -git

[Herbert Xu]

Ingo Molnar <mingo@elte.hu> wrote:
>
> but the only information i had at that point was that 'something in that 
> recent appearance of a large group of networking commits introduced the 
> problem'. There was no commit log entry of even the merge.

It is human nature to blame someone/something when things go wrong.
It just happens that the lack of a merge request was the first you
tacked onto in this case :)

I don't think we can seriously argue that had there been a merge
request on lkml that these build issues would have somehow gone
away or resolved any sooner than they have been.  The fact is
with or without the merge request you would have realised that
network bug reports == davem :)

> no. I simply failed to put this (trivial) bugreport into some sort of 
> existing context of discussion, and made a brief and neutral(-looking) 
> comment about that. Under no other development style could i have done 
> that because the context was simply missing from the mailing lists.

Unfortunately the email medium tends to make what one thinks of as
a neutral and balanced critique appear anything but to the recipient.
And I must say that in this case your messages did have that
appearance.

My only suggestion is for us to all move on to something a little
bit more useful than letting off steam.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt