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

["Stillwell Jr, Paul M" <paul.m.stillwell.jr@intel.com>]

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, June 5, 2019 8:59 AM
> To: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>; Rong, Leyi
> <leyi.rong@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Raj, Victor <victor.raj@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.