> -----Original Message-----
> From: Maxime Coquelin <maxime.coque...@redhat.com>
> Sent: Wednesday, June 5, 2019 9:28 AM
> To: Stillwell Jr, Paul M <paul.m.stillwell...@intel.com>; Rong, Leyi
> <leyi.r...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>
> Cc: dev@dpdk.org; Raj, Victor <victor....@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 07/49] net/ice/base: replay advanced rule
> after reset
>
>
>
> On 6/5/19 6:16 PM, Stillwell Jr, Paul M wrote:
> >> -----Original Message-----
> >> From: Maxime Coquelin <maxime.coque...@redhat.com>
> >> Sent: Wednesday, June 5, 2019 8:59 AM
> >> To: Stillwell Jr, Paul M <paul.m.stillwell...@intel.com>; Rong, Leyi
> >> <leyi.r...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>
> >> Cc: dev@dpdk.org; Raj, Victor <victor....@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).
>
Just to further clarify, when the patch goes upstream in Linux the
LIST_FOR_EACH_ENTRY_SAFE and all the other LIST_* macros in the code will get
replaced with the corresponding list_* that is in the linux kernel. We have a
coccinelle entry to replace these in Linux.
> Let's drop my comments about LIST_EMPTY then.
>
> Maxime
