* Re: [pull request][net-next 00/10] Mellanox, mlx5 and devlink updates 2018-07-31
From: Jakub Kicinski @ 2018-08-07 0:49 UTC (permalink / raw)
To: Eran Ben Elisha
Cc: David Miller, saeedm, netdev, jiri, alexander.duyck, helgaas
In-Reply-To: <952362c1-6886-5293-bee4-267d87a79cba@mellanox.com>
On Mon, 6 Aug 2018 16:01:25 +0300, Eran Ben Elisha wrote:
> >> Hi Dave,
> >> I would like to re-state that this feature was not meant to be a generic
> >> one. This feature was added in order to resolve a HW bug which exist in
> >> a small portion of our devices.
> >
> > Would you mind describing the HW bug in more detail? To a outside
> > reviewer it really looks like you're adding a feature. What are you
> > working around? Is the lack of full AQM on the PCIe side of the chip
> > considered a bug?
>
> In multiple function environment, there is an issue with buffer
> allocation per function which may lead to starvation.
Multi-function? I thought you have a PF per uplink on all mlx5
silicon. Does the problem occur in single host scenarios as well?
What if with single function the host is too slow with taking packets
off the RX ring? Can the problem occur? Would this feature help in
such scenario as well?
> There is an HW WA for mitigate this starvation by identifying this
> state and apply early drop/mark.
If I understand you correctly you presently have one shared buffer with
no way to place limits (quotas) on how much of it can be consumed by a
traffic for a single PF. It remains unclear why this have not been a
problem for you until now.
To avoid the starvation you are adding AQM which is a *feature* that
may help you avoid queue build up in the NIC. But even if you could
place quotas, why would you not expose the AQM scheme? It looks very
useful.
> >> Those params will be used only on those current HWs and won't be in
> >> use for our future devices.
> >
> > I'm glad that is your plan today, however, customers may get used to
> > the simple interface you're adding now. This means the API you are
> > adding is effectively becoming an API other drivers may need to
> > implement to keep compatibility with someone's proprietary
> > orchestration.
>
> This issue was refactored, thus no need to have this WA at all in
> future NICs. So I don't believe we will end up in the situation you are
> describing. It is less likely that other vendors will be facing the
> same issue and will have to support such param. it was burn out of a
> bug and not as a feature which other may follow.
Sure other vendors may have buffers quotas configurable by e.g.
devlink-sb. But the AQM you are adding is a feature which is
potentially already supported by others.
> >> During the discussions, several alternatives where offered to be
> >> used by various members of the community. These alternatives
> >> includes TC and enhancements to PCI configuration tools.
> >>
> >> Regarding the TC, from my perspective, this is not an option as:
> >> 1) The HW mechanism handles multiple functions and therefore
> >> cannot be configured on as a regular TC
> >
> > Could you elaborate? What are the multiple functions? You seem to
> > be adding a knob to enable ECN marking and a knob for choosing
> > between some predefined slopes.
>
> PSB, The sloped are dynamic and enabled in a dynamic way.
> Indeed, we are adding a very specific knob for very non standard
> specific issue which can be used in addition to standard ECN marking.
>
> > In what way would your solution not behave like a RED offload?
>
> Existing Algo (RED, PIE, etc) are static, configurable. Our HW WA is
> dynamic (dynamic slope), adjusted and auto enabled.
You mean like Adaptive RED? The lack of documentation is making this
conversation harder to have. What's dynamic and aggressive? These
are not antonyms. Are the parameters to the algorithm configurable?
Will you not want to expose the actual threshold and adjustment values
so the customers can tweak them on their own depending on the workload?
> > With TC offload you'd also get a well-defined set of statistics, I
> > presume right now you're planning on adding a set of ethtool -S
> > counters?
> >
> >> 2) No PF + representors modeling can be applied here, this is a
> >> MultiHost environment where one host is not aware to the other
> >> hosts, and each is running on its own pci/driver. It is a device
> >> working mode configuration.
> >
> > Yes, the multihost part makes it less pleasant. But this is a
> > problem we have to tackle separately, at some point. It's not a
> > center of attention here.
>
> Agree, however the multihost part makes it non-transparent if we
> chose a solution which is not based on direct vendor configuration.
> This will lead to a bad user experience.
In my experience multi-host is not a major issue in practice. And
switchdev mode gives some visibility into statistics of others hosts
etc., which people appreciate.
^ permalink raw reply
* for editing the photos
From: Jason James @ 2018-08-06 12:48 UTC (permalink / raw)
To: netdev
Want to follow up the email sent last week.
Do you have needs for photo editing?
We can edit 400 images within 24 hours.
We are working on all kinds of ecommerce photos, jewelry photos, and the
portrait images.
We do cutting out and clipping path and others, and also we provide
retouching for your photos,
You can throw us a photo and we will do testing for you to check our
quality.
Thanks,
Jason
^ permalink raw reply
* check with you
From: Jason James @ 2018-08-06 14:17 UTC (permalink / raw)
To: netdev
Want to follow up the email sent last week.
Do you have needs for photo editing?
We can edit 400 images within 24 hours.
We are working on all kinds of ecommerce photos, jewelry photos, and the
portrait images.
We do cutting out and clipping path and others, and also we provide
retouching for your photos,
You can throw us a photo and we will do testing for you to check our
quality.
Thanks,
Jason
^ permalink raw reply
* Re: [BUG] 4.18-rcX iptables regression
From: Stephen Rothwell @ 2018-08-07 3:42 UTC (permalink / raw)
To: Bob Tracy; +Cc: linux-net, linux-kernel, NetFilter, Networking
In-Reply-To: <20180807010049.GA1345@gherkin.frus.com>
[-- Attachment #1: Type: text/plain, Size: 399 bytes --]
Hi Bob,
[Juts cc'ing the netfilter and netdev mailing lists]
On Mon, 6 Aug 2018 20:00:49 -0500 Bob Tracy <rct@gherkin.frus.com> wrote:
>
> With iptables v1.6.0 and kernel version 4.18-rc7, "iptables [-t table] -L"
> produces diagnostic output to the effect of not being able to find the table.
> Kernel version 4.17.0 and earlier work fine.
>
> --Bob
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH net-next] net: allow to call netif_reset_xps_queues() under cpu_read_lock
From: Andrei Vagin @ 2018-08-07 4:14 UTC (permalink / raw)
To: David S. Miller
Cc: Nambiar, Amritha, Michael S. Tsirkin, netdev, virtualization,
Andrei Vagin
From: Andrei Vagin <avagin@gmail.com>
The definition of static_key_slow_inc() has cpus_read_lock in place. In the
virtio_net driver, XPS queues are initialized after setting the queue:cpu
affinity in virtnet_set_affinity() which is already protected within
cpus_read_lock. Lockdep prints a warning when we are trying to acquire
cpus_read_lock when it is already held.
This patch adds an ability to call __netif_set_xps_queue under
cpu_read_lock().
============================================
WARNING: possible recursive locking detected
4.18.0-rc3-next-20180703+ #1 Not tainted
--------------------------------------------
swapper/0/1 is trying to acquire lock:
00000000cf973d46 (cpu_hotplug_lock.rw_sem){++++}, at: static_key_slow_inc+0xe/0x20
but task is already holding lock:
00000000cf973d46 (cpu_hotplug_lock.rw_sem){++++}, at: init_vqs+0x513/0x5a0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(cpu_hotplug_lock.rw_sem);
lock(cpu_hotplug_lock.rw_sem);
*** DEADLOCK ***
May be due to missing lock nesting notation
3 locks held by swapper/0/1:
#0: 00000000244bc7da (&dev->mutex){....}, at: __driver_attach+0x5a/0x110
#1: 00000000cf973d46 (cpu_hotplug_lock.rw_sem){++++}, at: init_vqs+0x513/0x5a0
#2: 000000005cd8463f (xps_map_mutex){+.+.}, at: __netif_set_xps_queue+0x8d/0xc60
Cc: "Nambiar, Amritha" <amritha.nambiar@intel.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Fixes: 8af2c06ff4b1 ("net-sysfs: Add interface for Rx queue(s) map per Tx queue")
Signed-off-by: Andrei Vagin <avagin@gmail.com>
---
drivers/net/virtio_net.c | 4 +++-
include/linux/netdevice.h | 2 +-
net/core/dev.c | 23 +++++++++++++++++------
net/core/net-sysfs.c | 2 +-
4 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 62311dde6e71..a4abcfcf26b2 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1903,9 +1903,11 @@ static void virtnet_set_affinity(struct virtnet_info *vi)
i = 0;
for_each_online_cpu(cpu) {
+ const unsigned long *mask = cpumask_bits(cpumask_of(cpu));
+
virtqueue_set_affinity(vi->rq[i].vq, cpu);
virtqueue_set_affinity(vi->sq[i].vq, cpu);
- netif_set_xps_queue(vi->dev, cpumask_of(cpu), i);
+ __netif_set_xps_queue(vi->dev, mask, i, false, true);
i++;
}
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 282e2e95ad5b..124f9a00ce71 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3320,7 +3320,7 @@ static inline void netif_wake_subqueue(struct net_device *dev, u16 queue_index)
int netif_set_xps_queue(struct net_device *dev, const struct cpumask *mask,
u16 index);
int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
- u16 index, bool is_rxqs_map);
+ u16 index, bool is_rxqs_map, bool cpuslocked);
/**
* netif_attr_test_mask - Test a CPU or Rx queue set in a mask
diff --git a/net/core/dev.c b/net/core/dev.c
index f68122f0ab02..d6a0f64ccdd9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2176,6 +2176,7 @@ static void netif_reset_xps_queues(struct net_device *dev, u16 offset,
if (!static_key_false(&xps_needed))
return;
+ cpus_read_lock();
mutex_lock(&xps_map_mutex);
if (static_key_false(&xps_rxqs_needed)) {
@@ -2199,10 +2200,11 @@ static void netif_reset_xps_queues(struct net_device *dev, u16 offset,
out_no_maps:
if (static_key_enabled(&xps_rxqs_needed))
- static_key_slow_dec(&xps_rxqs_needed);
+ static_key_slow_dec_cpuslocked(&xps_rxqs_needed);
- static_key_slow_dec(&xps_needed);
+ static_key_slow_dec_cpuslocked(&xps_needed);
mutex_unlock(&xps_map_mutex);
+ cpus_read_unlock();
}
static void netif_reset_xps_queues_gt(struct net_device *dev, u16 index)
@@ -2251,7 +2253,7 @@ static struct xps_map *expand_xps_map(struct xps_map *map, int attr_index,
}
int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
- u16 index, bool is_rxqs_map)
+ u16 index, bool is_rxqs_map, bool cpuslocked)
{
const unsigned long *online_mask = NULL, *possible_mask = NULL;
struct xps_dev_maps *dev_maps, *new_dev_maps = NULL;
@@ -2275,6 +2277,9 @@ int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
return -EINVAL;
}
+ if (!cpuslocked)
+ cpus_read_lock();
+
mutex_lock(&xps_map_mutex);
if (is_rxqs_map) {
maps_sz = XPS_RXQ_DEV_MAPS_SIZE(num_tc, dev->num_rx_queues);
@@ -2317,9 +2322,9 @@ int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
if (!new_dev_maps)
goto out_no_new_maps;
- static_key_slow_inc(&xps_needed);
+ static_key_slow_inc_cpuslocked(&xps_needed);
if (is_rxqs_map)
- static_key_slow_inc(&xps_rxqs_needed);
+ static_key_slow_inc_cpuslocked(&xps_rxqs_needed);
for (j = -1; j = netif_attrmask_next(j, possible_mask, nr_ids),
j < nr_ids;) {
@@ -2427,6 +2432,8 @@ int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
out_no_maps:
mutex_unlock(&xps_map_mutex);
+ if (!cpuslocked)
+ cpus_read_unlock();
return 0;
error:
@@ -2444,15 +2451,19 @@ int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
}
mutex_unlock(&xps_map_mutex);
+ if (!cpuslocked)
+ cpus_read_unlock();
kfree(new_dev_maps);
return -ENOMEM;
}
+EXPORT_SYMBOL_GPL(__netif_set_xps_queue);
int netif_set_xps_queue(struct net_device *dev, const struct cpumask *mask,
u16 index)
{
- return __netif_set_xps_queue(dev, cpumask_bits(mask), index, false);
+ return __netif_set_xps_queue(dev, cpumask_bits(mask),
+ index, false, false);
}
EXPORT_SYMBOL(netif_set_xps_queue);
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 0a95bcf64cdc..06a141445d80 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1400,7 +1400,7 @@ static ssize_t xps_rxqs_store(struct netdev_queue *queue, const char *buf,
return err;
}
- err = __netif_set_xps_queue(dev, mask, index, true);
+ err = __netif_set_xps_queue(dev, mask, index, true, false);
kfree(mask);
return err ? : len;
}
--
2.17.1
^ permalink raw reply related
* we do the editing
From: Jason James @ 2018-08-06 14:17 UTC (permalink / raw)
To: netdev
Want to follow up the email sent last week.
Do you have needs for photo editing?
We can edit 400 images within 24 hours.
We are working on all kinds of ecommerce photos, jewelry photos, and the
portrait images.
We do cutting out and clipping path and others, and also we provide
retouching for your photos,
You can throw us a photo and we will do testing for you to check our
quality.
Thanks,
Jason
^ permalink raw reply
* [PATCH net-next 0/2] ibmvnic: Update firmware error reporting
From: Thomas Falcon @ 2018-08-07 2:39 UTC (permalink / raw)
To: netdev; +Cc: jallen, nfont, Thomas Falcon
This patch set cleans out a lot of dead code from the ibmvnic driver
and adds some more. The error ID field of the descriptor is not filled
in by firmware, so do not print it and do not use it to query for
more detailed information. Remove the unused code written for this.
Finally, update the message to print a string explainng the error
cause instead of just the error code.
Thomas Falcon (2):
ibmvnic: Remove code to request error information
ibmvnic: Update firmware error reporting with cause string
drivers/net/ethernet/ibm/ibmvnic.c | 168 ++++++-------------------------------
drivers/net/ethernet/ibm/ibmvnic.h | 33 --------
2 files changed, 26 insertions(+), 175 deletions(-)
--
2.12.3
^ permalink raw reply
* [PATCH net-next 1/2] ibmvnic: Remove code to request error information
From: Thomas Falcon @ 2018-08-07 2:39 UTC (permalink / raw)
To: netdev; +Cc: jallen, nfont, Thomas Falcon
In-Reply-To: <1533609599-10188-1-git-send-email-tlfalcon@linux.vnet.ibm.com>
When backing device firmware reports an error, it provides an
error ID, which is meant to be queried for more detailed error
information. Currently, however, an error ID is not provided by
the Virtual I/O server and there are not any plans to do so. For
now, it is always unfilled or zero, so request_error_information
will never be called. Remove it.
Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 144 +------------------------------------
drivers/net/ethernet/ibm/ibmvnic.h | 33 ---------
2 files changed, 1 insertion(+), 176 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index ffe7acbeaa22..109e4a58efad 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -718,23 +718,6 @@ static int init_tx_pools(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 void ibmvnic_napi_enable(struct ibmvnic_adapter *adapter)
{
int i;
@@ -896,7 +879,6 @@ static void release_resources(struct ibmvnic_adapter *adapter)
release_tx_pools(adapter);
release_rx_pools(adapter);
- release_error_buffers(adapter);
release_napi(adapter);
release_login_rsp_buffer(adapter);
}
@@ -3843,133 +3825,16 @@ static void handle_query_ip_offload_rsp(struct ibmvnic_adapter *adapter)
ibmvnic_send_crq(adapter, &crq);
}
-static void handle_error_info_rsp(union ibmvnic_crq *crq,
- struct ibmvnic_adapter *adapter)
-{
- struct device *dev = &adapter->vdev->dev;
- struct ibmvnic_error_buff *error_buff, *tmp;
- unsigned long flags;
- bool found = false;
- int i;
-
- if (!crq->request_error_rsp.rc.code) {
- dev_info(dev, "Request Error Rsp returned with rc=%x\n",
- crq->request_error_rsp.rc.code);
- return;
- }
-
- spin_lock_irqsave(&adapter->error_list_lock, flags);
- list_for_each_entry_safe(error_buff, tmp, &adapter->errors, list)
- if (error_buff->error_id == crq->request_error_rsp.error_id) {
- found = true;
- list_del(&error_buff->list);
- break;
- }
- spin_unlock_irqrestore(&adapter->error_list_lock, flags);
-
- if (!found) {
- dev_err(dev, "Couldn't find error id %x\n",
- be32_to_cpu(crq->request_error_rsp.error_id));
- return;
- }
-
- dev_err(dev, "Detailed info for error id %x:",
- be32_to_cpu(crq->request_error_rsp.error_id));
-
- for (i = 0; i < error_buff->len; i++) {
- pr_cont("%02x", (int)error_buff->buff[i]);
- if (i % 8 == 7)
- pr_cont(" ");
- }
- pr_cont("\n");
-
- dma_unmap_single(dev, error_buff->dma, error_buff->len,
- DMA_FROM_DEVICE);
- kfree(error_buff->buff);
- kfree(error_buff);
-}
-
-static void request_error_information(struct ibmvnic_adapter *adapter,
- union ibmvnic_crq *err_crq)
-{
- struct device *dev = &adapter->vdev->dev;
- struct net_device *netdev = adapter->netdev;
- struct ibmvnic_error_buff *error_buff;
- unsigned long timeout = msecs_to_jiffies(30000);
- union ibmvnic_crq crq;
- unsigned long flags;
- int rc, detail_len;
-
- error_buff = kmalloc(sizeof(*error_buff), GFP_ATOMIC);
- if (!error_buff)
- return;
-
- detail_len = be32_to_cpu(err_crq->error_indication.detail_error_sz);
- error_buff->buff = kmalloc(detail_len, GFP_ATOMIC);
- if (!error_buff->buff) {
- kfree(error_buff);
- return;
- }
-
- error_buff->dma = dma_map_single(dev, error_buff->buff, detail_len,
- DMA_FROM_DEVICE);
- if (dma_mapping_error(dev, error_buff->dma)) {
- netdev_err(netdev, "Couldn't map error buffer\n");
- kfree(error_buff->buff);
- kfree(error_buff);
- return;
- }
-
- error_buff->len = detail_len;
- error_buff->error_id = err_crq->error_indication.error_id;
-
- spin_lock_irqsave(&adapter->error_list_lock, flags);
- list_add_tail(&error_buff->list, &adapter->errors);
- spin_unlock_irqrestore(&adapter->error_list_lock, flags);
-
- memset(&crq, 0, sizeof(crq));
- crq.request_error_info.first = IBMVNIC_CRQ_CMD;
- crq.request_error_info.cmd = REQUEST_ERROR_INFO;
- crq.request_error_info.ioba = cpu_to_be32(error_buff->dma);
- crq.request_error_info.len = cpu_to_be32(detail_len);
- crq.request_error_info.error_id = err_crq->error_indication.error_id;
-
- rc = ibmvnic_send_crq(adapter, &crq);
- if (rc) {
- netdev_err(netdev, "failed to request error information\n");
- goto err_info_fail;
- }
-
- if (!wait_for_completion_timeout(&adapter->init_done, timeout)) {
- netdev_err(netdev, "timeout waiting for error information\n");
- goto err_info_fail;
- }
-
- return;
-
-err_info_fail:
- spin_lock_irqsave(&adapter->error_list_lock, flags);
- list_del(&error_buff->list);
- spin_unlock_irqrestore(&adapter->error_list_lock, flags);
-
- kfree(error_buff->buff);
- kfree(error_buff);
-}
-
static void handle_error_indication(union ibmvnic_crq *crq,
struct ibmvnic_adapter *adapter)
{
struct device *dev = &adapter->vdev->dev;
- dev_err(dev, "Firmware reports %serror id %x, cause %d\n",
+ dev_err(dev, "Firmware reports %serror, cause %d\n",
crq->error_indication.flags
& IBMVNIC_FATAL_ERROR ? "FATAL " : "",
- be32_to_cpu(crq->error_indication.error_id),
be16_to_cpu(crq->error_indication.error_cause));
- if (be32_to_cpu(crq->error_indication.error_id))
- request_error_information(adapter, crq);
-
if (crq->error_indication.flags & IBMVNIC_FATAL_ERROR)
ibmvnic_reset(adapter, VNIC_RESET_FATAL);
else
@@ -4468,10 +4333,6 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
netdev_dbg(netdev, "Got Error Indication\n");
handle_error_indication(crq, adapter);
break;
- case REQUEST_ERROR_RSP:
- netdev_dbg(netdev, "Got Error Detail Response\n");
- handle_error_info_rsp(crq, adapter);
- break;
case REQUEST_STATISTICS_RSP:
netdev_dbg(netdev, "Got Statistics Response\n");
complete(&adapter->stats_done);
@@ -4830,9 +4691,6 @@ 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);
- spin_lock_init(&adapter->error_list_lock);
-
INIT_WORK(&adapter->ibmvnic_reset, __ibmvnic_reset);
INIT_LIST_HEAD(&adapter->rwi_list);
mutex_init(&adapter->reset_lock);
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index f9fb780102ac..f06eec145ca6 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -512,24 +512,6 @@ struct ibmvnic_error_indication {
u8 reserved2[2];
} __packed __aligned(8);
-struct ibmvnic_request_error_info {
- u8 first;
- u8 cmd;
- u8 reserved[2];
- __be32 ioba;
- __be32 len;
- __be32 error_id;
-} __packed __aligned(8);
-
-struct ibmvnic_request_error_rsp {
- u8 first;
- u8 cmd;
- u8 reserved[2];
- __be32 error_id;
- __be32 len;
- struct ibmvnic_rc rc;
-} __packed __aligned(8);
-
struct ibmvnic_link_state_indication {
u8 first;
u8 cmd;
@@ -709,8 +691,6 @@ union ibmvnic_crq {
struct ibmvnic_request_debug_stats request_debug_stats;
struct ibmvnic_request_debug_stats request_debug_stats_rsp;
struct ibmvnic_error_indication error_indication;
- struct ibmvnic_request_error_info request_error_info;
- struct ibmvnic_request_error_rsp request_error_rsp;
struct ibmvnic_link_state_indication link_state_indication;
struct ibmvnic_change_mac_addr change_mac_addr;
struct ibmvnic_change_mac_addr change_mac_addr_rsp;
@@ -809,8 +789,6 @@ enum ibmvnic_commands {
SET_PHYS_PARMS = 0x07,
SET_PHYS_PARMS_RSP = 0x87,
ERROR_INDICATION = 0x08,
- REQUEST_ERROR_INFO = 0x09,
- REQUEST_ERROR_RSP = 0x89,
LOGICAL_LINK_STATE = 0x0C,
LOGICAL_LINK_STATE_RSP = 0x8C,
REQUEST_STATISTICS = 0x0D,
@@ -945,14 +923,6 @@ struct ibmvnic_rx_pool {
struct ibmvnic_long_term_buff long_term_buff;
};
-struct ibmvnic_error_buff {
- char *buff;
- dma_addr_t dma;
- int len;
- struct list_head list;
- __be32 error_id;
-};
-
struct ibmvnic_vpd {
unsigned char *buff;
dma_addr_t dma_addr;
@@ -1047,9 +1017,6 @@ struct ibmvnic_adapter {
struct completion init_done;
int init_done_rc;
- struct list_head errors;
- spinlock_t error_list_lock;
-
struct completion fw_done;
int fw_done_rc;
--
2.12.3
^ permalink raw reply related
* [PATCH net-next 2/2] ibmvnic: Update firmware error reporting with cause string
From: Thomas Falcon @ 2018-08-07 2:39 UTC (permalink / raw)
To: netdev; +Cc: jallen, nfont, Thomas Falcon
In-Reply-To: <1533609599-10188-1-git-send-email-tlfalcon@linux.vnet.ibm.com>
Print a string instead of the error code. Since there is a
possibility that the driver can recover, classify it as a
warning instead of an error.
Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 109e4a58efad..dafdd4ade705 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3825,15 +3825,41 @@ static void handle_query_ip_offload_rsp(struct ibmvnic_adapter *adapter)
ibmvnic_send_crq(adapter, &crq);
}
+static const char *ibmvnic_fw_err_cause(u16 cause)
+{
+ switch (cause) {
+ case ADAPTER_PROBLEM:
+ return "adapter problem";
+ case BUS_PROBLEM:
+ return "bus problem";
+ case FW_PROBLEM:
+ return "firmware problem";
+ case DD_PROBLEM:
+ return "device driver problem";
+ case EEH_RECOVERY:
+ return "EEH recovery";
+ case FW_UPDATED:
+ return "firmware updated";
+ case LOW_MEMORY:
+ return "low Memory";
+ default:
+ return "unknown";
+ }
+}
+
static void handle_error_indication(union ibmvnic_crq *crq,
struct ibmvnic_adapter *adapter)
{
struct device *dev = &adapter->vdev->dev;
+ u16 cause;
+
+ cause = be16_to_cpu(crq->error_indication.error_cause);
- dev_err(dev, "Firmware reports %serror, cause %d\n",
- crq->error_indication.flags
- & IBMVNIC_FATAL_ERROR ? "FATAL " : "",
- be16_to_cpu(crq->error_indication.error_cause));
+ dev_warn_ratelimited(dev,
+ "Firmware reports %serror, cause: %s. Starting recovery...\n",
+ crq->error_indication.flags
+ & IBMVNIC_FATAL_ERROR ? "FATAL " : "",
+ ibmvnic_fw_err_cause(cause));
if (crq->error_indication.flags & IBMVNIC_FATAL_ERROR)
ibmvnic_reset(adapter, VNIC_RESET_FATAL);
--
2.12.3
^ permalink raw reply related
* Re: [RFC bpf-next v2 3/3] docs: Split filter.txt into separate documents.
From: Tobin C. Harding @ 2018-08-07 2:48 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Daniel Borkmann, Alexei Starovoitov, David S. Miller, linux-doc,
netdev, linux-kernel
In-Reply-To: <20180803070818.3d3e52e4@lwn.net>
On Fri, Aug 03, 2018 at 07:08:18AM -0600, Jonathan Corbet wrote:
> On Fri, 3 Aug 2018 08:31:00 +1000
> "Tobin C. Harding" <me@tobin.cc> wrote:
>
> > In preparation for conversion of Documentation/networking/filter.txt it
> > was noticed that the document contains a lot of information. The
> > document may be more accessible if it was split up. Some parts pertain
> > to everyone, let's put these bits in core-api/. The more hard core bits
> > about eBPF internals could be put with the other BPF docs in
> > Documentation/bpf/. There is a small bit of information on testing and
> > miscellaneous matters that are useful for everyone (everyone does
> > testing, right) so lets keep that info at the bottom of both new
> > documents. (This includes the original authors.)
> >
> > Split Documentation/networking/filter.txt into
> > Documentation/bpf/eBPF.rst and Documentation/core-api/bpf.rst
> >
> > Signed-off-by: Tobin C. Harding <me@tobin.cc>
> > ---
> > .../{networking/filter.txt => bpf/eBPF.rst} | 590 +----------------
> > Documentation/core-api/bpf.rst | 599 ++++++++++++++++++
>
> Some overall thoughts...
>
> - A good step in the right direction, and worthwhile work. Thanks for
> doing this!
Thanks for you thoughts Jon.
> - The new eBPF.rst file is not actually an RST file. Giving it that
> extension while not converting the contents will confuse Sphinx. I'd
> call it .txt at this point.
I'm a bit stumped as the best way to structure a patch set that does
foo.txt -> foo.rst
conversion. FTR the conditions I'm trying to meet are:
1. Each patch should be discreet and leave the kernel in a _correct_
state.
2. One thing per patch.
3. Maintainers should be able to take the first 'n' patches without
taking the whole set.
How about these steps:
1. start with foo.txt
2. do typo and grammar fixes (any number of patches).
3. rename to foo.rst, do whitespace changes, code snippet
indentation, heading adornments, update references to this file.
(single patch).
4. Fix up references in the file text to use RST (i.e :ref: blah)
5. Fix up RST markers (backticks etc). (any number of patches)
Any better ideas or further suggestions? I like 4 and 5 separate since
they can be done in multiple ways so may be controversial.
> - The document now known as core-api/bpf.rst is still covering two
> separate things. One is the socket-filter API, while the other is
> classic BPF. Since cBPF is still used elsewhere (seccomp), it's of
> wider interest. Also, this is user-space API stuff, not kernel API
> stuff, so I think that Documentation/userspace-api/ is the right place
> for it.
>
> I'm kind of thinking this through as I type it, but I guess I'm arguing
> for the creation of three files, all in Documentation/userspace-api/:
>
> - socket-filter.rst on how to write socket filters
> - cBPF.rst describing classic BPF and its tools
> - eBPF.rst describing extended BPF
>
> Tying cBPF.rst into seccomp_filter.rst could also be helpful for our
> readers.
>
> Does this make sense?
Yep, that all makes sense. Patch set to come.
thanks,
Tobin.
^ permalink raw reply
* lening aanbod
From: FUNDING TRUST FINANCE @ 2018-08-07 2:40 UTC (permalink / raw)
Goede dag,
We FUNDING TRUST FINANCE geven leningen per postadvertentie. Wij bieden verschillende soorten leningen of projectleningen (korte en lange termijnleningen, persoonlijke leningen, leningen aan bedrijven enz.) Met een rentetarief van 3%. We verstrekken leningen aan mensen in nood, ongeacht hun locatie, geslacht, burgerlijke staat, opleiding of baan, maar moeten een legale manier van terugbetaling hebben. Onze leningen variëren tussen 5.000,00 tot 20.000.000,00 US Dollar of Euro of Pond met een maximale duur van 15 jaar. Als u geïnteresseerd bent in meer informatie, neem dan contact met ons op via mail, info.fundingtrustfinance@gmail.com of website: www.fundingtrustfinance.com
1-De klant moet een korte samenvatting van het project verzenden. Dit moet het totale bedrag omvatten dat vereist is voor het project, geschat rendement op investering, terugbetalingsperiode van de lening, dit mag niet meer dan 20 jaar zijn
Vul het onderstaande formulier in.
INFORMATIE NODIG:
Jullie namen:
Adres: ...........
Telefoon: ...........
Benodigde hoeveelheid: .......
Looptijd: ...............
Beroep: ...........
Maandelijks inkomensniveau: ......
Geslacht: ..............
Geboortedatum: .......
Staat: ...............
Land: .........
Doel: .........
Bedankt voor uw begrip.
Beste wensen,
Mr.Ronny Hens
FUNDING TRUST FINANCE
info.fundingtrustfinance@gmail.com
Website: www.fundingtrustfinance.com
^ permalink raw reply
* Re: [PATCH net-next 2/2] ibmvnic: Update firmware error reporting with cause string
From: Nathan Fontenot @ 2018-08-07 3:48 UTC (permalink / raw)
To: Thomas Falcon, netdev; +Cc: jallen
In-Reply-To: <1533609599-10188-3-git-send-email-tlfalcon@linux.vnet.ibm.com>
On 08/06/2018 09:39 PM, Thomas Falcon wrote:
> Print a string instead of the error code. Since there is a
> possibility that the driver can recover, classify it as a
> warning instead of an error.
>
> Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
> ---
> drivers/net/ethernet/ibm/ibmvnic.c | 34 ++++++++++++++++++++++++++++++----
> 1 file changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
> index 109e4a58efad..dafdd4ade705 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -3825,15 +3825,41 @@ static void handle_query_ip_offload_rsp(struct ibmvnic_adapter *adapter)
> ibmvnic_send_crq(adapter, &crq);
> }
>
> +static const char *ibmvnic_fw_err_cause(u16 cause)
> +{
> + switch (cause) {
> + case ADAPTER_PROBLEM:
> + return "adapter problem";
> + case BUS_PROBLEM:
> + return "bus problem";
> + case FW_PROBLEM:
> + return "firmware problem";
> + case DD_PROBLEM:
> + return "device driver problem";
> + case EEH_RECOVERY:
> + return "EEH recovery";
> + case FW_UPDATED:
> + return "firmware updated";
> + case LOW_MEMORY:
> + return "low Memory";
> + default:
> + return "unknown";
> + }
> +}
> +
> static void handle_error_indication(union ibmvnic_crq *crq,
> struct ibmvnic_adapter *adapter)
> {
> struct device *dev = &adapter->vdev->dev;
> + u16 cause;
> +
> + cause = be16_to_cpu(crq->error_indication.error_cause);
>
> - dev_err(dev, "Firmware reports %serror, cause %d\n",
> - crq->error_indication.flags
> - & IBMVNIC_FATAL_ERROR ? "FATAL " : "",
> - be16_to_cpu(crq->error_indication.error_cause));
> + dev_warn_ratelimited(dev,
> + "Firmware reports %serror, cause: %s. Starting recovery...\n",
^^^^^^^
You're going to want a space between after the %s here.
-Nathan
> + crq->error_indication.flags
> + & IBMVNIC_FATAL_ERROR ? "FATAL " : "",
> + ibmvnic_fw_err_cause(cause));
>
> if (crq->error_indication.flags & IBMVNIC_FATAL_ERROR)
> ibmvnic_reset(adapter, VNIC_RESET_FATAL);
>
^ permalink raw reply
* Re: [PATCH net-next 2/2] ibmvnic: Update firmware error reporting with cause string
From: Thomas Falcon @ 2018-08-07 4:34 UTC (permalink / raw)
To: Nathan Fontenot, netdev; +Cc: jallen
In-Reply-To: <4c6e57db-b5b7-fa9d-d3ef-9016dfe0be21@linux.vnet.ibm.com>
On 08/06/2018 10:48 PM, Nathan Fontenot wrote:
>
> On 08/06/2018 09:39 PM, Thomas Falcon wrote:
>> Print a string instead of the error code. Since there is a
>> possibility that the driver can recover, classify it as a
>> warning instead of an error.
>>
>> Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
>> ---
>> drivers/net/ethernet/ibm/ibmvnic.c | 34
>> ++++++++++++++++++++++++++++++----
>> 1 file changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
>> b/drivers/net/ethernet/ibm/ibmvnic.c
>> index 109e4a58efad..dafdd4ade705 100644
>> --- a/drivers/net/ethernet/ibm/ibmvnic.c
>> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
>> @@ -3825,15 +3825,41 @@ static void
>> handle_query_ip_offload_rsp(struct ibmvnic_adapter *adapter)
>> ibmvnic_send_crq(adapter, &crq);
>> }
>>
>> +static const char *ibmvnic_fw_err_cause(u16 cause)
>> +{
>> + switch (cause) {
>> + case ADAPTER_PROBLEM:
>> + return "adapter problem";
>> + case BUS_PROBLEM:
>> + return "bus problem";
>> + case FW_PROBLEM:
>> + return "firmware problem";
>> + case DD_PROBLEM:
>> + return "device driver problem";
>> + case EEH_RECOVERY:
>> + return "EEH recovery";
>> + case FW_UPDATED:
>> + return "firmware updated";
>> + case LOW_MEMORY:
>> + return "low Memory";
>> + default:
>> + return "unknown";
>> + }
>> +}
>> +
>> static void handle_error_indication(union ibmvnic_crq *crq,
>> struct ibmvnic_adapter *adapter)
>> {
>> struct device *dev = &adapter->vdev->dev;
>> + u16 cause;
>> +
>> + cause = be16_to_cpu(crq->error_indication.error_cause);
>>
>> - dev_err(dev, "Firmware reports %serror, cause %d\n",
>> - crq->error_indication.flags
>> - & IBMVNIC_FATAL_ERROR ? "FATAL " : "",
>> - be16_to_cpu(crq->error_indication.error_cause));
>> + dev_warn_ratelimited(dev,
>> + "Firmware reports %serror, cause: %s. Starting
>> recovery...\n",
>
> ^^^^^^^
>
> You're going to want a space between after the %s here.
>
> -Nathan
>
It does look odd at first glance, but there is a space after "FATAL" below.
Thanks,
Tom
>> + crq->error_indication.flags
>> + & IBMVNIC_FATAL_ERROR ? "FATAL " : "",
>> + ibmvnic_fw_err_cause(cause));
>>
>> if (crq->error_indication.flags & IBMVNIC_FATAL_ERROR)
>> ibmvnic_reset(adapter, VNIC_RESET_FATAL);
>>
^ permalink raw reply
* RE: [PATCH net-next] net/tls: Calculate nsg for zerocopy path without skb_cow_data.
From: Vakul Garg @ 2018-08-07 4:41 UTC (permalink / raw)
To: Doron Roberts-Kedes
Cc: David S . Miller, Dave Watson, Boris Pismenny, Aviad Yehezkel,
netdev@vger.kernel.org
In-Reply-To: <20180806183219.GA78698@doronrk-mbp.dhcp.thefacebook.com>
> -----Original Message-----
> From: Doron Roberts-Kedes [mailto:doronrk@fb.com]
> Sent: Tuesday, August 7, 2018 12:02 AM
> To: Vakul Garg <vakul.garg@nxp.com>
> Cc: David S . Miller <davem@davemloft.net>; Dave Watson
> <davejwatson@fb.com>; Boris Pismenny <borisp@mellanox.com>; Aviad
> Yehezkel <aviadye@mellanox.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH net-next] net/tls: Calculate nsg for zerocopy path
> without skb_cow_data.
>
> On Fri, Aug 03, 2018 at 11:49:02AM -0700, Doron Roberts-Kedes wrote:
> > On Fri, Aug 03, 2018 at 01:23:33AM +0000, Vakul Garg wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Doron Roberts-Kedes [mailto:doronrk@fb.com]
> > > > Sent: Friday, August 3, 2018 6:00 AM
> > > > To: David S . Miller <davem@davemloft.net>
> > > > Cc: Dave Watson <davejwatson@fb.com>; Vakul Garg
> > > > <vakul.garg@nxp.com>; Boris Pismenny <borisp@mellanox.com>;
> Aviad
> > > > Yehezkel <aviadye@mellanox.com>; netdev@vger.kernel.org; Doron
> > > > Roberts-Kedes <doronrk@fb.com>
> > > > Subject: [PATCH net-next] net/tls: Calculate nsg for zerocopy path
> > > > without skb_cow_data.
> > > >
> > > > decrypt_skb fails if the number of sg elements required to map is
> > > > greater than MAX_SKB_FRAGS. As noted by Vakul Garg, nsg must
> > > > always be calculated, but skb_cow_data adds unnecessary memcpy's
> > > > for the zerocopy case.
> > > >
> > > > The new function skb_nsg calculates the number of scatterlist
> > > > elements required to map the skb without the extra overhead of
> > > > skb_cow_data. This function mimics the structure of skb_to_sgvec.
> > > >
> > > > Fixes: c46234ebb4d1 ("tls: RX path for ktls")
> > > > Signed-off-by: Doron Roberts-Kedes <doronrk@fb.com>
> > > > ---
> > > > net/tls/tls_sw.c | 89
> > > > ++++++++++++++++++++++++++++++++++++++++++++++--
> > > > 1 file changed, 86 insertions(+), 3 deletions(-)
> > > >
> > > > memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
> > > > if (!sgout) {
> > > > - nsg = skb_cow_data(skb, 0, &unused) + 1;
> > > > + nsg = skb_cow_data(skb, 0, &unused);
> > > > + } else {
> > > > + nsg = skb_nsg(skb,
> > > > + rxm->offset + tls_ctx->rx.prepend_size,
> > > > + rxm->full_len - tls_ctx->rx.prepend_size);
> > > > + if (nsg <= 0)
> > > > + return nsg;
> > > Comparison should be (nsg < 1). TLS forbids '0' sized records.
> >
> > Yes true, v2 incoming
> >
>
> Glancing at this a second time, I actually don't believe this should be
> changed. nsg <= 0 is equivalent to nsg < 1.
Correct.
> Returning 0 if the record is
> 0 sized is the proper behavior here, since decrypting a zero-length skb is a no-
> op. It is true that zero sized TLS records are forbidden, but it is confusing to
> enforce that in this part of the code. I would be surpised to learn that
> tls_sw_recvmsg could be invoked with a len equal to 0, but if I'm wrong, and
> that case does need to be handled, then it should be in a different patch.
^ permalink raw reply
* Re: [PATCH 1/5] dt-binding: dwmac-sun8i: add H6 compatible string (w/ A64 fallback)
From: Chen-Yu Tsai @ 2018-08-07 7:31 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Rob Herring, Maxime Ripard, David S. Miller, Corentin Labbe,
netdev, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi
In-Reply-To: <20180731205614.GA25776@rob-hp-laptop>
On Wed, Aug 1, 2018 at 4:56 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Sun, Jul 22, 2018 at 01:39:51PM +0800, Icenowy Zheng wrote:
>> The Allwinner H6 SoC features a Ethernet MAC that is similar to the one
>> in A64.
>>
>> Add a compatible string for it with A64 fallback compatible string, in
>> this case the A64 driver can be used.
>>
>> The "internal" PHY is not internal from the perspective of the H6 main
>> die, instead it's on the co-packaged AC200 chip, and connected to the
>> main die with RMII at the in-package Port A PIO bank. So from the SoC
>> driver side it needs no special treatment.
>>
>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
>> ---
>> Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 1 +
>> 1 file changed, 1 insertion(+)
>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Reviewed-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
^ permalink raw reply
* [PATCH] tipc: fix an interrupt unsafe locking scenario
From: Ying Xue @ 2018-08-07 7:52 UTC (permalink / raw)
To: netdev, jon.maloy; +Cc: tipc-discussion, davem
Commit 9faa89d4ed9d ("tipc: make function tipc_net_finalize() thread
safe") tries to make it thread safe to set node address, so it uses
node_list_lock lock to serialize the whole process of setting node
address in tipc_net_finalize(). But it causes the following interrupt
unsafe locking scenario:
CPU0 CPU1
---- ----
rht_deferred_worker()
rhashtable_rehash_table()
lock(&(&ht->lock)->rlock)
tipc_nl_compat_doit()
tipc_net_finalize()
local_irq_disable();
lock(&(&tn->node_list_lock)->rlock);
tipc_sk_reinit()
rhashtable_walk_enter()
lock(&(&ht->lock)->rlock);
<Interrupt>
tipc_disc_rcv()
tipc_node_check_dest()
tipc_node_create()
lock(&(&tn->node_list_lock)->rlock);
*** DEADLOCK ***
When rhashtable_rehash_table() holds ht->lock on CPU0, it doesn't
disable BH. So if an interrupt happens after the lock, it can create
an inverse lock ordering between ht->lock and tn->node_list_lock. As
a consequence, deadlock might happen.
The reason causing the inverse lock ordering scenario above is because
the initial purpose of node_list_lock is not designed to do the
serialization of node address setting.
As cmpxchg() can guarantee CAS (compare-and-swap) process is atomic,
we use it to replace node_list_lock to ensure setting node address can
be atomically finished. It turns out the potential deadlock can be
avoided as well.
Fixes: 9faa89d4ed9d ("tipc: make function tipc_net_finalize() thread safe")
Signed-off-by: Ying Xue <ying.xue@windriver.com>
---
net/tipc/net.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/tipc/net.c b/net/tipc/net.c
index a7f6964..62199cf 100644
--- a/net/tipc/net.c
+++ b/net/tipc/net.c
@@ -123,15 +123,13 @@ void tipc_net_finalize(struct net *net, u32 addr)
{
struct tipc_net *tn = tipc_net(net);
- spin_lock_bh(&tn->node_list_lock);
- if (!tipc_own_addr(net)) {
+ if (!cmpxchg(&tn->node_addr, 0, addr)) {
tipc_set_node_addr(net, addr);
tipc_named_reinit(net);
tipc_sk_reinit(net);
tipc_nametbl_publish(net, TIPC_CFG_SRV, addr, addr,
TIPC_CLUSTER_SCOPE, 0, addr);
}
- spin_unlock_bh(&tn->node_list_lock);
}
void tipc_net_stop(struct net *net)
--
2.7.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related
* Re: e1000e driver stuck at 10Mbps after reconnection
From: Camille Bordignon @ 2018-08-07 6:42 UTC (permalink / raw)
To: Alexander Duyck; +Cc: Jeff Kirsher, David S. Miller, intel-wired-lan, Netdev
In-Reply-To: <CAKgT0UcUV8bQRAhyevfnvwE+yWnWjwAkH0WumUoUK4Fa9SCzhg@mail.gmail.com>
Le lundi 06 août 2018 à 15:45:29 (-0700), Alexander Duyck a écrit :
> On Mon, Aug 6, 2018 at 4:59 AM, Camille Bordignon
> <camille.bordignon@easymile.com> wrote:
> > Hello,
> >
> > Recently we experienced some issues with intel NIC (I219-LM and I219-V).
> > It seems that after a wire reconnection, auto-negotation "fails" and
> > link speed drips to 10 Mbps.
> >
> > From kernel logs:
> > [17616.346150] e1000e: enp0s31f6 NIC Link is Down
> > [17627.003322] e1000e: enp0s31f6 NIC Link is Up 10 Mbps Full Duplex, Flow Control: None
> > [17627.003325] e1000e 0000:00:1f.6 enp0s31f6: 10/100 speed: disabling TSO
> >
> >
> > $ethtool enp0s31f6
> > Settings for enp0s31f6:
> > Supported ports: [ TP ]
> > Supported link modes: 10baseT/Half 10baseT/Full
> > 100baseT/Half 100baseT/Full
> > 1000baseT/Full
> > Supported pause frame use: No
> > Supports auto-negotiation: Yes
> > Supported FEC modes: Not reported
> > Advertised link modes: 10baseT/Half 10baseT/Full
> > 100baseT/Half 100baseT/Full
> > 1000baseT/Full
> > Advertised pause frame use: No
> > Advertised auto-negotiation: Yes
> > Advertised FEC modes: Not reported
> > Speed: 10Mb/s
> > Duplex: Full
> > Port: Twisted Pair
> > PHYAD: 1
> > Transceiver: internal
> > Auto-negotiation: on
> > MDI-X: on (auto)
> > Supports Wake-on: pumbg
> > Wake-on: g
> > Current message level: 0x00000007 (7)
> > drv probe link
> > Link detected: yes
> >
> >
> > Notice that if disconnection last less than about 5 seconds,
> > nothing wrong happens.
> > And if after last failure, disconnection / connection occurs again and
> > last less than 5 seconds, link speed is back to 1000 Mbps.
> >
> > [18075.350678] e1000e: enp0s31f6 NIC Link is Down
> > [18078.716245] e1000e: enp0s31f6 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: None
> >
> > The following patch seems to fix this issue.
> > However I don't clearly understand why.
> >
> > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> > index 3ba0c90e7055..763c013960f1 100644
> > --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> > @@ -5069,7 +5069,7 @@ static bool e1000e_has_link(struct e1000_adapter *adapter)
> > case e1000_media_type_copper:
> > if (hw->mac.get_link_status) {
> > ret_val = hw->mac.ops.check_for_link(hw);
> > - link_active = !hw->mac.get_link_status;
> > + link_active = false;
> > } else {
> > link_active = true;
> > }
> >
> > Maybe this is related to watchdog task.
> >
> > I've found out this fix by comparing with last commit that works fine :
> > commit 0b76aae741abb9d16d2c0e67f8b1e766576f897d.
> > However I don't know if this information is relevant.
> >
> > Thank you.
> > Camille Bordignon
>
> What kernel were you testing this on? I know there have been a number
> of changes over the past few months in this area and it would be
> useful to know exactly what code base you started out with and what
> the latest version of the kernel is you have tested.
>
> Looking over the code change the net effect of it should be to add a 2
> second delay from the time the link has changed until you actually
> check the speed/duplex configuration. It is possible we could be
> seeing some sort of timing issue and adding the 2 second delay after
> the link event is enough time for things to stabilize and detect the
> link at 1000 instead of 10/100.
>
> - Alex
We've found out this issue using Fedora 27 (4.17.11-100.fc27.x86_64).
Then I've tested wth a more recent version of the driver v4.18-rc7 but
behavior looks the same.
Thanks for you reply.
Camille Bordignon
^ permalink raw reply
* [PATCH 0/3] net: macb: add pad and fcs support
From: Claudiu Beznea @ 2018-08-07 9:25 UTC (permalink / raw)
To: nicolas.ferre, davem
Cc: netdev, linux-kernel, harinik, jennifer.dahm, nathan.sullivan,
rafalo, harinikatakamlinux, Claudiu Beznea
Hi,
In [1] it was reported that UDP checksum is offloaded to hardware no mather
it was previously computed in software or not. The proposal on [1] was to
disable TX checksum offload.
This series (mostly patch 3/3) address the issue described at [1] by
setting NOCRC bit to TX buffer descriptor for SKBs that arrived from
networking stack with checksum computed. For these packets padding and FCS
need to be added (hardware doesn't compute them if NOCRC bit is set). The
minimum packet size that hardware expects is 64 bytes (including FCS).
This feature could not be used in case of GSO, so, it was used only for
no GSO SKBs.
For SKBs wich requires padding and FCS computation macb_pad_and_fcs()
checks if there is enough headroom and tailroom in SKB to avoid copying
SKB structure. Since macb_pad_and_fcs() may change SKB the
macb_pad_and_fcs() was places in macb_start_xmit() b/w macb_csum_clear()
and skb_headlen() calls.
This patch was tested with pktgen in kernel tool in a script like this:
(pktgen_sample01_simple.sh is at [2]):
minSize=1
maxSize=1500
for i in `seq $minSize $maxSize` ; do
copy="$(shuf -i 1-2000 -n 1)"
./pktgen_sample01_simple.sh -i eth0 \
-m <dst-mac-addr> -d <dst-ip-addr> -x -s $i -c $copy
done
minStep=1
maxStep=200
for i in `seq $minStep $maxStep` ; do
copy="$(shuf -i 1-2000 -n 1)"
size="$(shuf -i 1-1500 -n 1)"
./pktgen_sample01_simple.sh -i eth0 \
-m <dst-mac-addr> -d <dst-ip-addr> -x -s $size -c $copy
done
Changes since RFC:
- in patch 3/3 order local variables by their lenght (reverse christmas tree
format)
[1] https://www.spinics.net/lists/netdev/msg505065.html
[2] https://github.com/netoptimizer/network-testing/blob/master/pktgen/pktgen_sample01_simple.sh
Claudiu Beznea (3):
net: macb: use netdev_tx_t return type for ndo_start_xmit functions
net: macb: move checksum clearing outside of spinlock.
net: macb: add support for padding and fcs computation
drivers/net/ethernet/cadence/macb_main.c | 88 +++++++++++++++++++++++++++++---
1 file changed, 80 insertions(+), 8 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 1/3] net: macb: use netdev_tx_t return type for ndo_start_xmit functions
From: Claudiu Beznea @ 2018-08-07 9:25 UTC (permalink / raw)
To: nicolas.ferre, davem
Cc: netdev, linux-kernel, harinik, jennifer.dahm, nathan.sullivan,
rafalo, harinikatakamlinux, Claudiu Beznea
In-Reply-To: <1533633914-30264-1-git-send-email-claudiu.beznea@microchip.com>
Use netdev_tx_t return type for ndo_start_xmit function of macb driver.
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
drivers/net/ethernet/cadence/macb_main.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 96cc03a6d942..860436474c3e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1651,7 +1651,7 @@ static inline int macb_clear_csum(struct sk_buff *skb)
return 0;
}
-static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
u16 queue_index = skb_get_queue_mapping(skb);
struct macb *bp = netdev_priv(dev);
@@ -1660,6 +1660,7 @@ static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
unsigned int desc_cnt, nr_frags, frag_size, f;
unsigned int hdrlen;
bool is_lso, is_udp = 0;
+ netdev_tx_t ret = NETDEV_TX_OK;
is_lso = (skb_shinfo(skb)->gso_size != 0);
@@ -1739,7 +1740,7 @@ static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
unlock:
spin_unlock_irqrestore(&bp->lock, flags);
- return NETDEV_TX_OK;
+ return ret;
}
static void macb_init_rx_buffer_size(struct macb *bp, size_t size)
@@ -3547,7 +3548,8 @@ static int at91ether_close(struct net_device *dev)
}
/* Transmit packet */
-static int at91ether_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t at91ether_start_xmit(struct sk_buff *skb,
+ struct net_device *dev)
{
struct macb *lp = netdev_priv(dev);
--
2.7.4
^ permalink raw reply related
* [PATCH 3/3] net: macb: add support for padding and fcs computation
From: Claudiu Beznea @ 2018-08-07 9:25 UTC (permalink / raw)
To: nicolas.ferre, davem
Cc: netdev, linux-kernel, harinik, jennifer.dahm, nathan.sullivan,
rafalo, harinikatakamlinux, Claudiu Beznea
In-Reply-To: <1533633914-30264-1-git-send-email-claudiu.beznea@microchip.com>
For packets with computed IP/TCP/UDP checksum there is no need to tell
hardware to recompute it. For such kind of packets hardware expects the
packet to be at least 64 bytes and FCS to be computed.
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
drivers/net/ethernet/cadence/macb_main.c | 70 ++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 1c12afe4a0ce..c16a4b947688 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -10,6 +10,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/clk.h>
+#include <linux/crc32.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/kernel.h>
@@ -1565,6 +1566,9 @@ static unsigned int macb_tx_map(struct macb *bp,
if (i == queue->tx_head) {
ctrl |= MACB_BF(TX_LSO, lso_ctrl);
ctrl |= MACB_BF(TX_TCP_SEQ_SRC, seq_ctrl);
+ if ((bp->dev->features & NETIF_F_HW_CSUM) &&
+ skb->ip_summed != CHECKSUM_PARTIAL && !lso_ctrl)
+ ctrl |= MACB_BIT(TX_NOCRC);
} else
/* Only set MSS/MFS on payload descriptors
* (second or later descriptor)
@@ -1651,6 +1655,67 @@ static inline int macb_clear_csum(struct sk_buff *skb)
return 0;
}
+static int macb_pad_and_fcs(struct sk_buff **skb, struct net_device *ndev)
+{
+ bool cloned = skb_cloned(*skb) || skb_header_cloned(*skb);
+ int padlen = ETH_ZLEN - (*skb)->len;
+ int headroom = skb_headroom(*skb);
+ int tailroom = skb_tailroom(*skb);
+ struct sk_buff *nskb;
+ u32 fcs;
+
+ if (!(ndev->features & NETIF_F_HW_CSUM) ||
+ !((*skb)->ip_summed != CHECKSUM_PARTIAL) ||
+ skb_shinfo(*skb)->gso_size) /* Not available for GSO */
+ return 0;
+
+ if (padlen <= 0) {
+ /* FCS could be appeded to tailroom. */
+ if (tailroom >= ETH_FCS_LEN)
+ goto add_fcs;
+ /* FCS could be appeded by moving data to headroom. */
+ else if (!cloned && headroom + tailroom >= ETH_FCS_LEN)
+ padlen = 0;
+ /* No room for FCS, need to reallocate skb. */
+ else
+ padlen = ETH_FCS_LEN - tailroom;
+ } else {
+ /* Add room for FCS. */
+ padlen += ETH_FCS_LEN;
+ }
+
+ if (!cloned && headroom + tailroom >= padlen) {
+ (*skb)->data = memmove((*skb)->head, (*skb)->data, (*skb)->len);
+ skb_set_tail_pointer(*skb, (*skb)->len);
+ } else {
+ nskb = skb_copy_expand(*skb, 0, padlen, GFP_ATOMIC);
+ if (!nskb)
+ return -ENOMEM;
+
+ dev_kfree_skb_any(*skb);
+ *skb = nskb;
+ }
+
+ if (padlen) {
+ if (padlen >= ETH_FCS_LEN)
+ skb_put_zero(*skb, padlen - ETH_FCS_LEN);
+ else
+ skb_trim(*skb, ETH_FCS_LEN - padlen);
+ }
+
+add_fcs:
+ /* set FCS to packet */
+ fcs = crc32_le(~0, (*skb)->data, (*skb)->len);
+ fcs = ~fcs;
+
+ skb_put_u8(*skb, fcs & 0xff);
+ skb_put_u8(*skb, (fcs >> 8) & 0xff);
+ skb_put_u8(*skb, (fcs >> 16) & 0xff);
+ skb_put_u8(*skb, (fcs >> 24) & 0xff);
+
+ return 0;
+}
+
static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
u16 queue_index = skb_get_queue_mapping(skb);
@@ -1667,6 +1732,11 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
return ret;
}
+ if (macb_pad_and_fcs(&skb, dev)) {
+ dev_kfree_skb_any(skb);
+ return ret;
+ }
+
is_lso = (skb_shinfo(skb)->gso_size != 0);
if (is_lso) {
--
2.7.4
^ permalink raw reply related
* [PATCH v4] can: sja1000: plx_pci: add support for ASEM CAN raw device
From: Flavio Suligoi @ 2018-08-07 7:17 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde
Cc: David S . Miller, linux-can, netdev, linux-kernel, Flavio Suligoi
In-Reply-To: <53dae6d6-c25b-71d5-b43f-1d444d9c352b@pengutronix.de>
This patch adds support for ASEM opto-isolated dual channels
CAN raw device (http://www.asem.it)
Signed-off-by: Flavio Suligoi <f.suligoi@asem.it>
---
v2: - rename ASEM_... constants to reduce space size;
- remove "else" in "plx_pci_reset_asem_dual_can_raw" function to avoid
strings breaking
v3: - fix wrong comment for PLX_LINT2_POL
- put string into just one line in "plx_pci_reset_asem_dual_can_raw"
function
v4: - remove unnecessary variable "reset_bar" in
"plx_pci_reset_asem_dual_can_raw" function
drivers/net/can/sja1000/Kconfig | 1 +
drivers/net/can/sja1000/plx_pci.c | 66 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 66 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/sja1000/Kconfig b/drivers/net/can/sja1000/Kconfig
index 1e65cb6..f6dc899 100644
--- a/drivers/net/can/sja1000/Kconfig
+++ b/drivers/net/can/sja1000/Kconfig
@@ -88,6 +88,7 @@ config CAN_PLX_PCI
- TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/)
- IXXAT Automation PC-I 04/PCI card (http://www.ixxat.com/)
- Connect Tech Inc. CANpro/104-Plus Opto (CRG001) card (http://www.connecttech.com)
+ - ASEM CAN raw - 2 isolated CAN channels (www.asem.it)
config CAN_TSCAN1
tristate "TS-CAN1 PC104 boards"
diff --git a/drivers/net/can/sja1000/plx_pci.c b/drivers/net/can/sja1000/plx_pci.c
index f8ff25c..79fc327 100644
--- a/drivers/net/can/sja1000/plx_pci.c
+++ b/drivers/net/can/sja1000/plx_pci.c
@@ -46,7 +46,8 @@ MODULE_SUPPORTED_DEVICE("Adlink PCI-7841/cPCI-7841, "
"esd CAN-PCIe/2000, "
"Connect Tech Inc. CANpro/104-Plus Opto (CRG001), "
"IXXAT PC-I 04/PCI, "
- "ELCUS CAN-200-PCI")
+ "ELCUS CAN-200-PCI, "
+ "ASEM DUAL CAN-RAW")
MODULE_LICENSE("GPL v2");
#define PLX_PCI_MAX_CHAN 2
@@ -70,7 +71,9 @@ struct plx_pci_card {
*/
#define PLX_LINT1_EN 0x1 /* Local interrupt 1 enable */
+#define PLX_LINT1_POL (1 << 1) /* Local interrupt 1 polarity */
#define PLX_LINT2_EN (1 << 3) /* Local interrupt 2 enable */
+#define PLX_LINT2_POL (1 << 4) /* Local interrupt 2 polarity */
#define PLX_PCI_INT_EN (1 << 6) /* PCI Interrupt Enable */
#define PLX_PCI_RESET (1 << 30) /* PCI Adapter Software Reset */
@@ -92,6 +95,9 @@ struct plx_pci_card {
*/
#define PLX_PCI_OCR (OCR_TX0_PUSHPULL | OCR_TX1_PUSHPULL)
+/* OCR setting for ASEM Dual CAN raw */
+#define ASEM_PCI_OCR 0xfe
+
/*
* In the CDR register, you should set CBP to 1.
* You will probably also want to set the clock divider value to 7
@@ -145,10 +151,20 @@ struct plx_pci_card {
#define MOXA_PCI_VENDOR_ID 0x1393
#define MOXA_PCI_DEVICE_ID 0x0100
+#define ASEM_RAW_CAN_VENDOR_ID 0x10b5
+#define ASEM_RAW_CAN_DEVICE_ID 0x9030
+#define ASEM_RAW_CAN_SUB_VENDOR_ID 0x3000
+#define ASEM_RAW_CAN_SUB_DEVICE_ID 0x1001
+#define ASEM_RAW_CAN_SUB_DEVICE_ID_BIS 0x1002
+#define ASEM_RAW_CAN_RST_REGISTER 0x54
+#define ASEM_RAW_CAN_RST_MASK_CAN1 0x20
+#define ASEM_RAW_CAN_RST_MASK_CAN2 0x04
+
static void plx_pci_reset_common(struct pci_dev *pdev);
static void plx9056_pci_reset_common(struct pci_dev *pdev);
static void plx_pci_reset_marathon_pci(struct pci_dev *pdev);
static void plx_pci_reset_marathon_pcie(struct pci_dev *pdev);
+static void plx_pci_reset_asem_dual_can_raw(struct pci_dev *pdev);
struct plx_pci_channel_map {
u32 bar;
@@ -269,6 +285,14 @@ static struct plx_pci_card_info plx_pci_card_info_moxa = {
/* based on PLX9052 */
};
+static struct plx_pci_card_info plx_pci_card_info_asem_dual_can = {
+ "ASEM Dual CAN raw PCI", 2,
+ PLX_PCI_CAN_CLOCK, ASEM_PCI_OCR, PLX_PCI_CDR,
+ {0, 0x00, 0x00}, { {2, 0x00, 0x00}, {4, 0x00, 0x00} },
+ &plx_pci_reset_asem_dual_can_raw
+ /* based on PLX9030 */
+};
+
static const struct pci_device_id plx_pci_tbl[] = {
{
/* Adlink PCI-7841/cPCI-7841 */
@@ -375,6 +399,20 @@ static const struct pci_device_id plx_pci_tbl[] = {
0, 0,
(kernel_ulong_t)&plx_pci_card_info_moxa
},
+ {
+ /* ASEM Dual CAN raw */
+ ASEM_RAW_CAN_VENDOR_ID, ASEM_RAW_CAN_DEVICE_ID,
+ ASEM_RAW_CAN_SUB_VENDOR_ID, ASEM_RAW_CAN_SUB_DEVICE_ID,
+ 0, 0,
+ (kernel_ulong_t)&plx_pci_card_info_asem_dual_can
+ },
+ {
+ /* ASEM Dual CAN raw -new model */
+ ASEM_RAW_CAN_VENDOR_ID, ASEM_RAW_CAN_DEVICE_ID,
+ ASEM_RAW_CAN_SUB_VENDOR_ID, ASEM_RAW_CAN_SUB_DEVICE_ID_BIS,
+ 0, 0,
+ (kernel_ulong_t)&plx_pci_card_info_asem_dual_can
+ },
{ 0,}
};
MODULE_DEVICE_TABLE(pci, plx_pci_tbl);
@@ -524,6 +562,32 @@ static void plx_pci_reset_marathon_pcie(struct pci_dev *pdev)
}
}
+/* Special reset function for ASEM Dual CAN raw card */
+static void plx_pci_reset_asem_dual_can_raw(struct pci_dev *pdev)
+{
+ void __iomem *bar0_addr;
+ u8 tmpval;
+
+ plx_pci_reset_common(pdev);
+
+ bar0_addr = pci_iomap(pdev, 0, 0);
+ if (!bar0_addr) {
+ dev_err(&pdev->dev, "Failed to remap reset space %d (BAR%d)\n",
+ 0, 0);
+ return;
+ }
+
+ /* reset the two SJA1000 chips */
+ tmpval = ioread8(bar0_addr + ASEM_RAW_CAN_RST_REGISTER);
+ tmpval &= ~(ASEM_RAW_CAN_RST_MASK_CAN1 | ASEM_RAW_CAN_RST_MASK_CAN2);
+ iowrite8(tmpval, bar0_addr + ASEM_RAW_CAN_RST_REGISTER);
+ usleep_range(300, 400);
+ tmpval |= ASEM_RAW_CAN_RST_MASK_CAN1 | ASEM_RAW_CAN_RST_MASK_CAN2;
+ iowrite8(tmpval, bar0_addr + ASEM_RAW_CAN_RST_REGISTER);
+ usleep_range(300, 400);
+ pci_iounmap(pdev, bar0_addr);
+}
+
static void plx_pci_del_card(struct pci_dev *pdev)
{
struct plx_pci_card *card = pci_get_drvdata(pdev);
--
2.7.4
^ permalink raw reply related
* Re: [RFC PATCH 0/2] net: macb: Disable TX checksum offloading on all Zynq
From: Claudiu Beznea @ 2018-08-07 8:50 UTC (permalink / raw)
To: Harini Katakam, Jennifer Dahm
Cc: netdev, David S . Miller, Nathan Sullivan, Rafal Ozieblo,
Harini Katakam, Nicolas Ferre
In-Reply-To: <CAFcVEC+q1Z=QBD67afVgXrh5khMUDoRmt9c2we2=780JEW0njA@mail.gmail.com>
Hi Harini,
On 01.08.2018 15:53, Harini Katakam wrote:
> Hi Jennifer,
>
> On Tue, Jun 5, 2018 at 10:21 AM, Harini Katakam <harinik@xilinx.com> wrote:
>> Hi Jeniffer,
>>
>> On Mon, Jun 4, 2018 at 8:35 PM, Nicolas Ferre
>> <nicolas.ferre@microchip.com> wrote:
>>> Jennifer,
>>>
>>> On 25/05/2018 at 23:44, Jennifer Dahm wrote:
>>>>
>>>> During testing, I discovered that the Zynq GEM hardware overwrites all
>>>> outgoing UDP packet checksums, which is illegal in packet forwarding
>>>> cases. This happens both with and without the checksum-zeroing
>>>> behavior introduced in 007e4ba3ee137f4700f39aa6dbaf01a71047c5f6
>>>> ("net: macb: initialize checksum when using checksum offloading"). The
>>>> only solution to both the small packet bug and the packet forwarding
>>>> bug that I can find is to disable TX checksum offloading entirely.
>>>
>>>
>>
>> Thanks for the extensive testing.
>> I'll try to reproduce and see if it is something to be fixed in the driver.
>>
>>> Are the bugs listed above present in all revisions of the GEM IP, only for
>>> some revisions?
>>> Is there an errata that describe this issue for the Zynq GEM?
>>
>> @Nicolas, AFAIK, there is no errata for this in either Cadence or
>> Zynq documentation.
>
> I was unable to reproduce this issue on Zynq.
> Although I do not have HW with two GEM ports,
> I tried by routing one GEM via PL and another via on board RGMII.
> Since there was no specific errata related to this, I also tried on
> subsequent ZynqMP versions with multiple GEM ports but dint find any
> checksum issues. I discussed the same with cadence and they
> tried the test with 2 bytes of UDP payload on the Zynq GEM IP
> version in their regressions and did not hit any issue either.
>
> I tried to reach out earlier to see if you can share your exact
> application. Could you please let me know if you have any
> further updates?
I manage to reproduce the issue and provide a patch for this (see patch 3/3
from [1]).
[1] https://www.spinics.net/lists/netdev/msg513848.html
>
> Regards,
> Harini
>
^ permalink raw reply
* KASAN: use-after-free Read in iotlb_access_ok
From: syzbot @ 2018-08-07 11:16 UTC (permalink / raw)
To: jasowang, kvm, linux-kernel, mst, netdev, syzkaller-bugs,
virtualization
Hello,
syzbot found the following crash on:
HEAD commit: e30cb13c5a09 Merge git://git.kernel.org/pub/scm/linux/kern..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=10a153e0400000
kernel config: https://syzkaller.appspot.com/x/.config?x=2dc0cd7c2eefb46f
dashboard link: https://syzkaller.appspot.com/bug?extid=c51e6736a1bf614b3272
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+c51e6736a1bf614b3272@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: use-after-free in vhost_vq_meta_fetch drivers/vhost/vhost.c:702
[inline]
BUG: KASAN: use-after-free in iotlb_access_ok+0x5c9/0x600
drivers/vhost/vhost.c:1177
Read of size 8 at addr ffff880197df2fc0 by task vhost-8938/8941
CPU: 0 PID: 8941 Comm: vhost-8938 Not tainted 4.18.0-rc7+ #174
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
print_address_description+0x6c/0x20b mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
__asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
vhost_vq_meta_fetch drivers/vhost/vhost.c:702 [inline]
iotlb_access_ok+0x5c9/0x600 drivers/vhost/vhost.c:1177
vq_iotlb_prefetch+0x10e/0x230 drivers/vhost/vhost.c:1214
handle_rx+0x247/0x1f80 drivers/vhost/net.c:799
handle_rx_net+0x19/0x20 drivers/vhost/net.c:934
vhost_worker+0x283/0x490 drivers/vhost/vhost.c:360
kthread+0x345/0x410 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
Allocated by task 8938:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
kmem_cache_alloc_trace+0x152/0x780 mm/slab.c:3620
kmalloc include/linux/slab.h:513 [inline]
vhost_new_umem_range+0xcb/0x7c0 drivers/vhost/vhost.c:911
vhost_process_iotlb_msg drivers/vhost/vhost.c:1000 [inline]
vhost_chr_write_iter+0xe53/0x1a00 drivers/vhost/vhost.c:1043
vhost_net_chr_write_iter+0x59/0x70 drivers/vhost/net.c:1399
call_write_iter include/linux/fs.h:1793 [inline]
new_sync_write fs/read_write.c:474 [inline]
__vfs_write+0x6c6/0x9f0 fs/read_write.c:487
vfs_write+0x1f8/0x560 fs/read_write.c:549
ksys_write+0x101/0x260 fs/read_write.c:598
__do_sys_write fs/read_write.c:610 [inline]
__se_sys_write fs/read_write.c:607 [inline]
__x64_sys_write+0x73/0xb0 fs/read_write.c:607
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 8950:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
__cache_free mm/slab.c:3498 [inline]
kfree+0xd9/0x260 mm/slab.c:3813
vhost_umem_free+0x944/0x14d0 drivers/vhost/vhost.c:576
vhost_umem_clean+0x83/0xf0 drivers/vhost/vhost.c:588
vhost_init_device_iotlb+0x1d7/0x290 drivers/vhost/vhost.c:1568
vhost_net_set_features drivers/vhost/net.c:1292 [inline]
vhost_net_ioctl+0xff3/0x1a80 drivers/vhost/net.c:1357
vfs_ioctl fs/ioctl.c:46 [inline]
file_ioctl fs/ioctl.c:500 [inline]
do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:684
ksys_ioctl+0xa9/0xd0 fs/ioctl.c:701
__do_sys_ioctl fs/ioctl.c:708 [inline]
__se_sys_ioctl fs/ioctl.c:706 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:706
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
The buggy address belongs to the object at ffff880197df2f80
which belongs to the cache kmalloc-96 of size 96
The buggy address is located 64 bytes inside of
96-byte region [ffff880197df2f80, ffff880197df2fe0)
The buggy address belongs to the page:
page:ffffea00065f7c80 count:1 mapcount:0 mapping:ffff8801dac004c0 index:0x0
flags: 0x2fffc0000000100(slab)
raw: 02fffc0000000100 ffffea0007530148 ffffea000663b888 ffff8801dac004c0
raw: 0000000000000000 ffff880197df2000 0000000100000020 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff880197df2e80: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
ffff880197df2f00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> ffff880197df2f80: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
^
ffff880197df3000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff880197df3080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
^ permalink raw reply
* [PATCH 0/3] net: remove unneeded variable for return value
From: zhong jiang @ 2018-08-07 11:20 UTC (permalink / raw)
To: davem, anna.schumaker; +Cc: netdev, linux-kernel
zhong jiang (3):
net:svc_rdma_transport: remove unneeded variable 'ret' in
rdma_listen_handler
net:af_iucv: get rid of the unneeded variable 'err' in
afiucv_pm_freeze
net:mod: remove unneeded variable 'ret' in init_p9
net/9p/mod.c | 4 +---
net/iucv/af_iucv.c | 3 +--
net/sunrpc/xprtrdma/svc_rdma_transport.c | 3 +--
3 files changed, 3 insertions(+), 7 deletions(-)
--
1.7.12.4
^ permalink raw reply
* [PATCH 1/3] net:svc_rdma_transport: remove unneeded variable 'ret' in rdma_listen_handler
From: zhong jiang @ 2018-08-07 11:20 UTC (permalink / raw)
To: davem, anna.schumaker; +Cc: netdev, linux-kernel
In-Reply-To: <1533640809-1221-1-git-send-email-zhongjiang@huawei.com>
The ret is not modified after initalization, So just remove the variable
and return 0.
Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
net/sunrpc/xprtrdma/svc_rdma_transport.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
index 547b2cd..2848caf 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -296,7 +296,6 @@ static int rdma_listen_handler(struct rdma_cm_id *cma_id,
struct rdma_cm_event *event)
{
struct sockaddr *sap = (struct sockaddr *)&cma_id->route.addr.src_addr;
- int ret = 0;
trace_svcrdma_cm_event(event, sap);
@@ -315,7 +314,7 @@ static int rdma_listen_handler(struct rdma_cm_id *cma_id,
break;
}
- return ret;
+ return 0;
}
static int rdma_cma_handler(struct rdma_cm_id *cma_id,
--
1.7.12.4
^ 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