idpf: fix memory leak of flow steer list on rmmod

The flow steering list maintains entries that are added and removed as
ethtool creates and deletes flow steering rules. Module removal with active
entries causes memory leak as the list is not properly cleaned up.

Prevent this by iterating through the remaining entries in the list and
freeing the associated memory during module removal. Add a spinlock
(flow_steer_list_lock) to protect the list access from multiple threads.

Fixes: ada3e24b84a0 ("idpf: add flow steering support")
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Tested-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>

diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
index 8cfc68cbfa06d278ebb7c5d285c5dd340921aa49..a61821333f5df363ab47144686d437719fde27c1 100644 (file)
--- a/drivers/net/ethernet/intel/idpf/idpf.h
+++ b/drivers/net/ethernet/intel/idpf/idpf.h
@@ -558,6 +558,7 @@ struct idpf_vector_lifo {
  * @max_q: Maximum possible queues
  * @req_qs_chunks: Queue chunk data for requested queues
  * @mac_filter_list_lock: Lock to protect mac filters
+ * @flow_steer_list_lock: Lock to protect fsteer filters
  * @flags: See enum idpf_vport_config_flags
  */
 struct idpf_vport_config {
@@ -565,6 +566,7 @@ struct idpf_vport_config {
        struct idpf_vport_max_q max_q;
        struct virtchnl2_add_queues *req_qs_chunks;
        spinlock_t mac_filter_list_lock;
+       spinlock_t flow_steer_list_lock;
        DECLARE_BITMAP(flags, IDPF_VPORT_CONFIG_FLAGS_NBITS);
 };
 

diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
index 2589e124e41cdc7f335e5706c7cadd6933ed1308..00481fec817935e0f18f6cd405b7d6395403b219 100644 (file)
--- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
@@ -37,6 +37,7 @@ static int idpf_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd,
 {
        struct idpf_netdev_priv *np = netdev_priv(netdev);
        struct idpf_vport_user_config_data *user_config;
+       struct idpf_vport_config *vport_config;
        struct idpf_fsteer_fltr *f;
        struct idpf_vport *vport;
        unsigned int cnt = 0;
@@ -44,7 +45,8 @@ static int idpf_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd,
 
        idpf_vport_ctrl_lock(netdev);
        vport = idpf_netdev_to_vport(netdev);
-       user_config = &np->adapter->vport_config[np->vport_idx]->user_config;
+       vport_config = np->adapter->vport_config[np->vport_idx];
+       user_config = &vport_config->user_config;
 
        switch (cmd->cmd) {
        case ETHTOOL_GRXCLSRLCNT:
@@ -53,15 +55,18 @@ static int idpf_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd,
                break;
        case ETHTOOL_GRXCLSRULE:
                err = -EINVAL;
+               spin_lock_bh(&vport_config->flow_steer_list_lock);
                list_for_each_entry(f, &user_config->flow_steer_list, list)
                        if (f->loc == cmd->fs.location) {
                                cmd->fs.ring_cookie = f->q_index;
                                err = 0;
                                break;
                        }
+               spin_unlock_bh(&vport_config->flow_steer_list_lock);
                break;
        case ETHTOOL_GRXCLSRLALL:
                cmd->data = idpf_fsteer_max_rules(vport);
+               spin_lock_bh(&vport_config->flow_steer_list_lock);
                list_for_each_entry(f, &user_config->flow_steer_list, list) {
                        if (cnt == cmd->rule_cnt) {
                                err = -EMSGSIZE;
@@ -72,6 +77,7 @@ static int idpf_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd,
                }
                if (!err)
                        cmd->rule_cnt = user_config->num_fsteer_fltrs;
+               spin_unlock_bh(&vport_config->flow_steer_list_lock);
                break;
        default:
                break;
@@ -240,6 +246,7 @@ static int idpf_add_flow_steer(struct net_device *netdev,
 
        fltr->loc = fsp->location;
        fltr->q_index = q_index;
+       spin_lock_bh(&vport_config->flow_steer_list_lock);
        list_for_each_entry(f, &user_config->flow_steer_list, list) {
                if (f->loc >= fltr->loc)
                        break;
@@ -250,6 +257,7 @@ static int idpf_add_flow_steer(struct net_device *netdev,
                 list_add(&fltr->list, &user_config->flow_steer_list);
 
        user_config->num_fsteer_fltrs++;
+       spin_unlock_bh(&vport_config->flow_steer_list_lock);
 
 out:
        kfree(rule);
@@ -302,17 +310,20 @@ static int idpf_del_flow_steer(struct net_device *netdev,
                goto out;
        }
 
+       spin_lock_bh(&vport_config->flow_steer_list_lock);
        list_for_each_entry_safe(f, iter,
                                 &user_config->flow_steer_list, list) {
                if (f->loc == fsp->location) {
                        list_del(&f->list);
                        kfree(f);
                        user_config->num_fsteer_fltrs--;
-                       goto out;
+                       goto out_unlock;
                }
        }
        err = -EINVAL;
 
+out_unlock:
+       spin_unlock_bh(&vport_config->flow_steer_list_lock);
 out:
        kfree(rule);
        return err;

diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index e2ee8b137421f2a542725e411bf9e51ddf0447ef..d56366e676cf74d6ac615cb89ab92556880af372 100644 (file)
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -442,6 +442,29 @@ send_dealloc_vecs:
        return err;
 }
 
+/**
+ * idpf_del_all_flow_steer_filters - Delete all flow steer filters in list
+ * @vport: main vport struct
+ *
+ * Takes flow_steer_list_lock spinlock.  Deletes all filters
+ */
+static void idpf_del_all_flow_steer_filters(struct idpf_vport *vport)
+{
+       struct idpf_vport_config *vport_config;
+       struct idpf_fsteer_fltr *f, *ftmp;
+
+       vport_config = vport->adapter->vport_config[vport->idx];
+
+       spin_lock_bh(&vport_config->flow_steer_list_lock);
+       list_for_each_entry_safe(f, ftmp, &vport_config->user_config.flow_steer_list,
+                                list) {
+               list_del(&f->list);
+               kfree(f);
+       }
+       vport_config->user_config.num_fsteer_fltrs = 0;
+       spin_unlock_bh(&vport_config->flow_steer_list_lock);
+}
+
 /**
  * idpf_find_mac_filter - Search filter list for specific mac filter
  * @vconfig: Vport config structure
@@ -1107,8 +1130,10 @@ static void idpf_vport_dealloc(struct idpf_vport *vport)
                idpf_vport_stop(vport, true);
                idpf_decfg_netdev(vport);
        }
-       if (test_bit(IDPF_REMOVE_IN_PROG, adapter->flags))
+       if (test_bit(IDPF_REMOVE_IN_PROG, adapter->flags)) {
                idpf_del_all_mac_filters(vport);
+               idpf_del_all_flow_steer_filters(vport);
+       }
 
        if (adapter->netdevs[i]) {
                struct idpf_netdev_priv *np = netdev_priv(adapter->netdevs[i]);
@@ -1648,6 +1673,7 @@ void idpf_init_task(struct work_struct *work)
        vport_config = adapter->vport_config[index];
 
        spin_lock_init(&vport_config->mac_filter_list_lock);
+       spin_lock_init(&vport_config->flow_steer_list_lock);
 
        INIT_LIST_HEAD(&vport_config->user_config.mac_filter_list);
        INIT_LIST_HEAD(&vport_config->user_config.flow_steer_list);