Netdev List
 help / color / mirror / Atom feed
* Re: Allow bpf_perf_event_output to access packet data
From: Lorenz Bauer @ 2018-09-11  8:37 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev
In-Reply-To: <20180910102642.7ea6da00@cakuba>

On 10 September 2018 at 09:26, Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
> The 0x20ffffffffULL will mean use the index in the map for current CPU
> (0xffffffff), and output 32 bytes of the context (0x20 << 32).  For
> networking programs context means the packet (slightly confusingly).
>
> These are the relevant defines from bpf.h:
>
> /* BPF_FUNC_perf_event_output, BPF_FUNC_perf_event_read and
>  * BPF_FUNC_perf_event_read_value flags.
>  */
> #define BPF_F_INDEX_MASK                0xffffffffULL
> #define BPF_F_CURRENT_CPU               BPF_F_INDEX_MASK
> /* BPF_FUNC_perf_event_output for sk_buff input context. */
> #define BPF_F_CTXLEN_MASK               (0xfffffULL << 32)
>
> Also check out:
>
> bpftool map event_pipe id $ID
>
> For simple way to dump the events in user space.

Thanks for pointing me in the right direction! I managed to find
samples/bpf/xdp_sample_pkts_kern.c as well, which was helpful.

My next gotcha is that perf_event_output seems to ignore the
sample_period parameter passed to perf_event_output. This is not a big
problem since I can just implement the sampling in BPF, but am I
missing something again?


-- 
Lorenz Bauer  |  Systems Engineer
25 Lavington St., London SE1 0NZ

www.cloudflare.com

^ permalink raw reply

* Re: [PATCH v3 00/15] soc: octeontx2: Add RVU admin function driver
From: Arnd Bergmann @ 2018-09-11 13:36 UTC (permalink / raw)
  To: Sunil Kovvuri
  Cc: Linux Kernel Mailing List, Olof Johansson, Linux ARM, linux-soc,
	Andrew Lunn, David Miller, Networking, sgoutham
In-Reply-To: <CA+sq2CdNRkbGrk4rHOQX1qHrRc2+=DXBscw7cD9UKrX4G5OaVg@mail.gmail.com>

On Tue, Sep 11, 2018 at 2:37 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
>
> Didn't receive any feedback for the v3 patch series over a week's time.
> Can you please pick up these patches to merge into arm-soc ?

I would still prefer to see the whole thing as part of drivers/net/
instead of drivers/soc,
and reviewed in full on the netdev side, including the parts that are
not ethernet specific.

       Arnd

^ permalink raw reply

* [PATCH net-next 3/8] devlink: Add generic parameter msix_vec_per_pf_max
From: Vasundhara Volam @ 2018-09-11  8:45 UTC (permalink / raw)
  To: davem; +Cc: michael.chan, netdev
In-Reply-To: <1536655505-14387-1-git-send-email-vasundhara-v.volam@broadcom.com>

msix_vec_per_pf_max - This param sets the number of MSIX vectors
that the device requests from the host on driver initialization.
This value is set in the device which is applicable per PF.

Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 include/net/devlink.h | 4 ++++
 net/core/devlink.c    | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 8a6063e..9619a68 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -364,6 +364,7 @@ enum devlink_param_generic_id {
 	DEVLINK_PARAM_GENERIC_ID_REGION_SNAPSHOT,
 	DEVLINK_PARAM_GENERIC_ID_HW_TC_OFFLOAD,
 	DEVLINK_PARAM_GENERIC_ID_IGNORE_ARI,
+	DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX,
 
 	/* add new param generic ids above here*/
 	__DEVLINK_PARAM_GENERIC_ID_MAX,
@@ -388,6 +389,9 @@ enum devlink_param_generic_id {
 #define DEVLINK_PARAM_GENERIC_IGNORE_ARI_NAME "ignore_ari"
 #define DEVLINK_PARAM_GENERIC_IGNORE_ARI_TYPE DEVLINK_PARAM_TYPE_BOOL
 
+#define DEVLINK_PARAM_GENERIC_MSIX_VEC_PER_PF_MAX_NAME "msix_vec_per_pf_max"
+#define DEVLINK_PARAM_GENERIC_MSIX_VEC_PER_PF_MAX_TYPE DEVLINK_PARAM_TYPE_U32
+
 #define DEVLINK_PARAM_GENERIC(_id, _cmodes, _get, _set, _validate)	\
 {									\
 	.id = DEVLINK_PARAM_GENERIC_ID_##_id,				\
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 30f32e5..fce5c1d 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2686,6 +2686,11 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
 		.name = DEVLINK_PARAM_GENERIC_IGNORE_ARI_NAME,
 		.type = DEVLINK_PARAM_GENERIC_IGNORE_ARI_TYPE,
 	},
+	{
+		.id = DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX,
+		.name = DEVLINK_PARAM_GENERIC_MSIX_VEC_PER_PF_MAX_NAME,
+		.type = DEVLINK_PARAM_GENERIC_MSIX_VEC_PER_PF_MAX_TYPE,
+	},
 };
 
 static int devlink_param_generic_verify(const struct devlink_param *param)
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next 2/8] devlink: Add generic parameter ignore_ari
From: Vasundhara Volam @ 2018-09-11  8:44 UTC (permalink / raw)
  To: davem; +Cc: michael.chan, netdev
In-Reply-To: <1536655505-14387-1-git-send-email-vasundhara-v.volam@broadcom.com>

ignore_ari - Device ignores ARI(Alternate Routing ID) capability,
even when platforms has the support and creates same number of
partitions when platform does not support ARI capability.

Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 include/net/devlink.h | 4 ++++
 net/core/devlink.c    | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index a0e9ce9..8a6063e 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -363,6 +363,7 @@ enum devlink_param_generic_id {
 	DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV,
 	DEVLINK_PARAM_GENERIC_ID_REGION_SNAPSHOT,
 	DEVLINK_PARAM_GENERIC_ID_HW_TC_OFFLOAD,
+	DEVLINK_PARAM_GENERIC_ID_IGNORE_ARI,
 
 	/* add new param generic ids above here*/
 	__DEVLINK_PARAM_GENERIC_ID_MAX,
@@ -384,6 +385,9 @@ enum devlink_param_generic_id {
 #define DEVLINK_PARAM_GENERIC_HW_TC_OFFLOAD_NAME "hw_tc_offload"
 #define DEVLINK_PARAM_GENERIC_HW_TC_OFFLOAD_TYPE DEVLINK_PARAM_TYPE_BOOL
 
+#define DEVLINK_PARAM_GENERIC_IGNORE_ARI_NAME "ignore_ari"
+#define DEVLINK_PARAM_GENERIC_IGNORE_ARI_TYPE DEVLINK_PARAM_TYPE_BOOL
+
 #define DEVLINK_PARAM_GENERIC(_id, _cmodes, _get, _set, _validate)	\
 {									\
 	.id = DEVLINK_PARAM_GENERIC_ID_##_id,				\
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 020daa1..30f32e5 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2681,6 +2681,11 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
 		.name = DEVLINK_PARAM_GENERIC_HW_TC_OFFLOAD_NAME,
 		.type = DEVLINK_PARAM_GENERIC_HW_TC_OFFLOAD_TYPE,
 	},
+	{
+		.id = DEVLINK_PARAM_GENERIC_ID_IGNORE_ARI,
+		.name = DEVLINK_PARAM_GENERIC_IGNORE_ARI_NAME,
+		.type = DEVLINK_PARAM_GENERIC_IGNORE_ARI_TYPE,
+	},
 };
 
 static int devlink_param_generic_verify(const struct devlink_param *param)
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next 0/8] bnxt_en: devlink param updates
From: Vasundhara Volam @ 2018-09-11  8:44 UTC (permalink / raw)
  To: davem; +Cc: michael.chan, netdev

This patchset adds support for 4 generic and 1 driver-specific devlink
parameters.

Also, this patchset adds support to return proper error code if
HWRM_NVM_GET/SET_VARIABLE commands return error code
HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED.

Vasundhara Volam (8):
  devlink: Add generic parameter hw_tc_offload
  devlink: Add generic parameter ignore_ari
  devlink: Add generic parameter msix_vec_per_pf_max
  devlink: Add generic parameter msix_vec_per_pf_min
  bnxt_en: Use hw_tc_offload and ignore_ari devlink parameters
  bnxt_en: return proper error when FW returns
    HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED
  bnxt_en: Use msix_vec_per_pf_max and msix_vec_per_pf_min devlink
    params.
  bnxt_en: Add a driver specific devlink parameter.

 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 92 ++++++++++++++++++++++-
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h |  8 ++
 include/net/devlink.h                             | 16 ++++
 net/core/devlink.c                                | 20 +++++
 4 files changed, 132 insertions(+), 4 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* [PATCH net-next 4/8] devlink: Add generic parameter msix_vec_per_pf_min
From: Vasundhara Volam @ 2018-09-11  8:45 UTC (permalink / raw)
  To: davem; +Cc: michael.chan, netdev
In-Reply-To: <1536655505-14387-1-git-send-email-vasundhara-v.volam@broadcom.com>

msix_vec_per_pf_min - This param sets the number of minimal MSIX
vectors required for the device initialization. This value is set
in the device which limits MSIX vectors per PF.

Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 include/net/devlink.h | 4 ++++
 net/core/devlink.c    | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 9619a68..208cd4a 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -365,6 +365,7 @@ enum devlink_param_generic_id {
 	DEVLINK_PARAM_GENERIC_ID_HW_TC_OFFLOAD,
 	DEVLINK_PARAM_GENERIC_ID_IGNORE_ARI,
 	DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX,
+	DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN,
 
 	/* add new param generic ids above here*/
 	__DEVLINK_PARAM_GENERIC_ID_MAX,
@@ -392,6 +393,9 @@ enum devlink_param_generic_id {
 #define DEVLINK_PARAM_GENERIC_MSIX_VEC_PER_PF_MAX_NAME "msix_vec_per_pf_max"
 #define DEVLINK_PARAM_GENERIC_MSIX_VEC_PER_PF_MAX_TYPE DEVLINK_PARAM_TYPE_U32
 
+#define DEVLINK_PARAM_GENERIC_MSIX_VEC_PER_PF_MIN_NAME "msix_vec_per_pf_min"
+#define DEVLINK_PARAM_GENERIC_MSIX_VEC_PER_PF_MIN_TYPE DEVLINK_PARAM_TYPE_U32
+
 #define DEVLINK_PARAM_GENERIC(_id, _cmodes, _get, _set, _validate)	\
 {									\
 	.id = DEVLINK_PARAM_GENERIC_ID_##_id,				\
diff --git a/net/core/devlink.c b/net/core/devlink.c
index fce5c1d..177c474 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2691,6 +2691,11 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
 		.name = DEVLINK_PARAM_GENERIC_MSIX_VEC_PER_PF_MAX_NAME,
 		.type = DEVLINK_PARAM_GENERIC_MSIX_VEC_PER_PF_MAX_TYPE,
 	},
+	{
+		.id = DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN,
+		.name = DEVLINK_PARAM_GENERIC_MSIX_VEC_PER_PF_MIN_NAME,
+		.type = DEVLINK_PARAM_GENERIC_MSIX_VEC_PER_PF_MIN_TYPE,
+	},
 };
 
 static int devlink_param_generic_verify(const struct devlink_param *param)
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next 1/8] devlink: Add generic parameter hw_tc_offload
From: Vasundhara Volam @ 2018-09-11  8:44 UTC (permalink / raw)
  To: davem; +Cc: michael.chan, netdev
In-Reply-To: <1536655505-14387-1-git-send-email-vasundhara-v.volam@broadcom.com>

hw_tc_offload - Enable/Disable TC flower offload in the device.

Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 include/net/devlink.h | 4 ++++
 net/core/devlink.c    | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index b9b89d6..a0e9ce9 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -362,6 +362,7 @@ enum devlink_param_generic_id {
 	DEVLINK_PARAM_GENERIC_ID_MAX_MACS,
 	DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV,
 	DEVLINK_PARAM_GENERIC_ID_REGION_SNAPSHOT,
+	DEVLINK_PARAM_GENERIC_ID_HW_TC_OFFLOAD,
 
 	/* add new param generic ids above here*/
 	__DEVLINK_PARAM_GENERIC_ID_MAX,
@@ -380,6 +381,9 @@ enum devlink_param_generic_id {
 #define DEVLINK_PARAM_GENERIC_REGION_SNAPSHOT_NAME "region_snapshot_enable"
 #define DEVLINK_PARAM_GENERIC_REGION_SNAPSHOT_TYPE DEVLINK_PARAM_TYPE_BOOL
 
+#define DEVLINK_PARAM_GENERIC_HW_TC_OFFLOAD_NAME "hw_tc_offload"
+#define DEVLINK_PARAM_GENERIC_HW_TC_OFFLOAD_TYPE DEVLINK_PARAM_TYPE_BOOL
+
 #define DEVLINK_PARAM_GENERIC(_id, _cmodes, _get, _set, _validate)	\
 {									\
 	.id = DEVLINK_PARAM_GENERIC_ID_##_id,				\
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 65fc366..020daa1 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2676,6 +2676,11 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
 		.name = DEVLINK_PARAM_GENERIC_REGION_SNAPSHOT_NAME,
 		.type = DEVLINK_PARAM_GENERIC_REGION_SNAPSHOT_TYPE,
 	},
+	{
+		.id = DEVLINK_PARAM_GENERIC_ID_HW_TC_OFFLOAD,
+		.name = DEVLINK_PARAM_GENERIC_HW_TC_OFFLOAD_NAME,
+		.type = DEVLINK_PARAM_GENERIC_HW_TC_OFFLOAD_TYPE,
+	},
 };
 
 static int devlink_param_generic_verify(const struct devlink_param *param)
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next 6/8] bnxt_en: return proper error when FW returns HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED
From: Vasundhara Volam @ 2018-09-11  8:45 UTC (permalink / raw)
  To: davem; +Cc: michael.chan, netdev
In-Reply-To: <1536655505-14387-1-git-send-email-vasundhara-v.volam@broadcom.com>

Return proper error code when Firmware returns
HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED for HWRM_NVM_GET/SET_VARIABLE
commands.

Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 582e5b5..8d4ba33 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -79,8 +79,12 @@ static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
 		memcpy(buf, data_addr, bytesize);
 
 	dma_free_coherent(&bp->pdev->dev, bytesize, data_addr, data_dma_addr);
-	if (rc)
+	if (rc == HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED) {
+		netdev_err(bp->dev, "PF does not have admin privileges to modify NVM config\n");
+		return -EACCES;
+	} else if (rc) {
 		return -EIO;
+	}
 	return 0;
 }
 
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next 5/8] bnxt_en: Use hw_tc_offload and ignore_ari devlink parameters
From: Vasundhara Volam @ 2018-09-11  8:45 UTC (permalink / raw)
  To: davem; +Cc: michael.chan, netdev
In-Reply-To: <1536655505-14387-1-git-send-email-vasundhara-v.volam@broadcom.com>

This patch adds support for following generic permanent mode
devlink parameters. Both are disabled by default. They can be
enabled using devlink param commands.

hw_tc_offload - Enable/Disable TC flower offload in the device.

ignore_ari - Device ignores ARI(Alternate Routing ID) capability,
even when platforms has the support and creates same number of
partitions when platform does not support ARI capability.

Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 12 ++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index f3b9fbc..582e5b5 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -24,6 +24,10 @@
 static const struct bnxt_dl_nvm_param nvm_params[] = {
 	{DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV, NVM_OFF_ENABLE_SRIOV,
 	 BNXT_NVM_SHARED_CFG, 1},
+	{DEVLINK_PARAM_GENERIC_ID_HW_TC_OFFLOAD, NVM_OFF_HW_TC_OFFLOAD,
+	 BNXT_NVM_SHARED_CFG, 1},
+	{DEVLINK_PARAM_GENERIC_ID_IGNORE_ARI, NVM_OFF_IGNORE_ARI,
+	 BNXT_NVM_SHARED_CFG, 1},
 };
 
 static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
@@ -105,6 +109,14 @@ static int bnxt_dl_nvm_param_set(struct devlink *dl, u32 id,
 			      BIT(DEVLINK_PARAM_CMODE_PERMANENT),
 			      bnxt_dl_nvm_param_get, bnxt_dl_nvm_param_set,
 			      NULL),
+	DEVLINK_PARAM_GENERIC(HW_TC_OFFLOAD,
+			      BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			      bnxt_dl_nvm_param_get, bnxt_dl_nvm_param_set,
+			      NULL),
+	DEVLINK_PARAM_GENERIC(IGNORE_ARI,
+			      BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			      bnxt_dl_nvm_param_get, bnxt_dl_nvm_param_set,
+			      NULL),
 };
 
 int bnxt_dl_register(struct bnxt *bp)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
index 2f68dc0..da146492 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
@@ -33,6 +33,8 @@ static inline void bnxt_link_bp_to_dl(struct bnxt *bp, struct devlink *dl)
 	}
 }
 
+#define NVM_OFF_IGNORE_ARI		164
+#define NVM_OFF_HW_TC_OFFLOAD		170
 #define NVM_OFF_ENABLE_SRIOV		401
 
 enum bnxt_nvm_dir_type {
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next 7/8] bnxt_en: Use msix_vec_per_pf_max and msix_vec_per_pf_min devlink params.
From: Vasundhara Volam @ 2018-09-11  8:45 UTC (permalink / raw)
  To: davem; +Cc: michael.chan, netdev
In-Reply-To: <1536655505-14387-1-git-send-email-vasundhara-v.volam@broadcom.com>

This patch adds support for following generic permanent mode
devlink parameters. They can be modified using devlink param
commands.

msix_vec_per_pf_max - This param sets the number of MSIX vectors
that the device requests from the host on driver initialization.
This value is set in the device which limits MSIX vectors per PF.

msix_vec_per_pf_min - This param sets the number of minimal MSIX
vectors required for the device initialization. Value 0 indicates
a default value is selected. This value is set in the device which
limits MSIX vectors per PF.

Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 50 ++++++++++++++++++++++-
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h |  5 +++
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 8d4ba33..c9e7700 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -28,6 +28,10 @@
 	 BNXT_NVM_SHARED_CFG, 1},
 	{DEVLINK_PARAM_GENERIC_ID_IGNORE_ARI, NVM_OFF_IGNORE_ARI,
 	 BNXT_NVM_SHARED_CFG, 1},
+	{DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX,
+	 NVM_OFF_MSIX_VEC_PER_PF_MAX, BNXT_NVM_SHARED_CFG, 10},
+	{DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN,
+	 NVM_OFF_MSIX_VEC_PER_PF_MIN, BNXT_NVM_SHARED_CFG, 7},
 };
 
 static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
@@ -56,8 +60,22 @@ static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
 		idx = bp->pf.fw_fid - BNXT_FIRST_PF_FID;
 
 	bytesize = roundup(nvm_param.num_bits, BITS_PER_BYTE) / BITS_PER_BYTE;
-	if (nvm_param.num_bits == 1)
-		buf = &val->vbool;
+	switch (bytesize) {
+	case 1:
+		if (nvm_param.num_bits == 1)
+			buf = &val->vbool;
+		else
+			buf = &val->vu8;
+		break;
+	case 2:
+		buf = &val->vu16;
+		break;
+	case 4:
+		buf = &val->vu32;
+		break;
+	default:
+		return -EFAULT;
+	}
 
 	data_addr = dma_zalloc_coherent(&bp->pdev->dev, bytesize,
 					&data_dma_addr, GFP_KERNEL);
@@ -108,6 +126,26 @@ static int bnxt_dl_nvm_param_set(struct devlink *dl, u32 id,
 	return bnxt_hwrm_nvm_req(bp, id, &req, sizeof(req), &ctx->val);
 }
 
+static int bnxt_dl_msix_validate(struct devlink *dl, u32 id,
+				 union devlink_param_value val,
+				 struct netlink_ext_ack *extack)
+{
+	int max_val;
+
+	if (id == DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX)
+		max_val = BNXT_MSIX_VEC_MAX;
+
+	if (id == DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN)
+		max_val = BNXT_MSIX_VEC_MIN_MAX;
+
+	if (val.vu32 < 0 || val.vu32 > max_val) {
+		NL_SET_ERR_MSG_MOD(extack, "MSIX value is exceeding the range");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static const struct devlink_param bnxt_dl_params[] = {
 	DEVLINK_PARAM_GENERIC(ENABLE_SRIOV,
 			      BIT(DEVLINK_PARAM_CMODE_PERMANENT),
@@ -121,6 +159,14 @@ static int bnxt_dl_nvm_param_set(struct devlink *dl, u32 id,
 			      BIT(DEVLINK_PARAM_CMODE_PERMANENT),
 			      bnxt_dl_nvm_param_get, bnxt_dl_nvm_param_set,
 			      NULL),
+	DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MAX,
+			      BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			      bnxt_dl_nvm_param_get, bnxt_dl_nvm_param_set,
+			      bnxt_dl_msix_validate),
+	DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MIN,
+			      BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			      bnxt_dl_nvm_param_get, bnxt_dl_nvm_param_set,
+			      bnxt_dl_msix_validate),
 };
 
 int bnxt_dl_register(struct bnxt *bp)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
index da146492..0e67c05 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
@@ -33,10 +33,15 @@ static inline void bnxt_link_bp_to_dl(struct bnxt *bp, struct devlink *dl)
 	}
 }
 
+#define NVM_OFF_MSIX_VEC_PER_PF_MAX	108
+#define NVM_OFF_MSIX_VEC_PER_PF_MIN	114
 #define NVM_OFF_IGNORE_ARI		164
 #define NVM_OFF_HW_TC_OFFLOAD		170
 #define NVM_OFF_ENABLE_SRIOV		401
 
+#define BNXT_MSIX_VEC_MAX	1280
+#define BNXT_MSIX_VEC_MIN_MAX	128
+
 enum bnxt_nvm_dir_type {
 	BNXT_NVM_SHARED_CFG = 40,
 	BNXT_NVM_PORT_CFG,
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next 8/8] bnxt_en: Add a driver specific devlink parameter.
From: Vasundhara Volam @ 2018-09-11  8:45 UTC (permalink / raw)
  To: davem; +Cc: michael.chan, netdev
In-Reply-To: <1536655505-14387-1-git-send-email-vasundhara-v.volam@broadcom.com>

This patch adds following driver-specific permanent mode boolean
parameter.

gre_ver_check - When this param is disabled, device skips GRE
version check.

Signed-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 24 ++++++++++++++++++++++-
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h |  1 +
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index c9e7700..9778b88 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -21,6 +21,11 @@
 #endif /* CONFIG_BNXT_SRIOV */
 };
 
+enum bnxt_dl_param_id {
+	BNXT_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
+	BNXT_DEVLINK_PARAM_ID_GRE_VER_CHECK,
+};
+
 static const struct bnxt_dl_nvm_param nvm_params[] = {
 	{DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV, NVM_OFF_ENABLE_SRIOV,
 	 BNXT_NVM_SHARED_CFG, 1},
@@ -32,6 +37,8 @@
 	 NVM_OFF_MSIX_VEC_PER_PF_MAX, BNXT_NVM_SHARED_CFG, 10},
 	{DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN,
 	 NVM_OFF_MSIX_VEC_PER_PF_MIN, BNXT_NVM_SHARED_CFG, 7},
+	{BNXT_DEVLINK_PARAM_ID_GRE_VER_CHECK, NVM_OFF_DIS_GRE_VER_CHECK,
+	 BNXT_NVM_SHARED_CFG, 1},
 };
 
 static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
@@ -111,9 +118,15 @@ static int bnxt_dl_nvm_param_get(struct devlink *dl, u32 id,
 {
 	struct hwrm_nvm_get_variable_input req = {0};
 	struct bnxt *bp = bnxt_get_bp_from_dl(dl);
+	int rc;
 
 	bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_NVM_GET_VARIABLE, -1, -1);
-	return bnxt_hwrm_nvm_req(bp, id, &req, sizeof(req), &ctx->val);
+	rc = bnxt_hwrm_nvm_req(bp, id, &req, sizeof(req), &ctx->val);
+	if (!rc)
+		if (id == BNXT_DEVLINK_PARAM_ID_GRE_VER_CHECK)
+			ctx->val.vbool = !ctx->val.vbool;
+
+	return rc;
 }
 
 static int bnxt_dl_nvm_param_set(struct devlink *dl, u32 id,
@@ -123,6 +136,10 @@ static int bnxt_dl_nvm_param_set(struct devlink *dl, u32 id,
 	struct bnxt *bp = bnxt_get_bp_from_dl(dl);
 
 	bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_NVM_SET_VARIABLE, -1, -1);
+
+	if (id == BNXT_DEVLINK_PARAM_ID_GRE_VER_CHECK)
+		ctx->val.vbool = !ctx->val.vbool;
+
 	return bnxt_hwrm_nvm_req(bp, id, &req, sizeof(req), &ctx->val);
 }
 
@@ -167,6 +184,11 @@ static int bnxt_dl_msix_validate(struct devlink *dl, u32 id,
 			      BIT(DEVLINK_PARAM_CMODE_PERMANENT),
 			      bnxt_dl_nvm_param_get, bnxt_dl_nvm_param_set,
 			      bnxt_dl_msix_validate),
+	DEVLINK_PARAM_DRIVER(BNXT_DEVLINK_PARAM_ID_GRE_VER_CHECK,
+			     "gre_ver_check", DEVLINK_PARAM_TYPE_BOOL,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     bnxt_dl_nvm_param_get, bnxt_dl_nvm_param_set,
+			     NULL),
 };
 
 int bnxt_dl_register(struct bnxt *bp)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
index 0e67c05..e36e41a 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
@@ -37,6 +37,7 @@ static inline void bnxt_link_bp_to_dl(struct bnxt *bp, struct devlink *dl)
 #define NVM_OFF_MSIX_VEC_PER_PF_MIN	114
 #define NVM_OFF_IGNORE_ARI		164
 #define NVM_OFF_HW_TC_OFFLOAD		170
+#define NVM_OFF_DIS_GRE_VER_CHECK	171
 #define NVM_OFF_ENABLE_SRIOV		401
 
 #define BNXT_MSIX_VEC_MAX	1280
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH v2 net-next 05/12] net: ethernet: genet: Fix speed selection
From: Sergei Shtylyov @ 2018-09-11  9:02 UTC (permalink / raw)
  To: Andrew Lunn, David Miller; +Cc: netdev, Florian Fainelli
In-Reply-To: <1536616350-15442-6-git-send-email-andrew@lunn.ch>

Hello!

On 9/11/2018 12:52 AM, Andrew Lunn wrote:

> The phy supported speed is being used to determine if the MAC should
> be configured to 100 or 1G. The masking logic is broken. Instead, look

    Look at?

> 1G supported speeds to enable 1G MAC support.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
[...]

MBR, Sergei

^ permalink raw reply

* Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG
From: Sergei Shtylyov @ 2018-09-11  9:11 UTC (permalink / raw)
  To: zhong jiang; +Cc: davem, edumazet, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <5B9783ED.4080908@huawei.com>

On 9/11/2018 11:59 AM, zhong jiang wrote:

>>> The if condition can be removed if we use BUG_ON directly.
>>> The issule is detected with the help of Coccinelle.
>>
>>    Issue?
>>
>>>
>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>>> ---
>>>    net/ipv4/tcp_input.c | 8 ++++----
>>>    1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>>> index 62508a2..893bde3 100644
>>> --- a/net/ipv4/tcp_input.c
>>> +++ b/net/ipv4/tcp_input.c
>>> @@ -4934,8 +4934,8 @@ void tcp_rbtree_insert(struct rb_root *root, struct sk_buff *skb)
>>>                BUG_ON(offset < 0);
>>>                if (size > 0) {
>>>                    size = min(copy, size);
>>> -                if (skb_copy_bits(skb, offset, skb_put(nskb, size), size))
>>> -                    BUG();
>>> +                BUG(skb_copy_bits(skb, offset,
>>
>>     You said BUG_ON()?
>   Yep. Do you think that it is worthing to do

    I think BUG() doesn't take parameters, BUG_ON() does. Have you tried to 
build the kernel with your patch at all?

>   Thanks,
> zhong jiang
[...]

MBR, Sergei

^ permalink raw reply

* Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG
From: Sergei Shtylyov @ 2018-09-11  9:33 UTC (permalink / raw)
  To: zhong jiang; +Cc: davem, edumazet, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <5B97889F.3020303@huawei.com>

On 9/11/2018 12:19 PM, zhong jiang wrote:

>>>>> The if condition can be removed if we use BUG_ON directly.
>>>>> The issule is detected with the help of Coccinelle.
>>>>
>>>>     Issue?
>>>>
>>>>>
>>>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>>>>> ---
>>>>>     net/ipv4/tcp_input.c | 8 ++++----
>>>>>     1 file changed, 4 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>>>>> index 62508a2..893bde3 100644
>>>>> --- a/net/ipv4/tcp_input.c
>>>>> +++ b/net/ipv4/tcp_input.c
>>>>> @@ -4934,8 +4934,8 @@ void tcp_rbtree_insert(struct rb_root *root, struct sk_buff *skb)
>>>>>                 BUG_ON(offset < 0);
>>>>>                 if (size > 0) {
>>>>>                     size = min(copy, size);
>>>>> -                if (skb_copy_bits(skb, offset, skb_put(nskb, size), size))
>>>>> -                    BUG();
>>>>> +                BUG(skb_copy_bits(skb, offset,
>>>>
>>>>      You said BUG_ON()?
>>>    Yep. Do you think that it is worthing to do
>>
>>     I think BUG() doesn't take parameters, BUG_ON() does. Have you tried to build the kernel with your patch at all?
>>
>   I know that the patch should be BUG_ON instead of BUG. This is my mistake.  I just want to know that it is worthing to do so.

    Yes, probably.

>   Thanks,
>   zhong jiang
[...]

MBR, Sergei

^ permalink raw reply

* Re: [PATCH net-next 1/8] devlink: Add generic parameter hw_tc_offload
From: Jiri Pirko @ 2018-09-11  9:51 UTC (permalink / raw)
  To: Vasundhara Volam; +Cc: davem, michael.chan, netdev
In-Reply-To: <1536655505-14387-2-git-send-email-vasundhara-v.volam@broadcom.com>

Tue, Sep 11, 2018 at 10:44:58AM CEST, vasundhara-v.volam@broadcom.com wrote:
>hw_tc_offload - Enable/Disable TC flower offload in the device.
>
>Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
>---
> include/net/devlink.h | 4 ++++
> net/core/devlink.c    | 5 +++++
> 2 files changed, 9 insertions(+)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index b9b89d6..a0e9ce9 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -362,6 +362,7 @@ enum devlink_param_generic_id {
> 	DEVLINK_PARAM_GENERIC_ID_MAX_MACS,
> 	DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV,
> 	DEVLINK_PARAM_GENERIC_ID_REGION_SNAPSHOT,
>+	DEVLINK_PARAM_GENERIC_ID_HW_TC_OFFLOAD,

Could you please describe why do you need this here and why the
tc_offload flag in ethtool is not enough. How do you imagine the user
should use them together?

^ permalink raw reply

* Re: [PATCH net-next v3 02/17] zinc: introduce minimal cryptography library
From: Greg Kroah-Hartman @ 2018-09-11 14:56 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Jason A. Donenfeld, Linux Kernel Mailing List,
	<netdev@vger.kernel.org>, David S. Miller, Andy Lutomirski,
	Samuel Neves, Jean-Philippe Aumasson,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE
In-Reply-To: <CAKv+Gu9nSeo4PFVrtjtqGQ32eqyxUp3xVCczZfu+dhFz5yLM0A@mail.gmail.com>

On Tue, Sep 11, 2018 at 12:08:56PM +0200, Ard Biesheuvel wrote:
> > As Zinc is simply library code, its config options are un-menued, with
> > the exception of CONFIG_ZINC_DEBUG, which enables various selftests and
> > BUG_ONs.
> >
> 
> In spite of the wall of text, you fail to point out exactly why the
> existing AEAD API in unsuitable, and why fixing it is not an option.
> 
> As I pointed out in a previous version, I don't think we need a
> separate crypto API/library in the kernel, and I don't think you have
> convinced anyone else yet either.

Um, then why do people keep sprinkling new crypto/hash code all around
the kernel tree?  It's because what we have as a crypto api is complex
and is hard to use for many in-kernel users.

Something like this new interface (zinc) is a much "saner" api for
writing kernel code that has to interact with crypto/hash primitives.

I see no reason why the existing crypto code can be redone to use the
zinc crypto primitives over time, making there only be one main location
for the crypto algorithms.  But to do it the other way around is pretty
much impossible given the complexities in the existing api that has been
created over time.

Not to say that the existing api is not a viable one, but ugh, really?
You have to admit it is a pain to try to use in any "normal" type of
"here's a bytestream, go give me a hash" type of method, right?

Also there is the added benefit that the crypto primitives here have
been audited by a number of people (so Jason stated), and they are
written in a way that the crypto community can more easily interact and
contribute to.  Which is _way_ better than what we have today.

So this gets my "stamp of approval" for whatever it is worth :)

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH net-next v3 02/17] zinc: introduce minimal cryptography library
From: Ard Biesheuvel @ 2018-09-11 10:08 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Linux Kernel Mailing List, <netdev@vger.kernel.org>,
	David S. Miller, Greg Kroah-Hartman, Andy Lutomirski,
	Samuel Neves, Jean-Philippe Aumasson,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE
In-Reply-To: <20180911010838.8818-3-Jason@zx2c4.com>

On 11 September 2018 at 03:08, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Zinc stands for "Zinc Is Neat Crypto" or "Zinc as IN Crypto" or maybe
> just "Zx2c4's INsane Cryptolib." It's also short, easy to type, and
> plays nicely with the recent trend of naming crypto libraries after
> elements. The guiding principle is "don't overdo it". It's less of a
> library and more of a directory tree for organizing well-curated direct
> implementations of cryptography primitives.
>
> Zinc is a new cryptography API that is much more minimal and lower-level
> than the current one. It intends to complement it and provide a basis
> upon which the current crypto API might build, as the provider of
> software implementations of cryptographic primitives. It is motivated by
> three primary observations in crypto API design:
>
>   * Highly composable "cipher modes" and related abstractions from
>     90s cryptographers did not turn out to be as terrific an idea as
>     hoped, leading to a host of API misuse problems.
>
>   * Most programmers are afraid of crypto code, and so prefer to
>     integrate it into libraries in a highly abstracted manner, so as to
>     shield themselves from implementation details. Cryptographers, on
>     the other hand, prefer simple direct implementations, which they're
>     able to verify for high assurance and optimize in accordance with
>     their expertise.
>
>   * Overly abstracted and flexible cryptography APIs lead to a host of
>     dangerous problems and performance issues. The kernel is in the
>     business usually not of coming up with new uses of crypto, but
>     rather implementing various constructions, which means it essentially
>     needs a library of primitives, not a highly abstracted enterprise-ready
>     pluggable system, with a few particular exceptions.
>
> This last observation has seen itself play out several times over and
> over again within the kernel:
>
>   * The perennial move of actual primitives away from crypto/ and into
>     lib/, so that users can actually call these functions directly with
>     no overhead and without lots of allocations, function pointers,
>     string specifier parsing, and general clunkiness. For example:
>     sha256, chacha20, siphash, sha1, and so forth live in lib/ rather
>     than in crypto/. Zinc intends to stop the cluttering of lib/ and
>     introduce these direct primitives into their proper place, lib/zinc/.
>
>   * An abundance of misuse bugs with the present crypto API that have
>     been very unpleasant to clean up.
>
>   * A hesitance to even use cryptography, because of the overhead and
>     headaches involved in accessing the routines.
>
> Zinc goes in a rather different direction. Rather than providing a
> thoroughly designed and abstracted API, Zinc gives you simple functions,
> which implement some primitive, or some particular and specific
> construction of primitives. It is not dynamic in the least, though one
> could imagine implementing a complex dynamic dispatch mechanism (such as
> the current crypto API) on top of these basic functions. After all,
> dynamic dispatch is usually needed for applications with cipher agility,
> such as IPsec, dm-crypt, AF_ALG, and so forth, and the existing crypto
> API will continue to play that role. However, Zinc will provide a non-
> haphazard way of directly utilizing crypto routines in applications
> that do have neither the need nor desire for abstraction and dynamic
> dispatch.
>
> It also organizes the implementations in a simple, straight-forward,
> and direct manner, making it enjoyable and intuitive to work on.
> Rather than moving optimized assembly implementations into arch/, it
> keeps them all together in lib/zinc/, making it simple and obvious to
> compare and contrast what's happening. This is, notably, exactly what
> the lib/raid6/ tree does, and that seems to work out rather well. It's
> also the pattern of most successful crypto libraries. The architecture-
> specific glue-code is made a part of each translation unit, rather than
> being in a separate one, so that generic and architecture-optimized code
> are combined at compile-time, and incompatibility branches compiled out by
> the optimizer.
>
> All implementations have been extensively tested and fuzzed, and are
> selected for their quality, trustworthiness, and performance. Wherever
> possible and performant, formally verified implementations are used,
> such as those from HACL* [1] and Fiat-Crypto [2]. The routines also take
> special care to zero out secrets using memzero_explicit (and future work
> is planned to have gcc do this more reliably and performantly with
> compiler plugins). The performance of the selected implementations is
> state-of-the-art and unrivaled on a broad array of hardware, though of
> course we will continue to fine tune these to the hardware demands
> needed by kernel contributors. Each implementation also comes with
> extensive self-tests and crafted test vectors, pulled from various
> places such as Wycheproof [9].
>
> Regularity of function signatures is important, so that users can easily
> "guess" the name of the function they want. Though, individual
> primitives are oftentimes not trivially interchangeable, having been
> designed for different things and requiring different parameters and
> semantics, and so the function signatures they provide will directly
> reflect the realities of the primitives' usages, rather than hiding it
> behind (inevitably leaky) abstractions. Also, in contrast to the current
> crypto API, Zinc functions can work on stack buffers, and can be called
> with different keys, without requiring allocations or locking.
>
> SIMD is used automatically when available, though some routines may
> benefit from either having their SIMD disabled for particular
> invocations, or to have the SIMD initialization calls amortized over
> several invocations of the function, and so Zinc utilizes function
> signatures enabling that in conjunction with the recently introduced
> simd_context_t.
>
> More generally, Zinc provides function signatures that allow just what
> is required by the various callers. This isn't to say that users of the
> functions will be permitted to pollute the function semantics with weird
> particular needs, but we are trying very hard not to overdo it, and that
> means looking carefully at what's actually necessary, and doing just that,
> and not much more than that. Remember: practicality and cleanliness rather
> than over-zealous infrastructure.
>
> Zinc provides also an opening for the best implementers in academia to
> contribute their time and effort to the kernel, by being sufficiently
> simple and inviting. In discussing this commit with some of the best and
> brightest over the last few years, there are many who are eager to
> devote rare talent and energy to this effort.
>
> Following the merging of this, I expect for the primitives that
> currently exist in lib/ to work their way into lib/zinc/, after intense
> scrutiny of each implementation, potentially replacing them with either
> formally-verified implementations, or better studied and faster
> state-of-the-art implementations.
>
> Also following the merging of this, I expect for the old crypto API
> implementations to be ported over to use Zinc for their software-based
> implementations.
>
> As Zinc is simply library code, its config options are un-menued, with
> the exception of CONFIG_ZINC_DEBUG, which enables various selftests and
> BUG_ONs.
>

In spite of the wall of text, you fail to point out exactly why the
existing AEAD API in unsuitable, and why fixing it is not an option.

As I pointed out in a previous version, I don't think we need a
separate crypto API/library in the kernel, and I don't think you have
convinced anyone else yet either.

Perhaps you can devote /your/ rare talent and energy to improving what
we already have for everybody's sake, rather than providing a
completely separate crypto stack that only benefits WireGuard (unless
you yourself port the existing crypto API software algorithms to this
crypto stack first and present *that* work as a convincing case in
itself)

I won't go into the 1000s lines of generated assembly again - you
already know my position on that topic.

Please refrain from sending a v4 with just a couple of more tweaks on
top - these are fundamental issues that require discussion before
there is any chance of this being merged.


> [1] https://github.com/project-everest/hacl-star
> [2] https://github.com/mit-plv/fiat-crypto
> [3] https://cr.yp.to/ecdh.html
> [4] https://cr.yp.to/chacha.html
> [5] https://cr.yp.to/snuffle/xsalsa-20081128.pdf
> [6] https://cr.yp.to/mac.html
> [7] https://blake2.net/
> [8] https://tools.ietf.org/html/rfc8439
> [9] https://github.com/google/wycheproof
>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Greg KH <gregkh@linuxfoundation.org>
> Cc: Samuel Neves <sneves@dei.uc.pt>
> Cc: Jean-Philippe Aumasson <jeanphilippe.aumasson@gmail.com>
> Cc: linux-crypto@vger.kernel.org
> ---
>  MAINTAINERS       |  8 ++++++++
>  lib/Kconfig       |  2 ++
>  lib/Makefile      |  2 ++
>  lib/zinc/Kconfig  | 20 ++++++++++++++++++++
>  lib/zinc/Makefile |  8 ++++++++
>  lib/zinc/main.c   | 31 +++++++++++++++++++++++++++++++
>  6 files changed, 71 insertions(+)
>  create mode 100644 lib/zinc/Kconfig
>  create mode 100644 lib/zinc/Makefile
>  create mode 100644 lib/zinc/main.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2ef884b883c3..d2092e52320d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16160,6 +16160,14 @@ Q:     https://patchwork.linuxtv.org/project/linux-media/list/
>  S:     Maintained
>  F:     drivers/media/dvb-frontends/zd1301_demod*
>
> +ZINC CRYPTOGRAPHY LIBRARY
> +M:     Jason A. Donenfeld <Jason@zx2c4.com>
> +M:     Samuel Neves <sneves@dei.uc.pt>
> +S:     Maintained
> +F:     lib/zinc/
> +F:     include/zinc/
> +L:     linux-crypto@vger.kernel.org
> +
>  ZPOOL COMPRESSED PAGE STORAGE API
>  M:     Dan Streetman <ddstreet@ieee.org>
>  L:     linux-mm@kvack.org
> diff --git a/lib/Kconfig b/lib/Kconfig
> index a3928d4438b5..3e6848269c66 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -485,6 +485,8 @@ config GLOB_SELFTEST
>           module load) by a small amount, so you're welcome to play with
>           it, but you probably don't need it.
>
> +source "lib/zinc/Kconfig"
> +
>  #
>  # Netlink attribute parsing support is select'ed if needed
>  #
> diff --git a/lib/Makefile b/lib/Makefile
> index ca3f7ebb900d..3f16e35d2c11 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -214,6 +214,8 @@ obj-$(CONFIG_PERCPU_TEST) += percpu_test.o
>
>  obj-$(CONFIG_ASN1) += asn1_decoder.o
>
> +obj-$(CONFIG_ZINC) += zinc/
> +
>  obj-$(CONFIG_FONT_SUPPORT) += fonts/
>
>  obj-$(CONFIG_PRIME_NUMBERS) += prime_numbers.o
> diff --git a/lib/zinc/Kconfig b/lib/zinc/Kconfig
> new file mode 100644
> index 000000000000..aa4f8d449d6b
> --- /dev/null
> +++ b/lib/zinc/Kconfig
> @@ -0,0 +1,20 @@
> +config ZINC
> +       tristate
> +       select CRYPTO_BLKCIPHER
> +       select VFP
> +       select VFPv3
> +       select NEON
> +       select KERNEL_MODE_NEON
> +
> +config ZINC_DEBUG
> +       bool "Zinc cryptography library debugging and self-tests"
> +       depends on ZINC
> +       help
> +         This builds a series of self-tests for the Zinc crypto library, which
> +         help diagnose any cryptographic algorithm implementation issues that
> +         might be at the root cause of potential bugs. It also adds various
> +         debugging traps.
> +
> +         Unless you're developing and testing cryptographic routines, or are
> +         especially paranoid about correctness on your hardware, you may say
> +         N here.
> diff --git a/lib/zinc/Makefile b/lib/zinc/Makefile
> new file mode 100644
> index 000000000000..dad47573de42
> --- /dev/null
> +++ b/lib/zinc/Makefile
> @@ -0,0 +1,8 @@
> +ccflags-y := -O3
> +ccflags-y += -Wframe-larger-than=8192
> +ccflags-y += -D'pr_fmt(fmt)=KBUILD_MODNAME ": " fmt'
> +ccflags-$(CONFIG_ZINC_DEBUG) += -DDEBUG
> +
> +zinc-y += main.o
> +
> +obj-$(CONFIG_ZINC) := zinc.o
> diff --git a/lib/zinc/main.c b/lib/zinc/main.c
> new file mode 100644
> index 000000000000..ceece33ff5a7
> --- /dev/null
> +++ b/lib/zinc/main.c
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (C) 2015-2018 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +
> +#ifdef DEBUG
> +#define selftest(which) do { \
> +       if (!which ## _selftest()) \
> +               return -ENOTRECOVERABLE; \
> +} while (0)
> +#else
> +#define selftest(which)
> +#endif
> +
> +static int __init mod_init(void)
> +{
> +       return 0;
> +}
> +
> +static void __exit mod_exit(void)
> +{
> +}
> +
> +module_init(mod_init);
> +module_exit(mod_exit);
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Zinc cryptography library");
> +MODULE_AUTHOR("Jason A. Donenfeld <Jason@zx2c4.com>");
> --
> 2.18.0
>

^ permalink raw reply

* Re: [PATCH 0/3] xen-netback: hash mapping hanling adjustments
From: Wei Liu @ 2018-09-11 10:16 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Paul Durrant, Wei Liu, davem, xen-devel, netdev
In-Reply-To: <5B9778D702000078001E7069@prv1-mh.provo.novell.com>

On Tue, Sep 11, 2018 at 02:12:07AM -0600, Jan Beulich wrote:
> >>> On 28.08.18 at 16:54,  wrote:
> > First and foremost the fix for XSA-270. On top of that further changes
> > which looked desirable to me while investigating that XSA.
> > 
> > 1: fix input validation in xenvif_set_hash_mapping()
> > 2: validate queue numbers in xenvif_set_hash_mapping()
> > 3: handle page straddling in xenvif_set_hash_mapping()
> > 
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> What is the way forward here? I've got R-b-s from Paul for all three
> patches, and a minor change request on patch 2 from Wei. I'm not
> really certain what to do in this case (hints appreciated), but could
> at least the security fix (patch 1) be applied immediately?

If you happen to resend, please make the adjustment; otherwise I'm fine
with the patches as they are. I don't want to block useful things on
cosmetic issues.

Wei.

^ permalink raw reply

* Re: unexpected GRO/veth behavior
From: Eric Dumazet @ 2018-09-11 10:27 UTC (permalink / raw)
  To: Paolo Abeni, netdev; +Cc: eric.dumazet, Toshiaki Makita
In-Reply-To: <bcea5cafe139e2c561f036f5b3e2598d7d0dd970.camel@redhat.com>



On 09/10/2018 11:54 PM, Paolo Abeni wrote:
> Hi,
> 
> On Mon, 2018-09-10 at 16:44 +0200, Paolo Abeni wrote:
>> while testing some local patches I observed that the TCP tput in the
>> following scenario:
>>
>> # the following enable napi on veth0, so that we can trigger the
>> # GRO path with namespaces
>> ip netns add test
>> ip link add type veth
>> ip link set dev veth0 netns test
>> ip -n test link set lo up
>> ip -n test link set veth0 up
>> ip -n test addr add dev veth0 172.16.1.2/24
>> ip link set dev veth1 up
>> ip addr add dev veth1 172.16.1.1/24
>> IDX=`ip netns exec test cat /sys/class/net/veth0/ifindex`
>>
>> # 'xdp_pass' is a NO-OP XDP program that simply return XDP_PASS
>> ip netns exec test ./xdp_pass $IDX &
>> taskset 0x2 ip netns exec test iperf3 -s -i 60 &
>> taskset 0x1 iperf3 -c 172.16.1.2 -t 60 -i 60
> 
> In the same scenario, using instead:
> 
> iperf3 -c 172.16.1.2 -t 600  -i 60 -N -l 10K
> 
> I hit the following splat, on a recent, unpatched net-next:
> 
> [  362.098904] refcount_t overflow at skb_set_owner_w+0x5e/0xa0 in iperf3[1644], uid/euid: 0/0
> [  362.108239] WARNING: CPU: 0 PID: 1644 at kernel/panic.c:648 refcount_error_report+0xa0/0xa4
> [  362.117547] Modules linked in: tcp_diag inet_diag veth intel_rapl sb_edac x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel intel_cstate intel_uncore intel_rapl_perf ipmi_ssif iTCO_wdt sg ipmi_si iTCO_vendor_support ipmi_devintf mxm_wmi ipmi_msghandler pcspkr dcdbas mei_me wmi mei lpc_ich acpi_power_meter pcc_cpufreq xfs libcrc32c sd_mod mgag200 drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ixgbe igb ttm ahci mdio libahci ptp crc32c_intel drm pps_core libata i2c_algo_bit dca dm_mirror dm_region_hash dm_log dm_mod
> [  362.176622] CPU: 0 PID: 1644 Comm: iperf3 Not tainted 4.19.0-rc2.vanilla+ #2025
> [  362.184777] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.1.7 06/16/2016
> [  362.193124] RIP: 0010:refcount_error_report+0xa0/0xa4
> [  362.198758] Code: 08 00 00 48 8b 95 80 00 00 00 49 8d 8c 24 80 0a 00 00 41 89 c1 44 89 2c 24 48 89 de 48 c7 c7 18 4d e7 9d 31 c0 e8 30 fa ff ff <0f> 0b eb 88 0f 1f 44 00 00 55 48 89 e5 41 56 41 55 41 54 49 89 fc
> [  362.219711] RSP: 0018:ffff9ee6ff603c20 EFLAGS: 00010282
> [  362.225538] RAX: 0000000000000000 RBX: ffffffff9de83e10 RCX: 0000000000000000
> [  362.233497] RDX: 0000000000000001 RSI: ffff9ee6ff6167d8 RDI: ffff9ee6ff6167d8
> [  362.241457] RBP: ffff9ee6ff603d78 R08: 0000000000000490 R09: 0000000000000004
> [  362.249416] R10: 0000000000000000 R11: ffff9ee6ff603990 R12: ffff9ee664b94500
> [  362.257377] R13: 0000000000000000 R14: 0000000000000004 R15: ffffffff9de615f9
> [  362.265337] FS:  00007f1d22d28740(0000) GS:ffff9ee6ff600000(0000) knlGS:0000000000000000
> [  362.274363] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  362.280773] CR2: 00007f1d222f35d0 CR3: 0000001fddfec003 CR4: 00000000001606f0
> [  362.288733] Call Trace:
> [  362.291459]  <IRQ>
> [  362.293702]  ex_handler_refcount+0x4e/0x80
> [  362.298269]  fixup_exception+0x35/0x40
> [  362.302451]  do_trap+0x109/0x150
> [  362.306048]  do_error_trap+0xd5/0x130
> [  362.315766]  invalid_op+0x14/0x20
> [  362.319460] RIP: 0010:skb_set_owner_w+0x5e/0xa0
> [  362.324512] Code: ef ff ff 74 49 48 c7 43 60 20 7b 4a 9d 8b 85 f4 01 00 00 85 c0 75 16 8b 83 e0 00 00 00 f0 01 85 44 01 00 00 0f 88 d8 23 16 00 <5b> 5d c3 80 8b 91 00 00 00 01 8b 85 f4 01 00 00 89 83 a4 00 00 00
> [  362.345465] RSP: 0018:ffff9ee6ff603e20 EFLAGS: 00010a86
> [  362.351291] RAX: 0000000000001100 RBX: ffff9ee65deec700 RCX: ffff9ee65e829244
> [  362.359250] RDX: 0000000000000100 RSI: ffff9ee65e829100 RDI: ffff9ee65deec700
> [  362.367210] RBP: ffff9ee65e829100 R08: 000000000002a380 R09: 0000000000000000
> [  362.375169] R10: 0000000000000002 R11: fffff1a4bf77bb00 R12: ffffc0754661d000
> [  362.383130] R13: ffff9ee65deec200 R14: ffff9ee65f597000 R15: 00000000000000aa
> [  362.391092]  veth_xdp_rcv+0x4e4/0x890 [veth]
> [  362.399357]  veth_poll+0x4d/0x17a [veth]
> [  362.403731]  net_rx_action+0x2af/0x3f0
> [  362.407912]  __do_softirq+0xdd/0x29e
> [  362.411897]  do_softirq_own_stack+0x2a/0x40
> [  362.416561]  </IRQ>
> [  362.418899]  do_softirq+0x4b/0x70
> [  362.422594]  __local_bh_enable_ip+0x50/0x60
> [  362.427258]  ip_finish_output2+0x16a/0x390
> [  362.431824]  ip_output+0x71/0xe0
> [  362.440670]  __tcp_transmit_skb+0x583/0xab0
> [  362.445333]  tcp_write_xmit+0x247/0xfb0
> [  362.449609]  __tcp_push_pending_frames+0x2d/0xd0
> [  362.454760]  tcp_sendmsg_locked+0x857/0xd30
> [  362.459424]  tcp_sendmsg+0x27/0x40
> [  362.463216]  sock_sendmsg+0x36/0x50
> [  362.467104]  sock_write_iter+0x87/0x100
> [  362.471382]  __vfs_write+0x112/0x1a0
> [  362.475369]  vfs_write+0xad/0x1a0
> [  362.479062]  ksys_write+0x52/0xc0
> [  362.482759]  do_syscall_64+0x5b/0x180
> [  362.486841]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [  362.492473] RIP: 0033:0x7f1d22293238
> [  362.496458] Code: 89 02 48 c7 c0 ff ff ff ff eb b3 0f 1f 80 00 00 00 00 f3 0f 1e fa 48 8d 05 c5 54 2d 00 8b 00 85 c0 75 17 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 58 c3 0f 1f 80 00 00 00 00 41 54 49 89 d4 55
> [  362.517409] RSP: 002b:00007ffebaef8008 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> [  362.525855] RAX: ffffffffffffffda RBX: 0000000000002800 RCX: 00007f1d22293238
> [  362.533816] RDX: 0000000000002800 RSI: 00007f1d22d36000 RDI: 0000000000000005
> [  362.541775] RBP: 00007f1d22d36000 R08: 00000002db777a30 R09: 0000562b70712b20
> [  362.549734] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000005
> [  362.557693] R13: 0000000000002800 R14: 00007ffebaef8060 R15: 0000562b70712260
> 
> The problem, AFAICS, is that the GRO path changes the cumulative
> truesize of the skbs entering such code path without updating
> sk_wmem_alloc. The posted code tries to keep unchanged such cumulative
> truesize.
> 

As I said, skbs entering GRO should not have skb->sk set.
GRO fully owns skbs.

No need to convince us.

For some reason, Toshiaki Makita added XDP and GRO, and broke veth



> I *think* we can hit a similar condition with a tun device in IFF_NAPI
> mode.

Why ?

tun_get_user() does not attach skb to a socket, that would be quite useless since skb
is entering input path and would be orphaned right away.


Fix would probably be :

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 8d679c8b7f25c753d77cfb8821d9d2528c9c9048..96bd94480942b469403abf017f9f9d5be1e23ef5 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -602,9 +602,10 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget, unsigned int *xdp_xmit)
                        skb = veth_xdp_rcv_skb(rq, ptr, xdp_xmit);
                }
 
-               if (skb)
+               if (skb) {
+                       skb_orphan(skb);
                        napi_gro_receive(&rq->xdp_napi, skb);
-
+               }
                done++;
        }
 

^ permalink raw reply related

* [PATCH RFC] net: qca_spi: Introduce write register verification
From: Stefan Wahren @ 2018-09-11 15:26 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel, Stefan Wahren, Michael Heimpold

The SPI protocol for the QCA7000 doesn't have any fault detection.
In order to increase the drivers reliability in noisy environments,
we could implement a write verification inspired by the enc28j60.
This should avoid situations were the driver wrongly assumes the
receive interrupt is enabled and miss all incoming packets.

This function is disabled per default and can be controlled via module
parameter wr_verify.

Signed-off-by: Michael Heimpold <michael.heimpold@i2se.com>
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/net/ethernet/qualcomm/qca_7k.c    | 34 +++++++++++++++++++++++++++++--
 drivers/net/ethernet/qualcomm/qca_7k.h    |  2 +-
 drivers/net/ethernet/qualcomm/qca_debug.c |  1 +
 drivers/net/ethernet/qualcomm/qca_spi.c   | 28 ++++++++++++++++++-------
 drivers/net/ethernet/qualcomm/qca_spi.h   |  1 +
 5 files changed, 56 insertions(+), 10 deletions(-)

Hi,
this patch requires "net: qca_spi: Fix race condition in spi transfers"
to be applied.

Stefan

diff --git a/drivers/net/ethernet/qualcomm/qca_7k.c b/drivers/net/ethernet/qualcomm/qca_7k.c
index 6c8543f..4292c89 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k.c
+++ b/drivers/net/ethernet/qualcomm/qca_7k.c
@@ -81,8 +81,8 @@ qcaspi_read_register(struct qcaspi *qca, u16 reg, u16 *result)
 	return ret;
 }
 
-int
-qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value)
+static int
+__qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value)
 {
 	__be16 tx_data[2];
 	struct spi_transfer transfer[2];
@@ -117,3 +117,33 @@ qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value)
 
 	return ret;
 }
+
+int
+qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value, int retry)
+{
+	int ret, i = 0;
+	u16 confirmed;
+
+	do {
+		ret = __qcaspi_write_register(qca, reg, value);
+		if (ret)
+			return ret;
+
+		if (!retry)
+			return 0;
+
+		ret = qcaspi_read_register(qca, reg, &confirmed);
+		if (ret)
+			return ret;
+
+		ret = confirmed != value;
+		if (!ret)
+			return 0;
+
+		i++;
+		qca->stats.write_verify_failed++;
+
+	} while (i <= retry);
+
+	return ret;
+}
diff --git a/drivers/net/ethernet/qualcomm/qca_7k.h b/drivers/net/ethernet/qualcomm/qca_7k.h
index 27124c2..356de8e 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k.h
+++ b/drivers/net/ethernet/qualcomm/qca_7k.h
@@ -66,6 +66,6 @@
 
 void qcaspi_spi_error(struct qcaspi *qca);
 int qcaspi_read_register(struct qcaspi *qca, u16 reg, u16 *result);
-int qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value);
+int qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value, int retry);
 
 #endif /* _QCA_7K_H */
diff --git a/drivers/net/ethernet/qualcomm/qca_debug.c b/drivers/net/ethernet/qualcomm/qca_debug.c
index 51d89c8..a9f1bc0 100644
--- a/drivers/net/ethernet/qualcomm/qca_debug.c
+++ b/drivers/net/ethernet/qualcomm/qca_debug.c
@@ -60,6 +60,7 @@ static const char qcaspi_gstrings_stats[][ETH_GSTRING_LEN] = {
 	"Write buffer misses",
 	"Transmit ring full",
 	"SPI errors",
+	"Write verify errors",
 };
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c b/drivers/net/ethernet/qualcomm/qca_spi.c
index 66b775d..d531050 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -69,6 +69,12 @@ static int qcaspi_pluggable = QCASPI_PLUGGABLE_MIN;
 module_param(qcaspi_pluggable, int, 0);
 MODULE_PARM_DESC(qcaspi_pluggable, "Pluggable SPI connection (yes/no).");
 
+#define QCASPI_WRITE_VERIFY_MIN 0
+#define QCASPI_WRITE_VERIFY_MAX 3
+static int wr_verify = QCASPI_WRITE_VERIFY_MIN;
+module_param(wr_verify, int, 0);
+MODULE_PARM_DESC(wr_verify, "SPI register write verify trails. Use 0-3.");
+
 #define QCASPI_TX_TIMEOUT (1 * HZ)
 #define QCASPI_QCA7K_REBOOT_TIME_MS 1000
 
@@ -77,7 +83,7 @@ start_spi_intr_handling(struct qcaspi *qca, u16 *intr_cause)
 {
 	*intr_cause = 0;
 
-	qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, 0);
+	qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, 0, wr_verify);
 	qcaspi_read_register(qca, SPI_REG_INTR_CAUSE, intr_cause);
 	netdev_dbg(qca->net_dev, "interrupts: 0x%04x\n", *intr_cause);
 }
@@ -90,8 +96,8 @@ end_spi_intr_handling(struct qcaspi *qca, u16 intr_cause)
 			   SPI_INT_RDBUF_ERR |
 			   SPI_INT_WRBUF_ERR);
 
-	qcaspi_write_register(qca, SPI_REG_INTR_CAUSE, intr_cause);
-	qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, intr_enable);
+	qcaspi_write_register(qca, SPI_REG_INTR_CAUSE, intr_cause, 0);
+	qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, intr_enable, wr_verify);
 	netdev_dbg(qca->net_dev, "acking int: 0x%04x\n", intr_cause);
 }
 
@@ -239,7 +245,7 @@ qcaspi_tx_frame(struct qcaspi *qca, struct sk_buff *skb)
 
 	len = skb->len;
 
-	qcaspi_write_register(qca, SPI_REG_BFR_SIZE, len);
+	qcaspi_write_register(qca, SPI_REG_BFR_SIZE, len, wr_verify);
 	if (qca->legacy_mode)
 		qcaspi_tx_cmd(qca, QCA7K_SPI_WRITE | QCA7K_SPI_EXTERNAL);
 
@@ -345,6 +351,7 @@ qcaspi_receive(struct qcaspi *qca)
 
 	/* Read the packet size. */
 	qcaspi_read_register(qca, SPI_REG_RDBUF_BYTE_AVA, &available);
+
 	netdev_dbg(net_dev, "qcaspi_receive: SPI_REG_RDBUF_BYTE_AVA: Value: %08x\n",
 		   available);
 
@@ -353,7 +360,7 @@ qcaspi_receive(struct qcaspi *qca)
 		return -1;
 	}
 
-	qcaspi_write_register(qca, SPI_REG_BFR_SIZE, available);
+	qcaspi_write_register(qca, SPI_REG_BFR_SIZE, available, wr_verify);
 
 	if (qca->legacy_mode)
 		qcaspi_tx_cmd(qca, QCA7K_SPI_READ | QCA7K_SPI_EXTERNAL);
@@ -524,7 +531,7 @@ qcaspi_qca7k_sync(struct qcaspi *qca, int event)
 		netdev_dbg(qca->net_dev, "sync: resetting device.\n");
 		qcaspi_read_register(qca, SPI_REG_SPI_CONFIG, &spi_config);
 		spi_config |= QCASPI_SLAVE_RESET_BIT;
-		qcaspi_write_register(qca, SPI_REG_SPI_CONFIG, spi_config);
+		qcaspi_write_register(qca, SPI_REG_SPI_CONFIG, spi_config, 0);
 
 		qca->sync = QCASPI_SYNC_RESET;
 		qca->stats.trig_reset++;
@@ -684,7 +691,7 @@ qcaspi_netdev_close(struct net_device *dev)
 
 	netif_stop_queue(dev);
 
-	qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, 0);
+	qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, 0, wr_verify);
 	free_irq(qca->spi_dev->irq, qca);
 
 	kthread_stop(qca->spi_thread);
@@ -904,6 +911,13 @@ qca_spi_probe(struct spi_device *spi)
 		return -EINVAL;
 	}
 
+	if (wr_verify < QCASPI_WRITE_VERIFY_MIN ||
+	    wr_verify > QCASPI_WRITE_VERIFY_MAX) {
+		dev_err(&spi->dev, "Invalid write verify: %d\n",
+			wr_verify);
+		return -EINVAL;
+	}
+
 	dev_info(&spi->dev, "ver=%s, clkspeed=%d, burst_len=%d, pluggable=%d\n",
 		 QCASPI_DRV_VERSION,
 		 qcaspi_clkspeed,
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.h b/drivers/net/ethernet/qualcomm/qca_spi.h
index fc0e987..2d2c497 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.h
+++ b/drivers/net/ethernet/qualcomm/qca_spi.h
@@ -73,6 +73,7 @@ struct qcaspi_stats {
 	u64 write_buf_miss;
 	u64 ring_full;
 	u64 spi_err;
+	u64 write_verify_failed;
 };
 
 struct qcaspi {
-- 
2.7.4

^ permalink raw reply related

* Re: kernels > v4.12 oops/crash with ipsec-traffic: bisected to b838d5e1c5b6e57b10ec8af2268824041e3ea911: ipv4: mark DST_NOGC and remove the operation of dst_free()
From: Steffen Klassert @ 2018-09-11 10:33 UTC (permalink / raw)
  To: Kristian Evensen
  Cc: linux, Network Development, weiwan, Tobias Hommel, edumazet
In-Reply-To: <CAKfDRXg7EJeoO--QFFEJLtSPqRVngy4etSQHjSkQwNnhc4RFRA@mail.gmail.com>

On Mon, Sep 10, 2018 at 10:18:47AM +0200, Kristian Evensen wrote:
> Hi,
> 
> Thanks everyone for all the effort in debugging this issue.
> 
> On Mon, Sep 10, 2018 at 8:39 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> > The easy fix that could be backported to stable would be
> > to check skb->dst for NULL and drop the packet in that case.
> 
> Thought I should just chime in and say that we deployed this
> work-around when we started observing the error back in June. Since
> then we have not seen any crashes. Also, we have instrumented some of
> our kernels to count the number of times the error is hit (overall +
> consecutive). Compared to the overall number of packets, the error
> happens very rarely. With our workloads, we on average see the error
> once every couple of days.

Thanks for letting us know!

I plan to fix this in the ipsec tree with:

Subject: [PATCH RFC] xfrm: Fix NULL pointer dereference when skb_dst_force clears
 the dst_entry.

Since commit 222d7dbd258d ("net: prevent dst uses after free")
skb_dst_force() might clear the dst_entry attached to the skb.
The xfrm code don't expect this to happen, so we crash with
a NULL pointer dereference in this case. Fix it by checking
skb_dst(skb) for NULL after skb_dst_force() and drop the packet
in cast the dst_entry was cleared.

Fixes: 222d7dbd258d ("net: prevent dst uses after free")
Reported-by: Tobias Hommel <netdev-list@genoetigt.de>
Reported-by: Kristian Evensen <kristian.evensen@gmail.com>
Reported-by: Wolfgang Walter <linux@stwm.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_output.c | 4 ++++
 net/xfrm/xfrm_policy.c | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index 89b178a78dc7..36d15a38ce5e 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -101,6 +101,10 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
 		spin_unlock_bh(&x->lock);
 
 		skb_dst_force(skb);
+		if (!skb_dst(skb)) {
+			XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
+			goto error_nolock;
+		}
 
 		if (xfrm_offload(skb)) {
 			x->type_offload->encap(x, skb);
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 7c5e8978aeaa..626e0f4d1749 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2548,6 +2548,10 @@ int __xfrm_route_forward(struct sk_buff *skb, unsigned short family)
 	}
 
 	skb_dst_force(skb);
+	if (!skb_dst(skb)) {
+		XFRM_INC_STATS(net, LINUX_MIB_XFRMFWDHDRERROR);
+		return 0;
+	}
 
 	dst = xfrm_lookup(net, skb_dst(skb), &fl, NULL, XFRM_LOOKUP_QUEUE);
 	if (IS_ERR(dst)) {
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCHv2 iproute2] bridge/mdb: fix missing new line when show bridge mdb
From: Phil Sutter @ 2018-09-11 11:01 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: netdev, Stephen Hemminger, David Ahern
In-Reply-To: <1536629195-12540-1-git-send-email-liuhangbin@gmail.com>

Hi Hangbin,

On Tue, Sep 11, 2018 at 09:26:35AM +0800, Hangbin Liu wrote:
[...]
> +	if (!is_json_context() && !show_stats)
> +		print_string(PRINT_FP, NULL, "\n", NULL);

There is no need to check for !is_json_context() here. You give a type
of PRINT_FP which won't lead to output if JSON is active. For reference,
check print_color_string() in lib/json_print.c and _IS_JSON_CONTEXT
macro.

Cheers, Phil

^ permalink raw reply

* Re: unexpected GRO/veth behavior
From: Toshiaki Makita @ 2018-09-11 11:07 UTC (permalink / raw)
  To: Eric Dumazet, Paolo Abeni, netdev
In-Reply-To: <ecb079cc-033d-0726-75ce-00eada2ae765@gmail.com>

On 2018/09/11 19:27, Eric Dumazet wrote:
...
> Fix would probably be :
> 
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 8d679c8b7f25c753d77cfb8821d9d2528c9c9048..96bd94480942b469403abf017f9f9d5be1e23ef5 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -602,9 +602,10 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget, unsigned int *xdp_xmit)
>                         skb = veth_xdp_rcv_skb(rq, ptr, xdp_xmit);
>                 }
>  
> -               if (skb)
> +               if (skb) {
> +                       skb_orphan(skb);
>                         napi_gro_receive(&rq->xdp_napi, skb);
> -
> +               }
>                 done++;
>         }

Considering commit 9c4c3252 ("skbuff: preserve sock reference when
scrubbing the skb.") I'm not sure if we should unconditionally orphan
the skb here.
I was thinking I should call netif_receive_skb() for such packets
instead of napi_gro_receive().

-- 
Toshiaki Makita

^ permalink raw reply

* tools/bpf regression causing samples/bpf/ to hang
From: Björn Töpel @ 2018-09-11 11:11 UTC (permalink / raw)
  To: Netdev, yhs; +Cc: ast, Daniel Borkmann, Jesper Dangaard Brouer

Hi Yonghong, I tried to run the XDP samples from the bpf-next tip
today, and was hit by a regression.

Commit f7010770fbac ("tools/bpf: move bpf/lib netlink related
functions into a new file") adds a while(1) around the recv call in
bpf_set_link_xdp_fd making that call getting stuck in an infinite
loop.

I simply removed the loop, and that solved my problem (patch below).

However, I don't know if removing the loop would break bpftool for
you. If not, I can submit the patch as a proper one for bpf-next.

Thanks!
Björn

From: =?UTF-8?q?Bj=C3=B6rn=20T=C3=B6pel?= <bjorn.topel@intel.com>
Date: Tue, 11 Sep 2018 12:35:44 +0200
Subject: [PATCH] tools/bpf: remove loop around netlink recv

Commit f7010770fbac ("tools/bpf: move bpf/lib netlink related
functions into a new file") moved the bpf_set_link_xdp_fd and split it
up into multiple functions. The added receive function
bpf_netlink_recv added a loop around the recv syscall leading to
multiple recv calls. This resulted in all XDP samples in the
samples/bpf/ to stop working, since they were stuck in a blocking
recv.

This commits removes the while (1)-statement.

Fixes: f7010770fbac ("tools/bpf: move bpf/lib netlink related
functions into a new file")
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 tools/lib/bpf/netlink.c | 64 ++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 33 deletions(-)

diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
index 469e068dd0c5..0eae1fbf46c6 100644
--- a/tools/lib/bpf/netlink.c
+++ b/tools/lib/bpf/netlink.c
@@ -70,41 +70,39 @@ static int bpf_netlink_recv(int sock, __u32 nl_pid, int seq,
     char buf[4096];
     int len, ret;

-    while (1) {
-        len = recv(sock, buf, sizeof(buf), 0);
-        if (len < 0) {
-            ret = -errno;
+    len = recv(sock, buf, sizeof(buf), 0);
+    if (len < 0) {
+        ret = -errno;
+        goto done;
+    }
+
+    for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
+         nh = NLMSG_NEXT(nh, len)) {
+        if (nh->nlmsg_pid != nl_pid) {
+            ret = -LIBBPF_ERRNO__WRNGPID;
             goto done;
         }
-
-        for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
-             nh = NLMSG_NEXT(nh, len)) {
-            if (nh->nlmsg_pid != nl_pid) {
-                ret = -LIBBPF_ERRNO__WRNGPID;
-                goto done;
-            }
-            if (nh->nlmsg_seq != seq) {
-                ret = -LIBBPF_ERRNO__INVSEQ;
-                goto done;
-            }
-            switch (nh->nlmsg_type) {
-            case NLMSG_ERROR:
-                err = (struct nlmsgerr *)NLMSG_DATA(nh);
-                if (!err->error)
-                    continue;
-                ret = err->error;
-                nla_dump_errormsg(nh);
-                goto done;
-            case NLMSG_DONE:
-                return 0;
-            default:
-                break;
-            }
-            if (_fn) {
-                ret = _fn(nh, fn, cookie);
-                if (ret)
-                    return ret;
-            }
+        if (nh->nlmsg_seq != seq) {
+            ret = -LIBBPF_ERRNO__INVSEQ;
+            goto done;
+        }
+        switch (nh->nlmsg_type) {
+        case NLMSG_ERROR:
+            err = (struct nlmsgerr *)NLMSG_DATA(nh);
+            if (!err->error)
+                continue;
+            ret = err->error;
+            nla_dump_errormsg(nh);
+            goto done;
+        case NLMSG_DONE:
+            return 0;
+        default:
+            break;
+        }
+        if (_fn) {
+            ret = _fn(nh, fn, cookie);
+            if (ret)
+                return ret;
         }
     }
     ret = 0;
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH v3 00/15] soc: octeontx2: Add RVU admin function driver
From: Sunil Kovvuri @ 2018-09-11 16:28 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: LKML, olof, LAKML, linux-soc, Andrew Lunn, David S. Miller,
	Linux Netdev List, Sunil Goutham
In-Reply-To: <CAK8P3a32gpNX42NuzmbW1PbsHpq7v7LoQ6WDeKtt=QiYbuyM7w@mail.gmail.com>

On Tue, Sep 11, 2018 at 7:07 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Sep 11, 2018 at 2:37 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> >
> > Didn't receive any feedback for the v3 patch series over a week's time.
> > Can you please pick up these patches to merge into arm-soc ?
>
> I would still prefer to see the whole thing as part of drivers/net/
> instead of drivers/soc,
> and reviewed in full on the netdev side, including the parts that are
> not ethernet specific.
>
>        Arnd

Hmm.. I agree that there are many networking terms used in the driver
but it's not a
networking driver, it's just a HW configuration driver which includes
how HW should
parse the packet. This driver doesn't fit into drivers/net.

Let's say if netdev driver in drivers/net/ethernet doesn't make use of
crypto feature
then i guess netdev maintainers would reject any patches which configure crypto
block. Also as i have been saying there are other scenarios as well.
Future silicons may add support for other features into this resource
virtualization unit's domain.
An example would be compression. Any patches which do compression
related HW configuration
might be rejected by netdev maintainers, cause they are no way related
to networking.

I will keep netdev mailing list in all the patch submissions but
moving this driver into drivers/net
doesn't sound right, from it's functionality perspective.

Thanks,
Sunil.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox