[Intel-wired-lan] [net PATCH 2/2] ice: stop disabling VFs due to PF error responses

[Keller, Jacob E]

> -----Original Message-----
> From: Nguyen, Anthony L <anthony.l.nguyen at intel.com>
> Sent: Thursday, February 17, 2022 9:25 AM
> To: Keller, Jacob E <jacob.e.keller at intel.com>; intel-wired-lan at lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [net PATCH 2/2] ice: stop disabling VFs due to PF
> error responses
> 
> On Wed, 2022-02-16 at 16:51 -0800, Jacob Keller wrote:
> > The ice_vc_send_msg_to_vf function has logic to detect "failure"
> > responses being sent to a VF. If a VF is sent more than
> > ICE_DFLT_NUM_INVAL_MSGS_ALLOWED then the VF is marked as disabled.
> > Almost identical logic also existed in the i40e driver.
> >
> > This logic was added to the ice driver in commit 1071a8358a28 ("ice:
> > Implement virtchnl commands for AVF support") which itself copied
> > from
> > the i40e implementation in commit 5c3c48ac6bf5 ("i40e: implement
> > virtual
> > device interface").
> >
> > Neither commit provides a proper explanation or justification of the
> > check. In fact, later commits to i40e changed the logic to allow
> > bypassing the check in some specific instances.
> >
> > The "logic" for this 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 lack of resources, etc.
> >
> > Additionally, this causes the PF to log an info message for every
> > failed
> > VF response which may confuse users, and can spam the kernel log.
> >
> > This behavior is not documented as part of any requirement for our
> > products and other operating system drivers such as the FreeBSD
> > implementation of our drivers do not include this type of check.
> >
> > In fact, the change from dev_err to dev_info in i40e commit
> > 18b7af57d9c1
> > ("i40e: Lower some message levels") explains that these messages
> > typically don't actually indicate a real issue. It is quite likely
> > that
> > a user who hits this in practice will be very confused as the VF will
> > be
> > disabled without an obvious way to recover.
> >
> > We already have robust malicious driver detection logic using actual
> > hardware detection mechanisms that detect and prevent invalid device
> > usage. Remove the logic since its not a documented requirement and
> > the
> > behavior is not intuitive.
> >
> > Signed-off-by: Jacob Keller <jacob.e.keller at intel.com>
> 
> If this is for net, it should have a Fixes: as well.
> 
> Thanks,
> Tony

Fixes: 1071a8358a28 ("ice: Implement virtchnl commands for AVF support")