* [net-next v2 13/16] ixgbevf: Add code to check for Tx hang
From: Jeff Kirsher @ 2015-02-06 4:31 UTC (permalink / raw)
To: davem
Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene, kernel-team,
Alexander Duyck, Jeff Kirsher
In-Reply-To: <1423197085-32270-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Emil Tantilov <emil.s.tantilov@intel.com>
This patch adds code to allow for Tx hang checking. The idea is to provide
more robust debug info in the event of a transmit unit hang. Similar to the
logic in ixgbe.
CC: Alexander Duyck <alexander.h.duyck@redhat.com>
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 21 +++-
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 113 ++++++++++++++++++----
2 files changed, 115 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 65cda34..65c2aee 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -43,6 +43,13 @@
#define BP_EXTENDED_STATS
#endif
+#define IXGBE_MAX_TXD_PWR 14
+#define IXGBE_MAX_DATA_PER_TXD BIT(IXGBE_MAX_TXD_PWR)
+
+/* Tx Descriptors needed, worst case */
+#define TXD_USE_COUNT(S) DIV_ROUND_UP((S), IXGBE_MAX_DATA_PER_TXD)
+#define DESC_NEEDED (MAX_SKB_FRAGS + 4)
+
/* wrapper around a pointer to a socket buffer,
* so a DMA handle can be stored along with the buffer */
struct ixgbevf_tx_buffer {
@@ -85,6 +92,18 @@ struct ixgbevf_rx_queue_stats {
u64 csum_err;
};
+enum ixgbevf_ring_state_t {
+ __IXGBEVF_TX_DETECT_HANG,
+ __IXGBEVF_HANG_CHECK_ARMED,
+};
+
+#define check_for_tx_hang(ring) \
+ test_bit(__IXGBEVF_TX_DETECT_HANG, &(ring)->state)
+#define set_check_for_tx_hang(ring) \
+ set_bit(__IXGBEVF_TX_DETECT_HANG, &(ring)->state)
+#define clear_check_for_tx_hang(ring) \
+ clear_bit(__IXGBEVF_TX_DETECT_HANG, &(ring)->state)
+
struct ixgbevf_ring {
struct ixgbevf_ring *next;
struct net_device *netdev;
@@ -101,7 +120,7 @@ struct ixgbevf_ring {
struct ixgbevf_tx_buffer *tx_buffer_info;
struct ixgbevf_rx_buffer *rx_buffer_info;
};
-
+ unsigned long state;
struct ixgbevf_stats stats;
struct u64_stats_sync syncp;
union {
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index a4b3d66..87f9f86 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -199,14 +199,64 @@ static void ixgbevf_unmap_and_free_tx_resource(struct ixgbevf_ring *tx_ring,
/* tx_buffer must be completely set up in the transmit path */
}
-#define IXGBE_MAX_TXD_PWR 14
-#define IXGBE_MAX_DATA_PER_TXD (1 << IXGBE_MAX_TXD_PWR)
+static u64 ixgbevf_get_tx_completed(struct ixgbevf_ring *ring)
+{
+ return ring->stats.packets;
+}
+
+static u32 ixgbevf_get_tx_pending(struct ixgbevf_ring *ring)
+{
+ struct ixgbevf_adapter *adapter = netdev_priv(ring->netdev);
+ struct ixgbe_hw *hw = &adapter->hw;
+
+ u32 head = IXGBE_READ_REG(hw, IXGBE_VFTDH(ring->reg_idx));
+ u32 tail = IXGBE_READ_REG(hw, IXGBE_VFTDT(ring->reg_idx));
+
+ if (head != tail)
+ return (head < tail) ?
+ tail - head : (tail + ring->count - head);
+
+ return 0;
+}
+
+static inline bool ixgbevf_check_tx_hang(struct ixgbevf_ring *tx_ring)
+{
+ u32 tx_done = ixgbevf_get_tx_completed(tx_ring);
+ u32 tx_done_old = tx_ring->tx_stats.tx_done_old;
+ u32 tx_pending = ixgbevf_get_tx_pending(tx_ring);
+
+ clear_check_for_tx_hang(tx_ring);
-/* Tx Descriptors needed, worst case */
-#define TXD_USE_COUNT(S) DIV_ROUND_UP((S), IXGBE_MAX_DATA_PER_TXD)
-#define DESC_NEEDED (MAX_SKB_FRAGS + 4)
+ /* Check for a hung queue, but be thorough. This verifies
+ * that a transmit has been completed since the previous
+ * check AND there is at least one packet pending. The
+ * ARMED bit is set to indicate a potential hang.
+ */
+ if ((tx_done_old == tx_done) && tx_pending) {
+ /* make sure it is true for two checks in a row */
+ return test_and_set_bit(__IXGBEVF_HANG_CHECK_ARMED,
+ &tx_ring->state);
+ }
+ /* reset the countdown */
+ clear_bit(__IXGBEVF_HANG_CHECK_ARMED, &tx_ring->state);
+
+ /* update completed stats and continue */
+ tx_ring->tx_stats.tx_done_old = tx_done;
+
+ return false;
+}
+
+/**
+ * ixgbevf_tx_timeout - Respond to a Tx Hang
+ * @netdev: network interface device structure
+ **/
+static void ixgbevf_tx_timeout(struct net_device *netdev)
+{
+ struct ixgbevf_adapter *adapter = netdev_priv(netdev);
-static void ixgbevf_tx_timeout(struct net_device *netdev);
+ /* Do the reset outside of interrupt context */
+ schedule_work(&adapter->reset_task);
+}
/**
* ixgbevf_clean_tx_irq - Reclaim resources after transmit completes
@@ -311,6 +361,37 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
q_vector->tx.total_bytes += total_bytes;
q_vector->tx.total_packets += total_packets;
+ if (check_for_tx_hang(tx_ring) && ixgbevf_check_tx_hang(tx_ring)) {
+ struct ixgbe_hw *hw = &adapter->hw;
+ union ixgbe_adv_tx_desc *eop_desc;
+
+ eop_desc = tx_ring->tx_buffer_info[i].next_to_watch;
+
+ pr_err("Detected Tx Unit Hang\n"
+ " Tx Queue <%d>\n"
+ " TDH, TDT <%x>, <%x>\n"
+ " next_to_use <%x>\n"
+ " next_to_clean <%x>\n"
+ "tx_buffer_info[next_to_clean]\n"
+ " next_to_watch <%p>\n"
+ " eop_desc->wb.status <%x>\n"
+ " time_stamp <%lx>\n"
+ " jiffies <%lx>\n",
+ tx_ring->queue_index,
+ IXGBE_READ_REG(hw, IXGBE_VFTDH(tx_ring->reg_idx)),
+ IXGBE_READ_REG(hw, IXGBE_VFTDT(tx_ring->reg_idx)),
+ tx_ring->next_to_use, i,
+ eop_desc, (eop_desc ? eop_desc->wb.status : 0),
+ tx_ring->tx_buffer_info[i].time_stamp, jiffies);
+
+ netif_stop_subqueue(tx_ring->netdev, tx_ring->queue_index);
+
+ /* schedule immediate reset if we believe we hung */
+ schedule_work(&adapter->reset_task);
+
+ return true;
+ }
+
#define TX_WAKE_THRESHOLD (DESC_NEEDED * 2)
if (unlikely(total_packets && netif_carrier_ok(tx_ring->netdev) &&
(ixgbevf_desc_unused(tx_ring) >= TX_WAKE_THRESHOLD))) {
@@ -1479,6 +1560,8 @@ static void ixgbevf_configure_tx_ring(struct ixgbevf_adapter *adapter,
txdctl |= (1 << 8) | /* HTHRESH = 1 */
32; /* PTHRESH = 32 */
+ clear_bit(__IXGBEVF_HANG_CHECK_ARMED, &ring->state);
+
IXGBE_WRITE_REG(hw, IXGBE_VFTXDCTL(reg_idx), txdctl);
/* poll to verify queue is enabled */
@@ -2643,6 +2726,12 @@ static void ixgbevf_watchdog(unsigned long data)
if (test_bit(__IXGBEVF_DOWN, &adapter->state))
goto watchdog_short_circuit;
+ /* Force detection of hung controller */
+ if (netif_carrier_ok(adapter->netdev)) {
+ for (i = 0; i < adapter->num_tx_queues; i++)
+ set_check_for_tx_hang(adapter->tx_ring[i]);
+ }
+
/* get one bit for every active tx/rx interrupt vector */
for (i = 0; i < adapter->num_msix_vectors - NON_Q_VECTORS; i++) {
struct ixgbevf_q_vector *qv = adapter->q_vector[i];
@@ -2656,18 +2745,6 @@ watchdog_short_circuit:
schedule_work(&adapter->watchdog_task);
}
-/**
- * ixgbevf_tx_timeout - Respond to a Tx Hang
- * @netdev: network interface device structure
- **/
-static void ixgbevf_tx_timeout(struct net_device *netdev)
-{
- struct ixgbevf_adapter *adapter = netdev_priv(netdev);
-
- /* Do the reset outside of interrupt context */
- schedule_work(&adapter->reset_task);
-}
-
static void ixgbevf_reset_task(struct work_struct *work)
{
struct ixgbevf_adapter *adapter;
--
1.9.3
^ permalink raw reply related
* [net-next v2 14/16] ixgbevf: rewrite watchdog task to function similar to igbvf
From: Jeff Kirsher @ 2015-02-06 4:31 UTC (permalink / raw)
To: davem
Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene, kernel-team,
Alexander Duyck, Jeff Kirsher
In-Reply-To: <1423197085-32270-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Emil Tantilov <emil.s.tantilov@intel.com>
This patch cleans up the logic dealing with link down/up by breaking down the
link detection and up/down events into separate functions - similar to how these
events are handled in other drivers.
CC: Alexander Duyck <alexander.h.duyck@redhat.com>
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 +
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 187 +++++++++++++---------
2 files changed, 113 insertions(+), 75 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 65c2aee..a41ab37 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -436,6 +436,7 @@ struct ixgbevf_adapter {
bool link_up;
spinlock_t mbx_lock;
+ unsigned long last_reset;
struct work_struct watchdog_task;
};
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 87f9f86..c110065 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -2231,6 +2231,8 @@ void ixgbevf_reset(struct ixgbevf_adapter *adapter)
memcpy(netdev->perm_addr, adapter->hw.mac.addr,
netdev->addr_len);
}
+
+ adapter->last_reset = jiffies;
}
static int ixgbevf_acquire_msix_vectors(struct ixgbevf_adapter *adapter,
@@ -2684,7 +2686,8 @@ void ixgbevf_update_stats(struct ixgbevf_adapter *adapter)
struct ixgbe_hw *hw = &adapter->hw;
int i;
- if (!adapter->link_up)
+ if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
+ test_bit(__IXGBEVF_RESETTING, &adapter->state))
return;
UPDATE_VF_COUNTER_32bit(IXGBE_VFGPRC, adapter->stats.last_vfgprc,
@@ -2714,17 +2717,45 @@ void ixgbevf_update_stats(struct ixgbevf_adapter *adapter)
static void ixgbevf_watchdog(unsigned long data)
{
struct ixgbevf_adapter *adapter = (struct ixgbevf_adapter *)data;
+
+ /* Do the reset outside of interrupt context */
+ schedule_work(&adapter->watchdog_task);
+}
+
+static void ixgbevf_reset_task(struct work_struct *work)
+{
+ struct ixgbevf_adapter *adapter;
+
+ adapter = container_of(work, struct ixgbevf_adapter, reset_task);
+
+ /* If we're already down or resetting, just bail */
+ if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
+ test_bit(__IXGBEVF_RESETTING, &adapter->state))
+ return;
+
+ adapter->tx_timeout_count++;
+
+ ixgbevf_reinit_locked(adapter);
+}
+
+/* ixgbevf_check_hang_subtask - check for hung queues and dropped interrupts
+ * @adapter - pointer to the device adapter structure
+ *
+ * This function serves two purposes. First it strobes the interrupt lines
+ * in order to make certain interrupts are occurring. Secondly it sets the
+ * bits needed to check for TX hangs. As a result we should immediately
+ * determine if a hang has occurred.
+ */
+static void ixgbevf_check_hang_subtask(struct ixgbevf_adapter *adapter)
+{
struct ixgbe_hw *hw = &adapter->hw;
u32 eics = 0;
int i;
- /*
- * Do the watchdog outside of interrupt context due to the lovely
- * delays that some of the newer hardware requires
- */
-
- if (test_bit(__IXGBEVF_DOWN, &adapter->state))
- goto watchdog_short_circuit;
+ /* If we're down or resetting, just bail */
+ if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
+ test_bit(__IXGBEVF_RESETTING, &adapter->state))
+ return;
/* Force detection of hung controller */
if (netif_carrier_ok(adapter->netdev)) {
@@ -2739,26 +2770,80 @@ static void ixgbevf_watchdog(unsigned long data)
eics |= 1 << i;
}
+ /* Cause software interrupt to ensure rings are cleaned */
IXGBE_WRITE_REG(hw, IXGBE_VTEICS, eics);
+}
-watchdog_short_circuit:
- schedule_work(&adapter->watchdog_task);
+/**
+ * ixgbevf_watchdog_update_link - update the link status
+ * @adapter - pointer to the device adapter structure
+ **/
+static void ixgbevf_watchdog_update_link(struct ixgbevf_adapter *adapter)
+{
+ struct ixgbe_hw *hw = &adapter->hw;
+ u32 link_speed = adapter->link_speed;
+ bool link_up = adapter->link_up;
+ s32 err;
+
+ spin_lock_bh(&adapter->mbx_lock);
+
+ err = hw->mac.ops.check_link(hw, &link_speed, &link_up, false);
+
+ spin_unlock_bh(&adapter->mbx_lock);
+
+ /* if check for link returns error we will need to reset */
+ if (err && time_after(jiffies, adapter->last_reset + (10 * HZ))) {
+ schedule_work(&adapter->reset_task);
+ link_up = false;
+ }
+
+ adapter->link_up = link_up;
+ adapter->link_speed = link_speed;
}
-static void ixgbevf_reset_task(struct work_struct *work)
+/**
+ * ixgbevf_watchdog_link_is_up - update netif_carrier status and
+ * print link up message
+ * @adapter - pointer to the device adapter structure
+ **/
+static void ixgbevf_watchdog_link_is_up(struct ixgbevf_adapter *adapter)
{
- struct ixgbevf_adapter *adapter;
- adapter = container_of(work, struct ixgbevf_adapter, reset_task);
+ struct net_device *netdev = adapter->netdev;
- /* If we're already down or resetting, just bail */
- if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
- test_bit(__IXGBEVF_REMOVING, &adapter->state) ||
- test_bit(__IXGBEVF_RESETTING, &adapter->state))
+ /* only continue if link was previously down */
+ if (netif_carrier_ok(netdev))
return;
- adapter->tx_timeout_count++;
+ dev_info(&adapter->pdev->dev, "NIC Link is Up %s\n",
+ (adapter->link_speed == IXGBE_LINK_SPEED_10GB_FULL) ?
+ "10 Gbps" :
+ (adapter->link_speed == IXGBE_LINK_SPEED_1GB_FULL) ?
+ "1 Gbps" :
+ (adapter->link_speed == IXGBE_LINK_SPEED_100_FULL) ?
+ "100 Mbps" :
+ "unknown speed");
- ixgbevf_reinit_locked(adapter);
+ netif_carrier_on(netdev);
+}
+
+/**
+ * ixgbevf_watchdog_link_is_down - update netif_carrier status and
+ * print link down message
+ * @adapter - pointer to the adapter structure
+ **/
+static void ixgbevf_watchdog_link_is_down(struct ixgbevf_adapter *adapter)
+{
+ struct net_device *netdev = adapter->netdev;
+
+ adapter->link_speed = 0;
+
+ /* only continue if link was up previously */
+ if (!netif_carrier_ok(netdev))
+ return;
+
+ dev_info(&adapter->pdev->dev, "NIC Link is Down\n");
+
+ netif_carrier_off(netdev);
}
/**
@@ -2770,11 +2855,7 @@ static void ixgbevf_watchdog_task(struct work_struct *work)
struct ixgbevf_adapter *adapter = container_of(work,
struct ixgbevf_adapter,
watchdog_task);
- struct net_device *netdev = adapter->netdev;
struct ixgbe_hw *hw = &adapter->hw;
- u32 link_speed = adapter->link_speed;
- bool link_up = adapter->link_up;
- s32 need_reset;
if (IXGBE_REMOVED(hw->hw_addr)) {
if (!test_bit(__IXGBEVF_DOWN, &adapter->state)) {
@@ -2784,66 +2865,22 @@ static void ixgbevf_watchdog_task(struct work_struct *work)
}
return;
}
+
ixgbevf_queue_reset_subtask(adapter);
adapter->flags |= IXGBE_FLAG_IN_WATCHDOG_TASK;
- /*
- * Always check the link on the watchdog because we have
- * no LSC interrupt
- */
- spin_lock_bh(&adapter->mbx_lock);
-
- need_reset = hw->mac.ops.check_link(hw, &link_speed, &link_up, false);
-
- spin_unlock_bh(&adapter->mbx_lock);
-
- if (need_reset) {
- adapter->link_up = link_up;
- adapter->link_speed = link_speed;
- netif_carrier_off(netdev);
- netif_tx_stop_all_queues(netdev);
- schedule_work(&adapter->reset_task);
- goto pf_has_reset;
- }
- adapter->link_up = link_up;
- adapter->link_speed = link_speed;
+ ixgbevf_watchdog_update_link(adapter);
- if (link_up) {
- if (!netif_carrier_ok(netdev)) {
- char *link_speed_string;
- switch (link_speed) {
- case IXGBE_LINK_SPEED_10GB_FULL:
- link_speed_string = "10 Gbps";
- break;
- case IXGBE_LINK_SPEED_1GB_FULL:
- link_speed_string = "1 Gbps";
- break;
- case IXGBE_LINK_SPEED_100_FULL:
- link_speed_string = "100 Mbps";
- break;
- default:
- link_speed_string = "unknown speed";
- break;
- }
- dev_info(&adapter->pdev->dev,
- "NIC Link is Up, %s\n", link_speed_string);
- netif_carrier_on(netdev);
- netif_tx_wake_all_queues(netdev);
- }
- } else {
- adapter->link_up = false;
- adapter->link_speed = 0;
- if (netif_carrier_ok(netdev)) {
- dev_info(&adapter->pdev->dev, "NIC Link is Down\n");
- netif_carrier_off(netdev);
- netif_tx_stop_all_queues(netdev);
- }
- }
+ if (adapter->link_up)
+ ixgbevf_watchdog_link_is_up(adapter);
+ else
+ ixgbevf_watchdog_link_is_down(adapter);
ixgbevf_update_stats(adapter);
-pf_has_reset:
+ ixgbevf_check_hang_subtask(adapter);
+
/* Reset the timer */
if (!test_bit(__IXGBEVF_DOWN, &adapter->state) &&
!test_bit(__IXGBEVF_REMOVING, &adapter->state))
--
1.9.3
^ permalink raw reply related
* [net-next v2 15/16] ixgbevf: combine all of the tasks into a single service task
From: Jeff Kirsher @ 2015-02-06 4:31 UTC (permalink / raw)
To: davem
Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene, kernel-team,
Alexander Duyck, Jeff Kirsher
In-Reply-To: <1423197085-32270-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Emil Tantilov <emil.s.tantilov@intel.com>
This change combines the reset and watchdog tasklets into a single task.
The advantage of this is that we can avoid multiple schedules of the reset
task when we have a reset event needed due to either the mailbox going down
or transmit packets being present on a link down.
CC: Alexander Duyck <alexander.h.duyck@redhat.com>
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 13 +-
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 140 +++++++++++++---------
2 files changed, 87 insertions(+), 66 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index a41ab37..3a9b356 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -367,8 +367,6 @@ struct ixgbevf_adapter {
/* this field must be first, see ixgbevf_process_skb_fields */
unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
- struct timer_list watchdog_timer;
- struct work_struct reset_task;
struct ixgbevf_q_vector *q_vector[MAX_MSIX_Q_VECTORS];
/* Interrupt Throttle Rate */
@@ -398,8 +396,7 @@ struct ixgbevf_adapter {
* thus the additional *_CAPABLE flags.
*/
u32 flags;
-#define IXGBE_FLAG_IN_WATCHDOG_TASK (u32)(1)
-
+#define IXGBEVF_FLAG_RESET_REQUESTED (u32)(1)
#define IXGBEVF_FLAG_QUEUE_RESET_REQUESTED (u32)(1 << 2)
struct msix_entry *msix_entries;
@@ -435,10 +432,11 @@ struct ixgbevf_adapter {
u32 link_speed;
bool link_up;
+ struct timer_list service_timer;
+ struct work_struct service_task;
+
spinlock_t mbx_lock;
unsigned long last_reset;
-
- struct work_struct watchdog_task;
};
enum ixbgevf_state_t {
@@ -447,7 +445,8 @@ enum ixbgevf_state_t {
__IXGBEVF_DOWN,
__IXGBEVF_DISABLED,
__IXGBEVF_REMOVING,
- __IXGBEVF_WORK_INIT,
+ __IXGBEVF_SERVICE_SCHED,
+ __IXGBEVF_SERVICE_INITED,
};
enum ixgbevf_boards {
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index c110065..4186981 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -98,6 +98,23 @@ static int debug = -1;
module_param(debug, int, 0);
MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
+static void ixgbevf_service_event_schedule(struct ixgbevf_adapter *adapter)
+{
+ if (!test_bit(__IXGBEVF_DOWN, &adapter->state) &&
+ !test_bit(__IXGBEVF_REMOVING, &adapter->state) &&
+ !test_and_set_bit(__IXGBEVF_SERVICE_SCHED, &adapter->state))
+ schedule_work(&adapter->service_task);
+}
+
+static void ixgbevf_service_event_complete(struct ixgbevf_adapter *adapter)
+{
+ BUG_ON(!test_bit(__IXGBEVF_SERVICE_SCHED, &adapter->state));
+
+ /* flush memory to make sure state is correct before next watchdog */
+ smp_mb__before_atomic();
+ clear_bit(__IXGBEVF_SERVICE_SCHED, &adapter->state);
+}
+
/* forward decls */
static void ixgbevf_queue_reset_subtask(struct ixgbevf_adapter *adapter);
static void ixgbevf_set_itr(struct ixgbevf_q_vector *q_vector);
@@ -111,8 +128,8 @@ static void ixgbevf_remove_adapter(struct ixgbe_hw *hw)
return;
hw->hw_addr = NULL;
dev_err(&adapter->pdev->dev, "Adapter removed\n");
- if (test_bit(__IXGBEVF_WORK_INIT, &adapter->state))
- schedule_work(&adapter->watchdog_task);
+ if (test_bit(__IXGBEVF_SERVICE_INITED, &adapter->state))
+ ixgbevf_service_event_schedule(adapter);
}
static void ixgbevf_check_remove(struct ixgbe_hw *hw, u32 reg)
@@ -246,6 +263,15 @@ static inline bool ixgbevf_check_tx_hang(struct ixgbevf_ring *tx_ring)
return false;
}
+static void ixgbevf_tx_timeout_reset(struct ixgbevf_adapter *adapter)
+{
+ /* Do the reset outside of interrupt context */
+ if (!test_bit(__IXGBEVF_DOWN, &adapter->state)) {
+ adapter->flags |= IXGBEVF_FLAG_RESET_REQUESTED;
+ ixgbevf_service_event_schedule(adapter);
+ }
+}
+
/**
* ixgbevf_tx_timeout - Respond to a Tx Hang
* @netdev: network interface device structure
@@ -254,8 +280,7 @@ static void ixgbevf_tx_timeout(struct net_device *netdev)
{
struct ixgbevf_adapter *adapter = netdev_priv(netdev);
- /* Do the reset outside of interrupt context */
- schedule_work(&adapter->reset_task);
+ ixgbevf_tx_timeout_reset(adapter);
}
/**
@@ -387,7 +412,7 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
netif_stop_subqueue(tx_ring->netdev, tx_ring->queue_index);
/* schedule immediate reset if we believe we hung */
- schedule_work(&adapter->reset_task);
+ ixgbevf_tx_timeout_reset(adapter);
return true;
}
@@ -1239,9 +1264,7 @@ static irqreturn_t ixgbevf_msix_other(int irq, void *data)
hw->mac.get_link_status = 1;
- if (!test_bit(__IXGBEVF_DOWN, &adapter->state) &&
- !test_bit(__IXGBEVF_REMOVING, &adapter->state))
- mod_timer(&adapter->watchdog_timer, jiffies);
+ ixgbevf_service_event_schedule(adapter);
IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, adapter->eims_other);
@@ -2051,7 +2074,7 @@ static void ixgbevf_up_complete(struct ixgbevf_adapter *adapter)
ixgbevf_init_last_counter_stats(adapter);
hw->mac.get_link_status = 1;
- mod_timer(&adapter->watchdog_timer, jiffies);
+ mod_timer(&adapter->service_timer, jiffies);
}
void ixgbevf_up(struct ixgbevf_adapter *adapter)
@@ -2177,13 +2200,7 @@ void ixgbevf_down(struct ixgbevf_adapter *adapter)
ixgbevf_napi_disable_all(adapter);
- del_timer_sync(&adapter->watchdog_timer);
-
- /* can't call flush scheduled work here because it can deadlock
- * if linkwatch_event tries to acquire the rtnl_lock which we are
- * holding */
- while (adapter->flags & IXGBE_FLAG_IN_WATCHDOG_TASK)
- msleep(1);
+ del_timer_sync(&adapter->service_timer);
/* disable transmits in the hardware now that interrupts are off */
for (i = 0; i < adapter->num_tx_queues; i++) {
@@ -2711,22 +2728,25 @@ void ixgbevf_update_stats(struct ixgbevf_adapter *adapter)
}
/**
- * ixgbevf_watchdog - Timer Call-back
+ * ixgbevf_service_timer - Timer Call-back
* @data: pointer to adapter cast into an unsigned long
**/
-static void ixgbevf_watchdog(unsigned long data)
+static void ixgbevf_service_timer(unsigned long data)
{
struct ixgbevf_adapter *adapter = (struct ixgbevf_adapter *)data;
- /* Do the reset outside of interrupt context */
- schedule_work(&adapter->watchdog_task);
+ /* Reset the timer */
+ mod_timer(&adapter->service_timer, (HZ * 2) + jiffies);
+
+ ixgbevf_service_event_schedule(adapter);
}
-static void ixgbevf_reset_task(struct work_struct *work)
+static void ixgbevf_reset_subtask(struct ixgbevf_adapter *adapter)
{
- struct ixgbevf_adapter *adapter;
+ if (!(adapter->flags & IXGBEVF_FLAG_RESET_REQUESTED))
+ return;
- adapter = container_of(work, struct ixgbevf_adapter, reset_task);
+ adapter->flags &= ~IXGBEVF_FLAG_RESET_REQUESTED;
/* If we're already down or resetting, just bail */
if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
@@ -2766,6 +2786,7 @@ static void ixgbevf_check_hang_subtask(struct ixgbevf_adapter *adapter)
/* get one bit for every active tx/rx interrupt vector */
for (i = 0; i < adapter->num_msix_vectors - NON_Q_VECTORS; i++) {
struct ixgbevf_q_vector *qv = adapter->q_vector[i];
+
if (qv->rx.ring || qv->tx.ring)
eics |= 1 << i;
}
@@ -2793,7 +2814,7 @@ static void ixgbevf_watchdog_update_link(struct ixgbevf_adapter *adapter)
/* if check for link returns error we will need to reset */
if (err && time_after(jiffies, adapter->last_reset + (10 * HZ))) {
- schedule_work(&adapter->reset_task);
+ adapter->flags |= IXGBEVF_FLAG_RESET_REQUESTED;
link_up = false;
}
@@ -2847,14 +2868,35 @@ static void ixgbevf_watchdog_link_is_down(struct ixgbevf_adapter *adapter)
}
/**
- * ixgbevf_watchdog_task - worker thread to bring link up
+ * ixgbevf_watchdog_subtask - worker thread to bring link up
+ * @work: pointer to work_struct containing our data
+ **/
+static void ixgbevf_watchdog_subtask(struct ixgbevf_adapter *adapter)
+{
+ /* if interface is down do nothing */
+ if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
+ test_bit(__IXGBEVF_RESETTING, &adapter->state))
+ return;
+
+ ixgbevf_watchdog_update_link(adapter);
+
+ if (adapter->link_up)
+ ixgbevf_watchdog_link_is_up(adapter);
+ else
+ ixgbevf_watchdog_link_is_down(adapter);
+
+ ixgbevf_update_stats(adapter);
+}
+
+/**
+ * ixgbevf_service_task - manages and runs subtasks
* @work: pointer to work_struct containing our data
**/
-static void ixgbevf_watchdog_task(struct work_struct *work)
+static void ixgbevf_service_task(struct work_struct *work)
{
struct ixgbevf_adapter *adapter = container_of(work,
struct ixgbevf_adapter,
- watchdog_task);
+ service_task);
struct ixgbe_hw *hw = &adapter->hw;
if (IXGBE_REMOVED(hw->hw_addr)) {
@@ -2867,27 +2909,11 @@ static void ixgbevf_watchdog_task(struct work_struct *work)
}
ixgbevf_queue_reset_subtask(adapter);
-
- adapter->flags |= IXGBE_FLAG_IN_WATCHDOG_TASK;
-
- ixgbevf_watchdog_update_link(adapter);
-
- if (adapter->link_up)
- ixgbevf_watchdog_link_is_up(adapter);
- else
- ixgbevf_watchdog_link_is_down(adapter);
-
- ixgbevf_update_stats(adapter);
-
+ ixgbevf_reset_subtask(adapter);
+ ixgbevf_watchdog_subtask(adapter);
ixgbevf_check_hang_subtask(adapter);
- /* Reset the timer */
- if (!test_bit(__IXGBEVF_DOWN, &adapter->state) &&
- !test_bit(__IXGBEVF_REMOVING, &adapter->state))
- mod_timer(&adapter->watchdog_timer,
- round_jiffies(jiffies + (2 * HZ)));
-
- adapter->flags &= ~IXGBE_FLAG_IN_WATCHDOG_TASK;
+ ixgbevf_service_event_complete(adapter);
}
/**
@@ -3994,17 +4020,17 @@ static int ixgbevf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
netdev->priv_flags |= IFF_UNICAST_FLT;
- init_timer(&adapter->watchdog_timer);
- adapter->watchdog_timer.function = ixgbevf_watchdog;
- adapter->watchdog_timer.data = (unsigned long)adapter;
-
if (IXGBE_REMOVED(hw->hw_addr)) {
err = -EIO;
goto err_sw_init;
}
- INIT_WORK(&adapter->reset_task, ixgbevf_reset_task);
- INIT_WORK(&adapter->watchdog_task, ixgbevf_watchdog_task);
- set_bit(__IXGBEVF_WORK_INIT, &adapter->state);
+
+ setup_timer(&adapter->service_timer, &ixgbevf_service_timer,
+ (unsigned long)adapter);
+
+ INIT_WORK(&adapter->service_task, ixgbevf_service_task);
+ set_bit(__IXGBEVF_SERVICE_INITED, &adapter->state);
+ clear_bit(__IXGBEVF_SERVICE_SCHED, &adapter->state);
err = ixgbevf_init_interrupt_scheme(adapter);
if (err)
@@ -4078,11 +4104,7 @@ static void ixgbevf_remove(struct pci_dev *pdev)
adapter = netdev_priv(netdev);
set_bit(__IXGBEVF_REMOVING, &adapter->state);
-
- del_timer_sync(&adapter->watchdog_timer);
-
- cancel_work_sync(&adapter->reset_task);
- cancel_work_sync(&adapter->watchdog_task);
+ cancel_work_sync(&adapter->service_task);
if (netdev->reg_state == NETREG_REGISTERED)
unregister_netdev(netdev);
@@ -4116,7 +4138,7 @@ static pci_ers_result_t ixgbevf_io_error_detected(struct pci_dev *pdev,
struct net_device *netdev = pci_get_drvdata(pdev);
struct ixgbevf_adapter *adapter = netdev_priv(netdev);
- if (!test_bit(__IXGBEVF_WORK_INIT, &adapter->state))
+ if (!test_bit(__IXGBEVF_SERVICE_INITED, &adapter->state))
return PCI_ERS_RESULT_DISCONNECT;
rtnl_lock();
--
1.9.3
^ permalink raw reply related
* [net-next v2 16/16] ixgbe: add Tx anti spoofing support
From: Jeff Kirsher @ 2015-02-06 4:31 UTC (permalink / raw)
To: davem
Cc: Don Skidmore, netdev, nhorman, sassmann, jogreene, kernel-team,
Jeff Kirsher
In-Reply-To: <1423197085-32270-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Don Skidmore <donald.c.skidmore@intel.com>
This patch enables the ethertype Anti-Spoofing feature for affected
devices. It is configured such that LLDP packets sent by a VF will
be dropped.
Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 ++
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 14 ++++++++++++++
drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 7 +++++++
drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 24 ++++++++++++++++++++++++
4 files changed, 47 insertions(+)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 699117af..7dcbbec 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -76,6 +76,8 @@
#define IXGBE_MAX_RXD 4096
#define IXGBE_MIN_RXD 64
+#define IXGBE_ETH_P_LLDP 0x88CC
+
/* flow control */
#define IXGBE_MIN_FCRTL 0x40
#define IXGBE_MAX_FCRTL 0x7FF80
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 6aa9b96..70cc4c5 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3585,10 +3585,24 @@ static void ixgbe_configure_virtualization(struct ixgbe_adapter *adapter)
/* Enable MAC Anti-Spoofing */
hw->mac.ops.set_mac_anti_spoofing(hw, (adapter->num_vfs != 0),
adapter->num_vfs);
+
+ /* Ensure LLDP is set for Ethertype Antispoofing if we will be
+ * calling set_ethertype_anti_spoofing for each VF in loop below
+ */
+ if (hw->mac.ops.set_ethertype_anti_spoofing)
+ IXGBE_WRITE_REG(hw, IXGBE_ETQF(IXGBE_ETQF_FILTER_LLDP),
+ (IXGBE_ETQF_FILTER_EN | /* enable filter */
+ IXGBE_ETQF_TX_ANTISPOOF | /* tx antispoof */
+ IXGBE_ETH_P_LLDP)); /* LLDP eth type */
+
/* For VFs that have spoof checking turned off */
for (i = 0; i < adapter->num_vfs; i++) {
if (!adapter->vfinfo[i].spoofchk_enabled)
ixgbe_ndo_set_vf_spoofchk(adapter->netdev, i, false);
+
+ /* enable ethertype anti spoofing if hw supports it */
+ if (hw->mac.ops.set_ethertype_anti_spoofing)
+ hw->mac.ops.set_ethertype_anti_spoofing(hw, true, i);
}
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index 02b57bf..fc5ecee 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -378,6 +378,8 @@ struct ixgbe_thermal_sensor_data {
#define IXGBE_SPOOF_MACAS_MASK 0xFF
#define IXGBE_SPOOF_VLANAS_MASK 0xFF00
#define IXGBE_SPOOF_VLANAS_SHIFT 8
+#define IXGBE_SPOOF_ETHERTYPEAS 0xFF000000
+#define IXGBE_SPOOF_ETHERTYPEAS_SHIFT 16
#define IXGBE_PFVFSPOOF_REG_COUNT 8
#define IXGBE_DCA_TXCTRL(_i) (0x07200 + ((_i) * 4)) /* 16 of these (0-15) */
@@ -1541,6 +1543,7 @@ enum {
#define IXGBE_MAX_ETQF_FILTERS 8
#define IXGBE_ETQF_FCOE 0x08000000 /* bit 27 */
#define IXGBE_ETQF_BCN 0x10000000 /* bit 28 */
+#define IXGBE_ETQF_TX_ANTISPOOF 0x20000000 /* bit 29 */
#define IXGBE_ETQF_1588 0x40000000 /* bit 30 */
#define IXGBE_ETQF_FILTER_EN 0x80000000 /* bit 31 */
#define IXGBE_ETQF_POOL_ENABLE (1 << 26) /* bit 26 */
@@ -1566,6 +1569,9 @@ enum {
#define IXGBE_ETQF_FILTER_FCOE 2
#define IXGBE_ETQF_FILTER_1588 3
#define IXGBE_ETQF_FILTER_FIP 4
+#define IXGBE_ETQF_FILTER_LLDP 5
+#define IXGBE_ETQF_FILTER_LACP 6
+
/* VLAN Control Bit Masks */
#define IXGBE_VLNCTRL_VET 0x0000FFFF /* bits 0-15 */
#define IXGBE_VLNCTRL_CFI 0x10000000 /* bit 28 */
@@ -3061,6 +3067,7 @@ struct ixgbe_mac_operations {
s32 (*set_fw_drv_ver)(struct ixgbe_hw *, u8, u8, u8, u8);
s32 (*get_thermal_sensor_data)(struct ixgbe_hw *);
s32 (*init_thermal_sensor_thresh)(struct ixgbe_hw *hw);
+ void (*set_ethertype_anti_spoofing)(struct ixgbe_hw *, bool, int);
/* DMA Coalescing */
s32 (*dmac_config)(struct ixgbe_hw *hw);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
index 84affca..50bf819 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
@@ -1300,6 +1300,28 @@ mac_reset_top:
return status;
}
+/** ixgbe_set_ethertype_anti_spoofing_X550 - Enable/Disable Ethertype
+ * anti-spoofing
+ * @hw: pointer to hardware structure
+ * @enable: enable or disable switch for Ethertype anti-spoofing
+ * @vf: Virtual Function pool - VF Pool to set for Ethertype anti-spoofing
+ **/
+void ixgbe_set_ethertype_anti_spoofing_X550(struct ixgbe_hw *hw, bool enable,
+ int vf)
+{
+ int vf_target_reg = vf >> 3;
+ int vf_target_shift = vf % 8 + IXGBE_SPOOF_ETHERTYPEAS_SHIFT;
+ u32 pfvfspoof;
+
+ pfvfspoof = IXGBE_READ_REG(hw, IXGBE_PFVFSPOOF(vf_target_reg));
+ if (enable)
+ pfvfspoof |= (1 << vf_target_shift);
+ else
+ pfvfspoof &= ~(1 << vf_target_shift);
+
+ IXGBE_WRITE_REG(hw, IXGBE_PFVFSPOOF(vf_target_reg), pfvfspoof);
+}
+
#define X550_COMMON_MAC \
.init_hw = &ixgbe_init_hw_generic, \
.start_hw = &ixgbe_start_hw_X540, \
@@ -1334,6 +1356,8 @@ mac_reset_top:
.init_uta_tables = &ixgbe_init_uta_tables_generic, \
.set_mac_anti_spoofing = &ixgbe_set_mac_anti_spoofing, \
.set_vlan_anti_spoofing = &ixgbe_set_vlan_anti_spoofing, \
+ .set_ethertype_anti_spoofing = \
+ &ixgbe_set_ethertype_anti_spoofing_X550, \
.acquire_swfw_sync = &ixgbe_acquire_swfw_sync_X540, \
.release_swfw_sync = &ixgbe_release_swfw_sync_X540, \
.disable_rx_buff = &ixgbe_disable_rx_buff_generic, \
--
1.9.3
^ permalink raw reply related
* Re: [PATCH] af_packet: don't pass empty blocks for PACKET_V3
From: Alexander Drozdov @ 2015-02-06 4:49 UTC (permalink / raw)
To: Guy Harris, Willem de Bruijn
Cc: David S. Miller, Daniel Borkmann, Eric Dumazet, Al Viro,
Michael S. Tsirkin, Network Development, linux-kernel,
Dan Collins
In-Reply-To: <474D8071-4AEC-48F4-B0DC-02D9091CF3DB@alum.mit.edu>
On 06.02.2015 00:16:30 +0300 Guy Harris <guy@alum.mit.edu> wrote:
> On Feb 5, 2015, at 12:01 PM, Willem de Bruijn <willemb@google.com> wrote:
>
>> On Wed, Feb 4, 2015 at 9:58 PM, Alexander Drozdov <al.drozdov@gmail.com> wrote:
>>> Don't close an empty block on timeout. Its meaningless to
>>> pass it to the user. Moreover, passing empty blocks wastes
>>> CPU & buffer space increasing probability of packets
>>> dropping on small timeouts.
>>>
>>> Side effect of this patch is indefinite user-space wait
>>> in poll on idle links. But, I believe its better to set
>>> timeout for poll(2) when needed than to get empty blocks
>>> every millisecond when not needed.
>> This change would break existing applications that have come
>> to depend on the periodic signal.
>>
>> I don't disagree with the argument that the data ready signal
>> should be sent only when a block is full or a timer expires and
>> at least some data is waiting, but that is moot at this point.
> For what it's worth, the BPF packet capture mechanism (which really needs a new name, to distinguish itself from the BPF packet filter language and its implementation(s), but I digress) has the same issue - when the timer expires, a wakeup is delivered even if there are no packets to read.
>
> *However*, if there are no packets available, the buffers aren't rotated, so the empty buffer is left around to be filled up with packets, rather than being made the hold buffer.
>
> Given that before the previous TPACKET_V3 change, wakeups were delivered when packets arrived rather than when a block was closed, presumably code using TPACKET_V3 was capable of dealing with wakeups being delivered when no new blocks had been made available to userland; could TPACKET_V3 work a bit more like BPF and deliver a wakeup when the timer expires *without* closing the empty block?
Thank you all for your comments! I'll try to create two patches:
1. Wakeup by timeout without closing the empty block
2. Allow to not wakeup by timeout (the feature should be explicitly requested by a user)
^ permalink raw reply
* [PATCH 0/5] virtio 1.0 cleanups and one fix.
From: Rusty Russell @ 2015-02-06 5:06 UTC (permalink / raw)
To: lkml; +Cc: Rusty Russell, Michael S. Tsirkin, netdev@vger.kernel.org
Hi all,
Some minor fixes for my virtio-next tree. Michael, does
QEMU implement the (compuslory!) VIRTIO_PCI_CAP_PCI_CFG field? I'm
guessing not, since it wasn't defined in the Linux header :(
Rusty Russell (5):
virtio: define VIRTIO_PCI_CAP_PCI_CFG in header.
virtio: Don't expose legacy block features when VIRTIO_BLK_NO_LEGACY
defined.
virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY
defined.
virtio: Don't expose legacy config features when
VIRTIO_CONFIG_NO_LEGACY defined.
virtio: don't require a config space on the console device.
drivers/char/virtio_console.c | 12 ++++++++----
include/uapi/linux/virtio_blk.h | 17 +++++++++++++----
include/uapi/linux/virtio_config.h | 2 ++
include/uapi/linux/virtio_net.h | 30 ++++++++++++++++++++++++++++--
include/uapi/linux/virtio_pci.h | 4 +++-
5 files changed, 54 insertions(+), 11 deletions(-)
--
2.1.0
^ permalink raw reply
* [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.
From: Rusty Russell @ 2015-02-06 5:06 UTC (permalink / raw)
To: lkml; +Cc: Michael S. Tsirkin, netdev@vger.kernel.org
In-Reply-To: <1423199216-2094-1-git-send-email-rusty@rustcorp.com.au>
In particular, the virtio header always has the u16 num_buffers field.
We define a new 'struct virtio_net_modern_hdr' for this (rather than
simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
include/uapi/linux/virtio_net.h | 30 ++++++++++++++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index b5f1677b291c..32754f3000e8 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -35,7 +35,6 @@
#define VIRTIO_NET_F_CSUM 0 /* Host handles pkts w/ partial csum */
#define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */
#define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */
-#define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
#define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */
#define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */
#define VIRTIO_NET_F_GUEST_ECN 9 /* Guest can handle TSO[6] w/ ECN in. */
@@ -56,6 +55,10 @@
* Steering */
#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
+#ifndef VIRTIO_NET_NO_LEGACY
+#define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
+#endif /* VIRTIO_NET_NO_LEGACY */
+
#define VIRTIO_NET_S_LINK_UP 1 /* Link is up */
#define VIRTIO_NET_S_ANNOUNCE 2 /* Announcement is needed */
@@ -71,8 +74,9 @@ struct virtio_net_config {
__u16 max_virtqueue_pairs;
} __attribute__((packed));
+#ifndef VIRTIO_NET_NO_LEGACY
/* This header comes first in the scatter-gather list.
- * If VIRTIO_F_ANY_LAYOUT is not negotiated, it must
+ * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
* be the first element of the scatter-gather list. If you don't
* specify GSO or CSUM features, you can simply ignore the header. */
struct virtio_net_hdr {
@@ -97,6 +101,28 @@ struct virtio_net_hdr_mrg_rxbuf {
struct virtio_net_hdr hdr;
__virtio16 num_buffers; /* Number of merged rx buffers */
};
+#else /* ... VIRTIO_NET_NO_LEGACY */
+/*
+ * This header comes first in the scatter-gather list. If you don't
+ * specify GSO or CSUM features, you can simply ignore the header.
+ */
+struct virtio_net_modern_hdr {
+#define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */
+#define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */
+ __u8 flags;
+#define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */
+#define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */
+#define VIRTIO_NET_HDR_GSO_UDP 3 /* GSO frame, IPv4 UDP (UFO) */
+#define VIRTIO_NET_HDR_GSO_TCPV6 4 /* GSO frame, IPv6 TCP */
+#define VIRTIO_NET_HDR_GSO_ECN 0x80 /* TCP has ECN set */
+ __u8 gso_type;
+ __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
+ __virtio16 gso_size; /* Bytes to append to hdr_len per frame */
+ __virtio16 csum_start; /* Position to start checksumming from */
+ __virtio16 csum_offset; /* Offset after that to place checksum */
+ __virtio16 num_buffers; /* Number of merged rx buffers */
+};
+#endif /* ...VIRTIO_NET_NO_LEGACY */
/*
* Control virtqueue data structures
--
2.1.0
^ permalink raw reply related
* Re: [Patch net-next] ipv6: fix redefinition of in6_pktinfo and ip6_mtuinfo
From: Carlos O'Donell @ 2015-02-06 5:29 UTC (permalink / raw)
To: David Miller, xiyou.wangcong; +Cc: netdev, vlee
In-Reply-To: <20150108.193819.258816805892789860.davem@davemloft.net>
On 01/08/2015 10:38 PM, David Miller wrote:
> From: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Tue, 6 Jan 2015 15:45:31 -0800
>
>> Both netinet/in.h and linux/ipv6.h define these two structs,
>> if we include both of them, we got:
>>
>> /usr/include/linux/ipv6.h:19:8: error: redefinition of ‘struct in6_pktinfo’
>> struct in6_pktinfo {
>> ^
>> In file included from /usr/include/arpa/inet.h:22:0,
>> from txtimestamp.c:33:
>> /usr/include/netinet/in.h:524:8: note: originally defined here
>> struct in6_pktinfo
>> ^
>> In file included from txtimestamp.c:40:0:
>> /usr/include/linux/ipv6.h:24:8: error: redefinition of ‘struct ip6_mtuinfo’
>> struct ip6_mtuinfo {
>> ^
>> In file included from /usr/include/arpa/inet.h:22:0,
>> from txtimestamp.c:33:
>> /usr/include/netinet/in.h:531:8: note: originally defined here
>> struct ip6_mtuinfo
>> ^
>> So similarly to what we did for in6_addr, we need to sync with
>> libc header on their definitions.
>>
>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>
> Applied.
>
When does this hit mainline? I've got Cong's patch ready for glibc, but
the usual procedure is to commit after mainline has the matching patch.
Not that it really matters, but it's just a useful thing to be able to
say the trickle down is linux->glibc.
Cheers,
Carlos.
^ permalink raw reply
* Re: [RFC PATCH 19/29] net: vrf: Add vrf context to skb
From: David Ahern @ 2015-02-06 6:00 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: netdev
In-Reply-To: <878ugbwv7t.fsf@x220.int.ebiederm.org>
On 2/5/15 8:54 PM, Eric W. Biederman wrote:
> David Ahern<dsahern@gmail.com> writes:
>
>> >On ingress skb's inherit vrf context from the net_device. For TX skb's
>> >inherit the vrf context from the socket originating the packet. Update
>> >SKB related net_ctx macros to set vrf.
> Nacked-by: "Eric W. Biederman"<ebiederm@xmission.com>
>
> As I recall we had the conversation about modifying struct sk_buff when
> implementing network namespaces and it turns out it is a no go. It adds
> too much overhead. As I recall skbs as they are on the fastpath are
> things we want to make smaller.
>
Ok, I will look into removing the need for this change.
David
^ permalink raw reply
* Re: [RFC PATCH 00/29] net: VRF support
From: Shmulik Ladkani @ 2015-02-06 6:10 UTC (permalink / raw)
To: roopa
Cc: David Ahern, netdev, ebiederm, Dinesh Dutt, Vipin Kumar,
Nicolas Dichtel, hannes, Eyal Birger
In-Reply-To: <54D3F8F9.2060500@cumulusnetworks.com>
On Thu, 05 Feb 2015 15:12:57 -0800 roopa <roopa@cumulusnetworks.com> wrote:
> We have been playing with ip rules to implement vrfs. And the blocker
> today is that we cannot bind a socket to a vrf (routing tables in this
> case).
Hi Roopa,
One option would be using SO_MARK sockopt on that socket, and have an ip
rule which matches this mark to point to your table.
I don't know your exact use-cases, but you can play around with that
idea.
Regards,
Shmulik
^ permalink raw reply
* Re: [RFC PATCH 00/29] net: VRF support
From: David Ahern @ 2015-02-06 6:15 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: Stephen Hemminger, netdev
In-Reply-To: <87r3u3vfpr.fsf@x220.int.ebiederm.org>
On 2/5/15 9:14 PM, Eric W. Biederman wrote:
> David Ahern <dsahern@gmail.com> writes:
>
>> On 2/5/15 6:33 PM, Stephen Hemminger wrote:
>>> It is still not clear how adding another level of abstraction
>>> solves the scaling problem. Is it just because you can have one application
>>> connect to multiple VRF's? so you don't need N routing daemons?
>>>
>>>
>>
>> How do you provide a service in N VRFs? "Service" can be a daemon with a listen
>> socket (e.g., bgpd, sshd) or a monitoring app (e.g., collectd, snmpd). For the
>> current namespace only paradigm the options are:
>> 1. replicate the process for each namespace (e.g., N instances of sshd, bgpd,
>> collectd, snmpd, etc.)
>>
>> 2. a single process spawns a thread for each namespace
>>
>> 3. a single process opens a socket in each namespace
>>
>> All of those options are rather heavyweight and the number of 'things' is linear
>> with the number of VRFs. When multiplied by the number of services needed for a
>> full-featured product the end result is a lot of wasted resources.
>
> If all you want is a single listening socket there are other
> implementation possibilities that are focused on solving just that
> problem, and would be much more generally applicable.
These are examples of the higher level problem -- the current need for
replicating processes/threads/sockets per namespace, not to mention the
memory consumed by the creation of the namespace itself which is fairly
high. i.e., The problem is more than just a listening socket of a single
process.
>
>> The idea here is to simplify things by allowing a single process to have a
>> presence / provide a service in all VRFs within a namespace without the need to
>> spawn a thread, socket or another process.
>>
>> For example, 1 instance of a monitoring app can still see all of the netdevices
>> in the namespace and in the VRF_ANY context can still report network
>> configuration data. VRF unaware/agnostic L3/L4 apps (e.g., sshd) do not need to
>> be modified and will be able to provide service through any
>> interface.
>
> *Blink* sshd does not need to be modified????
> Which insecure implementation on which planet?
That would be the current one -- in both cases. It is an example, Eric,
(admittedly not a good one) that existing code does not *have* to be
modified to run in a 'VRF any' context. It can be made VRF aware of course.
>
> You mean you are not interested in logging which ip and vrf pair a login
> came from? You are not interested in performing any reverse DNS
> lookups?
>
> I do believe you are strongly mistaken. I can not imagine a case where
> making it impossible to know where someone is coming from when they try
> to login to any machine is at all desirable.
Aren't you conflating two problems? Network namespaces does not require
a separate DNS config for each namespace. A user may create 2+ network
namespaces and have them share the same /etc/resolv.conf. Correct?
>
> I think it is unrealistic to expect daemons in general to listen on all
> interfaces and in all vrfs, and require trimming down the set of
> interfaces inbound connections can come from with firewall rules. That
> just seems backwards. Telling the daemons which interfaces/address to
> listen on explicitly seems much more robust.
Networking products with 1000+ interfaces? Physical, sub-interfaces,
breakout ports, VLANs, SVIs, port channels, ...
>
> The objection about logging in-bound connections applies to every
> listening daemon I can think of. I can't see how you can possibly
> seriously be proposing totally changing the networking environment of
> applications and expecting those applications to work with out changes.
Nothing stops me from having xinetd launch /bin/bash as root for all
connections to 666/tcp. Nothing about the Linux networking stack
prevents someone from running telnet or ftp. ie., the existing code base
can be used in insecure ways.
Application code -- open source daemons -- can be modified to be VRF
aware as needed. Kernel side VRF support would be made a CONFIG option
that defaults off. The macros will ensure anything VRF related falls
out, so server deployments would not be impacted.
>
>
> I do think we can come up with an API that is much less awkward than we
> have today, that would allow minimal application changes, but no
> application changes I do not believe.
Can we agree that no L2 apps should require a single line of code to be
changed? If I create 4000 VRFs -- again an L3 construct -- not one L2
application (socket based, monitoring, etc) should care. It should not
have to be replicated or modified. For L3 and up they can be made VRF
aware as needed, but that is an application problem.
>
>> VRF aware
>> apps (e.g., bgpd) might require modifications per the implementation of the VRF
>> construct but they would able to provide service with a single
>> instance.
>
> A single service instance is all that is required with network
> namespaces.
N VRFs = N namespaces = N instances of every single process, where N is
1024, 2048, 4096, and up. Someone has already done the analysis for
quagga with 1024 instances showing what a huge waste of memory that is.
>
> I do not see how code modifications that result in a slower network
> stack can possibly solve any kind of scaling problem.
I'll see what I can do to remove the skb change. That is the only
comment you have made about performance. Do you have other concerns
about performance impacts of the higher level proposal -- s/struct
net/struct net_ctx/ where net_ctx is a namespace and a VRF?
David
^ permalink raw reply
* [PATCH net-next] bridge: add missing bridge port check for offloads
From: roopa @ 2015-02-06 6:24 UTC (permalink / raw)
To: bridge, dan.carpenter, stephen, netdev; +Cc: davem
From: Roopa Prabhu <roopa@cumulusnetworks.com>
This patch fixes a missing bridge port check caught by smatch.
setlink/dellink of attributes like vlans can come for a bridge device
and there is no need to offload those today. So, this patch adds a bridge
port check. (In these cases however, the BRIDGE_SELF flags will always be set
and we may not hit a problem with the current code).
smatch complaint:
The patch 68e331c785b8: "bridge: offload bridge port attributes to
switch asic if feature flag set" from Jan 29, 2015, leads to the
following Smatch complaint:
net/bridge/br_netlink.c:552 br_setlink()
error: we previously assumed 'p' could be null (see line 518)
net/bridge/br_netlink.c
517
518 if (p && protinfo) {
^
Check for NULL.
Reported-By: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
net/bridge/br_netlink.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index b93f42c..4fbcea0 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -543,7 +543,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
afspec, RTM_SETLINK);
}
- if (!(flags & BRIDGE_FLAGS_SELF)) {
+ if (p && !(flags & BRIDGE_FLAGS_SELF)) {
/* set bridge attributes in hardware if supported
*/
ret_offload = netdev_switch_port_bridge_setlink(dev, nlh,
@@ -583,7 +583,7 @@ int br_dellink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
*/
br_ifinfo_notify(RTM_NEWLINK, p);
- if (!(flags & BRIDGE_FLAGS_SELF)) {
+ if (p && !(flags & BRIDGE_FLAGS_SELF)) {
/* del bridge attributes in hardware
*/
ret_offload = netdev_switch_port_bridge_dellink(dev, nlh,
--
1.7.10.4
^ permalink raw reply related
* [PATCH V6 1/3] bfin_can: rewrite the blackfin style of read/write to common ones
From: Aaron Wu @ 2015-02-06 6:25 UTC (permalink / raw)
To: wg, mkl, linux-can, netdev, linux-kernel, linux-devel; +Cc: Aaron Wu
Replace the blackfin arch dependent style of bfin_read/bfin_write with
common readw/writew
Signed-off-by: Aaron Wu <Aaron.wu@analog.com>
---
drivers/net/can/bfin_can.c | 126 +++++++++++++++++++++-----------------------
1 file changed, 60 insertions(+), 66 deletions(-)
diff --git a/drivers/net/can/bfin_can.c b/drivers/net/can/bfin_can.c
index 417d5099..0a5eff4 100644
--- a/drivers/net/can/bfin_can.c
+++ b/drivers/net/can/bfin_can.c
@@ -78,8 +78,8 @@ static int bfin_can_set_bittiming(struct net_device *dev)
if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
timing |= SAM;
- bfin_write(®->clock, clk);
- bfin_write(®->timing, timing);
+ writew(clk, ®->clock);
+ writew(timing, ®->timing);
netdev_info(dev, "setting CLOCK=0x%04x TIMING=0x%04x\n", clk, timing);
@@ -94,16 +94,14 @@ static void bfin_can_set_reset_mode(struct net_device *dev)
int i;
/* disable interrupts */
- bfin_write(®->mbim1, 0);
- bfin_write(®->mbim2, 0);
- bfin_write(®->gim, 0);
+ writew(0, ®->mbim1);
+ writew(0, ®->mbim2);
+ writew(0, ®->gim);
/* reset can and enter configuration mode */
- bfin_write(®->control, SRS | CCR);
- SSYNC();
- bfin_write(®->control, CCR);
- SSYNC();
- while (!(bfin_read(®->control) & CCA)) {
+ writew(SRS | CCR, ®->control);
+ writew(CCR, ®->control);
+ while (!(readw(®->control) & CCA)) {
udelay(10);
if (--timeout == 0) {
netdev_err(dev, "fail to enter configuration mode\n");
@@ -116,34 +114,33 @@ static void bfin_can_set_reset_mode(struct net_device *dev)
* by writing to CAN Mailbox Configuration Registers 1 and 2
* For all bits: 0 - Mailbox disabled, 1 - Mailbox enabled
*/
- bfin_write(®->mc1, 0);
- bfin_write(®->mc2, 0);
+ writew(0, ®->mc1);
+ writew(0, ®->mc2);
/* Set Mailbox Direction */
- bfin_write(®->md1, 0xFFFF); /* mailbox 1-16 are RX */
- bfin_write(®->md2, 0); /* mailbox 17-32 are TX */
+ writew(0xFFFF, ®->md1); /* mailbox 1-16 are RX */
+ writew(0, ®->md2); /* mailbox 17-32 are TX */
/* RECEIVE_STD_CHL */
for (i = 0; i < 2; i++) {
- bfin_write(®->chl[RECEIVE_STD_CHL + i].id0, 0);
- bfin_write(®->chl[RECEIVE_STD_CHL + i].id1, AME);
- bfin_write(®->chl[RECEIVE_STD_CHL + i].dlc, 0);
- bfin_write(®->msk[RECEIVE_STD_CHL + i].amh, 0x1FFF);
- bfin_write(®->msk[RECEIVE_STD_CHL + i].aml, 0xFFFF);
+ writew(0, ®->chl[RECEIVE_STD_CHL + i].id0);
+ writew(AME, ®->chl[RECEIVE_STD_CHL + i].id1);
+ writew(0, ®->chl[RECEIVE_STD_CHL + i].dlc);
+ writew(0x1FFF, ®->msk[RECEIVE_STD_CHL + i].amh);
+ writew(0xFFFF, ®->msk[RECEIVE_STD_CHL + i].aml);
}
/* RECEIVE_EXT_CHL */
for (i = 0; i < 2; i++) {
- bfin_write(®->chl[RECEIVE_EXT_CHL + i].id0, 0);
- bfin_write(®->chl[RECEIVE_EXT_CHL + i].id1, AME | IDE);
- bfin_write(®->chl[RECEIVE_EXT_CHL + i].dlc, 0);
- bfin_write(®->msk[RECEIVE_EXT_CHL + i].amh, 0x1FFF);
- bfin_write(®->msk[RECEIVE_EXT_CHL + i].aml, 0xFFFF);
+ writew(0, ®->chl[RECEIVE_EXT_CHL + i].id0);
+ writew(AME | IDE, ®->chl[RECEIVE_EXT_CHL + i].id1);
+ writew(0, ®->chl[RECEIVE_EXT_CHL + i].dlc);
+ writew(0x1FFF, ®->msk[RECEIVE_EXT_CHL + i].amh);
+ writew(0xFFFF, ®->msk[RECEIVE_EXT_CHL + i].aml);
}
- bfin_write(®->mc2, BIT(TRANSMIT_CHL - 16));
- bfin_write(®->mc1, BIT(RECEIVE_STD_CHL) + BIT(RECEIVE_EXT_CHL));
- SSYNC();
+ writew(BIT(TRANSMIT_CHL - 16), ®->mc2);
+ writew(BIT(RECEIVE_STD_CHL) + BIT(RECEIVE_EXT_CHL), ®->mc1);
priv->can.state = CAN_STATE_STOPPED;
}
@@ -157,9 +154,9 @@ static void bfin_can_set_normal_mode(struct net_device *dev)
/*
* leave configuration mode
*/
- bfin_write(®->control, bfin_read(®->control) & ~CCR);
+ writew(readw(®->control) & ~CCR, ®->control);
- while (bfin_read(®->status) & CCA) {
+ while (readw(®->status) & CCA) {
udelay(10);
if (--timeout == 0) {
netdev_err(dev, "fail to leave configuration mode\n");
@@ -170,26 +167,25 @@ static void bfin_can_set_normal_mode(struct net_device *dev)
/*
* clear _All_ tx and rx interrupts
*/
- bfin_write(®->mbtif1, 0xFFFF);
- bfin_write(®->mbtif2, 0xFFFF);
- bfin_write(®->mbrif1, 0xFFFF);
- bfin_write(®->mbrif2, 0xFFFF);
+ writew(0xFFFF, ®->mbtif1);
+ writew(0xFFFF, ®->mbtif2);
+ writew(0xFFFF, ®->mbrif1);
+ writew(0xFFFF, ®->mbrif2);
/*
* clear global interrupt status register
*/
- bfin_write(®->gis, 0x7FF); /* overwrites with '1' */
+ writew(0x7FF, ®->gis); /* overwrites with '1' */
/*
* Initialize Interrupts
* - set bits in the mailbox interrupt mask register
* - global interrupt mask
*/
- bfin_write(®->mbim1, BIT(RECEIVE_STD_CHL) + BIT(RECEIVE_EXT_CHL));
- bfin_write(®->mbim2, BIT(TRANSMIT_CHL - 16));
+ writew(BIT(RECEIVE_STD_CHL) + BIT(RECEIVE_EXT_CHL), ®->mbim1);
+ writew(BIT(TRANSMIT_CHL - 16), ®->mbim2);
- bfin_write(®->gim, EPIM | BOIM | RMLIM);
- SSYNC();
+ writew(EPIM | BOIM | RMLIM, ®->gim);
}
static void bfin_can_start(struct net_device *dev)
@@ -226,7 +222,7 @@ static int bfin_can_get_berr_counter(const struct net_device *dev,
struct bfin_can_priv *priv = netdev_priv(dev);
struct bfin_can_regs __iomem *reg = priv->membase;
- u16 cec = bfin_read(®->cec);
+ u16 cec = readw(®->cec);
bec->txerr = cec >> 8;
bec->rxerr = cec;
@@ -252,28 +248,28 @@ static int bfin_can_start_xmit(struct sk_buff *skb, struct net_device *dev)
/* fill id */
if (id & CAN_EFF_FLAG) {
- bfin_write(®->chl[TRANSMIT_CHL].id0, id);
+ writew(id, ®->chl[TRANSMIT_CHL].id0);
val = ((id & 0x1FFF0000) >> 16) | IDE;
} else
val = (id << 2);
if (id & CAN_RTR_FLAG)
val |= RTR;
- bfin_write(®->chl[TRANSMIT_CHL].id1, val | AME);
+ writew(val | AME, ®->chl[TRANSMIT_CHL].id1);
/* fill payload */
for (i = 0; i < 8; i += 2) {
val = ((7 - i) < dlc ? (data[7 - i]) : 0) +
((6 - i) < dlc ? (data[6 - i] << 8) : 0);
- bfin_write(®->chl[TRANSMIT_CHL].data[i], val);
+ writew(val, ®->chl[TRANSMIT_CHL].data[i]);
}
/* fill data length code */
- bfin_write(®->chl[TRANSMIT_CHL].dlc, dlc);
+ writew(dlc, ®->chl[TRANSMIT_CHL].dlc);
can_put_echo_skb(skb, dev, 0);
/* set transmit request */
- bfin_write(®->trs2, BIT(TRANSMIT_CHL - 16));
+ writew(BIT(TRANSMIT_CHL - 16), ®->trs2);
return 0;
}
@@ -296,26 +292,26 @@ static void bfin_can_rx(struct net_device *dev, u16 isrc)
/* get id */
if (isrc & BIT(RECEIVE_EXT_CHL)) {
/* extended frame format (EFF) */
- cf->can_id = ((bfin_read(®->chl[RECEIVE_EXT_CHL].id1)
+ cf->can_id = ((readw(®->chl[RECEIVE_EXT_CHL].id1)
& 0x1FFF) << 16)
- + bfin_read(®->chl[RECEIVE_EXT_CHL].id0);
+ + readw(®->chl[RECEIVE_EXT_CHL].id0);
cf->can_id |= CAN_EFF_FLAG;
obj = RECEIVE_EXT_CHL;
} else {
/* standard frame format (SFF) */
- cf->can_id = (bfin_read(®->chl[RECEIVE_STD_CHL].id1)
+ cf->can_id = (readw(®->chl[RECEIVE_STD_CHL].id1)
& 0x1ffc) >> 2;
obj = RECEIVE_STD_CHL;
}
- if (bfin_read(®->chl[obj].id1) & RTR)
+ if (readw(®->chl[obj].id1) & RTR)
cf->can_id |= CAN_RTR_FLAG;
/* get data length code */
- cf->can_dlc = get_can_dlc(bfin_read(®->chl[obj].dlc) & 0xF);
+ cf->can_dlc = get_can_dlc(readw(®->chl[obj].dlc) & 0xF);
/* get payload */
for (i = 0; i < 8; i += 2) {
- val = bfin_read(®->chl[obj].data[i]);
+ val = readw(®->chl[obj].data[i]);
cf->data[7 - i] = (7 - i) < cf->can_dlc ? val : 0;
cf->data[6 - i] = (6 - i) < cf->can_dlc ? (val >> 8) : 0;
}
@@ -368,7 +364,7 @@ static int bfin_can_err(struct net_device *dev, u16 isrc, u16 status)
if (state != priv->can.state && (state == CAN_STATE_ERROR_WARNING ||
state == CAN_STATE_ERROR_PASSIVE)) {
- u16 cec = bfin_read(®->cec);
+ u16 cec = readw(®->cec);
u8 rxerr = cec;
u8 txerr = cec >> 8;
@@ -419,23 +415,23 @@ static irqreturn_t bfin_can_interrupt(int irq, void *dev_id)
struct net_device_stats *stats = &dev->stats;
u16 status, isrc;
- if ((irq == priv->tx_irq) && bfin_read(®->mbtif2)) {
+ if ((irq == priv->tx_irq) && readw(®->mbtif2)) {
/* transmission complete interrupt */
- bfin_write(®->mbtif2, 0xFFFF);
+ writew(0xFFFF, ®->mbtif2);
stats->tx_packets++;
- stats->tx_bytes += bfin_read(®->chl[TRANSMIT_CHL].dlc);
+ stats->tx_bytes += readw(®->chl[TRANSMIT_CHL].dlc);
can_get_echo_skb(dev, 0);
netif_wake_queue(dev);
- } else if ((irq == priv->rx_irq) && bfin_read(®->mbrif1)) {
+ } else if ((irq == priv->rx_irq) && readw(®->mbrif1)) {
/* receive interrupt */
- isrc = bfin_read(®->mbrif1);
- bfin_write(®->mbrif1, 0xFFFF);
+ isrc = readw(®->mbrif1);
+ writew(0xFFFF, ®->mbrif1);
bfin_can_rx(dev, isrc);
- } else if ((irq == priv->err_irq) && bfin_read(®->gis)) {
+ } else if ((irq == priv->err_irq) && readw(®->gis)) {
/* error interrupt */
- isrc = bfin_read(®->gis);
- status = bfin_read(®->esr);
- bfin_write(®->gis, 0x7FF);
+ isrc = readw(®->gis);
+ status = readw(®->esr);
+ writew(0x7FF, ®->gis);
bfin_can_err(dev, isrc, status);
} else {
return IRQ_NONE;
@@ -640,9 +636,8 @@ static int bfin_can_suspend(struct platform_device *pdev, pm_message_t mesg)
if (netif_running(dev)) {
/* enter sleep mode */
- bfin_write(®->control, bfin_read(®->control) | SMR);
- SSYNC();
- while (!(bfin_read(®->intr) & SMACK)) {
+ writew(readw(®->control) | SMR, ®->control);
+ while (!(readw(®->intr) & SMACK)) {
udelay(10);
if (--timeout == 0) {
netdev_err(dev, "fail to enter sleep mode\n");
@@ -662,8 +657,7 @@ static int bfin_can_resume(struct platform_device *pdev)
if (netif_running(dev)) {
/* leave sleep mode */
- bfin_write(®->intr, 0);
- SSYNC();
+ writew(0, ®->intr);
}
return 0;
--
1.7.9.5
^ permalink raw reply related
* [PATCH V6 2/3] bfin_can: introduce ioremap to comply to the arch with MMU
From: Aaron Wu @ 2015-02-06 6:25 UTC (permalink / raw)
To: wg, mkl, linux-can, netdev, linux-kernel, linux-devel; +Cc: Aaron Wu
In-Reply-To: <1423203952-23004-1-git-send-email-Aaron.wu@analog.com>
Blackfin was built without MMU, old driver code access the IO space by
physical address, introduce the ioremap approach to be compitable with
the common style supporting MMU enabled arch.
Signed-off-by: Aaron Wu <Aaron.wu@analog.com>
---
drivers/net/can/bfin_can.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/net/can/bfin_can.c b/drivers/net/can/bfin_can.c
index 0a5eff4..259d09a 100644
--- a/drivers/net/can/bfin_can.c
+++ b/drivers/net/can/bfin_can.c
@@ -551,16 +551,10 @@ static int bfin_can_probe(struct platform_device *pdev)
goto exit;
}
- if (!request_mem_region(res_mem->start, resource_size(res_mem),
- dev_name(&pdev->dev))) {
- err = -EBUSY;
- goto exit;
- }
-
/* request peripheral pins */
err = peripheral_request_list(pdata, dev_name(&pdev->dev));
if (err)
- goto exit_mem_release;
+ goto exit;
dev = alloc_bfin_candev();
if (!dev) {
@@ -569,7 +563,13 @@ static int bfin_can_probe(struct platform_device *pdev)
}
priv = netdev_priv(dev);
- priv->membase = (void __iomem *)res_mem->start;
+
+ priv->membase = devm_ioremap_resource(&pdev->dev, res_mem);
+ if (IS_ERR(priv->membase)) {
+ err = PTR_ERR(priv->membase);
+ goto exit_peri_pin_free;
+ }
+
priv->rx_irq = rx_irq->start;
priv->tx_irq = tx_irq->start;
priv->err_irq = err_irq->start;
@@ -601,8 +601,6 @@ exit_candev_free:
free_candev(dev);
exit_peri_pin_free:
peripheral_free_list(pdata);
-exit_mem_release:
- release_mem_region(res_mem->start, resource_size(res_mem));
exit:
return err;
}
@@ -611,15 +609,11 @@ static int bfin_can_remove(struct platform_device *pdev)
{
struct net_device *dev = platform_get_drvdata(pdev);
struct bfin_can_priv *priv = netdev_priv(dev);
- struct resource *res;
bfin_can_set_reset_mode(dev);
unregister_candev(dev);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- release_mem_region(res->start, resource_size(res));
-
peripheral_free_list(priv->pin_list);
free_candev(dev);
--
1.7.9.5
^ permalink raw reply related
* [PATCH V6 3/3] bfin_can: move and merge the content of header file into driver body
From: Aaron Wu @ 2015-02-06 6:25 UTC (permalink / raw)
To: wg, mkl, linux-can, netdev, linux-kernel, linux-devel; +Cc: Aaron Wu
In-Reply-To: <1423203952-23004-1-git-send-email-Aaron.wu@analog.com>
Header file was in arch dependent location arch/blackfin/include/asm/bfin_can.h,
Now move and merge the useful contents of header file into driver code, note
the original header file is reserved for full register sets access test by other
code so it survives.
Signed-off-by: Aaron Wu <Aaron.wu@analog.com>
---
drivers/net/can/bfin_can.c | 114 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 113 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/bfin_can.c b/drivers/net/can/bfin_can.c
index 259d09a..63fab5c 100644
--- a/drivers/net/can/bfin_can.c
+++ b/drivers/net/can/bfin_can.c
@@ -20,13 +20,125 @@
#include <linux/can/dev.h>
#include <linux/can/error.h>
-#include <asm/bfin_can.h>
#include <asm/portmux.h>
#define DRV_NAME "bfin_can"
#define BFIN_CAN_TIMEOUT 100
#define TX_ECHO_SKB_MAX 1
+/* transmit and receive channels */
+
+#define TRANSMIT_CHL 24
+#define RECEIVE_STD_CHL 0
+#define RECEIVE_EXT_CHL 4
+#define RECEIVE_RTR_CHL 8
+#define RECEIVE_EXT_RTR_CHL 12
+#define MAX_CHL_NUMBER 32
+
+/* All Blackfin system MMRs are padded to 32bits even if the register
+ * itself is only 16bits. So use a helper macro to streamline this
+*/
+#define __BFP(m) u16 m; u16 __pad_##m
+
+ /* bfin can registers layout
+ */
+struct bfin_can_mask_regs {
+ __BFP(aml);
+ __BFP(amh);
+};
+
+struct bfin_can_channel_regs {
+ /* data[0,2,4,6] -> data{0,1,2,3} while data[1,3,5,7] is padding */
+ u16 data[8];
+ __BFP(dlc);
+ __BFP(tsv);
+ __BFP(id0);
+ __BFP(id1);
+};
+
+struct bfin_can_regs {
+ /* global control and status registers
+ */
+ __BFP(mc1); /* offset 0x00 */
+ __BFP(md1); /* offset 0x04 */
+ __BFP(trs1); /* offset 0x08 */
+ __BFP(trr1); /* offset 0x0c */
+ __BFP(ta1); /* offset 0x10 */
+ __BFP(aa1); /* offset 0x14 */
+ __BFP(rmp1); /* offset 0x18 */
+ __BFP(rml1); /* offset 0x1c */
+ __BFP(mbtif1); /* offset 0x20 */
+ __BFP(mbrif1); /* offset 0x24 */
+ __BFP(mbim1); /* offset 0x28 */
+ __BFP(rfh1); /* offset 0x2c */
+ __BFP(opss1); /* offset 0x30 */
+ u32 __pad1[3];
+ __BFP(mc2); /* offset 0x40 */
+ __BFP(md2); /* offset 0x44 */
+ __BFP(trs2); /* offset 0x48 */
+ __BFP(trr2); /* offset 0x4c */
+ __BFP(ta2); /* offset 0x50 */
+ __BFP(aa2); /* offset 0x54 */
+ __BFP(rmp2); /* offset 0x58 */
+ __BFP(rml2); /* offset 0x5c */
+ __BFP(mbtif2); /* offset 0x60 */
+ __BFP(mbrif2); /* offset 0x64 */
+ __BFP(mbim2); /* offset 0x68 */
+ __BFP(rfh2); /* offset 0x6c */
+ __BFP(opss2); /* offset 0x70 */
+ u32 __pad2[3];
+ __BFP(clock); /* offset 0x80 */
+ __BFP(timing); /* offset 0x84 */
+ __BFP(debug); /* offset 0x88 */
+ __BFP(status); /* offset 0x8c */
+ __BFP(cec); /* offset 0x90 */
+ __BFP(gis); /* offset 0x94 */
+ __BFP(gim); /* offset 0x98 */
+ __BFP(gif); /* offset 0x9c */
+ __BFP(control); /* offset 0xa0 */
+ __BFP(intr); /* offset 0xa4 */
+ __BFP(version); /* offset 0xa8 */
+ __BFP(mbtd); /* offset 0xac */
+ __BFP(ewr); /* offset 0xb0 */
+ __BFP(esr); /* offset 0xb4 */
+ u32 __pad3[2];
+ __BFP(ucreg); /* offset 0xc0 */
+ __BFP(uccnt); /* offset 0xc4 */
+ __BFP(ucrc); /* offset 0xc8 */
+ __BFP(uccnf); /* offset 0xcc */
+ u32 __pad4[1];
+ __BFP(version2); /* offset 0xd4 */
+ u32 __pad5[10];
+
+ /* channel(mailbox) mask and message registers
+ */
+ struct bfin_can_mask_regs msk[MAX_CHL_NUMBER]; /* offset 0x100 */
+ struct bfin_can_channel_regs chl[MAX_CHL_NUMBER]; /* offset 0x200 */
+};
+
+#undef __BFP
+
+#define SRS 0x0001 /* Software Reset */
+#define SER 0x0008 /* Stuff Error */
+#define BOIM 0x0008 /* Enable Bus Off Interrupt */
+#define CCR 0x0080 /* CAN Configuration Mode Request */
+#define CCA 0x0080 /* Configuration Mode Acknowledge */
+#define SAM 0x0080 /* Sampling */
+#define AME 0x8000 /* Acceptance Mask Enable */
+#define RMLIM 0x0080 /* Enable RX Message Lost Interrupt */
+#define RMLIS 0x0080 /* RX Message Lost IRQ Status */
+#define RTR 0x4000 /* Remote Frame Transmission Request */
+#define BOIS 0x0008 /* Bus Off IRQ Status */
+#define IDE 0x2000 /* Identifier Extension */
+#define EPIS 0x0004 /* Error-Passive Mode IRQ Status */
+#define EPIM 0x0004 /* Enable Error-Passive Mode Interrupt */
+#define EWTIS 0x0001 /* TX Error Count IRQ Status */
+#define EWRIS 0x0002 /* RX Error Count IRQ Status */
+#define BEF 0x0040 /* Bit Error Flag */
+#define FER 0x0080 /* Form Error Flag */
+#define SMR 0x0020 /* Sleep Mode Request */
+#define SMACK 0x0008 /* Sleep Mode Acknowledge */
+
/*
* bfin can private data
*/
--
1.7.9.5
^ permalink raw reply related
* Re: cw1200: use msecs_to_jiffies for conversion
From: Kalle Valo @ 2015-02-06 6:43 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: Solomon Peachy, linux-wireless, netdev, linux-kernel,
Nicholas Mc Guire
In-Reply-To: <1423035589-3647-1-git-send-email-hofrat@osadl.org>
> This is only an API consolidation to make things more readable.
> Instances of HZ / CONST are replaced by appropriate msecs_to_jiffies().
>
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
Thanks, applied to wireless-drivers-next.git.
Kalle Valo
^ permalink raw reply
* Re: [1/3] orinoco: orinoco_plx use msecs_to_jiffies for conversion
From: Kalle Valo @ 2015-02-06 6:46 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: Benoit Taine, Bjorn Helgaas, linux-wireless, netdev, linux-kernel,
Nicholas Mc Guire
In-Reply-To: <1423037261-20193-1-git-send-email-hofrat@osadl.org>
> This is only an API consolidation and should make things more readable
> it replaces var * HZ / 1000 by msecs_to_jiffies(var).
>
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
Thanks, 3 patches applied to wireless-drivers-next.git:
ab458cc85fa2 orinoco: orinoco_plx use msecs_to_jiffies for conversion
3427da4597ae orinoco: orinoco_pci use msecs_to_jiffies for conversion
c1f1f6663b3c orinoco: orinoco_tmd use msecs_to_jiffies for conversion
Kalle Valo
^ permalink raw reply
* Re: [1/2] cw1200: Delete an unnecessary check before the function call "release_firmware"
From: Kalle Valo @ 2015-02-06 6:49 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Solomon Peachy, netdev, linux-wireless, LKML, kernel-janitors,
Julia Lawall
In-Reply-To: <54D24D1F.50001@users.sourceforge.net>
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 4 Feb 2015 16:32:15 +0100
>
> The release_firmware() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Thanks, 2 patches applied to wireless-drivers-next.git:
df970d39b90e cw1200: Delete an unnecessary check before the function call "release_firmware"
ee4ddad82356 cw1200: Less function calls in cw1200_load_firmware_cw1200() after error detection
Kalle Valo
^ permalink raw reply
* Re: ath9k: Delete an unnecessary check before the function call"relay_close"
From: Kalle Valo @ 2015-02-06 6:50 UTC (permalink / raw)
To: SF Markus Elfring
Cc: ath9k-devel, linux-wireless, QCA ath9k Development, netdev, LKML,
kernel-janitors, Julia Lawall
In-Reply-To: <54D25CDC.3040405@users.sourceforge.net>
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 4 Feb 2015 18:48:28 +0100
>
> The relay_close() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Thanks, applied to wireless-drivers-next.git.
Kalle Valo
^ permalink raw reply
* Re: orinoco: Delete an unnecessary check before the function call "kfree"
From: Kalle Valo @ 2015-02-06 6:51 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-wireless, netdev, LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54D26B73.1050804@users.sourceforge.net>
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 4 Feb 2015 19:53:11 +0100
>
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Thanks, applied to wireless-drivers-next.git.
Kalle Valo
^ permalink raw reply
* Re: hostap: Delete an unnecessary check before the function call "kfree"
From: Kalle Valo @ 2015-02-06 6:52 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Jouni Malinen, linux-wireless, netdev, LKML, kernel-janitors,
Julia Lawall
In-Reply-To: <54D26EA9.6020301@users.sourceforge.net>
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 4 Feb 2015 20:06:39 +0100
>
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Thanks, applied to wireless-drivers-next.git.
Kalle Valo
^ permalink raw reply
* Re: [PATCH net-next v5 0/7]: ixgbevf: Allow querying VFs RSS indirection table and key
From: Jeff Kirsher @ 2015-02-06 6:52 UTC (permalink / raw)
To: Vlad Zolotarov; +Cc: netdev, Gleb Natapov, Avi Kivity
In-Reply-To: <1420661025.2491.73.camel@jtkirshe-mobl>
On Wed, Jan 7, 2015 at 12:03 PM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> On Wed, 2015-01-07 at 21:26 +0200, Vlad Zolotarov wrote:
>> Add the ethtool ops to VF driver to allow querying the RSS indirection
>> table
>> and RSS Random Key.
>>
>> On some devices VFs share the RSS Redirection Table and Hash Key with
>> a PF and letting
>> the VF query this information may introduce some security risks.
>> Therefore we disable this
>> feature by default for such devices (e.g. 82599) and allow it for
>> those where there isn't any
>> possible risk (e.g. on x550). The new netdev op is going to allow a
>> system administrator to
>> change the default behaviour with "ip link set" command.
>>
>> - netdev: Add a new netdev op to allow/block VF from querying RSS
>> Indirection Table and
>> RSS Hash Key.
>> - PF driver: Add new VF-PF channel commands.
>> - VF driver: Utilize these new commands and add the corresponding
>> ethtool callbacks.
>>
>> New in v5:
>> - Added a new netdev op to allow/block VF from querying RSS
>> Indirection Table and
>> RSS Hash Key.
>> - Let VF query the RSS info only if VF is allowed to.
>>
>> New in v4:
>> - Forgot to run checkpatch on v3 and there were a few styling
>> things to fix. ;)
>>
>> New in v3:
>> - Added a missing support for x550 devices.
>> - Mask the indirection table values according to PSRTYPE[n].RQPL.
>> - Minimized the number of added VF-PF commands.
>>
>> New in v2:
>> - Added a detailed description to patches 4 and 5.
>>
>> New in v1 (compared to RFC):
>> - Use "if-else" statement instead of a "switch-case" for a single
>> option case.
>> More specifically: in cases where the newly added API version is
>> the only one
>> allowed. We may consider using a "switch-case" back again when
>> the list of
>> allowed API versions in these specific places grows up.
>>
>> Vlad Zolotarov (7):
>> if_link: Add an additional parameter to ifla_vf_info for RSS
>> querying
>> ixgbe: Add a new netdev op to allow/prevent a VF from querying an
>> RSS
>> info
>> ixgbe: Add a RETA query command to VF-PF channel API
>> ixgbevf: Add a RETA query code
>> ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
>> ixgbevf: Add RSS Key query code
>> ixgbevf: Add the appropriate ethtool ops to query RSS indirection
>> table and key
>>
>> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 +
>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 7 ++
>> drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 10 ++
>> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 119
>> +++++++++++++++++++
>> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h | 2 +
>> drivers/net/ethernet/intel/ixgbevf/ethtool.c | 42 +++++++
>> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 +-
>> drivers/net/ethernet/intel/ixgbevf/mbx.h | 10 ++
>> drivers/net/ethernet/intel/ixgbevf/vf.c | 132
>> ++++++++++++++++++++++
>> drivers/net/ethernet/intel/ixgbevf/vf.h | 2 +
>> include/linux/if_link.h | 1 +
>> include/linux/netdevice.h | 8 ++
>> include/uapi/linux/if_link.h | 8 ++
>> net/core/rtnetlink.c | 33 +++++-
>> 14 files changed, 372 insertions(+), 7 deletions(-)
>
> Thanks Vlad, I will add your patches to my queue.
Validation ran into issues with your patch series, they reported the following:
Ethtool has "Cannot get RX ring count: Operation not supported" errors
when trying to access RSS flow hash table.
So I am dropping the series for now and will await a v6.
--
Cheers,
Jeff
^ permalink raw reply
* Re: brcm80211: Delete unnecessary checks before two function calls
From: Kalle Valo @ 2015-02-06 6:53 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Arend van Spriel, Brett Rudley, Franky (Zhenhui) Lin,
Hante Meuleman, brcm80211-dev-list, linux-wireless, netdev, LKML,
kernel-janitors, Julia Lawall
In-Reply-To: <54D275A7.3050701@users.sourceforge.net>
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 4 Feb 2015 20:28:49 +0100
>
> The functions brcmu_pkt_buf_free_skb() and usb_free_urb() test whether
> their argument is NULL and then return immediately. Thus the test around
> the call is not needed.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Thanks, applied to wireless-drivers-next.git.
Kalle Valo
^ permalink raw reply
* Re: [PATCH] af_packet: don't pass empty blocks for PACKET_V3
From: Alexander Drozdov @ 2015-02-06 6:54 UTC (permalink / raw)
To: Willem de Bruijn
Cc: David S. Miller, Daniel Borkmann, Eric Dumazet, Al Viro,
Michael S. Tsirkin, Network Development, linux-kernel, Guy Harris,
Dan Collins
In-Reply-To: <CA+FuTScA_2W+MHsZdYfoOKS==1XyogmyF_d+qUy66A5MdyO-5Q@mail.gmail.com>
On 05.02.2015 23:01:38 +0300 Willem de Bruijn wrote:
> On Wed, Feb 4, 2015 at 9:58 PM, Alexander Drozdov <al.drozdov@gmail.com> wrote:
>> Don't close an empty block on timeout. Its meaningless to
>> pass it to the user. Moreover, passing empty blocks wastes
>> CPU & buffer space increasing probability of packets
>> dropping on small timeouts.
>>
>> Side effect of this patch is indefinite user-space wait
>> in poll on idle links. But, I believe its better to set
>> timeout for poll(2) when needed than to get empty blocks
>> every millisecond when not needed.
> This change would break existing applications that have come
> to depend on the periodic signal.
>
> I don't disagree with the argument that the data ready signal
> should be sent only when a block is full or a timer expires and
> at least some data is waiting, but that is moot at this point.
I missed something. As pointed by Guy Harris <guy@alum.mit.edu>,
before the previous patch periodic signal was not delivered. The previous patch
(da413eec729dae5dc by Dan Collins <dan@dcollins.co.nz>) is for 3.19 kernel only.
Should we care about existing 3.19-only applications?
>
>> Signed-off-by: Alexander Drozdov <al.drozdov@gmail.com>
>> ---
>> net/packet/af_packet.c | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
>> index 9cfe2e1..9a2f70a 100644
>> --- a/net/packet/af_packet.c
>> +++ b/net/packet/af_packet.c
>> @@ -698,6 +698,10 @@ static void prb_retire_rx_blk_timer_expired(unsigned long data)
>>
>> if (pkc->last_kactive_blk_num == pkc->kactive_blk_num) {
>> if (!frozen) {
>> + if (!BLOCK_NUM_PKTS(pbd)) {
>> + /* An empty block. Just refresh the timer. */
>> + goto refresh_timer;
>> + }
>> prb_retire_current_block(pkc, po, TP_STATUS_BLK_TMO);
>> if (!prb_dispatch_next_block(pkc, po))
>> goto refresh_timer;
>> @@ -798,7 +802,11 @@ static void prb_close_block(struct tpacket_kbdq_core *pkc1,
>> h1->ts_last_pkt.ts_sec = last_pkt->tp_sec;
>> h1->ts_last_pkt.ts_nsec = last_pkt->tp_nsec;
>> } else {
>> - /* Ok, we tmo'd - so get the current time */
>> + /* Ok, we tmo'd - so get the current time.
>> + *
>> + * It shouldn't really happen as we don't close empty
>> + * blocks. See prb_retire_rx_blk_timer_expired().
>> + */
>> struct timespec ts;
>> getnstimeofday(&ts);
>> h1->ts_last_pkt.ts_sec = ts.tv_sec;
>> --
>> 1.9.1
>>
^ permalink raw reply
* Re: ipv6: oops in datagram.c line 260
From: Chris Ruehl @ 2015-02-06 7:37 UTC (permalink / raw)
To: Steffen Klassert; +Cc: Hannes Frederic Sowa, netdev, davem
In-Reply-To: <20150127115814.GL13046@secunet.com>
On Tuesday, January 27, 2015 07:58 PM, Steffen Klassert wrote:
> On Tue, Jan 27, 2015 at 12:58:35PM +0800, Chris Ruehl wrote:
>> Steffen,
>>
>> your patch can't apply to the vanilla v3.14.29 can you cross check please.
> Sorry, this patch was based on the net tree.
>
>> I'm sorry but we running a productive system and I can't make to much
>> noise here!
>> Your patch is partly in the 3.14.29 and
>> skb->protocol = htons(ETH_P_IP)
>> from the xfrm4/6_output_finish() no removed. I do then
>>
>> --- linux-3.14.x/net/ipv4/xfrm4_output.c.orig 2015-01-27
>> 12:50:01.830651344 +0800
>> +++ linux-3.14.x/net/ipv4/xfrm4_output.c 2015-01-27 12:51:13.280386355
>> +0800
>> @@ -82,7 +82,6 @@
>> IPCB(skb)->flags |= IPSKB_XFRM_TRANSFORMED;
>> #endif
>>
>> - skb->protocol = htons(ETH_P_IP);
>> return xfrm_output(skb);
>> }
>>
>> --- linux-3.14.x/net/ipv6/xfrm6_output.c.orig 2015-01-27
>> 12:49:39.260735321 +0800
>> +++ linux-3.14.x/net/ipv6/xfrm6_output.c 2015-01-27 12:50:47.280482636
>> +0800
>> @@ -132,7 +132,6 @@
>> IP6CB(skb)->flags |= IP6SKB_XFRM_TRANSFORMED;
>> #endif
>>
>> - skb->protocol = htons(ETH_P_IPV6);
>> return xfrm_output(skb);
>> }
> Yes, that should be ok. Here is the complete patch for v3.14.29:
>
> Subject: [PATCH RFC v3.14.29] xfrm: Fix local error reporting crash with interfamily tunnels
>
> We set the outer mode protocol too early. As a result, the
> local error handler might dispatch to the wrong address family
> and report the error to a wrong socket type. We fix this by
> seting the outer protocol to the skb only after we accessed the
> inner mode for the last time, right before we do the atcual
> encapsulation where we switch finally to the outer mode.
> The settings in xfrm{4,6}_output_finish() are removed.
>
> Reported-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> ---
> net/ipv4/xfrm4_output.c | 1 -
> net/ipv6/xfrm6_output.c | 1 -
> 2 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
> index baa0f63..0cb9606 100644
> --- a/net/ipv4/xfrm4_output.c
> +++ b/net/ipv4/xfrm4_output.c
> @@ -82,7 +82,6 @@ int xfrm4_output_finish(struct sk_buff *skb)
> IPCB(skb)->flags |= IPSKB_XFRM_TRANSFORMED;
> #endif
>
> - skb->protocol = htons(ETH_P_IP);
> return xfrm_output(skb);
> }
>
> diff --git a/net/ipv6/xfrm6_output.c b/net/ipv6/xfrm6_output.c
> index 6cd625e..98396cf 100644
> --- a/net/ipv6/xfrm6_output.c
> +++ b/net/ipv6/xfrm6_output.c
> @@ -132,7 +132,6 @@ int xfrm6_output_finish(struct sk_buff *skb)
> IP6CB(skb)->flags |= IP6SKB_XFRM_TRANSFORMED;
> #endif
>
> - skb->protocol = htons(ETH_P_IPV6);
> return xfrm_output(skb);
> }
>
Hi Steffen,
server is up for 6 days no problems any more.
Please apply the patch!
Thank you very much
Chris
Tested-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
^ 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