From: Alexander Duyck <alexander.h.duyck@intel.com>

This adds an igbvf driver to handle virtual functions provided
by the igb driver.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/Kconfig         |   21 
 drivers/net/Makefile        |    1 
 drivers/net/igbvf/Makefile  |   38 +
 drivers/net/igbvf/defines.h |  125 ++
 drivers/net/igbvf/ethtool.c |  556 ++++++++
 drivers/net/igbvf/igbvf.h   |  333 +++++
 drivers/net/igbvf/mbx.c     |  350 +++++
 drivers/net/igbvf/mbx.h     |   75 +
 drivers/net/igbvf/netdev.c  | 2901 +++++++++++++++++++++++++++++++++++++++++++
 drivers/net/igbvf/regs.h    |  108 ++
 drivers/net/igbvf/vf.c      |  398 ++++++
 drivers/net/igbvf/vf.h      |  265 ++++
 12 files changed, 5171 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/igbvf/Makefile
 create mode 100644 drivers/net/igbvf/defines.h
 create mode 100644 drivers/net/igbvf/ethtool.c
 create mode 100644 drivers/net/igbvf/igbvf.h
 create mode 100644 drivers/net/igbvf/mbx.c
 create mode 100644 drivers/net/igbvf/mbx.h
 create mode 100644 drivers/net/igbvf/netdev.c
 create mode 100644 drivers/net/igbvf/regs.h
 create mode 100644 drivers/net/igbvf/vf.c
 create mode 100644 drivers/net/igbvf/vf.h

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 45403e6..e7f0713 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -2028,6 +2028,27 @@ config IGB_DCA
 	  driver.  DCA is a method for warming the CPU cache before data
 	  is used, with the intent of lessening the impact of cache misses.
 
+config IGBVF
+       tristate "Intel(R) 82576 Virtual Function Ethernet support"
+       depends on PCI
+       ---help---
+         This driver supports Intel(R) 82576 virtual functions.  For more
+         information on how to identify your adapter, go to the Adapter &
+         Driver ID Guide at:
+
+         <http://support.intel.com/support/network/adapter/pro100/21397.htm>
+
+  ...
[ message continues ]

From: Alexander Duyck <alexander.h.duyck@intel.com>

already set.

This patch addresses the unlikely situation that the PF adds a vlan
entry when the vlvf is full, and a vf later adds the vlan to the vlvf.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/igb/e1000_mac.c |   21 ++++++++++++++-------
 drivers/net/igb/e1000_mac.h |    2 +-
 drivers/net/igb/igb_main.c  |    9 +++++++--
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/net/igb/e1000_mac.c b/drivers/net/igb/e1000_mac.c
index 2804db0..f11592f 100644
--- a/drivers/net/igb/e1000_mac.c
+++ b/drivers/net/igb/e1000_mac.c
@@ -126,19 +126,26 @@ void igb_write_vfta(struct e1000_hw *hw, u32 offset, u32 value)
  *  Sets or clears a bit in the VLAN filter table array based on VLAN id
  *  and if we are adding or removing the filter
  **/
-void igb_vfta_set(struct e1000_hw *hw, u32 vid, bool add)
+s32 igb_vfta_set(struct e1000_hw *hw, u32 vid, bool add)
 {
 	u32 index = (vid >> E1000_VFTA_ENTRY_SHIFT) & E1000_VFTA_ENTRY_MASK;
 	u32 mask = 1 < (vid & E1000_VFTA_ENTRY_BIT_SHIFT_MASK);
-	u32 vfta;
+	u32 vfta = array_rd32(E1000_VFTA, index);
+	s32 ret_val = 0;
 
-	vfta = array_rd32(E1000_VFTA, index);
-	if (add)
-		vfta |= mask;
-	else
-		vfta &= ~mask;
+	/* bit was set/cleared before we started */
+	if ((!!(vfta & mask)) == add) {
+		ret_val = -E1000_ERR_CONFIG;
+	} else {
+		if (add)
+			vfta |= mask;
+		else
+			vfta &= ~mask;
+	}
 
 	igb_write_vfta(hw, index, vfta);
+
+	return ret_val;
 }
 
 /**
diff --git a/drivers/net/igb/e1000_mac.h b/drivers/net/igb/e1000_mac.h
index eccc353..a34de52 100644
--- a/drivers/net/igb/e1000_mac.h
+++ b/drivers/net/igb/e1000_mac.h
@@ -58,7 +58,7 @@ s32  igb_write_8bit_ctrl_reg(struct e1000_hw *hw, u32 reg,
 
 void igb_clear_hw_cntrs_base(struct e1000_hw *hw);
 void igb_clear_vfta(struct e1000_hw *hw);
-void igb_vfta_set(struct e1000_hw *hw, u32 vid, bool ...
[ message continues ]

<stares at it for a while>





That's odd-looking.  Why take a copy of rx_old and tx_old when we're

If this process has signal_pending(), msleep_interruptible() becomes a

yitch.


my eyes.

This macro evaluates its argument multiple times and will misbehave (or
be slow) if passed an expreesion with side-effects.


Maybe igbvf_ring.desc should have had type

	union {
		union e1000_adv_rx_desc;
		union e1000_adv_tx_desc;
		union e1000_adv_tx_context_desc;
	} *






It is conventional to terminate a kerneldoc comemnt with






flush_work() is a bit more modern - it will flush a specific work item

What a lot of code.
--

[ message continues ]

Thanks for all the comments.  I tried to incorporate most of them into 
the igbvf driver and also ended up porting some over to our other 
drivers, specifically igb since the igbvf driver copies much of the code.

I have added my comments inline below.

Thanks,

Alex



It doesn't, it has been dropped and references to it replaced with 

This bit isn't set in interrupt context.  This is always used out of 
interrupt context and is just to prevent multiple setting changes at the 

This whole section has been redone.  The approach here didn't work well 

Actually it has no effect on the driver if the sleep fails.  For the 
most part the only reason for having this is to allow enough time to 

Actually I think the ugliness is due to the fact that it is doing the 
check itself twice.  The code below is what it will look like after 
being redone.

                 data[i] = ((igbvf_gstrings_stats[i].sizeof_stat ==
                             sizeof(u64)) ? (*(u64 *)p - *(u64 *)b) :

Actually that was already done and the macro was unused.  It has been 

By switching these over to a typecast I can get away from some of the 
ugliness but it is still there.  Below are the changes I made to support 
what you suggested.

#define IGBVF_RX_DESC_ADV(R, i)     \
         (&((((R).desc))[i].rx_desc))
#define IGBVF_TX_DESC_ADV(R, i)     \
         (&((((R).desc))[i].tx_desc))
#define IGBVF_TX_CTXTDESC_ADV(R, i) \
         (&((((R).desc))[i].tx_context_desc))



union igbvf_desc {
         union e1000_adv_rx_desc rx_desc;
         union e1000_adv_tx_desc tx_desc;
         struct e1000_adv_tx_context_desc tx_context_desc;

For SR-IOV it is always possible that someone might decide to run some 
of the VFs in their DOM0 kernel so it is best to leave it in there to 

This can get to be up to 4K * size of igbvf_buffer which int total comes 

Calling pci_disable_msix is safe since the first thing it checks for is 


This is the coding convention we have been using this ...
[ message continues ]

^^ this comment was missed.

I was indirectly asking for an overview (preferably in the changelog) of

Oh.  Can't use plain old mutex_lock()?


--

[ message continues ]

Sorry, while I missed the comment in my response I had gotten to 
addressing it in the next version.  I updated it to more thoroughly 
describe what the VF driver is doing.  I also included instructions on 

We have one or two spots that actually check to see if the bit is set 
and just report a warning instead of actually waiting on the bit to clear.

--

[ message continues ]

mutex_is_locked()?
--

[ message continues ]

I suppose that would work, but I still would prefer to keep this bit of 
code as it is.  My main motivation is just to use what was already 
proven, and the fact is the e1000, e1000e, igb, and several other 
drivers all use this same approach and it works.

I don't think we need the extra overhead of the mutex lock since most of 
the calls that end up setting the __IGBVF_RESETTING bit will already be 
wrapped within rtnl_lock/unlock calls.  As far as I can tell it looks 
like the only two threads that would ever be competing for the lock 
would be the igbvf_reinit_locked and whatever ethtool or ifconfig 
requests that decide to make changes to the configuration of the netdevice.

Thanks,

Alex
--

[ message continues ]

You may well find that mutex_lock is more efficient than setting a timer
and waking up once per millisecond.  Certainly much lower latency on some
setups given clock granularities of as much as 10 milliseconds.

But it sounds like that's all a separate standalone exercise.
--

[ message continues ]