RE: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-26 Thread Dev, Vasu
> -Original Message-
> From: ethan zhao [mailto:ethan.z...@oracle.com]
> Sent: Monday, January 19, 2015 6:06 PM
> To: Dev, Vasu
> Cc: Kirsher, Jeffrey T; Ethan Zhao; Ronciak, John; Brandeburg, Jesse; Allan,
> Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick,
> Matthew; Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
> de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; brian.m...@oracle.com
> Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
> reset
> 
> 
> On 2015/1/20 5:10, Dev, Vasu wrote:
> >> -Original Message-
> >> From: ethan zhao [mailto:ethan.z...@oracle.com]
> >> Sent: Friday, January 16, 2015 7:01 PM
> >> To: Kirsher, Jeffrey T
> >> Cc: Dev, Vasu; Ethan Zhao; Ronciak, John; Brandeburg, Jesse; Allan,
> >> Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick,
> >> Matthew; Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
> >> de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
> >> ker...@vger.kernel.org; brian.m...@oracle.com
> >> Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when
> >> do PF reset
> >>
> >> Vasu,
> >>
> >> What' your idea about the v2, any suggestion ?  Jeff is looking
> >> forward to see it.
> >>
> > Jeff was asking for v2 in response to your last comment as "disable FCOE as
> default configuration as a temporary step" but I think that is the fix and 
> user
> should n't enable FCoE until they have FCoE enabled X710 FCoE with either
> fabric or VN2VN mode FCoE setup.
>   As a Linux distro, we don't know users have FCoE capable X710 or not, so
> we couldn't disable the FCoE configuration
>   by default in the released kernel except FCoE is officially not supported 
> yet
> with X710, but if yes, fix those bugs I
>   mentioned is the only choice.
> 

Yes must be fixed but I don't see your VLAN issue in my XL710  setup having 
engineering FW 4.33 with FCoE enabled, I could create VLANs and bring them up 
or down. 

As for the patch under discussion, it won't help any way whether fcoe enabled 
or not as I explained before. So I guess my setup differs in XL710 FW version, 
so upgrading FW should fix the issue and then you could keep FCoE configuration 
enabled in your distro w/o any concern to XL710 FCoE capable or not in XL710. 
We can take any FW related further discussion off list.

Thanks,
Vasu

> 
> Thanks,
> Ethan
> 
> 
> >
> > Thanks,
> > Vasu
> >
> >> Thanks,
> >> Ethan
> >>
> >>
> >> On 2015/1/16 22:47, Jeff Kirsher wrote:
> >>> On Fri, 2015-01-16 at 09:48 +0800, ethan zhao wrote:
> >>>> Vasu,
> >>>>
> >>>>   OK, disable FCOE as default configuration as a temporary step
> >>>> to make it  work.
> >>> Sounds like I should expect a v2 coming, correct?
> >>>
> >>>> Thanks,
> >>>> Ethan
> >>>>
> >>>> On 2015/1/16 7:45, Dev, Vasu wrote:
> >>>>>> -Original Message-
> >>>>>> From: ethan zhao [mailto:ethan.z...@oracle.com]
> >>>>>> Sent: Tuesday, January 13, 2015 6:41 PM
> >>>>>> To: Dev, Vasu
> >>>>>> Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg,
> >>>>>> Jesse; Allan, Bruce W; Wyborny, Carolyn; Skidmore, Donald C;
> >>>>>> Rose, Gregory V; Vick, Matthew; Williams, Mitch A; Parikh,
> >>>>>> Neerav; Linux NICS; e1000- de...@lists.sourceforge.net;
> >>>>>> net...@vger.kernel.org;
> >>>>>> linux- ker...@vger.kernel.org; brian.m...@oracle.com
> >>>>>> Subject: Re: [PATCH] i40e: don't enable and init FCOE by default
> >>>>>> when do PF reset
> >>>>>>
> >>>>>> Vasu,
> >>>>>>
> >>>>>> On 2015/1/14 3:38, Dev, Vasu wrote:
> >>>>>>>> -Original Message-
> >>>>>>>>>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> >>>>>>>>>>> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> >>>>>>>>>>> index a5f2660..a2572cc 100644
> >>>>>>>>>>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> >>>>>>>>>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> >&

RE: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-26 Thread Dev, Vasu
 -Original Message-
 From: ethan zhao [mailto:ethan.z...@oracle.com]
 Sent: Monday, January 19, 2015 6:06 PM
 To: Dev, Vasu
 Cc: Kirsher, Jeffrey T; Ethan Zhao; Ronciak, John; Brandeburg, Jesse; Allan,
 Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick,
 Matthew; Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
 de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
 ker...@vger.kernel.org; brian.m...@oracle.com
 Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
 reset
 
 
 On 2015/1/20 5:10, Dev, Vasu wrote:
  -Original Message-
  From: ethan zhao [mailto:ethan.z...@oracle.com]
  Sent: Friday, January 16, 2015 7:01 PM
  To: Kirsher, Jeffrey T
  Cc: Dev, Vasu; Ethan Zhao; Ronciak, John; Brandeburg, Jesse; Allan,
  Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick,
  Matthew; Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
  de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
  ker...@vger.kernel.org; brian.m...@oracle.com
  Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when
  do PF reset
 
  Vasu,
 
  What' your idea about the v2, any suggestion ?  Jeff is looking
  forward to see it.
 
  Jeff was asking for v2 in response to your last comment as disable FCOE as
 default configuration as a temporary step but I think that is the fix and 
 user
 should n't enable FCoE until they have FCoE enabled X710 FCoE with either
 fabric or VN2VN mode FCoE setup.
   As a Linux distro, we don't know users have FCoE capable X710 or not, so
 we couldn't disable the FCoE configuration
   by default in the released kernel except FCoE is officially not supported 
 yet
 with X710, but if yes, fix those bugs I
   mentioned is the only choice.
 

Yes must be fixed but I don't see your VLAN issue in my XL710  setup having 
engineering FW 4.33 with FCoE enabled, I could create VLANs and bring them up 
or down. 

As for the patch under discussion, it won't help any way whether fcoe enabled 
or not as I explained before. So I guess my setup differs in XL710 FW version, 
so upgrading FW should fix the issue and then you could keep FCoE configuration 
enabled in your distro w/o any concern to XL710 FCoE capable or not in XL710. 
We can take any FW related further discussion off list.

Thanks,
Vasu

 
 Thanks,
 Ethan
 
 
 
  Thanks,
  Vasu
 
  Thanks,
  Ethan
 
 
  On 2015/1/16 22:47, Jeff Kirsher wrote:
  On Fri, 2015-01-16 at 09:48 +0800, ethan zhao wrote:
  Vasu,
 
OK, disable FCOE as default configuration as a temporary step
  to make it  work.
  Sounds like I should expect a v2 coming, correct?
 
  Thanks,
  Ethan
 
  On 2015/1/16 7:45, Dev, Vasu wrote:
  -Original Message-
  From: ethan zhao [mailto:ethan.z...@oracle.com]
  Sent: Tuesday, January 13, 2015 6:41 PM
  To: Dev, Vasu
  Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg,
  Jesse; Allan, Bruce W; Wyborny, Carolyn; Skidmore, Donald C;
  Rose, Gregory V; Vick, Matthew; Williams, Mitch A; Parikh,
  Neerav; Linux NICS; e1000- de...@lists.sourceforge.net;
  net...@vger.kernel.org;
  linux- ker...@vger.kernel.org; brian.m...@oracle.com
  Subject: Re: [PATCH] i40e: don't enable and init FCOE by default
  when do PF reset
 
  Vasu,
 
  On 2015/1/14 3:38, Dev, Vasu wrote:
  -Original Message-
  diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
  b/drivers/net/ethernet/intel/i40e/i40e_main.c
  index a5f2660..a2572cc 100644
  --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
  +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
  @@ -6180,9 +6180,12 @@ static void
  i40e_reset_and_rebuild(struct i40e_pf *pf, bool reinit)
  }
   #endif /* CONFIG_I40E_DCB */
   #ifdef I40E_FCOE
  -   ret = i40e_init_pf_fcoe(pf);
  -   if (ret)
  -   dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n,
 ret);
  +   if (pf-flags  I40E_FLAG_FCOE_ENABLED) {
  +   ret = i40e_init_pf_fcoe(pf);
  Calling i40e_init_pf_fcoe() here conflicts with its
  I40E_FLAG_FCOE_ENABLED pre-condition since
  I40E_FLAG_FCOE_ENABLED is
  set by very same i40e_init_pf_fcoe(), in turn
  i40e_init_pf_fcoe() will never get called.
 
  I don't think so,  here ,i40e_reset_and_rebuild()  is not the
  only and the first place that  i40e_init_pf_fcoe() is called,
  see i40e_probe(), that is the first chance.
 
  i40e_probe()
  --i40e_sw_init()
   --i40e_init_pf_fcoe()
 
  And the I40E_FLAG_FCOE_ENABLED is possible be set by
  i40e_fcoe_enable() or i40e_fcoe_disable() interface before the
  reset action is to be done.
 
  It is set by i40e_init_pf_fcoe() and you are right that the
  modified call flow
  by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED
  anyway
  which could have prevented calling i40e_init_pf_fcoe() as I
  described above, so this is not an issue with the patch.
  BTW, the reason I post this patch is that we hit a bug, after
  setup vlan, the PF is enabled to FCOE.
 
  Then that BUG would still remain un

Re: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-19 Thread ethan zhao


On 2015/1/20 5:10, Dev, Vasu wrote:

-Original Message-
From: ethan zhao [mailto:ethan.z...@oracle.com]
Sent: Friday, January 16, 2015 7:01 PM
To: Kirsher, Jeffrey T
Cc: Dev, Vasu; Ethan Zhao; Ronciak, John; Brandeburg, Jesse; Allan, Bruce W;
Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick, Matthew;
Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
ker...@vger.kernel.org; brian.m...@oracle.com
Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
reset

Vasu,

What' your idea about the v2, any suggestion ?  Jeff is looking forward to
see it.


Jeff was asking for v2 in response to your last comment as "disable FCOE as default 
configuration as a temporary step" but I think that is the fix and user should n't 
enable FCoE until they have FCoE enabled X710 FCoE with either fabric or VN2VN mode FCoE 
setup.
 As a Linux distro, we don't know users have FCoE capable X710 or not, 
so we couldn't disable the FCoE configuration
 by default in the released kernel except FCoE is officially not 
supported yet with X710, but if yes, fix those bugs I

 mentioned is the only choice.


Thanks,
Ethan




Thanks,
Vasu


Thanks,
Ethan


On 2015/1/16 22:47, Jeff Kirsher wrote:

On Fri, 2015-01-16 at 09:48 +0800, ethan zhao wrote:

Vasu,

  OK, disable FCOE as default configuration as a temporary step to
make it  work.

Sounds like I should expect a v2 coming, correct?


Thanks,
Ethan

On 2015/1/16 7:45, Dev, Vasu wrote:

-Original Message-
From: ethan zhao [mailto:ethan.z...@oracle.com]
Sent: Tuesday, January 13, 2015 6:41 PM
To: Dev, Vasu
Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg,
Jesse; Allan, Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose,
Gregory V; Vick, Matthew; Williams, Mitch A; Parikh, Neerav; Linux
NICS; e1000- de...@lists.sourceforge.net; net...@vger.kernel.org;
linux- ker...@vger.kernel.org; brian.m...@oracle.com
Subject: Re: [PATCH] i40e: don't enable and init FCOE by default
when do PF reset

Vasu,

On 2015/1/14 3:38, Dev, Vasu wrote:

-Original Message-

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a5f2660..a2572cc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6180,9 +6180,12 @@ static void
i40e_reset_and_rebuild(struct i40e_pf *pf, bool reinit)
}
 #endif /* CONFIG_I40E_DCB */
 #ifdef I40E_FCOE
-   ret = i40e_init_pf_fcoe(pf);
-   if (ret)
-   dev_info(>pdev->dev, "init_pf_fcoe failed: %d\n", ret);
+   if (pf->flags & I40E_FLAG_FCOE_ENABLED) {
+   ret = i40e_init_pf_fcoe(pf);

Calling i40e_init_pf_fcoe() here conflicts with its

I40E_FLAG_FCOE_ENABLED pre-condition since

I40E_FLAG_FCOE_ENABLED is

set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
will never get called.

I don't think so,  here ,i40e_reset_and_rebuild()  is not the
only and the first place that  i40e_init_pf_fcoe() is called, see
i40e_probe(), that is the first chance.

i40e_probe()
-->i40e_sw_init()
 -->i40e_init_pf_fcoe()

And the I40E_FLAG_FCOE_ENABLED is possible be set by
i40e_fcoe_enable() or i40e_fcoe_disable() interface before the
reset action is to be done.


It is set by i40e_init_pf_fcoe() and you are right that the
modified call flow

by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED

anyway

which could have prevented calling i40e_init_pf_fcoe() as I
described above, so this is not an issue with the patch.

BTW, the reason I post this patch is that we hit a bug, after
setup vlan, the PF is enabled to FCOE.


Then that BUG would still remain un-fixed and calling
i40e_init_pf_fcoe()

under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to
fix anything. I mean I40E_FLAG_FCOE_ENABLED  condition will be true
with "pf-

hw.func_caps.fcoe == true" and otherwise calling
i40e_init_pf_fcoe() simply

returns back early on after checking "pf->hw.func_caps.fcoe ==
false", so how that bug is fixed here by added

I40E_FLAG_FCOE_ENABLED  condition ?

What is the bug ?
 The func_caps.fcoe is assigned by following call path, under
our test environment,

 i40e_probe()
  ->i40e_get_capabilities()
 ->i40e_aq_discover_capabilities()
->i40e_parse_discover_capabilities()

 Or

 i40e_reset_and_rebuild()
  ->i40e_get_capabilities()
->i40e_aq_discover_capabilities()
  ->i40e_parse_discover_capabilities()

 Under our test environment, the "pf->hw.func_caps.fcoe" is
true. so if
i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic

test.

 And then i40e_init_pf_fcoe() is to be called,

 While if (!pf->hw.func_caps.fcoe) wouldn't return,


I said it would return with "pf->hw.func_caps.fcoe == false" in my 

RE: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-19 Thread Dev, Vasu
> -Original Message-
> From: ethan zhao [mailto:ethan.z...@oracle.com]
> Sent: Friday, January 16, 2015 7:01 PM
> To: Kirsher, Jeffrey T
> Cc: Dev, Vasu; Ethan Zhao; Ronciak, John; Brandeburg, Jesse; Allan, Bruce W;
> Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick, Matthew;
> Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
> de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; brian.m...@oracle.com
> Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
> reset
> 
> Vasu,
> 
>What' your idea about the v2, any suggestion ?  Jeff is looking forward to
> see it.
> 

Jeff was asking for v2 in response to your last comment as "disable FCOE as 
default configuration as a temporary step" but I think that is the fix and user 
should n't enable FCoE until they have FCoE enabled X710 FCoE with either 
fabric or VN2VN mode FCoE setup.

Thanks,
Vasu

> Thanks,
> Ethan
> 
> 
> On 2015/1/16 22:47, Jeff Kirsher wrote:
> > On Fri, 2015-01-16 at 09:48 +0800, ethan zhao wrote:
> >> Vasu,
> >>
> >>  OK, disable FCOE as default configuration as a temporary step to
> >> make it  work.
> > Sounds like I should expect a v2 coming, correct?
> >
> >>
> >> Thanks,
> >> Ethan
> >>
> >> On 2015/1/16 7:45, Dev, Vasu wrote:
> >>>> -Original Message-
> >>>> From: ethan zhao [mailto:ethan.z...@oracle.com]
> >>>> Sent: Tuesday, January 13, 2015 6:41 PM
> >>>> To: Dev, Vasu
> >>>> Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg,
> >>>> Jesse; Allan, Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose,
> >>>> Gregory V; Vick, Matthew; Williams, Mitch A; Parikh, Neerav; Linux
> >>>> NICS; e1000- de...@lists.sourceforge.net; net...@vger.kernel.org;
> >>>> linux- ker...@vger.kernel.org; brian.m...@oracle.com
> >>>> Subject: Re: [PATCH] i40e: don't enable and init FCOE by default
> >>>> when do PF reset
> >>>>
> >>>> Vasu,
> >>>>
> >>>> On 2015/1/14 3:38, Dev, Vasu wrote:
> >>>>>> -Original Message-
> >>>>>>>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> >>>>>>>>> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> >>>>>>>>> index a5f2660..a2572cc 100644
> >>>>>>>>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> >>>>>>>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> >>>>>>>>> @@ -6180,9 +6180,12 @@ static void
> >>>>>>>>> i40e_reset_and_rebuild(struct i40e_pf *pf, bool reinit)
> >>>>>>>>>}
> >>>>>>>>> #endif /* CONFIG_I40E_DCB */
> >>>>>>>>> #ifdef I40E_FCOE
> >>>>>>>>> -   ret = i40e_init_pf_fcoe(pf);
> >>>>>>>>> -   if (ret)
> >>>>>>>>> -   dev_info(>pdev->dev, "init_pf_fcoe failed: %d\n", 
> >>>>>>>>> ret);
> >>>>>>>>> +   if (pf->flags & I40E_FLAG_FCOE_ENABLED) {
> >>>>>>>>> +   ret = i40e_init_pf_fcoe(pf);
> >>>>>>> Calling i40e_init_pf_fcoe() here conflicts with its
> >>>>>> I40E_FLAG_FCOE_ENABLED pre-condition since
> >>>> I40E_FLAG_FCOE_ENABLED is
> >>>>>> set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
> >>>>>> will never get called.
> >>>>>>
> >>>>>> I don't think so,  here ,i40e_reset_and_rebuild()  is not the
> >>>>>> only and the first place that  i40e_init_pf_fcoe() is called, see
> >>>>>> i40e_probe(), that is the first chance.
> >>>>>>
> >>>>>> i40e_probe()
> >>>>>> -->i40e_sw_init()
> >>>>>> -->i40e_init_pf_fcoe()
> >>>>>>
> >>>>>> And the I40E_FLAG_FCOE_ENABLED is possible be set by
> >>>>>> i40e_fcoe_enable() or i40e_fcoe_disable() interface before the
> >>>>>> reset action is to be done.
> >>>>>>
> >>>>> It is set by i40e_init_pf_fcoe() and you are right that the
> >>>>> modified call flow
> 

Re: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-19 Thread ethan zhao


On 2015/1/20 5:10, Dev, Vasu wrote:

-Original Message-
From: ethan zhao [mailto:ethan.z...@oracle.com]
Sent: Friday, January 16, 2015 7:01 PM
To: Kirsher, Jeffrey T
Cc: Dev, Vasu; Ethan Zhao; Ronciak, John; Brandeburg, Jesse; Allan, Bruce W;
Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick, Matthew;
Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
ker...@vger.kernel.org; brian.m...@oracle.com
Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
reset

Vasu,

What' your idea about the v2, any suggestion ?  Jeff is looking forward to
see it.


Jeff was asking for v2 in response to your last comment as disable FCOE as default 
configuration as a temporary step but I think that is the fix and user should n't 
enable FCoE until they have FCoE enabled X710 FCoE with either fabric or VN2VN mode FCoE 
setup.
 As a Linux distro, we don't know users have FCoE capable X710 or not, 
so we couldn't disable the FCoE configuration
 by default in the released kernel except FCoE is officially not 
supported yet with X710, but if yes, fix those bugs I

 mentioned is the only choice.


Thanks,
Ethan




Thanks,
Vasu


Thanks,
Ethan


On 2015/1/16 22:47, Jeff Kirsher wrote:

On Fri, 2015-01-16 at 09:48 +0800, ethan zhao wrote:

Vasu,

  OK, disable FCOE as default configuration as a temporary step to
make it  work.

Sounds like I should expect a v2 coming, correct?


Thanks,
Ethan

On 2015/1/16 7:45, Dev, Vasu wrote:

-Original Message-
From: ethan zhao [mailto:ethan.z...@oracle.com]
Sent: Tuesday, January 13, 2015 6:41 PM
To: Dev, Vasu
Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg,
Jesse; Allan, Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose,
Gregory V; Vick, Matthew; Williams, Mitch A; Parikh, Neerav; Linux
NICS; e1000- de...@lists.sourceforge.net; net...@vger.kernel.org;
linux- ker...@vger.kernel.org; brian.m...@oracle.com
Subject: Re: [PATCH] i40e: don't enable and init FCOE by default
when do PF reset

Vasu,

On 2015/1/14 3:38, Dev, Vasu wrote:

-Original Message-

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a5f2660..a2572cc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6180,9 +6180,12 @@ static void
i40e_reset_and_rebuild(struct i40e_pf *pf, bool reinit)
}
 #endif /* CONFIG_I40E_DCB */
 #ifdef I40E_FCOE
-   ret = i40e_init_pf_fcoe(pf);
-   if (ret)
-   dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n, ret);
+   if (pf-flags  I40E_FLAG_FCOE_ENABLED) {
+   ret = i40e_init_pf_fcoe(pf);

Calling i40e_init_pf_fcoe() here conflicts with its

I40E_FLAG_FCOE_ENABLED pre-condition since

I40E_FLAG_FCOE_ENABLED is

set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
will never get called.

I don't think so,  here ,i40e_reset_and_rebuild()  is not the
only and the first place that  i40e_init_pf_fcoe() is called, see
i40e_probe(), that is the first chance.

i40e_probe()
--i40e_sw_init()
 --i40e_init_pf_fcoe()

And the I40E_FLAG_FCOE_ENABLED is possible be set by
i40e_fcoe_enable() or i40e_fcoe_disable() interface before the
reset action is to be done.


It is set by i40e_init_pf_fcoe() and you are right that the
modified call flow

by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED

anyway

which could have prevented calling i40e_init_pf_fcoe() as I
described above, so this is not an issue with the patch.

BTW, the reason I post this patch is that we hit a bug, after
setup vlan, the PF is enabled to FCOE.


Then that BUG would still remain un-fixed and calling
i40e_init_pf_fcoe()

under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to
fix anything. I mean I40E_FLAG_FCOE_ENABLED  condition will be true
with pf-

hw.func_caps.fcoe == true and otherwise calling
i40e_init_pf_fcoe() simply

returns back early on after checking pf-hw.func_caps.fcoe ==
false, so how that bug is fixed here by added

I40E_FLAG_FCOE_ENABLED  condition ?

What is the bug ?
 The func_caps.fcoe is assigned by following call path, under
our test environment,

 i40e_probe()
  -i40e_get_capabilities()
 -i40e_aq_discover_capabilities()
-i40e_parse_discover_capabilities()

 Or

 i40e_reset_and_rebuild()
  -i40e_get_capabilities()
-i40e_aq_discover_capabilities()
  -i40e_parse_discover_capabilities()

 Under our test environment, the pf-hw.func_caps.fcoe is
true. so if
i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic

test.

 And then i40e_init_pf_fcoe() is to be called,

 While if (!pf-hw.func_caps.fcoe) wouldn't return,


I said it would return with pf-hw.func_caps.fcoe == false in my last

response, more details below.

 So  pf-flags is set to I40E_FLAG_FCOE_ENABLED.

 With my patch,  i40e_init_pf_fcoe

RE: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-19 Thread Dev, Vasu
 -Original Message-
 From: ethan zhao [mailto:ethan.z...@oracle.com]
 Sent: Friday, January 16, 2015 7:01 PM
 To: Kirsher, Jeffrey T
 Cc: Dev, Vasu; Ethan Zhao; Ronciak, John; Brandeburg, Jesse; Allan, Bruce W;
 Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick, Matthew;
 Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
 de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
 ker...@vger.kernel.org; brian.m...@oracle.com
 Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
 reset
 
 Vasu,
 
What' your idea about the v2, any suggestion ?  Jeff is looking forward to
 see it.
 

Jeff was asking for v2 in response to your last comment as disable FCOE as 
default configuration as a temporary step but I think that is the fix and user 
should n't enable FCoE until they have FCoE enabled X710 FCoE with either 
fabric or VN2VN mode FCoE setup.

Thanks,
Vasu

 Thanks,
 Ethan
 
 
 On 2015/1/16 22:47, Jeff Kirsher wrote:
  On Fri, 2015-01-16 at 09:48 +0800, ethan zhao wrote:
  Vasu,
 
   OK, disable FCOE as default configuration as a temporary step to
  make it  work.
  Sounds like I should expect a v2 coming, correct?
 
 
  Thanks,
  Ethan
 
  On 2015/1/16 7:45, Dev, Vasu wrote:
  -Original Message-
  From: ethan zhao [mailto:ethan.z...@oracle.com]
  Sent: Tuesday, January 13, 2015 6:41 PM
  To: Dev, Vasu
  Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg,
  Jesse; Allan, Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose,
  Gregory V; Vick, Matthew; Williams, Mitch A; Parikh, Neerav; Linux
  NICS; e1000- de...@lists.sourceforge.net; net...@vger.kernel.org;
  linux- ker...@vger.kernel.org; brian.m...@oracle.com
  Subject: Re: [PATCH] i40e: don't enable and init FCOE by default
  when do PF reset
 
  Vasu,
 
  On 2015/1/14 3:38, Dev, Vasu wrote:
  -Original Message-
  diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
  b/drivers/net/ethernet/intel/i40e/i40e_main.c
  index a5f2660..a2572cc 100644
  --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
  +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
  @@ -6180,9 +6180,12 @@ static void
  i40e_reset_and_rebuild(struct i40e_pf *pf, bool reinit)
 }
  #endif /* CONFIG_I40E_DCB */
  #ifdef I40E_FCOE
  -   ret = i40e_init_pf_fcoe(pf);
  -   if (ret)
  -   dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n, 
  ret);
  +   if (pf-flags  I40E_FLAG_FCOE_ENABLED) {
  +   ret = i40e_init_pf_fcoe(pf);
  Calling i40e_init_pf_fcoe() here conflicts with its
  I40E_FLAG_FCOE_ENABLED pre-condition since
  I40E_FLAG_FCOE_ENABLED is
  set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
  will never get called.
 
  I don't think so,  here ,i40e_reset_and_rebuild()  is not the
  only and the first place that  i40e_init_pf_fcoe() is called, see
  i40e_probe(), that is the first chance.
 
  i40e_probe()
  --i40e_sw_init()
  --i40e_init_pf_fcoe()
 
  And the I40E_FLAG_FCOE_ENABLED is possible be set by
  i40e_fcoe_enable() or i40e_fcoe_disable() interface before the
  reset action is to be done.
 
  It is set by i40e_init_pf_fcoe() and you are right that the
  modified call flow
  by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED
 anyway
  which could have prevented calling i40e_init_pf_fcoe() as I
  described above, so this is not an issue with the patch.
  BTW, the reason I post this patch is that we hit a bug, after
  setup vlan, the PF is enabled to FCOE.
 
  Then that BUG would still remain un-fixed and calling
  i40e_init_pf_fcoe()
  under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to
  fix anything. I mean I40E_FLAG_FCOE_ENABLED  condition will be true
  with pf-
  hw.func_caps.fcoe == true and otherwise calling
  i40e_init_pf_fcoe() simply
  returns back early on after checking pf-hw.func_caps.fcoe ==
  false, so how that bug is fixed here by added
 I40E_FLAG_FCOE_ENABLED  condition ?
  What is the bug ?
  The func_caps.fcoe is assigned by following call path, under
  our test environment,
 
  i40e_probe()
   -i40e_get_capabilities()
  -i40e_aq_discover_capabilities()
 -i40e_parse_discover_capabilities()
 
  Or
 
  i40e_reset_and_rebuild()
   -i40e_get_capabilities()
 -i40e_aq_discover_capabilities()
   -i40e_parse_discover_capabilities()
 
  Under our test environment, the pf-hw.func_caps.fcoe is
  true. so if
  i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic
 test.
  And then i40e_init_pf_fcoe() is to be called,
 
  While if (!pf-hw.func_caps.fcoe) wouldn't return,
 
  I said it would return with pf-hw.func_caps.fcoe == false in my last
 response, more details below.
 
  So  pf-flags is set to I40E_FLAG_FCOE_ENABLED.
 
  With my patch,  i40e_init_pf_fcoe() is only called after
  I40E_FLAG_FCOE_ENABLED is set before reset.
 
  Enable FCOE in i40e_probe() or not is another issue.
 
  Nope since both cases we

Re: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-16 Thread ethan zhao

Vasu,

  What' your idea about the v2, any suggestion ?  Jeff is looking 
forward to see it.


Thanks,
Ethan


On 2015/1/16 22:47, Jeff Kirsher wrote:

On Fri, 2015-01-16 at 09:48 +0800, ethan zhao wrote:

Vasu,

 OK, disable FCOE as default configuration as a temporary step to
make it  work.

Sounds like I should expect a v2 coming, correct?



Thanks,
Ethan

On 2015/1/16 7:45, Dev, Vasu wrote:

-Original Message-
From: ethan zhao [mailto:ethan.z...@oracle.com]
Sent: Tuesday, January 13, 2015 6:41 PM
To: Dev, Vasu
Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg, Jesse; Allan,
Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick,
Matthew; Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
ker...@vger.kernel.org; brian.m...@oracle.com
Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
reset

Vasu,

On 2015/1/14 3:38, Dev, Vasu wrote:

-Original Message-

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a5f2660..a2572cc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
i40e_pf *pf, bool reinit)
   }
#endif /* CONFIG_I40E_DCB */
#ifdef I40E_FCOE
-   ret = i40e_init_pf_fcoe(pf);
-   if (ret)
-   dev_info(>pdev->dev, "init_pf_fcoe failed: %d\n", ret);
+   if (pf->flags & I40E_FLAG_FCOE_ENABLED) {
+   ret = i40e_init_pf_fcoe(pf);

Calling i40e_init_pf_fcoe() here conflicts with its

I40E_FLAG_FCOE_ENABLED pre-condition since

I40E_FLAG_FCOE_ENABLED is

set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
will never get called.

I don't think so,  here ,i40e_reset_and_rebuild()  is not the only
and the first place that  i40e_init_pf_fcoe() is called, see
i40e_probe(), that is the first chance.

i40e_probe()
-->i40e_sw_init()
-->i40e_init_pf_fcoe()

And the I40E_FLAG_FCOE_ENABLED is possible be set by
i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset
action is to be done.


It is set by i40e_init_pf_fcoe() and you are right that the modified call flow

by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED anyway
which could have prevented calling i40e_init_pf_fcoe() as I described above,
so this is not an issue with the patch.

BTW, the reason I post this patch is that we hit a bug, after setup
vlan, the PF is enabled to FCOE.


Then that BUG would still remain un-fixed and calling i40e_init_pf_fcoe()

under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to fix
anything. I mean I40E_FLAG_FCOE_ENABLED  condition will be true with "pf-

hw.func_caps.fcoe == true" and otherwise calling i40e_init_pf_fcoe() simply

returns back early on after checking "pf->hw.func_caps.fcoe == false", so
how that bug is fixed here by added I40E_FLAG_FCOE_ENABLED  condition ?
What is the bug ?
The func_caps.fcoe is assigned by following call path, under our test
environment,

i40e_probe()
 ->i40e_get_capabilities()
->i40e_aq_discover_capabilities()
   ->i40e_parse_discover_capabilities()

Or

i40e_reset_and_rebuild()
 ->i40e_get_capabilities()
   ->i40e_aq_discover_capabilities()
 ->i40e_parse_discover_capabilities()

Under our test environment, the "pf->hw.func_caps.fcoe" is true. so if
i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic test.
And then i40e_init_pf_fcoe() is to be called,

While if (!pf->hw.func_caps.fcoe) wouldn't return,


I said it would return with "pf->hw.func_caps.fcoe == false" in my last 
response, more details below.


So  pf->flags is set to I40E_FLAG_FCOE_ENABLED.

With my patch,  i40e_init_pf_fcoe() is only called after
I40E_FLAG_FCOE_ENABLED is set before reset.

Enable FCOE in i40e_probe() or not is another issue.


Nope since both cases we should do i40e_init_pf_fcoe() or don't based on fcoe 
cap true or false.

I don't have much to add as I described before with the your patch that "calling i40e_init_pf_fcoe() 
under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to fix anything. I mean 
I40E_FLAG_FCOE_ENABLED  condition will be true with "pf->hw.func_caps.fcoe == true" and otherwise 
calling i40e_init_pf_fcoe() simply returns back early on after checking "pf->hw.func_caps.fcoe == 
false".

May be I'm missing something, I guess next either go with CONFIG_I40E_FCOE 
disable as I suggested before and now it in upstream kernel or we can have 
further off list discussion to fix the issue you are trying to fix with the 
patch.

Thanks,
Vasu


Thanks,
Ethan



Jeff Kirsher should be getting out a patch queued by me which adds

I40E_FCoE Kbuild option, in that FCoE is disabled b

Re: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-16 Thread Jeff Kirsher
On Fri, 2015-01-16 at 09:48 +0800, ethan zhao wrote:
> Vasu,
> 
> OK, disable FCOE as default configuration as a temporary step to 
> make it  work.

Sounds like I should expect a v2 coming, correct?

> 
> 
> Thanks,
> Ethan
> 
> On 2015/1/16 7:45, Dev, Vasu wrote:
> >> -Original Message-
> >> From: ethan zhao [mailto:ethan.z...@oracle.com]
> >> Sent: Tuesday, January 13, 2015 6:41 PM
> >> To: Dev, Vasu
> >> Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg, Jesse; 
> >> Allan,
> >> Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick,
> >> Matthew; Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
> >> de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
> >> ker...@vger.kernel.org; brian.m...@oracle.com
> >> Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
> >> reset
> >>
> >> Vasu,
> >>
> >> On 2015/1/14 3:38, Dev, Vasu wrote:
> >>>> -Original Message-
> >>>>>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> >>>>>>> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> >>>>>>> index a5f2660..a2572cc 100644
> >>>>>>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> >>>>>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> >>>>>>> @@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
> >>>>>>> i40e_pf *pf, bool reinit)
> >>>>>>>   }
> >>>>>>>#endif /* CONFIG_I40E_DCB */
> >>>>>>>#ifdef I40E_FCOE
> >>>>>>> -   ret = i40e_init_pf_fcoe(pf);
> >>>>>>> -   if (ret)
> >>>>>>> -   dev_info(>pdev->dev, "init_pf_fcoe failed: %d\n", 
> >>>>>>> ret);
> >>>>>>> +   if (pf->flags & I40E_FLAG_FCOE_ENABLED) {
> >>>>>>> +   ret = i40e_init_pf_fcoe(pf);
> >>>>> Calling i40e_init_pf_fcoe() here conflicts with its
> >>>> I40E_FLAG_FCOE_ENABLED pre-condition since
> >> I40E_FLAG_FCOE_ENABLED is
> >>>> set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
> >>>> will never get called.
> >>>>
> >>>> I don't think so,  here ,i40e_reset_and_rebuild()  is not the only
> >>>> and the first place that  i40e_init_pf_fcoe() is called, see
> >>>> i40e_probe(), that is the first chance.
> >>>>
> >>>> i40e_probe()
> >>>> -->i40e_sw_init()
> >>>>-->i40e_init_pf_fcoe()
> >>>>
> >>>> And the I40E_FLAG_FCOE_ENABLED is possible be set by
> >>>> i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset
> >>>> action is to be done.
> >>>>
> >>> It is set by i40e_init_pf_fcoe() and you are right that the modified call 
> >>> flow
> >> by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED anyway
> >> which could have prevented calling i40e_init_pf_fcoe() as I described 
> >> above,
> >> so this is not an issue with the patch.
> >>>> BTW, the reason I post this patch is that we hit a bug, after setup
> >>>> vlan, the PF is enabled to FCOE.
> >>>>
> >>> Then that BUG would still remain un-fixed and calling i40e_init_pf_fcoe()
> >> under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to fix
> >> anything. I mean I40E_FLAG_FCOE_ENABLED  condition will be true with "pf-
> >>> hw.func_caps.fcoe == true" and otherwise calling i40e_init_pf_fcoe() 
> >>> simply
> >> returns back early on after checking "pf->hw.func_caps.fcoe == false", so
> >> how that bug is fixed here by added I40E_FLAG_FCOE_ENABLED  condition ?
> >> What is the bug ?
> >>The func_caps.fcoe is assigned by following call path, under our test
> >> environment,
> >>
> >>i40e_probe()
> >> ->i40e_get_capabilities()
> >>->i40e_aq_discover_capabilities()
> >>   ->i40e_parse_discover_capabilities()
> >>
> >>Or
> >>
> >>i40e_reset_and_rebuild()
> >> ->i40e_get_capabilities()
> >>   ->i40e_aq_discover_capabilities()
> >> ->i40e_parse_discover_capabilities()
&g

Re: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-16 Thread ethan zhao

Vasu,

  What' your idea about the v2, any suggestion ?  Jeff is looking 
forward to see it.


Thanks,
Ethan


On 2015/1/16 22:47, Jeff Kirsher wrote:

On Fri, 2015-01-16 at 09:48 +0800, ethan zhao wrote:

Vasu,

 OK, disable FCOE as default configuration as a temporary step to
make it  work.

Sounds like I should expect a v2 coming, correct?



Thanks,
Ethan

On 2015/1/16 7:45, Dev, Vasu wrote:

-Original Message-
From: ethan zhao [mailto:ethan.z...@oracle.com]
Sent: Tuesday, January 13, 2015 6:41 PM
To: Dev, Vasu
Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg, Jesse; Allan,
Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick,
Matthew; Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
ker...@vger.kernel.org; brian.m...@oracle.com
Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
reset

Vasu,

On 2015/1/14 3:38, Dev, Vasu wrote:

-Original Message-

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a5f2660..a2572cc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
i40e_pf *pf, bool reinit)
   }
#endif /* CONFIG_I40E_DCB */
#ifdef I40E_FCOE
-   ret = i40e_init_pf_fcoe(pf);
-   if (ret)
-   dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n, ret);
+   if (pf-flags  I40E_FLAG_FCOE_ENABLED) {
+   ret = i40e_init_pf_fcoe(pf);

Calling i40e_init_pf_fcoe() here conflicts with its

I40E_FLAG_FCOE_ENABLED pre-condition since

I40E_FLAG_FCOE_ENABLED is

set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
will never get called.

I don't think so,  here ,i40e_reset_and_rebuild()  is not the only
and the first place that  i40e_init_pf_fcoe() is called, see
i40e_probe(), that is the first chance.

i40e_probe()
--i40e_sw_init()
--i40e_init_pf_fcoe()

And the I40E_FLAG_FCOE_ENABLED is possible be set by
i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset
action is to be done.


It is set by i40e_init_pf_fcoe() and you are right that the modified call flow

by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED anyway
which could have prevented calling i40e_init_pf_fcoe() as I described above,
so this is not an issue with the patch.

BTW, the reason I post this patch is that we hit a bug, after setup
vlan, the PF is enabled to FCOE.


Then that BUG would still remain un-fixed and calling i40e_init_pf_fcoe()

under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to fix
anything. I mean I40E_FLAG_FCOE_ENABLED  condition will be true with pf-

hw.func_caps.fcoe == true and otherwise calling i40e_init_pf_fcoe() simply

returns back early on after checking pf-hw.func_caps.fcoe == false, so
how that bug is fixed here by added I40E_FLAG_FCOE_ENABLED  condition ?
What is the bug ?
The func_caps.fcoe is assigned by following call path, under our test
environment,

i40e_probe()
 -i40e_get_capabilities()
-i40e_aq_discover_capabilities()
   -i40e_parse_discover_capabilities()

Or

i40e_reset_and_rebuild()
 -i40e_get_capabilities()
   -i40e_aq_discover_capabilities()
 -i40e_parse_discover_capabilities()

Under our test environment, the pf-hw.func_caps.fcoe is true. so if
i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic test.
And then i40e_init_pf_fcoe() is to be called,

While if (!pf-hw.func_caps.fcoe) wouldn't return,


I said it would return with pf-hw.func_caps.fcoe == false in my last 
response, more details below.


So  pf-flags is set to I40E_FLAG_FCOE_ENABLED.

With my patch,  i40e_init_pf_fcoe() is only called after
I40E_FLAG_FCOE_ENABLED is set before reset.

Enable FCOE in i40e_probe() or not is another issue.


Nope since both cases we should do i40e_init_pf_fcoe() or don't based on fcoe 
cap true or false.

I don't have much to add as I described before with the your patch that calling i40e_init_pf_fcoe() 
under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to fix anything. I mean 
I40E_FLAG_FCOE_ENABLED  condition will be true with pf-hw.func_caps.fcoe == true and otherwise 
calling i40e_init_pf_fcoe() simply returns back early on after checking pf-hw.func_caps.fcoe == 
false.

May be I'm missing something, I guess next either go with CONFIG_I40E_FCOE 
disable as I suggested before and now it in upstream kernel or we can have 
further off list discussion to fix the issue you are trying to fix with the 
patch.

Thanks,
Vasu


Thanks,
Ethan



Jeff Kirsher should be getting out a patch queued by me which adds

I40E_FCoE Kbuild option, in that FCoE is disabled by default and
user could enable FCoE only if needed, that patch would do same of
skipping
i40e_init_pf_fcoe() whether FCoE capability in device enabled

Re: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-16 Thread Jeff Kirsher
On Fri, 2015-01-16 at 09:48 +0800, ethan zhao wrote:
 Vasu,
 
 OK, disable FCOE as default configuration as a temporary step to 
 make it  work.

Sounds like I should expect a v2 coming, correct?

 
 
 Thanks,
 Ethan
 
 On 2015/1/16 7:45, Dev, Vasu wrote:
  -Original Message-
  From: ethan zhao [mailto:ethan.z...@oracle.com]
  Sent: Tuesday, January 13, 2015 6:41 PM
  To: Dev, Vasu
  Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg, Jesse; 
  Allan,
  Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick,
  Matthew; Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
  de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
  ker...@vger.kernel.org; brian.m...@oracle.com
  Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
  reset
 
  Vasu,
 
  On 2015/1/14 3:38, Dev, Vasu wrote:
  -Original Message-
  diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
  b/drivers/net/ethernet/intel/i40e/i40e_main.c
  index a5f2660..a2572cc 100644
  --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
  +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
  @@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
  i40e_pf *pf, bool reinit)
}
 #endif /* CONFIG_I40E_DCB */
 #ifdef I40E_FCOE
  -   ret = i40e_init_pf_fcoe(pf);
  -   if (ret)
  -   dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n, 
  ret);
  +   if (pf-flags  I40E_FLAG_FCOE_ENABLED) {
  +   ret = i40e_init_pf_fcoe(pf);
  Calling i40e_init_pf_fcoe() here conflicts with its
  I40E_FLAG_FCOE_ENABLED pre-condition since
  I40E_FLAG_FCOE_ENABLED is
  set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
  will never get called.
 
  I don't think so,  here ,i40e_reset_and_rebuild()  is not the only
  and the first place that  i40e_init_pf_fcoe() is called, see
  i40e_probe(), that is the first chance.
 
  i40e_probe()
  --i40e_sw_init()
 --i40e_init_pf_fcoe()
 
  And the I40E_FLAG_FCOE_ENABLED is possible be set by
  i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset
  action is to be done.
 
  It is set by i40e_init_pf_fcoe() and you are right that the modified call 
  flow
  by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED anyway
  which could have prevented calling i40e_init_pf_fcoe() as I described 
  above,
  so this is not an issue with the patch.
  BTW, the reason I post this patch is that we hit a bug, after setup
  vlan, the PF is enabled to FCOE.
 
  Then that BUG would still remain un-fixed and calling i40e_init_pf_fcoe()
  under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to fix
  anything. I mean I40E_FLAG_FCOE_ENABLED  condition will be true with pf-
  hw.func_caps.fcoe == true and otherwise calling i40e_init_pf_fcoe() 
  simply
  returns back early on after checking pf-hw.func_caps.fcoe == false, so
  how that bug is fixed here by added I40E_FLAG_FCOE_ENABLED  condition ?
  What is the bug ?
 The func_caps.fcoe is assigned by following call path, under our test
  environment,
 
 i40e_probe()
  -i40e_get_capabilities()
 -i40e_aq_discover_capabilities()
-i40e_parse_discover_capabilities()
 
 Or
 
 i40e_reset_and_rebuild()
  -i40e_get_capabilities()
-i40e_aq_discover_capabilities()
  -i40e_parse_discover_capabilities()
 
 Under our test environment, the pf-hw.func_caps.fcoe is true. so if
  i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic test.
 And then i40e_init_pf_fcoe() is to be called,
 
 While if (!pf-hw.func_caps.fcoe) wouldn't return,
 
  I said it would return with pf-hw.func_caps.fcoe == false in my last 
  response, more details below.
 
 So  pf-flags is set to I40E_FLAG_FCOE_ENABLED.
 
 With my patch,  i40e_init_pf_fcoe() is only called after
  I40E_FLAG_FCOE_ENABLED is set before reset.
 
  Enable FCOE in i40e_probe() or not is another issue.
 
  Nope since both cases we should do i40e_init_pf_fcoe() or don't based on 
  fcoe cap true or false.
 
  I don't have much to add as I described before with the your patch that 
  calling i40e_init_pf_fcoe() under I40E_FLAG_FCOE_ENABLED  flag really 
  won't affect call flow to fix anything. I mean I40E_FLAG_FCOE_ENABLED  
  condition will be true with pf-hw.func_caps.fcoe == true and otherwise 
  calling i40e_init_pf_fcoe() simply returns back early on after checking 
  pf-hw.func_caps.fcoe == false.
 
  May be I'm missing something, I guess next either go with CONFIG_I40E_FCOE 
  disable as I suggested before and now it in upstream kernel or we can have 
  further off list discussion to fix the issue you are trying to fix with the 
  patch.
 
  Thanks,
  Vasu
 
  Thanks,
  Ethan
 
 
  Jeff Kirsher should be getting out a patch queued by me which adds
  I40E_FCoE Kbuild option, in that FCoE is disabled by default and
  user could enable FCoE only if needed, that patch would do same of
  skipping
  i40e_init_pf_fcoe

Re: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-15 Thread ethan zhao

Vasu,

   OK, disable FCOE as default configuration as a temporary step to 
make it  work.



Thanks,
Ethan

On 2015/1/16 7:45, Dev, Vasu wrote:

-Original Message-
From: ethan zhao [mailto:ethan.z...@oracle.com]
Sent: Tuesday, January 13, 2015 6:41 PM
To: Dev, Vasu
Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg, Jesse; Allan,
Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick,
Matthew; Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
ker...@vger.kernel.org; brian.m...@oracle.com
Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
reset

Vasu,

On 2015/1/14 3:38, Dev, Vasu wrote:

-Original Message-

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a5f2660..a2572cc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
i40e_pf *pf, bool reinit)
  }
   #endif /* CONFIG_I40E_DCB */
   #ifdef I40E_FCOE
-   ret = i40e_init_pf_fcoe(pf);
-   if (ret)
-   dev_info(>pdev->dev, "init_pf_fcoe failed: %d\n", ret);
+   if (pf->flags & I40E_FLAG_FCOE_ENABLED) {
+   ret = i40e_init_pf_fcoe(pf);

Calling i40e_init_pf_fcoe() here conflicts with its

I40E_FLAG_FCOE_ENABLED pre-condition since

I40E_FLAG_FCOE_ENABLED is

set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
will never get called.

I don't think so,  here ,i40e_reset_and_rebuild()  is not the only
and the first place that  i40e_init_pf_fcoe() is called, see
i40e_probe(), that is the first chance.

i40e_probe()
-->i40e_sw_init()
   -->i40e_init_pf_fcoe()

And the I40E_FLAG_FCOE_ENABLED is possible be set by
i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset
action is to be done.


It is set by i40e_init_pf_fcoe() and you are right that the modified call flow

by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED anyway
which could have prevented calling i40e_init_pf_fcoe() as I described above,
so this is not an issue with the patch.

BTW, the reason I post this patch is that we hit a bug, after setup
vlan, the PF is enabled to FCOE.


Then that BUG would still remain un-fixed and calling i40e_init_pf_fcoe()

under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to fix
anything. I mean I40E_FLAG_FCOE_ENABLED  condition will be true with "pf-

hw.func_caps.fcoe == true" and otherwise calling i40e_init_pf_fcoe() simply

returns back early on after checking "pf->hw.func_caps.fcoe == false", so
how that bug is fixed here by added I40E_FLAG_FCOE_ENABLED  condition ?
What is the bug ?
   The func_caps.fcoe is assigned by following call path, under our test
environment,

   i40e_probe()
->i40e_get_capabilities()
   ->i40e_aq_discover_capabilities()
  ->i40e_parse_discover_capabilities()

   Or

   i40e_reset_and_rebuild()
->i40e_get_capabilities()
  ->i40e_aq_discover_capabilities()
->i40e_parse_discover_capabilities()

   Under our test environment, the "pf->hw.func_caps.fcoe" is true. so if
i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic test.
   And then i40e_init_pf_fcoe() is to be called,

   While if (!pf->hw.func_caps.fcoe) wouldn't return,


I said it would return with "pf->hw.func_caps.fcoe == false" in my last 
response, more details below.


   So  pf->flags is set to I40E_FLAG_FCOE_ENABLED.

   With my patch,  i40e_init_pf_fcoe() is only called after
I40E_FLAG_FCOE_ENABLED is set before reset.

Enable FCOE in i40e_probe() or not is another issue.


Nope since both cases we should do i40e_init_pf_fcoe() or don't based on fcoe 
cap true or false.

I don't have much to add as I described before with the your patch that "calling i40e_init_pf_fcoe() 
under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to fix anything. I mean 
I40E_FLAG_FCOE_ENABLED  condition will be true with "pf->hw.func_caps.fcoe == true" and otherwise 
calling i40e_init_pf_fcoe() simply returns back early on after checking "pf->hw.func_caps.fcoe == 
false".

May be I'm missing something, I guess next either go with CONFIG_I40E_FCOE 
disable as I suggested before and now it in upstream kernel or we can have 
further off list discussion to fix the issue you are trying to fix with the 
patch.

Thanks,
Vasu


Thanks,
Ethan



Jeff Kirsher should be getting out a patch queued by me which adds

I40E_FCoE Kbuild option, in that FCoE is disabled by default and
user could enable FCoE only if needed, that patch would do same of
skipping
i40e_init_pf_fcoe() whether FCoE capability in device enabled or not
in default config.
The following patch will not fix the above issue -- configuration of
PF will be changed via reset

RE: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-15 Thread Dev, Vasu
> -Original Message-
> From: ethan zhao [mailto:ethan.z...@oracle.com]
> Sent: Tuesday, January 13, 2015 6:41 PM
> To: Dev, Vasu
> Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg, Jesse; Allan,
> Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick,
> Matthew; Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
> de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; brian.m...@oracle.com
> Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
> reset
> 
> Vasu,
> 
> On 2015/1/14 3:38, Dev, Vasu wrote:
> >> -Original Message-
> >>>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> >>>>> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> >>>>> index a5f2660..a2572cc 100644
> >>>>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> >>>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> >>>>> @@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
> >>>>> i40e_pf *pf, bool reinit)
> >>>>>  }
> >>>>>   #endif /* CONFIG_I40E_DCB */
> >>>>>   #ifdef I40E_FCOE
> >>>>> -   ret = i40e_init_pf_fcoe(pf);
> >>>>> -   if (ret)
> >>>>> -   dev_info(>pdev->dev, "init_pf_fcoe failed: %d\n", ret);
> >>>>> +   if (pf->flags & I40E_FLAG_FCOE_ENABLED) {
> >>>>> +   ret = i40e_init_pf_fcoe(pf);
> >>> Calling i40e_init_pf_fcoe() here conflicts with its
> >> I40E_FLAG_FCOE_ENABLED pre-condition since
> I40E_FLAG_FCOE_ENABLED is
> >> set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
> >> will never get called.
> >>
> >> I don't think so,  here ,i40e_reset_and_rebuild()  is not the only
> >> and the first place that  i40e_init_pf_fcoe() is called, see
> >> i40e_probe(), that is the first chance.
> >>
> >> i40e_probe()
> >> -->i40e_sw_init()
> >>   -->i40e_init_pf_fcoe()
> >>
> >> And the I40E_FLAG_FCOE_ENABLED is possible be set by
> >> i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset
> >> action is to be done.
> >>
> > It is set by i40e_init_pf_fcoe() and you are right that the modified call 
> > flow
> by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED anyway
> which could have prevented calling i40e_init_pf_fcoe() as I described above,
> so this is not an issue with the patch.
> >
> >> BTW, the reason I post this patch is that we hit a bug, after setup
> >> vlan, the PF is enabled to FCOE.
> >>
> > Then that BUG would still remain un-fixed and calling i40e_init_pf_fcoe()
> under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to fix
> anything. I mean I40E_FLAG_FCOE_ENABLED  condition will be true with "pf-
> >hw.func_caps.fcoe == true" and otherwise calling i40e_init_pf_fcoe() simply
> returns back early on after checking "pf->hw.func_caps.fcoe == false", so
> how that bug is fixed here by added I40E_FLAG_FCOE_ENABLED  condition ?
> What is the bug ?
>   The func_caps.fcoe is assigned by following call path, under our test
> environment,
> 
>   i40e_probe()
>->i40e_get_capabilities()
>   ->i40e_aq_discover_capabilities()
>  ->i40e_parse_discover_capabilities()
> 
>   Or
> 
>   i40e_reset_and_rebuild()
>->i40e_get_capabilities()
>  ->i40e_aq_discover_capabilities()
>->i40e_parse_discover_capabilities()
> 
>   Under our test environment, the "pf->hw.func_caps.fcoe" is true. so if
> i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic test.
>   And then i40e_init_pf_fcoe() is to be called,
> 
>   While if (!pf->hw.func_caps.fcoe) wouldn't return,
> 

I said it would return with "pf->hw.func_caps.fcoe == false" in my last 
response, more details below.

>   So  pf->flags is set to I40E_FLAG_FCOE_ENABLED.
> 
>   With my patch,  i40e_init_pf_fcoe() is only called after
> I40E_FLAG_FCOE_ENABLED is set before reset.
> 
> Enable FCOE in i40e_probe() or not is another issue.
> 

Nope since both cases we should do i40e_init_pf_fcoe() or don't based on fcoe 
cap true or false. 

I don't have much to add as I described before with the your patch that 
"calling i40e_init_pf_fcoe() under I40E_FLAG_FCOE_ENABLED  flag really won't 
affect call flow to fix anything. I mean I40E_FLAG_FCOE_ENABLED  condition will 
be true with "pf->hw.func_caps.fcoe 

RE: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-15 Thread Dev, Vasu
 -Original Message-
 From: ethan zhao [mailto:ethan.z...@oracle.com]
 Sent: Tuesday, January 13, 2015 6:41 PM
 To: Dev, Vasu
 Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg, Jesse; Allan,
 Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick,
 Matthew; Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
 de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
 ker...@vger.kernel.org; brian.m...@oracle.com
 Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
 reset
 
 Vasu,
 
 On 2015/1/14 3:38, Dev, Vasu wrote:
  -Original Message-
  diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
  b/drivers/net/ethernet/intel/i40e/i40e_main.c
  index a5f2660..a2572cc 100644
  --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
  +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
  @@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
  i40e_pf *pf, bool reinit)
   }
#endif /* CONFIG_I40E_DCB */
#ifdef I40E_FCOE
  -   ret = i40e_init_pf_fcoe(pf);
  -   if (ret)
  -   dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n, ret);
  +   if (pf-flags  I40E_FLAG_FCOE_ENABLED) {
  +   ret = i40e_init_pf_fcoe(pf);
  Calling i40e_init_pf_fcoe() here conflicts with its
  I40E_FLAG_FCOE_ENABLED pre-condition since
 I40E_FLAG_FCOE_ENABLED is
  set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
  will never get called.
 
  I don't think so,  here ,i40e_reset_and_rebuild()  is not the only
  and the first place that  i40e_init_pf_fcoe() is called, see
  i40e_probe(), that is the first chance.
 
  i40e_probe()
  --i40e_sw_init()
--i40e_init_pf_fcoe()
 
  And the I40E_FLAG_FCOE_ENABLED is possible be set by
  i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset
  action is to be done.
 
  It is set by i40e_init_pf_fcoe() and you are right that the modified call 
  flow
 by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED anyway
 which could have prevented calling i40e_init_pf_fcoe() as I described above,
 so this is not an issue with the patch.
 
  BTW, the reason I post this patch is that we hit a bug, after setup
  vlan, the PF is enabled to FCOE.
 
  Then that BUG would still remain un-fixed and calling i40e_init_pf_fcoe()
 under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to fix
 anything. I mean I40E_FLAG_FCOE_ENABLED  condition will be true with pf-
 hw.func_caps.fcoe == true and otherwise calling i40e_init_pf_fcoe() simply
 returns back early on after checking pf-hw.func_caps.fcoe == false, so
 how that bug is fixed here by added I40E_FLAG_FCOE_ENABLED  condition ?
 What is the bug ?
   The func_caps.fcoe is assigned by following call path, under our test
 environment,
 
   i40e_probe()
-i40e_get_capabilities()
   -i40e_aq_discover_capabilities()
  -i40e_parse_discover_capabilities()
 
   Or
 
   i40e_reset_and_rebuild()
-i40e_get_capabilities()
  -i40e_aq_discover_capabilities()
-i40e_parse_discover_capabilities()
 
   Under our test environment, the pf-hw.func_caps.fcoe is true. so if
 i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic test.
   And then i40e_init_pf_fcoe() is to be called,
 
   While if (!pf-hw.func_caps.fcoe) wouldn't return,
 

I said it would return with pf-hw.func_caps.fcoe == false in my last 
response, more details below.

   So  pf-flags is set to I40E_FLAG_FCOE_ENABLED.
 
   With my patch,  i40e_init_pf_fcoe() is only called after
 I40E_FLAG_FCOE_ENABLED is set before reset.
 
 Enable FCOE in i40e_probe() or not is another issue.
 

Nope since both cases we should do i40e_init_pf_fcoe() or don't based on fcoe 
cap true or false. 

I don't have much to add as I described before with the your patch that 
calling i40e_init_pf_fcoe() under I40E_FLAG_FCOE_ENABLED  flag really won't 
affect call flow to fix anything. I mean I40E_FLAG_FCOE_ENABLED  condition will 
be true with pf-hw.func_caps.fcoe == true and otherwise calling 
i40e_init_pf_fcoe() simply returns back early on after checking 
pf-hw.func_caps.fcoe == false. 

May be I'm missing something, I guess next either go with CONFIG_I40E_FCOE 
disable as I suggested before and now it in upstream kernel or we can have 
further off list discussion to fix the issue you are trying to fix with the 
patch.

Thanks,
Vasu

 Thanks,
 Ethan
 
 
 
  Jeff Kirsher should be getting out a patch queued by me which adds
  I40E_FCoE Kbuild option, in that FCoE is disabled by default and
  user could enable FCoE only if needed, that patch would do same of
  skipping
  i40e_init_pf_fcoe() whether FCoE capability in device enabled or not
  in default config.
  The following patch will not fix the above issue -- configuration of
  PF will be changed via reset.
  How about the FCOE is configured and disabled by  i40e_fcoe_disable()
  , then reset happens ?
 
  May be but if the BUG is due to FCoE being enabled then having it disabled
 in config

Re: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-15 Thread ethan zhao

Vasu,

   OK, disable FCOE as default configuration as a temporary step to 
make it  work.



Thanks,
Ethan

On 2015/1/16 7:45, Dev, Vasu wrote:

-Original Message-
From: ethan zhao [mailto:ethan.z...@oracle.com]
Sent: Tuesday, January 13, 2015 6:41 PM
To: Dev, Vasu
Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg, Jesse; Allan,
Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick,
Matthew; Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
ker...@vger.kernel.org; brian.m...@oracle.com
Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
reset

Vasu,

On 2015/1/14 3:38, Dev, Vasu wrote:

-Original Message-

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a5f2660..a2572cc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
i40e_pf *pf, bool reinit)
  }
   #endif /* CONFIG_I40E_DCB */
   #ifdef I40E_FCOE
-   ret = i40e_init_pf_fcoe(pf);
-   if (ret)
-   dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n, ret);
+   if (pf-flags  I40E_FLAG_FCOE_ENABLED) {
+   ret = i40e_init_pf_fcoe(pf);

Calling i40e_init_pf_fcoe() here conflicts with its

I40E_FLAG_FCOE_ENABLED pre-condition since

I40E_FLAG_FCOE_ENABLED is

set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
will never get called.

I don't think so,  here ,i40e_reset_and_rebuild()  is not the only
and the first place that  i40e_init_pf_fcoe() is called, see
i40e_probe(), that is the first chance.

i40e_probe()
--i40e_sw_init()
   --i40e_init_pf_fcoe()

And the I40E_FLAG_FCOE_ENABLED is possible be set by
i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset
action is to be done.


It is set by i40e_init_pf_fcoe() and you are right that the modified call flow

by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED anyway
which could have prevented calling i40e_init_pf_fcoe() as I described above,
so this is not an issue with the patch.

BTW, the reason I post this patch is that we hit a bug, after setup
vlan, the PF is enabled to FCOE.


Then that BUG would still remain un-fixed and calling i40e_init_pf_fcoe()

under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to fix
anything. I mean I40E_FLAG_FCOE_ENABLED  condition will be true with pf-

hw.func_caps.fcoe == true and otherwise calling i40e_init_pf_fcoe() simply

returns back early on after checking pf-hw.func_caps.fcoe == false, so
how that bug is fixed here by added I40E_FLAG_FCOE_ENABLED  condition ?
What is the bug ?
   The func_caps.fcoe is assigned by following call path, under our test
environment,

   i40e_probe()
-i40e_get_capabilities()
   -i40e_aq_discover_capabilities()
  -i40e_parse_discover_capabilities()

   Or

   i40e_reset_and_rebuild()
-i40e_get_capabilities()
  -i40e_aq_discover_capabilities()
-i40e_parse_discover_capabilities()

   Under our test environment, the pf-hw.func_caps.fcoe is true. so if
i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic test.
   And then i40e_init_pf_fcoe() is to be called,

   While if (!pf-hw.func_caps.fcoe) wouldn't return,


I said it would return with pf-hw.func_caps.fcoe == false in my last 
response, more details below.


   So  pf-flags is set to I40E_FLAG_FCOE_ENABLED.

   With my patch,  i40e_init_pf_fcoe() is only called after
I40E_FLAG_FCOE_ENABLED is set before reset.

Enable FCOE in i40e_probe() or not is another issue.


Nope since both cases we should do i40e_init_pf_fcoe() or don't based on fcoe 
cap true or false.

I don't have much to add as I described before with the your patch that calling i40e_init_pf_fcoe() 
under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to fix anything. I mean 
I40E_FLAG_FCOE_ENABLED  condition will be true with pf-hw.func_caps.fcoe == true and otherwise 
calling i40e_init_pf_fcoe() simply returns back early on after checking pf-hw.func_caps.fcoe == 
false.

May be I'm missing something, I guess next either go with CONFIG_I40E_FCOE 
disable as I suggested before and now it in upstream kernel or we can have 
further off list discussion to fix the issue you are trying to fix with the 
patch.

Thanks,
Vasu


Thanks,
Ethan



Jeff Kirsher should be getting out a patch queued by me which adds

I40E_FCoE Kbuild option, in that FCoE is disabled by default and
user could enable FCoE only if needed, that patch would do same of
skipping
i40e_init_pf_fcoe() whether FCoE capability in device enabled or not
in default config.
The following patch will not fix the above issue -- configuration of
PF will be changed via reset.
How about the FCOE is configured and disabled by  i40e_fcoe_disable()
, then reset happens ?


May be but if the BUG is due to FCoE being enabled then having

Re: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-13 Thread ethan zhao

Vasu,

On 2015/1/14 3:38, Dev, Vasu wrote:

-Original Message-

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a5f2660..a2572cc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
i40e_pf *pf, bool reinit)
 }
  #endif /* CONFIG_I40E_DCB */
  #ifdef I40E_FCOE
-   ret = i40e_init_pf_fcoe(pf);
-   if (ret)
-   dev_info(>pdev->dev, "init_pf_fcoe failed: %d\n", ret);
+   if (pf->flags & I40E_FLAG_FCOE_ENABLED) {
+   ret = i40e_init_pf_fcoe(pf);

Calling i40e_init_pf_fcoe() here conflicts with its

I40E_FLAG_FCOE_ENABLED pre-condition since I40E_FLAG_FCOE_ENABLED is
set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe() will never get
called.

I don't think so,  here ,i40e_reset_and_rebuild()  is not the only and the first
place that  i40e_init_pf_fcoe() is called, see i40e_probe(), that is the first
chance.

i40e_probe()
-->i40e_sw_init()
  -->i40e_init_pf_fcoe()

And the I40E_FLAG_FCOE_ENABLED is possible be set by
i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset action is
to be done.


It is set by i40e_init_pf_fcoe() and you are right that the modified call flow 
by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED anyway which could 
have prevented calling i40e_init_pf_fcoe() as I described above, so this is not 
an issue with the patch.


BTW, the reason I post this patch is that we hit a bug, after setup vlan, the PF
is enabled to FCOE.


Then that BUG would still remain un-fixed and calling i40e_init_pf_fcoe() under I40E_FLAG_FCOE_ENABLED  
flag really won't affect call flow to fix anything. I mean I40E_FLAG_FCOE_ENABLED  condition will be true 
with "pf->hw.func_caps.fcoe == true" and otherwise calling i40e_init_pf_fcoe() simply 
returns back early on after checking "pf->hw.func_caps.fcoe == false", so how that bug is 
fixed here by added I40E_FLAG_FCOE_ENABLED  condition ? What is the bug ?
 The func_caps.fcoe is assigned by following call path, under our test 
environment,


 i40e_probe()
  ->i40e_get_capabilities()
 ->i40e_aq_discover_capabilities()
->i40e_parse_discover_capabilities()

 Or

 i40e_reset_and_rebuild()
  ->i40e_get_capabilities()
->i40e_aq_discover_capabilities()
  ->i40e_parse_discover_capabilities()

 Under our test environment, the "pf->hw.func_caps.fcoe" is true. so if 
i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic test.

 And then i40e_init_pf_fcoe() is to be called,

 While if (!pf->hw.func_caps.fcoe) wouldn't return,

 So  pf->flags is set to I40E_FLAG_FCOE_ENABLED.

 With my patch,  i40e_init_pf_fcoe() is only called after 
I40E_FLAG_FCOE_ENABLED is set before reset.


Enable FCOE in i40e_probe() or not is another issue.


Thanks,
Ethan





Jeff Kirsher should be getting out a patch queued by me which adds

I40E_FCoE Kbuild option, in that FCoE is disabled by default and  user could
enable FCoE only if needed, that patch would do same of skipping
i40e_init_pf_fcoe() whether FCoE capability in device enabled or not in
default config.
The following patch will not fix the above issue -- configuration of PF will be
changed via reset.
How about the FCOE is configured and disabled by  i40e_fcoe_disable() ,
then reset happens ?


May be but if the BUG is due to FCoE being enabled then having it disabled in 
config will avoid the bug for non FCoE config option and once bug is understood 
then that has to be fixed for FCoE enabled config also as I asked above.

Thanks Ethan for detailed response.
Vasu


 From patchwork Wed Oct  2 23:26:08 2013
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [net] i40e: adds FCoE configure option
Date: Thu, 03 Oct 2013 07:26:08 -
From: Vasu Dev 
X-Patchwork-Id: 11797

Adds FCoE config option I40E_FCOE, so that FCoE can be enabled as
needed but otherwise have it disabled by default.

This also eliminate multiple FCoE config checks, instead now just one
config check for CONFIG_I40E_FCOE.

The I40E FCoE was added with 3.17 kernel and therefore this patch
shall be applied to stable 3.17 kernel also.

CC: 
Signed-off-by: Vasu Dev 
Tested-by: Jim Young 

---
drivers/net/ethernet/intel/Kconfig   |   11 +++
  drivers/net/ethernet/intel/i40e/Makefile |2 +-
  drivers/net/ethernet/intel/i40e/i40e_osdep.h |4 ++--
  3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/Kconfig
b/drivers/net/ethernet/intel/Kconfig
index 5b8300a..4d61ef5 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -281,6 +281,17 @@ config I40E_DCB

   If unsure, say N.

+config I40E_FCOE
+   bool "Fibre Channel over Ethernet (FCoE)"
+   default n
+   depends on I40E && DCB && FCOE
+   ---help---
+ Say Y here 

RE: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-13 Thread Dev, Vasu
> -Original Message-
> >> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> >> > b/drivers/net/ethernet/intel/i40e/i40e_main.c
> >> > index a5f2660..a2572cc 100644
> >> > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> >> > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> >> > @@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
> >> > i40e_pf *pf, bool reinit)
> >> > }
> >> >  #endif /* CONFIG_I40E_DCB */
> >> >  #ifdef I40E_FCOE
> >> > -   ret = i40e_init_pf_fcoe(pf);
> >> > -   if (ret)
> >> > -   dev_info(>pdev->dev, "init_pf_fcoe failed: %d\n", ret);
> >> > +   if (pf->flags & I40E_FLAG_FCOE_ENABLED) {
> >> > +   ret = i40e_init_pf_fcoe(pf);
> >
> > Calling i40e_init_pf_fcoe() here conflicts with its
> I40E_FLAG_FCOE_ENABLED pre-condition since I40E_FLAG_FCOE_ENABLED is
> set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe() will never 
> get
> called.
> 
> I don't think so,  here ,i40e_reset_and_rebuild()  is not the only and the 
> first
> place that  i40e_init_pf_fcoe() is called, see i40e_probe(), that is the first
> chance.
> 
> i40e_probe()
> -->i40e_sw_init()
>  -->i40e_init_pf_fcoe()
> 
> And the I40E_FLAG_FCOE_ENABLED is possible be set by
> i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset action is
> to be done.
> 

It is set by i40e_init_pf_fcoe() and you are right that the modified call flow 
by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED anyway which could 
have prevented calling i40e_init_pf_fcoe() as I described above, so this is not 
an issue with the patch.

> BTW, the reason I post this patch is that we hit a bug, after setup vlan, the 
> PF
> is enabled to FCOE.
> 

Then that BUG would still remain un-fixed and calling i40e_init_pf_fcoe() under 
I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to fix anything. I 
mean I40E_FLAG_FCOE_ENABLED  condition will be true with "pf->hw.func_caps.fcoe 
== true" and otherwise calling i40e_init_pf_fcoe() simply returns back early on 
after checking "pf->hw.func_caps.fcoe == false", so how that bug is fixed here 
by added I40E_FLAG_FCOE_ENABLED  condition ? What is the bug ?

> >
> > Jeff Kirsher should be getting out a patch queued by me which adds
> I40E_FCoE Kbuild option, in that FCoE is disabled by default and  user could
> enable FCoE only if needed, that patch would do same of skipping
> i40e_init_pf_fcoe() whether FCoE capability in device enabled or not in
> default config.
> >
> 
> The following patch will not fix the above issue -- configuration of PF will 
> be
> changed via reset.
> How about the FCOE is configured and disabled by  i40e_fcoe_disable() ,
> then reset happens ?
> 

May be but if the BUG is due to FCoE being enabled then having it disabled in 
config will avoid the bug for non FCoE config option and once bug is understood 
then that has to be fixed for FCoE enabled config also as I asked above.

Thanks Ethan for detailed response.
Vasu

> > From patchwork Wed Oct  2 23:26:08 2013
> > Content-Type: text/plain; charset="utf-8"
> > MIME-Version: 1.0
> > Content-Transfer-Encoding: 7bit
> > Subject: [net] i40e: adds FCoE configure option
> > Date: Thu, 03 Oct 2013 07:26:08 -
> > From: Vasu Dev 
> > X-Patchwork-Id: 11797
> >
> > Adds FCoE config option I40E_FCOE, so that FCoE can be enabled as
> > needed but otherwise have it disabled by default.
> >
> > This also eliminate multiple FCoE config checks, instead now just one
> > config check for CONFIG_I40E_FCOE.
> >
> > The I40E FCoE was added with 3.17 kernel and therefore this patch
> > shall be applied to stable 3.17 kernel also.
> >
> > CC: 
> > Signed-off-by: Vasu Dev 
> > Tested-by: Jim Young 
> >
> > ---
> > drivers/net/ethernet/intel/Kconfig   |   11 +++
> >  drivers/net/ethernet/intel/i40e/Makefile |2 +-
> >  drivers/net/ethernet/intel/i40e/i40e_osdep.h |4 ++--
> >  3 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/Kconfig
> > b/drivers/net/ethernet/intel/Kconfig
> > index 5b8300a..4d61ef5 100644
> > --- a/drivers/net/ethernet/intel/Kconfig
> > +++ b/drivers/net/ethernet/intel/Kconfig
> > @@ -281,6 +281,17 @@ config I40E_DCB
> >
> >   If unsure, say N.
> >
> > +config I40E_FCOE
> > +   bool "Fibre Channel over Ethernet (FCoE)"
> > +   default n
> > +   depends on I40E && DCB && FCOE
> > +   ---help---
> > + Say Y here if you want to use Fibre Channel over Ethernet (FCoE)
> > + in the driver. This will create new netdev for exclusive FCoE
> > + use with XL710 FCoE offloads enabled.
> > +
> > + If unsure, say N.
> > +
> >  config I40EVF
> > tristate "Intel(R) XL710 X710 Virtual Function Ethernet support"
> > depends on PCI_MSI
> > diff --git a/drivers/net/ethernet/intel/i40e/Makefile
> > b/drivers/net/ethernet/intel/i40e/Makefile
> > index 4b94ddb..c405819 100644
> > --- 

RE: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-13 Thread Dev, Vasu
 -Original Message-
   diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
   b/drivers/net/ethernet/intel/i40e/i40e_main.c
   index a5f2660..a2572cc 100644
   --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
   +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
   @@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
   i40e_pf *pf, bool reinit)
   }
#endif /* CONFIG_I40E_DCB */
#ifdef I40E_FCOE
   -   ret = i40e_init_pf_fcoe(pf);
   -   if (ret)
   -   dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n, ret);
   +   if (pf-flags  I40E_FLAG_FCOE_ENABLED) {
   +   ret = i40e_init_pf_fcoe(pf);
 
  Calling i40e_init_pf_fcoe() here conflicts with its
 I40E_FLAG_FCOE_ENABLED pre-condition since I40E_FLAG_FCOE_ENABLED is
 set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe() will never 
 get
 called.
 
 I don't think so,  here ,i40e_reset_and_rebuild()  is not the only and the 
 first
 place that  i40e_init_pf_fcoe() is called, see i40e_probe(), that is the first
 chance.
 
 i40e_probe()
 --i40e_sw_init()
  --i40e_init_pf_fcoe()
 
 And the I40E_FLAG_FCOE_ENABLED is possible be set by
 i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset action is
 to be done.
 

It is set by i40e_init_pf_fcoe() and you are right that the modified call flow 
by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED anyway which could 
have prevented calling i40e_init_pf_fcoe() as I described above, so this is not 
an issue with the patch.

 BTW, the reason I post this patch is that we hit a bug, after setup vlan, the 
 PF
 is enabled to FCOE.
 

Then that BUG would still remain un-fixed and calling i40e_init_pf_fcoe() under 
I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to fix anything. I 
mean I40E_FLAG_FCOE_ENABLED  condition will be true with pf-hw.func_caps.fcoe 
== true and otherwise calling i40e_init_pf_fcoe() simply returns back early on 
after checking pf-hw.func_caps.fcoe == false, so how that bug is fixed here 
by added I40E_FLAG_FCOE_ENABLED  condition ? What is the bug ?

 
  Jeff Kirsher should be getting out a patch queued by me which adds
 I40E_FCoE Kbuild option, in that FCoE is disabled by default and  user could
 enable FCoE only if needed, that patch would do same of skipping
 i40e_init_pf_fcoe() whether FCoE capability in device enabled or not in
 default config.
 
 
 The following patch will not fix the above issue -- configuration of PF will 
 be
 changed via reset.
 How about the FCOE is configured and disabled by  i40e_fcoe_disable() ,
 then reset happens ?
 

May be but if the BUG is due to FCoE being enabled then having it disabled in 
config will avoid the bug for non FCoE config option and once bug is understood 
then that has to be fixed for FCoE enabled config also as I asked above.

Thanks Ethan for detailed response.
Vasu

  From patchwork Wed Oct  2 23:26:08 2013
  Content-Type: text/plain; charset=utf-8
  MIME-Version: 1.0
  Content-Transfer-Encoding: 7bit
  Subject: [net] i40e: adds FCoE configure option
  Date: Thu, 03 Oct 2013 07:26:08 -
  From: Vasu Dev vasu@intel.com
  X-Patchwork-Id: 11797
 
  Adds FCoE config option I40E_FCOE, so that FCoE can be enabled as
  needed but otherwise have it disabled by default.
 
  This also eliminate multiple FCoE config checks, instead now just one
  config check for CONFIG_I40E_FCOE.
 
  The I40E FCoE was added with 3.17 kernel and therefore this patch
  shall be applied to stable 3.17 kernel also.
 
  CC: sta...@vger.kernel.org
  Signed-off-by: Vasu Dev vasu@intel.com
  Tested-by: Jim Young jamesx.m.yo...@intel.com
 
  ---
  drivers/net/ethernet/intel/Kconfig   |   11 +++
   drivers/net/ethernet/intel/i40e/Makefile |2 +-
   drivers/net/ethernet/intel/i40e/i40e_osdep.h |4 ++--
   3 files changed, 14 insertions(+), 3 deletions(-)
 
  diff --git a/drivers/net/ethernet/intel/Kconfig
  b/drivers/net/ethernet/intel/Kconfig
  index 5b8300a..4d61ef5 100644
  --- a/drivers/net/ethernet/intel/Kconfig
  +++ b/drivers/net/ethernet/intel/Kconfig
  @@ -281,6 +281,17 @@ config I40E_DCB
 
If unsure, say N.
 
  +config I40E_FCOE
  +   bool Fibre Channel over Ethernet (FCoE)
  +   default n
  +   depends on I40E  DCB  FCOE
  +   ---help---
  + Say Y here if you want to use Fibre Channel over Ethernet (FCoE)
  + in the driver. This will create new netdev for exclusive FCoE
  + use with XL710 FCoE offloads enabled.
  +
  + If unsure, say N.
  +
   config I40EVF
  tristate Intel(R) XL710 X710 Virtual Function Ethernet support
  depends on PCI_MSI
  diff --git a/drivers/net/ethernet/intel/i40e/Makefile
  b/drivers/net/ethernet/intel/i40e/Makefile
  index 4b94ddb..c405819 100644
  --- a/drivers/net/ethernet/intel/i40e/Makefile
  +++ b/drivers/net/ethernet/intel/i40e/Makefile
  @@ -44,4 +44,4 @@ i40e-objs := i40e_main.o \
  i40e_virtchnl_pf.o
 
   

Re: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-13 Thread ethan zhao

Vasu,

On 2015/1/14 3:38, Dev, Vasu wrote:

-Original Message-

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a5f2660..a2572cc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
i40e_pf *pf, bool reinit)
 }
  #endif /* CONFIG_I40E_DCB */
  #ifdef I40E_FCOE
-   ret = i40e_init_pf_fcoe(pf);
-   if (ret)
-   dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n, ret);
+   if (pf-flags  I40E_FLAG_FCOE_ENABLED) {
+   ret = i40e_init_pf_fcoe(pf);

Calling i40e_init_pf_fcoe() here conflicts with its

I40E_FLAG_FCOE_ENABLED pre-condition since I40E_FLAG_FCOE_ENABLED is
set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe() will never get
called.

I don't think so,  here ,i40e_reset_and_rebuild()  is not the only and the first
place that  i40e_init_pf_fcoe() is called, see i40e_probe(), that is the first
chance.

i40e_probe()
--i40e_sw_init()
  --i40e_init_pf_fcoe()

And the I40E_FLAG_FCOE_ENABLED is possible be set by
i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset action is
to be done.


It is set by i40e_init_pf_fcoe() and you are right that the modified call flow 
by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED anyway which could 
have prevented calling i40e_init_pf_fcoe() as I described above, so this is not 
an issue with the patch.


BTW, the reason I post this patch is that we hit a bug, after setup vlan, the PF
is enabled to FCOE.


Then that BUG would still remain un-fixed and calling i40e_init_pf_fcoe() under I40E_FLAG_FCOE_ENABLED  
flag really won't affect call flow to fix anything. I mean I40E_FLAG_FCOE_ENABLED  condition will be true 
with pf-hw.func_caps.fcoe == true and otherwise calling i40e_init_pf_fcoe() simply 
returns back early on after checking pf-hw.func_caps.fcoe == false, so how that bug is 
fixed here by added I40E_FLAG_FCOE_ENABLED  condition ? What is the bug ?
 The func_caps.fcoe is assigned by following call path, under our test 
environment,


 i40e_probe()
  -i40e_get_capabilities()
 -i40e_aq_discover_capabilities()
-i40e_parse_discover_capabilities()

 Or

 i40e_reset_and_rebuild()
  -i40e_get_capabilities()
-i40e_aq_discover_capabilities()
  -i40e_parse_discover_capabilities()

 Under our test environment, the pf-hw.func_caps.fcoe is true. so if 
i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic test.

 And then i40e_init_pf_fcoe() is to be called,

 While if (!pf-hw.func_caps.fcoe) wouldn't return,

 So  pf-flags is set to I40E_FLAG_FCOE_ENABLED.

 With my patch,  i40e_init_pf_fcoe() is only called after 
I40E_FLAG_FCOE_ENABLED is set before reset.


Enable FCOE in i40e_probe() or not is another issue.


Thanks,
Ethan





Jeff Kirsher should be getting out a patch queued by me which adds

I40E_FCoE Kbuild option, in that FCoE is disabled by default and  user could
enable FCoE only if needed, that patch would do same of skipping
i40e_init_pf_fcoe() whether FCoE capability in device enabled or not in
default config.
The following patch will not fix the above issue -- configuration of PF will be
changed via reset.
How about the FCOE is configured and disabled by  i40e_fcoe_disable() ,
then reset happens ?


May be but if the BUG is due to FCoE being enabled then having it disabled in 
config will avoid the bug for non FCoE config option and once bug is understood 
then that has to be fixed for FCoE enabled config also as I asked above.

Thanks Ethan for detailed response.
Vasu


 From patchwork Wed Oct  2 23:26:08 2013
Content-Type: text/plain; charset=utf-8
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [net] i40e: adds FCoE configure option
Date: Thu, 03 Oct 2013 07:26:08 -
From: Vasu Dev vasu@intel.com
X-Patchwork-Id: 11797

Adds FCoE config option I40E_FCOE, so that FCoE can be enabled as
needed but otherwise have it disabled by default.

This also eliminate multiple FCoE config checks, instead now just one
config check for CONFIG_I40E_FCOE.

The I40E FCoE was added with 3.17 kernel and therefore this patch
shall be applied to stable 3.17 kernel also.

CC: sta...@vger.kernel.org
Signed-off-by: Vasu Dev vasu@intel.com
Tested-by: Jim Young jamesx.m.yo...@intel.com

---
drivers/net/ethernet/intel/Kconfig   |   11 +++
  drivers/net/ethernet/intel/i40e/Makefile |2 +-
  drivers/net/ethernet/intel/i40e/i40e_osdep.h |4 ++--
  3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/Kconfig
b/drivers/net/ethernet/intel/Kconfig
index 5b8300a..4d61ef5 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -281,6 +281,17 @@ config I40E_DCB

   If unsure, say N.

+config I40E_FCOE
+   bool Fibre Channel over Ethernet (FCoE)
+   default n
+   depends on I40E  

Re: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-12 Thread Ethan Zhao
Vasu,

On Sat, Jan 10, 2015 at 2:18 AM, Dev, Vasu  wrote:
>> -Original Message-
>> From: Ronciak, John
>> Sent: Friday, January 09, 2015 8:42 AM
>> To: Ethan Zhao; Kirsher, Jeffrey T; Brandeburg, Jesse; Allan, Bruce W;
>> Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick, Matthew;
>> Williams, Mitch A; Dev, Vasu; Parikh, Neerav
>> Cc: Linux NICS; e1000-de...@lists.sourceforge.net; net...@vger.kernel.org;
>> linux-kernel@vger.kernel.org; ethan.ker...@gmail.com;
>> brian.m...@oracle.com
>> Subject: RE: [PATCH] i40e: don't enable and init FCOE by default when do PF
>> reset
>>
>> Adding Vasu and Neerav
>>
>> Cheers,
>> John
>>
>> > -Original Message-
>> > From: Ethan Zhao [mailto:ethan.z...@oracle.com]
>> > Sent: Friday, January 9, 2015 8:38 AM
>> > To: Kirsher, Jeffrey T; Brandeburg, Jesse; Allan, Bruce W; Wyborny,
>> > Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick, Matthew; Ronciak,
>> > John; Williams, Mitch A
>> > Cc: Linux NICS; e1000-de...@lists.sourceforge.net;
>> > net...@vger.kernel.org; linux-kernel@vger.kernel.org;
>> > ethan.ker...@gmail.com; brian.m...@oracle.com; Ethan Zhao
>> > Subject: [PATCH] i40e: don't enable and init FCOE by default when do
>> > PF reset
>> >
>> > While do PF reset with function i40e_reset_and_rebuild(), it will call
>> > i40e_init_pf_fcoe() by default if FCOE is defined, thus if the PF is
>> > resetted, FCOE will be enabled whatever it was - enabled or not.
>> >
>> > Such bug might be hit when PF resumes from suspend, run diagnostic
>> > test with ethtool, setup VLAN etc.
>> >
>> > Passed building with v3.19-rc3.
>> >
>> > Signed-off-by: Ethan Zhao 
>> > ---
>> >  drivers/net/ethernet/intel/i40e/i40e_main.c | 9 ++---
>> >  1 file changed, 6 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> > b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> > index a5f2660..a2572cc 100644
>> > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> > @@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
>> > i40e_pf *pf, bool reinit)
>> > }
>> >  #endif /* CONFIG_I40E_DCB */
>> >  #ifdef I40E_FCOE
>> > -   ret = i40e_init_pf_fcoe(pf);
>> > -   if (ret)
>> > -   dev_info(>pdev->dev, "init_pf_fcoe failed: %d\n", ret);
>> > +   if (pf->flags & I40E_FLAG_FCOE_ENABLED) {
>> > +   ret = i40e_init_pf_fcoe(pf);
>
> Calling i40e_init_pf_fcoe() here conflicts with its I40E_FLAG_FCOE_ENABLED 
> pre-condition since I40E_FLAG_FCOE_ENABLED is set by very same 
> i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe() will never get called.

I don't think so,  here ,i40e_reset_and_rebuild()  is not the only and
the first place that  i40e_init_pf_fcoe() is called,
see i40e_probe(), that is the first chance.

i40e_probe()
-->i40e_sw_init()
 -->i40e_init_pf_fcoe()

And the I40E_FLAG_FCOE_ENABLED is possible be set by
i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset
action is to be done.

BTW, the reason I post this patch is that we hit a bug, after setup
vlan, the PF is enabled to FCOE.

>
> Jeff Kirsher should be getting out a patch queued by me which adds I40E_FCoE 
> Kbuild option, in that FCoE is disabled by default and  user could enable 
> FCoE only if needed, that patch would do same of skipping i40e_init_pf_fcoe() 
> whether FCoE capability in device enabled or not in default config.
>

The following patch will not fix the above issue -- configuration of
PF will be changed via reset.
How about the FCOE is configured and disabled by  i40e_fcoe_disable()
, then reset happens ?

> From patchwork Wed Oct  2 23:26:08 2013
> Content-Type: text/plain; charset="utf-8"
> MIME-Version: 1.0
> Content-Transfer-Encoding: 7bit
> Subject: [net] i40e: adds FCoE configure option
> Date: Thu, 03 Oct 2013 07:26:08 -
> From: Vasu Dev 
> X-Patchwork-Id: 11797
>
> Adds FCoE config option I40E_FCOE, so that FCoE can be enabled
> as needed but otherwise have it disabled by default.
>
> This also eliminate multiple FCoE config checks, instead now just
> one config check for CONFIG_I40E_FCOE.
>
> The I40E FCoE was added with 3.17 kernel and therefore this patch
> shall be applied to stable 3.17 kernel also.
>
> CC: 
> Signed-off-by: Vasu Dev 
> Tested-by: Jim Young 
>
> ---
> drivers/net/ethernet/intel/Kconfig   |   11 

Re: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-12 Thread Ethan Zhao
Vasu,

On Sat, Jan 10, 2015 at 2:18 AM, Dev, Vasu vasu@intel.com wrote:
 -Original Message-
 From: Ronciak, John
 Sent: Friday, January 09, 2015 8:42 AM
 To: Ethan Zhao; Kirsher, Jeffrey T; Brandeburg, Jesse; Allan, Bruce W;
 Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick, Matthew;
 Williams, Mitch A; Dev, Vasu; Parikh, Neerav
 Cc: Linux NICS; e1000-de...@lists.sourceforge.net; net...@vger.kernel.org;
 linux-kernel@vger.kernel.org; ethan.ker...@gmail.com;
 brian.m...@oracle.com
 Subject: RE: [PATCH] i40e: don't enable and init FCOE by default when do PF
 reset

 Adding Vasu and Neerav

 Cheers,
 John

  -Original Message-
  From: Ethan Zhao [mailto:ethan.z...@oracle.com]
  Sent: Friday, January 9, 2015 8:38 AM
  To: Kirsher, Jeffrey T; Brandeburg, Jesse; Allan, Bruce W; Wyborny,
  Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick, Matthew; Ronciak,
  John; Williams, Mitch A
  Cc: Linux NICS; e1000-de...@lists.sourceforge.net;
  net...@vger.kernel.org; linux-kernel@vger.kernel.org;
  ethan.ker...@gmail.com; brian.m...@oracle.com; Ethan Zhao
  Subject: [PATCH] i40e: don't enable and init FCOE by default when do
  PF reset
 
  While do PF reset with function i40e_reset_and_rebuild(), it will call
  i40e_init_pf_fcoe() by default if FCOE is defined, thus if the PF is
  resetted, FCOE will be enabled whatever it was - enabled or not.
 
  Such bug might be hit when PF resumes from suspend, run diagnostic
  test with ethtool, setup VLAN etc.
 
  Passed building with v3.19-rc3.
 
  Signed-off-by: Ethan Zhao ethan.z...@oracle.com
  ---
   drivers/net/ethernet/intel/i40e/i40e_main.c | 9 ++---
   1 file changed, 6 insertions(+), 3 deletions(-)
 
  diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
  b/drivers/net/ethernet/intel/i40e/i40e_main.c
  index a5f2660..a2572cc 100644
  --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
  +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
  @@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
  i40e_pf *pf, bool reinit)
  }
   #endif /* CONFIG_I40E_DCB */
   #ifdef I40E_FCOE
  -   ret = i40e_init_pf_fcoe(pf);
  -   if (ret)
  -   dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n, ret);
  +   if (pf-flags  I40E_FLAG_FCOE_ENABLED) {
  +   ret = i40e_init_pf_fcoe(pf);

 Calling i40e_init_pf_fcoe() here conflicts with its I40E_FLAG_FCOE_ENABLED 
 pre-condition since I40E_FLAG_FCOE_ENABLED is set by very same 
 i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe() will never get called.

I don't think so,  here ,i40e_reset_and_rebuild()  is not the only and
the first place that  i40e_init_pf_fcoe() is called,
see i40e_probe(), that is the first chance.

i40e_probe()
--i40e_sw_init()
 --i40e_init_pf_fcoe()

And the I40E_FLAG_FCOE_ENABLED is possible be set by
i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset
action is to be done.

BTW, the reason I post this patch is that we hit a bug, after setup
vlan, the PF is enabled to FCOE.


 Jeff Kirsher should be getting out a patch queued by me which adds I40E_FCoE 
 Kbuild option, in that FCoE is disabled by default and  user could enable 
 FCoE only if needed, that patch would do same of skipping i40e_init_pf_fcoe() 
 whether FCoE capability in device enabled or not in default config.


The following patch will not fix the above issue -- configuration of
PF will be changed via reset.
How about the FCOE is configured and disabled by  i40e_fcoe_disable()
, then reset happens ?

 From patchwork Wed Oct  2 23:26:08 2013
 Content-Type: text/plain; charset=utf-8
 MIME-Version: 1.0
 Content-Transfer-Encoding: 7bit
 Subject: [net] i40e: adds FCoE configure option
 Date: Thu, 03 Oct 2013 07:26:08 -
 From: Vasu Dev vasu@intel.com
 X-Patchwork-Id: 11797

 Adds FCoE config option I40E_FCOE, so that FCoE can be enabled
 as needed but otherwise have it disabled by default.

 This also eliminate multiple FCoE config checks, instead now just
 one config check for CONFIG_I40E_FCOE.

 The I40E FCoE was added with 3.17 kernel and therefore this patch
 shall be applied to stable 3.17 kernel also.

 CC: sta...@vger.kernel.org
 Signed-off-by: Vasu Dev vasu@intel.com
 Tested-by: Jim Young jamesx.m.yo...@intel.com

 ---
 drivers/net/ethernet/intel/Kconfig   |   11 +++
  drivers/net/ethernet/intel/i40e/Makefile |2 +-
  drivers/net/ethernet/intel/i40e/i40e_osdep.h |4 ++--
  3 files changed, 14 insertions(+), 3 deletions(-)

 diff --git a/drivers/net/ethernet/intel/Kconfig 
 b/drivers/net/ethernet/intel/Kconfig
 index 5b8300a..4d61ef5 100644
 --- a/drivers/net/ethernet/intel/Kconfig
 +++ b/drivers/net/ethernet/intel/Kconfig
 @@ -281,6 +281,17 @@ config I40E_DCB

   If unsure, say N.

 +config I40E_FCOE
 +   bool Fibre Channel over Ethernet (FCoE)
 +   default n
 +   depends on I40E  DCB  FCOE
 +   ---help---
 + Say Y here if you want to use Fibre Channel over Ethernet (FCoE)
 + in the driver

RE: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-09 Thread Dev, Vasu
> -Original Message-
> From: Ronciak, John
> Sent: Friday, January 09, 2015 8:42 AM
> To: Ethan Zhao; Kirsher, Jeffrey T; Brandeburg, Jesse; Allan, Bruce W;
> Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick, Matthew;
> Williams, Mitch A; Dev, Vasu; Parikh, Neerav
> Cc: Linux NICS; e1000-de...@lists.sourceforge.net; net...@vger.kernel.org;
> linux-kernel@vger.kernel.org; ethan.ker...@gmail.com;
> brian.m...@oracle.com
> Subject: RE: [PATCH] i40e: don't enable and init FCOE by default when do PF
> reset
> 
> Adding Vasu and Neerav
> 
> Cheers,
> John
> 
> > -Original Message-
> > From: Ethan Zhao [mailto:ethan.z...@oracle.com]
> > Sent: Friday, January 9, 2015 8:38 AM
> > To: Kirsher, Jeffrey T; Brandeburg, Jesse; Allan, Bruce W; Wyborny,
> > Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick, Matthew; Ronciak,
> > John; Williams, Mitch A
> > Cc: Linux NICS; e1000-de...@lists.sourceforge.net;
> > net...@vger.kernel.org; linux-kernel@vger.kernel.org;
> > ethan.ker...@gmail.com; brian.m...@oracle.com; Ethan Zhao
> > Subject: [PATCH] i40e: don't enable and init FCOE by default when do
> > PF reset
> >
> > While do PF reset with function i40e_reset_and_rebuild(), it will call
> > i40e_init_pf_fcoe() by default if FCOE is defined, thus if the PF is
> > resetted, FCOE will be enabled whatever it was - enabled or not.
> >
> > Such bug might be hit when PF resumes from suspend, run diagnostic
> > test with ethtool, setup VLAN etc.
> >
> > Passed building with v3.19-rc3.
> >
> > Signed-off-by: Ethan Zhao 
> > ---
> >  drivers/net/ethernet/intel/i40e/i40e_main.c | 9 ++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> > b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > index a5f2660..a2572cc 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > @@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
> > i40e_pf *pf, bool reinit)
> > }
> >  #endif /* CONFIG_I40E_DCB */
> >  #ifdef I40E_FCOE
> > -   ret = i40e_init_pf_fcoe(pf);
> > -   if (ret)
> > -   dev_info(>pdev->dev, "init_pf_fcoe failed: %d\n", ret);
> > +   if (pf->flags & I40E_FLAG_FCOE_ENABLED) {
> > +   ret = i40e_init_pf_fcoe(pf);

Calling i40e_init_pf_fcoe() here conflicts with its I40E_FLAG_FCOE_ENABLED 
pre-condition since I40E_FLAG_FCOE_ENABLED is set by very same 
i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe() will never get called.

Jeff Kirsher should be getting out a patch queued by me which adds I40E_FCoE 
Kbuild option, in that FCoE is disabled by default and  user could enable FCoE 
only if needed, that patch would do same of skipping i40e_init_pf_fcoe() 
whether FCoE capability in device enabled or not in default config.

>From patchwork Wed Oct  2 23:26:08 2013
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [net] i40e: adds FCoE configure option
Date: Thu, 03 Oct 2013 07:26:08 -
From: Vasu Dev 
X-Patchwork-Id: 11797

Adds FCoE config option I40E_FCOE, so that FCoE can be enabled
as needed but otherwise have it disabled by default.

This also eliminate multiple FCoE config checks, instead now just
one config check for CONFIG_I40E_FCOE.

The I40E FCoE was added with 3.17 kernel and therefore this patch
shall be applied to stable 3.17 kernel also.

CC: 
Signed-off-by: Vasu Dev 
Tested-by: Jim Young 

---
drivers/net/ethernet/intel/Kconfig   |   11 +++
 drivers/net/ethernet/intel/i40e/Makefile |2 +-
 drivers/net/ethernet/intel/i40e/i40e_osdep.h |4 ++--
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/Kconfig 
b/drivers/net/ethernet/intel/Kconfig
index 5b8300a..4d61ef5 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -281,6 +281,17 @@ config I40E_DCB
 
  If unsure, say N.
 
+config I40E_FCOE
+   bool "Fibre Channel over Ethernet (FCoE)"
+   default n
+   depends on I40E && DCB && FCOE
+   ---help---
+ Say Y here if you want to use Fibre Channel over Ethernet (FCoE)
+ in the driver. This will create new netdev for exclusive FCoE
+ use with XL710 FCoE offloads enabled.
+
+ If unsure, say N.
+
 config I40EVF
tristate "Intel(R) XL710 X710 Virtual Function Ethernet support"
depends on PCI_MSI
diff --git a/drivers/net/ethernet/intel/i40e/Makefile 
b/drivers/net/ethernet/intel/i40e/Makefile
index 4b94ddb..c405819 100644
--- a/drivers/net/ethernet/intel/i40e/

RE: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-09 Thread Ronciak, John
Adding Vasu and Neerav

Cheers,
John

> -Original Message-
> From: Ethan Zhao [mailto:ethan.z...@oracle.com]
> Sent: Friday, January 9, 2015 8:38 AM
> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Allan, Bruce W; Wyborny, Carolyn;
> Skidmore, Donald C; Rose, Gregory V; Vick, Matthew; Ronciak, John;
> Williams, Mitch A
> Cc: Linux NICS; e1000-de...@lists.sourceforge.net; net...@vger.kernel.org;
> linux-kernel@vger.kernel.org; ethan.ker...@gmail.com;
> brian.m...@oracle.com; Ethan Zhao
> Subject: [PATCH] i40e: don't enable and init FCOE by default when do PF
> reset
> 
> While do PF reset with function i40e_reset_and_rebuild(), it will call
> i40e_init_pf_fcoe() by default if FCOE is defined, thus if the PF is resetted,
> FCOE will be enabled whatever it was - enabled or not.
> 
> Such bug might be hit when PF resumes from suspend, run diagnostic test
> with ethtool, setup VLAN etc.
> 
> Passed building with v3.19-rc3.
> 
> Signed-off-by: Ethan Zhao 
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index a5f2660..a2572cc 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct i40e_pf
> *pf, bool reinit)
>   }
>  #endif /* CONFIG_I40E_DCB */
>  #ifdef I40E_FCOE
> - ret = i40e_init_pf_fcoe(pf);
> - if (ret)
> - dev_info(>pdev->dev, "init_pf_fcoe failed: %d\n", ret);
> + if (pf->flags & I40E_FLAG_FCOE_ENABLED) {
> + ret = i40e_init_pf_fcoe(pf);
> + if (ret)
> + dev_info(>pdev->dev,
> +  "init_pf_fcoe failed: %d\n", ret);
> + }
> 
>  #endif
>   /* do basic switch setup */
> --
> 1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-09 Thread Ronciak, John
Adding Vasu and Neerav

Cheers,
John

 -Original Message-
 From: Ethan Zhao [mailto:ethan.z...@oracle.com]
 Sent: Friday, January 9, 2015 8:38 AM
 To: Kirsher, Jeffrey T; Brandeburg, Jesse; Allan, Bruce W; Wyborny, Carolyn;
 Skidmore, Donald C; Rose, Gregory V; Vick, Matthew; Ronciak, John;
 Williams, Mitch A
 Cc: Linux NICS; e1000-de...@lists.sourceforge.net; net...@vger.kernel.org;
 linux-kernel@vger.kernel.org; ethan.ker...@gmail.com;
 brian.m...@oracle.com; Ethan Zhao
 Subject: [PATCH] i40e: don't enable and init FCOE by default when do PF
 reset
 
 While do PF reset with function i40e_reset_and_rebuild(), it will call
 i40e_init_pf_fcoe() by default if FCOE is defined, thus if the PF is resetted,
 FCOE will be enabled whatever it was - enabled or not.
 
 Such bug might be hit when PF resumes from suspend, run diagnostic test
 with ethtool, setup VLAN etc.
 
 Passed building with v3.19-rc3.
 
 Signed-off-by: Ethan Zhao ethan.z...@oracle.com
 ---
  drivers/net/ethernet/intel/i40e/i40e_main.c | 9 ++---
  1 file changed, 6 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
 b/drivers/net/ethernet/intel/i40e/i40e_main.c
 index a5f2660..a2572cc 100644
 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
 +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
 @@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct i40e_pf
 *pf, bool reinit)
   }
  #endif /* CONFIG_I40E_DCB */
  #ifdef I40E_FCOE
 - ret = i40e_init_pf_fcoe(pf);
 - if (ret)
 - dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n, ret);
 + if (pf-flags  I40E_FLAG_FCOE_ENABLED) {
 + ret = i40e_init_pf_fcoe(pf);
 + if (ret)
 + dev_info(pf-pdev-dev,
 +  init_pf_fcoe failed: %d\n, ret);
 + }
 
  #endif
   /* do basic switch setup */
 --
 1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] i40e: don't enable and init FCOE by default when do PF reset

2015-01-09 Thread Dev, Vasu
 -Original Message-
 From: Ronciak, John
 Sent: Friday, January 09, 2015 8:42 AM
 To: Ethan Zhao; Kirsher, Jeffrey T; Brandeburg, Jesse; Allan, Bruce W;
 Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick, Matthew;
 Williams, Mitch A; Dev, Vasu; Parikh, Neerav
 Cc: Linux NICS; e1000-de...@lists.sourceforge.net; net...@vger.kernel.org;
 linux-kernel@vger.kernel.org; ethan.ker...@gmail.com;
 brian.m...@oracle.com
 Subject: RE: [PATCH] i40e: don't enable and init FCOE by default when do PF
 reset
 
 Adding Vasu and Neerav
 
 Cheers,
 John
 
  -Original Message-
  From: Ethan Zhao [mailto:ethan.z...@oracle.com]
  Sent: Friday, January 9, 2015 8:38 AM
  To: Kirsher, Jeffrey T; Brandeburg, Jesse; Allan, Bruce W; Wyborny,
  Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick, Matthew; Ronciak,
  John; Williams, Mitch A
  Cc: Linux NICS; e1000-de...@lists.sourceforge.net;
  net...@vger.kernel.org; linux-kernel@vger.kernel.org;
  ethan.ker...@gmail.com; brian.m...@oracle.com; Ethan Zhao
  Subject: [PATCH] i40e: don't enable and init FCOE by default when do
  PF reset
 
  While do PF reset with function i40e_reset_and_rebuild(), it will call
  i40e_init_pf_fcoe() by default if FCOE is defined, thus if the PF is
  resetted, FCOE will be enabled whatever it was - enabled or not.
 
  Such bug might be hit when PF resumes from suspend, run diagnostic
  test with ethtool, setup VLAN etc.
 
  Passed building with v3.19-rc3.
 
  Signed-off-by: Ethan Zhao ethan.z...@oracle.com
  ---
   drivers/net/ethernet/intel/i40e/i40e_main.c | 9 ++---
   1 file changed, 6 insertions(+), 3 deletions(-)
 
  diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
  b/drivers/net/ethernet/intel/i40e/i40e_main.c
  index a5f2660..a2572cc 100644
  --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
  +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
  @@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
  i40e_pf *pf, bool reinit)
  }
   #endif /* CONFIG_I40E_DCB */
   #ifdef I40E_FCOE
  -   ret = i40e_init_pf_fcoe(pf);
  -   if (ret)
  -   dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n, ret);
  +   if (pf-flags  I40E_FLAG_FCOE_ENABLED) {
  +   ret = i40e_init_pf_fcoe(pf);

Calling i40e_init_pf_fcoe() here conflicts with its I40E_FLAG_FCOE_ENABLED 
pre-condition since I40E_FLAG_FCOE_ENABLED is set by very same 
i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe() will never get called.

Jeff Kirsher should be getting out a patch queued by me which adds I40E_FCoE 
Kbuild option, in that FCoE is disabled by default and  user could enable FCoE 
only if needed, that patch would do same of skipping i40e_init_pf_fcoe() 
whether FCoE capability in device enabled or not in default config.

From patchwork Wed Oct  2 23:26:08 2013
Content-Type: text/plain; charset=utf-8
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [net] i40e: adds FCoE configure option
Date: Thu, 03 Oct 2013 07:26:08 -
From: Vasu Dev vasu@intel.com
X-Patchwork-Id: 11797

Adds FCoE config option I40E_FCOE, so that FCoE can be enabled
as needed but otherwise have it disabled by default.

This also eliminate multiple FCoE config checks, instead now just
one config check for CONFIG_I40E_FCOE.

The I40E FCoE was added with 3.17 kernel and therefore this patch
shall be applied to stable 3.17 kernel also.

CC: sta...@vger.kernel.org
Signed-off-by: Vasu Dev vasu@intel.com
Tested-by: Jim Young jamesx.m.yo...@intel.com

---
drivers/net/ethernet/intel/Kconfig   |   11 +++
 drivers/net/ethernet/intel/i40e/Makefile |2 +-
 drivers/net/ethernet/intel/i40e/i40e_osdep.h |4 ++--
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/Kconfig 
b/drivers/net/ethernet/intel/Kconfig
index 5b8300a..4d61ef5 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -281,6 +281,17 @@ config I40E_DCB
 
  If unsure, say N.
 
+config I40E_FCOE
+   bool Fibre Channel over Ethernet (FCoE)
+   default n
+   depends on I40E  DCB  FCOE
+   ---help---
+ Say Y here if you want to use Fibre Channel over Ethernet (FCoE)
+ in the driver. This will create new netdev for exclusive FCoE
+ use with XL710 FCoE offloads enabled.
+
+ If unsure, say N.
+
 config I40EVF
tristate Intel(R) XL710 X710 Virtual Function Ethernet support
depends on PCI_MSI
diff --git a/drivers/net/ethernet/intel/i40e/Makefile 
b/drivers/net/ethernet/intel/i40e/Makefile
index 4b94ddb..c405819 100644
--- a/drivers/net/ethernet/intel/i40e/Makefile
+++ b/drivers/net/ethernet/intel/i40e/Makefile
@@ -44,4 +44,4 @@ i40e-objs := i40e_main.o \
i40e_virtchnl_pf.o
 
 i40e-$(CONFIG_I40E_DCB) += i40e_dcb.o i40e_dcb_nl.o
-i40e-$(CONFIG_FCOE:m=y) += i40e_fcoe.o
+i40e-$(CONFIG_I40E_FCOE) += i40e_fcoe.o
diff --git a/drivers/net/ethernet/intel/i40e/i40e_osdep.h 
b/drivers/net/ethernet/intel/i40e/i40e_osdep.h
index