| Message ID | 20200421014932.2743607-5-jeffrey.t.kirsher@intel.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | 40GbE Intel Wired LAN Driver Updates 2020-04-20 | expand |
On Mon, 20 Apr 2020 18:49:32 -0700 Jeff Kirsher wrote: > From: Todd Fujinaka <todd.fujinaka@intel.com> > > A customer was chain-booting to provision his systems and one of the > steps was setting MFS. MFS isn't cleared by normal warm reboots > (clearing requires a GLOBR) and there was no indication of why Jumbo > Frame receives were failing. > > Add a warning if MFS is set to anything lower than the default. > > Signed-off-by: Todd Fujinaka <todd.fujinaka@intel.com> > Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> > Tested-by: Andrew Bowers <andrewx.bowers@intel.com> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> > --- > drivers/net/ethernet/intel/i40e/i40e_main.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c > index 4c414208a22a..3fdbfede0b87 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c > @@ -15347,6 +15347,15 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > i40e_stat_str(&pf->hw, err), > i40e_aq_str(&pf->hw, pf->hw.aq.asq_last_status)); > > + /* make sure the MFS hasn't been set lower than the default */ > +#define MAX_FRAME_SIZE_DEFAULT 0x2600 > + for (i = 0; i < 4; i++) { Why is this a loop? AFAICS @i is only used in the warning message > + val = ((rd32(&pf->hw, I40E_PRTGL_SAH) & I40E_PRTGL_SAH_MFS_MASK) > + >> I40E_PRTGL_SAH_MFS_SHIFT); outer parens unnecessary > + if (val < MAX_FRAME_SIZE_DEFAULT) > + dev_warn(&pdev->dev, "MFS for port %x has been set below the default: %x\n", i, val); Shouldn't you just reset it to default at this point? If the value is not reset on warm boot this is not really a surprise. > + } > + > /* Add a filter to drop all Flow control frames from any VSI from being > * transmitted. By doing so we stop a malicious VF from sending out > * PAUSE or PFC frames and potentially controlling traffic for other
Sorry for the top-posting. Outlook. I can't seem to find this email on my non-Windows machine. I don't know why we don't reset the NIC, but I was told there is a reason. And the reason there's an index is because the datasheet shows four registers. Somehow while I was formatting that long line I lost the math bits that read four different registers. Let me see if I can fix this patch. Todd Fujinaka Software Application Engineer Data Center Group Intel Corporation todd.fujinaka@intel.com -----Original Message----- From: Jakub Kicinski <kuba@kernel.org> Sent: Tuesday, April 21, 2020 11:00 AM To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com> Cc: davem@davemloft.net; Fujinaka, Todd <todd.fujinaka@intel.com>; netdev@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com; Brandeburg, Jesse <jesse.brandeburg@intel.com>; Bowers, AndrewX <andrewx.bowers@intel.com> Subject: Re: [net-next 4/4] i40e: Add a check to see if MFS is set On Mon, 20 Apr 2020 18:49:32 -0700 Jeff Kirsher wrote: > From: Todd Fujinaka <todd.fujinaka@intel.com> > > A customer was chain-booting to provision his systems and one of the > steps was setting MFS. MFS isn't cleared by normal warm reboots > (clearing requires a GLOBR) and there was no indication of why Jumbo > Frame receives were failing. > > Add a warning if MFS is set to anything lower than the default. > > Signed-off-by: Todd Fujinaka <todd.fujinaka@intel.com> > Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> > Tested-by: Andrew Bowers <andrewx.bowers@intel.com> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> > --- > drivers/net/ethernet/intel/i40e/i40e_main.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c > b/drivers/net/ethernet/intel/i40e/i40e_main.c > index 4c414208a22a..3fdbfede0b87 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c > @@ -15347,6 +15347,15 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > i40e_stat_str(&pf->hw, err), > i40e_aq_str(&pf->hw, pf->hw.aq.asq_last_status)); > > + /* make sure the MFS hasn't been set lower than the default */ > +#define MAX_FRAME_SIZE_DEFAULT 0x2600 > + for (i = 0; i < 4; i++) { Why is this a loop? AFAICS @i is only used in the warning message > + val = ((rd32(&pf->hw, I40E_PRTGL_SAH) & I40E_PRTGL_SAH_MFS_MASK) > + >> I40E_PRTGL_SAH_MFS_SHIFT); outer parens unnecessary > + if (val < MAX_FRAME_SIZE_DEFAULT) > + dev_warn(&pdev->dev, "MFS for port %x has been set below the > +default: %x\n", i, val); Shouldn't you just reset it to default at this point? If the value is not reset on warm boot this is not really a surprise. > + } > + > /* Add a filter to drop all Flow control frames from any VSI from being > * transmitted. By doing so we stop a malicious VF from sending out > * PAUSE or PFC frames and potentially controlling traffic for other
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 4c414208a22a..3fdbfede0b87 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -15347,6 +15347,15 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent) i40e_stat_str(&pf->hw, err), i40e_aq_str(&pf->hw, pf->hw.aq.asq_last_status)); + /* make sure the MFS hasn't been set lower than the default */ +#define MAX_FRAME_SIZE_DEFAULT 0x2600 + for (i = 0; i < 4; i++) { + val = ((rd32(&pf->hw, I40E_PRTGL_SAH) & I40E_PRTGL_SAH_MFS_MASK) + >> I40E_PRTGL_SAH_MFS_SHIFT); + if (val < MAX_FRAME_SIZE_DEFAULT) + dev_warn(&pdev->dev, "MFS for port %x has been set below the default: %x\n", i, val); + } + /* Add a filter to drop all Flow control frames from any VSI from being * transmitted. By doing so we stop a malicious VF from sending out * PAUSE or PFC frames and potentially controlling traffic for other