Netdev List
 help / color / mirror / Atom feed
* [net-next 08/12] i40e: Update VEB's enabled_tc after reconfiguration
From: Jeff Kirsher @ 2014-11-15  6:08 UTC (permalink / raw)
  To: davem; +Cc: Neerav Parikh, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1416031715-32498-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Neerav Parikh <neerav.parikh@intel.com>

When the port TC configuration changes as a result of DCBx the driver
modifies the enabled TCs for the VEBs it manages. But, in the process
it did not update the enabled_tc value that it caches on a per VEB basis.

So, when the next reconfiguration event occurs where the number of TC
value is same as the value cached in enabled_tc for a given VEB; driver
does not modify it's TC configuration by calling appropriate AQ command
believing it is running with the same configuration as requested.
Now, as the VEB is not actually enabled for the TCs that are there any
TC configuration command for VSI attached to that VEB with TCs that are
not enabled for the VEB fails.

This patch fixes this issue.

Change-ID: Ife5694469b05494228e0d850429ea1734738cf29
Signed-off-by: Neerav Parikh <neerav.parikh@intel.com>
Tested-By: Jack Morgan <jack.morgan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 47f4ea4..1fcbf35 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -8283,6 +8283,7 @@ static int i40e_veb_get_bw_info(struct i40e_veb *veb)
 	veb->bw_limit = le16_to_cpu(ets_data.port_bw_limit);
 	veb->bw_max_quanta = ets_data.tc_bw_max;
 	veb->is_abs_credits = bw_data.absolute_credits_enable;
+	veb->enabled_tc = ets_data.tc_valid_bits;
 	tc_bw_max = le16_to_cpu(bw_data.tc_bw_max[0]) |
 		    (le16_to_cpu(bw_data.tc_bw_max[1]) << 16);
 	for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++) {
-- 
1.9.3

^ permalink raw reply related

* [net-next 09/12] i40e: Modify Tx disable wait flow in case of DCB reconfiguration
From: Jeff Kirsher @ 2014-11-15  6:08 UTC (permalink / raw)
  To: davem; +Cc: Neerav Parikh, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1416031715-32498-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Neerav Parikh <neerav.parikh@intel.com>

When DCB TC configuration changes the firmware suspends the port's Tx.
Now, as DCB TCs may have changed the PF driver tries to reconfigure the
TC configuration of the VSIs it manages. As part of this process it disables
the VSI queues but the Tx queue disable will not complete as the port's
Tx has been suspended. So, waiting for Tx queues to go to disable state
in this flow may lead to detection of Tx queue disable timeout errors.

Hence, this patch adds a new PF state so that if a port's Tx is in
suspended state the Tx queue disable flow would just put the request for
the queue to be disabled and return without waiting for the queue to be
actually disabled.
Once the VSI(s) TC reconfiguration has been done and driver has called
firmware AQC "Resume PF Traffic" the driver checks the Tx queues requested
to be disabled are actually disabled before re-enabling them again.

Change-ID: If3e03ce4813a4e342dbd5a1eb1d2861e952b7544
Signed-off-by: Neerav Parikh <neerav.parikh@intel.com>
Tested-By: Jack Morgan <jack.morgan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h      |  1 +
 drivers/net/ethernet/intel/i40e/i40e_main.c | 60 +++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index b7a807b..464342a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -146,6 +146,7 @@ enum i40e_state_t {
 	__I40E_DOWN_REQUESTED,
 	__I40E_FD_FLUSH_REQUESTED,
 	__I40E_RESET_FAILED,
+	__I40E_PORT_TX_SUSPENDED,
 };
 
 enum i40e_interrupt_policy {
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 1fcbf35..9660889 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -3492,6 +3492,9 @@ static int i40e_vsi_control_tx(struct i40e_vsi *vsi, bool enable)
 		}
 
 		wr32(hw, I40E_QTX_ENA(pf_q), tx_reg);
+		/* No waiting for the Tx queue to disable */
+		if (!enable && test_bit(__I40E_PORT_TX_SUSPENDED, &pf->state))
+			continue;
 
 		/* wait for the change to finish */
 		ret = i40e_pf_txq_wait(pf, pf_q, enable);
@@ -3911,6 +3914,56 @@ static void i40e_pf_unquiesce_all_vsi(struct i40e_pf *pf)
 	}
 }
 
+#ifdef CONFIG_I40E_DCB
+/**
+ * i40e_vsi_wait_txq_disabled - Wait for VSI's queues to be disabled
+ * @vsi: the VSI being configured
+ *
+ * This function waits for the given VSI's Tx queues to be disabled.
+ **/
+static int i40e_vsi_wait_txq_disabled(struct i40e_vsi *vsi)
+{
+	struct i40e_pf *pf = vsi->back;
+	int i, pf_q, ret;
+
+	pf_q = vsi->base_queue;
+	for (i = 0; i < vsi->num_queue_pairs; i++, pf_q++) {
+		/* Check and wait for the disable status of the queue */
+		ret = i40e_pf_txq_wait(pf, pf_q, false);
+		if (ret) {
+			dev_info(&pf->pdev->dev,
+				 "%s: VSI seid %d Tx ring %d disable timeout\n",
+				 __func__, vsi->seid, pf_q);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * i40e_pf_wait_txq_disabled - Wait for all queues of PF VSIs to be disabled
+ * @pf: the PF
+ *
+ * This function waits for the Tx queues to be in disabled state for all the
+ * VSIs that are managed by this PF.
+ **/
+static int i40e_pf_wait_txq_disabled(struct i40e_pf *pf)
+{
+	int v, ret = 0;
+
+	for (v = 0; v < pf->hw.func_caps.num_vsis; v++) {
+		if (pf->vsi[v]) {
+			ret = i40e_vsi_wait_txq_disabled(pf->vsi[v]);
+			if (ret)
+				break;
+		}
+	}
+
+	return ret;
+}
+
+#endif
 /**
  * i40e_dcb_get_num_tc -  Get the number of TCs from DCBx config
  * @dcbcfg: the corresponding DCBx configuration structure
@@ -5102,6 +5155,7 @@ static int i40e_handle_lldp_event(struct i40e_pf *pf,
 	else
 		pf->flags &= ~I40E_FLAG_DCB_ENABLED;
 
+	set_bit(__I40E_PORT_TX_SUSPENDED, &pf->state);
 	/* Reconfiguration needed quiesce all VSIs */
 	i40e_pf_quiesce_all_vsi(pf);
 
@@ -5110,7 +5164,13 @@ static int i40e_handle_lldp_event(struct i40e_pf *pf,
 
 	ret = i40e_resume_port_tx(pf);
 
+	clear_bit(__I40E_PORT_TX_SUSPENDED, &pf->state);
 	/* In case of error no point in resuming VSIs */
+	if (ret)
+		goto exit;
+
+	/* Wait for the PF's Tx queues to be disabled */
+	ret = i40e_pf_wait_txq_disabled(pf);
 	if (!ret)
 		i40e_pf_unquiesce_all_vsi(pf);
 exit:
-- 
1.9.3

^ permalink raw reply related

* [net-next 10/12] i40e: Do not disable/enable FCoE VSI with DCB reconfig
From: Jeff Kirsher @ 2014-11-15  6:08 UTC (permalink / raw)
  To: davem; +Cc: Neerav Parikh, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1416031715-32498-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Neerav Parikh <neerav.parikh@intel.com>

FCoE VSI Tx queue disable times out when reconfiguring as a result of
DCB TC configuration change event.

The hardware allows us to skip disabling and enabling of Tx queues for
VSIs with single TC enabled. As FCoE VSI is configured to have only
single TC we skip it from disable/enable flow.

Change-ID: Ia73ff3df8785ba2aa3db91e6f2c9005e61ebaec2
Signed-off-by: Neerav Parikh <neerav.parikh@intel.com>
Tested-By: Jack Morgan <jack.morgan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 9660889..7651925 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -3862,6 +3862,15 @@ static void i40e_quiesce_vsi(struct i40e_vsi *vsi)
 	if (test_bit(__I40E_DOWN, &vsi->state))
 		return;
 
+	/* No need to disable FCoE VSI when Tx suspended */
+	if ((test_bit(__I40E_PORT_TX_SUSPENDED, &vsi->back->state)) &&
+	    vsi->type == I40E_VSI_FCOE) {
+		dev_dbg(&vsi->back->pdev->dev,
+			"%s: VSI seid %d skipping FCoE VSI disable\n",
+			 __func__, vsi->seid);
+		return;
+	}
+
 	set_bit(__I40E_NEEDS_RESTART, &vsi->state);
 	if (vsi->netdev && netif_running(vsi->netdev)) {
 		vsi->netdev->netdev_ops->ndo_stop(vsi->netdev);
@@ -3953,7 +3962,8 @@ static int i40e_pf_wait_txq_disabled(struct i40e_pf *pf)
 	int v, ret = 0;
 
 	for (v = 0; v < pf->hw.func_caps.num_vsis; v++) {
-		if (pf->vsi[v]) {
+		/* No need to wait for FCoE VSI queues */
+		if (pf->vsi[v] && pf->vsi[v]->type != I40E_VSI_FCOE) {
 			ret = i40e_vsi_wait_txq_disabled(pf->vsi[v]);
 			if (ret)
 				break;
-- 
1.9.3

^ permalink raw reply related

* [net-next 12/12] i40e: Set XPS bit mask to zero in DCB mode
From: Jeff Kirsher @ 2014-11-15  6:08 UTC (permalink / raw)
  To: davem; +Cc: Neerav Parikh, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1416031715-32498-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Neerav Parikh <neerav.parikh@intel.com>

Due to DCBX configuration change if the VSI needs to use more than 1 TC;
it needs to disable the XPS maps that were set when operating in 1 TC mode.
Without disabling XPS the netdev layer will select queues based on those
settings and not use the TC queue mapping to make the queue selection.

This patch allows the driver to enable/disable the XPS based on the number
of TCs being enabled for the given VSI.

Change-ID: Idc4dec47a672d2a509f6d7fe11ed1ee65b4f0e08
Signed-off-by: Neerav Parikh <neerav.parikh@intel.com>
Tested-By: Jack Morgan <jack.morgan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 38 +++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 7651925..9c68236 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -2382,6 +2382,35 @@ static void i40e_vsi_free_rx_resources(struct i40e_vsi *vsi)
 }
 
 /**
+ * i40e_config_xps_tx_ring - Configure XPS for a Tx ring
+ * @ring: The Tx ring to configure
+ *
+ * This enables/disables XPS for a given Tx descriptor ring
+ * based on the TCs enabled for the VSI that ring belongs to.
+ **/
+static void i40e_config_xps_tx_ring(struct i40e_ring *ring)
+{
+	struct i40e_vsi *vsi = ring->vsi;
+	cpumask_var_t mask;
+
+	if (ring->q_vector && ring->netdev) {
+		/* Single TC mode enable XPS */
+		if (vsi->tc_config.numtc <= 1 &&
+		    !test_and_set_bit(__I40E_TX_XPS_INIT_DONE, &ring->state)) {
+			netif_set_xps_queue(ring->netdev,
+					    &ring->q_vector->affinity_mask,
+					    ring->queue_index);
+		} else if (alloc_cpumask_var(&mask, GFP_KERNEL)) {
+			/* Disable XPS to allow selection based on TC */
+			bitmap_zero(cpumask_bits(mask), nr_cpumask_bits);
+			netif_set_xps_queue(ring->netdev, mask,
+					    ring->queue_index);
+			free_cpumask_var(mask);
+		}
+	}
+}
+
+/**
  * i40e_configure_tx_ring - Configure a transmit ring context and rest
  * @ring: The Tx ring to configure
  *
@@ -2404,13 +2433,8 @@ static int i40e_configure_tx_ring(struct i40e_ring *ring)
 		ring->atr_sample_rate = 0;
 	}
 
-	/* initialize XPS */
-	if (ring->q_vector && ring->netdev &&
-	    vsi->tc_config.numtc <= 1 &&
-	    !test_and_set_bit(__I40E_TX_XPS_INIT_DONE, &ring->state))
-		netif_set_xps_queue(ring->netdev,
-				    &ring->q_vector->affinity_mask,
-				    ring->queue_index);
+	/* configure XPS */
+	i40e_config_xps_tx_ring(ring);
 
 	/* clear the context structure first */
 	memset(&tx_ctx, 0, sizeof(tx_ctx));
-- 
1.9.3

^ permalink raw reply related

* [net-next 11/12] i40e: Prevent link flow control settings when PFC is enabled
From: Jeff Kirsher @ 2014-11-15  6:08 UTC (permalink / raw)
  To: davem; +Cc: Neerav Parikh, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1416031715-32498-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Neerav Parikh <neerav.parikh@intel.com>

When PFC is enabled we should not proceed with setting the link flow control
parameters.  Also, always report the link flow Tx/Rx settings as off when
PFC is enabled.

Change-ID: Ib09ec58afdf0b2e587ac9d8851a5c80ad58206c4
Signed-off-by: Neerav Parikh <neerav.parikh@intel.com>
Tested-By: Jack Morgan <jack.morgan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index afad5aa..bb1698a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -644,11 +644,19 @@ static void i40e_get_pauseparam(struct net_device *netdev,
 	struct i40e_pf *pf = np->vsi->back;
 	struct i40e_hw *hw = &pf->hw;
 	struct i40e_link_status *hw_link_info = &hw->phy.link_info;
+	struct i40e_dcbx_config *dcbx_cfg = &hw->local_dcbx_config;
 
 	pause->autoneg =
 		((hw_link_info->an_info & I40E_AQ_AN_COMPLETED) ?
 		  AUTONEG_ENABLE : AUTONEG_DISABLE);
 
+	/* PFC enabled so report LFC as off */
+	if (dcbx_cfg->pfc.pfcenable) {
+		pause->rx_pause = 0;
+		pause->tx_pause = 0;
+		return;
+	}
+
 	if (hw->fc.current_mode == I40E_FC_RX_PAUSE) {
 		pause->rx_pause = 1;
 	} else if (hw->fc.current_mode == I40E_FC_TX_PAUSE) {
@@ -672,6 +680,7 @@ static int i40e_set_pauseparam(struct net_device *netdev,
 	struct i40e_vsi *vsi = np->vsi;
 	struct i40e_hw *hw = &pf->hw;
 	struct i40e_link_status *hw_link_info = &hw->phy.link_info;
+	struct i40e_dcbx_config *dcbx_cfg = &hw->local_dcbx_config;
 	bool link_up = hw_link_info->link_info & I40E_AQ_LINK_UP;
 	i40e_status status;
 	u8 aq_failures;
@@ -693,8 +702,9 @@ static int i40e_set_pauseparam(struct net_device *netdev,
 		netdev_info(netdev, "Autoneg did not complete so changing settings may not result in an actual change.\n");
 	}
 
-	if (hw->fc.current_mode == I40E_FC_PFC) {
-		netdev_info(netdev, "Priority flow control enabled. Cannot set link flow control.\n");
+	if (dcbx_cfg->pfc.pfcenable) {
+		netdev_info(netdev,
+			    "Priority flow control enabled. Cannot set link flow control.\n");
 		return -EOPNOTSUPP;
 	}
 
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH v3] ixgbe: Look up MAC address in Open Firmware or IDPROM
From: Jeff Kirsher @ 2014-11-15  6:21 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: netdev, linux.nics
In-Reply-To: <yq17fyxus1y.fsf@sermon.lab.mkp.net>

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

On Fri, 2014-11-14 at 21:16 -0500, Martin K. Petersen wrote:
> 
> Attempt to look up the MAC address in Open Firmware on systems that
> support it. On SPARC resort to using the IDPROM if no OF address is
> found.
> 
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

Thanks Martin, I will add your patch to my queue.

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

^ permalink raw reply

* Re: [PATCH] ieee802154: fix error handling in ieee802154fake_probe()
From: Alexander Aring @ 2014-11-15  7:16 UTC (permalink / raw)
  To: Alexey Khoroshilov
  Cc: David S . Miller, linux-wpan, netdev, linux-kernel, ldv-project
In-Reply-To: <1416006719-3450-1-git-send-email-khoroshilov@ispras.ru>

Hi,

On Sat, Nov 15, 2014 at 02:11:59AM +0300, Alexey Khoroshilov wrote:
> In case of any failure ieee802154fake_probe() just calls unregister_netdev().
> But it does not look safe to unregister netdevice before it was registered.
> 
> The patch implements straightforward resource deallocation in case of
> failure in ieee802154fake_probe().
> 
> Found by Linux Driver Verification project (linuxtesting.org).
> 

Thanks for the patch! But I already delete the whole fakehard driver.
The patch to remove this driver is already in net-next.

There was too many issues with the fake hardMAC drivers and we don't
really supported a "real" hardMAC transceiver. For my rework to grab the
good things from wireless implementation and put 802.15.4 functionality
on it I simple delete the whole driver and _maybe_ add a new one with
the new introduced interfaces for possible hardMAC functionality.

This patch could go into bluetooth(net)/stable but I don't think that
there are many users for this driver.

- Alex

^ permalink raw reply

* Вы выиграли лотерею Организация Объединенных Наций в размере 1,2 млн. долларов США
From: Dammy Lugg @ 2014-11-15  7:36 UTC (permalink / raw)
  To: Dammy Lugg

Уважаемые электронной почты на предъявителя,

любезно прислать информацию вместе с пин-код на это письмо:
unitednationnss@excite.co.uk ;

Единовременно в размере 1,2 млн. долл. США (один миллион двести тысяч
долларов США) были зачислены на Ваш адрес электронной почты от лотереи
Организации Объединенных Наций. Поздравляю. Ваш грант контактный номер
(UNO-154/4456/011). Подтвердите получение связавшись Dr. James Янг:
unitednationnss@excite.co.uk ; чтобы отправить вашу личную информацию
с ним.

Имя:
Адрес:
Возраст:
Пол:
Телефон:
Род занятий:
Страна:

С уважением,
Г-жа Сюзан Филиппа.
Организации Объединенных Наций.

^ permalink raw reply

* Re: [PATCH] ieee802154: fix error handling in ieee802154fake_probe()
From: Marcel Holtmann @ 2014-11-15  7:58 UTC (permalink / raw)
  To: Alexander Aring, David S. Miller
  Cc: Alexey Khoroshilov, linux-wpan, Network Development, linux-kernel,
	ldv-project
In-Reply-To: <20141115071603.GA3481@omega>

Hi Alex,

>> In case of any failure ieee802154fake_probe() just calls unregister_netdev().
>> But it does not look safe to unregister netdevice before it was registered.
>> 
>> The patch implements straightforward resource deallocation in case of
>> failure in ieee802154fake_probe().
>> 
>> Found by Linux Driver Verification project (linuxtesting.org).
>> 
> 
> Thanks for the patch! But I already delete the whole fakehard driver.
> The patch to remove this driver is already in net-next.
> 
> There was too many issues with the fake hardMAC drivers and we don't
> really supported a "real" hardMAC transceiver. For my rework to grab the
> good things from wireless implementation and put 802.15.4 functionality
> on it I simple delete the whole driver and _maybe_ add a new one with
> the new introduced interfaces for possible hardMAC functionality.
> 
> This patch could go into bluetooth(net)/stable but I don't think that
> there are many users for this driver.

I am not sure that this is worth it then.

Dave, do you want to just take it via net-stable tree? We currently have no urgent bluetooth-stable tree patches pending.

Regards

Marcel

^ permalink raw reply

* Re: Understanding what's going on when using a Huawei E173 USB 3G web-stick (UMTS/HSPA)
From: Sedat Dilek @ 2014-11-15  9:23 UTC (permalink / raw)
  To: Dan Williams
  Cc: Greg KH, David S. Miller, netdev@vger.kernel.org, linux-usb,
	Aleksander Morgado
In-Reply-To: <1415981880.22965.28.camel@dcbw.local>

On Fri, Nov 14, 2014 at 5:18 PM, Dan Williams <dcbw@redhat.com> wrote:
> On Fri, 2014-11-14 at 11:56 +0100, Sedat Dilek wrote:
>> On Wed, Nov 12, 2014 at 2:21 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>> > On Tue, Nov 4, 2014 at 5:55 PM, Dan Williams <dcbw@redhat.com> wrote:
>> >> On Tue, 2014-11-04 at 16:11 +0100, Sedat Dilek wrote:
>> >>> Hi,
>> >>>
>> >>> I wanted to understand what is going on the kernel-side when
>> >>> connecting to the Internet via a Huawei E173 USB web-stick (3rd
>> >>> Generation: UMTS / HSPA).
>> >>>
>> >>> Especially the correlation between the diverse USB/NET kernel-drivers
>> >>> and how the networking is setup.
>> >>
>> >
>> > [ Sitting in front of a foreign Windows machine ]
>> >
>> > [ CC Aleksander ]
>> >
>> > Hi Dan,
>> >
>> > sorry for the late (and short) response.
>> >
>> > AFAICS you have given a "skeleton" for a "usb-wwan-networking"
>> > documentation :-).
>> >
>> > Personally, I would like to take into account some kernel-config
>> > options and some more things.
>> >
>>
>> I started with documenting...
>>
>> I have still some difficulties in understanding USB WWAN Networking.
>> So, this is what I revealed...
>>
>> ##### USB: HUAWEI E173 3G/UMTS/HSPA INTERNET STICK
>>
>> ### USB-NETWORKING AND WWAN SETUP
>> CONFIG_USB_USBNET=m        <--- usb networking
>> CONFIG_USB_NET_CDCETHER=m  <--- usb-wwan (net) configuration
>> CONFIG_USB_SERIAL_WWAN=m   <--- usb-wwan (serial) configuration
>> CONFIG_USB_SERIAL_OPTION=m <--- usb-serial driver called "option"
>
> Most WWAN devices actually require option, because most WWAN devices
> have "serial" ports (even if they aren't used for PPP), and 'option' is
> the driver that handles this.  The 'option' name is historic, but the
> driver should really be called something like 'wwan-serial-generic' or
> something like that.
>

Is there sth. against renaming the "option" driver to "wwan-serial-generic"?

> You're missing a few other "net" type drivers:
>
> CONFIG_USB_NET_QMI_WWAN = Qualcomm QMI capable devices (net)
> CONFIG_USB_HSO = "Option High-Speed" (net) devices
> CONFIG_USB_NET_KALMIA = Samsung LTE dongle (net)
> CONFIG_USB_SIERRA_NET = Sierra devices (net)
> CONFIG_USB_NET_CDC_NCM = Ericsson F5321 (?) and some others (net)
> CONFIG_USB_NET_HUAWEI_CDC_NCM = Huawei NCM variant (net)
> CONFIG_USB_VL600 = LG VL600 / AD600 LTE device (net)
> CONFIG_USB_NET_CDC_MBIM = MBIM (net) devices (popular currently)
>
> and maybe even:
>
> CONFIG_USB_CDC_PHONET = Nokia phones and USB sticks, not "net" but
> proprietary protocol that handles both data/control channels
>
> For the "serial" side:
>
> CONFIG_USB_ACM = generic "serial" devices, many are *not* WWAN but many
> WWAN devices use this class/driver
> CONFIG_USB_SERIAL_QCAUX = Various Qualcomm-based devices' "auxiliary"
> ports (DIAG, GPS, etc)
> CONFIG_USB_SERIAL_QUALCOMM = Firmware loading and "auxiliary" ports on
> various Qualcomm Gobi devices
> CONFIG_USB_SERIAL_SIERRAWIRELESS = Older Sierra device serial ports for
> PPP/control and "auxiliary" functions
>
> WWAN devices basically mix & match these drivers depending on what
> interfaces the firmware exposes.
>
> For example, some Sierra devices may require both
> CONFIG_USB_SERIAL_SIERRAWIRELESS and CONFIG_USB_SIERRA_NET.
>
> Older Sierra devices may use only CONFIG_USB_SERIAL_SIERRAWIRELESS and
> do not provide a 'net' port at all, but use only PPP.
>
> Sierra devices based on Icera chips may use CONFIG_USB_ACM and either
> CONFIG_USB_SIERRA_NET or CONFIG_USB_NET_CDCETHER.
>
> Some Huawei devices may use CONFIG_USB_NET_CDCETHER and either
> CONFIG_USB_SERIAL_OPTION or CONFIG_USB_ACM.
>
> Other Huawei devices may use CONFIG_USB_NET_QMI_WWAN and
> CONFIG_USB_SERIAL_OPTION.
>
> Even other Huawei devices may be Qualcomm Gobi type and use
> CONFIG_USB_NET_QMI_WWAN and CONFIG_USB_SERIAL_QUALCOMM.
>
> So you see it really depends on exactly how the firmware is implemented.
> But in general, devices still fall into the categories I originally
> listed, and the drivers fall into specific categories too ("net",
> "serial", "proprietary"), and devices mix & match drivers to achieve the
> end result.
>

Hi Dan,

Hmm, yeah I see more clearly this usb-networking is not that simple.
Anyway, I will face you if I need some aid or examples.

Unfortunately, you did not answer my "cdc_ether" questions...

I am a bit irritated about the Kconfig help-text (might be outdated or
not precise enough)...
...
config USB_NET_CDCETHER
tristate "CDC Ethernet support (smart devices such as cable modems)"
...
Is this here a "GSM modem" (see dmesg extract below)?
Or even (taken from the logs) "Mobile Broadband Network Device".

...
 This driver creates an interface named "ethX", where X depends on
 what other networking devices you have in use.  However, if the
 IEEE 802 "local assignment" bit is set in the address, a "usbX"
 name is used instead.
,,,

Checking my logs I have no "ethX" or "usbX"...

$ dmesg | egrep -i 'cdc|wwan'
[   13.448398] cdc_ether 3-1.2:1.1 wwan0: register 'cdc_ether' at
usb-0000:00:1a.0-1.2, Mobile Broadband Network Device,
02:50:f3:00:00:00
[   13.448497] usbcore: registered new interface driver cdc_ether

...what do I have? "wwanX"?

I can send a patch to clarify on the Kconfig help-text.
Which MLs BTW (netdev and linux-usb, even linux-serial)?

Hope I do not annoy you that much.
Just curious.

- Sedat -

P.S.: Extract from my dmesg after plugging-in my Huawei Internet USB stick.

...
[   57.117170] usb 4-1.5: USB disconnect, device number 4
[   66.248136] usb 3-1.2: new high-speed USB device number 4 using ehci-pci
[   66.343169] usb 3-1.2: New USB device found, idVendor=12d1, idProduct=1446
[   66.343182] usb 3-1.2: New USB device strings: Mfr=3, Product=2,
SerialNumber=0
[   66.343188] usb 3-1.2: Product: HUAWEI Mobile
[   66.343193] usb 3-1.2: Manufacturer: HUAWEI Technology
[   66.388091] usb-storage 3-1.2:1.0: USB Mass Storage device detected
[   66.388430] scsi host6: usb-storage 3-1.2:1.0
[   66.388646] usb-storage 3-1.2:1.1: USB Mass Storage device detected
[   66.388805] scsi host7: usb-storage 3-1.2:1.1
[   66.389042] usbcore: registered new interface driver usb-storage
[   67.389689] scsi 6:0:0:0: CD-ROM            HUAWEI   Mass Storage
  2.31 PQ: 0 ANSI: 2
[   67.389737] scsi 7:0:0:0: Direct-Access     HUAWEI   SD Storage
  2.31 PQ: 0 ANSI: 2
[   67.393600] sr 6:0:0:0: [sr0] scsi-1 drive
[   67.393606] cdrom: Uniform CD-ROM driver Revision: 3.20
[   67.394901] sr 6:0:0:0: Attached scsi CD-ROM sr0
[   67.395973] sr 6:0:0:0: Attached scsi generic sg2 type 5
[   67.403214] sd 7:0:0:0: Attached scsi generic sg3 type 0
[   67.411095] sd 7:0:0:0: [sdc] Attached SCSI removable disk
[   67.834115] usb 3-1.2: USB disconnect, device number 4
[   71.612271] usb 3-1.2: new high-speed USB device number 5 using ehci-pci
[   71.707111] usb 3-1.2: New USB device found, idVendor=12d1, idProduct=1436
[   71.707123] usb 3-1.2: New USB device strings: Mfr=4, Product=3,
SerialNumber=0
[   71.707129] usb 3-1.2: Product: HUAWEI Mobile
[   71.707142] usb 3-1.2: Manufacturer: HUAWEI Technology
[   71.710324] usb-storage 3-1.2:1.0: USB Mass Storage device detected
[   71.711244] usb-storage 3-1.2:1.1: USB Mass Storage device detected
[   71.711563] usb-storage 3-1.2:1.2: USB Mass Storage device detected
[   71.711928] usb-storage 3-1.2:1.3: USB Mass Storage device detected
[   71.712279] usb-storage 3-1.2:1.4: USB Mass Storage device detected
[   71.712661] usb-storage 3-1.2:1.5: USB Mass Storage device detected
[   71.713143] scsi host13: usb-storage 3-1.2:1.5
[   71.713564] usb-storage 3-1.2:1.6: USB Mass Storage device detected
[   71.714058] scsi host14: usb-storage 3-1.2:1.6
[   71.747682] usbcore: registered new interface driver usbserial
[   71.758867] cdc_ether 3-1.2:1.1 wwan0: register 'cdc_ether' at
usb-0000:00:1a.0-1.2, Mobile Broadband Network Device,
02:50:f3:00:00:00
[   71.758955] usbcore: registered new interface driver cdc_ether
[   71.759467] usbcore: registered new interface driver option
[   71.761290] usbserial: USB Serial support registered for GSM modem (1-port)
[   71.761687] option 3-1.2:1.0: GSM modem (1-port) converter detected
[   71.762458] usb 3-1.2: GSM modem (1-port) converter now attached to ttyUSB0
[   71.762697] option 3-1.2:1.3: GSM modem (1-port) converter detected
[   71.763122] usb 3-1.2: GSM modem (1-port) converter now attached to ttyUSB1
[   71.763179] option 3-1.2:1.4: GSM modem (1-port) converter detected
[   71.763428] usb 3-1.2: GSM modem (1-port) converter now attached to ttyUSB2
[   72.713937] scsi 13:0:0:0: CD-ROM            HUAWEI   Mass Storage
   2.31 PQ: 0 ANSI: 2
[   72.713995] scsi 14:0:0:0: Direct-Access     HUAWEI   SD Storage
   2.31 PQ: 0 ANSI: 2
[   72.716804] sr 13:0:0:0: [sr0] scsi-1 drive
[   72.717888] sr 13:0:0:0: Attached scsi CD-ROM sr0
[   72.718765] sr 13:0:0:0: Attached scsi generic sg2 type 5
[   72.723309] sd 14:0:0:0: Attached scsi generic sg3 type 0
[   72.728186] sd 14:0:0:0: [sdc] Attached SCSI removable disk
[  107.625267] PPP BSD Compression module registered
[  107.632006] PPP Deflate Compression module registered
- EOT -


> Dan
>
>> ### PPP OPTIONS
>> CONFIG_PPP=y
>> CONFIG_PPP_BSDCOMP=m
>> CONFIG_PPP_DEFLATE=m
>> CONFIG_PPP_FILTER=y
>> CONFIG_PPP_MULTILINK=y
>> CONFIG_PPP_ASYNC=m
>>
>> Beyond the PPP options, I wanted to understand what
>> CONFIG_USB_NET_CDCETHER does and why I need it.
>> Can someone help?
>
>
>
>
>> Thanks.
>>
>> - Sedat -
>>
>> [1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/usb/Kconfig#n189
>>
>> P.S.: cdc_ether Kconfig option and checking my logs
>>
>> From [1]...
>> ...
>> config USB_NET_CDCETHER
>> tristate "CDC Ethernet support (smart devices such as cable modems)"
>> depends on USB_USBNET
>> default y
>> help
>>  This option supports devices conforming to the Communication Device
>>  Class (CDC) Ethernet Control Model, a specification that's easy to
>>  implement in device firmware.  The CDC specifications are available
>>  from <http://www.usb.org/>.
>>
>>  CDC Ethernet is an implementation option for DOCSIS cable modems
>>  that support USB connectivity, used for non-Microsoft USB hosts.
>>  The Linux-USB CDC Ethernet Gadget driver is an open implementation.
>>    This driver should work with at least the following devices:
>>
>>    * Dell Wireless 5530 HSPA
>>      * Ericsson PipeRider (all variants)
>>    * Ericsson Mobile Broadband Module (all variants)
>>      * Motorola (DM100 and SB4100)
>>      * Broadcom Cable Modem (reference design)
>>    * Toshiba (PCX1100U and F3507g/F3607gw)
>>    * ...
>>
>>  This driver creates an interface named "ethX", where X depends on
>>  what other networking devices you have in use.  However, if the
>>  IEEE 802 "local assignment" bit is set in the address, a "usbX"
>>  name is used instead.
>> ...
>>
>> From my logs...
>>
>> $ dmesg | egrep -i 'option|wwan|ppp|3-1.2|huawei|gsm|modem'
>> [    0.000000] please try 'cgroup_disable=memory' option if you don't
>> want memory cgroups
>> [    0.549498] PPP generic driver version 2.4.2
>> [    1.299059] usb 3-1.2: new high-speed USB device number 3 using ehci-pci
>> [    1.394084] usb 3-1.2: New USB device found, idVendor=12d1, idProduct=1436
>> [    1.394095] usb 3-1.2: New USB device strings: Mfr=4, Product=3,
>> SerialNumber=0
>> [    1.394100] usb 3-1.2: Product: HUAWEI Mobile
>> [    1.394103] usb 3-1.2: Manufacturer: HUAWEI Technology
>> [    2.115424] usb-storage 3-1.2:1.0: USB Mass Storage device detected
>> [    2.125026] usb-storage 3-1.2:1.1: USB Mass Storage device detected
>> [    2.125607] usb-storage 3-1.2:1.2: USB Mass Storage device detected
>> [    2.125888] usb-storage 3-1.2:1.3: USB Mass Storage device detected
>> [    2.126187] usb-storage 3-1.2:1.4: USB Mass Storage device detected
>> [    2.126461] usb-storage 3-1.2:1.5: USB Mass Storage device detected
>> [    2.127098] scsi host11: usb-storage 3-1.2:1.5
>> [    2.129370] usb-storage 3-1.2:1.6: USB Mass Storage device detected
>> [    2.131685] scsi host12: usb-storage 3-1.2:1.6
>> [    3.127317] scsi 11:0:0:0: CD-ROM            HUAWEI   Mass Storage
>>    2.31 PQ: 0 ANSI: 2
>> [    3.137589] scsi 12:0:0:0: Direct-Access     HUAWEI   SD Storage
>>    2.31 PQ: 0 ANSI: 2
>> [   13.500302] cdc_ether 3-1.2:1.1 wwan0: register 'cdc_ether' at
>> usb-0000:00:1a.0-1.2, Mobile Broadband Network Device,
>> 02:50:f3:00:00:00
>> [   14.160221] usbcore: registered new interface driver option
>> [   14.160820] usbserial: USB Serial support registered for GSM modem (1-port)
>> [   14.160940] option 3-1.2:1.0: GSM modem (1-port) converter detected
>> [   14.163032] usb 3-1.2: GSM modem (1-port) converter now attached to ttyUSB0
>> [   14.163305] option 3-1.2:1.3: GSM modem (1-port) converter detected
>> [   14.163676] usb 3-1.2: GSM modem (1-port) converter now attached to ttyUSB1
>> [   14.163742] option 3-1.2:1.4: GSM modem (1-port) converter detected
>> [   14.165227] usb 3-1.2: GSM modem (1-port) converter now attached to ttyUSB2
>> [   72.877065] PPP BSD Compression module registered
>> [   72.881701] PPP Deflate Compression module registered
>> - EOT -
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

^ permalink raw reply

* Re: [PATCH 4/4] rhashtable: Add parent argument to mutex_is_held
From: Thomas Graf @ 2014-11-15 11:16 UTC (permalink / raw)
  To: Herbert Xu; +Cc: netdev, eric.dumazet, paulmck, josh
In-Reply-To: <20141115032512.GA19299@gondor.apana.org.au>

On 11/15/14 at 11:25am, Herbert Xu wrote:
> On Thu, Nov 13, 2014 at 06:43:43PM +0800, Herbert Xu wrote:
> > On Thu, Nov 13, 2014 at 10:41:24AM +0000, Thomas Graf wrote:
> > >
> > > Never mind. You did fix it. I looked at the wrong patch.
> > 
> > OK.  Now comes the fun part of shoehorning the xfrm_policy bydst
> > hash into rhashtable :)
> 
> Thomas, it appears that rhashtable as it stands cannot handle
> changing the random seed because of the way it constructs the
> new hash table without degrading into a linked list.  Is there
> something I'm missing?
> 
> FWIW my hashtable in net/bridge/br_multicast.c handles rehashing
> correctly.  Any objections to me converting rhashtable to use my
> scheme instead?

Can you elaborate a bit?

The point of rhashtable is to not require two sets of linked list
pointers as done by MDB or OVS flow tables to work around the
increased cache footprint of that approach. The difference of the
two algos is dicussed in this paper [0].

The disadvantage of rhashtable is that, AFAIK, the hash function
cannot change while resizing as it would break the mutual linked
lists.

rhashtable will eventually be converted to per bucket locks with
deferred resizing to allow concurrent insertions and removal to
multiple buckets and to allow for mutations from atomic context
which is needed to use it for TCP hashtables. This work is in
progress:
http://lwn.net/Articles/611951/

Eric mentioned he is working on the rcu'ification of sockets which
is what is missing to make it work.

[0] https://www.usenix.org/legacy/event/atc11/tech/final_files/Triplett.pdf

^ permalink raw reply

* Re: [PATCH 4/4] rhashtable: Add parent argument to mutex_is_held
From: Herbert Xu @ 2014-11-15 11:23 UTC (permalink / raw)
  To: Thomas Graf; +Cc: netdev, eric.dumazet, paulmck, josh, David S. Miller
In-Reply-To: <20141115111626.GP19157@casper.infradead.org>

On Sat, Nov 15, 2014 at 11:16:26AM +0000, Thomas Graf wrote:
>
> > Thomas, it appears that rhashtable as it stands cannot handle
> > changing the random seed because of the way it constructs the
> > new hash table without degrading into a linked list.  Is there
> > something I'm missing?
> > 
> > FWIW my hashtable in net/bridge/br_multicast.c handles rehashing
> > correctly.  Any objections to me converting rhashtable to use my
> > scheme instead?
> 
> Can you elaborate a bit?
> 
> The point of rhashtable is to not require two sets of linked list
> pointers as done by MDB or OVS flow tables to work around the
> increased cache footprint of that approach. The difference of the
> two algos is dicussed in this paper [0].
> 
> The disadvantage of rhashtable is that, AFAIK, the hash function
> cannot change while resizing as it would break the mutual linked
> lists.

Well it doesn't break so much as degenerate into a linked list (I'm
talking about the concept rather than what the current code does).

This is I think a show-stopper because for anything that can be
influenced by remote parties we have to reseed and therefore
be able to cope with hashes changing on the fly.  Since most
hash tables in the network stack can be influenced by remote
entities (including the xfrm policy bydst hash that I am currently
working on), this is something rhashtable must be able to support
if it is to be used throughout the network stack.

Unless there is a better solution then I think keeping two lists
will have to do.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: phy/micrel: KSZ8031RNL RMII clock reconfiguration bug
From: Johan Hovold @ 2014-11-15 14:18 UTC (permalink / raw)
  To: Bruno Thomsen
  Cc: Johan Hovold, netdev@vger.kernel.org, f.fainelli@gmail.com,
	s.hauer@pengutronix.de, bruno.thomsen@gmail.com,
	linux-kernel@vger.kernel.org
In-Reply-To: <915054555B5659448ACF8A70E114824D01704B9131@Exchange2010.kamstrup.dk>

On Wed, Nov 12, 2014 at 12:17:57PM +0000, Bruno Thomsen wrote:
> Hi Johan,
> 
> > As you may have seen by now, I've been working on refactoring the
> > micrel phy driver to be able to use common initialisation code.
> >
> > Specifically, I've added generic support for disabling the broadcast
> > address, which is what the MII_KSZPHY_OMSO write above does.
> >
> > Generally you want this to be the first thing you do in order to
> > avoid unnecessary reconfigurations. If we ever were to allow
> > concurrent probing this would also be a requirement.
> >
> > Could you provide some detail about the setup were you find that the
> > PHY becomes unresponsive without your patch? Do you have more than
> > one PHY on the bus? Using what addresses? And using what clock modes
> > (i.e. 25 MHz or 50 MHz)?
> > 
> > Also, what exactly do you mean by "unresponsive"? Are you still able
> > to read the PHY registers for example?
>  
> I think it sounds like a good idea to refactor the init code.
> 
> My setup:
> iMX28 processor with dual Ethernet MAC; FEC0 (enabled) and FEC1 (disabled).
> There is a single KSZ8031 PHY that receives 50MHz RMII clock from the MAC.
> I am unable to read PHY registers from both user-land tools and extra
> debug PHY reads in driver code.

Did you specify a led-mode as well, or was the Operation Mode Strap
Override (OMSO) write the first access after the soft reset?

Did you try any other workarounds besides setting the clock mode before
doing the OMSO write?

And REF_CLK (pin 16) is not connected? 

> Boot trace:
> [   22.277785] fec 800f0000.ethernet eth0: Freescale FEC PHY driver [Micrel KSZ8031] (mii_bus:phy_addr=800f0000.etherne:00, irq=-1)
> [   22.292527] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
> [   24.276217] libphy: 800f0000.etherne:00 - Link is Up - 100/Full
> [   24.285094] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready

Ok, so you use a single PHY strapped to address 0. 

Would you able to test my series on your setup, and possibly a couple of
diagnostic patches on top?

Thanks,
Johan

^ permalink raw reply

* Re: [PATCH 4/4] rhashtable: Add parent argument to mutex_is_held
From: Herbert Xu @ 2014-11-15 15:51 UTC (permalink / raw)
  To: Thomas Graf; +Cc: netdev, eric.dumazet, paulmck, josh, David S. Miller
In-Reply-To: <20141115112313.GA20970@gondor.apana.org.au>

On Sat, Nov 15, 2014 at 07:23:13PM +0800, Herbert Xu wrote:
> 
> This is I think a show-stopper because for anything that can be
> influenced by remote parties we have to reseed and therefore
> be able to cope with hashes changing on the fly.  Since most
> hash tables in the network stack can be influenced by remote
> entities (including the xfrm policy bydst hash that I am currently
> working on), this is something rhashtable must be able to support
> if it is to be used throughout the network stack.

So I noticed that you got rid of the rehash when you converted
netlink over to rthashtable.  Was this aspect of the conversion
discussed anywhere? In particular, how do you protect against
a malicious user that's trying to attack the netlink hash table?

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [net-next 03/12] i40e: Handle a single mss packet with more than 8 frags
From: Eric Dumazet @ 2014-11-15 18:21 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: davem, Serey Kong, netdev, nhorman, sassmann, jogreene,
	Shannon Nelson
In-Reply-To: <1416031715-32498-4-git-send-email-jeffrey.t.kirsher@intel.com>

On Fri, 2014-11-14 at 22:08 -0800, Jeff Kirsher wrote:
> From: Serey Kong <serey.kong@intel.com>
> 
> This handles the case where a single packet with more than 8 data
> descriptors triggers a Malicious Driver Detect event in the device.
> 
> Change-ID: I2bfd657fd036bed616863e121f6aa86a35d2194c
> Signed-off-by: Serey Kong <serey.kong@intel.com>
> Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
> Tested-by: Jim Young <jamesx.m.young@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 14 ++++++++++----
>  drivers/net/ethernet/intel/i40e/i40e_txrx.h   |  1 +
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 14 ++++++++++----
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.h |  1 +
>  4 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index 3195d82..9731026 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> @@ -2113,12 +2113,12 @@ static void i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
>  			const u8 hdr_len, u32 td_cmd, u32 td_offset)
>  #endif
>  {
> -	unsigned int data_len = skb->data_len;
> -	unsigned int size = skb_headlen(skb);
>  	struct skb_frag_struct *frag;
>  	struct i40e_tx_buffer *tx_bi;
>  	struct i40e_tx_desc *tx_desc;
>  	u16 i = tx_ring->next_to_use;
> +	unsigned int data_len;
> +	unsigned int size;
>  	u32 td_tag = 0;
>  	dma_addr_t dma;
>  	u16 gso_segs;
> @@ -2129,10 +2129,16 @@ static void i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
>  			 I40E_TX_FLAGS_VLAN_SHIFT;
>  	}
>  
> -	if (tx_flags & (I40E_TX_FLAGS_TSO | I40E_TX_FLAGS_FSO))
> +	if (tx_flags & (I40E_TX_FLAGS_TSO | I40E_TX_FLAGS_FSO)) {
>  		gso_segs = skb_shinfo(skb)->gso_segs;
> -	else
> +	} else {
>  		gso_segs = 1;
> +		if (skb_shinfo(skb)->nr_frags >= I40E_MAX_BUFFER_TXD)
> +			skb_linearize(skb);

What exactly happens if skb_linearize() fails ?

Is this "Malicious Driver Detect event" fatal or simply packet is
dropped without additional harm ?

^ permalink raw reply

* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Tom Herbert @ 2014-11-15 18:41 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Eric Dumazet, Michael Kerrisk, David Miller, netdev, Ying Cai,
	Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <CALCETrU5Og8tfkFuA-rPY5evV5CkPsx=22f6mePHJPvfcRrBdA@mail.gmail.com>

On Fri, Nov 14, 2014 at 4:50 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Fri, Nov 14, 2014 at 4:40 PM, Tom Herbert <therbert@google.com> wrote:
>> On Fri, Nov 14, 2014 at 4:24 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> On Fri, Nov 14, 2014 at 4:06 PM, Tom Herbert <therbert@google.com> wrote:
>>>> On Fri, Nov 14, 2014 at 2:10 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>> On Fri, Nov 14, 2014 at 1:36 PM, Tom Herbert <therbert@google.com> wrote:
>>>>>> On Fri, Nov 14, 2014 at 12:34 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>>>> On Fri, Nov 14, 2014 at 12:25 PM, Tom Herbert <therbert@google.com> wrote:
>>>>>>>> On Fri, Nov 14, 2014 at 12:16 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>>>>>> On Fri, Nov 14, 2014 at 11:52 AM, Tom Herbert <therbert@google.com> wrote:
>>>>>>>>>> On Fri, Nov 14, 2014 at 11:33 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>>>>>>>>>> On Fri, 2014-11-14 at 09:17 -0800, Andy Lutomirski wrote:
>>>>>>>>>>>
>>>>>>>>>>>> As a heavy user of RFS (and finder of bugs in it, too), here's my
>>>>>>>>>>>> question about this API:
>>>>>>>>>>>>
>>>>>>>>>>>> How does an application tell whether the socket represents a
>>>>>>>>>>>> non-actively-steered flow?  If the flow is subject to RFS, then moving
>>>>>>>>>>>> the application handling to the socket's CPU seems problematic, as the
>>>>>>>>>>>> socket's CPU might move as well.  The current implementation in this
>>>>>>>>>>>> patch seems to tell me which CPU the most recent packet came in on,
>>>>>>>>>>>> which is not necessarily very useful.
>>>>>>>>>>>
>>>>>>>>>>> Its the cpu that hit the TCP stack, bringing dozens of cache lines in
>>>>>>>>>>> its cache. This is all that matters,
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Some possibilities:
>>>>>>>>>>>>
>>>>>>>>>>>> 1. Let SO_INCOMING_CPU fail if RFS or RPS are in play.
>>>>>>>>>>>
>>>>>>>>>>> Well, idea is to not use RFS at all. Otherwise, it is useless.
>>>>>>>>>
>>>>>>>>> Sure, but how do I know that it'll be the same CPU next time?
>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>> Bear in mind this is only an interface to report RX CPU and in itself
>>>>>>>>>> doesn't provide any functionality for changing scheduling, there is
>>>>>>>>>> obviously logic needed in user space that would need to do something.
>>>>>>>>>>
>>>>>>>>>> If we track the interrupting CPU in skb, the interface could be easily
>>>>>>>>>> extended to provide the interrupting CPU, the RPS CPU (calculated at
>>>>>>>>>> reported time), and the CPU processing transport (post steering which
>>>>>>>>>> is what is currently returned). That would provide the complete
>>>>>>>>>> picture to control scheduling a flow from userspace, and an interface
>>>>>>>>>> to selectively turn off RFS for a socket would make sense then.
>>>>>>>>>
>>>>>>>>> I think that a turn-off-RFS interface would also want a way to figure
>>>>>>>>> out where the flow would go without RFS.  Can the network stack do
>>>>>>>>> that (e.g. evaluate the rx indirection hash or whatever happens these
>>>>>>>>> days)?
>>>>>>>>>
>>>>>>>> Yes,. We need the rxhash and the CPU that packets are received on from
>>>>>>>> the device for the socket. The former we already have, the latter
>>>>>>>> might be done by adding a field to skbuff to set received CPU. Given
>>>>>>>> the L4 hash and interrupting CPU we can calculated the RPS CPU which
>>>>>>>> is where packet would have landed with RFS off.
>>>>>>>
>>>>>>> Hmm.  I think this would be useful for me.  It would *definitely* be
>>>>>>> useful for me if I could pin an RFS flow to a cpu of my choice.
>>>>>>>
>>>>>> Andy, can you elaborate a little more on your use case. I've thought
>>>>>> several times about an interface to program the flow table from
>>>>>> userspace, but never quite came up with a compelling use case and
>>>>>> there is the security concern that a user could "steal" cycles from
>>>>>> arbitrary CPUs.
>>>>>
>>>>> I have a bunch of threads that are pinned to various CPUs or groups of
>>>>> CPUs.  Each thread is responsible for a fixed set of flows.  I'd like
>>>>> those flows to go to those CPUs.
>>>>>
>>>>> RFS will eventually do it, but it would be nice if I could
>>>>> deterministically ask for a flow to be routed to the right CPU.  Also,
>>>>> if my thread bounces temporarily to another CPU, I don't really need
>>>>> the flow to follow it -- I'd like it to stay put.
>>>>>
>>>> Okay, how about it we have a SO_RFS_LOCK_FLOW sockopt. When this is
>>>> called on a socket we can lock the socket to CPU binding to the
>>>> current CPU it is called from. It could be unlocked at a later point.
>>>> Would this satisfy your requirements?
>>>
>>> Yes, I think.  Especially if it bypassed the hash table.
>>
>> Unfortunately we can't easily bypass the hash table. The only way I
>> know of to to do that is to perform the socket lookup to do steering
>> (I tried that early on, but it was pretty costly).
>
> What happens if you just call ndo_rx_flow_steer and do something to
> keep the result from expiring?
>
Okay, I will look at that. Do you know how many flows we are talking
about, both in the number you need and the number that can be put in
the HW without collision?

Thanks,
Tom

>>>
>>>> As I mentioned, there is no material functionality in this patch and
>>>> it should be independent of RFS. It simply returns the CPU where the
>>>> stack processed the packet. Whether or not this is meaningful
>>>> information to the algorithm being implemented in userspace is
>>>> completely up to the caller to decide.
>>>
>>> Agreed.
>>>
>>> My only concern is that writing that userspace algorithm might result
>>> in surprises if RFS is on.  Having the user program notice the problem
>>> early and alert the admin might help keep Murphy's Law at bay here.
>>>
>> By Murphy's law we'd also have to consider that the flow hash could
>> change after reading the results so that the scheduling done in
>> userspace is completely wrong until the CPU is read again.
>> Synchronizing kernel and device state with userspace state is not
>> always so easy. One way to mitigate is to use ancillary data which
>> would provide real time information and obviate the need for another
>> system call.
>
> Hmm.  That would work, too.  I don't know how annoyed user code would
> be at having to read ancillary data, though.
>
> The flow hash really shouldn't change much, though, right?
>
> --Andy

^ permalink raw reply

* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Andy Lutomirski @ 2014-11-15 18:47 UTC (permalink / raw)
  To: Tom Herbert
  Cc: Eric Dumazet, Michael Kerrisk, David Miller, netdev, Ying Cai,
	Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <CA+mtBx-=fbA+pW35K8cjXTJpw30jhawy1mhs-MDEYv9SCyD2MQ@mail.gmail.com>

On Sat, Nov 15, 2014 at 10:41 AM, Tom Herbert <therbert@google.com> wrote:
> On Fri, Nov 14, 2014 at 4:50 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Fri, Nov 14, 2014 at 4:40 PM, Tom Herbert <therbert@google.com> wrote:
>>> On Fri, Nov 14, 2014 at 4:24 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>> On Fri, Nov 14, 2014 at 4:06 PM, Tom Herbert <therbert@google.com> wrote:
>>>>> On Fri, Nov 14, 2014 at 2:10 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>>> On Fri, Nov 14, 2014 at 1:36 PM, Tom Herbert <therbert@google.com> wrote:
>>>>>>> On Fri, Nov 14, 2014 at 12:34 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>>>>> On Fri, Nov 14, 2014 at 12:25 PM, Tom Herbert <therbert@google.com> wrote:
>>>>>>>>> On Fri, Nov 14, 2014 at 12:16 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>>>>>>> On Fri, Nov 14, 2014 at 11:52 AM, Tom Herbert <therbert@google.com> wrote:
>>>>>>>>>>> On Fri, Nov 14, 2014 at 11:33 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>>>>>>>>>>> On Fri, 2014-11-14 at 09:17 -0800, Andy Lutomirski wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> As a heavy user of RFS (and finder of bugs in it, too), here's my
>>>>>>>>>>>>> question about this API:
>>>>>>>>>>>>>
>>>>>>>>>>>>> How does an application tell whether the socket represents a
>>>>>>>>>>>>> non-actively-steered flow?  If the flow is subject to RFS, then moving
>>>>>>>>>>>>> the application handling to the socket's CPU seems problematic, as the
>>>>>>>>>>>>> socket's CPU might move as well.  The current implementation in this
>>>>>>>>>>>>> patch seems to tell me which CPU the most recent packet came in on,
>>>>>>>>>>>>> which is not necessarily very useful.
>>>>>>>>>>>>
>>>>>>>>>>>> Its the cpu that hit the TCP stack, bringing dozens of cache lines in
>>>>>>>>>>>> its cache. This is all that matters,
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Some possibilities:
>>>>>>>>>>>>>
>>>>>>>>>>>>> 1. Let SO_INCOMING_CPU fail if RFS or RPS are in play.
>>>>>>>>>>>>
>>>>>>>>>>>> Well, idea is to not use RFS at all. Otherwise, it is useless.
>>>>>>>>>>
>>>>>>>>>> Sure, but how do I know that it'll be the same CPU next time?
>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>> Bear in mind this is only an interface to report RX CPU and in itself
>>>>>>>>>>> doesn't provide any functionality for changing scheduling, there is
>>>>>>>>>>> obviously logic needed in user space that would need to do something.
>>>>>>>>>>>
>>>>>>>>>>> If we track the interrupting CPU in skb, the interface could be easily
>>>>>>>>>>> extended to provide the interrupting CPU, the RPS CPU (calculated at
>>>>>>>>>>> reported time), and the CPU processing transport (post steering which
>>>>>>>>>>> is what is currently returned). That would provide the complete
>>>>>>>>>>> picture to control scheduling a flow from userspace, and an interface
>>>>>>>>>>> to selectively turn off RFS for a socket would make sense then.
>>>>>>>>>>
>>>>>>>>>> I think that a turn-off-RFS interface would also want a way to figure
>>>>>>>>>> out where the flow would go without RFS.  Can the network stack do
>>>>>>>>>> that (e.g. evaluate the rx indirection hash or whatever happens these
>>>>>>>>>> days)?
>>>>>>>>>>
>>>>>>>>> Yes,. We need the rxhash and the CPU that packets are received on from
>>>>>>>>> the device for the socket. The former we already have, the latter
>>>>>>>>> might be done by adding a field to skbuff to set received CPU. Given
>>>>>>>>> the L4 hash and interrupting CPU we can calculated the RPS CPU which
>>>>>>>>> is where packet would have landed with RFS off.
>>>>>>>>
>>>>>>>> Hmm.  I think this would be useful for me.  It would *definitely* be
>>>>>>>> useful for me if I could pin an RFS flow to a cpu of my choice.
>>>>>>>>
>>>>>>> Andy, can you elaborate a little more on your use case. I've thought
>>>>>>> several times about an interface to program the flow table from
>>>>>>> userspace, but never quite came up with a compelling use case and
>>>>>>> there is the security concern that a user could "steal" cycles from
>>>>>>> arbitrary CPUs.
>>>>>>
>>>>>> I have a bunch of threads that are pinned to various CPUs or groups of
>>>>>> CPUs.  Each thread is responsible for a fixed set of flows.  I'd like
>>>>>> those flows to go to those CPUs.
>>>>>>
>>>>>> RFS will eventually do it, but it would be nice if I could
>>>>>> deterministically ask for a flow to be routed to the right CPU.  Also,
>>>>>> if my thread bounces temporarily to another CPU, I don't really need
>>>>>> the flow to follow it -- I'd like it to stay put.
>>>>>>
>>>>> Okay, how about it we have a SO_RFS_LOCK_FLOW sockopt. When this is
>>>>> called on a socket we can lock the socket to CPU binding to the
>>>>> current CPU it is called from. It could be unlocked at a later point.
>>>>> Would this satisfy your requirements?
>>>>
>>>> Yes, I think.  Especially if it bypassed the hash table.
>>>
>>> Unfortunately we can't easily bypass the hash table. The only way I
>>> know of to to do that is to perform the socket lookup to do steering
>>> (I tried that early on, but it was pretty costly).
>>
>> What happens if you just call ndo_rx_flow_steer and do something to
>> keep the result from expiring?
>>
> Okay, I will look at that. Do you know how many flows we are talking
> about, both in the number you need and the number that can be put in
> the HW without collision?

I think I have on the order of 100 flows.  Maybe 400.  It's been a few
months since I checked, and this particular metric is much easier to
measure on a weekday.

I think the HW has several thousand slots.

--Andy

^ permalink raw reply

* Re: [PATCH 4/4] rhashtable: Add parent argument to mutex_is_held
From: Josh Triplett @ 2014-11-15 19:12 UTC (permalink / raw)
  To: Thomas Graf; +Cc: Herbert Xu, netdev, eric.dumazet, paulmck
In-Reply-To: <20141115111626.GP19157@casper.infradead.org>

On Sat, Nov 15, 2014 at 11:16:26AM +0000, Thomas Graf wrote:
> On 11/15/14 at 11:25am, Herbert Xu wrote:
> > On Thu, Nov 13, 2014 at 06:43:43PM +0800, Herbert Xu wrote:
> > > On Thu, Nov 13, 2014 at 10:41:24AM +0000, Thomas Graf wrote:
> > > >
> > > > Never mind. You did fix it. I looked at the wrong patch.
> > > 
> > > OK.  Now comes the fun part of shoehorning the xfrm_policy bydst
> > > hash into rhashtable :)
> > 
> > Thomas, it appears that rhashtable as it stands cannot handle
> > changing the random seed because of the way it constructs the
> > new hash table without degrading into a linked list.  Is there
> > something I'm missing?
> > 
> > FWIW my hashtable in net/bridge/br_multicast.c handles rehashing
> > correctly.  Any objections to me converting rhashtable to use my
> > scheme instead?
> 
> Can you elaborate a bit?
> 
> The point of rhashtable is to not require two sets of linked list
> pointers as done by MDB or OVS flow tables to work around the
> increased cache footprint of that approach. The difference of the
> two algos is dicussed in this paper [0].
> 
> The disadvantage of rhashtable is that, AFAIK, the hash function
> cannot change while resizing as it would break the mutual linked
> lists.

You can handle hash function changes with rhashtable without needing a
second set of linked-list pointers, actually.  You'd have to add ~1
unlikely() conditional to the reader common case, and you'd make readers
*during* a hash algorithm change (which I'd hope happens as rarely as a
resize) somewhat less efficient, but you wouldn't use any more memory
than a resize currently does, and you wouldn't use the memory of extra
linked-list pointers in the common case.

Rather than the current approach of switching out the hash table pointer
and having readers only search the new table (which will have
valid-but-crosslinked buckets during a resize), instead keep two hash
table pointers (each with their own hash parameters) and a single
toggleable old/new indicator.  Readers check both hash table pointers,
and if valid, check both tables, first old then new (order important to
make sure they don't miss a node).  Then, the rehashing algorithm can
incrementally move nodes from the old buckets to the new buckets,
*without* disrupting concurrent readers.

Rough rehashing algorithm sketch:

- Set up the new empty table with the new set of hash parameters.
- synchronize_rcu().  Readers will now search both old and new tables.
- Peel nodes off the ends of the old hash table and add them to the new
  hash table (similar to the unzip step in the resize algorithm).
  Because readers search old-then-new, and writers make each node appear
  in the new table before making it disappear from the old, you don't
  need a synchronize_rcu() here, just an smp_wmb() after linking the
  node into the new table and before unlinking the node from the old
  table.  Also, because you remove nodes from the *ends* of old-table
  buckets, you don't have to worry about a reader's linked-list walk
  getting dragged over to the new table and missing nodes from the old
  one.
- Once all nodes have moved to the new table, mark the pointer to the
  old table as NULL, synchronize_rcu(), and free the old table.
- Toggle the old/new flag.

Ordering principles in this algorithm:
- You want readers to see the changes made by the rehasher in order.
- If the reader reads location A then B, and the rehasher writes
  location B then A, the rehasher just needs an smp_wmb() in between.
- If the reader reads location A then B, and the rehasher writes
  location A then B, the rehasher needs a synchronize_rcu() in between.


- Josh Triplett

^ permalink raw reply

* Re: Understanding what's going on when using a Huawei E173 USB 3G web-stick (UMTS/HSPA)
From: Greg KH @ 2014-11-15 20:07 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Dan Williams, David S. Miller, netdev@vger.kernel.org, linux-usb,
	Aleksander Morgado
In-Reply-To: <CA+icZUUWK8R0zFLBHfvfE6zmZ7=+yP_FJgkkeELVbwdhPUYfRA@mail.gmail.com>

On Sat, Nov 15, 2014 at 10:23:55AM +0100, Sedat Dilek wrote:
> On Fri, Nov 14, 2014 at 5:18 PM, Dan Williams <dcbw@redhat.com> wrote:
> > On Fri, 2014-11-14 at 11:56 +0100, Sedat Dilek wrote:
> >> On Wed, Nov 12, 2014 at 2:21 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >> > On Tue, Nov 4, 2014 at 5:55 PM, Dan Williams <dcbw@redhat.com> wrote:
> >> >> On Tue, 2014-11-04 at 16:11 +0100, Sedat Dilek wrote:
> >> >>> Hi,
> >> >>>
> >> >>> I wanted to understand what is going on the kernel-side when
> >> >>> connecting to the Internet via a Huawei E173 USB web-stick (3rd
> >> >>> Generation: UMTS / HSPA).
> >> >>>
> >> >>> Especially the correlation between the diverse USB/NET kernel-drivers
> >> >>> and how the networking is setup.
> >> >>
> >> >
> >> > [ Sitting in front of a foreign Windows machine ]
> >> >
> >> > [ CC Aleksander ]
> >> >
> >> > Hi Dan,
> >> >
> >> > sorry for the late (and short) response.
> >> >
> >> > AFAICS you have given a "skeleton" for a "usb-wwan-networking"
> >> > documentation :-).
> >> >
> >> > Personally, I would like to take into account some kernel-config
> >> > options and some more things.
> >> >
> >>
> >> I started with documenting...
> >>
> >> I have still some difficulties in understanding USB WWAN Networking.
> >> So, this is what I revealed...
> >>
> >> ##### USB: HUAWEI E173 3G/UMTS/HSPA INTERNET STICK
> >>
> >> ### USB-NETWORKING AND WWAN SETUP
> >> CONFIG_USB_USBNET=m        <--- usb networking
> >> CONFIG_USB_NET_CDCETHER=m  <--- usb-wwan (net) configuration
> >> CONFIG_USB_SERIAL_WWAN=m   <--- usb-wwan (serial) configuration
> >> CONFIG_USB_SERIAL_OPTION=m <--- usb-serial driver called "option"
> >
> > Most WWAN devices actually require option, because most WWAN devices
> > have "serial" ports (even if they aren't used for PPP), and 'option' is
> > the driver that handles this.  The 'option' name is historic, but the
> > driver should really be called something like 'wwan-serial-generic' or
> > something like that.
> >
> 
> Is there sth. against renaming the "option" driver to "wwan-serial-generic"?

Yes, people's scripts might break that are hard-coded to use the
"option" driver.

thanks,

greg "here, have a vowel, they are cheap" k-h

^ permalink raw reply

* Re: [PATCH 2/3] r8169: Use load_acquire() and store_release() to reduce memory barrier overhead
From: Francois Romieu @ 2014-11-15 21:13 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Alexander Duyck, linux-arch, netdev, linux-kernel, mikey,
	tony.luck, mathieu.desnoyers, donald.c.skidmore, peterz, benh,
	heiko.carstens, oleg, will.deacon, davem, michael, matthew.vick,
	nic_swsd, geert, jeffrey.t.kirsher, fweisbec, schwidefsky, linux,
	paulmck, torvalds, mingo, Adam Nielsen
In-Reply-To: <54653ABF.3080400@gmail.com>

Alexander Duyck <alexander.duyck@gmail.com> :
> On 11/13/2014 01:30 PM, Francois Romieu wrote:
> > Alexander Duyck <alexander.h.duyck@redhat.com> :
> > [...]
> >> In addition the r8169 uses a rmb() however I believe it is placed incorrectly
> >> as I assume it supposed to be ordering descriptor reads after the check for
> >> ownership.
> > Not exactly. It's a barrier against compiler optimization from 2004.
> > It should not matter.
> 
> Okay.  Do you recall the kind of problem it was you were seeing ?

Mildly, I had to grep the local archives.

The relevant code used to be included in the irq handler at that time
(napi support for this driver took place in may 2004). One did not want
a runaway loop in the Tx reaper.

Compiler optimization was suggested by Manfred Spraul in the thread below:
http://marc.info/?l=linux-kernel&m=108096868119004

> The origin of the rmb() for the Intel drivers was a PowerPC issue in
> which it was fetching the length of a buffer before it checked the DD
> bit (equivalent of DescOwn).  I'm wondering if the issue you were seeing
> was something similar where it had reordered reads in the descriptor to
> cause that type of result.

The problem was only reported on Intel 32 bit + slackware + gcc 3.2.3.

Adam Nielsen - Cc: added - did not return for this bug.

-- 
Ueimor

^ permalink raw reply

* Re: [net-next 01/12] i40e: only warn once of PTP nonsupport in 100Mbit speed
From: Florian Fainelli @ 2014-11-15 21:20 UTC (permalink / raw)
  To: Jeff Kirsher, davem
  Cc: Shannon Nelson, netdev, nhorman, sassmann, jogreene, joe
In-Reply-To: <1416031715-32498-2-git-send-email-jeffrey.t.kirsher@intel.com>

Le 14/11/2014 22:08, Jeff Kirsher a écrit :
> From: Shannon Nelson <shannon.nelson@intel.com>
>
> Only warn once that PTP is not supported when linked at 100Mbit.
>
> Yes, using a static this way means that this once-only message is not
> port specific, but once only for the life of the driver, regardless of
> the number of ports.  That should be plenty.
>
> Change-ID: Ie6476530056df408452e195ef06afd4f57caa4b2
> Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_ptp.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> index 537b621..f9151037 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> @@ -382,11 +382,17 @@ void i40e_ptp_set_increment(struct i40e_pf *pf)
>   		incval = I40E_PTP_1GB_INCVAL;
>   		break;
>   	case I40E_LINK_SPEED_100MB:
> -		dev_warn(&pf->pdev->dev,
> -			 "%s: 1588 functionality is not supported at 100 Mbps. Stopping the PHC.\n",
> -			 __func__);
> +	{
> +		static int warn_once;
> +
> +		if (!warn_once) {
> +			dev_warn(&pf->pdev->dev,
> +				 "1588 functionality is not supported at 100 Mbps. Stopping the PHC.\n");
> +			warn_once++;
> +		}

I am sure Joe could come up with dev_warn_once() and others pretty 
easily. As a temporary change you could use pr_warn_once("%s: ...\n", 
dev_name(&pf->pdev->dev) which is almost identical?

>   		incval = 0;
>   		break;
> +	}
>   	case I40E_LINK_SPEED_40GB:
>   	default:
>   		incval = I40E_PTP_40GB_INCVAL;
>

^ permalink raw reply

* [PATCH 0/3] Add temperature reading and registers dump to mv88e6171
From: Andrew Lunn @ 2014-11-15 21:24 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux, Andrew Lunn

These patches centralize the temperature sensor reading code, and then
make use of it with the mv88e6171 which has a compatible
sensor. Additionally, support is added for reading the mv88e6171 via
ethtool.

Andrew Lunn (3):
  net: dsa: Centralise code for reading the temperature sensor
  net: dsa: mv88e6171: Add support for reading the temperature
  net: dsa: mv88e6171: Add support for reading switch registers

 drivers/net/dsa/mv88e6123_61_65.c | 50 +--------------------------------------
 drivers/net/dsa/mv88e6171.c       | 21 ++++++++++++++--
 drivers/net/dsa/mv88e6xxx.c       | 48 +++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx.h       |  1 +
 4 files changed, 69 insertions(+), 51 deletions(-)

-- 
2.1.3

^ permalink raw reply

* [PATCH 2/3] net: dsa: mv88e6171: Add support for reading the temperature
From: Andrew Lunn @ 2014-11-15 21:24 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux, Andrew Lunn
In-Reply-To: <1416086694-15790-1-git-send-email-andrew@lunn.ch>

This chip also has a temperature sensor which can be read using the
common code. In order to use it, add the needed mutex protection for
accessing registers via the shared code.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6171.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index 537eeedece21..ed4c79778823 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -316,6 +316,8 @@ static int mv88e6171_setup(struct dsa_switch *ds)
 			return ret;
 	}
 
+	mutex_init(&ps->phy_mutex);
+
 	return 0;
 }
 
@@ -329,18 +331,28 @@ static int mv88e6171_port_to_phy_addr(int port)
 static int
 mv88e6171_phy_read(struct dsa_switch *ds, int port, int regnum)
 {
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int addr = mv88e6171_port_to_phy_addr(port);
+	int ret;
 
-	return mv88e6xxx_phy_read(ds, addr, regnum);
+	mutex_lock(&ps->phy_mutex);
+	ret = mv88e6xxx_phy_read(ds, addr, regnum);
+	mutex_unlock(&ps->phy_mutex);
+	return ret;
 }
 
 static int
 mv88e6171_phy_write(struct dsa_switch *ds,
 		    int port, int regnum, u16 val)
 {
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int addr = mv88e6171_port_to_phy_addr(port);
+	int ret;
 
-	return mv88e6xxx_phy_write(ds, addr, regnum, val);
+	mutex_lock(&ps->phy_mutex);
+	ret = mv88e6xxx_phy_write(ds, addr, regnum, val);
+	mutex_unlock(&ps->phy_mutex);
+	return ret;
 }
 
 static struct mv88e6xxx_hw_stat mv88e6171_hw_stats[] = {
@@ -408,6 +420,9 @@ struct dsa_switch_driver mv88e6171_switch_driver = {
 	.get_strings		= mv88e6171_get_strings,
 	.get_ethtool_stats	= mv88e6171_get_ethtool_stats,
 	.get_sset_count		= mv88e6171_get_sset_count,
+#ifdef CONFIG_NET_DSA_HWMON
+	.get_temp               = mv88e6xxx_get_temp,
+#endif
 };
 
 MODULE_ALIAS("platform:mv88e6171");
-- 
2.1.3

^ permalink raw reply related

* [PATCH 3/3] net: dsa: mv88e6171: Add support for reading switch registers
From: Andrew Lunn @ 2014-11-15 21:24 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux, Andrew Lunn
In-Reply-To: <1416086694-15790-1-git-send-email-andrew@lunn.ch>

The infrastructure can now report switch registers to ethtool.
Add support for it to the mv88e6171 driver.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6171.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index ed4c79778823..aa33d16f2e22 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -423,6 +423,8 @@ struct dsa_switch_driver mv88e6171_switch_driver = {
 #ifdef CONFIG_NET_DSA_HWMON
 	.get_temp               = mv88e6xxx_get_temp,
 #endif
+	.get_regs_len		= mv88e6xxx_get_regs_len,
+	.get_regs		= mv88e6xxx_get_regs,
 };
 
 MODULE_ALIAS("platform:mv88e6171");
-- 
2.1.3

^ permalink raw reply related

* [PATCH 1/3] net: dsa: Centralise code for reading the temperature sensor
From: Andrew Lunn @ 2014-11-15 21:24 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux, Andrew Lunn
In-Reply-To: <1416086694-15790-1-git-send-email-andrew@lunn.ch>

The method to read the temperature used in the mve6123_61_65 driver
can also be used for other chips. Move the code into the shared code
base of mv88e6xxx.c.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6123_61_65.c | 50 +--------------------------------------
 drivers/net/dsa/mv88e6xxx.c       | 48 +++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx.h       |  1 +
 3 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/drivers/net/dsa/mv88e6123_61_65.c b/drivers/net/dsa/mv88e6123_61_65.c
index 9a3f9e0b0532..e9c736e1cef3 100644
--- a/drivers/net/dsa/mv88e6123_61_65.c
+++ b/drivers/net/dsa/mv88e6123_61_65.c
@@ -291,54 +291,6 @@ static int mv88e6123_61_65_setup_port(struct dsa_switch *ds, int p)
 	return 0;
 }
 
-#ifdef CONFIG_NET_DSA_HWMON
-
-static int  mv88e6123_61_65_get_temp(struct dsa_switch *ds, int *temp)
-{
-	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
-	int ret;
-	int val;
-
-	*temp = 0;
-
-	mutex_lock(&ps->phy_mutex);
-
-	ret = mv88e6xxx_phy_write(ds, 0x0, 0x16, 0x6);
-	if (ret < 0)
-		goto error;
-
-	/* Enable temperature sensor */
-	ret = mv88e6xxx_phy_read(ds, 0x0, 0x1a);
-	if (ret < 0)
-		goto error;
-
-	ret = mv88e6xxx_phy_write(ds, 0x0, 0x1a, ret | (1 << 5));
-	if (ret < 0)
-		goto error;
-
-	/* Wait for temperature to stabilize */
-	usleep_range(10000, 12000);
-
-	val = mv88e6xxx_phy_read(ds, 0x0, 0x1a);
-	if (val < 0) {
-		ret = val;
-		goto error;
-	}
-
-	/* Disable temperature sensor */
-	ret = mv88e6xxx_phy_write(ds, 0x0, 0x1a, ret & ~(1 << 5));
-	if (ret < 0)
-		goto error;
-
-	*temp = ((val & 0x1f) - 5) * 5;
-
-error:
-	mv88e6xxx_phy_write(ds, 0x0, 0x16, 0x0);
-	mutex_unlock(&ps->phy_mutex);
-	return ret;
-}
-#endif /* CONFIG_NET_DSA_HWMON */
-
 static int mv88e6123_61_65_setup(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
@@ -471,7 +423,7 @@ struct dsa_switch_driver mv88e6123_61_65_switch_driver = {
 	.get_ethtool_stats	= mv88e6123_61_65_get_ethtool_stats,
 	.get_sset_count		= mv88e6123_61_65_get_sset_count,
 #ifdef CONFIG_NET_DSA_HWMON
-	.get_temp		= mv88e6123_61_65_get_temp,
+	.get_temp		= mv88e6xxx_get_temp,
 #endif
 	.get_regs_len		= mv88e6xxx_get_regs_len,
 	.get_regs		= mv88e6xxx_get_regs,
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index da558d887dad..cd6807c6b4ed 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -539,6 +539,54 @@ void mv88e6xxx_get_regs(struct dsa_switch *ds, int port,
 	}
 }
 
+#ifdef CONFIG_NET_DSA_HWMON
+
+int  mv88e6xxx_get_temp(struct dsa_switch *ds, int *temp)
+{
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	int ret;
+	int val;
+
+	*temp = 0;
+
+	mutex_lock(&ps->phy_mutex);
+
+	ret = mv88e6xxx_phy_write(ds, 0x0, 0x16, 0x6);
+	if (ret < 0)
+		goto error;
+
+	/* Enable temperature sensor */
+	ret = mv88e6xxx_phy_read(ds, 0x0, 0x1a);
+	if (ret < 0)
+		goto error;
+
+	ret = mv88e6xxx_phy_write(ds, 0x0, 0x1a, ret | (1 << 5));
+	if (ret < 0)
+		goto error;
+
+	/* Wait for temperature to stabilize */
+	usleep_range(10000, 12000);
+
+	val = mv88e6xxx_phy_read(ds, 0x0, 0x1a);
+	if (val < 0) {
+		ret = val;
+		goto error;
+	}
+
+	/* Disable temperature sensor */
+	ret = mv88e6xxx_phy_write(ds, 0x0, 0x1a, ret & ~(1 << 5));
+	if (ret < 0)
+		goto error;
+
+	*temp = ((val & 0x1f) - 5) * 5;
+
+error:
+	mv88e6xxx_phy_write(ds, 0x0, 0x16, 0x0);
+	mutex_unlock(&ps->phy_mutex);
+	return ret;
+}
+#endif /* CONFIG_NET_DSA_HWMON */
+
 static int __init mv88e6xxx_init(void)
 {
 #if IS_ENABLED(CONFIG_NET_DSA_MV88E6131)
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index a0780b08bb4c..03e397efde36 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -81,6 +81,7 @@ void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds,
 int mv88e6xxx_get_regs_len(struct dsa_switch *ds, int port);
 void mv88e6xxx_get_regs(struct dsa_switch *ds, int port,
 			struct ethtool_regs *regs, void *_p);
+int  mv88e6xxx_get_temp(struct dsa_switch *ds, int *temp);
 
 extern struct dsa_switch_driver mv88e6131_switch_driver;
 extern struct dsa_switch_driver mv88e6123_61_65_switch_driver;
-- 
2.1.3

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox