New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 758081 link

Starred by 0 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Enable `sysctl net.unix.max_dgram_qlen` inside user namespaces, in an upstreamable way.

Project Member Reported by kinaba@chromium.org, Aug 23 2017

Issue description

Spinning out from b/64655659.

On the current Linux kernel,
the parameter is fixed to the default value 10 inside a network namespace owned by a (non-init) user namespace.

https://github.com/torvalds/linux/blob/v4.13-rc6/net/unix/af_unix.c#L2916
https://github.com/torvalds/linux/blob/v4.13-rc6/net/unix/sysctl_net_unix.c#L37

However it is useful for ARC container to be able to customized the value (to make it aligned to the stock Android that uses max_dgram_qlen=600.)
https://android.googlesource.com/platform/system/core/+/c4f21639543fc6ccd0c18bfcdc12d34d5d7d4339/rootdir/init.rc#126


We'd like to come up with a nice way acceptable to the upstream Linux community,
in a safe, and backward-compatible manner.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 24 2017

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/fa7f9d27f08874e13dcc6730a11985d897baf349

commit fa7f9d27f08874e13dcc6730a11985d897baf349
Author: Kazuhiro Inaba <kinaba@chromium.org>
Date: Thu Aug 24 04:17:29 2017

CHROMIUM: net: Increase net.unix.max_dgram_qlen.

The parameter is 10 by default and 600 on Android. This CL sets the
value to 60, large enough for ARC to pass the CTS tests that depend
on logcat and thus are largely affected by the config. Experiments on
a Reef device running the test noted in the TEST field showed:

   max_dgram_qlen  pass_rate
   -------------------------
        10            12%
        30            80%
        50            98%
        60           100%
       100           100%
       600           100%

Unfortunately, normal method like sysctl or /proc/sys cannot modify the
parameter inside namespaces. To quickly fix it up for the Android
container, we directly patch the kernel for now.

BUG=b:64655659
BUG=chromium:758081
TEST=test_that reef cheets_CTS_N.7.1_r8.x86.CtsServicesHostTestCases

Change-Id: Id00efd1a300620f57eb76b361d0b71c895dce2c2
Signed-off-by: Kazuhiro Inaba <kinaba@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/627966
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>

[modify] https://crrev.com/fa7f9d27f08874e13dcc6730a11985d897baf349/net/unix/af_unix.c

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 26 2017

Labels: merge-merged-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/3aa1451e5549a2a7b0a2397391df4241ae869ce4

commit 3aa1451e5549a2a7b0a2397391df4241ae869ce4
Author: Kazuhiro Inaba <kinaba@chromium.org>
Date: Sat Aug 26 09:44:50 2017

CHROMIUM: net: Increase net.unix.max_dgram_qlen.

The parameter is 10 by default and 600 on Android. This CL sets the
value to 60, large enough for ARC to pass the CTS tests that depend
on logcat and thus are largely affected by the config. Experiments on
a Reef device running the test noted in the TEST field showed:

   max_dgram_qlen  pass_rate
   -------------------------
        10            12%
        30            80%
        50            98%
        60           100%
       100           100%
       600           100%

Unfortunately, normal method like sysctl or /proc/sys cannot modify the
parameter inside namespaces. To quickly fix it up for the Android
container, we directly patch the kernel for now.

BUG=b:64655659
BUG=chromium:758081
TEST=test_that reef cheets_CTS_N.7.1_r8.x86.CtsServicesHostTestCases

Change-Id: Id00efd1a300620f57eb76b361d0b71c895dce2c2
Signed-off-by: Kazuhiro Inaba <kinaba@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/627966
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
(cherry picked from commit fa7f9d27f08874e13dcc6730a11985d897baf349)
Reviewed-on: https://chromium-review.googlesource.com/636923

[modify] https://crrev.com/3aa1451e5549a2a7b0a2397391df4241ae869ce4/net/unix/af_unix.c

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 26 2017

Labels: merge-merged-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/75b344ab78709b7cb10af30a6434d9cda0a9981b

commit 75b344ab78709b7cb10af30a6434d9cda0a9981b
Author: Kazuhiro Inaba <kinaba@chromium.org>
Date: Sat Aug 26 09:44:51 2017

CHROMIUM: net: Increase net.unix.max_dgram_qlen.

The parameter is 10 by default and 600 on Android. This CL sets the
value to 60, large enough for ARC to pass the CTS tests that depend
on logcat and thus are largely affected by the config. Experiments on
a Reef device running the test noted in the TEST field showed:

   max_dgram_qlen  pass_rate
   -------------------------
        10            12%
        30            80%
        50            98%
        60           100%
       100           100%
       600           100%

Unfortunately, normal method like sysctl or /proc/sys cannot modify the
parameter inside namespaces. To quickly fix it up for the Android
container, we directly patch the kernel for now.

BUG=b:64655659
BUG=chromium:758081
TEST=test_that reef cheets_CTS_N.7.1_r8.x86.CtsServicesHostTestCases

Change-Id: Id00efd1a300620f57eb76b361d0b71c895dce2c2
Signed-off-by: Kazuhiro Inaba <kinaba@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/627966
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
(cherry picked from commit fa7f9d27f08874e13dcc6730a11985d897baf349)
Reviewed-on: https://chromium-review.googlesource.com/636903

[modify] https://crrev.com/75b344ab78709b7cb10af30a6434d9cda0a9981b/net/unix/af_unix.c

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 28 2017

Labels: merge-merged-release-R61-9765.B-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/cc5d306fe1400cdfdde3e259237f3cfc2b8947f4

commit cc5d306fe1400cdfdde3e259237f3cfc2b8947f4
Author: Kazuhiro Inaba <kinaba@chromium.org>
Date: Mon Aug 28 21:38:44 2017

CHROMIUM: net: Increase net.unix.max_dgram_qlen.

The parameter is 10 by default and 600 on Android. This CL sets the
value to 60, large enough for ARC to pass the CTS tests that depend
on logcat and thus are largely affected by the config. Experiments on
a Reef device running the test noted in the TEST field showed:

   max_dgram_qlen  pass_rate
   -------------------------
        10            12%
        30            80%
        50            98%
        60           100%
       100           100%
       600           100%

Unfortunately, normal method like sysctl or /proc/sys cannot modify the
parameter inside namespaces. To quickly fix it up for the Android
container, we directly patch the kernel for now.

BUG=b:64655659
BUG=chromium:758081
TEST=test_that reef cheets_CTS_N.7.1_r8.x86.CtsServicesHostTestCases

Change-Id: Id00efd1a300620f57eb76b361d0b71c895dce2c2
Signed-off-by: Kazuhiro Inaba <kinaba@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/627966
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
(cherry picked from commit fa7f9d27f08874e13dcc6730a11985d897baf349)
Reviewed-on: https://chromium-review.googlesource.com/636923
(cherry picked from commit 3aa1451e5549a2a7b0a2397391df4241ae869ce4)
Reviewed-on: https://chromium-review.googlesource.com/639593

[modify] https://crrev.com/cc5d306fe1400cdfdde3e259237f3cfc2b8947f4/net/unix/af_unix.c

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 28 2017

Labels: merge-merged-release-R61-9765.B-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/2bac8d6a9eb17da3ad99341f10cd648cfbac0227

commit 2bac8d6a9eb17da3ad99341f10cd648cfbac0227
Author: Kazuhiro Inaba <kinaba@chromium.org>
Date: Mon Aug 28 21:38:55 2017

CHROMIUM: net: Increase net.unix.max_dgram_qlen.

The parameter is 10 by default and 600 on Android. This CL sets the
value to 60, large enough for ARC to pass the CTS tests that depend
on logcat and thus are largely affected by the config. Experiments on
a Reef device running the test noted in the TEST field showed:

   max_dgram_qlen  pass_rate
   -------------------------
        10            12%
        30            80%
        50            98%
        60           100%
       100           100%
       600           100%

Unfortunately, normal method like sysctl or /proc/sys cannot modify the
parameter inside namespaces. To quickly fix it up for the Android
container, we directly patch the kernel for now.

BUG=b:64655659
BUG=chromium:758081
TEST=test_that reef cheets_CTS_N.7.1_r8.x86.CtsServicesHostTestCases

Change-Id: Id00efd1a300620f57eb76b361d0b71c895dce2c2
Signed-off-by: Kazuhiro Inaba <kinaba@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/627966
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
(cherry picked from commit fa7f9d27f08874e13dcc6730a11985d897baf349)
Reviewed-on: https://chromium-review.googlesource.com/639594

[modify] https://crrev.com/2bac8d6a9eb17da3ad99341f10cd648cfbac0227/net/unix/af_unix.c

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 28 2017

Labels: merge-merged-release-R61-9765.B-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/cf8b2a86d44d9f6c5e3f19662cc05b45928895c9

commit cf8b2a86d44d9f6c5e3f19662cc05b45928895c9
Author: Kazuhiro Inaba <kinaba@chromium.org>
Date: Mon Aug 28 21:38:59 2017

CHROMIUM: net: Increase net.unix.max_dgram_qlen.

The parameter is 10 by default and 600 on Android. This CL sets the
value to 60, large enough for ARC to pass the CTS tests that depend
on logcat and thus are largely affected by the config. Experiments on
a Reef device running the test noted in the TEST field showed:

   max_dgram_qlen  pass_rate
   -------------------------
        10            12%
        30            80%
        50            98%
        60           100%
       100           100%
       600           100%

Unfortunately, normal method like sysctl or /proc/sys cannot modify the
parameter inside namespaces. To quickly fix it up for the Android
container, we directly patch the kernel for now.

BUG=b:64655659
BUG=chromium:758081
TEST=test_that reef cheets_CTS_N.7.1_r8.x86.CtsServicesHostTestCases

Change-Id: Id00efd1a300620f57eb76b361d0b71c895dce2c2
Signed-off-by: Kazuhiro Inaba <kinaba@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/627966
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
(cherry picked from commit fa7f9d27f08874e13dcc6730a11985d897baf349)
Reviewed-on: https://chromium-review.googlesource.com/636903
(cherry picked from commit 75b344ab78709b7cb10af30a6434d9cda0a9981b)
Reviewed-on: https://chromium-review.googlesource.com/639592

[modify] https://crrev.com/cf8b2a86d44d9f6c5e3f19662cc05b45928895c9/net/unix/af_unix.c

Cc: hugobenichi@chromium.org
Any idea/update about a proper way to do what's suggested in #1.

Another example that came up in Android P is https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1198622
There is also the tcp control sysctl configuration in ConnectivityService that at the moment fail: https://cs.corp.google.com/android/frameworks/base/services/core/java/com/android/server/ConnectivityService.java?rcl=4af85a3e826cceb647216b755056972f49632ba3&l=1977 (these ones are not enforced by CTS/VTS).

It seems to me that in general all these sysctl are exposed to the init user namespace only, even though most of them are actually net ns aware and can be modified independently in the host and in the ARC container.

That can be verified when the sysctl is registered for a given struct net, the "data" field of the struct ctl_table is changed to point to a field inside the struct net passed in: https://github.com/torvalds/linux/blob/v4.13-rc6/net/unix/sysctl_net_unix.c#L41
Sorry, totally no update from me. Great if you could help me on this.

Yes, I agree that these values are, at the level of kernel data structures, easily per-ns configurable.



In b/64655659#comment5 I did a quick survey on rejected patches that aimed to make max_dgram_qlen more customizable,
and my impression was it's tough. The last paragraph of b/64655659#comment5 is the only idea came up in my mind then,
but that may only be applicable to this "max" type attribute.

Sign in to add a comment