Re: [Intel-wired-lan] [PATCH 5/6] [net-next]net: i40e: Clean up of cloud filters

[Nambiar, Amritha]

On 8/1/2017 12:16 PM, Shannon Nelson wrote:
> On 7/31/2017 5:38 PM, Amritha Nambiar wrote:
>> Introduce the cloud filter datastructure and cleanup of cloud
>> filters associated with the device.
>>
>> Signed-off-by: Amritha Nambiar 
>> ---
>>   drivers/net/ethernet/intel/i40e/i40e.h  |   11 +++
>>   drivers/net/ethernet/intel/i40e/i40e_main.c |   27 
>> +++
>>   2 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h 
>> b/drivers/net/ethernet/intel/i40e/i40e.h
>> index 1391e5d..5c0cad5 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
>> @@ -252,6 +252,14 @@ struct i40e_fdir_filter {
>>  u32 fd_id;
>>   };
>>   
>> +struct i40e_cloud_filter {
>> +struct hlist_node cloud_node;
>> +/* cloud filter input set follows */
>> +unsigned long cookie;
>> +/* filter control */
>> +u16 seid;
>> +};
> 
> This would be cleaner and more readable with the field comments off to 
> the side rather than in line with the fields.

Will fix in the next version of the series.

> 
>> +
>>   #define I40E_ETH_P_LLDP0x88cc
>>   
>>   #define I40E_DCB_PRIO_TYPE_STRICT  0
>> @@ -419,6 +427,9 @@ struct i40e_pf {
>>  struct i40e_udp_port_config udp_ports[I40E_MAX_PF_UDP_OFFLOAD_PORTS];
>>  u16 pending_udp_bitmap;
>>   
>> +struct hlist_head cloud_filter_list;
>> +u16 num_cloud_filters;
>> +
>>  enum i40e_interrupt_policy int_policy;
>>  u16 rx_itr_default;
>>  u16 tx_itr_default;
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c 
>> b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> index f74..93f6fe2 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> @@ -6928,6 +6928,29 @@ static void i40e_fdir_filter_exit(struct i40e_pf *pf)
>>   }
>>   
>>   /**
>> + * i40e_cloud_filter_exit - Cleans up the Cloud Filters
>> + * @pf: Pointer to PF
>> + *
>> + * This function destroys the hlist where all the Cloud Filters
>> + * filters were saved.
>> + **/
>> +static void i40e_cloud_filter_exit(struct i40e_pf *pf)
>> +{
>> +struct i40e_cloud_filter *cfilter;
>> +struct hlist_node *node;
>> +
>> +if (hlist_empty(>cloud_filter_list))
>> +return;
> 
> Is this check really necessary?  Doesn't hlist_for_each_entry_safe() 
> check for this?

That's right. Will fix in the next version of the series.

> 
>> +
>> +hlist_for_each_entry_safe(cfilter, node,
>> +  >cloud_filter_list, cloud_node) {
>> +hlist_del(>cloud_node);
>> +kfree(cfilter);
>> +}
>> +pf->num_cloud_filters = 0;
>> +}
>> +
>> +/**
>>* i40e_close - Disables a network interface
>>* @netdev: network interface device structure
>>*
>> @@ -12137,6 +12160,7 @@ static int i40e_setup_pf_switch(struct i40e_pf *pf, 
>> bool reinit)
>>  vsi = i40e_vsi_reinit_setup(pf->vsi[pf->lan_vsi]);
>>  if (!vsi) {
>>  dev_info(>pdev->dev, "setup of MAIN VSI failed\n");
>> +i40e_cloud_filter_exit(pf);
>>  i40e_fdir_teardown(pf);
>>  return -EAGAIN;
>>  }
>> @@ -12961,6 +12985,8 @@ static void i40e_remove(struct pci_dev *pdev)
>>  if (pf->vsi[pf->lan_vsi])
>>  i40e_vsi_release(pf->vsi[pf->lan_vsi]);
>>   
>> +i40e_cloud_filter_exit(pf);
>> +
>>  /* remove attached clients */
>>  if (pf->flags & I40E_FLAG_IWARP_ENABLED) {
>>  ret_code = i40e_lan_del_device(pf);
>> @@ -13170,6 +13196,7 @@ static void i40e_shutdown(struct pci_dev *pdev)
>>   
>>  del_timer_sync(>service_timer);
>>  cancel_work_sync(>service_task);
>> +i40e_cloud_filter_exit(pf);
>>  i40e_fdir_teardown(pf);
>>   
>>  /* Client close must be called explicitly here because the timer
>>
>> ___
>> Intel-wired-lan mailing list
>> intel-wired-...@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
>>

Re: [Intel-wired-lan] [PATCH 5/6] [net-next]net: i40e: Clean up of cloud filters

[Shannon Nelson]

On 7/31/2017 5:38 PM, Amritha Nambiar wrote:

Introduce the cloud filter datastructure and cleanup of cloud
filters associated with the device.

Signed-off-by: Amritha Nambiar 
---
  drivers/net/ethernet/intel/i40e/i40e.h  |   11 +++
  drivers/net/ethernet/intel/i40e/i40e_main.c |   27 +++
  2 files changed, 38 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h 
b/drivers/net/ethernet/intel/i40e/i40e.h
index 1391e5d..5c0cad5 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -252,6 +252,14 @@ struct i40e_fdir_filter {
u32 fd_id;
  };
  
+struct i40e_cloud_filter {

+   struct hlist_node cloud_node;
+   /* cloud filter input set follows */
+   unsigned long cookie;
+   /* filter control */
+   u16 seid;
+};


This would be cleaner and more readable with the field comments off to 
the side rather than in line with the fields.



+
  #define I40E_ETH_P_LLDP   0x88cc
  
  #define I40E_DCB_PRIO_TYPE_STRICT	0

@@ -419,6 +427,9 @@ struct i40e_pf {
struct i40e_udp_port_config udp_ports[I40E_MAX_PF_UDP_OFFLOAD_PORTS];
u16 pending_udp_bitmap;
  
+	struct hlist_head cloud_filter_list;

+   u16 num_cloud_filters;
+
enum i40e_interrupt_policy int_policy;
u16 rx_itr_default;
u16 tx_itr_default;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c 
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index f74..93f6fe2 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6928,6 +6928,29 @@ static void i40e_fdir_filter_exit(struct i40e_pf *pf)
  }
  
  /**

+ * i40e_cloud_filter_exit - Cleans up the Cloud Filters
+ * @pf: Pointer to PF
+ *
+ * This function destroys the hlist where all the Cloud Filters
+ * filters were saved.
+ **/
+static void i40e_cloud_filter_exit(struct i40e_pf *pf)
+{
+   struct i40e_cloud_filter *cfilter;
+   struct hlist_node *node;
+
+   if (hlist_empty(>cloud_filter_list))
+   return;


Is this check really necessary?  Doesn't hlist_for_each_entry_safe() 
check for this?



+
+   hlist_for_each_entry_safe(cfilter, node,
+ >cloud_filter_list, cloud_node) {
+   hlist_del(>cloud_node);
+   kfree(cfilter);
+   }
+   pf->num_cloud_filters = 0;
+}
+
+/**
   * i40e_close - Disables a network interface
   * @netdev: network interface device structure
   *
@@ -12137,6 +12160,7 @@ static int i40e_setup_pf_switch(struct i40e_pf *pf, 
bool reinit)
vsi = i40e_vsi_reinit_setup(pf->vsi[pf->lan_vsi]);
if (!vsi) {
dev_info(>pdev->dev, "setup of MAIN VSI failed\n");
+   i40e_cloud_filter_exit(pf);
i40e_fdir_teardown(pf);
return -EAGAIN;
}
@@ -12961,6 +12985,8 @@ static void i40e_remove(struct pci_dev *pdev)
if (pf->vsi[pf->lan_vsi])
i40e_vsi_release(pf->vsi[pf->lan_vsi]);
  
+	i40e_cloud_filter_exit(pf);

+
/* remove attached clients */
if (pf->flags & I40E_FLAG_IWARP_ENABLED) {
ret_code = i40e_lan_del_device(pf);
@@ -13170,6 +13196,7 @@ static void i40e_shutdown(struct pci_dev *pdev)
  
  	del_timer_sync(>service_timer);

cancel_work_sync(>service_task);
+   i40e_cloud_filter_exit(pf);
i40e_fdir_teardown(pf);
  
  	/* Client close must be called explicitly here because the timer


___
Intel-wired-lan mailing list
intel-wired-...@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan