* [PATCH net 3/4] qed: Fix SPQ entries not returned to pool in error flows
From: Denis Bolotin @ 2018-11-08 14:46 UTC (permalink / raw)
To: davem, netdev; +Cc: ariel.elior, michal.kalderon, Denis Bolotin
In-Reply-To: <20181108144611.28598-1-denis.bolotin@cavium.com>
qed_sp_destroy_request() API was added for SPQ users that need to
free/return the entry they acquired in their error flows.
Signed-off-by: Denis Bolotin <denis.bolotin@cavium.com>
Signed-off-by: Michal Kalderon <michal.kalderon@cavium.com>
---
drivers/net/ethernet/qlogic/qed/qed_fcoe.c | 11 ++++++++---
drivers/net/ethernet/qlogic/qed/qed_iscsi.c | 1 +
drivers/net/ethernet/qlogic/qed/qed_l2.c | 12 ++++++++----
drivers/net/ethernet/qlogic/qed/qed_rdma.c | 1 +
drivers/net/ethernet/qlogic/qed/qed_roce.c | 1 +
drivers/net/ethernet/qlogic/qed/qed_sp.h | 11 +++++++++++
drivers/net/ethernet/qlogic/qed/qed_sp_commands.c | 22 ++++++++++++++--------
drivers/net/ethernet/qlogic/qed/qed_sriov.c | 1 +
8 files changed, 45 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_fcoe.c b/drivers/net/ethernet/qlogic/qed/qed_fcoe.c
index cc1b373..46dc93d 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_fcoe.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_fcoe.c
@@ -147,7 +147,8 @@ struct qed_fcoe_conn {
"Cannot satisfy CQ amount. CQs requested %d, CQs available %d. Aborting function start\n",
fcoe_pf_params->num_cqs,
p_hwfn->hw_info.feat_num[QED_FCOE_CQ]);
- return -EINVAL;
+ rc = -EINVAL;
+ goto err;
}
p_data->mtu = cpu_to_le16(fcoe_pf_params->mtu);
@@ -156,14 +157,14 @@ struct qed_fcoe_conn {
rc = qed_cxt_acquire_cid(p_hwfn, PROTOCOLID_FCOE, &dummy_cid);
if (rc)
- return rc;
+ goto err;
cxt_info.iid = dummy_cid;
rc = qed_cxt_get_cid_info(p_hwfn, &cxt_info);
if (rc) {
DP_NOTICE(p_hwfn, "Cannot find context info for dummy cid=%d\n",
dummy_cid);
- return rc;
+ goto err;
}
p_cxt = cxt_info.p_cxt;
SET_FIELD(p_cxt->tstorm_ag_context.flags3,
@@ -240,6 +241,10 @@ struct qed_fcoe_conn {
rc = qed_spq_post(p_hwfn, p_ent, NULL);
return rc;
+
+err:
+ qed_sp_destroy_request(p_hwfn, p_ent);
+ return rc;
}
static int
diff --git a/drivers/net/ethernet/qlogic/qed/qed_iscsi.c b/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
index 1135387..4f8a685 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
@@ -200,6 +200,7 @@ struct qed_iscsi_conn {
"Cannot satisfy CQ amount. Queues requested %d, CQs available %d. Aborting function start\n",
p_params->num_queues,
p_hwfn->hw_info.feat_num[QED_ISCSI_CQ]);
+ qed_sp_destroy_request(p_hwfn, p_ent);
return -EINVAL;
}
diff --git a/drivers/net/ethernet/qlogic/qed/qed_l2.c b/drivers/net/ethernet/qlogic/qed/qed_l2.c
index 82a1bd1..67c02ea93 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_l2.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_l2.c
@@ -740,8 +740,7 @@ int qed_sp_vport_update(struct qed_hwfn *p_hwfn,
rc = qed_sp_vport_update_rss(p_hwfn, p_ramrod, p_rss_params);
if (rc) {
- /* Return spq entry which is taken in qed_sp_init_request()*/
- qed_spq_return_entry(p_hwfn, p_ent);
+ qed_sp_destroy_request(p_hwfn, p_ent);
return rc;
}
@@ -1355,6 +1354,7 @@ static enum eth_filter_action qed_filter_action(enum qed_filter_opcode opcode)
DP_NOTICE(p_hwfn,
"%d is not supported yet\n",
p_filter_cmd->opcode);
+ qed_sp_destroy_request(p_hwfn, *pp_ent);
return -EINVAL;
}
@@ -2056,13 +2056,13 @@ void qed_arfs_mode_configure(struct qed_hwfn *p_hwfn,
} else {
rc = qed_fw_vport(p_hwfn, p_params->vport_id, &abs_vport_id);
if (rc)
- return rc;
+ goto err;
if (p_params->qid != QED_RFS_NTUPLE_QID_RSS) {
rc = qed_fw_l2_queue(p_hwfn, p_params->qid,
&abs_rx_q_id);
if (rc)
- return rc;
+ goto err;
p_ramrod->rx_qid_valid = 1;
p_ramrod->rx_qid = cpu_to_le16(abs_rx_q_id);
@@ -2083,6 +2083,10 @@ void qed_arfs_mode_configure(struct qed_hwfn *p_hwfn,
(u64)p_params->addr, p_params->length);
return qed_spq_post(p_hwfn, p_ent, NULL);
+
+err:
+ qed_sp_destroy_request(p_hwfn, p_ent);
+ return rc;
}
int qed_get_rxq_coalesce(struct qed_hwfn *p_hwfn,
diff --git a/drivers/net/ethernet/qlogic/qed/qed_rdma.c b/drivers/net/ethernet/qlogic/qed/qed_rdma.c
index c71391b..6211343 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_rdma.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_rdma.c
@@ -1514,6 +1514,7 @@ static int qed_rdma_modify_qp(void *rdma_cxt,
default:
rc = -EINVAL;
DP_VERBOSE(p_hwfn, QED_MSG_RDMA, "rc = %d\n", rc);
+ qed_sp_destroy_request(p_hwfn, p_ent);
return rc;
}
SET_FIELD(p_ramrod->flags1,
diff --git a/drivers/net/ethernet/qlogic/qed/qed_roce.c b/drivers/net/ethernet/qlogic/qed/qed_roce.c
index f9167d1..e49fada 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_roce.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_roce.c
@@ -745,6 +745,7 @@ static int qed_roce_sp_destroy_qp_responder(struct qed_hwfn *p_hwfn,
DP_NOTICE(p_hwfn,
"qed destroy responder failed: cannot allocate memory (ramrod). rc = %d\n",
rc);
+ qed_sp_destroy_request(p_hwfn, p_ent);
return rc;
}
diff --git a/drivers/net/ethernet/qlogic/qed/qed_sp.h b/drivers/net/ethernet/qlogic/qed/qed_sp.h
index 04259df..3157c0d 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_sp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_sp.h
@@ -399,6 +399,17 @@ struct qed_sp_init_data {
struct qed_spq_comp_cb *p_comp_data;
};
+/**
+ * @brief Returns a SPQ entry to the pool / frees the entry if allocated.
+ * Should be called on in error flows after initializing the SPQ entry
+ * and before posting it.
+ *
+ * @param p_hwfn
+ * @param p_ent
+ */
+void qed_sp_destroy_request(struct qed_hwfn *p_hwfn,
+ struct qed_spq_entry *p_ent);
+
int qed_sp_init_request(struct qed_hwfn *p_hwfn,
struct qed_spq_entry **pp_ent,
u8 cmd,
diff --git a/drivers/net/ethernet/qlogic/qed/qed_sp_commands.c b/drivers/net/ethernet/qlogic/qed/qed_sp_commands.c
index e86a1ea..888274f 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_sp_commands.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_sp_commands.c
@@ -47,6 +47,19 @@
#include "qed_sp.h"
#include "qed_sriov.h"
+void qed_sp_destroy_request(struct qed_hwfn *p_hwfn,
+ struct qed_spq_entry *p_ent)
+{
+ /* qed_spq_get_entry() can either get an entry from the free_pool,
+ * or, if no entries are left, allocate a new entry and add it to
+ * the unlimited_pending list.
+ */
+ if (p_ent->queue == &p_hwfn->p_spq->unlimited_pending)
+ kfree(p_ent);
+ else
+ qed_spq_return_entry(p_hwfn, p_ent);
+}
+
int qed_sp_init_request(struct qed_hwfn *p_hwfn,
struct qed_spq_entry **pp_ent,
u8 cmd, u8 protocol, struct qed_sp_init_data *p_data)
@@ -111,14 +124,7 @@ int qed_sp_init_request(struct qed_hwfn *p_hwfn,
return 0;
err:
- /* qed_spq_get_entry() can either get an entry from the free_pool,
- * or, if no entries are left, allocate a new entry and add it to
- * the unlimited_pending list.
- */
- if (p_ent->queue == &p_hwfn->p_spq->unlimited_pending)
- kfree(p_ent);
- else
- qed_spq_return_entry(p_hwfn, p_ent);
+ qed_sp_destroy_request(p_hwfn, p_ent);
return -EINVAL;
}
diff --git a/drivers/net/ethernet/qlogic/qed/qed_sriov.c b/drivers/net/ethernet/qlogic/qed/qed_sriov.c
index 9b08a9d..ca6290f 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_sriov.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_sriov.c
@@ -101,6 +101,7 @@ static int qed_sp_vf_start(struct qed_hwfn *p_hwfn, struct qed_vf_info *p_vf)
default:
DP_NOTICE(p_hwfn, "Unknown VF personality %d\n",
p_hwfn->hw_info.personality);
+ qed_sp_destroy_request(p_hwfn, p_ent);
return -EINVAL;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH net 4/4] qed: Fix potential memory corruption
From: Denis Bolotin @ 2018-11-08 14:46 UTC (permalink / raw)
To: davem, netdev; +Cc: ariel.elior, michal.kalderon, Sagiv Ozeri, Denis Bolotin
In-Reply-To: <20181108144611.28598-1-denis.bolotin@cavium.com>
From: Sagiv Ozeri <sagiv.ozeri@cavium.com>
A stuck ramrod should be deleted from the completion_pending list,
otherwise it will be added again in the future and corrupt the list.
Return error value to inform that ramrod is stuck and should be deleted.
Signed-off-by: Sagiv Ozeri <sagiv.ozeri@cavium.com>
Signed-off-by: Denis Bolotin <denis.bolotin@cavium.com>
---
drivers/net/ethernet/qlogic/qed/qed_spq.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_spq.c b/drivers/net/ethernet/qlogic/qed/qed_spq.c
index c1a81ec..0a9c5bb 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_spq.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_spq.c
@@ -142,6 +142,7 @@ static int qed_spq_block(struct qed_hwfn *p_hwfn,
DP_INFO(p_hwfn, "Ramrod is stuck, requesting MCP drain\n");
rc = qed_mcp_drain(p_hwfn, p_ptt);
+ qed_ptt_release(p_hwfn, p_ptt);
if (rc) {
DP_NOTICE(p_hwfn, "MCP drain failed\n");
goto err;
@@ -150,18 +151,15 @@ static int qed_spq_block(struct qed_hwfn *p_hwfn,
/* Retry after drain */
rc = __qed_spq_block(p_hwfn, p_ent, p_fw_ret, true);
if (!rc)
- goto out;
+ return 0;
comp_done = (struct qed_spq_comp_done *)p_ent->comp_cb.cookie;
- if (comp_done->done == 1)
+ if (comp_done->done == 1) {
if (p_fw_ret)
*p_fw_ret = comp_done->fw_return_code;
-out:
- qed_ptt_release(p_hwfn, p_ptt);
- return 0;
-
+ return 0;
+ }
err:
- qed_ptt_release(p_hwfn, p_ptt);
DP_NOTICE(p_hwfn,
"Ramrod is stuck [CID %08x cmd %02x protocol %02x echo %04x]\n",
le32_to_cpu(p_ent->elem.hdr.cid),
--
1.8.3.1
^ permalink raw reply related
* Re: [Patch net] net: hns3: bugfix for not checking return value
From: David Miller @ 2018-11-09 0:24 UTC (permalink / raw)
To: tanhuazhong; +Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm
In-Reply-To: <1541643199-57792-1-git-send-email-tanhuazhong@huawei.com>
From: Huazhong Tan <tanhuazhong@huawei.com>
Date: Thu, 8 Nov 2018 10:13:19 +0800
> hns3_reset_notify_init_enet() only return error early if the return
> value of hns3_restore_vlan() is not 0.
>
> This patch adds checking for the return value of hns3_restore_vlan.
>
> Fixes: 7fa6be4fd2f6 ("net: hns3: fix incorrect return value/type of some functions")
> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
Applied, thank you.
^ permalink raw reply
* Re: drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c:96: undefined reference to `thermal_zone_device_register'
From: Randy Dunlap @ 2018-11-09 0:46 UTC (permalink / raw)
To: kbuild test robot, Arnd Bergmann
Cc: kbuild-all, linux-kernel, Ganesh Goudar, David Miller,
netdev@vger.kernel.org
In-Reply-To: <201811081531.m0SEgZ58%fengguang.wu@intel.com>
[adding netdev + maintainers]
On 11/7/18 11:08 PM, kbuild test robot wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 85758777c2a227fd1541b6dd122a08ab79c347ce
> commit: e70a57fa59bb7fefe063780a49e063d0d0f61863 cxgb4: fix thermal configuration dependencies
> date: 4 weeks ago
> config: x86_64-randconfig-s0-11081213 (attached as .config)
> compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
> reproduce:
> git checkout e70a57fa59bb7fefe063780a49e063d0d0f61863
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.o: In function `cxgb4_thermal_init':
>>> drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c:96: undefined reference to `thermal_zone_device_register'
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.o: In function `cxgb4_thermal_remove':
>>> drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c:112: undefined reference to `thermal_zone_device_unregister'
>
Hi,
There was a Kconfig WARNING (clue!) before the build errors:
WARNING: unmet direct dependencies detected for CHELSIO_T4
Depends on [m]: NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_CHELSIO [=y] && PCI [=y] && (IPV6 [=n] || IPV6 [=n]=n) && (THERMAL [=m] || !THERMAL [=m])
Selected by [y]:
- SCSI_CXGB4_ISCSI [=y] && SCSI_LOWLEVEL [=y] && SCSI [=y] && PCI [=y] && INET [=y] && (IPV6 [=n] || IPV6 [=n]=n)
That says that the problem is with CONFIG_THERMAL=m while CONFIG_CHELSIO_T4=y.
This is caused by drivers/scsi/cxgbi/cxgb4i/Kconfig and its use of "select"
instead of "depends on":
config SCSI_CXGB4_ISCSI
tristate "Chelsio T4 iSCSI support"
depends on PCI && INET && (IPV6 || IPV6=n)
select NETDEVICES
select ETHERNET
select NET_VENDOR_CHELSIO
select CHELSIO_T4
select CHELSIO_LIB
select SCSI_ISCSI_ATTRS
---help---
This driver supports iSCSI offload for the Chelsio T4 devices.
> vim +96 drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
>
> b1871915 Ganesh Goudar 2018-10-09 72
> b1871915 Ganesh Goudar 2018-10-09 73 int cxgb4_thermal_init(struct adapter *adap)
> b1871915 Ganesh Goudar 2018-10-09 74 {
> b1871915 Ganesh Goudar 2018-10-09 75 struct ch_thermal *ch_thermal = &adap->ch_thermal;
> b1871915 Ganesh Goudar 2018-10-09 76 int num_trip = CXGB4_NUM_TRIPS;
> b1871915 Ganesh Goudar 2018-10-09 77 u32 param, val;
> b1871915 Ganesh Goudar 2018-10-09 78 int ret;
> b1871915 Ganesh Goudar 2018-10-09 79
> b1871915 Ganesh Goudar 2018-10-09 80 /* on older firmwares we may not get the trip temperature,
> b1871915 Ganesh Goudar 2018-10-09 81 * set the num of trips to 0.
> b1871915 Ganesh Goudar 2018-10-09 82 */
> b1871915 Ganesh Goudar 2018-10-09 83 param = (FW_PARAMS_MNEM_V(FW_PARAMS_MNEM_DEV) |
> b1871915 Ganesh Goudar 2018-10-09 84 FW_PARAMS_PARAM_X_V(FW_PARAMS_PARAM_DEV_DIAG) |
> b1871915 Ganesh Goudar 2018-10-09 85 FW_PARAMS_PARAM_Y_V(FW_PARAM_DEV_DIAG_MAXTMPTHRESH));
> b1871915 Ganesh Goudar 2018-10-09 86
> b1871915 Ganesh Goudar 2018-10-09 87 ret = t4_query_params(adap, adap->mbox, adap->pf, 0, 1,
> b1871915 Ganesh Goudar 2018-10-09 88 ¶m, &val);
> b1871915 Ganesh Goudar 2018-10-09 89 if (ret < 0) {
> b1871915 Ganesh Goudar 2018-10-09 90 num_trip = 0; /* could not get trip temperature */
> b1871915 Ganesh Goudar 2018-10-09 91 } else {
> b1871915 Ganesh Goudar 2018-10-09 92 ch_thermal->trip_temp = val * 1000;
> b1871915 Ganesh Goudar 2018-10-09 93 ch_thermal->trip_type = THERMAL_TRIP_CRITICAL;
> b1871915 Ganesh Goudar 2018-10-09 94 }
> b1871915 Ganesh Goudar 2018-10-09 95
> b1871915 Ganesh Goudar 2018-10-09 @96 ch_thermal->tzdev = thermal_zone_device_register("cxgb4", num_trip,
> b1871915 Ganesh Goudar 2018-10-09 97 0, adap,
> b1871915 Ganesh Goudar 2018-10-09 98 &cxgb4_thermal_ops,
> b1871915 Ganesh Goudar 2018-10-09 99 NULL, 0, 0);
> b1871915 Ganesh Goudar 2018-10-09 100 if (IS_ERR(ch_thermal->tzdev)) {
> b1871915 Ganesh Goudar 2018-10-09 101 ret = PTR_ERR(ch_thermal->tzdev);
> b1871915 Ganesh Goudar 2018-10-09 102 dev_err(adap->pdev_dev, "Failed to register thermal zone\n");
> b1871915 Ganesh Goudar 2018-10-09 103 ch_thermal->tzdev = NULL;
> b1871915 Ganesh Goudar 2018-10-09 104 return ret;
> b1871915 Ganesh Goudar 2018-10-09 105 }
> b1871915 Ganesh Goudar 2018-10-09 106 return 0;
> b1871915 Ganesh Goudar 2018-10-09 107 }
> b1871915 Ganesh Goudar 2018-10-09 108
> b1871915 Ganesh Goudar 2018-10-09 109 int cxgb4_thermal_remove(struct adapter *adap)
> b1871915 Ganesh Goudar 2018-10-09 110 {
> b1871915 Ganesh Goudar 2018-10-09 111 if (adap->ch_thermal.tzdev)
> b1871915 Ganesh Goudar 2018-10-09 @112 thermal_zone_device_unregister(adap->ch_thermal.tzdev);
>
> :::::: The code at line 96 was first introduced by commit
> :::::: b187191577629b5358acf4e234809ee8d441ceb4 cxgb4: Add thermal zone support
>
> :::::: TO: Ganesh Goudar <ganeshgr@chelsio.com>
> :::::: CC: David S. Miller <davem@davemloft.net>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
ciao.
--
~Randy
^ permalink raw reply
* [PATCH bpf-next] filter: add BPF_ADJ_ROOM_DATA mode to bpf_skb_adjust_room()
From: Nicolas Dichtel @ 2018-11-08 15:11 UTC (permalink / raw)
To: ast, daniel, davem; +Cc: netdev, Nicolas Dichtel
This new mode enables to add or remove an l2 header in a programmatic way
with cls_bpf.
For example, it enables to play with mpls headers.
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
include/uapi/linux/bpf.h | 3 ++
net/core/filter.c | 54 ++++++++++++++++++++++++++++++++++
tools/include/uapi/linux/bpf.h | 3 ++
3 files changed, 60 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 852dc17ab47a..47407fd5162b 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1467,6 +1467,8 @@ union bpf_attr {
*
* * **BPF_ADJ_ROOM_NET**: Adjust room at the network layer
* (room space is added or removed below the layer 3 header).
+ * * **BPF_ADJ_ROOM_DATA**: Adjust room at the beginning of the
+ * packet (room space is added or removed below skb->data).
*
* All values for *flags* are reserved for future usage, and must
* be left at zero.
@@ -2408,6 +2410,7 @@ enum bpf_func_id {
/* Mode for BPF_FUNC_skb_adjust_room helper. */
enum bpf_adj_room_mode {
BPF_ADJ_ROOM_NET,
+ BPF_ADJ_ROOM_DATA,
};
/* Mode for BPF_FUNC_skb_load_bytes_relative helper. */
diff --git a/net/core/filter.c b/net/core/filter.c
index e521c5ebc7d1..e699849b269d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2884,6 +2884,58 @@ static int bpf_skb_adjust_net(struct sk_buff *skb, s32 len_diff)
return ret;
}
+static int bpf_skb_data_shrink(struct sk_buff *skb, u32 len)
+{
+ unsigned short hhlen = skb->dev->header_ops ?
+ skb->dev->hard_header_len : 0;
+ int ret;
+
+ ret = skb_unclone(skb, GFP_ATOMIC);
+ if (unlikely(ret < 0))
+ return ret;
+
+ __skb_pull(skb, len);
+ skb_reset_mac_header(skb);
+ skb_reset_network_header(skb);
+ skb->network_header += hhlen;
+ skb_reset_transport_header(skb);
+ return 0;
+}
+
+static int bpf_skb_data_grow(struct sk_buff *skb, u32 len)
+{
+ unsigned short hhlen = skb->dev->header_ops ?
+ skb->dev->hard_header_len : 0;
+ int ret;
+
+ ret = skb_cow(skb, len);
+ if (unlikely(ret < 0))
+ return ret;
+
+ skb_push(skb, len);
+ skb_reset_mac_header(skb);
+ return 0;
+}
+
+static int bpf_skb_adjust_data(struct sk_buff *skb, s32 len_diff)
+{
+ u32 len_diff_abs = abs(len_diff);
+ bool shrink = len_diff < 0;
+ int ret;
+
+ if (unlikely(len_diff_abs > 0xfffU))
+ return -EFAULT;
+
+ if (shrink && len_diff_abs >= skb_headlen(skb))
+ return -EFAULT;
+
+ ret = shrink ? bpf_skb_data_shrink(skb, len_diff_abs) :
+ bpf_skb_data_grow(skb, len_diff_abs);
+
+ bpf_compute_data_pointers(skb);
+ return ret;
+}
+
BPF_CALL_4(bpf_skb_adjust_room, struct sk_buff *, skb, s32, len_diff,
u32, mode, u64, flags)
{
@@ -2891,6 +2943,8 @@ BPF_CALL_4(bpf_skb_adjust_room, struct sk_buff *, skb, s32, len_diff,
return -EINVAL;
if (likely(mode == BPF_ADJ_ROOM_NET))
return bpf_skb_adjust_net(skb, len_diff);
+ if (likely(mode == BPF_ADJ_ROOM_DATA))
+ return bpf_skb_adjust_data(skb, len_diff);
return -ENOTSUPP;
}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 852dc17ab47a..47407fd5162b 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1467,6 +1467,8 @@ union bpf_attr {
*
* * **BPF_ADJ_ROOM_NET**: Adjust room at the network layer
* (room space is added or removed below the layer 3 header).
+ * * **BPF_ADJ_ROOM_DATA**: Adjust room at the beginning of the
+ * packet (room space is added or removed below skb->data).
*
* All values for *flags* are reserved for future usage, and must
* be left at zero.
@@ -2408,6 +2410,7 @@ enum bpf_func_id {
/* Mode for BPF_FUNC_skb_adjust_room helper. */
enum bpf_adj_room_mode {
BPF_ADJ_ROOM_NET,
+ BPF_ADJ_ROOM_DATA,
};
/* Mode for BPF_FUNC_skb_load_bytes_relative helper. */
--
2.18.0
^ permalink raw reply related
* Re: (2) (2) (2) [Kernel][NET] Bug report on packet defragmenting
From: Eric Dumazet @ 2018-11-08 15:12 UTC (permalink / raw)
To: soukjin.bae, Eric Dumazet, netdev@vger.kernel.org
In-Reply-To: <20181108075837epcms1p2747d212aee83ba0df60cc14ffac316aa@epcms1p2>
On 11/07/2018 11:58 PM, 배석진 wrote:
>> --------- Original Message ---------
>> Sender : Eric Dumazet <eric.dumazet@gmail.com>
>> Date : 2018-11-08 15:13 (GMT+9)
>> Title : Re: (2) (2) [Kernel][NET] Bug report on packet defragmenting
>>
>> On 11/07/2018 08:26 PM, Eric Dumazet wrote:
>>>
>>>
>>> On 11/07/2018 08:10 PM, 배석진 wrote:
>>>>> --------- Original Message ---------
>>>>> Sender : Eric Dumazet <eric.dumazet@gmail.com>
>>>>> Date : 2018-11-08 12:57 (GMT+9)
>>>>> Title : Re: (2) [Kernel][NET] Bug report on packet defragmenting
>>>>>
>>>>> On 11/07/2018 07:24 PM, Eric Dumazet wrote:
>>>>>
>>>>>> Sure, it is better if RPS is smarter, but if there is a bug in IPv6 defrag unit
>>>>>> we must investigate and root-cause it.
>>>>>
>>>>> BTW, IPv4 defrag seems to have the same issue.
>>>>
>>>>
>>>> yes, it could be.
>>>> key point isn't limitted to ipv6.
>>>>
>>>> maybe because of faster air-network and modem,
>>>> it looks like occure more often and we got recognized that.
>>>>
>>>> anyway,
>>>> we'll apply our patch to resolve this problem.
>>>
>>> Yeah, and I will fix the defrag units.
>>>
>>> We can not rely on other layers doing proper no-reorder logic for us.
>>>
>>> Problem here is that multiple cpus attempt concurrent rhashtable_insert_fast()
>>> and do not properly recover in case -EEXIST is returned.
>>>
>>> This is silly, of course :/
>>
>> Patch would be https://patchwork.ozlabs.org/patch/994658/
>
>
> Dear Dumazet,
>
> with your patch, kernel got the panic when packet recieved.
> I double checked after disable your patch, then no problem.
>
>
> <6>[ 119.702054] I[3: kworker/u18:1: 1705] LNK-RX(1464): 6b 80 00 00 05 90 2c 3e 20 01 44 30 00 05 04 01 ...
> <6>[ 119.702120] I[3: kworker/u18:1: 1705] __skb_flow_dissect: ports: 77500000
> <6>[ 119.702153] I[3: kworker/u18:1: 1705] get_rps_cpu: cpu:2, hash:2055028308
> <6>[ 119.702203] I[3: kworker/u18:1: 1705] LNK-RX(1212): 6b 80 00 00 04 94 2c 3e 20 01 44 30 00 05 04 01 ...
> <6>[ 119.702231] I[3: kworker/u18:1: 1705] __skb_flow_dissect: ports: 3c7e2c6b
> <6>[ 119.702258] I[3: kworker/u18:1: 1705] get_rps_cpu: cpu:1, hash:671343869
> <6>[ 119.702365] I[1: Binder:11369_2:11382] ipv6_rcv +++
> <6>[ 119.702375] I[2: swapper/2: 0] ipv6_rcv +++
> <6>[ 119.702406] I[2: swapper/2: 0] ipv6_defrag +++
> <6>[ 119.702425] I[1: Binder:11369_2:11382] ipv6_defrag +++
> <6>[ 119.702494] I[2: swapper/2: 0] ipv6_defrag: EINPROGRESS
> <6>[ 119.702522] I[2: swapper/2: 0] ipv6_rcv ---
> <6>[ 119.702628] I[1: Binder:11369_2:11382] ipv6_defrag ---
> <6>[ 119.702892] I[1: Binder:11369_2:11382] ipv6_defrag +++
> <6>[ 119.702922] I[1: Binder:11369_2:11382] ipv6_defrag ---
> <6>[ 119.702966] I[1: Binder:11369_2:11382] ipv6_rcv ---
> <0>[ 119.703792] [1: Binder:11369_2:11382] BUG: sleeping function called from invalid context at arch/arm64/mm/fault.c:518
> <3>[ 119.703826] [1: Binder:11369_2:11382] in_atomic(): 0, irqs_disabled(): 0, pid: 11382, name: Binder:11369_2
> <3>[ 119.703854] [1: Binder:11369_2:11382] Preemption disabled at:
> <4>[ 119.703888] [1: Binder:11369_2:11382] [<ffffff80080b13d4>] __do_softirq+0x68/0x3c4
> <4>[ 119.703934] [1: Binder:11369_2:11382] CPU: 1 PID: 11382 Comm: Binder:11369_2 Tainted: G S W 4.14.75-20181108-163447-eng #0
> <4>[ 119.703960] [1: Binder:11369_2:11382] Hardware name: Samsung BEYOND2LTE KOR SINGLE 19 board based on EXYNOS9820 (DT)
> <4>[ 119.703987] [1: Binder:11369_2:11382] Call trace:
> <4>[ 119.704015] [1: Binder:11369_2:11382] [<ffffff80080bd87c>] dump_backtrace+0x0/0x280
> <4>[ 119.704045] [1: Binder:11369_2:11382] [<ffffff80080bddd4>] show_stack+0x18/0x24
> <4>[ 119.704074] [1: Binder:11369_2:11382] [<ffffff80090bb3f8>] dump_stack+0xb8/0xf8
> <4>[ 119.704104] [1: Binder:11369_2:11382] [<ffffff800811f180>] ___might_sleep+0x16c/0x178
> <4>[ 119.704132] [1: Binder:11369_2:11382] [<ffffff800811efdc>] __might_sleep+0x4c/0x84
> <4>[ 119.704164] [1: Binder:11369_2:11382] [<ffffff80090dcf60>] do_page_fault+0x2e8/0x4b8
> <4>[ 119.704193] [1: Binder:11369_2:11382] [<ffffff80090dcbf4>] do_translation_fault+0x7c/0x100
> <4>[ 119.704219] [1: Binder:11369_2:11382] [<ffffff80080b0d70>] do_mem_abort+0x4c/0x12c
> <4>[ 119.704243] [1: Binder:11369_2:11382] Exception stack(0xffffff8038bf3ec0 to 0xffffff8038bf4000)
> <4>[ 119.704266] [1: Binder:11369_2:11382] 3ec0: 00000077b8262600 00000077b1bd0800 00000000708fcae0 0000000000000018
> ...
> <4>[ 119.704459] [1: Binder:11369_2:11382] 3fe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> <4>[ 119.704480] [1: Binder:11369_2:11382] [<ffffff80080b33d0>] el0_da+0x20/0x24
> <4>[ 119.704509] [1: Binder:11369_2:11382] ------------[ cut here ]------------
> <0>[ 119.704541] [1: Binder:11369_2:11382] kernel BUG at kernel/sched/core.c:6152!
> <2>[ 119.704563] [1: Binder:11369_2:11382] sec_debug_set_extra_info_fault = BUG / 0xffffff800811f180
> <0>[ 119.704603] [1: Binder:11369_2:11382] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>
>
Thanks for testing.
This is not a pristine net-next tree, this dump seems unrelated to the patch ?
^ permalink raw reply
* Re: SACK compression patch causing performance drop
From: Eric Dumazet @ 2018-11-08 15:14 UTC (permalink / raw)
To: Jean-Louis Dupond, netdev, edumazet
In-Reply-To: <ce731718-3f9d-04df-d6d2-a300d3b4fc29@dupond.be>
On 11/08/2018 12:23 AM, Jean-Louis Dupond wrote:
> Hi,
>
> Was somebody able to check this?
> Really think this should be fixed :)
>
> Thanks
> Jean-Louis
>
I somehow missed this email.
Packet captures might help, please send me
tcpdump -s 128 -i ethX -w sack.pcap
of some samples with or without the sack compression enabled.
> On 3/11/18 16:59, Jean-Louis Dupond wrote:
>> Hi All,
>>
>> On recent kernels we noticed a way lower throughput to our SAN system than before.
>> While on pre 4.18 kernels we had 400-700MB/sec read speed, on 4.18+ we only had 70-120MB/sec.
>>
>> The SAN is connected via iSCSI over a 10G network (ixgbe/X520 NICS if it matters).
>>
>> After some debugging, I tried to bisect between 4.17 and 4.18 to see what commit caused the slowdown.
>> It showed that the addition of the SACK compression (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5d9f4262b7ea41ca9981cc790e37cca6e37c789e) was the cause.
>>
>> And indeed, if I set net.ipv4.tcp_comp_sack_nr to 0 on 4.19 for example, the throughput is (almost) back to normal again.
>> So it seems like this change causes quite some performance issues.
>>
>> Any ideas?
>>
>> Thanks
>> Jean-Louis
>>
^ permalink raw reply
* Re: [PATCH v3 net-next] sock: Reset dst when changing sk_mark via setsockopt
From: Eric Dumazet @ 2018-11-08 15:16 UTC (permalink / raw)
To: David Barmann, netdev
In-Reply-To: <20181108141335.GA29455@konacove.com>
On 11/08/2018 06:13 AM, David Barmann wrote:
> When setting the SO_MARK socket option, if the mark changes, the dst
> needs to be reset so that a new route lookup is performed.
>
> This fixes the case where an application wants to change routing by
> setting a new sk_mark. If this is done after some packets have already
> been sent, the dst is cached and has no effect.
>
> Signed-off-by: David Barmann <david.barmann@stackpath.com>
> ---
Reviewed-by: Eric Dumazet <edumazet@google.com>
Thanks David !
^ permalink raw reply
* [net-next, PATCH 1/2] net: socionext: different approach on DMA
From: Ilias Apalodimas @ 2018-11-08 15:19 UTC (permalink / raw)
To: netdev, jaswinder.singh, ard.biesheuvel
Cc: masami.hiramatsu, masahisa.kojima, brouer, arnd, Ilias Apalodimas
Current driver dynamically allocates an skb and maps it as DMA Rx
buffer. In order to prepare for upcoming XDP changes, let's introduce a
different allocation scheme.
Buffers are allocated dynamically and mapped into hardware.
During the Rx operation the driver uses build_skb() to produce the
necessary buffers for the network stack.
This change increases performance ~15% on 64b packets with smmu disabled
and ~5% with smmu enabled
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
drivers/net/ethernet/socionext/netsec.c | 227 ++++++++++++++++++--------------
1 file changed, 128 insertions(+), 99 deletions(-)
diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index d9d0d03..9d24f29 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -234,6 +234,9 @@
#define DESC_NUM 256
+#define NETSEC_SKB_PAD (NET_SKB_PAD + NET_IP_ALIGN)
+#define NETSEC_RX_BUF_SZ 1536
+
#define DESC_SZ sizeof(struct netsec_de)
#define NETSEC_F_NETSEC_VER_MAJOR_NUM(x) ((x) & 0xffff0000)
@@ -571,34 +574,10 @@ static const struct ethtool_ops netsec_ethtool_ops = {
/************* NETDEV_OPS FOLLOW *************/
-static struct sk_buff *netsec_alloc_skb(struct netsec_priv *priv,
- struct netsec_desc *desc)
-{
- struct sk_buff *skb;
-
- if (device_get_dma_attr(priv->dev) == DEV_DMA_COHERENT) {
- skb = netdev_alloc_skb_ip_align(priv->ndev, desc->len);
- } else {
- desc->len = L1_CACHE_ALIGN(desc->len);
- skb = netdev_alloc_skb(priv->ndev, desc->len);
- }
- if (!skb)
- return NULL;
-
- desc->addr = skb->data;
- desc->dma_addr = dma_map_single(priv->dev, desc->addr, desc->len,
- DMA_FROM_DEVICE);
- if (dma_mapping_error(priv->dev, desc->dma_addr)) {
- dev_kfree_skb_any(skb);
- return NULL;
- }
- return skb;
-}
static void netsec_set_rx_de(struct netsec_priv *priv,
struct netsec_desc_ring *dring, u16 idx,
- const struct netsec_desc *desc,
- struct sk_buff *skb)
+ const struct netsec_desc *desc)
{
struct netsec_de *de = dring->vaddr + DESC_SZ * idx;
u32 attr = (1 << NETSEC_RX_PKT_OWN_FIELD) |
@@ -617,59 +596,6 @@ static void netsec_set_rx_de(struct netsec_priv *priv,
dring->desc[idx].dma_addr = desc->dma_addr;
dring->desc[idx].addr = desc->addr;
dring->desc[idx].len = desc->len;
- dring->desc[idx].skb = skb;
-}
-
-static struct sk_buff *netsec_get_rx_de(struct netsec_priv *priv,
- struct netsec_desc_ring *dring,
- u16 idx,
- struct netsec_rx_pkt_info *rxpi,
- struct netsec_desc *desc, u16 *len)
-{
- struct netsec_de de = {};
-
- memcpy(&de, dring->vaddr + DESC_SZ * idx, DESC_SZ);
-
- *len = de.buf_len_info >> 16;
-
- rxpi->err_flag = (de.attr >> NETSEC_RX_PKT_ER_FIELD) & 1;
- rxpi->rx_cksum_result = (de.attr >> NETSEC_RX_PKT_CO_FIELD) & 3;
- rxpi->err_code = (de.attr >> NETSEC_RX_PKT_ERR_FIELD) &
- NETSEC_RX_PKT_ERR_MASK;
- *desc = dring->desc[idx];
- return desc->skb;
-}
-
-static struct sk_buff *netsec_get_rx_pkt_data(struct netsec_priv *priv,
- struct netsec_rx_pkt_info *rxpi,
- struct netsec_desc *desc,
- u16 *len)
-{
- struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
- struct sk_buff *tmp_skb, *skb = NULL;
- struct netsec_desc td;
- int tail;
-
- *rxpi = (struct netsec_rx_pkt_info){};
-
- td.len = priv->ndev->mtu + 22;
-
- tmp_skb = netsec_alloc_skb(priv, &td);
-
- tail = dring->tail;
-
- if (!tmp_skb) {
- netsec_set_rx_de(priv, dring, tail, &dring->desc[tail],
- dring->desc[tail].skb);
- } else {
- skb = netsec_get_rx_de(priv, dring, tail, rxpi, desc, len);
- netsec_set_rx_de(priv, dring, tail, &td, tmp_skb);
- }
-
- /* move tail ahead */
- dring->tail = (dring->tail + 1) % DESC_NUM;
-
- return skb;
}
static int netsec_clean_tx_dring(struct netsec_priv *priv, int budget)
@@ -736,19 +662,65 @@ static int netsec_process_tx(struct netsec_priv *priv, int budget)
return done;
}
+static void *netsec_alloc_rx_data(struct netsec_priv *priv,
+ dma_addr_t *dma_handle, u16 *desc_len)
+{
+ size_t total_len = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ size_t payload_len = NETSEC_RX_BUF_SZ;
+ dma_addr_t mapping;
+ void *buf;
+
+ total_len += SKB_DATA_ALIGN(payload_len + NETSEC_SKB_PAD);
+
+ buf = napi_alloc_frag(total_len);
+ if (!buf)
+ return NULL;
+
+ mapping = dma_map_single(priv->dev, buf + NETSEC_SKB_PAD, payload_len,
+ DMA_FROM_DEVICE);
+ if (unlikely(dma_mapping_error(priv->dev, mapping)))
+ goto err_out;
+
+ *dma_handle = mapping;
+ *desc_len = payload_len;
+
+ return buf;
+
+err_out:
+ skb_free_frag(buf);
+ return NULL;
+}
+
+static void netsec_rx_fill(struct netsec_priv *priv, u16 from, u16 num)
+{
+ struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
+ u16 idx = from;
+
+ while (num) {
+ netsec_set_rx_de(priv, dring, idx, &dring->desc[idx]);
+ idx++;
+ if (idx >= DESC_NUM)
+ idx = 0;
+ num--;
+ }
+}
+
static int netsec_process_rx(struct netsec_priv *priv, int budget)
{
struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
struct net_device *ndev = priv->ndev;
struct netsec_rx_pkt_info rx_info;
- int done = 0;
- struct netsec_desc desc;
struct sk_buff *skb;
- u16 len;
+ int done = 0;
while (done < budget) {
u16 idx = dring->tail;
struct netsec_de *de = dring->vaddr + (DESC_SZ * idx);
+ struct netsec_desc *desc = &dring->desc[idx];
+ u16 pkt_len, desc_len;
+ dma_addr_t dma_handle;
+ void *buf_addr;
+ u32 truesize;
if (de->attr & (1U << NETSEC_RX_PKT_OWN_FIELD)) {
/* reading the register clears the irq */
@@ -762,18 +734,59 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
*/
dma_rmb();
done++;
- skb = netsec_get_rx_pkt_data(priv, &rx_info, &desc, &len);
- if (unlikely(!skb) || rx_info.err_flag) {
+
+ pkt_len = de->buf_len_info >> 16;
+ rx_info.err_code = (de->attr >> NETSEC_RX_PKT_ERR_FIELD) &
+ NETSEC_RX_PKT_ERR_MASK;
+ rx_info.err_flag = (de->attr >> NETSEC_RX_PKT_ER_FIELD) & 1;
+ if (rx_info.err_flag) {
netif_err(priv, drv, priv->ndev,
- "%s: rx fail err(%d)\n",
- __func__, rx_info.err_code);
+ "%s: rx fail err(%d)\n", __func__,
+ rx_info.err_code);
ndev->stats.rx_dropped++;
+ dring->tail = (dring->tail + 1) % DESC_NUM;
+ /* reuse buffer page frag */
+ netsec_rx_fill(priv, idx, 1);
continue;
}
+ rx_info.rx_cksum_result =
+ (de->attr >> NETSEC_RX_PKT_CO_FIELD) & 3;
- dma_unmap_single(priv->dev, desc.dma_addr, desc.len,
- DMA_FROM_DEVICE);
- skb_put(skb, len);
+ /* allocate a fresh buffer and map it to the hardware.
+ * This will eventually replace the old buffer in the hardware
+ */
+ buf_addr = netsec_alloc_rx_data(priv, &dma_handle, &desc_len);
+ if (unlikely(!buf_addr))
+ break;
+
+ dma_sync_single_for_cpu(priv->dev, desc->dma_addr, pkt_len,
+ DMA_FROM_DEVICE);
+ prefetch(desc->addr);
+
+ truesize = SKB_DATA_ALIGN(desc->len + NETSEC_SKB_PAD) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ skb = build_skb(desc->addr, truesize);
+ if (unlikely(!skb)) {
+ /* free the newly allocated buffer, we are not going to
+ * use it
+ */
+ dma_unmap_single(priv->dev, dma_handle, desc_len,
+ DMA_FROM_DEVICE);
+ skb_free_frag(buf_addr);
+ netif_err(priv, drv, priv->ndev,
+ "rx failed to build skb\n");
+ break;
+ }
+ dma_unmap_single_attrs(priv->dev, desc->dma_addr, desc->len,
+ DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
+
+ /* Update the descriptor with the new buffer we allocated */
+ desc->len = desc_len;
+ desc->dma_addr = dma_handle;
+ desc->addr = buf_addr;
+
+ skb_reserve(skb, NETSEC_SKB_PAD);
+ skb_put(skb, pkt_len);
skb->protocol = eth_type_trans(skb, priv->ndev);
if (priv->rx_cksum_offload_flag &&
@@ -782,8 +795,11 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
if (napi_gro_receive(&priv->napi, skb) != GRO_DROP) {
ndev->stats.rx_packets++;
- ndev->stats.rx_bytes += len;
+ ndev->stats.rx_bytes += pkt_len;
}
+
+ netsec_rx_fill(priv, idx, 1);
+ dring->tail = (dring->tail + 1) % DESC_NUM;
}
return done;
@@ -946,7 +962,10 @@ static void netsec_uninit_pkt_dring(struct netsec_priv *priv, int id)
dma_unmap_single(priv->dev, desc->dma_addr, desc->len,
id == NETSEC_RING_RX ? DMA_FROM_DEVICE :
DMA_TO_DEVICE);
- dev_kfree_skb(desc->skb);
+ if (id == NETSEC_RING_RX)
+ skb_free_frag(desc->addr);
+ else if (id == NETSEC_RING_TX)
+ dev_kfree_skb(desc->skb);
}
memset(dring->desc, 0, sizeof(struct netsec_desc) * DESC_NUM);
@@ -1002,22 +1021,30 @@ static int netsec_alloc_dring(struct netsec_priv *priv, enum ring_id id)
static int netsec_setup_rx_dring(struct netsec_priv *priv)
{
struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
- struct netsec_desc desc;
- struct sk_buff *skb;
- int n;
+ int i;
- desc.len = priv->ndev->mtu + 22;
+ for (i = 0; i < DESC_NUM; i++) {
+ struct netsec_desc *desc = &dring->desc[i];
+ dma_addr_t dma_handle;
+ void *buf;
+ u16 len;
- for (n = 0; n < DESC_NUM; n++) {
- skb = netsec_alloc_skb(priv, &desc);
- if (!skb) {
+ buf = netsec_alloc_rx_data(priv, &dma_handle, &len);
+ if (!buf) {
netsec_uninit_pkt_dring(priv, NETSEC_RING_RX);
- return -ENOMEM;
+ goto err_out;
}
- netsec_set_rx_de(priv, dring, n, &desc, skb);
+ desc->dma_addr = dma_handle;
+ desc->addr = buf;
+ desc->len = len;
}
+ netsec_rx_fill(priv, 0, DESC_NUM);
+
return 0;
+
+err_out:
+ return -ENOMEM;
}
static int netsec_netdev_load_ucode_region(struct netsec_priv *priv, u32 reg,
@@ -1377,6 +1404,8 @@ static int netsec_netdev_init(struct net_device *ndev)
int ret;
u16 data;
+ BUILD_BUG_ON_NOT_POWER_OF_2(DESC_NUM);
+
ret = netsec_alloc_dring(priv, NETSEC_RING_TX);
if (ret)
return ret;
--
2.7.4
^ permalink raw reply related
* [net-next, PATCH 2/2] net: socionext: refactor netsec_alloc_dring()
From: Ilias Apalodimas @ 2018-11-08 15:19 UTC (permalink / raw)
To: netdev, jaswinder.singh, ard.biesheuvel
Cc: masami.hiramatsu, masahisa.kojima, brouer, arnd, Ilias Apalodimas
In-Reply-To: <1541690395-8337-1-git-send-email-ilias.apalodimas@linaro.org>
return -ENOMEM directly instead of assigning it in a variable
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
drivers/net/ethernet/socionext/netsec.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index 9d24f29..bba9733 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -996,26 +996,21 @@ static void netsec_free_dring(struct netsec_priv *priv, int id)
static int netsec_alloc_dring(struct netsec_priv *priv, enum ring_id id)
{
struct netsec_desc_ring *dring = &priv->desc_ring[id];
- int ret = 0;
dring->vaddr = dma_zalloc_coherent(priv->dev, DESC_SZ * DESC_NUM,
&dring->desc_dma, GFP_KERNEL);
- if (!dring->vaddr) {
- ret = -ENOMEM;
+ if (!dring->vaddr)
goto err;
- }
dring->desc = kcalloc(DESC_NUM, sizeof(*dring->desc), GFP_KERNEL);
- if (!dring->desc) {
- ret = -ENOMEM;
+ if (!dring->desc)
goto err;
- }
return 0;
err:
netsec_free_dring(priv, id);
- return ret;
+ return -ENOMEM;
}
static int netsec_setup_rx_dring(struct netsec_priv *priv)
--
2.7.4
^ permalink raw reply related
* Re: [PATCH][net-hns3-next] net: hns3: fix spelling mistake, "assertting" -> "asserting"
From: David Miller @ 2018-11-09 1:08 UTC (permalink / raw)
To: colin.king
Cc: yisen.zhuang, salil.mehta, lipeng321, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20181108103202.1212-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Thu, 8 Nov 2018 10:32:02 +0000
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistake in dev_err error message
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 0/7] net: sched: prepare for more Qdisc offloads
From: Jakub Kicinski @ 2018-11-08 16:02 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: davem, netdev, oss-drivers, jiri, xiyou.wangcong, jhs,
nogah.frankel
In-Reply-To: <877ehnvlh0.fsf@toke.dk>
On Thu, 08 Nov 2018 12:48:27 +0100, Toke Høiland-Jørgensen wrote:
> Jakub Kicinski <jakub.kicinski@netronome.com> writes:
> > Hi!
> >
> > This series refactors the "switchdev" Qdisc offloads a little. We have
> > a few Qdiscs which can be fully offloaded today to the forwarding plane
> > of switching devices.
> >
> > First patch adds a helper for handing statistic dumps, the code seems
> > to be copy pasted between PRIO and RED.
>
> Hi Jakub
>
> I didn't know there was offload capabilities for AQMs, that's cool! Do
> you have any plans to add offloads for any of the modern AQMs (CoDel or
> PIE)?
I'd really like to add CoDel offload, but it's not a plan at this
point :(
^ permalink raw reply
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: David Ahern @ 2018-11-08 16:06 UTC (permalink / raw)
To: Paweł Staszewski, Jesper Dangaard Brouer; +Cc: netdev, Yoel Caspersen
In-Reply-To: <8dde3b32-59ce-38f3-5913-2ce08264e9dc@itcare.pl>
On 11/8/18 6:33 AM, Paweł Staszewski wrote:
>
>
> W dniu 07.11.2018 o 22:06, David Ahern pisze:
>> On 11/3/18 6:24 PM, Paweł Staszewski wrote:
>>>> Does your setup have any other device types besides physical ports with
>>>> VLANs (e.g., any macvlans or bonds)?
>>>>
>>>>
>>> no.
>>> just
>>> phy(mlnx)->vlans only config
>> VLAN and non-VLAN (and a mix) seem to work ok. Patches are here:
>> https://github.com/dsahern/linux.git bpf/kernel-tables-wip
>>
>> I got lazy with the vlan exports; right now it requires 8021q to be
>> builtin (CONFIG_VLAN_8021Q=y)
>>
>> You can use the xdp_fwd sample:
>> make O=kbuild -C samples/bpf -j 8
>>
>> Copy samples/bpf/xdp_fwd_kern.o and samples/bpf/xdp_fwd to the server
>> and run:
>> ./xdp_fwd <list of NIC ports>
>>
>> e.g., in my testing I run:
>> xdp_fwd eth1 eth2 eth3 eth4
>>
>> All of the relevant forwarding ports need to be on the same command
>> line. This version populates a second map to verify the egress port has
>> XDP enabled.
> Installed today on some lab server with mellanox connectx4
>
> And trying some simple static routing first - but after enabling xdp
> program - receiver is not receiving frames
>
> Route table is simple as possible for tests :)
>
> icmp ping test send from 192.168.22.237 to 172.16.0.2 - incomming
> packets on vlan 4081
>
> ip r
> default via 192.168.22.236 dev vlan4081
> 172.16.0.0/30 dev vlan1740 proto kernel scope link src 172.16.0.1
> 192.168.22.0/24 dev vlan4081 proto kernel scope link src 192.168.22.205
>
> neigh table:
> ip neigh ls
>
> 192.168.22.237 dev vlan4081 lladdr 00:25:90:fb:a6:8d REACHABLE
> 172.16.0.2 dev vlan1740 lladdr ac:1f:6b:2c:2e:5a REACHABLE
>
> and interfaces:
> 4: enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state
> UP mode DEFAULT group default qlen 1000
> link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
> 5: enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state
> UP mode DEFAULT group default qlen 1000
> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
> 6: vlan4081@enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
> noqueue state UP mode DEFAULT group default qlen 1000
> link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
> 7: vlan1740@enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
> noqueue state UP mode DEFAULT group default qlen 1000
> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>
> 5: enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdp/id:5 qdisc
> mq state UP group default qlen 1000
> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
> inet6 fe80::ae1f:6bff:fe07:c891/64 scope link
> valid_lft forever preferred_lft forever
> 6: vlan4081@enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
> noqueue state UP group default qlen 1000
> link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
> inet 192.168.22.205/24 scope global vlan4081
> valid_lft forever preferred_lft forever
> inet6 fe80::ae1f:6bff:fe07:c890/64 scope link
> valid_lft forever preferred_lft forever
> 7: vlan1740@enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
> noqueue state UP group default qlen 1000
> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
> inet 172.16.0.1/30 scope global vlan1740
> valid_lft forever preferred_lft forever
> inet6 fe80::ae1f:6bff:fe07:c891/64 scope link
> valid_lft forever preferred_lft forever
>
>
> xdp program detached:
> Receiving side tcpdump:
> 14:28:09.141233 IP 192.168.22.237 > 172.16.0.2: ICMP echo request, id
> 30227, seq 487, length 64
>
> I can see icmp requests
>
>
> enabling xdp
> ./xdp_fwd enp175s0f1 enp175s0f0
>
> 4: enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdp qdisc mq
> state UP mode DEFAULT group default qlen 1000
> link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
> prog/xdp id 5 tag 3c231ff1e5e77f3f
> 5: enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdp qdisc mq
> state UP mode DEFAULT group default qlen 1000
> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
> prog/xdp id 5 tag 3c231ff1e5e77f3f
> 6: vlan4081@enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
> noqueue state UP mode DEFAULT group default qlen 1000
> link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
> 7: vlan1740@enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
> noqueue state UP mode DEFAULT group default qlen 1000
> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>
What hardware is this?
Start with:
echo 1 > /sys/kernel/debug/tracing/events/xdp/enable
cat /sys/kernel/debug/tracing/trace_pipe
>From there, you can check the FIB lookups:
sysctl -w kernel.perf_event_max_stack=16
perf record -e fib:* -a -g -- sleep 5
perf script
^ permalink raw reply
* Re: [PATCH] e1000e: Change watchdog task to be delayed work
From: Jeff Kirsher @ 2018-11-09 1:49 UTC (permalink / raw)
To: Robert Eshleman; +Cc: David S. Miller, intel-wired-lan, netdev, linux-kernel
In-Reply-To: <1541290645-25033-1-git-send-email-bobbyeshleman@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 407 bytes --]
On Sat, 2018-11-03 at 17:17 -0700, Robert Eshleman wrote:
> This completes a pending TODO to use queue_delayed_work() instead of
> schedule_work().
>
> Signed-off-by: Robert Eshleman <bobbyeshleman@gmail.com>
> ---
> drivers/net/ethernet/intel/e1000e/netdev.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
Dropping this patch due to the problems seen with it applied.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v2 3/5] virtio_ring: add packed ring support
From: Tiwei Bie @ 2018-11-09 1:50 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, virtualization, linux-kernel, netdev, virtio-dev,
wexu, jfreimann
In-Reply-To: <20181108103155-mutt-send-email-mst@kernel.org>
On Thu, Nov 08, 2018 at 10:56:02AM -0500, Michael S. Tsirkin wrote:
> On Thu, Nov 08, 2018 at 07:51:48PM +0800, Tiwei Bie wrote:
> > On Thu, Nov 08, 2018 at 04:18:25PM +0800, Jason Wang wrote:
> > >
> > > On 2018/11/8 上午9:38, Tiwei Bie wrote:
> > > > > > +
> > > > > > + if (vq->vq.num_free < descs_used) {
> > > > > > + pr_debug("Can't add buf len %i - avail = %i\n",
> > > > > > + descs_used, vq->vq.num_free);
> > > > > > + /* FIXME: for historical reasons, we force a notify here if
> > > > > > + * there are outgoing parts to the buffer. Presumably the
> > > > > > + * host should service the ring ASAP. */
> > > > > I don't think we have a reason to do this for packed ring.
> > > > > No historical baggage there, right?
> > > > Based on the original commit log, it seems that the notify here
> > > > is just an "optimization". But I don't quite understand what does
> > > > the "the heuristics which KVM uses" refer to. If it's safe to drop
> > > > this in packed ring, I'd like to do it.
> > >
> > >
> > > According to the commit log, it seems like a workaround of lguest networking
> > > backend.
> >
> > Do you know why removing this notify in Tx will break "the
> > heuristics which KVM uses"? Or what does "the heuristics
> > which KVM uses" refer to?
>
> Yes. QEMU has a mode where it disables notifications and processes TX
> ring periodically from a timer. It's off by default but used to be on
> by default a long time ago. If ring becomes full this causes traffic
> stalls. As a work-around Rusty put in this hack to kick on ring full
> even with notifications disabled. It's easy enough to make sure QEMU
> does not combine devices with packed ring support with the timer hack.
> And I am guessing it's safe enough to also block that option completely
> e.g. when virtio 1.0 is enabled.
I see. Thanks!
>
> >
> > > I agree to drop it, we should not have such burden.
> > >
> > > But we should notice that, with this removed, the compare between packed vs
> > > split is kind of unfair. Consider the removal of lguest support recently,
> > > maybe we can drop this for split ring as well?
> > >
> > > Thanks
> > >
> > >
> > > >
> > > > commit 44653eae1407f79dff6f52fcf594ae84cb165ec4
> > > > Author: Rusty Russell<rusty@rustcorp.com.au>
> > > > Date: Fri Jul 25 12:06:04 2008 -0500
> > > >
> > > > virtio: don't always force a notification when ring is full
> > > > We force notification when the ring is full, even if the host has
> > > > indicated it doesn't want to know. This seemed like a good idea at
> > > > the time: if we fill the transmit ring, we should tell the host
> > > > immediately.
> > > > Unfortunately this logic also applies to the receiving ring, which is
> > > > refilled constantly. We should introduce real notification thesholds
> > > > to replace this logic. Meanwhile, removing the logic altogether breaks
> > > > the heuristics which KVM uses, so we use a hack: only notify if there are
> > > > outgoing parts of the new buffer.
> > > > Here are the number of exits with lguest's crappy network implementation:
> > > > Before:
> > > > network xmit 7859051 recv 236420
> > > > After:
> > > > network xmit 7858610 recv 118136
> > > > Signed-off-by: Rusty Russell<rusty@rustcorp.com.au>
> > > >
> > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > index 72bf8bc09014..21d9a62767af 100644
> > > > --- a/drivers/virtio/virtio_ring.c
> > > > +++ b/drivers/virtio/virtio_ring.c
> > > > @@ -87,8 +87,11 @@ static int vring_add_buf(struct virtqueue *_vq,
> > > > if (vq->num_free < out + in) {
> > > > pr_debug("Can't add buf len %i - avail = %i\n",
> > > > out + in, vq->num_free);
> > > > - /* We notify*even if* VRING_USED_F_NO_NOTIFY is set here. */
> > > > - vq->notify(&vq->vq);
> > > > + /* FIXME: for historical reasons, we force a notify here if
> > > > + * there are outgoing parts to the buffer. Presumably the
> > > > + * host should service the ring ASAP. */
> > > > + if (out)
> > > > + vq->notify(&vq->vq);
> > > > END_USE(vq);
> > > > return -ENOSPC;
> > > > }
> > > >
> > > >
^ permalink raw reply
* Re: [PATCH net-next 1/3] devlink: Add fw_version_check generic parameter
From: Ido Schimmel @ 2018-11-08 16:22 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Ido Schimmel, netdev@vger.kernel.org, davem@davemloft.net,
Jiri Pirko, Shalom Toledo, Moshe Shemesh, dsahern@gmail.com,
andrew@lunn.ch, f.fainelli@gmail.com, mlxsw
In-Reply-To: <20181107110518.6502665a@cakuba.netronome.com>
On Wed, Nov 07, 2018 at 11:05:18AM -0800, Jakub Kicinski wrote:
> On Wed, 7 Nov 2018 12:11:32 +0200, Ido Schimmel wrote:
> > On Tue, Nov 06, 2018 at 02:47:13PM -0800, Jakub Kicinski wrote:
> > > On Tue, 6 Nov 2018 22:37:51 +0200, Ido Schimmel wrote:
> > > > On Tue, Nov 06, 2018 at 12:19:13PM -0800, Jakub Kicinski wrote:
> > > > > We have a FW loading policy for NFP, too, so it'd be good to see if we
> > > > > can find a common ground.
> > > >
> > > > If the parameter is set, then device runs with whatever firmware version
> > > > was last flashed (via ethtool, for example). Otherwise, the driver will
> > > > flash a version according to its policy. In mlxsw, it is a specific
> > > > version.
> > > >
> > > > Will that work for you?
> > >
> > > Our FW is always backward compatible so there is no need to downgrade.
> > >
> > > What we have is this more along these lines: there are two images one
> > > on disk and second in the flash. The FW loading policy can decide
> > > which of those should be preferred, or should the versions be compared
> > > and the newer one win (default). But we don't flash the newer FW, just
> > > potentially load it from disk today.
> >
> > Not sure I understand. You have a currently flashed firmware and another
> > firmware image on disk.
>
> Correct.
>
> > You potentially load the firmware from the disk, but never flash it?
>
> You can flash it if you want, but default is to load the
> linux-firmware/disk one when system boots.
>
> Flashing is useful for example if you have some super special FW that
> you prefer over whatever comes from linux-firmware updates, or (most
> commonly) if distributing FWs is hard in your provisioning system (ugh).
>
> > If so, why load it?
>
> We need to load some FW..
OK. Got it now. mlxsw must flash a firmware in order to load it, but in
your case it's not necessary.
> > > I'm not sure whether 'fw_version_check' describes the general behaviour
> > > of not updating the FW in flash. The policy of updating the FW in the
> > > flash if the one on disk is newer seems to be something we could adopt
> > > as well. Can we come up with a more general parameter which could
> > > select FW loading policy that'd for both cases?
> > >
> > > Would values like these make any sense to you?
> > > - driver preferred (your default behaviour, we don't support since
> > > driver doesn't care);
> > > - newest (our default, device compares images and picks newer);
> > > - always disk (always run with what's on the disk, regardless of
> > > versions);
> > > - always flash (always run with what's already in flash, don't look at
> > > disk);
> > >
> > > Separate bool parameter 'fw_flash_auto_update' would decide whether the
> > > selected FW should be flashed to the device (always true for you AFAIU).
> > >
> > > Let me know if that makes sense, it would be nice if we could converge
> > > on a common solution, or at least name our parameters sufficiently
> > > distinctly to avoid confusion :)
> >
> > I think that the above scheme is a bit too complicated and I'm not sure
> > this is warranted. I'll try to better explain the motivation for this
> > parameter and where we are coming from.
>
> Certainly, let me know if what I wrote above helps to understand the
> motivation.
Yes, it does. Thanks!
>
> > We want to keep things as simple as possible. This means we don't want
> > users to fiddle with devlink parameter unless they have to. Things
> > should just work.
>
> 100% agree, you can choose the default for the parameter to be whatever
> you want, nobody will have to touch it in normal operation.
>
> I'm just proposing widening the values of the parameter so it works for
> others (given you propose it as generic).
>
> > This parameter should only be used in exceptional cases.
> >
> > For example, when user reports a problem with current firmware version
> > enforced by the driver. Assuming we have a new firmware version with a
> > fix, we would like the user to try it and confirm bug was fixed.
> > Ideally, the user would do something like this:
> >
> > 1. Flash new firmware via ethtool
> > 2. Perform a reset via devlink to have changes take effect
> >
> > Problem is that after the reset the driver's init sequence will run and
> > overwrite the new firmware version with the one specified in its source
> > as a compatible version. The driver needs to enforce a specific version
> > because newer versions are not necessarily backward compatible.
> >
> > Therefore, we added this new parameter that gives the user the ability
> > to explicitly run with a different version than what was specified as
> > compatible. New sequence is therefore:
> >
> > 1. Flash new firmware via ethtool
> > 2. Toggle devlink parameter
> > 3. Perform a reset via devlink to have changes take effect
> >
> > Firmware loading policy is basically always go with what the driver is
> > enforcing (it knows best), unless user specified he/she knows better.
>
> Thanks, got it.
>
> > I think this is both generic and simple, but I possibly didn't
> > understand the full scope of your use cases.
>
> I agree, it is simple, but the semantics of the parameter you're adding
> unnecessarily involve firmware flashing, which is mlxsw quirk. My
> impression (mostly from my WiFi driver days) is that the FW is either in
> flash or in /lib/firmware, but rarely /lib/firmware autofeeds the flash.
>
> The commit message says "Many drivers checking the device's firmware
> version during the initialization flow and flashing a compatible
> version if the current version is not." Do Ethernet drivers do that
> or some other drivers?
Yes, the terminology is not accurate. The intention was that drivers
load (we used flashing...) a compatible firmware version during their
initialization routine.
> To reiterate I think the definition of the flag unnecessarily involves
> flashing. It may make sense to mlxsw users, but I'd postulate it won't
> to almost everyone else :)
>
> Therefore AFAICS we could make one of two improvements:
> - make this a full-fledged flash update policy with clear operational
> semantics which others can reuse; or
> - remove the flashing part and leave it unmentioned - definition of the
> parameter becomes in a nutshell "ignore all FW version
> incompatibilities"; the limitation of mlxsw having to flash a FW
> image to load it is then an implementation detail.
Discussed the topic with Jakub during the bi-weekly switchdev call. We
will submit a v2 with a parameter called 'fw_load_policy' that will have
these options:
* driver: Load firmware version preferred by the driver (default in
mlxsw)
* flash: Load firmware currently stored in flash
Therefore, new sequence is:
1. Flash new firmware via ethtool
2. Set devlink 'fw_load_policy' parameter to 'flash'
3. Perform a reset via devlink to have changes take effect
The parameter can be later extended with more options such as 'newest'
and 'disk' that Jakub mentioned earlier in the thread.
We will not add the 'fw_flash_auto_update' parameter as it is not needed
for mlxsw, but can be added for nfp/others in the future.
Jakub, thanks again for your time!
^ permalink raw reply
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Paweł Staszewski @ 2018-11-08 16:25 UTC (permalink / raw)
To: David Ahern, Jesper Dangaard Brouer; +Cc: netdev, Yoel Caspersen
In-Reply-To: <6165513d-1e27-31dc-8f94-9de029a73f93@gmail.com>
W dniu 08.11.2018 o 17:06, David Ahern pisze:
> On 11/8/18 6:33 AM, Paweł Staszewski wrote:
>>
>> W dniu 07.11.2018 o 22:06, David Ahern pisze:
>>> On 11/3/18 6:24 PM, Paweł Staszewski wrote:
>>>>> Does your setup have any other device types besides physical ports with
>>>>> VLANs (e.g., any macvlans or bonds)?
>>>>>
>>>>>
>>>> no.
>>>> just
>>>> phy(mlnx)->vlans only config
>>> VLAN and non-VLAN (and a mix) seem to work ok. Patches are here:
>>> https://github.com/dsahern/linux.git bpf/kernel-tables-wip
>>>
>>> I got lazy with the vlan exports; right now it requires 8021q to be
>>> builtin (CONFIG_VLAN_8021Q=y)
>>>
>>> You can use the xdp_fwd sample:
>>> make O=kbuild -C samples/bpf -j 8
>>>
>>> Copy samples/bpf/xdp_fwd_kern.o and samples/bpf/xdp_fwd to the server
>>> and run:
>>> ./xdp_fwd <list of NIC ports>
>>>
>>> e.g., in my testing I run:
>>> xdp_fwd eth1 eth2 eth3 eth4
>>>
>>> All of the relevant forwarding ports need to be on the same command
>>> line. This version populates a second map to verify the egress port has
>>> XDP enabled.
>> Installed today on some lab server with mellanox connectx4
>>
>> And trying some simple static routing first - but after enabling xdp
>> program - receiver is not receiving frames
>>
>> Route table is simple as possible for tests :)
>>
>> icmp ping test send from 192.168.22.237 to 172.16.0.2 - incomming
>> packets on vlan 4081
>>
>> ip r
>> default via 192.168.22.236 dev vlan4081
>> 172.16.0.0/30 dev vlan1740 proto kernel scope link src 172.16.0.1
>> 192.168.22.0/24 dev vlan4081 proto kernel scope link src 192.168.22.205
>>
>> neigh table:
>> ip neigh ls
>>
>> 192.168.22.237 dev vlan4081 lladdr 00:25:90:fb:a6:8d REACHABLE
>> 172.16.0.2 dev vlan1740 lladdr ac:1f:6b:2c:2e:5a REACHABLE
>>
>> and interfaces:
>> 4: enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state
>> UP mode DEFAULT group default qlen 1000
>> link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
>> 5: enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state
>> UP mode DEFAULT group default qlen 1000
>> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>> 6: vlan4081@enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
>> noqueue state UP mode DEFAULT group default qlen 1000
>> link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
>> 7: vlan1740@enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
>> noqueue state UP mode DEFAULT group default qlen 1000
>> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>>
>> 5: enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdp/id:5 qdisc
>> mq state UP group default qlen 1000
>> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>> inet6 fe80::ae1f:6bff:fe07:c891/64 scope link
>> valid_lft forever preferred_lft forever
>> 6: vlan4081@enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
>> noqueue state UP group default qlen 1000
>> link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
>> inet 192.168.22.205/24 scope global vlan4081
>> valid_lft forever preferred_lft forever
>> inet6 fe80::ae1f:6bff:fe07:c890/64 scope link
>> valid_lft forever preferred_lft forever
>> 7: vlan1740@enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
>> noqueue state UP group default qlen 1000
>> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>> inet 172.16.0.1/30 scope global vlan1740
>> valid_lft forever preferred_lft forever
>> inet6 fe80::ae1f:6bff:fe07:c891/64 scope link
>> valid_lft forever preferred_lft forever
>>
>>
>> xdp program detached:
>> Receiving side tcpdump:
>> 14:28:09.141233 IP 192.168.22.237 > 172.16.0.2: ICMP echo request, id
>> 30227, seq 487, length 64
>>
>> I can see icmp requests
>>
>>
>> enabling xdp
>> ./xdp_fwd enp175s0f1 enp175s0f0
>>
>> 4: enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdp qdisc mq
>> state UP mode DEFAULT group default qlen 1000
>> link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
>> prog/xdp id 5 tag 3c231ff1e5e77f3f
>> 5: enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdp qdisc mq
>> state UP mode DEFAULT group default qlen 1000
>> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>> prog/xdp id 5 tag 3c231ff1e5e77f3f
>> 6: vlan4081@enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
>> noqueue state UP mode DEFAULT group default qlen 1000
>> link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
>> 7: vlan1740@enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
>> noqueue state UP mode DEFAULT group default qlen 1000
>> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>>
> What hardware is this?
>
> Start with:
>
> echo 1 > /sys/kernel/debug/tracing/events/xdp/enable
> cat /sys/kernel/debug/tracing/trace_pipe
cat /sys/kernel/debug/tracing/trace_pipe
<idle>-0 [045] ..s. 68469.467752: xdp_devmap_xmit:
ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0 drops=1
from_ifindex=4 to_ifindex=5 err=-6
<idle>-0 [045] ..s. 68470.483836: xdp_redirect_map:
prog_id=30 action=REDIRECT ifindex=4 to_ifindex=5 err=0 map_id=32
map_index=5
<idle>-0 [045] ..s. 68470.483837: xdp_devmap_xmit:
ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0 drops=1
from_ifindex=4 to_ifindex=5 err=-6
<idle>-0 [045] ..s. 68471.503853: xdp_redirect_map:
prog_id=30 action=REDIRECT ifindex=4 to_ifindex=5 err=0 map_id=32
map_index=5
<idle>-0 [045] ..s. 68471.503853: xdp_devmap_xmit:
ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0 drops=1
from_ifindex=4 to_ifindex=5 err=-6
<idle>-0 [045] ..s. 68472.527871: xdp_redirect_map:
prog_id=30 action=REDIRECT ifindex=4 to_ifindex=5 err=0 map_id=32
map_index=5
<idle>-0 [045] ..s. 68472.527877: xdp_devmap_xmit:
ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0 drops=1
from_ifindex=4 to_ifindex=5 err=-6
<idle>-0 [045] ..s. 68473.551876: xdp_redirect_map:
prog_id=30 action=REDIRECT ifindex=4 to_ifindex=5 err=0 map_id=32
map_index=5
<idle>-0 [045] ..s. 68473.551880: xdp_devmap_xmit:
ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0 drops=1
from_ifindex=4 to_ifindex=5 err=-6
<idle>-0 [045] ..s. 68474.575893: xdp_redirect_map:
prog_id=30 action=REDIRECT ifindex=4 to_ifindex=5 err=0 map_id=32
map_index=5
<idle>-0 [045] ..s. 68474.575897: xdp_devmap_xmit:
ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0 drops=1
from_ifindex=4 to_ifindex=5 err=-6
<idle>-0 [045] ..s. 68475.599909: xdp_redirect_map:
prog_id=30 action=REDIRECT ifindex=4 to_ifindex=5 err=0 map_id=32
map_index=5
<idle>-0 [045] ..s. 68475.599912: xdp_devmap_xmit:
ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0 drops=1
from_ifindex=4 to_ifindex=5 err=-6
>
> >From there, you can check the FIB lookups:
> sysctl -w kernel.perf_event_max_stack=16
> perf record -e fib:* -a -g -- sleep 5
> perf script
>
swapper 0 [045] 68493.746274: fib:fib_table_lookup: table 254 oif 0
iif 6 proto 1 192.168.22.237/0 -> 172.16.0.2/0 tos 0 scope 0 flags 0 ==>
dev vlan1740 gw 0.0.0.0 src 172.16.0.1 err 0
7fff818c13b5 fib_table_lookup ([kernel.kallsyms])
swapper 0 [045] 68494.770287: fib:fib_table_lookup: table 254 oif 0
iif 6 proto 1 192.168.22.237/0 -> 172.16.0.2/0 tos 0 scope 0 flags 0 ==>
dev vlan1740 gw 0.0.0.0 src 172.16.0.1 err 0
7fff818c13b5 fib_table_lookup ([kernel.kallsyms])
swapper 0 [045] 68495.794304: fib:fib_table_lookup: table 254 oif 0
iif 6 proto 1 192.168.22.237/0 -> 172.16.0.2/0 tos 0 scope 0 flags 0 ==>
dev vlan1740 gw 0.0.0.0 src 172.16.0.1 err 0
7fff818c13b5 fib_table_lookup ([kernel.kallsyms])
swapper 0 [045] 68496.818308: fib:fib_table_lookup: table 254 oif 0
iif 6 proto 1 192.168.22.237/0 -> 172.16.0.2/0 tos 0 scope 0 flags 0 ==>
dev vlan1740 gw 0.0.0.0 src 172.16.0.1 err 0
7fff818c13b5 fib_table_lookup ([kernel.kallsyms])
swapper 0 [045] 68497.842313: fib:fib_table_lookup: table 254 oif 0
iif 6 proto 1 192.168.22.237/0 -> 172.16.0.2/0 tos 0 scope 0 flags 0 ==>
dev vlan1740 gw 0.0.0.0 src 172.16.0.1 err 0
7fff818c13b5 fib_table_lookup ([kernel.kallsyms])
^ permalink raw reply
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Paweł Staszewski @ 2018-11-08 16:27 UTC (permalink / raw)
To: David Ahern, Jesper Dangaard Brouer; +Cc: netdev, Yoel Caspersen
In-Reply-To: <11199f9f-da21-527b-f5db-0bbf1e448a8b@itcare.pl>
W dniu 08.11.2018 o 17:25, Paweł Staszewski pisze:
>
>
> W dniu 08.11.2018 o 17:06, David Ahern pisze:
>> On 11/8/18 6:33 AM, Paweł Staszewski wrote:
>>>
>>> W dniu 07.11.2018 o 22:06, David Ahern pisze:
>>>> On 11/3/18 6:24 PM, Paweł Staszewski wrote:
>>>>>> Does your setup have any other device types besides physical
>>>>>> ports with
>>>>>> VLANs (e.g., any macvlans or bonds)?
>>>>>>
>>>>>>
>>>>> no.
>>>>> just
>>>>> phy(mlnx)->vlans only config
>>>> VLAN and non-VLAN (and a mix) seem to work ok. Patches are here:
>>>> https://github.com/dsahern/linux.git bpf/kernel-tables-wip
>>>>
>>>> I got lazy with the vlan exports; right now it requires 8021q to be
>>>> builtin (CONFIG_VLAN_8021Q=y)
>>>>
>>>> You can use the xdp_fwd sample:
>>>> make O=kbuild -C samples/bpf -j 8
>>>>
>>>> Copy samples/bpf/xdp_fwd_kern.o and samples/bpf/xdp_fwd to the server
>>>> and run:
>>>> ./xdp_fwd <list of NIC ports>
>>>>
>>>> e.g., in my testing I run:
>>>> xdp_fwd eth1 eth2 eth3 eth4
>>>>
>>>> All of the relevant forwarding ports need to be on the same command
>>>> line. This version populates a second map to verify the egress port
>>>> has
>>>> XDP enabled.
>>> Installed today on some lab server with mellanox connectx4
>>>
>>> And trying some simple static routing first - but after enabling xdp
>>> program - receiver is not receiving frames
>>>
>>> Route table is simple as possible for tests :)
>>>
>>> icmp ping test send from 192.168.22.237 to 172.16.0.2 - incomming
>>> packets on vlan 4081
>>>
>>> ip r
>>> default via 192.168.22.236 dev vlan4081
>>> 172.16.0.0/30 dev vlan1740 proto kernel scope link src 172.16.0.1
>>> 192.168.22.0/24 dev vlan4081 proto kernel scope link src 192.168.22.205
>>>
>>> neigh table:
>>> ip neigh ls
>>>
>>> 192.168.22.237 dev vlan4081 lladdr 00:25:90:fb:a6:8d REACHABLE
>>> 172.16.0.2 dev vlan1740 lladdr ac:1f:6b:2c:2e:5a REACHABLE
>>>
>>> and interfaces:
>>> 4: enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq
>>> state
>>> UP mode DEFAULT group default qlen 1000
>>> link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
>>> 5: enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq
>>> state
>>> UP mode DEFAULT group default qlen 1000
>>> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>>> 6: vlan4081@enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500
>>> qdisc
>>> noqueue state UP mode DEFAULT group default qlen 1000
>>> link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
>>> 7: vlan1740@enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500
>>> qdisc
>>> noqueue state UP mode DEFAULT group default qlen 1000
>>> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>>>
>>> 5: enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdp/id:5
>>> qdisc
>>> mq state UP group default qlen 1000
>>> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>>> inet6 fe80::ae1f:6bff:fe07:c891/64 scope link
>>> valid_lft forever preferred_lft forever
>>> 6: vlan4081@enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500
>>> qdisc
>>> noqueue state UP group default qlen 1000
>>> link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
>>> inet 192.168.22.205/24 scope global vlan4081
>>> valid_lft forever preferred_lft forever
>>> inet6 fe80::ae1f:6bff:fe07:c890/64 scope link
>>> valid_lft forever preferred_lft forever
>>> 7: vlan1740@enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500
>>> qdisc
>>> noqueue state UP group default qlen 1000
>>> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>>> inet 172.16.0.1/30 scope global vlan1740
>>> valid_lft forever preferred_lft forever
>>> inet6 fe80::ae1f:6bff:fe07:c891/64 scope link
>>> valid_lft forever preferred_lft forever
>>>
>>>
>>> xdp program detached:
>>> Receiving side tcpdump:
>>> 14:28:09.141233 IP 192.168.22.237 > 172.16.0.2: ICMP echo request, id
>>> 30227, seq 487, length 64
>>>
>>> I can see icmp requests
>>>
>>>
>>> enabling xdp
>>> ./xdp_fwd enp175s0f1 enp175s0f0
>>>
>>> 4: enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdp qdisc mq
>>> state UP mode DEFAULT group default qlen 1000
>>> link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
>>> prog/xdp id 5 tag 3c231ff1e5e77f3f
>>> 5: enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdp qdisc mq
>>> state UP mode DEFAULT group default qlen 1000
>>> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>>> prog/xdp id 5 tag 3c231ff1e5e77f3f
>>> 6: vlan4081@enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500
>>> qdisc
>>> noqueue state UP mode DEFAULT group default qlen 1000
>>> link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
>>> 7: vlan1740@enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500
>>> qdisc
>>> noqueue state UP mode DEFAULT group default qlen 1000
>>> link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>>>
>> What hardware is this?
>>
mellanox connectx 4
ethtool -i enp175s0f0
driver: mlx5_core
version: 5.0-0
firmware-version: 12.21.1000 (SM_2001000001033)
expansion-rom-version:
bus-info: 0000:af:00.0
supports-statistics: yes
supports-test: yes
supports-eeprom-access: no
supports-register-dump: no
supports-priv-flags: yes
ethtool -i enp175s0f1
driver: mlx5_core
version: 5.0-0
firmware-version: 12.21.1000 (SM_2001000001033)
expansion-rom-version:
bus-info: 0000:af:00.1
supports-statistics: yes
supports-test: yes
supports-eeprom-access: no
supports-register-dump: no
supports-priv-flags: yes
>> Start with:
>>
>> echo 1 > /sys/kernel/debug/tracing/events/xdp/enable
>> cat /sys/kernel/debug/tracing/trace_pipe
> cat /sys/kernel/debug/tracing/trace_pipe
> <idle>-0 [045] ..s. 68469.467752: xdp_devmap_xmit:
> ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0 drops=1
> from_ifindex=4 to_ifindex=5 err=-6
> <idle>-0 [045] ..s. 68470.483836: xdp_redirect_map:
> prog_id=30 action=REDIRECT ifindex=4 to_ifindex=5 err=0 map_id=32
> map_index=5
> <idle>-0 [045] ..s. 68470.483837: xdp_devmap_xmit:
> ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0 drops=1
> from_ifindex=4 to_ifindex=5 err=-6
> <idle>-0 [045] ..s. 68471.503853: xdp_redirect_map:
> prog_id=30 action=REDIRECT ifindex=4 to_ifindex=5 err=0 map_id=32
> map_index=5
> <idle>-0 [045] ..s. 68471.503853: xdp_devmap_xmit:
> ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0 drops=1
> from_ifindex=4 to_ifindex=5 err=-6
> <idle>-0 [045] ..s. 68472.527871: xdp_redirect_map:
> prog_id=30 action=REDIRECT ifindex=4 to_ifindex=5 err=0 map_id=32
> map_index=5
> <idle>-0 [045] ..s. 68472.527877: xdp_devmap_xmit:
> ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0 drops=1
> from_ifindex=4 to_ifindex=5 err=-6
> <idle>-0 [045] ..s. 68473.551876: xdp_redirect_map:
> prog_id=30 action=REDIRECT ifindex=4 to_ifindex=5 err=0 map_id=32
> map_index=5
> <idle>-0 [045] ..s. 68473.551880: xdp_devmap_xmit:
> ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0 drops=1
> from_ifindex=4 to_ifindex=5 err=-6
> <idle>-0 [045] ..s. 68474.575893: xdp_redirect_map:
> prog_id=30 action=REDIRECT ifindex=4 to_ifindex=5 err=0 map_id=32
> map_index=5
> <idle>-0 [045] ..s. 68474.575897: xdp_devmap_xmit:
> ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0 drops=1
> from_ifindex=4 to_ifindex=5 err=-6
> <idle>-0 [045] ..s. 68475.599909: xdp_redirect_map:
> prog_id=30 action=REDIRECT ifindex=4 to_ifindex=5 err=0 map_id=32
> map_index=5
> <idle>-0 [045] ..s. 68475.599912: xdp_devmap_xmit:
> ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0 drops=1
> from_ifindex=4 to_ifindex=5 err=-6
>
>
>
>>
>> >From there, you can check the FIB lookups:
>> sysctl -w kernel.perf_event_max_stack=16
>> perf record -e fib:* -a -g -- sleep 5
>> perf script
>>
> swapper 0 [045] 68493.746274: fib:fib_table_lookup: table 254 oif
> 0 iif 6 proto 1 192.168.22.237/0 -> 172.16.0.2/0 tos 0 scope 0 flags 0
> ==> dev vlan1740 gw 0.0.0.0 src 172.16.0.1 err 0
> 7fff818c13b5 fib_table_lookup ([kernel.kallsyms])
>
> swapper 0 [045] 68494.770287: fib:fib_table_lookup: table 254 oif
> 0 iif 6 proto 1 192.168.22.237/0 -> 172.16.0.2/0 tos 0 scope 0 flags 0
> ==> dev vlan1740 gw 0.0.0.0 src 172.16.0.1 err 0
> 7fff818c13b5 fib_table_lookup ([kernel.kallsyms])
>
> swapper 0 [045] 68495.794304: fib:fib_table_lookup: table 254 oif
> 0 iif 6 proto 1 192.168.22.237/0 -> 172.16.0.2/0 tos 0 scope 0 flags 0
> ==> dev vlan1740 gw 0.0.0.0 src 172.16.0.1 err 0
> 7fff818c13b5 fib_table_lookup ([kernel.kallsyms])
>
> swapper 0 [045] 68496.818308: fib:fib_table_lookup: table 254 oif
> 0 iif 6 proto 1 192.168.22.237/0 -> 172.16.0.2/0 tos 0 scope 0 flags 0
> ==> dev vlan1740 gw 0.0.0.0 src 172.16.0.1 err 0
> 7fff818c13b5 fib_table_lookup ([kernel.kallsyms])
>
> swapper 0 [045] 68497.842313: fib:fib_table_lookup: table 254 oif
> 0 iif 6 proto 1 192.168.22.237/0 -> 172.16.0.2/0 tos 0 scope 0 flags 0
> ==> dev vlan1740 gw 0.0.0.0 src 172.16.0.1 err 0
> 7fff818c13b5 fib_table_lookup ([kernel.kallsyms])
>
>
^ permalink raw reply
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: David Ahern @ 2018-11-08 16:32 UTC (permalink / raw)
To: Paweł Staszewski, Jesper Dangaard Brouer; +Cc: netdev, Yoel Caspersen
In-Reply-To: <87a2a15c-f9bf-743b-b4c5-7d37da0bd887@itcare.pl>
On 11/8/18 9:27 AM, Paweł Staszewski wrote:
>>> What hardware is this?
>>>
> mellanox connectx 4
> ethtool -i enp175s0f0
> driver: mlx5_core
> version: 5.0-0
> firmware-version: 12.21.1000 (SM_2001000001033)
> expansion-rom-version:
> bus-info: 0000:af:00.0
> supports-statistics: yes
> supports-test: yes
> supports-eeprom-access: no
> supports-register-dump: no
> supports-priv-flags: yes
>
> ethtool -i enp175s0f1
> driver: mlx5_core
> version: 5.0-0
> firmware-version: 12.21.1000 (SM_2001000001033)
> expansion-rom-version:
> bus-info: 0000:af:00.1
> supports-statistics: yes
> supports-test: yes
> supports-eeprom-access: no
> supports-register-dump: no
> supports-priv-flags: yes
>
>>> Start with:
>>>
>>> echo 1 > /sys/kernel/debug/tracing/events/xdp/enable
>>> cat /sys/kernel/debug/tracing/trace_pipe
>> cat /sys/kernel/debug/tracing/trace_pipe
>> <idle>-0 [045] ..s. 68469.467752: xdp_devmap_xmit:
>> ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0 drops=1
>> from_ifindex=4 to_ifindex=5 err=-6
FIB lookup is good, the redirect is happening, but the mlx5 driver does
not like it.
I think the -6 is coming from the mlx5 driver and the packet is getting
dropped. Perhaps this check in mlx5e_xdp_xmit:
if (unlikely(sq_num >= priv->channels.num))
return -ENXIO;
>> swapper 0 [045] 68493.746274: fib:fib_table_lookup: table 254 oif
>> 0 iif 6 proto 1 192.168.22.237/0 -> 172.16.0.2/0 tos 0 scope 0 flags 0
>> ==> dev vlan1740 gw 0.0.0.0 src 172.16.0.1 err 0
>> 7fff818c13b5 fib_table_lookup ([kernel.kallsyms])
>>
>> swapper 0 [045] 68494.770287: fib:fib_table_lookup: table 254 oif
>> 0 iif 6 proto 1 192.168.22.237/0 -> 172.16.0.2/0 tos 0 scope 0 flags 0
>> ==> dev vlan1740 gw 0.0.0.0 src 172.16.0.1 err 0
>> 7fff818c13b5 fib_table_lookup ([kernel.kallsyms])
>>
>> swapper 0 [045] 68495.794304: fib:fib_table_lookup: table 254 oif
>> 0 iif 6 proto 1 192.168.22.237/0 -> 172.16.0.2/0 tos 0 scope 0 flags 0
>> ==> dev vlan1740 gw 0.0.0.0 src 172.16.0.1 err 0
>> 7fff818c13b5 fib_table_lookup ([kernel.kallsyms])
>>
>> swapper 0 [045] 68496.818308: fib:fib_table_lookup: table 254 oif
>> 0 iif 6 proto 1 192.168.22.237/0 -> 172.16.0.2/0 tos 0 scope 0 flags 0
>> ==> dev vlan1740 gw 0.0.0.0 src 172.16.0.1 err 0
>> 7fff818c13b5 fib_table_lookup ([kernel.kallsyms])
>>
>> swapper 0 [045] 68497.842313: fib:fib_table_lookup: table 254 oif
>> 0 iif 6 proto 1 192.168.22.237/0 -> 172.16.0.2/0 tos 0 scope 0 flags 0
>> ==> dev vlan1740 gw 0.0.0.0 src 172.16.0.1 err 0
>> 7fff818c13b5 fib_table_lookup ([kernel.kallsyms])
^ permalink raw reply
* [PATCH 3/3] ath6kl: Use debug instead of error message when disabled
From: Kyle Roeschley @ 2018-11-08 16:36 UTC (permalink / raw)
To: Kalle Valo
Cc: David S . Miller, linux-wireless, netdev, linux-kernel,
Kyle Roeschley
In-Reply-To: <20181108163659.19535-1-kyle.roeschley@ni.com>
This is not an unexpected condition, so we don't need to be shouting to the
world about it.
Signed-off-by: Kyle Roeschley <kyle.roeschley@ni.com>
---
drivers/net/wireless/ath/ath6kl/cfg80211.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index d7c626d9594e..59dd50866932 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -291,7 +291,7 @@ static bool ath6kl_cfg80211_ready(struct ath6kl_vif *vif)
}
if (!test_bit(WLAN_ENABLED, &vif->flags)) {
- ath6kl_err("wlan disabled\n");
+ ath6kl_dbg(ATH6KL_DBG_WLAN_CFG, "wlan disabled\n");
return false;
}
--
2.19.1
^ permalink raw reply related
* Re: [PATCH net-next v3 6/6] net/ncsi: Configure multi-package, multi-channel modes with failover
From: Samuel Mendoza-Jonas @ 2018-11-09 2:17 UTC (permalink / raw)
To: Justin.Lee1, netdev; +Cc: davem, linux-kernel, openbmc
In-Reply-To: <f9d0309dafe248a6aac8198f96f9bc4b@AUSX13MPS306.AMER.DELL.COM>
On Thu, 2018-11-08 at 22:48 +0000, Justin.Lee1@Dell.com wrote:
> Hi Samuel,
>
> For multi-package and multi-channel case, channel seems to be select correctly. Expect that,
> I still see the timing issue for back-to-back netlink command. Due to that, channel might be
> set to invisible state. Please refer to ncsi0 and ncsi2 below. The channel state is set to 3.
>
> cat /sys/kernel/debug/ncsi_protocol/ncsi_device_status
> IFIDX IFNAME NAME PID CID RX TX MP MC WP WC PC CS PS LS RU CR NQ HA
> =====================================================================
> 2 eth2 ncsi0 000 000 1 1 1 1 1 0 0 3 0 1 1 1 0 1
> 2 eth2 ncsi1 000 001 0 0 1 1 1 0 0 1 0 1 1 1 0 1
> 2 eth2 ncsi2 001 000 1 0 1 1 1 1 0 3 0 1 1 1 0 1
> 2 eth2 ncsi3 001 001 1 0 1 1 1 1 0 2 1 1 1 1 0 1
> =====================================================================
> MP: Multi-mode Package WP: Whitelist Package
> MC: Multi-mode Channel WC: Whitelist Channel
> PC: Primary Channel CS: Channel State IA/A/IV 1/2/3
> PS: Poll Status LS: Link Status
> RU: Running CR: Carrier OK
> NQ: Queue Stopped HA: Hardware Arbitration
>
> The timing issue is not only happening in application. If I use using the following way
> to send the request, I can see the issue as well.
>
> ncsi_netlink -l 2 -a 0x01 -m; ncsi_netlink -l 2 -p 0 -b 0x03 -m; ncsi_netlink -l 2 -p 1 -b 0x00 -m;
> ncsi_netlink -l 2 -a 0x03 -m; ncsi_netlink -l 2 -p 0 -b 0x00 -m; ncsi_netlink -l 2 -p 1 -b 0x03 -m;
This actually recreates for me as well; I see now what you mean about
channels getting stuck in the invisible state. I believe I've narrowed
down the issue. I've pasted an additional patch below if you are able to
test on your machine.
>
>
> Also, there is one issue below for non-multi-package/non-multi-channel case.
>
> Thanks,
> Justin
>
>
> > @@ -1008,32 +1164,49 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
> >
> > ncm = &nc->modes[NCSI_MODE_LINK];
> > if (ncm->data[2] & 0x1) {
> > - spin_unlock_irqrestore(&nc->lock, flags);
> > found = nc;
> > - goto out;
> > + with_link = true;
> > }
> >
> > - spin_unlock_irqrestore(&nc->lock, flags);
> > + /* If multi_channel is enabled configure all valid
> > + * channels whether or not they currently have link
> > + * so they will have AENs enabled.
> > + */
> > + if (with_link || np->multi_channel) {
> > + spin_lock_irqsave(&ndp->lock, flags);
> > + list_add_tail_rcu(&nc->link,
> > + &ndp->channel_queue);
> > + spin_unlock_irqrestore(&ndp->lock, flags);
> > +
> > + netdev_dbg(ndp->ndev.dev,
> > + "NCSI: Channel %u added to queue (link %s)\n",
> > + nc->id,
> > + ncm->data[2] & 0x1 ? "up" : "down");
> > + }
> > +
> > + spin_unlock_irqrestore(&nc->lock, cflags);
> > +
> > + if (with_link && !np->multi_channel)
> > + break;
>
> The line needs to change to "goto found". If not, all channels with link will be added
> even if the multi-channel is not enabled for that package. The ncsi1 below is enabled.
> There is no netlink command sent to enable multi-package or multi-channel.
>
> IFIDX IFNAME NAME PID CID RX TX MP MC WP WC PC CS PS LS RU CR NQ HA
> =====================================================================
> 2 eth2 ncsi0 000 000 1 1 0 0 1 1 0 2 1 1 1 1 0 1
> 2 eth2 ncsi1 000 001 1 0 0 0 1 1 0 2 1 1 1 1 0 1
> 2 eth2 ncsi2 001 000 0 0 0 0 1 1 0 1 0 1 1 1 0 1
> 2 eth2 ncsi3 001 001 0 0 0 0 1 1 0 1 0 1 1 1 0 1
> =====================================================================
> MP: Multi-mode Package WP: Whitelist Package
> MC: Multi-mode Channel WC: Whitelist Channel
> PC: Primary Channel CS: Channel State IA/A/IV 1/2/3
> PS: Poll Status LS: Link Status
> RU: Running CR: Carrier OK
> NQ: Queue Stopped HA: Hardware Arbitration
>
> > }
> > + if (with_link && !ndp->multi_package)
> > + break;
> > }
>
> found:
This *may* be part of the above issue, I don't see this in normal
operation. The combination of (with_link && !np->multi_channel) and
(with_link && !ndp->multi_package) should prevent additional channels
being added without the need for 'goto found'. Please let me know if you
still see it with the extra patch.
>
> After applying this change, I notice that if there is no link available to BMC when BMC
> starts, NC-SI can't properly configure channel once I plug in the Ethernet cable.
>
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_aen_handler_lsc() - pkg 0 ch 0 state up
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_aen_handler_lsc() - had_link 0, has_link 1, chained 0
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_stop_channel_monitor() - pkg 0 ch 0
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_process_next_channel()
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_process_next_channel() - pkg 0 ch 0 INVISIBLE
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_process_next_channel() - suspending pkg 0 ch 0
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_suspend_channel() - pkg 0 ch 0 state 0400 select
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_dev_work()
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_suspend_channel() - pkg 0 ch 0 state 0403 dc
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_dev_work()
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_suspend_channel() - pkg 0 ch 0 state 0404 deselect
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_dev_work()
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_suspend_channel() - pkg 0 ch 0 state 0405 done
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_rsp_handler_dp() - pkg 0 ch 0 INACTIVE
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_rsp_handler_dp() - pkg 0 ch 1 INACTIVE
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_dev_work()
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_suspend_channel() - pkg 0 ch 0 state 0406 deselect
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_suspend_channel() - pkg 0 ch 0 INACTIVE
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_process_next_channel()
> npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_process_next_channel() - No more channels to process
> npcm7xx-emc f0825000.eth eth2: NCSI interface down
Good find, there was a corner case in the LSC AEN handler changes that
led to this, I've fixed this in the patch as well. Thanks for testing!
>From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Date: Fri, 9 Nov 2018 13:11:03 +1100
Subject: [PATCH] net/ncsi: Reset state fixes, single-channel LSC
---
net/ncsi/ncsi-aen.c | 8 +++++---
net/ncsi/ncsi-manage.c | 19 +++++++++++++++----
2 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index 39c2e9eea2ba..034cb1dc5566 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -93,14 +93,16 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp,
if ((had_link == has_link) || chained)
return 0;
- if (!ndp->multi_package && !nc->package->multi_channel) {
- if (had_link)
- ndp->flags |= NCSI_DEV_RESHUFFLE;
+ if (!ndp->multi_package && !nc->package->multi_channel && had_link) {
+ ndp->flags |= NCSI_DEV_RESHUFFLE;
ncsi_stop_channel_monitor(nc);
spin_lock_irqsave(&ndp->lock, flags);
list_add_tail_rcu(&nc->link, &ndp->channel_queue);
spin_unlock_irqrestore(&ndp->lock, flags);
return ncsi_process_next_channel(ndp);
+ } else {
+ /* Configured channel came up */
+ return 0;
}
if (had_link) {
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index fa3c2144f5ba..92e59f07f9a7 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1063,17 +1063,17 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
case ncsi_dev_state_config_done:
netdev_dbg(ndp->ndev.dev, "NCSI: channel %u config done\n",
nc->id);
+ spin_lock_irqsave(&nc->lock, flags);
+ nc->state = NCSI_CHANNEL_ACTIVE;
+
if (ndp->flags & NCSI_DEV_RESET) {
/* A reset event happened during config, start it now */
- spin_lock_irqsave(&nc->lock, flags);
nc->reconfigure_needed = false;
spin_unlock_irqrestore(&nc->lock, flags);
- nd->state = ncsi_dev_state_functional;
ncsi_reset_dev(nd);
break;
}
- spin_lock_irqsave(&nc->lock, flags);
if (nc->reconfigure_needed) {
/* This channel's configuration has been updated
* part-way during the config state - start the
@@ -1092,7 +1092,6 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
break;
}
- nc->state = NCSI_CHANNEL_ACTIVE;
if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) {
hot_nc = nc;
} else {
@@ -1803,6 +1802,18 @@ int ncsi_reset_dev(struct ncsi_dev *nd)
spin_unlock_irqrestore(&ndp->lock, flags);
return 0;
}
+ } else {
+ switch (nd->state) {
+ case ncsi_dev_state_suspend_done:
+ case ncsi_dev_state_config_done:
+ case ncsi_dev_state_functional:
+ /* Ok */
+ break;
+ default:
+ /* Current reset operation happening */
+ spin_unlock_irqrestore(&ndp->lock, flags);
+ return 0;
+ }
}
if (!list_empty(&ndp->channel_queue)) {
--
2.19.1
^ permalink raw reply related
* Re: [PATCH net-next v2 3/5] virtio_ring: add packed ring support
From: Jason Wang @ 2018-11-09 2:25 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Tiwei Bie, virtualization, linux-kernel, netdev, virtio-dev, wexu,
jfreimann
In-Reply-To: <20181108091337-mutt-send-email-mst@kernel.org>
On 2018/11/8 下午10:14, Michael S. Tsirkin wrote:
> On Thu, Nov 08, 2018 at 04:18:25PM +0800, Jason Wang wrote:
>> On 2018/11/8 上午9:38, Tiwei Bie wrote:
>>>>> +
>>>>> + if (vq->vq.num_free < descs_used) {
>>>>> + pr_debug("Can't add buf len %i - avail = %i\n",
>>>>> + descs_used, vq->vq.num_free);
>>>>> + /* FIXME: for historical reasons, we force a notify here if
>>>>> + * there are outgoing parts to the buffer. Presumably the
>>>>> + * host should service the ring ASAP. */
>>>> I don't think we have a reason to do this for packed ring.
>>>> No historical baggage there, right?
>>> Based on the original commit log, it seems that the notify here
>>> is just an "optimization". But I don't quite understand what does
>>> the "the heuristics which KVM uses" refer to. If it's safe to drop
>>> this in packed ring, I'd like to do it.
>>
>> According to the commit log, it seems like a workaround of lguest networking
>> backend. I agree to drop it, we should not have such burden.
>>
>> But we should notice that, with this removed, the compare between packed vs
>> split is kind of unfair.
> I don't think this ever triggers to be frank. When would it?
I think it can happen e.g in the path of XDP transmission in
__virtnet_xdp_xmit_one():
err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdpf, GFP_ATOMIC);
if (unlikely(err))
return -ENOSPC; /* Caller handle free/refcnt */
>
>> Consider the removal of lguest support recently,
>> maybe we can drop this for split ring as well?
>>
>> Thanks
> If it's helpful, then for sure we can drop it for virtio 1.
> Can you see any perf differences at all? With which device?
I don't test but consider the case of XDP_TX in guest plus vhost_net in
host. Since vhost_net is half duplex, it's pretty easier to trigger this
condition.
Thanks
>
>>> commit 44653eae1407f79dff6f52fcf594ae84cb165ec4
>>> Author: Rusty Russell<rusty@rustcorp.com.au>
>>> Date: Fri Jul 25 12:06:04 2008 -0500
>>>
>>> virtio: don't always force a notification when ring is full
>>> We force notification when the ring is full, even if the host has
>>> indicated it doesn't want to know. This seemed like a good idea at
>>> the time: if we fill the transmit ring, we should tell the host
>>> immediately.
>>> Unfortunately this logic also applies to the receiving ring, which is
>>> refilled constantly. We should introduce real notification thesholds
>>> to replace this logic. Meanwhile, removing the logic altogether breaks
>>> the heuristics which KVM uses, so we use a hack: only notify if there are
>>> outgoing parts of the new buffer.
>>> Here are the number of exits with lguest's crappy network implementation:
>>> Before:
>>> network xmit 7859051 recv 236420
>>> After:
>>> network xmit 7858610 recv 118136
>>> Signed-off-by: Rusty Russell<rusty@rustcorp.com.au>
>>>
>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>> index 72bf8bc09014..21d9a62767af 100644
>>> --- a/drivers/virtio/virtio_ring.c
>>> +++ b/drivers/virtio/virtio_ring.c
>>> @@ -87,8 +87,11 @@ static int vring_add_buf(struct virtqueue *_vq,
>>> if (vq->num_free < out + in) {
>>> pr_debug("Can't add buf len %i - avail = %i\n",
>>> out + in, vq->num_free);
>>> - /* We notify*even if* VRING_USED_F_NO_NOTIFY is set here. */
>>> - vq->notify(&vq->vq);
>>> + /* FIXME: for historical reasons, we force a notify here if
>>> + * there are outgoing parts to the buffer. Presumably the
>>> + * host should service the ring ASAP. */
>>> + if (out)
>>> + vq->notify(&vq->vq);
>>> END_USE(vq);
>>> return -ENOSPC;
>>> }
>>>
>>>
^ permalink raw reply
* Re: [PATCH net-next v2 3/5] virtio_ring: add packed ring support
From: Jason Wang @ 2018-11-09 2:30 UTC (permalink / raw)
To: Michael S. Tsirkin, Tiwei Bie
Cc: virtualization, linux-kernel, netdev, virtio-dev, wexu, jfreimann
In-Reply-To: <20181108103155-mutt-send-email-mst@kernel.org>
On 2018/11/8 下午11:56, Michael S. Tsirkin wrote:
> On Thu, Nov 08, 2018 at 07:51:48PM +0800, Tiwei Bie wrote:
>> On Thu, Nov 08, 2018 at 04:18:25PM +0800, Jason Wang wrote:
>>> On 2018/11/8 上午9:38, Tiwei Bie wrote:
>>>>>> +
>>>>>> + if (vq->vq.num_free < descs_used) {
>>>>>> + pr_debug("Can't add buf len %i - avail = %i\n",
>>>>>> + descs_used, vq->vq.num_free);
>>>>>> + /* FIXME: for historical reasons, we force a notify here if
>>>>>> + * there are outgoing parts to the buffer. Presumably the
>>>>>> + * host should service the ring ASAP. */
>>>>> I don't think we have a reason to do this for packed ring.
>>>>> No historical baggage there, right?
>>>> Based on the original commit log, it seems that the notify here
>>>> is just an "optimization". But I don't quite understand what does
>>>> the "the heuristics which KVM uses" refer to. If it's safe to drop
>>>> this in packed ring, I'd like to do it.
>>>
>>> According to the commit log, it seems like a workaround of lguest networking
>>> backend.
>> Do you know why removing this notify in Tx will break "the
>> heuristics which KVM uses"? Or what does "the heuristics
>> which KVM uses" refer to?
> Yes. QEMU has a mode where it disables notifications and processes TX
> ring periodically from a timer. It's off by default but used to be on
> by default a long time ago. If ring becomes full this causes traffic
> stalls.
Do you mean tx-timer? If yes, we can still enable it for packed ring and
the timer will finally fired and we can go.
> As a work-around Rusty put in this hack to kick on ring full
> even with notifications disabled.
From the commit log it looks more like a performance workaround instead
of a bug fix.
> It's easy enough to make sure QEMU
> does not combine devices with packed ring support with the timer hack.
> And I am guessing it's safe enough to also block that option completely
> e.g. when virtio 1.0 is enabled.
I agree.
Thanks
>>> I agree to drop it, we should not have such burden.
>>>
>>> But we should notice that, with this removed, the compare between packed vs
>>> split is kind of unfair. Consider the removal of lguest support recently,
>>> maybe we can drop this for split ring as well?
>>>
>>> Thanks
>>>
>>>
>>>> commit 44653eae1407f79dff6f52fcf594ae84cb165ec4
>>>> Author: Rusty Russell<rusty@rustcorp.com.au>
>>>> Date: Fri Jul 25 12:06:04 2008 -0500
>>>>
>>>> virtio: don't always force a notification when ring is full
>>>> We force notification when the ring is full, even if the host has
>>>> indicated it doesn't want to know. This seemed like a good idea at
>>>> the time: if we fill the transmit ring, we should tell the host
>>>> immediately.
>>>> Unfortunately this logic also applies to the receiving ring, which is
>>>> refilled constantly. We should introduce real notification thesholds
>>>> to replace this logic. Meanwhile, removing the logic altogether breaks
>>>> the heuristics which KVM uses, so we use a hack: only notify if there are
>>>> outgoing parts of the new buffer.
>>>> Here are the number of exits with lguest's crappy network implementation:
>>>> Before:
>>>> network xmit 7859051 recv 236420
>>>> After:
>>>> network xmit 7858610 recv 118136
>>>> Signed-off-by: Rusty Russell<rusty@rustcorp.com.au>
>>>>
>>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>>> index 72bf8bc09014..21d9a62767af 100644
>>>> --- a/drivers/virtio/virtio_ring.c
>>>> +++ b/drivers/virtio/virtio_ring.c
>>>> @@ -87,8 +87,11 @@ static int vring_add_buf(struct virtqueue *_vq,
>>>> if (vq->num_free < out + in) {
>>>> pr_debug("Can't add buf len %i - avail = %i\n",
>>>> out + in, vq->num_free);
>>>> - /* We notify*even if* VRING_USED_F_NO_NOTIFY is set here. */
>>>> - vq->notify(&vq->vq);
>>>> + /* FIXME: for historical reasons, we force a notify here if
>>>> + * there are outgoing parts to the buffer. Presumably the
>>>> + * host should service the ring ASAP. */
>>>> + if (out)
>>>> + vq->notify(&vq->vq);
>>>> END_USE(vq);
>>>> return -ENOSPC;
>>>> }
>>>>
>>>>
^ permalink raw reply
* [PATCH bpf-next 2/4] bpf: Split bpf_sk_lookup
From: Andrey Ignatov @ 2018-11-08 16:54 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, joe, kernel-team
In-Reply-To: <cover.1541695683.git.rdna@fb.com>
Split bpf_sk_lookup to separate core functionality, that can be reused
to make socket lookup available to more program types, from
functionality specific to program types that have access to skb.
Core functionality is placed to __bpf_sk_lookup. And bpf_sk_lookup only
gets caller netns and ifindex from skb and passes it to __bpf_sk_lookup.
Program types that don't have access to skb can just pass NULL to
__bpf_sk_lookup that will be handled correctly by both inet{,6}_sdif and
lookup functions.
This is refactoring that simply moves blocks around and does NOT change
existing logic.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
net/core/filter.c | 38 +++++++++++++++++++++++---------------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index 9a1327eb25fa..dc0f86a707b7 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4825,14 +4825,10 @@ static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
#ifdef CONFIG_INET
static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
- struct sk_buff *skb, u8 family, u8 proto)
+ struct sk_buff *skb, u8 family, u8 proto, int dif)
{
bool refcounted = false;
struct sock *sk = NULL;
- int dif = 0;
-
- if (skb->dev)
- dif = skb->dev->ifindex;
if (family == AF_INET) {
__be32 src4 = tuple->ipv4.saddr;
@@ -4875,16 +4871,16 @@ static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
return sk;
}
-/* bpf_sk_lookup performs the core lookup for different types of sockets,
+/* __bpf_sk_lookup performs the core lookup for different types of sockets,
* taking a reference on the socket if it doesn't have the flag SOCK_RCU_FREE.
* Returns the socket as an 'unsigned long' to simplify the casting in the
* callers to satisfy BPF_CALL declarations.
*/
static unsigned long
-bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
- u8 proto, u64 netns_id, u64 flags)
+__bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
+ u8 proto, u64 netns_id, struct net *caller_net, int ifindex,
+ u64 flags)
{
- struct net *caller_net;
struct sock *sk = NULL;
u8 family = AF_UNSPEC;
struct net *net;
@@ -4893,19 +4889,15 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
if (unlikely(family == AF_UNSPEC || netns_id > U32_MAX || flags))
goto out;
- if (skb->dev)
- caller_net = dev_net(skb->dev);
- else
- caller_net = sock_net(skb->sk);
if (netns_id) {
net = get_net_ns_by_id(caller_net, netns_id);
if (unlikely(!net))
goto out;
- sk = sk_lookup(net, tuple, skb, family, proto);
+ sk = sk_lookup(net, tuple, skb, family, proto, ifindex);
put_net(net);
} else {
net = caller_net;
- sk = sk_lookup(net, tuple, skb, family, proto);
+ sk = sk_lookup(net, tuple, skb, family, proto, ifindex);
}
if (sk)
@@ -4914,6 +4906,22 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
return (unsigned long) sk;
}
+static unsigned long
+bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
+ u8 proto, u64 netns_id, u64 flags)
+{
+ struct net *caller_net = sock_net(skb->sk);
+ int ifindex = 0;
+
+ if (skb->dev) {
+ caller_net = dev_net(skb->dev);
+ ifindex = skb->dev->ifindex;
+ }
+
+ return __bpf_sk_lookup(skb, tuple, len, proto, netns_id, caller_net,
+ ifindex, flags);
+}
+
BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
{
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 0/4] bpf: Support socket lookup in CGROUP_SOCK_ADDR progs
From: Andrey Ignatov @ 2018-11-08 16:54 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, joe, kernel-team
This patch set makes bpf_sk_lookup_tcp, bpf_sk_lookup_udp and
bpf_sk_release helpers available in programs of type
BPF_PROG_TYPE_CGROUP_SOCK_ADDR.
Patch 1 is a fix for bpf_sk_lookup_udp that was already sent to netdev
separately for bpf (stable) tree. Here it's prerequisite for patch 4.
Patch 2 is refactoring to prepare for patch 3. Similar refactoring was done
as part of "bpf: Extend the sk_lookup() helper to XDP hookpoint." patch
published on netdev earlier but not merged yet. This patch set can reuse
the work done for xdp if it's merged. This patch doesn't make any logic
changes and simply moves code around.
Patch 3 is the main patch in the set, it makes the helpers available for
BPF_PROG_TYPE_CGROUP_SOCK_ADDR and provides more details about use-case.
Patch 4 adds selftest for new functionality.
Andrey Ignatov (4):
bpf: Fix IPv6 dport byte order in bpf_sk_lookup_udp
bpf: Split bpf_sk_lookup
bpf: Support socket lookup in CGROUP_SOCK_ADDR progs
selftest/bpf: Use bpf_sk_lookup_{tcp,udp} in test_sock_addr
net/core/filter.c | 96 +++++++++++++++++----
tools/testing/selftests/bpf/connect4_prog.c | 43 +++++++--
tools/testing/selftests/bpf/connect6_prog.c | 56 +++++++++---
3 files changed, 156 insertions(+), 39 deletions(-)
--
2.17.1
^ 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