Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 5/5] net: stmmac: Limit max speeds of XGMAC if asked to
From: Jose Abreu @ 2019-09-06  7:41 UTC (permalink / raw)
  To: netdev
  Cc: Joao Pinto, Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
	David S. Miller, Maxime Coquelin, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1567755423.git.joabreu@synopsys.com>

We may have some SoCs that can't achieve XGMAC max speed. Limit it if
asked to.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>

---
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 25 +++++++++++++++--------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index c3baca9f587b..686b82068142 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -831,15 +831,22 @@ static void stmmac_validate(struct phylink_config *config,
 		phylink_set(mask, 1000baseT_Full);
 		phylink_set(mask, 1000baseX_Full);
 	} else if (priv->plat->has_xgmac) {
-		phylink_set(mac_supported, 2500baseT_Full);
-		phylink_set(mac_supported, 5000baseT_Full);
-		phylink_set(mac_supported, 10000baseSR_Full);
-		phylink_set(mac_supported, 10000baseLR_Full);
-		phylink_set(mac_supported, 10000baseER_Full);
-		phylink_set(mac_supported, 10000baseLRM_Full);
-		phylink_set(mac_supported, 10000baseT_Full);
-		phylink_set(mac_supported, 10000baseKX4_Full);
-		phylink_set(mac_supported, 10000baseKR_Full);
+		if (!max_speed || (max_speed >= 2500)) {
+			phylink_set(mac_supported, 2500baseT_Full);
+			phylink_set(mac_supported, 2500baseX_Full);
+		}
+		if (!max_speed || (max_speed >= 5000)) {
+			phylink_set(mac_supported, 5000baseT_Full);
+		}
+		if (!max_speed || (max_speed >= 10000)) {
+			phylink_set(mac_supported, 10000baseSR_Full);
+			phylink_set(mac_supported, 10000baseLR_Full);
+			phylink_set(mac_supported, 10000baseER_Full);
+			phylink_set(mac_supported, 10000baseLRM_Full);
+			phylink_set(mac_supported, 10000baseT_Full);
+			phylink_set(mac_supported, 10000baseKX4_Full);
+			phylink_set(mac_supported, 10000baseKR_Full);
+		}
 	}
 
 	/* Half-Duplex can only work with single queue */
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next 1/5] net: stmmac: selftests: Add missing checks for support of SA
From: Jose Abreu @ 2019-09-06  7:41 UTC (permalink / raw)
  To: netdev
  Cc: Joao Pinto, Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
	David S. Miller, Maxime Coquelin, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1567755423.git.joabreu@synopsys.com>

Add checks for support of Source Address Insertion/Replacement before
running the test.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>

---
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
index 305d24935cf4..dce34c081a1e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
@@ -1057,6 +1057,9 @@ static int stmmac_test_desc_sai(struct stmmac_priv *priv)
 	struct stmmac_packet_attrs attr = { };
 	int ret;
 
+	if (!priv->dma_cap.vlins)
+		return -EOPNOTSUPP;
+
 	attr.remove_sa = true;
 	attr.sarc = true;
 	attr.src = src;
@@ -1076,6 +1079,9 @@ static int stmmac_test_desc_sar(struct stmmac_priv *priv)
 	struct stmmac_packet_attrs attr = { };
 	int ret;
 
+	if (!priv->dma_cap.vlins)
+		return -EOPNOTSUPP;
+
 	attr.sarc = true;
 	attr.src = src;
 	attr.dst = priv->dev->dev_addr;
@@ -1094,6 +1100,9 @@ static int stmmac_test_reg_sai(struct stmmac_priv *priv)
 	struct stmmac_packet_attrs attr = { };
 	int ret;
 
+	if (!priv->dma_cap.vlins)
+		return -EOPNOTSUPP;
+
 	attr.remove_sa = true;
 	attr.sarc = true;
 	attr.src = src;
@@ -1114,6 +1123,9 @@ static int stmmac_test_reg_sar(struct stmmac_priv *priv)
 	struct stmmac_packet_attrs attr = { };
 	int ret;
 
+	if (!priv->dma_cap.vlins)
+		return -EOPNOTSUPP;
+
 	attr.sarc = true;
 	attr.src = src;
 	attr.dst = priv->dev->dev_addr;
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next 4/5] net: stmmac: selftests: Add Split Header test
From: Jose Abreu @ 2019-09-06  7:41 UTC (permalink / raw)
  To: netdev
  Cc: Joao Pinto, Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
	David S. Miller, Maxime Coquelin, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1567755423.git.joabreu@synopsys.com>

Add a test to validate that Split Header feature is working correctly.
It works by using the rececently introduced counter that increments each
time a packet with split header is received.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>

---
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 .../net/ethernet/stmicro/stmmac/stmmac_selftests.c | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
index 2943943bec43..c56e89e1ae56 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
@@ -1603,6 +1603,44 @@ static int stmmac_test_mjumbo(struct stmmac_priv *priv)
 	return 0;
 }
 
+static int stmmac_test_sph(struct stmmac_priv *priv)
+{
+	unsigned long cnt_end, cnt_start = priv->xstats.rx_split_hdr_pkt_n;
+	struct stmmac_packet_attrs attr = { };
+	int ret;
+
+	if (!priv->sph)
+		return -EOPNOTSUPP;
+
+	/* Check for UDP first */
+	attr.dst = priv->dev->dev_addr;
+	attr.tcp = false;
+
+	ret = __stmmac_test_loopback(priv, &attr);
+	if (ret)
+		return ret;
+
+	cnt_end = priv->xstats.rx_split_hdr_pkt_n;
+	if (cnt_end <= cnt_start)
+		return -EINVAL;
+
+	/* Check for TCP now */
+	cnt_start = cnt_end;
+
+	attr.dst = priv->dev->dev_addr;
+	attr.tcp = true;
+
+	ret = __stmmac_test_loopback(priv, &attr);
+	if (ret)
+		return ret;
+
+	cnt_end = priv->xstats.rx_split_hdr_pkt_n;
+	if (cnt_end <= cnt_start)
+		return -EINVAL;
+
+	return 0;
+}
+
 #define STMMAC_LOOPBACK_NONE	0
 #define STMMAC_LOOPBACK_MAC	1
 #define STMMAC_LOOPBACK_PHY	2
@@ -1724,6 +1762,10 @@ static const struct stmmac_test {
 		.name = "Multichannel Jumbo  ",
 		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_mjumbo,
+	}, {
+		.name = "Split Header        ",
+		.lb = STMMAC_LOOPBACK_PHY,
+		.fn = stmmac_test_sph,
 	},
 };
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next 2/5] net: stmmac: selftests: Set RX tail pointer in Flow Control test
From: Jose Abreu @ 2019-09-06  7:41 UTC (permalink / raw)
  To: netdev
  Cc: Joao Pinto, Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
	David S. Miller, Maxime Coquelin, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1567755423.git.joabreu@synopsys.com>

We need to set the RX tail pointer so that RX engine starts working
again after finishing the Flow Control test.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>

---
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
index dce34c081a1e..2943943bec43 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
@@ -722,8 +722,14 @@ static int stmmac_test_flowctrl(struct stmmac_priv *priv)
 
 	for (i = 0; i < rx_cnt; i++) {
 		struct stmmac_channel *ch = &priv->channel[i];
+		u32 tail;
 
+		tail = priv->rx_queue[i].dma_rx_phy +
+			(DMA_RX_SIZE * sizeof(struct dma_desc));
+
+		stmmac_set_rx_tail_ptr(priv, priv->ioaddr, tail, i);
 		stmmac_start_rx(priv, priv->ioaddr, i);
+
 		local_bh_disable();
 		napi_reschedule(&ch->rx_napi);
 		local_bh_enable();
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next 0/5] net: stmmac: Improvements and fixes for -next
From: Jose Abreu @ 2019-09-06  7:41 UTC (permalink / raw)
  To: netdev
  Cc: Joao Pinto, Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
	David S. Miller, Maxime Coquelin, linux-arm-kernel, linux-kernel

Improvements and fixes for recently introduced features. All for -next tree.
More info in commit logs.

---
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---

Jose Abreu (5):
  net: stmmac: selftests: Add missing checks for support of SA
  net: stmmac: selftests: Set RX tail pointer in Flow Control test
  net: stmmac: dwmac4: Enable RX Jumbo frame support
  net: stmmac: selftests: Add Split Header test
  net: stmmac: Limit max speeds of XGMAC if asked to

 drivers/net/ethernet/stmicro/stmmac/dwmac4.h       |  3 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c  |  6 ---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 25 +++++----
 .../net/ethernet/stmicro/stmmac/stmmac_selftests.c | 60 ++++++++++++++++++++++
 4 files changed, 78 insertions(+), 16 deletions(-)

-- 
2.7.4


^ permalink raw reply

* [PATCHv3 net-next] ipmr: remove hard code cache_resolve_queue_len limit
From: Hangbin Liu @ 2019-09-06  7:36 UTC (permalink / raw)
  To: netdev
  Cc: Phil Karn, Sukumar Gopalakrishnan, David S . Miller,
	Alexey Kuznetsov, Hideaki YOSHIFUJI, Eric Dumazet, Hangbin Liu
In-Reply-To: <20190903084359.13310-1-liuhangbin@gmail.com>

This is a re-post of previous patch wrote by David Miller[1].

Phil Karn reported[2] that on busy networks with lots of unresolved
multicast routing entries, the creation of new multicast group routes
can be extremely slow and unreliable.

The reason is we hard-coded multicast route entries with unresolved source
addresses(cache_resolve_queue_len) to 10. If some multicast route never
resolves and the unresolved source addresses increased, there will
be no ability to create new multicast route cache.

To resolve this issue, we need either add a sysctl entry to make the
cache_resolve_queue_len configurable, or just remove cache_resolve_queue_len
limit directly, as we already have the socket receive queue limits of mrouted
socket, pointed by David.

From my side, I'd perfer to remove the cache_resolve_queue_len limit instead
of creating two more(IPv4 and IPv6 version) sysctl entry.

[1] https://lkml.org/lkml/2018/7/22/11
[2] https://lkml.org/lkml/2018/7/21/343

v3: instead of remove cache_resolve_queue_len totally, let's only remove
the hard code limit when allocate the unresolved cache, as Eric Dumazet
suggested, so we don't need to re-count it in other places.

v2: hold the mfc_unres_lock while walking the unresolved list in
queue_count(), as Nikolay Aleksandrov remind.

Reported-by: Phil Karn <karn@ka9q.net>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/ipv4/ipmr.c  | 4 ++--
 net/ipv6/ip6mr.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index c07bc82cbbe9..313470f6bb14 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1134,8 +1134,8 @@ static int ipmr_cache_unresolved(struct mr_table *mrt, vifi_t vifi,
 
 	if (!found) {
 		/* Create a new entry if allowable */
-		if (atomic_read(&mrt->cache_resolve_queue_len) >= 10 ||
-		    (c = ipmr_cache_alloc_unres()) == NULL) {
+		c = ipmr_cache_alloc_unres();
+		if (!c) {
 			spin_unlock_bh(&mfc_unres_lock);
 
 			kfree_skb(skb);
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index e80d36c5073d..857a89ad4d6c 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -1148,8 +1148,8 @@ static int ip6mr_cache_unresolved(struct mr_table *mrt, mifi_t mifi,
 		 *	Create a new entry if allowable
 		 */
 
-		if (atomic_read(&mrt->cache_resolve_queue_len) >= 10 ||
-		    (c = ip6mr_cache_alloc_unres()) == NULL) {
+		c = ip6mr_cache_alloc_unres();
+		if (!c) {
 			spin_unlock_bh(&mfc_unres_lock);
 
 			kfree_skb(skb);
-- 
2.19.2


^ permalink raw reply related

* [PATCH 7/7] libbpf: Return const struct btf_var_secinfo from btf_var_secinfos inline function
From: Jiri Olsa @ 2019-09-06  7:31 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Andrii Nakryiko, Yonghong Song, Martin KaFai Lau
In-Reply-To: <20190906073144.31068-1-jolsa@kernel.org>

I'm getting following errors when compiling with -Wcast-qual:

  bpf/btf.h: In function ‘btf_var_secinfo* btf_var_secinfos(const btf_type*)’:
  bpf/btf.h:302:41: warning: cast from type ‘const btf_type*’ to type
  ‘btf_var_secinfo*’ casts away qualifiers [-Wcast-qual]
    302 |  return (struct btf_var_secinfo *)(t + 1);
        |                                         ^

The argument is const so the cast to following struct btf_var_secinfo
pointer should be const as well. Casting the const away in btf.c call
where it's correctly used without const in deduplication code.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/btf.c | 5 +++--
 tools/lib/bpf/btf.h | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index b5121c79fd9f..32527622792d 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -526,7 +526,8 @@ static int btf_fixup_datasec(struct bpf_object *obj, struct btf *btf,
 
 	t->size = size;
 
-	for (i = 0, vsi = btf_var_secinfos(t); i < vars; i++, vsi++) {
+	for (i = 0, vsi = (struct btf_var_secinfo *) btf_var_secinfos(t);
+	     i < vars; i++, vsi++) {
 		t_var = btf__type_by_id(btf, vsi->type);
 		var = btf_var(t_var);
 
@@ -2830,7 +2831,7 @@ static int btf_dedup_remap_type(struct btf_dedup *d, __u32 type_id)
 	}
 
 	case BTF_KIND_DATASEC: {
-		struct btf_var_secinfo *var = btf_var_secinfos(t);
+		struct btf_var_secinfo *var = (struct btf_var_secinfo *) btf_var_secinfos(t);
 		__u16 vlen = btf_vlen(t);
 
 		for (i = 0; i < vlen; i++) {
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 480dbe780fa7..ecccde0988b1 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -296,10 +296,10 @@ static inline const struct btf_var *btf_var(const struct btf_type *t)
 	return (const struct btf_var *)(t + 1);
 }
 
-static inline struct btf_var_secinfo *
+static inline const struct btf_var_secinfo *
 btf_var_secinfos(const struct btf_type *t)
 {
-	return (struct btf_var_secinfo *)(t + 1);
+	return (const struct btf_var_secinfo *)(t + 1);
 }
 
 #ifdef __cplusplus
-- 
2.21.0


^ permalink raw reply related

* [PATCH 6/7] libbpf: Return const btf_var from btf_var inline function
From: Jiri Olsa @ 2019-09-06  7:31 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Andrii Nakryiko, Yonghong Song, Martin KaFai Lau
In-Reply-To: <20190906073144.31068-1-jolsa@kernel.org>

I'm getting following errors when compiling with -Wcast-qual:

  bpf/btf.h: In function ‘btf_var* btf_var(const btf_type*)’:
  bpf/btf.h:296:33: warning: cast from type ‘const btf_type*’ to type
  ‘btf_var*’ casts away qualifiers [-Wcast-qual]
    296 |  return (struct btf_var *)(t + 1);
        |                                 ^

The argument is const so the cast to following struct btf_var
pointer should be const as well.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/btf.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 2817cf7ce2ee..480dbe780fa7 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -291,9 +291,9 @@ static inline const struct btf_param *btf_params(const struct btf_type *t)
 	return (const struct btf_param *)(t + 1);
 }
 
-static inline struct btf_var *btf_var(const struct btf_type *t)
+static inline const struct btf_var *btf_var(const struct btf_type *t)
 {
-	return (struct btf_var *)(t + 1);
+	return (const struct btf_var *)(t + 1);
 }
 
 static inline struct btf_var_secinfo *
-- 
2.21.0


^ permalink raw reply related

* [PATCH 5/7] libbpf: Return const btf_param from btf_params inline function
From: Jiri Olsa @ 2019-09-06  7:31 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Andrii Nakryiko, Yonghong Song, Martin KaFai Lau
In-Reply-To: <20190906073144.31068-1-jolsa@kernel.org>

I'm getting following errors when compiling with -Wcast-qual:

  bpf/btf.h: In function ‘btf_param* btf_params(const btf_type*)’:
  bpf/btf.h:291:35: warning: cast from type ‘const btf_type*’ to type
  ‘btf_param*’ casts away qualifiers [-Wcast-qual]
    291 |  return (struct btf_param *)(t + 1);
        |                                   ^

The argument is const so the cast to following struct btf_param
pointer should be const as well. Casting the const away in btf.c
call where it's correctly used without const in deduplication
code.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/btf.c | 6 +++---
 tools/lib/bpf/btf.h | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 560d1ae33675..b5121c79fd9f 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -1487,7 +1487,7 @@ static int btf_for_each_str_off(struct btf_dedup *d, str_off_fn_t fn, void *ctx)
 			break;
 		}
 		case BTF_KIND_FUNC_PROTO: {
-			struct btf_param *m = btf_params(t);
+			struct btf_param *m = (struct btf_param *) btf_params(t);
 			__u16 vlen = btf_vlen(t);
 
 			for (j = 0; j < vlen; j++) {
@@ -2622,7 +2622,7 @@ static int btf_dedup_ref_type(struct btf_dedup *d, __u32 type_id)
 		t->type = ref_type_id;
 
 		vlen = btf_vlen(t);
-		param = btf_params(t);
+		param = (struct btf_param *) btf_params(t);
 		for (i = 0; i < vlen; i++) {
 			ref_type_id = btf_dedup_ref_type(d, param->type);
 			if (ref_type_id < 0)
@@ -2811,7 +2811,7 @@ static int btf_dedup_remap_type(struct btf_dedup *d, __u32 type_id)
 	}
 
 	case BTF_KIND_FUNC_PROTO: {
-		struct btf_param *param = btf_params(t);
+		struct btf_param *param = (struct btf_param *) btf_params(t);
 		__u16 vlen = btf_vlen(t);
 
 		r = btf_dedup_remap_type_id(d, t->type);
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index cd1bd018ba8b..2817cf7ce2ee 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -286,9 +286,9 @@ static inline __u32 btf_member_bitfield_size(const struct btf_type *t,
 	return kflag ? BTF_MEMBER_BITFIELD_SIZE(m->offset) : 0;
 }
 
-static inline struct btf_param *btf_params(const struct btf_type *t)
+static inline const struct btf_param *btf_params(const struct btf_type *t)
 {
-	return (struct btf_param *)(t + 1);
+	return (const struct btf_param *)(t + 1);
 }
 
 static inline struct btf_var *btf_var(const struct btf_type *t)
-- 
2.21.0


^ permalink raw reply related

* [PATCH 4/7] libbpf: Return const btf_member from btf_members inline function
From: Jiri Olsa @ 2019-09-06  7:31 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Andrii Nakryiko, Yonghong Song, Martin KaFai Lau
In-Reply-To: <20190906073144.31068-1-jolsa@kernel.org>

I'm getting following errors when compiling with -Wcast-qual:

  bpf/btf.h: In function ‘btf_member* btf_members(const btf_type*)’:
  bpf/btf.h:264:36: warning: cast from type ‘const btf_type*’ to type
  ‘btf_member*’ casts away qualifiers [-Wcast-qual]
    264 |  return (struct btf_member *)(t + 1);
        |                                    ^

The argument is const so the cast to following struct btf_member
pointer should be const as well. Casting the const away in btf.c
call where it's correctly used without const in deduplication
code.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/btf.c    | 4 ++--
 tools/lib/bpf/btf.h    | 4 ++--
 tools/lib/bpf/libbpf.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 5a39e9506760..560d1ae33675 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -1463,7 +1463,7 @@ static int btf_for_each_str_off(struct btf_dedup *d, str_off_fn_t fn, void *ctx)
 		switch (btf_kind(t)) {
 		case BTF_KIND_STRUCT:
 		case BTF_KIND_UNION: {
-			struct btf_member *m = btf_members(t);
+			struct btf_member *m = (struct btf_member *) btf_members(t);
 			__u16 vlen = btf_vlen(t);
 
 			for (j = 0; j < vlen; j++) {
@@ -2797,7 +2797,7 @@ static int btf_dedup_remap_type(struct btf_dedup *d, __u32 type_id)
 
 	case BTF_KIND_STRUCT:
 	case BTF_KIND_UNION: {
-		struct btf_member *member = btf_members(t);
+		struct btf_member *member = (struct btf_member *) btf_members(t);
 		__u16 vlen = btf_vlen(t);
 
 		for (i = 0; i < vlen; i++) {
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 3b3216fa348f..cd1bd018ba8b 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -259,9 +259,9 @@ static inline const struct btf_enum *btf_enum(const struct btf_type *t)
 	return (const struct btf_enum *)(t + 1);
 }
 
-static inline struct btf_member *btf_members(const struct btf_type *t)
+static inline const struct btf_member *btf_members(const struct btf_type *t)
 {
-	return (struct btf_member *)(t + 1);
+	return (const struct btf_member *)(t + 1);
 }
 
 /* Get bit offset of a member with specified index. */
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2233f919dd88..7d3d6284dd2e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1389,7 +1389,7 @@ static void bpf_object__sanitize_btf(struct bpf_object *obj)
 		} else if (!has_datasec && btf_is_datasec(t)) {
 			/* replace DATASEC with STRUCT */
 			const struct btf_var_secinfo *v = btf_var_secinfos(t);
-			struct btf_member *m = btf_members(t);
+			struct btf_member *m = (struct btf_member *) btf_members(t);
 			struct btf_type *vt;
 			char *name;
 
-- 
2.21.0


^ permalink raw reply related

* [PATCH 3/7] libbpf: Return const btf_enum from btf_enum inline function
From: Jiri Olsa @ 2019-09-06  7:31 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Andrii Nakryiko, Yonghong Song, Martin KaFai Lau
In-Reply-To: <20190906073144.31068-1-jolsa@kernel.org>

I'm getting following errors when compiling with -Wcast-qual:

  bpf/btf.h: In function ‘btf_enum* btf_enum(const btf_type*)’:
  bpf/btf.h:259:34: warning: cast from type ‘const btf_type*’ to type
  ‘btf_enum*’ casts away qualifier
    259 |  return (struct btf_enum *)(t + 1);
        |                                  ^

The argument is const so the cast to following struct btf_enum
pointer should be const as well. Casting the const away in btf.c
call where it's correctly used without const in deduplication
code.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/btf.c | 2 +-
 tools/lib/bpf/btf.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index d6327bcc713a..5a39e9506760 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -1475,7 +1475,7 @@ static int btf_for_each_str_off(struct btf_dedup *d, str_off_fn_t fn, void *ctx)
 			break;
 		}
 		case BTF_KIND_ENUM: {
-			struct btf_enum *m = btf_enum(t);
+			struct btf_enum *m = (struct btf_enum *) btf_enum(t);
 			__u16 vlen = btf_vlen(t);
 
 			for (j = 0; j < vlen; j++) {
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 6bbf5772aa61..3b3216fa348f 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -254,9 +254,9 @@ static inline const struct btf_array *btf_array(const struct btf_type *t)
 	return (const struct btf_array *)(t + 1);
 }
 
-static inline struct btf_enum *btf_enum(const struct btf_type *t)
+static inline const struct btf_enum *btf_enum(const struct btf_type *t)
 {
-	return (struct btf_enum *)(t + 1);
+	return (const struct btf_enum *)(t + 1);
 }
 
 static inline struct btf_member *btf_members(const struct btf_type *t)
-- 
2.21.0


^ permalink raw reply related

* [PATCH 2/7] libbpf: Return const btf_array from btf_array inline function
From: Jiri Olsa @ 2019-09-06  7:31 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Andrii Nakryiko, Yonghong Song, Martin KaFai Lau
In-Reply-To: <20190906073144.31068-1-jolsa@kernel.org>

I'm getting following errors when compiling with -Wcast-qual:

  bpf/btf.h: In function ‘btf_array* btf_array(const btf_type*)’:
  bpf/btf.h:254:35: warning: cast from type ‘const btf_type*’ to type
  ‘btf_array*’ casts away qualifiers [-Wcast-qual]
    254 |  return (struct btf_array *)(t + 1);
        |                                   ^

The argument is const so the cast to following struct btf_array
pointer should be const as well. Casting the const away in btf.c
calls where it's correctly used without const in deduplication
code.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/btf.c | 4 ++--
 tools/lib/bpf/btf.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 1aa189a9112a..d6327bcc713a 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -2587,7 +2587,7 @@ static int btf_dedup_ref_type(struct btf_dedup *d, __u32 type_id)
 		break;
 
 	case BTF_KIND_ARRAY: {
-		struct btf_array *info = btf_array(t);
+		struct btf_array *info = (struct btf_array *) btf_array(t);
 
 		ref_type_id = btf_dedup_ref_type(d, info->type);
 		if (ref_type_id < 0)
@@ -2782,7 +2782,7 @@ static int btf_dedup_remap_type(struct btf_dedup *d, __u32 type_id)
 		break;
 
 	case BTF_KIND_ARRAY: {
-		struct btf_array *arr_info = btf_array(t);
+		struct btf_array *arr_info = (struct btf_array *) btf_array(t);
 
 		r = btf_dedup_remap_type_id(d, arr_info->type);
 		if (r < 0)
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 952e3496467d..6bbf5772aa61 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -249,9 +249,9 @@ static inline __u8 btf_int_bits(const struct btf_type *t)
 	return BTF_INT_BITS(*(const __u32 *)(t + 1));
 }
 
-static inline struct btf_array *btf_array(const struct btf_type *t)
+static inline const struct btf_array *btf_array(const struct btf_type *t)
 {
-	return (struct btf_array *)(t + 1);
+	return (const struct btf_array *)(t + 1);
 }
 
 static inline struct btf_enum *btf_enum(const struct btf_type *t)
-- 
2.21.0


^ permalink raw reply related

* [PATCH 1/7] libbpf: Use const cast for btf_int_* functions
From: Jiri Olsa @ 2019-09-06  7:31 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Andrii Nakryiko, Yonghong Song, Martin KaFai Lau
In-Reply-To: <20190906073144.31068-1-jolsa@kernel.org>

I'm getting following errors when compiling with -Wcast-qual:

  bpf/btf.h: In function ‘__u8 btf_int_offset(const btf_type*)’:
  bpf/btf.h:244:40: warning: cast from type ‘const btf_type*’ to type
  ‘__u32*’ {aka ‘unsigned int*’} casts away qualifiers [-Wcast-qual]
    244 |  return BTF_INT_OFFSET(*(__u32 *)(t + 1));
        |                                        ^

The argument is const so the cast to following __u32
pointer should be const as well.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/btf.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 9cb44b4fbf60..952e3496467d 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -236,17 +236,17 @@ static inline bool btf_is_datasec(const struct btf_type *t)
 
 static inline __u8 btf_int_encoding(const struct btf_type *t)
 {
-	return BTF_INT_ENCODING(*(__u32 *)(t + 1));
+	return BTF_INT_ENCODING(*(const __u32 *)(t + 1));
 }
 
 static inline __u8 btf_int_offset(const struct btf_type *t)
 {
-	return BTF_INT_OFFSET(*(__u32 *)(t + 1));
+	return BTF_INT_OFFSET(*(const __u32 *)(t + 1));
 }
 
 static inline __u8 btf_int_bits(const struct btf_type *t)
 {
-	return BTF_INT_BITS(*(__u32 *)(t + 1));
+	return BTF_INT_BITS(*(const __u32 *)(t + 1));
 }
 
 static inline struct btf_array *btf_array(const struct btf_type *t)
-- 
2.21.0


^ permalink raw reply related

* [PATCH 0/7] libbpf: Fix cast away const qualifiers in btf.h
From: Jiri Olsa @ 2019-09-06  7:31 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Andrii Nakryiko, Yonghong Song, Martin KaFai Lau

hi,
when including btf.h in bpftrace, I'm getting -Wcast-qual warnings like:

  bpf/btf.h: In function ‘btf_var_secinfo* btf_var_secinfos(const btf_type*)’:
  bpf/btf.h:302:41: warning: cast from type ‘const btf_type*’ to type
  ‘btf_var_secinfo*’ casts away qualifiers [-Wcast-qual]
    302 |  return (struct btf_var_secinfo *)(t + 1);
        |                                         ^

I changed the btf.h header to comply with -Wcast-qual checks
and used const cast away casting in libbpf objects, where it's
all related to deduplication code, so I believe loosing const
is fine there.

thanks,
jirka


---
Jiri Olsa (7):
      libbpf: Use const cast for btf_int_* functions
      libbpf: Return const btf_array from btf_array inline function
      libbpf: Return const btf_enum from btf_enum inline function
      libbpf: Return const btf_member from btf_members inline function
      libbpf: Return const btf_param from btf_params inline function
      libbpf: Return const btf_var from btf_var inline function
      libbpf: Return const struct btf_var_secinfo from btf_var_secinfos inline function

 tools/lib/bpf/btf.c    | 21 +++++++++++----------
 tools/lib/bpf/btf.h    | 30 +++++++++++++++---------------
 tools/lib/bpf/libbpf.c |  2 +-
 3 files changed, 27 insertions(+), 26 deletions(-)

^ permalink raw reply

* Re: [PATCH] net: Remove the source address setting in connect() for UDP
From: Enke Chen (enkechen) @ 2019-09-06  7:23 UTC (permalink / raw)
  To: David Miller
  Cc: kuznet@ms2.inr.ac.ru, yoshfuji@linux-ipv6.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	xe-linux-external(mailer list), Enke Chen (enkechen)
In-Reply-To: <20190906.091350.2133455010162259391.davem@davemloft.net>

Hi, David:

Yes, I understand the code has been there for a long time.  But the issues are real, and it's really nasty when
You run into them.  As I described in the patch log, there is no backward compatibility Issue for fixing it.

---
There is no backward compatibility issue here as the source address setting
in connect() is not needed anyway.

  - No impact on the source address selection when the source address
    is explicitly specified by "bind()", or by the "IP_PKTINFO" option.

  - In the case that the source address is not explicitly specified,
    the selection of the source address would be more accurate and
    reliable based on the up-to-date routing table.
---

Thanks.  -- Enke

-----Original Message-----
From: <linux-kernel-owner@vger.kernel.org> on behalf of David Miller <davem@davemloft.net>
Date: Friday, September 6, 2019 at 12:14 AM
To: "Enke Chen (enkechen)" <enkechen@cisco.com>
Cc: "kuznet@ms2.inr.ac.ru" <kuznet@ms2.inr.ac.ru>, "yoshfuji@linux-ipv6.org" <yoshfuji@linux-ipv6.org>, "netdev@vger.kernel.org" <netdev@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "xe-linux-external(mailer list)" <xe-linux-external@cisco.com>
Subject: Re: [PATCH] net: Remove the source address setting in connect() for UDP

From: Enke Chen <enkechen@cisco.com>
Date: Thu,  5 Sep 2019 19:54:37 -0700

> The connect() system call for a UDP socket is for setting the destination
> address and port. But the current code mistakenly sets the source address
> for the socket as well. Remove the source address setting in connect() for
> UDP in this patch.

Do you have any idea how many decades of precedence this behavior has and
therefore how much you potentially will break userspace?

This boat has sailed a long time ago I'm afraid.


^ permalink raw reply

* Re: [PATCH net] tcp: ulp: fix possible crash in tcp_diag_get_aux_size()
From: Davide Caratti @ 2019-09-06  7:17 UTC (permalink / raw)
  To: Eric Dumazet, David S . Miller
  Cc: netdev, Eric Dumazet, Luke Hsiao, Neal Cardwell
In-Reply-To: <20190905202041.138085-1-edumazet@google.com>

On Thu, 2019-09-05 at 13:20 -0700, Eric Dumazet wrote:
> tcp_diag_get_aux_size() can be called with sockets in any state.
> 
> icsk_ulp_ops is only present for full sockets.
> 
> For SYN_RECV or TIME_WAIT ones we would access garbage.
> 

hello Eric, 

thanks for fixing this!

Acked-by: Davide Caratti <dcaratti@redhat.com>



^ permalink raw reply

* Re: [iproute2, master 2/2] devlink: Add a new time-stamp format for health reporter's dump
From: Aya Levin @ 2019-09-06  7:17 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev@vger.kernel.org, Jiri Pirko, Moshe Shemesh
In-Reply-To: <20190829162722.6275fb02@hermes.lan>



On 8/30/2019 2:27 AM, Stephen Hemminger wrote:
> On Thu, 22 Aug 2019 14:05:42 +0300
> Aya Levin <ayal@mellanox.com> wrote:
> 
>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>> index fc195cbd66f4..3f8532711315 100644
>> --- a/include/uapi/linux/devlink.h
>> +++ b/include/uapi/linux/devlink.h
>> @@ -348,6 +348,8 @@ enum devlink_attr {
>>   	DEVLINK_ATTR_PORT_PCI_PF_NUMBER,	/* u16 */
>>   	DEVLINK_ATTR_PORT_PCI_VF_NUMBER,	/* u16 */
>>   
>> +	DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TSPEC,
>> +
>>   	/* add new attributes above here, update the policy in devlink.c */
>>   
>>   	__DEVLINK_ATTR_MAX,
>> -- 
> 
> Since this is not upstream, this patch needs to go to iproute2-next.
> Which means if you want the other bug fix, send it again against master.
Thanks,
Will do that
> 

^ permalink raw reply

* Re: [PATCH] rtl8xxxu: add bluetooth co-existence support for single antenna
From: Kalle Valo @ 2019-09-06  7:17 UTC (permalink / raw)
  To: Chris Chiu
  Cc: Jes Sorensen, David Miller, linux-wireless, netdev, Linux Kernel,
	Linux Upstreaming Team
In-Reply-To: <CAB4CAwc5OBUWFThh__FedmG=fR-_1_GxUuiAb0J5yfU8c1aTfg@mail.gmail.com>

Chris Chiu <chiu@endlessm.com> writes:

> Gentle ping. Cheers.

Please edit your quotes. Including the full patch in quotes makes my use
of patchwork horrible:

https://patchwork.kernel.org/patch/11127227/

-- 
Kalle Valo

^ permalink raw reply

* Re: [iproute2, master 1/2] devlink: Print health reporter's dump time-stamp in a helper function
From: Aya Levin @ 2019-09-06  7:17 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev@vger.kernel.org, Jiri Pirko, Moshe Shemesh
In-Reply-To: <20190829162533.25606191@hermes.lan>



On 8/30/2019 2:25 AM, Stephen Hemminger wrote:
> On Thu, 22 Aug 2019 14:05:41 +0300
> Aya Levin <ayal@mellanox.com> wrote:
> 
>> Add pr_out_dump_reporter prefix to the helper function's name and
>> encapsulate the print in it.
>>
>> Fixes: 2f1242efe9d0 ("devlink: Add devlink health show command")
>> Signed-off-by: Aya Levin <ayal@mellanox.com>
>> Acked-by: Jiri Pirko <jiri@mellanox.com>
> 
> 
> Looks fine, but devlink needs to be converted from doing JSON
> printing its own way and use common iproute2 libraries.
Sorry for the late response.
You are correct, it is in our plans to complete a full transition to 
common iproute2 helpers in the following weeks.
I got your point, and will not submit more patches adding new print 
functions using the current API and will wait for submission after the 
transition to the common API.
This patch will be re-submitted after the transition.
> 

^ permalink raw reply

* Re: [PATCH] net: Remove the source address setting in connect() for UDP
From: David Miller @ 2019-09-06  7:13 UTC (permalink / raw)
  To: enkechen; +Cc: kuznet, yoshfuji, netdev, linux-kernel, xe-linux-external
In-Reply-To: <20190906025437.613-1-enkechen@cisco.com>

From: Enke Chen <enkechen@cisco.com>
Date: Thu,  5 Sep 2019 19:54:37 -0700

> The connect() system call for a UDP socket is for setting the destination
> address and port. But the current code mistakenly sets the source address
> for the socket as well. Remove the source address setting in connect() for
> UDP in this patch.

Do you have any idea how many decades of precedence this behavior has and
therefore how much you potentially will break userspace?

This boat has sailed a long time ago I'm afraid.

^ permalink raw reply

* Re: [PATCH V2 4/4] crypto: Add Xilinx AES driver
From: Corentin Labbe @ 2019-09-06  7:04 UTC (permalink / raw)
  To: Kalyani Akula
  Cc: herbert@gondor.apana.org.au, kstewart@linuxfoundation.org,
	gregkh@linuxfoundation.org, tglx@linutronix.de,
	pombredanne@nexb.com, linux-crypto@vger.kernel.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	Sarat Chand Savitala
In-Reply-To: <BN7PR02MB512445C31936CED70F02D15AAFB80@BN7PR02MB5124.namprd02.prod.outlook.com>

On Wed, Sep 04, 2019 at 05:40:22PM +0000, Kalyani Akula wrote:
> Hi Corentin,
> 
> Thanks for the review comments.
> Please find my response/queries inline.
> 
> > -----Original Message-----
> > From: Corentin Labbe <clabbe.montjoie@gmail.com>
> > Sent: Monday, September 2, 2019 12:29 PM
> > To: Kalyani Akula <kalyania@xilinx.com>
> > Cc: herbert@gondor.apana.org.au; kstewart@linuxfoundation.org;
> > gregkh@linuxfoundation.org; tglx@linutronix.de; pombredanne@nexb.com;
> > linux-crypto@vger.kernel.org; linux-kernel@vger.kernel.org;
> > netdev@vger.kernel.org; Kalyani Akula <kalyania@xilinx.com>
> > Subject: Re: [PATCH V2 4/4] crypto: Add Xilinx AES driver
> > 
> > On Sun, Sep 01, 2019 at 07:24:58PM +0530, Kalyani Akula wrote:
> > > This patch adds AES driver support for the Xilinx ZynqMP SoC.
> > >
> > > Signed-off-by: Kalyani Akula <kalyani.akula@xilinx.com>
> > > ---
> > 
> > Hello
> > 
> > I have some comment below
> > 
> > >  drivers/crypto/Kconfig          |  11 ++
> > >  drivers/crypto/Makefile         |   1 +
> > >  drivers/crypto/zynqmp-aes-gcm.c | 297
> > > ++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 309 insertions(+)
> > >  create mode 100644 drivers/crypto/zynqmp-aes-gcm.c
> > >
> > > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index
> > > 603413f..a0d058a 100644
> > > --- a/drivers/crypto/Kconfig
> > > +++ b/drivers/crypto/Kconfig
> > > @@ -677,6 +677,17 @@ config CRYPTO_DEV_ROCKCHIP
> > >  	  This driver interfaces with the hardware crypto accelerator.
> > >  	  Supporting cbc/ecb chainmode, and aes/des/des3_ede cipher mode.
> > >
> > > +config CRYPTO_DEV_ZYNQMP_AES
> > > +	tristate "Support for Xilinx ZynqMP AES hw accelerator"
> > > +	depends on ARCH_ZYNQMP || COMPILE_TEST
> > > +	select CRYPTO_AES
> > > +	select CRYPTO_SKCIPHER
> > > +	help
> > > +	  Xilinx ZynqMP has AES-GCM engine used for symmetric key
> > > +	  encryption and decryption. This driver interfaces with AES hw
> > > +	  accelerator. Select this if you want to use the ZynqMP module
> > > +	  for AES algorithms.
> > > +
> > >  config CRYPTO_DEV_MEDIATEK
> > >  	tristate "MediaTek's EIP97 Cryptographic Engine driver"
> > >  	depends on (ARM && ARCH_MEDIATEK) || COMPILE_TEST diff --git
> > > a/drivers/crypto/Makefile b/drivers/crypto/Makefile index
> > > afc4753..c99663a 100644
> > > --- a/drivers/crypto/Makefile
> > > +++ b/drivers/crypto/Makefile
> > > @@ -48,3 +48,4 @@ obj-$(CONFIG_CRYPTO_DEV_BCM_SPU) += bcm/
> > >  obj-$(CONFIG_CRYPTO_DEV_SAFEXCEL) += inside-secure/
> > >  obj-$(CONFIG_CRYPTO_DEV_ARTPEC6) += axis/  obj-y += hisilicon/
> > > +obj-$(CONFIG_CRYPTO_DEV_ZYNQMP_AES) += zynqmp-aes-gcm.o
> > > diff --git a/drivers/crypto/zynqmp-aes-gcm.c
> > > b/drivers/crypto/zynqmp-aes-gcm.c new file mode 100644 index
> > > 0000000..d65f038
> > > --- /dev/null
> > > +++ b/drivers/crypto/zynqmp-aes-gcm.c
> > > @@ -0,0 +1,297 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Xilinx ZynqMP AES Driver.
> > > + * Copyright (c) 2019 Xilinx Inc.
> > > + */
> > > +
> > > +#include <crypto/aes.h>
> > > +#include <crypto/scatterwalk.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of_device.h>
> > > +#include <linux/scatterlist.h>
> > > +#include <linux/firmware/xlnx-zynqmp.h>
> > > +
> > > +#define ZYNQMP_AES_IV_SIZE			12
> > > +#define ZYNQMP_AES_GCM_SIZE			16
> > > +#define ZYNQMP_AES_KEY_SIZE			32
> > > +
> > > +#define ZYNQMP_AES_DECRYPT			0
> > > +#define ZYNQMP_AES_ENCRYPT			1
> > > +
> > > +#define ZYNQMP_AES_KUP_KEY			0
> > > +#define ZYNQMP_AES_DEVICE_KEY			1
> > > +#define ZYNQMP_AES_PUF_KEY			2
> > > +
> > > +#define ZYNQMP_AES_GCM_TAG_MISMATCH_ERR		0x01
> > > +#define ZYNQMP_AES_SIZE_ERR			0x06
> > > +#define ZYNQMP_AES_WRONG_KEY_SRC_ERR		0x13
> > > +#define ZYNQMP_AES_PUF_NOT_PROGRAMMED		0xE300
> > > +
> > > +#define ZYNQMP_AES_BLOCKSIZE			0x04
> > > +
> > > +static const struct zynqmp_eemi_ops *eemi_ops; struct zynqmp_aes_dev
> > > +*aes_dd;
> > 
> > I still think that using a global variable for storing device driver data is bad.
> 
> I think storing the list of dd's would solve up the issue with global variable, but there is only one AES instance here.
> Please suggest
> 

Look what I do for amlogic driver https://patchwork.kernel.org/patch/11059633/.
I store the device driver in the instatiation of a crypto template.

[...]
> > > +static int zynqmp_setkey_blk(struct crypto_tfm *tfm, const u8 *key,
> > > +			     unsigned int len)
> > > +{
> > > +	struct zynqmp_aes_op *op = crypto_tfm_ctx(tfm);
> > > +
> > > +	if (((len != 1) && (len !=  ZYNQMP_AES_KEY_SIZE)) || (!key))
> > 
> > typo, two space
> 
> Will fix in the next version
> 
> > 
> > > +		return -EINVAL;
> > > +
> > > +	if (len == 1) {
> > > +		op->keytype = *key;
> > > +
> > > +		if ((op->keytype < ZYNQMP_AES_KUP_KEY) ||
> > > +			(op->keytype > ZYNQMP_AES_PUF_KEY))
> > > +			return -EINVAL;
> > > +
> > > +	} else if (len == ZYNQMP_AES_KEY_SIZE) {
> > > +		op->keytype = ZYNQMP_AES_KUP_KEY;
> > > +		op->keylen = len;
> > > +		memcpy(op->key, key, len);
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > 
> > It seems your driver does not support AES keysize of 128/196, you need to
> > fallback in that case.
> 
> [Kalyani] In case of 128/196 keysize, returning the error would suffice ?
> Or still algorithm need to work ?
> If error is enough, it is taken care by this condition 
> if (((len != 1) && (len !=  ZYNQMP_AES_KEY_SIZE)) || (!key))

I think this problem just simply show us that your driver is not tested against crypto selftest.
I have tried to refuse 128/196 in my driver and I get:
alg: skcipher: cbc-aes-sun8i-ce setkey failed on test vector 0; expected_error=0, actual_error=-22, flags=0x1

So if your hardware lack 128/196 keys support, you must fallback to a software version.

Anyway please test your driver with crypto selftest enabled (and also CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y)

Regards

^ permalink raw reply

* Re: [PATCH net] net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list
From: Shmulik Ladkani @ 2019-09-06  6:47 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Daniel Borkmann, Eric Dumazet, eyal, netdev, Shmulik Ladkani,
	Alexander Duyck
In-Reply-To: <CAF=yD-J9Ax9f7BsGBFAaG=QU6CPVw6sSzBkZJOHRW-m6o49oyw@mail.gmail.com>

On Thu, 5 Sep 2019 17:51:20 -0400
Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:

> On Thu, Sep 5, 2019 at 2:36 PM Shmulik Ladkani <shmulik@metanetworks.com> wrote:
> >
> > +       if (mss != GSO_BY_FRAGS &&
> > +           (skb_shinfo(head_skb)->gso_type & SKB_GSO_DODGY)) {
> > +               /* gso_size is untrusted.
> > +                *
> > +                * If head_skb has a frag_list with a linear non head_frag
> > +                * item, and head_skb's headlen does not fit requested
> > +                * gso_size, fall back to copying the skbs - by disabling sg.
> > +                *
> > +                * We assume checking the first frag suffices, i.e if either of
> > +                * the frags have non head_frag data, then the first frag is
> > +                * too.
> > +                */
> > +               if (list_skb && skb_headlen(list_skb) && !list_skb->head_frag &&
> > +                   (mss != skb_headlen(head_skb) - doffset)) {  
> 
> I thought the idea was to check skb_headlen(list_skb), as that is the
> cause of the problem. Is skb_headlen(head_skb) a good predictor of
> that? I can certainly imagine that it is, just not sure.

Yes, 'mss != skb_headlen(HEAD_SKB)' seems to be a very good predictor,
both for the test reproducer, and what's observered on a live system.

We *CANNOT* use 'mss != skb_headlen(LIST_SKB)' as the test condition.
The packet could have just a SINGLE frag_list member, and that member could
be a "small remainder" not reaching the full mss size - so we could hit
the test condition EVEN FOR NON gso_size mangled frag_list skbs -
which is not desired.

Also, is we test 'mss != skb_headlen(list_skb)' and execute 'sg=false'
ONLY IF 'list_skb' is *NOT* the last item, this is still bogus.
Imagine a gso_size mangled packet having just head_skb and a single
"small remainder" frag. This packet will hit the BUG_ON, as the
'sg=false' solution is now skipped according to the revised condition.

> Thanks for preparing the patch, and explaining the problem and
> solution clearly in the commit message. I'm pretty sure I'll have
> forgotten the finer details next time we have to look at this
> function again.

Indeed. Apparently I've been there myself few years back and forgot all
the gritty details :) see [0]

[0] https://patchwork.ozlabs.org/patch/661419/ 

^ permalink raw reply

* Re: [PATCH net] net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list
From: Shmulik Ladkani @ 2019-09-06  6:20 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Daniel Borkmann, Eric Dumazet, Willem de Bruijn, eyal, netdev,
	Shmulik Ladkani
In-Reply-To: <CAKgT0Uf-OvKKycJz7aTZ93J=RdUuwd=SFS9C9pTngieDxe0uYQ@mail.gmail.com>

On Thu, 5 Sep 2019 14:49:44 -0700
Alexander Duyck <alexander.duyck@gmail.com> wrote:

> I would change the order of the tests you use here so that we can
> eliminate the possibility of needing to perform many tests for the
> more common cases. You could probably swap "list_skb" and "mss !=
> GSO_BY_FRAGS" since list_skb is more likely to be false for many of
> the common cases such as a standard TSO send from a socket. You might
> even consider moving the GSO_BY_FRAGS check toward the end of your
> checks since SCTP is the only protocol that I believe uses it and the
> likelihood of encountering it is much lower compared to other
> protocols.
> 
> You could probably test for !list_skb->head_frag before seeing if
> there is a headlen since many NICs would be generating frames using
> head_frag, so in the GRO case you mentioned above it could probably
> save you some effort on a number of NICs.
> 
> You might also consider moving this code up before we push the mac
> header back on and instead of setting sg to false you could just clear
> the NETIF_F_SG flag from features. It would save you from having to
> then remove doffset in your last check.

Thanks Alexander for the input. Will encorporate as much as possible
into a v2 patch.

BTW, I've strugged with myself regarding order of tests, and came
up with this suggestion, as my motivation was to have the tests order
tell a coherent logical story when read top-to-bottom left-to-right, FWIW.
For example, although 'mss != skb_headlen(head_skb)' could be tested
earlier, the condition by itself isn't meaningful unless we have an
existing frag_list and with a !head_frag.

Best
Shmulik

^ permalink raw reply

* Re: [BACKPORT 4.14.y v2 5/6] ppp: mppe: Revert "ppp: mppe: Add softdep to arc4"
From: Baolin Wang @ 2019-09-06  6:13 UTC (permalink / raw)
  To: Baolin Wang, # 3.4.x, paulus, linux-ppp, Networking,
	Arnd Bergmann, Orson Zhai, Vincent Guittot, LKML
In-Reply-To: <20190905161642.GA5659@google.com>

On Fri, 6 Sep 2019 at 00:16, Eric Biggers <ebiggers@google.com> wrote:
>
> On Thu, Sep 05, 2019 at 11:10:45AM +0800, Baolin Wang wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > [Upstream commit 25a09ce79639a8775244808c17282c491cff89cf]
> >
> > Commit 0e5a610b5ca5 ("ppp: mppe: switch to RC4 library interface"),
> > which was merged through the crypto tree for v5.3, changed ppp_mppe.c to
> > use the new arc4_crypt() library function rather than access RC4 through
> > the dynamic crypto_skcipher API.
> >
> > Meanwhile commit aad1dcc4f011 ("ppp: mppe: Add softdep to arc4") was
> > merged through the net tree and added a module soft-dependency on "arc4".
> >
> > The latter commit no longer makes sense because the code now uses the
> > "libarc4" module rather than "arc4", and also due to the direct use of
> > arc4_crypt(), no module soft-dependency is required.
> >
> > So revert the latter commit.
> >
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> > ---
> >  drivers/net/ppp/ppp_mppe.c |    1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
> > index d9eda7c..6c7fd98 100644
> > --- a/drivers/net/ppp/ppp_mppe.c
> > +++ b/drivers/net/ppp/ppp_mppe.c
> > @@ -63,7 +63,6 @@
> >  MODULE_DESCRIPTION("Point-to-Point Protocol Microsoft Point-to-Point Encryption support");
> >  MODULE_LICENSE("Dual BSD/GPL");
> >  MODULE_ALIAS("ppp-compress-" __stringify(CI_MPPE));
> > -MODULE_SOFTDEP("pre: arc4");
>
> Why is this being backported?  This revert was only needed because of a
> different patch that was merged in v5.3, as I explained in the commit message.

Sorry I missed this. I should remove this patch from our product kernel too.

-- 
Baolin Wang
Best Regards

^ permalink raw reply

* Re: [patch net-next] net: fib_notifier: move fib_notifier_ops from struct net into per-net struct
From: Jiri Pirko @ 2019-09-06  6:05 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, davem, idosch, dsahern, mlxsw
In-Reply-To: <bb24e9d5-24c6-d590-e490-be2226016288@gmail.com>

Fri, Sep 06, 2019 at 07:54:39AM CEST, eric.dumazet@gmail.com wrote:
>
>
>On 9/5/19 8:06 PM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> No need for fib_notifier_ops to be in struct net. It is used only by
>> fib_notifier as a private data. Use net_generic to introduce per-net
>> fib_notifier struct and move fib_notifier_ops there.
>> 
>>
>
>...
>
>>  static struct pernet_operations fib_notifier_net_ops = {
>>  	.init = fib_notifier_net_init,
>>  	.exit = fib_notifier_net_exit,
>> +	.id = &fib_notifier_net_id,
>> +	.size = sizeof(struct fib_notifier_net),
>>  };
>>  
>>  static int __init fib_notifier_init(void)
>> 
>
>Note that this will allocate a block of memory (in ops_init()) to hold this,
>plus a second one to hold the pointer to this block.
>
>Due to kmalloc() constraints, this block will use more memory.

I'm aware. But we have net_generic for exactly this purpose not to
pullute struct net.


>
>Not sure your patch is a win, since it makes things a bit more complex.
>
>Is it a preparation patch so that you can add later other fields in struct fib_notifier_net ?

Yes.


>

^ 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