Message-ID: <701f6030-72d7-0f11-173b-a2365774b6f2@intel.com>
Date:   Fri, 10 Feb 2023 13:31:28 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH bpf] bpf, test_run: fix &xdp_frame misplacement for
 LIVE_FRAMES
Content-Language: en-US
To:     =?UTF-8?Q?Toke_H=c3=b8iland-J=c3=b8rgensen?= <toke@redhat.com>
CC:     Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        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: <20230209172827.874728-1-alexandr.lobakin@intel.com>
 <6db57eb4-02c2-9443-b9eb-21c499142c98@intel.com> <87sffe7e00.fsf@toke.dk>
From:   Alexander Lobakin <alexandr.lobakin@intel.com>
In-Reply-To: <87sffe7e00.fsf@toke.dk>
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?T0F5dzJPeGpack9pdFhXS2NieFpjMUdZK3pRZ0FNTVJGSk5MclVWMkJaNmxl?=
 =?utf-8?B?ZVdnTStNeENVVGpObU5scmEyZDJxQldBN0FIY2hkay9SUHd3R1ZyR09hRElY?=
 =?utf-8?B?NzUwQnJvUWFMdkhNVDlqbVlkQSt1MVJRTUxsUkFOWEF6bWpmK0FJQ04yRnpI?=
 =?utf-8?B?ZjV4UytvTlZsQTlpT3p6U3RYbFNNQ0RCZHIzdmJ3RTVJcGdKZUhHRUlEVXA1?=
 =?utf-8?B?R1owbGhxTHBwMm1MMjZPdkdIS3dxNk1rQ25LTWdrd1RGYmVxVllTb2JyK3lr?=
 =?utf-8?B?Qy9vMHVPSkZ2TlI0TjNoSlJlTzZvOWVDcE4zTFVnUWlsQXY0S2JNbHVRNWhp?=
 =?utf-8?B?eksvbnkvL0t4M1hNVUUyenV4VFpDeE9CZVp6MExxbnZvcGxJYkZjY3h6YUJa?=
 =?utf-8?B?QkxNWUo3ZjN6Ujkyc1N3M3JZOGpnczF4WGFNNnkyeWszYW53TjZRVFEzdEhO?=
 =?utf-8?B?YmYyQW5od09rWXZNWWFyeW9EUGlKV2dMdjI4NHN2TkdiQUZqNmRKaUtOTWkr?=
 =?utf-8?B?NFNDTWlId2dHMWVidWp5Z0JHL1p1NkRiRDEvM2RpL2ZRN0lBRXBBamNZb0Qv?=
 =?utf-8?B?eklmQ1B5dkZkWWcxaldDeDFVK3JxUENUb05yM2RWMXRHamJ3ZXpkeW9nVjd3?=
 =?utf-8?B?NEdsdUJ1SEV1WnNhNXlFQUVVRy9tdUtrUzU5ZllFQWQwVlpDMVcrelhDdjRK?=
 =?utf-8?B?cCtrTlBIU2htQTVqVkM1aUcrN3lGbms4bUw5d2YvcmdEcWFBM3U4U3hNcWRi?=
 =?utf-8?B?cFRsSXM3Tm1tVng1TGswbWZVUWd5MHBza0xsYjdxWi9GTmFKbVlEUksvclhw?=
 =?utf-8?B?NDREaXBHTGtBUzU5YkNlM05pWG1Rd1cyajA4YnVSVW5XQUZSUHpGT2lzNVMz?=
 =?utf-8?B?K0liZVN4eDlDSndMQXJXbkhKSWY1RVNNMGJPMnFnMDF3VHN6WkpmWUNTeGZ2?=
 =?utf-8?B?dXRsaVJXdGJhNUczNkxscng2aFhDZWRWbkRHY291VVJlMHVCRldoanYwVCtX?=
 =?utf-8?B?MDRhQi8xandrcG50emFqNDdjVlN0NkFOK1VBMlhndm5URUNDTFNBSDZiSlUw?=
 =?utf-8?B?U2pCVTVrbVIwcm52MkxOTlRIUFRlck9XT3RGeEZBdm1CUFlvbFhjSEdjVTR6?=
 =?utf-8?B?N0lmeHdWUGJEN0M4MFhhKzlDSjVVYmw5RGY3YUxRNlY1dkxoTnEwY3pYOXJ0?=
 =?utf-8?B?WmJWS2VwWGVnaG9KbEpSdnR4UUdXbmhKSUNmODF0RlFTcDFlUllyZ1QvSXkv?=
 =?utf-8?B?QTlsMzRHa0QzemhuSVQvZ1pCcjY4d1VOS2p5SnM4YkJyNlBuZ1orcFU1aG44?=
 =?utf-8?B?Z2ZSMGtFblN2QTZ4WlBzWnVtSlhOclRtVzIyY0NvUWxZbzZYVmszRS9DSmd0?=
 =?utf-8?B?T0FxWlJUVEZLT29UZ1JhVFd0UGVRUDN6K2YzL1Njak1oUitmcXdUbjQwell3?=
 =?utf-8?B?SEpiMzE0VklxR3E3eUJDditMQmYyRFVlL0JHTDBVTDVxZHVyVDRYODhyVSsy?=
 =?utf-8?B?RDROMUM3SFo5akdxL2ljcW5DZ3ZWb0JEWTVpV2s5ZUZ0SVkxTm4zZmtjSmhW?=
 =?utf-8?B?dGMwbTBVUllmVzVtTTVWdy9RS0d2YjNkczVxQ1A1SnBvWEZvRTdxYUpyRHFC?=
 =?utf-8?B?dWRsVFZOY2Y4VVgzQWZCU0ZNcXEzM210QkZqV3FlZE15ODdaSCsrRTJxSjA0?=
 =?utf-8?B?N1pIc0NzOVBaQkVHT1JtUjlPNWV0c0tvQk1CZVNPbEFYb2lFemIvZ1ZBKzVC?=
 =?utf-8?B?alBKR25FSXdGa0t4MUpTMk44bHB5N2U3VVFWOXBIUUZ6OEx2ZWI1dEtxYVMx?=
 =?utf-8?B?VHFwcVNaaXpGMHFKSEhGS2tQeDRrNzJDL3NVK29rOGlEeGR0R25COFozRDhP?=
 =?utf-8?B?eU45bVNsQ1BSZ1lYUHBvTUlrYnhMWCs5RlE3d29iTDA2UEF1OVRtZ1VSQzBD?=
 =?utf-8?B?dXQwSW1DRlY4SldqZTIzOVJpU1IzMEVBeXJEay8xaUsyV3dkUzlpRG9ER1Iz?=
 =?utf-8?B?emhWdFdvL0p5dEJDYmNJYmFLNkRrVDREeWJXbEM3KzFSL3djNGNrczdMbWR3?=
 =?utf-8?B?RUw1SlMvanZFQnVjUVNuWFlTOHJ4Um5nZnBEUit1a1JKOFkzUks5VUt2aDho?=
 =?utf-8?B?Wk9Kc2V1cUpNNXMyanJ4TzVKOXRrbEFQd3MydU5DRWN1MVdsSWt0ZHUxM3Bm?=
 =?utf-8?B?L3c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 66d37289-e4f4-4946-ac5d-08db0b62d2d3
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3624.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Feb 2023 12:32:07.8126
 (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: xnh6A06PwYnxZdczSzD3HSM1CN8djCSJPHcKXMKIq8AiIrtLhLCcy0EtnnvSoy6n4ClZZ6dzOFDxJZ+EUtaLFm4myWP8/w/pVFJ7Go0SPqc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB6803
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Toke Høiland-Jørgensen <toke@redhat.com>
Date: Thu, 09 Feb 2023 21:58:07 +0100

> Alexander Lobakin <alexandr.lobakin@intel.com> writes:
> 
>> From: Alexander Lobakin <alexandr.lobakin@intel.com>
>> Date: Thu, 9 Feb 2023 18:28:27 +0100
>>
>>> &xdp_buff and &xdp_frame are bound in a way that
>>>
>>> xdp_buff->data_hard_start == xdp_frame
>>
>> [...]
>>
>>> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
>>> index 2723623429ac..c3cce7a8d47d 100644
>>> --- a/net/bpf/test_run.c
>>> +++ b/net/bpf/test_run.c
>>> @@ -97,8 +97,11 @@ static bool bpf_test_timer_continue(struct bpf_test_timer *t, int iterations,
>>>  struct xdp_page_head {
>>>  	struct xdp_buff orig_ctx;
>>>  	struct xdp_buff ctx;
>>> -	struct xdp_frame frm;
>>> -	u8 data[];
>>> +	union {
>>> +		/* ::data_hard_start starts here */
>>> +		DECLARE_FLEX_ARRAY(struct xdp_frame, frm);
>>> +		DECLARE_FLEX_ARRAY(u8, data);
>>> +	};
>>
>> BTW, xdp_frame here starts at 112 byte offset, i.e. in 16 bytes a
>> cacheline boundary is hit, so xdp_frame gets sliced into halves: 16
>> bytes in CL1 + 24 bytes in CL2. Maybe we'd better align this union to
>> %NET_SKB_PAD / %SMP_CACHE_BYTES / ... to avoid this?
> 
> Hmm, IIRC my reasoning was that both those cache lines will be touched
> by the code in xdp_test_run_batch(), so it wouldn't matter? But if
> there's a performance benefit I don't mind adding an explicit alignment
> annotation, certainly!

Let me retest both ways and will see. I saw some huge CPU loads on
reading xdpf in ice_xdp_xmit(), so that was my first thought.

> 
>> (but in bpf-next probably)
> 
> Yeah...
> 
> -Toke
>

Thanks,
Olek