[dpdk-dev] [PATCH 07/49] net/ice/base: replay advanced rule after reset

Maxime Coquelin maxime.coquelin at redhat.com
Wed Jun 5 18:28:10 CEST 2019



On 6/5/19 6:16 PM, Stillwell Jr, Paul M wrote:
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin at redhat.com>
>> Sent: Wednesday, June 5, 2019 8:59 AM
>> To: Stillwell Jr, Paul M <paul.m.stillwell.jr at intel.com>; Rong, Leyi
>> <leyi.rong at intel.com>; Zhang, Qi Z <qi.z.zhang at intel.com>
>> Cc: dev at dpdk.org; Raj, Victor <victor.raj at intel.com>
>> Subject: Re: [dpdk-dev] [PATCH 07/49] net/ice/base: replay advanced rule
>> after reset
>>
>>
>>
>> On 6/5/19 5:53 PM, Stillwell Jr, Paul M wrote:
>>>>> diff --git a/drivers/net/ice/base/ice_switch.c
>>>>> b/drivers/net/ice/base/ice_switch.c
>>>>> index c53021aed..ca0497ca7 100644
>>>>> --- a/drivers/net/ice/base/ice_switch.c
>>>>> +++ b/drivers/net/ice/base/ice_switch.c
>>>>> @@ -3033,6 +3033,27 @@ ice_rem_sw_rule_info(struct ice_hw *hw,
>>>> struct LIST_HEAD_TYPE *rule_head)
>>>>>     	}
>>>>>     }
>>>>>
>>>>> +/**
>>>>> + * ice_rem_adv_rule_info
>>>>> + * @hw: pointer to the hardware structure
>>>>> + * @rule_head: pointer to the switch list structure that we want to
>>>>> +delete  */ static void ice_rem_adv_rule_info(struct ice_hw *hw,
>>>>> +struct LIST_HEAD_TYPE *rule_head) {
>>>>> +	struct ice_adv_fltr_mgmt_list_entry *tmp_entry;
>>>>> +	struct ice_adv_fltr_mgmt_list_entry *lst_itr;
>>>>> +
>>>>> +	if (LIST_EMPTY(rule_head))
>>>>> +		return;
>>>>
>>>> Is it necessary? If the list is empty, LIST_FOR_EACH_ENTRY will not
>>>> loop and status will be returned:
>>>>
>>> Yes, the check is necessary. This code gets consumed by multiple different
>> OSs and not all OSs implement the LIST_FOR_EACH_ENTRY_SAFE in the way
>> that DPDK did. For example, if I'm understanding the Linux code correctly,
>> the list_for_each_entry_safe code in Linux would not work correctly without
>> checking LIST_EMPTY since the Linux implementation doesn't have a check
>> for null in it's implementation of list_for_each_entry_safe.
>>
>> Do you mean the same patch is upstreamed into Linux Kernel without any
>> adaptations?
> 
> The same patch is planned to be upstreamed in the Linux kernel without any adaptations. Like I said, for Linux you have to check for LIST_EMPTY since the implementation of list_for_each_entry_safe doesn't check for NULL.
> 

OK, thanks for the clarification.
That's a surprise to me that OS abstraction layers are now accepted
in upstream kernel (Like ice_acquire_lock for instance).

Let's drop my comments about LIST_EMPTY then.

Maxime


More information about the dev mailing list