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).
Let's drop my comments about LIST_EMPTY then.
Maxime
