From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1F7B342927; Wed, 12 Apr 2023 14:04:42 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EDCFA410F2; Wed, 12 Apr 2023 14:04:41 +0200 (CEST) Received: from NAM12-BN8-obe.outbound.protection.outlook.com (mail-bn8nam12on2057.outbound.protection.outlook.com [40.107.237.57]) by mails.dpdk.org (Postfix) with ESMTP id 7FE8C406A2; Wed, 12 Apr 2023 14:04:40 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=j0vUZgoxUiHY/opeFWANZ1YRmWdpiUIf/3e0IPh0sp1/ZjG/Ir86l7t4xDnBiC03lXeQViFb9Gd+icNKqL3FFC6L/rvCYZkqtWwcajGClMvylJ2ZrRcgHdvEn9rbnml/2HLrPoQIcPaBw1zaTQwMRb+kHINx8DUEfC5tGY1JyAFkWeeuZeWAzpJdGuk99mE7PsW2tLCyYfu+0PwgBMO7NJC/aMsRfUVQqMD2+h+Gl5uGlhuDJHv1gSFyTsT1ye53kCgBwqDvE4+3gh7XHQe1Z4DOnmGeZ8i9toPr6/syOrnCCObBC4SktFr+B2XOf/MvReB3+njZjdxNwtYqayvXVA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=76u7odLRvCeNpvaEynuia8notkMsPTfwLvXAkSGQgMg=; b=oeAPwhGRjJDz7pdMaU3rEsBDORBrF1fMr6HiKRFs1HoaLN9iv9Ihdiu/SCvt2U5+Oz4bsCT6AD1rGdtGs31+bf5yyujVZh7B8ugyfu2UVNCNgJPfh/jnGdl1xtF81/I8Uq1qt403nkGQLHGpBdULTsPL3jRzrjQNDyhqCxGk0K7Oc3YeXpl0x5C60QhtVE9Fjkdev+B76Dtl5+4G/E9Z+j8EwomHipi9v2IK8qHZ5oKfxEjipVsp/d00fYWG0rXfnSb+x8PI3wY+LQ/J4rwLKIj4/N5y8VzfpUPQHtVeIhYSjUxflUPJe9LYv87ZVDLPQnDIcVY12DTx1vP6OVs9Hg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=76u7odLRvCeNpvaEynuia8notkMsPTfwLvXAkSGQgMg=; b=PYdl9knytYVhMcGhXeVQsU1VYjAW9miBLkwXMdohZ+gQ0E4QlNUnC6K2vf079DCkaphPTARNxcDqyeEQ3OCY/CSEdqrpQ/nj1jVNmXj9b8N3mejMIiNz+2bgBc9noMSHgfhyUGmel154vRKE6bdtEhAhzaGsV4ICVPITw6eCFg8= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) by MW6PR12MB8735.namprd12.prod.outlook.com (2603:10b6:303:245::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6254.35; Wed, 12 Apr 2023 12:04:36 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::5e2c:c0ed:88a6:a4c7]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::5e2c:c0ed:88a6:a4c7%7]) with mapi id 15.20.6298.030; Wed, 12 Apr 2023 12:04:36 +0000 Message-ID: Date: Wed, 12 Apr 2023 13:04:31 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.9.1 Subject: Re: [PATCH v3] net/sfc: stop misuse of Rx ingress m-port metadata on EF100 Content-Language: en-US To: Ivan Malov , dev@dpdk.org Cc: Andrew Rybchenko , stable@dpdk.org, Andy Moreton References: <20230309041101.8321-1-ivan.malov@arknetworks.am> <20230312105426.6732-1-ivan.malov@arknetworks.am> From: Ferruh Yigit In-Reply-To: <20230312105426.6732-1-ivan.malov@arknetworks.am> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO4P265CA0068.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:2af::22) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|MW6PR12MB8735:EE_ X-MS-Office365-Filtering-Correlation-Id: 0f7a41c1-5f73-49dc-e147-08db3b4e15c2 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: +QcqYWROIgSkC0ATJ/C9f4arBfK1719gF8yQkEFUccf9i4U+rAAPCDy0ZY1LBdBqSt4AJ3oW9p9naM6tZvWFiqb/8jr3tqVoaEZ5kHu/2Qe0j5WZ+uD0x0uC60pCPEwmXTjnT+iHjf8ryyLZIDflgUqGrrfDxRIX6k7e8htdPodTQuV3luvsivgmxIe6C1O2ffCFTMKNLMBEZFBeKFouUa9WLdhNZ8NrKIFQFtYKstxuOmaRYMhktuS+SCqWN56AJK7iSynhPm+m5yvzREcBpZpRryz7QZy1awcJ4WjvKiCF/GIxupLo/TqBnHuvpmyiKP6VrYvCDHLnw6FESI2mHGB81wddzVahpEVcgFaKniInjS3K2eC7pgmTJaey3HvkeJKzJtFK4sO480+KcWDny+c0dZ3wCQOUKXvybALppjuJ5618HZ2532nopiFY2GupWz909qV/hpWPLAfsjY7FQg/Ifxfmaem61B1dART9x7/4QOkd3CnmKI176WqkPD18CK/lRjfgtUcFhYc0f1JEdDq9vmyEAvg/bOWOibdWuiRqZRepGk0dnqoMllfqpLXJ5iDMA1nSrrYI+l1IM06Znn7PqDCaizD0ALguNIbrD/mZlQsn5bl/OyJiSNDk/etzQlHJzl2noNSW+6PRGp84rw== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH2PR12MB4294.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230028)(4636009)(39860400002)(376002)(396003)(136003)(366004)(346002)(451199021)(54906003)(478600001)(6512007)(83380400001)(26005)(6506007)(2616005)(6666004)(31696002)(86362001)(186003)(53546011)(36756003)(6486002)(44832011)(66556008)(66946007)(66476007)(31686004)(4326008)(38100700002)(41300700001)(8936002)(8676002)(2906002)(5660300002)(316002)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cm1vMmlRZWRRbUtveVZQZWJYcjd1U01ORHh1RTZwYmVZa0pOQ1FMR3FSVEth?= =?utf-8?B?c1p2Rzl4K1FVc3Z3MFVxVnVRNHNDMmFabENQcEVUbWVYaThMZitEN01FbWs5?= =?utf-8?B?c2o4a1R2cHBLTERabnV0RmYxZ0pib3JjdDZLSXcvaGFVMm9HbStXbTlCT2R3?= =?utf-8?B?UlRjY2R5bEc1VUlzcVl6K0VnaUMzWGJmZ09ZaXNFbUtwbHYzZGEvMngxaGRy?= =?utf-8?B?T0FDUXBxcXJWQU9oTndoclB3MmQrcjBpK2N3aFZ5blVMRVhIUEVhdUNGSGhk?= =?utf-8?B?WkdxRFFhV1RLaVVyNVArRDhsVmlJYU83YnhncTNOL0dPNFo0MCtBMkN2T3FP?= =?utf-8?B?UVVidmI4dTdRT0tnSEdtb3UwaEJORURuSjdoczFSdWhpQndyOUpjQ0xqZXdF?= =?utf-8?B?ZlZsMHRSUFlIMmFCdmlBTEpJdnZmVkpTWTk3NlI5NHRMTkRaaHpFRXVZL2dJ?= =?utf-8?B?YnJTRzNSY0xFWWxxYnNwUm5YVnZJTUEvSjMzTkdOR1NGYzdZbXZjMWhzUFEy?= =?utf-8?B?ZzZBYlN4RGVIZ3hqNWNrT1V4ZTNJUkIvc0V2Y1RIYllJbUtsU2N6c0JFaG9K?= =?utf-8?B?N0w5Sld3bFRHeUc5Tk8yZzg5dmpZTWUxNENWM24rSFo2RmZLVnE2ZWxtQlZG?= =?utf-8?B?aCsyZkE5eUkwZ3FlMXpycVJiNm9tZ3hxVFVnZlVScWhBTnNjaUc4MGdqL0VK?= =?utf-8?B?MHhPL3ArMjZQNWJ3eTNXNVZ2TmgvUXJDKzdwWUMxbGNnaUViODZvWTFXb1Yv?= =?utf-8?B?aW16azFYc2liaXNXNjExa1Vqa2hrSnNCNENvWjlPckIwWlExS1dRZkp6S1dC?= =?utf-8?B?ckZQS3k3TU9aVENZTzRrbDhlUGxNaVQ0dThucHlUMkIrc000S0xHYW1qS1hO?= =?utf-8?B?NEZUNUpHMU1oTjJFc2JUdEQ1bytXQ0lHSmpSSDM0azB5TTFOZjhKZnlvZzhF?= =?utf-8?B?THJtdUo4Y1I4NWFYT01sU2JBUjVHdWZoaWRiTEkxNTJFTVFCQmpJU2JBcEhE?= =?utf-8?B?ckhFTmR2MkpwSDVoSGhOR0dpQ0NkR3NsYU5VVER3dlhOdHByMyt2b3AzbXJV?= =?utf-8?B?UUJoZnJieFBZMnlaN0JIMGVwcVd0MWcvTk5JS1F3cGZRZ0pJWENSUEhXeDMr?= =?utf-8?B?SmxFd1RFRjFiOWFseldITzZHcVoxNStaQ0N0ZVlXOU81UmQ0WkhzR3ArdTZH?= =?utf-8?B?NGlYZmRYZTVuQVFRUHdSb1MwdzNlcUd6N0VISmk5U3hIR2VDM2RNdkRkVWFS?= =?utf-8?B?Q0dROXdEZ2dTa3RVb0R0TFA5Q3VJTm9rK2tQRnlTVVlyMm9KL2Z6WGNTZEtl?= =?utf-8?B?YkZVZHE4anlXYkIzaCtORThFTC9LQVNBVnJISkpmV2RRaHhZTVhRVFZsS1VV?= =?utf-8?B?MnBKc2NlMmtQY3c0N09NNTYrV3pROVpRVkw2UURCSXlWeC9SYXNmZTNHUFFZ?= =?utf-8?B?WW56bzdhem1ITm9JOFBpLzlwWm9XS1duWTN5K2N6T1pDQm01R1dSdDRENVl5?= =?utf-8?B?dWZKTFhFZm9zSHkzOGlUSktuYjNHeFRCWkpxZ2pIWHFjVG85dDc2azJaYjNI?= =?utf-8?B?ekdmZ3ZTRUgwbXpWRTltVlpWdFJVbDFYRTBUakJqZkZHWlpnMkFQMm1URjVh?= =?utf-8?B?YVltV3oxbys2YXF1aWRNUTcrVkZjYnRZRVZRZmUwWkxWY3VYT1hVL285QWE1?= =?utf-8?B?WkxPeUJoUnUvcG11dWsyRzZ4aGFCdTBuQVpJbGt2bm1rRUM4UlgvRW9BMkdQ?= =?utf-8?B?MHZUZHZVR3k2TnZUYXJ5Vmk1T0FUZ0FvWjdBZkpzM0w5aFJYYlljOXZ1Lzhv?= =?utf-8?B?MU5XdE0zVmtVZEdHV3FSLzJ0cHZDZ3QzRzB6RmtscGdndUdobkpjQStwSnp3?= =?utf-8?B?dVZxQ0NiejdjdFBCTTQyYktuT05VaS9QdnYzNnNWSXlaVGRTOEQ1cHlVSFVr?= =?utf-8?B?bGJjSktBS0JWUHNmZEpPT3hhaEVlcmpDeWNBamxkeDhMelJEeS9XZEdQSlVa?= =?utf-8?B?NVI3ZHA5ZzRjTVZNb0pQYjdxdTZneC9kUmlIWE4xR2hDR0VFRWZKVVpkTDNM?= =?utf-8?B?VVZaY1o5QXRyTDd0U1NjQVFPcWlWYlBjL2hyK0xwZ2piNy9UZkhyWWxHOTNz?= =?utf-8?Q?4D36ZqA5IFTnqWBj3MXnN1sPw?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 0f7a41c1-5f73-49dc-e147-08db3b4e15c2 X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Apr 2023 12:04:36.3910 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: hsHra/iHwj2UeMeVJqQxoes89ftvVisHi0lrZ/wJFa6hnUljRiiDwNjlzE1jmMoc X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW6PR12MB8735 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 3/12/2023 10:54 AM, Ivan Malov wrote: > The driver supports representor functionality. In it, > packets coming from VFs to the dedicated back-end Rx > queue get demultiplexed into front-end Rx queues of > representor ethdevs as per the per-packet metadata > indicating logical HW ingress ports. On transmit, > packets are provided with symmetrical metadata > by front-end Tx queues, and the back-end queue > transforms the data into so-called Tx override > descriptors. These let the packets bypass flow > lookup and go directly to the represented VFs. > > However, in the Rx part, the driver extracts > the said metadata on every HW Rx queue, that > is, not just on the one used by representors. > Doing so leads to a buggy behaviour. It is > revealed by operating testpmd as follows: > > dpdk-testpmd -a 0000:c6:00.0 -a 0000:c6:00.1 -- -i > > testpmd> flow create 0 transfer pattern port_representor \ > port_id is 0 / end actions port_representor port_id 1 / end > Flow rule #0 created > > testpmd> set fwd io > testpmd> start tx_first > > testpmd> flow destroy 0 rule 0 > Flow rule #0 destroyed > > testpmd> stop > > ---------------------- Forward statistics for port 0 ----------------- > RX-packets: 19196498 RX-dropped: 0 RX-total: 19196498 > TX-packets: 19196535 TX-dropped: 0 TX-total: 19196535 > ----------------------------------------------------------------------- > > ---------------------- Forward statistics for port 1 ----------------- > RX-packets: 19196503 RX-dropped: 0 RX-total: 19196503 > TX-packets: 19196530 TX-dropped: 0 TX-total: 19196530 > ----------------------------------------------------------------------- > > In this scenario, two physical functions of the adapter > do not have any corresponding "back-to-back" forwarder > on peer host. Packets transmitted from port 0 can only > be forwarded to port 1 by means of a special flow rule. > > The flow rule indeed works, but destroying it does not > stop forwarding. Port statistics carry on incrementing. > > Also, it is apparent that forwarding in the opposite > direction must not have worked in this case as the > flow is meant to target only one of the directions. > > Because of the bug, the first 32 mbufs received > as a result of the flow rule operation have the > said metadata present. In io mode, testpmd does > not tamper with mbufs and passes them directly > to transmit path, so this data remains in them > instructing the PMD to override destinations > of the packets via Tx option descriptors. > > Expected behaviour is as follows: > > ---------------------- Forward statistics for port 0 ----------------- > RX-packets: 0 RX-dropped: 0 RX-total: 0 > TX-packets: 15787496 TX-dropped: 0 TX-total: 15787496 > ----------------------------------------------------------------------- > > ---------------------- Forward statistics for port 1 ----------------- > RX-packets: 15787464 RX-dropped: 0 RX-total: 15787464 > TX-packets: 32 TX-dropped: 0 TX-total: 32 > ----------------------------------------------------------------------- > > These figures show the rule work only for one direction. > Also, removing the flow shall cause forwarding to cease. > > Provided patch fixes the bug accordingly. > > Fixes: d0f981a3efd8 ("net/sfc: handle ingress mport in EF100 Rx prefix") > Cc: stable@dpdk.org > > Signed-off-by: Ivan Malov > Reviewed-by: Andy Moreton > --- > v3: extra rework after review feedback > v2: address community review notes > > drivers/net/sfc/sfc_dp_rx.h | 1 + > drivers/net/sfc/sfc_ef100_rx.c | 18 ++++++++++++++---- > drivers/net/sfc/sfc_rx.c | 3 +++ > 3 files changed, 18 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/sfc/sfc_dp_rx.h b/drivers/net/sfc/sfc_dp_rx.h > index 246adbd87c..8a504bdcf1 100644 > --- a/drivers/net/sfc/sfc_dp_rx.h > +++ b/drivers/net/sfc/sfc_dp_rx.h > @@ -69,6 +69,7 @@ struct sfc_dp_rx_qcreate_info { > /** Receive queue flags initializer */ > unsigned int flags; > #define SFC_RXQ_FLAG_RSS_HASH 0x1 > +#define SFC_RXQ_FLAG_INGRESS_MPORT 0x2 > > /** Rx queue size */ > unsigned int rxq_entries; > diff --git a/drivers/net/sfc/sfc_ef100_rx.c b/drivers/net/sfc/sfc_ef100_rx.c > index b7e3397f77..e323156a26 100644 > --- a/drivers/net/sfc/sfc_ef100_rx.c > +++ b/drivers/net/sfc/sfc_ef100_rx.c > @@ -823,6 +823,9 @@ sfc_ef100_rx_qcreate(uint16_t port_id, uint16_t queue_id, > if (rxq->nic_dma_info->nb_regions > 0) > rxq->flags |= SFC_EF100_RXQ_NIC_DMA_MAP; > > + if (info->flags & SFC_RXQ_FLAG_INGRESS_MPORT) > + rxq->flags |= SFC_EF100_RXQ_INGRESS_MPORT; > + > sfc_ef100_rx_debug(rxq, "RxQ doorbell is %p", rxq->doorbell); > > *dp_rxqp = &rxq->dp; > @@ -889,11 +892,18 @@ sfc_ef100_rx_qstart(struct sfc_dp_rxq *dp_rxq, unsigned int evq_read_ptr, > else > rxq->flags &= ~SFC_EF100_RXQ_USER_MARK; > > + > + /* > + * At the moment, this feature is used only > + * by the representor proxy Rx queue and is > + * essential for representor support, so if > + * it has been requested but is unsupported, > + * point this inconsistency out to the user. > + */ > if ((unsup_rx_prefix_fields & > - (1U << EFX_RX_PREFIX_FIELD_INGRESS_MPORT)) == 0) > - rxq->flags |= SFC_EF100_RXQ_INGRESS_MPORT; > - else > - rxq->flags &= ~SFC_EF100_RXQ_INGRESS_MPORT; > + (1U << EFX_RX_PREFIX_FIELD_INGRESS_MPORT)) && > + (rxq->flags & SFC_EF100_RXQ_INGRESS_MPORT)) > + return ENOTSUP; > > if ((unsup_rx_prefix_fields & > (1U << EFX_RX_PREFIX_FIELD_VLAN_STRIP_TCI)) == 0) Hi Ivan, The patch doesn't apply cleanly, and I can't find 'EFX_RX_PREFIX_FIELD_VLAN_STRIP_TCI' in the repo. Can you please rebase the patch on top of latest 'next-net' and send a new version? So that CI can run. If there won't be any functional change, feel free to keep Andrew's ack in next version.