* [PATCH net-next 00/10] ibmvnic: Updates and bug fixes
From: Nathan Fontenot @ 2017-04-19 17:44 UTC (permalink / raw)
To: netdev; +Cc: brking, jallen, muvic, tlfalcon
This set of patches is a series of updates to remove some unneeded
and unused code in the driver as well as bug fixes for the
ibmvnic driver.
---
Brian King (4):
ibmvnic: Unmap longer term buffer before free
ibmvnic: Fixup atomic API usage
ibmvnic: Do not disable IRQ after scheduling tasklet
ibmvnic: Disable irq prior to close
Murilo Fossa Vicentini (1):
ibmvnic: Fix ibmvnic_change_mac_addr struct format
Nathan Fontenot (4):
ibmvnic: Remove inflight list
ibmvnic: Correct crq and resource releasing
ibmvnic: Allocate zero-filled memory for sub crqs
ibmvnic: Remove unused bouce buffer
Thomas Falcon (1):
ibmvnic: Report errors when failing to release sub-crqs
drivers/net/ethernet/ibm/ibmvnic.c | 223 ++++++++++--------------------------
drivers/net/ethernet/ibm/ibmvnic.h | 15 --
2 files changed, 61 insertions(+), 177 deletions(-)
^ permalink raw reply
* [PATCH net-next 01/10] ibmvnic: Report errors when failing to release sub-crqs
From: Nathan Fontenot @ 2017-04-19 17:44 UTC (permalink / raw)
To: netdev; +Cc: brking, jallen, muvic, tlfalcon
In-Reply-To: <20170419174015.37372.48544.stgit@ltcalpine2-lp23.aus.stglabs.ibm.com>
From: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
Add reporting of errors when releasing sub-crqs fails.
Signed-off-by: Thomas Falcon <tlfalcon@us.ibm.com>
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 7ba43cf..7bf3507 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1309,6 +1309,12 @@ static void release_sub_crq_queue(struct ibmvnic_adapter *adapter,
scrq->crq_num);
} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
+ if (rc) {
+ netdev_err(adapter->netdev,
+ "Failed to release sub-CRQ %16lx, rc = %ld\n",
+ scrq->crq_num, rc);
+ }
+
dma_unmap_single(dev, scrq->msg_token, 4 * PAGE_SIZE,
DMA_BIDIRECTIONAL);
free_pages((unsigned long)scrq->msgs, 2);
^ permalink raw reply related
* [PATCH net-next 02/10] ibmvnic: Fix ibmvnic_change_mac_addr struct format
From: Nathan Fontenot @ 2017-04-19 17:44 UTC (permalink / raw)
To: netdev; +Cc: brking, jallen, muvic, tlfalcon
In-Reply-To: <20170419174015.37372.48544.stgit@ltcalpine2-lp23.aus.stglabs.ibm.com>
From: Murilo Fossa Vicentini <muvic@linux.vnet.ibm.com>
The ibmvnic_change_mac_addr struct alignment was not matching the defined
format in PAPR+, it had the reserved and return code fields swapped. As a
consequence, the CHANGE_MAC_ADDR_RSP commands were being improperly handled
and executed even when the operation wasn't successfully completed by the
system firmware.
Also changing the endianness of the debug message to make it easier to
parse the CRQ content.
Signed-off-by: Murilo Fossa Vicentini <muvic@linux.vnet.ibm.com>
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 5 +++--
drivers/net/ethernet/ibm/ibmvnic.h | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 7bf3507..625896d 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2890,11 +2890,12 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
struct ibmvnic_generic_crq *gen_crq = &crq->generic;
struct net_device *netdev = adapter->netdev;
struct device *dev = &adapter->vdev->dev;
+ u64 *u64_crq = (u64 *)crq;
long rc;
netdev_dbg(netdev, "Handling CRQ: %016lx %016lx\n",
- ((unsigned long int *)crq)[0],
- ((unsigned long int *)crq)[1]);
+ (unsigned long int)cpu_to_be64(u64_crq[0]),
+ (unsigned long int)cpu_to_be64(u64_crq[1]));
switch (gen_crq->first) {
case IBMVNIC_CRQ_INIT_RSP:
switch (gen_crq->cmd) {
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index b0d0b89..1b404ca 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -518,8 +518,8 @@ struct ibmvnic_change_mac_addr {
u8 first;
u8 cmd;
u8 mac_addr[6];
- struct ibmvnic_rc rc;
u8 reserved[4];
+ struct ibmvnic_rc rc;
} __packed __aligned(8);
struct ibmvnic_multicast_ctrl {
^ permalink raw reply related
* [PATCH net-next 03/10] ibmvnic: Unmap longer term buffer before free
From: Nathan Fontenot @ 2017-04-19 17:44 UTC (permalink / raw)
To: netdev; +Cc: brking, jallen, muvic, tlfalcon
In-Reply-To: <20170419174015.37372.48544.stgit@ltcalpine2-lp23.aus.stglabs.ibm.com>
From: Brian King <brking@linux.vnet.ibm.com>
Make sure we unregister long term buffers from the adapter
prior to DMA unmapping it and freeing the buffer. Failure
to do so could result in a DMA to a now invalid address.
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 625896d..c10bae7 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -193,9 +193,9 @@ static void free_long_term_buff(struct ibmvnic_adapter *adapter,
if (!ltb->buff)
return;
- dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
if (!adapter->failover)
send_request_unmap(adapter, ltb->map_id);
+ dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
}
static void replenish_rx_pool(struct ibmvnic_adapter *adapter,
^ permalink raw reply related
* [PATCH net-next 04/10] ibmvnic: Fixup atomic API usage
From: Nathan Fontenot @ 2017-04-19 17:44 UTC (permalink / raw)
To: netdev; +Cc: brking, jallen, muvic, tlfalcon
In-Reply-To: <20170419174015.37372.48544.stgit@ltcalpine2-lp23.aus.stglabs.ibm.com>
From: Brian King <brking@linux.vnet.ibm.com>
Replace a couple of modifications of an atomic followed
by a read of the atomic, which is no longer atomic, to
use atomic_XX_return variants to avoid race conditions.
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index c10bae7..b5871df 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -962,9 +962,8 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
goto out;
}
- atomic_inc(&tx_scrq->used);
-
- if (atomic_read(&tx_scrq->used) >= adapter->req_tx_entries_per_subcrq) {
+ if (atomic_inc_return(&tx_scrq->used)
+ >= adapter->req_tx_entries_per_subcrq) {
netdev_info(netdev, "Stopping queue %d\n", queue_num);
netif_stop_subqueue(netdev, queue_num);
}
@@ -1499,9 +1498,8 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter *adapter,
}
if (txbuff->last_frag) {
- atomic_dec(&scrq->used);
-
- if (atomic_read(&scrq->used) <=
+ if (atomic_sub_return(next->tx_comp.num_comps,
+ &scrq->used) <=
(adapter->req_tx_entries_per_subcrq / 2) &&
netif_subqueue_stopped(adapter->netdev,
txbuff->skb)) {
^ permalink raw reply related
* [PATCH net-next 05/10] ibmvnic: Do not disable IRQ after scheduling tasklet
From: Nathan Fontenot @ 2017-04-19 17:44 UTC (permalink / raw)
To: netdev; +Cc: brking, jallen, muvic, tlfalcon
In-Reply-To: <20170419174015.37372.48544.stgit@ltcalpine2-lp23.aus.stglabs.ibm.com>
From: Brian King <brking@linux.vnet.ibm.com>
Since the primary CRQ is only used for service functions and
not in the performance path, simplify the code a bit and avoid
disabling the IRQ.
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 25 ++++++-------------------
1 file changed, 6 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index b5871df..27d7d27 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3027,12 +3027,8 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
static irqreturn_t ibmvnic_interrupt(int irq, void *instance)
{
struct ibmvnic_adapter *adapter = instance;
- unsigned long flags;
- spin_lock_irqsave(&adapter->crq.lock, flags);
- vio_disable_interrupts(adapter->vdev);
tasklet_schedule(&adapter->tasklet);
- spin_unlock_irqrestore(&adapter->crq.lock, flags);
return IRQ_HANDLED;
}
@@ -3040,32 +3036,23 @@ static void ibmvnic_tasklet(void *data)
{
struct ibmvnic_adapter *adapter = data;
struct ibmvnic_crq_queue *queue = &adapter->crq;
- struct vio_dev *vdev = adapter->vdev;
union ibmvnic_crq *crq;
unsigned long flags;
bool done = false;
spin_lock_irqsave(&queue->lock, flags);
- vio_disable_interrupts(vdev);
while (!done) {
/* Pull all the valid messages off the CRQ */
while ((crq = ibmvnic_next_crq(adapter)) != NULL) {
ibmvnic_handle_crq(crq, adapter);
crq->generic.first = 0;
}
- vio_enable_interrupts(vdev);
- crq = ibmvnic_next_crq(adapter);
- if (crq) {
- vio_disable_interrupts(vdev);
- ibmvnic_handle_crq(crq, adapter);
- crq->generic.first = 0;
- } else {
- /* remain in tasklet until all
- * capabilities responses are received
- */
- if (!adapter->wait_capability)
- done = true;
- }
+
+ /* remain in tasklet until all
+ * capabilities responses are received
+ */
+ if (!adapter->wait_capability)
+ done = true;
}
/* if capabilities CRQ's were sent in this tasklet, the following
* tasklet must wait until all responses are received
^ permalink raw reply related
* [PATCH net-next 06/10] ibmvnic: Remove inflight list
From: Nathan Fontenot @ 2017-04-19 17:44 UTC (permalink / raw)
To: netdev; +Cc: brking, jallen, muvic, tlfalcon
In-Reply-To: <20170419174015.37372.48544.stgit@ltcalpine2-lp23.aus.stglabs.ibm.com>
The inflight list used to track memory that is allocated for crq that are
inflight is not needed. The one piece of the inflight list that does need
to be cleaned at module exit is the error buffer list which is already
attached to the adapter struct.
This patch removes the inflight list and moves checking the error buffer
list to ibmvnic_remove.
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 98 +++++++-----------------------------
drivers/net/ethernet/ibm/ibmvnic.h | 9 ---
2 files changed, 19 insertions(+), 88 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 27d7d27..18673e2 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -546,6 +546,23 @@ static int init_bounce_buffer(struct net_device *netdev)
return 0;
}
+static void release_error_buffers(struct ibmvnic_adapter *adapter)
+{
+ struct device *dev = &adapter->vdev->dev;
+ struct ibmvnic_error_buff *error_buff, *tmp;
+ unsigned long flags;
+
+ spin_lock_irqsave(&adapter->error_list_lock, flags);
+ list_for_each_entry_safe(error_buff, tmp, &adapter->errors, list) {
+ list_del(&error_buff->list);
+ dma_unmap_single(dev, error_buff->dma, error_buff->len,
+ DMA_FROM_DEVICE);
+ kfree(error_buff->buff);
+ kfree(error_buff);
+ }
+ spin_unlock_irqrestore(&adapter->error_list_lock, flags);
+}
+
static int ibmvnic_login(struct net_device *netdev)
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
@@ -588,6 +605,7 @@ static void release_resources(struct ibmvnic_adapter *adapter)
release_crq_queue(adapter);
release_stats_token(adapter);
+ release_error_buffers(adapter);
}
static int ibmvnic_open(struct net_device *netdev)
@@ -1957,13 +1975,11 @@ static void send_login(struct ibmvnic_adapter *adapter)
{
struct ibmvnic_login_rsp_buffer *login_rsp_buffer;
struct ibmvnic_login_buffer *login_buffer;
- struct ibmvnic_inflight_cmd *inflight_cmd;
struct device *dev = &adapter->vdev->dev;
dma_addr_t rsp_buffer_token;
dma_addr_t buffer_token;
size_t rsp_buffer_size;
union ibmvnic_crq crq;
- unsigned long flags;
size_t buffer_size;
__be64 *tx_list_p;
__be64 *rx_list_p;
@@ -2000,11 +2016,7 @@ static void send_login(struct ibmvnic_adapter *adapter)
dev_err(dev, "Couldn't map login rsp buffer\n");
goto buf_rsp_map_failed;
}
- inflight_cmd = kmalloc(sizeof(*inflight_cmd), GFP_ATOMIC);
- if (!inflight_cmd) {
- dev_err(dev, "Couldn't allocate inflight_cmd\n");
- goto inflight_alloc_failed;
- }
+
adapter->login_buf = login_buffer;
adapter->login_buf_token = buffer_token;
adapter->login_buf_sz = buffer_size;
@@ -2055,20 +2067,10 @@ static void send_login(struct ibmvnic_adapter *adapter)
crq.login.cmd = LOGIN;
crq.login.ioba = cpu_to_be32(buffer_token);
crq.login.len = cpu_to_be32(buffer_size);
-
- memcpy(&inflight_cmd->crq, &crq, sizeof(crq));
-
- spin_lock_irqsave(&adapter->inflight_lock, flags);
- list_add_tail(&inflight_cmd->list, &adapter->inflight);
- spin_unlock_irqrestore(&adapter->inflight_lock, flags);
-
ibmvnic_send_crq(adapter, &crq);
return;
-inflight_alloc_failed:
- dma_unmap_single(dev, rsp_buffer_token, rsp_buffer_size,
- DMA_FROM_DEVICE);
buf_rsp_map_failed:
kfree(login_rsp_buffer);
buf_rsp_alloc_failed:
@@ -2374,7 +2376,6 @@ static void handle_error_indication(union ibmvnic_crq *crq,
struct ibmvnic_adapter *adapter)
{
int detail_len = be32_to_cpu(crq->error_indication.detail_error_sz);
- struct ibmvnic_inflight_cmd *inflight_cmd;
struct device *dev = &adapter->vdev->dev;
struct ibmvnic_error_buff *error_buff;
union ibmvnic_crq new_crq;
@@ -2406,15 +2407,6 @@ static void handle_error_indication(union ibmvnic_crq *crq,
return;
}
- inflight_cmd = kmalloc(sizeof(*inflight_cmd), GFP_ATOMIC);
- if (!inflight_cmd) {
- dma_unmap_single(dev, error_buff->dma, detail_len,
- DMA_FROM_DEVICE);
- kfree(error_buff->buff);
- kfree(error_buff);
- return;
- }
-
error_buff->len = detail_len;
error_buff->error_id = crq->error_indication.error_id;
@@ -2428,13 +2420,6 @@ static void handle_error_indication(union ibmvnic_crq *crq,
new_crq.request_error_info.ioba = cpu_to_be32(error_buff->dma);
new_crq.request_error_info.len = cpu_to_be32(detail_len);
new_crq.request_error_info.error_id = crq->error_indication.error_id;
-
- memcpy(&inflight_cmd->crq, &crq, sizeof(crq));
-
- spin_lock_irqsave(&adapter->inflight_lock, flags);
- list_add_tail(&inflight_cmd->list, &adapter->inflight);
- spin_unlock_irqrestore(&adapter->inflight_lock, flags);
-
ibmvnic_send_crq(adapter, &new_crq);
}
@@ -2819,48 +2804,6 @@ static void handle_query_cap_rsp(union ibmvnic_crq *crq,
}
}
-static void ibmvnic_free_inflight(struct ibmvnic_adapter *adapter)
-{
- struct ibmvnic_inflight_cmd *inflight_cmd, *tmp1;
- struct device *dev = &adapter->vdev->dev;
- struct ibmvnic_error_buff *error_buff, *tmp2;
- unsigned long flags;
- unsigned long flags2;
-
- spin_lock_irqsave(&adapter->inflight_lock, flags);
- list_for_each_entry_safe(inflight_cmd, tmp1, &adapter->inflight, list) {
- switch (inflight_cmd->crq.generic.cmd) {
- case LOGIN:
- dma_unmap_single(dev, adapter->login_buf_token,
- adapter->login_buf_sz,
- DMA_BIDIRECTIONAL);
- dma_unmap_single(dev, adapter->login_rsp_buf_token,
- adapter->login_rsp_buf_sz,
- DMA_BIDIRECTIONAL);
- kfree(adapter->login_rsp_buf);
- kfree(adapter->login_buf);
- break;
- case REQUEST_ERROR_INFO:
- spin_lock_irqsave(&adapter->error_list_lock, flags2);
- list_for_each_entry_safe(error_buff, tmp2,
- &adapter->errors, list) {
- dma_unmap_single(dev, error_buff->dma,
- error_buff->len,
- DMA_FROM_DEVICE);
- kfree(error_buff->buff);
- list_del(&error_buff->list);
- kfree(error_buff);
- }
- spin_unlock_irqrestore(&adapter->error_list_lock,
- flags2);
- break;
- }
- list_del(&inflight_cmd->list);
- kfree(inflight_cmd);
- }
- spin_unlock_irqrestore(&adapter->inflight_lock, flags);
-}
-
static void ibmvnic_xport_event(struct work_struct *work)
{
struct ibmvnic_adapter *adapter = container_of(work,
@@ -2869,7 +2812,6 @@ static void ibmvnic_xport_event(struct work_struct *work)
struct device *dev = &adapter->vdev->dev;
long rc;
- ibmvnic_free_inflight(adapter);
release_sub_crqs(adapter);
if (adapter->migrated) {
rc = ibmvnic_reenable_crq_queue(adapter);
@@ -3333,9 +3275,7 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
spin_lock_init(&adapter->stats_lock);
INIT_LIST_HEAD(&adapter->errors);
- INIT_LIST_HEAD(&adapter->inflight);
spin_lock_init(&adapter->error_list_lock);
- spin_lock_init(&adapter->inflight_lock);
rc = ibmvnic_init(adapter);
if (rc) {
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index 1b404ca..8fbe05d 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -913,11 +913,6 @@ struct ibmvnic_error_buff {
__be32 error_id;
};
-struct ibmvnic_inflight_cmd {
- union ibmvnic_crq crq;
- struct list_head list;
-};
-
struct ibmvnic_adapter {
struct vio_dev *vdev;
struct net_device *netdev;
@@ -978,10 +973,6 @@ struct ibmvnic_adapter {
struct completion fw_done;
- /* in-flight commands that allocate and/or map memory*/
- struct list_head inflight;
- spinlock_t inflight_lock;
-
/* partner capabilities */
u64 min_tx_queues;
u64 min_rx_queues;
^ permalink raw reply related
* [PATCH net-next 07/10] ibmvnic: Correct crq and resource releasing
From: Nathan Fontenot @ 2017-04-19 17:45 UTC (permalink / raw)
To: netdev; +Cc: brking, jallen, muvic, tlfalcon
In-Reply-To: <20170419174015.37372.48544.stgit@ltcalpine2-lp23.aus.stglabs.ibm.com>
We should not be releasing the crq's when calling close for the
adapter, these need to remain open to facilitate operations such
as updating the mac address. The crq's should be released in the
adpaters remove routine.
Additionally, we need to call release_reources from remove. This
corrects the scenario of trying to remove an adapter that has only
been probed.
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 18673e2..a8b3c57 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -601,9 +601,6 @@ static void release_resources(struct ibmvnic_adapter *adapter)
release_tx_pools(adapter);
release_rx_pools(adapter);
- release_sub_crqs(adapter);
- release_crq_queue(adapter);
-
release_stats_token(adapter);
release_error_buffers(adapter);
}
@@ -3300,8 +3297,14 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
static int ibmvnic_remove(struct vio_dev *dev)
{
struct net_device *netdev = dev_get_drvdata(&dev->dev);
+ struct ibmvnic_adapter *adapter = netdev_priv(netdev);
unregister_netdev(netdev);
+
+ release_resources(adapter);
+ release_sub_crqs(adapter);
+ release_crq_queue(adapter);
+
free_netdev(netdev);
dev_set_drvdata(&dev->dev, NULL);
^ permalink raw reply related
* [PATCH net-next 08/10] ibmvnic: Disable irq prior to close
From: Nathan Fontenot @ 2017-04-19 17:45 UTC (permalink / raw)
To: netdev; +Cc: brking, jallen, muvic, tlfalcon
In-Reply-To: <20170419174015.37372.48544.stgit@ltcalpine2-lp23.aus.stglabs.ibm.com>
From: Brian King <brking@linux.vnet.ibm.com>
Add some code to call disable_irq on all the vnic interface's irqs.
This fixes a crash observed when closing an active interface, as
seen in the oops below when we try to access a buffer in the interrupt
handler which we've already freed.
Unable to handle kernel paging request for data at address 0x00000001
Faulting instruction address: 0xd000000003886824
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: ibmvnic(OEN) rpadlpar_io(X) rpaphp(X) tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag rpcsec_
Supported: No, Unsupported modules are loaded
CPU: 8 PID: 0 Comm: swapper/8 Tainted: G OE NX 4.4.49-92.11-default #1
task: c00000007f990110 ti: c0000000fffa0000 task.ti: c00000007f9b8000
NIP: d000000003886824 LR: d000000003886824 CTR: c0000000007eff60
REGS: c0000000fffa3a70 TRAP: 0300 Tainted: G OE NX (4.4.49-92.11-default)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 22008042 XER: 20000008
CFAR: c000000000008468 DAR: 0000000000000001 DSISR: 40000000 SOFTE: 0
GPR00: d000000003886824 c0000000fffa3cf0 d000000003894118 0000000000000000
GPR04: 0000000000000000 0000000000000000 c000000001249da0 0000000000000000
GPR08: 000000000000000e 0000000000000000 c0000000ccb00000 d000000003889180
GPR12: c0000000007eff60 c000000007af4c00 0000000000000001 c0000000010def30
GPR16: c00000007f9b8000 c000000000b98c30 c00000007f9b8080 c000000000bab858
GPR20: 0000000000000005 0000000000000000 c0000000ff5d7e80 c0000000f809f648
GPR24: c0000000ff5d7ec8 0000000000000000 0000000000000000 c0000000ccb001a0
GPR28: 000000000000000a c0000000f809f600 c0000000fd4cd900 c0000000f9cd5b00
NIP [d000000003886824] ibmvnic_interrupt_tx+0x114/0x380 [ibmvnic]
LR [d000000003886824] ibmvnic_interrupt_tx+0x114/0x380 [ibmvnic]
Call Trace:
[c0000000fffa3cf0] [d000000003886824] ibmvnic_interrupt_tx+0x114/0x380 [ibmvnic] (unreliable)
[c0000000fffa3dd0] [c000000000132940] __handle_irq_event_percpu+0x90/0x2e0
[c0000000fffa3e90] [c000000000132bcc] handle_irq_event_percpu+0x3c/0x90
[c0000000fffa3ed0] [c000000000132c88] handle_irq_event+0x68/0xc0
[c0000000fffa3f00] [c000000000137edc] handle_fasteoi_irq+0xec/0x250
[c0000000fffa3f30] [c000000000131b04] generic_handle_irq+0x54/0x80
[c0000000fffa3f60] [c000000000011190] __do_irq+0x80/0x1d0
[c0000000fffa3f90] [c0000000000248d8] call_do_irq+0x14/0x24
[c00000007f9bb9e0] [c000000000011380] do_IRQ+0xa0/0x120
[c00000007f9bba40] [c000000000002594] hardware_interrupt_common+0x114/0x180
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index a8b3c57..ce8b147 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -689,6 +689,23 @@ static int ibmvnic_open(struct net_device *netdev)
return -ENOMEM;
}
+static void disable_sub_crqs(struct ibmvnic_adapter *adapter)
+{
+ int i;
+
+ if (adapter->tx_scrq) {
+ for (i = 0; i < adapter->req_tx_queues; i++)
+ if (adapter->tx_scrq[i])
+ disable_irq(adapter->tx_scrq[i]->irq);
+ }
+
+ if (adapter->rx_scrq) {
+ for (i = 0; i < adapter->req_rx_queues; i++)
+ if (adapter->rx_scrq[i])
+ disable_irq(adapter->rx_scrq[i]->irq);
+ }
+}
+
static int ibmvnic_close(struct net_device *netdev)
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
@@ -696,6 +713,7 @@ static int ibmvnic_close(struct net_device *netdev)
int i;
adapter->closing = true;
+ disable_sub_crqs(adapter);
for (i = 0; i < adapter->req_rx_queues; i++)
napi_disable(&adapter->napi[i]);
^ permalink raw reply related
* [PATCH net-next 09/10] ibmvnic: Allocate zero-filled memory for sub crqs
From: Nathan Fontenot @ 2017-04-19 17:45 UTC (permalink / raw)
To: netdev; +Cc: brking, jallen, muvic, tlfalcon
In-Reply-To: <20170419174015.37372.48544.stgit@ltcalpine2-lp23.aus.stglabs.ibm.com>
Update the allocation of memory for the sub crq structs and their
associated pages to allocate zero-filled memory.
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index ce8b147..221d652 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1360,12 +1360,12 @@ static struct ibmvnic_sub_crq_queue *init_sub_crq_queue(struct ibmvnic_adapter
struct ibmvnic_sub_crq_queue *scrq;
int rc;
- scrq = kmalloc(sizeof(*scrq), GFP_ATOMIC);
+ scrq = kzalloc(sizeof(*scrq), GFP_ATOMIC);
if (!scrq)
return NULL;
- scrq->msgs = (union sub_crq *)__get_free_pages(GFP_ATOMIC, 2);
- memset(scrq->msgs, 0, 4 * PAGE_SIZE);
+ scrq->msgs =
+ (union sub_crq *)__get_free_pages(GFP_ATOMIC | __GFP_ZERO, 2);
if (!scrq->msgs) {
dev_warn(dev, "Couldn't allocate crq queue messages page\n");
goto zero_page_failed;
@@ -1393,9 +1393,6 @@ static struct ibmvnic_sub_crq_queue *init_sub_crq_queue(struct ibmvnic_adapter
scrq->adapter = adapter;
scrq->size = 4 * PAGE_SIZE / sizeof(*scrq->msgs);
- scrq->cur = 0;
- atomic_set(&scrq->used, 0);
- scrq->rx_skb_top = NULL;
spin_lock_init(&scrq->lock);
netdev_dbg(adapter->netdev,
^ permalink raw reply related
* [PATCH net-next 10/10] ibmvnic: Remove unused bouce buffer
From: Nathan Fontenot @ 2017-04-19 17:45 UTC (permalink / raw)
To: netdev; +Cc: brking, jallen, muvic, tlfalcon
In-Reply-To: <20170419174015.37372.48544.stgit@ltcalpine2-lp23.aus.stglabs.ibm.com>
The bounce buffer is not used in the ibmvnic driver, just
get rid of it.
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 53 ------------------------------------
drivers/net/ethernet/ibm/ibmvnic.h | 4 ---
2 files changed, 57 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 221d652..e8c72ab 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -502,50 +502,6 @@ static int init_tx_pools(struct net_device *netdev)
return 0;
}
-static void release_bounce_buffer(struct ibmvnic_adapter *adapter)
-{
- struct device *dev = &adapter->vdev->dev;
-
- if (!adapter->bounce_buffer)
- return;
-
- if (!dma_mapping_error(dev, adapter->bounce_buffer_dma)) {
- dma_unmap_single(dev, adapter->bounce_buffer_dma,
- adapter->bounce_buffer_size,
- DMA_BIDIRECTIONAL);
- adapter->bounce_buffer_dma = DMA_ERROR_CODE;
- }
-
- kfree(adapter->bounce_buffer);
- adapter->bounce_buffer = NULL;
-}
-
-static int init_bounce_buffer(struct net_device *netdev)
-{
- struct ibmvnic_adapter *adapter = netdev_priv(netdev);
- struct device *dev = &adapter->vdev->dev;
- char *buf;
- int buf_sz;
- dma_addr_t map_addr;
-
- buf_sz = (netdev->mtu + ETH_HLEN - 1) / PAGE_SIZE + 1;
- buf = kmalloc(adapter->bounce_buffer_size, GFP_KERNEL);
- if (!buf)
- return -1;
-
- map_addr = dma_map_single(dev, buf, buf_sz, DMA_TO_DEVICE);
- if (dma_mapping_error(dev, map_addr)) {
- dev_err(dev, "Couldn't map bounce buffer\n");
- kfree(buf);
- return -1;
- }
-
- adapter->bounce_buffer = buf;
- adapter->bounce_buffer_size = buf_sz;
- adapter->bounce_buffer_dma = map_addr;
- return 0;
-}
-
static void release_error_buffers(struct ibmvnic_adapter *adapter)
{
struct device *dev = &adapter->vdev->dev;
@@ -597,7 +553,6 @@ static int ibmvnic_login(struct net_device *netdev)
static void release_resources(struct ibmvnic_adapter *adapter)
{
- release_bounce_buffer(adapter);
release_tx_pools(adapter);
release_rx_pools(adapter);
@@ -656,10 +611,6 @@ static int ibmvnic_open(struct net_device *netdev)
if (rc)
goto ibmvnic_open_fail;
- rc = init_bounce_buffer(netdev);
- if (rc)
- goto ibmvnic_open_fail;
-
replenish_pools(adapter);
/* We're ready to receive frames, enable the sub-crq interrupts and
@@ -880,7 +831,6 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
unsigned int tx_bytes = 0;
dma_addr_t data_dma_addr;
struct netdev_queue *txq;
- bool used_bounce = false;
unsigned long lpar_rc;
union sub_crq tx_crq;
unsigned int offset;
@@ -921,7 +871,6 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
tx_buff->index = index;
tx_buff->pool_index = queue_num;
tx_buff->last_frag = true;
- tx_buff->used_bounce = used_bounce;
memset(&tx_crq, 0, sizeof(tx_crq));
tx_crq.v1.first = IBMVNIC_CRQ_CMD;
@@ -1517,7 +1466,6 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter *adapter,
continue;
txbuff->data_dma[j] = 0;
- txbuff->used_bounce = false;
}
/* if sub_crq was sent indirectly */
first = txbuff->indir_arr[0].generic.first;
@@ -3343,7 +3291,6 @@ static unsigned long ibmvnic_get_desired_dma(struct vio_dev *vdev)
adapter = netdev_priv(netdev);
ret += PAGE_SIZE; /* the crq message queue */
- ret += adapter->bounce_buffer_size;
ret += IOMMU_PAGE_ALIGN(sizeof(struct ibmvnic_statistics), tbl);
for (i = 0; i < adapter->req_tx_queues + adapter->req_rx_queues; i++)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index 8fbe05d..355225c 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -868,7 +868,6 @@ struct ibmvnic_tx_buff {
int index;
int pool_index;
bool last_frag;
- bool used_bounce;
union sub_crq indir_arr[6];
u8 hdr_data[140];
dma_addr_t indir_dma;
@@ -924,9 +923,6 @@ struct ibmvnic_adapter {
dma_addr_t ip_offload_ctrl_tok;
bool migrated;
u32 msg_enable;
- void *bounce_buffer;
- int bounce_buffer_size;
- dma_addr_t bounce_buffer_dma;
/* Statistics */
struct ibmvnic_statistics stats;
^ permalink raw reply related
* Re: [PATCH net 3/4] qed: Fix possible system hang in the dcbnl-getdcbx() path.
From: Lance Richardson @ 2017-04-19 13:56 UTC (permalink / raw)
To: Sudarsana Reddy Kalluru; +Cc: davem, netdev, Yuval Mintz
In-Reply-To: <20170419101955.26444-4-sudarsana.kalluru@cavium.com>
> From: "Sudarsana Reddy Kalluru" <sudarsana.kalluru@cavium.com>
> To: davem@davemloft.net
> Cc: netdev@vger.kernel.org, "Yuval Mintz" <Yuval.Mintz@cavium.com>
> Sent: Wednesday, 19 April, 2017 6:19:54 AM
> Subject: [PATCH net 3/4] qed: Fix possible system hang in the dcbnl-getdcbx() path.
>
> qed_dcbnl_get_dcbx() API uses kmalloc in GFT_KERNEL mode. The API gets
> invoked in the interrupt context by qed_dcbnl_getdcbx callback. Need
> to invoke this kmalloc in atomic mode.
>
> Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
> Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
> ---
> drivers/net/ethernet/qlogic/qed/qed_dcbx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_dcbx.c
> b/drivers/net/ethernet/qlogic/qed/qed_dcbx.c
> index ff058a3..8f0783a 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_dcbx.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_dcbx.c
> @@ -1264,7 +1264,7 @@ static struct qed_dcbx_get *qed_dcbnl_get_dcbx(struct
> qed_hwfn *hwfn,
> {
> struct qed_dcbx_get *dcbx_info;
>
> - dcbx_info = kzalloc(sizeof(*dcbx_info), GFP_KERNEL);
> + dcbx_info = kmalloc(sizeof(*dcbx_info), GFP_ATOMIC);
You are changing a kzalloc to kmalloc, was that intentional?
> if (!dcbx_info)
> return NULL;
>
> --
> 1.8.3.1
>
>
^ permalink raw reply
* Re: ath9k: Add cast to u8 to FREQ2FBIN macro
From: Kalle Valo @ 2017-04-19 14:01 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Kalle Valo, ath9k Development, linux-kernel, linux-wireless,
netdev, Grant Grundler, Matthias Kaehlcke
In-Reply-To: <20170406212135.72157-1-mka@chromium.org>
Matthias Kaehlcke <mka@chromium.org> wrote:
> The macro results are assigned to u8 variables/fields. Adding the cast
> fixes plenty of clang warnings about "implicit conversion from 'int' to
> 'u8'".
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Patch applied to ath-next branch of ath.git, thanks.
627871b71c89 ath9k: Add cast to u8 to FREQ2FBIN macro
--
https://patchwork.kernel.org/patch/9668479/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] net: phy: fix auto-negotiation stall due to unavailable interrupt
From: Alexander Kochetkov @ 2017-04-19 14:05 UTC (permalink / raw)
To: Florian Fainelli, netdev, linux-kernel
In-Reply-To: <1492609604-16359-1-git-send-email-al.kochet@gmail.com>
Just found similar problem fixed in another PHY. See commit 99f81afc139c
("phy: micrel: Disable auto negotiation on startup»)
> 19 апр. 2017 г., в 16:46, Alexander Kochetkov <al.kochet@gmail.com> написал(а):
>
> The problem I fix related to SMSC LAN8710/LAN8720 PHY handled using
> interrupts. During power-up cycle the PHY do auto-negotiation, generate
> interrupt and set BMSR_ANEGCOMPLETE flag. Interrupt is handled by PHY
> state machine but doesn't update link because PHY is in PHY_READY state.
> After some time MAC bring up and connect with PHY. It start PHY using
> phy_start(). During startup PHY change state to PHY_AN but doesn't
> set BMCR_ANRESTART flag due to genphy_config_aneg() doesn't update MII_BMCR
> because there no new to advertising. As a result, state machine wait for
> interrupt from PHY and nether get "link is up". Because BMSR_ANEGCOMPLETE
> already set the patch schedule check link without waiting interrupt.
> In case genphy_config_aneg() update MII_BMCR and set BMCR_ANRESTART
> flag, BMSR_ANEGCOMPLETE will be cleared and state machine will continue
> on auto-negotiation interrupt.
>
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> ---
> drivers/net/phy/phy.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 7cc1b7d..da8f03d 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -1169,6 +1169,18 @@ void phy_state_machine(struct work_struct *work)
> if (phydev->irq == PHY_POLL)
> queue_delayed_work(system_power_efficient_wq, &phydev->state_queue,
> PHY_STATE_TIME * HZ);
> +
> + /* Re-schedule a PHY state machine to check PHY status because
> + * negotiation already done and aneg interrupt may not be generated.
> + */
> + if (needs_aneg && (phydev->irq > 0) && (phydev->state == PHY_AN)) {
> + err = phy_aneg_done(phydev);
> + if (err > 0)
> + queue_delayed_work(system_power_efficient_wq,
> + &phydev->state_queue, 0);
> + if (err < 0)
> + phy_error(phydev);
> + }
> }
>
> /**
> --
> 1.7.9.5
>
^ permalink raw reply
* [PATCH net] ipv6: sr: fix double free of skb after handling invalid SRH
From: David Lebrun @ 2017-04-19 14:10 UTC (permalink / raw)
To: netdev; +Cc: David Lebrun, Dan Carpenter
The icmpv6_param_prob() function already does a kfree_skb(),
this patch removes the duplicate one.
Fixes: 1ababeba4a21f3dba3da3523c670b207fb2feb62 ("ipv6: implement dataplane support for rthdr type 4 (Segment Routing Header)")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: David Lebrun <david.lebrun@uclouvain.be>
---
net/ipv6/exthdrs.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index 275cac6..25192a3 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -388,7 +388,6 @@ static int ipv6_srh_rcv(struct sk_buff *skb)
icmpv6_param_prob(skb, ICMPV6_HDR_FIELD,
((&hdr->segments_left) -
skb_network_header(skb)));
- kfree_skb(skb);
return -1;
}
--
2.10.2
^ permalink raw reply related
* Re: [PATCH v4] smsc95xx: Use skb_cow_head to deal with cloned skbs
From: Eric Dumazet @ 2017-04-19 14:13 UTC (permalink / raw)
To: James Hughes; +Cc: netdev, Steve Glendinning, Microchip Linux Driver Support
In-Reply-To: <20170419101340.27929-1-james.hughes@raspberrypi.org>
On Wed, 2017-04-19 at 11:13 +0100, James Hughes wrote:
> The driver was failing to check that the SKB wasn't cloned
> before adding checksum data.
> Replace existing handling to extend/copy the header buffer
> with skb_cow_head.
>
> Signed-off-by: James Hughes <james.hughes@raspberrypi.org>
> ---
Acked-by: Eric Dumazet <edumazet@google.com>
Thanks James
^ permalink raw reply
* Re: [PATCH net-next v4 2/2] net sched actions: add time filter for action dumping
From: Jiri Pirko @ 2017-04-19 14:14 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: davem, netdev, eric.dumazet, xiyou.wangcong
In-Reply-To: <f1704aae-af9d-75eb-47f7-64950f2b841b@mojatatu.com>
Wed, Apr 19, 2017 at 03:11:26PM CEST, jhs@mojatatu.com wrote:
>On 17-04-19 08:53 AM, Jiri Pirko wrote:
>> Wed, Apr 19, 2017 at 01:57:30PM CEST, jhs@mojatatu.com wrote:
>> > From: Jamal Hadi Salim <jhs@mojatatu.com>
>> >
>> > This adds support for filtering based on time since last used.
>> > When we are dumping a large number of actions it is useful to
>> > have the option of filtering based on when the action was last
>> > used to reduce the amount of data crossing to user space.
>> >
>> > With this patch the user space app sets the TCAA_ACT_TIME_FILTER
>> > attribute with the value in milliseconds with "time of interest
>> > since now". The kernel converts this to jiffies and does the
>> > filtering comparison matching entries that have seen activity
>> > since then and returns them to user space.
>> > Old kernels and old tc continue to work in legacy mode since
>> > they dont specify this attribute.
>> >
>> > Some example (we have 400 actions bound to 400 filters); at installation
>> > time using hacked tc which sets the time of interest to 120 seconds:
>> >
>> > prompt$ hackedtc actions ls action gact | grep index | wc -l
>> > 400
>> >
>> > go get some coffee and wait for > 120 seconds and try again:
>> >
>> > prompt$ hackedtc actions ls action gact | grep index | wc -l
>> > 0
>> >
>> > Lets see a filter bound to one of these actions:
>> > ..
>> > filter pref 10 u32
>> > filter pref 10 u32 fh 800: ht divisor 1
>> > filter pref 10 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:10 (rule hit 2 success 1)
>> > match 7f000002/ffffffff at 12 (success 1 )
>> > action order 1: gact action pass
>> > random type none pass val 0
>> > index 23 ref 2 bind 1 installed 1145 sec used 802 sec
>> > Action statistics:
>> > Sent 84 bytes 1 pkt (dropped 0, overlimits 0 requeues 0)
>> > backlog 0b 0p requeues 0
>> > ....
>> >
>> > that coffee took long, no? It was good.
>> >
>> > Now lets ping -c 1 127.0.0.2, then run the actions again:
>> >
>> > prompt$ hackedtc actions ls action gact | grep index | wc -l
>> > 1
>> >
>> > More details please:
>> >
>> > prompt$ hackedtc -s actions ls action gact
>>
>> I don't see where you pass the time.
>>
>
>User space sets the TCAA_ACT_TIME_FILTER value.
>Jiri - this is described in the commit log ;->
>I have some tc changes which set this value.
>In the example below it was 120 seconds.
So it is hard coded. That confused me. I believe it would be good to
provide actual tc patch that does the proper changes and include the
example of that usage in the description - instead of "hackedtc"
>
>
>>
>> >
>> > action order 0: gact action pass
>> > random type none pass val 0
>> > index 23 ref 2 bind 1 installed 1270 sec used 30 sec
>> > Action statistics:
>> > Sent 168 bytes 2 pkt (dropped 0, overlimits 0 requeues 0)
>> > backlog 0b 0p requeues 0
>> >
>> > And the filter?
>> >
>> > filter pref 10 u32
>> > filter pref 10 u32 fh 800: ht divisor 1
>> > filter pref 10 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:10 (rule hit 4 success 2)
>> > match 7f000002/ffffffff at 12 (success 2 )
>> > action order 1: gact action pass
>> > random type none pass val 0
>> > index 23 ref 2 bind 1 installed 1324 sec used 84 sec
>> > Action statistics:
>> > Sent 168 bytes 2 pkt (dropped 0, overlimits 0 requeues 0)
>> > backlog 0b 0p requeues 0
>> >
>> > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>> > ---
>> > include/uapi/linux/rtnetlink.h | 1 +
>> > net/sched/act_api.c | 25 +++++++++++++++++++++++--
>> > 2 files changed, 24 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
>> > index c7080ec..1b36cc0 100644
>> > --- a/include/uapi/linux/rtnetlink.h
>> > +++ b/include/uapi/linux/rtnetlink.h
>> > @@ -680,6 +680,7 @@ enum {
>> > TCAA_ACT_TAB,
>> > TCAA_ACT_FLAGS,
>> > TCAA_ACT_COUNT,
>> > + TCAA_ACT_TIME_FILTER,
>>
>> Another use of word "filter" for something else. I believe that we need
>> to reduce confusion, not to make it worse.
>>
>
>It is a time filter. What do you want to call it?
TIME_DELTA?
>
>
>>
>> > __TCAA_MAX
>> > };
>> >
>> > diff --git a/net/sched/act_api.c b/net/sched/act_api.c
>> > index 45e1cf7..b03863a 100644
>> > --- a/net/sched/act_api.c
>> > +++ b/net/sched/act_api.c
>> > @@ -84,11 +84,12 @@ static int tcf_dump_walker(struct tcf_hashinfo *hinfo, struct sk_buff *skb,
>> > {
>> > int err = 0, index = -1, i = 0, s_i = 0, n_i = 0;
>> > unsigned short act_flags = cb->args[2];
>> > + unsigned long jiffy_filter = cb->args[3];
>>
>> call this "jiffy_since" for example
>>
>
>Sure.
>
>
>>
>> > struct nlattr *nest;
>> >
>> > spin_lock_bh(&hinfo->lock);
>> >
>> > - s_i = cb->args[0];
>> > + s_i = cb->args[4];
>> >
>> > for (i = 0; i < (hinfo->hmask + 1); i++) {
>> > struct hlist_head *head;
>> > @@ -101,6 +102,11 @@ static int tcf_dump_walker(struct tcf_hashinfo *hinfo, struct sk_buff *skb,
>> > if (index < s_i)
>> > continue;
>> >
>> > + if (jiffy_filter &&
>> > + time_after(jiffy_filter,
>> > + (unsigned long)p->tcfa_tm.lastuse))
>> > + continue;
>> > +
>> > nest = nla_nest_start(skb, n_i);
>> > if (nest == NULL)
>> > goto nla_put_failure;
>> > @@ -118,6 +124,9 @@ static int tcf_dump_walker(struct tcf_hashinfo *hinfo, struct sk_buff *skb,
>> > }
>> > }
>> > done:
>> > + if (index > 0)
>> > + cb->args[4] = index + 1;
>> > +
>> > spin_unlock_bh(&hinfo->lock);
>> > if (n_i) {
>> > cb->args[0] += n_i;
>>
>> You don't use "cb->args[0]" anymore. Why do you need this?
>
>You are right - will remove this.
>
>> Just use cb->args[0] instead of 4
>>
>
>I can do that too.
>
>
>>
>> > @@ -1000,6 +1009,7 @@ static int tcf_action_add(struct net *net, struct nlattr *nla,
>> >
>> > static const struct nla_policy tcaa_policy[TCAA_MAX + 1] = {
>> > [TCAA_ACT_FLAGS] = { .type = NLA_U32 },
>> > + [TCAA_ACT_TIME_FILTER] = { .type = NLA_U32 },
>> > };
>> >
>> > static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
>> > @@ -1090,13 +1100,14 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
>> > struct tcamsg *t = (struct tcamsg *) nlmsg_data(cb->nlh);
>> > struct nlattr *kind = NULL;
>> > u32 act_flags = 0;
>> > + u32 msecs_filter = 0;
>> > + unsigned long jiffy_wanted = 0;
>>
>> Also, "jiffy_since". Same variable, same name please.
>>
>
>np.
>
>> >
>> > ret = nlmsg_parse(cb->nlh, sizeof(struct tcamsg), tcaa, TCAA_MAX,
>> > tcaa_policy, NULL);
>> > if (ret < 0)
>> > return ret;
>> >
>> > -
>>
>> This should not be part of this patch.
>>
>
>Will remove.
>
>>
>> > kind = find_dump_kind(tcaa);
>> > if (kind == NULL) {
>> > pr_info("tc_dump_action: action bad kind\n");
>> > @@ -1110,12 +1121,22 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
>> > if (tcaa[TCAA_ACT_FLAGS])
>> > act_flags = nla_get_u32(tcaa[TCAA_ACT_FLAGS]);
>> >
>> > + if (tcaa[TCAA_ACT_TIME_FILTER])
>> > + msecs_filter = nla_get_u32(tcaa[TCAA_ACT_TIME_FILTER]);
>> > +
>> > nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
>> > cb->nlh->nlmsg_type, sizeof(*t), 0);
>> > if (!nlh)
>> > goto out_module_put;
>> >
>> > + if (msecs_filter) {
>> > + unsigned long jiffy_msecs = msecs_to_jiffies(msecs_filter);
>> > +
>> > + jiffy_wanted = jiffies - jiffy_msecs;
>>
>> you can do just:
>> jiffy_wanted = jiffies - msecs_to_jiffies(msecs_filter);
>>
>> Also, you can put this under "if (tcaa[TCAA_ACT_TIME_FILTER])" so it's
>> all in one place.
>>
>
>Will do - next cycle run.
Thanks.
>
>
>cheers,
>jamal
^ permalink raw reply
* RE: [PATCH net 3/4] qed: Fix possible system hang in the dcbnl-getdcbx() path.
From: Mintz, Yuval @ 2017-04-19 14:14 UTC (permalink / raw)
To: Lance Richardson, Kalluru, Sudarsana
Cc: davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <1873138594.16006381.1492610196115.JavaMail.zimbra@redhat.com>
> > qed_dcbnl_get_dcbx() API uses kmalloc in GFT_KERNEL mode. The API gets
> > invoked in the interrupt context by qed_dcbnl_getdcbx callback. Need
> > to invoke this kmalloc in atomic mode.
> >
> > Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
> > Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
> > ---
> > drivers/net/ethernet/qlogic/qed/qed_dcbx.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/qlogic/qed/qed_dcbx.c
> > b/drivers/net/ethernet/qlogic/qed/qed_dcbx.c
> > index ff058a3..8f0783a 100644
> > --- a/drivers/net/ethernet/qlogic/qed/qed_dcbx.c
> > +++ b/drivers/net/ethernet/qlogic/qed/qed_dcbx.c
> > @@ -1264,7 +1264,7 @@ static struct qed_dcbx_get
> > *qed_dcbnl_get_dcbx(struct qed_hwfn *hwfn, {
> > struct qed_dcbx_get *dcbx_info;
> >
> > - dcbx_info = kzalloc(sizeof(*dcbx_info), GFP_KERNEL);
> > + dcbx_info = kmalloc(sizeof(*dcbx_info), GFP_ATOMIC);
>
> You are changing a kzalloc to kmalloc, was that intentional?
Not really. Apparently the confusion resulted from the fact that
immediately after the next condition we're memsetting dcbx_info
to zero [so current code zeros the memory twice], which also
means there's no functional problem with this change.
Dave, do you want us to re-spin this [even though change isn't
functionally broken]?
Notice this is a recurrent pattern, so in future we'd have to send
a cleanup for these kind of stuff, regardless of whether we re-spin
now or not.
>
> > if (!dcbx_info)
> > return NULL;
> >
> > --
> > 1.8.3.1
> >
> >
^ permalink raw reply
* RE: Re: [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER
From: Gabriele Paoloni @ 2017-04-19 14:28 UTC (permalink / raw)
To: David Laight, davem@davemloft.net
Cc: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy,
jeffrey.t.kirsher@intel.com, alexander.duyck@gmail.com,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
Dingtianhong, Linuxarm
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DCFFD3C52@AcuExch.aculab.com>
Hi David
Many thanks for your reply
> -----Original Message-----
> From: David Laight [mailto:David.Laight@ACULAB.COM]
> Sent: 18 April 2017 14:26
> To: Gabriele Paoloni; davem@davemloft.net
> Cc: Catalin Marinas; Will Deacon; Mark Rutland; Robin Murphy;
> jeffrey.t.kirsher@intel.com; alexander.duyck@gmail.com; linux-arm-
> kernel@lists.infradead.org; netdev@vger.kernel.org; Dingtianhong;
> Linuxarm
> Subject: RE: Re: [PATCH net-next 1/4] ixgbe: sparc: rename the
> ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER
>
> From: Gabriele Paoloni
> > Sent: 13 April 2017 10:11
> > > > Till now only the Intel ixgbe could support enable
> > > > Relaxed ordering in the drivers for special architecture,
> > > > but the ARCH_WANT_RELAX_ORDER is looks like a general name
> > > > for all arch, so rename to a specific name for intel
> > > > card looks more appropriate.
> > > >
> > > > Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> > >
> > > This is not a driver specific facility.
> > >
> > > Any driver can test this symbol and act accordingly.
> > >
> > > Just because IXGBE is the first and only user, doesn't mean
> > > the facility is driver specific.
> >
> >
> > Please correct me if I am wrong but my understanding is that the
> standard
> > way to enable/disable relaxed ordering is to set/clear bit 4 of the
> Device
> > Control Register (PCI_EXP_DEVCTL_RELAX_EN 0x0010 /* Enable relaxed
> > ordering */).
> > Now I have looked up for all drivers either enabling or disabling
> relaxed
> > ordering and none of them seems to need a symbol to decide whether to
> > enable it or not.
> > Also it seems to me enabling/disabling relaxed ordering is never
> bound to the
> > host architecture.
> >
> > So why this should be (or it is expected to be) a generic symbol?
> > Wouldn't it be more correct to have this as a driver specific symbol
> now and
> > move it to a generic one later once we have other drivers requiring
> it?
>
> 'Relaxed ordering' is a bit in the TLP header.
> A device (like the ixgbe hardware) can set it for some transactions and
> still have the transactions 'ordered enough' for the driver to work.
> (If all transactions have 'relaxed ordering' set then I suspect it is
> almost impossible to write a working driver.)
> The host side could (probably does) have a bit to enable 'relaxed
> ordering',
> it could also enforce stronger ordering than required by the PCIe spec
> (eg keeping reads in order).
My understanding is that from the host side the host is always allowed
(as long as it complies with the rules specified in sec.2.4.1 of the PCIe
Specs) to set the RO attribute in the TLP and the target function should
be abel to cope with it.
On the device side the device is allowed to set the RO attribute in the
TLP only if bit4 of the "Device Control Register" is set.
>
> Clearly, on some sparc64 systems, ixgbe needs to use 'relaxed
> ordering'.
> To me this requires two separate bits be enabled:
> 1) to the ixgbe driver to generate the 'relaxed' TLP.
> 2) to the host to actually act on them.
My understanding is that for performance reasons when possible we
should enable relaxed ordering and I think this is up to the host
(i.e. the host somehow should know when he is capable of handling
RO TLPs and therefore it will try to enable it on the driver)
> If the ixgbe driver works when both are enabled why have options to
> disable either (except for bug-finding)?
I think that by default the ixgbe driver disable RO since there are
issues with "some chipsets" according to commit 3d5c520727ce "ixgbe:
move disabling of relaxed ordering in start_hw()".
What this means is a bit obscure to me and seems to be not related to
the host architecture
Also looking at where and why the other drivers set/clear the "Enable
Relaxed Ordering" bit it seems that currently this is not tied to the
host architecture nor to any global symbol; instead it seems purely
dependent on the PCIe device chipset itself.
>
> David
^ permalink raw reply
* Re: [PATCH v4 net-next RFC] net: Generic XDP
From: Andy Gospodarek @ 2017-04-19 14:29 UTC (permalink / raw)
To: David Miller; +Cc: alexei.starovoitov, michael.chan, netdev, xdp-newbies
In-Reply-To: <20170418.152916.1361453741909754079.davem@davemloft.net>
On Tue, Apr 18, 2017 at 03:29:16PM -0400, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Tue, 18 Apr 2017 15:07:08 -0400 (EDT)
>
> > From: Andy Gospodarek <andy@greyhouse.net>
> > Date: Tue, 18 Apr 2017 15:05:35 -0400
> >
> >> On Fri, Apr 14, 2017 at 05:59:51PM -0700, Alexei Starovoitov wrote:
> >>> On Thu, Apr 13, 2017 at 04:23:15PM -0400, David Miller wrote:
> >>> > +
> >>> > + switch (act) {
> >>> > + case XDP_TX:
> >>> > + __skb_push(skb, skb->mac_len);
> >>>
> >>> s/skb->mac_len/mac_len/
> >>>
> >>
> >> I was away from my keyboard for a few days, but was able to get some
> >> time to test this today.
> >>
> >> When using this change above suggested by Alexei, XDP_DROP and XDP_TX
> >> actions appear to work well with xdp1 and xdp2.
> >>
> >> I'm seeing some rather odd behavior with xdp_tx_tunnel so it might be
> >> good to hold off on committing this just yet.
> >>
> >> At first glance, it looks like there is enough headroom for the new
> >> frame, but the resulting packet data do not look right and I'm actually
> >> seeing some data that may be left on the stack from a previous caller.
> >
> > Thanks for testing Andy, I'll take a look and see if I can figure it out.
>
> Andy, I think we might be getting burnt by signedness issues in the
> offset handling when the XDP program adjusts the packet data pointer.
>
> In netif_receive_generic_xdp(), try changing the offset handling code to
> read something like:
>
> off = xdp.data - orig_data;
> if (off > 0)
> __skb_pull(skb, off);
> else if (off < 0)
> __skb_push(skb, -off);
>
> If that doesn't work try adding:
>
> __skb_cow(skb, XDP_PACKET_HEADROOM, 0);
>
> right after the skb_linearize() call in that same function.
So I tried a variety of things and the simplest change on top of yours that
works well for xdp1, xdp2, and xdp_tx_iptunnel.
diff --git a/net/core/dev.c b/net/core/dev.c
index b3d3a6e..1bab3dc 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4316,11 +4316,11 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
off = xdp.data - orig_data;
if (off)
- __skb_push(skb, off);
+ __skb_push(skb, -off);
switch (act) {
case XDP_TX:
- __skb_push(skb, skb->mac_len);
+ __skb_push(skb, mac_len);
/* fall through */
case XDP_PASS:
break;
I ran this on top of a card that uses the bnxt_en driver on a desktop
class system with an i7-6700 CPU @ 3.40GHz, sending a single stream of
UDP traffic with flow control disabled and saw the following (all stats
in Million PPS).
xdp1 xdp2 xdp_tx_tunnel
Generic XDP 7.8 5.5 (1.3 actual) 4.6 (1.1 actual)
Optimized XDP 11.7 9.7 4.6
One thing to note is that the Generic XDP case shows some different
results for reported by the application vs actual (seen on the wire). I
did not debug where the drops are happening and what counter needs to be
incremented to note this -- I'll add that to my TODO list. The
Optimized XDP case does not have a difference in reported vs actual
frames on the wire.
I agree with all those who have asserted that this is great tool for
those that want to get started with XDP but do not have hardware, so I'd
say it's ready to have the 'RFC' tag dropped. Thanks for pushing this
forward, Dave! :-)
^ permalink raw reply related
* [PATCH] net: arc_emac: switch to phy_start()/phy_stop()
From: Alexander Kochetkov @ 2017-04-19 14:29 UTC (permalink / raw)
To: David S. Miller, Philippe Reynes, Peter Chen, Wei Yongjun, netdev,
linux-kernel
Cc: Alexander Kochetkov
The patch replace phy_start_aneg() with phy_start(). phy_start() call
phy_start_aneg() as a part of startup sequence and allow recover from
error (PHY_HALTED) state.
Also added call phy_stop() to arc_emac_remove() to stop PHY state machine
when MAC is down.
Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
drivers/net/ethernet/arc/emac_main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/arc/emac_main.c b/drivers/net/ethernet/arc/emac_main.c
index abc9f2a..188676d 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -434,7 +434,7 @@ static int arc_emac_open(struct net_device *ndev)
/* Enable EMAC */
arc_reg_or(priv, R_CTRL, EN_MASK);
- phy_start_aneg(ndev->phydev);
+ phy_start(ndev->phydev);
netif_start_queue(ndev);
@@ -556,6 +556,8 @@ static int arc_emac_stop(struct net_device *ndev)
napi_disable(&priv->napi);
netif_stop_queue(ndev);
+ phy_stop(ndev->phydev);
+
/* Disable interrupts */
arc_reg_clr(priv, R_ENABLE, RXINT_MASK | TXINT_MASK | ERR_MASK);
--
1.7.9.5
^ permalink raw reply related
* RE: [PATCH v4] smsc95xx: Use skb_cow_head to deal with cloned skbs
From: Woojung.Huh @ 2017-04-19 14:36 UTC (permalink / raw)
To: james.hughes, netdev; +Cc: steve.glendinning, UNGLinuxDriver
In-Reply-To: <20170419101340.27929-1-james.hughes@raspberrypi.org>
> The driver was failing to check that the SKB wasn't cloned
> before adding checksum data.
> Replace existing handling to extend/copy the header buffer
> with skb_cow_head.
>
> Signed-off-by: James Hughes <james.hughes@raspberrypi.org>
> ---
Acked-by: Woojung Huh <Woojung.Huh@microchip.com>
^ permalink raw reply
* RE: Re: [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER
From: Gabriele Paoloni @ 2017-04-19 14:46 UTC (permalink / raw)
To: Amir Ancel, David Laight, davem@davemloft.net
Cc: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy,
jeffrey.t.kirsher@intel.com, alexander.duyck@gmail.com,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
Dingtianhong, Linuxarm
In-Reply-To: <EE11001F9E5DDD47B7634E2F8A612F2E205352FC@FRAEML521-MBX.china.huawei.com>
Hi Amir
> From: Amir Ancel [mailto:amira@mellanox.com]
> Sent: 18 April 2017 21:18
> To: David Laight; Gabriele Paoloni; davem@davemloft.net
> Cc: Catalin Marinas; Will Deacon; Mark Rutland; Robin Murphy;
> jeffrey.t.kirsher@intel.com; alexander.duyck@gmail.com; linux-arm-
> kernel@lists.infradead.org; netdev@vger.kernel.org; Dingtianhong;
> Linuxarm
> Subject: Re: Re: [PATCH net-next 1/4] ixgbe: sparc: rename the
> ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER
>
> Hi,
> mlx5 driver is planned to have RO support this year.
> I believe drivers should be able to query whether the arch support it
I guess that here when you say query you mean having a config symbol
that is set accordingly to the host architecture, right?
As already said I have looked around a bit and other drivers do not seem
to enable/disable RO for their EP on the basis of the host architecture.
So why should mlx5 do it according to the host?
Also my understating is that some architectures (like ARM64 for example)
can have different PCI host controller implementations depending on the
vendor...therefore maybe it is not appropriate there to have a Kconfig
symbol selected by the architecture...
Thanks
Gab
> or not and enable it in the network adapter accordingly.
>
> -Amir
> ________________________________________
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> on
> behalf of David Laight <David.Laight@ACULAB.COM>
> Sent: Tuesday, April 18, 2017 4:25:44 PM
> To: 'Gabriele Paoloni'; davem@davemloft.net
> Cc: Catalin Marinas; Will Deacon; Mark Rutland; Robin Murphy;
> jeffrey.t.kirsher@intel.com; alexander.duyck@gmail.com; linux-arm-
> kernel@lists.infradead.org; netdev@vger.kernel.org; Dingtianhong;
> Linuxarm
> Subject: RE: Re: [PATCH net-next 1/4] ixgbe: sparc: rename the
> ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER
>
> From: Gabriele Paoloni
> > Sent: 13 April 2017 10:11
> > > > Till now only the Intel ixgbe could support enable
> > > > Relaxed ordering in the drivers for special architecture,
> > > > but the ARCH_WANT_RELAX_ORDER is looks like a general name
> > > > for all arch, so rename to a specific name for intel
> > > > card looks more appropriate.
> > > >
> > > > Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> > >
> > > This is not a driver specific facility.
> > >
> > > Any driver can test this symbol and act accordingly.
> > >
> > > Just because IXGBE is the first and only user, doesn't mean
> > > the facility is driver specific.
> >
> >
> > Please correct me if I am wrong but my understanding is that the
> standard
> > way to enable/disable relaxed ordering is to set/clear bit 4 of the
> Device
> > Control Register (PCI_EXP_DEVCTL_RELAX_EN 0x0010 /* Enable relaxed
> > ordering */).
> > Now I have looked up for all drivers either enabling or disabling
> relaxed
> > ordering and none of them seems to need a symbol to decide whether to
> > enable it or not.
> > Also it seems to me enabling/disabling relaxed ordering is never
> bound to the
> > host architecture.
> >
> > So why this should be (or it is expected to be) a generic symbol?
> > Wouldn't it be more correct to have this as a driver specific symbol
> now and
> > move it to a generic one later once we have other drivers requiring
> it?
>
> 'Relaxed ordering' is a bit in the TLP header.
> A device (like the ixgbe hardware) can set it for some transactions and
> still have the transactions 'ordered enough' for the driver to work.
> (If all transactions have 'relaxed ordering' set then I suspect it is
> almost impossible to write a working driver.)
> The host side could (probably does) have a bit to enable 'relaxed
> ordering',
> it could also enforce stronger ordering than required by the PCIe spec
> (eg keeping reads in order).
>
> Clearly, on some sparc64 systems, ixgbe needs to use 'relaxed
> ordering'.
> To me this requires two separate bits be enabled:
> 1) to the ixgbe driver to generate the 'relaxed' TLP.
> 2) to the host to actually act on them.
> If the ixgbe driver works when both are enabled why have options to
> disable either (except for bug-finding)?
>
> David
^ permalink raw reply
* Re: [PATCH v2 net 2/2] net sched actions: decrement module refcount earlier
From: Wolfgang Bumiller @ 2017-04-19 15:03 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang
Cc: Linux Kernel Network Developers, David S. Miller
In-Reply-To: <861dba23-1244-ee57-a980-a6dceffa2793@mojatatu.com>
> On April 19, 2017 at 1:32 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
>
> On 17-04-19 04:09 AM, Wolfgang Bumiller wrote:
>
> This solves one issue, but I am afraid the issue Cong mentioned is a
> possibility still.
> Lets say user did a replace and tried to also replace the cookie
> in that transaction. The init() succeeds but the cookie allocation
> fails. To be correct we'll have to undo the replace i.e something
> like uninit() which will restore back the old values.
> This is very complex and unnecessary.
>
> My suggestion:
> If we move the cookie allocation before init we can save it and
> only when init succeeds do we attach it to the action, otherwise
> we free it on error path.
Shouldn't the old code have freed an old a->act_cookie as well before
replacing it then? nla_memdup_cookie() starts off by assigning a->act_cookie.
(I've been running this loop for a while now:
# while : ; do tc actions change action ok index 500 cookie $i; let i++; done
and memory usage *seems* to be growing faster with the loop running - still
slow, but visible. (I stopped most but not all background processes in this
VM))
I don't see the growth with the change below (replacing both patches).
(although given the freeing of the old act_cookie pointer I wonder if
this needs additional locking?)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index b70aa57319ea..e05b924618a0 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -529,20 +529,20 @@ int tcf_action_dump(struct sk_buff *skb, struct list_head *actions,
return err;
}
-static int nla_memdup_cookie(struct tc_action *a, struct nlattr **tb)
+static struct tc_cookie *nla_memdup_cookie(struct nlattr **tb)
{
- a->act_cookie = kzalloc(sizeof(*a->act_cookie), GFP_KERNEL);
- if (!a->act_cookie)
- return -ENOMEM;
+ struct tc_cookie *c = kzalloc(sizeof(*c), GFP_KERNEL);
+ if (!c)
+ return NULL;
- a->act_cookie->data = nla_memdup(tb[TCA_ACT_COOKIE], GFP_KERNEL);
- if (!a->act_cookie->data) {
- kfree(a->act_cookie);
- return -ENOMEM;
+ c->data = nla_memdup(tb[TCA_ACT_COOKIE], GFP_KERNEL);
+ if (!c->data) {
+ kfree(c);
+ return NULL;
}
- a->act_cookie->len = nla_len(tb[TCA_ACT_COOKIE]);
+ c->len = nla_len(tb[TCA_ACT_COOKIE]);
- return 0;
+ return c;
}
struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla,
@@ -551,6 +551,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla,
{
struct tc_action *a;
struct tc_action_ops *a_o;
+ struct tc_cookie *cookie = NULL;
char act_name[IFNAMSIZ];
struct nlattr *tb[TCA_ACT_MAX + 1];
struct nlattr *kind;
@@ -566,6 +567,18 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla,
goto err_out;
if (nla_strlcpy(act_name, kind, IFNAMSIZ) >= IFNAMSIZ)
goto err_out;
+ if (tb[TCA_ACT_COOKIE]) {
+ int cklen = nla_len(tb[TCA_ACT_COOKIE]);
+
+ if (cklen > TC_COOKIE_MAX_SIZE)
+ goto err_out;
+
+ cookie = nla_memdup_cookie(tb);
+ if (!cookie) {
+ err = -ENOMEM;
+ goto err_out;
+ }
+ }
} else {
err = -EINVAL;
if (strlcpy(act_name, name, IFNAMSIZ) >= IFNAMSIZ)
@@ -604,20 +617,12 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla,
if (err < 0)
goto err_mod;
- if (tb[TCA_ACT_COOKIE]) {
- int cklen = nla_len(tb[TCA_ACT_COOKIE]);
-
- if (cklen > TC_COOKIE_MAX_SIZE) {
- err = -EINVAL;
- tcf_hash_release(a, bind);
- goto err_mod;
- }
-
- if (nla_memdup_cookie(a, tb) < 0) {
- err = -ENOMEM;
- tcf_hash_release(a, bind);
- goto err_mod;
+ if (name == NULL && tb[TCA_ACT_COOKIE]) {
+ if (a->act_cookie) {
+ kfree(a->act_cookie->data);
+ kfree(a->act_cookie);
}
+ a->act_cookie = cookie;
}
/* module count goes up only when brand new policy is created
@@ -632,6 +637,10 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla,
err_mod:
module_put(a_o->owner);
err_out:
+ if (cookie) {
+ kfree(cookie->data);
+ kfree(cookie);
+ }
return ERR_PTR(err);
}
^ permalink raw reply related
* Re: [PATCH net-next v3] bindings: net: stmmac: add missing note about LPI interrupt
From: Joao Pinto @ 2017-04-19 15:22 UTC (permalink / raw)
To: Niklas Cassel, Rob Herring, Mark Rutland, David S. Miller,
Joao Pinto, Niklas Cassel, Alexandre TORGUE, Giuseppe CAVALLARO,
Thierry Reding, Eric Engestrom
Cc: netdev, devicetree, linux-kernel
In-Reply-To: <20170418123955.21335-1-niklass@axis.com>
Hi Niklas,
Às 1:39 PM de 4/18/2017, Niklas Cassel escreveu:
> From: Niklas Cassel <niklas.cassel@axis.com>
>
> The hardware has a LPI interrupt.
> There is already code in the stmmac driver to parse and handle the
> interrupt. However, this information was missing from the DT binding.
>
> At the same time, improve the description of the existing interrupts.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
> Documentation/devicetree/bindings/net/stmmac.txt | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
> index f652b0c384ce..c3a7be6615c5 100644
> --- a/Documentation/devicetree/bindings/net/stmmac.txt
> +++ b/Documentation/devicetree/bindings/net/stmmac.txt
> @@ -7,9 +7,12 @@ Required properties:
> - interrupt-parent: Should be the phandle for the interrupt controller
> that services interrupts for this device
> - interrupts: Should contain the STMMAC interrupts
> -- interrupt-names: Should contain the interrupt names "macirq"
> - "eth_wake_irq" if this interrupt is supported in the "interrupts"
> - property
> +- interrupt-names: Should contain a list of interrupt names corresponding to
> + the interrupts in the interrupts property, if available.
> + Valid interrupt names are:
> + - "macirq" (combined signal for various interrupt events)
> + - "eth_wake_irq" (the interrupt to manage the remote wake-up packet detection)
> + - "eth_lpi" (the interrupt that occurs when Tx or Rx enters/exits LPI state)
> - phy-mode: See ethernet.txt file in the same directory.
> - snps,reset-gpio gpio number for phy reset.
> - snps,reset-active-low boolean flag to indicate if phy reset is active low.
> @@ -152,8 +155,8 @@ Examples:
> compatible = "st,spear600-gmac";
> reg = <0xe0800000 0x8000>;
> interrupt-parent = <&vic1>;
> - interrupts = <24 23>;
> - interrupt-names = "macirq", "eth_wake_irq";
> + interrupts = <24 23 22>;
> + interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
> mac-address = [000000000000]; /* Filled in by U-Boot */
> max-frame-size = <3800>;
> phy-mode = "gmii";
>
Acked-By: Joao Pinto <jpinto@synopsys.com>
Regards!
^ 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