* [PATCH net-next 06/10] nfp: dump all hwinfo
From: Simon Horman @ 2017-12-04 22:34 UTC (permalink / raw)
To: David Miller, Jakub Kicinski
Cc: netdev, oss-drivers, Carl Heymann, Simon Horman
In-Reply-To: <20171204223421.19174-1-simon.horman@netronome.com>
From: Carl Heymann <carl.heymann@netronome.com>
- Dump hwinfo as separate TLV chunk, in a packed format containing
zero-separated key and value strings.
- This provides additional debug context, if requested by the dumpspec.
Signed-off-by: Carl Heymann <carl.heymann@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
.../net/ethernet/netronome/nfp/nfp_net_debugdump.c | 33 ++++++++++++++++++++++
drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h | 2 ++
.../ethernet/netronome/nfp/nfpcore/nfp_hwinfo.c | 10 +++++++
3 files changed, 45 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
index 746e114a8da8..449cf0a3a158 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
@@ -44,6 +44,7 @@
enum nfp_dumpspec_type {
NFP_DUMPSPEC_TYPE_RTSYM = 4,
+ NFP_DUMPSPEC_TYPE_HWINFO = 5,
NFP_DUMPSPEC_TYPE_PROLOG = 10000,
NFP_DUMPSPEC_TYPE_ERROR = 10001,
};
@@ -222,11 +223,16 @@ static int
nfp_add_tlv_size(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
{
u32 *size = param;
+ u32 hwinfo_size;
switch (be32_to_cpu(tl->type)) {
case NFP_DUMPSPEC_TYPE_RTSYM:
*size += nfp_calc_rtsym_dump_sz(pf, tl);
break;
+ case NFP_DUMPSPEC_TYPE_HWINFO:
+ hwinfo_size = nfp_hwinfo_get_packed_str_size(pf->hwinfo);
+ *size += sizeof(struct nfp_dump_tl) + ALIGN8(hwinfo_size);
+ break;
default:
*size += nfp_dump_error_tlv_size(tl);
break;
@@ -307,6 +313,28 @@ nfp_dump_error_tlv(struct nfp_dump_tl *spec, int error,
}
static int
+nfp_dump_hwinfo(struct nfp_pf *pf, struct nfp_dump_tl *spec,
+ struct nfp_dump_state *dump)
+{
+ struct nfp_dump_tl *dump_header = dump->p;
+ u32 hwinfo_size, total_size;
+ char *hwinfo;
+ int err;
+
+ hwinfo = nfp_hwinfo_get_packed_strings(pf->hwinfo);
+ hwinfo_size = nfp_hwinfo_get_packed_str_size(pf->hwinfo);
+ total_size = sizeof(*dump_header) + ALIGN8(hwinfo_size);
+
+ err = nfp_add_tlv(NFP_DUMPSPEC_TYPE_HWINFO, total_size, dump);
+ if (err)
+ return err;
+
+ memcpy(dump_header->data, hwinfo, hwinfo_size);
+
+ return 0;
+}
+
+static int
nfp_dump_single_rtsym(struct nfp_pf *pf, struct nfp_dumpspec_rtsym *spec,
struct nfp_dump_state *dump)
{
@@ -377,6 +405,11 @@ nfp_dump_for_tlv(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
if (err)
return err;
break;
+ case NFP_DUMPSPEC_TYPE_HWINFO:
+ err = nfp_dump_hwinfo(pf, tl, dump);
+ if (err)
+ return err;
+ break;
default:
err = nfp_dump_error_tlv(tl, -EOPNOTSUPP, dump);
if (err)
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h
index 3ce51f03126f..ced62d112aa2 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h
@@ -49,6 +49,8 @@
struct nfp_hwinfo;
struct nfp_hwinfo *nfp_hwinfo_read(struct nfp_cpp *cpp);
const char *nfp_hwinfo_lookup(struct nfp_hwinfo *hwinfo, const char *lookup);
+char *nfp_hwinfo_get_packed_strings(struct nfp_hwinfo *hwinfo);
+u32 nfp_hwinfo_get_packed_str_size(struct nfp_hwinfo *hwinfo);
/* Implemented in nfp_nsp.c, low level functions */
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_hwinfo.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_hwinfo.c
index 4f24aff1e772..063a9a6243d6 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_hwinfo.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_hwinfo.c
@@ -302,3 +302,13 @@ const char *nfp_hwinfo_lookup(struct nfp_hwinfo *hwinfo, const char *lookup)
return NULL;
}
+
+char *nfp_hwinfo_get_packed_strings(struct nfp_hwinfo *hwinfo)
+{
+ return hwinfo->data;
+}
+
+u32 nfp_hwinfo_get_packed_str_size(struct nfp_hwinfo *hwinfo)
+{
+ return le32_to_cpu(hwinfo->size) - sizeof(u32);
+}
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 07/10] nfp: dump single hwinfo field by key
From: Simon Horman @ 2017-12-04 22:34 UTC (permalink / raw)
To: David Miller, Jakub Kicinski
Cc: netdev, oss-drivers, Carl Heymann, Simon Horman
In-Reply-To: <20171204223421.19174-1-simon.horman@netronome.com>
From: Carl Heymann <carl.heymann@netronome.com>
- Add spec TLV for hwinfo field, containing key string as data.
- Add dump TLV for hwinfo field, with data being key and value as packed
zero-terminated strings.
- If specified hwinfo field is not found, dump the spec TLV as -ENOENT
error.
Signed-off-by: Carl Heymann <carl.heymann@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
.../net/ethernet/netronome/nfp/nfp_net_debugdump.c | 57 ++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
index 449cf0a3a158..833036715ca0 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
@@ -45,6 +45,7 @@
enum nfp_dumpspec_type {
NFP_DUMPSPEC_TYPE_RTSYM = 4,
NFP_DUMPSPEC_TYPE_HWINFO = 5,
+ NFP_DUMPSPEC_TYPE_HWINFO_FIELD = 7,
NFP_DUMPSPEC_TYPE_PROLOG = 10000,
NFP_DUMPSPEC_TYPE_ERROR = 10001,
};
@@ -197,6 +198,23 @@ static int nfp_dump_error_tlv_size(struct nfp_dump_tl *spec)
be32_to_cpu(spec->length));
}
+static int nfp_calc_hwinfo_field_sz(struct nfp_pf *pf, struct nfp_dump_tl *spec)
+{
+ u32 tl_len, key_len;
+ const char *value;
+
+ tl_len = be32_to_cpu(spec->length);
+ key_len = strnlen(spec->data, tl_len);
+ if (key_len == tl_len)
+ return nfp_dump_error_tlv_size(spec);
+
+ value = nfp_hwinfo_lookup(pf->hwinfo, spec->data);
+ if (!value)
+ return nfp_dump_error_tlv_size(spec);
+
+ return sizeof(struct nfp_dump_tl) + ALIGN8(key_len + strlen(value) + 2);
+}
+
static int
nfp_calc_rtsym_dump_sz(struct nfp_pf *pf, struct nfp_dump_tl *spec)
{
@@ -233,6 +251,9 @@ nfp_add_tlv_size(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
hwinfo_size = nfp_hwinfo_get_packed_str_size(pf->hwinfo);
*size += sizeof(struct nfp_dump_tl) + ALIGN8(hwinfo_size);
break;
+ case NFP_DUMPSPEC_TYPE_HWINFO_FIELD:
+ *size += nfp_calc_hwinfo_field_sz(pf, tl);
+ break;
default:
*size += nfp_dump_error_tlv_size(tl);
break;
@@ -334,6 +355,37 @@ nfp_dump_hwinfo(struct nfp_pf *pf, struct nfp_dump_tl *spec,
return 0;
}
+static int nfp_dump_hwinfo_field(struct nfp_pf *pf, struct nfp_dump_tl *spec,
+ struct nfp_dump_state *dump)
+{
+ struct nfp_dump_tl *dump_header = dump->p;
+ u32 tl_len, key_len, val_len;
+ const char *key, *value;
+ u32 total_size;
+ int err;
+
+ tl_len = be32_to_cpu(spec->length);
+ key_len = strnlen(spec->data, tl_len);
+ if (key_len == tl_len)
+ return nfp_dump_error_tlv(spec, -EINVAL, dump);
+
+ key = spec->data;
+ value = nfp_hwinfo_lookup(pf->hwinfo, key);
+ if (!value)
+ return nfp_dump_error_tlv(spec, -ENOENT, dump);
+
+ val_len = strlen(value);
+ total_size = sizeof(*dump_header) + ALIGN8(key_len + val_len + 2);
+ err = nfp_add_tlv(NFP_DUMPSPEC_TYPE_HWINFO_FIELD, total_size, dump);
+ if (err)
+ return err;
+
+ memcpy(dump_header->data, key, key_len + 1);
+ memcpy(dump_header->data + key_len + 1, value, val_len + 1);
+
+ return 0;
+}
+
static int
nfp_dump_single_rtsym(struct nfp_pf *pf, struct nfp_dumpspec_rtsym *spec,
struct nfp_dump_state *dump)
@@ -410,6 +462,11 @@ nfp_dump_for_tlv(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
if (err)
return err;
break;
+ case NFP_DUMPSPEC_TYPE_HWINFO_FIELD:
+ err = nfp_dump_hwinfo_field(pf, tl, dump);
+ if (err)
+ return err;
+ break;
default:
err = nfp_dump_error_tlv(tl, -EOPNOTSUPP, dump);
if (err)
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 08/10] nfp: dump firmware name
From: Simon Horman @ 2017-12-04 22:34 UTC (permalink / raw)
To: David Miller, Jakub Kicinski
Cc: netdev, oss-drivers, Carl Heymann, Simon Horman
In-Reply-To: <20171204223421.19174-1-simon.horman@netronome.com>
From: Carl Heymann <carl.heymann@netronome.com>
Dump FW name as TLV, based on dump specification.
Signed-off-by: Carl Heymann <carl.heymann@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
.../net/ethernet/netronome/nfp/nfp_net_debugdump.c | 36 ++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
index 833036715ca0..1e57dab60abf 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
@@ -45,6 +45,7 @@
enum nfp_dumpspec_type {
NFP_DUMPSPEC_TYPE_RTSYM = 4,
NFP_DUMPSPEC_TYPE_HWINFO = 5,
+ NFP_DUMPSPEC_TYPE_FWNAME = 6,
NFP_DUMPSPEC_TYPE_HWINFO_FIELD = 7,
NFP_DUMPSPEC_TYPE_PROLOG = 10000,
NFP_DUMPSPEC_TYPE_ERROR = 10001,
@@ -198,6 +199,13 @@ static int nfp_dump_error_tlv_size(struct nfp_dump_tl *spec)
be32_to_cpu(spec->length));
}
+static int nfp_calc_fwname_tlv_size(struct nfp_pf *pf)
+{
+ u32 fwname_len = strlen(nfp_mip_name(pf->mip));
+
+ return sizeof(struct nfp_dump_tl) + ALIGN8(fwname_len + 1);
+}
+
static int nfp_calc_hwinfo_field_sz(struct nfp_pf *pf, struct nfp_dump_tl *spec)
{
u32 tl_len, key_len;
@@ -244,6 +252,9 @@ nfp_add_tlv_size(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
u32 hwinfo_size;
switch (be32_to_cpu(tl->type)) {
+ case NFP_DUMPSPEC_TYPE_FWNAME:
+ *size += nfp_calc_fwname_tlv_size(pf);
+ break;
case NFP_DUMPSPEC_TYPE_RTSYM:
*size += nfp_calc_rtsym_dump_sz(pf, tl);
break;
@@ -333,6 +344,26 @@ nfp_dump_error_tlv(struct nfp_dump_tl *spec, int error,
return 0;
}
+static int nfp_dump_fwname(struct nfp_pf *pf, struct nfp_dump_state *dump)
+{
+ struct nfp_dump_tl *dump_header = dump->p;
+ u32 fwname_len, total_size;
+ const char *fwname;
+ int err;
+
+ fwname = nfp_mip_name(pf->mip);
+ fwname_len = strlen(fwname);
+ total_size = sizeof(*dump_header) + ALIGN8(fwname_len + 1);
+
+ err = nfp_add_tlv(NFP_DUMPSPEC_TYPE_FWNAME, total_size, dump);
+ if (err)
+ return err;
+
+ memcpy(dump_header->data, fwname, fwname_len);
+
+ return 0;
+}
+
static int
nfp_dump_hwinfo(struct nfp_pf *pf, struct nfp_dump_tl *spec,
struct nfp_dump_state *dump)
@@ -451,6 +482,11 @@ nfp_dump_for_tlv(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
int err;
switch (be32_to_cpu(tl->type)) {
+ case NFP_DUMPSPEC_TYPE_FWNAME:
+ err = nfp_dump_fwname(pf, dump);
+ if (err)
+ return err;
+ break;
case NFP_DUMPSPEC_TYPE_RTSYM:
spec_rtsym = (struct nfp_dumpspec_rtsym *)tl;
err = nfp_dump_single_rtsym(pf, spec_rtsym, dump);
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 09/10] nfp: dump CPP, XPB and direct ME CSRs
From: Simon Horman @ 2017-12-04 22:34 UTC (permalink / raw)
To: David Miller, Jakub Kicinski
Cc: netdev, oss-drivers, Carl Heymann, Simon Horman
In-Reply-To: <20171204223421.19174-1-simon.horman@netronome.com>
From: Carl Heymann <carl.heymann@netronome.com>
- The spec defines CSR address ranges for these types.
- Dump each TLV chunk in the spec as a chunk that includes the spec and
the data over the defined address range.
Signed-off-by: Carl Heymann <carl.heymann@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
.../net/ethernet/netronome/nfp/nfp_net_debugdump.c | 102 +++++++++++++++++++++
1 file changed, 102 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
index 1e57dab60abf..b23e2e0570af 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
@@ -43,6 +43,9 @@
#define ALIGN8(x) ALIGN(x, 8)
enum nfp_dumpspec_type {
+ NFP_DUMPSPEC_TYPE_CPP_CSR = 0,
+ NFP_DUMPSPEC_TYPE_XPB_CSR = 1,
+ NFP_DUMPSPEC_TYPE_ME_CSR = 2,
NFP_DUMPSPEC_TYPE_RTSYM = 4,
NFP_DUMPSPEC_TYPE_HWINFO = 5,
NFP_DUMPSPEC_TYPE_FWNAME = 6,
@@ -77,11 +80,27 @@ struct nfp_dump_common_cpp {
__be32 dump_length; /* total bytes to dump, aligned to reg size */
};
+/* CSR dumpables */
+struct nfp_dumpspec_csr {
+ struct nfp_dump_tl tl;
+ struct nfp_dump_common_cpp cpp;
+ __be32 register_width; /* in bits */
+};
+
struct nfp_dumpspec_rtsym {
struct nfp_dump_tl tl;
char rtsym[0];
};
+/* header for register dumpable */
+struct nfp_dump_csr {
+ struct nfp_dump_tl tl;
+ struct nfp_dump_common_cpp cpp;
+ __be32 register_width; /* in bits */
+ __be32 error; /* error code encountered while reading */
+ __be32 error_offset; /* offset being read when error occurred */
+};
+
struct nfp_dump_rtsym {
struct nfp_dump_tl tl;
struct nfp_dump_common_cpp cpp;
@@ -223,6 +242,20 @@ static int nfp_calc_hwinfo_field_sz(struct nfp_pf *pf, struct nfp_dump_tl *spec)
return sizeof(struct nfp_dump_tl) + ALIGN8(key_len + strlen(value) + 2);
}
+static bool nfp_csr_spec_valid(struct nfp_dumpspec_csr *spec_csr)
+{
+ u32 required_read_sz = sizeof(*spec_csr) - sizeof(spec_csr->tl);
+ u32 available_sz = be32_to_cpu(spec_csr->tl.length);
+ u32 reg_width;
+
+ if (available_sz < required_read_sz)
+ return false;
+
+ reg_width = be32_to_cpu(spec_csr->register_width);
+
+ return reg_width == 32 || reg_width == 64;
+}
+
static int
nfp_calc_rtsym_dump_sz(struct nfp_pf *pf, struct nfp_dump_tl *spec)
{
@@ -248,6 +281,7 @@ nfp_calc_rtsym_dump_sz(struct nfp_pf *pf, struct nfp_dump_tl *spec)
static int
nfp_add_tlv_size(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
{
+ struct nfp_dumpspec_csr *spec_csr;
u32 *size = param;
u32 hwinfo_size;
@@ -255,6 +289,16 @@ nfp_add_tlv_size(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
case NFP_DUMPSPEC_TYPE_FWNAME:
*size += nfp_calc_fwname_tlv_size(pf);
break;
+ case NFP_DUMPSPEC_TYPE_CPP_CSR:
+ case NFP_DUMPSPEC_TYPE_XPB_CSR:
+ case NFP_DUMPSPEC_TYPE_ME_CSR:
+ spec_csr = (struct nfp_dumpspec_csr *)tl;
+ if (!nfp_csr_spec_valid(spec_csr))
+ *size += nfp_dump_error_tlv_size(tl);
+ else
+ *size += ALIGN8(sizeof(struct nfp_dump_csr)) +
+ ALIGN8(be32_to_cpu(spec_csr->cpp.dump_length));
+ break;
case NFP_DUMPSPEC_TYPE_RTSYM:
*size += nfp_calc_rtsym_dump_sz(pf, tl);
break;
@@ -418,6 +462,55 @@ static int nfp_dump_hwinfo_field(struct nfp_pf *pf, struct nfp_dump_tl *spec,
}
static int
+nfp_dump_csr_range(struct nfp_pf *pf, struct nfp_dumpspec_csr *spec_csr,
+ struct nfp_dump_state *dump)
+{
+ struct nfp_dump_csr *dump_header = dump->p;
+ u32 reg_sz, header_size, total_size;
+ u32 cpp_rd_addr, max_rd_addr;
+ int bytes_read;
+ void *dest;
+ u32 cpp_id;
+ int err;
+
+ if (!nfp_csr_spec_valid(spec_csr))
+ return nfp_dump_error_tlv(&spec_csr->tl, -EINVAL, dump);
+
+ reg_sz = be32_to_cpu(spec_csr->register_width) / BITS_PER_BYTE;
+ header_size = ALIGN8(sizeof(*dump_header));
+ total_size = header_size +
+ ALIGN8(be32_to_cpu(spec_csr->cpp.dump_length));
+ dest = dump->p + header_size;
+
+ err = nfp_add_tlv(be32_to_cpu(spec_csr->tl.type), total_size, dump);
+ if (err)
+ return err;
+
+ dump_header->cpp = spec_csr->cpp;
+ dump_header->register_width = spec_csr->register_width;
+
+ cpp_id = nfp_get_numeric_cpp_id(&spec_csr->cpp.cpp_id);
+ cpp_rd_addr = be32_to_cpu(spec_csr->cpp.offset);
+ max_rd_addr = cpp_rd_addr + be32_to_cpu(spec_csr->cpp.dump_length);
+
+ while (cpp_rd_addr < max_rd_addr) {
+ bytes_read = nfp_cpp_read(pf->cpp, cpp_id, cpp_rd_addr, dest,
+ reg_sz);
+ if (bytes_read != reg_sz) {
+ if (bytes_read >= 0)
+ bytes_read = -EIO;
+ dump_header->error = cpu_to_be32(bytes_read);
+ dump_header->error_offset = cpu_to_be32(cpp_rd_addr);
+ break;
+ }
+ cpp_rd_addr += reg_sz;
+ dest += reg_sz;
+ }
+
+ return 0;
+}
+
+static int
nfp_dump_single_rtsym(struct nfp_pf *pf, struct nfp_dumpspec_rtsym *spec,
struct nfp_dump_state *dump)
{
@@ -479,6 +572,7 @@ nfp_dump_for_tlv(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
{
struct nfp_dumpspec_rtsym *spec_rtsym;
struct nfp_dump_state *dump = param;
+ struct nfp_dumpspec_csr *spec_csr;
int err;
switch (be32_to_cpu(tl->type)) {
@@ -487,6 +581,14 @@ nfp_dump_for_tlv(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
if (err)
return err;
break;
+ case NFP_DUMPSPEC_TYPE_CPP_CSR:
+ case NFP_DUMPSPEC_TYPE_XPB_CSR:
+ case NFP_DUMPSPEC_TYPE_ME_CSR:
+ spec_csr = (struct nfp_dumpspec_csr *)tl;
+ err = nfp_dump_csr_range(pf, spec_csr, dump);
+ if (err)
+ return err;
+ break;
case NFP_DUMPSPEC_TYPE_RTSYM:
spec_rtsym = (struct nfp_dumpspec_rtsym *)tl;
err = nfp_dump_single_rtsym(pf, spec_rtsym, dump);
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 10/10] nfp: dump indirect ME CSRs
From: Simon Horman @ 2017-12-04 22:34 UTC (permalink / raw)
To: David Miller, Jakub Kicinski
Cc: netdev, oss-drivers, Carl Heymann, Simon Horman
In-Reply-To: <20171204223421.19174-1-simon.horman@netronome.com>
From: Carl Heymann <carl.heymann@netronome.com>
- The spec defines CSR address ranges for indirect ME CSRs. For Each TLV
chunk in the spec, dump a chunk that includes the spec and the data
over the defined address range.
- Each indirect CSR has 8 contexts. To read one context, first write the
context to a specific derived address, read it back, and then read the
register value.
- For each address, read and dump all 8 contexts in this manner.
Signed-off-by: Carl Heymann <carl.heymann@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
drivers/net/ethernet/netronome/nfp/nfp_asm.h | 10 ++
.../net/ethernet/netronome/nfp/nfp_net_debugdump.c | 113 +++++++++++++++++++++
2 files changed, 123 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_asm.h b/drivers/net/ethernet/netronome/nfp/nfp_asm.h
index 98803f9f40b6..3387e6926eb0 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_asm.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_asm.h
@@ -262,6 +262,7 @@ enum lcsr_wr_src {
#define OP_CARB_BASE 0x0e000000000ULL
#define OP_CARB_OR 0x00000010000ULL
+#define NFP_CSR_CTX_PTR 0x20
#define NFP_CSR_ACT_LM_ADDR0 0x64
#define NFP_CSR_ACT_LM_ADDR1 0x6c
#define NFP_CSR_ACT_LM_ADDR2 0x94
@@ -382,4 +383,13 @@ int swreg_to_restricted(swreg dst, swreg lreg, swreg rreg,
int nfp_ustore_check_valid_no_ecc(u64 insn);
u64 nfp_ustore_calc_ecc_insn(u64 insn);
+#define NFP_IND_ME_REFL_WR_SIG_INIT 3
+#define NFP_IND_ME_CTX_PTR_BASE_MASK GENMASK(9, 0)
+#define NFP_IND_NUM_CONTEXTS 8
+
+static inline u32 nfp_get_ind_csr_ctx_ptr_offs(u32 read_offset)
+{
+ return (read_offset & ~NFP_IND_ME_CTX_PTR_BASE_MASK) | NFP_CSR_CTX_PTR;
+}
+
#endif
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
index b23e2e0570af..cb74602f0907 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
@@ -34,6 +34,7 @@
#include <linux/ethtool.h>
#include <linux/vmalloc.h>
+#include "nfp_asm.h"
#include "nfp_main.h"
#include "nfpcore/nfp.h"
#include "nfpcore/nfp_nffw.h"
@@ -46,6 +47,7 @@ enum nfp_dumpspec_type {
NFP_DUMPSPEC_TYPE_CPP_CSR = 0,
NFP_DUMPSPEC_TYPE_XPB_CSR = 1,
NFP_DUMPSPEC_TYPE_ME_CSR = 2,
+ NFP_DUMPSPEC_TYPE_INDIRECT_ME_CSR = 3,
NFP_DUMPSPEC_TYPE_RTSYM = 4,
NFP_DUMPSPEC_TYPE_HWINFO = 5,
NFP_DUMPSPEC_TYPE_FWNAME = 6,
@@ -299,6 +301,15 @@ nfp_add_tlv_size(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
*size += ALIGN8(sizeof(struct nfp_dump_csr)) +
ALIGN8(be32_to_cpu(spec_csr->cpp.dump_length));
break;
+ case NFP_DUMPSPEC_TYPE_INDIRECT_ME_CSR:
+ spec_csr = (struct nfp_dumpspec_csr *)tl;
+ if (!nfp_csr_spec_valid(spec_csr))
+ *size += nfp_dump_error_tlv_size(tl);
+ else
+ *size += ALIGN8(sizeof(struct nfp_dump_csr)) +
+ ALIGN8(be32_to_cpu(spec_csr->cpp.dump_length) *
+ NFP_IND_NUM_CONTEXTS);
+ break;
case NFP_DUMPSPEC_TYPE_RTSYM:
*size += nfp_calc_rtsym_dump_sz(pf, tl);
break;
@@ -510,6 +521,102 @@ nfp_dump_csr_range(struct nfp_pf *pf, struct nfp_dumpspec_csr *spec_csr,
return 0;
}
+/* Write context to CSRCtxPtr, then read from it. Then the value can be read
+ * from IndCtxStatus.
+ */
+static int
+nfp_read_indirect_csr(struct nfp_cpp *cpp,
+ struct nfp_dumpspec_cpp_isl_id cpp_params, u32 offset,
+ u32 reg_sz, u32 context, void *dest)
+{
+ u32 csr_ctx_ptr_offs;
+ u32 cpp_id;
+ int result;
+
+ csr_ctx_ptr_offs = nfp_get_ind_csr_ctx_ptr_offs(offset);
+ cpp_id = NFP_CPP_ISLAND_ID(cpp_params.target,
+ NFP_IND_ME_REFL_WR_SIG_INIT,
+ cpp_params.token, cpp_params.island);
+ result = nfp_cpp_writel(cpp, cpp_id, csr_ctx_ptr_offs, context);
+ if (result != sizeof(context))
+ return result < 0 ? result : -EIO;
+
+ cpp_id = nfp_get_numeric_cpp_id(&cpp_params);
+ result = nfp_cpp_read(cpp, cpp_id, csr_ctx_ptr_offs, dest, reg_sz);
+ if (result != reg_sz)
+ return result < 0 ? result : -EIO;
+
+ result = nfp_cpp_read(cpp, cpp_id, offset, dest, reg_sz);
+ if (result != reg_sz)
+ return result < 0 ? result : -EIO;
+
+ return 0;
+}
+
+static int
+nfp_read_all_indirect_csr_ctx(struct nfp_cpp *cpp,
+ struct nfp_dumpspec_csr *spec_csr, u32 address,
+ u32 reg_sz, void *dest)
+{
+ u32 ctx;
+ int err;
+
+ for (ctx = 0; ctx < NFP_IND_NUM_CONTEXTS; ctx++) {
+ err = nfp_read_indirect_csr(cpp, spec_csr->cpp.cpp_id, address,
+ reg_sz, ctx, dest + ctx * reg_sz);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+static int
+nfp_dump_indirect_csr_range(struct nfp_pf *pf,
+ struct nfp_dumpspec_csr *spec_csr,
+ struct nfp_dump_state *dump)
+{
+ struct nfp_dump_csr *dump_header = dump->p;
+ u32 reg_sz, header_size, total_size;
+ u32 cpp_rd_addr, max_rd_addr;
+ u32 reg_data_length;
+ void *dest;
+ int err;
+
+ if (!nfp_csr_spec_valid(spec_csr))
+ return nfp_dump_error_tlv(&spec_csr->tl, -EINVAL, dump);
+
+ reg_sz = be32_to_cpu(spec_csr->register_width) / BITS_PER_BYTE;
+ header_size = ALIGN8(sizeof(*dump_header));
+ reg_data_length = be32_to_cpu(spec_csr->cpp.dump_length) *
+ NFP_IND_NUM_CONTEXTS;
+ total_size = header_size + ALIGN8(reg_data_length);
+ dest = dump->p + header_size;
+
+ err = nfp_add_tlv(be32_to_cpu(spec_csr->tl.type), total_size, dump);
+ if (err)
+ return err;
+
+ dump_header->cpp = spec_csr->cpp;
+ dump_header->register_width = spec_csr->register_width;
+
+ cpp_rd_addr = be32_to_cpu(spec_csr->cpp.offset);
+ max_rd_addr = cpp_rd_addr + be32_to_cpu(spec_csr->cpp.dump_length);
+ while (cpp_rd_addr < max_rd_addr) {
+ err = nfp_read_all_indirect_csr_ctx(pf->cpp, spec_csr,
+ cpp_rd_addr, reg_sz, dest);
+ if (err) {
+ dump_header->error = cpu_to_be32(err);
+ dump_header->error_offset = cpu_to_be32(cpp_rd_addr);
+ break;
+ }
+ cpp_rd_addr += reg_sz;
+ dest += reg_sz * NFP_IND_NUM_CONTEXTS;
+ }
+
+ return 0;
+}
+
static int
nfp_dump_single_rtsym(struct nfp_pf *pf, struct nfp_dumpspec_rtsym *spec,
struct nfp_dump_state *dump)
@@ -589,6 +696,12 @@ nfp_dump_for_tlv(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
if (err)
return err;
break;
+ case NFP_DUMPSPEC_TYPE_INDIRECT_ME_CSR:
+ spec_csr = (struct nfp_dumpspec_csr *)tl;
+ err = nfp_dump_indirect_csr_range(pf, spec_csr, dump);
+ if (err)
+ return err;
+ break;
case NFP_DUMPSPEC_TYPE_RTSYM:
spec_rtsym = (struct nfp_dumpspec_rtsym *)tl;
err = nfp_dump_single_rtsym(pf, spec_rtsym, dump);
--
2.11.0
^ permalink raw reply related
* Re: [Patch net v2] tipc: fix a null pointer deref on error path
From: Cong Wang @ 2017-12-04 22:34 UTC (permalink / raw)
To: Jon Maloy
Cc: David Miller, Linux Kernel Network Developers,
tipc-discussion@lists.sourceforge.net, Ying Xue
In-Reply-To: <AM4PR07MB17140DAC84D397BB05E84E449A3C0@AM4PR07MB1714.eurprd07.prod.outlook.com>
On Mon, Dec 4, 2017 at 12:32 PM, Jon Maloy <jon.maloy@ericsson.com> wrote:
>
>> You are right. The right solution is to just call conn_put() twice here.
>> I already have a patch ready for this, but it is part of a series that needs more
>> review.
>> I should probably post it separately...
>
> Well, calling conn_put() twice was ok in my series, but in the current upstream version it is not enough.
> I will find a different short term solution.
>
IMHO, for tipc_topsrv_kern_subscr() my v2 patch is more
correct, and for tipc_accept_from_sock(), v1 is needed too.
Of course v1 could fix both, but still v2 is better than v1 if
we only consider tipc_topsrv_kern_subscr() case. So this
depends if we want to consider these 2 paths as 2 cases
or 1 case. _I think_ it is better to consider them separately
since we already v2 is best fix for tipc_topsrv_kern_subscr(),
while still not sure what is the best for tipc_accept_from_sock().
^ permalink raw reply
* Re: [PATCH net-next 4/4] qede: Use NETIF_F_GRO_HW.
From: Michael Chan @ 2017-12-04 22:45 UTC (permalink / raw)
To: Yuval Mintz
Cc: davem@davemloft.net, netdev@vger.kernel.org, Ariel Elior,
everest-linux-l2@cavium.com
In-Reply-To: <DB6PR05MB35099AD2D93EDB3888923374BF3C0@DB6PR05MB3509.eurprd05.prod.outlook.com>
On Mon, Dec 4, 2017 at 1:48 PM, Yuval Mintz <yuvalm@mellanox.com> wrote:
>> Advertise NETIF_F_GRO_HW and turn on or off hardware GRO based on
>> NETIF_F_GRO_FW flag.
>>
>> Cc: Ariel Elior <Ariel.Elior@cavium.com>
>> Cc: everest-linux-l2@cavium.com
>> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
>> ---
>> drivers/net/ethernet/qlogic/qede/qede_filter.c | 9 ++-------
>> drivers/net/ethernet/qlogic/qede/qede_main.c | 4 ++--
>> 2 files changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c
>> b/drivers/net/ethernet/qlogic/qede/qede_filter.c
>> index c1a0708..7ee49b4 100644
>> --- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
>> +++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c
>> @@ -901,13 +901,8 @@ int qede_set_features(struct net_device *dev,
>> netdev_features_t features)
>> netdev_features_t changes = features ^ dev->features;
>> bool need_reload = false;
>>
>> - /* No action needed if hardware GRO is disabled during driver load */
>> - if (changes & NETIF_F_GRO) {
>> - if (dev->features & NETIF_F_GRO)
>> - need_reload = !edev->gro_disable;
>> - else
>> - need_reload = edev->gro_disable;
>> - }
>> + if (changes & NETIF_F_GRO_HW)
>> + need_reload = true;
>
> This doesn't look right; edev->gro_disable can change due to other
> conditions as well - otherwise, it would have been synonymous with
> (dev->features & NETIF_F_GRO).
>
Feel free to modify the patch. The idea is for the driver to
determine in ndo_fix_features()/ndo_set_features() if GRO_HW can be
enabled. For things like generic XDP, I think we can add code to
centrally disable GRO_HW and not have to worry about that in the
driver.
^ permalink raw reply
* Re: [PATCH 3/4] RFC: net: dsa: Add bindings for Realtek SMI DSAs
From: Andrew Lunn @ 2017-12-04 22:50 UTC (permalink / raw)
To: Linus Walleij
Cc: Florian Fainelli, Vivien Didelot, netdev-u79uwXL29TY76Z2rM5mHXA,
Antti Seppälä, Roman Yeryomin, Colin Leitner,
Gabor Juhos,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <CACRpkdYoMVNh8eaTnaDQ59bsh4bC88biLaYSXyhnc4W83PMWzA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
> So why not:
>
> switch@0 {
> compatible = "acme,switch";
> #address-cells = <1>;
> #size-cells = <0>;
>
> ports {
>
> port@0 {
> reg = <0>;
> phy@0 {
> reg = <0>;
> };
> };
Hi Linus
So you are suggesting put the PHY node inside the MAC node.
This is sometimes done, but does not describe the hardware. The PHYs
are on an MDIO bus, so device tree should show the MDIO bus and the
PHYs on it.
DSA does have an MDIO bus, so putting the PHYs in the MAC is just
confusing. Although that is not Florians preferred solution, he would
like the DSA driver to export its own MDIO bus and list the PHYs on
it.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next] bpf: move bpf csum flag check
From: Daniel Borkmann @ 2017-12-04 22:53 UTC (permalink / raw)
To: William Tu, netdev
In-Reply-To: <1512425909-105242-1-git-send-email-u9012063@gmail.com>
On 12/04/2017 11:18 PM, William Tu wrote:
> trivial move the BPF_F_ZERO_CSUM_TX check right below the
> 'flags & BPF_F_DONT_FRAGMENT', so common tun_flags handling
> is logically together.
>
> Signed-off-by: William Tu <u9012063@gmail.com>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Applied to bpf-next, thanks William.
^ permalink raw reply
* Re: [PATCH] rtlwifi: rtl818x: remove redundant check for cck_power > 15
From: Hin-Tak Leung @ 2017-12-04 22:57 UTC (permalink / raw)
To: Larry Finger, Kalle Valo, linux-wireless, netdev, Colin King
Cc: kernel-janitors, linux-kernel
In-Reply-To: <734417985.2789125.1512428275673.ref@mail.yahoo.com>
--------------------------------------------
On Tue, 14/11/17, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
> cck_poweri cannot be greated than 15 as
> is derived from the bottom 4 bits
> from riv->channels[channel -
> 1].hw_value & 0xf. Hence the check for it
> being greater than 15 is redundant and
> can be removed.
> Detected by CoverityScan, CID#744303
> ("Logically dead code")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Hin-Tak Leung <htl10@users.sourceforge.net>
^ permalink raw reply
* linux-next: manual merge of the net-next tree with Linus' tree
From: Stephen Rothwell @ 2017-12-04 22:59 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux-Next Mailing List, Linux Kernel Mailing List,
Marc Kleine-Budde, Pankaj Bansal
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
drivers/net/can/flexcan.c
between commit:
29c64b17a0bc ("can: flexcan: fix VF610 state transition issue")
from Linus' tree and commit:
99b7668c04b2 ("can: flexcan: adding platform specific details for LS1021A")
from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/net/can/flexcan.c
index 0626dcfd1f3d,3a370d8437b0..000000000000
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@@ -184,12 -184,13 +184,13 @@@
* Below is some version info we got:
* SOC Version IP-Version Glitch- [TR]WRN_INT IRQ Err Memory err RTR re-
* Filter? connected? Passive detection ception in MB
- * MX25 FlexCAN2 03.00.00.00 no no ? no no
+ * MX25 FlexCAN2 03.00.00.00 no no no no no
* MX28 FlexCAN2 03.00.04.00 yes yes no no no
- * MX35 FlexCAN2 03.00.00.00 no no ? no no
+ * MX35 FlexCAN2 03.00.00.00 no no no no no
* MX53 FlexCAN2 03.00.00.00 yes no no no no
* MX6s FlexCAN3 10.00.12.00 yes yes no no yes
- * VF610 FlexCAN3 ? no yes ? yes yes?
+ * VF610 FlexCAN3 ? no yes no yes yes?
+ * LS1021A FlexCAN2 03.00.04.00 no yes no no yes
*
* Some SOCs do not have the RX_WARN & TX_WARN interrupt line connected.
*/
@@@ -297,10 -302,15 +302,16 @@@ static const struct flexcan_devtype_dat
static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
- FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_OFF_TIMESTAMP,
+ FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
+ FLEXCAN_QUIRK_BROKEN_PERR_STATE,
};
+ static const struct flexcan_devtype_data fsl_ls1021a_r2_devtype_data = {
+ .quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
+ FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
+ FLEXCAN_QUIRK_USE_OFF_TIMESTAMP,
+ };
+
static const struct can_bittiming_const flexcan_bittiming_const = {
.name = DRV_NAME,
.tseg1_min = 4,
^ permalink raw reply
* Re: [PATCH net-next 1/4] net: Introduce NETIF_F_GRO_HW
From: Michael Chan @ 2017-12-04 23:05 UTC (permalink / raw)
To: Alexander Duyck; +Cc: David Miller, Netdev, Ariel Elior, everest-linux-l2
In-Reply-To: <CAKgT0UdzKdjQAbuephYa7DFShdi=0ZXZG0hZy7zzOaUKbp9q5A@mail.gmail.com>
On Mon, Dec 4, 2017 at 12:58 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Mon, Dec 4, 2017 at 11:52 AM, Michael Chan <michael.chan@broadcom.com> wrote:
>> NETIF_F_GRO_HW is a flag that depends on NETIF_F_GRO. In some ways,
>> it is similar to a private flag that depends on NETIF_F_LRO. But I
>> think a standard flag is better.
>
> Why would you make it dependent on NETIF_F_GRO? That doesn't make any
> sense to me. It gets in the way of GRO anyway as it can't assemble an
> already aggregated frame.
GRO_HW is basically hardware accelerated GRO. If the user doesn't
want GRO at all (let's say he is running generic XDP, or he wants
tcpdump to show the packets on the wire), disabling GRO should disable
everything. It's more intuitive that way.
>
> It seems more like the two features should be able to co-exist with
> either one being able to be disabled/enabled independently. It makes
> it much easier to debug things this way. Otherwise there is no way to
> tell if a given issue is software or hardware GRO since disabling
> software disables them both.
>
>>>
>>>>>
>>>>> I think I would rather have something like a netdev private flag that
>>>>> says LRO assembled frames are routable and just have this all run over
>>>>> the LRO flag with a test for the private flag to avoid triggering the
>>>>> LRO disable in the case of the flag being present. Really this is just
>>>>> a clean LRO implementation anyway so maybe we should just go that
>>>>> route where LRO is the hardware offload and GRO is the generic
>>>>> software implementation of that offload. That way when GRO gets some
>>>>> new feature that your hardware doesn't support we don't have to argue
>>>>> about the differences since LRO is meant to be a limited
>>>>> implementation anyway due to the nature of it being in hardware.
>>>>
>>>> Private flag will work. But a standard feature flag is better since
>>>> there are multiple drivers supporting this. A standard way to turn
>>>> this on/off is a better user experience. It's also consistent with
>>>> TSO/GSO on the transmit side.
>>>
>>> I agree, and that is why I would prefer to see this use the LRO flag.
>>> It is the flag that is normally used to indicating Rx coalescing in
>>> hardware. Coming up with a custom feature flag for a form of LRO that
>>> your hardware does doesn't make much sense to me. Otherwise I might as
>>> well go modify ixgbe and rename the LRO it does to GRO_HW since I can
>>> make it do most of what you are doing here.
>>
>> Again, there is enough difference between LRO and hardware GRO that it
>> makes sense to add a new flag. Functionally, a private flag will work
>> too, but a standard flag makes more intuitive sense to me and to
>> users.
>>
>> Yeah, the idea is that any vendor can use GRO_HW. Today, there are 3
>> drivers supporting it. I'm sure there will be other drivers
>> supporting this in the future. For something that is supported by
>> multiple vendors, that's another reason to use a standard flag.
>>
>>>
>>>>>
>>>>> To me it just seems like this is an attempt to use the GRO name as a
>>>>> marketing term and I really don't like the feel of it.
>>>>>
>>>>
>>>> I disagree with this. It's more than a marketing term.
>>>
>>> Not really. It is a subset of GRO offload in hardware. In my mind that
>>> is just LRO. I say subset since odds are you don't support all of the
>>> same protocols and tunnels that GRO in software does.
>>
>> Of course, hardware has some limitations, such as the number of TCP
>> connections it can aggregate, etc. But again, it is different from
>> LRO.
>
> The bit I don't like about this is that if you bond a device that is
> running LRO with one that is running GSO_HW you now have to disable
> two flags on the bond in order to disable hardware aggregation.
>
> Admittedly I haven't take a look at the entire patch set, but did you
> also take care of the flag sychronization issues this is going to
> cause with upper devices such as vlan, macvlan, bond, etc?
I will take another look. Thanks.
^ permalink raw reply
* [PATCH net-next 0/2] net: sched: sch_api: fix coding style issues for extack
From: Alexander Aring @ 2017-12-04 23:39 UTC (permalink / raw)
To: davem; +Cc: jhs, xiyou.wangcong, jiri, netdev, Alexander Aring, David Ahern
Hi,
this patch prepares to handle extack for qdiscs and fixes checkpatch
issues.
There are a bunch of warnings issued by checkpatch which bothered me.
This first patchset is to get rid of those warnings to make way for
the next patchsets.
I plan to followup with qdiscs, classifiers and actions after this.
- Alex
Cc: David Ahern <dsahern@gmail.com>
Alexander Aring (2):
net: sched: sch_api: fix code style issues
net: sched: sch_api: rearrange init handling
include/net/sch_generic.h | 10 +++--
net/sched/sch_api.c | 97 +++++++++++++++++++++++++----------------------
2 files changed, 58 insertions(+), 49 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH net-next 1/2] net: sched: sch_api: fix code style issues
From: Alexander Aring @ 2017-12-04 23:39 UTC (permalink / raw)
To: davem; +Cc: jhs, xiyou.wangcong, jiri, netdev, Alexander Aring, David Ahern
In-Reply-To: <20171204234000.20756-1-aring@mojatatu.com>
This patch fix checkpatch issues for upcomming patches according to the
sched api file. It changes checking on null pointer, remove unnecessary
brackets, add variable names for parameters and adjust 80 char width.
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
include/net/sch_generic.h | 10 ++++++----
net/sched/sch_api.c | 11 ++++++-----
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 65d0d25f2648..d842bebef771 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -161,7 +161,8 @@ struct Qdisc_class_ops {
void (*walk)(struct Qdisc *, struct qdisc_walker * arg);
/* Filter manipulation */
- struct tcf_block * (*tcf_block)(struct Qdisc *, unsigned long);
+ struct tcf_block * (*tcf_block)(struct Qdisc *sch,
+ unsigned long arg);
unsigned long (*bind_tcf)(struct Qdisc *, unsigned long,
u32 classid);
void (*unbind_tcf)(struct Qdisc *, unsigned long);
@@ -185,11 +186,12 @@ struct Qdisc_ops {
struct sk_buff * (*dequeue)(struct Qdisc *);
struct sk_buff * (*peek)(struct Qdisc *);
- int (*init)(struct Qdisc *, struct nlattr *arg);
+ int (*init)(struct Qdisc *sch, struct nlattr *arg);
void (*reset)(struct Qdisc *);
void (*destroy)(struct Qdisc *);
- int (*change)(struct Qdisc *, struct nlattr *arg);
- void (*attach)(struct Qdisc *);
+ int (*change)(struct Qdisc *sch,
+ struct nlattr *arg);
+ void (*attach)(struct Qdisc *sch);
int (*dump)(struct Qdisc *, struct sk_buff *);
int (*dump_stats)(struct Qdisc *, struct gnet_dump *);
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index b6c4f536876b..8f7f5378cc33 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1020,7 +1020,7 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
#endif
err = -ENOENT;
- if (ops == NULL)
+ if (!ops)
goto err_out;
sch = qdisc_alloc(dev_queue, ops);
@@ -1087,7 +1087,7 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
if (sch->flags & TCQ_F_MQROOT)
goto err_out4;
- if ((sch->parent != TC_H_ROOT) &&
+ if (sch->parent != TC_H_ROOT &&
!(sch->flags & TCQ_F_INGRESS) &&
(!p || !(p->flags & TCQ_F_MQROOT)))
running = qdisc_root_sleeping_running(sch);
@@ -1139,7 +1139,7 @@ static int qdisc_change(struct Qdisc *sch, struct nlattr **tca)
int err = 0;
if (tca[TCA_OPTIONS]) {
- if (sch->ops->change == NULL)
+ if (!sch->ops->change)
return -EINVAL;
err = sch->ops->change(sch, tca[TCA_OPTIONS]);
if (err)
@@ -1344,7 +1344,8 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n,
goto create_n_graft;
if (n->nlmsg_flags & NLM_F_EXCL)
return -EEXIST;
- if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], q->ops->id))
+ if (tca[TCA_KIND] &&
+ nla_strcmp(tca[TCA_KIND], q->ops->id))
return -EINVAL;
if (q == p ||
(p && check_loop(q, p, 0)))
@@ -1389,7 +1390,7 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n,
}
/* Change qdisc parameters */
- if (q == NULL)
+ if (!q)
return -ENOENT;
if (n->nlmsg_flags & NLM_F_EXCL)
return -EEXIST;
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 2/2] net: sched: sch_api: rearrange init handling
From: Alexander Aring @ 2017-12-04 23:40 UTC (permalink / raw)
To: davem; +Cc: jhs, xiyou.wangcong, jiri, netdev, Alexander Aring, David Ahern
In-Reply-To: <20171204234000.20756-1-aring@mojatatu.com>
This patch fixes the following checkpatch error:
ERROR: do not use assignment in if condition
by rearranging the if condition to execute init callback only if init
callback exists. The whole setup afterwards is called in any case,
doesn't matter if init callback is set or not. This patch has the same
behaviour as before, just without assign err variable in if condition.
It also makes the code easier to read.
Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
net/sched/sch_api.c | 88 ++++++++++++++++++++++++++++-------------------------
1 file changed, 47 insertions(+), 41 deletions(-)
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 8f7f5378cc33..a48ca41b7ecf 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1060,54 +1060,60 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
netdev_info(dev, "Caught tx_queue_len zero misconfig\n");
}
- if (!ops->init || (err = ops->init(sch, tca[TCA_OPTIONS])) == 0) {
- if (qdisc_is_percpu_stats(sch)) {
- sch->cpu_bstats =
- netdev_alloc_pcpu_stats(struct gnet_stats_basic_cpu);
- if (!sch->cpu_bstats)
- goto err_out4;
-
- sch->cpu_qstats = alloc_percpu(struct gnet_stats_queue);
- if (!sch->cpu_qstats)
- goto err_out4;
- }
+ if (ops->init) {
+ err = ops->init(sch, tca[TCA_OPTIONS]);
+ if (err != 0)
+ goto err_out5;
+ }
- if (tca[TCA_STAB]) {
- stab = qdisc_get_stab(tca[TCA_STAB]);
- if (IS_ERR(stab)) {
- err = PTR_ERR(stab);
- goto err_out4;
- }
- rcu_assign_pointer(sch->stab, stab);
- }
- if (tca[TCA_RATE]) {
- seqcount_t *running;
+ if (qdisc_is_percpu_stats(sch)) {
+ sch->cpu_bstats =
+ netdev_alloc_pcpu_stats(struct gnet_stats_basic_cpu);
+ if (!sch->cpu_bstats)
+ goto err_out4;
- err = -EOPNOTSUPP;
- if (sch->flags & TCQ_F_MQROOT)
- goto err_out4;
+ sch->cpu_qstats = alloc_percpu(struct gnet_stats_queue);
+ if (!sch->cpu_qstats)
+ goto err_out4;
+ }
- if (sch->parent != TC_H_ROOT &&
- !(sch->flags & TCQ_F_INGRESS) &&
- (!p || !(p->flags & TCQ_F_MQROOT)))
- running = qdisc_root_sleeping_running(sch);
- else
- running = &sch->running;
-
- err = gen_new_estimator(&sch->bstats,
- sch->cpu_bstats,
- &sch->rate_est,
- NULL,
- running,
- tca[TCA_RATE]);
- if (err)
- goto err_out4;
+ if (tca[TCA_STAB]) {
+ stab = qdisc_get_stab(tca[TCA_STAB]);
+ if (IS_ERR(stab)) {
+ err = PTR_ERR(stab);
+ goto err_out4;
}
+ rcu_assign_pointer(sch->stab, stab);
+ }
+ if (tca[TCA_RATE]) {
+ seqcount_t *running;
- qdisc_hash_add(sch, false);
+ err = -EOPNOTSUPP;
+ if (sch->flags & TCQ_F_MQROOT)
+ goto err_out4;
- return sch;
+ if (sch->parent != TC_H_ROOT &&
+ !(sch->flags & TCQ_F_INGRESS) &&
+ (!p || !(p->flags & TCQ_F_MQROOT)))
+ running = qdisc_root_sleeping_running(sch);
+ else
+ running = &sch->running;
+
+ err = gen_new_estimator(&sch->bstats,
+ sch->cpu_bstats,
+ &sch->rate_est,
+ NULL,
+ running,
+ tca[TCA_RATE]);
+ if (err)
+ goto err_out4;
}
+
+ qdisc_hash_add(sch, false);
+
+ return sch;
+
+err_out5:
/* ops->init() failed, we call ->destroy() like qdisc_create_dflt() */
if (ops->destroy)
ops->destroy(sch);
--
2.11.0
^ permalink raw reply related
* Re: Regression in e1000e since Kernel 4.14.3
From: Gabriel C @ 2017-12-04 23:47 UTC (permalink / raw)
To: rwarsow@gmx.de; +Cc: linux-kernel, stable, netdev@vger.kernel.org
In-Reply-To: <2d4f3853-a81d-d37d-6b1c-5b4416bcb91d@gmx.de>
On 04.12.2017 23:10, rwarsow@gmx.de wrote:
> Hallo
>
> someone and I got an regression with e1000e since kernel 4.14.3 and it seems there is 4.14.4 on the way without a fix.
>
>
> bug report is here:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=198047
( added stable and netdev to CC )
Yes I have a box with e1000e and it seems something at least breaks NM after 4.14.3.
Interesting here , when using connman the connection is stable.
Regards,
Gabriel C
^ permalink raw reply
* Re: [PATCH net-next 2/2] veth: allow configuring GSO maximums
From: Cong Wang @ 2017-12-05 0:03 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David Miller, Linux Kernel Network Developers, Stephen Hemminger
In-Reply-To: <20171201201158.25594-3-sthemmin@microsoft.com>
On Fri, Dec 1, 2017 at 12:11 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> Veth's can be used in environments (like Azure) where the underlying
> network device is impacted by large GSO packets. This patch allows
> gso maximum values to be passed in when creating the device via
> netlink.
>
> In theory, other pseudo devices could also use netlink attributes
> to set GSO maximums but for now veth is what has been observed
> to be an issue.
It looks odd that you only allow setting them but not dumping them.
^ permalink raw reply
* Re: [PATCH net-next 2/4] bnxt_en: Use NETIF_F_GRO_HW.
From: Michael Chan @ 2017-12-05 0:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Or Gerlitz, David Miller, Linux Netdev List
In-Reply-To: <1512424845.19682.64.camel@gmail.com>
On Mon, Dec 4, 2017 at 2:00 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2017-12-04 at 23:06 +0200, Or Gerlitz wrote:
>> On Mon, Dec 4, 2017 at 8:11 PM, Michael Chan <michael.chan@broadcom.c
>> om> wrote:
>>
>> > All the logic is already in these 3 drivers in the tree. You can
>> > see
>> > the additional logic in any of these drivers. It's just that these
>> > drivers have been using NETIF_F_GRO to turn on this mode in
>> > hardware/firmware.
>>
>> What happens for tcp encaped by some udp tunnel, does
>> the HW know how to GRO that?
>
> Yes it does, at least for common encaps (GRE, SIT, IPIP)
>
Correct, bnxt_en supports all common encaps. Basically, if we can do
TSO on the packet, we can do the reverse GRO_HW on the same packet
stream.
As already pointed out, GRO_HW is a subset of GRO. Packets that
cannot be aggregated in hardware (due to hardware resource limitations
or protocol types that it doesn't handle) can just be passed to the
stack for GRO aggregation.
^ permalink raw reply
* Re: [PATCH net-next V3] tun: add eBPF based queue selection method
From: Willem de Bruijn @ 2017-12-05 0:16 UTC (permalink / raw)
To: Jason Wang
Cc: Network Development, LKML, Michael S. Tsirkin, Tom Herbert,
aconole, wexu
In-Reply-To: <1512379883-11887-1-git-send-email-jasowang@redhat.com>
On Mon, Dec 4, 2017 at 4:31 AM, Jason Wang <jasowang@redhat.com> wrote:
> This patch introduces an eBPF based queue selection method. With this,
> the policy could be offloaded to userspace completely through a new
> ioctl TUNSETSTEERINGEBPF.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> +static u16 tun_ebpf_select_queue(struct tun_struct *tun, struct sk_buff *skb)
> +{
> + struct tun_steering_prog *prog;
> + u16 ret = 0;
> +
> + prog = rcu_dereference(tun->steering_prog);
> + if (prog)
> + ret = bpf_prog_run_clear_cb(prog->prog, skb);
This dereferences tun->steering_prog for a second time. It is safe
in this load balancing case to assign a few extra packets to queue 0.
But the issue can also be avoided by replacing the function with a
direct call in tun_net_xmit:
struct tun_steering_prog *s = rcu_dereference(tun->steering_prog);
if (s)
ret = bpf_prog_run_clear_cb(s->prog, skb) % tun->numqueues;
> /* Net device start xmit */
> -static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> +static void tun_automq_xmit(struct tun_struct *tun, struct sk_buff *skb)
> {
> - struct tun_struct *tun = netdev_priv(dev);
> - int txq = skb->queue_mapping;
> - struct tun_file *tfile;
> - u32 numqueues = 0;
> -
> - rcu_read_lock();
> - tfile = rcu_dereference(tun->tfiles[txq]);
> - numqueues = READ_ONCE(tun->numqueues);
> -
> - /* Drop packet if interface is not attached */
> - if (txq >= numqueues)
> - goto drop;
> -
> #ifdef CONFIG_RPS
> - if (numqueues == 1 && static_key_false(&rps_needed)) {
> + if (tun->numqueues == 1 && static_key_false(&rps_needed)) {
> /* Select queue was not called for the skbuff, so we extract the
> * RPS hash and save it into the flow_table here.
> */
> @@ -969,6 +986,26 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> }
> }
> #endif
> +}
> +
> +/* Net device start xmit */
> +static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct tun_struct *tun = netdev_priv(dev);
> + int txq = skb->queue_mapping;
> + struct tun_file *tfile;
> + u32 numqueues = 0;
> +
> + rcu_read_lock();
> + tfile = rcu_dereference(tun->tfiles[txq]);
> + numqueues = READ_ONCE(tun->numqueues);
Now tun->numqueues is read twice, reversing commit fa35864e0bb7
("tuntap: Fix for a race in accessing numqueues"). I don't see anything
left that would cause a divide by zero after the relevant code was
converted from divide to multiple and subsequently even removed.
But if it's safe to read multiple times, might as well remove the READ_ONCE.
> @@ -1551,7 +1588,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> int copylen;
> bool zerocopy = false;
> int err;
> - u32 rxhash;
> + u32 rxhash = 0;
> int skb_xdp = 1;
> bool frags = tun_napi_frags_enabled(tun);
>
> @@ -1739,7 +1776,10 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> rcu_read_unlock();
> }
>
> - rxhash = __skb_get_hash_symmetric(skb);
> + rcu_read_lock();
> + if (!rcu_dereference(tun->steering_prog))
> + rxhash = __skb_get_hash_symmetric(skb);
> + rcu_read_unlock();
>
> if (frags) {
> /* Exercise flow dissector code path. */
> @@ -1783,7 +1823,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> u64_stats_update_end(&stats->syncp);
> put_cpu_ptr(stats);
>
> - tun_flow_update(tun, rxhash, tfile);
> + if (rxhash)
> + tun_flow_update(tun, rxhash, tfile);
> +
Nit: zero is a valid hash? In which case, an int64_t initialized to -1 is the
safer check.
^ permalink raw reply
* Re: [PATCH net-next] bpf: Add access to snd_cwnd and others in sock_ops
From: Lawrence Brakmo @ 2017-12-05 0:21 UTC (permalink / raw)
To: Daniel Borkmann, netdev
Cc: Alexei Starovoitov, Kernel Team, Blake Matheny, Vlad Dumitrescu
In-Reply-To: <d83bcfb5-2a6b-5d9c-91d0-dd11d875a9cb@iogearbox.net>
On 12/4/17, 2:32 PM, "netdev-owner@vger.kernel.org on behalf of Daniel Borkmann" <netdev-owner@vger.kernel.org on behalf of daniel@iogearbox.net> wrote:
Hi Lawrence,
On 12/01/2017 07:15 PM, Lawrence Brakmo wrote:
> Adds read access to snd_cwnd and srtt_us fields of tcp_sock. Since these
> fields are only valid if the socket associated with the sock_ops program
> call is a full socket, the field is_fullsock is also added to the
> bpf_sock_ops struct. If the socket is not a full socket, reading these
> fields returns 0.
>
> Note that in most cases it will not be necessary to check is_fullsock to
> know if there is a full socket. The context of the call, as specified by
> the 'op' field, can sometimes determine whether there is a full socket.
>
> The struct bpf_sock_ops has the following fields added:
>
> __u32 is_fullsock; /* Some TCP fields are only valid if
> * there is a full socket. If not, the
> * fields read as zero.
> */
> __u32 snd_cwnd;
> __u32 srtt_us; /* Averaged RTT << 3 in usecs */
>
> There is a new macro, SOCK_OPS_GET_TCP32(NAME), to make it easier to add
> read access to more 32 bit tcp_sock fields.
>
> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
> ---
> include/linux/filter.h | 1 +
> include/net/tcp.h | 6 ++++--
> include/uapi/linux/bpf.h | 6 ++++++
> net/core/filter.c | 36 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 80b5b48..0062302 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -985,6 +985,7 @@ struct bpf_sock_ops_kern {
> u32 reply;
> u32 replylong[4];
> };
> + u32 is_fullsock;
> };
>
> #endif /* __LINUX_FILTER_H__ */
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 4e09398..89a6560 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2012,10 +2012,12 @@ static inline int tcp_call_bpf(struct sock *sk, int op)
> struct bpf_sock_ops_kern sock_ops;
> int ret;
>
> - if (sk_fullsock(sk))
> + memset(&sock_ops, 0, sizeof(sock_ops));
> + if (sk_fullsock(sk)) {
> + sock_ops.is_fullsock = 1;
> sock_owned_by_me(sk);
> + }
>
> - memset(&sock_ops, 0, sizeof(sock_ops));
> sock_ops.sk = sk;
> sock_ops.op = op;
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 4c223ab..80d62e8 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -941,6 +941,12 @@ struct bpf_sock_ops {
> __u32 local_ip6[4]; /* Stored in network byte order */
> __u32 remote_port; /* Stored in network byte order */
> __u32 local_port; /* stored in host byte order */
> + __u32 is_fullsock; /* Some TCP fields are only valid if
> + * there is a full socket. If not, the
> + * fields read as zero.
> + */
Just a question for clarification: do you need to have this in
the uapi struct as well? Meaning, do you have a specific use case
where you do this check out of the BPF program given the below
two snd_cwnd/srtt_us check this internally in the ctx rewrite?
Although is_fullsock is checked internally, the bpf program may want to
verify if a return value of zero is due to a non fullsock state. The issue is
that there are some ops that are called from both active and passive paths
so it is not possible to know just by the op type.
Do you plan to reuse the ctx assignment also for bpf_setsockopt()
and bpf_getsockopt() internally?
If you mean, check is_fullsock instead of the call to sk_fullsock, then yes, it is
a great idea. However, I would rather do it in another patch so we don’t delay
this one (I have other things almost ready that depend on this patch).
> + __u32 snd_cwnd;
> + __u32 srtt_us; /* Averaged RTT << 3 in usecs */
> };
>
> /* List of known BPF sock_ops operators.
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 6a85e67..bf31236 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4437,6 +4437,42 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
> *insn++ = BPF_LDX_MEM(BPF_H, si->dst_reg, si->dst_reg,
> offsetof(struct sock_common, skc_num));
> break;
> +
> + case offsetof(struct bpf_sock_ops, is_fullsock):
> + *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(
> + struct bpf_sock_ops_kern,
> + is_fullsock),
> + si->dst_reg, si->src_reg,
> + offsetof(struct bpf_sock_ops_kern,
> + is_fullsock));
> + break;
> +
> +/* Helper macro for adding read access to tcp_sock fields. */
> +#define SOCK_OPS_GET_TCP32(FIELD_NAME) \
> + do { \
> + BUILD_BUG_ON(FIELD_SIZEOF(struct tcp_sock, FIELD_NAME) != 4); \
> + *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF( \
> + struct bpf_sock_ops_kern, \
> + is_fullsock), \
> + si->dst_reg, si->src_reg, \
> + offsetof(struct bpf_sock_ops_kern, \
> + is_fullsock)); \
> + *insn++ = BPF_JMP_IMM(BPF_JEQ, si->dst_reg, 0, 2); \
> + *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF( \
> + struct bpf_sock_ops_kern, sk),\
> + si->dst_reg, si->src_reg, \
> + offsetof(struct bpf_sock_ops_kern, sk));\
> + *insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg, \
> + offsetof(struct tcp_sock, FIELD_NAME)); \
> + } while (0)
> +
> + case offsetof(struct bpf_sock_ops, snd_cwnd):
> + SOCK_OPS_GET_TCP32(snd_cwnd);
> + break;
> +
> + case offsetof(struct bpf_sock_ops, srtt_us):
> + SOCK_OPS_GET_TCP32(srtt_us);
> + break;
> }
> return insn - insn_buf;
> }
>
^ permalink raw reply
* Re: [PATCH 1/1] gianfar: fix a flooded alignment reports because of padding issue.
From: Zumeng Chen @ 2017-12-05 0:24 UTC (permalink / raw)
To: Claudiu Manoil, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: davem@davemloft.net
In-Reply-To: <AM5PR0401MB256141EAA45FA0BBB301D8B7963C0@AM5PR0401MB2561.eurprd04.prod.outlook.com>
On 12/05/2017 12:06 AM, Claudiu Manoil wrote:
>> -----Original Message-----
>> From: Zumeng Chen [mailto:zumeng.chen@gmail.com]
>> Sent: Monday, December 04, 2017 5:22 AM
>> To: netdev@vger.kernel.org; linux-kernel@vger.kernel.org
>> Cc: Claudiu Manoil <claudiu.manoil@nxp.com>; davem@davemloft.net
>> Subject: [PATCH 1/1] gianfar: fix a flooded alignment reports because of padding
>> issue.
>>
>> According to LS1021A RM, the value of PAL can be set so that the start of the
>> IP header in the receive data buffer is aligned to a 32-bit boundary. Normally,
>> setting PAL = 2 provides minimal padding to ensure such alignment of the IP
>> header.
>>
>> However every incoming packet's 8-byte time stamp will be inserted into the
>> packet data buffer as padding alignment bytes when hardware time stamping is
>> enabled.
>>
>> So we set the padding 8+2 here to avoid the flooded alignment faults:
>>
>> root@128:~# cat /proc/cpu/alignment
>> User: 0
>> System: 17539 (inet_gro_receive+0x114/0x2c0)
>> Skipped: 0
>> Half: 0
>> Word: 0
>> DWord: 0
>> Multi: 17539
>> User faults: 2 (fixup)
>>
> [...]
>> drivers/net/ethernet/freescale/gianfar.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/gianfar.c
>> b/drivers/net/ethernet/freescale/gianfar.c
>> index e616b71..e47945f 100644
>> --- a/drivers/net/ethernet/freescale/gianfar.c
>> +++ b/drivers/net/ethernet/freescale/gianfar.c
>> @@ -1413,9 +1413,11 @@ static int gfar_probe(struct platform_device *ofdev)
>>
>> gfar_init_addr_hash_table(priv);
>>
>> - /* Insert receive time stamps into padding alignment bytes */
>> + /* Insert receive time stamps into padding alignment bytes, and
>> + * plus 2 bytes padding to ensure the cpu alignment.
>> + */
>> if (priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)
>> - priv->padding = 8;
>> + priv->padding = 8 + DEFAULT_PADDING;
>>
>> if (dev->features & NETIF_F_IP_CSUM ||
>> priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)
>> --
>> 2.5.0
> Why handle only the rx timestamp path (HAS_TIMER) when the issue, as presented
> by you, should be applicable to the default path as well?
>
> The code change according to the patch description should be more likely:
>
> + priv->padding = DEFAULT_PADDING;
> /* Insert receive time stamps into padding alignment bytes */
> if (priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)
> - priv->padding = 8;
> + priv->padding += 8;
En, seems more reasonable and clear, feel free to merge it if
convenience, thanks :-)
>
> Do you have any performance numbers for this change?
No performance testing since just padding issue but with
the clear sky of /proc/cpu/alignment in arm side.
Cheers,
Zumeng
>
> Thanks,
> Claudiu
^ permalink raw reply
* Re: [PATCH 1/7 v2] net: bcmgenet: Fix platform_get_irq's error checking
From: Doug Berger @ 2017-12-05 1:01 UTC (permalink / raw)
To: Arvind Yadav, wg, mkl, michal.simek, f.fainelli, davem
Cc: linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1512409703-20881-2-git-send-email-arvind.yadav.cs@gmail.com>
On 12/04/2017 09:48 AM, Arvind Yadav wrote:
> The platform_get_irq() function returns negative number if an error occurs,
> Zero if No irq is found and positive number if irq gets successful.
> platform_get_irq() error checking only for zero is not correct.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
> changes in v2:
> commit message was not correct.
>
> 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 24b4f4c..e2f1268 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -3371,7 +3371,7 @@ static int bcmgenet_probe(struct platform_device *pdev)
> priv->irq0 = platform_get_irq(pdev, 0);
> priv->irq1 = platform_get_irq(pdev, 1);
> priv->wol_irq = platform_get_irq(pdev, 2);
> - if (!priv->irq0 || !priv->irq1) {
> + if (priv->irq0 <= 0 || priv->irq1 <= 0 || priv->wol_irq <= 0) {
> dev_err(&pdev->dev, "can't find IRQs\n");
> err = -EINVAL;
> goto err;
>
The absence of a Wake-on-LAN interrupt (wol_irq <= 0) is not a terminal
error for the driver so it should not be included in this check.
The error checking for irq0 and irq1 is appropriate to add, but it
sounds like David Miller is proposing changing platform_get_irq() so
I'll let that dust settle before saying whether <= or < is appropriate.
Thanks,
Doug
^ permalink raw reply
* Re: [PATCH net-next 2/2] veth: allow configuring GSO maximums
From: Eric Dumazet @ 2017-12-05 1:04 UTC (permalink / raw)
To: Cong Wang, Stephen Hemminger
Cc: David Miller, Linux Kernel Network Developers, Stephen Hemminger
In-Reply-To: <CAM_iQpWWmLk0SZk7TcD_mF7S5e4FwPyR-SfmCYo6gHOHFAsnRQ@mail.gmail.com>
On Mon, 2017-12-04 at 16:03 -0800, Cong Wang wrote:
> On Fri, Dec 1, 2017 at 12:11 PM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > Veth's can be used in environments (like Azure) where the
> > underlying
> > network device is impacted by large GSO packets. This patch allows
> > gso maximum values to be passed in when creating the device via
> > netlink.
> >
> > In theory, other pseudo devices could also use netlink attributes
> > to set GSO maximums but for now veth is what has been observed
> > to be an issue.
>
> It looks odd that you only allow setting them but not dumping them.
Dump is already supported/provided by core stack
# ip -d link sh dev wlan0
2: wlan0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq state UP
mode DORMANT group default qlen 1000
link/ether xx:xx:xx:xx:xx:xx brd ff:ff:ff:ff:ff:ff promiscuity 0
addrgenmode none numtxqueues 4 numrxqueues 1 gso_max_size 65536
gso_max_segs 65535
^ permalink raw reply
* Re: [PATCH 02/10] net: bcmgenet: free netdev on of_match_node() error
From: Doug Berger @ 2017-12-05 1:05 UTC (permalink / raw)
To: Arvind Yadav, wg, mkl, michal.simek, f.fainelli, davem,
Vladislav.Zakharov
Cc: netdev, linux-kernel, linux-arm-kernel
In-Reply-To: <1512242782-7134-3-git-send-email-arvind.yadav.cs@gmail.com>
On 12/02/2017 11:26 AM, Arvind Yadav wrote:
> The change is to call free_netdev(), If of_match_node() will fail.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index e2f1268..e0a8f79 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -3363,8 +3363,10 @@ static int bcmgenet_probe(struct platform_device *pdev)
>
> if (dn) {
> of_id = of_match_node(bcmgenet_match, dn);
> - if (!of_id)
> - return -EINVAL;
> + if (!of_id) {
> + err = -EINVAL;
> + goto err;
> + }
> }
>
> priv = netdev_priv(dev);
>
I agree with the fix if you want to resubmit separate from this series
and please include a "fixes" tag.
Thanks,
Doug
^ permalink raw reply
* [PATCH v4] perf_event_open.2: add type kprobe and uprobe
From: Song Liu @ 2017-12-05 1:27 UTC (permalink / raw)
To: peterz, rostedt, mingo, davem, netdev, linux-kernel, daniel
Cc: kernel-team, Song Liu
In-Reply-To: <20171205012729.358860-1-songliubraving@fb.com>
Two new types kprobe and uprobe are being added to perf_event_open,
which allow creating kprobe or uprobe with perf_event_open. This
patch adds information about these types.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
man2/perf_event_open.2 | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/man2/perf_event_open.2 b/man2/perf_event_open.2
index c91da3f..7a97ed3 100644
--- a/man2/perf_event_open.2
+++ b/man2/perf_event_open.2
@@ -256,11 +256,15 @@ struct perf_event_attr {
union {
__u64 bp_addr; /* breakpoint address */
+ __u64 kprobe_func; /* for perf_kprobe */
+ __u64 uprobe_path; /* for perf_uprobe */
__u64 config1; /* extension of config */
};
union {
__u64 bp_len; /* breakpoint length */
+ __u64 kprobe_addr; /* with kprobe_func == NULL */
+ __u64 probe_offset; /* for perf_[k,u]probe */
__u64 config2; /* extension of config1 */
};
__u64 branch_sample_type; /* enum perf_branch_sample_type */
@@ -336,6 +340,13 @@ field.
For instance,
.I /sys/bus/event_source/devices/cpu/type
contains the value for the core CPU PMU, which is usually 4.
+.TP
+.BR kprobe " and " uprobe " (since Linux 4.TBD)"
+These two dynamic PMU creates kprobe or uprobe with perf_event_open and
+attaches it to the file descriptor.
+See fields
+.IR kprobe_func ", " uprobe_path ", " kprobe_addr ", and " probe_offset
+for more details.
.RE
.TP
.I "size"
@@ -627,6 +638,44 @@ then leave
.I config
set to zero.
Its parameters are set in other places.
+.PP
+If
+.I type
+is
+.BR kprobe
+or
+.BR uprobe ,
+set
+.BR PERF_PROBE_CONFIG_IS_RETPROBE
+in
+.I config
+for kretprobe/uretprobe. See fields
+.IR kprobe_func ", " uprobe_path ", " kprobe_addr ", and " probe_offset
+for more details.
+.RE
+.TP
+.IR kprobe_func ", " uprobe_path ", " kprobe_addr ", and " probe_offset
+.EE
+These fields describes the kprobe/uprobe for dynamic PMU
+.BR kprobe
+and
+.BR uprobe .
+For
+.BR kprobe ": "
+use
+.I kprobe_func
+and
+.IR probe_offset ,
+or use
+.I kprobe_addr
+and leave
+.I kprobe_func
+as NULL. For
+.BR uprobe ": "
+use
+.I uprobe_path
+and
+.IR probe_offset .
.RE
.TP
.IR sample_period ", " sample_freq
--
2.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox