* [PATCH v2 net-next 7/7] qed: Fix possible race when reading firmware return code.
From: Manish Chopra @ 2016-10-13 11:22 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Manish Chopra
In-Reply-To: <1476357771-1578-1-git-send-email-manish.chopra@qlogic.com>
From: Manish Chopra <manish.chopra@caviumnetworks.com>
While handling SPQ ramrod completion, there is a possible race
where driver might not read updated fw return code based on
ramrod completion done. This patch ensures that fw return code
is written first and then completion done flag is updated
using appropriate memory barriers.
Signed-off-by: Manish Chopra <manish.chopra@caviumnetworks.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qed/qed_spq.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_spq.c b/drivers/net/ethernet/qlogic/qed/qed_spq.c
index 259a615..6c05402 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_spq.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_spq.c
@@ -54,11 +54,10 @@ static void qed_spq_blocking_cb(struct qed_hwfn *p_hwfn,
comp_done = (struct qed_spq_comp_done *)cookie;
- comp_done->done = 0x1;
- comp_done->fw_return_code = fw_return_code;
+ comp_done->fw_return_code = fw_return_code;
- /* make update visible to waiting thread */
- smp_wmb();
+ /* Make sure completion done is visible on waiting thread */
+ smp_store_release(&comp_done->done, 0x1);
}
static int __qed_spq_block(struct qed_hwfn *p_hwfn,
@@ -74,8 +73,9 @@ static int __qed_spq_block(struct qed_hwfn *p_hwfn,
while (iter_cnt--) {
/* Validate we receive completion update */
- smp_rmb();
- if (comp_done->done == 1) {
+ if (READ_ONCE(comp_done->done) == 1) {
+ /* Read updated FW return value */
+ smp_read_barrier_depends();
if (p_fw_ret)
*p_fw_ret = comp_done->fw_return_code;
return 0;
--
2.7.2
^ permalink raw reply related
* [PATCH v2 net-next 0/7] qed*: driver updates
From: Manish Chopra @ 2016-10-13 11:22 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Manish Chopra
From: Manish Chopra <manish.chopra@caviumnetworks.com>
Hi David,
There are several new additions in this series;
Most are connected to either Tx offloading or Rx classifications
[either fastpath changes or supporting configuration].
In addition, there's a single IOV enhancement.
Please consider applying this series to `net-next'.
V2:
Added a fix for the race in ramrod handling
pointed by Eric Dumazet [patch 7].
Thanks,
Manish
Manish Chopra (3):
qede: GSO support for tunnels with outer csum
qede: Prevent GSO on long Geneve headers
qed: Fix possible race when reading firmware return code.
Yuval Mintz (4):
qed: Pass MAC hints to VFs
qed*: Allow unicast filtering
qed: Allow chance for fast ramrod completions
qed: Handle malicious VFs events
drivers/net/ethernet/qlogic/qed/qed_l2.c | 12 ++-
drivers/net/ethernet/qlogic/qed/qed_spq.c | 97 +++++++++++++++--------
drivers/net/ethernet/qlogic/qed/qed_sriov.c | 114 ++++++++++++++++++++++-----
drivers/net/ethernet/qlogic/qed/qed_sriov.h | 1 +
drivers/net/ethernet/qlogic/qed/qed_vf.c | 4 +-
drivers/net/ethernet/qlogic/qed/qed_vf.h | 1 +
drivers/net/ethernet/qlogic/qede/qede.h | 1 +
drivers/net/ethernet/qlogic/qede/qede_main.c | 71 +++++++++++++++--
include/linux/qed/qed_eth_if.h | 3 +-
9 files changed, 242 insertions(+), 62 deletions(-)
--
2.7.2
^ permalink raw reply
* [PATCH v2 net-next 6/7] qed: Handle malicious VFs events
From: Manish Chopra @ 2016-10-13 11:22 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Yuval Mintz
In-Reply-To: <1476357771-1578-1-git-send-email-manish.chopra@qlogic.com>
From: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
Malicious VFs might be caught in several different methods:
- Misusing their bar permission and being blocked by hardware.
- Misusing their fastpath logic and being blocked by firmware.
- Misusing their interaction with their PF via hw-channel,
and being blocked by PF driver.
On the first two items, firmware would indicate to driver that
the VF is to be considered malicious, but would sometime still
allow the VF to communicate with the PF [depending on the exact
nature of the malicious activity done by the VF].
The current existing logic on the PF side lacks handling of such events,
and might allow the PF to perform some incorrect configuration on behalf
of a VF that was previously indicated as malicious.
The new scheme is simple -
Once the PF determines a VF is malicious it would:
a. Ignore any further requests on behalf of the VF-driver.
b. Prevent any configurations initiated by the hyperuser for
the malicious VF, as firmware isn't willing to serve such.
The malicious indication would be cleared upon the VF flr,
after which it would become usable once again.
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qed/qed_sriov.c | 114 +++++++++++++++++++++++-----
drivers/net/ethernet/qlogic/qed/qed_sriov.h | 1 +
drivers/net/ethernet/qlogic/qed/qed_vf.h | 1 +
3 files changed, 96 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_sriov.c b/drivers/net/ethernet/qlogic/qed/qed_sriov.c
index d2d6621..6f029f9 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_sriov.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_sriov.c
@@ -109,7 +109,8 @@ static int qed_sp_vf_stop(struct qed_hwfn *p_hwfn,
}
static bool qed_iov_is_valid_vfid(struct qed_hwfn *p_hwfn,
- int rel_vf_id, bool b_enabled_only)
+ int rel_vf_id,
+ bool b_enabled_only, bool b_non_malicious)
{
if (!p_hwfn->pf_iov_info) {
DP_NOTICE(p_hwfn->cdev, "No iov info\n");
@@ -124,6 +125,10 @@ static bool qed_iov_is_valid_vfid(struct qed_hwfn *p_hwfn,
b_enabled_only)
return false;
+ if ((p_hwfn->pf_iov_info->vfs_array[rel_vf_id].b_malicious) &&
+ b_non_malicious)
+ return false;
+
return true;
}
@@ -138,7 +143,8 @@ static struct qed_vf_info *qed_iov_get_vf_info(struct qed_hwfn *p_hwfn,
return NULL;
}
- if (qed_iov_is_valid_vfid(p_hwfn, relative_vf_id, b_enabled_only))
+ if (qed_iov_is_valid_vfid(p_hwfn, relative_vf_id,
+ b_enabled_only, false))
vf = &p_hwfn->pf_iov_info->vfs_array[relative_vf_id];
else
DP_ERR(p_hwfn, "qed_iov_get_vf_info: VF[%d] is not enabled\n",
@@ -542,7 +548,8 @@ int qed_iov_hw_info(struct qed_hwfn *p_hwfn)
return 0;
}
-static bool qed_iov_pf_sanity_check(struct qed_hwfn *p_hwfn, int vfid)
+bool _qed_iov_pf_sanity_check(struct qed_hwfn *p_hwfn,
+ int vfid, bool b_fail_malicious)
{
/* Check PF supports sriov */
if (IS_VF(p_hwfn->cdev) || !IS_QED_SRIOV(p_hwfn->cdev) ||
@@ -550,12 +557,17 @@ static bool qed_iov_pf_sanity_check(struct qed_hwfn *p_hwfn, int vfid)
return false;
/* Check VF validity */
- if (!qed_iov_is_valid_vfid(p_hwfn, vfid, true))
+ if (!qed_iov_is_valid_vfid(p_hwfn, vfid, true, b_fail_malicious))
return false;
return true;
}
+bool qed_iov_pf_sanity_check(struct qed_hwfn *p_hwfn, int vfid)
+{
+ return _qed_iov_pf_sanity_check(p_hwfn, vfid, true);
+}
+
static void qed_iov_set_vf_to_disable(struct qed_dev *cdev,
u16 rel_vf_id, u8 to_disable)
{
@@ -652,6 +664,9 @@ static int qed_iov_enable_vf_access(struct qed_hwfn *p_hwfn,
qed_iov_vf_igu_reset(p_hwfn, p_ptt, vf);
+ /* It's possible VF was previously considered malicious */
+ vf->b_malicious = false;
+
rc = qed_mcp_config_vf_msix(p_hwfn, p_ptt, vf->abs_vf_id, vf->num_sbs);
if (rc)
return rc;
@@ -2804,6 +2819,13 @@ qed_iov_execute_vf_flr_cleanup(struct qed_hwfn *p_hwfn,
return rc;
}
+ /* Workaround to make VF-PF channel ready, as FW
+ * doesn't do that as a part of FLR.
+ */
+ REG_WR(p_hwfn,
+ GTT_BAR0_MAP_REG_USDM_RAM +
+ USTORM_VF_PF_CHANNEL_READY_OFFSET(vfid), 1);
+
/* VF_STOPPED has to be set only after final cleanup
* but prior to re-enabling the VF.
*/
@@ -2942,7 +2964,8 @@ static void qed_iov_process_mbx_req(struct qed_hwfn *p_hwfn,
mbx->first_tlv = mbx->req_virt->first_tlv;
/* check if tlv type is known */
- if (qed_iov_tlv_supported(mbx->first_tlv.tl.type)) {
+ if (qed_iov_tlv_supported(mbx->first_tlv.tl.type) &&
+ !p_vf->b_malicious) {
switch (mbx->first_tlv.tl.type) {
case CHANNEL_TLV_ACQUIRE:
qed_iov_vf_mbx_acquire(p_hwfn, p_ptt, p_vf);
@@ -2984,6 +3007,15 @@ static void qed_iov_process_mbx_req(struct qed_hwfn *p_hwfn,
qed_iov_vf_mbx_release(p_hwfn, p_ptt, p_vf);
break;
}
+ } else if (qed_iov_tlv_supported(mbx->first_tlv.tl.type)) {
+ DP_VERBOSE(p_hwfn, QED_MSG_IOV,
+ "VF [%02x] - considered malicious; Ignoring TLV [%04x]\n",
+ p_vf->abs_vf_id, mbx->first_tlv.tl.type);
+
+ qed_iov_prepare_resp(p_hwfn, p_ptt, p_vf,
+ mbx->first_tlv.tl.type,
+ sizeof(struct pfvf_def_resp_tlv),
+ PFVF_STATUS_MALICIOUS);
} else {
/* unknown TLV - this may belong to a VF driver from the future
* - a version written after this PF driver was written, which
@@ -3033,20 +3065,30 @@ static void qed_iov_pf_get_and_clear_pending_events(struct qed_hwfn *p_hwfn,
memset(p_pending_events, 0, sizeof(u64) * QED_VF_ARRAY_LENGTH);
}
-static int qed_sriov_vfpf_msg(struct qed_hwfn *p_hwfn,
- u16 abs_vfid, struct regpair *vf_msg)
+static struct qed_vf_info *qed_sriov_get_vf_from_absid(struct qed_hwfn *p_hwfn,
+ u16 abs_vfid)
{
- u8 min = (u8)p_hwfn->cdev->p_iov_info->first_vf_in_pf;
- struct qed_vf_info *p_vf;
+ u8 min = (u8) p_hwfn->cdev->p_iov_info->first_vf_in_pf;
- if (!qed_iov_pf_sanity_check(p_hwfn, (int)abs_vfid - min)) {
+ if (!_qed_iov_pf_sanity_check(p_hwfn, (int)abs_vfid - min, false)) {
DP_VERBOSE(p_hwfn,
QED_MSG_IOV,
- "Got a message from VF [abs 0x%08x] that cannot be handled by PF\n",
+ "Got indication for VF [abs 0x%08x] that cannot be handled by PF\n",
abs_vfid);
- return 0;
+ return NULL;
}
- p_vf = &p_hwfn->pf_iov_info->vfs_array[(u8)abs_vfid - min];
+
+ return &p_hwfn->pf_iov_info->vfs_array[(u8) abs_vfid - min];
+}
+
+static int qed_sriov_vfpf_msg(struct qed_hwfn *p_hwfn,
+ u16 abs_vfid, struct regpair *vf_msg)
+{
+ struct qed_vf_info *p_vf = qed_sriov_get_vf_from_absid(p_hwfn,
+ abs_vfid);
+
+ if (!p_vf)
+ return 0;
/* List the physical address of the request so that handler
* could later on copy the message from it.
@@ -3060,6 +3102,23 @@ static int qed_sriov_vfpf_msg(struct qed_hwfn *p_hwfn,
return 0;
}
+static void qed_sriov_vfpf_malicious(struct qed_hwfn *p_hwfn,
+ struct malicious_vf_eqe_data *p_data)
+{
+ struct qed_vf_info *p_vf;
+
+ p_vf = qed_sriov_get_vf_from_absid(p_hwfn, p_data->vf_id);
+
+ if (!p_vf)
+ return;
+
+ DP_INFO(p_hwfn,
+ "VF [%d] - Malicious behavior [%02x]\n",
+ p_vf->abs_vf_id, p_data->err_id);
+
+ p_vf->b_malicious = true;
+}
+
int qed_sriov_eqe_event(struct qed_hwfn *p_hwfn,
u8 opcode, __le16 echo, union event_ring_data *data)
{
@@ -3067,6 +3126,9 @@ int qed_sriov_eqe_event(struct qed_hwfn *p_hwfn,
case COMMON_EVENT_VF_PF_CHANNEL:
return qed_sriov_vfpf_msg(p_hwfn, le16_to_cpu(echo),
&data->vf_pf_channel.msg_addr);
+ case COMMON_EVENT_MALICIOUS_VF:
+ qed_sriov_vfpf_malicious(p_hwfn, &data->malicious_vf);
+ return 0;
default:
DP_INFO(p_hwfn->cdev, "Unknown sriov eqe event 0x%02x\n",
opcode);
@@ -3083,7 +3145,7 @@ u16 qed_iov_get_next_active_vf(struct qed_hwfn *p_hwfn, u16 rel_vf_id)
goto out;
for (i = rel_vf_id; i < p_iov->total_vfs; i++)
- if (qed_iov_is_valid_vfid(p_hwfn, rel_vf_id, true))
+ if (qed_iov_is_valid_vfid(p_hwfn, rel_vf_id, true, false))
return i;
out:
@@ -3130,6 +3192,12 @@ static void qed_iov_bulletin_set_forced_mac(struct qed_hwfn *p_hwfn,
return;
}
+ if (vf_info->b_malicious) {
+ DP_NOTICE(p_hwfn->cdev,
+ "Can't set forced MAC to malicious VF [%d]\n", vfid);
+ return;
+ }
+
feature = 1 << MAC_ADDR_FORCED;
memcpy(vf_info->bulletin.p_virt->mac, mac, ETH_ALEN);
@@ -3153,6 +3221,12 @@ static void qed_iov_bulletin_set_forced_vlan(struct qed_hwfn *p_hwfn,
return;
}
+ if (vf_info->b_malicious) {
+ DP_NOTICE(p_hwfn->cdev,
+ "Can't set forced vlan to malicious VF [%d]\n", vfid);
+ return;
+ }
+
feature = 1 << VLAN_ADDR_FORCED;
vf_info->bulletin.p_virt->pvid = pvid;
if (pvid)
@@ -3367,7 +3441,7 @@ int qed_sriov_disable(struct qed_dev *cdev, bool pci_enabled)
qed_for_each_vf(hwfn, j) {
int k;
- if (!qed_iov_is_valid_vfid(hwfn, j, true))
+ if (!qed_iov_is_valid_vfid(hwfn, j, true, false))
continue;
/* Wait until VF is disabled before releasing */
@@ -3425,7 +3499,7 @@ static int qed_sriov_enable(struct qed_dev *cdev, int num)
num_sbs = min_t(int, sb_cnt_info.sb_free_blk, limit);
for (i = 0; i < num; i++) {
- if (!qed_iov_is_valid_vfid(hwfn, i, false))
+ if (!qed_iov_is_valid_vfid(hwfn, i, false, true))
continue;
rc = qed_iov_init_hw_for_vf(hwfn,
@@ -3477,7 +3551,7 @@ static int qed_sriov_pf_set_mac(struct qed_dev *cdev, u8 *mac, int vfid)
return -EINVAL;
}
- if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vfid, true)) {
+ if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vfid, true, true)) {
DP_VERBOSE(cdev, QED_MSG_IOV,
"Cannot set VF[%d] MAC (VF is not active)\n", vfid);
return -EINVAL;
@@ -3509,7 +3583,7 @@ static int qed_sriov_pf_set_vlan(struct qed_dev *cdev, u16 vid, int vfid)
return -EINVAL;
}
- if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vfid, true)) {
+ if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vfid, true, true)) {
DP_VERBOSE(cdev, QED_MSG_IOV,
"Cannot set VF[%d] MAC (VF is not active)\n", vfid);
return -EINVAL;
@@ -3543,7 +3617,7 @@ static int qed_get_vf_config(struct qed_dev *cdev,
if (IS_VF(cdev))
return -EINVAL;
- if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vf_id, true)) {
+ if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vf_id, true, false)) {
DP_VERBOSE(cdev, QED_MSG_IOV,
"VF index [%d] isn't active\n", vf_id);
return -EINVAL;
@@ -3647,7 +3721,7 @@ static int qed_set_vf_link_state(struct qed_dev *cdev,
if (IS_VF(cdev))
return -EINVAL;
- if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vf_id, true)) {
+ if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vf_id, true, true)) {
DP_VERBOSE(cdev, QED_MSG_IOV,
"VF index [%d] isn't active\n", vf_id);
return -EINVAL;
diff --git a/drivers/net/ethernet/qlogic/qed/qed_sriov.h b/drivers/net/ethernet/qlogic/qed/qed_sriov.h
index 0dd23e4..3cf515b 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_sriov.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_sriov.h
@@ -132,6 +132,7 @@ struct qed_vf_info {
struct qed_iov_vf_mbx vf_mbx;
enum vf_state state;
bool b_init;
+ bool b_malicious;
u8 to_disable;
struct qed_bulletin bulletin;
diff --git a/drivers/net/ethernet/qlogic/qed/qed_vf.h b/drivers/net/ethernet/qlogic/qed/qed_vf.h
index 35db7a28..944745b 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_vf.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_vf.h
@@ -40,6 +40,7 @@ enum {
PFVF_STATUS_NOT_SUPPORTED,
PFVF_STATUS_NO_RESOURCE,
PFVF_STATUS_FORCED,
+ PFVF_STATUS_MALICIOUS,
};
/* vf pf channel tlvs */
--
2.7.2
^ permalink raw reply related
* [PATCH v2 net-next 3/7] qede: Prevent GSO on long Geneve headers
From: Manish Chopra @ 2016-10-13 11:22 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Manish Chopra
In-Reply-To: <1476357771-1578-1-git-send-email-manish.chopra@qlogic.com>
From: Manish Chopra <manish.chopra@caviumnetworks.com>
Due to hardware limitation, when transmitting a geneve-encapsulated
packet with more than 32 bytes worth of geneve options the hardware
would not be able to crack the packet and consider it a regular UDP
packet.
This implements the ndo_features_check() in qede in order to prevent
GSO on said transmitted packets.
Signed-off-by: Manish Chopra <manish.chopra@caviumnetworks.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qede/qede_main.c | 35 ++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 7d5dc1e..6c2b09c 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -2240,6 +2240,40 @@ static void qede_udp_tunnel_del(struct net_device *dev,
schedule_delayed_work(&edev->sp_task, 0);
}
+/* 8B udp header + 8B base tunnel header + 32B option length */
+#define QEDE_MAX_TUN_HDR_LEN 48
+
+static netdev_features_t qede_features_check(struct sk_buff *skb,
+ struct net_device *dev,
+ netdev_features_t features)
+{
+ if (skb->encapsulation) {
+ u8 l4_proto = 0;
+
+ switch (vlan_get_protocol(skb)) {
+ case htons(ETH_P_IP):
+ l4_proto = ip_hdr(skb)->protocol;
+ break;
+ case htons(ETH_P_IPV6):
+ l4_proto = ipv6_hdr(skb)->nexthdr;
+ break;
+ default:
+ return features;
+ }
+
+ /* Disable offloads for geneve tunnels, as HW can't parse
+ * the geneve header which has option length greater than 32B.
+ */
+ if ((l4_proto == IPPROTO_UDP) &&
+ ((skb_inner_mac_header(skb) -
+ skb_transport_header(skb)) > QEDE_MAX_TUN_HDR_LEN))
+ return features & ~(NETIF_F_CSUM_MASK |
+ NETIF_F_GSO_MASK);
+ }
+
+ return features;
+}
+
static const struct net_device_ops qede_netdev_ops = {
.ndo_open = qede_open,
.ndo_stop = qede_close,
@@ -2264,6 +2298,7 @@ static const struct net_device_ops qede_netdev_ops = {
#endif
.ndo_udp_tunnel_add = qede_udp_tunnel_add,
.ndo_udp_tunnel_del = qede_udp_tunnel_del,
+ .ndo_features_check = qede_features_check,
};
/* -------------------------------------------------------------------------
--
2.7.2
^ permalink raw reply related
* [PATCH v2 net-next 5/7] qed: Allow chance for fast ramrod completions
From: Manish Chopra @ 2016-10-13 11:22 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Yuval Mintz
In-Reply-To: <1476357771-1578-1-git-send-email-manish.chopra@qlogic.com>
From: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
Whenever a ramrod is being sent for some device configuration,
the driver is going to sleep at least 5ms between each iteration
of polling on the completion of the ramrod.
However, in almost every configuration scenario the firmware
would be able to comply and complete the ramrod in a manner of
several usecs. This is especially important in cases where there
might be a lot of sequential configurations applying to the hardware
[e.g., RoCE], in which case the existing scheme might cause some
visible user delays.
This patch changes the completion scheme - instead of immediately
starting to sleep for a 'long' period, allow the device to quickly
poll on the first iteration after a couple of usecs.
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qed/qed_spq.c | 85 +++++++++++++++++++++----------
1 file changed, 59 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_spq.c b/drivers/net/ethernet/qlogic/qed/qed_spq.c
index caff415..259a615 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_spq.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_spq.c
@@ -37,7 +37,11 @@
***************************************************************************/
#define SPQ_HIGH_PRI_RESERVE_DEFAULT (1)
-#define SPQ_BLOCK_SLEEP_LENGTH (1000)
+
+#define SPQ_BLOCK_DELAY_MAX_ITER (10)
+#define SPQ_BLOCK_DELAY_US (10)
+#define SPQ_BLOCK_SLEEP_MAX_ITER (1000)
+#define SPQ_BLOCK_SLEEP_MS (5)
/***************************************************************************
* Blocking Imp. (BLOCK/EBLOCK mode)
@@ -57,53 +61,81 @@ static void qed_spq_blocking_cb(struct qed_hwfn *p_hwfn,
smp_wmb();
}
-static int qed_spq_block(struct qed_hwfn *p_hwfn,
- struct qed_spq_entry *p_ent,
- u8 *p_fw_ret)
+static int __qed_spq_block(struct qed_hwfn *p_hwfn,
+ struct qed_spq_entry *p_ent,
+ u8 *p_fw_ret, bool sleep_between_iter)
{
- int sleep_count = SPQ_BLOCK_SLEEP_LENGTH;
struct qed_spq_comp_done *comp_done;
- int rc;
+ u32 iter_cnt;
comp_done = (struct qed_spq_comp_done *)p_ent->comp_cb.cookie;
- while (sleep_count) {
- /* validate we receive completion update */
+ iter_cnt = sleep_between_iter ? SPQ_BLOCK_SLEEP_MAX_ITER
+ : SPQ_BLOCK_DELAY_MAX_ITER;
+
+ while (iter_cnt--) {
+ /* Validate we receive completion update */
smp_rmb();
if (comp_done->done == 1) {
if (p_fw_ret)
*p_fw_ret = comp_done->fw_return_code;
return 0;
}
- usleep_range(5000, 10000);
- sleep_count--;
+
+ if (sleep_between_iter)
+ msleep(SPQ_BLOCK_SLEEP_MS);
+ else
+ udelay(SPQ_BLOCK_DELAY_US);
}
+ return -EBUSY;
+}
+
+static int qed_spq_block(struct qed_hwfn *p_hwfn,
+ struct qed_spq_entry *p_ent,
+ u8 *p_fw_ret, bool skip_quick_poll)
+{
+ struct qed_spq_comp_done *comp_done;
+ int rc;
+
+ /* A relatively short polling period w/o sleeping, to allow the FW to
+ * complete the ramrod and thus possibly to avoid the following sleeps.
+ */
+ if (!skip_quick_poll) {
+ rc = __qed_spq_block(p_hwfn, p_ent, p_fw_ret, false);
+ if (!rc)
+ return 0;
+ }
+
+ /* Move to polling with a sleeping period between iterations */
+ rc = __qed_spq_block(p_hwfn, p_ent, p_fw_ret, true);
+ if (!rc)
+ return 0;
+
DP_INFO(p_hwfn, "Ramrod is stuck, requesting MCP drain\n");
rc = qed_mcp_drain(p_hwfn, p_hwfn->p_main_ptt);
- if (rc != 0)
+ if (rc) {
DP_NOTICE(p_hwfn, "MCP drain failed\n");
+ goto err;
+ }
/* Retry after drain */
- sleep_count = SPQ_BLOCK_SLEEP_LENGTH;
- while (sleep_count) {
- /* validate we receive completion update */
- smp_rmb();
- if (comp_done->done == 1) {
- if (p_fw_ret)
- *p_fw_ret = comp_done->fw_return_code;
- return 0;
- }
- usleep_range(5000, 10000);
- sleep_count--;
- }
+ rc = __qed_spq_block(p_hwfn, p_ent, p_fw_ret, true);
+ if (!rc)
+ return 0;
+ comp_done = (struct qed_spq_comp_done *)p_ent->comp_cb.cookie;
if (comp_done->done == 1) {
if (p_fw_ret)
*p_fw_ret = comp_done->fw_return_code;
return 0;
}
-
- DP_NOTICE(p_hwfn, "Ramrod is stuck, MCP drain failed\n");
+err:
+ DP_NOTICE(p_hwfn,
+ "Ramrod is stuck [CID %08x cmd %02x protocol %02x echo %04x]\n",
+ le32_to_cpu(p_ent->elem.hdr.cid),
+ p_ent->elem.hdr.cmd_id,
+ p_ent->elem.hdr.protocol_id,
+ le16_to_cpu(p_ent->elem.hdr.echo));
return -EBUSY;
}
@@ -729,7 +761,8 @@ int qed_spq_post(struct qed_hwfn *p_hwfn,
* access p_ent here to see whether it's successful or not.
* Thus, after gaining the answer perform the cleanup here.
*/
- rc = qed_spq_block(p_hwfn, p_ent, fw_return_code);
+ rc = qed_spq_block(p_hwfn, p_ent, fw_return_code,
+ p_ent->queue == &p_spq->unlimited_pending);
if (p_ent->queue == &p_spq->unlimited_pending) {
/* This is an allocated p_ent which does not need to
--
2.7.2
^ permalink raw reply related
* [PATCH v2 net-next 2/7] qede: GSO support for tunnels with outer csum
From: Manish Chopra @ 2016-10-13 11:22 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Manish Chopra
In-Reply-To: <1476357771-1578-1-git-send-email-manish.chopra@qlogic.com>
From: Manish Chopra <manish.chopra@caviumnetworks.com>
This patch adds GSO support for GRE and UDP tunnels
where outer checksums are enabled.
Signed-off-by: Manish Chopra <manish.chopra@caviumnetworks.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qede/qede.h | 1 +
drivers/net/ethernet/qlogic/qede/qede_main.c | 26 +++++++++++++++++++++++---
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede.h b/drivers/net/ethernet/qlogic/qede/qede.h
index 28c0e9f..f50e527 100644
--- a/drivers/net/ethernet/qlogic/qede/qede.h
+++ b/drivers/net/ethernet/qlogic/qede/qede.h
@@ -320,6 +320,7 @@ struct qede_fastpath {
#define XMIT_L4_CSUM BIT(0)
#define XMIT_LSO BIT(1)
#define XMIT_ENC BIT(2)
+#define XMIT_ENC_GSO_L4_CSUM BIT(3)
#define QEDE_CSUM_ERROR BIT(0)
#define QEDE_CSUM_UNNECESSARY BIT(1)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 9866d95..7d5dc1e 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -400,8 +400,19 @@ static u32 qede_xmit_type(struct qede_dev *edev,
(ipv6_hdr(skb)->nexthdr == NEXTHDR_IPV6))
*ipv6_ext = 1;
- if (skb->encapsulation)
+ if (skb->encapsulation) {
rc |= XMIT_ENC;
+ if (skb_is_gso(skb)) {
+ unsigned short gso_type = skb_shinfo(skb)->gso_type;
+
+ if ((gso_type & SKB_GSO_UDP_TUNNEL_CSUM) ||
+ (gso_type & SKB_GSO_GRE_CSUM))
+ rc |= XMIT_ENC_GSO_L4_CSUM;
+
+ rc |= XMIT_LSO;
+ return rc;
+ }
+ }
if (skb_is_gso(skb))
rc |= XMIT_LSO;
@@ -637,6 +648,12 @@ static netdev_tx_t qede_start_xmit(struct sk_buff *skb,
if (unlikely(xmit_type & XMIT_ENC)) {
first_bd->data.bd_flags.bitfields |=
1 << ETH_TX_1ST_BD_FLAGS_TUNN_IP_CSUM_SHIFT;
+
+ if (xmit_type & XMIT_ENC_GSO_L4_CSUM) {
+ u8 tmp = ETH_TX_1ST_BD_FLAGS_TUNN_L4_CSUM_SHIFT;
+
+ first_bd->data.bd_flags.bitfields |= 1 << tmp;
+ }
hlen = qede_get_skb_hlen(skb, true);
} else {
first_bd->data.bd_flags.bitfields |=
@@ -2320,11 +2337,14 @@ static void qede_init_ndev(struct qede_dev *edev)
/* Encap features*/
hw_features |= NETIF_F_GSO_GRE | NETIF_F_GSO_UDP_TUNNEL |
- NETIF_F_TSO_ECN;
+ NETIF_F_TSO_ECN | NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_GRE_CSUM;
ndev->hw_enc_features = NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
NETIF_F_SG | NETIF_F_TSO | NETIF_F_TSO_ECN |
NETIF_F_TSO6 | NETIF_F_GSO_GRE |
- NETIF_F_GSO_UDP_TUNNEL | NETIF_F_RXCSUM;
+ NETIF_F_GSO_UDP_TUNNEL | NETIF_F_RXCSUM |
+ NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_GRE_CSUM;
ndev->vlan_features = hw_features | NETIF_F_RXHASH | NETIF_F_RXCSUM |
NETIF_F_HIGHDMA;
--
2.7.2
^ permalink raw reply related
* [PATCH v2 net-next 1/7] qed: Pass MAC hints to VFs
From: Manish Chopra @ 2016-10-13 11:22 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Yuval Mintz
In-Reply-To: <1476357771-1578-1-git-send-email-manish.chopra@qlogic.com>
From: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
Some hypervisors can support MAC hints to their VFs.
Even though we don't have such a hypervisor API in linux, we add
sufficient logic for the VF to be able to receive such hints and
set the mac accordingly - as long as the VF has not been set with
a MAC already.
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qed/qed_vf.c | 4 ++--
drivers/net/ethernet/qlogic/qede/qede_main.c | 6 +++++-
include/linux/qed/qed_eth_if.h | 2 +-
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_vf.c b/drivers/net/ethernet/qlogic/qed/qed_vf.c
index abf5bf1..f580bf4 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_vf.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_vf.c
@@ -1230,8 +1230,8 @@ static void qed_handle_bulletin_change(struct qed_hwfn *hwfn)
is_mac_exist = qed_vf_bulletin_get_forced_mac(hwfn, mac,
&is_mac_forced);
- if (is_mac_exist && is_mac_forced && cookie)
- ops->force_mac(cookie, mac);
+ if (is_mac_exist && cookie)
+ ops->force_mac(cookie, mac, !!is_mac_forced);
/* Always update link configuration according to bulletin */
qed_link_update(hwfn);
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 343038c..9866d95 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -171,10 +171,14 @@ static struct pci_driver qede_pci_driver = {
#endif
};
-static void qede_force_mac(void *dev, u8 *mac)
+static void qede_force_mac(void *dev, u8 *mac, bool forced)
{
struct qede_dev *edev = dev;
+ /* MAC hints take effect only if we haven't set one already */
+ if (is_valid_ether_addr(edev->ndev->dev_addr) && !forced)
+ return;
+
ether_addr_copy(edev->ndev->dev_addr, mac);
ether_addr_copy(edev->primary_mac, mac);
}
diff --git a/include/linux/qed/qed_eth_if.h b/include/linux/qed/qed_eth_if.h
index 33c24eb..1c77948 100644
--- a/include/linux/qed/qed_eth_if.h
+++ b/include/linux/qed/qed_eth_if.h
@@ -129,7 +129,7 @@ struct qed_tunn_params {
struct qed_eth_cb_ops {
struct qed_common_cb_ops common;
- void (*force_mac) (void *dev, u8 *mac);
+ void (*force_mac) (void *dev, u8 *mac, bool forced);
};
#ifdef CONFIG_DCB
--
2.7.2
^ permalink raw reply related
* [PATCH v2 net-next 4/7] qed*: Allow unicast filtering
From: Manish Chopra @ 2016-10-13 11:22 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Yuval Mintz
In-Reply-To: <1476357771-1578-1-git-send-email-manish.chopra@qlogic.com>
From: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
Apparently qede fails to set IFF_UNICAST_FLT, and as a result is not
actually performing unicast MAC filtering.
While we're at it - relax a hard-coded limitation that limits each
interface into using at most 15 unicast MAC addresses before turning
promiscuous. Instead utilize the HW resources to their limit.
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qed/qed_l2.c | 12 ++++++++++--
drivers/net/ethernet/qlogic/qede/qede_main.c | 4 +++-
include/linux/qed/qed_eth_if.h | 1 +
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_l2.c b/drivers/net/ethernet/qlogic/qed/qed_l2.c
index ddd410a..6b0e22d 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_l2.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_l2.c
@@ -1652,6 +1652,7 @@ static int qed_fill_eth_dev_info(struct qed_dev *cdev,
if (IS_PF(cdev)) {
int max_vf_vlan_filters = 0;
+ int max_vf_mac_filters = 0;
if (cdev->int_params.out.int_mode == QED_INT_MODE_MSIX) {
for_each_hwfn(cdev, i)
@@ -1665,11 +1666,18 @@ static int qed_fill_eth_dev_info(struct qed_dev *cdev,
info->num_queues = cdev->num_hwfns;
}
- if (IS_QED_SRIOV(cdev))
+ if (IS_QED_SRIOV(cdev)) {
max_vf_vlan_filters = cdev->p_iov_info->total_vfs *
QED_ETH_VF_NUM_VLAN_FILTERS;
- info->num_vlan_filters = RESC_NUM(&cdev->hwfns[0], QED_VLAN) -
+ max_vf_mac_filters = cdev->p_iov_info->total_vfs *
+ QED_ETH_VF_NUM_MAC_FILTERS;
+ }
+ info->num_vlan_filters = RESC_NUM(QED_LEADING_HWFN(cdev),
+ QED_VLAN) -
max_vf_vlan_filters;
+ info->num_mac_filters = RESC_NUM(QED_LEADING_HWFN(cdev),
+ QED_MAC) -
+ max_vf_mac_filters;
ether_addr_copy(info->port_mac,
cdev->hwfns[0].hw_info.hw_mac_addr);
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 6c2b09c..0e483af 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -2365,6 +2365,8 @@ static void qede_init_ndev(struct qede_dev *edev)
qede_set_ethtool_ops(ndev);
+ ndev->priv_flags = IFF_UNICAST_FLT;
+
/* user-changeble features */
hw_features = NETIF_F_GRO | NETIF_F_SG |
NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
@@ -3937,7 +3939,7 @@ static void qede_config_rx_mode(struct net_device *ndev)
/* Check for promiscuous */
if ((ndev->flags & IFF_PROMISC) ||
- (uc_count > 15)) { /* @@@TBD resource allocation - 1 */
+ (uc_count > edev->dev_info.num_mac_filters - 1)) {
accept_flags = QED_FILTER_RX_MODE_TYPE_PROMISC;
} else {
/* Add MAC filters according to the unicast secondary macs */
diff --git a/include/linux/qed/qed_eth_if.h b/include/linux/qed/qed_eth_if.h
index 1c77948..1513080 100644
--- a/include/linux/qed/qed_eth_if.h
+++ b/include/linux/qed/qed_eth_if.h
@@ -23,6 +23,7 @@ struct qed_dev_eth_info {
u8 port_mac[ETH_ALEN];
u8 num_vlan_filters;
+ u16 num_mac_filters;
/* Legacy VF - this affects the datapath, so qede has to know */
bool is_legacy;
--
2.7.2
^ permalink raw reply related
* Re: [PATCH V2 for-next 0/8] Bug Fixes and Code Improvement in HNS driver
From: Doug Ledford @ 2016-10-13 11:57 UTC (permalink / raw)
To: Salil Mehta, David Miller
Cc: Zhuangyuzeng (Yisen), Huwei (Xavier), oulijun,
mehta.salil.lnk@gmail.com, linux-rdma@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Linuxarm
In-Reply-To: <F4CC6FACFEB3C54C9141D49AD221F7F91A74F326@lhreml503-mbx>
[-- Attachment #1.1: Type: text/plain, Size: 1595 bytes --]
On 10/13/2016 7:26 AM, Salil Mehta wrote:
>> -----Original Message-----
>> From: David Miller [mailto:davem@davemloft.net]
>> Sent: Friday, September 30, 2016 6:34 AM
>> To: Salil Mehta
>> Cc: dledford@redhat.com; Zhuangyuzeng (Yisen); Huwei (Xavier); oulijun;
>> mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
>> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
>> Subject: Re: [PATCH V2 for-next 0/8] Bug Fixes and Code Improvement in
>> HNS driver
>>
>> From: Salil Mehta <salil.mehta@huawei.com>
>> Date: Thu, 29 Sep 2016 18:09:08 +0100
>>
>>> This patch-set introduces fix to some Bugs, potential problems
>>> and code improvements identified during internal review and
>>> testing of Hisilicon Network Subsystem driver.
>>>
>>> Submit Change
>>> V1->V2: This addresses the feedbacks provided by David Miller
>>> and Doug Ledford
>>
>> So Doug my understanding is if this makes it through review
> Hi Dave,
> A gentle reminder regarding this. I was wondering if you think these
> have been reviewed enough and by any chance these patches can be included
> in current 4.9 merge window. This will save us a lot of effort.
>
> As I understand Doug is waiting for your review approval on this
> patch-set.
I am, and I was just getting ready to ask again today myself.
> Thanks in anticipation
> Best regards
> Salil
>> this is going to be merged into your tree, you prepare a
>> branch for me, and then I pull from that?
>>
>> Thanks in advance.
--
Doug Ledford <dledford@redhat.com>
GPG Key ID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: [PATCH v4 net-next 2/4] net/sched: act_mirred: Refactor detection whether dev needs xmit at mac header
From: Jamal Hadi Salim @ 2016-10-13 11:56 UTC (permalink / raw)
To: Shmulik Ladkani, David Miller
Cc: Eric Dumazet, WANG Cong, Daniel Borkmann, netdev
In-Reply-To: <1476338804-25440-3-git-send-email-shmulik.ladkani@gmail.com>
On 16-10-13 02:06 AM, Shmulik Ladkani wrote:
> Move detection logic that tests whether device expects skb data to point
> at mac_header upon xmit into a function.
>
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
^ permalink raw reply
* Re: [PATCH v4 net-next 1/4] net/sched: act_mirred: Rename tcfm_ok_push to tcfm_mac_header_xmit and make it a bool
From: Jamal Hadi Salim @ 2016-10-13 11:55 UTC (permalink / raw)
To: Shmulik Ladkani, David Miller
Cc: Eric Dumazet, WANG Cong, Daniel Borkmann, netdev
In-Reply-To: <1476338804-25440-2-git-send-email-shmulik.ladkani@gmail.com>
On 16-10-13 02:06 AM, Shmulik Ladkani wrote:
> 'tcfm_ok_push' specifies whether a mac_len sized push is needed upon
> egress to the target device (if action is performed at ingress).
>
> Rename it to 'tcfm_mac_header_xmit' as this is actually an attribute of
> the target device (and use a bool instead of int).
>
> This allows to decouple the attribute from the action to be taken.
>
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
cheers,
jamal
^ permalink raw reply
* Re: [PATCH net] net_sched: do not broadcast RTM_GETTFILTER result
From: Jamal Hadi Salim @ 2016-10-13 11:54 UTC (permalink / raw)
To: Eric Dumazet, Cong Wang; +Cc: David Miller, netdev
In-Reply-To: <1476344769.5650.25.camel@edumazet-glaptop3.roam.corp.google.com>
On 16-10-13 03:46 AM, Eric Dumazet wrote:
> On Wed, 2016-10-12 at 09:36 -0700, Cong Wang wrote:
>> On Sun, Oct 9, 2016 at 8:25 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>> + if (unicast)
>>> + return netlink_unicast(net->rtnl, skb, portid, MSG_DONTWAIT);
>>
>> Nit: rtnl_unicast() is simpler.
>
> I copied code in rtnetlink_send(), I guess we could use rtnl_unicast()
> there as well.
I would toss a coin and if it lands on tail then make the change ;->
(probably in a separate patch).
cheers,
jamal
^ permalink raw reply
* Re: [PATCH iproute2 1/1] tc filters: add support to get individual filters by handle
From: Jamal Hadi Salim @ 2016-10-13 11:51 UTC (permalink / raw)
To: Cong Wang
Cc: Stephen Hemminger, Linux Kernel Network Developers, Eric Dumazet,
Roman Mashak
In-Reply-To: <CAM_iQpVo-+jvJaBkJWR5ojkHKWY1fszDjPT+BYG20Vmb-dMtLQ@mail.gmail.com>
On 16-10-12 12:46 PM, Cong Wang wrote:
> On Mon, Oct 10, 2016 at 9:45 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>> tc/tc_filter.c | 185 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 175 insertions(+), 10 deletions(-)
>
> Please update man/man8/tc.8 too, but can be a separated patch. ;)
>
Excellent point.
Punted to Roman ;->
cheers,
jamal
^ permalink raw reply
* [PATCH] net: axienet: Remove unused parameter from __axienet_device_reset
From: Tobias Klauser @ 2016-10-13 11:28 UTC (permalink / raw)
To: Anirudha Sarangi, John Linn; +Cc: Michal Simek, Sören Brinkmann, netdev
The dev parameter passed to __axienet_device_reset() is not used inside
the function, so remove it.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 35f9f9742a48..c688d68c39aa 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -431,8 +431,7 @@ static void axienet_setoptions(struct net_device *ndev, u32 options)
lp->options |= options;
}
-static void __axienet_device_reset(struct axienet_local *lp,
- struct device *dev, off_t offset)
+static void __axienet_device_reset(struct axienet_local *lp, off_t offset)
{
u32 timeout;
/* Reset Axi DMA. This would reset Axi Ethernet core as well. The reset
@@ -468,8 +467,8 @@ static void axienet_device_reset(struct net_device *ndev)
u32 axienet_status;
struct axienet_local *lp = netdev_priv(ndev);
- __axienet_device_reset(lp, &ndev->dev, XAXIDMA_TX_CR_OFFSET);
- __axienet_device_reset(lp, &ndev->dev, XAXIDMA_RX_CR_OFFSET);
+ __axienet_device_reset(lp, XAXIDMA_TX_CR_OFFSET);
+ __axienet_device_reset(lp, XAXIDMA_RX_CR_OFFSET);
lp->max_frm_size = XAE_MAX_VLAN_FRAME_SIZE;
lp->options |= XAE_OPTION_VLAN;
@@ -1338,8 +1337,8 @@ static void axienet_dma_err_handler(unsigned long data)
axienet_iow(lp, XAE_MDIO_MC_OFFSET, (mdio_mcreg &
~XAE_MDIO_MC_MDIOEN_MASK));
- __axienet_device_reset(lp, &ndev->dev, XAXIDMA_TX_CR_OFFSET);
- __axienet_device_reset(lp, &ndev->dev, XAXIDMA_RX_CR_OFFSET);
+ __axienet_device_reset(lp, XAXIDMA_TX_CR_OFFSET);
+ __axienet_device_reset(lp, XAXIDMA_RX_CR_OFFSET);
axienet_iow(lp, XAE_MDIO_MC_OFFSET, mdio_mcreg);
axienet_mdio_wait_until_ready(lp);
--
2.9.0
^ permalink raw reply related
* Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing
From: Luca Coelho @ 2016-10-13 11:30 UTC (permalink / raw)
To: Paul Bolle, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
chris-FAYdYWbMHyxBDgjK7y7TUQ
Cc: linuxwifi-ral2JQCrhuEAvxtiuMwx3w,
emmanuel.grumbach-ral2JQCrhuEAvxtiuMwx3w,
johannes-cdvu00un1VgdHxzADdlk8Q, kvalo-sgV2jX0FEOL9JmXXK+q4OQ,
oren.givon-ral2JQCrhuEAvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476358075.1999.5.camel-IWqWACnzNjzz+pZb47iToQ@public.gmane.org>
On Thu, 2016-10-13 at 13:27 +0200, Paul Bolle wrote:
> Luca,
>
> On Thu, 2016-10-13 at 13:21 +0300, Luca Coelho wrote:
> > Could you please give this a spin? I have tested it with some handmade
> > ACPI tables in QEMU and it seems to work fine now.
>
>
> Tested-by: Paul Bolle <pebolle-IWqWACnzNjzz+pZb47iToQ@public.gmane.org>
>
> Not that this test was worth a lot: it builds cleanly (on top of
> 4.8.1), the error at boot is gone, obviously, and wifi still works (as
> you're reading a message that was sent out over wifi). And I haven't
> even tested this on another machine than my XPS 13 (9350).
Thanks for testing!
I forgot to say... could you load the iwlwifi module with debug=0x01
(module parameter), so we can see the messages the driver is printing
when it doesn't find a proper structure?
--
Luca.
^ permalink raw reply
* Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing
From: Paul Bolle @ 2016-10-13 11:27 UTC (permalink / raw)
To: Luca Coelho, linux-wireless, chris
Cc: linuxwifi, emmanuel.grumbach, johannes, kvalo, oren.givon, netdev,
linux-kernel
In-Reply-To: <20161013102155.14547-1-luca@coelho.fi>
Luca,
On Thu, 2016-10-13 at 13:21 +0300, Luca Coelho wrote:
> Could you please give this a spin? I have tested it with some handmade
> ACPI tables in QEMU and it seems to work fine now.
Tested-by: Paul Bolle <pebolle@tiscali.nl>
Not that this test was worth a lot: it builds cleanly (on top of
4.8.1), the error at boot is gone, obviously, and wifi still works (as
you're reading a message that was sent out over wifi). And I haven't
even tested this on another machine than my XPS 13 (9350).
Thanks!
Paul Bolle
^ permalink raw reply
* RE: [PATCH V2 for-next 0/8] Bug Fixes and Code Improvement in HNS driver
From: Salil Mehta @ 2016-10-13 11:26 UTC (permalink / raw)
To: David Miller
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
Zhuangyuzeng (Yisen), Huwei (Xavier), oulijun,
mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linuxarm
In-Reply-To: <20160930.013341.118711534087556597.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> -----Original Message-----
> From: David Miller [mailto:davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org]
> Sent: Friday, September 30, 2016 6:34 AM
> To: Salil Mehta
> Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Zhuangyuzeng (Yisen); Huwei (Xavier); oulijun;
> mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Linuxarm
> Subject: Re: [PATCH V2 for-next 0/8] Bug Fixes and Code Improvement in
> HNS driver
>
> From: Salil Mehta <salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Date: Thu, 29 Sep 2016 18:09:08 +0100
>
> > This patch-set introduces fix to some Bugs, potential problems
> > and code improvements identified during internal review and
> > testing of Hisilicon Network Subsystem driver.
> >
> > Submit Change
> > V1->V2: This addresses the feedbacks provided by David Miller
> > and Doug Ledford
>
> So Doug my understanding is if this makes it through review
Hi Dave,
A gentle reminder regarding this. I was wondering if you think these
have been reviewed enough and by any chance these patches can be included
in current 4.9 merge window. This will save us a lot of effort.
As I understand Doug is waiting for your review approval on this
patch-set.
Thanks in anticipation
Best regards
Salil
> this is going to be merged into your tree, you prepare a
> branch for me, and then I pull from that?
>
> Thanks in advance.
--
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: igb driver can cause cache invalidation of non-owned memory?
From: Nikita Yushchenko @ 2016-10-13 11:00 UTC (permalink / raw)
To: Alexander Duyck
Cc: David Laight, Eric Dumazet, David Miller, Jeff Kirsher,
intel-wired-lan, Netdev, linux-kernel@vger.kernel.org,
Chris Healy
In-Reply-To: <CAKgT0UdMG4D8CZCVhBD7vF+E5aOk7zyPEp1xXaBukg3v0aYfFg@mail.gmail.com>
>>> It would make more sense to update the DMA API for
>>> __dma_page_cpu_to_dev on ARM so that you don't invalidate the cache if
>>> the direction is DMA_FROM_DEVICE.
>>
>> No, in generic case it's unsafe.
>>
>> If CPU issued a write to a location, and sometime later that location is
>> used as DMA buffer, there is danger that write is still in cache only,
>> and writeback is pending. Later this writeback can overwrite data
>> written to memory via DMA, causing corruption.
>
> Okay so if I understand it correctly then the invalidation in
> sync_for_device is to force any writes to be flushed out, and the
> invalidation in sync_for_cpu is to flush out any speculative reads.
> So without speculative reads then the sync_for_cpu portion is not
> needed. You might want to determine if the core you are using
> supports the speculative reads, if not you might be able to get away
> without having to do the sync_for_cpu at all.
pl310 L2 cache controller does support prefetching - and I think most
arm systems use pl310.
>>> Changing the driver code for this won't necessarily work on all
>>> architectures, and on top of it we have some changes planned which
>>> will end up making the pages writable in the future to support the
>>> ongoing XDP effort. That is one of the reasons why I would not be
>>> okay with changing the driver to make this work.
>>
>> Well I was not really serious about removing that sync_for_device() in
>> mainline :) Although >20% throughput win that this provides is
>> impressive...
>
> I agree the improvement is pretty impressive. The think is there are
> similar gains we can generate on x86 by stripping out bits and pieces
> that are needed for other architectures. I'm just wanting to make
> certain we aren't optimizing for one architecture at the detriment of
> others.
Well in ideal world needs of other architectures should not limit x86 -
and if they do then that's a bug and should be fixed - by designing
proper set of abstractions :)
Perhaps issue is that "do whatever needed for device to perform DMA
correctly" semantics of dma_map() / sync_for_device() - and symmetric
for dma_unmap() / sync_for_cpu() - is too abstract and that hurts
performance. In particular, "setup i/o" and "sync caches" is different
activity with conflicting performance properties: for better
performance, one wants to setup i/o for larger blocks, but sync caches
for smaller blocks. Probably separation of these activities into
different calls is the way for better performance.
>> But what about doing something safer, e.g. adding a bit of tracking and
>> only sync_for_device() what was previously sync_for_cpu()ed? Will you
>> accept that?
>
> The problem is that as we move things over for XDP we will be looking
> at having the CPU potentially write to any spot in the region that was
> mapped as we could append headers to the front or pad data onto the
> end of the frame. It is probably safest for us to invalidate the
> entire region just to make sure we don't have a collision with
> something that is writing to the page.
Can't comment without knowning particular access patterns that XDP will
cause. Still rule is simple - "invalidate more" does hurt performance,
thus need to invalidate minimal required area. To avoid this
invalidation thing hurting performance on x86 that does not need
invalidation at all, good idea is to use some compile-time magic - just
to compile out unneeded things completely.
Still, XDP is future question, currently igb does not use it. Why not
improve sync_for_cpu() / sync_for_device() pairing in the current code?
I can propare such a patch. If XDP will make it irrelevant in future,
perhaps it could be just undone (and if this will cause performance
degradation, then it will be something to work on)
> So for example in the near future I am planning to expand out the
> DMA_ATTR_SKIP_CPU_SYNC DMA attribute beyond just the ARM architecture
> to see if I can expand it for use with SWIOTLB. If we can use this on
> unmap we might be able to solve some of the existing problems that
> required us to make the page read-only since we could unmap the page
> without invalidating any existing writes on the page.
Ack. Actually it is the same decoupling between "setup i/o" and "sync
caches" I've mentioned above.
Nikita
^ permalink raw reply
* RE: Kernel 4.6.7-rt13: Intel Ethernet driver igb causes huge latencies in cyclictest
From: Koehrer Mathias (ETAS/ESW5) @ 2016-10-13 10:57 UTC (permalink / raw)
To: Koehrer Mathias (ETAS/ESW5), Julia Cartwright, Williams, Mitch A,
Kirsher, Jeffrey T
Cc: linux-rt-users@vger.kernel.org, Sebastian Andrzej Siewior,
netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org, Greg
In-Reply-To: <e9661abf93194ad3849e3864a8361993@FE-MBX1012.de.bosch.com>
Hi all!
>
> Hi Julia,
>
> thanks for the detailed analysis!
> >
> > [...]
> > Okay, we finally received our wakeup event. We were expecting to be
> > woken up at 10024735653388ns, but were actually woken up at
> 10024735682387ns.
> >
> > 10024735682387 - 10024735653388 = 28999ns
> >
> > Our timer fired ~29us late! But why...?
> >
> > Sorry I don't have answers, just more questions. I do wonder what
> > kworker/0:3 was up to at the time the timer interrupt should have fired.
> >
> > Julia
> I have now instrumented the igb driver to generate trace points that allows me to
> identify in more details what's going on.
> This delivered interesting results!
>
> Here are some places where I added traces:
> In file igb_ptp.c:
> void igb_ptp_rx_hang(struct igb_adapter *adapter) {
> struct e1000_hw *hw = &adapter->hw;
> unsigned long rx_event;
> u32 tsyncrxctl;
> trace_igb(700);
> tsyncrxctl = rd32(E1000_TSYNCRXCTL);
> trace_igb(701);
>
> /* Other hardware uses per-packet timestamps */
> if (hw->mac.type != e1000_82576)
> return;
> ...
>
> In file igb_main.c:
> static void igb_check_lvmmc(struct igb_adapter *adapter) {
> struct e1000_hw *hw = &adapter->hw;
> u32 lvmmc;
>
> trace_igb(600);
> lvmmc = rd32(E1000_LVMMC);
> trace_igb(601);
> if (lvmmc) {
> ...
>
> When I run now my test, I get the following trace:
> [...]
> kworker/-607 0....... 107315621us+: igb: val: 700
> kworker/-607 0d..h... 107315640us : irq_handler_entry: irq=47 name=eth2-rx-0
> kworker/-607 0d..h... 107315640us : irq_handler_exit: irq=47 ret=handled
> kworker/-607 0d..h1.. 107315640us : sched_waking: comm=irq/47-eth2-rx-
> pid=18009 prio=49 target_cpu=000
> kworker/-607 0dN.h2.. 107315641us : sched_wakeup: comm=irq/47-eth2-rx-
> pid=18009 prio=49 target_cpu=000
> kworker/-607 0dN.h1.. 107315643us : irq_handler_entry: irq=48 name=eth2-tx-0
> kworker/-607 0dN.h1.. 107315643us : irq_handler_exit: irq=48 ret=handled
> kworker/-607 0dN.h2.. 107315643us : sched_waking: comm=irq/48-eth2-tx-
> pid=18010 prio=49 target_cpu=000
> kworker/-607 0dN.h3.. 107315644us : sched_wakeup: comm=irq/48-eth2-tx-
> pid=18010 prio=49 target_cpu=000
> kworker/-607 0dN..1.. 107315644us : rcu_utilization: Start context switch
> kworker/-607 0dN..1.. 107315644us : rcu_utilization: End context switch
> kworker/-607 0dN..2.. 107315644us : sched_stat_runtime: comm=kworker/0:1
> pid=607 runtime=88996 [ns] vruntime=49754678074 [ns]
> kworker/-607 0d...2.. 107315645us : sched_switch: prev_comm=kworker/0:1
> prev_pid=607 prev_prio=120 prev_state=R+ ==> next_comm=irq/47-eth2-rx-
> next_pid=18009 next_prio=49
> irq/47-e-18009 0d....11 107315646us : softirq_raise: vec=3 [action=NET_RX]
> irq/47-e-18009 0.....12 107315646us : softirq_entry: vec=3 [action=NET_RX]
> irq/47-e-18009 0.....12 107315647us : napi_poll: napi poll on napi struct
> ffff88040ae58c50 for device eth2 work 0 budget 64
> irq/47-e-18009 0.....12 107315647us : softirq_exit: vec=3 [action=NET_RX]
> irq/47-e-18009 0d...1.. 107315648us : rcu_utilization: Start context switch
> irq/47-e-18009 0d...1.. 107315648us : rcu_utilization: End context switch
> irq/47-e-18009 0d...2.. 107315648us : sched_switch: prev_comm=irq/47-eth2-rx-
> prev_pid=18009 prev_prio=49 prev_state=S ==> next_comm=irq/48-eth2-tx-
> next_pid=18010 next_prio=49
> irq/48-e-18010 0d....11 107315649us : softirq_raise: vec=3 [action=NET_RX]
> irq/48-e-18010 0.....12 107315649us : softirq_entry: vec=3 [action=NET_RX]
> irq/48-e-18010 0.....12 107315650us : napi_poll: napi poll on napi struct
> ffff88040ae5f450 for device eth2 work 0 budget 64
> irq/48-e-18010 0.....12 107315650us : softirq_exit: vec=3 [action=NET_RX]
> irq/48-e-18010 0d...1.. 107315651us : rcu_utilization: Start context switch
> irq/48-e-18010 0d...1.. 107315651us : rcu_utilization: End context switch
> irq/48-e-18010 0d...2.. 107315651us : sched_switch: prev_comm=irq/48-eth2-tx-
> prev_pid=18010 prev_prio=49 prev_state=S ==> next_comm=kworker/0:1
> next_pid=607 next_prio=120
> kworker/-607 0....... 107315652us : igb: val: 701
> kworker/-607 0....... 107315652us : igb: val: 106
> kworker/-607 0....... 107315652us : igb: val: 107
> kworker/-607 0....... 107315652us+: igb: val: 600
> kworker/-607 0d..h... 107315689us : local_timer_entry: vector=239
> kworker/-607 0d..h1.. 107315689us : hrtimer_interrupt: cpu=0 offset=-34521
> curr=kworker/0:1[120] thread=cyclictest[19]
> kworker/-607 0d..h1.. 107315689us : hrtimer_cancel: hrtimer=ffff8803d42efe18
> kworker/-607 0d..h... 107315689us : hrtimer_expire_entry:
> hrtimer=ffff8803d42efe18 function=hrtimer_wakeup now=752735681960
> kworker/-607 0d..h1.. 107315689us : sched_waking: comm=cyclictest pid=18015
> prio=19 target_cpu=000
> kworker/-607 0dN.h2.. 107315690us : sched_wakeup: comm=cyclictest
> pid=18015 prio=19 target_cpu=000
> kworker/-607 0dN.h... 107315690us : hrtimer_expire_exit:
> hrtimer=ffff8803d42efe18
> kworker/-607 0dN.h1.. 107315690us : hrtimer_interrupt: cpu=0 offset=318040
> curr=kworker/0:1[120] thread=<none>[-1]
> kworker/-607 0dN.h... 107315690us : write_msr: 6e0, value 28096cdb9ce
> kworker/-607 0dN.h... 107315690us : local_timer_exit: vector=239
> kworker/-607 0dN..1.. 107315690us : rcu_utilization: Start context switch
> kworker/-607 0dN..1.. 107315691us : rcu_utilization: End context switch
> kworker/-607 0dN..2.. 107315691us : sched_stat_runtime: comm=kworker/0:1
> pid=607 runtime=38439 [ns] vruntime=49754716513 [ns]
> kworker/-607 0d...2.. 107315691us : sched_switch: prev_comm=kworker/0:1
> prev_pid=607 prev_prio=120 prev_state=R+ ==> next_comm=cyclictest
> next_pid=18015 next_prio=19
> kworker/-607 0d...2.. 107315691us : x86_fpu_regs_activated: x86/fpu:
> ffff8803f7f55940 fpregs_active: 1 fpstate_active: 1 counter: 99 xfeatures: 2
> xcomp_bv: 0
> kworker/-607 0d...2.. 107315691us : write_msr: c0000100, value 7ffff7400700
> cyclicte-18015 0....... 107315692us : sys_exit: NR 230 = 0
> cyclicte-18015 0....... 107315697us : sys_enter: NR 1 (5, 7ffff7400300, 1f,
> 7ffff77a5460, 2, 7ffff744c99a)
> cyclicte-18015 0.....11 107315698us : tracing_mark_write: hit latency threshold (37
> > 33)
> cyclicte-18015 0....... 107315699us : sys_exit: NR 1 = 31
> cyclicte-18015 0....... 107315699us : sys_enter: NR 1 (4, 4076b0, 2, 7ffff77a5460,
> 2, 7ffff744c99a)
>
>
> Very interesting is also the trace that I get in an idle system - without cyclictest
> running.
> When I just enable my igb tracepoint I got the following result:
> [...]
> kworker/-607 0....... 585779012us+: igb: val: 700
> kworker/-607 0....... 585779042us : igb: val: 701
> kworker/-607 0....... 585779042us : igb: val: 106
> kworker/-607 0....... 585779043us : igb: val: 107
> kworker/-607 0....... 585779043us+: igb: val: 600
> kworker/-607 0....... 585779080us : igb: val: 601
>
> The time between my trace points 700 and 701 is about 30us, the time between my
> trace points 600 and 601 is even 37us!!
> The code in between is
> tsyncrxctl = rd32(E1000_TSYNCRXCTL); resp.
> lvmmc = rd32(E1000_LVMMC);
>
> In both cases this is a single read from a register.
> I have no idea why this single read could take that much time!
> Is it possible that the igb hardware is in a state that delays the read access and this is
> why the whole I/O system might be delayed?
>
To have a proper comparison, I did the same with kernel 3.18.27-rt27.
Also here, I instrumented the igb driver to get traces for the rd32 calls.
However, here everything is generally much faster!
In the idle system the maximum I got for a read was about 6us, most times it was 1-2us.
On the 4.8 kernel this is always much slower (see above).
My question is now: Is there any kernel config option that has been introduced in the meantime
that may lead to this effect and which is not set in my 4.8 config?
Regards
Mathias
^ permalink raw reply
* Re: [PATCH] qede: fix CONFIG_INFINIBAND_QEDR=m build error
From: Arnd Bergmann @ 2016-10-13 10:50 UTC (permalink / raw)
To: Mintz, Yuval
Cc: Ariel Elior, everest-linux-l2@qlogic.com, Alexander Duyck,
Amrani, Ram, netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
David S. Miller
In-Reply-To: <BL2PR07MB230634C0879011355A10B29A8DDC0@BL2PR07MB2306.namprd07.prod.outlook.com>
On Thursday, October 13, 2016 10:44:36 AM CEST Mintz, Yuval wrote:
> > > > > While I don't mind, you could have argued is that we're not
> > > > > removing enough, not too much.
> > > > > I.e., perhaps the rdma_msix_* fields should also have been
> > > > > ifdef-ed instead. [in which case this solution would not have
> > > > > worked]
> > > >
> > > > That would add even more #ifdefs though.
> > >
> > > I agree. Although I'm never clear on the guidelines for the tradeoff -
> > > How much memory/code is considered too much so that you'd have To
> > > ifdef code out instead of 'wasting'?
> > > [I obviously don't claim 64 bytes of memory hit that threshold]
> >
> > I don't think code size should ever be a reason for an #ifdef in a .c
> > file: if the code is well-structured, you can always get the same object code
> > using if(IS_ENABLED()) checks within the code at better readability or better
> > compile-time coverage.
> >
> > Between if(IS_ENABLED()) checks and inline helpers, it usually doesn't matter
> > much either way as long as the separation between the modules is clear enough.
> > In the example above, removing the structure fields however would require to
> > move the debugging output into another inline function though.
>
> Still, the question remains - If we were to allocate X bytes of memory
> per-something [in this case, per qed owned PCI function], and that memory
> wouldn't be functional without a some CONFIG option enabled,
> how big should X become before we'd decide the fields should also be
> dependent on the option?
> It bears no real relevance to this case, as the fields involved are
> insignificantly small. But still - is there a rule of thumb here?
I don't think there is a good general rule for that, given the
vastly different memory sizes in machines. For a tiny embedded
machine with 2MB of RAM, saving one kilobyte is very significant,
while an any machine that uses a 500KB qed driver module probably
has many gigabytes of RAM and doesn't care much about a
a wasted megabyte.
Arnd
^ permalink raw reply
* RE: [PATCH] qede: fix CONFIG_INFINIBAND_QEDR=m build error
From: Mintz, Yuval @ 2016-10-13 10:44 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ariel Elior, everest-linux-l2@qlogic.com, Alexander Duyck,
Amrani, Ram, netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
David S. Miller
In-Reply-To: <5662883.yo8OytxU65@wuerfel>
> > > > While I don't mind, you could have argued is that we're not
> > > > removing enough, not too much.
> > > > I.e., perhaps the rdma_msix_* fields should also have been
> > > > ifdef-ed instead. [in which case this solution would not have
> > > > worked]
> > >
> > > That would add even more #ifdefs though.
> >
> > I agree. Although I'm never clear on the guidelines for the tradeoff -
> > How much memory/code is considered too much so that you'd have To
> > ifdef code out instead of 'wasting'?
> > [I obviously don't claim 64 bytes of memory hit that threshold]
>
> I don't think code size should ever be a reason for an #ifdef in a .c
> file: if the code is well-structured, you can always get the same object code
> using if(IS_ENABLED()) checks within the code at better readability or better
> compile-time coverage.
>
> Between if(IS_ENABLED()) checks and inline helpers, it usually doesn't matter
> much either way as long as the separation between the modules is clear enough.
> In the example above, removing the structure fields however would require to
> move the debugging output into another inline function though.
Still, the question remains - If we were to allocate X bytes of memory
per-something [in this case, per qed owned PCI function], and that memory
wouldn't be functional without a some CONFIG option enabled,
how big should X become before we'd decide the fields should also be
dependent on the option?
It bears no real relevance to this case, as the fields involved are insignificantly
small. But still - is there a rule of thumb here?
> > BTW, are you interested in doing a v2 for this? Or would you prefer if
> > we'd pick it up from here?
>
> I think it's better if you do a v2, as you better understand the long-term plans. I'd
> be happy to test your patch in my randconfig build setup if you like.
Sure.
^ permalink raw reply
* RE: igb driver can cause cache invalidation of non-owned memory?
From: David Laight @ 2016-10-13 10:39 UTC (permalink / raw)
To: 'Alexander Duyck', Nikita Yushchenko
Cc: Eric Dumazet, David Miller, Jeff Kirsher, intel-wired-lan, Netdev,
linux-kernel@vger.kernel.org, Chris Healy
In-Reply-To: <CAKgT0UdMG4D8CZCVhBD7vF+E5aOk7zyPEp1xXaBukg3v0aYfFg@mail.gmail.com>
From: Alexander Duyck
> Sent: 12 October 2016 19:30
> On Wed, Oct 12, 2016 at 11:12 AM, Nikita Yushchenko
> <nikita.yoush@cogentembedded.com> wrote:
> >> It would make more sense to update the DMA API for
> >> __dma_page_cpu_to_dev on ARM so that you don't invalidate the cache if
> >> the direction is DMA_FROM_DEVICE.
> >
> > No, in generic case it's unsafe.
> >
> > If CPU issued a write to a location, and sometime later that location is
> > used as DMA buffer, there is danger that write is still in cache only,
> > and writeback is pending. Later this writeback can overwrite data
> > written to memory via DMA, causing corruption.
>
> Okay so if I understand it correctly then the invalidation in
> sync_for_device is to force any writes to be flushed out, and the
> invalidation in sync_for_cpu is to flush out any speculative reads.
> So without speculative reads then the sync_for_cpu portion is not
> needed. You might want to determine if the core you are using
> supports the speculative reads, if not you might be able to get away
> without having to do the sync_for_cpu at all.
For reads the sync_for_device invalidates (ie discards the contents of)
the cache lines to ensure the cpu won't write back dirty lines.
The sync_for_cpu invalidates the cache to ensure the cpu actually
reads from memory.
For writes you need to flush (write back) cache lines prior to the DMA.
I don't think anything is needed when the transfer finishes.
If headers/trailers might be added the sync_for_device must cover
the entire area the frame will be written to.
Clearly the sync_for_cpu need only cover the valid frame data.
I suspect you can safely skip the sync_for_device in the 'copybreak'
path but not if the frame is passed up the protocol stack.
David
^ permalink raw reply
* [PATCH] iwlwifi: pcie: fix SPLC structure parsing
From: Luca Coelho @ 2016-10-13 10:21 UTC (permalink / raw)
To: linux-wireless, chris, pebolle
Cc: linuxwifi, emmanuel.grumbach, johannes, kvalo, oren.givon, netdev,
linux-kernel
In-Reply-To: <1476349269.3880.18.camel@coelho.fi>
From: Luca Coelho <luciano.coelho@intel.com>
The SPLC data parsing is too restrictive and was not trying find the
correct element for WiFi. This causes problems with some BIOSes where
the SPLC method exists, but doesn't have a WiFi entry on the first
element of the list. The domain type values are also incorrect
according to the specification.
Fix this by complying with the actual specification.
Additionally, replace all occurrences of SPLX to SPLC, since SPLX is
only a structure internal to the ACPI tables, and may not even exist.
Fixes: bcb079a14d75 ("iwlwifi: pcie: retrieve and parse ACPI power limitations")
Reported-by: Chris Rorvick <chris@rorvick.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
Paul, Chris,
Could you please give this a spin? I have tested it with some handmade
ACPI tables in QEMU and it seems to work fine now.
Thanks!
drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 79 ++++++++++++++++-----------
1 file changed, 48 insertions(+), 31 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
index 001be40..da4810f 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
@@ -541,48 +541,64 @@ static const struct pci_device_id iwl_hw_card_ids[] = {
MODULE_DEVICE_TABLE(pci, iwl_hw_card_ids);
#ifdef CONFIG_ACPI
-#define SPL_METHOD "SPLC"
-#define SPL_DOMAINTYPE_MODULE BIT(0)
-#define SPL_DOMAINTYPE_WIFI BIT(1)
-#define SPL_DOMAINTYPE_WIGIG BIT(2)
-#define SPL_DOMAINTYPE_RFEM BIT(3)
+#define ACPI_SPLC_METHOD "SPLC"
+#define ACPI_SPLC_DOMAIN_WIFI (0x07)
-static u64 splx_get_pwr_limit(struct iwl_trans *trans, union acpi_object *splx)
+static u64 splc_get_pwr_limit(struct iwl_trans *trans, union acpi_object *splc)
{
- union acpi_object *limits, *domain_type, *power_limit;
-
- if (splx->type != ACPI_TYPE_PACKAGE ||
- splx->package.count != 2 ||
- splx->package.elements[0].type != ACPI_TYPE_INTEGER ||
- splx->package.elements[0].integer.value != 0) {
- IWL_ERR(trans, "Unsupported splx structure\n");
+ union acpi_object *data_pkg, *dflt_pwr_limit;
+ int i;
+
+ /* We need at least two elemets, one for the revision and one
+ * for the data itself. Also check that the revision is
+ * supported (currently only revision 0).
+ */
+ if (splc->type != ACPI_TYPE_PACKAGE ||
+ splc->package.count < 2 ||
+ splc->package.elements[0].type != ACPI_TYPE_INTEGER ||
+ splc->package.elements[0].integer.value != 0) {
+ IWL_DEBUG_INFO(trans,
+ "Unsupported structure returned by the SPLC method. Ignoring.\n");
return 0;
}
- limits = &splx->package.elements[1];
- if (limits->type != ACPI_TYPE_PACKAGE ||
- limits->package.count < 2 ||
- limits->package.elements[0].type != ACPI_TYPE_INTEGER ||
- limits->package.elements[1].type != ACPI_TYPE_INTEGER) {
- IWL_ERR(trans, "Invalid limits element\n");
- return 0;
+ /* loop through all the packages to find the one for WiFi */
+ for (i = 1; i < splc->package.count; i++) {
+ union acpi_object *domain;
+
+ data_pkg = &splc->package.elements[i];
+
+ /* Skip anything that is not a package with the right
+ * amount of elements (i.e. at least 2 integers).
+ */
+ if (data_pkg->type != ACPI_TYPE_PACKAGE ||
+ data_pkg->package.count < 2 ||
+ data_pkg->package.elements[0].type != ACPI_TYPE_INTEGER ||
+ data_pkg->package.elements[1].type != ACPI_TYPE_INTEGER)
+ continue;
+
+ domain = &data_pkg->package.elements[0];
+ if (domain->integer.value == ACPI_SPLC_DOMAIN_WIFI)
+ break;
+
+ data_pkg = NULL;
}
- domain_type = &limits->package.elements[0];
- power_limit = &limits->package.elements[1];
- if (!(domain_type->integer.value & SPL_DOMAINTYPE_WIFI)) {
- IWL_DEBUG_INFO(trans, "WiFi power is not limited\n");
+ if (!data_pkg) {
+ IWL_DEBUG_INFO(trans,
+ "No element for the WiFi domain returned by the SPLC method.\n");
return 0;
}
- return power_limit->integer.value;
+ dflt_pwr_limit = &data_pkg->package.elements[1];
+ return dflt_pwr_limit->integer.value;
}
static void set_dflt_pwr_limit(struct iwl_trans *trans, struct pci_dev *pdev)
{
acpi_handle pxsx_handle;
acpi_handle handle;
- struct acpi_buffer splx = {ACPI_ALLOCATE_BUFFER, NULL};
+ struct acpi_buffer splc = {ACPI_ALLOCATE_BUFFER, NULL};
acpi_status status;
pxsx_handle = ACPI_HANDLE(&pdev->dev);
@@ -593,23 +609,24 @@ static void set_dflt_pwr_limit(struct iwl_trans *trans, struct pci_dev *pdev)
}
/* Get the method's handle */
- status = acpi_get_handle(pxsx_handle, (acpi_string)SPL_METHOD, &handle);
+ status = acpi_get_handle(pxsx_handle, (acpi_string)ACPI_SPLC_METHOD,
+ &handle);
if (ACPI_FAILURE(status)) {
- IWL_DEBUG_INFO(trans, "SPL method not found\n");
+ IWL_DEBUG_INFO(trans, "SPLC method not found\n");
return;
}
/* Call SPLC with no arguments */
- status = acpi_evaluate_object(handle, NULL, NULL, &splx);
+ status = acpi_evaluate_object(handle, NULL, NULL, &splc);
if (ACPI_FAILURE(status)) {
IWL_ERR(trans, "SPLC invocation failed (0x%x)\n", status);
return;
}
- trans->dflt_pwr_limit = splx_get_pwr_limit(trans, splx.pointer);
+ trans->dflt_pwr_limit = splc_get_pwr_limit(trans, splc.pointer);
IWL_DEBUG_INFO(trans, "Default power limit set to %lld\n",
trans->dflt_pwr_limit);
- kfree(splx.pointer);
+ kfree(splc.pointer);
}
#else /* CONFIG_ACPI */
--
2.9.3
^ permalink raw reply related
* Re: [PATCH] qede: fix CONFIG_INFINIBAND_QEDR=m build error
From: Arnd Bergmann @ 2016-10-13 10:20 UTC (permalink / raw)
To: Mintz, Yuval
Cc: Ariel Elior, everest-linux-l2@qlogic.com, Alexander Duyck,
Amrani, Ram, netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
David S. Miller
In-Reply-To: <BY2PR07MB2309D9E2BEBC5A2091AC79E98DDC0@BY2PR07MB2309.namprd07.prod.outlook.com>
On Thursday, October 13, 2016 9:34:41 AM CEST Mintz, Yuval wrote:
>
> > > > - if (cond)
> > > > + if (IS_ENABLED(CONFIG_INFINIBAND_QEDR) && cond)
> > > > qed_rdma_dpm_bar(p_hwfn, p_ptt);
> > >
> > > Why not simply fix the qed_roce.h empty implementation?
> >
> > Mainly for consistency: we have a couple of interfaces that are called from the
> > qed driver that are implemented in qed_roce.c. We can either use a 'static inline'
> > helper for all of them, or use if(IS_ENABLED()) everywhere. Since this was the
> > only function that had a helper and that helper was defined incorrectly, I went
> > with the second option.
>
> Actually, that's not the case. I think with this exception, all the rest of the prototypes
> in qed_roce.h aren't really needed, as those functions should only be accessed via
> the qed_rdma_ops. I'll remove those later [or we can remove them as part of v2].
Ok, making those functions static and removing the prototypes would be
a good improvement. Note that building with 'make C=1' or 'make W=1'
will also warn if you have a global function that has no prototype,
so just removing the prototypes without making the functions static
would be bad (there is currently work going on to enable the warning
by default).
The functions I was thinking of are qed_async_roce_event,
qed_ll2b_complete_tx_gsi_packet, qed_ll2b_complete_rx_gsi_packet
and qed_ll2b_release_tx_gsi_packet, all of which do get called
from the qed driver, so you will still need to add inline wrappers
for those.
> The genereal qed* preference is to have empty static-inline implementations
> in case content is compiled-out [Look at iov for example].
Ok, fair enough.
> > > > -#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
> > > > + if (!IS_ENABLED(CONFIG_INFINIBAND_QEDR))
> > > > + return 0;
> > > > +
> > > > num_l2_queues = 0;
> > > > for_each_hwfn(cdev, i)
> > > > num_l2_queues += FEAT_NUM(&cdev->hwfns[i],
> > QED_PF_L2_QUE); @@
> > > > -738,7 +736,6 @@ static int qed_slowpath_setup_int(struct qed_dev
> > > > *cdev,
> > > > DP_VERBOSE(cdev, QED_MSG_RDMA, "roce_msix_cnt=%d
> > > > roce_msix_base=%d\n",
> > > > cdev->int_params.rdma_msix_cnt,
> > > > cdev->int_params.rdma_msix_base); -#endif
> > >
> > > While I don't mind, you could have argued is that we're not removing
> > > enough, not too much.
> > > I.e., perhaps the rdma_msix_* fields should also have been ifdef-ed
> > > instead. [in which case this solution would not have worked]
> >
> > That would add even more #ifdefs though.
>
> I agree. Although I'm never clear on the guidelines for the tradeoff -
> How much memory/code is considered too much so that you'd have
> To ifdef code out instead of 'wasting'?
> [I obviously don't claim 64 bytes of memory hit that threshold]
I don't think code size should ever be a reason for an #ifdef in a .c
file: if the code is well-structured, you can always get the same
object code using if(IS_ENABLED()) checks within the code at better
readability or better compile-time coverage.
Between if(IS_ENABLED()) checks and inline helpers, it usually
doesn't matter much either way as long as the separation between
the modules is clear enough. In the example above, removing
the structure fields however would require to move the debugging
output into another inline function though.
> BTW, are you interested in doing a v2 for this? Or would you prefer
> if we'd pick it up from here?
I think it's better if you do a v2, as you better understand the
long-term plans. I'd be happy to test your patch in my randconfig
build setup if you like.
Arnd
^ permalink raw reply
* Re: [patch net-next RFC 0/6] Add support for offloading packet-sampling
From: Roopa Prabhu @ 2016-10-13 7:29 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, yotamg, idosch, eladr, nogahf, ogerlitz, jhs,
geert+renesas, stephen, xiyou.wangcong, linux
In-Reply-To: <1476276069-5315-1-git-send-email-jiri@resnulli.us>
On 10/12/16, 5:41 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Add the sample tc action, which allows to sample packet matching
> a classifier. The sample action peeks randomly packets, duplicates them,
> truncates them and adds informative metadata on the packet, for example,
> the input interface and the original packet length. The sampled packets
> are marked to allow matching them and redirecting them to a specific
> collector device.
>
> The sampled packets metadata is packed using ife encapsulation. To do
> that, this patch-set extracts ife logics from the tc_ife action into an
> independent ife module, and uses that functionality to pack the metadata.
> To include all the needed metadata, this patch-set introduces some new
> IFE_META tlv types.
>
> In addition, Add the support for offloading the matchall-sample tc command
> in the Mellanox mlxsw driver, for ingress qdiscs.
>
> Yotam Gigi (6):
> Introduce ife encapsulation module
> act_ife: Change to use ife module
> ife: Introduce new metadata tlv types
> Introduce sample tc action
> mlxsw: reg: add the Monitoring Packet Sampling Configuration Register
> mlxsw: packet sample: Add packet sample offloading support
>
we spoke with yotam about this at netdev1.2. and also remember speaking about this on our switchdev calls:
Today our driver uses NFLOG to log packets to a netlink socket and hsflowd supported by the sflow
people (at http://www.sflow.net/) is capable of reading from a nflog socket. NFLOG has the required netlink
attribute markers for packet header/data (which we can possibly extend). We could also add nflog like action
in tc if needed.
sflow agents like hsflowd are capable of sending packets to an external collector with the required sflow header.
Instead of re-inventing a new API for sflow, would be better to standardize/unify on existing mechanisms.
Also, this patch series requires a new device to be created which can be avoided if we used
existing mechanisms like NFLOG.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox