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;
Message-ID: <32332bb4-1848-0280-9482-5189ab912b02@fb.com>
Date:   Thu, 4 Nov 2021 11:06:21 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.2.1
Subject: Re: [PATCH bpf-next v2 1/2] bpf: Add ifindex to bpf_sk_lookup
Content-Language: en-US
To:     Mark Pashmfouroush <markpash@cloudflare.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
        David Ahern <dsahern@kernel.org>
CC:     <kernel-team@cloudflare.com>, <netdev@vger.kernel.org>,
        <bpf@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20211104122304.962104-1-markpash@cloudflare.com>
 <20211104122304.962104-2-markpash@cloudflare.com>
From:   Yonghong Song <yhs@fb.com>
In-Reply-To: <20211104122304.962104-2-markpash@cloudflare.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SGF3L3FVY3c3OGZXbWgyaklsNC9zUGowSlFUZkpjOTBtNm9KbUNXVmRueTMz?=
 =?utf-8?B?ZUVXeTFvRFRtdVlsWGpqNHNYUUtJN2RzRUFxZnZhakg1N1dQeStBRWxkTTFH?=
 =?utf-8?B?NHZTL0NYYzNhNlNMYkVhUld3WWkrQkdRcTRidnNHcGl0SWt2V25WeDZ5Y1lR?=
 =?utf-8?B?V0U4NzJHeHc4QmhuUDRjNTJkZlFhUit5TndCTnVid0h6QVJGd2dhQWx5UURO?=
 =?utf-8?B?VkdWTFRCc1VWVkNOSXIySUgyb01QNkk5cFRSdWh3L1hySkE4azdQZkhFZkJO?=
 =?utf-8?B?c1NvUGZRV2FYOW41UUNFQXlYb2dHc01haFYwOWwrdWlZUXhkV2ljSmJIL0Vv?=
 =?utf-8?B?UGY5OGdNcmRzRDRyTjE2ZitZTXFUb1ZtcGxoVzlHQWtEREQ3cXZ4bHZ4ODdW?=
 =?utf-8?B?ZngzTTZBY2Z3UlBxeTRTTFNxRk1sRFRXYmVGSG1jUTZIMm5ENlVUVzZlSDRD?=
 =?utf-8?B?ZVpTcDFvTUZIbzd2OXVwcndqVS9hR0t0YXZ2Q21NUXQ2VFk1YmorUXV2REFP?=
 =?utf-8?B?Rm95YmhrNVF4L1BOcGdhRzlIRmtIVVdNV25OclA3eHZoNUxoYlJEaUpQaXNG?=
 =?utf-8?B?WmsrWGc0Zzl2c2tqK2psZWNUOFhJRFViMFlLSzJNRXdnRngrVFE1UzlPbG5t?=
 =?utf-8?B?akIvR0dVMWkwY1dWa2hqUDVuSFNJZERTbXpiL1cyRDhVNk1aSW1QbytBVWdO?=
 =?utf-8?B?a1lscUpWQ3JVS2FoU1VVbDhUbWVEdHIrZHhpNEltZ3R4MTFYSmY4Y0haa0dK?=
 =?utf-8?B?WjFXNW9CVHBxV0dMcG5vb2lvZTNLalFuaVRkY1NhRlhKelI0dTNiQkVoUmxI?=
 =?utf-8?B?eFV0NzN5cVlVL2JPdm1KL2htL2lLdGFqOXprb0lEbGJKOEozaGI4RHFwdk5I?=
 =?utf-8?B?cUpZOTdOT2JETVRsM3Jselowd0h5QWovVWdSVmdyWEFvN2hvZ29jb2pKZ3lu?=
 =?utf-8?B?Mm9YZXNJT2kwTkt5eXRSMlYxTG1VMCsrcjZsUHNsSDR6TU9Fbk0ySG85bExo?=
 =?utf-8?B?Z2krbkE2RFdwbExES0pQSVJhbWJRajZ6OE1RZXFwNnA1R0JsTkIyek43WEVz?=
 =?utf-8?B?OUxnaXo0RHRXYmxjR2kzc1ZudE1mOGY5SVgzdEJKN0srdGY1bWVHTmVkWTl0?=
 =?utf-8?B?OGRTdDlCeFkxU3VMWUxxclh6Nzd6dVVRdG1GVXJTR1Z2cjhrRFdBTkhFMXBy?=
 =?utf-8?B?QTdNMTBqcDlqUHlKM1lpenZPMmlXTmFCenlBalZPMUUxa3lpUEdyZDI0UjlW?=
 =?utf-8?B?Rk9FZ0w2YzN6eXBCY2lvMVhkdWh0V3M1TG5BRDM1bjg4dVBqckVVbmo4OC9U?=
 =?utf-8?B?SEZ3LzR6RlNvazh1Ylc2VHMwbWNMTE5HZ0FvWjZZUlVlQnBmQzRlWVpCY1hh?=
 =?utf-8?B?WGFxTWR2UEVxNXkrcHRqK3pHU3BXUWN0MlIrUXB0aWZYYzdwWi8rVHVVbXpO?=
 =?utf-8?B?bHVuT2dxSDY1c0VLNWlrUjVQbEdhVmxqU3NRc1pHQnc0UXJrODIrK2JMVTUv?=
 =?utf-8?B?d3Uxb0MxVXYzUmlVTDdpOGxBMXg3RHJCTFE3azY0SHpwR0hBbDhocjJRY3ps?=
 =?utf-8?B?Yi9zUjIwdzdWM1lrTTNYVi9pZUZvSGZ5QzJBK1BqMUt4aVBjamhnV1l6enpr?=
 =?utf-8?B?bWsyRDljOFlDL05vS3Q0K05DTmZUWU9MNlZOMFFLbGJ2VVdQT3QwaitZdHNM?=
 =?utf-8?B?ZW1zUkxZZmtPNmhpeG96aDVOTkkrK2podzFLRDRGRUxUeGE4M21kUXE0L0w4?=
 =?utf-8?B?aExnbUwzcUFGQnova0tSblAzaEtCb25OV3Eva2tlVXlaUDVmL0lhZTFvWkQz?=
 =?utf-8?B?Q3ZHZ0U4czgzK1d3OExveG9rcDZyeVZCYzBlQkhZekp4bDltaW52ZHJyeExG?=
 =?utf-8?B?ZlQ4UUxCYjJUeVlpVlZUWndMTVBvLytpa0xpY2p0K3RON1RtZ2hrRXY4dUIy?=
 =?utf-8?B?U1Q2aURDSWJuU1ZlMGVzTFRjRGVRMTBoM1ZXM0d0dGRCenkrRlcrM2h4MWlm?=
 =?utf-8?B?ano4ZHFQQ3VKc0FKYmVDNGtBeGlIUnltM0lhUkpicldMV3NOSkdJV2JNRVhz?=
 =?utf-8?B?RVFFMitGMk5aNUlybDBjUHFCRXFieGxmTE80WGJ5Q1AvZE1QK20xb0thVWE1?=
 =?utf-8?B?bWxwRWE5WGlsUmhxQkZzYllpdTJUcllKNkRaVjFMcnV0ZnN1SDErM1NPVCtx?=
 =?utf-8?B?OUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 9f2415f8-7786-4bd5-b7b6-08d99fbdd0a1
X-MS-Exchange-CrossTenant-AuthSource: SN6PR1501MB2064.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Nov 2021 18:06:24.8619
 (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: nNQ7uGe9haHDScPOujNF9wUAc5OMUyQwMsL5iI8BOVWgHrauz7134eDnHApJ0Ycr
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR15MB4903
X-OriginatorOrg: fb.com
X-Proofpoint-ORIG-GUID: RtJYm5smMX2vylpXnOq1byxmxH5kFIoC
X-Proofpoint-GUID: RtJYm5smMX2vylpXnOq1byxmxH5kFIoC
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.0.607.475
 definitions=2021-11-04_06,2021-11-03_01,2020-04-07_01
X-Proofpoint-Spam-Details: rule=fb_default_notspam policy=fb_default score=0 priorityscore=1501
 malwarescore=0 adultscore=0 mlxlogscore=999 clxscore=1011 mlxscore=0
 lowpriorityscore=0 suspectscore=0 spamscore=0 phishscore=0 bulkscore=0
 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2110150000 definitions=main-2111040070
X-FB-Internal: deliver
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 11/4/21 5:23 AM, Mark Pashmfouroush wrote:
> It may be helpful to have access to the ifindex during bpf socket
> lookup. An example may be to scope certain socket lookup logic to
> specific interfaces, i.e. an interface may be made exempt from custom
> lookup code.
> 
> Add the ifindex of the arriving connection to the bpf_sk_lookup API.
> 
> Signed-off-by: Mark Pashmfouroush <markpash@cloudflare.com>
> 
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 24b7ed2677af..0012a5176a32 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -1374,6 +1374,7 @@ struct bpf_sk_lookup_kern {
>   		const struct in6_addr *daddr;
>   	} v6;
>   	struct sock	*selected_sk;
> +	u32		ifindex;

In struct __sk_buff, we have two ifindex related fields:

         __u32 ingress_ifindex;
         __u32 ifindex;

Does newly-added ifindex corresponds to skb->ingress_ifindex or
skb->ifindex? From comments:
   > +	__u32 ifindex;		/* The arriving interface. Determined by inet_iif. */

looks like it corresponds to ingress? Should be use the name
ingress_ifindex to be consistent with __sk_buff?

>   	bool		no_reuseport;
>   };
>   
> @@ -1436,7 +1437,7 @@ extern struct static_key_false bpf_sk_lookup_enabled;
>   static inline bool bpf_sk_lookup_run_v4(struct net *net, int protocol,
>   					const __be32 saddr, const __be16 sport,
>   					const __be32 daddr, const u16 dport,
> -					struct sock **psk)
> +					const int ifindex, struct sock **psk)
>   {
>   	struct bpf_prog_array *run_array;
>   	struct sock *selected_sk = NULL;
> @@ -1452,6 +1453,7 @@ static inline bool bpf_sk_lookup_run_v4(struct net *net, int protocol,
>   			.v4.daddr	= daddr,
>   			.sport		= sport,
>   			.dport		= dport,
> +			.ifindex	= ifindex,
>   		};
>   		u32 act;
>   
> @@ -1474,7 +1476,7 @@ static inline bool bpf_sk_lookup_run_v6(struct net *net, int protocol,
>   					const __be16 sport,
>   					const struct in6_addr *daddr,
>   					const u16 dport,
> -					struct sock **psk)
> +					const int ifindex, struct sock **psk)
>   {
>   	struct bpf_prog_array *run_array;
>   	struct sock *selected_sk = NULL;
> @@ -1490,6 +1492,7 @@ static inline bool bpf_sk_lookup_run_v6(struct net *net, int protocol,
>   			.v6.daddr	= daddr,
>   			.sport		= sport,
>   			.dport		= dport,
> +			.ifindex	= ifindex,
>   		};
>   		u32 act;
>   
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index ba5af15e25f5..5b8618a4d485 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -6296,6 +6296,7 @@ struct bpf_sk_lookup {
>   	__u32 local_ip4;	/* Network byte order */
>   	__u32 local_ip6[4];	/* Network byte order */
>   	__u32 local_port;	/* Host byte order */
> +	__u32 ifindex;		/* The arriving interface. Determined by inet_iif. */
>   };
[...]