Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d53f0150-d74b-7cf6-8fe7-324131b43982@intel.com>
Date:   Mon, 22 May 2023 17:28:15 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH RESEND bpf-next 14/15] net, xdp: allow metadata > 32
Content-Language: en-US
To:     Jesper Dangaard Brouer <jbrouer@redhat.com>,
        Daniel Borkmann <daniel@iogearbox.net>
CC:     <brouer@redhat.com>, Larysa Zaremba <larysa.zaremba@intel.com>,
        <bpf@vger.kernel.org>, Stanislav Fomichev <sdf@google.com>,
        "Alexei Starovoitov" <ast@kernel.org>,
        Andrii Nakryiko <andrii@kernel.org>,
        "Jakub Kicinski" <kuba@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        "Song Liu" <song@kernel.org>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>, Jiri Olsa <jolsa@kernel.org>,
        Jesse Brandeburg <jesse.brandeburg@intel.com>,
        "Tony Nguyen" <anthony.l.nguyen@intel.com>,
        Anatoly Burakov <anatoly.burakov@intel.com>,
        Alexander Lobakin <alexandr.lobakin@intel.com>,
        Magnus Karlsson <magnus.karlsson@gmail.com>,
        Maryam Tahhan <mtahhan@redhat.com>,
        <xdp-hints@xdp-project.net>, <netdev@vger.kernel.org>,
        <intel-wired-lan@lists.osuosl.org>, <linux-kernel@vger.kernel.org>
References: <20230512152607.992209-1-larysa.zaremba@intel.com>
 <20230512152607.992209-15-larysa.zaremba@intel.com>
 <ee1ad4f2-34ab-4377-14d5-532cb0687180@redhat.com> <ZGJnFxzDTV2qE4zZ@lincoln>
 <b9a879b2-bb62-ba18-0bdd-5c126a1086a9@intel.com>
 <a37db72f-2e83-c838-7c81-8f01a5a0df32@redhat.com>
 <5b817d49-eefa-51c9-3b51-01f1dba17d42@intel.com>
 <fed6ef09-0f5b-8c3d-0484-bb0995d09282@redhat.com>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <fed6ef09-0f5b-8c3d-0484-bb0995d09282@redhat.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?c25LMW5ZRDlTcFZzOFlxYW9qZ1psNW5PUkVvdi8xZWlUS0Zyb0NZdHp0bnEx?=
 =?utf-8?B?ZXJCY0JBejBsY1ZPRXFjYnNWVUxXVmtFWFZ6eXZxcUtoMEZmYlk3SW1nd00z?=
 =?utf-8?B?RDEyMFVKZytmald4ODZUZm8xSmJCTTVBaklSLzlJY2RQWjBpQW90V2NIaUly?=
 =?utf-8?B?KzNpd1VZalMvUG9NSXQ4MlNYSzNKaVJyRW5yZ1lPbDdwY2h0WHl1RnozSno5?=
 =?utf-8?B?aVZ3b1RHQk1ZNGFVR1huQUNVUFBiQ3lXN0JFdWJRSWJCY2xJU0YxM2NKbXJv?=
 =?utf-8?B?OW40aUR1QWpyRmh4U0RKWGUwR0VneHJtNVpqeXo4a2U2bEVpOFYweUo5TmJs?=
 =?utf-8?B?S1p4ZGZWRVZGT0JtQXdlRW1lNmF0M2UzZk1IbzVoRVc5bGFkcityQmtoR29T?=
 =?utf-8?B?MHR5L0dXZDZVdHFVajN3Rkd4alJhSUo2WE9icW14N0FnR0Z5NnFvek13TjZO?=
 =?utf-8?B?UGZuM2U2VWRCV1VsZis3K2xMelVndTIwR2pQNkpkeDRDVlZVdFZpZ1Z0NnhP?=
 =?utf-8?B?dWhsZGRsNnVEMXgzQUkzQTQzUUxvb1drYjNPL203U24yU054R3FsM3hzOHQr?=
 =?utf-8?B?SkoxamtjTmNJVlJQRlBiYmxDeUZTQnpFMGl6azlpaXRkWjdZanVvQXBzV2lR?=
 =?utf-8?B?N3RTQjRsb1pPM3B5ZnhYSGUrcHBJZUE4aVQ1VmFXdEN3NGY2NTBVSmxwVDBC?=
 =?utf-8?B?ZzBwY3oycFF5Y0ZFTU16YVFCcWRSTGVBY1RpUDAzNDFNaFllNjB4dUgvWVRS?=
 =?utf-8?B?d0puOURiRitBQU9XY0FTV25kMVAwR0U4dWhoM2hWNE9ZRzZkdkx6eWVHREI5?=
 =?utf-8?B?bFBLM2pnbm1VM2M1VEh0eUF0MGQvazZ4Z1NBTWsrbkdqVlhwOExPQm1tZm5n?=
 =?utf-8?B?UXpOMmZhRUlYWHIvb0R4WFpBWVRUc1ZWQW95U0FjWUtRZU5HRkZGVUIrY2px?=
 =?utf-8?B?YjdsYU1CbDRVbTdIazlmd3ZGMjNHU0xzb1ZwTWhlaWdhNUZhWW8vKzFDTm5h?=
 =?utf-8?B?UUFPRXNNRHRrbUozQ0VQRTdySzkxMG9tSlcycEluQmcvbW1iNHM0VUk3Z1hL?=
 =?utf-8?B?dGZ1cmhYQmdJTWRhdWlxZ2dIem9nUG9Id1VYSytkcEphWUUzakdhV0JjdHZW?=
 =?utf-8?B?UnhOWjVEYTNQNTJHUUFIKy9oRXZ4RUhhU29ZOFBocVhZeHA0aGk5Z2tmT2w3?=
 =?utf-8?B?MFpMYUJ3UXNLYTN0UVBjR3d6STluQlhUMkxCNjFqa3B2eTk2aUlkZ3MxZUQr?=
 =?utf-8?B?THJtM01LQUV5NFNVWTUzajFQWjVvTmN6WDgvM29IUlRrMUk1TG5vZ2tIMlFm?=
 =?utf-8?B?WjYrbGFDSzYyT3JYT3hGY3RzcTVDTmRSZmVSOG5XbmtsZjFYTVZiMm03SllX?=
 =?utf-8?B?MzFRcUZFL0VLdFVPTFhJTVc3c3QwQ3Q0THFwY3hONGZLU2JBeU9HdGxCYUZO?=
 =?utf-8?B?UzNIbytDYzVpaVBoZVB3V3J2TjRmLzY3djRpUVR6cUZOOWhjYVNubnVMQVNR?=
 =?utf-8?B?aVpyUDloRkRqZ1ZHbEcxc0lOYmQyL3dOejR5YUZ4S2lseFNFKzMwYU93eUQw?=
 =?utf-8?B?UUJOQ0FWeUZMQWNxTXFHakc4b2VPTDB6UkFwWEwweXRFNmxYWCsrbGc5cXo5?=
 =?utf-8?B?RFBJbFgwS2NDUlJIVFZUaFI2VDU5MHZmWEw3aDBQd2IrTkNLUHBwSHp2ZWRp?=
 =?utf-8?B?NGlnOEo2UHdOUVhuc1BuOUtCQlBSR0VIUjFmRXk2elREQXFvVVlOLytuWFhH?=
 =?utf-8?B?WC80VENSS1VyT1p6NjV5czBHU2hJbmpSa1NvbjFseTczZFBYYXpXU2d5bjcr?=
 =?utf-8?B?dVVFdUZURDhGVDVNcWVSZkxZNEc0NGNlQkxvajJWdzRXeXNyL09xWFpuSUdD?=
 =?utf-8?B?ZitPZmZrQzFVM29EVzRjVWZwby9FTHF4R0ZWMmRCZkFMc2hPU2hKUlJzNjZN?=
 =?utf-8?B?dUNOajFWcDNYNWpNSDRVZ2JpRXVGL3UrUXlpNndLU1FoUGkvZWt1aVZBQWVH?=
 =?utf-8?B?ZTZlZ3lwT04wY1crYnBNS3E4Mys4Q1pSS2MvaHpkNmtRU0Y1TVFBZk5saUJP?=
 =?utf-8?B?dnVIR1htSGNUZXVsU05mOElZRnRQOG44bzlQajIyODJJd21HZWE4Q3B5Rksr?=
 =?utf-8?B?S1JPM2ZOODdyRFdUOWlwc25UVHRLbU5qaTB3dVloSEYwWVhPRW1HN29sTWY0?=
 =?utf-8?B?RkE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 20d2f506-f26a-4980-4cbc-08db5ad96155
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 May 2023 15:29:49.6054
 (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: 0pSQlWBWt0AWsMIJUYB+1qeJTxPBFc8Nhpy5LNtZBJgSm5XZF5stgU8LEiiOu6PI3THnj6m2BmGENMhLEZC6Ttiz60DNQKF7/nqee5YmO6s=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR11MB6413
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Jesper Dangaard Brouer <jbrouer@redhat.com>
Date: Mon, 22 May 2023 13:41:43 +0200

> 
> 
> On 19/05/2023 18.35, Alexander Lobakin wrote:
>> From: Jesper Dangaard Brouer <jbrouer@redhat.com>
>> Date: Tue, 16 May 2023 17:35:27 +0200

[...]

> Not talking about your changes (in this patch).
> 
> I'm realizing that SKBs using metadata area will have a performance hit
> due to accessing another cacheline (the meta_len in skb_shared_info).
> 
> IIRC Daniel complained about this performance hit (in the past), I guess
> this explains it.  IIRC Cilium changed to use percpu variables/datastore
> to workaround this.

Why should we compare metadata of skbs on GRO anyway? I was disabling it
the old hints series (conditionally, if driver asks), moreover...
...if metadata contains full checksum, GRO will be broken completely due
to this comparison (or any other frame-unique fields. VLAN tags and
hashes are okay).

> 
> 
>> The whole xdp_metalen_invalid() gets expanded into:
>>
>>     return (metalen % 4) || metalen > 255;
>>
>> at compile-time. All those typeof shenanigans are only to not open-code
>> meta_len's type/size/max.
>>
>>>
>>> But only use for SKBs that gets created from xdp with metadata, right?
>>>
> 
> Normal netstack processing actually access this skb_shinfo->meta_len in
> gro_list_prepare().  As the caller dev_gro_receive() later access other
> memory in skb_shared_info, then the GRO code path already takes this hit
> to begin with.

You access skb_shinfo() often even before running XDP program, for
example, when a frame is multi-buffer. Plus HW timestamps are also
there, and so on.

> 
> --Jesper
> 

Thanks,
Olek