* Re: RFC on writel and writel_relaxed
From: okaya @ 2018-03-28 11:41 UTC (permalink / raw)
To: Linus Torvalds
Cc: Benjamin Herrenschmidt, Alexander Duyck, Will Deacon,
Arnd Bergmann, Jason Gunthorpe, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), linux-rdma,
Alexander Duyck, Paul E. McKenney, netdev, linus971
In-Reply-To: <CA+55aFxULkhZGhg-tTBBxBBvXbL7QV7f6zvwW7U2wHFijqGEqw@mail.gmail.com>
On 2018-03-28 02:14, Linus Torvalds wrote:
> On Tue, Mar 27, 2018 at 5:24 PM, Sinan Kaya <okaya@codeaurora.org>
> wrote:
>>
>> Basically changing it to
>>
>> dma_buffer->foo = 1; /* WB */
>> wmb()
>> writel_relaxed(KICK, DMA_KICK_REGISTER); /* UC */
>> mmiowb()
>
> Why?
>
> Why not just remove the wmb(), and keep the barrier in the writel()?
Yes, we want to get there indeed. It is because of some arch not
implementing writel properly. Maintainers want to play safe.
That is why I asked if IA64 and other well known archs follow the
strongly ordered rule at this moment like PPC and ARM.
Or should we go and inform every arch about this before yanking wmb()?
Maintainers are afraid of introducing a regression.
>
> The above code makes no sense, and just looks stupid to me. It also
> generates pointlessly bad code on x86, so it's bad there too.
>
> Linus
^ permalink raw reply
* [PATCH] test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()
From: Dan Carpenter @ 2018-03-28 11:48 UTC (permalink / raw)
To: Alexei Starovoitov, Yonghong Song
Cc: Daniel Borkmann, netdev, kernel-janitors
The skb_segment() function returns error pointers on error. It never
returns NULL.
Fixes: 76db8087c4c9 ("net: bpf: add a test for skb_segment in test_bpf module")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index b2badf6b23cd..8e157806df7a 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -6649,7 +6649,7 @@ static __init int test_skb_segment(void)
}
segs = skb_segment(skb, features);
- if (segs) {
+ if (!IS_ERR(segs)) {
kfree_skb_list(segs);
ret = 0;
pr_info("%s: success in skb_segment!", __func__);
^ permalink raw reply related
* Re: [iproute PATCH] man: ip-route.8: ssthresh parameter is NUMBER
From: Simon Horman @ 2018-03-28 11:52 UTC (permalink / raw)
To: Phil Sutter; +Cc: Stephen Hemminger, netdev
In-Reply-To: <20180322140038.21369-1-phil@nwl.cc>
On Thu, Mar 22, 2018 at 03:00:38PM +0100, Phil Sutter wrote:
> Synopsis section was inconsistent with regards to help text and later
> description of ssthresh parameter.
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
^ permalink raw reply
* ipv6: stable request
From: Denys Zagorui @ 2018-03-28 11:45 UTC (permalink / raw)
To: netdev; +Cc: edumazet, davem
12d94a804946af291e24b80fc53ec86264765781
<https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?h=v4.14.29&id=12d94a804946af291e24b80fc53ec86264765781>
ipv6: fix NULL dereference in ip6_route_dev_notify()
This BUG touches 4.1 4.4 4.9.
^ permalink raw reply
* [PATCH net-next v3 1/5] qed: Populate nvm image attribute shadow.
From: Sudarsana Reddy Kalluru @ 2018-03-28 12:14 UTC (permalink / raw)
To: davem; +Cc: netdev, Ariel.Elior, yuvalm
In-Reply-To: <20180328121423.7718-1-sudarsana.kalluru@cavium.com>
This patch adds support for populating the flash image attributes.
Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
Signed-off-by: Ariel Elior <ariel.elior@cavium.com>
---
drivers/net/ethernet/qlogic/qed/qed.h | 8 +++
drivers/net/ethernet/qlogic/qed/qed_dev.c | 24 ++++++-
drivers/net/ethernet/qlogic/qed/qed_mcp.c | 98 +++++++++++++++++++-------
drivers/net/ethernet/qlogic/qed/qed_mcp.h | 22 ++++--
drivers/net/ethernet/qlogic/qed/qed_selftest.c | 6 +-
5 files changed, 120 insertions(+), 38 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed.h b/drivers/net/ethernet/qlogic/qed/qed.h
index 6948855..b409499 100644
--- a/drivers/net/ethernet/qlogic/qed/qed.h
+++ b/drivers/net/ethernet/qlogic/qed/qed.h
@@ -437,6 +437,11 @@ enum BAR_ID {
BAR_ID_1 /* Used for doorbells */
};
+struct qed_nvm_image_info {
+ u32 num_images;
+ struct bist_nvm_image_att *image_att;
+};
+
#define DRV_MODULE_VERSION \
__stringify(QED_MAJOR_VERSION) "." \
__stringify(QED_MINOR_VERSION) "." \
@@ -561,6 +566,9 @@ struct qed_hwfn {
/* L2-related */
struct qed_l2_info *p_l2_info;
+ /* Nvm images number and attributes */
+ struct qed_nvm_image_info nvm_info;
+
struct qed_ptt *p_arfs_ptt;
struct qed_simd_fp_handler simd_proto_handler[64];
diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c
index 553a6d1..7ddec55 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
@@ -2930,6 +2930,12 @@ static int qed_get_dev_info(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
return 0;
}
+static void qed_nvm_info_free(struct qed_hwfn *p_hwfn)
+{
+ kfree(p_hwfn->nvm_info.image_att);
+ p_hwfn->nvm_info.image_att = NULL;
+}
+
static int qed_hw_prepare_single(struct qed_hwfn *p_hwfn,
void __iomem *p_regview,
void __iomem *p_doorbells,
@@ -2993,12 +2999,25 @@ static int qed_hw_prepare_single(struct qed_hwfn *p_hwfn,
DP_NOTICE(p_hwfn, "Failed to initiate PF FLR\n");
}
+ /* NVRAM info initialization and population */
+ if (IS_LEAD_HWFN(p_hwfn)) {
+ rc = qed_mcp_nvm_info_populate(p_hwfn);
+ if (rc) {
+ DP_NOTICE(p_hwfn,
+ "Failed to populate nvm info shadow\n");
+ goto err2;
+ }
+ }
+
/* Allocate the init RT array and initialize the init-ops engine */
rc = qed_init_alloc(p_hwfn);
if (rc)
- goto err2;
+ goto err3;
return rc;
+err3:
+ if (IS_LEAD_HWFN(p_hwfn))
+ qed_nvm_info_free(p_hwfn);
err2:
if (IS_LEAD_HWFN(p_hwfn))
qed_iov_free_hw_info(p_hwfn->cdev);
@@ -3054,6 +3073,7 @@ int qed_hw_prepare(struct qed_dev *cdev,
if (rc) {
if (IS_PF(cdev)) {
qed_init_free(p_hwfn);
+ qed_nvm_info_free(p_hwfn);
qed_mcp_free(p_hwfn);
qed_hw_hwfn_free(p_hwfn);
}
@@ -3086,6 +3106,8 @@ void qed_hw_remove(struct qed_dev *cdev)
}
qed_iov_free_hw_info(cdev);
+
+ qed_nvm_info_free(p_hwfn);
}
static void qed_chain_free_next_ptr(struct qed_dev *cdev,
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
index 6f46cb1..2519e71 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
@@ -2303,9 +2303,9 @@ int qed_mcp_bist_clock_test(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
return rc;
}
-int qed_mcp_bist_nvm_test_get_num_images(struct qed_hwfn *p_hwfn,
- struct qed_ptt *p_ptt,
- u32 *num_images)
+int qed_mcp_bist_nvm_get_num_images(struct qed_hwfn *p_hwfn,
+ struct qed_ptt *p_ptt,
+ u32 *num_images)
{
u32 drv_mb_param = 0, rsp;
int rc = 0;
@@ -2324,10 +2324,10 @@ int qed_mcp_bist_nvm_test_get_num_images(struct qed_hwfn *p_hwfn,
return rc;
}
-int qed_mcp_bist_nvm_test_get_image_att(struct qed_hwfn *p_hwfn,
- struct qed_ptt *p_ptt,
- struct bist_nvm_image_att *p_image_att,
- u32 image_index)
+int qed_mcp_bist_nvm_get_image_att(struct qed_hwfn *p_hwfn,
+ struct qed_ptt *p_ptt,
+ struct bist_nvm_image_att *p_image_att,
+ u32 image_index)
{
u32 buf_size = 0, param, resp = 0, resp_param = 0;
int rc;
@@ -2351,16 +2351,71 @@ int qed_mcp_bist_nvm_test_get_image_att(struct qed_hwfn *p_hwfn,
return rc;
}
+int qed_mcp_nvm_info_populate(struct qed_hwfn *p_hwfn)
+{
+ struct qed_nvm_image_info *nvm_info = &p_hwfn->nvm_info;
+ struct qed_ptt *p_ptt;
+ int rc;
+ u32 i;
+
+ p_ptt = qed_ptt_acquire(p_hwfn);
+ if (!p_ptt) {
+ DP_ERR(p_hwfn, "failed to acquire ptt\n");
+ return -EBUSY;
+ }
+
+ /* Acquire from MFW the amount of available images */
+ nvm_info->num_images = 0;
+ rc = qed_mcp_bist_nvm_get_num_images(p_hwfn,
+ p_ptt, &nvm_info->num_images);
+ if (rc == -EOPNOTSUPP) {
+ DP_INFO(p_hwfn, "DRV_MSG_CODE_BIST_TEST is not supported\n");
+ goto out;
+ } else if (rc || !nvm_info->num_images) {
+ DP_ERR(p_hwfn, "Failed getting number of images\n");
+ goto err0;
+ }
+
+ nvm_info->image_att = kmalloc(nvm_info->num_images *
+ sizeof(struct bist_nvm_image_att),
+ GFP_KERNEL);
+ if (!nvm_info->image_att) {
+ rc = -ENOMEM;
+ goto err0;
+ }
+
+ /* Iterate over images and get their attributes */
+ for (i = 0; i < nvm_info->num_images; i++) {
+ rc = qed_mcp_bist_nvm_get_image_att(p_hwfn, p_ptt,
+ &nvm_info->image_att[i], i);
+ if (rc) {
+ DP_ERR(p_hwfn,
+ "Failed getting image index %d attributes\n", i);
+ goto err1;
+ }
+
+ DP_VERBOSE(p_hwfn, QED_MSG_SP, "image index %d, size %x\n", i,
+ nvm_info->image_att[i].len);
+ }
+out:
+ qed_ptt_release(p_hwfn, p_ptt);
+ return 0;
+
+err1:
+ kfree(nvm_info->image_att);
+err0:
+ qed_ptt_release(p_hwfn, p_ptt);
+ return rc;
+}
+
static int
qed_mcp_get_nvm_image_att(struct qed_hwfn *p_hwfn,
struct qed_ptt *p_ptt,
enum qed_nvm_images image_id,
struct qed_nvm_image_att *p_image_att)
{
- struct bist_nvm_image_att mfw_image_att;
enum nvm_image_type type;
- u32 num_images, i;
- int rc;
+ u32 i;
/* Translate image_id into MFW definitions */
switch (image_id) {
@@ -2376,29 +2431,18 @@ int qed_mcp_bist_nvm_test_get_image_att(struct qed_hwfn *p_hwfn,
return -EINVAL;
}
- /* Learn number of images, then traverse and see if one fits */
- rc = qed_mcp_bist_nvm_test_get_num_images(p_hwfn, p_ptt, &num_images);
- if (rc || !num_images)
- return -EINVAL;
-
- for (i = 0; i < num_images; i++) {
- rc = qed_mcp_bist_nvm_test_get_image_att(p_hwfn, p_ptt,
- &mfw_image_att, i);
- if (rc)
- return rc;
-
- if (type == mfw_image_att.image_type)
+ for (i = 0; i < p_hwfn->nvm_info.num_images; i++)
+ if (type == p_hwfn->nvm_info.image_att[i].image_type)
break;
- }
- if (i == num_images) {
+ if (i == p_hwfn->nvm_info.num_images) {
DP_VERBOSE(p_hwfn, QED_MSG_STORAGE,
"Failed to find nvram image of type %08x\n",
image_id);
- return -EINVAL;
+ return -ENOENT;
}
- p_image_att->start_addr = mfw_image_att.nvm_start_addr;
- p_image_att->length = mfw_image_att.len;
+ p_image_att->start_addr = p_hwfn->nvm_info.image_att[i].nvm_start_addr;
+ p_image_att->length = p_hwfn->nvm_info.image_att[i].len;
return 0;
}
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.h b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
index c7ec239..7d33354 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
@@ -496,9 +496,9 @@ int qed_mcp_bist_clock_test(struct qed_hwfn *p_hwfn,
*
* @return int - 0 - operation was successful.
*/
-int qed_mcp_bist_nvm_test_get_num_images(struct qed_hwfn *p_hwfn,
- struct qed_ptt *p_ptt,
- u32 *num_images);
+int qed_mcp_bist_nvm_get_num_images(struct qed_hwfn *p_hwfn,
+ struct qed_ptt *p_ptt,
+ u32 *num_images);
/**
* @brief Bist nvm test - get image attributes by index
@@ -510,10 +510,10 @@ int qed_mcp_bist_nvm_test_get_num_images(struct qed_hwfn *p_hwfn,
*
* @return int - 0 - operation was successful.
*/
-int qed_mcp_bist_nvm_test_get_image_att(struct qed_hwfn *p_hwfn,
- struct qed_ptt *p_ptt,
- struct bist_nvm_image_att *p_image_att,
- u32 image_index);
+int qed_mcp_bist_nvm_get_image_att(struct qed_hwfn *p_hwfn,
+ struct qed_ptt *p_ptt,
+ struct bist_nvm_image_att *p_image_att,
+ u32 image_index);
/* Using hwfn number (and not pf_num) is required since in CMT mode,
* same pf_num may be used by two different hwfn
@@ -957,4 +957,12 @@ void qed_mcp_resc_lock_default_init(struct qed_resc_lock_params *p_lock,
* @param p_ptt
*/
int qed_mcp_set_capabilities(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt);
+
+/**
+ * @brief Populate the nvm info shadow in the given hardware function
+ *
+ * @param p_hwfn
+ */
+int qed_mcp_nvm_info_populate(struct qed_hwfn *p_hwfn);
+
#endif
diff --git a/drivers/net/ethernet/qlogic/qed/qed_selftest.c b/drivers/net/ethernet/qlogic/qed/qed_selftest.c
index 1bafc05..b88082f 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_selftest.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_selftest.c
@@ -125,7 +125,7 @@ int qed_selftest_nvram(struct qed_dev *cdev)
}
/* Acquire from MFW the amount of available images */
- rc = qed_mcp_bist_nvm_test_get_num_images(p_hwfn, p_ptt, &num_images);
+ rc = qed_mcp_bist_nvm_get_num_images(p_hwfn, p_ptt, &num_images);
if (rc || !num_images) {
DP_ERR(p_hwfn, "Failed getting number of images\n");
return -EINVAL;
@@ -136,8 +136,8 @@ int qed_selftest_nvram(struct qed_dev *cdev)
/* This mailbox returns information about the image required for
* reading it.
*/
- rc = qed_mcp_bist_nvm_test_get_image_att(p_hwfn, p_ptt,
- &image_att, i);
+ rc = qed_mcp_bist_nvm_get_image_att(p_hwfn, p_ptt,
+ &image_att, i);
if (rc) {
DP_ERR(p_hwfn,
"Failed getting image index %d attributes\n",
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 0/5] qed*: Flash upgrade support.
From: Sudarsana Reddy Kalluru @ 2018-03-28 12:14 UTC (permalink / raw)
To: davem; +Cc: netdev, Ariel.Elior, yuvalm, Sudarsana Reddy Kalluru
From: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
The patch series adds adapter flash upgrade support for qed/qede drivers.
Please consider applying it to net-next branch.
Sudarsana Reddy Kalluru (5):
qed: Populate nvm image attribute shadow.
qed: Fix PTT entry leak in the selftest error flow.
qed: Add APIs for flash access.
qed: Adapter flash update support.
qede: Ethtool flash update support.
drivers/net/ethernet/qlogic/qed/qed.h | 15 ++
drivers/net/ethernet/qlogic/qed/qed_dev.c | 24 +-
drivers/net/ethernet/qlogic/qed/qed_hsi.h | 7 +-
drivers/net/ethernet/qlogic/qed/qed_main.c | 338 ++++++++++++++++++++++++
drivers/net/ethernet/qlogic/qed/qed_mcp.c | 219 +++++++++++++--
drivers/net/ethernet/qlogic/qed/qed_mcp.h | 56 +++-
drivers/net/ethernet/qlogic/qed/qed_selftest.c | 9 +-
drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 9 +
include/linux/qed/qed_if.h | 19 ++
9 files changed, 656 insertions(+), 40 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH net-next v3 2/5] qed: Fix PTT entry leak in the selftest error flow.
From: Sudarsana Reddy Kalluru @ 2018-03-28 12:14 UTC (permalink / raw)
To: davem; +Cc: netdev, Ariel.Elior, yuvalm
In-Reply-To: <20180328121423.7718-1-sudarsana.kalluru@cavium.com>
Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
Signed-off-by: Ariel Elior <ariel.elior@cavium.com>
---
drivers/net/ethernet/qlogic/qed/qed_selftest.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_selftest.c b/drivers/net/ethernet/qlogic/qed/qed_selftest.c
index b88082f..cf1d447 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_selftest.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_selftest.c
@@ -128,7 +128,8 @@ int qed_selftest_nvram(struct qed_dev *cdev)
rc = qed_mcp_bist_nvm_get_num_images(p_hwfn, p_ptt, &num_images);
if (rc || !num_images) {
DP_ERR(p_hwfn, "Failed getting number of images\n");
- return -EINVAL;
+ rc = -EINVAL;
+ goto err0;
}
/* Iterate over images and validate CRC */
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 3/5] qed: Add APIs for flash access.
From: Sudarsana Reddy Kalluru @ 2018-03-28 12:14 UTC (permalink / raw)
To: davem; +Cc: netdev, Ariel.Elior, yuvalm
In-Reply-To: <20180328121423.7718-1-sudarsana.kalluru@cavium.com>
This patch adds APIs for flash access.
Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
Signed-off-by: Ariel Elior <ariel.elior@cavium.com>
---
drivers/net/ethernet/qlogic/qed/qed.h | 7 ++
drivers/net/ethernet/qlogic/qed/qed_hsi.h | 7 +-
drivers/net/ethernet/qlogic/qed/qed_mcp.c | 121 ++++++++++++++++++++++++++++++
drivers/net/ethernet/qlogic/qed/qed_mcp.h | 34 +++++++++
4 files changed, 168 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed.h b/drivers/net/ethernet/qlogic/qed/qed.h
index b409499..e07460a 100644
--- a/drivers/net/ethernet/qlogic/qed/qed.h
+++ b/drivers/net/ethernet/qlogic/qed/qed.h
@@ -81,6 +81,13 @@ enum qed_coalescing_mode {
QED_COAL_MODE_ENABLE
};
+enum qed_nvm_cmd {
+ QED_PUT_FILE_BEGIN = DRV_MSG_CODE_NVM_PUT_FILE_BEGIN,
+ QED_PUT_FILE_DATA = DRV_MSG_CODE_NVM_PUT_FILE_DATA,
+ QED_NVM_WRITE_NVRAM = DRV_MSG_CODE_NVM_WRITE_NVRAM,
+ QED_GET_MCP_NVM_RESP = 0xFFFFFF00
+};
+
struct qed_eth_cb_ops;
struct qed_dev_info;
union qed_mcp_protocol_stats;
diff --git a/drivers/net/ethernet/qlogic/qed/qed_hsi.h b/drivers/net/ethernet/qlogic/qed/qed_hsi.h
index de873d7..ea3cfe9 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_hsi.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_hsi.h
@@ -12210,8 +12210,11 @@ struct public_drv_mb {
#define DRV_MSG_CODE_VF_DISABLED_DONE 0xc0000000
#define DRV_MSG_CODE_CFG_VF_MSIX 0xc0010000
#define DRV_MSG_CODE_CFG_PF_VFS_MSIX 0xc0020000
+#define DRV_MSG_CODE_NVM_PUT_FILE_BEGIN 0x00010000
+#define DRV_MSG_CODE_NVM_PUT_FILE_DATA 0x00020000
#define DRV_MSG_CODE_NVM_GET_FILE_ATT 0x00030000
#define DRV_MSG_CODE_NVM_READ_NVRAM 0x00050000
+#define DRV_MSG_CODE_NVM_WRITE_NVRAM 0x00060000
#define DRV_MSG_CODE_MCP_RESET 0x00090000
#define DRV_MSG_CODE_SET_VERSION 0x000f0000
#define DRV_MSG_CODE_MCP_HALT 0x00100000
@@ -12265,7 +12268,6 @@ struct public_drv_mb {
#define DRV_MSG_CODE_FEATURE_SUPPORT 0x00300000
#define DRV_MSG_CODE_GET_MFW_FEATURE_SUPPORT 0x00310000
-
#define DRV_MSG_SEQ_NUMBER_MASK 0x0000ffff
u32 drv_mb_param;
@@ -12377,7 +12379,10 @@ struct public_drv_mb {
#define FW_MSG_CODE_DRV_CFG_VF_MSIX_DONE 0xb0010000
#define FW_MSG_CODE_NVM_OK 0x00010000
+#define FW_MSG_CODE_NVM_PUT_FILE_FINISH_OK 0x00400000
+#define FW_MSG_CODE_PHY_OK 0x00110000
#define FW_MSG_CODE_OK 0x00160000
+#define FW_MSG_CODE_ERROR 0x00170000
#define FW_MSG_CODE_OS_WOL_SUPPORTED 0x00800000
#define FW_MSG_CODE_OS_WOL_NOT_SUPPORTED 0x00810000
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
index 2519e71..ec0d425 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
@@ -569,6 +569,31 @@ int qed_mcp_cmd(struct qed_hwfn *p_hwfn,
return 0;
}
+int qed_mcp_nvm_wr_cmd(struct qed_hwfn *p_hwfn,
+ struct qed_ptt *p_ptt,
+ u32 cmd,
+ u32 param,
+ u32 *o_mcp_resp,
+ u32 *o_mcp_param, u32 i_txn_size, u32 *i_buf)
+{
+ struct qed_mcp_mb_params mb_params;
+ int rc;
+
+ memset(&mb_params, 0, sizeof(mb_params));
+ mb_params.cmd = cmd;
+ mb_params.param = param;
+ mb_params.p_data_src = i_buf;
+ mb_params.data_src_size = (u8)i_txn_size;
+ rc = qed_mcp_cmd_and_union(p_hwfn, p_ptt, &mb_params);
+ if (rc)
+ return rc;
+
+ *o_mcp_resp = mb_params.mcp_resp;
+ *o_mcp_param = mb_params.mcp_param;
+
+ return 0;
+}
+
int qed_mcp_nvm_rd_cmd(struct qed_hwfn *p_hwfn,
struct qed_ptt *p_ptt,
u32 cmd,
@@ -2261,6 +2286,102 @@ int qed_mcp_nvm_read(struct qed_dev *cdev, u32 addr, u8 *p_buf, u32 len)
return rc;
}
+int qed_mcp_nvm_resp(struct qed_dev *cdev, u8 *p_buf)
+{
+ struct qed_hwfn *p_hwfn = QED_LEADING_HWFN(cdev);
+ struct qed_ptt *p_ptt;
+
+ p_ptt = qed_ptt_acquire(p_hwfn);
+ if (!p_ptt)
+ return -EBUSY;
+
+ memcpy(p_buf, &cdev->mcp_nvm_resp, sizeof(cdev->mcp_nvm_resp));
+ qed_ptt_release(p_hwfn, p_ptt);
+
+ return 0;
+}
+
+int qed_mcp_nvm_put_file_begin(struct qed_dev *cdev, u32 addr)
+{
+ struct qed_hwfn *p_hwfn = QED_LEADING_HWFN(cdev);
+ struct qed_ptt *p_ptt;
+ u32 resp, param;
+ int rc;
+
+ p_ptt = qed_ptt_acquire(p_hwfn);
+ if (!p_ptt)
+ return -EBUSY;
+ rc = qed_mcp_cmd(p_hwfn, p_ptt, DRV_MSG_CODE_NVM_PUT_FILE_BEGIN, addr,
+ &resp, ¶m);
+ cdev->mcp_nvm_resp = resp;
+ qed_ptt_release(p_hwfn, p_ptt);
+
+ return rc;
+}
+
+int qed_mcp_nvm_write(struct qed_dev *cdev,
+ u32 cmd, u32 addr, u8 *p_buf, u32 len)
+{
+ u32 buf_idx = 0, buf_size, nvm_cmd, nvm_offset, resp = 0, param;
+ struct qed_hwfn *p_hwfn = QED_LEADING_HWFN(cdev);
+ struct qed_ptt *p_ptt;
+ int rc = -EINVAL;
+
+ p_ptt = qed_ptt_acquire(p_hwfn);
+ if (!p_ptt)
+ return -EBUSY;
+
+ switch (cmd) {
+ case QED_PUT_FILE_DATA:
+ nvm_cmd = DRV_MSG_CODE_NVM_PUT_FILE_DATA;
+ break;
+ case QED_NVM_WRITE_NVRAM:
+ nvm_cmd = DRV_MSG_CODE_NVM_WRITE_NVRAM;
+ break;
+ default:
+ DP_NOTICE(p_hwfn, "Invalid nvm write command 0x%x\n", cmd);
+ rc = -EINVAL;
+ goto out;
+ }
+
+ while (buf_idx < len) {
+ buf_size = min_t(u32, (len - buf_idx), MCP_DRV_NVM_BUF_LEN);
+ nvm_offset = ((buf_size << DRV_MB_PARAM_NVM_LEN_OFFSET) |
+ addr) + buf_idx;
+ rc = qed_mcp_nvm_wr_cmd(p_hwfn, p_ptt, nvm_cmd, nvm_offset,
+ &resp, ¶m, buf_size,
+ (u32 *)&p_buf[buf_idx]);
+ if (rc) {
+ DP_NOTICE(cdev, "nvm write failed, rc = %d\n", rc);
+ resp = FW_MSG_CODE_ERROR;
+ break;
+ }
+
+ if (resp != FW_MSG_CODE_OK &&
+ resp != FW_MSG_CODE_NVM_OK &&
+ resp != FW_MSG_CODE_NVM_PUT_FILE_FINISH_OK) {
+ DP_NOTICE(cdev,
+ "nvm write failed, resp = 0x%08x\n", resp);
+ rc = -EINVAL;
+ break;
+ }
+
+ /* This can be a lengthy process, and it's possible scheduler
+ * isn't pre-emptable. Sleep a bit to prevent CPU hogging.
+ */
+ if (buf_idx % 0x1000 > (buf_idx + buf_size) % 0x1000)
+ usleep_range(1000, 2000);
+
+ buf_idx += buf_size;
+ }
+
+ cdev->mcp_nvm_resp = resp;
+out:
+ qed_ptt_release(p_hwfn, p_ptt);
+
+ return rc;
+}
+
int qed_mcp_bist_register_test(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
{
u32 drv_mb_param = 0, rsp, param;
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.h b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
index 7d33354..8a5c988 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
@@ -443,6 +443,40 @@ int qed_mcp_set_led(struct qed_hwfn *p_hwfn,
*/
int qed_mcp_nvm_read(struct qed_dev *cdev, u32 addr, u8 *p_buf, u32 len);
+/**
+ * @brief Write to nvm
+ *
+ * @param cdev
+ * @param addr - nvm offset
+ * @param cmd - nvm command
+ * @param p_buf - nvm write buffer
+ * @param len - buffer len
+ *
+ * @return int - 0 - operation was successful.
+ */
+int qed_mcp_nvm_write(struct qed_dev *cdev,
+ u32 cmd, u32 addr, u8 *p_buf, u32 len);
+
+/**
+ * @brief Put file begin
+ *
+ * @param cdev
+ * @param addr - nvm offset
+ *
+ * @return int - 0 - operation was successful.
+ */
+int qed_mcp_nvm_put_file_begin(struct qed_dev *cdev, u32 addr);
+
+/**
+ * @brief Check latest response
+ *
+ * @param cdev
+ * @param p_buf - nvm write buffer
+ *
+ * @return int - 0 - operation was successful.
+ */
+int qed_mcp_nvm_resp(struct qed_dev *cdev, u8 *p_buf);
+
struct qed_nvm_image_att {
u32 start_addr;
u32 length;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 4/5] qed: Adapter flash update support.
From: Sudarsana Reddy Kalluru @ 2018-03-28 12:14 UTC (permalink / raw)
To: davem; +Cc: netdev, Ariel.Elior, yuvalm
In-Reply-To: <20180328121423.7718-1-sudarsana.kalluru@cavium.com>
This patch adds the required driver support for updating the flash or
non volatile memory of the adapter. At highlevel, flash upgrade comprises
of reading the flash images from the input file, validating the images and
writing them to the respective paritions.
Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
Signed-off-by: Ariel Elior <ariel.elior@cavium.com>
---
drivers/net/ethernet/qlogic/qed/qed_main.c | 338 +++++++++++++++++++++++++++++
include/linux/qed/qed_if.h | 19 ++
2 files changed, 357 insertions(+)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 2783288..9854aa9 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -45,6 +45,7 @@
#include <linux/etherdevice.h>
#include <linux/vmalloc.h>
#include <linux/crash_dump.h>
+#include <linux/crc32.h>
#include <linux/qed/qed_if.h>
#include <linux/qed/qed_ll2_if.h>
@@ -1553,6 +1554,342 @@ static int qed_drain(struct qed_dev *cdev)
return 0;
}
+static u32 qed_nvm_flash_image_access_crc(struct qed_dev *cdev,
+ struct qed_nvm_image_att *nvm_image,
+ u32 *crc)
+{
+ u8 *buf = NULL;
+ int rc, j;
+ u32 val;
+
+ /* Allocate a buffer for holding the nvram image */
+ buf = kzalloc(nvm_image->length, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ /* Read image into buffer */
+ rc = qed_mcp_nvm_read(cdev, nvm_image->start_addr,
+ buf, nvm_image->length);
+ if (rc) {
+ DP_ERR(cdev, "Failed reading image from nvm\n");
+ goto out;
+ }
+
+ /* Convert the buffer into big-endian format (excluding the
+ * closing 4 bytes of CRC).
+ */
+ for (j = 0; j < nvm_image->length - 4; j += 4) {
+ val = cpu_to_be32(*(u32 *)&buf[j]);
+ *(u32 *)&buf[j] = val;
+ }
+
+ /* Calc CRC for the "actual" image buffer, i.e. not including
+ * the last 4 CRC bytes.
+ */
+ *crc = (~cpu_to_be32(crc32(0xffffffff, buf, nvm_image->length - 4)));
+
+out:
+ kfree(buf);
+
+ return rc;
+}
+
+/* Binary file format -
+ * /----------------------------------------------------------------------\
+ * 0B | 0x4 [command index] |
+ * 4B | image_type | Options | Number of register settings |
+ * 8B | Value |
+ * 12B | Mask |
+ * 16B | Offset |
+ * \----------------------------------------------------------------------/
+ * There can be several Value-Mask-Offset sets as specified by 'Number of...'.
+ * Options - 0'b - Calculate & Update CRC for image
+ */
+static int qed_nvm_flash_image_access(struct qed_dev *cdev, const u8 **data,
+ bool *check_resp)
+{
+ struct qed_nvm_image_att nvm_image;
+ struct qed_hwfn *p_hwfn;
+ bool is_crc = false;
+ u32 image_type;
+ int rc = 0, i;
+ u16 len;
+
+ *data += 4;
+ image_type = **data;
+ p_hwfn = QED_LEADING_HWFN(cdev);
+ for (i = 0; i < p_hwfn->nvm_info.num_images; i++)
+ if (image_type == p_hwfn->nvm_info.image_att[i].image_type)
+ break;
+ if (i == p_hwfn->nvm_info.num_images) {
+ DP_ERR(cdev, "Failed to find nvram image of type %08x\n",
+ image_type);
+ return -ENOENT;
+ }
+
+ nvm_image.start_addr = p_hwfn->nvm_info.image_att[i].nvm_start_addr;
+ nvm_image.length = p_hwfn->nvm_info.image_att[i].len;
+
+ DP_VERBOSE(cdev, NETIF_MSG_DRV,
+ "Read image %02x; type = %08x; NVM [%08x,...,%08x]\n",
+ **data, image_type, nvm_image.start_addr,
+ nvm_image.start_addr + nvm_image.length - 1);
+ (*data)++;
+ is_crc = !!(**data & BIT(0));
+ (*data)++;
+ len = *((u16 *)*data);
+ *data += 2;
+ if (is_crc) {
+ u32 crc = 0;
+
+ rc = qed_nvm_flash_image_access_crc(cdev, &nvm_image, &crc);
+ if (rc) {
+ DP_ERR(cdev, "Failed calculating CRC, rc = %d\n", rc);
+ goto exit;
+ }
+
+ rc = qed_mcp_nvm_write(cdev, QED_NVM_WRITE_NVRAM,
+ (nvm_image.start_addr +
+ nvm_image.length - 4), (u8 *)&crc, 4);
+ if (rc)
+ DP_ERR(cdev, "Failed writing to %08x, rc = %d\n",
+ nvm_image.start_addr + nvm_image.length - 4, rc);
+ goto exit;
+ }
+
+ /* Iterate over the values for setting */
+ while (len) {
+ u32 offset, mask, value, cur_value;
+ u8 buf[4];
+
+ value = *((u32 *)*data);
+ *data += 4;
+ mask = *((u32 *)*data);
+ *data += 4;
+ offset = *((u32 *)*data);
+ *data += 4;
+
+ rc = qed_mcp_nvm_read(cdev, nvm_image.start_addr + offset, buf,
+ 4);
+ if (rc) {
+ DP_ERR(cdev, "Failed reading from %08x\n",
+ nvm_image.start_addr + offset);
+ goto exit;
+ }
+
+ cur_value = le32_to_cpu(*((__le32 *)buf));
+ DP_VERBOSE(cdev, NETIF_MSG_DRV,
+ "NVM %08x: %08x -> %08x [Value %08x Mask %08x]\n",
+ nvm_image.start_addr + offset, cur_value,
+ (cur_value & ~mask) | (value & mask), value, mask);
+ value = (value & mask) | (cur_value & ~mask);
+ rc = qed_mcp_nvm_write(cdev, QED_NVM_WRITE_NVRAM,
+ nvm_image.start_addr + offset,
+ (u8 *)&value, 4);
+ if (rc) {
+ DP_ERR(cdev, "Failed writing to %08x\n",
+ nvm_image.start_addr + offset);
+ goto exit;
+ }
+
+ len--;
+ }
+exit:
+ return rc;
+}
+
+/* Binary file format -
+ * /----------------------------------------------------------------------\
+ * 0B | 0x3 [command index] |
+ * 4B | b'0: check_response? | b'1-31 reserved |
+ * 8B | File-type | reserved |
+ * \----------------------------------------------------------------------/
+ * Start a new file of the provided type
+ */
+static int qed_nvm_flash_image_file_start(struct qed_dev *cdev,
+ const u8 **data, bool *check_resp)
+{
+ int rc;
+
+ *data += 4;
+ *check_resp = !!(**data & BIT(0));
+ *data += 4;
+
+ DP_VERBOSE(cdev, NETIF_MSG_DRV,
+ "About to start a new file of type %02x\n", **data);
+ rc = qed_mcp_nvm_put_file_begin(cdev, **data);
+ *data += 4;
+
+ return rc;
+}
+
+/* Binary file format -
+ * /----------------------------------------------------------------------\
+ * 0B | 0x2 [command index] |
+ * 4B | Length in bytes |
+ * 8B | b'0: check_response? | b'1-31 reserved |
+ * 12B | Offset in bytes |
+ * 16B | Data ... |
+ * \----------------------------------------------------------------------/
+ * Write data as part of a file that was previously started. Data should be
+ * of length equal to that provided in the message
+ */
+static int qed_nvm_flash_image_file_data(struct qed_dev *cdev,
+ const u8 **data, bool *check_resp)
+{
+ u32 offset, len;
+ int rc;
+
+ *data += 4;
+ len = *((u32 *)(*data));
+ *data += 4;
+ *check_resp = !!(**data & BIT(0));
+ *data += 4;
+ offset = *((u32 *)(*data));
+ *data += 4;
+
+ DP_VERBOSE(cdev, NETIF_MSG_DRV,
+ "About to write File-data: %08x bytes to offset %08x\n",
+ len, offset);
+
+ rc = qed_mcp_nvm_write(cdev, QED_PUT_FILE_DATA, offset,
+ (char *)(*data), len);
+ *data += len;
+
+ return rc;
+}
+
+/* Binary file format [General header] -
+ * /----------------------------------------------------------------------\
+ * 0B | QED_NVM_SIGNATURE |
+ * 4B | Length in bytes |
+ * 8B | Highest command in this batchfile | Reserved |
+ * \----------------------------------------------------------------------/
+ */
+static int qed_nvm_flash_image_validate(struct qed_dev *cdev,
+ const struct firmware *image,
+ const u8 **data)
+{
+ u32 signature, len;
+
+ /* Check minimum size */
+ if (image->size < 12) {
+ DP_ERR(cdev, "Image is too short [%08x]\n", (u32)image->size);
+ return -EINVAL;
+ }
+
+ /* Check signature */
+ signature = *((u32 *)(*data));
+ if (signature != QED_NVM_SIGNATURE) {
+ DP_ERR(cdev, "Wrong signature '%08x'\n", signature);
+ return -EINVAL;
+ }
+
+ *data += 4;
+ /* Validate internal size equals the image-size */
+ len = *((u32 *)(*data));
+ if (len != image->size) {
+ DP_ERR(cdev, "Size mismatch: internal = %08x image = %08x\n",
+ len, (u32)image->size);
+ return -EINVAL;
+ }
+
+ *data += 4;
+ /* Make sure driver familiar with all commands necessary for this */
+ if (*((u16 *)(*data)) >= QED_NVM_FLASH_CMD_NVM_MAX) {
+ DP_ERR(cdev, "File contains unsupported commands [Need %04x]\n",
+ *((u16 *)(*data)));
+ return -EINVAL;
+ }
+
+ *data += 4;
+
+ return 0;
+}
+
+static int qed_nvm_flash(struct qed_dev *cdev, const char *name)
+{
+ const struct firmware *image;
+ const u8 *data, *data_end;
+ u32 cmd_type;
+ int rc;
+
+ rc = request_firmware(&image, name, &cdev->pdev->dev);
+ if (rc) {
+ DP_ERR(cdev, "Failed to find '%s'\n", name);
+ return rc;
+ }
+
+ DP_VERBOSE(cdev, NETIF_MSG_DRV,
+ "Flashing '%s' - firmware's data at %p, size is %08x\n",
+ name, image->data, (u32)image->size);
+ data = image->data;
+ data_end = data + image->size;
+
+ rc = qed_nvm_flash_image_validate(cdev, image, &data);
+ if (rc)
+ goto exit;
+
+ while (data < data_end) {
+ bool check_resp = false;
+
+ /* Parse the actual command */
+ cmd_type = *((u32 *)data);
+ switch (cmd_type) {
+ case QED_NVM_FLASH_CMD_FILE_DATA:
+ rc = qed_nvm_flash_image_file_data(cdev, &data,
+ &check_resp);
+ break;
+ case QED_NVM_FLASH_CMD_FILE_START:
+ rc = qed_nvm_flash_image_file_start(cdev, &data,
+ &check_resp);
+ break;
+ case QED_NVM_FLASH_CMD_NVM_CHANGE:
+ rc = qed_nvm_flash_image_access(cdev, &data,
+ &check_resp);
+ break;
+ default:
+ DP_ERR(cdev, "Unknown command %08x\n", cmd_type);
+ rc = -EINVAL;
+ goto exit;
+ }
+
+ if (rc) {
+ DP_ERR(cdev, "Command %08x failed\n", cmd_type);
+ goto exit;
+ }
+
+ /* Check response if needed */
+ if (check_resp) {
+ u32 mcp_response = 0;
+
+ if (qed_mcp_nvm_resp(cdev, (u8 *)&mcp_response)) {
+ DP_ERR(cdev, "Failed getting MCP response\n");
+ rc = -EINVAL;
+ goto exit;
+ }
+
+ switch (mcp_response & FW_MSG_CODE_MASK) {
+ case FW_MSG_CODE_OK:
+ case FW_MSG_CODE_NVM_OK:
+ case FW_MSG_CODE_NVM_PUT_FILE_FINISH_OK:
+ case FW_MSG_CODE_PHY_OK:
+ break;
+ default:
+ DP_ERR(cdev, "MFW returns error: %08x\n",
+ mcp_response);
+ rc = -EINVAL;
+ goto exit;
+ }
+ }
+ }
+
+exit:
+ release_firmware(image);
+
+ return rc;
+}
+
static int qed_nvm_get_image(struct qed_dev *cdev, enum qed_nvm_images type,
u8 *buf, u16 len)
{
@@ -1719,6 +2056,7 @@ static int qed_update_mtu(struct qed_dev *cdev, u16 mtu)
.dbg_all_data_size = &qed_dbg_all_data_size,
.chain_alloc = &qed_chain_alloc,
.chain_free = &qed_chain_free,
+ .nvm_flash = &qed_nvm_flash,
.nvm_get_image = &qed_nvm_get_image,
.set_coalesce = &qed_set_coalesce,
.set_led = &qed_set_led,
diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
index 15e398c..b5b2bc9 100644
--- a/include/linux/qed/qed_if.h
+++ b/include/linux/qed/qed_if.h
@@ -483,6 +483,15 @@ struct qed_int_info {
u8 used_cnt;
};
+#define QED_NVM_SIGNATURE 0x12435687
+
+enum qed_nvm_flash_cmd {
+ QED_NVM_FLASH_CMD_FILE_DATA = 0x2,
+ QED_NVM_FLASH_CMD_FILE_START = 0x3,
+ QED_NVM_FLASH_CMD_NVM_CHANGE = 0x4,
+ QED_NVM_FLASH_CMD_NVM_MAX,
+};
+
struct qed_common_cb_ops {
void (*arfs_filter_op)(void *dev, void *fltr, u8 fw_rc);
void (*link_update)(void *dev,
@@ -658,6 +667,16 @@ struct qed_common_ops {
struct qed_chain *p_chain);
/**
+ * @brief nvm_flash - Flash nvm data.
+ *
+ * @param cdev
+ * @param name - file containing the data
+ *
+ * @return 0 on success, error otherwise.
+ */
+ int (*nvm_flash)(struct qed_dev *cdev, const char *name);
+
+/**
* @brief nvm_get_image - reads an entire image from nvram
*
* @param cdev
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 5/5] qede: Ethtool flash update support.
From: Sudarsana Reddy Kalluru @ 2018-03-28 12:14 UTC (permalink / raw)
To: davem; +Cc: netdev, Ariel.Elior, yuvalm
In-Reply-To: <20180328121423.7718-1-sudarsana.kalluru@cavium.com>
The patch adds ethtool callback implementation for flash update.
Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
Signed-off-by: Ariel Elior <ariel.elior@cavium.com>
---
drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index 4ca3847..ecbf1de 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -699,6 +699,14 @@ static u32 qede_get_link(struct net_device *dev)
return current_link.link_up;
}
+static int qede_flash_device(struct net_device *dev,
+ struct ethtool_flash *flash)
+{
+ struct qede_dev *edev = netdev_priv(dev);
+
+ return edev->ops->common->nvm_flash(edev->cdev, flash->data);
+}
+
static int qede_get_coalesce(struct net_device *dev,
struct ethtool_coalesce *coal)
{
@@ -1806,6 +1814,7 @@ static int qede_set_eee(struct net_device *dev, struct ethtool_eee *edata)
.get_tunable = qede_get_tunable,
.set_tunable = qede_set_tunable,
+ .flash_device = qede_flash_device,
};
static const struct ethtool_ops qede_vf_ethtool_ops = {
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 3/7] net: thunderx: add multicast filter management support
From: kbuild test robot @ 2018-03-28 12:29 UTC (permalink / raw)
To: Vadim Lomovtsev
Cc: kbuild-all, sgoutham, sunil.kovvuri, robert.richter,
linux-arm-kernel, netdev, linux-kernel, dnelson, Vadim Lomovtsev
In-Reply-To: <20180327150736.10718-4-Vadim.Lomovtsev@caviumnetworks.com>
[-- Attachment #1: Type: text/plain, Size: 3695 bytes --]
Hi Vadim,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
[also build test WARNING on v4.16-rc7 next-20180328]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Vadim-Lomovtsev/net-thunderx-implement-DMAC-filtering-support/20180328-190332
config: x86_64-randconfig-s1-03281908 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
drivers/net/ethernet/cavium/thunder/thunder_bgx.c: In function 'bgx_lmac_save_filter':
>> drivers/net/ethernet/cavium/thunder/thunder_bgx.c:286:3: warning: 'return' with no value, in function returning non-void [-Wreturn-type]
return;
^~~~~~
drivers/net/ethernet/cavium/thunder/thunder_bgx.c:281:12: note: declared here
static int bgx_lmac_save_filter(struct lmac *lmac, u64 dmac, u8 vf_id)
^~~~~~~~~~~~~~~~~~~~
In file included from drivers/net/ethernet/cavium/thunder/nic.h:15:0,
from drivers/net/ethernet/cavium/thunder/thunder_bgx.c:21:
drivers/net/ethernet/cavium/thunder/thunder_bgx.c: In function 'bgx_set_dmac_cam_filter_mac':
>> drivers/net/ethernet/cavium/thunder/thunder_bgx.h:61:38: warning: left shift count >= width of type [-Wshift-count-overflow]
#define RX_DMACX_CAM_LMACID(x) (x << 49)
^
>> drivers/net/ethernet/cavium/thunder/thunder_bgx.c:324:8: note: in expansion of macro 'RX_DMACX_CAM_LMACID'
cfg = RX_DMACX_CAM_LMACID(lmacid & LMAC_ID_MASK) |
^~~~~~~~~~~~~~~~~~~
vim +/return +286 drivers/net/ethernet/cavium/thunder/thunder_bgx.c
280
> 281 static int bgx_lmac_save_filter(struct lmac *lmac, u64 dmac, u8 vf_id)
282 {
283 u8 i = 0;
284
285 if (!lmac)
> 286 return;
287
288 /* At the same time we could have several VFs 'attached' to some
289 * particular LMAC, and each VF is represented as network interface
290 * for kernel. So from user perspective it should be possible to
291 * manipulate with its' (VF) receive modes. However from PF
292 * driver perspective we need to keep track of filter configurations
293 * for different VFs to prevent filter values dupes
294 */
295 for (i = 0; i < lmac->dmacs_cfg; i++) {
296 if (lmac->dmacs[i].dmac == dmac) {
297 lmac->dmacs[i].vf_map |= BIT_ULL(vf_id);
298 return -1;
299 }
300 }
301
302 if (!(lmac->dmacs_cfg < lmac->dmacs_count))
303 return -1;
304
305 /* keep it for further tracking */
306 lmac->dmacs[lmac->dmacs_cfg].dmac = dmac;
307 lmac->dmacs[lmac->dmacs_cfg].vf_map = BIT_ULL(vf_id);
308 lmac->dmacs_cfg++;
309 return 0;
310 }
311
312 static int bgx_set_dmac_cam_filter_mac(struct bgx *bgx, int lmacid, u64 cam_dmac, u8 idx)
313 {
314 struct lmac *lmac = NULL;
315 u64 cfg = 0;
316
317 /* skip zero addresses as meaningless */
318 if (!cam_dmac || !bgx)
319 return -1;
320
321 lmac = &bgx->lmac[lmacid];
322
323 /* configure DCAM filtering for designated LMAC */
> 324 cfg = RX_DMACX_CAM_LMACID(lmacid & LMAC_ID_MASK) |
325 RX_DMACX_CAM_EN | cam_dmac;
326 bgx_reg_write(bgx, 0, BGX_CMR_RX_DMACX_CAM + ((lmacid * lmac->dmacs_count) + idx) * sizeof(u64), cfg);
327 return 0;
328 }
329
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29217 bytes --]
^ permalink raw reply
* Re: [PATCH] net: fec: set dma_coherent_mask
From: Greg Ungerer @ 2018-03-28 12:29 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: netdev, Linux/m68k
In-Reply-To: <CAMuHMdXzySkpAi+kS+XCXR8kjY_AsV4h6t+XsuXx4o_0u2GSEA@mail.gmail.com>
Hi Geert,
On 28/03/18 17:57, Geert Uytterhoeven wrote:
[skip]
>> [PATCH] m68k: set dma and coherent masks for platform FEC ethernets
>>
>> As of commit 205e1b7f51e4 ("dma-mapping: warn when there is no
>> coherent_dma_mask") the Freescale FEC driver is issuing the following
>> warning on driver initialization on ColdFire systems:
>>
>> WARNING: CPU: 0 PID: 1 at ./include/linux/dma-mapping.h:516 0x40159e20
>> Modules linked in:
>> CPU: 0 PID: 1 Comm: swapper Not tainted 4.16.0-rc7-dirty #4
>> Stack from 41833dd8:
>> 41833dd8 40259c53 40025534 40279e26 00000003 00000000 4004e514 41827000
>> 400255de 40244e42 00000204 40159e20 00000009 00000000 00000000 4024531d
>> 40159e20 40244e42 00000204 00000000 00000000 00000000 00000007 00000000
>> 00000000 40279e26 4028d040 40226576 4003ae88 40279e26 418273f6 41833ef8
>> 7fffffff 418273f2 41867028 4003c9a2 4180ac6c 00000004 41833f8c 4013e71c
>> 40279e1c 40279e26 40226c16 4013ced2 40279e26 40279e58 4028d040 00000000
>> Call Trace:
>> [<40025534>] 0x40025534
>> [<4004e514>] 0x4004e514
>> [<400255de>] 0x400255de
>> [<40159e20>] 0x40159e20
>> [<40159e20>] 0x40159e20
>>
>> It is not fatal, the driver and the system continue to function normally.
>>
>> As per the warning the coherent_dma_mask is not set on this device.
>> There is nothing special about the DMA memory coherency on this hardware
>> so we can just set the mask to 32bits in the platform data for the FEC
>> ethernet devices.
>>
>> Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
>> ---
>> arch/m68k/coldfire/device.c | 14 ++++++++++++--
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/m68k/coldfire/device.c b/arch/m68k/coldfire/device.c
>> index 84938fd..f93e0e5 100644
>> --- a/arch/m68k/coldfire/device.c
>> +++ b/arch/m68k/coldfire/device.c
>> @@ -130,12 +130,18 @@
>> },
>> };
>>
>> +static u64 mcf_dma_mask = DMA_BIT_MASK(32);
>> +
>> static struct platform_device mcf_fec0 = {
>> .name = FEC_NAME,
>> .id = 0,
>> .num_resources = ARRAY_SIZE(mcf_fec0_resources),
>> .resource = mcf_fec0_resources,
>> - .dev.platform_data = FEC_PDATA,
>> + .dev = {
>> + .dma_mask = &mcf_dma_mask,
>
> Can you make this &mcf_fec0.dev.coherent_dma_mask, removing the need for
> mcf_dma_mask, or doesn't C allow that?
Yes, sure can. Looks like some other architectures do that too.
I'll change it. Thanks.
Regards
Greg
>> + .coherent_dma_mask = DMA_BIT_MASK(32),
>> + .platform_data = FEC_PDATA,
>> + }
>> };
>>
>> #ifdef MCFFEC_BASE1
>> @@ -167,7 +173,11 @@
>> .id = 1,
>> .num_resources = ARRAY_SIZE(mcf_fec1_resources),
>> .resource = mcf_fec1_resources,
>> - .dev.platform_data = FEC_PDATA,
>> + .dev = {
>> + .dma_mask = &mcf_dma_mask,
>
> Likewise.
>
>> + .coherent_dma_mask = DMA_BIT_MASK(32),
>> + .platform_data = FEC_PDATA,
>> + }
>
> Gr{oetje,eeting}s,
>
> Geert
>
^ permalink raw reply
* [PATCH net-next] ice: Fix error return code in ice_init_hw()
From: Wei Yongjun @ 2018-03-28 12:50 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: Wei Yongjun, intel-wired-lan, netdev, kernel-janitors
Fix to return error code ICE_ERR_NO_MEMORY from the alloc error
handling case instead of 0, as done elsewhere in this function.
Fixes: dc49c7723676 ("ice: Get MAC/PHY/link info and scheduler topology")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
drivers/net/ethernet/intel/ice/ice_common.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 385f5d4..21977ec 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -468,8 +468,10 @@ enum ice_status ice_init_hw(struct ice_hw *hw)
mac_buf_len = sizeof(struct ice_aqc_manage_mac_read_resp);
mac_buf = devm_kzalloc(ice_hw_to_dev(hw), mac_buf_len, GFP_KERNEL);
- if (!mac_buf)
+ if (!mac_buf) {
+ status = ICE_ERR_NO_MEMORY;
goto err_unroll_fltr_mgmt_struct;
+ }
status = ice_aq_manage_mac_read(hw, mac_buf, mac_buf_len, NULL);
devm_kfree(ice_hw_to_dev(hw), mac_buf);
^ permalink raw reply related
* [PATCH net-next] net: hns3: remove unnecessary pci_set_drvdata() and devm_kfree()
From: Wei Yongjun @ 2018-03-28 12:51 UTC (permalink / raw)
To: Yisen Zhuang, Salil Mehta; +Cc: Wei Yongjun, netdev, kernel-janitors
The devm_kfree function allocates memory that is released when a
driver detaches. Also the driver core clears the driver data to NULL
after device release. So remove the unnecessary pci_set_drvdata()
and devm_kfree().
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index a31b4ad..8c55965 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -1614,10 +1614,6 @@ static void hns3_remove(struct pci_dev *pdev)
struct hnae3_ae_dev *ae_dev = pci_get_drvdata(pdev);
hnae3_unregister_ae_dev(ae_dev);
-
- devm_kfree(&pdev->dev, ae_dev);
-
- pci_set_drvdata(pdev, NULL);
}
static struct pci_driver hns3_driver = {
^ permalink raw reply related
* [PATCH net-next] net: bcmgenet: return NULL instead of plain integer
From: Wei Yongjun @ 2018-03-28 12:51 UTC (permalink / raw)
To: Doug Berger, Florian Fainelli; +Cc: Wei Yongjun, netdev, kernel-janitors
Fixes the following sparse warning:
drivers/net/ethernet/broadcom/genet/bcmgenet.c:1351:16: warning:
Using plain integer as NULL pointer
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 7db8edc..2f4cb5c 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1348,7 +1348,7 @@ static struct sk_buff *bcmgenet_free_tx_cb(struct device *dev,
dma_unmap_addr_set(cb, dma_addr, 0);
}
- return 0;
+ return NULL;
}
/* Simple helper to free a receive control block's resources */
^ permalink raw reply related
* [PATCH net-next] cxgb4: fix error return code in adap_init0()
From: Wei Yongjun @ 2018-03-28 12:51 UTC (permalink / raw)
To: Ganesh Goudar, Kumar Sanghvi; +Cc: Wei Yongjun, netdev, kernel-janitors
Fix to return a negative error code from the hash filter init error
handling case instead of 0, as done elsewhere in this function.
Fixes: 5c31254e35a8 ("cxgb4: initialize hash-filter configuration")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 57d38f8..0072580 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -4449,7 +4449,8 @@ static int adap_init0(struct adapter *adap)
adap->params.ofldq_wr_cred = val[5];
if (caps_cmd.niccaps & htons(FW_CAPS_CONFIG_NIC_HASHFILTER)) {
- if (init_hash_filter(adap) < 0)
+ ret = init_hash_filter(adap);
+ if (ret < 0)
goto bye;
} else {
adap->params.offload = 1;
^ permalink raw reply related
* Re: [PATCH v3 net 1/5] tcp: feed correct number of pkts acked to cc modules also in recovery
From: Ilpo Järvinen @ 2018-03-28 12:45 UTC (permalink / raw)
To: Yuchung Cheng; +Cc: netdev, Neal Cardwell, Eric Dumazet, Sergei Shtylyov
In-Reply-To: <CAK6E8=e7=h0Z7PV9CpyurhiW-jccF+aUDfD1eSXYTWkLwUKT=A@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 8746 bytes --]
On Tue, 27 Mar 2018, Yuchung Cheng wrote:
> On Tue, Mar 27, 2018 at 7:23 AM, Ilpo Järvinen
> <ilpo.jarvinen@helsinki.fi> wrote:
> > On Mon, 26 Mar 2018, Yuchung Cheng wrote:
> >
> >> On Tue, Mar 13, 2018 at 3:25 AM, Ilpo Järvinen
> >> <ilpo.jarvinen@helsinki.fi> wrote:
> >> >
> >> > A miscalculation for the number of acknowledged packets occurs during
> >> > RTO recovery whenever SACK is not enabled and a cumulative ACK covers
> >> > any non-retransmitted skbs. The reason is that pkts_acked value
> >> > calculated in tcp_clean_rtx_queue is not correct for slow start after
> >> > RTO as it may include segments that were not lost and therefore did
> >> > not need retransmissions in the slow start following the RTO. Then
> >> > tcp_slow_start will add the excess into cwnd bloating it and
> >> > triggering a burst.
> >> >
> >> > Instead, we want to pass only the number of retransmitted segments
> >> > that were covered by the cumulative ACK (and potentially newly sent
> >> > data segments too if the cumulative ACK covers that far).
> >> >
> >> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> >> > ---
> >>
> >> My understanding is there are two problems
> >>
> >> 1) your fix: the reordering logic in tcp-remove_reno_sacks requires
> >> precise cumulatively acked count, not newly acked count?
> >
> > While I'm not entirely sure if you intented to say that my fix is broken
> > or not, I thought this very difference alot while making the fix and I
> > believe that this fix is needed because of the discontinuity at RTO
> > (sacked_out is cleared as we set L-bits + lost_out). This is an artifact
> > in the imitation of sacked_out for non-SACK but at RTO we can't keep that
> > in sync because we set L-bits (and have no S-bits to guide us). Thus, we
> > cannot anymore "use" those skbs with only L-bit for the reno_sacks logic.
> >
> > In tcp_remove_reno_sacks acked - sacked_out is being used to calculate
> > tp->delivered, using plain cumulative acked causes congestion control
> > breakage later as call to tcp_cong_control will directly use the
> > difference in tp->delivered.
> >
> > This boils down the exact definition of tp->delivered (the one given in
> > the header is not detailed enough). I guess you might have better idea
> > what it exactly is since one of you has added it? There are subtle things
> > in the defination that can make it entirely unsuitable for cc decisions.
> > Should those segments that we (possibly) already counted into
> > tp->delivered during (potentially preceeding) CA_Recovery be added to it
> > for _second time_ or not? This fix avoids such double counting (the
> Where is the double counting, assuming normal DUPACK behavior?
>
> In the non-sack case:
>
> 1. upon receiving a DUPACK, we assume one packet has been delivered by
> incrementing tp->delivered in tcp_add_reno_sack()
1b. RTO here. We clear tp->sacked_out at RTO (i.e., the discontinuity
I've tried to point out quite many times already)...
> 2. upon receiving a partial ACK or an ACK that acks recovery point
> (high_seq), tp->delivered is incremented by (cumulatively acked -
> #dupacks) in tcp_remove_reno_sacks()
...and this won't happen correctly anymore after RTO (since non-SACK
won't keep #dupacks due to the discontinuity). Thus we end up adding
cumulatively acked - 0 to tp->delivered on those ACKs.
> therefore tp->delivered is tracking the # of packets delivered
> (sacked, acked, DUPACK'd) with the most information it could have
> inferred.
Since you didn't answer any of my questions about tp->delivered directly,
let me rephrase them to this example (non-SACK, of course):
4 segments outstanding. RTO recovery underway (lost_out=4, sacked_out=0).
Cwnd = 2 so the sender rexmits 2 out of 4. We get cumulative ACK for
three segments. How much should tp->delivered be incremented? 2 or 3?
...I think 2 is the right answer.
> From congestion control's perspective, it cares about the delivery
> information (e.g. how much), not the sequences (what or how).
I guess you must have missed my point. I'm talking about "how much"
whole the time. It's just about when can we account for it (and when not).
> I am pointing out that
>
> 1) your fix may fix one corner CC packet accounting issue in the
> non-SACK and CA_Loss
> 2) but it does not fix the other (major) CC packet accounting issue in
> the SACK case
You say major but it's major if and only if one is using one of the
affected cc modules which were not that many and IMHO not very mainstream
anyway (check my list from the previous email, not that I'd mind if they'd
get fixed too). The rest of the cc modules do not seem to use the incorrect
value even if some of them have the pkts_acked callback.
Other CC packet accounting (regardless of SACK) is based on tp->delivered
and tp->delivered is NOT similarly miscounted with SACK because the logic
depend on !TCPCB_SACKED_ACKED (that fails only if we have very high ACK
loss).
> 3) it breaks the dynamic dupthresh / reordering in the non-SACK case
> as tcp_check_reno_reordering requires strictly cum. ack.
I think that the current non-SACK reordering detection logic is not that
sound after RTO (but I'm yet to prove this). ...There seems to be some
failure modes which is why I even thought of disabling the whole thing
for non-SACK RTO recoveries and as it now seems you do care more about
non-SACK than you initial claimed, I might even have motivation to fix
more those corners rather than the worst bugs only ;-). ...But I'll make
the tp->delivered fix only in this patch to avoid any change this part of
the code.
> Therefore I prefer
> a) using tp->delivered to fix (1)(2)
> b) perhaps improving tp->delivered SACK emulation code in the non-SACK case
Somehow I get an impression that you might assume/say here that
tp->delivered is all correct for non-SACK? ...It isn't without a fix.
Therefore a) won't help any for non-SACK. Tp->delivered for non-SACK is
currently (mis!)calculated in tcp_remove_reno_sacks because the incorrect
pkts_acked is fed to it. ...Thus, b) is an intermediate step in the
miscalculation chain I'm fixing with this change. B) resolves also (1)
without additional changes to logic!
I've included below the updated change that only fixes tp->delivered
calculation (not tested besides compiling). ...But I think it's worse than
my previous version because tcp_remove_reno_sacks then assumes
non-sensical L|S skbs (but there seem to be other, worse limitations in
sacked_out imitation after RTO because we've all skbs marked with L-bit so
it's not that big deal for me as most of the that imitation code is no-op
anyway after RTO).
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 9a1b3c1..e11748d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1868,7 +1868,6 @@ static void tcp_remove_reno_sacks(struct sock *sk, int acked)
if (acked > 0) {
/* One ACK acked hole. The rest eat duplicate ACKs. */
- tp->delivered += max_t(int, acked - tp->sacked_out, 1);
if (acked - 1 >= tp->sacked_out)
tp->sacked_out = 0;
else
@@ -1878,6 +1877,12 @@ static void tcp_remove_reno_sacks(struct sock *sk, int acked)
tcp_verify_left_out(tp);
}
+static void tcp_update_reno_delivered(struct tcp_sock *tp, int acked)
+{
+ if (acked > 0)
+ tp->delivered += max_t(int, acked - tp->sacked_out, 1);
+}
+
static inline void tcp_reset_reno_sack(struct tcp_sock *tp)
{
tp->sacked_out = 0;
@@ -3027,6 +3032,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
long seq_rtt_us = -1L;
long ca_rtt_us = -1L;
u32 pkts_acked = 0;
+ u32 rexmit_acked = 0;
+ u32 newdata_acked = 0;
u32 last_in_flight = 0;
bool rtt_update;
int flag = 0;
@@ -3056,8 +3063,10 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
}
if (unlikely(sacked & TCPCB_RETRANS)) {
- if (sacked & TCPCB_SACKED_RETRANS)
+ if (sacked & TCPCB_SACKED_RETRANS) {
tp->retrans_out -= acked_pcount;
+ rexmit_acked += acked_pcount;
+ }
flag |= FLAG_RETRANS_DATA_ACKED;
} else if (!(sacked & TCPCB_SACKED_ACKED)) {
last_ackt = skb->skb_mstamp;
@@ -3070,6 +3079,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
reord = start_seq;
if (!after(scb->end_seq, tp->high_seq))
flag |= FLAG_ORIG_SACK_ACKED;
+ else
+ newdata_acked += acked_pcount;
}
if (sacked & TCPCB_SACKED_ACKED) {
@@ -3151,6 +3162,9 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
}
if (tcp_is_reno(tp)) {
+ tcp_update_reno_delivered(tp, icsk->icsk_ca_state != TCP_CA_Loss ?
+ pkts_acked :
+ rexmit_acked + newdata_acked);
tcp_remove_reno_sacks(sk, pkts_acked);
} else {
int delta;
^ permalink raw reply related
* [PATCH net-next] net: cavium: use module_pci_driver to simplify the code
From: Wei Yongjun @ 2018-03-28 12:51 UTC (permalink / raw)
To: Radoslaw Biernacki, Aleksey Makarov, Philippe Ombredanne,
Jan Glauber
Cc: Wei Yongjun, netdev, kernel-janitors
Use the module_pci_driver() macro to make the code simpler
by eliminating module_init and module_exit calls.
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
drivers/net/ethernet/cavium/common/cavium_ptp.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/cavium/common/cavium_ptp.c b/drivers/net/ethernet/cavium/common/cavium_ptp.c
index d59497a..6aeb104 100644
--- a/drivers/net/ethernet/cavium/common/cavium_ptp.c
+++ b/drivers/net/ethernet/cavium/common/cavium_ptp.c
@@ -336,18 +336,7 @@ static void cavium_ptp_remove(struct pci_dev *pdev)
.remove = cavium_ptp_remove,
};
-static int __init cavium_ptp_init_module(void)
-{
- return pci_register_driver(&cavium_ptp_driver);
-}
-
-static void __exit cavium_ptp_cleanup_module(void)
-{
- pci_unregister_driver(&cavium_ptp_driver);
-}
-
-module_init(cavium_ptp_init_module);
-module_exit(cavium_ptp_cleanup_module);
+module_pci_driver(cavium_ptp_driver);
MODULE_DESCRIPTION(DRV_NAME);
MODULE_AUTHOR("Cavium Networks <support@cavium.com>");
^ permalink raw reply related
* Re: [RFC v3 net-next 13/18] net/sched: Introduce the TBS Qdisc
From: Henrik Austad @ 2018-03-28 13:07 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Jesus Sanchez-Palencia, netdev, jhs, xiyou.wangcong, jiri,
vinicius.gomes, richardcochran, anna-maria, John Stultz,
levi.pearson, edumazet, willemb, mlichvar
In-Reply-To: <alpine.DEB.2.21.1803280808490.3247@nanos.tec.linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 13445 bytes --]
On Wed, Mar 28, 2018 at 09:48:05AM +0200, Thomas Gleixner wrote:
> Jesus,
Thomas, Jesus,
> On Tue, 27 Mar 2018, Jesus Sanchez-Palencia wrote:
> > On 03/25/2018 04:46 AM, Thomas Gleixner wrote:
> > > This is missing right now and you want to get that right from the very
> > > beginning. Duct taping it on the interface later on is a bad idea.
> >
> > Agreed that this is needed. On the SO_TXTIME + tbs proposal, I believe it's been
> > covered by the (per-packet) SCM_DROP_IF_LATE. Do you think we need a different
> > mechanism for expressing that?
>
> Uuurgh. No. DROP_IF_LATE is just crap to be honest.
>
> There are two modes:
>
> 1) Send at the given TX time (Explicit mode)
>
> 2) Send before given TX time (Deadline mode)
>
> There is no need to specify 'drop if late' simply because if the message is
> handed in past the given TX time, it's too late by definition. What you are
> trying to implement is a hybrid of TSN and general purpose (not time aware)
> networking in one go. And you do that because your overall design is not
> looking at the big picture. You designed from a given use case assumption
> and tried to fit other things into it with duct tape.
Yes, +1 to this. The whole point of bandwidth reservation is to not drop
frames, you should never, ever miss a deadline, if you do, then your
admission tests are inadequate.
> > > So you really want a way for the application to query the timing
> > > constraints and perhaps other properties of the channel it connects
> > > to. And you want that now before the first application starts to use the
> > > new ABI. If the application developer does not use it, you still have to
> > > fix the application, but you have to fix it because the developer was a
> > > lazy bastard and not because the design was bad. That's a major
> > > difference.
> >
> > Ok, this is something that we have considered in the past, but then the feedback
> > here drove us onto a different direction. The overall input we got here was that
> > applications would have to be adjusted or that userspace would have to handle
> > the coordination between applications somehow (e.g.: a daemon could be developed
> > separately to accommodate the fully dynamic use-cases, etc).
>
> The only thing which will happen is that you get applications which require
> to control the full interface themself because they are so important and
> the only ones which get it right. Good luck with fixing them up.
>
> That extra daemon if it ever surfaces will be just a PITA. Think about
> 20khz control loops. Do you really want queueing, locking, several context
> switches and priority configuration nightmares in such a scenario?
> Definitely not! You want a fast channel directly to the root qdisc which
> takes care of getting it out at the right point, which might be immediate
> handover if the adapter supports hw scheduling.
>
> > This is a new requirement for the entire discussion.
> > If I'm not missing anything, however, underutilization of the time slots is only
> > a problem:
> >
> > 1) for the fully dynamic use-cases and;
> > 2) because now you are designing applications in terms of time slices, right?
>
> No. It's a general problem. I'm not designing applications in terms of time
> slices. Time slices are a fundamental property of TSN. Whether you use them
> for explicit scheduling or bandwidth reservation or make them flat does not
> matter.
>
> The application does not necessarily need to know about the time
> constraints at all. But if it wants to use timed scheduling then it better
> does know about them.
yep, +1 in a lot of A/V cases here, the application will have to know about
presentation_time, and the delay through the network stack should be "low
and deterministic", but apart from that, the application shouldn't have to
care about SO_TXTIME and what other applications may or may not do.
> > We have not thought of making any of the proposed qdiscs capable of (optionally)
> > adjusting the "time slices", but mainly because this is not a problem we had
> > here before. Our assumption was that per-port Tx schedules would only be used
> > for static systems. In other words, no, we didn't think that re-balancing the
> > slots was a requirement, not even for 'taprio'.
>
> Sigh. Utilization is not something entirely new in the network space. I'm
> not saying that this needs to be implemented right away, but designing it
> in a way which forces underutilization is just wrong.
>
> > > Coming back to the overall scheme. If you start upfront with a time slice
> > > manager which is designed to:
> > >
> > > - Handle multiple channels
> > >
> > > - Expose the time constraints, properties per channel
> > >
> > > then you can fit all kind of use cases, whether designed by committee or
> > > not. You can configure that thing per node or network wide. It does not
> > > make a difference. The only difference are the resulting constraints.
> >
> >
> > Ok, and I believe the above was covered by what we had proposed before, unless
> > what you meant by time constraints is beyond the configured port schedule.
> >
> > Are you suggesting that we'll need to have a kernel entity that is not only
> > aware of the current traffic classes 'schedule', but also of the resources that
> > are still available for new streams to be accommodated into the classes? Putting
> > it differently, is the TAS you envision just an entity that runs a schedule, or
> > is it a time-aware 'orchestrator'?
>
> In the first place its something which runs a defined schedule.
>
> The accomodation for new streams is required, but not necessarily at the
> root qdisc level. That might be a qdisc feeding into it.
>
> Assume you have a bandwidth reservation, aka time slot, for audio. If your
> audio related qdisc does deadline scheduling then you can add new streams
> to it up to the point where it's not longer able to fit.
>
> The only thing which might be needed at the root qdisc is the ability to
> utilize unused time slots for other purposes, but that's not required to be
> there in the first place as long as its designed in a way that it can be
> added later on.
>
> > > So lets look once more at the picture in an abstract way:
> > >
> > > [ NIC ]
> > > |
> > > [ Time slice manager ]
> > > | |
> > > [ Ch 0 ] ... [ Ch N ]
> > >
> > > So you have a bunch of properties here:
> > >
> > > 1) Number of Channels ranging from 1 to N
> > >
> > > 2) Start point, slice period and slice length per channel
> >
> > Ok, so we agree that a TAS entity is needed. Assuming that channels are traffic
> > classes, do you have something else in mind other than a new root qdisc?
>
> Whatever you call it, the important point is that it is the gate keeper to
> the network adapter and there is no way around it. It fully controls the
> timed schedule how simple or how complex it may be.
>
> > > 3) Queueing modes assigned per channel. Again that might be anything from
> > > 'feed through' over FIFO, PRIO to more complex things like EDF.
> > >
> > > The queueing mode can also influence properties like the meaning of the
> > > TX time, i.e. strict or deadline.
> >
> >
> > Ok, but how are the queueing modes assigned / configured per channel?
> >
> > Just to make sure we re-visit some ideas from the past:
> >
> > * TAS:
> >
> > The idea we are currently exploring is to add a "time-aware", priority based
> > qdisc, that also exposes the Tx queues available and provides a mechanism for
> > mapping priority <-> traffic class <-> Tx queues in a similar fashion as
> > mqprio. We are calling this qdisc 'taprio', and its 'tc' cmd line would be:
> >
> > $ $ tc qdisc add dev ens4 parent root handle 100 taprio num_tc 4 \
> > map 2 2 1 0 3 3 3 3 3 3 3 3 3 3 3 3 \
> > queues 0 1 2 3 \
> > sched-file gates.sched [base-time <interval>] \
> > [cycle-time <interval>] [extension-time <interval>]
> >
> > <file> is multi-line, with each line being of the following format:
> > <cmd> <gate mask> <interval in nanoseconds>
> >
> > Qbv only defines one <cmd>: "S" for 'SetGates'
> >
> > For example:
> >
> > S 0x01 300
> > S 0x03 500
> >
> > This means that there are two intervals, the first will have the gate
> > for traffic class 0 open for 300 nanoseconds, the second will have
> > both traffic classes open for 500 nanoseconds.
>
> To accomodate stuff like control systems you also need a base line, which
> is not expressed as interval. Otherwise you can't schedule network wide
> explicit plans. That's either an absolute network-time (TAI) time stamp or
> an offset to a well defined network-time (TAI) time stamp, e.g. start of
> epoch or something else which is agreed on. The actual schedule then fast
> forwards past now (TAI) and sets up the slots from there. That makes node
> hotplug possible as well.
Ok, so this is perhaps a bit of a sidetrack, but based on other discussions
in this patch-series, does it really make sense to discuss anything *but*
TAI?
If you have a TSN-stream (or any other time-sensitive way of prioritizing
frames based on time), then the network is going to be PTP synched anyway,
and all the rest of the network is going to operate on PTP-time. Why even
bother adding CLOCK_REALTIME and CLOCK_MONOTONIC to the discussion? Sure,
use CLOCK_REALTIME locally and sync that to TAI, but the kernel should
worry about ptp-time _for_that_adapter_, and we should make it pretty
obvious to userspace that if you want to specify tx-time, then there's this
thing called 'PTP' and it rules this domain. My $0.02 etc
> Btw, it's not only control systems. Think about complex multi source A/V
> streams. They are reality in recording and life mixing and looking at the
> timing constraints of such scenarios, collision avoidance is key there. So
> you want to be able to do network wide traffic orchestration.
Yep, and if are too bursty, the network is free to drop your frames, which
is not desired.
> > It would handle multiple channels and expose their constraints / properties.
> > Each channel also becomes a traffic class, so other qdiscs can be attached to
> > them separately.
>
> Right.
I don't think you need a separate qdisc for each channel, if you describe a
channel with
- period (what AVB calls observation interval)
- max data
- deadline
you should be able to keep a sorted rb-tree and handle that pretty
efficiently. Or perhaps I'm completely missing the mark here. If so, my
apologies
> > So, in summary, because our entire design is based on qdisc interfaces, what we
> > had proposed was a root qdisc (the time slice manager, as you put) that allows
> > for other qdiscs to be attached to each channel. The inner qdiscs define the
> > queueing modes for each channel, and tbs is just one of those modes. I
> > understand now that you want to allow for fully dynamic use-cases to be
> > supported as well, which we hadn't covered with our TAS proposal before because
> > we hadn't envisioned it being used for these systems' design.
>
> Yes, you have the root qdisc, which is in charge of the overall scheduling
> plan, how complex or not it is defined does not matter. It exposes traffic
> classes which have properties defined by the configuration.
>
> The qdiscs which are attached to those traffic classes can be anything
> including:
>
> - Simple feed through (Applications are time contraints aware and set the
> exact schedule). qdisc has admission control.
>
> - Deadline aware qdisc to handle e.g. A/V streams. Applications are aware
> of time constraints and provide the packet deadline. qdisc has admission
> control. This can be a simple first comes, first served scheduler or
> something like EDF which allows optimized utilization. The qdisc sets
> the TX time depending on the deadline and feeds into the root.
As a small nitpick, it would make more sense to do a laxity-approach here,
both for explicit mode and deadline-mode. We know the size of the frame to
send, we know the outgoing rate, so keep a ready-queue sorted based on
laxity
laxity = absolute_deadline - (size / outgoing_rate)
Also, given that we use a *single* tx-queue for time-triggered
transmission, this boils down to a uniprocessor equivalent and we have a
lot of func real-time scheduling academia to draw from.
This could then probably handle both of the above (Direct + deadline), but
that's implementatino specific I guess.
> - FIFO/PRIO/XXX for general traffic. Applications do not know anything
> about timing constraints. These qdiscs obviously have neither admission
> control nor do they set a TX time. The root qdisc just pulls from there
> when the assigned time slot is due or if it (optionally) decides to use
> underutilized time slots from other classes.
>
> - .... Add your favourite scheduling mode(s).
Just give it sub-qdiscs and offload enqueue/dequeue to those I suppose.
--
Henrik Austad
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH] test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()
From: Daniel Borkmann @ 2018-03-28 13:25 UTC (permalink / raw)
To: Dan Carpenter, Alexei Starovoitov, Yonghong Song; +Cc: netdev, kernel-janitors
In-Reply-To: <20180328114836.GD29050@mwanda>
On 03/28/2018 01:48 PM, Dan Carpenter wrote:
> The skb_segment() function returns error pointers on error. It never
> returns NULL.
>
> Fixes: 76db8087c4c9 ("net: bpf: add a test for skb_segment in test_bpf module")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Patch is directly for net-next, thanks!
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: 4.14.29 - tcp_push() - null skb's cb dereference
From: Eric Dumazet @ 2018-03-28 13:38 UTC (permalink / raw)
To: Krzysztof Blaszkowski, netdev; +Cc: Greg Kroah-Hartman, David S. Miller
In-Reply-To: <1522234312.25303.15.camel@home.com.pl>
On 03/28/2018 03:51 AM, Krzysztof Blaszkowski wrote:
> Hi,
>
> I noticed a kernel bug report like below:
>
> [95576.826393] BUG: unable to handle kernel NULL pointer dereference at
> 0000000000000038
> [95576.834296] IP: tcp_push+0x3d/0x110
> [95576.837829] PGD 2c8474067 P4D 2c8474067 PUD 1119cf067 PMD 0
> [95576.843536] Oops: 0002 [#1] SMP NOPTI
> [95576.847247] CPU: 3 PID: 1682 Comm: nginx Not tainted 4.14.29 #1
> [95576.854421] Hardware name: PC-FACTORY empty/Tyan Transport GT24-
> B3992-E, BIOS 'V1.06.B10 ' 06/23/2009
> [95576.863678] task: ffff9e4f150b2580 task.stack: ffffb21401a50000
> [95576.869641] RIP: 0010:tcp_push+0x3d/0x110
> [95576.873692] RSP: 0018:ffffb21401a53be0 EFLAGS: 00010246
> [95576.878959] RAX: 0000000000000000 RBX: 0000000000009310 RCX:
> 0000000000000000
> [95576.886133] RDX: 0000000000000001 RSI: 0000000000000040 RDI:
> ffff9e4e79871f00
> [95576.893306] RBP: ffffb21401a53c78 R08: 00000000000065d0 R09:
> ffff9e4e79872048
> [95576.900479] R10: 00000000000005a8 R11: 0000000000000000 R12:
> 00000000ffffffe0
> [95576.907652] R13: ffffb21401a53cf8 R14: ffff9e4e79871f00 R15:
> ffff9e4f68727100
> [95576.914824] FS: 00007f683b4e1700(0000) GS:ffff9e50732c0000(0000)
> knlGS:0000000000000000
> [95576.922953] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [95576.928737] CR2: 0000000000000038 CR3: 000000020aa0e000 CR4:
> 00000000000006e0
> [95576.935908] Call Trace:
> [95576.938403] ? tcp_sendmsg_locked+0x60a/0xd90
> [95576.942872] tcp_sendmsg+0x27/0x40
> [95576.946386] inet_sendmsg+0x2c/0xa0
> [95576.949987] sock_sendmsg+0x33/0x40
> [95576.953587] sock_write_iter+0x76/0xd0
> [95576.957448] do_iter_readv_writev+0x108/0x160
> [95576.961917] do_iter_write+0x82/0x190
> [95576.965690] vfs_writev+0xbb/0x120
> [95576.969203] ? ep_poll+0x240/0x3f0
> [95576.972714] do_writev+0x4d/0xd0
> [95576.976052] ? do_writev+0x4d/0xd0
> [95576.979564] SyS_writev+0xb/0x10
> [95576.982905] do_syscall_64+0x5a/0x110
> [95576.986678] entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> [95576.991838] RIP: 0033:0x7f6839e56170
> [95576.995523] RSP: 002b:00007ffe34c9cf48 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000014
> [95577.003265] RAX: ffffffffffffffda RBX: 0000000004b622c0 RCX:
> 00007f6839e56170
> [95577.010507] RDX: 0000000000000017 RSI: 00007ffe34c9cfd0 RDI:
> 0000000000000012
> [95577.017749] RBP: 00007ffe34c9cfb0 R08: 00000000004b1c2f R09:
> 00007ffe34c9d130
> [95577.024989] R10: 000000000001d6c2 R11: 0000000000000246 R12:
> 0000000002f3eec0
> [95577.032228] R13: 000000007fffefff R14: 0000000000000000 R15:
> 0000000002f3eec0
> [95577.039469] Code: 01 00 00 4c 8d 8f 48 01 00 00 41 89 d2 41 89 f3 89
> ca b9 00 00 00 00 49 39 c1 48 0f 44 c1 41 81 e3 00 80 00 00 0f 85 ac 00
> 00 00 <80> 48 38 08 8b 8f 64 06 00 00 89 8f 6c 06 00 00 83 e6 01 74 0c
> [95577.058617] RIP: tcp_push+0x3d/0x110 RSP: ffffb21401a53be0
> [95577.064208] CR2: 0000000000000038
> [95577.068056] ---[ end trace 19bfaf872fd3ef10 ]--
>
>
>
> further report analysis ended up in net/ipv4/tcp.c :
>
>
> (gdb) list *(tcp_push+0x3d)
> 0xfd is in tcp_push (/data/work/linux-4.14.29/net/ipv4/tcp.c:630).
> 625 }
> 626 EXPORT_SYMBOL(tcp_ioctl);
> 627
> 628 static inline void tcp_mark_push(struct tcp_sock *tp, struct
> sk_buff *skb)
> 629 {
> 630 TCP_SKB_CB(skb)->tcp_flags |= TCPHDR_PSH;
> 631 tp->pushed_seq = tp->write_seq;
> 632 }
>
>
> and tcp_mark_push() is inlined in tcp_push() indeed.
>
> Looking forward to seeing rationale for tcp_write_queue_tail(sk)
> returning null skb - assuming the above is correct.
>
> Thanks
This has been reported 5 or 6 times already :/
For some reason the fix is missing from 4.14 stable tree.
https://patchwork.ozlabs.org/patch/886324/
Note this patch was needed even before a27fid7a8ed38 (tcp: purge write queue upon RST)
since tcp disconnect would have caused the same issue.
Thanks.
^ permalink raw reply
* Re: [PATCH] samples/bpf: fix spelling mistake: "revieve" -> "receive"
From: Daniel Borkmann @ 2018-03-28 13:38 UTC (permalink / raw)
To: Colin King, Alexei Starovoitov, netdev; +Cc: kernel-janitors, linux-kernel
In-Reply-To: <20180328100701.13289-1-colin.king@canonical.com>
On 03/28/2018 12:07 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistake in error message text
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied to bpf-next, thanks Colin!
^ permalink raw reply
* Re: 4.14.29 - tcp_push() - null skb's cb dereference
From: Krzysztof Blaszkowski @ 2018-03-28 13:43 UTC (permalink / raw)
To: Eric Dumazet, netdev; +Cc: Greg Kroah-Hartman, David S. Miller
In-Reply-To: <14343877-a2a0-215e-e052-2dfb7e4c4b09@gmail.com>
On Wed, 2018-03-28 at 06:38 -0700, Eric Dumazet wrote:
>
> On 03/28/2018 03:51 AM, Krzysztof Blaszkowski wrote:
> >
> > Hi,
> >
> > I noticed a kernel bug report like below:
> >
> > [95576.826393] BUG: unable to handle kernel NULL pointer
> > dereference at
> > 0000000000000038
> > [95576.834296] IP: tcp_push+0x3d/0x110
> > [95576.837829] PGD 2c8474067 P4D 2c8474067 PUD 1119cf067 PMD 0
> > [95576.843536] Oops: 0002 [#1] SMP NOPTI
> > [95576.847247] CPU: 3 PID: 1682 Comm: nginx Not tainted 4.14.29 #1
> > [95576.854421] Hardware name: PC-FACTORY empty/Tyan Transport GT24-
> > B3992-E, BIOS 'V1.06.B10 ' 06/23/2009
> > [95576.863678] task: ffff9e4f150b2580 task.stack: ffffb21401a50000
> > [95576.869641] RIP: 0010:tcp_push+0x3d/0x110
> > [95576.873692] RSP: 0018:ffffb21401a53be0 EFLAGS: 00010246
> > [95576.878959] RAX: 0000000000000000 RBX: 0000000000009310 RCX:
> > 0000000000000000
> > [95576.886133] RDX: 0000000000000001 RSI: 0000000000000040 RDI:
> > ffff9e4e79871f00
> > [95576.893306] RBP: ffffb21401a53c78 R08: 00000000000065d0 R09:
> > ffff9e4e79872048
> > [95576.900479] R10: 00000000000005a8 R11: 0000000000000000 R12:
> > 00000000ffffffe0
> > [95576.907652] R13: ffffb21401a53cf8 R14: ffff9e4e79871f00 R15:
> > ffff9e4f68727100
> > [95576.914824] FS: 00007f683b4e1700(0000)
> > GS:ffff9e50732c0000(0000)
> > knlGS:0000000000000000
> > [95576.922953] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [95576.928737] CR2: 0000000000000038 CR3: 000000020aa0e000 CR4:
> > 00000000000006e0
> > [95576.935908] Call Trace:
> > [95576.938403] ? tcp_sendmsg_locked+0x60a/0xd90
> > [95576.942872] tcp_sendmsg+0x27/0x40
> > [95576.946386] inet_sendmsg+0x2c/0xa0
> > [95576.949987] sock_sendmsg+0x33/0x40
> > [95576.953587] sock_write_iter+0x76/0xd0
> > [95576.957448] do_iter_readv_writev+0x108/0x160
> > [95576.961917] do_iter_write+0x82/0x190
> > [95576.965690] vfs_writev+0xbb/0x120
> > [95576.969203] ? ep_poll+0x240/0x3f0
> > [95576.972714] do_writev+0x4d/0xd0
> > [95576.976052] ? do_writev+0x4d/0xd0
> > [95576.979564] SyS_writev+0xb/0x10
> > [95576.982905] do_syscall_64+0x5a/0x110
> > [95576.986678] entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> > [95576.991838] RIP: 0033:0x7f6839e56170
> > [95576.995523] RSP: 002b:00007ffe34c9cf48 EFLAGS: 00000246
> > ORIG_RAX:
> > 0000000000000014
> > [95577.003265] RAX: ffffffffffffffda RBX: 0000000004b622c0 RCX:
> > 00007f6839e56170
> > [95577.010507] RDX: 0000000000000017 RSI: 00007ffe34c9cfd0 RDI:
> > 0000000000000012
> > [95577.017749] RBP: 00007ffe34c9cfb0 R08: 00000000004b1c2f R09:
> > 00007ffe34c9d130
> > [95577.024989] R10: 000000000001d6c2 R11: 0000000000000246 R12:
> > 0000000002f3eec0
> > [95577.032228] R13: 000000007fffefff R14: 0000000000000000 R15:
> > 0000000002f3eec0
> > [95577.039469] Code: 01 00 00 4c 8d 8f 48 01 00 00 41 89 d2 41 89
> > f3 89
> > ca b9 00 00 00 00 49 39 c1 48 0f 44 c1 41 81 e3 00 80 00 00 0f 85
> > ac 00
> > 00 00 <80> 48 38 08 8b 8f 64 06 00 00 89 8f 6c 06 00 00 83 e6 01 74
> > 0c
> > [95577.058617] RIP: tcp_push+0x3d/0x110 RSP: ffffb21401a53be0
> > [95577.064208] CR2: 0000000000000038
> > [95577.068056] ---[ end trace 19bfaf872fd3ef10 ]--
> >
> >
> >
> > further report analysis ended up in net/ipv4/tcp.c :
> >
> >
> > (gdb) list *(tcp_push+0x3d)
> > 0xfd is in tcp_push (/data/work/linux-4.14.29/net/ipv4/tcp.c:630).
> > 625 }
> > 626 EXPORT_SYMBOL(tcp_ioctl);
> > 627
> > 628 static inline void tcp_mark_push(struct tcp_sock *tp,
> > struct
> > sk_buff *skb)
> > 629 {
> > 630 TCP_SKB_CB(skb)->tcp_flags |= TCPHDR_PSH;
> > 631 tp->pushed_seq = tp->write_seq;
> > 632 }
> >
> >
> > and tcp_mark_push() is inlined in tcp_push() indeed.
> >
> > Looking forward to seeing rationale for tcp_write_queue_tail(sk)
> > returning null skb - assuming the above is correct.
> >
> > Thanks
>
> This has been reported 5 or 6 times already :/
>
> For some reason the fix is missing from 4.14 stable tree.
>
> https://patchwork.ozlabs.org/patch/886324/
>
> Note this patch was needed even before a27fid7a8ed38 (tcp: purge
> write queue upon RST)
> since tcp disconnect would have caused the same issue.
>
>
>
> Thanks.
I see. Many thanks.
>
>
^ permalink raw reply
* Re: [PATCH bpf-next]: add sock_ops R/W access to ipv4 tos
From: Daniel Borkmann @ 2018-03-28 13:47 UTC (permalink / raw)
To: Nikita V. Shirokov, brakmo, ast, netdev; +Cc: kernel-team
In-Reply-To: <20180326153657.2229959-1-tehnerd@fb.com>
On 03/26/2018 05:36 PM, Nikita V. Shirokov wrote:
> bpf: Add sock_ops R/W access to ipv4 tos
>
> Sample usage for tos:
>
> bpf_getsockopt(skops, SOL_IP, IP_TOS, &v, sizeof(v))
>
> where skops is a pointer to the ctx (struct bpf_sock_ops).
>
> Signed-off-by: Nikita V. Shirokov <tehnerd@fb.com>
> ---
> net/core/filter.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 00c711c..afd8255 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3462,6 +3462,27 @@ BPF_CALL_5(bpf_setsockopt, struct bpf_sock_ops_kern *, bpf_sock,
> ret = -EINVAL;
> }
> #ifdef CONFIG_INET
> + } else if (level == SOL_IP) {
> + if (optlen != sizeof(int) || sk->sk_family != AF_INET)
> + return -EINVAL;
> +
> + val = *((int *)optval);
> + /* Only some options are supported */
> + switch (optname) {
> + case IP_TOS:
> + if (val < -1 || val > 0xff) {
> + ret = -EINVAL;
> + } else {
> + struct inet_sock *inet = inet_sk(sk);
> +
> + if (val == -1)
> + val = 0;
> + inet->tos = val;
Should this not have the exact same semantics given the helper resembles
the normal setsockopt? do_ip_setsockopt() does the following when setting
IP_TOS:
case IP_TOS: /* This sets both TOS and Precedence */
if (sk->sk_type == SOCK_STREAM) {
val &= ~INET_ECN_MASK;
val |= inet->tos & INET_ECN_MASK;
}
if (inet->tos != val) {
inet->tos = val;
sk->sk_priority = rt_tos2priority(val);
sk_dst_reset(sk);
}
break;
E.g. why we don't need to set sk->sk_priority as well or reset the dst
entry here?
> + }
> + break;
> + default:
> + ret = -EINVAL;
> + }
> #if IS_ENABLED(CONFIG_IPV6)
> } else if (level == SOL_IPV6) {
> if (optlen != sizeof(int) || sk->sk_family != AF_INET6)
> @@ -3561,6 +3582,20 @@ BPF_CALL_5(bpf_getsockopt, struct bpf_sock_ops_kern *, bpf_sock,
> } else {
> goto err_clear;
> }
> + } else if (level == SOL_IP) {
> + struct inet_sock *inet = inet_sk(sk);
> +
> + if (optlen != sizeof(int) || sk->sk_family != AF_INET)
> + goto err_clear;
> +
> + /* Only some options are supported */
> + switch (optname) {
> + case IP_TOS:
> + *((int *)optval) = (int)inet->tos;
This part is fine though, same as in do_ip_getsockopt().
> + break;
> + default:
> + goto err_clear;
> + }
> #if IS_ENABLED(CONFIG_IPV6)
> } else if (level == SOL_IPV6) {
> struct ipv6_pinfo *np = inet6_sk(sk);
>
^ permalink raw reply
* Re: [PATCH v7 bpf-next 06/10] tracepoint: compute num_args at build time
From: Mathieu Desnoyers @ 2018-03-28 13:49 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Daniel Borkmann, Linus Torvalds, Peter Zijlstra,
rostedt, netdev, kernel-team, linux-api
In-Reply-To: <20180328021105.4061744-7-ast@fb.com>
----- On Mar 27, 2018, at 10:11 PM, Alexei Starovoitov ast@fb.com wrote:
> From: Alexei Starovoitov <ast@kernel.org>
>
> compute number of arguments passed into tracepoint
> at compile time and store it as part of 'struct tracepoint'.
> The number is necessary to check safety of bpf program access that
> is coming in subsequent patch.
Hi Alexei,
Given that only eBPF needs this parameter count, we can move
it to the struct bpf_raw_event_map newly introduced by Steven,
right ? This would reduce bloat of struct tracepoint. For instance,
we don't need to keep this count around when eBPF is configured
out.
Thanks,
Mathieu
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> include/linux/tracepoint-defs.h | 1 +
> include/linux/tracepoint.h | 12 ++++++------
> include/trace/define_trace.h | 14 +++++++-------
> 3 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
> index 64ed7064f1fa..39a283c61c51 100644
> --- a/include/linux/tracepoint-defs.h
> +++ b/include/linux/tracepoint-defs.h
> @@ -33,6 +33,7 @@ struct tracepoint {
> int (*regfunc)(void);
> void (*unregfunc)(void);
> struct tracepoint_func __rcu *funcs;
> + u32 num_args;
> };
>
> #endif
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index c94f466d57ef..c92f4adbc0d7 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -230,18 +230,18 @@ extern void syscall_unregfunc(void);
> * structures, so we create an array of pointers that will be used for iteration
> * on the tracepoints.
> */
> -#define DEFINE_TRACE_FN(name, reg, unreg) \
> +#define DEFINE_TRACE_FN(name, reg, unreg, num_args) \
> static const char __tpstrtab_##name[] \
> __attribute__((section("__tracepoints_strings"))) = #name; \
> struct tracepoint __tracepoint_##name \
> __attribute__((section("__tracepoints"))) = \
> - { __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL };\
> + { __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL, num_args };\
> static struct tracepoint * const __tracepoint_ptr_##name __used \
> __attribute__((section("__tracepoints_ptrs"))) = \
> &__tracepoint_##name;
>
> -#define DEFINE_TRACE(name) \
> - DEFINE_TRACE_FN(name, NULL, NULL);
> +#define DEFINE_TRACE(name, num_args) \
> + DEFINE_TRACE_FN(name, NULL, NULL, num_args);
>
> #define EXPORT_TRACEPOINT_SYMBOL_GPL(name) \
> EXPORT_SYMBOL_GPL(__tracepoint_##name)
> @@ -275,8 +275,8 @@ extern void syscall_unregfunc(void);
> return false; \
> }
>
> -#define DEFINE_TRACE_FN(name, reg, unreg)
> -#define DEFINE_TRACE(name)
> +#define DEFINE_TRACE_FN(name, reg, unreg, num_args)
> +#define DEFINE_TRACE(name, num_args)
> #define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
> #define EXPORT_TRACEPOINT_SYMBOL(name)
>
> diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
> index d9e3d4aa3f6e..96b22ace9ae7 100644
> --- a/include/trace/define_trace.h
> +++ b/include/trace/define_trace.h
> @@ -25,7 +25,7 @@
>
> #undef TRACE_EVENT
> #define TRACE_EVENT(name, proto, args, tstruct, assign, print) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef TRACE_EVENT_CONDITION
> #define TRACE_EVENT_CONDITION(name, proto, args, cond, tstruct, assign, print) \
> @@ -39,24 +39,24 @@
> #undef TRACE_EVENT_FN
> #define TRACE_EVENT_FN(name, proto, args, tstruct, \
> assign, print, reg, unreg) \
> - DEFINE_TRACE_FN(name, reg, unreg)
> + DEFINE_TRACE_FN(name, reg, unreg, COUNT_ARGS(args))
>
> #undef TRACE_EVENT_FN_COND
> #define TRACE_EVENT_FN_COND(name, proto, args, cond, tstruct, \
> assign, print, reg, unreg) \
> - DEFINE_TRACE_FN(name, reg, unreg)
> + DEFINE_TRACE_FN(name, reg, unreg, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT
> #define DEFINE_EVENT(template, name, proto, args) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT_FN
> #define DEFINE_EVENT_FN(template, name, proto, args, reg, unreg) \
> - DEFINE_TRACE_FN(name, reg, unreg)
> + DEFINE_TRACE_FN(name, reg, unreg, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT_PRINT
> #define DEFINE_EVENT_PRINT(template, name, proto, args, print) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT_CONDITION
> #define DEFINE_EVENT_CONDITION(template, name, proto, args, cond) \
> @@ -64,7 +64,7 @@
>
> #undef DECLARE_TRACE
> #define DECLARE_TRACE(name, proto, args) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef TRACE_INCLUDE
> #undef __TRACE_INCLUDE
> --
> 2.9.5
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ 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