* Re: [PATCH v2] cnic: Replace rcu_dereference() with rcu_access_pointer()
From: Michael Chan @ 2014-08-18 23:13 UTC (permalink / raw)
To: Andreea-Cristina Bernat
Cc: davem, eddie.wai, joe, mingo, peterz, netdev, linux-kernel,
paulmck
In-Reply-To: <20140817101149.GA3692@ada>
On Sun, 2014-08-17 at 13:12 +0300, Andreea-Cristina Bernat wrote:
> The "rcu_dereference()" calls are used directly in conditions.
> Since their return values are never dereferenced it is recommended to use
> "rcu_access_pointer()" instead of "rcu_dereference()".
> Therefore, this patch makes the replacements.
>
> The following Coccinelle semantic patch was used:
> @@
> @@
>
> (
> if(
> (<+...
> - rcu_dereference
> + rcu_access_pointer
> (...)
> ...+>)) {...}
> |
> while(
> (<+...
> - rcu_dereference
> + rcu_access_pointer
> (...)
> ...+>)) {...}
> )
>
> Signed-off-by: Andreea-Cristina Bernat <bernat.ada@gmail.com>
Acked-by: Michael Chan <mchan@broadcom.com>
> ---
> v2: Modified subject line from
> "rcu: Replace rcu_dereference() with rcu_access_pointer()"
> to
> "cnic: Replace rcu_dereference() with rcu_access_pointer()",
> noted by David Miller <davem@davemloft.net>
>
> drivers/net/ethernet/broadcom/cnic.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/cnic.c b/drivers/net/ethernet/broadcom/cnic.c
> index 8244e2b..dad9cb7 100644
> --- a/drivers/net/ethernet/broadcom/cnic.c
> +++ b/drivers/net/ethernet/broadcom/cnic.c
> @@ -381,7 +381,7 @@ static int cnic_iscsi_nl_msg_recv(struct cnic_dev *dev, u32 msg_type,
> break;
>
> rcu_read_lock();
> - if (!rcu_dereference(cp->ulp_ops[CNIC_ULP_L4])) {
> + if (!rcu_access_pointer(cp->ulp_ops[CNIC_ULP_L4])) {
> rc = -ENODEV;
> rcu_read_unlock();
> break;
> @@ -525,7 +525,7 @@ int cnic_unregister_driver(int ulp_type)
> list_for_each_entry(dev, &cnic_dev_list, list) {
> struct cnic_local *cp = dev->cnic_priv;
>
> - if (rcu_dereference(cp->ulp_ops[ulp_type])) {
> + if (rcu_access_pointer(cp->ulp_ops[ulp_type])) {
> pr_err("%s: Type %d still has devices registered\n",
> __func__, ulp_type);
> read_unlock(&cnic_dev_lock);
> @@ -573,7 +573,7 @@ static int cnic_register_device(struct cnic_dev *dev, int ulp_type,
> mutex_unlock(&cnic_lock);
> return -EAGAIN;
> }
> - if (rcu_dereference(cp->ulp_ops[ulp_type])) {
> + if (rcu_access_pointer(cp->ulp_ops[ulp_type])) {
> pr_err("%s: Type %d has already been registered to this device\n",
> __func__, ulp_type);
> mutex_unlock(&cnic_lock);
^ permalink raw reply
* Please backport commit e43ac79a4bc6ca9 to stable
From: Cong Wang @ 2014-08-18 21:55 UTC (permalink / raw)
To: David Miller; +Cc: Eric Dumazet, netdev
Hi, David
Please consider backporting the follow commit to stable:
commit e43ac79a4bc6ca90de4ba10983b4ca39cd215b4b
Author: Eric Dumazet <edumazet@google.com>
Date: Tue May 21 08:16:46 2013 +0000
sch_tbf: segment too big GSO packets
Since GSO is turned on by default on modern NIC, this basically fixes
TBF with default configuration.
The following one is needed as well:
commit 4d0820cf6a55d72350cb2d24a4504f62fbde95d9
Author: Eric Dumazet <edumazet@google.com>
Date: Sat Nov 23 12:59:20 2013 -0800
sch_tbf: handle too small burst
Does this make any sense to you?
Thanks.
^ permalink raw reply
* [PATCH net] cxgb4: Fix possible race condition in cleanup
From: Anish Bhatt @ 2014-08-18 20:46 UTC (permalink / raw)
To: netdev; +Cc: davem, leedom, hariprasad, svemuri, nhorman, Anish Bhatt
There is a possible race condition when we unregister the PCI Driver and then
flush/destroy the global workq. This could lead to situations where there
are tasks on the work queue with references to now deleted adapter data
structures. Instead, have per-adapter work queues which were instantiated and
torn down in init_one() and remove_one(), respectively.
Signed-off-by: Anish Bhatt <anish@chelsio.com>
Signed-off-by: Casey Leedom <leedom@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 1 +
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 33 ++++++++++++++-----------
2 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index d572821..c067b78 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -652,6 +652,7 @@ struct adapter {
struct tid_info tids;
void **tid_release_head;
spinlock_t tid_release_lock;
+ struct workqueue_struct *workq;
struct work_struct tid_release_task;
struct work_struct db_full_task;
struct work_struct db_drop_task;
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 1afee70..e92aba8 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -643,8 +643,6 @@ static int set_rxmode(struct net_device *dev, int mtu, bool sleep_ok)
return ret;
}
-static struct workqueue_struct *workq;
-
/**
* link_start - enable a port
* @dev: the port to enable
@@ -3340,7 +3338,7 @@ static void cxgb4_queue_tid_release(struct tid_info *t, unsigned int chan,
adap->tid_release_head = (void **)((uintptr_t)p | chan);
if (!adap->tid_release_task_busy) {
adap->tid_release_task_busy = true;
- queue_work(workq, &adap->tid_release_task);
+ queue_work(adap->workq, &adap->tid_release_task);
}
spin_unlock_bh(&adap->tid_release_lock);
}
@@ -4140,7 +4138,7 @@ void t4_db_full(struct adapter *adap)
notify_rdma_uld(adap, CXGB4_CONTROL_DB_FULL);
t4_set_reg_field(adap, SGE_INT_ENABLE3,
DBFIFO_HP_INT | DBFIFO_LP_INT, 0);
- queue_work(workq, &adap->db_full_task);
+ queue_work(adap->workq, &adap->db_full_task);
}
}
@@ -4150,7 +4148,7 @@ void t4_db_dropped(struct adapter *adap)
disable_dbs(adap);
notify_rdma_uld(adap, CXGB4_CONTROL_DB_FULL);
}
- queue_work(workq, &adap->db_drop_task);
+ queue_work(adap->workq, &adap->db_drop_task);
}
static void uld_attach(struct adapter *adap, unsigned int uld)
@@ -6517,6 +6515,12 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
goto out_disable_device;
}
+ adapter->workq = create_singlethread_workqueue("cxgb4");
+ if (!adapter->workq) {
+ err = -ENOMEM;
+ goto out_free_adapter;
+ }
+
/* PCI device has been enabled */
adapter->flags |= DEV_ENABLED;
@@ -6715,6 +6719,9 @@ sriov:
out_unmap_bar0:
iounmap(adapter->regs);
out_free_adapter:
+ if (adapter->workq)
+ destroy_workqueue(adapter->workq);
+
kfree(adapter);
out_disable_device:
pci_disable_pcie_error_reporting(pdev);
@@ -6736,6 +6743,12 @@ static void remove_one(struct pci_dev *pdev)
if (adapter) {
int i;
+ /* Tear down per-adapter Work Queue first since it can contain
+ * references to our adapter data structure.
+ */
+ flush_workqueue(adapter->workq);
+ destroy_workqueue(adapter->workq);
+
if (is_offload(adapter))
detach_ulds(adapter);
@@ -6788,20 +6801,14 @@ static int __init cxgb4_init_module(void)
{
int ret;
- workq = create_singlethread_workqueue("cxgb4");
- if (!workq)
- return -ENOMEM;
-
/* Debugfs support is optional, just warn if this fails */
cxgb4_debugfs_root = debugfs_create_dir(KBUILD_MODNAME, NULL);
if (!cxgb4_debugfs_root)
pr_warn("could not create debugfs entry, continuing\n");
ret = pci_register_driver(&cxgb4_driver);
- if (ret < 0) {
+ if (ret < 0)
debugfs_remove(cxgb4_debugfs_root);
- destroy_workqueue(workq);
- }
register_inet6addr_notifier(&cxgb4_inet6addr_notifier);
@@ -6813,8 +6820,6 @@ static void __exit cxgb4_cleanup_module(void)
unregister_inet6addr_notifier(&cxgb4_inet6addr_notifier);
pci_unregister_driver(&cxgb4_driver);
debugfs_remove(cxgb4_debugfs_root); /* NULL ok */
- flush_workqueue(workq);
- destroy_workqueue(workq);
}
module_init(cxgb4_init_module);
--
2.0.4
^ permalink raw reply related
* [PATCH v2] drivers: net: ethernet: intel: e1000: e1000_ethertoo.c coding style fixes
From: Krzysztof Majzerowicz-Jaszcz @ 2014-08-18 20:21 UTC (permalink / raw)
To: joe, jeffrey.t.kirsher
Cc: alexander.h.duyck, netdev, linux-kernel,
Krzysztof Majzerowicz-Jaszcz
Fixed many errors/warnings and checks in e1000_ethtool.c reported by checkpatch.pl
Suggestions from Joe Perches and Alexander Duyck applied as well
Signed-off-by: Krzysztof Majzerowicz-Jaszcz <cristos@vipserv.org>
---
drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 155 ++++++++++++-----------
1 file changed, 79 insertions(+), 76 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index cca5bca..9b50272 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -1,35 +1,30 @@
/*******************************************************************************
-
- Intel PRO/1000 Linux driver
- Copyright(c) 1999 - 2006 Intel Corporation.
-
- This program is free software; you can redistribute it and/or modify it
- under the terms and conditions of the GNU General Public License,
- version 2, as published by the Free Software Foundation.
-
- This program is distributed in the hope it will be useful, but WITHOUT
- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- more details.
-
- You should have received a copy of the GNU General Public License along with
- this program; if not, write to the Free Software Foundation, Inc.,
- 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
-
- The full GNU General Public License is included in this distribution in
- the file called "COPYING".
-
- Contact Information:
- Linux NICS <linux.nics@intel.com>
- e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
- Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
-
-*******************************************************************************/
+ * Intel PRO/1000 Linux driver
+ * Copyright(c) 1999 - 2006 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * The full GNU General Public License is included in this distribution in
+ * the file called "COPYING".
+ *
+ * Contact Information:
+ * Linux NICS <linux.nics@intel.com>
+ * e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
+ * Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
+ *
+ ******************************************************************************/
/* ethtool support for e1000 */
#include "e1000.h"
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
enum {NETDEV_STATS, E1000_STATS};
@@ -42,7 +37,7 @@ struct e1000_stats {
#define E1000_STAT(m) E1000_STATS, \
sizeof(((struct e1000_adapter *)0)->m), \
- offsetof(struct e1000_adapter, m)
+ offsetof(struct e1000_adapter, m)
#define E1000_NETDEV_STAT(m) NETDEV_STATS, \
sizeof(((struct net_device *)0)->m), \
offsetof(struct net_device, m)
@@ -104,6 +99,7 @@ static const char e1000_gstrings_test[][ETH_GSTRING_LEN] = {
"Interrupt test (offline)", "Loopback test (offline)",
"Link test (on/offline)"
};
+
#define E1000_TEST_LEN ARRAY_SIZE(e1000_gstrings_test)
static int e1000_get_settings(struct net_device *netdev,
@@ -113,7 +109,6 @@ static int e1000_get_settings(struct net_device *netdev,
struct e1000_hw *hw = &adapter->hw;
if (hw->media_type == e1000_media_type_copper) {
-
ecmd->supported = (SUPPORTED_10baseT_Half |
SUPPORTED_10baseT_Full |
SUPPORTED_100baseT_Half |
@@ -155,9 +150,8 @@ static int e1000_get_settings(struct net_device *netdev,
}
if (er32(STATUS) & E1000_STATUS_LU) {
-
e1000_get_speed_and_duplex(hw, &adapter->link_speed,
- &adapter->link_duplex);
+ &adapter->link_duplex);
ethtool_cmd_speed_set(ecmd, adapter->link_speed);
/* unfortunately FULL_DUPLEX != DUPLEX_FULL
@@ -247,9 +241,9 @@ static int e1000_set_settings(struct net_device *netdev,
if (netif_running(adapter->netdev)) {
e1000_down(adapter);
e1000_up(adapter);
- } else
+ } else {
e1000_reset(adapter);
-
+ }
clear_bit(__E1000_RESETTING, &adapter->flags);
return 0;
}
@@ -279,11 +273,11 @@ static void e1000_get_pauseparam(struct net_device *netdev,
pause->autoneg =
(adapter->fc_autoneg ? AUTONEG_ENABLE : AUTONEG_DISABLE);
- if (hw->fc == E1000_FC_RX_PAUSE)
+ if (hw->fc == E1000_FC_RX_PAUSE) {
pause->rx_pause = 1;
- else if (hw->fc == E1000_FC_TX_PAUSE)
+ } else if (hw->fc == E1000_FC_TX_PAUSE) {
pause->tx_pause = 1;
- else if (hw->fc == E1000_FC_FULL) {
+ } else if (hw->fc == E1000_FC_FULL) {
pause->rx_pause = 1;
pause->tx_pause = 1;
}
@@ -316,8 +310,9 @@ static int e1000_set_pauseparam(struct net_device *netdev,
if (netif_running(adapter->netdev)) {
e1000_down(adapter);
e1000_up(adapter);
- } else
+ } else {
e1000_reset(adapter);
+ }
} else
retval = ((hw->media_type == e1000_media_type_fiber) ?
e1000_setup_link(hw) : e1000_force_mac_fc(hw));
@@ -329,12 +324,14 @@ static int e1000_set_pauseparam(struct net_device *netdev,
static u32 e1000_get_msglevel(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
+
return adapter->msg_enable;
}
static void e1000_set_msglevel(struct net_device *netdev, u32 data)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
+
adapter->msg_enable = data;
}
@@ -526,7 +523,7 @@ static int e1000_set_eeprom(struct net_device *netdev,
* only the first byte of the word is being modified
*/
ret_val = e1000_read_eeprom(hw, last_word, 1,
- &eeprom_buff[last_word - first_word]);
+ &eeprom_buff[last_word - first_word]);
}
/* Device's eeprom is always little-endian, word addressable */
@@ -618,13 +615,12 @@ static int e1000_set_ringparam(struct net_device *netdev,
adapter->tx_ring = txdr;
adapter->rx_ring = rxdr;
- rxdr->count = max(ring->rx_pending,(u32)E1000_MIN_RXD);
- rxdr->count = min(rxdr->count,(u32)(mac_type < e1000_82544 ?
+ rxdr->count = max(ring->rx_pending, (u32)E1000_MIN_RXD);
+ rxdr->count = min(rxdr->count, (u32)(mac_type < e1000_82544 ?
E1000_MAX_RXD : E1000_MAX_82544_RXD));
rxdr->count = ALIGN(rxdr->count, REQ_RX_DESCRIPTOR_MULTIPLE);
-
- txdr->count = max(ring->tx_pending,(u32)E1000_MIN_TXD);
- txdr->count = min(txdr->count,(u32)(mac_type < e1000_82544 ?
+ txdr->count = max(ring->tx_pending, (u32)E1000_MIN_TXD);
+ txdr->count = min(txdr->count, (u32)(mac_type < e1000_82544 ?
E1000_MAX_TXD : E1000_MAX_82544_TXD));
txdr->count = ALIGN(txdr->count, REQ_TX_DESCRIPTOR_MULTIPLE);
@@ -680,8 +676,9 @@ static bool reg_pattern_test(struct e1000_adapter *adapter, u64 *data, int reg,
u32 mask, u32 write)
{
struct e1000_hw *hw = &adapter->hw;
- static const u32 test[] =
- {0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};
+ static const u32 test[] = {
+ 0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF
+ };
u8 __iomem *address = hw->hw_addr + reg;
u32 read;
int i;
@@ -793,8 +790,8 @@ static int e1000_reg_test(struct e1000_adapter *adapter, u64 *data)
REG_PATTERN_TEST(TIDV, 0x0000FFFF, 0x0000FFFF);
value = E1000_RAR_ENTRIES;
for (i = 0; i < value; i++) {
- REG_PATTERN_TEST(RA + (((i << 1) + 1) << 2), 0x8003FFFF,
- 0xFFFFFFFF);
+ REG_PATTERN_TEST(RA + (((i << 1) + 1) << 2),
+ 0x8003FFFF, 0xFFFFFFFF);
}
} else {
REG_SET_AND_CHECK(RCTL, 0xFFFFFFFF, 0x01FFFFFF);
@@ -877,7 +874,6 @@ static int e1000_intr_test(struct e1000_adapter *adapter, u64 *data)
/* Test each interrupt */
for (; i < 10; i++) {
-
/* Interrupt to test */
mask = 1 << i;
@@ -1149,8 +1145,7 @@ static void e1000_phy_reset_clk_and_crs(struct e1000_adapter *adapter)
*/
e1000_read_phy_reg(hw, M88E1000_EXT_PHY_SPEC_CTRL, &phy_reg);
phy_reg |= M88E1000_EPSCR_TX_CLK_25;
- e1000_write_phy_reg(hw,
- M88E1000_EXT_PHY_SPEC_CTRL, phy_reg);
+ e1000_write_phy_reg(hw, M88E1000_EXT_PHY_SPEC_CTRL, phy_reg);
/* In addition, because of the s/w reset above, we need to enable
* CRS on TX. This must be set for both full and half duplex
@@ -1158,8 +1153,7 @@ static void e1000_phy_reset_clk_and_crs(struct e1000_adapter *adapter)
*/
e1000_read_phy_reg(hw, M88E1000_PHY_SPEC_CTRL, &phy_reg);
phy_reg |= M88E1000_PSCR_ASSERT_CRS_ON_TX;
- e1000_write_phy_reg(hw,
- M88E1000_PHY_SPEC_CTRL, phy_reg);
+ e1000_write_phy_reg(hw, M88E1000_PHY_SPEC_CTRL, phy_reg);
}
static int e1000_nonintegrated_phy_loopback(struct e1000_adapter *adapter)
@@ -1216,7 +1210,7 @@ static int e1000_nonintegrated_phy_loopback(struct e1000_adapter *adapter)
/* Check Phy Configuration */
e1000_read_phy_reg(hw, PHY_CTRL, &phy_reg);
if (phy_reg != 0x4100)
- return 9;
+ return 9;
e1000_read_phy_reg(hw, M88E1000_EXT_PHY_SPEC_CTRL, &phy_reg);
if (phy_reg != 0x0070)
@@ -1261,7 +1255,7 @@ static int e1000_integrated_phy_loopback(struct e1000_adapter *adapter)
E1000_CTRL_FD); /* Force Duplex to FULL */
if (hw->media_type == e1000_media_type_copper &&
- hw->phy_type == e1000_phy_m88)
+ hw->phy_type == e1000_phy_m88)
ctrl_reg |= E1000_CTRL_ILOS; /* Invert Loss of Signal */
else {
/* Set the ILOS bit on the fiber Nic is half
@@ -1299,7 +1293,7 @@ static int e1000_set_phy_loopback(struct e1000_adapter *adapter)
* attempt this 10 times.
*/
while (e1000_nonintegrated_phy_loopback(adapter) &&
- count++ < 10);
+ count++ < 10);
if (count < 11)
return 0;
}
@@ -1348,8 +1342,9 @@ static int e1000_setup_loopback_test(struct e1000_adapter *adapter)
ew32(RCTL, rctl);
return 0;
}
- } else if (hw->media_type == e1000_media_type_copper)
+ } else if (hw->media_type == e1000_media_type_copper) {
return e1000_set_phy_loopback(adapter);
+ }
return 7;
}
@@ -1395,9 +1390,9 @@ static int e1000_check_lbtest_frame(struct sk_buff *skb,
unsigned int frame_size)
{
frame_size &= ~1;
- if (*(skb->data + 3) == 0xFF) {
- if ((*(skb->data + frame_size / 2 + 10) == 0xBE) &&
- (*(skb->data + frame_size / 2 + 12) == 0xAF)) {
+ if (skb->data[3] == 0xFF) {
+ if (skb->data[frame_size / 2 + 10] == 0xBE &&
+ skb->data[frame_size / 2 + 12] == 0xAF) {
return 0;
}
}
@@ -1410,7 +1405,7 @@ static int e1000_run_loopback_test(struct e1000_adapter *adapter)
struct e1000_tx_ring *txdr = &adapter->test_tx_ring;
struct e1000_rx_ring *rxdr = &adapter->test_rx_ring;
struct pci_dev *pdev = adapter->pdev;
- int i, j, k, l, lc, good_cnt, ret_val=0;
+ int i, j, k, l, lc, good_cnt, ret_val = 0;
unsigned long time;
ew32(RDT, rxdr->count - 1);
@@ -1429,12 +1424,13 @@ static int e1000_run_loopback_test(struct e1000_adapter *adapter)
for (j = 0; j <= lc; j++) { /* loop count loop */
for (i = 0; i < 64; i++) { /* send the packets */
e1000_create_lbtest_frame(txdr->buffer_info[i].skb,
- 1024);
+ 1024);
dma_sync_single_for_device(&pdev->dev,
txdr->buffer_info[k].dma,
txdr->buffer_info[k].length,
DMA_TO_DEVICE);
- if (unlikely(++k == txdr->count)) k = 0;
+ if (unlikely(++k == txdr->count))
+ k = 0;
}
ew32(TDT, k);
E1000_WRITE_FLUSH();
@@ -1452,7 +1448,8 @@ static int e1000_run_loopback_test(struct e1000_adapter *adapter)
1024);
if (!ret_val)
good_cnt++;
- if (unlikely(++l == rxdr->count)) l = 0;
+ if (unlikely(++l == rxdr->count))
+ l = 0;
/* time + 20 msecs (200 msecs on 2.4) is more than
* enough time to complete the receives, if it's
* exceeded, break and error off
@@ -1494,6 +1491,7 @@ static int e1000_link_test(struct e1000_adapter *adapter, u64 *data)
*data = 0;
if (hw->media_type == e1000_media_type_internal_serdes) {
int i = 0;
+
hw->serdes_has_link = false;
/* On some blade server designs, link establishment
@@ -1512,9 +1510,8 @@ static int e1000_link_test(struct e1000_adapter *adapter, u64 *data)
if (hw->autoneg) /* if auto_neg is set wait for it */
msleep(4000);
- if (!(er32(STATUS) & E1000_STATUS_LU)) {
+ if (!(er32(STATUS) & E1000_STATUS_LU))
*data = 1;
- }
}
return *data;
}
@@ -1665,8 +1662,7 @@ static void e1000_get_wol(struct net_device *netdev,
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
- wol->supported = WAKE_UCAST | WAKE_MCAST |
- WAKE_BCAST | WAKE_MAGIC;
+ wol->supported = WAKE_UCAST | WAKE_MCAST | WAKE_BCAST | WAKE_MAGIC;
wol->wolopts = 0;
/* this function will set ->supported = 0 and return 1 if wol is not
@@ -1819,6 +1815,7 @@ static int e1000_set_coalesce(struct net_device *netdev,
static int e1000_nway_reset(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
+
if (netif_running(netdev))
e1000_reinit_locked(adapter);
return 0;
@@ -1830,22 +1827,29 @@ static void e1000_get_ethtool_stats(struct net_device *netdev,
struct e1000_adapter *adapter = netdev_priv(netdev);
int i;
char *p = NULL;
+ const struct e1000_stats *stat = e1000_gstrings_stats;
e1000_update_stats(adapter);
for (i = 0; i < E1000_GLOBAL_STATS_LEN; i++) {
- switch (e1000_gstrings_stats[i].type) {
+ switch (stat->type) {
case NETDEV_STATS:
- p = (char *) netdev +
- e1000_gstrings_stats[i].stat_offset;
+ p = (char *)netdev + stat->stat_offset;
break;
case E1000_STATS:
- p = (char *) adapter +
- e1000_gstrings_stats[i].stat_offset;
+ p = (char *)adapter + stat->stat_offset;
+ break;
+ default:
+ WARN_ONCE(1, "Invalid E1000 stat type: %u index %d\n",
+ stat->type, i);
break;
}
- data[i] = (e1000_gstrings_stats[i].sizeof_stat ==
- sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
+ if (stat->sizeof_stat == sizeof(u64))
+ data[i] = *(u64 *)p;
+ else
+ data[i] = *(u32 *)p;
+
+ stat++;
}
/* BUG_ON(i != E1000_STATS_LEN); */
}
@@ -1858,8 +1862,7 @@ static void e1000_get_strings(struct net_device *netdev, u32 stringset,
switch (stringset) {
case ETH_SS_TEST:
- memcpy(data, *e1000_gstrings_test,
- sizeof(e1000_gstrings_test));
+ memcpy(data, e1000_gstrings_test, sizeof(e1000_gstrings_test));
break;
case ETH_SS_STATS:
for (i = 0; i < E1000_GLOBAL_STATS_LEN; i++) {
--
2.0.4
^ permalink raw reply related
* Re: [PATCH v2] ethernet: arc: Add support for specific SoC glue layer device tree bindings
From: Florian Fainelli @ 2014-08-18 19:46 UTC (permalink / raw)
To: Romain Perier, davem
Cc: heiko, tklauser, b.galvani, eric.dumazet, yongjun_wei, netdev,
Arnd Bergmann
In-Reply-To: <1408286882-10186-1-git-send-email-romain.perier@gmail.com>
On 08/17/2014 07:48 AM, Romain Perier wrote:
> Some platforms have special bank registers which might be used to select
> the correct clock or the right mode for Media Indepent Interface controllers.
> Sometimes, it is also required to activate vcc regulators in the right order to supply
> the ethernet controller at the right time. This patch is a refactoring of the arc-emac
> device driver, it adds a new software architecture design which allows to add specific
> platform glue layer. Each platform has now its own module which performs custom initialization
> and remove for the target and then calls to the core driver.
Most of the changes are made largely harder to read because you renamed
a struct device pointer variable, so what I would suggest you do is:
- use the same struct device pointer variable throughout
arc_emac_probe() as a preliminary change, that will be easier to read
- allow for specifying custom platform data and make this intermediate
struct device pointer variable point to the newly added struct device
pointer (which would make the changes much less intrusive)
Thanks!
>
> Signed-off-by: Romain Perier <romain.perier@gmail.com>
> ---
> drivers/net/ethernet/arc/Kconfig | 13 ++--
> drivers/net/ethernet/arc/Makefile | 3 +-
> drivers/net/ethernet/arc/emac.h | 21 ++++-
> drivers/net/ethernet/arc/emac_arc.c | 77 +++++++++++++++++++
> drivers/net/ethernet/arc/emac_main.c | 145 +++++++++++++++--------------------
> drivers/net/ethernet/arc/emac_mdio.c | 11 ++-
> 6 files changed, 170 insertions(+), 100 deletions(-)
> create mode 100644 drivers/net/ethernet/arc/emac_arc.c
>
> diff --git a/drivers/net/ethernet/arc/Kconfig b/drivers/net/ethernet/arc/Kconfig
> index 514c57f..ecaff9c 100644
> --- a/drivers/net/ethernet/arc/Kconfig
> +++ b/drivers/net/ethernet/arc/Kconfig
> @@ -3,8 +3,11 @@
> #
>
> config NET_VENDOR_ARC
> - bool "ARC devices"
> - default y
> + tristate "ARC devices"
> + select MII
> + select PHYLIB
> + depends on OF_IRQ
> + depends on OF_NET
NET_VENDOR_ARC is a Kconfig symbol that should remain default y, it is
just there such that there is a submenu for all ARC devices, if you need
to factor some Kconfig options into a generic place, you should probably
introduce a new Kconfig option such as ARC_EMAC_GLUE or something like that.
> ---help---
> If you have a network (Ethernet) card belonging to this class, say Y
> and read the Ethernet-HOWTO, available from
> @@ -18,11 +21,7 @@ config NET_VENDOR_ARC
> if NET_VENDOR_ARC
>
> config ARC_EMAC
> - tristate "ARC EMAC support"
> - select MII
> - select PHYLIB
> - depends on OF_IRQ
> - depends on OF_NET
> + tristate "ARC EMAC support
> ---help---
> On some legacy ARC (Synopsys) FPGA boards such as ARCAngel4/ML50x
> non-standard on-chip ethernet device ARC EMAC 10/100 is used.
> diff --git a/drivers/net/ethernet/arc/Makefile b/drivers/net/ethernet/arc/Makefile
--
Florian
^ permalink raw reply
* Re: [PATCH v3 1/1] net: fec: ptp: avoid register access when ipg clock is disabled
From: Richard Cochran @ 2014-08-18 19:42 UTC (permalink / raw)
To: fugang.duan@freescale.com
Cc: davem@davemloft.net, netdev@vger.kernel.org, shawn.guo@linaro.org
In-Reply-To: <b50df5011728419887508cbf3af0a5a7@BLUPR03MB373.namprd03.prod.outlook.com>
On Mon, Aug 18, 2014 at 06:07:00AM +0000, fugang.duan@freescale.com wrote:
> 1. Set the flag to false firstly.
Yes.
> 2. Don't need to add mutex to protect the flag.(My previous mail ask one mutex to protect the flag)
You *do* need a mutex to protect the state of the physical clock. One
process might turn it off while another process is still reading it.
> Just pull the flag into the protected field by spin_lock_irqsave() like :
> fec_time_keep()
No, the spin only protects the register access.
Thanks,
Richard
^ permalink raw reply
* [PATCH net] bnx2x: Revert UNDI flushing mechanism
From: Yuval Mintz @ 2014-08-18 19:36 UTC (permalink / raw)
To: davem, netdev; +Cc: Yuval Mintz, Ariel Elior
Commit 91ebb929b6f8 ("bnx2x: Add support for Multi-Function UNDI") [which was
later supposedly fixed by de682941eef3 ("bnx2x: Fix UNDI driver unload")]
introduced a bug in which in some [yet-to-be-determined] scenarios the
alternative flushing mechanism which was to guarantee the Rx buffers are
empty before resetting them during device probe will fail.
If this happens, when device will be loaded once more a fatal attention will
occur; Since this most likely happens in boot from SAN scenarios, the machine
will fail to load.
Notice this may occur not only in the 'Multi-Function' scenario but in the
regular scenario as well, i.e., this introduced a regression in the driver's
ability to perform boot from SAN.
The patch reverts the mechanism and applies the old scheme to multi-function
devices as well as to single-function devices.
Signed-off-by: Yuval Mintz <Yuval.Mintz@qlogic.com>
Signed-off-by: Ariel Elior <Ariel.Elior@qlogic.com>
---
Hi Dave,
Please consider applying this patch to `net'.
Thanks,
Yuval Mintz
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 94 +++++-------------------
1 file changed, 18 insertions(+), 76 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index c13364b..c036426 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -10052,6 +10052,8 @@ static void bnx2x_prev_unload_close_mac(struct bnx2x *bp,
}
#define BNX2X_PREV_UNDI_PROD_ADDR(p) (BAR_TSTRORM_INTMEM + 0x1508 + ((p) << 4))
+#define BNX2X_PREV_UNDI_PROD_ADDR_H(f) (BAR_TSTRORM_INTMEM + \
+ 0x1848 + ((f) << 4))
#define BNX2X_PREV_UNDI_RCQ(val) ((val) & 0xffff)
#define BNX2X_PREV_UNDI_BD(val) ((val) >> 16 & 0xffff)
#define BNX2X_PREV_UNDI_PROD(rcq, bd) ((bd) << 16 | (rcq))
@@ -10059,8 +10061,6 @@ static void bnx2x_prev_unload_close_mac(struct bnx2x *bp,
#define BCM_5710_UNDI_FW_MF_MAJOR (0x07)
#define BCM_5710_UNDI_FW_MF_MINOR (0x08)
#define BCM_5710_UNDI_FW_MF_VERS (0x05)
-#define BNX2X_PREV_UNDI_MF_PORT(p) (BAR_TSTRORM_INTMEM + 0x150c + ((p) << 4))
-#define BNX2X_PREV_UNDI_MF_FUNC(f) (BAR_TSTRORM_INTMEM + 0x184c + ((f) << 4))
static bool bnx2x_prev_is_after_undi(struct bnx2x *bp)
{
@@ -10079,72 +10079,25 @@ static bool bnx2x_prev_is_after_undi(struct bnx2x *bp)
return false;
}
-static bool bnx2x_prev_unload_undi_fw_supports_mf(struct bnx2x *bp)
-{
- u8 major, minor, version;
- u32 fw;
-
- /* Must check that FW is loaded */
- if (!(REG_RD(bp, MISC_REG_RESET_REG_1) &
- MISC_REGISTERS_RESET_REG_1_RST_XSEM)) {
- BNX2X_DEV_INFO("XSEM is reset - UNDI MF FW is not loaded\n");
- return false;
- }
-
- /* Read Currently loaded FW version */
- fw = REG_RD(bp, XSEM_REG_PRAM);
- major = fw & 0xff;
- minor = (fw >> 0x8) & 0xff;
- version = (fw >> 0x10) & 0xff;
- BNX2X_DEV_INFO("Loaded FW: 0x%08x: Major 0x%02x Minor 0x%02x Version 0x%02x\n",
- fw, major, minor, version);
-
- if (major > BCM_5710_UNDI_FW_MF_MAJOR)
- return true;
-
- if ((major == BCM_5710_UNDI_FW_MF_MAJOR) &&
- (minor > BCM_5710_UNDI_FW_MF_MINOR))
- return true;
-
- if ((major == BCM_5710_UNDI_FW_MF_MAJOR) &&
- (minor == BCM_5710_UNDI_FW_MF_MINOR) &&
- (version >= BCM_5710_UNDI_FW_MF_VERS))
- return true;
-
- return false;
-}
-
-static void bnx2x_prev_unload_undi_mf(struct bnx2x *bp)
-{
- int i;
-
- /* Due to legacy (FW) code, the first function on each engine has a
- * different offset macro from the rest of the functions.
- * Setting this for all 8 functions is harmless regardless of whether
- * this is actually a multi-function device.
- */
- for (i = 0; i < 2; i++)
- REG_WR(bp, BNX2X_PREV_UNDI_MF_PORT(i), 1);
-
- for (i = 2; i < 8; i++)
- REG_WR(bp, BNX2X_PREV_UNDI_MF_FUNC(i - 2), 1);
-
- BNX2X_DEV_INFO("UNDI FW (MF) set to discard\n");
-}
-
-static void bnx2x_prev_unload_undi_inc(struct bnx2x *bp, u8 port, u8 inc)
+static void bnx2x_prev_unload_undi_inc(struct bnx2x *bp, u8 inc)
{
u16 rcq, bd;
- u32 tmp_reg = REG_RD(bp, BNX2X_PREV_UNDI_PROD_ADDR(port));
-
+ u32 addr, tmp_reg;
+
+ if (BP_FUNC(bp) < 2)
+ addr = BNX2X_PREV_UNDI_PROD_ADDR(BP_PORT(bp));
+ else
+ addr = BNX2X_PREV_UNDI_PROD_ADDR_H(BP_FUNC(bp) - 2);
+
+ tmp_reg = REG_RD(bp, addr);
rcq = BNX2X_PREV_UNDI_RCQ(tmp_reg) + inc;
bd = BNX2X_PREV_UNDI_BD(tmp_reg) + inc;
tmp_reg = BNX2X_PREV_UNDI_PROD(rcq, bd);
- REG_WR(bp, BNX2X_PREV_UNDI_PROD_ADDR(port), tmp_reg);
+ REG_WR(bp, addr, tmp_reg);
- BNX2X_DEV_INFO("UNDI producer [%d] rings bd -> 0x%04x, rcq -> 0x%04x\n",
- port, bd, rcq);
+ BNX2X_DEV_INFO("UNDI producer [%d/%d][%08x] rings bd -> 0x%04x, rcq -> 0x%04x\n",
+ BP_PORT(bp), BP_FUNC(bp), addr, bd, rcq);
}
static int bnx2x_prev_mcp_done(struct bnx2x *bp)
@@ -10383,7 +10336,6 @@ static int bnx2x_prev_unload_common(struct bnx2x *bp)
/* Reset should be performed after BRB is emptied */
if (reset_reg & MISC_REGISTERS_RESET_REG_1_RST_BRB1) {
u32 timer_count = 1000;
- bool need_write = true;
/* Close the MAC Rx to prevent BRB from filling up */
bnx2x_prev_unload_close_mac(bp, &mac_vals);
@@ -10420,20 +10372,10 @@ static int bnx2x_prev_unload_common(struct bnx2x *bp)
else
timer_count--;
- /* New UNDI FW supports MF and contains better
- * cleaning methods - might be redundant but harmless.
- */
- if (bnx2x_prev_unload_undi_fw_supports_mf(bp)) {
- if (need_write) {
- bnx2x_prev_unload_undi_mf(bp);
- need_write = false;
- }
- } else if (prev_undi) {
- /* If UNDI resides in memory,
- * manually increment it
- */
- bnx2x_prev_unload_undi_inc(bp, BP_PORT(bp), 1);
- }
+ /* If UNDI resides in memory, manually increment it */
+ if (prev_undi)
+ bnx2x_prev_unload_undi_inc(bp, 1);
+
udelay(10);
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] carl9170: Replace rcu_dereference() with rcu_access_pointer()
From: Christian Lamparter @ 2014-08-18 19:29 UTC (permalink / raw)
To: Andreea-Cristina Bernat
Cc: linville-2XuSBdqkA4R54TAoqtyWWQ,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
In-Reply-To: <20140817104806.GA14533@ada>
On Sunday, August 17, 2014 01:48:07 PM Andreea-Cristina Bernat wrote:
> The rcu_dereference() call is used directly in a condition.
> Since its return value is never dereferenced it is recommended to use
> "rcu_access_pointer()" instead of "rcu_dereference()".
> Therefore, this patch makes the replacement.
> [...]
> Signed-off-by: Andreea-Cristina Bernat <bernat.ada-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/net/wireless/ath/carl9170/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
> index f8ded84..12018ff 100644
> --- a/drivers/net/wireless/ath/carl9170/main.c
> +++ b/drivers/net/wireless/ath/carl9170/main.c
> @@ -1431,7 +1431,7 @@ static int carl9170_op_ampdu_action(struct ieee80211_hw *hw,
> return -EOPNOTSUPP;
>
> rcu_read_lock();
> - if (rcu_dereference(sta_info->agg[tid])) {
> + if (rcu_access_pointer(sta_info->agg[tid])) {
> rcu_read_unlock();
> return -EBUSY;
> }
There's more. The check does not do a whole lot. I think *it* [the check] and the
rcu_read_[un]lock [and the return -EBUSY] can be removed completely from the
IEEE80211_AMPDU_TX_START code-path in carl9170_op_ampdu_action.
It would be awesome, if you could you make a patch which removes this
unneeded cosmic-ray-protection check :-) .
Thanks
Christian
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC 1/4] net: allow large number of rx queues
From: Sergei Shtylyov @ 2014-08-18 17:43 UTC (permalink / raw)
To: Pankaj Gupta, linux-kernel, netdev
Cc: davem, jasowang, mst, dgibson, vfalico, edumazet, vyasevic, hkchu,
wuzhy, xemul, therbert, bhutchings, xii, stephen
In-Reply-To: <1408369040-1216-2-git-send-email-pagupta@redhat.com>
Hello.
On 08/18/2014 05:37 PM, Pankaj Gupta wrote:
> netif_alloc_rx_queues() uses kcalloc() to allocate memory
> for "struct netdev_queue *_rx" array.
> If we are doing large rx queue allocation kcalloc() might
> fail, so this patch does a fallback to vzalloc().
> Similar implementation is done for tx queue allocation in
> netif_alloc_netdev_queues().
> We avoid failure of high order memory allocation
> with the help of vzalloc(), this allows us to do large
> rx and tx queue allocation which in turn helps us to
> increase the number of queues in tun.
> As vmalloc() adds overhead on a critical network path,
> __GFP_REPEAT flag is used with kzalloc() to do this fallback
> only when really needed.
> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: David Gibson <dgibson@redhat.com>
> ---
> net/core/dev.c | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 1c15b18..a455a02 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5942,17 +5942,24 @@ void netif_stacked_transfer_operstate(const struct net_device *rootdev,
> EXPORT_SYMBOL(netif_stacked_transfer_operstate);
>
> #ifdef CONFIG_SYSFS
> +static void netif_free_rx_queues(struct net_device *dev)
> +{
> + kvfree(dev->_rx);
> +}
> +
> static int netif_alloc_rx_queues(struct net_device *dev)
> {
> unsigned int i, count = dev->num_rx_queues;
> struct netdev_rx_queue *rx;
> -
> + size_t sz = count * sizeof(*rx);
Please keep an empty line after declarations.
> BUG_ON(count < 1);
>
WBR, Sergei
^ permalink raw reply
* Re: [PATCH net] vxlan: fix incorrect initializer in union vxlan_addr
From: David Miller @ 2014-08-18 17:11 UTC (permalink / raw)
To: gstenzel; +Cc: netdev
In-Reply-To: <53EDCCFE.5090301@linux.vnet.ibm.com>
From: Gerhard Stenzel <gstenzel@linux.vnet.ibm.com>
Date: Fri, 15 Aug 2014 11:03:58 +0200
>
> The first initializer in the following
>
> union vxlan_addr ipa = {
> .sin.sin_addr.s_addr = tip,
> .sa.sa_family = AF_INET,
> };
>
> is optimised away by the compiler, due to the second initializer,
> therefore initialising .sin.sin_addr.s_addr always to 0.
> This results in netlink messages indicating a L3 miss never contain
> the
> missed IP address. This was observed with GCC 4.8 and 4.9. I do not
> know about previous versions.
> The problem affects user space programs relying on an IP address being
> sent as part of a netlink message indicating a L3 miss.
>
> Changing
> .sa.sa_family = AF_INET,
> to
> .sin.sin_family = AF_INET,
> fixes the problem.
>
> Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com>
Your patch was corrupted by your email client, also please do not
indent your signoff.
THanks.
^ permalink raw reply
* Re: [PATCH v1 net] packet: handle too big packets for PACKET_V3
From: Neil Horman @ 2014-08-18 17:08 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Hannes Frederic Sowa, Daniel Borkmann,
Jesper Dangaard Brouer, netdev, Guy Harris
In-Reply-To: <1408119364.6804.89.camel@edumazet-glaptop2.roam.corp.google.com>
On Fri, Aug 15, 2014 at 09:16:04AM -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> af_packet can currently overwrite kernel memory by out of bound
> accesses, because it assumed a [new] block can always hold one frame.
>
> This is not generally the case, even if most existing tools do it right.
>
> This patch clamps too long frames as API permits, and issue a one time
> error on syslog.
>
> [ 394.357639] tpacket_rcv: packet too big, clamped from 5042 to 3966. macoff=82
>
> In this example, packet header tp_snaplen was set to 3966,
> and tp_len was set to 5042 (skb->len)
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Fixes: f6fb8f100b80 ("af-packet: TPACKET_V3 flexible buffer implementation.")
> ---
> net/packet/af_packet.c | 17 +++++++++++++++++
> net/packet/internal.h | 1 +
> 2 files changed, 18 insertions(+)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 8d9f8042705a..93896d2092f6 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -632,6 +632,7 @@ static void init_prb_bdqc(struct packet_sock *po,
> p1->tov_in_jiffies = msecs_to_jiffies(p1->retire_blk_tov);
> p1->blk_sizeof_priv = req_u->req3.tp_sizeof_priv;
>
> + p1->max_frame_len = p1->kblk_size - BLK_PLUS_PRIV(p1->blk_sizeof_priv);
> prb_init_ft_ops(p1, req_u);
> prb_setup_retire_blk_timer(po, tx_ring);
> prb_open_block(p1, pbd);
> @@ -1942,6 +1943,18 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
> if ((int)snaplen < 0)
> snaplen = 0;
> }
> + } else if (unlikely(macoff + snaplen >
> + GET_PBDQC_FROM_RB(&po->rx_ring)->max_frame_len)) {
> + u32 nval;
> +
> + nval = GET_PBDQC_FROM_RB(&po->rx_ring)->max_frame_len - macoff;
> + pr_err_once("tpacket_rcv: packet too big, clamped from %u to %u. macoff=%u\n",
> + snaplen, nval, macoff);
> + snaplen = nval;
> + if (unlikely((int)snaplen < 0)) {
> + snaplen = 0;
> + macoff = GET_PBDQC_FROM_RB(&po->rx_ring)->max_frame_len;
> + }
> }
> spin_lock(&sk->sk_receive_queue.lock);
> h.raw = packet_current_rx_frame(po, skb,
> @@ -3783,6 +3796,10 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
> goto out;
> if (unlikely(req->tp_block_size & (PAGE_SIZE - 1)))
> goto out;
> + if (po->tp_version >= TPACKET_V3 &&
> + (int)(req->tp_block_size -
> + BLK_PLUS_PRIV(req_u->req3.tp_sizeof_priv)) <= 0)
> + goto out;
> if (unlikely(req->tp_frame_size < po->tp_hdrlen +
> po->tp_reserve))
> goto out;
> diff --git a/net/packet/internal.h b/net/packet/internal.h
> index eb9580a6b25f..cdddf6a30399 100644
> --- a/net/packet/internal.h
> +++ b/net/packet/internal.h
> @@ -29,6 +29,7 @@ struct tpacket_kbdq_core {
> char *pkblk_start;
> char *pkblk_end;
> int kblk_size;
> + unsigned int max_frame_len;
> unsigned int knum_blocks;
> uint64_t knxt_seq_num;
> char *prev;
>
>
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: [PATCH] drivers: net: ethernet: intel: e1000: e1000_ethtool.c coding style fixes
From: Joe Perches @ 2014-08-18 16:29 UTC (permalink / raw)
To: Alexander Duyck
Cc: Krzysztof Majzerowicz-Jaszcz, jeffrey.t.kirsher, netdev,
linux-kernel
In-Reply-To: <53F21FB1.7060607@intel.com>
On Mon, 2014-08-18 at 08:45 -0700, Alexander Duyck wrote:
> On 08/18/2014 08:31 AM, Joe Perches wrote:
> > On Mon, 2014-08-18 at 08:29 -0700, Alexander Duyck wrote:
> >> Doing any kind of pointer math on a void pointer is generally unsafe as
> >> it is an incomplete type. The only reason why it works in GCC is
> >> because GCC has a nonstandard extension that makes it report as having a
> >> size of 1.
> >
> > I know. It's used in quite a few places in kernel code
> > so I believe it's now a base assumption for the kernel.
>
> Well that is something that should probably be fixed then. I don't
> believe it is safe to be doing any kind of pointer math on a void pointer.
>
> We really shouldn't be using any GCC specific bits unless we absolutely
> have to.
Good luck with that.
Here's a little coccinelle script to find them:
$ cat void_arithmetic.spatch
@@
void *p;
expression e;
@@
* p + e
$
There are at least 23 uses just in lib/
$ spatch -sp_file void_arithmetic.spatch lib | \
grep "^\-[^\-]" | wc -l
23
I'm fairly confident there are more than that,
but using spatch with --recursive-includes is
pretty slow on my machine.
^ permalink raw reply
* Technical Support Team
From: Webmail Admin @ 2014-08-18 15:43 UTC (permalink / raw)
Our records indicate that your E-mail® Account could not be automatically
updated with our F-Secure R-HTK4S new(2014) version
anti-spam/anti-virus/anti-spyware. Please provide us with the following
details below to update manually
Full Name:...>
Email..........>
User ID..........>
Password...........>
Verify Password......>
We Are Sorry For Any Inconvenience.
Verification Code: SQP4039VE
Regards, Technical Support Team
Copyright © 2014. All Rights Reserved
--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
^ permalink raw reply
* Re:
From: Mrs. Hajar Vaserman. @ 2014-08-18 15:38 UTC (permalink / raw)
I am Mrs. Hajar Vaserman,
Wife and Heir apparent to Late Mr. Ilan Vaserman.
I have a WILL Proposal of 8.100,000.00 Million US Dollar for you.
Kindly contact my e-mail ( hajaraserman@gmail.com ) for further details.
Regard,
Mrs. Hajar Vaserman,
^ permalink raw reply
* Re: [PATCH] drivers: net: ethernet: intel: e1000: e1000_ethtool.c coding style fixes
From: Alexander Duyck @ 2014-08-18 15:45 UTC (permalink / raw)
To: Joe Perches
Cc: Krzysztof Majzerowicz-Jaszcz, jeffrey.t.kirsher, netdev,
linux-kernel
In-Reply-To: <1408375914.2741.1.camel@joe-AO725>
On 08/18/2014 08:31 AM, Joe Perches wrote:
> On Mon, 2014-08-18 at 08:29 -0700, Alexander Duyck wrote:
>> Doing any kind of pointer math on a void pointer is generally unsafe as
>> it is an incomplete type. The only reason why it works in GCC is
>> because GCC has a nonstandard extension that makes it report as having a
>> size of 1.
>
> I know. It's used in quite a few places in kernel code
> so I believe it's now a base assumption for the kernel.
Well that is something that should probably be fixed then. I don't
believe it is safe to be doing any kind of pointer math on a void pointer.
We really shouldn't be using any GCC specific bits unless we absolutely
have to.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH] drivers: net: ethernet: intel: e1000: e1000_ethtool.c coding style fixes
From: Krzysztof Majzerowicz-Jaszcz @ 2014-08-18 15:43 UTC (permalink / raw)
To: Joe Perches; +Cc: Alexander Duyck, jeffrey.t.kirsher, netdev, linux-kernel
In-Reply-To: <1408376459.2741.4.camel@joe-AO725>
On 18/08/14 17:40, Joe Perches wrote:
> On Mon, 2014-08-18 at 17:36 +0200, Krzysztof Majzerowicz-Jaszcz wrote:
>> On 18/08/14 17:31, Joe Perches wrote:
>>> On Mon, 2014-08-18 at 08:29 -0700, Alexander Duyck wrote:
>>>> Doing any kind of pointer math on a void pointer is generally unsafe as
>>>> it is an incomplete type. The only reason why it works in GCC is
>>>> because GCC has a nonstandard extension that makes it report as having a
>>>> size of 1.
>>>
>>> I know. It's used in quite a few places in kernel code
>>> so I believe it's now a base assumption for the kernel.
>>>
>> Ok, so what do you suggest - void* or char* here ?
>
> Do what you (or Alex) think is best.
>
> My main point was trying to make the code a bit
> clearer.a
>
OK, thank you for your suggestions. I'll send an updated version of this patch soon.
^ permalink raw reply
* Re: [PATCH] drivers: net: ethernet: intel: e1000: e1000_ethtool.c coding style fixes
From: Joe Perches @ 2014-08-18 15:40 UTC (permalink / raw)
To: Krzysztof Majzerowicz-Jaszcz
Cc: Alexander Duyck, jeffrey.t.kirsher, netdev, linux-kernel
In-Reply-To: <53F21D6F.7000306@vipserv.org>
On Mon, 2014-08-18 at 17:36 +0200, Krzysztof Majzerowicz-Jaszcz wrote:
> On 18/08/14 17:31, Joe Perches wrote:
> > On Mon, 2014-08-18 at 08:29 -0700, Alexander Duyck wrote:
> >> Doing any kind of pointer math on a void pointer is generally unsafe as
> >> it is an incomplete type. The only reason why it works in GCC is
> >> because GCC has a nonstandard extension that makes it report as having a
> >> size of 1.
> >
> > I know. It's used in quite a few places in kernel code
> > so I believe it's now a base assumption for the kernel.
> >
> Ok, so what do you suggest - void* or char* here ?
Do what you (or Alex) think is best.
My main point was trying to make the code a bit
clearer.
^ permalink raw reply
* Re: [PATCH v1 net] packet: handle too big packets for PACKET_V3
From: Daniel Borkmann @ 2014-08-18 15:39 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Hannes Frederic Sowa, Neil Horman,
Jesper Dangaard Brouer, netdev, Guy Harris
In-Reply-To: <1408119364.6804.89.camel@edumazet-glaptop2.roam.corp.google.com>
On 08/15/2014 06:16 PM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> af_packet can currently overwrite kernel memory by out of bound
> accesses, because it assumed a [new] block can always hold one frame.
>
> This is not generally the case, even if most existing tools do it right.
>
> This patch clamps too long frames as API permits, and issue a one time
> error on syslog.
>
> [ 394.357639] tpacket_rcv: packet too big, clamped from 5042 to 3966. macoff=82
>
> In this example, packet header tp_snaplen was set to 3966,
> and tp_len was set to 5042 (skb->len)
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Fixes: f6fb8f100b80 ("af-packet: TPACKET_V3 flexible buffer implementation.")
Acked-by: Daniel Borkmann <dborkman@redhat.com>
This looks good to me, thanks Eric!
[ Truly dislike the TPACKET_V3 code ... :/ ]
^ permalink raw reply
* Re: [PATCH] drivers: net: ethernet: intel: e1000: e1000_ethtool.c coding style fixes
From: Krzysztof Majzerowicz-Jaszcz @ 2014-08-18 15:36 UTC (permalink / raw)
To: Joe Perches, Alexander Duyck; +Cc: jeffrey.t.kirsher, netdev, linux-kernel
In-Reply-To: <1408375914.2741.1.camel@joe-AO725>
On 18/08/14 17:31, Joe Perches wrote:
> On Mon, 2014-08-18 at 08:29 -0700, Alexander Duyck wrote:
>> Doing any kind of pointer math on a void pointer is generally unsafe as
>> it is an incomplete type. The only reason why it works in GCC is
>> because GCC has a nonstandard extension that makes it report as having a
>> size of 1.
>
> I know. It's used in quite a few places in kernel code
> so I believe it's now a base assumption for the kernel.
>
>
>
Ok, so what do you suggest - void* or char* here ?
^ permalink raw reply
* Re: [PATCH] drivers: net: ethernet: intel: e1000: e1000_ethtool.c coding style fixes
From: Joe Perches @ 2014-08-18 15:31 UTC (permalink / raw)
To: Alexander Duyck
Cc: Krzysztof Majzerowicz-Jaszcz, jeffrey.t.kirsher, netdev,
linux-kernel
In-Reply-To: <53F21BEC.2070204@intel.com>
On Mon, 2014-08-18 at 08:29 -0700, Alexander Duyck wrote:
> Doing any kind of pointer math on a void pointer is generally unsafe as
> it is an incomplete type. The only reason why it works in GCC is
> because GCC has a nonstandard extension that makes it report as having a
> size of 1.
I know. It's used in quite a few places in kernel code
so I believe it's now a base assumption for the kernel.
^ permalink raw reply
* Re: [PATCH] drivers: net: ethernet: intel: e1000: e1000_ethtool.c coding style fixes
From: Alexander Duyck @ 2014-08-18 15:29 UTC (permalink / raw)
To: Joe Perches, Krzysztof Majzerowicz-Jaszcz
Cc: jeffrey.t.kirsher, netdev, linux-kernel
In-Reply-To: <1408214489.2683.87.camel@joe-AO725>
On 08/16/2014 11:41 AM, Joe Perches wrote:
> On Sat, 2014-08-16 at 11:12 +0200, Krzysztof Majzerowicz-Jaszcz wrote:
>> @@ -1835,11 +1830,11 @@ static void e1000_get_ethtool_stats(struct net_device *netdev,
>> for (i = 0; i < E1000_GLOBAL_STATS_LEN; i++) {
>> switch (e1000_gstrings_stats[i].type) {
>> case NETDEV_STATS:
>> - p = (char *) netdev +
>> + p = (char *)netdev +
>> e1000_gstrings_stats[i].stat_offset;
>> break;
>> case E1000_STATS:
>> - p = (char *) adapter +
>> + p = (char *)adapter +
>> e1000_gstrings_stats[i].stat_offset;
>> brseak;
>> }
>
> Maybe use a temporary for &e1000_gstring_stats[i]
>
> Something like: (w/ void * for char *, WARN_ONCE, trigraph->if/else)
>
> static void e1000_get_ethtool_stats(struct net_device *netdev,
> struct ethtool_stats *stats, u64 *data)
> {
> struct e1000_adapter *adapter = netdev_priv(netdev);
> int i;
> void *p = NULL;
> const struct e1000_stats *stat = e1000_gstring_stats;
>
> e1000_update_stats(adapter);
>
> for (i = 0; i < E1000_GLOBAL_STATS_LEN; i++) {
> switch (stat->type) {
> case NETDEV_STATS:
> p = (void *)netdev + stat->stat_offset;
> break;
> case E1000_STATS:
> p = (void *)adapter + stat->stat_offset;
> break;
> default:
> WARN_ONCE(1, "Invalid E1000 stat type: %u index %d\n",
> stat->type, i);
> break;
> }
>
> if (stat->sizeof_stat == sizeof(u64))
> data[i] = *(u64 *)p;
> else
> data[i] = *(u32 *)p;
>
> stat++;
> }
> }
>
Doing any kind of pointer math on a void pointer is generally unsafe as
it is an incomplete type. The only reason why it works in GCC is
because GCC has a nonstandard extension that makes it report as having a
size of 1.
This is why the math is being done on a char * as it is a complete type
with a size of 1.
Thanks,
Alex
^ permalink raw reply
* Re: Performance regression on kernels 3.10 and newer
From: Rick Jones @ 2014-08-18 15:29 UTC (permalink / raw)
To: Alexander Duyck, David Laight, Tom Herbert
Cc: David Miller, Eric Dumazet, Linux Netdev List
In-Reply-To: <53F21A38.50905@intel.com>
Alex -
Why don't you go ahead and shoot me a patch to netperf to conditionally
put a poll() or select() in front of receive. I guess the "omni" code
would be the place to do it. The netperf-talk or netperf-dev lists
would be the place to send it I suspect.
happy benchmarking,
rick jones
^ permalink raw reply
* Re: Performance regression on kernels 3.10 and newer
From: Alexander Duyck @ 2014-08-18 15:22 UTC (permalink / raw)
To: David Laight, Tom Herbert
Cc: David Miller, Eric Dumazet, Linux Netdev List, Rick Jones
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1747969E@AcuExch.aculab.com>
On 08/18/2014 02:03 AM, David Laight wrote:
> From: Alexander Duyck
>> ...
>> Another test I tried was to hack the nettest_bsd.c file in netperf to
>> perform a poll() based receive. That resolved the issue and had all the
>> performance of the tcp_low_latency case. I may see if I can work with
>> Rick to push something like that into netperf as I really would prefer
>> to avoid having to advise everyone on how to setup the sysctl for
>> tcp_low_latency.
>
> Doesn't that generate 2 system calls per receive?
> Unless it now returns more data per receive I'm surprised that
> it actually faster.
If you haven't been keeping up with the thread what I am gaining by
doing this is avoiding a significant cache thrash issue with the dst
entry as the prequeue path involves updating the reference count that is
shared by all of my CPUs.
By using poll to wait for it I don't load frames onto the TCP prequeue
and thereby avoid it.
> OTOH I've some code that runs a lot better when I run while :; do :; done
> for all but one of the cpus.
> I think that is because the processes spinning in userspace don't
> get pre-empted.
>
> David
>
What you are probably seeing is that the CPU doesn't go into a deep
sleep state so it likely runs better. You might try the same thing with
a kernel booted with idle=poll and you would probably see the same result.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH v3 1/3] init / kthread: add module_long_probe_init() and module_long_probe_exit()
From: Oleg Nesterov @ 2014-08-18 15:19 UTC (permalink / raw)
To: Takashi Iwai
Cc: Luis R. Rodriguez, Luis R. Rodriguez, gregkh, linux-kernel,
Tetsuo Handa, Joseph Salisbury, Kay Sievers, One Thousand Gnomes,
Tim Gardner, Pierre Fersing, Andrew Morton, Benjamin Poirier,
Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
Abhijit Mahajan, Hariprasad S, Santosh Rastapur,
MPT-FusionLinux.pdl, linux-scsi, netdev
In-Reply-To: <s5ha97253yo.wl-tiwai@suse.de>
On 08/18, Takashi Iwai wrote:
>
> At Mon, 18 Aug 2014 14:22:17 +0200,
> Oleg Nesterov wrote:
> >
> > On 08/18, Takashi Iwai wrote:
> > >
> > > #define module_long_probe_init(initfn) \
> > > static int _long_probe_##initfn(void *arg) \
> > > { \
> > > module_put_and_exit(initfn()); \
> > > return 0; \
> > > } \
> > > static int __init __long_probe_##initfn(void) \
> > > { \
> > > struct task_struct *__init_thread = \
> > > kthread_create(_long_probe_##initfn, \
> > > NULL, #initfn); \
> > > if (IS_ERR(__init_thread)) \
> > > return PTR_ERR(__init_thread); \
> > > __module_get(THIS_MODULE); \
> > > wake_up_process(__init_thread); \
> > > return 0; \
> > > } \
> > > module_init(__long_probe_##initfn)
> > >
> > > ... and module_exit() remains identical as the normal version.
> >
> > Aaaah. This is not true, module_exit() should not call exitfn() if initfn()
> > fails... So _long_probe_##initfn() needs to save the error code which should
> > be checked by module_exit().
>
> Oh, right. So we need a reference in the module exit path in anyway,
We only need to save the error code,
static int _long_probe_retval;
static int _long_probe_##initfn(void *arg)
{
_long_probe_retval = initfn();
module_put_and_exit(0); /* noreturn */
}
static void __long_probe_##exitfn(void)
{
if (!_long_probe_retval)
exitfn();
}
> and Luis' version might be shorter in the end.
I dont't think that "shorter" does matter in this case. The real difference
is sys_delete_module() behaviour if it is called before initfn() completes.
And, again, I do not really know which version is better.
Oleg.
^ permalink raw reply
* Re: [PATCH 1/1] sctp: not send SCTP_PEER_ADDR_CHANGE notifications with failed probe
From: Daniel Borkmann @ 2014-08-18 15:01 UTC (permalink / raw)
To: Zhu Yanjun
Cc: linux-kernel, netdev, vyasevich, tuexen, khandelwal.deepak.1987,
Yue.Tao, alexandre.dietsch, davem, Zhu Yanjun
In-Reply-To: <1408094820-11725-2-git-send-email-Yanjun.Zhu@windriver.com>
On 08/15/2014 11:27 AM, Zhu Yanjun wrote:
> When a failed probe comes along UNCONFIRMED path, it is not necessary
> to send SCTP_PEER_ADDR_CHANGE notification.
I do not find this in the RFC, but it seems reasonable - at least, I would
have liked to see a more elaborate commit message from you explaining why
it's okay to do; at least RFC6458 I read:
SCTP_ADDR_UNREACHABLE:
The address specified can no longer be reached. Any data sent
to this address is rerouted to an alternate until this address
becomes reachable. This notification is provided whenever an
address *becomes* unreachable.
Given that the transport has always been in state SCTP_UNCONFIRMED, it
therefore wasn't active before and hasn't been used before, and one could
argue that it doesn't "become" "unreachable" but always has been, so we
wouldn't need to bug the user with a notification about it.
> Reported-by: DEEPAK KHANDELWAL <khandelwal.deepak.1987@gmail.com>
Nit: please write names normally: Deepak Khandelwal
> Suggested-by: Vlad Yasevich <vyasevich@gmail.com>
> Suggested-by: Michael Tuexen <tuexen@fh-muenster.de>
> Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
> ---
> net/sctp/associola.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 9de23a2..2e23f6b 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -813,6 +813,7 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
> else {
> dst_release(transport->dst);
> transport->dst = NULL;
> + ulp_notify = false;
> }
>
> spc_state = SCTP_ADDR_UNREACHABLE;
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox