Re: [Intel-wired-lan] [PATCH net-next 3/3] ice: Handle LLDP MIB Pending change

[Anatolii Gerasymenko <anatolii.gerasymenko@intel.com>]

On 01.08.2022 17:52, Tony Nguyen wrote:
> 
> 
> On 8/1/2022 1:06 AM, Anatolii Gerasymenko wrote:
>> On 29.07.2022 20:55, Tony Nguyen wrote:
>>>
>>>
>>> On 7/28/2022 7:04 AM, Anatolii Gerasymenko wrote:
>>>> If the number of Traffic Classes (TC) is decreased, the FW will no
>>>> longer remove TC nodes, but will send a pending change notification. This
>>>> will allow RDMA to destroy corresponding Control QP markers. After RDMA
>>>> finishes outstanding operations, the ice driver will send an execute MIB
>>>> Pending change admin queue command to FW to finish DCB configuration
>>>> change.
>>>>
>>>> The FW will buffer all incoming Pending changes, so there can be only
>>>> one active Pending change.
>>>>
>>>> RDMA driver guarantees to remove Control QP markers within 5000 ms.
>>>> Hence, LLDP response timeout txTTL (default 30 sec) will be met.
>>>>
>>>> Signed-off-by: Anatolii Gerasymenko <anatolii.gerasymenko@intel.com>
>>>> ---
>>>>    drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 25 ++++++++++++++++++--
>>>>    1 file changed, 23 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
>>>> index 1c54eb643b6f..f16e4c54409f 100644
>>>> --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
>>>> +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
>>>> @@ -933,6 +933,16 @@ ice_tx_prepare_vlan_flags_dcb(struct ice_tx_ring *tx_ring,
>>>>        }
>>>>    }
>>>>    +/**
>>>> + * ice_dcb_is_mib_change_pending - Check if MIB change is pending
>>>> + * @state: MIB change state
>>>> + */
>>>> +static inline bool ice_dcb_is_mib_change_pending(u8 state)
>>>
>>> No 'inline' in .c files
>>>
>>
>> Cannot find such rule in the coding style. Is this some kind of tacit agreement?
> 
> It's a requirement of netdev. If you search archive history, you can find plenty of comments on it (a couple of examples [1][2]). It's also something specifically checked for on netdev CI [3]; note the 'netdev/source_inline' check.
> 

Thank you for the info, Tony!

>> So, just remove inline hint and leave the function static? Or move it as static inline to *.h?
> 
> Just remove the inline hint and leave the function static.
> 

Fixed in v2.

>> The function is only used in this *.c file and doesn't need to be exported.
>>
>>>> +{
>>>> +    return ICE_AQ_LLDP_MIB_CHANGE_PENDING ==
>>>> +        FIELD_GET(ICE_AQ_LLDP_MIB_CHANGE_STATE_M, state);
>>>> +}
>>>> +
>>>
>>>
> 
> [1] https://lore.kernel.org/netdev/20200731.142538.868196979893920242.davem@davemloft.net/
> [2] https://lore.kernel.org/netdev/20201011122422.56beacaa@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com/
> [3] https://patchwork.kernel.org/project/netdevbpf/patch/20220517073259.23476-2-harini.katakam@xilinx.com/
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan