Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Message-ID: <6ba23f52-6a80-4db4-8081-f6568c81b6ca@intel.com>
Date:   Thu, 7 Dec 2023 15:27:13 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH iwl] idpf: fix corrupted frames and skb leaks in singleq
 mode
Content-Language: en-US
To:     Eric Dumazet <edumazet@google.com>
CC:     Simon Horman <horms@kernel.org>,
        <intel-wired-lan@lists.osuosl.org>,
        Michal Kubiak <michal.kubiak@intel.com>,
        Przemek Kitszel <przemyslaw.kitszel@intel.com>,
        Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <20231201143821.1091005-1-aleksander.lobakin@intel.com>
 <20231207115751.GG50400@kernel.org>
 <CANn89iKiiexYLXP1fW3anaZt0TJWPUBBmXdjsAV8t3CH3Ra8aw@mail.gmail.com>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <CANn89iKiiexYLXP1fW3anaZt0TJWPUBBmXdjsAV8t3CH3Ra8aw@mail.gmail.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?S1lNR1EvWFlHbXdCMGorclhVOTdYck9DWFJzaVNvK0NOVjZPRkFRMmEwZlJW?=
 =?utf-8?B?VGphRjZqeWpSd09HSzdYWVk2ZFVsSDJVczZwZ0RwSE9yTHhDbWpHb2dkTUhC?=
 =?utf-8?B?cUlrRlM5cjlTS0tLYXRaV1lYM2tsalRLR3hKbVp6RFlkQjdCTVFpb0phT0dS?=
 =?utf-8?B?ZjlJM2JZQ3BSa3k4YVBzelQvc3ZTT001bjlpeE4zekJwbkVmNDJQL3NveUJs?=
 =?utf-8?B?dVUxV01vUkdGdlFpRmk1VC9WL0NHYWs5VEdsNEdzYjNqdUJkcWlOMnkxWXZU?=
 =?utf-8?B?aHk1STY3NXhXTzJiazI1Yi9kV0JzUG9kOEY3WmNRdWs4aGxjSXNzd2szb2lN?=
 =?utf-8?B?ZTgxTXd0TUVRQkMrb2txUkw4ZG5KVXBEVHZsWlIyb2VyY3h3SFZyTkdFUVhQ?=
 =?utf-8?B?Kzl4OHZ6U1pWTUh2cm1NbHh2SjVObHBFamdndXdlWUpJZU41QlZlTytpYTJx?=
 =?utf-8?B?L1RQWGVPc2RtMC9idGltNG1nNWNjWGh5WUVlbUpHZ2ZBaEZUS3V2RUVHMWZu?=
 =?utf-8?B?SG1IRzRjK1NVaVhFRjNqcWVOMW1Dc0RaRHpIeVZQM0xSUzNuOU9CbHpXaWpY?=
 =?utf-8?B?dDFucGVRWkthdjU4anQrOSt4Rzl5cDJjanBjRWIrbk5KRU1KUCttTGwyS2dG?=
 =?utf-8?B?WlZWRExZTHFSOUxOckhZT3RGc2NyTVVjVzBqUWlOSXhyYmZTcG1XYnVyNjVh?=
 =?utf-8?B?eFBjQXFMUW56MlFjRFZIekFzWjA3NzRsSjc1bng3dWttUFBoNEVmc0h2clVk?=
 =?utf-8?B?QS8zT3VXaEJRZW5qUGtoRnBFKzlsT0VETHpMOEQyWlh4Q29SeDVld0JiRTRM?=
 =?utf-8?B?SEsrbWdkN1BsaGtrS0tjeVRXK3VqdC9BQW1RMGJMekRwelhmQm1iZU05MjZH?=
 =?utf-8?B?c0djMlNyZHBEQXQ0Y2NRUTUvOGFURGlEQU1Kc2lqRW03MGQ5Y0RWL3VBU2Rt?=
 =?utf-8?B?ZytaUEQzS3Iva0tHS3VWNzhLTHpSN2hXS3JQc2xsNlJHbVJkdWtQanJzZjN0?=
 =?utf-8?B?ekVjbFpiSVV1VXZqZjNldFlGbTdyYzBYbGh6NldEb1JxL0pBdStYZmRIWWFj?=
 =?utf-8?B?SC8vS0JNL09qRUFvakxBQmtDaGZPS1NJMU0xOTFoWjJ3UjhZaStwQ0hsSjBQ?=
 =?utf-8?B?bzgrajU1cmpIWTVRQys5N3RySjcxNURBbVQ2dzBkZTltaDdPRngremt0aWRo?=
 =?utf-8?B?VjJWUk8xQkZZUWNGeTkxem5wZTZYS1FuNUx4c2tveU1hY1dGU1ZwOFA5QmhO?=
 =?utf-8?B?Y1VqcUREajhObE4xaEp2QlpmU2dsSWExMWE2UmVEY2VHRzFDdm13NEdkUTJM?=
 =?utf-8?B?RjQxek44Q0l6eDZkU2FVOEpqb3NleDhZenlzMG45OEJQTFZ5cndESW5hcG95?=
 =?utf-8?B?Y3R1QlUvajVoZ3lwcDlJWldiUWNOeGtHbXk0bzV3ZjJaVE92NS85WXAyNlNB?=
 =?utf-8?B?alJuZGI5MDBMaHRvL1MzVFg3SVprTUtaNjBWcVJYMmtHSWJzUjBmR1dHalM4?=
 =?utf-8?B?c3ZCay9TUzV4KzArYTgrL3lpdjNWWXV3djBiYmgybnhxcGI5M3g3dVEwRFZv?=
 =?utf-8?B?dm9JaDIzQ2dpTHNqU3JXTmhjQVZsSWZMUjlJMjhXV09ETW5IS2ZiaHU0Q2hU?=
 =?utf-8?B?dFpDK2w1bHBrYUEwWUZlRkp5Y2t0QmUrdmt1U1d2TzAweG16WFNHTFh6SUI3?=
 =?utf-8?B?L3R4U3BNdnJrRkhGVm1RVWNJVEJqVmZrTjhFdDgzci9Ed1lLTlRJNnFGbHVX?=
 =?utf-8?B?YnpFYUk1aG50VENsRnJHSTVGcHF2UlFxS01MWXZyRENOTlVmM3VlNzIrajgy?=
 =?utf-8?B?aXhma1BPS1dtaHlsQThIZFRYblk4bjBpOUlKSE9xbzlndG15THpTS2QvOVJi?=
 =?utf-8?B?eXZaMGpudTlzS3djdXd0NytldXZ4MU5JeVRtSjhnYlNEM0Q1M3dOMzdTMzBO?=
 =?utf-8?B?STZOVVZXZXJuZVYvMktYY0ltdVFJdlZtMlA5eWVIYTA0OTdrQlJzQWlSZkM1?=
 =?utf-8?B?UEpnc0pXa1RXSjgwQUJvcjRRT3gxdWpIR3JBTHo5Wm9JZXlyU1JjZ3ZBemha?=
 =?utf-8?B?ZXRlTngzbzFpMDlDS0tPVkF0Qlh1ZmU1NnZXMDZpc0k2Tk5LQjAzSW0ydW93?=
 =?utf-8?B?ZUNxR0hsNXFYU1pScThNYUV6SlUwUEFBVktISWFWdVFvS0JkYkhJcjZKREpw?=
 =?utf-8?B?T2c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 45c4d7c4-20dc-4df2-0cda-08dbf730b31b
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Dec 2023 14:27:54.0856
 (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: P5wVSq15oTAyCAmBIeAMdv8PZb4ezYT6UaTEiER03KS5E8qxu7v4e0pEJcsKHPTlRDz1WrR1OPLMF3kWnyEq/5MU0bmZcbldfkXOAHjth8E=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR11MB4685
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Thu, 07 Dec 2023 06:28:14 -0800 (PST)

From: Eric Dumazet <edumazet@google.com>
Date: Thu, 7 Dec 2023 13:48:52 +0100

> On Thu, Dec 7, 2023 at 12:58 PM Simon Horman <horms@kernel.org> wrote:
>>
>> On Fri, Dec 01, 2023 at 03:38:21PM +0100, Alexander Lobakin wrote:
>>> idpf_ring::skb serves only for keeping an incomplete frame between
>>> several NAPI Rx polling cycles, as one cycle may end up before
>>> processing the end of packet descriptor. The pointer is taken from
>>> the ring onto the stack before entering the loop and gets written
>>> there after the loop exits. When inside the loop, only the onstack
>>> pointer is used.
>>> For some reason, the logics is broken in the singleq mode, where the
>>> pointer is taken from the ring each iteration. This means that if a
>>> frame got fragmented into several descriptors, each fragment will have
>>> its own skb, but only the last one will be passed up the stack
>>> (containing garbage), leaving the rest leaked.
>>> Just don't touch the ring skb field inside the polling loop, letting
>>> the onstack skb pointer work as expected: build a new skb if it's the
>>> first frame descriptor and attach a frag otherwise.
>>>
>>> Fixes: a5ab9ee0df0b ("idpf: add singleq start_xmit and napi poll")
>>> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>>> Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
>>> Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
>>
>> Reviewed-by: Simon Horman <horms@kernel.org>
> 
> It seems singlequeue mode is not really used on idpf :)

From what I know, there's currently no hardware supporting singleq mode.
I'd remove it completely, but seems like it was decided to keep it in
case someone would like to support it one day...

> 
> Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks,
Olek