[PATCH v5 2/3] common/sfc_efx/base: add support to enable VLAN stripping
Artemii Morozov
Artemii.Morozov at arknetworks.am
Tue Jun 20 11:55:19 CEST 2023
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
> 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.
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.
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?
>
>> 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