From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
To: Artemii Morozov <Artemii.Morozov@arknetworks.am>, dev@dpdk.org
Cc: Ivan Malov <ivan.malov@arknetworks.am>,
Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>,
Andy Moreton <amoreton@xilinx.com>
Subject: Re: [PATCH v5 2/3] common/sfc_efx/base: add support to enable VLAN stripping
Date: Tue, 20 Jun 2023 14:50:28 +0300 [thread overview]
Message-ID: <f4db8ec2-b449-d05a-9f1f-38e00298b0a0@oktetlabs.ru> (raw)
In-Reply-To: <34d912b9-4fc0-246d-b89d-be6e4906f36f@arknetworks.am>
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)
>>
next prev parent reply other threads:[~2023-06-20 11:50 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-31 13:41 [PATCH 0/3] net/sfc: support VLAN stripping offload Artemii Morozov
2023-05-31 13:41 ` [PATCH 1/3] common/sfc_efx/base: report VLAN stripping capability Artemii Morozov
2023-06-01 8:12 ` [PATCH v2 0/3] net/sfc: support VLAN stripping offload Artemii Morozov
2023-06-01 8:12 ` [PATCH v2 1/3] common/sfc_efx/base: report VLAN stripping capability Artemii Morozov
2023-06-01 14:35 ` [PATCH v3 0/3] net/sfc: support VLAN stripping offload Artemii Morozov
2023-06-01 14:35 ` [PATCH v3 1/3] common/sfc_efx/base: report VLAN stripping capability Artemii Morozov
2023-06-01 14:35 ` [PATCH v3 2/3] common/sfc_efx/base: add support to enable VLAN stripping Artemii Morozov
2023-06-01 14:35 ` [PATCH v3 3/3] net/sfc: support VLAN stripping offload Artemii Morozov
2023-06-01 8:12 ` [PATCH v2 2/3] common/sfc_efx/base: add support to enable VLAN stripping Artemii Morozov
2023-06-01 8:12 ` [PATCH v2 3/3] net/sfc: support VLAN stripping offload Artemii Morozov
2023-05-31 13:41 ` [PATCH 2/3] common/sfc_efx/base: add support to enable VLAN stripping Artemii Morozov
2023-05-31 13:41 ` [PATCH 3/3] net/sfc: support VLAN stripping offload Artemii Morozov
2023-06-01 15:30 ` [PATCH v4 0/3] " Artemii Morozov
2023-06-01 15:30 ` [PATCH v4 1/3] common/sfc_efx/base: report VLAN stripping capability Artemii Morozov
2023-06-02 7:22 ` Andrew Rybchenko
2023-06-01 15:30 ` [PATCH v4 2/3] common/sfc_efx/base: add support to enable VLAN stripping Artemii Morozov
2023-06-02 8:32 ` Andrew Rybchenko
2023-06-08 11:16 ` Artemii Morozov
2023-06-08 12:37 ` Andrew Rybchenko
2023-06-01 15:30 ` [PATCH v4 3/3] net/sfc: support VLAN stripping offload Artemii Morozov
2023-06-02 8:46 ` Andrew Rybchenko
2023-06-13 15:12 ` [PATCH v5 0/3] " Artemii Morozov
2023-06-13 15:12 ` [PATCH v5 1/3] common/sfc_efx/base: report VLAN stripping capability Artemii Morozov
2023-06-19 9:43 ` Andrew Rybchenko
2023-06-13 15:12 ` [PATCH v5 2/3] common/sfc_efx/base: add support to enable VLAN stripping Artemii Morozov
2023-06-19 10:28 ` Andrew Rybchenko
2023-06-20 9:55 ` Artemii Morozov
2023-06-20 11:50 ` Andrew Rybchenko [this message]
2023-06-20 13:10 ` Artemii Morozov
2023-06-20 13:53 ` Andrew Rybchenko
2023-06-13 15:12 ` [PATCH v5 3/3] net/sfc: support VLAN stripping offload Artemii Morozov
2023-06-19 10:36 ` Andrew Rybchenko
2023-06-22 11:31 ` [PATCH v6 0/4] " Artemii Morozov
2023-06-22 11:31 ` [PATCH v6 1/4] common/sfc_efx/base: report VLAN stripping capability Artemii Morozov
2023-06-22 11:46 ` Andrew Rybchenko
2023-06-22 11:31 ` [PATCH v6 2/4] common/sfc_efx/base: add API to get installed filters count Artemii Morozov
2023-06-22 11:51 ` Andrew Rybchenko
2023-06-22 11:31 ` [PATCH v6 3/4] common/sfc_efx/base: add support to enable VLAN stripping Artemii Morozov
2023-06-22 11:54 ` Andrew Rybchenko
2023-06-22 11:31 ` [PATCH v6 4/4] net/sfc: support VLAN stripping offload Artemii Morozov
2023-06-22 12:07 ` Andrew Rybchenko
2023-06-22 15:11 ` [PATCH v7 0/4] " Artemii Morozov
2023-06-22 15:11 ` [PATCH v7 1/4] common/sfc_efx/base: report VLAN stripping capability Artemii Morozov
2023-06-22 15:11 ` [PATCH v7 2/4] common/sfc_efx/base: add API to get installed filters count Artemii Morozov
2023-06-22 15:42 ` Andrew Rybchenko
2023-06-22 15:11 ` [PATCH v7 3/4] common/sfc_efx/base: add support to enable VLAN stripping Artemii Morozov
2023-06-22 15:40 ` Andrew Rybchenko
2023-06-22 15:11 ` [PATCH v7 4/4] net/sfc: support VLAN stripping offload Artemii Morozov
2023-06-22 15:41 ` Andrew Rybchenko
2023-06-22 16:05 ` Ferruh Yigit
2023-06-23 5:47 ` [PATCH v8 0/4] " Artemii Morozov
2023-06-23 5:47 ` [PATCH v8 1/4] common/sfc_efx/base: report VLAN stripping capability Artemii Morozov
2023-06-23 5:47 ` [PATCH v8 2/4] common/sfc_efx/base: add API to get installed filters count Artemii Morozov
2023-06-23 7:24 ` Ivan Malov
2023-06-23 5:47 ` [PATCH v8 3/4] common/sfc_efx/base: add support to enable VLAN stripping Artemii Morozov
2023-06-23 5:47 ` [PATCH v8 4/4] net/sfc: support VLAN stripping offload Artemii Morozov
2023-06-23 12:35 ` [PATCH v8 0/4] " Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f4db8ec2-b449-d05a-9f1f-38e00298b0a0@oktetlabs.ru \
--to=andrew.rybchenko@oktetlabs.ru \
--cc=Artemii.Morozov@arknetworks.am \
--cc=amoreton@xilinx.com \
--cc=dev@dpdk.org \
--cc=ivan.malov@arknetworks.am \
--cc=viacheslav.galaktionov@arknetworks.am \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).