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 E5E0242D07; Tue, 20 Jun 2023 13:50:31 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C00A14068E; Tue, 20 Jun 2023 13:50:31 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 39636400D6 for ; Tue, 20 Jun 2023 13:50:30 +0200 (CEST) Received: from [192.168.1.40] (unknown [188.170.81.161]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 67F3B50; Tue, 20 Jun 2023 14:50:29 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 67F3B50 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1687261829; bh=sGpX2raKjH56xS6lFfP5jvYspDB02JFgahl+SSPX+QE=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=KPSvjfJu+n2t7VgtMshofSsk4bKHCa8p+cS5gWGBVTK1bnSKBEgbJFz3JoaNixp8v x14Dsy/UbC2IIlJ55YdQC23ljcVCA6+dPr1aePXkeVgOFKE/8JoXE2ghIfSWJthW9N pjGhOX2Wlv5xAvaVN0ydPiXkTcewM21+wwQJctVE= Message-ID: Date: Tue, 20 Jun 2023 14:50:28 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [PATCH v5 2/3] common/sfc_efx/base: add support to enable VLAN stripping Content-Language: en-US To: Artemii Morozov , dev@dpdk.org Cc: Ivan Malov , Viacheslav Galaktionov , Andy Moreton References: <20230531134122.119508-1-artemii.morozov@arknetworks.am> <20230613151238.59112-1-artemii.morozov@arknetworks.am> <20230613151238.59112-3-artemii.morozov@arknetworks.am> <533f1b4a-2b8a-3b45-2015-f241d8c66efd@oktetlabs.ru> <34d912b9-4fc0-246d-b89d-be6e4906f36f@arknetworks.am> From: Andrew Rybchenko In-Reply-To: <34d912b9-4fc0-246d-b89d-be6e4906f36f@arknetworks.am> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 6/20/23 12:55, Artemii Morozov wrote: > On 6/19/23 14:28, Andrew Rybchenko wrote: > >> On 6/13/23 18:12, Artemii Morozov wrote: >>> To enable VLAN stripping, two conditions must be met: >>> the corresponding flag must be set and the appropriate >>> RX prefix should be requested. >> >> RX -> Rx >> >>> VLAN stripping is supported for ef100 datapath only. >> >> "ef100 datapath" does not make sense in libefx. >> "VLAN stripping is supported on EF100 only." >> However, such string could be confusing in the future. So, I'd drop >> "only" >> >> LGTM, but the problem of the patch is that it does not ensure and does >> not highlight in any way that efx_port_vlan_strip_set() must be called >> before any filter insertion to have things consistent. > > This function is called before the sfc_rx_default_rxq_set_filter I'm talking about a way to make sure that it is always the case. Not this particular case. >> efx_port_vlan_strip_set() should check that no filters are installed >> (directly or indirectly via default RxQ set). > > I believe we have the following options: > > 1. Move the function efx_port_vlan_strip_set before filter > initialization(efx_filter_init) and include the statement > "EFSYS_ASSERT(!(enp->en_mod_flags & EFX_MOD_FILTER))" at the beginning > of the efx_port_vlan_strip_set function. This one is too strong. Also it does the check in debug build only. We need run-time check and less strong. Filters could be initialized, but it should be no Rx filters installed. > 2. Introduce a new callback function called > efx_mac_filter_default_rxq_get. Invoke this function at the start of > efx_port_vlan_strip_set and verify that it returns NULL. This one is wrong since it could be filters installed via add. So, it is not a question about default filters only. All Rx filters. > > I don't see any other ways to check that the default filters have been > initialized. Could you advise me how to do it better? Am I missing > something? EF10 counts all installed filters if I remember correctly. So, if number of installed filters is 0, everything is OK. > >> >>> diff --git a/drivers/common/sfc_efx/base/efx_port.c >>> b/drivers/common/sfc_efx/base/efx_port.c >>> index a5f982e335..7804eb76bc 100644 >>> --- a/drivers/common/sfc_efx/base/efx_port.c >>> +++ b/drivers/common/sfc_efx/base/efx_port.c >>> @@ -204,6 +204,24 @@ efx_loopback_type_name( >>>     #endif    /* EFSYS_OPT_LOOPBACK */ >>>   +    __checkReturn    efx_rc_t >>> +efx_port_vlan_strip_set( >>> +    __in        efx_nic_t *enp, >>> +    __in        boolean_t enabled) >>> +{ >>> +    efx_port_t *epp = &(enp->en_port); >>> +    efx_nic_cfg_t *encp = &(enp->en_nic_cfg); >>> + >>> +    EFSYS_ASSERT3U(enp->en_magic, ==, EFX_NIC_MAGIC); >>> + >>> +    if (enabled && !encp->enc_rx_vlan_stripping_supported) >>> +        return ENOTSUP; >> >> Return value must be in parenthesis in libefx (common/sfc_efx/base). >> >>> + >>> +    epp->ep_vlan_strip = enabled; >>> + >>> +    return 0; >> >> Return value must be in parenthesis in libefx (common/sfc_efx/base). >> >>> +} >>> + >>>               void >>>   efx_port_fini( >>>       __in        efx_nic_t *enp) >>