* [PATCH net-next 05/11] ibmvnic: Non-fatal error handling
From: Nathan Fontenot @ 2017-05-26 14:30 UTC (permalink / raw)
To: netdev, tlfalcon, jallen
In-Reply-To: <20170526142523.63648.62938.stgit@ltcalpine2-lp14.aus.stglabs.ibm.com>
From: John Allen <jallen@linux.vnet.ibm.com>
Handle non-fatal error conditions. The process to do this when
resetting the driver is to just do __ibmvnic_close followed by
__ibmvnic_open.
Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 56 ++++++++++++++++++++----------------
drivers/net/ethernet/ibm/ibmvnic.h | 1 +
2 files changed, 32 insertions(+), 25 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 0f705e6..def867a 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1225,37 +1225,41 @@ static int do_reset(struct ibmvnic_adapter *adapter,
if (rc)
return rc;
- /* remove the closed state so when we call open it appears
- * we are coming from the probed state.
- */
- adapter->state = VNIC_PROBED;
+ if (adapter->reset_reason != VNIC_RESET_NON_FATAL) {
+ /* remove the closed state so when we call open it appears
+ * we are coming from the probed state.
+ */
+ adapter->state = VNIC_PROBED;
- release_resources(adapter);
- release_sub_crqs(adapter);
- release_crq_queue(adapter);
+ release_resources(adapter);
+ release_sub_crqs(adapter);
+ release_crq_queue(adapter);
- rc = ibmvnic_init(adapter);
- if (rc)
- return 0;
+ rc = ibmvnic_init(adapter);
+ if (rc)
+ return 0;
- /* If the adapter was in PROBE state prior to the reset, exit here. */
- if (reset_state == VNIC_PROBED)
- return 0;
+ /* If the adapter was in PROBE state prior to the reset,
+ * exit here.
+ */
+ if (reset_state == VNIC_PROBED)
+ return 0;
- rc = ibmvnic_login(netdev);
- if (rc) {
- adapter->state = VNIC_PROBED;
- return 0;
- }
+ rc = ibmvnic_login(netdev);
+ if (rc) {
+ adapter->state = VNIC_PROBED;
+ return 0;
+ }
- rtnl_lock();
- rc = init_resources(adapter);
- rtnl_unlock();
- if (rc)
- return rc;
+ rtnl_lock();
+ rc = init_resources(adapter);
+ rtnl_unlock();
+ if (rc)
+ return rc;
- if (reset_state == VNIC_CLOSED)
- return 0;
+ if (reset_state == VNIC_CLOSED)
+ return 0;
+ }
rc = __ibmvnic_open(netdev);
if (rc) {
@@ -2763,6 +2767,8 @@ static void handle_error_indication(union ibmvnic_crq *crq,
if (crq->error_indication.flags & IBMVNIC_FATAL_ERROR)
ibmvnic_reset(adapter, VNIC_RESET_FATAL);
+ else
+ ibmvnic_reset(adapter, VNIC_RESET_NON_FATAL);
}
static void handle_change_mac_rsp(union ibmvnic_crq *crq,
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index fa6ac4e..7e2300e 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -925,6 +925,7 @@ enum vnic_state {VNIC_PROBING = 1,
enum ibmvnic_reset_reason {VNIC_RESET_FAILOVER = 1,
VNIC_RESET_MOBILITY,
VNIC_RESET_FATAL,
+ VNIC_RESET_NON_FATAL,
VNIC_RESET_TIMEOUT};
struct ibmvnic_rwi {
^ permalink raw reply related
* [PATCH net-next 06/11] ibmvnic: Halt TX and report carrier off on H_CLOSED return code
From: Nathan Fontenot @ 2017-05-26 14:30 UTC (permalink / raw)
To: netdev, tlfalcon, jallen
In-Reply-To: <20170526142523.63648.62938.stgit@ltcalpine2-lp14.aus.stglabs.ibm.com>
From: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
This patch disables transmissions and reports carrier off if xmit
function returns that the hardware TX queue is closed. The driver can
then await a signal from firmware to determine the correct reset method.
Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index def867a..1c3f1ed 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1111,8 +1111,14 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
dev_kfree_skb_any(skb);
tx_buff->skb = NULL;
- if (lpar_rc == H_CLOSED)
- netif_stop_subqueue(netdev, queue_num);
+ if (lpar_rc == H_CLOSED) {
+ /* Disable TX and report carrier off if queue is closed.
+ * Firmware guarantees that a signal will be sent to the
+ * driver, triggering a reset or some other action.
+ */
+ netif_tx_stop_all_queues(netdev);
+ netif_carrier_off(netdev);
+ }
tx_send_failed++;
tx_dropped++;
^ permalink raw reply related
* [PATCH net-next 07/11] ibmvnic: Deactivate RX pool buffer replenishment on H_CLOSED
From: Nathan Fontenot @ 2017-05-26 14:30 UTC (permalink / raw)
To: netdev, tlfalcon, jallen
In-Reply-To: <20170526142523.63648.62938.stgit@ltcalpine2-lp14.aus.stglabs.ibm.com>
From: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
If H_CLOSED is returned, halt RX buffer replenishment activity
until firmware sends a notification that the driver can reset.
Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 1c3f1ed..47421e4 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -200,6 +200,15 @@ static void free_long_term_buff(struct ibmvnic_adapter *adapter,
dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
}
+static void deactivate_rx_pools(struct ibmvnic_adapter *adapter)
+{
+ int i;
+
+ for (i = 0; i < be32_to_cpu(adapter->login_rsp_buf->num_rxadd_subcrqs);
+ i++)
+ adapter->rx_pool[i].active = 0;
+}
+
static void replenish_rx_pool(struct ibmvnic_adapter *adapter,
struct ibmvnic_rx_pool *pool)
{
@@ -217,6 +226,9 @@ static void replenish_rx_pool(struct ibmvnic_adapter *adapter,
int index;
int i;
+ if (!pool->active)
+ return;
+
handle_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
be32_to_cpu(adapter->login_rsp_buf->
off_rxadd_subcrqs));
@@ -287,6 +299,15 @@ static void replenish_rx_pool(struct ibmvnic_adapter *adapter,
dev_kfree_skb_any(skb);
adapter->replenish_add_buff_failure++;
atomic_add(buffers_added, &pool->available);
+
+ if (lpar_rc == H_CLOSED) {
+ /* Disable buffer pool replenishment and report carrier off if
+ * queue is closed. Firmware guarantees that a signal will
+ * be sent to the driver, triggering a reset.
+ */
+ deactivate_rx_pools(adapter);
+ netif_carrier_off(adapter->netdev);
+ }
}
static void replenish_pools(struct ibmvnic_adapter *adapter)
^ permalink raw reply related
* [PATCH net-next 08/11] ibmvnic: Check adapter state during ibmvnic_poll
From: Nathan Fontenot @ 2017-05-26 14:30 UTC (permalink / raw)
To: netdev, tlfalcon, jallen
In-Reply-To: <20170526142523.63648.62938.stgit@ltcalpine2-lp14.aus.stglabs.ibm.com>
We do not want to process any receive frames if the ibmvnic_poll
routine is invoked while a reset is in process. Also, before
replenishing the rx pools in the ibmvnic_poll, we want to
make sure the adapter is not in the process of closing.
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 47421e4..760352f 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1435,6 +1435,10 @@ static int ibmvnic_poll(struct napi_struct *napi, int budget)
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
int scrq_num = (int)(napi - adapter->napi);
int frames_processed = 0;
+
+ if (adapter->resetting)
+ return 0;
+
restart_poll:
while (frames_processed < budget) {
struct sk_buff *skb;
@@ -1493,7 +1497,9 @@ static int ibmvnic_poll(struct napi_struct *napi, int budget)
netdev->stats.rx_bytes += length;
frames_processed++;
}
- replenish_rx_pool(adapter, &adapter->rx_pool[scrq_num]);
+
+ if (adapter->state != VNIC_CLOSING)
+ replenish_rx_pool(adapter, &adapter->rx_pool[scrq_num]);
if (frames_processed < budget) {
enable_scrq_irq(adapter, adapter->rx_scrq[scrq_num]);
^ permalink raw reply related
* [PATCH net-next 09/11] ibmvnic: Reset the CRQ queue during driver reset
From: Nathan Fontenot @ 2017-05-26 14:31 UTC (permalink / raw)
To: netdev, tlfalcon, jallen
In-Reply-To: <20170526142523.63648.62938.stgit@ltcalpine2-lp14.aus.stglabs.ibm.com>
When a driver reset operation occurs there is not a need to release
the CRQ resources and re-allocate them. Instead a reset of the CRQ
will suffice.
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 760352f..b9b0c69 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1260,7 +1260,6 @@ static int do_reset(struct ibmvnic_adapter *adapter,
release_resources(adapter);
release_sub_crqs(adapter);
- release_crq_queue(adapter);
rc = ibmvnic_init(adapter);
if (rc)
@@ -3517,7 +3516,14 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
unsigned long timeout = msecs_to_jiffies(30000);
int rc;
- rc = init_crq_queue(adapter);
+ if (adapter->resetting) {
+ rc = ibmvnic_reset_crq(adapter);
+ if (!rc)
+ rc = vio_enable_interrupts(adapter->vdev);
+ } else {
+ rc = init_crq_queue(adapter);
+ }
+
if (rc) {
dev_err(dev, "Couldn't initialize crq. rc=%d\n", rc);
return rc;
^ permalink raw reply related
* [PATCH net-next 10/11] ibmvnic: Reset tx/rx pools on driver reset
From: Nathan Fontenot @ 2017-05-26 14:31 UTC (permalink / raw)
To: netdev, tlfalcon, jallen
In-Reply-To: <20170526142523.63648.62938.stgit@ltcalpine2-lp14.aus.stglabs.ibm.com>
When resetting the ibmvnic driver there is not a need to release
and re-allocate the resources for the tx and rx pools. These
resources can just be reset to avoid the re-allocations.
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 71 ++++++++++++++++++++++++++++++++++--
1 file changed, 67 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index b9b0c69..5661a04 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -163,6 +163,16 @@ static long h_reg_sub_crq(unsigned long unit_address, unsigned long token,
return rc;
}
+static void reset_long_term_buff(struct ibmvnic_adapter *adapter,
+ struct ibmvnic_long_term_buff *ltb)
+{
+ memset(ltb->buff, 0, ltb->size);
+
+ init_completion(&adapter->fw_done);
+ send_request_map(adapter, ltb->addr, ltb->size, ltb->map_id);
+ wait_for_completion(&adapter->fw_done);
+}
+
static int alloc_long_term_buff(struct ibmvnic_adapter *adapter,
struct ibmvnic_long_term_buff *ltb, int size)
{
@@ -352,6 +362,32 @@ static int init_stats_token(struct ibmvnic_adapter *adapter)
return 0;
}
+static int reset_rx_pools(struct ibmvnic_adapter *adapter)
+{
+ struct ibmvnic_rx_pool *rx_pool;
+ int rx_scrqs;
+ int i, j;
+
+ rx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_rxadd_subcrqs);
+ for (i = 0; i < rx_scrqs; i++) {
+ rx_pool = &adapter->rx_pool[i];
+
+ reset_long_term_buff(adapter, &rx_pool->long_term_buff);
+
+ for (j = 0; j < rx_pool->size; j++)
+ rx_pool->free_map[j] = j;
+
+ memset(rx_pool->rx_buff, 0,
+ rx_pool->size * sizeof(struct ibmvnic_rx_buff));
+
+ atomic_set(&rx_pool->available, 0);
+ rx_pool->next_alloc = 0;
+ rx_pool->next_free = 0;
+ }
+
+ return 0;
+}
+
static void release_rx_pools(struct ibmvnic_adapter *adapter)
{
struct ibmvnic_rx_pool *rx_pool;
@@ -453,6 +489,32 @@ static int init_rx_pools(struct net_device *netdev)
return 0;
}
+static int reset_tx_pools(struct ibmvnic_adapter *adapter)
+{
+ struct ibmvnic_tx_pool *tx_pool;
+ int tx_scrqs;
+ int i, j;
+
+ tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
+ for (i = 0; i < tx_scrqs; i++) {
+ tx_pool = &adapter->tx_pool[i];
+
+ reset_long_term_buff(adapter, &tx_pool->long_term_buff);
+
+ memset(tx_pool->tx_buff, 0,
+ adapter->req_tx_entries_per_subcrq *
+ sizeof(struct ibmvnic_tx_buff));
+
+ for (j = 0; j < adapter->req_tx_entries_per_subcrq; j++)
+ tx_pool->free_map[j] = j;
+
+ tx_pool->consumer_index = 0;
+ tx_pool->producer_index = 0;
+ }
+
+ return 0;
+}
+
static void release_tx_pools(struct ibmvnic_adapter *adapter)
{
struct ibmvnic_tx_pool *tx_pool;
@@ -1258,7 +1320,6 @@ static int do_reset(struct ibmvnic_adapter *adapter,
*/
adapter->state = VNIC_PROBED;
- release_resources(adapter);
release_sub_crqs(adapter);
rc = ibmvnic_init(adapter);
@@ -1277,9 +1338,11 @@ static int do_reset(struct ibmvnic_adapter *adapter,
return 0;
}
- rtnl_lock();
- rc = init_resources(adapter);
- rtnl_unlock();
+ rc = reset_tx_pools(adapter);
+ if (rc)
+ return rc;
+
+ rc = reset_rx_pools(adapter);
if (rc)
return rc;
^ permalink raw reply related
* [PATCH net-next 11/11] ibmvnic: Reset sub-crqs during driver reset
From: Nathan Fontenot @ 2017-05-26 14:31 UTC (permalink / raw)
To: netdev, tlfalcon, jallen
In-Reply-To: <20170526142523.63648.62938.stgit@ltcalpine2-lp14.aus.stglabs.ibm.com>
When the ibmvnic driver is resetting, we can just reset the sub crqs
instead of releasing all of their resources and re-allocting them.
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 46 ++++++++++++++++++++++++++++++++++--
1 file changed, 43 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 5661a04..8dcf580 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1320,8 +1320,6 @@ static int do_reset(struct ibmvnic_adapter *adapter,
*/
adapter->state = VNIC_PROBED;
- release_sub_crqs(adapter);
-
rc = ibmvnic_init(adapter);
if (rc)
return 0;
@@ -1728,6 +1726,45 @@ static void ibmvnic_get_ethtool_stats(struct net_device *dev,
/* Routines for managing CRQs/sCRQs */
+static int reset_one_sub_crq_queue(struct ibmvnic_adapter *adapter,
+ struct ibmvnic_sub_crq_queue *scrq)
+{
+ int rc;
+
+ if (scrq->irq) {
+ free_irq(scrq->irq, scrq);
+ irq_dispose_mapping(scrq->irq);
+ scrq->irq = 0;
+ }
+
+ memset(scrq->msgs, 0, 2 * PAGE_SIZE);
+ scrq->cur = 0;
+
+ rc = h_reg_sub_crq(adapter->vdev->unit_address, scrq->msg_token,
+ 4 * PAGE_SIZE, &scrq->crq_num, &scrq->hw_irq);
+ return rc;
+}
+
+static int reset_sub_crq_queues(struct ibmvnic_adapter *adapter)
+{
+ int i, rc;
+
+ for (i = 0; i < adapter->req_tx_queues; i++) {
+ rc = reset_one_sub_crq_queue(adapter, adapter->tx_scrq[i]);
+ if (rc)
+ return rc;
+ }
+
+ for (i = 0; i < adapter->req_rx_queues; i++) {
+ rc = reset_one_sub_crq_queue(adapter, adapter->rx_scrq[i]);
+ if (rc)
+ return rc;
+ }
+
+ rc = init_sub_crq_irqs(adapter);
+ return rc;
+}
+
static void release_sub_crq_queue(struct ibmvnic_adapter *adapter,
struct ibmvnic_sub_crq_queue *scrq)
{
@@ -3607,7 +3644,10 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
return -1;
}
- rc = init_sub_crqs(adapter);
+ if (adapter->resetting)
+ rc = reset_sub_crq_queues(adapter);
+ else
+ rc = init_sub_crqs(adapter);
if (rc) {
dev_err(dev, "Initialization of sub crqs failed\n");
release_crq_queue(adapter);
^ permalink raw reply related
* Re: please revert. Was: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova
From: David Miller @ 2017-05-26 14:51 UTC (permalink / raw)
To: alexei.starovoitov-Re5JQEeQqe8AvxtiuMwx3w
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170526044057.qpk3stulth5tfxcx@ast-mbp>
From: Alexei Starovoitov <alexei.starovoitov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Thu, 25 May 2017 21:40:59 -0700
> On Fri, May 26, 2017 at 12:13:27AM -0400, David Miller wrote:
>> From: Alexei Starovoitov <alexei.starovoitov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Date: Thu, 25 May 2017 20:58:32 -0700
>>
>> > Dave, please revert this Innova fpga stuff.
>> > I think you pushed it by accident, since it was mixed with
>> > other valid changes.
>> > The discussion didn't conclude.
>> > Myself and Jes are clearly against such changes.
>> > It definitely needs more discussion and wider consensus.
>>
>> Why don't you finish your discussion, then I can revert or leave it in
>> there?
>
> Not really. What you're saying is 'shut up. mellanox can do
> whatever they like as long as it's hidden behind pcie id'.
No, what I'm saying is, discuss things.
And if in the end you agree with Mellanox, we don't have flapping of
the change in and out of the tree over and over.
My understanding is that this FPGA offloads processing the data
stream, which in a way is not much different than JIT'ing eBPF
onto Netronome cpus. :-)
Maybe the programming interface is different, maybe all the algorithms
are implemented in discrete ASICs, but effect is quite similar.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [Patch net-next] net_sched: only create filter chains for new filters/actions
From: David Miller @ 2017-05-26 14:54 UTC (permalink / raw)
To: jiri; +Cc: xiyou.wangcong, netdev, jhs, jiri
In-Reply-To: <20170526055352.GA1898@nanopsycho>
From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 26 May 2017 07:53:52 +0200
> Thu, May 25, 2017 at 06:14:56PM CEST, davem@davemloft.net wrote:
>>From: Cong Wang <xiyou.wangcong@gmail.com>
>>Date: Tue, 23 May 2017 09:42:37 -0700
>>
>>> tcf_chain_get() always creates a new filter chain if not found
>>> in existing ones. This is totally unnecessary when we get or
>>> delete filters, new chain should be only created for new filters
>>> (or new actions).
>>>
>>> Fixes: 5bc1701881e3 ("net: sched: introduce multichain support for filters")
>>> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
>>> Cc: Jiri Pirko <jiri@mellanox.com>
>>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>>
>>Indeed, get and delete requests should not create new objects, ever.
>>
>>I have pretty much no idea why Jiri is making such a big fuss about
>>this change, to be quite honest. :-)
>
> Because it makes already hard to read code even worse, for *no* benefit.
> That's why.
Jiri, if you say the same thing 100 times, it doesn't help anyone
understand your arguments any better.
Creating new objects when a GET or a DEL is requested is flat out
wrong.
And Cong is fixing that.
And I also didn't find the boolean logic hard to understand at all.
It is in fact a very common pattern to pass a "create" boolean into
lookup functions, to tell them whether to create a new object on
lookup failure or not. And then also to control that boolean via
what kind of netlink request we are processing.
So you tell me what's so bad about his change given the above?
Give me details and real facts, like I just did, rather than vague
statements about "benefit" and "hard to read".
Thank you.
^ permalink raw reply
* Re: [PATCH v2 net-next 1/3] perf, bpf: Add BPF support to all perf_event types
From: David Miller @ 2017-05-26 14:55 UTC (permalink / raw)
To: ast; +Cc: peterz, bgregg, daniel, qinteng, netdev, linux-kernel
In-Reply-To: <20170526055549.557818-2-ast@fb.com>
From: Alexei Starovoitov <ast@fb.com>
Date: Thu, 25 May 2017 22:55:47 -0700
> + if (event->attach_state & PERF_ATTACH_TASK)
> + goto err_out;
The attach_state member only exists when PERF_EVENTS is enabled.
^ permalink raw reply
* Re: [RFC] skbuff: introduce skb_put_zero()
From: David Miller @ 2017-05-26 14:58 UTC (permalink / raw)
To: johannes-cdvu00un1VgdHxzADdlk8Q
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1495781929.2572.0.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
From: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
Date: Fri, 26 May 2017 08:58:49 +0200
> On Thu, 2017-05-25 at 13:01 -0400, David Miller wrote:
>> From: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
>> Date: Wed, 24 May 2017 09:07:47 +0200
>>
>> > From: Johannes Berg <johannes.berg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> >
>> > This pattern was introduced a number of times in mac80211 just now,
>> > and since it's present in a number of other places it makes sense
>> > to add a little helper for it.
>> >
>> > The transformations of all code were done with the following
>> spatch:
> [...]
>> > NB: the patch won't apply to net-next because a few of the mac80211
>> > users don't exist there yet
>>
>> Acked-by: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
>>
>> No objections, feel free to push this in via your mac80211 tree.
>
> Do you think I should include the conversions in other drivers, or
> would you prefer to get those as a separate patch later?
Probably better to keep that as a separate patch.
^ permalink raw reply
* Re: [patch net-next] net/sched: let chain_get to figure out the return value
From: David Miller @ 2017-05-26 14:59 UTC (permalink / raw)
To: jiri; +Cc: netdev, jhs, xiyou.wangcong, mlxsw
In-Reply-To: <20170526072129.11075-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 26 May 2017 09:21:29 +0200
> From: Jiri Pirko <jiri@mellanox.com>
>
> Alhough I believe that this create/nocreate dance is completelly
> pointless, at least make it a bit nicer and easier to read.
> Push the decision on what error value is returned to chain_get function
> and use ERR macros.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
No, this is quite worse.
You're leaving pointer error values in structures. That's extremely
error prone.
And as stated in the other thread, I don't think Cong's logic is strange
or hard to understand at all.
^ permalink raw reply
* Re: [PATCH net-next v2 0/6] nfp: devlink port implementation
From: David Miller @ 2017-05-26 15:00 UTC (permalink / raw)
To: jakub.kicinski; +Cc: netdev, jiri, oss-drivers
In-Reply-To: <20170526080336.32689-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 26 May 2017 01:03:30 -0700
> This series adds basic devlink support. The operations we can perform
> are port show and port split/unsplit.
>
> v2:
> Register devlink first, and then register all the ports. Port {,un}split
> searches the port list, which is protected by a mutex. If port split
> is requested before ports are registered we will simply not find the port
> and return -EINVAL.
Great, I'll give Jiri a chance to review this.
^ permalink raw reply
* Re: [PATCH net-next v2 0/6] nfp: devlink port implementation
From: David Miller @ 2017-05-26 15:02 UTC (permalink / raw)
To: jiri; +Cc: jakub.kicinski, netdev, oss-drivers
In-Reply-To: <20170526082532.GC1898@nanopsycho>
From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 26 May 2017 10:25:32 +0200
> Fri, May 26, 2017 at 10:03:30AM CEST, jakub.kicinski@netronome.com wrote:
>>Hi!
>>
>>This series adds basic devlink support. The operations we can perform
>>are port show and port split/unsplit.
>>
>>v2:
>>Register devlink first, and then register all the ports. Port {,un}split
>>searches the port list, which is protected by a mutex. If port split
>>is requested before ports are registered we will simply not find the port
>>and return -EINVAL.
>
> Looks fine to me now. Thanks!
Wow, you review stuff fast :-)
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH 5/5] MIPS: Add support for eBPF JIT.
From: Daniel Borkmann @ 2017-05-26 15:14 UTC (permalink / raw)
To: David Daney, Alexei Starovoitov, netdev, linux-kernel, linux-mips,
ralf
Cc: Markos Chandras
In-Reply-To: <20170526003826.10834-6-david.daney@cavium.com>
On 05/26/2017 02:38 AM, David Daney wrote:
> Since the eBPF machine has 64-bit registers, we only support this in
> 64-bit kernels. As of the writing of this commit log test-bpf is showing:
>
> test_bpf: Summary: 316 PASSED, 0 FAILED, [308/308 JIT'ed]
>
> All current test cases are successfully compiled.
>
> Signed-off-by: David Daney <david.daney@cavium.com>
Awesome work!
Did you also manage to run tools/testing/selftests/bpf/ fine with
the JIT enabled?
[...]
> +struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
> +{
> + struct jit_ctx ctx;
> + unsigned int alloc_size;
> +
> + /* Only 64-bit kernel supports eBPF */
> + if (!IS_ENABLED(CONFIG_64BIT) || !bpf_jit_enable)
Isn't this already reflected by the following?
select HAVE_EBPF_JIT if (64BIT && !CPU_MICROMIPS)
> + return prog;
> +
> + memset(&ctx, 0, sizeof(ctx));
> +
> + ctx.offsets = kcalloc(prog->len + 1, sizeof(*ctx.offsets), GFP_KERNEL);
> + if (ctx.offsets == NULL)
> + goto out;
> +
> + ctx.reg_val_types = kcalloc(prog->len + 1, sizeof(*ctx.reg_val_types), GFP_KERNEL);
> + if (ctx.reg_val_types == NULL)
> + goto out;
> +
> + ctx.skf = prog;
> +
> + if (reg_val_propagate(&ctx))
> + goto out;
> +
> + /* First pass discovers used resources */
> + if (build_int_body(&ctx))
> + goto out;
> +
> + /* Second pass generates offsets */
> + ctx.idx = 0;
> + if (gen_int_prologue(&ctx))
> + goto out;
> + if (build_int_body(&ctx))
> + goto out;
> + if (build_int_epilogue(&ctx))
> + goto out;
> +
> + alloc_size = 4 * ctx.idx;
> +
> + ctx.target = module_alloc(alloc_size);
You would need to use bpf_jit_binary_alloc() like all other
eBPF JITs do, otherwise kallsyms of the JITed progs would
break.
> + if (ctx.target == NULL)
> + goto out;
> +
> + /* Clean it */
> + memset(ctx.target, 0, alloc_size);
> +
> + /* Third pass generates the code */
> + ctx.idx = 0;
> + if (gen_int_prologue(&ctx))
> + goto out;
> + if (build_int_body(&ctx))
> + goto out;
> + if (build_int_epilogue(&ctx))
> + goto out;
> + /* Update the icache */
> + flush_icache_range((ptr)ctx.target, (ptr)(ctx.target + ctx.idx));
> +
> + if (bpf_jit_enable > 1)
> + /* Dump JIT code */
> + bpf_jit_dump(prog->len, alloc_size, 2, ctx.target);
> +
> + prog->bpf_func = (void *)ctx.target;
> + prog->jited = 1;
> +
> +out:
> + kfree(ctx.offsets);
> + kfree(ctx.reg_val_types);
> +
> + return prog;
> +}
^ permalink raw reply
* Re: [PATCH net-next 3/4] tls: kernel TLS support
From: David Miller @ 2017-05-26 15:18 UTC (permalink / raw)
To: eric.dumazet
Cc: davejwatson, ilyal, aviadye, borisp, liranl, matanb, netdev, tom,
herbert, linux-crypto, hannes, alexei.starovoitov, nmav,
fridolin.pokorny
In-Reply-To: <1495808219.6465.118.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 26 May 2017 07:16:59 -0700
> On Wed, 2017-05-24 at 09:27 -0700, Dave Watson wrote:
>> Software implementation of transport layer security, implemented using ULP
>> infrastructure. tcp proto_ops are replaced with tls equivalents of sendmsg and
>> sendpage.
>
> ...
>
>> +
>> +int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
>> +{
> ...
>> +
>> + lock_sock(sk);
>> +
>> + /* Only one writer at a time is allowed */
>> + if (sk->sk_write_pending)
>> + return -EBUSY;
>
> Ouch...
Well, as I understand it, it is the same restriction userspace must
itself enforce either in the application or in the SSL library.
^ permalink raw reply
* Re: [PATCH 5/5] MIPS: Add support for eBPF JIT.
From: David Miller @ 2017-05-26 15:29 UTC (permalink / raw)
To: david.daney
Cc: ast, daniel, netdev, linux-kernel, linux-mips, ralf,
markos.chandras
In-Reply-To: <20170526003826.10834-6-david.daney@cavium.com>
From: David Daney <david.daney@cavium.com>
Date: Thu, 25 May 2017 17:38:26 -0700
> +static int gen_int_prologue(struct jit_ctx *ctx)
> +{
> + int stack_adjust = 0;
> + int store_offset;
> + int locals_size;
> +
> + if (ctx->flags & EBPF_SAVE_RA)
> + /*
> + * If RA we are doing a function call and may need
> + * extra 8-byte tmp area.
> + */
> + stack_adjust += 16;
> + if (ctx->flags & EBPF_SAVE_S0)
> + stack_adjust += 8;
> + if (ctx->flags & EBPF_SAVE_S1)
> + stack_adjust += 8;
> + if (ctx->flags & EBPF_SAVE_S2)
> + stack_adjust += 8;
> + if (ctx->flags & EBPF_SAVE_S3)
> + stack_adjust += 8;
> +
> + BUILD_BUG_ON(MAX_BPF_STACK & 7);
> + locals_size = (ctx->flags & EBPF_SEEN_FP) ? MAX_BPF_STACK : 0;
You will also need to use MAX_BPF_STACK here when you see a tail call,
but it appears you haven't implemented tail call support yet.
Which also several of the eBPF samples won't JIT and thus be tested
under this new MIPS JIT, since they make use of tail calls.
> +/*
> + * Track the value range (i.e. 32-bit vs. 64-bit) of each register at
> + * each eBPF insn. This allows unneeded sign and zero extension
> + * operations to be omitted.
> + *
> + * Doesn't handle yet confluence of control paths with conflicting
> + * ranges, but it is good enough for most sane code.
> + */
> +static int reg_val_propagate(struct jit_ctx *ctx)
Very interesting technique. I may adopt this for Sparc as well :-)
Perhaps at a some point, when the BPF verifier has real data flow
analysis, it can compute this for the JIT.
^ permalink raw reply
* Re: [PATCH 5/5] MIPS: Add support for eBPF JIT.
From: Daniel Borkmann @ 2017-05-26 15:35 UTC (permalink / raw)
To: David Daney, Alexei Starovoitov, netdev, linux-kernel, linux-mips,
ralf
Cc: Markos Chandras
In-Reply-To: <5928463C.5000204@iogearbox.net>
On 05/26/2017 05:14 PM, Daniel Borkmann wrote:
> On 05/26/2017 02:38 AM, David Daney wrote:
>> Since the eBPF machine has 64-bit registers, we only support this in
>> 64-bit kernels. As of the writing of this commit log test-bpf is showing:
>>
>> test_bpf: Summary: 316 PASSED, 0 FAILED, [308/308 JIT'ed]
>>
>> All current test cases are successfully compiled.
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>
> Awesome work!
>
> Did you also manage to run tools/testing/selftests/bpf/ fine with
> the JIT enabled?
>
> [...]
>> +struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>> +{
>> + struct jit_ctx ctx;
>> + unsigned int alloc_size;
>> +
>> + /* Only 64-bit kernel supports eBPF */
>> + if (!IS_ENABLED(CONFIG_64BIT) || !bpf_jit_enable)
>
> Isn't this already reflected by the following?
>
> select HAVE_EBPF_JIT if (64BIT && !CPU_MICROMIPS)
Oh, overlooked that you keep both JITs in the same file. ppc and
sparc also carry cBPF JITs, but strictly separated at compile time,
x86 threw out the cBPF one and only uses eBPF. Have you considered
separating them as well (which the current model assumes right now)?
(Need to double check all assumption we currently make and whether
they would still hold, but separation like all others do would
definitely be preferred.)
>> + return prog;
>> +
>> + memset(&ctx, 0, sizeof(ctx));
>> +
>> + ctx.offsets = kcalloc(prog->len + 1, sizeof(*ctx.offsets), GFP_KERNEL);
>> + if (ctx.offsets == NULL)
>> + goto out;
>> +
>> + ctx.reg_val_types = kcalloc(prog->len + 1, sizeof(*ctx.reg_val_types), GFP_KERNEL);
>> + if (ctx.reg_val_types == NULL)
>> + goto out;
>> +
>> + ctx.skf = prog;
>> +
>> + if (reg_val_propagate(&ctx))
>> + goto out;
>> +
>> + /* First pass discovers used resources */
>> + if (build_int_body(&ctx))
>> + goto out;
>> +
>> + /* Second pass generates offsets */
>> + ctx.idx = 0;
>> + if (gen_int_prologue(&ctx))
>> + goto out;
>> + if (build_int_body(&ctx))
>> + goto out;
>> + if (build_int_epilogue(&ctx))
>> + goto out;
>> +
>> + alloc_size = 4 * ctx.idx;
>> +
>> + ctx.target = module_alloc(alloc_size);
>
> You would need to use bpf_jit_binary_alloc() like all other
> eBPF JITs do, otherwise kallsyms of the JITed progs would
> break.
>
>> + if (ctx.target == NULL)
>> + goto out;
>> +
>> + /* Clean it */
>> + memset(ctx.target, 0, alloc_size);
>> +
>> + /* Third pass generates the code */
>> + ctx.idx = 0;
>> + if (gen_int_prologue(&ctx))
>> + goto out;
>> + if (build_int_body(&ctx))
>> + goto out;
>> + if (build_int_epilogue(&ctx))
>> + goto out;
>> + /* Update the icache */
>> + flush_icache_range((ptr)ctx.target, (ptr)(ctx.target + ctx.idx));
>> +
>> + if (bpf_jit_enable > 1)
>> + /* Dump JIT code */
>> + bpf_jit_dump(prog->len, alloc_size, 2, ctx.target);
>> +
>> + prog->bpf_func = (void *)ctx.target;
>> + prog->jited = 1;
>> +
>> +out:
>> + kfree(ctx.offsets);
>> + kfree(ctx.reg_val_types);
>> +
>> + return prog;
>> +}
^ permalink raw reply
* Re: [PATCH 5/5] MIPS: Add support for eBPF JIT.
From: David Daney @ 2017-05-26 15:39 UTC (permalink / raw)
To: Daniel Borkmann, David Daney, Alexei Starovoitov, netdev,
linux-kernel, linux-mips, ralf
Cc: Markos Chandras
In-Reply-To: <5928463C.5000204@iogearbox.net>
On 05/26/2017 08:14 AM, Daniel Borkmann wrote:
> On 05/26/2017 02:38 AM, David Daney wrote:
>> Since the eBPF machine has 64-bit registers, we only support this in
>> 64-bit kernels. As of the writing of this commit log test-bpf is
>> showing:
>>
>> test_bpf: Summary: 316 PASSED, 0 FAILED, [308/308 JIT'ed]
>>
>> All current test cases are successfully compiled.
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>
> Awesome work!
>
> Did you also manage to run tools/testing/selftests/bpf/ fine with
> the JIT enabled?
I haven't done that yet, I will before the next revision.
>
> [...]
>> +struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>> +{
>> + struct jit_ctx ctx;
>> + unsigned int alloc_size;
>> +
>> + /* Only 64-bit kernel supports eBPF */
>> + if (!IS_ENABLED(CONFIG_64BIT) || !bpf_jit_enable)
>
> Isn't this already reflected by the following?
>
> select HAVE_EBPF_JIT if (64BIT && !CPU_MICROMIPS)
Not exactly. The eBPF JIT is in the same file as the classic-BPF JIT,
so when HAVE_EBPF_JIT is false this will indeed never be called. But
the kernel would otherwise contain all the JIT code.
By putting in !IS_ENABLED(CONFIG_64BIT) we allow gcc to eliminate all
the dead code when compiling the JITs.
>
>> + return prog;
>> +
>> + memset(&ctx, 0, sizeof(ctx));
>> +
>> + ctx.offsets = kcalloc(prog->len + 1, sizeof(*ctx.offsets),
>> GFP_KERNEL);
>> + if (ctx.offsets == NULL)
>> + goto out;
>> +
>> + ctx.reg_val_types = kcalloc(prog->len + 1,
>> sizeof(*ctx.reg_val_types), GFP_KERNEL);
>> + if (ctx.reg_val_types == NULL)
>> + goto out;
>> +
>> + ctx.skf = prog;
>> +
>> + if (reg_val_propagate(&ctx))
>> + goto out;
>> +
>> + /* First pass discovers used resources */
>> + if (build_int_body(&ctx))
>> + goto out;
>> +
>> + /* Second pass generates offsets */
>> + ctx.idx = 0;
>> + if (gen_int_prologue(&ctx))
>> + goto out;
>> + if (build_int_body(&ctx))
>> + goto out;
>> + if (build_int_epilogue(&ctx))
>> + goto out;
>> +
>> + alloc_size = 4 * ctx.idx;
>> +
>> + ctx.target = module_alloc(alloc_size);
>
> You would need to use bpf_jit_binary_alloc() like all other
> eBPF JITs do, otherwise kallsyms of the JITed progs would
> break.
OK, I was just copying code from the classic-BPF JIT in the same file.
I will fix this.
>
>> + if (ctx.target == NULL)
>> + goto out;
>> +
>> + /* Clean it */
>> + memset(ctx.target, 0, alloc_size);
>> +
>> + /* Third pass generates the code */
>> + ctx.idx = 0;
>> + if (gen_int_prologue(&ctx))
>> + goto out;
>> + if (build_int_body(&ctx))
>> + goto out;
>> + if (build_int_epilogue(&ctx))
>> + goto out;
>> + /* Update the icache */
>> + flush_icache_range((ptr)ctx.target, (ptr)(ctx.target + ctx.idx));
>> +
>> + if (bpf_jit_enable > 1)
>> + /* Dump JIT code */
>> + bpf_jit_dump(prog->len, alloc_size, 2, ctx.target);
>> +
>> + prog->bpf_func = (void *)ctx.target;
>> + prog->jited = 1;
>> +
>> +out:
>> + kfree(ctx.offsets);
>> + kfree(ctx.reg_val_types);
>> +
>> + return prog;
>> +}
>
^ permalink raw reply
* Re: [PATCH net] sky2: Do not deadlock on sky2_hw_down
From: Stephen Hemminger @ 2017-05-26 15:43 UTC (permalink / raw)
To: David Miller; +Cc: jemele, jemele, netdev, jemele, mlindner, linux-kernel
In-Reply-To: <20170525.235830.986562840550889246.davem@davemloft.net>
On Thu, 25 May 2017 23:58:30 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Thu, 25 May 2017 15:05:02 -0700
>
> > Ok, the issue is that lockdep is being stupid and thinking that
> > seqcount's behave like locks.
>
> Well.. they do. That's why they have that annotation.
Your right, but it has some lock like properties because
the seqcount's assumption about odd and even values.
Lockdep is reporting that accessing a sequence count with softirq disabled
and in another case with softirq enabled is a problem.
Potential race theoretical race:
seqcount_begin() ++count
...
---> IRQ
---> Soft IRQ
seqcount_begin() ++count
update stats
seqcount_end() ++count
Anything doing seqcount read during this softirq
will spin thinking still in critical section
But for this case of statistics, there is nothing reading statistics in softirq
context so it can't happen. Lockdep needs to be smarter to know that.
Simplest way to shut this up is to just disable softirq during the cleanup.
^ permalink raw reply
* Re: [PATCH net-next 3/4] tls: kernel TLS support
From: Eric Dumazet @ 2017-05-26 15:49 UTC (permalink / raw)
To: David Miller
Cc: davejwatson, ilyal, aviadye, borisp, liranl, matanb, netdev, tom,
herbert, linux-crypto, hannes, alexei.starovoitov, nmav,
fridolin.pokorny
In-Reply-To: <20170526.111852.235696953256744250.davem@davemloft.net>
On Fri, 2017-05-26 at 11:18 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 26 May 2017 07:16:59 -0700
>
> > On Wed, 2017-05-24 at 09:27 -0700, Dave Watson wrote:
> >> Software implementation of transport layer security, implemented using ULP
> >> infrastructure. tcp proto_ops are replaced with tls equivalents of sendmsg and
> >> sendpage.
> >
> > ...
> >
> >> +
> >> +int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
> >> +{
> > ...
> >> +
> >> + lock_sock(sk);
> >> +
> >> + /* Only one writer at a time is allowed */
> >> + if (sk->sk_write_pending)
> >> + return -EBUSY;
> >
> > Ouch...
>
> Well, as I understand it, it is the same restriction userspace must
> itself enforce either in the application or in the SSL library.
The problem here is to lock_sock(sk), then returning without releasing
the socket.
Some basic lock imbalance really.
^ permalink raw reply
* [PATCH v2 0/6] stmmac: pci: Refactor DMI probing
From: Jan Kiszka @ 2017-05-26 16:07 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, David Miller
Cc: netdev, linux-kernel, Andy Shevchenko
Some cleanups of the way we probe DMI platforms in the driver. Reduces
a bit of open-coding and makes the logic easier reusable for any
potential DMI platform != Quark.
Tested on IOT2000 and Galileo Gen2.
Changes in v2:
- fixed bug that broke x86-64 build (and likely more)
- refactored series to do smaller steps
All this remains cosmetic from a certain distance, but the result looks
more appealing, at least to me.
Jan
Jan Kiszka (6):
stmmac: pci: Make stmmac_pci_info structure constant
stmmac: pci: Use stmmac_pci_info for all devices
stmmac: pci: Make stmmac_pci_find_phy_addr truly generic
stmmac: pci: Select quark_pci_dmi_data from quark_default_data
stmmac: pci: Use dmi_system_id table for retrieving PHY addresses
stmmac: pci: Remove setup handler indirection via stmmac_pci_info
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 184 ++++++++++++-----------
1 file changed, 98 insertions(+), 86 deletions(-)
--
2.12.0
^ permalink raw reply
* [PATCH v2 1/6] stmmac: pci: Make stmmac_pci_info structure constant
From: Jan Kiszka @ 2017-05-26 16:07 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, David Miller
Cc: netdev, linux-kernel, Andy Shevchenko
In-Reply-To: <cover.1495814872.git.jan.kiszka@siemens.com>
By removing the PCI device reference from the structure and passing it
as parameters to the interested functions, we can make quark_pci_info
const.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 22f910795be4..0efe42659a37 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -38,17 +38,17 @@ struct stmmac_pci_dmi_data {
};
struct stmmac_pci_info {
- struct pci_dev *pdev;
- int (*setup)(struct plat_stmmacenet_data *plat,
- struct stmmac_pci_info *info);
+ int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat,
+ const struct stmmac_pci_info *info);
struct stmmac_pci_dmi_data *dmi;
};
-static int stmmac_pci_find_phy_addr(struct stmmac_pci_info *info)
+static int stmmac_pci_find_phy_addr(struct pci_dev *pdev,
+ const struct stmmac_pci_info *info)
{
const char *name = dmi_get_system_info(DMI_BOARD_NAME);
const char *asset_tag = dmi_get_system_info(DMI_BOARD_ASSET_TAG);
- unsigned int func = PCI_FUNC(info->pdev->devfn);
+ unsigned int func = PCI_FUNC(pdev->devfn);
struct stmmac_pci_dmi_data *dmi;
/*
@@ -114,10 +114,10 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat)
/* TODO: AXI */
}
-static int quark_default_data(struct plat_stmmacenet_data *plat,
- struct stmmac_pci_info *info)
+static int quark_default_data(struct pci_dev *pdev,
+ struct plat_stmmacenet_data *plat,
+ const struct stmmac_pci_info *info)
{
- struct pci_dev *pdev = info->pdev;
int ret;
/* Set common default data first */
@@ -127,7 +127,7 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
* Refuse to load the driver and register net device if MAC controller
* does not connect to any PHY interface.
*/
- ret = stmmac_pci_find_phy_addr(info);
+ ret = stmmac_pci_find_phy_addr(pdev, info);
if (ret < 0)
return ret;
@@ -175,7 +175,7 @@ static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
{}
};
-static struct stmmac_pci_info quark_pci_info = {
+static const struct stmmac_pci_info quark_pci_info = {
.setup = quark_default_data,
.dmi = quark_pci_dmi_data,
};
@@ -237,9 +237,8 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
pci_set_master(pdev);
if (info) {
- info->pdev = pdev;
if (info->setup) {
- ret = info->setup(plat, info);
+ ret = info->setup(pdev, plat, info);
if (ret)
return ret;
}
--
2.12.0
^ permalink raw reply related
* [PATCH v2 3/6] stmmac: pci: Make stmmac_pci_find_phy_addr truly generic
From: Jan Kiszka @ 2017-05-26 16:07 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, David Miller
Cc: netdev, linux-kernel, Andy Shevchenko
In-Reply-To: <cover.1495814872.git.jan.kiszka@siemens.com>
Move the special case for the early Galileo firmware into
quark_default_setup. This allows to use stmmac_pci_find_phy_addr for
non-quark cases.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 9aca14f8b55e..1a89fa9ee39d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -51,12 +51,8 @@ static int stmmac_pci_find_phy_addr(struct pci_dev *pdev,
unsigned int func = PCI_FUNC(pdev->devfn);
struct stmmac_pci_dmi_data *dmi;
- /*
- * Galileo boards with old firmware don't support DMI. We always return
- * 1 here, so at least first found MAC controller would be probed.
- */
if (!name)
- return 1;
+ return -ENODEV;
for (dmi = info->dmi; dmi->name && *dmi->name; dmi++) {
if (!strcmp(dmi->name, name) && dmi->func == func) {
@@ -136,8 +132,17 @@ static int quark_default_data(struct pci_dev *pdev,
* does not connect to any PHY interface.
*/
ret = stmmac_pci_find_phy_addr(pdev, info);
- if (ret < 0)
- return ret;
+ if (ret < 0) {
+ /*
+ * Galileo boards with old firmware don't support DMI. We always
+ * use 1 here as PHY address, so at least the first found MAC
+ * controller would be probed.
+ */
+ if (!dmi_get_system_info(DMI_BOARD_NAME))
+ ret = 1;
+ else
+ return ret;
+ }
plat->bus_id = PCI_DEVID(pdev->bus->number, pdev->devfn);
plat->phy_addr = ret;
--
2.12.0
^ permalink raw reply related
* [PATCH v2 4/6] stmmac: pci: Select quark_pci_dmi_data from quark_default_data
From: Jan Kiszka @ 2017-05-26 16:07 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, David Miller
Cc: netdev, linux-kernel, Andy Shevchenko
In-Reply-To: <cover.1495814872.git.jan.kiszka@siemens.com>
No need to carry this reference in stmmac_pci_info - the Quark-specific
setup handler knows that it needs to use the Quark-specific DMI table.
This also allows to drop the stmmac_pci_info reference from the setup
handler parameter list.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 83 +++++++++++-------------
1 file changed, 39 insertions(+), 44 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 1a89fa9ee39d..07af42531fd4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -38,13 +38,11 @@ struct stmmac_pci_dmi_data {
};
struct stmmac_pci_info {
- int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat,
- const struct stmmac_pci_info *info);
- struct stmmac_pci_dmi_data *dmi;
+ int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat);
};
static int stmmac_pci_find_phy_addr(struct pci_dev *pdev,
- const struct stmmac_pci_info *info)
+ struct stmmac_pci_dmi_data *dmi_data)
{
const char *name = dmi_get_system_info(DMI_BOARD_NAME);
const char *asset_tag = dmi_get_system_info(DMI_BOARD_ASSET_TAG);
@@ -54,7 +52,7 @@ static int stmmac_pci_find_phy_addr(struct pci_dev *pdev,
if (!name)
return -ENODEV;
- for (dmi = info->dmi; dmi->name && *dmi->name; dmi++) {
+ for (dmi = dmi_data; dmi->name && *dmi->name; dmi++) {
if (!strcmp(dmi->name, name) && dmi->func == func) {
/* If asset tag is provided, match on it as well. */
if (dmi->asset_tag && strcmp(dmi->asset_tag, asset_tag))
@@ -97,8 +95,7 @@ static void common_default_data(struct plat_stmmacenet_data *plat)
}
static int stmmac_default_data(struct pci_dev *pdev,
- struct plat_stmmacenet_data *plat,
- const struct stmmac_pci_info *info)
+ struct plat_stmmacenet_data *plat)
{
/* Set common default data first */
common_default_data(plat);
@@ -118,9 +115,40 @@ static const struct stmmac_pci_info stmmac_pci_info = {
.setup = stmmac_default_data,
};
+static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
+ {
+ .name = "Galileo",
+ .func = 6,
+ .phy_addr = 1,
+ },
+ {
+ .name = "GalileoGen2",
+ .func = 6,
+ .phy_addr = 1,
+ },
+ {
+ .name = "SIMATIC IOT2000",
+ .asset_tag = "6ES7647-0AA00-0YA2",
+ .func = 6,
+ .phy_addr = 1,
+ },
+ {
+ .name = "SIMATIC IOT2000",
+ .asset_tag = "6ES7647-0AA00-1YA2",
+ .func = 6,
+ .phy_addr = 1,
+ },
+ {
+ .name = "SIMATIC IOT2000",
+ .asset_tag = "6ES7647-0AA00-1YA2",
+ .func = 7,
+ .phy_addr = 1,
+ },
+ {}
+};
+
static int quark_default_data(struct pci_dev *pdev,
- struct plat_stmmacenet_data *plat,
- const struct stmmac_pci_info *info)
+ struct plat_stmmacenet_data *plat)
{
int ret;
@@ -131,7 +159,7 @@ static int quark_default_data(struct pci_dev *pdev,
* Refuse to load the driver and register net device if MAC controller
* does not connect to any PHY interface.
*/
- ret = stmmac_pci_find_phy_addr(pdev, info);
+ ret = stmmac_pci_find_phy_addr(pdev, quark_pci_dmi_data);
if (ret < 0) {
/*
* Galileo boards with old firmware don't support DMI. We always
@@ -156,41 +184,8 @@ static int quark_default_data(struct pci_dev *pdev,
return 0;
}
-static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
- {
- .name = "Galileo",
- .func = 6,
- .phy_addr = 1,
- },
- {
- .name = "GalileoGen2",
- .func = 6,
- .phy_addr = 1,
- },
- {
- .name = "SIMATIC IOT2000",
- .asset_tag = "6ES7647-0AA00-0YA2",
- .func = 6,
- .phy_addr = 1,
- },
- {
- .name = "SIMATIC IOT2000",
- .asset_tag = "6ES7647-0AA00-1YA2",
- .func = 6,
- .phy_addr = 1,
- },
- {
- .name = "SIMATIC IOT2000",
- .asset_tag = "6ES7647-0AA00-1YA2",
- .func = 7,
- .phy_addr = 1,
- },
- {}
-};
-
static const struct stmmac_pci_info quark_pci_info = {
.setup = quark_default_data,
- .dmi = quark_pci_dmi_data,
};
/**
@@ -249,7 +244,7 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
pci_set_master(pdev);
- ret = info->setup(pdev, plat, info);
+ ret = info->setup(pdev, plat);
if (ret)
return ret;
--
2.12.0
^ permalink raw reply related
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