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: <5b817d49-eefa-51c9-3b51-01f1dba17d42@intel.com>
Date:   Fri, 19 May 2023 18:35:44 +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>
CC:     Larysa Zaremba <larysa.zaremba@intel.com>, <brouer@redhat.com>,
        <bpf@vger.kernel.org>, Stanislav Fomichev <sdf@google.com>,
        "Alexei Starovoitov" <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        "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>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <a37db72f-2e83-c838-7c81-8f01a5a0df32@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?a2MwNzNyejBkdWxWdm5jOS9qeDlVSmJtd3hzUnlOTUNFaytoMkRjcmxTcmJC?=
 =?utf-8?B?OWI0QTIzOWxnaWlNZTdIN2NmUEZJN2NqbjhFYXhWazZFWDUxbHFNY2RPd0dj?=
 =?utf-8?B?MUNzWW94L2h6N21lVUJ2aFQvL2g4OXByVDFvMlYrNEFRRVpGOXhWL2NEaWli?=
 =?utf-8?B?QmltRk5CYnhOY2NxeUlUTDdkWFNlZ3BuQW5Ja2hxOGN5Yk5lODVVZG0xa2RD?=
 =?utf-8?B?WGMxcXhERWswT0lONUs4Sk43NkRPSUd2a1FVdTgyZVRHM0hhNXpEKzBzTWhQ?=
 =?utf-8?B?T1lRc3BKSFAxRVZQRTBpRnJUTVhhd3RIeWxGK3pZNlQ4T3ZqMGt6OUd5TXho?=
 =?utf-8?B?MkJsVi83WC80K2xOMVZXZUQ3eTZneXBQOGhZTWRlNVZubS9yM0RhMEQxZEE1?=
 =?utf-8?B?K1R3SUh2N0pjN0E0bVJYS0t2QjlvTlQ4cFUvMHdhMDBpc2Nmbk9EeFptT0tX?=
 =?utf-8?B?Q0J0NFRpdXJnZklwT2I1UGUyTmtkc3V1dDdhTjhmdHdUUy9pamN1eGNkVnlu?=
 =?utf-8?B?OXZPQ1FaRXpDSmpFK3FxUGJTNGtLTEk1OVdoeTRXRG10bWc4Q0NKbTk1cGwv?=
 =?utf-8?B?RG1EcVQwektBVC9zamdMcmhGS0ZORGo0RlM4MjFZR1ZLRTA2YXdFWElRZG9y?=
 =?utf-8?B?TkRiNk1WWWVXZVl2UXpRYm81OHh1ZGszVGM2L3F6VHY4Ym5CVUhVL2kyaFcy?=
 =?utf-8?B?cWIxOUVwTEpsa1VnZjgybUFUZzlKU0l3SnRQK2pxTWZqOFVCMEZsazNGekhz?=
 =?utf-8?B?a3R6SmJjdHIvWXVxbkNiSm5VSFFDZTl4dWdsV3dDTklCdmMwd2x3dk8vcUdL?=
 =?utf-8?B?SDQzTDJZWnQ4cGRxK1lBbkFEaGRFQ0ZGeDlURHI3Mkh1RGQ2U3RQcG9wY29O?=
 =?utf-8?B?QmNKdlJIR1g3R2dFdUxGc01qenBFRDRhaHp5SVZQS3l2dzFRdmlubkVqWS9P?=
 =?utf-8?B?enVES2FYcWdzSHJCQjdRbkcxVUxOYW11NGJVcVF4ZCtScTd1NUFLbThtS0VI?=
 =?utf-8?B?TVhZb08rREN2RHpScld2YkZYLytJc3JxOGVOWHIrOEphc3Qzc1k0K2R2cjFJ?=
 =?utf-8?B?eXVNa2ZoQS9TM3hJaHVoUkZiSU1GdFoyS21HYU02RjFGcWF5OUZXRDJMOEIy?=
 =?utf-8?B?RWVWUXFReXQrdFUyTDJSRkQ4ZXNYdFp1aDhObjJTK29yby8zdms0ODRGMENi?=
 =?utf-8?B?dHVPMVlSV2RvUkhNbFdtaTJLd0pKUE1pQ0RUb2M4bEJTUXFDZWQvVzR5UTJ1?=
 =?utf-8?B?dGJwS2pmbkdNWDJIejl1TE5KUXM5UUs0VVdwalNQU3Q5bDQ3R3ZjdSt5TnJj?=
 =?utf-8?B?dmtNZnZiTlR4c3FNbGRQYUdNdXN5QnB5VnpUYktmTHhCRVVUdW44ZkV1VG9p?=
 =?utf-8?B?MXRNdVN2WE9nRnhQZ21iZjQvMk1vVXBadlpiVy8yTm55ajJQNUE1eDZvdzRY?=
 =?utf-8?B?enZmYUxiTVN5T3ZFVzRWd2F3MlNEc1dtL2RVV0I1OCtvU0hPdVUxRzR6MEp3?=
 =?utf-8?B?UGNlcUIvMkxqOEJDWE5CMUlNcDUzWllKUXR3Ums4TFVaNW5KN01ZYU9UNlRE?=
 =?utf-8?B?eGRGNWdEU28vU2IzTGRERXVZV21SckJWN1JYTXgvcGpaNzlIS2JOOHNIU1Jl?=
 =?utf-8?B?K0JJS2NUclhQZmZ0K2QvaE9uTHUwUUtvU3F6YXYrSHluV3VEWUFTUWRuTTFr?=
 =?utf-8?B?VllMaFphSlU4cE1Lak1pTGNGOEd0bGhIV1BmM3Q0RVNSUlVUTlJ0VWhyMjZN?=
 =?utf-8?B?L3lTTHBCSHdGQkRNSlBKeDNjMXFhamFWeE00SFptVTlxdVBocGViNS9kMUVq?=
 =?utf-8?B?UCtHL0VVdWxhcFhWdE1WWE8xRjVNSDVETG5DZ0RMbnpOYWFNSTViMnM5K2xI?=
 =?utf-8?B?K1VYVXAvbDRBcHo4NGF4N1Bmc0I4NVVIeWxMYXdlQnRSL3hOQWZwMVdJa244?=
 =?utf-8?B?dlpUUVp0eDdqVmpWTlQ4LzVCanlMakRoS1pPenhVSkNWZGFsVXZKd3VqbzBL?=
 =?utf-8?B?NnZ6VkowWFFpbDRtRU4waWQ5V2ZLM3ZFaU4vMnBrOTE0RG1GOWtSeW02TFBF?=
 =?utf-8?B?SWs0cVNhRWRpbnl6dFNGNGlXUC9CbXhzckNxaWlqbGdjNTJTUVhzTnBBMzcx?=
 =?utf-8?B?YWh2aXZWNTBibEZRMFpCVW9VN0s1T0d6QTJoYjJwMnRPL2xtWWk4T1Z2U3hB?=
 =?utf-8?B?N3c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 55b8c19c-cd85-4ffb-6b86-08db588741f4
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 May 2023 16:36:55.8372
 (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: MHNCpTHq3icYlnW9IxzWlOaXOQpm8744qj7BFK4KIYUwOazOwvBVBtVuhqemPG3fuxy3Vc5KX2It6vNQxnPuYjRrvXX9mL/KYH1/lDm8738=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR11MB7981
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.9 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,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED 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: Tue, 16 May 2023 17:35:27 +0200

> 
> 
> On 16/05/2023 14.37, Alexander Lobakin wrote:
>> From: Larysa Zaremba<larysa.zaremba@intel.com>
>> Date: Mon, 15 May 2023 19:08:39 +0200
>>
>>> On Mon, May 15, 2023 at 06:17:02PM +0200, Jesper Dangaard Brouer wrote:
>>>>
>>>> On 12/05/2023 17.26, Larysa Zaremba wrote:
>>>>> From: Aleksander Lobakin<aleksander.lobakin@intel.com>
>>>>>
>>>>> When using XDP hints, metadata sometimes has to be much bigger
>>>>> than 32 bytes. Relax the restriction, allow metadata larger than 32
>>>>> bytes
>>>>> and make __skb_metadata_differs() work with bigger lengths.
>>>>>
>>>>> Now size of metadata is only limited by the fact it is stored as u8
>>>>> in skb_shared_info, so maximum possible value is 255.
>>>>
>>>> I'm confused, IIRC the metadata area isn't stored "in skb_shared_info".
>>>> The maximum possible size is limited by the XDP headroom, which is also
>>>> shared/limited with/by xdp_frame.  I must be reading the sentence
>>>> wrong,
>>>> somehow.
>>
>> skb_shared_info::meta_size  is u8. Since metadata gets carried from
>> xdp_buff to skb, this check is needed (it's compile-time constant
>> anyway).
>> Check for headroom is done separately already (two sentences below).
>>
> 
> Damn, argh, for SKBs the "meta_len" is stored in skb_shared_info, which
> is located on another cacheline.
> That is a sure way to KILL performance! :-(

Have you read the code? I use type_max(typeof_member(shinfo, meta_len)),
what performance are you talking about?

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?
> 
> 
> 
>>> It's not 'metadata is stored as u8', it's 'metadata size is stored as
>>> u8' :)
>>> Maybe I should rephrase it better in v2.
> 
> Yes, a rephrase will be good.
> 
> --Jesper
> 
> 
> 
> static inline u8 skb_metadata_len(const struct sk_buff *skb)
> {
>     return skb_shinfo(skb)->meta_len;
> }
> 

Thanks,
Olek