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: <b6dbc792-da77-8fe1-3cf5-dc9d65e7857e@intel.com>
Date:   Mon, 10 Jul 2023 14:58:51 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH V2 net 2/4] net: fec: recycle pages for transmitted XDP
 frames
Content-Language: en-US
To:     Wei Fang <wei.fang@nxp.com>
CC:     "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "ast@kernel.org" <ast@kernel.org>,
        "daniel@iogearbox.net" <daniel@iogearbox.net>,
        "hawk@kernel.org" <hawk@kernel.org>,
        "john.fastabend@gmail.com" <john.fastabend@gmail.com>,
        Shenwei Wang <shenwei.wang@nxp.com>,
        Clark Wang <xiaoning.wang@nxp.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        dl-linux-imx <linux-imx@nxp.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "bpf@vger.kernel.org" <bpf@vger.kernel.org>
References: <20230706081012.2278063-1-wei.fang@nxp.com>
 <20230706081012.2278063-3-wei.fang@nxp.com>
 <7df8d76c-e177-b06c-20e4-e6aacbea6471@intel.com>
 <AM5PR04MB3139BAF71F71A7681E3D0D57882DA@AM5PR04MB3139.eurprd04.prod.outlook.com>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <AM5PR04MB3139BAF71F71A7681E3D0D57882DA@AM5PR04MB3139.eurprd04.prod.outlook.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?ZzQ3Rit2T2h1NkpxL0d0bTdJV3FybEVKWVBBckpWV1dQQ0x2by9jckgvZC94?=
 =?utf-8?B?MVp3T1VQMVVySEs2VFZmSW12ZER5c2YzRXpVQWtaeURibGNuS2UzMUhQeTNr?=
 =?utf-8?B?YW0xNHMrSmY1VElCNm1NcGRWUmcxY1RNc3NuMzVrU242WkRXdnlIcHQ2L2J3?=
 =?utf-8?B?ajg2RENmVXRMRnVQekpaV1pZZklkdk01aHlhUWdtemVaOERiYlhaVnloSC9a?=
 =?utf-8?B?ajEzMUN1S1F4eW5rekVFczZpUjFhcWxBTkJCK0kvdDZGVUs0Ri9YbURISFE3?=
 =?utf-8?B?VEFyWHhQVjFsbzYzY3FVTERKWXBPVlhtcGhud0U3eTNvQ1crTVN6VU8xYk1V?=
 =?utf-8?B?enVpdlNXZ1QvNVRlRllQZldPbnVGenZuNE1Gdk1hQzlGVE1zamIyV3h4NUo2?=
 =?utf-8?B?a1lBdnpDYzJkczduakUvQ1pPNjdSUEc1YlFkZm95bWkyd2pYektpbjBDc1hF?=
 =?utf-8?B?L3VuSUxJaEJZOXJXMDVOcDNpSXBBb0RnN0xDWjBGZzZOY2NWQkZ1UUVWVXlC?=
 =?utf-8?B?ZXRuT2FMaWtIUkdDU1ZhVUtkU1J2S2pUT01KTWZDNldMZzhnWmNldjEzT1Ni?=
 =?utf-8?B?VW5iMkJFZVlzTy9lZjRnU1IzSGxBNThwM2M2NmQxRjRmRXYvdXN5Ny94eit5?=
 =?utf-8?B?T05ZdkdIQUFiakkrSi9xVUh2eEcrdUlRYXo5aTlrZDVLVjdXbkRlODY0K2Zz?=
 =?utf-8?B?OE1FNGNlZHZpNk9RbW0yVEt1emJIMzhCVUVadysyZzNBd2Z4cktDckRaaVA0?=
 =?utf-8?B?ZmplMDd6U1FUSFN1QU1LZWJvem1pM3JHUVZhS2JaUXJRQlFrSWUyam56V0xQ?=
 =?utf-8?B?V09YeTdVVVF6dTFYUW81K2VHbFZxMXBROGtFelhVdXZ1RENRdmtweGExUi9X?=
 =?utf-8?B?dEg5anFEZ05IZG5NQTc4WGtGbzh2Vk9hSUZzYk5TYW00ckY2OTh3RHdmNnF2?=
 =?utf-8?B?c1ZERXRleDF1bW40RlpqNThhOGY1UVVUUVFVV3hoVXh3dGY2S3MvRldCRFcw?=
 =?utf-8?B?bWJiQXhNZjlwcVhxanFGS3VRbkduVXdPWDN5L3lRRnFLQXlJdGhGTkJEWjY4?=
 =?utf-8?B?TVdvcGp4WFg5YmVhS2N1UDUwaW1EQXZpM0Z2SVJXRElORCtYblhjU2dPUTJn?=
 =?utf-8?B?czNEakhDSENoTk5XRmhpNlF0aW02dExNanY4K0NsaXdPaDRRMWFId2ozNWMx?=
 =?utf-8?B?NnRiZzZVOUlRKy9FUTRqZGx0a1RVaEhndUdpUGE1SGxTallQM2FJL0x3bzU5?=
 =?utf-8?B?UUtwT1JveEhkbFp4U2VzZWQ2Q3RUUUxpTk9WL2tEY01nalNRWkhMbVhpNXRq?=
 =?utf-8?B?SEhLRS9POC9aT3NkK3ExbS9CcHR6bzJGakZOZ1NTNzRodXg3ZngycXRvSnlt?=
 =?utf-8?B?STVyRko4UktwZHN6WjFmR0V5RThOTXhuSkRjQ2ZPVHFJMW15cDlMOEMxbFZr?=
 =?utf-8?B?c3NPNUtsU2p1MFRoMmd2NUJkOHRDeGhhL2tRenl2cU9pWEdvUFB5MkJvSnFj?=
 =?utf-8?B?Tmdha2Z4b0p4YThaTDJTREU5THBXWkRvd2FYUXBibTMwbithMmdKQ2t0Syt0?=
 =?utf-8?B?b0sxSG5ML3plWmVHNDZHNC8rdE11NWJjZ2xXWU91SVBpZTl1SVFCRDRmUkM1?=
 =?utf-8?B?VzZqS3VpQ2xDRCtSY2trQktMNU5XVzVpaFdBR0I4VTljSCtBb01vWkUvWFMz?=
 =?utf-8?B?L2c1YU4yS2FYdkQ4eXFmMUhDaFQ1TjJRaitmMUVnWjNDWCtmVUxzRXdOSGpC?=
 =?utf-8?B?bG1VVkJWQ3FOdHF3UHczSm05QnBETzZvRXM2MHVBdTQxdTB2YnRkSC9YU1lG?=
 =?utf-8?B?dWxpNlZMT0tMZTVNamZFMHFtVzhSR0NJbkxORFI5bzk2elBIeGVKemFYcFNX?=
 =?utf-8?B?RmxtUGpDdzB4YTN1QnZTNmcwcUVPSDg4RnllTlYzeW5nT1lXTUVnbGUxK3Zt?=
 =?utf-8?B?eHgxQzhPVVg3anNRVzU4cFQrb3dSK2s1NWZUZWU2VSsvZzloWjRXUjc2Qngx?=
 =?utf-8?B?aitERG9tZGdZTUtmT1VxRHdFUEhJRll4NmRCVnU0NDRHaHdNakNTUXAwUnVl?=
 =?utf-8?B?dktuUk5VcXM3WHpmREhvZ0dNOWFjRTVDY2oydjAwN2pXY3FQNmRLYkIzZjZ3?=
 =?utf-8?B?dVFjN1Y4RCtIYm5CdmQvTVQvcHlLcnpqaEo1MGtKSVV1UHBvemptTm1veWhw?=
 =?utf-8?B?eVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 6f5c909f-e2aa-4c8b-402b-08db8145b197
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Jul 2023 13:00:53.8288
 (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: a7JYGo8AF2ybVK7wBwgsyr0S8SiVq56RBVfwk75ViXDm0bD9xZA3HXonfWgxKlgvtoT13IfEo6vA42bqLWblWB/8KbU8sCkxjzC3Ui7tGVA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB7489
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.2 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_BLOCKED,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: Wei Fang <wei.fang@nxp.com>
Date: Fri, 7 Jul 2023 01:54:04 +0000

>> -----Original Message-----
>> From: Alexander Lobakin <aleksander.lobakin@intel.com>
>> Sent: 2023年7月6日 19:59
>> To: Wei Fang <wei.fang@nxp.com>
>> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
>> pabeni@redhat.com; ast@kernel.org; daniel@iogearbox.net;
>> hawk@kernel.org; john.fastabend@gmail.com; Shenwei Wang
>> <shenwei.wang@nxp.com>; Clark Wang <xiaoning.wang@nxp.com>;
>> netdev@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
>> linux-kernel@vger.kernel.org; bpf@vger.kernel.org
>> Subject: Re: [PATCH V2 net 2/4] net: fec: recycle pages for transmitted XDP
>> frames
>>
>> From: Wei Fang <wei.fang@nxp.com>
>> Date: Thu,  6 Jul 2023 16:10:10 +0800
>>
>>> From: Wei Fang <wei.fang@nxp.com>
>>>
>>> Once the XDP frames have been successfully transmitted through the
>>> ndo_xdp_xmit() interface, it's the driver responsibility to free the
>>> frames so that the page_pool can recycle the pages and reuse them.
>>> However, this action is not implemented in the fec driver.
>>> This leads to a user-visible problem that the console will print the
>>> following warning log.
>>
>> [...]
>>
>>> +				if (txq->tx_buf[i].xdp) {
>>> +					xdp_return_frame(txq->tx_buf[i].xdp);
>>> +					txq->tx_buf[i].xdp = NULL;
>>> +				}
>>> +
>>> +				/* restore default tx buffer type: FEC_TXBUF_T_SKB */
>>> +				txq->tx_buf[i].type = FEC_TXBUF_T_SKB;
>>
>> Here and in the related places below: maybe set ::type dynamically when
>> sending to either SKB or XDP instead of setting it only for XDP and then
>> restoring each time?
> I also considered this method. but when the skb has frags or needs to be TSO,
> only the last tx_buf of the skb needs to store the skb pointer, but all the tx_buf
> of the skb needs to set the type explicitly, I think it is a bit mess and not concise.
> So I restore the type to default when recycle the BDs. Anyway, it;s just a difference
> in implement, if you guys insist it's better to set the type explicitly, I will modify
> the patch. Thanks!

Just more of personal preference, no problems. Moreover, your
explanation makes sense to me.

>>
>>>  			}
>>> +
>>>  			bdp->cbd_bufaddr = cpu_to_fec32(0);
>>>  			bdp = fec_enet_get_nextdesc(bdp, &txq->bd);
>>>  		}
>> [...]
>>
>> Thanks,
>> Olek

Thanks,
Olek