Message-ID: <e62296a6-7016-c98a-8419-69428f65d9cc@intel.com>
Date:   Tue, 14 Feb 2023 17:04:30 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH v2 bpf] bpf, test_run: fix &xdp_frame misplacement for
 LIVE_FRAMES
Content-Language: en-US
From:   Alexander Lobakin <alexandr.lobakin@intel.com>
To:     =?UTF-8?Q?Toke_H=c3=b8iland-J=c3=b8rgensen?= <toke@redhat.com>,
        "Daniel Borkmann" <daniel@iogearbox.net>
CC:     Alexei Starovoitov <ast@kernel.org>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>,
        "Jesper Dangaard Brouer" <hawk@kernel.org>,
        Jakub Kicinski <kuba@kernel.org>, <bpf@vger.kernel.org>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20230213142747.3225479-1-alexandr.lobakin@intel.com>
 <8fffeae7-06a7-158e-e494-c17f4fdc689f@iogearbox.net>
 <6823f918-7b6c-7349-abb7-7bfb5c7600c2@intel.com>
In-Reply-To: <6823f918-7b6c-7349-abb7-7bfb5c7600c2@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bHN6UENhZlVKbDZGNmtUNkN6QnN0SUk0ZHJkUmVNdTlzRzJ3T0Q1SW9XejBB?=
 =?utf-8?B?cTVkWFN4bnhVNjYxYzNaQXBTMVRnSktSaGZHTmRUbi9PYVZUKytBSi8rZndP?=
 =?utf-8?B?azRsSzRUS01NeEVoenhoZTZSQ3BsMzBhWlpVNUdaOUh2YlBpYm0xdXNIM0Mw?=
 =?utf-8?B?ZzdtelJZVXcrcFlBN002c1BDT3NTMTdEQndiM1dMTmp1WU0zU3d4Sm0xK2cw?=
 =?utf-8?B?bWwvQTltMVVZeFdweGtpM1p0MkRpY1BrY1FIVGZtNC81SjYrNGdQaG9jd241?=
 =?utf-8?B?RFV3cGdUN0V1cWN5Tlc5ZEF1SFZtQ0VkRWpBY29XYmtKeTlOWXlYbEZlZU0z?=
 =?utf-8?B?alpmektwZCtIS0xoQVJSYVZ6MnN2WDkrU2hDSlNkbktVdXFYdmZWSDRGam1X?=
 =?utf-8?B?clVSMEc1WTBFTE1LTEhub3NMZ2p0RVlEMDJRblg4WE5ac0M1NlBlT0UzZWJj?=
 =?utf-8?B?THRtOWpXOWVYckRTM2NTaGxWUncxQ0EzZDQwTERUVG9PZ3ZlWWt6ZGtMdjAy?=
 =?utf-8?B?aVBQSkhmdFE5SnNDcHBzdjRSSE1NMHVScjIrRDdhQlBlczZuUTZUWEVBY1VK?=
 =?utf-8?B?bjd3amlXT2l4T1A5eUxGQzNBeHFMd2xQcU96b1lBc0MzeDUwbHBHQ1RyWElm?=
 =?utf-8?B?aVlUMlFFdDZUWUc1NEFlZ0lUSlI3Rmx3YVpHN3VwY3cva1RSWFkwU0xHSHBs?=
 =?utf-8?B?WlJ4cU1YaTVLUnM0N3YwNUJxcDZFM0JidE5qaW9HM2kzS3lSeVV4SVovT3Jo?=
 =?utf-8?B?WkprM1RPdWpaR0dQb3VudTBYV3ZjZUR6UFpRUlNPWEhPUEtuMzZHQ1oxSjNa?=
 =?utf-8?B?Nlk1dVFOQjVqMk5uOWtoc2swdExzVVE2dGdYUE1wMVUzanN6cVBJL25mMlFj?=
 =?utf-8?B?UW8xM3pGS3MvM2xXT3B5cVpzVVJJSmJjSGRvV0phMEJPS3RuS25YalVWVWZB?=
 =?utf-8?B?WjQyZ2VhL1JQSjdWRHZUOHJTRlExWlpSKy96ZTVZdU9zMmNxOHRYY2tYbW1T?=
 =?utf-8?B?cjF3RjBLUllZRUN1NWFzNU1LQ3k1d2pud3lob2U5Ujh3MXd2TzFUMzl0N2x1?=
 =?utf-8?B?RmlnekRCZ01yQ0l4Vm92aGRuTmoveEN0Z2JVSE1KN3hlTGdkLysrTGVqTkZF?=
 =?utf-8?B?b1NFeEFUclhqWU5HRjFGalBmM3FBVEVkc0pIbkIwWG5kcElhMjA3RG5EOHow?=
 =?utf-8?B?YXVCbXBqVm16NkFJbjFtOHRkY2IwMnEvcTJMSnRNTDcyc3ArR251M05kNlpN?=
 =?utf-8?B?enlNK0NGZXpGbUpTSXppd1lDRExIN29vOWdsZDJiaWZSOVJkbUJPN1JqNVU5?=
 =?utf-8?B?Ykc3ZkI2dStmTitVdGhEWE1zcExyYWY4a3BDVzdXQ3FmMEJzRE1oRmRVdnd5?=
 =?utf-8?B?Qkpzb0UyTENadCtob29wd211MFVHbTVLZlB6U3pob04wZnlqWmJqb2t1anIy?=
 =?utf-8?B?eGw4U3kwSGMxR2dadjJLcHRHcE9xQVo3bXNWK0xUeGFBeVRiNXRwTzIydlJE?=
 =?utf-8?B?dm1zbjlBMGYrMjlhL0F5b1gvVFJVcU5OZkExL3Zmam1jdXh2TWtxb2hISk5v?=
 =?utf-8?B?SDEyQU5VM3lkQmFQeittV0cvaFp1OTdIcFRYbW54QlkzVDMwcnlaNSt5NkRM?=
 =?utf-8?B?Vk9ZTU1pWkdkckUzWUMwTGpTVlRES0gwYVJUVmtyMlR4VGhKVlNHTGVRcEJT?=
 =?utf-8?B?Y2QrR3U5WGdHd0pmZjJCUnYrc0FscUhqbmttdkJ2NWlYc2QvM1IzRVNDTFd3?=
 =?utf-8?B?UGFKTS9VNXZhK0Z3ZEtVbDBiZnFKY3dFL2UydnRSejFkSjhoTFErQXp0R0dt?=
 =?utf-8?B?SlVlaWNzcFFFMUdvYkhTZ0p6eks3NGhzMDJ2UUd5aVpRSE9jWWh6SFZYNGpX?=
 =?utf-8?B?TEhzakQrQXlBaFZkZjRaR1lwK3d0c2tZMmRwM0c4dk1ud1BneE40bjVSWVBB?=
 =?utf-8?B?QThpVW1iS0ptV0NVTEk5S3h3NnF5RWpvaVVlcyszY011L09ObEtSc3dSWVk2?=
 =?utf-8?B?K3AvTnVtL1lvTnlyR2xOaWprZWJ3Z05tbWFkU0xTVEdtbXEzVGxrMlNBNFds?=
 =?utf-8?B?enRnQ1hDL2YvUmVUOHlpNFlPbnorbTFLSXdBbGF6cG1EYnYwZGNld0JVS1Vs?=
 =?utf-8?B?WmJTRVF3blF1V09hUllmdGNvWjlPRUJpR29jb0pBdFEwWEMyU0p2eVJlK2c4?=
 =?utf-8?B?WEE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 07cd7d0c-a92e-443a-25fc-08db0ea552c3
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Feb 2023 16:05:42.6626
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: zEIE74CwmYG5YzUiZ/8yK9a7IBtxNJhYs5LJb0YiqS2RE1qKRry5vtzYq0ERH0LzQmWr/E95MKu3CU5T/PZMszf+Zj73ddOxAK05LMaHbAw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB5960
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Alexander Lobakin <alexandr.lobakin@intel.com>
Date: Tue, 14 Feb 2023 16:39:25 +0100

> From: Daniel Borkmann <daniel@iogearbox.net>
> Date: Tue, 14 Feb 2023 16:24:10 +0100
> 
>> On 2/13/23 3:27 PM, Alexander Lobakin wrote:

[...]

>>> Fixes: b530e9e1063e ("bpf: Add "live packet" mode for XDP in
>>> BPF_PROG_RUN")
>>> Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
>>
>> Could you double check BPF CI? Looks like a number of XDP related tests
>> are failing on your patch which I'm not seeing on other patches where runs
>> are green, for example test_progs on several archs report the below:
>>
>> https://github.com/kernel-patches/bpf/actions/runs/4164593416/jobs/7207290499
>>
>>   [...]
>>   test_xdp_do_redirect:PASS:prog_run 0 nsec
>>   test_xdp_do_redirect:PASS:pkt_count_xdp 0 nsec
>>   test_xdp_do_redirect:PASS:pkt_count_zero 0 nsec
>>   test_xdp_do_redirect:PASS:pkt_count_tc 0 nsec
>>   test_max_pkt_size:PASS:prog_run_max_size 0 nsec
>>   test_max_pkt_size:FAIL:prog_run_too_big unexpected prog_run_too_big:
>> actual -28 != expected -22
>>   close_netns:PASS:setns 0 nsec
>>   #275     xdp_do_redirect:FAIL
>>   Summary: 273/1581 PASSED, 21 SKIPPED, 2 FAILED
> Ah I see. xdp_do_redirect.c test defines:
> 
> /* The maximum permissible size is: PAGE_SIZE -
>  * sizeof(struct xdp_page_head) - sizeof(struct skb_shared_info) -
>  * XDP_PACKET_HEADROOM = 3368 bytes
>  */
> #define MAX_PKT_SIZE 3368
> 
> This needs to be updated as it now became bigger. The test checks that
> this size passes and size + 1 fails, but now it doesn't.
> Will send v3 in a couple minutes.

Problem :s

This 3368/3408 assumes %L1_CACHE_BYTES is 64 and we're running on a
64-bit arch. For 32 bits the value will be bigger, also for cachelines
bigger than 64 it will be smaller (skb_shared_info has to be aligned).
Given that selftests are generic / arch-independent, how to approach
this? I added a static_assert() to test_run.c to make sure this value
is in sync to not run into the same problem in future, but then realized
it will fail on a number of architectures.

My first thought was to hardcode the worst-case value (64 bit, cacheline
is 128) in test_run.c for every architecture, but there might be more
elegant ways.

> 
> Thanks,
> Olek
Thanks,
Olek