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: <b9a879b2-bb62-ba18-0bdd-5c126a1086a9@intel.com>
Date:   Tue, 16 May 2023 14:37:13 +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
To:     Larysa Zaremba <larysa.zaremba@intel.com>
CC:     Jesper Dangaard Brouer <jbrouer@redhat.com>, <bpf@vger.kernel.org>,
        <brouer@redhat.com>, 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>
Content-Language: en-US
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <ZGJnFxzDTV2qE4zZ@lincoln>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WVVuK2VXOVBTdnlDMHRaa0g1L2kvUnF6RExxUnpZZGlqeU56VmFFSUhyd0d2?=
 =?utf-8?B?QWRPZmFTQ2RyUk1Iby9KaUlqQU1JV3M4N3IwY1dlditVN3FYVSszelBheGpB?=
 =?utf-8?B?d0kzRThQVVdJR0hWTmNQaUduRUNFRDdiUlppL0FtUU9KU1loZ3BYUytiY3lw?=
 =?utf-8?B?QmZHR1ljZUpBdUdtQk9GTmxLRnN4S3lnK0syc01lZGpmczNxbGY0VDV2Zzdr?=
 =?utf-8?B?WU1jUHhJWjlOSU96VU8xeDBOZzUvTTYwcmtHV3ZqeFRpL2JIQjljUmpSNlNW?=
 =?utf-8?B?djA0MjRzU1FyNVNiUkhtdHIrV3Q1dHpkZWUwbCtvdmh3Y21hNVhiZm14enZZ?=
 =?utf-8?B?UDMvajRMR1RGVXRITlhWT0lySmU2dWZtV2s2blB3aUluNU4rcmFYY3Z3c1U2?=
 =?utf-8?B?OWh6a29JcGV2Z1Z0ZVlRazJKdGRTVDc1VkRVNGxWVEh5eEZJc0lMTlhmMCts?=
 =?utf-8?B?Y3FUOStOZHdoeCtNdFhlbFpGWm9kVURrN1NnZEEveGhLcGtlNzBTZWtRL2k3?=
 =?utf-8?B?TTFjdTN1SHJDTEVaZ3h4V0NUWDRxR21MSGQ1RDhGVzRuSTYvUk1zS3FYOWVQ?=
 =?utf-8?B?amJXSXc3YnRXWm11a1JtaVZZTFJZdWNNTlpRL1BqZ0huU3Y0cXJKUTlPWnZn?=
 =?utf-8?B?QnRXZHVtZFRueEFmZG5BZE1ocHZtWmtibTRaYkVqbmZhb3NIK094U3RyOERS?=
 =?utf-8?B?b2tIWmhsQk81Ui84d0M1bWkyeXhER0ZHR3JvMXlDUlMwdmI0QkhQWjFGbXh5?=
 =?utf-8?B?VUdUelU2bnBySWxoZ3lYTnhaMWNxNTBtODlLUGRtb3BxcVdPUXNseWR1Kzdl?=
 =?utf-8?B?dVpKM3hCQ2VYdEJlRDVDYng2bnc3cGViOCtlMHBvbS90U2h2dE0rK3RRa1NO?=
 =?utf-8?B?dGRZZUw3dFhPM3QxNURNcTZHSWl2SXQvR09zZ2lMaHQ5R09mSjBWdlVNUWJ4?=
 =?utf-8?B?MG1PUVMyNmRqVnkwbzVMdHZjOUlZUlBCMUthQnFjZzdCeTVEYk9BM2ppVzlH?=
 =?utf-8?B?YkFLb2w2czZTVnQvdUwyTmdGMXByQmY0MTVHWkRqdXBiSVNzSkJtc0Qxc3cv?=
 =?utf-8?B?VkQzUDFMNVI0Y09PU1FpRmlpZzcvMjV5NTF0MFNBRk5wRGVjUzQ1THR6ZEZa?=
 =?utf-8?B?TmdKYzNhTWJxZ0UvalhvQVhDdXVRYitlMTV2SU93dTE0QW03aWpBZU1haHp5?=
 =?utf-8?B?Z3FwZHZxNlFTWVY5a2VocjJ1ZjEvMmJ3aDZLTVdLMkxCWTVzODhIbWRYYWZL?=
 =?utf-8?B?WHd4d1hOdllzb3VYUTZ0c0UxUmY0V2NCajB1VGhrN21xNHhNdnJzMExDU1I1?=
 =?utf-8?B?bjRXaDI5WDJLTXF6TFcwWjE5aUxlcHVYYmxUMWNTOXBBdEJ4L1NNV2kxUWw0?=
 =?utf-8?B?U213NFE1V054OC83TE1QVzFFUmQvZWU5d05yTW1DekxmYlB3UGRVR1REOENp?=
 =?utf-8?B?aktHMCs4SGtncXpyR2Ftai9telhSM1Z1bFpwZnlSZ2FIMnNNSHFOam1hY3J4?=
 =?utf-8?B?SGdDOFl4VUM2L1lwTVVvbFhFUERuTEd5WktVa1duYmpMOGF4bEtOa3dnRWl6?=
 =?utf-8?B?S1pkK3BjUGRzdW5LSDlTbDVLUTNid1ExYkFWTEZhWlhXMmw5Z2g4eElZWkor?=
 =?utf-8?B?WlFVYkphMC8vNDJGZ211UW1nUmZ0THZmTjRiMjdvQUtxcHRUNllwWmZsekVs?=
 =?utf-8?B?VkRrL21TN2FwbEhqQUlLNmZpU05aUGNVTnY1cktGN3RMeGFBNHVicWU1cnRO?=
 =?utf-8?B?SEZuSUxmN2F4NkRsQ2VkNmtnMks1eVBUOVNPZ1NFQUg1ZCtiUGdVYkt3NVlL?=
 =?utf-8?B?QjY1NUlmRXlzdHM2d0x3Vk4rRXFuN0tHbFVOUkZybVFtQThNeGFqY0U0NEI4?=
 =?utf-8?B?SGZPbnpENXc4SVZZc21CRmJ3bTk2MnJLMzhkWnBJWk1zbVBzRUpCMGlPT2Ex?=
 =?utf-8?B?OGJQTTVqb0Y4RGJscjJpZURmdVU2dGY0NTNCL2NoVUx1M3R4dThKTTJxdDFD?=
 =?utf-8?B?K3BDZVVTcU1jejk5WTJrdVNEam92cWs5Rkg5UmdvWW5ScW1XNjBxOW14bmIy?=
 =?utf-8?B?d0hmS2RQeFBXakhMK2s2eHdsS0I0MnQ5Y0hMdks0M1AxYWZ1M3NYaVNMcmJT?=
 =?utf-8?B?dklSTnZ6ZzVaRzYyT3dXeHZabUNpYzlKWTBmRDRaL0loeVYxNUUwR21Ua1ZV?=
 =?utf-8?B?Rmc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 7dbe54f3-af93-44fd-665e-08db560a60c4
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 May 2023 12:37:57.9041
 (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: D5C5wvt8ggVR4/xBtKg9oi9Iz9b6m9WwqPR1gmTFWI5LbFdQpzqCmGsQQCYbkkmBB5irj2xm2G0wnAOzfX6L1p5X0TA/4WrsmFaf4UwIpME=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5243
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.1 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,
        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: 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).

> 
> It's not 'metadata is stored as u8', it's 'metadata size is stored as u8' :)
> Maybe I should rephrase it better in v2.
> 
>>
>>> Other important
>>> conditions, such as having enough space for xdp_frame building, are already
>>> checked in bpf_xdp_adjust_meta().
>>>
>>> The requirement of having its length aligned to 4 bytes is still
>>> valid.
BTW I decided to not expand switch-case in __skb_metadata_differs() with
more size values because: 1) it's not a common case; 2) memcmp() is +/-
fast on x86; 3) it's gross already. But I can if needed :D I think it
can be compressed via some macro hell.

(this function is called for each skb when GROing if it carries any
 meta, so sometimes may hurt. Larysa, have you noticed any perf
 regression between meta <= 32 and > 32?)

Thanks,
Olek