]> git.baikalelectronics.ru Git - kernel.git/commitdiff
i40e: stop disabling VFs due to PF error responses
authorJacob Keller <jacob.e.keller@intel.com>
Thu, 17 Feb 2022 00:51:35 +0000 (16:51 -0800)
committerTony Nguyen <anthony.l.nguyen@intel.com>
Tue, 8 Mar 2022 21:31:08 +0000 (13:31 -0800)
The i40e_vc_send_msg_to_vf_ex (and its wrapper i40e_vc_send_msg_to_vf)
function has logic to detect "failure" responses sent to the VF. If a VF
is sent more than I40E_DEFAULT_NUM_INVALID_MSGS_ALLOWED, then the VF is
marked as disabled. In either case, a dev_info message is printed
stating that a VF opcode failed.

This logic originates from the early implementation of VF support in
commit eb89ebf92394 ("i40e: implement virtual device interface").

That commit did not go far enough. The "logic" for this behavior seems
to be that error responses somehow indicate a malicious VF. This is not
really true. The PF might be sending an error for any number of reasons
such as lacking resources, an unsupported operation, etc. This does not
indicate a malicious VF. We already have a separate robust malicious VF
detection which relies on hardware logic to detect and prevent a variety
of behaviors.

There is no justification for this behavior in the original
implementation. In fact, a later commit 7e23a86ac5ca ("i40e: Lower some
message levels") reduced the opcode failure message from a dev_err to a
dev_info. In addition, recent commit 6ca71f777c8b ("i40e: Fix to not
show opcode msg on unsuccessful VF MAC change") changed the logic to
allow quieting it for expected failures.

That commit prevented this logic from kicking in for specific
circumstances. This change did not go far enough. The behavior is not
documented nor is it part of any requirement for our products. Other
operating systems such as the FreeBSD implementation of our driver do
not include this logic.

It is clear this check does not make sense, and causes problems which
led to ugly workarounds.

Fix this by just removing the entire logic and the need for the
i40e_vc_send_msg_to_vf_ex function.

Fixes: 6ca71f777c8b ("i40e: Fix to not show opcode msg on unsuccessful VF MAC change")
Fixes: eb89ebf92394 ("i40e: implement virtual device interface")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Konrad Jankowski <konrad0.jankowski@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
drivers/net/ethernet/intel/i40e/i40e_debugfs.c
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h

index 1e57cc8c47d7bbd7ee15e632c3b6ee097f18e191..9db5001297c7ec641dc00b5989424bb1941affab 100644 (file)
@@ -742,10 +742,8 @@ static void i40e_dbg_dump_vf(struct i40e_pf *pf, int vf_id)
                vsi = pf->vsi[vf->lan_vsi_idx];
                dev_info(&pf->pdev->dev, "vf %2d: VSI id=%d, seid=%d, qps=%d\n",
                         vf_id, vf->lan_vsi_id, vsi->seid, vf->num_queue_pairs);
-               dev_info(&pf->pdev->dev, "       num MDD=%lld, invalid msg=%lld, valid msg=%lld\n",
-                        vf->num_mdd_events,
-                        vf->num_invalid_msgs,
-                        vf->num_valid_msgs);
+               dev_info(&pf->pdev->dev, "       num MDD=%lld\n",
+                        vf->num_mdd_events);
        } else {
                dev_info(&pf->pdev->dev, "invalid VF id %d\n", vf_id);
        }
index dfdb6e786461a5d5eaf153f56a98b5a37aa8c27c..2606e8f0f19be4764d48ec0cbe28850e3735d5d3 100644 (file)
@@ -1917,19 +1917,17 @@ sriov_configure_out:
 /***********************virtual channel routines******************/
 
 /**
- * i40e_vc_send_msg_to_vf_ex
+ * i40e_vc_send_msg_to_vf
  * @vf: pointer to the VF info
  * @v_opcode: virtual channel opcode
  * @v_retval: virtual channel return value
  * @msg: pointer to the msg buffer
  * @msglen: msg length
- * @is_quiet: true for not printing unsuccessful return values, false otherwise
  *
  * send msg to VF
  **/
-static int i40e_vc_send_msg_to_vf_ex(struct i40e_vf *vf, u32 v_opcode,
-                                    u32 v_retval, u8 *msg, u16 msglen,
-                                    bool is_quiet)
+static int i40e_vc_send_msg_to_vf(struct i40e_vf *vf, u32 v_opcode,
+                                 u32 v_retval, u8 *msg, u16 msglen)
 {
        struct i40e_pf *pf;
        struct i40e_hw *hw;
@@ -1944,25 +1942,6 @@ static int i40e_vc_send_msg_to_vf_ex(struct i40e_vf *vf, u32 v_opcode,
        hw = &pf->hw;
        abs_vf_id = vf->vf_id + hw->func_caps.vf_base_id;
 
-       /* single place to detect unsuccessful return values */
-       if (v_retval && !is_quiet) {
-               vf->num_invalid_msgs++;
-               dev_info(&pf->pdev->dev, "VF %d failed opcode %d, retval: %d\n",
-                        vf->vf_id, v_opcode, v_retval);
-               if (vf->num_invalid_msgs >
-                   I40E_DEFAULT_NUM_INVALID_MSGS_ALLOWED) {
-                       dev_err(&pf->pdev->dev,
-                               "Number of invalid messages exceeded for VF %d\n",
-                               vf->vf_id);
-                       dev_err(&pf->pdev->dev, "Use PF Control I/F to enable the VF\n");
-                       set_bit(I40E_VF_STATE_DISABLED, &vf->vf_states);
-               }
-       } else {
-               vf->num_valid_msgs++;
-               /* reset the invalid counter, if a valid message is received. */
-               vf->num_invalid_msgs = 0;
-       }
-
        aq_ret = i40e_aq_send_msg_to_vf(hw, abs_vf_id,  v_opcode, v_retval,
                                        msg, msglen, NULL);
        if (aq_ret) {
@@ -1975,23 +1954,6 @@ static int i40e_vc_send_msg_to_vf_ex(struct i40e_vf *vf, u32 v_opcode,
        return 0;
 }
 
-/**
- * i40e_vc_send_msg_to_vf
- * @vf: pointer to the VF info
- * @v_opcode: virtual channel opcode
- * @v_retval: virtual channel return value
- * @msg: pointer to the msg buffer
- * @msglen: msg length
- *
- * send msg to VF
- **/
-static int i40e_vc_send_msg_to_vf(struct i40e_vf *vf, u32 v_opcode,
-                                 u32 v_retval, u8 *msg, u16 msglen)
-{
-       return i40e_vc_send_msg_to_vf_ex(vf, v_opcode, v_retval,
-                                        msg, msglen, false);
-}
-
 /**
  * i40e_vc_send_resp_to_vf
  * @vf: pointer to the VF info
@@ -2822,7 +2784,6 @@ error_param:
  * i40e_check_vf_permission
  * @vf: pointer to the VF info
  * @al: MAC address list from virtchnl
- * @is_quiet: set true for printing msg without opcode info, false otherwise
  *
  * Check that the given list of MAC addresses is allowed. Will return -EPERM
  * if any address in the list is not valid. Checks the following conditions:
@@ -2837,8 +2798,7 @@ error_param:
  * addresses might not be accurate.
  **/
 static inline int i40e_check_vf_permission(struct i40e_vf *vf,
-                                          struct virtchnl_ether_addr_list *al,
-                                          bool *is_quiet)
+                                          struct virtchnl_ether_addr_list *al)
 {
        struct i40e_pf *pf = vf->pf;
        struct i40e_vsi *vsi = pf->vsi[vf->lan_vsi_idx];
@@ -2846,7 +2806,6 @@ static inline int i40e_check_vf_permission(struct i40e_vf *vf,
        int mac2add_cnt = 0;
        int i;
 
-       *is_quiet = false;
        for (i = 0; i < al->num_elements; i++) {
                struct i40e_mac_filter *f;
                u8 *addr = al->list[i].addr;
@@ -2870,7 +2829,6 @@ static inline int i40e_check_vf_permission(struct i40e_vf *vf,
                    !ether_addr_equal(addr, vf->default_lan_addr.addr)) {
                        dev_err(&pf->pdev->dev,
                                "VF attempting to override administratively set MAC address, bring down and up the VF interface to resume normal operation\n");
-                       *is_quiet = true;
                        return -EPERM;
                }
 
@@ -2921,7 +2879,6 @@ static int i40e_vc_add_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
            (struct virtchnl_ether_addr_list *)msg;
        struct i40e_pf *pf = vf->pf;
        struct i40e_vsi *vsi = NULL;
-       bool is_quiet = false;
        i40e_status ret = 0;
        int i;
 
@@ -2938,7 +2895,7 @@ static int i40e_vc_add_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
         */
        spin_lock_bh(&vsi->mac_filter_hash_lock);
 
-       ret = i40e_check_vf_permission(vf, al, &is_quiet);
+       ret = i40e_check_vf_permission(vf, al);
        if (ret) {
                spin_unlock_bh(&vsi->mac_filter_hash_lock);
                goto error_param;
@@ -2976,8 +2933,8 @@ static int i40e_vc_add_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
 
 error_param:
        /* send the response to the VF */
-       return i40e_vc_send_msg_to_vf_ex(vf, VIRTCHNL_OP_ADD_ETH_ADDR,
-                                      ret, NULL, 0, is_quiet);
+       return i40e_vc_send_msg_to_vf(vf, VIRTCHNL_OP_ADD_ETH_ADDR,
+                                     ret, NULL, 0);
 }
 
 /**
index 03c42fd0fea19326916d302d67a7585456dc0a7c..a554d0a0b09bd56fb9904defd0a390df877cfa89 100644 (file)
@@ -10,8 +10,6 @@
 
 #define I40E_VIRTCHNL_SUPPORTED_QTYPES 2
 
-#define I40E_DEFAULT_NUM_INVALID_MSGS_ALLOWED  10
-
 #define I40E_VLAN_PRIORITY_SHIFT       13
 #define I40E_VLAN_MASK                 0xFFF
 #define I40E_PRIORITY_MASK             0xE000
@@ -92,9 +90,6 @@ struct i40e_vf {
        u8 num_queue_pairs;     /* num of qps assigned to VF vsis */
        u8 num_req_queues;      /* num of requested qps */
        u64 num_mdd_events;     /* num of mdd events detected */
-       /* num of continuous malformed or invalid msgs detected */
-       u64 num_invalid_msgs;
-       u64 num_valid_msgs;     /* num of valid msgs detected */
 
        unsigned long vf_caps;  /* vf's adv. capabilities */
        unsigned long vf_states;        /* vf's runtime states */