2014-01-15 14:14:21

by ethan zhao

[permalink] [raw]
Subject: [PATCH 1/2 v3] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63

Because ixgbe driver limit the max number of VF functions could be enabled
to 63, so define one macro IXGBE_MAX_VFS_DRV_LIMIT and cleanup the const 63
in code.

v2: fix a typo.
v3: fix a encoding issue.

Signed-off-by: Ethan Zhao <[email protected]>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h | 5 +++++
3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 0ade0cd..47e9b44 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4818,7 +4818,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
#ifdef CONFIG_PCI_IOV
/* assign number of SR-IOV VFs */
if (hw->mac.type != ixgbe_mac_82598EB)
- adapter->num_vfs = (max_vfs > 63) ? 0 : max_vfs;
+ adapter->num_vfs = (max_vfs > IXGBE_MAX_VFS_DRV_LIMIT) ? 0 : max_vfs;

#endif
/* enable itr by default in dynamic mode */
@@ -7640,7 +7640,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
ixgbe_init_mbx_params_pf(hw);
memcpy(&hw->mbx.ops, ii->mbx_ops, sizeof(hw->mbx.ops));
ixgbe_enable_sriov(adapter);
- pci_sriov_set_totalvfs(pdev, 63);
+ pci_sriov_set_totalvfs(pdev, IXGBE_MAX_VFS_DRV_LIMIT);
skip_sriov:

#endif
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 276d7b1..6925979 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -152,7 +152,8 @@ void ixgbe_enable_sriov(struct ixgbe_adapter *adapter)
* physical function. If the user requests greater thn
* 63 VFs then it is an error - reset to default of zero.
*/
- adapter->num_vfs = min_t(unsigned int, adapter->num_vfs, 63);
+ adapter->num_vfs = min_t(unsigned int,
+ adapter->num_vfs, IXGBE_MAX_VFS_DRV_LIMIT);

err = pci_enable_sriov(adapter->pdev, adapter->num_vfs);
if (err) {
@@ -259,7 +260,7 @@ static int ixgbe_pci_sriov_enable(struct pci_dev *dev, int num_vfs)
* PF. The PCI bus driver already checks for other values out of
* range.
*/
- if (num_vfs > 63) {
+ if (num_vfs > IXGBE_MAX_VFS_DRV_LIMIT) {
err = -EPERM;
goto err_out;
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
index 4713f9f..2593666 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
@@ -28,6 +28,11 @@
#ifndef _IXGBE_SRIOV_H_
#define _IXGBE_SRIOV_H_

+/* ixgbe driver limit the max number of VFs could be enabled to
+ * 63 (IXGBE_MAX_VF_FUNCTIONS - 1)
+ */
+#define IXGBE_MAX_VFS_DRV_LIMIT (IXGBE_MAX_VF_FUNCTIONS - 1)
+
void ixgbe_restore_vf_multicasts(struct ixgbe_adapter *adapter);
void ixgbe_msg_task(struct ixgbe_adapter *adapter);
int ixgbe_vf_configuration(struct pci_dev *pdev, unsigned int event_mask);
--
1.8.3.4 (Apple Git-47)


2014-01-15 22:03:05

by Brown, Aaron F

[permalink] [raw]
Subject: Re: [PATCH 1/2 v3] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63

On Wed, 2014-01-15 at 22:12 +0800, Ethan Zhao wrote:
> Because ixgbe driver limit the max number of VF functions could be enabled
> to 63, so define one macro IXGBE_MAX_VFS_DRV_LIMIT and cleanup the const 63
> in code.
>
> v2: fix a typo.
> v3: fix a encoding issue.
>
> Signed-off-by: Ethan Zhao <[email protected]>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +++--
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h | 5 +++++
> 3 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 0ade0cd..47e9b44 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -4818,7 +4818,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
> #ifdef CONFIG_PCI_IOV
> /* assign number of SR-IOV VFs */
> if (hw->mac.type != ixgbe_mac_82598EB)
> - adapter->num_vfs = (max_vfs > 63) ? 0 : max_vfs;


Unfortunately the if statement got changed considerably with a recent
commit:

commit 170e85430bcbe4d18e81b5a70bb163c741381092
ixgbe: add warning when max_vfs is out of range.

And the pattern no longer exists to make a match. In other words, this
patch no longer applies to net-next and I have to ask you for yet
another spin if you still want to squash the magic number.

Thanks,
Aaron
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-01-16 01:27:54

by ethan zhao

[permalink] [raw]
Subject: Re: [PATCH 1/2 v3] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63

On Thu, Jan 16, 2014 at 6:00 AM, Brown, Aaron F <[email protected]> wrote:
> On Wed, 2014-01-15 at 22:12 +0800, Ethan Zhao wrote:
>> Because ixgbe driver limit the max number of VF functions could be enabled
>> to 63, so define one macro IXGBE_MAX_VFS_DRV_LIMIT and cleanup the const 63
>> in code.
>>
>> v2: fix a typo.
>> v3: fix a encoding issue.
>>
>> Signed-off-by: Ethan Zhao <[email protected]>
>> ---
>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
>> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +++--
>> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h | 5 +++++
>> 3 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> index 0ade0cd..47e9b44 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> @@ -4818,7 +4818,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
>> #ifdef CONFIG_PCI_IOV
>> /* assign number of SR-IOV VFs */
>> if (hw->mac.type != ixgbe_mac_82598EB)
>> - adapter->num_vfs = (max_vfs > 63) ? 0 : max_vfs;
>
>
> Unfortunately the if statement got changed considerably with a recent
> commit:
>
> commit 170e85430bcbe4d18e81b5a70bb163c741381092
> ixgbe: add warning when max_vfs is out of range.
>
> And the pattern no longer exists to make a match. In other words, this
> patch no longer applies to net-next and I have to ask you for yet
> another spin if you still want to squash the magic number.

It's not a good news. Our distro is waiting for this patch showing up in stable.
OK, info me if there is a windows for me to revise the patch.

Thanks,
Ethan

>
> Thanks,
> Aaron

2014-01-16 01:54:31

by Brown, Aaron F

[permalink] [raw]
Subject: Re: [PATCH 1/2 v3] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63

On Thu, 2014-01-16 at 09:27 +0800, Ethan Zhao wrote:
> On Thu, Jan 16, 2014 at 6:00 AM, Brown, Aaron F <[email protected]> wrote:
> > On Wed, 2014-01-15 at 22:12 +0800, Ethan Zhao wrote:
> >> Because ixgbe driver limit the max number of VF functions could be enabled
> >> to 63, so define one macro IXGBE_MAX_VFS_DRV_LIMIT and cleanup the const 63
> >> in code.
> >>
> >> v2: fix a typo.
> >> v3: fix a encoding issue.
> >>
> >> Signed-off-by: Ethan Zhao <[email protected]>
> >> ---
> >> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
> >> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +++--
> >> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h | 5 +++++
> >> 3 files changed, 10 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >> index 0ade0cd..47e9b44 100644
> >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >> @@ -4818,7 +4818,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
> >> #ifdef CONFIG_PCI_IOV
> >> /* assign number of SR-IOV VFs */
> >> if (hw->mac.type != ixgbe_mac_82598EB)
> >> - adapter->num_vfs = (max_vfs > 63) ? 0 : max_vfs;
> >
> >
> > Unfortunately the if statement got changed considerably with a recent
> > commit:
> >
> > commit 170e85430bcbe4d18e81b5a70bb163c741381092
> > ixgbe: add warning when max_vfs is out of range.
> >
> > And the pattern no longer exists to make a match. In other words, this
> > patch no longer applies to net-next and I have to ask you for yet
> > another spin if you still want to squash the magic number.
>
> It's not a good news. Our distro is waiting for this patch showing up in stable.
> OK, info me if there is a windows for me to revise the patch.

I don't think any particular window of time is better than another. I
don't see this change as needing very thorough testing so if you send in
(yet) another version I'll try to get it through our internal process as
rapidly as I can.

>
> Thanks,
> Ethan
>
> >
> > Thanks,
> > Aaron

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-01-16 01:58:42

by ethan zhao

[permalink] [raw]
Subject: Re: [PATCH 1/2 v3] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63

Aaron,

Is this your net-next repo ? if so, I rebuild the patch with this repo
right now .
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next.git

Thanks,
Ethan

On Thu, Jan 16, 2014 at 9:54 AM, Brown, Aaron F <[email protected]> wrote:
> On Thu, 2014-01-16 at 09:27 +0800, Ethan Zhao wrote:
>> On Thu, Jan 16, 2014 at 6:00 AM, Brown, Aaron F <[email protected]> wrote:
>> > On Wed, 2014-01-15 at 22:12 +0800, Ethan Zhao wrote:
>> >> Because ixgbe driver limit the max number of VF functions could be enabled
>> >> to 63, so define one macro IXGBE_MAX_VFS_DRV_LIMIT and cleanup the const 63
>> >> in code.
>> >>
>> >> v2: fix a typo.
>> >> v3: fix a encoding issue.
>> >>
>> >> Signed-off-by: Ethan Zhao <[email protected]>
>> >> ---
>> >> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
>> >> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +++--
>> >> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h | 5 +++++
>> >> 3 files changed, 10 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> >> index 0ade0cd..47e9b44 100644
>> >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> >> @@ -4818,7 +4818,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
>> >> #ifdef CONFIG_PCI_IOV
>> >> /* assign number of SR-IOV VFs */
>> >> if (hw->mac.type != ixgbe_mac_82598EB)
>> >> - adapter->num_vfs = (max_vfs > 63) ? 0 : max_vfs;
>> >
>> >
>> > Unfortunately the if statement got changed considerably with a recent
>> > commit:
>> >
>> > commit 170e85430bcbe4d18e81b5a70bb163c741381092
>> > ixgbe: add warning when max_vfs is out of range.
>> >
>> > And the pattern no longer exists to make a match. In other words, this
>> > patch no longer applies to net-next and I have to ask you for yet
>> > another spin if you still want to squash the magic number.
>>
>> It's not a good news. Our distro is waiting for this patch showing up in stable.
>> OK, info me if there is a windows for me to revise the patch.
>
> I don't think any particular window of time is better than another. I
> don't see this change as needing very thorough testing so if you send in
> (yet) another version I'll try to get it through our internal process as
> rapidly as I can.
>
>>
>> Thanks,
>> Ethan
>>
>> >
>> > Thanks,
>> > Aaron
>

2014-01-16 02:52:00

by Brown, Aaron F

[permalink] [raw]
Subject: Re: [PATCH 1/2 v3] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63

On Thu, 2014-01-16 at 09:58 +0800, Ethan Zhao wrote:
> Aaron,
>
> Is this your net-next repo ? if so, I rebuild the patch with this repo
> right now .
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next.git
>
> Thanks,
> Ethan
>
Only sort of. Jeff uses it to push patches up, but I don't have an
account there so am simply sending them up the old fashioned way, via
email, hence it is probably not getting updated while Jeff is out.

As long as your patch can apply cleanly to Dave Miller's net-next tree I
should be able to pull it into our internal ones.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-01-16 03:08:42

by ethan zhao

[permalink] [raw]
Subject: Re: [PATCH 1/2 v3] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63

Aaron,
Ok, Dave Miler's net-next tree.

Thanks,
Etan

On Thu, Jan 16, 2014 at 10:51 AM, Brown, Aaron F
<[email protected]> wrote:
> On Thu, 2014-01-16 at 09:58 +0800, Ethan Zhao wrote:
>> Aaron,
>>
>> Is this your net-next repo ? if so, I rebuild the patch with this repo
>> right now .
>> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next.git
>>
>> Thanks,
>> Ethan
>>
> Only sort of. Jeff uses it to push patches up, but I don't have an
> account there so am simply sending them up the old fashioned way, via
> email, hence it is probably not getting updated while Jeff is out.
>
> As long as your patch can apply cleanly to Dave Miller's net-next tree I
> should be able to pull it into our internal ones.

2014-01-16 04:20:22

by ethan zhao

[permalink] [raw]
Subject: Re: [PATCH 1/2 v3] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63

Aaron,
Revised those patches for Dave Miller's net-next OK, passed
building. resent.

Thanks,
Ethan

On Thu, Jan 16, 2014 at 11:08 AM, Ethan Zhao <[email protected]> wrote:
> Aaron,
> Ok, Dave Miler's net-next tree.
>
> Thanks,
> Etan
>
> On Thu, Jan 16, 2014 at 10:51 AM, Brown, Aaron F
> <[email protected]> wrote:
>> On Thu, 2014-01-16 at 09:58 +0800, Ethan Zhao wrote:
>>> Aaron,
>>>
>>> Is this your net-next repo ? if so, I rebuild the patch with this repo
>>> right now .
>>> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next.git
>>>
>>> Thanks,
>>> Ethan
>>>
>> Only sort of. Jeff uses it to push patches up, but I don't have an
>> account there so am simply sending them up the old fashioned way, via
>> email, hence it is probably not getting updated while Jeff is out.
>>
>> As long as your patch can apply cleanly to Dave Miller's net-next tree I
>> should be able to pull it into our internal ones.