linux-next: manual merge of the sysctl tree with the net tree

linux-next: manual merge of the sysctl tree with the net tree

[Stephen Rothwell]

Hi Eric,

Today's linux-next merge of the sysctl tree got a conflict in
net/sctp/sysctl.c between commit 90f2f5318b3a5b0898fef0fec9b91376c7de7a2c
("sctp: Update SWS avaoidance receiver side algorithm") from the net tree
and commit f8572d8f2a2ba75408b97dc24ef47c83671795d7 ("sysctl net: Remove
unused binary sysctl code") from the sysctl tree.

I fixed it up (see below) and can carry the fix as necessary.  I also
removed the strategy member from the new added ctl_table entry.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc net/sctp/sysctl.c
index ae03ded,d50a042..0000000
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@@ -285,19 -241,7 +242,17 @@@ static ctl_table sctp_table[] = 
  		.extra1		= &zero,
  		.extra2		= &addr_scope_max,
  	},
 +	{
- 		.ctl_name	= CTL_UNNUMBERED,
 +		.procname	= "rwnd_update_shift",
 +		.data		= &sctp_rwnd_upd_shift,
 +		.maxlen		= sizeof(int),
 +		.mode		= 0644,
- 		.proc_handler	= &proc_dointvec_minmax,
- 		.strategy	= &sysctl_intvec,
++		.proc_handler	= proc_dointvec_minmax,
 +		.extra1		= &one,
 +		.extra2		= &rwnd_scale_max,
 +	},
 +
- 	{ .ctl_name = 0 }
+ 	{ }
  };
  
  static struct ctl_path sctp_path[] = {

Re: linux-next: manual merge of the sysctl tree with the net tree

[Américo Wang]

On Mon, Nov 30, 2009 at 1:44 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi Eric,
>
> Today's linux-next merge of the sysctl tree got a conflict in
> net/sctp/sysctl.c between commit 90f2f5318b3a5b0898fef0fec9b91376c7de7a2c
> ("sctp: Update SWS avaoidance receiver side algorithm") from the net tree
> and commit f8572d8f2a2ba75408b97dc24ef47c83671795d7 ("sysctl net: Remove
> unused binary sysctl code") from the sysctl tree.
>
> I fixed it up (see below) and can carry the fix as necessary.  I also
> removed the strategy member from the new added ctl_table entry.
> --
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
>
> diff --cc net/sctp/sysctl.c
> index ae03ded,d50a042..0000000
> --- a/net/sctp/sysctl.c
> +++ b/net/sctp/sysctl.c
> @@@ -285,19 -241,7 +242,17 @@@ static ctl_table sctp_table[] =
>                .extra1         = &zero,
>                .extra2         = &addr_scope_max,
>        },
>  +      {
> -               .ctl_name       = CTL_UNNUMBERED,
>  +              .procname       = "rwnd_update_shift",
>  +              .data           = &sctp_rwnd_upd_shift,
>  +              .maxlen         = sizeof(int),
>  +              .mode           = 0644,
> -               .proc_handler   = &proc_dointvec_minmax,
> -               .strategy       = &sysctl_intvec,
> ++              .proc_handler   = proc_dointvec_minmax,

Hey, what's this??

Re: linux-next: manual merge of the sysctl tree with the net tree

[Eric W. Biederman]

Américo Wang <xiyou.wangcong@gmail.com> writes:

> On Mon, Nov 30, 2009 at 1:44 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>> Hi Eric,
>>
>> Today's linux-next merge of the sysctl tree got a conflict in
>> net/sctp/sysctl.c between commit 90f2f5318b3a5b0898fef0fec9b91376c7de7a2c
>> ("sctp: Update SWS avaoidance receiver side algorithm") from the net tree
>> and commit f8572d8f2a2ba75408b97dc24ef47c83671795d7 ("sysctl net: Remove
>> unused binary sysctl code") from the sysctl tree.
>>
>> I fixed it up (see below) and can carry the fix as necessary.  I also
>> removed the strategy member from the new added ctl_table entry.
>> --
>> Cheers,
>> Stephen Rothwell                    sfr@canb.auug.org.au
>>
>> diff --cc net/sctp/sysctl.c
>> index ae03ded,d50a042..0000000
>> --- a/net/sctp/sysctl.c
>> +++ b/net/sctp/sysctl.c
>> @@@ -285,19 -241,7 +242,17 @@@ static ctl_table sctp_table[] =
>>                .extra1         = &zero,
>>                .extra2         = &addr_scope_max,
>>        },
>>  +      {
>> -               .ctl_name       = CTL_UNNUMBERED,
>>  +              .procname       = "rwnd_update_shift",
>>  +              .data           = &sctp_rwnd_upd_shift,
>>  +              .maxlen         = sizeof(int),
>>  +              .mode           = 0644,
>> -               .proc_handler   = &proc_dointvec_minmax,
>> -               .strategy       = &sysctl_intvec,
>> ++              .proc_handler   = proc_dointvec_minmax,
>
> Hey, what's this??

The short version is I am running a git tree that holds all of
the necessary cleanups to remove the support for binary sysctl handlers.

The binary sysctl support continues to be provided in kernel/sysctl_binary.c
with a compatibility wrapper.  This has been reviewed on linux-kernel
and written up in lwn.

In my tree .ctl_name and .strategy have been removed as they exist
only to support binary sysctls and are not strictly needed today.
.ctl_name = CTL_UNNUMBERED is equivalent to .ctl_name = 0, and setting
.strategy on new sysctl table entries without a ctl_name is a harmless
bug.  Since I was in there I also removed all of the unnecessary ampersands
from in front of proc_dointvec_minmax.

Since I have touched practically every sysctl table entry in the kernel
new sysctl additions will almost inevitably cause a small by trivially
to resolve conflict (due to the fact I have almost certainly changed
the proceeding and succeeding sysctl table entries).

Currently this only the second sysctl added this kernel cycle, and it
looks like this work happened in parallel, with my changes, and somehow
David missed this commit in his September pull, so the changes just
showed up in net-next.

It would seem to require talent to mess up the merge conflicts, and
getting it wrong will result in a tree that won't compile so I am not
going to worry about it until Linux pulls one of our trees.

Eric

Re: linux-next: manual merge of the sysctl tree with the net tree

[Américo Wang]

On Mon, Nov 30, 2009 at 2:39 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Américo Wang <xiyou.wangcong@gmail.com> writes:
>
>> On Mon, Nov 30, 2009 at 1:44 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>> Hi Eric,
>>>
>>> Today's linux-next merge of the sysctl tree got a conflict in
>>> net/sctp/sysctl.c between commit 90f2f5318b3a5b0898fef0fec9b91376c7de7a2c
>>> ("sctp: Update SWS avaoidance receiver side algorithm") from the net tree
>>> and commit f8572d8f2a2ba75408b97dc24ef47c83671795d7 ("sysctl net: Remove
>>> unused binary sysctl code") from the sysctl tree.
>>>
>>> I fixed it up (see below) and can carry the fix as necessary.  I also
>>> removed the strategy member from the new added ctl_table entry.
>>> --
>>> Cheers,
>>> Stephen Rothwell                    sfr@canb.auug.org.au
>>>
>>> diff --cc net/sctp/sysctl.c
>>> index ae03ded,d50a042..0000000
>>> --- a/net/sctp/sysctl.c
>>> +++ b/net/sctp/sysctl.c
>>> @@@ -285,19 -241,7 +242,17 @@@ static ctl_table sctp_table[] =
>>>                .extra1         = &zero,
>>>                .extra2         = &addr_scope_max,
>>>        },
>>>  +      {
>>> -               .ctl_name       = CTL_UNNUMBERED,
>>>  +              .procname       = "rwnd_update_shift",
>>>  +              .data           = &sctp_rwnd_upd_shift,
>>>  +              .maxlen         = sizeof(int),
>>>  +              .mode           = 0644,
>>> -               .proc_handler   = &proc_dointvec_minmax,
>>> -               .strategy       = &sysctl_intvec,
>>> ++              .proc_handler   = proc_dointvec_minmax,
>>
>> Hey, what's this??
>
> The short version is I am running a git tree that holds all of
> the necessary cleanups to remove the support for binary sysctl handlers.
>
> The binary sysctl support continues to be provided in kernel/sysctl_binary.c
> with a compatibility wrapper.  This has been reviewed on linux-kernel
> and written up in lwn.

Yeah, I saw your patches, but didn't have a chance to look at them closely.

>
> In my tree .ctl_name and .strategy have been removed as they exist
> only to support binary sysctls and are not strictly needed today.
> .ctl_name = CTL_UNNUMBERED is equivalent to .ctl_name = 0, and setting
> .strategy on new sysctl table entries without a ctl_name is a harmless
> bug.  Since I was in there I also removed all of the unnecessary ampersands
> from in front of proc_dointvec_minmax.
>
> Since I have touched practically every sysctl table entry in the kernel
> new sysctl additions will almost inevitably cause a small by trivially
> to resolve conflict (due to the fact I have almost certainly changed
> the proceeding and succeeding sysctl table entries).
>
> Currently this only the second sysctl added this kernel cycle, and it
> looks like this work happened in parallel, with my changes, and somehow
> David missed this commit in his September pull, so the changes just
> showed up in net-next.
>
> It would seem to require talent to mess up the merge conflicts, and
> getting it wrong will result in a tree that won't compile so I am not
> going to worry about it until Linux pulls one of our trees.

Thanks for this explanation, I see...

Re: linux-next: manual merge of the sysctl tree with the net tree

[Stephen Rothwell]

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

Hi Américo,

On Mon, 30 Nov 2009 13:51:03 +0800 Américo Wang <xiyou.wangcong@gmail.com> wrote:
>
> > diff --cc net/sctp/sysctl.c
> > index ae03ded,d50a042..0000000
> > --- a/net/sctp/sysctl.c
> > +++ b/net/sctp/sysctl.c
> > @@@ -285,19 -241,7 +242,17 @@@ static ctl_table sctp_table[] =
> >                .extra1         = &zero,
> >                .extra2         = &addr_scope_max,
> >        },
> >  +      {
> > -               .ctl_name       = CTL_UNNUMBERED,
> >  +              .procname       = "rwnd_update_shift",
> >  +              .data           = &sctp_rwnd_upd_shift,
> >  +              .maxlen         = sizeof(int),
> >  +              .mode           = 0644,
> > -               .proc_handler   = &proc_dointvec_minmax,
> > -               .strategy       = &sysctl_intvec,
> > ++              .proc_handler   = proc_dointvec_minmax,
> 
> Hey, what's this??

If you mean "what is this strange looking patch", then it is the output
from "git diff --cc" after a merge conflict has been fixed up, but before
it is committed.  The '-' lines were added by one side of the merge or
the other, but removed from the final result.  The line with '++' did not
appear in either branch, but is in the final result.  The lines with a
single '+' appear in one of the branches or the other and in the final
result as well.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: manual merge of the sysctl tree with the net tree

[Américo Wang]

On Mon, Nov 30, 2009 at 2:41 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi Américo,
>
> On Mon, 30 Nov 2009 13:51:03 +0800 Américo Wang <xiyou.wangcong@gmail.com> wrote:
>>
>> > diff --cc net/sctp/sysctl.c
>> > index ae03ded,d50a042..0000000
>> > --- a/net/sctp/sysctl.c
>> > +++ b/net/sctp/sysctl.c
>> > @@@ -285,19 -241,7 +242,17 @@@ static ctl_table sctp_table[] =
>> >                .extra1         = &zero,
>> >                .extra2         = &addr_scope_max,
>> >        },
>> >  +      {
>> > -               .ctl_name       = CTL_UNNUMBERED,
>> >  +              .procname       = "rwnd_update_shift",
>> >  +              .data           = &sctp_rwnd_upd_shift,
>> >  +              .maxlen         = sizeof(int),
>> >  +              .mode           = 0644,
>> > -               .proc_handler   = &proc_dointvec_minmax,
>> > -               .strategy       = &sysctl_intvec,
>> > ++              .proc_handler   = proc_dointvec_minmax,
>>
>> Hey, what's this??
>
> If you mean "what is this strange looking patch", then it is the output
> from "git diff --cc" after a merge conflict has been fixed up, but before
> it is committed.  The '-' lines were added by one side of the merge or
> the other, but removed from the final result.  The line with '++' did not
> appear in either branch, but is in the final result.  The lines with a
> single '+' appear in one of the branches or the other and in the final
> result as well.
>

Thanks for teaching this! I thought it was a mistake, it is not. :)
No problem then.

Re: linux-next: manual merge of the sysctl tree with the net tree

[Vlad Yasevich]

Stephen Rothwell wrote:
> Hi Eric,
> 
> Today's linux-next merge of the sysctl tree got a conflict in
> net/sctp/sysctl.c between commit 90f2f5318b3a5b0898fef0fec9b91376c7de7a2c
> ("sctp: Update SWS avaoidance receiver side algorithm") from the net tree
> and commit f8572d8f2a2ba75408b97dc24ef47c83671795d7 ("sysctl net: Remove
> unused binary sysctl code") from the sysctl tree.
> 
> I fixed it up (see below) and can carry the fix as necessary.  I also
> removed the strategy member from the new added ctl_table entry.

Thanks for fixing this up Stephen.  I was basing my work on linux-next
which, it appears, didn't have the sysctl changes.

-vlad

linux-next: manual merge of the sysctl tree with the net tree

[Stephen Rothwell]

Hi Eric,

Today's linux-next merge of the sysctl tree got a conflict in
net/ipv4/sysctl_net_ipv4.c between commit
519855c508b9a17878c0977a3cdefc09b59b30df ("TCPCT part 1c:
sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS") from the
net tree and commit f8572d8f2a2ba75408b97dc24ef47c83671795d7 ("sysctl
net: Remove unused binary sysctl code") from the sysctl tree.

I fixed it up (see below) and can carry the fix as necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc net/ipv4/sysctl_net_ipv4.c
index 13f7ab6,3000567..0000000
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@@ -713,15 -569,6 +569,13 @@@ static struct ctl_table ipv4_table[] = 
  		.proc_handler	= proc_dointvec,
  	},
  	{
- 		.ctl_name	= CTL_UNNUMBERED,
 +		.procname	= "tcp_cookie_size",
 +		.data		= &sysctl_tcp_cookie_size,
 +		.maxlen		= sizeof(int),
 +		.mode		= 0644,
 +		.proc_handler	= proc_dointvec
 +	},
 +	{
- 		.ctl_name	= CTL_UNNUMBERED,
  		.procname	= "udp_mem",
  		.data		= &sysctl_udp_mem,
  		.maxlen		= sizeof(sysctl_udp_mem),

Re: linux-next: manual merge of the sysctl tree with the net tree

[William Allen Simpson]

Stephen Rothwell wrote:
> Hi Eric,
> 
> Today's linux-next merge of the sysctl tree got a conflict in
> net/ipv4/sysctl_net_ipv4.c between commit
> 519855c508b9a17878c0977a3cdefc09b59b30df ("TCPCT part 1c:
> sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS") from the
> net tree and commit f8572d8f2a2ba75408b97dc24ef47c83671795d7 ("sysctl
> net: Remove unused binary sysctl code") from the sysctl tree.
> 
> I fixed it up (see below) and can carry the fix as necessary.

Hi, I'm not Eric, but it's my TCPCT patch -- is there something more
that I need to do?

Adding a sysctl was my very first query to Linux lists.  I thought
everything was covered in that and subsequent threads.

    Subject: query: adding a sysctl
    Date: Fri, 02 Oct 2009 00:00:05 -0400

Re: linux-next: manual merge of the sysctl tree with the net tree

[Stephen Rothwell]

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

Hi William,

On Fri, 04 Dec 2009 07:16:35 -0500 William Allen Simpson <william.allen.simpson@gmail.com> wrote:
>
> Hi, I'm not Eric, but it's my TCPCT patch -- is there something more
> that I need to do?

No, at some point the maintainer of whichever tree gets merged last by
Linus will fix this up (or maybe Linus himself will).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: manual merge of the sysctl tree with the net tree

[Eric W. Biederman]

William Allen Simpson <william.allen.simpson@gmail.com> writes:

> Stephen Rothwell wrote:
>> Hi Eric,
>>
>> Today's linux-next merge of the sysctl tree got a conflict in
>> net/ipv4/sysctl_net_ipv4.c between commit
>> 519855c508b9a17878c0977a3cdefc09b59b30df ("TCPCT part 1c:
>> sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS") from the
>> net tree and commit f8572d8f2a2ba75408b97dc24ef47c83671795d7 ("sysctl
>> net: Remove unused binary sysctl code") from the sysctl tree.
>>
>> I fixed it up (see below) and can carry the fix as necessary.
>
> Hi, I'm not Eric, but it's my TCPCT patch -- is there something more
> that I need to do?
>
> Adding a sysctl was my very first query to Linux lists.  I thought
> everything was covered in that and subsequent threads.

My apologies for not giving you a heads up earlier.

You initially ran into the problem in sysctl_check where you added a
new binary sysctl and you had errors.  That works to keep people from
adding new binary sysctl but as you experienced it is not the most
obvious way of communicating.

In my sysctl tree I have written a compatibility layer for all of the
existing binary sysctl handlers, and have removed the .ctl_name and
.strategy fields, that used to be used for implementing binary
sysctls.

In the best case scenario you could have seen that coming and not
included the .ctl_name line in your patch.  Then there would have been
a trivial conflict that would have required no changes to fixup.
However since I removed .ctl_name from before and after your new entry
a minor merge conflict was inevitable.

The price it seems for multiple independent development trees are the
occasionally conflicts in trees hosting kernel wide cleanups and fixes.

Eric

linux-next: manual merge of the sysctl tree with the net tree

[Stephen Rothwell]

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

Hi Eric,

Today's linux-next merge of the sysctl tree got a conflict in
kernel/sysctl_check.c between commit
8153a10c08f1312af563bb92532002e46d3f504a ("ipv4: add sysctl to accept
packets with local source addresses") from the net tree and commit
83ac201b4f06eb8aeb7ac93cf162651ba30e0b28 ("sysctl: Remove dead code from
sysctl_check") from the sysctl tree.

The latter removes the need for the former to change this file.  I just
used the latter.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: manual merge of the sysctl tree with the net tree

[Eric W. Biederman]

Stephen Rothwell <sfr@canb.auug.org.au> writes:

> Hi Eric,
>
> Today's linux-next merge of the sysctl tree got a conflict in
> kernel/sysctl_check.c between commit
> 8153a10c08f1312af563bb92532002e46d3f504a ("ipv4: add sysctl to accept
> packets with local source addresses") from the net tree and commit
> 83ac201b4f06eb8aeb7ac93cf162651ba30e0b28 ("sysctl: Remove dead code from
> sysctl_check") from the sysctl tree.
>
> The latter removes the need for the former to change this file.  I just
> used the latter.

Rats.  I feel blind.  I totally missed the fact that sysctl_check was
changed in that one.

Eric

linux-next: manual merge of the sysctl tree with the net tree

[Stephen Rothwell]

Hi Eric,

Today's linux-next merge of the sysctl tree got a conflict in
net/ipv6/addrconf.c between commit
f7734fdf61ec6bb848e0bafc1fb8bad2c124bb50 ("make TLLAO option for NA
packets configurable") from the net tree and commit
f8572d8f2a2ba75408b97dc24ef47c83671795d7 ("sysctl net: Remove unused
binary sysctl code") from the sysctl tree.

I fixed it up (see below) including removing the new ctl_name (thanks for
the heads up, Eric).   I can carry this fixup as necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc net/ipv6/addrconf.c
index 522bdc7,f918399..0000000
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@@ -4388,15 -4285,7 +4320,14 @@@ static struct addrconf_sysctl_tabl
  			.proc_handler	=	proc_dointvec,
  		},
  		{
- 			.ctl_name       = CTL_UNNUMBERED,
 +			.procname       = "force_tllao",
 +			.data           = &ipv6_devconf.force_tllao,
 +			.maxlen         = sizeof(int),
 +			.mode           = 0644,
 +			.proc_handler   = proc_dointvec
 +		},
 +		{
- 			.ctl_name	=	0,	/* sentinel */
+ 			/* sentinel */
  		}
  	},
  };

Re: linux-next: manual merge of the sysctl tree with the net tree

[Eric W. Biederman]

Stephen Rothwell <sfr@canb.auug.org.au> writes:

> Hi Eric,
>
> Today's linux-next merge of the sysctl tree got a conflict in
> net/ipv6/addrconf.c between commit
> f7734fdf61ec6bb848e0bafc1fb8bad2c124bb50 ("make TLLAO option for NA
> packets configurable") from the net tree and commit
> f8572d8f2a2ba75408b97dc24ef47c83671795d7 ("sysctl net: Remove unused
> binary sysctl code") from the sysctl tree.
>
> I fixed it up (see below) including removing the new ctl_name (thanks for
> the heads up, Eric).   I can carry this fixup as necessary.

Would it be of any value for me to send David a change killing ctl_name
for the new entry?  That is equivalent to .ctl_name = CTL_UNNUMBERED and
would make this a trivial conflict.

Fundamentally the conflict detection will always kick in here because
as I am modifying previous and succeeding entries in the table so the
context will always be different.

I'm still getting a hang of running a public git tree.

Eric

> diff --cc net/ipv6/addrconf.c
> index 522bdc7,f918399..0000000
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@@ -4388,15 -4285,7 +4320,14 @@@ static struct addrconf_sysctl_tabl
>   			.proc_handler	=	proc_dointvec,
>   		},
>   		{
> - 			.ctl_name       = CTL_UNNUMBERED,
>  +			.procname       = "force_tllao",
>  +			.data           = &ipv6_devconf.force_tllao,
>  +			.maxlen         = sizeof(int),
>  +			.mode           = 0644,
>  +			.proc_handler   = proc_dointvec
>  +		},
>  +		{
> - 			.ctl_name	=	0,	/* sentinel */
> + 			/* sentinel */
>   		}
>   	},
>   };