Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Song Liu <songliubraving@fb.com>
To:     Xu Liu <liuxu623@gmail.com>
CC:     Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>, Martin Lau <kafai@fb.com>,
        Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        "kpsingh@kernel.org" <kpsingh@kernel.org>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "bpf@vger.kernel.org" <bpf@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] bpf: Allow bpf_get_netns_cookie in BPF_PROG_TYPE_SOCK_OPS
Thread-Topic: [PATCH] bpf: Allow bpf_get_netns_cookie in
 BPF_PROG_TYPE_SOCK_OPS
Thread-Index: AQHXkoypynyJL9aEeUureyT0dFGeO6t2tYQA
Date:   Tue, 17 Aug 2021 16:18:11 +0000
Message-ID: <16DA85DF-84F7-40EB-B674-CDB07DEB4133@fb.com>
References: <20210816105114.34781-1-liuxu623@gmail.com>
In-Reply-To: <20210816105114.34781-1-liuxu623@gmail.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?kD1UghN+RBJUZwaJsDyo1hk0Qx6nl0ZVZXWYcAxShU441x3UBkhLy4Prrnfc?=
 =?us-ascii?Q?tMPyqUh2BFOivxJLP8oEdRpHnQX45tN4i1cDvD3Mq0RUP7h0h1G4ge3U93OK?=
 =?us-ascii?Q?xnEyjog0N9LDqiH1IVCZHl5c4pGZ+cxp+TEvul6PRRdcqWWxh/94FVD9YUam?=
 =?us-ascii?Q?x7C8YMPuEMVx8qHapXywuGIPCDWm6sc8D8DZaFo3D2p03YrgzDImVh14eypl?=
 =?us-ascii?Q?u/tW1VoDLtLEfcDJTA6MYuEXhX/LOPZXdX8vXMieFUjx+8h74gOxSU0EWiIA?=
 =?us-ascii?Q?ZBKOlWMSUg+HQU/NybK5lsTrznZkTYEuPW+yjT+8DJWw+uX8xkZC1D/RcZ+8?=
 =?us-ascii?Q?gRIfK0Whwb/800Fqaky5MXeR8gOUxQ9pZyu5e4HWba4corshnKoohj8+NRQZ?=
 =?us-ascii?Q?G0tF7lwRWoHnrdaPz6orbsaTM96zkj+PFjeeEhFs+0d98GV65sSKTtD/ZiqV?=
 =?us-ascii?Q?/TZesY7c1J89Mnmd/T1if/FI9UTy7nNodIuzC7fPcIgQ5L76JlF41dAqAimg?=
 =?us-ascii?Q?sajlCN7pe/JBcq7xofxvKIGMqZmfYI/2TqjERci3SvOFKDxLpfuFUyT6Uor8?=
 =?us-ascii?Q?jJHTXgmNlUETtymwcCMa7Uv+GOGPbK4xzwDRZKx5WB6F+iiKh1Zw8843EScN?=
 =?us-ascii?Q?ktAVO1+oYFHXvwY2EYWhWXn3K5+P+Ye+nKkmbf5DxXifrtxnU5+tGfUUl587?=
 =?us-ascii?Q?YCHIzwvkf7vwdfWlj8aGpPHJ0HNgDHSGaoYWZcVGC7bymM0N66d1Q5tvqjNx?=
 =?us-ascii?Q?HDpe1gbHPLB4xwy09ZpbLZIInnc7hKJci5ShGYxRSaMkeN5uZGVdHAZS4wDp?=
 =?us-ascii?Q?ls/k6aZeO5RMCLaldF9gjAzMgrWRRlgp1W8jDgIxkcp1BdeyyoN/VW0T9o4y?=
 =?us-ascii?Q?6HNZe/VYaB+mX2+2DfdYageZffjBVfEmn9rcdUbl+xp2/Tbu7rRd4gEEG39S?=
 =?us-ascii?Q?Sz7H+eopKq9nPrtcUBe3iFsxVPWVkTL/UmmoieJTqw3lf5fxCbTVBKKwQpj8?=
 =?us-ascii?Q?hCTXJo7J2RcG+whxRuDuH4KmMKHVRB9hJa6bbtsefdVbRGkRrL6aH7KZwton?=
 =?us-ascii?Q?d9N8RSHYFa0y45K7YpjxgkaDGtIuRKL3wyNGR9jDZ8BrtYL9YU61McrLKDrO?=
 =?us-ascii?Q?X3i9H/LCzFoOVyyOXt6q5hbpn1fZcJCQBLP90y0SK9wSVo0nC8wg5WZGRAf+?=
 =?us-ascii?Q?JdA5cyesXMNoo5KA7hiHBQ20gZMFoTjTyJLmKdLL8nbilOTISMP3Bg4J+Q+a?=
 =?us-ascii?Q?NNLb6nVNpVVys371PV2yuEfASUKrU8HMJJPTU76uOkdvJfZayw1d1iZe7jMI?=
 =?us-ascii?Q?PbH1khDbU6XBXjrV79nn4L1l+jmz5AEsYEPx0oX6I2cxNNKBUczEHYNB3wht?=
 =?us-ascii?Q?zH3pFEM=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <9AADF234DB1FEE4E84C6722165322ADD@namprd15.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SA1PR15MB5109.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0ca20c93-cd29-4901-7e11-08d9619a9bf0
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Aug 2021 16:18:11.7568
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: I5pM6O8xLPGjTXlA+PtopGaNIAtXBzdzxhi2dqq3H0ZbrL63lEQCxq0Bu+ERPoPEtkPzCvaeKCYxLxP3MPuYUg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR15MB5109
X-OriginatorOrg: fb.com
X-Proofpoint-ORIG-GUID: vlRJ_WgsA4RhztDcT-HAVTc6tvGmS3Wo
X-Proofpoint-GUID: vlRJ_WgsA4RhztDcT-HAVTc6tvGmS3Wo
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.790
 definitions=2021-08-17_05:2021-08-17,2021-08-17 signatures=0
X-Proofpoint-Spam-Details: rule=fb_default_notspam policy=fb_default score=0 priorityscore=1501
 malwarescore=0 clxscore=1011 bulkscore=0 lowpriorityscore=0
 mlxlogscore=999 mlxscore=0 impostorscore=0 phishscore=0 spamscore=0
 suspectscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx
 scancount=1 engine=8.12.0-2107140000 definitions=main-2108170101
X-FB-Internal: deliver
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Xu,=20

> On Aug 16, 2021, at 3:51 AM, Xu Liu <liuxu623@gmail.com> wrote:
>=20
> We'd like to be able to identify netns from sockops hooks
> to accelerate local process communication form different netns.
>=20
> Signed-off-by: Xu Liu <liuxu623@gmail.com>

The change looks good to me. Some logistics issue:

1. Please prefix the subject based on target tree, like [PATCH bpf] or
   [PATCH bpf-next]. This change should target bpf-next. Also, please=20
   include v2 (or v3, v4...) when sending the revisions of the patch.=20
   So the next version of this change should be [PATCH bpf-next v2] or=20
   similar.=20

2. Please add a selftest (see tools/testing/selftests/bpf/) to exercise=20
   this function from sock_ops

Thanks,
Song


> ---
> net/core/filter.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>=20
> diff --git a/net/core/filter.c b/net/core/filter.c
> index d70187ce851b..34938a537931 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4664,6 +4664,18 @@ static const struct bpf_func_proto bpf_get_netns_c=
ookie_sock_addr_proto =3D {
> 	.arg1_type	=3D ARG_PTR_TO_CTX_OR_NULL,
> };
>=20
> +BPF_CALL_1(bpf_get_netns_cookie_sock_ops, struct bpf_sock_ops_kern *, ct=
x)
> +{
> +	return __bpf_get_netns_cookie(ctx ? ctx->sk : NULL);
> +}
> +
> +static const struct bpf_func_proto bpf_get_netns_cookie_sock_ops_proto =
=3D {
> +	.func		=3D bpf_get_netns_cookie_sock_ops,
> +	.gpl_only	=3D false,
> +	.ret_type	=3D RET_INTEGER,
> +	.arg1_type	=3D ARG_PTR_TO_CTX_OR_NULL,
> +};
> +
> BPF_CALL_1(bpf_get_socket_uid, struct sk_buff *, skb)
> {
> 	struct sock *sk =3D sk_to_full_sk(skb->sk);
> @@ -7445,6 +7457,8 @@ sock_ops_func_proto(enum bpf_func_id func_id, const=
 struct bpf_prog *prog)
> 		return &bpf_sk_storage_get_proto;
> 	case BPF_FUNC_sk_storage_delete:
> 		return &bpf_sk_storage_delete_proto;
> +	case BPF_FUNC_get_netns_cookie:
> +		return &bpf_get_netns_cookie_sock_ops_proto;
> #ifdef CONFIG_INET
> 	case BPF_FUNC_load_hdr_opt:
> 		return &bpf_sock_ops_load_hdr_opt_proto;
> --=20
> 2.28.0
>=20