[net-next PATCH 0/2] ixgbe: macvlan fixes

[net-next PATCH 0/2] ixgbe: macvlan fixes

[John Fastabend]

Two additional fixes for ixgbe l2 forwarding acceleration. One
was reported by kbuild build bot and the second after hardening
my test scripts.

Typically I would send ixgbe fixes through JeffK's tree but I
know a few people who are looking at the feature in their setup's 
so wanted to be sure the patches were visible.

For reference the ixgbe accelerated macvlan patch is here:

commit 2a47fa45d4dfbc54659d28de311a1f764b296a3c
Author: John Fastabend <john.r.fastabend@intel.com>
Date:   Wed Nov 6 09:54:52 2013 -0800

    ixgbe: enable l2 forwarding acceleration for macvlans

Thanks,
John

---

John Fastabend (2):
      ixgbe: fix build err, num_rx_queues is only available with CONFIG_RPS
      ixgbe: deleting dfwd stations out of order can cause null ptr deref


 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

-- 
Signature

[net-next PATCH 1/2] ixgbe: fix build err, num_rx_queues is only available with CONFIG_RPS

[John Fastabend]

In the recent support for layer 2 hardware acceleration, I added a
few references to real_num_rx_queues and num_rx_queues which are
only available with CONFIG_RPS.

The fix is first to remove unnecessary references to num_rx_queues.
Because the hardware offload case is limited to cases where RX queues
and TX queues are equal we only need a single check. Then wrap the
single case in an ifdef.

The patch that introduce this is here,

commit a6cc0cfa72e0b6d9f2c8fd858aacc32313c4f272
Author: John Fastabend <john.r.fastabend@intel.com>
Date:   Wed Nov 6 09:54:46 2013 -0800

    net: Add layer 2 hardware acceleration operations for macvlan devices

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 607275d..2e17c30 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4164,7 +4164,7 @@ static void ixgbe_add_mac_filter(struct ixgbe_adapter *adapter,
 static void ixgbe_fwd_psrtype(struct ixgbe_fwd_adapter *vadapter)
 {
 	struct ixgbe_adapter *adapter = vadapter->real_adapter;
-	int rss_i = vadapter->netdev->real_num_rx_queues;
+	int rss_i = adapter->num_rx_queues_per_pool;
 	struct ixgbe_hw *hw = &adapter->hw;
 	u16 pool = vadapter->pool;
 	u32 psrtype = IXGBE_PSRTYPE_TCPHDR |
@@ -4315,8 +4315,6 @@ static int ixgbe_fwd_ring_up(struct net_device *vdev,
 	if (err)
 		goto fwd_queue_err;
 
-	queues = min_t(unsigned int,
-		       adapter->num_rx_queues_per_pool, vdev->num_rx_queues);
 	err = netif_set_real_num_rx_queues(vdev, queues);
 	if (err)
 		goto fwd_queue_err;
@@ -7540,9 +7538,15 @@ static void *ixgbe_fwd_add(struct net_device *pdev, struct net_device *vdev)
 	struct ixgbe_adapter *adapter = netdev_priv(pdev);
 	int pool, err;
 
+#ifdef CONFIG_RPS
+	if (vdev->num_rx_queues != vdev->num_tx_queues) {
+		netdev_info(pdev, "%s: Only supports a single queue count for TX and RX\n",
+			    vdev->name);
+		return ERR_PTR(-EINVAL);
+	}
+#endif
 	/* Check for hardware restriction on number of rx/tx queues */
-	if (vdev->num_rx_queues != vdev->num_tx_queues ||
-	    vdev->num_tx_queues > IXGBE_MAX_L2A_QUEUES ||
+	if (vdev->num_tx_queues > IXGBE_MAX_L2A_QUEUES ||
 	    vdev->num_tx_queues == IXGBE_BAD_L2A_QUEUE) {
 		netdev_info(pdev,
 			    "%s: Supports RX/TX Queue counts 1,2, and 4\n",
@@ -7566,7 +7570,7 @@ static void *ixgbe_fwd_add(struct net_device *pdev, struct net_device *vdev)
 	/* Enable VMDq flag so device will be set in VM mode */
 	adapter->flags |= IXGBE_FLAG_VMDQ_ENABLED | IXGBE_FLAG_SRIOV_ENABLED;
 	adapter->ring_feature[RING_F_VMDQ].limit = adapter->num_rx_pools;
-	adapter->ring_feature[RING_F_RSS].limit = vdev->num_rx_queues;
+	adapter->ring_feature[RING_F_RSS].limit = vdev->num_tx_queues;
 
 	/* Force reinit of ring allocation with VMDQ enabled */
 	err = ixgbe_setup_tc(pdev, netdev_get_num_tc(pdev));

Re: [net-next PATCH 1/2] ixgbe: fix build err, num_rx_queues is only available with CONFIG_RPS

[Neil Horman]

On Fri, Nov 08, 2013 at 12:50:32AM -0800, John Fastabend wrote:
> In the recent support for layer 2 hardware acceleration, I added a
> few references to real_num_rx_queues and num_rx_queues which are
> only available with CONFIG_RPS.
> 
> The fix is first to remove unnecessary references to num_rx_queues.
> Because the hardware offload case is limited to cases where RX queues
> and TX queues are equal we only need a single check. Then wrap the
> single case in an ifdef.
> 
> The patch that introduce this is here,
> 
> commit a6cc0cfa72e0b6d9f2c8fd858aacc32313c4f272
> Author: John Fastabend <john.r.fastabend@intel.com>
> Date:   Wed Nov 6 09:54:46 2013 -0800
> 
>     net: Add layer 2 hardware acceleration operations for macvlan devices
> 
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>

Re: [net-next PATCH 1/2] ixgbe: fix build err, num_rx_queues is only available with CONFIG_RPS

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 939 bytes --]

On Fri, 2013-11-08 at 00:50 -0800, John Fastabend wrote:
> In the recent support for layer 2 hardware acceleration, I added a
> few references to real_num_rx_queues and num_rx_queues which are
> only available with CONFIG_RPS.
> 
> The fix is first to remove unnecessary references to num_rx_queues.
> Because the hardware offload case is limited to cases where RX queues
> and TX queues are equal we only need a single check. Then wrap the
> single case in an ifdef.
> 
> The patch that introduce this is here,
> 
> commit a6cc0cfa72e0b6d9f2c8fd858aacc32313c4f272
> Author: John Fastabend <john.r.fastabend@intel.com>
> Date:   Wed Nov 6 09:54:46 2013 -0800
> 
>     net: Add layer 2 hardware acceleration operations for macvlan
> devices
> 
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[net-next PATCH 2/2] ixgbe: deleting dfwd stations out of order can cause null ptr deref

[John Fastabend]

The number of stations in use is kept in the num_rx_pools counter
in the ixgbe_adapter structure. This is in turn used by the queue
allocation scheme to determine how many queues are needed to support
the number of pools in use with the current feature set.

This works as long as the pools are added and destroyed in order
because (num_rx_pools * queues_per_pool) is equal to the last
queue in use by a pool. But as soon as you delete a pool out of
order this is no longer the case. So the above multiplication
allocates to few queues and a pool may reference a ring that has
not been allocated/initialized.

To resolve use the bit mask of in use pools to determine the final
pool being used and allocate enough queues so that we don't
inadvertently remove its queues.

# ip link add link eth2 \
	numtxqueues 4 numrxqueues 4 txqueuelen 50 type macvlan
# ip link set dev macvlan0 up
# ip link add link eth2 \
	numtxqueues 4 numrxqueues 4 txqueuelen 50 type macvlan
# ip link set dev macvlan1 up
# for i in {0..100}; do
  ip link set dev macvlan0 down; ip link set dev macvlan0 up;
  done;

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 2e17c30..ec1bf3e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7536,6 +7536,7 @@ static void *ixgbe_fwd_add(struct net_device *pdev, struct net_device *vdev)
 {
 	struct ixgbe_fwd_adapter *fwd_adapter = NULL;
 	struct ixgbe_adapter *adapter = netdev_priv(pdev);
+	unsigned int limit;
 	int pool, err;
 
 #ifdef CONFIG_RPS
@@ -7566,10 +7567,11 @@ static void *ixgbe_fwd_add(struct net_device *pdev, struct net_device *vdev)
 	pool = find_first_zero_bit(&adapter->fwd_bitmask, 32);
 	adapter->num_rx_pools++;
 	set_bit(pool, &adapter->fwd_bitmask);
+	limit = find_last_bit(&adapter->fwd_bitmask, 32);
 
 	/* Enable VMDq flag so device will be set in VM mode */
 	adapter->flags |= IXGBE_FLAG_VMDQ_ENABLED | IXGBE_FLAG_SRIOV_ENABLED;
-	adapter->ring_feature[RING_F_VMDQ].limit = adapter->num_rx_pools;
+	adapter->ring_feature[RING_F_VMDQ].limit = limit + 1;
 	adapter->ring_feature[RING_F_RSS].limit = vdev->num_tx_queues;
 
 	/* Force reinit of ring allocation with VMDQ enabled */
@@ -7597,11 +7599,13 @@ static void ixgbe_fwd_del(struct net_device *pdev, void *priv)
 {
 	struct ixgbe_fwd_adapter *fwd_adapter = priv;
 	struct ixgbe_adapter *adapter = fwd_adapter->real_adapter;
+	unsigned int limit;
 
 	clear_bit(fwd_adapter->pool, &adapter->fwd_bitmask);
 	adapter->num_rx_pools--;
 
-	adapter->ring_feature[RING_F_VMDQ].limit = adapter->num_rx_pools;
+	limit = find_last_bit(&adapter->fwd_bitmask, 32);
+	adapter->ring_feature[RING_F_VMDQ].limit = limit + 1;
 	ixgbe_fwd_ring_down(fwd_adapter->netdev, fwd_adapter);
 	ixgbe_setup_tc(pdev, netdev_get_num_tc(pdev));
 	netdev_dbg(pdev, "pool %i:%i queues %i:%i VSI bitmask %lx\n",

Re: [net-next PATCH 2/2] ixgbe: deleting dfwd stations out of order can cause null ptr deref

[Neil Horman]

On Fri, Nov 08, 2013 at 12:51:10AM -0800, John Fastabend wrote:
> The number of stations in use is kept in the num_rx_pools counter
> in the ixgbe_adapter structure. This is in turn used by the queue
> allocation scheme to determine how many queues are needed to support
> the number of pools in use with the current feature set.
> 
> This works as long as the pools are added and destroyed in order
> because (num_rx_pools * queues_per_pool) is equal to the last
> queue in use by a pool. But as soon as you delete a pool out of
> order this is no longer the case. So the above multiplication
> allocates to few queues and a pool may reference a ring that has
> not been allocated/initialized.
> 
> To resolve use the bit mask of in use pools to determine the final
> pool being used and allocate enough queues so that we don't
> inadvertently remove its queues.
> 
> # ip link add link eth2 \
> 	numtxqueues 4 numrxqueues 4 txqueuelen 50 type macvlan
> # ip link set dev macvlan0 up
> # ip link add link eth2 \
> 	numtxqueues 4 numrxqueues 4 txqueuelen 50 type macvlan
> # ip link set dev macvlan1 up
> # for i in {0..100}; do
>   ip link set dev macvlan0 down; ip link set dev macvlan0 up;
>   done;
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>

Re: [net-next PATCH 2/2] ixgbe: deleting dfwd stations out of order can cause null ptr deref

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1483 bytes --]

On Fri, 2013-11-08 at 00:51 -0800, John Fastabend wrote:
> The number of stations in use is kept in the num_rx_pools counter
> in the ixgbe_adapter structure. This is in turn used by the queue
> allocation scheme to determine how many queues are needed to support
> the number of pools in use with the current feature set.
> 
> This works as long as the pools are added and destroyed in order
> because (num_rx_pools * queues_per_pool) is equal to the last
> queue in use by a pool. But as soon as you delete a pool out of
> order this is no longer the case. So the above multiplication
> allocates to few queues and a pool may reference a ring that has
> not been allocated/initialized.
> 
> To resolve use the bit mask of in use pools to determine the final
> pool being used and allocate enough queues so that we don't
> inadvertently remove its queues.
> 
> # ip link add link eth2 \
>         numtxqueues 4 numrxqueues 4 txqueuelen 50 type macvlan
> # ip link set dev macvlan0 up
> # ip link add link eth2 \
>         numtxqueues 4 numrxqueues 4 txqueuelen 50 type macvlan
> # ip link set dev macvlan1 up
> # for i in {0..100}; do
>   ip link set dev macvlan0 down; ip link set dev macvlan0 up;
>   done;
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [net-next PATCH 0/2] ixgbe: macvlan fixes

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1194 bytes --]

On Fri, 2013-11-08 at 00:50 -0800, John Fastabend wrote:
> Two additional fixes for ixgbe l2 forwarding acceleration. One
> was reported by kbuild build bot and the second after hardening
> my test scripts.
> 
> Typically I would send ixgbe fixes through JeffK's tree but I
> know a few people who are looking at the feature in their setup's 
> so wanted to be sure the patches were visible.

I told John to go ahead and send these patches directly to netdev, so
that Dave can pick these up before he pushes his net-next to Linus.

> 
> For reference the ixgbe accelerated macvlan patch is here:
> 
> commit 2a47fa45d4dfbc54659d28de311a1f764b296a3c
> Author: John Fastabend <john.r.fastabend@intel.com>
> Date:   Wed Nov 6 09:54:52 2013 -0800
> 
>     ixgbe: enable l2 forwarding acceleration for macvlans
> 
> Thanks,
> John
> 
> ---
> 
> John Fastabend (2):
>       ixgbe: fix build err, num_rx_queues is only available with CONFIG_RPS
>       ixgbe: deleting dfwd stations out of order can cause null ptr deref
> 
> 
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [net-next PATCH 0/2] ixgbe: macvlan fixes

[David Miller]

From: John Fastabend <john.fastabend@gmail.com>
Date: Fri, 08 Nov 2013 00:50:07 -0800

> Two additional fixes for ixgbe l2 forwarding acceleration. One
> was reported by kbuild build bot and the second after hardening
> my test scripts.
> 
> Typically I would send ixgbe fixes through JeffK's tree but I
> know a few people who are looking at the feature in their setup's 
> so wanted to be sure the patches were visible.
> 
> For reference the ixgbe accelerated macvlan patch is here:
> 
> commit 2a47fa45d4dfbc54659d28de311a1f764b296a3c
> Author: John Fastabend <john.r.fastabend@intel.com>
> Date:   Wed Nov 6 09:54:52 2013 -0800
> 
>     ixgbe: enable l2 forwarding acceleration for macvlans

Both applied, thanks John.

kernel network panic stack traces on ubuntu 12.04

[Templin, Fred L]

Hi,

I am hacking the kernel network stack on a Dell running ubuntu 12.04
and when I add code that I know for sure is causing a panic the system
locks up as if it is hung and never prints out a stack trace. The only
way out is to hard reset using the power button. This happens on several
different linux kernel versions - the most recent being 3.10.17. Any
ideas how I can get a stack trace?

Thanks - Fred