Date:   Thu, 9 Mar 2023 17:34:55 +0100
From:   Simon Horman <simon.horman@corigine.com>
To:     Leesoo Ahn <lsahn@ooseel.net>
Cc:     Giuseppe Cavallaro <peppe.cavallaro@st.com>,
        Alexandre Torgue <alexandre.torgue@foss.st.com>,
        Jose Abreu <joabreu@synopsys.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        John Fastabend <john.fastabend@gmail.com>,
        netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        bpf@vger.kernel.org
Subject: Re: [PATCH net-next] net: stmmac: call stmmac_finalize_xdp_rx() on a
 condition
Message-ID: <ZAoKr+EtakUhrUVI@corigine.com>
References: <20230308162619.329372-1-lsahn@ooseel.net>
 <ZAnh0TGtDkVUl/1m@corigine.com>
 <e75d2a42-4154-e469-bbd7-9409471ab724@ooseel.net>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <e75d2a42-4154-e469-bbd7-9409471ab724@ooseel.net>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?anIwZkRkQWhHTGE2bkppUmhDSjA5cVFCYmU4T0VVNnZURk90VDVwb2laTTB4?=
 =?utf-8?B?OVZtOWQxbU5lQWw3b2hKa1A1T1lINEZFTTg3NGN0YnVmc0E3ZXM4RlJYbWRu?=
 =?utf-8?B?STVnS1RkTFdNWFVadmk1WnhWdHp4UndzenRFUTIzaGI0d2RlWHUxVTYrdloy?=
 =?utf-8?B?ZVJFNGpVcGE5c3N5MGlWdC9hbjFlZGFoa3dUN2kvL3QzZEloOXJoRnBWc2Fs?=
 =?utf-8?B?aWhBNlEvNlYrSERYVjc3V1QvQ1dlZDVLM1RhbnV4dUs3OGY0VkZsdG0xQTU3?=
 =?utf-8?B?T1UyUmd0QTExM0lqNFVoTkxNdzlwUmRWdFZ1bmtOVWt5Y05KNG1JUHVpd3ZL?=
 =?utf-8?B?aHhQTnlYdGV6enJqdXJzaFZsUDZoRG4xRVNqcEIxeHAxNVZ2NVJpeHRWVHRl?=
 =?utf-8?B?aiszbkhoVXo0c3MzeXY3T2dlaVRqeHIwNktGbmd1bzZUc0ZLMTZaS01RdEY0?=
 =?utf-8?B?Mkg3MEE2dFJtQ0tVM2ZjS3cvZFhyNllYeFd6OGk5SnFVUlhGQ3pVSHkwQWxJ?=
 =?utf-8?B?dENJamhxbjNUMVlvQjdlQmtnWUR1R3ZvVm5CbFRnR1BoVEJ6SXlrY1QvNjMw?=
 =?utf-8?B?dG9pSUlBR204c1dmeEM5SVRtVm1QNkE5RHA2QVNMNy9BWTBteFNNb2tVTWsx?=
 =?utf-8?B?TWNYM3JCM2haVGM3Vml6bzR3czd4OHNWd3M3QWU5ZXlISGxqU0RvdnJKckpC?=
 =?utf-8?B?OUpRSDhmUUZYd3A4WTZMR2ZjVFJUWTRBVkRuRnVJaksrd2h2R3J0NVVqZ3pi?=
 =?utf-8?B?UXFDMHVvZ0tnUzhJL1JaRkpFNWt6ZUoxQmp4RldmVUpMNFRXM3dUTjZ0QWYw?=
 =?utf-8?B?RFlpcFgrblhHN1c0WHZYRlZWb0lQRDc2Qm90RjNwVG1OYnNXSHBBNS9VakFy?=
 =?utf-8?B?dkwrV1lGRVMyWE9mcCtYeDV6bTRwZURsR1BLWVhrb3hEV3YvZm50czl2OFQz?=
 =?utf-8?B?REFvbG1UZDN1ckFJaUhiNThZOHpvM3ZOK3pLNjN1MnEza3NmcXZ5WHpTcS8x?=
 =?utf-8?B?bXRINlAvVm9OUEt6RnlwZU1hSDFBRUZUQ3ljdmJscnZ2Yng0ZzRBT2c1R2Rs?=
 =?utf-8?B?MUZZa2t5YmJKM3p2VjJ2QnlCenRxMWtROVNZQmlXd1cveVI2VGxUZWVST3kx?=
 =?utf-8?B?bUJFM3pNNXoyTUUyVTBjci9IeEUwSHhHYm1tc21IL2FlR050YkxFNWJmaGF1?=
 =?utf-8?B?S0lPeGNLbS9rNSt4cW4vcG4zMzdrRFRwQktmR3JvU240YjBwbjBYUmFxNGFF?=
 =?utf-8?B?SGIwSi81ZjUwQVpub0tXa0dYOHJ1L3VoOWpRNEVtQUpFZjA0Q0VyMU4yWDVS?=
 =?utf-8?B?VjE1UDJrRVVVSmFDclVqN0xGdks3aFUwUlEvajBmT2VLSEt0NXVMRFlKM0FJ?=
 =?utf-8?B?bkJYVGxmS2FNVFgrU0M5a1o1VWhoS0VydU85dlNwamo5RGx1ZXNTYW53Mk5k?=
 =?utf-8?B?bUZWZUZISXlFczNLQlRnY09OKzNLUjBWbm96dENYMUxoZlAxNXBLRXlDSTZ1?=
 =?utf-8?B?K3dxb2IzNkppUkJxQisyNmxhTjBQdXFoL1JBcjZYelJGb29QbFFkTXpjaVNS?=
 =?utf-8?B?S3FBTjFYL202Zjl2Ni9WOGpqM1V1NjhCdzV3VEIrYkIyWHFTeThoNFYvRzNN?=
 =?utf-8?B?VmUyaDFPUktrWk1IbFkxVTBRV05DU0k0YjM1b2tMZVdOaU0rcVZtOHFzMTZF?=
 =?utf-8?B?cXdSRTRqNTY1Q3BYSUdNN0tDTjVNRVhIbVQrOUF0MzhMOS9zWXZWZGMrMnJM?=
 =?utf-8?B?di9kRHJHOUVIZGVtSnBzQnoxQ3pXZC9QNHF2cXVlSnI5elhYYUFBRXRzdTd5?=
 =?utf-8?B?RzI0YjgwNUcvMzEzMExxVnZFVWxVNThnS05IK2ZoU1MwWVFMY0JDNDNDVDky?=
 =?utf-8?B?bFBkN0RqRU51eTFxajRuOWprdU1WbTVvQVBjNHZRRjRSbkdkMFRaRlFMTFcv?=
 =?utf-8?B?dzh6ZFNwT2NRbmwwejIwWTdwTG5rUGxES1hDakUrVm1WbzRCWG9adXRWVmVC?=
 =?utf-8?B?L1pGcWRqN3FZcFR0ZkZqd09NOUU3NmhzMXkzcHhDZVR4TjdkaDViTHoxcWsw?=
 =?utf-8?B?SEZYSENYOWtiaVdIWVVGb1pVRkxjbThyYjMwYnpBV1IzQXFrMXY2RndKQXI3?=
 =?utf-8?B?QXNDeUFNMmhQejBqenJPbytCdVkwbUY5azlzdEF4d25UcDFvV01GcldOS0F5?=
 =?utf-8?B?R0lGZ2w0ZURjdFdCNnRoT2V2NjJYelcwWnpZNWFsTUF4VnFYUDEreHpLWXpP?=
 =?utf-8?B?YVM3RUwxS040ZmMvM3Z4WVUvV3FRPT0=?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1282c72e-38d8-402f-38f5-08db20bc3bdd
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Mar 2023 16:35:03.4913
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: CE7S7e2/VSky1izBGGWe1ducfa5bUcRhBRTOJzyqf0d8i5Bzj2jpyXCNkw6cgbi4m62GxrfGj+N6gHpYR8FRJxTSXk6OSMFW3vaOv2zOcjs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR13MB5188
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Mar 10, 2023 at 12:08:29AM +0900, Leesoo Ahn wrote:
> 
> 
> On 23. 3. 9. 22:40, Simon Horman wrote:
> > On Thu, Mar 09, 2023 at 01:26:18AM +0900, Leesoo Ahn wrote:
> > > The current codebase calls the function no matter net device has XDP
> > > programs or not. So the finalize function is being called everytime when RX
> > > bottom-half in progress. It needs a few machine instructions for nothing
> > > in the case that XDP programs are not attached at all.
> > > 
> > > Lets it call the function on a condition that if xdp_status variable has
> > > not zero value. That means XDP programs are attached to the net device
> > > and it should be finalized based on the variable.
> > > 
> > > The following instructions show that it's better than calling the function
> > > unconditionally.
> > > 
> > >    0.31 │6b8:   ldr     w0, [sp, #196]
> > >         │    ┌──cbz     w0, 6cc
> > >         │    │  mov     x1, x0
> > >         │    │  mov     x0, x27
> > >         │    │→ bl     stmmac_finalize_xdp_rx
> > >         │6cc:└─→ldr    x1, [sp, #176]
> > > 
> > > with 'if (xdp_status)' statement, jump to '6cc' label if xdp_status has
> > > zero value.
> > > 
> > > Signed-off-by: Leesoo Ahn <lsahn@ooseel.net>
> > Hi Leesoo,
> > 
> > I am curious to know if you considered going a step further and using
> > a static key.
> > 
> > Link: https://www.kernel.org/doc/html/latest/staging/static-keys.html
> 
> Thank you for the review.
> 
> The function must be called for only XDP_TX or XDP_REDIRECT cases.
> So using a static key doesn't look good and the commit message is not clear
> for 'why' as well.
> I think that's why you suggested for using 'static key' by the latter
> reason.

Yes, my suggestion was based on the performance optimisation
aspect of your patch.

> I will edit the message and post v2 soon.

Thanks