[PATCH v5 2/3] common/sfc_efx/base: add support to enable VLAN stripping
Andrew Rybchenko
andrew.rybchenko at oktetlabs.ru
Tue Jun 20 15:53:02 CEST 2023
On 6/20/23 16:10, Artemii Morozov wrote:
>
> On 6/20/23 15:50, Andrew Rybchenko wrote:
>> 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.
> I think you're talking about eft_unicast_filter_count,
> eft_multicast_filter_count and eft_encap_filter_count from
> ef10_filter_table_t. But these fields are specific to ef10 filters. But
> efx_port_vlan_strip_set is a generic function, and we cannot use fields
> specific to ef10 inside it, since something else may appear in the
> future. Maybe we need a new API to find out the number of filters
> installed? (emo_filter_count or something like that).
Of course, I'm talking about solution in principle. Yes, it is required
to wrap it in a right code.
>>
>>>
>>>>
>>>>> 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)
>>>>
>>
More information about the dev
mailing list