Re: [dpdk-dev] [PATCH] igb_uio: cap max VFs at 7 to reserve one for PF

[Chris Wright <chrisw@redhat.com>]

* Richardson, Bruce (bruce.richardson@intel.com) wrote:
> > -----Original Message-----
> > From: Chris Wright [mailto:chrisw@redhat.com]
> > Sent: Friday, June 13, 2014 11:14 AM
> > To: Richardson, Bruce
> > Cc: Chris Wright; Stephen Hemminger; Thomas Monjalon; dev@dpdk.org
> > Subject: Re: [PATCH] igb_uio: cap max VFs at 7 to reserve one for PF
> > 
> > * Richardson, Bruce (bruce.richardson@intel.com) wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Chris Wright [mailto:chrisw@redhat.com]
> > > > Sent: Friday, June 13, 2014 10:52 AM
> > > > To: Richardson, Bruce; Stephen Hemminger
> > > > Cc: Thomas Monjalon; dev@dpdk.org
> > > > Subject: [PATCH] igb_uio: cap max VFs at 7 to reserve one for PF
> > > >
> > > > To keep from confusing users, cap max VFs at 7, despite PCI SR-IOV config
> > > > space showing a max of 8.  This reserves a queue pair for the PF.
> > > >
> > > > This issue was cited here:
> > > >
> > > >  http://dpdk.org/ml/archives/dev/2014-April/001832.html
> > > >
> > > > Cc: Bruce Richardson <bruce.richardson@intel.com>
> > > > Cc: Stephen Hemminger <stephen@networkplumber.org>
> > > > Signed-off-by: Chris Wright <chrisw@redhat.com>
> > > > ---
> > > >
> > > > This is what Linux kernel driver does.  I have only
> > > > compile tested it.  Stephen sending to you and Bruce
> > > > in case you want to Ack and add to your current queue.
> > > >
> > >
> > > Sorry, NAK - at least for this implementation.
> > 
> > Oh, that's fine.
> > 
> > > Hardcoding this to 7 is a bad idea, as the actual max number of VFs supported
> > will depend on the actual hardware used. For someone using an 82599, they can
> > have up to 64 VFs, or 63+PF, so limiting so 7 in that case is a major reduction in
> > capability. What might work there is querying the max number of VFs and
> > limiting to max - 1.
> > 
> > But this is igb_uio, not 82599 (ixgbe).
> 
> igb_uio is used as the supporting kernel module for both the e1000/igb and ixgbe pmd implementations (as well as for the forthcoming i40e pmd). Despite the name, it's not just for igb-based NICs.

Oh, right, sorry, was looking at pmd side for each driver.

> > > However, even with that, I would suggest that any limit should be possible to
> > override. It's entirely possible that someone max actually want to reserve the
> > full number of VFs, either because they don't want to use the NIC on the host at
> > all, or because they are happy to use a VF on the host instead. Module
> > parameter to allow override might work - and information on it could be added
> > to the error message when we limit the VFs inside the driver.
> > 
> > It's been a while since I've looked at this, but my recollection is
> > the PF must be there (basic mailbox handling, for example).
> > 
> > Would you rather a simple warning message as a hint?
> 
> I'm not sure about the PF still needing to be there or not - I'm not an expert in that area, so you may indeed be right. 
> However, as for this patch, I'd probably be ok for now with a version that queried the max_vfs and limited based on that. If in future we do need to add an override it should be trivial to add later-on.

I'll look at that idea.

thanks,
-chris