| Message ID | e0397d30-7405-a7af-286c-fe76887caf0a@oracle.com |
|---|---|
| State | Superseded |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next,1/5] RDS: Re-add pf/sol access via sysctl | expand |
On 8/13/19 11:20 AM, Gerd Rausch wrote: > From: Andy Grover <andy.grover@oracle.com> > Date: Tue, 24 Nov 2009 15:35:51 -0800 > > Although RDS has an official PF_RDS value now, existing software > expects to look for rds sysctls to determine it. We need to maintain > these for now, for backwards compatibility. > > Signed-off-by: Andy Grover <andy.grover@oracle.com> > Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com> > --- Acked-by: Santosh Shilimkar<santosh.shilimkar@oracle.com>
On Tue, 2019-08-13 at 11:20 -0700, Gerd Rausch wrote: > From: Andy Grover <andy.grover@oracle.com> > Date: Tue, 24 Nov 2009 15:35:51 -0800 > > Although RDS has an official PF_RDS value now, existing software > expects to look for rds sysctls to determine it. We need to maintain > these for now, for backwards compatibility. > > Signed-off-by: Andy Grover <andy.grover@oracle.com> > Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com> > --- > net/rds/sysctl.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/net/rds/sysctl.c b/net/rds/sysctl.c > index e381bbcd9cc1..9760292a0af4 100644 > --- a/net/rds/sysctl.c > +++ b/net/rds/sysctl.c > @@ -49,6 +49,13 @@ unsigned int rds_sysctl_max_unacked_bytes = (16 << > 20); > > unsigned int rds_sysctl_ping_enable = 1; > > +/* > + * We have official values, but must maintain the sysctl interface > for existing > + * software that expects to find these values here. > + */ > +static int rds_sysctl_pf_rds = PF_RDS; > +static int rds_sysctl_sol_rds = SOL_RDS; > + > static struct ctl_table rds_sysctl_rds_table[] = { > { > .procname = "reconnect_min_delay_ms", > @@ -68,6 +75,20 @@ static struct ctl_table rds_sysctl_rds_table[] = { > .extra1 = &rds_sysctl_reconnect_min_jiffies, > .extra2 = &rds_sysctl_reconnect_max, > }, > + { > + .procname = "pf_rds", > + .data = &rds_sysctl_pf_rds, > + .maxlen = sizeof(int), > + .mode = 0444, > + .proc_handler = &proc_dointvec, > + }, > + { > + .procname = "sol_rds", > + .data = &rds_sysctl_sol_rds, > + .maxlen = sizeof(int), > + .mode = 0444, > + .proc_handler = &proc_dointvec, > + }, > { > .procname = "max_unacked_packets", > .data = &rds_sysctl_max_unacked_packets, Good Lord...RDS was taken into the kernel in Feb of 2009, so over 10 years ago. The patch to put PF_RDS/AF_RDS/SOL_RDS was taken into include/linux/socket.h Feb 26, 2009. The RDS ports were allocated by IANA on Feb 27 and May 20, 2009. And you *still* have software that needs this? The only software that has ever used RDS was Oracle software. I would have expected you guys to update your source code to do the right thing long before now. In fact, I would expect you were ready to retire all of the legacy software that needs this by now. As of today, does your current build of Oracle software still require this, or have you at least fixed it up in your modern builds?
Hi Doug, On 14/08/2019 08.56, Doug Ledford wrote: > Good Lord...RDS was taken into the kernel in Feb of 2009, so over 10 > years ago. The patch to put PF_RDS/AF_RDS/SOL_RDS was taken into > include/linux/socket.h Feb 26, 2009. The RDS ports were allocated by > IANA on Feb 27 and May 20, 2009. And you *still* have software that > needs this? I'll let Santosh elaborate on this, but it looks like we (i.e. Oracle) do: From our Gerrit, posted on Aug 08, 2019, 10:39:29 AM UTC-07:00: --------%<--------%<--------%<--------%<--------%<--------%<-------- Santosh Shilimkar Acked-by +1 Patch Set 1: Acked-by+1 Unfortunately we need to keep these around. --------%<--------%<--------%<--------%<--------%<--------%<-------- > As of today, does your current build of Oracle software still require this, > or have you at least fixed it up in your modern builds? > I'll let Santosh answer that question as well. Thanks, Gerd
On 8/14/19 8:56 AM, Doug Ledford wrote: > On Tue, 2019-08-13 at 11:20 -0700, Gerd Rausch wrote: >> From: Andy Grover <andy.grover@oracle.com> >> Date: Tue, 24 Nov 2009 15:35:51 -0800 >> >> Although RDS has an official PF_RDS value now, existing software >> expects to look for rds sysctls to determine it. We need to maintain >> these for now, for backwards compatibility. >> >> Signed-off-by: Andy Grover <andy.grover@oracle.com> >> Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com> >> --- >> net/rds/sysctl.c | 21 +++++++++++++++++++++ >> 1 file changed, 21 insertions(+) >> >> diff --git a/net/rds/sysctl.c b/net/rds/sysctl.c >> index e381bbcd9cc1..9760292a0af4 100644 >> --- a/net/rds/sysctl.c >> +++ b/net/rds/sysctl.c >> @@ -49,6 +49,13 @@ unsigned int rds_sysctl_max_unacked_bytes = (16 << >> 20); >> >> unsigned int rds_sysctl_ping_enable = 1; >> >> +/* >> + * We have official values, but must maintain the sysctl interface >> for existing >> + * software that expects to find these values here. >> + */ >> +static int rds_sysctl_pf_rds = PF_RDS; >> +static int rds_sysctl_sol_rds = SOL_RDS; >> + >> static struct ctl_table rds_sysctl_rds_table[] = { >> { >> .procname = "reconnect_min_delay_ms", >> @@ -68,6 +75,20 @@ static struct ctl_table rds_sysctl_rds_table[] = { >> .extra1 = &rds_sysctl_reconnect_min_jiffies, >> .extra2 = &rds_sysctl_reconnect_max, >> }, >> + { >> + .procname = "pf_rds", >> + .data = &rds_sysctl_pf_rds, >> + .maxlen = sizeof(int), >> + .mode = 0444, >> + .proc_handler = &proc_dointvec, >> + }, >> + { >> + .procname = "sol_rds", >> + .data = &rds_sysctl_sol_rds, >> + .maxlen = sizeof(int), >> + .mode = 0444, >> + .proc_handler = &proc_dointvec, >> + }, >> { >> .procname = "max_unacked_packets", >> .data = &rds_sysctl_max_unacked_packets, > > Good Lord...RDS was taken into the kernel in Feb of 2009, so over 10 > years ago. The patch to put PF_RDS/AF_RDS/SOL_RDS was taken into > include/linux/socket.h Feb 26, 2009. The RDS ports were allocated by > IANA on Feb 27 and May 20, 2009. And you *still* have software that > needs this? The only software that has ever used RDS was Oracle > software. I would have expected you guys to update your source code to > do the right thing long before now. In fact, I would expect you were > ready to retire all of the legacy software that needs this by now. As > of today, does your current build of Oracle software still require this, > or have you at least fixed it up in your modern builds? > Some of the application software was released before 2009 and ended up using these proc entries from downstream kernel. The newer lib/app using RDS don't use these. Unfortunately lot of customer still use Oracle 9, 10, 11 which were released before 2007 and run these apps on modern kernels. Regards, Snatosh
From: santosh.shilimkar@oracle.com Date: Wed, 14 Aug 2019 11:01:36 -0700 > Some of the application software was released before 2009 and ended up > using these proc entries from downstream kernel. The newer lib/app > using RDS don't use these. Unfortunately lot of customer still use > Oracle 9, 10, 11 which were released before 2007 and run these apps > on modern kernels. So those apps are using proc entries that were never upstream... Sorry, this is completely and utterly inappropriate.
On 8/14/19 11:21 AM, David Miller wrote: > From: santosh.shilimkar@oracle.com > Date: Wed, 14 Aug 2019 11:01:36 -0700 > >> Some of the application software was released before 2009 and ended up >> using these proc entries from downstream kernel. The newer lib/app >> using RDS don't use these. Unfortunately lot of customer still use >> Oracle 9, 10, 11 which were released before 2007 and run these apps >> on modern kernels. > > So those apps are using proc entries that were never upstream... > > Sorry, this is completely and utterly inappropriate. > Agree. Unfortunately one of the legacy application library didn't get upgraded even after the ports were registered with IANA. Oracle 11 is still very active release and hence this patch. It is fine to drop $subject patch from this series. Regards, Santosh
From: santosh.shilimkar@oracle.com Date: Wed, 14 Aug 2019 11:36:19 -0700 > On 8/14/19 11:21 AM, David Miller wrote: >> From: santosh.shilimkar@oracle.com >> Date: Wed, 14 Aug 2019 11:01:36 -0700 >> >>> Some of the application software was released before 2009 and ended up >>> using these proc entries from downstream kernel. The newer lib/app >>> using RDS don't use these. Unfortunately lot of customer still use >>> Oracle 9, 10, 11 which were released before 2007 and run these apps >>> on modern kernels. >> So those apps are using proc entries that were never upstream... >> >> Sorry, this is completely and utterly inappropriate. >> > Agree. Unfortunately one of the legacy application library didn't > get upgraded even after the ports were registered with IANA. > Oracle 11 is still very active release and hence this patch. > > It is fine to drop $subject patch from this series. The appropriate procedure is to resubmit the series with the patch removed. Thank you.
Hi David, On 14/08/2019 14.31, David Miller wrote: > From: santosh.shilimkar@oracle.com > Date: Wed, 14 Aug 2019 11:36:19 -0700 > >> On 8/14/19 11:21 AM, David Miller wrote: >>> From: santosh.shilimkar@oracle.com >>> Date: Wed, 14 Aug 2019 11:01:36 -0700 >>> >>>> Some of the application software was released before 2009 and ended up >>>> using these proc entries from downstream kernel. The newer lib/app >>>> using RDS don't use these. Unfortunately lot of customer still use >>>> Oracle 9, 10, 11 which were released before 2007 and run these apps >>>> on modern kernels. >>> So those apps are using proc entries that were never upstream... >>> >>> Sorry, this is completely and utterly inappropriate. >>> >> Agree. Unfortunately one of the legacy application library didn't >> get upgraded even after the ports were registered with IANA. >> Oracle 11 is still very active release and hence this patch. >> >> It is fine to drop $subject patch from this series. > > The appropriate procedure is to resubmit the series with the patch > removed. > For my understanding: Are you saying that... a) It is utterly inappropriate to have Oracle applications rely on a /proc/sys API that was kept out of Upstream-Linux for this long b) It is utterly inappropriate to include such a /proc/sys API that Oracle applications have depended on this late c) ... something else ... At first I read your comment as "a)", which would then imply that this commit shall be included now (albeit late). If your answer is "not a)", or implies that Oracle ought to continue to carry this change in our own repository only, please let me know, and I will re-submit the series without this patch, to follow appropriate procedure. Thanks, Gerd
From: Gerd Rausch <gerd.rausch@oracle.com> Date: Wed, 14 Aug 2019 14:45:21 -0700 > a) It is utterly inappropriate to have Oracle applications > rely on a /proc/sys API that was kept out of Upstream-Linux > for this long Yes. > > b) It is utterly inappropriate to include such a /proc/sys API > that Oracle applications have depended on this late Also yes.
diff --git a/net/rds/sysctl.c b/net/rds/sysctl.c index e381bbcd9cc1..9760292a0af4 100644 --- a/net/rds/sysctl.c +++ b/net/rds/sysctl.c @@ -49,6 +49,13 @@ unsigned int rds_sysctl_max_unacked_bytes = (16 << 20); unsigned int rds_sysctl_ping_enable = 1; +/* + * We have official values, but must maintain the sysctl interface for existing + * software that expects to find these values here. + */ +static int rds_sysctl_pf_rds = PF_RDS; +static int rds_sysctl_sol_rds = SOL_RDS; + static struct ctl_table rds_sysctl_rds_table[] = { { .procname = "reconnect_min_delay_ms", @@ -68,6 +75,20 @@ static struct ctl_table rds_sysctl_rds_table[] = { .extra1 = &rds_sysctl_reconnect_min_jiffies, .extra2 = &rds_sysctl_reconnect_max, }, + { + .procname = "pf_rds", + .data = &rds_sysctl_pf_rds, + .maxlen = sizeof(int), + .mode = 0444, + .proc_handler = &proc_dointvec, + }, + { + .procname = "sol_rds", + .data = &rds_sysctl_sol_rds, + .maxlen = sizeof(int), + .mode = 0444, + .proc_handler = &proc_dointvec, + }, { .procname = "max_unacked_packets", .data = &rds_sysctl_max_unacked_packets,